fix(core): handle Windows separators in the remaining grep_search parsers - #13042
fix(core): handle Windows separators in the remaining grep_search parsers#13042LHMQ878 wants to merge 1 commit into
Conversation
ripgrep is invoked with `.` as the search root, so with `--heading` it echoes that root back on every file-heading line using the platform separator: `./path` on POSIX, `.\path` on Windows. Four independent parsers of that output assumed `./`, so on Windows each one silently discarded results that ripgrep had already found. continuedev#13027 named one of them and continuedev#13037 fixes it. This covers the remaining three: - `core/tools/implementations/grepSearch.ts` — `splitGrepResultsByFile` matches headings with `/^\.\/([^\n]+)$/gm` and strips them with a second `^\.\/` regex. With `splitByFile: true` on Windows this matches nothing, so the tool returns an *empty array* of context items rather than the "no results" string. - The same function's captured path becomes a `type: "file"` ContextItem URI, and those URIs are glob-matched to decide which rules apply (`core/llm/rules/getSystemMessageWithRules.ts`). In a glob a backslash escapes the next character instead of separating segments, so `src\calc.ts` would match no rule pattern. The path is now normalised to forward slashes. - `extensions/vscode/src/VsCodeIde.ts` — the `maxResults` truncation splits on `(\n--|\n\.\/)`, so on Windows it finds no boundaries and cannot honour the limit. To keep the four sites from drifting apart again, the predicate and its regex equivalent now live together in `core/util/grepSearch.ts` as `isGrepResultPathLine()` and `GREP_RESULT_PATH_PREFIX_SOURCE`. Both IDE integrations pass `.` as the ripgrep root — `VsCodeIde.ts:609` and `IntelliJIde.kt:488` — so both feed `.\` into the shared core parser on Windows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
All contributors have signed the CLA ✍️ ✅ |
CI statusAll 12 Two red checks, neither caused by this PR:
So the only thing standing between this and green is the CLA. |
|
I have read the CLA Document and I hereby sign the CLA |
CI, final state
This PR touches no autocomplete code — it changes three ripgrep-output parsers and adds tests.
Everything attributable to this change is green, including all 12 (Separately: |
|
The two red checks here are not from this PR, and I'd rather say so with the evidence than leave them looking like mine.
I checked whether it is specific to this branch by tallying the same check across the 25 most recently updated PRs in the repo: The failures include documentation-only PRs (#13066, #12579), which cannot affect autocomplete behaviour, and PRs touching unrelated areas of the GUI, the CLI and provider code. This PR changes Nothing here needs doing on my side, but two things may be worth knowing: with Happy to rebase once it's green if that helps confirm the branch itself passes. |
|
The red
This PR touches 5 files, none of them under So nothing in this branch can turn it green. Flagging it rather than pushing an empty commit to re-run, since a re-run against the same base should fail the same way. The vitest suites added here cover the change itself and are green. Happy to rebase onto a |
Description
Follow-up to #13027. This is complementary to @guillaume-flambard's #13037, not a replacement — that PR fixes the one site the issue named, this one covers the three that were left, including a worse variant of the same bug.
ripgrep is invoked with
.as the search root, so with--headingit echoes that root back on every file-heading line using the platform separator:./pathon POSIX,.\pathon Windows. Four independent parsers of that output assumed./:core/util/grepSearch.ts:60—formatGrepSearchResultsheading checknumResultsstays 0 → "The search returned no results."core/tools/implementations/grepSearch.ts—splitGrepResultsByFileheading regex + strip regexsplitByFile: true, returns an empty array of context items — not even the "no results" messagetype: "file"ContextItem URIextensions/vscode/src/VsCodeIde.ts:620—maxResultstruncationmatchAllSite 2 is the one worth flagging: fixing only site 1 makes
splitByFile: truego from "no results" to silently returning zero context items, which reads as success to the caller. Verified by control experiment below.Site 3 is a consequence of fixing site 2 rather than extra scope. That captured path is consumed as a
fileContextItem URI, whichcore/llm/rules/getSystemMessageWithRules.ts:288-290glob-matches to decide which rules apply — and in a glob a backslash escapes the next character rather than separating segments, sosrc\calc.tswould match no rule pattern. It is normalised to forward slashes.Both IDE integrations pass
.as the ripgrep root (extensions/vscode/src/VsCodeIde.ts:609,extensions/intellij/.../IntelliJIde.kt:488), so both feed.\into the shared core parser on Windows — this is not VS Code-specific.To stop the four sites from drifting apart again, the predicate and its regex equivalent now live together in
core/util/grepSearch.ts:If #13037 lands first, site 1 here is a trivial conflict — happy to rebase onto it and drop that hunk, or to have this rebased however you prefer.
Tests
core/util/grepSearch.vitest.ts— 4 added (9 → 13):formats Windows-style backslash headings— nested.\src\test.ktalongside--separatorscounts Windows headings so results are not reported as empty— asserts the reported symptom directlyhandles mixed separators in a single result set— multi-root workspaces concatenate one ripgrep run per directorydoes not treat a bare relative path as a heading— negative case: a content line starting with.(.method(),...spread) must not open a new resultcore/tools/implementations/grepSearch.vitest.ts— new file, 5 tests.grepSearchImplhad no test file at all, which is why sites 2–4 went unnoticed. Covers POSIX and Windows for bothsplitByFilemodes, the URI normalisation, and that a genuinely empty search still reports empty.Reviewer test plan
cd core npx vitest run util/grepSearch.vitest.ts tools/implementations/grepSearch.vitest.ts→ 18 passed (13 + 5).
Control experiment — do the tests actually catch the bug?
Reverting each source file in isolation, leaving the tests in place:
→ 3 fail (
expected 1 to be 3,expected +0 to be 1,expected 1 to be 2); the 4th is the negative case, which correctly still passes.→ 1 fail:
expected [] to have a length of 2 but got +0— the empty-array symptom, reproduced.Regression / lint
npx vitest run tools/ util/— 356 passed, 16 failed. Cleanmainbaseline on the same machine: 347 passed, 16 failed. Identical failure set (pre-existing POSIX path assumptions inresolveWorkingDirectory.vitest.tsand 3 other files, unrelated to grep); my change adds +9 passing and 0 failures.npx tsc --noEmit -p core— 151 errors, and clean-mainbaseline is also 151 (unbuilt@continuedev/*workspace packages). No new errors;grep -c grepSearchon the output → 0.npx eslint --max-warnings 0on all changed files — clean.npx prettier --check— "All matched files use Prettier code style!"Tested on
Not validated: the extension end-to-end with a live
rg.exe, and the IntelliJ path. The parsers are pure functions and the tests feed them the exact byte sequences ripgrep emits, but I have not watched a real search return results in the UI. Worth a maintainer sanity-check on a Windows box before merge.Screen recording or screenshot
N/A — no UI change; this is parser behaviour covered by unit tests.
Checklist
Credit to @SpikedCola for pinpointing the root cause in #13027 with Process Monitor, and to @guillaume-flambard for #13037.