fix: grep tool no longer counts "No matches found" as 1 match - #1118
fix: grep tool no longer counts "No matches found" as 1 match#1118josemonteiro wants to merge 2 commits into
Conversation
When ripgrep finds nothing, pi's grep tool returns the literal sentinel "No matches found" as a single text line. Kimchi's grep renderResult counted every non-empty line as a match, so the sentinel was miscounted and shown as "1 matches". Filter out the sentinel before counting — real match lines always carry a path:line: prefix, so genuine results are unaffected. Adds regression tests covering both the no-match sentinel and real match lines. Co-Authored-By: Kimchi <noreply@kimchi.dev>
Kimchi Code Review
Summary📊 Review Score: 90/100 (overall code quality — 0 lowest, 100 highest) 🧪 Tests: yes — Tests are added for both the fix (the 'No matches found' sentinel is not counted as a match) and the regression case (real match lines are still counted correctly). The tests exercise the actual No significant issues found. LGTM! 🎉 What to expectKimchi will analyze the changes in this pull request and post:
The review typically completes within a few minutes. This comment will be updated once the review is ready. Interact with Kimchi
ConfigurationReviews are configured by your organization admin. Powered by Kimchi — AI-powered code review by CAST AI |
![kimchi-review[bot]](https://avatars.githubusercontent.com/in/3223788?s=60&v=4){kind=small}
There was a problem hiding this comment.
📊 Review Score: 90/100 (overall code quality — 0 lowest, 100 highest)
⏱️ Estimated effort to review: 1/5 (1 = trivial, 5 = very complex)
🧪 Tests: yes — Tests are added for both the fix (the 'No matches found' sentinel is not counted as a match) and the regression case (real match lines are still counted correctly). The tests exercise the actual renderResult path registered by the extension.
No significant issues found. LGTM! 🎉
- Extract GREP_NO_MATCHES_SENTINEL constant instead of hardcoding the string in both the renderer and tests (Primitive Obsession / Shotgun Surgery risk flagged by review). - Detect the sentinel at the content level rather than per-line, so a genuine match whose text happens to equal the sentinel is no longer at risk of being swallowed. - Remove the .map(line => line.trim()) scope creep that mutated genuine match lines displayed whitespace, restoring the original filter-only predicate that preserves line content. - Add regression test: a real match line containing the sentinel text is counted as 1 match, not treated as zero-match. Co-Authored-By: Kimchi <noreply@kimchi.dev>
1 participant
Linked issue
N/A — no open tracking issue exists for this bug.
What does this PR do?
When ripgrep finds nothing, pi's grep tool (upstream) returns the literal sentinel
"No matches found"as a single text line instead of empty content. Kimchi's greprenderResultinsrc/extensions/tool-rendering.tscounted every non-empty line as a "match", so the sentinel was miscounted and displayed as1 matches— contradicting the "No matches found" text underneath.This PR filters the sentinel out before counting. Real match lines always carry a
path:line:prefix, so the exact-string filter cannot swallow genuine results. A zero-match result now correctly rendersno matches.Investigation summary
src/extensions/tool-rendering.test.ts:no matches, never1 matches2 matchesChecklist
pnpm run test) —tool-rendering.test.ts74/74 passpnpm run check) — biome clean