From d4cfc9e5d659b62b6347bbfc894ca789fdd98843 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 6 Aug 2026 09:09:34 +0100 Subject: [PATCH 01/13] docs: spec for RATE_MEMORIES display snapshots + terse directive Root cause: agentcore exposures use AWS mem- ids; local title join misses, so the Stop-hook directive renders blank memory lines. Design: snapshot display text at exposure-record time, COALESCE on read, and cut the directive to a minimal format (rules stay in the skill). Co-Authored-By: Claude Fable 5 --- ...026-08-06-rate-directive-display-design.md | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-06-rate-directive-display-design.md diff --git a/docs/superpowers/specs/2026-08-06-rate-directive-display-design.md b/docs/superpowers/specs/2026-08-06-rate-directive-display-design.md new file mode 100644 index 0000000..b003ef3 --- /dev/null +++ b/docs/superpowers/specs/2026-08-06-rate-directive-display-design.md @@ -0,0 +1,127 @@ +# RATE_MEMORIES directive: display snapshots + terse format — design + +Date: 2026-08-06. Status: approved (user picked terse variant "minimal + 1 rule +line" and backfill option A). + +## Problem + +The Stop-hook RATE_MEMORIES directive renders each pending memory as an opaque +id with nothing after the colon: + +``` +- mem-f3ce58e6-0c5a-4a37-a87a-2c35272a8d82 [contextual]: +``` + +and the directive body restates ~600 chars of rating rules that already live in +the `rate-session-memories` skill. The user cannot tell what is being rated, +and the block is far larger than it needs to be. + +## Root cause (verified against live DB) + +`exposure_log.list_unrated` (better_memory/services/exposure_log.py) already +joins display text: + +```sql +COALESCE(r.title, s.content) AS display +LEFT JOIN reflections r ON e.memory_kind='reflection' AND e.memory_id = r.id +LEFT JOIN semantic_memories s ON e.memory_kind='semantic' AND e.memory_id = s.id +``` + +That join works for the sqlite backend, whose ids are bare 32-hex. The +agentcore backend records exposures under AWS-minted public ids +(`mem-`, server-minted per aws_record_dialect) which do not exist +in the local `reflections` / `semantic_memories` tables. The LEFT JOIN misses, +`display` is NULL, and both the Stop-hook directive and +`memory.list_session_exposures` render blank titles. + +The hook's inline copy of this query (better_memory/hooks/session_close.py, +`_emit_rating_directive_if_unrated`) has the same behaviour by construction — +it is a deliberate standalone copy to avoid a service-layer import. + +## Design + +### Change 1 — snapshot display text at exposure-record time + +Backend-agnostic fix: capture the display string when the exposure is written, +because every write site holds the full record at that moment. No network in +the Stop hook — it must stay synchronous and local +(docs/decisions/stop-hook-must-be-sync.md). + +- **Migration**: `ALTER TABLE session_memory_exposure ADD COLUMN display TEXT` + (nullable), registered via the repo's `schema_migrations` pre-claim + convention (see #122). +- **`exposure_log.record`**: items change from `(kind, id)` tuples to + `(kind, id, display)` triples. `display` is truncated to 120 chars at write + time and may be `None`. First-source-wins dedup semantics unchanged. +- **Write sites** (all already hold the record content): + - `storage/agentcore.py` — `_record_retrieve_exposures` (buckets contain + parsed records with titles) and the bootstrap/`record_exposures` path. + - sqlite path — `services/reflection.py` retrieve + + `services/session_bootstrap.py`. + - `storage/protocol.py` `record_exposures` takes the same + `(kind, id, display)` triples as `exposure_log.record` (one shape + everywhere, no separate display param); `hooks/contextual_inject.py` + passes it from the `retrieve_relevant` items it already renders. +- **Read path**: `list_unrated` and the hook's inline copy select + `COALESCE(e.display, r.title, s.content)`. Local-backend rows written before + the migration keep resolving via the join fallback. +- `memory.list_session_exposures` picks up the fix automatically (it delegates + to the same query shape), so the rating skill's authoritative list also + shows titles. + +### Change 2 — terse directive + +Approved format ("minimal + 1 rule line"): + +``` +RATE_MEMORIES: 15 unrated. Invoke skill `rate-session-memories`. +Evidence line first; none possible = `ignored`. +Reflections (13): +- mem-f3ce58e6... [contextual] Keep website and README in sync with every code change +- mem-1b742a09... [retrieve] Session startup MUST call knowledge_list + memory_retrieve +Semantic (2): +- mem-5f0f9a12... [contextual] Visualiser: mermaid + full HTML doc +``` + +- Dropped from the directive: the "before this session ends…" preamble, the + per-source counts line, the evidence-procedure paragraph, the class list, + and the rejection-rule sentence. All of those live in the + `rate-session-memories` skill, which the directive instructs the model to + invoke. +- Kept: the unrated count, one line per memory + (`- [] `), the single safety rule line + `Evidence line first; none possible = `ignored`.`, and the skill pointer. +- The `reason` field (`RATE_MEMORIES — N pending rating(s) for this session`) + is unchanged; the 8 KB cap with the truncate-and-point-at- + `memory.list_session_exposures` fallback is unchanged. + +### Backfill decision (option A — accepted) + +Exposure rows written before the migration have NULL `display`; agentcore-id +rows among them still render blank. Accepted as-is: unrated rows leave the +pool at the next rating turn, so the blank lines self-clear within one rating +cycle per session. No backfill script, no purge. + +## Error handling + +Unchanged. `record` stays best-effort (never blocks retrieve/inject); +`_emit_rating_directive_if_unrated` keeps its swallow-everything guard and +`record_hook_error` reporting. A NULL/absent display renders as an empty +suffix, never raises. + +## Testing + +- `tests/hooks/test_session_close_rating_directive.py`: golden assertions move + to the terse format; add a case proving snapshot display text appears for an + agentcore-style `mem-` id with no local table row. +- `tests/services/test_exposure_log.py`: triple items, display persisted and + truncated at 120, COALESCE precedence (snapshot beats join, join rescues + NULL snapshot). +- Migration test per repo convention (pre-claim + DDL). +- `tests/e2e/test_sqlite_journey.py`: update any pinned directive text. + +## Docs + +`website/mcp-tools.md` (list_session_exposures output now carries display) and +any README/website text quoting the directive format get swept in the same PR; +if genuinely unaffected, the PR description states "docs unaffected". From 8808657aa1e141cbb25c1c79d65d50ee9f27a99e Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 6 Aug 2026 09:16:47 +0100 Subject: [PATCH 02/13] docs: implementation plan for RATE_MEMORIES display snapshots Six tasks: display-column migration, exposure_log triples + COALESCE, protocol/backend threading, terse directive, full-suite gate, docs sweep. Spec amended: inline sqlite retrieve INSERTs stay join-resolved. Co-Authored-By: Claude Fable 5 --- .../2026-08-06-rate-directive-display.md | 626 ++++++++++++++++++ ...026-08-06-rate-directive-display-design.md | 7 +- 2 files changed, 631 insertions(+), 2 deletions(-) create mode 100644 docs/superpowers/plans/2026-08-06-rate-directive-display.md diff --git a/docs/superpowers/plans/2026-08-06-rate-directive-display.md b/docs/superpowers/plans/2026-08-06-rate-directive-display.md new file mode 100644 index 0000000..42d9125 --- /dev/null +++ b/docs/superpowers/plans/2026-08-06-rate-directive-display.md @@ -0,0 +1,626 @@ +# RATE_MEMORIES Display Snapshots + Terse Directive Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Stop-hook RATE_MEMORIES directive shows each pending memory's title instead of an opaque id, and shrinks to a minimal format; `memory.list_session_exposures` shows titles for agentcore-backed memories too. + +**Architecture:** Snapshot the memory's display text into a new nullable `display` column on `session_memory_exposure` at exposure-record time (every write site holds the full record then). Read path selects `COALESCE(e.display, r.title, s.content)` so pre-migration local rows keep resolving via the join. The Stop hook directive drops all rule text that lives in the `rate-session-memories` skill. + +**Tech Stack:** Python 3.12, sqlite3, pytest, ruff. No new dependencies. + +**Spec:** `docs/superpowers/specs/2026-08-06-rate-directive-display-design.md` + +## Guardrails (from project memory / standards) + +- **[[stop-hook-sync]]** (decision doc `docs/decisions/stop-hook-must-be-sync.md`): the Stop hook is synchronous and blocking. NO network calls, NO AWS SDK imports in `session_close.py`. This plan reads only the local ledger. (conf: accepted ADR) +- **[[docs-in-sync]]** (reflection mem-f3ce58e6, conf 1.0, used 21x): sweep `website/mcp-tools.md`, `README.md`, and module docstrings in the same PR; state "docs unaffected" explicitly if so. Task 6. +- **[[hooks-never-fail]]** (repo convention, session_close.py docstring): hooks always exit 0; exposure writes are best-effort and must never block retrieve/inject. Preserve every existing try/except guard. +- **[[ruff-py312]]** (standards doc): `from datetime import UTC`, no `(str, Enum)`, run `uv run ruff check` on all copied code before commit. +- Dismissed: visualiser/plan-render reflections (process, not code); worktree-on-Windows reflection (no worktree used); htmx/TypeScript/tempfile reflections (not touched by this plan). + +## Global Constraints + +- The Stop hook (`better_memory/hooks/session_close.py`) keeps its standalone inline SQL copy — do NOT import `better_memory.services.exposure_log` from it (no service-layer dependency, per the comment at session_close.py:69-72). +- `exposure_log` functions never call `conn.commit()` — callers own the transaction (module docstring contract). +- Exposure-ledger semantics are unchanged: at most one row per (session, kind, id), first source wins, dedup beats exploration tag. +- Display snapshots: truncate to 120 chars at write time; directive renders at most 80 chars per line (existing `TRUNC = 80`). +- The two inline sqlite exposure INSERTs in `services/reflection.py:1634` and `services/semantic.py:349` are intentionally NOT changed: they only ever write local-table ids, which the `r.title`/`s.content` join fallback resolves live. (Spec bullet listing them as snapshot writers is superseded by this — recorded here deliberately.) +- Test suite runner: `uv run pytest`. Lint: `uv run ruff check .`. + +--- + +### Task 1: Migration — `display` column on session_memory_exposure + +**Files:** +- Create: `better_memory/db/migrations/0017_exposure_display.sql` +- Test: `tests/services/test_exposure_log.py` (new test in existing file — its `conn` fixture already applies migrations) + +**Interfaces:** +- Produces: `session_memory_exposure.display TEXT NULL` — Tasks 2-5 depend on the column existing. + +- [ ] **Step 1: Write the failing test** + +Add to `tests/services/test_exposure_log.py` (top-level, after existing classes; reuse the module's `conn` fixture): + +```python +class TestDisplayColumn: + def test_session_memory_exposure_has_display_column(self, conn): + cols = { + row[1] for row in conn.execute( + "PRAGMA table_info(session_memory_exposure)" + ) + } + assert "display" in cols +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `uv run pytest tests/services/test_exposure_log.py::TestDisplayColumn -v` +Expected: FAIL — `assert 'display' in cols` (column absent) + +- [ ] **Step 3: Write the migration** + +Create `better_memory/db/migrations/0017_exposure_display.sql`: + +```sql +-- Display-text snapshot captured at exposure-record time. Nullable: rows +-- written before this migration (and callers with no display in hand) leave +-- it NULL and the read path falls back to joining reflections.title / +-- semantic_memories.content — which only resolves local-table ids, not +-- agentcore's AWS-minted mem- ids. See +-- docs/superpowers/specs/2026-08-06-rate-directive-display-design.md. +ALTER TABLE session_memory_exposure ADD COLUMN display TEXT; +``` + +(Before writing, open `better_memory/db/migrations/0016_rating_evidence.sql` and copy its exact header/comment style if it differs from the above.) + +- [ ] **Step 4: Run test to verify it passes** + +Run: `uv run pytest tests/services/test_exposure_log.py::TestDisplayColumn -v` +Expected: PASS + +- [ ] **Step 5: Run the full existing exposure/migration suites (regression)** + +Run: `uv run pytest tests/services/test_exposure_log.py tests/db -v` +Expected: all PASS + +- [ ] **Step 6: Commit** + +```bash +git add better_memory/db/migrations/0017_exposure_display.sql tests/services/test_exposure_log.py +git commit -m "feat: add display snapshot column to session_memory_exposure" +``` + +**Confidence: 95%** — additive nullable column, migration framework verified (`schema.py` globs `NNNN_*.sql`, 0016 is the latest). + +--- + +### Task 2: `exposure_log.record` triples + `list_unrated` COALESCE + +**Files:** +- Modify: `better_memory/services/exposure_log.py` +- Test: `tests/services/test_exposure_log.py` + +**Interfaces:** +- Consumes: `display` column from Task 1. +- Produces: `record(conn, *, session_id, items: list[tuple[str, str, str | None]], source, now, exploration_ids)` — items are `(kind, id, display)` triples, display may be None, truncated to 120 chars on write. `list_unrated` rows' `display` column becomes `COALESCE(e.display, r.title, s.content)`. Tasks 3-5 rely on exactly these signatures. + +- [ ] **Step 1: Write the failing tests** + +Add to `tests/services/test_exposure_log.py`: + +```python +class TestRecordDisplay: + def test_display_persisted(self, conn): + exposure_log.record( + conn, session_id="s1", + items=[("reflection", "mem-aws-1", "AWS Title")], + source="retrieve", now="2026-08-06T10:00:00+00:00", + ) + row = conn.execute( + "SELECT display FROM session_memory_exposure " + "WHERE memory_id = 'mem-aws-1'" + ).fetchone() + assert row["display"] == "AWS Title" + + def test_display_truncated_to_120(self, conn): + exposure_log.record( + conn, session_id="s1", + items=[("reflection", "mem-aws-2", "x" * 300)], + source="retrieve", now="2026-08-06T10:00:00+00:00", + ) + row = conn.execute( + "SELECT display FROM session_memory_exposure " + "WHERE memory_id = 'mem-aws-2'" + ).fetchone() + assert row["display"] == "x" * 120 + + def test_display_none_allowed(self, conn): + exposure_log.record( + conn, session_id="s1", + items=[("reflection", "mem-aws-3", None)], + source="retrieve", now="2026-08-06T10:00:00+00:00", + ) + row = conn.execute( + "SELECT display FROM session_memory_exposure " + "WHERE memory_id = 'mem-aws-3'" + ).fetchone() + assert row["display"] is None + + +class TestListUnratedDisplayCoalesce: + def test_snapshot_beats_join(self, conn): + conn.execute( + "INSERT INTO reflections (id, title, project, phase, polarity," + " use_cases, hints, confidence, created_at, updated_at)" + " VALUES ('r1', 'Live Title', 'p', 'general', 'do', 'uc', '[]'," + " 0.5, '2026-01-01', '2026-01-01')" + ) + exposure_log.record( + conn, session_id="s1", + items=[("reflection", "r1", "Snapshot Title")], + source="retrieve", now="2026-08-06T10:00:00+00:00", + ) + rows = exposure_log.list_unrated(conn, session_id="s1") + assert rows[0]["display"] == "Snapshot Title" + + def test_null_snapshot_falls_back_to_join(self, conn): + conn.execute( + "INSERT INTO reflections (id, title, project, phase, polarity," + " use_cases, hints, confidence, created_at, updated_at)" + " VALUES ('r2', 'Live Title', 'p', 'general', 'do', 'uc', '[]'," + " 0.5, '2026-01-01', '2026-01-01')" + ) + exposure_log.record( + conn, session_id="s1", + items=[("reflection", "r2", None)], + source="retrieve", now="2026-08-06T10:00:00+00:00", + ) + rows = exposure_log.list_unrated(conn, session_id="s1") + assert rows[0]["display"] == "Live Title" + + def test_foreign_id_null_snapshot_yields_none(self, conn): + exposure_log.record( + conn, session_id="s1", + items=[("reflection", "mem-aws-9", None)], + source="retrieve", now="2026-08-06T10:00:00+00:00", + ) + rows = exposure_log.list_unrated(conn, session_id="s1") + assert rows[0]["display"] is None +``` + +- [ ] **Step 2: Update existing `record` call sites in this test file to triples** + +Every existing `exposure_log.record(...)` call in `tests/services/test_exposure_log.py` passes 2-tuples like `("reflection", "r1")`. Change each to a 3-tuple appending `None`: `("reflection", "r1", None)`. Pure mechanical sweep of that one file. + +- [ ] **Step 3: Run tests to verify the new ones fail** + +Run: `uv run pytest tests/services/test_exposure_log.py -v` +Expected: `TestRecordDisplay` / `TestListUnratedDisplayCoalesce` FAIL (record raises or display column never written); pre-existing tests also FAIL on arity until Step 4. + +- [ ] **Step 4: Implement in `better_memory/services/exposure_log.py`** + +`record` — new signature and INSERT (dedup guard, no-commit contract, and exploration tagging unchanged): + +```python +_DISPLAY_TRUNC = 120 + + +def record( + conn: sqlite3.Connection, + *, + session_id: str, + items: list[tuple[str, str, str | None]], + source: str, + now: str, + exploration_ids: frozenset[str] = frozenset(), +) -> None: + if not session_id or not items: + return + conn.executemany( + "INSERT INTO session_memory_exposure " + "(session_id, memory_kind, memory_id, exposed_at, source, " + " via_exploration, display) " + "SELECT ?, ?, ?, ?, ?, ?, ? " + "WHERE NOT EXISTS (" + " SELECT 1 FROM session_memory_exposure " + " WHERE session_id = ? AND memory_kind = ? AND memory_id = ?)", + [ + ( + session_id, kind, mid, now, source, + 1 if mid in exploration_ids else 0, + (display[:_DISPLAY_TRUNC] if display else None), + session_id, kind, mid, + ) + for kind, mid, display in items + ], + ) +``` + +Update the docstring: items are `(kind, id, display)` triples; display is a snapshot of the memory's title/content captured at exposure time (None when the caller has none), truncated to 120 chars — needed because agentcore ids don't exist in local content tables. + +`list_unrated` — change only the display expression: + +```sql +COALESCE(e.display, r.title, s.content) AS display +``` + +Update the module docstring's "lifted verbatim" paragraph to note the display extension (this module is no longer byte-identical to the original inline SQL). + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `uv run pytest tests/services/test_exposure_log.py -v` +Expected: all PASS + +- [ ] **Step 6: Commit** + +```bash +git add better_memory/services/exposure_log.py tests/services/test_exposure_log.py +git commit -m "feat: exposure_log takes (kind, id, display) triples; COALESCE snapshot on read" +``` + +**Confidence: 95%** — single module, contract pinned by its own test suite which this task updates. + +--- + +### Task 3: Thread display through protocol + backends + hooks + +**Files:** +- Modify: `better_memory/storage/protocol.py:377-383` (record_exposures signature + docstring) +- Modify: `better_memory/storage/sqlite.py:439-448` +- Modify: `better_memory/storage/agentcore.py` (`_record_retrieve_exposures` ~:352-385, `record_exposures` ~:2076-2114) +- Modify: `better_memory/services/session_bootstrap.py` (`record_exposures` :146-177, `_record_exposure` :179-197, `bootstrap` deferred :285-289 and non-deferred :357-361) +- Modify: `better_memory/hooks/contextual_inject.py:167-173` +- Test: `tests/services/test_exposure_dedup.py`, `tests/services/test_exposure_tracking.py`, `tests/storage/test_agentcore_rating_loop.py`, `tests/storage/test_agentcore_unit.py`, `tests/integration/test_contextual_rating_e2e.py`, `tests/hooks/test_contextual_inject.py`, `tests/hooks/test_session_bootstrap.py` + +**Interfaces:** +- Consumes: Task 2's triple-taking `exposure_log.record`. +- Produces: `StorageBackend.record_exposures(*, session_id: str, items: list[tuple[str, str, str | None]], source: str)` — every implementer and caller uses `(kind, id, display)` triples. No other Protocol methods change. + +- [ ] **Step 1: Write the failing test (new behaviour: agentcore retrieve snapshots titles)** + +Add to `tests/storage/test_agentcore_unit.py`, near the existing `record_exposures` tests (~:2740; reuse that test's `backend_with_local_conn` fixture and follow its call style): + +```python +def test_record_exposures_persists_display(backend_with_local_conn): + backend, conn = backend_with_local_conn + backend.record_exposures( + session_id="s-disp", + items=[("reflection", "mem-aws-1", "AWS Reflection Title")], + source="contextual", + ) + row = conn.execute( + "SELECT display FROM session_memory_exposure " + "WHERE session_id = 's-disp' AND memory_id = 'mem-aws-1'" + ).fetchone() + assert row["display"] == "AWS Reflection Title" +``` + +(If `backend_with_local_conn` unpacks differently, mirror the fixture usage of the test at :2740 exactly.) + +- [ ] **Step 2: Run it to verify it fails** + +Run: `uv run pytest tests/storage/test_agentcore_unit.py::test_record_exposures_persists_display -v` +Expected: FAIL (TypeError on tuple arity, or display NULL) + +- [ ] **Step 3: Update production code — every site below, in one pass** + +1. `storage/protocol.py:381`: `items: list[tuple[str, str, str | None]]`; docstring: "Record (kind, id, display) memory exposures for later rating. display is a snapshot of the memory's title/content at exposure time (None when unavailable); it makes agentcore-id exposures renderable without any local content row." +2. `storage/sqlite.py:443`: same annotation; body unchanged (pass-through). +3. `storage/agentcore.py:2080`: same annotation; body unchanged (items flow into `exposure_log.record` verbatim). +4. `storage/agentcore.py` `_record_retrieve_exposures`: replace the `all_ids` list-comprehension + items construction: + +```python + all_items = [ + ("reflection", r["id"], r.get("title")) + for bucket in buckets.values() for r in bucket + ] + if not all_items: + return + ... + exposure_log.record( + self._local_conn, + session_id=sid, + items=all_items, + source="retrieve", + now=datetime.now(UTC).isoformat(), + exploration_ids=frozenset(exploration_ids), + ) +``` + +(Parsed records carry `"title"` — set in `_parse_reflection_record` at agentcore.py:953.) +5. `services/session_bootstrap.py:150`: `items: list[tuple[str, str, str | None]]` (docstring: "(kind, id, display) triples"); body unchanged (delegates to `exposure_log.record`). +6. `services/session_bootstrap.py` `_record_exposure`: change signature to take display maps and build triples: + +```python + def _record_exposure( + self, + *, + session_id: str, + reflection_ids: list[str], + semantic_ids: list[str], + reflection_display: dict[str, str | None], + semantic_display: dict[str, str | None], + ) -> None: + self.record_exposures( + session_id=session_id, + items=[("reflection", rid, reflection_display.get(rid)) + for rid in reflection_ids] + + [("semantic", sid, semantic_display.get(sid)) + for sid in semantic_ids], + source="bootstrap", + ) +``` + +Callers build the maps from objects already in scope: + - Non-deferred (`bootstrap`, ~:357): `reflection_display={r["id"]: r.get("title") for _, r in flat_reflections}`, `semantic_display={m.id: m.content for m in semantic}`. + - Deferred (~:285): `reflection_display={}`, `semantic_display={m.id: m.content for m in general_only}`. +7. `hooks/contextual_inject.py:167`: `survivors = [(m.kind, m.id, m.text) for m in items]` — but note `seen.mark_seen(survivors)` at :179 and `seen.filter_unseen(pairs, ...)` operate on (kind, id) pairs. Keep the pair list for the seen-store and build a separate triple list for `record_exposures`: + +```python + survivors = [(m.kind, m.id) for m in items] + exposure_items = [(m.kind, m.id, m.text) for m in items] + ... + backend.record_exposures( + session_id=session_id, + items=exposure_items, + source="contextual", + ) + ... + seen.mark_seen(survivors) +``` + +- [ ] **Step 4: Sweep remaining 2-tuple call sites** + +Run: `grep -rn "record_exposures(\|exposure_log.record(" better_memory/ tests/` +Every call passing 2-tuples gets `None` appended (tests) or a real display (production — but after Step 3 there should be NO production 2-tuple sites left; if grep finds one, fix it with a real display, not None). Files known to need the mechanical test sweep: `tests/services/test_exposure_dedup.py`, `tests/services/test_exposure_tracking.py`, `tests/storage/test_agentcore_rating_loop.py`, `tests/storage/test_agentcore_unit.py`, `tests/integration/test_contextual_rating_e2e.py`, `tests/hooks/test_contextual_inject.py` (its fake `record_exposures` at :305 forwards to `exposure_log.record` — forward the triples), `tests/hooks/test_session_bootstrap.py:428`. + +- [ ] **Step 5: Run the affected suites** + +Run: `uv run pytest tests/services tests/storage tests/hooks tests/integration -v` +Expected: all PASS (including Step 1's new test) + +- [ ] **Step 6: Type-check and lint** + +Run: `uv run ruff check . && uv run mypy better_memory` (skip mypy if the repo has no mypy config — check `pyproject.toml`; ruff is mandatory) +Expected: clean + +- [ ] **Step 7: Commit** + +```bash +git add better_memory/storage/protocol.py better_memory/storage/sqlite.py better_memory/storage/agentcore.py better_memory/services/session_bootstrap.py better_memory/hooks/contextual_inject.py tests/ +git commit -m "feat: thread display snapshots through record_exposures to all write sites" +``` + +**Confidence: 91%** — all production call sites read and enumerated above; residual risk is test-fixture shapes in the agentcore suites, contained by the Step 4 grep + Step 5 suite run. + +--- + +### Task 4: Terse directive + inline COALESCE in the Stop hook + +**Files:** +- Modify: `better_memory/hooks/session_close.py:73-134` (`_emit_rating_directive_if_unrated`) +- Test: `tests/hooks/test_session_close_rating_directive.py` + +**Interfaces:** +- Consumes: `display` column (Task 1). No dependency on Tasks 2-3 (inline SQL copy). +- Produces: directive format below. `reason` string and block-payload shape unchanged. + +Directive format (exact): + +``` +RATE_MEMORIES: {total} unrated. Invoke skill `rate-session-memories`. +Evidence line first; none possible = `ignored`. +Reflections ({n}): +- {memory_id} [{source}] {display-or-empty, max 80 chars} +Semantic ({m}): +- {memory_id} [{source}] {display-or-empty, max 80 chars} +``` + +Rules: a bucket with zero entries is omitted entirely (no "(none)" placeholder). A NULL display renders the line as `- {id} [{source}]` with no trailing space. The 8 KB cap and its truncation suffix (`(list truncated; call memory.list_session_exposures for the full set)`) are kept verbatim. + +- [ ] **Step 1: Update/replace tests** + +In `tests/hooks/test_session_close_rating_directive.py`: + +1. DELETE `test_directive_lists_overlooked_class` (class list now lives only in the skill). +2. REPLACE the body of `test_directive_requires_evidence_first` with: + +```python + def test_directive_keeps_one_rule_line(self, tmp_path, tmp_memory_db): + _seed_unrated_exposure(tmp_memory_db, "S1") + env = { + "BETTER_MEMORY_HOME": str(tmp_memory_db.parent), + "CLAUDE_SESSION_ID": "S1", + } + result = _run_hook(env) + assert result.returncode == 0 + payload = json.loads(result.stdout) + directive = payload["hookSpecificOutput"]["additionalContext"] + assert "Evidence line first; none possible = `ignored`." in directive + assert "rate-session-memories" in directive + # Rules that now live only in the skill must NOT be restated. + assert "Non-ignored ratings" not in directive + assert "overlooked" not in directive +``` + +3. REPLACE `test_directive_shows_source_labels_and_counts` with: + +```python + def test_directive_shows_source_labels_without_counts_line( + self, tmp_path, tmp_memory_db, + ): + _seed_unrated_exposure(tmp_memory_db, "S1", source="bootstrap") + _seed_semantic_exposure(tmp_memory_db, "S1", source="contextual") + env = { + "BETTER_MEMORY_HOME": str(tmp_memory_db.parent), + "CLAUDE_SESSION_ID": "S1", + } + result = _run_hook(env) + assert result.returncode == 0 + payload = json.loads(result.stdout) + directive = payload["hookSpecificOutput"]["additionalContext"] + assert "[bootstrap]" in directive + assert "[contextual]" in directive + assert "sources:" not in directive + assert "My Title" in directive + assert "My Semantic Fact" in directive +``` + +4. ADD two tests: + +```python + def test_display_snapshot_shown_for_foreign_id( + self, tmp_path, tmp_memory_db, + ): + """An agentcore-style exposure (mem- id, NO local content row) + renders its snapshotted display text.""" + c = connect(tmp_memory_db) + apply_migrations(c) + c.execute( + """INSERT INTO session_memory_exposure + (session_id, memory_kind, memory_id, exposed_at, source, display) + VALUES ('S1', 'reflection', 'mem-abc-123', + '2026-08-06T10:00:00+00:00', 'retrieve', + 'Snapshotted AWS Title')""" + ) + c.commit() + c.close() + env = { + "BETTER_MEMORY_HOME": str(tmp_memory_db.parent), + "CLAUDE_SESSION_ID": "S1", + } + result = _run_hook(env) + assert result.returncode == 0 + payload = json.loads(result.stdout) + directive = payload["hookSpecificOutput"]["additionalContext"] + assert "Snapshotted AWS Title" in directive + + def test_empty_semantic_bucket_omitted(self, tmp_path, tmp_memory_db): + _seed_unrated_exposure(tmp_memory_db, "S1") # reflection only + env = { + "BETTER_MEMORY_HOME": str(tmp_memory_db.parent), + "CLAUDE_SESSION_ID": "S1", + } + result = _run_hook(env) + payload = json.loads(result.stdout) + directive = payload["hookSpecificOutput"]["additionalContext"] + assert "Semantic" not in directive + assert "(none)" not in directive +``` + +5. KEEP `test_non_empty_unrated_emits_decision_block`, `test_multi_row_exposure_dedupes_in_directive` (its `directive.count("- r-dup [") == 1` assertion is format-compatible), `test_empty_unrated_writes_marker_no_directive`, `test_stop_hook_active_reentry_writes_marker_and_no_directive`, `test_db_error_falls_back_to_marker` — unchanged. + +- [ ] **Step 2: Run tests to verify new/changed ones fail** + +Run: `uv run pytest tests/hooks/test_session_close_rating_directive.py -v` +Expected: the four new/replaced tests FAIL against the old directive; kept tests PASS. + +- [ ] **Step 3: Implement in `session_close.py`** + +In `_emit_rating_directive_if_unrated`: + +1. Inline query: change the display expression to `COALESCE(e.display, r.title, s.content) AS display` (keep everything else, including the standalone-copy comment — update that comment to mention the display column). +2. Replace the line/directive construction (drop `source_counts` entirely): + +```python + TRUNC = 80 + CAP_BYTES = 8 * 1024 + refl_lines = [] + sem_lines = [] + for r in rows: + display = (r["display"] or "")[:TRUNC] + source = r["source"] or "bootstrap" + line = f"- {r['memory_id']} [{source}] {display}".rstrip() + if r["memory_kind"] == "reflection": + refl_lines.append(line) + else: + sem_lines.append(line) + + sections = [ + f"RATE_MEMORIES: {len(rows)} unrated. " + "Invoke skill `rate-session-memories`.", + "Evidence line first; none possible = `ignored`.", + ] + if refl_lines: + sections.append(f"Reflections ({len(refl_lines)}):") + sections.extend(refl_lines) + if sem_lines: + sections.append(f"Semantic ({len(sem_lines)}):") + sections.extend(sem_lines) + directive = "\n".join(sections) +``` + +3. Keep the `CAP_BYTES` truncation block and the `payload` dict (reason string included) byte-identical to today. + +- [ ] **Step 4: Run the hook suite** + +Run: `uv run pytest tests/hooks/test_session_close_rating_directive.py -v` +Expected: all PASS + +- [ ] **Step 5: Run the e2e journey (pinned substrings: `RATE_MEMORIES` in reason, sem_id + `rate-session-memories` in directive — all survive)** + +Run: `uv run pytest tests/e2e/test_sqlite_journey.py -v` +Expected: PASS. If any assertion trips on directive text, update it to the new format — but the substrings verified at plan time all survive. + +- [ ] **Step 6: Commit** + +```bash +git add better_memory/hooks/session_close.py tests/hooks/test_session_close_rating_directive.py +git commit -m "feat: terse RATE_MEMORIES directive with display snapshots" +``` + +**Confidence: 93%** — directive is a pure string format change behind tests updated in the same task; inline SQL mirrors Task 2's verified expression. + +--- + +### Task 5: Full suite + lint gate + +**Files:** none new. + +- [ ] **Step 1: Run everything** + +Run: `uv run pytest` +Expected: all PASS. Fix any straggler (most likely: a test asserting old directive text or 2-tuple exposure items missed by Task 3's grep). + +- [ ] **Step 2: Lint** + +Run: `uv run ruff check .` +Expected: clean + +- [ ] **Step 3: Commit only if fixes were needed** + +```bash +git add -u +git commit -m "test: sweep stragglers for display-snapshot exposure items" +``` + +**Confidence: 95%** + +--- + +### Task 6: Docs sweep + +**Files:** +- Modify (check each; edit only where stale): `website/mcp-tools.md`, `README.md`, `website/architecture.md`, `website/observation-lifecycle.md` + +- [ ] **Step 1: Sweep for stale claims** + +Run: `grep -rn "list_session_exposures\|RATE_MEMORIES\|session_memory_exposure" website/ README.md docs/decisions/` +For each hit, verify the prose against the new behaviour: +- `website/mcp-tools.md`: if it documents `list_session_exposures`'s `title`/`content` fields as join-derived or nullable-only-for-agentcore, update to "display snapshot captured at exposure time; falls back to live title/content for local rows". +- `docs/decisions/stop-hook-must-be-sync.md` quotes only the `reason` string — unchanged, leave it. +- Any doc quoting the old directive body verbatim: update to the new format. + +- [ ] **Step 2: Commit (or record "docs unaffected")** + +If edits were made: + +```bash +git add website/ README.md +git commit -m "docs: display-snapshot exposure ledger + terse RATE_MEMORIES directive" +``` + +If no edits were needed, note "docs unaffected" in the eventual PR description — do not skip the grep. + +**Confidence: 95%** diff --git a/docs/superpowers/specs/2026-08-06-rate-directive-display-design.md b/docs/superpowers/specs/2026-08-06-rate-directive-display-design.md index b003ef3..f33dad7 100644 --- a/docs/superpowers/specs/2026-08-06-rate-directive-display-design.md +++ b/docs/superpowers/specs/2026-08-06-rate-directive-display-design.md @@ -56,8 +56,11 @@ the Stop hook — it must stay synchronous and local - **Write sites** (all already hold the record content): - `storage/agentcore.py` — `_record_retrieve_exposures` (buckets contain parsed records with titles) and the bootstrap/`record_exposures` path. - - sqlite path — `services/reflection.py` retrieve + - `services/session_bootstrap.py`. + - sqlite path — `services/session_bootstrap.py` (bootstrap exposures). + - NOT changed: the two inline sqlite exposure INSERTs in + `services/reflection.py` and `services/semantic.py` retrieve paths. + They only ever write local-table ids, which the join fallback resolves + live; snapshotting there would freeze titles for no benefit. - `storage/protocol.py` `record_exposures` takes the same `(kind, id, display)` triples as `exposure_log.record` (one shape everywhere, no separate display param); `hooks/contextual_inject.py` From 0924f8bde5bdf4c5317b5e1745412abe8eed9c9d Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 6 Aug 2026 10:09:25 +0100 Subject: [PATCH 03/13] feat: add display snapshot column to session_memory_exposure Co-Authored-By: Claude Fable 5 --- .../db/migrations/0017_exposure_display.sql | 10 ++++++++++ tests/db/test_migration_0009.py | 2 +- tests/db/test_migration_0010.py | 2 +- .../db/test_migration_0012_contextual_exposure.py | 2 +- tests/services/test_exposure_log.py | 15 +++++++++++++++ 5 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 better_memory/db/migrations/0017_exposure_display.sql diff --git a/better_memory/db/migrations/0017_exposure_display.sql b/better_memory/db/migrations/0017_exposure_display.sql new file mode 100644 index 0000000..04ca6a5 --- /dev/null +++ b/better_memory/db/migrations/0017_exposure_display.sql @@ -0,0 +1,10 @@ +-- Migration 0017: display-text snapshot on rated exposures. +-- +-- Display-text snapshot captured at exposure-record time. Nullable: rows +-- written before this migration (and callers with no display in hand) leave +-- it NULL and the read path falls back to joining reflections.title / +-- semantic_memories.content — which only resolves local-table ids, not +-- agentcore's AWS-minted mem- ids. See +-- docs/superpowers/specs/2026-08-06-rate-directive-display-design.md. + +ALTER TABLE session_memory_exposure ADD COLUMN display TEXT; diff --git a/tests/db/test_migration_0009.py b/tests/db/test_migration_0009.py index 078933a..327cabb 100644 --- a/tests/db/test_migration_0009.py +++ b/tests/db/test_migration_0009.py @@ -29,7 +29,7 @@ def test_table_created_with_expected_columns(self, conn): assert cols == { "session_id", "memory_kind", "memory_id", "exposed_at", "source", "rated_at", "classification", - "via_exploration", "evidence", + "via_exploration", "evidence", "display", } def test_primary_key_includes_exposed_at(self, conn): diff --git a/tests/db/test_migration_0010.py b/tests/db/test_migration_0010.py index 070f953..3564352 100644 --- a/tests/db/test_migration_0010.py +++ b/tests/db/test_migration_0010.py @@ -59,7 +59,7 @@ def test_exposure_table_columns_preserved(self, conn): assert cols == { "session_id", "memory_kind", "memory_id", "exposed_at", "source", "rated_at", "classification", - "via_exploration", "evidence", + "via_exploration", "evidence", "display", } def test_primary_key_preserved(self, conn): diff --git a/tests/db/test_migration_0012_contextual_exposure.py b/tests/db/test_migration_0012_contextual_exposure.py index 4214967..fde8759 100644 --- a/tests/db/test_migration_0012_contextual_exposure.py +++ b/tests/db/test_migration_0012_contextual_exposure.py @@ -63,7 +63,7 @@ def test_exposure_table_columns_preserved(self, conn): assert cols == { "session_id", "memory_kind", "memory_id", "exposed_at", "source", "rated_at", "classification", - "via_exploration", "evidence", + "via_exploration", "evidence", "display", } def test_primary_key_preserved(self, conn): diff --git a/tests/services/test_exposure_log.py b/tests/services/test_exposure_log.py index 4995459..3601579 100644 --- a/tests/services/test_exposure_log.py +++ b/tests/services/test_exposure_log.py @@ -361,3 +361,18 @@ def test_no_commit_inside_stamp(self, conn): ) assert conn.in_transaction is True conn.commit() + + +# --------------------------------------------------------------------------- +# display column +# --------------------------------------------------------------------------- + + +class TestDisplayColumn: + def test_session_memory_exposure_has_display_column(self, conn): + cols = { + row[1] for row in conn.execute( + "PRAGMA table_info(session_memory_exposure)" + ) + } + assert "display" in cols From 40d7f519bdf0f0a707a690c528f10579fcd6dfda Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 6 Aug 2026 10:14:24 +0100 Subject: [PATCH 04/13] feat: exposure_log takes (kind, id, display) triples; COALESCE snapshot on read Co-Authored-By: Claude Fable 5 --- better_memory/services/exposure_log.py | 38 +++-- tests/services/test_exposure_log.py | 205 ++++++++++++++++++++----- 2 files changed, 197 insertions(+), 46 deletions(-) diff --git a/better_memory/services/exposure_log.py b/better_memory/services/exposure_log.py index 0fcaf3b..4c0beec 100644 --- a/better_memory/services/exposure_log.py +++ b/better_memory/services/exposure_log.py @@ -11,9 +11,13 @@ mirrors the ``via_exploration`` write in ``ReflectionSynthesisService.retrieve_reflections`` (``reflection.py``). Passing no ``exploration_ids`` (the default, an empty frozenset) reproduces - the original ``record_exposures`` behaviour exactly. + the original ``record_exposures`` behaviour exactly. Extended with ``display`` + column to snapshot title/content at exposure time, truncated to 120 chars — + needed because agentcore memory ids do not exist in local content tables. - ``list_unrated``: the grouped/deduped/display-joined query originally - inline in ``SessionBootstrapService.list_session_exposures``. + inline in ``SessionBootstrapService.list_session_exposures``, extended to + COALESCE the snapshot ``display`` column over the joined title/content + for display values. - ``stamp``: the exposure-row ``UPDATE`` originally inline in ``MemoryRatingService._apply_one``. Copied, not rewired — the sqlite rating service keeps its own inline copy; this one is for the agentcore @@ -23,21 +27,24 @@ existing call site already commits after invoking the original inline SQL, so callers must continue to do so here. """ + from __future__ import annotations import sqlite3 +_DISPLAY_TRUNC = 120 + def record( conn: sqlite3.Connection, *, session_id: str, - items: list[tuple[str, str]], + items: list[tuple[str, str, str | None]], source: str, now: str, exploration_ids: frozenset[str] = frozenset(), ) -> None: - """Write one ``session_memory_exposure`` row per (kind, id) item. + """Write one ``session_memory_exposure`` row per (kind, id, display) item. At most one row per (session, kind, id), regardless of how many times the memory is re-served within the session — first source (and first @@ -45,6 +52,10 @@ def record( (session, kind, id) is a no-op, even if it would have tagged the row as an exploration serve. + Display is a snapshot of the memory's title/content captured at exposure + time (None when the caller has none), truncated to 120 chars — needed + because agentcore memory ids do not exist in local content tables. + Best-effort: no-op when ``session_id`` or ``items`` is empty. Does not commit — the caller owns the transaction. """ @@ -53,18 +64,25 @@ def record( conn.executemany( "INSERT INTO session_memory_exposure " "(session_id, memory_kind, memory_id, exposed_at, source, " - " via_exploration) " - "SELECT ?, ?, ?, ?, ?, ? " + " via_exploration, display) " + "SELECT ?, ?, ?, ?, ?, ?, ? " "WHERE NOT EXISTS (" " SELECT 1 FROM session_memory_exposure " " WHERE session_id = ? AND memory_kind = ? AND memory_id = ?)", [ ( - session_id, kind, mid, now, source, + session_id, + kind, + mid, + now, + source, 1 if mid in exploration_ids else 0, - session_id, kind, mid, + (display[:_DISPLAY_TRUNC] if display else None), + session_id, + kind, + mid, ) - for kind, mid in items + for kind, mid, display in items ], ) @@ -89,7 +107,7 @@ def list_unrated(conn: sqlite3.Connection, *, session_id: str) -> list[sqlite3.R SELECT e.memory_kind, e.memory_id, MIN(e.exposed_at) AS exposed_at, MIN(e.source) AS source, - COALESCE(r.title, s.content) AS display + COALESCE(e.display, r.title, s.content) AS display FROM session_memory_exposure e LEFT JOIN reflections r ON e.memory_kind='reflection' AND e.memory_id = r.id diff --git a/tests/services/test_exposure_log.py b/tests/services/test_exposure_log.py index 3601579..59faa33 100644 --- a/tests/services/test_exposure_log.py +++ b/tests/services/test_exposure_log.py @@ -15,6 +15,7 @@ No commits inside the module — callers own the transaction. """ + from __future__ import annotations from pathlib import Path @@ -77,7 +78,7 @@ def test_inserts_one_row_per_item(self, conn): exposure_log.record( conn, session_id="s1", - items=[("reflection", "r-a"), ("semantic", "s-a")], + items=[("reflection", "r-a", None), ("semantic", "s-a", None)], source="bootstrap", now="2026-01-01T00:00:00Z", ) @@ -88,15 +89,21 @@ def test_inserts_one_row_per_item(self, conn): def test_no_op_when_session_id_empty(self, conn): exposure_log.record( - conn, session_id="", items=[("reflection", "r-a")], - source="bootstrap", now="2026-01-01T00:00:00Z", + conn, + session_id="", + items=[("reflection", "r-a", None)], + source="bootstrap", + now="2026-01-01T00:00:00Z", ) conn.commit() assert _rows(conn, session_id="") == [] def test_no_op_when_items_empty(self, conn): exposure_log.record( - conn, session_id="s1", items=[], source="bootstrap", + conn, + session_id="s1", + items=[], + source="bootstrap", now="2026-01-01T00:00:00Z", ) conn.commit() @@ -104,12 +111,18 @@ def test_no_op_when_items_empty(self, conn): def test_first_source_wins_dedup(self, conn): exposure_log.record( - conn, session_id="s1", items=[("reflection", "r-a")], - source="bootstrap", now="2026-01-01T00:00:00Z", + conn, + session_id="s1", + items=[("reflection", "r-a", None)], + source="bootstrap", + now="2026-01-01T00:00:00Z", ) exposure_log.record( - conn, session_id="s1", items=[("reflection", "r-a")], - source="retrieve", now="2026-01-01T00:01:00Z", + conn, + session_id="s1", + items=[("reflection", "r-a", None)], + source="retrieve", + now="2026-01-01T00:01:00Z", ) conn.commit() rows = _rows(conn) @@ -118,12 +131,18 @@ def test_first_source_wins_dedup(self, conn): def test_other_sessions_unaffected(self, conn): exposure_log.record( - conn, session_id="s1", items=[("reflection", "r-a")], - source="bootstrap", now="2026-01-01T00:00:00Z", + conn, + session_id="s1", + items=[("reflection", "r-a", None)], + source="bootstrap", + now="2026-01-01T00:00:00Z", ) exposure_log.record( - conn, session_id="s2", items=[("reflection", "r-a")], - source="bootstrap", now="2026-01-01T00:00:00Z", + conn, + session_id="s2", + items=[("reflection", "r-a", None)], + source="bootstrap", + now="2026-01-01T00:00:00Z", ) conn.commit() assert len(_rows(conn, "s1")) == 1 @@ -131,9 +150,11 @@ def test_other_sessions_unaffected(self, conn): def test_exploration_ids_tag_via_exploration(self, conn): exposure_log.record( - conn, session_id="s1", - items=[("reflection", "r-untested"), ("reflection", "r-proven")], - source="retrieve", now="2026-01-01T00:00:00Z", + conn, + session_id="s1", + items=[("reflection", "r-untested", None), ("reflection", "r-proven", None)], + source="retrieve", + now="2026-01-01T00:00:00Z", exploration_ids=frozenset({"r-untested"}), ) conn.commit() @@ -143,8 +164,11 @@ def test_exploration_ids_tag_via_exploration(self, conn): def test_default_exploration_ids_is_empty_frozenset(self, conn): exposure_log.record( - conn, session_id="s1", items=[("reflection", "r-a")], - source="bootstrap", now="2026-01-01T00:00:00Z", + conn, + session_id="s1", + items=[("reflection", "r-a", None)], + source="bootstrap", + now="2026-01-01T00:00:00Z", ) conn.commit() assert _rows(conn)[0]["via_exploration"] == 0 @@ -154,12 +178,18 @@ def test_dedup_wins_over_exploration_tag(self, conn): # would have tagged it as an exploration serve must not retag it; # first-source-wins applies to the whole row, tag included. exposure_log.record( - conn, session_id="s1", items=[("reflection", "r-x")], - source="bootstrap", now="2026-01-01T00:00:00Z", + conn, + session_id="s1", + items=[("reflection", "r-x", None)], + source="bootstrap", + now="2026-01-01T00:00:00Z", ) exposure_log.record( - conn, session_id="s1", items=[("reflection", "r-x")], - source="retrieve", now="2026-01-01T00:01:00Z", + conn, + session_id="s1", + items=[("reflection", "r-x", None)], + source="retrieve", + now="2026-01-01T00:01:00Z", exploration_ids=frozenset({"r-x"}), ) conn.commit() @@ -169,8 +199,11 @@ def test_dedup_wins_over_exploration_tag(self, conn): def test_no_commit_inside_record(self, conn): exposure_log.record( - conn, session_id="s1", items=[("reflection", "r-a")], - source="bootstrap", now="2026-01-01T00:00:00Z", + conn, + session_id="s1", + items=[("reflection", "r-a", None)], + source="bootstrap", + now="2026-01-01T00:00:00Z", ) # The module never commits — the write started an implicit # transaction that only the caller's own commit() can close. @@ -280,8 +313,12 @@ def test_stamps_evidence_classification_and_rated_at(self, conn): conn.commit() rowcount = exposure_log.stamp( - conn, session_id="s1", kind="reflection", memory_id=rid, - classification="cited", evidence="fixed the bug", + conn, + session_id="s1", + kind="reflection", + memory_id=rid, + classification="cited", + evidence="fixed the bug", now="2026-01-01T13:00:00Z", ) conn.commit() @@ -313,8 +350,12 @@ def test_only_updates_unrated_rows(self, conn): conn.commit() rowcount = exposure_log.stamp( - conn, session_id="s1", kind="reflection", memory_id=rid, - classification="shaped", evidence="used it", + conn, + session_id="s1", + kind="reflection", + memory_id=rid, + classification="shaped", + evidence="used it", now="2026-01-01T14:00:00Z", ) conn.commit() @@ -335,8 +376,12 @@ def test_returns_zero_when_already_rated(self, conn): conn.commit() rowcount = exposure_log.stamp( - conn, session_id="s1", kind="reflection", memory_id=rid, - classification="shaped", evidence="second pass", + conn, + session_id="s1", + kind="reflection", + memory_id=rid, + classification="shaped", + evidence="second pass", now="2026-01-01T15:00:00Z", ) conn.commit() @@ -355,8 +400,12 @@ def test_no_commit_inside_stamp(self, conn): conn.commit() exposure_log.stamp( - conn, session_id="s1", kind="reflection", memory_id=rid, - classification="cited", evidence="fixed the bug", + conn, + session_id="s1", + kind="reflection", + memory_id=rid, + classification="cited", + evidence="fixed the bug", now="2026-01-01T13:00:00Z", ) assert conn.in_transaction is True @@ -370,9 +419,93 @@ def test_no_commit_inside_stamp(self, conn): class TestDisplayColumn: def test_session_memory_exposure_has_display_column(self, conn): - cols = { - row[1] for row in conn.execute( - "PRAGMA table_info(session_memory_exposure)" - ) - } + cols = {row[1] for row in conn.execute("PRAGMA table_info(session_memory_exposure)")} assert "display" in cols + + +class TestRecordDisplay: + def test_display_persisted(self, conn): + exposure_log.record( + conn, + session_id="s1", + items=[("reflection", "mem-aws-1", "AWS Title")], + source="retrieve", + now="2026-08-06T10:00:00+00:00", + ) + row = conn.execute( + "SELECT display FROM session_memory_exposure WHERE memory_id = 'mem-aws-1'" + ).fetchone() + assert row["display"] == "AWS Title" + + def test_display_truncated_to_120(self, conn): + exposure_log.record( + conn, + session_id="s1", + items=[("reflection", "mem-aws-2", "x" * 300)], + source="retrieve", + now="2026-08-06T10:00:00+00:00", + ) + row = conn.execute( + "SELECT display FROM session_memory_exposure WHERE memory_id = 'mem-aws-2'" + ).fetchone() + assert row["display"] == "x" * 120 + + def test_display_none_allowed(self, conn): + exposure_log.record( + conn, + session_id="s1", + items=[("reflection", "mem-aws-3", None)], + source="retrieve", + now="2026-08-06T10:00:00+00:00", + ) + row = conn.execute( + "SELECT display FROM session_memory_exposure WHERE memory_id = 'mem-aws-3'" + ).fetchone() + assert row["display"] is None + + +class TestListUnratedDisplayCoalesce: + def test_snapshot_beats_join(self, conn): + conn.execute( + "INSERT INTO reflections (id, title, project, phase, polarity," + " use_cases, hints, confidence, created_at, updated_at)" + " VALUES ('r1', 'Live Title', 'p', 'general', 'do', 'uc', '[]'," + " 0.5, '2026-01-01', '2026-01-01')" + ) + exposure_log.record( + conn, + session_id="s1", + items=[("reflection", "r1", "Snapshot Title")], + source="retrieve", + now="2026-08-06T10:00:00+00:00", + ) + rows = exposure_log.list_unrated(conn, session_id="s1") + assert rows[0]["display"] == "Snapshot Title" + + def test_null_snapshot_falls_back_to_join(self, conn): + conn.execute( + "INSERT INTO reflections (id, title, project, phase, polarity," + " use_cases, hints, confidence, created_at, updated_at)" + " VALUES ('r2', 'Live Title', 'p', 'general', 'do', 'uc', '[]'," + " 0.5, '2026-01-01', '2026-01-01')" + ) + exposure_log.record( + conn, + session_id="s1", + items=[("reflection", "r2", None)], + source="retrieve", + now="2026-08-06T10:00:00+00:00", + ) + rows = exposure_log.list_unrated(conn, session_id="s1") + assert rows[0]["display"] == "Live Title" + + def test_foreign_id_null_snapshot_yields_none(self, conn): + exposure_log.record( + conn, + session_id="s1", + items=[("reflection", "mem-aws-9", None)], + source="retrieve", + now="2026-08-06T10:00:00+00:00", + ) + rows = exposure_log.list_unrated(conn, session_id="s1") + assert rows[0]["display"] is None From 08db0ea2777eab39029ae9e29a8e604a97da1283 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 6 Aug 2026 10:17:38 +0100 Subject: [PATCH 05/13] =?UTF-8?q?docs:=20correct=20exposure=5Flog=20module?= =?UTF-8?q?=20docstring=20=E2=80=94=20no=20longer=20byte-identical?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- better_memory/services/exposure_log.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/better_memory/services/exposure_log.py b/better_memory/services/exposure_log.py index 4c0beec..42a00cc 100644 --- a/better_memory/services/exposure_log.py +++ b/better_memory/services/exposure_log.py @@ -1,9 +1,12 @@ """Shared exposure-ledger SQL: one implementation for every storage backend. -Three primitives, lifted verbatim from their original call sites so sqlite -behaviour stays byte-identical (this module is a pure extraction, pinned by -``tests/services/test_exposure_log.py`` + the four other suites listed in its -docstring): +Three primitives, largely lifted from their original call sites (this module is +pinned by ``tests/services/test_exposure_log.py`` + the four other suites +listed in its docstring). Extended with display-column snapshot support: +``record`` writes a ``display`` column (title/content captured at exposure time, +truncated to 120 chars); ``list_unrated`` COALESCEs the snapshot over joined +title/content. These extensions mean the module is no longer byte-identical to +the original inline SQL: - ``record``: the first-source-wins ``INSERT..WHERE NOT EXISTS`` dedup guard originally inline in ``SessionBootstrapService.record_exposures`` From d0349cd99e2322c8f0ee767f533e127714ba8a36 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 6 Aug 2026 10:26:15 +0100 Subject: [PATCH 06/13] feat: thread display snapshots through record_exposures to all write sites Task 3 of rate-directive-display: every StorageBackend.record_exposures implementer and caller now passes (kind, id, display) triples end to end, so agentcore-id exposures carry a title/content snapshot without needing a local content row. Co-Authored-By: Claude Fable 5 --- better_memory/hooks/contextual_inject.py | 3 ++- better_memory/services/session_bootstrap.py | 16 +++++++++--- better_memory/storage/agentcore.py | 11 +++++--- better_memory/storage/protocol.py | 8 ++++-- better_memory/storage/sqlite.py | 2 +- tests/hooks/test_session_bootstrap.py | 2 +- .../integration/test_contextual_rating_e2e.py | 2 +- tests/services/test_exposure_dedup.py | 10 +++---- tests/services/test_exposure_tracking.py | 4 +-- tests/storage/test_agentcore_rating_loop.py | 26 +++++++++++-------- tests/storage/test_agentcore_unit.py | 21 ++++++++++++--- 11 files changed, 69 insertions(+), 36 deletions(-) diff --git a/better_memory/hooks/contextual_inject.py b/better_memory/hooks/contextual_inject.py index 31355ab..f0bb729 100644 --- a/better_memory/hooks/contextual_inject.py +++ b/better_memory/hooks/contextual_inject.py @@ -165,10 +165,11 @@ def main() -> None: if items: rendered = format_relevant(items) survivors = [(m.kind, m.id) for m in items] + exposure_items = [(m.kind, m.id, m.text) for m in items] try: backend.record_exposures( session_id=session_id, - items=survivors, + items=exposure_items, source="contextual", ) except BaseException as exc: # noqa: BLE001 - never block injection diff --git a/better_memory/services/session_bootstrap.py b/better_memory/services/session_bootstrap.py index 5cbf519..0a6775f 100644 --- a/better_memory/services/session_bootstrap.py +++ b/better_memory/services/session_bootstrap.py @@ -147,10 +147,10 @@ def record_exposures( self, *, session_id: str, - items: list[tuple[str, str]], + items: list[tuple[str, str, str | None]], source: str, ) -> None: - """Write one session_memory_exposure row per (kind, id) item. + """Write one session_memory_exposure row per (kind, id, display) triple. At most one row per (session, kind, id), regardless of how many times the memory is re-served within the session. The rating vocabulary @@ -182,6 +182,8 @@ def _record_exposure( session_id: str, reflection_ids: list[str], semantic_ids: list[str], + reflection_display: dict[str, str | None], + semantic_display: dict[str, str | None], ) -> None: """Write one row per injected memory into session_memory_exposure. @@ -191,8 +193,10 @@ def _record_exposure( """ self.record_exposures( session_id=session_id, - items=[("reflection", rid) for rid in reflection_ids] - + [("semantic", sid) for sid in semantic_ids], + items=[("reflection", rid, reflection_display.get(rid)) + for rid in reflection_ids] + + [("semantic", sid, semantic_display.get(sid)) + for sid in semantic_ids], source="bootstrap", ) @@ -286,6 +290,8 @@ def bootstrap( session_id=session_id, reflection_ids=[], semantic_ids=deferred_semantic_ids, + reflection_display={}, + semantic_display={m.id: m.content for m in general_only}, ) return BootstrapResult( @@ -358,6 +364,8 @@ def bootstrap( session_id=session_id, reflection_ids=reflection_ids, semantic_ids=semantic_ids, + reflection_display={r["id"]: r.get("title") for _, r in flat_reflections}, + semantic_display={m.id: m.content for m in semantic}, ) return BootstrapResult( diff --git a/better_memory/storage/agentcore.py b/better_memory/storage/agentcore.py index e3dbfee..e556592 100644 --- a/better_memory/storage/agentcore.py +++ b/better_memory/storage/agentcore.py @@ -366,8 +366,11 @@ def _record_retrieve_exposures( return if not sid: return - all_ids = [r["id"] for bucket in buckets.values() for r in bucket] - if not all_ids: + all_items = [ + ("reflection", r["id"], r.get("title")) + for bucket in buckets.values() for r in bucket + ] + if not all_items: return try: from better_memory.services import exposure_log @@ -375,7 +378,7 @@ def _record_retrieve_exposures( exposure_log.record( self._local_conn, session_id=sid, - items=[("reflection", rid) for rid in all_ids], + items=all_items, source="retrieve", now=datetime.now(UTC).isoformat(), exploration_ids=frozenset(exploration_ids), @@ -2077,7 +2080,7 @@ def record_exposures( self, *, session_id: str, - items: list[tuple[str, str]], + items: list[tuple[str, str, str | None]], source: str, ) -> None: """Write exposures to the local ledger when one is available. diff --git a/better_memory/storage/protocol.py b/better_memory/storage/protocol.py index fa90db2..915cf7f 100644 --- a/better_memory/storage/protocol.py +++ b/better_memory/storage/protocol.py @@ -378,10 +378,14 @@ def record_exposures( self, *, session_id: str, - items: list[tuple[str, str]], + items: list[tuple[str, str, str | None]], source: str, ) -> None: - """Record (kind, id) memory exposures for later rating. + """Record (kind, id, display) memory exposures for later rating. + + display is a snapshot of the memory's title/content at exposure + time (None when unavailable); it makes agentcore-id exposures + renderable without any local content row. Backend-agnostic: both backends write the SAME local ``session_memory_exposure`` table via ``services.exposure_log``. diff --git a/better_memory/storage/sqlite.py b/better_memory/storage/sqlite.py index ae0dbeb..a97c443 100644 --- a/better_memory/storage/sqlite.py +++ b/better_memory/storage/sqlite.py @@ -440,7 +440,7 @@ def record_exposures( self, *, session_id: str, - items: list[tuple[str, str]], + items: list[tuple[str, str, str | None]], source: str, ) -> None: self._session_bootstrap.record_exposures( diff --git a/tests/hooks/test_session_bootstrap.py b/tests/hooks/test_session_bootstrap.py index 9b18ce9..0fec82a 100644 --- a/tests/hooks/test_session_bootstrap.py +++ b/tests/hooks/test_session_bootstrap.py @@ -428,7 +428,7 @@ def session_bootstrap(self, **kwargs): exposure_log.record( self._conn, session_id=kwargs["session_id"], - items=[("semantic", "sem-general-1")], + items=[("semantic", "sem-general-1", None)], source="bootstrap", now=datetime.now(UTC).isoformat(), ) diff --git a/tests/integration/test_contextual_rating_e2e.py b/tests/integration/test_contextual_rating_e2e.py index 852d16d..246d5ef 100644 --- a/tests/integration/test_contextual_rating_e2e.py +++ b/tests/integration/test_contextual_rating_e2e.py @@ -79,7 +79,7 @@ def test_contextual_injection_full_rating_loop(conn, monkeypatch, tmp_path): # 4. Record the contextual exposure for everything just injected. backend.record_exposures( session_id="e2e-sess", - items=[(m.kind, m.id) for m in items], + items=[(m.kind, m.id, None) for m in items], source="contextual", ) diff --git a/tests/services/test_exposure_dedup.py b/tests/services/test_exposure_dedup.py index d64c538..9bc57de 100644 --- a/tests/services/test_exposure_dedup.py +++ b/tests/services/test_exposure_dedup.py @@ -52,7 +52,7 @@ def _seed_reflection(conn, rid): class TestExposureDedup: def test_record_exposures_is_idempotent_per_session(self, conn): svc = SessionBootstrapService(conn) - items = [("reflection", "r-a"), ("semantic", "s-a")] + items = [("reflection", "r-a", None), ("semantic", "s-a", None)] svc.record_exposures(session_id="s1", items=items, source="bootstrap") svc.record_exposures(session_id="s1", items=items, source="bootstrap") assert len(_rows(conn)) == 2 @@ -62,10 +62,10 @@ def test_reserve_keeps_first_source(self, conn): # not relabel or duplicate it. svc = SessionBootstrapService(conn) svc.record_exposures( - session_id="s1", items=[("reflection", "r-a")], source="bootstrap" + session_id="s1", items=[("reflection", "r-a", None)], source="bootstrap" ) svc.record_exposures( - session_id="s1", items=[("reflection", "r-a")], source="retrieve" + session_id="s1", items=[("reflection", "r-a", None)], source="retrieve" ) rows = _rows(conn) assert len(rows) == 1 @@ -74,10 +74,10 @@ def test_reserve_keeps_first_source(self, conn): def test_other_sessions_unaffected(self, conn): svc = SessionBootstrapService(conn) svc.record_exposures( - session_id="s1", items=[("reflection", "r-a")], source="bootstrap" + session_id="s1", items=[("reflection", "r-a", None)], source="bootstrap" ) svc.record_exposures( - session_id="s2", items=[("reflection", "r-a")], source="bootstrap" + session_id="s2", items=[("reflection", "r-a", None)], source="bootstrap" ) assert len(_rows(conn, "s1")) == 1 assert len(_rows(conn, "s2")) == 1 diff --git a/tests/services/test_exposure_tracking.py b/tests/services/test_exposure_tracking.py index c2531ea..8eda092 100644 --- a/tests/services/test_exposure_tracking.py +++ b/tests/services/test_exposure_tracking.py @@ -223,7 +223,7 @@ def test_record_exposures_contextual_source(self, conn, fixed_clock): svc = SessionBootstrapService(conn, clock=fixed_clock) svc.record_exposures( session_id="s-ctx", - items=[("reflection", "r1"), ("semantic", "m1")], + items=[("reflection", "r1", None), ("semantic", "m1", None)], source="contextual", ) rows = conn.execute( @@ -240,7 +240,7 @@ def test_record_exposures_empty_session_id_is_noop(self, conn, fixed_clock): svc = SessionBootstrapService(conn, clock=fixed_clock) svc.record_exposures( - session_id="", items=[("reflection", "r1")], source="contextual", + session_id="", items=[("reflection", "r1", None)], source="contextual", ) n = conn.execute( "SELECT COUNT(*) AS n FROM session_memory_exposure" diff --git a/tests/storage/test_agentcore_rating_loop.py b/tests/storage/test_agentcore_rating_loop.py index 3ba58a6..f4a5a02 100644 --- a/tests/storage/test_agentcore_rating_loop.py +++ b/tests/storage/test_agentcore_rating_loop.py @@ -158,9 +158,9 @@ def test_sweep_stamps_local_rows_bumps_counters_and_emits_one_event( backend_with_ledger.record_exposures( session_id="test-session-xyz", items=[ - ("reflection", "r-cited"), - ("reflection", "r-ignored"), - ("reflection", "r-misled"), + ("reflection", "r-cited", None), + ("reflection", "r-ignored", None), + ("reflection", "r-misled", None), ], source="contextual", ) @@ -252,7 +252,7 @@ def test_non_ignored_without_evidence_rejects_whole_batch( _seed_local_reflection(local_conn, "r1", title="t1") backend_with_ledger.record_exposures( session_id="test-session-xyz", - items=[("reflection", "r1")], + items=[("reflection", "r1", None)], source="contextual", ) with pytest.raises(ValueError, match="requires a non-empty evidence line"): @@ -276,7 +276,7 @@ def test_ignored_accepted_in_sweep( _seed_local_reflection(local_conn, "r1", title="t1") backend_with_ledger.record_exposures( session_id="test-session-xyz", - items=[("reflection", "r1")], + items=[("reflection", "r1", None)], source="contextual", ) mock_data_client.get_memory_record.return_value = _make_record_response("r1") @@ -308,7 +308,7 @@ def test_create_event_raising_does_not_fail_the_sweep( _seed_local_reflection(local_conn, "r1", title="t1") backend_with_ledger.record_exposures( session_id="test-session-xyz", - items=[("reflection", "r1")], + items=[("reflection", "r1", None)], source="contextual", ) mock_data_client.get_memory_record.return_value = _make_record_response("r1") @@ -354,7 +354,11 @@ def test_non_404_client_error_on_one_entry_does_not_abort_the_sweep( _seed_local_reflection(local_conn, rid, title=f"title-{rid}") backend_with_ledger.record_exposures( session_id="test-session-xyz", - items=[("reflection", "r1"), ("reflection", "r2"), ("reflection", "r3")], + items=[ + ("reflection", "r1", None), + ("reflection", "r2", None), + ("reflection", "r3", None), + ], source="contextual", ) @@ -424,7 +428,7 @@ def test_all_counter_pushes_failing_still_returns_normally( _seed_local_reflection(local_conn, rid, title=f"title-{rid}") backend_with_ledger.record_exposures( session_id="test-session-xyz", - items=[("reflection", "r1"), ("reflection", "r2")], + items=[("reflection", "r1", None), ("reflection", "r2", None)], source="contextual", ) @@ -476,7 +480,7 @@ def test_not_exposed_and_already_rated( _seed_local_reflection(local_conn, "r1", title="t1") backend_with_ledger.record_exposures( session_id="test-session-xyz", - items=[("reflection", "r1")], + items=[("reflection", "r1", None)], source="contextual", ) mock_data_client.get_memory_record.return_value = _make_record_response("r1") @@ -548,7 +552,7 @@ def test_mid_batch_ledger_commit_failure_does_not_abort_sweep( _seed_local_reflection(local_conn, rid, title=f"t-{rid}") backend_with_ledger.record_exposures( session_id="test-session-xyz", - items=[("reflection", rid) for rid in ("r1", "r2", "r3")], + items=[("reflection", rid, None) for rid in ("r1", "r2", "r3")], source="contextual", ) mock_data_client.get_memory_record.side_effect = [ @@ -624,7 +628,7 @@ def test_commit_failure_after_stamp_is_rolled_back( _seed_local_reflection(local_conn, rid, title=f"t-{rid}") backend_with_ledger.record_exposures( session_id="test-session-xyz", - items=[("reflection", rid) for rid in ("r1", "r2", "r3")], + items=[("reflection", rid, None) for rid in ("r1", "r2", "r3")], source="contextual", ) mock_data_client.get_memory_record.side_effect = [ diff --git a/tests/storage/test_agentcore_unit.py b/tests/storage/test_agentcore_unit.py index 2f4c913..1eda11b 100644 --- a/tests/storage/test_agentcore_unit.py +++ b/tests/storage/test_agentcore_unit.py @@ -1214,7 +1214,7 @@ def test_record_exposures_is_noop( """No exposure log in agentcore mode (see list_session_exposures). Must not raise and must not call either boto client.""" result = backend.record_exposures( - session_id="s", items=[("reflection", "r1")], source="contextual", + session_id="s", items=[("reflection", "r1", None)], source="contextual", ) assert result is None assert mock_data_client.method_calls == [] @@ -2739,7 +2739,7 @@ def test_record_exposures_writes_to_local_ledger_when_available( backend_with_local_conn.record_exposures( session_id="test-session-xyz", - items=[("reflection", "refl-ledger-1")], + items=[("reflection", "refl-ledger-1", None)], source="contextual", ) @@ -2760,13 +2760,26 @@ def test_record_exposures_without_local_conn_stays_legacy_noop( """No local_conn wired (the `backend` fixture default) — record_exposures must keep the pre-existing no-op contract: no wire calls, no exception.""" result = backend.record_exposures( - session_id="s", items=[("reflection", "r1")], source="contextual", + session_id="s", items=[("reflection", "r1", None)], source="contextual", ) assert result is None assert mock_data_client.method_calls == [] assert mock_control_client.method_calls == [] +def test_record_exposures_persists_display(backend_with_local_conn, local_conn) -> None: + backend_with_local_conn.record_exposures( + session_id="s-disp", + items=[("reflection", "mem-aws-1", "AWS Reflection Title")], + source="contextual", + ) + row = local_conn.execute( + "SELECT display FROM session_memory_exposure " + "WHERE session_id = 's-disp' AND memory_id = 'mem-aws-1'" + ).fetchone() + assert row["display"] == "AWS Reflection Title" + + def test_list_session_exposures_without_local_conn_stays_legacy_empty(backend) -> None: """No local_conn wired — list_session_exposures keeps the pre-existing empty-envelope contract.""" @@ -2780,7 +2793,7 @@ def test_record_exposures_empty_session_id_writes_nothing( """Even with a real local_conn available, an empty session_id must not write any row (mirrors exposure_log.record's own best-effort guard).""" backend_with_local_conn.record_exposures( - session_id="", items=[("reflection", "r1")], source="contextual", + session_id="", items=[("reflection", "r1", None)], source="contextual", ) row = local_conn.execute("SELECT COUNT(*) AS n FROM session_memory_exposure").fetchone() assert row["n"] == 0 From 61112dc334296da134b8945bd44880c222e361b5 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 6 Aug 2026 10:34:37 +0100 Subject: [PATCH 07/13] test: bootstrap exposure rows carry display snapshots Adds test_bootstrap_exposure_rows_carry_display_snapshots, which seeds a reflection with a known title and a semantic memory with known content, runs a real SessionBootstrapService.bootstrap() call, and asserts the resulting session_memory_exposure rows carry those exact display values. Closes a coverage gap flagged in review: the existing display test built a triple by hand and never exercised _record_exposure's reflection_display/semantic_display dict-comprehension wiring (r.get("title") / m.content), so a future edit pointing at the wrong field or the wrong source list would have passed every existing test. Verified by temporarily breaking the wiring locally and confirming this test fails, then reverting. Co-Authored-By: Claude Fable 5 --- tests/services/test_session_bootstrap.py | 26 ++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/services/test_session_bootstrap.py b/tests/services/test_session_bootstrap.py index db97ab1..6bf280d 100644 --- a/tests/services/test_session_bootstrap.py +++ b/tests/services/test_session_bootstrap.py @@ -402,6 +402,32 @@ def test_exposures_only_for_full_renders(conn, git_repo: Path) -> None: assert mid not in exposed_ids +def test_bootstrap_exposure_rows_carry_display_snapshots(conn, git_repo: Path) -> None: + """Task 3 fix: prove the reflection_display/semantic_display wiring in + _record_exposure (r.get("title") / m.content) end to end through a real + bootstrap() call, not just a hand-built triple — a future edit swapping + the wrong field or the wrong source list would fail this.""" + proj = git_repo.name + rid = _seed_reflection( + conn, project=proj, polarity="do", scope="project", + title="known reflection title", + ) + sid = _seed_semantic( + conn, content="known semantic content", project=proj, scope="project", + ) + + svc = SessionBootstrapService(conn, top_n=5) + svc.bootstrap(source="startup", session_id="sess-display", cwd=git_repo) + + rows = conn.execute( + "SELECT memory_id, display FROM session_memory_exposure WHERE session_id = ?", + ("sess-display",), + ).fetchall() + display_by_id = {r["memory_id"]: r["display"] for r in rows} + assert display_by_id[rid] == "known reflection title" + assert display_by_id[sid] == "known semantic content" + + # --------------------------------------------------------------------------- # list_session_exposures (Task 5a) — extracted from inline MCP handler. # --------------------------------------------------------------------------- From 4db08d09419601ae930e4004f9e5ae33cc865021 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 6 Aug 2026 10:39:38 +0100 Subject: [PATCH 08/13] feat: terse RATE_MEMORIES directive with display snapshots Co-Authored-By: Claude Fable 5 --- better_memory/hooks/session_close.py | 40 +++++-------- .../test_session_close_rating_directive.py | 60 +++++++++++++------ 2 files changed, 59 insertions(+), 41 deletions(-) diff --git a/better_memory/hooks/session_close.py b/better_memory/hooks/session_close.py index 657b909..0011969 100644 --- a/better_memory/hooks/session_close.py +++ b/better_memory/hooks/session_close.py @@ -69,13 +69,14 @@ def _emit_rating_directive_if_unrated(session_id: str) -> bool: # Standalone copy, intentionally not delegated to # better_memory.services.exposure_log.list_unrated (see that # module for the shared implementation) — kept inline here so - # this hook has no service-layer import dependency. + # this hook has no service-layer import dependency. Uses the + # display snapshot column (Task 1) to support foreign IDs. rows = conn.execute( """ SELECT e.memory_kind, e.memory_id, MIN(e.exposed_at) AS exposed_at, MIN(e.source) AS source, - COALESCE(r.title, s.content) AS display + COALESCE(e.display, r.title, s.content) AS display FROM session_memory_exposure e LEFT JOIN reflections r ON e.memory_kind='reflection' AND e.memory_id = r.id @@ -96,36 +97,27 @@ def _emit_rating_directive_if_unrated(session_id: str) -> bool: CAP_BYTES = 8 * 1024 refl_lines = [] sem_lines = [] - source_counts: dict[str, int] = {} for r in rows: display = (r["display"] or "")[:TRUNC] source = r["source"] or "bootstrap" - source_counts[source] = source_counts.get(source, 0) + 1 - line = f"- {r['memory_id']} [{source}]: {display}" + line = f"- {r['memory_id']} [{source}] {display}".rstrip() if r["memory_kind"] == "reflection": refl_lines.append(line) else: sem_lines.append(line) - counts_line = "sources: " + ", ".join( - f"{k} {v}" for k, v in sorted(source_counts.items()) - ) - directive = ( - "RATE_MEMORIES — before this session ends, classify the " - "memories that were exposed during this session and that you " - "did NOT already credit via memory.credit.\n" - f"({counts_line})\n\n" - f"Reflections ({len(refl_lines)}):\n" - + ("\n".join(refl_lines) if refl_lines else " (none)") - + f"\n\nSemantic memories ({len(sem_lines)}):\n" - + ("\n".join(sem_lines) if sem_lines else " (none)") - + "\n\n" - "For each id: FIRST write one line of evidence (what the memory " - "changed, or a quote) - if you cannot, the class is `ignored`.\n" - "Classes: cited / shaped / ignored / misled / overlooked.\n" - "Non-ignored ratings without an evidence line are rejected. " - "Invoke the skill `rate-session-memories`." - ) + sections = [ + f"RATE_MEMORIES: {len(rows)} unrated. " + "Invoke skill `rate-session-memories`.", + "Evidence line first; none possible = `ignored`.", + ] + if refl_lines: + sections.append(f"Reflections ({len(refl_lines)}):") + sections.extend(refl_lines) + if sem_lines: + sections.append(f"Semantic ({len(sem_lines)}):") + sections.extend(sem_lines) + directive = "\n".join(sections) encoded = directive.encode("utf-8") if len(encoded) > CAP_BYTES: directive = encoded[: CAP_BYTES - 200].decode("utf-8", errors="ignore") + ( diff --git a/tests/hooks/test_session_close_rating_directive.py b/tests/hooks/test_session_close_rating_directive.py index 7718618..61bee9c 100644 --- a/tests/hooks/test_session_close_rating_directive.py +++ b/tests/hooks/test_session_close_rating_directive.py @@ -211,7 +211,7 @@ def test_db_error_falls_back_to_marker(self, tmp_path): markers = list((home / "spool").glob("*_session_end_*.json")) assert len(markers) == 1 - def test_directive_lists_overlooked_class(self, tmp_path, tmp_memory_db): + def test_directive_keeps_one_rule_line(self, tmp_path, tmp_memory_db): _seed_unrated_exposure(tmp_memory_db, "S1") env = { "BETTER_MEMORY_HOME": str(tmp_memory_db.parent), @@ -221,10 +221,17 @@ def test_directive_lists_overlooked_class(self, tmp_path, tmp_memory_db): assert result.returncode == 0 payload = json.loads(result.stdout) directive = payload["hookSpecificOutput"]["additionalContext"] - assert "overlooked" in directive + assert "Evidence line first; none possible = `ignored`." in directive + assert "rate-session-memories" in directive + # Rules that now live only in the skill must NOT be restated. + assert "Non-ignored ratings" not in directive + assert "overlooked" not in directive - def test_directive_requires_evidence_first(self, tmp_path, tmp_memory_db): - _seed_unrated_exposure(tmp_memory_db, "S1") + def test_directive_shows_source_labels_without_counts_line( + self, tmp_path, tmp_memory_db, + ): + _seed_unrated_exposure(tmp_memory_db, "S1", source="bootstrap") + _seed_semantic_exposure(tmp_memory_db, "S1", source="contextual") env = { "BETTER_MEMORY_HOME": str(tmp_memory_db.parent), "CLAUDE_SESSION_ID": "S1", @@ -233,19 +240,28 @@ def test_directive_requires_evidence_first(self, tmp_path, tmp_memory_db): assert result.returncode == 0 payload = json.loads(result.stdout) directive = payload["hookSpecificOutput"]["additionalContext"] - assert "evidence" in directive - assert ( - "FIRST write one line of evidence (what the memory changed, " - "or a quote)" in directive - ) - assert "if you cannot, the class is `ignored`" in directive - assert "Non-ignored ratings without an evidence line are rejected" in directive + assert "[bootstrap]" in directive + assert "[contextual]" in directive + assert "sources:" not in directive + assert "My Title" in directive + assert "My Semantic Fact" in directive - def test_directive_shows_source_labels_and_counts( + def test_display_snapshot_shown_for_foreign_id( self, tmp_path, tmp_memory_db, ): - _seed_unrated_exposure(tmp_memory_db, "S1", source="bootstrap") - _seed_semantic_exposure(tmp_memory_db, "S1", source="contextual") + """An agentcore-style exposure (mem- id, NO local content row) + renders its snapshotted display text.""" + c = connect(tmp_memory_db) + apply_migrations(c) + c.execute( + """INSERT INTO session_memory_exposure + (session_id, memory_kind, memory_id, exposed_at, source, display) + VALUES ('S1', 'reflection', 'mem-abc-123', + '2026-08-06T10:00:00+00:00', 'retrieve', + 'Snapshotted AWS Title')""" + ) + c.commit() + c.close() env = { "BETTER_MEMORY_HOME": str(tmp_memory_db.parent), "CLAUDE_SESSION_ID": "S1", @@ -254,6 +270,16 @@ def test_directive_shows_source_labels_and_counts( assert result.returncode == 0 payload = json.loads(result.stdout) directive = payload["hookSpecificOutput"]["additionalContext"] - assert "[bootstrap]" in directive - assert "[contextual]" in directive - assert "sources: bootstrap 1, contextual 1" in directive + assert "Snapshotted AWS Title" in directive + + def test_empty_semantic_bucket_omitted(self, tmp_path, tmp_memory_db): + _seed_unrated_exposure(tmp_memory_db, "S1") # reflection only + env = { + "BETTER_MEMORY_HOME": str(tmp_memory_db.parent), + "CLAUDE_SESSION_ID": "S1", + } + result = _run_hook(env) + payload = json.loads(result.stdout) + directive = payload["hookSpecificOutput"]["additionalContext"] + assert "Semantic" not in directive + assert "(none)" not in directive From b7074124393e999a2c49d76c23dd847d3ee5efac Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 6 Aug 2026 13:03:16 +0100 Subject: [PATCH 09/13] docs: display-snapshot exposure ledger + terse RATE_MEMORIES directive Co-Authored-By: Claude Fable 5 --- website/architecture.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/website/architecture.md b/website/architecture.md index 4fee2ba..8c3301c 100644 --- a/website/architecture.md +++ b/website/architecture.md @@ -201,13 +201,14 @@ described below: [`session_close`](https://github.com/emp3thy/better-memory/blob/main/better_memory/hooks/session_close.py) hook checks for unrated exposures. If any exist, it emits a `Stop` block directive triggering the `rate-session-memories` skill. The - directive lists each pending exposure with its source tag - (`[bootstrap]` / `[retrieve]` / `[contextual]`) and a leading - `sources: bootstrap N, contextual N, retrieve N` counts line, and - instructs (evidence-first): for each id, write the one-line evidence - FIRST — if there is nothing to point at, the class is `ignored`; only - once evidence exists does a `cited`/`shaped`/`misled`/`overlooked` class - follow. The skill then calls `memory.list_session_exposures` and + directive opens with an `Evidence line first; none possible = ignored` + instruction, then lists pending exposures grouped by kind + (`Reflections (N):` / `Semantic (N):`), one line per exposure — + `- {id} [{source}] {display}`, `display` truncated to 80 chars and + read via `COALESCE(e.display, r.title, s.content)` off the + `session_memory_exposure` row (falling back to the live title/content + only when no display snapshot was captured at exposure time). The + skill then calls `memory.list_session_exposures` and submits `memory.apply_session_ratings` with one `{class, evidence}` per id (`cited` / `shaped` / `ignored` / `misled` / `overlooked`; `ignored` is the only class evidence is optional for). `MemoryRatingService` From 48244ff0b9aa0c5bb6a88786f9591904c6f28dd7 Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 6 Aug 2026 13:06:36 +0100 Subject: [PATCH 10/13] =?UTF-8?q?docs:=20architecture.md=20directive=20des?= =?UTF-8?q?cription=20=E2=80=94=20restore=20RATE=5FMEMORIES=20header?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- website/architecture.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/website/architecture.md b/website/architecture.md index 8c3301c..0a2ecd8 100644 --- a/website/architecture.md +++ b/website/architecture.md @@ -201,11 +201,12 @@ described below: [`session_close`](https://github.com/emp3thy/better-memory/blob/main/better_memory/hooks/session_close.py) hook checks for unrated exposures. If any exist, it emits a `Stop` block directive triggering the `rate-session-memories` skill. The - directive opens with an `Evidence line first; none possible = ignored` - instruction, then lists pending exposures grouped by kind - (`Reflections (N):` / `Semantic (N):`), one line per exposure — - `- {id} [{source}] {display}`, `display` truncated to 80 chars and - read via `COALESCE(e.display, r.title, s.content)` off the + directive opens with a `RATE_MEMORIES: {n} unrated. Invoke skill + rate-session-memories.` header line, followed by an `Evidence line + first; none possible = ignored.` rule line, then pending exposures + grouped by kind (`Reflections (N):` / `Semantic (N):`), one line per + exposure — `- {id} [{source}] {display}`, `display` truncated to 80 + chars and read via `COALESCE(e.display, r.title, s.content)` off the `session_memory_exposure` row (falling back to the live title/content only when no display snapshot was captured at exposure time). The skill then calls `memory.list_session_exposures` and From f11c4ac8a98215422a851185659b0bed9fc3d9ed Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 6 Aug 2026 13:13:32 +0100 Subject: [PATCH 11/13] polish: deterministic display aggregation, legacy-dup comments, directive test precision Co-Authored-By: Claude Fable 5 --- better_memory/hooks/session_close.py | 14 ++++++++------ better_memory/services/exposure_log.py | 19 +++++++++++-------- .../test_session_close_rating_directive.py | 17 ++++++++++++++++- 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/better_memory/hooks/session_close.py b/better_memory/hooks/session_close.py index 0011969..2b4dcd4 100644 --- a/better_memory/hooks/session_close.py +++ b/better_memory/hooks/session_close.py @@ -60,11 +60,13 @@ def _emit_rating_directive_if_unrated(session_id: str) -> bool: return False conn = connect(cfg.memory_db) try: - # Dedupe by (memory_kind, memory_id) — a memory can have two - # exposure rows (bootstrap + retrieve) in one session. The - # MIN(exposed_at) keeps deterministic ordering; the rating - # apply path stamps ALL unrated rows per (kind, id) in one - # UPDATE, so one rating per unique memory is the correct + # Dedupe by (memory_kind, memory_id) — every current writer + # enforces at most one exposure row per (session, kind, id) via + # a NOT-EXISTS guard, so duplicates here can only be legacy rows + # predating that guard. The GROUP BY handles those legacy + # duplicates; MIN(exposed_at) keeps deterministic ordering, and + # the rating apply path stamps ALL unrated rows per (kind, id) + # in one UPDATE, so one rating per unique memory is the correct # contract to surface to the LLM. # Standalone copy, intentionally not delegated to # better_memory.services.exposure_log.list_unrated (see that @@ -76,7 +78,7 @@ def _emit_rating_directive_if_unrated(session_id: str) -> bool: SELECT e.memory_kind, e.memory_id, MIN(e.exposed_at) AS exposed_at, MIN(e.source) AS source, - COALESCE(e.display, r.title, s.content) AS display + COALESCE(MAX(e.display), r.title, s.content) AS display FROM session_memory_exposure e LEFT JOIN reflections r ON e.memory_kind='reflection' AND e.memory_id = r.id diff --git a/better_memory/services/exposure_log.py b/better_memory/services/exposure_log.py index 42a00cc..3783720 100644 --- a/better_memory/services/exposure_log.py +++ b/better_memory/services/exposure_log.py @@ -93,13 +93,16 @@ def record( def list_unrated(conn: sqlite3.Connection, *, session_id: str) -> list[sqlite3.Row]: """Return unrated exposure rows for ``session_id``, grouped by memory. - A memory can have two exposure rows (bootstrap + retrieve) in one - session; the rating apply path stamps ALL unrated rows per (kind, id) in - one UPDATE, so callers need one entry per unique memory rather than one - per raw row. Rows are grouped by (memory_kind, memory_id) with - ``MIN(exposed_at)`` / ``MIN(source)`` for deterministic first-exposure - values, joined against ``reflections.title`` / ``semantic_memories.content`` - for display, ordered by first exposure ascending, and rated rows + Every current writer enforces at most one exposure row per (session, + kind, id) via a NOT-EXISTS guard, so a memory having more than one row + here can only be a legacy duplicate predating that guard. The GROUP BY + handles those legacy duplicates; the rating apply path stamps ALL + unrated rows per (kind, id) in one UPDATE, so callers need one entry per + unique memory rather than one per raw row. Rows are grouped by + (memory_kind, memory_id) with ``MIN(exposed_at)`` / ``MIN(source)`` for + deterministic first-exposure values, joined against + ``reflections.title`` / ``semantic_memories.content`` for display, + ordered by first exposure ascending, and rated rows (``rated_at IS NOT NULL``) excluded. Returns rows with columns: ``memory_kind``, ``memory_id``, ``exposed_at``, @@ -110,7 +113,7 @@ def list_unrated(conn: sqlite3.Connection, *, session_id: str) -> list[sqlite3.R SELECT e.memory_kind, e.memory_id, MIN(e.exposed_at) AS exposed_at, MIN(e.source) AS source, - COALESCE(e.display, r.title, s.content) AS display + COALESCE(MAX(e.display), r.title, s.content) AS display FROM session_memory_exposure e LEFT JOIN reflections r ON e.memory_kind='reflection' AND e.memory_id = r.id diff --git a/tests/hooks/test_session_close_rating_directive.py b/tests/hooks/test_session_close_rating_directive.py index 61bee9c..9503c0b 100644 --- a/tests/hooks/test_session_close_rating_directive.py +++ b/tests/hooks/test_session_close_rating_directive.py @@ -281,5 +281,20 @@ def test_empty_semantic_bucket_omitted(self, tmp_path, tmp_memory_db): result = _run_hook(env) payload = json.loads(result.stdout) directive = payload["hookSpecificOutput"]["additionalContext"] - assert "Semantic" not in directive + assert "Semantic (" not in directive assert "(none)" not in directive + + def test_empty_reflection_bucket_omitted(self, tmp_path, tmp_memory_db): + c = connect(tmp_memory_db) + apply_migrations(c) + c.close() + _seed_semantic_exposure(tmp_memory_db, "S1") # semantic only + env = { + "BETTER_MEMORY_HOME": str(tmp_memory_db.parent), + "CLAUDE_SESSION_ID": "S1", + } + result = _run_hook(env) + payload = json.loads(result.stdout) + directive = payload["hookSpecificOutput"]["additionalContext"] + assert "Reflections (" not in directive + assert "My Semantic Fact" in directive From c9afa6996dd236b1ed0db25c4cf5c81109f860dc Mon Sep 17 00:00:00 2001 From: gethin Date: Thu, 6 Aug 2026 18:40:15 +0100 Subject: [PATCH 12/13] =?UTF-8?q?fix:=20covariant=20Sequence=20for=20expos?= =?UTF-8?q?ure=20items=20=E2=80=94=20pyright=20invariance?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- better_memory/services/exposure_log.py | 3 ++- better_memory/services/session_bootstrap.py | 4 ++-- better_memory/storage/agentcore.py | 4 ++-- better_memory/storage/protocol.py | 4 ++-- better_memory/storage/sqlite.py | 3 ++- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/better_memory/services/exposure_log.py b/better_memory/services/exposure_log.py index 3783720..d2142fc 100644 --- a/better_memory/services/exposure_log.py +++ b/better_memory/services/exposure_log.py @@ -34,6 +34,7 @@ from __future__ import annotations import sqlite3 +from collections.abc import Sequence _DISPLAY_TRUNC = 120 @@ -42,7 +43,7 @@ def record( conn: sqlite3.Connection, *, session_id: str, - items: list[tuple[str, str, str | None]], + items: Sequence[tuple[str, str, str | None]], source: str, now: str, exploration_ids: frozenset[str] = frozenset(), diff --git a/better_memory/services/session_bootstrap.py b/better_memory/services/session_bootstrap.py index 0a6775f..2ed2c01 100644 --- a/better_memory/services/session_bootstrap.py +++ b/better_memory/services/session_bootstrap.py @@ -18,7 +18,7 @@ """ from __future__ import annotations -from collections.abc import Callable +from collections.abc import Callable, Sequence from dataclasses import dataclass, field from datetime import UTC, datetime from pathlib import Path @@ -147,7 +147,7 @@ def record_exposures( self, *, session_id: str, - items: list[tuple[str, str, str | None]], + items: Sequence[tuple[str, str, str | None]], source: str, ) -> None: """Write one session_memory_exposure row per (kind, id, display) triple. diff --git a/better_memory/storage/agentcore.py b/better_memory/storage/agentcore.py index e556592..d4fc3ff 100644 --- a/better_memory/storage/agentcore.py +++ b/better_memory/storage/agentcore.py @@ -35,7 +35,7 @@ import json import sqlite3 import time -from collections.abc import Callable +from collections.abc import Callable, Sequence from datetime import UTC, datetime from typing import TYPE_CHECKING, Any from uuid import uuid4 @@ -2080,7 +2080,7 @@ def record_exposures( self, *, session_id: str, - items: list[tuple[str, str, str | None]], + items: Sequence[tuple[str, str, str | None]], source: str, ) -> None: """Write exposures to the local ledger when one is available. diff --git a/better_memory/storage/protocol.py b/better_memory/storage/protocol.py index 915cf7f..de63396 100644 --- a/better_memory/storage/protocol.py +++ b/better_memory/storage/protocol.py @@ -29,9 +29,9 @@ from __future__ import annotations +from collections.abc import Sequence from typing import Any, Literal, Protocol, runtime_checkable - # Outcome aliases are DEFINED here (not imported from services) to avoid a # `better_memory.storage → services` import edge in the public surface. The same # literal set also exists in `services/observation.py:76` and `search/hybrid.py:33`; @@ -378,7 +378,7 @@ def record_exposures( self, *, session_id: str, - items: list[tuple[str, str, str | None]], + items: Sequence[tuple[str, str, str | None]], source: str, ) -> None: """Record (kind, id, display) memory exposures for later rating. diff --git a/better_memory/storage/sqlite.py b/better_memory/storage/sqlite.py index a97c443..e86fdef 100644 --- a/better_memory/storage/sqlite.py +++ b/better_memory/storage/sqlite.py @@ -29,6 +29,7 @@ from __future__ import annotations import sqlite3 +from collections.abc import Sequence from typing import Any from better_memory.services.episode import EpisodeService @@ -440,7 +441,7 @@ def record_exposures( self, *, session_id: str, - items: list[tuple[str, str, str | None]], + items: Sequence[tuple[str, str, str | None]], source: str, ) -> None: self._session_bootstrap.record_exposures( From a30d49961a367553884c5df743fa5c6f9b8ee651 Mon Sep 17 00:00:00 2001 From: gethin Date: Fri, 7 Aug 2026 01:42:52 +0100 Subject: [PATCH 13/13] chore: retrigger CI after GitHub Actions outage Co-Authored-By: Claude Fable 5