From ca50a86158a9f29ed35c244a8067e550fb7a3cf4 Mon Sep 17 00:00:00 2001 From: gethin Date: Mon, 31 Aug 2026 13:40:18 +0100 Subject: [PATCH 1/7] Add RATE_MEMORIES session-id pass-through plan Three tasks: directive names its session id, rating tools accept an explicit session_id (explicit > env > marker), skill and docs pass it through. Fixes the counter/list mismatch diagnosed 2026-08-31. Co-Authored-By: Claude Fable 5 --- .../2026-08-31-rate-memories-session-id.md | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-31-rate-memories-session-id.md diff --git a/docs/superpowers/plans/2026-08-31-rate-memories-session-id.md b/docs/superpowers/plans/2026-08-31-rate-memories-session-id.md new file mode 100644 index 0000000..3feb372 --- /dev/null +++ b/docs/superpowers/plans/2026-08-31-rate-memories-session-id.md @@ -0,0 +1,73 @@ +# RATE_MEMORIES Session-Id Pass-Through 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:** The RATE_MEMORIES flow becomes deterministic end-to-end: the Stop-hook directive names the session id it counted, and the rating tools accept that id explicitly instead of guessing from env/marker. + +**Architecture:** Three small changes along one data path: `hooks/session_close.py` (directive text), `mcp/tools.py` + `mcp/handlers/sessions.py` (optional `session_id` param, explicit > env > marker), `.claude/skills/rate-session-memories/SKILL.md` (pass it through). Docs ship in the same PR. + +**Tech Stack:** Python 3.12 stdlib; pytest; ruff. + +**Spec:** Root-cause record: observation `0000001788170115314#bf82ddc6` (2026-08-31). Summary: hook-side writers key exposures to the stdin-payload session id; the MCP server resolves env-first from its spawn env (which can hold a different id after an MCP respawn), and the per-project marker file loses last-writer-wins races between concurrent sessions. Result: `list_session_exposures` returns empty while the Stop hook counts N pending, forever. + +## Global Constraints + +- Hooks never raise and never block beyond the existing decision:block contract. +- Backward compatible: both tools keep working with NO `session_id` argument (existing resolution unchanged, fallback path). +- MCP tool COUNT unchanged (schema edits only) — do not touch tool-count claims in docs. +- Lint/test gates per touched dirs: `uv run ruff check ` clean (pre-existing E501 in tests/hooks/test_post_commit.py:169 is not yours); focused pytest per task; full `uv run pytest -q` once before the branch review. +- Commit messages end with: `Co-Authored-By: Claude Fable 5 ` + +--- + +### Task 1: Directive names its session id (confidence 95%) + +**Files:** +- Modify: `better_memory/hooks/session_close.py` (in `_emit_rating_directive_if_unrated`, sections list ~line 111) +- Test: extend `tests/hooks/test_session_close_rating_directive.py` + +**Interfaces:** +- Consumes: existing `_emit_rating_directive_if_unrated(session_id)`. +- Produces: directive `additionalContext` whose SECOND line is exactly `Session: ` (after the `RATE_MEMORIES: N unrated...` header line); the truncation suffix message becomes `(list truncated; call memory.list_session_exposures with this session id for the full set)`. + +- [ ] **Step 1: Failing tests** — extend the existing directive tests: (a) directive contains line `Session: abc-123` when called with that id; (b) the `Session:` line survives truncation (build >8KB of rows, assert the line is still present in the truncated output — it is in the header section before the cap slice). Follow the file's existing seeding/capture style. +- [ ] **Step 2: Run to verify fail** — `uv run pytest tests/hooks/test_session_close_rating_directive.py -q` → new tests FAIL. +- [ ] **Step 3: Implement** — insert `f"Session: {session_id}"` as the second element of `sections` (between the header string and the evidence-first line); update the truncation suffix text. +- [ ] **Step 4: Pass + lint** — same pytest command PASS; `uv run ruff check better_memory/hooks tests/hooks`. +- [ ] **Step 5: Commit** — `fix(hooks): RATE_MEMORIES directive names the session id it counted` + +### Task 2: Rating tools accept explicit session_id (confidence 92%) + +**Files:** +- Modify: `better_memory/mcp/tools.py` (~:459 `memory.list_session_exposures`, ~:474 `memory.apply_session_ratings` inputSchemas) +- Modify: `better_memory/mcp/handlers/sessions.py` (the two handler methods; read their current session-resolution first) +- Test: extend the existing handler tests (locate via `grep -rl list_session_exposures tests/`) + +**Interfaces:** +- Consumes: existing handler resolution (env/marker via `_common`/`session_marker`). +- Produces: both tools accept optional `session_id: string` (schema: `{"type": "string"}`, NOT required). Resolution order in BOTH handlers: explicit argument (non-empty, stripped) > existing env/marker resolution. Empty/whitespace argument = absent. + +- [ ] **Step 1: Failing tests** — for each handler: seed exposures under session `S1`; call handler with `session_id="S1"` while the ambient resolution points elsewhere (monkeypatch env/marker per the existing tests' style) → rows returned / ratings applied; and one test that omitting the param preserves current behavior (regression guard). Follow existing handler-test fixtures. +- [ ] **Step 2: Run to verify fail** — focused pytest on the touched test files → FAIL (unknown argument or empty result). +- [ ] **Step 3: Implement** — schema: add `session_id` property + description ("Explicit session id from the RATE_MEMORIES directive; overrides env/marker resolution"). Handlers: `explicit = str(arguments.get("session_id") or "").strip()` then `session_id = explicit or `. +- [ ] **Step 4: Pass + lint** — focused pytest PASS; `uv run ruff check better_memory/mcp tests`. +- [ ] **Step 5: Commit** — `fix(mcp): list/apply session-rating tools accept explicit session_id` + +### Task 3: Skill pass-through + docs sweep (confidence 93%) + +**Files:** +- Modify: `.claude/skills/rate-session-memories/SKILL.md` (repo copy — user-scope is a link to it) +- Modify: `website/mcp-tools.md` (the two tools' schema sections), `README.md` ONLY if it details these two tools' parameters (grep first; tool count untouched) + +**Interfaces:** none (prose), but every parameter name copied verbatim from the Task 2 schemas in `mcp/tools.py`. + +- [ ] **Step 1: Skill update** — STEP 1: "Read the `Session:` line from the RATE_MEMORIES directive; call `memory.list_session_exposures` with `{"session_id": ""}`. If the directive has no Session line (older hook), call with no arguments as before." STEP 3: apply call includes the same `session_id`. Keep the anti-hallucination rule (authoritative list) and evidence-first rules intact. +- [ ] **Step 2: Docs** — update the two tools' parameter tables/schemas in `website/mcp-tools.md`, copying names from `mcp/tools.py`. Verify README mentions (grep `list_session_exposures` in README.md) and update only if parameters are shown. +- [ ] **Step 3: Gates + full suite** — `uv run pytest -q` full run green (exact counts in report); `uv run ruff check better_memory` clean for touched dirs. +- [ ] **Step 4: Commit** — `docs(skill): rate-session-memories passes the directive session id` + +## Self-review record + +- Coverage: root cause has three legs (env drift, marker race, no explicit channel); the fix adds the explicit channel end-to-end (T1 producer → T2 consumer → T3 instructions) and leaves fallbacks untouched (constraint). Docs-sync guardrail satisfied by T3. +- Placeholders: none — exact line texts, resolution order, and schema shapes specified; handler internals deliberately referenced by grep because current signatures are read at implementation time (both tasks instruct reading first). +- Type consistency: `session_id` string param named identically in schema, handler, skill, and directive line. From 16e2f57159706f1b96bde770f53152a2010eb11b Mon Sep 17 00:00:00 2001 From: gethin Date: Mon, 31 Aug 2026 13:44:15 +0100 Subject: [PATCH 2/7] fix(hooks): RATE_MEMORIES directive names the session id it counted Co-Authored-By: Claude Fable 5 --- better_memory/hooks/session_close.py | 3 +- .../test_session_close_rating_directive.py | 70 +++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/better_memory/hooks/session_close.py b/better_memory/hooks/session_close.py index 2b4dcd4..600fb8f 100644 --- a/better_memory/hooks/session_close.py +++ b/better_memory/hooks/session_close.py @@ -111,6 +111,7 @@ def _emit_rating_directive_if_unrated(session_id: str) -> bool: sections = [ f"RATE_MEMORIES: {len(rows)} unrated. " "Invoke skill `rate-session-memories`.", + f"Session: {session_id}", "Evidence line first; none possible = `ignored`.", ] if refl_lines: @@ -124,7 +125,7 @@ def _emit_rating_directive_if_unrated(session_id: str) -> bool: if len(encoded) > CAP_BYTES: directive = encoded[: CAP_BYTES - 200].decode("utf-8", errors="ignore") + ( "\n\n(list truncated; call memory.list_session_exposures " - "for the full set)" + "with this session id for the full set)" ) payload = { diff --git a/tests/hooks/test_session_close_rating_directive.py b/tests/hooks/test_session_close_rating_directive.py index 9503c0b..f5b1c9a 100644 --- a/tests/hooks/test_session_close_rating_directive.py +++ b/tests/hooks/test_session_close_rating_directive.py @@ -43,6 +43,34 @@ def _seed_unrated_exposure(db_path: Path, sid: str = "S1", source: str = "bootst c.close() +def _seed_many_unrated(db_path: Path, sid: str, count: int): + """Seed ``count`` distinct reflections + unrated exposures for ``sid``, + each with a long title so the rendered directive comfortably exceeds + the hook's 8KB truncation cap.""" + c = connect(db_path) + apply_migrations(c) + long_title = "T" * 80 + for i in range(count): + rid = f"r{i}" + c.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, long_title), + ) + c.execute( + """INSERT INTO session_memory_exposure + (session_id, memory_kind, memory_id, exposed_at, source) + VALUES (?, 'reflection', ?, '2026-05-11T11:00:00+00:00', + 'bootstrap')""", + (sid, rid), + ) + c.commit() + c.close() + + def _seed_semantic_exposure(db_path: Path, sid: str = "S1", source: str = "contextual"): """Seed a semantic-memory unrated exposure. Assumes migrations already applied (call after _seed_unrated_exposure, which applies them).""" @@ -298,3 +326,45 @@ def test_empty_reflection_bucket_omitted(self, tmp_path, tmp_memory_db): directive = payload["hookSpecificOutput"]["additionalContext"] assert "Reflections (" not in directive assert "My Semantic Fact" in directive + + def test_directive_names_the_session_id_it_counted( + self, tmp_path, tmp_memory_db, + ): + """The MCP rating tools can't reliably re-derive the session id the + Stop hook counted against (env drift after MCP respawn + marker + races), so the directive must name it explicitly, as the second + line, right after the RATE_MEMORIES header.""" + _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"] + lines = directive.split("\n") + assert lines[0].startswith("RATE_MEMORIES:") + assert lines[1] == "Session: S1" + + def test_session_line_survives_truncation(self, tmp_path, tmp_memory_db): + """Seed enough unrated rows to blow past the 8KB truncation cap and + confirm the ``Session:`` line (which precedes the capped slice) + survives, and that the truncation message points the LLM at + list_session_exposures with the id it should use.""" + _seed_many_unrated(tmp_memory_db, "S1", 200) + 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"] + # Confirm truncation actually happened so this test is meaningful. + assert "list truncated" in directive + assert "Session: S1" in directive.split("\n") + assert ( + "(list truncated; call memory.list_session_exposures " + "with this session id for the full set)" + ) in directive From 0cb6a0ec980e2f23b81f70c493d6f66a45019d13 Mon Sep 17 00:00:00 2001 From: gethin Date: Mon, 31 Aug 2026 13:57:08 +0100 Subject: [PATCH 3/7] fix(mcp): rating tools accept explicit session_id from the directive memory.list_session_exposures and memory.apply_session_ratings now accept an optional session_id argument that overrides env/marker resolution when non-empty (whitespace-only values fall back to the existing resolution). This lets the RATE_MEMORIES directive pass the session id it already resolved instead of relying on the handler re-deriving it independently. Co-Authored-By: Claude Fable 5 --- better_memory/mcp/handlers/sessions.py | 6 +- better_memory/mcp/tools.py | 39 +++++-- tests/mcp/test_handlers_remote.py | 38 +++++++ tests/mcp/test_rating_tools.py | 144 ++++++++++++++++++++++++- 4 files changed, 213 insertions(+), 14 deletions(-) diff --git a/better_memory/mcp/handlers/sessions.py b/better_memory/mcp/handlers/sessions.py index bc39462..e8b6a2b 100644 --- a/better_memory/mcp/handlers/sessions.py +++ b/better_memory/mcp/handlers/sessions.py @@ -102,7 +102,8 @@ async def session_bootstrap(self, args: dict[str, Any]) -> list[TextContent]: async def list_session_exposures( self, args: dict[str, Any] ) -> list[TextContent]: - sid = resolve_session_id(self._home) or "" + explicit = str(args.get("session_id") or "").strip() + sid = explicit or resolve_session_id(self._home) or "" if self._remote is not None: payload = self._remote.list_session_exposures(session_id=sid) return [TextContent(type="text", text=json.dumps(payload))] @@ -114,7 +115,8 @@ async def list_session_exposures( async def apply_session_ratings( self, args: dict[str, Any] ) -> list[TextContent]: - sid = resolve_session_id(self._home) + explicit = str(args.get("session_id") or "").strip() + sid = explicit or resolve_session_id(self._home) if not sid: raise ValueError( "No active session: CLAUDE_SESSION_ID / " diff --git a/better_memory/mcp/tools.py b/better_memory/mcp/tools.py index 076077c..90a48b9 100644 --- a/better_memory/mcp/tools.py +++ b/better_memory/mcp/tools.py @@ -460,33 +460,50 @@ def tool_definitions( description=( "Return the unrated session_memory_exposure rows for the " "current Claude session (resolved server-side from " - "CLAUDE_SESSION_ID env). Read-only; no side effects. " - "Used by the rate-session-memories skill as the " - "authoritative anti-hallucination list." + "CLAUDE_SESSION_ID env, or from an explicit session_id " + "argument). Read-only; no side effects. Used by the " + "rate-session-memories skill as the authoritative " + "anti-hallucination list." ), inputSchema={ "type": "object", "additionalProperties": False, - "properties": {}, + "properties": { + "session_id": { + "type": "string", + "description": ( + "Explicit session id from the RATE_MEMORIES " + "directive; overrides env/marker resolution." + ), + }, + }, }, ), Tool( name="memory.apply_session_ratings", description=( "Atomic batch rating for the current Claude session " - "(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. 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`." + "(resolved server-side from CLAUDE_SESSION_ID, or from an " + "explicit session_id argument). Called at session end by " + "the rate-session-memories skill. Raises if no session " + "can be resolved — call only inside 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", "additionalProperties": False, "required": ["ratings"], "properties": { + "session_id": { + "type": "string", + "description": ( + "Explicit session id from the RATE_MEMORIES " + "directive; overrides env/marker resolution." + ), + }, "ratings": { "type": "array", "minItems": 1, diff --git a/tests/mcp/test_handlers_remote.py b/tests/mcp/test_handlers_remote.py index 27194d7..6af0ce9 100644 --- a/tests/mcp/test_handlers_remote.py +++ b/tests/mcp/test_handlers_remote.py @@ -454,6 +454,44 @@ async def test_apply_session_ratings_routes_to_remote( ) rating.apply_session_ratings.assert_not_called() + async def test_list_session_exposures_explicit_session_id_wins_over_env( + self, tmp_path: Path, remote, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Task 2: an explicit session_id argument overrides ambient env + resolution on the remote branch too — same sid variable feeds + both self._remote and the local service path.""" + monkeypatch.setenv("CLAUDE_SESSION_ID", "sid-env-1") + svc = _bootstrap_service() + handlers = SessionToolHandlers( + session_bootstrap=svc, + memory_rating=MagicMock(), + home=tmp_path, + remote=remote, + ) + await handlers.list_session_exposures({"session_id": "sid-explicit"}) + remote.list_session_exposures.assert_called_once_with( + session_id="sid-explicit" + ) + + async def test_apply_session_ratings_explicit_session_id_wins_over_env( + self, tmp_path: Path, remote, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("CLAUDE_SESSION_ID", "sid-env-1") + rating = MagicMock(name="MemoryRatingService") + handlers = SessionToolHandlers( + session_bootstrap=_bootstrap_service(), + memory_rating=rating, + home=tmp_path, + remote=remote, + ) + ratings = [{"kind": "reflection", "id": _RECORD_ID_40, "class": "cited"}] + await handlers.apply_session_ratings( + {"session_id": "sid-explicit", "ratings": ratings} + ) + remote.apply_session_ratings.assert_called_once_with( + session_id="sid-explicit", ratings=ratings + ) + async def test_apply_session_ratings_keeps_no_session_guard_with_remote( self, tmp_path: Path, remote ) -> None: diff --git a/tests/mcp/test_rating_tools.py b/tests/mcp/test_rating_tools.py index 76c8f7e..41be2ce 100644 --- a/tests/mcp/test_rating_tools.py +++ b/tests/mcp/test_rating_tools.py @@ -185,7 +185,15 @@ def test_tool_schema_has_no_required_properties(self): if t.name == "memory.list_session_exposures" ) assert tool.inputSchema["additionalProperties"] is False - assert tool.inputSchema["properties"] == {} + assert tool.inputSchema["properties"] == { + "session_id": { + "type": "string", + "description": ( + "Explicit session id from the RATE_MEMORIES " + "directive; overrides env/marker resolution." + ), + }, + } assert "required" not in tool.inputSchema @@ -520,3 +528,137 @@ def test_apply_ratings_description_states_evidence_first_rule(self): assert tool.description is not None assert "evidence" in tool.description assert "ignored" in tool.description + + +class TestApplyRatingsSessionIdSchema: + """Task 2: memory.apply_session_ratings gains an optional explicit + session_id, mirroring memory.list_session_exposures.""" + + def test_schema_has_session_id_property(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.inputSchema["properties"]["session_id"] == { + "type": "string", + "description": ( + "Explicit session id from the RATE_MEMORIES directive; " + "overrides env/marker resolution." + ), + } + # session_id must stay optional — required is unchanged. + assert tool.inputSchema["required"] == ["ratings"] + + def test_description_mentions_explicit_session_id(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 "session_id" in tool.description + + +class TestListExposuresSessionIdDescription: + def test_description_mentions_explicit_session_id(self): + from better_memory.mcp.server import _tool_definitions + tool = next( + t for t in _tool_definitions() + if t.name == "memory.list_session_exposures" + ) + assert "session_id" in tool.description + + +class TestExplicitSessionIdOverridesAmbientResolution: + """Task 2: both rating tools accept an explicit session_id argument + (from the RATE_MEMORIES directive) that overrides env/marker + resolution. Whitespace-only values are treated as absent.""" + + def test_list_exposures_explicit_session_id_overrides_env( + self, memory_db, monkeypatch, + ): + from better_memory.mcp import server as srv_mod + + conn, _ = memory_db + _seed_reflection(conn, "r-s1") + _seed_exposure(conn, "S1", "reflection", "r-s1") + # Ambient env resolution points elsewhere. + _seed_reflection(conn, "r-other") + _seed_exposure(conn, "OTHER", "reflection", "r-other") + monkeypatch.setenv("CLAUDE_SESSION_ID", "OTHER") + + result = run_async(srv_mod._dispatch_for_tests( + "memory.list_session_exposures", {"session_id": "S1"}, + )) + payload = json.loads(result[0].text) + assert payload["session_id"] == "S1" + assert len(payload["exposures"]) == 1 + assert payload["exposures"][0]["id"] == "r-s1" + + def test_list_exposures_whitespace_session_id_falls_back_to_ambient( + self, memory_db, monkeypatch, + ): + from better_memory.mcp import server as srv_mod + + conn, _ = memory_db + _seed_reflection(conn, "r-amb") + _seed_exposure(conn, "AMBIENT", "reflection", "r-amb") + monkeypatch.setenv("CLAUDE_SESSION_ID", "AMBIENT") + + result = run_async(srv_mod._dispatch_for_tests( + "memory.list_session_exposures", {"session_id": " "}, + )) + payload = json.loads(result[0].text) + assert payload["session_id"] == "AMBIENT" + assert len(payload["exposures"]) == 1 + assert payload["exposures"][0]["id"] == "r-amb" + + def test_apply_ratings_explicit_session_id_overrides_env( + self, memory_db, monkeypatch, + ): + from better_memory.mcp import server as srv_mod + + conn, _ = memory_db + _seed_reflection(conn, "r-s1") + _seed_exposure(conn, "S1", "reflection", "r-s1") + _seed_reflection(conn, "r-other") + _seed_exposure(conn, "OTHER", "reflection", "r-other") + monkeypatch.setenv("CLAUDE_SESSION_ID", "OTHER") + + result = run_async(srv_mod._dispatch_for_tests( + "memory.apply_session_ratings", + { + "session_id": "S1", + "ratings": [ + {"kind": "reflection", "id": "r-s1", "class": "cited", + "evidence": "used it to fix the bug"}, + ], + }, + )) + payload = json.loads(result[0].text) + assert payload["session_id"] == "S1" + assert payload["applied"]["cited"] == 1 + + def test_apply_ratings_whitespace_session_id_falls_back_to_ambient( + self, memory_db, monkeypatch, + ): + from better_memory.mcp import server as srv_mod + + conn, _ = memory_db + _seed_reflection(conn, "r-amb") + _seed_exposure(conn, "AMBIENT", "reflection", "r-amb") + monkeypatch.setenv("CLAUDE_SESSION_ID", "AMBIENT") + + result = run_async(srv_mod._dispatch_for_tests( + "memory.apply_session_ratings", + { + "session_id": " ", + "ratings": [ + {"kind": "reflection", "id": "r-amb", "class": "cited", + "evidence": "used it to fix the bug"}, + ], + }, + )) + payload = json.loads(result[0].text) + assert payload["session_id"] == "AMBIENT" + assert payload["applied"]["cited"] == 1 From c5d3f104499353d22f1e6a587ff615a3991f23c8 Mon Sep 17 00:00:00 2001 From: gethin Date: Mon, 31 Aug 2026 14:02:11 +0100 Subject: [PATCH 4/7] fix(mcp): narrow description asserts for pyright, rename schema test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tool.description is typed str | None, so a bare `"session_id" in tool.description` trips pyright's reportOperatorIssue (typecheck.yml runs pyright on tests/**). Add the `is not None` guard already used elsewhere in this file before the two new session_id description checks. Also rename test_tool_schema_has_no_required_properties to test_tool_schema_shape_and_optional_session_id — it now asserts the session_id property's shape too, not just the absence of required fields. Co-Authored-By: Claude Fable 5 --- tests/mcp/test_rating_tools.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/mcp/test_rating_tools.py b/tests/mcp/test_rating_tools.py index 41be2ce..b1cd444 100644 --- a/tests/mcp/test_rating_tools.py +++ b/tests/mcp/test_rating_tools.py @@ -178,7 +178,7 @@ def test_tool_is_in_definitions(self): names = {t.name for t in _tool_definitions()} assert "memory.list_session_exposures" in names - def test_tool_schema_has_no_required_properties(self): + def test_tool_schema_shape_and_optional_session_id(self): from better_memory.mcp.server import _tool_definitions tool = next( t for t in _tool_definitions() @@ -556,6 +556,7 @@ def test_description_mentions_explicit_session_id(self): t for t in _tool_definitions() if t.name == "memory.apply_session_ratings" ) + assert tool.description is not None assert "session_id" in tool.description @@ -566,6 +567,7 @@ def test_description_mentions_explicit_session_id(self): t for t in _tool_definitions() if t.name == "memory.list_session_exposures" ) + assert tool.description is not None assert "session_id" in tool.description From 2579db15f02b197a8f8e323a62f3c6c443403f89 Mon Sep 17 00:00:00 2001 From: gethin Date: Mon, 31 Aug 2026 14:20:56 +0100 Subject: [PATCH 5/7] docs(skill): rate-session-memories passes the directive session id Co-Authored-By: Claude Fable 5 --- .claude/skills/rate-session-memories/SKILL.md | 17 ++++++++++++----- README.md | 6 +++--- website/mcp-tools.md | 18 +++++++++++++----- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/.claude/skills/rate-session-memories/SKILL.md b/.claude/skills/rate-session-memories/SKILL.md index da82e8a..069782a 100644 --- a/.claude/skills/rate-session-memories/SKILL.md +++ b/.claude/skills/rate-session-memories/SKILL.md @@ -10,9 +10,14 @@ already been credited via `memory.credit` mid-session. ## STEP 1 — Refresh the list -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. +Read the `Session:` line from the RATE_MEMORIES directive (its second line, +e.g. `Session: `). If present, call `memory.list_session_exposures` with +`{"session_id": ""}`. If the directive has no `Session:` line (older +hook), call `memory.list_session_exposures` with no arguments — the server +resolves the current session from env, as before. + +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 — Evidence first, then classify @@ -42,11 +47,13 @@ Rules: ## STEP 3 — Submit ALL ratings in ONE call -Call `memory.apply_session_ratings` with this exact shape (no `session_id` -— the server resolves it): +Call `memory.apply_session_ratings` with the same `session_id` from STEP 1 +alongside `ratings` (omit `session_id` if the directive had no `Session:` +line — the server then resolves it as before): ```json { + "session_id": "", "ratings": [ {"kind": "reflection", "id": "r-abc...", "class": "cited", "evidence": "Used its junit-xml flag verbatim in the pytest invocation."}, diff --git a/README.md b/README.md index fef1fb3..858a003 100644 --- a/README.md +++ b/README.md @@ -203,13 +203,13 @@ The server registers 22 tools, grouped below. Full schemas are in [`website/mcp- | `memory.synthesize_next_get_context(project?)` | One pending episode's full context for the IDE-LLM to act on. | | `memory.synthesize_next_apply(episode_id, decision, project?)` | Atomically apply Claude's `new` / `augment` / `merge` / `ignore` decision. | -**Rating** — closed-loop reinforcement on top of `memory.record_use`. Session id is resolved server-side from `CLAUDE_SESSION_ID`; none of these tools take a session id. +**Rating** — closed-loop reinforcement on top of `memory.record_use`. Session id is resolved server-side from `CLAUDE_SESSION_ID` by default; `list_session_exposures` and `apply_session_ratings` also accept an explicit `session_id` (from the RATE_MEMORIES directive's `Session:` line), which overrides env/marker resolution. `memory.credit` takes no session id. | Tool | Purpose | |---|---| | `memory.credit(kind, id, class, evidence)` | Opportunistic per-tool-use credit. `class` ∈ `cited` / `shaped` / `misled` / `overlooked` (not `ignored`). `evidence` is required: one line — what the memory changed, or a quote. If you can't write one, the memory was ignored; don't call credit. | -| `memory.list_session_exposures()` | Unrated exposure rows for the current session. Read-only; used by the `rate-session-memories` skill. | -| `memory.apply_session_ratings(ratings)` | Atomic end-of-session batch rating. Each entry: `{kind, id, class, evidence?}` — evidence is required for every non-`ignored` class (write the evidence line first; nothing to point at means the class is `ignored`). Violating batches are rejected whole. | +| `memory.list_session_exposures(session_id?)` | Unrated exposure rows for the current session. Read-only; used by the `rate-session-memories` skill. | +| `memory.apply_session_ratings(ratings, session_id?)` | Atomic end-of-session batch rating. Each entry: `{kind, id, class, evidence?}` — evidence is required for every non-`ignored` class (write the evidence line first; nothing to point at means the class is `ignored`). Violating batches are rejected whole. | **Knowledge** — human-authored markdown corpus. diff --git a/website/mcp-tools.md b/website/mcp-tools.md index d1c6ce2..8c2a0c3 100644 --- a/website/mcp-tools.md +++ b/website/mcp-tools.md @@ -242,17 +242,25 @@ context compaction; the session-end sweep catches anything missed. ### `memory.list_session_exposures` Return the unrated `session_memory_exposure` rows for the current Claude -session. Read-only; no side effects. Used by the `rate-session-memories` -skill as the authoritative anti-hallucination list. No parameters. +session (resolved server-side from `CLAUDE_SESSION_ID` env, or from an +explicit `session_id` argument). Read-only; no side effects. Used by the +`rate-session-memories` skill as the authoritative anti-hallucination list. + +| Parameter | Type | Required | Notes | +|---|---|---|---| +| `session_id` | string | no | Explicit session id from the RATE_MEMORIES directive's `Session:` line; overrides env/marker resolution. | ### `memory.apply_session_ratings` -Atomic batch rating for the current Claude session. Called by the -`rate-session-memories` skill at session end after `memory.list_session_exposures`. -Raises if `CLAUDE_SESSION_ID` is unset. +Atomic batch rating for the current Claude session (resolved server-side +from `CLAUDE_SESSION_ID`, or from an explicit `session_id` argument). Called +by the `rate-session-memories` skill at session end after +`memory.list_session_exposures`. Raises if no session can be resolved — +call only inside an active Claude session. | Parameter | Type | Required | Notes | |---|---|---|---| +| `session_id` | string | no | Explicit session id from the RATE_MEMORIES directive's `Session:` line; overrides env/marker resolution. | | `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 From 60e51d73875344a915193aab6e570e861dbf9b4a Mon Sep 17 00:00:00 2001 From: gethin Date: Mon, 31 Aug 2026 14:24:01 +0100 Subject: [PATCH 6/7] docs: fix stale rating-tools intro contradicting session_id tables Co-Authored-By: Claude Fable 5 --- website/mcp-tools.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/website/mcp-tools.md b/website/mcp-tools.md index 8c2a0c3..1338832 100644 --- a/website/mcp-tools.md +++ b/website/mcp-tools.md @@ -222,8 +222,11 @@ 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. +Session ids resolve server-side from `CLAUDE_SESSION_ID` by default; +`memory.list_session_exposures` and `memory.apply_session_ratings` also +accept an explicit `session_id` (from the RATE_MEMORIES directive's +`Session:` line), which overrides env/marker resolution. `memory.credit` +takes no session id. ### `memory.credit` From ad44deb3d72a095885faa8f5ae8bac01790ed8a0 Mon Sep 17 00:00:00 2001 From: gethin Date: Mon, 31 Aug 2026 14:30:25 +0100 Subject: [PATCH 7/7] docs: architecture.md reflects Session line in RATE_MEMORIES directive Co-Authored-By: Claude Fable 5 --- website/architecture.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/architecture.md b/website/architecture.md index a5bce67..25c128e 100644 --- a/website/architecture.md +++ b/website/architecture.md @@ -214,14 +214,14 @@ described below: hook checks for unrated exposures. If any exist, it emits a `Stop` block directive triggering the `rate-session-memories` skill. The directive opens with a `RATE_MEMORIES: {n} unrated. Invoke skill - rate-session-memories.` header line, followed by an `Evidence line + rate-session-memories.` header line, followed by a `Session: {session_id}` line, then an `Evidence line first; none possible = ignored.` rule line, then pending exposures grouped by kind (`Reflections (N):` / `Semantic (N):`), one line per exposure — `- {id} [{source}] {display}`, `display` truncated to 80 chars and read via `COALESCE(e.display, r.title, s.content)` off the `session_memory_exposure` row (falling back to the live title/content only when no display snapshot was captured at exposure time). The - skill then calls `memory.list_session_exposures` and + skill then calls `memory.list_session_exposures` (passing the directive's session id as the explicit `session_id` argument) 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`