-
Notifications
You must be signed in to change notification settings - Fork 4
feat(directive_detection): Path A intent-prefix filter (#374) #467
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,3 +45,50 @@ def test_directive_positives(text: str) -> None: | |
| ) | ||
| def test_directive_negatives(text: str) -> None: | ||
| assert detect_directive(text) is False | ||
|
|
||
|
|
||
| # Path A: head-position coding-task prefix short-circuits to False even when | ||
| # a downstream imperative verb would otherwise fire. Each row embeds a verb | ||
| # from the imperative bank ("never", "ensure", "must", "only", "avoid", …) | ||
| # so the test would pass under the old detector if Path A weren't applied — | ||
| # making the test load-bearing for the new branch. | ||
| @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", | ||
|
Comment on lines
+55
to
+64
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Consider adding cases with leading whitespace/punctuation before the coding-task verb to lock in the intended anchoring behavior. The |
||
| "Make the worker shutdown ensure no half-flushed batches", | ||
| "Build a fixture that requires the v0.1 corpus path", | ||
|
Comment on lines
+57
to
+66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Add negative controls where coding-task verbs appear non-initially so we prove the filter is strictly head-position based. Since |
||
| "Remove the dead branch before merging", | ||
| "Rename _emit so it cannot collide with _emit_core", | ||
| "Extract the helper unless the call site needs inlining", | ||
| "Merge the two threads after the gate passes", | ||
| "Split the test so each case must check exactly one signal", | ||
| "Move the docstring before the type annotations", | ||
| "Delete the cache whenever the schema bumps", | ||
| ], | ||
| ) | ||
| def test_directive_coding_task_prefix_short_circuits(text: str) -> None: | ||
| assert detect_directive(text) is False | ||
|
|
||
|
|
||
| # 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 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.