fix: stabilize code indexing retries#10465
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| try { | ||
| await this.vectorStore.deletePointsByMultipleFilePaths(unique) | ||
| return undefined | ||
| } catch (error: any) { |
There was a problem hiding this comment.
WARNING: catch (error: any) with return error as Error is unsafe and violates the style guide's "avoid any" rule.
If error is a non-Error value (e.g. a plain string or number), casting it as Error is a type lie — callers that call .message on the returned value will get undefined. Consider:
const err = error instanceof Error ? error : new Error(String(error))
return errThis pattern is pre-existing in the file but is newly introduced in _clearBatchReplacements.
| export const INITIAL_RETRY_DELAY_MS = 500 | ||
| export const PARSING_CONCURRENCY = 10 | ||
| export const MAX_PENDING_BATCHES = 20 // Maximum number of batches to accumulate before waiting | ||
| export const EMBEDDING_REQUEST_CONCURRENCY = 1 |
There was a problem hiding this comment.
SUGGESTION: EMBEDDING_REQUEST_CONCURRENCY = 1 is a hard-coded global constant shared by both DirectoryScanner and FileWatcher. Setting it to 1 fully serializes all embedding calls, which is correct for rate-limit pressure but significantly reduces throughput on large workspaces where the provider isn't rate-limiting.
Consider making this user-configurable (e.g. as a config key) or at least a higher default (e.g. 3) that can be overridden, rather than a fixed compile-time value.
| if (this._cancelled) return undefined | ||
|
|
||
| log.debug(`Creating embeddings for ${batchTexts.length} texts`) | ||
| const { embeddings } = await this.embedLimit(() => this.embedder.createEmbeddings(batchTexts)) |
There was a problem hiding this comment.
SUGGESTION: The embedding step is now extracted outside the retry loop, which is correct — there's no point regenerating identical embeddings on retry. However, note that the deletion step (lines 594–631) still runs on every retry attempt. If the first attempt successfully deletes but then the upsert fails, the second retry will call deletePointsByMultipleFilePaths again on already-deleted paths. This is a pre-existing behavior and likely idempotent in Qdrant, but it may be worth a comment near the deletion step noting this is intentionally idempotent.
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Other Observations (not in diff)
Files Reviewed (13 files)
Fix these issues in Kilo Cloud Reviewed by claude-sonnet-4.6 · 341,142 tokens Review guidance: REVIEW.md from base branch |
1 participant
Large code indexing runs can overload embedding providers with bursty requests, then leave incremental updates looking complete even when replacement work failed.
This change bounds indexing embedding pressure, preserves existing vectors until replacements are ready, and keeps failed incremental work retryable and visible instead of reporting a false complete state.
Settings saves also finish their UI acknowledgement when the post-write config refresh stalls during reload churn, so persisted config changes do not leave the settings surface stuck dirty.
Fixes #8311.