fix(thinking): config thinking_budget_tokens no longer silently discarded - #113
Merged
Brian Krabach (bkrabach) merged 1 commit intoSep 2, 2026
Merged
Conversation
…carded
An explicitly configured `thinking_budget_tokens` was accepted without
complaint and then thrown away. Lane d0q measured it inert in 4 of 5
reachable configurations on claude-haiku-4-5 -- the model where
`thinking.budget_tokens` is the ENTIRE reasoning dial, since `effort` never
reaches the wire there at all. The only budgets a config could select were
{4096 (effort: low), 32000 (everything else)}.
Root cause: the budget chain resolved
kwargs > effort_budget > config > model default
and the effort ladder always produced a non-None `effort_budget` whenever ANY
reasoning_effort was set -- so config sat below a value that was always
present. For every effort except "low" that ladder value is literally
`request_caps.default_thinking_budget`, i.e. the config key was shadowed by
the default it was meant to override.
The defect is the SILENCE, not the precedence -- the same defect class a
discarded `effort` was given a loader guard for.
Three changes:
1. Precedence. An explicit budget (kwargs > config) now outranks the
effort-implied one. The ladder is a derived default; a configured number
is caller intent, and explicit beats derived.
2. A silent-discard guard. When an explicitly requested budget does not reach
`thinking.budget_tokens`, one warning names the key, the value asked for,
the value actually sent, and why -- covering thinking-off, no-thinking
models, always-on models, adaptive mode (where the API forbids
budget_tokens outright), clamping, and a non-integer value. A typo now
warns and falls back instead of raising ValueError out of every request.
3. `extended_thinking` as a config key. This is the fifth configuration's
remedy: before it, a config-only caller could not turn thinking on without
also choosing an effort, so the budget was never read at all on that path.
Opt-in only; `false` is an explicit opt-out that overrides a configured
`reasoning_effort`, mirroring the kwarg one level up.
NON-REGRESSION. Default behaviour is byte-identical: with no explicit budget,
`requested_budget` is None and the chain collapses to exactly what it was.
Verified by stash-comparing the constructed request body across 13
default-path cells (haiku 4.5 / haiku 3.5 / sonnet 4.5 / sonnet 4.6 /
fable 5.1, every effort) against HEAD 6abfcff -- all identical, and the new
guard emits nothing on that path.
Tests: 36 new (14 fail on HEAD 6abfcff), full suite 806 passed.
No API spend -- every assertion is on the constructed request body.
Brian Krabach (bkrabach)
marked this pull request as ready for review
September 2, 2026 22:18
Collaborator
Author
Merge-queue verification — PASS (all 3 gates)Verified in a fresh scratch clone ( Gates
Full suite + lint
Fail-before / pass-afterMethod: checked out parent
VerdictAll three critical gates pass, full suite passes, byte-identity of the default path is independently confirmed (not just trusted from the PR's own pinned test), and the diff is contained to the 4 files the title describes. Approving for merge. |
Collaborator
Author
|
Merge note: normal |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DONE-NOTE —
thinking_budget_tokenssilently inert (model_performance-v81)Lane:
lane/v81-haiku-thinking-budget-inert· base:main@6abfcffSpend: $0.00. No API calls, no DTU, no infrastructure created. Every number below
comes from driving the real provider with a mocked transport and reading the params it
would have put on the wire.
1. The exact inert paths found — which configs, which models
Reproduced at HEAD
6abfcffwithscripts/thinking_budget_reachability.py(added in thisPR, re-runnable against any commit). 12 of 13 cells that set an explicit budget discarded
it with no warning at all. The four d0q reported are the first four rows; the rest are
paths this lane found while pinning the root cause.
budget: 8000+ requesteffort: highbudget: 64000+ requesteffort: highbudget: 8000+ requesteffort: lowbudget: 8000+ configeffort: lowbudget: 8000, no effort anywherebudget: 8000+ configextended_thinking: trueextended_thinkingwas not a config key at allbudget: 8000+effort: highbudget: 8000+effort: highbudget_tokensbudget: 8000+effort: highbudget: 8000+effort: highbudget: 0orbudget: "not-a-number"int(), so never even failed loudlyThe only cell that worked was the per-request
thinking_budget_tokenskwarg.Root cause (
amplifier_module_provider_anthropic/__init__.py, HEAD6abfcff)::3569-3573resolvedkwargs > effort_budget > config > default. The ladder at:3546-3566always sets a non-Noneeffort_budgetwhenever anyreasoning_effortisset, so config was unreachable. And for every effort except
low, that ladder value isliterally
request_caps.default_thinking_budget— the config key was shadowed by the verydefault it exists to override.
:3418gated thinking onbool(kwargs["extended_thinking"]), andextended_thinkingwasnot in
_CONSUMED_CONFIG_KEYS(:678-714) — so a config-only caller could not turnthinking on, and the budget was never read on that path.
:1964default_thinking_budget=32000 if is_45_plus else 0is the pinned value the ladderkept substituting; it is unchanged by this PR.
2. What now reaches the wire
0 of 13 cells are silently discarded. 6 are honored as written; the other 7 cannot be
honored (the API or the model forbids it) and each now emits one targeted warning naming the
key, the value asked for, the value sent, and the remedy.
64000→63999)reasoning_effortandextended_thinkingas remediesextended_thinkingis now a config key)ValueErrorout ofint()Three changes, all in
_build_params:ladder is a derived default; a configured number is caller intent.
explicitly requested budget ≠ what landed on the wire. Fires only when the caller
explicitly asked, so the default path stays silent.
extended_thinkingconfig key — opt-in only.falseis an explicit opt-out thatoverrides a configured
reasoning_effort, mirroring the kwarg one level up.README.mdgains a "Thinking Budget" section and rows forextended_thinking,thinking_budget_tokens,thinking_budget_buffer,thinking_type— none of which theconfig reference documented, despite all four being allow-listed and read.
3. Byte-identity of the default path — ANTHROPIC IS THE DAILY DRIVER
With no
thinking_budget_tokensand noextended_thinkingset anywhere, theconstructed request body is byte-for-byte what HEAD
6abfcffsends. Measured by stashing thesource change, capturing, restoring, and re-capturing with the same harness:
Why it must hold structurally, not just empirically:
requested_budgetisNonewhen nothingis set, so
requested_budget or effort_budget or defaultcollapses to the originaleffort_budget or default;config_extended_thinkingisNonewhen unset, so thereasoning_effortimplication is untouched; and the guard is gated onrequested_budget_source is not None.TestDefaultPathByteIdentitypins all 8 haiku/sonnet cells in-repo with the pre-fix bodiestranscribed verbatim, plus a second parametrised test asserting the guard emits no new log
noise on the default path.
4. Test evidence
6abfcff)and this PR's test file in place: 14 failed, 19 passed. The 19 that passed are the
byte-identity and kwargs-precedence guards — which is the point: the baseline they assert
was captured from the real pre-fix source, not from post-fix behaviour.
tests/test_thinking_budget_config.py36 passed.uv run pytest -q), up from 803 pre-change. No test modified,none skipped, none xfailed.
33687789868): pytest passed on ubuntu / macos /windows x py3.11 / py3.12, plus
license/cla. Worth noting because the CI file itself warnsthat
test_validation.py/test_behavioral.pygo red whenANTHROPIC_API_KEYis absenton a runner -- that did not happen here, so this PR is green on its own merits rather than
green-with-known-failures.
5. Does this affect d0q's "keep 32000, do not ship a budget preset knob"?
The "keep 32000" half: unaffected. The default is untouched —
default_thinking_budgetstill pins 32000 for 4.5+, and §3 shows the default request body is byte-identical.
The "do not ship a budget preset knob" half: the product decision stands, but one of its
supporting facts is now gone. Before this PR, a preset that wanted to move the budget
could not, except through the
extra_request_paramsescape hatch — so "don't ship the knob"was partly a description of reality. After it,
thinking_budget_tokensin provider config isa working, documented, first-class dial. If d0q's recommendation rested on "the knob doesn't
work anyway", that rationale no longer holds and the recommendation should be re-derived on
cost/quality grounds alone.
Those grounds are not settled here, and this lane did not measure them. What is on record is
d0q's own measurement that the dial is real (knob: thinking.budget_tokens · haiku-4-5 ·
measured, n=3/arm, bare single request · d0q capture root): output_tokens medians
2022 / 4411 / 8006 / 9767 across budgets 1024 / 4096 / 32000 / 60000, monotone with
non-overlapping extremes. A dial that moves output tokens ~4.8x across its range is worth a
deliberate decision rather than an accidental one — but that decision is now a decision,
where before it was a defect. Recommend a follow-up item to price a low-budget haiku preset
against quality, rather than treating this PR as authorising one.
6. Deviations, choices made without asking, and what is NOT claimed
("the config value honoured or a warning emitted"). Honouring was chosen because a
key that works is strictly better than a key that explains why it doesn't, and because the
goal statement asked for it explicitly. Warnings still cover every case where honouring is
impossible.
extended_thinking: true. That would silentlyturn thinking on — and therefore raise cost — for anyone who has a stale
thinking_budget_tokensin their settings today. Added an explicitextended_thinkingconfig key instead: opt-in, no behaviour change unless set.
never haiku-specific. It is fixed for every model; haiku is simply where it hurts most,
because
budget_tokensis the only reasoning dial that model has.run against the live API. The only claims are about which bytes leave the process.
untouched — none are in files this PR modifies, and CI runs pytest only.
7. Artifacts
6abfcfftreatment-validation/20260902-v81-haiku-thinking-budget/raw/reachability-before-6abfcff.jsontreatment-validation/20260902-v81-haiku-thinking-budget/raw/reachability-after.jsontreatment-validation/20260902-v81-haiku-thinking-budget/byte-identity.txtscripts/thinking_budget_reachability.py(capture root:
/home/bkrabach/dev/openai-evals-team-ci/.amplifier/evaluation/)