feat(gate-findings): frame findings as hypotheses at evaluator-fed fix stages - #185
Conversation
…ages (#167) Task 1 of #167: behaviour-neutral primitives that let a fixer treat a reviewer's finding as a hypothesis to verify rather than a command to obey. - FIX_SCHEMA.rebutted ({finding_id, evidence}), not in required — mirrors REVIEW_SCHEMA.issues staying out of its own required list. - FINDING_HYPOTHESIS_ASK beside HANDOFF_ASK/COMMIT_SHA_ASK: verify before acting, record disproof in `rebutted` if wrong, rebut only what you didn't fix, only bracketed-id findings are rebuttable, and a gate-agnostic consequence clause (no immediate-exit claim, since pr-review continues into another review round). - normalizeRebuttals(raw, findings): pure, fails toward today's behavior on every drop (non-array, blank finding_id/evidence, unrendered id); carries the matched finding's summary through. - contestedBlock(ctx): pure, renders rebutted-but-not-adjudicated findings back to the next reviewer with an inverted (vs settledBlock) closing contract, and states the iteration-2+ do-not-re-flag instruction doesn't apply to a contested entry. Never calls settleDecision(). Rendered immediately after settledBlock() at the quality/spec/code review prompts; emits '' until a later task populates ctx.contested. - retypeGateDisposition(ctx, gate, from, to): pure, moves one disposition count between buckets without touching count/severity; from === to is a documented count-preserving no-op, reachable at MAX_QUALITY_ITERATIONS. - ctx.metrics.rebuttal_only_rounds, plus tests/harness.js freshMetrics() parity. None of these are wired into the fix stages' control flow yet (task 2/3); FINDING_HYPOTHESIS_ASK and retypeGateDisposition are unused in production until then. 45 new unit tests in tests/gate-findings.test.js. node scripts/lint-engine.js --fix kept .claude/workflows/ticketmill.js in lockstep. node --test: 763/763 passing.
…st loops (#167) Renders FINDING_HYPOTHESIS_ASK in the quality-fix and test-quality-fix prompts only when the reviewer/validator named structured findings, and wires normalizeRebuttals()/retypeGateDisposition()/contestedBlock() (from task 1) into both evaluator-fed fix loops so a fix that rebuts every finding and applies none can never approve its own gate: - runQualityLoop: a third loop-exit flag `rebutted`, kept separate from `degraded` so the rolling degrade window is untouched, retypes the disposition already booked for that iteration to 'carried-unresolved', records the rebuttal as contested (not settled), and rolls up to exactly one VERIFY_SKIPS line per issue (ctx.quality_rebuttals / quality_rebuttal_skip_index), mirroring the existing cap roll-up. - runTestLoop: same predicate at test-quality-fix, exits through the loop's own { ok: true } path (never ok:false) so a rebuttal-only round can never route through the 'test-loop' merge-block stage key. No gate key to retype here (test-quality books none today). The two ORACLE-fed fix stages (test-fix, browser-fix) are untouched and keep their anti-rebuttal guards — a failing test is ground truth, not a hypothesis to verify. Extends tests/quality-loop.test.js and tests/test-loop.test.js with the rebuttal-only exit, partial-rebuttal/omitted-field/blank-evidence non-triggers, the rolled-up VERIFY_SKIPS line, and scope-pinning assertions proving FINDING_HYPOTHESIS_ASK renders only at the three evaluator-fed sites and never at test-fix.
…ntinue Renders FINDING_HYPOTHESIS_ASK in the pr-fix prompt when either reviewer returned structured findings, and evaluates the rebuttalOnly predicate over the union of both reviewers' rendered finding sets. Unlike the quality and test loops (which exit their gate on a rebuttal-only round), pr-review CONTINUES: it retypes the iteration's disposition to carried-unresolved, carries the dispute into ctx.contested so the next iteration's reviewers see it, records one un-rolled Verification Gaps line, and moves on to the next review iteration instead of ending the gate. A per-call counter permits exactly one such round per issue; a second sets haltReason and falls through to the existing needs_human path, leaving the PR open. The `continue` precedes runQualityLoop so a rebuttal-only round (empty files_changed) never burns a quality gate against an untouched tree. Issue #167
…ix stages (#167) Add a "Rebuttal: a finding is a hypothesis, not a command" section to gate-hygiene.md covering what a rebuttal is and its evidence bar, why the framing is scoped to quality-fix/test-quality-fix/pr-fix and not test-fix/browser-fix, FIX_SCHEMA.rebutted being schema-wide but read at only three sites, normalizeRebuttals' fail-toward-today drops, the three per-gate exits and why only pr-review can block a merge, contestedBlock vs settledBlock's inverted contract, and rebuttal_only_rounds (a continuation, not an exit, on its first pr-review increment). Extend the gate-hygiene.md row in index.md's file map, and correct three now-stale passages in engine-internals.md that #167 falsifies: the carried-unresolved disposition enumeration (missing the rebuttal-only route), the "only findings_empty_exits distinguishes them" claim at pr-review, and findingsBlock's description of a rendered finding's job list.
Spec Review (Iteration 1)Verdict: approved PR #185 implements issue #167 in full. Verified against Acceptance criteria, checked against the diff:
No scope creep found. No changes requested. |
Code Review (Iteration 1)Verdict: changes_requested (1 major, 4 minor) Baseline is clean: Major1. The engine grew 17,355 bytes and crossed the 92% Workflow-cap warning line that the immediately preceding commit added to prevent a repeat incident. The engine still launches, so this is not blocking, but it regresses the guard from one commit ago and the fix is mechanical with zero behaviour change. Minor2. A second rebuttal-only pr-fix round is the only terminal exit from 3. 4. 5. Nothing ever closes a Observed, deliberately not filedA partial rebuttal (rebut some findings, fix others) discards |
…ally gap - Move the #167 rebuttal-framing comment blocks (contestedBlock, FINDING_HYPOTHESIS_ASK, retypeGateDisposition, normalizeRebuttals, and the three in-loop rebuttalOnly explainers) out of workflows/ticketmill.js into one-sentence pointers at docs/architecture/gate-hygiene.md, where the same rationale already lives — clears lint-engine's 92% size warning. - reviewAndMerge()'s second (halting) rebuttal-only pr-fix round now calls retypeGateDisposition before breaking, so gate_findings['pr-review'] never leaves a leftover 're-litigated' count on the run a human is sent to inspect. - Reword gate-hygiene.md's normalizeRebuttals claim: it enforces only non-blankness, not the prompt's concrete-evidence bar. - Make contestedBlock's do-not-re-flag override site-agnostic so it also covers spec review's "stay consistent with prior reviews" instruction. - Correct the contestedBlock comment and gate-hygiene.md to state plainly that nothing closes a ctx.contested entry today — a known gap, not the "only a reviewer closes it" claim that didn't match the code. Fixes code review findings code-i1-1 through code-i1-5 on PR #185.
PR Review Fix (iteration 1)Commit: eaa7458
node --test: 781/781 passing. node scripts/lint-engine.js: clean, no size warning. |
Spec Review (Iteration 2)Verdict: approved Re-verified PR #185 against issue #167 at HEAD My iteration-1 approval already confirmed the PR meets every acceptance criterion in #167. This pass checks whether the intervening "PR Review Fix (iteration 1)" commit ( Diff-audited
Grepped the full |
Code Review (Iteration 2)Verdict: changes_requested (1 minor) Re-reviewed at Iteration-1 findings, re-verified as fixed
No regressions from the fix commit: the trim is comments-only apart from the one Minor1. Fix direction: correct the doc, not the code — one sentence in that section saying the halting second round is deliberately not counted (the counter tracks continuations, and the halt is already carried by Observed, deliberately not filed
|
[code-i2-1] The halting second pr-fix rebuttal-only round retypes the disposition and breaks without incrementing the counter (by design, pinned at tests/pr-review-gate.test.js:568-570), but the prose said the counter "bounds an issue to at most two rebuttal-only pr-fix rounds" without noting the halt itself is never counted. Clarify that the counter tracks continuations, not rounds, and that the halt is already carried by haltReason and the needs_human status.
PR Review Fix (iteration 2)Commit: af03409
|
Spec Review (Iteration 3)Verdict: approved Re-verified PR #185 against issue #167 at HEAD Iterations 1 and 2 already confirmed the PR meets every acceptance criterion in #167; this pass checks whether All acceptance criteria from #167 still hold, unaffected by this doc-only round:
No scope creep. No changes requested. |
Code Review (Iteration 3)Verdict: approved Re-reviewed at Baseline clean: Iteration-2 finding, re-verified as fixedcode-i2-1 ( Independently re-verified this pass (not just trusted from prior iterations)
Observed, deliberately not filed
Nothing must be fixed. Approved for merge. |
Implementation CompleteBranch
Deferred Suggestions for Follow-up
|
Closes #167
Summary
Applies the verify-then-adopt-or-rebut framing already used at the two contrarian revision stages to the three evaluator-fed fix stages (quality-fix, test-quality-fix, pr-fix), while leaving the two oracle-fed stages (test-fix, browser-fix) untouched since they already carry a correct anti-rebuttal guard that this framing would invert.
Implementation
rebutted: [{finding_id, evidence}]toFIX_SCHEMA(not inrequired), mirroring howissuesstays out ofREVIEW_SCHEMA.required.FINDING_HYPOTHESIS_ASKconstant besideHANDOFF_ASK/COMMIT_SHA_ASK, rendered only when structured findings are present, and wired into exactly the three evaluator-fed fix prompts (quality-fix, test-quality-fix, pr-fix).normalizeRebuttals,contestedBlock, andretypeGateDisposition, all fail-toward-existing-behavior on malformed input.rebuttalOnlypredicate (rebutted findings present,fixes_appliedandfiles_changedboth empty) evaluated uniformly at all three gates, after existing handoff/commit-SHA bookkeeping so those checks still fire.degraded) rather than falling through to the iteration cap path, and incrementctx.metrics.rebuttal_only_rounds.needs_humanwith acarried-unresolveddisposition and a Verification Gaps line on a second consecutive rebuttal-only round..claude/workflows/copy in sync vianode scripts/lint-engine.js --fix.docs/architecture/gate-hygiene.md.Key decisions
rebuttalOnlyis evaluated aftercollectNotes/collectPostedCommitat each site so handoff notes and the fabricated-SHA check still run first.tallyTouchesis a verified no-op on an empty rebuttal array, so metrics accounting stays correct when nothing was rebutted.Testing
tests/gate-findings.test.jscovering the new primitives.node --test.Token usage (approximate, this issue only): 516192 output tokens