feat: add invalid model detection and display in ModelSelectModal #1879
Conversation
WalkthroughAdds a modelMapping prop parsed from inputs.model_mapping in EditChannelModal and threads it into ModelSelectModal. ModelSelectModal now categorizes selected models into new, existing, and invalid based on models and modelMapping, adjusts default/active tabs, rendering, and selection summaries to include the invalid category. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant EditChannelModal
participant ModelSelectModal
participant Parser as JSON Parser
User->>EditChannelModal: Open "Edit Channel"
EditChannelModal->>Parser: Parse inputs.model_mapping
Parser-->>EditChannelModal: modelMapping or {}
EditChannelModal->>ModelSelectModal: Open with { models, selected, modelMapping }
rect rgb(245,245,255)
note over ModelSelectModal: Categorize selection
ModelSelectModal->>ModelSelectModal: Derive newModels, existingModels
ModelSelectModal->>ModelSelectModal: Derive invalidModels using modelMapping
end
alt newModels exist
ModelSelectModal-->>User: Activate "New" tab
else existingModels exist
ModelSelectModal-->>User: Activate "Existing" tab
else invalidModels exist
ModelSelectModal-->>User: Activate "Invalid" tab
else
ModelSelectModal-->>User: Empty state
end
User->>ModelSelectModal: Review/select models
ModelSelectModal-->>EditChannelModal: onConfirm(selected)
EditChannelModal-->>User: Close modal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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. 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: 2
🧹 Nitpick comments (5)
web/src/components/table/channels/modals/ModelSelectModal.jsx (4)
81-88: Don’t override user-selected tab on every selection change. Only switch when current tab has no items.The effect fires on
selectedchanges and can bounce the tab back to “new”, harming UX. Limit switching to when the active tab becomes empty. Also align invalid counts with the filtered set.Apply this diff:
- useEffect(() => { - if (visible) { - // 默认显示新获取模型tab，如果没有新模型则显示已有模型，如果都没有则显示无效模型 - if (newModels.length > 0) { - setActiveTab('new'); - } else if (existingModels.length > 0) { - setActiveTab('existing'); - } else if (invalidModels.length > 0) { - setActiveTab('invalid'); - } - } - }, [visible, newModels.length, existingModels.length, invalidModels.length, selected]); + useEffect(() => { + if (!visible) return; + // 仅当当前激活面板无内容时，才自动切换 + const counts = { + new: newModels.length, + existing: existingModels.length, + invalid: invalidFilteredModels.length, + }; + const currentLen = counts[activeTab] ?? 0; + if (currentLen === 0) { + if (counts.new > 0) setActiveTab('new'); + else if (counts.existing > 0) setActiveTab('existing'); + else if (counts.invalid > 0) setActiveTab('invalid'); + } + }, [visible, activeTab, newModels.length, existingModels.length, invalidFilteredModels.length]);Also applies to: 90-90
137-138: Categorize the filtered invalid set.Keeps the rendered list in sync with search results.
Apply this diff:
- const invalidModelsByCategory = categorizeModels(invalidModels); + const invalidModelsByCategory = categorizeModels(invalidFilteredModels);
157-164: Use filtered invalid count in tab label and presence.Prevents tab count/content mismatch under search.
Apply this diff:
- ...(invalidModels.length > 0 + ...(invalidFilteredModels.length > 0 ? [ { - tab: `${t('无效的模型')} (${invalidModels.length})`, + tab: `${t('无效的模型')} (${invalidFilteredModels.length})`, itemKey: 'invalid', }, ] : []),
324-326: Render gate should follow filtered invalid set.Avoid showing an empty panel container when the search filters all invalids out.
Apply this diff:
- {activeTab === 'invalid' && invalidModels.length > 0 && ( + {activeTab === 'invalid' && invalidFilteredModels.length > 0 && ( <div>{renderModelsByCategory(invalidModelsByCategory, 'invalid')}</div> )}web/src/components/table/channels/modals/EditChannelModal.jsx (1)
2381-2387: Memoize and validate parsed model mapping to reduce re-parsing and guard non-object values.Parsing JSON inline on every render creates a new object each time and may accept arrays/scalars. Prefer memoization and a plain-object check.
Apply this change (outside the selected lines) near other hooks:
const parsedModelMapping = React.useMemo(() => { try { const obj = JSON.parse(inputs.model_mapping || '{}'); return obj && typeof obj === 'object' && !Array.isArray(obj) ? obj : {}; } catch { return {}; } }, [inputs.model_mapping]);Then update the prop here:
- modelMapping={(() => { - try { - return JSON.parse(inputs.model_mapping || '{}'); - } catch { - return {}; - } - })()} + modelMapping={parsedModelMapping}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/components/table/channels/modals/EditChannelModal.jsx(1 hunks)web/src/components/table/channels/modals/ModelSelectModal.jsx(7 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: AAEE86
PR: QuantumNous/new-api#1658
File: web/src/components/table/channels/modals/EditChannelModal.jsx:555-569
Timestamp: 2025-08-27T02:15:25.448Z
Learning: In EditChannelModal.jsx, the applyModelMapping function transforms the models list by replacing original model names (mapping values) with display names (mapping keys). The database stores this transformed list containing mapped keys. On channel load, data.models contains these mapped display names, making the initialization filter if (data.models.includes(key)) correct.
Learnt from: AAEE86
PR: QuantumNous/new-api#1658
File: web/src/components/table/channels/modals/EditChannelModal.jsx:555-569
Timestamp: 2025-08-27T02:15:25.448Z
Learning: In EditChannelModal.jsx, the database stores mapped keys (display names) in the models field after applying model mapping transformations. When loading a channel, data.models contains the mapped keys, not the original model names. The filtering logic if (data.models.includes(key)) in the initialization is correct.
📚 Learning: 2025-08-27T02:15:25.448Z
Learnt from: AAEE86
PR: QuantumNous/new-api#1658
File: web/src/components/table/channels/modals/EditChannelModal.jsx:555-569
Timestamp: 2025-08-27T02:15:25.448Z
Learning: In EditChannelModal.jsx, the applyModelMapping function transforms the models list by replacing original model names (mapping values) with display names (mapping keys). The database stores this transformed list containing mapped keys. On channel load, data.models contains these mapped display names, making the initialization filter if (data.models.includes(key)) correct.
Applied to files:
web/src/components/table/channels/modals/EditChannelModal.jsxweb/src/components/table/channels/modals/ModelSelectModal.jsx
📚 Learning: 2025-08-27T02:15:25.448Z
Learnt from: AAEE86
PR: QuantumNous/new-api#1658
File: web/src/components/table/channels/modals/EditChannelModal.jsx:555-569
Timestamp: 2025-08-27T02:15:25.448Z
Learning: In EditChannelModal.jsx, the database stores mapped keys (display names) in the models field after applying model mapping transformations. When loading a channel, data.models contains the mapped keys, not the original model names. The filtering logic if (data.models.includes(key)) in the initialization is correct.
Applied to files:
web/src/components/table/channels/modals/EditChannelModal.jsxweb/src/components/table/channels/modals/ModelSelectModal.jsx
🔇 Additional comments (1)
web/src/components/table/channels/modals/ModelSelectModal.jsx (1)
44-44: New propmodelMappingplumbed correctly.Defaulting to
{}avoids undefined access and aligns with the modal’s new invalid-model logic.
| // 计算无效的模型：models中不包含，selected 中包含，且（如果它存在于modelMapping的键中）在modelMapping中作为键它映射的值也不被models包含 的模型 | ||
| const invalidModels = selected.filter(model => | ||
| !models.includes(model) && | ||
| (!modelMapping[model] || !models.includes(modelMapping[model])) | ||
| ); |
There was a problem hiding this comment.
Invalid models ignore search and bulk actions affect hidden items. Apply keyword filter to invalid set.
Currently, invalidModels is unfiltered by the search keyword. This causes:
- Tab count/contents mismatch when searching.
- Bottom “Select All” toggling models not visible in the invalid tab.
Introduce a filtered invalid set consistent with new/existing.
Apply this diff:
- const invalidModels = selected.filter(model =>
- !models.includes(model) &&
- (!modelMapping[model] || !models.includes(modelMapping[model]))
- );
+ const invalidModels = selected.filter(model =>
+ !models.includes(model) &&
+ (!modelMapping[model] || !models.includes(modelMapping[model]))
+ );
+ // Apply search filter to invalid models for consistent UX
+ const invalidFilteredModels = invalidModels.filter((m) =>
+ m.toLowerCase().includes(keyword.toLowerCase()),
+ );Committable suggestion skipped: line range outside the PR's diff.
| activeTab === 'new' ? newModels : activeTab === 'existing' ? existingModels : invalidModels; | ||
| const currentSelected = currentModels.filter((model) => |
There was a problem hiding this comment.
Bottom summary’s “Select All” should act on filtered invalid set.
Otherwise, it toggles hidden invalid models, which is surprising and risky.
Apply this diff:
- const currentModels =
- activeTab === 'new' ? newModels : activeTab === 'existing' ? existingModels : invalidModels;
+ const currentModels =
+ activeTab === 'new'
+ ? newModels
+ : activeTab === 'existing'
+ ? existingModels
+ : invalidFilteredModels;📝 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.
| activeTab === 'new' ? newModels : activeTab === 'existing' ? existingModels : invalidModels; | |
| const currentSelected = currentModels.filter((model) => | |
| const currentModels = | |
| activeTab === 'new' | |
| ? newModels | |
| : activeTab === 'existing' | |
| ? existingModels | |
| : invalidFilteredModels; | |
| const currentSelected = currentModels.filter((model) => |
🤖 Prompt for AI Agents
In web/src/components/table/channels/modals/ModelSelectModal.jsx around lines
340-341, the "Select All" logic currently acts on the full invalidModels array
(including hidden items) instead of the filtered/visible invalid set; update the
code so when activeTab === 'invalid' the array used for "Select All" is the
filtered invalid models (the same list used to render rows) — i.e., compute
currentModels to be newModels, existingModels, or the filtered invalidModels
(not the unfiltered invalidModels) and then derive currentSelected/toggle-all
from that currentModels array.
1 participant
Add invalid model tab on [Channel Edit - Get Model List] page, make it easier to remove invalid models from channel.
Summary by CodeRabbit
New Features
Bug Fixes