Improve snapshot reliability and usability

tests/snapshot_diff.test.mjs

@@ -0,0 +1,88 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+
+import {
+    computeDetailedDiff,
+    deepEqual,
+    detectChangeType,
+    isMeaningfulChangeType,
+} from "../js/snapshot_diff.js";
+
+function graph(overrides = {}) {
+    return {
+        nodes: [{
+            id: 1,
+            type: "Example",
+            pos: [10, 20],
+            size: [200, 100],
+            flags: {},
+            mode: 0,
+            widgets_values: [{ enabled: true, values: [1, 2] }],
+            properties: { precision: { value: 16 } },
+        }],
+        links: [[1, 1, 0, 2, 0, "IMAGE"]],
+        ...overrides,
+    };
+}
+
+test("deepEqual compares composite widget values structurally", () => {
+    assert.equal(deepEqual({ a: [1, { b: true }] }, { a: [1, { b: true }] }), true);
+    assert.equal(deepEqual({ a: [1, { b: true }] }, { a: [1, { b: false }] }), false);
+});
+
+test("equal composite widget content is not a parameter change", () => {
+    const before = graph();
+    const after = structuredClone(before);
+    after.nodes[0].pos = [40, 60];
+    assert.equal(detectChangeType(before, after), "cosmetic");
+    assert.equal(isMeaningfulChangeType(detectChangeType(before, after)), false);
+});
+
+test("nested widget content changes are parameter changes", () => {
+    const before = graph();
+    const after = structuredClone(before);
+    after.nodes[0].widgets_values[0].values[1] = 3;
+    assert.equal(detectChangeType(before, after), "param");
+});
+
+test("properties changed during a move remain meaningful", () => {
+    const before = graph();
+    const after = structuredClone(before);
+    after.nodes[0].pos = [100, 200];
+    after.nodes[0].properties.precision.value = 32;
+    assert.equal(detectChangeType(before, after), "param");
+});
+
+test("a rewired link with the same id is represented as remove plus add", () => {
+    const before = graph();
+    const after = structuredClone(before);
+    after.links[0][3] = 3;
+    assert.equal(detectChangeType(before, after), "connection");
+    const diff = computeDetailedDiff(before, after);
+    assert.equal(diff.removedLinks.length, 1);
+    assert.equal(diff.addedLinks.length, 1);
+    assert.equal(diff.removedLinks[0].destNodeId, 2);
+    assert.equal(diff.addedLinks[0].destNodeId, 3);
+});
+
+test("node additions take precedence over their associated links", () => {
+    const before = graph({ nodes: [], links: [] });
+    const after = graph();
+    assert.equal(detectChangeType(before, after), "node_add");
+});
+
+test("group movement is cosmetic but group naming is meaningful", () => {
+    const before = graph({
+        groups: [{ id: 1, title: "Inputs", bounding: [0, 0, 200, 200], color: "#333" }],
+    });
+    const moved = structuredClone(before);
+    moved.groups[0].bounding = [50, 50, 200, 200];
+    assert.equal(detectChangeType(before, moved), "cosmetic");
+
+    const renamed = structuredClone(before);
+    renamed.groups[0].title = "Sources";
+    assert.equal(detectChangeType(before, renamed), "param");
+    const diff = computeDetailedDiff(before, renamed);
+    assert.equal(diff.summary.groupsChanged, 1);
+    assert.match(diff.groupChanges[0].detail, /Inputs.*Sources/);
+});

tests/test_snapshot_storage.py

@@ -0,0 +1,126 @@
+import json
+import tempfile
+import unittest
+from pathlib import Path
+
+import snapshot_storage as storage
+
+
+def make_record(snapshot_id, timestamp=1, **overrides):
+    record = {
+        "id": snapshot_id,
+        "workflowKey": "workflow/example.json",
+        "timestamp": timestamp,
+        "label": "Auto",
+        "source": "auto",
+        "locked": False,
+        "graphData": {"nodes": [{"id": 1, "type": "Test"}], "links": []},
+    }
+    record.update(overrides)
+    return record
+
+
+class SnapshotStorageTests(unittest.TestCase):
+    def setUp(self):
+        self.tempdir = tempfile.TemporaryDirectory()
+        self.old_data_dir = storage._DATA_DIR
+        self.old_profiles_dir = storage._PROFILES_DIR
+        storage._DATA_DIR = self.tempdir.name
+        storage._PROFILES_DIR = str(Path(self.tempdir.name) / "profiles")
+        storage._cache.clear()
+        storage._cache_warmed.clear()
+        storage._profile_cache = None
+
+    def tearDown(self):
+        storage._cache.clear()
+        storage._cache_warmed.clear()
+        storage._profile_cache = None
+        storage._DATA_DIR = self.old_data_dir
+        storage._PROFILES_DIR = self.old_profiles_dir
+        self.tempdir.cleanup()
+
+    def test_put_lists_metadata_and_round_trips_graph(self):
+        record = make_record("snap-1")
+        storage.put(record)
+
+        listed = storage.get_all_for_workflow(record["workflowKey"])
+        self.assertEqual([entry["id"] for entry in listed], ["snap-1"])
+        self.assertNotIn("graphData", listed[0])
+        self.assertEqual(storage.get_full_record(record["workflowKey"], "snap-1"), record)
+
+        snapshot_files = [path for path in Path(self.tempdir.name).rglob("*.json") if path.is_file()]
+        self.assertEqual(len(snapshot_files), 1)
+        json.loads(snapshot_files[0].read_text(encoding="utf-8"))
+
+    def test_rejects_invalid_record_and_path_components(self):
+        with self.assertRaises(ValueError):
+            storage.put(make_record("../escape"))
+        with self.assertRaises(ValueError):
+            storage.put(make_record("snap-1", graphData={"links": []}))
+        with self.assertRaises(ValueError):
+            storage.put(make_record("snap-1", source="mystery"))
+        with self.assertRaises(ValueError):
+            storage.get_full_record("workflow", 123)
+
+    def test_metadata_allowlist_cannot_replace_graph(self):
+        record = make_record("snap-1")
+        storage.put(record)
+
+        with self.assertRaises(ValueError):
+            storage.update_meta(
+                record["workflowKey"],
+                record["id"],
+                {"graphData": {"nodes": []}},
+            )
+
+        full = storage.get_full_record(record["workflowKey"], record["id"])
+        self.assertEqual(full["graphData"], record["graphData"])
+        self.assertTrue(storage.update_meta(
+            record["workflowKey"], record["id"], {"notes": "keeper", "locked": True}
+        ))
+        updated = storage.get_full_record(record["workflowKey"], record["id"])
+        self.assertEqual(updated["notes"], "keeper")
+        self.assertTrue(updated["locked"])
+
+    def test_prune_preserves_locked_and_keeps_newest_unlocked(self):
+        key = "workflow/example.json"
+        storage.put(make_record("old", timestamp=1))
+        storage.put(make_record("locked", timestamp=2, locked=True))
+        storage.put(make_record("new", timestamp=3))
+
+        self.assertEqual(storage.prune(key, 1, source="regular"), 1)
+        remaining = {entry["id"] for entry in storage.get_all_for_workflow(key)}
+        self.assertEqual(remaining, {"locked", "new"})
+
+    def test_delete_all_keeps_locked_snapshots(self):
+        key = "workflow/example.json"
+        storage.put(make_record("unlocked", timestamp=1))
+        storage.put(make_record("locked", timestamp=2, locked=True))
+
+        self.assertEqual(storage.delete_all_for_workflow(key), {"lockedCount": 1})
+        self.assertEqual(
+            [entry["id"] for entry in storage.get_all_for_workflow(key)],
+            ["locked"],
+        )
+
+    def test_profile_accepts_exact_snapshot_refs_and_rejects_bad_shape(self):
+        profile = {
+            "id": "profile-1",
+            "name": "Editing",
+            "timestamp": 10,
+            "activeWorkflowKey": "workflow/example.json",
+            "workflows": [{
+                "workflowKey": "workflow/example.json",
+                "displayName": "Example",
+                "snapshotId": "snap-1",
+            }],
+        }
+        storage.profile_put(profile)
+        self.assertEqual(storage.profile_get("profile-1"), profile)
+
+        with self.assertRaises(ValueError):
+            storage.profile_put({"id": "bad", "name": "Bad", "timestamp": 1, "workflows": "nope"})
+
+
+if __name__ == "__main__":
+    unittest.main()