Add explanatory inclusion/exclusion logging #197
Conversation
Selectors.MatchesIncludes now returns (bool, string). Context logs human-readable reasons for including or skipping items. Add table-driven tests and FEATURE_EXPLANATORY_LOGGING.md doc.
There was a problem hiding this comment.
Pull request overview
This PR enhances the CLI's logging to provide clear explanations for why items (rules, tasks, skills, commands) are included or excluded during context assembly. The MatchesIncludes method now returns both a boolean match result and a human-readable reason string.
Changes:
- Modified
Selectors.MatchesIncludesto return(bool, string)with detailed match/mismatch reasons - Enhanced logging throughout
context.goto display inclusion/exclusion explanations - Added comprehensive table-driven tests for the new functionality
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/codingcontext/selectors/selectors.go | Refactored MatchesIncludes to return match result and reason string |
| pkg/codingcontext/context.go | Updated all MatchesIncludes call sites to handle the new return signature and log reasons |
| pkg/codingcontext/selectors/selectors_test.go | Added table-driven tests for reason string validation and updated existing test |
| FEATURE_EXPLANATORY_LOGGING.md | Added comprehensive documentation for the new explanatory logging feature |
| // No selectors specified | ||
| return true, "no selectors specified (included by default)" |
There was a problem hiding this comment.
This return statement is unreachable. The early return at lines 98-100 handles the case when there are no selectors (nil or empty map), so execution cannot reach line 141. Remove this unreachable code block.
| selectors: []string{"env=production"}, | ||
| frontmatter: markdown.BaseFrontMatter{Content: map[string]any{"language": "go"}}, | ||
| wantMatch: true, | ||
| wantReason: "no selectors specified (included by default)", |
There was a problem hiding this comment.
This test case expects the reason 'no selectors specified (included by default)', but the code returns an empty string when there are no selectors (line 99 in selectors.go). The test expectation should be an empty string to match the actual behavior.
Selectors.MatchesIncludes now returns (bool, string). Context logs human-readable reasons for including or skipping items. Add table-driven tests and FEATURE_EXPLANATORY_LOGGING.md doc.