Skip to content

feat(ci): fail when a phrase checker has nothing calling it - #480

Merged
EdbertChan merged 1 commit into
mainfrom
stack/EdbertChan/feat/diu-jargon-ban-20260911/fail-phrase-checker-nothing-calling--dab82b59
Sep 12, 2026
Merged

feat(ci): fail when a phrase checker has nothing calling it#480
EdbertChan merged 1 commit into
mainfrom
stack/EdbertChan/feat/diu-jargon-ban-20260911/fail-phrase-checker-nothing-calling--dab82b59

Conversation

@EdbertChan

@EdbertChan EdbertChan commented Sep 12, 2026

Copy link
Copy Markdown
Owner

Summary

A rule can be written in this repo and have nothing calling it. It then reads as
covered in review and never fires.

The build now fails when that happens. A word-list rule has to be reachable from
code, or say in writing why it ships switched off.

Nothing is switched off by this change and no rule text changes. It only adds a
check.

Review Claim

The build fails unless every phrase checker is named in quotes by a non-test
file, discovered at runtime by a file in its own hook that lists the phrases
directory, or declares an unwired_reason.

Review Lane

behavior

Review Unit

engine-runtime

Safety Invariant

The check only reads files and returns an exit code. It runs in CI, imports
nothing from the hooks it inspects, and changes no runtime behaviour, so the
worst failure it can cause is a red build.

Slice Rationale

The check ships alone because everything it inspects already exists and already
passes. It needed no companion change, which is also why it could be verified
against the real tree rather than a fixture.

Non-goals

  • Adds no wordlist and changes no rule text. Judgement stays with the model.
  • Does not cover hooks or skills; test_install.py and the three-harness gate
    already do those.
  • Does not check that a caller is ever actually reached at runtime, only that
    one exists.

Test Plan

Test Plan
  • python3 tests/test_rules_are_wired.py
  • python3 scripts/check_rules_are_wired.py
  • python3 tests/test_check_docstrings_match_regexes.py

Two wrong answers were found while building this, both now pinned by tests.

First, matching the name anywhere in a file passed example off three files that
only used the word in prose:

example -> ['engine/hooks/scratchpad-collision/detect.py',
            'engine/hooks/llm-judge/phrases.py',
            'engine/hooks/no-comments/detect.py']

Requiring the quoted form then failed all five plain-words-* checkers, which
are genuinely wired — diu-stop/plain_words.py loads them by listing the
directory, so it names none of them. A check that only understood literals would
have failed the repo's own working code.

Final behaviour on the real tree:

dynamic plain-words-code-names: loaded by engine/hooks/diu-stop/plain_words.py
dynamic plain-words-internal-names: loaded by engine/hooks/diu-stop/plain_words.py
dynamic plain-words-made-up-labels: loaded by engine/hooks/diu-stop/plain_words.py
dynamic plain-words-status-words: loaded by engine/hooks/diu-stop/plain_words.py
dynamic plain-words-tech-jargon: loaded by engine/hooks/diu-stop/plain_words.py
opt-out example: the worked example for llm-judge's README ...
check_rules_are_wired: OK (7 checker(s) checked)

The promised catch and allow exemplars, run:

CATCH (must all be True):
  no-caller            -> True
  prose-mention-only   -> True
  empty-unwired-reason -> True
  no-checker-name      -> True
ALLOW (must all be False):
  quoted-caller         -> False
  stated-unwired-reason -> False
  runtime-discovery     -> False

Suites:

Ran 18 tests in 4.035s    OK    (the check)
Ran 5 tests in 1.561s     OK    (the meta-gate that audits every gate)

Fixture vs live: the check runs against this repo's own tree above, not a
fixture. It has no runtime side effects to exercise.

Revert Plan

Revert Plan
  • Safe to revert? Yes
  • Revert command: git revert <sha>
  • Post-revert steps: None. The build stops checking; nothing else changes.
  • Data migration? No

catstack enforces that a HOOK is wired -- test_install.py's
TestEveryClaudeHookScriptIsWired caught exactly that on a new hook earlier
today. Nothing enforced it for phrase checkers, so a rule could ship as data
with no caller, read as covered in review, and never fire.

scripts/check_rules_are_wired.py fails unless each checker is either named in
quotes by a non-test file, discovered at runtime by a file in its own hook that
enumerates the phrases directory, or declares an unwired_reason. That last one
mirrors the repo's subagent_stop.inherit:false + reason opt-out: dormant is
allowed, silent is not. Wired into CI beside the hook coverage gate.

Two false answers were found and fixed while building it, both pinned by tests:

- A bare substring match passed `example` off three files that only used the
  word in prose (scratchpad-collision/detect.py, llm-judge/phrases.py,
  no-comments/detect.py). The pattern now requires the quoted form;
  test_an_unquoted_prose_mention_is_not_a_caller holds that.
- Requiring the quoted form then failed all five plain-words checkers, which
  are genuinely wired: diu-stop/plain_words.py loads them by listing the
  directory, so it names none of them. A gate that only understood literals
  would have failed the repo's own working code.
  test_a_hook_that_lists_its_phrases_dir_wires_all_of_them holds that, and
  test_mentioning_phrases_without_listing_it_is_not_wiring holds the boundary.

On the real tree the gate now reports the five as dynamic and flags only
llm-judge/phrases/example.json, which has no caller by design and now says so.

18 gate tests, 5 meta-gate tests, preflight green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018u8S5ct3kFhosinSbybc7W
Change-Id: Idab82b59bdeef9499f5af5956e23b5be9e32b26b
@cursor

cursor Bot commented Sep 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_61dded38-074e-43cf-8f9f-5177dac8d57d)

@EdbertChan
EdbertChan merged commit 2754b47 into main Sep 12, 2026
4 checks passed
@EdbertChan

Copy link
Copy Markdown
Owner Author

@Mergifyio queue

@mergify

mergify Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • 🟠 Waiting for queue conditions
  • ⏳ Enter queue
  • ⏳ Run checks
  • ⏳ Merge
Waiting for
  • -closed [📌 queue requirement]
All conditions
  • -closed [📌 queue requirement]
  • -conflict [📌 queue requirement]
  • -draft [📌 queue requirement]
  • any of [📌 queue -> configuration change requirements]:
    • -mergify-configuration-changed
    • check-success = @mergify/Configuration changed
    • check-success = @mergify/Configuration has been deleted
  • any of [🔀 queue conditions]:
    • all of [📌 queue conditions of queue rule admin-bypass]:
      • -draft
      • base=main
      • label=admin-bypass

EdbertChan added a commit that referenced this pull request Sep 12, 2026
Resolve the add/add conflicts by keeping main's files. The task's work
already landed through #444, and main's versions are newer: example.json
carries the unwired_reason that #480's checker requires.

Drop the cursor_post_tool_use.py -> cursor_posttooluse.py renames in
bug-complaint-leak and repeat-error-stop from d654dce. A read-only verify
task made them, and main's repeat-error-stop/tests/test_hooks.py imports
cursor_post_tool_use, so the renames would break that test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant