Approval Stage Plan Hygiene

Approval-Stage Plan Hygiene

Use when drafting or revising a GitHub issue plan that will go through adversarial plan review.

When to use

• A plan keeps getting MAJOR findings even after multiple rewrites
• Reviewers complain about stale review-state language, empty review artifacts, or vague test contracts
• The plan introduces a machine-checkable JSON/report contract
• The plan changes launcher/runtime behavior and reviewers want stronger proof than string replacement

Core rules

1. Keep the plan body about the implementation contract only.

• Do not embed review-process narration like:
  • "this redraft fixes..."
  • "fresh rerun required"
  • "prepared for another approval pass"
  • "latest verdicts are ..."
• Review history belongs in review artifacts or issue comments, not in the plan contract.

2. Do not cite empty review artifacts as evidence.

• If a scripts/review/results/... file is empty or unavailable, do not mention it as supporting evidence in the plan.
• Treat review artifacts as external state, not plan content.

3. Remove approval-state/process language from acceptance criteria.

• Avoid acceptance items like:
  • "plan remains draft"
  • "after rerun this can advance to plan-review"
• Acceptance criteria should describe deliverable behavior or artifact state after implementation, not workflow-state transitions.

4. If the plan defines a machine-checkable schema, pin the full contract. At minimum specify:

• canonical artifact path
• exact field names
• exact top-level JSON shape (for example provider-keyed object vs array of rows)
• allowed enum values
• nullability/omission rules
• first-run / no-history behavior
• malformed-history behavior
• missing-row / key-mismatch behavior
• which artifact is the single source of truth vs derived renderings

Important learned pattern from repeated #2332 reruns:

• reviewers will keep returning MAJOR if the plan says a schema is canonical but does not say whether provider-specific keys must be omitted or present as null on other providers
• if a plan introduces multiple non-baseline states (for example no_prior_audit, prior_snapshot_unreadable, provider_missing_from_prior_snapshot), define the full state table in one place, including the resulting compliance/status fields for each branch
• do not spread the same contract across plural wording like "JSON/scorecard outputs"; choose one canonical machine-checked artifact and describe other outputs as derived renderings

5. Tests must prove behavior, not just rendering.

• Add schema-completeness tests, not only "contains these fields" prose checks.
• Add exact branch tests for normal, first-run, malformed-history, missing-row, over-cap, under-cap, etc.
• For launcher/runtime changes, include one behavior-preservation test that exercises the actual call path or an injected runner for that call path, not only a grep/string replacement test.
• For wrapper/launcher rewrites, command-capture checks should preserve the full subprocess contract where relevant: argv tail, cwd, key kwargs, and the fact that the real function reaches the rewritten call site.
• If only one file gets a true end-to-end smoke and the others only get imports or object construction, reviewers may still return MAJOR because the rewritten launcher path itself was never exercised.

Approval checklist

Before rerunning adversarial review, verify:

• No Review artifacts: metadata line if it would point to empty artifacts
• No Adversarial Review Summary section containing stale process text
• No acceptance criteria about re-review or status transitions
• If JSON/report contract exists, one canonical machine artifact is explicitly named
• First-run semantics are fully specified
• Behavior-preservation test exists for any runtime/launcher swap
• Resource-intelligence wording is implementation-focused, not reviewer-facing narration (avoid phrases like approval-stage planning should, this redraft fixes, prepared for rerun, none blocking approval readiness)
• State tables do not contradict cap tables or branch rules; if non-baseline branches exist, baseline/cap/compliance fields are defined once and reused consistently everywhere

Additional learned guardrails from repeated #2332 review loops

6. Keep resource-intelligence sections factual and repo-grounded, not meta-commentary.

• Even when the facts are correct, reviewers may keep returning MAJOR if the plan reads like a review diary instead of an implementation contract.
• Prefer neutral statements like:
  • X is the canonical generated artifact
  • Y is the first-wave target
  • Z is excluded from actionable debt
• Avoid process-oriented phrasing like:
  • approval-stage planning should...
  • this redraft fixes...
  • none blocking approval readiness

7. For multi-branch schema contracts, define the state table in one place and make every other section conform to it.

• If you have branches such as:
  • baseline_available
  • no_prior_audit
  • prior_snapshot_unreadable
  • provider_missing_from_prior_snapshot then specify in one canonical block, for each branch:
  • baseline field value
  • delta field value
  • cap field value
  • compliance status
  • compliance reason
  • markdown/rendering expectation
• Then mirror that exact contract in tests. Reviewers will keep flagging MAJOR if one section says caps are fixed while another says they are reseeded/null.

8. If a repeated one-provider MAJOR persists after many tight reruns, stop the loop and escalate explicitly.

• After several focused reruns where one reviewer continues to object to increasingly narrow contract details, the next best move may be:
  • make the final policy choice explicitly
  • surface the issue link to the user
  • request human judgment instead of burning more cycles
• This is especially true when another strong reviewer consistently returns APPROVE and the remaining blocker has shrunk to a policy interpretation rather than broad plan unsoundness.

Common MAJOR triggers this prevents

• "Plan still cites empty review artifacts"
• "Plan includes stale review-process narration"
• "Machine-checkable schema is underspecified"
• "First-run/no-history behavior is ambiguous"
• "Tests only prove string replacement, not real behavior"

Outcome to aim for

A plan that reviewers can assess purely on current implementation intent, with no confusion from stale process metadata and no ambiguity in schema or runtime-contract behavior.