mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(inference): cap extraction max_tokens + classify provider 402 to stop the retry/Sentry storm (#3616) (#3617)
Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
This commit is contained in:
co-authored by
Steven Enamakel
parent
5b80f25cd4
commit
445b11afc2
@@ -189,6 +189,7 @@ Emit tool calls as `<tool_call>name[arg1|arg2]</tool_call>` blocks.
|
||||
messages: &messages,
|
||||
tools: tools_for_request.as_deref(),
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
|
||||
eprintln!("[probe] >>> raw provider.chat()...");
|
||||
|
||||
@@ -2191,6 +2191,52 @@ pub fn is_budget_event(event: &sentry::protocol::Event<'_>) -> bool {
|
||||
event_contains_budget_exhausted_message(event)
|
||||
}
|
||||
|
||||
/// Defense-in-depth `before_send` filter for **insufficient-credits 402**
|
||||
/// provider events (TAURI-RUST-C62): the user's own BYO provider account
|
||||
/// (e.g. OpenRouter) is out of balance — a billing state OpenHuman has no
|
||||
/// lever over once the request already caps `max_tokens`.
|
||||
///
|
||||
/// The primary emit-site demotion lives in the `Provider::chat()` native_chat
|
||||
/// cascade (`is_provider_insufficient_credits_402`), but the compatible
|
||||
/// provider reports the same failure from several other paths
|
||||
/// (`chat_with_system`, `chat_with_history`, the streaming gates, and the
|
||||
/// shared `api_error` helper) that don't run that cascade. This filter is the
|
||||
/// single outermost net that catches all of them, keyed on the formatted
|
||||
/// message rather than tags so it matches regardless of which path emitted it.
|
||||
///
|
||||
/// Match criteria (all required):
|
||||
/// - the event message or any exception value names a 402 / payment-required
|
||||
/// failure (`"402"` or `"payment required"`), AND
|
||||
/// - that same text carries an insufficient-credits phrase
|
||||
/// (`provider::body_indicates_insufficient_credits`).
|
||||
pub fn is_insufficient_credits_event(event: &sentry::protocol::Event<'_>) -> bool {
|
||||
fn text_is_insufficient_credits_402(text: &str) -> bool {
|
||||
let lower = text.to_ascii_lowercase();
|
||||
// Anchor the 402 to a status shape — the emit sites format the message
|
||||
// as "<provider> API error (402 Payment Required): <body>". Matching a
|
||||
// bare "402" would false-positive on body digits (e.g. a 400 error
|
||||
// whose body says "can only afford 402 tokens"), which is NOT this
|
||||
// user-state and must keep reaching Sentry.
|
||||
let is_402_status = lower.contains("(402") || lower.contains("402 payment required");
|
||||
is_402_status
|
||||
&& crate::openhuman::inference::provider::body_indicates_insufficient_credits(text)
|
||||
}
|
||||
|
||||
if event
|
||||
.message
|
||||
.as_deref()
|
||||
.is_some_and(text_is_insufficient_credits_402)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
event.exception.values.iter().any(|exception| {
|
||||
exception
|
||||
.value
|
||||
.as_deref()
|
||||
.is_some_and(text_is_insufficient_credits_402)
|
||||
})
|
||||
}
|
||||
|
||||
/// 404 on PATCH/DELETE to a channel-message path is an expected backend state
|
||||
/// (user deleted the message provider-side, backend GC'd the relay row). The
|
||||
/// primary suppression lives in `authed_json` via `parse_message_path` +
|
||||
@@ -5128,6 +5174,49 @@ mod tests {
|
||||
event
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn insufficient_credits_filter_matches_message_path() {
|
||||
// Verbatim TAURI-RUST-C62 message as formatted by the provider emit
|
||||
// sites: "<provider> API error (402 Payment Required): <body>".
|
||||
let event = event_with_message(
|
||||
"myopenrouter API error (402 Payment Required): This request requires more credits, \
|
||||
or fewer max_tokens. You requested up to 65536 tokens, but can only afford 49732.",
|
||||
);
|
||||
assert!(is_insufficient_credits_event(&event));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn insufficient_credits_filter_matches_exception_path() {
|
||||
let event = event_with_exception_value(
|
||||
"myopenrouter API error (402 Payment Required): insufficient balance",
|
||||
);
|
||||
assert!(is_insufficient_credits_event(&event));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn insufficient_credits_filter_requires_both_402_and_credit_phrase() {
|
||||
// A 402 with no credit phrase must NOT be swallowed (could be another
|
||||
// payment semantic) ...
|
||||
assert!(!is_insufficient_credits_event(&event_with_message(
|
||||
"provider API error (402): some unrelated condition"
|
||||
)));
|
||||
// ... and a credit phrase without a 402 must NOT be swallowed (e.g. a
|
||||
// 400/500 that merely mentions balance) so a real defect still pages.
|
||||
assert!(!is_insufficient_credits_event(&event_with_message(
|
||||
"provider API error (500): internal error, insufficient memory"
|
||||
)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn insufficient_credits_filter_ignores_402_digits_in_a_non_402_body() {
|
||||
// A non-402 error whose body merely contains the digits "402" and a
|
||||
// credit phrase must NOT be suppressed — the 402 must be the status,
|
||||
// not an arbitrary number in the body.
|
||||
assert!(!is_insufficient_credits_event(&event_with_message(
|
||||
"provider API error (400): can only afford 402 tokens"
|
||||
)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn max_iterations_filter_matches_message_path() {
|
||||
// `report_error_message` calls `sentry::capture_message`, which
|
||||
|
||||
@@ -84,6 +84,14 @@ fn main() {
|
||||
if openhuman_core::core::observability::is_budget_event(&event) {
|
||||
return None;
|
||||
}
|
||||
// Defense-in-depth for insufficient-credits 402s. The native_chat
|
||||
// emit site demotes them, but the compatible provider reports the
|
||||
// same out-of-balance 402 from chat_with_system / chat_with_history
|
||||
// / the streaming gates / api_error too; this is the single net
|
||||
// that catches every path (TAURI-RUST-C62).
|
||||
if openhuman_core::core::observability::is_insufficient_credits_event(&event) {
|
||||
return None;
|
||||
}
|
||||
// Defense-in-depth: drop max-tool-iterations cap events that
|
||||
// slipped past the call-site filters in
|
||||
// `agent::harness::session::runtime::run_single`,
|
||||
|
||||
@@ -424,6 +424,7 @@ pub(crate) async fn run_turn_engine(
|
||||
messages: &prepared_messages_vec,
|
||||
tools: request_tools,
|
||||
stream: delta_tx_opt.as_ref(),
|
||||
max_tokens: None,
|
||||
},
|
||||
model,
|
||||
temperature,
|
||||
|
||||
@@ -120,6 +120,7 @@ impl Agent {
|
||||
messages: &messages,
|
||||
tools: None,
|
||||
stream: delta_tx_opt.as_ref(),
|
||||
max_tokens: None,
|
||||
},
|
||||
effective_model,
|
||||
self.temperature,
|
||||
|
||||
@@ -472,6 +472,7 @@ impl CheckpointStrategy for AgentCheckpoint {
|
||||
messages: &messages,
|
||||
tools: None,
|
||||
stream: delta_tx_opt.as_ref(),
|
||||
max_tokens: None,
|
||||
},
|
||||
&self.model,
|
||||
self.temperature,
|
||||
|
||||
@@ -44,6 +44,7 @@ impl super::super::super::engine::CheckpointStrategy for SubagentCheckpoint<'_>
|
||||
messages: &summary_input,
|
||||
tools: None,
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
},
|
||||
&self.model,
|
||||
self.temperature,
|
||||
|
||||
@@ -52,6 +52,7 @@ async fn keyword_provider_records_forced_then_fallback_turns() {
|
||||
messages: &messages,
|
||||
tools: None,
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
},
|
||||
"test-model",
|
||||
0.0,
|
||||
@@ -66,6 +67,7 @@ async fn keyword_provider_records_forced_then_fallback_turns() {
|
||||
messages: &messages,
|
||||
tools: None,
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
},
|
||||
"test-model",
|
||||
0.0,
|
||||
@@ -101,6 +103,7 @@ async fn keyword_provider_prompt_guided_text_wraps_tool_calls_and_honors_fire_li
|
||||
messages: &messages,
|
||||
tools: None,
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
},
|
||||
"test-model",
|
||||
0.0,
|
||||
@@ -121,6 +124,7 @@ async fn keyword_provider_prompt_guided_text_wraps_tool_calls_and_honors_fire_li
|
||||
messages: &messages,
|
||||
tools: None,
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
},
|
||||
"test-model",
|
||||
0.0,
|
||||
|
||||
@@ -229,6 +229,7 @@ impl Provider for ClaudeCodeProvider {
|
||||
messages: &messages,
|
||||
tools: None,
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
let resp = self.run_chat(request, Some(model)).await?;
|
||||
Ok(resp.text.unwrap_or_default())
|
||||
@@ -244,6 +245,7 @@ impl Provider for ClaudeCodeProvider {
|
||||
messages,
|
||||
tools: None,
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
let resp = self.run_chat(request, Some(model)).await?;
|
||||
Ok(resp.text.unwrap_or_default())
|
||||
|
||||
@@ -20,6 +20,7 @@ impl OpenAiCompatibleProvider {
|
||||
credential: Option<&str>,
|
||||
messages: &[ChatMessage],
|
||||
model: &str,
|
||||
max_output_tokens: Option<u32>,
|
||||
) -> anyhow::Result<String> {
|
||||
let (instructions, input) = build_responses_prompt(messages);
|
||||
if input.is_empty() {
|
||||
@@ -29,6 +30,13 @@ impl OpenAiCompatibleProvider {
|
||||
);
|
||||
}
|
||||
|
||||
log::debug!(
|
||||
"[provider] {} responses-path model={model} max_output_tokens={:?} input_msgs={}",
|
||||
self.name,
|
||||
max_output_tokens,
|
||||
input.len(),
|
||||
);
|
||||
|
||||
// #3201: the Codex/ChatGPT OAuth Responses endpoint rejects `stream: false`
|
||||
// outright. This branch lifts the constraint for that endpoint specifically
|
||||
// and parses the resulting SSE body so the existing non-streaming call
|
||||
@@ -51,6 +59,7 @@ impl OpenAiCompatibleProvider {
|
||||
instructions,
|
||||
stream: Some(is_codex_oauth_responses),
|
||||
store: Some(false),
|
||||
max_output_tokens,
|
||||
};
|
||||
|
||||
let url = self.responses_url();
|
||||
|
||||
@@ -80,7 +80,7 @@ impl Provider for OpenAiCompatibleProvider {
|
||||
|
||||
if self.responses_api_primary {
|
||||
return self
|
||||
.chat_via_responses(credential, &fallback_messages, model)
|
||||
.chat_via_responses(credential, &fallback_messages, model, None)
|
||||
.await;
|
||||
}
|
||||
|
||||
@@ -94,7 +94,7 @@ impl Provider for OpenAiCompatibleProvider {
|
||||
if self.supports_responses_fallback {
|
||||
let detail = super::super::format_error_chain(&chat_error);
|
||||
return self
|
||||
.chat_via_responses(credential, &fallback_messages, model)
|
||||
.chat_via_responses(credential, &fallback_messages, model, None)
|
||||
.await
|
||||
.map_err(|responses_err| {
|
||||
let fb = super::super::format_anyhow_chain(&responses_err);
|
||||
@@ -124,7 +124,7 @@ impl Provider for OpenAiCompatibleProvider {
|
||||
|
||||
if status == reqwest::StatusCode::NOT_FOUND && self.supports_responses_fallback {
|
||||
return self
|
||||
.chat_via_responses(credential, &fallback_messages, model)
|
||||
.chat_via_responses(credential, &fallback_messages, model, None)
|
||||
.await
|
||||
.map_err(|responses_err| {
|
||||
let fb = super::super::format_anyhow_chain(&responses_err);
|
||||
@@ -252,7 +252,7 @@ impl Provider for OpenAiCompatibleProvider {
|
||||
let url = self.chat_completions_url();
|
||||
if self.responses_api_primary {
|
||||
return self
|
||||
.chat_via_responses(credential, &effective_messages, model)
|
||||
.chat_via_responses(credential, &effective_messages, model, None)
|
||||
.await;
|
||||
}
|
||||
|
||||
@@ -266,7 +266,7 @@ impl Provider for OpenAiCompatibleProvider {
|
||||
if self.supports_responses_fallback {
|
||||
let detail = super::super::format_error_chain(&chat_error);
|
||||
return self
|
||||
.chat_via_responses(credential, &effective_messages, model)
|
||||
.chat_via_responses(credential, &effective_messages, model, None)
|
||||
.await
|
||||
.map_err(|responses_err| {
|
||||
let fb = super::super::format_anyhow_chain(&responses_err);
|
||||
@@ -294,7 +294,7 @@ impl Provider for OpenAiCompatibleProvider {
|
||||
|
||||
if self.supports_responses_fallback {
|
||||
return self
|
||||
.chat_via_responses(credential, &effective_messages, model)
|
||||
.chat_via_responses(credential, &effective_messages, model, None)
|
||||
.await
|
||||
.map_err(|responses_err| {
|
||||
let fb = super::super::format_anyhow_chain(&responses_err);
|
||||
@@ -489,7 +489,7 @@ impl Provider for OpenAiCompatibleProvider {
|
||||
effective_messages.clone()
|
||||
};
|
||||
let text = self
|
||||
.chat_via_responses(credential, &response_messages, model)
|
||||
.chat_via_responses(credential, &response_messages, model, request.max_tokens)
|
||||
.await?;
|
||||
if let Some(tx) = request.stream {
|
||||
let _ = tx
|
||||
@@ -525,6 +525,7 @@ impl Provider for OpenAiCompatibleProvider {
|
||||
// field (Google Gemini shim — TAURI-RUST-4PJ); the reactive
|
||||
// retry below stays as defense-in-depth for unknown providers.
|
||||
frequency_penalty: self.effective_frequency_penalty(),
|
||||
max_tokens: request.max_tokens,
|
||||
};
|
||||
let stream_dump_seq = reserve_dump_seq();
|
||||
dump_prompt_if_enabled(&self.name, model, stream_dump_seq, &native_request);
|
||||
@@ -611,6 +612,7 @@ impl Provider for OpenAiCompatibleProvider {
|
||||
// The buffered non-streaming path omits `frequency_penalty` for maximum
|
||||
// compatibility. The streaming path carries it and retries without on rejection.
|
||||
frequency_penalty: None,
|
||||
max_tokens: request.max_tokens,
|
||||
};
|
||||
let dump_seq = reserve_dump_seq();
|
||||
dump_prompt_if_enabled(&self.name, model, dump_seq, &native_request);
|
||||
@@ -629,7 +631,12 @@ impl Provider for OpenAiCompatibleProvider {
|
||||
if self.supports_responses_fallback {
|
||||
let detail = super::super::format_error_chain(&chat_error);
|
||||
return self
|
||||
.chat_via_responses(credential, &effective_messages, model)
|
||||
.chat_via_responses(
|
||||
credential,
|
||||
&effective_messages,
|
||||
model,
|
||||
request.max_tokens,
|
||||
)
|
||||
.await
|
||||
.map(|text| ProviderChatResponse {
|
||||
text: Some(text),
|
||||
@@ -679,7 +686,7 @@ impl Provider for OpenAiCompatibleProvider {
|
||||
|
||||
if status == reqwest::StatusCode::NOT_FOUND && self.supports_responses_fallback {
|
||||
return self
|
||||
.chat_via_responses(credential, &effective_messages, model)
|
||||
.chat_via_responses(credential, &effective_messages, model, request.max_tokens)
|
||||
.await
|
||||
.map(|text| ProviderChatResponse {
|
||||
text: Some(text),
|
||||
@@ -737,6 +744,17 @@ impl Provider for OpenAiCompatibleProvider {
|
||||
Some(model),
|
||||
status,
|
||||
);
|
||||
} else if super::super::is_provider_insufficient_credits_402(status, &error) {
|
||||
// Residual 402 after the request already caps max_tokens: the
|
||||
// user's own BYO provider balance is exhausted — no local lever,
|
||||
// so demote to info instead of paging on every retry
|
||||
// (TAURI-RUST-C62).
|
||||
super::super::log_provider_insufficient_credits_402(
|
||||
"native_chat",
|
||||
self.name.as_str(),
|
||||
Some(model),
|
||||
status,
|
||||
);
|
||||
} else if super::super::should_report_provider_http_failure(status) {
|
||||
crate::core::observability::report_error(
|
||||
message.as_str(),
|
||||
|
||||
@@ -67,6 +67,7 @@ fn native_request_emits_thread_id_when_present() {
|
||||
stream_options: None,
|
||||
options: None,
|
||||
frequency_penalty: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
let json = serde_json::to_value(&req).unwrap();
|
||||
assert_eq!(
|
||||
@@ -86,6 +87,7 @@ fn native_request_emits_thread_id_when_present() {
|
||||
stream_options: None,
|
||||
options: None,
|
||||
frequency_penalty: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
let json_no_thread = serde_json::to_value(&req_no_thread).unwrap();
|
||||
assert!(
|
||||
@@ -107,6 +109,7 @@ fn native_request_serializes_frequency_penalty_only_when_set() {
|
||||
stream_options: None,
|
||||
options: None,
|
||||
frequency_penalty: Some(0.3),
|
||||
max_tokens: None,
|
||||
};
|
||||
let json = serde_json::to_value(&base).unwrap();
|
||||
assert_eq!(
|
||||
@@ -127,6 +130,74 @@ fn native_request_serializes_frequency_penalty_only_when_set() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn native_request_serializes_max_tokens_only_when_set() {
|
||||
// A set cap must reach the wire as OpenAI `max_tokens` so a credit-metered
|
||||
// provider prices the request against a realistic output budget rather than
|
||||
// the model's full window (TAURI-RUST-C62).
|
||||
let with_cap = super::NativeChatRequest {
|
||||
model: "anthropic/claude-fable-5".to_string(),
|
||||
messages: Vec::new(),
|
||||
temperature: Some(0.0),
|
||||
stream: Some(false),
|
||||
tools: None,
|
||||
tool_choice: None,
|
||||
thread_id: None,
|
||||
stream_options: None,
|
||||
options: None,
|
||||
frequency_penalty: None,
|
||||
max_tokens: Some(8192),
|
||||
};
|
||||
let json = serde_json::to_value(&with_cap).unwrap();
|
||||
assert_eq!(
|
||||
json.get("max_tokens").and_then(serde_json::Value::as_u64),
|
||||
Some(8192),
|
||||
"a set max_tokens must be forwarded so the provider's balance pre-flight is bounded"
|
||||
);
|
||||
|
||||
let no_cap = super::NativeChatRequest {
|
||||
max_tokens: None,
|
||||
..with_cap
|
||||
};
|
||||
let json_none = serde_json::to_value(&no_cap).unwrap();
|
||||
assert!(
|
||||
json_none.get("max_tokens").is_none(),
|
||||
"absent max_tokens must be omitted so open-ended generations are unaffected"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn responses_request_serializes_max_output_tokens_only_when_set() {
|
||||
// The Responses-API branch must carry the cap as `max_output_tokens` so a
|
||||
// capped request isn't silently uncapped when responses_api_primary is on
|
||||
// (TAURI-RUST-C62).
|
||||
let with_cap = super::compatible_types::ResponsesRequest {
|
||||
model: "gpt-x".to_string(),
|
||||
input: vec![],
|
||||
instructions: None,
|
||||
stream: Some(false),
|
||||
store: Some(false),
|
||||
max_output_tokens: Some(8192),
|
||||
};
|
||||
let json = serde_json::to_value(&with_cap).unwrap();
|
||||
assert_eq!(
|
||||
json.get("max_output_tokens")
|
||||
.and_then(serde_json::Value::as_u64),
|
||||
Some(8192),
|
||||
"a set cap must reach the Responses API as max_output_tokens"
|
||||
);
|
||||
|
||||
let no_cap = super::compatible_types::ResponsesRequest {
|
||||
max_output_tokens: None,
|
||||
..with_cap
|
||||
};
|
||||
let json_none = serde_json::to_value(&no_cap).unwrap();
|
||||
assert!(
|
||||
json_none.get("max_output_tokens").is_none(),
|
||||
"absent cap must be omitted"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn detects_frequency_penalty_rejection_for_retry() {
|
||||
use super::OpenAiCompatibleProvider as P;
|
||||
@@ -248,6 +319,7 @@ async fn streaming_chat_frequency_penalty_rejection_not_reported_to_sentry() {
|
||||
}),
|
||||
options: None,
|
||||
frequency_penalty: Some(super::compatible_repeat::CHAT_FREQUENCY_PENALTY),
|
||||
max_tokens: None,
|
||||
};
|
||||
let (delta_tx, _delta_rx) = tokio::sync::mpsc::channel(8);
|
||||
|
||||
@@ -287,6 +359,7 @@ fn streaming_request_sets_stream_options_include_usage() {
|
||||
}),
|
||||
options: None,
|
||||
frequency_penalty: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
let json = serde_json::to_value(&req).unwrap();
|
||||
assert_eq!(
|
||||
@@ -310,6 +383,7 @@ fn non_streaming_request_omits_stream_options() {
|
||||
stream_options: None,
|
||||
options: None,
|
||||
frequency_penalty: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
let json = serde_json::to_value(&req).unwrap();
|
||||
assert!(
|
||||
@@ -333,6 +407,7 @@ fn ollama_options_num_ctx_serializes_correctly() {
|
||||
num_ctx: Some(32768),
|
||||
}),
|
||||
frequency_penalty: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
let json = serde_json::to_value(&req).unwrap();
|
||||
assert_eq!(
|
||||
@@ -355,6 +430,7 @@ fn ollama_options_none_is_omitted() {
|
||||
stream_options: None,
|
||||
options: None,
|
||||
frequency_penalty: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
let json = serde_json::to_value(&req).unwrap();
|
||||
assert!(
|
||||
@@ -911,6 +987,7 @@ async fn chat_via_responses_requires_non_system_message() {
|
||||
Some("test-key"),
|
||||
&[ChatMessage::system("policy")],
|
||||
"gpt-test",
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.expect_err("system-only fallback payload should fail");
|
||||
@@ -969,6 +1046,7 @@ async fn streaming_chat_config_rejection_propagates_error_without_sentry_report(
|
||||
}),
|
||||
options: None,
|
||||
frequency_penalty: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
let (delta_tx, _delta_rx) = tokio::sync::mpsc::channel(8);
|
||||
|
||||
@@ -1434,6 +1512,7 @@ async fn streaming_tool_call_captures_extra_content() {
|
||||
}),
|
||||
options: None,
|
||||
frequency_penalty: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
let (delta_tx, _delta_rx) = tokio::sync::mpsc::channel(64);
|
||||
let resp = provider
|
||||
@@ -1494,6 +1573,7 @@ async fn streaming_empty_continuation_id_does_not_clobber_tool_call_id() {
|
||||
}),
|
||||
options: None,
|
||||
frequency_penalty: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
let (delta_tx, _delta_rx) = tokio::sync::mpsc::channel(64);
|
||||
let resp = provider
|
||||
@@ -1548,6 +1628,7 @@ async fn streaming_parallel_tool_calls_preserve_ids_against_empty_continuations(
|
||||
}),
|
||||
options: None,
|
||||
frequency_penalty: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
let (delta_tx, _delta_rx) = tokio::sync::mpsc::channel(64);
|
||||
let resp = provider
|
||||
@@ -1606,6 +1687,7 @@ async fn streaming_omitted_continuation_id_preserves_tool_call_id() {
|
||||
}),
|
||||
options: None,
|
||||
frequency_penalty: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
let (delta_tx, _delta_rx) = tokio::sync::mpsc::channel(64);
|
||||
let resp = provider
|
||||
|
||||
@@ -194,6 +194,13 @@ pub(crate) struct NativeChatRequest {
|
||||
/// don't accept it are unaffected.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) frequency_penalty: Option<f64>,
|
||||
/// OpenAI-compatible `max_tokens` — upper bound on output tokens.
|
||||
/// Set by callers whose output is bounded (memory extraction) so
|
||||
/// credit-metered providers don't price the request against the full
|
||||
/// model output window during their balance pre-flight (TAURI-RUST-C62).
|
||||
/// Skipped when `None` so open-ended generations are unaffected.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) max_tokens: Option<u32>,
|
||||
}
|
||||
|
||||
/// Ollama-specific request options passed in the `options` field.
|
||||
@@ -240,6 +247,12 @@ pub(crate) struct ResponsesRequest {
|
||||
pub(crate) stream: Option<bool>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) store: Option<bool>,
|
||||
/// Responses-API output-token cap (`max_output_tokens`). Carries the
|
||||
/// caller's `ChatRequest::max_tokens` through the Responses path so a
|
||||
/// capped request isn't silently uncapped when `responses_api_primary`
|
||||
/// is enabled (TAURI-RUST-C62). Skipped when `None`.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) max_output_tokens: Option<u32>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Serialize)]
|
||||
|
||||
@@ -1531,6 +1531,7 @@ async fn live_lmstudio_provider_streams_thinking_and_text() {
|
||||
messages: &messages,
|
||||
tools: None,
|
||||
stream: Some(&tx),
|
||||
max_tokens: None,
|
||||
},
|
||||
&resolved_model,
|
||||
0.0,
|
||||
@@ -1589,6 +1590,7 @@ async fn live_ollama_provider_streams_text() {
|
||||
messages: &messages,
|
||||
tools: None,
|
||||
stream: Some(&tx),
|
||||
max_tokens: None,
|
||||
},
|
||||
&resolved_model,
|
||||
0.0,
|
||||
|
||||
@@ -155,6 +155,65 @@ pub fn log_provider_access_policy_denied_http_403(
|
||||
);
|
||||
}
|
||||
|
||||
/// Whether a provider non-2xx response is a deterministic
|
||||
/// **insufficient-credits** user-state error — the BYO provider account
|
||||
/// (e.g. OpenRouter) lacks the balance to satisfy the request.
|
||||
///
|
||||
/// This is the *residual* case once the request already caps `max_tokens`
|
||||
/// (so the provider's pre-flight is priced against a realistic output budget
|
||||
/// rather than the model's full window — see
|
||||
/// [`crate::openhuman::inference::provider::ChatRequest::max_tokens`]): a 402
|
||||
/// that still arrives means the user's own third-party account is genuinely
|
||||
/// out of credit, a billing state OpenHuman has no lever over. Demote from
|
||||
/// Sentry to an info log rather than page once per retry
|
||||
/// (TAURI-RUST-C62: 12k events from a single low-balance user).
|
||||
///
|
||||
/// Gated on the 402 status **and** a credit/payment phrase so an unrelated
|
||||
/// 402 is not swallowed. The phrase list is covered by a verbatim-body test
|
||||
/// so a provider wording drift fails CI instead of silently leaking events.
|
||||
pub fn is_provider_insufficient_credits_402(status: reqwest::StatusCode, body: &str) -> bool {
|
||||
status == reqwest::StatusCode::PAYMENT_REQUIRED && body_indicates_insufficient_credits(body)
|
||||
}
|
||||
|
||||
/// Phrase-level matcher for an insufficient-credits / out-of-balance provider
|
||||
/// error body. Single source of truth for the credit-phrase set, shared by the
|
||||
/// emit-site guard [`is_provider_insufficient_credits_402`] (which adds the 402
|
||||
/// status gate) and the `before_send` defense-in-depth filter
|
||||
/// [`crate::core::observability::is_insufficient_credits_event`] (which matches
|
||||
/// the formatted `<provider> API error (402 …): <body>` message so the demotion
|
||||
/// reaches every compatible-provider HTTP path — `chat_with_system`,
|
||||
/// `chat_with_history`, the streaming gates, and `api_error` — not just
|
||||
/// `Provider::chat()`'s `native_chat` cascade). TAURI-RUST-C62.
|
||||
pub fn body_indicates_insufficient_credits(body: &str) -> bool {
|
||||
let lower = body.to_ascii_lowercase();
|
||||
lower.contains("requires more credits")
|
||||
|| lower.contains("more credits")
|
||||
|| lower.contains("can only afford")
|
||||
|| lower.contains("insufficient credit")
|
||||
|| lower.contains("insufficient balance")
|
||||
|| lower.contains("insufficient funds")
|
||||
|| lower.contains("payment required")
|
||||
}
|
||||
|
||||
pub fn log_provider_insufficient_credits_402(
|
||||
operation: &str,
|
||||
provider: &str,
|
||||
model: Option<&str>,
|
||||
status: reqwest::StatusCode,
|
||||
) {
|
||||
tracing::info!(
|
||||
domain = "llm_provider",
|
||||
operation = operation,
|
||||
provider = provider,
|
||||
model = model.unwrap_or(""),
|
||||
status = status.as_u16(),
|
||||
failure = "non_2xx",
|
||||
kind = "insufficient_credits",
|
||||
"[llm_provider] {operation} provider insufficient-credits 402 — BYO account out of \
|
||||
balance (no local lever), not reporting to Sentry"
|
||||
);
|
||||
}
|
||||
|
||||
/// Whether a provider non-2xx response is a deterministic
|
||||
/// **configuration-rejection** user-state error (unknown model id,
|
||||
/// abstract tier leaked to a custom provider, model-specific temperature
|
||||
@@ -450,3 +509,64 @@ pub async fn api_error(provider: &str, response: reqwest::Response) -> anyhow::E
|
||||
}
|
||||
anyhow::anyhow!(message)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use reqwest::StatusCode;
|
||||
|
||||
/// Verbatim TAURI-RUST-C62 provider body. The matcher keys on this prose,
|
||||
/// so coupling the test to the exact string makes a provider wording drift
|
||||
/// fail CI rather than silently leak events back to Sentry.
|
||||
const C62_BODY: &str = "myopenrouter API error (402 Payment Required): \
|
||||
{\"error\":{\"message\":\"This request requires more credits, or fewer max_tokens. \
|
||||
You requested up to 65536 tokens, but can only afford 49732.\"}}";
|
||||
|
||||
#[test]
|
||||
fn insufficient_credits_402_matches_verbatim_c62_body() {
|
||||
assert!(is_provider_insufficient_credits_402(
|
||||
StatusCode::PAYMENT_REQUIRED,
|
||||
C62_BODY
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn insufficient_credits_402_matches_common_phrasings() {
|
||||
for body in [
|
||||
"insufficient balance",
|
||||
"Insufficient credits to complete this request",
|
||||
"insufficient funds on account",
|
||||
"you can only afford 100 tokens",
|
||||
"402 Payment Required",
|
||||
] {
|
||||
assert!(
|
||||
is_provider_insufficient_credits_402(StatusCode::PAYMENT_REQUIRED, body),
|
||||
"should match: {body:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn insufficient_credits_402_ignores_non_402_status() {
|
||||
// Same prose but a non-402 status is not this user-state — must stay
|
||||
// reportable so a genuine bug elsewhere isn't swallowed.
|
||||
assert!(!is_provider_insufficient_credits_402(
|
||||
StatusCode::BAD_REQUEST,
|
||||
C62_BODY
|
||||
));
|
||||
assert!(!is_provider_insufficient_credits_402(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
C62_BODY
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn insufficient_credits_402_ignores_unrelated_402_body() {
|
||||
// A 402 without any credit/payment phrase (reserved for other payment
|
||||
// semantics) is not swallowed by this guard.
|
||||
assert!(!is_provider_insufficient_credits_402(
|
||||
StatusCode::PAYMENT_REQUIRED,
|
||||
"{\"error\":{\"message\":\"some unrelated condition\"}}"
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,13 +19,14 @@ pub use sanitize::{
|
||||
};
|
||||
|
||||
pub use http_error::{
|
||||
api_error, is_backend_auth_failure, is_backend_error_code_owned, is_budget_exhausted_http_400,
|
||||
is_context_window_exceeded_message, is_custom_openai_upstream_bad_request_http_400,
|
||||
is_provider_access_policy_denied_http_403, is_provider_config_rejection_http,
|
||||
api_error, body_indicates_insufficient_credits, is_backend_auth_failure,
|
||||
is_backend_error_code_owned, is_budget_exhausted_http_400, is_context_window_exceeded_message,
|
||||
is_custom_openai_upstream_bad_request_http_400, is_provider_access_policy_denied_http_403,
|
||||
is_provider_config_rejection_http, is_provider_insufficient_credits_402,
|
||||
log_backend_error_code_owned, log_budget_exhausted_http_400, log_context_window_exceeded,
|
||||
log_custom_openai_upstream_bad_request_http_400, log_provider_access_policy_denied_http_403,
|
||||
log_provider_config_rejection, publish_backend_session_expired,
|
||||
should_report_provider_http_failure,
|
||||
log_provider_config_rejection, log_provider_insufficient_credits_402,
|
||||
publish_backend_session_expired, should_report_provider_http_failure,
|
||||
};
|
||||
|
||||
pub use models::{
|
||||
|
||||
@@ -33,7 +33,13 @@ fn structured_http_4xx(msg: &str) -> Option<u16> {
|
||||
}
|
||||
|
||||
/// Check if an error is non-retryable (client errors that won't resolve with retries).
|
||||
fn is_non_retryable(err: &anyhow::Error) -> bool {
|
||||
///
|
||||
/// `pub(crate)` so other layers that run their own retry loop over a provider
|
||||
/// call (e.g. `memory_tree::extract::llm`) classify failures against the same
|
||||
/// source of truth instead of treating every error as a retryable transport
|
||||
/// blip — retrying a permanent 4xx (402 out of credits, bad key, model gone)
|
||||
/// only multiplies wasted calls and Sentry events (TAURI-RUST-C62).
|
||||
pub(crate) fn is_non_retryable(err: &anyhow::Error) -> bool {
|
||||
if is_context_window_exceeded(err) {
|
||||
return true;
|
||||
}
|
||||
@@ -760,6 +766,7 @@ impl Provider for ReliableProvider {
|
||||
messages: request.messages,
|
||||
tools: request.tools,
|
||||
stream: stream_this_attempt,
|
||||
max_tokens: request.max_tokens,
|
||||
};
|
||||
match provider.chat(req, current_model, temperature).await {
|
||||
Ok(resp) => {
|
||||
|
||||
@@ -158,6 +158,20 @@ pub struct ChatRequest<'a> {
|
||||
/// implementation ignore the sender and return only the aggregated
|
||||
/// response.
|
||||
pub stream: Option<&'a tokio::sync::mpsc::Sender<ProviderDelta>>,
|
||||
/// Optional upper bound on output tokens to request from the provider
|
||||
/// (`max_tokens` on the OpenAI-compatible wire).
|
||||
///
|
||||
/// Left `None` for open-ended generation (orchestrator, agent turns)
|
||||
/// where the model should use its full budget. Set to a small concrete
|
||||
/// value by callers whose output is bounded by construction — notably
|
||||
/// memory extraction, whose response is a tiny structured-JSON object.
|
||||
/// Beyond capping wasted generation, this stops credit-metered providers
|
||||
/// (e.g. OpenRouter) from reserving the model's *entire* output window
|
||||
/// during their pre-flight balance check: an unset `max_tokens` makes
|
||||
/// OpenRouter price the request against the full 64k+ window and 402 a
|
||||
/// low-balance BYO user who could easily afford the few thousand tokens
|
||||
/// an extraction actually needs (TAURI-RUST-C62).
|
||||
pub max_tokens: Option<u32>,
|
||||
}
|
||||
|
||||
/// A tool result to feed back to the LLM.
|
||||
@@ -410,12 +424,30 @@ pub trait Provider: Send + Sync {
|
||||
}
|
||||
|
||||
/// Structured chat API for agent loop callers.
|
||||
///
|
||||
/// **`max_tokens` caveat:** the default implementation delegates to
|
||||
/// [`Self::chat_with_history`], whose signature carries no output-token
|
||||
/// budget, so a `request.max_tokens` set by the caller is **not** honored
|
||||
/// on this path. Providers that need to enforce an output cap (e.g. the
|
||||
/// OpenAI-compatible provider, which threads it onto the wire for
|
||||
/// credit-metered backends — TAURI-RUST-C62) override `chat()` directly.
|
||||
/// The drop is logged below rather than silently swallowed; it is not a
|
||||
/// hard error because no production caller both sets `max_tokens` and
|
||||
/// routes through a default-`chat()` provider (agent turns pass `None`;
|
||||
/// memory extraction uses the compatible provider).
|
||||
async fn chat(
|
||||
&self,
|
||||
request: ChatRequest<'_>,
|
||||
model: &str,
|
||||
temperature: f64,
|
||||
) -> anyhow::Result<ChatResponse> {
|
||||
if let Some(cap) = request.max_tokens {
|
||||
log::debug!(
|
||||
"[provider] default chat() for model={model} ignores max_tokens={cap} — \
|
||||
this provider does not override chat() and chat_with_history() carries no \
|
||||
output budget; the cap will not reach the wire"
|
||||
);
|
||||
}
|
||||
let log_prompts = should_log_prompts();
|
||||
// If tools are provided but provider doesn't support native tools,
|
||||
// inject tool instructions into system prompt as fallback.
|
||||
|
||||
@@ -271,6 +271,7 @@ async fn provider_chat_prompt_guided_fallback() {
|
||||
messages: &[ChatMessage::user("Hello")],
|
||||
tools: Some(&tools),
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
|
||||
let response = provider.chat(request, "model", 0.7).await.unwrap();
|
||||
@@ -289,6 +290,7 @@ async fn provider_chat_without_tools() {
|
||||
messages: &[ChatMessage::user("Hello")],
|
||||
tools: None,
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
|
||||
let response = provider.chat(request, "model", 0.7).await.unwrap();
|
||||
@@ -390,6 +392,7 @@ async fn provider_chat_prompt_guided_preserves_existing_system_not_first() {
|
||||
],
|
||||
tools: Some(&tools),
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
|
||||
let response = provider.chat(request, "model", 0.7).await.unwrap();
|
||||
@@ -413,6 +416,7 @@ async fn provider_chat_prompt_guided_uses_convert_tools_override() {
|
||||
messages: &[ChatMessage::system("BASE"), ChatMessage::user("Hello")],
|
||||
tools: Some(&tools),
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
|
||||
let response = provider.chat(request, "model", 0.7).await.unwrap();
|
||||
@@ -436,6 +440,7 @@ async fn provider_chat_prompt_guided_rejects_non_prompt_payload() {
|
||||
messages: &[ChatMessage::user("Hello")],
|
||||
tools: Some(&tools),
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
|
||||
let err = provider.chat(request, "model", 0.7).await.unwrap_err();
|
||||
|
||||
@@ -23,6 +23,12 @@ pub struct ChatPrompt {
|
||||
pub user: String,
|
||||
pub temperature: f64,
|
||||
pub kind: &'static str,
|
||||
/// Optional output-token cap forwarded to the provider as `max_tokens`.
|
||||
/// `None` leaves generation open-ended. Memory callers with a bounded
|
||||
/// response (entity extraction) set a small value so credit-metered
|
||||
/// providers don't reserve the model's full output window in their
|
||||
/// balance pre-flight (TAURI-RUST-C62).
|
||||
pub max_tokens: Option<u32>,
|
||||
}
|
||||
|
||||
/// Pluggable LLM surface used by the memory layer.
|
||||
@@ -101,6 +107,7 @@ impl InferenceChatProvider {
|
||||
messages: &messages,
|
||||
tools: None,
|
||||
stream: None,
|
||||
max_tokens: prompt.max_tokens,
|
||||
};
|
||||
|
||||
let response = self
|
||||
@@ -318,6 +325,7 @@ mod tests {
|
||||
user: "u".into(),
|
||||
temperature: 0.0,
|
||||
kind: "test",
|
||||
max_tokens: None,
|
||||
};
|
||||
assert_eq!(p.chat_for_json(&prompt).await.unwrap(), "hello");
|
||||
assert_eq!(p.calls.load(std::sync::atomic::Ordering::SeqCst), 1);
|
||||
@@ -335,6 +343,7 @@ mod tests {
|
||||
user: "u".into(),
|
||||
temperature: 0.0,
|
||||
kind: "test",
|
||||
max_tokens: None,
|
||||
};
|
||||
let (text, usage) = p.chat_for_text_with_usage(&prompt).await.unwrap();
|
||||
assert_eq!(text, "summary text");
|
||||
|
||||
@@ -161,6 +161,7 @@ impl Provider for ChatProviderAdapter {
|
||||
user: message.to_string(),
|
||||
temperature,
|
||||
kind: "memory_smart_walk",
|
||||
max_tokens: None,
|
||||
};
|
||||
self.inner.chat_for_text(&prompt).await
|
||||
}
|
||||
|
||||
@@ -389,6 +389,7 @@ impl Provider for ChatProviderAdapter {
|
||||
user: message.to_string(),
|
||||
temperature,
|
||||
kind: "memory_tree_walk",
|
||||
max_tokens: None,
|
||||
};
|
||||
self.inner.chat_for_text(&prompt).await
|
||||
}
|
||||
|
||||
@@ -295,6 +295,7 @@ pub(super) fn handle_smart_walk(params: Map<String, Value>) -> ControllerFuture
|
||||
user: message.to_string(),
|
||||
temperature,
|
||||
kind: "memory_smart_walk_rpc",
|
||||
max_tokens: None,
|
||||
};
|
||||
self.inner.chat_for_text(&prompt).await
|
||||
}
|
||||
|
||||
@@ -37,6 +37,19 @@ use super::types::{EntityKind, ExtractedEntities, ExtractedEntity, ExtractedTopi
|
||||
use super::EntityExtractor;
|
||||
use crate::openhuman::memory::chat::{ChatPrompt, ChatProvider};
|
||||
|
||||
/// Output-token cap requested for an extraction call (`max_tokens` on the
|
||||
/// wire). An extraction response is one small structured-JSON object
|
||||
/// (an entities array + an importance float + a one-sentence reason), so a
|
||||
/// few thousand tokens is generous. The cap exists less to bound generation
|
||||
/// than to keep credit-metered providers (OpenRouter) from reserving the
|
||||
/// model's *entire* output window during their balance pre-flight: with no
|
||||
/// `max_tokens`, OpenRouter priced extraction against the full 64k+ window
|
||||
/// and returned `402 Payment Required` to any BYO user whose balance couldn't
|
||||
/// cover it, on every call (TAURI-RUST-C62). 8192 is well under any plausible
|
||||
/// extraction output yet small enough to clear the pre-flight for a
|
||||
/// low-balance account.
|
||||
const EXTRACTION_MAX_OUTPUT_TOKENS: u32 = 8192;
|
||||
|
||||
// ── Configuration ────────────────────────────────────────────────────────
|
||||
|
||||
/// Configuration for [`LlmEntityExtractor`].
|
||||
@@ -123,6 +136,7 @@ impl LlmEntityExtractor {
|
||||
user: format!("Text:\n{text}\n\nReturn JSON only."),
|
||||
temperature: 0.0,
|
||||
kind: "memory_tree::extract",
|
||||
max_tokens: Some(EXTRACTION_MAX_OUTPUT_TOKENS),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -149,15 +163,16 @@ impl EntityExtractor for LlmEntityExtractor {
|
||||
|
||||
for attempt in 0..MAX_ATTEMPTS {
|
||||
match self.try_extract(text).await {
|
||||
Some(extracted) => {
|
||||
// #3365: the structure-degraded latch means "the extraction
|
||||
// model is timing out / unreachable" — set ONLY after
|
||||
// MAX_ATTEMPTS transport failures (below). A *completed* call
|
||||
// disproves that: `try_extract` returns `Some` whenever the
|
||||
// provider responded, including the valid-but-empty and
|
||||
// malformed-JSON-as-empty cases. So a completed extraction
|
||||
// clears the latch regardless of whether this particular text
|
||||
// yielded entities — liveness, not content, is what it tracks.
|
||||
AttemptOutcome::Done(extracted) => {
|
||||
// #3365 + #002 (T013): the structure-degraded latch means
|
||||
// "the extraction model is timing out / unreachable" — set
|
||||
// ONLY after MAX_ATTEMPTS transport failures (below). A
|
||||
// *completed* call disproves that: `try_extract` returns
|
||||
// `Done` whenever the provider responded, including the
|
||||
// valid-but-empty and malformed-JSON-as-empty cases. So a
|
||||
// completed extraction clears the latch regardless of whether
|
||||
// this particular text yielded entities — liveness, not
|
||||
// content, is what it tracks.
|
||||
//
|
||||
// (Clearing only on a non-empty result left the latch stuck
|
||||
// after the model recovered whenever later texts were
|
||||
@@ -166,9 +181,29 @@ impl EntityExtractor for LlmEntityExtractor {
|
||||
crate::openhuman::memory_tree::health::clear_structure_degraded();
|
||||
return Ok(extracted);
|
||||
}
|
||||
None => {
|
||||
// Transport failure. Retry with exponential backoff
|
||||
// unless we've exhausted attempts.
|
||||
AttemptOutcome::Permanent(code) => {
|
||||
// Non-retryable client error — e.g. a 402 (the BYO
|
||||
// provider account is out of credits), a rejected API
|
||||
// key, or a model that no longer exists. Retrying
|
||||
// reproduces the identical error and, on a credit-metered
|
||||
// provider, fires one more Sentry event per attempt
|
||||
// (TAURI-RUST-C62: 12k events from a single user). Skip
|
||||
// the backoff loop entirely and fall back to the same
|
||||
// degraded-but-empty result the exhausted-retry path
|
||||
// returns — but tag the cause precisely so doctor/status
|
||||
// can steer the user ("top up credits" / "fix the key")
|
||||
// instead of "extraction is timing out".
|
||||
log::warn!(
|
||||
"[memory_tree::extract::llm] non-retryable provider failure ({}) — \
|
||||
returning empty extraction without retry (structure degraded)",
|
||||
code.as_str()
|
||||
);
|
||||
crate::openhuman::memory_tree::health::mark_structure_degraded(code);
|
||||
return Ok(ExtractedEntities::default());
|
||||
}
|
||||
AttemptOutcome::Retryable => {
|
||||
// Transport / transient failure. Retry with exponential
|
||||
// backoff unless we've exhausted attempts.
|
||||
if attempt + 1 < MAX_ATTEMPTS {
|
||||
let delay_ms = BASE_BACKOFF_MS * 2u64.pow(attempt);
|
||||
log::warn!(
|
||||
@@ -202,17 +237,29 @@ impl EntityExtractor for LlmEntityExtractor {
|
||||
}
|
||||
}
|
||||
|
||||
/// Outcome of a single [`LlmEntityExtractor::try_extract`] attempt.
|
||||
///
|
||||
/// Splits the old `Option` ("`None` = retry") into three cases so the retry
|
||||
/// loop can tell a transient blip apart from a permanent client error and
|
||||
/// stop hammering the latter (TAURI-RUST-C62).
|
||||
enum AttemptOutcome {
|
||||
/// The call completed (provider returned content). Includes the
|
||||
/// "malformed wrong-shape JSON → empty" case, because re-sending the
|
||||
/// same input won't change the response.
|
||||
Done(ExtractedEntities),
|
||||
/// Transport / transient failure (unreachable backend, 5xx, truncated
|
||||
/// mid-JSON). Retrying may help.
|
||||
Retryable,
|
||||
/// Permanent, non-retryable provider failure (4xx client error: 402 out
|
||||
/// of credits, rejected key, model gone). Retrying reproduces the same
|
||||
/// error. Carries the typed [`FailureCode`] to record on the degraded
|
||||
/// signal so doctor/status can steer the user.
|
||||
Permanent(crate::openhuman::memory_tree::health::FailureCode),
|
||||
}
|
||||
|
||||
impl LlmEntityExtractor {
|
||||
/// Internal: one attempt at calling the chat provider.
|
||||
///
|
||||
/// Returns:
|
||||
/// - `Some(extracted)` — call completed (provider returned content).
|
||||
/// Includes the "malformed JSON" case which returns `Some(empty)`
|
||||
/// because retrying the same input won't help.
|
||||
/// - `None` — transport-level / provider-level failure where retrying
|
||||
/// might help (e.g. unreachable backend, transient HTTP 5xx). Caller
|
||||
/// may retry.
|
||||
async fn try_extract(&self, text: &str) -> Option<ExtractedEntities> {
|
||||
async fn try_extract(&self, text: &str) -> AttemptOutcome {
|
||||
let prompt = self.build_prompt(text);
|
||||
log::debug!(
|
||||
"[memory_tree::extract::llm] chat provider={} model={} text_chars={}",
|
||||
@@ -224,11 +271,23 @@ impl LlmEntityExtractor {
|
||||
let raw = match self.provider.chat_for_json(&prompt).await {
|
||||
Ok(v) => v,
|
||||
Err(e) => {
|
||||
// Classify against the provider stack's single source of
|
||||
// truth. A non-retryable client error (402/401/403/404/…)
|
||||
// must not be retried — that is the 12k-event amplifier in
|
||||
// TAURI-RUST-C62. Only genuine transport/transient failures
|
||||
// earn the backoff loop.
|
||||
if crate::openhuman::inference::provider::reliable::is_non_retryable(&e) {
|
||||
log::warn!(
|
||||
"[memory_tree::extract::llm] chat provider={} permanently failed: {e:#}",
|
||||
self.provider.name()
|
||||
);
|
||||
return AttemptOutcome::Permanent(permanent_failure_code(&e));
|
||||
}
|
||||
log::warn!(
|
||||
"[memory_tree::extract::llm] chat provider={} failed: {e:#}",
|
||||
self.provider.name()
|
||||
);
|
||||
return None;
|
||||
return AttemptOutcome::Retryable;
|
||||
}
|
||||
};
|
||||
log::debug!(
|
||||
@@ -243,15 +302,15 @@ impl LlmEntityExtractor {
|
||||
// Truncated mid-JSON: the response stream closed before the
|
||||
// closing brace (token budget or a server-side timeout cut
|
||||
// it short). Unlike a wrong-shape body, this is transient —
|
||||
// signal a retryable failure (`None`) so `extract`'s backoff
|
||||
// loop tries again rather than silently dropping the whole
|
||||
// chunk (bug-report-2026-05-26 I1).
|
||||
// signal a retryable failure so `extract`'s backoff loop
|
||||
// tries again rather than silently dropping the whole chunk
|
||||
// (bug-report-2026-05-26 I1).
|
||||
log::warn!(
|
||||
"[memory_tree::extract::llm] LLM response truncated mid-JSON ({e}); \
|
||||
response_bytes={} — retrying",
|
||||
raw.len()
|
||||
);
|
||||
return None;
|
||||
return AttemptOutcome::Retryable;
|
||||
}
|
||||
Err(e) => {
|
||||
log::warn!(
|
||||
@@ -259,11 +318,46 @@ impl LlmEntityExtractor {
|
||||
response: {e}; content was: {} — returning empty extraction",
|
||||
truncate_for_log(&raw, 400)
|
||||
);
|
||||
return Some(ExtractedEntities::default());
|
||||
return AttemptOutcome::Done(ExtractedEntities::default());
|
||||
}
|
||||
};
|
||||
|
||||
Some(parsed.into_extracted_entities(text, &self.cfg))
|
||||
AttemptOutcome::Done(parsed.into_extracted_entities(text, &self.cfg))
|
||||
}
|
||||
}
|
||||
|
||||
/// Map a permanent (non-retryable) extraction provider error to the most
|
||||
/// honest *existing* [`FailureCode`], so the degraded-structure signal steers
|
||||
/// the user without minting a new variant (and its 14-locale remediation key).
|
||||
///
|
||||
/// - 402 / "payment required" / "requires more credits" / "insufficient" →
|
||||
/// [`FailureCode::BudgetExhausted`] (its remediation already reads "out of
|
||||
/// budget; bring your own key or top up — retrying won't help"), the
|
||||
/// dominant TAURI-RUST-C62 case.
|
||||
/// - 401 / 403 / auth-key rejection → [`FailureCode::AuthInvalid`].
|
||||
/// - any other permanent failure → [`FailureCode::ExtractionTimeout`], the
|
||||
/// existing "extraction model isn't producing" bucket.
|
||||
fn permanent_failure_code(
|
||||
err: &anyhow::Error,
|
||||
) -> crate::openhuman::memory_tree::health::FailureCode {
|
||||
use crate::openhuman::memory_tree::health::FailureCode;
|
||||
let lower = format!("{err:#}").to_lowercase();
|
||||
if lower.contains("402")
|
||||
|| lower.contains("payment required")
|
||||
|| lower.contains("requires more credits")
|
||||
|| lower.contains("insufficient")
|
||||
{
|
||||
FailureCode::BudgetExhausted
|
||||
} else if lower.contains("401")
|
||||
|| lower.contains("403")
|
||||
|| lower.contains("unauthorized")
|
||||
|| lower.contains("forbidden")
|
||||
|| lower.contains("invalid api key")
|
||||
|| lower.contains("incorrect api key")
|
||||
{
|
||||
FailureCode::AuthInvalid
|
||||
} else {
|
||||
FailureCode::ExtractionTimeout
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -565,3 +565,132 @@ async fn extract_does_not_retry_on_wrong_shape_response() {
|
||||
"wrong-shape response should yield an empty extraction"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_prompt_sets_extraction_max_tokens_cap() {
|
||||
// Extraction must cap output tokens so a credit-metered provider
|
||||
// (OpenRouter) prices the request against a realistic budget instead of
|
||||
// the model's full output window — an unset cap is what 402'd low-balance
|
||||
// BYO users on every call (TAURI-RUST-C62). build_prompt is the single
|
||||
// source of that cap.
|
||||
use crate::openhuman::memory::chat::{ChatPrompt, ChatProvider};
|
||||
use async_trait::async_trait;
|
||||
use std::sync::Arc;
|
||||
|
||||
struct NoopProvider;
|
||||
#[async_trait]
|
||||
impl ChatProvider for NoopProvider {
|
||||
fn name(&self) -> &str {
|
||||
"test:noop"
|
||||
}
|
||||
async fn chat_for_json(&self, _p: &ChatPrompt) -> anyhow::Result<String> {
|
||||
Ok("{}".into())
|
||||
}
|
||||
}
|
||||
|
||||
let ex = LlmEntityExtractor::new(LlmExtractorConfig::default(), Arc::new(NoopProvider));
|
||||
let prompt = ex.build_prompt("hello");
|
||||
assert_eq!(prompt.max_tokens, Some(EXTRACTION_MAX_OUTPUT_TOKENS));
|
||||
// Lock the value: it must stay well under any plausible per-account
|
||||
// OpenRouter balance so the pre-flight clears for a low-balance user.
|
||||
assert_eq!(EXTRACTION_MAX_OUTPUT_TOKENS, 8192);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn extract_does_not_retry_on_permanent_402() {
|
||||
// A 402 (the BYO provider account is out of credits) is a permanent client
|
||||
// error: retrying reproduces it and fires one more Sentry event per attempt
|
||||
// (TAURI-RUST-C62: 12k events from a single user). extract() must call the
|
||||
// provider exactly once, return empty, and mark structure degraded with the
|
||||
// credits-exhausted cause (not extraction-timeout).
|
||||
let _health_guard = crate::openhuman::memory_tree::health::test_guard();
|
||||
use crate::openhuman::memory::chat::{ChatPrompt, ChatProvider};
|
||||
use async_trait::async_trait;
|
||||
use std::sync::atomic::{AtomicUsize, Ordering};
|
||||
use std::sync::Arc;
|
||||
|
||||
struct InsufficientCreditsProvider {
|
||||
calls: AtomicUsize,
|
||||
}
|
||||
#[async_trait]
|
||||
impl ChatProvider for InsufficientCreditsProvider {
|
||||
fn name(&self) -> &str {
|
||||
"test:402"
|
||||
}
|
||||
async fn chat_for_json(&self, _p: &ChatPrompt) -> anyhow::Result<String> {
|
||||
self.calls.fetch_add(1, Ordering::SeqCst);
|
||||
// Verbatim shape of the TAURI-RUST-C62 provider error.
|
||||
Err(anyhow::anyhow!(
|
||||
"myopenrouter API error (402 Payment Required): This request requires more \
|
||||
credits, or fewer max_tokens. You requested up to 65536 tokens, but can only \
|
||||
afford 49732."
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
let mock = Arc::new(InsufficientCreditsProvider {
|
||||
calls: AtomicUsize::new(0),
|
||||
});
|
||||
let ex = LlmEntityExtractor::new(LlmExtractorConfig::default(), mock.clone());
|
||||
let out = ex.extract("some text").await.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
mock.calls.load(Ordering::SeqCst),
|
||||
1,
|
||||
"a permanent 402 must not be retried"
|
||||
);
|
||||
assert!(out.entities.is_empty());
|
||||
|
||||
let state = crate::openhuman::memory_tree::health::current_degraded_state();
|
||||
assert!(
|
||||
state.structure,
|
||||
"a permanent extraction failure must still mark structure degraded"
|
||||
);
|
||||
assert_eq!(
|
||||
state.cause.expect("degraded cause present").code,
|
||||
crate::openhuman::memory_tree::health::FailureCode::BudgetExhausted,
|
||||
"a 402 must map to the credits-exhausted cause, not extraction-timeout"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn extract_retries_transient_provider_error() {
|
||||
// The de-amplification fix must only short-circuit *permanent* errors. A
|
||||
// transport/transient failure (no 4xx, no auth marker) must still exhaust
|
||||
// the retry budget before falling back, or a flaky-network blip would drop
|
||||
// the chunk on the first try.
|
||||
let _health_guard = crate::openhuman::memory_tree::health::test_guard();
|
||||
use crate::openhuman::memory::chat::{ChatPrompt, ChatProvider};
|
||||
use async_trait::async_trait;
|
||||
use std::sync::atomic::{AtomicUsize, Ordering};
|
||||
use std::sync::Arc;
|
||||
|
||||
struct TransientProvider {
|
||||
calls: AtomicUsize,
|
||||
}
|
||||
#[async_trait]
|
||||
impl ChatProvider for TransientProvider {
|
||||
fn name(&self) -> &str {
|
||||
"test:transient"
|
||||
}
|
||||
async fn chat_for_json(&self, _p: &ChatPrompt) -> anyhow::Result<String> {
|
||||
self.calls.fetch_add(1, Ordering::SeqCst);
|
||||
Err(anyhow::anyhow!(
|
||||
"error sending request for url (https://api): connection refused"
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
let mock = Arc::new(TransientProvider {
|
||||
calls: AtomicUsize::new(0),
|
||||
});
|
||||
let ex = LlmEntityExtractor::new(LlmExtractorConfig::default(), mock.clone());
|
||||
let out = ex.extract("some text").await.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
mock.calls.load(Ordering::SeqCst),
|
||||
3,
|
||||
"a transient error must still exhaust the retry budget"
|
||||
);
|
||||
assert!(out.entities.is_empty());
|
||||
}
|
||||
|
||||
@@ -120,6 +120,12 @@ pub async fn summarise(
|
||||
user: body,
|
||||
temperature: 0.0,
|
||||
kind: "memory_tree::summarise",
|
||||
// Left open-ended: the summariser's output budget varies by node
|
||||
// level (up to ~20k tokens at root) and is enforced prompt-side, so
|
||||
// a hard wire cap risks truncating a valid large summary. The 402
|
||||
// precheck fix is scoped to extraction (TAURI-RUST-C62); revisit if
|
||||
// the summarise path shows the same low-balance 402 shape.
|
||||
max_tokens: None,
|
||||
};
|
||||
|
||||
log::debug!(
|
||||
|
||||
@@ -592,6 +592,7 @@ async fn tools_present_forces_remote_even_when_local_healthy_and_lightweight() {
|
||||
messages: &messages,
|
||||
tools: Some(&tools),
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
};
|
||||
|
||||
r.chat(request, "hint:reaction", 0.7).await.unwrap();
|
||||
|
||||
@@ -2183,6 +2183,7 @@ async fn inference_provider_trait_defaults_cover_prompt_guided_paths() {
|
||||
messages: &[ChatMessage::user("need docs")],
|
||||
tools: Some(&[tool_spec.clone()]),
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
},
|
||||
"agentic-v1",
|
||||
0.4,
|
||||
@@ -2198,6 +2199,7 @@ async fn inference_provider_trait_defaults_cover_prompt_guided_paths() {
|
||||
messages: &[ChatMessage::user("plain")],
|
||||
tools: None,
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
},
|
||||
"agentic-v1",
|
||||
0.5,
|
||||
@@ -2283,6 +2285,7 @@ async fn inference_openai_compatible_provider_covers_native_streaming_and_fallba
|
||||
],
|
||||
tools: Some(&[tool_spec.clone(), tool_spec.clone()]),
|
||||
stream: Some(&delta_tx),
|
||||
max_tokens: None,
|
||||
},
|
||||
"stream-native",
|
||||
0.9,
|
||||
@@ -2322,6 +2325,7 @@ async fn inference_openai_compatible_provider_covers_native_streaming_and_fallba
|
||||
messages: &[ChatMessage::user("json encoded tool call")],
|
||||
tools: None,
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
},
|
||||
"tool-content-json",
|
||||
0.2,
|
||||
@@ -4429,6 +4433,7 @@ async fn inference_router_provider_covers_hint_tier_and_passthrough_routing() {
|
||||
messages: &[ChatMessage::user("fallback")],
|
||||
tools: None,
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
},
|
||||
"reasoning-v1",
|
||||
0.4,
|
||||
@@ -4548,6 +4553,7 @@ async fn inference_reliable_provider_covers_retry_fallback_and_aggregate_errors(
|
||||
messages: &[ChatMessage::user("fail")],
|
||||
tools: None,
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
},
|
||||
"missing-model",
|
||||
0.0,
|
||||
|
||||
@@ -141,6 +141,7 @@ async fn compatible_native_leftovers_cover_tool_history_function_call_and_stream
|
||||
],
|
||||
tools: Some(&tools),
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
},
|
||||
"function-call-model",
|
||||
0.4,
|
||||
@@ -165,6 +166,7 @@ async fn compatible_native_leftovers_cover_tool_history_function_call_and_stream
|
||||
messages: &[ChatMessage::user("content json")],
|
||||
tools: Some(&tools),
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
},
|
||||
"content-json-tools",
|
||||
0.4,
|
||||
@@ -186,6 +188,7 @@ async fn compatible_native_leftovers_cover_tool_history_function_call_and_stream
|
||||
messages: &[ChatMessage::user("stream ordering")],
|
||||
tools: Some(&tools),
|
||||
stream: Some(&tx),
|
||||
max_tokens: None,
|
||||
},
|
||||
"stream-out-of-order-tool",
|
||||
0.4,
|
||||
|
||||
@@ -154,6 +154,7 @@ async fn openai_compatible_matrix_covers_auth_requests_responses_and_streaming()
|
||||
],
|
||||
tools: Some(&tools),
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
},
|
||||
"native-tools",
|
||||
0.2,
|
||||
@@ -181,6 +182,7 @@ async fn openai_compatible_matrix_covers_auth_requests_responses_and_streaming()
|
||||
messages: &[ChatMessage::user("stream")],
|
||||
tools: Some(&tools),
|
||||
stream: Some(&tx),
|
||||
max_tokens: None,
|
||||
},
|
||||
"stream-sse",
|
||||
0.3,
|
||||
@@ -212,6 +214,7 @@ async fn openai_compatible_matrix_covers_auth_requests_responses_and_streaming()
|
||||
messages: &[ChatMessage::user("json stream")],
|
||||
tools: None,
|
||||
stream: Some(&json_tx),
|
||||
max_tokens: None,
|
||||
},
|
||||
"stream-json",
|
||||
0.3,
|
||||
@@ -229,6 +232,7 @@ async fn openai_compatible_matrix_covers_auth_requests_responses_and_streaming()
|
||||
messages: &[ChatMessage::user("retry without tools")],
|
||||
tools: Some(&tools),
|
||||
stream: Some(&retry_tx),
|
||||
max_tokens: None,
|
||||
},
|
||||
"stream-tools-unsupported",
|
||||
0.3,
|
||||
@@ -350,6 +354,7 @@ async fn compatible_error_matrix_covers_status_malformed_and_no_fallback_paths()
|
||||
messages: &[ChatMessage::user("stream fail")],
|
||||
tools: None,
|
||||
stream: Some(&sse_tx),
|
||||
max_tokens: None,
|
||||
},
|
||||
"stream-status-error",
|
||||
0.1,
|
||||
@@ -407,6 +412,7 @@ async fn ollama_compatible_matrix_covers_authless_chat_and_streaming_errors() {
|
||||
messages: &[ChatMessage::user("ollama stream")],
|
||||
tools: None,
|
||||
stream: Some(&tx),
|
||||
max_tokens: None,
|
||||
},
|
||||
"ollama-stream",
|
||||
0.0,
|
||||
|
||||
@@ -151,6 +151,7 @@ async fn compatible_provider_covers_retry_headers_responses_and_parse_errors() {
|
||||
],
|
||||
tools: Some(&tools),
|
||||
stream: Some(&tx),
|
||||
max_tokens: None,
|
||||
},
|
||||
"stream-tools-unsupported",
|
||||
0.2,
|
||||
|
||||
@@ -412,6 +412,7 @@ async fn reliable_provider_covers_chat_tools_streaming_and_context_bail_edges()
|
||||
messages: &[ChatMessage::user("retry me")],
|
||||
tools: None,
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
},
|
||||
"retry-model",
|
||||
0.2,
|
||||
|
||||
@@ -112,6 +112,7 @@ async fn compatible_provider_covers_chat_responses_streaming_tools_and_errors()
|
||||
messages: &[ChatMessage::user("use a tool")],
|
||||
tools: Some(&tools),
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
},
|
||||
"tool-model",
|
||||
0.1,
|
||||
@@ -135,6 +136,7 @@ async fn compatible_provider_covers_chat_responses_streaming_tools_and_errors()
|
||||
messages: &[ChatMessage::user("stream it")],
|
||||
tools: Some(&tools),
|
||||
stream: Some(&tx),
|
||||
max_tokens: None,
|
||||
},
|
||||
"stream-model",
|
||||
0.1,
|
||||
|
||||
@@ -111,6 +111,7 @@ async fn compatible_streaming_covers_tool_deltas_json_fallback_and_retry_without
|
||||
messages: &messages,
|
||||
tools: Some(&tools),
|
||||
stream: Some(&delta_tx),
|
||||
max_tokens: None,
|
||||
},
|
||||
"stream-tools",
|
||||
0.4,
|
||||
@@ -154,6 +155,7 @@ async fn compatible_streaming_covers_tool_deltas_json_fallback_and_retry_without
|
||||
messages: &[ChatMessage::user("json stream fallback")],
|
||||
tools: None,
|
||||
stream: Some(&json_tx),
|
||||
max_tokens: None,
|
||||
},
|
||||
"json-stream",
|
||||
0.2,
|
||||
@@ -170,6 +172,7 @@ async fn compatible_streaming_covers_tool_deltas_json_fallback_and_retry_without
|
||||
messages: &[ChatMessage::user("retry without tools")],
|
||||
tools: Some(&[tool_spec("lookup")]),
|
||||
stream: Some(&retry_tx),
|
||||
max_tokens: None,
|
||||
},
|
||||
"tool-retry",
|
||||
0.2,
|
||||
|
||||
@@ -362,6 +362,7 @@ async fn openai_compatible_provider_covers_auth_temperature_tool_fallback_and_re
|
||||
messages: &messages,
|
||||
tools: Some(&[tool.clone(), tool]),
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
},
|
||||
"gpt-5-mini",
|
||||
0.6,
|
||||
@@ -383,6 +384,7 @@ async fn openai_compatible_provider_covers_auth_temperature_tool_fallback_and_re
|
||||
messages: &messages,
|
||||
tools: None,
|
||||
stream: None,
|
||||
max_tokens: None,
|
||||
},
|
||||
"gpt-5-mini",
|
||||
0.6,
|
||||
@@ -450,6 +452,7 @@ async fn openai_compatible_provider_streaming_json_fallback_aggregates_response(
|
||||
messages: &messages,
|
||||
tools: None,
|
||||
stream: Some(&tx),
|
||||
max_tokens: None,
|
||||
},
|
||||
"stream-model",
|
||||
0.7,
|
||||
|
||||
Reference in New Issue
Block a user