mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-05 08:10:14 +00:00
fix: handle codebase search indexing state properly (#5662)
- Always show codebase_search tool in the tool list - Add runtime state checking with user-friendly feedback - Provide clear messages for each indexing state (Standby, Indexing, Error) - Suggest alternative tools when semantic search is unavailable - Add comprehensive tests for the new behavior This ensures Roo doesn't try to use semantic search when indexing is incomplete and provides clear feedback to users about the current state.
This commit is contained in:
parent
ed3a077c81
commit
616cafc3a1
4 changed files with 377 additions and 10 deletions
|
|
@ -101,13 +101,8 @@ export function getToolDescriptionsForMode(
|
|||
// Add always available tools
|
||||
ALWAYS_AVAILABLE_TOOLS.forEach((tool) => tools.add(tool))
|
||||
|
||||
// Conditionally exclude codebase_search if feature is disabled or not configured
|
||||
if (
|
||||
!codeIndexManager ||
|
||||
!(codeIndexManager.isFeatureEnabled && codeIndexManager.isFeatureConfigured && codeIndexManager.isInitialized)
|
||||
) {
|
||||
tools.delete("codebase_search")
|
||||
}
|
||||
// Note: codebase_search is now always included in the tool list
|
||||
// The tool itself will check the indexing state at runtime and provide appropriate feedback
|
||||
|
||||
// Conditionally exclude update_todo_list if disabled in settings
|
||||
if (settings?.todoListEnabled === false) {
|
||||
|
|
|
|||
344
src/core/tools/__tests__/codebaseSearchTool.spec.ts
Normal file
344
src/core/tools/__tests__/codebaseSearchTool.spec.ts
Normal file
|
|
@ -0,0 +1,344 @@
|
|||
import { describe, it, expect, vi, beforeEach } from "vitest"
|
||||
import { codebaseSearchTool } from "../codebaseSearchTool"
|
||||
import { CodeIndexManager } from "../../../services/code-index/manager"
|
||||
import { Task } from "../../task/Task"
|
||||
import { ToolUse } from "../../../shared/tools"
|
||||
|
||||
// Mock dependencies
|
||||
vi.mock("../../../services/code-index/manager")
|
||||
vi.mock("../../../utils/path", () => ({
|
||||
getWorkspacePath: vi.fn(() => "/test/workspace"),
|
||||
}))
|
||||
vi.mock("../../prompts/responses", () => ({
|
||||
formatResponse: {
|
||||
toolDenied: vi.fn(() => "Tool denied"),
|
||||
},
|
||||
}))
|
||||
vi.mock("vscode", () => ({
|
||||
workspace: {
|
||||
asRelativePath: vi.fn((path: string) => path.replace("/test/workspace/", "")),
|
||||
},
|
||||
}))
|
||||
|
||||
describe("codebaseSearchTool", () => {
|
||||
let mockTask: Task
|
||||
let mockAskApproval: any
|
||||
let mockHandleError: any
|
||||
let mockPushToolResult: any
|
||||
let mockRemoveClosingTag: any
|
||||
let mockCodeIndexManager: any
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
|
||||
// Setup mock task
|
||||
mockTask = {
|
||||
ask: vi.fn().mockResolvedValue(undefined),
|
||||
sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing parameter error"),
|
||||
consecutiveMistakeCount: 0,
|
||||
providerRef: {
|
||||
deref: vi.fn(() => ({
|
||||
context: {},
|
||||
})),
|
||||
},
|
||||
say: vi.fn().mockResolvedValue(undefined),
|
||||
} as any
|
||||
|
||||
// Setup mock functions
|
||||
mockAskApproval = vi.fn().mockResolvedValue(true)
|
||||
mockHandleError = vi.fn()
|
||||
mockPushToolResult = vi.fn()
|
||||
mockRemoveClosingTag = vi.fn((tag, value) => value)
|
||||
|
||||
// Setup mock CodeIndexManager
|
||||
mockCodeIndexManager = {
|
||||
isFeatureEnabled: true,
|
||||
isFeatureConfigured: true,
|
||||
isInitialized: true,
|
||||
state: "Indexed",
|
||||
searchIndex: vi.fn().mockResolvedValue([
|
||||
{
|
||||
score: 0.9,
|
||||
payload: {
|
||||
filePath: "/test/workspace/src/file.ts",
|
||||
startLine: 10,
|
||||
endLine: 20,
|
||||
codeChunk: "test code",
|
||||
},
|
||||
},
|
||||
]),
|
||||
}
|
||||
|
||||
vi.mocked(CodeIndexManager).getInstance = vi.fn((_context: any) => mockCodeIndexManager as any)
|
||||
})
|
||||
|
||||
describe("indexing state checks", () => {
|
||||
it("should provide feedback when indexing is in Standby state", async () => {
|
||||
mockCodeIndexManager.state = "Standby"
|
||||
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "codebase_search",
|
||||
params: { query: "test query" },
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await codebaseSearchTool(
|
||||
mockTask,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
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"))
|
||||
expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("should provide feedback when indexing is in progress", async () => {
|
||||
mockCodeIndexManager.state = "Indexing"
|
||||
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "codebase_search",
|
||||
params: { query: "test query" },
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await codebaseSearchTool(
|
||||
mockTask,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
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"),
|
||||
)
|
||||
expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("should provide feedback when indexing is in Error state", async () => {
|
||||
mockCodeIndexManager.state = "Error"
|
||||
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "codebase_search",
|
||||
params: { query: "test query" },
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await codebaseSearchTool(
|
||||
mockTask,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
expect(mockPushToolResult).toHaveBeenCalledWith(
|
||||
expect.stringContaining("Semantic search is not available yet (currently Error)"),
|
||||
)
|
||||
expect(mockPushToolResult).toHaveBeenCalledWith(
|
||||
expect.stringContaining("Code indexing encountered an error"),
|
||||
)
|
||||
expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("should perform search when indexing is complete (Indexed state)", async () => {
|
||||
mockCodeIndexManager.state = "Indexed"
|
||||
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "codebase_search",
|
||||
params: { query: "test query" },
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await codebaseSearchTool(
|
||||
mockTask,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
expect(mockCodeIndexManager.searchIndex).toHaveBeenCalledWith("test query", undefined)
|
||||
// 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"))
|
||||
expect(mockPushToolResult).not.toHaveBeenCalledWith(
|
||||
expect.stringContaining("Semantic search is not available"),
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe("feature configuration checks", () => {
|
||||
it("should throw error when feature is disabled", async () => {
|
||||
mockCodeIndexManager.isFeatureEnabled = false
|
||||
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "codebase_search",
|
||||
params: { query: "test query" },
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await codebaseSearchTool(
|
||||
mockTask,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
expect(mockHandleError).toHaveBeenCalledWith(
|
||||
"codebase_search",
|
||||
expect.objectContaining({
|
||||
message: "Code Indexing is disabled in the settings.",
|
||||
}),
|
||||
)
|
||||
})
|
||||
|
||||
it("should throw error when feature is not configured", async () => {
|
||||
mockCodeIndexManager.isFeatureConfigured = false
|
||||
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "codebase_search",
|
||||
params: { query: "test query" },
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await codebaseSearchTool(
|
||||
mockTask,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
expect(mockHandleError).toHaveBeenCalledWith(
|
||||
"codebase_search",
|
||||
expect.objectContaining({
|
||||
message: "Code Indexing is not configured (Missing OpenAI Key or Qdrant URL).",
|
||||
}),
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe("parameter validation", () => {
|
||||
it("should handle missing query parameter", async () => {
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "codebase_search",
|
||||
params: {},
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await codebaseSearchTool(
|
||||
mockTask,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
expect(mockTask.sayAndCreateMissingParamError).toHaveBeenCalledWith("codebase_search", "query")
|
||||
expect(mockPushToolResult).toHaveBeenCalledWith("Missing parameter error")
|
||||
})
|
||||
|
||||
it("should handle partial tool use", async () => {
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "codebase_search",
|
||||
params: { query: "test" },
|
||||
partial: true,
|
||||
}
|
||||
|
||||
await codebaseSearchTool(
|
||||
mockTask,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
expect(mockTask.ask).toHaveBeenCalled()
|
||||
expect(mockCodeIndexManager.searchIndex).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe("search results handling", () => {
|
||||
it("should handle empty search results", async () => {
|
||||
mockCodeIndexManager.searchIndex.mockResolvedValue([])
|
||||
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "codebase_search",
|
||||
params: { query: "test query" },
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await codebaseSearchTool(
|
||||
mockTask,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
expect(mockPushToolResult).toHaveBeenCalledWith(
|
||||
'No relevant code snippets found for the query: "test query"',
|
||||
)
|
||||
})
|
||||
|
||||
it("should format search results correctly", async () => {
|
||||
const block: ToolUse = {
|
||||
type: "tool_use",
|
||||
name: "codebase_search",
|
||||
params: { query: "test query" },
|
||||
partial: false,
|
||||
}
|
||||
|
||||
await codebaseSearchTool(
|
||||
mockTask,
|
||||
block,
|
||||
mockAskApproval,
|
||||
mockHandleError,
|
||||
mockPushToolResult,
|
||||
mockRemoveClosingTag,
|
||||
)
|
||||
|
||||
// The tool should call pushToolResult with a single formatted string containing all results
|
||||
expect(mockPushToolResult).toHaveBeenCalledTimes(1)
|
||||
const resultString = mockPushToolResult.mock.calls[0][0]
|
||||
expect(resultString).toContain("Query: test query")
|
||||
expect(resultString).toContain("File path: src/file.ts")
|
||||
expect(resultString).toContain("Score: 0.9")
|
||||
expect(resultString).toContain("Lines: 10-20")
|
||||
expect(resultString).toContain("Code Chunk: test code")
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -82,6 +82,33 @@ export async function codebaseSearchTool(
|
|||
throw new Error("Code Indexing is not configured (Missing OpenAI Key or Qdrant URL).")
|
||||
}
|
||||
|
||||
// Check indexing state at runtime
|
||||
const indexingState = manager.state
|
||||
if (indexingState !== "Indexed") {
|
||||
let stateMessage = ""
|
||||
switch (indexingState) {
|
||||
case "Standby":
|
||||
stateMessage =
|
||||
"Code indexing has not started yet. Please wait for the initial indexing to complete."
|
||||
break
|
||||
case "Indexing":
|
||||
stateMessage =
|
||||
"Code indexing is currently in progress. Semantic search will be available once indexing is complete."
|
||||
break
|
||||
case "Error":
|
||||
stateMessage = "Code indexing encountered an error. Please check your configuration and try again."
|
||||
break
|
||||
default:
|
||||
stateMessage = `Code indexing is in an unexpected state: ${indexingState}`
|
||||
}
|
||||
|
||||
// Return informative message instead of throwing error
|
||||
pushToolResult(
|
||||
`Semantic search is not available yet (currently ${indexingState}).\n\n${stateMessage}\n\nPlease use file reading tools (read_file, search_files) for now.`,
|
||||
)
|
||||
return
|
||||
}
|
||||
|
||||
const searchResults: VectorStoreSearchResult[] = await manager.searchIndex(query, directoryPrefix)
|
||||
|
||||
// 3. Format and push results
|
||||
|
|
|
|||
|
|
@ -34,10 +34,11 @@ export class CodeIndexSearchService {
|
|||
const minScore = this.configManager.currentSearchMinScore
|
||||
const maxResults = this.configManager.currentSearchMaxResults
|
||||
|
||||
// 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 !== "Indexed" && currentState !== "Indexing") {
|
||||
// Allow search during Indexing too
|
||||
throw new Error(`Code index is not ready for search. Current state: ${currentState}`)
|
||||
if (currentState === "Error") {
|
||||
throw new Error(`Code index is in error state. Please check your configuration.`)
|
||||
}
|
||||
|
||||
try {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue