From a9148952a703c58a1f630ced0f90e08b728a373f Mon Sep 17 00:00:00 2001 From: Edbert Chan Date: Wed, 9 Sep 2026 12:55:08 -0700 Subject: [PATCH 1/2] hook+gate: catch the shape of a retraction, and stop the vacuous coverage 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 --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 --head HEAD -> fail The second is what preflight now runs. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JQMWSLRArEfEm1psa7RKdD --- engine/hooks/wrong-check-reflect/detect.py | 43 ++++++++++++++++++- .../wrong-check-reflect/tests/test_hooks.py | 40 +++++++++++++++++ engine/skills/make-pr/scripts/preflight.py | 5 ++- engine/skills/make-pr/tests/test_preflight.py | 20 +++++++++ 4 files changed, 106 insertions(+), 2 deletions(-) diff --git a/engine/hooks/wrong-check-reflect/detect.py b/engine/hooks/wrong-check-reflect/detect.py index 465d1d98..8464ed27 100644 --- a/engine/hooks/wrong-check-reflect/detect.py +++ b/engine/hooks/wrong-check-reflect/detect.py @@ -107,6 +107,47 @@ ), ] +# --- Structural layer ------------------------------------------------------- +# The list above enumerates sentences someone actually wrote, which means it +# always lags the next phrasing. It missed "a claim I made earlier was wrong" +# and "I told you X ... that run was vacuous" -- the admission that prompted +# this layer. So also match the SHAPE of a retraction rather than its wording: +# a first-person marker, a reference to something already stated, and a +# wrongness word, inside one window. See +# corpus/skills/principle-assert-invariants-not-last-bug. +# +# This stays a shape matcher. The judgment half -- "any admission of fault, in +# any wording, is the trigger" -- cannot be enumerated and lives in +# corpus/skills/principle-flag-your-own-corrections, which auto-fires. +FIRST_PERSON_RE = re.compile(r"(?i)\b(?:i|i'?m|i'?ve|i'?d|my|mine)\b") +PRIOR_STATEMENT_RE = re.compile( + r"(?i)\b(?:earlier|previously|prior|before|already|above|last\s+turn|" + r"told\s+you|said|stated|reported|claimed|cited|wrote|answered|called\s+it|" + r"claim|check|citation|statement|answer|assessment|verdict|summary|report|" + r"read|grep|assumption|number|count)\b" +) +WRONGNESS_RE = re.compile( + r"(?i)\b(?:wrong|incorrect|inaccurate|false|untrue|not\s+true|mistaken|" + r"misread|mis-read|misstated|overstated|vacuous|premature|bogus|" + r"retract(?:ing|ed)?|take\s+(?:that|it)\s+back|" + r"does(?:n'?t|\s+not)\s+hold|did(?:n'?t|\s+not)\s+hold)\b" +) +# How far from the wrongness word the other two markers may sit. A retraction +# often spans two sentences ("I told you X. That was vacuous."). +WINDOW_BEFORE = 260 +WINDOW_AFTER = 140 + + +def structural_admission(cleaned: str) -> str | None: + """Match the shape of a first-person retraction, not a fixed phrasing.""" + for hit in WRONGNESS_RE.finditer(cleaned): + start = max(0, hit.start() - WINDOW_BEFORE) + window = cleaned[start:hit.end() + WINDOW_AFTER] + if FIRST_PERSON_RE.search(window) and PRIOR_STATEMENT_RE.search(window): + return hit.group(0) + return None + + FOLLOWUP = ( "Wrong-check admission on this transcript ({match}). This is a FAILURE, " "not a preference ping: a claim went out before a real check. Finish the " @@ -144,7 +185,7 @@ def find_admission(text: str) -> str | None: match = pattern.search(cleaned) if match: return match.group(0) - return None + return structural_admission(cleaned) def _state_file(transcript_path: str) -> str: diff --git a/engine/hooks/wrong-check-reflect/tests/test_hooks.py b/engine/hooks/wrong-check-reflect/tests/test_hooks.py index d6bf2dbd..d83f9065 100644 --- a/engine/hooks/wrong-check-reflect/tests/test_hooks.py +++ b/engine/hooks/wrong-check-reflect/tests/test_hooks.py @@ -251,6 +251,46 @@ def test_no_hit_hypothetical_reversed_word_order(self): ) +class TestStructuralAdmission(unittest.TestCase): + """The enumerated list always lags the next phrasing. These are the real + admissions it missed, which is why structural_admission() exists.""" + + def test_hit_a_claim_i_made_earlier_was_wrong(self): + """The live miss: the user had to play the hook's role manually.""" + self.assertIsNotNone(detect.find_admission("Also: a claim I made earlier was wrong.")) + + def test_hit_a_claim_i_made_was_wrong_no_time_word(self): + self.assertIsNotNone(detect.find_admission("A claim I made was wrong.")) + + def test_hit_retraction_spanning_two_sentences(self): + self.assertIsNotNone( + detect.find_admission( + "I told you PR #303 was full preflight green. That coverage " + "run was vacuous." + ) + ) + + def test_hit_explicit_retraction_verb(self): + self.assertIsNotNone( + detect.find_admission("Earlier I said the suite passed; I am retracting that.") + ) + + def test_no_hit_present_tense_opinion_about_product(self): + """"I think the UI is wrong" is not a retraction of anything stated.""" + self.assertIsNone( + detect.find_admission("I think the UI is wrong here, want me to restyle it?") + ) + + def test_no_hit_wrongness_without_a_prior_statement_marker(self): + self.assertIsNone( + detect.find_admission("The export writes duplicate rows after a retry.") + ) + + def test_no_hit_hypothetical_keeps_precedence_over_structure(self): + """NEGATIVE_RES runs first, so a conditional cannot reach the window.""" + self.assertIsNone(detect.find_admission("If my earlier check was wrong we should redo it.")) + + class TestDecideOnce(unittest.TestCase): def setUp(self): self.tmp = tempfile.TemporaryDirectory() diff --git a/engine/skills/make-pr/scripts/preflight.py b/engine/skills/make-pr/scripts/preflight.py index 3dfc8266..c45e90cf 100644 --- a/engine/skills/make-pr/scripts/preflight.py +++ b/engine/skills/make-pr/scripts/preflight.py @@ -100,7 +100,10 @@ def gates_for(paths: list[str], base: str | None = None) -> list[list[str]]: ["python3", "scripts/check_skills_three_harnesses.py"], ["python3", "scripts/check_ecosystem_boundaries.py"], ["python3", "scripts/check_skill_file_refs.py"], - ["python3", "scripts/check_skill_test_coverage.py"], + # Diff-aware: without the slice refs it defaults to origin/main and + # can report ok for a slice it never compared (a vacuous pass). + ["python3", "scripts/check_skill_test_coverage.py"] + + (["--base", base, "--head", "HEAD"] if base is not None else []), ["python3", "scripts/check_skill_trigger_mechanism.py"], ["python3", "scripts/check_skill_trigger_policy.py"], ["python3", "scripts/check_subagent_scope_contract.py"], diff --git a/engine/skills/make-pr/tests/test_preflight.py b/engine/skills/make-pr/tests/test_preflight.py index a02e8876..8506b13a 100644 --- a/engine/skills/make-pr/tests/test_preflight.py +++ b/engine/skills/make-pr/tests/test_preflight.py @@ -84,6 +84,26 @@ def test_gates_for_skill_slice_run_the_scenario_suite(self): cmds = pf.gates_for(["product/skills/how/SKILL.md"]) self.assertIn(["python3", "scripts/run_skill_scenarios.py"], cmds) + def test_coverage_gate_carries_the_slice_refs(self): + """Without refs the coverage gate defaults to origin/main and can print + ok for a stacked slice it never compared -- a vacuous pass. This + reported a stacked slice as fully green in a real session.""" + cmds = pf.gates_for(["product/skills/how/SKILL.md"], base="origin/main") + coverage = [c for c in cmds if "check_skill_test_coverage.py" in " ".join(c)] + self.assertEqual(len(coverage), 1, cmds) + self.assertEqual( + coverage[0], + ["python3", "scripts/check_skill_test_coverage.py", + "--base", "origin/main", "--head", "HEAD"], + ) + + def test_coverage_gate_omits_refs_when_there_is_no_base(self): + """Under --paths there is no real git ref, so the flags must be absent + rather than passed as the string 'None'.""" + cmds = pf.gates_for(["product/skills/how/SKILL.md"], base=None) + coverage = [c for c in cmds if "check_skill_test_coverage.py" in " ".join(c)] + self.assertEqual(coverage, [["python3", "scripts/check_skill_test_coverage.py"]]) + def test_gates_for_rule_prose_with_base_includes_dated_provenance_check(self): self.assertIn( ["python3", "scripts/check_no_dated_provenance.py", "--base", "origin/main"], From b47fcd20cf4a07d023a2c16c7e0674e8a1a7c27e Mon Sep 17 00:00:00 2001 From: Edbert Chan Date: Wed, 9 Sep 2026 20:43:55 -0700 Subject: [PATCH 2/2] fix CI: keep engine Python off corpus/ paths and out of comments 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) Claude-Session: https://claude.ai/code/session_013mDsvDgGuwaBuyRtYgvktd --- engine/hooks/wrong-check-reflect/detect.py | 30 ++++++++++++---------- engine/skills/make-pr/scripts/preflight.py | 9 ++++--- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/engine/hooks/wrong-check-reflect/detect.py b/engine/hooks/wrong-check-reflect/detect.py index 8464ed27..0c00ed54 100644 --- a/engine/hooks/wrong-check-reflect/detect.py +++ b/engine/hooks/wrong-check-reflect/detect.py @@ -9,6 +9,22 @@ Assistant text only. Fail-open: parse/IO errors mean no hit. Once per transcript. Skip if the user already asked /reflect. + +ADMISSION_RES enumerates sentences someone actually wrote, so it always lags +the next phrasing: it missed "a claim I made earlier was wrong" and "I told +you X ... that run was vacuous", the admission that prompted the structural +layer below. FIRST_PERSON_RE / PRIOR_STATEMENT_RE / WRONGNESS_RE therefore +match the SHAPE of a retraction rather than its wording -- a first-person +marker, a reference to something already stated, and a wrongness word inside +one window. That instantiates principle-assert-invariants-not-last-bug. + +It stays a shape matcher. The judgment half -- "any admission of fault, in any +wording, is the trigger" -- cannot be enumerated and lives in the +principle-flag-your-own-corrections skill, which auto-fires. + +WINDOW_BEFORE / WINDOW_AFTER are how far from the wrongness word the other two +markers may sit; a retraction often spans two sentences ("I told you X. That +was vacuous."). """ from __future__ import annotations @@ -107,18 +123,6 @@ ), ] -# --- Structural layer ------------------------------------------------------- -# The list above enumerates sentences someone actually wrote, which means it -# always lags the next phrasing. It missed "a claim I made earlier was wrong" -# and "I told you X ... that run was vacuous" -- the admission that prompted -# this layer. So also match the SHAPE of a retraction rather than its wording: -# a first-person marker, a reference to something already stated, and a -# wrongness word, inside one window. See -# corpus/skills/principle-assert-invariants-not-last-bug. -# -# This stays a shape matcher. The judgment half -- "any admission of fault, in -# any wording, is the trigger" -- cannot be enumerated and lives in -# corpus/skills/principle-flag-your-own-corrections, which auto-fires. FIRST_PERSON_RE = re.compile(r"(?i)\b(?:i|i'?m|i'?ve|i'?d|my|mine)\b") PRIOR_STATEMENT_RE = re.compile( r"(?i)\b(?:earlier|previously|prior|before|already|above|last\s+turn|" @@ -132,8 +136,6 @@ r"retract(?:ing|ed)?|take\s+(?:that|it)\s+back|" r"does(?:n'?t|\s+not)\s+hold|did(?:n'?t|\s+not)\s+hold)\b" ) -# How far from the wrongness word the other two markers may sit. A retraction -# often spans two sentences ("I told you X. That was vacuous."). WINDOW_BEFORE = 260 WINDOW_AFTER = 140 diff --git a/engine/skills/make-pr/scripts/preflight.py b/engine/skills/make-pr/scripts/preflight.py index c45e90cf..b659e488 100644 --- a/engine/skills/make-pr/scripts/preflight.py +++ b/engine/skills/make-pr/scripts/preflight.py @@ -85,7 +85,12 @@ def touches_rule_prose(paths: list[str]) -> bool: def gates_for(paths: list[str], base: str | None = None) -> list[list[str]]: """Commands to run, in order. Paths are repo-relative. `base` is the real git ref being diffed against; omit it (e.g. under --paths) to skip gates - that need actual git history.""" + that need actual git history. + + check_skill_test_coverage.py is diff-aware: without the slice refs it + defaults to origin/main and can report ok for a slice it never compared, + which is a vacuous pass. So it gets --base/--head whenever `base` is real. + """ cmds: list[list[str]] = [] if touches_rule_prose(paths): # thrash-reflect-automate: a codified invariant needs code enforcing it. @@ -100,8 +105,6 @@ def gates_for(paths: list[str], base: str | None = None) -> list[list[str]]: ["python3", "scripts/check_skills_three_harnesses.py"], ["python3", "scripts/check_ecosystem_boundaries.py"], ["python3", "scripts/check_skill_file_refs.py"], - # Diff-aware: without the slice refs it defaults to origin/main and - # can report ok for a slice it never compared (a vacuous pass). ["python3", "scripts/check_skill_test_coverage.py"] + (["--base", base, "--head", "HEAD"] if base is not None else []), ["python3", "scripts/check_skill_trigger_mechanism.py"],