Skip to content

fix: surface summarize/retrieve failures instead of masking them#160

Open
fancyboi999 wants to merge 1 commit intoagentscope-ai:mainfrom
fancyboi999:fancy/fix-156-memory-error-propagation
Open

fix: surface summarize/retrieve failures instead of masking them#160
fancyboi999 wants to merge 1 commit intoagentscope-ai:mainfrom
fancyboi999:fancy/fix-156-memory-error-propagation

Conversation

@fancyboi999
Copy link

Summary

  • make summarize_memory() and retrieve_memory() fail loudly instead of masking inner errors with a secondary TypeError
  • require ReMe to be started before these memory APIs run
  • configure the summarize/retrieve agent chain to propagate inner exceptions on these paths

Root cause

BaseOp.call() returns a stringified error on the final retry when raise_exception=False.
Both summarize_memory() and retrieve_memory() then assumed the result was always a dict and accessed result["answer"] unconditionally.

That created two problems:

  • the original failure was hidden behind TypeError: string indices must be integers, not 'str'
  • return_dict=True was not a reliable workaround because the failure path could still return a string

What changed

  • add a small boundary check in ReMe to validate memory API results before unwrapping them
  • raise a clear RuntimeError if these APIs are called before await reme.start()
  • set raise_exception=True for the agents and tools created inside the summarize/retrieve flows so inner failures surface directly

Validation

  • python3 -m py_compile reme/reme.py
  • ad hoc local Python 3.11 verification:
    • calling summarize_memory() / retrieve_memory() before start() now raises a clear RuntimeError
    • forced inner failures from ReMeSummarizer / ReMeRetriever now surface as the original RuntimeError instead of being re-wrapped as TypeError

Closes #156

@cla-assistant
Copy link

cla-assistant bot commented Mar 17, 2026

CLA assistant check
All committers have signed the CLA.

@cla-assistant
Copy link

cla-assistant bot commented Mar 17, 2026

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.

@jinliyl jinliyl requested review from jinliyl and nitwtog March 17, 2026 06:58
@jinliyl
Copy link
Member

jinliyl commented Mar 20, 2026

@fancyboi999
Thanks for the feedback!
Hardcoding raise_exception to True might not be the best approach. I suggest making it a parameter of the ReMe function, with a default value of either False or True.
Currently, raise_exception triggers logger.exception within the BaseOp._handle_failure method to expose the error.

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.

summarize_memory() with return_dict=False (default) raises TypeError: string indices must be integers, not 'str'

2 participants