@ruvector/rvf: saveMappings()/loadMappings() swallow errors on the load-bearing .idmap.json sidecar — silent no-op deletes, label collisions on corrupt load

[KevinAIP]

Sibling of #634 — same durability defect class (silent swallow + non-atomic write + corrupt-load-to-empty-defaults), different package: @ruvector/rvf's NodeBackend id-map sidecar. Verified on @ruvector/rvf@0.2.2 (dist is byte-identical to 0.2.1 for backend.js).

Context

Since 0.2.0, NodeBackend maintains a string↔i64-label map persisted as a .idmap.json sidecar next to the store. This sidecar is load-bearing: delete(ids) resolves string ids through idToLabel and silently filters out anything unresolvable, so with a missing/corrupt/stale sidecar it returns {deleted: 0} with no error. Two swallows make that failure mode easy to hit and impossible to see:

Defect 1 — saveMappings() (dist/backend.js:381)

fs.writeFileSync(mp, data, 'utf-8');   // non-atomic
...
catch {
    // Non-fatal: mapping persistence is best-effort (e.g. read-only FS).
}

• A failed write is silently discarded → every vector ingested since the last good save becomes undeletable by id, with zero signal.
• A torn write (crash/ENOSPC mid-writeFileSync) leaves invalid JSON on disk, which then triggers defect 2 on the next open.

Mapping persistence isn't "best-effort" when delete() correctness depends on it.

Defect 2 — loadMappings() (dist/backend.js:399)

catch {
    // Non-fatal: start with empty mappings.
}

A corrupt sidecar silently degrades to empty maps, which is worse than failing:

• nextLabel resets to idToLabel.size + 1 = 1, so subsequent ingestBatch calls assign labels that collide with existing vectors' labels — silent data corruption, not just data loss.
• delete() silently no-ops for every historical id.
• The next saveMappings() overwrites the (recoverable) corrupt sidecar with the empty/colliding state, destroying the evidence.

Suggested fix (patch we're running in production)

Atomic tmp+rename in saveMappings() with the error surfaced; quarantine + throw on corrupt load instead of empty-defaults. Applied via patch-package on 0.2.2:

patches/@ruvector+rvf+0.2.2.patch

diff --git a/node_modules/@ruvector/rvf/dist/backend.js b/node_modules/@ruvector/rvf/dist/backend.js
index 64ffa6c..fdf1973 100644
--- a/node_modules/@ruvector/rvf/dist/backend.js
+++ b/node_modules/@ruvector/rvf/dist/backend.js
@@ -382,17 +382,29 @@ class NodeBackend {
         const mp = this.mappingsPath();
         if (!mp)
             return;
+        // AIP PATCH (#8 / ADR-0017 recipe): the sidecar is load-bearing —
+        // without it delete() silently no-ops for every historical id. The
+        // stock build wrapped a non-atomic writeFileSync in a bare catch{},
+        // so a failed or torn write silently orphaned vectors. Write
+        // atomically (tmp + rename) and surface failures instead.
+        const fs = await Promise.resolve().then(() => __importStar(require('fs')));
+        const data = JSON.stringify({
+            idToLabel: Object.fromEntries(this.idToLabel),
+            labelToId: Object.fromEntries(Array.from(this.labelToId.entries()).map(([k, v]) => [String(k), v])),
+            nextLabel: this.nextLabel,
+        });
+        const tmp = `${mp}.tmp.${process.pid}`;
         try {
-            const fs = await Promise.resolve().then(() => __importStar(require('fs')));
-            const data = JSON.stringify({
-                idToLabel: Object.fromEntries(this.idToLabel),
-                labelToId: Object.fromEntries(Array.from(this.labelToId.entries()).map(([k, v]) => [String(k), v])),
-                nextLabel: this.nextLabel,
-            });
-            fs.writeFileSync(mp, data, 'utf-8');
+            fs.writeFileSync(tmp, data, 'utf-8');
+            fs.renameSync(tmp, mp);
         }
-        catch {
-            // Non-fatal: mapping persistence is best-effort (e.g. read-only FS).
+        catch (err) {
+            try {
+                fs.rmSync(tmp, { force: true });
+            }
+            catch { }
+            console.error(`[rvf] failed to persist id-map sidecar at ${mp}: ${err?.message ?? err}`);
+            throw err;
         }
     }
     /** Load the string↔label mapping from the sidecar JSON file if it exists. */
@@ -400,17 +412,29 @@ class NodeBackend {
         const mp = this.mappingsPath();
         if (!mp)
             return;
+        const fs = await Promise.resolve().then(() => __importStar(require('fs')));
+        if (!fs.existsSync(mp))
+            return;
+        // AIP PATCH (#8 / ADR-0017 recipe): a corrupt sidecar must not
+        // silently degrade to empty mappings — that resets nextLabel to 1
+        // (label collisions with existing vectors) and turns delete() into
+        // a silent no-op for every historical id. Quarantine the bad file
+        // and fail loudly so recovery (sidecar resynth or --rebuild-store)
+        // is a deliberate act, not an accident.
         try {
-            const fs = await Promise.resolve().then(() => __importStar(require('fs')));
-            if (!fs.existsSync(mp))
-                return;
             const raw = JSON.parse(fs.readFileSync(mp, 'utf-8'));
             this.idToLabel = new Map(Object.entries(raw.idToLabel ?? {}).map(([k, v]) => [k, Number(v)]));
             this.labelToId = new Map(Object.entries(raw.labelToId ?? {}).map(([k, v]) => [Number(k), v]));
             this.nextLabel = raw.nextLabel ?? this.idToLabel.size + 1;
         }
-        catch {
-            // Non-fatal: start with empty mappings.
+        catch (err) {
+            const quarantine = `${mp}.corrupt.${Date.now()}`;
+            try {
+                fs.renameSync(mp, quarantine);
+            }
+            catch { }
+            console.error(`[rvf] id-map sidecar at ${mp} is unreadable (${err?.message ?? err}); quarantined to ${quarantine}`);
+            throw err;
         }
     }
 }

Related observations from the same verification pass

• @ruvector/rvf@0.2.x declares "@ruvector/rvf-node": "^0.1.7", so a stock install resolves a 0.1.x native layer — worth checking whether deleteByFilter (added to the SDK in 0.2.0) exists in the native it actually gets. (@ruvector/rvf-node@0.2.0's own optionalDependencies pin darwin-arm64 at 0.1.7 as well.)
• The SDK's ingestBatch never forwards entry metadata to the NAPI layer (this.handle.ingestBatch(flat, ids) — the metadata? param from the signature comment is dropped), so deleteByFilter has nothing to filter on for SDK-ingested stores.

Happy to PR the sidecar durability patch if useful.