[xxwtiancai]

Link to Issue or Description of Change

Fixes #7688. Follows the maintainer's requested approach and validation.

A visible-text SQLite memory match currently returns thought parts from the same stored event, and LoadMemoryTool includes them in the next model request. Filter thought-marked parts in _row_to_memory_entry() immediately after model_validate_json.

The two-line production change covers existing rows and both FTS/LIKE paths. Stored content_json, visible text and non-text parts, Content.role, and existing identity metadata are preserved. Ingestion, schema, indexing, matching, and ranking are unchanged.

Testing Plan

Validated on b89dda409e062bd20015ad9508cf75d252dd2ab9 plus this three-file patch, using existing Python 3.11–3.14 environments on macOS arm64.

Update: the branch has since been rebased onto main at c31856d17ab54d82dbe89b4e511d34395ffc977b (the upstream additions touch eval, CLI ASGI disconnect handling, class descriptors, and the BigQuery analytics plugin — none overlap these files or the SQLite/Runner/LoadMemoryTool implementation). The focused suites below were re-run on the rebased branch on Python 3.11.14, macOS arm64: 68 passed, with the same revert check still failing 12 of 14 cases without the fix.

Maintainer acceptance coverage — 14 added cases:

Test	Cases	What it verifies
test_search_returns_visible_parts_without_mutating_events	8	FTS on/off × memory/file DB × event/session ingestion; removes text and non-text thoughts; preserves visible inline data, ordering, role and identity fields; input Event and file-backed stored JSON remain unchanged.
test_search_does_not_match_thought_text	2	Thought-only queries still return no match in either FTS mode; matching behavior is unchanged.
test_search_filters_preexisting_thoughts_without_rewriting_storage	2	Directly inserts literal old content_json through SQLite, without ingestion, then reopens and searches; preserves exact stored JSON, visible file data, role and identity metadata.
test_runner_loads_only_visible_sqlite_memory	2	Real Runner and LoadMemoryTool with the existing scripted MockModel; inspects the actual second model request's FunctionResponse and excludes the synthetic thought token.

The old-row cases use the current supported schema; they do not claim every historical schema. Independent Astra xhigh review found no actionable patch issue: all 14 cases passed, six additional review-only boundary probes passed, and restoring the original row decoder in an isolated test process caused 12 expected content-assertion failures while the two thought-query controls still passed.

Focused unit and Runner checks:

PYTHONPATH=src python -m pytest -q \
  tests/unittests/memory/test_sqlite_memory_service.py \
  tests/unittests/memory/test_in_memory_memory_service.py \
  tests/unittests/tools/test_load_memory_tool.py \
  tests/unittests/tools/test_preload_memory_tool.py

Python	Result
3.11.14	137 passed
3.12.15	137 passed
3.13.5	137 passed
3.14.9	137 passed

No focused case skipped. To reproduce only the real Runner/model-input boundary:

PYTHONPATH=src python -m pytest -q \
  tests/unittests/tools/test_load_memory_tool.py \
  -k runner_loads_only_visible_sqlite_memory
# 2 passed (FTS off/on)

The test sets up SQLite history, a scripted BaseLlm via the existing MockModel, and the real Runner wrapper with load_memory_tool. Its first response calls load_memory(query="saffron"); assertions inspect response.result.memories in the next received request, not the scripted final answer. No model credentials or remote provider calls were used.

Recorded console excerpts (Python 3.11):

# Four-file source suite
137 passed, 2 warnings in 102.13s (0:01:42)
EXIT_CODE=0

# Runner-only cases against the unpacked, source-verified wheel
2 passed, 6 deselected, 3 warnings in 0.89s

Broader supported-Python tox:

The full suite is not green. All four final runs completed:

Python	Passed	Failed	Skipped	Xfailed	Xpassed
3.11.14	20,966	1	97	18	2
3.12.15	20,957	1	98	18	2
3.13.5	20,957	1	98	18	2
3.14.9	20,958	1	97	18	2

Each run also passed 42 subtests and exited 1; no collection errors remain in these final runs.

The runs used the repository tox configuration with existing dependencies (--skip-uv-sync --skip-env-install --no-provision), one version at a time, and two pytest workers. The full tests/unittests tree was selected except the one previously denied socket test below. This is reuse-mode validation, not a pristine dependency rebuild.

• The sole failing node in each completed final run is tests/unittests/integrations/vmaas/test_agent_engine_environment.py::test_reading_directory_raises (DID NOT RAISE OSError). It fails identically on the exact unchanged b89dda4 baseline under all four interpreters.
• Explicitly excluded: tests/unittests/models/test_apigee_llm.py::test_completions_http_client_streams_longer_than_request_timeout. An earlier asyncio.start_server(..., "127.0.0.1", 0) was denied by the sandbox. That bind was not retried or escalated; this behavior remains unverified here.
• The first current-base full attempt also encountered two CrewAI collection errors when its default storage path tried to create a directory outside the workspace. That run was stopped. After inspecting its supported CREWAI_STORAGE_DIR setting, storage was placed inside the task workspace; all nine CrewAI controls passed on both the unchanged base and patch before the final runs above. The original default storage directory was not created; no security settings or dependencies were changed.
• The MacBook executor disconnected during the corrected Python 3.12 run. Its incomplete result was retained separately; after reconnection and checking that no process still held its output log, only unfinished versions were resumed. The completed Python 3.11 result was retained without repeating that run.

Additional validation:

• Pyink, isort, ruff, codespell, ADK compliance, documentation checks, and git diff --check passed for the applicable scope.
• Paired current-base/patched mypy logs are byte-identical: 236 existing imported-file errors, no added errors, and none in the changed production module.
• Wheel build passed with an existing compliant flit-core backend; 976 packaged files match the validated source. From the unpacked wheel, the minimal SQLite check passes both FTS modes and the real Runner cases report 2 passed, 6 deselected.
• No pristine tox rebuild, clean-environment wheel installation, aggregate pre-commit wrapper run, or live-provider behavior is claimed. Hook caches were unavailable; applicable standalone checks ran directly.

Checklist

• Read CONTRIBUTING.md and reviewed the focused change.
• Added tests demonstrating the defect, old-row protection, and preserved data.
• Tested the actual Runner/LoadMemoryTool model-input boundary.
• Completed all four supported Python versions with explicit results and environment limits.
• All project unit tests pass locally; the baseline failure and exclusion are documented above.
• Complete pristine-environment and clean wheel-install validation.
• No dependent downstream change is required.

Prepared and independently reviewed with AI assistance. InMemoryMemoryService and PreloadMemoryTool are existing fixed controls; the production change is limited to SQLite return filtering.

[xxwtiancai]

Hi @surajksharma07 — gentle ping on this PR, which implements the approach you suggested in #7688 (filtering thought-marked parts in _row_to_memory_entry() right after model_validate_json). All checks are green and the 14 added cases cover both FTS modes, legacy rows, and the real Runner model-input boundary. Happy to address any feedback, or to reroute if someone else is the right reviewer — thank you!

[BlueRaddish]

Reviewed 4197c69f16293d2c1b41aafb9a5063da86d42867 and the agreed scope in #7688. No blocking finding in the three-file change: filtering immediately after row deserialization covers both search paths and existing rows while leaving stored content and search indexing unchanged.

Independent execution on Ubuntu / Python 3.13:

• All 14 added author cases pass, including the actual Runner + LoadMemoryTool second-request boundary with the existing scripted model. Restoring the original production decoder from upstream base c31856d17ab54d82dbe89b4e511d34395ffc977b gives 12 assertion failures / 2 thought-query controls passing.
• Six independent public SqliteMemoryService cases pass with real SQLite/FTS5: both FTS modes, mixed visible/thought parts, pre-seeded rows whose returned parts become empty, and visible-only controls. They also check Content role, identity metadata, unchanged input/stored JSON, isolation from caller mutation, and user scoping. The original decoder fails the four thought-filtering cases and passes the two visible controls.
• The complete SQLite/InMemory memory and LoadMemory/PreloadMemory owning suites pass 137 tests, 0 skipped.
• Normal changed-file pre-commit checks pass without tracked changes; four nonapplicable hooks are skipped.

Actual corrected job and raw console output. The first review-controller attempt omitted the async marker on my external probes and stopped before their bodies ran; that failed attempt is retained separately. The corrected attempt adds only the explicit pytest-asyncio marker and keeps source and assertions unchanged.

This approves the focused return-filtering implementation. I did not rerun the full tox matrix, build/install a wheel, or evaluate a live provider/model; the PR's broader tox baseline failure remains outside this independent check. Current-head GitHub checks are administrative checks, not evidence of a full test-suite run. Review prepared with AI assistance; no author source or PR metadata was edited.