Skip to content

Conversation

AAEE86
Copy link

@AAEE86 AAEE86 commented Aug 25, 2025

refactor(json-editor): 优化键值对添加逻辑及布局调整

feat(channels): 添加模型映射解析及反向映射显示功能

提示:设置重定向后,系统会自动将模型配置中对应的"值"替换为"键"

可以简化配置模型重定向的操作!

Clipchamp.mp4

Summary by CodeRabbit

  • New Features

    • Channels: configurable display-name ↔ original-name mappings for models with real-time updates and ability to restore original names.
  • Style

    • JSON editor: wider key input and narrower value input for improved readability.
    • Model-mapping editor: added an explanatory tip about how redirects affect model configuration.
  • Behavioral Changes

    • New JSON editor rows start with an empty key and are ignored until a valid key is entered; duplicate-key checks still ignore empty keys.

Copy link
Contributor

coderabbitai bot commented Aug 25, 2025

Walkthrough

JSONEditor: new rows now start with an empty key and key/value input column widths adjusted. EditChannelModal: added model-mapping parsing, helpers, and modelOriginalMapping state integrated into load and input handling; this mapping logic is duplicated in two places. Added an English i18n tip entry.

Changes

Cohort / File(s) Summary of changes
JSON editor row behavior and layout
web/src/components/common/ui/JSONEditor.jsx
New rows append { key: "", value: "" } instead of auto-generated field_n keys; empty keys are ignored when constructing the JSON object and in duplicate-key detection. Adjusted key/value column spans from 6/16 to 12/10. No public API changes.
Model mapping and helpers in EditChannelModal (duplicated)
web/src/components/table/channels/modals/EditChannelModal.jsx
Added modelOriginalMapping state and helper functions (parseModelMapping, getCurrentModels, updateModelsList, restoreModelsToOriginalNames, applyModelMapping, syncModelMappingToModels) to parse/apply JSON mappings, update/deduplicate models, and persist mapping state. Initialized from loaded channel data, synchronized on model_mapping input changes, cleared on unmount/when modal hides. The mapping logic and helpers are duplicated in two locations inside the file. No public API changes.
i18n English locale addition
web/src/i18n/locales/en.json
Added a new English translation entry for the UI tip about model redirection (new key/value) to enable showing the redirect hint in the UI.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant JSONEditor
  participant State

  User->>JSONEditor: Click "Add key/value"
  JSONEditor->>State: Append { key: "", value: "" }
  Note over JSONEditor,State: No auto-generated field_n keys

  User->>JSONEditor: Edit key/value
  JSONEditor->>JSONEditor: Build JSON output
  Note over JSONEditor: Rows with empty key are ignored in output and duplicate checks
Loading
sequenceDiagram
  autonumber
  participant Loader as loadChannel
  participant Modal as EditChannelModal
  participant Helpers as MappingHelpers
  participant UI as Models list/UI

  Loader->>Modal: channel data (models, model_mapping)
  Modal->>Helpers: parseModelMapping(data.model_mapping)
  alt valid mapping
    Helpers-->>Modal: mapping object
  else invalid mapping
    Helpers-->>Modal: null (reset)
  end
  Modal->>Helpers: applyModelMapping(mapping, data.models)
  Helpers-->>UI: updatedModels
  Modal->>Modal: set models, set modelOriginalMapping
  UI->>Modal: user edits model_mapping JSON
  Modal->>Helpers: syncModelMappingToModels(newMapping)
  Helpers-->>UI: merged/deduplicated models
  Note over Modal: Mapping helpers duplicated in two places within file
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

I hop through code with nimble paws,
Empty keys wait for careful claws.
Models don small masks to play,
Two echoes mirror what they say.
I twitch my nose — the changes stay. 🐇

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
web/src/components/common/ui/JSONEditor.jsx (2)

252-252: Good UX change: default new key is empty (prevents accidental keys).

Empty keys are intentionally ignored by keyValueArrayToObject, so this won’t pollute the serialized JSON. Behavior aligns with duplicate-key detection that also ignores empty keys.

Optional: auto-focus the newly added key field to speed up entry.

Additional patch (outside the selected range):

--- a/web/src/components/common/ui/JSONEditor.jsx
+++ b/web/src/components/common/ui/JSONEditor.jsx
@@
-                  <Input
+                  <Input
+                    autoFocus={index === keyValuePairs.length - 1 && !pair.key}
                     placeholder={t('键名')}
                     value={pair.key}
                     onChange={(newKey) => updateKey(pair.id, newKey)}
                     status={isDuplicate ? 'warning' : undefined}
                   />

404-404: Layout spans still sum to 24; verify consistency across editors.

Key/value spans changed to 12/10 (+ delete 2) which is valid. In the “region” editor below, spans are 10/12/2. Consider unifying for visual consistency, or extracting constants for spans to avoid drift.

Please quickly eyeball both editors on narrow viewports to ensure truncation doesn’t occur on long keys.

Also applies to: 431-431

web/src/components/table/channels/modals/EditChannelModal.jsx (1)

569-571: Localize the tooltip text and avoid hard-coded Chinese.

Use i18n so non-Chinese locales see a translated hint. Minimal change keeps the title attribute.

Apply:

-            <span title={originalModelName !== displayModelName ? `原始名称: ${originalModelName}` : undefined}>
+            <span title={
+              originalModelName !== displayModelName
+                ? t('原始名称: {{name}}', { name: originalModelName })
+                : undefined
+            }>
               {displayModelName}
             </span>

Optional (preferred): use a proper Tooltip for consistent UX (requires import). Outside selected range:

--- a/web/src/components/table/channels/modals/EditChannelModal.jsx
+++ b/web/src/components/table/channels/modals/EditChannelModal.jsx
@@
-import {
+import {
   SideSheet,
   Space,
   Spin,
   Button,
   Typography,
   Checkbox,
   Banner,
   Modal,
   ImagePreview,
   Card,
   Tag,
   Avatar,
   Form,
   Row,
   Col,
   Highlight,
+  Tooltip,
 } from '@douyinfe/semi-ui';

And replace the inline title span with a Tooltip-wrapped label:

-        label: (
-          <span className="flex items-center gap-1">
-            {icon}
-            <span title={originalModelName !== displayModelName ? t('原始名称: {{name}}', { name: originalModelName }) : undefined}>
-              {displayModelName}
-            </span>
-          </span>
-        ),
+        label: (
+          <Tooltip
+            content={
+              originalModelName !== displayModelName
+                ? t('原始名称: {{name}}', { name: originalModelName })
+                : null
+            }
+          >
+            <span className="flex items-center gap-1">
+              {icon}
+              <span>{displayModelName}</span>
+            </span>
+          </Tooltip>
+        ),
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a3c2b28 and 49685da.

📒 Files selected for processing (2)
  • web/src/components/common/ui/JSONEditor.jsx (3 hunks)
  • web/src/components/table/channels/modals/EditChannelModal.jsx (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
web/src/components/table/channels/modals/EditChannelModal.jsx (1)
web/src/components/table/channels/modals/EditTagModal.jsx (2)
  • inputs (73-73)
  • originModelOptions (62-62)
🔇 Additional comments (3)
web/src/components/table/channels/modals/EditChannelModal.jsx (3)

546-549: Correct reactivity: recompute mappings when model_mapping changes.

Parsing the mapping inside this effect and deriving a reverse map is appropriate. Depending on inputs.model_mapping ensures the labels update immediately after edits.


552-555: Preserve filtering accuracy by using the original model id for category match.

Using originalModelName for category.filter keeps icon/category inference stable even when the display name is mapped. Good separation of display vs. identity.

Also applies to: 558-558


577-579: Good: dependency array includes inputs.model_mapping.

This ensures option labels refresh when the mapping changes. Including t also updates labels when language switches.

@AAEE86 AAEE86 marked this pull request as draft August 26, 2025 00:54
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
web/src/components/table/channels/modals/EditChannelModal.jsx (1)

221-236: Harden JSON parsing and sanitize entries; also normalize to {} instead of null.

Return a sanitized plain object and trim k/v. This prevents arrays/strings sneaking through and stabilizes downstream logic. Adjust syncModelMappingToModels to treat an empty object as “no mapping”.

-  const parseModelMapping = (mappingValue) => {
-    if (!mappingValue || typeof mappingValue !== 'string' || mappingValue.trim() === '') {
-      return null;
-    }
-    
-    try {
-      const mapping = JSON.parse(mappingValue);
-      if (typeof mapping !== 'object' || mapping === null) {
-        return null;
-      }
-      return mapping;
-    } catch (error) {
-      console.warn('模型重定向 JSON 解析失败:', error);
-      return null;
-    }
-  };
+  const parseModelMapping = (mappingValue) => {
+    if (!mappingValue || typeof mappingValue !== 'string' || mappingValue.trim() === '') {
+      return {};
+    }
+    try {
+      const parsed = JSON.parse(mappingValue);
+      // 仅允许“非数组的对象”
+      if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return {};
+      // 仅保留非空 string→string,并做 trim 归一化
+      return Object.fromEntries(
+        Object.entries(parsed).filter(([k, v]) =>
+          typeof k === 'string' &&
+          typeof v === 'string' &&
+          k.trim() !== '' &&
+          v.trim() !== ''
+        ).map(([k, v]) => [k.trim(), v.trim()])
+      );
+    } catch (error) {
+      console.warn('模型重定向 JSON 解析失败:', error);
+      return {};
+    }
+  };
🧹 Nitpick comments (7)
web/src/components/table/channels/modals/EditChannelModal.jsx (7)

217-219: Clarify mapping direction with a brief inline doc (displayName → originalName).

A short comment avoids future confusion: modelOriginalMapping holds { displayName: originalName } used to restore names.

-  // 用于追踪模型的原始名称映射关系 { displayName: originalName }
+  // 用于追踪模型的原始名称映射关系,结构:{ displayName -> originalName }
+  // 注意:displayName 为“映射后的键名”,originalName 为“实际原始模型名/值”

246-255: Sanitize and dedupe model list robustly (string-cast, trim, preserve order).

Casts non-string entries, trims, and preserves insertion order while deduping.

-  const updateModelsList = (newModels, newMapping) => {
-    const uniqueModels = Array.from(new Set(newModels.filter(model => model && model.trim())));
+  const updateModelsList = (newModels, newMapping) => {
+    const seen = new Set();
+    const uniqueModels = [];
+    for (const m of newModels) {
+      const v = typeof m === 'string' ? m.trim() : String(m ?? '').trim();
+      if (!v || seen.has(v)) continue;
+      seen.add(v);
+      uniqueModels.push(v);
+    }

270-322: Make mapping application O(n) and warn on value-collisions.

Pre-index models by their “originalName” to avoid repeated scans, and detect when multiple keys map to the same value (last one silently wins otherwise).

-  const applyModelMapping = (mapping, currentModels, currentMapping) => {
-    let updatedModels = [...currentModels];
-    let newMapping = { ...currentMapping };
-    let hasChanges = false;
-
-    // 遍历重定向映射
-    Object.entries(mapping).forEach(([key, mappedValue]) => {
-      if (typeof key === 'string' && typeof mappedValue === 'string') {
-        const keyTrimmed = key.trim();
-        const valueTrimmed = mappedValue.trim();
-
-        if (keyTrimmed && valueTrimmed) {
-          // 查找模型配置中是否存在重定向的"值"(原始模型名)
-          const valueIndex = updatedModels.findIndex(model => {
-            return model === valueTrimmed || newMapping[model] === valueTrimmed;
-          });
-
-          if (valueIndex !== -1) {
-            const currentDisplayName = updatedModels[valueIndex];
-            if (currentDisplayName !== keyTrimmed) {
-              // 记录原始映射关系
-              if (!newMapping[keyTrimmed]) {
-                newMapping[keyTrimmed] = newMapping[currentDisplayName] || currentDisplayName;
-              }
-              // 清理旧的映射关系
-              if (newMapping[currentDisplayName]) {
-                delete newMapping[currentDisplayName];
-              }
-              // 更新显示名称为重定向的键
-              updatedModels[valueIndex] = keyTrimmed;
-              hasChanges = true;
-            }
-          }
-        }
-      }
-    });
-
-    // 处理不在映射中的模型,恢复为原始名称
-    const mappingKeys = new Set(Object.keys(mapping).map(key => key.trim()));
-    updatedModels = updatedModels.map(model => {
-      if (!mappingKeys.has(model) && newMapping[model]) {
-        const originalName = newMapping[model];
-        delete newMapping[model];
-        hasChanges = true;
-        return originalName;
-      }
-      return model;
-    });
-
-    return { updatedModels, newMapping, hasChanges };
-  };
+  const applyModelMapping = (mapping, currentModels, currentMapping) => {
+    let updatedModels = [...currentModels];
+    let newMapping = { ...currentMapping };
+    let hasChanges = false;
+
+    // 预索引:originalName -> index
+    const originalIndex = new Map();
+    for (let i = 0; i < updatedModels.length; i++) {
+      const name = updatedModels[i];
+      const orig = newMapping[name] || name; // 若已映射,取其原名
+      if (!originalIndex.has(orig)) originalIndex.set(orig, i);
+    }
+
+    // 检测 value 冲突(多个键映射到同一原名)
+    const seenValues = new Map(); // valueTrimmed -> keyTrimmed
+
+    for (const [key, mappedValue] of Object.entries(mapping)) {
+      if (typeof key !== 'string' || typeof mappedValue !== 'string') continue;
+      const keyTrimmed = key.trim();
+      const valueTrimmed = mappedValue.trim();
+      if (!keyTrimmed || !valueTrimmed) continue;
+
+      if (seenValues.has(valueTrimmed) && seenValues.get(valueTrimmed) !== keyTrimmed) {
+        console.warn('Duplicate model_mapping value detected; last one wins:', valueTrimmed, 'from', seenValues.get(valueTrimmed), 'to', keyTrimmed);
+      }
+      seenValues.set(valueTrimmed, keyTrimmed);
+
+      const idx = originalIndex.get(valueTrimmed);
+      if (idx !== undefined) {
+        const currentDisplay = updatedModels[idx];
+        if (currentDisplay !== keyTrimmed) {
+          if (!newMapping[keyTrimmed]) {
+            newMapping[keyTrimmed] = newMapping[currentDisplay] || currentDisplay;
+          }
+          if (newMapping[currentDisplay]) {
+            delete newMapping[currentDisplay];
+          }
+          updatedModels[idx] = keyTrimmed;
+          hasChanges = true;
+          // 更新索引:当前 display 改名了,但 original 仍对应 valueTrimmed
+        }
+      }
+    }
+
+    // 清理:不在 mapping 中的 display 名称恢复为原名
+    const mappingKeys = new Set(Object.keys(mapping).map((k) => k.trim()));
+    updatedModels = updatedModels.map((model) => {
+      if (!mappingKeys.has(model) && newMapping[model]) {
+        const originalName = newMapping[model];
+        delete newMapping[model];
+        hasChanges = true;
+        return originalName;
+      }
+      return model;
+    });
+
+    return { updatedModels, newMapping, hasChanges };
+  };

323-343: Treat empty-object mapping as reset (consistent with sanitized parser); consider debounce.

After normalizing parseModelMapping to return {}, treat an empty mapping as “restore to original names”. Also consider debouncing to reduce flicker while the user is typing JSON.

-  const syncModelMappingToModels = (mappingValue) => {
-    const mapping = parseModelMapping(mappingValue);
-    
-    if (!mapping) {
-      restoreModelsToOriginalNames();
-      return;
-    }
+  const syncModelMappingToModels = (mappingValue) => {
+    const mapping = parseModelMapping(mappingValue);
+    // 空对象代表“无映射/清空映射”
+    if (!mapping || Object.keys(mapping).length === 0) {
+      restoreModelsToOriginalNames();
+      return;
+    }

Additional (optional) outside-change snippet to debounce:

// imports: add useCallback to React import list
const mappingSyncTimer = useRef(null);
const debouncedSyncModelMappingToModels = useCallback((val) => {
  if (mappingSyncTimer.current) clearTimeout(mappingSyncTimer.current);
  mappingSyncTimer.current = setTimeout(() => syncModelMappingToModels(val), 200);
}, [syncModelMappingToModels]);

And call debouncedSyncModelMappingToModels from handleInputChange when name === 'model_mapping'.


364-370: Reduce re-renders: debounce real-time sync on model_mapping edits.

Typing JSON triggers parse/sync on each keystroke. Debouncing ~200ms smooths UX and avoids unnecessary work.

-    if (name === 'model_mapping') {
-      setInputs((inputs) => ({ ...inputs, [name]: value }));
-      syncModelMappingToModels(value);
-      return;
-    }
+    if (name === 'model_mapping') {
+      setInputs((inputs) => ({ ...inputs, [name]: value }));
+      // 若引入了 debounce,替换为 debouncedSyncModelMappingToModels
+      // debouncedSyncModelMappingToModels ? debouncedSyncModelMappingToModels(value) : syncModelMappingToModels(value);
+      syncModelMappingToModels(value);
+      return;
+    }

Note: include the optional debounced helper and add useCallback import if you adopt it.


514-528: Initialize original mapping only for present keys; guard with sanitized parser.

This looks correct. With the sanitized parser (returning {}), this block remains safe. Optionally, trim model names in the includes check to handle stray whitespace.

-        Object.entries(mapping).forEach(([key, value]) => {
-          if (data.models.includes(key)) {
+        Object.entries(mapping).forEach(([key, value]) => {
+          if (data.models.map(m => (m || '').trim()).includes(key.trim())) {
             initialMapping[key] = value;
           }
         });

1769-1777: Copy nit: clarify “键/值” roles to reduce confusion.

Minor wording tweak to align with the behavior: display uses “键名”,原始为“值”。 Consider removing the emoji if it clashes with product tone.

-                        <div className="text-blue-600 text-xs mt-1">
-                          {t('💡 提示:设置重定向后,系统自动将“模型配置”中对应的"值"替换为"键"')}
-                        </div>
+                        <div className="text-blue-600 text-xs mt-1">
+                          {t('提示:设置重定向后,系统会将“模型配置”中与映射“值”(原始模型名)匹配的项,显示为对应的“键”(重定向后的名称)')}
+                        </div>
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 49685da and 93d29b9.

📒 Files selected for processing (2)
  • web/src/components/common/ui/JSONEditor.jsx (3 hunks)
  • web/src/components/table/channels/modals/EditChannelModal.jsx (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • web/src/components/common/ui/JSONEditor.jsx
🔇 Additional comments (4)
web/src/components/table/channels/modals/EditChannelModal.jsx (4)

238-244: LGTM: Prefer form state with safe fallback.

getCurrentModels correctly prefers live form values and safely falls back to inputs.models.


256-269: LGTM: Restore flow is idempotent and guarded by an efficient change check.

The array-wise comparison avoids stringify and only updates when needed.


746-748: LGTM: Clearing mapping state when modal closes prevents bleed-through.


753-759: LGTM: Cleanup on unmount.

State reset avoids stale mapping when navigating away.

@AAEE86 AAEE86 marked this pull request as ready for review August 26, 2025 04:02
@AAEE86 AAEE86 changed the title feat(channels): 根据模型重定向键名自动修改对应模型配置中的名称 feat: 实现模型重定向中“键”自动映射到模型配置并优化JSON编辑器布局 Aug 26, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (2)
web/src/components/table/channels/modals/EditChannelModal.jsx (2)

217-236: Sanitize and validate model_mapping JSON; treat only plain object with non-empty string→string pairs as valid.

Currently arrays or non-plain objects pass the type check and can produce surprising no-ops. Also, keys/values aren’t trimmed or validated. Prefer returning a sanitized object; invalid/empty should degrade gracefully to {}.

Apply:

-  const parseModelMapping = (mappingValue) => {
-    if (!mappingValue || typeof mappingValue !== 'string' || mappingValue.trim() === '') {
-      return null;
-    }
-    
-    try {
-      const mapping = JSON.parse(mappingValue);
-      if (typeof mapping !== 'object' || mapping === null) {
-        return null;
-      }
-      return mapping;
-    } catch (error) {
-      console.warn('模型重定向 JSON 解析失败:', error);
-      return null;
-    }
-  };
+  const parseModelMapping = (mappingStr) => {
+    if (!mappingStr || mappingStr.trim() === '') return {};
+    try {
+      const parsed = JSON.parse(mappingStr);
+      if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) {
+        return {};
+      }
+      // Keep only trimmed, non-empty string→string entries
+      return Object.fromEntries(
+        Object.entries(parsed)
+          .filter(([k, v]) => typeof k === 'string' && typeof v === 'string')
+          .map(([k, v]) => [k.trim(), v.trim()])
+          .filter(([k, v]) => k !== '' && v !== '')
+      );
+    } catch (error) {
+      console.warn('模型重定向 JSON 解析失败:', error);
+      return {};
+    }
+  };

676-694: ReactNode labels break select search unless selectFilter extracts text. Keep label plain-text or add a searchable field.

optionsWithIcon sets label to JSX (icon + text). If selectFilter uses option.label.toString(), search will degrade to “[object Object]”.

Two safe options:

  • Keep label as the plain string, render icon via renderOptionItem; or
  • Add searchText: " " to each option and update selectFilter to include it.

Example (keep label string and add a render prop per option):

-    const optionsWithIcon = Array.from(modelMap.values()).map((opt) => {
-      const modelName = opt.value;
-      let icon = null;
-      ...
-      return {
-        ...opt,
-        label: (
-          <span className="flex items-center gap-1">
-            {icon}
-            {modelName}
-          </span>
-        ),
-      };
-    });
+    const optionsWithIcon = Array.from(modelMap.values()).map((opt) => {
+      const modelName = opt.value;
+      let icon = null;
+      ...
+      return {
+        ...opt,
+        label: modelName, // keep searchable text
+        render: () => (  // Semi supports custom item render via option.render
+          <span className="flex items-center gap-1">
+            {icon}
+            {modelName}
+          </span>
+        ),
+      };
+    });

To confirm selectFilter expectations and whether it reads option.render or a custom searchText, run:

#!/bin/bash
# Inspect selectFilter implementation and its usage of option props
rg -nP -C3 'export\s+const\s+selectFilter|function\s+selectFilter|\bselectFilter\b' web/src/helpers
🧹 Nitpick comments (5)
web/src/components/table/channels/modals/EditChannelModal.jsx (5)

256-269: LGTM with a tiny nit.

Comparing lengths in hasChanges is redundant since restore keeps the count unchanged. Not necessary to change, just noting.


270-321: Guard duplicate targets and normalize key/value trimming once; warn on collisions.

If multiple display keys map to the same original value, last-write wins silently. Emit a warning so surprises are detectable and normalize once to avoid repeated trims.

Apply:

   const applyModelMapping = (mapping, currentModels, currentMapping) => {
     let updatedModels = [...currentModels];
     let newMapping = { ...currentMapping };
     let hasChanges = false;
+    const valueOwners = Object.create(null); // valueTrimmed -> keyTrimmed

     // 遍历重定向映射
-    Object.entries(mapping).forEach(([key, mappedValue]) => {
-      if (typeof key === 'string' && typeof mappedValue === 'string') {
-        const keyTrimmed = key.trim();
-        const valueTrimmed = mappedValue.trim();
+    Object.entries(mapping).forEach(([key, mappedValue]) => {
+      if (typeof key === 'string' && typeof mappedValue === 'string') {
+        const keyTrimmed = key;
+        const valueTrimmed = mappedValue;

         if (keyTrimmed && valueTrimmed) {
+          // Detect duplicate targets (same original value → different display keys)
+          if (valueOwners[valueTrimmed] && valueOwners[valueTrimmed] !== keyTrimmed) {
+            console.warn(
+              'Duplicate model_mapping value detected; last one wins:',
+              valueTrimmed,
+              'from',
+              valueOwners[valueTrimmed],
+              'to',
+              keyTrimmed
+            );
+          }
+          valueOwners[valueTrimmed] = keyTrimmed;
+
           // 查找模型配置中是否存在重定向的"值"(原始模型名)
           const valueIndex = updatedModels.findIndex(model => {
             return model === valueTrimmed || newMapping[model] === valueTrimmed;
           });

           if (valueIndex !== -1) {
             const currentDisplayName = updatedModels[valueIndex];
             if (currentDisplayName !== keyTrimmed) {
               // 记录原始映射关系
               if (!newMapping[keyTrimmed]) {
                 newMapping[keyTrimmed] = newMapping[currentDisplayName] || currentDisplayName;
               }
               // 清理旧的映射关系
               if (newMapping[currentDisplayName]) {
                 delete newMapping[currentDisplayName];
               }
               // 更新显示名称为重定向的键
               updatedModels[valueIndex] = keyTrimmed;
               hasChanges = true;
             }
           }
         }
       }
     });

323-343: Treat empty mapping as a signal to restore originals.

After hardening parseModelMapping to return {} on invalid input, sync should consider “no entries” equivalent to “restore”.

Apply:

-  const syncModelMappingToModels = (mappingValue) => {
-    const mapping = parseModelMapping(mappingValue);
-    
-    if (!mapping) {
-      restoreModelsToOriginalNames();
-      return;
-    }
+  const syncModelMappingToModels = (mappingValue) => {
+    const mapping = parseModelMapping(mappingValue);
+    if (!mapping || Object.keys(mapping).length === 0) {
+      restoreModelsToOriginalNames();
+      return;
+    }

364-370: Also re-apply mapping when the models list changes.

Right now, mapping sync only runs when model_mapping changes. If the user adds/removes models, mapped display names won’t update until the JSON is touched.

Apply:

     // 处理模型重定向变更时自动同步模型配置(实时同步)
-    if (name === 'model_mapping') {
+    if (name === 'model_mapping') {
       setInputs((inputs) => ({ ...inputs, [name]: value }));
       syncModelMappingToModels(value);
       return;
     }
+    // 当模型列表本身发生变化时,同步一次映射,确保新增/删除立即体现在展示名称上
+    if (name === 'models') {
+      setInputs((inputs) => ({ ...inputs, [name]: value }));
+      const currentMappingStr = formApiRef.current?.getValue('model_mapping') ?? inputs.model_mapping ?? '';
+      syncModelMappingToModels(currentMappingStr);
+      return;
+    }

753-759: Avoid setState in unmount cleanup; it’s a no-op and can trigger warnings in some setups.

You don’t need to reset state on unmount. Remove this effect.

Apply:

-  // 组件卸载时清理资源
-  useEffect(() => {
-    return () => {
-      setModelOriginalMapping({});
-    };
-  }, []);
+  // 组件卸载无需显式清理 modelOriginalMapping
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f5882a2 and 0696d29.

📒 Files selected for processing (2)
  • web/src/components/common/ui/JSONEditor.jsx (3 hunks)
  • web/src/components/table/channels/modals/EditChannelModal.jsx (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • web/src/components/common/ui/JSONEditor.jsx
🔇 Additional comments (4)
web/src/components/table/channels/modals/EditChannelModal.jsx (4)

238-255: LGTM: unified models updater is clean and dedups without reordering.

The Set-based dedupe preserves insertion order. Updating both form and local state keeps UI in sync.


746-748: LGTM: clearing mapping state when the sheet closes avoids stale display names.

This matches the lifecycle reset behavior elsewhere.


1769-1777: LGTM: helpful, localized hint text clarifies “键/值” semantics for mapping.

Clear guidance that UI will auto-rewrite “值→键” in 模型配置 aligns with the feature intent.


514-528: Initial load doesn’t apply mapping to the models list; violates the “display mapped names first” goal.

On load, you only seed modelOriginalMapping if models already contain the “key” names. If models are the original names (common case), the UI won’t show mapped names until the user edits mapping. Apply the mapping once during load.

Apply:

-      // 初始化模型原始映射关系
-      const mapping = parseModelMapping(data.model_mapping);
-      if (mapping) {
-        const initialMapping = {};
-        // 根据当前的模型映射和模型列表,建立原始映射关系
-        Object.entries(mapping).forEach(([key, value]) => {
-          if (data.models.includes(key)) {
-            initialMapping[key] = value;
-          }
-        });
-        setModelOriginalMapping(initialMapping);
-      } else {
-        setModelOriginalMapping({});
-      }
+      // 初始化并应用模型映射到模型列表,让“模型配置”优先显示映射后的名称
+      const mapping = parseModelMapping(data.model_mapping);
+      if (mapping && Object.keys(mapping).length > 0) {
+        const { updatedModels, newMapping } = applyModelMapping(mapping, data.models || [], {});
+        updateModelsList(updatedModels, newMapping);
+      } else {
+        setModelOriginalMapping({});
+      }

Likely an incorrect or invalid review comment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
web/src/i18n/locales/en.json (1)

2001-2001: Use a stable, semantic i18n key for the redirect mapping tip

The translation itself is solid, but using the full string (with emoji and curly quotes) as the lookup key can be fragile and error-prone. I recommend introducing a dedicated alias key—e.g. channels.redirect.mappingTip—and updating the UI to reference it instead of the raw string.

Action items:

  • In web/src/i18n/locales/en.json, add the alias key:
    • Line 2001 currently contains the raw lookup.
    • Append:
     "已复制:{{name}}": "Copied: {{name}}",
     "💡 提示:设置重定向后,系统自动将“模型配置”中对应的“值”替换为“键”": "💡 Tip: After setting the redirect, the system automatically replaces the corresponding \"value\" in \"Model Configuration\" with \"key\"",
    +"channels.redirect.mappingTip": "💡 Tip: After setting the redirect, the system automatically replaces the corresponding \"value\" in \"Model Configuration\" with \"key\""
  • Replicate the channels.redirect.mappingTip entry (with appropriate translations) in each of your other locale JSON files under web/src/i18n/locales to avoid missing translations.
  • Update any UI components or lookup calls to use channels.redirect.mappingTip instead of the raw string key.

Verification:

  • The original string exists only in en.json at line 2001.
  • The alias channels.redirect.mappingTip is not yet defined in any locale file.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0696d29 and e81005b.

📒 Files selected for processing (1)
  • web/src/i18n/locales/en.json (1 hunks)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
web/src/components/table/channels/modals/EditChannelModal.jsx (2)

262-277: Harden JSON parsing in parseModelMapping function.

The current implementation doesn't validate that the parsed JSON is a plain object or sanitize the entries. This could lead to unexpected behavior with arrays or other non-object JSON values.


258-277: Duplicate code detected in model mapping utilities.

The model mapping parsing and related helper functions are defined twice in the component. This creates maintenance issues and violates the DRY principle. The entire block of utility functions (lines 258-383) appears to be a complete duplicate of functionality that should only be defined once.

Remove the duplicate definitions and keep only one set of these utility functions. Consider moving them to the top of the component or to a separate utilities file if they might be reused elsewhere.

🧹 Nitpick comments (1)
web/src/components/table/channels/modals/EditChannelModal.jsx (1)

311-362: Consider extracting complex model mapping logic.

The applyModelMapping function is quite complex with nested conditions and multiple responsibilities. Consider breaking it down into smaller, more focused functions for better maintainability.

Consider extracting the mapping application logic into smaller helper functions:

+const findModelIndex = (models, targetValue, mapping) => {
+  return models.findIndex(model => 
+    model === targetValue || mapping[model] === targetValue
+  );
+};
+
+const updateMappingForKey = (mapping, key, currentDisplayName, newKey) => {
+  const updatedMapping = { ...mapping };
+  if (!updatedMapping[newKey]) {
+    updatedMapping[newKey] = updatedMapping[currentDisplayName] || currentDisplayName;
+  }
+  if (updatedMapping[currentDisplayName]) {
+    delete updatedMapping[currentDisplayName];
+  }
+  return updatedMapping;
+};

 const applyModelMapping = (mapping, currentModels, currentMapping) => {
   let updatedModels = [...currentModels];
   let newMapping = { ...currentMapping };
   let hasChanges = false;
 
   Object.entries(mapping).forEach(([key, mappedValue]) => {
     if (typeof key === 'string' && typeof mappedValue === 'string') {
       const keyTrimmed = key.trim();
       const valueTrimmed = mappedValue.trim();
 
       if (keyTrimmed && valueTrimmed) {
-        const valueIndex = updatedModels.findIndex(model => {
-          return model === valueTrimmed || newMapping[model] === valueTrimmed;
-        });
+        const valueIndex = findModelIndex(updatedModels, valueTrimmed, newMapping);
 
         if (valueIndex !== -1) {
           const currentDisplayName = updatedModels[valueIndex];
           if (currentDisplayName !== keyTrimmed) {
-            // Record original mapping
-            if (!newMapping[keyTrimmed]) {
-              newMapping[keyTrimmed] = newMapping[currentDisplayName] || currentDisplayName;
-            }
-            // Clean up old mapping
-            if (newMapping[currentDisplayName]) {
-              delete newMapping[currentDisplayName];
-            }
+            newMapping = updateMappingForKey(newMapping, currentDisplayName, keyTrimmed);
             updatedModels[valueIndex] = keyTrimmed;
             hasChanges = true;
           }
         }
       }
     }
   });
   // ... rest of the function
 };
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e81005b and 8955f0d.

📒 Files selected for processing (2)
  • web/src/components/table/channels/modals/EditChannelModal.jsx (5 hunks)
  • web/src/i18n/locales/en.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • web/src/i18n/locales/en.json
🔇 Additional comments (5)
web/src/components/table/channels/modals/EditChannelModal.jsx (5)

298-309: Optimize array comparison in restoreModelsToOriginalNames.

Using a more explicit array comparison approach improves readability and performance compared to using JSON.stringify for arrays.

The optimization using length check and array.some() is a good improvement over JSON.stringify comparison.


405-410: Real-time model mapping synchronization implemented correctly.

The implementation properly handles model mapping changes and triggers synchronization with the models list. This provides a good user experience with immediate feedback.


837-837: Good cleanup of model mapping state.

Properly resetting modelOriginalMapping when the modal is closed prevents state leakage between editing sessions.


840-845: Component unmount cleanup is properly implemented.

The cleanup effect properly clears the model mapping state when the component unmounts, preventing memory leaks.


1887-1894: User-friendly tooltip for model redirection feature.

The addition of the informative tooltip helps users understand how the automatic model mapping synchronization works. The visual hint with the 💡 emoji makes it more noticeable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant