feat(directive_detection): bench gate + candidate detector for #374 H1 - #377
Merged
Merged
Conversation
Reviewer's GuideAdds a candidate regex-based directive detector plus a benchmark gate and corpus schema wiring so H1 directive-detection can be quantitatively evaluated against a labeled corpus before implementation unblocks. Sequence diagram for bench gate evaluation of directive detectionsequenceDiagram
actor Dev
participant Pytest as pytest
participant BenchTest as test_directive_detection
participant CorpusLoader as corpus_loader
participant Detector as directive_detector
participant Metrics as metrics
Dev->>Pytest: run tests with AELFRICE_CORPUS_ROOT set
Pytest->>BenchTest: execute bench gate test
BenchTest->>CorpusLoader: load labeled prompts from corpus
CorpusLoader-->>BenchTest: prompts_with_labels
loop for each prompt
BenchTest->>Detector: detect_directive(text)
Detector-->>BenchTest: is_directive
BenchTest->>Metrics: record prediction and gold label
end
BenchTest->>Metrics: compute precision, recall, n
Metrics-->>BenchTest: P, R, n
BenchTest->>BenchTest: assert P >= 0.80 and R >= 0.60 and n >= 200
BenchTest-->>Pytest: pass or fail based on gate
Pytest-->>Dev: report test results
Sequence diagram for detect_directive filtering logicsequenceDiagram
participant Caller
participant Detector as directive_detector
Caller->>Detector: detect_directive(text)
alt empty or whitespace
Detector-->>Caller: False
else nonempty
Detector->>Detector: stripped = text.strip()
alt stripped endswith ?
Detector-->>Caller: False
else wh question leading
Detector-->>Caller: False
else narration pattern matches
Detector-->>Caller: False
else hedge pattern matches
Detector-->>Caller: False
else imperative verb pattern matches
Detector-->>Caller: True
else no imperative verb
Detector-->>Caller: False
end
end
Class diagram for directive_detector module structureclassDiagram
class directive_detector {
<<module>>
+tuple~str~ _IMPERATIVE_VERBS
+Pattern _VERB_PATTERN
+Pattern _HEDGE_PATTERN
+Pattern _NARRATION_PATTERN
+Pattern _WH_QUESTION_LEADING
+bool detect_directive(text: str)
}
class re {
<<stdlib_module>>
+Pattern compile(pattern: str, flags: int)
+str escape(text: str)
}
directive_detector ..> re : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR implements directive detection functionality for issue ChangesDirective Detection Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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. Review rate limit: 0/1 reviews remaining, refill in 58 minutes and 56 seconds.Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The precision/recall thresholds and minimum-row gate are hard-coded in the bench test; consider centralizing these (or importing from a single config/module) to avoid silent drift from the values documented in
docs/v2_enforcement.md§ H1. - The narration regex currently keys off any first-person deontic phrase followed by a linker (e.g.
I must X when Y), which may be stricter than necessary; if the corpus shows over-filtering, consider tightening it to focus on clearly habitual formulations (e.g. including explicit frequency markers) rather than all such conditionals.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The precision/recall thresholds and minimum-row gate are hard-coded in the bench test; consider centralizing these (or importing from a single config/module) to avoid silent drift from the values documented in `docs/v2_enforcement.md` § H1.
- The narration regex currently keys off any first-person deontic phrase followed by a linker (e.g. `I must X when Y`), which may be stricter than necessary; if the corpus shows over-filtering, consider tightening it to focus on clearly habitual formulations (e.g. including explicit frequency markers) rather than all such conditionals.
## Individual Comments
### Comment 1
<location path="src/aelfrice/directive_detector.py" line_range="63-65" />
<code_context>
+
+# Build a single alternation regex. Sort by length desc so multi-word
+# phrases match before their single-word prefixes.
+_VERB_PATTERN = re.compile(
+ r"\b(?:" + "|".join(re.escape(v) for v in sorted(_IMPERATIVE_VERBS, key=len, reverse=True)) + r")\b",
+ re.IGNORECASE,
+)
+
</code_context>
<issue_to_address>
**issue:** Contraction handling is limited to ASCII apostrophes, which misses common Unicode variants in real inputs.
This will only match verbs with straight ASCII apostrophes ("don't", "can't"), so inputs using curly or other Unicode apostrophes (e.g., “don’t”) will be missed. If production text may include those, either normalize input (map Unicode quotes to ASCII before matching) or extend the regex to include common Unicode apostrophe characters.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+63
to
+65
| _VERB_PATTERN = re.compile( | ||
| r"\b(?:" + "|".join(re.escape(v) for v in sorted(_IMPERATIVE_VERBS, key=len, reverse=True)) + r")\b", | ||
| re.IGNORECASE, |
There was a problem hiding this comment.
issue: Contraction handling is limited to ASCII apostrophes, which misses common Unicode variants in real inputs.
This will only match verbs with straight ASCII apostrophes ("don't", "can't"), so inputs using curly or other Unicode apostrophes (e.g., “don’t”) will be missed. If production text may include those, either normalize input (map Unicode quotes to ASCII before matching) or extend the regex to include common Unicode apostrophe characters.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/aelfrice/directive_detector.py`:
- Around line 87-90: The current WH-question leading regex _WH_QUESTION_LEADING
includes "when" which causes conditional directives like "when merging, always
squash" to be filtered out; remove "when" from the _WH_QUESTION_LEADING pattern
so it no longer treats leading "when" as an interrogative, and optionally add a
narrow check elsewhere (e.g., in the same directive detection flow or the
trailing-question filter) that specifically matches "when should|can|will|do"
(paired with a trailing '?' check) to still catch true "when" questions without
reintroducing directive false negatives; update any tests that assert behavior
of _WH_QUESTION_LEADING or the verb-question gating to reflect the new behavior.
In `@tests/bench_gate/test_directive_detection.py`:
- Around line 29-48: The test currently uses all rows (including seed rows) for
the MIN_ROWS guard and for computing TP/FP/FN/TN; fix by filtering out seed rows
first (e.g., create a filtered_rows = [r for r in rows if not r.get("seed")] or
similar) and use filtered_rows for the length check against MIN_ROWS and for the
scoring loop (replace usages of rows in the skip guard and the for row in rows
loop); keep detect_directive(...) and the tp/fp/fn/tn counters unchanged but
iterate only over non-seed examples so both the floor check and precision/recall
reflect real examples.
In `@tests/test_directive_detector.py`:
- Around line 31-47: The negative test list in test_directive_negatives doesn't
include a WH-leading sentence without a trailing question mark, so the
_WH_QUESTION_LEADING filter is never tested in isolation; update the
parameterized inputs in test_directive_negatives (which calls detect_directive)
to include at least one WH-leading negative example that lacks a trailing "?"
(e.g., "what does the staging gate check" or similar) so that
_WH_QUESTION_LEADING is exercised without the trailing-question filter firing
first.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9c0e99d6-e2f8-4abf-b2b7-c34880bafd88
📒 Files selected for processing (6)
src/aelfrice/directive_detector.pytests/bench_gate/test_directive_detection.pytests/corpus/v2_0/README.mdtests/corpus/v2_0/directive_detection/.gitkeeptests/test_corpus_schema.pytests/test_directive_detector.py
Comment on lines
+87
to
+90
| _WH_QUESTION_LEADING = re.compile( | ||
| r"^\s*(?:what|why|how|when|where|who|which|whose|whom)\b", | ||
| re.IGNORECASE, | ||
| ) |
There was a problem hiding this comment.
"when" in _WH_QUESTION_LEADING silently drops conditional directives, hurting recall.
"when" is the only entry in this list that commonly begins durable conditional rules rather than questions — e.g. "when merging, always squash", "when possible, avoid force-push". Every such directive is killed by filter 3 before the verb check fires, producing false negatives. The recall gate is already set at only 0.60; if the labeled corpus skews toward conditional form this filter alone can sink the gate.
The other WH-words (what, why, how, where, who, which, whose, whom) are genuinely interrogative and rarely open directives, so they can stay.
Consider removing when from this list and, if needed, adding a targeted check for "when should/can/will/do …" (paired with the existing trailing-? filter) to avoid re-introducing question false-positives.
💡 Suggested fix
_WH_QUESTION_LEADING = re.compile(
- r"^\s*(?:what|why|how|when|where|who|which|whose|whom)\b",
+ r"^\s*(?:what|why|how|where|who|which|whose|whom)\b",
re.IGNORECASE,
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/aelfrice/directive_detector.py` around lines 87 - 90, The current
WH-question leading regex _WH_QUESTION_LEADING includes "when" which causes
conditional directives like "when merging, always squash" to be filtered out;
remove "when" from the _WH_QUESTION_LEADING pattern so it no longer treats
leading "when" as an interrogative, and optionally add a narrow check elsewhere
(e.g., in the same directive detection flow or the trailing-question filter)
that specifically matches "when should|can|will|do" (paired with a trailing '?'
check) to still catch true "when" questions without reintroducing directive
false negatives; update any tests that assert behavior of _WH_QUESTION_LEADING
or the verb-question gating to reflect the new behavior.
Comment on lines
+29
to
+48
| if len(rows) < MIN_ROWS: | ||
| pytest.skip( | ||
| f"directive_detection corpus has {len(rows)} rows; gate requires " | ||
| f"≥{MIN_ROWS} per docs/v2_enforcement.md § H1" | ||
| ) | ||
|
|
||
| from aelfrice.directive_detector import detect_directive | ||
|
|
||
| tp = fp = fn = tn = 0 | ||
| for row in rows: | ||
| actual = row["label"] == "directive" | ||
| predicted = detect_directive(row["prompt"]) | ||
| if predicted and actual: | ||
| tp += 1 | ||
| elif predicted and not actual: | ||
| fp += 1 | ||
| elif (not predicted) and actual: | ||
| fn += 1 | ||
| else: | ||
| tn += 1 |
There was a problem hiding this comment.
Seed rows are included in both the MIN_ROWS floor check and the P/R computation.
load_corpus_module returns all rows, including entries marked "seed": true. Per the README, seed rows are "committed only to anchor the schema" and explicitly excluded from the v0.1 count target. Including them here means:
- The 200-row floor can be satisfied by synthetic seed rows, bypassing the intent of requiring 200 real coding-prompt examples before the gate fires.
- Precision/recall can be skewed by schema-anchoring examples that are not representative of the distribution the gate is designed to measure.
Both the skip guard and the scoring loop should operate on non-seed rows only:
🛡️ Proposed fix
rows = load_corpus_module(aelfrice_corpus_root, "directive_detection")
+ # Seed rows anchor the schema only; exclude them from gate evaluation
+ # per README § "Seed rows do NOT count toward the v0.1 ≥50/module target".
+ rows = [r for r in rows if not r.get("seed")]
+
if len(rows) < MIN_ROWS:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/bench_gate/test_directive_detection.py` around lines 29 - 48, The test
currently uses all rows (including seed rows) for the MIN_ROWS guard and for
computing TP/FP/FN/TN; fix by filtering out seed rows first (e.g., create a
filtered_rows = [r for r in rows if not r.get("seed")] or similar) and use
filtered_rows for the length check against MIN_ROWS and for the scoring loop
(replace usages of rows in the skip guard and the for row in rows loop); keep
detect_directive(...) and the tp/fp/fn/tn counters unchanged but iterate only
over non-seed examples so both the floor check and precision/recall reflect real
examples.
Comment on lines
+31
to
+47
| @pytest.mark.parametrize( | ||
| "text", | ||
| [ | ||
| "", | ||
| " ", | ||
| "what does the staging gate check?", | ||
| "should we rebase or merge here?", | ||
| "I never push to main when I'm tired", | ||
| "I always check git status because the worktrees confuse me", | ||
| "maybe we should never use force-push", | ||
| "I think we must rebase", | ||
| "the deploy ran fine", | ||
| "looks good", | ||
| ], | ||
| ) | ||
| def test_directive_negatives(text: str) -> None: | ||
| assert detect_directive(text) is False |
There was a problem hiding this comment.
Filter 3 (_WH_QUESTION_LEADING) is never the sole gate in any negative test case.
Both negative cases that exercise WH-question detection ("what does the staging gate check?") also end with "?", so filter 2 (trailing ?) fires first and filter 3 is never reached. A regression that removes _WH_QUESTION_LEADING entirely would pass the current suite. Add at least one negative without a trailing ?:
"I think we must rebase",
"the deploy ran fine",
"looks good",
+ "what the staging gate checks", # WH-leading, no trailing ?
+ "why we run the bench gate", # WH-leading, no trailing ?
],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_directive_detector.py` around lines 31 - 47, The negative test
list in test_directive_negatives doesn't include a WH-leading sentence without a
trailing question mark, so the _WH_QUESTION_LEADING filter is never tested in
isolation; update the parameterized inputs in test_directive_negatives (which
calls detect_directive) to include at least one WH-leading negative example that
lacks a trailing "?" (e.g., "what does the staging gate check" or similar) so
that _WH_QUESTION_LEADING is exercised without the trailing-question filter
firing first.
Collaborator
Author
|
[claim:merge:toug:2026-05-03T16:40:50Z] |
Collaborator
Author
|
[claim:review:Setr:2026-05-03T16:41:09Z] |
|
This PR is now behind Auto-rebase was removed because the bot has no signing key; rebasing as the bot strips author signatures and the |
Adds the H1 re-entry harness so #374's deferral can actually be evaluated: - src/aelfrice/directive_detector.py: candidate regex detector (29 imperative verbs, wh-question / hedging / habitual-narration filters). Spec at docs/v2_enforcement.md § H1. - tests/corpus/v2_0/directive_detection/: new corpus module dir (lab-side fixtures, directory-of-origin rule). - tests/corpus/v2_0/README.md: schema row + per-line shape (`prompt`: str; label `directive|not_directive`); re-entry-gate section restating P>=0.80 / R>=0.60 / n>=200. - tests/test_corpus_schema.py: validate the new module shape. - tests/bench_gate/test_directive_detection.py: bench-gated test, scores detector against corpus, asserts P>=0.80 + R>=0.60 + n>=200; skips on public CI without AELFRICE_CORPUS_ROOT. - tests/test_directive_detector.py: 18 sanity cases for the four filter behaviors. Public-CI runnable. Does not implement process_directive, the TODO lifecycle, escalation, or any hook wiring. Those stay deferred until the gate passes against a real labeled corpus.
Collaborator
Author
|
[claim:review:Kulili:2026-05-03T16:42:01Z] |
Collaborator
Author
|
[release:review:Kulili:2026-05-03T16:42:06Z] |
robotrocketscience
force-pushed
the
feat/issue-374-directive-bench
branch
2 times, most recently
from
May 3, 2026 16:42
0400820 to
e5ea00b
Compare
yoshi280
enabled auto-merge (rebase)
May 3, 2026 16:43
Collaborator
Author
|
[release:review:Setr:2026-05-03T16:47:46Z] |
Collaborator
Author
|
[release:merge:toug:2026-05-03T16:49:13Z] |
This was referenced May 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds the H1 directive-detection re-entry harness so #374's bench-gate deferral can actually be evaluated. Does not implement H1 — the
process_directive/ TODO lifecycle / escalation / hook wiring stay deferred until the gate passes.Why
Per
docs/v2_enforcement.md§ H1 (and the row indocs/V2_REENTRY_QUEUE.md), H1 reopens for implementation only when a labeled sample of ≥200 coding prompts shows ≥0.80 precision and ≥0.60 recall. Until this PR, that gate had no candidate detector to score and no schema entry in the v2.0 corpus contract — so the deferral was unfalsifiable. This PR makes the gate evaluable.Changes
src/aelfrice/directive_detector.py— candidate regex detector. 29-imperative-verb alternation reconstructed from the spec exemplars, plus three filters: wh-leading question, trailing?, hedge markers (maybe,I think, …), and habitual-narration ("I never X when Y" pattern from § H1's own counter-example). Pure stdlibre. ~85 LOC.tests/corpus/v2_0/directive_detection/— new corpus module dir (.gitkeeponly — lab populates the labeled JSONL per directory-of-origin rule).tests/corpus/v2_0/README.md— adds the module row, per-line shape (prompt: str, labeldirective|not_directive), and a "re-entry gate" subsection restating the P/R/n thresholds.tests/test_corpus_schema.py— adds the new module to theMODULESvalidator dict.tests/bench_gate/test_directive_detection.py— bench-gated; skips on public CI withoutAELFRICE_CORPUS_ROOT. With corpus mounted: scores detector, computes P/R, assertsP ≥ 0.80 ∧ R ≥ 0.60 ∧ n ≥ 200. Failure assertions citedocs/v2_enforcement.md § H1so the next reader knows what re-deferring looks like.tests/test_directive_detector.py— 18 public-CI-runnable sanity cases covering the four filter behaviors.Out of scope
tests/corpus/v2_0/README.mdand the directory-of-origin rule; lab populatesaelfrice-lab/tests/corpus/v2_0/directive_detection/*.jsonlseparately.process_directive, the TODO lifecycle, repetition counter, escalation table, hook wiring. All gated on this gate passing.directive_detector.py; if the corpus shows specific verbs over- or under-fire, tune in a follow-up.Verification
uv run pytest -q— 2181 passed, 22 skipped (the new bench-gate test skips clean without corpus, as designed).Closes nothing — #374 stays open until the corpus lands and the gate either passes (→ implementation unblocks) or fails (→ stays deferred per § H1).
Summary by Sourcery
Add a candidate directive-detection detector and benchmark gate to make the H1 re-entry condition for #374 evaluable against a labeled corpus.
New Features:
detect_directivehelper for identifying durable imperative directives in prompts.directive_detectioncorpus module with schema and re-entry gate documentation for labeled prompts.Enhancements:
directive_detectionmodule.Tests:
Summary by CodeRabbit
New Features
Tests
Documentation