feat: auto-detect embedding provider from backend .env #571
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a new IPC endpoint and preload API to detect backend embedding provider from a backend .env (GRAPHITI_EMBEDDER_PROVIDER); new shared types and IPC constant; main-process .env parsing and mapping helpers; renderer wiring and onboarding/project settings UI updates to support an OpenRouter provider and OpenRouter API key; mock support for browser mode. Changes
Sequence Diagram(s)sequenceDiagram
participant R as Renderer (MemoryStep / Settings UI)
participant P as Preload (ipcRenderer)
participant M as Main process (ipcMain)
participant FS as File System
R->>P: getBackendEnvEmbeddingConfig()
activate P
P->>M: BACKEND_ENV_GET_EMBEDDING_CONFIG
activate M
M->>M: getEffectiveSourcePath()
alt source path found
M->>FS: readFileSync(<source>/.env)
activate FS
FS-->>M: file contents / error
deactivate FS
rect rgb(220,245,255)
Note over M: parseEnvFile -> extract GRAPHITI_EMBEDDER_PROVIDER\nmapEmbeddingProvider -> BackendEmbeddingProvider
end
M-->>P: IPCResult{ success, data:{ embeddingProvider, envExists:true, sourcePath } }
else no source path or no .env
M-->>P: IPCResult{ success, data:{ embeddingProvider:'openai', envExists:false, sourcePath:undefined } }
end
deactivate M
P-->>R: IPCResult<BackendEnvEmbeddingConfig>
deactivate P
alt success && provider supported
R->>R: apply detected provider to UI/config (may show OpenRouter key input)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
🧰 Additional context used📓 Path-based instructions (3)apps/frontend/src/**/*.{ts,tsx,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
apps/frontend/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
apps/frontend/**/*.{ts,tsx}⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (4)📚 Learning: 2025-12-30T16:38:36.314ZApplied to files:
📚 Learning: 2025-12-30T16:38:36.314ZApplied to files:
📚 Learning: 2025-12-30T16:38:36.314ZApplied to files:
📚 Learning: 2025-12-30T16:38:36.314ZApplied to files:
🧬 Code graph analysis (6)apps/frontend/src/preload/api/settings-api.ts (1)
apps/frontend/src/renderer/components/onboarding/MemoryStep.tsx (2)
apps/frontend/src/main/ipc-handlers/settings-handlers.ts (4)
apps/frontend/src/shared/types/ipc.ts (1)
apps/frontend/src/renderer/components/project-settings/SecuritySettings.tsx (2)
apps/frontend/src/renderer/components/project-settings/MemoryBackendSection.tsx (2)
🔇 Additional comments (20)
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 |
Summary of ChangesHello @doublefx, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant quality-of-life improvement by automating the setup of embedding providers. It enables the frontend to read the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a helpful feature to auto-detect the embedding provider from the backend's .env file, which improves the onboarding experience by pre-configuring the memory settings. The implementation is well-structured, adding a new IPC handler to read the configuration in the main process and applying it in the frontend's MemoryStep component.
My review focuses on enhancing the robustness and maintainability of the new code. I've suggested using a dedicated library for .env parsing to handle edge cases gracefully, refactoring a couple of functions for better clarity and conciseness, and improving error handling in the React component. Overall, this is a great addition.
apps/frontend/src/renderer/components/onboarding/MemoryStep.tsx
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
apps/frontend/src/main/ipc-handlers/settings-handlers.tsapps/frontend/src/preload/api/settings-api.tsapps/frontend/src/renderer/components/onboarding/MemoryStep.tsxapps/frontend/src/renderer/lib/mocks/settings-mock.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/shared/types/ipc.tsapps/frontend/src/shared/types/settings.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/renderer/components/onboarding/MemoryStep.tsxapps/frontend/src/shared/types/ipc.tsapps/frontend/src/preload/api/settings-api.tsapps/frontend/src/shared/types/settings.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/renderer/lib/mocks/settings-mock.tsapps/frontend/src/main/ipc-handlers/settings-handlers.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/renderer/components/onboarding/MemoryStep.tsxapps/frontend/src/shared/types/ipc.tsapps/frontend/src/preload/api/settings-api.tsapps/frontend/src/shared/types/settings.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/renderer/lib/mocks/settings-mock.tsapps/frontend/src/main/ipc-handlers/settings-handlers.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/onboarding/MemoryStep.tsxapps/frontend/src/shared/types/ipc.tsapps/frontend/src/preload/api/settings-api.tsapps/frontend/src/shared/types/settings.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/renderer/lib/mocks/settings-mock.tsapps/frontend/src/main/ipc-handlers/settings-handlers.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/backend/.env* : Configure memory system credentials in `apps/backend/.env` and validate with `graphiti_config.py`
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/backend/.env* : Enable Electron MCP for E2E testing by setting `ELECTRON_MCP_ENABLED=true` in `.env` and starting the Electron app with `npm run dev`
Applied to files:
apps/frontend/src/shared/types/ipc.tsapps/frontend/src/main/ipc-handlers/settings-handlers.ts
🧬 Code graph analysis (3)
apps/frontend/src/shared/types/ipc.ts (1)
apps/frontend/src/shared/types/settings.ts (1)
BackendEnvEmbeddingConfig(324-331)
apps/frontend/src/preload/api/settings-api.ts (1)
apps/frontend/src/shared/types/settings.ts (1)
BackendEnvEmbeddingConfig(324-331)
apps/frontend/src/main/ipc-handlers/settings-handlers.ts (2)
apps/frontend/src/shared/types/settings.ts (2)
BackendEmbeddingProvider(322-322)BackendEnvEmbeddingConfig(324-331)apps/frontend/src/main/updater/path-resolver.ts (1)
getEffectiveSourcePath(57-89)
🔇 Additional comments (11)
apps/frontend/src/shared/constants/ipc.ts (1)
398-400: LGTM!The new IPC channel constant follows the established naming convention and is appropriately grouped with other environment configuration channels.
apps/frontend/src/renderer/components/onboarding/MemoryStep.tsx (1)
76-88: LGTM on the default provider change.The default
embeddingProvider: 'openai'aligns with the backend.env.exampledocumentation, and the comment clearly explains the intent.apps/frontend/src/renderer/lib/mocks/settings-mock.ts (1)
39-47: LGTM!The mock implementation correctly returns a default configuration matching the
BackendEnvEmbeddingConfiginterface, with sensible defaults for browser mode where no backend.envexists.apps/frontend/src/shared/types/ipc.ts (2)
62-62: LGTM on import addition.The
BackendEnvEmbeddingConfigimport is correctly added to support the new ElectronAPI method.
580-582: LGTM on ElectronAPI extension.The new
getBackendEnvEmbeddingConfigmethod follows the established pattern and correctly returnsPromise<IPCResult<BackendEnvEmbeddingConfig>>.apps/frontend/src/preload/api/settings-api.ts (3)
8-10: LGTM on import additions.The imports are correctly updated to include
BackendEnvEmbeddingConfig.
32-35: LGTM on interface extension.The
getBackendEnvEmbeddingConfigmethod is correctly added to theSettingsAPIinterface with appropriate typing.
68-71: LGTM on implementation.The implementation correctly invokes the IPC channel and the return type matches the interface definition.
apps/frontend/src/main/ipc-handlers/settings-handlers.ts (3)
2-2: LGTM on import addition.The
readFileSyncimport is needed for the new.envfile reading functionality.
538-557: LGTM on provider mapping.The mapping function correctly handles all
BackendEmbeddingProvidervalues and provides a sensible default. The normalization withtoLowerCase().trim()handles common.envformatting variations.
559-608: LGTM on IPC handler implementation.The handler correctly:
- Uses
getEffectiveSourcePath()for proper path resolution- Returns sensible defaults when source path or
.envdoesn't exist- Parses the
.envfile and extractsGRAPHITI_EMBEDDER_PROVIDER- Has comprehensive error handling
Note:
readFileSyncis synchronous, but this is acceptable for.envfiles which are typically small. If this handler is called frequently or the file grows large, consider using asyncreadFileinstead.
Addresses all feedback from code review: 1. Add 'openrouter' to BackendEmbeddingProvider type - Updated settings.ts to include 'openrouter' in union type - Matches GraphitiEmbeddingProvider type 2. Simplify mapEmbeddingProvider function - Replaced switch statement with array-based validation - Cleaner, more maintainable code 3. Add error handling and logging - Added console.error in catch block for getBackendEnvEmbeddingConfig - Improves debugging when .env file read fails 4. Simplify boolean assignments - Changed ternary (? true : false) to !! operator in MemoryStep.tsx - More idiomatic TypeScript 5. Add 'openrouter' to EMBEDDING_PROVIDERS array - Added OpenRouter option to onboarding UI - Ensures detected provider is available in dropdown 6. Validate detected provider before applying - Added isValidProvider check before setting config - Prevents invalid providers from being applied Signed-off-by: Frederic Thomas <[email protected]>
Code Review Issues Addressed ✅All issues from code review have been resolved in commit dc6fad8: Issue #1: Provider Type MismatchFixed in: Added export type BackendEmbeddingProvider = 'openai' | 'voyage' | 'google' | 'azure_openai' | 'ollama' | 'openrouter';Issue #2: Complex Switch StatementFixed in: Simplified Before: switch (envValue.toLowerCase().trim()) {
case 'openai': return 'openai';
case 'voyage': return 'voyage';
// ... many more cases
}After: const mapEmbeddingProvider = (envValue: string | undefined): BackendEmbeddingProvider => {
if (!envValue) return 'openai';
const normalized = envValue.toLowerCase().trim() as BackendEmbeddingProvider;
const validProviders: BackendEmbeddingProvider[] = ['openai', 'voyage', 'google', 'azure_openai', 'ollama', 'openrouter'];
return validProviders.includes(normalized) ? normalized : 'openai';
};Issue #3: Missing Error LoggingFixed in: Added error logging in catch block: } catch (error) {
console.error('[Settings] Failed to get backend env embedding config:', error);
return { success: false, error: 'Failed to check backend embedding config' };
}Issue #4: Boolean Assignment SimplificationFixed in: Replaced ternary operator with Before: setKuzuAvailable(infraResult?.success && infraResult?.data?.memory?.kuzuInstalled ? true : false);After: setKuzuAvailable(!!(infraResult?.success && infraResult?.data?.memory?.kuzuInstalled));Issue #5: Missing 'openrouter' in UIFixed in: Added OpenRouter to { id: 'openrouter', name: 'OpenRouter', description: 'Multi-provider API', requiresApiKey: true },Issue #6: Missing Provider ValidationFixed in: Added validation before applying detected provider: const detectedProvider = envConfigResult.data.embeddingProvider;
// Validate that the detected provider is supported
const isValidProvider = EMBEDDING_PROVIDERS.some(p => p.id === detectedProvider);
if (detectedProvider && isValidProvider) {
setConfig(prev => ({
...prev,
embeddingProvider: detectedProvider
}));
}Note: Issue about using Commit: dc6fad8 |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/frontend/src/renderer/components/onboarding/MemoryStep.tsx (1)
122-124: Add error logging to catch block.The catch block silently swallows errors from the parallel initialization checks. While the fallback behavior (setting
kuzuAvailableto false) is acceptable, logging helps with debugging if the backend env config fetch or infrastructure check fails.🔎 Suggested fix
} catch (error) { + console.error('[MemoryStep] Failed to initialize memory config:', error); setKuzuAvailable(false); } finally {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
apps/frontend/src/main/ipc-handlers/settings-handlers.tsapps/frontend/src/renderer/components/onboarding/MemoryStep.tsxapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/shared/types/ipc.tsapps/frontend/src/shared/types/settings.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/shared/types/ipc.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/renderer/components/onboarding/MemoryStep.tsxapps/frontend/src/shared/types/settings.tsapps/frontend/src/main/ipc-handlers/settings-handlers.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/shared/types/ipc.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/renderer/components/onboarding/MemoryStep.tsxapps/frontend/src/shared/types/settings.tsapps/frontend/src/main/ipc-handlers/settings-handlers.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/shared/types/ipc.tsapps/frontend/src/shared/constants/ipc.tsapps/frontend/src/renderer/components/onboarding/MemoryStep.tsxapps/frontend/src/shared/types/settings.tsapps/frontend/src/main/ipc-handlers/settings-handlers.ts
🧠 Learnings (2)
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/backend/.env* : Enable Electron MCP for E2E testing by setting `ELECTRON_MCP_ENABLED=true` in `.env` and starting the Electron app with `npm run dev`
Applied to files:
apps/frontend/src/shared/types/ipc.tsapps/frontend/src/main/ipc-handlers/settings-handlers.ts
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/backend/integrations/graphiti/**/*.py : Use the Graphiti-provided multi-provider support for LLMs (OpenAI, Anthropic, Azure OpenAI, Ollama, Google AI) via `integrations/graphiti/graphiti_providers.py`
Applied to files:
apps/frontend/src/shared/types/settings.ts
🧬 Code graph analysis (2)
apps/frontend/src/shared/types/ipc.ts (1)
apps/frontend/src/shared/types/settings.ts (1)
BackendEnvEmbeddingConfig(324-331)
apps/frontend/src/main/ipc-handlers/settings-handlers.ts (3)
apps/frontend/src/main/ipc-handlers/context/utils.ts (1)
parseEnvFile(38-61)apps/frontend/src/shared/types/settings.ts (2)
BackendEmbeddingProvider(322-322)BackendEnvEmbeddingConfig(324-331)apps/frontend/src/main/updater/path-resolver.ts (1)
getEffectiveSourcePath(57-89)
🔇 Additional comments (6)
apps/frontend/src/shared/types/ipc.ts (1)
62-62: LGTM! Type definitions are consistent.The import and API signature for
getBackendEnvEmbeddingConfigare correctly typed and aligned with the newBackendEnvEmbeddingConfigtype defined in settings.ts.Also applies to: 591-592
apps/frontend/src/shared/types/settings.ts (1)
320-331: LGTM! Type definitions are complete.The
BackendEmbeddingProvidertype now includes all supported providers including 'openrouter', and theBackendEnvEmbeddingConfiginterface is well-documented and appropriately structured.apps/frontend/src/shared/constants/ipc.ts (1)
409-410: LGTM! IPC channel constant follows conventions.The new
BACKEND_ENV_GET_EMBEDDING_CONFIGchannel is properly defined and documented.apps/frontend/src/renderer/components/onboarding/MemoryStep.tsx (1)
96-121: LGTM! Parallel initialization with proper validation.The parallel execution of infrastructure check and backend env config fetch is efficient, and the provider validation against
EMBEDDING_PROVIDERSensures only supported providers are applied. The use of!!for boolean conversion is correct.apps/frontend/src/main/ipc-handlers/settings-handlers.ts (2)
538-548: LGTM! Clean provider mapping implementation.The simplified array-based validation with normalization is much more maintainable than the previous switch statement approach. Defaulting to 'openai' aligns with the backend
.env.exampledocumentation.
550-600: LGTM! Robust error handling with proper defaults.The handler correctly:
- Returns defaults when source path or .env is unavailable
- Logs errors with
console.errorfor debugging- Provides graceful fallback to 'openai' provider
- Includes
sourcePathandenvExistsmetadata for UI display
MikeeBuilds
added
feature
- Add openrouterApiKey field to MemoryConfig interface - Initialize OpenRouter API key from settings - Add validation for OpenRouter provider - Add UI rendering for OpenRouter API key input - Save OpenRouter API key to global settings - Fixes CodeRabbit review comment about incomplete OpenRouter provider Related to PR AndyMik90#571
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/frontend/src/renderer/components/onboarding/MemoryStep.tsx (2)
44-44: OpenRouter provider entry looks good.The OpenRouter provider is correctly added to
EMBEDDING_PROVIDERSwithrequiresApiKey: true. The hardcoded strings fornameanddescriptionare consistent with the existing providers in this array; converting all providers to i18n keys should be done in a separate refactor for consistency.
125-127: Add error logging in catch block.The catch block silently swallows errors from the
Promise.allcall. While the fallback behavior is acceptable, logging the error aids debugging when either API call fails.🔎 Proposed fix
- } catch { + } catch (error) { + console.error('[MemoryStep] Failed to initialize memory config:', error); setKuzuAvailable(false); } finally {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/frontend/src/renderer/components/onboarding/MemoryStep.tsx
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/renderer/components/onboarding/MemoryStep.tsx
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/renderer/components/onboarding/MemoryStep.tsx
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/onboarding/MemoryStep.tsx
🔇 Additional comments (5)
apps/frontend/src/renderer/components/onboarding/MemoryStep.tsx (5)
52-53: LGTM!The
openrouterApiKeyfield is correctly added to theMemoryConfiginterface, following the existing pattern for other provider API keys.
79-92: LGTM!The state initialization is well-structured:
- Default
embeddingProviderset to'openai'with a clear comment explaining it will be updated from.envon mount.openrouterApiKeycorrectly initialized fromsettings.globalOpenRouterApiKeyfollowing the same pattern as other API keys.
149-150: LGTM!The OpenRouter API key validation is correctly added following the same pattern as other providers.
178-180: LGTM!The OpenRouter API key is correctly persisted to settings with the same pattern used for other global API keys.
198-200: LGTM!The local store update correctly includes
globalOpenRouterApiKey, keeping it in sync with the saved settings.
Add IPC handler to read GRAPHITI_EMBEDDER_PROVIDER from backend .env file and use it as the default in the Memory Step of the setup wizard. Changes: - Add BACKEND_ENV_GET_EMBEDDING_CONFIG IPC channel - Add BackendEnvEmbeddingConfig type for IPC communication - Add settings-handlers.ts handler to read backend .env - Update MemoryStep.tsx to auto-detect provider on mount - Change default provider from 'ollama' to 'openai' per .env.example docs This ensures the Setup Wizard's memory configuration matches the backend's configured embedding provider. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Addresses all feedback from code review: 1. Add 'openrouter' to BackendEmbeddingProvider type - Updated settings.ts to include 'openrouter' in union type - Matches GraphitiEmbeddingProvider type 2. Simplify mapEmbeddingProvider function - Replaced switch statement with array-based validation - Cleaner, more maintainable code 3. Add error handling and logging - Added console.error in catch block for getBackendEnvEmbeddingConfig - Improves debugging when .env file read fails 4. Simplify boolean assignments - Changed ternary (? true : false) to !! operator in MemoryStep.tsx - More idiomatic TypeScript 5. Add 'openrouter' to EMBEDDING_PROVIDERS array - Added OpenRouter option to onboarding UI - Ensures detected provider is available in dropdown 6. Validate detected provider before applying - Added isValidProvider check before setting config - Prevents invalid providers from being applied Signed-off-by: Frederic Thomas <[email protected]>
- Add openrouterApiKey field to MemoryConfig interface - Initialize OpenRouter API key from settings - Add validation for OpenRouter provider - Add UI rendering for OpenRouter API key input - Save OpenRouter API key to global settings - Fixes CodeRabbit review comment about incomplete OpenRouter provider Related to PR AndyMik90#571
- Add OpenRouter to embedding provider dropdown in SecuritySettings.tsx - Add OpenRouter credential fields with password toggle - Add openrouterKeyIsGlobal type to ProjectEnvConfig - Update MemoryBackendSection.tsx for consistency (unused component) Fixes missing OpenRouter configuration in Settings page while it was already available in Setup Wizard.
Resolved conflicts: - settings-handlers.ts: Kept getEffectiveSourcePath import (still used on line 557) - MemoryStep.tsx: Kept globalOpenRouterApiKey additions for OpenRouter support
doublefx
force-pushed
the
feat/auto-detect-embedding-provider
branch
from
8e5bf58 to
1c390c3
Compare
doublefx
left a comment
doublefx
left a comment
There was a problem hiding this comment.
Response to i18n Suggestion
I agree with CodeRabbit's assessment that i18n should be used for user-facing text. However, as the review correctly points out:
"Since all other providers in this file also use hardcoded strings, this should be addressed consistently in a separate refactor."
Why I'm not fixing this in this PR:
- Consistency: All 6 embedding providers in
EMBEDDING_PROVIDERSarray use hardcoded strings (name,description) - Scope creep: Converting only OpenRouter to i18n while leaving the other 5 providers hardcoded would create inconsistency
- Better as separate work: A dedicated PR to add i18n support for all embedding providers would be more appropriate and easier to review
Proposed follow-up:
- Create a separate issue/PR to add i18n keys for all embedding provider strings
- This would include: provider names, descriptions, API key labels, and help text
- Estimated scope: ~6 providers × 3-4 strings each = comprehensive i18n refactor
For now, I'm following the established pattern in the file to maintain consistency. The OpenRouter implementation is functionally complete and matches the existing code style.
Does this reasoning make sense?
OpenRouter Provider Support - Implementation SummaryThis PR now includes complete OpenRouter support across both the Setup Wizard and Settings page, addressing the unresolved CodeRabbit conversation. What was added:1. Setup Wizard (MemoryStep.tsx) - Commit 1e0521f
2. Settings → Memory Page (SecuritySettings.tsx) - Commit cc4669d
3. Type Definitions (project.ts)
4. Consistency (MemoryBackendSection.tsx)
Why this matters:Before this PR:
After this PR:
Testing performed:
This implementation follows the exact same patterns used for OpenAI, Voyage, and other providers, ensuring consistency across the codebase. |
3 participants
Base Branch
developbranch (required for all feature/fix PRs)Description
Automatically detects the embedding provider configuration from the backend
.envfile and applies it to the frontend settings. This eliminates the need for users to manually configure embedding settings in the UI when they've already set them up in the backend.The feature reads
GRAPHITI_EMBEDDER_PROVIDERand related environment variables from the backend, then populates the frontend memory settings accordingly.Related Issue
Type of Change
Area
Checklist
developbranchCI/Testing Requirements
Feature Toggle
Breaking Changes
Breaking: No
Summary by CodeRabbit
New Features
Changes
Tests / Mocks
✏️ Tip: You can customize this high-level summary in your review settings.