Severity: ⚪ low • Category: api-design
Location: /Users/tolo/Projects/warp_cache/warp_cache/_decorator.py : 184, 186
What's wrong
Neither AsyncCachedFunction nor the Rust CachedFunction/SharedCachedFunction implements get (verified: no get in src/ or warp_cache/). When @cache() decorates an instance method, the resulting cache object is a non-descriptor class attribute, so instance.method(arg) returns the cache object unbound and calls it without binding self, causing the first positional argument to be misaligned (self omitted). Unlike functools.lru_cache, whose wrapper is a plain function and therefore a descriptor that binds self, these objects cannot be used as methods. This is undocumented and untested (no test/example decorates a method).
Trigger
class A:\n @cache()\n def m(self, x): return x # A().m(5) does not bind self correctly.
Suggested fix
Implement get on the Python wrapper (and/or wrap sync results in a Python object) using functools.partial/MethodType binding, or document that methods are unsupported and recommend caching a module-level function. At minimum add a test pinning the intended behavior.
Adversarial verification note
Verified the mechanism in real code. In _decorator.py, cache()'s decorator returns either the Rust pyclass instance inner (CachedFunction/SharedCachedFunction, line 186) or an AsyncCachedFunction (line 184). I confirmed via grep that get is not implemented anywhere in src/ or warp_cache/ (grep returns no matches, exit 1). AsyncCachedFunction (lines 49-115) defines only call, cache_info, cache_clear, repr — no get. Therefore when @cache() decorates an instance method, the resulting object is a non-descriptor class attribute: instance.method(arg) retrieves the cache object without binding self, so call is invoked with only the explicit positional args. Unlike functools.lru_cache (a plain-function wrapper that IS a descriptor), this object cannot bind self. This is a genuine, reachable API limitation. No test/example decorates an actual instance method (the @cache uses in tests wrap nested plain functions, e.g. tests/test_basic.py:7-11), so it is untested and undocumented as claimed.\n\nTwo corrections to the reviewer's framing that lower severity: (1) For a typical def m(self, x), the missed self does NOT silently misalign into wrong results — calling cache_obj(5) maps 5 to the self parameter (since fn is stored and invoked positionally inside Rust) leaving x unfilled, producing a TypeError on first call, not silent data corruption. The 'misaligned/self omitted causing wrong answers' phrasing overstates this; it's more likely a loud failure. (2) It only matters if a user attempts an unsupported pattern; documented usage (module-level/nested functions) is unaffected. This makes it an api-design/documentation gap rather than a correctness bug under realistic supported use, so I downgrade medium -> low.
Verifier correction: The bug is real (no get, methods can't bind self), but the claimed effect of "first positional argument misaligned / silent wrong results" is inaccurate: for a normal def m(self, x) the lookup arg lands on self and x is left unbound, raising TypeError on the first call rather than silently returning wrong results. It is a loud failure / API limitation, not silent corruption. Severity is low (api-design/doc gap affecting only an unsupported usage pattern), not medium.
Filed from a multi-agent code review (finder → adversarial verification → synthesis). Confirmed real after a skeptic re-read the code.
Severity: ⚪ low • Category: api-design
Location:
/Users/tolo/Projects/warp_cache/warp_cache/_decorator.py: 184, 186What's wrong
Neither AsyncCachedFunction nor the Rust CachedFunction/SharedCachedFunction implements get (verified: no get in src/ or warp_cache/). When @cache() decorates an instance method, the resulting cache object is a non-descriptor class attribute, so
instance.method(arg)returns the cache object unbound and calls it without bindingself, causing the first positional argument to be misaligned (self omitted). Unlike functools.lru_cache, whose wrapper is a plain function and therefore a descriptor that binds self, these objects cannot be used as methods. This is undocumented and untested (no test/example decorates a method).Trigger
class A:\n @cache()\n def m(self, x): return x # A().m(5) does not bind self correctly.
Suggested fix
Implement get on the Python wrapper (and/or wrap sync results in a Python object) using functools.partial/MethodType binding, or document that methods are unsupported and recommend caching a module-level function. At minimum add a test pinning the intended behavior.
Adversarial verification note
Verified the mechanism in real code. In _decorator.py, cache()'s decorator returns either the Rust pyclass instance
inner(CachedFunction/SharedCachedFunction, line 186) or an AsyncCachedFunction (line 184). I confirmed via grep that get is not implemented anywhere in src/ or warp_cache/ (grep returns no matches, exit 1). AsyncCachedFunction (lines 49-115) defines only call, cache_info, cache_clear, repr — no get. Therefore when @cache() decorates an instance method, the resulting object is a non-descriptor class attribute:instance.method(arg)retrieves the cache object without bindingself, so call is invoked with only the explicit positional args. Unlike functools.lru_cache (a plain-function wrapper that IS a descriptor), this object cannot bind self. This is a genuine, reachable API limitation. No test/example decorates an actual instance method (the @cache uses in tests wrap nested plain functions, e.g. tests/test_basic.py:7-11), so it is untested and undocumented as claimed.\n\nTwo corrections to the reviewer's framing that lower severity: (1) For a typicaldef m(self, x), the missed self does NOT silently misalign into wrong results — calling cache_obj(5) maps 5 to theselfparameter (since fn is stored and invoked positionally inside Rust) leavingxunfilled, producing a TypeError on first call, not silent data corruption. The 'misaligned/self omitted causing wrong answers' phrasing overstates this; it's more likely a loud failure. (2) It only matters if a user attempts an unsupported pattern; documented usage (module-level/nested functions) is unaffected. This makes it an api-design/documentation gap rather than a correctness bug under realistic supported use, so I downgrade medium -> low.Filed from a multi-agent code review (finder → adversarial verification → synthesis). Confirmed real after a skeptic re-read the code.