fix(quality-gate): tally the quality gate and bound its cap line (#163) by aaddrick · Pull Request #172 · aaddrick/ticketmill

aaddrick · 2026-07-28T06:04:48Z

Closes #163

Summary

The quality loop (runQualityLoop in workflows/ticketmill.js) was the only capped gate that never called recordGateOutcome and never reported gate_findings. It could return 'degraded' on cap exhaustion (MAX_QUALITY_ITERATIONS = 5) with both callers silently ignoring that return value: no cap note, no Verification Gaps line, no gate_findings row, and no metric that isolated cap-exhaustion from agent-death. This PR ties the quality gate into the same observability machinery every other gate already uses.

Added five recordGateOutcome(ctx, 'quality', ...) calls inside runQualityLoop, one per iteration, matching the branch map: simplify-agent death and review-agent death both record [] / 'dismissed'; an approved review records the findings / 'accepted'; the empty-findings exit records 'carried-unresolved'; a changes_requested verdict records 're-litigated' on non-final iterations and 'carried-unresolved' on the cap iteration (mirroring the pr-review disposition ternary). This gives the exact invariant sum(gate_findings.quality.disposition) === ctx.metrics.quality_iters per issue.
Rolled cap exhaustion up to exactly one VERIFY_SKIPS entry per issue via a lazily-initialized ctx.quality_caps array and a remembered ctx.quality_cap_skip_index, rewriting the same line in place across multiple quality-loop invocations (per task, per PR-fix iteration) rather than appending duplicates.
Deliberately did not write to ctx.unresolved from the quality loop — that field feeds frictionFields's contrarian_capped/unresolved_count, which must stay 0/false on runs where no CONTRARIAN gate capped. The new quality key in gate_findings carries this instead.
Clarified the quality_degrades metric (in its FRICTION_WEIGHTS comment and doc-clarified elsewhere) to state plainly that it counts agent death inside the loop, not quality regressing or cap exhaustion.
Added a conditional footnote in computeGateYield, guarded on byGate.quality, explaining that quality's denominator is naturally larger than pr-review's (one call per task plus one per PR-fix iteration, not one per PR).
Extended docs/architecture/gate-hygiene.md with a "## The quality gate" section covering the five-branch disposition map, the invariant, why the empty-findings exit tallies carried-unresolved but still returns approved, what quality_degrades does and doesn't count, and the one-line-per-issue VERIFY_SKIPS cap roll-up. Updated the provenance/stale-prose bookkeeping in AGENTS.md/CLAUDE.md/index.md to match, including noting that docs/architecture/metrics.md:114 goes stale on merge but can't be corrected in place (the provenance test hashes that file as a single tracked segment).

Key decisions

Disposition placement matches pr-review's call site exactly — after the review verdict, before the fix stage — so the same reasoning about "what counts as a disposition" applies to both gates.
STOP-aborted iterations don't count: STOP.tripped returns before quality_iters++, so the invariant holds unconditionally, not just on the happy path.
Cap-exhaustion wording avoids any "merged without a clean review" claim — that path returns 'halted', and both callers turn 'halted' into fail(), so the VERIFY_SKIPS line only asserts what's true: the loop hit its iteration cap.
Gate Yield's has_signal gets more sensitive as a side effect: quality reviews name findings on most tasks, so the Gate Yield section will now render on essentially every run where it previously could legitimately print "No gate findings recorded this run." Noted for anyone diffing run-over-run gate-yield trends across this release boundary.
Deferred follow-up: the index-rewrite roll-up pattern (ctx.quality_cap_skip_index) now exists at two call sites of the same shape; a future issue should extract a shared helper instead of a third copy under a different field name.

Token usage (approximate, this issue only): 283847 output tokens

runQualityLoop was the only capped gate that never called recordGateOutcome or reported gate_findings, and its cap-exhaustion exit ('degraded') was silently swallowed by both callers. Add one recordGateOutcome(ctx, 'quality', ...) call per loop iteration (dismissed on simplify/review agent death, accepted on approval, carried-unresolved on the empty-findings clean exit, and carried-unresolved/re-litigated on changes_requested depending on whether the cap was hit), plus a single rolled-up VERIFY_SKIPS line per issue when the cap is exhausted without approval or agent death (runQualityLoop runs once per task plus once per PR-fix round, so the line rewrites in place instead of duplicating). Also add a computeGateYield footnote noting quality's larger denominator versus pr-review, and correct three stale doc comments: the recordGateOutcome header/dismissed/bound/carried-unresolved clauses now cover quality, and the FRICTION_WEIGHTS quality_degrades comment now says it counts agent death inside the loop, not cap exhaustion, pointing readers at gate_findings.quality['carried-unresolved'] instead. Refs #163

…-op guarantees Extend tests/quality-loop.test.js with one test per recordGateOutcome disposition branch from task 1's map (simplify-death and review-death -> dismissed, approved -> accepted, changes_requested-before-cap -> re-litigated), a typed mixed-severity issues array proving non-zero gate_findings.quality.severity (mirrors pr-review-gate.test.js:390), the exact invariant sum(gate_findings.quality.disposition) === ctx.metrics.quality_iters (including across a later mid-loop death), a fully capped loop asserting disposition {'re-litigated': 4, 'carried-unresolved': 1} / result 'degraded' / quality_degrades 0 / exactly one VERIFY_SKIPS entry, two capped scopes on one ctx rolling up to exactly one entry naming both, and a converging loop pushing none. Add a carried-unresolved assertion to the existing empty-findings-exit test. Extend tests/friction-fields.test.js to prove a capped quality loop leaves contrarian_capped false and unresolved_count 0 -- the quality cap writes to gate_findings, never to ctx.unresolved. Extend tests/gate-yield.test.js to prove a 'quality' key tallies into by_gate without perturbing escaped_defects (neither an early gate nor the escape gate), and that the quality-denominator footnote renders only when a quality key is present. Extend tests/gate-findings.test.js to prove a 'quality' key tallies independently of the pre-existing approach/plan buckets on the same ctx. Full profile test_command (node --check, lint-engine, bash -n, manifest JSON parse, node --test, setup-worktree.test.sh) is green: 641/641 unit tests, 32/32 worktree-script cases. Refs #163

… cap Adds a "## The quality gate" section to gate-hygiene.md covering the five-branch disposition map runQualityLoop now records through recordGateOutcome, the sum(disposition) === quality_iters invariant, why the empty-findings exit tallies carried-unresolved but returns approved (computeGateYield's accepted:dismissed ratio and the approach contrarian gate's carried-unresolved precedent), what quality_degrades does and doesn't count, and the one-line-per-issue VERIFY_SKIPS cap roll-up. Extends the page's provenance section to also name metrics.md:114 ("rolls the three gates' gate_findings tallies"), which goes stale now that a fourth gate (quality) routinely appears in the rollup, and can't be corrected in place because architecture-provenance.test.js hashes metrics.md's one tracked segment — the entire file — verbatim. Extends the "frozen passage" section in both docs/architecture/AGENTS.md and docs/architecture/CLAUDE.md with identical text (copied byte for byte per the freeze-pair convention), and updates the gate-hygiene.md file-map row in index.md, AGENTS.md, and CLAUDE.md to reflect the page's broadened scope. Issue #163 task 3.

aaddrick · 2026-07-28T06:07:12Z

Spec Review (Iteration 1)

Verdict: approved

Verified PR #172 against issue #163's three required changes and hard constraints, diffing against the correct base (origin/Batch_2026-07-27_225225, which already contains #162 — not main, which is stale by one merge).

1. recordGateOutcome(ctx, 'quality', findings, disposition) wired into runQualityLoop. All five branches present and correctly mapped: simplify-agent-death → [] / 'dismissed'; review-agent-death → [] / 'dismissed'; rev.result === 'approved' → revFindings || [] / 'accepted'; empty-findings exit → revFindings || [] / 'carried-unresolved'; changes_requested with real findings → revFindings || [] with the iter === MAX_QUALITY_ITERATIONS ? 'carried-unresolved' : 're-litigated' ternary lifted from prReviewDisposition. Each call sits immediately after its verdict and before the fix stage, matching the pr-review call site's position.

2. Cap exhaustion rolled up to one VERIFY_SKIPS line per issue. !approved && !degraded at the loop tail (after the quality_degrades increment, before the degrade-window halt) is exactly cap exhaustion — both agent-death branches break before reaching it, both clean exits set approved. The roll-up rewrites VERIFY_SKIPS[ctx.quality_cap_skip_index] in place across multiple calls to runQualityLoop for the same issue (once per task, once per PR-fix round), verified by the "two capped scopes on the same ctx... roll up to exactly one VERIFY_SKIPS entry" test. ctx.quality_caps/ctx.quality_cap_skip_index are lazily initialized, correctly avoiding a tests/harness.js makeCtx() update.

3. quality_degrades doc-clarified, not renamed, in both the FRICTION_WEIGHTS comment and docs/architecture/gate-hygiene.md's new "What quality_degrades counts, and what it doesn't" section — it counts agent death, not cap exhaustion, which now lives at gate_findings.quality.disposition['carried-unresolved'].

Hard constraints:

ctx.unresolved is never written from runQualityLoop — grep confirms the only writes remain at the pre-existing approach (:4445) and plan (:4589) gate sites. frictionFields's contrarian_capped/unresolved_count stay false/0 on a capped quality loop, per the new dedicated test in tests/friction-fields.test.js.
The new 'quality' key in gate_findings rolls into computeGateYield's generic byGate loop without perturbing escaped_defects/by_issue, confirmed by two new tests in tests/gate-yield.test.js.
The quality-denominator footnote in computeGateYield is conditional on byGate.quality and pushes an empty line first, so it doesn't glue to the markdown table — matches the plan and the approach-gate contrarian's caveat.

Scope: diff against the correct base is tightly bounded to the three planned tasks — engine (workflows/ticketmill.js + its lockstep-identical .claude/workflows/ticketmill.js copy), tests (quality-loop.test.js, gate-findings.test.js, gate-yield.test.js, friction-fields.test.js), and docs (gate-hygiene.md + its three file-map/provenance echoes in index.md/AGENTS.md/CLAUDE.md, byte-identical between the latter two). No scope creep found. node --test: 641/641 pass.

aaddrick · 2026-07-28T06:09:11Z

Code Review (Iteration 1)

Verdict: changes requested — one major documentation-accuracy defect plus two minors. The engine change itself is correct: I traced every branch of runQualityLoop and the invariant holds.

Verified clean

Baseline: node --check workflows/ticketmill.js passes; node --test is green at 641/641; diff workflows/ticketmill.js .claude/workflows/ticketmill.js is byte-identical, so the mirror stays in lockstep.
Sandbox: no Date.now(), argless new Date(), Math.random(), require(, or Node API use on any added line.
The invariant is real. ctx.metrics.quality_iters++ (workflows/ticketmill.js:3064) sits after the STOP.tripped early return, and every path out of an iteration passes exactly one recordGateOutcome first — including the fix-agent death at :3144, which is reached only after the re-litigated/carried-unresolved call at :3125. sum(disposition) === quality_iters holds unconditionally.
The cap predicate is exactly cap exhaustion. All three degraded = true branches break, both clean exits set approved, and STOP returns before the tail, so !approved && !degraded at :3169 is reachable only by exhausting MAX_QUALITY_ITERATIONS.
The roll-up index is safe. VERIFY_SKIPS is declared once at :404 and never reassigned; every other write is a push. The remembered index therefore stays valid under pool concurrency, and ctx is per-issue so a consolidation group gets one line for the group.
No ctx.unresolved write, per the issue's hard constraint, and tests/friction-fields.test.js pins it.
computeGateYield's escaped-defect pass is untouched: quality is neither ESCAPE_GATE nor in EARLY_GATES, so escaped_defects/by_issue are bit-for-bit what they were.

Findings

1. MAJOR — docs/architecture/gate-hygiene.md:317 contradicts the engine about what quality_degrades counts.

The new section says quality_degrades fires on "only the two agent-death branches above (simplify dies, review dies)". There are three degrade branches. The fix agent dying at workflows/ticketmill.js:3144 also sets degraded = true and therefore also increments quality_degrades:

if (!fix || fix.status === 'error') { degraded = true; log(... 'fix degraded'); break }

The engine's own FRICTION_WEIGHTS comment, added by this same commit at workflows/ticketmill.js:2524, gets it right: "each time the quality loop's simplify, review, or fix AGENT DIED". So the diff ships two descriptions of the same metric that disagree, and the wrong one is on the page designated as the durable source of truth. That page is the deliverable for the issue's requirement 3 ("doc-clarify quality_degrades so it's clear it counts agent death"), which makes this a miss on the requirement itself, not a stray typo: an operator reading it will read a count of 3 as three reviewer/simplifier deaths and go looking in the wrong stage.

gate-hygiene.md:245 ("Both agent-death branches break immediately") carries the same undercount, and the fix-stage death is not mentioned anywhere else on the page, so there is nothing to correct the impression.

Fix direction: name all three deaths at :317 (simplify, review, fix) and reword :245 so the fix-stage death is not excluded by the word "Both" — the five-branch disposition map is still five branches, because the fix death happens after that iteration's disposition is already recorded, and saying so explicitly is what makes the invariant section legible.

2. MINOR — workflows/ticketmill.js:5640: the escaped-defects header now over-claims.

The rendered line is Escaped defects (findings at "pr-review" that every earlier gate missed). Before this change that was literally true: approach and plan were the only earlier gates that ever appeared in gate_findings, so EARLY_GATES was the complete set of earlier gates. After this change quality is an earlier gate that records findings and is deliberately excluded from EARLY_GATES — tests/gate-yield.test.js pins that exclusion, so the computation is intentional and I am not challenging it. The prose is what drifts: an issue where quality raised four findings and pr-review raised one will now be listed under a header telling the human reviewer that every earlier gate missed it.

Fix direction: prose only, no computation change — name the gates the check actually consults, e.g. findings at "pr-review" that neither "approach" nor "plan" raised.

3. MINOR — docs/architecture/gate-hygiene.md:393 (and the same sentence duplicated at AGENTS.md:127 / CLAUDE.md:127) overstates the tracked segment.

"tests/fixtures/architecture-split.json records exactly one segment for the whole file, 329 lines starting at its first heading, which is the entire body ... so it hashes the entire file." The segment starts at ## Friction & churn (metrics.md:5) and runs 329 lines to the end of a 332-line file; metrics.md:1-4, the synthetic H1 and lede, are outside it. The load-bearing claim is still correct — metrics.md:114 sits well inside the segment, so it genuinely cannot be edited without turning tests/architecture-provenance.test.js red — but a page whose entire subject is provenance precision should not describe a 329-of-332-line segment as "the entire file".

Fix direction: "hashes the file from its first heading to the end, which includes line 114" or similar, in all three copies (AGENTS.md and CLAUDE.md must stay byte-identical).

Not flagged, for the record

Gate Yield's has_signal becoming true on essentially every run: already adjudicated at the approach gate and carried in the PR body.
The CHANGELOG entry and plugin.json bump: batch-level, owned by the Report-phase release stage, not a per-issue concern.
The empty-findings exit still pushing one unbounded VERIFY_SKIPS line per iteration (:3121): pre-existing from Make review findings the load-bearing artifact #162, and the issue's bounding constraint was scoped to cap exhaustion.
Extracting the shared index-rewrite helper for the two *_skip_index call sites: already recorded as a deferred follow-up.

…erclaims Code review for PR #172 found gate-hygiene.md describing quality_degrades as counting only two agent deaths (simplify, review) when the fix agent's death also increments it; fix all three deaths in both places that made the claim. Also corrects two provenance passages (gate-hygiene.md and its AGENTS.md/CLAUDE.md freeze pair) that described metrics.md's 329-of-332-line tracked segment as "the entire file," and reworks the escaped-defects header in computeGateYield so it names the gates the check actually consults instead of claiming every earlier gate missed the finding.

aaddrick · 2026-07-28T06:12:37Z

PR Review Fix (iteration 1)

Commit d7b2d49

MAJOR: gate-hygiene.md said quality_degrades counts only the simplify/review agent deaths; fixed both statements of that claim (:245, :317) to name the fix-agent death too, and explained why it stays outside the five-branch disposition map.
MINOR: reworded computeGateYield's escaped-defects header (workflows/ticketmill.js, mirrored into .claude/workflows/ticketmill.js) from "every earlier gate missed" to name the gates the check actually consults (neither "approach" nor "plan" raised).
MINOR: corrected gate-hygiene.md and its AGENTS.md/CLAUDE.md freeze pair, which described metrics.md's 329-of-332-line tracked segment as "the entire file" — now states it runs from the first heading to the end, excluding the four-line synthetic H1/lede.

node --test: 641/641 pass. node scripts/lint-engine.js: lockstep pairs in sync. Full test_command (check + lint + setup-worktree bash tests) green.

The "329 lines starting at its first heading, running to the end of the 332-line file" wording implied a segment that literally spans lines 5-332 on disk, which is 328 lines, not 329. The tracked segment's true length includes a trailing blank line that writeLines() strips at write time and architecture-provenance.test.js pads back in for hashing -- it never exists on disk. Say so directly: 329 tracked lines, 328 of them visible on disk (5-332), plus the one stripped trailing blank.

aaddrick · 2026-07-28T06:24:26Z

Spec Review (Iteration 2)

Verdict: approved

Re-verified PR #172 against issue #163's three required changes and hard constraints, on top of the "PR Review Fix (iteration 1)" commits (d7b2d49, bd66476), diffing against the correct base (origin/Batch_2026-07-27_225225).

All three requirements stand as approved at Iteration 1 — no regression from the fix commits:

recordGateOutcome(ctx, 'quality', findings, disposition) wired into runQualityLoop, all five branches correctly mapped and positioned before the fix stage.
Cap exhaustion rolled up to exactly one VERIFY_SKIPS line per issue via ctx.quality_cap_skip_index, lazily initialized.
quality_degrades doc-clarified in both the FRICTION_WEIGHTS comment and gate-hygiene.md.

Code Review (Iteration 1)'s three findings are fixed, verified directly in the tree:

MAJOR (gate-hygiene.md undercounting quality_degrades at two agent deaths instead of three): fixed at both :245 and :318 — the fix-agent death (workflows/ticketmill.js:3144) is now named alongside simplify/review, with an explanation of why it stays outside the five-branch disposition map while still counting toward quality_degrades.
MINOR (computeGateYield's escaped-defects header overclaiming "every earlier gate missed"): fixed at workflows/ticketmill.js:5640, now reads neither "approach" nor "plan" raised, mirrored in .claude/workflows/ticketmill.js.
MINOR (provenance passage overstating metrics.md's tracked segment as "the entire file"): fixed in gate-hygiene.md and its AGENTS.md/CLAUDE.md freeze pair. A follow-up commit (bd66476, not requested by the code review but a legitimate self-caught refinement of the same fix) corrects an off-by-one in the fix's own wording — "329 lines... running to the end of the 332-line file" implied 328 on-disk lines, not 329; now states 329 tracked lines = 328 on disk (5-332) + one trailing blank line stripped at write time and padded back in by the provenance test. This is a same-file precision correction, not scope creep.

Verified directly:

node --test: 641/641 pass.
diff workflows/ticketmill.js .claude/workflows/ticketmill.js: byte-identical (lockstep intact).
diff docs/architecture/AGENTS.md docs/architecture/CLAUDE.md: byte-identical (freeze pair intact).
No ctx.unresolved write from the quality loop; frictionFields's contrarian_capped/unresolved_count stay false/0 on a capped quality loop, per tests/friction-fields.test.js.

Scope: unchanged from Iteration 1 — engine, tests, and docs only, all within the three planned tasks plus the code-review fix-up. No scope creep found.

aaddrick · 2026-07-28T06:27:34Z

Code Review (Iteration 2)

Verdict: changes requested - one MINOR left over from iteration 1's MAJOR. Both documentation fixes and the escaped-defects header reword landed correctly and I re-verified each of them; the engine logic is unchanged since iteration 1 and still correct.

Iteration 1 findings: re-verified

MAJOR (quality_degrades undercount) - fixed in the docs. docs/architecture/gate-hygiene.md:318-322 now names all three deaths ("the two agent-death branches in the disposition map above (simplify dies, review dies), plus a third that sits outside that map: the fix agent dying in the fix stage"), and :245-252 replaces "Both agent-death branches" with a paragraph that explicitly keeps the disposition map at five branches while stating "All three deaths break immediately". That is the right shape: the fix-stage death at workflows/ticketmill.js:3144 genuinely does not need a disposition branch, because the re-litigated/carried-unresolved call at :3125 already fired for that iteration.
MINOR (escaped-defects header) - fixed. workflows/ticketmill.js:5640 now renders from the constant it actually consults: EARLY_GATES.join('" nor "') produces neither "approach" nor "plan" raised, so the prose can no longer drift from EARLY_GATES the way the hard-coded wording did.
MINOR (metrics.md segment overclaim) - fixed, and the correction is itself exact. I checked the arithmetic rather than taking the new wording on faith: docs/architecture/metrics.md is 332 lines; tests/fixtures/architecture-split.json:87 records one segment starting at ## Friction & churn (line 5) for 329 lines; reverseSegment (tests/architecture-provenance.test.js:104-125) pops the trailing empty split element, so endIdx (332) lands one past onDiskLength (332), the while loop pads exactly one blank line, and the endIdx >= onDiskLength branch substitutes lastLine. So "329 lines ... 328 of them on disk ... plus one trailing blank line stripped when the file was written" is correct to the line, in all three copies. metrics.md:114 sits inside the segment, so the supersession-rather-than-edit conclusion still holds.

Finding

MINOR - workflows/ticketmill.js:3156 (and the identical .claude/workflows/ticketmill.js:3156): the cap-exhaustion comment still carries the undercount iteration 1 flagged.

  // Cap exhaustion (issue #163): the loop ran out of iterations without a
  // clean review AND without an agent dying (both degrade branches above
  // break out before reaching here).

There are three degraded = true sites in runQualityLoop, all above this comment: simplify death :3086, review death :3112, fix death :3144. "both degrade branches" counts two. This is the same claim iteration 1 raised as MAJOR against gate-hygiene.md; the fix commit corrected the two doc statements and the FRICTION_WEIGHTS comment (:2524, which correctly says "simplify, review, or fix"), but left this third copy - the one physically adjacent to the code, and the one a maintainer reading runQualityLoop will hit first. grep -rn "both degrade branches" over the engine and docs now returns exactly this one line, in both lockstep copies.

The predicate itself is correct and I am not asking for a logic change: !degraded covers all three deaths, so the guard means what the comment concludes. The enumeration in the parenthetical is what is wrong, and it is wrong in the same direction that made it a MAJOR one iteration ago: it leaves a reader believing a fix-agent death is not one of the branches this block is guarding against.

Fix direction: change "both degrade branches above" to "all three degrade branches above" (or "every degrade branch above"), in workflows/ticketmill.js and its .claude/workflows/ticketmill.js mirror, so node scripts/lint-engine.js stays green.

Verified clean

Baseline: node --check workflows/ticketmill.js passes; node scripts/lint-engine.js reports "clean (workflows/ticketmill.js, 2 lockstep pairs in sync)"; diff workflows/ticketmill.js .claude/workflows/ticketmill.js is byte-identical; node --test is green at 641/641.
Sandbox: no Date.now(), argless new Date(), Math.random(), require(, fs., process., or agentType on any line added since the base.
Engine logic unchanged since iteration 1, and re-traced anyway: the five recordGateOutcome calls still each sit between their verdict and the fix stage; quality_iters++ at :3064 still sits after the STOP.tripped return, so sum(disposition) === quality_iters is exact; !approved && !degraded at :3169 is still reachable only by exhausting MAX_QUALITY_ITERATIONS.
The roll-up index is still safe: VERIFY_SKIPS is declared once at :404 and never reassigned (every other reference is a push, a read, or the new in-place rewrite), and ctx is built fresh per unit in processIssue at :5066 with no rehydration from persisted state on resume, so ctx.quality_cap_skip_index can never point into a VERIFY_SKIPS from a different run.
computeGateYield still isolates quality from the escape calculation: ESCAPE_GATE/EARLY_GATES (:5541-5542) are unchanged, escaped reads only those keys, and the new footnote is guarded on byGate.quality with a leading lines.push('') so it cannot glue to the markdown table. The accepted tally on an approved-with-nits quality review matches how pr-review already tallies at :4893.
ctx.unresolved is still never written from the quality loop, and tests/friction-fields.test.js pins contrarian_capped: false / unresolved_count: 0 on a capped loop.
docs/architecture/AGENTS.md and docs/architecture/CLAUDE.md are byte-identical (diff clean), and index.md:15's file-map row matches the copies in both.

Not flagged, for the record

Gate Yield's has_signal going true on essentially every run: adjudicated at the approach gate, carried in the PR body.
The empty-findings exit's unbounded per-iteration VERIFY_SKIPS line at :3121: pre-existing from Make review findings the load-bearing artifact #162, and the bounding constraint in Tally the quality gate and stop swallowing its cap #163 was scoped to cap exhaustion.
Extracting a shared helper for the two *_skip_index call sites: already recorded as a deferred follow-up.
CHANGELOG entry and plugin.json bump: batch-level, owned by the Report-phase release stage.

…count The cap-exhaustion comment claimed "both degrade branches above" but the loop has three (simplify, review, fix) — a leftover from before the quality_degrades undercount fix added the third branch.

aaddrick · 2026-07-28T06:28:53Z

PR Review Fix (iteration 2)

Commit: 9a63771

Fixed the code review's one-word finding: the quality-gate cap-exhaustion comment at workflows/ticketmill.js:3156 claimed "both degrade branches above" when the loop actually has three (simplify, review, fix) — a leftover from before the prior iteration's quality_degrades undercount fix added the third branch. Applied identically to both lockstep copies (workflows/ticketmill.js and .claude/workflows/ticketmill.js), verified byte-identical after the edit. Full suite: 641/641 green.

aaddrick · 2026-07-28T06:32:31Z

Spec Review (Iteration 3)

Verdict: approved

Re-verified PR #172 against issue #163's three required changes and hard constraints, on top of the "PR Review Fix (iteration 2)" commit (9a63771), diffing against the correct base (origin/Batch_2026-07-27_225225).

All three requirements stand as approved at Iterations 1 and 2, unaffected by the latest commit:

recordGateOutcome(ctx, 'quality', findings, disposition) wired into runQualityLoop, all five branches correctly mapped and positioned before the fix stage.
Cap exhaustion rolled up to exactly one VERIFY_SKIPS line per issue via ctx.quality_cap_skip_index, lazily initialized.
quality_degrades doc-clarified in both the FRICTION_WEIGHTS comment and gate-hygiene.md.

Code Review (Iteration 2)'s one remaining finding is fixed, verified directly in the tree:

MINOR (workflows/ticketmill.js:3156 cap-exhaustion comment undercounting degrade branches at two instead of three): fixed by 9a63771 in both lockstep copies — "both degrade branches above" is now "three degrade branches above". grep -rn "both degrade branches" over the engine and docs returns nothing. This is a pure comment-wording correction; the predicate it describes (!approved && !degraded) was never wrong and is unchanged.

Verified directly:

node --check workflows/ticketmill.js: clean.
node scripts/lint-engine.js: clean, 2 lockstep pairs in sync.
diff workflows/ticketmill.js .claude/workflows/ticketmill.js: byte-identical (lockstep intact).
diff docs/architecture/AGENTS.md docs/architecture/CLAUDE.md: byte-identical (freeze pair intact).
node --test: 641/641 pass.

Scope: unchanged from Iteration 2 — the only commit since then is a two-line comment fix in the two lockstep engine files, addressing exactly the code review's finding. No scope creep found.

aaddrick · 2026-07-28T06:34:34Z

Code Review (Iteration 3)

Verdict: approved — no blocking findings. Iteration 2's single MINOR is fixed, and a fresh pass over the whole diff against origin/Batch_2026-07-27_225225 (base 4b034e2) turned up nothing new.

Iteration 2's finding: fixed

MINOR (workflows/ticketmill.js:3156 cap-exhaustion comment undercounting degrade branches) — fixed. Commit 9a63771 changes "both degrade branches above" to "three degrade branches above", applied to both lockstep copies. grep -rn "both degrade branches" over the engine and docs now returns nothing, and the three degraded = true sites (simplify :3086, review :3112, fix :3144) all sit above the !approved && !degraded guard at :3169, so the parenthetical is now true as written. The commit touches nothing but that one word in the two mirrored files.

Re-verified rather than assumed

Because this is the merge gate, I re-ran the full baseline and re-traced the parts a one-word commit could not have broken but that a stale earlier reading could have gotten wrong:

Baseline: node --check workflows/ticketmill.js passes; node scripts/lint-engine.js reports "clean (workflows/ticketmill.js, 2 lockstep pairs in sync)"; diff workflows/ticketmill.js .claude/workflows/ticketmill.js is byte-identical; diff docs/architecture/AGENTS.md docs/architecture/CLAUDE.md is byte-identical; node --test is green at 641/641.
Sandbox: no Date.now(), argless new Date(), Math.random(), require(, fs., process., or agentType on any line added since the base.
The invariant. ctx.metrics.quality_iters++ (:3065) sits immediately after the STOP.tripped return (:3064), and each of the five branches records exactly one disposition before its iteration ends. The fix-agent death at :3144 is reached only after the re-litigated/carried-unresolved call at :3125, so it cannot skew the sum. sum(gate_findings.quality.disposition) === quality_iters holds unconditionally, per issue, across calls.
VERIFY_SKIPS index safety, re-derived from scratch. grep -n "VERIFY_SKIPS" | grep -v "VERIFY_SKIPS.push" returns the let declaration at :404, the new indexed write at :3174/:3176, and three reads (:7835, :7873, :7982) — no reassignment, no splice, no length = 0. The array is append-only apart from this in-place rewrite, so ctx.quality_cap_skip_index stays valid under pool concurrency, and the index < VERIFY_SKIPS.length guard degrades to a fresh push rather than throwing if it ever were not.
Constants used by the new footnote are above the harness split. MAX_QUALITY_ITERATIONS (:287) and MAX_PR_REVIEW_ITERATIONS (:293) are both far above the TICKETMILL-TEST-HARNESS-SPLIT marker at :6779, so computeGateYield's footnote stays evaluable inside tests/harness.js's vm — the same purity contract the function's own comment claims.
The provenance arithmetic, checked against the fixture rather than the prose. tests/fixtures/architecture-split.json:87 records one segment for metrics.md starting at ## Friction & churn with "lines": 329; docs/architecture/metrics.md is 332 lines and that heading is line 5. So 5-332 is 328 on-disk lines plus one stripped trailing blank = 329, "everything but the four-line synthetic H1 and lede" is exact, and metrics.md:114 (the computeGateYield(results) lede, which does say "the three gates") sits inside it. The supersession-rather-than-edit conclusion is correct in all three copies.
No new docs drift. grep over README.md, skills/, and docs/ finds no other page enumerating the gates that record into gate_findings; skills/mill-review/SKILL.md:118 consumes gate_yield verbatim with no gate-name enumeration, so a fourth row flows through it without a doc edit.
computeGateYield still isolates quality from the escape calculation: ESCAPE_GATE/EARLY_GATES unchanged, escaped reads only those keys, the footnote is guarded on byGate.quality with a leading lines.push(''), and the header now renders from EARLY_GATES.join(...) so the prose cannot drift from the constant again.
ctx.unresolved still never written from the quality loop, pinned by tests/friction-fields.test.js.

Not flagged, for the record

Gate Yield's has_signal going true on essentially every run: adjudicated at the approach gate, carried in the PR body.
The empty-findings exit's unbounded per-iteration VERIFY_SKIPS line at :3121: pre-existing from Make review findings the load-bearing artifact #162; Tally the quality gate and stop swallowing its cap #163's bounding constraint was scoped to cap exhaustion.
Extracting a shared helper for the two *_skip_index call sites: already recorded as a deferred follow-up.
CHANGELOG entry and plugin.json bump: batch-level, owned by the Report-phase release stage.

aaddrick · 2026-07-28T06:37:33Z

Implementation Complete

Branch issue-163-tally-the-quality-gate-and-stop-swallowing-its-cap squash-merged into Batch_2026-07-27_225225.

runQualityLoop now calls recordGateOutcome(ctx, 'quality', ...) at every exit branch (agent death x2, approved, empty-findings cap, changes-requested cap/re-litigate), giving sum(gate_findings.quality.disposition) === ctx.metrics.quality_iters per issue.
Cap exhaustion now surfaces as exactly one rolled-up VERIFY_SKIPS entry per issue via a lazily-initialized ctx.quality_caps array keyed by ctx.quality_cap_skip_index, rewriting in place across repeat quality-loop invocations rather than duplicating.
ctx.unresolved / frictionFields (unresolved_count, contrarian_capped) deliberately untouched — those stay scoped to CONTRARIAN gate caps; the new quality key in gate_findings carries the quality-gate signal instead.
quality_degrades metric documentation clarified to state it counts agent death inside the loop, not quality regression or cap exhaustion.

Reviews passed: spec review and code review, two fix iterations each (contrarian challenge on approach, then a code-review pass that caught a stale doc line and an off-by-one in its own follow-up wording, both corrected).

Deferred Suggestions for Follow-up

Extract a shared "remembered index, rewrite in place" roll-up helper for the VERIFY_SKIPS pattern this PR introduced (ctx.quality_cap_skip_index), and point the two existing unbounded twins at it: :3092 (quality loop empty-findings push) and :3624 (test loop's twin). Both currently fire per scope per iteration with no roll-up. Filed as Extract shared roll-up helper for VERIFY_SKIPS index-rewrite pattern #173.

aaddrick added 3 commits July 28, 2026 01:41

fix(observability): correct the quality cap comment's degrade-branch …

9a63771

…count The cap-exhaustion comment claimed "both degrade branches above" but the loop has three (simplify, review, fix) — a leftover from before the quality_degrades undercount fix added the third branch.

aaddrick merged commit 3568b29 into Batch_2026-07-27_225225 Jul 28, 2026
1 check passed

aaddrick deleted the issue-163-tally-the-quality-gate-and-stop-swallowing-its-cap branch July 28, 2026 06:36

aaddrick mentioned this pull request Jul 28, 2026

Extract shared roll-up helper for VERIFY_SKIPS index-rewrite pattern #173

Open

This was referenced Jul 28, 2026

Tally the quality gate and stop swallowing its cap #163

Closed

Fix the quality friction ratio's denominator #165

Closed

fix(observability): pool the quality friction denominator by scope count #176

Merged

Navigation Menu

fix(quality-gate): tally the quality gate and bound its cap line (#163) - #172

fix(quality-gate): tally the quality gate and bound its cap line (#163)#172
aaddrick merged 6 commits into
Batch_2026-07-27_225225from
issue-163-tally-the-quality-gate-and-stop-swallowing-its-cap

aaddrick commented Jul 28, 2026

Uh oh!

aaddrick commented Jul 28, 2026

Uh oh!

aaddrick commented Jul 28, 2026

Uh oh!

aaddrick commented Jul 28, 2026

Uh oh!

aaddrick commented Jul 28, 2026

Uh oh!

aaddrick commented Jul 28, 2026

Uh oh!

aaddrick commented Jul 28, 2026

Uh oh!

aaddrick commented Jul 28, 2026

Uh oh!

aaddrick commented Jul 28, 2026

Uh oh!

Uh oh!

aaddrick commented Jul 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

aaddrick commented Jul 28, 2026

Summary

Key decisions

Uh oh!

aaddrick commented Jul 28, 2026

Spec Review (Iteration 1)

Uh oh!

aaddrick commented Jul 28, 2026

Code Review (Iteration 1)

Verified clean

Findings

Not flagged, for the record

Uh oh!

aaddrick commented Jul 28, 2026

PR Review Fix (iteration 1)

Uh oh!

aaddrick commented Jul 28, 2026

Spec Review (Iteration 2)

Uh oh!

aaddrick commented Jul 28, 2026

Code Review (Iteration 2)

Iteration 1 findings: re-verified

Finding

Verified clean

Not flagged, for the record

Uh oh!

aaddrick commented Jul 28, 2026

PR Review Fix (iteration 2)

Uh oh!

aaddrick commented Jul 28, 2026

Spec Review (Iteration 3)

Uh oh!

aaddrick commented Jul 28, 2026

Code Review (Iteration 3)

Iteration 2's finding: fixed

Re-verified rather than assumed

Not flagged, for the record

Uh oh!

Uh oh!

aaddrick commented Jul 28, 2026

Implementation Complete

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant