Add wrapped_self() — Phase 1 fix for #18 (self is unwrapped in delegation-wrapped stores)#73
Merged
Merged
Conversation
…wrapped stores) dol wraps by delegation (has-a): a method defined on a wrap_kvs-decorated class runs bound to the inner, unwrapped store, so `self[k]` inside it bypasses the transform pipeline (#18). This adds `wrapped_self(self)` — an additive, opt-in accessor that recovers the outer, transform-applying store, so authors write `wrapped_self(self)[k]`. - id-keyed weakref registry populated in Store.__init__ (covers class- AND instance-wraps) and re-populated in Store.__setstate__ (unpickle bypasses __init__); guarded compare-and-pop finalizer. - climbs to the OUTERMOST wrapper for stacked/Pipe wraps; safe no-op on direct Store subclasses and plain objects. - Zero default behavior change: dependents test-gate (top 15) stays green, 0 PASS->FAIL. 7 regression tests + a runnable doctest. Design rationale and the deferred structural fix (is-a wrapping, which would also close #6) are in misc/docs/dol_issue18_design.md. Refs #18 Claude-Session: https://claude.ai/code/session_01H8PmV6xmMhzV64zi1Twraw
This was referenced Jul 5, 2026
…dentity guard) Adversarial review found a real defect: copy.copy of a wrapped store shares the inner; the copy re-registered id(inner) and, when GC'd, its finalizer popped the entry — silently reverting the still-alive original to raw #18 behavior. Root cause: single-slot dict[id(inner) -> weakref] can't represent one-inner:N-wrappers. Fixes (all contained to base.py): - Registry is now dict[id -> list[weakref]]; register appends (idempotent per live wrapper), the finalizer removes only its OWN ref and drops the id when empty. GC of a transient copy can no longer evict a live sibling. - wrapped_self climb adds an identity guard: follow a back-ref only if the wrapper still holds `cur` as its .store — rejects stale entries from post-init .store reassignment (and id-reuse false positives), and disambiguates shared inners. - Documented the residual one-inner:N-live-wrappers-with-different-transforms ambiguity (returns a valid wrapper, never raw; doesn't arise for class-wraps or copy.copy). Renamed an incidental `wrapped_self` local in DelegatedAttribute. 4 new regression tests (copy.copy+GC, deepcopy, no-leak over 1000 wrappers, shared-inner). Full dol suite green (122 passed). Claude-Session: https://claude.ai/code/session_01H8PmV6xmMhzV64zi1Twraw
thorwhalen
added a commit
that referenced
this pull request
Jul 5, 2026
…r) (#74) Adds misc/docs/dol_issue18_design.md — a decision-ready design for #18 (self is the unwrapped inner store inside a delegation-wrapped class's own methods): mechanism, a 26-site ecosystem blast-radius classification (0 break, 6 latent bugs), four fix designs with adversarial judging, an empirical verification log, and the staged recommendation. Also wires it into the misc/docs index and updates the issues report's Wave-1 status (#18 Phase 1 shipped; #6 deferred to the is-a restructure). Companion to the code change (wrapped_self) in PR #73. Refs #18 Claude-Session: https://claude.ai/code/session_01H8PmV6xmMhzV64zi1Twraw
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds
wrapped_self()— the Phase-1, backward-compatible fix for #18. dol wraps by delegation (has-a), so a method defined on awrap_kvs-decorated class runs bound to the inner, unwrapped store;self[k]inside it bypasses the transform pipeline.wrapped_self(self)[k]recovers the outer, transform-applying store.How
id-keyed weakref registry populated once inStore.__init__(covers class- and instance-wraps) and re-populated inStore.__setstate__(unpickle bypasses__init__); guarded compare-and-pop finalizer.Pipewraps; safe no-op on directStoresubclasses and plain objects.DelegatedAttribute.__get__, thedelegate_tocopy loop, or thebase.py:451signature graft.Why this design (not method-rebinding)
A full analysis of 4 approaches (+ adversarial review + empirical verification) is in
misc/docs/dol_issue18_design.md(docs PR). Rebindingselfto the wrapper was rejected: underPipe/stacked wraps it binds to the innermost layer (silent write corruption) and crashes onsuper()/dict-native ops.wrapped_selfavoids all of that and uniquely covers the instance-wrap path. The structural fix (is-a wrapping, which would also close #6) is deferred to a major version.Safety
hubcap,msword,raglab-bak) fail identically at baseline (pre-existing collection/import errors, unrelated).Refs #18
https://claude.ai/code/session_01H8PmV6xmMhzV64zi1Twraw