Skip to content

fix(searcher): scope _expand_with_neighbors by parent_drawer_id (#1580) - #1792

Closed
ggettert wants to merge 1 commit into
MemPalace:developfrom
ggettert:fix/expand-neighbors-parent-drawer-scope
Closed

fix(searcher): scope _expand_with_neighbors by parent_drawer_id (#1580)#1792
ggettert wants to merge 1 commit into
MemPalace:developfrom
ggettert:fix/expand-neighbors-parent-drawer-scope

Conversation

@ggettert

Copy link
Copy Markdown
Contributor

Fixes #1580.

Summary

Implements Option B from the issue: scope _expand_with_neighbors
by parent_drawer_id (in addition to source_file) when the matched
chunk carries one.

Why this is bigger than the one-line guard relaxation

Pre-fix, the helper bailed early when source_file was empty:

if not src or not isinstance(chunk_idx, int):
    return {"text": matched_doc, "drawer_index": chunk_idx, "total_drawers": None}

That guard masked the bug on the MCP-chunked path (where every chunk
has source_file="") by silently disabling neighbor expansion for it.
So the user-visible symptom on develop today is "neighbor expansion
quietly doesn't work for tool_add_drawer oversized content"; the
real leak only surfaces if two drawers ever share a non-empty
source_file.

This PR:

  1. Relaxes the early-return: bail only when both source_file and
    parent_drawer_id are missing.
  2. Adds parent_drawer_id to the $and filter (neighbor query and
    the total_drawers count query) when it's present on the matched
    chunk.
  3. Net effect: neighbor expansion is restored for the MCP-chunked path
    and hardened so cross-drawer leakage is impossible even if
    source_file collides.

Live verification

Beyond unit tests, I built three harnesses:

1. Adversarial collision (two drawers, shared non-empty
source_file, distinct parent_drawer_id):

DEVELOP BRANCH
Expansion contains ALPHA-MARKER
Expansion contains BETA-MARKER ❌ leak ✅ scoped out
total_drawers reported 6 (combined) 3 (correct)

2. Natural MCP path (oversized tool_add_drawer, no
source_file — Igor's exact repro):

DEVELOP BRANCH
Neighbor expansion runs ❌ short-circuits ✅ runs, scoped
total_drawers reported None 2 (correct)

3. Regression fingerprint (diary chunks + small drawers + closet
hits + MCP chunked drawers, 3 query patterns):

diff develop.json branch.jsonzero diff. Existing file-backed
and closet-boosted paths are byte-identical pre/post.

Test coverage

tests/test_closets.py::TestDrawerGrepExpansion:

Full suite: 259/259 in tests/test_closets.py + tests/test_mcp_server.py.
Ruff clean.

Out of scope

  • Whether source_file should default to a per-drawer synthetic key
    on the MCP path (the issue calls this out explicitly).
  • Whether to invalidate / repair palaces that already accumulated
    leak-prone metadata (no migration needed; the fix is purely at the
    query layer).

…alace#1580)

Implements Option B from MemPalace#1580: when the matched chunk carries a
parent_drawer_id, narrow both the neighbor query and the total_drawers
count query by it so cross-drawer chunk leakage cannot happen even when
multiple drawers share a non-empty source_file.

Also relaxes the early-return so neighbor expansion runs for the MCP
oversized-content path (where source_file="" but parent_drawer_id is
always present). Bails only when both keys are missing.

Adds two regression tests pinning the MemPalace#1580 repro shape and the
unchanged legacy source_file-only path.

Fixes MemPalace#1580

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request addresses issue #1580 by scoping neighbor expansion in _expand_with_neighbors using parent_drawer_id when source_file is empty or missing. This prevents chunks from unrelated drawers with empty source files from leaking into the neighbor query. The PR also updates the total drawers lookup to use the same scoped filter and adds corresponding unit tests to verify the new scoping behavior and ensure no regression on the legacy source-file-only path. There are no review comments, and I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@ggettert

Copy link
Copy Markdown
Contributor Author

Closing — I missed PR #1628 when scoping this work. #1628 also closes #1580 and implements the same parent_drawer_id scoping fix, covers both _expand_with_neighbors and the sibling search_memories enrichment loop via a shared _scoped_source_filter helper, and already has test coverage for both paths plus the empty-string edge case. Apologies for the duplicate noise.

@ggettert ggettert closed this Jun 12, 2026
@ggettert
ggettert deleted the fix/expand-neighbors-parent-drawer-scope branch June 12, 2026 17:00
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.

searcher: _expand_with_neighbors stitches unrelated chunks across MCP drawers sharing empty source_file

1 participant