fix(store): round-trip VAL_BUFFER through ext_store instead of encoding it as null

CHANGELOG.md

@@ -36,6 +36,36 @@ All notable changes to EigenScript are documented here.
 
 ### Fixed
 
+- **EigenStore round-trips buffers instead of dropping them (#805).**
+  `store_json_encode` wrote a `VAL_BUFFER` as JSON `null`, so
+  `store_put of [db, "c", {"b": buf}]` followed by `store_get` handed
+  back a null and the bytes were gone — the data-loss gap the #738
+  exhaustiveness sweep made visible. A buffer now encodes as a tagged
+  object, `{"_eigs_buffer":[…],"_eigs_shape":[rows,cols]}`, that the
+  store's decoder rebuilds as a `VAL_BUFFER` at any nesting depth. The
+  alternative — a bare numeric array — round-trips as a **list**, and a
+  store that silently changes a value's type is the same class of bug.
+  Shape travels with it (`shape of` reads rows/cols back, so dropping
+  them is data loss too), elements use `%.17g` so a non-integral buffer
+  comes back bit-exact (as trace.c's tape encoding already did), and a
+  non-finite element — reachable, the flat-buffer matmul kernel
+  accumulates unguarded — rides as `"inf"`/`"-inf"`/`"nan"` rather than
+  as a bare `inf` that would make the whole *record* unparseable.
+  **The tag's cost, stated plainly:** a user dict with exactly the two
+  tag keys, a body list of numbers (or those sentinels), and a shape
+  list of two integers satisfying `rows*cols == count` (or `[0,0]`) now
+  decodes as a buffer. Nothing else does — a third key, a missing key, a
+  non-list body, a foreign string element, or an inconsistent shape all
+  stay a dict, and a top-level record is never at risk because
+  `store_put` stamps `_id` onto it. `STORE_VERSION` is deliberately not
+  bumped (`store_read_header` rejects any other version, so a bump would
+  make every existing database unopenable); old files decode exactly as
+  before, and an older binary reading a new file sees a plain dict
+  rather than failing. One consequence: a buffer costs real bytes where
+  `null` cost four, so a large one can push a record past
+  `STORE_MAX_RECORD_SIZE` and raise "record exceeds page size" — the
+  limit a long list already hits, and a loud refusal beats a silent null.
+
 - **The `ValType` switches are now exhaustive too, closing out the #738
   sweep — and the first build immediately caught real drift (#738).**
   The ASTType half of #738 landed earlier (see below); this finishes the

src/ext_store.c

@@ -84,6 +84,77 @@
 
 static int page_data_used(Page *page);
 
+/* ---- Tagged VAL_BUFFER encoding (#805) ---------------------------------
+ *
+ * JSON has no buffer type, so a stored buffer used to encode as `null` and a
+ * load returned null where bytes were saved — silent data loss. A buffer now
+ * rides as a two-key object:
+ *
+ *     {"_eigs_buffer":[<elements>],"_eigs_shape":[rows,cols]}
+ *
+ * The encoder always writes both keys in that order; the decoder looks them
+ * up by name, so key order is not load-bearing. `_eigs_shape` carries the
+ * value's own rows/cols ([0,0] for an unshaped 1-D buffer — `shape of` reads
+ * those back, so dropping them would be data loss of the same class).
+ * Encoding as a bare numeric array was the alternative; it round-trips as a
+ * VAL_LIST, and a store that silently changes a value's type is the bug being
+ * fixed.
+ *
+ * A stored buffer now costs real bytes, where before it cost four (`null`),
+ * so a large buffer can push a record past STORE_MAX_RECORD_SIZE and make
+ * store_put raise "record exceeds page size". That is the same per-record
+ * limit a long list already hits, and a raise is the outcome this issue is
+ * about: a loud refusal instead of a silent null.
+ *
+ * There is no pre-existing type-tag convention in this file to follow: the
+ * only reserved names are the `_`-prefixed handle/record fields (`_id`,
+ * `_store`, `_store_id`, `_type`), and trace.c's `b[...]` buffer form belongs
+ * to the tape's own non-JSON grammar. The `_eigs_` prefix is new here and
+ * matches that `_`-prefixed reserved-name shape.
+ *
+ * COLLISION — the cost of any tagged encoding, stated plainly: a user dict
+ * with exactly these two keys, `_eigs_buffer` holding a list whose every
+ * element is a number or one of the three sentinels below, and `_eigs_shape`
+ * holding exactly two integral numbers that satisfy the shape invariant
+ * (rows==0 && cols==0, or rows*cols == element count), now decodes as a
+ * buffer. Nothing else does — a third key, a missing key, a non-list body, a
+ * string element that is not a sentinel, or an inconsistent shape all leave
+ * the value a plain dict. The top-level record is never at risk either:
+ * store_put stamps `_id` onto it, so it always carries a third key.
+ *
+ * BACKWARD COMPATIBILITY — the store is a file (`store_open of path`), so
+ * databases written by older builds do exist. The old encoder never emitted
+ * this shape (buffers became `null`), so old records decode exactly as
+ * before, apart from the collision above. STORE_VERSION is deliberately NOT
+ * bumped: store_read_header rejects any other version outright, so a bump
+ * would make every existing database unopenable to fix a value that was
+ * already lost. An older binary reading a new file sees the tag as a plain
+ * dict rather than failing.
+ */
+#define STORE_BUF_TAG   "_eigs_buffer"
+#define STORE_SHAPE_TAG "_eigs_shape"
+
+/* Non-finite buffer elements. JSON has no nan/infinity literal, and emitting
+ * a bare `nan` would make the whole record unparseable — turning one lost
+ * buffer into one lost record. They ride as these three strings, which are
+ * meaningful only inside a tag body. Returns 1 and (if `out`) the value. */
+static int store_nonfinite_sentinel(const char *s, double *out) {
+    if (strcmp(s, "nan") == 0)  { if (out) *out = (double)NAN; return 1; }
+    if (strcmp(s, "inf") == 0)  { if (out) *out = (double)INFINITY; return 1; }
+    if (strcmp(s, "-inf") == 0) { if (out) *out = -(double)INFINITY; return 1; }
+    return 0;
+}
+
+/* Buffer elements use the 17-significant-digit form, as trace.c's buffer tape
+ * encoding does: %.17g is the shortest width that round-trips a double
+ * bit-exactly, and a buffer holding samples or weights would otherwise come
+ * back subtly altered. (VAL_NUM's own %d/%.15g encoding is untouched.) */
+static void store_json_encode_buf_elem(strbuf *out, double d) {
+    if (isnan(d)) { strbuf_append(out, "\"nan\""); return; }
+    if (isinf(d)) { strbuf_append(out, d < 0 ? "\"-inf\"" : "\"inf\""); return; }
+    strbuf_append_fmt(out, "%.17g", d);
+}
+
 static void store_json_encode(Value *v, strbuf *out) {
     if (!v || v->type == VAL_NULL || v->type == VAL_FN || v->type == VAL_BUILTIN) {
         strbuf_append(out, "null");
@@ -127,18 +198,28 @@
             strbuf_append_char(out, '}');
             break;
         }
+        case VAL_BUFFER: {
+            /* Tagged so the decoder rebuilds a VAL_BUFFER rather than a list
+             * or a null — see the STORE_BUF_TAG block above for the shape,
+             * its collision surface, and the compatibility argument. */
+            strbuf_append(out, "{\"" STORE_BUF_TAG "\":[");
+            for (int i = 0; i < v->data.buffer.count; i++) {
+                if (i > 0) strbuf_append_char(out, ',');
+                store_json_encode_buf_elem(out, v->data.buffer.data[i]);
+            }
+            strbuf_append_fmt(out, "],\"" STORE_SHAPE_TAG "\":[%d,%d]}",
+                              v->data.buffer.rows, v->data.buffer.cols);
+            break;
+        }
         /* VAL_NULL/VAL_FN/VAL_BUILTIN are handled by the guard above; raw
-         * JSON, text builders and buffers have no store encoding (buffers
-         * silently store as null — a data-loss gap this enumeration made
-         * visible; tracked upstream). Enumerated rather than covered by a
-         * `default:` so -Werror=switch forces a new ValType to choose its
-         * store-JSON encoding here. */
+         * JSON and text builders have no store encoding. Enumerated rather
+         * than covered by a `default:` so -Werror=switch forces a new ValType
+         * to choose its store-JSON encoding here. */
         case VAL_NULL:
         case VAL_FN:
         case VAL_BUILTIN:
         case VAL_JSON_RAW:
         case VAL_TEXT_BUILDER:
-        case VAL_BUFFER:
             strbuf_append(out, "null");
             break;
     }
@@ -220,6 +301,73 @@
     }
 }
 
+/* If `dict` is exactly the STORE_BUF_TAG shape, build the VAL_BUFFER it
+ * encodes; otherwise return NULL and leave `dict` alone. Every clause here is
+ * a collision guard — the tighter the match, the less user data the tag can
+ * swallow (see the STORE_BUF_TAG block for the surface that remains).
+ * Ownership: the returned buffer is a fresh owned ref; the caller drops the
+ * dict. */
+static Value* store_buffer_from_tag(Value *dict) {
+    if (dict->data.dict.count != 2) return NULL;
+    Value *body  = dict_get(dict, STORE_BUF_TAG);
+    Value *shape = dict_get(dict, STORE_SHAPE_TAG);
+    if (!body || body->type != VAL_LIST) return NULL;
+    if (!shape || shape->type != VAL_LIST || shape->data.list.count != 2) return NULL;
+
+    Value *rv = shape->data.list.items[0];
+    Value *cv = shape->data.list.items[1];
+    if (!rv || rv->type != VAL_NUM || !cv || cv->type != VAL_NUM) return NULL;
+
+    /* Range-gate BEFORE narrowing. A stored shape is whatever double the file
+     * holds, and (int)d is undefined when d is outside int's range (C11
+     * 6.3.1.4p1) — reachable here with e.g. [1e300, 0]. The negated form also
+     * rejects a NaN, which fails every comparison. Only once the value is
+     * known in-range is the exact `==` below meaningful: it is an INTEGRALITY
+     * test (is this dimension a whole number?), so exact equality is the
+     * correct operator and a tolerance would be the bug — 2.0000000000000004
+     * is not a row count. This also establishes rows/cols >= 0. */
+    double rd = rv->data.num, cd = cv->data.num;
+    if (!(rd >= 0 && rd <= (double)INT_MAX)) return NULL;
+    if (!(cd >= 0 && cd <= (double)INT_MAX)) return NULL;
+    int rows = (int)rd, cols = (int)cd;
+    if (rd != (double)rows || cd != (double)cols) return NULL;
+
+    /* Accept exactly the (rows, cols, count) triples the buffer constructors
+     * can produce, no more. rows>0 is a real 2-D shape, so rows*cols must be
+     * the element count. rows==0 with cols>0 is `buffer of [0, n]` — a
+     * degenerate shape that yields an empty buffer, so the count must be 0.
+     * rows==0 && cols==0 is the unshaped 1-D buffer, any count. */
+    int count = body->data.list.count;
+    if (rows > 0) {
+        if ((int64_t)rows * (int64_t)cols != (int64_t)count) return NULL;
+    } else if (cols > 0) {
+        if (count != 0) return NULL;
+    }
+    for (int i = 0; i < count; i++) {
+        Value *e = body->data.list.items[i];
+        if (!e) return NULL;
+        if (e->type == VAL_NUM) continue;
+        if (e->type == VAL_STR && store_nonfinite_sentinel(e->data.str, NULL)) continue;
+        return NULL;
+    }
+
+    Value *buf = xcalloc(1, sizeof(Value));
+    buf->type = VAL_BUFFER;
+    buf->refcount = 1;
+    buf->data.buffer.count = count;
+    buf->data.buffer.rows = rows;
+    buf->data.buffer.cols = cols;
+    buf->data.buffer.data = xcalloc(count > 0 ? (size_t)count : 1, sizeof(double));
+    for (int i = 0; i < count; i++) {
+        Value *e = body->data.list.items[i];
+        double d = 0;
+        if (e->type == VAL_NUM) d = e->data.num;
+        else store_nonfinite_sentinel(e->data.str, &d);
+        buf->data.buffer.data[i] = d;
+    }
+    return buf;
+}
+
 static Value* store_json_parse_object(const char *s, int *pos) {
     if (s[*pos] != '{') return NULL;
     (*pos)++;
@@ -244,7 +392,14 @@
         val_decref(key);
         store_json_skip_ws(s, pos);
         if (s[*pos] == ',') { (*pos)++; continue; }
-        if (s[*pos] == '}') { (*pos)++; return dict; }
+        if (s[*pos] == '}') {
+            (*pos)++;
+            /* A completed object may be the buffer tag (#805) — at any depth,
+             * so the check lives here rather than at the record root. */
+            Value *buf = store_buffer_from_tag(dict);
+            if (buf) { val_decref(dict); return buf; }
+            return dict;
+        }
         val_decref(dict); return NULL;  /* expected , or } */
     }
 }

tests/test_store.eigs

@@ -84,12 +84,110 @@ assert_eq of [li.tags[2], "blue", "list last element"]
 assert_eq of [li.scores[1], 20, "numeric list element"]
 assert_eq of [len of li.empty, 0, "empty list field round-trips"]
 
+# Buffer-valued fields — #805. store_json_encode wrote VAL_BUFFER as JSON
+# `null`, so a stored buffer came back as null and the bytes were silently
+# gone. The tagged encoding restores both the type and the exact contents.
+# The interesting bytes are a zero byte and bytes that are not valid UTF-8
+# (0xFF/0xFE): those are exactly where a naive string encoding breaks.
+bz is buffer of 5
+bz[0] is 0
+bz[1] is 65
+bz[2] is 0
+bz[3] is 255
+bz[4] is 254
+bk is store_put of [db2, "blobs", {"name": "raw", "bytes": bz}]
+bi is store_get of [db2, "blobs", bk]
+assert_eq of [type of bi.bytes, "buffer", "buffer field decodes as a buffer"]
+assert_eq of [len of bi.bytes, 5, "buffer field length"]
+assert_eq of [bi.bytes[0], 0, "buffer keeps a leading zero byte"]
+assert_eq of [bi.bytes[1], 65, "buffer keeps an ASCII byte"]
+assert_eq of [bi.bytes[2], 0, "buffer keeps an interior zero byte"]
+assert_eq of [bi.bytes[3], 255, "buffer keeps 0xFF (invalid UTF-8)"]
+assert_eq of [bi.bytes[4], 254, "buffer keeps 0xFE (invalid UTF-8)"]
+assert_eq of [bi.name, "raw", "sibling field survives next to a buffer"]
+
+# Non-integral elements round-trip bit-exactly (the element encoder uses the
+# 17-significant-digit form, as trace.c's buffer tape encoding does).
+fb is buffer of 2
+fb[0] is 0.1
+fb[1] is 0 - 2.5
+fk is store_put of [db2, "blobs", {"f": fb}]
+fi is store_get of [db2, "blobs", fk]
+assert_eq of [fi.f[0], 0.1, "fractional buffer element is exact"]
+assert_eq of [fi.f[1], 0 - 2.5, "negative buffer element is exact"]
+
+# A non-finite element survives too. It is reachable — the flat-buffer matmul
+# kernel accumulates without num_guard, so a large enough product overflows to
+# +Inf — and `inf` is not a JSON literal, so encoding it bare would cost the
+# whole RECORD, not just the buffer. It rides as a sentinel string instead.
+nb is buffer of 4
+nb[0] is 1e300
+nb[1] is 1e300
+nb[2] is 1e300
+nb[3] is 1e300
+nm is reshape of [nb, 2, 2]
+np is matmul of [nm, nm]
+assert_true of [np[0] > 1e308, "matmul overflow really is non-finite"]
+nk is store_put of [db2, "blobs", {"n": np}]
+ni is store_get of [db2, "blobs", nk]
+assert_eq of [type of ni.n, "buffer", "non-finite buffer still decodes as a buffer"]
+assert_eq of [ni.n[0], np[0], "non-finite buffer element round-trips"]
+
+# An empty buffer stays an empty buffer, not a null and not an empty list.
+eb is buffer of 0
+ek is store_put of [db2, "blobs", {"e": eb}]
+ei is store_get of [db2, "blobs", ek]
+assert_eq of [type of ei.e, "buffer", "empty buffer decodes as a buffer"]
+assert_eq of [len of ei.e, 0, "empty buffer length"]
+
+# The degenerate shapes `buffer of` accepts — [0, n] (rows 0, cols kept, no
+# elements) and [n, 0] — are still buffers on the way back. The decoder's
+# shape check has to accept every triple the constructors can build, or it
+# quietly hands back a dict.
+zb is buffer of [0, 5]
+wb is buffer of [3, 0]
+zk is store_put of [db2, "blobs", {"z": zb, "w": wb}]
+zi is store_get of [db2, "blobs", zk]
+assert_eq of [type of zi.z, "buffer", "buffer of [0, n] decodes as a buffer"]
+assert_eq of [type of zi.w, "buffer", "buffer of [n, 0] decodes as a buffer"]
+
+# A 2-D shaped buffer keeps its shape (rows/cols are part of the value —
+# `shape of` reads them back, so dropping them is data loss too).
+gb is buffer of 4
+gb[0] is 1
+gb[1] is 2
+gb[2] is 3
+gb[3] is 4
+gs is reshape of [gb, 2, 2]
+gk is store_put of [db2, "blobs", {"m": gs}]
+gi is store_get of [db2, "blobs", gk]
+assert_eq of [type of gi.m, "buffer", "shaped buffer decodes as a buffer"]
+assert_eq of [len of (shape of gi.m), 2, "shaped buffer stays 2-D"]
+assert_eq of [(shape of gi.m)[0], 2, "shaped buffer rows"]
+assert_eq of [(shape of gi.m)[1], 2, "shaped buffer cols"]
+assert_eq of [gi.m[3], 4, "shaped buffer contents"]
+assert_eq of [len of (shape of bi.bytes), 1, "unshaped buffer stays 1-D"]
+
+# The buffer tag must not swallow ordinary values: a plain dict field and a
+# plain list field still decode as dict and list.
+dk is store_put of [db2, "blobs", {"d": {"a": 1}, "l": [1, 2]}]
+di is store_get of [db2, "blobs", dk]
+assert_eq of [type of di.d, "dict", "an ordinary dict field is still a dict"]
+assert_eq of [di.d.a, 1, "ordinary dict field contents"]
+assert_eq of [type of di.l, "list", "an ordinary list field is still a list"]
+
 # Nested list survives a close/reopen too (parsed from disk, not cache).
 store_close of db2
 db3 is store_open of "/tmp/test_eigenstore.db"
 li2 is store_get of [db3, "items", lk]
 assert_eq of [len of li2.tags, 3, "list field persists across reopen"]
 assert_eq of [li2.scores[2], 30, "numeric list persists"]
+bi2 is store_get of [db3, "blobs", bk]
+assert_eq of [type of bi2.bytes, "buffer", "buffer field persists as a buffer"]
+assert_eq of [bi2.bytes[0], 0, "buffer zero byte persists"]
+assert_eq of [bi2.bytes[3], 255, "buffer 0xFF persists"]
+gi2 is store_get of [db3, "blobs", gk]
+assert_eq of [(shape of gi2.m)[1], 2, "buffer shape persists"]
 store_close of db3
 
 # --- Corruption recovery: store_open rejects invalid files cleanly ---