Skip to content

Add wrapped_self() — Phase 1 fix for #18 (self is unwrapped in delegation-wrapped stores)#73

Merged
thorwhalen merged 2 commits into
masterfrom
fix/wrap-kvs-wrapped-self
Jul 5, 2026
Merged

Add wrapped_self() — Phase 1 fix for #18 (self is unwrapped in delegation-wrapped stores)#73
thorwhalen merged 2 commits into
masterfrom
fix/wrap-kvs-wrapped-self

Conversation

@thorwhalen

Copy link
Copy Markdown
Member

What

Adds wrapped_self() — the Phase-1, backward-compatible fix for #18. dol wraps by delegation (has-a), so a method defined on a wrap_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.

from dol import wrap_kvs, wrapped_self

@wrap_kvs(data_of_obj=lambda x: x*x, obj_of_data=lambda x: math.sqrt(x))
class S(dict):
    def via_self(self, k):          return self[k]                  # #18: returns raw (4)
    def via_wrapped_self(self, k):  return wrapped_self(self)[k]    # transformed (2.0)

How

  • id-keyed weakref registry populated once 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.
  • Does not touch DelegatedAttribute.__get__, the delegate_to copy loop, or the base.py:451 signature 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). Rebinding self to the wrapper was rejected: under Pipe/stacked wraps it binds to the innermost layer (silent write corruption) and crashes on super()/dict-native ops. wrapped_self avoids 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

  • Zero default behavior change (additive; registry writes only).
  • Dependents test-gate (top 15): 0 PASS→FAIL. The 3 non-passing dependents (hubcap, msword, raglab-bak) fail identically at baseline (pre-existing collection/import errors, unrelated).
  • Full dol suite green; 7 regression tests (single-layer, nested climb, instance-wrap, pickle round-trip, direct-subclass no-op, plain-object no-op, zero-behavior-change) + a runnable doctest.

Refs #18

https://claude.ai/code/session_01H8PmV6xmMhzV64zi1Twraw

…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
…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 thorwhalen merged commit 6e8fac4 into master Jul 5, 2026
12 checks passed
@thorwhalen thorwhalen deleted the fix/wrap-kvs-wrapped-self branch July 5, 2026 07:33
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Store.wrap breaks the signature of subclasses.

1 participant