diff --git a/changelog.d/7203-lru-cache-stale-compat-comment.md b/changelog.d/7203-lru-cache-stale-compat-comment.md new file mode 100644 index 0000000000..583f1da267 --- /dev/null +++ b/changelog.d/7203-lru-cache-stale-compat-comment.md @@ -0,0 +1,37 @@ +### Documentation + +- **Corrected the stale `lru-cache` compat comment in + `well_known_bindings.toml`.** The row justified its `compat = "partial"` + marker by claiming "the wrapper's store is numeric-value-oriented" — which + stopped being true when #7136 landed. That PR gave the binding real JS keys + and values, content-compared string keys, GC rooting for cached heap values, + `ttl`, and `updateAgeOnGet`, so the comment left readers with a wrong mental + model and an obvious-looking reason to flip the marker. + + Re-measured both directions against npm `lru-cache@11.5.2` on Node 26.5.0 + with the bundled binding linked in. A 20-assertion probe of real-world usage + (string keys, key-compare-by-content, miss, `has`, object values by identity, + mutation through the cached reference, overwrite, `size`, `delete`, `clear`, + eviction at `max`, `get`-promotes / `peek`-does-not, numeric values, `ttl`, + object survival across forced heap churn) prints **identical** output on both + — the binding is genuinely faithful for the surface it implements. + + The marker nonetheless stays `partial`, and the comment now says why. + `full` means an exhaustively audited drop-in for the package's *entire* + public API, and `is_faithful()` gates exactly one decision: whether Perry may + auto-prefer this wrapper over a user's installed `node_modules/lru-cache`. + The wrapper exports 8 entry points and omits `maxSize`/`sizeCalculation`, + `dispose`/`disposeAfter`, `fetch`/`forceFetch`, `allowStale`, per-call + `set`/`get` option objects, and the iterator surface. Two of those gaps fail + *silently* rather than loudly — measured, `forEach` visits nothing where npm + visits every entry, and a `dispose` callback is never invoked where npm + invokes it on eviction — so a `full` marker would let Perry swap a wrong + implementation in for a correct installed one with no diagnostic. This makes + `PERRY_REQUIRE_FAITHFUL_BINDINGS=1` refusing `lru-cache` the correct outcome, + not a false positive. + + Added `lru_cache_stays_partial_until_the_silent_gaps_are_closed` + (`crates/perry/src/commands/compile/well_known.rs`) so the next person + tempted to promote the marker meets the reasoning and the evidence together + instead of re-deriving them. No behavior change — the marker value is + unchanged. diff --git a/crates/perry/src/commands/compile/well_known.rs b/crates/perry/src/commands/compile/well_known.rs index 40df2747fa..8227b62aa5 100644 --- a/crates/perry/src/commands/compile/well_known.rs +++ b/crates/perry/src/commands/compile/well_known.rs @@ -396,6 +396,41 @@ mod tests { assert!(lookup_well_known("definitely-not-a-real-package").is_none()); } + /// `lru-cache` must stay `partial`, and for a reason that outlives the + /// comment in the toml. + /// + /// #7136 made the binding genuinely faithful for the surface it *does* + /// implement — JS-value keys and values, content-compared string keys, GC + /// rooting of cached values, `ttl`, `updateAgeOnGet` — which invites the + /// conclusion that the marker should be flipped. It should not. `full` + /// means an exhaustively audited drop-in for the package's ENTIRE public + /// API, and it licenses auto-preferring this wrapper over a user's + /// installed `node_modules/lru-cache`. Measured against npm + /// `lru-cache@11.5.2`, two of the wrapper's gaps fail SILENTLY rather + /// than loudly: `cache.forEach(...)` visits nothing where npm visits + /// every entry, and a `dispose` callback is never invoked where npm + /// invokes it on eviction. `maxSize`/`sizeCalculation`, `fetch`, + /// `allowStale`, per-call option objects, and the rest of the iterator + /// surface are absent too. + /// + /// Flipping this to `full` would therefore let Perry silently swap a + /// wrong implementation in for a correct installed one. Promote it only + /// once those surfaces exist and are conformance-tested — and update this + /// test with the evidence when you do. + #[test] + fn lru_cache_stays_partial_until_the_silent_gaps_are_closed() { + let binding = + lookup_well_known("lru-cache").expect("lru-cache must be a well-known binding"); + assert_eq!(binding.krate, "perry-ext-lru-cache"); + assert_eq!( + binding.compat, + BindingCompat::Partial, + "lru-cache's wrapper silently no-ops forEach/dispose — it cannot be \ + auto-preferred over an installed copy" + ); + assert!(!binding.is_faithful()); + } + #[test] fn compat_defaults_to_partial_when_absent() { let raw = r#" diff --git a/crates/perry/well_known_bindings.toml b/crates/perry/well_known_bindings.toml index 53437d9591..b0e6af947c 100644 --- a/crates/perry/well_known_bindings.toml +++ b/crates/perry/well_known_bindings.toml @@ -151,9 +151,38 @@ date = "2026-07-30" crate = "perry-ext-lru-cache" lib = "perry_ext_lru_cache" tracking = "#466" -# PARTIAL (explicit): the wrapper's store is numeric-value-oriented and -# does not faithfully reproduce lru-cache's full generic/option surface. -# Kept off the auto-prefer faithful path until the port is completed. +# PARTIAL (explicit) — but NOT for the reason this comment used to give. +# +# The old wording ("the wrapper's store is numeric-value-oriented") went +# stale when #7136 landed: the store holds real JS keys and values, string +# keys hash and compare by CONTENT, cached heap values are registered GC +# roots (marked and rewritten across copying evacuation), and `ttl` / +# `updateAgeOnGet` follow npm's clock semantics. A 20-assertion probe of +# real-world usage — string keys, object values, object identity, mutation +# through the cached reference, overwrite, size, delete, clear, miss, +# has, eviction at max, get-promotes / peek-does-not, numeric values, ttl, +# and object survival across forced heap churn — prints IDENTICAL output +# under npm `lru-cache@11.5.2` on Node 26.5.0 and under this binding. +# +# It stays `partial` because `full` is a stronger claim than "faithful for +# what it implements": per docs/src/native-libraries/zero-config-and- +# faithfulness.md it means an exhaustively audited drop-in for the pinned +# package's ENTIRE public API, and it licenses auto-preferring this wrapper +# over a user's installed `node_modules/lru-cache`. This wrapper exports 8 +# entry points and does not cover `maxSize`/`sizeCalculation`, +# `dispose`/`disposeAfter`, `fetch`/`forceFetch`, `allowStale`, per-call +# `set`/`get` option objects, or the iterator / `forEach` / `entries` / +# `keys` / `values` surface (the ABI only carries `(key, value)`). +# +# Two of those gaps fail SILENTLY rather than loudly, which is precisely +# what the marker exists to prevent: measured against the same probe, +# `cache.forEach(...)` visits nothing (npm visits every entry) and a +# `dispose` callback is never invoked (npm invokes it on eviction). Both +# print a wrong answer instead of throwing, so a `full` marker here would +# let Perry silently substitute this wrapper for a correct installed copy. +# Promote to `full` only once those surfaces exist and are conformance- +# tested; until then `PERRY_REQUIRE_FAITHFUL_BINDINGS=1` refusing this +# binding is the correct outcome, not a false positive. compat = "partial" [bindings.lru-cache.upstream]