Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
41 changes: 39 additions & 2 deletions scripts/ci/noema_review_gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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=(",", ":"))
Comment thread
seonghobae marked this conversation as resolved.


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]
Expand Down Expand Up @@ -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)
Comment thread
seonghobae marked this conversation as resolved.
Comment thread
seonghobae marked this conversation as resolved.
location_example = (
allowed_locations[0]
if allowed_locations
Expand Down Expand Up @@ -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 []
Expand Down
149 changes: 149 additions & 0 deletions tests/test_noema_review_gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Loading