fix(core): key search_index uniqueness on the row kind

[phernandez]

Closes #1437. Part of #1438.

The defect

A relation's permalink is from/type/to with the relation type authored by the user, so a note that says

- [decision] redis
- observations [[decision/redis]]

gives its observation and its relation the identical address — …/observations/decision/redis — because observations is both the segment every observation permalink opens with and a relation type anyone can type. No reserved segment closes this: the colliding segment is the author's own text.

Reproduced on origin/main (da1ac946) before changing anything:

backend	what actually happened
Postgres	IntegrityError — insert or update on table "search_index_fts_chunks" violates foreign key constraint … Key (search_index_id, search_index_type, project_id)=(1, observation, 1) is not present in table "search_index". The upsert overwrote the observation row (its type changed), so the chunk insert pointed at a row that no longer existed.
SQLite	Both rows coexisted under one permalink through the bulk path; through index_item the delete-before-insert keyed on permalink alone destroyed the observation row outright. Nothing surfaced either way.

Address or uniqueness — uniqueness, and why

Changing the address was rejected. Permalinks are the user-facing identifier: memory:// URLs, relation targets written into markdown, search results, the API. Moving them breaks links people have already written. And it does not even work — the trap is documented on OBSERVATION_SEGMENT in models/knowledge.py: a marker that survives content is reachable by content, and one that escapes content is erased by the normalization every memory:// lookup performs.

Uniqueness is the outlier, so uniqueness is what moved. search_index's primary key is already (id, type, project_id) — the row kind is half of the table's own notion of identity. The unique index on (permalink, project_id) was narrower than that. Widening it to (permalink, type, project_id) makes uniqueness agree with the primary key, keeps every existing permalink byte-identical, and still constrains the within-kind duplicates that #909/#929/SPEC-82 were about.

One rule, one place

SEARCH_INDEX_ROW_KEY in models/search.py is the single definition, following the file_path_prefix_condition / note_type_filters precedent. Five call sites derive from it:

• the Postgres unique-index DDL,
• both Postgres upsert conflict targets (index_item, bulk_index_items),
• the accepted-note upsert (accepted_note_search_repository.py) — this one had to move, since ON CONFLICT (permalink, project_id) errors once the narrow index is gone,
• the shared delete-before-insert in search_repository_base.index_item, which is the only rule SQLite has (its FTS5 virtual table carries no unique index at all),
• delete_by_permalink, which now takes the kind that owns the address. Deleting one note's relation row used to take another note's observation row with it, and nothing rebuilds a projection whose source row still exists — the orphan sweep only removes.

⚠️ Migration — w6k7i8n9d0a1

Replaces uix_search_index_permalink_project with uix_search_index_permalink_type_project on Postgres. Widening a unique index cannot fail on existing data (every row satisfying the narrow key satisfies the wide one) and no permalink changes. The wide index is created before the narrow one is dropped. SQLite is a genuine no-op — its FTS5 table has no unique index to change.

The downgrade must shed rows the narrow index cannot admit — exactly the pairs the upgrade made legal. It keeps one row per (project_id, permalink), preferring entity over observation over relation (ORDER BY type, id); search rows are derived state the next index pass rebuilds, so shedding them is recoverable where a failed downgrade is not.

Migration verified for real, against live databases

The test suite uses create_all + stamp, so it never runs migrations. This was run manually.

Postgres (pgvector/pgvector:pg16, seeded with a project, an entity, search_index rows and their FTS-chunk children):

=== 1. alembic upgrade v5o6b7s8d9e0 (the head before this change) ===
  unique indexes on search_index: ['uix_search_index_permalink_project']
=== 2. seed a database that already has rows ===
  rows: [('entity', 1, 'test/redis-decision'), ('observation', 2, '…/observations/decision/redis')]
  fts chunks: [('entity', 1, 'entity body'), ('observation', 2, 'observation body')]
  relation at the observation's address -> REJECTED (UniqueViolation)
=== 3. alembic upgrade head (w6k7i8n9d0a1) ===
  unique indexes on search_index: ['uix_search_index_permalink_type_project']
  --- data survived the migration? ---
  rows: [('entity', 1, …), ('observation', 2, …)]          <- unchanged
  fts chunks: [('entity', 1, 'entity body'), ('observation', 2, 'observation body')]
  --- constraint holds? ---
  relation at the observation's address -> ACCEPTED
  SECOND observation at that address    -> REJECTED (UniqueViolation)
  SECOND entity at the entity address   -> REJECTED (UniqueViolation)
=== 4. alembic downgrade v5o6b7s8d9e0 ===
  unique indexes on search_index: ['uix_search_index_permalink_project']
  rows: [('entity', 1, …), ('observation', 2, …)]          <- relation shed, observation kept
  fts chunks: [('entity', 1), ('observation', 2)]          <- followed via ON DELETE CASCADE
  relation at the observation's address -> REJECTED (UniqueViolation)
=== 5. alembic upgrade head again ===
  unique indexes on search_index: ['uix_search_index_permalink_type_project']
  alembic_version: w6k7i8n9d0a1

Fresh-install path (base → head on an empty database) produces exactly:

CREATE UNIQUE INDEX uix_search_index_permalink_type_project ON public.search_index
USING btree (permalink, type, project_id) WHERE (permalink IS NOT NULL)

…which is byte-identical to what CREATE_POSTGRES_SEARCH_INDEX_PERMALINK in models/search.py creates (compared programmatically via pg_indexes.indexdef, IDENTICAL: True), so the migrated schema and the schema the test suite builds cannot drift.

SQLite (file database seeded with all three row kinds, including a colliding pair):

before upgrade:    version=v5o6b7s8d9e0  rows=[entity/1, observation/2, relation/3]
after upgrade:     version=w6k7i8n9d0a1  rows=[entity/1, observation/2, relation/3]
after downgrade:   version=v5o6b7s8d9e0  rows=[entity/1, observation/2, relation/3]
after re-upgrade:  version=w6k7i8n9d0a1  rows=[entity/1, observation/2, relation/3]

Chain: alembic heads → w6k7i8n9d0a1 (head), single head. alembic branches shows only the pre-existing f8a9b2c3d4e5 branchpoint already merged by 6830751f5fb6. No dangling revisions.

One pre-existing failure, reported rather than papered over: alembic downgrade base on Postgres is already broken at the previous head v5o6b7s8d9e0 (constraint "fk_entity_project_id" of relation "entity" does not exist, from the 9d9c1cb7d8f5/a1b2c3d4e5f6 batch-mode downgrades). Verified independently of this branch. Not touched here; downgrade to the previous revision — the case that matters for this change — works.

Tests

New tests fail on main and pass here, on both backends, verified by stashing the source hunks:

test	before (both backends)
test_observation_and_relation_sharing_an_address_keep_separate_rows	assert [('relation', 22)] == [('observation', 11), ('relation', 22)] — the observation row was gone
test_reindexing_one_kind_replaces_only_its_own_row	assert [('observation', 33)] == [('observation', 33), ('relation', 22)]
test_delete_by_permalink_leaves_the_other_kind_at_that_address	the kind-scoped API did not exist

The SQLite assertions check the stored rows directly — that both kinds are present with their own ids — rather than merely that no error was raised, since silence was the original symptom there. test_reindexing_one_kind_replaces_only_its_own_row guards the other direction: widening the key must not become no key, so a second write of the same kind still replaces rather than duplicates.

tests/services/test_search_service.py::test_note_whose_relation_spells_an_observation_address_indexes_both is the issue's reproduction end-to-end through index_entity_markdown (the bulk_index_items path), asserting the two permalinks genuinely collide before asserting both rows survive.

tests/test_search_index_row_kind_migration.py covers the migration itself (100% line coverage on the revision).

Verification run

• uv run ruff check src tests test-int — All checks passed
• uv run ruff format --check . — 1103 files already formatted
• uv run ty check src tests test-int — All checks passed
• SQLite tests/repository tests/services tests/indexing tests/index tests/db — 2321 passed, 41 skipped
• SQLite remainder of tests/ — 4575 passed
• Postgres (BASIC_MEMORY_TEST_POSTGRES=1) same five directories — 2302 passed, 59 skipped, 1 failed: test_tokenizing_scales_linearly_with_query_length, a CPU-ratio timing assertion over relaxation_word_tokens (a pure string function, no database, untouched here) that went red under three concurrent suites; passes in isolation
• Migration tests on both backends — 58 passed each

🤖 Generated with Claude Code

https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-03T12:25:22.536118Z	41b4197	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.