Skip to content

fix(inference): cap extraction max_tokens + classify provider 402 to stop the retry/Sentry storm (#3616) - #3617

Merged
senamakel merged 10 commits into
tinyhumansai:mainfrom
oxoxDev:fix/3616-memory-extract-maxtokens-402
Jun 16, 2026
Merged

senamakel merged 10 commits into
tinyhumansai:mainfrom
oxoxDev:fix/3616-memory-extract-maxtokens-402

Conversation

@oxoxDev

@oxoxDev oxoxDev commented Jun 12, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Memory extraction sent chat completions with no max_tokens, so OpenRouter's credit pre-flight reserved the model's full output window (65536 for anthropic/claude-fable-5) and returned 402 Payment Required to any BYO user whose balance couldn't cover it — on every extraction call.
  • The extraction retry loop treated the permanent 402 as a transient transport failure and retried it 3×, and each failure was reported to Sentry — 12k events from a single user (TAURI-RUST-C62).
  • Fix caps extraction output at source (prevention), stops retrying permanent provider errors (de-amplification), and demotes only the residual genuinely-out-of-credits 402 (defense-in-depth).

Problem

memory_tree::extract::llm builds a ChatPrompt and routes it through the inference provider stack. The outbound NativeChatRequest had no max_tokens field, so OpenRouter priced the request against the model's entire output window during its balance check:

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.

An extraction response is one small structured-JSON object (entities + an importance float). The 64k reservation is pure headroom the user never uses, yet it blocks every extraction for a low-balance account. Two amplifiers turned a benign user-state into a 12k-event storm:

  1. Retry of a permanent error — try_extract returned "retry" for any provider Err, so the 402 was retried 3× per call.
  2. Suppression gap — the existing insufficient-credits Sentry demotion only matches OpenHuman managed-backend bodies (errorCode: USER_INSUFFICIENT_CREDITS). A direct BYO-provider 402 has no errorCode, so it fell through every guard to error-level reporting.

Solution

  • Cap at source (prevention). Thread an optional max_tokens through ChatRequest → NativeChatRequest → the OpenAI-compatible wire. Memory extraction sets EXTRACTION_MAX_OUTPUT_TOKENS = 8192, so the provider's pre-flight is priced realistically and clears for a low-balance account. Every other call site passes None (unchanged, open-ended generation).
  • Stop retrying permanent failures (de-amplification). try_extract now returns AttemptOutcome::{Done, Retryable, Permanent}, classifying the error against the provider stack's single source of truth (reliable::is_non_retryable). A permanent 4xx (402 / rejected key / model gone) skips the backoff loop and degrades immediately with a typed cause — a 402 maps to FailureCode::BudgetExhausted, so doctor/status steers the user to top up instead of reporting "extraction is timing out". Transient failures still exhaust the retry budget.
  • Demote the residual 402 (defense-in-depth). is_provider_insufficient_credits_402 recognises the direct-provider out-of-credits shape and demotes it from Sentry error to an info log. This is the residual case once max_tokens is capped: the user's own third-party account is genuinely out of balance — a billing state with no local lever. The matcher is coupled to a verbatim-body test so a provider wording drift fails CI rather than silently leaking events. Honest degradation is preserved — the structure-degraded signal still surfaces to doctor/status; only the Sentry alarm is demoted.

RCA-vs-suppress gate: Bucket = mixed — preventable (the max_tokens cap) + real-defect (the retry of a permanent error); the residual unpreventable out-of-credits tail is the only part suppressed, and only as defense-in-depth behind the two real fixes.

Submission Checklist

If a section does not apply to this change, mark the item as N/A with a one-line reason. Do not delete items.

  • Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
  • Diff coverage ≥ 80% — changed lines (Vitest + cargo-llvm-cov merged via diff-cover) meet the gate enforced by .github/workflows/pr-ci.yml. Run pnpm test:coverage and pnpm test:rust locally; PRs below 80% on changed lines will not merge.
  • N/A: behaviour-only change — no coverage-matrix feature rows added/removed/renamed in docs/TEST-COVERAGE-MATRIX.md.
  • N/A: no feature IDs to list — behaviour-only change touches no matrix rows.
  • No new external network dependencies introduced (mock backend used per Testing Strategy)
  • N/A: does not touch release-cut surfaces — Manual smoke checklist unchanged.
  • Linked issue closed via Closes #NNN in the ## Related section

Impact

  • Runtime: desktop (Rust core). No frontend, mobile, or CLI surface change.
  • Behavior: memory extraction now requests at most 8192 output tokens (ample for the structured-JSON response) and no longer retries a permanent provider error or floods Sentry with BYO out-of-credits 402s. Open-ended generations (orchestrator, agent turns) are unchanged — they pass max_tokens: None.
  • Security/compat: none. The new struct field is additive and serialised only when set.

Related

  • Closes: Memory extraction floods Sentry with OpenRouter 402 for low-balance BYO users #3616
  • Sentry-Issue: TAURI-RUST-C62
  • Closes: TAURI-RUST-4QF (same shape on deepseek native_chat — 402 Payment Required: Insufficient Balance; caught by the same is_provider_insufficient_credits_402 arm)
  • Follow-up PR(s)/TODOs: the summariser path (memory_tree::summarise) shares the same provider and could exhibit the same 402 if a low-balance user has a large per-level budget; left max_tokens: None for now (its output budget is enforced prompt-side and a hard cap risks truncating a valid large summary). Revisit if it surfaces.

AI Authored PR Metadata (required for Codex/Linear PRs)

Keep this section for AI-authored PRs. For human-only PRs, mark each field N/A.

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: fix/3616-memory-extract-maxtokens-402
  • Commit SHA: 9f7738f

Validation Run

  • pnpm --filter openhuman-app format:check
  • pnpm typecheck
  • Focused tests: cargo test --lib openhuman::memory_tree::score::extract::llm (31 pass), …::inference::provider::ops::http_error (4 pass), …::inference::provider::compatible (178 pass)
  • Rust fmt/check (if changed): cargo fmt --check clean; cargo check --all-targets clean
  • Tauri fmt/check (if changed): pnpm rust:check (pre-push)

Validation Blocked

  • command: N/A
  • error: N/A
  • impact: Live BYO-low-balance 402 repro is not reproducible locally (needs a credit-limited OpenRouter key); all three facets are covered deterministically by unit tests.

Behavior Changes

  • Intended behavior change: memory extraction caps output tokens; permanent provider errors are not retried; BYO out-of-credits 402 is demoted from Sentry.
  • User-visible effect: low-balance BYO users can run memory extraction again; doctor/status reports "out of credits" instead of "timing out"; no behaviour change for users with sufficient balance.

Parity Contract

  • Legacy behavior preserved: all non-extraction call sites pass max_tokens: None (identical wire output); transient failures still retry 3×; truncated-JSON still retries; wrong-shape JSON still returns empty without retry.
  • Guard/fallback/dispatch parity checks: the 402 demotion sits before the catch-all should_report_provider_http_failure, mirroring the existing is_provider_access_policy_denied_http_403 guard shape.

Duplicate / Superseded PR Handling

  • Duplicate PR(s): none (dedupe ran — open + 30d-merged; the merged "402" PRs are the x402 crypto-payment protocol, unrelated)
  • Canonical PR: this
  • Resolution (closed/superseded/updated): N/A

Summary by CodeRabbit

  • New Features

    • Optional output-token cap for chat requests (max tokens / max output tokens), propagated across provider flows and Responses API fallbacks.
    • Added dedicated handling for “insufficient credits” (402) and suppressing those events from error reporting.
  • Improvements

    • More reliable LLM extraction with explicit retry vs permanent-failure classification, including retry on truncated JSON.
    • Summarizer and memory-related prompt paths now use output-budget configuration without unintended truncation.
  • Tests

    • Updated and added unit/E2E coverage for token-cap serialization/propagation, retry behavior, and 402 detection/suppression.

…stop the retry/Sentry storm (tinyhumansai#3616)

Memory extraction issued chat calls with no max_tokens, so OpenRouter's
credit pre-flight reserved the model's full output window (65536 for
anthropic/claude-fable-5) and returned 402 Payment Required to any BYO
user whose balance couldn't cover it — on every call. The extraction
retry loop then retried the permanent 402 three times, and each failure
was reported to Sentry, producing 12k events from a single user
(TAURI-RUST-C62).

- Thread an optional max_tokens through the chat-request stack
  (ChatRequest -> NativeChatRequest -> wire). Memory extraction sets a
  bounded 8192-token cap so the provider's balance pre-flight is priced
  against the few thousand tokens an extraction actually needs, not the
  full model window. Other call sites pass None (unchanged behavior).
- Classify extraction failures against the provider stack's
  is_non_retryable: a permanent 4xx (402 out of credits, rejected key,
  model gone) skips the retry loop and degrades immediately with a typed
  cause instead of hammering the provider and multiplying Sentry events.
- Demote a residual BYO insufficient-credits 402 (the user's own
  third-party account is out of balance — no local lever) from Sentry
  error to an info log, gated on a verbatim-body-tested matcher so a
  provider wording drift fails CI instead of silently leaking events.
@oxoxDev
oxoxDev requested a review from a team June 12, 2026 08:39
@oxoxDev oxoxDev added the bug label Jun 12, 2026
@coderabbitai

coderabbitai Bot commented Jun 12, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e35e7984-c326-437b-9305-a65ff3781a47

📥 Commits

Reviewing files that changed from the base of the PR and between 712df5c and 3814d01.

⛔ Files ignored due to path filters (1)
  • app/src-tauri/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • src/core/observability.rs
💤 Files with no reviewable changes (1)
  • src/core/observability.rs

📝 Walkthrough

Walkthrough

This PR threads an optional max_tokens output-token cap through chat request contracts, provider implementations, and memory paths to prevent insufficient-credits 402 errors. It adds HTTP 402 detection, logs it at info level, and suppresses it from Sentry. Memory extraction is refactored to classify permanent provider errors (401/403/402) as non-retryable and treat truncated JSON as retryable, using a new AttemptOutcome enum. Callsites and tests are updated to include the new field throughout.

Changes

Max-tokens output cap and 402 insufficient-credits handling

Layer / File(s) Summary
Chat request contract and wire types
src/openhuman/inference/provider/traits.rs, src/openhuman/inference/provider/compatible_types.rs
Adds ChatRequest.max_tokens: Option<u32> with documentation, NativeChatRequest.max_tokens: Option<u32>, and ResponsesRequest.max_output_tokens: Option<u32>, all serialized only when Some(...).
Provider forwarding and fallback paths
src/openhuman/inference/provider/compatible_provider_impl.rs, src/openhuman/inference/provider/compatible_helpers.rs
Forwards request.max_tokens into native NativeChatRequest and chat_via_responses(); updates all fallback callsites (transport error, 404 responses) to pass max_tokens; adds 402 insufficient-credits detection branch in native error handling.
Reliable provider and retry classification
src/openhuman/inference/provider/reliable.rs
Preserves max_tokens field across per-attempt ChatRequest construction in retry loops; exposes is_non_retryable() as pub(crate) for shared error classification across layers.
HTTP 402 detection and logging
src/openhuman/inference/provider/ops/http_error.rs, src/openhuman/inference/provider/ops/mod.rs
Adds is_provider_insufficient_credits_402() and body_indicates_insufficient_credits() predicates matching PAYMENT_REQUIRED with credit-phrase bodies; adds log_provider_insufficient_credits_402() info logger; includes unit tests for verbatim/variant phrase matching and negative cases; rewraps ops re-exports formatting.
Sentry event suppression
src/core/observability.rs, src/main.rs
Adds is_insufficient_credits_event() filter matching 402 + insufficient-credits predicates in event.message or exception values; integrates into Sentry before_send to drop events before reporting; includes tests for message/exception paths and combined predicate validation.
Memory extraction retry and budget cap
src/openhuman/memory_tree/score/extract/llm.rs, src/openhuman/memory_tree/score/extract/llm_tests.rs
Introduces EXTRACTION_MAX_OUTPUT_TOKENS (8192) constant; refactors extract/try_extract from Option-based logic to AttemptOutcome enum (Done/Retryable/Permanent); classifies permanent client errors (401/403/402) and non-JSON responses as Permanent; treats EOF/truncated mid-JSON as Retryable; adds permanent_failure_code() mapper for error-message patterns; includes tests for cap enforcement, permanent 402 behavior, and retry exhaustion.
Chat prompt integration
src/openhuman/memory/chat.rs, src/openhuman/memory/query/walk.rs, src/openhuman/memory/query/smart_walk/tool.rs, src/openhuman/memory/schema/handlers.rs, src/openhuman/memory_tree/summarise.rs
Adds ChatPrompt.max_tokens: Option<u32> field; wires prompt.max_tokens into ChatRequest in InferenceChatProvider; updates memory adapters/handlers to set or leave max_tokens unset; summariser sets None with in-code rationale for prompt-side enforcement; updates test prompts.
Agent, harness, and provider callsites
src/bin/inference_probe.rs, src/openhuman/agent/harness/..., src/openhuman/inference/provider/claude_code/mod.rs, src/openhuman/routing/provider_tests.rs
Updates ~15 ChatRequest literals across agent loop, harness engine, checkpoint handlers, and routing tests to set max_tokens: None; updates LM Studio and Ollama factory tests.
Request serialization and type tests
src/openhuman/inference/provider/compatible_tests.rs, src/openhuman/inference/provider/traits_tests.rs
Adds unit tests asserting max_tokens/max_output_tokens serialize only when Some(...); updates serialization, frequency_penalty, include_usage, and provider-trait tests to set max_tokens: None for stable JSON assertions; updates provider-config rejection tests.
End-to-end test updates
tests/inference_agent_raw_coverage_e2e.rs, tests/inference_compatible_*.rs, tests/inference_provider_*.rs, tests/inference_round26_raw_coverage_e2e.rs, tests/owned_domain_raw_coverage_e2e.rs
Updates ~25 ChatRequest literals across provider trait tests, OpenAI-compatible streaming/content, router fallback, reliable provider exhaustion, compatible matrix tests, and owned domain tests to set max_tokens: None for consistent request shapes.

Sequence Diagram

sequenceDiagram
  participant Extractor
  participant ChatProvider
  participant NativeAPI
  participant Observability
  Extractor->>ChatProvider: ChatRequest(max_tokens=8192)
  ChatProvider->>NativeAPI: NativeChatRequest(max_tokens=8192)
  NativeAPI-->>ChatProvider: 200 OK or HTTP error
  alt 402 Insufficient Credits
    ChatProvider->>Observability: log_provider_insufficient_credits_402()
    ChatProvider-->>Extractor: AttemptOutcome::Permanent
    Observability->>Observability: is_insufficient_credits_event drop
  else Transient Error
    ChatProvider-->>Extractor: AttemptOutcome::Retryable
    Extractor->>Extractor: retry with backoff
  else Success
    ChatProvider-->>Extractor: AttemptOutcome::Done
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

The PR introduces schema changes across multiple layers (request contracts, wire types, prompt structures), refactors core extraction retry semantics with a new enum, threads a new field through provider paths and fallbacks, adds error classification and Sentry filtering logic, and updates 60+ test/callsite literals. The changes are coherent but span diverse files and require understanding the extraction refactor, provider error handling, and Sentry integration context.

Suggested reviewers

  • senamakel
  • graycyrus

Poem

🐰 With bounded tokens, balance preserved,

402s logged—no Sentry swerved.
Extraction now knows: when credits run dry,
it gracefully stops, no retry storm nigh. 🥕✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main changes: capping extraction max_tokens and classifying provider 402 errors to prevent retry/Sentry storms.
Linked Issues check ✅ Passed All objectives from issue #3616 are met: extraction max_tokens capped at 8192, permanent failures no longer retried, 402 events demoted in Sentry, and unit tests added with >80% coverage.
Out of Scope Changes check ✅ Passed All changes align with issue #3616 objectives. The PR threads max_tokens through ChatRequest infrastructure, classifies extraction errors, adds 402 classification and demotion, and covers edge cases with tests.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot added rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure. memory Memory store, memory tree, recall, summarization, and embeddings in src/openhuman/memory/. sentry-traced-bug Bug identified via Sentry triage labels Jun 12, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/openhuman/inference/provider/compatible_provider_impl.rs (1)

485-509: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Forward max_tokens through the Responses API branch too.

chat() returns early here, before either NativeChatRequest builder, so request.max_tokens is dropped whenever responses_api_primary is enabled. That leaves capped callers open-ended on this provider mode unless chat_via_responses(...) gets an equivalent budget parameter (or explicitly rejects Some(max_tokens)).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/inference/provider/compatible_provider_impl.rs` around lines
485 - 509, The Responses-API branch drops request.max_tokens because
chat_via_responses is called without a budget parameter; update the branch so
max_tokens is forwarded or validated: modify chat_via_responses signature to
accept an Option<u32> (or add a new parameter) and pass request.max_tokens when
calling it in the responses_api_primary branch (i.e., change the call in the
block guarded by responses_api_primary to self.chat_via_responses(credential,
&response_messages, model, request.max_tokens). Then update the
chat_via_responses implementation to propagate that max_tokens into the
Responses API request (or explicitly return an error if Some(max_tokens) cannot
be honored). Ensure usages of chat_via_responses (and its new signature) are
updated accordingly so max_tokens is no longer dropped.
🧹 Nitpick comments (2)
src/openhuman/inference/provider/compatible_provider_impl.rs (1)

512-529: ⚡ Quick win

Add debug diagnostics for the new max_tokens plumbing.

Right now the new cap is only visible in dump files, so normal logs do not show whether a request actually forwarded max_tokens or left it unset. Please log it on the streaming and buffered request paths with the existing provider correlation fields. As per coding guidelines, "Include debug logging with verbose diagnostics on all new/changed flows: log entry/exit, branches, external calls, retries/timeouts, state transitions, and errors with stable grep-friendly prefixes ([domain], [rpc]) and correlation fields."

Also applies to: 602-616

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/inference/provider/compatible_provider_impl.rs` around lines
512 - 529, The new max_tokens field in NativeChatRequest is only written to
dumps; add debug diagnostics that log whether max_tokens was forwarded or left
unset on both the streaming and buffered request paths (the block that builds
NativeChatRequest and the analogous block at lines ~602-616), using the same
provider correlation fields and stable grep-friendly prefixes like "[rpc]" or
"[domain]". Instrument the code paths that call NativeChatRequest (e.g., where
NativeChatRequest is constructed and where requests are dispatched/streamed) to
emit entry/exit logs and a debug line showing model, computed max_tokens (or
"unset"/"none"), effective_temperature(self.effective_temperature(...)),
tool_choice/tools, thread_id, and any retry/meta info; include the existing
correlation id fields so logs correlate with dumps and use consistent message
formatting for easy grepping.

Source: Coding guidelines

src/openhuman/memory/chat.rs (1)

92-99: ⚡ Quick win

Log max_tokens for 402 diagnostics.

The request log doesn't include prompt.max_tokens, but this field is central to preventing 402 errors (the PR's primary goal). When diagnosing low-balance BYO 402 failures, knowing whether max_tokens was set (and to what value) would help confirm whether the cap is working as intended.

📊 Proposed observability enhancement
 log::debug!(
-    "[memory::chat] provider={} kind={} model={} sys_chars={} user_chars={}",
+    "[memory::chat] provider={} kind={} model={} sys_chars={} user_chars={} max_tokens={:?}",
     self.display,
     prompt.kind,
     self.model,
     prompt.system.len(),
-    prompt.user.len()
+    prompt.user.len(),
+    prompt.max_tokens
 );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/memory/chat.rs` around lines 92 - 99, The debug log in
memory::chat omits prompt.max_tokens which is essential for diagnosing 402 BYO
quota issues; update the log::debug call that prints self.display, prompt.kind,
self.model, prompt.system.len(), and prompt.user.len() to also include
prompt.max_tokens (or its Option/nullable state) so the emitted message shows
whether a max_tokens cap was set and its value; locate the log::debug invocation
in the chat memory code (the call referencing self.display and prompt.*) and
append the max_tokens value with a clear label in the formatted string.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/openhuman/inference/provider/compatible_provider_impl.rs`:
- Around line 742-752: The 402 demotion logic currently applied in
Provider::chat (the native_chat path) must be applied to all HTTP-compatible
provider call sites so direct-provider insufficient-credits errors don't bypass
it; update chat_with_system, chat_with_history, stream_chat_with_system, and
stream_chat_with_history to check
super::super::is_provider_insufficient_credits_402(status, &error) where they
currently fall through to generic provider-failure handling and, when true, call
super::super::log_provider_insufficient_credits_402("native_chat",
self.name.as_str(), Some(model), status) (or the appropriate model/context
argument) and return the same demoted/info outcome instead of the generic
failure path.

In `@src/openhuman/inference/provider/traits.rs`:
- Around line 161-174: The default Provider::chat implementation currently
delegates to Provider::chat_with_history and ignores Request.max_tokens; update
the default chat method to check request.max_tokens and fail fast (return an
Err) if Some(..) and the provider hasn't overridden chat, or alternatively call
a new budget-aware hook before delegating; specifically modify the default impl
of Provider::chat to inspect the Request struct's max_tokens field and either
return a clear error indicating the provider must implement a capped chat path
or invoke a new Provider::chat_with_budget (or similar) hook that honors
max_tokens; references: Provider::chat, Provider::chat_with_history,
Request.max_tokens.

---

Outside diff comments:
In `@src/openhuman/inference/provider/compatible_provider_impl.rs`:
- Around line 485-509: The Responses-API branch drops request.max_tokens because
chat_via_responses is called without a budget parameter; update the branch so
max_tokens is forwarded or validated: modify chat_via_responses signature to
accept an Option<u32> (or add a new parameter) and pass request.max_tokens when
calling it in the responses_api_primary branch (i.e., change the call in the
block guarded by responses_api_primary to self.chat_via_responses(credential,
&response_messages, model, request.max_tokens). Then update the
chat_via_responses implementation to propagate that max_tokens into the
Responses API request (or explicitly return an error if Some(max_tokens) cannot
be honored). Ensure usages of chat_via_responses (and its new signature) are
updated accordingly so max_tokens is no longer dropped.

---

Nitpick comments:
In `@src/openhuman/inference/provider/compatible_provider_impl.rs`:
- Around line 512-529: The new max_tokens field in NativeChatRequest is only
written to dumps; add debug diagnostics that log whether max_tokens was
forwarded or left unset on both the streaming and buffered request paths (the
block that builds NativeChatRequest and the analogous block at lines ~602-616),
using the same provider correlation fields and stable grep-friendly prefixes
like "[rpc]" or "[domain]". Instrument the code paths that call
NativeChatRequest (e.g., where NativeChatRequest is constructed and where
requests are dispatched/streamed) to emit entry/exit logs and a debug line
showing model, computed max_tokens (or "unset"/"none"),
effective_temperature(self.effective_temperature(...)), tool_choice/tools,
thread_id, and any retry/meta info; include the existing correlation id fields
so logs correlate with dumps and use consistent message formatting for easy
grepping.

In `@src/openhuman/memory/chat.rs`:
- Around line 92-99: The debug log in memory::chat omits prompt.max_tokens which
is essential for diagnosing 402 BYO quota issues; update the log::debug call
that prints self.display, prompt.kind, self.model, prompt.system.len(), and
prompt.user.len() to also include prompt.max_tokens (or its Option/nullable
state) so the emitted message shows whether a max_tokens cap was set and its
value; locate the log::debug invocation in the chat memory code (the call
referencing self.display and prompt.*) and append the max_tokens value with a
clear label in the formatted string.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 03ad3010-4534-46b9-8c27-c1824e2734d8

📥 Commits

Reviewing files that changed from the base of the PR and between 5bf7dae and 9f7738f.

📒 Files selected for processing (32)
  • src/bin/inference_probe.rs
  • src/openhuman/agent/harness/engine/core.rs
  • src/openhuman/agent/harness/session/turn/session_io.rs
  • src/openhuman/agent/harness/session/turn_engine_adapter.rs
  • src/openhuman/agent/harness/subagent_runner/ops/checkpoint.rs
  • src/openhuman/agent/harness/test_support_tests.rs
  • src/openhuman/inference/provider/claude_code/mod.rs
  • src/openhuman/inference/provider/compatible_provider_impl.rs
  • src/openhuman/inference/provider/compatible_tests.rs
  • src/openhuman/inference/provider/compatible_types.rs
  • src/openhuman/inference/provider/factory_tests.rs
  • src/openhuman/inference/provider/ops/http_error.rs
  • src/openhuman/inference/provider/ops/mod.rs
  • src/openhuman/inference/provider/reliable.rs
  • src/openhuman/inference/provider/traits.rs
  • src/openhuman/inference/provider/traits_tests.rs
  • src/openhuman/memory/chat.rs
  • src/openhuman/memory/query/smart_walk/tool.rs
  • src/openhuman/memory/query/walk.rs
  • src/openhuman/memory/schema/handlers.rs
  • src/openhuman/memory_tree/score/extract/llm.rs
  • src/openhuman/memory_tree/score/extract/llm_tests.rs
  • src/openhuman/memory_tree/summarise.rs
  • src/openhuman/routing/provider_tests.rs
  • tests/inference_agent_raw_coverage_e2e.rs
  • tests/inference_compatible_admin_leftovers_raw_coverage_e2e.rs
  • tests/inference_compatible_matrix_raw_coverage_e2e.rs
  • tests/inference_local_admin_raw_coverage_e2e.rs
  • tests/inference_provider_admin_round22_raw_coverage_e2e.rs
  • tests/inference_provider_raw_coverage_e2e.rs
  • tests/inference_round26_raw_coverage_e2e.rs
  • tests/owned_domain_raw_coverage_e2e.rs

@oxoxDev

oxoxDev commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

CI note: the three red checks are pre-existing failures on main, unrelated to this PR

This change is Rust-core only (inference provider + memory extraction). All three failing checks are in code this PR does not touch, and reproduce on a clean checkout of main (dca389712):

  • Rust Core Coverage — openhuman::agent::tests::auto_save_stores_messages_in_memory fails with Expected at least 2 memory entries, got 1. Verified to fail identically on main @ dca389712 with zero changes from this branch (3/3 deterministic). It's an agent auto-save regression, independent of the 402 fix here.
  • Frontend Coverage (Vitest) — src/pages/__tests__/Conversations.render.test.tsx (budget/quota-pill cases). Frontend React; untouched by this diff.
  • E2E (Playwright / web lane 1/4) — connector-gmail-composio.spec.ts. Frontend E2E; untouched by this diff.

This PR's own gates pass: Rust Quality (fmt, clippy), Rust E2E (mock backend), Rust Tauri Coverage, i18n, Frontend Quality, PR Submission Checklist, Coverage Matrix Sync, and Playwright web lanes 2–4. Happy to rebase once main is green.

oxoxDev added 3 commits June 12, 2026 18:15
…hat path (tinyhumansai#3616)

CodeRabbit: the default `Provider::chat()` delegates to `chat_with_history`,
which carries no output budget, so a caller's `max_tokens` was silently
ignored on that path. No production caller both sets a cap and routes through
a default-chat() provider (agent turns pass None; memory extraction uses the
compatible provider, which overrides chat()), so this logs the drop rather
than hard-failing — failing would risk breaking agent turns via
ClaudeAgentSdkProvider. Documents the caveat on the trait method.
…nyhumansai#3616)

CodeRabbit: when responses_api_primary is enabled, chat() returns early via
chat_via_responses before building NativeChatRequest, dropping
request.max_tokens. Add max_output_tokens to ResponsesRequest and thread the
caller's cap through chat_via_responses (forwarded from the three chat()
call sites; the chat_with_system/chat_with_history fallbacks have no request
budget and pass None). Keeps a capped request capped on responses-API
providers (TAURI-RUST-C62).
…ath via before_send (tinyhumansai#3616)

CodeRabbit: the 402 demotion was only wired into Provider::chat()'s native_chat
cascade, so the same out-of-balance 402 reported from chat_with_system,
chat_with_history, the streaming gates, and the shared api_error helper still
paged Sentry. Add is_insufficient_credits_event to the before_send chain (the
established defense-in-depth chokepoint alongside is_budget_event) so the
demotion applies to every path in one place. Extract the credit-phrase set
into body_indicates_insufficient_credits as the single source shared by the
emit-site guard and the central filter (TAURI-RUST-C62).
@oxoxDev

oxoxDev commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

Addressed CodeRabbit's outside-diff finding (Responses API drops max_tokens) in 7008090 — added max_output_tokens to ResponsesRequest and threaded the caller's cap through chat_via_responses, forwarded from all three chat() call sites (the responses_api_primary branch + the transport/404 fallbacks). The chat_with_system/chat_with_history fallbacks have no request budget and pass None. Serialization covered by responses_request_serializes_max_output_tokens_only_when_set.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/core/observability.rs`:
- Around line 2136-2140: The helper text_is_insufficient_credits_402 currently
uses a loose lower.contains("402") and must be tightened to only match 402 as an
HTTP/status anchor; change the numeric check to a status-shaped match (e.g. use
a case-insensitive anchored pattern that looks for word-boundary 402 preceded
optionally by common status anchors such as "status", "error", or an HTTP status
prefix like "HTTP/1.1" — e.g. a regex similar to (?i)\b(?:status[: ]|error[:
]|http/[\d.]+\s+)?402\b) and keep the existing call to
provider::body_indicates_insufficient_credits; also add a regression test
adjacent to insufficient_credits_filter_requires_both_402_and_credit_phrase that
asserts a message like "provider API error (400): can only afford 402 tokens"
does NOT return true.

In `@src/openhuman/inference/provider/compatible_helpers.rs`:
- Around line 23-24: Add debug/trace logging at the point where the new runtime
control max_output_tokens is forwarded: in the function that accepts the
max_output_tokens parameter, emit a grep-friendly debug/trace line (e.g.
starting with “[rpc]” or “[domain]”) showing the incoming max_output_tokens
value, whether it is Some/None, and which forwarding branch was taken; do the
same for the related forwarding block around the subsequent cap-propagation code
(the block that currently handles propagation in the file) so callers and future
triage can see the exact values and path.

In `@src/openhuman/inference/provider/traits.rs`:
- Around line 444-450: Change the diagnostic to use a debug/trace-level log with
the repo’s stable grep-friendly prefix style instead of log::warn and the custom
"[provider]" tag: when request.max_tokens is Some(cap) inside the default chat()
implementation that ignores the cap (and when chat_with_history() carries no
budget), emit a debug/trace log (not warn) using the approved prefix (e.g.
"[domain]" or other repo-stable prefix) and keep the model and cap in the
message so the info still surfaces for debugging.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 89b8bf79-987a-41b8-ab47-1a78f3a41c1b

📥 Commits

Reviewing files that changed from the base of the PR and between 9f7738f and 139352f.

📒 Files selected for processing (9)
  • src/core/observability.rs
  • src/main.rs
  • src/openhuman/inference/provider/compatible_helpers.rs
  • src/openhuman/inference/provider/compatible_provider_impl.rs
  • src/openhuman/inference/provider/compatible_tests.rs
  • src/openhuman/inference/provider/compatible_types.rs
  • src/openhuman/inference/provider/ops/http_error.rs
  • src/openhuman/inference/provider/ops/mod.rs
  • src/openhuman/inference/provider/traits.rs
✅ Files skipped from review due to trivial changes (1)
  • src/openhuman/inference/provider/ops/mod.rs
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/openhuman/inference/provider/ops/http_error.rs
  • src/openhuman/inference/provider/compatible_provider_impl.rs

Comment thread src/core/observability.rs Resolved
…debug diagnostics (tinyhumansai#3616)

- observability: anchor the insufficient-credits filter's 402 match to a
  status shape ('(402' / '402 payment required') so a non-402 error whose
  body merely contains the digits '402' plus a credit phrase isn't suppressed.
  Adds a false-positive regression test.
- compatible_helpers: add a [provider] debug line at the Responses-path
  cap-forwarding point so max_output_tokens propagation is traceable.
- traits: the default-chat() dropped-cap diagnostic is debug (not warn),
  matching the repo's Rust-core diagnostics contract.
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 12, 2026
senamakel and others added 2 commits June 12, 2026 09:48
…1.93 (tinyhumansai#3616)

The insufficient-credits filter's 402-status check fit on one line under
rustfmt 1.93's max_width; a stray wrap survived the earlier nit commit and
tripped `cargo fmt --all --check`. No behavior change.
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 12, 2026
sanil-23
sanil-23 previously approved these changes Jun 15, 2026

@sanil-23 sanil-23 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve — well-scoped, well-tested, CI green.

The three-layer fix (cap max_tokens at source → stop retrying permanent 4xx via is_non_retryable → demote residual BYO out-of-credits 402) is sound and correctly targets the one gap neither existing mechanism covered: a raw BYO direct-provider 402 carries no managed-backend errorCode and no 400 "Insufficient budget" shape, so it slipped past both is_budget_exhausted_http_400 and the BackendErrorCodeOwned ownership rule. The managed path (400 + USER_INSUFFICIENT_CREDITS) is untouched and still handled. Honest degradation is preserved — mark_structure_degraded(BudgetExhausted) still surfaces to doctor/status; only Sentry + retries are suppressed. Good test coverage (1-call-on-permanent vs 3-on-transient, skip-when-None serialization for both wire shapes, verbatim-C62 coupling).

Non-blocking nit (fine as a follow-up): the before_send net is_insufficient_credits_event matches the formatted message "X API error (402 Payment Required): ...", and body_indicates_insufficient_credits lists "payment required" — which is the 402 status reason itself, present in every 402 message. So the status && phrase AND-gate effectively collapses to "is it a 402", and a non-credit BYO 402 (suspended/region-blocked account) gets Sentry-suppressed too. Dropping "payment required" from the phrase list (the raw-body credit signals are sufficient) makes the gate match the PR's stated intent. The emit-site guard is unaffected since it matches the raw body.

@senamakel senamakel self-assigned this Jun 15, 2026
# Conflicts:
#	src/openhuman/memory_tree/score/extract/llm.rs
@senamakel
senamakel dismissed stale reviews from sanil-23 and coderabbitai[bot] via 60a1f74 June 15, 2026 23:59
@senamakel
senamakel merged commit 445b11a into tinyhumansai:main Jun 16, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug memory Memory store, memory tree, recall, summarization, and embeddings in src/openhuman/memory/. rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure. sentry-traced-bug Bug identified via Sentry triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory extraction floods Sentry with OpenRouter 402 for low-balance BYO users

3 participants