engram: fix saved-but-not-findable — dedup boot-load scan by full id, not id_hash
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).
This commit is contained in:
+40
-23
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user