diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c93d15..26b135e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,74 @@ 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. 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 + 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 0171b3b..6ecb38e 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) | ~~six-week read~~ **gate waived by maintainer 2026-08-07** (conscious strategy change, recorded per the gate rule: recommendation approved + lane-A gate waived; sequence before WP4 preserved) | **CLAIMED 2026-08-07** (`wp15-entity-collation`; 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) | ~~six-week read~~ **gate waived by maintainer 2026-08-07** (conscious strategy change, recorded per the gate rule: recommendation approved + lane-A gate waived; sequence before WP4 preserved) | **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. --- @@ -631,17 +636,128 @@ 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), `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`, +`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. +- **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 + 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 `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 +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 - "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/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/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 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..411cae1 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, @@ -28,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 0c2d538..fec6868 100644 --- a/src/lean_memory/store/sqlite_store.py +++ b/src/lean_memory/store/sqlite_store.py @@ -23,11 +23,18 @@ 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 +# 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. @@ -113,30 +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 - - 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. @@ -181,17 +243,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/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/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 0000000..5f19628 Binary files /dev/null and b/tests/fixtures/v2_format.db differ 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..5a5a3ac 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,15 @@ import numpy as np 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 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 +66,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 +100,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 +120,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 +173,187 @@ 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_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 + 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 + # ...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 - 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: