diff --git a/engine/hooks/llm-judge/phrases/wrong-check-reflect.json b/engine/hooks/llm-judge/phrases/wrong-check-reflect.json new file mode 100644 index 0000000..a9932a1 --- /dev/null +++ b/engine/hooks/llm-judge/phrases/wrong-check-reflect.json @@ -0,0 +1,22 @@ +{ + "checker": "wrong-check-reflect", + "reads": "reply", + "meaning": "The latest assistant reply admits that something it told the user earlier was wrong, misread, or answered the wrong question.", + "match": [ + "my earlier check was wrong", + "You're right, I misread that", + "I incorrectly assumed that file was unused", + "my mistake", + "I was wrong about the path", + "Correction: the file I pointed you to earlier is not the one in use; the real one is src/b.py.", + "Good catch. The earlier number was off; the real count is 12." + ], + "not_match": [ + "You're right. Let's go with option B.", + "I double-checked my earlier count and it holds; nothing in it was wrong.", + "If my earlier check was wrong, say so.", + "The test was wrong, not the code.", + "He said I was wrong about the timeout." + ], + "on_hit": "Wrong-check admission on this transcript. This is a FAILURE, not a preference ping: a claim went out before a real check. Finish the live correction first. Then read the reflect skill and spawn a subagent for steps 1-4 on this exact transcript. Present Accepted / Backlog / Route-to-automate-me / Rejected. Do not skip because the task also finished." +} diff --git a/engine/hooks/wrong-check-reflect/README.md b/engine/hooks/wrong-check-reflect/README.md index d72483f..705f0aa 100644 --- a/engine/hooks/wrong-check-reflect/README.md +++ b/engine/hooks/wrong-check-reflect/README.md @@ -1,56 +1,42 @@ # wrong-check-reflect -When the assistant admits a prior check/claim was wrong -("Good catch — my earlier check was wrong", "You're right, I misread the file", -"I incorrectly assumed…", "the file I cited was a duplicate", "My mistake — I -misread it", "I misread the front matter on that skill"), inject a -`/reflect` follow-up. - -A reply that opens with a standalone "You're right." (or "You are right —") -counts: it concedes that the user caught something the agent's own checks -did not. "You're right that option B is cheaper" is agreement with a claim -and stays silent. "I misread / misunderstood / mixed up" counts with any -object ("I misread which diff you meant"), not only it/that/the. - -A bare "I was wrong" counts, with no named check after it. The retraction -that follows a false claim is often the shortest sentence in the turn, and -requiring it to name the check it retracts let the plainest concession -through. The hypothetical ("if I was wrong about this…"), reported-speech -("the reviewer said I was wrong"), product-blame ("the test was wrong"), -third-person, quote, backtick and fence guards all still hold, so only an -admission asserted in the agent's own voice fires. Finish the live correction first. Fail-open. +When the assistant takes back an earlier check or claim, inject a `/reflect` +follow-up on a later turn. + +The hook does not decide that from local wording rules. On every Stop it hands +the last assistant reply to the background judge using +[`engine/hooks/llm-judge/phrases/wrong-check-reflect.json`](../llm-judge/phrases/wrong-check-reflect.json). +A hit arrives on a later turn through the shared [`llm-judge`](../llm-judge/README.md) +inbox and carries the dictionary's `on_hit` text. The live reply is never held +up. If the judge result was unchecked, the inbox reports "could not judge" +instead of treating the reply as clean. Finish the live correction first. +Fail-open. + Once per transcript. Skip if the user already said `/reflect`. Not word-count (`diu-stop`). Not token_audit thrash (`reflect-on-thrash`). -Assistant text only — user messages and fenced code stay silent. +Assistant text only - user messages and fenced code stay silent. ## Model-judged path -The regexes keep missing new wordings. So when they stay silent, the hook -also asks a small model, through the shared [`llm-judge`](../llm-judge/README.md): -did the user push back, and did the reply take something back? - -`enqueue_judge` in `detect.py` reads the transcript and takes three messages: -the current reply, the user message before it, and the assistant message before -that. Each is cut to its last 4000 characters and put under the labels -`EARLIER ASSISTANT`, `USER` and `ASSISTANT` in a prompt that asks for one line -of JSON: `pushback`, `self_correction`, and a `quote`. It is a hit only when -both `pushback` and `self_correction` are `true`. +`enqueue_judge` in `detect.py` reads the transcript, takes the current reply, +builds a phrase-dictionary job, and sends it to `llm-judge`. The dictionary +defines the meaning with `match` and `not_match` examples and supplies the +static `on_hit` follow-up text. -No job is sent when `stop_hook_active` is set, when the regex already fired, -when this transcript was already prompted, or when any of the three messages is -missing (for example, on the first user message, or when the payload names no -transcript). Inside a judge run -(`CATSTACK_LLM_JUDGE_CHILD=1`) `llm-judge` refuses the job. +No job is sent when `stop_hook_active` is set, when this transcript or reply +was already prompted, when the reply is empty, or when the user already asked +for `/reflect`. Inside a judge run (`CATSTACK_LLM_JUDGE_CHILD=1`) `llm-judge` +refuses the job. The model call runs in a detached background process, so the reply is never held up. Runners are tried in `llm-judge` order: `codex` (gpt-5.3-codex-spark), then `claude` (haiku, hooks off), then `cursor-agent`, first answer wins. The verdict reports one turn later. On the next prompt the `llm-judge` inbox -shows a hit as the same reflect follow-up, with `model judge` as the match. If -no runner could answer, the inbox says so instead of staying quiet. A clean -verdict shows nothing. +shows a hit as the dictionary's `on_hit` text. If no runner could answer, or +the result could not be checked, the inbox says "could not judge" instead of +staying quiet. A clean verdict shows nothing. `llm-judge` is loaded from the sibling folder (`../llm-judge/judge.py`), which sits next to this one in the repo and in each harness's `hooks/` folder. If it @@ -58,9 +44,13 @@ cannot be loaded, or the transcript cannot be read, the hook writes `wrong-check-reflect: judge enqueue failed: ` to stderr and its exit status and output stay the same. +To grow coverage, add the real text of any miss to the dictionary's `match` +phrases, or the real text of any false alarm to `not_match`. Do not add a +pattern to this hook; the prose meaning belongs in the phrase dictionary. + ## Files -- `detect.py` — shared admission regex + once-per-transcript state +- `detect.py` - judge enqueue + once-per-transcript state - `claude_stop_check.py` — Claude `Stop` (stderr + exit 2) - `cursor_session.py` — Cursor `stop` / `sessionEnd` (`followup_message`) - `codex_notify.py` — Codex `notify` (advisory print + chain) diff --git a/engine/hooks/wrong-check-reflect/claude_stop_check.py b/engine/hooks/wrong-check-reflect/claude_stop_check.py index 14a7de9..f83368b 100644 --- a/engine/hooks/wrong-check-reflect/claude_stop_check.py +++ b/engine/hooks/wrong-check-reflect/claude_stop_check.py @@ -1,16 +1,11 @@ #!/usr/bin/env python3 -"""Claude Code Stop hook: inject reflect on first-person wrong-check admission. - -Exit 2 with the reflect prompt when the last assistant message admits a prior -check/claim was wrong. Fail-open. When the regex stays silent, ask the -background llm-judge instead; its verdict arrives on the next prompt. -""" +"""Claude Code Stop hook for wrong-check-reflect.""" from __future__ import annotations import json import sys -from detect import decide, try_enqueue_judge +from detect import try_enqueue_judge def main() -> None: @@ -19,14 +14,7 @@ def main() -> None: except (json.JSONDecodeError, OSError): return payload = payload if isinstance(payload, dict) else {} - try: - message = decide(payload) - except Exception: - return - try_enqueue_judge(payload, bool(message)) - if message: - sys.stderr.write(message + "\n") - sys.exit(2) + try_enqueue_judge(payload) if __name__ == "__main__": diff --git a/engine/hooks/wrong-check-reflect/codex_notify.py b/engine/hooks/wrong-check-reflect/codex_notify.py index 474a175..366b53c 100644 --- a/engine/hooks/wrong-check-reflect/codex_notify.py +++ b/engine/hooks/wrong-check-reflect/codex_notify.py @@ -1,19 +1,12 @@ #!/usr/bin/env python3 -"""Codex `notify` hook: advisory wrong-check admission heads-up. - -Codex fires notify after the turn is over — no way to block or force a -rewrite. Print a heads-up; chain to any prior notify command. When the regex -stays silent and the payload names a transcript, ask the background llm-judge. - - notify = ["python3", "/path/to/codex_notify.py", "/path/to/old-notify", ...] -""" +"""Codex notify hook for wrong-check-reflect.""" from __future__ import annotations import json import subprocess import sys -from detect import CODEX_ADVISORY, find_admission, try_enqueue_judge +from detect import try_enqueue_judge def main() -> None: @@ -36,11 +29,7 @@ def main() -> None: if payload.get("type") != "agent-turn-complete": return - message = payload.get("last-assistant-message") or "" - match = find_admission(message) - try_enqueue_judge(payload, bool(match)) - if match: - print(CODEX_ADVISORY.format(match=match), file=sys.stderr) + try_enqueue_judge(payload) if __name__ == "__main__": diff --git a/engine/hooks/wrong-check-reflect/cursor_session.py b/engine/hooks/wrong-check-reflect/cursor_session.py index a8b4515..8c57541 100644 --- a/engine/hooks/wrong-check-reflect/cursor_session.py +++ b/engine/hooks/wrong-check-reflect/cursor_session.py @@ -1,17 +1,11 @@ #!/usr/bin/env python3 -"""Cursor stop / sessionEnd for wrong-check-reflect. - -`stop` delivers followup_message when the last assistant message admits a -prior check was wrong. `sessionEnd` stays silent if already prompted. -Fail-open. When the regex stays silent, ask the background llm-judge instead; -its verdict arrives on the next turn. -""" +"""Cursor stop / sessionEnd hook for wrong-check-reflect.""" from __future__ import annotations import json import sys -from detect import decide, try_enqueue_judge +from detect import try_enqueue_judge def main() -> None: @@ -21,13 +15,8 @@ def main() -> None: print(json.dumps({"followup_message": ""})) return payload = payload if isinstance(payload, dict) else {} - try: - message = decide(payload) - except Exception: - print(json.dumps({"followup_message": ""})) - return - try_enqueue_judge(payload, bool(message)) - print(json.dumps({"followup_message": message or ""})) + try_enqueue_judge(payload) + print(json.dumps({"followup_message": ""})) if __name__ == "__main__": diff --git a/engine/hooks/wrong-check-reflect/detect.py b/engine/hooks/wrong-check-reflect/detect.py index 399e9ad..fd2b7d2 100644 --- a/engine/hooks/wrong-check-reflect/detect.py +++ b/engine/hooks/wrong-check-reflect/detect.py @@ -1,31 +1,3 @@ -"""Detect first-person “my earlier check was wrong” admissions. - -A bare “I was wrong” counts. The retraction that follows a false claim is -often the shortest sentence in the turn, and the earlier requirement that it -name the check it retracts let the plainest concession through. The -hypothetical, product-blame, reported-speech, quote and fence guards below -still hold, so only a first-person admission asserted in the agent's own -voice fires. - -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 import functools @@ -34,12 +6,12 @@ import json import os import re -import sys import uuid -from typing import Iterable HOOKS_DIR = os.path.dirname(os.path.abspath(__file__)) -LLM_JUDGE_PATH = os.path.join(os.path.dirname(HOOKS_DIR), "llm-judge", "judge.py") +LLM_JUDGE_DIR = os.path.join(os.path.dirname(HOOKS_DIR), "llm-judge") +LLM_JUDGE_PATH = os.path.join(LLM_JUDGE_DIR, "judge.py") +PHRASES_PATH = os.path.join(LLM_JUDGE_DIR, "phrases.py") STATE_DIR = os.environ.get( "WRONG_CHECK_REFLECT_STATE_DIR", @@ -49,160 +21,14 @@ ALREADY_REFLECT_RE = re.compile(r"(?i)\b/?reflect\b|\b/?automate-me\b|\bautomate me\b") META_USER_PREFIXES = ("-]*(?:you[’']?re|you\s+are)\s+right[*_]*\s*[.!:—–]" - ), - re.compile( - r"(?i)\byour\s+(?:instinct|hunch|gut|suspicion|read)\s+(?:was|were)\s+right\b" - ), - re.compile( - r"(?i)^\s*[*_#\s>-]*(?:you'?re\s+right|you\s+are\s+right|good\s+catch)\b" - r".{0,200}?(?:verifying\s+(?:it\s+|that\s+)?now|checking\s+(?:it\s+|that\s+)?now|" - r"i\s+hadn'?t\b|i\s+had\s+not\b|i\s+didn'?t\b|i\s+did\s+not\b|" - r"i\s+should\s+have\b|instead\s+of\s+(?:labeling|labelling|assuming|guessing)|" - r"i\s+never\s+(?:ran|checked|read|verified))", - re.DOTALL, - ), -] - -# Hypothetical / product-blame shapes that must stay silent even if a -# substring of a positive pattern appears nearby. -NEGATIVE_RES = [ - re.compile(r"(?i)\bif\s+my\s+(earlier|previous|prior)\s+check\s+was\s+wrong\b"), - re.compile( - r"(?i)\bif\s+i\s+(read|got|took|marked|logged|noted)\s+(that|this|it)\s+wrong\b" - ), - re.compile(r"(?i)\bthe\s+(test|ui|build|product|code)\s+was\s+wrong\b"), - re.compile(r"(?i)\bif\b.{0,40}\bmy\s+mistake\b"), - re.compile( - r"(?i)\b(?:if|unless|whether|in\s+case|suppose|assuming)\s+i\s+" - r"(?:misread|mis-read|misunderstood|mixed\s+up)\b" - ), - re.compile( - r"(?i)\b(?:if|unless|whether|in\s+case|suppose|assuming)\s+i\s+" - r"(?:was|were)\s+wrong\b" - ), - re.compile( - r"(?i)\b(?:says?|said|thinks?|thought|claims?|claimed|argued|insisted|" - r"told\s+me|telling\s+me)\s+(?:that\s+)?i\s+(?:was|were)\s+wrong\b" - ), -] - -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, " + "Wrong-check admission on this transcript. This is a FAILURE, " "not a preference ping: a claim went out before a real check. Finish the " "live correction first. Then read the reflect skill and spawn a subagent " - "for steps 1-4 on this exact file: {path}. Present Accepted / Backlog / " + "for steps 1-4 on this exact transcript. Present Accepted / Backlog / " "Route-to-automate-me / Rejected. Do not skip because the task also finished." ) -CODEX_ADVISORY = ( - "wrong-check-reflect: assistant admitted a prior check/claim was wrong " - "({match}). Codex cannot force a rewrite — run /reflect on this session " - "when convenient." -) - - -def strip_fences(text: str) -> str: - return FENCE_RE.sub("", text or "") - - -def strip_quoted_spans(text: str) -> str: - cleaned = DOUBLE_QUOTE_RE.sub("", text or "") - return BACKTICK_RE.sub("", cleaned) - - -def find_admission(text: str) -> str | None: - """Return the matched phrase if text is a first-person wrong-check - admission, else None.""" - cleaned = strip_quoted_spans(strip_fences(text)) - if not cleaned.strip(): - return None - for pattern in NEGATIVE_RES: - if pattern.search(cleaned): - return None - for pattern in ADMISSION_RES: - match = pattern.search(cleaned) - if match: - return match.group(0) - return structural_admission(cleaned) - def _state_file(transcript_path: str) -> str: key = transcript_path or "no-transcript" @@ -216,12 +42,9 @@ def already_prompted(transcript_path: str) -> bool: def mark_prompted(transcript_path: str) -> None: path = _state_file(transcript_path or "no-transcript") - try: - os.makedirs(os.path.dirname(path), exist_ok=True) - with open(path, "w", encoding="utf-8") as handle: - handle.write((transcript_path or "") + "\n") - except OSError: - pass + os.makedirs(os.path.dirname(path), exist_ok=True) + with open(path, "w", encoding="utf-8") as handle: + handle.write((transcript_path or "") + "\n") def _is_user_line(data: dict) -> bool: @@ -270,11 +93,10 @@ def user_already_asked_reflect(path: str) -> bool: def resolve_transcript(payload: dict) -> str: - direct = ( - payload.get("agent_transcript_path") - or payload.get("transcript_path") - or payload.get("transcriptPath") - ) + agent = payload.get("agent_transcript_path") + if isinstance(agent, str): + return agent if os.path.isfile(agent) else "" + direct = payload.get("transcript_path") or payload.get("transcriptPath") if isinstance(direct, str) and os.path.isfile(direct): return direct conv = payload.get("conversation_id") or payload.get("conversationId") @@ -333,40 +155,8 @@ def last_assistant_text(payload: dict, transcript_path: str = "") -> str: return last_assistant_from_transcript(transcript_path) -def followup_for(match: str, path: str) -> str: - return FOLLOWUP.format(match=match, path=path or "(no transcript path)") - - def decide(payload: dict) -> str | None: - """Return the follow-up instruction, or None to stay silent.""" - if not isinstance(payload, dict): - return None - if payload.get("stop_hook_active"): - return None - path = resolve_transcript(payload) - text = last_assistant_text(payload, path) - match = find_admission(text) - if not match: - return None - if path and user_already_asked_reflect(path): - return None - key = path or text[:200] - if already_prompted(key): - return None - mark_prompted(key) - return followup_for(match, path) - - -JUDGE_PROMPT = ( - 'You are a classifier. Answer with exactly one line of JSON and nothing else: ' - '{"pushback": true|false, "self_correction": true|false, ' - '"quote": ""}. ' - "pushback = the USER message disputes, questions, or corrects something the " - "assistant said earlier. self_correction = the latest ASSISTANT reply admits, " - "in any wording, that something it previously told the user was wrong, " - "misread, or answered the wrong question." -) -JUDGE_MESSAGE_LIMIT = 4000 + return None @functools.cache @@ -379,88 +169,37 @@ def _judge(): return module -def last_exchange(path: str) -> tuple[str, str, str] | None: - """(earlier assistant, user, current reply) from the transcript, or None. - - Consecutive text lines of one role are one message, so tool calls and - tool results inside a turn do not split it. - """ - turns: list[tuple[str, list[str]]] = [] - with open(path, encoding="utf-8") as handle: - for line in handle: - try: - data = json.loads(line) - except json.JSONDecodeError: - continue - if not isinstance(data, dict): - continue - if _is_assistant_line(data): - role = "assistant" - elif _is_user_line(data): - role = "user" - else: - continue - text = _message_text(data) - if not text.strip() or (role == "user" and text.lstrip().startswith(META_USER_PREFIXES)): - continue - if turns and turns[-1][0] == role: - turns[-1][1].append(text) - else: - turns.append((role, [text])) - if len(turns) < 3 or turns[-1][0] != "assistant": - return None - earlier, user, reply = ("\n".join(parts) for _, parts in turns[-3:]) - return earlier, user, reply - - -def judge_prompt(earlier: str, user: str, reply: str) -> str: - cut = JUDGE_MESSAGE_LIMIT - return ( - f"{JUDGE_PROMPT}\n\n" - f"EARLIER ASSISTANT:\n{earlier[-cut:]}\n\n" - f"USER:\n{user[-cut:]}\n\n" - f"ASSISTANT:\n{reply[-cut:]}" - ) - +@functools.cache +def _phrases(): + spec = importlib.util.spec_from_file_location("llm_judge_phrases", PHRASES_PATH) + if spec is None or spec.loader is None: + raise ImportError(f"cannot load llm-judge phrases from {PHRASES_PATH}") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module -def enqueue_judge(payload: dict, regex_fired: bool) -> str | None: - """Ask llm-judge, in the background, whether the user pushed back and the - reply took something back. Returns the job id, or None when not asked. - Only runs when the regex stayed silent. A hit is delivered one turn later - by the llm-judge inbox, as the same reflect follow-up. - """ - if not isinstance(payload, dict) or payload.get("stop_hook_active") or regex_fired: +def enqueue_judge(payload: dict) -> str | None: + if not isinstance(payload, dict) or payload.get("stop_hook_active"): return None path = resolve_transcript(payload) - if not path or already_prompted(path): + text = last_assistant_text(payload, path) + key = path or text[:200] + if not text.strip() or already_prompted(key): return None - exchange = last_exchange(path) - if exchange is None: + if path and user_already_asked_reflect(path): return None - return _judge().enqueue({ - "id": uuid.uuid4().hex, - "hook": "wrong-check-reflect", - "transcript": path, - "prompt": judge_prompt(*exchange), - "hit_if_all_true": ["pushback", "self_correction"], - "on_hit": followup_for("model judge", path), - }) - - -def try_enqueue_judge(payload: dict, regex_fired: bool) -> None: - """enqueue_judge for the harness scripts: an error is logged, never raised.""" + dictionary = _phrases().load("wrong-check-reflect") + job = _phrases().job(dictionary, path, text) + job["id"] = uuid.uuid4().hex + job_id = _judge().enqueue(job) + if job_id is not None: + mark_prompted(key) + return job_id + + +def try_enqueue_judge(payload: dict) -> None: try: - enqueue_judge(payload, regex_fired) - except Exception as exc: - sys.stderr.write(f"wrong-check-reflect: judge enqueue failed: {exc}\n") - - -def scan_assistant_texts(texts: Iterable[str]) -> list[str]: - """Return matched admission phrases from a list of assistant texts.""" - hits = [] - for text in texts: - match = find_admission(text or "") - if match: - hits.append(match) - return hits + enqueue_judge(payload) + except Exception: + return diff --git a/engine/hooks/wrong-check-reflect/eval_dictionary.py b/engine/hooks/wrong-check-reflect/eval_dictionary.py new file mode 100644 index 0000000..258dabb --- /dev/null +++ b/engine/hooks/wrong-check-reflect/eval_dictionary.py @@ -0,0 +1,46 @@ +#!/usr/bin/env python3 +from __future__ import annotations + +import argparse +import json +import os +import sys + +LLM_JUDGE_DIR = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "llm-judge") +sys.path.insert(0, LLM_JUDGE_DIR) + +import judge # noqa: E402 +import phrases # noqa: E402 + +HIT_TEXT = "Correction: the file I pointed you to earlier is not the one in use; the real one is src/b.py." +CASES = ( + (HIT_TEXT, True), + ("You're right. Let's go with option B.", False), + ("I double-checked my earlier count and it holds; nothing in it was wrong.", False), +) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description="Evaluate the wrong-check-reflect phrase dictionary against the real llm-judge runner.", + epilog="--runner-env note: set CATSTACK_LLM_JUDGE_RUNNERS to choose the model command; without it this calls a real model.", + ) + parser.add_argument("--runner-env", action="store_true", help="print the runner environment note and exit") + args = parser.parse_args(argv) + if args.runner_env: + print("Set CATSTACK_LLM_JUDGE_RUNNERS to choose the model command; without it this calls a real model.") + return 0 + dictionary = phrases.load("wrong-check-reflect") + ok = True + for text, expected in CASES: + result = judge.ask(phrases.prompt(dictionary, text)) + answer = result.get("answer") if result.get("outcome") == "answered" else None + matched = isinstance(answer, dict) and answer.get("match") is True + print(f"{text}\t{json.dumps(answer, sort_keys=True)}") + if matched is not expected: + ok = False + return 0 if ok else 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/engine/hooks/wrong-check-reflect/tests/test_hooks.py b/engine/hooks/wrong-check-reflect/tests/test_hooks.py index 3beee47..4165be6 100644 --- a/engine/hooks/wrong-check-reflect/tests/test_hooks.py +++ b/engine/hooks/wrong-check-reflect/tests/test_hooks.py @@ -1,8 +1,5 @@ #!/usr/bin/env python3 -"""Tests for wrong-check-reflect. - -Run: python3 -m unittest discover -s engine/hooks/wrong-check-reflect/tests -v -""" +"""Tests for wrong-check-reflect.""" from __future__ import annotations import io @@ -27,6 +24,19 @@ sys.path.append(os.path.dirname(detect.LLM_JUDGE_PATH)) import inbox as judge_inbox # noqa: E402 import judge # noqa: E402 +import phrases # noqa: E402 + + +PY = sys.executable +HIT_TEXT = "Correction: the file I pointed you to earlier is not the one in use; the real one is src/b.py." +OPTION_TEXT = "You're right. Let's go with option B." +COUNT_TEXT = "I double-checked my earlier count and it holds; nothing in it was wrong." +JUDGE_SAYS_HIT = json.dumps({"match": True, "closest": HIT_TEXT}) +JUDGE_SAYS_CLEAN = json.dumps({"match": False, "closest": ""}) +ANSWERS_HIT = ["fake", [PY, "-c", f"print({JUDGE_SAYS_HIT!r})", "{prompt}"]] +ANSWERS_CLEAN = ["fake", [PY, "-c", f"print({JUDGE_SAYS_CLEAN!r})", "{prompt}"]] +SLOW_CLEAN = ["slow", [PY, "-c", f"import time; time.sleep(2); print({JUDGE_SAYS_CLEAN!r})", "{prompt}"]] +MISSING = ["ghost", ["catstack-llm-judge-no-such-binary", "{prompt}"]] def run_claude(payload: dict): @@ -40,12 +50,13 @@ def run_claude(payload: dict): return False, err.getvalue() -def run_cursor(payload: dict) -> dict: +def run_cursor(payload: dict) -> tuple[dict, str]: out = io.StringIO() + err = io.StringIO() with patch.object(sys, "stdin", io.StringIO(json.dumps(payload))): - with redirect_stdout(out): + with redirect_stdout(out), redirect_stderr(err): cursor_session.main() - return json.loads(out.getvalue() or "{}") + return json.loads(out.getvalue() or "{}"), err.getvalue() def run_codex_notify(argv: list[str]) -> str: @@ -56,480 +67,11 @@ def run_codex_notify(argv: list[str]) -> str: return err.getvalue() -class TestFindAdmission(unittest.TestCase): - def test_hit_good_catch_earlier_check_was_wrong(self): - match = detect.find_admission( - "Good catch — my earlier check was wrong. The real file is elsewhere." - ) - self.assertIsNotNone(match) - self.assertIn("earlier check was wrong", match.lower()) - - def test_hit_youre_right_i_misread(self): - self.assertIsNotNone( - detect.find_admission("You're right, I misread the file.") - ) - - def test_hit_incorrectly_assumed(self): - self.assertIsNotNone( - detect.find_admission("I incorrectly assumed that was the source.") - ) - - def test_hit_previous_grep_was_wrong(self): - self.assertIsNotNone( - detect.find_admission("my previous grep was wrong — that path is dead.") - ) - - def test_hit_file_i_cited_was_duplicate(self): - self.assertIsNotNone( - detect.find_admission("the file I cited was a duplicate.") - ) - - def test_hit_my_mistake_misread_own_skill(self): - self.assertIsNotNone( - detect.find_admission( - "My mistake — the skill does have " - "disable-model-invocation: true (I misread it), so " - "fires_example.md needs the literal invocation string." - ) - ) - - def test_hit_i_misread_without_youre_right_prefix(self): - self.assertIsNotNone( - detect.find_admission("I misread the front matter on that skill.") - ) - - def test_hit_youre_right_verifying_it_now(self): - self.assertIsNotNone(detect.find_admission( - "You're right. Verifying it now instead of labeling it." - )) - - def test_hit_good_catch_i_should_have_checked(self): - self.assertIsNotNone(detect.find_admission( - "Good catch on the hook. I should have run the two greps before sending that." - )) - - def test_hit_standalone_youre_right_then_misread_which(self): - """The live miss: a standalone concession, then 'I misread which ...'.""" - self.assertIsNotNone(detect.find_admission( - "You're right. PR #377 doesn't have many deletes: it's +628 / -157. " - "I misread which diff you meant. The ~9,500 deletions only showed up " - "in a local comparison I ran against today's `main`, not in the PR. " - "Please ignore that part." - )) - - def test_hit_standalone_youre_right_alone(self): - self.assertIsNotNone(detect.find_admission( - "**You are right** — PR #12 has two commits, not one." - )) - - def test_hit_i_misunderstood_the_question(self): - self.assertIsNotNone(detect.find_admission( - "I misunderstood the question, so the numbers above answer a different one." - )) - - def test_no_hit_youre_right_mid_reply(self): - self.assertIsNone(detect.find_admission( - "The build is green. The reviewer asked whether you're right. Checking." - )) - - def test_no_hit_hypothetical_misunderstood(self): - self.assertIsNone(detect.find_admission( - "Unless I misunderstood the ask, the report covers both repos." - )) - - def test_no_hit_youre_right_agreeing_with_a_choice(self): - self.assertIsNone(detect.find_admission( - "You're right that the second option is cheaper, so I will build that one." - )) - - def test_hit_your_instinct_was_right_stands_alone(self): - self.assertIsNotNone(detect.find_admission( - "Your instinct was right — the size cap was silently skipping files." - )) - - def test_hit_your_hunch_was_right(self): - self.assertIsNotNone(detect.find_admission( - "Your hunch was right, the wrapper path was never resolved." - )) - - def test_no_hit_product_test_was_wrong(self): - self.assertIsNone(detect.find_admission("the test was wrong")) - - def test_no_hit_hypothetical(self): - self.assertIsNone( - detect.find_admission("if my earlier check was wrong we'd see X") - ) - - def test_no_hit_hypothetical_my_mistake(self): - self.assertIsNone( - detect.find_admission("if that turns out to be my mistake, I'll fix it") - ) - - def test_no_hit_hypothetical_misread(self): - self.assertIsNone( - detect.find_admission("if I misread this, let me know") - ) - - def test_no_hit_good_catch_alone(self): - self.assertIsNone(detect.find_admission("Good catch")) - - def test_no_hit_inside_code_fence(self): - text = ( - "Here is the pattern:\n" - "```\n" - "Good catch — my earlier check was wrong\n" - "```\n" - "That is what the detector looks for." - ) - self.assertIsNone(detect.find_admission(text)) - - def test_no_hit_empty(self): - self.assertIsNone(detect.find_admission("")) - self.assertIsNone(detect.find_admission(None)) # type: ignore[arg-type] - - def test_hit_unquoted_admission_still_fires(self): - match = detect.find_admission( - "Real talk: my earlier check was wrong, the endpoint moved." - ) - self.assertIsNotNone(match) - self.assertIn("earlier check was wrong", match.lower()) - - def test_no_hit_quoted_readme_example(self): - text = ( - 'wrong-check-reflect fired on quoted example phrases in that ' - "hook's README, not a real admission. It catches things like " - '"Good catch — my earlier check was wrong" when that text is ' - "actually being cited, not asserted." - ) - self.assertIsNone(detect.find_admission(text)) - - def test_no_hit_backtick_quoted_phrase(self): - text = ( - "The regex looks for phrases like `my earlier check was wrong` " - "in assistant text -- describing the pattern, not admitting one." - ) - self.assertIsNone(detect.find_admission(text)) - - def test_hit_reversed_word_order_labeled_without_verifying_at_the_time(self): - match = detect.find_admission( - "I read that wrong in my earlier summary table (labeled them " - "ready without actually verifying status at the time). " - "Confirmed now." - ) - self.assertIsNotNone(match) - - def test_no_hit_normal_correction_language(self): - self.assertIsNone( - detect.find_admission( - "Let me also check the summary table before confirming -- " - "I'll verify this next." - ) - ) - - def test_hit_bare_i_was_wrong(self): - match = detect.find_admission("I was wrong. The pool never tracked that slot.") - self.assertIsNotNone(match) - self.assertIn("i was wrong", match.lower()) - - def test_hit_bare_i_was_wrong_conceding_a_live_diagnosis(self): - self.assertIsNotNone(detect.find_admission( - "I was wrong - it **is** genuinely computing. 10 workers in R state " - "at ~96% CPU." - )) - - def test_hit_i_got_that_wrong(self): - self.assertIsNotNone( - detect.find_admission("I got that wrong -- the worker was live the whole time.") - ) - - def test_no_hit_hypothetical_bare_i_was_wrong(self): - self.assertIsNone(detect.find_admission( - "If I was wrong about this, then the pool would show a free slot." - )) - - def test_no_hit_unless_i_was_wrong(self): - self.assertIsNone(detect.find_admission( - "Unless I was wrong about the ordering, the queue drains first." - )) - - def test_no_hit_reported_speech_someone_said_i_was_wrong(self): - self.assertIsNone(detect.find_admission( - "The reviewer said I was wrong, but the diff shows the guard is present." - )) - - def test_no_hit_third_person_was_wrong(self): - self.assertIsNone(detect.find_admission("He was wrong about the pool, not me.")) - - def test_no_hit_bare_i_was_wrong_inside_code_fence(self): - self.assertIsNone(detect.find_admission( - "Here is the shape:\n```\nI was wrong - it is genuinely computing.\n```\n" - "That is what fires." - )) - - def test_no_hit_bare_i_was_wrong_quoted(self): - self.assertIsNone(detect.find_admission( - 'The hook catches replies like "I was wrong" when they are asserted, ' - "not cited." - )) - - def test_no_hit_bare_i_was_wrong_backticked(self): - self.assertIsNone(detect.find_admission( - "The regex looks for `I was wrong` in assistant text." - )) - - def test_no_hit_hypothetical_reversed_word_order(self): - self.assertIsNone( - detect.find_admission( - "If I read that wrong in my earlier note, let me know and " - "I'll recheck." - ) - ) - - -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() - os.environ["WRONG_CHECK_REFLECT_STATE_DIR"] = self.tmp.name - detect.STATE_DIR = self.tmp.name - - def tearDown(self): - self.tmp.cleanup() - - def test_fires_then_stays_silent(self): - path = os.path.join(self.tmp.name, "sess.jsonl") - with open(path, "w", encoding="utf-8") as handle: - handle.write( - json.dumps( - { - "type": "assistant", - "message": { - "role": "assistant", - "content": "Good catch — my earlier check was wrong", - }, - } - ) - + "\n" - ) - first = detect.decide( - { - "transcript_path": path, - "last_assistant_message": "Good catch — my earlier check was wrong", - } - ) - self.assertIsNotNone(first) - self.assertIn("FAILURE", first) - self.assertIn("reflect", first.lower()) - self.assertIn(path, first) - second = detect.decide( - { - "transcript_path": path, - "last_assistant_message": "Good catch — my earlier check was wrong", - } - ) - self.assertIsNone(second) - - def test_user_already_said_reflect_skips(self): - path = os.path.join(self.tmp.name, "asked.jsonl") - with open(path, "w", encoding="utf-8") as handle: - handle.write( - json.dumps( - { - "type": "user", - "message": {"role": "user", "content": "please /reflect"}, - } - ) - + "\n" - ) - self.assertIsNone( - detect.decide( - { - "transcript_path": path, - "last_assistant_message": "my earlier check was wrong", - } - ) - ) - - def test_stop_hook_active_skips(self): - self.assertIsNone( - detect.decide( - { - "stop_hook_active": True, - "last_assistant_message": "my earlier check was wrong", - } - ) - ) - - def test_clean_does_not_prompt(self): - self.assertIsNone( - detect.decide({"last_assistant_message": "I'll check the logs next."}) - ) - - -class TestHarnessWrappers(unittest.TestCase): - def setUp(self): - self.tmp = tempfile.TemporaryDirectory() - os.environ["WRONG_CHECK_REFLECT_STATE_DIR"] = self.tmp.name - detect.STATE_DIR = self.tmp.name - - def tearDown(self): - self.tmp.cleanup() - - def test_claude_blocks_on_admission(self): - blocked, err = run_claude( - {"last_assistant_message": "Good catch — my earlier check was wrong"} - ) - self.assertTrue(blocked) - self.assertIn("FAILURE", err) - self.assertIn("reflect", err.lower()) - - def test_claude_allows_clean(self): - blocked, err = run_claude({"last_assistant_message": "short reply"}) - self.assertFalse(blocked) - self.assertEqual(err, "") - - def test_claude_malformed_stdin_fail_open(self): - err = io.StringIO() - with patch.object(sys, "stdin", io.StringIO("not-json")): - with redirect_stderr(err): - claude_stop_check.main() - self.assertEqual(err.getvalue(), "") - - def test_cursor_followup_on_admission(self): - body = run_cursor( - {"last_assistant_message": "You're right, I misread the file."} - ) - self.assertIn("FAILURE", body.get("followup_message", "")) - - def test_cursor_empty_on_clean(self): - body = run_cursor({"last_assistant_message": "all good"}) - self.assertEqual(body.get("followup_message"), "") - - def test_cursor_malformed_stdin_fail_open(self): - out = io.StringIO() - with patch.object(sys, "stdin", io.StringIO("not-json")): - with redirect_stdout(out): - cursor_session.main() - self.assertEqual(json.loads(out.getvalue()).get("followup_message"), "") - - def test_codex_prints_on_admission(self): - payload = json.dumps( - { - "type": "agent-turn-complete", - "last-assistant-message": "I incorrectly assumed that was the source.", - } - ) - out = run_codex_notify([payload]) - self.assertIn("wrong-check-reflect", out) - - def test_codex_silent_on_clean(self): - payload = json.dumps( - { - "type": "agent-turn-complete", - "last-assistant-message": "short reply", - } - ) - self.assertEqual(run_codex_notify([payload]), "") - - def test_codex_still_chains(self): - chain = os.path.join(self.tmp.name, "chain.sh") - marker = os.path.join(self.tmp.name, "chained") - with open(chain, "w", encoding="utf-8") as handle: - handle.write(f"#!/bin/sh\necho ok > {marker}\n") - os.chmod(chain, 0o755) - payload = json.dumps( - { - "type": "agent-turn-complete", - "last-assistant-message": "my earlier check was wrong", - } - ) - run_codex_notify([chain, payload]) - self.assertTrue(os.path.isfile(marker)) - - -class TestCodexInstaller(unittest.TestCase): - def test_compute_notify_update_prepends(self): - from install_codex_notify import compute_notify_update - - text = 'notify = ["python3", "/home/x/.codex/hooks/diu-stop/codex_notify.py"]\n' - new_text, changed, _ = compute_notify_update( - text, "/home/x/.codex/hooks/wrong-check-reflect/codex_notify.py" - ) - self.assertTrue(changed) - self.assertIn("wrong-check-reflect/codex_notify.py", new_text) - self.assertIn("diu-stop/codex_notify.py", new_text) - # wrong-check comes first so it chains to diu-stop - self.assertLess( - new_text.index("wrong-check-reflect"), - new_text.index("diu-stop"), - ) - - -PY = sys.executable -EARLIER = "The diff is 151 files, -9569, because main moved 26 commits ahead." -PUSHBACK = "what do you mena? I don't see a lot of deletes in the 377 pr?" -CONCESSION = ( - "You're right. PR #377 doesn't have many deletes: it's +628 / -157. " - "I misread which diff you meant." -) -QUIET_CONCESSION = ( - "Fair point: PR #377 itself is +628 / -157. The 9,569 number came from a " - "local comparison against today's main, not from the PR." -) -JUDGE_SAYS_HIT = json.dumps({"pushback": True, "self_correction": True, "quote": "I misread which diff you meant."}) -JUDGE_SAYS_CLEAN = json.dumps({"pushback": True, "self_correction": False, "quote": ""}) -ANSWERS_HIT = ["fake", [PY, "-c", f"print({JUDGE_SAYS_HIT!r})", "{prompt}"]] -ANSWERS_CLEAN = ["fake", [PY, "-c", f"print({JUDGE_SAYS_CLEAN!r})", "{prompt}"]] -SLOW_HIT = ["slow", [PY, "-c", f"import time; time.sleep(2); print({JUDGE_SAYS_HIT!r})", "{prompt}"]] - - def transcript_line(role: str, text: str) -> str: return json.dumps({"type": role, "message": {"role": role, "content": [{"type": "text", "text": text}]}}) -class TestModelJudge(unittest.TestCase): - """The regex-silent path: the background llm-judge decides, and the - llm-judge inbox delivers the reflect follow-up one turn later.""" - +class TestWrongCheckReflect(unittest.TestCase): def setUp(self): self.reflect_state = tempfile.TemporaryDirectory() self.judge_state = tempfile.TemporaryDirectory() @@ -541,6 +83,8 @@ def setUp(self): self.env.start() os.environ.pop(judge.CHILD_ENV, None) detect.STATE_DIR = self.reflect_state.name + detect._judge.cache_clear() + detect._phrases.cache_clear() caught = warnings.catch_warnings() caught.__enter__() self.addCleanup(caught.__exit__, None, None, None) @@ -553,6 +97,8 @@ def tearDown(self): self.env.stop() self.judge_state.cleanup() self.reflect_state.cleanup() + detect._judge.cache_clear() + detect._phrases.cache_clear() def jobs(self) -> list[str]: folder = os.path.join(self.judge_state.name, "jobs") @@ -565,8 +111,14 @@ def write_transcript(self, *lines: tuple[str, str], name: str = "session.jsonl") handle.write(transcript_line(role, text) + "\n") return path - def pushback_transcript(self, reply: str = CONCESSION) -> str: - return self.write_transcript(("assistant", EARLIER), ("user", PUSHBACK), ("assistant", reply)) + def wait_for_jobs(self, count: int, seconds: float = 5) -> list[str]: + deadline = time.monotonic() + seconds + while time.monotonic() < deadline: + jobs = self.jobs() + if len(jobs) == count: + return jobs + time.sleep(0.05) + return self.jobs() def wait_for_messages(self, path: str, seconds: float = 15) -> list[str]: deadline = time.monotonic() + seconds @@ -577,146 +129,103 @@ def wait_for_messages(self, path: str, seconds: float = 15) -> list[str]: time.sleep(0.1) return [] - def test_judge_hit_on_pushback_and_concession_reaches_inbox(self): - path = self.pushback_transcript() - self.assertIsNotNone(detect.enqueue_judge({"transcript_path": path}, regex_fired=False)) - self.assertEqual(self.wait_for_messages(path), [detect.followup_for("model judge", path)]) + def test_dictionary_loads(self): + dictionary = phrases.load("wrong-check-reflect") + self.assertEqual(dictionary["checker"], "wrong-check-reflect") + self.assertEqual(dictionary["on_hit"], detect.FOLLOWUP) + + def test_decide_no_longer_returns_pattern_hit(self): + self.assertIsNone(detect.decide({"last_assistant_message": HIT_TEXT})) - def test_judge_clean_verdict_prints_nothing(self): + def test_claude_stop_queues_job_for_normal_reply(self): + os.environ[judge.RUNNERS_ENV] = json.dumps([SLOW_CLEAN]) + path = self.write_transcript(("assistant", HIT_TEXT)) + blocked, err = run_claude({"transcript_path": path}) + self.assertFalse(blocked) + self.assertEqual(err, "") + jobs = self.wait_for_jobs(1) + self.assertEqual(len(jobs), 1) + with open(os.path.join(self.judge_state.name, "jobs", jobs[0]), encoding="utf-8") as handle: + job = json.load(handle) + self.assertEqual(job["hook"], "wrong-check-reflect") + self.assertEqual(job["transcript"], path) + self.assertEqual(job["on_hit"], detect.FOLLOWUP) + + def test_hit_verdict_reaches_agent_as_dictionary_on_hit(self): + path = self.write_transcript(("assistant", HIT_TEXT)) + self.assertIsNotNone(detect.enqueue_judge({"transcript_path": path})) + self.assertEqual(self.wait_for_messages(path), [detect.FOLLOWUP]) + + def test_clean_verdict_says_nothing(self): os.environ[judge.RUNNERS_ENV] = json.dumps([ANSWERS_CLEAN]) - path = self.pushback_transcript() - self.assertIsNotNone(detect.enqueue_judge({"transcript_path": path}, regex_fired=False)) + path = self.write_transcript(("assistant", OPTION_TEXT)) + self.assertIsNotNone(detect.enqueue_judge({"transcript_path": path})) deadline = time.monotonic() + 15 while self.jobs() and time.monotonic() < deadline: time.sleep(0.1) - self.assertEqual(self.jobs(), []) self.assertEqual(judge_inbox.messages(path), []) - def test_claude_stop_returns_at_once_while_judge_runs(self): - os.environ[judge.RUNNERS_ENV] = json.dumps([SLOW_HIT]) - self.assertIsNone(detect.find_admission(QUIET_CONCESSION)) - path = self.pushback_transcript(QUIET_CONCESSION) - started = time.monotonic() - blocked, err = run_claude({"transcript_path": path}) - elapsed = time.monotonic() - started - self.assertLess(elapsed, 1.0) - self.assertFalse(blocked) - self.assertEqual(err, "") - self.assertEqual(len(self.jobs()), 1) - self.assertEqual(self.wait_for_messages(path), [detect.followup_for("model judge", path)]) + def test_unchecked_verdict_says_could_not_judge(self): + os.environ[judge.RUNNERS_ENV] = json.dumps([MISSING]) + path = self.write_transcript(("assistant", COUNT_TEXT)) + self.assertIsNotNone(detect.enqueue_judge({"transcript_path": path})) + messages = self.wait_for_messages(path) + self.assertEqual(len(messages), 1) + self.assertIn("could not judge", messages[0]) def test_judge_not_enqueued_when_stop_hook_active(self): - path = self.pushback_transcript() - self.assertIsNone(detect.enqueue_judge({"transcript_path": path, "stop_hook_active": True}, regex_fired=False)) - self.assertEqual(self.jobs(), []) - - def test_judge_not_enqueued_when_regex_fired(self): - path = self.pushback_transcript() - self.assertIsNone(detect.enqueue_judge({"transcript_path": path}, regex_fired=True)) - self.assertEqual(self.jobs(), []) - - def test_claude_stop_never_enqueues_when_its_regex_blocks(self): - path = self.pushback_transcript() - blocked, _ = run_claude({"transcript_path": path}) - self.assertTrue(blocked) - self.assertEqual(self.jobs(), []) - - def test_judge_not_enqueued_on_first_user_message(self): - path = self.write_transcript(("user", PUSHBACK), ("assistant", CONCESSION)) - self.assertIsNone(detect.enqueue_judge({"transcript_path": path}, regex_fired=False)) + path = self.write_transcript(("assistant", HIT_TEXT)) + self.assertIsNone(detect.enqueue_judge({"transcript_path": path, "stop_hook_active": True})) self.assertEqual(self.jobs(), []) def test_judge_not_enqueued_when_already_prompted(self): - path = self.pushback_transcript() + path = self.write_transcript(("assistant", HIT_TEXT)) detect.mark_prompted(path) - self.assertIsNone(detect.enqueue_judge({"transcript_path": path}, regex_fired=False)) - self.assertEqual(self.jobs(), []) - - def test_judge_not_enqueued_with_missing_transcript(self): - gone = os.path.join(self.reflect_state.name, "gone.jsonl") - self.assertIsNone(detect.enqueue_judge({"transcript_path": gone}, regex_fired=False)) + self.assertIsNone(detect.enqueue_judge({"transcript_path": path})) self.assertEqual(self.jobs(), []) - def test_judge_not_enqueued_inside_a_judge_child(self): - os.environ[judge.CHILD_ENV] = "1" - path = self.pushback_transcript() - self.assertIsNone(detect.enqueue_judge({"transcript_path": path}, regex_fired=False)) + def test_judge_not_enqueued_when_user_already_asked_reflect(self): + path = self.write_transcript(("user", "please /reflect"), ("assistant", HIT_TEXT)) + self.assertIsNone(detect.enqueue_judge({"transcript_path": path})) self.assertEqual(self.jobs(), []) - def test_last_exchange_skips_tool_lines_and_harness_text(self): - path = os.path.join(self.reflect_state.name, "tools.jsonl") - tool_use = {"type": "assistant", "message": {"role": "assistant", "content": [{"type": "tool_use", "name": "Bash"}]}} - tool_result = {"type": "user", "message": {"role": "user", "content": [{"type": "tool_result", "content": "ok"}]}} - with open(path, "w", encoding="utf-8") as handle: - for line in ( - transcript_line("user", "how big is the diff?"), - json.dumps(tool_use), - json.dumps(tool_result), - transcript_line("assistant", EARLIER), - transcript_line("user", "ignore me"), - transcript_line("user", PUSHBACK), - "not json", - transcript_line("assistant", "Checking."), - json.dumps(tool_use), - json.dumps(tool_result), - transcript_line("assistant", CONCESSION), - ): - handle.write(line + "\n") - self.assertEqual(detect.last_exchange(path), (EARLIER, PUSHBACK, "Checking.\n" + CONCESSION)) - - def test_judge_prompt_labels_and_cuts_each_message(self): - prompt = detect.judge_prompt("e" * 5000 + "END", PUSHBACK, CONCESSION) - self.assertTrue(prompt.startswith(detect.JUDGE_PROMPT)) - self.assertIn("EARLIER ASSISTANT:\n" + "e" * 3997 + "END\n\nUSER:\n" + PUSHBACK, prompt) - self.assertNotIn("e" * 3998, prompt) - self.assertTrue(prompt.endswith("ASSISTANT:\n" + CONCESSION)) - - def test_judge_enqueue_failure_is_logged_and_does_not_block(self): - clean = {"last_assistant_message": "short reply", "type": "agent-turn-complete", - "last-assistant-message": "short reply"} - with patch.object(detect, "enqueue_judge", side_effect=RuntimeError("boom")): - blocked, err = run_claude(clean) - cursor_err = io.StringIO() - with redirect_stderr(cursor_err): - body = run_cursor(clean) - codex_err = run_codex_notify([json.dumps(clean)]) - expected = "wrong-check-reflect: judge enqueue failed: boom\n" - self.assertFalse(blocked) - self.assertEqual(err, expected) - self.assertEqual(body, {"followup_message": ""}) - self.assertEqual(cursor_err.getvalue(), expected) - self.assertEqual(codex_err, expected) - - def test_missing_llm_judge_is_logged_not_silent(self): - path = self.pushback_transcript() - detect._judge.cache_clear() - self.addCleanup(detect._judge.cache_clear) + def test_claude_malformed_stdin_fail_open(self): err = io.StringIO() - with patch.object(detect, "LLM_JUDGE_PATH", os.path.join(self.reflect_state.name, "no-judge.py")): + with patch.object(sys, "stdin", io.StringIO("not-json")): with redirect_stderr(err): - detect.try_enqueue_judge({"transcript_path": path}, regex_fired=False) - self.assertIn("wrong-check-reflect: judge enqueue failed:", err.getvalue()) - self.assertEqual(self.jobs(), []) + claude_stop_check.main() + self.assertEqual(err.getvalue(), "") - def test_unreadable_transcript_is_logged_not_silent(self): - path = os.path.join(self.reflect_state.name, "binary.jsonl") - with open(path, "wb") as handle: - handle.write(b"\xff\xfe\xfa\n") - err = io.StringIO() - with redirect_stderr(err): - detect.try_enqueue_judge({"transcript_path": path}, regex_fired=False) - self.assertIn("wrong-check-reflect: judge enqueue failed:", err.getvalue()) - self.assertEqual(self.jobs(), []) + def test_cursor_returns_empty_followup(self): + path = self.write_transcript(("assistant", HIT_TEXT)) + body, err = run_cursor({"transcript_path": path}) + self.assertEqual(body, {"followup_message": ""}) + self.assertEqual(err, "") + def test_codex_still_chains(self): + chain = os.path.join(self.reflect_state.name, "chain.sh") + marker = os.path.join(self.reflect_state.name, "chained") + with open(chain, "w", encoding="utf-8") as handle: + handle.write(f"#!/bin/sh\necho ok > {marker}\n") + os.chmod(chain, 0o755) + payload = json.dumps({"type": "agent-turn-complete", "last-assistant-message": HIT_TEXT}) + run_codex_notify([chain, payload]) + self.assertTrue(os.path.isfile(marker)) -if __name__ == "__main__": - unittest.main() + def test_judge_enqueue_failure_leaves_reply_untouched(self): + payload = {"last_assistant_message": HIT_TEXT, "type": "agent-turn-complete", "last-assistant-message": HIT_TEXT} + with patch.object(detect, "enqueue_judge", side_effect=RuntimeError("boom")): + blocked, err = run_claude(payload) + body, cursor_err = run_cursor(payload) + codex_err = run_codex_notify([json.dumps(payload)]) + self.assertFalse(blocked) + self.assertEqual(err, "") + self.assertEqual(body, {"followup_message": ""}) + self.assertEqual(cursor_err, "") + self.assertEqual(codex_err, "") class TestSubagentTranscript(unittest.TestCase): - """Under SubagentStop, `agent_transcript_path` (the subagent's own file) - wins over `transcript_path` (the parent session's).""" - def test_resolve_transcript_prefers_agent_transcript_path(self): tmp = tempfile.TemporaryDirectory() self.addCleanup(tmp.cleanup) @@ -742,3 +251,24 @@ def test_missing_agent_transcript_does_not_fall_back_to_the_parent(self): detect.resolve_transcript({"transcript_path": parent, "agent_transcript_path": gone}), "", ) + + +class TestCodexInstaller(unittest.TestCase): + def test_compute_notify_update_prepends(self): + from install_codex_notify import compute_notify_update + + text = 'notify = ["python3", "/home/x/.codex/hooks/diu-stop/codex_notify.py"]\n' + new_text, changed, _ = compute_notify_update( + text, "/home/x/.codex/hooks/wrong-check-reflect/codex_notify.py" + ) + self.assertTrue(changed) + self.assertIn("wrong-check-reflect/codex_notify.py", new_text) + self.assertIn("diu-stop/codex_notify.py", new_text) + self.assertLess( + new_text.index("wrong-check-reflect"), + new_text.index("diu-stop"), + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/engine/skills/reflect/scripts/self_retraction_scan.py b/engine/skills/reflect/scripts/self_retraction_scan.py new file mode 100644 index 0000000..d0d533d --- /dev/null +++ b/engine/skills/reflect/scripts/self_retraction_scan.py @@ -0,0 +1,149 @@ +"""Offline scan for first-person wrong-check admissions. + +Used only by the reflect miner over whole transcripts. The wrong-check-reflect +hook no longer decides from phrasings; it asks the background judge. This copy +stays text-only so mining historical transcripts needs no model calls. +""" +from __future__ import annotations + +import re +from typing import Iterable + +FENCE_RE = re.compile(r"```.*?```", re.DOTALL) +DOUBLE_QUOTE_RE = re.compile(r'"[^"]*"', re.DOTALL) +BACKTICK_RE = re.compile(r"`[^`]*`", re.DOTALL) + +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 + +ADMISSION_RES = [ + re.compile( + r"(?i)\bmy\s+(earlier|previous|prior)\s+" + r"(check|grep|read|assumption|claim|citation)\s+was\s+wrong\b" + ), + re.compile( + r"(?i)\byou'?re\s+right,?\s+i\s+(misread|mis-read|misunderstood)\b" + ), + re.compile( + r"(?i)\bi\s+incorrectly\s+assumed\b" + ), + re.compile( + r"(?i)\bthe\s+file\s+i\s+(cited|named|pointed\s+to)\s+was\s+(a\s+)?duplicate\b" + ), + re.compile( + r"(?i)\bgood\s+catch\b.{0,80}\bmy\s+(earlier|previous|prior)\s+" + r"(check|grep|read|assumption|claim)\s+was\s+wrong\b", + re.DOTALL, + ), + re.compile( + r"(?i)\bi\s+(was\s+wrong|got\s+it\s+wrong)\s+(about|on)\s+" + r"(the\s+)?(file|path|source|check|assumption)\b" + ), + re.compile( + r"(?i)\bi\s+(?:was\s+wrong|got\s+(?:it|that|this)\s+wrong)\b" + ), + re.compile( + r"(?i)\bi\s+(read|got|took|marked|logged|noted)\s+(that|this|it)\s+wrong\s+" + r"in\s+my\s+(earlier|previous|prior)\s+\w+" + ), + re.compile( + r"(?i)\bi\s+.{0,120}\b(labeled|marked|claimed|described|reported)\b" + r".{0,100}\bwithout\s+(actually\s+)?(verifying|checking|confirming)\b" + r".{0,80}\bat\s+the\s+time\b", + re.DOTALL, + ), + re.compile( + r"(?i)\bmy\s+mistake\b" + ), + re.compile( + r"(?i)\bi\s+(?:misread|mis-read|misunderstood|mixed\s+up)\b" + ), + re.compile( + r"(?i)^\s*[*_#\s>-]*(?:you[’']?re|you\s+are)\s+right[*_]*\s*[.!:—–]" + ), + re.compile( + r"(?i)\byour\s+(?:instinct|hunch|gut|suspicion|read)\s+(?:was|were)\s+right\b" + ), + re.compile( + r"(?i)^\s*[*_#\s>-]*(?:you'?re\s+right|you\s+are\s+right|good\s+catch)\b" + r".{0,200}?(?:verifying\s+(?:it\s+|that\s+)?now|checking\s+(?:it\s+|that\s+)?now|" + r"i\s+hadn'?t\b|i\s+had\s+not\b|i\s+didn'?t\b|i\s+did\s+not\b|" + r"i\s+should\s+have\b|instead\s+of\s+(?:labeling|labelling|assuming|guessing)|" + r"i\s+never\s+(?:ran|checked|read|verified))", + re.DOTALL, + ), +] + +NEGATIVE_RES = [ + re.compile(r"(?i)\bif\s+my\s+(earlier|previous|prior)\s+check\s+was\s+wrong\b"), + re.compile( + r"(?i)\bif\s+i\s+(read|got|took|marked|logged|noted)\s+(that|this|it)\s+wrong\b" + ), + re.compile(r"(?i)\bthe\s+(test|ui|build|product|code)\s+was\s+wrong\b"), + re.compile(r"(?i)\bif\b.{0,40}\bmy\s+mistake\b"), + re.compile( + r"(?i)\b(?:if|unless|whether|in\s+case|suppose|assuming)\s+i\s+" + r"(?:misread|mis-read|misunderstood|mixed\s+up)\b" + ), + re.compile( + r"(?i)\b(?:if|unless|whether|in\s+case|suppose|assuming)\s+i\s+" + r"(?:was|were)\s+wrong\b" + ), + re.compile( + r"(?i)\b(?:says?|said|thinks?|thought|claims?|claimed|argued|insisted|" + r"told\s+me|telling\s+me)\s+(?:that\s+)?i\s+(?:was|were)\s+wrong\b" + ), +] + +def strip_fences(text: str) -> str: + return FENCE_RE.sub("", text or "") + + +def strip_quoted_spans(text: str) -> str: + cleaned = DOUBLE_QUOTE_RE.sub("", text or "") + return BACKTICK_RE.sub("", cleaned) + + +def structural_admission(cleaned: str) -> str | None: + 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 + + +def find_admission(text: str) -> str | None: + cleaned = strip_quoted_spans(strip_fences(text)) + if not cleaned.strip(): + return None + for pattern in NEGATIVE_RES: + if pattern.search(cleaned): + return None + for pattern in ADMISSION_RES: + match = pattern.search(cleaned) + if match: + return match.group(0) + return structural_admission(cleaned) + + +def scan_assistant_texts(texts: Iterable[str]) -> list[str]: + hits = [] + for text in texts: + match = find_admission(text or "") + if match: + hits.append(match) + return hits diff --git a/engine/skills/reflect/scripts/tests/test_self_retraction_scan.py b/engine/skills/reflect/scripts/tests/test_self_retraction_scan.py new file mode 100644 index 0000000..ca45136 --- /dev/null +++ b/engine/skills/reflect/scripts/tests/test_self_retraction_scan.py @@ -0,0 +1,53 @@ +#!/usr/bin/env python3 +"""Unit tests for self_retraction_scan.py, the reflect miner's offline scan. + +Run: python3 -m unittest discover -s engine/skills/reflect/scripts/tests -v + +The wrong-check-reflect hook asks the background judge instead of matching +phrasings. This scan stays text-only so mining historical transcripts needs no +model calls, and it carries the cases the scenario suite no longer pins. +""" +import os +import sys +import unittest + +SCRIPTS_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, SCRIPTS_DIR) + +import self_retraction_scan # noqa: E402 + + +class TestFindAdmission(unittest.TestCase): + def test_listed_admission_matches(self): + self.assertIsNotNone(self_retraction_scan.find_admission("My mistake — the count was 12.")) + + def test_unlisted_wording_matches_structurally(self): + text = "Also: a claim I made earlier was wrong. I told you the slice was green." + self.assertIsNotNone(self_retraction_scan.find_admission(text)) + + def test_hypothetical_stays_clean(self): + text = "If my earlier check was wrong we should redo it — want me to re-read the file?" + self.assertIsNone(self_retraction_scan.find_admission(text)) + + def test_product_blame_is_not_self_correction(self): + text = "The test was wrong, not the code — the fixture asserted the old brand green." + self.assertIsNone(self_retraction_scan.find_admission(text)) + + def test_quoted_admission_does_not_fire(self): + text = 'The rule says "my earlier check was wrong" is an admission.' + self.assertIsNone(self_retraction_scan.find_admission(text)) + + +class TestScanAssistantTexts(unittest.TestCase): + def test_collects_one_hit_per_admission(self): + hits = self_retraction_scan.scan_assistant_texts( + ["My mistake — the count was 12.", "The migration finished.", "I misread that file."] + ) + self.assertEqual(len(hits), 2) + + def test_empty_input_yields_no_hits(self): + self.assertEqual(self_retraction_scan.scan_assistant_texts(["", None]), []) + + +if __name__ == "__main__": + unittest.main() diff --git a/engine/skills/reflect/scripts/token_audit.py b/engine/skills/reflect/scripts/token_audit.py index 9ec4e26..3a5e0ac 100644 --- a/engine/skills/reflect/scripts/token_audit.py +++ b/engine/skills/reflect/scripts/token_audit.py @@ -48,7 +48,7 @@ running an audit against a remote host is a separate, explicitly-confirmed step outside this script. """ -import bisect, json, sys, hashlib, os, re, importlib.util +import bisect, json, sys, hashlib, os, re from datetime import datetime from collections import Counter @@ -312,29 +312,17 @@ def intervention_must_automate(frustration): return yes, count, rationale -def _load_wrong_check_detect(): - """Load engine/hooks/wrong-check-reflect/detect.py without polluting sys.path.""" - here = os.path.dirname(os.path.abspath(__file__)) - # scripts -> reflect -> skills -> engine -> repo - detect_path = os.path.normpath( - os.path.join(here, "..", "..", "..", "hooks", "wrong-check-reflect", "detect.py") - ) - spec = importlib.util.spec_from_file_location("wrong_check_reflect_detect", detect_path) - if spec is None or spec.loader is None: - return None - mod = importlib.util.module_from_spec(spec) - spec.loader.exec_module(mod) - return mod - - def self_retraction_hits(assistant_texts): - """Assistant-only first-person wrong-check admissions. Same detector as - engine/hooks/wrong-check-reflect/. Fail-open on import/scan errors.""" + """Assistant-only first-person wrong-check admissions, scanned offline. + + The wrong-check-reflect hook asks the background judge instead of matching + phrasings; this miner keeps a text scan so historical transcripts stay + minable without model calls. Fail-open on import/scan errors. + """ try: - mod = _load_wrong_check_detect() - if mod is None: - return [] - return list(mod.scan_assistant_texts(assistant_texts)) + import self_retraction_scan + + return list(self_retraction_scan.scan_assistant_texts(assistant_texts)) except Exception: return [] diff --git a/scripts/run_skill_scenarios.py b/scripts/run_skill_scenarios.py index 1b6d97d..3b70328 100755 --- a/scripts/run_skill_scenarios.py +++ b/scripts/run_skill_scenarios.py @@ -29,12 +29,18 @@ import importlib.util import json import sys +import os +import shutil import tempfile from pathlib import Path REPO_ROOT = Path(__file__).resolve().parents[1] SCENARIO_DIR = REPO_ROOT / "tests" / "scenarios" HOOK_DIR = REPO_ROOT / "engine" / "hooks" +HOOK_STATE_ENV = "WRONG_CHECK_REFLECT_STATE_DIR" +JUDGE_STATE_ENV = "CATSTACK_LLM_JUDGE_STATE_DIR" +JUDGE_RUNNERS_ENV = "CATSTACK_LLM_JUDGE_RUNNERS" +FAKE_JUDGE_RUNNER = ["scenario-fake", [sys.executable, "-c", "print('{\"match\": false}')", "{prompt}"]] _DETECT_CACHE: dict[str, object] = {} @@ -113,6 +119,44 @@ def hook_message(hook: str, scenario: dict, transcript_path: str) -> str | None: return decide(payload) +def enqueued(hook: str, scenario: dict, transcript_path: str) -> bool: + """True when the hook queued a judge job for this scenario.""" + module = load_detect(hook) + enqueue = getattr(module, "enqueue_judge", None) + if enqueue is None: + raise SystemExit(f"fail\t{hook}: detect.py exposes no enqueue_judge") + state_root = tempfile.mkdtemp(prefix="scenario-judge-") + hook_state = os.path.join(state_root, "hook") + judge_state = os.path.join(state_root, "judge") + os.makedirs(hook_state, exist_ok=True) + os.makedirs(judge_state, exist_ok=True) + saved = {k: os.environ.get(k) for k in (HOOK_STATE_ENV, JUDGE_STATE_ENV, JUDGE_RUNNERS_ENV)} + os.environ[HOOK_STATE_ENV] = hook_state + os.environ[JUDGE_STATE_ENV] = judge_state + os.environ[JUDGE_RUNNERS_ENV] = json.dumps([FAKE_JUDGE_RUNNER]) + previous_state_dir = getattr(module, "STATE_DIR", None) + module.STATE_DIR = hook_state + for cached in ("_judge", "_phrases"): + fn = getattr(module, cached, None) + if fn is not None and hasattr(fn, "cache_clear"): + fn.cache_clear() + try: + payload = { + "last_assistant_message": scenario.get("reply") or "", + "transcript_path": transcript_path, + } + return enqueue(payload) is not None + finally: + for key, value in saved.items(): + if value is None: + os.environ.pop(key, None) + else: + os.environ[key] = value + if previous_state_dir is not None: + module.STATE_DIR = previous_state_dir + shutil.rmtree(state_root, ignore_errors=True) + + def skill_frontmatter(name: str) -> str | None: for bucket in ("engine/skills", "corpus/skills", "product/skills"): md = REPO_ROOT / bucket / name / "SKILL.md" @@ -142,6 +186,16 @@ def check_scenario(scenario: dict, verbose: bool = False) -> list[str]: if msg: failures.append(f"{hook}: expected SILENCE, fired: {msg.splitlines()[0][:120]}") + for hook in scenario.get("expect_enqueue") or []: + if not enqueued(hook, scenario, path): + failures.append(f"{hook}: expected to ASK THE JUDGE, queued nothing") + elif verbose: + print(f" {hook} queued a judge job") + + for hook in scenario.get("expect_no_enqueue") or []: + if enqueued(hook, scenario, path): + failures.append(f"{hook}: expected NO judge job, queued one") + for skill in scenario.get("expect_skill_auto") or []: fm = skill_frontmatter(skill) if fm is None: diff --git a/tests/scenarios/self-correction.json b/tests/scenarios/self-correction.json index a798314..8d41bd4 100644 --- a/tests/scenarios/self-correction.json +++ b/tests/scenarios/self-correction.json @@ -1,28 +1,10 @@ [ { - "name": "admission-in-unlisted-wording-still-fires", - "situation": "The live miss from this session. A retraction phrased as 'a claim I made earlier was wrong' matched none of the eight enumerated patterns, so the hook stayed silent and the user had to play its role by hand. The user turn here must NOT mention /reflect: the hook deliberately skips when the user already asked for it.", + "name": "admission-in-unlisted-wording-still-asks-the-judge", + "situation": "The live miss from this session. The hook no longer matches phrasings; it must still hand this retraction to the judge.", "user": "is the stacked slice actually green?", "reply": "Also: a claim I made earlier was wrong. I told you the stacked slice was fully green, but that coverage run compared against origin/main rather than the slice, so the pass was vacuous.", - "expect_fire": [ - "wrong-check-reflect" - ] - }, - { - "name": "hypothetical-correction-stays-silent", - "situation": "A conditional about being wrong is not an admission; NEGATIVE_RES must keep precedence over the structural window.", - "user": "double-check that citation", - "reply": "If my earlier check was wrong we should redo it \u2014 want me to re-read the file at origin/main first?", - "expect_silent": [ - "wrong-check-reflect" - ] - }, - { - "name": "product-is-wrong-is-not-self-correction", - "situation": "Blaming the artifact is not retracting a claim. Guards the widened regex against the obvious false positive.", - "user": "the button is the wrong colour", - "reply": "The test was wrong, not the code \u2014 the fixture asserted the old brand green.", - "expect_silent": [ + "expect_enqueue": [ "wrong-check-reflect" ] }, @@ -40,7 +22,7 @@ "situation": "Pins the documented skip: ALREADY_REFLECT_RE suppresses the follow-up when the user's own message already asked for /reflect, so the hook does not nag for something already in flight. Caught by writing the scenario above with the user's literal message, which contained /reflect.", "user": "\"Claim I made was wrong\" is a trigger for /reflect", "reply": "Also: a claim I made earlier was wrong. The coverage run compared against origin/main rather than the slice, so the pass was vacuous.", - "expect_silent": [ + "expect_no_enqueue": [ "wrong-check-reflect" ] } diff --git a/tests/scenarios/verdict-flip.json b/tests/scenarios/verdict-flip.json index 98d11c2..8c0cc1d 100644 --- a/tests/scenarios/verdict-flip.json +++ b/tests/scenarios/verdict-flip.json @@ -1,36 +1,59 @@ [ { "name": "stale-green-caught-without-any-admission", - "situation": "The backstop layer. The same gate printed ok with its default scope and failed once given the slice refs, and the outgoing message reports the new result without saying the earlier one was wrong. Neither the regex layer nor the model noticing is involved — the transcript alone proves the earlier green is stale.", + "situation": "The backstop layer. The same gate printed ok with its default scope and failed once given the slice refs, and the outgoing message reports the new result without saying the earlier one was wrong. Neither the regex layer nor the model noticing is involved \u2014 the transcript alone proves the earlier green is stale.", "user": "verify the stacked slice", "ran": [ - {"cmd": "python3 scripts/check_skill_test_coverage.py", "output": "ok skill test coverage"}, - {"cmd": "python3 scripts/check_skill_test_coverage.py --base skill-scenarios --head HEAD", "output": "fail engine/skills/make-pr: changed without a corresponding test change"} + { + "cmd": "python3 scripts/check_skill_test_coverage.py", + "output": "ok skill test coverage" + }, + { + "cmd": "python3 scripts/check_skill_test_coverage.py --base skill-scenarios --head HEAD", + "output": "fail engine/skills/make-pr: changed without a corresponding test change" + } ], "reply": "Coverage for the slice needs a colocated test under engine/skills/make-pr/tests/. Adding one now.", - "expect_fire": ["verdict-flip-watch"], - "expect_silent": ["wrong-check-reflect"] + "expect_fire": [ + "verdict-flip-watch" + ] }, { "name": "stale-green-already-owned-by-the-reply", "situation": "Same transcript, but the reply names the earlier result as vacuous. The backstop stays quiet because the correction is already made.", "user": "verify the stacked slice", "ran": [ - {"cmd": "python3 scripts/check_skill_test_coverage.py", "output": "ok skill test coverage"}, - {"cmd": "python3 scripts/check_skill_test_coverage.py --base skill-scenarios --head HEAD", "output": "fail engine/skills/make-pr: changed without a corresponding test change"} + { + "cmd": "python3 scripts/check_skill_test_coverage.py", + "output": "ok skill test coverage" + }, + { + "cmd": "python3 scripts/check_skill_test_coverage.py --base skill-scenarios --head HEAD", + "output": "fail engine/skills/make-pr: changed without a corresponding test change" + } ], - "reply": "The earlier green was vacuous — that run compared against origin/main, not the slice. With the slice refs it fails, so the coverage claim I gave you was wrong.", - "expect_silent": ["verdict-flip-watch"] + "reply": "The earlier green was vacuous \u2014 that run compared against origin/main, not the slice. With the slice refs it fails, so the coverage claim I gave you was wrong.", + "expect_silent": [ + "verdict-flip-watch" + ] }, { "name": "red-then-green-is-a-fix-not-a-stale-claim", "situation": "A verifier that failed and then passed is ordinary repair work. Firing here would punish fixing things.", "user": "fix the failing hook test", "ran": [ - {"cmd": "python3 -m unittest discover -s engine/hooks/verdict-flip-watch/tests", "output": "FAILED (failures=1)"}, - {"cmd": "python3 -m unittest discover -s engine/hooks/verdict-flip-watch/tests", "output": "Ran 20 tests in 0.18s\n\nOK"} + { + "cmd": "python3 -m unittest discover -s engine/hooks/verdict-flip-watch/tests", + "output": "FAILED (failures=1)" + }, + { + "cmd": "python3 -m unittest discover -s engine/hooks/verdict-flip-watch/tests", + "output": "Ran 20 tests in 0.18s\n\nOK" + } ], - "reply": "Fixed — the classifier was treating mixed output as a pass. 20 tests pass now.", - "expect_silent": ["verdict-flip-watch"] + "reply": "Fixed \u2014 the classifier was treating mixed output as a pass. 20 tests pass now.", + "expect_silent": [ + "verdict-flip-watch" + ] } ] diff --git a/tests/test_skill_scenarios.py b/tests/test_skill_scenarios.py index aa42cdf..b4d9c90 100644 --- a/tests/test_skill_scenarios.py +++ b/tests/test_skill_scenarios.py @@ -28,7 +28,7 @@ def test_every_scenario_behaves_as_declared(self): def test_every_scenario_declares_at_least_one_expectation(self): """A scenario with no expectations passes trivially and proves nothing.""" - keys = ("expect_fire", "expect_silent", "expect_skill_auto", "expect_skill_named") + keys = ("expect_fire", "expect_silent", "expect_enqueue", "expect_no_enqueue", "expect_skill_auto", "expect_skill_named") for scenario in rs.load_scenarios(): with self.subTest(scenario=scenario["name"]): self.assertTrue(