-
-
Notifications
You must be signed in to change notification settings - Fork 95
feat/optional key support #16
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
base: main
Are you sure you want to change the base?
feat/optional key support #16
Conversation
|
@mihirpenugonda is attempting to deploy a commit to the Harsh's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis update introduces multi-provider support for AI model selection throughout the application. The backend completion API now dynamically selects the AI provider based on available API keys in the request. The frontend adapts to this by validating the presence of any key, auto-selecting models, and visually indicating loading states for chat threads. Several new hooks and stores facilitate these behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant APIKeyForm
participant APIKeyStore
participant ChatInput
participant useAutoSelectModel
participant CompletionAPI
participant AIProvider
User->>APIKeyForm: Enter API keys
APIKeyForm->>APIKeyStore: Store keys
ChatInput->>useAutoSelectModel: On mount/use
useAutoSelectModel->>APIKeyStore: Get available keys & current model
useAutoSelectModel->>ChatInput: Update selected model if needed
User->>ChatInput: Submit message
ChatInput->>CompletionAPI: POST /completion with available key in header
CompletionAPI->>AIProvider: Initialize client based on key/provider
AIProvider-->>CompletionAPI: Generate completion
CompletionAPI-->>ChatInput: Return completion
ChatInput-->>User: Display response
sequenceDiagram
participant User
participant ChatSidebar
participant TitleLoadingStore
participant useMessageSummary
participant CompletionAPI
User->>ChatSidebar: View threads
useMessageSummary->>TitleLoadingStore: setLoading(threadId, true)
useMessageSummary->>CompletionAPI: Request summary/title
CompletionAPI-->>useMessageSummary: Respond with title
useMessageSummary->>TitleLoadingStore: setLoading(threadId, false)
ChatSidebar->>TitleLoadingStore: isLoading(threadId)
TitleLoadingStore-->>ChatSidebar: Loading state (show spinner if true)
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
frontend/components/APIKeyForm.tsxOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/eslint-config-next@15.3.2_eslint@9.28.0_jiti@2.4.2__typescript@5.8.3/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting. app/api/completion/route.tsOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/eslint-config-next@15.3.2_eslint@9.28.0_jiti@2.4.2__typescript@5.8.3/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting. frontend/components/ChatInput.tsxOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/eslint-config-next@15.3.2_eslint@9.28.0_jiti@2.4.2__typescript@5.8.3/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting.
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 5
🧹 Nitpick comments (2)
frontend/components/ChatInput.tsx (1)
27-27: Remove unused import.The
useTitleLoadingStoreimport is not used anywhere in this component.-import { useTitleLoadingStore } from '../stores/TitleLoadingStore';frontend/hooks/useMessageSummary.ts (1)
45-45: Error message could be more specific.The generic error message doesn't indicate which operation failed (title generation vs. summary generation) or provide guidance on potential causes.
- toast.error("Failed to generate a summary for the message"); + toast.error(payload.isTitle ? "Failed to generate title" : "Failed to generate summary");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/api/completion/route.ts(2 hunks)frontend/components/APIKeyForm.tsx(1 hunks)frontend/components/ChatInput.tsx(2 hunks)frontend/components/ChatSidebar.tsx(2 hunks)frontend/hooks/useAutoSelectModel.ts(1 hunks)frontend/hooks/useMessageSummary.ts(3 hunks)frontend/stores/APIKeyStore.ts(2 hunks)frontend/stores/TitleLoadingStore.ts(1 hunks)lib/models.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
frontend/components/ChatInput.tsx (1)
frontend/hooks/useAutoSelectModel.ts (1)
useAutoSelectModel(6-28)
app/api/completion/route.ts (1)
app/api/chat/route.ts (1)
POST(11-88)
frontend/hooks/useAutoSelectModel.ts (3)
frontend/stores/APIKeyStore.ts (1)
useAPIKeyStore(36-74)frontend/stores/ModelStore.ts (1)
useModelStore(30-49)lib/models.ts (2)
getModelConfig(53-55)AI_MODELS(3-10)
lib/models.ts (1)
frontend/stores/APIKeyStore.ts (1)
Provider(5-5)
frontend/components/ChatSidebar.tsx (2)
frontend/dexie/queries.ts (1)
getThreads(6-8)frontend/stores/TitleLoadingStore.ts (1)
useTitleLoadingStore(9-27)
frontend/hooks/useMessageSummary.ts (4)
frontend/stores/APIKeyStore.ts (1)
useAPIKeyStore(36-74)frontend/stores/TitleLoadingStore.ts (1)
useTitleLoadingStore(9-27)lib/models.ts (1)
getProviderHeaderKey(57-65)frontend/dexie/queries.ts (1)
createMessageSummary(108-120)
🔇 Additional comments (21)
frontend/components/ChatInput.tsx (2)
26-26: LGTM: Auto-model selection integration.The
useAutoSelectModelhook is correctly imported and will enable automatic model selection based on available API keys, supporting the PR's optional key functionality.
64-64: LGTM: Auto-model selection hook usage.The hook is correctly invoked and will automatically switch to a model with an available API key when the current model lacks one.
frontend/components/APIKeyForm.tsx (1)
21-30: LGTM: Validation schema correctly implements optional key support.The schema appropriately makes individual API keys optional while ensuring at least one is provided through the refinement. This perfectly aligns with the PR's objective of supporting optional API keys.
frontend/stores/APIKeyStore.ts (3)
14-14: LGTM: Type definition supports multi-provider functionality.The new
getFirstAvailableKeymethod signature correctly returns both provider and key information, enabling dynamic provider selection.
52-54: LGTM: Validation logic supports any provider.The modified
hasRequiredKeysmethod correctly checks for any available API key instead of requiring only Google, supporting the optional key functionality.
61-67: LGTM: Well-implemented provider priority logic.The
getFirstAvailableKeyimplementation follows a reasonable priority order (Google → OpenAI → OpenRouter) and returns the necessary information for dynamic provider selection.lib/models.ts (3)
1-1: LGTM: Provider type import supports multi-provider functionality.The import correctly brings in the Provider type needed for the new
getProviderHeaderKeyfunction.
4-50: LGTM: Consistent quote style.The standardization to double quotes improves code consistency.
57-65:Details
✅ Verification successful
LGTM: Well-designed provider header mapping function.
The function correctly maps providers to their header keys with a sensible fallback mechanism. The implementation supports the dynamic provider selection functionality.
Verify that the fallback header key format matches expected conventions for all providers:
🏁 Script executed:
#!/bin/bash # Description: Check if any API documentation or existing code references specific header formats # Expected: Confirm the fallback format `X-{Provider}-API-Key` is universally appropriate # Search for any existing header key patterns in the codebase rg -i "x-.*-api-key" --type ts --type js # Search for any provider-specific header documentation or comments rg -i "(header|key).*openrouter|openai|google" --type ts --type js -A 3 -B 3Length of output: 10483
LGTM: Header key conventions validated
A project-wide search confirms that all existing providers use the
X-{Provider}-API-Keypattern, and every known provider has an explicitheaderKeyentry in lib/models.ts to preserve correct casing. The fallback (X-${provider.charAt(0).toUpperCase() + provider.slice(1)}-API-Key) only applies to any new providers and follows the same convention.frontend/components/ChatSidebar.tsx (3)
16-16: LGTM: Clean import additions for loading functionality.The imports for
Loader2icon anduseTitleLoadingStoreare correctly added to support the new loading state feature.Also applies to: 19-19
25-25: LGTM: Proper store usage pattern.Correctly extracts the
isLoadingfunction from the store using Zustand's selector pattern, which ensures optimal re-renders.
50-55: LGTM: Well-implemented loading state UI.The loading spinner implementation is clean and follows good UX practices:
- Uses flexbox for proper alignment
- Applies appropriate sizing (
h-3 w-3) and animation- Conditionally renders based on loading state
- Uses
shrink-0to prevent spinner from shrinkingapp/api/completion/route.ts (5)
1-5: LGTM: Appropriate imports for multi-provider support.The imports correctly include the necessary AI SDK providers and utilities for the enhanced functionality.
9-11: LGTM: Consistent header key reading pattern.The API key extraction from headers follows a consistent pattern across all three providers.
34-41: LGTM: Appropriate error handling for missing API keys.The error message clearly indicates the requirement and returns an appropriate HTTP status code.
45-45: LGTM: Dynamic model usage.Correctly uses the dynamically selected model instead of a hardcoded one.
57-59: LGTM: Improved error logging and messages.The error handling is more specific and includes proper logging.
frontend/stores/TitleLoadingStore.ts (2)
3-7: LGTM: Well-defined TypeScript interface.The type definition clearly specifies the store's structure and methods with appropriate parameter types.
9-27: LGTM: Excellent Zustand store implementation.The store implementation follows best practices:
- Immutable updates: Creates a new
Setinstance insetLoadingto ensure proper state updates- Efficient data structure: Uses
Setfor O(1) lookup and modification operations- Clean API: Simple, intuitive methods for managing loading states
- Proper state management: Uses Zustand's
setandgetfunctions correctlyThe implementation is thread-safe and efficient for managing multiple loading states simultaneously.
frontend/hooks/useMessageSummary.ts (2)
24-29: Header generation logic is sound.The dynamic header generation using
getProviderHeaderKeycorrectly adapts to different providers. The conditional header object ensures compatibility when no key is available.
66-66: Function signature change maintains interface compatibility.Returning the wrapper function instead of the original
completefunction maintains the expected interface while adding the loading state functionality.
| if (googleApiKey) { | ||
| const google = createGoogleGenerativeAI({ | ||
| apiKey: googleApiKey, | ||
| }); | ||
| model = google("gemini-2.5-flash-preview-04-17"); | ||
| } else if (openaiApiKey) { | ||
| const openai = createOpenAI({ | ||
| apiKey: openaiApiKey, | ||
| }); | ||
| model = openai("gpt-4.1-mini"); | ||
| } else if (openrouterApiKey) { | ||
| const openrouter = createOpenAI({ | ||
| baseURL: "https://openrouter.ai/api/v1", | ||
| apiKey: openrouterApiKey, | ||
| }); | ||
| model = openrouter("deepseek/deepseek-chat-v3-0324:free"); | ||
| } else { |
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.
🛠️ Refactor suggestion
Improve consistency and model selection logic.
The provider selection logic works but has some inconsistencies:
-
Hardcoded model IDs: Each provider uses a hardcoded model ID instead of leveraging the model configuration system used elsewhere in the codebase.
-
OpenRouter setup: Uses
createOpenAIwith a custom base URL, which is correct but could be more explicit.
Consider using the existing model configuration system for consistency:
- let model;
+ let model;
+ let provider: 'google' | 'openai' | 'openrouter';
if (googleApiKey) {
+ provider = 'google';
const google = createGoogleGenerativeAI({
apiKey: googleApiKey,
});
- model = google("gemini-2.5-flash-preview-04-17");
+ model = google("gemini-2.5-flash");
} else if (openaiApiKey) {
+ provider = 'openai';
const openai = createOpenAI({
apiKey: openaiApiKey,
});
- model = openai("gpt-4.1-mini");
+ model = openai("gpt-4o-mini");
} else if (openrouterApiKey) {
+ provider = 'openrouter';
const openrouter = createOpenAI({
baseURL: "https://openrouter.ai/api/v1",
apiKey: openrouterApiKey,
});
- model = openrouter("deepseek/deepseek-chat-v3-0324:free");
+ model = openrouter("deepseek/deepseek-r1:free");This approach would be more maintainable and consistent with the model configuration pattern used in app/api/chat/route.ts.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/api/completion/route.ts between lines 17 and 33, the current provider
selection uses hardcoded model IDs and directly calls createOpenAI or
createGoogleGenerativeAI, which is inconsistent with the model configuration
system used elsewhere. Refactor this section to use the existing model
configuration system for selecting models, ensuring that model IDs come from the
configuration rather than being hardcoded. Also, make the OpenRouter setup more
explicit by clearly defining its client creation and model selection consistent
with other providers, following the pattern in app/api/chat/route.ts.
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.
i think this cleanup should happen in a separate pr where we improve model registration. currently following whats already being done
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.
@mihirpenugonda That makes perfect sense! Keeping this PR focused on the optional key support and handling the model registration improvements separately is a good approach for maintaining clean, focused changes.
Would you like me to create a GitHub issue to track the model registration cleanup for the future PR? This could help ensure the refactoring doesn't get forgotten and provide a place to document the specific improvements needed.
🐰
┌─────────────────┐
│ ◕ ◕ │
│ ω │
│ Noted! 📝 │
└─────────────────┘
| const getFirstAvailableKey = useAPIKeyStore( | ||
| (state) => state.getFirstAvailableKey | ||
| ); | ||
| const setLoading = useTitleLoadingStore((state) => state.setLoading); | ||
|
|
||
| const availableKey = getFirstAvailableKey(); |
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.
Add validation for available API keys before making requests.
The hook retrieves the first available key but doesn't validate that one exists before proceeding with API calls. This could lead to failed requests with empty headers.
const availableKey = getFirstAvailableKey();
+
+ if (!availableKey) {
+ toast.error("No API key available. Please configure at least one API key.");
+ return {
+ complete: () => {},
+ isLoading: false,
+ };
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const getFirstAvailableKey = useAPIKeyStore( | |
| (state) => state.getFirstAvailableKey | |
| ); | |
| const setLoading = useTitleLoadingStore((state) => state.setLoading); | |
| const availableKey = getFirstAvailableKey(); | |
| const getFirstAvailableKey = useAPIKeyStore( | |
| (state) => state.getFirstAvailableKey | |
| ); | |
| const setLoading = useTitleLoadingStore((state) => state.setLoading); | |
| const availableKey = getFirstAvailableKey(); | |
| if (!availableKey) { | |
| toast.error("No API key available. Please configure at least one API key."); | |
| return { | |
| complete: () => {}, | |
| isLoading: false, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In frontend/hooks/useMessageSummary.ts around lines 16 to 21, the code retrieves
the first available API key but does not check if the key exists before using it
in API requests. Add a validation step after getting the availableKey to confirm
it is not null or undefined. If no key is available, prevent the API call from
proceeding or handle the error gracefully to avoid making requests with empty
headers.
|
This is a great change. |
Summary -
This PR makes API keys more flexible by allowing users to start with any single API key instead of requiring all three. Previously, users had to enter all API keys before using the app. Now they only need one from any provider (Google, OpenAI, or OpenRouter).
Summary by CodeRabbit
New Features
Improvements