runtime: publish the vector index instead of guarding it #143

2026-08-16T16:33:02Z

will.anderson commented

2026-08-16 16:33:02 +00:00

Three read paths mutated five process-global statics. engram_activate, eg_knn_for_node (whose own comment says "No writes.") and engram_geo_reify_run_json all called eg_vindex_sync, which frees the index, reallocs the seen-map and inserts — on a read. Every frame of the production crash (engram_activate → eg_vindex_sync → vindex_insert → _realloc → _xzm_xzone_malloc_freelist_outlined) is accounted for.

Three moves, in decreasing order of how much they dissolve

1. Misfiled scratch is not shared state. visited/visit_epoch/visited_cap were never owned by the index — they are one traversal’s local, hoisted into struct VIndex as an allocation optimisation. Two concurrent reads stomped each other purely because of this. They want neither a lock nor a capability nor a pool, just to go back in the call frame.

2. const IS the capability. Once the scratch leaves the struct, search reads and nothing else, so vindex_search takes a const VIndex*. That is exactly what a capability-pointer ABI would have bought — a read path physically cannot call vindex_insert, compiler-enforced on every future caller — for one qualifier instead of an ABI swept across hundreds of builtins.

3. What survives is publication, not ownership. HNSW insert is not an append: it rewires the neighbour links of already-existing elements and reallocs elems[], so the store’s append-only property does not transfer to the index derived from it. eg_vindex_sync splits into eg_vindex_maintain (exclusive, sole mutator) and eg_vindex_view (shared, returns const VIndex*). A read path may demand a current snapshot exist — a request to the owner, not a mutation by the reader.

Write-side owner. eg_vindex_note_embedded hooks the embedding-assignment sites rather than the append sites: a node with no embedding cannot be in a vector index, so embedding assignment is the event that owns membership. One O(log n) insert, no O(node_count) presence scan. Retires the staleness note where a lazily-embedded older node stayed invisible to route_nearest/autoconnect until a full rebuild (the embed-gap #20 shape).

Evidence

The old harness conflated two hazards, which is why fixing half of it read as failure. Split four ways:

half	before	after
single, 3000 vec, ASan+UBSan	clean	clean
concurrent readers, no writer, TSan	race at `engram_vindex.c:195`	clean
writer+reader, bare index	race	race — expected, permanent
owner+4 readers via boundary	(did not exist)	clean, 3000/3000 inserts landed

RESULT: PASS. recall@10 = 0.9365 at ef_search=128 (gate ≥0.90); determinism byte-identical across two independent builds. Re-run green after rebase onto dev (on top of #141 and #142), with all three fixes verified coexisting in one binary.

The bare-index half is now permanently expected to race, deliberately — it is the executable proof that the boundary must live above the data structure.

`fb32d15` is KEPT — correcting this design’s own §5

The design said delete the guard. Measured, it covers two structures and only one was converted here: g->nodes/g->edges are realloc’d in place (el_runtime.c:7618,7629) and engram_activate_inner’s embed-backfill writes n->emb through exactly such a borrowed pointer. Deleting it reintroduces a measured 11171→9579 edge loss. Kept, with its comment narrowed to the RAM graph and the deletion precondition named.

That corrects the ordering claim too: the residual is not one ABI that dissolves everything at once — it is a property applied per structure. Residues evaporate in the order the property is applied, and a residue whose structure has not been converted must be left standing.

Three read paths mutated five process-global statics. `engram_activate`, `eg_knn_for_node` (whose own comment says *"No writes."*) and `engram_geo_reify_run_json` all called `eg_vindex_sync`, which frees the index, reallocs the seen-map and inserts — on a read. Every frame of the production crash (`engram_activate → eg_vindex_sync → vindex_insert → _realloc → _xzm_xzone_malloc_freelist_outlined`) is accounted for. ### Three moves, in decreasing order of how much they dissolve **1. Misfiled scratch is not shared state.** `visited`/`visit_epoch`/`visited_cap` were never owned by the index — they are one traversal’s local, hoisted into `struct VIndex` as an allocation optimisation. Two concurrent **reads** stomped each other purely because of this. They want neither a lock nor a capability nor a pool, just to go back in the call frame. **2. `const` IS the capability.** Once the scratch leaves the struct, search reads and nothing else, so `vindex_search` takes a `const VIndex*`. That is exactly what a capability-pointer ABI would have bought — a read path physically cannot call `vindex_insert`, compiler-enforced on every future caller — for one qualifier instead of an ABI swept across hundreds of builtins. **3. What survives is publication, not ownership.** **HNSW insert is not an append**: it rewires the neighbour links of already-existing elements and reallocs `elems[]`, so the store’s append-only property does *not* transfer to the index derived from it. `eg_vindex_sync` splits into `eg_vindex_maintain` (exclusive, sole mutator) and `eg_vindex_view` (shared, returns `const VIndex*`). A read path may *demand a current snapshot exist* — a request to the owner, not a mutation by the reader. **Write-side owner.** `eg_vindex_note_embedded` hooks the embedding-**assignment** sites rather than the append sites: a node with no embedding cannot be in a vector index, so embedding assignment is the event that owns membership. One O(log n) insert, no O(node_count) presence scan. Retires the staleness note where a lazily-embedded *older* node stayed invisible to `route_nearest`/autoconnect until a full rebuild (the embed-gap #20 shape). ### Evidence The old harness conflated two hazards, which is why fixing half of it read as failure. Split four ways: | half | before | after | |---|---|---| | single, 3000 vec, ASan+UBSan | clean | clean | | concurrent readers, no writer, TSan | **race** at `engram_vindex.c:195` | **clean** | | writer+reader, bare index | race | **race — expected, permanent** | | owner+4 readers via boundary | *(did not exist)* | **clean**, 3000/3000 inserts landed | `RESULT: PASS`. `recall@10 = 0.9365` at `ef_search=128` (gate ≥0.90); determinism byte-identical across two independent builds. Re-run green after rebase onto `dev` (on top of #141 and #142), with all three fixes verified coexisting in one binary. The bare-index half is now *permanently* expected to race, deliberately — it is the executable proof that the boundary must live above the data structure. ### `fb32d15` is KEPT — correcting this design’s own §5 The design said delete the guard. Measured, it covers **two** structures and only one was converted here: `g->nodes`/`g->edges` are realloc’d in place (`el_runtime.c:7618,7629`) and `engram_activate_inner`’s embed-backfill writes `n->emb` through exactly such a borrowed pointer. Deleting it reintroduces a **measured 11171→9579 edge loss**. Kept, with its comment narrowed to the RAM graph and the deletion precondition named. That corrects the ordering claim too: the residual is not one ABI that dissolves everything at once — it is a **property applied per structure**. Residues evaporate in the order the property is applied, and a residue whose structure has not been converted must be left standing.

will.anderson added 3 commits 2026-08-16 16:33:04 +00:00

runtime: guard engram activation against the unsynchronized awareness thread bdc1f99fb9

The soul daemon had two engram callers and only one of them locked.
soul.el:729 starts the HTTP server via http_serve_async (spawning
http_worker threads); soul.el:731 then runs awareness_run() on the MAIN
thread. awareness.el's perceive() -> engram_activate_json() ->
engram_activate() -> eg_vindex_sync() -> vindex_insert() mutates the same
g->nodes/g->edges and the process-global _eg_vindex HNSW index that the
workers touch. g_engram_req_lock existed to serialize exactly this, but it
was only ever taken inside http_worker: engram_req_lock/engram_req_unlock
appear in ZERO .el sources, so the awareness loop ran lock-free beside the
workers on every tick (SOUL_TICK_MS=1000).

Result was a crash-loop under launchd KeepAlive: five crashes in ~4 minutes
on 2026-08-16 with varying faulting frames -- search_layer<-vindex_insert
<-eg_vindex_sync, engram_activate, abort, and one inside xzm_realloc's own
freelist. Varying sites plus a fault in allocator metadata means heap
corruption. The SIGSEGV address 0x65646f4e6d617267 is little-endian ASCII
"gramNode": string bytes dereferenced as an Elem vector pointer.

Diagnosed by bisection rather than inspection:
  - Replaying all 13,820 real dim-768 vectors harvested from the live store
    through the index single-threaded under ASan is 100% clean, which rules
    out an HNSW logic/bounds bug.
  - Two threads on one index trip ThreadSanitizer immediately at
    engram_vindex.c:195 (visited_reset), reached from both vindex_search and
    vindex_insert. VIndex keeps a SHARED visited-epoch scratch buffer, so
    even two concurrent READS corrupt each other's traversal and walk bogus
    element indices.
So this is purely a concurrency defect, not an HNSW logic error. (An
inspection-derived hypothesis about an out-of-bounds reverse-link write at
engram_vindex.c:340 was disproved by the single-threaded run.)

Fix: a thread-local ownership depth (_eg_req_depth) lets engram entry points
self-guard. engram_activate() becomes a wrapper over engram_activate_inner()
that acquires g_engram_req_lock when called with depth 0 (the awareness
thread) and passes through when depth > 0 (nested inside an http_worker that
already holds it), so the non-recursive mutex cannot self-deadlock. The depth
is a plain counter, never a recursive-mutex count, preserving
engram_self_reify_beat_json's contract of genuinely releasing the lock
mid-beat.

test: regression harness for the vindex concurrency crash e99a4640e2

Promotes the two throwaway sanitizer harnesses used to diagnose the
2026-08-16 soul crash into engram/test/ so the bug cannot silently regress.

The harness has two halves and the PAIR is the point — it is what localises
the defect to concurrency rather than to HNSW logic:

  single      3000 clustered vectors, one thread, ASan+UBSan. The CONTROL.
              Must always be clean. During diagnosis this cleared all 13,820
              real dim-768 vectors from the live store, which DISPROVED an
              inspection-derived hypothesis about an out-of-bounds
              reverse-link write at engram_vindex.c:340.

  concurrent  writer + reader on one shared index, TSan. Currently reports a
              race at engram_vindex.c:195 (visited_reset) reached from both
              vindex_search and vindex_insert, because VIndex still owns its
              visited[]/visit_epoch scratch — so even two concurrent READS
              corrupt each other's traversal.

Verified: half 1 passes, half 2 reproduces the race.

Gated on EXPECT_RACE, default 1, so the concurrent half documents the known
defect without failing the suite today. When the visited set moves to a
per-query checkout pool (hnswlib VisitedListPool style — NOT thread_local,
since http_worker is a thread per connection and a __thread buffer would leak
~55KB per connection), flip EXPECT_RACE=0 and it becomes a real gate.

runtime: publish the vector index instead of guarding it

El SDK CI - dev / build-and-test (pull_request) Failing after 10m59s

Details

8e9d88fc01

The crash (SIGTRAP in engram_activate -> eg_vindex_sync -> vindex_insert ->
_realloc) had three read paths mutating five process-global statics.
engram_activate, eg_knn_for_node (whose own comment says "No writes.") and
engram_geo_reify_run_json all called eg_vindex_sync, which frees the index,
reallocs the seen-map and inserts — on a read.

Three moves, in decreasing order of how much they dissolve:

1. Misfiled scratch is not shared state. visited/visit_epoch/visited_cap
   were never owned by the index; they are one traversal's local, hoisted
   into struct VIndex as an allocation optimisation. They want neither a
   lock nor a capability nor a pool — just to go back in the call frame.
   Two concurrent READS stomped each other purely because of this.

2. const IS the capability. Once the scratch leaves the struct, search
   reads and nothing else, so vindex_search takes a const VIndex*. That is
   exactly what a capability-pointer ABI would have bought — a read path
   physically cannot call vindex_insert, enforced by the compiler on every
   future caller — for one qualifier instead of an ABI swept across
   hundreds of builtins.

3. What survives is publication, not ownership. HNSW insert is NOT an
   append: it rewires the neighbour links of already-existing elements and
   reallocs elems[], so the store's append-only property does not transfer
   to the index derived from it. eg_vindex_sync therefore splits into
   eg_vindex_maintain (exclusive, sole mutator) and eg_vindex_view (shared,
   returns const VIndex*). A read path may demand that a current snapshot
   exist — a request to the owner, not a mutation by the reader.

Write-side owner: eg_vindex_note_embedded hooks the embedding-ASSIGNMENT
sites rather than the append sites, because a node with no embedding cannot
be in a vector index — embedding assignment is the event that owns index
membership. One O(log n) insert, no O(node_count) presence scan. This also
retires the "STALENESS (honest tradeoff)" note where a lazily-embedded
older node stayed invisible to route_nearest/autoconnect until a full
rebuild (the embed-gap #20 shape).

Evidence. The existing harness conflated two hazards, which is why fixing
half of it read as failure. Split into four:

  single (3000 vec, ASan+UBSan)          clean  ->  clean
  readers (4 readers, no writer, TSan)   RACE   ->  clean
  unsynchronized (writer+reader, bare)   race   ->  race, expected forever
  published (owner + 4 readers)          n/a    ->  clean, 3000/3000 landed

RESULT: PASS. recall@10 = 0.9365 at ef_search=128 (gate >= 0.90);
determinism byte-identical across two independent builds.

The unsynchronized half is now permanently expected to race, deliberately:
it is the executable proof that the boundary must live above the data
structure, not inside it.

fb32d15's guard is KEPT, correcting this design's own section 5. Measured,
it guards TWO structures and only one was converted here: g->nodes/g->edges
are realloc'd in place (el_runtime.c:7618,7629) and engram_activate_inner's
embed-backfill writes n->emb through exactly such a borrowed pointer.
Deleting the guard reintroduces a measured 11171->9579 edge loss. Its
comment is narrowed to the RAM graph and the deletion precondition named.

That corrects the ordering claim too: the residual is not one ABI that
dissolves everything at once, it is a PROPERTY applied per structure.
Residues evaporate in the order the property is applied, and a residue
whose structure has not been converted must be left standing.

will.anderson merged commit a6cef4b983 into dev

2026-08-16 16:33:32 +00:00

will.anderson referenced this pull request

2026-08-16 16:39:06 +00:00

runtime: transduction is a language concern, so move it into the language #144

will.anderson referenced this issue from a commit

2026-08-16 17:20:26 +00:00

spec: correspondence and censorship — the root beneath the day's defects

~~will.anderson referenced this pull request 2026-08-16 17:20:41 +00:00~~

spec: correspondence, grounding, and the provenance of decisions #149

will.anderson referenced this issue from a commit

2026-08-16 20:50:05 +00:00