feat(directive_detection): Path A intent-prefix filter (#374) - #467
Conversation
Pre-filter the leading clause: imperative coding-task verbs in head position short-circuit detect_directive() to False unless a rule-marker connective is also present. Targets the dominant FP cluster from lab corpus v0.1 (P=0.664 / R=0.937) — imperative-grammar one-shot coding tasks like "Refactor X so it never blocks" or "Add a test that ensures …" that fire on the 29-verb regex but encode session tasks, not durable rules. Verb bank (16 head-position verbs, ratified 2026-05-07 on #374): refactor, add, implement, write, create, update, fix, make, build, remove, rename, extract, merge, split, move, delete. Rule-marker connective list: empty (ratified 2026-05-07). Coding-task prefix always wins on first iteration; expand only with corpus evidence. The pattern is precompiled to None when the list is empty so the hot path skips the search. Spec: docs/v2_directive_detection.md (PR #466). Iteration target is precision; recall has 23 percentage points of headroom over the 0.60 floor. Closes the iteration-spec gap; does not close #374. Re-entry still requires P≥0.80 ∧ R≥0.60 ∧ n≥200 from a lab-side bench-gate run against AELFRICE_CORPUS_ROOT/directive_detection/v0_1.jsonl.
Two parametrized test functions: - test_directive_coding_task_prefix_short_circuits — 16 rows, one per head-position verb in the ratified bank. Each row deliberately embeds a downstream imperative-bank verb (never, ensure, must, only, avoid, …) so the test would have passed under the pre-Path-A detector if the short-circuit weren't applied; the row is load-bearing for the new branch. - test_directive_prefix_filter_does_not_swallow_rules — regression: durable rules with a non-coding-task head verb (always, never, must, only, before) still classify True. Confirms the prefix filter is positional, not a verb-bank shrinker. 39 sanity tests pass locally (full suite: 2616 pass, 41 skip).
Reviewer's GuideAdds a Path A intent-prefix filter to detect_directive() to suppress one-shot coding-task imperatives when they appear as head-position verbs, along with tests that ensure the new prefix filter short-circuits non-durable coding tasks while preserving durable rule detection. Class diagram for directive_detector module with Path A prefix filterclassDiagram
class directive_detector {
<<module>>
+_CODING_TASK_PREFIX_VERBS : tuple~str~
+_CODING_TASK_PREFIX_PATTERN : re.Pattern~str~
+_RULE_MARKER_CONNECTIVES : tuple~str~
+_RULE_MARKER_PATTERN : re.Pattern~str~ | None
+_HEDGE_PATTERN : re.Pattern~str~
+_VERB_PATTERN : re.Pattern~str~
+detect_directive(text: str) bool
}
class _CODING_TASK_PREFIX_VERBS_details {
<<value>>
refactor
add
implement
write
create
update
fix
make
build
remove
rename
extract
merge
split
move
delete
}
directive_detector --> _CODING_TASK_PREFIX_VERBS_details : defines
Flow diagram for updated detect_directive intent-prefix filterflowchart TD
start([Start detect_directive])
strip[Strip input text]
empty{Is text empty or whitespace?}
question{Matches question pattern?}
narration{Matches narration pattern?}
hedge{Matches hedge pattern?}
prefix{Matches _CODING_TASK_PREFIX_PATTERN at head?}
ruleMarkerConfigured{Is _RULE_MARKER_PATTERN configured?}
ruleMarkerPresent{Does _RULE_MARKER_PATTERN match?}
verbMatch{Matches _VERB_PATTERN?}
trueResult([Return True])
falseResult([Return False])
start --> strip --> empty
empty -- Yes --> falseResult
empty -- No --> question
question -- Yes --> falseResult
question -- No --> narration
narration -- Yes --> falseResult
narration -- No --> hedge
hedge -- Yes --> falseResult
hedge -- No --> prefix
prefix -- No --> verbMatch
prefix -- Yes --> ruleMarkerConfigured
ruleMarkerConfigured -- No --> falseResult
ruleMarkerConfigured -- Yes --> ruleMarkerPresent
ruleMarkerPresent -- No --> falseResult
ruleMarkerPresent -- Yes --> verbMatch
verbMatch -- Yes --> trueResult
verbMatch -- No --> falseResult
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds a coding-task intent-prefix filter to ChangesCoding-Task Prefix Filter
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/aelfrice/directive_detector.py" line_range="102-105" />
<code_context>
+ "fix", "make", "build", "remove", "rename", "extract",
+ "merge", "split", "move", "delete",
+)
+_CODING_TASK_PREFIX_PATTERN = re.compile(
+ r"^\s*(?:" + "|".join(_CODING_TASK_PREFIX_VERBS) + r")\b",
+ re.IGNORECASE,
+)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Escape the verbs when building the prefix pattern to future‑proof against special regex characters.
Currently this is safe because all verbs are simple alphabetic tokens, but if a future verb includes regex metacharacters (e.g. `C++`, `.net`, or a hyphen), the pattern could behave incorrectly. To make this robust, build the alternation with `"|".join(re.escape(v) for v in _CODING_TASK_PREFIX_VERBS)` as you did for `_RULE_MARKER_CONNECTIVES`.
```suggestion
_CODING_TASK_PREFIX_PATTERN = re.compile(
r"^\s*(?:" + "|".join(re.escape(v) for v in _CODING_TASK_PREFIX_VERBS) + r")\b",
re.IGNORECASE,
)
```
</issue_to_address>
### Comment 2
<location path="tests/test_directive_detector.py" line_range="55-64" />
<code_context>
+# Regression: leading deontic anchors and durable rules where the head verb
+# is NOT in the coding-task bank still classify True. The prefix filter is
+# case-insensitive but positional, and only fires on the head verb.
+@pytest.mark.parametrize(
+ "text",
+ [
+ "always update the changelog before tagging",
+ "never delete a worktree without releasing the claim first",
+ "must rename the temp file before commit",
+ "only merge after the gate passes",
+ "before merging, ensure CI is green",
+ ],
+)
+def test_directive_prefix_filter_does_not_swallow_rules(text: str) -> None:
+ assert detect_directive(text) is True
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding cases with leading whitespace/punctuation before the coding-task verb to lock in the intended anchoring behavior.
The `_CODING_TASK_PREFIX_PATTERN` already supports leading whitespace, but the current parametrization only covers verbs at column 0. Adding a couple of cases like `" refactor X so it never blocks"` or `"\tAdd a test that ensures …"` would validate that the prefix filter still behaves correctly with common leading-whitespace formats and guard against regressions if the regex is changed later.
</issue_to_address>
### Comment 3
<location path="tests/test_directive_detector.py" line_range="57-66" />
<code_context>
+# case-insensitive but positional, and only fires on the head verb.
+@pytest.mark.parametrize(
+ "text",
+ [
+ "always update the changelog before tagging",
+ "never delete a worktree without releasing the claim first",
+ "must rename the temp file before commit",
+ "only merge after the gate passes",
+ "before merging, ensure CI is green",
+ ],
+)
+def test_directive_prefix_filter_does_not_swallow_rules(text: str) -> None:
+ assert detect_directive(text) is True
</code_context>
<issue_to_address>
**suggestion (testing):** Add negative controls where coding-task verbs appear non-initially so we prove the filter is strictly head-position based.
Since `detect_directive` is explicitly head-verb positional, add a few cases where a non-coding verb appears first and the coding-task verb appears later (e.g. `"before merging, add a test that ensures …"`). These should still be `True` to validate that the filter only keys off the head verb and ignores non-head occurrences of coding-task verbs.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| _CODING_TASK_PREFIX_PATTERN = re.compile( | ||
| r"^\s*(?:" + "|".join(_CODING_TASK_PREFIX_VERBS) + r")\b", | ||
| re.IGNORECASE, | ||
| ) |
There was a problem hiding this comment.
suggestion (bug_risk): Escape the verbs when building the prefix pattern to future‑proof against special regex characters.
Currently this is safe because all verbs are simple alphabetic tokens, but if a future verb includes regex metacharacters (e.g. C++, .net, or a hyphen), the pattern could behave incorrectly. To make this robust, build the alternation with "|".join(re.escape(v) for v in _CODING_TASK_PREFIX_VERBS) as you did for _RULE_MARKER_CONNECTIVES.
| _CODING_TASK_PREFIX_PATTERN = re.compile( | |
| r"^\s*(?:" + "|".join(_CODING_TASK_PREFIX_VERBS) + r")\b", | |
| re.IGNORECASE, | |
| ) | |
| _CODING_TASK_PREFIX_PATTERN = re.compile( | |
| r"^\s*(?:" + "|".join(re.escape(v) for v in _CODING_TASK_PREFIX_VERBS) + r")\b", | |
| re.IGNORECASE, | |
| ) |
| @pytest.mark.parametrize( | ||
| "text", | ||
| [ | ||
| "Refactor X so it never blocks", | ||
| "Add a test that ensures the gate fires", | ||
| "Implement the parser so it must reject empty input", | ||
| "Write a guard that always returns False on the empty case", | ||
| "Create a wrapper that should not propagate exceptions", | ||
| "Update the README so it only mentions the public API", | ||
| "Fix the pre-push hook to avoid bypassing on rebase", |
There was a problem hiding this comment.
suggestion (testing): Consider adding cases with leading whitespace/punctuation before the coding-task verb to lock in the intended anchoring behavior.
The _CODING_TASK_PREFIX_PATTERN already supports leading whitespace, but the current parametrization only covers verbs at column 0. Adding a couple of cases like " refactor X so it never blocks" or "\tAdd a test that ensures …" would validate that the prefix filter still behaves correctly with common leading-whitespace formats and guard against regressions if the regex is changed later.
| [ | ||
| "Refactor X so it never blocks", | ||
| "Add a test that ensures the gate fires", | ||
| "Implement the parser so it must reject empty input", | ||
| "Write a guard that always returns False on the empty case", | ||
| "Create a wrapper that should not propagate exceptions", | ||
| "Update the README so it only mentions the public API", | ||
| "Fix the pre-push hook to avoid bypassing on rebase", | ||
| "Make the worker shutdown ensure no half-flushed batches", | ||
| "Build a fixture that requires the v0.1 corpus path", |
There was a problem hiding this comment.
suggestion (testing): Add negative controls where coding-task verbs appear non-initially so we prove the filter is strictly head-position based.
Since detect_directive is explicitly head-verb positional, add a few cases where a non-coding verb appears first and the coding-task verb appears later (e.g. "before merging, add a test that ensures …"). These should still be True to validate that the filter only keys off the head verb and ignores non-head occurrences of coding-task verbs.
|
[claim:review:Gylf:2026-05-07T20:14:52Z] |
|
[claim:review:Toug:2026-05-07T20:14:59Z] |
|
[release:review:Toug:2026-05-07T20:15:04Z] |
|
[claim:review:Setr:2026-05-07T20:16:32Z] |
|
[release:review:Setr:2026-05-07T20:16:37Z] |
|
Merged via FF push (no merge commit). Both commits signed, all green CI checks, discretion grep clean. Non-blocking nit (post-merge, for future awareness): one short-circuit row in Worth fixing in a follow-up only if the row inventory changes. |
|
[release:review:Gylf:2026-05-07T20:17:00Z] |
…ding (#1341) Both docs carried P=0.664 as the detector's standing measurement. That was the pre-Path-A number; Path A shipped in #467 and the confirming re-run this memo asked for never happened. The shipped detector measures P=0.706 / R=0.937, and Path A removed 8 false positives rather than the ~45 estimated. The larger correction is that the gate cannot presently certify anything: v0.1 separates its classes by opening vocabulary, so head-position rules buy free precision. Records the measurement, replaces the monolithic-cluster failure analysis with the six families actually present, and makes corpus v0.2 the blocking item ahead of any further detector iteration.
What
Implements the Path A intent-prefix filter ratified on issue #374 (2026-05-07). Adds a head-position lexical pre-filter to
detect_directive()so imperative coding-task verbs short-circuit toFalseunless a rule-marker connective is also present. Targets the monolithic FP cluster identified against lab corpus v0.1 (P=0.664 / R=0.937) — imperative-grammar one-shot coding tasks like "Refactor X so it never blocks" that the 29-verb regex misclassifies.Why
The H1 enforcement gate is
P ≥ 0.80 ∧ R ≥ 0.60 ∧ n ≥ 200. Current measurement is below the precision floor with 23 percentage points of recall headroom — the iteration is a precision problem. Failure-mode analysis on the 45 FPs showed a single dominant cluster: imperatives the user issues to the agent for the immediate session, not durable rules. Path A isolates that cluster on a structural property (head-verb position) without weakening the imperative bank or breaking determinism.Changes
src/aelfrice/directive_detector.py(+34, -1)_CODING_TASK_PREFIX_VERBS(16 verbs) and_CODING_TASK_PREFIX_PATTERN(anchored regex)._RULE_MARKER_CONNECTIVES(empty per feat(hook): directive detection — #199 H1 split (deferred, bench-gated) #374 decision) and_RULE_MARKER_PATTERN(precompiled toNonewhen the list is empty so the hot path skips the search).detect_directive()between hedge check and verb check; short-circuit toFalsewhen prefix matches and no connective is registered or present.tests/test_directive_detector.py(+47)test_directive_coding_task_prefix_short_circuits— 16 rows, one per head-position verb. Each embeds a downstream imperative-bank verb so the row would pass under the pre-Path-A detector if the short-circuit weren't applied; load-bearing for the new branch.test_directive_prefix_filter_does_not_swallow_rules— regression: rules with a non-coding-task head verb (always, never, must, only, before) still classify True.Decision provenance
All four iteration-spec asks ratified on the issue thread (2026-05-07):
mainVerification
uv run pytest tests/test_directive_detector.py— 39 passed.uv run pytest— 2616 passed, 41 skipped, no regressions.feat(directive_detection):+test(directive_detection):).Out of scope
process_directive, the TODO lifecycle, the repetition counter, the escalation table, hook wiring. All gated on the bench gate passing perdocs/v2_enforcement.md§ H1.Closes / refs
Refs #374, #199 (umbrella). Does not close #374 — that requires a passing bench-gate run against
AELFRICE_CORPUS_ROOT/directive_detection/v0_1.jsonl(now landed on labmain) demonstrating P ≥ 0.80 ∧ R ≥ 0.60 ∧ n ≥ 200. Lab-side closing PR strikes the H1 row fromdocs/V2_REENTRY_QUEUE.mdand posts the bench numbers in the PR body.Summary by Sourcery
Introduce a head-position coding-task prefix filter to improve directive detection precision while preserving durable rule detection.
New Features:
Enhancements:
Tests:
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests