Skip to content

fix(memory): prevent add_to_memory() from overwriting events of the same type#243

Closed
abikooo wants to merge 2 commits intomesa:mainfrom
abikooo:fix/memory-add-to-memory-overwrite
Closed

fix(memory): prevent add_to_memory() from overwriting events of the same type#243
abikooo wants to merge 2 commits intomesa:mainfrom
abikooo:fix/memory-add-to-memory-overwrite

Conversation

@abikooo
Copy link
Copy Markdown
Contributor

@abikooo abikooo commented Mar 18, 2026

Fixes #242
Fixes #137

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 18, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: df35a51e-e9c6-4e50-98a4-d7d0f397146f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@abikooo abikooo force-pushed the fix/memory-add-to-memory-overwrite branch from 93cb8c4 to baeb38f Compare March 18, 2026 21:10
@souro26
Copy link
Copy Markdown

souro26 commented Mar 19, 2026

Please add the new tests in existing test files instead of making a new one

f"got {content.__class__.__name__}: {content!r}"
)

if type == "observation":
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

observation still stores a dict while other types now store lists. this creates a mixed structure in step_content. It should be unified so all types follow the same format. also, all consumers of step_content now need to handle lists. please keep a single consistent structure to avoid bugs.

@abikooo abikooo force-pushed the fix/memory-add-to-memory-overwrite branch 2 times, most recently from a497afa to 7842511 Compare March 19, 2026 13:09
@abikooo abikooo force-pushed the fix/memory-add-to-memory-overwrite branch from 9908027 to ee6779c Compare March 19, 2026 13:20
@abikooo
Copy link
Copy Markdown
Contributor Author

abikooo commented Mar 19, 2026

@souro26

thanks for the review. looked into both points. moved tests into existing files and unified all step_content types including observations to use the same list-based structure. all 283 tests passing. lmk if anything else needs attention.

@wang-boyu
Copy link
Copy Markdown
Member

Thank you for working on this. Closing as superseded by #165, which has now been merged and closed #137.

@wang-boyu wang-boyu closed this Apr 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

3 participants