fix(memory): return custom_metadata from VertexAiMemoryBankService.search_memory - #6947
Open
Yatsuiii wants to merge 1 commit into
Open
fix(memory): return custom_metadata from VertexAiMemoryBankService.search_memory#6947Yatsuiii wants to merge 1 commit into
Yatsuiii wants to merge 1 commit into
Conversation
…arch_memory search_memory constructed every returned MemoryEntry with only author, content, and timestamp, silently dropping custom_metadata even though this service's own write path already treats it as first-class: _add_memories_via_create merges MemoryEntry.custom_metadata via _merge_custom_metadata_for_memory before writing, and add_memory's own docstring documents metadata as meaningful, round-tripped data. Adds _from_vertex_metadata / _from_vertex_metadata_value as the symmetric read-side counterpart to the existing write-side _to_vertex_metadata_value, handling the same four value kinds (bool/double/string/timestamp) rather than only strings. Verified with a live create -> retrieve round trip against a real Vertex AI Memory Bank instance before implementing, confirming the underlying SDK genuinely returns populated metadata that this class was never reading. Fixes google#6946
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6946
Summary
search_memoryconstructed every returnedMemoryEntrywith onlyauthor,content, andtimestamp, silently droppingcustom_metadataeven though this service's own write path already treats it as first-class:_add_memories_via_createmergesMemoryEntry.custom_metadatavia_merge_custom_metadata_for_memorybefore writing, andadd_memory's own docstring documents metadata as meaningful, round-tripped data. The write side reads it; the read side never returned it.Adds
_from_vertex_metadata/_from_vertex_metadata_valueas the symmetric read-side counterpart to the existing write-side_to_vertex_metadata_value, handling the same four value kinds (bool/double/string/timestamp) rather than only strings.Why this matters
A caller that writes
custom_metadataat creation time -- an application-specific identifier used to correlate a memory back to its source, for example -- had no way to get it back throughsearch_memory. No error, no warning; the field was silently empty on every returnedMemoryEntry.Testing plan
create->retrieveround trip against a real Vertex AI Memory Bank instance before implementing the fix, confirming the underlying SDK genuinely returns populatedmetadatathat this class was never reading (details in VertexAiMemoryBankService.search_memory never returns MemoryEntry.custom_metadata, even though the write path treats it as first-class #6946).test_search_memory_returns_custom_metadata(covers bool/double/string values) andtest_search_memory_with_no_metadata_returns_empty_dict(backward-compat: no metadata ->{}, neverNone), using the existingmock_vertexai_client/_AsyncListIteratorfixtures already in the test file.test_search_memoryconfirmingcustom_metadata == {}for the pre-existing no-metadata case.pytest tests/unittests/memory/: 89 passed.pytest tests/unittests(full suite): 13493 passed, 0 failed (90 skipped / 31 xfailed / 2 xpassed are pre-existing and unrelated).pre-commit run --all-fileson the changed files: all hooks pass, including the repo's own "ADK Compliance Checks".Scope note
An earlier draft of #6946 also claimed
MemoryEntry.id/memory_idforwarding was broken. Testing that specific claim live contradicted it (forwarding is conditional on an SDK-version compatibility check this file's own code already handles correctly via_get_create_memory_config_keys, and a direct low-level SDK call bypassing that check isn't a fair test of it). That claim was cut from the issue before filing and is out of scope for this PR.