feat: add brief retrieval mode for wing/room overviews#601
feat: add brief retrieval mode for wing/room overviews#601mvanhorn wants to merge 4 commits intoMemPalace:developfrom
Conversation
Add MemoryStack.brief() that returns a deduplicated, truncated overview of a wing/room's content. Uses L2 retrieval with deduplication (70% word overlap threshold) and a 2000-char cap. Also adds: - searcher.brief() wrapper function - `mempalace brief` CLI subcommand - Test in test_searcher.py Closes MemPalace#73.
Adds guard for "Retrieval error" prefix alongside existing "No " check so L2 errors don't get parsed as content lines. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
web3guru888
left a comment
There was a problem hiding this comment.
Good rebase — this feature has been wanted for a while and the implementation is clean. A few observations:
What works well
- The 70% word-overlap dedup threshold is reasonable for catching near-duplicate drawer summaries
- 2000-char cap is a sensible default for a "catch me up" context injection
- Dispatching to L2 for the underlying retrieval is correct (fast, no embedding cost)
- CLI integration follows the existing pattern exactly
One correctness concern — dedup math
The overlap check:
if len(words & prev_words) > 0.7 * max(len(words), len(prev_words), 1):This is asymmetric in a way that can produce false positives. A short line (3 words) that shares 3 words with a long line (20 words) will pass because 3 > 0.7 * 3, even though the long line has lots of unique content. Consider Jaccard similarity instead:
intersection = len(words & prev_words)
union = len(words | prev_words)
if union > 0 and intersection / union > 0.6: # Jaccard thresholdOr keep the current approach but use min() instead of max():
if len(words & prev_words) > 0.7 * min(len(words), len(prev_words), 1):Minor: parsing L2 output
brief() parses the string output of l2.retrieve() to get individual lines — this creates a text→parse→text pipeline that's fragile if L2's output format changes. If l2.retrieve() can return raw drawer data (or if there's a retrieve_raw() method), using that would be more robust. Not a blocker, just worth considering for the long term.
Tests
test_brief checks the output contains "Brief" and "topics" — that's the minimum needed to pass. A test for the dedup behavior (e.g., two near-identical drawers → only one in output) would give more confidence. Also worth a test for the empty-palace case since brief() has a special code path for raw.startswith("No ").
568 tests passing is solid. LGTM with the minor dedup note — would be comfortable approving after the math is verified.
Adds two tests to back the brief() retrieval mode: - test_brief_empty_palace: verifies brief() surfaces L2's "no drawers" message instead of crashing when the palace has no data - test_brief_dedupes_near_identical_drawers: verifies the >70% word overlap dedup actually collapses duplicates while preserving distinct content
|
Thanks for the careful read @web3guru888. On the dedup math: I traced through the example you raised. With Good points on the test coverage though. Added two tests in f4b507e:
On the L2 string-parsing point: agreed it's slightly fragile, but adding a 570 tests passing. |
|
Thanks for the trace-through on the dedup math — you're right, I had it backwards. The new tests look solid — 570 passing + these additions. LGTM from me. |
web3guru888
left a comment
There was a problem hiding this comment.
All feedback addressed — dedup math confirmed correct (max() is intentional and right), test gaps filled with two new tests, L2 string parsing noted for follow-up PR. 570 passing.
|
Thanks @web3guru888 — appreciate the close read. Agree retrieve_raw is a separate PR. |
Reopens #174 (closed for merge conflicts) — rebased onto current main and conflicts resolved.
What does this PR do?
Adds a
brief()retrieval mode that returns a deduplicated, truncated overview of a wing or room's contents. Complements the existingrecall()(raw L2 retrieval) andsearch()(semantic L3 search) with a "catch me up" mode.How it works:
Available via:
MemoryStack.brief(wing=..., room=...)in Pythonsearcher.brief()wrappermempalace brief --wing X --room YCLI subcommandCloses #73.
Rebase notes
Conflicts resolved in two files:
mempalace/cli.py— dispatch table (kept bothmcpand newbriefentries)tests/test_searcher.py— kept all the newTestSearchCLItests from main and addedtest_brieftoTestSearchMemoriesHow to test
Or run the test:
Checklist
pytest tests/→ 568 passed)ruff check .)This contribution was developed with AI assistance (Codex).