fix: separate reasoning metadata from provider config - #9699
Merged
Conversation
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The frontend dialog unconditionally strips the
reasoningfield when editing/saving providers; if standalone providers also use this dialog, their preserved reasoning metadata will be lost—consider restricting this to source-backed providers or otherwise ensuring standalone provider metadata is not removed. - The
_strip_legacy_reasoning_metadatahelper currently only keys offprovider_source_id; if there are other forms of source-backed providers or future metadata fields that should be stripped, you may want a more robust criteria or central documentation of which fields are considered capability metadata vs. configuration.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The frontend dialog unconditionally strips the `reasoning` field when editing/saving providers; if standalone providers also use this dialog, their preserved reasoning metadata will be lost—consider restricting this to source-backed providers or otherwise ensuring standalone provider metadata is not removed.
- The `_strip_legacy_reasoning_metadata` helper currently only keys off `provider_source_id`; if there are other forms of source-backed providers or future metadata fields that should be stripped, you may want a more robust criteria or central documentation of which fields are considered capability metadata vs. configuration.
## Individual Comments
### Comment 1
<location path="dashboard/src/composables/useProviderModelConfigDialog.ts" line_range="89-93" />
<code_context>
function openProviderEdit(provider: any) {
const editableProvider = JSON.parse(JSON.stringify(provider))
+ delete editableProvider.reasoning
providerEditData.value = editableProvider
providerEditOriginalId.value = provider.id
</code_context>
<issue_to_address>
**suggestion:** Reasoning is deleted twice in the edit flow; consider centralizing this cleanup.
`openProviderEdit` deletes `editableProvider.reasoning`, and `saveEditedProvider` deletes `providerEditData.value.reasoning` again, even though `providerEditData.value` comes from `editableProvider`. The second deletion is therefore redundant. Centralize this cleanup in one place to keep the edit flow simpler and make it clear where transient fields are removed.
```suggestion
async function saveEditedProvider() {
if (!providerEditData.value) return
savingProviders.value.push(providerEditData.value.id)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
astrbot-docs | c009ca3 | Commit Preview URL Branch Preview URL |
Aug 15 2026, 08:32 AM |
BegoniaHe
pushed a commit
to Xero-Team/AstrBot
that referenced
this pull request
Aug 15, 2026
Upstream-Commit: fe22f75 Upstream-Author: Soulter <37870767+Soulter@users.noreply.github.com> Upstream-PR: AstrBotDevs#9699 Sync-Disposition: adapt Fork-Adaptation: Preserved the injected model metadata catalog, kept the Chinese and English locale set, and stripped legacy reasoning fields from source-backed provider configurations. Tested: uv run pytest tests/test_fastapi_v1_dashboard.py -q; cd dashboard && pnpm exec vitest run --config vitest.config.ts tests/providerPage.vitest.ts tests/providerSourcesPanel.vitest.ts
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
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.
Model capability metadata can include
reasoning: true. The dashboard was copying that descriptive flag into per-model provider configuration, which caused the generic configuration editor to expose it as an editable request setting. Capability metadata and request configuration need to remain separate.Modifications / 改动点
reasoninginto newly created model providers and remove legacy copies from source-backed provider records at dashboard API boundaries.reasoning_effortstring preset to custom request body parameters withhighas its default value.Screenshots or Test Results / 运行截图或测试结果
uv run ruff format .— 501 files unchangeduv run ruff check .— passeduv run pytest tests/test_fastapi_v1_dashboard.py -q— 89 passedcd dashboard && pnpm typecheck— passedChecklist / 检查清单
Summary by Sourcery
Separate reasoning capability metadata from provider configuration across dashboard and API flows.
New Features:
Bug Fixes:
Enhancements:
Tests: