Conversation
nc-review: needs work — 1 blocking, 1 important, 2 nits@tusharui — there is a blocking item below. PR #1369 addresses a documented bug (issue #1341) where ripgrep's match-count kill budget could be exhausted by matches in default-ignored directories, leaving downstream JS filters with zero kept results. The described two-layer fix (only count kept matches; pass a merged ignore file to rg) is conceptually correct and aligns with the architecture in source/utils/file-search.ts. The changeset file is appropriate for a user-facing bug fix. However, only one of the seven claimed new tests ('a .nanocoderignore directory is pruned during traversal') is visible in source/utils/file-search.spec.ts on disk; the regression tests named for the bug (#1341 plain, #1341 with context, 🔴 blocking · The PR description claims seven new AVA tests: '#1341 plain', '#1341 with context', '**/-anchored rebase guard', 'walkProjectEntries pruning', 'rebaseIgnoreLine unit suite', '! re-include', and 'abort temp-dir cleanup'. Only one — 'a .nanocoderignore directory is pruned during traversal, not just filtered from results' (around line 411) — is visible in the spec file on disk. The two regression tests that would actually fail if the Layer 1 (kept-match-only counting) fix regressed — ones that build a fixture with many matches in a default-ignored directory (e.g. dist/) and a few matches in source/, then call Reproducing #1341 in a test looks like this (sketch — adapt to the actual API): mkdirSync(join(testDir, 'dist'), {recursive: true});
mkdirSync(join(testDir, 'src'), {recursive: true});
// dist/ contains 20 matches for the query
for (let i = 0; i < 20; i++) writeFileSync(join(testDir, 'dist', `f${i}.js`), 'searchTarget');
writeFileSync(join(testDir, 'src', 'app.ts'), 'searchTarget');
const result = await searchProjectContents('searchTarget', testDir, 5, false);
t.is(result.matches.length, 1);
t.is(result.matches[0]?.file, 'src/app.ts');A test like this would have failed on the base code (0 matches) and passes after the fix (1 match). Without it, the fix could regress silently. 🟠 important · The diff adds a new ⚪ nit · I cannot fully verify the correctness of the new ⚪ nit · The changeset is present and well-shaped (closes the issue, patch bump, names the right package). No issue here — flagging only because the rubric asks reviewers to confirm the changeset, and this one is correct. 🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with |
Summary
Closes #1341.
search_file_contentscould return 0 matches (withtruncated: true)when ripgrep's early-kill match budget (
rgMaxCount) was consumed by matches insideignored directories. ripgrep still counts matches in ignored paths toward its
budget, so those lines were burned before reaching any kept result — and since the
only drop to ripgrep was the
--max-count, the JS filter downstream never got akept line at all.
Fix
Two layers, applied together:
Layer 1 — don't count ignored output against the budget
runRipgrepnow streams results viaonLineand increments the kill counter onlyfor kept (non-ignored)
type: "match"lines, againstrgMaxCount = maxResults + contextLines.Layer 2 — keep ignored paths out of ripgrep entirely
A merged ignore set (
DEFAULT_IGNORE_DIRS→.gitignore→.nanocoderignore) iswritten to a temp
--ignore-fileand wired into bothsearchProjectContentsandwalkProjectEntries, so rg never traverses/matches ignored directories in the first place.Correctness backstop: the existing downstream JS filter (
projectIgnore.ignores) still applies.Hardening (follow-up commit, from maintainer review)
**/-prefixed rules kept verbatim (git depth-agnostic semantics)mode: 0o600.gitignoreskips the temp file entirelyonLineis tri-state (keep | skip | stop);stdoutholds only kept lineswalkUnsortedFileStream/searchProjectContentsDev tooling (same branch)
pnpm test:allnow runs viascripts/test-all.mjs(cross-platform Node orchestrator,same 7 gates as
scripts/test.sh), so it works on Windows too.Testing
**/-anchored rebase guard,walkProjectEntriespruning,rebaseIgnoreLineunit suite,!re-include,abort temp-dir cleanup
test:format,test:lint,test:types,test:types:vscode,test:knip,test:auditavafailures are pre-existing Windows-environment issues(confirmed reproducible on pre-fix code) — unaffected by this change
Changeset
.changeset/search-ignored-dir-budget.md—patch— Closes #1341