-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: Gate semantic search until initial indexing completes #8235
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add indexing status check to codebaseSearchTool before allowing search - Show clear error messages when indexing is in progress or not ready - Add pause/skip dialog when task starts during active indexing - Show progress updates if user chooses to wait for indexing - Fall back to file-based search tools when semantic search unavailable - Add comprehensive tests for indexing gate functionality Fixes #8234
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self-review complete. Found some edge cases my past self conveniently ignored.
| await this.say("text", "⏳ Waiting for code indexing to complete...") | ||
|
|
||
| // Poll for indexing completion | ||
| while (!this.abort) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential issue: This polling loop doesn't have a maximum timeout. If indexing gets stuck or fails silently, the task could wait indefinitely.
Suggestion: Add a maximum wait time (e.g., 5-10 minutes) after which it proceeds anyway:
const MAX_WAIT_TIME = 10 * 60 * 1000; // 10 minutes
const startTime = Date.now();
while (!this.abort) {
if (Date.now() - startTime > MAX_WAIT_TIME) {
await this.say("text", "⚠️ Indexing is taking longer than expected. Proceeding without semantic search.");
break;
}
// ... rest of the polling logic
}| // Check if code indexing is in progress and gate semantic search if needed | ||
| const context = this.providerRef.deref()?.context | ||
| if (context) { | ||
| const codeIndexManager = CodeIndexManager.getInstance(context) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing null check: CodeIndexManager.getInstance() can return undefined if no workspace is open. This could cause a runtime error.
Fix:
const codeIndexManager = CodeIndexManager.getInstance(context)
if (!codeIndexManager) {
return; // Early exit if no manager available
}
if (codeIndexManager.isFeatureEnabled && codeIndexManager.isFeatureConfigured) {
// ... rest of the logic
}| if (status.systemStatus === "Indexing") { | ||
| const progressPercentage = | ||
| status.totalItems > 0 ? Math.round((status.processedItems / status.totalItems) * 100) : 0 | ||
| throw new Error( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Make error messages more actionable by suggesting alternatives:
throw new Error(
`Code indexing is currently in progress (${progressPercentage}% complete). ` +
`Please wait for indexing to complete, or use the 'search_files' tool for regex-based search instead.`
)|
|
||
| // Check if index is ready | ||
| if (status.systemStatus === "Standby" || status.systemStatus === "Error") { | ||
| throw new Error( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Provide more helpful guidance in error messages:
throw new Error(
`Code index is not ready (status: ${status.systemStatus}). ` +
`The index needs to be built before semantic search can be used. ` +
`Please check your indexing configuration in settings, or use the 'search_files' tool as an alternative.`
)| undefined, | ||
| true, // partial update | ||
| ) | ||
| await delay(2000) // Check every 2 seconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: The 2-second polling interval might be too frequent for long indexing operations. Consider using an adaptive interval that increases over time:
let pollInterval = 2000;
const MAX_POLL_INTERVAL = 10000;
// ...
await delay(pollInterval);
pollInterval = Math.min(pollInterval * 1.5, MAX_POLL_INTERVAL);| vi.mocked(formatResponse.missingToolParameterError).mockReturnValue("Missing parameter") | ||
| }) | ||
|
|
||
| describe("indexing status checks", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test coverage suggestion: Consider adding tests for:
- Indexing transitions from "Indexing" to "Error" during the wait
- Case-insensitive variations of "wait" response
- Multiple concurrent tasks trying to use semantic search during indexing
Summary
This PR implements gating for semantic search to ensure it's only available after the initial code index has been built, addressing Issue #8234.
Problem
Solution
The implementation adds intelligent gating that:
Changes
1. Enhanced codebaseSearchTool.ts
2. Task initialization improvements (Task.ts)
initiateTaskLoop3. Comprehensive test coverage
Testing
Screenshots/Demo
The user experience when indexing is in progress:
Fixes #8234
Important
Gates semantic search until initial indexing completes, with user options to wait or skip, and adds tests for indexing status handling.
initiateTaskLoop()checks indexing status and handles "Indexing", "Standby", and "Error" states.codebaseSearchTool.spec.tsfor indexing status checks and search behavior.This description was created by
for cf4f3e1. You can customize this summary. It will automatically update as commits are pushed.