Implement incremental corrective commits with a final whole-PR review gate

[BaseInfinity]

Goal

Evaluate an incremental review workflow where completed slices are tested, self-reviewed, independently reviewed, and committed so the remaining uncommitted review scope becomes progressively smaller.

Required invariant

The final release/merge gate must review the whole branch diff against its merge base, plus any remaining uncommitted work. Reviewing only uncommitted changes is insufficient because defects can exist in committed slices or in interactions between slices.

Research questions

• Which native Codex review scopes cover staged, uncommitted, individual commit, and whole-branch changes?
• How should test/review proof be bound and invalidated after each fix or commit?
• Should Fable High review each slice, or only the final whole-branch candidate after self-review and Sol High pass?
• Does committing clean slices materially reduce review latency without hiding cross-slice risk?
• What exact commands and evidence make the workflow understandable and reproducible?

Candidate workflow

1. Implement one bounded slice with TDD.
2. Run affected tests and required proof.
3. Perform exact-diff author self-review.
4. Run independent Sol High review.
5. Fix findings and repeat until clean.
6. Commit that clean slice.
7. Repeat for later slices.
8. Before merge/release, run full tests and review the complete merge-base-to-HEAD branch diff plus any remaining changes.
9. Run Fable High only after the final self-review and Sol High gates are clean.

Acceptance criteria

• Benchmark current monolithic review against incremental slice review.
• Document exact native commands and their review scope.
• Never commit failing tests and never merge failing CI.
• Preserve the final whole-branch Sol High and Fable High gates.
• Explicitly handle fixes that invalidate earlier proof.
• Prototype enforcement only if the measured workflow is simpler or faster without reducing coverage.

Boundary

Research/backlog item; not a blocker for the current 0.7.36 release.

[BaseInfinity]

Dogfooding clarification from m180-jumpseat:

The desired workflow is not "commit the known-bad baseline after the first review." It is a review checkpoint over the current working tree.

After an initial full codex review --uncommitted, Codex should be able to:

• retain an immutable identifier/hash for the reviewed HEAD + index + worktree state;
• retain the findings and their resolved/unresolved state;
• run a follow-up review over only the corrective edits since that checkpoint, while still checking every unresolved finding;
• invalidate or widen the checkpoint if unrelated edits, a rebase, or other baseline-changing work occurs;
• avoid requiring a commit before the slice is green;
• still run the final whole-branch/merge-base review before release.

A possible native shape would be codex review --continue <review-id> or --since-review <review-id>, with output separating previously reviewed content, new corrective edits, and unresolved findings.

Today, rerunning review --uncommitted rereads the entire HEAD-to-worktree diff. Committing only clean slices reduces that scope, but there is no equivalent incremental loop for fixing findings inside one not-yet-clean slice.

[BaseInfinity]

Prioritized for 0.7.37 based on the #86 release-candidate loop.

Measured failure mode on 2026-08-08:

• one 28-file / ~3,400-line uncommitted patch
• each independent finding invalidated the whole frozen candidate
• the full proof suite took roughly 16 minutes per rerun
• whole-patch Sol High review took roughly 3–5 minutes per rerun
• real findings were valuable, but repeatedly re-certifying unrelated clean work dominated cycle time

Required implementation shape:

1. Keep each logical slice small: TDD RED/GREEN, affected tests, exact-diff author review, independent Sol High, then commit the clean slice.
2. Treat a reviewer correction as its own bounded corrective commit. Bind it to the finding it resolves and verify the affected behavior before committing.
3. Do not rerun Fable for every slice. Fable High remains the final cross-model gate.
4. At PR/release completion, run the full suite on the assembled branch, review the complete merge-base-to-HEAD diff plus any remaining staged work, and verify that every earlier finding has a corresponding corrective commit/test.
5. If the final whole-PR review finds a defect, add a small corrective commit, rerun affected proof plus the required release suite, rerun whole-PR Sol High, then Fable High.
6. Never merge failing tests or CI; incremental certification reduces repeated scope but cannot replace final integration certification.

The final gate should produce a concise ledger mapping: finding -> corrective commit -> regression test/proof -> final whole-PR certification.

[BaseInfinity]

Adopted workflow clarification: chained incremental certification

The goal is not to rereview the whole branch after every correction.

Local branch loop

1. Implement one bounded change while it is uncommitted.
2. Run the affected tests and exact-diff author review.
3. Sol High and Fable High review that uncommitted delta through the local gate.
4. Address findings while the delta remains uncommitted; reviewers inspect only the correction/new delta required by the gate.
5. Commit once that delta is clean. The commit becomes an immutable reviewed checkpoint.
6. Repeat for the next uncommitted slice. A PR is optional until collaboration or merge readiness requires one.

Initial pull-request gate

1. Push the accumulated reviewed commits to one branch and open/update one PR.
2. Run integration tests/CI.
3. Sol High and Fable High each review the whole PR once (merge base -> current PR head).
4. Record that PR head as the certified baseline.

PR correction loop

1. Apply requested fixes as new uncommitted deltas on the same branch.
2. Use the normal local delta review/test/commit loop.
3. Push those commits to the same PR.
4. Review only previously certified PR head -> new PR head; do not reread the unchanged whole PR.
5. Advance the certified PR head only when both reviewers and required tests/CI are clean.
6. Repeat until no findings remain.

Final merge invariant

Before merge, verify mechanically that:

• the current PR head exactly equals the latest dual-certified head;
• every commit/delta since the initial whole-PR review is covered by a later incremental Sol + Fable certification;
• required tests and CI are green;
• no uncommitted or unreviewed changes remain.

This produces cumulative assurance: one whole-PR review establishes the baseline; dual-reviewed incremental deltas extend it. A new whole-PR review is required only when the certification chain is missing/broken, history is rewritten, the merge base changes materially, or an integration-risk rule explicitly requires it.

CI/CD may run tests and enforce the certification ledger, but it is not itself a code reviewer unless an actual configured review agent produces a bound review result.

[BaseInfinity]

Fresh dogfood evidence from the #98/#91 Windows repair on 2026-08-09:

• The exact staged one-file patch passed the focused npm suite 25/25 and the full 11/11 proof suite.
• Exact-diff author review, Sol High, and Fable High were clean.
• Because the full suite was invoked directly, the commit guard had no supported way to consume that completed exact-tree result. It required the same full suite again through git-guard.cjs prove --reviewed solely to write its stamp.
• On Enforce High effort for planning and review unless the user explicitly overrides implementation #86, a freshly written worktree-scoped proof was still rejected by the parent session because the PreToolUse hook resolved the main worktree; a fresh Codex session rooted in the linked worktree committed safely.

Required KISS outcome for this issue: one canonical proof invocation per frozen tree, proof/review receipts bound to the exact tree hash rather than expiring while the tree is unchanged, and correct command-working-directory/worktree resolution. Do not make agents rerun an unchanged 8–18 minute suite merely to translate already-current evidence into another local stamp.

[BaseInfinity]

Audit evidence from the 2026-08-09 overnight run: between 01:00 and 15:00 PDT the repo produced five local commits, closed two tracking issues, merged zero PRs, and shipped zero releases. The current release slice completed repeated full-proof runs plus reviewer-triggered duplicate suites while the uncommitted parser patch kept growing. The gates found real P1 defects, but delivery cadence entered diminishing returns.

Add these requirements to the prototype:

• Full proof once per frozen candidate; reviewers consume tree-bound proof and may run only targeted probes tied to a concrete suspected defect.
• Commit each bounded green/reviewed slice before starting the next so uncommitted scope shrinks.
• After one corrective review round, triage new findings: P0/P1 in the changed contract block the slice; unrelated or speculative expansion becomes a linked follow-up.
• Final merge gate reviews the complete merge-base-to-HEAD diff, then Fable High runs only after author self-review and Sol High are clean.
• Record elapsed implementation, proof, Sol review, correction, Fable review, CI, and release time.
• Define an explicit overnight stop condition: never loop indefinitely, never bypass gates, and stop before human-only release/Windows/auth/history operations.

[BaseInfinity]

Proposed cadence after an m180 negative pilot

The m180 v0.3 candidate is a clean counterexample to “review more often” as a complete answer. Commit dff6867 combined three issues into 37 files (+4,692/-253). The full 1,659-test Pester suite stayed green, but two completed review rounds found eight legitimate defects and further review attempts timed out or returned no usable result. The pushed branch closed no issue.

I think the efficient form of this issue is:

1. Each bounded slice: red/green, affected proof, self-review exact diff, commit.
2. Risk-based slice review: one independent reviewer for high-risk code/proof changes; ordinary low-risk commits do not need both models.
3. Final delivery candidate: freeze the SHA; run the broad suite once; Sol and Fable review independently; reconcile once in a compact ledger.
4. Corrections: commit bounded deltas and run targeted proof. Ask the reviewer responsible for the finding to recheck the delta. Do not rerun both models and the full suite unless the tree/config/runner surface that the receipt covers changed.
5. Termination: two corrective rounds per frozen scope. A third same-plan round means split/redesign or explicit human approval.
6. Invariant: default branch may advance only when its head equals the latest dual-cleared frozen candidate and its tree-bound proof is current.

That preserves the real-life two-reviewer merge gate the maintainer wants without turning each development commit into a miniature merge ceremony. It also makes #73's tree-bound proof receipt load-bearing rather than an optimization.

Suggested pilot metrics for 10 issue-sized changes: planned vs actual diff, tripwire count, unique findings per reviewer, correction rounds, broad-suite runs, review wall time, issues closed, PRs merged, and milestones closed. “Green candidate pushed” should be recorded separately from “delivered.”

[BaseInfinity]

VisualHUD comparison: incremental checkpoints delivered; an uncommitted final slice stalled\n\nProduct-repo dogfood from Sunday 2026-08-09 gives a useful A/B result for this issue:\n\n- Nine incremental commits closed five v1.2 issues; each bounded issue reached green CI and main ended synchronized.\n- The remaining uncommitted issue #15 grew to 684 new-file lines plus 70 tracked additions / 174 deletions, including a 333-line custom auditor and 279-line test.\n- Repeated whole-slice review found valid new defects but kept recertifying the broad candidate and expanding its mechanisms.\n- The unattended goal reached about 12.9 hours / 3.9 million tokens with the milestone still 5/8 and no release.\n- Two commits were pushed directly to main with red CI before follow-up repairs.\n\nInterpretation: immutable reviewed increments materially improved delivery and attribution, but the incremental sensor needs #110's breaker. The measured default should be:\n\n1. bounded TDD + affected proof + exact-delta review per increment;\n2. one tree-bound full proof and whole-issue Sol/Fable gate at completion;\n3. one compact reconciliation only on blocker/disagreement;\n4. bounded corrective delta review, not whole-issue rereview;\n5. stop/replan/split after two correction rounds, >2x estimated scope, or a new parser/enforcement mechanism outside the scope card;\n6. high-risk work uses an issue branch and required latest-SHA CI before main.\n\nTrack closed issues and milestone boundaries, not commit count alone. Also record proof duplication, review rounds, blocker locus, wall time, and tokens per completed issue.\n\nFull source receipt: claude-sdlc-harness#534.