From 90c66e24b046f4dcd12d5625448eeb0dd2dd9e8f Mon Sep 17 00:00:00 2001 From: Edbert Chan Date: Sun, 11 Oct 2026 06:28:16 -0700 Subject: [PATCH] Drop the lesson-router comments that failed the comment gate. The boundary-parser allow mark stays, and the comment checker treats that mark as a machine directive so the two gates agree. Co-authored-by: Cursor Change-Id: I0a048d4d387df53de81d2638521a5198de499b36 --- engine/hooks/no-comments/detect.py | 2 +- engine/hooks/no-comments/tests/test_hooks.py | 2 +- engine/skills/reflect/scripts/lesson_route.py | 18 ++---------------- .../reflect/scripts/tests/test_lesson_route.py | 1 - 4 files changed, 4 insertions(+), 19 deletions(-) diff --git a/engine/hooks/no-comments/detect.py b/engine/hooks/no-comments/detect.py index 17f73ff42..fc4ee4e28 100644 --- a/engine/hooks/no-comments/detect.py +++ b/engine/hooks/no-comments/detect.py @@ -53,7 +53,7 @@ TRAILING_SLASH_RE = re.compile(r"\s//\s*\S") TRAILING_DIRECTIVE_RE = re.compile( r"\s(?:#|//)\s*(?:noqa|type:|pragma|pylint|mypy|ruff|fmt:|nosec|isort|nolint|" - r"eslint|prettier|@ts-|tslint|istanbul|biome|swiftlint|NOSONAR)", + r"eslint|prettier|@ts-|tslint|istanbul|biome|swiftlint|NOSONAR|text-match-decision-warn:)", re.IGNORECASE, ) RULE_ADDED_COMMENT = "no-comments.added-comment" diff --git a/engine/hooks/no-comments/tests/test_hooks.py b/engine/hooks/no-comments/tests/test_hooks.py index 211e0c528..0de7181dc 100644 --- a/engine/hooks/no-comments/tests/test_hooks.py +++ b/engine/hooks/no-comments/tests/test_hooks.py @@ -133,7 +133,7 @@ def test_blocks_write_and_multiedit_shapes(self): class TestAllows(unittest.TestCase): def test_allows_directives_and_shebang(self): - text = "#!/usr/bin/env python3\n# -*- coding: utf-8 -*-\nimport x # noqa: E402\ny = 1 # type: ignore\n# pragma: no cover\n" + text = "#!/usr/bin/env python3\n# -*- coding: utf-8 -*-\nimport x # noqa: E402\ny = 1 # type: ignore\n# pragma: no cover\nz = pat.search(text) # text-match-decision-warn: allow\n" self.assertEqual(detect.comment_lines("/r/a.py", text), []) js = "// eslint-disable-next-line no-console\n// @ts-ignore\n/* istanbul ignore next */\n// SPDX-License-Identifier: MIT\n" self.assertEqual(detect.comment_lines("/r/a.ts", js), []) diff --git a/engine/skills/reflect/scripts/lesson_route.py b/engine/skills/reflect/scripts/lesson_route.py index e3d54c64d..7a79cecc2 100644 --- a/engine/skills/reflect/scripts/lesson_route.py +++ b/engine/skills/reflect/scripts/lesson_route.py @@ -37,7 +37,6 @@ BUCKETS = (ALREADY_COVERED, SKILL_NOT_USED, AUTOMATE_ME, ACCEPTED) -# Assistant admits the skill/rule existed and was not followed. SKIP_ADMISSION = re.compile( r"\bi skipped (it|that|the skill|the rule)\b" r"|\bit is;?\s*i skipped\b" @@ -46,7 +45,6 @@ re.I, ) -# Human reminds the agent of a rule that already exists. RESTATED_RULE_SHAPE = re.compile( r"\bi thought that was a skill\b" r"|\bisn'?t there already a (rule|skill)\b" @@ -57,7 +55,6 @@ re.I, ) -# Working preference that is not "the rule already exists." PREFERENCE_SHAPE = re.compile( r"\b(always|never|from now on|by default|don'?t ask)\b" r"|\bwe don'?t have a rule for this yet\b" @@ -67,8 +64,8 @@ ) SESSION_SPECIFIC = re.compile( - r"\b[0-9a-f]{7,40}\b" # SHAs - r"|/(?:Users|home|tmp)/[^\s]+" # absolute paths + r"\b[0-9a-f]{7,40}\b" + r"|/(?:Users|home|tmp)/[^\s]+" r"|\b(?:ticker|case|pr)\s*#?\d+\b", re.I, ) @@ -133,16 +130,12 @@ def route( kind = (kind or "").strip().lower().replace("_", "-") reasons = [] - # Boundary parse: transcript prose → typed admitted_skip. Callers may also - # pass assistant_reply="" and set kind/skill_text from structured fields. admitted_skip = bool(SKIP_ADMISSION.search(assistant_reply)) # text-match-decision-warn: allow restated = kind in ("restated-rule", "restated") or bool( RESTATED_RULE_SHAPE.search(quote) # text-match-decision-warn: allow ) skill_covers = _skill_states_rule(skill_text, quote) if skill_text else False - # 1. Restated existing rule / admitted skip → already-covered. - # By definition of the restated-rule checker, the rule already exists. if admitted_skip or restated or (skill_covers and not _truthy_preference(kind, quote)): if admitted_skip: reasons.append("assistant admitted the skill/rule exists and was skipped") @@ -160,7 +153,6 @@ def route( "rationale": "; ".join(reasons) + "; do not route to automate-me", } - # 2. Named skill never opened in the transcript. if skill_used is False: if missed_trigger: return { @@ -174,7 +166,6 @@ def route( "rationale": "transcript never opened the named skill; reject as skill-not-used", } - # 3. Working preference with no existing skill home. if _truthy_preference(kind, quote) and not skill_covers: return { "bucket": AUTOMATE_ME, @@ -182,7 +173,6 @@ def route( "rationale": "working preference with no existing skill stating it", } - # 4. Accepted only when durable and decision-changing, and not a one-off. session_specific = bool(SESSION_SPECIFIC.search(quote)) if durable and decision_changing and not session_specific: return { @@ -250,13 +240,10 @@ def findings_from_transcript(path): pending_quote = None for row in rows: msg = row.get("message") or {} - # Claude session JSONL often sets role only via top-level `type` - # (`"type": "assistant"`) and omits `message.role`. role = msg.get("role") or row.get("type") text = _claude_text_blocks(row).strip() if not text: continue - # Skip tool_result-only user rows and harness noise. if role == "user": if text.startswith("[Request interrupted"): continue @@ -298,7 +285,6 @@ def route_transcript(path, **kwargs): ) findings.append({**pair, **result}) - # Strictest wins: already-covered > skill-not-used > automate-me > accepted. rank = {ALREADY_COVERED: 0, SKILL_NOT_USED: 1, AUTOMATE_ME: 2, ACCEPTED: 3} best = min(findings, key=lambda f: rank.get(f["bucket"], 99)) return { diff --git a/engine/skills/reflect/scripts/tests/test_lesson_route.py b/engine/skills/reflect/scripts/tests/test_lesson_route.py index 48e2689f9..cb298a5d9 100644 --- a/engine/skills/reflect/scripts/tests/test_lesson_route.py +++ b/engine/skills/reflect/scripts/tests/test_lesson_route.py @@ -21,7 +21,6 @@ SCRIPT = os.path.join(SCRIPTS, "lesson_route.py") FIRES = os.path.join(FIXTURES, "restated_rule", "fires.jsonl") -# SCRIPTS = .../engine/skills/reflect/scripts → reflect → skills → engine → repo REPO = os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(SCRIPTS)))) PHRASE = os.path.join(REPO, "engine", "hooks", "llm-judge", "phrases", "restated-rule.json") SKILL_MD = os.path.join(os.path.dirname(SCRIPTS), "SKILL.md")