refactor(storage): declare record children through one ChildItems interface

[pmrv]

A minimal take on your suggestion in #797 (comment):

Believe it or not the solution to the gc() vs Path problem is more abstract indirection! Behind both path and destructuring mixin we add another interface that recurses into selected items and route all item access through that. path and destructuring then implement that for their specific types.

Based on temppath and targeting it, so #797's diff stays reviewable on its own.

The shape

storage.base.ChildItems — one interface, below both mixins:

class ChildItems(ABC):
    @abstractmethod
    def child_items(self) -> Iterable[tuple[Any, Any]]:
        """``(label, child)`` for every child slot this record holds."""

    def child_digests(self) -> set[Digest]:
        return {child for _, child in self.child_items() if isinstance(child, Digest)}

Implemented for their specific types by the two layers' record classes:

record	child_items()
DigestedIterable	[(None, v) for v in self.items]
DigestedMapping	keys then values, flat (a key can be a Digest too)
DigestedFields	list(self.fields.items())
FileBlob	[(self.name, self.content)]
DirectoryBlob	list(self.contents.items())

ValueStorage._raw_sub_digests becomes one generic reader over it — raw.child_digests() if isinstance(raw, ChildItems) else set() — and both per-mixin overrides go away: the 22-line match in DestructuringMixin and the blob branches in PathValueMixin. Directory materialization walks blob.child_items(), the same list the sweep walks. _raw_sub_digests stays overridable, for a storage whose records are of a type it cannot make implement the interface.

What this fixes, and what it only tidies

The path blobs are already fixed. Here this is locality, not a fix.

28df4cd closed finding 1 with two changes, and it is worth being exact about them because the first is easy to misremember as a missing override. On main, _raw_sub_digests is a private helper of DestructuringMixin — there is no ValueStorage._raw_sub_digests and no load_raw. That commit is what lifted the method onto ValueStorage, made it a cooperative chain, and added the path override. There was no unfilled slot; the fix built the slot and filled it.

The second cause is the one an override alone would not have reached: the walk read through load, which mends, and mending resolves the child references away — a materialized Path no longer knows which blob it came from. Notably that read (super().load) had been effectively raw on main, landing on ValueMixin.load; inserting PathValueMixin below DestructuringMixin in the MRO is what turned the same call into a mending one. Hence load_raw.

So for the blobs alone, ChildItems moves the declaration onto FileBlob / DirectoryBlob, next to the fields it describes. That is cleanup. It is not what makes this worth merging.

The Digested side is an open correctness bug: #883.

The match in DestructuringMixin._raw_sub_digests enumerates the three built-in wrapper types by name. register_destructurer's extension point is a class, not a mixin — so a user's wrapper is named by no override anywhere and reports zero children whatever it holds. gc reclaims its sub-values as orphans and the next load raises KeyError, which the wrapper reports as an ordinary cache miss. Silent data loss on routine maintenance, and no amount of overriding on any mixin can reach it, because there is no mixin involved in registering the type.

Your tracker entry proposes exactly this hook (child_digests() on the classes). The only addition here is that the path blobs implement the same one rather than a parallel mechanism.

test_gc_follows_a_custom_destructurer_s_wrapper pins it and is differential — run against temppath's src/ to be sure it fails for the right reason:

assert len(cache.values.child_digests(result_key)) == 2
E   AssertionError: assert 0 == 2

That is the "differential gc test with a custom destructurer" #883 asks for; nothing exercised that combination before.

Summary: one open correctness bug closed, plus a consolidation that removes two hand-written overrides. If you would rather take #883 on its own later, temppath needs nothing from this PR.

A smaller alternative, for the record

#883 can also be closed without a new interface: keep DestructuringMixin._raw_sub_digests, but make it generic over Digested.underlying() — shallow-scan that for Digests instead of matching three type names. underlying() is already abstract, so every subclass has one. Smaller diff, no contract change, and PathValueMixin keeps its blob arm.

I did not take it because underlying()'s documented contract is digest-equivalence, not child enumeration — the two coincide today rather than by design, and a shallow scan needs its own rule for a wrapper holding a plain container of Digests one level down. It also leaves the path side declared away from the type. Both are judgement calls; say the word if you prefer the smaller one.

What I deliberately did not do

• mend still reads its own attributes rather than routing through child_items. I tried it. The two read the same attribute, so there is no drift to eliminate, and the tuple-per-element the indirection costs measured ~70% slower on a 10k-element mend (555 µs → 943 µs) — a real cost on the load path for no correctness gain. child_items is the declaration for readers that walk the record; reconstruction stays each type's own business.
• The save side is untouched. _slots / sunder / PathValueMixin.save stay as they are. Unifying admission too would mean reconciling destructuring's inline-vs-store depth logic with "a path always writes out", which is a much bigger change than the gc problem needs.
• No find_path changes. It mirrors digest, deliberately, not this.

Contract change, worth a look

child_items is abstract, so an out-of-tree Digested subclass must now implement it — it breaks loudly at instantiation instead of silently reporting no children. I think loud is right for a hook whose failure mode is data loss, and there are no such subclasses in-repo, but it is a judgement call and easy to reverse: a concrete default of type(self)._slots(self.underlying()) covers every shape-preserving wrapper, at the cost of going quiet again for one that isn't. docs/dev/extending_destructurer.rst documents the contract either way.

Verification

• pytest tests/ — 1917 passed, 11 skipped (the +1 over temppath is the new test)
• ty check src/ — clean, on the pinned ty == 0.0.69
• CI green on all of build 3.11–3.14, sql-backends, ty, triage
• Benchmarks — 0 significant changes (|Δ| > 10%) across 240 rows, which is the independent check on the mend decision above
• Sphinx build — clean; the only warnings are from notebooks I had to exclude because this container has no pandoc, and none are from the new references
• Docs updated where they described the old per-mixin hook: docs/dev/path_storage.rst ("Blobs must declare their references"), docs/dev/extending_destructurer.rst (the subclass contract), and the destructuring.py module-map line in agents/DEVELOPING.md

Left alone for you: #883's tracker entry, and the four questions still open on #797 — none of them are blocked by this.

---

Body revised after checking the branch history: the original claimed the path bug was a layer describing its records "once per reader", which reads as if an override slot sat unfilled. It didn't — 28df4cd created the slot. The case for this PR is #883, not a recurrence of finding 1.

[github-actions]

Benchmark Results

Baseline: bb3fe62 (recomputed) · HEAD~: cfe0ed9

Delta

Significant changes (|Δ| > 10%): 0 — 240 other rows hidden

no significant changes

Full results

Call Storage

calls

configuration	contains_hit	contains_miss	evict	load	save
🔴 SqlFile	🟩 250 µs	🟩 240 µs	🟩 450 µs	🟩 720 µs	🟥 2.1 ms
🔴 SqlMemory	🟩 240 µs	🟩 230 µs	🟩 280 µs	🟩 700 µs	🟧 1.9 ms

Digest

workload	digest
Dict (small)	🟩 1.5 ms
Float	🟩 260 µs
Integer	🟩 130 µs
List (integers, len<100)	🟩 940 µs
List (integers, len>100)	🟥 27 ms
Nested (Random Hypothesis)	🟩 3 ms
None	🟩 120 µs
Numpy (integers, len<100)	🟩 910 µs
Numpy (integers, len>100)	🟩 2.6 ms
String (len<100)	🟩 140 µs
String (len>100)	🟩 220 µs

Integration

compute_heavy

configuration	contains_hit	contains_miss	hit	miss
🟠 H5+Sql	🟩 370 µs	🟩 360 µs	🟨 3 ms	🟥 7.3 ms
🟢 Pickle+Sql	🟩 380 µs	🟩 360 µs	🟩 1.1 ms	🟨 3.7 ms
🟣 Memory	🟩 77 µs	🟩 77 µs	🟩 160 µs	🟩 1.2 ms
🟣 Memory(Raw)	🟩 70 µs	🟩 71 µs	🟩 150 µs	🟩 1.1 ms
🟣 SizeLimitedCache(Memory,max=10)	🟩 79 µs	🟩 79 µs	🟩 190 µs	🟩 1.2 ms
🟣 SizeLimitedCache(Memory,max=100)	🟩 78 µs	🟩 81 µs	🟩 160 µs	🟩 1.2 ms
🟤 Memory+Sqlite(:memory:)	🟩 360 µs	🟩 350 µs	🟩 940 µs	🟨 2.7 ms

data_heavy

configuration	contains_hit	contains_miss	hit	miss
🟠 H5+Sql	🟩 380 µs	🟩 360 µs	🟨 3.1 ms	🟥 7.7 ms
🟢 Pickle+Sql	🟩 370 µs	🟩 370 µs	🟩 1.1 ms	🟨 3.1 ms
🟣 Memory	🟩 77 µs	🟩 78 µs	🟩 170 µs	🟩 510 µs
🟣 Memory(Raw)	🟩 71 µs	🟩 71 µs	🟩 160 µs	🟩 460 µs
🟣 SizeLimitedCache(Memory,max=10)	🟩 78 µs	🟩 79 µs	🟩 190 µs	🟩 520 µs
🟣 SizeLimitedCache(Memory,max=100)	🟩 78 µs	🟩 78 µs	🟩 170 µs	🟩 510 µs
🟤 Memory+Sqlite(:memory:)	🟩 360 µs	🟩 350 µs	🟩 960 µs	🟩 2 ms

lightweight

configuration	contains_hit	contains_miss	hit	miss
🟠 H5+Sql	🟩 380 µs	🟩 370 µs	🟨 2.9 ms	🟥 6.7 ms
🟢 Pickle+Sql	🟩 380 µs	🟩 370 µs	🟩 1.1 ms	🟨 2.8 ms
🟣 Memory	🟩 77 µs	🟩 78 µs	🟩 160 µs	🟩 370 µs
🟣 Memory(Raw)	🟩 70 µs	🟩 72 µs	🟩 150 µs	🟩 320 µs
🟣 SizeLimitedCache(Memory,max=10)	🟩 79 µs	🟩 79 µs	🟩 170 µs	🟩 390 µs
🟣 SizeLimitedCache(Memory,max=100)	🟩 78 µs	🟩 79 µs	🟩 160 µs	🟩 380 µs
🟤 Memory+Sqlite(:memory:)	🟩 360 µs	🟩 360 µs	🟩 930 µs	🟩 1.8 ms

Value Storage

nested_structures

configuration	contains_hit	contains_miss	evict	load	save
🔵 CloudpickleFile	🟩 30 µs	🟩 30 µs	🟩 33 µs	🟩 54 µs	🟩 190 µs
🔵 CloudpickleFile_Signed	🟩 30 µs	🟩 30 µs	🟩 33 µs	🟩 63 µs	🟩 200 µs
🟠 BagOfHoldingH5File	🟩 280 µs	🟩 31 µs	🟩 260 µs	🟧 1.1 ms	🟥 1.5 ms
🟡 DillFile	🟩 30 µs	🟩 30 µs	🟩 34 µs	🟩 61 µs	🟩 210 µs
🟡 DillFile_Signed	🟩 30 µs	🟩 30 µs	🟩 34 µs	🟩 71 µs	🟩 220 µs
🟢 PickleFile	🟩 30 µs	🟩 30 µs	🟩 33 µs	🟩 55 µs	🟩 180 µs
🟢 PickleFile_Signed	🟩 30 µs	🟩 30 µs	🟩 33 µs	🟩 63 µs	🟩 200 µs
🟣 Memory	🟩 6.8 µs	🟩 6.8 µs	🟩 7.1 µs	🟩 8.6 µs	🟩 9.2 µs
🟣 Memory(Raw)	🟩 1.8 µs	🟩 1.8 µs	🟩 2 µs	🟩 3.2 µs	🟩 3.7 µs

numpy_arrays

configuration	contains_hit	contains_miss	evict	load	save
🔵 CloudpickleFile	🟩 30 µs	🟩 30 µs	🟩 71 µs	🟩 79 µs	🟩 300 µs
🔵 CloudpickleFile_Signed	🟩 30 µs	🟩 30 µs	🟩 72 µs	🟩 140 µs	🟩 370 µs
🟠 BagOfHoldingH5File	🟩 270 µs	🟩 31 µs	🟩 640 µs	🟧 1.8 ms	🟥 2.2 ms
🟡 DillFile	🟩 30 µs	🟩 30 µs	🟩 70 µs	🟩 92 µs	🟩 450 µs
🟡 DillFile_Signed	🟩 30 µs	🟩 30 µs	🟩 71 µs	🟩 160 µs	🟩 510 µs
🟢 PickleFile	🟩 30 µs	🟩 30 µs	🟩 71 µs	🟩 79 µs	🟩 260 µs
🟢 PickleFile_Signed	🟩 30 µs	🟩 30 µs	🟩 71 µs	🟩 150 µs	🟩 340 µs
🟣 Memory	🟩 6.7 µs	🟩 6.6 µs	🟩 7.1 µs	🟩 13 µs	🟩 14 µs
🟣 Memory(Raw)	🟩 1.8 µs	🟩 1.8 µs	🟩 2 µs	🟩 7.2 µs	🟩 7.8 µs

small_strings

configuration	contains_hit	contains_miss	evict	load	save
🔵 CloudpickleFile	🟩 30 µs	🟩 30 µs	🟩 63 µs	🟩 54 µs	🟩 190 µs
🔵 CloudpickleFile_Signed	🟩 29 µs	🟩 30 µs	🟩 63 µs	🟩 63 µs	🟩 210 µs
🟠 BagOfHoldingH5File	🟩 280 µs	🟩 32 µs	🟨 620 µs	🟥 1.7 ms	🟥 1.7 ms
🟡 DillFile	🟩 30 µs	🟩 30 µs	🟩 63 µs	🟩 61 µs	🟩 210 µs
🟡 DillFile_Signed	🟩 29 µs	🟩 30 µs	🟩 62 µs	🟩 70 µs	🟩 240 µs
🟢 PickleFile	🟩 30 µs	🟩 30 µs	🟩 64 µs	🟩 55 µs	🟩 200 µs
🟢 PickleFile_Signed	🟩 30 µs	🟩 30 µs	🟩 63 µs	🟩 62 µs	🟩 200 µs
🟣 Memory	🟩 6.7 µs	🟩 6.6 µs	🟩 6.7 µs	🟩 8.4 µs	🟩 9 µs
🟣 Memory(Raw)	🟩 1.8 µs	🟩 1.8 µs	🟩 1.9 µs	🟩 3.1 µs	🟩 3.6 µs