feat: show removed upstream models in fetch models modal - #4470
Merged
Calcium-Ion merged 1 commit intoApr 26, 2026
Merged
Conversation
Contributor
WalkthroughThe PR extends model selection functionality by parsing model mapping configurations to track source model names. EditChannelModal extracts mapping keys and passes them to ModelSelectModal via a new Changes
Sequence DiagramsequenceDiagram
participant EC as EditChannelModal
participant Mapping as model_mapping<br/>(JSON data)
participant MSM as ModelSelectModal
participant Tabs as Tab Rendering
EC->>Mapping: Parse JSON to extract keys
Mapping-->>EC: Source model names (trimmed,<br/>de-duplicated)
EC->>MSM: Pass redirectSourceModels prop
MSM->>MSM: Compute normalized source<br/>model set
MSM->>MSM: Identify removed models:<br/>selected but not in fetched list
MSM->>Tabs: Include "removed" tab<br/>if removed models exist
Tabs-->>MSM: Render active tab content<br/>(available/removed models)
MSM-->>EC: User confirms selection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
web/src/components/table/channels/modals/ModelSelectModal.jsx (2)
132-141: Consider memoizingremovedModelsfor consistency.
fetchedModelSetisuseMemo-ized butremovedModels(which is also derived from props) is recomputed every render and feeds both the tabList rendering and the default-tab effect's deps. Wrapping it inuseMemokeeps the pattern consistent with the surrounding sets and makes the effect dep semantics clearer.♻️ Proposed memoization
- const fetchedModelSet = useMemo( - () => new Set(normalizeModelList(models)), - [models], - ); - const removedModels = normalizeModelList(selected).filter( - (model) => - !fetchedModelSet.has(model) && - !normalizedRedirectSourceSet.has(model) && - model.toLowerCase().includes(keyword.toLowerCase()), - ); + const fetchedModelSet = useMemo( + () => new Set(normalizeModelList(models)), + [models], + ); + const removedModels = useMemo(() => { + const lowered = keyword.toLowerCase(); + return normalizeModelList(selected).filter( + (model) => + !fetchedModelSet.has(model) && + !normalizedRedirectSourceSet.has(model) && + model.toLowerCase().includes(lowered), + ); + }, [selected, fetchedModelSet, normalizedRedirectSourceSet, keyword]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/channels/modals/ModelSelectModal.jsx` around lines 132 - 141, Wrap the computed removedModels in useMemo to avoid recomputing each render: replace the direct computation of removedModels with a useMemo that returns normalizeModelList(selected).filter(...) and include its dependencies (selected, models-derived fetchedModelSet source if needed, normalizedRedirectSourceSet, and keyword) so the value only recalculates when inputs change; reference the existing symbols removedModels, fetchedModelSet, normalizeModelList, normalizedRedirectSourceSet, keyword, and ensure any effects or tabList rendering that depend on removedModels use the memoized value.
151-161: Tab auto-switch can override the user's tab selection while typing.The effect re-evaluates the active tab whenever
newModels.lengthorremovedModels.lengthchanges, which happens on every keystroke in the search box. If the user is onexisting/removedand types a keyword that happens to also match a "new" model, their tab gets force-switched tonew. This pattern was already present fornew/existing; the addedremovedbranch inherits the same behavior.If you want the auto-pick to fire only on open (not on every search), gate it on
visibletransitioning to true:♻️ Suggested guard
useEffect(() => { - if (visible) { - if (newModels.length > 0) { - setActiveTab('new'); - } else if (removedModels.length > 0) { - setActiveTab('removed'); - } else { - setActiveTab('existing'); - } - } - }, [visible, newModels.length, removedModels.length, selected]); + if (!visible) return; + if (newModels.length > 0) setActiveTab('new'); + else if (removedModels.length > 0) setActiveTab('removed'); + else setActiveTab('existing'); + // Only re-pick the default tab when the modal opens or the + // underlying selection changes — not on every keystroke. + }, [visible, selected]);web/src/components/table/channels/modals/EditChannelModal.jsx (1)
254-289: Combine the twomodel_mappingparsers to avoid parsing the same JSON twice.
redirectModelKeyListandredirectModelList(lines 254‑271) duplicate ~95% of the same logic and parseinputs.model_mappingindependently on every change. A singleuseMemoreturning both lists is cleaner and avoids the duplicate try/parse on every keystroke in the mapping editor.♻️ Proposed consolidation
- const redirectModelList = useMemo(() => { - const mapping = inputs.model_mapping; - if (typeof mapping !== 'string') return []; - const trimmed = mapping.trim(); - if (!trimmed) return []; - try { - const parsed = JSON.parse(trimmed); - if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { - return []; - } - const values = Object.values(parsed) - .map((value) => (typeof value === 'string' ? value.trim() : undefined)) - .filter((value) => value); - return Array.from(new Set(values)); - } catch (error) { - return []; - } - }, [inputs.model_mapping]); - const redirectModelKeyList = useMemo(() => { - const mapping = inputs.model_mapping; - if (typeof mapping !== 'string') return []; - const trimmed = mapping.trim(); - if (!trimmed) return []; - try { - const parsed = JSON.parse(trimmed); - if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { - return []; - } - const keys = Object.keys(parsed) - .map((key) => key.trim()) - .filter((key) => key); - return Array.from(new Set(keys)); - } catch (error) { - return []; - } - }, [inputs.model_mapping]); + const { redirectModelList, redirectModelKeyList } = useMemo(() => { + const empty = { redirectModelList: [], redirectModelKeyList: [] }; + const mapping = inputs.model_mapping; + if (typeof mapping !== 'string') return empty; + const trimmed = mapping.trim(); + if (!trimmed) return empty; + try { + const parsed = JSON.parse(trimmed); + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { + return empty; + } + const values = Object.values(parsed) + .map((v) => (typeof v === 'string' ? v.trim() : '')) + .filter(Boolean); + const keys = Object.keys(parsed) + .map((k) => k.trim()) + .filter(Boolean); + return { + redirectModelList: Array.from(new Set(values)), + redirectModelKeyList: Array.from(new Set(keys)), + }; + } catch { + return empty; + } + }, [inputs.model_mapping]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/channels/modals/EditChannelModal.jsx` around lines 254 - 289, The two useMemo hooks redirectModelList and redirectModelKeyList duplicate parsing of inputs.model_mapping; replace them with a single useMemo that reads and trims inputs.model_mapping once, JSON.parse inside one try/catch, validates parsed is a non-array object, then build unique key list (Object.keys -> trim -> filter -> Set -> Array) and unique value list (Object.values -> keep strings -> trim -> filter -> Set -> Array) and return both lists (e.g., { keys, values }) so the component can destructure to redirectModelKeyList and redirectModelList; keep the same dependency [inputs.model_mapping] and ensure on parse error you return empty arrays for both.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@web/src/components/table/channels/modals/EditChannelModal.jsx`:
- Around line 254-289: The two useMemo hooks redirectModelList and
redirectModelKeyList duplicate parsing of inputs.model_mapping; replace them
with a single useMemo that reads and trims inputs.model_mapping once, JSON.parse
inside one try/catch, validates parsed is a non-array object, then build unique
key list (Object.keys -> trim -> filter -> Set -> Array) and unique value list
(Object.values -> keep strings -> trim -> filter -> Set -> Array) and return
both lists (e.g., { keys, values }) so the component can destructure to
redirectModelKeyList and redirectModelList; keep the same dependency
[inputs.model_mapping] and ensure on parse error you return empty arrays for
both.
In `@web/src/components/table/channels/modals/ModelSelectModal.jsx`:
- Around line 132-141: Wrap the computed removedModels in useMemo to avoid
recomputing each render: replace the direct computation of removedModels with a
useMemo that returns normalizeModelList(selected).filter(...) and include its
dependencies (selected, models-derived fetchedModelSet source if needed,
normalizedRedirectSourceSet, and keyword) so the value only recalculates when
inputs change; reference the existing symbols removedModels, fetchedModelSet,
normalizeModelList, normalizedRedirectSourceSet, keyword, and ensure any effects
or tabList rendering that depend on removedModels use the memoized value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 96562d2c-8da1-4fd2-a40c-0d9446bf60c8
📒 Files selected for processing (2)
web/src/components/table/channels/modals/EditChannelModal.jsxweb/src/components/table/channels/modals/ModelSelectModal.jsx
Jinxuans
referenced
this pull request
in TokFlux-Org/TokFlux
May 9, 2026
…models feat: show removed upstream models in fetch models modal
11 tasks
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
2 participants
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.
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
获取上游模型列表新增 上游已经删除的模型 tab，方便主动去除模型
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit
New Features