Separate issue acceptance criteria from signed human requirements in agent prompts

[wwind123]

Problem

A plan review for llm-dialectic#352 correctly approved the plan, but Claude treated the issue acceptance criteria as signed human requirements and emitted identifiers such as:

Requirement: tooltip works with mouse, keyboard, and touch

The response validator only accepts exact IDs surfaced by the signed-comment protocol, for example Requirement 1. No signed requirements were surfaced for this run, so the valid response should have used human_requirement_dispositions: [] and omitted <!-- HUMAN_REQUIREMENTS_RESOLVED -->. The Antigravity repair pass repeated the invalid acknowledgement instead of removing it.

Cause

The compact plan-review prompt says signed human requirements are approval-critical, then presents ordinary issue acceptance criteria nearby without clearly identifying whether any signed requirements were actually surfaced. Agents reasonably infer that the acceptance criteria are the requirements to acknowledge, while validation rejects that interpretation.

Expected behavior

• Render an explicit, separate Signed human requirements section in every relevant prompt.
• When none were surfaced, say that explicitly and direct agents to emit an empty dispositions array and no resolved marker.
• Keep ordinary issue acceptance criteria in a distinct section and state that they must not be used as signed-requirement IDs.
• Make the repair prompt apply the same distinction and remove, rather than preserve, invalid human-requirement acknowledgements.
• Add regression coverage for a plan review with acceptance criteria but no signed human requirements.

Related

• Repair plan revisions must remove fabricated human-requirements acknowledgements #558 covers repair behavior for fabricated human-requirement acknowledgements.
• This issue covers the upstream prompt/schema ambiguity that caused the fabrication.

[wwind123]

Plan

Clarify planning prompts so ordinary issue acceptance criteria and signed human requirements are rendered as separate, unambiguous contexts, including the explicit no-signed-requirements path used by repair.

Plan steps

1. Update the issue/planning prompt helpers in src/coding_review_agent_loop/prompts.py to always render a dedicated Signed human requirements section, enumerate only protocol-surfaced Requirement N entries there, and in the empty case explicitly require human_requirement_dispositions: [] with no HUMAN_REQUIREMENTS_ADDRESSED or HUMAN_REQUIREMENTS_RESOLVED marker.
2. Render the issue body and its ordinary acceptance criteria/context in a separate labeled prompt section, with direct wording that acceptance criteria guide the implementation but are never signed-requirement IDs; apply the distinction consistently to initial-plan, full and compact plan-review, plan-revision, and decomposition planning prompt paths while preserving bounded/omitted-requirement behavior.
3. Align the plan-review response protocol and src/coding_review_agent_loop/repair.py planning repair instructions so a repair receives the same authoritative signed-requirements versus issue-criteria distinction and strips fabricated dispositions/legacy acknowledgement markup when no signed requirements were surfaced.
4. Extend tests/test_prompts.py and tests/test_repair.py with regression cases containing issue acceptance criteria but an empty signed-requirements set, asserting the dedicated empty section, empty dispositions/no resolved marker guidance, criteria separation in both full and compact planning prompts, and repair removal behavior; retain surfaced-requirement and truncation/direct-discussion coverage, then run the targeted prompt/repair tests followed by python -m pytest.

-- OpenAI Codex: gpt-5.6-terra

[wwind123]

Review verdict: Blocking

The prompt-side fix (steps 1-2) is well-grounded in the actual gap in _human_requirements_block/_human_requirements_review_guidance/_coder_human_requirements_guidance, which all currently return "" when no signed requirements are surfaced instead of rendering an explicit empty-case section. Step 3 omits the orchestrator wiring that actually causes the reported repair failure: the generic schema-validation repair path never forwards reviewer_requirement_ids/surfaced_requirement_ids for a plan_review kind, so a repair pass triggered by an invalid human_requirement_dispositions entry gets zero signed-requirements context today.

Blocking plan issues

• The plan's repair scope (step 3) targets 'repair.py planning repair instructions' but does not name the actual defect that produced the issue's symptom. validate_human_requirement_dispositions (protocol.py:947) raises a plain AgentLoopError for a fabricated/unknown requirement_id or a non-empty array when none were surfaced. That error flows into the generic schema-validation repair path in _run_validated_agent (orchestrator.py:2121-2151), which for repair_expected_kind == "plan_review" only sets repair_kwargs = {"expected_kind": ..., "allowed_prior_item_ids": ...} -- it never sets reviewer_requirement_ids. _reviewer_human_requirements_instruction (repair.py:1357) short-circuits to "" when reviewer_requirement_ids is None (repair.py:1361-1362), and attempt_repair/_build_repair_prompt reject passing surfaced_requirement_ids for plan_review entirely (repair.py:1068-1074, enforced by _build_repair_prompt's ValueError and covered by tests/test_repair.py:1696-1704). The only place reviewer_requirement_ids is actually populated today is the separate, narrower 'approved review missing HUMAN_REQUIREMENTS_RESOLVED' post-processing repair at orchestrator.py:3426-3444 (and the pr_review analogue near line 4671), which only fires when issue_context.human_requirements is truthy (orchestrator.py:3362) and only rewords a missing marker -- it does not run for a plan_review response that already failed the mandatory JSON schema check due to a fabricated human_requirement_dispositions entry. Net effect: today, when a reviewer fabricates a disposition like the issue's Requirement: tooltip works with mouse, keyboard, and touch example with zero signed requirements surfaced, the generic repair pass receives the raw malformed text plus only the static Format-B example ({"requirement_id": "Requirement 1", ...} in _REPAIR_PROMPT, repair.py:244-246) and no instruction that the array must be empty in this run -- which is exactly why the Antigravity repair pass in the linked run preserved rather than removed the fabrication. Please extend the plan to (a) wire the equivalent of reviewer_requirement_ids (or surfaced_requirement_ids) into the generic repair_kwargs builder in _run_validated_agent/orchestrator.py for plan_review (and, for consistency, pr_review) schema-validation failures -- not just the missing-marker path -- and (b) add or adapt a repair.py instruction (distinct from _reviewer_human_requirements_instruction's 'missing HUMAN_REQUIREMENTS_RESOLVED' wording, which doesn't fit an 'unknown/fabricated requirement_id' or 'must be empty' validation failure) that tells the repair pass to strip invalid human_requirement_dispositions entries and re-emit [] when the authoritative surfaced-ID list is empty.
• Because of the above, the step 4 regression coverage as scoped (tests/test_prompts.py and tests/test_repair.py only) would validate the new prompt wording and repair.py's prompt-building helpers in isolation, but would not catch the orchestrator-level wiring gap that is the actual cause of the issue's reported symptom ('Antigravity repair pass repeated the invalid acknowledgement instead of removing it'). Add an orchestrator-level regression case (tests/test_agent_loop.py, in the style of the existing _run_validated_agent/FakeRunner-based plan_review repair tests) that feeds a malformed plan_review response containing a fabricated human_requirement_dispositions entry with no surfaced signed requirements through the real repair path and asserts the repair call/prompt actually receives the empty-surfaced-IDs context and the final validated output has human_requirement_dispositions: [].

-- Anthropic Claude: claude-sonnet-5

[wwind123]

Review verdict: Blocking

The plan correctly addresses the ambiguity for planning prompts, but needs to extend the same separation and empty-requirements guidance to PR-level prompts.

Same-plan follow-ups

• Apply the distinct issue-criteria separation and explicit empty-requirements guidance (forbidding the marker) to build_review_prompt (PR review) and build_issue_prompt (PR implementation). PR reviewers currently see 'optional <!-- HUMAN_REQUIREMENTS_RESOLVED -->' in their schema block and can hallucinate the marker for ordinary acceptance criteria, just like planning agents.

-- Google Antigravity: Gemini 3.1 Pro (High)

[wwind123]

Revised plan

Revise every planning, implementation, and PR-review prompt to separate issue acceptance criteria from an always-explicit signed-human-requirements context, and make generic review-schema repair receive and obey the authoritative surfaced-ID set.

Prior plan item dispositions

• [item-1] Blocking issue from Anthropic Claude: claude-sonnet-5, round 1: The plan's repair scope (step 3) targets 'repair.py planning repair instructions' but does not na... -> resolved: The revised plan adds the missing generic repair-path wiring in _run_validated_agent for both plan and PR reviews, plus a reviewer-repair instruction for fabricated/empty dispositions rather than relying on the narrower missing-marker flow.
• [item-2] Blocking issue from Anthropic Claude: claude-sonnet-5, round 1: Because of the above, the step 4 regression coverage as scoped (tests/test_prompts.py and tests/t... -> resolved: The revised plan adds an orchestrator-level malformed plan-review regression that exercises validation, repair arguments/prompt context, and final revalidation with an empty dispositions array.
• [item-3] Same-plan follow-up from Google Antigravity: Gemini 3.1 Pro (High), round 1: Apply the distinct issue-criteria separation and explicit empty-requirements guidance (forbidding... -> resolved: The revised prompt scope now includes full and compact build_review_prompt PR-review paths and build_issue_prompt implementation paths, alongside planning prompts.

Plan steps

1. Refactor the signed-human-requirements rendering and guidance helpers in src/coding_review_agent_loop/prompts.py so every relevant caller emits a dedicated Signed human requirements section, including an explicit empty-state section. For an actually empty surfaced set, state that ordinary issue acceptance criteria, issue prose, reviewer items, and comments are not signed-requirement IDs; require human_requirement_dispositions: [] where that schema has the field; and forbid HUMAN_REQUIREMENTS_ADDRESSED/HUMAN_REQUIREMENTS_RESOLVED markers. Preserve the existing bounded-context behavior: enumerate only surfaced Requirement N IDs, retain direct-discussion acknowledgement requirements when all signed entries were omitted for size, and never treat an omitted or malformed response as evidence of a requirement.
2. Apply that shared distinction consistently to build_issue_prompt and the issue implementation/approved-plan handoff prompts, plus initial plan, plan revision, plan decomposition, full plan review, and compact plan review construction. Render the ordinary GitHub issue body, acceptance criteria, and unsigned discussion in a separate labeled issue-context section that explicitly says it guides planning/implementation/review but cannot supply protocol IDs. Keep response-schema examples conditional or explicitly empty so an empty signed-requirements context cannot be contradicted by a Requirement 1 example.
3. Extend the same explicit empty/surfaced requirements and criteria-separation contract to build_review_prompt and _build_compact_pr_review_prompt. Ensure PR reviewers are told that the resolved marker is required only for the authoritative non-empty signed set, prohibited when none were surfaced, and that issue acceptance criteria remain normal PR-review context rather than acknowledgement IDs.
4. Fix the generic structured schema-repair path in src/coding_review_agent_loop/orchestrator.py: when _run_validated_agent repairs plan_review or pr_review, pass the authoritative reviewer requirement IDs (including an explicit empty tuple) into repair_kwargs, using the same scoped rendering source used by each caller. Update the plan-review and PR-review invocation sites to supply those IDs, while preserving the existing narrower post-approval missing-marker repair behavior.
5. In src/coding_review_agent_loop/repair.py, make reviewer repair accept that generic context for plan_review and pr_review and add a distinct schema-failure instruction, separate from the current missing-HUMAN_REQUIREMENTS_RESOLVED instruction. It must identify the surfaced IDs as authoritative; for no IDs, remove fabricated human_requirement_dispositions, emit [] for plan reviews, and remove/omit the resolved marker for both review kinds; for a mixed/non-empty set, retain only exact surfaced IDs and require valid plan-review dispositions. Update the repair-kind argument validation and static format guidance/examples so their Requirement 1 examples cannot override an explicit empty reviewer context.
6. Add regression coverage in tests/test_prompts.py for issue acceptance criteria with zero signed requirements across full and compact plan review, full and compact PR review, and issue implementation prompts, asserting the separate sections and empty-array/no-marker rules; retain non-empty and truncation/direct-discussion cases. Add focused tests/test_repair.py cases for generic plan/PR reviewer repair prompt construction with empty and mixed surfaced-ID sets, including removal of fabricated IDs and marker guidance. Add an orchestrator-level tests/test_agent_loop.py regression in the existing _run_validated_agent/FakeRunner style: submit a malformed plan_review with a fabricated disposition and no signed requirements, assert the real repair call gets reviewer_requirement_ids=(), assert its prompt receives the authoritative empty context, and verify the repaired, fully validated response has human_requirement_dispositions: [] with no resolved marker. Add the analogous PR-review argument-path assertion where practical, then run the targeted prompt/repair/orchestrator tests followed by python -m pytest.

-- OpenAI Codex: gpt-5.6-terra

[wwind123]

Review verdict: Approved

The revised plan adds the missing piece: step 4 wires authoritative reviewer requirement IDs (including the empty case) into the generic schema-validation repair path in orchestrator.py, and step 5 gives repair.py a distinct instruction for stripping fabricated/invalid human_requirement_dispositions rather than reusing the narrower missing-marker wording. I traced the current code (validate_human_requirement_dispositions in protocol.py:947, the generic repair_kwargs builder in orchestrator.py:2121-2151, and _reviewer_human_requirements_instruction in repair.py:1357) and confirmed this is exactly the gap that let a fabricated disposition survive a repair pass in the original incident. Step 6's orchestrator-level regression test closes the coverage gap I flagged. All three prior round-1 items are addressed.

Prior unresolved plan item dispositions

• [item-1] Blocking issue from Anthropic Claude: claude-sonnet-5, round 1: The plan's repair scope (step 3) targets 'repair.py planning repair instructions' but does not na... -> resolved: Step 4 explicitly commits to wiring reviewer requirement IDs (empty-tuple included) into the generic repair_kwargs builder in orchestrator.py for plan_review/pr_review schema-validation failures, and step 5 adds a distinct repair.py instruction (separate from the missing-marker wording) for stripping fabricated human_requirement_dispositions -- covering the exact mechanism that let the original fabricated 'Requirement: tooltip works...' entry survive repair.
• [item-2] Blocking issue from Anthropic Claude: claude-sonnet-5, round 1: Because of the above, the step 4 regression coverage as scoped (tests/test_prompts.py and tests/t... -> resolved: Step 6 now adds an orchestrator-level tests/test_agent_loop.py regression using the existing _run_validated_agent/FakeRunner pattern that exercises the real repair call path and asserts reviewer_requirement_ids=() reaches the repair prompt and the final output has human_requirement_dispositions: [], closing the gap where prompt/repair-only tests wouldn't catch the orchestrator wiring defect.
• [item-3] Same-plan follow-up from Google Antigravity: Gemini 3.1 Pro (High), round 1: Apply the distinct issue-criteria separation and explicit empty-requirements guidance (forbidding... -> resolved: Step 3 extends the same issue-criteria separation and empty-requirements/prohibited-marker guidance to build_review_prompt and _build_compact_pr_review_prompt, and step 6 adds corresponding tests/test_prompts.py coverage for full and compact PR review prompts.

-- Anthropic Claude: claude-sonnet-5

[wwind123]

Review verdict: Approved

The plan thoroughly addresses all prior items by refactoring the prompt generation to clearly separate acceptance criteria from signed human requirements, wiring the requirement IDs into the generic repair path in the orchestrator, and providing comprehensive orchestrator-level test coverage.

Prior unresolved plan item dispositions

• [item-1] Blocking issue from Anthropic Claude: claude-sonnet-5, round 1: The plan's repair scope (step 3) targets 'repair.py planning repair instructions' but does not na... -> resolved: Addressed in steps 4 and 5 by wiring reviewer_requirement_ids into orchestrator.py and adding a distinct repair instruction in repair.py to handle empty/fabricated requirement IDs.
• [item-2] Blocking issue from Anthropic Claude: claude-sonnet-5, round 1: Because of the above, the step 4 regression coverage as scoped (tests/test_prompts.py and tests/t... -> resolved: Addressed in step 6 by adding the specific orchestrator-level regression test in tests/test_agent_loop.py.
• [item-3] Same-plan follow-up from Google Antigravity: Gemini 3.1 Pro (High), round 1: Apply the distinct issue-criteria separation and explicit empty-requirements guidance (forbidding... -> resolved: Addressed in steps 2 and 3 by extending the explicit empty/surfaced requirement logic to both build_issue_prompt and build_review_prompt.

-- Google Antigravity: Gemini 3.1 Pro (High)

[wwind123]

Planning complete for issue #561.

Outcome: implement

Approved plan:

Revise every planning, implementation, and PR-review prompt to separate issue acceptance criteria from an always-explicit signed-human-requirements context, and make generic review-schema repair receive and obey the authoritative surfaced-ID set.

Prior plan item dispositions

• [item-1] Blocking issue from Anthropic Claude: claude-sonnet-5, round 1: The plan's repair scope (step 3) targets 'repair.py planning repair instructions' but does not na... -> resolved: The revised plan adds the missing generic repair-path wiring in _run_validated_agent for both plan and PR reviews, plus a reviewer-repair instruction for fabricated/empty dispositions rather than relying on the narrower missing-marker flow.
• [item-2] Blocking issue from Anthropic Claude: claude-sonnet-5, round 1: Because of the above, the step 4 regression coverage as scoped (tests/test_prompts.py and tests/t... -> resolved: The revised plan adds an orchestrator-level malformed plan-review regression that exercises validation, repair arguments/prompt context, and final revalidation with an empty dispositions array.
• [item-3] Same-plan follow-up from Google Antigravity: Gemini 3.1 Pro (High), round 1: Apply the distinct issue-criteria separation and explicit empty-requirements guidance (forbidding... -> resolved: The revised prompt scope now includes full and compact build_review_prompt PR-review paths and build_issue_prompt implementation paths, alongside planning prompts.

Plan steps

1. Refactor the signed-human-requirements rendering and guidance helpers in src/coding_review_agent_loop/prompts.py so every relevant caller emits a dedicated Signed human requirements section, including an explicit empty-state section. For an actually empty surfaced set, state that ordinary issue acceptance criteria, issue prose, reviewer items, and comments are not signed-requirement IDs; require human_requirement_dispositions: [] where that schema has the field; and forbid HUMAN_REQUIREMENTS_ADDRESSED/HUMAN_REQUIREMENTS_RESOLVED markers. Preserve the existing bounded-context behavior: enumerate only surfaced Requirement N IDs, retain direct-discussion acknowledgement requirements when all signed entries were omitted for size, and never treat an omitted or malformed response as evidence of a requirement.
2. Apply that shared distinction consistently to build_issue_prompt and the issue implementation/approved-plan handoff prompts, plus initial plan, plan revision, plan decomposition, full plan review, and compact plan review construction. Render the ordinary GitHub issue body, acceptance criteria, and unsigned discussion in a separate labeled issue-context section that explicitly says it guides planning/implementation/review but cannot supply protocol IDs. Keep response-schema examples conditional or explicitly empty so an empty signed-requirements context cannot be contradicted by a Requirement 1 example.
3. Extend the same explicit empty/surfaced requirements and criteria-separation contract to build_review_prompt and _build_compact_pr_review_prompt. Ensure PR reviewers are told that the resolved marker is required only for the authoritative non-empty signed set, prohibited when none were surfaced, and that issue acceptance criteria remain normal PR-review context rather than acknowledgement IDs.
4. Fix the generic structured schema-repair path in src/coding_review_agent_loop/orchestrator.py: when _run_validated_agent repairs plan_review or pr_review, pass the authoritative reviewer requirement IDs (including an explicit empty tuple) into repair_kwargs, using the same scoped rendering source used by each caller. Update the plan-review and PR-review invocation sites to supply those IDs, while preserving the existing narrower post-approval missing-marker repair behavior.
5. In src/coding_review_agent_loop/repair.py, make reviewer repair accept that generic context for plan_review and pr_review and add a distinct schema-failure instruction, separate from the current missing-HUMAN_REQUIREMENTS_RESOLVED instruction. It must identify the surfaced IDs as authoritative; for no IDs, remove fabricated human_requirement_dispositions, emit [] for plan reviews, and remove/omit the resolved marker for both review kinds; for a mixed/non-empty set, retain only exact surfaced IDs and require valid plan-review dispositions. Update the repair-kind argument validation and static format guidance/examples so their Requirement 1 examples cannot override an explicit empty reviewer context.
6. Add regression coverage in tests/test_prompts.py for issue acceptance criteria with zero signed requirements across full and compact plan review, full and compact PR review, and issue implementation prompts, asserting the separate sections and empty-array/no-marker rules; retain non-empty and truncation/direct-discussion cases. Add focused tests/test_repair.py cases for generic plan/PR reviewer repair prompt construction with empty and mixed surfaced-ID sets, including removal of fabricated IDs and marker guidance. Add an orchestrator-level tests/test_agent_loop.py regression in the existing _run_validated_agent/FakeRunner style: submit a malformed plan_review with a fabricated disposition and no signed requirements, assert the real repair call gets reviewer_requirement_ids=(), assert its prompt receives the authoritative empty context, and verify the repaired, fully validated response has human_requirement_dispositions: [] with no resolved marker. Add the analogous PR-review argument-path assertion where practical, then run the targeted prompt/repair/orchestrator tests followed by python -m pytest.

-- coding-review-agent-loop

[wwind123]

Possible unfiled scope narrowing

The approved plan's text appears to narrow scope (mentions a stage, follow-up issue, or out-of-scope work), but no deferred_stages were structurally declared and no discuss split proposals exist for this issue. If this plan intentionally defers work, declare it via deferred_stages in a future revision or file the follow-up issue(s) manually. This is a heuristic warning only; the orchestrator never auto-creates issues from prose.

-- coding-review-agent-loop

[wwind123]

Approved plan for issue #561 handed off to PR #565 for one-shot implementation.

Mode: implement-one-shot
Plan hash: 5413858cd75ac12e
Plan subject: 5413858cd75ac12ea080409c34ea5c8e260d72a692a9ede5ce5b3ac25105ae8d

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

-- coding-review-agent-loop