feat: 二次确认添加重定向前模型 && 重定向后模式视为已有模型 - #2277
Conversation
WalkthroughExtract and normalize model redirect configurations from channel model mappings to enable models defined in redirects to be treated as available during channel setup. Updates EditChannelModal and ModelSelectModal to validate, deduplicate, and display mapped models, with UI indicators for redirected-only models. Changes
Sequence DiagramsequenceDiagram
participant User
participant EditChannelModal
participant ModelSelectModal
participant Validation
User->>EditChannelModal: Open channel editor
EditChannelModal->>EditChannelModal: Extract redirectModelList from model_mapping JSON
EditChannelModal->>EditChannelModal: Store initial models & mapping in refs
User->>ModelSelectModal: Open model selection
EditChannelModal->>ModelSelectModal: Pass redirectModels (normalized redirect values)
ModelSelectModal->>ModelSelectModal: Classify models (existing vs new vs redirect-only)
ModelSelectModal->>User: Render models with Tooltip for redirect-only items
User->>EditChannelModal: Modify models & submit
EditChannelModal->>Validation: Compare with initial state (hasModelConfigChanged)
alt Models in mapping but not in list
Validation->>EditChannelModal: Check for missing models
EditChannelModal->>User: Show confirmMissingModelMappings modal
User->>EditChannelModal: Choose action (cancel/submit/add missing)
opt User selects "add missing"
EditChannelModal->>EditChannelModal: Augment models list with missing redirects
end
end
EditChannelModal->>EditChannelModal: Normalize final models (trim, dedupe)
EditChannelModal->>Validation: Submit with normalized models & mapping
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40–50 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
web/src/components/table/channels/modals/ModelSelectModal.jsx (2)
41-46: Redirect model normalization & classification look correct; consider minor hook-deps cleanupThe logic around
redirectModels,normalizedRedirectModels,normalizedSelectedSet, andclassificationSetcorrectly treats both selected models and redirect targets as “existing”, which aligns with the need to surface redirect targets in the existing-model tab. UsingSet+useMemois a good choice for performance and deduping.One minor point:
normalizeModelNameis used inside theuseMemohooks but is not part of their dependency arrays. This is semantically safe here (no external closure state), but it may tripreact-hooks/exhaustive-deps. If that lint rule is enabled, consider wrappingnormalizeModelNameinuseCallbackand adding it to the dependency arrays, or adding an inline disable if you intentionally rely on the current behavior.Also applies to: 55-91, 94-94, 98-103
272-285: Redirect-only tooltip integration is reasonable; watch translation and accessibilityShowing an info icon + tooltip for models in
redirectOnlySetis a nice way to surface “redirect-only, not in models list” status, and the normalization withnormalizeModelNamekeeps it consistent with earlier sets.Just ensure the translation key for
t('来自模型重定向,尚未加入模型列表')is added, and consider whether theIconInfoCircleinsideTooltipneeds any ARIA attributes for screen readers beyond whatTooltipalready provides.web/src/components/table/channels/modals/EditChannelModal.jsx (3)
247-248: Change-detection for models/mapping works but is order- and formatting-sensitiveUsing
initialModelsRef/initialModelMappingRefplushasModelConfigChangedgives you a clear way to only prompt about missing mapped models when the model configuration actually changed, which is good.Note, though:
normalizedModelsare compared toinitialModelsby index, so reordering the same set of models will count as a change.model_mappingis compared as trimmed strings, so reformatting or reordering keys (without semantic change) will also be treated as “changed”.If you’d prefer purely semantic detection, you could compare model sets (e.g., sorted arrays or
Sets) and parsemodel_mappingto compare objects instead of strings; otherwise the current behavior is acceptable but a bit more eager to show the confirmation dialog.Also applies to: 624-627, 863-868, 1001-1015
943-999: Missing-model confirmation flow is good; ensure the confirm modal always resolves the PromiseThe
confirmMissingModelMappingshelper and its integration insubmitnicely implement the “second confirmation” flow and give users three clear choices (cancel/submit/add), with the'add'branch correctly mergingmissingModelsintolocalInputs.modelsand syncing viahandleInputChange.One edge case: the
Modal.confirmconfig doesn’t specify anonCancelhandler. If Semi UI’s confirm dialog can be closed via overlay click or ESC, the returned Promise may never resolve, leavingawait confirmMissingModelMappings(...)hanging. To be safe, consider wiringonCanceltomodal.destroy(); resolve('cancel');so that every close path settles the Promise.Please confirm from
@douyinfe/semi-uidocs whetherModal.confirmis non-closable by mask/ESC by default; if it is closable, addingonCancelas described would avoid a stuck submit flow.Also applies to: 1123-1146
1100-1115: Model mapping validation and model normalization are sensible; minor duplication onlyThe new block that:
- checks
model_mappingis a non-empty string,- validates it via
verifyJSON+JSON.parsewith user-friendly messages, and- then normalizes
localInputs.modelsby trimming and filtering empties,is sound and will prevent malformed mappings from slipping through while also cleaning up the model list.
There is a small duplication in that
verifyJSONalready parses the JSON and then you callJSON.parseagain; if you want to micro-optimize, you could skipverifyJSONhere and rely on a singletry/catch JSON.parsewith the same error message. Functionally, though, the current implementation is fine.Also applies to: 1117-1121
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/components/table/channels/modals/EditChannelModal.jsx(7 hunks)web/src/components/table/channels/modals/ModelSelectModal.jsx(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: AAEE86
Repo: QuantumNous/new-api PR: 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
Repo: QuantumNous/new-api PR: 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
Repo: QuantumNous/new-api PR: 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/ModelSelectModal.jsxweb/src/components/table/channels/modals/EditChannelModal.jsx
📚 Learning: 2025-08-27T02:15:25.448Z
Learnt from: AAEE86
Repo: QuantumNous/new-api PR: 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/ModelSelectModal.jsxweb/src/components/table/channels/modals/EditChannelModal.jsx
🧬 Code graph analysis (1)
web/src/components/table/channels/modals/EditChannelModal.jsx (1)
web/src/helpers/utils.jsx (5)
i(468-468)i(480-480)verifyJSON(265-272)verifyJSON(265-272)showInfo(161-163)
🔇 Additional comments (1)
web/src/components/table/channels/modals/EditChannelModal.jsx (1)
193-216: Redirect model extraction for ModelSelectModal is robust and aligned with intent
redirectModelListcleanly parsesinputs.model_mapping, validates it’s a non-array object, trims string values, dedupes withSet, and gracefully falls back to[]on invalid/empty JSON. Passing this asredirectModelsintoModelSelectModalis a solid way to mark redirect targets as “existing” when fetching upstream models.No functional issues stand out here.
Also applies to: 3067-3072
…fetch feat: 二次确认添加重定向前模型 && 重定向后模式视为已有模型
fix #2198 #2187
Summary by CodeRabbit
New Features
✏️ Tip: You can customize this high-level summary in your review settings.