refactor!: delete dead L1 namespace-index/bulk-invalidation machinery (LAB-1433) - #258
Conversation
This comment has been minimized.
This comment has been minimized.
WalkthroughThe L1 cache no longer supports namespace-indexed invalidation. Related configuration and documentation were removed. Invalidation tests and benchmarks were deleted. ChangesL1 namespace invalidation removal
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cachekit/l1_cache.py (1)
57-62: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd a return annotation to
L1Cache.__init__.
L1Cache.__init__is a public API. Add-> Noneto meet the required public API type-hint rule.Proposed fix
def __init__( self, max_memory_mb: int = 100, ttl_buffer_seconds: float = 1.0, namespace: str = "default", - ): + ) -> None:As per path instructions, type hints on public APIs are required.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cachekit/l1_cache.py` around lines 57 - 62, Update the public constructor L1Cache.__init__ to include a -> None return annotation, leaving its parameters and initialization behavior unchanged.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/configuration.md`:
- Line 350: Update the io() row in the configuration table to replace “Managed
SaaS backend” with the established service name “CachekitIO,” preserving the
surrounding access and stale-TTL details.
---
Outside diff comments:
In `@src/cachekit/l1_cache.py`:
- Around line 57-62: Update the public constructor L1Cache.__init__ to include a
-> None return annotation, leaving its parameters and initialization behavior
unchanged.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 273158f0-4f7a-4c70-9f37-51271abf84c8
📒 Files selected for processing (9)
.secrets.baselineREADME.mddocs/configuration.mddocs/features/l1-invalidation.mdsrc/cachekit/config/decorator.pysrc/cachekit/config/nested.pysrc/cachekit/l1_cache.pytests/performance/test_l1_invalidation_benchmarks.pytests/unit/test_l1_invalidation.py
💤 Files with no reviewable changes (5)
- tests/performance/test_l1_invalidation_benchmarks.py
- src/cachekit/config/nested.py
- src/cachekit/config/decorator.py
- README.md
- tests/unit/test_l1_invalidation.py
![kodus-27b[bot]](https://avatars.githubusercontent.com/u/223823690?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…AB-1433) CodeRabbit review on #258: the io() row said "Managed SaaS backend" generically instead of naming CachekitIO, the established service name used elsewhere in the docs.
This comment has been minimized.
This comment has been minimized.
… (LAB-1433) The L1 namespace-index / bulk-invalidation surface (CacheEntry.namespace, L1Cache's config param + _namespace_index, put()'s namespace= kwarg, invalidate_by_key/invalidate_by_namespace/invalidate_all, and L1CacheConfig.namespace_index) was inert end-to-end: the production L1Cache is constructed without config, the decorator wrapper never passes namespace= to put(), and there were zero src/ callers of the three invalidate_by_* methods. Same trust-bug family as LAB-388/LAB-520 — delete rather than wire, per that precedent. Docs (README preset matrix, configuration.md, l1-invalidation.md) corrected to stop claiming the feature. The live per-key invalidation path (L1Cache.invalidate(), used by invalidate_cache()) is untouched. BREAKING CHANGE: L1CacheConfig.namespace_index is removed. The flag was read by nothing and toggled no behavior, but it shipped in v0.17.1 and docs/configuration.md documented a copy-pasteable L1CacheConfig(..., namespace_index=True) example — L1CacheConfig is a frozen dataclass, so constructors still passing it now raise TypeError instead of silently lying. L1Cache.invalidate_by_key(), .invalidate_by_namespace() and .invalidate_all() are removed with it; per-key L1Cache.invalidate() is unaffected. Same removal shape as L1CacheConfig.invalidation_enabled in v0.16.0 (LAB-520).
…AB-1433) CodeRabbit review on #258: the io() row said "Managed SaaS backend" generically instead of naming CachekitIO, the established service name used elsewhere in the docs.
ccd5532 to
383eeba
Compare
27Bslash6
changed the title
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
Review-gate finding: this removal is breaking, and was typed as a patchCaught during the review-signoff gate. No code changed — The problem. Removing a field from a frozen dataclass is not a silent ignore; Precedent is unambiguous. This repo has removed a dead
That was #237 ( The fix. Two changes, no code:
Both were needed: this repo squash-merges with Net effect: release-please now cuts 0.18.0 with a ⚠ BREAKING CHANGES entry instead of a silent 0.17.2. Everything else stands. The deletion itself is correct and stays as reviewed — zero residual references to |
1 participant
Summary
This PR removes unused (dead) namespace-index and bulk-invalidation functionality from the L1 cache, along with all associated configuration and documentation.
Changes
Configuration (
L1CacheConfig)namespace_indexconfig option entirely fromnested.py.namespace_index=...settings from theminimal(),production(),secure(),dev(),test(), andio()intent presets indecorator.py.L1 Cache (
l1_cache.py)namespacefield from theCacheEntrydataclass.configandnamespaceparameters fromL1Cache.__init__andput()._namespace_indexdata structure and all logic that maintained it duringput()and_remove_entry().invalidate_by_key(),invalidate_by_namespace(), andinvalidate_all().Documentation
namespace_indexreferences and the "Namespace Index" feature column fromREADME.md,docs/configuration.md, anddocs/features/l1-invalidation.md, including updated feature/preset matrices.Purpose
The namespace-index and bulk-invalidation machinery was dead code — the configuration was exposed and documented but the invalidation methods were unused. This change deletes it to simplify the L1 cache implementation, its configuration surface, and the associated documentation (LAB-1433).
Notes
The
.secrets.baselinefile was updated to reflect the shifted line number indecorator.pyand a new generation timestamp.Based on the code changes provided, I can only see a single documentation change in
docs/configuration.md.Description
This pull request updates a documentation label in
docs/configuration.md, changing the description of theio()preset's backend from "Managed SaaS backend" to "CachekitIO managed SaaS backend" to more explicitly identify the managed service by name.Note on Scope
The PR title indicates this change should delete dead namespace-index/bulk-invalidation machinery in the L1 layer (LAB-1433). However, the code changes provided contain only a minor documentation wording update in
docs/configuration.md.The actual dead-code deletion described in the title is not reflected in the provided patches. If the L1 namespace-index/bulk-invalidation removal is intended to be part of this PR, those file changes appear to be missing from the diff shown here. I'd recommend verifying that all intended changes were included before merging.
Summary
This PR removes the dead L1 namespace-index and bulk-invalidation code that was never wired into any active code path (LAB-1433).
Changes
Configuration cleanup:
namespace_indexfield fromL1CacheConfiginsrc/cachekit/config/nested.pynamespace_indexsettings from the six intent presets (minimal,production,secure,dev,test,io) insrc/cachekit/config/decorator.pyL1 cache implementation cleanup (
src/cachekit/l1_cache.py):namespacefield fromCacheEntry_namespace_indexdata structure and all bookkeeping tied to it input,_remove_entry, and initializationconfigandnamespace/per-entry namespace parameters from__init__andputinvalidate_by_key,invalidate_by_namespace, andinvalidate_alldefaultdict)Documentation updates:
namespace_indexand the L1 Namespace Index feature fromREADME.md,docs/configuration.md, anddocs/features/l1-invalidation.md, including the corresponding feature-matrix columnsOther:
.secrets.baselineto reflect shifted line numbers and a new generation timestampPurpose
The namespace-index/bulk-invalidation machinery was unused dead code. Removing it simplifies the L1 cache, configuration surface, and documentation while eliminating per-entry namespace tracking overhead.
Summary by CodeRabbit
Changes
Documentation