Skip to content

feat: add exact-key deletion across all ports - #219

Open
lan17 wants to merge 8 commits into
mainfrom
codex/exact-key-delete
Open

lan17 wants to merge 8 commits into
mainfrom
codex/exact-key-delete

Conversation

@lan17

@lan17 lan17 commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

Closes #218.

Adds exact-key delete / Delete in 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 inside disable(). 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; prebuilt Key objects reject before dispatch, metrics, or cache changes so they cannot override the required identity arguments.

Bundled adapters issue one primary-routed DEL and 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:

  • Regular CI passes on 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.
  • The Go decoder fix passes targeted deletion tests with race detection, full make check-go, make integration-go, and make audit. The decoder regression fails against the original implementation and passes with the fix.
  • The final follow-up passes make check-python (2,153 tests), make check-ts (3,553 tests, typecheck, build and packed-package checks), make docs, and make 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.
  • Real Redis, Valkey, and Cluster deletion tests passed before the input-boundary follow-up. Cross-language integration covers 396 cases per server phase, including 96 exact-delete writer/deleter/reader combinations across standalone and Cluster. CI also executes the GLIDE Cluster checks that the existing Docker Desktop harness skips locally.
  • Before the Go decoder fix, all four complete native replay and completion gates passed at 3414d5f against 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.
  • Full formal verification was dispatched for the final head 9b5248a after the review fixups. The earlier final-head run 36347923680 was cancelled when check-models reached 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 on 1a9d212 and 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 referenceDescriptor projection once supported differential reference revisions include the deletions field. The current merge-base still requires it. Keep profile-local observation channels in mind for the later put slice.

Rust compatibility: custom LocalStore implementations must add remove, 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 🤖

@codecov-commenter

codecov-commenter commented Sep 27, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.11401% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.05%. Comparing base (d470b2f) to head (9b5248a).

Files with missing lines Patch % Lines
python/dialcache/cache.py 87.87% 4 Missing ⚠️
rust/src/engine.rs 94.87% 4 Missing ⚠️
rust/src/error.rs 25.00% 3 Missing ⚠️
rust/src/remote.rs 50.00% 3 Missing ⚠️
typescript/src/context.ts 75.00% 0 Missing and 1 partial ⚠️
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     
Flag Coverage Δ
go 91.51% <100.00%> (+0.39%) ⬆️
python 92.17% <91.66%> (+0.09%) ⬆️
rust 94.72% <94.47%> (+0.43%) ⬆️
typescript 96.57% <97.05%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lan17 lan17 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  • deletions joined the shared observation record. That one change in conformance-observations.qnt is why twenty-plus smoke traces and witness fixtures churned and why differential.mjs needed referenceDescriptor to project a zero baseline into corpora generated from origin/main. The bridge is correct and its test is thorough, including the negative controls, and it is self-described as temporary. Track its removal once main declares the field, and consider a profile-local channel for the put slice 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.delete takes a URN string while put and get take a DialCacheKey.

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 BrokenRemoval store, and the Python remote assertion each pin this.
  • Rust locking. delete takes the cache state lock, then the owner lock. Every other acquisition site in engine.rs, execution.rs, and scope.rs takes 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.live is read under c.mu, the mutex the scope's done writes 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 DEL in the required-command list and the Rust breaking-change notes for LocalStore::remove, Event, and MetricKind.

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

Comment thread python/dialcache/cache.py Outdated
Comment thread formal/differential.mjs
Comment thread README.md Outdated
Comment thread typescript/src/internal/local-cache.ts Outdated

@lan17 lan17 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add exact-key delete for one cached result (child of #217)

2 participants