fix(fabrika): derive every published KEEP-corpus figure from the enumeration (#4823) - #5177
Conversation
No preview deploy
|
|
review-code: FAIL @ 44ce6a0 Verified against #4823 at head The central claim checks out — except for the one criterion it rests onThe PR argues five of six acceptance criteria were already delivered by PR #4837 and live on Per-criterion
The two standing traps — both verified by doing, both hold1. Does the check assert the right property? Re-ran all three claimed mutations in the head worktree, reverting between each. Every one reds:
The figure is genuinely derived, not typed: 2. Can a read that did not run count as clean? No. Each surface is read by path inside the assertion, so an unreadable one throws (mutation 3). The non-empty surface assert is real and independently reds — I emptied The disclosed limit — acceptable bound, correctly disclosedPositive half = verbatim phrase match, negative half = fixed deny-list. It catches drift and a copy of the known-bad wording, not a cardinality worded a new way. For this ticket that bound is fine: the failure that actually happens is a stale copy, and a prose parser for "the corpus size" would be a check asserting its own cleverness. The disclosure is honest and well-classified. What the disclosure gets wrong is not the bound but the premise — it claims completeness of the surface set ("the last in-tree page"), and that claim is false, which is what the FAIL above rests on. Other gate facets
VerdictFAIL — conjunctive. Five criteria hold, both standing traps are genuinely handled, and the mechanism is well-built. One criterion — the only one this PR set out to close — is still open, on a page the PR's own deny-list catches the moment it is declared. Leaving #4649's stale figure untouched was the right boundary: it is an issue body this lane holds no claim on, the mis-attribution guard refuses the write, and the finding is already recorded on #4823. That call stands. Verdict-written: 2026-08-09T20:30:15Z |
…eration (#4823) The enumeration landed in PR #4837, but the ticket's scope line — correct the published cardinality at each place it appears — was discharged surface by surface, and one was missed: the fabrika authoring-brief contract still told every brief author to pull incident rows from "the 74-issue KEEP corpus" and pointed at #4642/#4634 rather than at the committed file. It predates the correction, so it is the same defect as #4838 one directory over. The doc now points at ruled-keeps.json and publishes 66 members plus 1 pending, keeping #4642/#4634 as the enumeration's provenance rather than as the recipe a reader is asked to re-run. To stop the next copy going stale, the figure is derived rather than written: `publishedFigure` computes it from the rows, and `publishedFigureViolations` checks each artifact that publishes it. The data test declares both surfaces — the corpus README and the authoring-brief contract — and reads each by path, so an unreadable surface throws rather than counting as clean; "could not read it" and "it carries no stale figure" are different facts. Proven by mutation, not by reading: deleting one member row reds both surfaces (65 members plus 1 pending is published nowhere), reintroducing the string "74-issue KEEP corpus" reds the contract doc, and pointing a declared surface at a missing file reds with ENOENT. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… guard it (#4823) The `authoring brief` row still published "the 74-issue KEEP corpus" — the definition a brief author reads before the contract doc, and the one page the previous round's completeness claim missed. It now links `ruled-keeps.json` and carries the derived "66 members plus 1 pending". The mechanism was sound; its surface list was not. `.glossary/TERMS.md` joins `SURFACES` in the data test, so the guard holds the page going forward. A full-tree sweep for every `DISCREDITED_FIGURES` phrase now returns zero live prose hits — the remaining matches are the deny-list itself and two test fixtures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
44ce6a0 to
da8be91
Compare
|
review-code: PASS @ da8be91 — merge-ready Repair round 1. The prior verdict at File count 5 → 6. The added file is The round-0 blocker is closed, and closed the right wayThe FAIL rested on one thing: The claim that mattered: the glossary figure is DERIVED, not hand-typedThe repair claims the glossary's Reverted, green again (13/13 in the data file). The glossary number is bound to the enumeration, not typed beside it. The sweep, re-run independently — this is where round 0 went wrongRound 0's blocker existed because a sweep was assumed rather than run, so I ran it myself over the whole head tree (
I also swept wider than the deny-list — every Per-criterion (#4823)
The three round-0 mutation proofs still redEach applied in the head worktree, run, then reverted; worktree confirmed clean afterwards.
The ADR 0092 zero-scope split is untouched — and still correctI emptied The disclosure is honestThe repair-round entry retracts the false premise explicitly — round 0's "the authoring-brief contract is the last in-tree page publishing the discredited figure" is named as wrong, attributed to the gate catching it, and replaced with a sweep result rather than a fresh assertion. Critically it leaves the bound standing (phrase match + fixed deny-list, disclosed as a Class 1 narrowing): the bound was never the problem, and re-litigating it would have been the wrong repair. That is the correct split. One accuracy nit, not a defect: the body's "What changed" sentence accounts for the remaining matches as "the deny-list itself and two test fixtures" — five of the six. The sixth (the data test's Other gate facets
Run-evidence bundle: PRESENT for head Read the PR head (§HEAD): all files under review sourced from All criteria pass. This PR is merge-ready. review-code does not merge — Verdict-written: 2026-08-09T20:48:12Z |
1 participant
The ruled KEEP corpus is a committed list you read (
ruled-keeps.json), but two pages still told authors otherwise: the glossary'sauthoring briefentry and the fabrika authoring-brief contract both asked a brief author to pull incident rows from "the 74-issue KEEP corpus", and the contract pointed at two issues to join by hand. Both now point at the file and publish the real figure. To stop the next copy going stale, the figure is no longer typed by hand anywhere — it is computed from the enumeration, and a test checks every page that publishes it.Fixes #4823
What changed
claude-plugins/fabrika/docs/authoring-brief-contract.md— field 3 now namesruled-keeps.jsonandfabrika eval keeps, publishes 66 members plus 1 pending, and keepsDecision (founder-decision-fork): approve the 137-issue kill batch from the #4634 sweep #4642 / Investigation: sweep the 222 open pipeline issues — which encode real incidents worth preserving as eval cases #4634 as the enumeration's provenance rather than as a recipe the reader re-runs. The
ruling's 74 is described as superseded, not restated as fact. The worked example's caption and
the "cannot open 74 issues" line follow.
.glossary/TERMS.md— theauthoring briefrow's field-3 clause, the definition a brief authorreaches before the contract doc, now links
ruled-keeps.jsonand publishes the same derivedphrase instead of "the 74-issue KEEP corpus".
packages/fabrika-cli/src/eval/ruled-keeps.ts—publishedFigurederives the cardinalityphrase from the rows;
publishedFigureViolationschecks one artifact against it, and againstDISCREDITED_FIGURES(the exact phrasings the 74 was published as in-tree).packages/fabrika-cli/src/eval/incident-corpus/README.md— same derived phrase, so the pagesagree by construction instead of by coincidence.
surfaces (corpus README, glossary, contract doc) and reads each by path.
Why this closes #4823 rather than opening a new ticket
Five of the six acceptance criteria were delivered by PR #4837 and are live at
main; I verifiedeach rather than re-implementing it:
mainruled-keeps.json, 67 rows (66member+ 1pending); coverage joined fromprovenance.jsonat read timederivation.recipe/sources/arithmeticpending,pendingReasoncarries "hereby retracted"5162555158, appended not editedThe scope line "correct the published cardinality at each place it appears" was discharged one
surface at a time — README, then
provenance.json(#4838), then the contract doc and the glossaryentry here. A full-tree sweep for every
DISCREDITED_FIGURESphrase now returns zero liveprose hits: the only remaining matches are the deny-list itself and two test fixtures. With that,
every criterion holds at merge.
The check reds — proven by mutation, not by reading
Each mutation applied to the worktree, the data test run, then reverted:
memberrow fromruled-keeps.jsondoes not publish the enumerated figure '65 members plus 1 pending'74-issue KEEP corpusin the contract docasserts the discredited '74-issue KEEP corpus'ENOENTThe third is the point of reading each surface by path: a surface that cannot be read throws.
"Could not read it" and "it carries no stale figure" are different facts, and the check never
collapses the first into the second (ADR 0092). The surface list is asserted non-empty for the
same reason.
Also run at the repair-round head: the two
ruled-keepstest files green (31 tests),pnpm typecheck31/31,pnpm lint:worktreeclean. Round 0 additionally ran the wholepackages/fabrika-clisuite (175 files / 2485 tests) and thekeepsCLI verb, which printed66 member(s) plus 1 pendingand{"members":66,"pending":1,"borderline":7,"covered":7,"uncovered":59}; this round changes nomember row, so that output is unchanged.
Deviations
Class 1 (scope narrowing) — the positive check is a phrase match, not a parser. Said: correct
the published cardinality wherever it appears. Did: required each declared surface to carry the
derived phrase verbatim, and denied a fixed list of stale phrasings. Why: the derived half reds on
any drift, which is the failure that actually happens; a deny-list cannot catch a cardinality
someone words in a new way, and a prose parser for "the corpus size" would be a check that asserts
its own cleverness rather than the property. Disposition: no action needed — stated here so a
reviewer judges the limit rather than inferring completeness.
Class 1 (scope narrowing) — the data test reaches outside its package. Said: nothing. Did: the
fabrika-clidata test readsclaude-plugins/fabrika/docs/authoring-brief-contract.mdby relativepath. Why: the corpus publishes its size to a page outside the package, so a check confined to the
package would go green while the page an author reads still said 74 — the exact shape of the defect
being fixed. Disposition: for the reviewer to judge; the alternative (a repo-wide guard in
pipeline-cli) is a larger surface than this ticket asked for.Class 2 (found a sibling defect, left it) — the stale 74 in #4649's body. Said: correct each
place the figure is published. Did: left #4649's Destination / bar bullet / Pitch untouched. Why:
it is an issue body this lane holds no claim on, and the mis-attribution guard refuses the write;
the 2026-08-08 amendment on #4823 already records the finding as #4649's own lane. Disposition:
recorded here and in the progress comment so it is not lost.
Class 3 (did not re-verify a prior result) — the 66 itself. Said: the enumeration is the
deliverable. Did: took
ruled-keeps.json's membership as given and re-derived nothing against#4634 / #4642. Why: that derivation was run twice and gated when PR #4837 landed; re-running it
here would change no row and would put a second, unreviewed derivation in the record. Disposition:
no action needed — this PR changes no member row.
(repair round 1) Retraction — an earlier round's disclosure carried a false claim. Said, in round
0: the authoring-brief contract "is the last in-tree page publishing the discredited figure". That
was wrong —
.glossary/TERMS.md:35published the same phrase, and the gate caught it. The claim isretracted and replaced above with a sweep result rather than an assertion: every
DISCREDITED_FIGURESphrase, searched across the whole tree, returns zero live prose hits.Disposition: fixed, not merely disclosed — the glossary row is corrected and added to
SURFACES,so the guard holds it going forward. The bound on the check (phrase match + fixed deny-list)
stands as disclosed above; what was false was the completeness premise, not the bound.
Not touched:
packages/fabrika-cli/package.json(versioning is the release lane's), and the--modelalias work in flight on #5158 / #5148 — this diff does not reach the capture manifest.