fix(models): default thinking level to high when no effort is set#2766
Merged
Conversation
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
fd8bcfc to
7251bfc
Compare
Contributor
|
Reviews (1): Last reviewed commit: "default thinking level to high for effor..." | Re-trigger Greptile |
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.
Problem
Users who pick claude-fable-5 hit hard failures whenever a generation runs without a configured effort/thinking level (8 users and 142 failed generations on 06-11 alone, invisible to error monitoring since
$ai_http_status=None).fable-5 only supports adaptive thinking and rejects the SDK's default
thinking: { type: "disabled" }withinvalid_request_error: "thinking.type.disabled" is not supported for this model. The effort level only reaches the SDK when present, so a fable-5 session with no effort set lets the SDK default to disabled thinking, which fable-5 rejects outright.Same issue as #2618. This PR takes a different approach: instead of scoping a
mediumdefault to fable-5 behind arequiresAdaptiveThinkingflag, it makes high the default thinking level for every effort-capable model. That keeps thinking enabled by default and removes the disabled-thinking request shape that fable-5 rejects.Changes
DEFAULT_EFFORT("high") and aresolveEffortForModelhelper insession/models.ts. The helper honors an explicit effort, falls back to high for effort-capable models, and leaves models without effort support (e.g. haiku) unset so they keep disabled thinking.claude-agent.tscreateSession, after the gatewaymodelIdis resolved, apply the default effort viaapplyFlagSettingswhen the user has not chosen one. Gating on the resolved id (not the raw setting) means the default also applies to the default model and aliases, not just explicit fable-5 picks.rebuildEffortConfigOption): when switching to an effort-capable model with no explicit effort, apply the default to the live query instead of only updating the display. This closes the same fable-5 failure when the model is changed from the picker, which fix(models): default fable-5 to adaptive thinking when no effort is set #2618 left open.buildConfigOptionsfallback to high so the UI matches the request shape.How did you test this?
models.test.ts:resolveEffortForModel(effort-capable models default to high; haiku/opus-4-6 stay unset; explicit effort is honored, includingmaxon fable-5) andDEFAULT_EFFORT === "high".claude-agent.resume-model.test.tsquery mock to includeapplyFlagSettings(the real SDK query exposes it; the mock did not).pnpm --filter @posthog/agent exec vitest run src/adapters/claude src/server— 486 tests pass.pnpm --filter @posthog/agent typecheckpasses;biome checkclean on changed files;node scripts/check-host-boundaries.mjsshows no new violations.Automatic notifications