feat(reasoning): support anthropic adaptive reasoning#1487
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds provider-aware reasoning visibility (UI, persistence, normalization), generalizes capability/route resolution for providers/models, and threads capability-aware Anthropic handling through presenters, runtime, provider mappings, shared types, SQLite schema, and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Renderer UI
participant Presenter
participant Runtime
participant Provider
participant DB as SQLite
User->>UI: select model / set reasoningVisibility
UI->>Presenter: updateGenerationSettings(reasoningVisibility)
Presenter->>DB: persist generation settings (reasoning_visibility)
UI->>Presenter: request run (includes capabilityProviderId hint)
Presenter->>Runtime: buildRuntimeContext(capabilityProviderId, supportsOfficialAnthropicReasoning)
Runtime->>Provider: buildProviderOptions (may include Anthropic adaptive fields)
Provider->>Provider: resolve route & endpointType (capability mapping)
Runtime->>Provider: send request (providerOptions, thinking config)
Provider->>Runtime: response / stream events
Runtime->>UI: emit stream events / traces
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/components/chat/ChatStatusBar.vue (1)
1956-1979:⚠️ Potential issue | 🟠 MajorUse the shared effective-reasoning check for Anthropic draft defaults.
The renderer gates defaults on
modelConfig.reasoning === true, while the backend usesgetReasoningEffectiveEnabledForProvider(...)with bothreasoningandreasoningEffort. This can make draft sessions dropreasoningEffort/reasoningVisibilityfor models that are effectively enabled through the default effort/portrait path.🐛 Proposed fix to align renderer defaults with backend sanitization
import { ANTHROPIC_REASONING_VISIBILITY_VALUES, DEFAULT_REASONING_EFFORT_OPTIONS as FALLBACK_REASONING_EFFORT_OPTIONS, + getReasoningEffectiveEnabledForProvider, hasAnthropicReasoningToggle, isReasoningEffort, normalizeAnthropicReasoningVisibilityValue, isVerbosity, type AnthropicReasoningVisibility, @@ const anthropicReasoningToggle = hasAnthropicReasoningToggle( resolvedCapabilityProviderId, portrait ) - const canDefaultReasoningEffort = !anthropicReasoningToggle || modelConfig.reasoning === true + const anthropicReasoningEnabled = anthropicReasoningToggle + ? getReasoningEffectiveEnabledForProvider(resolvedCapabilityProviderId, portrait, { + reasoning: modelConfig.reasoning, + reasoningEffort: modelConfig.reasoningEffort + }) + : true - if (supportsReasoningEffort(portrait) && canDefaultReasoningEffort) { + if ( + supportsReasoningEffort(portrait) && + (!anthropicReasoningToggle || anthropicReasoningEnabled) + ) { const effort = normalizeReasoningEffort( portrait, modelConfig.reasoningEffort ?? portrait?.effort @@ const reasoningVisibility = normalizeReasoningVisibility( resolvedCapabilityProviderId, portrait, modelConfig.reasoningVisibility ?? portrait?.visibility ) - if (modelConfig.reasoning === true && reasoningVisibility) { + if (anthropicReasoningToggle && anthropicReasoningEnabled && reasoningVisibility) { defaults.reasoningVisibility = reasoningVisibility }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/components/chat/ChatStatusBar.vue` around lines 1956 - 1979, The renderer currently gates setting defaults.reasoningEffort and defaults.reasoningVisibility on modelConfig.reasoning === true (via canDefaultReasoningEffort), which diverges from backend logic that uses getReasoningEffectiveEnabledForProvider(...) and considers the effective reasoning state including default effort/portrait; update the renderer to use the same effective check rather than only modelConfig.reasoning: replace the canDefaultReasoningEffort logic (which uses anthropicReasoningToggle and modelConfig.reasoning) with a call to the shared effective reasoning check (the same predicate/utility used by getReasoningEffectiveEnabledForProvider or a new exported helper), and then allow normalizeReasoningEffort(...) and normalizeReasoningVisibility(...) to set defaults when that effective-enabled check returns true (keeping supportsReasoningEffort, normalizeReasoningEffort, normalizeReasoningVisibility, and anthropicReasoningToggle references intact).
🧹 Nitpick comments (4)
src/main/presenter/configPresenter/modelConfig.ts (1)
431-476: IncludereasoningVisibilityin fallback/merge branches for consistency.
reasoningVisibilityis new but is not listed alongside the other reasoning-derived fields in either branch:
- In the default-fallback object (Lines 432–446),
reasoningEffortandverbosityare explicitly initialized toundefined;reasoningVisibilityis missing, sofinalConfig!.isUserDefined = falsereturns an object without that key at all.- In the stored-non-user merge (Lines 449–475), every other portrait-derived field (
reasoning,thinkingBudget,forceInterleavedThinkingCompat,reasoningEffort,verbosity) is explicitly re-pinned fromfinalConfigto ensure portrait changes win over stale stored values.reasoningVisibilityis only carried via the...finalConfigspread, making the intent inconsistent and easy to break in future edits.♻️ Suggested alignment
reasoningEffort: undefined, + reasoningVisibility: undefined, verbosity: undefined,reasoningEffort: finalConfig.reasoningEffort, + reasoningVisibility: finalConfig.reasoningVisibility, verbosity: finalConfig.verbosity🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/configPresenter/modelConfig.ts` around lines 431 - 476, The code omits reasoningVisibility from the default fallback and the explicit merge overrides, causing inconsistent propagation; add reasoningVisibility: undefined to the initial finalConfig fallback (near DEFAULT_MODEL_CAPABILITY_FALLBACKS initialization) and add reasoningVisibility: finalConfig.reasoningVisibility to the storedConfig/storedSource !== 'user' merge block (alongside reasoning, thinkingBudget, forceInterleavedThinkingCompat, reasoningEffort, verbosity) so portrait-derived visibility is explicitly preserved.src/main/presenter/llmProviderPresenter/providers/aiSdkProvider.ts (2)
228-321: Minor: route argument could includetype/supportedEndpointTypesfor future-proofing.Each
resolveProviderCapabilityProviderId(this.provider.id, { endpointType }, modelId)call passes onlyendpointType. That's sufficient today becauseresolveNewApiEndpointTypeFromRouteshort-circuits on a validendpointType— but if the helper's precedence ever changes (e.g., givingtype: ModelType.ImageGenerationhigher priority), these sites would silently drift. Since you already havethis.resolveNewApiEndpointType(modelId)above, consider passing a richer route shape (e.g., also include the stored model'ssupportedEndpointTypes/type) or hardcoding the capability provider per case ('anthropic','gemini','openai') given the switch already disambiguates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/llmProviderPresenter/providers/aiSdkProvider.ts` around lines 228 - 321, The calls to resolveProviderCapabilityProviderId(this.provider.id, { endpointType }, modelId) only pass endpointType, which may allow future precedence changes in resolveNewApiEndpointTypeFromRoute to pick the wrong capability; update each call in the switch inside the strategy === 'new-api' branch (where resolveNewApiEndpointType(modelId) is used and providerPatch is built for cases 'anthropic', 'gemini', 'openai-response', 'image-generation', and default) to pass a richer route object (e.g., include type: ModelType.ImageGeneration or supportedEndpointTypes from the model) or explicitly choose the capability provider id per case so the resolved capability cannot drift if helper precedence changes.
172-179: Consider validatinganthropicBaseUrlat provider construction for earlier failure detection.The
getConfiguredAnthropicBaseUrl()method throws duringresolveRouteDecision()on anthropic/* requests ifthis.definition.anthropicBaseUrlis missing. While zenmux's registry definition correctly setsanthropicBaseUrl: 'https://zenmux.ai/api/anthropic', validating at AiSdkProvider construction would catch any future misconfiguration earlier in the provider lifecycle rather than at first request time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/llmProviderPresenter/providers/aiSdkProvider.ts` around lines 172 - 179, The provider currently defers validating this.definition.anthropicBaseUrl until getConfiguredAnthropicBaseUrl() is called, causing runtime failures in resolveRouteDecision(); instead validate and normalize anthropicBaseUrl in the AiSdkProvider constructor: check this.definition.anthropicBaseUrl exists and .trim() is non-empty, throw a clear Error referencing this.provider.id if missing, and optionally store the trimmed value on a new private field (e.g., this.configuredAnthropicBaseUrl) to replace getConfiguredAnthropicBaseUrl() callers so misconfiguration is detected at construction time.src/renderer/src/components/settings/ModelConfigDialog.vue (1)
996-1011: Optional: redundant normalization branches.The
supportsReasoningVisibilityblock at lines 1004–1011 runs unconditionally and only overwritesconfig.value.reasoningVisibilitywhennormalizeAnthropicReasoningVisibilityValue(...)returns a truthy value — i.e. it only no-ops or idempotently rewrites a valid value. For user-defined configs with an invalid visibility, the stale value is kept here and then corrected by the watcher at line 1320 once reasoning is enabled. Functionally correct, but the two-pass normalization is a bit hard to follow; consider collapsing into a single normalize-then-default step insideloadConfig.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/components/settings/ModelConfigDialog.vue` around lines 996 - 1011, The code runs normalization twice: first in loadConfig (setting config.value.reasoningVisibility to normalizeAnthropicReasoningVisibilityValue(...) ?? capabilityReasoningVisibilityDefault.value) and then again unconditionally in a later supportsReasoningVisibility.value block that only overwrites when normalization returns truthy; collapse these into one clear step by removing the redundant second branch and ensure loadConfig performs normalizeAnthropicReasoningVisibilityValue(config.value.reasoningVisibility) once and falls back to capabilityReasoningVisibilityDefault.value when normalization returns falsy; update the watcher (the reasoning-enabled watcher) only if necessary so there’s a single canonical normalization path tied to loadConfig and the supportsReasoningVisibility flag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/presenter/llmProviderPresenter/aiSdk/runtime.ts`:
- Around line 154-158: The current runtime uses the provider-level fallback for
capabilityProviderId when calling buildProviderOptions and when checking
supportsTemperatureControlRuntime, which lets routed providers (e.g., "new-api"
Claude) bypass model-aware routing; update both call sites to resolve
capabilityProviderId using the presenter’s model-aware helper
getCapabilityProviderId(providerId, modelId) (pass context.provider.id and
context.provider.modelId) and fall back to context.provider.capabilityProviderId
or context.provider.id only afterwards, then pass that resolved id into
buildProviderOptions and into supportsTemperatureControlRuntime so routing
honors model-level mappings.
In `@src/renderer/src/i18n/da-DK/settings.json`:
- Around line 422-429: The option labels under reasoningVisibility still have
English prefixes; update the values for options.omitted and options.summarized
to use Danish prefixes (e.g., change "Omitted - Skjul ræsonneringsindhold" to a
Danish-prefixed string like "Udeladt - Skjul ræsonneringsindhold" and
"Summarized - Vis ræsonnering som resumé" to "Opsummeret - Vis ræsonnering som
resumé") so the user-facing text is fully localized while keeping the rest of
each phrase intact.
In `@src/shared/types/presenters/legacy.presenters.d.ts`:
- Line 168: The CONVERSATION_SETTINGS declaration in legacy.presenters.d.ts is
missing the new optional property reasoningVisibility, so update the legacy
conversation settings type(s) to include reasoningVisibility?:
ReasoningVisibility wherever CONVERSATION_SETTINGS (and the equivalent legacy
conversation settings type around the other noted location) is defined; ensure
both the type used by createConversation and updateConversationSettings accept
this optional field so legacy callers can pass the persisted session setting
through.
In `@test/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.ts`:
- Around line 2086-2158: The test's getReasoningPortrait mock doesn't cover the
capability-provider path so production code that queries the portrait via the
mapped provider falls back to the generic portrait; update the mock for
configPresenter.getReasoningPortrait to return the disabled Anthropics portrait
when the capability provider is used (i.e. treat providerId === 'anthropic' and
modelId === 'claude-opus-4-7' the same as the current new-api branch), so the
sequence involving configPresenter.getCapabilityProviderId,
agent.initSession('s1', { providerId: 'new-api', modelId: 'claude-opus-4-7' })
and agent.updateGenerationSettings('s1', ...) exercises the Anthropic capability
route and validates the reasoning fields are dropped.
In `@test/main/presenter/llmProviderPresenter/aiSdkRuntime.test.ts`:
- Around line 175-209: The test currently asserts absence of temperature on
tracePayloads[0]?.body but the runtime emits temperature at the top-level of the
trace payload; update the assertion to check tracePayloads[0] itself (e.g.,
expect(tracePayloads[0]).not.toHaveProperty('temperature')) so the test fails if
temperature is emitted at the top level; modify the assertion in
aiSdkRuntime.test (near the runAiSdkCoreStream invocation and the
emitRequestTrace mock) to validate the top-level payload rather than
payload.body.
In `@test/main/presenter/sqlitePresenter.test.ts`:
- Line 509: The long SQL select literal 'SELECT system_prompt, summary_text,
summary_cursor_order_seq, force_interleaved_thinking_compat,
reasoning_visibility FROM deepchat_sessions WHERE id = ?' exceeds 100 chars;
split the string into multiple single-quoted parts so no line is longer than 100
characters (e.g. break after column list and before the FROM/WHERE clause) and
concatenate them (keep single quotes and no semicolons) where this literal
appears in the test file so the final SQL is unchanged but wrapped to satisfy
the Oxfmt width rule.
In `@test/renderer/components/ChatStatusBar.test.ts`:
- Around line 361-363: The mock for getCapabilityProviderId must match the real
synchronous signature (providerId: string, modelId: string); update the mock in
ChatStatusBar.test to remove async and accept two parameters so it returns
options.capabilityProviderId ?? providerId synchronously (e.g.,
vi.fn().mockImplementation((providerId: string, modelId: string) =>
options.capabilityProviderId ?? providerId)), ensuring callers like
getReasoningPortrait and supportsReasoningCapability see the correct signature.
---
Outside diff comments:
In `@src/renderer/src/components/chat/ChatStatusBar.vue`:
- Around line 1956-1979: The renderer currently gates setting
defaults.reasoningEffort and defaults.reasoningVisibility on
modelConfig.reasoning === true (via canDefaultReasoningEffort), which diverges
from backend logic that uses getReasoningEffectiveEnabledForProvider(...) and
considers the effective reasoning state including default effort/portrait;
update the renderer to use the same effective check rather than only
modelConfig.reasoning: replace the canDefaultReasoningEffort logic (which uses
anthropicReasoningToggle and modelConfig.reasoning) with a call to the shared
effective reasoning check (the same predicate/utility used by
getReasoningEffectiveEnabledForProvider or a new exported helper), and then
allow normalizeReasoningEffort(...) and normalizeReasoningVisibility(...) to set
defaults when that effective-enabled check returns true (keeping
supportsReasoningEffort, normalizeReasoningEffort, normalizeReasoningVisibility,
and anthropicReasoningToggle references intact).
---
Nitpick comments:
In `@src/main/presenter/configPresenter/modelConfig.ts`:
- Around line 431-476: The code omits reasoningVisibility from the default
fallback and the explicit merge overrides, causing inconsistent propagation; add
reasoningVisibility: undefined to the initial finalConfig fallback (near
DEFAULT_MODEL_CAPABILITY_FALLBACKS initialization) and add reasoningVisibility:
finalConfig.reasoningVisibility to the storedConfig/storedSource !== 'user'
merge block (alongside reasoning, thinkingBudget,
forceInterleavedThinkingCompat, reasoningEffort, verbosity) so portrait-derived
visibility is explicitly preserved.
In `@src/main/presenter/llmProviderPresenter/providers/aiSdkProvider.ts`:
- Around line 228-321: The calls to
resolveProviderCapabilityProviderId(this.provider.id, { endpointType }, modelId)
only pass endpointType, which may allow future precedence changes in
resolveNewApiEndpointTypeFromRoute to pick the wrong capability; update each
call in the switch inside the strategy === 'new-api' branch (where
resolveNewApiEndpointType(modelId) is used and providerPatch is built for cases
'anthropic', 'gemini', 'openai-response', 'image-generation', and default) to
pass a richer route object (e.g., include type: ModelType.ImageGeneration or
supportedEndpointTypes from the model) or explicitly choose the capability
provider id per case so the resolved capability cannot drift if helper
precedence changes.
- Around line 172-179: The provider currently defers validating
this.definition.anthropicBaseUrl until getConfiguredAnthropicBaseUrl() is
called, causing runtime failures in resolveRouteDecision(); instead validate and
normalize anthropicBaseUrl in the AiSdkProvider constructor: check
this.definition.anthropicBaseUrl exists and .trim() is non-empty, throw a clear
Error referencing this.provider.id if missing, and optionally store the trimmed
value on a new private field (e.g., this.configuredAnthropicBaseUrl) to replace
getConfiguredAnthropicBaseUrl() callers so misconfiguration is detected at
construction time.
In `@src/renderer/src/components/settings/ModelConfigDialog.vue`:
- Around line 996-1011: The code runs normalization twice: first in loadConfig
(setting config.value.reasoningVisibility to
normalizeAnthropicReasoningVisibilityValue(...) ??
capabilityReasoningVisibilityDefault.value) and then again unconditionally in a
later supportsReasoningVisibility.value block that only overwrites when
normalization returns truthy; collapse these into one clear step by removing the
redundant second branch and ensure loadConfig performs
normalizeAnthropicReasoningVisibilityValue(config.value.reasoningVisibility)
once and falls back to capabilityReasoningVisibilityDefault.value when
normalization returns falsy; update the watcher (the reasoning-enabled watcher)
only if necessary so there’s a single canonical normalization path tied to
loadConfig and the supportsReasoningVisibility flag.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 284f4a1d-9bde-4fde-a061-126caf8ba55f
📒 Files selected for processing (46)
package.jsonresources/acp-registry/registry.jsonresources/model-db/providers.jsonsrc/main/presenter/agentRuntimePresenter/index.tssrc/main/presenter/configPresenter/index.tssrc/main/presenter/configPresenter/modelCapabilities.tssrc/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/llmProviderPresenter/aiSdk/providerOptionsMapper.tssrc/main/presenter/llmProviderPresenter/aiSdk/runtime.tssrc/main/presenter/llmProviderPresenter/providerRegistry.tssrc/main/presenter/llmProviderPresenter/providers/aiSdkProvider.tssrc/main/presenter/sqlitePresenter/tables/deepchatSessions.tssrc/renderer/src/components/chat/ChatStatusBar.vuesrc/renderer/src/components/settings/ModelConfigDialog.vuesrc/renderer/src/i18n/da-DK/settings.jsonsrc/renderer/src/i18n/en-US/settings.jsonsrc/renderer/src/i18n/fa-IR/settings.jsonsrc/renderer/src/i18n/fr-FR/settings.jsonsrc/renderer/src/i18n/he-IL/settings.jsonsrc/renderer/src/i18n/ja-JP/settings.jsonsrc/renderer/src/i18n/ko-KR/settings.jsonsrc/renderer/src/i18n/pt-BR/settings.jsonsrc/renderer/src/i18n/ru-RU/settings.jsonsrc/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/zh-HK/settings.jsonsrc/renderer/src/i18n/zh-TW/settings.jsonsrc/renderer/src/pages/NewThreadPage.vuesrc/renderer/src/stores/ui/draft.tssrc/shared/model.tssrc/shared/types/agent-interface.d.tssrc/shared/types/model-db.tssrc/shared/types/presenters/legacy.presenters.d.tssrc/shared/types/presenters/session.presenter.d.tstest/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.tstest/main/presenter/configPresenter/modelCapabilities.test.tstest/main/presenter/configPresenter/providerModelCapabilityMapping.test.tstest/main/presenter/llmProviderPresenter/aiSdkProviderOptionsMapper.test.tstest/main/presenter/llmProviderPresenter/aiSdkRuntime.test.tstest/main/presenter/llmProviderPresenter/newApiProvider.test.tstest/main/presenter/llmProviderPresenter/promptCacheStrategy.test.tstest/main/presenter/modelConfig.test.tstest/main/presenter/sqlitePresenter.test.tstest/main/shared/model.test.tstest/main/shared/modelDb.test.tstest/renderer/components/ChatStatusBar.test.tstest/renderer/components/ModelConfigDialog.test.ts
Comment on lines
+422
to
+429
| "reasoningVisibility": { | ||
| "label": "Synlighed for ræsonnering", | ||
| "description": "Styrer om ræsonneringsindhold skjules eller vises som et kort resumé, når ræsonnering er aktiveret", | ||
| "placeholder": "Vælg synlighed for ræsonnering", | ||
| "options": { | ||
| "omitted": "Omitted - Skjul ræsonneringsindhold", | ||
| "summarized": "Summarized - Vis ræsonnering som resumé" | ||
| } |
Contributor
There was a problem hiding this comment.
Localize the visible option prefixes.
Omitted and Summarized are user-facing English labels in the Danish locale; translate the prefixes to keep this control consistent with the rest of the page.
🌐 Proposed localization tweak
- "omitted": "Omitted - Skjul ræsonneringsindhold",
- "summarized": "Summarized - Vis ræsonnering som resumé"
+ "omitted": "Udeladt - Skjul ræsonneringsindhold",
+ "summarized": "Opsummeret - Vis ræsonnering som resumé"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "reasoningVisibility": { | |
| "label": "Synlighed for ræsonnering", | |
| "description": "Styrer om ræsonneringsindhold skjules eller vises som et kort resumé, når ræsonnering er aktiveret", | |
| "placeholder": "Vælg synlighed for ræsonnering", | |
| "options": { | |
| "omitted": "Omitted - Skjul ræsonneringsindhold", | |
| "summarized": "Summarized - Vis ræsonnering som resumé" | |
| } | |
| "reasoningVisibility": { | |
| "label": "Synlighed for ræsonnering", | |
| "description": "Styrer om ræsonneringsindhold skjules eller vises som et kort resumé, når ræsonnering er aktiveret", | |
| "placeholder": "Vælg synlighed for ræsonnering", | |
| "options": { | |
| "omitted": "Udeladt - Skjul ræsonneringsindhold", | |
| "summarized": "Opsummeret - Vis ræsonnering som resumé" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/src/i18n/da-DK/settings.json` around lines 422 - 429, The option
labels under reasoningVisibility still have English prefixes; update the values
for options.omitted and options.summarized to use Danish prefixes (e.g., change
"Omitted - Skjul ræsonneringsindhold" to a Danish-prefixed string like "Udeladt
- Skjul ræsonneringsindhold" and "Summarized - Vis ræsonnering som resumé" to
"Opsummeret - Vis ræsonnering som resumé") so the user-facing text is fully
localized while keeping the rest of each phrase intact.
| forceInterleavedThinkingCompat?: boolean | ||
| // New parameters for GPT-5 series | ||
| reasoningEffort?: ReasoningEffort | ||
| reasoningVisibility?: ReasoningVisibility |
Contributor
There was a problem hiding this comment.
Add reasoningVisibility to legacy conversation settings too.
reasoningVisibility is now part of persisted session settings, but CONVERSATION_SETTINGS still omits it. Legacy createConversation / updateConversationSettings callers will not be able to pass the new setting through this declaration.
Suggested type alignment
enabledMcpTools?: string[]
thinkingBudget?: number
reasoningEffort?: ReasoningEffort
+ reasoningVisibility?: ReasoningVisibility
verbosity?: Verbosity
selectedVariantsMap?: Record<string, string>Also applies to: 1247-1250
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/types/presenters/legacy.presenters.d.ts` at line 168, The
CONVERSATION_SETTINGS declaration in legacy.presenters.d.ts is missing the new
optional property reasoningVisibility, so update the legacy conversation
settings type(s) to include reasoningVisibility?: ReasoningVisibility wherever
CONVERSATION_SETTINGS (and the equivalent legacy conversation settings type
around the other noted location) is defined; ensure both the type used by
createConversation and updateConversationSettings accept this optional field so
legacy callers can pass the persisted session setting through.
Comment on lines
+175
to
+209
| const tracePayloads: Array<{ body?: Record<string, unknown> }> = [] | ||
| const context = { | ||
| providerKind: 'openai-compatible', | ||
| provider: { | ||
| id: 'aihubmix', | ||
| apiType: 'openai-compatible' | ||
| }, | ||
| configPresenter: { | ||
| supportsTemperatureControl: vi.fn().mockReturnValue(false) | ||
| }, | ||
| defaultHeaders: {}, | ||
| emitRequestTrace: vi.fn(async (_modelConfig, payload) => { | ||
| tracePayloads.push(payload) | ||
| }) | ||
| } as any | ||
|
|
||
| const events = [] | ||
| for await (const event of runAiSdkCoreStream( | ||
| context, | ||
| [], | ||
| modelId, | ||
| { | ||
| apiEndpoint: 'chat', | ||
| functionCall: false | ||
| } as any, | ||
| 0.5, | ||
| 2048, | ||
| [] | ||
| )) { | ||
| events.push(event) | ||
| } | ||
|
|
||
| const events = [] | ||
| for await (const event of runAiSdkCoreStream( | ||
| context, | ||
| [], | ||
| 'claude-opus-4-7-think', | ||
| { | ||
| apiEndpoint: 'chat', | ||
| functionCall: false | ||
| } as any, | ||
| 0.5, | ||
| 2048, | ||
| [] | ||
| )) { | ||
| events.push(event) | ||
| const request = mockStreamText.mock.calls[0]?.[0] as Record<string, unknown> | ||
| expect(request).not.toHaveProperty('temperature') | ||
| expect(tracePayloads[0]?.body).not.toHaveProperty('temperature') |
Contributor
There was a problem hiding this comment.
Assert the actual trace payload, not body.
The runtime emits temperature at the top level of the trace payload, so checking tracePayloads[0]?.body can pass even if the trace regresses and includes temperature.
Suggested test fix
- const tracePayloads: Array<{ body?: Record<string, unknown> }> = []
+ const tracePayloads: Array<Record<string, unknown>> = []
@@
const request = mockStreamText.mock.calls[0]?.[0] as Record<string, unknown>
+ expect(tracePayloads).toHaveLength(1)
expect(request).not.toHaveProperty('temperature')
- expect(tracePayloads[0]?.body).not.toHaveProperty('temperature')
+ expect(tracePayloads[0]).not.toHaveProperty('temperature')
expect(events).toEqual([])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/main/presenter/llmProviderPresenter/aiSdkRuntime.test.ts` around lines
175 - 209, The test currently asserts absence of temperature on
tracePayloads[0]?.body but the runtime emits temperature at the top-level of the
trace payload; update the assertion to check tracePayloads[0] itself (e.g.,
expect(tracePayloads[0]).not.toHaveProperty('temperature')) so the test fails if
temperature is emitted at the top level; modify the assertion in
aiSdkRuntime.test (near the runAiSdkCoreStream invocation and the
emitRequestTrace mock) to validate the top-level payload rather than
payload.body.
| const row = checkDb | ||
| .prepare( | ||
| 'SELECT system_prompt, summary_text, summary_cursor_order_seq, force_interleaved_thinking_compat FROM deepchat_sessions WHERE id = ?' | ||
| 'SELECT system_prompt, summary_text, summary_cursor_order_seq, force_interleaved_thinking_compat, reasoning_visibility FROM deepchat_sessions WHERE id = ?' |
Contributor
There was a problem hiding this comment.
Wrap the long SQL select string.
Line 509 exceeds the configured 100-character width.
Suggested formatting
- 'SELECT system_prompt, summary_text, summary_cursor_order_seq, force_interleaved_thinking_compat, reasoning_visibility FROM deepchat_sessions WHERE id = ?'
+ [
+ 'SELECT system_prompt, summary_text, summary_cursor_order_seq,',
+ 'force_interleaved_thinking_compat, reasoning_visibility',
+ 'FROM deepchat_sessions WHERE id = ?'
+ ].join(' ')As per coding guidelines, **/*.{ts,tsx,js,jsx}: Use single quotes, no semicolons, and maintain 100 character line width per Oxfmt configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/main/presenter/sqlitePresenter.test.ts` at line 509, The long SQL select
literal 'SELECT system_prompt, summary_text, summary_cursor_order_seq,
force_interleaved_thinking_compat, reasoning_visibility FROM deepchat_sessions
WHERE id = ?' exceeds 100 chars; split the string into multiple single-quoted
parts so no line is longer than 100 characters (e.g. break after column list and
before the FROM/WHERE clause) and concatenate them (keep single quotes and no
semicolons) where this literal appears in the test file so the final SQL is
unchanged but wrapped to satisfy the Oxfmt width rule.
Comment on lines
+361
to
+363
| getCapabilityProviderId: vi | ||
| .fn() | ||
| .mockImplementation(async (providerId: string) => options.capabilityProviderId ?? providerId), |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the real signature and how ChatStatusBar.vue consumes it.
rg -nP --type=ts -C2 '\bgetCapabilityProviderId\s*\(' -g '!**/*.test.ts' -g '!**/*.spec.ts'
fd -i 'ChatStatusBar.vue' --exec rg -nP -C3 'getCapabilityProviderId|capabilityProviderId' {}Repository: ThinkInAIXYZ/deepchat
Length of output: 7705
🏁 Script executed:
# Check if test exercises getCapabilityProviderId through other methods
sed -n '361,400p' test/renderer/components/ChatStatusBar.test.ts
# Look for actual calls to configPresenter methods in the test that might use getCapabilityProviderId
rg -n 'configPresenter\.(supportsReasoningCapability|getReasoningPortrait|getThinkingBudgetRange)' test/renderer/components/ChatStatusBar.test.ts | head -20Repository: ThinkInAIXYZ/deepchat
Length of output: 1762
🏁 Script executed:
# Check real signatures of these methods in configPresenter
rg -n 'supportsReasoningCapability|getReasoningPortrait|getThinkingBudgetRange' src/main/presenter/configPresenter/index.ts -A1
# Check if the test actually calls these presenter methods
rg -n 'configPresenter\.(supportsReasoningCapability|getReasoningPortrait|getThinkingBudgetRange|getCapabilityProviderId)' test/renderer/components/ChatStatusBar.test.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 685
🏁 Script executed:
# Check what the test does with configPresenter - look for actual calls
rg -n 'configPresenter\.' test/renderer/components/ChatStatusBar.test.ts | head -30
# Also check the broader context of how the presenter is used
sed -n '300,400p' test/renderer/components/ChatStatusBar.test.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 4318
🏁 Script executed:
# Check if getCapabilityProviderId is ever called by the component or test
rg -n 'configPresenter\.getCapabilityProviderId' test/renderer/components/ChatStatusBar.test.ts
rg -n 'this\.getCapabilityProviderId' src/renderer/components/ChatStatusBar.vueRepository: ThinkInAIXYZ/deepchat
Length of output: 146
🏁 Script executed:
# Find ChatStatusBar.vue
fd -i 'ChatStatusBar.vue'
# Check for calls to getCapabilityProviderId in test
rg -n 'getCapabilityProviderId' test/renderer/components/ChatStatusBar.test.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 151
🏁 Script executed:
# Check if ChatStatusBar.vue calls getCapabilityProviderId
rg -n 'getCapabilityProviderId' src/renderer/src/components/chat/ChatStatusBar.vue
# Also check the real implementation calls
rg -n 'getCapabilityProviderId' src/main/presenter/configPresenter/index.ts | wc -lRepository: ThinkInAIXYZ/deepchat
Length of output: 67
🏁 Script executed:
# Get actual lines where getCapabilityProviderId appears in ChatStatusBar.vue
rg -n 'getCapabilityProviderId' src/renderer/src/components/chat/ChatStatusBar.vue -A2
# Confirm the mock is async while real is sync
sed -n '691p' src/main/presenter/configPresenter/index.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 137
🏁 Script executed:
# Check full context around getCapabilityProviderId in ChatStatusBar.vue
rg -C5 'getCapabilityProviderId' src/renderer/src/components/chat/ChatStatusBar.vue
# Also verify the test doesn't call configPresenter.getCapabilityProviderId directly
rg 'configPresenter\.getCapabilityProviderId\(' test/renderer/components/ChatStatusBar.test.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 47
🏁 Script executed:
# Let's check the first rg result more carefully - get actual context
rg -n 'getCapabilityProviderId' src/renderer/src/components/chat/ChatStatusBar.vue
# Also check if test uses mocked methods that internally depend on getCapabilityProviderId
rg -n 'configPresenter\.(supportsReasoningCapability|getReasoningPortrait|getThinkingBudgetRange)' test/renderer/components/ChatStatusBar.test.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 47
🏁 Script executed:
# Check if ChatStatusBar is actually using configPresenter at all in the test
rg -n 'configPresenter' src/renderer/src/components/chat/ChatStatusBar.vue | head -5
# Look at what methods the component actually calls on configPresenter
rg -o 'configPresenter\.\w+' src/renderer/src/components/chat/ChatStatusBar.vue | sort | uniq -cRepository: ThinkInAIXYZ/deepchat
Length of output: 791
🏁 Script executed:
# Check what getReasoningPortrait does - see if it actually calls getCapabilityProviderId
sed -n '706,711p' src/main/presenter/configPresenter/index.ts
# Check what the test mocks for getReasoningPortrait
grep -A2 'getReasoningPortrait:' test/renderer/components/ChatStatusBar.test.tsRepository: ThinkInAIXYZ/deepchat
Length of output: 542
🏁 Script executed:
# Final verification: is getCapabilityProviderId ever actually invoked in any context?
# Check if test uses any method that would call getCapabilityProviderId
grep -n 'getReasoningPortrait\|supportsReasoningCapability\|getThinkingBudgetRange\|getTemperatureCapability\|supportsTemperatureControl\|getReasoningEffortDefault\|supportsReasoningEffortCapability\|getVerbosityDefault\|supportsVerbosityCapability' test/renderer/components/ChatStatusBar.test.ts
# Also verify the mocked methods are actually called in tests
rg -n '\.toHaveBeenCalled' test/renderer/components/ChatStatusBar.test.ts | grep -E 'getReasoningPortrait|supportsReasoningCapability|getThinkingBudgetRange'Repository: ThinkInAIXYZ/deepchat
Length of output: 812
🏁 Script executed:
# Check if any test assertions verify getCapabilityProviderId was called
grep -n 'getCapabilityProviderId' test/renderer/components/ChatStatusBar.test.ts
# Check if the mocked methods that depend on getCapabilityProviderId are actually used/asserted
rg -n 'expect.*getReasoningPortrait|expect.*supportsReasoningCapability|expect.*getThinkingBudgetRange' test/renderer/components/ChatStatusBar.test.ts | head -10Repository: ThinkInAIXYZ/deepchat
Length of output: 100
Mock signature does not match the real method.
getCapabilityProviderId(providerId: string, modelId: string) in src/main/presenter/configPresenter/index.ts:691 is synchronous and takes 2 parameters, but the mock (lines 361-363) is async and accepts only 1 parameter. While the mock is not directly invoked in this test, it should match the real signature for consistency and to prevent bugs if the test is later modified to exercise the dependent methods (getReasoningPortrait, supportsReasoningCapability, etc. that call getCapabilityProviderId internally).
🔧 Suggested fix
- getCapabilityProviderId: vi
- .fn()
- .mockImplementation(async (providerId: string) => options.capabilityProviderId ?? providerId),
+ getCapabilityProviderId: vi
+ .fn()
+ .mockImplementation(
+ (providerId: string, _modelId: string) => options.capabilityProviderId ?? providerId
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| getCapabilityProviderId: vi | |
| .fn() | |
| .mockImplementation(async (providerId: string) => options.capabilityProviderId ?? providerId), | |
| getCapabilityProviderId: vi | |
| .fn() | |
| .mockImplementation( | |
| (providerId: string, _modelId: string) => options.capabilityProviderId ?? providerId | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/renderer/components/ChatStatusBar.test.ts` around lines 361 - 363, The
mock for getCapabilityProviderId must match the real synchronous signature
(providerId: string, modelId: string); update the mock in ChatStatusBar.test to
remove async and accept two parameters so it returns
options.capabilityProviderId ?? providerId synchronously (e.g.,
vi.fn().mockImplementation((providerId: string, modelId: string) =>
options.capabilityProviderId ?? providerId)), ensuring callers like
getReasoningPortrait and supportsReasoningCapability see the correct signature.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
test/main/presenter/llmProviderPresenter/newApiProvider.test.ts (1)
251-256: Guard against an undefinedanthropicBaseUrlsilently passing.
toBe(definition?.anthropicBaseUrl)will pass trivially if both sides areundefined(e.g., registry regression dropsanthropicBaseUrlfrom zenmux). Consider asserting the expected value upfront so the intent of the test is preserved even if the registry changes.♻️ Suggested tightening
const definition = resolveAiSdkProviderDefinition(zenmuxProvider) + expect(definition?.anthropicBaseUrl).toBeTruthy() expect(routeDecision.providerKind).toBe('anthropic') expect(routeDecision.supportsOfficialAnthropicReasoning).toBe(true) expect(runtimeProvider.apiType).toBe('anthropic') expect(runtimeProvider.baseUrl).toBe(definition?.anthropicBaseUrl)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/main/presenter/llmProviderPresenter/newApiProvider.test.ts` around lines 251 - 256, The test currently compares runtimeProvider.baseUrl to definition?.anthropicBaseUrl which can both be undefined and silently pass; before the existing expect runtimeProvider.baseUrl assertion (and using resolveAiSdkProviderDefinition(zenmuxProvider)), add an explicit assertion that definition?.anthropicBaseUrl is defined (e.g., expect(definition?.anthropicBaseUrl).toBeDefined()) or assert it equals the expected literal URL, then assert runtimeProvider.baseUrl equals that defined value so the test fails if the registry drops anthropicBaseUrl; reference resolveAiSdkProviderDefinition, definition, runtimeProvider, and routeDecision in your changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/src/components/chat/ChatStatusBar.vue`:
- Around line 1680-1686: The computed showReasoningVisibility currently hides
the control when localSettings.value?.reasoningVisibility is undefined; remove
that presence check so the control is shown whenever Anthropic
capabilities/options are available. Update the computed (referencing
showReasoningVisibility, isAcpAgent, localSettings, hasAnthropicReasoningToggle,
capabilityProviderId, capabilityReasoningPortrait) to gate only on
!isAcpAgent.value, Boolean(localSettings.value), and
hasAnthropicReasoningToggle(...), letting the Select's existing fallback handle
missing persisted reasoningVisibility values.
In `@src/shared/model.ts`:
- Around line 80-83: The function isClaudeOpus47FamilyModelId has a misleading
"Family" suffix but performs exact matches; rename the function to
isClaudeOpus47ModelId (and update all call sites) to reflect exact-match
semantics, or alternatively change its check inside normalizeUnprefixedModelId
to use startsWith('claude-opus-4-7') if you prefer family-style matching; update
the exported function name and any imports/usages accordingly to keep API
consistent with isClaudeFamilyModelId naming.
In `@test/main/presenter/llmProviderPresenter/aiSdkRuntime.test.ts`:
- Around line 177-210: The test must exercise the capability-provider route:
change the context to use providerKind 'capability-provider' (instead of
'openai-compatible') and make configPresenter.supportsTemperatureControl return
true so the runtime takes the capability-provider branch when calling
runAiSdkCoreStream; keep the rest of the setup and then assert that the outgoing
request (mockStreamText.mock.calls[0][0]) and tracePayloads[0].body still do not
include a 'temperature' property. Use the symbols context.providerKind,
context.configPresenter.supportsTemperatureControl, and runAiSdkCoreStream to
locate and modify the test.
- Around line 254-304: The test currently uses portraitSpy =
vi.spyOn(modelCapabilities, 'getReasoningPortrait').mockReturnValue(...) which
masks whether getReasoningPortrait was invoked with the correct provider and
model; after calling runAiSdkGenerateText assert the spy received the expected
routing args (e.g. expect(portraitSpy).toHaveBeenCalledWith('anthropic',
'zenmux', 'anthropic/claude-opus-4-7') or the exact two/three-arg signature used
by getReasoningPortrait in your code) before restoring the spy so the test
verifies the lookup route as well as the returned portrait; locate
getReasoningPortrait, portraitSpy and runAiSdkGenerateText in the test to add
this assertion.
---
Nitpick comments:
In `@test/main/presenter/llmProviderPresenter/newApiProvider.test.ts`:
- Around line 251-256: The test currently compares runtimeProvider.baseUrl to
definition?.anthropicBaseUrl which can both be undefined and silently pass;
before the existing expect runtimeProvider.baseUrl assertion (and using
resolveAiSdkProviderDefinition(zenmuxProvider)), add an explicit assertion that
definition?.anthropicBaseUrl is defined (e.g.,
expect(definition?.anthropicBaseUrl).toBeDefined()) or assert it equals the
expected literal URL, then assert runtimeProvider.baseUrl equals that defined
value so the test fails if the registry drops anthropicBaseUrl; reference
resolveAiSdkProviderDefinition, definition, runtimeProvider, and routeDecision
in your changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bea5f950-c397-4919-8ad0-162289e7b15b
📒 Files selected for processing (14)
src/main/presenter/configPresenter/index.tssrc/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/llmProviderPresenter/providers/aiSdkProvider.tssrc/renderer/src/components/chat/ChatStatusBar.vuesrc/renderer/src/components/settings/ModelConfigDialog.vuesrc/shared/model.tstest/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.tstest/main/presenter/configPresenter/providerModelCapabilityMapping.test.tstest/main/presenter/llmProviderPresenter/aiSdkRuntime.test.tstest/main/presenter/llmProviderPresenter/newApiProvider.test.tstest/main/presenter/modelConfig.test.tstest/main/shared/model.test.tstest/renderer/components/ChatStatusBar.test.tstest/renderer/components/ModelConfigDialog.test.ts
✅ Files skipped from review due to trivial changes (1)
- src/main/presenter/llmProviderPresenter/providers/aiSdkProvider.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- test/main/presenter/modelConfig.test.ts
- src/main/presenter/configPresenter/modelConfig.ts
- test/main/shared/model.test.ts
- src/renderer/src/components/settings/ModelConfigDialog.vue
- test/renderer/components/ChatStatusBar.test.ts
Comment on lines
+80
to
+83
| export function isClaudeOpus47FamilyModelId(modelId: string | undefined): boolean { | ||
| const normalizedModelId = normalizeUnprefixedModelId(modelId) | ||
| return normalizedModelId === 'claude-opus-4-7' || normalizedModelId === 'claude-opus-4-7-think' | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all call sites to confirm whether exact-match or family semantics is expected.
rg -nP --type=ts -C3 '\bisClaudeOpus47FamilyModelId\s*\('Repository: ThinkInAIXYZ/deepchat
Length of output: 1873
🏁 Script executed:
#!/bin/bash
# Check for other model ID classifier functions to see naming/pattern conventions
rg -nP --type=ts 'export\s+function\s+is.*ModelId\s*\(' | head -20Repository: ThinkInAIXYZ/deepchat
Length of output: 365
🏁 Script executed:
#!/bin/bash
# Search for any dated variant model IDs or version patterns in the codebase
rg -nP '(claude-opus-4-7|claude-[a-z0-9-]+)-\d{8}|claude-[a-z0-9-]+-latest|claude-[a-z0-9-]+-v\d+'Repository: ThinkInAIXYZ/deepchat
Length of output: 15921
🏁 Script executed:
#!/bin/bash
# Check how normalizeUnprefixedModelId works to understand what gets stripped
rg -nP --type=ts -A5 'function normalizeUnprefixedModelId'Repository: ThinkInAIXYZ/deepchat
Length of output: 464
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts -A3 'export function isClaudeFamilyModelId'Repository: ThinkInAIXYZ/deepchat
Length of output: 291
Misleading function name — rename for consistency with isClaudeFamilyModelId.
The "family" suffix suggests broad matching like isClaudeFamilyModelId() (which uses .includes('claude')), but this function only matches exact IDs: claude-opus-4-7 and claude-opus-4-7-think. Rename to isClaudeOpus47ModelId to accurately reflect exact-match semantics, or switch to startsWith('claude-opus-4-7') if Anthropic ships dated variants (though none exist in the model-db currently).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/model.ts` around lines 80 - 83, The function
isClaudeOpus47FamilyModelId has a misleading "Family" suffix but performs exact
matches; rename the function to isClaudeOpus47ModelId (and update all call
sites) to reflect exact-match semantics, or alternatively change its check
inside normalizeUnprefixedModelId to use startsWith('claude-opus-4-7') if you
prefer family-style matching; update the exported function name and any
imports/usages accordingly to keep API consistent with isClaudeFamilyModelId
naming.
Comment on lines
+177
to
+210
| const context = { | ||
| providerKind: 'openai-compatible', | ||
| provider: { | ||
| id: 'aihubmix', | ||
| apiType: 'openai-compatible' | ||
| }, | ||
| configPresenter: { | ||
| supportsTemperatureControl: vi.fn().mockReturnValue(false) | ||
| }, | ||
| defaultHeaders: {}, | ||
| emitRequestTrace: vi.fn(async (_modelConfig, payload) => { | ||
| tracePayloads.push(payload) | ||
| }) | ||
| } as any | ||
|
|
||
| const events = [] | ||
| for await (const event of runAiSdkCoreStream( | ||
| context, | ||
| [], | ||
| modelId, | ||
| { | ||
| apiEndpoint: 'chat', | ||
| functionCall: false | ||
| } as any, | ||
| 0.5, | ||
| 2048, | ||
| [] | ||
| )) { | ||
| events.push(event) | ||
| } | ||
|
|
||
| const request = mockStreamText.mock.calls[0]?.[0] as Record<string, unknown> | ||
| expect(request).not.toHaveProperty('temperature') | ||
| expect(tracePayloads[0]?.body).not.toHaveProperty('temperature') |
Contributor
There was a problem hiding this comment.
Exercise the capability-provider route in this test.
Right now supportsTemperatureControl returns false for any provider, so this passes even if the runtime queries aihubmix instead of Anthropic capabilities.
Suggested test tightening
const context = {
providerKind: 'openai-compatible',
provider: {
id: 'aihubmix',
- apiType: 'openai-compatible'
+ apiType: 'openai-compatible',
+ capabilityProviderId: 'anthropic'
},
configPresenter: {
supportsTemperatureControl: vi.fn().mockReturnValue(false)
@@
const request = mockStreamText.mock.calls[0]?.[0] as Record<string, unknown>
+ expect(context.configPresenter.supportsTemperatureControl).toHaveBeenCalledWith(
+ 'anthropic',
+ modelId
+ )
expect(request).not.toHaveProperty('temperature')
expect(tracePayloads[0]?.body).not.toHaveProperty('temperature')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const context = { | |
| providerKind: 'openai-compatible', | |
| provider: { | |
| id: 'aihubmix', | |
| apiType: 'openai-compatible' | |
| }, | |
| configPresenter: { | |
| supportsTemperatureControl: vi.fn().mockReturnValue(false) | |
| }, | |
| defaultHeaders: {}, | |
| emitRequestTrace: vi.fn(async (_modelConfig, payload) => { | |
| tracePayloads.push(payload) | |
| }) | |
| } as any | |
| const events = [] | |
| for await (const event of runAiSdkCoreStream( | |
| context, | |
| [], | |
| modelId, | |
| { | |
| apiEndpoint: 'chat', | |
| functionCall: false | |
| } as any, | |
| 0.5, | |
| 2048, | |
| [] | |
| )) { | |
| events.push(event) | |
| } | |
| const request = mockStreamText.mock.calls[0]?.[0] as Record<string, unknown> | |
| expect(request).not.toHaveProperty('temperature') | |
| expect(tracePayloads[0]?.body).not.toHaveProperty('temperature') | |
| const context = { | |
| providerKind: 'openai-compatible', | |
| provider: { | |
| id: 'aihubmix', | |
| apiType: 'openai-compatible', | |
| capabilityProviderId: 'anthropic' | |
| }, | |
| configPresenter: { | |
| supportsTemperatureControl: vi.fn().mockReturnValue(false) | |
| }, | |
| defaultHeaders: {}, | |
| emitRequestTrace: vi.fn(async (_modelConfig, payload) => { | |
| tracePayloads.push(payload) | |
| }) | |
| } as any | |
| const events = [] | |
| for await (const event of runAiSdkCoreStream( | |
| context, | |
| [], | |
| modelId, | |
| { | |
| apiEndpoint: 'chat', | |
| functionCall: false | |
| } as any, | |
| 0.5, | |
| 2048, | |
| [] | |
| )) { | |
| events.push(event) | |
| } | |
| const request = mockStreamText.mock.calls[0]?.[0] as Record<string, unknown> | |
| expect(context.configPresenter.supportsTemperatureControl).toHaveBeenCalledWith( | |
| 'anthropic', | |
| modelId | |
| ) | |
| expect(request).not.toHaveProperty('temperature') | |
| expect(tracePayloads[0]?.body).not.toHaveProperty('temperature') |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/main/presenter/llmProviderPresenter/aiSdkRuntime.test.ts` around lines
177 - 210, The test must exercise the capability-provider route: change the
context to use providerKind 'capability-provider' (instead of
'openai-compatible') and make configPresenter.supportsTemperatureControl return
true so the runtime takes the capability-provider branch when calling
runAiSdkCoreStream; keep the rest of the setup and then assert that the outgoing
request (mockStreamText.mock.calls[0][0]) and tracePayloads[0].body still do not
include a 'temperature' property. Use the symbols context.providerKind,
context.configPresenter.supportsTemperatureControl, and runAiSdkCoreStream to
locate and modify the test.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/components/chat/ChatStatusBar.vue (1)
2001-2028:⚠️ Potential issue | 🟠 MajorGuard capability writes against stale async responses.
syncGenerationSettingschecksgenerationSyncTokenonly afterfetchCapabilitiesreturns, butfetchCapabilitieshas already mutated capability refs by then. A slow request from the previous model can overwrite the current model’scapabilityProviderId/portrait and render or persist the wrong reasoning controls.🐛 Proposed fix
-const fetchCapabilities = async (providerId: string, modelId: string): Promise<void> => { +const fetchCapabilities = async ( + providerId: string, + modelId: string, + isStale: () => boolean +): Promise<void> => { try { const modelConfig = await configPresenter.getModelConfig(modelId, providerId) - capabilityProviderId.value = resolveCapabilityProviderIdForSelection( + const nextCapabilityProviderId = resolveCapabilityProviderIdForSelection( providerId, modelId, modelConfig.endpointType ) const portrait = (await configPresenter.getReasoningPortrait?.(providerId, modelId)) ?? null const temperatureSupport = await configPresenter.supportsTemperatureControl?.( providerId, modelId ) + const nextTemperatureSupport = + typeof temperatureSupport === 'boolean' + ? temperatureSupport + : ((await configPresenter.getTemperatureCapability?.(providerId, modelId)) ?? null) + + if (isStale()) { + return + } + capabilityProviderId.value = nextCapabilityProviderId capabilityReasoningPortrait.value = portrait capabilitySupportsReasoning.value = typeof portrait?.supported === 'boolean' ? portrait.supported : null - capabilitySupportsTemperature.value = - typeof temperatureSupport === 'boolean' - ? temperatureSupport - : ((await configPresenter.getTemperatureCapability?.(providerId, modelId)) ?? null) + capabilitySupportsTemperature.value = nextTemperatureSupport } catch (error) { console.warn('[ChatStatusBar] Failed to fetch model capabilities:', error) + if (isStale()) { + return + } capabilityProviderId.value = providerId capabilitySupportsReasoning.value = null capabilityReasoningPortrait.value = null capabilitySupportsTemperature.value = null } }- await fetchCapabilities(selection.providerId, selection.modelId) + await fetchCapabilities( + selection.providerId, + selection.modelId, + () => token !== generationSyncToken + ) if (token !== generationSyncToken) { return }Also applies to: 2151-2154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/components/chat/ChatStatusBar.vue` around lines 2001 - 2028, fetchCapabilities mutates capability refs after awaiting async calls, allowing stale responses to overwrite current model settings; capture the current generationSyncToken (the same token used by syncGenerationSettings) at the start of fetchCapabilities and verify it is unchanged before applying any mutations (capabilityProviderId, capabilityReasoningPortrait, capabilitySupportsReasoning, capabilitySupportsTemperature), aborting writes if the token has changed; apply the same guard to the other capability-write block referenced around lines 2151-2154 so only responses matching the captured token update state.
🧹 Nitpick comments (2)
test/main/presenter/llmProviderPresenter/aiSdkProviderOptionsMapper.test.ts (1)
156-192: Minor: assertresult.providerOptions?.openaiis absent for the zenmux case.This test pins the Anthropic-shape output but doesn't verify that zenmux — which is a routed provider — doesn't also accidentally emit an
openaioptions bucket alongside. A single extraexpect(result.providerOptions?.openai).toBeUndefined()would make the test resilient to future mapper changes that inadvertently add a second namespace for routed providers.Proposed tightening
expect(result.providerOptions?.anthropic).toMatchObject({ toolStreaming: true, sendReasoning: true, effort: 'xhigh', thinking: { type: 'adaptive', display: 'summarized' } }) + expect(result.providerOptions?.openai).toBeUndefined() })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/main/presenter/llmProviderPresenter/aiSdkProviderOptionsMapper.test.ts` around lines 156 - 192, Add an assertion to the 'maps zenmux anthropic routes to official anthropic adaptive reasoning controls' test that ensures routed providers don't accidentally emit an OpenAI options bucket: after calling buildProviderOptions and storing in result, add expect(result.providerOptions?.openai).toBeUndefined() to confirm only the anthropic namespace is present; reference the test's result variable and buildProviderOptions call to locate where to insert the new assertion.src/main/presenter/llmProviderPresenter/aiSdk/runtime.ts (1)
21-40: Narrow theconfigPresentercontract by makinggetCapabilityProviderIdrequired.The interface
IConfigPresentermarksgetCapabilityProviderIdas optional (line 521 insrc/shared/types/presenters/legacy.presenters.d.ts), but the actualConfigPresenterimplementation always provides it (line 699 insrc/main/presenter/configPresenter/index.ts). The code inresolveCapabilityProviderIduses optional chaining defensively (?.), which can mask implementation issues. Since this method is now integral to capability routing across the codebase, declare it as required onIConfigPresenterso the TypeScript contract matches runtime expectations and call sites reflect the actual dependency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/llmProviderPresenter/aiSdk/runtime.ts` around lines 21 - 40, The IConfigPresenter interface currently marks getCapabilityProviderId as optional but runtime and ConfigPresenter implementation always provide it, so update the IConfigPresenter declaration to make getCapabilityProviderId required (remove the optional flag) so the TypeScript contract matches runtime; then update any call sites like resolveCapabilityProviderId that use optional chaining on configPresenter.getCapabilityProviderId to call it directly, ensuring references to AiSdkRuntimeContext.configPresenter reflect the tightened contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/src/components/chat/ChatStatusBar.vue`:
- Around line 443-445: The Select is binding localSettings.reasoningVisibility
directly, but persisted values like "hidden"/"visible" may not match the current
reasoningVisibilityOptions ("omitted"/"summarized"), so add a normalized
computed (or method) used in the model-value binding that maps legacy values
("hidden"->"omitted", "visible"->"summarized") and falls back to
reasoningVisibilityOptions[0]?.value; update the model-value to use that
computed (referencing localSettings.reasoningVisibility and
reasoningVisibilityOptions) so older sessions show a valid option instead of an
out-of-options selection.
---
Outside diff comments:
In `@src/renderer/src/components/chat/ChatStatusBar.vue`:
- Around line 2001-2028: fetchCapabilities mutates capability refs after
awaiting async calls, allowing stale responses to overwrite current model
settings; capture the current generationSyncToken (the same token used by
syncGenerationSettings) at the start of fetchCapabilities and verify it is
unchanged before applying any mutations (capabilityProviderId,
capabilityReasoningPortrait, capabilitySupportsReasoning,
capabilitySupportsTemperature), aborting writes if the token has changed; apply
the same guard to the other capability-write block referenced around lines
2151-2154 so only responses matching the captured token update state.
---
Nitpick comments:
In `@src/main/presenter/llmProviderPresenter/aiSdk/runtime.ts`:
- Around line 21-40: The IConfigPresenter interface currently marks
getCapabilityProviderId as optional but runtime and ConfigPresenter
implementation always provide it, so update the IConfigPresenter declaration to
make getCapabilityProviderId required (remove the optional flag) so the
TypeScript contract matches runtime; then update any call sites like
resolveCapabilityProviderId that use optional chaining on
configPresenter.getCapabilityProviderId to call it directly, ensuring references
to AiSdkRuntimeContext.configPresenter reflect the tightened contract.
In `@test/main/presenter/llmProviderPresenter/aiSdkProviderOptionsMapper.test.ts`:
- Around line 156-192: Add an assertion to the 'maps zenmux anthropic routes to
official anthropic adaptive reasoning controls' test that ensures routed
providers don't accidentally emit an OpenAI options bucket: after calling
buildProviderOptions and storing in result, add
expect(result.providerOptions?.openai).toBeUndefined() to confirm only the
anthropic namespace is present; reference the test's result variable and
buildProviderOptions call to locate where to insert the new assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71f5fc96-4ac5-4ce0-b556-1a97a753a79b
📒 Files selected for processing (11)
resources/acp-registry/registry.jsonresources/model-db/providers.jsonsrc/main/presenter/llmProviderPresenter/aiSdk/runtime.tssrc/main/presenter/llmProviderPresenter/providers/aiSdkProvider.tssrc/renderer/src/components/chat/ChatStatusBar.vuesrc/shared/model.tstest/main/presenter/llmProviderPresenter/aiSdkProviderOptionsMapper.test.tstest/main/presenter/llmProviderPresenter/aiSdkRuntime.test.tstest/main/presenter/llmProviderPresenter/newApiProvider.test.tstest/main/shared/model.test.tstest/renderer/components/ChatStatusBar.test.ts
✅ Files skipped from review due to trivial changes (2)
- test/main/presenter/llmProviderPresenter/aiSdkRuntime.test.ts
- test/main/shared/model.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- test/main/presenter/llmProviderPresenter/newApiProvider.test.ts
- test/renderer/components/ChatStatusBar.test.ts
- src/main/presenter/llmProviderPresenter/providers/aiSdkProvider.ts
- resources/acp-registry/registry.json
Comment on lines
+443
to
+445
| :model-value=" | ||
| localSettings.reasoningVisibility ?? reasoningVisibilityOptions[0]?.value | ||
| " |
Contributor
There was a problem hiding this comment.
Normalize persisted visibility before binding it to the Select.
localSettings.reasoningVisibility can contain broader persisted values like hidden/visible, but this Select only renders Anthropic options (omitted, summarized). Bind the normalized value so older sessions don’t show an out-of-options selection.
🐛 Proposed fix
:model-value="
- localSettings.reasoningVisibility ?? reasoningVisibilityOptions[0]?.value
+ normalizeReasoningVisibility(
+ capabilityProviderId,
+ capabilityReasoningPortrait,
+ localSettings.reasoningVisibility
+ ) ?? reasoningVisibilityOptions[0]?.value
"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| :model-value=" | |
| localSettings.reasoningVisibility ?? reasoningVisibilityOptions[0]?.value | |
| " | |
| :model-value=" | |
| normalizeReasoningVisibility( | |
| capabilityProviderId, | |
| capabilityReasoningPortrait, | |
| localSettings.reasoningVisibility | |
| ) ?? reasoningVisibilityOptions[0]?.value | |
| " |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/src/components/chat/ChatStatusBar.vue` around lines 443 - 445,
The Select is binding localSettings.reasoningVisibility directly, but persisted
values like "hidden"/"visible" may not match the current
reasoningVisibilityOptions ("omitted"/"summarized"), so add a normalized
computed (or method) used in the model-value binding that maps legacy values
("hidden"->"omitted", "visible"->"summarized") and falls back to
reasoningVisibilityOptions[0]?.value; update the model-value to use that
computed (referencing localSettings.reasoningVisibility and
reasoningVisibilityOptions) so older sessions show a valid option instead of an
out-of-options selection.
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
2 participants
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.
support anthropic adaptive reasoning
Summary by CodeRabbit
New Features
Chores