mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(triage): defer instead of error on budget-exceeded (OPENHUMAN-TAURI-X) (#1558)
Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
This commit is contained in:
co-authored by
Steven Enamakel
parent
f82f2d524d
commit
149f98272d
@@ -206,10 +206,27 @@ where
|
||||
F: FnOnce() -> Fut,
|
||||
Fut: Future<Output = Option<LlmPermit>>,
|
||||
{
|
||||
// Track whether the cloud arm bailed because of user budget so the
|
||||
// eventual Deferred reason explains *why* we're sitting idle rather
|
||||
// than the generic "both arms failed" copy.
|
||||
let mut cloud_budget_exhausted: Option<anyhow::Error> = None;
|
||||
|
||||
// ── Cloud arm ──────────────────────────────────────────────────
|
||||
match try_arm(&cloud, envelope, TriageResolutionPath::Cloud).await {
|
||||
Ok(run) => return Ok(TriageOutcome::Decision(run)),
|
||||
Err(ArmError::Fatal(err)) => return Err(err),
|
||||
Err(ArmError::BudgetExhausted(err)) => {
|
||||
tracing::warn!(
|
||||
source = %envelope.source.slug(),
|
||||
label = %envelope.display_label,
|
||||
external_id = %envelope.external_id,
|
||||
path = TriageResolutionPath::Cloud.as_str(),
|
||||
error = %err,
|
||||
"[triage::evaluator] cloud rejected for budget; \
|
||||
skipping retry and falling back to local arm"
|
||||
);
|
||||
cloud_budget_exhausted = Some(err);
|
||||
}
|
||||
Err(ArmError::Retryable { retry_after_ms, .. }) => {
|
||||
// Sleep before the cloud retry. Honour Retry-After when
|
||||
// present; otherwise use a short backoff so the second
|
||||
@@ -228,6 +245,18 @@ where
|
||||
match try_arm(&cloud, envelope, TriageResolutionPath::CloudAfterRetry).await {
|
||||
Ok(run) => return Ok(TriageOutcome::Decision(run)),
|
||||
Err(ArmError::Fatal(err)) => return Err(err),
|
||||
Err(ArmError::BudgetExhausted(err)) => {
|
||||
tracing::warn!(
|
||||
source = %envelope.source.slug(),
|
||||
label = %envelope.display_label,
|
||||
external_id = %envelope.external_id,
|
||||
path = TriageResolutionPath::CloudAfterRetry.as_str(),
|
||||
error = %err,
|
||||
"[triage::evaluator] cloud rejected for budget on retry; \
|
||||
falling back to local arm"
|
||||
);
|
||||
cloud_budget_exhausted = Some(err);
|
||||
}
|
||||
Err(ArmError::Retryable { .. }) => {
|
||||
// Exhausted cloud budget — fall through to local.
|
||||
tracing::warn!(
|
||||
@@ -244,9 +273,28 @@ where
|
||||
// No local arm available at all (runtime disabled, no model
|
||||
// configured) — the only honest outcome is a deferral so the
|
||||
// next tick retries the whole chain.
|
||||
//
|
||||
// `reason` is part of `TriageOutcome::Deferred` and may be
|
||||
// forwarded into telemetry / UI, so it must stay a stable,
|
||||
// scrubbed string. Raw upstream error text goes to the debug
|
||||
// log instead, where it is operator-visible but not surfaced.
|
||||
let reason = match &cloud_budget_exhausted {
|
||||
Some(err) => {
|
||||
tracing::debug!(
|
||||
target: "[triage::evaluator]",
|
||||
source = %envelope.source.slug(),
|
||||
label = %envelope.display_label,
|
||||
external_id = %envelope.external_id,
|
||||
error = %err,
|
||||
"cloud budget exhausted; no local arm — full upstream error"
|
||||
);
|
||||
"cloud budget exhausted; local arm unavailable".to_string()
|
||||
}
|
||||
None => "cloud retry exhausted; local arm unavailable".to_string(),
|
||||
};
|
||||
return Ok(TriageOutcome::Deferred {
|
||||
defer_until_ms: now_ms().saturating_add(DEFER_WAKEUP_MS),
|
||||
reason: "cloud retry exhausted; local arm unavailable".to_string(),
|
||||
reason,
|
||||
});
|
||||
};
|
||||
|
||||
@@ -256,15 +304,34 @@ where
|
||||
|
||||
match try_arm(&local, envelope, TriageResolutionPath::LocalFallback).await {
|
||||
Ok(run) => Ok(TriageOutcome::Decision(run)),
|
||||
Err(ArmError::Fatal(err)) | Err(ArmError::Retryable { source: err, .. }) => {
|
||||
Err(ArmError::Fatal(err))
|
||||
| Err(ArmError::BudgetExhausted(err))
|
||||
| Err(ArmError::Retryable { source: err, .. }) => {
|
||||
// Local also failed — defer rather than surface a hard
|
||||
// error. Today's "hard fail" is the wrong default for a
|
||||
// transient blocker per #1257.
|
||||
let reason = format!("cloud + local both failed: {err}");
|
||||
//
|
||||
// `reason` is part of the public Deferred outcome and may
|
||||
// flow into telemetry / UI, so keep it scrubbed. Raw error
|
||||
// text from cloud + local lives in the structured warn
|
||||
// fields below — visible to operators, not callers.
|
||||
let reason = match cloud_budget_exhausted.as_ref() {
|
||||
Some(_) => "cloud budget exhausted; local arm also failed".to_string(),
|
||||
None => "cloud retry exhausted; local arm also failed".to_string(),
|
||||
};
|
||||
tracing::warn!(
|
||||
error = %reason,
|
||||
target: "[triage::evaluator]",
|
||||
source = %envelope.source.slug(),
|
||||
label = %envelope.display_label,
|
||||
external_id = %envelope.external_id,
|
||||
local_error = %err,
|
||||
cloud_error = cloud_budget_exhausted
|
||||
.as_ref()
|
||||
.map(|e| e.to_string())
|
||||
.unwrap_or_default(),
|
||||
defer_ms = DEFER_WAKEUP_MS,
|
||||
"[triage::evaluator] both arms failed; deferring"
|
||||
reason = %reason,
|
||||
"both arms failed; deferring"
|
||||
);
|
||||
Ok(TriageOutcome::Deferred {
|
||||
defer_until_ms: now_ms().saturating_add(DEFER_WAKEUP_MS),
|
||||
@@ -287,6 +354,13 @@ enum ArmError {
|
||||
/// Auth failure, missing model, prompt parse error, registry
|
||||
/// missing, etc. — retry / fallback would not change the result.
|
||||
Fatal(anyhow::Error),
|
||||
/// Cloud upstream rejected the call because the user is out of
|
||||
/// budget / credits. Retrying the cloud arm would just burn the
|
||||
/// same wall, but the local arm has no upstream cost — so we
|
||||
/// skip cloud retry, try local, and defer if local also fails.
|
||||
/// This is **not** a fatal error: the user takes an explicit
|
||||
/// action (top up) to fix it, so it must not page Sentry.
|
||||
BudgetExhausted(anyhow::Error),
|
||||
}
|
||||
|
||||
/// Run a single arm: dispatch the agent turn through the native bus
|
||||
@@ -438,9 +512,61 @@ fn classify_error(message: String) -> ArmError {
|
||||
source: err,
|
||||
};
|
||||
}
|
||||
// Budget-exceeded is technically a 400 (not 5xx/429), so the
|
||||
// generic transient checks above won't catch it — but it is a
|
||||
// user-actionable upstream blocker, not a code bug, so we route
|
||||
// it through `BudgetExhausted` to avoid Sentry pages.
|
||||
if is_inference_budget_exceeded(&message) {
|
||||
return ArmError::BudgetExhausted(err);
|
||||
}
|
||||
ArmError::Fatal(err)
|
||||
}
|
||||
|
||||
/// Returns `true` when `message` signals that the upstream rejected the
|
||||
/// call because the user's inference budget or credit balance is empty —
|
||||
/// meaning a retry would hit the same wall.
|
||||
///
|
||||
/// The vocabulary matches the OpenHuman backend's error copy and common
|
||||
/// third-party provider phrasing. It does **not** mirror the
|
||||
/// *semantics* of `channels/providers/web.rs` (a different code path);
|
||||
/// it is an independent, conservative allowlist evaluated inline so the
|
||||
/// triage evaluator carries no cross-domain import.
|
||||
///
|
||||
/// Kept conservative on purpose: a false positive would silently
|
||||
/// reclassify a real `Fatal` error as `BudgetExhausted`, hiding it from
|
||||
/// Sentry.
|
||||
fn is_inference_budget_exceeded(message: &str) -> bool {
|
||||
// Normalize: lowercase, replace non-alphanumeric with spaces, then
|
||||
// split into whitespace-separated tokens. This lets us do
|
||||
// whole-word matching: a raw `contains("top up")` against the
|
||||
// normalized text would also fire on "stop updating" (which
|
||||
// contains the substring "top up" across word boundaries).
|
||||
let normalized: String = message
|
||||
.trim()
|
||||
.to_ascii_lowercase()
|
||||
.chars()
|
||||
.map(|c| if c.is_ascii_alphanumeric() { c } else { ' ' })
|
||||
.collect();
|
||||
let words: Vec<&str> = normalized.split_whitespace().collect();
|
||||
const NEEDLES: &[&str] = &[
|
||||
"budget exceeded",
|
||||
"budget exceeds",
|
||||
"top up",
|
||||
"add credits",
|
||||
"out of credits",
|
||||
"no remaining credits",
|
||||
];
|
||||
NEEDLES.iter().any(|needle| {
|
||||
let needle_tokens: Vec<&str> = needle.split_whitespace().collect();
|
||||
if needle_tokens.is_empty() || words.len() < needle_tokens.len() {
|
||||
return false;
|
||||
}
|
||||
words
|
||||
.windows(needle_tokens.len())
|
||||
.any(|window| window == needle_tokens.as_slice())
|
||||
})
|
||||
}
|
||||
|
||||
/// Heuristic for transient cloud failures the provider stack didn't
|
||||
/// already classify — connection resets, timeouts, generic 5xx text.
|
||||
/// Mirrors the conservative match shape used by `is_upstream_unhealthy`.
|
||||
|
||||
@@ -102,6 +102,75 @@ fn classify_string_treats_auth_failure_as_fatal() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_string_recognises_budget_exceeded_as_budget_exhausted() {
|
||||
// Matches the real payload that fired OPENHUMAN-TAURI-X in Sentry.
|
||||
let err = classify_error(
|
||||
"OpenHuman API error (400 Bad Request): {\"success\":false,\
|
||||
\"error\":\"Budget exceeded — add credits to continue\"}"
|
||||
.to_string(),
|
||||
);
|
||||
assert!(
|
||||
matches!(err, ArmError::BudgetExhausted(_)),
|
||||
"budget-exceeded must classify as BudgetExhausted, not Fatal (which pages Sentry)"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_string_recognises_budget_exceeds_your_limit() {
|
||||
// Exercises the "budget exceeds" needle added to NEEDLES as a
|
||||
// grammatically-correct variant of "budget exceeded" (past
|
||||
// tense vs. present tense) — matches e.g. "Your budget exceeds
|
||||
// your limit for this billing period."
|
||||
let err = classify_error("Your budget exceeds your limit for this billing period.".to_string());
|
||||
assert!(
|
||||
matches!(err, ArmError::BudgetExhausted(_)),
|
||||
"\"budget exceeds\" must classify as BudgetExhausted"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_string_does_not_match_budget_phrases_across_word_boundaries() {
|
||||
// Regression: a substring-based check would fire BudgetExhausted
|
||||
// on "stop updating" because the normalized text contains the
|
||||
// substring "top up" — across the boundary between "stop" and
|
||||
// "updating". Whole-word (token-window) matching prevents this.
|
||||
for msg in [
|
||||
"please stop updating the row",
|
||||
"stop updating now",
|
||||
"topup completed",
|
||||
] {
|
||||
let err = classify_error(msg.to_string());
|
||||
assert!(
|
||||
matches!(err, ArmError::Fatal(_)),
|
||||
"expected Fatal (no spurious BudgetExhausted) for {msg:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_string_recognises_top_up_and_out_of_credits_as_budget_exhausted() {
|
||||
for msg in [
|
||||
"please top up your account",
|
||||
"you are out of credits, add credits to continue",
|
||||
"no remaining credits available",
|
||||
] {
|
||||
let err = classify_error(msg.to_string());
|
||||
// Match by reference so `err` is only inspected (not moved) —
|
||||
// lets us reuse it for both the match-check and the failure
|
||||
// label without a double-move.
|
||||
let label = match &err {
|
||||
ArmError::Retryable { .. } => "Retryable",
|
||||
ArmError::Fatal(_) => "Fatal",
|
||||
ArmError::BudgetExhausted(_) => "BudgetExhausted",
|
||||
};
|
||||
assert!(
|
||||
matches!(&err, ArmError::BudgetExhausted(_)),
|
||||
"expected BudgetExhausted for {msg:?}, got {label}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// ── Tiered fallback integration tests ───────────────────────────
|
||||
//
|
||||
// These drive `run_triage_with_arms` end-to-end through the agent
|
||||
@@ -303,7 +372,7 @@ async fn cloud_then_local_failure_returns_deferred() {
|
||||
"defer_until_ms must be in the future"
|
||||
);
|
||||
assert!(
|
||||
reason.to_lowercase().contains("503") || reason.contains("cloud"),
|
||||
reason.contains("cloud retry exhausted"),
|
||||
"reason should reference the upstream failure: {reason}"
|
||||
);
|
||||
}
|
||||
@@ -343,6 +412,151 @@ async fn fatal_cloud_error_short_circuits_without_local_attempt() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cloud_budget_exhausted_skips_retry_and_falls_to_local() {
|
||||
// Regression for OPENHUMAN-TAURI-X: when the cloud arm returns
|
||||
// "Budget exceeded — add credits to continue" we must not retry
|
||||
// the cloud arm (the second call would burn the same wall) and
|
||||
// we must not surface the error as Fatal (that paged Sentry).
|
||||
AgentDefinitionRegistry::init_global_builtins().expect("init_global_builtins");
|
||||
let counter = StdArc::new(AtomicUsize::new(0));
|
||||
let counter_for_stub = StdArc::clone(&counter);
|
||||
|
||||
let _guard = mock_agent_run_turn(move |req| {
|
||||
let counter = StdArc::clone(&counter_for_stub);
|
||||
async move {
|
||||
let n = counter.fetch_add(1, Ordering::SeqCst);
|
||||
if n == 0 {
|
||||
assert_eq!(req.provider_name, "stub-cloud", "first call must hit cloud");
|
||||
Err("OpenHuman API error (400 Bad Request): \
|
||||
{\"success\":false,\"error\":\"Budget exceeded — add credits to continue\"}"
|
||||
.to_string())
|
||||
} else {
|
||||
// No second cloud call — should jump straight to local.
|
||||
assert_eq!(
|
||||
req.provider_name, "stub-local",
|
||||
"budget-exhausted must skip cloud retry and dispatch to local"
|
||||
);
|
||||
Ok(AgentTurnResponse {
|
||||
text: VALID_JSON_REPLY.to_string(),
|
||||
})
|
||||
}
|
||||
}
|
||||
})
|
||||
.await;
|
||||
|
||||
let outcome = run_triage_with_arms_for_test(cloud_arm(), Some(local_arm()), &envelope())
|
||||
.await
|
||||
.expect("budget-exhausted must not surface as Err");
|
||||
|
||||
let run = outcome.into_decision().expect("decision");
|
||||
assert_eq!(run.resolution_path, TriageResolutionPath::LocalFallback);
|
||||
assert!(run.used_local);
|
||||
assert_eq!(
|
||||
counter.load(Ordering::SeqCst),
|
||||
2,
|
||||
"1 cloud (rejected for budget) + 1 local = 2 (no cloud retry)"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cloud_budget_exhausted_on_retry_falls_through_to_local() {
|
||||
// Variant of OPENHUMAN-TAURI-X: cloud arm trips a transient first
|
||||
// (so we *do* schedule the cloud retry), but the retry itself
|
||||
// comes back as Budget exceeded. We must not run a third cloud
|
||||
// call, and we must fall through to local rather than surface
|
||||
// the budget error as Fatal.
|
||||
AgentDefinitionRegistry::init_global_builtins().expect("init_global_builtins");
|
||||
let counter = StdArc::new(AtomicUsize::new(0));
|
||||
let counter_for_stub = StdArc::clone(&counter);
|
||||
|
||||
let _guard = mock_agent_run_turn(move |req| {
|
||||
let counter = StdArc::clone(&counter_for_stub);
|
||||
async move {
|
||||
let n = counter.fetch_add(1, Ordering::SeqCst);
|
||||
match n {
|
||||
0 => {
|
||||
assert_eq!(req.provider_name, "stub-cloud", "first call must hit cloud");
|
||||
Err("HTTP 503 Service Unavailable".to_string())
|
||||
}
|
||||
1 => {
|
||||
assert_eq!(
|
||||
req.provider_name, "stub-cloud",
|
||||
"second call must be the cloud retry"
|
||||
);
|
||||
Err("OpenHuman API error (400 Bad Request): \
|
||||
{\"success\":false,\"error\":\"Budget exceeded — add credits to continue\"}"
|
||||
.to_string())
|
||||
}
|
||||
_ => {
|
||||
// After the retry returned a budget error, we
|
||||
// must jump straight to local — never a third
|
||||
// cloud call.
|
||||
assert_eq!(
|
||||
req.provider_name, "stub-local",
|
||||
"post-budget dispatch must land on local"
|
||||
);
|
||||
Ok(AgentTurnResponse {
|
||||
text: VALID_JSON_REPLY.to_string(),
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
.await;
|
||||
|
||||
let outcome = run_triage_with_arms_for_test(cloud_arm(), Some(local_arm()), &envelope())
|
||||
.await
|
||||
.expect("budget on retry must not surface as Err");
|
||||
|
||||
let run = outcome.into_decision().expect("decision");
|
||||
assert_eq!(run.resolution_path, TriageResolutionPath::LocalFallback);
|
||||
assert!(run.used_local);
|
||||
assert_eq!(
|
||||
counter.load(Ordering::SeqCst),
|
||||
3,
|
||||
"1 cloud (transient) + 1 cloud retry (budget) + 1 local = 3 (no extra cloud retry)"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cloud_budget_exhausted_without_local_returns_deferred_not_err() {
|
||||
AgentDefinitionRegistry::init_global_builtins().expect("init_global_builtins");
|
||||
let counter = StdArc::new(AtomicUsize::new(0));
|
||||
let counter_for_stub = StdArc::clone(&counter);
|
||||
|
||||
let _guard = mock_agent_run_turn(move |_req| {
|
||||
let counter = StdArc::clone(&counter_for_stub);
|
||||
async move {
|
||||
counter.fetch_add(1, Ordering::SeqCst);
|
||||
Err(
|
||||
"OpenHuman API error (400 Bad Request): Budget exceeded — add credits to continue"
|
||||
.to_string(),
|
||||
)
|
||||
}
|
||||
})
|
||||
.await;
|
||||
|
||||
let outcome = run_triage_with_arms_for_test(cloud_arm(), None, &envelope())
|
||||
.await
|
||||
.expect("budget-exhausted with no local must be Deferred, not Err");
|
||||
|
||||
match outcome {
|
||||
TriageOutcome::Deferred { reason, .. } => {
|
||||
assert!(
|
||||
reason.to_lowercase().contains("budget"),
|
||||
"deferral reason should name the budget cause: {reason}"
|
||||
);
|
||||
}
|
||||
TriageOutcome::Decision(_) => panic!("expected Deferred, got Decision"),
|
||||
}
|
||||
assert_eq!(
|
||||
counter.load(Ordering::SeqCst),
|
||||
1,
|
||||
"no retry — budget would block the second cloud call too"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn no_local_arm_returns_deferred_after_cloud_exhaustion() {
|
||||
AgentDefinitionRegistry::init_global_builtins().expect("init_global_builtins");
|
||||
|
||||
Reference in New Issue
Block a user