#219's NameIndex silently narrowed #218's global endpoint fallback

[verveguy]

Feature Specification: #219's NameIndex silently narrowed #218's global endpoint fallback

Feature Branch: fabrik/issue-283
Created: 2026-07-29
Status: Draft
Input: User description: "#219's NameIndex silently narrowed #218's global endpoint fallback"

Background

Found while investigating #202. This is not the cause of the drops reported there, but it means the #209 fix (landed as PR #218) is weaker than its own tests suggest, and the weakness is invisible until a specific class of write happens.

Two commits landed about three hours apart, and the second undercut the first:

commit	when	what
04aacec (PR #218, issue #209)	2026-07-24 20:16 -0700	Added a persisted-entity fallback to edge-endpoint validation, calling get_entity_by_name_ci, which was then a WHERE lower(e.name) = $name table scan — slow but always correct.
f0a8ed3 (issue #219)	2026-07-25 02:06 -0400	Replaced that scan with the in-process NameIndex, with no scan fallback on a miss.

crates/core/src/db.rs:1134-1152 (get_entity_by_name_ci) now resolves entirely through self.name_index.lookup(name, group_id) and returns Ok(None) immediately on a miss. The doc comment on that method says so outright: "There is deliberately no scan fallback on a miss." ADR-0038 accepts the consequence explicitly — a stale index degrades to "a spurious miss, falling through to the embedding-based dedup path or a dropped edge, both pre-existing behaviors."

That reasoning holds for the dedup path, where a miss costs an extra embedding comparison. It does not hold for #218's fallback, whose entire purpose is to be the authority on "does this entity exist anywhere in the group." After f0a8ed3 that question is answered by an in-memory map rather than the database, so #218's guarantee is now conditional on index coherence — which the #218 tests never exercise, because they build state through insert_entity, the one path that keeps the index in sync.

Ways the index can be blind to a persisted entity

1. Raw Cypher writes. handle_query_cypher (crates/core/src/handlers.rs:630-652) takes the write lock and runs arbitrary caller Cypher with no NameIndex hook. Any CREATE (:Entity …) or SET e.name = … desyncs the index until restart. SET e.name is doubly bad: the old key fails verify-on-hit (db.rs:1146) and the new name has no key at all. Reachable from the MCP cypher scope.
2. WAL replay whose index rebuild failed. Replay executes raw templates, bypassing insert_entity. The repair is rebuild_name_index(), and at both knowledge_rebuild_from_wal branches it is non-fatal and only logged (handlers.rs:1499-1503 and handlers.rs:1771-1775). crates/core/tests/name_index_coherence.rs:255-289 proves replayed entities are invisible until an explicit rebuild.
3. A second process writing the same database — eval harness, migration (crates/service/src/migration.rs:248, :402), a CLI. The running service never learns.
4. Verify-on-hit failure with no second chance. lookup returns only the BTreeSet minimum (name_index.rs:78). If that UUID fails verification, the call returns None without trying the other same-named rows. Covered by name_index_coherence.rs:200-232.
5. Db::open_or_rebuild only rebuilds when the DB file was absent (db.rs:100-110); the DB-exists branch returns an empty index. No production caller today, but a live trap.

User Scenarios & Testing (mandatory)

User Story 1 - Endpoint resolution is authoritative regardless of index state (Priority: P1)

An entity that genuinely exists in the persisted graph must be found by edge-endpoint resolution, even when the in-process NameIndex doesn't know about it. Today, resolution trusts the index alone and gives up on a miss, so a real entity can be treated as nonexistent purely because of how or when it was written — not because it's actually absent.

Why this priority: This is the core correctness guarantee #218 was built to provide. Without it, edges to legitimately-existing entities are dropped, and the failure is silent — nothing in the current tests or logs points at it.

Independent Test: Persist an entity through a path that does not update the NameIndex (raw Cypher, or WAL replay with a forced index-rebuild failure), then ingest a chunk whose extracted edge names that entity. Verify the edge is retained rather than dropped.

Acceptance Scenarios:

1. Given an entity persisted via raw Cypher (not insert_entity), When a later chunk emits an edge naming it, Then the edge resolves and is retained.
2. Given an entity restored by WAL replay whose rebuild_name_index() failed, When a later chunk emits an edge naming it, Then the edge resolves.

---

User Story 2 - Index desync is observable (Priority: P2)

When the NameIndex misses on a name that a full scan would have resolved, that event must be countable and visible through existing status/telemetry surfaces, so index desync can be diagnosed operationally instead of only by reading source code or reproducing a bug locally.

Why this priority: Once User Story 1 restores correctness via a fallback path, desync becomes a performance/health concern rather than a correctness one — but it still needs to be visible, since a fallback that fires constantly signals a deeper index-maintenance bug worth fixing (e.g. an untracked write path).

Independent Test: Force an index miss that a scan resolves (e.g. via the raw-Cypher path from User Story 1), then query knowledge_status or equivalent telemetry and confirm the fallback/miss is reflected in the counters.

Acceptance Scenarios:

1. Given a NameIndex miss that a scan would have resolved, When it occurs, Then it is counted and surfaced so desync is diagnosable without reading source.

---

Edge Cases

• A scan fallback on a large graph must not reintroduce the full-table-scan cost Entity name lookup is a full table scan on every ingest — restore an indexed access path (regression vs graphiti) #219 removed. It must be bounded to the set of names a batch actually failed to resolve — this set is already deduplicated at episode.rs:255-268 (missing_names), so the fallback should reuse that shape rather than scanning per-edge.
• merge_entities leaves aliases in the index deliberately (name_index_coherence.rs:158-166), so a lookup can resolve to a Merged-labelled tombstone. Whether a scan fallback should honor that same behavior (resolve to the tombstone) or treat it differently needs to be confirmed once a fallback path exists — see Assumptions.
• group_id is never normalized on either side of the index (lookup or scan). "liminis", " liminis", and "Liminis" are three distinct namespaces today, and this issue does not change that — a fallback scan must reproduce the same non-normalization behavior as the index it's backing up, not silently start normalizing.

Requirements (mandatory)

Functional Requirements

• FR-001: Edge-endpoint resolution (get_entity_by_name_ci as called from fix(episode): resolve edge endpoints against persisted Entity table (#209) #218's fallback path) MUST NOT depend solely on the in-process NameIndex being coherent. Either restore a scan fallback on a miss for this call site, or make the index authoritative by construction (e.g. every write path that can create/rename/delete an Entity row updates or invalidates the index before the write is considered complete).
• FR-002: If a scan fallback is restored, it MUST be bounded so Entity name lookup is a full table scan on every ingest — restore an indexed access path (regression vs graphiti) #219's performance win is not lost on the common case (an index that is coherent). Concretely: scan only for the names a batch actually failed to resolve — a deduplicated set already computed at episode.rs:255-268 — not per-edge and not the whole batch.
• FR-003: rebuild_name_index() failure after WAL replay (handlers.rs:1499-1503, handlers.rs:1771-1775) MUST NOT remain silently non-fatal. It MUST either fail the reload/replay operation outright, or mark the index untrusted so that subsequent lookups fall back to a scan until the index is successfully rebuilt.
• FR-004: handle_query_cypher (crates/core/src/handlers.rs:630-652) MUST either invalidate the affected portion of the NameIndex when it executes an entity-mutating statement, or reject entity-mutating statements on that path, rather than leaving the index silently stale after an arbitrary write.
• FR-005: NameIndex::lookup (crates/core/src/name_index.rs:71-79) verify-on-hit failure MUST try the remaining same-named candidates (the rest of the BTreeSet for that key) before returning None, instead of giving up after the first (deterministic-minimum) candidate fails verification.
• FR-006: Tests for this fix MUST build state through a non-insert_entity path (raw Cypher or WAL replay) rather than exclusively through insert_entity, so this class of bug — index-bypassing writes silently breaking endpoint resolution — cannot pass undetected again.

Key Entities

• NameIndex: In-process, in-memory accelerator (crates/core/src/name_index.rs) mapping (group_id, lowercased name) to a BTreeSet of (created_at, uuid) candidates, keeping a deterministic "winner" per name. Populated by insert/update_created_at (kept in sync automatically) and by rebuild (a full one-shot scan used at startup, after recovery, and after WAL rebuild).
• get_entity_by_name_ci: The lookup used by fix(episode): resolve edge endpoints against persisted Entity table (#209) #218's edge-endpoint fallback (db.rs:1134-1152). Currently resolves solely through NameIndex.lookup, re-verifying the result against the database but never retrying or scanning on a miss.
• Index trust state: Not currently modeled. This issue introduces the need for some notion of "the index may be stale" (FR-003) that downstream lookups can consult to decide whether to fall back to a scan.

Success Criteria (mandatory)

Measurable Outcomes

• SC-001: A test that creates an entity via raw Cypher, then ingests an edge naming it, retains the edge. This test fails on main today (pre-fix) and passes after the fix.
• SC-002: A test that replays a WAL with a forced rebuild_name_index() failure still resolves endpoints correctly (does not silently drop edges to entities restored by that replay).
• SC-003: The Entity name lookup is a full table scan on every ingest — restore an indexed access path (regression vs graphiti) #219 dedup benchmark (dedup_overlap_check, see the project's bench workflow) shows no meaningful regression from this change on the common case (a coherent index, no fallback scans triggered). "No meaningful regression" is intentionally not pinned to a specific percentage in this spec — see Assumptions; the concrete threshold is a Plan/Review-stage decision informed by actual before/after measurements.
• SC-004: knowledge_status or an equivalent telemetry surface exposes (a) whether the NameIndex is currently considered trusted, and (b) a count of fallback scans performed, so index desync is diagnosable without reading source or reproducing the bug.

Assumptions

• Entity name lookup is a full table scan on every ingest — restore an indexed access path (regression vs graphiti) #219's performance motivation is still valid; this issue is about correctness of the miss path, not about reverting the index.
• On SC-003's "agreed margin": no numeric regression threshold is fixed at spec time. The scan fallback is only exercised on a miss (FR-002 bounds it to unresolved names), so the common, already-benchmarked case — a coherent index with no misses — is expected to see no added cost. The Plan stage should decide the concrete acceptable-regression threshold (if any) once an implementation approach is chosen, informed by actual dedup_overlap_check numbers; Review should confirm the measured delta stays within whatever threshold Plan set.
• On the merge_entities/Merged-tombstone edge case: this spec assumes a scan fallback should reproduce the same resolution behavior the index currently has — including resolving to a Merged-labelled tombstone where the index would. Research/Plan should confirm this is still the intended behavior once a concrete fallback mechanism is chosen; it is called out explicitly rather than silently decided because it affects what "resolves" means for an entity that has been merged away.
• The choice between "restore a scan fallback" and "make the index authoritative by construction" (FR-001, FR-003, FR-004) is left open deliberately — it is an implementation/architecture decision for the Plan stage, not a product-level requirement of this spec.

Out of Scope

• The endpoint-drift defect from Edges dropped when source endpoint is missing from the current batch's entity list, despite the entity existing earlier — likely batch-local resolution instead of global entity lookup #202 (a separate, already-filed issue) — that one drops edges for endpoints which never existed at all, which is a different failure mode from this issue's "endpoint exists but the index doesn't know it."
• Normalizing group_id handling (case/whitespace) across the index and any fallback scan. Both must behave consistently with each other, but changing that behavior itself is not in scope here.

Source References

• Issue Edges to already-existing entities silently dropped — resolve edge endpoints against the global entity table #209 / PR fix(episode): resolve edge endpoints against persisted Entity table (#209) #218 (04aacec) — original persisted-entity fallback for edge-endpoint validation.
• Issue Entity name lookup is a full table scan on every ingest — restore an indexed access path (regression vs graphiti) #219 (f0a8ed3) — NameIndex introduction that removed the scan fallback.
• ADR-0038 (docs/adr/0038-in-process-name-index.md) — accepts index-miss degradation for the dedup path; does not address the endpoint-fallback path this issue is about.
• crates/core/src/db.rs:1125-1152 — get_entity_by_name_ci.
• crates/core/src/name_index.rs:71-79 — NameIndex::lookup.
• crates/core/src/handlers.rs:630-652 — handle_query_cypher.
• crates/core/src/handlers.rs:1499-1503, :1771-1775 — non-fatal rebuild_name_index() failure handling after WAL replay.
• crates/core/src/episode.rs:245-268 — existing deduplicated missing_names set that a bounded scan fallback should reuse.
• crates/core/tests/name_index_coherence.rs:158-166, :200-232, :255-289 — existing coverage of merge-tombstone aliasing, verify-on-hit failure, and post-replay index blindness.
• Issue Edges dropped when source endpoint is missing from the current batch's entity list, despite the entity existing earlier — likely batch-local resolution instead of global entity lookup #202 — the investigation that surfaced this issue; explicitly not the same defect.

[verveguy]

🏭 Fabrik — stage: Specify
branch: fabrik/issue-283 | commit: 836f86c | main: 5c388d5 | 2026-07-30 02:46 UTC

The spec is committed. Now updating the issue body to match the finalized Spec Kit format.

I verified every file:line reference and code claim in the original issue against the current codebase (db.rs, handlers.rs, name_index.rs, episode.rs) — all checked out exactly. The issue was already written to near-Spec Kit precision, so I converted it into the canonical format, preserved the full investigative narrative in Background, and organized the existing FR/SC/Edge Cases/Assumptions into the standard sections. I left two genuinely open questions (the SC-003 performance margin, and whether a fallback should honor Merged-tombstone resolution) as explicit Assumptions rather than blocking, since both are architecture/measurement decisions better resolved during Plan/Review with concrete numbers in hand, not something a human needs to answer up front. The spec is committed to specs/283-219-s-nameindex-silently/spec.md.

---

Used 12/100 turns, 2k input / 18k output tokens.

[verveguy]

🏭 Fabrik — stage: Research
branch: fabrik/issue-283 | commit: 836f86c | main: 5c388d5 | 2026-07-30 02:50 UTC

I now have a thorough understanding of the codebase. Let me write up the research findings.

Research Findings

Relevant Code

• crates/core/src/name_index.rs: NameIndex struct — RwLock<NameIndexState> with by_key: HashMap<(group_id, lower_name), BTreeSet<(created_at, uuid)>> and by_uuid reverse map. insert/update_created_at/rebuild are the three mutation entry points; lookup (line 72-80) returns only set.iter().next() — the deterministic-minimum candidate — with no fallback to later candidates in the set if that one later fails verification (FR-005's target).
• crates/core/src/db.rs:17-24: Db.name_index: NameIndex field, doc-commented as living on Db (not Conn) because Db is the long-lived, wholesale-swapped handle.
• crates/core/src/db.rs:1134-1151: get_entity_by_name_ci — the function this issue is about. Single index lookup + verify-on-hit against get_entity_by_uuid; returns Ok(None) on miss with no scan.
• crates/core/src/db.rs:1156-1167: count_entities_by_name_ci — already contains the exact scan predicate (WHERE lower(e.name) = $lower_name AND e.group_id = $gid) a fallback would need, though it's count(e) only, no ORDER BY/winner-selection, and it's test-only today ("Primarily used in tests").
• crates/core/src/db.rs:305-323: rebuild_name_index() — one MATCH (e:Entity) RETURN uuid, name, group_id, created_at scan, replacing state wholesale.
• crates/core/src/db.rs:79-115: Db::open_or_rebuild — confirmed: the db_exists branch (line 84, 98) never calls rebuild_name_index(); only the fresh-DB-from-WAL branch does. This matches the issue's point 5 exactly — a live trap, no production caller today (production goes through main.rs's own explicit, fatal rebuild_name_index()? call at line 489, which is a separate call site not inside open_or_rebuild).
• crates/core/src/handlers.rs:630-652: handle_query_cypher — confirmed no NameIndex interaction anywhere in this function. Runs conn.cypher_query(&query) (arbitrary caller Cypher, explicitly out of type-coercion scope per issue AUDIT: route all direct-write paths through shared type coercion (timestamps/FLOAT[]) to match WAL replay #170 FR-008) under the write lock, then WAL-flushes. Reachable via the MCP cypher scope per crates/service/src/mcp/tools.rs.
• crates/core/src/handlers.rs:1490-1503 and :1762-1775: the two non-fatal rebuild_name_index() call sites inside knowledge_rebuild_from_wal's streaming and background-job branches, respectively — both only eprintln! on failure. crates/core/src/recovery.rs:258 calls the same function with ? (fatal), confirming the non-fatal posture is specific to these two handler branches, not general practice.
• crates/core/src/episode.rs: four get_entity_by_name_ci call sites — this is the key architectural fact for Plan:
  • Line 104 (Phase B, per extracted entity name, name-first dedup short-circuit) — one call per entity, not deduplicated across a batch.
  • Line 281 (Site 1, "global fallback" for edge validation) — already deduplicated via missing_names: HashSet (lines 255-268), one call per unique unresolved name in the batch. This is the shape FR-002 says a bounded scan fallback should reuse.
  • Lines 555 and 561 (Phase C / Site 2, commit-time endpoint resolution, one write-locked spawn_blocking) — per edge, not deduplicated — called once per edge whose endpoint isn't already in this batch's freshly-built name_to_uuid map (line 540), even though Site 1 already proved these same names resolve globally moments earlier in the same request.
• crates/core/tests/name_index_coherence.rs: existing coverage already builds state through non-insert_entity paths in two places — wal_replay_only_populates_name_index_after_explicit_rebuild (raw WAL replay, index blind until rebuild_name_index()) and stale_index_entry_after_out_of_band_delete_degrades_to_miss (raw run_cypher DETACH DELETE, proving verify-on-hit). These are strong templates for the FR-006-required new tests, but note: none of the existing tests here use get_entity_by_name_ci to model the raw-Cypher-INSERT case explicitly named in the spec's Acceptance Scenario 1 (entity created via raw Cypher, not deleted) — that's new coverage to add.
• crates/core/src/corrections.rs: merge_entities labels aliases "Merged" (never removes them from the graph or the index) — confirmed by apply_same_as_label_mutation_does_not_affect_name_lookups in the coherence tests, which asserts a Merged-labelled alias still resolves via get_entity_by_name_ci to its own UUID. This directly answers the spec's open Assumption: the index today does not filter Merged tombstones out of resolution, so a scan fallback must reproduce that (i.e., no WHERE NOT 'Merged' IN e.labels filter) to stay behaviorally consistent with the index it backs up.
• crates/core/src/app_state.rs: AppState.indices_built: Arc<AtomicBool> — existing prior-art pattern for a boolean health/trust flag surfaced through knowledge_status (handlers.rs:268,327) and manipulated across handler call sites. Directly relevant as a template for FR-003's "index untrusted" state and SC-004's telemetry, but it lives on AppState, not Db— see Technical Questions.
• crates/core/src/handlers.rs:215-334: handle_knowledge_status — builds its JSON response inline; indices_built and wal.* fields show the existing pattern for adding new status fields (a value captured inside the spawn_blocking block or read from an Arc<AtomicBool>/Arc<AtomicU64> on state, then spliced into the json!({...}) at the end).
• README.md:287-320: knowledge_status response fields, including indices_built, are documented in prose here — any new SC-004 field needs a matching doc update.

Architecture Notes

NameIndex lives on Db (long-lived, wholesale-swapped on clear_all/recovery), while Conn<'db> borrows it per-request. get_entity_by_name_ci is a single shared function called from four sites in episode.rs with very different volume/dedup characteristics: one dedup-path call per entity (Phase B), one deduplicated-per-batch call for the edge-validation "does it exist somewhere" fallback (Site 1), and two undeduplicated per-edge calls at commit time (Phase C/Site 2) that effectively re-derive information Site 1 already computed moments earlier in the same request, just to obtain the actual UUID (Site 1 only needed a boolean).

This matters because FR-001/FR-002 require the scan fallback to exist for "the fallback path" while staying bounded to the batch's deduplicated missing_names set. If a scan fallback is added inside get_entity_by_name_ci itself, it fires at all four call sites — including Phase B (per entity) and Phase C (per edge, undeduplicated) — which would silently reintroduce the O(edges × |Entity|) scan cost on any stale-index period, exactly what ADR-0038 rejected. Restoring correctness without regressing performance therefore likely requires either (a) a new, explicitly-named method/parameter used only by the bounded Site 1 caller, with Phase C refactored to consume Site 1's already-resolved name→UUID map instead of re-querying per edge, or (b) some other mechanism that preserves the "one scan per batch, not per edge/entity" bound. This is the central open design question for Plan.

Relevant ADRs

• ADR-0038 (docs/adr/0038-in-process-name-index.md): The ADR this issue directly qualifies. It explicitly rejected a "verify, then scan-fallback" design (Alternatives Considered) on performance grounds, covering all four get_entity_by_name_ci call sites uniformly — it did not distinguish the endpoint-fallback call site (bounded, deduplicated) from the dedup/commit-time call sites (unbounded, per-entity/per-edge). This issue's FR-001/FR-002 effectively ask for a narrower reversal of that decision, scoped only to the endpoint-authority use case — Plan needs to reconcile the two by changing call-site shape, not just get_entity_by_name_ci's body.
• ADR-0029 (referenced, not yet re-read in full): introduced get_entity_by_name_ci and the now-disproven materialized-column-index remedy; amended by ADR-0038. No new conflict from this issue beyond what ADR-0038 already amended.
• No ADR currently documents an "index trust state" concept (FR-003's requirement) — this would be new ground, though AppState.indices_built is a directly analogous existing pattern for the style of such a flag (a boolean gate that downstream logic consults, exposed via knowledge_status), just at the wrong layer (AppState, not Db/NameIndex) for something that must survive a Db swap correctly, i.e., an untrusted index on a fresh Db should not need re-marking, and a swapped-in fresh Db's new NameIndex should start trusted.

Constraints

• get_entity_by_name_ci's shared-function shape is the binding constraint. Any change to its body affects all four episode.rs call sites; FR-002's bound can only be honored if the scan fallback is either scoped to a distinct call path, or all four call sites are individually bounded/deduplicated (Phase B and Phase C currently are not).
• NameIndex::lookup's BTreeSet only stores one entry per (created_at, uuid) tuple per key — retrying "remaining candidates" (FR-005) means iterating set.iter() past the first failed-verification element, which is a small, self-contained change inside lookup (or a new method) and does not touch call sites.
• handle_query_cypher already holds state.write_lock.write() for its whole body (line 638), so any index-invalidation/rebuild added there is naturally serialized with all other writes — no new concurrency primitive needed, just a call to conn.rebuild_name_index() (or a more surgical invalidation) before/after cypher_query.
• Detecting "was this an entity-mutating statement" from raw Cypher text is unreliable — the codebase's own stance (issue AUDIT: route all direct-write paths through shared type coercion (timestamps/FLOAT[]) to match WAL replay #170 FR-008) is that raw Cypher gets no parsing/coercion. A full rebuild_name_index() after every handle_query_cypher call sidesteps needing to parse the statement, at the cost of one scan per raw-Cypher IPC call (an administrative/rare path, not a hot ingest path) — versus FR-004's alternative of textually detecting mutating statements and rejecting them outright, which would change existing behavior for legitimate non-Entity-mutating Cypher (e.g., relationship queries) if the detection is imprecise.
• Merged-tombstone resolution is already the index's actual behavior (see Architecture Notes) — a scan fallback must not add filtering that the index itself doesn't do, or the two paths would disagree on what "resolves" means for a merged-away alias.
• group_id non-normalization must be preserved — any new scan query must filter by exact group_id string match (AND e.group_id = $gid), matching both the index's key and count_entities_by_name_ci's existing scan predicate; this is explicitly out of scope to change per the spec's Edge Cases.
• SC-003's benchmark surface already exists: crates/core/benches/search.rs has bench_name_lookup_scan_baseline_10k/bench_name_lookup_indexed_10k and bench_dedup_overlap_check (the CI-gated correctness check, distinct from the on-demand perf measurement per bench.yml). Plan/Review can measure before/after directly against these rather than needing new benchmark infrastructure.

Technical Questions

• Should the scan fallback be implemented as a new, distinctly-named/parameterized lookup path used only by the bounded Site 1 caller (episode.rs:270-289), with Phase C's per-edge lookups (episode.rs:552-563) refactored to reuse Site 1's already-resolved name→UUID results instead of re-calling get_entity_by_name_ci? Doing it inside get_entity_by_name_ci itself would also affect the per-entity Phase B call (line 104) and the undeduplicated per-edge Phase C calls (lines 555/561), reintroducing the exact per-edge/per-entity scan cost ADR-0038 rejected.
• Where should the FR-003 "index untrusted" state and SC-004 fallback-scan counter live — on NameIndex/Db (consistent with the ADR-0038 placement rationale: state that must reset correctly whenever Db is swapped wholesale) or on AppState (consistent with the existing indices_built: Arc<AtomicBool> pattern already surfaced through knowledge_status)? A Db-swap that doesn't also reset an AppState-level flag would leave a stale "untrusted" mark attached to a brand-new, empty (and therefore vacuously coherent) index.
• For FR-004, is an unconditional rebuild_name_index() after every handle_query_cypher-executed statement (simple, correct, costs one scan per rare admin-path call) acceptable, or does Plan want textual detection of entity-mutating statements to avoid the scan on read-only Cypher queries (which are presumably the common case on this path)?
• This issue is architecturally complex — it touches a shared, multi-call-site accelerator function; introduces new cross-request trust/telemetry state; must reconcile with an explicit prior ADR decision (ADR-0038) that traded off correctness for performance uniformly across four call sites; and requires new non-insert_entity-path test infrastructure (raw Cypher, forced WAL-rebuild failure). Consider adding the model:opus label for Implement and Review stages.

Risks

• Reconciling FR-002's bound with get_entity_by_name_ci's shared shape is the largest implementation risk. Getting this wrong either reintroduces the O(edges × |Entity|) regression ADR-0038 fixed (if the fallback is unconditionally embedded) or leaves Phase B/Phase C still vulnerable to the exact silent-drop bug this issue describes (if the fallback is scoped too narrowly and doesn't cover the Phase C per-edge case, which is arguably just as much "the fallback path" as Site 1).
• rebuild_name_index() after every raw-Cypher call (FR-004) is a full Entity table scan on a hot-ish administrative path — bounded relative to ingest, but not free; if handle_query_cypher is called frequently in some deployment (e.g., scripted corrections), this could be a new, unbudgeted cost class.
• Forcing a rebuild_name_index() failure for the SC-002 test requires either a test seam (e.g., an injectable failure point) or an environmental trick (corrupt the query in a way rebuild_name_index's scan fails) — no existing test does this today; name_index_coherence.rs's replay test only exercises the success path of rebuild after replay.
• FR-003's "mark index untrusted until successfully rebuilt" implies lookups need to consult this state and behave differently (presumably: always scan-fallback while untrusted) — this couples FR-003 to whatever mechanism FR-001/FR-002 land on, so the two need a consistent design rather than independent patches.

Documentation Impact

README.md:287-320 documents the knowledge_status response shape in prose, including indices_built. SC-004 requires new status fields (index-trust state, fallback-scan count) — the README needs a corresponding update, following the existing indices_built documentation as a template (what it means, when it changes, what a client should do in response). No CLI commands, config options, or environment variables are introduced by this issue. No closed-issue search was needed beyond what's already cited in the spec (#209/#218/#219/#202/#220/#221) since this issue is purely an internal-correctness follow-up to already-well-documented prior work, not new user-facing surface — the Background section's own investigative narrative already supersedes what an external GitHub issue search would add.

Repositories

verveguy/liminis-context-graph

---

Used 35/100 turns, 5k input / 20k output tokens.