Skip to content

Commit 53f58bd

Browse files
committed
fix(code-index): exclude metadata points at query-time in Qdrant and gate orchestrator cleanup behind indexingStarted
- Merge must_not: [{ key: "type", match: { value: "metadata" } }] into QdrantVectorStore.search() filter ([QdrantVectorStore.search()](src/services/code-index/vector-store/qdrant-client.ts:383)) - Normalize constants import path in [qdrant-client.ts](src/services/code-index/vector-store/qdrant-client.ts:8) - Only invoke clearCollection()/cache clear after initialize() succeeds in [CodeIndexOrchestrator.startIndexing()](src/services/code-index/orchestrator.ts:291) test: update qdrant-client search expectations and add orchestrator error-path gating test
1 parent 9ddaa58 commit 53f58bd

File tree

4 files changed

+272
-88
lines changed

4 files changed

+272
-88
lines changed
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
import { describe, it, expect, beforeEach, vi } from "vitest"
2+
import { CodeIndexOrchestrator } from "../orchestrator"
3+
4+
// Mock vscode workspace so startIndexing passes workspace check
5+
vi.mock("vscode", () => {
6+
const path = require("path")
7+
const testWorkspacePath = path.join(path.sep, "test", "workspace")
8+
return {
9+
window: {
10+
activeTextEditor: null,
11+
},
12+
workspace: {
13+
workspaceFolders: [
14+
{
15+
uri: { fsPath: testWorkspacePath },
16+
name: "test",
17+
index: 0,
18+
},
19+
],
20+
createFileSystemWatcher: vi.fn().mockReturnValue({
21+
onDidCreate: vi.fn().mockReturnValue({ dispose: vi.fn() }),
22+
onDidChange: vi.fn().mockReturnValue({ dispose: vi.fn() }),
23+
onDidDelete: vi.fn().mockReturnValue({ dispose: vi.fn() }),
24+
dispose: vi.fn(),
25+
}),
26+
},
27+
RelativePattern: vi.fn().mockImplementation((base: string, pattern: string) => ({ base, pattern })),
28+
}
29+
})
30+
31+
// Mock TelemetryService
32+
vi.mock("@roo-code/telemetry", () => ({
33+
TelemetryService: {
34+
instance: {
35+
captureEvent: vi.fn(),
36+
},
37+
},
38+
}))
39+
40+
// Mock i18n translator used in orchestrator messages
41+
vi.mock("../../i18n", () => ({
42+
t: (key: string, params?: any) => {
43+
if (key === "embeddings:orchestrator.failedDuringInitialScan" && params?.errorMessage) {
44+
return `Failed during initial scan: ${params.errorMessage}`
45+
}
46+
return key
47+
},
48+
}))
49+
50+
describe("CodeIndexOrchestrator - error path cleanup gating", () => {
51+
const workspacePath = "/test/workspace"
52+
53+
let configManager: any
54+
let stateManager: any
55+
let cacheManager: any
56+
let vectorStore: any
57+
let scanner: any
58+
let fileWatcher: any
59+
60+
beforeEach(() => {
61+
vi.clearAllMocks()
62+
63+
configManager = {
64+
isFeatureConfigured: true,
65+
}
66+
67+
// Minimal state manager that tracks state transitions
68+
let currentState = "Standby"
69+
stateManager = {
70+
get state() {
71+
return currentState
72+
},
73+
setSystemState: vi.fn().mockImplementation((state: string, _msg: string) => {
74+
currentState = state
75+
}),
76+
reportFileQueueProgress: vi.fn(),
77+
reportBlockIndexingProgress: vi.fn(),
78+
}
79+
80+
cacheManager = {
81+
clearCacheFile: vi.fn().mockResolvedValue(undefined),
82+
}
83+
84+
vectorStore = {
85+
initialize: vi.fn(),
86+
hasIndexedData: vi.fn(),
87+
markIndexingIncomplete: vi.fn(),
88+
markIndexingComplete: vi.fn(),
89+
clearCollection: vi.fn().mockResolvedValue(undefined),
90+
}
91+
92+
scanner = {
93+
scanDirectory: vi.fn(),
94+
}
95+
96+
fileWatcher = {
97+
initialize: vi.fn().mockResolvedValue(undefined),
98+
onDidStartBatchProcessing: vi.fn().mockReturnValue({ dispose: vi.fn() }),
99+
onBatchProgressUpdate: vi.fn().mockReturnValue({ dispose: vi.fn() }),
100+
onDidFinishBatchProcessing: vi.fn().mockReturnValue({ dispose: vi.fn() }),
101+
dispose: vi.fn(),
102+
}
103+
})
104+
105+
it("should not call clearCollection() or clear cache when initialize() fails (indexing not started)", async () => {
106+
// Arrange: fail at initialize()
107+
vectorStore.initialize.mockRejectedValue(new Error("Qdrant unreachable"))
108+
109+
const orchestrator = new CodeIndexOrchestrator(
110+
configManager,
111+
stateManager,
112+
workspacePath,
113+
cacheManager,
114+
vectorStore,
115+
scanner,
116+
fileWatcher,
117+
)
118+
119+
// Act
120+
await orchestrator.startIndexing()
121+
122+
// Assert
123+
expect(vectorStore.clearCollection).not.toHaveBeenCalled()
124+
expect(cacheManager.clearCacheFile).not.toHaveBeenCalled()
125+
126+
// Error state should be set
127+
expect(stateManager.setSystemState).toHaveBeenCalled()
128+
const lastCall = stateManager.setSystemState.mock.calls[stateManager.setSystemState.mock.calls.length - 1]
129+
expect(lastCall[0]).toBe("Error")
130+
})
131+
132+
it("should call clearCollection() and clear cache when an error occurs after initialize() succeeds (indexing started)", async () => {
133+
// Arrange: initialize succeeds; fail soon after to enter error path with indexingStarted=true
134+
vectorStore.initialize.mockResolvedValue(false) // existing collection
135+
vectorStore.hasIndexedData.mockResolvedValue(false) // force full scan path
136+
vectorStore.markIndexingIncomplete.mockRejectedValue(new Error("mark incomplete failure"))
137+
138+
const orchestrator = new CodeIndexOrchestrator(
139+
configManager,
140+
stateManager,
141+
workspacePath,
142+
cacheManager,
143+
vectorStore,
144+
scanner,
145+
fileWatcher,
146+
)
147+
148+
// Act
149+
await orchestrator.startIndexing()
150+
151+
// Assert: cleanup gated behind indexingStarted should have happened
152+
expect(vectorStore.clearCollection).toHaveBeenCalledTimes(1)
153+
expect(cacheManager.clearCacheFile).toHaveBeenCalledTimes(1)
154+
155+
// Error state should be set
156+
expect(stateManager.setSystemState).toHaveBeenCalled()
157+
const lastCall = stateManager.setSystemState.mock.calls[stateManager.setSystemState.mock.calls.length - 1]
158+
expect(lastCall[0]).toBe("Error")
159+
})
160+
})

src/services/code-index/orchestrator.ts

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -288,15 +288,17 @@ export class CodeIndexOrchestrator {
288288
stack: error instanceof Error ? error.stack : undefined,
289289
location: "startIndexing",
290290
})
291-
try {
292-
await this.vectorStore.clearCollection()
293-
} catch (cleanupError) {
294-
console.error("[CodeIndexOrchestrator] Failed to clean up after error:", cleanupError)
295-
TelemetryService.instance.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, {
296-
error: cleanupError instanceof Error ? cleanupError.message : String(cleanupError),
297-
stack: cleanupError instanceof Error ? cleanupError.stack : undefined,
298-
location: "startIndexing.cleanup",
299-
})
291+
if (indexingStarted) {
292+
try {
293+
await this.vectorStore.clearCollection()
294+
} catch (cleanupError) {
295+
console.error("[CodeIndexOrchestrator] Failed to clean up after error:", cleanupError)
296+
TelemetryService.instance.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, {
297+
error: cleanupError instanceof Error ? cleanupError.message : String(cleanupError),
298+
stack: cleanupError instanceof Error ? cleanupError.stack : undefined,
299+
location: "startIndexing.cleanup",
300+
})
301+
}
300302
}
301303

302304
// Only clear cache if indexing had started (Qdrant connection succeeded)

0 commit comments

Comments
 (0)