From 85f62f83376779d0a98b2a0a1f33dbd9c6d4f837 Mon Sep 17 00:00:00 2001 From: asto Date: Tue, 10 Nov 2026 18:04:07 +0800 Subject: [PATCH] fix(engine): share the route output budget composition and pin its inputs The transport clamp and the host-facing route_effective_max_output_tokens held two parallel copies of normalize-then-intersect. Both now delegate to a single route_budget helper so they cannot drift from each other. The wrapper test gains a Some(RouteLimits) control (the doc's primary instructed input) and a normalization-sensitive pin (OpenRouter kimi-code: 32,768 canonicalized vs 8,192 raw uncatalogued), and states what the DeepSeek alias leg actually pins. The wrapper doc records the window-clamp arithmetic, the dispatched-base_url contract, and the full rename set; #[must_use] aligns the wrapper with the layers beneath it. Signed-off-by: asto --- crates/tui/src/client.rs | 10 ++-- crates/tui/src/core/engine/context.rs | 68 ++++++++++++++++++++++----- crates/tui/src/route_budget.rs | 20 ++++++++ 3 files changed, 82 insertions(+), 16 deletions(-) diff --git a/crates/tui/src/client.rs b/crates/tui/src/client.rs index 4bdb1e64e6..3177cbc6f1 100644 --- a/crates/tui/src/client.rs +++ b/crates/tui/src/client.rs @@ -2341,11 +2341,13 @@ impl DeepSeekClient { requested_model: &str, route_limits: Option, ) -> u32 { - let wire_model = - wire_model_for_provider_route(self.api_provider, &self.base_url, requested_model); - crate::route_budget::effective_max_output_tokens_for_route( + // Delegate to the shared normalize-then-intersect copy in + // `route_budget`; the host-facing engine wrapper calls the same + // helper, so this clamp cannot drift from what hosts compute. + crate::route_budget::route_effective_max_output_tokens( self.api_provider, - &wire_model, + &self.base_url, + requested_model, route_limits, ) } diff --git a/crates/tui/src/core/engine/context.rs b/crates/tui/src/core/engine/context.rs index 07a0f1e9e6..7dd7aff3f3 100644 --- a/crates/tui/src/core/engine/context.rs +++ b/crates/tui/src/core/engine/context.rs @@ -957,26 +957,36 @@ pub(super) fn context_input_budget_for_provider( /// request cap after explicit overrides, the documented ceiling intersected with /// the automatic per-request output cap, compatibility/route ceilings, and the /// route window are intersected (a concrete route cap replaces only the generic -/// uncatalogued guess) — instead of hard-coding a constant and silently -/// drifting from the engine. The wire model is normalized exactly as -/// the transport normalizes it (`wire_model_for_provider_route`). +/// uncatalogued guess; the window rung clamps to `window − 2_048`, the input +/// floor plus headroom reserve, and the result is floored at 1) — instead of +/// hard-coding a constant and silently drifting from the engine. The wire model +/// is normalized exactly as the transport normalizes it +/// (`wire_model_for_provider_route`); the transport clamp delegates to the same +/// `route_budget` copy, so the two callers cannot drift. /// /// Pass the same effective [`RouteLimits`] the engine session resolved for the /// route (host-declared facts included); `None` falls back to catalogue -/// defaults and diverges wherever the route carries declared facts. Ambient +/// defaults and diverges wherever the route carries declared facts. Pass the +/// same `base_url` the request will actually dispatch on — custom endpoints +/// own their model-id namespace and skip normalization entirely. Ambient /// `CODEWHALE_MAX_OUTPUT_TOKENS` / `DEEPSEEK_MAX_OUTPUT_TOKENS` overrides move /// this value exactly as they move the main turn's. Per-dialect field shape -/// (the Codex Responses route omits the field; some OpenAI reasoning routes -/// rename it `max_completion_tokens`) remains transport knowledge. +/// (the Codex Responses route omits the field; Xiaomi MiMo, OpenAI reasoning +/// families, MiniMax M3, and the direct Moonshot K3 route rename it +/// `max_completion_tokens`) remains transport knowledge. +#[must_use] pub fn route_effective_max_output_tokens( provider: ApiProvider, base_url: &str, requested_model: &str, route_limits: Option, ) -> u32 { - let wire_model = - crate::config::wire_model_for_provider_route(provider, base_url, requested_model); - crate::route_budget::effective_max_output_tokens_for_route(provider, &wire_model, route_limits) + crate::route_budget::route_effective_max_output_tokens( + provider, + base_url, + requested_model, + route_limits, + ) } /// Public so external callers (e.g. a host/bridge deriving its own compaction @@ -1050,7 +1060,7 @@ pub(super) fn is_image_input_rejection_message(message: &str) -> bool { #[cfg(test)] mod tests { - use super::{emergency_trim_budget, is_image_input_rejection_message}; + use super::{RouteLimits, emergency_trim_budget, is_image_input_rejection_message}; #[test] fn emergency_trim_budget_leaves_a_hysteresis_margin() { @@ -1089,8 +1099,11 @@ mod tests { None, ) ); - // A compact DeepSeek alias must reach the same cap as its canonical - // spelling: hosts may hold either form. + // Budget equality across these two spellings is guaranteed below + // normalization too (`provider_capability` recognizes the compact and + // retired alias forms directly), so this leg pins recognition + // redundancy, not normalization — the load-bearing normalization guard + // is the OpenRouter leg below. let deepseek = crate::config::ApiProvider::Deepseek; let deepseek_url = "https://api.deepseek.com/v1"; assert_eq!( @@ -1107,6 +1120,37 @@ mod tests { None, ) ); + // The one place wire normalization is budget-load-bearing on today's + // catalogue: the short Kimi alias only reaches its catalogued 32,768 + // ceiling after canonicalization to `moonshotai/kimi-k2.7-code`; raw, + // it is uncatalogued and fails closed at the 8,192 compatibility + // ceiling. Deleting the wrapper's normalization call turns this + // assertion red. + let openrouter = crate::config::ApiProvider::Openrouter; + let openrouter_url = "https://openrouter.ai/api/v1"; + assert_eq!( + super::route_effective_max_output_tokens(openrouter, openrouter_url, "kimi-code", None), + 32_768 + ); + // Route-declared facts ride through untouched: a `Some(output_tokens)` + // ceiling clamps the catalogue cap exactly as the main turn's + // `active_route_limits` do (the doc's primary instructed input), and + // the composition stays identical with limits attached. + let limits = RouteLimits { + output_tokens: Some(1_000), + ..RouteLimits::default() + }; + let via_limits = + super::route_effective_max_output_tokens(provider, base_url, "kimi-k3", Some(limits)); + assert_eq!(via_limits, 1_000); + assert_eq!( + via_limits, + crate::route_budget::effective_max_output_tokens_for_route( + provider, + &crate::config::wire_model_for_provider_route(provider, base_url, "kimi-k3"), + Some(limits), + ) + ); } #[test] diff --git a/crates/tui/src/route_budget.rs b/crates/tui/src/route_budget.rs index 30b3e0b0b1..e6635d5c30 100644 --- a/crates/tui/src/route_budget.rs +++ b/crates/tui/src/route_budget.rs @@ -286,6 +286,26 @@ pub(crate) fn output_ceiling_source(provider: ApiProvider, model: &str) -> Outpu ) } +/// Effective request output cap for a `(provider, base_url, requested model)` +/// route: normalize the requested id exactly as the transport does +/// (`wire_model_for_provider_route`), then apply +/// [`effective_max_output_tokens_for_route`]. The transport clamp +/// (`client.rs`) and the host-facing +/// `core::engine::route_effective_max_output_tokens` both delegate here, so +/// the normalize-then-intersect composition has a single shared copy and the +/// two callers cannot silently drift. +#[must_use] +pub(crate) fn route_effective_max_output_tokens( + provider: ApiProvider, + base_url: &str, + requested_model: &str, + route_limits: Option, +) -> u32 { + let wire_model = + crate::config::wire_model_for_provider_route(provider, base_url, requested_model); + effective_max_output_tokens_for_route(provider, &wire_model, route_limits) +} + /// Effective request output cap for a fully resolved provider/model route. #[must_use] pub(crate) fn effective_max_output_tokens_for_route(