feat: add @cache.local() for in-process reference caching#96
Conversation
Cache opaque Python objects (SDK clients, connections, ML models) that can't be serialized. Stores references directly in memory with entry-count LRU eviction, TTL, thread safety, and async support. - ObjectCache: standalone thread-safe cache (OrderedDict + RLock) - create_local_wrapper: decorator bridge with sync/async detection - Intent short-circuit in decorator() before backend/config resolution - Same wrapper API: invalidate_cache, cache_clear, cache_info - 38 tests (10 critical, 28 unit), all passing in <1s Closes the product gap where users abandon cachekit for DIY dict caches when hitting serialization failures on opaque objects.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 8 minutes and 49 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR introduces Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Decorator as `@cache.local`() Decorator
participant KeyGen as CacheKeyGenerator
participant Cache as ObjectCache
participant Function as Original Function
Caller->>Decorator: Call with args
Decorator->>KeyGen: Generate cache key from args
KeyGen-->>Decorator: Return deterministic key
Decorator->>Cache: get(key)
alt Cache Hit
Cache-->>Decorator: (True, cached_object)
Decorator-->>Caller: Return cached object
else Cache Miss
Decorator->>Function: Compute result
Function-->>Decorator: Return result
Decorator->>Cache: put(key, result, ttl)
Cache-->>Decorator: Stored in ObjectCache
Decorator-->>Caller: Return result
end
sequenceDiagram
participant Async as Async Caller
participant Decorator as `@cache.local`() Decorator
participant KeyGen as CacheKeyGenerator
participant Cache as ObjectCache
participant Function as Async Function
Async->>Decorator: await call with args
Decorator->>KeyGen: Generate cache key from args
KeyGen-->>Decorator: Return deterministic key
Decorator->>Cache: get(key)
alt Cache Hit (Awaited Value)
Cache-->>Decorator: (True, cached_object)
Decorator-->>Async: Return cached object
else Cache Miss
Decorator->>Function: Await function execution
Function-->>Decorator: Return result
Decorator->>Cache: put(key, result, ttl)
Cache-->>Decorator: Stored in ObjectCache
Decorator-->>Async: Return result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Remove notest from 4 self-contained examples (mutation, identity, wrapper API). Keep notest only on examples requiring external deps (Langfuse, torch, sqlalchemy, httpx). Fix SQLAlchemy.create_engine typo.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
The test_serializer_codes_mapping test hardcodes the expected dict. Adding 'local': 'l' to match the key_generator change from feat.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
src/cachekit/decorators/intent.py (1)
110-118: Short-circuit placement and config rejection look correct.Placing the local-intent branch before the
backendpop /l1_enabledmapping is necessary — otherwise those kwargs would be silently consumed andcreate_local_wrapper's strict whitelist would never see them. Good catch.One small wording note: the
TypeErrormessage framesconfig=rejection purely as an encryption concern, butDecoratorConfigcarries many things besides encryption (circuit breaker, timeouts, serializer, etc.), and the more general reasonconfig=is rejected is that@cache.local()bypasses the entire backend pipeline. Consider broadening the message slightly, e.g.:- "@cache.local() stores object references in-process — encryption " - "requires serialization to bytes. For encrypted caching, use `@cache.secure`()." + "@cache.local() stores object references in-process and bypasses the " + "backend/serializer/encryption pipeline, so config= is not supported. " + "Use `@cache`(config=...) for serialized caching, or `@cache.secure`() for encryption."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cachekit/decorators/intent.py` around lines 110 - 118, The TypeError message raised in the local-intent branch (when _intent == "local") is too narrowly worded around encryption; update the message in that branch (the check that rejects config passed to `@cache.local`()) to explain that DecoratorConfig contains multiple backend-related settings and `@cache.local`() bypasses the backend pipeline, so config= is not supported; keep the same location (the if _intent == "local" block) and retain the rejection behavior and reference to create_local_wrapper.tests/unit/test_local_wrapper.py (2)
130-138: Asynccache_clearsmoke test is intentionally minimal — consider also asserting state.
cache_clear()is exercised on the wrapper, but the test only checks that it doesn't raise. A small follow-up that populates the cache (await afn(1)), clears, then checksafn.cache_info().currsize == 0would lock down behavior in addition to "doesn't raise".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/test_local_wrapper.py` around lines 130 - 138, Update the test_cache_clear_on_async_does_not_raise to also assert state: after wrapping async function afn with cache.local(), call and await afn(1) to populate the cache, then call afn.cache_clear(), and finally assert afn.cache_info().currsize == 0 (or equivalent property) to verify the cache was actually cleared; keep the existing "does not raise" check and add these assertions referencing afn, cache.local(), cache_clear, and cache_info.
18-58:match="@cache.local()"is a regex — escape it or use a substring without special chars.
pytest.raises(..., match=...)treats the string as a regex. Here.is "any char" and()is an empty capture group, so the pattern effectively matches "@cache.local" anywhere in the message. It happens to pass today, but it would also pass against an unrelated message like "@cache_local". Recommend escaping or matching a more specific fragment.♻️ Example fix (apply to all six occurrences)
- with pytest.raises(TypeError, match="@cache.local()"): + with pytest.raises(TypeError, match=r"@cache\.local\(\)"):Or anchor on the more specific error fragment, e.g.
match=r"only accepts: key, max_entries, namespace, ttl".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/test_local_wrapper.py` around lines 18 - 58, The tests in tests/unit/test_local_wrapper.py use pytest.raises(..., match="@cache.local()") which is interpreted as a regex (dots and parens are special); update the six tests that decorate functions with `@cache.local`(...) so the match argument either escapes the regex (e.g. use r"\@cache\.local\(\)") or replace match with a more specific, non-regex substring/error fragment (e.g. r"only accepts: key, max_entries, namespace, ttl") or remove the match and assert the TypeError without a regex; locate the assertions around the cache.local decorator usages inside test_serializer_rejected, test_encryption_rejected, test_master_key_rejected, test_integrity_checking_rejected, test_config_rejected and the adjacent backend-rejected test and change their pytest.raises match accordingly.src/cachekit/decorators/local_wrapper.py (3)
107-130: Concurrent first-time misses can create duplicate expensive resources.The sync and async wrappers both follow a classic check-then-act pattern (
get→ if miss, callfunc, thenput). Under concurrent invocation with the same key on a cold cache, multiple callers will each invokefunc()and the lastputwins. For the documented headline use cases this is consequential:
- ML models (
load_model("resnet50")) — multiple loads of the same large weight matrix.- DB connections / SQLAlchemy engines — duplicate engines created and the losers leak (no
on_evictcleanup yet).- HTTP/SDK clients with auth handshakes — duplicate authenticated sessions.
functools.lru_cachehas the same property, so this is defensible, but the docs indocs/features/reference-caching.mdactively pitch "load once, reuse across requests". Consider one of:
- Documenting the behavior explicitly in the Mutation/Identity section.
- Adding a per-key lock (e.g.,
defaultdict(threading.Lock)for sync andasyncio.Lockfor async, evicted alongside entries) so first-miss work is deduplicated.If you want to keep this PR focused, a short doc note plus a tracking issue would be sufficient.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cachekit/decorators/local_wrapper.py` around lines 107 - 130, The wrappers (async_wrapper and sync_wrapper) use a check-then-act pattern around object_cache.get/put causing duplicate expensive work on concurrent first-time misses; fix by introducing per-key deduplication: compute cache_key via _make_key, then acquire a per-key lock before calling func so only the first waiter executes the expensive work, release the lock after put (use threading.Lock for the sync_wrapper and asyncio.Lock for async_wrapper), store locks in a dict keyed by cache_key (e.g., defaultdict) and ensure locks are cleaned up/evicted when the cache entry is evicted; alternatively, if you prefer not to change runtime behavior now, add a clear note in docs (reference caching docs and these wrappers) and open a tracking issue for implementing per-key locks.
51-59: Minor: no type validation onttl/max_entries.
kwargs.get("ttl", 300)accepts any type; the< 1checks happen to also fail for most non-numerics by raisingTypeErrorfrom comparison, but e.g.ttl=1.5silently flows through toObjectCache.put(..., ttl)and is added totime.monotonic(). Probably harmless, but a smallisinstance(ttl, int)check would make the failure mode explicit and consistent with theintannotation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cachekit/decorators/local_wrapper.py` around lines 51 - 59, Validate types for ttl and max_entries before the range checks: ensure ttl and max_entries are ints (use isinstance(ttl, int) / isinstance(max_entries, int)) and raise a TypeError with a clear message if not, then proceed with the existing ValueError checks (e.g., "ttl must be an int, got {type(ttl).__name__}"). Update the block that currently reads/annotates ttl and max_entries (and related variables like namespace/key) so the explicit type checks happen before the "if ttl < 1" and "if max_entries < 1" comparisons.
79-85:invalidate_cache/ainvalidate_cacheignore a user-suppliedkey=that depends on call context.When the user passes a custom
key=callable,invalidate_cache(*args, **kw)regenerates the key by re-invokingkey(*args, **kw). That works for pure key functions, but if a user provides a key callable that pulls from request context, threadlocals, etc. (a reasonable pattern for namespaces/multi-tenancy), the regenerated key may not match the one used at insert time, and the entry won't be evicted.Optional improvement: also accept a
key=override oninvalidate_cache, or document this constraint onkey=callables indocs/features/reference-caching.md.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cachekit/decorators/local_wrapper.py` around lines 79 - 85, invalidate_cache and ainvalidate_cache currently regenerate keys via _make_key(args, kw) which fails when callers used a context-dependent key callable; change both functions to accept an optional key= parameter (e.g., def invalidate_cache(*args, key: Optional[Union[str, Callable]] = None, **kw)) and if key is provided use it directly (if str) or call it with the same args/kw (if callable) to produce the deletion key, otherwise fall back to _make_key(args, kw); apply the same change to ainvalidate_cache and ensure you call object_cache.delete with the resolved key; alternatively document the limitation in docs/features/reference-caching.md if you don't want to change the API.src/cachekit/key_generator.py (1)
53-78: Updateserializer_typedocstring to include"local".Line 70 still lists only
("std", "auto", "orjson", "arrow"). Now that"local"is a valid input (and used bycreate_local_wrapper), please add it for completeness.📝 Proposed doc tweak
- serializer_type: Serializer type code ("std", "auto", "orjson", "arrow") + serializer_type: Serializer type code ("std", "auto", "orjson", "arrow", "local")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cachekit/key_generator.py` around lines 53 - 78, The docstring for generate_key needs to list "local" as an accepted serializer_type; update the serializer_type description in the generate_key method to include "local" alongside "std", "auto", "orjson", and "arrow" (since create_local_wrapper uses "local"), so the doc accurately documents all valid serializer_type values and the compact metadata suffix note remains unchanged.src/cachekit/object_cache.py (1)
117-118: Nit:move_to_endafter insertion is redundant.Newly assigned keys in an
OrderedDictare already placed at the end, so themove_to_end(key)on Line 118 is a no-op for a fresh insert (the in-place update path on Line 110 needs it; this branch does not).♻️ Suggested cleanup
self._store[key] = (value, expires_at) - self._store.move_to_end(key)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cachekit/object_cache.py` around lines 117 - 118, The insertion path in the cache method that does self._store[key] = (value, expires_at) redundantly calls self._store.move_to_end(key) afterward; remove that move_to_end call in the fresh-insert branch (leave the existing move_to_end in the in-place update branch intact) so only the update path uses move_to_end on _store in object_cache.py.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/features/reference-caching.md`:
- Around line 139-152: The fenced code block in the lifecycle diagram is missing
a language tag (MD040); update the opening triple-backtick to include a language
such as "text" (e.g. change ``` to ```text) so the diagram block is properly
annotated and linting passes; locate the diagram block in reference-caching.md
and add the language to its opening fence.
In `@src/cachekit/object_cache.py`:
- Around line 93-118: The put method allows ttl values < 1 contrary to its
docstring; add validation at the start of ObjectCache.put to enforce ttl >= 1
(raise ValueError with a clear message), mirroring the validation pattern used
in __init__ for _max_entries, so that expires_at calculation and interactions
with _store/_evict_to_make_room behave correctly for all entries.
In `@tests/critical/test_local_cache_works.py`:
- Around line 62-72: The test_invalidate_cache is flaky because it relies on
time.monotonic() changing between cached calls; replace the time-based
uniqueness with a deterministic side-effect counter similar to
test_basic_cache_hit: create a mutable counter/closure or use a
nonlocal/incremented variable inside the decorated greet function (the function
wrapped by cache.local) to produce different outputs on re-execution, call
greet("alice"), invalidate_cache("alice"), call greet("alice") again, and assert
the outputs differ; ensure you reference the decorated function name greet and
the method greet.invalidate_cache for the invalidation step.
---
Nitpick comments:
In `@src/cachekit/decorators/intent.py`:
- Around line 110-118: The TypeError message raised in the local-intent branch
(when _intent == "local") is too narrowly worded around encryption; update the
message in that branch (the check that rejects config passed to `@cache.local`())
to explain that DecoratorConfig contains multiple backend-related settings and
`@cache.local`() bypasses the backend pipeline, so config= is not supported; keep
the same location (the if _intent == "local" block) and retain the rejection
behavior and reference to create_local_wrapper.
In `@src/cachekit/decorators/local_wrapper.py`:
- Around line 107-130: The wrappers (async_wrapper and sync_wrapper) use a
check-then-act pattern around object_cache.get/put causing duplicate expensive
work on concurrent first-time misses; fix by introducing per-key deduplication:
compute cache_key via _make_key, then acquire a per-key lock before calling func
so only the first waiter executes the expensive work, release the lock after put
(use threading.Lock for the sync_wrapper and asyncio.Lock for async_wrapper),
store locks in a dict keyed by cache_key (e.g., defaultdict) and ensure locks
are cleaned up/evicted when the cache entry is evicted; alternatively, if you
prefer not to change runtime behavior now, add a clear note in docs (reference
caching docs and these wrappers) and open a tracking issue for implementing
per-key locks.
- Around line 51-59: Validate types for ttl and max_entries before the range
checks: ensure ttl and max_entries are ints (use isinstance(ttl, int) /
isinstance(max_entries, int)) and raise a TypeError with a clear message if not,
then proceed with the existing ValueError checks (e.g., "ttl must be an int, got
{type(ttl).__name__}"). Update the block that currently reads/annotates ttl and
max_entries (and related variables like namespace/key) so the explicit type
checks happen before the "if ttl < 1" and "if max_entries < 1" comparisons.
- Around line 79-85: invalidate_cache and ainvalidate_cache currently regenerate
keys via _make_key(args, kw) which fails when callers used a context-dependent
key callable; change both functions to accept an optional key= parameter (e.g.,
def invalidate_cache(*args, key: Optional[Union[str, Callable]] = None, **kw))
and if key is provided use it directly (if str) or call it with the same args/kw
(if callable) to produce the deletion key, otherwise fall back to
_make_key(args, kw); apply the same change to ainvalidate_cache and ensure you
call object_cache.delete with the resolved key; alternatively document the
limitation in docs/features/reference-caching.md if you don't want to change the
API.
In `@src/cachekit/key_generator.py`:
- Around line 53-78: The docstring for generate_key needs to list "local" as an
accepted serializer_type; update the serializer_type description in the
generate_key method to include "local" alongside "std", "auto", "orjson", and
"arrow" (since create_local_wrapper uses "local"), so the doc accurately
documents all valid serializer_type values and the compact metadata suffix note
remains unchanged.
In `@src/cachekit/object_cache.py`:
- Around line 117-118: The insertion path in the cache method that does
self._store[key] = (value, expires_at) redundantly calls
self._store.move_to_end(key) afterward; remove that move_to_end call in the
fresh-insert branch (leave the existing move_to_end in the in-place update
branch intact) so only the update path uses move_to_end on _store in
object_cache.py.
In `@tests/unit/test_local_wrapper.py`:
- Around line 130-138: Update the test_cache_clear_on_async_does_not_raise to
also assert state: after wrapping async function afn with cache.local(), call
and await afn(1) to populate the cache, then call afn.cache_clear(), and finally
assert afn.cache_info().currsize == 0 (or equivalent property) to verify the
cache was actually cleared; keep the existing "does not raise" check and add
these assertions referencing afn, cache.local(), cache_clear, and cache_info.
- Around line 18-58: The tests in tests/unit/test_local_wrapper.py use
pytest.raises(..., match="@cache.local()") which is interpreted as a regex (dots
and parens are special); update the six tests that decorate functions with
`@cache.local`(...) so the match argument either escapes the regex (e.g. use
r"\@cache\.local\(\)") or replace match with a more specific, non-regex
substring/error fragment (e.g. r"only accepts: key, max_entries, namespace,
ttl") or remove the match and assert the TypeError without a regex; locate the
assertions around the cache.local decorator usages inside
test_serializer_rejected, test_encryption_rejected, test_master_key_rejected,
test_integrity_checking_rejected, test_config_rejected and the adjacent
backend-rejected test and change their pytest.raises match accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 63950e2a-b04c-448f-b2ce-76be3a08a9b1
📒 Files selected for processing (11)
README.mddocs/features/reference-caching.mdsrc/cachekit/decorators/intent.pysrc/cachekit/decorators/local_wrapper.pysrc/cachekit/key_generator.pysrc/cachekit/object_cache.pytests/critical/conftest.pytests/critical/test_local_cache_works.pytests/unit/test_key_generator_blake2b.pytests/unit/test_local_wrapper.pytests/unit/test_object_cache.py
Inline fixes: - Add ttl >= 1 validation in ObjectCache.put() (docstring promised it) - Add language tag to lifecycle diagram fenced block (MD040) - Make test_invalidate_cache deterministic (counter, not monotonic) Nitpick fixes: - Broaden config= TypeError message (not just encryption) - Add isinstance checks for ttl/max_entries types in local_wrapper - Add "local" to generate_key() docstring's serializer_type list - Remove redundant move_to_end on fresh insert in ObjectCache - Assert cache state in async cache_clear test - Escape regex in pytest.raises match strings
1 participant
Summary
@cache.local()decorator preset for caching opaque Python objects (SDK clients, connections, ML models) that can't be serializedObjectCacheclass with thread-safe entry-count LRU + TTL — no modifications to L1Cache or wrapper.py@cache:invalidate_cache,cache_clear,cache_info,__wrapped__Spec:
.spec-workflow/specs/cache-local/(requirements, design, tasks — all approved)What's New
src/cachekit/object_cache.pysrc/cachekit/decorators/local_wrapper.pysrc/cachekit/decorators/intent.pycache.localpreset + intent short-circuitsrc/cachekit/key_generator.py"local": "l"in SERIALIZER_CODESdocs/features/reference-caching.mdREADME.mdTest plan
make quick-checkpasses (lint + format + type-check + critical tests)Summary by CodeRabbit
Release Notes
New Features
@cache.local()preset decorator for in-process caching of opaque, non-serializable Python objects with LRU/TTL eviction, per-key invalidation, and cache statistics.Documentation
@cache.local()covering use cases, configuration parameters, lifecycle semantics, and comparison with alternatives.@cache.local()option alongside existing cache strategies.