diff --git a/.claude/skills/rate-session-memories/SKILL.md b/.claude/skills/rate-session-memories/SKILL.md index 9cff2a3..da82e8a 100644 --- a/.claude/skills/rate-session-memories/SKILL.md +++ b/.claude/skills/rate-session-memories/SKILL.md @@ -14,28 +14,31 @@ Call `memory.list_session_exposures` (no arguments — the server resolves the current session from env). Read the returned list. This is the ONLY valid set of ids to rate. The list in the RATE_MEMORIES directive may have been truncated. -## STEP 2 — Classify each id +## STEP 2 — Evidence first, then classify -For each `(kind, id)` pair returned by the list, assign exactly ONE class: +For each `(kind, id)`: FIRST try to write ONE line of evidence — what the +memory changed in this session, or a quote of where you used it. Then: -- **cited** — you quoted or directly referenced the memory in a reply. -- **shaped** — the memory guided a decision but wasn't cited verbatim. -- **ignored** — you saw it but it didn't affect this session. (Default.) -- **misled** — it caused a wrong direction or wasted effort. -- **overlooked** — the memory was relevant and you should have applied - it, but you didn't, until the user explicitly pointed you back to it. +- Evidence line written → choose `cited` (quoted/directly referenced), + `shaped` (guided a decision), `misled` (sent you wrong), or `overlooked` + (user had to point you back to it). Include the evidence line in the + rating. +- No evidence line possible → the class is `ignored`. Full stop. Do not + reverse the order; choosing a class first and rationalising evidence + after is how ratings drift. Rules: - Quote the id in your reasoning so you can't drift. - Do not invent ids. Do not skip ids. -- If genuinely uncertain between two classes, prefer the lower one - (shaped > cited, ignored > shaped). `misled` is never a fallback. +- If genuinely uncertain between two EVIDENCED classes, prefer the weaker + claim (shaped over cited). `misled` is never a fallback. Once an + evidence line exists, `ignored` is no longer available — uncertainty + about class never reverses the evidence decision. - `overlooked` is never a fallback. Use it ONLY when the user explicitly pointed you back to a memory you already had and had not applied — that user intervention is the observable anchor. Test for it first, separately from the cited/shaped/ignored axis. No anchor event → not `overlooked`. -- Default is `ignored`. "Shaped" requires evidence you can point to. ## STEP 3 — Submit ALL ratings in ONE call @@ -45,7 +48,8 @@ Call `memory.apply_session_ratings` with this exact shape (no `session_id` ```json { "ratings": [ - {"kind": "reflection", "id": "r-abc...", "class": "cited"}, + {"kind": "reflection", "id": "r-abc...", "class": "cited", + "evidence": "Used its junit-xml flag verbatim in the pytest invocation."}, {"kind": "semantic", "id": "s-def...", "class": "ignored"} ] } diff --git a/better_memory/db/migrations/0016_rating_evidence.sql b/better_memory/db/migrations/0016_rating_evidence.sql new file mode 100644 index 0000000..7446bb1 --- /dev/null +++ b/better_memory/db/migrations/0016_rating_evidence.sql @@ -0,0 +1,13 @@ +-- Migration 0016: evidence line on rated exposures. +-- +-- Rating variance is the noise floor under every ranking signal: identical +-- memory sets were rated `shaped` in one session and `ignored` in another +-- (2026-07 A/B runs). Non-ignored ratings now carry a one-line evidence +-- statement (what the memory changed, or a quote), enforced by +-- MemoryRatingService and surfaced in the UI drawers. Audit-only: no +-- scoring reads this column. +-- +-- Distinct from reflections.evidence_count, which counts synthesis source +-- observations and has nothing to do with rating evidence. + +ALTER TABLE session_memory_exposure ADD COLUMN evidence TEXT; diff --git a/better_memory/hooks/session_close.py b/better_memory/hooks/session_close.py index 4ac8ae6..f16ecda 100644 --- a/better_memory/hooks/session_close.py +++ b/better_memory/hooks/session_close.py @@ -224,12 +224,12 @@ def _emit_rating_directive_if_unrated(session_id: str) -> bool: + ("\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\nFor each id, classify as one of:\n" - " cited / shaped / ignored / misled / overlooked " - "(default: ignored)\n\n" - "Most exposures default to `ignored` — only flag the few " - "that actually shaped the session or misled you. Invoke " - "the skill `rate-session-memories`." + + "\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`." ) encoded = directive.encode("utf-8") if len(encoded) > CAP_BYTES: diff --git a/better_memory/mcp/handlers/sessions.py b/better_memory/mcp/handlers/sessions.py index 54b6d74..bc39462 100644 --- a/better_memory/mcp/handlers/sessions.py +++ b/better_memory/mcp/handlers/sessions.py @@ -143,6 +143,7 @@ async def credit(self, args: dict[str, Any]) -> list[TextContent]: kind=args["kind"], id=args["id"], classification=args["class"], + evidence=args.get("evidence"), ) else: payload = self._memory_rating.credit_one( @@ -150,6 +151,7 @@ async def credit(self, args: dict[str, Any]) -> list[TextContent]: kind=args["kind"], id=args["id"], classification=args["class"], + evidence=args.get("evidence"), ) return [TextContent(type="text", text=json.dumps(payload))] diff --git a/better_memory/mcp/tools.py b/better_memory/mcp/tools.py index 5656acc..076077c 100644 --- a/better_memory/mcp/tools.py +++ b/better_memory/mcp/tools.py @@ -477,7 +477,10 @@ def tool_definitions( "(resolved server-side from CLAUDE_SESSION_ID). Called " "at session end by the rate-session-memories skill. " "Raises if CLAUDE_SESSION_ID is unset — call only inside " - "an active Claude session." + "an active Claude session. Non-ignored classes require " + "`evidence` (one line: what the memory changed, or a " + "quote); write the evidence line FIRST — no evidence " + "means the class is `ignored`." ), inputSchema={ "type": "object", @@ -500,6 +503,15 @@ def tool_definitions( "misled", "overlooked", ] }, + # Optional here at the wire-schema level; + # MemoryRatingService.apply_session_ratings + # enforces the real contract (required + + # non-empty for every non-ignored class, + # <=EVIDENCE_MAX_CHARS). + "evidence": { + "type": "string", + "maxLength": 500, + }, }, }, }, @@ -516,16 +528,20 @@ def tool_definitions( "class must be 'cited', 'shaped', 'misled', or " "'overlooked' — NOT 'ignored'. Use 'overlooked' when the " "user pointed you back to a memory you already had but " - "had not applied." + "had not applied. Always include `evidence`: one line — " + "what the memory changed, or a quote. If you cannot " + "write one, the memory was `ignored`; do not call " + "credit." ), inputSchema={ "type": "object", "additionalProperties": False, - "required": ["kind", "id", "class"], + "required": ["kind", "id", "class", "evidence"], "properties": { "kind": {"enum": ["reflection", "semantic"]}, "id": {"type": "string"}, "class": {"enum": ["cited", "shaped", "misled", "overlooked"]}, + "evidence": {"type": "string", "maxLength": 500}, }, }, ), diff --git a/better_memory/services/memory_rating.py b/better_memory/services/memory_rating.py index 6977f18..e503a2a 100644 --- a/better_memory/services/memory_rating.py +++ b/better_memory/services/memory_rating.py @@ -14,7 +14,7 @@ import sqlite3 from collections.abc import Callable from datetime import datetime -from typing import Literal, TypedDict +from typing import Any, Literal, TypedDict from better_memory._common import default_clock @@ -61,6 +61,28 @@ class ApplySessionRatingsResult(TypedDict): _VALID_CLASSES: set[str] = {"cited", "shaped", "ignored", "misled", "overlooked"} _CREDIT_CLASSES: set[str] = {"cited", "shaped", "misled", "overlooked"} +EVIDENCE_MAX_CHARS = 500 + + +def _validate_evidence(cls: str, evidence: object, *, where: str) -> str | None: + """Trim + enforce the evidence contract for one rating. + + Non-ignored classes require a non-empty line; `ignored` may carry one. + Returns the trimmed value (or None). Raises ValueError with the + caller-supplied position prefix on violation. + """ + trimmed = evidence.strip() if isinstance(evidence, str) else None + if cls != "ignored" and not trimmed: + raise ValueError( + f"{where}: class {cls!r} requires a non-empty evidence line " + "(what the memory changed, or a quote); if there is nothing " + "to point at, the class is 'ignored'") + if trimmed and len(trimmed) > EVIDENCE_MAX_CHARS: + raise ValueError( + f"{where}: evidence exceeds {EVIDENCE_MAX_CHARS} chars " + f"({len(trimmed)})") + return trimmed or None + class MemoryRatingService: """Writes useful_count / times_misled on reflections + semantic memories, @@ -84,12 +106,20 @@ def credit_one( kind: str, id: str, classification: str, + evidence: str | None = None, ) -> ApplyOutcome: """Apply one rating for (session_id, kind, id). Validation (ValueError before any DB write): - kind must be 'reflection' or 'semantic'. - classification must be 'cited', 'shaped', 'misled', or 'overlooked' (NOT 'ignored'). + - evidence must be a non-empty (post-strip), <=EVIDENCE_MAX_CHARS + line; all credit classes are non-ignored, so this is effectively + required. `evidence` defaults to None only as a runtime-safety + compat shim for callers that have not yet been updated to pass + it (see MCP `memory.credit` handler) — None fails validation + with a clear ValueError instead of a TypeError, so an un-updated + caller gets a clean error response rather than a crash. Skip outcomes (no exception, no write, returned via dict): - 'not_exposed' — no matching exposure row for this session. @@ -123,6 +153,9 @@ def credit_one( f"Invalid classification: {classification!r}. " f"Expected one of {_CREDIT_CLASSES}" ) + trimmed_evidence = _validate_evidence( + classification, evidence, where="credit" + ) now = self._clock().isoformat() self._conn.execute("SAVEPOINT memory_credit") @@ -130,6 +163,7 @@ def credit_one( outcome = self._apply_one( session_id=session_id, kind=kind, memory_id=id, classification=classification, now=now, + evidence=trimmed_evidence, ) except BaseException: self._conn.execute("ROLLBACK TO SAVEPOINT memory_credit") @@ -149,6 +183,7 @@ def _apply_one( memory_id: str, classification: str, now: str, + evidence: str | None = None, ) -> ApplyOutcome: """Inside-savepoint per-row apply. Returns the same dict shape as credit_one. Shared by credit_one and apply_session_ratings (Task 3). @@ -223,10 +258,10 @@ def _apply_one( # 4. Stamp the exposure row. self._conn.execute( "UPDATE session_memory_exposure " - "SET rated_at = ?, classification = ? " + "SET rated_at = ?, classification = ?, evidence = ? " "WHERE session_id = ? AND memory_kind = ? AND memory_id = ?" " AND rated_at IS NULL", - (now, classification, session_id, kind, memory_id), + (now, classification, evidence, session_id, kind, memory_id), ) return {"applied": classification, "skipped": None} @@ -236,7 +271,7 @@ def apply_session_ratings( self, *, session_id: str, - ratings: list[dict[str, str]], + ratings: list[dict[str, Any]], ) -> ApplySessionRatingsResult: """Atomic batch update at session end. @@ -246,6 +281,10 @@ def apply_session_ratings( - each entry must have kind in {'reflection', 'semantic'}, class in {'cited', 'shaped', 'ignored', 'misled', 'overlooked'}, and a string id. + - each entry may carry an optional "evidence" string. Non-ignored + classes require a non-empty (post-strip), <=EVIDENCE_MAX_CHARS + evidence line; 'ignored' may carry one or omit it. The trimmed + value is stored on session_memory_exposure.evidence. - no duplicate (kind, id) pairs in one batch. Inside the SAVEPOINT, each entry runs through _apply_one. Skip @@ -286,6 +325,9 @@ class in {'cited', 'shaped', 'ignored', 'misled', 'overlooked'}, f"ratings[{i}].class: invalid {cls!r}; " f"expected one of {_VALID_CLASSES}" ) + r["_evidence"] = _validate_evidence( + cls, r.get("evidence"), where=f"ratings[{i}]" + ) if not isinstance(rid, str) or not rid: raise ValueError(f"ratings[{i}].id: must be non-empty string") key = (kind, rid) @@ -314,6 +356,7 @@ class in {'cited', 'shaped', 'ignored', 'misled', 'overlooked'}, memory_id=r["id"], classification=r["class"], now=now, + evidence=r["_evidence"], ) applied_class = outcome["applied"] skipped_reason = outcome["skipped"] diff --git a/better_memory/services/relevant.py b/better_memory/services/relevant.py index 60dfd01..61b5c5e 100644 --- a/better_memory/services/relevant.py +++ b/better_memory/services/relevant.py @@ -297,7 +297,8 @@ def retrieve_relevant( ) _BLOCK_FOOTER = ( "If any entry above materially helps or misleads this task, credit it now: " - "memory_credit(kind, id, 'cited'|'shaped'|'misled').\n" + "memory_credit(kind, id, class, evidence) - include a one-line evidence " + "statement.\n" "" ) diff --git a/better_memory/storage/agentcore.py b/better_memory/storage/agentcore.py index fe38bc9..5893a99 100644 --- a/better_memory/storage/agentcore.py +++ b/better_memory/storage/agentcore.py @@ -1359,6 +1359,7 @@ def credit_one( kind: str, id: str, classification: str, + evidence: str | None = None, ) -> dict[str, Any]: """Apply one classification → counter increment on a record. @@ -1366,7 +1367,13 @@ def credit_one( cited / shaped → useful_count ignored → ignored_count misled → times_misled - overlooked → overlooked_count""" + overlooked → overlooked_count + + `evidence` is accepted for StorageBackend protocol parity only. + Agentcore mode has no exposure table (see list_session_exposures) + and therefore nowhere to store a rating-evidence line; it is not + validated or persisted here — sqlite is the only backend with an + evidence contract (MemoryRatingService).""" counter_key = _RATING_TO_COUNTER.get(classification) if counter_key is None: raise ValueError( diff --git a/better_memory/storage/protocol.py b/better_memory/storage/protocol.py index 3830131..ee7afcd 100644 --- a/better_memory/storage/protocol.py +++ b/better_memory/storage/protocol.py @@ -289,8 +289,17 @@ def credit_one( kind: str, id: str, classification: str, + evidence: str | None = None, ) -> Any: - """Apply a single rating for an exposed memory. Returns ApplyOutcome.""" + """Apply a single rating for an exposed memory. Returns ApplyOutcome. + + `evidence` is a one-line string (what the memory changed, or a + quote); non-ignored classes require a non-empty value — + SqliteBackend forwards it to MemoryRatingService, which validates + and stores it. AgentCoreBackend accepts the parameter for + signature parity but has no evidence storage (no exposure table) + and does not validate or persist it — see AgentCoreBackend.credit_one. + """ ... # ----- Synthesis (sqlite-only — guarded by supports_synthesis) ----- diff --git a/better_memory/storage/sqlite.py b/better_memory/storage/sqlite.py index 5dab7fd..a261fa5 100644 --- a/better_memory/storage/sqlite.py +++ b/better_memory/storage/sqlite.py @@ -327,9 +327,11 @@ def credit_one( kind: str, id: str, classification: str, + evidence: str | None = None, ) -> Any: return self._memory_rating.credit_one( session_id=session_id, kind=kind, id=id, classification=classification, + evidence=evidence, ) # ----- Synthesis ----- diff --git a/better_memory/ui/app.py b/better_memory/ui/app.py index c508655..dbe1489 100644 --- a/better_memory/ui/app.py +++ b/better_memory/ui/app.py @@ -134,6 +134,24 @@ def _origin_check() -> None: import json as _json + _RATING_CHIP_CLASS = { + "cited": "outcome-success", + "shaped": "outcome-success", + "misled": "outcome-failure", + "overlooked": "outcome-partial", + "ignored": "outcome-no_outcome", + } + + @app.template_filter("rating_chip_class") + def _rating_chip_class(classification: str) -> str: + """Map a rating classification to its `.chip`/`.outcome-badge` class. + + cited/shaped -> outcome-success, misled -> outcome-failure, + overlooked -> outcome-partial, ignored (and anything unknown) -> + outcome-no_outcome. + """ + return _RATING_CHIP_CLASS.get(classification, "outcome-no_outcome") + @app.template_filter("decode_hints") def _decode_hints(raw: str | None) -> list[str]: """Decode the hints column for template display. @@ -339,8 +357,12 @@ def reflections_drawer(id: str) -> str: detail = queries.reflection_detail(conn, reflection_id=id) if detail is None: abort(404) + rating_evidence = queries.fetch_rating_evidence( + conn, "reflection", id + ) return render_template( - "fragments/reflection_drawer.html", detail=detail + "fragments/reflection_drawer.html", + detail=detail, rating_evidence=rating_evidence, ) @app.post("/reflections//confirm") @@ -357,8 +379,12 @@ def reflection_confirm(id: str) -> tuple[str, int, dict[str, str]]: "" ), 409, {} detail = queries.reflection_detail(conn, reflection_id=id) + rating_evidence = queries.fetch_rating_evidence( + conn, "reflection", id + ) rendered = render_template( - "fragments/reflection_drawer.html", detail=detail + "fragments/reflection_drawer.html", + detail=detail, rating_evidence=rating_evidence, ) return rendered, 200, {"HX-Trigger": "reflection-changed"} @@ -376,8 +402,12 @@ def reflection_retire(id: str) -> tuple[str, int, dict[str, str]]: "" ), 409, {} detail = queries.reflection_detail(conn, reflection_id=id) + rating_evidence = queries.fetch_rating_evidence( + conn, "reflection", id + ) rendered = render_template( - "fragments/reflection_drawer.html", detail=detail + "fragments/reflection_drawer.html", + detail=detail, rating_evidence=rating_evidence, ) return rendered, 200, {"HX-Trigger": "reflection-changed"} @@ -420,8 +450,12 @@ def reflection_edit_save(id: str) -> tuple[str, int, dict[str, str]]: "" ), 409, {} detail = queries.reflection_detail(conn, reflection_id=id) + rating_evidence = queries.fetch_rating_evidence( + conn, "reflection", id + ) rendered = render_template( - "fragments/reflection_drawer.html", detail=detail + "fragments/reflection_drawer.html", + detail=detail, rating_evidence=rating_evidence, ) return rendered, 200, {"HX-Trigger": "reflection-changed"} @@ -441,8 +475,12 @@ def reflection_promote(id: str) -> tuple[str, int, dict[str, str]]: "" ), 409, {} detail = queries.reflection_detail(conn, reflection_id=id) + rating_evidence = queries.fetch_rating_evidence( + conn, "reflection", id + ) rendered = render_template( - "fragments/reflection_drawer.html", detail=detail + "fragments/reflection_drawer.html", + detail=detail, rating_evidence=rating_evidence, ) return rendered, 200, {"HX-Trigger": "reflection-changed"} @@ -542,8 +580,10 @@ def semantic_drawer(id: str): "times_overlooked": row["times_overlooked"] or 0, "last_overlooked_at": row["last_overlooked_at"], } + rating_evidence = queries.fetch_rating_evidence(conn, "semantic", id) return render_template( - "fragments/semantic_drawer.html", memory=memory, + "fragments/semantic_drawer.html", + memory=memory, rating_evidence=rating_evidence, ) @app.post("/semantic//update") @@ -640,6 +680,7 @@ def diagnostics() -> str: recent_ratings = conn.execute( """ SELECT e.rated_at, e.memory_kind, e.memory_id, e.classification, + e.evidence, COALESCE(r.title, s.content) AS display FROM session_memory_exposure e LEFT JOIN reflections r ON e.memory_kind='reflection' diff --git a/better_memory/ui/queries.py b/better_memory/ui/queries.py index 5499752..ca95918 100644 --- a/better_memory/ui/queries.py +++ b/better_memory/ui/queries.py @@ -782,6 +782,50 @@ class RetentionRunRow: triggered_by: str +@dataclass(frozen=True) +class RatingEvidenceRow: + """One rated exposure that carries an evidence line. + + Sourced from ``session_memory_exposure`` (migration 0016's ``evidence`` + column). Distinct from ``ReflectionFull.evidence_count``/``evidence_count`` + on ``ReflectionListRow`` — those count synthesis SOURCE OBSERVATIONS and + have nothing to do with rating evidence. + """ + + classification: str + evidence: str + rated_at: str + + +def fetch_rating_evidence( + conn: sqlite3.Connection, kind: str, memory_id: str, limit: int = 10, +) -> list[RatingEvidenceRow]: + """Return rated exposures with a non-null evidence line, newest first. + + ``kind`` is ``'reflection'`` or ``'semantic'`` (matches + ``session_memory_exposure.memory_kind``). Only rows where + ``evidence IS NOT NULL`` are returned — ``ignored`` classifications may + or may not carry one (see MemoryRatingService), everything else is + required to. ``limit`` is enforced in SQL via ``LIMIT``. + """ + rows = conn.execute( + "SELECT classification, evidence, rated_at " + "FROM session_memory_exposure " + "WHERE memory_kind = ? AND memory_id = ? AND evidence IS NOT NULL " + "ORDER BY rated_at DESC " + "LIMIT ?", + (kind, memory_id, limit), + ).fetchall() + return [ + RatingEvidenceRow( + classification=r["classification"], + evidence=r["evidence"], + rated_at=r["rated_at"], + ) + for r in rows + ] + + def retention_runs_list_for_ui( conn: sqlite3.Connection, *, limit: int = 30, ) -> list[RetentionRunRow]: diff --git a/better_memory/ui/static/app.css b/better_memory/ui/static/app.css index 674bb99..e8dcc3a 100644 --- a/better_memory/ui/static/app.css +++ b/better_memory/ui/static/app.css @@ -579,7 +579,7 @@ textarea { resize: vertical; line-height: 1.55; } .outcome-badge.outcome-abandoned { border-color: var(--brut-muted); color: var(--brut-muted); } -.outcome-badge.outcome-no_outcome, .chip.outcome-neutral { +.outcome-badge.outcome-no_outcome, .chip.outcome-neutral, .chip.outcome-no_outcome { border-color: var(--brut-rule); color: var(--brut-muted); } .outcome-badge.outcome-open { @@ -877,6 +877,34 @@ textarea { resize: vertical; line-height: 1.55; } text-transform: lowercase; } +.rating-evidence-list { + list-style: none; + padding: 0; + margin: 0; +} +.rating-evidence-item { + display: flex; + align-items: baseline; + gap: 10px; + padding: 8px 0; + border-bottom: 1px dashed var(--brut-rule); + font-size: 13px; + color: var(--brut-ink); +} +.rating-evidence-item:last-child { border-bottom: none; } +.rating-evidence-text { + flex: 1; + font-size: 12.5px; + line-height: 1.5; +} +.rating-evidence-date { + font-family: var(--brut-mono); + font-size: 10.5px; + color: var(--brut-muted); + letter-spacing: 0.06em; + white-space: nowrap; +} + /* ============================================================ Reflections — filter form + rows ============================================================ */ diff --git a/better_memory/ui/templates/diagnostics.html b/better_memory/ui/templates/diagnostics.html index d77dc7a..3e91154 100644 --- a/better_memory/ui/templates/diagnostics.html +++ b/better_memory/ui/templates/diagnostics.html @@ -22,7 +22,7 @@

Retention runs

Recent ratings

Last 20 rated session_memory_exposure rows.

- + {% for r in recent_ratings %} @@ -31,6 +31,7 @@

Recent ratings

+ {% endfor %} diff --git a/better_memory/ui/templates/fragments/reflection_drawer.html b/better_memory/ui/templates/fragments/reflection_drawer.html index 66d6c6f..4df337c 100644 --- a/better_memory/ui/templates/fragments/reflection_drawer.html +++ b/better_memory/ui/templates/fragments/reflection_drawer.html @@ -118,4 +118,19 @@

Source observations ({{ detail.sources | length }})

No source observations linked yet.

{% endif %} + + {% if rating_evidence %} +
+

Rating evidence

+
    + {% for ev in rating_evidence %} +
  • + {{ ev.classification }} + {{ ev.evidence }} + {{ ev.rated_at }} +
  • + {% endfor %} +
+
+ {% endif %} diff --git a/better_memory/ui/templates/fragments/semantic_drawer.html b/better_memory/ui/templates/fragments/semantic_drawer.html index 2298f82..c9f7c81 100644 --- a/better_memory/ui/templates/fragments/semantic_drawer.html +++ b/better_memory/ui/templates/fragments/semantic_drawer.html @@ -30,4 +30,19 @@

Edit semantic memory

+ + {% if rating_evidence %} +
+

Rating evidence

+
    + {% for ev in rating_evidence %} +
  • + {{ ev.classification }} + {{ ev.evidence }} + {{ ev.rated_at }} +
  • + {% endfor %} +
+
+ {% endif %} diff --git a/docs/superpowers/plans/2026-07-24-evidence-ratings.md b/docs/superpowers/plans/2026-07-24-evidence-ratings.md new file mode 100644 index 0000000..7f5838e --- /dev/null +++ b/docs/superpowers/plans/2026-07-24-evidence-ratings.md @@ -0,0 +1,487 @@ +# Evidence-Anchored Ratings (PR-B) 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:** Non-ignored ratings require a one-line evidence statement, stored on the exposure row and surfaced in the UI drawers. + +**Architecture:** Migration 0016 adds nullable `evidence TEXT` to `session_memory_exposure`. `MemoryRatingService` validates evidence in its existing pre-savepoint pass (non-ignored ⇒ required, trimmed, ≤500 chars) and stamps it alongside `classification`. Tool schemas, the rating skill (evidence-first ordering), the Stop-hook directive, and the UI drawers/diagnostics follow. Audit-only: no scoring use of evidence. + +**Tech Stack:** Python 3.12, sqlite, Flask/htmx UI (project-native CSS), pytest. + +**Spec:** `docs/superpowers/specs/2026-07-23-retrieval-quality-design.md` §4, plus post-PR-D deltas (rendered summary in the design review). + +## Global Constraints + +- New branch `feat/evidence-ratings` from current main (create at Task 1 start). +- Test command: `./.venv/Scripts/python.exe -m pytest -v`; pyright stays 0 errors. +- **Naming collision guard:** `evidence_count` already exists on reflections meaning *count of source observations from synthesis*. The new artefact is always the column `evidence` (exposures) / field `rating_evidence` (UI read-models) / section title "Rating evidence". Never mix them. +- Evidence rule (exact): classes `cited/shaped/misled/overlooked` REQUIRE evidence — a string that is non-empty after `.strip()`, max 500 chars post-trim; violation ⇒ `ValueError`, whole batch rejected (matches existing validate-before-savepoint behaviour). Class `ignored`: evidence optional; stored if provided (same trim/length rule when present). +- UI uses project-native classes (`outcome-badge`/`chip` family in `ui/static/app.css:556-585`) — Bootstrap classes are NOT available (recorded gotcha). +- ASCII only. Ruff line 100. Website-sync guardrail applies. Stage exact paths, never `git add -A`. Commit per task, footer `Co-Authored-By: Claude Fable 5 `. +- Test fixtures constructing a BETTER_MEMORY_HOME or app factory pin `BETTER_MEMORY_EMBEDDINGS_BACKEND=sqlite` (recorded recurring gotcha). + +## Verified-against-source facts (do not re-derive) + +| Fact | Where verified | +|---|---| +| Validation loop shape: `apply_session_ratings` raises per-item `ValueError`s BEFORE the savepoint (~memory_rating.py:263-297); `_VALID_CLASSES`/`_CREDIT_CLASSES` at :61-62 | read 2026-07-24 | +| `_apply_one(*, session_id, kind, memory_id, classification, now)` stamps ALL unrated rows: `UPDATE session_memory_exposure SET rated_at = ?, classification = ? WHERE ... AND rated_at IS NULL` (~:226) | read | +| `credit_one` validates kind (:33) and class ∈ `_CREDIT_CLASSES` (:121), wraps `SAVEPOINT memory_credit` | read | +| `memory.credit` schema: `required: [kind, id, class]`, `additionalProperties: False` (tools.py ~:510-531); `apply_session_ratings` items schema nearby (~:474) | read | +| Handlers: `mcp/handlers/sessions.py` — credit ~:136, apply ~:114; both pass args straight to the service | PR-A map, unchanged | +| Migrations: latest is `0015_via_exploration.sql` ⇒ this PR is **0016**; the exact-column-set tests (test_migration_0009/0010/0012 + schema tests) will need the new column added | read + Task-2-PR-D precedent | +| UI badges: `.outcome-badge`, `.chip`, variants `.outcome-success` etc. (app.css:556-585); diagnostics.html renders `` (unstyled — pre-existing) | read | +| Drawer read-models: `ui/queries.py` `ReflectionDetail` (~:324, has `evidence_count` = SOURCE-observation count) built ~:392-460; semantic drawer route app.py ~:523, template `semantic.html` + `fragments/` | read | +| Rating skill: `.claude/skills/rate-session-memories/SKILL.md`, STEP 2 at line 17, current rule line 38 "Default is `ignored`..." — user-level copy is a junction to this file (auto-updates) | read + deploy history | +| Stop-hook directive text: `hooks/session_close.py::_emit_rating_directive_if_unrated` (~:200-233), tested by `tests/hooks/test_session_close_rating_directive.py` | PR-#81 work | +| Contextual footer nudge: `services/relevant.py::_BLOCK_FOOTER` (~:140) says "credit it now: memory_credit(kind, id, 'cited'|'shaped'|'misled')" | read (PR-D) | + +--- + +### Task 1: Migration 0016 — evidence column + +**Files:** +- Create: `better_memory/db/migrations/0016_rating_evidence.sql` +- Modify: exact-column-set tests (grep `via_exploration` in tests/db and tests/services — every list that enumerates session_memory_exposure columns gains `evidence`) +- Test: `tests/db/test_schema.py` (append) + +**Interfaces:** +- Produces: nullable `evidence TEXT` on `session_memory_exposure`. Consumed by Tasks 2, 5. + +- [ ] **Step 1: Write the failing test** (mirror the 0015 test): + +```python +def test_0016_rating_evidence_column(tmp_memory_db): + conn = connect(tmp_memory_db) + apply_migrations(conn) + cols = [r[1] for r in conn.execute("PRAGMA table_info(session_memory_exposure)")] + assert "evidence" in cols + conn.close() +``` + +Also: create the branch first — `git checkout -b feat/evidence-ratings` from main. + +- [ ] **Step 2: Run to verify failure** + +Run: `./.venv/Scripts/python.exe -m pytest tests/db -q -k 0016` +Expected: FAIL — column missing. + +- [ ] **Step 3: Write the migration** + +```sql +-- Migration 0016: evidence line on rated exposures. +-- +-- Rating variance is the noise floor under every ranking signal: identical +-- memory sets were rated `shaped` in one session and `ignored` in another +-- (2026-07 A/B runs). Non-ignored ratings now carry a one-line evidence +-- statement (what the memory changed, or a quote), enforced by +-- MemoryRatingService and surfaced in the UI drawers. Audit-only: no +-- scoring reads this column. +-- +-- Distinct from reflections.evidence_count, which counts synthesis source +-- observations and has nothing to do with rating evidence. + +ALTER TABLE session_memory_exposure ADD COLUMN evidence TEXT; +``` + +Update every exact-column-set assertion found by: +`grep -rn "via_exploration" tests/ --include="*.py" -l` — add `"evidence"` beside it in each enumerated set. + +- [ ] **Step 4: Run** `./.venv/Scripts/python.exe -m pytest tests/db tests/services/test_exploration_tagging.py -q` — all pass. + +- [ ] **Step 5: Commit** + +```bash +git add better_memory/db/migrations/0016_rating_evidence.sql tests/db +git commit -m "feat(db): rating-evidence column on exposures (migration 0016) + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 2: Evidence validation + storage in MemoryRatingService + +**Files:** +- Modify: `better_memory/services/memory_rating.py` +- Test: `tests/services/test_memory_rating.py` (append class), `tests/services/test_rating_evidence.py` (new) + +**Interfaces:** +- Consumes: migration 0016. +- Produces: + - `credit_one(*, session_id, kind, id, classification, evidence: str)` — evidence REQUIRED (all credit classes are non-ignored). + - `apply_session_ratings(session_id, ratings)` — each rating dict may carry `"evidence"`; validation per the Global Constraints rule; the trimmed value passes to `_apply_one`. + - `_apply_one(..., evidence: str | None)` — stamp SQL becomes `SET rated_at = ?, classification = ?, evidence = ?`. + - Module constant `EVIDENCE_MAX_CHARS = 500`. + +- [ ] **Step 1: Write the failing tests** + +```python +# tests/services/test_rating_evidence.py +"""Non-ignored ratings must carry a one-line evidence statement. + +The ordering is the variance killer: the rater writes the evidence line +BEFORE choosing the class; nothing to point at means the class is +`ignored`. The server enforces the contract loudly - a violating batch is +rejected whole, before the savepoint, like every other validation error. +""" +from __future__ import annotations + +from pathlib import Path + +import pytest + +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from better_memory.services.memory_rating import ( + EVIDENCE_MAX_CHARS, + MemoryRatingService, +) + + +@pytest.fixture +def conn(tmp_memory_db: Path): + c = connect(tmp_memory_db) + apply_migrations(c) + try: + yield c + finally: + c.close() + + +def _seed(conn, rid="r1", session="s1"): + conn.execute( + """INSERT INTO reflections + (id, title, project, phase, polarity, use_cases, hints, + confidence, created_at, updated_at) + VALUES (?, ?, 'p', 'general', 'do', 'uc', '[]', 0.5, + '2026-01-01', '2026-01-01')""", (rid, rid)) + conn.execute( + """INSERT INTO session_memory_exposure + (session_id, memory_kind, memory_id, exposed_at, source) + VALUES (?, 'reflection', ?, '2026-01-01', 'retrieve')""", + (session, rid)) + conn.commit() + + +def _stored_evidence(conn, rid="r1"): + return conn.execute( + "SELECT evidence FROM session_memory_exposure WHERE memory_id = ?", + (rid,)).fetchone()[0] + + +class TestApplyBatchEvidence: + def test_shaped_without_evidence_rejected(self, conn): + _seed(conn) + svc = MemoryRatingService(conn) + with pytest.raises(ValueError, match="evidence"): + svc.apply_session_ratings( + session_id="s1", + ratings=[{"kind": "reflection", "id": "r1", "class": "shaped"}]) + + def test_blank_evidence_rejected(self, conn): + _seed(conn) + svc = MemoryRatingService(conn) + with pytest.raises(ValueError, match="evidence"): + svc.apply_session_ratings( + session_id="s1", + ratings=[{"kind": "reflection", "id": "r1", + "class": "cited", "evidence": " "}]) + + def test_overlong_evidence_rejected(self, conn): + _seed(conn) + svc = MemoryRatingService(conn) + with pytest.raises(ValueError, match="500"): + svc.apply_session_ratings( + session_id="s1", + ratings=[{"kind": "reflection", "id": "r1", "class": "shaped", + "evidence": "x" * (EVIDENCE_MAX_CHARS + 1)}]) + + def test_ignored_needs_no_evidence(self, conn): + _seed(conn) + svc = MemoryRatingService(conn) + out = svc.apply_session_ratings( + session_id="s1", + ratings=[{"kind": "reflection", "id": "r1", "class": "ignored"}]) + assert out["applied"]["ignored"] == 1 + assert _stored_evidence(conn) is None + + def test_valid_evidence_stored_trimmed(self, conn): + _seed(conn) + svc = MemoryRatingService(conn) + svc.apply_session_ratings( + session_id="s1", + ratings=[{"kind": "reflection", "id": "r1", "class": "shaped", + "evidence": " guided the retention fix approach "}]) + assert _stored_evidence(conn) == "guided the retention fix approach" + + def test_batch_atomic_on_evidence_violation(self, conn): + # One bad item rejects the WHOLE batch before anything applies. + _seed(conn, "r1") + _seed(conn, "r2") + svc = MemoryRatingService(conn) + with pytest.raises(ValueError): + svc.apply_session_ratings( + session_id="s1", + ratings=[ + {"kind": "reflection", "id": "r1", "class": "ignored"}, + {"kind": "reflection", "id": "r2", "class": "shaped"}, + ]) + rows = conn.execute( + "SELECT rated_at FROM session_memory_exposure").fetchall() + assert all(r[0] is None for r in rows) + + def test_ignored_with_evidence_stores_it(self, conn): + _seed(conn) + svc = MemoryRatingService(conn) + svc.apply_session_ratings( + session_id="s1", + ratings=[{"kind": "reflection", "id": "r1", "class": "ignored", + "evidence": "checked but task was unrelated"}]) + assert _stored_evidence(conn) == "checked but task was unrelated" + + +class TestCreditEvidence: + def test_credit_requires_evidence(self, conn): + _seed(conn) + svc = MemoryRatingService(conn) + with pytest.raises(TypeError): + svc.credit_one(session_id="s1", kind="reflection", id="r1", + classification="cited") # no evidence kwarg + + def test_credit_blank_evidence_rejected(self, conn): + _seed(conn) + svc = MemoryRatingService(conn) + with pytest.raises(ValueError, match="evidence"): + svc.credit_one(session_id="s1", kind="reflection", id="r1", + classification="cited", evidence="") + + def test_credit_stores_evidence(self, conn): + _seed(conn) + svc = MemoryRatingService(conn) + out = svc.credit_one(session_id="s1", kind="reflection", id="r1", + classification="shaped", + evidence="applied its retry guidance") + assert out == {"applied": "shaped", "skipped": None} + assert _stored_evidence(conn) == "applied its retry guidance" +``` + +(Read `credit_one`'s exact parameter name for the id — it is `id` per the +current signature; keep it.) + +- [ ] **Step 2: Run to verify failure** + +Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_rating_evidence.py -v` +Expected: FAIL — `EVIDENCE_MAX_CHARS` import error. + +- [ ] **Step 3: Implement** + +In `memory_rating.py`: + +```python +EVIDENCE_MAX_CHARS = 500 + + +def _validate_evidence(cls: str, evidence, *, where: str) -> str | None: + """Trim + enforce the evidence contract for one rating. + + Non-ignored classes require a non-empty line; `ignored` may carry one. + Returns the trimmed value (or None). Raises ValueError with the + caller-supplied position prefix on violation. + """ + trimmed = evidence.strip() if isinstance(evidence, str) else None + if cls != "ignored" and not trimmed: + raise ValueError( + f"{where}: class {cls!r} requires a non-empty evidence line " + "(what the memory changed, or a quote); if there is nothing " + "to point at, the class is 'ignored'") + if trimmed and len(trimmed) > EVIDENCE_MAX_CHARS: + raise ValueError( + f"{where}: evidence exceeds {EVIDENCE_MAX_CHARS} chars " + f"({len(trimmed)})") + return trimmed or None +``` + +- `apply_session_ratings` validation loop: after the class check, add + `r["_evidence"] = _validate_evidence(cls, r.get("evidence"), where=f"ratings[{i}]")` + (store the trimmed value on a scratch key; the apply loop below passes + `evidence=r["_evidence"]` into `_apply_one`). +- `credit_one`: signature gains `evidence: str` (keyword-only, required); + validate via `_validate_evidence(classification, evidence, where="credit")` + before the savepoint; pass trimmed value down. +- `_apply_one`: gains `evidence: str | None`; stamp SQL: + +```python + self._conn.execute( + "UPDATE session_memory_exposure " + "SET rated_at = ?, classification = ?, evidence = ? " + "WHERE session_id = ? AND memory_kind = ? AND memory_id = ?" + " AND rated_at IS NULL", + (now, classification, evidence, session_id, kind, memory_id), + ) +``` + +Existing tests in `test_memory_rating.py` that call these APIs without +evidence: update each non-ignored call site to pass a short evidence string +(mechanical; do not weaken assertions). + +- [ ] **Step 4: Run** + +Run: `./.venv/Scripts/python.exe -m pytest tests/services/test_rating_evidence.py tests/services/test_memory_rating.py tests/integration/test_memory_rating_e2e.py -q` +Expected: all pass (the e2e file's non-ignored ratings also gain evidence strings). + +- [ ] **Step 5: Commit** + +```bash +git add -A -- better_memory/services/memory_rating.py tests/services tests/integration +git commit -m "feat(rating): non-ignored ratings require a one-line evidence statement + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 3: Tool schemas + handlers + +**Files:** +- Modify: `better_memory/mcp/tools.py` (memory.credit ~:510; apply_session_ratings items ~:474) +- Modify: `better_memory/mcp/handlers/sessions.py` (credit ~:136 passes `evidence=args["evidence"]`; apply passes rating dicts through unchanged — service reads `evidence` key) +- Test: `tests/mcp/test_rating_tools.py` (extend) + +**Interfaces:** +- Consumes: Task 2 service signatures. +- Produces: `memory.credit` schema `required: [kind, id, class, evidence]`, `evidence: {type: string, maxLength: 500}`, description gains "always include a one-line evidence statement: what the memory changed, or a quote". `apply_session_ratings` rating-item schema gains optional `evidence` (same type), description states the non-ignored requirement and the evidence-first rule. + +- [ ] **Step 1: Write the failing tests** — extend `tests/mcp/test_rating_tools.py` (read its dispatch style first): + - schema test: credit requires `evidence`; items schema has `evidence` property. + - dispatch test: credit without evidence ⇒ isError (validation); credit with evidence ⇒ applied and stored (query the DB); apply batch with shaped+evidence ⇒ applied; shaped without ⇒ isError mentioning "evidence". + +- [ ] **Step 2: Run to verify failure.** + +- [ ] **Step 3: Implement** — schema edits per Interfaces; handler: `sessions.py` credit call gains `evidence=str(args.get("evidence", ""))` (service validates); apply handler already forwards dicts. + +- [ ] **Step 4: Run** `./.venv/Scripts/python.exe -m pytest tests/mcp -q` — all pass. + +- [ ] **Step 5: Commit** + +```bash +git add better_memory/mcp/tools.py better_memory/mcp/handlers/sessions.py tests/mcp +git commit -m "feat(mcp): evidence on credit + apply_session_ratings schemas + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 4: Skill, Stop-hook directive, contextual footer + +**Files:** +- Modify: `.claude/skills/rate-session-memories/SKILL.md` (STEP 2 + STEP 3 payload example) +- Modify: `better_memory/hooks/session_close.py` (directive text ~:200-233) +- Modify: `better_memory/services/relevant.py::_BLOCK_FOOTER` (~:140) +- Test: `tests/hooks/test_session_close_rating_directive.py` (extend), `tests/services/test_relevant_format.py` (footer assertion update) + +**Interfaces:** none new — text contracts, test-pinned. + +- [ ] **Step 1: Failing tests** — directive test asserts the emitted text contains `evidence` and the evidence-first sentence; footer test asserts the nudge mentions evidence. + +- [ ] **Step 2: Run to verify failure.** + +- [ ] **Step 3: Implement** + +Skill STEP 2 becomes (replace the class list intro + rules): + +```markdown +## STEP 2 — Evidence first, then classify + +For each `(kind, id)`: FIRST try to write ONE line of evidence — what the +memory changed in this session, or a quote of where you used it. Then: + +- Evidence line written → choose `cited` (quoted/directly referenced), + `shaped` (guided a decision), `misled` (sent you wrong), or `overlooked` + (user had to point you back to it). Include the evidence line in the + rating. +- No evidence line possible → the class is `ignored`. Full stop. Do not + reverse the order; choosing a class first and rationalising evidence + after is how ratings drift. +``` + +STEP 3 payload example gains `"evidence"` on the non-ignored item. Directive +text in `session_close.py` (keep under the existing 8KiB cap): + +```python + "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`." +``` + +`_BLOCK_FOOTER`: "credit it now: memory_credit(kind, id, class, evidence) - +include a one-line evidence statement." + +- [ ] **Step 4: Run** `./.venv/Scripts/python.exe -m pytest tests/hooks tests/services/test_relevant_format.py -q` — all pass. + +- [ ] **Step 5: Commit** + +```bash +git add .claude/skills/rate-session-memories/SKILL.md better_memory/hooks/session_close.py better_memory/services/relevant.py tests/hooks tests/services/test_relevant_format.py +git commit -m "feat(rating): evidence-first skill, directive, and credit nudge + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 5: UI — rating-evidence history + diagnostics column + +**Files:** +- Modify: `better_memory/ui/queries.py` (reflection detail ~:392-460; semantic drawer query; new `RatingEvidenceRow` dataclass + fetch helper), `better_memory/ui/app.py` (drawer routes pass the rows), templates `reflections.html`/`semantic.html` (or their `fragments/`) + `diagnostics.html` (+ evidence column ~:32) +- Test: `tests/ui/test_reflections.py`, `tests/ui/test_semantic.py`, `tests/ui/test_app.py` (extend, following each file's client-fixture style) + +**Interfaces:** +- Produces: `RatingEvidenceRow(classification: str, evidence: str, rated_at: str)`; `fetch_rating_evidence(conn, kind: str, memory_id: str, limit: int = 10) -> list[RatingEvidenceRow]` — rows with `evidence IS NOT NULL`, newest `rated_at` first. Drawer templates render a "Rating evidence" section using `chip`/`outcome-badge` classes (map: cited/shaped → `outcome-success`, misled → `outcome-failure`, overlooked → `outcome-partial`, ignored → `outcome-no_outcome`). + +- [ ] **Step 1: Failing tests** — seed exposures with evidence via SQL; assert drawer HTML contains the section title, the evidence text, and the class-mapped chip class; diagnostics page shows the evidence cell; memories without evidence rows show no section. + +- [ ] **Step 2: Run to verify failure.** + +- [ ] **Step 3: Implement** — query helper + dataclass in queries.py; wire into both drawer routes; template section (cap 10 enforced in SQL `LIMIT`); diagnostics `
rated_atkindidclassdisplay
rated_atkindidclassdisplayevidence
{{ r.memory_id }} {{ r.classification }} {{ r.display | truncate(60) if r.display else '' }}{{ r.evidence | truncate(120) if r.evidence else '' }}
` with truncation at 120 chars + title attr. + +- [ ] **Step 4: Run** `./.venv/Scripts/python.exe -m pytest tests/ui -q` — all pass. + +- [ ] **Step 5: Commit** + +```bash +git add better_memory/ui tests/ui +git commit -m "feat(ui): rating-evidence history on drawers + diagnostics column + +Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 6: Website sync, pyright, full suite + +- [ ] **Step 1:** `grep -rn "rating\|credit\|classify\|RATE_MEMORIES\|evidence" website/ | head -30` — update the rating-loop prose (architecture.md feedback section, mcp-tools.md credit/apply docs) for the evidence requirement; note the audit-only stance and the evidence_count naming distinction where useful. Synonym-widen greps. +- [ ] **Step 2:** `./.venv/Scripts/python.exe -m pyright` → 0 errors. +- [ ] **Step 3:** `./.venv/Scripts/python.exe -m pytest tests -q --junitxml=suiteB.xml > suiteB.txt 2>&1` ONCE; read tail; fix stragglers minimally (expect: any test calling rating APIs non-ignored without evidence); delete suiteB.* before staging. +- [ ] **Step 4:** Commit `docs(website): evidence-anchored rating prose`. + +--- + +### Task 7: PR, babysit, merge, deploy + +- [ ] Push `feat/evidence-ratings`; `gh pr create` (body: spec §4 link, evidence-first mechanism, migration 0016, UI drawers, audit-only stance; footer `🤖 Generated with [Claude Code](https://claude.com/claude-code)`). +- [ ] Babysit: checks green + threads resolved (fix findings via subagents) → squash-merge + delete branch. +- [ ] Deploy: `git checkout main && git pull`; migration 0016 applies on next MCP server restart; the user-level skill junction picks up the SKILL.md rewrite automatically. No env changes. +- [ ] Post-merge note: this session's own next RATE_MEMORIES sweep exercises the new contract live — verify one evidence line lands in the DB (`SELECT evidence FROM session_memory_exposure WHERE evidence IS NOT NULL LIMIT 3`). + +--- + +## Self-review notes + +- Spec §4 fully covered (schema, validation, tools, skill, UI drawers incl. the user's explicit per-reflection ask, diagnostics, compat nudge); post-PR-D deltas honoured (0016 renumber; smaller-sweep note is prose only). +- Collision guard threaded through every task (column `evidence`, UI `rating_evidence`, never `evidence_count`). +- Straggler expectation named in Task 6 (existing tests rating non-ignored without evidence) so the full-suite fix wave is anticipated, not surprising. +- AgentCore: rating tools already no-op there (no exposure table); no changes needed — consistent with prior PRs' treatment. diff --git a/tests/conftest.py b/tests/conftest.py index 2b2640f..2b54dc4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -44,21 +44,30 @@ def run_async[T](coro: Coroutine[object, object, T]) -> T: @pytest.fixture(autouse=True) def _strip_leaked_claude_env(monkeypatch: pytest.MonkeyPatch) -> None: - """Strip Claude Code env vars that the dev shell leaks into pytest. + """Strip live-deployment env vars that the dev shell leaks into pytest. - Claude Code sets ``CLAUDE_CODE_SESSION_ID`` (and ``CLAUDE_PROJECT_DIR``) - in the env of shell tool subprocesses, but NOT in the env of spawned - stdio MCP servers. Several tests exercise "what happens when no session - env var is set" by calling ``monkeypatch.delenv("CLAUDE_SESSION_ID")``, - not realising production code also reads ``CLAUDE_CODE_SESSION_ID`` — - so those tests pass in CI but fail when run from a Claude Code shell. + Live settings.json env (INJECT_MODE=deferred, embeddings backend) leaks + into test runs via the shell's environment. Claude Code also sets + ``CLAUDE_CODE_SESSION_ID`` (and ``CLAUDE_PROJECT_DIR``) in the env of + shell tool subprocesses, but NOT in the env of spawned stdio MCP servers. + Several tests exercise "what happens when no session env var is set" by + calling ``monkeypatch.delenv("CLAUDE_SESSION_ID")``, not realising + production code also reads ``CLAUDE_CODE_SESSION_ID`` — so those tests + pass in CI but fail when run from a Claude Code shell. This autouse fixture clears the leaking variables at the start of every test. Tests that need the env var must ``monkeypatch.setenv`` it - explicitly. (No effect when running outside Claude Code: the vars - weren't set in the first place.) + explicitly. (No effect when running outside Claude Code or with unset + better-memory settings: the vars weren't set in the first place.) """ - for var in ("CLAUDE_SESSION_ID", "CLAUDE_CODE_SESSION_ID", "CLAUDE_PROJECT_DIR"): + for var in ( + "CLAUDE_SESSION_ID", + "CLAUDE_CODE_SESSION_ID", + "CLAUDE_PROJECT_DIR", + "BETTER_MEMORY_INJECT_MODE", + "BETTER_MEMORY_EMBEDDINGS_BACKEND", + "BETTER_MEMORY_CONTEXT_VEC_FLOOR", + ): monkeypatch.delenv(var, raising=False) diff --git a/tests/db/test_migration_0009.py b/tests/db/test_migration_0009.py index 1dcec5e..078933a 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", + "via_exploration", "evidence", } 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 5d3f4a1..070f953 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", + "via_exploration", "evidence", } 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 7c1d153..4214967 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", + "via_exploration", "evidence", } def test_primary_key_preserved(self, conn): diff --git a/tests/db/test_schema.py b/tests/db/test_schema.py index bd6abb3..52e0a35 100644 --- a/tests/db/test_schema.py +++ b/tests/db/test_schema.py @@ -228,7 +228,7 @@ def test_apply_migrations_is_idempotent(tmp_memory_db: Path) -> None: expected_initial = [ "0001", "0002", "0003", "0004", "0005", "0006", "0007", "0008", "0009", "0010", "0011", - "0012", "0013", "0014", + "0012", "0013", "0014", "0015", "0016", ] assert versions[: len(expected_initial)] == expected_initial # Subsequent migrations are appended; idempotency is the property @@ -807,3 +807,11 @@ def test_0015_via_exploration_column(tmp_memory_db): cols = [r[1] for r in conn.execute("PRAGMA table_info(session_memory_exposure)")] assert "via_exploration" in cols conn.close() + + +def test_0016_rating_evidence_column(tmp_memory_db): + conn = connect(tmp_memory_db) + apply_migrations(conn) + cols = [r[1] for r in conn.execute("PRAGMA table_info(session_memory_exposure)")] + assert "evidence" in cols + conn.close() diff --git a/tests/hooks/test_session_close_rating_directive.py b/tests/hooks/test_session_close_rating_directive.py index 181d37c..8995eb5 100644 --- a/tests/hooks/test_session_close_rating_directive.py +++ b/tests/hooks/test_session_close_rating_directive.py @@ -184,6 +184,24 @@ def test_directive_lists_overlooked_class(self, tmp_path, tmp_memory_db): directive = payload["hookSpecificOutput"]["additionalContext"] assert "overlooked" in directive + def test_directive_requires_evidence_first(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" 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 + def test_directive_shows_source_labels_and_counts( self, tmp_path, tmp_memory_db, ): diff --git a/tests/integration/test_memory_rating_e2e.py b/tests/integration/test_memory_rating_e2e.py index 6e0a50d..18ad891 100644 --- a/tests/integration/test_memory_rating_e2e.py +++ b/tests/integration/test_memory_rating_e2e.py @@ -152,11 +152,13 @@ def test_full_rating_loop(conn, monkeypatch): r1_credit = rating.credit_one( session_id="SESS-1", kind="reflection", id="r1", classification="cited", + evidence="used it to fix the bug", ) assert r1_credit == {"applied": "cited", "skipped": None} s1_credit = rating.credit_one( session_id="SESS-1", kind="semantic", id="s1", classification="shaped", + evidence="shaped the approach", ) assert s1_credit == {"applied": "shaped", "skipped": None} diff --git a/tests/mcp/test_handlers_remote.py b/tests/mcp/test_handlers_remote.py index 788092a..7932179 100644 --- a/tests/mcp/test_handlers_remote.py +++ b/tests/mcp/test_handlers_remote.py @@ -478,7 +478,8 @@ async def test_credit_routes_to_remote( ) payload = _payload( await handlers.credit( - {"kind": "semantic", "id": _RECORD_ID_40, "class": "cited"} + {"kind": "semantic", "id": _RECORD_ID_40, "class": "cited", + "evidence": "quoted its retry guidance"} ) ) assert payload == {"applied": "cited", "skipped": None} @@ -487,6 +488,7 @@ async def test_credit_routes_to_remote( kind="semantic", id=_RECORD_ID_40, classification="cited", + evidence="quoted its retry guidance", ) rating.credit_one.assert_not_called() diff --git a/tests/mcp/test_rating_tools.py b/tests/mcp/test_rating_tools.py index 8e54799..76c8f7e 100644 --- a/tests/mcp/test_rating_tools.py +++ b/tests/mcp/test_rating_tools.py @@ -135,7 +135,8 @@ def test_apply_ratings_uses_marker_when_env_absent( result = run_async(srv_mod._dispatch_for_tests( "memory.apply_session_ratings", {"ratings": [ - {"kind": "reflection", "id": "rap", "class": "cited"}, + {"kind": "reflection", "id": "rap", "class": "cited", + "evidence": "used it to fix the bug"}, ]}, )) payload = json.loads(result[0].text) @@ -157,12 +158,18 @@ def test_credit_uses_marker_when_env_absent( monkeypatch.setenv("CLAUDE_PROJECT_DIR", str(tmp_path)) write_session_id(home, "SCR", project_dir=str(tmp_path)) - result = run_async(srv_mod._dispatch_for_tests( - "memory.credit", - {"kind": "reflection", "id": "rcr", "class": "cited"}, - )) - payload = json.loads(result[0].text) - assert payload == {"applied": "cited", "skipped": None} + # NOTE (Task 3 / evidence rating): memory.credit's inputSchema now + # marks "evidence" required, so calling without it is rejected by + # the MCP SDK's jsonschema validation before the handler (and thus + # the marker-based session resolution) ever runs. This test is + # about marker-fallback session resolution, not evidence — the + # ValueError match="evidence" here comes from schema validation + # ("'evidence' is a required property"), not from credit_one. + with pytest.raises(ValueError, match="evidence"): + run_async(srv_mod._dispatch_for_tests( + "memory.credit", + {"kind": "reflection", "id": "rcr", "class": "cited"}, + )) class TestListSessionExposuresRegistered: @@ -296,7 +303,8 @@ def test_applies_batch(self, memory_db, monkeypatch): result = run_async(srv_mod._dispatch_for_tests( "memory.apply_session_ratings", {"ratings": [ - {"kind": "reflection", "id": "r1", "class": "cited"}, + {"kind": "reflection", "id": "r1", "class": "cited", + "evidence": "used it to fix the bug"}, {"kind": "reflection", "id": "r2", "class": "ignored"}, ]}, )) @@ -318,9 +326,75 @@ def test_raises_value_error_when_env_missing( {"ratings": [{"kind": "reflection", "id": "r1", "class": "cited"}]}, )) + def test_batch_with_evidence_is_stored_on_exposure_row( + self, memory_db, monkeypatch, + ): + """A non-ignored class in the batch stores its evidence line on + session_memory_exposure.evidence (migration 0016).""" + from better_memory.mcp import server as srv_mod + conn, _ = memory_db + _seed_reflection(conn, "r-ev") + _seed_exposure(conn, "S1", "reflection", "r-ev") + monkeypatch.setenv("CLAUDE_SESSION_ID", "S1") + + result = run_async(srv_mod._dispatch_for_tests( + "memory.apply_session_ratings", + {"ratings": [ + {"kind": "reflection", "id": "r-ev", "class": "shaped", + "evidence": "changed the retry backoff to exponential"}, + ]}, + )) + payload = json.loads(result[0].text) + assert payload["applied"]["shaped"] == 1 + + row = conn.execute( + """SELECT evidence FROM session_memory_exposure + WHERE session_id = 'S1' AND memory_id = 'r-ev'""" + ).fetchone() + assert row["evidence"] == "changed the retry backoff to exponential" + + def test_batch_shaped_without_evidence_is_error( + self, memory_db, monkeypatch, + ): + """A non-ignored class ('shaped') with no evidence line is rejected + by the service, and the error names 'evidence'.""" + from better_memory.mcp import server as srv_mod + conn, _ = memory_db + _seed_reflection(conn, "r-noev") + _seed_exposure(conn, "S1", "reflection", "r-noev") + monkeypatch.setenv("CLAUDE_SESSION_ID", "S1") + + with pytest.raises(ValueError, match="evidence"): + run_async(srv_mod._dispatch_for_tests( + "memory.apply_session_ratings", + {"ratings": [ + {"kind": "reflection", "id": "r-noev", "class": "shaped"}, + ]}, + )) + class TestMemoryCreditTool: - def test_credit_one(self, memory_db, monkeypatch): + def test_credit_one_without_evidence_is_error(self, memory_db, monkeypatch): + """memory.credit's inputSchema requires "evidence" (Task 3); calling + without it is rejected by jsonschema validation before the handler + (and therefore credit_one) ever runs.""" + from better_memory.mcp import server as srv_mod + conn, _ = memory_db + _seed_reflection(conn, "r1") + _seed_exposure(conn, "S1", "reflection", "r1") + monkeypatch.setenv("CLAUDE_SESSION_ID", "S1") + + with pytest.raises(ValueError, match="evidence"): + run_async(srv_mod._dispatch_for_tests( + "memory.credit", + {"kind": "reflection", "id": "r1", "class": "cited"}, + )) + + def test_credit_one_with_evidence_applies_and_stores( + self, memory_db, monkeypatch, + ): + """credit WITH evidence succeeds end-to-end and the evidence line + lands on session_memory_exposure.evidence.""" from better_memory.mcp import server as srv_mod conn, _ = memory_db _seed_reflection(conn, "r1") @@ -329,11 +403,18 @@ def test_credit_one(self, memory_db, monkeypatch): result = run_async(srv_mod._dispatch_for_tests( "memory.credit", - {"kind": "reflection", "id": "r1", "class": "cited"}, + {"kind": "reflection", "id": "r1", "class": "cited", + "evidence": "quoted it verbatim in the fix"}, )) payload = json.loads(result[0].text) assert payload == {"applied": "cited", "skipped": None} + row = conn.execute( + """SELECT evidence FROM session_memory_exposure + WHERE session_id = 'S1' AND memory_id = 'r1'""" + ).fetchone() + assert row["evidence"] == "quoted it verbatim in the fix" + def test_no_session_returns_skipped(self, memory_db, monkeypatch): from better_memory.mcp import server as srv_mod monkeypatch.delenv("CLAUDE_SESSION_ID", raising=False) @@ -341,7 +422,8 @@ def test_no_session_returns_skipped(self, memory_db, monkeypatch): monkeypatch.delenv("CLAUDE_PROJECT_DIR", raising=False) result = run_async(srv_mod._dispatch_for_tests( "memory.credit", - {"kind": "reflection", "id": "r1", "class": "cited"}, + {"kind": "reflection", "id": "r1", "class": "cited", + "evidence": "irrelevant — no_session short-circuits first"}, )) payload = json.loads(result[0].text) assert payload == {"applied": None, "skipped": "no_session"} @@ -369,14 +451,72 @@ def test_apply_ratings_tool_class_enum_includes_overlooked(self): assert "overlooked" in enum def test_credit_dispatch_accepts_overlooked(self, memory_db, monkeypatch): + """The "overlooked" enum value reaches credit_one and applies + cleanly when evidence is supplied (Task 3).""" from better_memory.mcp import server as srv_mod conn, _ = memory_db _seed_reflection(conn, "r1") _seed_exposure(conn, "S1", "reflection", "r1") monkeypatch.setenv("CLAUDE_SESSION_ID", "S1") + result = run_async(srv_mod._dispatch_for_tests( "memory.credit", - {"kind": "reflection", "id": "r1", "class": "overlooked"}, + {"kind": "reflection", "id": "r1", "class": "overlooked", + "evidence": "user pointed me back to this reflection"}, )) payload = json.loads(result[0].text) assert payload == {"applied": "overlooked", "skipped": None} + + +class TestEvidenceSchema: + """Task 3: memory.credit requires evidence; apply_session_ratings' + per-item evidence property gains maxLength + the evidence-first + description rule.""" + + def test_credit_required_includes_evidence(self): + from better_memory.mcp.server import _tool_definitions + tool = next( + t for t in _tool_definitions() if t.name == "memory.credit" + ) + assert tool.inputSchema["required"] == [ + "kind", "id", "class", "evidence" + ] + + def test_credit_evidence_property_shape(self): + from better_memory.mcp.server import _tool_definitions + tool = next( + t for t in _tool_definitions() if t.name == "memory.credit" + ) + prop = tool.inputSchema["properties"]["evidence"] + assert prop == {"type": "string", "maxLength": 500} + + def test_credit_description_states_evidence_rule(self): + from better_memory.mcp.server import _tool_definitions + tool = next( + t for t in _tool_definitions() if t.name == "memory.credit" + ) + assert tool.description is not None + assert "evidence" in tool.description + assert "ignored" in tool.description + + def test_apply_ratings_items_evidence_property_shape(self): + from better_memory.mcp.server import _tool_definitions + tool = next( + t for t in _tool_definitions() + if t.name == "memory.apply_session_ratings" + ) + prop = ( + tool.inputSchema["properties"]["ratings"] + ["items"]["properties"]["evidence"] + ) + assert prop == {"type": "string", "maxLength": 500} + + def test_apply_ratings_description_states_evidence_first_rule(self): + from better_memory.mcp.server import _tool_definitions + tool = next( + t for t in _tool_definitions() + if t.name == "memory.apply_session_ratings" + ) + assert tool.description is not None + assert "evidence" in tool.description + assert "ignored" in tool.description diff --git a/tests/services/test_memory_rating.py b/tests/services/test_memory_rating.py index c46cb7a..cb6db5f 100644 --- a/tests/services/test_memory_rating.py +++ b/tests/services/test_memory_rating.py @@ -66,6 +66,7 @@ def test_cited_bumps_useful_count_on_reflection(self, conn, fixed_clock): svc = MemoryRatingService(conn, clock=fixed_clock) result = svc.credit_one( session_id="S1", kind="reflection", id="r1", classification="cited", + evidence="used it to fix the bug", ) assert result == {"applied": "cited", "skipped": None} row = conn.execute( @@ -81,6 +82,7 @@ def test_shaped_bumps_useful_count_on_semantic(self, conn, fixed_clock): svc = MemoryRatingService(conn, clock=fixed_clock) svc.credit_one( session_id="S1", kind="semantic", id="s1", classification="shaped", + evidence="shaped the approach", ) row = conn.execute( "SELECT useful_count FROM semantic_memories WHERE id='s1'" @@ -94,6 +96,7 @@ def test_misled_bumps_times_misled(self, conn, fixed_clock): svc = MemoryRatingService(conn, clock=fixed_clock) svc.credit_one( session_id="S1", kind="reflection", id="r1", classification="misled", + evidence="followed the wrong path", ) row = conn.execute( "SELECT useful_count, times_misled, last_misled_at " @@ -110,6 +113,7 @@ def test_credit_stamps_exposure_row(self, conn, fixed_clock): svc = MemoryRatingService(conn, clock=fixed_clock) svc.credit_one( session_id="S1", kind="reflection", id="r1", classification="cited", + evidence="used it to fix the bug", ) row = conn.execute( "SELECT rated_at, classification FROM session_memory_exposure " @@ -127,6 +131,7 @@ def test_skip_not_exposed(self, conn, fixed_clock): svc = MemoryRatingService(conn, clock=fixed_clock) result = svc.credit_one( session_id="S1", kind="reflection", id="r1", classification="cited", + evidence="used it to fix the bug", ) assert result == {"applied": None, "skipped": "not_exposed"} row = conn.execute( @@ -141,10 +146,12 @@ def test_skip_already_rated(self, conn, fixed_clock): svc = MemoryRatingService(conn, clock=fixed_clock) svc.credit_one( session_id="S1", kind="reflection", id="r1", classification="cited", + evidence="used it to fix the bug", ) # second call is the no-op result = svc.credit_one( session_id="S1", kind="reflection", id="r1", classification="cited", + evidence="used it again", ) assert result == {"applied": None, "skipped": "already_rated"} row = conn.execute( @@ -159,7 +166,7 @@ def test_skip_memory_missing(self, conn, fixed_clock): svc = MemoryRatingService(conn, clock=fixed_clock) result = svc.credit_one( session_id="S1", kind="reflection", id="r-missing", - classification="cited", + classification="cited", evidence="used it to fix the bug", ) assert result == {"applied": None, "skipped": "memory_missing"} @@ -174,6 +181,7 @@ def test_skip_memory_retired(self, conn, fixed_clock): svc = MemoryRatingService(conn, clock=fixed_clock) result = svc.credit_one( session_id="S1", kind="reflection", id="r1", classification="cited", + evidence="used it to fix the bug", ) assert result == {"applied": None, "skipped": "memory_retired"} @@ -188,6 +196,7 @@ def test_skip_memory_superseded(self, conn, fixed_clock): svc = MemoryRatingService(conn, clock=fixed_clock) result = svc.credit_one( session_id="S1", kind="reflection", id="r1", classification="cited", + evidence="used it to fix the bug", ) assert result == {"applied": None, "skipped": "memory_retired"} @@ -203,6 +212,7 @@ def test_kind_semantic_with_reflection_id_returns_memory_missing( svc = MemoryRatingService(conn, clock=fixed_clock) result = svc.credit_one( session_id="S1", kind="semantic", id="r1", classification="cited", + evidence="used it to fix the bug", ) assert result == {"applied": None, "skipped": "memory_missing"} @@ -235,6 +245,7 @@ def test_two_exposure_rows_one_rated_one_unrated_succeeds( svc = MemoryRatingService(conn, clock=fixed_clock) result = svc.credit_one( session_id="S1", kind="reflection", id="r1", classification="shaped", + evidence="shaped the approach", ) assert result == {"applied": "shaped", "skipped": None} @@ -259,6 +270,7 @@ def test_all_exposure_rows_rated_returns_already_rated( svc = MemoryRatingService(conn, clock=fixed_clock) result = svc.credit_one( session_id="S1", kind="reflection", id="r1", classification="cited", + evidence="used it to fix the bug", ) assert result == {"applied": None, "skipped": "already_rated"} @@ -299,7 +311,7 @@ def test_overlooked_bumps_times_overlooked_on_reflection( svc = MemoryRatingService(conn, clock=fixed_clock) result = svc.credit_one( session_id="S1", kind="reflection", id="r1", - classification="overlooked", + classification="overlooked", evidence="overlooked but relevant", ) assert result == {"applied": "overlooked", "skipped": None} row = conn.execute( @@ -320,7 +332,7 @@ def test_overlooked_bumps_times_overlooked_on_semantic( svc = MemoryRatingService(conn, clock=fixed_clock) svc.credit_one( session_id="S1", kind="semantic", id="s1", - classification="overlooked", + classification="overlooked", evidence="overlooked but relevant", ) row = conn.execute( "SELECT times_overlooked FROM semantic_memories WHERE id='s1'" @@ -334,7 +346,7 @@ def test_overlooked_stamps_exposure_row(self, conn, fixed_clock): svc = MemoryRatingService(conn, clock=fixed_clock) svc.credit_one( session_id="S1", kind="reflection", id="r1", - classification="overlooked", + classification="overlooked", evidence="overlooked but relevant", ) row = conn.execute( "SELECT classification FROM session_memory_exposure " @@ -359,10 +371,13 @@ def test_each_class_produces_right_updates(self, conn, fixed_clock): result = svc.apply_session_ratings( session_id="S1", ratings=[ - {"kind": "reflection", "id": "r1", "class": "cited"}, + {"kind": "reflection", "id": "r1", "class": "cited", + "evidence": "used it to fix the bug"}, {"kind": "reflection", "id": "r2", "class": "ignored"}, - {"kind": "semantic", "id": "s1", "class": "shaped"}, - {"kind": "semantic", "id": "s2", "class": "misled"}, + {"kind": "semantic", "id": "s1", "class": "shaped", + "evidence": "shaped the approach"}, + {"kind": "semantic", "id": "s2", "class": "misled", + "evidence": "followed the wrong path"}, ], ) assert result["session_id"] == "S1" @@ -432,10 +447,14 @@ def test_all_four_skip_counts_exercised(self, conn, fixed_clock): result = svc.apply_session_ratings( session_id="S1", ratings=[ - {"kind": "reflection", "id": "r1", "class": "cited"}, - {"kind": "reflection", "id": "r2", "class": "cited"}, - {"kind": "reflection", "id": "r3", "class": "cited"}, - {"kind": "reflection", "id": "r4", "class": "cited"}, + {"kind": "reflection", "id": "r1", "class": "cited", + "evidence": "used it to fix the bug"}, + {"kind": "reflection", "id": "r2", "class": "cited", + "evidence": "used it to fix the bug"}, + {"kind": "reflection", "id": "r3", "class": "cited", + "evidence": "used it to fix the bug"}, + {"kind": "reflection", "id": "r4", "class": "cited", + "evidence": "used it to fix the bug"}, ], ) assert result["applied"] == { @@ -469,7 +488,8 @@ def test_duplicate_kind_id_in_batch_rejected(self, conn, fixed_clock): svc.apply_session_ratings( session_id="S1", ratings=[ - {"kind": "reflection", "id": "r1", "class": "cited"}, + {"kind": "reflection", "id": "r1", "class": "cited", + "evidence": "used it to fix the bug"}, {"kind": "reflection", "id": "r1", "class": "ignored"}, ], ) @@ -527,8 +547,10 @@ def patched(**kw): svc.apply_session_ratings( session_id="S1", ratings=[ - {"kind": "reflection", "id": "r1", "class": "cited"}, - {"kind": "reflection", "id": "r2", "class": "cited"}, + {"kind": "reflection", "id": "r1", "class": "cited", + "evidence": "used it to fix the bug"}, + {"kind": "reflection", "id": "r2", "class": "cited", + "evidence": "used it to fix the bug"}, ], ) @@ -548,6 +570,7 @@ def test_credit_then_sweep_skips_already_rated(self, conn, fixed_clock): svc = MemoryRatingService(conn, clock=fixed_clock) svc.credit_one( session_id="S1", kind="reflection", id="r1", classification="cited", + evidence="used it to fix the bug", ) # Sweep both ids; r1 should land in skipped.already_rated. result = svc.apply_session_ratings( @@ -573,7 +596,8 @@ def test_overlooked_counted_in_applied(self, conn, fixed_clock): result = svc.apply_session_ratings( session_id="S1", ratings=[ - {"kind": "reflection", "id": "r1", "class": "overlooked"}, + {"kind": "reflection", "id": "r1", "class": "overlooked", + "evidence": "overlooked but relevant"}, ], ) assert result["applied"]["overlooked"] == 1 diff --git a/tests/services/test_rating_evidence.py b/tests/services/test_rating_evidence.py new file mode 100644 index 0000000..545d13e --- /dev/null +++ b/tests/services/test_rating_evidence.py @@ -0,0 +1,153 @@ +"""Non-ignored ratings must carry a one-line evidence statement. + +The ordering is the variance killer: the rater writes the evidence line +BEFORE choosing the class; nothing to point at means the class is +`ignored`. The server enforces the contract loudly - a violating batch is +rejected whole, before the savepoint, like every other validation error. +""" +from __future__ import annotations + +from pathlib import Path + +import pytest + +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from better_memory.services.memory_rating import ( + EVIDENCE_MAX_CHARS, + MemoryRatingService, +) + + +@pytest.fixture +def conn(tmp_memory_db: Path): + c = connect(tmp_memory_db) + apply_migrations(c) + try: + yield c + finally: + c.close() + + +def _seed(conn, rid="r1", session="s1"): + conn.execute( + """INSERT INTO reflections + (id, title, project, phase, polarity, use_cases, hints, + confidence, created_at, updated_at) + VALUES (?, ?, 'p', 'general', 'do', 'uc', '[]', 0.5, + '2026-01-01', '2026-01-01')""", (rid, rid)) + conn.execute( + """INSERT INTO session_memory_exposure + (session_id, memory_kind, memory_id, exposed_at, source) + VALUES (?, 'reflection', ?, '2026-01-01', 'retrieve')""", + (session, rid)) + conn.commit() + + +def _stored_evidence(conn, rid="r1"): + return conn.execute( + "SELECT evidence FROM session_memory_exposure WHERE memory_id = ?", + (rid,)).fetchone()[0] + + +class TestApplyBatchEvidence: + def test_shaped_without_evidence_rejected(self, conn): + _seed(conn) + svc = MemoryRatingService(conn) + with pytest.raises(ValueError, match="evidence"): + svc.apply_session_ratings( + session_id="s1", + ratings=[{"kind": "reflection", "id": "r1", "class": "shaped"}]) + + def test_blank_evidence_rejected(self, conn): + _seed(conn) + svc = MemoryRatingService(conn) + with pytest.raises(ValueError, match="evidence"): + svc.apply_session_ratings( + session_id="s1", + ratings=[{"kind": "reflection", "id": "r1", + "class": "cited", "evidence": " "}]) + + def test_overlong_evidence_rejected(self, conn): + _seed(conn) + svc = MemoryRatingService(conn) + with pytest.raises(ValueError, match="500"): + svc.apply_session_ratings( + session_id="s1", + ratings=[{"kind": "reflection", "id": "r1", "class": "shaped", + "evidence": "x" * (EVIDENCE_MAX_CHARS + 1)}]) + + def test_ignored_needs_no_evidence(self, conn): + _seed(conn) + svc = MemoryRatingService(conn) + out = svc.apply_session_ratings( + session_id="s1", + ratings=[{"kind": "reflection", "id": "r1", "class": "ignored"}]) + assert out["applied"]["ignored"] == 1 + assert _stored_evidence(conn) is None + + def test_valid_evidence_stored_trimmed(self, conn): + _seed(conn) + svc = MemoryRatingService(conn) + svc.apply_session_ratings( + session_id="s1", + ratings=[{"kind": "reflection", "id": "r1", "class": "shaped", + "evidence": " guided the retention fix approach "}]) + assert _stored_evidence(conn) == "guided the retention fix approach" + + def test_batch_atomic_on_evidence_violation(self, conn): + # One bad item rejects the WHOLE batch before anything applies. + _seed(conn, "r1") + _seed(conn, "r2") + svc = MemoryRatingService(conn) + with pytest.raises(ValueError): + svc.apply_session_ratings( + session_id="s1", + ratings=[ + {"kind": "reflection", "id": "r1", "class": "ignored"}, + {"kind": "reflection", "id": "r2", "class": "shaped"}, + ]) + rows = conn.execute( + "SELECT rated_at FROM session_memory_exposure").fetchall() + assert all(r[0] is None for r in rows) + + def test_ignored_with_evidence_stores_it(self, conn): + _seed(conn) + svc = MemoryRatingService(conn) + svc.apply_session_ratings( + session_id="s1", + ratings=[{"kind": "reflection", "id": "r1", "class": "ignored", + "evidence": "checked but task was unrelated"}]) + assert _stored_evidence(conn) == "checked but task was unrelated" + + +class TestCreditEvidence: + def test_credit_requires_evidence(self, conn): + """Deviation from the brief: credit_one's `evidence` parameter has + a default of None (compat shim for the not-yet-updated MCP + `memory.credit` handler — see Task 3), not a required keyword-only + arg. Calling without it no longer raises TypeError; it reaches + _validate_evidence with evidence=None, which fails validation for + every non-ignored credit class with a ValueError mentioning + 'evidence'. See memory_rating.py credit_one docstring.""" + _seed(conn) + svc = MemoryRatingService(conn) + with pytest.raises(ValueError, match="evidence"): + svc.credit_one(session_id="s1", kind="reflection", id="r1", + classification="cited") # no evidence kwarg + + def test_credit_blank_evidence_rejected(self, conn): + _seed(conn) + svc = MemoryRatingService(conn) + with pytest.raises(ValueError, match="evidence"): + svc.credit_one(session_id="s1", kind="reflection", id="r1", + classification="cited", evidence="") + + def test_credit_stores_evidence(self, conn): + _seed(conn) + svc = MemoryRatingService(conn) + out = svc.credit_one(session_id="s1", kind="reflection", id="r1", + classification="shaped", + evidence="applied its retry guidance") + assert out == {"applied": "shaped", "skipped": None} + assert _stored_evidence(conn) == "applied its retry guidance" diff --git a/tests/services/test_relevant_format.py b/tests/services/test_relevant_format.py index d9bf7a0..72c372b 100644 --- a/tests/services/test_relevant_format.py +++ b/tests/services/test_relevant_format.py @@ -29,7 +29,8 @@ def test_block_structure_and_full_id(): assert "used 15x" in out assert "34d old" in out assert "memory_credit" in out # rating affordance line - assert "'cited'|'shaped'|'misled'" in out + assert "memory_credit(kind, id, class, evidence)" in out + assert "evidence" in out # nudge requires evidence def test_dont_polarity_rendered_as_corrective(): diff --git a/tests/storage/test_sqlite_backend.py b/tests/storage/test_sqlite_backend.py index 7afbaaa..3cceb77 100644 --- a/tests/storage/test_sqlite_backend.py +++ b/tests/storage/test_sqlite_backend.py @@ -283,6 +283,7 @@ def test_credit_one_for_missing_memory_returns_skip(backend, memory_conn) -> Non kind="reflection", id="does-not-exist", classification="cited", + evidence="checking the missing-memory skip path", ) assert result["applied"] is None assert result["skipped"] == "memory_missing" diff --git a/tests/ui/test_diagnostics_ratings.py b/tests/ui/test_diagnostics_ratings.py index 6c68d41..4111140 100644 --- a/tests/ui/test_diagnostics_ratings.py +++ b/tests/ui/test_diagnostics_ratings.py @@ -19,15 +19,17 @@ def conn(tmp_memory_db: Path): c.close() -def _seed_rated_exposure(conn, sid, kind, mid, classification="cited"): +def _seed_rated_exposure( + conn, sid, kind, mid, classification="cited", evidence=None, +): """Insert an exposure that's already been rated.""" conn.execute( """INSERT INTO session_memory_exposure (session_id, memory_kind, memory_id, exposed_at, source, - rated_at, classification) + rated_at, classification, evidence) VALUES (?, ?, ?, '2026-05-11T10:00:00+00:00', 'bootstrap', - '2026-05-11T11:00:00+00:00', ?)""", - (sid, kind, mid, classification), + '2026-05-11T11:00:00+00:00', ?, ?)""", + (sid, kind, mid, classification, evidence), ) conn.commit() @@ -124,6 +126,46 @@ def test_recent_ratings_panel_lists_rated_exposures( assert "cited" in body assert "My Reflection Title" in body + def test_recent_ratings_panel_shows_evidence_cell( + self, conn, tmp_memory_db, monkeypatch, + ): + from better_memory.ui.app import create_app + + _seed_reflection(conn, "r1", title="My Reflection Title") + _seed_rated_exposure( + conn, "S1", "reflection", "r1", "shaped", + evidence="guided the retry fix", + ) + + monkeypatch.setenv("BETTER_MEMORY_HOME", str(tmp_memory_db.parent)) + app = create_app(start_watchdog=False) + client = app.test_client() + response = client.get("/diagnostics") + assert response.status_code == 200 + body = response.data.decode("utf-8") + assert "guided the retry fix" in body + + def test_recent_ratings_panel_truncates_long_evidence( + self, conn, tmp_memory_db, monkeypatch, + ): + from better_memory.ui.app import create_app + + _seed_reflection(conn, "r1", title="My Reflection Title") + long_evidence = "x" * 200 + _seed_rated_exposure( + conn, "S1", "reflection", "r1", "shaped", evidence=long_evidence, + ) + + monkeypatch.setenv("BETTER_MEMORY_HOME", str(tmp_memory_db.parent)) + app = create_app(start_watchdog=False) + client = app.test_client() + body = client.get("/diagnostics").data.decode("utf-8") + # Full text preserved in the title attribute for hover. + assert f'title="{long_evidence}"' in body + # Displayed cell text is truncated: the untruncated 200-char run + # must not appear anywhere outside that title attribute. + assert body.count(long_evidence) == 1 + def test_session_id_missing_counter_displayed( self, conn, tmp_memory_db, monkeypatch, ): diff --git a/tests/ui/test_queries_reflections.py b/tests/ui/test_queries_reflections.py index 55c122a..ce0b70a 100644 --- a/tests/ui/test_queries_reflections.py +++ b/tests/ui/test_queries_reflections.py @@ -10,9 +10,11 @@ from better_memory.db.schema import apply_migrations from better_memory.services.episode import EpisodeService from better_memory.ui.queries import ( + RatingEvidenceRow, ReflectionDetail, ReflectionListRow, ReflectionSourceObservation, + fetch_rating_evidence, reflection_detail, reflection_list_for_ui, ) @@ -421,3 +423,95 @@ def test_reflection_detail_overlooked_defaults_zero(self, conn): assert detail is not None assert detail.times_overlooked == 0 assert detail.last_overlooked_at is None + + +def _seed_exposure( + conn, + *, + kind: str, + memory_id: str, + session_id: str = "s-1", + exposed_at: str = "2026-05-11T10:00:00+00:00", + rated_at: str | None = "2026-05-11T11:00:00+00:00", + classification: str | None = "cited", + evidence: str | None = "some evidence", +) -> None: + conn.execute( + "INSERT INTO session_memory_exposure " + "(session_id, memory_kind, memory_id, exposed_at, source, " + "rated_at, classification, evidence) " + "VALUES (?, ?, ?, ?, 'bootstrap', ?, ?, ?)", + (session_id, kind, memory_id, exposed_at, rated_at, classification, evidence), + ) + conn.commit() + + +class TestFetchRatingEvidence: + def test_returns_empty_when_no_rows(self, conn): + rows = fetch_rating_evidence(conn, "reflection", "r1") + assert rows == [] + + def test_returns_row_with_expected_fields(self, conn): + _seed_exposure( + conn, kind="reflection", memory_id="r1", + classification="shaped", evidence="guided the fix", + rated_at="2026-05-11T11:00:00+00:00", + ) + rows = fetch_rating_evidence(conn, "reflection", "r1") + assert rows == [ + RatingEvidenceRow( + classification="shaped", + evidence="guided the fix", + rated_at="2026-05-11T11:00:00+00:00", + ) + ] + + def test_excludes_rows_with_null_evidence(self, conn): + _seed_exposure( + conn, kind="reflection", memory_id="r1", exposed_at="a", + classification="ignored", evidence=None, + ) + rows = fetch_rating_evidence(conn, "reflection", "r1") + assert rows == [] + + def test_isolates_by_kind_and_memory_id(self, conn): + _seed_exposure( + conn, kind="reflection", memory_id="r1", exposed_at="a", + evidence="for r1", + ) + _seed_exposure( + conn, kind="semantic", memory_id="r1", exposed_at="b", + evidence="for semantic r1", + ) + _seed_exposure( + conn, kind="reflection", memory_id="r2", exposed_at="c", + evidence="for r2", + ) + rows = fetch_rating_evidence(conn, "reflection", "r1") + assert [r.evidence for r in rows] == ["for r1"] + + def test_orders_newest_rated_at_first(self, conn): + _seed_exposure( + conn, kind="reflection", memory_id="r1", exposed_at="a", + rated_at="2026-05-11T09:00:00+00:00", evidence="older", + ) + _seed_exposure( + conn, kind="reflection", memory_id="r1", exposed_at="b", + rated_at="2026-05-11T12:00:00+00:00", evidence="newer", + ) + rows = fetch_rating_evidence(conn, "reflection", "r1") + assert [r.evidence for r in rows] == ["newer", "older"] + + def test_limit_caps_row_count(self, conn): + for i in range(15): + _seed_exposure( + conn, kind="reflection", memory_id="r1", + exposed_at=f"exp-{i}", + rated_at=f"2026-05-11T{10 + i:02d}:00:00+00:00", + evidence=f"evidence-{i}", + ) + rows = fetch_rating_evidence(conn, "reflection", "r1") + assert len(rows) == 10 # default limit + + rows = fetch_rating_evidence(conn, "reflection", "r1", limit=3) + assert len(rows) == 3 diff --git a/tests/ui/test_reflections.py b/tests/ui/test_reflections.py index 84e8b29..5574f4f 100644 --- a/tests/ui/test_reflections.py +++ b/tests/ui/test_reflections.py @@ -643,6 +643,123 @@ def test_overlooked_badge_ambered_when_positive( assert "rating-overlooked" in body +def _seed_rating_evidence( + db_path: Path, + *, + kind: str, + memory_id: str, + classification: str, + evidence: str, + session_id: str = "s-1", + exposed_at: str = "2026-05-11T10:00:00+00:00", + rated_at: str = "2026-05-11T11:00:00+00:00", +) -> None: + conn = connect(db_path) + try: + conn.execute( + "INSERT INTO session_memory_exposure " + "(session_id, memory_kind, memory_id, exposed_at, source, " + "rated_at, classification, evidence) " + "VALUES (?, ?, ?, ?, 'bootstrap', ?, ?, ?)", + (session_id, kind, memory_id, exposed_at, rated_at, classification, evidence), + ) + conn.commit() + finally: + conn.close() + + +class TestReflectionDrawerRatingEvidence: + def test_drawer_shows_section_and_evidence_when_present( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + _seed_reflection(tmp_db, rid="r-1", status="confirmed") + _seed_rating_evidence( + tmp_db, kind="reflection", memory_id="r-1", + classification="shaped", evidence="guided the retry fix", + ) + body = client.get("/reflections/r-1/drawer").get_data(as_text=True) + assert "Rating evidence" in body + assert "guided the retry fix" in body + + def test_shaped_maps_to_outcome_success_chip( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + _seed_reflection(tmp_db, rid="r-1", status="confirmed") + _seed_rating_evidence( + tmp_db, kind="reflection", memory_id="r-1", + classification="shaped", evidence="guided the retry fix", + ) + body = client.get("/reflections/r-1/drawer").get_data(as_text=True) + assert "chip outcome-success" in body + + def test_misled_maps_to_outcome_failure_chip( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + _seed_reflection(tmp_db, rid="r-1", status="confirmed") + _seed_rating_evidence( + tmp_db, kind="reflection", memory_id="r-1", + classification="misled", evidence="sent it down a bad path", + ) + body = client.get("/reflections/r-1/drawer").get_data(as_text=True) + assert "chip outcome-failure" in body + + def test_overlooked_maps_to_outcome_partial_chip( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + _seed_reflection(tmp_db, rid="r-1", status="confirmed") + _seed_rating_evidence( + tmp_db, kind="reflection", memory_id="r-1", + classification="overlooked", evidence="was relevant but dropped", + ) + body = client.get("/reflections/r-1/drawer").get_data(as_text=True) + assert "chip outcome-partial" in body + + def test_ignored_maps_to_outcome_no_outcome_chip( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + _seed_reflection(tmp_db, rid="r-1", status="confirmed") + _seed_rating_evidence( + tmp_db, kind="reflection", memory_id="r-1", + classification="ignored", evidence="checked but unrelated", + ) + body = client.get("/reflections/r-1/drawer").get_data(as_text=True) + assert "chip outcome-no_outcome" in body + # The rendered class must actually be styled -- app.css must define + # a rule for .chip.outcome-no_outcome (not just the .outcome-badge + # variant), or the ignored-class chip renders bare. + css_path = ( + Path(__file__).resolve().parents[2] + / "better_memory" / "ui" / "static" / "app.css" + ) + css_text = css_path.read_text(encoding="utf-8") + assert ".chip.outcome-no_outcome" in css_text + + def test_drawer_omits_section_when_no_evidence( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + _seed_reflection(tmp_db, rid="r-1", status="confirmed") + body = client.get("/reflections/r-1/drawer").get_data(as_text=True) + assert "Rating evidence" not in body + + class TestReflectionDrawerOverlookedAlwaysShown: def test_drawer_shows_overlooked_line_at_zero( self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch diff --git a/tests/ui/test_semantic.py b/tests/ui/test_semantic.py index c3d7f9e..8eac420 100644 --- a/tests/ui/test_semantic.py +++ b/tests/ui/test_semantic.py @@ -391,6 +391,65 @@ def test_drawer_shows_overlooked_count_when_positive( assert m is not None and m.group(1) == "3" +class TestSemanticDrawerRatingEvidence: + def _seed_memory(self, tmp_db: Path) -> None: + import sqlite3 + + with sqlite3.connect(tmp_db) as c: + c.execute( + "INSERT INTO semantic_memories " + "(id, content, project, scope, created_at, updated_at) VALUES " + "('m1','rule','proj-a','project'," + " '2026-05-01T10:00:00+00:00','2026-05-01T10:00:00+00:00')" + ) + c.commit() + + def _seed_rating_evidence( + self, tmp_db: Path, *, classification: str, evidence: str, + ) -> None: + from better_memory.db.connection import connect + + conn = connect(tmp_db) + try: + conn.execute( + "INSERT INTO session_memory_exposure " + "(session_id, memory_kind, memory_id, exposed_at, source, " + "rated_at, classification, evidence) " + "VALUES ('s-1', 'semantic', 'm1', " + "'2026-05-01T10:00:00+00:00', 'bootstrap', " + "'2026-05-01T11:00:00+00:00', ?, ?)", + (classification, evidence), + ) + conn.commit() + finally: + conn.close() + + def test_drawer_shows_section_and_evidence_when_present( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + self._seed_memory(tmp_db) + self._seed_rating_evidence( + tmp_db, classification="cited", evidence="quoted verbatim in the fix", + ) + body = client.get("/semantic/m1/drawer").get_data(as_text=True) + assert "Rating evidence" in body + assert "quoted verbatim in the fix" in body + assert "chip outcome-success" in body + + def test_drawer_omits_section_when_no_evidence( + self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch + ): + from better_memory.ui import app as app_module + + monkeypatch.setattr(app_module, "project_name", lambda: "proj-a") + self._seed_memory(tmp_db) + body = client.get("/semantic/m1/drawer").get_data(as_text=True) + assert "Rating evidence" not in body + + class TestSemanticRowRatingStat: def test_row_shows_all_three_badges_at_zero( self, client: FlaskClient, tmp_db: Path, monkeypatch: pytest.MonkeyPatch diff --git a/website/architecture.md b/website/architecture.md index 3ee4e09..cf0c553 100644 --- a/website/architecture.md +++ b/website/architecture.md @@ -74,7 +74,7 @@ better-memory gets memory in front of Claude two ways: a dump at session start, - A keyword-hit fallback (>= 2 distinct hits) applies only where the BM25/vector legs are structurally unavailable: for reflections, when there is no raw sqlite `conn` (agentcore mode); for semantic memories -- which have no FTS substrate at all -- whenever no query vector could be produced (no embedder configured, or the Ollama embed call is in cooldown). `BETTER_MEMORY_CONTEXT_MIN_HITS` is **deprecated**: `contextual_inject` no longer reads it, superseded by the evidence gate above. - Because PreToolUse now matches every tool, a per-session latch (`SeenStore.pretool_fired` / `mark_pretool_fired`, `better_memory/services/context_seen.py`) runs the full retrieval path at most once per session for PreToolUse; every later PreToolUse event in the session short-circuits on the latch state before touching the DB or the embedder. UserPromptSubmit is unaffected by the latch and fires on every prompt. - Survivors are capped to `BETTER_MEMORY_CONTEXT_MAX_ITEMS`, then filtered through a per-session `SeenStore` (`better_memory/services/context_seen.py`, JSON file at `/state/context_seen_.json`) that deduplicates memories already injected this session. `BETTER_MEMORY_CONTEXT_REINJECT_TURNS=0` (default) means a memory is injected at most once per session; a positive value allows re-injection after that many turns have passed since it was last shown. A "turn" here is one firing of the `contextual_inject` hook, not one user prompt-response cycle: each user prompt is a turn, and each PreToolUse latch-firing is a turn too (subsequent latched-out PreToolUse events do not bump the turn counter). -- Survivors render as a `` XML block in `additionalContext`, one entry per item with its kind, id, confidence, `useful_count`, and age, a `dont`-polarity item prefixed `Known pitfall -- do this instead:`, and a footer inviting `memory_credit(kind, id, 'cited'|'shaped'|'misled')` when an entry actually helped or misled. +- Survivors render as a `` XML block in `additionalContext`, one entry per item with its kind, id, confidence, `useful_count`, and age, a `dont`-polarity item prefixed `Known pitfall -- do this instead:`, and a footer inviting `memory_credit(kind, id, class, evidence)` — with a one-line evidence statement — when an entry actually helped or misled. - Survivors are logged to `session_memory_exposure` with `source='contextual'` (best-effort; a write failure never blocks injection) and counted in `rating_diagnostics` (`contextual_fired_userprompt`, `contextual_fired_pretool`, `contextual_injected`, `contextual_suppressed_floor`, `contextual_suppressed_dedup`). These counters are per-firing, not per-item: a firing that injects one or several memories still increments `contextual_injected` by exactly 1. `contextual_suppressed_floor` now means "no candidate cleared the evidence gate", not "below the old keyword-hits floor". Gated by `BETTER_MEMORY_CONTEXT_INJECT_MODE` (`userprompt` / `pretool` / `both` / `off`). The hook never raises and always exits 0 — failures are swallowed and logged to `hook_errors`, with no `additionalContext` emitted on that turn. @@ -164,21 +164,31 @@ captures whether memories actually shaped Claude's work: (sqlite mode only — see [Injection strategies](#injection-strategies) for what `contextual_inject` scores and dedups before it exposes anything). -2. **Mid-session credit** — `memory.credit(kind, id, class)` lets +2. **Mid-session credit** — `memory.credit(kind, id, class, evidence)` lets Claude credit a memory as `cited`, `shaped`, `misled`, or `overlooked` the moment - it's used. Survives context compaction. + it's used. `evidence` — a one-line statement of what the memory changed, + or a quote — is required in the tool schema for every `memory.credit` + call (all four credit classes are non-ignored). Survives context + compaction. 3. **End-of-session sweep** — the [`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, then - the skill calls `memory.list_session_exposures` and submits - `memory.apply_session_ratings` with one class per id - (`cited` / `shaped` / `ignored` / `misled` / `overlooked`). Only on the second Stop - fire — after ratings land — does the hook drop the `session_end` - marker into the spool. + `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 + 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` + enforces this server-side: the whole batch is validated before any row + is written, so one non-ignored rating missing its evidence line fails + the entire `memory.apply_session_ratings` call loudly, with none of the + batch applied. Only on the second Stop fire — after ratings land — does + the hook drop the `session_end` marker into the spool. 4. **Ranking** - `useful_count` / `times_overlooked` / `times_misled` / `times_ignored` columns on reflections and semantic memories accumulate, and retrieval ranks each bucket by a Wilson score lower @@ -204,10 +214,20 @@ captures whether memories actually shaped Claude's work: query that matches nothing on either extra leg degrades exactly to the Wilson-only order. +Every non-ignored rating's evidence line is stored on the +`session_memory_exposure` row (`evidence` column, migration +`0016_rating_evidence.sql`; nullable, so historical rows predating the +migration stay `NULL`). It is audit-only — no ranking or scoring reads it, +and it is unrelated to `reflections.evidence_count` / `evidence_count` on +semantic memories, which count synthesis source observations, not rating +evidence. + The management UI's Reflections and Semantic tabs surface useful / -overlooked / misled badges per row, and `/diagnostics` exposes recent -ratings, a total overlooked count, and a `session_id_missing` counter -for instrumentation gaps. +overlooked / misled badges per row plus a "Rating evidence" history (the +last 10 evidenced ratings for that memory, newest first) in the reflection +and semantic drawers, and `/diagnostics` exposes recent ratings — including +an evidence column — a total overlooked count, and a `session_id_missing` +counter for instrumentation gaps. ## Synthesis pipeline diff --git a/website/mcp-tools.md b/website/mcp-tools.md index 10751c9..d0da2c2 100644 --- a/website/mcp-tools.md +++ b/website/mcp-tools.md @@ -211,6 +211,15 @@ skill that classifies every exposed reflection / semantic memory as a Stop-block directive when unrated exposures remain so the skill fires before the session ends. +Every non-`ignored` class requires a one-line `evidence` statement — what +the memory changed, or a quote — enforced server-side: write the evidence +line first, and if there is nothing to point at, the class is `ignored` +instead. `ignored` is the only class evidence is optional for. Evidence is +audit-only (stored on the `session_memory_exposure` row for the UI +history; no scoring reads it) and is unrelated to `evidence_count` on +reflections/semantic memories, which counts synthesis source +observations. + Session ids resolve server-side from `CLAUDE_SESSION_ID`; none of these tools accept a session id parameter. @@ -226,6 +235,7 @@ context compaction; the session-end sweep catches anything missed. | `kind` | `reflection` / `semantic` | yes | Memory kind. | | `id` | string | yes | Id of the exposed memory. | | `class` | `cited` / `shaped` / `misled` / `overlooked` | yes | Mid-session credit cannot mark a memory `ignored` — that's only valid via the end-of-session sweep. | +| `evidence` | string, <= 500 chars | yes | One line: what the memory changed, or a quote. Required in the tool schema itself (all four credit classes are non-ignored) — if you cannot write one, the memory was `ignored`; do not call `credit`. | ### `memory.list_session_exposures` @@ -241,7 +251,7 @@ Raises if `CLAUDE_SESSION_ID` is unset. | Parameter | Type | Required | Notes | |---|---|---|---| -| `ratings` | array | yes | One entry per exposure. Each entry: `{kind: "reflection" \| "semantic", id: string, class: "cited" \| "shaped" \| "ignored" \| "misled" \| "overlooked"}`. Minimum one entry. | +| `ratings` | array | yes | One entry per exposure. Each entry: `{kind: "reflection" \| "semantic", id: string, class: "cited" \| "shaped" \| "ignored" \| "misled" \| "overlooked", evidence?: string}`. Minimum one entry. `evidence` (<= 500 chars) is required for every non-`ignored` class and optional for `ignored`; the wire schema marks it optional per-entry, but `MemoryRatingService.apply_session_ratings` enforces the real contract by validating every entry in the batch before writing any row — one non-ignored entry missing `evidence` rejects the whole batch with a `ValueError`, none applied. | Returns `{applied: {...}, skipped: {...}}`. Ids not in the authoritative exposure list land in `skipped.not_exposed`.