fix: fall back to the model default for thinking efforts outside support_efforts - #3144
Conversation
…ort_efforts Non-Kimi providers used to pass a configured thinking effort through unchanged even when the model declared a support_efforts list that did not contain it, so backends rejecting unknown efforts failed every session. Both engines now resolve an unlisted concrete effort to the model's default effort (declared default_effort, else the middle list entry) whenever a support_efforts list is declared, and emit a one-time warning naming the configured value and the applied fallback. Models without a declared list keep the pass-through behavior.
🦋 Changeset detectedLatest commit: b290ac3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@codex review |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be43c7e5bb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…inking capability A custom model may declare support_efforts/default_effort while omitting the thinking capability. The unlisted-effort fallback went through defaultThinkingEffortFor(Model), whose capability gate resolved such a model to 'off' — silently disabling thinking instead of applying the declared default. The fallback now derives directly from the declared list (declared default_effort when listed, else the middle entry), and the fallback report reuses the normalized result so it cannot drift from what is actually applied.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a24ef25445
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…n resume Sessions persisted before the declared-list fallback restore their recorded thinkingEffort verbatim: wire replay folds ProfileBind and ConfigUpdate straight into profile state, so an effort the model does not list was still sent upstream and rejected. The profile's thinkingLevel getter now always re-resolves the stored value against the current model (idempotent for already-normalized values, and the always-thinking off-clamp it used to special-case is just the general path), and a post-restore hook emits the one-time fallback warning when the persisted value had to be corrected.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08902a0a65
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…isted KIMI_MODEL_THINKING_EFFORT is applied after the model-aware resolution and bypasses the declared support_efforts list by design, so the new fallback warning never covers it and the request-time check this PR removed was the last diagnostic for an out-of-list pin. Validate the final overridden effort at the point the override is applied and emit a one-time warning that the value will be sent unchanged, mirroring the v2 requester-layer diagnostic.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1217ec206
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d32ed942f9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…rt wins When KIMI_MODEL_THINKING_EFFORT (v1) or thinking.forcedEffort (v2) decides the final effort, the configured value never reaches the wire, so warning that it will fall back to the model default misleads. Both engines now skip the fallback warning whenever a forced override applies; an out-of-list forced value is still reported by the override/passthrough warning that describes what is actually sent.
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a1f7500fc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ng effort Live sessions resolve the thinking effort once and cache it, while the ProviderManager reads model metadata lazily from the core config; a reload that narrows a model's declared support_efforts therefore pairs the new list with the stale cached effort and sends it upstream with no diagnostic (the pre-fallback request-time checker covered this for Anthropic-routed providers). A minimal request-time check now compares the provider's effort against the freshly resolved declared list and emits a one-time warning that the value is sent unchanged; the wire value itself is untouched, as it was before this PR.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b8c8da7e8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…model default createSession pre-resolved an explicit thinking effort against the raw model alias before handing it to ConfigState, so the fallback metadata was discarded and the bootstrap update saw an already-normalized, in-list value — the one-time fallback warning never fired on this path. Forward the raw requested effort and let ConfigState.update() resolve it against the resolved provider, the same single resolution point used by every other path.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 134c1f6a30
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…nfig reload A reload that changes both [providers] and [models] fires one service event per section, and the per-event revalidation ran against the intermediate state — e.g. a provider switch whose inferred list lacks the stored effort produced a fallback warning and status flip moments before the model record carrying the explicit declaration arrived. Both listeners now schedule a single macrotask-coalesced revalidation, so the check always runs against the fully bridged configuration.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 534780d12d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The effective-model composition dropped an inherited default_effort whenever the raw override support_efforts list did not contain it verbatim, so a padded or differently-cased declaration discarded the declared default and resolution fell back to the middle entry. The coverage check now uses the resolvers' normalization (trimmed, case-insensitive); the inherited default is left untouched and the resolver still returns the declared canonical form.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a4a92cafc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ted scheduler The reload revalidation is scheduled with setTimeout(0) in production, and the tests flushed the real event loop with setImmediate polling to wait for it — phase-dependent and environment-sensitive. The profile service now accepts an optional scheduler through its existing configure() options (defaulting to setTimeout(0)); tests inject a manual queue and run the coalesced revalidation explicitly, which also pins the merge itself (both reload events produce exactly one queued run).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 742c3700a5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…s too The request-time and override warnings still compared the effective effort against the declared list with exact string membership, so a mixed-case declaration like ["High"] false-warned about a forced "high" that the resolver had already accepted. All three diagnostic paths (v1 override, v1 stale, v2 requester) now share the resolver's normalized, case-insensitive membership check; the warning text keeps showing the declared entries in their canonical form.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9799e01b38
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd09758da7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 611438dd70
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ad revalidation The reload revalidation compared the effective effort against a memo that every public read path refreshed, so any consumer reading the level between the metadata change and the scheduled revalidation moved the baseline to the new value and the status republication was skipped while clients still showed the old one. The baseline now only advances when a status carrying the effort is actually emitted, on both the dispatch and the custom emitStatusUpdated paths.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
1 participant
Related Issue
Resolve #3143
Problem
See linked issue. In short: only Kimi-protocol providers fell back when the configured thinking effort was missing from a model's declared
support_efforts; every other provider passed the value through verbatim, so a single stale global[thinking].effortcould 400 every session and headless process using a self-hosted OpenAI-compatible model (e.g. vLLM endpoints declaring["low", "medium", "xhigh"]receivinghigh).What changed
agent-coreandagent-core-v2(normalizeThinkingEffortForModel, both engines): when a model declares asupport_effortslist, an unlisted concrete effort now resolves to the model's default effort (declareddefault_effort, else the middle list entry) regardless of protocol — the same rule Kimi-protocol providers already used. Models without a declared list keep the pass-through behavior, so their backend still makes the final capability decision;on/offhandling is unchanged.resolveThinkingEffort*WithFallbackvariant that reports the fallback ({configured, resolved}); the former anthropic-only warnings (v1agent/index.ts, v2profileService.ts) were generalized to fire for any protocol when a fallback occurs, with the message naming the configured value and the applied fallback. The v2llmRequesterServicepre-request check keeps its pass-through semantics (that layer does not normalize) but no longer gates on the anthropic protocol.kosongandagent-core-v2now describes the declared-list fallback instead of claiming efforts are never mapped client-side.Why this approach: a declared
support_effortslist means the client knows the valid value set, so falling back is grounded rather than a guess; leaving undeclared models untouched preserves the deliberate "let the backend decide" contract for them.Tests: updated expectations in the existing thinking-effort suites of both engines and added coverage for the Qwen-style case (declared
["low","medium","xhigh"], defaultxhigh, configuredhigh→ resolvesxhigh+ warning), undeclared-list pass-through, andon/offregressions. Fullagent-core(4194 tests) andagent-core-v2(5502 tests) suites pass,tsc --noEmitclean,check-no-commentsand oxlint clean.Note: the web UI persisting a session-level effort choice into the global config (the other half of the user report in #3143) lives in the code-app frontend and is not addressed here.
Checklist
/approve).gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.