Skip to content

fix(search): return round-trippable drawer IDs - #2090

Merged
igorls merged 1 commit into
MemPalace:developfrom
fatkobra:fix/2080-round-trippable-drawer-id
Aug 2, 2026
Merged

fix(search): return round-trippable drawer IDs#2090
igorls merged 1 commit into
MemPalace:developfrom
fatkobra:fix/2080-round-trippable-drawer-id

Conversation

@fatkobra

Copy link
Copy Markdown
Contributor

What does this PR do?

  • add a round-trippable drawer_id to every search_memories() result path
  • return parent_drawer_id for chunked MCP drawers
  • fall back to the stored row ID for ordinary, mined, and legacy drawers
  • preserve query IDs through the filtered-query recovery path
  • add regression coverage for vector, SQLite BM25-only, and backend lexical-union results

Problem

A single ID choice does not work for every stored drawer.

A raw physical chunk ID is insufficient for a chunked MCP drawer because
mempalace_get_drawer resolves that physical row directly and returns only
that chunk. Conversely, mined and ordinary single-row drawers generally do
not have a parent_drawer_id.

The public result contract therefore needs to be conditional per hit:

drawer_id = parent_drawer_id or stored_drawer_id

This makes the returned ID directly usable with mempalace_get_drawer for
both chunked and non-chunked drawers.

Implementation

Vector and hybrid results

The IDs returned by the backend query are aligned with the document column.
Each result exposes its logical parent when one exists and otherwise exposes
its own stored ID.

The ID list is defensively padded for compatibility with legacy test mocks
that return documents but omit query IDs.

Filtered-query recovery

The unfiltered retry plus Python-side metadata filtering now preserves IDs
alongside documents, metadata, and distances.

Without this, a filtered search that entered the recovery path would still
lose the drawer identity.

SQLite BM25-only fallback

The metadata query now joins embedding_metadata to embeddings and
retrieves embeddings.embedding_id.

This is the stored public Chroma ID. The internal numeric SQLite row ID is
not exposed to callers.

Backend lexical union

_merge_bm25_union_candidates() uses LexicalHit.id as the stored-ID
fallback and uses parent_drawer_id when the lexical hit represents a
chunked logical drawer.

MCP retrieval

This PR does not change the MCP drawer retrieval implementation. It fixes
the ID produced by search so the existing mempalace_get_drawer logical
chunk-group reconstruction is reached for chunked drawers.

Tests

Regression coverage verifies:

  • a chunked vector hit returns parent_drawer_id;
  • a non-chunked vector hit returns its stored row ID;
  • a real non-chunked Chroma result exposes its expected stored ID;
  • the ID returned for a chunked search hit round-trips through
    mempalace_get_drawer and reconstructs the complete content;
  • IDs remain aligned when the filtered-query recovery path removes
    non-matching results;
  • SQLite BM25-only results use parent-or-stored-ID semantics;
  • backend lexical-union results use parent-or-LexicalHit.id semantics;
  • internal _parent_drawer_id remains absent from the public response.

Relationship to #1219

#1219 exposes the raw ID returned by the normal vector query.

This PR implements the broader contract reported in #2080:

  • chunked results need the logical parent ID;
  • ordinary and mined results need their own stored ID;
  • the same behavior must hold in the vector/hybrid, SQLite BM25-only, and
    backend lexical-union result builders;
  • the filtered-query recovery path must preserve IDs as well.

This is not intended to disregard #1219. Maintainers can decide whether this
PR should replace that implementation or whether the existing PR should be
adapted.

Fixes #2080

Related to #1026
Related to #1219

How to test

  python3 -m ruff format --check .
  python3 -m ruff check .
  python3 -m pytest tests/test_search_drawer_id.py -v
  python -m pytest tests/ -v

Checklist

  • Tests pass (python -m pytest tests/ -v)
  • No hardcoded paths
  • Linter passes (ruff check .)

gnusam pushed a commit to gnusam/mempalace-pgsql that referenced this pull request Jul 29, 2026
Adapts upstream PR MemPalace#2090. search_memories hits carried no ID at all in
this fork, so a search result could not feed mempalace_delete_drawer or
any dedup flow without a separate lookup. Each hit now includes the
stored drawer ID alongside text/wing/room/similarity.

Co-authored-by: fatkobra <fatkobra@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@igorls
igorls merged commit c57b1cd into MemPalace:develop Aug 2, 2026
8 checks passed
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.

search_memories drops the parent drawer id, so a hit cannot be traced back to its drawer

2 participants