fix(review-code): the glossary gate says so when it cannot see (#4299)#4331
Conversation
Step 3c's empty-scan branch had two causes wearing one line: "this PR added no new surface" and "this repo has no surface of the kind I know how to look for". Both emitted the not-applicable skip, so a repo that adopted .glossary/TERMS.md while keeping its own layout got a green gate on every PR forever. Add a positive-evidence probe (ADR 0092's "pass on positive evidence of scope"): count the surfaces the detector's own patterns can express on fresh base. With at least one, an empty scan is a fact about the PR and keeps the existing skip. With zero, it is a fact about the detector, and the gate now emits a distinct detector-blind line and folds an [UNVERIFIABLE] row into the conjunctive table — a blocking soft fail, not an omitted row. The other three outcomes are untouched: glossary absent on base still takes the graceful-absence skip (evaluated first, so it short-circuits the new branch), new surface + glossary touched still PASSes, new surface + glossary untouched still FAILs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
No preview deploy
|
|
claim: fcd74bd3-8872-4023-b100-a81ac870eb43 · 2026-07-26T22:27:24.627Z · presence 09f7c847e681/23523 |
|
review-skill: FAIL @ f841260 — changes-requested Reviewed-head: @ f841260 §CP blocking-set PR — the diff touches Verified PR #4331 against #4299's acceptance criteria + the skill-rigor checklist. The substance of this fix is sound and independently corroborated — one defect blocks it, and its remedy is a single restructure inside the existing scope fence. Acceptance criteria
The AC 3 defect — the ordering is asserted, never enforcedThe PR body states the graceful-absence probe "runs first and short-circuits the whole step." I verified that claim in the actual text, as asked. What is true: SKILL.md:1161–1164 adds a sentence asserting the ordering, and SKILL.md:1108–1109 conditions the
Why this is newly load-bearing rather than pre-existing noise. The same textual ordering existed on base — but there the The failure mode lands on exactly the population this campaign targets: a fresh adopter with no glossary at all — the most common foreign-repo state, and the one triage explicitly carved out as correct-by-design — now risks a hard gate FAIL accompanied by a false explanation. That is worse than the vacuous pass being fixed: a false blocking claim beats a silent green for damage. And it reintroduces this issue's own sin one branch over — an emitted self-assertion stating something the gate never established. Remedy — one restructure, entirely inside the existing scope fence. Make the ordering structural instead of asserted by leading the same if-chain at SKILL.md:1117–1131 with the probe the blockquote already describes: if ! git cat-file -e "origin/$BASE_REF:.glossary/TERMS.md" 2>/dev/null; then
echo "glossary-freshness: not applicable — no .glossary/TERMS.md on base" # graceful absence, §ZS #3 skip
elif [ -n "$(printf '%s' "$NEW_SURFACE" | tr -d ' ')" ]; then
...Nothing else in the PR needs to move; the blockquote's assertion then describes a property the block actually has. (Hoisting the blockquote above the block would also work, but the guard-in-the-chain is the version that cannot drift.) Skill rigor
On the disclosed fixture gap (judged, not deferred)Judging the AC and the honesty of the disclosure as separate questions, as asked: the AC is met, and the reasoning holds on its own. AC 1's checkbox requires only that the replay's Sanity-checks on the two coder observations
Read the PR head (§HEAD): all skill text under review was sourced from One failing item above must be addressed before this PR can merge. The PR stays open and unmerged; #4299 stays open and assigned. review-skill does not merge. Re-request review once the if-chain leads with the graceful-absence probe. Verdict-written: 2026-07-26T22:37:16Z |
…not prose (#4299) Step 3c's verdict chain assumed, but never tested, that a repo without .glossary/TERMS.md on base short-circuits before the new detector-blind branch. The probe existed only as a parenthetical inside a prose blockquote that sits physically after the chain, so the final `else` was reachable with the glossary absent and emitted a blocking UNVERIFIABLE asserting ".glossary/TERMS.md is on origin/$BASE_REF" — a fact it never checked. On base that ambiguity was inert (the else was a benign not-applicable skip); this branch made the two paths diverge, so a glossary-less fresh adopter — the most common foreign-repo state — risked a hard FAIL carrying a false explanation. Lead the chain with the executed `git cat-file -e "origin/$BASE_REF:.glossary/TERMS.md"` probe, making the detector-blind branch structurally unreachable when the glossary is absent, and reword the blockquote to describe the guard instead of asserting an ordering. Replayed both directions: with the glossary on base and DETECTOR_SURFACES_N=0 the chain still reaches UNVERIFIABLE; against a real pre-glossary base tree (026dbdf^) it emits the graceful-absence skip. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
review-skill: advisory — blocking-set PR (manual merge) PR #4331 touches the control plane ( Reviewed-head: @ 632fe91 Round-2 re-review of the repair ( The blocking item — cleared, verified structurally AND by executionRound 1 failed this PR because the graceful-absence probe was asserted in a prose blockquote at line 1155, physically after the verdict block, and never executed — so the Structural check. At head the chain leads with the executed probe ( The Execution check. I extracted
Run A is the corroboration you asked for, and it is the one that matters most: the fix did not disarm the branch it was added to enforce. Glossary on base, detector blind ⇒ the Runs A and C together are the whole point of the repair: the two states now have separate lines and neither can wear the other's clothes. Round-2 delta — scope confirmed
Nothing outside Step 3c moved in either round: all five hunks of the full PR diff land at Acceptance criteria
Skill rigor
Specialist fan-out (ADR 0079) — no findings
VerdictAll 6 acceptance criteria and all 5 rigor checks pass. Advisory only — this authorizes no merge. §CP requires a Read the PR head (§HEAD): all skill text under review sourced from Verdict-written: 2026-07-26T23:20:05Z |
Handoff — banked, awaiting a control-plane approvalEngine instance State: PASS at head Next step, and the only one: a Do not rebase or push to this branch. Any head movement un-binds the verdict (ADR 0058) and would cost a re-review. Three human approvals were destroyed that way today. What it fixes, for whoever picks it up: the glossary-freshness gate computed Marker note: this PR carries verdict markers at more than one head. That is the documented contract — |
The reviewer's glossary-freshness check used to say the same thing whether a pull request genuinely added no new surface or the checker simply had no idea what a surface looks like in this repository. Those are very different facts, and only one of them is good news. This change makes the checker prove it can see something before it reports "nothing to check" — and when it cannot see anything at all, it now says so out loud and blocks, instead of waving the pull request through.
Fixes #4299
What changed
One file, one step:
claude-plugins/kampus-pipeline/skills/review-code/SKILL.md, Step 3c.DETECTOR_SURFACES_N): directory entries underapps/web/worker/features/, pluspackages/<pkg>/package.jsonfiles. Read-only againstorigin/$BASE_REF, same posture as every other probe in the step (formats §RO).DETECTOR_SURFACES_N > 0— the detector demonstrably works here, so an emptyNEW_SURFACEis a fact about the pull request: the existing not-applicable skip, now also naming the candidate count.DETECTOR_SURFACES_N == 0(with.glossary/TERMS.mdpresent on base) — the emptiness is a fact about the detector. The step emits a distinctdetector blindline and folds an[UNVERIFIABLE]row into the conjunctive table. OneUNVERIFIABLEfails the gate, so the vacuous case now stops the line rather than reading green.git cat-file -e "origin/$BASE_REF:.glossary/TERMS.md"is the verdict chain's firstif, so a repo that never adopted the glossary takes its honest skip and the newdetector blindbranch is structurally unreachable for it — the ordering is enforced by the block, not asserted by the prose around it. The defect surface is exactly the intersection triage identified: glossary present, layout absent.Which fork: the fail-closed one. ADR 0092's bias — "default FAIL, pass on positive evidence of scope" — makes that the floor. Configurable surface detection stays unbuilt; the new line names it as one of the three remedies a blind repo can take.
Replay — the required confirmation (AC 1)
Triage required replaying the Step 3c detection block against a real pull request's file list with the
apps/web/worker/features/andpackages/prefixes altered, before building. Run against PR #2258 (which addedpackages/design-capture/), with the base probes resolved at that pull request's ownbase.sha— the tree the reviewer would have seen at the time:NEW_SURFACEapps/web/worker/features/,packages/)packages/design-captureglossary-freshness: scanned new surfaces => packages/design-capture→FAILsrc/modules/,libs/)glossary-freshness: not applicable — no new feature folder / public package / export in this PRSame pull request, same file list, same diff. Only the layout prefixes changed, and the gate went from a correct FAIL to a clean skip. Prediction confirmed, not falsified — the vacuous pass is real, and its emitted line is byte-identical to a legitimate skip.
The new probe was then exercised on both sides:
origin/main→DETECTOR_SURFACES_N=49→ not-applicable skip branch (AC 4: no new false FAIL here).src/modules,libs) →DETECTOR_SURFACES_N=0→UNVERIFIABLEdetector-blind branch.Repair round 1 — the ordering is now enforced, not asserted
The
review-skillFAIL was right: the graceful-absence probe existed only as a parenthetical inside the prose blockquote that sits after the verdict block, so theelsewas reachable with the glossary absent and emitted a blockingUNVERIFIABLEasserting.glossary/TERMS.md is on origin/$BASE_REF— a fact it never tested. That was inert on base (theelsewas a benign skip) and load-bearing here (the paths now diverge).The chain now leads with the executed probe. Replayed both directions against real trees:
DETECTOR_SURFACES_Norigin/main(glossary present)UNVERIFIABLE — detector blind— the new branch is still reachable when the glossary is on base026dbdf6^(real pre-glossary tree)not applicable — no .glossary/TERMS.md on base— the graceful-absence skip,detector blindunreachableNothing else moved: the
DETECTOR_SURFACES_Nprobe and its numbers, the[UNVERIFIABLE]table row, the count > 0 skip, and the no-fixture decision are all untouched. The diff of this round is 10 insertions / 4 deletions in one file, both hunks inside Step 3c.Verification
pipeline-cli gh-phoenix lint-skillsover the full corpus (the CI invocation shape): clean. Single-file invocation exits 3 on zero gh-call scope both before and after this change, so the corpus-wide run is the meaningful one.pnpm lint:worktree: no biome-handled changed files — clean skip on a markdown-only diff.cp-classify classify: control-plane, exit 0 (path-match onclaude-plugins/**). This is a §CP pull request: blocking, human merge.Deviations
Class: narrowed a suggested scope.
Class: chose between two forks the issue left open.
Class: did not run the full acceptance test.
review-coderun in a repository without the phoenix layout.