From 9d92a9da4f3270e7dc5775c6ef6c36158d3efaae Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 22:50:34 +0900 Subject: [PATCH 1/4] test(noema): reproduce ungrounded changed-line repair retry Capture the BandScope #1122 failure where the third reviewed-line coordinate is outside the parsed diff, and require one authoritative LEFT/RIGHT coordinate manifest on both the first request and repair retry. --- ...9-01-noema-exact-changed-line-grounding.md | 58 +++++++ tests/test_noema_review_gate.py | 149 ++++++++++++++++++ 2 files changed, 207 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-01-noema-exact-changed-line-grounding.md diff --git a/docs/superpowers/plans/2026-09-01-noema-exact-changed-line-grounding.md b/docs/superpowers/plans/2026-09-01-noema-exact-changed-line-grounding.md new file mode 100644 index 0000000000..9403db25d2 --- /dev/null +++ b/docs/superpowers/plans/2026-09-01-noema-exact-changed-line-grounding.md @@ -0,0 +1,58 @@ +# Noema Exact Changed-Line Grounding Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Keep Noema's exact changed-side validator fail-closed while giving both the first LLM request and its one repair retry an authoritative, compact coordinate manifest. + +**Architecture:** Parse the bounded unified diff once, group exact `(path, line, side)` locations by file and side, and encode consecutive lines as inclusive ranges in compact JSON. Put that manifest in the prompt as the sole authority for `reviewed_lines`, adversarial probes, and finding coordinates; retain the existing deterministic post-response validator unchanged. + +**Tech Stack:** Python 3.14, pytest, GitHub Actions, OpenAI-compatible chat-completion transport. + +**Spec:** Live incident `ContextualWisdomLab/bandscope#1122`, Actions job `99792663163` (`Noema reviewed line 3 is not an exact changed-side line`). + +## Global Constraints + +- Do not coerce, snap, filter, or silently discard invalid model coordinates. +- Do not downgrade a formal verdict to a comment to obtain a green check. +- Keep retry count, reviewer identity, provider routing, merge authority, and current-head validation unchanged. +- The manifest must be deterministic, UTF-8 safe, and compact enough not to duplicate every changed line as a JSON object. + +--- + +### Task 1: Reproduce the ungrounded retry + +**Files:** +- Test: `tests/test_noema_review_gate.py` + +**Interfaces:** +- Consumes: `changed_diff_locations()` and `call_llm()`. +- Produces: regressions for range compression and for manifest presence on both initial and repair requests. + +- [ ] Add a mixed LEFT/RIGHT multi-file diff fixture and exact manifest expectation. +- [ ] Add a two-response LLM fixture whose first verdict fails specifically at reviewed-line item 3. +- [ ] Run the focused test before implementation and require the expected missing-interface failure. + +### Task 2: Ground formal review coordinates + +**Files:** +- Modify: `scripts/ci/noema_review_gate.py` +- Modify: `CHANGELOG.md` + +**Interfaces:** +- Produces: `_compact_line_ranges(lines: Sequence[int]) -> str` and `changed_line_manifest(locations: Sequence[tuple[str, int, str]]) -> str`. + +- [ ] Compress sorted line numbers into inclusive ranges. +- [ ] Serialize one deterministic JSON entry per path with only populated LEFT/RIGHT sides. +- [ ] Include the authoritative manifest in every request and point repair guidance at it. +- [ ] Leave `validate_substantive_verdict()` unchanged. + +### Task 3: Verify and publish + +**Files:** +- Remove: `.github/workflows/noema-coordinate-grounding-writer.yml` +- Remove: `scripts/ci/_temporary_apply_noema_coordinate_grounding.py` + +- [ ] Run the focused RED test and confirm the expected failure. +- [ ] Run the focused Noema suite after implementation. +- [ ] Run the full pytest/coverage/docstring/compile/diff gates. +- [ ] Preserve separate test-first and implementation commits and remove one-shot writer files. diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index a86ee3b499..3151cbc83e 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -2762,3 +2762,152 @@ def test_parse_args_and_main(monkeypatch): noema.main( ["--repo", "owner/repo", "--pr-number", "9", "--expected-head", "A" * 40] ) + + + +def _coordinate_grounding_diff() -> str: + """Return a multi-file diff with sparse LEFT/RIGHT changed coordinates.""" + return """diff --git a/a.py b/a.py +index 1111111..2222222 100644 +--- a/a.py ++++ b/a.py +@@ -1,6 +1,7 @@ + one +-old-two ++new-two ++new-three + four +-old-five ++new-five + six +diff --git a/new.py b/new.py +new file mode 100644 +index 0000000..3333333 +--- /dev/null ++++ b/new.py +@@ -0,0 +1,3 @@ ++alpha ++beta ++gamma +""" + + +def test_compact_line_ranges_handles_empty_single_sparse_and_contiguous_values(): + """The manifest must compress coordinates without losing sparse boundaries.""" + assert noema._compact_line_ranges([]) == "" + assert noema._compact_line_ranges([9]) == "9" + assert noema._compact_line_ranges([1, 2, 4, 6, 7]) == "1-2,4,6-7" + + +def test_changed_line_manifest_compacts_exact_coordinates(): + """One path entry must preserve every parsed side-specific changed line.""" + diff = _coordinate_grounding_diff() + + manifest = json.loads( + noema.changed_line_manifest(sorted(noema.changed_diff_locations(diff))) + ) + + assert manifest == [ + {"path": "a.py", "LEFT": "2,4", "RIGHT": "2-3,5"}, + {"path": "new.py", "RIGHT": "1-3"}, + ] + assert noema.changed_line_manifest([]) == "[]" + + +def test_call_llm_grounds_initial_and_repair_requests_in_authoritative_manifest(monkeypatch): + """Reproduce BandScope's third-item miss and prove the retry gets exact coordinates.""" + monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example/v1/chat/completions") + monkeypatch.setenv("NOEMA_LLM_API_KEY", "test-key") + diff = _coordinate_grounding_diff() + valid_reviewed_lines = [ + {"path": "a.py", "line": 2, "side": "RIGHT", "analysis": "Replacement keeps the value explicit."}, + {"path": "a.py", "line": 3, "side": "RIGHT", "analysis": "The inserted line is independently reviewed."}, + ] + probes = [ + { + "path": "a.py", + "line": 2, + "side": "RIGHT", + "hypothesis": "The replacement can erase the expected value.", + "attack_or_counterexample": "Trace the changed assignment through its caller.", + "evidence": "The caller still receives the explicit replacement value.", + "outcome": "falsified", + }, + { + "path": "a.py", + "line": 3, + "side": "RIGHT", + "hypothesis": "The inserted line can change ordering semantics.", + "attack_or_counterexample": "Compare execution order before and after the insertion.", + "evidence": "The insertion runs in the intended sequence.", + "outcome": "falsified", + }, + ] + invalid = { + "decision": "approve", + "summary": "First attempt uses an invented third coordinate.", + "reviewed_lines": [ + *valid_reviewed_lines, + {"path": "a.py", "line": 99, "side": "RIGHT", "analysis": "Invented coordinate."}, + ], + "adversarial_validation": { + "status": "passed", + "residual_risk": "The fixture does not execute product code.", + "probes": probes, + }, + "findings": [], + } + corrected = { + **invalid, + "summary": "Repair uses only authoritative coordinates.", + "reviewed_lines": valid_reviewed_lines, + } + contents = iter((json.dumps(invalid), json.dumps(corrected))) + requests = [] + + class Response: + """Return successive OpenAI-compatible response envelopes.""" + + def __enter__(self): + return self + + def __exit__(self, *args): + return None + + def read(self): + return json.dumps( + {"choices": [{"message": {"content": next(contents)}}]} + ).encode() + + def open_response(_opener, request, **_kwargs): + """Capture both request payloads while serving deterministic responses.""" + requests.append(request) + return Response() + + monkeypatch.setattr(noema.urllib.request.OpenerDirector, "open", open_response) + monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr(headRefOid="head")) + + verdict = noema.call_llm( + "owner/repo", + 7, + make_pr(headRefOid="head"), + diff, + False, + "head", + changed_paths=("a.py", "new.py"), + ) + + assert verdict["decision"] == "approve" + assert len(requests) == 2 + prompts = [ + json.loads(request.data)["messages"][1]["content"] for request in requests + ] + expected_manifest = noema.changed_line_manifest( + sorted(noema.changed_diff_locations(diff)) + ) + for prompt in prompts: + assert "Authoritative exact changed-side coordinate manifest" in prompt + assert expected_manifest in prompt + assert "sole coordinate authority" in prompt + assert "Noema reviewed line 3 is not an exact changed-side line" in prompts[1] + assert "using only exact changed-side locations from the authoritative coordinate manifest" in prompts[1] From 32d79f1dcc5c57a5618d9d5eb5f13d8a29ab612e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 22:50:50 +0900 Subject: [PATCH 2/4] fix(noema): ground formal verdicts in exact changed-line manifest Provide the model with a deterministic, compact JSON manifest of every allowed path, diff side, and inclusive changed-line range on both initial and repair requests. Preserve the existing fail-closed post-response validator without coordinate snapping, filtering, or verdict downgrade. --- CHANGELOG.md | 6 +++++ scripts/ci/noema_review_gate.py | 41 +++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d7c6d40ae7..f685290280 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- **Ground Noema formal-review coordinates before the first request and its repair retry.** + The trusted exact-line validator remains fail-closed, but the model now receives a compact, + deterministic JSON manifest of every allowed path, side, and inclusive changed-line range. + This prevents long-running retries from guessing coordinates out of whole-file context, as + observed in `ContextualWisdomLab/bandscope#1122` job `99792663163`, while preserving rejection + of any reviewed line, adversarial probe, or finding outside the parsed unified diff. - **Fix `opencode-review.yml` admission gaps around stale/out-of-order events (`#1568`).** Building on the draft-poll exemption's live PR/head validation, Devin Review found two further defects. (1) The concurrency group was keyed only by repository and PR number, so diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index ef270872a2..3ecc09e51e 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -300,6 +300,38 @@ def changed_diff_locations(diff: str) -> set[tuple[str, int, str]]: return locations +def _compact_line_ranges(lines: Sequence[int]) -> str: + """Compress sorted line numbers into comma-separated inclusive ranges.""" + if not lines: + return "" + ranges: list[str] = [] + start = previous = lines[0] + for line in lines[1:]: + if line == previous + 1: + previous = line + continue + ranges.append(str(start) if start == previous else f"{start}-{previous}") + start = previous = line + ranges.append(str(start) if start == previous else f"{start}-{previous}") + return ",".join(ranges) + + +def changed_line_manifest(locations: Sequence[tuple[str, int, str]]) -> str: + """Serialize exact changed-side locations as compact deterministic JSON.""" + grouped: dict[str, dict[str, set[int]]] = {} + for path, line, side in locations: + grouped.setdefault(path, {"LEFT": set(), "RIGHT": set()})[side].add(line) + entries: list[dict[str, str]] = [] + for path in sorted(grouped): + entry = {"path": path} + for side in ("LEFT", "RIGHT"): + lines = sorted(grouped[path][side]) + if lines: + entry[side] = _compact_line_ranges(lines) + entries.append(entry) + return json.dumps(entries, ensure_ascii=False, separators=(",", ":")) + + def parse_diff_path(raw: str, prefix: str) -> str: """Decode a Git unified-diff path, including C-quoted UTF-8 paths.""" value = raw.split("\t", 1)[0] @@ -1058,10 +1090,12 @@ def call_llm( raise RuntimeError("Noema LLM review unavailable: NOEMA_LLM_API_URL or NOEMA_LLM_API_KEY is not configured.") reject_private_llm_url(api_url) + changed_locations = sorted(changed_diff_locations(diff)) allowed_locations = [ {"path": path, "line": line, "side": side} - for path, line, side in sorted(changed_diff_locations(diff)) + for path, line, side in changed_locations ] + location_manifest = changed_line_manifest(changed_locations) location_example = ( allowed_locations[0] if allowed_locations @@ -1105,13 +1139,16 @@ def call_llm( }, separators=(",", ":"), ), + "Authoritative exact changed-side coordinate manifest (inclusive line ranges):", + location_manifest, + "For reviewed_lines.path, adversarial_validation.probes.path, and findings.file, copy a manifest path and side exactly and choose an integer line inside that side's ranges. This manifest, not additional context or visual line counting, is the sole coordinate authority.", "Every formal verdict must cite exact changed-side lines. APPROVE requires falsifying concrete regression hypotheses; source or test changes require at least two distinct probes and other changes require at least one. REQUEST_CHANGES requires a confirmed probe at a finding location.", "Use request_changes only for blocking, concrete issues. A generic no-issues statement is not review evidence.", *( [ "Your prior verdict was rejected by the trusted validator: " f"{repair_error or 'no diagnostic message was available'}", - "Return one corrected JSON verdict using only exact changed-side locations from the supplied diff.", + "Return one corrected JSON verdict using only exact changed-side locations from the authoritative coordinate manifest.", ] if is_retry else [] From 63897f68f9307a81c6be9ef065bc7cbb88cc4afd Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 1 Sep 2026 23:01:29 +0900 Subject: [PATCH 3/4] ci(noema): regenerate exact-head evidence after #1610 From 2619998ea2af238390309b22e5c689d644b09770 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 23:21:35 +0000 Subject: [PATCH 4/4] fix(test): commit the manifest-grounding test rewrite dropped from the merge commit The prior merge commit (1e09e0ab) resolved noema_review_gate.py's real logic conflict (dropping the retry-specific branch incompatible with main's single-request architecture) but the corresponding test file rewrite -- test_call_llm_grounds_initial_and_repair_requests_in_authoritative_manifest -> test_call_llm_grounds_its_single_request_in_authoritative_manifest -- was edited on disk but never staged before that commit, so it was verified locally (full suite, coverage, interrogate all ran against the on-disk working tree and genuinely passed) but not actually pushed. Committing it now as a follow-up rather than rewriting the prior commit's history. Verified again standalone: tests/test_noema_review_gate.py -q -> 107 passed. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- tests/test_noema_review_gate.py | 93 +++++++++++++++------------------ 1 file changed, 43 insertions(+), 50 deletions(-) diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index f0c92212ee..bb070f1ad8 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -2414,59 +2414,57 @@ def test_changed_line_manifest_compacts_exact_coordinates(): assert noema.changed_line_manifest([]) == "[]" -def test_call_llm_grounds_initial_and_repair_requests_in_authoritative_manifest(monkeypatch): - """Reproduce BandScope's third-item miss and prove the retry gets exact coordinates.""" +def test_call_llm_grounds_its_single_request_in_authoritative_manifest(monkeypatch): + """Guard the BandScope #1122 root cause: the model's one request must carry + every changed-side coordinate, not just the raw diff and a schema example. + + Noema is single-request only (contextual-orchestrator owns repair/failover, + see "Noema single-request gateway ownership" in CHANGELOG.md), so unlike an + earlier draft of this fix there is no local repair retry to also ground -- + proving fail-closed rejection of an invented coordinate is already covered + generically by validate_substantive_verdict's own direct tests above; this + test only needs to prove the manifest reaches the one request Noema sends. + """ monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example/v1/chat/completions") monkeypatch.setenv("NOEMA_LLM_API_KEY", "test-key") diff = _coordinate_grounding_diff() - valid_reviewed_lines = [ - {"path": "a.py", "line": 2, "side": "RIGHT", "analysis": "Replacement keeps the value explicit."}, - {"path": "a.py", "line": 3, "side": "RIGHT", "analysis": "The inserted line is independently reviewed."}, - ] - probes = [ - { - "path": "a.py", - "line": 2, - "side": "RIGHT", - "hypothesis": "The replacement can erase the expected value.", - "attack_or_counterexample": "Trace the changed assignment through its caller.", - "evidence": "The caller still receives the explicit replacement value.", - "outcome": "falsified", - }, - { - "path": "a.py", - "line": 3, - "side": "RIGHT", - "hypothesis": "The inserted line can change ordering semantics.", - "attack_or_counterexample": "Compare execution order before and after the insertion.", - "evidence": "The insertion runs in the intended sequence.", - "outcome": "falsified", - }, - ] - invalid = { + valid = { "decision": "approve", - "summary": "First attempt uses an invented third coordinate.", + "summary": "Both changed lines are independently reviewed.", "reviewed_lines": [ - *valid_reviewed_lines, - {"path": "a.py", "line": 99, "side": "RIGHT", "analysis": "Invented coordinate."}, + {"path": "a.py", "line": 2, "side": "RIGHT", "analysis": "Replacement keeps the value explicit."}, + {"path": "a.py", "line": 3, "side": "RIGHT", "analysis": "The inserted line is independently reviewed."}, ], "adversarial_validation": { "status": "passed", "residual_risk": "The fixture does not execute product code.", - "probes": probes, + "probes": [ + { + "path": "a.py", + "line": 2, + "side": "RIGHT", + "hypothesis": "The replacement can erase the expected value.", + "attack_or_counterexample": "Trace the changed assignment through its caller.", + "evidence": "The caller still receives the explicit replacement value.", + "outcome": "falsified", + }, + { + "path": "a.py", + "line": 3, + "side": "RIGHT", + "hypothesis": "The inserted line can change ordering semantics.", + "attack_or_counterexample": "Compare execution order before and after the insertion.", + "evidence": "The insertion runs in the intended sequence.", + "outcome": "falsified", + }, + ], }, "findings": [], } - corrected = { - **invalid, - "summary": "Repair uses only authoritative coordinates.", - "reviewed_lines": valid_reviewed_lines, - } - contents = iter((json.dumps(invalid), json.dumps(corrected))) requests = [] class Response: - """Return successive OpenAI-compatible response envelopes.""" + """Return one OpenAI-compatible response envelope.""" def __enter__(self): return self @@ -2476,11 +2474,11 @@ def __exit__(self, *args): def read(self): return json.dumps( - {"choices": [{"message": {"content": next(contents)}}]} + {"choices": [{"message": {"content": json.dumps(valid)}}]} ).encode() def open_response(_opener, request, **_kwargs): - """Capture both request payloads while serving deterministic responses.""" + """Capture the single request payload while serving the deterministic response.""" requests.append(request) return Response() @@ -2498,16 +2496,11 @@ def open_response(_opener, request, **_kwargs): ) assert verdict["decision"] == "approve" - assert len(requests) == 2 - prompts = [ - json.loads(request.data)["messages"][1]["content"] for request in requests - ] + assert len(requests) == 1 + prompt = json.loads(requests[0].data)["messages"][1]["content"] expected_manifest = noema.changed_line_manifest( sorted(noema.changed_diff_locations(diff)) ) - for prompt in prompts: - assert "Authoritative exact changed-side coordinate manifest" in prompt - assert expected_manifest in prompt - assert "sole coordinate authority" in prompt - assert "Noema reviewed line 3 is not an exact changed-side line" in prompts[1] - assert "using only exact changed-side locations from the authoritative coordinate manifest" in prompts[1] + assert "Authoritative exact changed-side coordinate manifest" in prompt + assert expected_manifest in prompt + assert "sole coordinate authority" in prompt