diff --git a/engine/hooks/wrong-check-reflect/detect.py b/engine/hooks/wrong-check-reflect/detect.py index 465d1d98..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,6 +123,33 @@ ), ] +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" +) +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 +187,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..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,7 +105,8 @@ 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"], + ["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"],