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
This commit is contained in:
Daniel Riccio 2025-07-23 12:23:54 -05:00
parent 5ce9ea50f2
commit 2ff2b04434
No known key found for this signature in database
GPG key ID: FFD5FD825F8E8209
5 changed files with 191 additions and 30 deletions

View file

@ -19,6 +19,16 @@ vi.mock("vscode", () => ({
asRelativePath: vi.fn((path: string) => path.replace("/test/workspace/", "")),
},
}))
vi.mock("../../../i18n", () => ({
t: vi.fn((key: string) => {
const translations: Record<string, string> = {
"tools.codebaseSearch.errors.disabled": "Code Indexing is disabled in the settings.",
"tools.codebaseSearch.errors.notConfigured":
"Code Indexing is not configured (Missing OpenAI Key or Qdrant URL).",
}
return translations[key] || key
}),
}))
describe("codebaseSearchTool", () => {
let mockTask: Task
@ -92,13 +102,11 @@ describe("codebaseSearchTool", () => {
mockRemoveClosingTag,
)
expect(mockPushToolResult).toHaveBeenCalledWith(
expect.stringContaining("Semantic search is not available yet (currently Standby)"),
)
expect(mockPushToolResult).toHaveBeenCalledWith(
expect.stringContaining("Code indexing has not started yet"),
)
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Please use file reading tools"))
// Verify the complete message was pushed
const pushedMessage = mockPushToolResult.mock.calls[0][0]
expect(pushedMessage).toContain("Semantic search is not available yet (currently Standby)")
expect(pushedMessage).toContain("Code indexing has not started yet")
expect(pushedMessage).toContain("Please use file reading tools")
expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled()
})
@ -121,12 +129,10 @@ describe("codebaseSearchTool", () => {
mockRemoveClosingTag,
)
expect(mockPushToolResult).toHaveBeenCalledWith(
expect.stringContaining("Semantic search is not available yet (currently Indexing)"),
)
expect(mockPushToolResult).toHaveBeenCalledWith(
expect.stringContaining("Code indexing is currently in progress"),
)
// Verify the complete message was pushed
const pushedMessage = mockPushToolResult.mock.calls[0][0]
expect(pushedMessage).toContain("Semantic search is not available yet (currently Indexing)")
expect(pushedMessage).toContain("Code indexing is currently in progress")
expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled()
})
@ -149,12 +155,10 @@ describe("codebaseSearchTool", () => {
mockRemoveClosingTag,
)
expect(mockPushToolResult).toHaveBeenCalledWith(
expect.stringContaining("Semantic search is not available yet (currently Error)"),
)
expect(mockPushToolResult).toHaveBeenCalledWith(
expect.stringContaining("Code indexing encountered an error"),
)
// Verify the complete message was pushed
const pushedMessage = mockPushToolResult.mock.calls[0][0]
expect(pushedMessage).toContain("Semantic search is not available yet (currently Error)")
expect(pushedMessage).toContain("Code indexing encountered an error")
expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled()
})
@ -181,8 +185,9 @@ describe("codebaseSearchTool", () => {
// Check that say was called with the search results
expect(mockTask.say).toHaveBeenCalledWith("codebase_search_result", expect.stringContaining("test code"))
// Check that pushToolResult was called with the formatted output
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("Query: test query"))
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("test code"))
const pushedResult = mockPushToolResult.mock.calls[0][0]
expect(pushedResult).toContain("Query: test query")
expect(pushedResult).toContain("test code")
expect(mockPushToolResult).not.toHaveBeenCalledWith(
expect.stringContaining("Semantic search is not available"),
)
@ -212,7 +217,7 @@ describe("codebaseSearchTool", () => {
expect(mockHandleError).toHaveBeenCalledWith(
"codebase_search",
expect.objectContaining({
message: "Code Indexing is disabled in the settings.",
message: expect.stringContaining("Code Indexing is disabled"),
}),
)
})
@ -239,7 +244,7 @@ describe("codebaseSearchTool", () => {
expect(mockHandleError).toHaveBeenCalledWith(
"codebase_search",
expect.objectContaining({
message: "Code Indexing is not configured (Missing OpenAI Key or Qdrant URL).",
message: expect.stringContaining("Code Indexing is not configured"),
}),
)
})

View file

@ -7,6 +7,7 @@ import { formatResponse } from "../prompts/responses"
import { VectorStoreSearchResult } from "../../services/code-index/interfaces"
import { AskApproval, HandleError, PushToolResult, RemoveClosingTag, ToolUse } from "../../shared/tools"
import path from "path"
import { t } from "../../i18n"
export async function codebaseSearchTool(
cline: Task,
@ -76,10 +77,10 @@ export async function codebaseSearchTool(
}
if (!manager.isFeatureEnabled) {
throw new Error("Code Indexing is disabled in the settings.")
throw new Error(t("tools.codebaseSearch.errors.disabled"))
}
if (!manager.isFeatureConfigured) {
throw new Error("Code Indexing is not configured (Missing OpenAI Key or Qdrant URL).")
throw new Error(t("tools.codebaseSearch.errors.notConfigured"))
}
// Check indexing state at runtime

View file

@ -6,7 +6,11 @@
},
"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.",
"codebaseSearch": {
"approval": "Searching for '{{query}}' in codebase..."
"approval": "Searching for '{{query}}' in codebase...",
"errors": {
"disabled": "Code Indexing is disabled in the settings.",
"notConfigured": "Code Indexing is not configured (Missing OpenAI Key or Qdrant URL)."
}
},
"newTask": {
"errors": {

View file

@ -0,0 +1,155 @@
import { describe, it, expect, vi, beforeEach } from "vitest"
import { CodeIndexSearchService } from "../search-service"
import { CodeIndexConfigManager } from "../config-manager"
import { CodeIndexStateManager } from "../state-manager"
import { IEmbedder } from "../interfaces/embedder"
import { IVectorStore } from "../interfaces/vector-store"
import { TelemetryService } from "@roo-code/telemetry"
import { TelemetryEventName } from "@roo-code/types"
// Mock dependencies
vi.mock("@roo-code/telemetry")
describe("CodeIndexSearchService", () => {
let searchService: CodeIndexSearchService
let mockConfigManager: any
let mockStateManager: any
let mockEmbedder: any
let mockVectorStore: any
beforeEach(() => {
vi.clearAllMocks()
// Setup mock config manager
mockConfigManager = {
isFeatureEnabled: true,
isFeatureConfigured: true,
currentSearchMinScore: 0.5,
currentSearchMaxResults: 10,
}
// Setup mock state manager
mockStateManager = {
getCurrentStatus: vi.fn(() => ({ systemStatus: "Indexed" })),
setSystemState: vi.fn(),
}
// Setup mock embedder
mockEmbedder = {
createEmbeddings: vi.fn().mockResolvedValue({
embeddings: [[0.1, 0.2, 0.3]],
}),
}
// Setup mock vector store
mockVectorStore = {
search: vi.fn().mockResolvedValue([
{
score: 0.9,
payload: {
filePath: "/test/file.ts",
startLine: 1,
endLine: 10,
codeChunk: "test code",
},
},
]),
}
// Setup mock telemetry
const mockTelemetryInstance = {
captureEvent: vi.fn(),
}
vi.spyOn(TelemetryService, "instance", "get").mockReturnValue(mockTelemetryInstance as any)
searchService = new CodeIndexSearchService(
mockConfigManager as CodeIndexConfigManager,
mockStateManager as CodeIndexStateManager,
mockEmbedder as IEmbedder,
mockVectorStore as IVectorStore,
)
})
describe("searchIndex", () => {
it("should throw error when feature is disabled", async () => {
mockConfigManager.isFeatureEnabled = false
await expect(searchService.searchIndex("test query")).rejects.toThrow(
"Code index feature is disabled or not configured.",
)
})
it("should throw error when feature is not configured", async () => {
mockConfigManager.isFeatureConfigured = false
await expect(searchService.searchIndex("test query")).rejects.toThrow(
"Code index feature is disabled or not configured.",
)
})
it("should perform search successfully when in Indexed state", async () => {
const query = "test query"
const results = await searchService.searchIndex(query)
expect(mockEmbedder.createEmbeddings).toHaveBeenCalledWith([query])
expect(mockVectorStore.search).toHaveBeenCalledWith([0.1, 0.2, 0.3], undefined, 0.5, 10)
expect(results).toHaveLength(1)
expect(results[0].score).toBe(0.9)
})
it("should handle directory prefix correctly", async () => {
const query = "test query"
const directoryPrefix = "src/components"
await searchService.searchIndex(query, directoryPrefix)
expect(mockVectorStore.search).toHaveBeenCalledWith([0.1, 0.2, 0.3], "src/components", 0.5, 10)
})
it("should NOT throw error when in Error state (state checking moved to tool)", async () => {
mockStateManager.getCurrentStatus.mockReturnValue({ systemStatus: "Error" })
// Should not throw, as state checking is now handled in the tool
const results = await searchService.searchIndex("test query")
expect(results).toHaveLength(1)
})
it("should handle embedding generation failure", async () => {
mockEmbedder.createEmbeddings.mockResolvedValue({ embeddings: [] })
await expect(searchService.searchIndex("test query")).rejects.toThrow(
"Failed to generate embedding for query.",
)
})
it("should capture telemetry and set error state on search failure", async () => {
const error = new Error("Vector store error")
mockVectorStore.search.mockRejectedValue(error)
await expect(searchService.searchIndex("test query")).rejects.toThrow("Vector store error")
expect(mockStateManager.setSystemState).toHaveBeenCalledWith("Error", "Search failed: Vector store error")
expect(TelemetryService.instance.captureEvent).toHaveBeenCalledWith(TelemetryEventName.CODE_INDEX_ERROR, {
error: "Vector store error",
stack: expect.any(String),
location: "searchIndex",
})
})
it("should work correctly when in Indexing state", async () => {
mockStateManager.getCurrentStatus.mockReturnValue({ systemStatus: "Indexing" })
// Should not throw, as state checking is now handled in the tool
const results = await searchService.searchIndex("test query")
expect(results).toHaveLength(1)
})
it("should work correctly when in Standby state", async () => {
mockStateManager.getCurrentStatus.mockReturnValue({ systemStatus: "Standby" })
// Should not throw, as state checking is now handled in the tool
const results = await searchService.searchIndex("test query")
expect(results).toHaveLength(1)
})
})
})

View file

@ -36,10 +36,6 @@ export class CodeIndexSearchService {
// Note: State checking is now handled in the codebaseSearchTool
// This allows the tool to provide more user-friendly feedback
const currentState = this.stateManager.getCurrentStatus().systemStatus
if (currentState === "Error") {
throw new Error(`Code index is in error state. Please check your configuration.`)
}
try {
// Generate embedding for query