diff --git a/CHANGELOG.md b/CHANGELOG.md index cf380d7..7371e67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,21 @@ tell you. ## [Unreleased] +### Added + +- **`recall --policy`** decides which of two disagreeing hits comes first: + `latest-user` (the user's own words newest first, then the rest), `latest` + (newest first, whoever said it), `relevance` (best match first), or your own + `module:function`. Also settable with `GITMEMORY_POLICY`. Nothing is deleted. + See [USAGE.md](docs/USAGE.md#when-memories-disagree). +- `recall` prints each hit's date, and `Hit` carries the turn's `ts`. + +### Changed + +- `recall` now lists the user's own words newest first by default, not best + match first. Pass `--policy relevance` for the old order. `index.search` is + unchanged. + ## [0.1.0] - 2026-09-28 The first public release. What it contains: diff --git a/README.md b/README.md index 2fe4d61..d21f3a9 100644 --- a/README.md +++ b/README.md @@ -97,7 +97,7 @@ agent after compaction, derived key ideas, the dashboard — is in | `gitmemory capture ` | Copies out one transcript's new bytes, by hand | | `gitmemory verify` | Checks every manifest's contiguity proof | | `gitmemory index` | Rebuilds the SQLite FTS5 index from the store | -| `gitmemory recall ""` | Searches the index and prints one line per turn, best first | +| `gitmemory recall ""` | Searches the index and prints one line per turn, dated, the user's own words newest first ([`--policy`](docs/USAGE.md#when-memories-disagree)) | | `gitmemory derive [--graph]` | Rebuilds key ideas and a timeline; `--graph` adds the decision graph (opt-in) | | `gitmemory dashboard` | Serves the index with Datasette, read-only, on loopback, behind a sign-in | | `gitmemory push` | Runs the redaction gate over what a push would send (it does not send yet) | @@ -183,13 +183,13 @@ real text, and that is a measurement rather than a suspicion. | What | How it was measured | Result | |---|---|---| | Hook cost in the agent's critical path | Timed against spawning `true` the same way, three runs of 400 | p50 **7.4 – 7.5 ms**, p99 **10.2 – 11.5 ms** | -| The suite | On a fresh checkout, no downloads | **1,026 tests**, and **328 conformance cases** against three third-party corpora, one gated on the LongMemEval download and one on `pip install -e '.[serve]'` | +| The suite | On a fresh checkout, no downloads | **1,036 tests**, and **328 conformance cases** against three third-party corpora, one gated on the LongMemEval download and one on `pip install -e '.[serve]'` | | Whether the tests hold anything | Every fix mutated to remove its behaviour; the named test must fail | **606** negative controls | The two suite counts do not add up, and should not. Switching the corpora on -collects 1351, not 1354. Three conformance cases fill parametrisations that +collects 1361, not 1364. Three conformance cases fill parametrisations that collect as one empty placeholder each while the corpora are absent, so they -replace three of the 1026 rather than joining them. Every figure on this board +replace three of the 1036 rather than joining them. Every figure on this board is pinned by a test, which is how it stays true. **What is not measured**, and is shown as a row on the dashboard rather than diff --git a/docs/USAGE.md b/docs/USAGE.md index 6ed2e53..d70a8e0 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -62,21 +62,88 @@ can be rerun at any time; the index is never the source of truth. $ gitmemory index 1 generation(s) 4 turn(s) 4 block(s) content=f99ac9c9b888 $ gitmemory recall "why did we drop the retry loop?" - -0.000 claude-code/0f3a9c2e-…-fc59494095426389/g00@0 user/text Don't add a retry loop around the upload; the API is idempotent only per request id. - -0.000 claude-code/0f3a9c2e-…-fc59494095426389/g00@288 assistant/text Understood. I dropped the retry loop and pass a request id instead, so a replay cannot double-charge. + -0.000 2026-09-28T10:02Z claude-code/0f3a9c2e-…-fc59494095426389/g00@0 user/text Don't add a retry loop around the upload; the API is idempotent only per request id. + -0.000 2026-09-28T10:03Z claude-code/0f3a9c2e-…-fc59494095426389/g00@288 assistant/text Understood. I dropped the retry loop and pass a request id instead, so a replay cannot double-charge. ``` -One line per turn, best first: +One line per turn: what the user said first, newest first, then everything else +newest first (see [When memories disagree](#when-memories-disagree)): | Field | Meaning | |---|---| | `-0.000` | The BM25 score. Lower is better; on a store this small every score rounds to zero | +| `2026-09-28T10:03Z` | When the turn was written, in UTC. `undated` if the transcript did not say | | `claude-code//g00` | Agent, session and generation. A new generation starts only when the source file was rewritten | | `@288` | The byte offset of the turn inside that generation's raw bytes | | `assistant/text` | Role and block kind | `-k` sets how many lines come back. +## When memories disagree + +A decision made three weeks ago and reversed last week are both in the store, +and both match the same search. gitmemory never deletes either one. What +`--policy` decides is which comes first, and the first line is the one an agent +tends to act on. + +| Policy | Order | Use it when | +|---|---|---| +| `latest-user` (default) | The user's own words newest first, then the agent's turns and tool output newest first | The user's later word should win, and nothing else should | +| `latest` | Newest first, whoever said it | Any later turn should win, the agent's included | +| `relevance` | Best BM25 match first | You want the agent to decide: every line carries its date, so it can see which turn came later | +| `module:function` | Whatever your function returns | Anything else: pinned decisions, a cut-off date, a different rule per project | + +Why the default is not plain `latest`: the newest mention is often not a +reversal. After a compaction drops "don't add a retry loop", the agent proposes +one, and that proposal is newer than the rule and matches the same words. Under +`latest` it comes first; under `latest-user` the rule does, and a user who +changes their mind still wins, because their reversal is newer user text. +Ties go to the better match, and undated turns go last. + +Set it per call with `--policy`, or for every call with `GITMEMORY_POLICY`. The +flag wins. Every policy reorders only the top `-k` hits, so a reversal that does +not match the query well enough to make the list cannot win. Raise `-k` if you +suspect one. + +If you choose `relevance`, tell the agent what the dates are for: + +```markdown +Before asking the user to repeat a decision, run `gitmemory recall ""`. +If two lines disagree, the later date is the current decision; say so. +``` + +### Your own rule + +A policy is a function from a list of `Hit`s to a list of `Hit`s. It can +reorder and it can drop; it gets nothing that is not already in the store. A +`Hit` has the fields `recall` prints, plus `text` and `ts`, and +`gitmemory.index.when(hit)` reads `ts` as a UTC `datetime`. + +A validity window, where everything before a date you name is out of date, is +one such rule: + +```python +# myrules.py +from datetime import UTC, datetime + +from gitmemory.index import latest_user, when + +REVERSED = datetime(2026, 9, 14, tzinfo=UTC) # the day the upload rules changed + + +def since_reversal(hits): + """Drop what was said before the reversal, then order as the default does.""" + return latest_user([h for h in hits if (t := when(h)) and t >= REVERSED]) +``` + +```console +$ PYTHONPATH=. gitmemory recall "upload retries" --policy myrules:since_reversal +``` + +The module is imported and runs with your permissions, so name only code you +would run anyway. A name that does not resolve is an error, not a silent +fallback to the default. + The offset is the point: it takes you back to the exact bytes, not to a paraphrase of them. The raw segments live under `raw///g/`, named by the byte range they hold, so a turn at diff --git a/src/gitmemory/__main__.py b/src/gitmemory/__main__.py index b9e50f9..f036882 100644 --- a/src/gitmemory/__main__.py +++ b/src/gitmemory/__main__.py @@ -301,6 +301,8 @@ def _derive(args) -> int: def _recall(args) -> int: + # First, so a mistyped policy is the error you see, with or without an index. + resolve = index.policy(args.policy) path = args.db or index.db_path(args.home) if not os.path.exists(path): print(f"no index at {path}; run `gitmemory index` first", file=sys.stderr) @@ -318,11 +320,23 @@ def _recall(args) -> int: f"query truncated to {index.MAX_TERMS} terms; {hits.dropped} dropped", file=sys.stderr, ) - for h in hits: + try: + shown = list(resolve(hits)) + except Exception as exc: # a custom policy is the user's code: report it, no traceback + raise ValueError(f"policy {args.policy!r} failed: {exc!r}") from exc + for h in shown: head = " ".join(h.text.split())[:160] - print(f"{h.score:8.3f} {h.session_key}@{h.byte_offset} {h.role}/{h.kind} {head}") + t = index.when(h) + # The date on every line, whatever the policy: it is what lets the + # reader see that two disagreeing turns are three weeks apart. + date = t.strftime("%Y-%m-%dT%H:%MZ") if t else "undated".ljust(17) + print(f"{h.score:8.3f} {date} {h.session_key}@{h.byte_offset} {h.role}/{h.kind} {head}") if not hits: print("no matches", file=sys.stderr) + elif not shown: + # Not a bare "no matches": the search found something and the policy + # kept none of it, which is a different fact about the store. + print(f"no matches kept: policy {args.policy!r} dropped all {len(hits)}", file=sys.stderr) return 0 @@ -395,6 +409,12 @@ def main(argv: list[str] | None = None) -> int: rec.add_argument("query") rec.add_argument("-k", type=_positive_int, default=10) rec.add_argument("--db", default=None) + rec.add_argument( + "--policy", + default=os.environ.get("GITMEMORY_POLICY") or index.DEFAULT_POLICY, + help="which hit goes first when they disagree: latest-user, latest, relevance, or " + "module:function (default $GITMEMORY_POLICY or %(default)s)", + ) rec.set_defaults(fn=_recall) args = ap.parse_args(argv) diff --git a/src/gitmemory/index.py b/src/gitmemory/index.py index 89ff974..6f278ae 100644 --- a/src/gitmemory/index.py +++ b/src/gitmemory/index.py @@ -32,13 +32,16 @@ import contextlib import hashlib +import importlib import os import re import shutil import sqlite3 import tempfile import unicodedata +from collections.abc import Callable from dataclasses import dataclass +from datetime import UTC, datetime from urllib.parse import quote from . import store @@ -46,17 +49,25 @@ from .records import Session, canonical_json, safe_text __all__ = [ + "DEFAULT_POLICY", "DEFAULT_WEIGHTS", + "POLICIES", "Hit", "Hits", + "Policy", "Stats", "Weights", "build", "db_path", + "latest", + "latest_user", "match_expr", "open_db", + "policy", + "relevance", "retriever", "search", + "when", ] SCHEMA = 2 @@ -421,6 +432,7 @@ class Hit: kind: str score: float # BM25; more negative is a better match, as FTS5 defines it text: str + ts: str | None = None # the turn's timestamp exactly as the agent wrote it @dataclass(frozen=True, slots=True) @@ -1004,6 +1016,7 @@ def search( SELECT b.byte_offset, b.byte_len, b.turn_id, b.session_key, b.agent, b.session_id, b.generation, b.role, b.kind, b.block_seq, b.prose || b.tool_use || b.tool_result AS text, s.score AS score, + b.ts, -- Partitioned by turn *and session*, not by turn alone. -- `turn_id` is content-derived over the record's sessionId, -- which the Claude Code adapter warns is reused across a @@ -1024,7 +1037,7 @@ def search( FROM scored s JOIN blocks b ON b.rowid = s.rid ) SELECT byte_offset, byte_len, turn_id, session_key, agent, - session_id, generation, role, kind, text, score + session_id, generation, role, kind, text, score, ts FROM ranked WHERE rn = 1 -- Ties are broken by position, never by rowid: the answer must not -- depend on the order generations happened to be indexed in. @@ -1046,6 +1059,7 @@ def search( kind=r["kind"], score=r["score"], text=r["text"], + ts=r["ts"], ) for r in rows ) @@ -1060,3 +1074,110 @@ def retrieve(query: str, k: int) -> list[int]: return [h.byte_offset for h in search(db, query, k=k, weights=weights)] return retrieve + + +# --------------------------------------------------------------------------- # +# conflict policy: which of two hits that disagree goes first +# --------------------------------------------------------------------------- # + +# A policy takes the hits `search` ranked by relevance and returns them in the +# order a reader should trust them. It may reorder or drop; it must not invent. +# `search` itself never applies one, so `bench/` keeps measuring plain BM25 and +# a policy is a choice made where the hits are shown — `recall --policy`. +Policy = Callable[[list[Hit]], list[Hit]] + + +def when(hit: Hit) -> datetime | None: + """`hit.ts` as an aware UTC datetime, or None if it is absent or unreadable. + + A timestamp is a string the agent wrote, so a bad one is ordinary input and + not an error. One without an offset is read as UTC, because comparing an + aware datetime with a naive one raises rather than answers. + """ + try: + t = datetime.fromisoformat(hit.ts or "") + except ValueError: + return None + return t.replace(tzinfo=UTC) if t.tzinfo is None else t.astimezone(UTC) + + +def latest(hits: list[Hit]) -> list[Hit]: + """Newest first, whoever said it: a decision reversed later is read after + the reversal, not before it. See `latest_user` for why it is not the default. + + Relevance breaks ties, because the sort is stable and `search` hands over + best-first. An undated hit goes last: a turn of unknown age cannot claim to + be the newer word on anything. + + ponytail: reorders the top `k` only, so a reversal that ranked k+1 cannot + win. The upgrade is to search a wider pool than is shown and cut after. + """ + + def key(h: Hit) -> tuple[bool, float]: + t = when(h) + return (t is None, -t.timestamp() if t else 0.0) + + return sorted(hits, key=key) + + +def latest_user(hits: list[Hit]) -> list[Hit]: + """The user's own words newest first, then everything else newest first. + The default: the latest word *the user* said wins. + + Plain `latest` lets the newest mention win, and the newest mention is often + not a reversal. After a compaction drops "don't add a retry loop", the agent + proposes one; that proposal is newer than the rule and matches the same + words, so `latest` leads with the mistake. So does a captured `recall` from + an earlier session, which is newer than every turn it quotes. A user who + reverses themselves still wins here, because their reversal is user text. + + "The user's own words" is the admission rule `derive` uses for prose: a + `text` block in the user role that no program injected. A tool result is a + user-role line in Claude Code and is not something the user said. + + ponytail: a skill's loaded instructions and a slash command's expansion are + user text with no marker, so they pass as the user's words — the same gap + `derive._injected` documents. The upgrade lands there, not here. + """ + from .derive import _injected # lazy: derive imports this module + + def typed(h: Hit) -> bool: + return h.role == "user" and h.kind == "text" and not _injected(h.text.strip()) + + ordered = latest(hits) + return [h for h in ordered if typed(h)] + [h for h in ordered if not typed(h)] + + +def relevance(hits: list[Hit]) -> list[Hit]: + """Best match first, as `search` ranked it. For a reader that reconciles + conflicts itself: `recall` prints every hit's date, so the agent can see + which of two disagreeing turns came later and decide.""" + return list(hits) + + +POLICIES: dict[str, Policy] = { + "latest-user": latest_user, + "latest": latest, + "relevance": relevance, +} +DEFAULT_POLICY = "latest-user" + + +def policy(name: str) -> Policy: + """A built-in policy by name, or your own as `module:function`. + + The module is imported, so it runs with your permissions — the same trust + as anything else on your `PYTHONPATH`, and named by you, never by the store. + """ + if name in POLICIES: + return POLICIES[name] + module, sep, attr = name.partition(":") + if not (sep and module and attr): + raise ValueError(f"unknown policy {name!r}: use {', '.join(POLICIES)} or module:function") + try: + fn = getattr(importlib.import_module(module), attr) + except (ImportError, AttributeError) as exc: + raise ValueError(f"cannot load policy {name!r}: {exc}") from exc + if not callable(fn): + raise ValueError(f"policy {name!r} is not callable") + return fn diff --git a/tests/test_index.py b/tests/test_index.py index 394bff2..edf1b7f 100644 --- a/tests/test_index.py +++ b/tests/test_index.py @@ -1401,3 +1401,152 @@ def test_search_rejects_nothing_it_can_reach_the_database_with(home, src, capsys assert main(["--home", home, "recall", "marmoset"]) == 2 assert "error:" in capsys.readouterr().err + + +# --------------------------------------------------------------------------- # +# conflict policy +# --------------------------------------------------------------------------- # + + +def dated(uid: str, text: str, ts: str) -> dict: + return user(uid, text) | {"timestamp": ts} + + +# The older turn says "uploads" three times, so BM25 ranks it first; the newer +# one reverses it. Which one `recall` leads with is the whole question. +REVERSED = [ + dated("u1", "uploads uploads: store the uploads as CSV", "2026-09-01T10:00:00Z"), + dated("u2", "scratch that, store uploads as Parquet", "2026-09-22T10:00:00Z"), +] + + +def test_latest_leads_with_the_reversal_that_relevance_ranks_second(home, src): + db = built(home, src, REVERSED) + try: + hits = index.search(db, "uploads") + finally: + db.close() + assert [h.ts for h in hits] == ["2026-09-01T10:00:00Z", "2026-09-22T10:00:00Z"] + assert "Parquet" in index.latest(hits)[0].text + assert index.relevance(hits) == list(hits) + + +def _hit(ts: str | None, offset: int) -> index.Hit: + return index.Hit(offset, 1, "t", "k", "a", "s", 0, "user", "text", -1.0, "x", ts) + + +def test_latest_reads_offsets_and_puts_the_undated_last(): + """A turn of unknown age cannot be the newer word. An offset is honoured, so + 11:00+02:00 is older than 10:00Z; a naive stamp is read as UTC rather than + raising on the comparison with an aware one.""" + hits = [ + _hit(None, 0), + _hit("not-a-date", 1), + _hit("2026-09-22T11:00:00+02:00", 2), + _hit("2026-09-22T10:00:00Z", 3), + _hit("2026-09-22T09:30:00", 4), + ] + assert [h.byte_offset for h in index.latest(hits)] == [3, 4, 2, 0, 1] + + +def test_cli_recall_defaults_to_the_latest_user_word_and_dates_every_line(home, src, capsys): + write(src, REVERSED) + assert main(["--home", home, "capture", src]) == 0 + assert main(["--home", home, "index"]) == 0 + capsys.readouterr() + assert main(["--home", home, "recall", "uploads"]) == 0 + first, second = capsys.readouterr().out.splitlines() + assert "2026-09-22T10:00Z" in first and "Parquet" in first + assert "2026-09-01T10:00Z" in second + + +def test_cli_recall_takes_a_policy_by_name_env_or_module(home, src, capsys, monkeypatch): + write(src, REVERSED) + assert main(["--home", home, "capture", src]) == 0 + assert main(["--home", home, "index"]) == 0 + capsys.readouterr() + + def first_line(*argv: str) -> str: + assert main(["--home", home, "recall", "uploads", *argv]) == 0 + return capsys.readouterr().out.splitlines()[0] + + assert "CSV" in first_line("--policy", "relevance") + assert "CSV" in first_line("--policy", "gitmemory.index:relevance") + monkeypatch.setenv("GITMEMORY_POLICY", "relevance") + assert "CSV" in first_line() + assert "Parquet" in first_line("--policy", "latest") # the flag beats the env + + +@pytest.mark.parametrize( + ("name", "says"), + [ + ("newest", "unknown policy"), + ("gitmemory.no_such_module:fn", "cannot load"), + ("gitmemory.index:no_such_fn", "cannot load"), + ("gitmemory.index:SCHEMA", "not callable"), + ], +) +def test_a_bad_policy_is_an_error_message_not_a_traceback(home, capsys, name, says): + """Checked before the index is opened, so it fails the same with no index.""" + assert main(["--home", home, "recall", "q", "--policy", name]) == 2 + assert says in capsys.readouterr().err + + +def test_a_custom_policy_that_keeps_nothing_or_breaks_is_reported( + home, src, capsys, monkeypatch, tmp_path +): + """A policy is the user's code. Dropping every hit is a legitimate answer and + is said so, not printed as silence; returning nothing or raising is an error + message, not a traceback.""" + (tmp_path / "rules.py").write_text( + "def none(hits): return []\n" + "def broken(hits): return None\n" + "def raises(hits): raise RuntimeError('boom')\n" + ) + monkeypatch.syspath_prepend(str(tmp_path)) + write(src, REVERSED) + assert main(["--home", home, "capture", src]) == 0 + assert main(["--home", home, "index"]) == 0 + capsys.readouterr() + + assert main(["--home", home, "recall", "uploads", "--policy", "rules:none"]) == 0 + out, err = capsys.readouterr() + assert out == "" and "dropped all 2" in err + for name in ("rules:broken", "rules:raises"): + assert main(["--home", home, "recall", "uploads", "--policy", name]) == 2 + assert f"policy '{name}' failed" in capsys.readouterr().err + + +def test_the_default_does_not_let_a_forgetful_agent_outrank_the_users_rule(home, src, capsys): + """The README's demo, in miniature. Compaction drops the rule; the agent then + proposes exactly what it forbade, and an earlier recall's output is captured + as a tool result. Both are newer than the rule and match it. Plain `latest` + leads with the mistake; the default leads with what the user said.""" + write( + src, + [ + dated("u1", "Don't add a retry loop around the upload.", "2026-09-28T10:00:00Z"), + assistant("a1", [{"type": "text", "text": "I'll add a retry loop to the upload."}]) + | {"timestamp": "2026-09-28T11:00:00Z"}, + user("u2", "") + | { + "timestamp": "2026-09-28T12:00:00Z", + "message": { + "role": "user", + "content": [ + {"type": "tool_result", "tool_use_id": "t1", "content": "retry loop"} + ], + }, + }, + ], + ) + assert main(["--home", home, "capture", src]) == 0 + assert main(["--home", home, "index"]) == 0 + capsys.readouterr() + + def roles(*argv: str) -> list[str]: + assert main(["--home", home, "recall", "retry loop", *argv]) == 0 + return [line.split()[3] for line in capsys.readouterr().out.splitlines()] + + assert roles() == ["user/text", "user/tool_result", "assistant/text"] + assert roles("--policy", "latest") == ["user/tool_result", "assistant/text", "user/text"]