Skip to content

feat(memory): improve multi-part text handling in PreloadMemoryTool #375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

kshitizz36
Copy link
Contributor

Summary

This PR enhances the PreloadMemoryTool class to properly handle multiple text parts within memory events. The implementation now correctly joins text parts from each event to provide comprehensive context to the LLM.

Changes

  • Improved text extraction from memory event parts with robust attribute checking
  • Enhanced the memory formatting logic to join multiple text parts with spaces
  • Added comprehensive error handling for various edge cases (missing attributes, empty collections)
  • Updated tests to validate correct handling of multi-part text events
  • Maintained clean output by properly filtering non-text content

Implementation Details

The implementation now:

  • Properly iterates through all events in each memory entry
  • Collects and joins all text parts from each event with proper spacing
  • Maintains author attribution and timestamp information
  • Handles edge cases gracefully (missing content, empty parts, non-text parts)

Testing

All tests are now passing with the updated implementation. The test suite covers:

  • Single text part events
  • Multiple text part events
  • Mixed content events (text and non-text)
  • Events without text content
  • Edge cases (missing or empty attributes)

This implementation focuses specifically on improving text part handling as requested in the review feedback.
#129

@kshitizz36
Copy link
Contributor Author

@hangfei @boyangsvl PTAL

@hangfei hangfei requested a review from seanzhou1023 May 30, 2025 19:25
@kshitizz36 kshitizz36 closed this May 31, 2025
@kshitizz36 kshitizz36 reopened this May 31, 2025
@seanzhou1023
Copy link
Collaborator

@kshitizz36 thank you for the PR and sorry for the delay. We were busy with Google I/O and just catch up. Could you please resolve the conflicts first ? Thank you :)

@kshitizz36
Copy link
Contributor Author

Hi @seanzhou1023 ,
Thank you for getting back to me! No worries about the delay - I completely understand how busy things get around Google I/O.
I've been trying to resolve the conflicts locally for some time now, but I'm running into issues that I can't seem to resolve in my current local environment setup. Despite trying multiple approaches (fetching latest changes, merging from main, rebasing, etc.), I'm unable to get the conflicts to resolve properly locally.
Would it be acceptable if I create a fresh PR with the same changes?

@kshitizz36 kshitizz36 closed this Jun 13, 2025
@kshitizz36 kshitizz36 force-pushed the enhance-preload-memory-tool branch from 7b64a5c to dbdeb49 Compare June 13, 2025 03:53
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.

2 participants