Preserve evidence and test caveats during response repair - #753
Conversation
|
Review verdict: Blocking The lossless-repair prompt contract and the new bounded preservation guard are well scoped, correctly wired into both execute_repair and the legacy attempt_repair path, and covered by focused tests; docs are updated. Blocking on one coverage gap: the guard silently skips the most common malformed shape (JSON wrapped in a code fence), because _payload only parses text whose first non-space character is '{'. GitHub PR checks are reported as passing; I ran no tests myself. Blocking issues
Same-PR follow-ups
-- Anthropic Claude: claude-opus-5 (medium) |
|
PR review round 1 reconciliation: settled reviewers: Claude, Antigravity. Finalization continues after reconciliation. |
Coder follow-upImplemented and pushed both requested repair-preservation fixes in commit 5735f5f. Addressed items
Remaining items
Tests run
-- OpenAI Codex: gpt-5.6-sol (medium) |
|
Review verdict: Approved Round 2 resolves both round-1 items. _payload now strips a leading ```/~~~ fence (with matching closing fence, tilde and longer-fence variants covered by a parametrized test) before extracting the source payload, so fenced malformed responses no longer bypass the loss check; _FINDING_METADATA_KEYS excludes short non-prose keys (severity, category, state, disposition, verdict) from required fragments while file/line/evidence text stays required, and the new test asserts both the accept and the reject side. Fence parsing degrades safely: an unparseable extraction returns None and the candidate falls back to the ordinary validator path rather than being wrongly rejected. Docs describe the guard's bounded scope accurately and no user-facing flag changed. GitHub checks are still pending (test in_progress), so I am not claiming the suite passed; I ran no tests locally. Prior unresolved item dispositions
-- Anthropic Claude: claude-opus-5 (medium) |
|
PR review round 2 reconciliation: settled reviewers: Claude, Antigravity. Finalization continues after reconciliation. |
|
Review status: Incomplete No coder follow-up was started for the unavailable reviewer(s). Their failure is not a code finding and does not count as approval. Missing required reviewer input
Healthy reviewer approvals
Resolve the reviewer problem or rerun with a replacement reviewer/model before merging PR #753. |
|
Review verdict: Blocking Both carried round-1 defects are fixed, but the preservation guard still allows repair to discard material coder evidence and change signed-requirement outcomes. Blocking issues
Prior unresolved item dispositions
-- OpenAI Codex: gpt-5.6-sol (medium) |
Coder follow-upImplemented and pushed the lossless repair-preservation fix for disputed-item evidence and signed human requirement outcomes. Addressed items
Remaining items
Tests run
-- OpenAI Codex: gpt-5.6-sol (medium) |
|
Review verdict: Blocking Round 3 resolves the prior dispute/requirement preservation gap, but the new guard now pins content that the schema and context validators require the repair to remove, making some coder_followup repairs unsatisfiable. Blocking issues
Prior unresolved item dispositions
-- Anthropic Claude: claude-opus-5 (medium) |
|
Review verdict: Blocking The round-2 dispute and signed-requirement outcome preservation gap is covered, and GitHub checks are passing, but the new guard conflicts with authoritative requirement-context repair when the source contains fabricated dispositions. Blocking issues
Prior unresolved item dispositions
-- OpenAI Codex: gpt-5.6-sol (medium) |
|
PR review round 3 reconciliation: settled reviewers: Claude, Codex. Finalization continues after reconciliation. |
Coder follow-upBound repair preservation to the authoritative unresolved-item and surfaced-requirement contexts, taught coder follow-up repair about disputes, added regressions, and pushed commit 421358a to the existing PR branch. Addressed items
Remaining items
Tests run
-- OpenAI Codex: gpt-5.6-sol (medium) |
|
Review verdict: Blocking Round 4 fixes the coder_followup dispute deadlock and the requirement label/context deadlock (item-4, item-5 resolved). item-6 remains blocking for its Prior unresolved item dispositions
-- Anthropic Claude: claude-opus-5 (medium) |
|
Review verdict: Blocking The round-3 requirement/dispute deadlocks are fixed, but the preservation guard still conflicts with authoritative validation in two other parseable repair cases. GitHub checks are passing; tests were not run because this review was restricted to read-only inspection. Blocking issues
Prior unresolved item dispositions
-- OpenAI Codex: gpt-5.6-sol (medium) |
|
PR review round 4 reconciliation: settled reviewers: Claude, Codex. Finalization continues after reconciliation. |
Coder follow-upUpdated repair preservation to follow authoritative item and signed-requirement context, added end-to-end regressions, and pushed commit 83ea74d to the existing PR branch. Addressed items
Remaining items
Tests run
-- OpenAI Codex: gpt-5.6-sol (medium) |
|
Review verdict: Approved Round 5 threads the reviewer requirement context into the preservation guard (pr_review pins nothing because the schema forbids the field; plan_review uses reviewer_requirement_ids) and exempts unknown item-note IDs alongside the existing dispute exemptions, with end-to-end execute_repair regressions for both review kinds and the unknown-note cleanup. I verified all four pr_review/plan_review repair call sites supply reviewer_requirement_ids and that plan_state/plan_revision/issue_implementation/coder_followup still receive surfaced_requirement_ids, so no schema-mandated removal is pinned any more. All three carried items are resolved and I found no new blockers. Note: GitHub checks are still pending (test in_progress), so I cannot confirm a green suite. Prior unresolved item dispositions
-- Anthropic Claude: claude-opus-5 (medium) |
|
Review verdict: Approved The preservation guard now honors authoritative coder and reviewer contexts, allowing fabricated requirement rows and unknown coder notes to be removed while retaining surfaced requirement dispositions and evidence. All three carried items are resolved. GitHub's test check is still in progress; no tests were run locally under the review constraints. Prior unresolved item dispositions
-- OpenAI Codex: gpt-5.6-sol (medium) |
|
PR review round 5 reconciliation: settled reviewers: Claude, Codex. Finalization continues after reconciliation. |
|
Reviewers approved PR #753; watching GitHub checks in the foreground. No coder or reviewer agents will run while checks remain pending. -- coding-review-agent-loop |
1 participant
Why
A replay of the latest ten retained Gemini repairs showed why schema acceptance alone is insufficient: GPT-OSS passed 10/10 and Luna passed 9/10, but both condensed detailed findings. GPT-OSS omitted actionable implementation/test requirements in one accepted repair. Luna also invented reviewer-item IDs as human requirements in one rejected repair, and removed a failing-test caveat. The historical Gemini baseline was not lossless either: one accepted repair omitted a test-command list.
The existing request to preserve intent is too permissive. Repair needs to fix representation without becoming another reviewer or summarizer.
Changes
Deliberate Limits
This is not semantic-equivalence certification or a new JSON-repair parser. Unparseable JSON, unsupported kinds, and individual strings containing reserved protocol grammar are outside the local content comparison. They retain the stronger prompt and existing schema/context validation. Required schema corrections, forbidden future-item removal, and protocol-safety sanitization remain allowed. The guard does not establish that underlying code/test claims are true.
No repair backend, model default, or CLI permission policy changes. No live model calls are needed by the regression tests.
Validation
python -m pytest -x -q).The full suite needed access outside the read-only-home sandbox because existing configuration tests create log directories under ~/.cache. An initial sandboxed run reported failures and hit its 180-second cap; a fail-fast rerun identified the read-only-directory error. The final full-suite run above completed with that access allowed.
-- OpenAI Codex