Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #219 +/- ##
==========================================
+ Coverage 93.73% 94.05% +0.32%
==========================================
Files 78 78
Lines 11357 11659 +302
Branches 656 669 +13
==========================================
+ Hits 10645 10966 +321
+ Misses 642 622 -20
- Partials 70 71 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
lan17
left a comment
There was a problem hiding this comment.
Review of 3414d5f against the #218 design. One item to fix before merge, three non-blocking notes, and a list of what was verified. Inline comments mark the exact spots.
Fix before merge
Python delete has an undocumented Key-instance branch that silently ignores its own arguments. When key is a Key, the required key_type and use_case keyword arguments and track_for_invalidation are discarded without checking that they agree with the Key. Nothing in the tests, docs, interop harness, or examples exercises this branch. It can target a different entry than the caller named, and because a missing entry succeeds the mismatch is invisible. Delete the branch, or make the three kwargs optional when a Key is passed, reject disagreement, and test it.
Non-blocking
deletionsjoined the shared observation record. That one change inconformance-observations.qntis why twenty-plus smoke traces and witness fixtures churned and whydifferential.mjsneededreferenceDescriptorto project a zero baseline into corpora generated fromorigin/main. The bridge is correct and its test is thorough, including the negative controls, and it is self-described as temporary. Track its removal oncemaindeclares the field, and consider a profile-local channel for theputslice so the next maintenance counter does not repeat the churn.- README bullet got long. "Off by default: readers cache only inside
enable(). Maintenance calls (invalidateRemote,delete) always act." reads better than the semicolon one-liner. - Signature nit.
LocalCache.deletetakes a URN string whileputandgettake aDialCacheKey.
Verified
- Ordering and atomicity in every port. Validate, then capability check, then count, then remote, then local and memo with no await between them. Remote failure leaves both memory stores intact. Unsupported adapters fail before any mutation and before the counter. The TypeScript gate test, the Go remote-callback assertion, the Rust
BrokenRemovalstore, and the Python remote assertion each pin this. - Rust locking.
deletetakes the cache state lock, then the owner lock. Every other acquisition site inengine.rs,execution.rs, andscope.rstakes one or the other, never both, so there is no reverse order. Displaced entries drop outside both locks per the existing convention. - Go race safety.
owner.liveis read underc.mu, the mutex the scope'sdonewrites under. - Adapters. One keyed
DEL, slot-primary routing in node-redis, GLIDE, go-redis, and the Rust connection, reply validation accepting only integers 0 and 1, no retry. Cluster integration confirms the same-slot watermark survives. - Formal. Four kernel transitions, eight invariants (seven new), fourteen named regressions, and eight fault probes covering all seven new invariants. Seven native mutants each for TypeScript and Go, matched by seven Rust mutants, spanning C61 to C63 including "unsupported reports success" and "detach flight". The witness classifier reads only public inputs and observations.
- Interop. Writer, deleter, and reader permutations across all languages for tracked and untracked identities, with the watermark asserted unchanged.
- Docs match the implementation everywhere checked, including
DELin the required-command list and the Rust breaking-change notes forLocalStore::remove,Event, andMetricKind.
Merge gate
Draft by design until the full formal workflow and the pending differential shards finish. Every proposed decision in #218 was resolved the way the issue recommended; tick that checklist on merge.
🤖 Generated with Claude Code
lan17
left a comment
There was a problem hiding this comment.
Re-review of 1a9d212 (the two fix commits since 3414d5f). Both address the earlier review correctly; nothing new to fix.
Blocking item resolved. The Python Key branch is gone rather than validated. A Key instance now takes the scalar path, where scalar_string raises a deterministic TypeError before the capability check, the counter, or any store change. This is not accidental: str(Key) would have produced a nested URN, but the normalizer never falls back to str for unknown objects. The new test covers the matching case and three mismatched kwargs and asserts no adapter call, no event, and intact local and memo entries. The Python language guide documents the rejection.
Go change is a real fix beyond the review. go-redis's typed Del helper coerces numeric strings into integers, so a proxy answering +1 or bulk "1" would have passed validation. Switching to the raw Do call preserves the reply type, matching how the adapter already dispatches SET, so cluster routing and ACL behavior are unchanged in kind. The new test drives go-redis's real RESP decoder over a pipe and rejects status strings, bulk strings, big integers, doubles, booleans, nulls, and out-of-range integers. The Go package passes locally under the race detector.
Nits taken. README bullet is the two-sentence form. LocalCache.delete takes a DialCacheKey, and the four mutation entries whose anchor text changed were updated with it, so the fault campaign still compiles. Evidence catalogs were refreshed along the usual hash chain.
Unchanged, as expected. The differential bridge stays; its removal after merge remains the open follow-up.
CI at this head. TypeScript, Python 3.11 and 3.14, Go, Rust, wire, and docs are green. Quint and the four differential shards were still pending when I checked, and the full formal workflow gates the draft flip. The three review threads the fixes addressed can be resolved.
🤖 Generated with Claude Code
- Raise the full formal check-models budget to 240 minutes: main needs 2 h 28 min and the final-head run was cancelled at exactly 150 minutes. - Export CacheUseCaseOptions, which docs/api.md already names, and pin it in the packed-package consumer check. - Build Python identities through one helper shared by readers and delete, so a mapping without an id raises TypeError before any effect; test it. - Restore ruff import order in four Python files and move the typing fixture's imports to the top of the file. - Match the TypeScript README's "Off by default" bullet to the root README. - Note that an acquired snapshot re-warms the request memo and local store. - Cite #222 for removing the differential reference bridge, document ValidateRedisDelReply, and merge the errors import in redis-cache.ts. - Refresh the source-audit and go-parity ledger pins for the changed files.
Closes #218.
Adds exact-key
delete/Deletein TypeScript, Go, Rust, and Python. Deletion validates the identity and optional remote capability, removes the remote value first, then removes this instance's local entry and the live request memo—even insidedisable(). Missing entries succeed; unsupported adapters and remote errors preserve local state. Identity includes arguments and tracking mode. Python deletion accepts a scalar ID or an{id, args}mapping; prebuiltKeyobjects reject before dispatch, metrics, or cache changes so they cannot override the required identity arguments.Bundled adapters issue one primary-routed
DELand accept only integer 0/1 replies. Go tests exercise the real Redis response decoder so numeric strings and big integers cannot be mistaken for successful deletion. Deletion has its own metrics and errors, executable documentation examples, and a shared Quint profile with generated histories, independent properties, and fault probes. Watermarks, sibling keys, other instances, and in-flight work remain unchanged. An earlier load can still publish after deletion.Validation:
1a9d212: TypeScript, Go, Rust, Python 3.11 and 3.14, and cross-language Redis integration. Documentation, CodeQL, and formal smoke also pass on this head.make check-go,make integration-go, andmake audit. The decoder regression fails against the original implementation and passes with the fix.make check-python(2,153 tests),make check-ts(3,553 tests, typecheck, build and packed-package checks),make docs, andmake audit. Four new Python input-boundary cases fail before the fix and pass afterward. The affected TypeScript mutations M65/M67/M68/M69 retain every required detection against the generated corpus; this targeted measurement is partial evidence, not the complete mutation gate.3414d5fagainst the same generated corpus: 7,902 required cases per port, including 6,164 histories, 244 scenarios, 1,477 protocol cases, and 17 witness checks. Go ran with race detection. The deletion profile's 128 sampled histories and 14 named regressions passed in every port. Full corpus generation, committed fixture recomputation, and required shared witnesses also passed. The follow-up fixes leave the shared models and corpus unchanged.9b5248aafter the review fixups. The earlier final-head run 36347923680 was cancelled whencheck-modelsreached its 150-minute budget; main's own runs need about 2 h 28 min, so this PR raises that budget to 240 minutes. The bidirectional comparison against main and the Quint lane passed on1a9d212and rerun on the new head. Merge waits for the dispatched full run to finish green. Earlier-head results are not presented as final-head full validation.Follow-up after merge (#222): remove the temporary
referenceDescriptorprojection once supported differential reference revisions include thedeletionsfield. The current merge-base still requires it. Keep profile-local observation channels in mind for the laterputslice.Rust compatibility: custom
LocalStoreimplementations must addremove, and exhaustive observer/error matches must handle the new variants. Existing remote adapters retain their previous required methods; deletion is an optional capability in every port.— Levicus 🤖