Skip to content

fix(memory): re-raise LLM extraction failures instead of returning [] - #5178

Closed
thalesfsp wants to merge 1 commit into
mem0ai:mainfrom
thalesfsp:fix/surface-llm-extraction-errors
Closed

thalesfsp wants to merge 1 commit into
mem0ai:mainfrom
thalesfsp:fix/surface-llm-extraction-errors

Conversation

@thalesfsp

Copy link
Copy Markdown

Summary

Today, when self.llm.generate_response raises during fact extraction in _add_to_vector_store (and its async counterpart), mem0 catches the exception, logs LLM extraction failed: {e}, and returns an empty list. The caller has no way to distinguish "LLM unavailable" from "LLM extracted no facts" — both look like [].

This makes provider-fallback wrappers impossible to build cleanly: anything wanting to retry on 429 / 5xx / connection error / timeout can't tell whether to fall through.

Change

Re-raise after logging, in both sync (_add_to_vector_store) and async (_add_to_vector_store_async) paths. The error is still logged at ERROR level for visibility.

Why this is safe

  • Existing callers that don't try/except around .add() will surface previously-silent failures. This is the goal — callers should be told the LLM is down, not given a silent empty result.
  • Embedding failures already propagate (no analogous swallow), so this aligns .add()'s contract: external dependency failures bubble; internal parsing/extraction-empty cases continue to return [].
  • The parsing path immediately below (the second try/except) still swallows json.JSONDecodeError — that's a different class of failure (LLM responded with non-JSON) and isn't changed.

Use case driving this

Building a fallback chain in a wrapper service: primary LLM (xAI grok) → fallback (Anthropic) → fallback (local). The wrapper needs to retry .add() on the next provider when the current one errors. With the current behavior the wrapper sees an empty success and stops.

Test plan

  • Existing unit tests in tests/memory/ still pass (no test currently asserts the silent-empty behavior).
  • Manual: with an invalid openai_base_url, .add() previously returned []; after this change, it raises openai.APIConnectionError (or similar).

Companion PR

The same fork also has #5176 (fact-shape normalization) which is unrelated and can land independently.

🤖 Generated with Claude Code

The original behavior swallows any exception from ``self.llm.generate_response``
(network error, auth failure, 429, 5xx, timeout) and returns an empty memory
list. Callers cannot distinguish "LLM unavailable" from "LLM extracted no facts"
— so a wrapper service that wants to implement a provider fallback chain can't
detect when to fall through.

This patch re-raises after logging in both the sync (``_add_to_vector_store``)
and async (``_add_to_vector_store_async``) paths. Behavior change for callers:
``.add()`` now propagates the underlying exception when the LLM call itself
fails, matching the existing behavior of other failures (e.g. embedding
failures already bubble up).

Required for the openclaw-mem0 / mem0-server stack to fall back from xAI to
Anthropic/OpenAI/local when the primary provider rate-limits or errors out.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@blazingretinol

Copy link
Copy Markdown
Contributor

closing this pr in favour of #5878

thank you for your contribution!

RudrenduPaul added a commit to RudrenduPaul/memtrust that referenced this pull request Jul 17, 2026
mem0's store()/add() can return a normal-shaped response with no
exception while its extraction pipeline persisted nothing -- the
response is byte-identical to a genuine successful store from the
adapter's vantage point (mem0ai/mem0#5178). _extract_memory_id()
previously returned "" silently in that case with no other signal.

Add ExtractionSignal (FACTS_EXTRACTED/EMPTY_EXTRACTION/NOT_APPLICABLE)
to base.py, mirroring ConflictSignal/RankingSignal/CorruptionSignal's
conventions, and wire it into StoreResult.extraction_signal. Set it in
Mem0Adapter, Mem0SelfHostedAdapter, and Mem0DirectAdapter's store()
(NOT_APPLICABLE for Mem0DirectAdapter's CONFIG_REJECTED path, since a
rejected construction never ran extraction at all). Add unit tests for
both the empty- and populated-extraction cases across all three
adapters. Document the signal and its honest scope (a value-neutral
flag, not a bug-vs-legitimate-empty-extraction verdict) in
docs/methodology.md and CHANGELOG.md.
RudrenduPaul added a commit to RudrenduPaul/memtrust that referenced this pull request Jul 18, 2026
mem0's store()/add() can return a normal-shaped response with no
exception while its extraction pipeline persisted nothing -- the
response is byte-identical to a genuine successful store from the
adapter's vantage point (mem0ai/mem0#5178). _extract_memory_id()
previously returned "" silently in that case with no other signal.

Add ExtractionSignal (FACTS_EXTRACTED/EMPTY_EXTRACTION/NOT_APPLICABLE)
to base.py, mirroring ConflictSignal/RankingSignal/CorruptionSignal's
conventions, and wire it into StoreResult.extraction_signal. Set it in
Mem0Adapter, Mem0SelfHostedAdapter, and Mem0DirectAdapter's store()
(NOT_APPLICABLE for Mem0DirectAdapter's CONFIG_REJECTED path, since a
rejected construction never ran extraction at all). Add unit tests for
both the empty- and populated-extraction cases across all three
adapters. Document the signal and its honest scope (a value-neutral
flag, not a bug-vs-legitimate-empty-extraction verdict) in
docs/methodology.md and CHANGELOG.md.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants