✨ WP15: entity name collation (name_key) + extractor re.I — case variants resolve to one entity - #21
Merged
Merged
Conversation
… module 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B1GNoTx3qTPrb8GQcxooWj
…d key 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B1GNoTx3qTPrb8GQcxooWj
…ection 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B1GNoTx3qTPrb8GQcxooWj
… not brick the store
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B1GNoTx3qTPrb8GQcxooWj
…e; trigger event was dropped) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B1GNoTx3qTPrb8GQcxooWj
…row conflict) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B1GNoTx3qTPrb8GQcxooWj
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #14. Implements the 2026-08-06 decision doc (
docs/superpowers/specs/2026-08-06-entity-case-collation-decision.md) in full; lane-A gate waived by maintainer 2026-08-07 (recorded in workpackets.md).The fix is two independent halves, per the doc:
entity.name_key— NFC → casefold → whitespace-collapse of the name, stored alongside the first-seen surface form (display never changes).upsert_entityresolves on(namespace, name_key, type)withORDER BY created_at, id LIMIT 1as the deterministic tie-break for legacy split rows. Sharednormalize_textpromoted tolean_memory.normalize(WP10a re-exports it — one definition in the tree).re.Ion the stub extractor's first-person regex — "i work at acme." now attributes touser. The false-merge class this creates (Mine/ME/bare-i) is pinned as a decision, not hidden.Schema v3, atomic: ALTER + Python backfill +
ix_entity_key+ stamp, all insideBEGIN IMMEDIATEwith the version re-read under the lock — the review caught that DDL runs in autocommit, and a SIGKILL mid-migration left the store permanently unopenable; reproduced with realkill -9A/B probes pre/post-fix, plus a concurrent-first-open race probe. Both migration paths verified live: v1→3, v2→3 (new checked-in v2 fixture carrying a deliberate pre-existingAcme/ACMEsplit), fresh create →3, reopen idempotent. Pre-existing splits are NOT silently healed (no-heal policy per the doc).Replacement known limit, pinned with recoverability: genuinely case-distinct entities ("Polish"/"polish") now merge; the pin asserts the merge AND that the retired fact stays reachable via
as_of— the doc's "a false merge is recoverable" argument is executable.Tests: WP11's known-limit pin deleted per its own docstring's instruction; its variant folded into the trivial-variants test. Core 346 passed (319 + 27, arithmetic verified per-file), console 198 passed (zero cross-package radius, measured). Adversarial review: 1 important + 4 minor, all independently re-verified and fixed.
🤖 Generated with Claude Code
https://claude.ai/code/session_01B1GNoTx3qTPrb8GQcxooWj