🐛 Fix trailing boundary regex in verify-deliverables#58
🐛 Fix trailing boundary regex in verify-deliverables#58TechNickAI wants to merge 1 commit intomainfrom
Conversation
The completion regex applied `([^a-zA-Z]|$)` uniformly to all alternatives. For space-ending patterns like `implemented `, `created the `, `wrote the `, `added the `, `saved the `, the boundary required a non-letter after the trailing space — which in English is almost always a letter (the next word). These five patterns were effectively dead code. Fix: split into two grep calls — one for patterns needing a trailing boundary (e.g., "fixed it" vs "fixed itself") and one for space-terminated patterns where the space IS the boundary. Adds test case confirming "Implemented the fix in src/x.ts" now correctly triggers the advisory. Addresses review feedback from PR #54 (Claude review #2, Cursor). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code Review — PR #58OverviewThis PR correctly identifies and fixes a genuine regex bug: space-terminated patterns like Correctness ✅The fix is sound. Key points that hold up:
Suggestions1. Test coverage for the other four space-terminated patterns Only for phrase in "created the" "wrote the" "added the" "saved the"; do
TX=$(write_transcript "..." '...<text>'"$phrase"' solution in src/x.ts</text>...')
run_case "$phrase triggers advisory" "$(make_payload "$TX")" "made no file changes"
done2. Edge case: pattern at end of string without trailing word What happens with "I implemented." (period, no following word)? The period isn't a space, so Group 2 ( 3. Minor comment nit The inline comment is accurate and helpful — no change needed. The PR description is also clear and well-structured. RisksLow. The change is confined to a single advisory hook that:
The only risk is a new false positive from Group 2 lacking a trailing boundary, but that's mitigated by the other guards (zero-mutations check, negation filter, file-path token). SummaryThe fix is correct and well-motivated. The test addition validates the primary regression. The main gap is test coverage for the four non- |
Summary
implemented,created the,wrote the,added the,saved the) that were effectively dead code due to trailing boundary([^a-zA-Z]|$)requiring a non-letter after the spaceAddresses review feedback from PR #54 (Claude review, Cursor Bugbot).
Test plan
🤖 Generated with Claude Code