fix(advisor): match signals as whole words with intent, not as prefixes - #49
Conversation
Audit recommendations 4 and 6 from the same run that produced #33 and #47. classify_task matched every signal as a bare PREFIX: the leading `(?<![a-z])` was there, the trailing boundary was not. With MIN_SIGNAL_HITS = 1, one substring is enough to bind a task type, so: "a read-only audit of the implementation of the config loader; do not change code" -> ['implement', 'review'] `implement` inside the noun "implementation" offered a code-mutating lane to work that must not touch code. `ui` likewise reached "uid", `test` reached "testgen". The rule is whole-word-with-intent: inflections that PRESERVE intent still count (SIGNAL_INFLECTIONS: plurals, participles, agent/result nouns), while derivational drift does not — above all `-ation`, which turns a verb into the name of a thing that already exists. Two-letter signals take no inflection, because they are initialisms and initialisms do not inflect. Without that carve-out `ui` still reached "uid" through the bare `-d` ending (which exists for the -e verbs: dedupe/deduped), so the boundary would have LOOKED like it fixed a false positive it had not. The rule was chosen from measurement, not intuition. Tested against a corpus of realistic task sentences, a naive trailing boundary caused three collateral losses: "run the testgen lane", "screenshot the output" and "formatting only" (gemination — `format` + t + ing is unreachable by any suffix rule). Each is restored by spelling the form out in TASK_SIGNALS, which is that table's existing idiom: it already lists "tests" beside "test" and "documentation" beside "docs". Re-measured after: exactly ONE behaviour change remains, the audit's own case. Also documents local_verify.py's precondition (recommendation 6): the fix must ALREADY be in the worktree. It is a phase-4 tool, so pointing it at a bare finding makes step 1 fail, and a step-1 failure means "your test command does not pass here", NOT "the finding is unreal" — the two read identically if you expected a verdict on the finding. Test gate: `python3 capability_advisor.py --selftest` pins the audit's case, the four inflected verb forms, the three restored signals, the initialism carve-out, and the -e verb keeping its bare -d, with a deliberate break -> revert on the trailing boundary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 34 minutes Limit details: You’ve used the included review currently available. Your 70 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
Audit recommendations 4 and 6 from the same run that produced #33 and #47.
The defect
classify_taskmatched every signal as a bare prefix. The leading(?<![a-z])was there; thetrailing boundary was not. With
MIN_SIGNAL_HITS = 1, a single substring is enough to bind a tasktype:
implement, inside the noun implementation, offered a code-mutating lane to work that must nottouch code.
uilikewise reached "uid";testreached "testgen".The rule
Whole-word-with-intent. Inflections that preserve intent still count (
SIGNAL_INFLECTIONS—plurals, participles, agent/result nouns); derivational drift does not, above all
-ation, whichturns a verb into the name of a thing that already exists. That is the distinction that matters here:
"implement the exporter" is work to do, "the implementation of the loader" is a noun in a
read-only audit.
Two-letter signals take no inflection, because they are initialisms and initialisms do not
inflect. Without that carve-out
uistill reached "uid" through the bare-dending (which existsfor the
-everbs: dedupe → deduped), so the trailing boundary would have looked like it fixed afalse positive it had not fixed.
The rule was measured, not guessed
Tested against a corpus of realistic task sentences. A naive trailing boundary caused three
collateral losses:
run the testgen lanetestgen— "testgen" is nottest+ an inflectionscreenshot the outputux_reviewformatting onlymechanical— gemination:format+ t +ingis unreachable by any suffix ruleEach is restored by spelling the form out in
TASK_SIGNALS, which is that table's existing idiom —it already lists
testsbesidetestanddocumentationbesidedocs. Re-measured afterwards:exactly one behaviour change remains, the audit's own case.
deduped the rowsstill classifies(the
-everb keeps its bare-d), andcheck the uid field/uxbridge stationnow classify asnothing.
Also: recommendation 6
Documents
local_verify.py's precondition — the fix must already be in the worktree. It is aphase-4 tool, so pointing it at a bare finding makes step 1 fail, and a step-1 failure means "your
test command does not pass here", not "the finding is unreal". The two read identically if you
were expecting a verdict on the finding, which is how a real audit run reached for it one phase
early. Docstring only; no behaviour change.
Test gate
python3 capability_advisor.py --selftestpins the audit's case, the four inflected verb forms(
implement/implementing/implements/implemented), the three restored signals, the initialismcarve-out, and the
-everb keeping its bare-d— with a deliberate break → revert on thetrailing boundary asserting
"break did not change behaviour — test is vacuous".Blast radius
capability_advisoris a live rail: #37 made the tick consult it. Kept in its own PR, separate from#47, so it can be reverted independently.
Verification
python3 verify.pywith both changes: 368 passed, 0 failed, 83/83 selftests, 5 of 5 gates green,zero skips.
🤖 Generated with Claude Code