From 333d1bb282ffdaefbf99a227ec7d6c5c53e7a41d Mon Sep 17 00:00:00 2001 From: oxoxDev <164490987+oxoxDev@users.noreply.github.com> Date: Tue, 23 Jun 2026 00:46:24 +0530 Subject: [PATCH] fix(agent): actionable error when system prompt exceeds local model context (TAURI-RUST-6V0) (#3771) Co-authored-by: Steven Enamakel --- src/core/observability.rs | 35 +++ src/openhuman/agent/harness/engine/core.rs | 85 +++++++- src/openhuman/agent/harness/mod.rs | 2 +- .../agent/harness/session/turn/core.rs | 18 ++ src/openhuman/agent/harness/token_budget.rs | 206 +++++++++++++++++- src/openhuman/inference/model_context.rs | 54 +++++ .../provider/compatible_provider_impl.rs | 18 ++ .../inference/provider/ops/http_error.rs | 15 ++ src/openhuman/inference/provider/ops_tests.rs | 25 +++ src/openhuman/inference/provider/reliable.rs | 39 ++++ src/openhuman/inference/provider/router.rs | 47 ++++ .../inference/provider/router_tests.rs | 98 +++++++++ src/openhuman/inference/provider/traits.rs | 47 ++++ 13 files changed, 686 insertions(+), 3 deletions(-) diff --git a/src/core/observability.rs b/src/core/observability.rs index 8c3fdf4be..6c3c7d6b0 100644 --- a/src/core/observability.rs +++ b/src/core/observability.rs @@ -2849,6 +2849,41 @@ mod tests { } } + #[test] + fn classifies_lmstudio_n_keep_exceeds_n_ctx_rereport() { + // TAURI-RUST-6V0: the verbatim LM Studio 400 body — the un-evictable + // prefix (`n_keep`) is larger than the model's loaded context + // (`n_ctx`). When this 400 slips past the pre-dispatch guard and is + // re-raised by the agent/web_channel, `report_error_or_expected` must + // classify it as expected user-state so it stays out of Sentry. + assert_eq!( + expected_error_kind( + "lmstudio API error (400 Bad Request): {\"error\":\"The number of tokens to keep from the initial prompt is greater than the context length (n_keep: 10978 >= n_ctx: 8192). Try to load the model with a larger context length, or provide a shorter input.\"}" + ), + Some(ExpectedErrorKind::ContextWindowExceeded) + ); + } + + #[test] + fn context_prefix_too_large_error_display_classifies_as_expected() { + // S3.5.d coupling test: the pre-dispatch actionable error's Display + // string MUST classify as the suppressed ContextWindowExceeded bucket, + // so a wording drift in the user-facing message (which is what gets + // re-raised and re-reported up the stack) fails CI instead of silently + // leaking the event to Sentry. + let err = crate::openhuman::agent::harness::token_budget::ContextPrefixTooLargeError { + prefix_tokens: 10_978, + context_window: 8_192, + max_input_tokens: 7_372, + }; + assert_eq!( + expected_error_kind(&err.to_string()), + Some(ExpectedErrorKind::ContextWindowExceeded), + "ContextPrefixTooLargeError Display must stay coupled to the \ + context-window-exceeded classifier (drift would leak Sentry events)" + ); + } + #[test] fn does_not_classify_unrelated_messages_as_context_window_exceeded() { // Anchors are context-overflow specific. A generic "window" or diff --git a/src/openhuman/agent/harness/engine/core.rs b/src/openhuman/agent/harness/engine/core.rs index 6d8c5ea16..0309ef077 100644 --- a/src/openhuman/agent/harness/engine/core.rs +++ b/src/openhuman/agent/harness/engine/core.rs @@ -115,6 +115,14 @@ pub(crate) async fn run_turn_engine( // LM Studio) report their *runtime-loaded* window here, which can be far // smaller than the model's trained maximum in the static table — trimming // to the max would overflow the loaded `n_ctx` (#3550 / TAURI-RUST-6V0). + // + // For local providers this is now always `Some` (a conservative floor backs + // up any missing profile default — see + // `context_window_for_model_with_local_fallback`), so trimming always + // engages for them. `None` therefore means a *cloud* provider with an + // unknown model: those windows are large, so skipping the pre-dispatch trim + // is the correct conservative choice (a tiny floor would needlessly truncate + // a legitimate large-context request). let effective_context_window = provider.effective_context_window(model).await; match effective_context_window { Some(context_window) => tracing::debug!( @@ -126,9 +134,34 @@ pub(crate) async fn run_turn_engine( None => tracing::debug!( provider = provider_name, model, - "[agent_loop] effective context window unavailable; pre-dispatch trimming disabled this turn" + "[agent_loop] effective context window unavailable (cloud unknown model); pre-dispatch trimming skipped this turn" ), } + // Model-aware locality: a router whose *default* is cloud may still route + // THIS model to a local provider, so gate the pre-dispatch un-evictable + // prefix abort on the routed provider, not the default (#3550 / + // TAURI-RUST-6V0; PR #3771 review). + let model_is_local = provider.is_local_provider_for_model(model); + // Authoritative runtime-loaded window for the hard abort. `effective_context + // _window` above may be a *guess* (profile default / conservative floor) for + // a local model that exposes no loaded window — safe to TRIM against, but + // aborting with "reload with a larger context length" against a guess would + // wrongly reject a request the real loaded window would accept. So the abort + // only consults the genuinely-reported window (e.g. LM Studio's loaded + // n_ctx); `None` ⇒ window unknown ⇒ no hard abort, trimming still runs. + let loaded_context_window = if model_is_local { + provider.loaded_context_window(model).await + } else { + None + }; + if let Some(loaded) = loaded_context_window { + tracing::debug!( + provider = provider_name, + model, + loaded_context_window = loaded, + "[agent_loop] authoritative loaded context window resolved (pre-dispatch prefix abort armed)" + ); + } let mut context_guard = effective_context_window .map(ContextGuard::with_context_window) .unwrap_or_else(ContextGuard::new); @@ -425,6 +458,56 @@ pub(crate) async fn run_turn_engine( } } + // Pre-dispatch guard for the un-evictable-prefix overflow + // (TAURI-RUST-6V0 / #3550). Trimming above can only drop conversation + // history — never the system prefix or the current user turn. When a + // *local* model is loaded with a context window smaller than that + // un-evictable prefix (the runtime `n_keep >= n_ctx`), no amount of + // trimming can fit the prompt, so dispatching guarantees an opaque + // upstream `400`. Detect it here and surface the remedy the user + // actually controls — reload the model with a larger context length — + // instead of letting the cryptic provider error fly. + // + // Gated on `loaded_context_window`, the model's **authoritative** + // runtime window — `Some` only for a routed-local model whose runtime + // actually reports its loaded `n_ctx` (e.g. LM Studio). A guessed window + // (profile default / conservative floor) is deliberately NOT used here: + // it is safe to trim against (over-trim just costs reply room) but must + // not drive a hard "reload with a larger context length" abort, which + // would wrongly reject a request the real loaded window would accept + // (Codex P1 review on PR #3771). This is an expected user-state + // condition (S3.5: preventable-user-state), so it is demoted from Sentry + // via `report_error_or_expected` (its Display string matches + // `is_context_window_exceeded_message`). + if let Some(loaded) = loaded_context_window { + if let Some(prefix_err) = + crate::openhuman::agent::harness::token_budget::unevictable_prefix_overflow( + &prepared_messages_vec, + loaded, + ) + { + log::warn!( + "[agent_loop] un-evictable prefix overflows local context window — aborting pre-dispatch model={} loaded_context_window={} prefix_tokens={} max_input_tokens={}", + model, + loaded, + prefix_err.prefix_tokens, + prefix_err.max_input_tokens + ); + crate::core::observability::report_error_or_expected( + &prefix_err, + "agent", + "context_prefix_too_large", + &[ + ("provider", provider_name), + ("model", model), + ("context_window", &loaded.to_string()), + ("prefix_tokens", &prefix_err.prefix_tokens.to_string()), + ], + ); + return Err(prefix_err.into()); + } + } + // Recomputed each iteration: a `ToolSource` may register tools lazily // mid-turn, so native-tool enablement can flip from off to on. let request_tools = if provider.supports_native_tools() && !tools.request_specs().is_empty() diff --git a/src/openhuman/agent/harness/mod.rs b/src/openhuman/agent/harness/mod.rs index 87ed9ade3..8385998da 100644 --- a/src/openhuman/agent/harness/mod.rs +++ b/src/openhuman/agent/harness/mod.rs @@ -42,7 +42,7 @@ pub(crate) mod session_queue; pub(crate) mod spawn_depth_context; pub mod subagent_runner; pub mod task_recency_context; -mod token_budget; +pub(crate) mod token_budget; pub(crate) mod tool_filter; mod tool_loop; pub(crate) mod tool_result_artifacts; diff --git a/src/openhuman/agent/harness/session/turn/core.rs b/src/openhuman/agent/harness/session/turn/core.rs index 2a8b5b4ea..31677a955 100644 --- a/src/openhuman/agent/harness/session/turn/core.rs +++ b/src/openhuman/agent/harness/session/turn/core.rs @@ -518,10 +518,28 @@ impl Agent { // local providers (LM Studio) trim to their runtime-loaded n_ctx // rather than the trained-max table (#3550 / TAURI-RUST-6V0). // Must run before `agent: self` takes the &mut borrow below. + // + // For local providers this is always `Some` (a conservative floor + // backs up any missing profile default), so trimming always engages. + // `None` means a cloud provider with an unknown model — trimming is + // intentionally skipped there (large window; over-trimming is worse). let turn_context_window = self .provider .effective_context_window(&effective_model) .await; + match turn_context_window { + Some(context_window) => tracing::debug!( + provider = %provider_name, + model = %effective_model, + context_window, + "[agent_loop] effective context window resolved for turn" + ), + None => tracing::debug!( + provider = %provider_name, + model = %effective_model, + "[agent_loop] effective context window unavailable (cloud unknown model); pre-dispatch trimming skipped this turn" + ), + } let mut observer = AgentObserver { agent: self, artifact_store, diff --git a/src/openhuman/agent/harness/token_budget.rs b/src/openhuman/agent/harness/token_budget.rs index 754e81a27..5c2b9f010 100644 --- a/src/openhuman/agent/harness/token_budget.rs +++ b/src/openhuman/agent/harness/token_budget.rs @@ -114,10 +114,84 @@ fn output_reserve_tokens(context_window: u64) -> u64 { .min(DEFAULT_OUTPUT_RESERVE_TOKENS.max(context_window / 4)) } -fn max_input_tokens(context_window: u64) -> u64 { +/// Token budget available for the *input* prompt after reserving room for the +/// model's completion + overhead. Public so the agent engine can detect the +/// un-evictable-prefix overflow (see [`unevictable_prefix_overflow`]). +pub fn max_input_tokens(context_window: u64) -> u64 { context_window.saturating_sub(output_reserve_tokens(context_window)) } +/// Estimated token count of the prompt's **un-evictable prefix** — the +/// messages [`trim_chat_messages_to_budget`] can never drop: every `system` +/// message plus the single newest non-system message (the current user turn). +/// These are exactly the messages a maximal trim leaves behind, so this is the +/// floor below which trimming cannot take the prompt. +fn unevictable_prefix_tokens(messages: &[ChatMessage]) -> usize { + let newest_non_system = messages.iter().rposition(|m| m.role != "system"); + messages + .iter() + .enumerate() + .filter(|(idx, m)| m.role == "system" || Some(*idx) == newest_non_system) + .map(|(_, m)| estimate_chat_message_tokens(m)) + .sum() +} + +/// The actionable, user-facing condition behind Sentry TAURI-RUST-6V0: a local +/// model was loaded with a context window (`context_window`, the runtime +/// `n_ctx`) smaller than the assistant's un-evictable system/prompt prefix +/// (`prefix_tokens`, the runtime `n_keep`). Trimming can evict conversation +/// history but never the system prefix or the current turn, so it can never get +/// the prompt under budget — the request is doomed to the opaque upstream +/// `400 (n_keep >= n_ctx)`. We detect it pre-dispatch and surface the remedy the +/// user actually controls: reload the model with a larger context length. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +#[error( + "Your local model's context length ({context_window} tokens) is smaller than \ + the assistant's required system prompt (~{prefix_tokens} tokens), so the \ + prompt can't be trimmed to fit (n_keep >= n_ctx). Reload the model in your \ + local server (e.g. LM Studio) with a larger context length, then try again." +)] +pub struct ContextPrefixTooLargeError { + /// Tokens in the un-evictable prefix (system + current turn) — the `n_keep`. + pub prefix_tokens: usize, + /// The model's effective (runtime-loaded) context window — the `n_ctx`. + pub context_window: u64, + /// The derived input budget the prefix must fit within. + pub max_input_tokens: u64, +} + +/// Detect the [`ContextPrefixTooLargeError`] condition: the un-evictable prefix +/// alone overflows the model's runtime context window, so even a maximal trim +/// (which can only drop evictable history) can never get the prompt to fit. +/// Returns `Some(err)` only when the prompt can *never* fit — i.e. the prefix is +/// at least as large as the whole `context_window` (`n_keep >= n_ctx`), the exact +/// condition the runtime rejects — so the caller surfaces the actionable error +/// instead of dispatching. `None` when trimming can keep the prompt within +/// budget. +/// +/// The predicate is anchored on `context_window` (the runtime `n_ctx`), **not** +/// `max_input_tokens` (which subtracts a conservative reply reserve). Comparing +/// against `max_input_tokens` would abort valid requests whose prefix fits inside +/// the context window but exceeds the reply reserve — those are over-trim +/// candidates (the reply just gets less room), not the un-fixable +/// `n_keep >= n_ctx` failure this actionable error is meant to report (#3550 / +/// Sentry TAURI-RUST-6V0; CodeRabbit review on PR #3771). +pub fn unevictable_prefix_overflow( + messages: &[ChatMessage], + context_window: u64, +) -> Option { + let max_input = max_input_tokens(context_window); + let prefix_tokens = unevictable_prefix_tokens(messages); + // `n_keep >= n_ctx`: the un-evictable prefix is at least the whole runtime + // window. This is the precise condition the local runtime rejects, and the + // only one no trim can rescue. + (prefix_tokens as u64 >= context_window).then_some(ContextPrefixTooLargeError { + prefix_tokens, + context_window, + max_input_tokens: max_input, + }) +} + /// Trim `messages` oldest-first (never removing `system` role) until the /// estimated prompt fits `context_window`. pub fn trim_chat_messages_to_budget( @@ -316,6 +390,136 @@ mod tests { ); } + #[test] + fn conservative_local_floor_bounds_oversized_history() { + // TAURI-RUST-6V0 / #3550: when a local provider resolves a conservative + // fallback window (instead of `None`, which would skip trimming and let + // the prompt overflow the runtime `n_ctx` → a hard 400), trimming MUST + // still engage and leave the prompt BOUNDED — not unbounded. Mirrors the + // 4096-token floor returned by + // `context_window_for_model_with_local_fallback` for a local provider + // with no profile default. + let floor: u64 = 4_096; + let mut messages = vec![ + ChatMessage::system("system prompt"), + user_msg(&"x".repeat(400_000)), // ~100k tokens, far over the floor + user_msg("newest"), + ]; + let outcome = trim_chat_messages_to_budget(&mut messages, floor); + assert!(outcome.trimmed, "oversized history must be trimmed"); + // The retained prompt must fit the input budget derived from the floor — + // i.e. it is bounded, never left at the original ~100k tokens. + let max_input = max_input_tokens(floor) as usize; + assert!( + outcome.final_tokens <= max_input, + "trimmed prompt ({} tokens) must fit the conservative floor budget ({max_input})", + outcome.final_tokens + ); + assert!(outcome.final_tokens < outcome.original_tokens); + // The newest user turn survives. + assert!(messages.iter().any(|m| m.content == "newest")); + } + + #[test] + fn unevictable_prefix_overflow_fires_when_system_prefix_exceeds_local_ctx() { + // TAURI-RUST-6V0: a large system prompt (the un-evictable `n_keep`) + // alone exceeds a small local context window (`n_ctx`). Trimming can + // never help — it never drops the system message — so the pre-dispatch + // guard must fire with the numbers needed for the actionable error. + let ctx: u64 = 8_192; // LM Studio loaded n_ctx from the live events + let mut messages = vec![ + // ~11k-token system prefix (the reported n_keep ≈ 10978). + ChatMessage::system(&"s".repeat(44_000)), + user_msg("hi"), + ]; + // Even a maximal trim leaves the prompt over budget. + let outcome = trim_chat_messages_to_budget(&mut messages, ctx); + assert!( + outcome.final_tokens as u64 > max_input_tokens(ctx), + "precondition: trim cannot fit the prompt" + ); + + let overflow = unevictable_prefix_overflow(&messages, ctx) + .expect("un-evictable prefix overflow must be detected"); + assert_eq!(overflow.context_window, ctx); + assert!( + overflow.prefix_tokens as u64 > overflow.max_input_tokens, + "prefix must exceed the input budget" + ); + // The user-facing remedy + the diagnostic anchor are both present. + let msg = overflow.to_string(); + assert!( + msg.contains("larger context length"), + "must name the remedy" + ); + assert!( + msg.contains("n_keep >= n_ctx"), + "must carry the diagnostic anchor" + ); + } + + #[test] + fn unevictable_prefix_overflow_anchors_on_context_window_not_reply_reserve() { + // CodeRabbit boundary case (PR #3771): a prefix that lands in the band + // `max_input_tokens < prefix < context_window` fits the runtime `n_ctx` + // — the reply just gets less room — so it MUST NOT be aborted as the + // un-fixable `n_keep >= n_ctx` failure. Anchoring on `max_input_tokens` + // (the old `>` predicate) would have wrongly aborted it. + let ctx: u64 = 8_192; + // output_reserve = max(819, min(8192/4=2048, 8192/10=819)) = 819, so + // max_input = 8192 - 819 = 7373. + let max_input = max_input_tokens(ctx); + // ~7700-token system prefix: above max_input (7373), below ctx (8192). + let messages = vec![ + ChatMessage::system(&"s".repeat(30_800)), // (30800+3)/4 = 7700 tokens + user_msg("hi"), + ]; + let prefix = unevictable_prefix_tokens(&messages) as u64; + assert!( + prefix > max_input && prefix < ctx, + "test precondition: prefix ({prefix}) must sit between max_input \ + ({max_input}) and context_window ({ctx})" + ); + assert!( + unevictable_prefix_overflow(&messages, ctx).is_none(), + "prefix that fits the context window (just over the reply reserve) \ + must NOT be aborted — it is an over-trim case, not n_keep >= n_ctx" + ); + + // The same prefix against a context window it actually overflows + // (prefix >= n_ctx) MUST fire. + let small_ctx: u64 = prefix; // n_ctx == n_keep ⇒ the boundary itself fires + assert!( + unevictable_prefix_overflow(&messages, small_ctx).is_some(), + "prefix >= context_window (n_keep >= n_ctx) must be detected" + ); + } + + #[test] + fn unevictable_prefix_overflow_none_when_prompt_fits() { + // A small prompt against a normal window: trimming isn't needed and the + // guard must NOT fire (no false-positive actionable error). + let messages = vec![ChatMessage::system("short system"), user_msg("hello")]; + assert!(unevictable_prefix_overflow(&messages, 8_192).is_none()); + } + + #[test] + fn unevictable_prefix_overflow_counts_newest_turn_not_old_history() { + // The un-evictable prefix is system + the NEWEST non-system turn. Old + // history is evictable, so a huge oldest turn must NOT, by itself, + // trigger the guard when the system prefix + newest turn fit. + let ctx: u64 = 32_000; + let messages = vec![ + ChatMessage::system("small system"), + user_msg(&"x".repeat(400_000)), // oldest, evictable — ~100k tokens + user_msg("newest small turn"), + ]; + assert!( + unevictable_prefix_overflow(&messages, ctx).is_none(), + "evictable old history must not count toward the un-evictable prefix" + ); + } + #[test] fn trim_conversation_history_drops_oldest_messages() { let mut messages = vec![ConversationMessage::Chat(user_msg(&"y".repeat(80_000)))]; diff --git a/src/openhuman/inference/model_context.rs b/src/openhuman/inference/model_context.rs index e09c8912a..519ad13df 100644 --- a/src/openhuman/inference/model_context.rs +++ b/src/openhuman/inference/model_context.rs @@ -13,6 +13,30 @@ use crate::openhuman::config::{ const TIER_LARGE_CONTEXT: u64 = 200_000; const TIER_STANDARD_CONTEXT: u64 = 128_000; const TIER_LOCAL_CONTEXT: u64 = 8_192; + +/// Last-resort context window (tokens) for a **local** provider when neither the +/// static model table nor the provider profile declares one. +/// +/// Local runtimes (llama.cpp / vLLM via `LocalProviderKind::LocalOpenai`, whose +/// profile default is `None`) enforce the model's *loaded* `n_ctx` and reject an +/// over-budget prompt with a hard `400` (`n_keep >= n_ctx`) — issue #3550 / +/// Sentry TAURI-RUST-6V0. Returning `None` for those would **disable** +/// pre-dispatch trimming and let the overflow through. So instead of skipping, +/// we trim against this conservative floor: a slightly-too-aggressive trim is +/// strictly better than a guaranteed 400. The value is the smallest real local +/// profile default (MLX = 4096), chosen because any local runtime can hold at +/// least this much, while keeping the floor low enough to actually bound the +/// prompt. Only applied to local providers — cloud providers with an unknown +/// model keep `None` (their windows are large; a tiny floor would needlessly +/// truncate legitimate large-context requests). +/// +/// This floor is a **guess**, used for *trimming only*. It is deliberately not +/// fed into the engine's hard un-evictable-prefix abort ("reload with a larger +/// context length") — that abort consults the provider's *authoritative* +/// [`crate::openhuman::inference::provider::Provider::loaded_context_window`] +/// instead, so we never reject a request against a guessed window the real +/// loaded `n_ctx` would have accepted (Codex P1 review on PR #3771). +const CONSERVATIVE_LOCAL_CONTEXT_FLOOR: u64 = 4_096; /// Summarization tier. `summarization-v1` resolves to a long-context flash /// model (currently DeepSeek v4 flash, ~1M tokens). `extract_from_result` /// uses this window to single-shot whole oversized payloads instead of @@ -114,6 +138,14 @@ fn tier_context_window(model: &str) -> Option { /// function falls back to the provider profile's declared default context /// window. This ensures preflight trimming still works for local models /// even when the exact model name isn't in the static pattern table. +/// +/// For a **local** provider this never returns `None`: if the profile itself +/// declares no default (e.g. `LocalProviderKind::LocalOpenai` = llama.cpp / +/// vLLM), it falls back once more to [`CONSERVATIVE_LOCAL_CONTEXT_FLOOR`] so +/// trimming still engages instead of being silently skipped and overflowing +/// the runtime `n_ctx` (issue #3550 / Sentry TAURI-RUST-6V0). `None` is only +/// returned when `local_kind` is `None` (cloud provider, unknown model) — +/// where over-trimming a large window is worse than skipping the trim. pub fn context_window_for_model_with_local_fallback( model: &str, local_kind: Option, @@ -133,6 +165,17 @@ pub fn context_window_for_model_with_local_fallback( ); return Some(default_ctx); } + // Local provider with no declared default (llama.cpp / vLLM). Never + // return `None` here — that would disable pre-dispatch trimming and let + // the prompt overflow the runtime `n_ctx` (the TAURI-RUST-6V0 400). Trim + // against a conservative floor instead. + tracing::debug!( + model, + provider = kind.as_str(), + context_window = CONSERVATIVE_LOCAL_CONTEXT_FLOOR, + "[model_context] local provider has no profile default; using conservative context floor" + ); + return Some(CONSERVATIVE_LOCAL_CONTEXT_FLOOR); } None } @@ -205,6 +248,17 @@ mod tests { context_window_for_model_with_local_fallback("qwen3:14b", None), None ); + // Local provider whose profile declares no default (llama.cpp / vLLM via + // LocalOpenai) → conservative floor, NOT None. None here would disable + // pre-dispatch trimming and let the prompt overflow the runtime n_ctx + // (the TAURI-RUST-6V0 400). Must stay bounded. + assert_eq!( + context_window_for_model_with_local_fallback( + "some-unlisted-gguf", + Some(LocalProviderKind::LocalOpenai) + ), + Some(super::CONSERVATIVE_LOCAL_CONTEXT_FLOOR) + ); // Known model ignores local fallback assert_eq!( context_window_for_model_with_local_fallback( diff --git a/src/openhuman/inference/provider/compatible_provider_impl.rs b/src/openhuman/inference/provider/compatible_provider_impl.rs index 01ef76d8b..82fb0ffc5 100644 --- a/src/openhuman/inference/provider/compatible_provider_impl.rs +++ b/src/openhuman/inference/provider/compatible_provider_impl.rs @@ -1257,6 +1257,10 @@ impl Provider for OpenAiCompatibleProvider { Ok(()) } + fn is_local_provider(&self) -> bool { + self.local_provider_kind.is_some() + } + /// Resolve the effective context window for pre-dispatch trimming. /// /// For cloud (non-local) providers this is the static model table. For @@ -1285,6 +1289,20 @@ impl Provider for OpenAiCompatibleProvider { Some(kind), ) } + + /// Authoritative runtime-loaded window: only the value the local runtime + /// genuinely reports. Today that is LM Studio's native `/api/v0/models` + /// `loaded_context_length`. For every other case — cloud, llama.cpp / vLLM + /// (no loaded window endpoint), or a missing probe — we return `None` + /// rather than a guess, so the engine's hard pre-dispatch abort never fires + /// on an estimated window (Codex P1 review on PR #3771 / TAURI-RUST-6V0). + async fn loaded_context_window(&self, model: &str) -> Option { + use crate::openhuman::inference::local::profile::LocalProviderKind; + if self.local_provider_kind == Some(LocalProviderKind::LmStudio) { + return self.lm_studio_loaded_context_window(model).await; + } + None + } } impl OpenAiCompatibleProvider { diff --git a/src/openhuman/inference/provider/ops/http_error.rs b/src/openhuman/inference/provider/ops/http_error.rs index c8750cb18..0d3f0bc51 100644 --- a/src/openhuman/inference/provider/ops/http_error.rs +++ b/src/openhuman/inference/provider/ops/http_error.rs @@ -334,11 +334,26 @@ pub fn is_context_window_exceeded_message(body: &str) -> bool { "context size has been exceeded", "prompt is too long", "input is too long", + // LM Studio / llama.cpp un-evictable-prefix overflow (TAURI-RUST-6V0): + // `"The number of tokens to keep from the initial prompt is greater + // than the context length (n_keep: 10978 >= n_ctx: 8192). Try to + // load the model with a larger context length, …"`. The user's local + // model was loaded with an `n_ctx` smaller than the system/un-evictable + // prefix; the remediation lives in the user's local server (reload with + // a larger context), so this is expected user-state, not a product bug. + "greater than the context length", ]; if CONTEXT_HINTS.iter().any(|hint| lower.contains(hint)) { return true; } + // LM Studio / llama.cpp emit the overflow as a paired `n_keep … n_ctx` + // diagnostic. Require BOTH tokens so the arm stays anchored to that exact + // shape (TAURI-RUST-6V0) and never broadens to unrelated `n_ctx` logging. + if lower.contains("n_keep") && lower.contains("n_ctx") { + return true; + } + // Token-count phrases are ambiguous with token-per-minute RATE limits. // Treat them as context-overflow only when the body carries no // rate-limit marker — otherwise a transient TPM 429 would be silenced diff --git a/src/openhuman/inference/provider/ops_tests.rs b/src/openhuman/inference/provider/ops_tests.rs index b502dc1e5..8618f95dc 100644 --- a/src/openhuman/inference/provider/ops_tests.rs +++ b/src/openhuman/inference/provider/ops_tests.rs @@ -681,6 +681,28 @@ mod context_window_exceeded_suppression { } } + #[test] + fn classifies_lmstudio_n_keep_exceeds_n_ctx_body() { + // TAURI-RUST-6V0: LM Studio / llama.cpp reject a prompt whose + // un-evictable prefix (`n_keep`) is larger than the model's loaded + // context (`n_ctx`). The user loaded the model with too small a + // context length; the remediation lives in the user's local server, + // so the matcher must demote this from Sentry. Verbatim wire body. + let body = "lmstudio API error (400 Bad Request): {\"error\":\"The number of tokens to keep from the initial prompt is greater than the context length (n_keep: 10978 >= n_ctx: 8192). Try to load the model with a larger context length, or provide a shorter input.\"}"; + assert!( + is_context_window_exceeded_message(body), + "LM Studio n_keep >= n_ctx body must classify as context-window overflow" + ); + // Both anchors fire independently: the `greater than the context + // length` phrase AND the paired `n_keep`/`n_ctx` diagnostic. + assert!(is_context_window_exceeded_message( + "request rejected: prompt is greater than the context length of the loaded model" + )); + assert!(is_context_window_exceeded_message( + "n_keep: 9000 >= n_ctx: 4096" + )); + } + #[test] fn does_not_match_unrelated_bodies() { for body in [ @@ -688,6 +710,9 @@ mod context_window_exceeded_suppression { "Invalid request: model not found", "Insufficient budget", "tool call exceeded the allowed budget", + // Only one of the paired n_keep/n_ctx tokens present — must NOT + // match (guards the paired-anchor arm against bare n_ctx logging). + "loaded model with n_ctx: 8192 and 32 layers", ] { assert!( !is_context_window_exceeded_message(body), diff --git a/src/openhuman/inference/provider/reliable.rs b/src/openhuman/inference/provider/reliable.rs index 0816c7d24..c2723771d 100644 --- a/src/openhuman/inference/provider/reliable.rs +++ b/src/openhuman/inference/provider/reliable.rs @@ -450,6 +450,45 @@ impl Provider for ReliableProvider { Ok(()) } + /// Delegate to the primary provider so a wrapped local runtime reports its + /// runtime-loaded window (LM Studio `n_ctx`) for pre-dispatch trimming + /// instead of the static-table default (#3550 / TAURI-RUST-6V0). + async fn effective_context_window(&self, model: &str) -> Option { + match self.providers.first() { + Some((_, provider)) => provider.effective_context_window(model).await, + None => crate::openhuman::inference::context_window_for_model(model), + } + } + + /// Delegate to the primary provider so the engine's pre-dispatch + /// un-evictable-prefix guard fires for a wrapped local model (#3550). + fn is_local_provider(&self) -> bool { + self.providers + .first() + .map(|(_, p)| p.is_local_provider()) + .unwrap_or(false) + } + + /// Delegate the model-aware locality to the primary provider so a wrapped + /// router resolves `model` to its actual (possibly local) provider for the + /// engine's pre-dispatch guard (#3550 / PR #3771). + fn is_local_provider_for_model(&self, model: &str) -> bool { + self.providers + .first() + .map(|(_, p)| p.is_local_provider_for_model(model)) + .unwrap_or(false) + } + + /// Delegate the authoritative runtime-loaded window to the primary provider + /// so the engine's hard pre-dispatch abort sees the wrapped local runtime's + /// loaded `n_ctx` (#3550 / PR #3771). + async fn loaded_context_window(&self, model: &str) -> Option { + match self.providers.first() { + Some((_, provider)) => provider.loaded_context_window(model).await, + None => None, + } + } + async fn chat_with_system( &self, system_prompt: Option<&str>, diff --git a/src/openhuman/inference/provider/router.rs b/src/openhuman/inference/provider/router.rs index 04b229ada..7659c20e9 100644 --- a/src/openhuman/inference/provider/router.rs +++ b/src/openhuman/inference/provider/router.rs @@ -234,6 +234,53 @@ impl Provider for RouterProvider { .any(|(_, provider)| provider.supports_vision()) } + /// Delegate to the provider that actually handles `model` so local + /// runtimes report their runtime-loaded window (LM Studio `n_ctx`) instead + /// of the static-table default the trait would otherwise return (#3550 / + /// TAURI-RUST-6V0). + async fn effective_context_window(&self, model: &str) -> Option { + let (provider_idx, resolved_model) = self.resolve(model); + let (_, provider) = &self.providers[provider_idx]; + provider.effective_context_window(&resolved_model).await + } + + /// Whether the *default* provider is local. Model-blind — kept for callers + /// that have no model in hand. The engine's pre-dispatch guard uses the + /// model-aware [`Provider::is_local_provider_for_model`] below instead, so a + /// cloud-default router still gates correctly when it routes a model to a + /// local provider (#3550 / TAURI-RUST-6V0). + fn is_local_provider(&self) -> bool { + self.providers + .get(self.default_index) + .map(|(_, p)| p.is_local_provider()) + .unwrap_or(false) + } + + /// Resolve `model` to the provider that actually handles it and report + /// *that* provider's locality. Without this, a router whose default is + /// cloud reports `is_local_provider() == false` even when `model` routes to + /// a local provider, so the engine's pre-dispatch un-evictable-prefix guard + /// is skipped and the opaque local `400 (n_keep >= n_ctx)` reaches the user + /// (Codex P2 + CodeRabbit review on PR #3771). `effective_context_window` + /// already resolves the routed provider, so this keeps the two in step. + fn is_local_provider_for_model(&self, model: &str) -> bool { + let (provider_idx, _) = self.resolve(model); + self.providers + .get(provider_idx) + .map(|(_, p)| p.is_local_provider()) + .unwrap_or(false) + } + + /// Delegate the authoritative runtime-loaded window to the routed provider, + /// mirroring [`RouterProvider::effective_context_window`] so the engine's + /// hard pre-dispatch abort sees the same routed provider's loaded `n_ctx` + /// (#3550 / TAURI-RUST-6V0). + async fn loaded_context_window(&self, model: &str) -> Option { + let (provider_idx, resolved_model) = self.resolve(model); + let (_, provider) = &self.providers[provider_idx]; + provider.loaded_context_window(&resolved_model).await + } + async fn warmup(&self) -> anyhow::Result<()> { for (name, provider) in &self.providers { tracing::info!(provider = name, "Warming up routed provider"); diff --git a/src/openhuman/inference/provider/router_tests.rs b/src/openhuman/inference/provider/router_tests.rs index 2baea833b..05727e034 100644 --- a/src/openhuman/inference/provider/router_tests.rs +++ b/src/openhuman/inference/provider/router_tests.rs @@ -6,6 +6,10 @@ struct MockProvider { calls: Arc, response: &'static str, last_model: parking_lot::Mutex, + /// When set, this mock reports as a local runtime and exposes an + /// authoritative loaded context window — used to exercise the model-aware + /// locality routing for the engine's pre-dispatch prefix guard (#3771). + local_loaded_ctx: Option, } impl MockProvider { @@ -14,6 +18,17 @@ impl MockProvider { calls: Arc::new(AtomicUsize::new(0)), response, last_model: parking_lot::Mutex::new(String::new()), + local_loaded_ctx: None, + } + } + + /// A mock that reports as local with the given authoritative loaded window. + fn new_local(response: &'static str, loaded_ctx: u64) -> Self { + Self { + calls: Arc::new(AtomicUsize::new(0)), + response, + last_model: parking_lot::Mutex::new(String::new()), + local_loaded_ctx: Some(loaded_ctx), } } @@ -39,6 +54,14 @@ impl Provider for MockProvider { *self.last_model.lock() = model.to_string(); Ok(self.response.to_string()) } + + fn is_local_provider(&self) -> bool { + self.local_loaded_ctx.is_some() + } + + async fn loaded_context_window(&self, _model: &str) -> Option { + self.local_loaded_ctx + } } fn make_router( @@ -93,6 +116,14 @@ impl Provider for Arc { .chat_with_system(system_prompt, message, model, temperature) .await } + + fn is_local_provider(&self) -> bool { + self.as_ref().is_local_provider() + } + + async fn loaded_context_window(&self, model: &str) -> Option { + self.as_ref().loaded_context_window(model).await + } } #[tokio::test] @@ -308,6 +339,73 @@ fn skips_routes_with_unknown_provider() { assert!(!router.routes.contains_key("broken")); } +// -- #3771: model-aware locality for the pre-dispatch un-evictable-prefix guard -- + +#[tokio::test] +async fn is_local_provider_for_model_resolves_routed_provider_not_default() { + // Default provider is CLOUD; a hint routes a model to a LOCAL provider. + // The model-blind `is_local_provider()` reports the default (cloud), but + // the model-aware `is_local_provider_for_model()` must follow the route to + // the local provider so the engine arms its prefix guard (Codex P2 + + // CodeRabbit PR #3771). `effective_context_window`/`loaded_context_window` + // already resolve the route, so all three must agree per-model. + let cloud = Arc::new(MockProvider::new("cloud")); + let local = Arc::new(MockProvider::new_local("local", 8_192)); + let router = RouterProvider::new( + vec![ + ( + "cloud".to_string(), + Box::new(Arc::clone(&cloud)) as Box, + ), + ( + "local".to_string(), + Box::new(Arc::clone(&local)) as Box, + ), + ], + vec![( + "fast".to_string(), + Route { + provider_name: "local".to_string(), + model: "qwen3:8b".to_string(), + context_window: None, + }, + )], + "cloud-default-model".to_string(), + ); + + // Model-blind locality reflects the (cloud) default. + assert!( + !router.is_local_provider(), + "default provider is cloud → model-blind locality is false" + ); + + // A model that routes to the local provider must report local + expose its + // authoritative loaded window. + assert!( + router.is_local_provider_for_model("hint:fast"), + "a model routed to the local provider must report local" + ); + assert_eq!( + router.loaded_context_window("hint:fast").await, + Some(8_192), + "loaded window must come from the routed local provider" + ); + + // A model that falls through to the cloud default must NOT report local and + // has no authoritative loaded window. + assert!( + !router.is_local_provider_for_model("anthropic/claude-sonnet-4"), + "a model on the cloud default must not report local" + ); + assert_eq!( + router + .loaded_context_window("anthropic/claude-sonnet-4") + .await, + None, + "cloud provider exposes no authoritative loaded window" + ); +} + #[tokio::test] async fn warmup_calls_all_providers() { let (router, _) = make_router(vec![("a", "ok"), ("b", "ok")], vec![]); diff --git a/src/openhuman/inference/provider/traits.rs b/src/openhuman/inference/provider/traits.rs index 86c189fbf..2dd2c999a 100644 --- a/src/openhuman/inference/provider/traits.rs +++ b/src/openhuman/inference/provider/traits.rs @@ -538,6 +538,53 @@ pub trait Provider: Send + Sync { crate::openhuman::inference::context_window_for_model(model) } + /// Whether this provider talks to a **local** runtime (LM Studio, Ollama, + /// llama.cpp, vLLM, …) rather than a cloud API. Local runtimes enforce the + /// model's *runtime-loaded* `n_ctx` and can be loaded with a window smaller + /// than the assistant's un-evictable system prefix — the + /// `n_keep >= n_ctx` overflow (#3550 / TAURI-RUST-6V0). The agent engine + /// uses this to gate its pre-dispatch un-evictable-prefix guard, which + /// surfaces an actionable "reload with a larger context length" error only + /// for local providers (cloud windows are large enough that the guard would + /// only ever fire on a genuine overflow the user can't remedy by reloading). + /// Defaults to `false`. + fn is_local_provider(&self) -> bool { + false + } + + /// Like [`Provider::is_local_provider`] but resolved for the specific + /// `model` about to be dispatched. A router whose *default* provider is + /// cloud may still route a given model to a local provider; the engine's + /// pre-dispatch un-evictable-prefix guard keys off this so the actionable + /// "reload with a larger context length" error fires for that routed local + /// model instead of letting the opaque local `400 (n_keep >= n_ctx)` reach + /// the user (#3550 / TAURI-RUST-6V0; Codex/CodeRabbit review on PR #3771). + /// + /// Defaults to the model-blind [`Provider::is_local_provider`]; only a + /// routing wrapper needs to override it. + fn is_local_provider_for_model(&self, _model: &str) -> bool { + self.is_local_provider() + } + + /// The model's **authoritative runtime-loaded** context window, when the + /// local runtime actually reports it (e.g. LM Studio's native + /// `/api/v0/models` `loaded_context_length`). Returns `None` whenever the + /// window is unknown or merely *guessed* — a cloud provider, a local + /// runtime that exposes no loaded window (llama.cpp / vLLM), or a + /// profile-default / conservative-floor fallback. + /// + /// Distinct from [`Provider::effective_context_window`], which always + /// yields a value for local providers (falling back to a guess) so + /// pre-dispatch *trimming* still engages. Trimming may safely run against a + /// guess (over-trim is harmless), but the hard pre-dispatch abort must only + /// fire on an authoritative window — aborting with "reload with a larger + /// context length" against a guessed 4096 floor would wrongly reject a + /// request that the real (e.g. 32k) loaded window would have accepted + /// (Codex P1 review on PR #3771). Defaults to `None`. + async fn loaded_context_window(&self, _model: &str) -> Option { + None + } + /// Warm up the HTTP connection pool (TLS handshake, DNS, HTTP/2 setup). /// Default implementation is a no-op; providers with HTTP clients should override. async fn warmup(&self) -> anyhow::Result<()> {