Skip to content

fix(noema): ground formal verdicts in exact changed-line manifest - #1612

Open
seonghobae wants to merge 8 commits into
mainfrom
fix/noema-exact-changed-line-manifest-20260901
Open

fix(noema): ground formal verdicts in exact changed-line manifest#1612
seonghobae wants to merge 8 commits into
mainfrom
fix/noema-exact-changed-line-manifest-20260901

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Incident

BandScope PR #1122 failed in Noema job 99792663163 with:

RuntimeError: Noema reviewed line 3 is not an exact changed-side line

Here, line 3 is the third reviewed_lines item. The strict validator correctly rejected a model-produced coordinate that was not present on the corresponding side of the unified diff.

Root cause

noema_review_gate.py already parsed the authoritative (path, line, side) set, but the model received only the raw diff and one schema example. The repair retry repeated a generic “use exact changed-side locations” instruction without exposing the permitted coordinates. A long-running retry could therefore make the same class of coordinate guess again.

Repair

  • serialize every parsed changed-side coordinate into a compact deterministic JSON manifest, grouped by file and LEFT/RIGHT inclusive ranges;
  • include the same manifest in the initial request and the one repair request;
  • declare it the sole coordinate authority for reviewed_lines, adversarial probes, and findings;
  • point the repair instruction at the manifest and include the exact validation error.

The deterministic post-response validator remains unchanged and fail-closed. This patch does not snap, coerce, filter, or silently discard invalid coordinates, and does not downgrade a formal verdict to obtain a green check.

TDD evidence

  • RED: the BandScope-style invalid third reviewed-line item failed before implementation because the manifest interface did not exist;
  • focused Noema suite: 116 passed;
  • combined exact-tree repository verification: 2342 passed, 1 skipped, 21 subtests passed;
  • scripts/ci: 11019 statements, 4440 branches, 0 missed, 0 partial, 100%;
  • docstring, compile, and git diff --check gates passed.

Verification run: https://github.com/ContextualWisdomLab/.github/actions/runs/33514831758

Dependency and rollout

PR #1610 is a separate test-only repair for a protected-main Strix coverage regression discovered while running the full suite. It must land first so this PR's repository-wide coverage-evidence check reflects the Noema change rather than that pre-existing baseline gap.

After both PRs merge, rerun Noema against BandScope PR #1122 and require a fresh exact-head verdict before considering the production incident closed.


Devin Review

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.
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.
@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: f95a9660-16b5-4113-8404-9c65bd123132


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

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

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 potential issues.

Devin Review

Comment thread scripts/ci/noema_review_gate.py
Comment thread scripts/ci/noema_review_gate.py
Comment thread scripts/ci/noema_review_gate.py
seonghobae added a commit that referenced this pull request Sep 1, 2026
QUEUE_SATURATION_CHICKEN_EGG: exact head 1e121e0 is a one-file test-only coverage repair with RED/GREEN/full-suite evidence, zero review threads, Devin no-issues and CodeRabbit/Devin success; remaining required workflows are queued under central Actions saturation. This repair is prerequisite evidence infrastructure for #1612.
Construct this reconciliation tree from protected main a86177e, then reapply only PR #1612's four-file semantic delta. Preserve the concurrent exact-head-evidence wake commit 63897f6, #1610's Strix coverage repair, and every unrelated main change. No force push or destructive rebase.

Copy link
Copy Markdown
Contributor Author

The exact-head-path-policy check failure on this PR's head is not from this PR's diff (which only touches noema_review_gate.py + tests). scripts/ci/test_strix_quick_gate.sh's bash suite asserts .github/workflows/strix.yml exports LLM_TIMEOUT=0, but the workflow currently hardcodes LLM_TIMEOUT=300 — confirmed this same assertion fails identically against origin/main directly, independent of this PR.

Opened #1658 with the one-line fix (3000, matching the three sibling timeout env vars on the following lines, which are already 0). Once #1658 merges, this PR's exact-head-path-policy check will pass without needing a rebase (the check re-clones main fresh each run). Not merging it into this PR itself since it's unrelated to the noema line-manifest change.

Claude-Session: https://claude.ai/code/session_019YgmaEKnPhrsbSyQBierFH


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Both dependencies this PR's exact-head-path-policy failure needed are now merged to main: #1610 (Strix compat-entrypoint coverage, merged 13:57) and #1658 (LLM_TIMEOUT=0, merged 00:57). Re-triggered the failed job (rerun_failed_jobs on run 33517130928) to get a fresh evaluation against current main.

Claude-Session: https://claude.ai/code/session_019YgmaEKnPhrsbSyQBierFH


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Naruon consumer-path advancement from fresh live state: dependency #1610 is now merged, but this PR is still open with head 03437786ad57dad849a996c334911d294dc4bcee, while protected .github/main has advanced to 2792b964b321d096ed292979e175510cf94aa03c and the PR currently reports non-mergeable. Please reconstruct/restack this owner repair non-destructively on current protected main, preserving only the exact changed-line-manifest delta and regenerating all current-head evidence rather than transferring the earlier 33514831758 run.

Please also include the existing Naruon #1613 canary in post-merge acceptance: ContextualWisdomLab/naruon#1485@4e2cb3b39f89aff811d5113962861bd1963d2996 previously failed required Noema run 33444969216 / job 99781918923 with Noema reviewed line 1 is not an exact changed-side line after repository-native gates were green. After the repaired central path reaches protected main, Naruon will rerun that unchanged head. GREEN is a genuine exact-line formal verdict or an early typed review-unavailable/model-output-invalid result; nearest-line coercion, synthetic source findings, predecessor evidence, and leaf source churn remain prohibited.

@seonghobae seonghobae added area: ci-cd CI, GitHub Actions, checks, release, or supply chain bug Something isn't working priority: high High-priority or P1 work status: blocked Blocked by conflict, dependency, or required prerequisite type: bug Defect or incorrect behavior labels Sep 2, 2026 — with ChatGPT Codex Connector
# Conflicts:
#	CHANGELOG.md
#	scripts/ci/noema_review_gate.py

Copy link
Copy Markdown
Contributor Author

Stale base resolved — real logic conflict, adapted (not mechanically merged) to match main's now-single-request architecture

Branch was dirty against current main (base recorded here was a86177e). git fetch origin main && git merge --no-edit conflicted in CHANGELOG.md and scripts/ci/noema_review_gate.py (tests/test_noema_review_gate.py auto-merged cleanly).

The noema_review_gate.py conflict was a real logic conflict, not a formatting one — please review the adaptation. main has since shipped "Noema single-request gateway ownership" (removed the repository-owned repair-retry entirely; call_llm's current signature has no is_retry/repair_error parameters — confirmed these are genuinely gone, not just unused, since keeping them would be a NameError against the now-current signature). This PR's own retry-correction branch (*(["Your prior verdict was rejected..."] if is_retry else [])) is therefore incompatible with main as it stands.

Kept this PR's actual root-cause fix — changed_line_manifest() and the "Authoritative exact changed-side coordinate manifest... sole coordinate authority" prompt guidance — and included it in main's one response_format-based request. Dropped only the retry-specific branch, since there is no longer a second request to ground. location_example's raw JSON-shape example (superseded by main's response_format schema) was also dropped rather than kept alongside it. Correspondingly rewrote tests/test_noema_review_gate.py::test_call_llm_grounds_initial_and_repair_requests_in_authoritative_manifesttest_call_llm_grounds_its_single_request_in_authoritative_manifest: asserts the manifest reaches the one request call_llm now sends (len(requests) == 1), rather than mocking two sequential responses and asserting retry-specific prompt text that no longer exists. Fail-closed rejection of an invented coordinate is unaffected and already covered generically by validate_substantive_verdict's own direct tests in the same file (line ~2259), independent of this PR.

Also updated the CHANGELOG entry's wording to describe grounding "the one structured-output request" rather than "the first request and its repair retry," with a pointer to the single-request-ownership entry for context.

Verified before pushing (head confirmed unchanged at 03437786 immediately prior):

  • python3 -m py_compile scripts/ci/noema_review_gate.py — compiles; confirmed call_llm's post-merge signature has no leftover is_retry/repair_error references
  • python3 -m pytest tests/test_noema_review_gate.py -q107 passed
  • Full suite: coverage run -m pytest tests -q2764 passed, 1 skipped, 21 subtests (excluding the 2 pre-existing Python-3.11-vs-3.12+ sandbox-incompatible files, unrelated to this diff)
  • coverage report --include="scripts/ci/*"100% (12367 statements / 5006 branches)
  • interrogate --fail-under=100 scripts/ci100%
  • git diff --check — clean

Pushed non-force as 1e09e0ab (two-parent merge commit, 03437786..1e09e0ab). Given the substantive adaptation (not a mechanical merge), this is a good candidate for a careful human/reviewer look at the diff before it's treated as routine — fresh exact-head checks and review are required regardless per this repo's governance model.


Generated by Claude Code

…e merge commit

The prior merge commit (1e09e0a) 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4

Copy link
Copy Markdown
Contributor Author

Correction: the test rewrite from my previous comment was verified locally but not actually pushed — fixed now

My prior comment on this PR described tests/test_noema_review_gate.py's rewrite as part of commit 1e09e0ab. It was edited and genuinely verified on disk (the "107 passed" and full-suite numbers I reported were real, run against the working tree with that edit present) but never git added before that commit — so the pushed 1e09e0ab still contained the old, now-incompatible test (referencing the removed is_retry/repair_error retry path), which would fail. Sorry for the confusion — caught it myself on a routine follow-up check, before anyone else acted on the earlier comment.

Committed the missing test-file change as a separate follow-up commit and re-ran everything against the now-actually-complete tree:

  • python3 -m pytest tests/test_noema_review_gate.py -q107 passed
  • Full suite: coverage run -m pytest tests -q2764 passed, 1 skipped, 21 subtests
  • coverage report --include="scripts/ci/*"100%
  • interrogate --fail-under=100 scripts/ci100%

Pushed non-force as 2619998e (1e09e0ab..2619998e). Head is now 2619998e; that's the SHA fresh checks and review should evaluate, not 1e09e0ab.


Generated by Claude Code

seonghobae and others added 2 commits September 5, 2026 21:10
Resolve the CHANGELOG.md conflict by keeping both sides: this branch's entry
above main's, matching the file's newest-first convention. No entry dropped.

Verified after resolution: 2893 passed, 1 skipped, coverage 100%,
interrogate 100%; zero conflict markers; `ruff check --select F821` clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci-cd CI, GitHub Actions, checks, release, or supply chain bug Something isn't working priority: high High-priority or P1 work status: blocked Blocked by conflict, dependency, or required prerequisite type: bug Defect or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants