Skip to content

Memory recap gives up after a single summariser attempt — no retry on a transient failure #6187

Description

@YellowSnnowmann

Summary

The memory archivist's segment recap calls the summariser exactly once. A transient failure — a dropped connection, a 503, a 429 — is treated as final, and the segment is left unsummarised. There is no retry and no backoff anywhere on this path.

This is the piece #6183 deliberately left out. That PR stopped a failed recap from writing a fake summary; it did nothing to reduce how often the recap fails in the first place.

Problem

#6156's log is the canonical case:

07:18:59 WRN [providers][openhuman-backend] managed invoke failed: status=None code=None provider=OpenHuman retryable=true
  detail=request to https://api.tinyhumans.ai/openai/v1/chat/completions failed: error sending request

07:18:59 WRN [archivist] summarize_entries: LLM recap failed (non-fatal)
  segment=seg-18d3352de2d024987810f9e6: memory_tree::summarise: provider=inference:summarization-v1 — heuristic fallback

Same second, one attempt, gone. The provider itself classified the failure retryable=true and nothing acted on it.

There is no retry at any layer of this call. Verified against main @ 537854715:

Layer File Why it doesn't retry
Memory chat adapter vendor/tinymemory/crates/tinymemory-core/src/chat.rs:113 InferenceChatProvider::run_with_usage issues a single self.inner.invoke(&(), request).await?
Model construction src/openhuman/inference/provider/factory_part_02.rs:1 create_chat_model_with_model_id_inner returns a bare concrete ChatModel — no fallback chain, no resilience wrapper at this site
Managed backend src/openhuman/inference/provider/openhuman_backend_model.rs:484 OpenHumanBackendModel::invoke has no retry loop; the retryable flag it computes at :461 is logged and discarded
TinyAgents resilience tinyagents-harness/src/middleware/library/resilience.rs:52, tinyagents-graph/src/compiled/executor.rs:1180 RetryPolicy / RetryMiddleware wrap graph and subagent nodes. A memory recap is a direct model invoke and never enters a graph, so none of it applies

Impact

Post-#6183 the consequence is no longer corruption — it is a permanent hole. Every transient blip costs one segment its summary, and (until #6186) nothing ever comes back for it. A thirty-second outage during an active session can strand several segments.

The user-visible effect is degraded recall on those stretches of history, with no indication in the product that anything happened.

Steps to reproduce

  1. Point the summarization / memory role at a reachable managed backend.
  2. Start a session and produce enough turns to close at least one segment.
  3. Sever connectivity to the provider for ~5 seconds, spanning segment close.
  4. Restore connectivity.

Expected: the recap retries and succeeds. Actual: one attempt, LLM recap failed (non-fatal), segment left unsummarised forever.

Solution

A bounded retry with backoff around the summarise call — small (2–3 attempts), jittered, and skipping classes that cannot succeed on a retry (auth, unknown model, quota exhausted, non-retryable rate limit).

Where it has to live, and why it is not obvious

The natural-looking place is host-side in summarize_entries (src/openhuman/agent/harness/archivist/recap.rs:202), around fold_through_driver. That placement cannot classify errors, and this is the crux:

error_classify::classify (src/openhuman/inference/provider/error_classify.rs:8) works from err.to_string() plus err.downcast_ref::<reqwest::Error>() for the status code. Neither survives the trip to the host:

  • Self::other (tinymemory-tinycortex/src/engine/mod.rs:364) formats "{context}: {error}". {error} on an anyhow::Error renders only the outermost context, so the cause chain — and any message a classifier could match on — is dropped.
  • The result then crosses the tinybus wire as ai.tinyhumans.tinymemory.Error.Other plus that flattened string, so there is no reqwest::Error left to downcast.

By the time summarize_entries sees it, the error reads summarise tree inputs: memory_tree::summarise: provider=inference:summarization-v1 and carries no signal whatsoever. A host-side retry would be blind — it would burn attempts on terminal auth/quota failures and would ignore Retry-After on a 429.

Two viable options:

A. Retry inside tinymemory-core::tree::summarise::summarise (vendor/tinymemory/crates/tinymemory-core/src/tree/summarise.rs:24) — around the provider.chat_for_text_with_usage(...) call, where the typed anyhow::Error and the underlying reqwest::Error are both still in hand. No wire-contract change (the function signature is untouched), but it needs a tinymemory release and a re-pin. Covers every caller of summarise, not only the archivist. Preferred.

B. Retry host-side, after fixing the truncation. Change Self::other to {error:#} so the cause string reaches the host, then classify on the message alone. Cheaper, but still loses the reqwest::Error downcast and the Retry-After header, so classification stays weaker than A.

Either way, reuse tinyagents_harness::retry::{RetryPolicy, classify_provider_failure} rather than hand-rolling backoff.

Constraint worth stating

Post-turn hooks are detached (tokio::spawn, src/openhuman/agent/hooks.rs:307), so retry latency on the normal segment-close path is invisible to the user. But flush_open_segment is awaited unbounded at session wind-down (src/openhuman/agent/harness/session/turn/session_io_impl_01_part_02.rs:294). An unbounded retry loop there would add directly to shutdown time. The policy needs a hard total-elapsed ceiling, not just an attempt count.

Acceptance criteria

  • Transient recovers — a summarise call that fails once with a retryable class and succeeds on the next attempt produces an LLM recap, and the segment is persisted normally.
  • Terminal classes are not retried — auth failure, unknown model, quota exhausted and non-retryable rate limit each consume exactly one attempt.
  • Bounded — attempt count and a total-elapsed ceiling are both enforced, so flush_open_segment at wind-down cannot be extended indefinitely.
  • Backoff is jittered — concurrent segment closes do not retry in lockstep.
  • Exhaustion is still visible — after the final attempt the existing [archivist] summarize_entries: LLM recap failed (non-fatal) WARN still fires, and reports the attempt count.
  • Regression safety — a test drives a provider double that fails N-1 times then succeeds, and a second that returns a terminal class and asserts a single attempt.
  • Diff coverage ≥ 80% — the fix PR meets the changed-lines coverage gate.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

bugmemoryMemory store, memory tree, recall, summarization, and embeddings in src/openhuman/memory/.

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions