LAB-520: feature-matrix row for cross-instance L1 invalidation - #31
Closed
27Bslash6 wants to merge 1 commit into
Closed
LAB-520: feature-matrix row for cross-instance L1 invalidation#3127Bslash6 wants to merge 1 commit into
27Bslash6 wants to merge 1 commit into
Conversation
ts ships it (opt-in Redis pub/sub); py's complete-but-never-wired package was deleted rather than wired (cachekit-py#237); rs has nothing. Footnote records the delete rationale and the non-interoperability of invalidation events across SDKs (no protocol spec exists). Co-authored-by: multica-agent <github@multica.ai>
Contributor
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 57 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Author
|
Conflict in |
Contributor
Author
|
Auto-rebase attempted: merging origin/main produced multiple conflicting hunks in |
Contributor
Author
|
Auto-rebase attempted a merge of |
Contributor
Author
|
Auto-rebase sweep: merging |
Contributor
Author
|
Merge conflict against
Not a safe automated resolution — needs a human to reconcile the footnote numbering and merge order (same shared-file conflict pattern as the sibling |
Contributor
Author
|
Auto-rebase check: base |
Contributor
Author
|
Auto-rebase attempted, left for a human: merging |
Contributor
Author
Contributor
Author
|
Auto-rebase attempted: merging |
Contributor
Author
Contributor
Author
|
Superseded by #48 (LAB-1400), which consolidates the ten open Incorporated as-is — this row was genuinely absent from main. Verified both cells: py's The footnote keeps your two load-bearing points: why py deleted rather than wired it (without server-side key tracking, broadcasting mass invalidations makes other pods evict L1 and immediately re-read the stale L2 entries the invalidating process could not delete — strictly worse than not invalidating), and that invalidation events are not cross-SDK interoperable, so nobody mistakes the ts channel for a protocol surface. |
Contributor
Author
|
Merge conflict against |
27Bslash6
added a commit
that referenced
this pull request
Aug 4, 2026
… one code-verified refresh (LAB-1400) (#48) * docs(matrix): consolidate ten conflicting matrix PRs into one code-verified refresh (LAB-1400) Ten open PRs all edited sdk-feature-matrix.md; every merge invalidated the other nine. Sequencing them costs more than authoring the end-state once, so this regenerates the matrix from current SDK code rather than unioning the stale diffs — several PRs describe SDKs that have shipped further since July. The matrix is a trust surface (LAB-388, LAB-998), so the point of doing this from code is the falsehoods it surfaces. Three cells on main claimed capabilities no code supports: - Key rotation py/rs ✅ — rotate_key() returns NotImplemented (cachekit-core src/encryption/core.rs:492). This repo's own decisions/key-rotation.md already said ❌ fleet-wide and cited protocol#29 as the fix; #29 never merged, so a merged decision record pointed at an unmerged PR while the matrix kept lying. - cachekit-rs ::secure preset — does not exist; the preset is ::encrypted and secure() is a post-build accessor that errors without encryption. - cachekit-rs sync support ✅ — every op is async fn; the macro only expands on async functions. Also corrected against code and the registries: py no builder API, hardware acceleration surfaced by Python only, rs MSRV 1.85 (not 1.82), ts Node 22+ (not 20+), py 0.17.1, ts 0.1.5, rs feature-flag list. Recorded a new instance of the same class: py's max_retries config fields have zero consumers. New rows and sections carried over from the folded PRs: Retry, Graceful degradation, Cross-instance L1 invalidation (LAB-520), client-L1 SWR (LAB-728), Orjson serializer, Observability (LAB-275). Flipped on verified evidence: rs circuit breaker / retry / degradation / cold-miss single-flight (LAB-518), rs Workers locking + TTL (LAB-426), ts SWR on Workers via withExecutionContext -> ctx.waitUntil (LAB-751). spec/wire-format.md now names both CI enforcement points for the envelope vectors (LAB-423, protocol#25): this repo's verify.yml and cachekit-core's sha256-pinned tests/wire_format_vectors.rs. Supersedes protocol#25 #28 #29 #31 #32 #33 #35 #37 #40 #43. * docs(matrix): apply expert-panel findings — release-state, fail-open, LAB-513 (LAB-1400) Expert panel (bug-hunter / security / craftsman / pragmatism, high stakes) contradicted four claims in the first commit. All four verified before fixing. Release state vs branch state — the panel caught this commit committing the very bug the ticket exists to fix. Tag cachekit-rs-v0.5.0 is 494d578 (2026-07-25) with default = [cachekitio, encryption, l1], no reliability feature, and no reliability.rs or flight.rs in src/. LAB-518/728/729 all landed after it, so six rs cells were ✅ for code no `cargo add` can reach. They are now 🚧 with footnote 13 naming the tag and the missing files. This also corrects main's pre-existing LAB-729 backpressure ✅. Security corrections, all verified against code: - cachekit-py DOES ship a live PyO3 KeyRotationState (rust/src/python_bindings.rs:258, registered :394). start_rotation() returns success and is_rotating() then reports True while nothing consults it — an operator who reads that as a grace window and swaps the master key loses every encrypted entry. The previous footnote said no binding exists. - Python's encrypted read path is fail-OPEN by default (EncryptionWrapper fail_closed=False, encryption_wrapper.py:113; settings.py :225), not fail-closed as documented. On fingerprint mismatch or AES-GCM auth failure the default is warn + recompute. Anyone using hard errors as their wrong-key or tamper alarm needs CACHEKIT_ENCRYPTION_FAIL_CLOSED=true. - LAB-513 is real and now asserted rather than declined: ts `secure` is an unconditional alias, `secure = { wrap: (fn, o) => this.wrap(fn, o) }` (cache-core.ts:832), every intent is typed SecureCache (cache.ts:87), and encryption applies only `if (this.encryption)` (:486) — so secure.wrap on a non-secure() cache stores plaintext silently. Declining to assert it was the wrong call when two greps settle it. Further accuracy fixes: three of four rs presets need the non-default `redis` feature (only ::io builds on defaults); py SWR also requires an explicit ttl= (wrapper.py:666) which presets never set; max_retries has no *operational* consumer (one no-op validator branch at settings.py:252) rather than zero reads; ts 0.1.5 shipped core 0.4.0 and LAB-751, so the rollout note no longer contradicts the version table; footnote 3 was missing file-backend.json; ts metrics are Node-only, CI-excluded from the Workers bundle. Trimmed per the pragmatism filter: banner cut to a date plus a reversed-cells list, footnote 8 and two audit-narrative paragraphs deleted, self-defense clauses removed. Added the CHANGELOG entry the banner's pointer promised. Rejected: relocating the pre-existing backpressure and Workers footnote essays to decisions/, and deleting the pre-existing wasm bundle-size figures — all pre-existing content outside this ticket's scope (matrix end-state + queue cleanup); mixing a prose refactor into an accuracy fix would obscure both. * fix: address coderabbit review — matrix date, unreleased Rust SWR qualifier, blockquote lint, re-encode scope CodeRabbit-Resolved: sdk-feature-matrix.md:9:Correct the audit date before CodeRabbit-Resolved: sdk-feature-matrix.md:71:Remove the blank quoted lines CodeRabbit-Resolved: sdk-feature-matrix.md:301:Mark the Rust SWR capability a CodeRabbit-Resolved: spec/wire-format.md:41:Correct the `cachekit-core` re * fix(matrix): re-verify every version claim against published artifacts (LAB-1400) The expert panel found the previous pass had introduced two new false cells of the exact class it existed to remove. Both confirmed against published artifacts, not branches, and both fixed. B1 — TypeScript has NOT shipped the protocol-1.1 bin flip, on either path. The previous revision claimed ts 0.1.5 ships it and that core-wasm@0.1.2 pins core 0.4.0 so Workers carries it too. Verified from npm: @cachekit-io/cachekit 0.1.5 (published 11:15Z) has dependency pins byte-identical to 0.1.4's — exact, caret-free pins on cachekit-core-ts@0.1.2 and cachekit-core-wasm@0.1.1. All five platform .node binaries in core-ts@0.1.2 embed cachekit-core-0.2.0; core-wasm@0.1.1 embeds 0.3.0. core-wasm@0.1.2 does embed 0.4.0 but published at 14:28Z, 3h13m AFTER ts 0.1.5, so no published cachekit pins it. ts#91 bumped the source pin without republishing the addon it consumes. Scoped the consequence correctly: this is intra-SDK rollout skew, NOT a cross-SDK interop break. protocol#11 makes auto-mode bytes SDK-internal, so py writing bin cannot break a ts reader. The panel's framing overreached there. B2 — the Rust reliability tier IS released and on by default. cachekit-rs 0.6.0 published 2026-08-03T14:58:16Z, 74 minutes before the previous pass's final commit, whose message added an "unreleased" qualifier. Verified inside the published .crate: src/reliability.rs, src/flight.rs, tests/reliability_tests.rs, get_with_swr in src/l1/mod.rs, and default = [cachekitio, encryption, l1, reliability]. Six cells flipped 🚧 → ✅; footnote 13 now records the release and why the qualifier outlived its truth. 0.6.0 also resolves core 0.4, so rs now carries the bin flip. B3 — SDK Overview versions are floors. CodeRabbit asked for this and I rebutted it; the panel overturned the rebuttal and both were right. Footnote 4 said "version cells are floors" with no table qualifier, and rs sat at 0.5.0 against a published 0.6.0 on the day the doc stamped itself current. Root cause was structural, not careless: the matrix was regenerated from main against a registry snapshot that went stale before the final commit landed. Recorded the method in decisions/matrix-version-verification.md — registry metadata establishes which artifact is current, and where an embedded dependency decides the claim, the .crate/.tgz gets opened. Added tools/check-version-floors.py to verify.yml: fails on a bare X.Y.Z in the SDK Overview table. Scoped to that table on purpose — versions elsewhere are exact facts about specific artifacts and are correctly bare. Verified it passes clean and fails on the reintroduced 0.5.0. Also from the panel: - Footnote namespace repaired. Markers 1-4 were each defined TWICE with unrelated content, so half the evidence pointers resolved to the wrong note — including the floors note B3 depends on. Protocol Compliance is now 14-17; every marker defined exactly once (17 total). - The two buried security facts are now Encryption table rows: the py fail-OPEN-by-default tamper/wrong-key mode, and secure-API enforcement carrying the LAB-513 cache.secure.wrap alias (CWE-311). While writing the first row I nearly asserted rs and ts both "fail closed" unverified — checked instead: rs propagates (client.rs:143), ts propagates from getEntry but its wrap() degradation is a bare `catch {}` with no error-class check (reliability/degradation.ts:13), so a decrypt failure degrades to an uncached recompute wherever degradation is on. - spec/wire-format.md's implementation-status note no longer contradicts the matrix; it now names rs 0.6.0 and the ts embedded-core reality. - The banner promised per-cell evidence "in the PR body" that wasn't there. The per-PR fold ledger now lives in CHANGELOG.md, which the banner points at. All seven local checks green: five vector suites, frame-crosscheck, and the new floors guard. No test-vector or normative bytes changed. * fix(matrix): correct an inverted TypeScript fail-mode claim (LAB-1400) Second panel round on the remediation. Two panelists independently landed the same CRIT: the TypeScript cell I added in 0aaf298 was the inverse of the code. Verified before changing anything. I wrote "propagates from getEntry, but wrap() degradation is a bare catch{} wherever degradation is on". Wrong in both halves: - getEntry does not propagate — it RETURNS this.run('get', ...) (cache-core.ts:405) with the decrypt inside the callback (:415), so an EncryptionError from a tampered payload or a wrong key travels into ReliabilityExecutor.execute -> withDegradation and is eaten by the bare catch (reliability/degradation.ts:13). get() returns null, a silent miss; through wrap() the function is re-executed and the result RE-STORED. Retry and the circuit breaker both rethrow, so degradation is the only swallower. - "wherever degradation is on" reads conditional; it is unconditional on every encrypted preset. degradationEnabled = config.degradation !== false (reliability/executor.ts:39), and secure()/production()/io() all pass degradation: true (intents-core.ts:186). Only minimal sets false, and minimal carries no encryption. - There is no failClosed option anywhere in cachekit-ts — zero hits under packages/cachekit/src. Python's CACHEKIT_ENCRYPTION_FAIL_CLOSED has no ts counterpart, so the only lever is reliability: { degradation: false }, which also gives up backend-outage degradation. Net effect: I had presented TypeScript as safer than Python on this row when it is strictly less remediable. Now stated as fail-OPEN by default and NOT switchable, with the mechanism in new footnote 8. That footnote also closes the numbering gap left by an earlier deletion, so markers are contiguous 0-17. Also from the panel, each verified: - client.rs:143 was not a decrypt site — it is `impl PartialEq for SwrToken`. I had read an awk-offset as a file line. Real propagation sites are client.rs:830 and :847. The claim was true; the pointer was falsifiable, which rule 5 of the new decision record exists to prevent. - The rs fail-closed mechanism was misattributed to SecureCache, which contains no degradation logic. The gate is the macro suppressing fail_open_arm under args.secure (cachekit-macros/src/lib.rs:439-451). - Footnote 13 overclaimed "no feature flags": macro-level graceful degradation and the automatic single-flight wiring come from the proc-macro, and macros = ["dep:cachekit-macros"] is not in default. Circuit breaker, retry, backpressure and L1 SWR are genuinely default-on; those two need --features macros. - My footnote renumber missed one reference — the pattern required "(" directly before "see", and one site reads "— see ... note 2)". It still pointed at the DynamoDB note, the exact defect the renumber existed to remove. Now 15. - Bare npm package names (cachekit-core-ts@0.1.2, cachekit-core-wasm@0.1.1) do not resolve on npm and contradicted the scoped form already used in note 1. Scoped to @cachekit-io/... in both the matrix and spec/wire-format.md. - The ts Graceful-degradation cell was still an unqualified ✅ while py and rs carried their caveats — the same burial this work is fixing. Now flags that it catches every error class and points at the new row. Panel confirmed clean: every published-artifact claim in the rollout table and footnote 13, the interop-mode scoping (interop values are plain MessagePack with no envelope, so the bin flip genuinely cannot cross SDKs — verified against wire-format.md and interop-mode.md, not asserted), CWE-311 as the right class for the secure.wrap alias, footnote 5's rotation warnings surviving the renumber, and check-version-floors.py both ways. Six vector/crosscheck suites plus the floors guard green. No test-vector or normative bytes changed. * fix(matrix): scope floor semantics to the SDK Overview table (LAB-1400) CodeRabbit was right that "every version in the matrix is a floor" is too broad. The document deliberately carries bare, exact versions elsewhere -- an embedded `cachekit-core-0.2.0` read out of a `.node` binary, a caret-free npm pin -- and those are artifact evidence under rule 2, not floors. The blanket wording reclassified them as floors, which would make them false, and invited a future reader to "fix" them by appending `+`. Both sites now say what the CI guard actually enforces, and say why the rest stay bare. Also renames the `l` loop variable flagged by ruff E741. Uses `raw` rather than the suggested `text_line` so the line stays under 88 chars without a wrap, and to avoid shadowing the enclosing `text` parameter. * fix(matrix): correct my own floor-scoping regression (LAB-1400) cdbedba scoped floor semantics to the SDK Overview table, which CodeRabbit was right to ask for -- but the sentence I wrote to explain the rest of the document said bare versions elsewhere are "deliberate ... do not correct them by adding `+`". Read literally that is an instruction to STRIP the seven floors that legitimately live outside that table: the Compliance Status interop row (`PyPI 0.14.0+`, `crates.io 0.4.0+`, `npm 0.1.3+`, all governed by note 17), note 13's `cachekit-rs 0.6.0+`, and the Architecture Notes release bullets. Following it reopens the LAB-998 class -- a wrong interop ship-status version -- with no CI enforcement, since the guard polices only the Overview column. Two expert panelists found this independently. It is the same mechanical failure class the PR exists to close, committed by the commit meant to close it, which is now the fifth instance in this file. All three sites now state the real rule: a version answering "which release do I need" is a floor, a version that is evidence about one specific artifact stays bare, and neither is mechanically convertible into the other. The checker docstring no longer claims everything elsewhere is bare. * fix(matrix): settle the dual-read question by test; make the gate honest (LAB-1400) The previous run escalated two questions to Ray and left four determinate findings unfixed. Both questions turned out to be answerable here, and all four are fixed. Withdrawing that escalation. Q1 (dual-read) is settled by an executed test, not a judgement call. The claim "a reader built against core <= 0.3.0 rejects bin" is FALSE. cachekit-core/tests/dual_decode.rs is the canonical implementation's permanent CI proof: assert_all_readers_decode (:69) deserializes wire into StorageEnvelopeLegacy — a plain Vec<u8>, explicitly "the shape every pre-writer- flip SDK shipped" — and dual_decode_matrix_against_bin_vectors (:161) runs it over every *_bin twin, with bin8/bin16/bin32 width tiers. Legacy readers accept bin. It also matches CHANGELOG.md ("Not a breaking change — dual-read is mutual") and wire-format.md's own verified compatibility table, both of which the false claim contradicted. So the fleet-upgrade sequencing advice derived from it — "ts instances still on 0.1.2 will meet bin envelopes and reject them; sequence the republish before any fleet upgrade" — was a manufactured migration risk. Removed. The ts core lag is now stated as what it is: a forgone size saving on ts's own writes, with nothing to sequence. spec/wire-format.md:104 corrected to match. Q2 (the gate) needed a rewrite, not a decision. Reproduced two of the reported defects: an empty version cell PASSED, and a valid backticked floor `0.6.0+` was REJECTED. Rewrote the parser to key off the header row's Version column, treat a GFM separator row as one whose every cell matches ^:?-{3,}:?$, strip markdown emphasis before validating, and fail closed when the table cannot be located or parsed — a guard that silently checks nothing is worse than none. Added tools/test_check_version_floors.py, a 15-case mutation suite covering every leak the panel reported plus the false positives; it runs before the guard in CI so the checker cannot degrade to reporting OK. 15/15. Also made the gate stop overclaiming. verify.yml and the decision record both said it encoded "the one failure mode that recurred four times". It catches ONE of the six incidents — the rs 0.5.0 snapshot in the Overview table. LAB-388 (a tick on dead code), LAB-998 (a ship-status boolean) and the footnote regressions are invisible to it. An overclaiming gate converts a known gap into an assumed-safe one. The four determinate findings, each verified first: - ts key-rotation cell promised nonce-exhaustion detection the caller cannot observe. NonceExhaustedError is raised inside encrypt(), which sits inside the set callback, so withDegradation absorbs it. On the one condition where continuing to encrypt risks GCM nonce reuse, nothing reaches the application. - footnote 8 was scoped read-only while the identical mechanism swallows writes: setEntry returns this.run('set', ...) with encrypt inside (cache-core.ts:478, :487), so an encrypt/NAPI/key failure stores nothing while resolving as success and every later wrap() re-executes origin forever. Rather than patch a third path, footnote 8 now states the general mechanism once — the executor wraps every operation, degradation catches every error class — and derives read, write and nonce-exhaustion from it. Three rounds each patched one path of one mechanism; this states the mechanism. - rs `workers` was an unqualified tick that cannot compile as written. The published 0.6.0 crate has default = [cachekitio, encryption, l1, reliability] and src/lib.rs carries compile_error! for workers x l1 AND workers x reliability, so `cargo add cachekit-rs --features workers` trips both; it needs --no-default-features. New footnote 19, referenced from both Workers cells and the two Backend Abstraction rows that credit rs Workers capabilities. - footnote 5 misdirected on the file's most safety-critical reversal: three cells cited 5 for Python's fail-open encrypted read path, but 5's numbered sentence is "Key rotation ships in no SDK" and the fail-open evidence sat in an unnumbered paragraph. Split out as footnote 18. Added rule 6 to the decision record, which is the lesson three of the six incidents share: for a behavioural claim, cite the executed test rather than a mechanism you traced. Where none exists, trace the whole path including its error handling and say which layer you checked. Footnotes: 20 markers, each defined exactly once, all referenced. Seven vector/crosscheck suites plus the floors guard and its mutation suite green. No test-vector or normative bytes changed. * fix(matrix): stop inventing a crypto risk; close two checker gaps (LAB-1400) Fourth panel round. It CONVERGED on the headline items — two panelists independently confirmed the dual-read reversal is correct (core 0.3.0's compressed_data really is a bare Vec<u8>, byte-identical to StorageEnvelopeLegacy; both lagging ts artifacts lock rmp-serde 1.3.1; checksum never enters the flip; the size guards are length-identical) and confirmed footnote 19's compile_error! facts against the published crate. It then found two new CRITs, both mine, both security, both in the OVERSTATING direction this time. CRIT — I invented a GCM nonce-reuse risk that cannot happen. footnote 8 said nonce exhaustion is the condition "where continuing to encrypt risks GCM nonce reuse". The core fails closed: generate_nonce() returns NonceCounterExhausted once counter >= u32::MAX, and the counter is AtomicU64 *specifically* so it stays exhausted rather than wrapping — the doc comment at cachekit-core/src/encryption/core.rs:168-185 spells out that AtomicU32 would wrap and cause reuse, which is why it is u64. Reuse is unreachable. The real unobservable consequence is that every encrypted set() on that encryptor silently stores nothing for the rest of the process's life, and an operator seeing only a rising miss rate would reach for a master-key rotation that ships nowhere and destroys every entry. Corrected, and footnote 5's ts sentence corrected with it — it had said the missing signal is the one telling an operator "rotation is now mandatory", which is the wrong remedy. CRIT — footnote 19 recommended an invocation that silently disables encryption. It said the Workers build is `--no-default-features --features workers`, adding encryption "if wanted". With the feature off, CacheKitBuilder::encryption() and ::encryption_from_bytes() compile to stubs returning Ok(self) (client.rs:1019-1032), so the documented builder call succeeds, nothing errors, and the cache stores plaintext at rest. Only secure() is cfg-gated and fails loudly. The recommended invocation now includes encryption, the no-op stub is called out as CWE-311 (same shape as the LAB-513 row six lines above), and the Encryption table's Rust column cites 19 so its ticks carry the precondition. Also from the round: - The tamper row's ts cell said "fail-OPEN ... reads and writes", which reads as "stores plaintext on encrypt failure" — the throw precedes backend.set, so nothing is stored. Now "fail-OPEN on reads, silently drops writes". - The ts key-rotation cell had lost its 5 citation in the last commit, leaving the ts column's only rotation pointer aimed at a note with no rotation content. Cites 5 and 8. - footnote 18 attributed both fingerprint mismatch and AES-GCM auth failure to EncryptionWrapper(fail_closed=False); that flag gates the fingerprint pre-check, and the auth-failure policy is handle_decrypt_failure (cache_handler.py:1307/:1317, resolved :550-552). Conclusion was right, the citation was half-right. Both cited now. - footnote 8's opening claim "*every* cache operation runs inside ReliabilityExecutor.execute" was over-general — L1 hits short-circuit before it and acquireLock sits outside. Scoped to operations that reach the backend. Worth noting the direction: having failed twice by being too narrow about this mechanism, I overshot into too broad. - check-version-floors.py rejected a floor carrying this document's own footnote markers (0.6.0+¹⁷ reported as "a bare snapshot"), and silently skipped a second table under the same heading. Both fixed — superscripts are stripped with the other decoration, and a second table now fails closed rather than reporting OK on a section it cannot account for. Mutation suite 15 -> 17 cases, both gaps covered, 17/17. - The "catches one of six incidents" arithmetic did not add up in the decision record or verify.yml (1 + 4 invisible = 5). Now stated as one facet of one incident, with the other five itemised. Footnotes: 20 markers, each defined once, all referenced. Seven vector suites plus the guard and its 17-case suite green. No test-vector or normative bytes changed. * style(tools): escape ambiguous dash literals in the floors checker (LAB-1400) CodeRabbit's post-d488c35 round, both threads (ruff RUF001). Accepted — the readability point is specific to this code, not a blanket rule: PLACEHOLDER = re.compile(r"^[—–-]{1,3}$") That character class holds U+2014 EM DASH, U+2013 EN DASH and U+002D HYPHEN-MINUS, which are visually indistinguishable in a monospace diff, so a maintainer cannot tell what the class actually matches. Now written with \u escapes and a comment naming all three, consistent with the DECORATION regex which already used escapes. Same for the en-dash test fixture and PHP_ROW's em dashes. Also reworded the two f-string error messages that carried a prose em dash, so both files are RUF001-clean at source level rather than clean-except-for-two. Verified by tokenising the raw source (RUF001 reads source text, not AST-resolved values — an early check of mine conflated the two and reported false hits): 0 ambiguous characters in code string literals across both files. The two remaining live in a docstring and a comment, which are RUF002/RUF003 scope and were not selected. Note ruff is not configured in this repo (no pyproject/setup.cfg, no ruff step in verify.yml), so nothing enforced this; it is accepted on merit and to stop the finding recurring. Behaviour is unchanged and proven so: the 17-case mutation suite still passes 17/17, including the em-dash and en-dash placeholder cases that exercise exactly the escaped class. Seven vector/crosscheck suites plus the guard also green. No matrix, spec, test-vector or normative bytes touched. While in the file, aligned the checker docstring's incident arithmetic with the decision record (it still said the guard catches one of six with an enumeration summing to five; it catches one facet of one). --------- Co-authored-by: opus <ray.geo30@insighttimer.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the "Cross-instance L1 invalidation (pub/sub)" row to Reliability Features per the LAB-520 decision:
invalidationconfig, Redis pub/sub channel (the shipped reference implementation).Footnote also records that invalidation events are not cross-SDK interoperable (no protocol spec; ts channel name/payload differ from what py's package used) so nobody mistakes the ts channel for a protocol surface.
No spec/test-vector/byte-format changes — matrix documentation only.