feat(provider): 为模型的temperature/top_p/max_tokens 增加参数开关#8422
Open
kawayiYokami wants to merge 2 commits into
Open
feat(provider): 为模型的temperature/top_p/max_tokens 增加参数开关#8422kawayiYokami wants to merge 2 commits into
kawayiYokami wants to merge 2 commits into
Conversation
dosubot
Bot
added
size:L
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
isTemplateValueModifiedhelper always compares viaNumber(...), which may misbehave for non-numeric template types (e.g., booleans or string defaults); consider doing type-aware comparison based ontemplate.typeinstead of forcing numeric casting. - The magic key
'_disabled_keys'is now referenced in multiple places (Vue editor, provider base, and sources); consider centralizing it in a shared constant or helper to avoid typos and keep the semantics in sync across UI and backend.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `isTemplateValueModified` helper always compares via `Number(...)`, which may misbehave for non-numeric template types (e.g., booleans or string defaults); consider doing type-aware comparison based on `template.type` instead of forcing numeric casting.
- The magic key `'_disabled_keys'` is now referenced in multiple places (Vue editor, provider base, and sources); consider centralizing it in a shared constant or helper to avoid typos and keep the semantics in sync across UI and backend.
## Individual Comments
### Comment 1
<location path="dashboard/src/components/shared/ObjectEditor.vue" line_range="490-495" />
<code_context>
+ return localDisabledKeys.value.includes(templateKey)
+}
+
+function isTemplateValueModified(templateKey) {
+ const template = templateSchema.value[templateKey]
+ if (!template || template.default === undefined) return false
+ const pair = localKeyValuePairs.value.find(p => p.key === templateKey)
+ if (!pair) return false
+ return Number(pair.value) !== Number(template.default)
+}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Numeric comparison in isTemplateValueModified can misbehave for non-numeric defaults (NaN comparisons will always look "modified").
`isTemplateValueModified` always coerces both values with `Number(...)`. For non-numeric defaults (string/boolean), this often produces `NaN`, and `NaN !== NaN` is always true, so the field will always appear "modified" and the reset button will never disable. Even for numeric templates, `Number('')` or malformed input can behave unexpectedly.
Since `template.type` is available, consider branching on it instead:
- numeric types: numeric comparison with explicit `NaN` handling
- other types: compare as strings (or deep-compare for structured values)
This keeps the reset state consistent with actual user changes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
kawayiYokami
force-pushed
the
feat/disable-custom-params-injection
branch
from
9128797 to
56304a7
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to disable specific keys in the custom_extra_body configuration for LLM providers, updating the backend provider sources and the frontend ObjectEditor component to support disabling, enabling, and resetting template parameters. Feedback on the changes includes addressing a bug in isTemplateValueModified where non-numeric values are incorrectly compared using Number(), adding null-safety guards for props.modelValue in ObjectEditor.vue to prevent runtime crashes, removing redundant type checks in openai_source.py, and avoiding in-place modification of payloads in gemini_source.py by refactoring the logic into a shared helper function.
Comment on lines
+490
to
+496
| function isTemplateValueModified(templateKey) { | ||
| const template = templateSchema.value[templateKey] | ||
| if (!template || template.default === undefined) return false | ||
| const pair = localKeyValuePairs.value.find(p => p.key === templateKey) | ||
| if (!pair) return false | ||
| const type = template.type || 'string' | ||
| if (type === 'number' || type === 'float' || type === 'int') { |
Contributor
There was a problem hiding this comment.
当前 isTemplateValueModified 的实现对所有模板参数都使用了 Number(pair.value) !== Number(template.default)。然而，ObjectEditor 是一个通用组件，可以处理非数值类型（如字符串或 JSON）。对于非数值字符串，Number() 会返回 NaN。由于在 JavaScript 中 NaN !== NaN 总是为 true，这会导致该函数在字符串值未被修改时也错误地返回 true，从而使非数值参数的“还原默认值”按钮一直处于启用状态。我们应该根据模板的类型进行针对性的类型安全比较。
function isTemplateValueModified(templateKey) {
const template = templateSchema.value[templateKey]
if (!template || template.default === undefined) return false
const pair = localKeyValuePairs.value.find(p => p.key === templateKey)
if (!pair) return false
if (template.type === 'int' || template.type === 'float' || template.type === 'number') {
return Number(pair.value) !== Number(template.default)
}
if (template.type === 'bool' || template.type === 'boolean') {
return Boolean(pair.value) !== Boolean(template.default)
}
return pair.value !== template.default
}
Comment on lines
326
to
328
| const displayKeys = computed(() => { | ||
| return Object.keys(props.modelValue).slice(0, props.maxDisplayItems) | ||
| return Object.keys(props.modelValue).filter(k => k !== '_disabled_keys').slice(0, props.maxDisplayItems) | ||
| }) |
Contributor
There was a problem hiding this comment.
如果 props.modelValue 为 null 或 undefined，调用 Object.keys(props.modelValue) 将会抛出 TypeError 并导致组件崩溃。添加空值保护可以确保组件在模型值暂不可用时也能安全渲染。
const displayKeys = computed(() => {
if (!props.modelValue) return []
return Object.keys(props.modelValue).filter(k => k !== '_disabled_keys').slice(0, props.maxDisplayItems)
})
| } | ||
| originalDisabledKeys.value = [...localDisabledKeys.value] | ||
|
|
||
| for (const [key, value] of Object.entries(props.modelValue)) { |
Contributor
There was a problem hiding this comment.
如果 props.modelValue 为 null 或 undefined，调用 Object.entries(props.modelValue) 将会抛出 TypeError。建议在此处添加 props.modelValue || {} 的空值保护，以防止潜在的运行时崩溃。
for (const [key, value] of Object.entries(props.modelValue || {})) {
Comment on lines
588
to
590
| custom_extra_body = self.get_effective_custom_extra_body() | ||
| if isinstance(custom_extra_body, dict): | ||
| extra_body.update(custom_extra_body) |
Contributor
There was a problem hiding this comment.
由于 get_effective_custom_extra_body() 保证会返回一个 dict（空字典或已过滤的字典），因此这里的 isinstance(custom_extra_body, dict) 类型检查是多余的。我们可以直接使用返回的字典来更新 extra_body，使代码更加简洁。
# 读取并合并 custom_extra_body 配置
extra_body.update(self.get_effective_custom_extra_body())
Comment on lines
634
to
636
| custom_extra_body = self.get_effective_custom_extra_body() | ||
| if isinstance(custom_extra_body, dict): | ||
| extra_body.update(custom_extra_body) |
Contributor
There was a problem hiding this comment.
由于 get_effective_custom_extra_body() 保证会返回一个 dict，这里的 isinstance(custom_extra_body, dict) 类型检查是多余的。我们可以直接使用返回的字典来更新 extra_body，使代码更加简洁。
# 读取并合并 custom_extra_body 配置
extra_body.update(self.get_effective_custom_extra_body())
Comment on lines
+608
to
+612
| # Merge effective custom_extra_body into payloads | ||
| custom_extra_body = self.get_effective_custom_extra_body() | ||
| for k, v in custom_extra_body.items(): | ||
| if k not in payloads: | ||
| payloads[k] = v |
Contributor
There was a problem hiding this comment.
直接在原处修改传入的 payloads 字典可能会对调用者或重试循环产生意料之外的副作用。更安全的做法是在合并自定义额外请求体参数之前，先对 payloads 进行浅拷贝。此外，由于此合并逻辑在多处重复，建议将其重构为一个共享的辅助函数以避免代码重复。
Suggested change
| # Merge effective custom_extra_body into payloads | |
| custom_extra_body = self.get_effective_custom_extra_body() | |
| for k, v in custom_extra_body.items(): | |
| if k not in payloads: | |
| payloads[k] = v | |
| payloads = self._merge_custom_extra_body(payloads) |
References
- When implementing similar functionality for different cases (e.g., direct vs. quoted attachments), refactor the logic into a shared helper function to avoid code duplication.
Comment on lines
+703
to
+707
| # Merge effective custom_extra_body into payloads | ||
| custom_extra_body = self.get_effective_custom_extra_body() | ||
| for k, v in custom_extra_body.items(): | ||
| if k not in payloads: | ||
| payloads[k] = v |
Contributor
There was a problem hiding this comment.
直接在原处修改传入的 payloads 字典可能会对调用者或重试循环产生意料之外的副作用。更安全的做法是在合并自定义额外请求体参数之前，先对 payloads 进行浅拷贝。此外，由于此合并逻辑在多处重复，建议将其重构为一个共享的辅助函数以避免代码重复。
Suggested change
| # Merge effective custom_extra_body into payloads | |
| custom_extra_body = self.get_effective_custom_extra_body() | |
| for k, v in custom_extra_body.items(): | |
| if k not in payloads: | |
| payloads[k] = v | |
| payloads = self._merge_custom_extra_body(payloads) |
References
- When implementing similar functionality for different cases (e.g., direct vs. quoted attachments), refactor the logic into a shared helper function to avoid code duplication.
kawayiYokami
changed the title
kawayiYokami
changed the title
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
1 participant
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.
Motivation / 动机
当前
custom_extra_body中的 temperature/top_p/max_tokens 默认注入到所有供应商请求中，存在以下问题：结论：设置不如不设置，新供应商应默认不注入这些参数。
Modifications / 改动点
_disabled_keys机制）custom_extra_body支持（此前缺失）核心文件：
astrbot/core/provider/provider.py— 新增get_effective_custom_extra_body()方法astrbot/core/provider/sources/openai_source.py— 使用新方法astrbot/core/provider/sources/anthropic_source.py— 使用新方法 + max_tokens 强制注入astrbot/core/provider/sources/gemini_source.py— 新增 custom_extra_body 支持astrbot/core/config/default.py— 默认值和 metadata 调整dashboard/src/components/shared/ObjectEditor.vue— UI 改造dashboard/src/composables/useProviderModelConfigDialog.ts— Anthropic max_tokens 锁定This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
Summary by Sourcery
Introduce configurable disabling of custom parameter injection for model providers and update defaults to better align with provider-recommended behavior.
New Features:
Enhancements: