-
Notifications
You must be signed in to change notification settings - Fork 2
allow KVCache wrapping #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #18 +/- ##
==========================================
+ Coverage 97.98% 99.24% +1.26%
==========================================
Files 8 8
Lines 1190 1192 +2
==========================================
+ Hits 1166 1183 +17
+ Misses 18 7 -11
+ Partials 6 2 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enables KVCache to be used with the Locker and Updater wrapper types by refactoring the ListByPrefix method to use interface-based type assertions instead of concrete type checks. Previously, only the KV wrapper could be used with ListByPrefix in these wrappers; now both KV and KVCache implementations are supported.
Key changes:
- Introduced
listerByPrefix[V]interface to enable duck typing for any cache implementingListByPrefix - Updated type assertions in
LockerandUpdaterto use the new interface instead of checking for concrete*KV[V]type - Added test coverage for
KVCacheusage with both wrappers
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| locker.go | Defines listerByPrefix interface and updates ListByPrefix method to use interface-based type assertion |
| locker_test.go | Adds TestLockerListByPrefixCache to verify KVCache works with Locker wrapper |
| updater.go | Updates ListByPrefix method to use interface-based type assertion (interface defined in locker.go) |
| updater_test.go | Adds TestUpdaterListByPrefixCache to verify KVCache works with Updater wrapper |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type listerByPrefix[V any] interface { | ||
| ListByPrefix(prefix string) ([]V, error) | ||
| } |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The listerByPrefix interface is currently defined in locker.go but is also used in updater.go. While this works since both files are in the same package, it would be better to define this interface in a more central location (e.g., doc.go alongside the Geche interface) for better code organization and discoverability. This makes it clearer that this is a shared interface used across multiple wrapper types.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
allow KVCache ListByPrefix to be accessed from Locker and Updater wrappers