From 5e154fa15241a086c4efa5df56776db048f8f5b4 Mon Sep 17 00:00:00 2001 From: Will Anderson Date: Wed, 12 Aug 2026 16:44:09 -0500 Subject: [PATCH] =?UTF-8?q?engram:=20fix=20saved-but-not-findable=20?= =?UTF-8?q?=E2=80=94=20dedup=20boot-load=20scan=20by=20full=20id,=20not=20?= =?UTF-8?q?id=5Fhash?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit store_scan_nodes/store_scan_edges deduplicated emitted records by their 64-bit id_hash (FNV-1a-64) rather than the full id string. Two distinct ids that collide under id_hash emitted only the first; the second was durably on a live page and findable by store_get_node (which disambiguates by strcmp), yet silently dropped from the resident boot-load. After any store reopen that node was unretrievable by id, absent from lexical search, and missing from the recent list — the reported memory-integrity gap. Replace the hash-keyed U64Set with a StrSet: bucket by id_hash for O(1) probing but compare full ids by strcmp, mirroring the primary B+-tree readers. Same change for edges. Adds test_scan_collision.c (real FNV-1a-64 colliding ids). --- engram/test/test_scan_collision.c | 163 ++++++++++++++++++++++++++++++ lang/runtime/engram_store.c | 63 +++++++----- 2 files changed, 203 insertions(+), 23 deletions(-) create mode 100644 engram/test/test_scan_collision.c diff --git a/engram/test/test_scan_collision.c b/engram/test/test_scan_collision.c new file mode 100644 index 0000000..6780683 --- /dev/null +++ b/engram/test/test_scan_collision.c @@ -0,0 +1,163 @@ +/* test_scan_collision.c — regression gate for the "saved but not findable" bug. + * + * ROOT CAUSE UNDER TEST: store_scan_nodes / store_scan_edges (the boot-load + * path that populates the resident in-RAM graph — engram_store_boot -> + * eg_load_node_cb) deduplicated emitted records by their 64-bit id_hash + * (FNV-1a-64), NOT by the full id string. Two DISTINCT ids that collide under + * id_hash therefore emitted only the FIRST: the second node/edge was durably + * present in neuron.egm (store_get_node finds it), physically on a live page, + * yet was SILENTLY DROPPED from the resident load. After any store reopen it + * was unretrievable by id, absent from lexical search, and missing from the + * recent list — exactly the reported symptom. + * + * The two ids below are real FNV-1a-64 collisions (found offline via Brent's + * cycle detection over fnv1a(hex16(x))); both hash to 0x15141fdadfa24abe. + * + * Pure C. Writes ONLY under a throwaway /tmp dir. Never touches ~/.neuron. + */ +#include "../../lang/runtime/engram_store.h" + +#include +#include +#include +#include +#include +#include + +static int g_pass = 0, g_fail = 0; +static void ok(const char* name, int cond){ + printf(" [%s] %s\n", cond ? "PASS" : "FAIL", name); + if (cond) g_pass++; else g_fail++; +} + +/* Confirmed FNV-1a-64 collision (distinct strings, equal id_hash). */ +#define ID_A "d2c61ec7d015dc98" +#define ID_B "bf85e965a2aefbdd" + +static uint64_t fnv1a(const char* s){ + uint64_t h = 1469598103934665603ULL; + for (; *s; ++s){ h ^= (uint8_t)*s; h *= 1099511628211ULL; } + return h; +} + +static char g_dir[512]; +static void mk_dir(void){ + snprintf(g_dir, sizeof g_dir, "/tmp/engram-scancol-%d", (int)getpid()); + mkdir(g_dir, 0700); +} + +/* ── scan collectors: record which ids the boot-load scan actually emits ── */ +typedef struct { const char* want[8]; int seen[8]; int nwant; int total; } Collect; +static void node_cb(const StoreNode* n, void* ctx){ + Collect* c = ctx; c->total++; + for (int i=0;inwant;i++) if (n->id && strcmp(n->id, c->want[i])==0) c->seen[i]=1; +} +static void edge_cb(const StoreEdge* e, void* ctx){ + Collect* c = ctx; c->total++; + for (int i=0;inwant;i++) if (e->id && strcmp(e->id, c->want[i])==0) c->seen[i]=1; +} + +static void mk_node(StoreNode* n, const char* id, const char* content){ + memset(n, 0, sizeof *n); + n->id = strdup(id); + n->content = strdup(content); + n->node_type = strdup("Memory"); + n->label = strdup(content); + n->tier = strdup("Working"); + n->tags = strdup(""); + n->metadata = strdup("{}"); + n->salience = 0.5; n->importance = 0.5; n->confidence = 1.0; + n->created_at = 1700000000000LL; n->updated_at = 1700000000000LL; + n->last_activated = 1700000000000LL; +} +static void mk_edge(StoreEdge* e, const char* id, const char* from, const char* to){ + memset(e, 0, sizeof *e); + e->id = strdup(id); e->from_id = strdup(from); e->to_id = strdup(to); + e->relation = strdup("assoc"); e->metadata = strdup("{}"); + e->weight = 1.0; e->confidence = 1.0; + e->created_at = 1700000000000LL; e->updated_at = 1700000000000LL; +} + +int main(void){ + mk_dir(); + printf("== scan-collision regression (saved-but-not-findable) ==\n"); + printf(" id_hash(%s) = %016llx\n", ID_A, (unsigned long long)fnv1a(ID_A)); + printf(" id_hash(%s) = %016llx\n", ID_B, (unsigned long long)fnv1a(ID_B)); + ok("precondition: the two ids genuinely collide under id_hash", + fnv1a(ID_A) == fnv1a(ID_B) && strcmp(ID_A, ID_B) != 0); + + /* ---- Control: a single node survives a full store round-trip. ---- */ + { + EngramPagedStore* s = engram_open(g_dir); + StoreNode n; mk_node(&n, ID_A, "alpha distinctiveword"); + store_put_node(s, &n); + engram_close(s); /* checkpoint + close */ + + EngramPagedStore* r = engram_open(g_dir); + StoreNode got; + ok("control: single node found by id after reopen", store_get_node(r, ID_A, &got)==1); + if (0) {} else store_node_free(&got); + Collect c = {{ID_A}, {0}, 1, 0}; + store_scan_nodes(r, node_cb, &c); + ok("control: single node emitted by boot-load scan", c.seen[0]==1); + engram_close(r); + store_node_free(&n); + } + + /* ---- Bug: two id-hash-colliding NODES, both durable, both must load. ---- */ + { + char dir2[600]; snprintf(dir2, sizeof dir2, "%s/nodes", g_dir); mkdir(dir2, 0700); + EngramPagedStore* s = engram_open(dir2); + StoreNode a, b; + mk_node(&a, ID_A, "alpha distinctiveword-A"); + mk_node(&b, ID_B, "beta distinctiveword-B"); + store_put_node(s, &a); + store_put_node(s, &b); + engram_close(s); + store_node_free(&a); store_node_free(&b); + + EngramPagedStore* r = engram_open(dir2); + /* Both are individually durable (store_get_node disambiguates by strcmp). */ + StoreNode ga, gb; + int hit_a = store_get_node(r, ID_A, &ga); if (hit_a==1) store_node_free(&ga); + int hit_b = store_get_node(r, ID_B, &gb); if (hit_b==1) store_node_free(&gb); + ok("both colliding nodes are durably present (store_get_node)", hit_a==1 && hit_b==1); + + /* THE REGRESSION: the boot-load scan must emit BOTH, not silently drop one. */ + Collect c = {{ID_A, ID_B}, {0,0}, 2, 0}; + store_scan_nodes(r, node_cb, &c); + printf(" scan emitted A=%d B=%d (total=%d)\n", c.seen[0], c.seen[1], c.total); + ok("boot-load scan emits node A (would be resident)", c.seen[0]==1); + ok("boot-load scan emits node B (the dropped/unretrievable one)", c.seen[1]==1); + engram_close(r); + } + + /* ---- Bug: two id-hash-colliding EDGES, both must load. ---- */ + { + char dir3[600]; snprintf(dir3, sizeof dir3, "%s/edges", g_dir); mkdir(dir3, 0700); + EngramPagedStore* s = engram_open(dir3); + StoreNode na, nb; mk_node(&na, "src", "s"); mk_node(&nb, "dst", "d"); + store_put_node(s, &na); store_put_node(s, &nb); + StoreEdge ea, eb; + mk_edge(&ea, ID_A, "src", "dst"); + mk_edge(&eb, ID_B, "src", "dst"); + store_put_edge(s, &ea); + store_put_edge(s, &eb); + engram_close(s); + store_node_free(&na); store_node_free(&nb); + store_edge_free(&ea); store_edge_free(&eb); + + EngramPagedStore* r = engram_open(dir3); + Collect c = {{ID_A, ID_B}, {0,0}, 2, 0}; + store_scan_edges(r, edge_cb, &c); + printf(" scan emitted edgeA=%d edgeB=%d\n", c.seen[0], c.seen[1]); + ok("boot-load scan emits edge A", c.seen[0]==1); + ok("boot-load scan emits edge B (the dropped one)", c.seen[1]==1); + engram_close(r); + } + + printf("\n %d passed, %d failed\n", g_pass, g_fail); + /* cleanup */ + char cmd[600]; snprintf(cmd, sizeof cmd, "rm -rf %s", g_dir); if (system(cmd)){} + return g_fail ? 1 : 0; +} diff --git a/lang/runtime/engram_store.c b/lang/runtime/engram_store.c index d477296..017e393 100644 --- a/lang/runtime/engram_store.c +++ b/lang/runtime/engram_store.c @@ -1177,31 +1177,48 @@ uint64_t store_page_count(const EngramPagedStore* s){ return s ? s->page_count : /* ── M3: full live enumeration (boundary-clean; StoreNode/StoreEdge out only) ── * Page-walk every NODE/EDGE page, emitting each DISTINCT live record. A re-put * leaves several live records for one id (apply_node_put appends; reads dedup), - * so we track ids already emitted by their 64-bit id-hash — the same key the - * primary B+-tree uses (design §2.4) — and fetch the canonical latest-live via - * the point-read path so a scan and a get agree exactly. Used by the caller - * (el_runtime) to load the whole store resident at boot and to export JSON. */ -typedef struct { uint64_t* h; size_t n, cap; } U64Set; -static int u64set_add(U64Set* s, uint64_t v){ /* 1 = newly added, 0 = present */ + * so we track ids already emitted and fetch the canonical latest-live via the + * point-read path so a scan and a get agree exactly. Used by the caller + * (el_runtime) to load the whole store resident at boot and to export JSON. + * + * DEDUP IS BY FULL ID STRING, NOT BY id_hash. (2026-08-12 self-review — the + * "saved but not findable" bug.) The dedup set formerly keyed on the 64-bit + * id_hash alone; two DISTINCT ids that collide under FNV-1a-64 therefore + * emitted only the first, and the second — durably on a live page and findable + * by store_get_node, which disambiguates by strcmp — was SILENTLY DROPPED from + * the resident boot-load. After any reopen it was unretrievable by id, absent + * from lexical search, and missing from the recent list. The primary B+-tree + * keys on id_hash too, but every reader there re-reads the record and strcmp's + * the id; the scan's dedup must apply the same full-id discipline. Keyed on the + * hash for O(1) bucketing, compared by strcmp for correctness. */ +typedef struct { char* key; } StrSlot; +typedef struct { StrSlot* t; size_t n, cap; } StrSet; +static int strset_add(StrSet* s, const char* id){ /* 1 = newly added, 0 = present */ + if (!id) return 1; if ((s->n + 1) * 4 >= s->cap * 3){ size_t nc = s->cap ? s->cap * 2 : 1024; - uint64_t* nh = (uint64_t*)calloc(nc, sizeof(uint64_t)); - if (!nh) return 1; /* degrade rather than crash */ + StrSlot* nt = (StrSlot*)calloc(nc, sizeof(StrSlot)); + if (!nt) return 1; /* degrade rather than crash */ for (size_t i = 0; i < s->cap; i++){ - uint64_t k = s->h[i]; - if (k){ size_t j = k & (nc - 1); while (nh[j]) j = (j + 1) & (nc - 1); nh[j] = k; } + char* k = s->t[i].key; + if (k){ size_t j = id_hash(k) & (nc - 1); while (nt[j].key) j = (j + 1) & (nc - 1); nt[j].key = k; } } - free(s->h); s->h = nh; s->cap = nc; + free(s->t); s->t = nt; s->cap = nc; } - uint64_t k = v ? v : 1; /* 0 reserved as empty slot */ - size_t j = k & (s->cap - 1); - while (s->h[j]){ if (s->h[j] == k) return 0; j = (j + 1) & (s->cap - 1); } - s->h[j] = k; s->n++; return 1; + size_t j = id_hash(id) & (s->cap - 1); + while (s->t[j].key){ if (strcmp(s->t[j].key, id) == 0) return 0; j = (j + 1) & (s->cap - 1); } + s->t[j].key = strdup(id); + if (!s->t[j].key) return 1; /* OOM: don't dedup, never drop */ + s->n++; return 1; +} +static void strset_free(StrSet* s){ + for (size_t i = 0; i < s->cap; i++) free(s->t[i].key); + free(s->t); s->t = NULL; s->n = s->cap = 0; } int store_scan_nodes(EngramPagedStore* s, StoreNodeScanCb cb, void* ctx){ if (!s || !cb) return -1; - U64Set seen = {0, 0, 0}; + StrSet seen = {0, 0, 0}; uint8_t buf[STORE_PAGE_SIZE]; int count = 0; for (uint64_t pg = 2; pg < s->page_count; pg++){ @@ -1215,23 +1232,23 @@ int store_scan_nodes(EngramPagedStore* s, StoreNodeScanCb cb, void* ctx){ uint8_t* body; size_t blen; int live; if (read_body(s, pg, (uint16_t)i, &body, &blen, &live) != 0) continue; StoreNode cand; node_parse(body, blen, &cand); free(body); - if (cand.id && u64set_add(&seen, id_hash(cand.id))){ + if (cand.id && *cand.id && strset_add(&seen, cand.id)){ StoreNode canon; if (store_get_node(s, cand.id, &canon) == 1){ - cb(&canon, ctx); count++; + cb(&canon, ctx); count++; /* canonical latest-live */ store_node_free(&canon); } } store_node_free(&cand); } } - free(seen.h); + strset_free(&seen); return count; } int store_scan_edges(EngramPagedStore* s, StoreEdgeScanCb cb, void* ctx){ if (!s || !cb) return -1; - U64Set seen = {0, 0, 0}; + StrSet seen = {0, 0, 0}; uint8_t buf[STORE_PAGE_SIZE]; int count = 0; for (uint64_t pg = 2; pg < s->page_count; pg++){ @@ -1245,17 +1262,17 @@ int store_scan_edges(EngramPagedStore* s, StoreEdgeScanCb cb, void* ctx){ uint8_t* body; size_t blen; int live; if (read_body(s, pg, (uint16_t)i, &body, &blen, &live) != 0) continue; StoreEdge cand; edge_parse(body, blen, &cand); free(body); - if (cand.id && u64set_add(&seen, id_hash(cand.id))){ + if (cand.id && *cand.id && strset_add(&seen, cand.id)){ StoreEdge canon; if (store_get_edge(s, cand.id, &canon) == 1){ - cb(&canon, ctx); count++; + cb(&canon, ctx); count++; /* canonical latest-live */ store_edge_free(&canon); } } store_edge_free(&cand); } } - free(seen.h); + strset_free(&seen); return count; }