Skip to content

fix: clarify-intent misses plural 'tests'/'specs' + add unit tests - #247

Open
TerminalGravity wants to merge 1 commit into
mainfrom
fix/clarify-intent-plural-patterns
Open

fix: clarify-intent misses plural 'tests'/'specs' + add unit tests#247
TerminalGravity wants to merge 1 commit into
mainfrom
fix/clarify-intent-plural-patterns

Conversation

@TerminalGravity

Copy link
Copy Markdown
Collaborator

What

  • Bug fix: extractSignals regex used \b(test|spec)\b which doesn't match plurals like 'tests' or 'specs'. Changed to tests?/specs?.
  • Tests: Added 16 unit tests for extractSignals covering all intent patterns, context hints, multi-intent detection, and the plural fix.

Why

Saying 'run the tests' wouldn't trigger the TESTS intent signal — only 'run the test' would. Common phrasing was silently falling through to UNCLEAR.

Test

npm test  # 59 passed (was 43)

- extractSignals regex used \b(test|spec)\b which fails on plurals
- Changed to tests?/specs? so 'run the tests' correctly triggers TESTS signal
- Exported extractSignals for testability
- Added comprehensive test suite for all intent patterns

@TerminalGravity TerminalGravity left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests?/specs? regex fix is obviously correct — surprised we missed plural forms. Good test coverage on the plural cases. Clean PR. ✅

@TerminalGravity TerminalGravity left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense — plurals like 'tests' and 'specs' are common in real prompts. Tests look solid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant