Skip to content

feat(memory): make external provider prefetch timeout configurable - #98045

Closed
Navlem wants to merge 1 commit into
NousResearch:mainfrom
Navlem:feat/configurable-memory-prefetch-timeout
Closed

Navlem wants to merge 1 commit into
NousResearch:mainfrom
Navlem:feat/configurable-memory-prefetch-timeout

Conversation

@Navlem

@Navlem Navlem commented Aug 29, 2026

Copy link
Copy Markdown

Problem

agent/memory_manager.py hardcodes _EXTERNAL_PREFETCH_TIMEOUT_S = 8.0, but the correct value depends on the external memory provider's recall latency — something the core cannot know.

This is not only a "memory arrives late" issue. In MemoryManager._prefetch_external (memory_manager.py:611-632) a timed-out worker thread is not cancelled:

thread.join(self._external_prefetch_timeout)
if thread.is_alive():
    logger.warning("Memory provider '%s' prefetch timed out after %.1fs; ...")
    return ""

The thread keeps running, and lines 611-619 skip prefetch on any later turn while it is still alive. So when a provider's recall reliably exceeds the timeout, prefetch is never applied at all — not merely delayed. The only symptom is a repeating warning, which reads like noise rather than "your memory provider is switched off."

Observed with the Hindsight provider against a bank whose local CPU cross-encoder reranked 300 candidates per recall: recall measured 15-22s against the 8s ceiling, 135 warnings accumulated, and prefetch was effectively disabled for the entire period. Raising the timeout restored it immediately.

Change

Adds memory.external_prefetch_timeout (default 8.0, so existing behaviour is unchanged) and threads it from agent_init into MemoryManager.

Deliberate choices:

  • config.yaml, not an env var — this is a behavioural setting, not a credential.
  • Invalid values fall back, they do not raise. A non-numeric or non-positive value in user config must not break startup, so it degrades to the existing default. MemoryManager's own positive-value guard is left intact for direct callers.

Tests

tests/agent/test_memory_prefetch_timeout_config.py:

  • Resolution chain: configured float / int / numeric string, absent key, None, zero, negative, garbage, empty config.
  • Asserts the documented default in config_defaults.py matches _EXTERNAL_PREFETCH_TIMEOUT_S, so the two cannot silently drift.
  • Guards the real agent_init call site. The parametrised cases mirror agent_init's resolution logic, which means they would stay green if agent_init reverted to a bare MemoryManager() — so that case is asserted separately against the source.

Mutation-verified: breaking the wiring fails the guard test (exit 1); restoring it passes (exit 0). 88 tests pass alongside the existing tests/agent/test_memory_provider.py suite.

Verification

Exercised on a live install rather than only in tests: set to 15, restarted the gateway, confirmed the value resolves through load_config() → MemoryManager as 15.0, and confirmed real hindsight_recall calls now complete instead of being discarded. Zero timeout warnings from sessions created after the restart.

One behaviour worth noting for reviewers: the timeout is read when a session's agent is constructed, so already-running long-lived sessions keep their previous value until they end. That is consistent with how other memory.* settings behave.

`_EXTERNAL_PREFETCH_TIMEOUT_S = 8.0` was hardcoded, but the right value
depends entirely on the provider's recall latency, which the core cannot
know. Expose it as `memory.external_prefetch_timeout` (default 8.0, so
existing behaviour is unchanged) and pass it through from agent_init.

This is not just a "recall arrives late" nicety. In
MemoryManager._prefetch_external (memory_manager.py:611-632) a timed-out
worker thread is NOT cancelled: join() gives up, the call returns "", and
the thread keeps running. The next turn sees that thread still alive and
skips prefetch entirely. So when a provider's recall reliably exceeds the
timeout, prefetch is not merely delayed — it is never applied at all, and
the only symptom is a repeating warning.

Observed with the Hindsight provider on a bank whose local CPU cross-encoder
reranked 300 candidates per recall: recall measured 15-22s against the 8s
ceiling, and 135 warnings accumulated with prefetch effectively disabled the
whole time. Raising the timeout restored it immediately.

Settings belong in config.yaml rather than an env var, and a bad value must
not break startup, so a non-numeric or non-positive value falls back to the
existing default instead of raising. MemoryManager's own positive-value guard
is left intact for direct callers.

Tests: tests/agent/test_memory_prefetch_timeout_config.py covers the
resolution chain (configured float/int/string, absent, None, zero, negative,
garbage, empty config), asserts the documented default matches the runtime
fallback so the two cannot drift, and guards the real agent_init call site —
the parametrised cases mirror agent_init's logic and would stay green if it
reverted to a bare MemoryManager(), so that case is asserted separately.
Verified by mutation: breaking the wiring fails the guard, restoring it
passes. 88 tests pass alongside the existing memory suite.
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/memory Memory tool and memory providers area/config Config system, migrations, profiles sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state duplicate This issue or pull request already exists labels Aug 29, 2026
@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Duplicate of #87028. Both implement the same memory.external_prefetch_timeout config-to-MemoryManager handoff; #87028 is the earlier open and more complete implementation.

@rodrigogs

Copy link
Copy Markdown

I've posted the field evidence (Bedrock-only Docker gateway with a local Ollama embedder: 8s timeouts coinciding with 18.4-47.1s embeds and a 400 after 60s, 101/197 agent starts with no injected recall) on #87028, since the triage bot already flagged this PR as a duplicate of it and #87028 is the earlier, more complete head. Two of the same gaps apply here: cli-config.yaml.example's memory: block (ends at line 908) doesn't get the new external_prefetch_timeout key, and test_agent_init_actually_passes_the_timeout asserts on inspect.getsource(agent_init) text, which is brittle against the _init_memory refactor already on main (agent_init.py:1222-1290) - a call-based test (invoke _init_memory directly with a patched load_memory_provider and assert on agent._memory_manager._external_prefetch_timeout) would survive that. Not duplicating the full note here - see #87028 for the rest.

@Navlem

Navlem commented Oct 1, 2026

Copy link
Copy Markdown
Author

Closing as a duplicate of open #87028, which covers the same external_prefetch_timeout registration/handoff with stronger validation. This is not fixed on main. The keeper still needs rebase, actual _init_memory wiring tests, sample/docs, and coordination with provider-specific defaults. Preserve configured/invalid/missing-value tests in that work.

@Navlem Navlem closed this Oct 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/memory Memory tool and memory providers type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants