Skip to content

Commit 2ff2b04

Browse files
committed
fix: remove error state check from search service and improve tests
- Remove error state check from search-service.ts as state checking is now handled in codebaseSearchTool - Add comprehensive test coverage for search service - Add translations for error messages - Consolidate duplicate test assertions for better readability
1 parent 5ce9ea5 commit 2ff2b04

File tree

5 files changed

+191
-30
lines changed

5 files changed

+191
-30
lines changed

src/core/tools/__tests__/codebaseSearchTool.spec.ts

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,16 @@ vi.mock("vscode", () => ({
1919
asRelativePath: vi.fn((path: string) => path.replace("/test/workspace/", "")),
2020
},
2121
}))
22+
vi.mock("../../../i18n", () => ({
23+
t: vi.fn((key: string) => {
24+
const translations: Record<string, string> = {
25+
"tools.codebaseSearch.errors.disabled": "Code Indexing is disabled in the settings.",
26+
"tools.codebaseSearch.errors.notConfigured":
27+
"Code Indexing is not configured (Missing OpenAI Key or Qdrant URL).",
28+
}
29+
return translations[key] || key
30+
}),
31+
}))
2232

2333
describe("codebaseSearchTool", () => {
2434
let mockTask: Task
@@ -92,13 +102,11 @@ describe("codebaseSearchTool", () => {
92102
mockRemoveClosingTag,
93103
)
94104

95-
expect(mockPushToolResult).toHaveBeenCalledWith(
96-
expect.stringContaining("Semantic search is not available yet (currently Standby)"),
97-
)
98-
expect(mockPushToolResult).toHaveBeenCalledWith(
99-
expect.stringContaining("Code indexing has not started yet"),
100-
)
101-
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Please use file reading tools"))
105+
// Verify the complete message was pushed
106+
const pushedMessage = mockPushToolResult.mock.calls[0][0]
107+
expect(pushedMessage).toContain("Semantic search is not available yet (currently Standby)")
108+
expect(pushedMessage).toContain("Code indexing has not started yet")
109+
expect(pushedMessage).toContain("Please use file reading tools")
102110
expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled()
103111
})
104112

@@ -121,12 +129,10 @@ describe("codebaseSearchTool", () => {
121129
mockRemoveClosingTag,
122130
)
123131

124-
expect(mockPushToolResult).toHaveBeenCalledWith(
125-
expect.stringContaining("Semantic search is not available yet (currently Indexing)"),
126-
)
127-
expect(mockPushToolResult).toHaveBeenCalledWith(
128-
expect.stringContaining("Code indexing is currently in progress"),
129-
)
132+
// Verify the complete message was pushed
133+
const pushedMessage = mockPushToolResult.mock.calls[0][0]
134+
expect(pushedMessage).toContain("Semantic search is not available yet (currently Indexing)")
135+
expect(pushedMessage).toContain("Code indexing is currently in progress")
130136
expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled()
131137
})
132138

@@ -149,12 +155,10 @@ describe("codebaseSearchTool", () => {
149155
mockRemoveClosingTag,
150156
)
151157

152-
expect(mockPushToolResult).toHaveBeenCalledWith(
153-
expect.stringContaining("Semantic search is not available yet (currently Error)"),
154-
)
155-
expect(mockPushToolResult).toHaveBeenCalledWith(
156-
expect.stringContaining("Code indexing encountered an error"),
157-
)
158+
// Verify the complete message was pushed
159+
const pushedMessage = mockPushToolResult.mock.calls[0][0]
160+
expect(pushedMessage).toContain("Semantic search is not available yet (currently Error)")
161+
expect(pushedMessage).toContain("Code indexing encountered an error")
158162
expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled()
159163
})
160164

@@ -181,8 +185,9 @@ describe("codebaseSearchTool", () => {
181185
// Check that say was called with the search results
182186
expect(mockTask.say).toHaveBeenCalledWith("codebase_search_result", expect.stringContaining("test code"))
183187
// Check that pushToolResult was called with the formatted output
184-
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Query: test query"))
185-
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("test code"))
188+
const pushedResult = mockPushToolResult.mock.calls[0][0]
189+
expect(pushedResult).toContain("Query: test query")
190+
expect(pushedResult).toContain("test code")
186191
expect(mockPushToolResult).not.toHaveBeenCalledWith(
187192
expect.stringContaining("Semantic search is not available"),
188193
)
@@ -212,7 +217,7 @@ describe("codebaseSearchTool", () => {
212217
expect(mockHandleError).toHaveBeenCalledWith(
213218
"codebase_search",
214219
expect.objectContaining({
215-
message: "Code Indexing is disabled in the settings.",
220+
message: expect.stringContaining("Code Indexing is disabled"),
216221
}),
217222
)
218223
})
@@ -239,7 +244,7 @@ describe("codebaseSearchTool", () => {
239244
expect(mockHandleError).toHaveBeenCalledWith(
240245
"codebase_search",
241246
expect.objectContaining({
242-
message: "Code Indexing is not configured (Missing OpenAI Key or Qdrant URL).",
247+
message: expect.stringContaining("Code Indexing is not configured"),
243248
}),
244249
)
245250
})

src/core/tools/codebaseSearchTool.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { formatResponse } from "../prompts/responses"
77
import { VectorStoreSearchResult } from "../../services/code-index/interfaces"
88
import { AskApproval, HandleError, PushToolResult, RemoveClosingTag, ToolUse } from "../../shared/tools"
99
import path from "path"
10+
import { t } from "../../i18n"
1011

1112
export async function codebaseSearchTool(
1213
cline: Task,
@@ -76,10 +77,10 @@ export async function codebaseSearchTool(
7677
}
7778

7879
if (!manager.isFeatureEnabled) {
79-
throw new Error("Code Indexing is disabled in the settings.")
80+
throw new Error(t("tools.codebaseSearch.errors.disabled"))
8081
}
8182
if (!manager.isFeatureConfigured) {
82-
throw new Error("Code Indexing is not configured (Missing OpenAI Key or Qdrant URL).")
83+
throw new Error(t("tools.codebaseSearch.errors.notConfigured"))
8384
}
8485

8586
// Check indexing state at runtime

src/i18n/locales/en/tools.json

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@
66
},
77
"toolRepetitionLimitReached": "Roo appears to be stuck in a loop, attempting the same action ({{toolName}}) repeatedly. This might indicate a problem with its current strategy. Consider rephrasing the task, providing more specific instructions, or guiding it towards a different approach.",
88
"codebaseSearch": {
9-
"approval": "Searching for '{{query}}' in codebase..."
9+
"approval": "Searching for '{{query}}' in codebase...",
10+
"errors": {
11+
"disabled": "Code Indexing is disabled in the settings.",
12+
"notConfigured": "Code Indexing is not configured (Missing OpenAI Key or Qdrant URL)."
13+
}
1014
},
1115
"newTask": {
1216
"errors": {
Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
import { describe, it, expect, vi, beforeEach } from "vitest"
2+
import { CodeIndexSearchService } from "../search-service"
3+
import { CodeIndexConfigManager } from "../config-manager"
4+
import { CodeIndexStateManager } from "../state-manager"
5+
import { IEmbedder } from "../interfaces/embedder"
6+
import { IVectorStore } from "../interfaces/vector-store"
7+
import { TelemetryService } from "@roo-code/telemetry"
8+
import { TelemetryEventName } from "@roo-code/types"
9+
10+
// Mock dependencies
11+
vi.mock("@roo-code/telemetry")
12+
13+
describe("CodeIndexSearchService", () => {
14+
let searchService: CodeIndexSearchService
15+
let mockConfigManager: any
16+
let mockStateManager: any
17+
let mockEmbedder: any
18+
let mockVectorStore: any
19+
20+
beforeEach(() => {
21+
vi.clearAllMocks()
22+
23+
// Setup mock config manager
24+
mockConfigManager = {
25+
isFeatureEnabled: true,
26+
isFeatureConfigured: true,
27+
currentSearchMinScore: 0.5,
28+
currentSearchMaxResults: 10,
29+
}
30+
31+
// Setup mock state manager
32+
mockStateManager = {
33+
getCurrentStatus: vi.fn(() => ({ systemStatus: "Indexed" })),
34+
setSystemState: vi.fn(),
35+
}
36+
37+
// Setup mock embedder
38+
mockEmbedder = {
39+
createEmbeddings: vi.fn().mockResolvedValue({
40+
embeddings: [[0.1, 0.2, 0.3]],
41+
}),
42+
}
43+
44+
// Setup mock vector store
45+
mockVectorStore = {
46+
search: vi.fn().mockResolvedValue([
47+
{
48+
score: 0.9,
49+
payload: {
50+
filePath: "/test/file.ts",
51+
startLine: 1,
52+
endLine: 10,
53+
codeChunk: "test code",
54+
},
55+
},
56+
]),
57+
}
58+
59+
// Setup mock telemetry
60+
const mockTelemetryInstance = {
61+
captureEvent: vi.fn(),
62+
}
63+
vi.spyOn(TelemetryService, "instance", "get").mockReturnValue(mockTelemetryInstance as any)
64+
65+
searchService = new CodeIndexSearchService(
66+
mockConfigManager as CodeIndexConfigManager,
67+
mockStateManager as CodeIndexStateManager,
68+
mockEmbedder as IEmbedder,
69+
mockVectorStore as IVectorStore,
70+
)
71+
})
72+
73+
describe("searchIndex", () => {
74+
it("should throw error when feature is disabled", async () => {
75+
mockConfigManager.isFeatureEnabled = false
76+
77+
await expect(searchService.searchIndex("test query")).rejects.toThrow(
78+
"Code index feature is disabled or not configured.",
79+
)
80+
})
81+
82+
it("should throw error when feature is not configured", async () => {
83+
mockConfigManager.isFeatureConfigured = false
84+
85+
await expect(searchService.searchIndex("test query")).rejects.toThrow(
86+
"Code index feature is disabled or not configured.",
87+
)
88+
})
89+
90+
it("should perform search successfully when in Indexed state", async () => {
91+
const query = "test query"
92+
const results = await searchService.searchIndex(query)
93+
94+
expect(mockEmbedder.createEmbeddings).toHaveBeenCalledWith([query])
95+
expect(mockVectorStore.search).toHaveBeenCalledWith([0.1, 0.2, 0.3], undefined, 0.5, 10)
96+
expect(results).toHaveLength(1)
97+
expect(results[0].score).toBe(0.9)
98+
})
99+
100+
it("should handle directory prefix correctly", async () => {
101+
const query = "test query"
102+
const directoryPrefix = "src/components"
103+
104+
await searchService.searchIndex(query, directoryPrefix)
105+
106+
expect(mockVectorStore.search).toHaveBeenCalledWith([0.1, 0.2, 0.3], "src/components", 0.5, 10)
107+
})
108+
109+
it("should NOT throw error when in Error state (state checking moved to tool)", async () => {
110+
mockStateManager.getCurrentStatus.mockReturnValue({ systemStatus: "Error" })
111+
112+
// Should not throw, as state checking is now handled in the tool
113+
const results = await searchService.searchIndex("test query")
114+
expect(results).toHaveLength(1)
115+
})
116+
117+
it("should handle embedding generation failure", async () => {
118+
mockEmbedder.createEmbeddings.mockResolvedValue({ embeddings: [] })
119+
120+
await expect(searchService.searchIndex("test query")).rejects.toThrow(
121+
"Failed to generate embedding for query.",
122+
)
123+
})
124+
125+
it("should capture telemetry and set error state on search failure", async () => {
126+
const error = new Error("Vector store error")
127+
mockVectorStore.search.mockRejectedValue(error)
128+
129+
await expect(searchService.searchIndex("test query")).rejects.toThrow("Vector store error")
130+
131+
expect(mockStateManager.setSystemState).toHaveBeenCalledWith("Error", "Search failed: Vector store error")
132+
expect(TelemetryService.instance.captureEvent).toHaveBeenCalledWith(TelemetryEventName.CODE_INDEX_ERROR, {
133+
error: "Vector store error",
134+
stack: expect.any(String),
135+
location: "searchIndex",
136+
})
137+
})
138+
139+
it("should work correctly when in Indexing state", async () => {
140+
mockStateManager.getCurrentStatus.mockReturnValue({ systemStatus: "Indexing" })
141+
142+
// Should not throw, as state checking is now handled in the tool
143+
const results = await searchService.searchIndex("test query")
144+
expect(results).toHaveLength(1)
145+
})
146+
147+
it("should work correctly when in Standby state", async () => {
148+
mockStateManager.getCurrentStatus.mockReturnValue({ systemStatus: "Standby" })
149+
150+
// Should not throw, as state checking is now handled in the tool
151+
const results = await searchService.searchIndex("test query")
152+
expect(results).toHaveLength(1)
153+
})
154+
})
155+
})

src/services/code-index/search-service.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,6 @@ export class CodeIndexSearchService {
3636

3737
// Note: State checking is now handled in the codebaseSearchTool
3838
// This allows the tool to provide more user-friendly feedback
39-
const currentState = this.stateManager.getCurrentStatus().systemStatus
40-
if (currentState === "Error") {
41-
throw new Error(`Code index is in error state. Please check your configuration.`)
42-
}
4339

4440
try {
4541
// Generate embedding for query

0 commit comments

Comments
 (0)