feat: Enforce BearType on Store/Wrapper methods #126
Conversation
- Add bear_enforce decorator to enable strict type checking - Apply enforcement to BaseStore methods (get, get_many, ttl, ttl_many, put, put_many, delete, delete_many) - Apply enforcement to BaseWrapper methods (same 8 methods) - Update documentation to reflect new enforcement policy - Regenerate sync library with enforcement decorators Core protocol methods now raise TypeError for type violations instead of just warnings, providing stronger type safety at the API boundary between user code and storage implementations. Closes #120 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: William Easton <strawgate@users.noreply.github.com>
📝 WalkthroughWalkthroughAdds a strict beartype enforcement path and a Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Wrapper
participant Enforcer as "bear_enforce\n(type-check wrapper)"
participant Store
Caller->>Wrapper: call get/put/delete/ttl(...)
Wrapper->>Enforcer: invoke decorated method
note right of Enforcer #E6F2FF: Validate args/returns\nraise TypeError on violation
Enforcer->>Store: forward validated call
Store-->>Enforcer: return result or raise existing errors
Enforcer-->>Wrapper: return validated result or propagate TypeError
Wrapper-->>Caller: return result or propagate exception
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.md📄 CodeRabbit inference engine (AGENTS.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (1)
Comment |
|
@claude can you fix up this PR please |
|
Claude finished @strawgate's task —— View job SummaryI've fixed up the PR to address the SonarQube Quality Gate failure. What was wrongThe SonarQube analysis was failing because it detected 4.5% code duplication in the new code (the threshold is ≤ 3%). The duplication was coming from:
Changes made
Verification
The SonarQube Quality Gate should now pass on the next CI run, as the generated sync library will be excluded from duplication checks. --- | |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…cation checks The sync library is automatically generated from the async library, so code duplication between them is expected and intentional. This change adds sonar-project.properties to exclude the sync library from duplication detection, which should resolve the Quality Gate failure. Also fixes import ordering in base wrapper per linter requirements. Co-authored-by: William Easton <strawgate@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
README.md(1 hunks)key-value/key-value-aio/src/key_value/aio/stores/base.py(8 hunks)key-value/key-value-sync/src/key_value/sync/code_gen/stores/base.py(8 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
key-value/key-value-sync/**
📄 CodeRabbit inference engine (AGENTS.md)
Never edit files in key-value/key-value-sync/ directly; regenerate from async via make codegen
Files:
key-value/key-value-sync/src/key_value/sync/code_gen/stores/base.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Ruff for Python formatting and linting (configured in pyproject.toml)
Python line length is 140 characters
Use Basedpyright in strict mode for static type checking
Files:
key-value/key-value-sync/src/key_value/sync/code_gen/stores/base.pykey-value/key-value-aio/src/key_value/aio/stores/base.py
key-value/key-value-sync/src/key_value/sync/code_gen/stores/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Sync store files are generated mirrors of async stores; do not hand-edit implementations here
Files:
key-value/key-value-sync/src/key_value/sync/code_gen/stores/base.py
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
**/*.md: Use Ruff/markdown support and markdownlint for Markdown files per repository configuration
Markdown line length is 80 characters (excluding code blocks and tables)
Files:
README.md
key-value/key-value-aio/src/key_value/aio/stores/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
key-value/key-value-aio/src/key_value/aio/stores/**/*.py: Store implementations must persist ManagedEntry wrappers (not raw values) with value, creation timestamp, and TTL metadata
Async store implementations must implement the AsyncKeyValue protocol (key_value/aio/protocols/key_value.py)
Files:
key-value/key-value-aio/src/key_value/aio/stores/base.py
🧠 Learnings (1)
📚 Learning: 2025-10-26T16:57:34.087Z
Learnt from: CR
PR: strawgate/py-key-value#0
File: AGENTS.md:0-0
Timestamp: 2025-10-26T16:57:34.087Z
Learning: Applies to key-value/key-value-sync/src/key_value/sync/code_gen/stores/**/*.py : Sync store files are generated mirrors of async stores; do not hand-edit implementations here
Applied to files:
key-value/key-value-sync/src/key_value/sync/code_gen/stores/base.py
🧬 Code graph analysis (2)
key-value/key-value-sync/src/key_value/sync/code_gen/stores/base.py (1)
key-value/key-value-shared/src/key_value/shared/type_checking/bear_spray.py (1)
bear_enforce(21-23)
key-value/key-value-aio/src/key_value/aio/stores/base.py (1)
key-value/key-value-shared/src/key_value/shared/type_checking/bear_spray.py (1)
bear_enforce(21-23)
🔇 Additional comments (4)
key-value/key-value-sync/src/key_value/sync/code_gen/stores/base.py (2)
1-17: Auto-generated file correctly reflects async source.This file is auto-generated from the async implementation (as indicated by the header warning). The addition of the
bear_enforceimport correctly mirrors the async source file.Based on learnings
114-252: Enforcement decorators correctly applied to all core protocol methods.The
@bear_enforcedecorator has been correctly applied to all 8 core protocol methods (get, get_many, ttl, ttl_many, put, put_many, delete, delete_many). The decorator order is correct (@bear_enforce before @OverRide), and internal helper methods are appropriately left undecorated.key-value/key-value-aio/src/key_value/aio/stores/base.py (2)
14-14: Import added correctly.The
bear_enforceimport from the shared type checking module is correctly added.
111-272: Excellent enforcement boundary at the API layer.The
@bear_enforcedecorator is correctly applied to all 8 core protocol methods (get, get_many, ttl, ttl_many, put, put_many, delete, delete_many), establishing strict type enforcement at the API boundary. Internal helper methods are appropriately left undecorated, keeping enforcement focused on the public interface. Decorator ordering (@bear_enforce before @OverRide) is correct.
|
@claude we have various tests that pass invalid types which will now cause BearType exceptions, we'll need to update our tests accordingly |
|
Claude finished @strawgate's task —— View job SummaryI've successfully updated the tests to be compatible with BearType enforcement on core protocol methods. What was the issue?The BearType enforcement added in this PR correctly validates that the
With BearType enforcement, these now fail with Changes madeFile modified: Removed test cases:
These cases are now redundant because BearType's type enforcement provides stronger guarantees at the API boundary. Verification✅ All tests passing: 2293 passed, 267 skipped The removed test cases were testing behavior that violates the type signature. With beartype enforcement, the library now correctly rejects these at the API boundary with clear TypeError messages. --- | Latest commit |
…artype enforcement Remove test cases that pass dicts with non-string keys (None, int, float, bool, datetime, UUID, bytes, tuple) as these violate the Mapping[str, Any] type signature and are now correctly rejected by beartype before reaching serialization. The library should enforce strict typing at the API boundary. Removed: - Positive cases expecting implicit key serialization (e.g., None -> 'null') - Negative cases with non-string keys (now caught by type checking) Co-authored-by: William Easton <strawgate@users.noreply.github.com>
|
@claude please remove comments that say something was removed. Also see pr feedback, coderabbit is wrong the protocol isnt enforced but the stores and wrappers should be and we should mentioned that. Then do a thorough line by line review of the PR |
|
Claude finished @strawgate's task —— View job Comprehensive Line-by-Line Review Complete ✅Todo List:
Changes Made1. Removed "Removed:" comments from test cases (key-value-shared-test/src/key_value/shared_test/cases.py)
2. Updated README.md to clarify enforcement scope (lines 104-109)
Comprehensive Line-by-Line Review✅ File: key-value-shared/src/key_value/shared/type_checking/bear_spray.pyLines 10-11: New enforcement configuration enforce_bear_type_conf = BeartypeConf(strategy=BeartypeStrategy.O1, violation_type=TypeError)
enforce_bear_type = beartype(conf=enforce_bear_type_conf)
Lines 21-23: New decorator def bear_enforce(func: Callable[P, R]) -> Callable[P, R]:
"""Enforce beartype with exceptions instead of warnings."""
return enforce_bear_type(func)
✅ File: key-value-aio/src/key_value/aio/stores/base.pyLine 14: Import added from key_value.shared.type_checking.bear_spray import bear_enforce
Lines 138-139, 168-169, 177-178, 190-191, 224-225, 254-255, 291-292, 299-300: @bear_enforce
@override
async def get(self, key: str, *, collection: str | None = None) -> dict[str, Any] | None:
✅ File: key-value-aio/src/key_value/aio/wrappers/base.pyLine 4: Import added from key_value.shared.type_checking.bear_spray import bear_enforce
Lines 15-16, 20-21, 25-26, 30-31, 35-36, 40-41, 52-53, 57-58: @bear_enforce
@override
async def get(self, key: str, *, collection: str | None = None) -> dict[str, Any] | None:
return await self.key_value.get(collection=collection, key=key)
✅ File: key-value-sync/src/key_value/sync/code_gen/stores/base.pyLine 17: Import added (code-generated) from key_value.shared.type_checking.bear_spray import bear_enforce
Lines 144-145, 169-170, 178-179, 191-192, 216-217, 242-243, 274-275, 282-283:
✅ File: key-value-sync/src/key_value/sync/code_gen/wrappers/base.pyLine 7: Import added (code-generated)
Lines 18-19, 24-25, 29-30, 34-35, 39-40, 44-45, 51-52, 56-57:
✅ File: sonar-project.propertiesLines 1-7: New SonarQube configuration sonar.projectKey=strawgate_py-key-value
sonar.organization=strawgate
sonar.cpd.exclusions=**/key-value-sync/**/*.py
✅ File: README.mdLines 104-109: Updated enforcement documentation - **Dislike of Bear Bros**: Beartype is used for runtime type checking. Core
protocol methods in store and wrapper implementations (put/get/delete/ttl
and their batch variants) enforce types and will raise TypeError for
violations. Other code produces warnings. You can disable all beartype
checks by setting `PY_KEY_VALUE_DISABLE_BEARTYPE=true` or suppress warnings
via the warnings module.
✅ File: key-value-shared-test/src/key_value/shared_test/cases.pyLines 111-112, 129-131, 145, 158, 182, 204, 219, 237: Removed comments cleaned up
Context: Why tests were removed
Issues & Concerns🟢 No Critical Issues FoundThe implementation is solid and well-executed:
🟡 Minor Observations (Not Issues)
SummaryCompleted all requested tasks:
Recommendation: APPROVE ✅ This PR successfully implements beartype enforcement exactly as designed:
The changes improve type safety without compromising flexibility or performance. |
strawgate
changed the title
- Update README to specify enforcement is in store/wrapper implementations - Remove "Removed:" comments from test cases for cleaner code - Clarify that batch variants (get_many, put_many, etc.) are also enforced Co-authored-by: William Easton <strawgate@users.noreply.github.com>
|
Implements beartype enforcement on core protocol methods as identified in issue #120.
Changes
bear_enforcedecorator to enable strict type checkingCore protocol methods now raise TypeError for type violations instead of just warnings, providing stronger type safety at the API boundary between user code and storage implementations.
Closes #120
🤖 Generated with Claude Code) | View branch | [View job run](https://github.com/strawgate/py-key-value/actions/runs/18813925806
Summary by CodeRabbit
New Features
Documentation
Tests
Chores