From 2b647ed9a178704e47abc5c93b15660a8095771f Mon Sep 17 00:00:00 2001 From: "roomote[bot]" <219738659+roomote[bot]@users.noreply.github.com> Date: Wed, 6 Aug 2025 06:28:28 -0700 Subject: [PATCH] fix: handle current directory path "." correctly in codebase_search tool (#6517) * fix: handle current directory path "." correctly in codebase_search tool - Fix path filtering logic in QdrantVectorStore.search() to properly handle current directory representations - When directoryPrefix is ".", "./", "", or similar, set filter to undefined to search entire workspace - Add comprehensive tests covering various current directory path formats including cross-platform support - Resolves issue where codebase_search with path="." returned no results Fixes #6514 * fix: normalize directory prefix handling in Qdrant vector store * fix: normalize paths starting with './' and fix OS-dependency issue - Use forward slash for splitting after toPosix() conversion - Remove leading './' from paths like './src' to normalize them to 'src' - Update test expectations to match correct behavior * refactor: use path.posix.normalize instead of custom toPosix method - Replaced directoryPrefix.toPosix() with path.posix.normalize() - Added proper handling of backslashes before normalization - Updated test mock to include posix.normalize method - All tests passing (381 tests in code-index service) * refactor: address review comments - improve path normalization - Keep check for './' after normalization as path.posix.normalize('./') returns './' - Use actual Node.js path.posix implementation in tests instead of custom mock - Apply path.posix.normalize to cleanedPrefix for consistency All 381 code-index tests pass * fix: apply path.posix.normalize when cleaning prefix to avoid redundant normalization Addresses review comment from @mrubens to normalize the path at line 385 instead of normalizing twice * fix: correct current directory detection logic The issue was that the condition checked for an empty string after normalization, but path.posix.normalize('') actually returns '.', not ''. This caused the current directory check to fail when an empty string was passed. Removed the redundant empty string check since normalize('') returns '.' which is already handled by the first condition. --------- Co-authored-by: Roo Code Co-authored-by: Daniel Riccio Co-authored-by: hannesrudolph --- .../__tests__/qdrant-client.spec.ts | 212 +++++++++++++++++- .../code-index/vector-store/qdrant-client.ts | 27 ++- 2 files changed, 228 insertions(+), 11 deletions(-) diff --git a/src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts b/src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts index e539c2edde..822832d17c 100644 --- a/src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts +++ b/src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts @@ -21,10 +21,14 @@ vitest.mock("../../../../i18n", () => ({ return key // Just return the key for other cases }, })) -vitest.mock("path", () => ({ - ...vitest.importActual("path"), - sep: "/", -})) +vitest.mock("path", async () => { + const actual = await vitest.importActual("path") + return { + ...actual, + sep: "/", + posix: actual.posix, + } +}) const mockQdrantClientInstance = { getCollection: vitest.fn(), @@ -1526,5 +1530,205 @@ describe("QdrantVectorStore", () => { expect(callArgs.limit).toBe(DEFAULT_MAX_SEARCH_RESULTS) expect(callArgs.score_threshold).toBe(DEFAULT_SEARCH_MIN_SCORE) }) + + describe("current directory path handling", () => { + it("should not apply filter when directoryPrefix is '.'", async () => { + const queryVector = [0.1, 0.2, 0.3] + const directoryPrefix = "." + const mockQdrantResults = { + points: [ + { + id: "test-id-1", + score: 0.85, + payload: { + filePath: "src/test.ts", + codeChunk: "test code", + startLine: 1, + endLine: 5, + pathSegments: { "0": "src", "1": "test.ts" }, + }, + }, + ], + } + + mockQdrantClientInstance.query.mockResolvedValue(mockQdrantResults) + + const results = await vectorStore.search(queryVector, directoryPrefix) + + expect(mockQdrantClientInstance.query).toHaveBeenCalledWith(expectedCollectionName, { + query: queryVector, + filter: undefined, // Should be undefined for current directory + score_threshold: DEFAULT_SEARCH_MIN_SCORE, + limit: DEFAULT_MAX_SEARCH_RESULTS, + params: { + hnsw_ef: 128, + exact: false, + }, + with_payload: { + include: ["filePath", "codeChunk", "startLine", "endLine", "pathSegments"], + }, + }) + + expect(results).toEqual(mockQdrantResults.points) + }) + + it("should not apply filter when directoryPrefix is './'", async () => { + const queryVector = [0.1, 0.2, 0.3] + const directoryPrefix = "./" + const mockQdrantResults = { points: [] } + + mockQdrantClientInstance.query.mockResolvedValue(mockQdrantResults) + + await vectorStore.search(queryVector, directoryPrefix) + + expect(mockQdrantClientInstance.query).toHaveBeenCalledWith(expectedCollectionName, { + query: queryVector, + filter: undefined, // Should be undefined for current directory + score_threshold: DEFAULT_SEARCH_MIN_SCORE, + limit: DEFAULT_MAX_SEARCH_RESULTS, + params: { + hnsw_ef: 128, + exact: false, + }, + with_payload: { + include: ["filePath", "codeChunk", "startLine", "endLine", "pathSegments"], + }, + }) + }) + + it("should not apply filter when directoryPrefix is empty string", async () => { + const queryVector = [0.1, 0.2, 0.3] + const directoryPrefix = "" + const mockQdrantResults = { points: [] } + + mockQdrantClientInstance.query.mockResolvedValue(mockQdrantResults) + + await vectorStore.search(queryVector, directoryPrefix) + + expect(mockQdrantClientInstance.query).toHaveBeenCalledWith(expectedCollectionName, { + query: queryVector, + filter: undefined, // Should be undefined for empty string + score_threshold: DEFAULT_SEARCH_MIN_SCORE, + limit: DEFAULT_MAX_SEARCH_RESULTS, + params: { + hnsw_ef: 128, + exact: false, + }, + with_payload: { + include: ["filePath", "codeChunk", "startLine", "endLine", "pathSegments"], + }, + }) + }) + + it("should not apply filter when directoryPrefix is '.\\' (Windows style)", async () => { + const queryVector = [0.1, 0.2, 0.3] + const directoryPrefix = ".\\" + const mockQdrantResults = { points: [] } + + mockQdrantClientInstance.query.mockResolvedValue(mockQdrantResults) + + await vectorStore.search(queryVector, directoryPrefix) + + expect(mockQdrantClientInstance.query).toHaveBeenCalledWith(expectedCollectionName, { + query: queryVector, + filter: undefined, // Should be undefined for Windows current directory + score_threshold: DEFAULT_SEARCH_MIN_SCORE, + limit: DEFAULT_MAX_SEARCH_RESULTS, + params: { + hnsw_ef: 128, + exact: false, + }, + with_payload: { + include: ["filePath", "codeChunk", "startLine", "endLine", "pathSegments"], + }, + }) + }) + + it("should not apply filter when directoryPrefix has trailing slashes", async () => { + const queryVector = [0.1, 0.2, 0.3] + const directoryPrefix = ".///" + const mockQdrantResults = { points: [] } + + mockQdrantClientInstance.query.mockResolvedValue(mockQdrantResults) + + await vectorStore.search(queryVector, directoryPrefix) + + expect(mockQdrantClientInstance.query).toHaveBeenCalledWith(expectedCollectionName, { + query: queryVector, + filter: undefined, // Should be undefined after normalizing trailing slashes + score_threshold: DEFAULT_SEARCH_MIN_SCORE, + limit: DEFAULT_MAX_SEARCH_RESULTS, + params: { + hnsw_ef: 128, + exact: false, + }, + with_payload: { + include: ["filePath", "codeChunk", "startLine", "endLine", "pathSegments"], + }, + }) + }) + + it("should still apply filter for relative paths like './src'", async () => { + const queryVector = [0.1, 0.2, 0.3] + const directoryPrefix = "./src" + const mockQdrantResults = { points: [] } + + mockQdrantClientInstance.query.mockResolvedValue(mockQdrantResults) + + await vectorStore.search(queryVector, directoryPrefix) + + expect(mockQdrantClientInstance.query).toHaveBeenCalledWith(expectedCollectionName, { + query: queryVector, + filter: { + must: [ + { + key: "pathSegments.0", + match: { value: "src" }, + }, + ], + }, // Should normalize "./src" to "src" + score_threshold: DEFAULT_SEARCH_MIN_SCORE, + limit: DEFAULT_MAX_SEARCH_RESULTS, + params: { + hnsw_ef: 128, + exact: false, + }, + with_payload: { + include: ["filePath", "codeChunk", "startLine", "endLine", "pathSegments"], + }, + }) + }) + + it("should still apply filter for regular directory paths", async () => { + const queryVector = [0.1, 0.2, 0.3] + const directoryPrefix = "src" + const mockQdrantResults = { points: [] } + + mockQdrantClientInstance.query.mockResolvedValue(mockQdrantResults) + + await vectorStore.search(queryVector, directoryPrefix) + + expect(mockQdrantClientInstance.query).toHaveBeenCalledWith(expectedCollectionName, { + query: queryVector, + filter: { + must: [ + { + key: "pathSegments.0", + match: { value: "src" }, + }, + ], + }, // Should still create filter for regular paths + score_threshold: DEFAULT_SEARCH_MIN_SCORE, + limit: DEFAULT_MAX_SEARCH_RESULTS, + params: { + hnsw_ef: 128, + exact: false, + }, + with_payload: { + include: ["filePath", "codeChunk", "startLine", "endLine", "pathSegments"], + }, + }) + }) + }) }) }) diff --git a/src/services/code-index/vector-store/qdrant-client.ts b/src/services/code-index/vector-store/qdrant-client.ts index 0218e37295..50f39666c4 100644 --- a/src/services/code-index/vector-store/qdrant-client.ts +++ b/src/services/code-index/vector-store/qdrant-client.ts @@ -375,13 +375,26 @@ export class QdrantVectorStore implements IVectorStore { let filter = undefined if (directoryPrefix) { - const segments = directoryPrefix.split(path.sep).filter(Boolean) - - filter = { - must: segments.map((segment, index) => ({ - key: `pathSegments.${index}`, - match: { value: segment }, - })), + // Check if the path represents current directory + const normalizedPrefix = path.posix.normalize(directoryPrefix.replace(/\\/g, "/")) + // Note: path.posix.normalize("") returns ".", and normalize("./") returns "./" + if (normalizedPrefix === "." || normalizedPrefix === "./") { + // Don't create a filter - search entire workspace + filter = undefined + } else { + // Remove leading "./" from paths like "./src" to normalize them + const cleanedPrefix = path.posix.normalize( + normalizedPrefix.startsWith("./") ? normalizedPrefix.slice(2) : normalizedPrefix, + ) + const segments = cleanedPrefix.split("/").filter(Boolean) + if (segments.length > 0) { + filter = { + must: segments.map((segment, index) => ({ + key: `pathSegments.${index}`, + match: { value: segment }, + })), + } + } } }