What does a review assess?

A minimal-API endpoint is a `static` (or local) handler function that takes its dependencies as parameters (`HttpContext`, registered services, route/body values) via ASP.NET Core's parameter binding, and returns a typed `IResult` (`Results.Ok`, `Results.Created`, `Results.NoContent`, ...). Route registration (`app.MapGet`/`MapPost`/...) stays a one-line pointer to that handler.

Why it matters

`backend/QualityStudio.Api/Program.cs` already establishes this shape consistently (`Guidelines`, `InstallGuideline`, `CreateGuideline`, ...): the handler is independently testable without spinning up the HTTP pipeline, dependencies are explicit in the signature instead of pulled from an ambient service locator, and every route returns the same small set of typed results the framework can serialize predictably.

How to detect it

Look at the `app.Map*` registrations for inline lambdas with bodies longer than one expression, and at handlers for `IServiceProvider`/`GetRequiredService` use, `HttpContext.Response` written by hand, or a return type that is not `IResult`/`Task<IResult>`. A one-line lambda that forwards to a named handler is the intended shape.

Severity: medium. Deterministic mappings: Reviewer judgment; no deterministic mapping.

static IResult InstallGuideline(HttpContext context, string catalogueId, RepositoryRegistry registry, GuidelineStore store)
{
    var (_, repository) = ResolveRepository(context, registry);
    var installed = store.Install(repository.Root, catalogueId);
    return Results.Created($"{context.Request.PathBase}/api/guidelines/{Uri.EscapeDataString(installed.Id)}", installed);
}

app.MapPost("/api/widgets", async (HttpContext context) =>
{
    var store = context.RequestServices.GetRequiredService<WidgetStore>(); // service-locator pull
    var body = await JsonSerializer.DeserializeAsync<WidgetRequest>(context.Request.Body);
    await store.SaveAsync(body!);
    context.Response.StatusCode = 201; // untyped result, no IResult
});

Register a service as `Singleton` only when it is stateless or its internal state is safe to share across all requests; register per-operation or short-lived collaborators as `Transient` (or `Scoped` where request-scoped state is genuinely needed). Consume dependencies through constructor (or primary-constructor) parameters — never resolve them ad hoc via `IServiceProvider.GetService`/`GetRequiredService` inside a class that could take them as constructor parameters instead.

Why it matters

`backend/QualityStudio.Api/Program.cs` registers `GuidelineStore` as `Singleton` (stateless, delegates to the filesystem per call) but `GuidelineImpactAnalyzer` as `Transient` (does per-analysis work); mismatching this — e.g. making a per-request analyzer a singleton — risks leaking state across unrelated requests. Constructor injection keeps a class's true dependencies visible in its signature and keeps it trivially constructible in unit tests without a DI container.

How to detect it

Read the `builder.Services.Add*` registration next to the type's actual state. Flag `AddSingleton` on a type holding per-operation mutable fields, and any `GetService`/`GetRequiredService` call inside a class that already has a constructor able to take the dependency. `IServiceProvider` use inside composition-root code is not a violation.

Severity: medium. Deterministic mappings: Reviewer judgment; no deterministic mapping.

// Program.cs
builder.Services.AddSingleton<GuidelineStore>();
builder.Services.AddTransient<GuidelineImpactAnalyzer>();

// ReviewJobs.cs — dependencies arrive as primary-constructor parameters
public sealed class ReviewExecutorFactory(
    SensorRegistry sensors,
    StalenessEvaluator stalenessEvaluator) : IReviewExecutorFactory

public sealed class ReviewExecutorFactory : IReviewExecutorFactory
{
    public IReviewExecutor Create(IServiceProvider provider, ...) =>
        new ReviewExecutor(new ReviewRunner(sensorRegistry: provider.GetRequiredService<SensorRegistry>()));
        // hides the real dependency behind a service-locator pull instead of a constructor parameter
}

Every `async` method that does I/O accepts a `CancellationToken` and passes it to every awaited call that accepts one. `async` methods return `Task`/`Task<T>` (or `ValueTask`/`ValueTask<T>`) — never `async void`, except for a framework-mandated event handler. Do not block on async work with `.Result`, `.Wait()`, or `GetAwaiter().GetResult()`.

Why it matters

`GuidelineImpactAnalyzer.AnalyzeAsync(string, GuidelineImpactRequest, CancellationToken, ...)` and `ReviewRunner`'s `Async` methods thread a `CancellationToken` through every awaited call so a caller can actually cancel a long-running review. `async void` swallows exceptions instead of surfacing them on the returned task, and sync-over-async blocking can deadlock a request thread under load — both defeat the cancellation and error-propagation guarantees the rest of the codebase already relies on.

How to detect it

Search for `async void`, `.Result`, `.Wait()`, `GetAwaiter().GetResult()`, and for awaited calls that have a `CancellationToken` overload but are called without one while a token is in scope. Also flag an `async` method doing I/O whose signature takes no `CancellationToken`.

Severity: high. Deterministic mappings: Reviewer judgment; no deterministic mapping.

public virtual async Task<GuidelineImpactResult> AnalyzeAsync(
    string repositoryRoot, GuidelineImpactRequest request, CancellationToken cancellationToken)
{
    var content = await File.ReadAllTextAsync(path, cancellationToken).ConfigureAwait(false);
    // ...
}

public async void Refresh() // async void: exceptions never surface to the caller
{
    var content = File.ReadAllText(path); // blocking I/O on an async method
    var result = LoadAsync().Result;      // sync-over-async: can deadlock
}

A test method name describes the observable behavior being verified (e.g. `Project_input_overrides_global_by_id`), not the method under test. Each test arranges its own isolated fixture (a fresh temp directory, in-memory store, etc.), performs one action, and asserts on outcomes — it does not depend on state left behind by another test or on execution order. Dispose owned resources (implement `IDisposable` when a fixture creates files/directories).

Why it matters

`InputResolverTests` creates a unique temp directory per test instance and implements `IDisposable` to clean it up, and every test method name states the behavior under test rather than just naming the method it calls. This is what makes the suite safe to run in parallel and lets a failing test name alone tell a reader what broke, without opening the test body first.

How to detect it

Check that the test class owns its fixture (a per-instance temp path, its own store) and disposes it, that the method name states a behavior rather than a member name, and that no test reads or writes state another test created. Shared immutable fixture data is not a violation.

Severity: low. Deterministic mappings: Reviewer judgment; no deterministic mapping.

public sealed class InputResolverTests : IDisposable
{
    private readonly string root = Path.Combine(Path.GetTempPath(), "quality-input-tests", Guid.NewGuid().ToString("N"));

    [Fact]
    public void Project_input_overrides_global_by_id()
    {
        Write(global, "rules.md", "rules", "all", "all", 10, "global body");
        Write(Project, "rules.md", "rules", "code", "file", 1, "project body");

        var result = new InputResolver().Resolve(root, "code", ReviewLevel.File, global);

        Assert.Equal("project body", Assert.Single(result.Inputs).Content);
    }
}

[Fact]
public void Test1() // name describes nothing; shares a hard-coded path with other tests
{
    Directory.CreateDirectory("/tmp/shared-fixture");
    var resolver = new InputResolver();
    // ... asserts three unrelated behaviors in one test, no cleanup
}

A path that reaches the filesystem from a request, a configuration file, or a stored document is canonicalised with `Path.GetFullPath`, checked to sit under its configured root, and walked segment by segment to reject symbolic links and junctions — through `PathConfinement`, not through a second implementation. Reject rooted paths and `..` segments instead of normalising them away silently.

Why it matters

`RepositoryAccess.NormalizeRelativePath` and `RepositoryAccess.ResolveFile` funnel every Studio file read through `PathConfinement.IsWithin` and `PathConfinement.RejectReparseTraversal`, which is why `GET /api/file?path=../other-repo/Second.cs` answers 400 instead of serving another repository's source. A prefix comparison on its own is not enough on Windows, where case differs and a junction inside the root points anywhere; a second, slightly different copy of the check is how one call site ends up with the weaker half.

How to detect it

Look for `Path.Combine` or `Path.GetFullPath` on a value that arrived from outside the process followed by a file or directory operation with no containment check, for `StartsWith` comparisons that skip the trailing separator or use the wrong `StringComparison`, and for a private `IsWithin`/`ContainedPath` helper duplicating `PathConfinement`. A path built entirely from constants is not a violation.

Severity: critical. Deterministic mappings: Reviewer judgment; no deterministic mapping.

// backend/QualityStudio.Api/PathConfinement.cs
public static void RejectReparseTraversal(string root, string candidate)
{
    if (!IsWithin(root, candidate)) throw new ArgumentException("Path escapes its configured root.");
    var current = Path.GetFullPath(root);
    foreach (var segment in Path.GetRelativePath(root, candidate).Split(
                 [Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar], StringSplitOptions.RemoveEmptyEntries))
    {
        current = Path.Combine(current, segment);
        if (File.GetAttributes(current).HasFlag(FileAttributes.ReparsePoint))
            throw new ArgumentException("Paths cannot traverse symbolic links or junctions.");
    }
}

var absolute = Path.Combine(repositoryRoot, request.Path);   // ".." is still in there
if (absolute.StartsWith(repositoryRoot))                     // no separator, no case rule, no link check
{
    return await File.ReadAllTextAsync(absolute, cancellationToken);
}

The executable of a spawned process comes from code or a host-owned allowlist, never from repository or request data. Arguments go into `ProcessStartInfo.ArgumentList` one at a time — never a concatenated `Arguments` string — with `UseShellExecute = false`, `CreateNoWindow = true`, redirected streams, and a `CancellationToken` on the wait.

Why it matters

`ProcessSensorCommandRunner.RunAsync`, `GitleaksSecurityScanner.RunScanAsync`, and `GitleaksBinaryResolver.RunVersionAsync` all use `ArgumentList`, so a repository path containing a space or a quote is an argument and not a new command. The executable is the part that argument escaping cannot protect: a configured command string lets a repository name a shell, and that process reads beyond its working directory, inherits the host's credentials, reaches the network, and writes host-visible files — the whole confinement story above it is then decoration.

How to detect it

Look for `ProcessStartInfo.Arguments` assigned from an interpolated string, for the executable being read from configuration, a request, or a repository file, and for `WaitForExit()` without a token or a timeout. `ArgumentList.Add` on values that only reach the child as data is the intended shape; the file name is what must be fixed.

Severity: critical. Deterministic mappings: Reviewer judgment; no deterministic mapping.

// backend/AgentOrchestrator.CodeQuality/DependencyVulnerabilitySensor.cs
StartInfo = new ProcessStartInfo(executable)
{
    WorkingDirectory = workingDirectory,
    RedirectStandardOutput = true, RedirectStandardError = true,
    UseShellExecute = false, CreateNoWindow = true,
},
...
foreach (var argument in arguments) process.StartInfo.ArgumentList.Add(argument);
await process.WaitForExitAsync(cancellationToken).ConfigureAwait(false);

var command = configuration.AnalyzerCommand;              // free-form, repository-owned
var parts = command.Split(' ');
using var process = Process.Start(new ProcessStartInfo(parts[0])
{
    Arguments = string.Join(' ', parts.Skip(1)) + " " + target,   // one quote away from a new command
});
process!.WaitForExit();                                   // no token, no timeout

A secret a scanner found is never read into a model that can be written, logged, or returned. An exception message, a filesystem path, and a child process's output stay in the log; a caller gets a stable title and a status code. Delete a temporary report that held sensitive output in a `finally`.

Why it matters

`GitleaksSecurityScanner.ParseJsonFinding` reads the rule id, the location, the description, and the fingerprint and never touches gitleaks' `Secret` or `Match` fields, and `BuildFinding` passes `null` for the finding's evidence, so the secret is structurally absent from anything Quality Studio persists — redaction that cannot be forgotten later. The API mirrors it: only `ReviewModelSelectionException.Message` is echoed to a caller so a path or an internal detail is never handed back.

How to detect it

Look for a scanner's secret-bearing field being mapped into a record, for `exception.Message`, `exception.ToString()`, a full path, or a child process's stdout or stderr flowing into an HTTP response or a persisted document, and for a temporary file holding scanner output that is not deleted on every path. Logging the same detail through the logger is the intended shape.

Severity: high. Deterministic mappings: Reviewer judgment; no deterministic mapping.

// backend/AgentOrchestrator.CodeQuality/GitleaksSecurityScanner.cs
process.StartInfo.ArgumentList.Add("--redact=100");
// ParseJsonFinding reads RuleID, File, the range, Description and Fingerprint —
// never the Secret or Match fields the report also carries.
return new SecurityFindingRecord(ruleId, severity, description, location, Evidence: null, path, accepted);

catch (IOException exception)
{
    // Hands the caller the host path and the scanner's raw output, secret included.
    return Results.Problem(detail: exception.ToString() + "\n" + process.StandardOutput.ReadToEnd());
}

Deserializing a document that came from a repository, a request, or another process checks its schema id and version before its content is used, rejects members the contract does not declare (`JsonUnmappedMemberHandling.Disallow`), and bounds what it will read — a size limit before the bytes are loaded, an explicit `MaxDepth`, and no `AllowTrailingCommas` shortcut.

Why it matters

`QualityRunReport` sets `UnmappedMemberHandling.Disallow` and refuses a document whose `schemaVersion` or `$schema` is not the one it understands, so an old or foreign document fails loudly instead of binding half its fields; `ReviewMetaContract`, `QualityFindingContract`, and `FindingStateStore` follow the same shape. The missing half is size: reading a repository file into a string with no cap lets one caller exhaust process memory by repeating the request, and nothing in this codebase sets `MaxDepth`, so the 64-level default is the only depth bound there is.

How to detect it

Look for `JsonSerializer.Deserialize`, `JsonDocument.Parse`, or `File.ReadAllText`/`ReadAllBytes` on a path or stream the process does not own, and check for three things before the result is used: a schema and version gate, unmapped-member rejection, and a byte or length limit. A round-trip of a document this process just wrote is not a violation.

Severity: high. Deterministic mappings: Reviewer judgment; no deterministic mapping.

// backend/AgentOrchestrator.CodeQuality/QualityRunReport.cs
private static JsonSerializerOptions CreateOptions() => new(JsonSerializerDefaults.Web)
{
    WriteIndented = true,
    Encoder = JavaScriptEncoder.Default,
    UnmappedMemberHandling = JsonUnmappedMemberHandling.Disallow,
};

if (report.SchemaVersion != 1 || !string.Equals(report.Schema, SchemaId, StringComparison.Ordinal))
    throw new JsonException("Unsupported quality run report schema.");

// No size check, no depth bound, no version gate: the file decides how much memory this costs.
var text = await File.ReadAllTextAsync(sidecarPath, cancellationToken);
var meta = JsonSerializer.Deserialize<ReviewMeta>(text)!;
return meta.Findings;

Code that runs while a request is open uses the asynchronous file, stream, and process APIs and awaits them with a `CancellationToken`. No `File.ReadAllText`, `StandardOutput.ReadToEnd`, `WaitForExit()`, or `.Result` on a request path — and none of them inside a `lock`, which converts one slow call into a queue for every other caller of the same gate.

Why it matters

Quality Studio's browser contract is under 100 ms to a visible transition and under 500 ms to a usable dashboard, and `PERF.md` measures a cold hierarchy scan at 85% of a repository switch, so a request path has no room for a thread parked on a disk or a child process. Blocking inside a held lock is the worse half: `RepositoryHierarchyCache` reads every changed file while holding the slot gate, which makes a multi-second scan a multi-second wait for every concurrent switch of that repository, not just the one that paid for it.

How to detect it

Look for the synchronous `File`, `Directory`, `Stream`, and `Process` members in anything an endpoint, a handler, or a background reader can reach, for `.Result`, `.Wait()`, and `GetAwaiter().GetResult()`, and for any of them lexically inside a `lock` block or between a `Semaphore.Wait` and its release. Synchronous I/O in start-up code, a CLI command, or a test fixture is not a violation.

Severity: high. Deterministic mappings: Reviewer judgment; no deterministic mapping.

// backend/QualityStudio.Api/RepositorySnapshotPrewarmer.cs
protected override async Task ExecuteAsync(CancellationToken stoppingToken)
{
    // Keep host startup non-blocking: the API becomes reachable while snapshots warm in the background.
    await Task.Yield();
    await foreach (var registration in queue.Reader.ReadAllAsync(stoppingToken))
    {
        var hierarchy = await Task.Run(() => hierarchyCache.GetMeasured(registration.Root), stoppingToken);
    }
}

lock (slot.Gate)                                  // every other caller of this slot now waits too
{
    var head = process.StandardOutput.ReadToEnd();     // blocking read of a child process
    process.WaitForExit();                             // no token, no timeout
    foreach (var path in changed) hash.Append(File.ReadAllText(path));
}

A cache key is derived from the content it stands for, so a stale entry is impossible rather than unlikely — not a timestamp, not a time-to-live. Every cache also declares its bound: a size cap with eviction, or a key space that provably cannot grow (one entry per registered repository, not one per observed commit).

Why it matters

`RepositoryHierarchyCache.GetMeasured` keys a slot on the Git state — HEAD, the staged index, and the hashed contents of every dirty or untracked path — which is why a warm switch is 17.60 ms against a 9,781.93 ms cold scan without a freshness window to guess at. The bound is the part that is easy to forget: `ProjectDashboardService` uses the same derived key but keeps one full dashboard per `(root, gitState)` pair, so every commit and every dirty state ever observed stays in memory for the life of the process.

How to detect it

Look at each `ConcurrentDictionary`, `Dictionary`, or `MemoryCache` field for two things: what the key is derived from, and what removes an entry. A key containing a timestamp, a run id, or a monotonically growing value with no eviction is a violation; a key space bounded by a registration list is not.

Severity: medium. Deterministic mappings: Reviewer judgment; no deterministic mapping.

// backend/QualityStudio.Api/ProjectDashboard.cs
var key = root + "\0" + snapshot.GitState;   // derived from content, so a stale hit cannot happen
if (cache.TryGetValue(key, out var cached)) return cached;

// One retained entry per commit and per dirty state, for the life of the process.
private readonly ConcurrentDictionary<string, Dashboard> cache = new();
public Dashboard Get(string root, string gitState) =>
    cache.GetOrAdd(root + "\0" + gitState, _ => Build(root));

When a tool can answer for a whole solution or repository, invoke it once and project its result onto the units that need it. Do not launch a process, open a connection, or repeat a repository-wide scan inside a loop over projects, files, or findings.

Why it matters

`DependencyVulnerabilitySensor` runs `dotnet list <project> package --vulnerable` once per discovered project where one solution-wide command would answer the same question, and the review runner invokes repository-wide security sensors inside each file operation and then filters the result down to that one file — a 92-file sweep repeats the whole repository scan 92 times. Process start-up and repository traversal dominate these costs, so the loop, not the tool, is what makes the sweep slow.

How to detect it

Look for a process start, an HTTP call, a database round trip, or a full-repository traversal inside a `foreach` over projects, files, or findings, and for a call whose result is immediately filtered to a single subject. Batch APIs that genuinely take one subject per call are not a violation; repeating a call that already accepts the whole scope is.

Severity: high. Deterministic mappings: Reviewer judgment; no deterministic mapping.

// One repository-wide scan, projected onto the subjects that need it.
var evidence = request.DeterministicEvidence
    ?? await CollectDeterministicEvidenceAsync(request, root, cancellationToken).ConfigureAwait(false);
var forThisUnit = DeterministicEvidenceProjection.ForSubjects(evidence, subjectPaths);

foreach (var project in discoveredProjects)   // one process per project
{
    var result = await runner.RunAsync("dotnet",
        ["list", project, "package", "--vulnerable", "--format", "json"], root, cancellationToken);
    findings.AddRange(Parse(result));
}

An operation that waits on a process, a network call, or an agent runs under an explicit wall-clock timeout as well as its `CancellationToken`. A queue's reader starts such work and supervises it; it does not await it to completion, so one operation that never returns cannot stop every operation behind it.

Why it matters

`ReviewJobService.ExecuteAsync` is a single-reader channel that awaits each run, so an operation that never returns parks the reader for good, and cancelling it makes the durable state terminal without freeing the reader — the queue is then permanently stopped while looking healthy. The boundary sensor is the demonstration: 1.4 s on 144 files, and no return at all within a 300-second client timeout on a 1,269-file frontend.

How to detect it

Look for `await`s on a process, HTTP, or agent call with no linked timeout token, and for a channel or queue reader whose loop body awaits the whole operation. A timeout that only exists on the client is not one; the bound has to be on the side that holds the resource.

Severity: high. Deterministic mappings: Reviewer judgment; no deterministic mapping.

using var timeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
timeout.CancelAfter(OperationTimeout);
var run = Task.Run(() => sensor.ScanAsync(root, timeout.Token), timeout.Token);
_supervisor.Track(operationId, run);   // the reader keeps draining the queue

await foreach (var job in reader.ReadAllAsync(stoppingToken))
{
    // One non-returning operation parks the only reader; nothing behind it is ever picked up.
    await runner.RunAsync(job, stoppingToken);
}

Repository source, model output, sensor output, and anything a caller supplies is data. When it is embedded in a prompt, a command, a template, a query, or a document, mark or escape its boundary so the content cannot end that boundary itself, and validate what comes back rather than trusting what it claims about itself.

Why it matters

`ReviewPromptBuilder` generates a fresh 128-bit boundary marker per prompt precisely so file content cannot guess and forge the closing marker, and the prompts state that everything between the markers is untrusted no matter what it claims to be. The output side is the other half: `ReviewResponseParser` rejects a finding that claims deterministic provenance and `FindingIdentity` computes the excerpt hashes itself, because a value the model supplied about its own trustworthiness is not evidence. Prompt wording alone is not a boundary — the marker, the validation, and the host-computed anchors are.

How to detect it

Look for untrusted content concatenated into a prompt, a shell command, an SQL statement, a path, or markup with a fixed or predictable delimiter, and for a field the producer controls being used to decide how much that producer is trusted — a claimed source, a claimed hash, a claimed severity. Content passed as a bound parameter, an argument-list entry, or an escaped value is not a violation.

Severity: high. Deterministic mappings: Reviewer judgment; no deterministic mapping.

// backend/AgentOrchestrator.CodeQuality/ReviewPromptBuilder.cs
// Fresh per prompt so repository content cannot pre-guess and forge a closing marker.
private static string GenerateContentBoundary() =>
    "QS-CONTENT-" + Convert.ToHexStringLower(RandomNumberGenerator.GetBytes(16));

var prompt = "Review this file:\n---\n" + fileContent + "\n---\n" + instructions;
// and, on the way back, believing what the response says about itself:
if (response["source"]?.GetValue<string>() == "analyzer") finding.Trusted = true;

Anything whose size someone else decides carries a stated limit: request and document bytes, nesting depth, collection and result counts, retained cache entries, concurrent operations, and wall-clock duration. The limit is validated where it is configured and enforced where the resource is consumed, and exceeding it is a clear refusal rather than a slow failure.

Why it matters

`ApiSecurity` validates its limits at start-up — request bodies between 1 KiB and 10 MiB, 1 to 1024 concurrent requests, 1 to 1000 spend requests per minute — so a misconfiguration fails the host instead of quietly removing a bound. The gaps show what happens without that discipline: reading a repository file with no size check lets one caller exhaust process memory by repeating a request, and the sidecar index retains a parsed document per file with no byte cap, so cost grows with a number the repository chooses rather than one this process agreed to.

How to detect it

For each resource crossing a boundary, ask what its limit is and where it is enforced: a read with no length check, a parse with no depth bound, a collection accumulated in a loop with no cap, a cache with no eviction, a wait with no timeout, a retry with no ceiling. A limit only present in documentation or in the client is not a bound.

Severity: high. Deterministic mappings: Reviewer judgment; no deterministic mapping.

// backend/QualityStudio.Api/ApiSecurity.cs
if (MaxRequestBodyBytes is < 1024 or > 10 * 1024 * 1024)
    throw new InvalidOperationException("MaxRequestBodyBytes must be between 1 KiB and 10 MiB.");
if (MaxConcurrentRequests is < 1 or > 1024)
    throw new InvalidOperationException("MaxConcurrentRequests must be between 1 and 1024.");

// Size chosen by the repository, retained for the life of the process, parsed at default depth.
var payload = JsonDocument.Parse(File.ReadAllText(sidecarPath));
index[relativePath] = payload.RootElement.Clone();

Respect the repository's declared source ownership and dependency direction. When a `quality-architecture.json` contract exists, keep required directories, retired source locations and directory allowlists consistent with it. Treat an intentional architecture change as a coordinated update to code, contract, scripts and documentation.

Why it matters

An asymmetric or drifting repository layout makes ownership harder to discover and leaves build, test and review tools referring to stale locations. A version-controlled contract turns an agreed architecture into reviewable evidence without assuming one folder convention is correct for every repository.

How to detect it

Use the architecture sensor's source-located findings and compare touched paths to the repository-owned contract. Flag source returned to a retired location, missing required directories, unexpected direct children and invalid contracts. Do not infer violations from folder names alone when a repository has no contract. Generated build output and historical review metadata are not product source. Directory checks do not prove component cohesion or dependency direction; language-native analyzers and reviewer judgment cover those.

Severity: medium. Deterministic mappings: architecture/missing-directory, architecture/forbidden-source-path, architecture/unexpected-entry, architecture/invalid-contract

{
  "schemaVersion": 1,
  "requiredDirectories": ["backend", "backend/tests", "frontend/src"],
  "forbiddenSourcePaths": ["src", "backend/src"],
  "directoryRules": []
}

quality-architecture.json  # declares backend and forbids retired src locations
backend/Api/Program.cs
backend/src/AnotherApi/Program.cs  # reintroduces an unnecessary retired source wrapper

Ground every finding in a concrete source location and a plausible trigger. Explain the affected behavior or trust boundary, the consequence and a proportionate correction. Use the violated engineering rule's id when one applies. Distinguish a repository contract violation from a personal preference, and state uncertainty when required context is absent.

Why it matters

A review is useful when another engineer can understand what fails and why the proposed change fixes it. Unsupported severity claims and arbitrary style demands produce noise, while failure paths, concurrency and user-visible regressions deserve explicit reasoning.

How to detect it

For a suspected issue, trace the input or state transition to the observable consequence; inspect adjacent callers, the repository contract and relevant tests. Name the missing condition or incorrect dependency, not only a broad smell. Do not turn an unverified assumption, a folder-name preference or a test-count target into a finding. Prefer a small behavior regression test when it demonstrates the failure; do not demand tests that merely repeat trivial implementation details.

Severity: medium. Deterministic mappings: Reviewer judgment; no deterministic mapping.

When request A finishes after the user selects repository B, its response replaces B's tree.
The completion handler does not compare the captured repository id with the current one.
Guard the response before applying it and test the A/B response order.

The service is long, so it must contain critical bugs. Rename src and add more tests.

For HTML pages explicitly intended for search discovery, keep the delivered response and crawler directives consistent with that intent. Preserve deliberate exclusions for private, preview and non-search pages.

Why it matters

A public product page cannot explain the product in search when its deployment accidentally retains a preview noindex directive. Crawl access and index permission are separate: robots.txt is neither access control nor a reliable way to remove a URL from search. Primary references: [Google block indexing](https://developers.google.com/search/docs/crawling-indexing/block-indexing) and [robots.txt location and scope](https://developers.google.com/crawling/docs/robots-txt/create-robots-txt).

How to detect it

Establish the intended audience and route before reporting a defect; public-facing alone does not imply SEO relevance. Cite a conflicting production response, robots meta tag, X-Robots-Tag, authentication redirect or applicable host-root robots.txt directive. A robots.txt inside /quality/ does not govern the host. A crawler blocked by robots.txt cannot read that page's noindex directive. Do not recommend removing authentication, deliberate noindex or privacy controls. Missing robots.txt is not itself a defect. Without deployment/header evidence, report only the source-level conflict you can establish; do not claim a live indexing failure or ranking loss.

Severity: high. Deterministic mappings: Reviewer judgment; no deterministic mapping.

<!-- Public product overview intended for search; no preview exclusion. -->
<!doctype html>
<html lang="de">
<head><title>Quality Studio: nachvollziehbare Code-Reviews</title></head>
<body><main><h1>Code-Reviews mit nachvollziehbaren Findings</h1></main></body>
</html>

<!-- Production overview still inherits the preview-only directive. -->
<meta name="robots" content="noindex">
<h1>Discover Quality Studio</h1>

For search-relevant HTML, keep declared canonical URLs, redirects and language alternates consistent with the actual pages. Give separately targeted language versions stable URLs and reciprocal alternate references when hreflang is used.

Why it matters

A language switch on a single URL cannot describe several independently addressable language pages. Conflicting canonical and alternate declarations can obscure which page a visitor should reach. Canonical declarations are signals; they do not guarantee the search engine's selected URL. Primary references: [Google canonicalization](https://developers.google.com/search/docs/crawling-indexing/consolidate-duplicate-urls) and [localized versions](https://developers.google.com/search/docs/specialty/international/localized-versions).

How to detect it

Compare the reviewed page's delivered language, canonical, redirects and declared alternate targets. Report conflicting canonicals, nonexistent locale URLs or missing return links in an existing hreflang set, with exact source or response evidence. Keep each language's canonical in that language where an equivalent exists; do not canonicalize every locale to the German home page. Inspect rendered output when templates generate href values. Absence of canonical or hreflang alone is not a universal defect; establish duplicates or a declared localization requirement. Do not infer that a JavaScript application cannot be indexed, or that an unvisited target is broken.

Severity: medium. Deterministic mappings: Reviewer judgment; no deterministic mapping.

<!-- /quality/en/criteria/; the German page links back to this URL. -->
<html lang="en">
<head>
  <link rel="canonical" href="https://agent-orchestrator.dev/quality/en/criteria/">
  <link rel="alternate" hreflang="en" href="https://agent-orchestrator.dev/quality/en/criteria/">
  <link rel="alternate" hreflang="de" href="https://agent-orchestrator.dev/quality/criteria/">
</head>
</html>

<!-- English criteria content points to an unrelated German overview. -->
<link rel="canonical" href="https://agent-orchestrator.dev/quality/">
<link rel="alternate" hreflang="en" href="https://agent-orchestrator.dev/quality/#english">

Give search-relevant HTML pages descriptive titles that match their visible purpose and language. Where descriptions are supplied, make them accurate summaries of the corresponding page.

Why it matters

Distinct titles help people choose between reviews, criteria and model information. Copied or misleading metadata promises the wrong content. Search engines can generate different title links and snippets; authoring metadata does not control their exact display. Primary references: [Google title links](https://developers.google.com/search/docs/appearance/title-link) and [search snippets](https://developers.google.com/search/docs/appearance/snippet).

How to detect it

Inspect delivered or rendered metadata together with the page's main content. Cite an absent or empty title, demonstrably misleading language/purpose, or identical boilerplate across materially different reviewed pages. Treat a missing description as an improvement opportunity unless an explicit project requirement makes it a defect. Do not flag every repeated brand name, impose arbitrary character counts or keyword density, or promise a ranking increase. A source template without a title is insufficient evidence if a known build or render stage supplies it. Private dashboards and JSON API responses are outside this rule.

Severity: medium. Deterministic mappings: Reviewer judgment; no deterministic mapping.

<!-- Criteria page; main content explains rule evidence and limits. -->
<title>Review-Kriterien und Qualitätsmetriken | Quality Studio</title>
<meta name="description" content="Wie Quality Studio Regeln, Messwerte und Review-Evidenz erklärt und welche Grenzen die Ergebnisse haben.">
<h1>Qualität nachvollziehbar prüfen</h1>

<!-- Every generated page receives this unrelated template metadata. -->
<title>Home</title>
<meta name="description" content="Buy shoes at the best price.">
<h1>Code review models and token costs</h1>

Connect search-relevant pages with meaningful links that resolve to their intended destinations. When the project publishes a sitemap, keep its entries aligned with the canonical public routes.

Why it matters

A crawler and a visitor need a route to the content. Real link destinations also support opening a new tab and navigation without custom click code. A sitemap can aid discovery, but neither replaces usable navigation nor guarantees indexing. Primary references: [Google crawlable links](https://developers.google.com/search/docs/crawling-indexing/links-crawlable) and [sitemap purpose](https://developers.google.com/search/docs/crawling-indexing/sitemaps/overview).

How to detect it

For intended public HTML, check rendered anchors for usable href destinations and describe an evidenced broken or inaccessible navigation path. A framework's source router directive may generate a valid href, so inspect its output before flagging it. For an existing sitemap, compare declared entries with known canonical routes and explicit indexability intent; identify the concrete stale, private or nonexistent entry. Missing sitemap is not a universal violation, especially on a small well-linked site. Buttons that perform actions rather than navigation are valid. Do not demand a fixed link count or claim unseen pages are orphaned from a partial file review.

Severity: medium. Deterministic mappings: Reviewer judgment; no deterministic mapping.

<nav aria-label="Quality Studio">
  <a href="/quality/reviews/">Reviews verstehen</a>
  <a href="/quality/criteria/">Kriterien und Metriken</a>
  <a href="/quality/models/">Modelle vergleichen</a>
</nav>

<!-- The only route to the criteria page exists inside a click handler. -->
<span onclick="location.href='/quality/criteria/'">Click here</span>

Component styles must reference the central design-token custom properties (`--studio-*`, `--space-*`, `--font-*`, `--syntax-*`, etc., defined once in `frontend/src/app/shared/styles/tokens.css`) for color, spacing, radius, and typography. Do not hard-code hex colors, raw pixel values, or one-off font sizes in a component's own `.css` file.

Why it matters

A single token source is what lets the whole app re-theme (light/dark, `data-theme`) and stay visually consistent without hunting through every feature folder. Every raw value a component invents is a value the token system and future theme changes cannot see or move together.

How to detect it

The local PostCSS typography check reports literal font sizes and `--studio-font-size-*` tokens below the declared 11px minimum. Relative sizes require a rendered review. Read the component's `.css` for literal colors (`#rrggbb`, `rgb(`, named colors), raw `px`/`rem` lengths on padding, margin, gap, `border-radius`, and `font-size`, and for shadows written out by hand. A value is a violation when an equivalent `--studio-*`, `--space-*`, or `--font-*` token exists in `frontend/src/app/shared/styles/tokens.css`; `0`, `1px` hairlines, and percentage/`fr` layout values are not.

Severity: medium. Deterministic mappings: quality-architecture/minimum-font-size

/* frontend/src/app/features/reviews/review-panel/review-panel.css */
.severity {
  padding: var(--studio-space-1) var(--studio-space-2);
  border-radius: var(--studio-radius-badge);
  background: var(--studio-bg-elevated);
  font-size: var(--studio-font-size-label);
}
.severity.critical { color: var(--studio-severity-critical); }

/* a new feature invents its own palette and spacing instead of reusing tokens */
.priority-tag {
  padding: 4px 8px;
  border-radius: 4px;
  background: #eef1f5;
  font-size: 11px;
  color: #991b1b;
}

Before adding a new badge, panel, or control markup pattern, check `frontend/src/app/shared/styles/primitives.css` and `frontend/src/app/shared/ui/` for an existing shared primitive (e.g. `.severity`, `.pane`, `.pane-header`) or a reusable standalone component. Extend or reuse it instead of writing a parallel one-off implementation with its own markup and styling.

Why it matters

`frontend/src/app/shared/styles/primitives.css` already documents this as an explicit convention: "Shared workbench primitives reused across shell panes (Explorer, Editor, ReviewPanel)." Duplicated one-off primitives drift from each other over time (spacing, states, accessibility), double the maintenance surface, and are exactly the failure mode design tokens alone cannot prevent — a component can use tokens correctly and still reinvent a pattern that already exists.

How to detect it

Compare the component's markup and class names against the shared primitives declared in `frontend/src/app/shared/styles/primitives.css` (`.studio-button`, `.studio-field`, `.studio-table-head`, `.studio-badge`, `.pane`) and the shared UI components. A new element whose class list and structure duplicate an existing primitive under a different name is a violation; a genuinely new visual pattern is not.

Severity: medium. Deterministic mappings: Reviewer judgment; no deterministic mapping.

<!-- frontend/src/app/features/reviews/review-panel/review-panel.html: reuses the shared .severity primitive -->
<span class="severity" [class]="'severity ' + finding.severity">{{ finding.severity }}</span>

<!-- a new feature reimplements its own severity chip instead of reusing .severity -->
<span class="status-chip" [ngStyle]="{ background: severityColor(finding.severity) }">
  {{ finding.severity }}
</span>

Group Angular code by declared ownership: infrastructure and contracts in core, reusable presentation and pure utilities in shared, product behavior in features, and application composition in shell. Follow the repository's own architecture contract when it declares a different layout. Keep components focused and colocate their implementation, template, styles and tests. Extract smaller components through explicit inputs/outputs when a feature becomes complex; one feature may contain several collaborating components.

Why it matters

Flat component folders and broad app components obscure ownership. Feature services leaking into shared presentation or lower layers importing feature components produce dependency cycles and make otherwise reusable controls depend on the entire application. Component composition should make ownership clearer instead of forcing one large component per feature.

How to detect it

Compare ownership and import direction against the declared architecture. Quality Studio's `frontend/lint/architecture.config.mjs` permits core to use core and pure shared utilities; shared uses shared and core models; features use core, shared and their own feature; shell composes features. Flag reverse dependencies, undeclared cross-feature imports and unrelated concerns accumulated in a component. Do not flag a documented alternative layout or a focused child component simply because a feature contains multiple components.

Severity: medium. Deterministic mappings: quality-architecture/layer-imports

frontend/src/app/
  core/api/quality-api.ts
  shared/ui/empty-state/empty-state.ts
  features/code/editor/editor.ts
  features/code/container-view/container-view.ts
  shell/workbench/workbench.ts

// shared/ui/status.ts: reusable presentation now owns a product feature dependency.
import { Editor } from '../../features/code/editor/editor';

Templates use the built-in control-flow blocks (`@for`, `@if`, `@empty`) with an explicit `track` expression on every `@for`, and call only simple, side-effect-free reads (signals, plain fields) directly in the template. Non-trivial derivation belongs in a `computed()` or a named method on the component, not inlined as a template expression.

Why it matters

`track` is what lets Angular reuse DOM nodes across re-renders instead of tearing down and rebuilding a list on every change; omitting it silently degrades to identity-based tracking and defeats `OnPush`'s benefit. Pushing derivation into the template makes it invisible to unit tests and re-evaluated on every check, whereas a `computed()` is both testable and memoized.

How to detect it

Search the template for `@for` blocks without a `track` expression, for `*ngFor` in new code, and for template expressions that call methods with arguments, index into arrays, filter, sort, or build objects inline. Bindings that read a signal, a `computed()`, or a plain field are fine.

Severity: medium. Deterministic mappings: Reviewer judgment; no deterministic mapping.

<!-- frontend/src/app/review-panel/review-panel.html -->
@for (finding of visibleFindings(); track finding.fingerprint ?? finding.id) {
  <button class="finding-card" (click)="selectFindingLocation(finding)">...</button>
} @empty {
  <div class="empty-findings">No findings match these filters.</div>
}

<!-- missing track, and re-sorting inline on every check -->
@for (finding of findings().slice().sort((a, b) => a.severity.localeCompare(b.severity))) {
  <button class="finding-card">...</button>
}

Every component sets `changeDetection: ChangeDetectionStrategy.OnPush` and drives its template from `signal`/`computed`/`input`/`output`, not from mutated plain fields or manual subscriptions that write to component properties outside Angular's change-detection triggers.

Why it matters

Every feature component in `frontend/src/app` already declares `OnPush` and reads state through signals; this is what keeps `explorer`, `review-panel`, and `usage-history` fast on large trees and result sets. A component that drops to `Default` (or mutates a field from inside a manual `subscribe()`) silently re-introduces full subtree re-checks and can even fail to render at all under `OnPush` siblings if it relies on ambient change detection to pick up its mutations.

How to detect it

Check the `@Component` decorator for `changeDetection: ChangeDetectionStrategy.OnPush`. Then look for state written outside signals: plain mutable fields assigned from `subscribe()` callbacks, `setTimeout`/event handlers, or `ChangeDetectorRef.detectChanges()` calls that exist only to make such mutations visible.

Severity: medium. Deterministic mappings: Reviewer judgment; no deterministic mapping.

// frontend/src/app/review-panel/review-panel.ts
@Component({
  selector: 'app-review-panel',
  changeDetection: ChangeDetectionStrategy.OnPush,
  // ...
})
export class ReviewPanel {
  readonly severityFilter = signal<string>('all');
  readonly visibleFindings = computed(() => /* derive from signals */ []);
}

@Component({ selector: 'app-widget' /* no changeDetection: OnPush */ })
export class Widget implements OnInit {
  findings: Finding[] = [];
  ngOnInit() {
    this.api.findings$.subscribe(value => { this.findings = value; }); // mutates a plain field
  }
}

`postMessage` names the expected recipient origin, taken from trusted configuration or a validated embedding handshake; `'*'` is not a target origin. The payload carries the fields the contract declares and nothing else — never a whole `location.href`. Every inbound `message` listener checks `event.origin` before it reads `event.data`, and validates the shape of what it reads.

Why it matters

`reportUrlPreviewNavigation` types its target origin as the literal `'*'`, so no caller can supply a real one, and `app.ts` passes it straight to `window.parent.postMessage`: any origin that can embed the preview receives its navigation messages. The payload compounds it — `url.href` carries the origin, the fragment, user information, and every pre-existing query parameter, so a token already in the address bar travels to that unknown parent along with the three fields the contract actually declares.

How to detect it

Search for `postMessage` with `'*'` or a variable that is never compared against an allowlist, for payloads built from `location.href`, `document.URL`, or a whole `URL` object, and for `addEventListener('message', ...)` handlers that read `event.data` before checking `event.origin`. A dedicated Web Worker channel has no origin to check, but still needs its message shape validated.

Severity: high. Deterministic mappings: Reviewer judgment; no deterministic mapping.

const parentOrigin = environment.embedOrigin;            // trusted configuration, not the message
if (parentOrigin) {
  window.parent.postMessage(
    { type: 'qs.url-preview.navigate', repo, path, kind },   // only the declared fields
    parentOrigin);
}

// frontend/src/app/url-preview-embed.ts: any embedding origin receives this, href and all
postToParent({ type: 'qs.url-preview.navigate', url: environment.href }, '*');

Content that did not come from this component's own template — repository source, a model's prose, a finding title, anything fetched — is rendered through interpolation or property binding. No `innerHTML`, `outerHTML`, `insertAdjacentHTML`, `document.write`, `eval`, or `DomSanitizer.bypassSecurityTrust*`. If markup is genuinely required, render it from a parsed model into elements, not from a string.

Why it matters

There is no `innerHTML` or sanitizer bypass anywhere under `frontend/src` today: the editor renders every line as `{{ segment.text }}` inside spans and the review panel renders model-authored descriptions as `<p>{{ finding.description }}</p>`, and the HTML report encodes with `WebUtility.HtmlEncode` before writing. That is the property worth keeping, because the strings in question are exactly the untrusted ones — source code a repository controls and prose a model wrote.

How to detect it

Search the component and its template for `innerHTML`, `outerHTML`, `insertAdjacentHTML`, `bypassSecurityTrust`, `DomSanitizer`, `document.write`, `eval(`, and `new Function(`. A `[innerText]` or `[textContent]` binding is not a violation, and neither is `[attr.*]` on a value the sanitizer still sees.

Severity: high. Deterministic mappings: Reviewer judgment; no deterministic mapping.

<!-- frontend/src/app/editor/editor.html -->
<code>@for (segment of segmentedLine(row.number, row.text, row.findings); track $index) {
  <span [class]="segmentClass(segment)" [attr.aria-label]="segmentAriaLabel(segment, row.number)">{{ segment.text }}</span>
}</code>

<!-- The description is model-authored prose; this hands it to the HTML parser. -->
<p [innerHTML]="sanitizer.bypassSecurityTrustHtml(finding.description)"></p>

`localStorage`, `sessionStorage`, IndexedDB, cookies written from script, the URL, and component state hold non-secret presentation state only — a theme, a layout, a selected filter. Tokens, credentials, and authorization headers are not constructed, stored, or logged in the browser.

Why it matters

The Angular client stores exactly two things, `'qs-theme'` and the layout key, and builds no `Authorization` header at all: authentication is a bearer token and a client id enforced by `ApiSecurity` on the server, which is why nothing that leaks from the browser — a shared device, a console log, an embedding parent — is a credential. Client-held storage is readable by any script on the origin and survives the session, so a token placed there outlives the reason it was issued.

How to detect it

Look at every `localStorage`/`sessionStorage` key and every value assigned to a header, a query parameter, or a logged object for names like token, key, secret, password, or authorization, and for a credential arriving in a route parameter or fragment. A non-secret repository id or view preference in storage is not a violation.

Severity: high. Deterministic mappings: Reviewer judgment; no deterministic mapping.

// frontend/src/app/app.ts
localStorage.setItem('qs-theme', theme);          // presentation state only
localStorage.setItem(LAYOUT_STORAGE_KEY, JSON.stringify(sizes));

localStorage.setItem('qs-api-token', token);      // readable by any script on this origin, forever
this.http.get(url, { headers: { Authorization: `Bearer ${token}` } });

Path segments go through `encodeURIComponent` and query values through `HttpParams` or `URLSearchParams`; a URL is never assembled by concatenating raw values. A navigation target — a `[href]`, `router.navigateByUrl`, `window.open`, or an assignment to `location` — is a same-origin route this application computed, never a URL that arrived in a response, a query parameter, or a message.

Why it matters

`quality-api.ts` escapes every id it puts in a path and passes every value as a parameter, so a repository or finding id containing a slash or an ampersand stays one segment instead of becoming a new path or a new parameter, and every request stays same-origin and relative. The navigation half is what turns a formatting bug into an open redirect: a link target taken from data sends the user, and the referrer, wherever that data says.

How to detect it

Look for template strings that interpolate an id or a user value into a URL without `encodeURIComponent`, for query strings built with `+` or interpolation instead of `HttpParams`, and for `[href]`, `window.open`, `location.href =`, or `navigateByUrl` bound to a value from a response, a route parameter, or a message. A relative URL built from constants is not a violation.

Severity: medium. Deterministic mappings: Reviewer judgment; no deterministic mapping.

// frontend/src/app/quality-api.ts
private repositoryApiBase(): string {
  return this.legacyApi ? '/api' : `/api/repos/${encodeURIComponent(repositoryId)}`;
}
const file = await firstValueFrom(
  this.http.get<FileDocument>(`${this.repositoryApiBase()}/file`, { params: { path } }));

// An id with a slash becomes a new path segment; the link target comes from the response.
this.http.get(`/api/repos/${repositoryId}/file?path=` + path);
window.open(response.externalUrl, '_blank');

A collection a template iterates or measures is a `computed()` signal. A template expression reads a signal or a plain field; it does not call a method that filters, maps, sorts, slices, reverses, or otherwise allocates — and the same derivation is not written twice in one template.

Why it matters

`review-panel.ts` and `review-actions.ts` already expose `scopeRuns`, `comparableRuns`, `runFindings`, `filteredModels`, and `fileCount` as `computed()`, so each is recomputed when its inputs change and not once per check. Where that slipped, the cost is visible: `review-actions.html` calls `runFiles(run, states)` six times per change-detection cycle — twice per line, once for the length check and once for the loop — and each call returns a fresh array, so `track file.path` reconciles against objects it has never seen and the list rebuilds.

How to detect it

Read every `{{ }}`, `@if`, and `@for` expression: a method call with arguments, `.filter(`, `.map(`, `.slice(`, `.sort(`, `.reverse(`, an array or object literal, or the same call appearing twice in one template is a violation. Reading a signal, a `computed()`, or a plain field is not, and a pure formatting call on a scalar is not either.

Severity: medium. Deterministic mappings: Reviewer judgment; no deterministic mapping.

// frontend/src/app/review-panel/review-panel.ts
readonly scopeRuns = computed(() => this.api.runs().filter(run => run.scope === this.scope()));
readonly runFindings = computed(() => this.indexFindings(this.selectedRun()));

<!-- runFiles() allocates a new array on every call, and this calls it twice per line -->
@if (runFiles(run, states).length) {
  @for (file of runFiles(run, states); track file.path) { <li>{{ file.path }}</li> }
}

A view over data whose size the repository decides — a file tree, a source file, a findings list, a run history — renders a fixed window of rows plus a small overscan, sized by the viewport and not by the data. Index the data once when it arrives instead of scanning it per rendered row.

Why it matters

`explorer.ts` flattens the tree in a `computed()` and slices a window out of it, and the editor keeps an 80-line overscanned window regardless of file length, which is what holds the under-100 ms transition on a repository with 3,450 tracked files. The alternative is not slower by a constant: DOM nodes, bindings, and per-row work all scale with the data, so the first pathological repository turns a usable pane into an unusable one.

How to detect it

For each `@for` over data of repository-determined size, look for a window computed from a scroll offset and a row height, and check whether per-row expressions search or filter a collection rather than reading an index built once. A list bounded by a small constant — review kinds, severities, adapters — is not a violation.

Severity: medium. Deterministic mappings: Reviewer judgment; no deterministic mapping.

// frontend/src/app/explorer/explorer.ts
readonly treeRows = computed(() => flattenTree(this.api.tree(), this.expanded()));
readonly visibleRows = computed(() =>
  this.filteredRows().slice(start, start + count).map((node, i) => ({ node, top: (start + i) * ROW_HEIGHT })));

<!-- Every row of every file, and a scan of all findings per row -->
@for (row of allLines(); track row.number) {
  <span>{{ findings().filter(f => f.line === row.number).length }}</span>
}

A feature that is not on the first screen loads as a deferred chunk (`@defer`, a lazy route) rather than as an eager import, and a new dependency is weighed against the production budgets in `frontend/angular.json` before it is added. Component stylesheets stay inside the per-stylesheet budget instead of accumulating one-off blocks.

Why it matters

The production build errors, not warns, when the initial bundle passes its limit: a build has already failed at 494.94 kB against the 480 kB error budget, so an eager import of a rarely used feature does not degrade the app gradually — it stops the build for whoever commits next. The attack matrix shows the intended shape, a deferred 17 kB chunk that costs the first screen nothing.

How to detect it

Check whether a newly imported component, library, or icon set is reachable from the initial route and whether a heavy feature is declared with `@defer` or a lazy route. Compare added stylesheet weight against the `anyComponentStyle` budget in `frontend/angular.json` — the authoritative numbers are there, not in prose that may have drifted from it.

Severity: medium. Deterministic mappings: Reviewer judgment; no deterministic mapping.

<!-- A rarely opened, heavy pane costs the first screen nothing -->
@defer (on interaction) {
  <app-attack-coverage />
} @placeholder {
  <button class="pane-header">Attack coverage</button>
}

// Eagerly imported into the shell, so it is in the initial bundle for everyone
import { AttackCoverage } from './attack-coverage/attack-coverage';
import * as everything from 'some-charting-library';

Work whose cost scales with repository data — tokenizing a file, diffing, indexing — runs in a worker, starts after the first paint, proceeds in bounded chunks, and is cancelled when its subject changes. At most one such job is in flight per subject; a superseded job is cancelled, not awaited.

Why it matters

Whole-file main-thread highlighting is prohibited in this codebase for a measurable reason: tokenization runs in a cancellable single-concurrency worker in 200-line chunks with a 200 kB cap, which is what keeps opening a large file inside the under-100 ms transition budget. Without chunking and cancellation, scrolling through five files queues five full tokenizations and the pane stops responding to the sixth.

How to detect it

Look for parsing, tokenizing, or diffing loops over whole-file content on the main thread, for `await` of such work directly in a component initializer, and for a worker call with no cancellation when its input changes. Work bounded by a small constant, or done once at start-up, is not a violation.

Severity: medium. Deterministic mappings: Reviewer judgment; no deterministic mapping.

// One job per subject: the previous one is cancelled rather than awaited.
this.pending?.cancel();
this.pending = this.highlighter.tokenize(path, text, { chunkLines: 200, maxBytes: 200_000 });

ngOnInit() {
  // Whole-file tokenization on the main thread, on every open, uncancellable
  this.tokens = tokenizeEveryLine(this.file.text);
}