merge queue: checking #309 on main (bcfc509)

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:

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()

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"],

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"],