-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: enhance configuration editor with template schema support and UI improvements #4267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…I improvements - Added support for template schemas in the configuration editor, allowing users to define and manage additional parameters like temperature, top_p, and max_tokens. - Improved UI components in ProviderModelsPanel and ObjectEditor for better user interaction, including new configuration buttons and enhanced input handling. - Updated localization files to include new configuration options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - 我发现了 2 个问题,并给出了一些整体性的反馈:
- 在普通键值对和模板字段之间存在不少重复的类型处理逻辑(string/number/boolean + slider 和转换分支);可以考虑把这些逻辑集中到共享的 helper 或一个小的子组件中,以降低复杂度并保持行为一致。
- 当前值的类型系统(string/number/float/int/bool/boolean/json)是通过分散在初始化、UI 渲染以及 confirmDialog 转换中的字符串字面量来表达的;你可以通过定义一个统一的映射或类 enum 结构来同时驱动展示和序列化,让这部分更健壮。
- 像
:style="pair.slider ? 'max-width: 120px;' : ''"和模板字段的透明度样式这样的内联样式,如果改成命名的 CSS 工具类而不是字符串形式的内联样式,会更易于维护和调整。
给 AI Agent 的提示词
Please address the comments from this code review:
## Overall Comments
- 在普通键值对和模板字段之间存在不少重复的类型处理逻辑(string/number/boolean + slider 和转换分支);可以考虑把这些逻辑集中到共享的 helper 或一个小的子组件中,以降低复杂度并保持行为一致。
- 当前值的类型系统(string/number/float/int/bool/boolean/json)是通过分散在初始化、UI 渲染以及 confirmDialog 转换中的字符串字面量来表达的;你可以通过定义一个统一的映射或类 enum 结构来同时驱动展示和序列化,让这部分更健壮。
- 像 `:style="pair.slider ? 'max-width: 120px;' : ''"` 和模板字段的透明度样式这样的内联样式,如果改成命名的 CSS 工具类而不是字符串形式的内联样式,会更易于维护和调整。
## Individual Comments
### Comment 1
<location> `dashboard/src/components/shared/ObjectEditor.vue:30-31` </location>
<code_context>
- <div v-for="(pair, index) in localKeyValuePairs" :key="index" class="key-value-pair">
+ <!-- Regular key-value pairs (non-template) -->
+ <div v-if="nonTemplatePairs.length > 0">
+ <div v-for="(pair, index) in nonTemplatePairs" :key="index" class="key-value-pair">
<v-row no-gutters align="center" class="mb-2">
<v-col cols="4">
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 请为 v-for 列表使用稳定的 key(例如 pair.key),而不是索引。
因为这个列表现在是从 `nonTemplatePairs` 派生出来的,并且删除操作是通过 key(`removeKeyValuePairByKey(pair.key)`)进行的,如果仍然用数组索引作为 `:key`,在条目重排或删除时就有可能出现 DOM 更新错位。把 `:key` 绑定到 `pair.key`(或其他稳定的唯一 ID)可以让 DOM 复用和底层数据保持正确对应。
```suggestion
<div v-if="nonTemplatePairs.length > 0">
<div v-for="(pair, index) in nonTemplatePairs" :key="pair.key" class="key-value-pair">
```
</issue_to_address>
### Comment 2
<location> `dashboard/src/components/shared/ObjectEditor.vue:130` </location>
<code_context>
+ hide-details
+ placeholder="字符串值"
+ ></v-text-field>
+ <div v-else-if="template.type === 'number' || template.type === 'float' || template.type === 'int'" class="d-flex align-center ga-4 flex-grow-1">
+ <v-slider
+ v-if="template.slider"
</code_context>
<issue_to_address>
**issue (typo):** `ga-4` 这个 class 名看起来像是个拼写错误,可能不会产生任何效果。
在其它地方你使用的是 `gap-2`(例如 `class="d-flex align-center gap-2 flex-grow-1"`),但这里是 `ga-4`,这和常见的工具类命名不符,很可能本意是 `gap-4`。请确认并进行调整,以保持间距的一致性。
```suggestion
<div v-else-if="template.type === 'number' || template.type === 'float' || template.type === 'int'" class="d-flex align-center gap-4 flex-grow-1">
```
</issue_to_address>帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- There is quite a bit of duplicated type-handling logic between regular key-value pairs and template fields (string/number/boolean + slider and conversion branches); consider centralizing this into shared helpers or a small subcomponent to reduce complexity and keep behavior consistent.
- The type system for values (string/number/float/int/bool/boolean/json) is currently encoded as scattered string literals across initialization, UI rendering, and confirmDialog conversion; you could make this more robust by defining a single mapping or enum-like structure to drive both display and serialization.
- Inline styles like
:style="pair.slider ? 'max-width: 120px;' : ''"and the template-field opacity styling would be easier to maintain and tweak if moved into named CSS utility classes instead of string-based inline styles.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There is quite a bit of duplicated type-handling logic between regular key-value pairs and template fields (string/number/boolean + slider and conversion branches); consider centralizing this into shared helpers or a small subcomponent to reduce complexity and keep behavior consistent.
- The type system for values (string/number/float/int/bool/boolean/json) is currently encoded as scattered string literals across initialization, UI rendering, and confirmDialog conversion; you could make this more robust by defining a single mapping or enum-like structure to drive both display and serialization.
- Inline styles like `:style="pair.slider ? 'max-width: 120px;' : ''"` and the template-field opacity styling would be easier to maintain and tweak if moved into named CSS utility classes instead of string-based inline styles.
## Individual Comments
### Comment 1
<location> `dashboard/src/components/shared/ObjectEditor.vue:30-31` </location>
<code_context>
- <div v-for="(pair, index) in localKeyValuePairs" :key="index" class="key-value-pair">
+ <!-- Regular key-value pairs (non-template) -->
+ <div v-if="nonTemplatePairs.length > 0">
+ <div v-for="(pair, index) in nonTemplatePairs" :key="index" class="key-value-pair">
<v-row no-gutters align="center" class="mb-2">
<v-col cols="4">
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Use a stable key (e.g., pair.key) instead of the index for the v-for list.
Because this list is now derived from `nonTemplatePairs` and deletions work by key (`removeKeyValuePairByKey(pair.key)`), using the array index as the `:key` risks mismatched DOM updates when items are reordered or removed. Binding `:key` to `pair.key` (or another stable unique ID) keeps DOM reuse correctly aligned with the underlying data.
```suggestion
<div v-if="nonTemplatePairs.length > 0">
<div v-for="(pair, index) in nonTemplatePairs" :key="pair.key" class="key-value-pair">
```
</issue_to_address>
### Comment 2
<location> `dashboard/src/components/shared/ObjectEditor.vue:130` </location>
<code_context>
+ hide-details
+ placeholder="字符串值"
+ ></v-text-field>
+ <div v-else-if="template.type === 'number' || template.type === 'float' || template.type === 'int'" class="d-flex align-center ga-4 flex-grow-1">
+ <v-slider
+ v-if="template.slider"
</code_context>
<issue_to_address>
**issue (typo):** The `ga-4` class name looks like a typo and may not have any effect.
In other places you use `gap-2` (e.g. `class="d-flex align-center gap-2 flex-grow-1"), but here it's `ga-4`, which doesn’t match standard utility classes and is likely meant to be `gap-4`. Please confirm and adjust for consistent spacing.
```suggestion
<div v-else-if="template.type === 'number' || template.type === 'float' || template.type === 'int'" class="d-flex align-center gap-4 flex-grow-1">
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| <div v-if="nonTemplatePairs.length > 0"> | ||
| <div v-for="(pair, index) in nonTemplatePairs" :key="index" class="key-value-pair"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (bug_risk): 请为 v-for 列表使用稳定的 key(例如 pair.key),而不是索引。
因为这个列表现在是从 nonTemplatePairs 派生出来的,并且删除操作是通过 key(removeKeyValuePairByKey(pair.key))进行的,如果仍然用数组索引作为 :key,在条目重排或删除时就有可能出现 DOM 更新错位。把 :key 绑定到 pair.key(或其他稳定的唯一 ID)可以让 DOM 复用和底层数据保持正确对应。
| <div v-if="nonTemplatePairs.length > 0"> | |
| <div v-for="(pair, index) in nonTemplatePairs" :key="index" class="key-value-pair"> | |
| <div v-if="nonTemplatePairs.length > 0"> | |
| <div v-for="(pair, index) in nonTemplatePairs" :key="pair.key" class="key-value-pair"> |
Original comment in English
suggestion (bug_risk): Use a stable key (e.g., pair.key) instead of the index for the v-for list.
Because this list is now derived from nonTemplatePairs and deletions work by key (removeKeyValuePairByKey(pair.key)), using the array index as the :key risks mismatched DOM updates when items are reordered or removed. Binding :key to pair.key (or another stable unique ID) keeps DOM reuse correctly aligned with the underlying data.
| <div v-if="nonTemplatePairs.length > 0"> | |
| <div v-for="(pair, index) in nonTemplatePairs" :key="index" class="key-value-pair"> | |
| <div v-if="nonTemplatePairs.length > 0"> | |
| <div v-for="(pair, index) in nonTemplatePairs" :key="pair.key" class="key-value-pair"> |
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
为 provider 的
extra_body配置添加基于模板 schema 的编辑功能,并改进用于已类型化参数的配置编辑器界面。New Features:
temperature、top_p和max_tokens。int、float、bool和json值进行类型感知转换。Enhancements:
Chores:
Original summary in English
Summary by Sourcery
Add template schema–driven editing for provider extra_body configuration and improve the configuration editor UI for typed parameters.
New Features:
Enhancements:
Chores:
新功能:
temperature、top_p和max_tokens等预定义字段,并提供默认值和类型处理。int、float、bool、json)。增强:
杂务:
Original summary in English
Summary by Sourcery
为 provider 的
extra_body配置添加基于模板 schema 的编辑功能,并改进用于已类型化参数的配置编辑器界面。New Features:
temperature、top_p和max_tokens。int、float、bool和json值进行类型感知转换。Enhancements:
Chores:
Original summary in English
Summary by Sourcery
Add template schema–driven editing for provider extra_body configuration and improve the configuration editor UI for typed parameters.
New Features:
Enhancements:
Chores: