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/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/hooks/session_close.py b/better_memory/hooks/session_close.py index 657b909..2b4dcd4 100644 --- a/better_memory/hooks/session_close.py +++ b/better_memory/hooks/session_close.py @@ -60,22 +60,25 @@ 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 # 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(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 @@ -96,36 +99,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/better_memory/services/exposure_log.py b/better_memory/services/exposure_log.py index 0fcaf3b..d2142fc 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`` @@ -11,9 +14,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 +30,25 @@ 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 +from collections.abc import Sequence + +_DISPLAY_TRUNC = 120 def record( conn: sqlite3.Connection, *, session_id: str, - items: list[tuple[str, str]], + items: Sequence[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 +56,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 +68,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 ], ) @@ -72,13 +94,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``, @@ -89,7 +114,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(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/session_bootstrap.py b/better_memory/services/session_bootstrap.py index 5cbf519..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,10 +147,10 @@ def record_exposures( self, *, session_id: str, - items: list[tuple[str, str]], + items: Sequence[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..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 @@ -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: 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 fa90db2..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,10 +378,14 @@ def record_exposures( self, *, session_id: str, - items: list[tuple[str, str]], + items: Sequence[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..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]], + items: Sequence[tuple[str, str, str | None]], source: str, ) -> None: self._session_bootstrap.record_exposures( 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 new file mode 100644 index 0000000..f33dad7 --- /dev/null +++ b/docs/superpowers/specs/2026-08-06-rate-directive-display-design.md @@ -0,0 +1,130 @@ +# 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/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` + 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". 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/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/hooks/test_session_close_rating_directive.py b/tests/hooks/test_session_close_rating_directive.py index 7718618..9503c0b 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,31 @@ 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 + + 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 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_log.py b/tests/services/test_exposure_log.py index 4995459..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,9 +400,112 @@ 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 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 + + +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 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/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. # --------------------------------------------------------------------------- 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 diff --git a/website/architecture.md b/website/architecture.md index 4fee2ba..0a2ecd8 100644 --- a/website/architecture.md +++ b/website/architecture.md @@ -201,13 +201,15 @@ 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 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 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`