From 399f26503e58a015b295060d400707b2ac504cfe Mon Sep 17 00:00:00 2001 From: Wuesteon Date: Fri, 7 Aug 2026 01:24:16 +0800 Subject: [PATCH 1/5] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20refactor:=20promote=20?= =?UTF-8?q?normalize=5Ftext=20to=20a=20shared=20lean=5Fmemory.normalize=20?= =?UTF-8?q?module?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two subsystems need "are these two strings the same value written differently?": WP10a's DEDUP-EXACT over fact_text, and (next commit) entity identity over entity.name. They must not answer it differently, so the function moves to a neutral module neither owns, with an `entity_key` alias so call sites read as what they are. `maintain.transforms` re-exports it — WP10a's import path and tests are untouched. Pure move: the definition is byte-identical apart from an expanded docstring recording what the fold deliberately does NOT do (no punctuation stripping, no diacritic folding, NFC not NFKC, no locale tailoring) and why each choice is the right one for a name as well as for a sentence. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01B1GNoTx3qTPrb8GQcxooWj --- src/lean_memory/maintain/transforms.py | 20 +++------ src/lean_memory/normalize.py | 57 ++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 14 deletions(-) create mode 100644 src/lean_memory/normalize.py diff --git a/src/lean_memory/maintain/transforms.py b/src/lean_memory/maintain/transforms.py index dfda044..29c0a9f 100644 --- a/src/lean_memory/maintain/transforms.py +++ b/src/lean_memory/maintain/transforms.py @@ -14,7 +14,6 @@ from __future__ import annotations import json -import unicodedata from collections import defaultdict from dataclasses import dataclass, field from typing import Iterable, Optional @@ -22,6 +21,7 @@ import numpy as np from ..extract.contradiction import is_multivalued +from ..normalize import normalize_text from ..store.base import Store from ..types import Fact from . import score @@ -32,19 +32,11 @@ # ── value-preserving text normalization (DEDUP-EXACT, §4.1) ────────────────── -def normalize_text(s: str) -> str: - """Value-PRESERVING normalization for exact-duplicate detection (§4.1). - - NFC (canonical Unicode composition) + case-fold + whitespace collapse — and - NOTHING else. Never stemming, never synonyms: a lossy normalization could merge - genuinely distinct values ('salary 100k' vs 'salary 110k', 'likes jazz' vs - 'likes blues') — the verified risk that makes DEDUP-EXACT safe to auto-apply. - Two texts share a normal form iff they are the same value written differently - (case / spacing / Unicode form). - """ - nfc = unicodedata.normalize("NFC", s) - folded = nfc.casefold() - return " ".join(folded.split()) +# `normalize_text` is imported above, not defined here: it moved to +# `lean_memory.normalize` when WP15 made entity identity (`entity.name_key`) use +# the SAME fold. Two copies of a normalization is how the `_norm`/`normalize_text` +# drift started; this module re-exports the one definition so WP10a's callers and +# tests keep importing `maintain.transforms.normalize_text` unchanged. # ── reports (what each transform did / would do) ───────────────────────────── diff --git a/src/lean_memory/normalize.py b/src/lean_memory/normalize.py new file mode 100644 index 0000000..b8655af --- /dev/null +++ b/src/lean_memory/normalize.py @@ -0,0 +1,57 @@ +"""The tree's ONE value-preserving text normalization. + +Two independent subsystems need "are these two strings the same value written +differently?": WP10a's DEDUP-EXACT clustering over `fact_text` +(`maintain/transforms.py`) and WP15's entity identity over `entity.name` +(`store/sqlite_store.py`). They must not answer it differently, so the function +lives here — a neutral module neither of them owns — and `maintain.transforms` +re-exports it so WP10a's import path and tests are unchanged. + +Not in scope for this module: `extract/router.py`'s `_norm` (`strip().lower()` ++ whitespace collapse). That is a coref *heuristic* over pronouns, not an +identity decision, and deliberately stays local to the router. +""" + +from __future__ import annotations + +import unicodedata + +__all__ = ["normalize_text", "entity_key"] + + +def normalize_text(s: str) -> str: + """Value-PRESERVING normalization: NFC + case-fold + whitespace collapse. + + NFC (canonical Unicode composition) + case-fold + whitespace collapse — and + NOTHING else. Never stemming, never synonyms: a lossy normalization could merge + genuinely distinct values ('salary 100k' vs 'salary 110k', 'likes jazz' vs + 'likes blues') — the verified risk that makes DEDUP-EXACT safe to auto-apply + (§4.1). Two texts share a normal form iff they are the same value written + differently (case / spacing / Unicode form). + + `str.casefold()` is a FULL Unicode case fold, not an ASCII map — which is the + whole reason entity collation uses this rather than SQLite's `NOCASE` + collation: `Café`/`CAFÉ` and `ЖУК`/`жук` fold here and do not under NOCASE. + + Deliberately NOT done, and their consequences: + - no punctuation stripping → `Yahoo!` ≠ `Yahoo`, `Acme.` ≠ `Acme` + (unlike `Memory._restatement_key`, which strips EDGE punctuation off + fact *text*; a name is not a sentence — stripping would kill `Yahoo!`); + - no diacritic folding → `Café` ≠ `Cafe` (that is transliteration); + - NFC, not NFKC → fullwidth `ＡＣＭＥ` ≠ `ACME`. NFKC is a + *compatibility* fold (`fi`→`fi`, `½`→`1⁄2`), which is lossier than "the + same value written differently"; + - no locale tailoring → Turkish dotted `İ` does not fold to `i` + (`'İ'.casefold()` is `i` + U+0307), and German `Weiß` DOES fold onto + `Weiss`. Both are locale problems no locale-independent fold can solve, + and picking a locale would break determinism. + """ + nfc = unicodedata.normalize("NFC", s) + folded = nfc.casefold() + return " ".join(folded.split()) + + +#: Entity identity (`entity.name_key`) uses the SAME definition as DEDUP-EXACT — +#: an alias, not a copy, so the two can never drift. Read it at the call site as +#: "this string is being used as an entity key", not as a second policy. +entity_key = normalize_text From 5faebb941927c146777f22564bf04c1d3ca167d0 Mon Sep 17 00:00:00 2001 From: Wuesteon Date: Fri, 7 Aug 2026 01:24:36 +0800 Subject: [PATCH 2/5] =?UTF-8?q?=E2=9C=A8=20feat:=20one=20subject,=20one=20?= =?UTF-8?q?entity=20=E2=80=94=20collate=20entity=20names=20on=20a=20normal?= =?UTF-8?q?ized=20key?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WP15, closes #14. Two independent defects wore one symptom; neither half closes the issue alone (measured — decision doc §1.3 config B/C), so they ship together. Store half: upsert_entity matched the raw surface form under SQLite's BINARY collation, so 'Acme' and 'ACME' were two identities. The spine is keyed on (subject_id, predicate), so the second landed in its own slot and STRUCTURALLY bypassed both the WP11 restatement skip and contradiction resolution — two co-valid current answers for one subject, silently, on the default surface. Identity is now (namespace, name_key, type) with name_key = NFC + Unicode casefold + whitespace collapse, ORDER BY created_at, id LIMIT 1 as a deterministic tie-break for legacy split rows. entity.name keeps the first-seen surface form verbatim: nothing user-visible is lowercased. A full Unicode fold, not NOCASE — Café/CAFÉ and ЖУК/жук collate, which ASCII-only NOCASE cannot; Yahoo!/Yahoo and Café/Cafe deliberately do not. Schema v3, ADD-only: ALTER + Python backfill (SQLite has no casefold()) + ix_entity_key, ALL inside the versioned `< 3` branch. Neither the column nor the index may appear in the always-run blob — the blob runs on every open, so that would break every FRESH store with 'duplicate column name: name_key'. schema.py gets a comment only; it is worded without DDL keywords because the console's engine-schema tripwire digests every schema.py line containing one and would otherwise redden the console suite over a comment. Forward-fix: pre-existing splits keep both rows and both facts; no fact is re-pointed, no interval moves, the as-of surface is identical across the migration. Extractor half: _FIRST_PERSON was case-sensitive, so "i work at acme." missed it and produced an entity literally named 'i' — #14's own example splits on user/i, not Acme/acme. Now re.I, with the two disclosed consequences pinned by tests rather than inherited silently: the ME/Mine/bare-i false-merge class into 'user', and the offline routing shift. rules.py's `I am` alternative is deliberately NOT copied — it is unreachable (leftmost-first alternation means `I` always matches first); the divergence is stated at the pattern instead. llm_typer canonicalizes known entities on the same shared key instead of a local .lower(), and StubTyper's docstring stops claiming a coreference behavior it never had. Tests: new tests/test_entity_collation.py (fold matrix, non-folds, first-seen display, tie-break, end-to-end, and the REPLACEMENT known limit pinned WITH its as-of recoverability); the five schema stamps 2 → 3; a v2 fixture + 2→3 migration coverage incl. "the migration never heals a split"; extractor pins; WP11's known-limit pin deleted and folded into the trivial-variants test. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01B1GNoTx3qTPrb8GQcxooWj --- src/lean_memory/extract/gliner_extractor.py | 27 +- src/lean_memory/extract/llm_typer.py | 23 +- src/lean_memory/store/base.py | 19 +- src/lean_memory/store/schema.py | 12 + src/lean_memory/store/sqlite_store.py | 64 ++++- tests/fixtures/make_v2_fixture.py | 273 ++++++++++++++++++++ tests/fixtures/v2_format.db | Bin 0 -> 286720 bytes tests/test_entity_collation.py | 227 ++++++++++++++++ tests/test_phase1_extraction.py | 47 ++++ tests/test_restatement_dedupe.py | 24 +- tests/test_schema_migration.py | 189 ++++++++++++-- tests/test_schema_version.py | 18 +- 12 files changed, 857 insertions(+), 66 deletions(-) create mode 100644 tests/fixtures/make_v2_fixture.py create mode 100644 tests/fixtures/v2_format.db create mode 100644 tests/test_entity_collation.py diff --git a/src/lean_memory/extract/gliner_extractor.py b/src/lean_memory/extract/gliner_extractor.py index e1ab493..4c8ebf5 100644 --- a/src/lean_memory/extract/gliner_extractor.py +++ b/src/lean_memory/extract/gliner_extractor.py @@ -284,7 +284,32 @@ def _relations_to_candidates( # First-person markers → the candidate subject is the configured default subject (the user), # matching RulesExtractor's behaviour so stub and rules agree on "I/my/me" → "user". -_FIRST_PERSON = re.compile(r"\b(?:I|I'm|my|me|mine)\b") +# +# re.I since WP15 (issue #14): without it a chat user's lowercase "i work at acme." +# missed this pattern, fell through to the Capitalized-run/lead-token fallback in +# _subject(), and produced an entity literally named 'i' — a split from 'user' that +# no amount of store-side name collation can heal, because the two names are +# genuinely different strings. +# +# Two consequences, both deliberate: +# 1. It is not a pure bugfix. my/me/mine are already lowercase here, so the flag +# also admits 'ME', 'Mine', 'MY' and a BARE 'i' anywhere in the sentence — +# "Mine uses explosives." now attributes to 'user'. That false-merge class is +# pinned in tests/test_phase1_extraction.py; see the WP15 decision doc §3.5. +# 2. It also shifts offline ROUTING: `explicit = first_person and relation in +# _EXPLICIT_RELATIONS` below, so lowercase-'i' sentences become high-confidence +# and route `direct` instead of escalating. No published calibration number +# moves (those were measured on the real GLiNER2 backbone, not this stub), but +# any FUTURE offline escalation measurement shifts. +# +# Divergence from rules.py:36 (`\b(I|I'm|I am|my|me|mine)\b`, re.I), stated rather +# than cargo-culted: the `I am` alternative is NOT carried over because it is +# unreachable. Alternation is leftmost-first, so wherever "I am" would match, the +# earlier `I` alternative already matches (the `\b` after `I` holds before a +# space), and both patterns are only ever used as a boolean `.search()`. Adding it +# would imply a behavioral difference that does not exist. The only real +# difference left is the non-capturing group, which this pattern does not need. +_FIRST_PERSON = re.compile(r"\b(?:I|I'm|my|me|mine)\b", re.I) _SENT_SPLIT = re.compile(r"(?<=[.!?])\s+") # A "named entity" heuristic: a (possibly multi-word) Capitalized run, e.g. "Acme", "San Francisco". _CAP_RUN = re.compile(r"\b[A-Z][\w.&-]*(?:\s+[A-Z][\w.&-]*)*") diff --git a/src/lean_memory/extract/llm_typer.py b/src/lean_memory/extract/llm_typer.py index dcad0b3..bdabc3c 100644 --- a/src/lean_memory/extract/llm_typer.py +++ b/src/lean_memory/extract/llm_typer.py @@ -38,6 +38,8 @@ from dataclasses import dataclass from typing import Optional +from ..normalize import entity_key + # ── Relation taxonomy + Candidate: the ONE cross-module contract. ── # taxonomy.py is the single source of truth for both the relation set and the # pre-typing `Candidate` shape. Pass 2 (gliner_extractor) emits canonical Candidates @@ -179,10 +181,13 @@ class StubTyper(Typer): * an explicit inference cue in the surface text → ``derives`` (is_inference=1) * everything else → ``asserts`` (is_inference=0) - It performs **no coreference resolution beyond identity**: a candidate's - ``subject_name`` is matched case-insensitively against ``known_entities`` and, - on an exact hit, ``subject_id`` is left as-is (already resolved) — it never - invents a link. ``supersedes``/``extends`` are intentionally NOT decided here: + It performs **no coreference resolution at all**: ``known_entities`` is + accepted for interface parity with the Ollama typer and deliberately IGNORED + — the stub passes every candidate's ``subject_name`` through untouched and + never invents a link. (Canonicalizing a resolved subject onto a known surface + form is the LLM path's job, ``OllamaTyper._apply_decisions``; entity identity + itself is decided once, in ``Store.upsert_entity``, on the shared normalized + key.) ``supersedes``/``extends`` are intentionally NOT decided here: those are a *slot-level* judgment (does the new object contradict the latest object in the same subject+predicate slot?) made by the cheap-then-escalate contradiction check in ``Memory.add`` (Pass 5), not by per-candidate typing. @@ -411,7 +416,12 @@ def _apply_decisions( decisions: list[dict], known_entities: Optional[list[str]], ) -> list[TypedFact]: - known_lookup = {e.lower(): e for e in (known_entities or [])} + # Canonicalize on the SHARED entity key, not a local .lower(): the store + # resolves identity with exactly this fold (normalize.entity_key), so a + # second, weaker definition here would canonicalize onto a surface form + # the store then keys differently ('.lower()' misses ß→ss, fi, and + # whitespace runs). One policy, one function. + known_lookup = {entity_key(e): e for e in (known_entities or [])} out: list[TypedFact] = [] for c, d in zip(candidates, decisions): relation = _norm_relation(d.get("relation")) @@ -421,8 +431,7 @@ def _apply_decisions( resolved = d.get("subject") subject_name = c.subject_name if isinstance(resolved, str) and resolved.strip(): - key = resolved.strip().lower() - subject_name = known_lookup.get(key, resolved.strip()) + subject_name = known_lookup.get(entity_key(resolved), resolved.strip()) out.append( TypedFact( subject_name=subject_name, diff --git a/src/lean_memory/store/base.py b/src/lean_memory/store/base.py index a3c2224..f5b8648 100644 --- a/src/lean_memory/store/base.py +++ b/src/lean_memory/store/base.py @@ -31,8 +31,23 @@ def add_episode(self, episode: Episode) -> None: ... # ── entities ── @abstractmethod def upsert_entity(self, entity: Entity) -> Entity: - """Resolve-or-create. If an entity with the same (namespace, name, type) - exists, return it; otherwise insert `entity` and return it.""" + """Resolve-or-create on the NORMALIZED name key (schema v3). + + The identity key is `(namespace, normalize(name), type)`, where + `normalize` is `lean_memory.normalize.normalize_text` (NFC + Unicode + case-fold + whitespace collapse) — NOT the raw surface form. Every + implementation MUST use that function, not a local `.lower()`: an + ASCII-only fold silently splits 'Café'/'CAFÉ' and 'ЖУК'/'жук'. + + If a matching entity exists, return it; otherwise insert `entity` and + return it. Two guarantees callers rely on: + - DISPLAY: the returned/stored `name` is the FIRST-seen surface form, + verbatim. Resolving a later case variant never rewrites it, and + nothing user-visible is lowercased. + - DETERMINISM: if several stored rows share one key (possible only on a + store written before v3, which is backfilled but never healed), the + OLDEST row wins — `ORDER BY created_at, id LIMIT 1`. + """ @abstractmethod def get_entity(self, entity_id: str) -> Optional[Entity]: ... diff --git a/src/lean_memory/store/schema.py b/src/lean_memory/store/schema.py index b190f1c..eb72152 100644 --- a/src/lean_memory/store/schema.py +++ b/src/lean_memory/store/schema.py @@ -19,6 +19,18 @@ ); -- ── ENTITY LAYER ──────────────────────────────────────────────────── +-- NOTE (schema v3): `name` is the FIRST-SEEN surface form, kept verbatim for +-- display. Identity resolves on `entity.name_key` (NFC + casefold + whitespace +-- collapse of `name`) via ix_entity_key. BOTH the name_key column and that index +-- live ONLY in the versioned `if user_version < 3:` branch of _init_schema and +-- must never appear here: this blob runs on EVERY open, so (a) an index over +-- name_key here would reference a column a pre-v3 file does not have, and (b) +-- declaring name_key in the table below would collide with the branch's ADD +-- COLUMN on every FRESH store ('duplicate column name: name_key' — a fresh DB is +-- stamped 1 and flows through the same branch). Same trap as fact.record_kind. +-- (Keep DDL keywords out of these comment lines: the console's engine-schema +-- tripwire, inspect_sql.compute_engine_schema_fingerprint, digests every line +-- containing one, so a prose mention flips its hash with the DDL unchanged.) CREATE TABLE IF NOT EXISTS entity ( id TEXT PRIMARY KEY, namespace TEXT NOT NULL, diff --git a/src/lean_memory/store/sqlite_store.py b/src/lean_memory/store/sqlite_store.py index 0c2d538..2a68f31 100644 --- a/src/lean_memory/store/sqlite_store.py +++ b/src/lean_memory/store/sqlite_store.py @@ -23,6 +23,7 @@ import numpy as np from sqlite_vec import serialize_float32 +from ..normalize import entity_key from ..types import Entity, Episode, Fact, new_id from .base import Store from .schema import SCHEMA_SQL @@ -136,6 +137,36 @@ def _init_schema(self) -> None: self._db.execute("PRAGMA user_version = 2") version = 2 + # v3 — entity name collation (WP15). ADD-only and forward-fix: the + # backfill writes the derived key onto existing rows and touches nothing + # else. No row is deleted, no fact is re-pointed, no validity interval + # moves, so the as-of surface is byte-identical across the migration. + # Pre-existing case-split rows ('Acme' + 'ACME') are NOT healed — both + # keep their facts; upsert_entity's tie-break just converges new mentions + # on the oldest. (Healing means re-pointing fact.subject_id, a new + # mutation verb and its own decision — deferred to a possible + # merge_entity review proposal.) + # The backfill is a Python loop by necessity: SQLite has no casefold(); + # `lower()` is ASCII-only and would silently mis-key every non-ASCII name. + # The table holds one row per distinct subject, and this runs inside the + # single _init_schema transaction. The CREATE INDEX belongs here, NOT in + # SCHEMA_SQL, for the same reason as the ALTER (see schema.py). + if version < 3: + self._db.execute( + "ALTER TABLE entity ADD COLUMN name_key TEXT NOT NULL DEFAULT ''" + ) + for row in self._db.execute("SELECT id, name FROM entity").fetchall(): + self._db.execute( + "UPDATE entity SET name_key=? WHERE id=?", + (entity_key(row["name"]), row["id"]), + ) + self._db.execute( + "CREATE INDEX IF NOT EXISTS ix_entity_key " + "ON entity(namespace, name_key, type)" + ) + self._db.execute("PRAGMA user_version = 3") + version = 3 + self._db.commit() def _check_existing_dims(self) -> None: @@ -181,17 +212,38 @@ def add_episode(self, episode: Episode) -> None: # ── entities ── def upsert_entity(self, entity: Entity) -> Entity: + """Resolve-or-create on the NORMALIZED key (schema v3, WP15). + + The lookup keys on `(namespace, name_key, type)`, not the raw surface + form: under SQLite's default BINARY collation 'Acme' and 'ACME' were two + identities, so one real-world subject got two `subject_id`s — and because + the whole spine is keyed on `(subject_id, predicate)`, the restatement + skip and contradiction resolution were never even consulted for the + second one. `type` stays in the key (Mercury/person vs Mercury/planet). + + The INSERT stores the surface form verbatim in `name` — the FIRST-seen + spelling wins the display, and later variants resolve onto it rather than + rewriting it. ADD-only: a resolve never updates an existing row. + + `ORDER BY created_at, id LIMIT 1` is the tie-break for legacy split rows + (a store written before v3 can hold several rows sharing one name_key; + the migration backfills but never heals). Oldest row wins — deterministic + and policy-free, so new mentions converge on the identity that already + owns the history. A store written entirely after v3 never has a tie. + """ row = self._db.execute( - "SELECT * FROM entity WHERE namespace=? AND name=? AND IFNULL(type,'')=IFNULL(?,'')", - (entity.namespace, entity.name, entity.type), + "SELECT * FROM entity " + "WHERE namespace=? AND name_key=? AND IFNULL(type,'')=IFNULL(?,'') " + "ORDER BY created_at, id LIMIT 1", + (entity.namespace, entity_key(entity.name), entity.type), ).fetchone() if row: return _row_to_entity(row) self._db.execute( - "INSERT INTO entity(id, namespace, name, type, summary, resolved_id, created_at) " - "VALUES (?,?,?,?,?,?,?)", - (entity.id, entity.namespace, entity.name, entity.type, - entity.summary, entity.resolved_id, entity.created_at), + "INSERT INTO entity(id, namespace, name, name_key, type, summary, " + "resolved_id, created_at) VALUES (?,?,?,?,?,?,?,?)", + (entity.id, entity.namespace, entity.name, entity_key(entity.name), + entity.type, entity.summary, entity.resolved_id, entity.created_at), ) self._commit() return entity diff --git a/tests/fixtures/make_v2_fixture.py b/tests/fixtures/make_v2_fixture.py new file mode 100644 index 0000000..a425f0b --- /dev/null +++ b/tests/fixtures/make_v2_fixture.py @@ -0,0 +1,273 @@ +"""Build the checked-in v2-format fixture DB (tests/fixtures/v2_format.db). + +This reproduces the schema **as it existed at user_version=2** — the 0.2.x +sleep-time-maintenance layout, BEFORE the schema-v3 entity-collation migration +(`fact.record_kind` and the maintenance tables ARE present; `entity.name_key` and +`ix_entity_key` are NOT). Like make_v1_fixture.py it does NOT import the current +SqliteStore, whose _init_schema would immediately migrate the file to v3 and +defeat the purpose; it lays the v2 DDL down by hand. + +The fixture deliberately carries a **pre-existing case-split entity pair** +('Acme' written earlier, 'ACME' written later, one fact each) — exactly the +damage v3 is a forward-fix for. The migration test asserts that both rows and +both facts survive the upgrade untouched (the migration backfills `name_key`; it +never heals a split, because re-pointing `fact.subject_id` is a new mutation verb +and its own decision) and that the next mention resolves to the OLDEST row. + +Determinism: fixed ids, fixed timestamps, fixed tiny vectors — so re-running the +script byte-reproduces the same DB. Run from the repo root: + + .venv/bin/python tests/fixtures/make_v2_fixture.py + +The migration regression tests (tests/test_schema_migration.py) open the result +with the CURRENT code and assert a clean, once-only 2→3 upgrade. +""" + +from __future__ import annotations + +import struct +from pathlib import Path + +import sqlite_vec + +# ── v2 SCHEMA (verbatim from the store/schema.py layout at user_version=2, plus +# the record_kind column the v2 branch ALTERs in; entity has NO name_key) ── +DIM = 8 +COARSE_DIM = 4 + +V2_SCHEMA = f""" +CREATE TABLE IF NOT EXISTS episode ( + id TEXT PRIMARY KEY, + namespace TEXT NOT NULL, + raw TEXT NOT NULL, + source TEXT, + t_ref INTEGER NOT NULL, + created_at INTEGER NOT NULL +); + +CREATE TABLE IF NOT EXISTS entity ( + id TEXT PRIMARY KEY, + namespace TEXT NOT NULL, + name TEXT NOT NULL, + type TEXT, + summary TEXT, + resolved_id TEXT, + created_at INTEGER NOT NULL +); +CREATE INDEX IF NOT EXISTS ix_entity_lookup ON entity(namespace, name, type); + +CREATE TABLE IF NOT EXISTS fact ( + id TEXT PRIMARY KEY, + namespace TEXT NOT NULL, + subject_id TEXT NOT NULL REFERENCES entity(id), + predicate TEXT NOT NULL, + object_id TEXT REFERENCES entity(id), + object_literal TEXT, + fact_text TEXT NOT NULL, + + valid_at INTEGER NOT NULL, + valid_to INTEGER, + superseded_by TEXT REFERENCES fact(id), + is_latest INTEGER NOT NULL DEFAULT 1, + + ingested_at INTEGER NOT NULL, + expired_at INTEGER, + invalidated_by TEXT REFERENCES fact(id), + + confidence REAL NOT NULL DEFAULT 1.0, + salience REAL NOT NULL DEFAULT 0.0, + last_access INTEGER, + access_count INTEGER NOT NULL DEFAULT 0, + is_inference INTEGER NOT NULL DEFAULT 0, + tier TEXT NOT NULL DEFAULT 'hot', + episode_id TEXT NOT NULL REFERENCES episode(id), + created_at INTEGER NOT NULL, + -- the v2 ALTER, materialized here because this file IS a v2 file + record_kind TEXT NOT NULL DEFAULT 'fact' +); +CREATE INDEX IF NOT EXISTS ix_fact_ns_latest ON fact(namespace, is_latest); +CREATE INDEX IF NOT EXISTS ix_fact_slot ON fact(namespace, subject_id, predicate); +CREATE INDEX IF NOT EXISTS ix_fact_valid ON fact(namespace, valid_at, valid_to); + +CREATE VIRTUAL TABLE IF NOT EXISTS fact_vec USING vec0( + fact_id TEXT PRIMARY KEY, + is_latest INTEGER, + tier TEXT, + namespace TEXT, + embedding FLOAT[{DIM}], + embedding_256 FLOAT[{COARSE_DIM}] +); + +CREATE VIRTUAL TABLE IF NOT EXISTS fact_fts USING fts5( + fact_id UNINDEXED, + fact_text +); + +-- v2 maintenance layer +CREATE TABLE IF NOT EXISTS fact_derivation ( + summary_id TEXT NOT NULL REFERENCES fact(id), + source_id TEXT NOT NULL REFERENCES fact(id), + run_id TEXT NOT NULL, + created_at INTEGER NOT NULL, + PRIMARY KEY (summary_id, source_id) +); +CREATE INDEX IF NOT EXISTS ix_derivation_source ON fact_derivation(source_id); + +CREATE TABLE IF NOT EXISTS maintenance_run ( + id TEXT PRIMARY KEY, namespace TEXT NOT NULL, + started_at INTEGER NOT NULL, finished_at INTEGER, + heartbeat_at INTEGER, + trigger TEXT NOT NULL, + cursor_id TEXT, + config_hash TEXT, stats_json TEXT, + status TEXT NOT NULL DEFAULT 'running' +); +CREATE UNIQUE INDEX IF NOT EXISTS ux_run_live + ON maintenance_run(namespace) WHERE status='running'; + +CREATE TABLE IF NOT EXISTS maintenance_proposal ( + id TEXT PRIMARY KEY, run_id TEXT NOT NULL REFERENCES maintenance_run(id), + namespace TEXT NOT NULL, + kind TEXT NOT NULL, + payload_json TEXT NOT NULL, + status TEXT NOT NULL DEFAULT 'pending', + expiry_reason TEXT, + created_at INTEGER NOT NULL, expires_at INTEGER NOT NULL, + decided_at INTEGER, decided_by TEXT, + applied_at INTEGER, edited_text TEXT, + evidence_backend TEXT +); +""" + +FIXTURE_PATH = Path(__file__).with_name("v2_format.db") + +NS = "v2user" +EP_ID = "0000ep000000-0000000000000001" + +T0 = 1_700_000_000_000 + +# 'user' plus the case-split pair the v3 migration must NOT heal. Written in the +# order a real store would have: 'Acme' first, 'ACME' later. +ENTITIES = [ + ("0000en000000-0000000000000001", "user", "person", T0), + ("0000en000000-0000000000000002", "Acme", None, T0 + 1_000), + ("0000en000000-0000000000000003", "ACME", None, T0 + 2_000), +] + +FACTS = [ + { + "id": "0000fa000000-0000000000000001", + "subject_id": "0000en000000-0000000000000001", + "predicate": "works_at", + "object_literal": "Acme", + "fact_text": "The user works at Acme.", + "valid_at": T0, + "is_latest": 1, + }, + { + "id": "0000fa000000-0000000000000002", + "subject_id": "0000en000000-0000000000000002", + "predicate": "uses", + "object_literal": "Postgres", + "fact_text": "Acme uses Postgres.", + "valid_at": T0 + 1_000, + "is_latest": 1, + }, + { + # The split's damage, frozen into the fixture: one company, two identities. + "id": "0000fa000000-0000000000000003", + "subject_id": "0000en000000-0000000000000003", + "predicate": "uses", + "object_literal": "Redis", + "fact_text": "ACME uses Redis.", + "valid_at": T0 + 2_000, + "is_latest": 1, + }, +] + + +def _vec(seed: int, dim: int) -> bytes: + """A fixed, L2-normalized float32 vector — vec0's float32 wire format.""" + raw = [float((seed * (i + 1)) % 7 + 1) for i in range(dim)] + norm = sum(x * x for x in raw) ** 0.5 + unit = [x / norm for x in raw] + return struct.pack(f"{dim}f", *unit) + + +def build(path: Path = FIXTURE_PATH) -> Path: + import sqlite3 + + if path.exists(): + path.unlink() + db = sqlite3.connect(path) + db.enable_load_extension(True) + sqlite_vec.load(db) + db.enable_load_extension(False) + db.executescript(V2_SCHEMA) + + db.execute( + "INSERT INTO episode(id, namespace, raw, source, t_ref, created_at) " + "VALUES (?,?,?,?,?,?)", + (EP_ID, NS, "seed episode", "user", T0, T0), + ) + for eid, name, etype, created in ENTITIES: + db.execute( + "INSERT INTO entity(id, namespace, name, type, summary, resolved_id, created_at) " + "VALUES (?,?,?,?,?,?,?)", + (eid, NS, name, etype, None, None, created), + ) + + for i, f in enumerate(FACTS, start=1): + # v2 fact column list — WITH record_kind, WITHOUT anything v3 adds. + db.execute( + """INSERT INTO fact( + id, namespace, subject_id, predicate, object_id, object_literal, fact_text, + valid_at, valid_to, superseded_by, is_latest, + ingested_at, expired_at, invalidated_by, + confidence, salience, last_access, access_count, is_inference, tier, + record_kind, episode_id, created_at) + VALUES (?,?,?,?,?,?,?, ?,?,?,?, ?,?,?, ?,?,?,?,?,?, ?,?,?)""", + (f["id"], NS, f["subject_id"], f["predicate"], None, f["object_literal"], + f["fact_text"], + f["valid_at"], None, None, f["is_latest"], + f["valid_at"], None, None, + 1.0, 1.0, None, 0, 0, "hot", + "fact", EP_ID, f["valid_at"]), + ) + db.execute( + "INSERT INTO fact_vec(fact_id, namespace, is_latest, tier, embedding, embedding_256) " + "VALUES (?,?,?,?,?,?)", + (f["id"], NS, f["is_latest"], "hot", _vec(i, DIM), _vec(i, COARSE_DIM)), + ) + db.execute( + "INSERT INTO fact_fts(fact_id, fact_text) VALUES (?,?)", + (f["id"], f["fact_text"]), + ) + + # The v2 stamp — this is what makes the file "v2-format" for the migration. + db.execute("PRAGMA user_version = 2") + db.commit() + db.close() + return path + + +if __name__ == "__main__": + out = build() + import sqlite3 + + check = sqlite3.connect(out) + version = check.execute("PRAGMA user_version").fetchone()[0] + fact_cols = [r[1] for r in check.execute("PRAGMA table_info(fact)").fetchall()] + ent_cols = [r[1] for r in check.execute("PRAGMA table_info(entity)").fetchall()] + tables = [ + r[0] + for r in check.execute( + "SELECT name FROM sqlite_master WHERE type='table' ORDER BY name" + ).fetchall() + ] + check.close() + assert version == 2, version + assert "record_kind" in fact_cols, "fixture must be v2-format (record_kind present)" + assert "name_key" not in ent_cols, "fixture must be PRE-v3 (no entity.name_key)" + assert "maintenance_run" in tables, "fixture must be v2-format (maintenance tables)" + print(f"built {out} — user_version={version}, entity cols={ent_cols}, tables={tables}") diff --git a/tests/fixtures/v2_format.db b/tests/fixtures/v2_format.db new file mode 100644 index 0000000000000000000000000000000000000000..5f196281d08daaf72a7dee6935292104e1b3a110 GIT binary patch literal 286720 zcmeI*eQX=&eZcWMzDlyCbDSu&>UzC6btOiWL&I-pC3 ztSyQ?&%Nf8{H`Dck zMB8)5dbMm%=N658HMe2rbE{_6C>T|vYF?|RM{}cX5)Rk;4#g7_6KZX^YFu71w`9vN zt(TUq^w#CQghOZMGn3hjo}GMnCZiv1TfotwO8IKBpg%f4eSUKOlK#HTrIapPs6RY2 z_pm-Ym(^z%W@eHX24V?wV%PC`&1ey!mo!l8yK^@&rrWhWedq3COz%3z#LdK*ZXaVh zQR}@Uo;ZD4t&KGHqZkX-awX?k_#rmUzoSirP1#xn=wz{)x^b>QmdKypu^XGp4@B6M z(qHR6xQhVmeb~5s`rxhtY}tct0=)eu0t|LxQv+|Wr4Pgt!^7%xiN+qZR~yUkIA`)e zn{b>|ubZZ4vze*Pd~lu9Cl|7F)3ajn^O@OfO01CC%;Q;oVRrh$LdIVUQ}%etiBz`; zcSd<}(K4&v%;f3YVu=gGJ9e_us6gL3Gj4lSi^+g5+3~uYGRu5=-7MwJrb(?K-loK% zlCf&0#FEL{{e6z%u|`8@`1^*=WM4y>^(~i^dUUASU>ioO_>AeP)Dbs~m13b--E3Sn zQcm=ZGn3~2cw%r+t#vmxyjWT+`^}aU``a{HrXAIXmd#CFZu0fDN^#YwZ0cf0N*4`T zH+7@5nS8J}mN-ASV~@$w0{tZ?duvzr#S=%4bX~u@p^0AW+;%-SJ)d2eoN;#tUZTEm zae8)27ehWh)T8V6Y%#dphdly=`vsAuXysPK&daJgdAwmLQmRF>;w>qj<-#d3TaE#1 z&B&Whnm5~Ay=)c=#nKgL_PLq4$?OvkJSms(lX7Dx->vIT(tS_%B=bF@FZ$J5!rrGA zuep0w*_*QE``l2cXU}FH*Bg@UO6F$m0PRs}x5(~*Vj*W#y;s$8((H~Wj)_W+G*n`( zl&i9io|bjUR40xWby(|{pAs8;vC^hQ@0*2UUic+H&=pVIE2@b$R8#VFWnJAZ>+-Yc zE$Z@nC;5CN7Eko|t1m43qe7M$cgu8+3gjGJc6`0h>%A0Qty*-XvwiXo4`haVpn*cX z4ElWLTxLEqdnR*HH%nEq_6-#aNx7JB;w-hb+_L8v%+R(tH@7_gsu(NvaVTTW*{fiS z)}?wxL+s6h7z}PTTelm2o@%)%j}!iyS+UH5S;$@96zTTb6KJMvfETmhPi{Hh4#C$t zn>jbRFq73s?UsqvM67Y5V7GD2*=6|$#@}@6e!GsR?U=>7SI?@8PHvSs?POjttZL54i+zY?7aiDkICFFP@_NbU zZw}C}*RECQi7lU5@oL^achh#MaeVm&4=$CfgSO*Z(JB{AxsSCUHkRKnf&Mg|j9!=K zE2a(Xji2U5$+AaA^y-qSZ;a`aGuh01N*8A~W~C?)pD_#il88vZZ03!1%M|&FmcCeA z5xcL8x*->l6QYcYnJ-rgxn;54Xwg2ee+K1f8|+CA#$$<7{oC$W>{Ey?BN|VP42b=l zw>@%J(%ee9yu7~VOvIWuNDX=9_Q+kc{cV2A-u$QZ>gJl6teN3>VqieM?i>U-d7ZzJ zhRu>QZ_9?s?un+|kzbRrXy_@)EfbwiY>_CN6il>-^ZKg2bC&aEX>wl0w8|?RV$Bf) z!kZWD>*jv$Ne)M1iHU)B=1y?AQRg{Y`=A<696qdGcaARILFGP$8d;nH_r_f+##Jx! zt!P$xz2dF=UcA*@#awh3X^vMKZSOG>iX|owxAkz>w^29duya!V_WoY+Le)O6i2r#Y zfB*srAb*>c_tRZ%c~vw@1XKYTs4F|2z;t009ILKmY**5I_I{1Q0-=GX?bURJXhw(Dn*} zy#F8878LCz?c?H!2LcEnfB*srAbQ@pY;)mI~ z<-cyFtLyfuXurJMyl8CuZuM<%NQ=`-YhL{JiY4FEuFEOio;>cnzuoyOegeWSWUX~Q zqfRdQ%L=uTkkis5CvK#r?i=K{YpMKNJnPZ2RlOp9cwkbT9XpNn=Gg6BepR+)`PJRD zcllMjy`!pZ?-8Z{)Qz;)eP{l5?H#>Zt}I*PS4U)9v*O1sk-!rQ>fgpeY0tg_000IagfB*srbee$o?*6t{Wc>I4x64s~|9`t2 z?)(40t!O{h{$2ZarxDR!1Q0*~0R#|0009ILKmY**5K#6-2D;tP1cU>xLp@@?`$+)j z{=cPYU)TOjvoJ#d0R#|0009ILKmY**5I_KdH%VY5Dn5PTo|bR-rvDT2Lki*D=C(i5 z5K4=(kGQAy+m-EqpuqqBzaMM=BEANY2LcEnfB*srAbEolvAC&k1Bibhw?Z?_TwZ9cFcp!iP0tg_000IagfB*srAb`NHLf}xF z%K(uU3HDWhaH|A)6(Dq5IN5aFK;HjX`wl4Le;x=RfB*srAb4HAU+Pi2 zl+bhgYtQbFDDhu$;Hq1H$bAm~ircgIs=v2a`;ijTUQ)D|w13)r;Smu62q1s}0tg_0 z00IagfB*t}M&NceJghDn`D$*%%;zfQtHpv9J{SrQcl$H*OY5a&`R)F9)aN-F<^BJ# z_6bG%miBq^!~+2Y5I_I{1Q0*~0R#|0009Kv8iBjRQDv%Im0#b#Xvn|m^oaB1|2+{p z8j_j1<(zhz#*V5oPuF&NMxF2f7vKN)UG0l}``=sB!PJ5P0tg_000IagfB*srAb>zX z;I8o0_7?%dJLMP)>EWquuMvzopa0jCJ}po)FA+ch0R#|0009ILKmY**_PW4x&qmc~ z_eY-H|Kg|b8CR5N$6x&Pr^go-zA0WS6AKId6Jq+bOpCtwq49ga@Qv}(*A7lx`tsoT zr7wR<%=@`~J}uM!{PjR3Oxx>_9TJ~?|6kR9 zp@{!^Ab|v z_Wl3op56b*rH_sODEf`@M=tG~_@jG|PaOZuUyT3aJ>3(=J06<&#P2;Z{{6ErjZc5; zo{7&`zd!yN>)^!BMU&$e6nId6{-68*w_wlEBm@vZ009ILKmY**5I_I{1l}Zp`u+a{ zO5cGusZfeR009ILKmY**5I_I{1Q6&5fxnZV|F1o}Uv*pC{M->i(8%2ioaX-jZmGzJ z00IagfB*srAbMq)f00IagfB*srAbaz7Mm~4rF{azcm`=>~#}fI|JFCUs%ZaD{;ymUpbNL7U9k)FD_bU z)ti}oREx2IOE+%a3=d2 z%B*j>oYbR3%|UGAU=^P+y|C&E8^%hpP^@k?j^vbs+&DA&U~epOesIU$FP0X|fxefM zz2^RSVsKEcbvMQ=)BK3##Qrw%%Cw{U(6YIy%RQ35Rw=F;l}%mDNa-SmbyGJ=o5|Xh zeeuMRBVE_;Zm7qr!Y%x<>G|x!WV9ky0(16>mxLEEi6R*>Vh6YewF5(!AN`>SeP~D3-1`v(L@UO=h2X z;7PfJpOhOr`EFfzlJ0x5Cz`mG7eQv1Jvu87p>kY|vC3CZO zfcB`gTVyL13pu0ey{eXzW_LVsOjL5Dp%QDQT$Oe7w5&s>I&rk9!&<-ml-Rn9wJs&P z&@2@5!Y}!Ou6W{JQBAa=nv$n0>*{V$@nc%r{wePP)j6|&5@Tc&eV zAm`|^U&E%Ef#W zXQ{2_mOZ~eRLf0yobcDo zie(neLhkaWNVnIXKr>|nyqNuda?A0y=)Tt3%(=;hnXEo)w@j=iVvQ38yNzqkE~_`M ztrf-S^0IiXmr2Ev9hw++PSe`AOk~cNON+&V+|)a56GxFV{-#s++jTr`$1K*pdRAR@ za;wZ~C-aJ7RdYsO>_aTO=)ktanVZX(*Go2kbAW!mcCA8BY#YsrSM&C{o3=}hn%qR=HrxouvJ+vHX4s^rz`$^tv=(F>PRP{4_U8mOV0}SC>qEV@#i%$!6wL z`l?u@QWS{Km<4@FM5JFf^TxVmihM;&Uo5VO-Pc9kkc-F(QAWkgmn((bve<64XrI?V zgL1SD_9O@6vBatVZFgVxDMXhMjVDG1#Lmjw9yu#%ZlzpaUSD%2V$B<*hCFh6%K5T1Ij>?`<&_Pw=7<5|%?tK*b3gYaha<7X#6UZ9C%D|G^Bk>x zP>m-JA6BnBhllQ7dLU*j|ZCx+M-y8pG>|bM7 zqd$v&JbEPZSK&g$3jMeGbm*^@pY$CQ3v7L~nzuFA&CZz`i6x#m*)G0Sqgbk%B|~gb zb8D6ITG=vIa-*&09{;W3c;fD1wf3OB4z6FzRn|+nmEwjO%q`}n++pD!-8;uSf$XA3 z#JbqDQ2nqcslWdbu`AXsaiwG3Iz1>#EQvkL;ByZp?Su4>+;1N;G%epWc~8U8Y)bTo zsJ%u8*Vw?(pq=JzSM8OrYE;~P+?FvWHh!g|wdCyH+*G;Dk|~zCEH=Dyw-=aStrV|Z z5r?B&D#@=`EODM4T$knk?n-XSu$G+l+jg;XPg&)Xzrs6Ijm6b>9Ufp$^7K91ohvpe zHI4#N$Z&Zmo;WE6z&>vyTo8MQ4Wn8tmvYW}X&=SZ=O*1=@DF+GN$vvPQDUy1GUOB` z5R%XBf0y0m*XR7B#6ZTT$_5*{yeZM{a?9C%dc5_+hQMx9F6Zqt{f0DcpMwXB!TiEV zmMtG!d$zv+-72fl*gf3~xE7Dun86di#i}KaYUBvB zm#$B~s{zyAUFYhlgG0-~7K_spvC>XwADLUo>hp8&pFX?#PO-ZlQER>Rev?ZV%6WOp z>(}K@A8S*Yn>;A`!&`eAtF@l7PluB$cRSVXYphO0CN^#MiILZMIsMKy)w$W*SC>zx z^!#Y@Lr29a*fFskY^<$VxaJ2YUfentWj~;DPy-aH#;pLTbe%Ucy((TjsTHun1=t8d!xAYdL?IM$V+Adz5 z44$@&{ee97&775+M!O(+zSEQZ@NdKur%tK0{<^0)<8Qp&J!a?MI$LiDvwktA;ad5a zcV#NDmbn*^<$9Vt|Bf5K{yrACoYGj?UA4Zq@8-Jk){vYlNa?ZP*dZt{U;D5cuZ>?g z5=-3pb>mhMdNsde-=8{1L2vSHg%7`N=b^Cmo`Q2jsKlomXy0{`&!PHxogJeO4%sXPelB~Y4>bNoV+z22G`F>#0499ZQZ$`B`*wa ziZgg4ct#UAW#4kR>|RT>0w+}sbr;OMxI$=OYV^*zbZ?%x^eH2eO*Gcl#MQyZY~u2_ zJjb)I!+QD54d;4e?y`|z77e!x=t=g9^KrG0@BhEq%MvVy00IagfB*srAbpe_-2q1s}0tg_000IagfB*srI0AhCKO+GF1Q0*~0R#|0009ILKmdWgFCahvAJ(2y zv>#~yC7yU7fB*srAbO0NhLwFB||Nk>Z``H@`j)f6G009ILKmY** z5I_I{1Q0;rmK2Cay7#Lg=l=hbiuSVhyV{etWJ75b0tg_000IagfB*srAb>W^Z!4HOwcS#q*(H0Sif+F-Ko`H{F3UOYod5q&(f(KaiT1-=o0HZdfB*srAbWgZa9BB@grgznNqi#UfD(1y|Nokzy|&}>WIzA`1Q0*~0R#|0009IL zKmdUb64)0$tZtYUt5_~Y|p_Z|8Ea>E}fB*srAb-Y!=009IL zKmY**5I_I{1Q0*~0R(muII&X((h)!a0R#|0009ILKmY**5I~^g1@7zk2nYZH1Q0*~ z0R#|0009ILKmY**b`m(bQwGuzKmY**5I_I{1Q0*~0R#|0pyLJJ-SH6+00IagfB*sr zAbeY{y4H00!}3!Llt2nYZH1Q0*~0R#|0 j009ILKmY**5I_I{1Q0*~0R#|0009ILKmdVXbAkT@0~npl literal 0 HcmV?d00001 diff --git a/tests/test_entity_collation.py b/tests/test_entity_collation.py new file mode 100644 index 0000000..12a0809 --- /dev/null +++ b/tests/test_entity_collation.py @@ -0,0 +1,227 @@ +"""Entity name collation — one real-world subject, one entity (WP15, issue #14). + +`upsert_entity` resolves on a stored, Unicode-normalized key +(`entity.name_key` = NFC → casefold → whitespace collapse) instead of the raw +surface form under SQLite's BINARY collation. Two halves are pinned here: + + - the FOLD: case/whitespace/Unicode-form variants of a name land on ONE + entity, so the WP11 restatement skip and contradiction resolution (both + keyed on `(subject_id, predicate)`) actually apply to it; + - the NON-FOLDS: punctuation and diacritics are NOT folded — `Yahoo!` is not + `Yahoo` and `Café` is not `Cafe`. Folding those is transliteration, not + case, and would kill genuinely distinct names. + +`entity.name` keeps the FIRST-SEEN surface form verbatim — the fold lives only +in `name_key`, so nothing user-visible becomes lowercase. + +The deliberate cost of the fold — genuinely case-distinct subjects merging — is +pinned at the bottom as the replacement known limit (it replaces WP11's +`test_entity_case_variant_splits_the_slot_known_limit`, deleted with this +change). All offline: stub embedder/extractor, no downloads. +""" + +from __future__ import annotations + +import pytest + +from lean_memory import Memory +from lean_memory.normalize import normalize_text +from lean_memory.store.sqlite_store import SqliteStore +from lean_memory.types import Entity + + +@pytest.fixture +def store(tmp_path): + s = SqliteStore(tmp_path / "ns.db", dim=768) + yield s + s.close() + + +@pytest.fixture +def mem(tmp_path): + m = Memory(root=tmp_path) + yield m + m.close() + + +def _entity_names(mem: Memory, ns: str) -> list[str]: + store = mem._store(ns) + return [ + r["name"] + for r in store._db.execute( + "SELECT name FROM entity ORDER BY created_at, id" + ).fetchall() + ] + + +def _upsert(store: SqliteStore, name: str) -> Entity: + return store.upsert_entity(Entity(namespace="ns", name=name, type=None)) + + +# ── the key itself ── +def test_name_key_is_stored_and_folded(store): + got = _upsert(store, " Acme Corp ") + row = store._db.execute( + "SELECT name, name_key FROM entity WHERE id=?", (got.id,) + ).fetchone() + assert row["name"] == " Acme Corp ", "surface form stored verbatim" + assert row["name_key"] == "acme corp" == normalize_text(" Acme Corp ") + + +# ── the fold: variants resolve to ONE entity ── +@pytest.mark.parametrize( + "first,second", + [ + ("Acme", "ACME"), # acronym styling + ("Acme", "acme"), # sentence-initial casing + ("Café", "CAFÉ"), # non-ASCII case — NOCASE misses this + ("ЖУК", "жук"), # Cyrillic case — NOCASE misses this + ("Weiß", "WEISS"), # full case fold (ß → ss) + ("Acme Corp", "Acme Corp"), # whitespace collapse + ("Caf\u00e9", "Cafe\u0301"), # NFC vs NFD — same value, two encodings + ], +) +def test_variants_resolve_to_one_entity(store, first, second): + a = _upsert(store, first) + b = _upsert(store, second) + assert b.id == a.id, f"{second!r} must resolve to the {first!r} entity" + assert b.name == first, "display form is the FIRST-seen surface form" + assert store._db.execute("SELECT COUNT(*) c FROM entity").fetchone()["c"] == 1 + + +# ── the non-folds: value-preserving, not lossy ── +@pytest.mark.parametrize( + "first,second", + [ + ("Acme", "Acme."), # trailing punctuation is part of the name + ("Yahoo!", "Yahoo"), # ...and stripping it would kill 'Yahoo!' + ("Café", "Cafe"), # diacritic folding is transliteration, not case + ], +) +def test_deliberate_non_merges_stay_distinct(store, first, second): + a = _upsert(store, first) + b = _upsert(store, second) + assert b.id != a.id + assert store._db.execute("SELECT COUNT(*) c FROM entity").fetchone()["c"] == 2 + + +def test_type_still_separates_entities(store): + """`type` stays in the key: the ingest path passes NULL today, but typed + entities (WP4+) must keep Mercury/person and Mercury/planet apart.""" + person = store.upsert_entity(Entity(namespace="ns", name="Mercury", type="person")) + planet = store.upsert_entity(Entity(namespace="ns", name="mercury", type="planet")) + assert person.id != planet.id + + +def test_namespace_still_separates_entities(store): + a = store.upsert_entity(Entity(namespace="ns", name="Acme", type=None)) + b = store.upsert_entity(Entity(namespace="other", name="acme", type=None)) + assert a.id != b.id + + +def test_legacy_split_rows_resolve_to_the_oldest_row(store): + """The tie-break. A store written BEFORE v3 can hold two rows sharing one + name_key (the migration backfills, it never heals — see + test_schema_migration.py). The lookup's `ORDER BY created_at, id LIMIT 1` + makes the winner deterministic: oldest row, so the store converges forward + on the identity that already owns the most history.""" + rows = [ + ("ent-newer", "ACME", 1_700_000_005_000), + ("ent-older", "Acme", 1_700_000_000_000), + ] + for eid, name, created in rows: + store._db.execute( + "INSERT INTO entity(id, namespace, name, name_key, type, summary, " + "resolved_id, created_at) VALUES (?,?,?,?,?,?,?,?)", + (eid, "ns", name, normalize_text(name), None, None, None, created), + ) + store._db.commit() + + got = _upsert(store, "AcMe") + assert got.id == "ent-older" + assert got.name == "Acme" + assert store._db.execute("SELECT COUNT(*) c FROM entity").fetchone()["c"] == 2, ( + "resolving must not create a third row — and must not heal the split either" + ) + + +# ── end-to-end: the failure #14 reported ── +def test_case_variants_land_in_one_slot_end_to_end(mem): + """The observable symptom: 'Acme'/'ACME'/'acme' in subject position used to + make three entities and three co-valid latest facts (one company, three + identities). Now: one entity, one fact — the restatement skip finally sees + them as the same slot.""" + mem.add("ns", "Acme likes coffee.", t_ref=1_000) + mem.add("ns", "ACME likes coffee.", t_ref=2_000) + mem.add("ns", "acme likes coffee.", t_ref=3_000) + + assert _entity_names(mem, "ns") == ["Acme"], "one entity, first-seen display form" + rows = mem._store("ns")._db.execute( + "SELECT fact_text, is_latest FROM fact" + ).fetchall() + assert len(rows) == 1, [r["fact_text"] for r in rows] + assert rows[0]["is_latest"] == 1 + + +def test_unicode_variants_merge_end_to_end(mem): + """The cases a SQLite NOCASE collation cannot reach — it is ASCII-only.""" + mem.add("cafe", "Café likes coffee.", t_ref=1_000) + mem.add("cafe", "CAFÉ likes coffee.", t_ref=2_000) + assert _entity_names(mem, "cafe") == ["Café"] + + mem.add("bug", "ЖУК likes coffee.", t_ref=1_000) + mem.add("bug", "жук likes coffee.", t_ref=2_000) + assert _entity_names(mem, "bug") == ["ЖУК"] + + +def test_first_person_case_variant_lands_in_the_user_slot(mem): + """#14's literal example. Both halves of the fix are needed: the store-side + key alone leaves 'i' as its own entity, because the extractor's first-person + regex used to be case-SENSITIVE (see test_phase1_extraction.py).""" + mem.add("ns", "I work at Acme.", t_ref=1_000) + mem.add("ns", "i work at acme.", t_ref=2_000) + + assert _entity_names(mem, "ns") == ["user"] + rows = mem._store("ns")._db.execute("SELECT fact_text FROM fact").fetchall() + assert len(rows) == 1, [r["fact_text"] for r in rows] + + +# ── the replacement known limit ── +def test_case_distinct_subjects_merge_known_limit(mem): + """KNOWN LIMIT, pinned (replaces WP11's + `test_entity_case_variant_splits_the_slot_known_limit`, which pinned the + opposite behavior and was deleted when this landed): + + Two GENUINELY distinct entities in one namespace whose names differ only by + case fold to one entity. On a FUNCTIONAL predicate that retires the earlier + fact from the current surface. This is the deliberate cost of the fold — a + false SPLIT is silent and permanent, a false MERGE is visible in the + supersession chain and nothing is deleted, so the asymmetry decides it. + + The recoverability half of that argument is executable, not rhetorical: the + retired fact is still readable at `as_of`.""" + mem.add("ns", "Mercury lives in Rome.", t_ref=1_000) + mem.add("ns", "mercury lives in thermometers.", t_ref=2_000) + + assert _entity_names(mem, "ns") == ["Mercury"], "one entity — the merge" + + rows = mem._store("ns")._db.execute( + "SELECT fact_text, is_latest, valid_to, superseded_by FROM fact " + "ORDER BY valid_at" + ).fetchall() + assert len(rows) == 2, "ADD-only: nothing is deleted by the merge" + old, new = rows + assert "Rome" in old["fact_text"] + assert old["is_latest"] == 0, "the earlier fact left the current surface" + assert old["valid_to"] == 2_000 + assert old["superseded_by"] is not None, "supersession chain records the merge" + assert new["is_latest"] == 1 + + # ...and it is still retrievable as-of the interval in which it was true. + hits = mem.search( + "ns", "Mercury lives in Rome", k=5, as_of=1_500, is_latest_only=False + ) + assert any("Rome" in h.fact.fact_text for h in hits), ( + "the retired fact must stay readable via as_of — 'recoverable' is the " + "whole reason this trade is acceptable" + ) diff --git a/tests/test_phase1_extraction.py b/tests/test_phase1_extraction.py index a7d27df..34e90e0 100644 --- a/tests/test_phase1_extraction.py +++ b/tests/test_phase1_extraction.py @@ -66,6 +66,53 @@ def test_inference_cue_word_boundary(): assert all(t.is_inference == 0 for t in typed) +# ── first-person detection is case-insensitive (WP15, issue #14) ── +def _subjects(raw: str) -> list[str]: + ep = Episode(namespace="t", raw=raw, t_ref=1_700_000_000_000) + return [c.subject_name for c in StubCandidateGenerator().generate(ep)] + + +def test_lowercase_first_person_resolves_to_the_default_subject(): + """WP15's extractor half. `_FIRST_PERSON` used to be case-SENSITIVE (unlike + RulesExtractor's, which has carried re.I since Phase 0), so a chat user + typing a lowercase 'i' fell through to the Capitalized-run/lead-token + subject fallback and got an entity literally named 'i'. That is half of + issue #14 — the store-side name_key fold alone does NOT close it, because + the split is 'user' vs 'i', not 'Acme' vs 'acme'.""" + assert _subjects("i work at acme.") == ["user"] + + +@pytest.mark.parametrize( + "raw,was", + [ + ("Mine uses explosives.", "Mine"), + ("ME uses Postgres.", "ME"), + ("The company Acme uses i as a variable.", "The"), + ], +) +def test_re_i_first_person_false_merge_class_is_pinned(raw, was): + """KNOWN REGRESSION CLASS, pinned — the price of the re.I above. + + `my`/`me`/`mine` were already lowercase in the pattern, so re.I admits not + only the wanted lowercase 'i' but also 'ME', 'Mine', 'MY', and a bare 'i' + ANYWHERE in the sentence. A sentence whose genuine subject is that noun is + silently re-attributed to `default_subject` — a false merge into 'user', + the busiest slot in any store. It is bounded (the token must appear before + the relation verb) and recoverable on the same as_of/history terms as the + entity-fold known limit, but it is a DECISION, not an accident: this test + is what makes it one. (`was` records the pre-WP15 subject.) + + Note what is NOT a test of this change: "my budget is 40 euros." already + resolved to 'user' before re.I, because `my` is lowercase in the pattern. + The only genuinely-new lowercase form is the bare `i`.""" + assert _subjects(raw) == ["user"], f"pre-WP15 this was {was!r}" + + +def test_lowercase_my_was_already_first_person(): + """The control for the test above — unchanged by re.I in both directions.""" + assert _subjects("my budget is 40 euros.") == ["user"] + + # ── contradiction → supersession ── def test_supersession_via_contradiction_resolver(mem): mem.add("u", "I work at Acme.", t_ref=1_700_000_000_000) diff --git a/tests/test_restatement_dedupe.py b/tests/test_restatement_dedupe.py index f68e392..1fb896a 100644 --- a/tests/test_restatement_dedupe.py +++ b/tests/test_restatement_dedupe.py @@ -41,30 +41,18 @@ def test_trivial_formatting_variants_are_treated_as_restatements(tmp_path): mem.add("ns", "I work at Acme,", t_ref=2_000) mem.add("ns", "I WORK AT Acme.", t_ref=3_000) mem.add("ns", "I work at\tAcme.", t_ref=4_000) + # The ENTITY-surface case variant belongs here too since WP15: 'i'/'acme' + # used to resolve to their own entities (BINARY-collation name match), land + # in a different slot, and bypass this skip entirely. That was pinned as a + # known limit; entity resolution now folds case (tests/test_entity_collation.py), + # so it is just another trivial variant. + mem.add("ns", "i work at acme.", t_ref=5_000) rows = _facts_in(mem, "ns", "works_at") assert len(rows) == 1, f"formatting variants stacked rows: {[r['fact_text'] for r in rows]}" mem.close() -def test_entity_case_variant_splits_the_slot_known_limit(tmp_path): - """KNOWN LIMIT, pinned: a case variant of the ENTITY surface form ('acme' - vs 'Acme') resolves to a different entity via upsert_entity's BINARY- - collation name match, so it lands in a different slot and bypasses both - restatement dedupe and contradiction resolution. Fixing this is an - entity-resolution change (collation policy on every lookup, with real - distinct-by-case counterexamples like 'Polish'/'polish') — out of WP11 - scope. If this test starts failing with 1 row, that fix landed: fold the - lowercase variant into the trivial-variants test above and delete this.""" - mem = Memory(root=tmp_path) - mem.add("ns", "I work at Acme.", t_ref=1_000) - mem.add("ns", "i work at acme.", t_ref=2_000) - - rows = _facts_in(mem, "ns", "works_at") - assert len(rows) == 2 - mem.close() - - def test_internal_punctuation_difference_is_not_a_restatement(tmp_path): """Internal punctuation can carry meaning — normalization strips edges only, so these must fall through to contradiction resolution, not skip.""" diff --git a/tests/test_schema_migration.py b/tests/test_schema_migration.py index 5bb4c88..4f406f5 100644 --- a/tests/test_schema_migration.py +++ b/tests/test_schema_migration.py @@ -1,22 +1,30 @@ -"""Schema v2 migration + the ledger/proposal CRUD (design spec §5, §4.0). +"""Schema migrations (v1→v2→v3) + the ledger/proposal CRUD (design spec §5, §4.0). -The migration is the project's FIRST persisted-format change, so it carries a -checked-in v1-format fixture DB (tests/fixtures/v1_format.db, built by -make_v1_fixture.py) and pins the whole 1→2 upgrade end-to-end: +Each persisted-format change carries a checked-in fixture DB of the format it +upgrades FROM (tests/fixtures/v1_format.db, v2_format.db — built by +make_v1_fixture.py / make_v2_fixture.py) and pins the upgrade end-to-end: - a genuine v1-format file opens, migrates ONCE (adds fact.record_kind + the - maintenance tables), and REOPENS cleanly — the ALTER-idempotence trap, where a - second open would raise 'duplicate column name' if the ADD COLUMN lived in the - always-run schema blob instead of the versioned `< 2` branch; - - after migration user_version == 2 and a search still round-trips; - - the fresh-create path stamps 2 and reopens clean; + maintenance tables, then entity.name_key + ix_entity_key), and REOPENS + cleanly — the ALTER-idempotence trap, where a second open would raise + 'duplicate column name' if the ADD COLUMN lived in the always-run schema blob + instead of the versioned branch; + - a genuine v2-format file migrates 2→3: name_key is backfilled for EVERY + pre-existing row, the file reopens clean, and pre-existing case-split entity + rows keep both rows and both facts (the migration backfills, it never heals); + - after migration user_version == the current version and a search still + round-trips; + - the fresh-create path stamps the current version and reopens clean — a fresh + DB is stamped 1 and flows through the SAME versioned branches, so a column + declared in the always-run blob AND ALTERed here would break every + first-run; - a DB stamped by a newer release is never downgraded. The CRUD half pins pure row round-trips plus the ux_run_live partial-unique-index race (a second live run for a namespace hits the constraint). No decide/apply logic is exercised here — that is a later task. -All offline (no model download); the fixture's tiny 8-dim vectors are opened with +All offline (no model download); the fixtures' tiny 8-dim vectors are opened with matching dims so _check_existing_dims passes. """ @@ -29,12 +37,14 @@ import numpy as np import pytest +from lean_memory.normalize import normalize_text from lean_memory.store.sqlite_store import SqliteStore from lean_memory.types import Entity, Episode, Fact FIXTURE_SRC = Path(__file__).parent / "fixtures" / "v1_format.db" -# The fixture was built with these tiny dims (make_v1_fixture.py); the store must -# open it with matching dims or _check_existing_dims refuses the vec0 mismatch. +V2_FIXTURE_SRC = Path(__file__).parent / "fixtures" / "v2_format.db" +# The fixtures were built with these tiny dims (make_v*_fixture.py); the store must +# open them with matching dims or _check_existing_dims refuses the vec0 mismatch. FIXTURE_DIM = 8 FIXTURE_COARSE_DIM = 4 @@ -55,6 +65,18 @@ def v1_db(tmp_path): return dst +@pytest.fixture +def v2_db(tmp_path): + """A writable copy of the checked-in v2-format fixture.""" + assert V2_FIXTURE_SRC.exists(), ( + f"missing {V2_FIXTURE_SRC} — rebuild it with " + "`.venv/bin/python tests/fixtures/make_v2_fixture.py`" + ) + dst = tmp_path / "v2user.db" + shutil.copy(V2_FIXTURE_SRC, dst) + return dst + + # ── Migration: the v1-format fixture ── def test_v1_fixture_is_genuinely_v1(v1_db): """Guard the fixture itself: it must be a v1 file (version 1, no record_kind, @@ -77,10 +99,11 @@ def test_v1_fixture_is_genuinely_v1(v1_db): db.close() -def test_v1_migrates_once_to_v2(v1_db): +def test_v1_migrates_once_to_current_version(v1_db): + """A v1 file crosses BOTH versioned branches in a single open (1→2→3).""" store = SqliteStore(v1_db, dim=FIXTURE_DIM, coarse_dim=FIXTURE_COARSE_DIM) try: - assert _user_version(store) == 2, "upgraded to schema v2" + assert _user_version(store) == 3, "upgraded to schema v3" # record_kind added, default 'fact' backfilled on the existing rows. cols = [r[1] for r in store._db.execute("PRAGMA table_info(fact)").fetchall()] assert "record_kind" in cols @@ -96,20 +119,28 @@ def test_v1_migrates_once_to_v2(v1_db): ).fetchall() } assert {"fact_derivation", "maintenance_run", "maintenance_proposal"} <= tables + # ...and the v3 half: entity.name_key backfilled from the surface form. + ent_cols = [ + r[1] for r in store._db.execute("PRAGMA table_info(entity)").fetchall() + ] + assert "name_key" in ent_cols + rows = store._db.execute("SELECT name, name_key FROM entity").fetchall() + assert rows, "the fixture carries entities to backfill" + assert all(r["name_key"] == normalize_text(r["name"]) for r in rows) finally: store.close() def test_v1_reopens_cleanly_after_migration(v1_db): """The ALTER-idempotence trap: a SECOND open must not raise 'duplicate column - name'. This is the whole point of gating the ADD COLUMN behind `user_version < 2` - instead of putting it in the always-run schema blob.""" + name'. This is the whole point of gating each ADD COLUMN behind its + `user_version < N` branch instead of putting it in the always-run schema blob.""" SqliteStore(v1_db, dim=FIXTURE_DIM, coarse_dim=FIXTURE_COARSE_DIM).close() # A real second open of the now-migrated file — must be clean. reopened = SqliteStore(v1_db, dim=FIXTURE_DIM, coarse_dim=FIXTURE_COARSE_DIM) try: - assert _user_version(reopened) == 2 + assert _user_version(reopened) == 3 finally: reopened.close() @@ -141,26 +172,136 @@ def test_migrated_v1_search_roundtrips(v1_db): store.close() -# ── Migration: fresh-create + no-downgrade (the same versioned branch) ── -def test_fresh_create_stamps_v2_and_reopens_clean(tmp_path): +# ── Migration: the v2-format fixture (2→3, WP15) ── +def test_v2_fixture_is_genuinely_v2(v2_db): + """Guard the fixture itself: a v2 file (version 2, record_kind + maintenance + tables present, entity.name_key ABSENT) — otherwise the 2→3 test proves + nothing.""" + db = sqlite3.connect(v2_db) + try: + assert db.execute("PRAGMA user_version").fetchone()[0] == 2 + fact_cols = [r[1] for r in db.execute("PRAGMA table_info(fact)").fetchall()] + assert "record_kind" in fact_cols + ent_cols = [r[1] for r in db.execute("PRAGMA table_info(entity)").fetchall()] + assert "name_key" not in ent_cols + tables = { + r[0] + for r in db.execute( + "SELECT name FROM sqlite_master WHERE type='table'" + ).fetchall() + } + assert {"fact_derivation", "maintenance_run", "maintenance_proposal"} <= tables + indexes = { + r[0] + for r in db.execute( + "SELECT name FROM sqlite_master WHERE type='index'" + ).fetchall() + } + assert "ix_entity_key" not in indexes + finally: + db.close() + + +def test_v2_migrates_once_to_v3(v2_db): + """The 2→3 upgrade: ALTER + Python backfill (SQLite has no casefold()) + + ix_entity_key, all inside the versioned branch.""" + store = SqliteStore(v2_db, dim=FIXTURE_DIM, coarse_dim=FIXTURE_COARSE_DIM) + try: + assert _user_version(store) == 3 + cols = [r[1] for r in store._db.execute("PRAGMA table_info(entity)").fetchall()] + assert "name_key" in cols + + rows = store._db.execute("SELECT name, name_key FROM entity").fetchall() + assert len(rows) >= 3, "the fixture carries several pre-existing entities" + for r in rows: + assert r["name_key"] == normalize_text(r["name"]), ( + f"{r['name']!r} not backfilled" + ) + assert r["name_key"] != "", "no row may be left on the DEFAULT ''" + + indexes = { + r[0] + for r in store._db.execute( + "SELECT name FROM sqlite_master WHERE type='index'" + ).fetchall() + } + assert "ix_entity_key" in indexes + finally: + store.close() + + +def test_v2_reopens_cleanly_after_migration(v2_db): + """ALTER-idempotence again, for the v3 branch: the second open must not raise + 'duplicate column name: name_key'.""" + SqliteStore(v2_db, dim=FIXTURE_DIM, coarse_dim=FIXTURE_COARSE_DIM).close() + + reopened = SqliteStore(v2_db, dim=FIXTURE_DIM, coarse_dim=FIXTURE_COARSE_DIM) + try: + assert _user_version(reopened) == 3 + finally: + reopened.close() + + +def test_v2_case_split_entities_are_not_healed(v2_db): + """Forward-fix only: the migration BACKFILLS, it never re-points a fact. A + pre-existing 'Acme'/'ACME' split keeps both rows and both facts (healing + would need a new mutation verb — deliberately deferred). The next mention + resolves to the OLDEST row, so the store converges going forward with one + legacy remnant.""" + store = SqliteStore(v2_db, dim=FIXTURE_DIM, coarse_dim=FIXTURE_COARSE_DIM) + try: + split = store._db.execute( + "SELECT id, name, created_at FROM entity WHERE name_key='acme' " + "ORDER BY created_at, id" + ).fetchall() + assert [r["name"] for r in split] == ["Acme", "ACME"], "both rows survive" + for row in split: + owned = store._db.execute( + "SELECT COUNT(*) c FROM fact WHERE subject_id=?", (row["id"],) + ).fetchone()["c"] + assert owned == 1, "each legacy row keeps its own facts" + + namespace = store._db.execute("SELECT namespace FROM entity").fetchone()[ + "namespace" + ] + resolved = store.upsert_entity( + Entity(namespace=namespace, name="aCmE", type=None) + ) + assert resolved.id == split[0]["id"], "oldest row wins the tie-break" + assert ( + store._db.execute( + "SELECT COUNT(*) c FROM entity WHERE name_key='acme'" + ).fetchone()["c"] + == 2 + ), "still two rows — resolving neither heals nor duplicates" + finally: + store.close() + + +# ── Migration: fresh-create + no-downgrade (the same versioned branches) ── +def test_fresh_create_stamps_current_version_and_reopens_clean(tmp_path): path = tmp_path / "fresh.db" store = SqliteStore(path, dim=768) - assert _user_version(store) == 2 - # record_kind present on a fresh DB too (same ALTER branch, fresh is version 1 - # at that point). + assert _user_version(store) == 3 + # record_kind + name_key present on a fresh DB too (same ALTER branches — + # fresh is version 1 at that point and flows through both). Neither column + # may be declared in the always-run blob, or this open would have raised + # 'duplicate column name'. cols = [r[1] for r in store._db.execute("PRAGMA table_info(fact)").fetchall()] assert "record_kind" in cols + ent_cols = [r[1] for r in store._db.execute("PRAGMA table_info(entity)").fetchall()] + assert "name_key" in ent_cols store.close() reopened = SqliteStore(path, dim=768) # must not raise - assert _user_version(reopened) == 2 + assert _user_version(reopened) == 3 reopened.close() def test_newer_version_db_not_downgraded(tmp_path): path = tmp_path / "future.db" store = SqliteStore(path, dim=768) - store._db.execute("PRAGMA user_version = 5") # a hypothetical v>2 release + store._db.execute("PRAGMA user_version = 5") # a hypothetical v>3 release store._db.commit() store.close() diff --git a/tests/test_schema_version.py b/tests/test_schema_version.py index 2b08aa5..bd2ed17 100644 --- a/tests/test_schema_version.py +++ b/tests/test_schema_version.py @@ -3,18 +3,20 @@ v0.1.0-0.1.2 shipped files with user_version=0 and no migration anchor; a schema change would have had no way to tell an old-but-valid file from a foreign SQLite database. Version 1 == the 0.1.x layout; version 2 == the sleep-time-maintenance -layout (adds fact.record_kind + the maintenance tables). A fresh DB is stamped at -the current version (2); older files migrate in place; a file stamped by a NEWER -release is never downgraded. - -The genuine v1-format-file → v2 upgrade path (incl. the ALTER-idempotence reopen -trap) is pinned end-to-end in test_schema_migration.py against a checked-in -v1-format fixture DB. This module pins the stamp arithmetic. +layout (adds fact.record_kind + the maintenance tables); version 3 == the entity +name-collation layout (adds entity.name_key + ix_entity_key, WP15). A fresh DB is +stamped at the current version (3); older files migrate in place; a file stamped +by a NEWER release is never downgraded. + +The genuine old-format-file upgrade paths (v1→current and v2→current, incl. the +ALTER-idempotence reopen trap) are pinned end-to-end in test_schema_migration.py +against checked-in v1/v2-format fixture DBs. This module pins the stamp +arithmetic. """ from lean_memory.store.sqlite_store import SqliteStore -CURRENT_SCHEMA_VERSION = 2 +CURRENT_SCHEMA_VERSION = 3 def _user_version(store: SqliteStore) -> int: From 8831881adf14411109c5453f0d2f9fb3713b5d59 Mon Sep 17 00:00:00 2001 From: Wuesteon Date: Fri, 7 Aug 2026 01:24:45 +0800 Subject: [PATCH 3/5] =?UTF-8?q?=F0=9F=93=9D=20docs:=20WP15=20entity=20coll?= =?UTF-8?q?ation=20=E2=80=94=20README=20note,=20CHANGELOG,=20workpackets?= =?UTF-8?q?=20section?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit README: what the fold does and does not do, in the How It Works section, including the new known limit and its as-of recoverability. CHANGELOG: a new [Unreleased] section (v0.2.4 shipped 2026-08-06) covering the schema v3 migration, the display-form guarantee, the one-way downgrade expectation, the forward-fix/no-heal policy, the replacement known limit, and the extractor fix with its disclosed routing shift. workpackets: a WP15 section recording what shipped, what was deliberately left out, and the four decisions the design doc delegated to implementation — notably why rules.py's `I am` alternative was not copied, and the console schema-tripwire sharp edge found while wording schema.py's comment. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01B1GNoTx3qTPrb8GQcxooWj --- CHANGELOG.md | 66 ++++++++++++++++++++++++++ README.md | 2 + docs/superpowers/workpackets.md | 84 +++++++++++++++++++++++++++++++++ 3 files changed, 152 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c93d15..5b5eb23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,72 @@ All notable changes to lean-memory are documented here. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed + +- **One subject, one entity — entity names now resolve on a normalized key + (schema v3, WP15)** — `upsert_entity` used to match the raw surface form under + SQLite's BINARY collation, so `Acme` and `ACME` were two identities. Because + the whole spine is keyed on `(subject_id, predicate)`, the second one landed + in its own slot and *structurally* bypassed both the WP11 restatement skip and + contradiction resolution: two co-valid "current" answers for one real-world + subject, on the default surface, silently. Identity is now + `(namespace, name_key, type)`, where `name_key` is NFC + Unicode case-fold + + whitespace collapse — the same `normalize_text` DEDUP-EXACT has always used, + promoted to a shared `lean_memory.normalize` module so the tree carries one + definition instead of two that drift. A full Unicode fold, not SQLite + `NOCASE`: `Café`/`CAFÉ` and `ЖУК`/`жук` collate, which `NOCASE` (ASCII-only) + does not. Punctuation and diacritics are deliberately NOT folded — `Yahoo!` is + not `Yahoo`, `Café` is not `Cafe`. Closes + [#14](https://github.com/Wuesteon/lean-memory/issues/14). + - **Display is unchanged.** `entity.name` keeps the FIRST-seen surface form + verbatim; only the new key column is folded. Nothing user-visible is + lowercased, and sorting/FTS/vec0/every `subject_id`-keyed path is untouched. + - **Schema v3** — `user_version`-gated 2→3 migration (v1- and v2-format files + upgrade in place, both verified by checked-in fixtures) adding + `entity.name_key` (ALTER + Python backfill, because SQLite has no + `casefold()`) and the `ix_entity_key` index. ADD-only: no row is deleted, no + fact re-pointed, no validity interval moved — the as-of surface is identical + across the migration. + - **Pre-existing splits are not healed** (forward-fix only). After migration + `Acme` and `ACME` both carry `name_key='acme'` and keep their own facts; new + mentions resolve to the OLDEST row (`ORDER BY created_at, id LIMIT 1`), so a + store converges going forward with one legacy remnant. Healing means + re-pointing `fact.subject_id` — a new mutation verb and its own decision. + - **Downgrade is one-way**, same expectation as schema v2: a v3 file opened by + ≤0.2.4 code works (the extra column is ignored), but that older code writes + rows with an empty `name_key` which newer code then mis-resolves. + - **New known limit, pinned:** two genuinely distinct subjects in one + namespace whose names differ only by case now collate (`Mercury`/`mercury`). + On a functional predicate that retires the earlier fact from the current + surface. Nothing is deleted — it stays readable via + `search(as_of=…, is_latest_only=False)`. This replaces the opposite + known limit v0.2.2 pinned; the trade is deliberate, because a false *split* + is silent and permanent while a false *merge* is visible in the supersession + chain. + +### Fixed + +- **Lowercase first person no longer creates an entity named `i` (WP15)** — the + offline stub generator's `_FIRST_PERSON` regex was case-sensitive (unlike the + Phase-0 rules extractor's, which has carried `re.I` since the start), so + `"i work at acme."` missed it, fell through to the subject fallback, and got + its own entity. That is the other half of #14: the store-side key alone does + not close it, because the split is `user` vs `i`. Two disclosed consequences, + both now pinned by tests: `my`/`me`/`mine` were already lowercase in the + pattern, so `re.I` also re-attributes `ME`/`Mine`/a bare `i` before the + relation verb to the default subject; and offline routing shifts (lowercase-`i` + sentences become "trivially explicit" and route `direct` instead of + escalating), so any FUTURE offline escalation measurement moves. No published + calibration number changes — those were measured on the real GLiNER2 backbone, + not this stub. +- **`StubTyper` docstring claimed a coreference behavior it never had** — it + said `subject_name` was matched case-insensitively against `known_entities`; + the stub never reads that argument. `OllamaTyper`'s known-entity + canonicalization, which does, now uses the shared entity key instead of a + local `.lower()` (which missed `ß`, `fi`, and whitespace runs). + ## [0.2.4] - 2026-08-06 Patch release: the console's MCP tools get the same metadata treatment the diff --git a/README.md b/README.md index 78159e5..a3ea06f 100644 --- a/README.md +++ b/README.md @@ -216,6 +216,8 @@ Each `mem.add()` call runs a 4-pass hybrid extraction pipeline: Contradiction detection runs cheap-first (slot match → cosine → token subsumption → LLM). Conflicting facts are superseded, not deleted — the old fact stays with `is_latest=False` and a `superseded_by` pointer. +Entity identity resolves on a normalized name key — NFC + Unicode case-fold + whitespace collapse — so `Acme`, `ACME` and `acme` are one subject and dedupe/supersession actually apply to them. It is a full Unicode fold, not SQLite's ASCII-only `NOCASE`: `Café`/`CAFÉ` and `ЖУК`/`жук` collate too. Punctuation and diacritics are deliberately *not* folded (`Yahoo!` ≠ `Yahoo`, `Café` ≠ `Cafe`), and the display name keeps the first spelling you used. The trade-off: two genuinely distinct subjects differing only by case (`Mercury` the planet vs `mercury` the metal) collate into one — nothing is deleted, and the retired fact stays readable via `search(as_of=…, is_latest_only=False)`. + Retrieval fuses two-stage Matryoshka dense search (256-dim coarse KNN → full-dim (1024 for the default embedder) re-score) with BM25 sparse, applies RRF fusion, reranks with a cross-encoder, and scores with salience-decay (`0.6·relevance + 0.2·recency + 0.2·importance`). ## Develop diff --git a/docs/superpowers/workpackets.md b/docs/superpowers/workpackets.md index 9d6c292..798f773 100644 --- a/docs/superpowers/workpackets.md +++ b/docs/superpowers/workpackets.md @@ -631,6 +631,90 @@ accessor. --- +## WP15 — Entity name collation (`name_key`) + +**Branch:** `wp15-entity-collation` · **Blocked by:** — (design: +`docs/superpowers/specs/2026-08-06-entity-case-collation-decision.md`, rev 2) · +**Gate:** lane-A six-week read — **waived by the maintainer 2026-08-07**, with +the recommendation approved the same day · **Effort:** S · **Lane A — claims +`memory.py` + `store/` + `extract/`** · Sequenced **before WP4**, whose +`get_all(subject=…)` is the project's first public name-keyed read and must +inherit a settled policy rather than invent one. + +**Goal:** One real-world subject resolves to one entity regardless of the casing +the user typed, so the WP11 restatement skip and contradiction resolution — both +keyed on `(subject_id, predicate)` — actually apply to it. Closes +[#14](https://github.com/Wuesteon/lean-memory/issues/14). + +**Shipped:** option (c) of the decision doc. `entity.name_key` (NFC + casefold + +whitespace collapse) resolved on `(namespace, name_key, type)` with an +`ORDER BY created_at, id LIMIT 1` tie-break for legacy split rows; `entity.name` +keeps the first-seen surface form; schema v3 versioned migration (ALTER + Python +backfill + `ix_entity_key`, all inside the `< 3` branch, nothing in the +always-run blob); `normalize_text` promoted to `lean_memory/normalize.py` and +re-exported from `maintain/transforms.py`; `re.I` on the stub generator's +`_FIRST_PERSON`; `llm_typer` known-entity canonicalization on the shared key. +**Not** shipped, deliberately: healing pre-existing splits, object-side entity +resolution, alias/semantic linking, the `_CAP_RUN` lowercase-proper-noun +misextraction, console `name_key` adoption (separate package/release). + +**Files:** `src/lean_memory/normalize.py` (new), `store/base.py` (contract +docstring), `store/schema.py` (comment only — no DDL change), +`store/sqlite_store.py`, `maintain/transforms.py` (re-export), +`extract/gliner_extractor.py`, `extract/llm_typer.py`, +`tests/test_entity_collation.py` (new), `tests/fixtures/make_v2_fixture.py` + +`v2_format.db` (new), `tests/test_schema_migration.py`, +`tests/test_schema_version.py`, `tests/test_restatement_dedupe.py`, +`tests/test_phase1_extraction.py`, `README.md`, `CHANGELOG.md`. + +**Decisions the design doc delegated to WP15:** + +- **The stub's first-person pattern does NOT gain rules.py's `I am` + alternative** (§3.1 left this open: add it for real parity, or state the + divergence). It is stated, in a comment at the pattern: `I am` is + *unreachable*. Alternation is leftmost-first, so wherever `I am` would match + the earlier `I` alternative already matches (the `\b` after `I` holds before a + space), and both patterns are only ever consumed as a boolean `.search()`. + Verified over a 13-case probe including `"I am a doctor."`, `"i am tired"`, + `"Iam"`, `"hI am"`: the three patterns (stub with `re.I`, stub + `I am`, + rules.py) agree on every input. Adding it would advertise a behavioral + difference that does not exist — the same cargo-culting that produced the + `_norm`/`normalize_text` drift this packet is undoing. The only remaining + divergence is the non-capturing group, which the boolean use does not need. +- **The shared key is exported as both `normalize_text` and an + `entity_key` alias** from `lean_memory.normalize`. One function object, two + names: call sites read as what they are (`entity_key(name)` in the store and + typer, `normalize_text(fact_text)` in DEDUP-EXACT) while remaining incapable + of drifting apart. `router.py`'s `_norm` stays local and unchanged — it is a + coref heuristic, not an identity decision. +- **The replacement known limit is pinned WITH its recoverability.** + `test_case_distinct_subjects_merge_known_limit` asserts the merge, the + supersession chain (`is_latest=0` + `superseded_by`), *and* that the retired + fact is still returned by `search(as_of=…, is_latest_only=False)` — so the + doc's "a false merge is recoverable" argument is executable rather than + rhetorical. +- **`schema.py`'s comment avoids DDL keywords.** The console's engine-schema + tripwire (`inspect_sql.compute_engine_schema_fingerprint`) digests every line + of `store/schema.py` containing `create`, case-insensitively — comments + included. A first draft of the v3 note mentioned the statements by name and + flipped that hash with the DDL byte-identical, reddening the console suite. + Reworded rather than re-baselined, since WP15 must not touch console source. + **Recorded for whoever owns the tripwire next:** it digests `schema.py` only, + and the real v3 DDL lives in `sqlite_store.py`'s versioned branch — so the + tripwire is simultaneously over-sensitive to prose and blind to the actual + schema change. Worth revisiting when the console adopts `name_key`. + +**Verification:** core `345 passed` (baseline `319`, minus the deleted WP11 +known-limit pin, plus 27 new: 18 collation, 5 extractor, 4 migration); console +`198 passed`, unchanged. The decision doc's §6.4 blast-radius prediction was +re-measured against the real implementation rather than trusted: the finished +`src/` over the **unmodified** test suite gives `313 passed, 6 failed` — the +exact six tests §6.4 names. `PRAGMA user_version` = 3 confirmed on a fresh +store, on the v2 fixture, and on the v1 fixture (which crosses both branches in +a single open). + +--- + ## Open follow-ups (recorded, not yet packets) - **Entity case-collation policy** ([#14](https://github.com/Wuesteon/lean-memory/issues/14)) — WP11's pinned known limit ("acme" vs From 184394dc6a44fa2d20f4d49c549fbbd620ee715f Mon Sep 17 00:00:00 2001 From: Wuesteon Date: Fri, 7 Aug 2026 01:46:53 +0800 Subject: [PATCH 4/5] =?UTF-8?q?=F0=9F=90=9B=20fix:=20make=20the=20schema?= =?UTF-8?q?=20migration=20atomic=20=E2=80=94=20an=20interrupted=20upgrade?= =?UTF-8?q?=20must=20not=20brick=20the=20store?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review carry-in (1 important + 4 minors) on the WP15 branch. **Important — the v3 migration was not atomic.** Python's `sqlite3` opens an implicit transaction for DML only, never for DDL, so `ALTER TABLE entity ADD COLUMN name_key` ran in autocommit and was durable the instant it executed, while the backfill loop, the index and `PRAGMA user_version = 3` committed later. Interrupt that window and the file kept `name_key` under `user_version = 2`; every later open re-entered the `< 3` branch and raised `duplicate column name: name_key` — permanently unopenable, no recovery short of manual sqlite surgery. The block's own comment claimed the opposite ("this runs inside the single _init_schema transaction"), which is why widening the window from v2's two adjacent statements to a Python loop over every entity row looked safe. Fix: wrap the whole versioned-migration section in an explicit `BEGIN IMMEDIATE`, re-read `user_version` under the write lock, roll back on any exception. The re-read also closes the sibling race, where two processes first-opening one pre-v3 file both saw version 2 and the loser repeated the ALTER. A new `SCHEMA_VERSION` guard keeps the common already-current open lock-free. The v2 branch carried the same latent hazard and is now covered too. A/B-verified by SIGKILLing between the ALTER and the commit, and by racing two real processes on one v2 fixture: both scenarios brick the store on the pre-fix code and pass on the fixed code. Pinned by `test_interrupted_migration_rolls_back_whole`, confirmed to redden against the pre-fix `sqlite_store.py`. **Minors** - `workpackets.md` contradicted itself in three places about the behavior this branch reversed: WP11's known-limit paragraph still described the case-split as current and cited a test this branch deletes; the WP15 roster row still read "open"; the #14 open-follow-up still said "awaiting maintainer decision". All three are WP15's own rows — repointed at `test_case_distinct_subjects_merge_known_limit` and marked shipped/resolved. - `Entity`'s docstring still declared the old `(namespace, name, type)` identity contract. - `ix_entity_lookup` is dead as of v3 (no read keys on `entity.name`) but is still built and maintained. Documented as deliberately retained rather than dropped — retiring it needs a `DROP INDEX` in the versioned branch plus an edit to a `create`-bearing line, which flips the console's engine-schema tripwire. The new comment is worded to leave that hash byte-identical (verified; a first draft said "created_at" and flipped it). - The fresh-create migration test asserted `name_key` but not `ix_entity_key`, so moving the index behind a migration-only path would have left every new store scanning with the suite green. core 346 passed, console 198 passed. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01B1GNoTx3qTPrb8GQcxooWj --- CHANGELOG.md | 4 +- docs/superpowers/workpackets.md | 76 ++++++++++----- src/lean_memory/store/schema.py | 10 ++ src/lean_memory/store/sqlite_store.py | 133 ++++++++++++++++---------- src/lean_memory/types.py | 9 +- tests/test_schema_migration.py | 52 ++++++++++ 6 files changed, 209 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b5eb23..26b135e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,9 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). `entity.name_key` (ALTER + Python backfill, because SQLite has no `casefold()`) and the `ix_entity_key` index. ADD-only: no row is deleted, no fact re-pointed, no validity interval moved — the as-of surface is identical - across the migration. + across the migration. The upgrade runs in ONE transaction, so an interrupted + one (crash, kill, full disk) rolls back whole and the next open retries it — + it cannot leave the file half-migrated and unopenable. - **Pre-existing splits are not healed** (forward-fix only). After migration `Acme` and `ACME` both carry `name_key='acme'` and keep their own facts; new mentions resolve to the OLDEST row (`ORDER BY created_at, id LIMIT 1`), so a diff --git a/docs/superpowers/workpackets.md b/docs/superpowers/workpackets.md index 798f773..d88366a 100644 --- a/docs/superpowers/workpackets.md +++ b/docs/superpowers/workpackets.md @@ -46,7 +46,7 @@ on the same files). | WP12 mcp 2.0 migration | `worktree-wp12-mcp2-migration` | A | — | none — dependency-driven | **MERGED** (2026-07-29: PR #10 → main 4efe4ca; dual-path compat, pin widened to mcp>=1.2,<3; fixed the 2.0 worker-thread SQLite crash (check_same_thread=False, serialized); all four suite combos green locally, CI green on fresh-resolved 2.0; shipped in v0.2.3 (2026-07-29); lane A released) | | WP11 Write-time restatement dedupe | `worktree-wp11-restatement-dedup` | A | — | — | **MERGED** (2026-07-29: PR #7 → main 52d21fd; core 289 + console 153 green on merged branch; rode along: mcp>=1.2,<2 pin for the mcp 2.0.0 fastmcp removal, gated WP9 LLM-judge design doc; lane A released) | | WP14 Console tool metadata | `wp14-console-tool-metadata` | D | — | precondition: console listed on Glama? | **MERGED** (2026-08-06: PR #19 → main 9586b59, closes [#13](https://github.com/Wuesteon/lean-memory/issues/13); honest annotations + 100% param descriptions + when-to-use guidance on all 6 console tools, both surfaces — shared registration makes stdio/HTTP parity structural; contract pinned in `console/tests/test_mcp_tool_metadata.py` (45 tests, wire-name asserts); console 198 green on mcp 1.28.1 AND 2.0.0 scratch venv, live stdio wire check on both majors; core untouched (319); CI 6/6; precondition answered NO — console not independently listed on Glama (listings are repo-keyed), done as hygiene/parity; deliberate core divergences recorded in the section; unreleased — ships with the next tag; lane D released) | -| WP15 Entity name collation (`name_key`) | `wp15-entity-collation` | A | — (design done) | maintainer decision on the 2026-08-06 decision doc + six-week read (lane A); sequence **before WP4** | open (decision doc: `docs/superpowers/specs/2026-08-06-entity-case-collation-decision.md`; [#14](https://github.com/Wuesteon/lean-memory/issues/14)) | +| WP15 Entity name collation (`name_key`) | `wp15-entity-collation` | A | — (design done) | gate **WAIVED by the maintainer 2026-08-07** (recommendation approved same day); sequenced **before WP4** | **SHIPPED on branch** (2026-08-07: option (c) of `docs/superpowers/specs/2026-08-06-entity-case-collation-decision.md` — `entity.name_key` + schema v3 migration + shared `lean_memory/normalize.py`; core 346 + console 198 green; closes [#14](https://github.com/Wuesteon/lean-memory/issues/14) and supersedes WP11's case-split known limit; see §WP15 — not yet merged/released) | | WP13 MCP tool metadata (Glama audit) | `worktree-wp12-mcp-tool-metadata` | A | — | — | **MERGED** (2026-07-29: PR #11 → main 342461e; branch name predates the renumber — claimed as WP12 concurrently with the mcp 2.0 migration; annotations + param descriptions + usage guidance on all 7 server tools, no behavior change beyond k/limit schema minimums; contract pinned in `tests/test_mcp_tool_metadata.py`, green on both mcp majors (core 319 on 1.28.0, 318+1 skip on 2.0.0 scratch venv), console 153, CI 6/6; shipped in v0.2.3 (2026-07-29) — Glama re-scan triggered; lane A released) | Lanes: **A** = engine/API surface (`src/lean_memory/` hot zone — strictly @@ -533,13 +533,18 @@ new engine surface. offline `dedup_near` band; re-assertion bookkeeping (seen-counts, last-asserted timestamps) — design it in WP4+ if the demand read asks for it. -**Known limit (pinned by test):** a case variant of the ENTITY surface form -("acme" vs "Acme") resolves to a different entity (`upsert_entity` matches -`name=?` under BINARY collation) → different slot → bypasses dedupe AND -contradiction resolution entirely. An entity-resolution collation policy is -its own decision (real distinct-by-case counterexamples: "Polish"/"polish"); -pinned in `tests/test_restatement_dedupe.py:: -test_entity_case_variant_splits_the_slot_known_limit`. +**Known limit — SUPERSEDED by WP15 (2026-08-07).** As shipped, a case variant of +the ENTITY surface form ("acme" vs "Acme") resolved to a different entity +(`upsert_entity` matched `name=?` under BINARY collation) → different slot → +bypassed dedupe AND contradiction resolution entirely, and that was pinned in +`tests/test_restatement_dedupe.py::test_entity_case_variant_splits_the_slot_known_limit`. +WP15 took the collation decision (real distinct-by-case counterexamples: +"Polish"/"polish") and closed it: identity now resolves on `entity.name_key`, so +variants share one slot and the WP11 skip applies to them. That test is deleted; +the replacement known limit — genuinely case-distinct subjects now MERGE, and +why that is the accepted trade (recoverable via supersession) — is pinned in +`tests/test_entity_collation.py::test_case_distinct_subjects_merge_known_limit`. +See §WP15. --- @@ -659,8 +664,8 @@ resolution, alias/semantic linking, the `_CAP_RUN` lowercase-proper-noun misextraction, console `name_key` adoption (separate package/release). **Files:** `src/lean_memory/normalize.py` (new), `store/base.py` (contract -docstring), `store/schema.py` (comment only — no DDL change), -`store/sqlite_store.py`, `maintain/transforms.py` (re-export), +docstring), `types.py` (Entity docstring), `store/schema.py` (comments only — no +DDL change), `store/sqlite_store.py`, `maintain/transforms.py` (re-export), `extract/gliner_extractor.py`, `extract/llm_typer.py`, `tests/test_entity_collation.py` (new), `tests/fixtures/make_v2_fixture.py` + `v2_format.db` (new), `tests/test_schema_migration.py`, @@ -693,6 +698,25 @@ docstring), `store/schema.py` (comment only — no DDL change), fact is still returned by `search(as_of=…, is_latest_only=False)` — so the doc's "a false merge is recoverable" argument is executable rather than rhetorical. +- **The versioned migrations now run inside an explicit `BEGIN IMMEDIATE`** + (review carry-in, 2026-08-07). Python's `sqlite3` opens an implicit + transaction for DML only, never for DDL, so the v3 `ALTER TABLE ... ADD COLUMN + name_key` was autocommitted and durable *before* the backfill / index / stamp + it depends on. A/B-verified by SIGKILLing between the ALTER and the commit: + pre-fix the file kept `name_key` under `user_version = 2` and **every** later + open raised `duplicate column name: name_key` — permanently unopenable; + post-fix the ALTER rolls back and the next open migrates cleanly. The same + probe run concurrently (two processes first-opening one v2 file) failed the + same way pre-fix and passes post-fix, because the version is re-read *under* + the write lock. The outer `< SCHEMA_VERSION` guard keeps the common + already-current open lock-free. The v2 branch inherited the same latent hazard + in a two-statement window and is now covered too. +- **`ix_entity_lookup(namespace, name, type)` is retained but vestigial** and + now says so in `schema.py`. No engine read keys on `entity.name` after v3, so + it is pure write cost; retiring it needs a `DROP INDEX` in the versioned + branch *and* an edit to a `create`-bearing line, which flips the console's + engine-schema tripwire — a cross-package call, deliberately not slipped in + here. Revisit with the console's `name_key` adoption. - **`schema.py`'s comment avoids DDL keywords.** The console's engine-schema tripwire (`inspect_sql.compute_engine_schema_fingerprint`) digests every line of `store/schema.py` containing `create`, case-insensitively — comments @@ -704,9 +728,13 @@ docstring), `store/schema.py` (comment only — no DDL change), tripwire is simultaneously over-sensitive to prose and blind to the actual schema change. Worth revisiting when the console adopts `name_key`. -**Verification:** core `345 passed` (baseline `319`, minus the deleted WP11 -known-limit pin, plus 27 new: 18 collation, 5 extractor, 4 migration); console -`198 passed`, unchanged. The decision doc's §6.4 blast-radius prediction was +**Verification:** core `346 passed` (baseline `319`, minus the deleted WP11 +known-limit pin, plus 28 new: 18 collation, 5 extractor, 5 migration — the last +of those being the review carry-in's atomicity pin, +`test_interrupted_migration_rolls_back_whole`, confirmed to redden against the +pre-fix `sqlite_store.py`); console `198 passed`, unchanged — the engine-schema +tripwire hash was re-measured byte-identical after each `schema.py` comment edit, +not assumed. The decision doc's §6.4 blast-radius prediction was re-measured against the real implementation rather than trusted: the finished `src/` over the **unmodified** test suite gives `313 passed, 6 failed` — the exact six tests §6.4 names. `PRAGMA user_version` = 3 confirmed on a fresh @@ -717,15 +745,19 @@ a single open). ## Open follow-ups (recorded, not yet packets) -- **Entity case-collation policy** ([#14](https://github.com/Wuesteon/lean-memory/issues/14)) — WP11's pinned known limit ("acme" vs - "Acme" split the slot and bypass dedupe + contradiction resolution; see the - WP11 section). Needs a decision, not just code: case-insensitive lookup has - real counterexamples ("Polish"/"polish"). **Decision doc recorded - 2026-08-06** (`docs/superpowers/specs/2026-08-06-entity-case-collation-decision.md`, - adversarially reviewed rev 2): recommends a stored `entity.name_key` - casefold column + `re.I` on the extractor's first-person regex (the headline - example is two independent defects), packaged as WP15 sequenced before WP4. - Awaiting maintainer decision; #14 stays open until WP15 lands. +- ~~**Entity case-collation policy** ([#14](https://github.com/Wuesteon/lean-memory/issues/14))~~ — **RESOLVED by WP15 + (2026-08-07)**, no longer an open follow-up. WP11's pinned known limit ("acme" + vs "Acme" split the slot and bypass dedupe + contradiction resolution) needed + a decision, not just code, because case-insensitive lookup has real + counterexamples ("Polish"/"polish"). Decision doc recorded 2026-08-06 + (`docs/superpowers/specs/2026-08-06-entity-case-collation-decision.md`, + adversarially reviewed rev 2); the maintainer approved its recommendation and + waived the six-week gate on 2026-08-07. Shipped as WP15: stored + `entity.name_key` casefold column + `re.I` on the extractor's first-person + regex (the headline example was two independent defects). #14 closes when the + branch merges; the *new* known limit (case-distinct subjects merge) is + recorded in §WP15, not here — it is an accepted, pinned trade, not an open + question. - **WP2 mem0 comparison arm** ([#15](https://github.com/Wuesteon/lean-memory/issues/15)) — designed as Task 5 of the WP2 plan (`--arm mem0`, version-pinned output, exit-2 on missing install); needs the user's go-ahead plus a configured mem0 LLM path (Ollama or API key) to run. diff --git a/src/lean_memory/store/schema.py b/src/lean_memory/store/schema.py index eb72152..411cae1 100644 --- a/src/lean_memory/store/schema.py +++ b/src/lean_memory/store/schema.py @@ -40,6 +40,16 @@ resolved_id TEXT, created_at INTEGER NOT NULL ); +-- ix_entity_lookup below is VESTIGIAL as of v3 and retained on purpose. Nothing +-- in the engine keys on `entity.name` any more: `upsert_entity` resolves on +-- name_key via ix_entity_key, `memory.py`'s known-entity list is a namespace +-- scan ordered newest-first, and the console's one name predicate +-- (`LOWER(e.name) = LOWER(?)`) is non-sargable against it. So every entity row +-- pays to maintain it and no read uses it. Retiring it is a deliberate +-- cross-package call, not cleanup to slip into this packet: it needs a matching +-- `DROP INDEX IF EXISTS` in the versioned branch AND an edit to the line below, +-- which flips the console's engine-schema tripwire hash. Revisit alongside the +-- console's own name_key adoption. CREATE INDEX IF NOT EXISTS ix_entity_lookup ON entity(namespace, name, type); -- ── FACT LAYER (monotemporal spine always on; audit axis opt-in) ───── diff --git a/src/lean_memory/store/sqlite_store.py b/src/lean_memory/store/sqlite_store.py index 2a68f31..fec6868 100644 --- a/src/lean_memory/store/sqlite_store.py +++ b/src/lean_memory/store/sqlite_store.py @@ -29,6 +29,12 @@ from .schema import SCHEMA_SQL +# Current persisted-format version. Bump it together with a new +# `if version < N:` branch in _init_schema — it is the outer guard that decides +# whether an open needs the migration transaction at all. +SCHEMA_VERSION = 3 + + def _serialize(vec: np.ndarray) -> bytes: """L2-normalized float32 → vec0's float32 wire format. @@ -114,60 +120,85 @@ def _init_schema(self) -> None: sql = SCHEMA_SQL.format(dim=self.dim, coarse_dim=self.coarse_dim) self._db.executescript(sql) + # ── Versioned migrations, all inside ONE explicit transaction. + # # Schema-version stamp — the migration anchor for future releases. # Version 1 == the 0.1.x layout; pre-stamp files (0.1.0–0.1.2, version 0) # have an identical spine and are treated as version 1. Never write over a - # NEWER release's stamp. - version = self._db.execute("PRAGMA user_version").fetchone()[0] - if version == 0: - version = 1 - self._db.execute("PRAGMA user_version = 1") - - # ── Versioned migrations. Each branch runs the NON-idempotent DDL for its - # version exactly once, keyed off user_version. A fresh DB is version 1 - # here (just stamped above), so it flows through the same `< 2` branch and - # gains record_kind via the SAME ALTER — there is no separate fresh path. - # ADD COLUMN is not idempotent (raises 'duplicate column name' on reopen), - # so it MUST live here and never in the always-run blob. - if version < 2: - self._db.execute( - "ALTER TABLE fact ADD COLUMN record_kind TEXT NOT NULL " - "DEFAULT 'fact'" # 'fact'|'summary' - ) - self._db.execute("PRAGMA user_version = 2") - version = 2 - - # v3 — entity name collation (WP15). ADD-only and forward-fix: the - # backfill writes the derived key onto existing rows and touches nothing - # else. No row is deleted, no fact is re-pointed, no validity interval - # moves, so the as-of surface is byte-identical across the migration. - # Pre-existing case-split rows ('Acme' + 'ACME') are NOT healed — both - # keep their facts; upsert_entity's tie-break just converges new mentions - # on the oldest. (Healing means re-pointing fact.subject_id, a new - # mutation verb and its own decision — deferred to a possible - # merge_entity review proposal.) - # The backfill is a Python loop by necessity: SQLite has no casefold(); - # `lower()` is ASCII-only and would silently mis-key every non-ASCII name. - # The table holds one row per distinct subject, and this runs inside the - # single _init_schema transaction. The CREATE INDEX belongs here, NOT in - # SCHEMA_SQL, for the same reason as the ALTER (see schema.py). - if version < 3: - self._db.execute( - "ALTER TABLE entity ADD COLUMN name_key TEXT NOT NULL DEFAULT ''" - ) - for row in self._db.execute("SELECT id, name FROM entity").fetchall(): - self._db.execute( - "UPDATE entity SET name_key=? WHERE id=?", - (entity_key(row["name"]), row["id"]), - ) - self._db.execute( - "CREATE INDEX IF NOT EXISTS ix_entity_key " - "ON entity(namespace, name_key, type)" - ) - self._db.execute("PRAGMA user_version = 3") - version = 3 - - self._db.commit() + # NEWER release's stamp. Each branch below runs the NON-idempotent DDL for + # its version exactly once, keyed off user_version. A fresh DB is version 1 + # here (stamped just inside), so it flows through the SAME branches and + # gains record_kind AND name_key via the SAME ALTERs — there is no separate + # fresh path. ADD COLUMN is not idempotent (raises 'duplicate column name' + # on reopen), so it MUST live here and never in the always-run blob. + # + # The explicit BEGIN IMMEDIATE is load-bearing, not decoration. Python's + # sqlite3 opens an implicit transaction for DML only — NEVER for DDL — so + # without it an `ALTER TABLE ... ADD COLUMN` runs in autocommit and is + # durable the instant it executes, while the backfill / index / version + # stamp that make it coherent commit later. Interrupt that window and the + # file keeps the new column under the OLD user_version; every later open + # then re-enters the branch and dies on 'duplicate column name' — + # permanently unopenable, no recovery short of manual sqlite surgery. + # (SQLite's own DDL is transactional; it is Python's autocommit heuristic, + # not SQLite, that would strand the ALTER outside a transaction.) + # Re-reading user_version under the write lock closes the sibling race: + # two processes opening one pre-v3 file both read the old version, and + # without the re-read the loser repeats the ALTER. The outer guard keeps + # the common already-current open lock-free. + if self._db.execute("PRAGMA user_version").fetchone()[0] < SCHEMA_VERSION: + self._db.execute("BEGIN IMMEDIATE") + try: + version = self._db.execute("PRAGMA user_version").fetchone()[0] + if version == 0: + version = 1 + self._db.execute("PRAGMA user_version = 1") + + if version < 2: + self._db.execute( + "ALTER TABLE fact ADD COLUMN record_kind TEXT NOT NULL " + "DEFAULT 'fact'" # 'fact'|'summary' + ) + self._db.execute("PRAGMA user_version = 2") + version = 2 + + # v3 — entity name collation (WP15). ADD-only and forward-fix: the + # backfill writes the derived key onto existing rows and touches + # nothing else. No row is deleted, no fact is re-pointed, no + # validity interval moves, so the as-of surface is byte-identical + # across the migration. Pre-existing case-split rows ('Acme' + + # 'ACME') are NOT healed — both keep their facts; upsert_entity's + # tie-break just converges new mentions on the oldest. (Healing + # means re-pointing fact.subject_id, a new mutation verb and its + # own decision — deferred to a possible merge_entity review + # proposal.) + # The backfill is a Python loop by necessity: SQLite has no + # casefold(); `lower()` is ASCII-only and would silently mis-key + # every non-ASCII name. The table holds one row per distinct + # subject. The CREATE INDEX belongs here, NOT in SCHEMA_SQL, for + # the same reason as the ALTER (see schema.py). + if version < 3: + self._db.execute( + "ALTER TABLE entity ADD COLUMN name_key TEXT NOT NULL " + "DEFAULT ''" + ) + for row in self._db.execute( + "SELECT id, name FROM entity" + ).fetchall(): + self._db.execute( + "UPDATE entity SET name_key=? WHERE id=?", + (entity_key(row["name"]), row["id"]), + ) + self._db.execute( + "CREATE INDEX IF NOT EXISTS ix_entity_key " + "ON entity(namespace, name_key, type)" + ) + self._db.execute("PRAGMA user_version = 3") + version = 3 + except BaseException: + self._db.rollback() + raise + self._db.commit() def _check_existing_dims(self) -> None: """Refuse to open a store whose vec0 table was created for a different embedder. diff --git a/src/lean_memory/types.py b/src/lean_memory/types.py index 09d4bb0..0375316 100644 --- a/src/lean_memory/types.py +++ b/src/lean_memory/types.py @@ -41,7 +41,14 @@ class Episode: @dataclass class Entity: - """Canonical entity (person/org/place/...). Phase 0 resolves by (namespace, name, type).""" + """Canonical entity (person/org/place/...). + + Resolved on `(namespace, normalize_text(name), type)` since schema v3 — see + `Store.upsert_entity` for the full contract. `name` is the FIRST-seen surface + form, kept verbatim for display; the derived key lives only in the store row's + `name_key` column and deliberately has no field here, so nothing can set it + out of step with `name`. + """ namespace: str name: str diff --git a/tests/test_schema_migration.py b/tests/test_schema_migration.py index 4f406f5..5a5a3ac 100644 --- a/tests/test_schema_migration.py +++ b/tests/test_schema_migration.py @@ -38,6 +38,7 @@ import pytest from lean_memory.normalize import normalize_text +from lean_memory.store import sqlite_store from lean_memory.store.sqlite_store import SqliteStore from lean_memory.types import Entity, Episode, Fact @@ -242,6 +243,46 @@ def test_v2_reopens_cleanly_after_migration(v2_db): reopened.close() +def test_interrupted_migration_rolls_back_whole(v2_db, monkeypatch): + """ATOMICITY: the ALTER must live INSIDE the migration transaction. + + Python's sqlite3 opens an implicit transaction for DML only — never for DDL — + so without the explicit BEGIN IMMEDIATE the `ALTER TABLE entity ADD COLUMN + name_key` autocommits and is durable the instant it runs, while the backfill, + the index and the version stamp commit later. Fail in that window and the + file keeps `name_key` under `user_version = 2`; EVERY later open then + re-enters the `< 3` branch and raises 'duplicate column name: name_key' — + permanently unopenable, no recovery short of manual sqlite surgery. + + Here the backfill blows up mid-migration. The file must roll back whole (v2, + no column) and migrate cleanly on the next open. Regression pin: reverting to + an implicit transaction reddens this immediately. + """ + def explode(_name: str) -> str: + raise RuntimeError("backfill exploded") + + monkeypatch.setattr(sqlite_store, "entity_key", explode) + with pytest.raises(RuntimeError, match="backfill exploded"): + SqliteStore(v2_db, dim=FIXTURE_DIM, coarse_dim=FIXTURE_COARSE_DIM) + + raw = sqlite3.connect(v2_db) + try: + assert raw.execute("PRAGMA user_version").fetchone()[0] == 2, "stamp untouched" + cols = [r[1] for r in raw.execute("PRAGMA table_info(entity)").fetchall()] + assert "name_key" not in cols, "the ALTER rolled back with the transaction" + finally: + raw.close() + + monkeypatch.undo() + store = SqliteStore(v2_db, dim=FIXTURE_DIM, coarse_dim=FIXTURE_COARSE_DIM) + try: + assert _user_version(store) == 3, "the retry migrates cleanly" + rows = store._db.execute("SELECT name, name_key FROM entity").fetchall() + assert rows and all(r["name_key"] == normalize_text(r["name"]) for r in rows) + finally: + store.close() + + def test_v2_case_split_entities_are_not_healed(v2_db): """Forward-fix only: the migration BACKFILLS, it never re-points a fact. A pre-existing 'Acme'/'ACME' split keeps both rows and both facts (healing @@ -291,6 +332,17 @@ def test_fresh_create_stamps_current_version_and_reopens_clean(tmp_path): assert "record_kind" in cols ent_cols = [r[1] for r in store._db.execute("PRAGMA table_info(entity)").fetchall()] assert "name_key" in ent_cols + # ...and the index over it — the other half of the v3 branch, and the reason + # upsert_entity is a lookup rather than a scan. Pinned on the FRESH path too + # (not only the migrated one), so moving it behind a migration-only condition + # cannot silently leave every new store scanning. + indexes = { + r[0] + for r in store._db.execute( + "SELECT name FROM sqlite_master WHERE type='index'" + ).fetchall() + } + assert "ix_entity_key" in indexes store.close() reopened = SqliteStore(path, dim=768) # must not raise From 2e1597c440a9c32be7003e1c18f710d92cc4e40a Mon Sep 17 00:00:00 2001 From: Wuesteon Date: Fri, 7 Aug 2026 08:09:10 +0800 Subject: [PATCH 5/5] =?UTF-8?q?=F0=9F=91=B7=20ci:=20retrigger=20checks=20(?= =?UTF-8?q?PR=20opened=20during=20the=202026-08-07=20Actions=20outage;=20t?= =?UTF-8?q?rigger=20event=20was=20dropped)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01B1GNoTx3qTPrb8GQcxooWj