Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/agent/agent_loop/h7_smoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ async fn h7_scenario_1_simple_text() {
goal: None,
max_turns: None,
reasoning: None,
max_tokens: None,
bg_store: None,
memory_provider: None,
};
Expand Down Expand Up @@ -434,6 +435,7 @@ async fn h7_scenario_2_turn_boundaries() {
goal: None,
max_turns: None,
reasoning: None,
max_tokens: None,
bg_store: None,
memory_provider: None,
};
Expand Down Expand Up @@ -570,6 +572,7 @@ async fn h7_scenario_5_auth_error_surfaces() {
goal: None,
max_turns: None,
reasoning: None,
max_tokens: None,
bg_store: None,
memory_provider: None,
};
Expand Down Expand Up @@ -759,6 +762,7 @@ async fn h7_scenario_3_tool_dispatch() {
goal: None,
max_turns: None,
reasoning: None,
max_tokens: None,
bg_store: None,
memory_provider: None,
};
Expand Down Expand Up @@ -909,6 +913,7 @@ async fn h7_glm_scenario_1_simple_text() {
goal: None,
max_turns: None,
reasoning: None,
max_tokens: None,
bg_store: None,
memory_provider: None,
};
Expand Down Expand Up @@ -1067,6 +1072,7 @@ async fn h7_glm_scenario_3_tool_dispatch() {
goal: None,
max_turns: None,
reasoning: None,
max_tokens: None,
bg_store: None,
memory_provider: None,
};
Expand Down Expand Up @@ -1177,6 +1183,7 @@ fn cerebras_spawn_config(
goal: None,
max_turns: None,
reasoning: None,
max_tokens: None,
bg_store: None,
memory_provider: None,
}
Expand Down
11 changes: 11 additions & 0 deletions src/agent/agent_loop/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,15 @@ pub struct LoopSpawnConfig {
/// default (`ThinkingLevel::Off`).
pub reasoning: Option<super::types::ThinkingLevel>,

/// GH #816: `max_tokens` to pin on non-reasoning requests — the user's
/// explicitly configured cap, or dirge's default only for Anthropic
/// model ids rig has no per-model default for. Seeded from the agent
/// and forwarded to `LoopConfig.max_tokens` — the field the stream
/// builder reads per turn so non-reasoning Anthropic requests carry the
/// `max_tokens` rig 0.41 requires. `None` leaves requests unset so the
/// provider's own default applies (rig-recognised ids; tests).
pub max_tokens: Option<u64>,

/// dirge-9tfq: per-session background-task store. When `Some`,
/// `spawn_loop_runner` installs a `get_followup_messages` hook
/// that drains the store's pending notifications at every
Expand Down Expand Up @@ -595,6 +604,7 @@ impl LoopSpawnConfig {
goal: None,
max_turns: None,
reasoning: None,
max_tokens: None,
bg_store: None,
memory_provider: None,
}
Expand Down Expand Up @@ -645,6 +655,7 @@ pub fn spawn_loop_runner(cfg: LoopSpawnConfig) -> LoopRunner {
}),
reasoning: cfg.reasoning,
thinking_budgets: None,
max_tokens: cfg.max_tokens,
headers: std::collections::HashMap::new(),
metadata: std::collections::HashMap::new(),
provider_name: cfg.provider_name.clone(),
Expand Down
1 change: 1 addition & 0 deletions src/agent/agent_loop/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ fn build_config() -> LoopConfig {
should_defer_finalization: None,
reasoning: None,
thinking_budgets: None,
max_tokens: None,
headers: std::collections::HashMap::new(),
metadata: std::collections::HashMap::new(),
provider_name: None,
Expand Down
1 change: 1 addition & 0 deletions src/agent/agent_loop/plugin_hooks_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ fn build_config() -> LoopConfig {
should_defer_finalization: None,
reasoning: None,
thinking_budgets: None,
max_tokens: None,
headers: std::collections::HashMap::new(),
metadata: std::collections::HashMap::new(),
provider_name: None,
Expand Down
144 changes: 131 additions & 13 deletions src/agent/agent_loop/rig_stream_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,19 +312,12 @@ where
if let Some(v) = additional {
builder = builder.additional_params(v);
}
// Pin `max_tokens` whenever a thinking BUDGET is on the wire. Anthropic
// counts thinking against max_tokens and rejects the request unless
// budget_tokens is strictly below it — and if we leave max_tokens unset
// rig picks 2048 for any model id it doesn't recognise, which is every
// Claude 5 id. That combination 400s every turn above `minimal`.
if let Some(level) = opts.reasoning
&& let Some(ceiling) = crate::provider::adapter::max_tokens_for_reasoning(
provider,
level,
opts.thinking_budgets.as_ref(),
)
{
builder = builder.max_tokens(ceiling);
// Pin `max_tokens` when this request needs one — the reasoning
// ceiling on thinking turns, the resolved `max_tokens` config on
// non-reasoning Anthropic turns (GH #816). The rules live in
// `request_max_tokens`.
if let Some(max_tokens) = request_max_tokens(provider, &opts) {
builder = builder.max_tokens(max_tokens);
}
let request = builder.build();

Expand Down Expand Up @@ -1170,6 +1163,47 @@ pub fn turn_reasoning_enabled(
reasoning_params(provider_name, opts).is_some()
}

/// The `max_tokens` to pin on this request, or `None` to leave the field
/// unset (the provider's own default applies).
///
/// Two independent reasons to set one:
/// - A turn that puts a thinking BUDGET on the wire carries the reasoning
/// ceiling. Anthropic counts thinking against `max_tokens` and rejects
/// the request unless `budget_tokens` is strictly below it — and if we
/// leave `max_tokens` unset rig picks 2048 for any model id it doesn't
/// recognise, which is every Claude 5 id. That combination 400s every
/// turn above `minimal`. The ceiling always wins over the configured
/// value: a user-configured 8192 must never undercut a 16384+ budget.
/// - A NON-reasoning turn on an Anthropic-shaped provider carries the
/// resolved `max_tokens` config (GH #816). rig 0.41 hard-errors with
/// "`max_tokens` must be set for Anthropic" when the request has none
/// and the model id is one it has no default for — again every Claude 5
/// id — so with reasoning off every request failed before the HTTP call.
///
/// Every other case stays unset, byte-identical to before: effort-string
/// providers send no budget, their backends accept an absent `max_tokens`,
/// and forcing a cap on a reasoning turn there could strangle reasoning
/// output (OpenAI counts reasoning tokens against the output limit).
fn request_max_tokens(provider: Option<&str>, opts: &super::stream::StreamOptions) -> Option<u64> {
if let Some(level) = opts.reasoning
&& let Some(ceiling) = crate::provider::adapter::max_tokens_for_reasoning(
provider,
level,
opts.thinking_budgets.as_ref(),
)
{
return Some(ceiling);
}
let anthropic_shaped = matches!(
crate::provider::adapter::reasoning_profile(provider).effort,
crate::provider::adapter::EffortWire::AnthropicBudget
);
if anthropic_shaped && !turn_reasoning_enabled(provider, opts) {
return opts.max_tokens;
}
None
}

/// Say so, once per process, when a request-override knob is set but has no
/// consumer (dirge-vpma.25).
///
Expand Down Expand Up @@ -2336,6 +2370,90 @@ mod tests {
o
}

// ============================================================
// request_max_tokens (GH #816)
// ============================================================

/// GH #816: with reasoning off, an Anthropic request must still carry
/// the resolved `max_tokens` — rig 0.41 hard-errors ("`max_tokens` must
/// be set for Anthropic") before the HTTP call when the field is unset
/// and the model id is one it has no default for (every Claude 5 id).
#[test]
fn non_reasoning_anthropic_request_carries_configured_max_tokens() {
let mut o = StreamOptions::from_signal(AbortSignal::new());
o.max_tokens = Some(8192);
assert_eq!(request_max_tokens(Some("anthropic"), &o), Some(8192));
}

/// `Some(Off)` puts no thinking params on the wire, so it is a
/// non-reasoning turn and gets the configured value too.
#[test]
fn reasoning_off_level_anthropic_request_carries_configured_max_tokens() {
let mut o = opts_with_reasoning(ThinkingLevel::Off);
o.max_tokens = Some(8192);
assert_eq!(request_max_tokens(Some("anthropic"), &o), Some(8192));
}

/// A reasoning turn keeps the budget ceiling, NOT the configured value:
/// Anthropic requires `budget_tokens` strictly below `max_tokens`, so a
/// configured 8192 must never undercut a level's budget + headroom
/// (the invariant `anthropic_ceiling_clears_every_budget` pins).
#[test]
fn reasoning_anthropic_request_keeps_ceiling_over_configured_value() {
let mut o = opts_with_reasoning(ThinkingLevel::High);
o.max_tokens = Some(8192);
let expected = crate::provider::adapter::max_tokens_for_reasoning(
Some("anthropic"),
ThinkingLevel::High,
None,
)
.expect("high puts a budget on the wire");
assert_eq!(request_max_tokens(Some("anthropic"), &o), Some(expected));
assert!(
expected > 8192,
"the ceiling must clear the configured value for this test to bite",
);
}

/// No resolved config (tests, paths built without one) leaves the
/// request unset — byte-identical to the pre-fix behaviour.
#[test]
fn non_reasoning_anthropic_request_without_config_stays_unset() {
let o = StreamOptions::from_signal(AbortSignal::new());
assert_eq!(request_max_tokens(Some("anthropic"), &o), None);
}

/// Effort-string providers never had a cap forced on them and still
/// don't — their backends accept an absent `max_tokens`, and capping an
/// OpenAI reasoning turn could strangle output (reasoning tokens count
/// against the output limit there).
#[test]
fn other_providers_stay_uncapped_with_and_without_reasoning() {
for provider in [
"openai",
"deepseek",
"glm",
"cerebras",
"custom",
"openrouter",
] {
let mut off = StreamOptions::from_signal(AbortSignal::new());
off.max_tokens = Some(8192);
assert_eq!(
request_max_tokens(Some(provider), &off),
None,
"{provider}: non-reasoning turn must stay uncapped",
);
let mut on = opts_with_reasoning(ThinkingLevel::High);
on.max_tokens = Some(8192);
assert_eq!(
request_max_tokens(Some(provider), &on),
None,
"{provider}: reasoning turn must stay uncapped",
);
}
}

/// Anthropic gets `thinking: { type: "enabled", budget_tokens
/// }`. Verifies the budget defaults are sane for each level.
#[test]
Expand Down
1 change: 1 addition & 0 deletions src/agent/agent_loop/run_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ fn build_config() -> LoopConfig {
should_defer_finalization: None,
reasoning: None,
thinking_budgets: None,
max_tokens: None,
headers: std::collections::HashMap::new(),
metadata: std::collections::HashMap::new(),
provider_name: None,
Expand Down
1 change: 1 addition & 0 deletions src/agent/agent_loop/steering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ mod tests {
should_defer_finalization: None,
reasoning: None,
thinking_budgets: None,
max_tokens: None,
headers: std::collections::HashMap::new(),
metadata: std::collections::HashMap::new(),
provider_name: None,
Expand Down
11 changes: 11 additions & 0 deletions src/agent/agent_loop/stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,13 @@ pub struct StreamOptions {
pub api_key: Option<String>,
pub reasoning: Option<super::types::ThinkingLevel>,
pub thinking_budgets: Option<super::types::ThinkingBudgets>,
/// GH #816: `max_tokens` to pin on non-reasoning requests — the user's
/// explicitly configured cap, or dirge's default only for Anthropic
/// model ids rig has no per-model default for. Applied on providers
/// that require the field (Anthropic). `None` leaves the request field
/// unset so the provider's own (often larger) per-model default
/// applies — kept by tests and by any path with nothing configured.
pub max_tokens: Option<u64>,
pub headers: std::collections::HashMap<String, String>,
pub metadata: std::collections::HashMap<String, serde_json::Value>,
/// dirge-e31n.6: per-request tool gating. `None` sends nothing and leaves
Expand All @@ -116,6 +123,7 @@ impl StreamOptions {
api_key: None,
reasoning: None,
thinking_budgets: None,
max_tokens: None,
headers: std::collections::HashMap::new(),
metadata: std::collections::HashMap::new(),
tool_choice: None,
Expand Down Expand Up @@ -204,6 +212,7 @@ pub async fn stream_assistant_response(
api_key: resolved_api_key,
reasoning: config.reasoning,
thinking_budgets: config.thinking_budgets.clone(),
max_tokens: config.max_tokens,
headers: config.headers.clone(),
metadata: config.metadata.clone(),
tool_choice,
Expand Down Expand Up @@ -581,6 +590,7 @@ mod tests {
high: Some(8192),
..Default::default()
});
config.max_tokens = Some(4096);
config
.headers
.insert("X-Test".to_string(), "yes".to_string());
Expand All @@ -605,6 +615,7 @@ mod tests {
opts.thinking_budgets.as_ref().and_then(|b| b.high),
Some(8192)
);
assert_eq!(opts.max_tokens, Some(4096));
assert_eq!(opts.headers.get("X-Test").map(String::as_str), Some("yes"));
assert_eq!(
opts.metadata.get("user_id"),
Expand Down
1 change: 1 addition & 0 deletions src/agent/agent_loop/tools_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ fn build_config() -> LoopConfig {
should_defer_finalization: None,
reasoning: None,
thinking_budgets: None,
max_tokens: None,
headers: std::collections::HashMap::new(),
metadata: std::collections::HashMap::new(),
provider_name: None,
Expand Down
16 changes: 14 additions & 2 deletions src/agent/agent_loop/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,13 +472,25 @@ mod tests {

/// The one path every test in this module shares, since the sink is set
/// once per process.
///
/// The name carries the PID as well as the run stamp. `traced` 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
/// file belongs to one process. Under a process-per-test runner the run
/// stamp alone is not enough to guarantee that: it is a `SystemTime`
/// reading taken as each process starts, and two processes launched in the
/// same clock tick read the same value, land on the same file, and then
/// read each other's records back through overlapping sequence ranges.
/// That surfaced as a macOS CI failure where `recs[0]` was another test's
/// record. The PID makes the file private to the process that writes it.
fn sink_path() -> std::path::PathBuf {
static FIRST: OnceLock<std::path::PathBuf> = OnceLock::new();
FIRST
.get_or_init(|| {
std::env::temp_dir().join(format!(
"dirge-trace-test-{}.jsonl",
crate::text::test_run_stamp()
"dirge-trace-test-{}-{}.jsonl",
crate::text::test_run_stamp(),
std::process::id()
))
})
.clone()
Expand Down
Loading
Loading