fix(neo4j): scope get_node's fallback MATCH to :Node so it seeks instead of scanning

[Diego Colombo (colombod)]

Problem — get_node() full-scans the graph, exhausting the shared bolt pool

get_node()'s Neo4j fallback issued a label-free MATCH:

MATCH (n) WHERE n.node_id = $id AND n.workspace = $workspace
RETURN properties(n) AS props, labels(n) AS lbls

Neo4j property indexes are label-scoped, so a label-free MATCH cannot use any
index. On the production graph (12,091,131 nodes) the planner emits an
AllNodesScan:

| Operator        | Id | Details                                        | Estimated Rows |
| +ProduceResults |  0 | props, lbls                                    |          30228 |
| +Projection     |  1 | properties(n) AS props, labels(n) AS lbls      |          30228 |
| +Filter         |  2 | (n.node_id = $id AND n.workspace = $workspace) |          30228 |
| +AllNodesScan   |  3 | n                                              |       12091187 |

This is the hot per-event read (touch_session → get_node → this query), not a
rare path. Captured live via SHOW TRANSACTIONS on the production instance:

"neo4j-transaction-4564782", PT1M3.135S, "Running", "10.100.0.37:48620",
  "MATCH (n) WHERE n.node_id = $id AND n.workspace = $workspace RETURN ..."
"neo4j-transaction-4564839", PT59.41S,   "Running", "10.100.0.37:48372",  ... (same query)
"neo4j-transaction-4564896", PT51.681S,  "Running", "10.100.0.37:48492",  ... (same query)

51 concurrent transactions, all this query, 48–63 s each. Each holds its
pooled bolt connection for the whole scan. Against the shared 50-connection pool
(neo4j_max_connection_pool_size, default 50) that is total saturation, so every
further acquire died on the 30 s connection_acquisition_timeout:

neo4j.exceptions.ConnectionAcquisitionTimeoutError:
  failed to obtain a connection from the pool within 30.0s (timeout)

Batches then burned max_delivery_attempts and landed in
_handle_exhausted_batch, which dead-letters each line and commits the queue
offset past it — permanent loss of live activity records. Sampled rate: ~88
dead_letter lines in a 2-second window.

Host impact: the Neo4j VM (Standard_E8s_v5, 8 vCPU) sat at 99% CPU for ~41
hours (onset 2026-09-07 21:00 UTC), load average 49, while committing only
~1.2 txn/s — the signature of enormous read work producing almost no throughput.
GC was not implicated (VmPauseMonitor reported 205–305 ms pauses with
gcTime=0, gcCount=0); checkpoints completed normally in ~0.6–1.1 s.

This is the same bug class as #19, in the one place that was missed

neo4j_store.py already fixed this class everywhere else via the universal
:Node label: _NODE_MERGE_CYPHER (writes), _edge_merge_cypher (edge
endpoints), and _NODE_MATCH_BY_ID (label SET / label-patch add/remove) all
scope to :Node and are guarded by plan assertions in
tests/neo4j/test_node_index_seek.py. get_node() hand-rolled its own query
string instead of reusing the shared prefix, so it inherited none of that —
and none of the existing guards covered it.

Every index is present and healthy (SHOW INDEXES: all ONLINE,
populationPercent: 100.0), including node_node_id_workspace_unique on
:Node(node_id, workspace), which covers this lookup exactly. The index was
never missing — the query just never named a label the planner could use it for.

Fix

• New _NODE_GET_BY_ID_CYPHER, composed from the existing _NODE_MATCH_BY_ID
  prefix so it cannot drift back to a label-free MATCH — one logic home, the
  same pattern the other three call sites already use.
• get_node() uses it, binding $node_id (was $id) to match the shared prefix.

Query-plan effect, verified against a live Neo4j 5.26:

query	plan operator	estimated rows
before	AllNodesScan	12,091,187
after	NodeUniqueIndexSeek UNIQUE n:Node(node_id, workspace)	1

No schema change, no migration, no config change — every node already carries
:Node and the backing constraint already exists.

Regression guards

Two, deliberately at different levels:

1. tests/test_neo4j_store.py::test_get_node_fallback_is_label_scoped_for_index_seek
   (unit) — asserts the issued query scopes to :Node. The live plan tests are
   marked neo4j and are deselected from the default suite (90 deselected),
   so this is the guard that actually runs in ordinary CI.
2. tests/neo4j/test_node_index_seek.py::test_get_node_fallback_uses_index_seek_not_allnodesscan
   (live) — EXPLAINs the real production query string and asserts no
   AllNodesScan and a present IndexSeek, matching the three existing plan
   guards in that file. Imported via its own try/except (not folded into the
   existing import block, which would make _NODE_MATCH_BY_ID fall back and
   break the sibling tests for the wrong reason); the fallback is byte-identical
   to the unfixed query so a revert still fails RED for the right reason.

test_get_node_fallback_queries_by_node_id_property was updated, not weakened:
its assertion "n.node_id" in query was a textual proxy for "keys on the
node_id property", which the map-pattern syntax breaks while preserving the
intent. It now asserts the intent robustly (node_id present, n.id absent) and
additionally checks the $node_id parameter binding.

Both guards verified to fail without the fix (source reverted to HEAD, tests
kept):

FAILED tests/test_neo4j_store.py::test_get_node_fallback_is_label_scoped_for_index_seek
  AssertionError: ... Issued query: 'MATCH (n) WHERE n.node_id = $id AND ...'

FAILED tests/neo4j/test_node_index_seek.py::test_get_node_fallback_uses_index_seek_not_allnodesscan
  AssertionError: get_node()'s Neo4j fallback still does a full-graph AllNodesScan
  Plan operators: ['ProduceResults@neo4j', 'Projection@neo4j', 'Filter@neo4j', 'AllNodesScan@neo4j']

Test results

• pytest -m "not neo4j" — 1971 passed, 7 skipped, 90 deselected
• pytest tests/neo4j/test_node_index_seek.py -m neo4j (live container) — 6 passed
• ruff check / ruff format --check on changed files — no new findings
  (pre-existing counts in tests/test_neo4j_store.py unchanged: 20 before, 20 after)

Deployment note

The currently-deployed revision is context-intelligence-api--0000015, image tag
80e289f1edc3833fddd09858252876b53bae23da (= main HEAD, built 2026-09-01).
PR #91 (shared bounded driver) is deployed and is not at fault — it converted
an unbounded driver leak into a bounded pool. This query was pathological either
way; #91 only changed which error the exhaustion surfaces as.

Follow-up (deliberately NOT in this PR)

Neo4jGraphStore._ensure_schema() runs on every flush until
_schema_initialized latches, and it latches only when every one of ~12
catalog statements succeeds. Under pool starvation none of them can acquire a
connection, so the flag never latches and every flush on every session re-attempts
the full schema pass — adding pool pressure to an already-starved pool
(ensure_neo4j_schema: could not create index (connectivity error) appeared 3×
in a 2-second sample). It is self-healing by design and goes quiet once this fix
removes the starvation, but the retry has no backoff, which turns a connectivity
blip into a per-flush storm. Worth a separate change; out of scope for a hotfix.

---

Additional changes (second commit) — containing the bug class, not the instance

Review of the original fix asked two follow-up questions: does anything else in
this module scan, and what made one slow query take the whole service down. Both
had concrete answers, so they are addressed here rather than deferred.

Live measurement taken during review: GET /version on the deployed
instance — a route that touches no Neo4j at all — took 114.8 s (HTTP
200). /status and /cypher did not return inside 60 s; unauthenticated calls
401 instantly, so this is the backend, not the gateway. Ingest drain workers and
the HTTP API share one process and one event loop, so a saturated ingest path
starves every route, including ones with no graph dependency. That is why a
single unindexed read became a total API outage.

1. The remaining unindexed reads

get_node() was not the only read that could not use an index.

Read	Before	After
get_edge()	MATCH ()-[r]->() WHERE r.src_id = ... — relationship property indexes are type-scoped, and this names no type, so no index could back it: AllRelationshipsScan (a larger set than the AllNodesScan just fixed)	Both endpoints anchored on :Node identity patterns → the planner seeks the (node_id, workspace) unique index and expands only that node's own relationships
find_delegation_by_sub_session()	Query shape was already correct, but no index existed on Delegation.sub_session_id, so it planned as a NodeByLabelScan across every :Delegation node	New composite index idx_delegation_sub_session on :Delegation(sub_session_id, workspace) → NodeIndexSeek

get_edge's relationship-property predicates are retained after the anchor.
They now filter a handful of relationships incident to one node instead of every
relationship in the graph, so they cost nothing — and keeping them makes this a
strictly narrowing change. Any edge the old form returned and the new one does
not is one whose endpoints do not exist as :Node, which the edge writer
(_edge_merge_cypher, which MERGEs both :Node endpoints) structurally cannot
produce.

find_delegation_by_sub_session is a live ingest path — the self-delegation
resolver reaches it whenever the parent Delegation has already flushed out of the
in-memory buffer — and its cost grew with delegation volume. get_edge has no
in-repo caller today, so it was latent rather than live; it is fixed anyway
because it is on the public GraphStore protocol and is the worst-planning query
in the module.

2. _ensure_schema on the flush path — the amplifier

This is what kept the incident burning and prevented self-recovery.

_flush_body awaits _ensure_schema() before Phase 1, i.e. before any data
is written. Its latch, _schema_initialized, is per store instance — and a
store is constructed per session (registry.get_or_create). So:

• Healthy steady state: every new session paid a full ~11-statement catalog pass
  on its first flush. Continuous background DDL, proportional to session churn.
• Degraded: the latch never set, so every flush of every session re-ran the
  whole pass, each statement waiting out the full connection-acquisition timeout
  against the very pool it was starving — with no backoff, retrying on the
  immediately following flush.

In the server this work is pure redundancy: main.py already runs
ensure_neo4j_schema(..., fail_on_data_conflict=True) during lifespan, before
accepting a single request, and refuses to boot otherwise. A per-flush pass can
only re-confirm what cold start already established.

Two changes:

1. _SCHEMA_READY is process-wide, seeded by the lifespan. In the server the
   per-flush call becomes a single boolean read. Contexts with no lifespan (tests,
   CLI tools, direct store use) never seed it and keep today's behaviour — the
   first store to fully establish the schema latches it for the process.
2. _SCHEMA_RETRY_BACKOFF_SECONDS puts a floor between un-latched attempts,
   so a connectivity blip can no longer become a per-flush storm. The self-heal is
   delayed, never disabled.

The lifespan seeds the latch only on a fully-established pass.
fail_on_data_conflict=True fails closed on a :Node constraint data conflict,
but a connectivity failure on any individual index/constraint is deliberately
swallowed and reported through the return value. Latching unconditionally would
mark a half-built schema ready and permanently disable the per-flush self-heal —
reopening the "constraint created once, never retried" gap. The seed is therefore
gated on the return value, with a WARNING on the unseeded path
(TestLifespanSchemaLatchSeeding guards both branches).

3. Source-level tripwire

get_node() broke because it hand-rolled a query string while three sibling call
sites composed theirs from the shared _NODE_MATCH_BY_ID prefix — so it inherited
none of their index-scoped-ness, and no existing guard covered it. Guarding one
call site per outage does not scale.

test_no_unindexed_scan_patterns_in_neo4j_store_source scans the module for
label-free MATCH (n) and untyped MATCH ()-[r]-> patterns and fails on anything
outside a documented allow-list. The allow-list is the complete reviewed inventory
of deliberately label-free statements — each either answered from Neo4j's O(1)
counts store or an explicitly O(graph-size) doctor --fix repair path, never on
the hot ingest path. Adding an entry is the review checkpoint.

This covers queries nobody has written yet, which is the part the per-call-site
guards cannot do.

Deployment safety

• Additive only. One new index; no constraint change, no migration, no config
  change, no data rewrite.
• CREATE INDEX ... IF NOT EXISTS does not block boot. The statement returns
  immediately and the index populates in the background; population is bounded by
  :Delegation cardinality, not graph size. Until it is ONLINE the planner
  simply does not use it — the delegation query string is unchanged, so
  pre-index behaviour is exactly today's behaviour.
• Connectivity failure at boot is already tolerated. A DriverError on any
  index/constraint is swallowed, ensure_neo4j_schema returns False, the latch
  stays unset, and the flush path self-heals (now rate-limited). Boot proceeds.
• Rolling deploy is safe. Old and new instances can run concurrently; an index
  is transparent to instances that do not know about it.
• Known tradeoff, stated plainly: because the latch is now process-wide, a
  schema object dropped out of band after boot is no longer re-created by a new
  session's first flush within that process's lifetime. Recovery is a restart or
  doctor --fix. That per-session re-run is precisely the storm being removed, so
  the tradeoff is deliberate.

Verification

• pytest -m "not neo4j" — 1982 passed, 7 skipped, 92 deselected
• pytest tests/neo4j/test_node_index_seek.py -m neo4j (live Neo4j container) —
  8 passed, including two new live plan guards:
  test_get_edge_fallback_does_not_scan_all_relationships (no
  AllRelationshipsScan, no AllNodesScan, IndexSeek present) and
  test_delegation_lookup_uses_index_seek_not_label_scan (no NodeByLabelScan,
  IndexSeek present).
• Six new guards verified RED against this branch's own prior source (source
  reverted, tests kept): test_get_edge_fallback_is_node_anchored_not_all_relationships_scan,
  test_schema_creates_delegation_sub_session_index,
  test_no_unindexed_scan_patterns_in_neo4j_store_source,
  TestSchemaProcessLatch (both), TestSchemaRetryBackoff::test_failed_pass_does_not_retry_on_the_next_flush.
• ruff format --check — clean on all five changed files. ruff check — no new
  rule classes; the only count change is four additional # noqa directives of
  the same kinds the files already use (E402, PLC0415).
• Guarded imports (try/except ImportError) were added in tests/conftest.py and
  tests/test_neo4j_store.py so the suite still collects against unfixed
  source and the new guards fail for the reason they exist rather than at import
  time. Each fallback is byte-identical to the unfixed production value.

Deliberately NOT in this PR

• No read timeouts and no limits on POST /cypher — out of scope by
  direction. These changes make wide queries seek; they do not bound or
  restrict what callers may run.
• _handle_exhausted_batch transient-vs-poison classification. It catches
  bare Exception, so a ConnectionAcquisitionTimeoutError is dead-lettered and
  the queue offset committed past it exactly like a malformed line — which is what
  turned an infrastructure stall into permanent loss of live activity records.
  Fixing it means touching queue-commit semantics, which does not belong in the
  same change as a hotfix. Worth doing next.