merge queue: checking #309 on main (bcfc509) - #342
Closed
mergify[bot] wants to merge 3 commits into
Closed
Conversation
…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
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎉 This pull request has been checked successfully and will be merged soon. 🎉
#309 is queued for merge on branch main (bcfc509).
This pull request has been created by Mergify to check the mergeability of #309.
You don't need to do anything. Mergify will close this pull request automatically when it is complete.
Required conditions of queue rule
admin-bypassfor merge:check-success = lintcheck-success = testRequired conditions to stay in the queue:
-draftbase=mainlabel=admin-bypass