Mirror: Make OpenAI API key optional for local indexing (#5849)#41
Mirror: Make OpenAI API key optional for local indexing (#5849)#41jeremylongshore wants to merge 1 commit intomainfrom
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoMake OpenAI-compatible API key optional for local codebase indexing
WalkthroughsDescription• Make OpenAI-compatible API key optional for local codebase indexing • Update validation logic to only require base URL, not API key • Add fallback dummy API key when none provided to SDK • Extend test coverage for empty API key scenarios Diagramflowchart LR
A["OpenAI-compatible Config"] -->|Previously required| B["API Key + Base URL"]
A -->|Now requires only| C["Base URL"]
C -->|Optional| D["API Key"]
D -->|If empty| E["Use Dummy Key EMPTY"]
E -->|Pass to SDK| F["OpenAI Client"]
C -->|Direct fetch| G["Omit auth headers if empty"]
File Changes1. src/services/code-index/config-manager.ts
|
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Code Review by Qodo
1. Dummy key on SDK path
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| constructor(baseUrl: string, apiKey: string | undefined, modelId?: string, maxItemTokens?: number) { | ||
| if (!baseUrl) { | ||
| throw new Error(t("embeddings:validation.baseUrlRequired")) | ||
| } | ||
| if (!apiKey) { | ||
| throw new Error(t("embeddings:validation.apiKeyRequired")) | ||
| } | ||
|
|
||
| this.baseUrl = baseUrl | ||
| this.apiKey = apiKey | ||
| // kilocode_change | ||
| this.apiKey = (apiKey ?? "").trim() | ||
| const sdkApiKey = this.apiKey || OPENAI_COMPATIBLE_DUMMY_API_KEY | ||
|
|
||
| // Wrap OpenAI client creation to handle invalid API key characters | ||
| try { | ||
| this.embeddingsClient = new OpenAI({ | ||
| baseURL: baseUrl, | ||
| apiKey: apiKey, | ||
| apiKey: sdkApiKey, | ||
| }) |
There was a problem hiding this comment.
1. Dummy key on sdk path 🐞 Bug ✓ Correctness
For non-full endpoint base URLs (e.g. ending in /v1), OpenAICompatibleEmbedder still uses the OpenAI
SDK path, but now instantiates the SDK with a dummy apiKey ("EMPTY") when the real key is empty.
This prevents truly unauthenticated requests for the common “base URL + no key” local setup and can
cause unexpected auth failures.
Agent Prompt
### Issue description
`OpenAICompatibleEmbedder` now accepts an empty API key by creating the OpenAI SDK client with a dummy key (`"EMPTY"`). But for **base URLs** (e.g. `http://localhost:8080/v1`), the embedder still uses the **OpenAI SDK path** (`embeddingsClient.embeddings.create`). That path cannot omit authentication, so the “API key optional” behavior effectively only works for **full endpoint URLs** that trigger the direct-fetch path.
### Issue Context
- Direct-fetch requests correctly omit auth headers when `this.apiKey` is empty.
- The SDK path is still used whenever `isFullUrl` is false, which is common when the UI asks for a “base URL”.
### Fix Focus Areas
- src/services/code-index/embedders/openai-compatible.ts[63-88]
- src/services/code-index/embedders/openai-compatible.ts[180-194]
- src/services/code-index/embedders/openai-compatible.ts[264-291]
- src/services/code-index/embedders/openai-compatible.ts[371-390]
### Suggested implementation direction
- If `this.apiKey` is empty, route both batching and validation through `makeDirectEmbeddingRequest`.
- For non-full URLs, build an embeddings URL safely, e.g. `new URL("embeddings", baseUrl.endsWith("/") ? baseUrl : baseUrl + "/")` (taking care with `/v1` and trailing slashes).
- Add/extend unit tests to cover:
- baseUrl like `http://localhost:8080/v1` with empty key should NOT send `Authorization`/`api-key` headers
- validateConfiguration for baseUrl + empty key uses direct fetch and omits auth headers
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
@greptileai review |
|
Merged into |
Review: kilocode Kilo-Org#5849
Checklist
Findings1. Always-initialize pattern in webview handler removes guard checks (severity: yellow)The webview message handler for "saveCodeIndexSettings" now always calls // Before: guarded by isFeatureEnabled && isFeatureConfigured
if (currentCodeIndexManager.isFeatureEnabled && currentCodeIndexManager.isFeatureConfigured) {
if (!currentCodeIndexManager.isInitialized) {
await currentCodeIndexManager.initialize(provider.contextProxy)
}
}
// After: always initialize
await currentCodeIndexManager.initialize(provider.contextProxy)This is intentional -- the author's reasoning is that 2. "EMPTY" fallback API key is functional but could leak in logs (severity: yellow)The embedder uses const OPENAI_COMPATIBLE_DUMMY_API_KEY = "EMPTY"
const sdkApiKey = this.apiKey || OPENAI_COMPATIBLE_DUMMY_API_KEYThis is sent in auth headers for direct fetch mode: headers["api-key"] = authToken // "EMPTY"
headers.Authorization = `Bearer ${authToken}` // "Bearer EMPTY"This works for local providers like LM Studio that do not validate authentication. However:
Not blocking -- the behavior is correct for the target use case, and a Discord user confirmed it works with LM Studio. 3. Manager partial-initialization lifecycle fix is sound (severity: gray)The // Before: assigned immediately, could leave half-initialized state
this._serviceFactory = new CodeIndexServiceFactory(...)
// ... services created from _serviceFactory ...
// After: local variable until success
const serviceFactory = new CodeIndexServiceFactory(...)
// ... services created from serviceFactory ...
this._serviceFactory = serviceFactory // only after successAnd const needsServiceRecreation = requiresRestart || !this.isInitialized || !this._serviceFactoryThis prevents the "CodeIndexManager not initialized" error that occurred when 4. Validation schema exported for testing (severity: gray)
CI Status
Code SnippetsConfig manager -- API key now optional: // config-manager.ts
this.openAiCompatibleOptions = openAiCompatibleBaseUrl
? {
baseUrl: openAiCompatibleBaseUrl,
apiKey: openAiCompatibleApiKey, // may be empty/undefined
}
: undefinedConfiguration check -- only base URL and Qdrant required: // Before: required baseUrl && apiKey && qdrantUrl
const isConfigured = !!(baseUrl && apiKey && qdrantUrl)
// After: only baseUrl && qdrantUrl
const isConfigured = !!(baseUrl && qdrantUrl)VerdictAPPROVE -- This PR correctly addresses a real user-reported issue where local embedding providers (like LM Studio) could not be used for codebase indexing because the UI required an API key. The fix is comprehensive: config manager, service factory, embedder, webview validation, and manager lifecycle are all updated consistently. The "EMPTY" fallback key is functional for the target use case and the manager partial-initialization fix prevents a genuine lifecycle bug. All CI checks pass and a Discord user confirmed the fix works end-to-end. |
Review Journal: kilocode Kilo-Org#5849
SummaryMakes API key optional for OpenAI-compatible embedding providers in codebase indexing, enabling local/self-hosted setups like LM Studio. Also fixes a manager lifecycle bug where partial initialization caused "not initialized" errors. Comprehensive changes across config, service factory, embedder, UI validation, and manager. Approve. First ImpressionsThis is a multi-layer fix touching 13 files across the code-index subsystem. The PR description is detailed with clear user scenario, implementation breakdown, and manual test steps. The scope is broader than the title suggests -- it also fixes a lifecycle regression in the manager. What I Looked At
AnalysisThe fix spans four conceptual layers:
The interaction between these layers is well thought out. The webview handler changes are the most impactful -- removing the Verification
Lessons Learned
Review methodology: AI PR Review Case Studies | Reviewed with GWI + Claude Code |
2 participants
Mirror of Kilo-Org#5849
This PR mirrors the upstream change for multi-AI review analysis.
Bot Review Tracker
Links