Query-cache hit skips _last_search_summary, so a reused index reports zero passages for a full result

[laynepenney]

Summary

TranscriptIndex.lookup nulls _last_search_summary at entry and only assigns it deep in the computation. The query cache returns above that assignment. So on a cache hit the retrieval returns byte-identical content while the summary stays None — and any caller that reads the summary to report how much was retrieved reports zero.

The count is bound to a cache-miss side effect rather than to the retrieval.

Mechanism

src/synapt/recall/core.py:

1858   self._last_search_summary = None          # reset at entry

1875   cached = self._query_cache.get(cache_key)
1876   if cached is not None:
1877       return cached                          # early return, above the assignment

3578   self._last_search_summary = SearchResultSummary(...)   # only reached on a miss
3587   self._last_search_summary = SearchResultSummary(...)

The cache is per-instance (_query_cache, max 32 entries), so this affects any caller that holds one index across repeated queries.

Reproduction

Runs against this package alone; no other dependency.

import tempfile
from pathlib import Path
from synapt.recall.core import TranscriptChunk, TranscriptIndex

TOPICS = ["a bicycle repair", "a garden fence", "a kitchen faucet", "a laptop battery",
          "a bookshelf", "a car alternator", "a roof gutter", "a violin bridge",
          "a camera lens", "a door hinge", "a water heater", "a bathroom tile",
          "a lawn mower", "a window latch", "a stair rail"]

chunks = [
    TranscriptChunk(
        id=f"s{i:02d}:t0",
        session_id=f"{i:08d}-0000-0000-0000-000000000000",
        timestamp=f"2026-08-{6 + i:02d}T12:00:00+00:00",
        turn_index=0,
        user_text=f"doc-{i:02d} maintenance notes for {t}.",
        assistant_text=f"Recorded maintenance notes for {t}.",
    )
    for i, t in enumerate(TOPICS)
]

with tempfile.TemporaryDirectory() as d:
    index = TranscriptIndex(chunks, cache_dir=Path(d))
    for i in range(12):
        result = index.lookup("maintenance", max_chunks=5, max_tokens=2400)
        summary = getattr(index, "_last_search_summary", None)
        print(i + 1, len(result), "None" if summary is None else summary.chunk_blocks)

Observed:

call  len(result)  _last_search_summary
   1         1686  10
   2         1686  None
   3         1686  None
   ...
  12         1686  None

len(result) is identical on every call — the retrieval is correct and unchanged throughout. Only the reported summary disappears.

Why it matters

Any surface that renders "N passages retrieved" from this summary shows a correct result body beside the number 0, on every repeat of a query. A caller cannot distinguish "nothing matched" from "cached hit" without reaching further into private state, and the two mean opposite things.

Suggested fix

Either cache the summary alongside the result and restore it on a hit, or move the summary assignment above the cache return so it describes what is being returned in both paths. The first keeps the cache's benefit; the second is simpler if recomputing the summary is cheap.

A regression test wants two calls, not one: a single-call test passes under every implementation, because the defect only appears from the second identical query onward.

[laynepenney]

Apollo — correcting my own closing note. It points at a fix that does not fix.

The note says a regression test wants two calls rather than one. A two-call test already exists and passes.

tests/recall/test_hybrid_integration.py::test_cache_hit_returns_same_result:

index = _build_index(tmp_path)
result1 = index.lookup("database schema", max_chunks=5)
result2 = index.lookup("database schema", max_chunks=5)
assert result1 == result2
assert result1  # Non-empty

Both assertions are true, and true for the reason the defect exists: the cache returns byte-identical content, so result1 == result2 holds exactly. In the reproduction above, len(result) is 1686 on all twelve calls. The test is correct about the result body and blind to the summary, which is the thing that changes.

So the defect sits directly underneath a passing two-call cache test, and my note could send someone to add a second call, watch it go green, and conclude the issue is addressed.

The missing assertion is on the summary, not the second call. A regression test needs to read _last_search_summary (or whatever public accessor replaces it) on the second call and assert it still describes the returned content:

result1 = index.lookup("database schema", max_chunks=5)
summary1 = index._last_search_summary
result2 = index.lookup("database schema", max_chunks=5)
summary2 = index._last_search_summary

assert result1 == result2          # already covered by the existing test
assert summary2 is not None        # currently fails
assert summary2.chunk_blocks == summary1.chunk_blocks

The existing test is worth keeping as-is; it covers a different property correctly. This is an additional assertion, not a replacement.

One note for whoever picks this up: the two tests immediately below it, test_different_params_different_cache and test_cache_eviction, exercise the cache from the outside via _query_cache length. They are unaffected by this and are a good place to see the cache's intended contract.