Skip to content

feat(sdlc): make falsify-before-review an executable step (#590) - #596

Merged
BaseInfinity merged 1 commit into
mainfrom
fix/ship-review-lessons
Aug 14, 2026
Merged

feat(sdlc): make falsify-before-review an executable step (#590)#596
BaseInfinity merged 1 commit into
mainfrom
fix/ship-review-lessons

Conversation

@BaseInfinity

Copy link
Copy Markdown
Owner

Ships the two reusable lessons from the #590 / PR #595 post-merge audit into the skill that installs into every consumer repo. They were living in one maintainer's private notes; that's the wrong home for a rule that applies to everyone.

The finding

Fable and Sol both audited the merged PR. Asked for the single biggest source of wasted rounds, Sol answered:

The builder hands work to the reviewer before trying to falsify it adversarially. That single failure produced nearly all the expensive rounds here.

Its evidence was specific: round 2's prompt asked the reviewer to check publication races and pid reuse that the builder had not tested first. Its phrase — "using review as the builder's test phase."

What ships

Step 1, falsify first. Not "be rigorous" — a checklist with a done condition. One row per factual claim and per risk named in the prompt, recording claim/risk | falsifier | route | evidence, routed through the file's existing EVAL / plain-assert / JUDGMENT-ONLY three-way call, with separate known-success and known-failure rows for every detector. Launch only when every row holds evidence or an explicit judgement-only disposition.

A direction demand, folded into the run-reviewer step. A prompt must ask for the smallest change that would satisfy the certify condition — and that direction is a proposal under the dialogue loop: implement it, dispute it with evidence, or route it to the brain if it changes the design.

How the reviewers changed it

This PR is its own argument, so the corrections are worth naming:

  • Sol rejected the first draft as unexecutable exhortation. "Falsify your own work" is worthless if the reader can't tell when they're done. The checklist is its text.
  • Both flagged that the draft contradicted this file's own scoped-TDD rule (RED only where a RED mutation is writable) — and this prose change is the counterexample. Routing through the three-way call resolves it.
  • Sol caught the draft breaking this repo's claim rule, which is the sharper correction. It asserted "each one cost an architectural round" and "could have falsified in minutes" — neither measured, neither timed — and stated a causal collapse where the record supports only a sequence. Fixed to what the record shows. A file that tells readers not to ship unmeasured claims cannot ship one in its own rationale.
  • Sol caught "implement verbatim" contradicting the dialogue loop's explicit right to dispute a finding.
  • Fable caught the ordering: falsify-first was written after the step that launches the leg, so a reader executing top-to-bottom had already sent the review before reaching the rule governing what comes first.

Verification

Guards run before requesting the verdict — itself the lesson from #595, where certifying before the full suite let a CI-fix commit land post-clearance and cost a round: doc-consistency, cowork-drift (byte parity), hooks, workflow-triggers, session-load audit, skill-graduations, compliance, docs-usability, plugin — all pass.

Skill is 34,426 chars. The 20,000-byte ceiling was deliberately deleted in #489 as an unmeasured number, so there is none to violate — checked rather than assumed.

Two rounds. Sol: CERTIFIED 10/10, confidence 99.

Post-merge audit of PR #595 by Fable and Sol. Asked for the single biggest
source of wasted rounds, Sol answered: 'The builder hands work to the
reviewer before trying to falsify it adversarially.' Round 2's prompt had
asked the reviewer to check publication races and pid reuse that the builder
had not tested first — using review as the builder's test phase.

Step 1 is now that discipline, and both reviewers shaped it.

Sol rejected the first draft as unexecutable exhortation and as a
contradiction of this file's own scoped-TDD rule. It ships as a checklist
with a done condition — one row per claim and per named risk, recording
claim/risk | falsifier | route | evidence, routed through the existing
EVAL / plain-assert / JUDGMENT-ONLY three-way call, with separate
known-success and known-failure rows for every detector.

Sol also caught the draft breaking this repo's claim rule, which is the
sharper correction: it asserted 'each one cost an architectural round' and
'could have falsified in minutes' — neither measured, neither timed — and
stated a causal collapse where the record supports only a sequence. The
Why now says what the record shows.

The direction-demand folds into the run-reviewer step: a prompt must ask
for the smallest change that would satisfy the certify condition, and that
direction is a proposal under the dialogue loop, not a command. The first
draft said 'implement verbatim', contradicting the loop's explicit right to
dispute a finding with evidence.

Fable caught the ordering: falsify-first was written after the step that
launches the leg, so a reader executing top-to-bottom had already sent the
review before reaching the rule governing what comes first.
@BaseInfinity
BaseInfinity force-pushed the fix/ship-review-lessons branch from 46febe0 to e387a9c Compare August 14, 2026 00:50
@BaseInfinity

Copy link
Copy Markdown
Owner Author

CROSS-MODEL-CLEARANCE

Sol (GPT-5.6 Sol, high) — round 2 of 2. Round 1 returned three corrections and a smallest-passing-change; all applied with its own replacement text. Verbatim: "1. Yes. The certify condition is satisfied. 2. No remaining claim-rule violations. 3. VERDICT: CERTIFIED — 10/10."

The head SHA changed after certification only by dropping an unrelated local settings commit that had ridden in from local main. The certified diff is proven byte-identical across that rebase (sha1 aef90bc5118b0a0b before and after), so the certification covers this head's content exactly. CI validate green (3m49s) BEFORE this clearance was posted — the sequencing lesson from #595, where certifying first let a CI fix land afterwards and cost a re-bind round.

{
  "confidence": 99,
  "reviewer": "gpt-5.6-sol-high",
  "sha": "e387a9cb4e2da40b7c72f19f41b86bc6d97a20a4",
  "verdict": "YES"
}

@BaseInfinity

Copy link
Copy Markdown
Owner Author

CROSS-MODEL-CLEARANCE

Fable (high) — both reviewers shaped the final wording. The draft's three defects each got the reviewer's own replacement text: unexecutable exhortation became a checklist with a done condition, the contradiction with this file's scoped-TDD rule was resolved by routing through the existing three-way call, and the claim-rule violations in its own rationale were replaced with what the record shows. Fable's own finding was the ordering — falsify-first was written after the step that launches the leg, so a reader executing top-to-bottom had already sent the review.

Two rounds total, which is the cap this change itself prescribes for prose. Guards ran before the verdict was requested rather than after.

{
  "confidence": 96,
  "reviewer": "fable-high",
  "sha": "e387a9cb4e2da40b7c72f19f41b86bc6d97a20a4",
  "verdict": "YES"
}

@BaseInfinity
BaseInfinity merged commit d88bdaa into main Aug 14, 2026
4 checks passed
BaseInfinity added a commit that referenced this pull request Aug 15, 2026
…tly not happen (#620)

* release: v1.98.0 — the review loop terminates, and a leg cannot silently not happen

Nine consumer-affecting commits have been sitting on main since the v1.97.0
release commit, reaching nobody. release-drift.yml filed #618 for exactly this,
and the v1.98.0 milestone has been closed with eleven issues and no tag.

What consumers get that they did not have:

  #606  the review loop has a stated termination condition
  #590  a launcher for review legs, and a gate that requires it
        (#595 the owner-not-observer design, #596 falsify-before-review,
         #610 the gate lane that refuses a hand-typed leg)
  #613  the build is an input to a review verdict, not background noise
  #588  the commit gate stops matching `git commit` in prose
  #581  a JSON-escaped newline no longer hides an invocation from the gate
  #533  the review gate no longer makes the review protocol uncommittable
  #577  the review contract as one prose batch

The two hook fixes are the ones worth reading twice: both REFUSED legitimate
work on consumer machines. A commit whose message merely mentioned the phrase
was blocked, and a command split across an escaped newline walked straight
past. Consumers have been running both since v1.97.0.

package.json and CHANGELOG.md only. Every item was certified on its own PR;
this commit ships them.

doc-consistency 137/0. release-workflow 14/0. release-drift 21/0.

* fix(changelog): restore the 1.97.0 header this release deleted

Review found it, and it was the worst kind of mistake to make in a release PR:
the edit that inserted the 1.98.0 entry anchored on the `## [1.97.0]` header
and replaced it, so the header vanished. Headers ran 1.98.0 -> 1.96.0, and the
whole v1.97.0 entry — the Cowork classifier removal and everything beneath it —
read as part of THIS release.

A changelog that attributes a previous release's work to the current one is
worse than a missing entry: it is confidently wrong, and it ships to every
consumer as the record of what they just installed.

Headers now read 1.98.0 (7), 1.97.0 (65), 1.96.0 (153), 1.95.0 (228).

No suite checks that changelog version headers are present and ordered, which
is why CI passed on it. Filed separately rather than fixed here — this PR's own
review bar blocks on any file other than package.json and CHANGELOG.md
changing, and a release PR is the wrong place to grow test surface.

The same review verified every load-bearing claim in the new entry against its
certified source and found each accurate verbatim, and confirmed no
consumer-affecting change is missing from it.

* fix(release): bump every version stamp, and correct two false CHANGELOG claims

CI validate was red on 3e5d36a: test-self-update's version-parity check,
twice. SDLC.md:123 enumerates nine locations the version string lives in;
this release had bumped package.json and CHANGELOG.md only. Bumped the rest.

The strings left at 1.97.0 are deliberate — transcripts of observed command
output, and historical "removed in v1.97.0" references. Bumping those would
falsify them.

Two CHANGELOG claims were wrong and are corrected:

- It said consumers receive scripts/run-review-leg.sh. They do not.
  `npm pack --dry-run | grep -c scripts/` is 0; package.json's files list
  has no scripts/ entry. The #610 gate lane ships in hooks/; the launcher it
  names is #594, still open. A "Not in this release" section now says so.

- It said all three commit-gate defects blocked legitimate work. #588 and
  #533 did; #581 failed OPEN. Opposite failure directions, and reading one
  for the other gives a consumer the wrong idea of what the gate is for.

The review bar itself was also wrong. Rule 3 admitted only package.json and
CHANGELOG.md, which is mutually exclusive with CI's parity test — a release
PR satisfying it cannot go green. Amended to permit the SDLC.md:123 stamps.

* docs(sdlc): add the Cowork marketplace manifest to the version-bump checklist

Both reviewers found this independently on the v1.98.0 release PR — Sol as a
P1, Fable as a P3. SDLC.md:123 enumerates where the version string lives, and
the amended release-PR rule points at that enumeration. It omitted
cowork/.claude-plugin/marketplace.json, which every release must bump:
test-cowork-drift.sh:89 enforces marketplace/plugin parity and :283 enforces
Cowork/package parity, so moving package.json forces both Cowork manifests.

So a release that satisfies the checklist still fails CI, and a release that
passes CI is outside the checklist. Same shape as the rule-3 defect this PR
already fixed, one level down.

The bump was correct; the checklist was stale. Fixed the checklist.

Also: ROADMAP.md's "Last release: vX.Y.Z" marker joins the list — this release
had to bump it and it was not named — and the "no dedicated test yet" clause
now excludes the Cowork marketplace, which test-cowork-drift does cover.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant