fix(noema): report the actual rejected location, not just an array position

[seonghobae]

The bug

scripts/ci/noema_review_gate.py's validate_substantive_verdict() has two loops that validate the Noema LLM reviewer's JSON verdict cites real changed-diff lines:

for index, reviewed in enumerate(reviewed_lines, start=1):
    ...
    if location not in locations:
        raise RuntimeError(f"Noema reviewed line {index} is not an exact changed-side line")

and the analogous loop for probes. index here is the entry's array position within reviewed_lines/probes — not a source-code line number — but the message text ("Noema reviewed line 3 is not an exact changed-side line") reads exactly like a citation to literal file line 3. Worse, neither loop ever included the actual rejected (path, line, side) tuple the model submitted, nor anything from the diff's real changed-line set, in the raised message.

This was originally investigated on ContextualWisdomLab/naruon#1503, where the check fired twice — once as "Noema reviewed line 3 is not an exact changed-side line" and once as "Noema reviewed line 1 is not an exact changed-side line" (job ids 99740003119, 99740827824, 99742973829, 99745529545, 99746600989, 99748382284, 99873344797). Both were initially misread as the LLM hallucinating a citation to literal file line 1 or 3, before tracing into this code and realizing "1" and "3" were just array positions, with the actual submitted path/line/side nowhere visible in the CI logs.

Since this required workflow gates every PR org-wide, every occurrence of this failure — in any sibling repo — has been undiagnosable from CI output alone until now.

What changed

In scripts/ci/noema_review_gate.py:

• _entry_ordinal(position, total) — renders "entry N/total (array index N-1, not a source line)" in place of the bare index. It keeps the fixed "Noema reviewed line " / "Noema adversarial probe " prefix so _stable_failure_diagnostic()'s trusted-prefix allowlist still passes these messages through unredacted on the repair-retry path (that allowlist keys off the literal prefix text, so it did not need to change).
• _format_location(path, line, side) — repr()s the raw rejected values so a wrong path, a wrong side, None, or a non-int line is visible and unambiguous in the message.
• _nearby_changed_locations(locations, path, line) — adds up to 5 of the nearest real changed locations sharing the same path, sorted by distance from the cited line, so an off-by-one or near-miss citation is obvious at a glance. Empty when the path doesn't appear in the diff at all.

Example, before vs. after (same underlying failure — an approve verdict citing tool.py:99 RIGHT when the diff only touched line 2):

Before: Noema reviewed line 1 is not an exact changed-side line
After:  Noema reviewed line entry 1/1 (array index 0, not a source line) cites
        path='tool.py' line=99 side='RIGHT', which is not an exact changed-side
        line; nearest changed lines for tool.py: tool.py:2 (LEFT), tool.py:2 (RIGHT)

Both enumerate(..., start=1) loops in this file follow the fixed pattern above and are now identical in shape; a repo-wide grep confirmed these are the only two occurrences of this pattern in noema_review_gate.py.

Investigated but ruled out: a location-computation bug

Per the task brief, I also checked changed_diff_locations() and the diff/prompt-construction path for a systematic off-by-one or path-stripping bug that could produce a wrong location rather than just report it poorly. I built a real multi-hunk git diff (context lines, a pure insertion, a line replacement near the end of a hunk, a second modified file, and a new file) and verified every (path, line, side) tuple changed_diff_locations() computed against the actual file content — all matched exactly, no off-by-one. I also confirmed the exact same diff string is used both to compute the location set and to build the model's prompt (so the model is never shown a diff view that disagrees with what's validated), and that the JSON schema example embedded in the prompt uses the same prefix-stripped path convention ("file.py", not "a/file.py") that the validator expects. No provable location-computation bug found — only the reporting gap above was fixed, and the validation itself was not loosened.

Tests

• Updated the two contract-style assertions in test_substantive_verdict_fail_closed_boundaries that pinned the old "reviewed line 1 must be an object" / "probe 1 must be an object" exact text.
• Added test_entry_ordinal_names_an_array_position_not_a_line_number, test_format_location_reprs_every_raw_field, and test_nearby_changed_locations_covers_every_branch (non-string path, no-match path, int-line distance sort, non-int-line fallback sort, and the +N more truncation branch) for the three new helpers directly.
• Added test_validate_substantive_verdict_reports_rejected_location_and_nearby_hint and test_validate_substantive_verdict_probe_rejection_reports_location_and_hint for the end-to-end enriched messages, including the no-hint case (cited path never touched by the diff).

Verified locally (Python 3.12, matching this repo's target):

coverage run -m pytest tests -q       # 2605 passed, 1 skipped, 21 subtests passed
coverage report --show-missing        # scripts/ci: 100% lines, 100% branches (fail_under=100)
interrogate                            # 100% docstrings (fail-under=100), noema_review_gate.py: 100%
ruff check .                           # only pre-existing findings elsewhere in the repo; zero new
                                        # findings in either file this PR touches (ruff has no config
                                        # in this repo and isn't wired into any workflow, so it's not
                                        # an enforced gate here — checked anyway per the task brief)

Constraints respected

• Validation strictness is unchanged — a non-matching citation is still rejected exactly as before; only the diagnostic text was enriched.
• No develop/main changes, no force-push, no merge — this PR is left open for the org's normal review/merge automation.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_016ErwZSYW3pm585NiM3Q7aN

---

Generated by Claude Code

---

[coderabbitai]

Warning

Review limit reached

Next included review available in 2 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 03f5130d-40e5-4229-8af1-27fe93a2db0d

📥 Commits

Reviewing files that changed from the base of the PR and between 669505b and 9f2b404.

📒 Files selected for processing (2)

• scripts/ci/noema_review_gate.py
• tests/test_noema_review_gate.py

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[seonghobae]

QUEUE_SATURATION_CHICKEN_EGG admission decision for exact head 9f2b4048953fcc2f20f8743d6d0f4a0125a38f72: protected main advanced to 89fe60d0682fdd28c0639bb5eade7f2caeb36e19 after #1670, and GitHub REST now reports this PR mergeable=true, rebaseable=true, mergeable_state=behind (not conflicted). Its two-file scope is isolated to Noema diagnostic rendering plus tests; validation strictness is unchanged. Devin Review reports no issues, unresolved review threads are zero, CodeRabbit/Devin exact-head statuses are success, and the branch records 2,605 tests passed with 100% line/branch coverage and 100% docstrings. All visible exact-head hosted security workflows remain queued under the saturated central Actions fleet. The change removes a cross-repository diagnosis blind spot in a required reviewer gate and does not weaken admission semantics. No substantive failing test, security finding, CHANGES_REQUESTED, malformed provenance, conflict, or unrelated policy defect is being bypassed. Proceeding with exact-head guarded squash merge; GitHub will construct the merge against the live protected base.