Skip to content

Commit 2b647ed

Browse files
roomote[bot]roomotedaniel-lxshannesrudolph
authored
fix: handle current directory path "." correctly in codebase_search tool (RooCodeInc#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 RooCodeInc#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 <[email protected]> Co-authored-by: Daniel Riccio <[email protected]> Co-authored-by: hannesrudolph <[email protected]>
1 parent 34fb5b7 commit 2b647ed

File tree

2 files changed

+228
-11
lines changed

2 files changed

+228
-11
lines changed

‎src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts‎

Lines changed: 208 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,14 @@ vitest.mock("../../../../i18n", () => ({
2121
return key // Just return the key for other cases
2222
},
2323
}))
24-
vitest.mock("path", () => ({
25-
...vitest.importActual("path"),
26-
sep: "/",
27-
}))
24+
vitest.mock("path", async () => {
25+
const actual = await vitest.importActual("path")
26+
return {
27+
...actual,
28+
sep: "/",
29+
posix: actual.posix,
30+
}
31+
})
2832

2933
const mockQdrantClientInstance = {
3034
getCollection: vitest.fn(),
@@ -1526,5 +1530,205 @@ describe("QdrantVectorStore", () => {
15261530
expect(callArgs.limit).toBe(DEFAULT_MAX_SEARCH_RESULTS)
15271531
expect(callArgs.score_threshold).toBe(DEFAULT_SEARCH_MIN_SCORE)
15281532
})
1533+
1534+
describe("current directory path handling", () => {
1535+
it("should not apply filter when directoryPrefix is '.'", async () => {
1536+
const queryVector = [0.1, 0.2, 0.3]
1537+
const directoryPrefix = "."
1538+
const mockQdrantResults = {
1539+
points: [
1540+
{
1541+
id: "test-id-1",
1542+
score: 0.85,
1543+
payload: {
1544+
filePath: "src/test.ts",
1545+
codeChunk: "test code",
1546+
startLine: 1,
1547+
endLine: 5,
1548+
pathSegments: { "0": "src", "1": "test.ts" },
1549+
},
1550+
},
1551+
],
1552+
}
1553+
1554+
mockQdrantClientInstance.query.mockResolvedValue(mockQdrantResults)
1555+
1556+
const results = await vectorStore.search(queryVector, directoryPrefix)
1557+
1558+
expect(mockQdrantClientInstance.query).toHaveBeenCalledWith(expectedCollectionName, {
1559+
query: queryVector,
1560+
filter: undefined, // Should be undefined for current directory
1561+
score_threshold: DEFAULT_SEARCH_MIN_SCORE,
1562+
limit: DEFAULT_MAX_SEARCH_RESULTS,
1563+
params: {
1564+
hnsw_ef: 128,
1565+
exact: false,
1566+
},
1567+
with_payload: {
1568+
include: ["filePath", "codeChunk", "startLine", "endLine", "pathSegments"],
1569+
},
1570+
})
1571+
1572+
expect(results).toEqual(mockQdrantResults.points)
1573+
})
1574+
1575+
it("should not apply filter when directoryPrefix is './'", async () => {
1576+
const queryVector = [0.1, 0.2, 0.3]
1577+
const directoryPrefix = "./"
1578+
const mockQdrantResults = { points: [] }
1579+
1580+
mockQdrantClientInstance.query.mockResolvedValue(mockQdrantResults)
1581+
1582+
await vectorStore.search(queryVector, directoryPrefix)
1583+
1584+
expect(mockQdrantClientInstance.query).toHaveBeenCalledWith(expectedCollectionName, {
1585+
query: queryVector,
1586+
filter: undefined, // Should be undefined for current directory
1587+
score_threshold: DEFAULT_SEARCH_MIN_SCORE,
1588+
limit: DEFAULT_MAX_SEARCH_RESULTS,
1589+
params: {
1590+
hnsw_ef: 128,
1591+
exact: false,
1592+
},
1593+
with_payload: {
1594+
include: ["filePath", "codeChunk", "startLine", "endLine", "pathSegments"],
1595+
},
1596+
})
1597+
})
1598+
1599+
it("should not apply filter when directoryPrefix is empty string", async () => {
1600+
const queryVector = [0.1, 0.2, 0.3]
1601+
const directoryPrefix = ""
1602+
const mockQdrantResults = { points: [] }
1603+
1604+
mockQdrantClientInstance.query.mockResolvedValue(mockQdrantResults)
1605+
1606+
await vectorStore.search(queryVector, directoryPrefix)
1607+
1608+
expect(mockQdrantClientInstance.query).toHaveBeenCalledWith(expectedCollectionName, {
1609+
query: queryVector,
1610+
filter: undefined, // Should be undefined for empty string
1611+
score_threshold: DEFAULT_SEARCH_MIN_SCORE,
1612+
limit: DEFAULT_MAX_SEARCH_RESULTS,
1613+
params: {
1614+
hnsw_ef: 128,
1615+
exact: false,
1616+
},
1617+
with_payload: {
1618+
include: ["filePath", "codeChunk", "startLine", "endLine", "pathSegments"],
1619+
},
1620+
})
1621+
})
1622+
1623+
it("should not apply filter when directoryPrefix is '.\\' (Windows style)", async () => {
1624+
const queryVector = [0.1, 0.2, 0.3]
1625+
const directoryPrefix = ".\\"
1626+
const mockQdrantResults = { points: [] }
1627+
1628+
mockQdrantClientInstance.query.mockResolvedValue(mockQdrantResults)
1629+
1630+
await vectorStore.search(queryVector, directoryPrefix)
1631+
1632+
expect(mockQdrantClientInstance.query).toHaveBeenCalledWith(expectedCollectionName, {
1633+
query: queryVector,
1634+
filter: undefined, // Should be undefined for Windows current directory
1635+
score_threshold: DEFAULT_SEARCH_MIN_SCORE,
1636+
limit: DEFAULT_MAX_SEARCH_RESULTS,
1637+
params: {
1638+
hnsw_ef: 128,
1639+
exact: false,
1640+
},
1641+
with_payload: {
1642+
include: ["filePath", "codeChunk", "startLine", "endLine", "pathSegments"],
1643+
},
1644+
})
1645+
})
1646+
1647+
it("should not apply filter when directoryPrefix has trailing slashes", async () => {
1648+
const queryVector = [0.1, 0.2, 0.3]
1649+
const directoryPrefix = ".///"
1650+
const mockQdrantResults = { points: [] }
1651+
1652+
mockQdrantClientInstance.query.mockResolvedValue(mockQdrantResults)
1653+
1654+
await vectorStore.search(queryVector, directoryPrefix)
1655+
1656+
expect(mockQdrantClientInstance.query).toHaveBeenCalledWith(expectedCollectionName, {
1657+
query: queryVector,
1658+
filter: undefined, // Should be undefined after normalizing trailing slashes
1659+
score_threshold: DEFAULT_SEARCH_MIN_SCORE,
1660+
limit: DEFAULT_MAX_SEARCH_RESULTS,
1661+
params: {
1662+
hnsw_ef: 128,
1663+
exact: false,
1664+
},
1665+
with_payload: {
1666+
include: ["filePath", "codeChunk", "startLine", "endLine", "pathSegments"],
1667+
},
1668+
})
1669+
})
1670+
1671+
it("should still apply filter for relative paths like './src'", async () => {
1672+
const queryVector = [0.1, 0.2, 0.3]
1673+
const directoryPrefix = "./src"
1674+
const mockQdrantResults = { points: [] }
1675+
1676+
mockQdrantClientInstance.query.mockResolvedValue(mockQdrantResults)
1677+
1678+
await vectorStore.search(queryVector, directoryPrefix)
1679+
1680+
expect(mockQdrantClientInstance.query).toHaveBeenCalledWith(expectedCollectionName, {
1681+
query: queryVector,
1682+
filter: {
1683+
must: [
1684+
{
1685+
key: "pathSegments.0",
1686+
match: { value: "src" },
1687+
},
1688+
],
1689+
}, // Should normalize "./src" to "src"
1690+
score_threshold: DEFAULT_SEARCH_MIN_SCORE,
1691+
limit: DEFAULT_MAX_SEARCH_RESULTS,
1692+
params: {
1693+
hnsw_ef: 128,
1694+
exact: false,
1695+
},
1696+
with_payload: {
1697+
include: ["filePath", "codeChunk", "startLine", "endLine", "pathSegments"],
1698+
},
1699+
})
1700+
})
1701+
1702+
it("should still apply filter for regular directory paths", async () => {
1703+
const queryVector = [0.1, 0.2, 0.3]
1704+
const directoryPrefix = "src"
1705+
const mockQdrantResults = { points: [] }
1706+
1707+
mockQdrantClientInstance.query.mockResolvedValue(mockQdrantResults)
1708+
1709+
await vectorStore.search(queryVector, directoryPrefix)
1710+
1711+
expect(mockQdrantClientInstance.query).toHaveBeenCalledWith(expectedCollectionName, {
1712+
query: queryVector,
1713+
filter: {
1714+
must: [
1715+
{
1716+
key: "pathSegments.0",
1717+
match: { value: "src" },
1718+
},
1719+
],
1720+
}, // Should still create filter for regular paths
1721+
score_threshold: DEFAULT_SEARCH_MIN_SCORE,
1722+
limit: DEFAULT_MAX_SEARCH_RESULTS,
1723+
params: {
1724+
hnsw_ef: 128,
1725+
exact: false,
1726+
},
1727+
with_payload: {
1728+
include: ["filePath", "codeChunk", "startLine", "endLine", "pathSegments"],
1729+
},
1730+
})
1731+
})
1732+
})
15291733
})
15301734
})

‎src/services/code-index/vector-store/qdrant-client.ts‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -375,13 +375,26 @@ export class QdrantVectorStore implements IVectorStore {
375375
let filter = undefined
376376

377377
if (directoryPrefix) {
378-
const segments = directoryPrefix.split(path.sep).filter(Boolean)
379-
380-
filter = {
381-
must: segments.map((segment, index) => ({
382-
key: `pathSegments.${index}`,
383-
match: { value: segment },
384-
})),
378+
// Check if the path represents current directory
379+
const normalizedPrefix = path.posix.normalize(directoryPrefix.replace(/\\/g, "/"))
380+
// Note: path.posix.normalize("") returns ".", and normalize("./") returns "./"
381+
if (normalizedPrefix === "." || normalizedPrefix === "./") {
382+
// Don't create a filter - search entire workspace
383+
filter = undefined
384+
} else {
385+
// Remove leading "./" from paths like "./src" to normalize them
386+
const cleanedPrefix = path.posix.normalize(
387+
normalizedPrefix.startsWith("./") ? normalizedPrefix.slice(2) : normalizedPrefix,
388+
)
389+
const segments = cleanedPrefix.split("/").filter(Boolean)
390+
if (segments.length > 0) {
391+
filter = {
392+
must: segments.map((segment, index) => ({
393+
key: `pathSegments.${index}`,
394+
match: { value: segment },
395+
})),
396+
}
397+
}
385398
}
386399
}
387400

0 commit comments

Comments
 (0)