-
Notifications
You must be signed in to change notification settings - Fork 4
feat: 增加用户填写LLM信息的对话框和本地缓存支持 #26
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
Conversation
WalkthroughThe changes implement a dual LLM configuration mechanism, allowing users to provide LLM URL and API key through either environment variables or a modal input form, with environment variables taking precedence. Supporting logic, UI, and documentation were updated accordingly, including new utility exports, Vue component declarations, and dependency updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App (Vue)
participant LocalStorage
participant EnvVars
User->>App (Vue): Open app
App (Vue)->>EnvVars: Check for LLM config in env
alt Env config found
App (Vue)-->>User: App loads with env config
else Env config not found
App (Vue)->>LocalStorage: Check for LLM config in local storage
alt Local storage config found
App (Vue)-->>User: App loads with local config
else No config found
App (Vue)-->>User: Show modal dialog for LLM input
User->>App (Vue): Enter LLM URL & API key, click Save
App (Vue)->>LocalStorage: Save config
App (Vue)-->>User: Reload app with new config
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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
🧹 Nitpick comments (3)
docs/src/composable/utils.ts (1)
22-23: Good implementation of configuration detection logic.The boolean constants correctly implement the dual configuration approach. The logic properly checks for both required fields (URL and API key) in each configuration source.
Consider adding JSDoc comments to document the purpose and behavior of these constants:
+/** + * 检查是否通过环境变量定义了LLM配置 + * 需要同时设置 VITE_LLM_API_KEY 和 VITE_LLM_URL + */ export const isEnvLLMDefined = Boolean(import.meta.env.VITE_LLM_API_KEY && import.meta.env.VITE_LLM_URL) +/** + * 检查是否通过本地存储定义了LLM配置 + * 需要同时设置 llmUrl 和 llmApiKey + */ export const isLocalLLMDefined = Boolean($local.llmUrl && $local.llmApiKey)docs/src/App.vue (2)
13-37: Well-structured modal dialog implementation.The modal dialog properly prevents users from bypassing the configuration requirement. Good use of form validation and required field rules.
Consider enhancing the URL validation to ensure proper format:
-<tiny-form-item label="LLM URL" prop="llmUrl" :rules="{ required: true, messages: '必填', trigger: 'blur' }"> +<tiny-form-item label="LLM URL" prop="llmUrl" :rules="[ + { required: true, message: '必填', trigger: 'blur' }, + { type: 'url', message: '请输入有效的URL格式', trigger: 'blur' } +]">
55-62: Consider improving user experience in the submit handler.The current implementation works but could be enhanced for better UX:
- Add error handling for validation failures
- Provide user feedback during save operation
- Consider a more graceful alternative to
window.location.reload()const submit = () => { - formRef.value.validate().then(() => { + formRef.value.validate().then(() => { + // Add loading state here if needed $local.llmUrl = createData.llmUrl $local.llmApiKey = createData.llmApiKey boxVisibility.value = false - window.location.reload() + // Consider using router navigation or reactive updates instead of reload + window.location.reload() - }) + }).catch((error) => { + // Handle validation errors + console.error('表单验证失败:', error) + }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/public/llm.pngis excluded by!**/*.png
📒 Files selected for processing (7)
README.md(1 hunks)docs/.env(1 hunks)docs/components.d.ts(1 hunks)docs/package.json(1 hunks)docs/src/App.vue(2 hunks)docs/src/composable/AgentModelProvider.ts(3 hunks)docs/src/composable/utils.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
docs/src/composable/AgentModelProvider.ts (1)
docs/src/composable/utils.ts (2)
isEnvLLMDefined(22-22)$local(12-12)
🔇 Additional comments (10)
docs/.env (1)
1-1: LGTM! Environment variable correctly cleared for dual-configuration support.This change properly implements the dual-configuration mechanism by clearing the placeholder value, allowing the application to detect when environment variables are not set and prompt users for LLM configuration via the modal dialog.
docs/components.d.ts (1)
15-17: LGTM! New component declarations properly added for modal dialog functionality.The TinyDialogBox, TinyForm, and TinyFormItem components are correctly declared following the established pattern and are necessary for implementing the LLM configuration modal dialog.
docs/package.json (1)
19-19: Ensure compatibility with @opentiny/next-sdk 0.0.1-alpha.5Ran
npm view @opentiny/next-sdk@0.0.1-alpha.5:
- version: 0.0.1-alpha.5
- deprecated: null
- engines: null
All alpha versions (alpha.1 through alpha.5) are published with no deprecation warnings or engine requirements. Before merging, please:
- Review the release notes or changelog for any breaking changes.
- Verify your test suite covers the new UI components introduced in alpha.5.
- Confirm there are no new security advisories in your CI or
npm auditreports.File for review:
- docs/package.json (line 19)
README.md (1)
47-52: Excellent documentation update for the dual-configuration feature.The documentation clearly explains both configuration methods and establishes the priority system (environment variables > local cache). The inclusion of the screenshot reference enhances user understanding of the feature.
docs/src/composable/AgentModelProvider.ts (3)
8-8: LGTM! Necessary imports added for dual-configuration support.The new utility imports are required for implementing the conditional LLM configuration logic.
39-39: Good UX improvement with ellipsis for loading state.Adding ellipsis to the tool call status message provides better visual indication of an ongoing process.
60-61: All checks passed: dual-configuration logic and storage utilities verifiedThe
isEnvLLMDefinedflag and$localproxy instorage.tscorrectly detect, parse, and fall back to empty strings when values aren’t present. No further changes required—ready to merge.docs/src/composable/utils.ts (1)
10-12: LGTM: Clean export pattern refactoring.The import-then-export pattern maintains the same API while improving code organization.
docs/src/App.vue (2)
46-53: Good reactive state setup with proper fallbacks.The form data initialization correctly prioritizes local storage over environment variables, which aligns with the expected behavior.
64-66: Correct logic for showing the configuration dialog.The condition properly checks both environment variables and local storage to determine when user input is needed.
Summary by CodeRabbit
New Features
Documentation
Style
Chores