Repair plan revisions must remove fabricated human-requirements acknowledgements

[wwind123]

Problem

While revising #554, Codex produced a valid plan_revision JSON payload but appended:

<!-- HUMAN_REQUIREMENTS_ADDRESSED -->
### Human requirements
- Requirement 1: addressed ...

The issue had no signed -- Human Reviewer comment, so no human requirements were surfaced. The validator correctly rejected the response. The Antigravity repair pass then reproduced the same invalid marker and section.

The repair prompt is contradictory: it says that no surfaced IDs requires an empty acknowledgement set, but separately instructs plan-revision repair to preserve a human-requirements marker and section whenever the malformed original contains them. The latter wins in practice, so repair cannot resolve this deterministic formatting failure.

Expected behavior

• Treat the active repair_surfaced_requirement_ids / direct-discussion context as authoritative.
• For plan_state and plan_revision, when there are no surfaced signed human requirements and no required direct-discussion acknowledgement, remove any fabricated HUMAN_REQUIREMENTS_ADDRESSED marker and ### Human requirements section from the repaired output.
• Preserve an acknowledgement only when the active repair context actually has surfaced requirement IDs or explicitly requires direct-discussion acknowledgement.
• The repair prompt must not use a malformed response as evidence that a human requirement exists.

Regression coverage

Add a plan-revision repair fixture where an issue discusses Requirement N labels as acceptance criteria but contains no signed human comment. Seed a malformed plan revision with a fabricated acknowledgement and assert repair produces a schema-valid revision without the marker/section. Also retain coverage that genuinely surfaced signed requirements preserve their acknowledgement.

[wwind123]

Plan

Make plan-state and plan-revision repair use the active human-requirements context, so fabricated acknowledgements are removed when no signed requirement or direct-discussion acknowledgement is required while legitimate acknowledgements remain intact.

Plan steps

1. Update the plan-state/plan-revision repair prompt contract in src/coding_review_agent_loop/repair.py so active surfaced requirement IDs and direct-discussion status, rather than acknowledgement text in the malformed response, determine whether the HUMAN_REQUIREMENTS_ADDRESSED marker and Human requirements section must be emitted, preserved, or removed.
2. Extend the repair prompt/attempt plumbing to accept the planning human-requirements context for plan_state and plan_revision (while retaining the existing coder-followup and reviewer-specific paths), and have _run_validated_agent forward repair_surfaced_requirement_ids plus repair_requires_direct_discussion_ack for both initial planning and revision repair calls in src/coding_review_agent_loop/orchestrator.py.
3. Revise the contradictory Format J/Format D/footer and worked-example instructions so an empty active context explicitly strips any fabricated legacy acknowledgement, while surfaced signed IDs or an explicitly required direct-discussion check continue to require a valid acknowledgement and matching structured dispositions.
4. Add focused tests in tests/test_repair.py for the generated repair prompt and repair arguments across no-requirement, surfaced-signed-requirement, and direct-discussion-required planning contexts; ensure existing signed-requirement preservation coverage remains valid.
5. Add an issue-loop or validated-agent regression fixture in tests/test_orchestrator_issue.py or tests/test_repair.py that uses Requirement N acceptance-criteria text without a signed Human Reviewer comment, seeds a malformed plan_revision with a fabricated acknowledgement, returns a repaired bare structured revision, and asserts it validates and contains neither the marker nor section; run the focused repair/orchestrator tests, then the project pytest suite.

-- OpenAI Codex: gpt-5.6-terra

[wwind123]

Review verdict: Blocking

Plan correctly targets the real bug: repair.py's Format D notes (line 652) and Format J note (line 420) preserve/keep the HUMAN_REQUIREMENTS_ADDRESSED marker based on whether the malformed original contained it, and orchestrator.py's repair_kwargs gate (around line 2134-2138) forwards surfaced_requirement_ids only for expected_kind == "coder_followup", so the context already computed and passed at the plan_revision call site (orchestrator.py:3825-3830) is silently dropped, and the initial plan_state call (orchestrator.py:3058-3075) never computes/passes this context at all. The plan's five steps correctly scope the fix to repair.py's prompt contract, orchestrator.py's plumbing for both plan_state and plan_revision, and new/retained tests. One same-plan refinement is needed before implementation.

Same-plan follow-ups

• tests/test_repair.py::test_repair_prompt_plan_revision_preserves_human_requirements_acknowledgement (around line 1510-1513) asserts the literal prompt text 'If the original plan revision includes and a ### Human requirements section, preserve both after the JSON and before ' (repair.py:652). That sentence is itself the contradictory 'preserve whenever the malformed original contains them' rule this issue asks to remove, so step 3's rewrite must replace it with wording conditioned on the active repair_surfaced_requirement_ids / direct-discussion-ack context, not on what the malformed input contained. Step 4 currently only says to 'ensure existing signed-requirement preservation coverage remains valid,' which could be misread as 'leave this test's assertion untouched.' Make step 4 explicit that this test (and the analogous Format J note at repair.py:420) must be rewritten to pin the new conditional-on-active-context wording, so the implementer doesn't either break this test or keep the old unconditional-preserve prompt text just to keep it green.

-- Anthropic Claude: claude-sonnet-5

[wwind123]

Review verdict: Approved

The plan correctly addresses the issue by threading the active human-requirements context into the repair prompt and updating the prompt instructions to make the context authoritative.

-- Google Antigravity: Gemini 3.1 Pro (High)

[wwind123]

Revised plan

Make planning-response repair derive human-requirements acknowledgements exclusively from the active signed-requirement/direct-discussion context, removing fabricated legacy blocks while retaining required legitimate acknowledgements.

Prior plan item dispositions

• [item-1] Same-plan follow-up from Anthropic Claude: claude-sonnet-5, round 1: tests/test_repair.py::test_repair_prompt_plan_revision_preserves_human_requirements_acknowledgeme... -> resolved: The prompt-contract and test steps now explicitly replace the stale unconditional plan-revision preservation assertion and the analogous Format J instruction with active-context-conditioned behavior.

Plan steps

1. In src/coding_review_agent_loop/repair.py, generalize the repair prompt/attempt inputs for plan_state and plan_revision so they receive the active surfaced signed requirement IDs and whether direct-discussion acknowledgement is required. Render planning-specific instructions that make this context authoritative: when IDs are empty and direct acknowledgement is not required, remove any legacy <!-- HUMAN_REQUIREMENTS_ADDRESSED --> marker and ### Human requirements section and emit no fabricated dispositions; when signed IDs are surfaced or direct acknowledgement is required, require the valid acknowledgement placement and the corresponding structured human-requirement dispositions. Preserve the existing coder-followup and reviewer-specific prompt paths and their input validation.
2. Rewrite repair.py's Format J, Format D, footer guidance, and worked plan-revision example so neither an acknowledgement block nor its absence in the malformed response is evidence of a requirement. In particular, replace the Format D note currently asserting that an original plan revision with both legacy elements must preserve them, and replace the analogous Format J wording, with the conditional active-context rule; keep the example explicitly scoped to a context that has genuinely surfaced signed requirements.
3. In src/coding_review_agent_loop/orchestrator.py, forward repair_surfaced_requirement_ids and repair_requires_direct_discussion_ack to the structured repair invocation for planning expected kinds as well as the existing coder-followup path. Before the initial plan_state call in _run_plan_first_loop, build the same planning human-requirements context already built before each plan_revision call and pass its IDs/direct-discussion flag into _run_validated_agent; retain the existing revision context and recovery/validation behavior.
4. Update focused prompt and attempt-plumbing tests in tests/test_repair.py for plan_state and plan_revision in all three contexts: no surfaced signed requirements, surfaced signed Requirement N IDs, and direct-discussion acknowledgement required without IDs. Assert the no-context guidance tells repair to strip both legacy markdown elements, while the other contexts require a correctly placed acknowledgement and valid structured dispositions. Rewrite test_repair_prompt_plan_revision_preserves_human_requirements_acknowledgement rather than retaining its literal unconditional-preserve assertion, and add/adjust the analogous Format J assertion so both pin the new active-context wording and assert the obsolete malformed-original rule is absent.
5. Add a repair/validated-agent regression fixture (in tests/test_repair.py, or the issue-loop coverage if it better exercises the call site) whose issue discussion contains Requirement N acceptance-criteria prose but no signed -- Human Reviewer comment. Seed a malformed plan_revision containing a fabricated marker/section, have the repair result return a bare structured revision, and assert validation succeeds with no marker or Human requirements section. Retain or extend the signed-requirement fixture to prove repair preserves a required acknowledgement, and cover initial-plan context forwarding. Run the focused repair and orchestration tests, then python -m pytest.

-- OpenAI Codex: gpt-5.6-terra

[wwind123]

Review verdict: Approved

Round 2 plan resolves the prior same-plan follow-up: step 2 explicitly targets rewriting the Format D note (repair.py:652) and analogous Format J wording so preservation depends on active surfaced-requirement/direct-discussion context rather than what the malformed original contained, and step 4 explicitly commits to rewriting test_repair_prompt_plan_revision_preserves_human_requirements_acknowledgement's literal assertion (not just retaining it) plus an adjusted Format J assertion. Verified against the current code: orchestrator.py's repair_kwargs gate only forwards surfaced_requirement_ids when repair_expected_kind == 'coder_followup', repair.py has no requires_direct_discussion_ack parameter in its prompt-building at all today (that flag currently only drives a separate _recover_plan_revision_human_requirements_acknowledgement fallback, which is already correctly gated), and the initial plan_state call in _run_plan_first_loop never builds/passes this context. Plan steps 1 and 3 correctly commit to adding this plumbing for both plan_state and plan_revision while preserving the coder-followup and reviewer paths, and step 4's three-context test matrix (no requirements, surfaced signed IDs, direct-discussion required) covers the relevant edge cases. No blocking issues or further same-plan refinements found this round.

Prior unresolved plan item dispositions

• [item-1] Same-plan follow-up from Anthropic Claude: claude-sonnet-5, round 1: tests/test_repair.py::test_repair_prompt_plan_revision_preserves_human_requirements_acknowledgeme... -> resolved: Plan step 2 now explicitly rewrites the Format D preserve-whenever-malformed-contains-it note and the analogous Format J wording to be conditioned on active context, and step 4 explicitly commits to rewriting (not just retaining) test_repair_prompt_plan_revision_preserves_human_requirements_acknowledgement's literal assertion plus an adjusted Format J assertion asserting the obsolete rule is absent.

-- Anthropic Claude: claude-sonnet-5

[wwind123]

Review verdict: Approved

The plan accurately addresses the issue by relying on the active repair context rather than the malformed response for human requirements acknowledgements, and correctly updates orchestrator plumbing and repair tests.

Prior unresolved plan item dispositions

• [item-1] Same-plan follow-up from Anthropic Claude: claude-sonnet-5, round 1: tests/test_repair.py::test_repair_prompt_plan_revision_preserves_human_requirements_acknowledgeme... -> resolved: Replaced the unconditional preservation prompt rules and updated the related tests to assert the active context logic.

-- Google Antigravity: Gemini 3.1 Pro (High)

[wwind123]

Planning complete for issue #558.

Outcome: implement

Approved plan:

Make planning-response repair derive human-requirements acknowledgements exclusively from the active signed-requirement/direct-discussion context, removing fabricated legacy blocks while retaining required legitimate acknowledgements.

Prior plan item dispositions

• [item-1] Same-plan follow-up from Anthropic Claude: claude-sonnet-5, round 1: tests/test_repair.py::test_repair_prompt_plan_revision_preserves_human_requirements_acknowledgeme... -> resolved: The prompt-contract and test steps now explicitly replace the stale unconditional plan-revision preservation assertion and the analogous Format J instruction with active-context-conditioned behavior.

Plan steps

1. In src/coding_review_agent_loop/repair.py, generalize the repair prompt/attempt inputs for plan_state and plan_revision so they receive the active surfaced signed requirement IDs and whether direct-discussion acknowledgement is required. Render planning-specific instructions that make this context authoritative: when IDs are empty and direct acknowledgement is not required, remove any legacy <!-- HUMAN_REQUIREMENTS_ADDRESSED --> marker and ### Human requirements section and emit no fabricated dispositions; when signed IDs are surfaced or direct acknowledgement is required, require the valid acknowledgement placement and the corresponding structured human-requirement dispositions. Preserve the existing coder-followup and reviewer-specific prompt paths and their input validation.
2. Rewrite repair.py's Format J, Format D, footer guidance, and worked plan-revision example so neither an acknowledgement block nor its absence in the malformed response is evidence of a requirement. In particular, replace the Format D note currently asserting that an original plan revision with both legacy elements must preserve them, and replace the analogous Format J wording, with the conditional active-context rule; keep the example explicitly scoped to a context that has genuinely surfaced signed requirements.
3. In src/coding_review_agent_loop/orchestrator.py, forward repair_surfaced_requirement_ids and repair_requires_direct_discussion_ack to the structured repair invocation for planning expected kinds as well as the existing coder-followup path. Before the initial plan_state call in _run_plan_first_loop, build the same planning human-requirements context already built before each plan_revision call and pass its IDs/direct-discussion flag into _run_validated_agent; retain the existing revision context and recovery/validation behavior.
4. Update focused prompt and attempt-plumbing tests in tests/test_repair.py for plan_state and plan_revision in all three contexts: no surfaced signed requirements, surfaced signed Requirement N IDs, and direct-discussion acknowledgement required without IDs. Assert the no-context guidance tells repair to strip both legacy markdown elements, while the other contexts require a correctly placed acknowledgement and valid structured dispositions. Rewrite test_repair_prompt_plan_revision_preserves_human_requirements_acknowledgement rather than retaining its literal unconditional-preserve assertion, and add/adjust the analogous Format J assertion so both pin the new active-context wording and assert the obsolete malformed-original rule is absent.
5. Add a repair/validated-agent regression fixture (in tests/test_repair.py, or the issue-loop coverage if it better exercises the call site) whose issue discussion contains Requirement N acceptance-criteria prose but no signed -- Human Reviewer comment. Seed a malformed plan_revision containing a fabricated marker/section, have the repair result return a bare structured revision, and assert validation succeeds with no marker or Human requirements section. Retain or extend the signed-requirement fixture to prove repair preserves a required acknowledgement, and cover initial-plan context forwarding. Run the focused repair and orchestration tests, then python -m pytest.

-- coding-review-agent-loop

[wwind123]

Approved plan for issue #558 handed off to PR #564 for one-shot implementation.

Mode: implement-one-shot
Plan hash: 4560c4332e2303f2
Plan subject: 4560c4332e2303f2a7cc9f341aeb167a85c406f83eefa7642eea0b44339c6a96

Parent reruns will resume the PR review loop for this PR instead of re-implementing.

-- coding-review-agent-loop