feat: add export/import for provider configurations#1450
feat: add export/import for provider configurations#1450yudanmao123 wants to merge 1 commit intomoeru-ai:mainfrom
Conversation
Summary of ChangesHello, 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 functionality to export and import provider configurations within the application. This enhancement allows developers to easily save and restore their configurations, which is especially useful during development cycles where data loss may occur. The changes involve adding two new functions, Highlights
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. Footnotes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6cf8085caa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const providerIds = store.configuredChatProviders.map(p => p.id) | ||
| providerIds.forEach(id => { | ||
| config[id] = store.getProviderConfig(id) || {} |
There was a problem hiding this comment.
Export every persisted provider, not just configured chat ones
This loop only serializes chat providers that are currently marked configured. In the same store we already track configured speech/transcription providers (configuredSpeechProvidersMetadata, configuredTranscriptionProvidersMetadata at packages/stage-ui/src/stores/providers.ts:2158-2168) and persisted-but-not-yet-configured entries via shouldListProvider() (packages/stage-ui/src/stores/providers.ts:2179-2184). As a result, backing up a setup with TTS/STT providers or a chat provider that is temporarily failing validation/manual testing will silently omit those credentials, so the subsequent import cannot restore the user's full provider setup.
Useful? React with 👍 / 👎.
| Object.entries(config).forEach(([providerId, providerConfig]) => { | ||
| store.setProviderConfig(providerId, providerConfig) |
There was a problem hiding this comment.
Preserve added-provider state during import
importProviderConfigs() restores only the credential object, but visibility is gated by addedProviders or a non-default config (shouldListProvider() in packages/stage-ui/src/stores/providers.ts:2179-2184). That is why useProviderValidation() explicitly calls markProviderAdded() for valid providers such as LM Studio whose working config can exactly match the defaults (packages/stage-ui/src/composables/use-provider-validation.ts:83-89). After exporting and re-importing one of those providers, the credentials come back but addedProviders stays empty, so the provider disappears from selectors/settings until the user opens the page and revalidates it manually.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request adds functionality to export and import provider configurations. There are a couple of issues in the implementation. The exportProviderConfigs function attempts to access a non-existent property on the store and only exports chat providers, which is inconsistent with the feature's goal. The importProviderConfigs function calls a method that doesn't exist, which will cause a runtime error. Suggestions have been provided to fix these bugs and improve the feature's correctness and completeness.
| Object.entries(config).forEach(([providerId, providerConfig]) => { | ||
| store.setProviderConfig(providerId, providerConfig) | ||
| }) |
There was a problem hiding this comment.
The method store.setProviderConfig does not exist on the providers store. This will cause a runtime error. You should directly modify the providers state. Since store.providers is a ref, you need to assign to its .value property. A safe and efficient way to do this is to merge the imported configurations with the existing ones.
| Object.entries(config).forEach(([providerId, providerConfig]) => { | |
| store.setProviderConfig(providerId, providerConfig) | |
| }) | |
| store.providers.value = { | |
| ...store.providers.value, | |
| ...config, | |
| }; |
| const providerIds = store.configuredChatProviders.map(p => p.id) | ||
| providerIds.forEach(id => { | ||
| config[id] = store.getProviderConfig(id) || {} | ||
| }) |
There was a problem hiding this comment.
The property store.configuredChatProviders does not exist. It seems you intended to use store.configuredChatProvidersMetadata. Additionally, this function only exports chat provider configurations, while the function name and PR description imply all provider configurations should be exported. To fix this and make it more robust, you could iterate over store.availableProviders which contains all configured providers regardless of their category.
| const providerIds = store.configuredChatProviders.map(p => p.id) | |
| providerIds.forEach(id => { | |
| config[id] = store.getProviderConfig(id) || {} | |
| }) | |
| for (const providerId of store.availableProviders) { | |
| const providerConfig = store.getProviderConfig(providerId); | |
| if (providerConfig) { | |
| config[providerId] = providerConfig; | |
| } | |
| } |
Adds exportProviderConfigs() and importProviderConfigs() functions to the providers store.
This allows users to export their provider configurations to a JSON file and import them later, useful during development when data gets purged.
Usage:
Closes #973