Skip to content

Set max_tokens on non-reasoning Anthropic requests (#816) - #822

Merged
yogthos merged 2 commits into
dirge-code:mainfrom
chrismurrph:fix/816-max-tokens-non-reasoning
Aug 27, 2026
Merged

Set max_tokens on non-reasoning Anthropic requests (#816)#822
yogthos merged 2 commits into
dirge-code:mainfrom
chrismurrph:fix/816-max-tokens-non-reasoning

Conversation

@chrismurrph

Copy link
Copy Markdown
Contributor

Fixes #816.

Problem

With reasoning off, an Anthropic request carries no max_tokens and fails before the HTTP call:

Error: rig stream call failed: RequestError: `max_tokens` must be set for Anthropic

Reproduced 3/3 on v0.25.0 with {"provider_type":"anthropic","model":"claude-opus-5","auth":"claude-code","effort":"off"}, and via /model claude-opus-5 interactively.

Root cause

rig_stream_factory.rs set max_tokens on the per-request builder only from the reasoning-ceiling branch, so with reasoning off nothing set it. rig's Anthropic model then falls back to default_max_tokens_for_model(), which recognizes only claude-opus-4* / claude-sonnet-4* / claude-haiku-4-5* — every Claude 5 id yields None and the request is rejected.

The streaming path never carried the agent's max_tokens: build_agent sets it on the AgentBuilder, but since 0.41 AnyAgentInner stores the model directly rather than a rig Agent, and the per-request builder is constructed independently. rig's per-model default was papering over the gap for recognized ids.

Fix

Thread the resolved cap along the same path reasoning already travels — no factory signature changes — into a small pure helper:

fn request_max_tokens(provider: Option<&str>, opts: &StreamOptions) -> Option<u64> {
    if let Some(level) = opts.reasoning
        && let Some(ceiling) = adapter::max_tokens_for_reasoning(provider, level, opts.thinking_budgets.as_ref())
    { return Some(ceiling); }                       // reasoning ceiling still wins
    let anthropic_shaped = matches!(adapter::reasoning_profile(provider).effort, EffortWire::AnthropicBudget);
    if anthropic_shaped && !turn_reasoning_enabled(provider, opts) {
        return opts.max_tokens;
    }
    None                                            // every other provider unchanged
}

The reasoning ceiling short-circuits first, so the budget_tokens < max_tokens invariant from v0.24.1 is untouched (anthropic_ceiling_clears_every_budget still passes; adapter.rs is not modified). Gating on turn_reasoning_enabled means an OpenAI effort: high turn — which produces no ceiling — is not capped, and gating on EffortWire::AnthropicBudget keeps every non-Anthropic provider byte-identical.

Not lowering existing defaults

A first cut used resolve_max_tokens unconditionally, whose unwrap_or(8192) erased the difference between "user chose 8192" and "nobody said". That would have cut unconfigured claude-opus-4-6 from rig's 128,000 to 8,192 — silent truncation for users who never hit the bug. Instead a value is invented only where rig has none, decided by reading rig's own resolved default off the stored model:

let configured_max_tokens = cli.max_tokens.or(cfg.max_tokens);
let max_tokens = configured_max_tokens
    .or_else(|| agent.anthropic_needs_max_tokens_fallback().then(|| cli.resolve_max_tokens(cfg)));

No model-prefix list is duplicated in dirge, so this tracks rig with no drift. Unconfigured recognized ids keep rig's exact per-model values; an explicitly configured cap is honored (the max_tokens config key and --max-tokens flag reach the streaming path for the first time).

Tests

4 new in provider::tests::max_tokens_816_tests, including a wire-level pin that stands a local capture server up and asserts an unconfigured non-reasoning request for claude-opus-4-6 carries rig's own default (read off the model, not hardcoded) and not 8192. Plus 5 in rig_stream_factory.rs covering the helper across providers and reasoning states, and an extended propagation assertion in stream.rs.

Full suite: 5414 passed, 0 failed, 1 ignored. clippy --all-targets -- -D warnings clean, fmt --check clean.

End-to-end: the failing repro above now completes normally.

Disclosures / known adjacent cases

  • Build, clippy and tests were run with --no-default-features --features no-plugin — the plugin feature's janet toolchain is unavailable on this host (Build fails on Linux #712). The single max_tokens: None, literal added to plugin_hooks_tests.rs is therefore verified only by CI with default features.
  • Escalation/review edge: the value is computed once per agent from the main model. An escalation or review route to a different Anthropic model shares it, so a Claude-5 escalation model behind a recognized-id main model could still hit the original error. Per-model threading would widen the diff considerably — left out of scope deliberately.
  • dispatch.rs one_shot! / btw_query builds an AgentBuilder with no max_tokens, so one-shot side calls on unrecognized Anthropic ids have the same latent failure. Not addressed here.

@yogthos

yogthos commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Thanks, I'll take a look at merging and have this ship in the next release.

With reasoning off (or effort absent) nothing set max_tokens on the
per-request CompletionRequestBuilder in rig_stream_factory: the
reasoning-ceiling branch was the only writer, and rig 0.41 hard-errors
with "`max_tokens` must be set for Anthropic" before the HTTP call
for any model id outside its per-model default table — every Claude 5
id. Every non-reasoning turn on such a model failed.

Thread a per-agent cap along the same path reasoning travels:
build_agent -> AnyAgent.max_tokens -> LoopSpawnConfig -> LoopConfig ->
StreamOptions -> the stream builder. The value is the cap the user
explicitly configured (CLI --max-tokens > config max_tokens); when
nothing is configured, dirge invents its 8192 default ONLY where rig
has no per-model default of its own — read off the model's
default_max_tokens field, not a duplicated model list — so an
unconfigured user on a rig-recognised id (opus-4.x 128k,
sonnet-4/haiku-4.5 64k) keeps rig's larger cap instead of a silent cut
to 8192.

A new request_max_tokens helper decides the request value: a thinking
turn keeps the reasoning ceiling (budget_tokens must stay strictly
below max_tokens — the v0.24.1 invariant
anthropic_ceiling_clears_every_budget pins), a non-reasoning turn on an
Anthropic-shaped provider carries the threaded value, and every other
provider stays unset, byte-identical to before.
@chrismurrph
chrismurrph force-pushed the fix/816-max-tokens-non-reasoning branch from a987943 to ef5c536 Compare August 27, 2026 00:11
@yogthos

yogthos commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Thanks, if you could address the macos failure, happy to merge it in.

The trace test helper selects records by sequence number, and SEQ is a
process-global counter that starts at zero in every process, so the
filter is only sound while a trace file belongs to one process. The
sink filename carried only a SystemTime run stamp read as each process
starts; under a process-per-test runner two processes launched in the
same clock tick read the same stamp, share a file, and then read each
other's records back through overlapping sequence ranges.

That surfaced as a macOS CI failure in which recs[0] was another test's
record: a_tool_call_and_its_result_share_an_id saw kind "tool_end"
where it wrote tool_start, and oversized_payloads_are_bounded_and_marked
saw a short unmarked excerpt. The two failures came from adjacent PIDs.

Adding the PID to the filename makes each file private to its writer.
Test-helper only; no production code changes.
@chrismurrph

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (so it now sits on top of #823) and pushed a second commit that should fix the macOS job.

The macOS failure isn't from this patch — it's a pre-existing race in the trace.rs test helper that this PR's added tests exposed by shifting nextest's scheduling. Evidence from the failing run:

thread '...trace::tests::a_tool_call_and_its_result_share_an_id' (51694) panicked
  left: String("tool_end")   right: "tool_start"

thread '...trace::tests::oversized_payloads_are_bounded_and_marked' (51693) panicked
  a truncated excerpt must say so: xxxxxxxxxxx

In both, recs[0] is another test's record, and the two PIDs are adjacent — two processes running at the same moment, sharing one trace file.

traced() selects records by sequence number, and SEQ is a process-global counter starting at zero in every process, so that filter is only sound while a file belongs to a single process. The sink filename was dirge-trace-test-{test_run_stamp()}.jsonl, and test_run_stamp() is a SystemTime reading taken as each process starts. Under nextest's process-per-test model, two processes launched in the same clock tick read the same stamp, land on the same file, and then read each other's records back through overlapping sequence ranges. LOCK can't help — it only serialises within a process.

Fix is one line plus a comment: add the PID to the filename, making each file private to its writer. Test-helper only, no production code. Two concurrent processes can't share a PID, so the race is closed rather than narrowed.

Two adjacent things I noticed but deliberately did not change, in case you want them handled differently:

  1. ENABLED is never reset to false once a trace test sets it, so tracing stays on process-wide for the rest of that process. Under nextest that's harmless (one test per process), but under a single-process runner any concurrent non-trace test that emits a loop event would write into the shared file inside the sequence window and pollute it. Same class of bug, different trigger — it only bites if the runner model changes.
  2. Nothing cleans up the temp trace files, so a run leaves one per test process behind in temp_dir().

Neither is in scope here; real isolation would mean injecting the sink rather than filtering a shared log, which is a production-code refactor I didn't want to fold into a bug fix.

Local verification of the branch as pushed: full suite 5423 passed / 0 failed / 1 ignored, clippy --all-targets -- -D warnings clean, cargo fmt --check clean — all under --no-default-features --features no-plugin, since the plugin feature needs a janet toolchain this host lacks (#712). CI covers that gap, and build (plugin) and build (all-features) both passed on the previous run.

@yogthos
yogthos merged commit 29d4d57 into dirge-code:main Aug 27, 2026
15 checks passed
@yogthos

yogthos commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Thanks for digging in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

max_tokens not set on CompletionRequestBuilder for non-reasoning Anthropic requests → 400 error

2 participants