-
-
Notifications
You must be signed in to change notification settings - Fork 95
Google API Key & API Call Optional #30
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?
Conversation
|
""" WalkthroughThe changes introduce a centralized constants module for model providers, replacing hardcoded provider strings throughout the codebase. Conditional logic is added so that certain actions, such as message completion, only occur when the selected provider is Google. The Google API key is now optional in the form validation and UI. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatInput
participant ModelStore
participant ThreadManager
participant MessageCompleter
User->>ChatInput: Submit message
ChatInput->>ModelStore: Get selected model
alt Provider is Google
ChatInput->>ThreadManager: Create thread (if needed)
ChatInput->>MessageCompleter: complete(message, threadId, ...)
else Provider is not Google
ChatInput-->>User: Skip completion/thread creation
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 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/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. lib/constants.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. lib/models.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. Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
🧪 Generate unit tests
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: 0
🔭 Outside diff range comments (1)
frontend/components/ChatInput.tsx (1)
110-120: Missing dependency in useCallback.The
handleSubmitcallback now depends onselectedModelthroughgetModelConfig(selectedModel).providerbutselectedModelis not included in the dependency array. This could lead to stale closure issues.Add
selectedModelto the dependency array:}, [ input, status, setInput, adjustHeight, append, id, textareaRef, threadId, complete, + selectedModel, ]);
🧹 Nitpick comments (2)
lib/models.ts (2)
24-26: Consider derivingheaderKeyfromproviderto cut duplicationNow that provider values are centralised, the
headerKeyvalues follow a predictable pattern (X-<Provider>-API-Key). Encoding them manually in every entry risks drift if either the header naming scheme or the provider list changes.
- Pros: single authoritative mapping or helper makes future additions cheaper and less error-prone.
- Possible approach:
export const MODEL_CONFIGS = { 'Deepseek R1 0528': { modelId: 'deepseek/deepseek-r1-0528:free', - provider: MODEL_PROVIDERS.OPEN_ROUTER, - headerKey: 'X-OpenRouter-API-Key', + provider: MODEL_PROVIDERS.OPEN_ROUTER, + headerKey: HEADER_KEYS[MODEL_PROVIDERS.OPEN_ROUTER], }, // … } as const;Where
HEADER_KEYSis defined once:export const HEADER_KEYS: Record<Provider, string> = { [MODEL_PROVIDERS.OPEN_ROUTER]: 'X-OpenRouter-API-Key', [MODEL_PROVIDERS.GOOGLE]: 'X-Google-API-Key', [MODEL_PROVIDERS.OPEN_AI]: 'X-OpenAI-API-Key', } as const;This keeps the model table focused on model specifics rather than transport details.
Also applies to: 29-31, 34-36, 39-41, 44-46, 49-51
1-3: Path-alias consistency nitElsewhere you use the
@/…alias ('@/frontend/stores/APIKeyStore').
For symmetry you might import constants the same way ('@/lib/constants') if the alias is configured forlib/*.-import { MODEL_PROVIDERS } from './constants'; +import { MODEL_PROVIDERS } from '@/lib/constants';Purely aesthetic; ignore if the alias does not cover
lib.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
frontend/components/APIKeyForm.tsx(1 hunks)frontend/components/ChatInput.tsx(3 hunks)frontend/components/MessageEditor.tsx(3 hunks)frontend/hooks/useMessageSummary.ts(2 hunks)lib/constants.ts(1 hunks)lib/models.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
frontend/hooks/useMessageSummary.ts (1)
lib/constants.ts (1)
MODEL_PROVIDERS(3-9)
frontend/components/ChatInput.tsx (1)
lib/constants.ts (1)
MODEL_PROVIDERS(3-9)
lib/constants.ts (1)
frontend/stores/APIKeyStore.ts (1)
Provider(5-5)
frontend/components/MessageEditor.tsx (1)
lib/constants.ts (1)
MODEL_PROVIDERS(3-9)
lib/models.ts (1)
lib/constants.ts (1)
MODEL_PROVIDERS(3-9)
🔇 Additional comments (11)
lib/constants.ts (1)
1-9: LGTM! Excellent centralization of provider constants.This implementation follows TypeScript best practices with proper typing, immutability via
as const, and clear naming conventions. The centralized approach will eliminate hardcoded provider strings across the codebase and improve maintainability.frontend/hooks/useMessageSummary.ts (2)
5-5: Good addition of centralized constants import.
19-21: Excellent replacement of hardcoded strings with constants.The replacement of hardcoded
'google'strings withMODEL_PROVIDERS.GOOGLEimproves maintainability and consistency across the codebase.frontend/components/APIKeyForm.tsx (1)
21-21: Appropriate change to make Google API key optional.This change correctly aligns with the PR objective of making the Google API key optional, allowing users to use other providers without requiring a Google API key.
frontend/components/ChatInput.tsx (2)
26-26: Good addition of centralized constants import.
92-102: Excellent conditional logic for Google-specific functionality.The conditional execution ensures that thread creation and completion logic only runs for Google providers, which aligns perfectly with the PR objective of avoiding Google API calls for other providers.
frontend/components/MessageEditor.tsx (4)
15-17: Good addition of necessary imports for provider-specific logic.
38-38: Appropriate use of selectedModel state.
42-44: Excellent use of centralized constants for conditional headers.The replacement of hardcoded strings with
MODEL_PROVIDERS.GOOGLEimproves consistency and maintainability.
92-99: Perfect implementation of conditional Google API logic.The conditional execution ensures that the completion API is only called for Google providers, which aligns with the PR objective of avoiding unnecessary Google API calls for other providers.
lib/models.ts (1)
2-2: Centralised provider constants ✔️Importing
MODEL_PROVIDERSremoves scattered string literals and tightens type-safety. Nice win for maintainability.
PS: I was not completely sure of the need of Title Generation as I was getting enough information from the normal open AI api's itself, Let me know if it was there for a reason due to which it was written that way
Summary by CodeRabbit
New Features
Bug Fixes
Refactor