FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(tools): use full query text and own logger in PreloadMemoryTool by chelsealong · Pull Request #6885 · google/adk-python · GitHub

fix(tools): use full query text and own logger in PreloadMemoryTool - #6885

Open
chelsealong wants to merge 1 commit into
google:mainfrom
chelsealong:fix-preload-memory-query-and-logging
Open

fix(tools): use full query text and own logger in PreloadMemoryTool#6885
chelsealong wants to merge 1 commit into
google:mainfrom
chelsealong:fix-preload-memory-query-and-logging

Conversation

Copy link
Copy Markdown
Contributor

Fixes #6884

Problem

PreloadMemoryTool.process_llm_request in preload_memory_tool.py had two issues, both present on main:

  1. The search query was read only from user_content.parts[0].text. If the first content part is non-text (a file, image, or a placeholder text part some LLM providers require ahead of a non-text part), the tool either returns early and never searches memory, or searches memory using the placeholder text instead of the user's actual question. The memory-rendering side of the same file (_memory_entry_utils.extract_text) already joins text across all parts — only the query-building side didn't.
  2. A failed search_memory call was logged via the root logging module instead of the module's own google_adk.* logger, so applications that configure logging by the google_adk namespace never see the failure. Combined with (1)'s silent early return, a memory backend that is down or misconfigured is indistinguishable from "no memories matched".

Fix

  • Build the query by joining text across every part of user_content, mirroring the existing extract_text idiom, instead of only looking at parts[0].
  • Log retrieval failures through the module's own logger (google_adk.<module>) instead of the root logging module.

This keeps the existing fail-open behavior (per #3069) — retrieval failures still don't raise — but they are now observable through the correct logger, and the query no longer silently drops or mis-targets when the first part isn't text.

Test plan

Added two tests to tests/unittests/tools/test_preload_memory_tool.py:

  • test_preload_memory_uses_text_from_every_part — a leading empty-text part must not blank out the real query in a later part.
  • test_preload_memory_logs_search_failure_on_own_logger — a retrieval failure must be observable on the google_adk logger.

Verified both fail without the fix (reverted preload_memory_tool.py to the pre-fix version and reran):

FAILED tests/unittests/tools/test_preload_memory_tool.py::test_preload_memory_uses_text_from_every_part - AssertionError: Expected search_memory to have been awaited once. Awaited 0 times.
FAILED tests/unittests/tools/test_preload_memory_tool.py::test_preload_memory_logs_search_failure_on_own_logger - assert False
2 failed, 4 passed in 0.85s

With the fix restored, the full file passes:

tests/unittests/tools/test_preload_memory_tool.py::test_preload_memory_keeps_system_prefix_stable PASSED
tests/unittests/tools/test_preload_memory_tool.py::test_preload_memory_stays_after_function_response_boundary PASSED
tests/unittests/tools/test_preload_memory_tool.py::test_preload_memory_does_not_change_cacheable_prefix_fingerprint PASSED
tests/unittests/tools/test_preload_memory_tool.py::test_preload_memory_search_failure_is_noop PASSED
tests/unittests/tools/test_preload_memory_tool.py::test_preload_memory_uses_text_from_every_part PASSED
tests/unittests/tools/test_preload_memory_tool.py::test_preload_memory_logs_search_failure_on_own_logger PASSED
6 passed in 0.70s

Also ran the broader tools suite filtered on preload (7 passed, no regressions), and formatted with isort/pyink per CONTRIBUTING.md.

AI assistance disclosure

This change was written with the assistance of an AI coding agent (Claude), with the diff reviewed and tests verified by the submitter before opening this PR.

PreloadMemoryTool only read the search query from the first content
part, so a leading non-text part (a file/image, or a placeholder text
part some providers require) either skipped the search entirely or
searched memory using the placeholder instead of the user's actual
question. Join text across all parts, matching the same idiom already
used on the memory-rendering side in _memory_entry_utils.extract_text.

Also log retrieval failures through the module's own google_adk logger
instead of the root logger, so operators who configure logging by the
google_adk namespace can observe and alert on a failing memory backend
instead of it looking identical to "no memories matched".

Fixes google#6884
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PreloadMemoryTool builds its memory query from the first content part only, and reports retrieval failure only to the root logger

2 participants


Back | FazBrowse Home | New Git URL