hook+gate: catch the shape of a retraction, and stop the vacuous coverage pass - #309
Conversation
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_bd2c2c6b-8bc8-4507-acea-7623e8e30638) |
0061d58 to
0f8259e
Compare
e20c86b to
4be7345
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_1082dad2-fe38-4d9b-8fa5-d92c7d8a44b2) |
0f8259e to
f42cee8
Compare
4be7345 to
ee6b6fa
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_2c829dbb-60a4-41c3-a1db-98863ceacef4) |
f42cee8 to
0a504a2
Compare
ee6b6fa to
fff52cf
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_a2d9965c-b0a0-4c6c-be89-64f3f5659e68) |
0a504a2 to
7bb20aa
Compare
…rage pass
## Context
- Two defects, both found by the same incident this session. A claim went out
("PR #303: full preflight green") that was never actually checked, and the
guard that exists to catch that class stayed silent.
- Defect 1, the vacuous pass: preflight.py ran
check_skill_test_coverage.py with no refs, while `base` was already in scope
two lines above (used for check_no_dated_provenance). With no refs the gate
defaults to origin/main, so for a stacked slice it compares the whole stack
instead of the slice and can print "ok" for a slice it never looked at.
That is how "full preflight green" was reported for a slice that had
changed preflight.py with no colocated test.
- Defect 2, the missed admission: wrong-check-reflect exists precisely to fire
on "a prior check was wrong" and inject /reflect. It stayed silent on
"Also: a claim I made earlier was wrong." The user had to play the hook's
role by hand.
## Considerations
- Defect 1 is fixed categorically, not by a rule asking anyone to remember the
flags: preflight now passes --base <slice base> --head HEAD, so the wider
default is unreachable from the gate runner. Tier 1 of
reflect/references/lenses.md's fix hierarchy rather than tier 4.
- Defect 2's root cause is the detector's shape, not a missing phrase.
ADMISSION_RES enumerates eight sentences someone actually wrote, which
guarantees it lags the ninth -- exactly what
principle-assert-invariants-not-last-bug warns about. Adding
"a claim I made was wrong" as pattern nine would repeat the mistake.
- So the fix matches the SHAPE of a retraction: a first-person marker, a
reference to something already stated, and a wrongness word, within one
~260-char window (a retraction often spans two sentences). The enumerated
list is kept -- it costs nothing and documents real phrasings -- with the
structural check as the fallback.
- NEGATIVE_RES still runs first, so hypotheticals ("if my earlier check was
wrong") and product blame ("the test was wrong") cannot reach the window.
- Deliberately NOT attempted here: enumerating every way a model can admit
fault. That is not enumerable. The judgment half is the next slice, which
makes principle-flag-your-own-corrections auto-fire.
## Blast Radius
- wrong-check-reflect now fires on more shapes, so false positives are the
risk. Guarded by four new negative tests plus the 12 pre-existing ones; the
whole 36-test suite was re-run unchanged before adding any.
- preflight reports failures it previously hid. Expect previously "green"
stacked slices to surface real gaps -- that is the point.
- Modified: engine/hooks/wrong-check-reflect/{detect.py,tests/test_hooks.py},
engine/skills/make-pr/scripts/preflight.py.
- Revertable with git revert.
## Verification
- Structural layer, 9 cases, 0 mismatches, including the 4 real misses and 4
shapes that must stay silent (present-tense product opinion, no
prior-statement marker, hypothetical, unrelated bug report).
- python3 -m unittest discover -s engine/hooks/wrong-check-reflect/tests
-> Ran 36 tests, OK (before adding new tests: no regression)
-> Ran 43 tests, OK (after)
- python3 scripts/check_hook_test_coverage.py engine/hooks/wrong-check-reflect
-> check_hook_test_coverage: OK (1 hook(s) checked)
- Vacuous-pass divergence, same tree, one skill touched with no test change:
check_skill_test_coverage.py (no refs) -> ok
check_skill_test_coverage.py --base <slice> --head HEAD -> fail
The second is what preflight now runs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD
Two gates were red on this branch. check_ecosystem_boundaries.py rejects any engine/ Python that names a corpus/skills or product/skills path, and wrong-check-reflect/detect.py cited two principle skills by full path. check_no_new_comments.py rejected four more comment lines here and in make-pr/scripts/preflight.py. Both fixes are the same move: the prose goes into the module or function docstring, which both gates exempt, and the principle names lose their corpus/skills/ prefix. No behavior changes. The ecosystem failure was invisible from a worktree under .worktrees/ -- that gate skips those paths and prints ok having read no files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013mDsvDgGuwaBuyRtYgvktd
fff52cf to
b47fcd2
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_fba993f7-d1c0-4022-aa97-0e0101d663ba) |
|
Queued — the merge queue status continues in this comment ↓. |
|
@Mergifyio queue |
Merge Queue Status
This pull request spent 3 minutes 42 seconds in the queue, including 3 minutes 10 seconds running CI. Required conditions to merge
|
- Problem: plan-first Step 3 asks which gates a planned change will trip, and
at what scope. Answering that by hand means reading preflight.py's path
table and every gate's argparse, which nobody does -- so the answer arrives
at publish time as a re-split or a false pass.
- make-pr's preflight.py already does the lookup, but only for a diff that
exists. This asks the same question from a list of paths you intend to
touch, so a re-split costs a line edit instead of a rebase cascade.
- Imports preflight's classify/gates_for rather than restating the unit table
(principle-bind-to-named-inventory). Two copies of that table would drift,
and the drift would be silent.
- Derives ref-awareness by reading each gate's own argparse flags, not from a
list kept in this script. A gate that gains --base starts being reported
without an edit here, which is the same reasoning that made the skill
trigger inventory generated rather than hand-written.
- Exit 1 when the plan as stated would be rejected later: mixed units, a
ref-aware gate planned without refs, or a stale base. It never edits
anything, so the exit code is the whole output contract.
- Reports unknowns unconditionally. A plan with no unknowns has not been
examined, and three of them are outside what a path list can answer.
Run against a realistic plan, the lever reported
`check_codify_has_code.py` as UNSCOPED. Reading
scripts/check_codify_has_code.py:91 confirmed `default="origin/main"`, while
preflight.py:93 invoked it with no refs -- the same vacuous-pass class already
fixed for the coverage gate, in a sibling nobody had checked.
Proved on a throwaway two-slice stack (slice A: code only; slice B: prose
only, adding a rule-shaped line):
codify --base origin/main -> exit=0 vacuous: A's code satisfied B's prose
codify --base <slice A> -> exit=1 the actual violation
Fixed in preflight.py, pinned by two colocated tests.
The lever flagged `check_skill_test_coverage` as "may pass vacuously" for
these paths. preflight.py --base plan-first duly reported `ok skill test
coverage`. Run with explicit refs:
check_skill_test_coverage.py --base plan-first --head HEAD
fail engine/skills/make-pr: changed without a corresponding test change
So this slice had changed preflight.py with no colocated test, and the gate
hid it exactly as predicted. The two tests above are that fix. (The coverage
gate's own scope fix is in flight on #309 and deliberately not duplicated
here.)
- New: scripts/plan_preflight.py (read-only; imports detectors, runs git
rev-parse/rev-list, writes nothing), tests/test_plan_preflight.py.
- Modified: engine/skills/make-pr/scripts/preflight.py passes --base to the
codify gate, so it now reports violations it previously hid on stacked
slices. Expect previously-green stacked slices to surface real gaps.
- Revertable with git revert.
- python3 -m unittest tests.test_plan_preflight -> Ran 19 tests, OK
- python3 -m unittest discover -s engine/skills/make-pr/tests -> Ran 16, OK
- python3 engine/skills/make-pr/scripts/preflight.py --base plan-first
-> ok preflight passed (single unit engine-runtime, 7 gates)
- python3 scripts/check_skill_test_coverage.py --base plan-first --head HEAD
-> ok skill test coverage (after the colocated tests; fail before them,
both outputs above)
- Two-slice codify repro: exit=0 at origin/main, exit=1 at the slice base,
both pasted above.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD
- Problem: plan-first Step 3 asks which gates a planned change will trip, and
at what scope. Answering that by hand means reading preflight.py's path
table and every gate's argparse, which nobody does -- so the answer arrives
at publish time as a re-split or a false pass.
- make-pr's preflight.py already does the lookup, but only for a diff that
exists. This asks the same question from a list of paths you intend to
touch, so a re-split costs a line edit instead of a rebase cascade.
- Imports preflight's classify/gates_for rather than restating the unit table
(principle-bind-to-named-inventory). Two copies of that table would drift,
and the drift would be silent.
- Derives ref-awareness by reading each gate's own argparse flags, not from a
list kept in this script. A gate that gains --base starts being reported
without an edit here, which is the same reasoning that made the skill
trigger inventory generated rather than hand-written.
- Exit 1 when the plan as stated would be rejected later: mixed units, a
ref-aware gate planned without refs, or a stale base. It never edits
anything, so the exit code is the whole output contract.
- Reports unknowns unconditionally. A plan with no unknowns has not been
examined, and three of them are outside what a path list can answer.
Run against a realistic plan, the lever reported
`check_codify_has_code.py` as UNSCOPED. Reading
scripts/check_codify_has_code.py:91 confirmed `default="origin/main"`, while
preflight.py:93 invoked it with no refs -- the same vacuous-pass class already
fixed for the coverage gate, in a sibling nobody had checked.
Proved on a throwaway two-slice stack (slice A: code only; slice B: prose
only, adding a rule-shaped line):
codify --base origin/main -> exit=0 vacuous: A's code satisfied B's prose
codify --base <slice A> -> exit=1 the actual violation
Fixed in preflight.py, pinned by two colocated tests.
The lever flagged `check_skill_test_coverage` as "may pass vacuously" for
these paths. preflight.py --base plan-first duly reported `ok skill test
coverage`. Run with explicit refs:
check_skill_test_coverage.py --base plan-first --head HEAD
fail engine/skills/make-pr: changed without a corresponding test change
So this slice had changed preflight.py with no colocated test, and the gate
hid it exactly as predicted. The two tests above are that fix. (The coverage
gate's own scope fix is in flight on #309 and deliberately not duplicated
here.)
- New: scripts/plan_preflight.py (read-only; imports detectors, runs git
rev-parse/rev-list, writes nothing), tests/test_plan_preflight.py.
- Modified: engine/skills/make-pr/scripts/preflight.py passes --base to the
codify gate, so it now reports violations it previously hid on stacked
slices. Expect previously-green stacked slices to surface real gaps.
- Revertable with git revert.
- python3 -m unittest tests.test_plan_preflight -> Ran 19 tests, OK
- python3 -m unittest discover -s engine/skills/make-pr/tests -> Ran 16, OK
- python3 engine/skills/make-pr/scripts/preflight.py --base plan-first
-> ok preflight passed (single unit engine-runtime, 7 gates)
- python3 scripts/check_skill_test_coverage.py --base plan-first --head HEAD
-> ok skill test coverage (after the colocated tests; fail before them,
both outputs above)
- Two-slice codify repro: exit=0 at origin/main, exit=1 at the slice base,
both pasted above.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD
- Problem: plan-first Step 3 asks which gates a planned change will trip, and
at what scope. Answering that by hand means reading preflight.py's path
table and every gate's argparse, which nobody does -- so the answer arrives
at publish time as a re-split or a false pass.
- make-pr's preflight.py already does the lookup, but only for a diff that
exists. This asks the same question from a list of paths you intend to
touch, so a re-split costs a line edit instead of a rebase cascade.
- Imports preflight's classify/gates_for rather than restating the unit table
(principle-bind-to-named-inventory). Two copies of that table would drift,
and the drift would be silent.
- Derives ref-awareness by reading each gate's own argparse flags, not from a
list kept in this script. A gate that gains --base starts being reported
without an edit here, which is the same reasoning that made the skill
trigger inventory generated rather than hand-written.
- Exit 1 when the plan as stated would be rejected later: mixed units, a
ref-aware gate planned without refs, or a stale base. It never edits
anything, so the exit code is the whole output contract.
- Reports unknowns unconditionally. A plan with no unknowns has not been
examined, and three of them are outside what a path list can answer.
Run against a realistic plan, the lever reported
`check_codify_has_code.py` as UNSCOPED. Reading
scripts/check_codify_has_code.py:91 confirmed `default="origin/main"`, while
preflight.py:93 invoked it with no refs -- the same vacuous-pass class already
fixed for the coverage gate, in a sibling nobody had checked.
Proved on a throwaway two-slice stack (slice A: code only; slice B: prose
only, adding a rule-shaped line):
codify --base origin/main -> exit=0 vacuous: A's code satisfied B's prose
codify --base <slice A> -> exit=1 the actual violation
Fixed in preflight.py, pinned by two colocated tests.
The lever flagged `check_skill_test_coverage` as "may pass vacuously" for
these paths. preflight.py --base plan-first duly reported `ok skill test
coverage`. Run with explicit refs:
check_skill_test_coverage.py --base plan-first --head HEAD
fail engine/skills/make-pr: changed without a corresponding test change
So this slice had changed preflight.py with no colocated test, and the gate
hid it exactly as predicted. The two tests above are that fix. (The coverage
gate's own scope fix is in flight on #309 and deliberately not duplicated
here.)
- New: scripts/plan_preflight.py (read-only; imports detectors, runs git
rev-parse/rev-list, writes nothing), tests/test_plan_preflight.py.
- Modified: engine/skills/make-pr/scripts/preflight.py passes --base to the
codify gate, so it now reports violations it previously hid on stacked
slices. Expect previously-green stacked slices to surface real gaps.
- Revertable with git revert.
- python3 -m unittest tests.test_plan_preflight -> Ran 19 tests, OK
- python3 -m unittest discover -s engine/skills/make-pr/tests -> Ran 16, OK
- python3 engine/skills/make-pr/scripts/preflight.py --base plan-first
-> ok preflight passed (single unit engine-runtime, 7 gates)
- python3 scripts/check_skill_test_coverage.py --base plan-first --head HEAD
-> ok skill test coverage (after the colocated tests; fail before them,
both outputs above)
- Two-slice codify repro: exit=0 at origin/main, exit=1 at the slice base,
both pasted above.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD
* lever: answer plan-first's gate question before the paths exist
- Problem: plan-first Step 3 asks which gates a planned change will trip, and
at what scope. Answering that by hand means reading preflight.py's path
table and every gate's argparse, which nobody does -- so the answer arrives
at publish time as a re-split or a false pass.
- make-pr's preflight.py already does the lookup, but only for a diff that
exists. This asks the same question from a list of paths you intend to
touch, so a re-split costs a line edit instead of a rebase cascade.
- Imports preflight's classify/gates_for rather than restating the unit table
(principle-bind-to-named-inventory). Two copies of that table would drift,
and the drift would be silent.
- Derives ref-awareness by reading each gate's own argparse flags, not from a
list kept in this script. A gate that gains --base starts being reported
without an edit here, which is the same reasoning that made the skill
trigger inventory generated rather than hand-written.
- Exit 1 when the plan as stated would be rejected later: mixed units, a
ref-aware gate planned without refs, or a stale base. It never edits
anything, so the exit code is the whole output contract.
- Reports unknowns unconditionally. A plan with no unknowns has not been
examined, and three of them are outside what a path list can answer.
Run against a realistic plan, the lever reported
`check_codify_has_code.py` as UNSCOPED. Reading
scripts/check_codify_has_code.py:91 confirmed `default="origin/main"`, while
preflight.py:93 invoked it with no refs -- the same vacuous-pass class already
fixed for the coverage gate, in a sibling nobody had checked.
Proved on a throwaway two-slice stack (slice A: code only; slice B: prose
only, adding a rule-shaped line):
codify --base origin/main -> exit=0 vacuous: A's code satisfied B's prose
codify --base <slice A> -> exit=1 the actual violation
Fixed in preflight.py, pinned by two colocated tests.
The lever flagged `check_skill_test_coverage` as "may pass vacuously" for
these paths. preflight.py --base plan-first duly reported `ok skill test
coverage`. Run with explicit refs:
check_skill_test_coverage.py --base plan-first --head HEAD
fail engine/skills/make-pr: changed without a corresponding test change
So this slice had changed preflight.py with no colocated test, and the gate
hid it exactly as predicted. The two tests above are that fix. (The coverage
gate's own scope fix is in flight on #309 and deliberately not duplicated
here.)
- New: scripts/plan_preflight.py (read-only; imports detectors, runs git
rev-parse/rev-list, writes nothing), tests/test_plan_preflight.py.
- Modified: engine/skills/make-pr/scripts/preflight.py passes --base to the
codify gate, so it now reports violations it previously hid on stacked
slices. Expect previously-green stacked slices to surface real gaps.
- Revertable with git revert.
- python3 -m unittest tests.test_plan_preflight -> Ran 19 tests, OK
- python3 -m unittest discover -s engine/skills/make-pr/tests -> Ran 16, OK
- python3 engine/skills/make-pr/scripts/preflight.py --base plan-first
-> ok preflight passed (single unit engine-runtime, 7 gates)
- python3 scripts/check_skill_test_coverage.py --base plan-first --head HEAD
-> ok skill test coverage (after the colocated tests; fail before them,
both outputs above)
- Two-slice codify repro: exit=0 at origin/main, exit=1 at the slice base,
both pasted above.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD
* no-comments: carry the diff-aware gate notes in docstrings
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013sYZswUQ3HEnUEdVWrUbvq
Change-Id: Ib96cec6fd81eb67ece42e6d9dfa54545dd1f0c5a
---------
Co-authored-by: Edbert Chan <chanedbert@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
Two defects from one incident. A claim went out that a stacked slice was "fully green," and the guard that exists to catch that class stayed silent.
The green was vacuous:
preflight.pyran the coverage gate with no refs, while the slice base was already in scope two lines above. With no refs the gate defaults to trunk, so a stacked slice gets compared against the whole stack. It now passes the slice refs, making the wide default unreachable.The missed admission was a shape problem, not a missing phrase. The detector enumerated eight sentences someone had actually written, which guarantees it lags the ninth.
Review Claim
A verification gate must be invoked at the scope of the thing it verifies, and a retraction detector must match the shape of a retraction rather than a list of phrasings.
Review Lane
behaviorReview Unit
engine-runtimeSafety Invariant
Both changes surface more, never less.
preflight.pyreports failures it previously hid — expect previously "green" stacked slices to show real gaps, which is the point. The widened detector risks false positives, guarded by 11 negative tests;NEGATIVE_RESstill runs first, so hypotheticals and product blame cannot reach the structural window. The full 36-test suite was re-run unchanged before any new test was added.Slice Rationale
Both defects are
engine-runtimeand both came from the same incident, so they share a slice: the vacuous pass is how the wrong claim got made, and the missed admission is why nothing caught it. Splitting them would give a reviewer half a causal chain.The judgment half — "any admission of fault in any wording is the trigger" — is deliberately not here. It is not enumerable, so it lands in the next slice as prose that auto-fires.
Non-goals
Test Plan
Test Plan
Structural layer, 9 cases, 0 mismatches — the 4 real misses plus 4 shapes that must stay silent (present-tense product opinion, no prior-statement marker, hypothetical, unrelated bug report).
python3 -m unittest discover -s engine/hooks/wrong-check-reflect/tests→Ran 36 tests ... OKbefore new tests (no regression),Ran 43 tests ... OKafterpython3 scripts/check_hook_test_coverage.py engine/hooks/wrong-check-reflect→OK (1 hook(s) checked)python3 -m unittest discover -s engine/skills/make-pr/tests→Ran 17 tests ... OKVacuous-pass divergence, same tree, one skill touched with no test change:
The second is what
preflight.pynow runs. The fixed gate immediately caught this slice itself — it changedpreflight.pywith no colocated test — which is the two newtest_coverage_gate_*cases.Revert Plan
Revert Plan
git revert <sha>./install.shfrom the main checkout to restore the prior hook🤖 Generated with Claude Code
https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD
Note
Medium Risk
Broader admission matching may increase hook false positives (mitigated by negatives-first ordering and new tests); preflight may fail stacked slices that previously got a vacuous green coverage pass.
Overview
Fixes two linked runtime gaps from one incident: a stacked slice could read “fully green” while the wrong-check hook stayed silent on a real admission.
wrong-check-reflect keeps the existing enumerated admission regexes and NEGATIVE_RES guards, but
find_admissionnow falls back tostructural_admission: around each wrongness token it requires a first-person marker and a prior-statement marker withinWINDOW_BEFORE/WINDOW_AFTER, so phrasings like “a claim I made earlier was wrong” or two-sentence “I told you X … that run was vacuous” trip the hook without adding every wording to the list.make-pr preflight now invokes
check_skill_test_coverage.pywith--baseand--head HEADwhenever a real git base is in scope (same pattern as the dated-provenance gate), so skill slices are checked against the actual diff instead of defaulting to trunk and passing vacuously.--pathsmode still omits those flags.New unit tests cover the structural hits/negatives and the coverage gate command shape.
Reviewed by Cursor Bugbot for commit b47fcd2. Bugbot is set up for automated code reviews on this repo. Configure here.