fix：Create duplicate model error

[lichunn]

English | 简体中文

PR

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our Commit Message Guidelines
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)
• Built its own designer, fully self-validated

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• Other... Please describe:

Background and solution

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

• Yes
• No

Other information

Summary by CodeRabbit

• New Features

  • Added model deletion capability with automatic panel cleanup.

• Bug Fixes & Improvements

  • Enhanced save validation and error handling for model configuration.
  • Prevented duplicate concurrent save operations.
  • Improved data handling for model options with better serialization support.
  • Added user notifications for successful and failed save operations.

[coderabbitai]

Walkthrough

The ModelSetting.vue component was refactored to introduce an isSaving flag and completely rewrote the saveModel method to include form validation, data sanitization, enum serialization, and error handling. A new deleteModel method was added, and the props.model watcher was enhanced with deep copying and immediate execution.

Changes

Cohort / File(s)	Summary
Model Management Logic packages/plugins/model-manager/src/components/ModelSetting.vue	Introduced isSaving flag to prevent concurrent saves. Rewrote saveModel with Promise-based validation, data sanitization, enum serialization, and error handling. Added new deleteModel method that emits callback and closes panel. Enhanced props.model watcher with deep copy, immediate execution, and enum deserialization. Both new methods are now exposed via component setup return.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A model now saves with care so true,
Deep clones and validation too!
Enums are cleaned, errors caught bright,
Delete flows smooth, and saves done right—
Settings now panel with grace anew! ✨

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'fix：Create duplicate model error' is partially related to the changeset. The commit messages reveal the actual issue is duplicate model creation when fields are enum values, but the title omits this critical detail.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into release/v2.10.x

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• 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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

🤖 Fix all issues with AI agents

In `@packages/plugins/model-manager/src/components/ModelSetting.vue`:
- Around line 143-145: modelBasicFormRef.value.validate() can throw if
modelBasicFormRef.value is null; update the validation call to guard against a
missing ref (either use optional chaining like
modelBasicFormRef.value?.validate() or check and early-return when
modelBasicFormRef.value is falsy) before entering the .then(...) chain so
validate() is only invoked when modelBasicFormRef.value exists; ensure related
logic that reads getLocalValue() stays consistent
(modelBasicFormRef.value?.getLocalValue()) and the form flow handles the
early-return case gracefully.
- Around line 188-204: Wrap the calls to createModel and updateModel inside a
try/catch around the block that sets emit('editCallback'), Notify(...),
closeModelSettingPanel(), selectedModel.value = null and isSaving.value = false;
specifically catch errors from createModel/updateModel (in the branch using
latestModelData.id and the else branch using updateModel(newModel.id,
newModel)), set isSaving.value = false in the catch, and call Notify({ type:
'error', message: '保存失败: ' + <error message> }) (or similar) so the user sees
the API error instead of silent failure; ensure existing emit/close/reset logic
only runs on success.
- Around line 160-176: The loop over newModel.parameters overwrites
isModelRefRelative and propertyName for each ModelRef so only the last
ModelRef's validation survives; change the logic in the parameters iteration
(where item.type === 'ModelRef') to preserve the first failure by, upon
encountering a ModelRef with no defaultValue, set isModelRefRelative = false and
propertyName = item.prop and then stop further processing for validation
purposes (either break the forEach by switching to a for-loop or track that a
failure was found and skip subsequent updates), ensuring subsequent ModelRef
fields do not overwrite the recorded first invalid result.
- Around line 220-242: The watcher assigns props directly in the else branch
causing parent mutation: when isSaving.value is true and newModel is set the
code does selectedModel.value = newModel which keeps a reference to the prop;
change this to either return early when isSaving.value is true (skip the watcher
update) or perform the same deep clone and enum deserialization used above
before assigning (use JSON.parse(JSON.stringify(newModel)) and run the
parameters Enum parsing) so selectedModel.value never holds a direct reference
to newModel; adjust the watcher logic around isSaving, selectedModel, and
newModel accordingly.

🧹 Nitpick comments (2)

packages/plugins/model-manager/src/components/ModelSetting.vue (2)

155-158: Guard against undefined latestModelData or missing parameters.

If modelBasicFormRef.value?.getLocalValue() returns undefined or an object without parameters, Line 157 (latestModelData.parameters.filter(...)) will throw. Add a defensive check.

Proposed guard

+            const params = latestModelData?.parameters ?? []
             const newModel = {
               description: latestModelData.description,
               ...
-              parameters: JSON.parse(JSON.stringify(latestModelData.parameters.filter((item) => !!item.prop)))
+              parameters: JSON.parse(JSON.stringify(params.filter((item) => !!item.prop)))
             }

---

140-209: Mixing async/await with .then()/.catch() chains reduces readability.

The function is declared async but uses .then().catch() for the validation promise. Consider using try/await/catch throughout for a consistent and more readable async pattern.

Sketch

 const saveModel = async () => {
   const latestModelData = modelBasicFormRef.value?.getLocalValue()
-  modelBasicFormRef.value
-    .validate()
-    .then(async (valid) => {
-      if (valid) {
-        // ... save logic
-      }
-    })
-    .catch(() => {
-      isSaving.value = false
-    })
+  try {
+    const valid = await modelBasicFormRef.value?.validate()
+    if (!valid) return
+    isSaving.value = true
+    // ... save logic
+  } catch {
+    isSaving.value = false
+  }
 }