Conversation
📝 WalkthroughWalkthroughModel creation UI split into provider selection and provider-model fields. The form derives provider/providerModel from initial data, initializes and renders provider-specific credential inputs from per-provider schemas, re-normalizes drafts on provider change, and builds Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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
🧹 Nitpick comments (1)
ui/src/components/models/model-form.tsx (1)
291-299: Preserve provider-specific drafts when switching providers.
normalizeProviderConfigValues(nextProvider, current)drops any fields that are not in the newly selected schema. In practice, switching OpenAI → Bedrock → OpenAI clears the already entered API key/base values. A small per-provider draft cache would make the new selector much safer to use.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/models/model-form.tsx` around lines 291 - 299, handleProviderChange currently calls normalizeProviderConfigValues(nextProvider, current) which drops fields not in the new schema and causes switching back to lose previously entered secrets; modify handleProviderChange to keep a per-provider draft cache keyed by provider id (e.g., providerConfigDrafts state) so when switching away you save the current provider config (before normalize) into providerConfigDrafts[provider], and when switching to nextProvider you merge any existing providerConfigDrafts[nextProvider] into the normalized values before calling setProviderConfigValues. Update places that call setProvider and setProviderConfigValues in handleProviderChange to first save the current draft, then load/merge the draft for nextProvider with normalizeProviderConfigValues(nextProvider, draftOrCurrent) so provider-specific drafts are preserved across switches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/src/components/models/model-form.tsx`:
- Around line 275-281: The form currently constructs payload.model using
`${provider}/${value.model.trim()}` but doesn’t prevent a whitespace-only model
ID (e.g. " ") which becomes `${provider}/`; modify the submit/validation in
model-form.tsx (the onSubmit handler that builds the payload and the form field
validation for value.model) to check value.model.trim().length > 0 before
composing payload.model, and if it fails set a form error (or prevent
submission) with a clear message; keep using serializeProviderConfig(provider,
providerConfigValues) and only build the Model object after the trimmed model ID
is validated.
- Around line 118-130: The placeholders for the Bedrock fields are hardcoded;
change the secret_access_key and session_token entries to use placeholderKey
instead of placeholder (e.g., keep titleKey and descriptionKey as-is and add
placeholderKey: 'models.form.secretAccessKeyPlaceholder' and placeholderKey:
'models.form.sessionTokenPlaceholder') and then add those two keys to the i18n
locale files (including zh-CN) with appropriate translated strings so
placeholders are localized like the labels and hints.
- Around line 157-159: The code seeds new-model state with provider: 'openai'
(via splitModelIdentifier) causing an implicit OpenAI selection; change any
default provider values in this file that set provider: 'openai' to an empty
string (provider: '') and providerModel to '' so the provider select shows the
placeholder, and update the form submit/validation (the submit handler or
validate function used by the model form) to block submit unless provider is
non-empty; search for usages of splitModelIdentifier and any return blocks that
set provider: 'openai' (including the shown block and the other similar blocks)
and replace them with provider: '' and enforce explicit provider selection
before allowing save.
---
Nitpick comments:
In `@ui/src/components/models/model-form.tsx`:
- Around line 291-299: handleProviderChange currently calls
normalizeProviderConfigValues(nextProvider, current) which drops fields not in
the new schema and causes switching back to lose previously entered secrets;
modify handleProviderChange to keep a per-provider draft cache keyed by provider
id (e.g., providerConfigDrafts state) so when switching away you save the
current provider config (before normalize) into providerConfigDrafts[provider],
and when switching to nextProvider you merge any existing
providerConfigDrafts[nextProvider] into the normalized values before calling
setProviderConfigValues. Update places that call setProvider and
setProviderConfigValues in handleProviderChange to first save the current draft,
then load/merge the draft for nextProvider with
normalizeProviderConfigValues(nextProvider, draftOrCurrent) so provider-specific
drafts are preserved across switches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a72095ae-5e64-4f3e-9d24-97ce0d2ea3d0
📒 Files selected for processing (3)
ui/src/components/models/model-form.tsxui/src/i18n/locales/en.jsonui/src/i18n/locales/zh-CN.json
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/src/i18n/locales/zh-CN.json (1)
84-126:⚠️ Potential issue | 🟡 MinorMixed English copy in
zh-CNlocale reduces localization quality.On Line 105, Line 109, and Line 117 (and nearby placeholders/hints), multiple new labels are English-only in the Chinese locale file. This creates inconsistent UX in the same form.
🌐 Suggested localization patch
- "createDesc": "必填：name、provider、provider model、provider 专属配置。可选：timeout 和 rate limits。", + "createDesc": "必填：name、provider、provider model、provider 专属配置。可选：timeout 和 限流。", @@ - "providerLabel": "Provider *", - "providerPlaceholder": "选择 provider", - "providerRequired": "保存前请先选择 provider。", - "providerConfigSelectHint": "先选择 provider，再加载对应的配置表单。", - "modelLabel": "Provider Model *", + "providerLabel": "提供方 *", + "providerPlaceholder": "选择提供方", + "providerRequired": "保存前请先选择提供方。", + "providerConfigSelectHint": "先选择提供方，再加载对应的配置表单。", + "modelLabel": "提供方模型 *", "modelPlaceholder": "例如：gpt-4.1、deepseek-chat 或 anthropic.claude-3-5-sonnet-20240620-v1:0", - "modelRequired": "请输入 provider 侧模型标识。", + "modelRequired": "请输入提供方侧模型标识。", @@ - "regionLabel": "AWS Region", + "regionLabel": "AWS 区域", "regionHint": "用于选择默认的 Bedrock 运行时地址。", "accessKeyIdLabel": "AWS Access Key ID", "secretAccessKeyLabel": "AWS Secret Access Key", - "secretAccessKeyPlaceholder": "AWS secret access key", + "secretAccessKeyPlaceholder": "AWS 密钥", "sessionTokenLabel": "AWS Session Token", "sessionTokenHint": "使用 STS 或 AssumeRole 临时凭证时可选。", "sessionTokenPlaceholder": "可选的临时凭证", - "endpointLabel": "Bedrock Endpoint Override", + "endpointLabel": "Bedrock 端点覆盖", "endpointHint": "留空则根据所选 Region 使用标准运行时地址。",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/i18n/locales/zh-CN.json` around lines 84 - 126, The zh-CN locale has mixed English entries; update the English-only values to Chinese for consistency by translating the keys such as "modelPlaceholder", "modelLabel" placeholders (e.g., example model names can remain but the surrounding text should be Chinese), "apiBasePlaceholder" and "apiBaseHint", "secretAccessKeyPlaceholder", "sessionTokenPlaceholder" and any nearby hints/placeholders so all user-facing strings in the "form" object are fully Chinese; keep technical examples (URLs, model IDs, keys) as-is but replace English UI text with appropriate Chinese phrases matching the style of other entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@ui/src/i18n/locales/zh-CN.json`:
- Around line 84-126: The zh-CN locale has mixed English entries; update the
English-only values to Chinese for consistency by translating the keys such as
"modelPlaceholder", "modelLabel" placeholders (e.g., example model names can
remain but the surrounding text should be Chinese), "apiBasePlaceholder" and
"apiBaseHint", "secretAccessKeyPlaceholder", "sessionTokenPlaceholder" and any
nearby hints/placeholders so all user-facing strings in the "form" object are
fully Chinese; keep technical examples (URLs, model IDs, keys) as-is but replace
English UI text with appropriate Chinese phrases matching the style of other
entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 121c8e2a-842a-4b25-b01d-0c73f753bbad
📒 Files selected for processing (3)
ui/src/components/models/model-form.tsxui/src/i18n/locales/en.jsonui/src/i18n/locales/zh-CN.json
✅ Files skipped from review due to trivial changes (1)
- ui/src/components/models/model-form.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- ui/src/i18n/locales/en.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
UI Updates