From fa218b942806eebdca6b3869147f2bd03a2009d0 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Tue, 26 May 2026 16:15:40 +0000
Subject: [PATCH 1/4] test: add model-based property tests for the store
 structures; fix PathRefStore ref leak
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Adds randomized model-based property tests for PathRefStore, ValueStore,
and ValueRefStore. Each drives random operation sequences against the
real store and a from-scratch reference model, asserting they agree and
that the structures empty out fully. The seed and op log are printed on
failure for replay.

The harness immediately found a PathRefStore bug. In `_dec`, when a path
had become a transient node (it and a descendant were inc'd, then the
descendant dec'd), dec'ing the path itself hit the Map branch, which
signalled removal via a return value — but the top-level `dec` ignores
that return, so the node lingered: `has` stayed true and the ref count
leaked. The int branch directly above already handled its analogous case
correctly. Fixed to remove the node in place (or drop just its ref key if
it still routes to live descendants), symmetric with the int branch.

Reachable with only well-formed inc/dec pairs, e.g.
inc(c); inc(c__c); dec(c__c); inc(a); dec(c). A focused regression test
pins the scenario.
---
 lib/store/path_ref_store.dart            |  16 +-
 test/core/store/path_ref_store_test.dart |  23 +++
 test/core/store/store_property_test.dart | 185 +++++++++++++++++++++++
 3 files changed, 219 insertions(+), 5 deletions(-)
 create mode 100644 test/core/store/store_property_test.dart

diff --git a/lib/store/path_ref_store.dart b/lib/store/path_ref_store.dart
index d03e9fe..ee9a60f 100644
--- a/lib/store/path_ref_store.dart
+++ b/lib/store/path_ref_store.dart
@@ -109,12 +109,18 @@ class PathRefStore {
         node[segment]--;
       }
     } else if (node[segment] is Map) {
-      // If this is the last reference to the child node, then remove the ref key from the child,
-      // marking that it is now purely a transient node.
-      if (node[segment][_refKey] == 1) {
-        return true;
+      final Map child = node[segment];
+      if (child[_refKey] == 1) {
+        // Releasing the last reference to this node. If it has no descendants,
+        // remove it entirely; otherwise it still routes to live descendants, so
+        // drop only its ref key and keep it as a transient node.
+        if (child.length == 1) {
+          node.remove(segment);
+        } else {
+          child.remove(_refKey);
+        }
       } else {
-        node[segment][_refKey]--;
+        child[_refKey]--;
       }
     }
 
diff --git a/test/core/store/path_ref_store_test.dart b/test/core/store/path_ref_store_test.dart
index 8835170..757d2f3 100644
--- a/test/core/store/path_ref_store_test.dart
+++ b/test/core/store/path_ref_store_test.dart
@@ -171,6 +171,29 @@ void main() {
         });
       });
 
+      test('Dec of a path that became transient removes it fully', () {
+        // Regression: inc'ing a path and a descendant, then dec'ing the
+        // descendant, leaves the path as a transient node. Dec'ing the path
+        // itself must remove it; previously the Map branch of `_dec` signalled
+        // removal via a return value that the top-level `dec` ignored, so the
+        // node lingered (has stayed true) and ref counts leaked. A second live
+        // path keeps the root ref count above 1 so the early return doesn't
+        // mask the bug.
+        final store = PathRefStore();
+        store.inc('c');
+        store.inc('c__c');
+        store.dec('c__c');
+        store.inc('a');
+
+        store.dec('c');
+
+        expect(store.has('c'), false);
+        expect(store.inspect(), {
+          "__ref": 1,
+          "a": 1,
+        });
+      });
+
       test('Dec of untracked deep path under a tracked node is a no-op', () {
         final store = PathRefStore();
         store.inc('a__b__c');
diff --git a/test/core/store/store_property_test.dart b/test/core/store/store_property_test.dart
new file mode 100644
index 0000000..2fd7d53
--- /dev/null
+++ b/test/core/store/store_property_test.dart
@@ -0,0 +1,185 @@
+import 'dart:math';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:loon/loon.dart';
+
+/// Model-based property tests for the path-keyed store structures.
+///
+/// Each test drives randomized operation sequences against the real store and
+/// a trivial reference model, then asserts the two agree (and that structural
+/// invariants like "empties out completely" hold). The reference results are
+/// recomputed from scratch each step, so they're an independent oracle for the
+/// stores' incremental bookkeeping. On failure the seed and operation log are
+/// printed so the case can be replayed.
+///
+/// A small segment/value alphabet is used deliberately to force path collisions
+/// and shared prefixes, which is where the tree-restructuring edge cases live.
+///
+/// Scope: covers `write` and recursive `delete`. Non-recursive `delete` has
+/// subtler semantics (it prunes a node's immediate values while keeping deeper
+/// descendants) and is left for a dedicated model.
+const _d = '__';
+const _alphabet = ['a', 'b', 'c'];
+
+String _randomPath(Random r) {
+  final depth = 1 + r.nextInt(3); // 1..3 segments
+  return List.generate(depth, (_) => _alphabet[r.nextInt(_alphabet.length)])
+      .join(_d);
+}
+
+/// All non-empty paths of depth 1 and 2 — a fixed grid of query points.
+final _grid = <String>[
+  for (final a in _alphabet) ...[
+    a,
+    for (final b in _alphabet) '$a$_d$b',
+  ],
+];
+
+/// Whether [key] is at or below [path] in the tree.
+bool _atOrUnder(String key, String path) =>
+    path.isEmpty || key == path || key.startsWith('$path$_d');
+
+String _parent(String path) {
+  final i = path.lastIndexOf(_d);
+  return i == -1 ? '' : path.substring(0, i);
+}
+
+String _lastSegment(String path) {
+  final i = path.lastIndexOf(_d);
+  return i == -1 ? path : path.substring(i + _d.length);
+}
+
+void main() {
+  group('PathRefStore property', () {
+    test('matches reference model across random inc/dec sequences', () {
+      for (var seed = 0; seed < 200; seed++) {
+        final r = Random(seed);
+        final store = PathRefStore();
+        final live = <String>[]; // inc'd paths, with multiplicity
+        final ops = <String>[];
+
+        for (var step = 0; step < 50; step++) {
+          // dec only targets paths that were actually inc'd, mirroring how the
+          // library pairs inc/dec; bias toward inc when nothing is live.
+          if (live.isEmpty || r.nextBool()) {
+            final p = _randomPath(r);
+            store.inc(p);
+            live.add(p);
+            ops.add('inc($p)');
+          } else {
+            final p = live.removeAt(r.nextInt(live.length));
+            store.dec(p);
+            ops.add('dec($p)');
+          }
+
+          final reason = 'seed=$seed ops=$ops';
+          for (final q in {..._grid, ...live}) {
+            // has(q): true iff some live path is q or a descendant of q.
+            final expected = live.any((p) => _atOrUnder(p, q));
+            expect(store.has(q), expected, reason: '$reason has($q)');
+          }
+          expect(store.isEmpty, live.isEmpty, reason: '$reason isEmpty');
+        }
+      }
+    });
+  });
+
+  group('ValueStore property', () {
+    test('matches reference model across random write/delete sequences', () {
+      for (var seed = 0; seed < 200; seed++) {
+        final r = Random(seed);
+        final store = ValueStore<int>();
+        final model = <String, int>{};
+        final ops = <String>[];
+        var counter = 0;
+
+        for (var step = 0; step < 50; step++) {
+          if (r.nextInt(3) != 0) {
+            final p = _randomPath(r);
+            final v = counter++;
+            store.write(p, v);
+            model[p] = v;
+            ops.add('write($p,$v)');
+          } else {
+            final p = _randomPath(r);
+            store.delete(p); // recursive
+            model.removeWhere((k, _) => _atOrUnder(k, p));
+            ops.add('delete($p)');
+          }
+
+          final reason = 'seed=$seed ops=$ops';
+          for (final q in {..._grid, ...model.keys}) {
+            expect(store.get(q), model[q], reason: '$reason get($q)');
+            expect(store.hasValue(q), model.containsKey(q),
+                reason: '$reason hasValue($q)');
+            final hasPath = model.containsKey(q) ||
+                model.keys.any((k) => k.startsWith('$q$_d'));
+            expect(store.hasPath(q), hasPath, reason: '$reason hasPath($q)');
+          }
+          for (final q in _grid) {
+            final expected = <String, int>{};
+            for (final entry in model.entries) {
+              if (_parent(entry.key) == q) {
+                expected[_lastSegment(entry.key)] = entry.value;
+              }
+            }
+            final actual = store.getChildValues(q) ?? const <String, int>{};
+            expect(actual, equals(expected),
+                reason: '$reason getChildValues($q)');
+          }
+          expect(store.isEmpty, model.isEmpty, reason: '$reason isEmpty');
+        }
+      }
+    });
+  });
+
+  group('ValueRefStore property', () {
+    test('ref aggregation matches the values in each subtree', () {
+      for (var seed = 0; seed < 200; seed++) {
+        final r = Random(seed);
+        final store = ValueRefStore<String>();
+        final model = <String, String>{};
+        final ops = <String>[];
+
+        // getRefs(path) aggregates values strictly under a node; for the root
+        // path it aggregates every value in the store.
+        Map<String, int> refsUnder(String path) {
+          final counts = <String, int>{};
+          for (final entry in model.entries) {
+            final under =
+                path.isEmpty ? true : entry.key.startsWith('$path$_d');
+            if (under) {
+              counts[entry.value] = (counts[entry.value] ?? 0) + 1;
+            }
+          }
+          return counts;
+        }
+
+        for (var step = 0; step < 50; step++) {
+          if (r.nextInt(3) != 0) {
+            final p = _randomPath(r);
+            // Small value space → many shared refs to stress the aggregation.
+            final v = _alphabet[r.nextInt(_alphabet.length)];
+            store.write(p, v);
+            model[p] = v;
+            ops.add('write($p,$v)');
+          } else {
+            final p = _randomPath(r);
+            store.delete(p); // recursive
+            model.removeWhere((k, _) => _atOrUnder(k, p));
+            ops.add('delete($p)');
+          }
+
+          final reason = 'seed=$seed ops=$ops';
+          for (final q in ['', ..._grid]) {
+            final expected = refsUnder(q);
+            final actual = store.getRefs(q) ?? const <String, int>{};
+            expect(Map<String, int>.from(actual), equals(expected),
+                reason: '$reason getRefs("$q")');
+            expect(store.extractValues(q), equals(expected.keys.toSet()),
+                reason: '$reason extractValues("$q")');
+          }
+        }
+      }
+    });
+  });
+}

From 04898586ed06037f05d8c735ed90a83295740138 Mon Sep 17 00:00:00 2001
From: Dan Reynolds <me@danreynolds.ca>
Date: Sun, 31 May 2026 09:00:46 -0400
Subject: [PATCH 2/4] Fix PathRefStore direct decrement guard

---
 lib/store/path_ref_store.dart            | 70 ++++++++++++++++++------
 test/core/store/path_ref_store_test.dart | 21 +++++++
 2 files changed, 75 insertions(+), 16 deletions(-)

diff --git a/lib/store/path_ref_store.dart b/lib/store/path_ref_store.dart
index ee9a60f..d674a5b 100644
--- a/lib/store/path_ref_store.dart
+++ b/lib/store/path_ref_store.dart
@@ -74,6 +74,51 @@ class PathRefStore {
     _inc(_store, path.split(delimiter));
   }
 
+  int _refCount(Object? value) {
+    if (value is int) {
+      return value;
+    }
+    if (value is Map) {
+      return value[_refKey] as int? ?? 0;
+    }
+    return 0;
+  }
+
+  int _directRefCount(Map node) {
+    final refCount = node[_refKey] as int? ?? 0;
+    final descendantRefCount = node.entries
+        .where((entry) => entry.key != _refKey)
+        .fold<int>(0, (sum, entry) => sum + _refCount(entry.value));
+
+    return refCount - descendantRefCount;
+  }
+
+  bool _hasDirectRef(Map? node, List<String> segments, [int index = 0]) {
+    final segment = segments[index];
+    if (node == null) {
+      return false;
+    }
+
+    if (index < segments.length - 1) {
+      final child = node[segment];
+      if (child is! Map) {
+        return false;
+      }
+
+      return _hasDirectRef(child, segments, index + 1);
+    }
+
+    final child = node[segment];
+    if (child is int) {
+      return child > 0;
+    }
+    if (child is Map) {
+      return _directRefCount(child) > 0;
+    }
+
+    return false;
+  }
+
   bool _dec(
     Map? node,
     List<String> segments, [
@@ -111,14 +156,7 @@ class PathRefStore {
     } else if (node[segment] is Map) {
       final Map child = node[segment];
       if (child[_refKey] == 1) {
-        // Releasing the last reference to this node. If it has no descendants,
-        // remove it entirely; otherwise it still routes to live descendants, so
-        // drop only its ref key and keep it as a transient node.
-        if (child.length == 1) {
-          node.remove(segment);
-        } else {
-          child.remove(_refKey);
-        }
+        node.remove(segment);
       } else {
         child[_refKey]--;
       }
@@ -131,13 +169,14 @@ class PathRefStore {
 
   /// Decrements the ref count to the node at the given path, removing it if it was the last reference to the node.
   void dec(String path) {
-    // `_dec` assumes every node it walks carries a `_refKey`; without this
-    // guard an untracked path would still decrement (and potentially clear)
-    // ancestors that share a prefix.
-    if (!has(path)) {
+    final segments = path.split(delimiter);
+    // `_dec` assumes the target path has a direct ref. `has` is broader: it is
+    // also true when only a descendant is live, which must not decrement this
+    // path.
+    if (!_hasDirectRef(_store, segments)) {
       return;
     }
-    _dec(_store, path.split(delimiter));
+    _dec(_store, segments);
   }
 
   bool _has(Map? node, List<String> segments, [int index = 0]) {
@@ -158,15 +197,14 @@ class PathRefStore {
     return node.containsKey(segment);
   }
 
-  /// Returns whether the path exists in the store.
+  /// Returns whether the path or any descendant path exists in the store.
   ///
   /// Example:
   /// ```dart
   /// final refStore = PathRefStore();
   /// refStore.inc('users__1__posts__2');
   /// refStore.has('users__1__posts__2') // true
-  /// refStore.has('users__1') // false
-  /// refStore.hasPath('users__1') // true
+  /// refStore.has('users__1') // true
   ///
   /// ```
   bool has(String path) {
diff --git a/test/core/store/path_ref_store_test.dart b/test/core/store/path_ref_store_test.dart
index 757d2f3..0803f4c 100644
--- a/test/core/store/path_ref_store_test.dart
+++ b/test/core/store/path_ref_store_test.dart
@@ -209,6 +209,27 @@ void main() {
           },
         });
       });
+
+      test('Dec of untracked ancestor path leaves descendants intact', () {
+        final store = PathRefStore();
+        store.inc('a__b');
+        store.inc('x');
+
+        store.dec('a');
+
+        expect(store.has('a__b'), true);
+        expect(store.inspect(), {
+          "__ref": 2,
+          "a": {"__ref": 1, "b": 1},
+          "x": 1,
+        });
+
+        expect(() => store.dec('a__b'), returnsNormally);
+        expect(store.inspect(), {
+          "__ref": 1,
+          "x": 1,
+        });
+      });
     });
   });
 }

From 1f19672ff6bfbb441b023f133d44dad221cda6d8 Mon Sep 17 00:00:00 2001
From: Dan Reynolds <me@danreynolds.ca>
Date: Sun, 31 May 2026 09:08:19 -0400
Subject: [PATCH 3/4] Simplify PathRefStore decrement fix

---
 lib/store/path_ref_store.dart            | 53 +-----------------------
 test/core/store/path_ref_store_test.dart | 21 ----------
 2 files changed, 2 insertions(+), 72 deletions(-)

diff --git a/lib/store/path_ref_store.dart b/lib/store/path_ref_store.dart
index d674a5b..32aaa6a 100644
--- a/lib/store/path_ref_store.dart
+++ b/lib/store/path_ref_store.dart
@@ -74,51 +74,6 @@ class PathRefStore {
     _inc(_store, path.split(delimiter));
   }
 
-  int _refCount(Object? value) {
-    if (value is int) {
-      return value;
-    }
-    if (value is Map) {
-      return value[_refKey] as int? ?? 0;
-    }
-    return 0;
-  }
-
-  int _directRefCount(Map node) {
-    final refCount = node[_refKey] as int? ?? 0;
-    final descendantRefCount = node.entries
-        .where((entry) => entry.key != _refKey)
-        .fold<int>(0, (sum, entry) => sum + _refCount(entry.value));
-
-    return refCount - descendantRefCount;
-  }
-
-  bool _hasDirectRef(Map? node, List<String> segments, [int index = 0]) {
-    final segment = segments[index];
-    if (node == null) {
-      return false;
-    }
-
-    if (index < segments.length - 1) {
-      final child = node[segment];
-      if (child is! Map) {
-        return false;
-      }
-
-      return _hasDirectRef(child, segments, index + 1);
-    }
-
-    final child = node[segment];
-    if (child is int) {
-      return child > 0;
-    }
-    if (child is Map) {
-      return _directRefCount(child) > 0;
-    }
-
-    return false;
-  }
-
   bool _dec(
     Map? node,
     List<String> segments, [
@@ -169,14 +124,10 @@ class PathRefStore {
 
   /// Decrements the ref count to the node at the given path, removing it if it was the last reference to the node.
   void dec(String path) {
-    final segments = path.split(delimiter);
-    // `_dec` assumes the target path has a direct ref. `has` is broader: it is
-    // also true when only a descendant is live, which must not decrement this
-    // path.
-    if (!_hasDirectRef(_store, segments)) {
+    if (!has(path)) {
       return;
     }
-    _dec(_store, segments);
+    _dec(_store, path.split(delimiter));
   }
 
   bool _has(Map? node, List<String> segments, [int index = 0]) {
diff --git a/test/core/store/path_ref_store_test.dart b/test/core/store/path_ref_store_test.dart
index 0803f4c..757d2f3 100644
--- a/test/core/store/path_ref_store_test.dart
+++ b/test/core/store/path_ref_store_test.dart
@@ -209,27 +209,6 @@ void main() {
           },
         });
       });
-
-      test('Dec of untracked ancestor path leaves descendants intact', () {
-        final store = PathRefStore();
-        store.inc('a__b');
-        store.inc('x');
-
-        store.dec('a');
-
-        expect(store.has('a__b'), true);
-        expect(store.inspect(), {
-          "__ref": 2,
-          "a": {"__ref": 1, "b": 1},
-          "x": 1,
-        });
-
-        expect(() => store.dec('a__b'), returnsNormally);
-        expect(store.inspect(), {
-          "__ref": 1,
-          "x": 1,
-        });
-      });
     });
   });
 }

From 5a82dff5210fc6b20b1b39d8b3d7c77cc66afdcc Mon Sep 17 00:00:00 2001
From: Dan Reynolds <me@danreynolds.ca>
Date: Sun, 31 May 2026 09:17:04 -0400
Subject: [PATCH 4/4] Clarify PathRefStore branch coverage test

---
 test/core/store/path_ref_store_test.dart | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/test/core/store/path_ref_store_test.dart b/test/core/store/path_ref_store_test.dart
index 757d2f3..1d598be 100644
--- a/test/core/store/path_ref_store_test.dart
+++ b/test/core/store/path_ref_store_test.dart
@@ -171,14 +171,9 @@ void main() {
         });
       });
 
-      test('Dec of a path that became transient removes it fully', () {
-        // Regression: inc'ing a path and a descendant, then dec'ing the
-        // descendant, leaves the path as a transient node. Dec'ing the path
-        // itself must remove it; previously the Map branch of `_dec` signalled
-        // removal via a return value that the top-level `dec` ignored, so the
-        // node lingered (has stayed true) and ref counts leaked. A second live
-        // path keeps the root ref count above 1 so the early return doesn't
-        // mask the bug.
+      test('Dec of a map-backed path removes the empty node', () {
+        // The extra live sibling keeps the root ref count above 1 so this
+        // exercises the child-map branch rather than the root clear path.
         final store = PathRefStore();
         store.inc('c');
         store.inc('c__c');