noema-review: allowed-locations truncation silently makes whole files unreviewable, biased against tests/

[seonghobae]

Summary

scripts/ci/noema_review_gate.py bounds the allowed_locations list it sends to the model by taking the largest prefix that fits a 32 KiB budget, from a list sorted alphabetically by path. Truncation therefore drops a suffix, deterministically starving the alphabetically-last paths — and tests/ sorts last.

The consequence is not a failure. It is a silent one: the prompt instructs the model to omit verdicts for unlisted locations, so a noema-review can return APPROVE having been structurally unable to review entire files, with nothing in the job log indicating it.

This was first observed by another session replaying the gate against contextual-orchestrator#1031's real diff. I reproduced it independently below, using the production functions rather than that report.

Mechanism, on origin/main@7fcada597

scripts/ci/noema_review_gate.py:1534-1537 — the list is sorted, so ordering is alphabetical by path:

allowed_locations = [
    {"path": path, "line": line, "side": side}
    for path, line, side in sorted(changed_diff_locations(diff))
]

:1393-1417 — _bounded_allowed_locations_json binary-searches for the largest prefix that fits MAX_ALLOWED_LOCATIONS_JSON_BYTES = 32 * 1024 (:61):

"locations": list(allowed_locations[:count]),

:1550 — and the prompt tells the model to stay silent about anything not listed:

"Use only path, line, and side tuples listed in the bounded allowed-locations JSON below. If it is truncated, omit a formal verdict for any location not listed instead of guessing."

A sorted list plus a prefix budget means the dropped set is never arbitrary and never rotates. The same paths lose every time.

Independent reproduction

Driving the production functions directly — no reimplementation — with a synthetic three-file diff of 700 changed lines each:

MAX_ALLOWED_LOCATIONS_JSON_BYTES = 32768
total_count=2100  kept=437  truncated=True

  contextual_orchestrator/aaa_module.py         kept  437 /  700
  docs/mmm_notes.md                             kept    0 /  700
  tests/test_zzz_critical.py                    kept    0 /  700

1,400 changed lines across two entire files received zero allowed locations. The reporting session measured the same shape against a real PR: tests/test_opencode_go_subscription_contract.py 0/36 and tests/test_provider_catalog_bootstrap.py 0/118 fully starved, while contextual_orchestrator/model_discovery.py kept 106/106.

Why this is worse than an ordinary defect

Both branches of the model's behaviour are bad, and the harmless-looking branch is the dangerous one:

• Model obeys the instruction → it silently issues no verdict on the starved files. A review can come back APPROVE having never been permitted to look at them. Nothing distinguishes this from a genuine approval.
• Model disobeys → it cites an unlisted location, validate_substantive_verdict rejects the verdict, and the entire ~21-minute review is discarded.

The bias compounds the risk in the worst possible direction: larger PRs truncate more, and tests/ — the evidence that a change is correct — is always first to be dropped. The gate is least able to see the review evidence exactly when there is most of it.

Observability gap

The truncation flag exists but never reaches an operator. :1402 places "truncated": count < total_count inside the JSON string sent to the model. The only truncation signal in the job log is :1558:

f"Diff truncated: {truncated}",

— which is the diff truncation flag from :477, an unrelated variable. So a log can read Diff truncated: False while the allowed-locations list has silently lost two entire files. Confirming truncation currently requires replaying the production functions offline, which is how both this report and the original one were produced.

Suggested directions (deliberately not prescribing a fix)

Raising MAX_ALLOWED_LOCATIONS_JSON_BYTES only moves the threshold; the bias direction is unchanged. Some options that do change it:

1. Per-file budget instead of a global prefix — removes the alphabetical bias, but still depends on the model obeying "don't cite unlisted locations", which validate_substantive_verdict can only punish after the fact by discarding the review.
2. Narrow the declared verdict scope to the files whose locations actually fit, so "this review did not cover file X" becomes a checkable invariant rather than a model-compliance hope. This seems the strongest option: it converts a silent gap into an explicit one.
3. Log the flag and the starved paths regardless of which fix lands. This is independently worth doing — it is what makes the condition diagnosable in one step instead of an offline replay.

Reproduction script

import importlib.util, json, sys
from collections import Counter

spec = importlib.util.spec_from_file_location("ngr", "scripts/ci/noema_review_gate.py")
m = importlib.util.module_from_spec(spec); sys.modules["ngr"] = m; spec.loader.exec_module(m)

def hunk(path, n):
    body = "".join(f"+line {i}\n" for i in range(n))
    return (f"diff --git a/{path} b/{path}\n--- a/{path}\n+++ b/{path}\n@@ -0,0 +1,{n} @@\n{body}")

paths = [("contextual_orchestrator/aaa_module.py", 700),
         ("docs/mmm_notes.md", 700),
         ("tests/test_zzz_critical.py", 700)]
diff = "".join(hunk(p, n) for p, n in paths)

locs = sorted(m.changed_diff_locations(diff))
allowed = [{"path": p, "line": l, "side": s} for p, l, s in locs]
d = json.loads(m._bounded_allowed_locations_json(allowed))

tot, kc = Counter(p for p, _, _ in locs), Counter(x["path"] for x in d["locations"])
print(f"total={d['total_count']} kept={len(d['locations'])} truncated={d['truncated']}")
for p, _ in paths:
    print(f"  {p:<45} kept {kc.get(p,0):>4} / {tot[p]:>4}")

Scope note

Filing rather than fixing: this is production review-gate behaviour with organization-wide blast radius, and the design choice between options 1–3 changes what an APPROVE verdict means. That belongs to the gate's owner, not to a drive-by patch. Related triage context is being catalogued in #1913.