fix(memory): close the rating loop and stop diluting what gets served#81
Merged
Conversation
Three defects kept memories from being marked useful. Measured with live headless A/B sessions against sandboxed copies of a real memory DB. 1. Nothing was ever rated. The Stop hook was registered async, but its RATE_MEMORIES sweep replies with a `decision: "block"` payload — a control-flow response Claude Code only honours from a blocking hook. Registered async it is silently dropped, so the rating turn never runs and the ranking/retention feedback loop never closes. Identical task, identical DB: async => 44 exposed / 0 rated / 0 useful; sync => 44 / 44 / 9 (20.45%). On the live DB, 30 of 39 sessions in the measured month had zero ratings. Ruled out by probe: pythonw.exe does NOT null stdout through a pipe. See docs/decisions/stop-hook-must-be-sync.md. 2. Retrieval was not conditioned on the task. memory.retrieve accepted only project/tech/phase/polarity and ranked by a pure popularity prior (useful_count + 3*times_overlooked), so every caller got the same rows whatever they were doing, and use_cases/hints — the columns describing when a reflection applies — were never scored against anything. Adds an optional `query`: BM25 over title/use_cases/hints, RRF-fused with the prior. It promotes, never discards; a query matching nothing degrades exactly to the previous order. Not marked required — that would break every existing caller for a param whose absence is already safe. 3. Nothing demoted memories that keep not mattering. `ignored` was a no-op on the memory row, so the ranking key had no negative term: a reflection served 142 times and useful zero times sorted level with one never served at all. On a live DB, 55 such memories accounted for 27.5% of every rated exposure. Migration 0013 adds times_ignored/last_ignored_at and backfills them from existing rating history, so the signal is live immediately rather than relearned. Demotion applies only past a 10-session floor and only to memories with no useful history — the best-performing memory in the live DB is ignored more often than it is used. Also drops the default limit_per_bucket from 20 to 5. An LLM working one task draws on ~5 memories however many it is handed (measured 5.3 +/- 2.4 useful against 42.7 exposed per session); the rest is dilution and burnt context. Counts times_ignored once per session, not once per exposure row: a memory retrieved five times in one session that lands nowhere failed once, not five times. This matches how _apply_one already stamps every row for a memory with a single classification. Test-shape updates are golden-value assertions that pinned the old async / pythonw Stop registration and the pre-Phase-6 absence of `query`, not behavioural checks. Full suite: 1492 passed, 22 skipped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e time
A session's exposures are a set: list_session_exposures groups by (kind, id)
and the rating path stamps every row for a memory with one classification.
But the PK includes exposed_at, so every re-serve of an already-exposed
memory added a new row that existed only to inflate statistics computed over
the raw table. Measured on live data the same behaviour read as 16.08%
"useful" raw and 9.25% deduplicated.
Guard all three write paths (bootstrap, reflection retrieve, semantic
retrieve) with INSERT ... WHERE NOT EXISTS on (session_id, kind, memory_id).
First exposure wins, including its source label.
Updates test_full_rating_loop, which had codified the double-count ("PK is
distinct -> 5 new rows") as expected behaviour.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟢 Claude BugBot Analysis
Reviewed the diff (hook install sync/stdout fix, ignored-counter migration + demotion ranking, query-relevance RRF fusion, and exposure-dedup INSERT..WHERE NOT EXISTS changes) and verified SQL parameter ordering, HookSpec field ordering, FTS5 rowid linkage, and cross-file signature consistency. No genuine defects found.
No bugs were detected in this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Overnight A/B-validated fixes that raise the share of served memories an LLM marks useful (
cited/shaped) from 0% (shipped config) to 22.96% (n=24 live headless sessions, 100% rating coverage). Full experiment log inautoresearch/memuse-260721-run/on the workstation (REPORT.md / PLAN.md).Defects fixed
Stophook was registeredasync: true, but the RATE_MEMORIES sweep replies with adecision: "block"control-flow payload that Claude Code only honours from a blocking, stdout-attached hook. Result: 30/39 recent live sessions rated zero memories. Nowis_async=False, needs_stdout=True. Seedocs/decisions/stop-hook-must-be-sync.md. (pythonw stdout-nulling was probed and ruled out as the cause.)memory.retrieveaccepted onlyproject/tech/phase/polarityand ranked purely by popularity, so every session got the same rows. Adds optionalquery: BM25 overtitle/use_cases/hints(existingreflection_fts), RRF-fused with the popularity prior. Promotes, never discards; no-match degrades to the previous order. Deliberately not required — that breaks every existing caller.limit_per_bucket20 → 5. Measured: an LLM applies ~5 memories per session (5.3 ± 2.4) regardless of how many are served (42.7 mean).ignoredratings were write-only; 55 never-useful memories consumed 27.5% of all rated exposures. Migration 0013 addstimes_ignored/last_ignored_at(backfilled from existing history, counted per distinct session). Ranking demotes only past a 10-ignored-session floor and only for memories with zero useful history.exposed_at, so re-serves added rows — same behaviour read as 16.08% raw vs 9.25% deduped on live data. All three write paths now insert at most one row per (session, kind, memory); first source wins.Validation
Live A/B: real
claude -psessions against sandboxedBETTER_MEMORY_HOMEcopies of the production DB; 12 tasks across 5 projects; genuine exposure + rating rows. Live DB never touched.async/pythonwStop, absence ofquery) are golden-value assertions updated to the new contract, not weakened behaviour checks.Deploy notes
After merge: rerun
python -m better_memory.cli.install_hooks, restart Claude Code. Migration 0013 applies on first connection; the backfill makes demotion effective immediately.🤖 Generated with Claude Code