Skip to content

fix: improve code index error handling and add text-embedding-v4 support #7357

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

fix: improve code index error handling and add text-embedding-v4 support #7357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 92 additions & 5 deletions src/core/webview/webviewMessageHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2266,21 +2266,108 @@ export const webviewMessageHandler = async (
}
if (manager.isFeatureEnabled && manager.isFeatureConfigured) {
if (!manager.isInitialized) {
await manager.initialize(provider.contextProxy)
try {
await manager.initialize(provider.contextProxy)
} catch (initError) {
// Initialization failed - send detailed error to user
const errorMessage = initError instanceof Error ? initError.message : String(initError)
provider.log(`Code index initialization failed: ${errorMessage}`)

// Send error status to webview with user-friendly message
provider.postMessageToWebview({
type: "indexingStatusUpdate",
values: {
systemStatus: "Error",
message: errorMessage,
processedItems: 0,
totalItems: 0,
currentItemUnit: "items",
},
})

// Show error notification to user
vscode.window.showErrorMessage(
t("embeddings:validation.initializationFailed", { error: errorMessage }) ||
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid using inline fallback English strings in translation calls. The t() calls (e.g. for 'initializationFailed' and 'startIndexingFailed') already have definitions in the i18n JSON; remove the || fallback portions.

Suggested change
t("embeddings:validation.initializationFailed", { error: errorMessage }) ||
t("embeddings:validation.initializationFailed", { error: errorMessage }),

This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.

`Code indexing initialization failed: ${errorMessage}`,
)
return
}
Copy link
Contributor Author

@roomote roomote bot Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice there's duplicate error handling logic between the initialization (lines 2269-2294) and re-initialization (lines 2302-2329) blocks. Could we extract this into a helper function to reduce duplication? Something like:

failed: Code indexing initialization failed:

}

// startIndexing now handles error recovery internally
manager.startIndexing()

// If startIndexing recovered from error, we need to reinitialize
if (!manager.isInitialized) {
Copy link
Contributor Author

@roomote roomote bot Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is immediate re-initialization after failure intentional? Consider adding a delay or maximum retry count to prevent rapid repeated failures that could impact performance.

await manager.initialize(provider.contextProxy)
// Try starting again after initialization
manager.startIndexing()
try {
await manager.initialize(provider.contextProxy)
// Try starting again after initialization
manager.startIndexing()
} catch (reinitError) {
// Re-initialization failed - send detailed error to user
const errorMessage =
reinitError instanceof Error ? reinitError.message : String(reinitError)
provider.log(`Code index re-initialization failed: ${errorMessage}`)

// Send error status to webview
provider.postMessageToWebview({
type: "indexingStatusUpdate",
values: {
systemStatus: "Error",
message: errorMessage,
processedItems: 0,
totalItems: 0,
currentItemUnit: "items",
},
})

// Show error notification to user
vscode.window.showErrorMessage(
t("embeddings:validation.initializationFailed", { error: errorMessage }) ||
`Code indexing initialization failed: ${errorMessage}`,
)
}
}
Copy link
Contributor Author

@roomote roomote bot Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding test coverage for these new error handling scenarios. Would be good to have tests for:

  • Initialization failure with text-embedding-v4
  • Error message display to users
  • Re-initialization failure scenarios

This would help ensure the error handling continues to work as expected.

} else {
// Feature is not enabled or not configured
const message = !manager.isFeatureEnabled
? t("embeddings:validation.featureDisabled") || "Code indexing is disabled"
: t("embeddings:validation.notConfigured") || "Code indexing is not properly configured"

provider.postMessageToWebview({
type: "indexingStatusUpdate",
values: {
systemStatus: "Error",
message: message,
processedItems: 0,
totalItems: 0,
currentItemUnit: "items",
},
})

vscode.window.showErrorMessage(message)
}
} catch (error) {
provider.log(`Error starting indexing: ${error instanceof Error ? error.message : String(error)}`)
const errorMessage = error instanceof Error ? error.message : String(error)
provider.log(`Error starting indexing: ${errorMessage}`)

// Send error status to webview
provider.postMessageToWebview({
type: "indexingStatusUpdate",
values: {
systemStatus: "Error",
message: errorMessage,
processedItems: 0,
totalItems: 0,
currentItemUnit: "items",
},
})

// Show error notification to user
vscode.window.showErrorMessage(
t("embeddings:validation.startIndexingFailed", { error: errorMessage }) ||
`Failed to start indexing: ${errorMessage}`,
)
}
break
}
Expand Down
6 changes: 5 additions & 1 deletion src/i18n/locales/en/embeddings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@
"invalidModel": "Invalid model. Please check your model configuration.",
"invalidResponse": "Invalid response from embedder service. Please check your configuration.",
"apiKeyRequired": "API key is required for this embedder",
"baseUrlRequired": "Base URL is required for this embedder"
"baseUrlRequired": "Base URL is required for this embedder",
"initializationFailed": "Code indexing initialization failed: {{error}}",
"featureDisabled": "Code indexing is disabled. Please enable it in the settings.",
"notConfigured": "Code indexing is not properly configured. Please check your settings.",
"startIndexingFailed": "Failed to start indexing: {{error}}"
},
"serviceFactory": {
"openAiConfigMissing": "OpenAI configuration missing for embedder creation",
Expand Down
1 change: 1 addition & 0 deletions src/shared/embeddingModels.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export const EMBEDDING_MODEL_PROFILES: EmbeddingModelProfiles = {
"text-embedding-3-small": { dimension: 1536, scoreThreshold: 0.4 },
"text-embedding-3-large": { dimension: 3072, scoreThreshold: 0.4 },
"text-embedding-ada-002": { dimension: 1536, scoreThreshold: 0.4 },
"text-embedding-v4": { dimension: 1536, scoreThreshold: 0.4 }, // Added support for text-embedding-v4
Copy link
Contributor Author

@roomote roomote bot Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment could be more descriptive. Consider:

"nomic-embed-code": {
dimension: 3584,
scoreThreshold: 0.15,
Expand Down
Loading