Codex caught a real documentation accuracy bug in the previous
canonicalization doc commit (f521aab). The doc claimed that rows
written under aliases before fb6298a "still work via the
unregistered-name fallback path" — that is wrong for REGISTERED
aliases, which is exactly the case that matters.
The unregistered-name fallback only saves you when the project was
never in the registry: a row stored under "orphan-project" is read
back via "orphan-project", both pass through resolve_project_name
unchanged, and the strings line up. For a registered alias like
"p05", the helper rewrites the read key to "p05-interferometer"
but does NOT rewrite the storage key, so the legacy row becomes
silently invisible.
This commit corrects the doc and locks the gap behavior in with
a regression test, so the issue cannot be lost again.
docs/architecture/project-identity-canonicalization.md
------------------------------------------------------
- Removed the misleading claim from the "What this rule does NOT
cover" section. Replaced with a pointer to the new gap section
and an explicit statement that the migration is required before
engineering V1 ships.
- New "Compatibility gap: legacy alias-keyed rows" section between
"Why this is the trust hierarchy in action" and "The rule for
new entry points". This is the natural insertion point because
the gap is exactly the trust hierarchy failing for legacy data.
The section covers:
* a worked T0/T1 timeline showing the exact failure mode
* what is at risk on the live Dalidou DB, ranked by trust tier:
projects table (shadow rows), project_state (highest risk
because Layer 3 is most-authoritative), memories, interactions
* inspection SQL queries for measuring the actual blast radius
on the live DB before running any migration
* the spec for the migration script: walk projects, find shadow
rows, merge dependent state via the conflict model when there
are collisions, dry-run mode, idempotent
* explicit statement that this is required pre-V1 because V1
will add new project-keyed tables and the killer correctness
queries from engineering-query-catalog.md would report wrong
results against any project that has shadow rows
- "Open follow-ups" item 1 promoted from "tracked optional" to
"REQUIRED before engineering V1 ships, NOT optional" with a
more honest cost estimate (~150 LOC migration + ~50 LOC tests
+ supervised live run, not the previous optimistic ~30 LOC)
- TL;DR rewritten to mention the gap explicitly and re-order
the open follow-ups so the migration is the top priority
tests/test_project_state.py
---------------------------
- New test_legacy_alias_keyed_state_is_invisible_until_migrated
- Inserts a "p05" project row + a project_state row pointing at
it via raw SQL (bypassing set_state which now canonicalizes),
simulating a pre-fix legacy row
- Verifies the canonicalized get_state path can NOT see the row
via either the alias or the canonical id — this is the bug
- Verifies the row is still in the database (just unreachable),
so the migration script has something to find
- The docstring explicitly says: "When the legacy alias migration
script lands, this test must be inverted." Future readers will
know exactly when and how to update it.
Full suite: 175 passing (was 174), 1 warning. The +1 is the new
gap regression test.
What this commit does NOT do
----------------------------
- The migration script itself is NOT in this commit. Codex's
finding was a doc accuracy issue, and the right scope is fix
the doc + lock the gap behavior in. Writing the migration is
the next concrete step but is bigger (~200 LOC + dry-run mode
+ collision handling via the conflict model + supervised run
on the live Dalidou DB), warrants its own commit, and probably
warrants a "draft + review the dry-run output before applying"
workflow rather than a single shot.
- Existing tests are unchanged. The new test stands alone as a
documented gap; the 12 canonicalization tests from fb6298a
still pass without modification.
Codifies the helper-at-every-service-boundary rule that fb6298a
implemented across the eight current callsites. The contract is
intentionally simple but easy to forget, so it lives in its own
doc that the engineering layer V1 implementation sprint can read
before adding new project-keyed entity surfaces.
docs/architecture/project-identity-canonicalization.md
------------------------------------------------------
- The contract: every read/write that takes a project name MUST
call resolve_project_name() before the value crosses a service
boundary; canonicalization happens once, at the first statement
after input validation, never later
- The helper API: resolve_project_name(name) returns the canonical
project_id for registered names, the input unchanged for empty
or unregistered names (the second case is the backwards-compat
path for hand-curated state predating the registry)
- Full table of the 8 current callsites: builder.build_context,
project_state.set_state/get_state/invalidate_state,
interactions.record_interaction/list_interactions,
memory.create_memory/get_memories
- Where the helper is intentionally NOT called and why: legacy
ensure_project lookup, retriever's own _project_match_boost
(which already calls get_registered_project), _rank_chunks
secondary substring boost (multiplicative not filter, can't
drop relevant chunks), update_memory (no project field update),
unregistered names (the rule applied to a name with no record)
- Why this is the trust hierarchy in action: Layer 3 trusted
state has to be findable to win the trust battle; an
un-canonicalized lookup silently makes Layer 3 invisible and
the system falls through to lower-trust retrieved chunks with
no signal to the human
- The 4-step rule for new entry points: identify project-keyed
reads/writes, place the call as the first statement after
validation, add a regression test using the project_registry
fixture, verify None/empty paths
- How the project_registry fixture works with a copy-pasteable
example
- What the rule does NOT cover: alias creation (registry's own
write path), registry hot-reloading (no in-process cache by
design), cross-project dedup (collision detection at
registration), time-bounded canonicalization (canonical id is
stable forever), legacy data migration (open follow-up)
- Engineering layer V1 implications: every new service entry
point in the entities/relationships/conflicts/mirror modules
must apply the helper at the first statement after validation;
treated as code review failure if missing
- Open follow-ups: legacy data migration script (~30 LOC),
registry file caching when projects scale beyond ~50, case
sensitivity audit when entity-side storage lands, _rank_chunks
cleanup, documentation discoverability (intentional redundancy
between this doc, the helper docstring, and per-callsite comments)
- Quick reference card: copy-pasteable template for new service
functions
master-plan-status.md updated
-----------------------------
- New doc added to the engineering-layer planning sprint listing
- Marked as required reading before V1 implementation begins
- Note that V1 must apply the contract at every new service-layer
entry point
Pure doc work, no code changes. Full suite stays at 174 passing
because no source changed.
