feat(metrics): classify cache misses by reason

[lan17]

Summary

Implements #145 (replaces the stale attempt in #130).

Every DialCache miss event now says why it missed. Until now, a miss storm during a future-buffer invalidation window was indistinguishable from a cold cache or an eviction problem: the miss counter only told you that a layer missed, never whether the value was truly absent or a stored frame was being rejected by an invalidation watermark. This PR adds one required, bounded reason to the existing miss event — no new metric instrument, no new Redis command, no extra round trip, and no change to serving, refill, invalidation, stale-recovery, or shadow behavior.

What changes in your metrics

miss events now carry reason alongside the existing cacheNamespace / useCase / keyType / layer labels, on every layer (request_local, local, remote, remote_shadow):

reason	Fires when	Typical cause
value_absent	The layer had no retrievable value at all: never populated, physically expired or evicted, Redis nil, or a tracked-MGET wrong-type member (which Redis reports as nil). Request-local and process-local misses are always this.	Cold keys, physical TTL expiry, eviction pressure.
expired	A complete, supported frame with a valid, non-future timestamp was present, but its logical age against the reader's clock reached the effective remote TTL F. Covers age >= M misses and F <= age < M frames retained as stale-on-error candidates, on both caller and shadow reads.	Ordinary TTL churn on hot keys; the routine steady-state miss reason whenever staleOnErrorMaxAgeSec is configured.
watermark_fenced	A complete, supported tracked frame with a positive safe-integer timestamp was rejected because its createdAtMs was at or below a valid observed invalidation watermark. Decided before deserialization.	Reads inside an active futureBufferMs invalidation window; writer/invalidator clock skew.
unclassified	The miss is real but attributable to none of the above: unrecognized custom-adapter results (including null), short/unsupported frames, zero-stamped tracked frames, malformed watermark metadata paired with a present frame, future or invalid timestamps, and deserialization failures.	Un-migrated custom adapters; protocol-edge states.

The operational win: during an invalidation window you can now separate fence churn from genuine absence at a glance; routine TTL churn (including the stale-on-error F..M window, where every read of a retained frame is a miss) is expired rather than noise in the other buckets; and because expiry no longer lands there, a sudden unclassified plateau genuinely points at protocol-edge states (skewed clocks, corrupt frames, deserialization failures, legacy adapters) instead of hiding inside the aggregate.

Deviation from #145: the issue specified a three-value union and named unclassified as the safe default for stale-on-error retained frames "unless a separately justified reason is added". Review found that with staleOnErrorMaxAgeSec configured every read of a retained F..M frame is a miss, so unclassified would have been the routine steady-state reason for exactly the use cases that enable stale-on-error, and the plateau signal above would not hold. expired is that separately justified reason; the union is four values and the cardinality note below is 4x accordingly.

Prometheus — dialcache_miss_counter keeps its name and gains reason as a fifth label:

# Fence churn by use case during an invalidation window
sum by (use_case) (rate(dialcache_miss_counter{layer="remote", reason="watermark_fenced"}[5m]))
# Pre-existing total-miss and hit-ratio queries: aggregate reason away
sum by (cache_namespace, use_case, key_type, layer) (rate(dialcache_miss_counter[5m]))

Datadog — dialcache.miss.count keeps its name and gains the same reason tag, e.g. sum:dialcache.miss.count{reason:watermark_fenced} by {use_case}. Expect up to 4× the miss-series cardinality (four bounded reasons; only remote and remote_shadow tuples can carry every reason, since request-local and local misses are always value_absent).

Migration

• Prometheus registries: the label set changed, so registering against a registry that already holds a 4-label dialcache_miss_counter (e.g. a not-yet-upgraded sidecar library) fails loudly at construction — upgrade producers together per registry. Nothing silently mislabels.
• Mixed-fleet rollouts: old processes emit reason-less series while new ones carry reason. Total-miss and miss/request-ratio queries must sum by (...) the shared labels (example above); reason-aware dashboards should group by reason explicitly.
• Custom metrics adapters:DialCacheMetricsAdapter.miss now receives MissMetricLabels (extends the unchanged CacheMetricLabels with required reason: CacheMissReason). Adapters whose miss parameter is typed as the broader CacheMetricLabels compile unchanged and may ignore the field; exact label snapshots, exhaustive Records over reasons, and adapters that reject or forward unknown fields must add it.
• Custom Redis adapters:RedisReadResult is now DecodedRedisFrame | RedisReadMiss { kind: "miss", reason, observedWatermarkMs? }. null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed; use decodeRedisReadResult (untracked) / decodeTrackedRedisReadResult (tracked) and the exported isRedisReadMiss guard. An un-migrated adapter still fails open at runtime (null or any unrecognized result is an unclassified miss with a normal refill) but loses reason precision and fenced-refill suppression until it returns the typed miss.

How classification works

Bundled decoders return one miss shape, RedisReadMiss { kind: "miss", reason, observedWatermarkMs? }, attaching observedWatermarkMs only when the same atomic tracked snapshot carried a valid numeric watermark. Cause and fence are deliberately independent: Redis nil is decisive evidence of absence, so an absent value reports value_absent while still carrying the observed watermark that lets PR 143's two-sample admission skip a known-fenced refill — only a complete supported frame actually rejected by the watermark reports watermark_fenced.

Core treats the adapter boundary as untrusted and normalizes every result once, at one choke point: kind: "miss" is the only discriminator; invalid or missing reasons become unclassified, a watermark_fenced claim without a valid tracked fence is demoted to unclassified, invalid fences (NaN, negatives, non-integers, untracked keys) are dropped, and null, undefined, or any other unrecognized value is an unclassified miss with a normal refill. Objects without kind: "miss" are frame candidates and go through the existing timestamp and serializer validation.

Decoded hits keep the existing { payload, createdAtMs } shape. Core-side rejections never acquire a refill fence: logical-age rejections emit expired, future timestamps and deserialization failures emit unclassified; conditional refill suppression remains exactly PR 143's adapter-level watermark-miss path, byte-compatible with main (differential-verified across the decode input matrix).

Breaking change

DialCacheMetricsAdapter.miss now receives MissMetricLabels, and first-party miss metrics require a reason label/tag. RedisReadResult is now DecodedRedisFrame | RedisReadMiss; null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed, and bundled Redis adapters return RedisReadMiss for every semantic miss.

Under the pre-1.0 release policy, this is a minor release.

Validation

• Node.js 22.22.0: corepack pnpm check
  • typecheck
  • 656 unit tests with coverage thresholds
  • ESM/CJS build and declaration generation
  • packed-package type and runtime checks
• Node.js 22.22.0: corepack pnpm test:integration
  • 143 passed
  • 2 expected local GLIDE Cluster skips because announced container IPs are not host-routable
• Review follow-up (fix(metrics): preserve existing refill and decoder behavior) removed the fence-carry scope creep and restored decoder/refill parity with main, differential-verified across the decode input matrix

BREAKING CHANGE: DialCacheMetricsAdapter.miss now receives MissMetricLabels and first-party miss metrics require a reason label/tag; RedisReadResult is now DecodedRedisFrame | RedisReadMiss ({ kind: "miss", reason, observedWatermarkMs? }); null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed, and bundled Redis adapters return RedisReadMiss for every semantic miss.

[lan17]

Verdict: request changes(recorded as a comment — GitHub refuses a requested-changes review on the author's own PR)

Reviewed at max effort (10 finder angles, adversarial verification, differential decode matrices up to 280 cells against main, plus a gap sweep). Typecheck and all 645 unit tests pass at 83e8a7c; the classification logic itself held up everywhere it was attacked: fence boundaries exact, zero-watermark/zero-timestamp edges probe-verified, no unbounded label can reach a metrics adapter, Prometheus re-registration fails loudly rather than mislabeling.

Requesting changes on three items. Each is cheap now and compounds after merge, because this PR freezes both the migration documentation and the public custom-adapter contract.

1. Align the docs with the fence extension (README.md:981, README.md:724, README.md:~485)

Threading observedWatermarkMs through decoded hit frames makes this more than a metrics migration: refills after core-side rejections (retained stale-on-error candidates, logical-age expiry, future-dated frames, deserialization failures) are now watermark-fenced — your own tests pin write suppression at candidate = watermark+0 on the retained path, fill_fenced where filled was emitted, and core-supplied createdAtMs on writes that never carried it. The behavior is good (verification showed every suppressed write would have been permanently unreadable, and the retained-path skip preserves the stale-on-error candidate the old refill destroyed) — but README line 981 still says this migration "does not change cache serving, refill, invalidation, stale recovery, or shadow-verdict behavior", which is now false, and issue #145 scoped behavior changes out. The PR body has been updated with a "Behavior changes" section; the README needs the matching fix:

• Rewrite the line-981 claim to scope it to what actually holds (serving verdicts, invalidation, stale-recovery classification, shadow match/mismatch/superseded verdicts).
• Line 724: "createdAtMs <= watermark is watermark_fenced" needs the zero-timestamp carve-out (a complete frame stamped 0 under a valid watermark classifies unclassified, probe-verified).

2. Give RedisReadMiss a discriminant (src/redis-client.ts:92)

RedisReadMiss has no kind, so isRedisReadMiss falls back to in-presence checks — reintroducing the exact hazard PR 143 added kind: "watermark_miss" to kill. Probe-verified: { reason: "value_absent", payload: undefined, createdAtMs: undefined } type-checks for strict consumers without exactOptionalPropertyTypes (tsc exit 0) and is misrouted as a frame at runtime, silently degrading every one of that adapter's declared reasons to unclassified — the feature defeated with no signal. Adding kind: "read_miss" pre-merge is free; after adapters adopt, it's another breaking change.

Stronger option worth 30 minutes while you're here: one discriminated miss type { kind; reason; observedWatermarkMs?: number }. That single change also collapses the twin constructors (redisReadMiss in redis-payload.ts:205 vs classifiedRedisReadMiss in redis-cache.ts:660), deletes the never-released-compat reason?: optionality (redis-client.ts:108 — no tag contains PR 143, so the optionality serves zero consumers) and its missReason() ceremony, and halves the 4× fence ternaries in getWithResolvedConfig.

3. Document the hit-shape widening (README Serialization section + migration list)

Bundled tracked hits now return { payload, createdAtMs, observedWatermarkMs } whenever a valid watermark exists (including 0). Your own exact-shape tests had to change (test/node-redis.test.ts, test/valkey-glide.test.ts) — consumers' will too. The README still describes DecodedRedisFrame as payload + createdAtMs only, and the migration list omits it. One sentence plus one bullet. (The PR body and BREAKING CHANGE footer now carry it.)

---

Non-blocking, comment-tier (take or leave; several vanish if you take the unified miss type):

• Corrupt-timestamp tracked frames are now silently reclassified at decode (redis-payload.ts:186): the legacy decodeTrackedRedisFrame returns null where main returned a frame or threw DialCacheRedisPayloadEncodingError (18 differential cells), the doubly-corrupt class loses its alertable error signal, and tracked vs untracked decoders now disagree on the same corrupt frame. Document as intended hardening or align the two.
• unclassifiedFrameMiss (redis-cache.ts:676) trusts an adapter-attached hit fence with envelope-only validation; a one-line observedWatermarkMs < frame.createdAtMs cross-check restores the pre-PR blast radius for contract-violating adapters.
• Taxonomy coherence: local TTL expiry reports value_absent while the same logical condition remotely reports unclassified; and the internal RedisCacheMissReason vs public CacheMissReason enums collide undocumented at the deser site (redis-cache.ts:184). Evidence-based and defensible — record the rationale in a comment/README clause so it reads as deliberate.
• validObservedWatermarkMs (redis-cache.ts:680) re-duplicates the safe-int≥0 envelope of isValidRedisTimestampMs (same finding as PR 143's isValidRedisWatermarkMiss, reincarnated) and runs twice per miss.
• isCacheMissReason (redis-cache.ts:686) has no exhaustiveness tie to the union — a future fourth reason compiles everywhere and silently demotes to unclassified at runtime. const CACHE_MISS_REASONS satisfies Record<CacheMissReason, true> beside the type fixes it.
• CacheMissReason isn't exported from the dialcache/redis-protocol subpath that exports RedisReadMiss — even scripts/test-package.mjs has to root-import it. One re-export line.
• Decode micro: readFrameCreatedAtMs runs twice per served tracked frame; decodedRedisFrame builds-then-spreads; the watermark-parse expression is duplicated in two branches (redis-payload.ts:139-158).
• Both legacy decoders are now zero-consumer wrappers, and this diff grows decodeTrackedRedisFrame with a strip projection. They were released in 0.22.0, so deleting is your policy call — this feat! is the natural moment.

Credit where due: the FakeRedis delegation to the real decoders kills the untracked-fence divergence from the PR 143 review, the fence-carry closes the core-side-rejection gap that PR 143 deliberately deferred, and the invalid-watermark normalization matrix and boundary tests are exactly the coverage this layer needs.

One revision pass and this is an easy approve.

[lan17]

Verdict: approve(recorded as a comment — GitHub refuses a formal review state on the author's own PR)

Re-reviewed at dc6199d (fix(metrics): preserve existing refill and decoder behavior). All three requested changes from the previous review are resolved; typecheck is clean, 653/653 unit tests pass, and a fresh sweep over the fix commit found nothing new.

Requested changes — all addressed

1. Scope / README behavior claim — fixed by reversion. The fence-carry is fully removed (DecodedRedisFrame.observedWatermarkMs, unclassifiedFrameMiss, the retained-path carry, and the validateFrameAge miss objects are gone). A 216-cell decoder differential against main shows byte-parity on the legacy decodeRedisFrame/decodeTrackedRedisFrame surfaces and zero hit/fence/throw drift on the classified decoders (only the added reason labels differ). The rewritten README claim — "Miss classification adds no new refill-fencing paths and preserves serving eligibility, invalidation, stale-recovery policy, and shadow outcomes" — is verified true: fencing occurs solely via PR 143's adapter-level watermark-miss path. This also moots the adapter-fence-trust hardening item, since frames no longer carry fences at all.
2. RedisReadMiss guard hazard — fixed via value-based guards. Both isRedisReadMiss and isRedisWatermarkMiss now treat explicitly-undefinedpayload/createdAtMs as absent, closing the misclassification for the new miss type and the PR-143-era residual on the kinded miss; probe coverage landed in the unit tests and the packed-package script (including the @ts-expect-error unbounded-reason negative). Guard value-reads are short-circuited behind the reason/kind presence checks and every classified miss is canonicalized inside validateReadResult, so no new throw path escapes unrecorded.
3. Hit-shape documentation — moot and documented. The widened hit shape is deleted; the README now states explicitly that decoded hits keep { payload, createdAtMs }.

Also verified: the unsafe-timestamp reclassification is fully reverted (doubly-corrupt frames throw DialCacheRedisPayloadEncodingError again; only the zero-baseline check remains, matching main), and the zero-timestamp carve-out landed in both the invalidation section and the metrics-table watermark_fenced row. The PR body has been updated to match the reverted scope (behavior-parity statement, corrected test count, footer back to the two real breaks).

Deferred (non-blocking, unchanged from the previous review)

Twin miss constructors (redisReadMiss / classifiedRedisReadMiss), the duplicated safe-int≥0 timestamp envelope in validObservedWatermarkMs, reason?: optionality serving a never-released shape, the non-exhaustive isCacheMissReason (a future fourth reason silently demotes to unclassified), CacheMissReason missing from the dialcache/redis-protocol exports, the local-vs-remote expired-entry taxonomy note, decode micro-items (double readFrameCreatedAtMs, duplicated watermark-parse expression), and the legacy-decoder deletion policy call. Several of these collapse together if the miss union is ever unified into one kinded type.

Good ship. The guard fix in particular is stronger than what was asked — it hardens both miss types instead of one.

[lan17]

Follow-up guidance: post-merge cleanup plan(non-blocking — the approve verdict above stands; this consolidates the deferred findings from both review rounds into an actionable plan for a single refactor: PR before 0.23 ships)

Two review rounds across #143/#146 kept finding one failure pattern: a correctness-critical invariant hand-written in more than one module, already divergent at birth (the three isRedisWatermarkMiss copies in #143 differed within one commit; the safe-int≥0 timestamp envelope has now been written three times). The cleanup principle is therefore one owner per invariant — not "no repeated lines". Some repetition here is deliberate and load-bearing; the last section lists what to leave alone.

DRY batch (descending leverage)

1. Unify the miss union into one kinded type — { kind, reason, observedWatermarkMs?: number }. Retires three deferred findings at once: the twin constructors (redisReadMiss in internal/redis-payload.ts vs classifiedRedisReadMiss in internal/redis-cache.ts — every bundled miss is currently built twice per read), the reason?: optionality that exists only for source compatibility with a shape that never shipped (no release contains #143's reason-less RedisWatermarkMiss, so missReason()'s undefined-handling at five sites serves nobody), and the composite isRedisReadMiss guard, which becomes a single kind check. Free now — none of this surface is released; expensive after adapter adoption. ~1 hour.
2. Exhaustiveness-tie the reason guard — isCacheMissReason (internal/redis-cache.ts) hand-lists the three strings with no compiler link to CacheMissReason. A future fourth reason compiles everywhere while validateReadResult silently demotes every adapter-supplied instance to unclassified — discovered in dashboards, not at build time. Fix: const CACHE_MISS_REASONS = { value_absent: true, watermark_fenced: true, unclassified: true } satisfies Record<CacheMissReason, true> beside the type, guard via Object.hasOwn (the exact pattern the tests and test-package.mjs already use).
3. One owner for the timestamp envelope — export boolean isValidRedisTimestampMs from internal/redis-payload.ts (widened to unknown) and define validObservedWatermarkMs on top of it. Prevents the envelope forking between bundled decoders and core validation of custom-adapter fences (e.g., if an upper bound is ever added so a garbage far-future watermark can't fence writes for years).
4. Move createdAtMs defaulting into the protocol — encodeRedisFrame(payload, createdAtMs?) defaulting to Date.now() internally (or one exported resolver). Today the "honor supplied value exactly, else sample" rule is a ternary copy-pasted into node-redis.ts and valkey-glide.ts and prescribed as README prose every custom adapter must transcribe; the natural mis-spelling request.createdAtMs || Date.now() silently breaks the fence-decision/stored-frame coupling at createdAtMs: 0. Make the correct behavior the only expressible one.
5. Small fry, batch opportunistically: hoist the duplicated watermarkFrame === null ? undefined : parseRedisWatermark(watermarkFrame) ?? undefined expression in decodeTrackedRedisReadResult; pass the already-read createdAtMs into decodedRedisFrame (removes the double readBigUInt64BE per served tracked hit); merge the two adjacent miss literals in getWithResolvedConfig; dedupe the README's twice-pasted "Conditional refill suppression reuses the existing tracked MGET result..." sentence (network-shape §, keep; ACL §, drop). Tests: fold the ~52-line ESM/CJS classified-decode paste in scripts/test-package.mjs into its existing shared-check mechanism (verifyPackedInvalidation precedent), and extract the thrice-copied throwing-serializer fixture.

Not DRY, but higher value than most of the above

• Meter the caller-path fence skip — putWithLayer still has three unmetered return false paths, so an active fence window is indistinguishable from a broken write path on dashboards (the same telemetry-gap shape as #141, which #143 fixed for the shadow side only). One new MetricErrorKind (fenced_refill_skipped — precedent: the advisory tracked_ttl_clamped) plus a typed 'dispatched' | 'fenced' | 'abandoned' return also removes the hidden invariant the shadow fill_fenced ternary depends on (sticky abandonment re-checked before the ternary). Extending MetricErrorKind is a type-surface change for exhaustive consumers — cheapest before 0.23, compounding after.
• Decide the legacy-decoder question — decodeRedisFrame / decodeTrackedRedisFrame are null-collapsing wrappers with zero in-repo consumers, kept for hypothetical external adapters; they drifted twice within this PR's own history before being re-pinned. Either delete them in this feat! window (they shipped in 0.22.0 but are unadopted; house policy is delete-don't-deprecate) or explicitly commit to the four-decoders-in-lockstep burden. Default-by-inaction is the worst option.
• Two one-liners: re-export CacheMissReason from dialcache/redis-protocol (it types RedisReadMiss.reason, which IS exported there; test-package.mjs currently has to root-import it), and a cross-reference comment at the deser site + one README table clause recording that local TTL expiry → value_absent vs remote logical expiry → unclassified is a deliberate evidence-based choice (and that internal RedisCacheMissReason is a disposition enum, not the metric reason) — so nobody "fixes" it later with another breaking metrics change.
• Optional hardening: make put's fence parameter a required RedisWatermarkMiss | undefined so any future fill path must decide explicitly instead of silently bypassing the fence.

Deliberate repetition — do NOT DRY these

• validateReadResult's reconstruction of adapter results is the single untrusted-input choke point, not redundant validation — keep it (with the union unification it gets simpler, not removed).
• The per-adapter label spreads ({...cacheLabels(labels), reason} in prometheus.ts/datadog.ts) are deliberate per-metric cardinality allowlists, consistent with disabled/error; prom-client rejects unknown labels, so wholesale pass-through isn't viable.
• FakeRedis's hand-rolled encodeFrame/decodeFrame test fixtures exist to construct malformed frames the real encoder refuses — that independence is the point (its read path already delegates to the real decoders, which is the right split).

Suggested packaging: items 1–4 + the one-liners as one refactor(redis): single-owner invariants for miss classification PR, the fence-skip metric as its own small feat(metrics) (it extends a public union), and the legacy-decoder deletion as a standalone commit gated on the adoption decision — all before the 0.23 release so every type-shape change rides the same pre-adoption window. Total estimated effort excluding the decoder decision: about half a day.

[lan17]

Summary

Implements #145 (replaces the stale attempt in #130).

Every DialCache miss event now says why it missed. Until now, a miss storm during a future-buffer invalidation window was indistinguishable from a cold cache or an eviction problem: the miss counter only told you that a layer missed, never whether the value was truly absent or a stored frame was being rejected by an invalidation watermark. This PR adds one required, bounded reason to the existing miss event — no new metric instrument, no new Redis command, no extra round trip, and no change to serving, refill, invalidation, stale-recovery, or shadow behavior.

What changes in your metrics

miss events now carry reason alongside the existing cacheNamespace / useCase / keyType / layer labels, on every layer (request_local, local, remote, remote_shadow):

reason	Fires when	Typical cause
value_absent	The layer had no retrievable value at all: never populated, physically expired or evicted, Redis nil, or a tracked-MGET wrong-type member (which Redis reports as nil). Request-local and process-local misses are always this.	Cold keys, physical TTL expiry, eviction pressure.
expired	A complete, supported frame with a valid, non-future timestamp was present, but its logical age against the reader's clock reached the effective remote TTL F. Covers age >= M misses and F <= age < M frames retained as stale-on-error candidates, on both caller and shadow reads.	Ordinary TTL churn on hot keys; the routine steady-state miss reason whenever staleOnErrorMaxAgeSec is configured.
watermark_fenced	A complete, supported tracked frame with a positive safe-integer timestamp was rejected because its createdAtMs was at or below a valid observed invalidation watermark. Decided before deserialization.	Reads inside an active futureBufferMs invalidation window; writer/invalidator clock skew.
unclassified	The miss is real but attributable to none of the above: unrecognized custom-adapter results (including null), short/unsupported frames, zero-stamped tracked frames, malformed watermark metadata paired with a present frame, future or invalid timestamps, and deserialization failures.	Un-migrated custom adapters; protocol-edge states.

The operational win: during an invalidation window you can now separate fence churn from genuine absence at a glance; routine TTL churn (including the stale-on-error F..M window, where every read of a retained frame is a miss) is expired rather than noise in the other buckets; and because expiry no longer lands there, a sudden unclassified plateau genuinely points at protocol-edge states (skewed clocks, corrupt frames, deserialization failures, legacy adapters) instead of hiding inside the aggregate.

Deviation from #145: the issue specified a three-value union and named unclassified as the safe default for stale-on-error retained frames "unless a separately justified reason is added". Review found that with staleOnErrorMaxAgeSec configured every read of a retained F..M frame is a miss, so unclassified would have been the routine steady-state reason for exactly the use cases that enable stale-on-error, and the plateau signal above would not hold. expired is that separately justified reason; the union is four values and the cardinality note below is 4x accordingly.

Prometheus — dialcache_miss_counter keeps its name and gains reason as a fifth label:

# Fence churn by use case during an invalidation window
sum by (use_case) (rate(dialcache_miss_counter{layer="remote", reason="watermark_fenced"}[5m]))
# Pre-existing total-miss and hit-ratio queries: aggregate reason away
sum by (cache_namespace, use_case, key_type, layer) (rate(dialcache_miss_counter[5m]))

Datadog — dialcache.miss.count keeps its name and gains the same reason tag, e.g. sum:dialcache.miss.count{reason:watermark_fenced} by {use_case}. Expect up to 4× the miss-series cardinality (four bounded reasons; only remote and remote_shadow tuples can carry every reason, since request-local and local misses are always value_absent).

Migration

• Prometheus registries: the label set changed, so registering against a registry that already holds a 4-label dialcache_miss_counter (e.g. a not-yet-upgraded sidecar library) fails loudly at construction — upgrade producers together per registry. Nothing silently mislabels.
• Mixed-fleet rollouts: old processes emit reason-less series while new ones carry reason. Total-miss and miss/request-ratio queries must sum by (...) the shared labels (example above); reason-aware dashboards should group by reason explicitly.
• Custom metrics adapters:DialCacheMetricsAdapter.miss now receives MissMetricLabels (extends the unchanged CacheMetricLabels with required reason: CacheMissReason). Adapters whose miss parameter is typed as the broader CacheMetricLabels compile unchanged and may ignore the field; exact label snapshots, exhaustive Records over reasons, and adapters that reject or forward unknown fields must add it.
• Custom Redis adapters:RedisReadResult is now DecodedRedisFrame | RedisReadMiss { kind: "miss", reason, observedWatermarkMs? }. null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed; use decodeRedisReadResult (untracked) / decodeTrackedRedisReadResult (tracked) and the exported isRedisReadMiss guard. An un-migrated adapter still fails open at runtime (null or any unrecognized result is an unclassified miss with a normal refill) but loses reason precision and fenced-refill suppression until it returns the typed miss.

How classification works

Bundled decoders return one miss shape, RedisReadMiss { kind: "miss", reason, observedWatermarkMs? }, attaching observedWatermarkMs only when the same atomic tracked snapshot carried a valid numeric watermark. Cause and fence are deliberately independent: Redis nil is decisive evidence of absence, so an absent value reports value_absent while still carrying the observed watermark that lets PR 143's two-sample admission skip a known-fenced refill — only a complete supported frame actually rejected by the watermark reports watermark_fenced.

Core treats the adapter boundary as untrusted and normalizes every result once, at one choke point: kind: "miss" is the only discriminator; invalid or missing reasons become unclassified, a watermark_fenced claim without a valid tracked fence is demoted to unclassified, invalid fences (NaN, negatives, non-integers, untracked keys) are dropped, and null, undefined, or any other unrecognized value is an unclassified miss with a normal refill. Objects without kind: "miss" are frame candidates and go through the existing timestamp and serializer validation.

Decoded hits keep the existing { payload, createdAtMs } shape. Core-side rejections never acquire a refill fence: logical-age rejections emit expired, future timestamps and deserialization failures emit unclassified; conditional refill suppression remains exactly PR 143's adapter-level watermark-miss path, byte-compatible with main (differential-verified across the decode input matrix).

Breaking change

DialCacheMetricsAdapter.miss now receives MissMetricLabels, and first-party miss metrics require a reason label/tag. RedisReadResult is now DecodedRedisFrame | RedisReadMiss; null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed, and bundled Redis adapters return RedisReadMiss for every semantic miss.

Under the pre-1.0 release policy, this is a minor release.

Validation

• Node.js 22.22.0: corepack pnpm check
  • typecheck
  • 656 unit tests with coverage thresholds
  • ESM/CJS build and declaration generation
  • packed-package type and runtime checks
• Node.js 22.22.0: corepack pnpm test:integration
  • 143 passed
  • 2 expected local GLIDE Cluster skips because announced container IPs are not host-routable
• Review follow-up (fix(metrics): preserve existing refill and decoder behavior) removed the fence-carry scope creep and restored decoder/refill parity with main, differential-verified across the decode input matrix

BREAKING CHANGE: DialCacheMetricsAdapter.miss now receives MissMetricLabels and first-party miss metrics require a reason label/tag; RedisReadResult is now DecodedRedisFrame | RedisReadMiss ({ kind: "miss", reason, observedWatermarkMs? }); null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed, and bundled Redis adapters return RedisReadMiss for every semantic miss.

[lan17]

Verdict: request changes(recorded as a comment — GitHub refuses a requested-changes review on the author's own PR)

Reviewed at max effort (10 finder angles, adversarial verification, differential decode matrices up to 280 cells against main, plus a gap sweep). Typecheck and all 645 unit tests pass at 83e8a7c; the classification logic itself held up everywhere it was attacked: fence boundaries exact, zero-watermark/zero-timestamp edges probe-verified, no unbounded label can reach a metrics adapter, Prometheus re-registration fails loudly rather than mislabeling.

Requesting changes on three items. Each is cheap now and compounds after merge, because this PR freezes both the migration documentation and the public custom-adapter contract.

1. Align the docs with the fence extension (README.md:981, README.md:724, README.md:~485)

Threading observedWatermarkMs through decoded hit frames makes this more than a metrics migration: refills after core-side rejections (retained stale-on-error candidates, logical-age expiry, future-dated frames, deserialization failures) are now watermark-fenced — your own tests pin write suppression at candidate = watermark+0 on the retained path, fill_fenced where filled was emitted, and core-supplied createdAtMs on writes that never carried it. The behavior is good (verification showed every suppressed write would have been permanently unreadable, and the retained-path skip preserves the stale-on-error candidate the old refill destroyed) — but README line 981 still says this migration "does not change cache serving, refill, invalidation, stale recovery, or shadow-verdict behavior", which is now false, and issue #145 scoped behavior changes out. The PR body has been updated with a "Behavior changes" section; the README needs the matching fix:

• Rewrite the line-981 claim to scope it to what actually holds (serving verdicts, invalidation, stale-recovery classification, shadow match/mismatch/superseded verdicts).
• Line 724: "createdAtMs <= watermark is watermark_fenced" needs the zero-timestamp carve-out (a complete frame stamped 0 under a valid watermark classifies unclassified, probe-verified).

2. Give RedisReadMiss a discriminant (src/redis-client.ts:92)

RedisReadMiss has no kind, so isRedisReadMiss falls back to in-presence checks — reintroducing the exact hazard PR 143 added kind: "watermark_miss" to kill. Probe-verified: { reason: "value_absent", payload: undefined, createdAtMs: undefined } type-checks for strict consumers without exactOptionalPropertyTypes (tsc exit 0) and is misrouted as a frame at runtime, silently degrading every one of that adapter's declared reasons to unclassified — the feature defeated with no signal. Adding kind: "read_miss" pre-merge is free; after adapters adopt, it's another breaking change.

Stronger option worth 30 minutes while you're here: one discriminated miss type { kind; reason; observedWatermarkMs?: number }. That single change also collapses the twin constructors (redisReadMiss in redis-payload.ts:205 vs classifiedRedisReadMiss in redis-cache.ts:660), deletes the never-released-compat reason?: optionality (redis-client.ts:108 — no tag contains PR 143, so the optionality serves zero consumers) and its missReason() ceremony, and halves the 4× fence ternaries in getWithResolvedConfig.

3. Document the hit-shape widening (README Serialization section + migration list)

Bundled tracked hits now return { payload, createdAtMs, observedWatermarkMs } whenever a valid watermark exists (including 0). Your own exact-shape tests had to change (test/node-redis.test.ts, test/valkey-glide.test.ts) — consumers' will too. The README still describes DecodedRedisFrame as payload + createdAtMs only, and the migration list omits it. One sentence plus one bullet. (The PR body and BREAKING CHANGE footer now carry it.)

---

Non-blocking, comment-tier (take or leave; several vanish if you take the unified miss type):

• Corrupt-timestamp tracked frames are now silently reclassified at decode (redis-payload.ts:186): the legacy decodeTrackedRedisFrame returns null where main returned a frame or threw DialCacheRedisPayloadEncodingError (18 differential cells), the doubly-corrupt class loses its alertable error signal, and tracked vs untracked decoders now disagree on the same corrupt frame. Document as intended hardening or align the two.
• unclassifiedFrameMiss (redis-cache.ts:676) trusts an adapter-attached hit fence with envelope-only validation; a one-line observedWatermarkMs < frame.createdAtMs cross-check restores the pre-PR blast radius for contract-violating adapters.
• Taxonomy coherence: local TTL expiry reports value_absent while the same logical condition remotely reports unclassified; and the internal RedisCacheMissReason vs public CacheMissReason enums collide undocumented at the deser site (redis-cache.ts:184). Evidence-based and defensible — record the rationale in a comment/README clause so it reads as deliberate.
• validObservedWatermarkMs (redis-cache.ts:680) re-duplicates the safe-int≥0 envelope of isValidRedisTimestampMs (same finding as PR 143's isValidRedisWatermarkMiss, reincarnated) and runs twice per miss.
• isCacheMissReason (redis-cache.ts:686) has no exhaustiveness tie to the union — a future fourth reason compiles everywhere and silently demotes to unclassified at runtime. const CACHE_MISS_REASONS satisfies Record<CacheMissReason, true> beside the type fixes it.
• CacheMissReason isn't exported from the dialcache/redis-protocol subpath that exports RedisReadMiss — even scripts/test-package.mjs has to root-import it. One re-export line.
• Decode micro: readFrameCreatedAtMs runs twice per served tracked frame; decodedRedisFrame builds-then-spreads; the watermark-parse expression is duplicated in two branches (redis-payload.ts:139-158).
• Both legacy decoders are now zero-consumer wrappers, and this diff grows decodeTrackedRedisFrame with a strip projection. They were released in 0.22.0, so deleting is your policy call — this feat! is the natural moment.

Credit where due: the FakeRedis delegation to the real decoders kills the untracked-fence divergence from the PR 143 review, the fence-carry closes the core-side-rejection gap that PR 143 deliberately deferred, and the invalid-watermark normalization matrix and boundary tests are exactly the coverage this layer needs.

One revision pass and this is an easy approve.

[lan17]

Verdict: approve(recorded as a comment — GitHub refuses a formal review state on the author's own PR)

Re-reviewed at dc6199d (fix(metrics): preserve existing refill and decoder behavior). All three requested changes from the previous review are resolved; typecheck is clean, 653/653 unit tests pass, and a fresh sweep over the fix commit found nothing new.

Requested changes — all addressed

1. Scope / README behavior claim — fixed by reversion. The fence-carry is fully removed (DecodedRedisFrame.observedWatermarkMs, unclassifiedFrameMiss, the retained-path carry, and the validateFrameAge miss objects are gone). A 216-cell decoder differential against main shows byte-parity on the legacy decodeRedisFrame/decodeTrackedRedisFrame surfaces and zero hit/fence/throw drift on the classified decoders (only the added reason labels differ). The rewritten README claim — "Miss classification adds no new refill-fencing paths and preserves serving eligibility, invalidation, stale-recovery policy, and shadow outcomes" — is verified true: fencing occurs solely via PR 143's adapter-level watermark-miss path. This also moots the adapter-fence-trust hardening item, since frames no longer carry fences at all.
2. RedisReadMiss guard hazard — fixed via value-based guards. Both isRedisReadMiss and isRedisWatermarkMiss now treat explicitly-undefinedpayload/createdAtMs as absent, closing the misclassification for the new miss type and the PR-143-era residual on the kinded miss; probe coverage landed in the unit tests and the packed-package script (including the @ts-expect-error unbounded-reason negative). Guard value-reads are short-circuited behind the reason/kind presence checks and every classified miss is canonicalized inside validateReadResult, so no new throw path escapes unrecorded.
3. Hit-shape documentation — moot and documented. The widened hit shape is deleted; the README now states explicitly that decoded hits keep { payload, createdAtMs }.

Also verified: the unsafe-timestamp reclassification is fully reverted (doubly-corrupt frames throw DialCacheRedisPayloadEncodingError again; only the zero-baseline check remains, matching main), and the zero-timestamp carve-out landed in both the invalidation section and the metrics-table watermark_fenced row. The PR body has been updated to match the reverted scope (behavior-parity statement, corrected test count, footer back to the two real breaks).

Deferred (non-blocking, unchanged from the previous review)

Twin miss constructors (redisReadMiss / classifiedRedisReadMiss), the duplicated safe-int≥0 timestamp envelope in validObservedWatermarkMs, reason?: optionality serving a never-released shape, the non-exhaustive isCacheMissReason (a future fourth reason silently demotes to unclassified), CacheMissReason missing from the dialcache/redis-protocol exports, the local-vs-remote expired-entry taxonomy note, decode micro-items (double readFrameCreatedAtMs, duplicated watermark-parse expression), and the legacy-decoder deletion policy call. Several of these collapse together if the miss union is ever unified into one kinded type.

Good ship. The guard fix in particular is stronger than what was asked — it hardens both miss types instead of one.

[lan17]

Follow-up guidance: post-merge cleanup plan(non-blocking — the approve verdict above stands; this consolidates the deferred findings from both review rounds into an actionable plan for a single refactor: PR before 0.23 ships)

Two review rounds across #143/#146 kept finding one failure pattern: a correctness-critical invariant hand-written in more than one module, already divergent at birth (the three isRedisWatermarkMiss copies in #143 differed within one commit; the safe-int≥0 timestamp envelope has now been written three times). The cleanup principle is therefore one owner per invariant — not "no repeated lines". Some repetition here is deliberate and load-bearing; the last section lists what to leave alone.

DRY batch (descending leverage)

1. Unify the miss union into one kinded type — { kind, reason, observedWatermarkMs?: number }. Retires three deferred findings at once: the twin constructors (redisReadMiss in internal/redis-payload.ts vs classifiedRedisReadMiss in internal/redis-cache.ts — every bundled miss is currently built twice per read), the reason?: optionality that exists only for source compatibility with a shape that never shipped (no release contains #143's reason-less RedisWatermarkMiss, so missReason()'s undefined-handling at five sites serves nobody), and the composite isRedisReadMiss guard, which becomes a single kind check. Free now — none of this surface is released; expensive after adapter adoption. ~1 hour.
2. Exhaustiveness-tie the reason guard — isCacheMissReason (internal/redis-cache.ts) hand-lists the three strings with no compiler link to CacheMissReason. A future fourth reason compiles everywhere while validateReadResult silently demotes every adapter-supplied instance to unclassified — discovered in dashboards, not at build time. Fix: const CACHE_MISS_REASONS = { value_absent: true, watermark_fenced: true, unclassified: true } satisfies Record<CacheMissReason, true> beside the type, guard via Object.hasOwn (the exact pattern the tests and test-package.mjs already use).
3. One owner for the timestamp envelope — export boolean isValidRedisTimestampMs from internal/redis-payload.ts (widened to unknown) and define validObservedWatermarkMs on top of it. Prevents the envelope forking between bundled decoders and core validation of custom-adapter fences (e.g., if an upper bound is ever added so a garbage far-future watermark can't fence writes for years).
4. Move createdAtMs defaulting into the protocol — encodeRedisFrame(payload, createdAtMs?) defaulting to Date.now() internally (or one exported resolver). Today the "honor supplied value exactly, else sample" rule is a ternary copy-pasted into node-redis.ts and valkey-glide.ts and prescribed as README prose every custom adapter must transcribe; the natural mis-spelling request.createdAtMs || Date.now() silently breaks the fence-decision/stored-frame coupling at createdAtMs: 0. Make the correct behavior the only expressible one.
5. Small fry, batch opportunistically: hoist the duplicated watermarkFrame === null ? undefined : parseRedisWatermark(watermarkFrame) ?? undefined expression in decodeTrackedRedisReadResult; pass the already-read createdAtMs into decodedRedisFrame (removes the double readBigUInt64BE per served tracked hit); merge the two adjacent miss literals in getWithResolvedConfig; dedupe the README's twice-pasted "Conditional refill suppression reuses the existing tracked MGET result..." sentence (network-shape §, keep; ACL §, drop). Tests: fold the ~52-line ESM/CJS classified-decode paste in scripts/test-package.mjs into its existing shared-check mechanism (verifyPackedInvalidation precedent), and extract the thrice-copied throwing-serializer fixture.

Not DRY, but higher value than most of the above

• Meter the caller-path fence skip — putWithLayer still has three unmetered return false paths, so an active fence window is indistinguishable from a broken write path on dashboards (the same telemetry-gap shape as #141, which #143 fixed for the shadow side only). One new MetricErrorKind (fenced_refill_skipped — precedent: the advisory tracked_ttl_clamped) plus a typed 'dispatched' | 'fenced' | 'abandoned' return also removes the hidden invariant the shadow fill_fenced ternary depends on (sticky abandonment re-checked before the ternary). Extending MetricErrorKind is a type-surface change for exhaustive consumers — cheapest before 0.23, compounding after.
• Decide the legacy-decoder question — decodeRedisFrame / decodeTrackedRedisFrame are null-collapsing wrappers with zero in-repo consumers, kept for hypothetical external adapters; they drifted twice within this PR's own history before being re-pinned. Either delete them in this feat! window (they shipped in 0.22.0 but are unadopted; house policy is delete-don't-deprecate) or explicitly commit to the four-decoders-in-lockstep burden. Default-by-inaction is the worst option.
• Two one-liners: re-export CacheMissReason from dialcache/redis-protocol (it types RedisReadMiss.reason, which IS exported there; test-package.mjs currently has to root-import it), and a cross-reference comment at the deser site + one README table clause recording that local TTL expiry → value_absent vs remote logical expiry → unclassified is a deliberate evidence-based choice (and that internal RedisCacheMissReason is a disposition enum, not the metric reason) — so nobody "fixes" it later with another breaking metrics change.
• Optional hardening: make put's fence parameter a required RedisWatermarkMiss | undefined so any future fill path must decide explicitly instead of silently bypassing the fence.

Deliberate repetition — do NOT DRY these

• validateReadResult's reconstruction of adapter results is the single untrusted-input choke point, not redundant validation — keep it (with the union unification it gets simpler, not removed).
• The per-adapter label spreads ({...cacheLabels(labels), reason} in prometheus.ts/datadog.ts) are deliberate per-metric cardinality allowlists, consistent with disabled/error; prom-client rejects unknown labels, so wholesale pass-through isn't viable.
• FakeRedis's hand-rolled encodeFrame/decodeFrame test fixtures exist to construct malformed frames the real encoder refuses — that independence is the point (its read path already delegates to the real decoders, which is the right split).

Suggested packaging: items 1–4 + the one-liners as one refactor(redis): single-owner invariants for miss classification PR, the fence-skip metric as its own small feat(metrics) (it extends a public union), and the legacy-decoder deletion as a standalone commit gated on the adoption decision — all before the 0.23 release so every type-shape change rides the same pre-adoption window. Total estimated effort excluding the decoder decision: about half a day.

[lan17]

Summary

Implements #145 (replaces the stale attempt in #130).

Every DialCache miss event now says why it missed. Until now, a miss storm during a future-buffer invalidation window was indistinguishable from a cold cache or an eviction problem: the miss counter only told you that a layer missed, never whether the value was truly absent or a stored frame was being rejected by an invalidation watermark. This PR adds one required, bounded reason to the existing miss event — no new metric instrument, no new Redis command, no extra round trip, and no change to serving, refill, invalidation, stale-recovery, or shadow behavior.

What changes in your metrics

miss events now carry reason alongside the existing cacheNamespace / useCase / keyType / layer labels, on every layer (request_local, local, remote, remote_shadow):

reason	Fires when	Typical cause
value_absent	The layer had no retrievable value at all: never populated, physically expired or evicted, Redis nil, or a tracked-MGET wrong-type member (which Redis reports as nil). Request-local and process-local misses are always this.	Cold keys, physical TTL expiry, eviction pressure.
expired	A complete, supported frame with a valid, non-future timestamp was present, but its logical age against the reader's clock reached the effective remote TTL F. Covers age >= M misses and F <= age < M frames retained as stale-on-error candidates, on both caller and shadow reads.	Ordinary TTL churn on hot keys; the routine steady-state miss reason whenever staleOnErrorMaxAgeSec is configured.
watermark_fenced	A complete, supported tracked frame with a positive safe-integer timestamp was rejected because its createdAtMs was at or below a valid observed invalidation watermark. Decided before deserialization.	Reads inside an active futureBufferMs invalidation window; writer/invalidator clock skew.
unclassified	The miss is real but attributable to none of the above: unrecognized custom-adapter results (including null), short/unsupported frames, zero-stamped tracked frames, malformed watermark metadata paired with a present frame, future or invalid timestamps, and deserialization failures.	Un-migrated custom adapters; protocol-edge states.

The operational win: during an invalidation window you can now separate fence churn from genuine absence at a glance; routine TTL churn (including the stale-on-error F..M window, where every read of a retained frame is a miss) is expired rather than noise in the other buckets; and because expiry no longer lands there, a sudden unclassified plateau genuinely points at protocol-edge states (skewed clocks, corrupt frames, deserialization failures, legacy adapters) instead of hiding inside the aggregate.

Deviation from #145: the issue specified a three-value union and named unclassified as the safe default for stale-on-error retained frames "unless a separately justified reason is added". Review found that with staleOnErrorMaxAgeSec configured every read of a retained F..M frame is a miss, so unclassified would have been the routine steady-state reason for exactly the use cases that enable stale-on-error, and the plateau signal above would not hold. expired is that separately justified reason; the union is four values and the cardinality note below is 4x accordingly.

Prometheus — dialcache_miss_counter keeps its name and gains reason as a fifth label:

# Fence churn by use case during an invalidation window
sum by (use_case) (rate(dialcache_miss_counter{layer="remote", reason="watermark_fenced"}[5m]))
# Pre-existing total-miss and hit-ratio queries: aggregate reason away
sum by (cache_namespace, use_case, key_type, layer) (rate(dialcache_miss_counter[5m]))

Datadog — dialcache.miss.count keeps its name and gains the same reason tag, e.g. sum:dialcache.miss.count{reason:watermark_fenced} by {use_case}. Expect up to 4× the miss-series cardinality (four bounded reasons; only remote and remote_shadow tuples can carry every reason, since request-local and local misses are always value_absent).

Migration

• Prometheus registries: the label set changed, so registering against a registry that already holds a 4-label dialcache_miss_counter (e.g. a not-yet-upgraded sidecar library) fails loudly at construction — upgrade producers together per registry. Nothing silently mislabels.
• Mixed-fleet rollouts: old processes emit reason-less series while new ones carry reason. Total-miss and miss/request-ratio queries must sum by (...) the shared labels (example above); reason-aware dashboards should group by reason explicitly.
• Custom metrics adapters:DialCacheMetricsAdapter.miss now receives MissMetricLabels (extends the unchanged CacheMetricLabels with required reason: CacheMissReason). Adapters whose miss parameter is typed as the broader CacheMetricLabels compile unchanged and may ignore the field; exact label snapshots, exhaustive Records over reasons, and adapters that reject or forward unknown fields must add it.
• Custom Redis adapters:RedisReadResult is now DecodedRedisFrame | RedisReadMiss { kind: "miss", reason, observedWatermarkMs? }. null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed; use decodeRedisReadResult (untracked) / decodeTrackedRedisReadResult (tracked) and the exported isRedisReadMiss guard. An un-migrated adapter still fails open at runtime (null or any unrecognized result is an unclassified miss with a normal refill) but loses reason precision and fenced-refill suppression until it returns the typed miss.

How classification works

Bundled decoders return one miss shape, RedisReadMiss { kind: "miss", reason, observedWatermarkMs? }, attaching observedWatermarkMs only when the same atomic tracked snapshot carried a valid numeric watermark. Cause and fence are deliberately independent: Redis nil is decisive evidence of absence, so an absent value reports value_absent while still carrying the observed watermark that lets PR 143's two-sample admission skip a known-fenced refill — only a complete supported frame actually rejected by the watermark reports watermark_fenced.

Core treats the adapter boundary as untrusted and normalizes every result once, at one choke point: kind: "miss" is the only discriminator; invalid or missing reasons become unclassified, a watermark_fenced claim without a valid tracked fence is demoted to unclassified, invalid fences (NaN, negatives, non-integers, untracked keys) are dropped, and null, undefined, or any other unrecognized value is an unclassified miss with a normal refill. Objects without kind: "miss" are frame candidates and go through the existing timestamp and serializer validation.

Decoded hits keep the existing { payload, createdAtMs } shape. Core-side rejections never acquire a refill fence: logical-age rejections emit expired, future timestamps and deserialization failures emit unclassified; conditional refill suppression remains exactly PR 143's adapter-level watermark-miss path, byte-compatible with main (differential-verified across the decode input matrix).

Breaking change

DialCacheMetricsAdapter.miss now receives MissMetricLabels, and first-party miss metrics require a reason label/tag. RedisReadResult is now DecodedRedisFrame | RedisReadMiss; null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed, and bundled Redis adapters return RedisReadMiss for every semantic miss.

Under the pre-1.0 release policy, this is a minor release.

Validation

• Node.js 22.22.0: corepack pnpm check
  • typecheck
  • 656 unit tests with coverage thresholds
  • ESM/CJS build and declaration generation
  • packed-package type and runtime checks
• Node.js 22.22.0: corepack pnpm test:integration
  • 143 passed
  • 2 expected local GLIDE Cluster skips because announced container IPs are not host-routable
• Review follow-up (fix(metrics): preserve existing refill and decoder behavior) removed the fence-carry scope creep and restored decoder/refill parity with main, differential-verified across the decode input matrix

BREAKING CHANGE: DialCacheMetricsAdapter.miss now receives MissMetricLabels and first-party miss metrics require a reason label/tag; RedisReadResult is now DecodedRedisFrame | RedisReadMiss ({ kind: "miss", reason, observedWatermarkMs? }); null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed, and bundled Redis adapters return RedisReadMiss for every semantic miss.

[lan17]

Verdict: request changes(recorded as a comment — GitHub refuses a requested-changes review on the author's own PR)

Reviewed at max effort (10 finder angles, adversarial verification, differential decode matrices up to 280 cells against main, plus a gap sweep). Typecheck and all 645 unit tests pass at 83e8a7c; the classification logic itself held up everywhere it was attacked: fence boundaries exact, zero-watermark/zero-timestamp edges probe-verified, no unbounded label can reach a metrics adapter, Prometheus re-registration fails loudly rather than mislabeling.

Requesting changes on three items. Each is cheap now and compounds after merge, because this PR freezes both the migration documentation and the public custom-adapter contract.

1. Align the docs with the fence extension (README.md:981, README.md:724, README.md:~485)

Threading observedWatermarkMs through decoded hit frames makes this more than a metrics migration: refills after core-side rejections (retained stale-on-error candidates, logical-age expiry, future-dated frames, deserialization failures) are now watermark-fenced — your own tests pin write suppression at candidate = watermark+0 on the retained path, fill_fenced where filled was emitted, and core-supplied createdAtMs on writes that never carried it. The behavior is good (verification showed every suppressed write would have been permanently unreadable, and the retained-path skip preserves the stale-on-error candidate the old refill destroyed) — but README line 981 still says this migration "does not change cache serving, refill, invalidation, stale recovery, or shadow-verdict behavior", which is now false, and issue #145 scoped behavior changes out. The PR body has been updated with a "Behavior changes" section; the README needs the matching fix:

• Rewrite the line-981 claim to scope it to what actually holds (serving verdicts, invalidation, stale-recovery classification, shadow match/mismatch/superseded verdicts).
• Line 724: "createdAtMs <= watermark is watermark_fenced" needs the zero-timestamp carve-out (a complete frame stamped 0 under a valid watermark classifies unclassified, probe-verified).

2. Give RedisReadMiss a discriminant (src/redis-client.ts:92)

RedisReadMiss has no kind, so isRedisReadMiss falls back to in-presence checks — reintroducing the exact hazard PR 143 added kind: "watermark_miss" to kill. Probe-verified: { reason: "value_absent", payload: undefined, createdAtMs: undefined } type-checks for strict consumers without exactOptionalPropertyTypes (tsc exit 0) and is misrouted as a frame at runtime, silently degrading every one of that adapter's declared reasons to unclassified — the feature defeated with no signal. Adding kind: "read_miss" pre-merge is free; after adapters adopt, it's another breaking change.

Stronger option worth 30 minutes while you're here: one discriminated miss type { kind; reason; observedWatermarkMs?: number }. That single change also collapses the twin constructors (redisReadMiss in redis-payload.ts:205 vs classifiedRedisReadMiss in redis-cache.ts:660), deletes the never-released-compat reason?: optionality (redis-client.ts:108 — no tag contains PR 143, so the optionality serves zero consumers) and its missReason() ceremony, and halves the 4× fence ternaries in getWithResolvedConfig.

3. Document the hit-shape widening (README Serialization section + migration list)

Bundled tracked hits now return { payload, createdAtMs, observedWatermarkMs } whenever a valid watermark exists (including 0). Your own exact-shape tests had to change (test/node-redis.test.ts, test/valkey-glide.test.ts) — consumers' will too. The README still describes DecodedRedisFrame as payload + createdAtMs only, and the migration list omits it. One sentence plus one bullet. (The PR body and BREAKING CHANGE footer now carry it.)

---

Non-blocking, comment-tier (take or leave; several vanish if you take the unified miss type):

• Corrupt-timestamp tracked frames are now silently reclassified at decode (redis-payload.ts:186): the legacy decodeTrackedRedisFrame returns null where main returned a frame or threw DialCacheRedisPayloadEncodingError (18 differential cells), the doubly-corrupt class loses its alertable error signal, and tracked vs untracked decoders now disagree on the same corrupt frame. Document as intended hardening or align the two.
• unclassifiedFrameMiss (redis-cache.ts:676) trusts an adapter-attached hit fence with envelope-only validation; a one-line observedWatermarkMs < frame.createdAtMs cross-check restores the pre-PR blast radius for contract-violating adapters.
• Taxonomy coherence: local TTL expiry reports value_absent while the same logical condition remotely reports unclassified; and the internal RedisCacheMissReason vs public CacheMissReason enums collide undocumented at the deser site (redis-cache.ts:184). Evidence-based and defensible — record the rationale in a comment/README clause so it reads as deliberate.
• validObservedWatermarkMs (redis-cache.ts:680) re-duplicates the safe-int≥0 envelope of isValidRedisTimestampMs (same finding as PR 143's isValidRedisWatermarkMiss, reincarnated) and runs twice per miss.
• isCacheMissReason (redis-cache.ts:686) has no exhaustiveness tie to the union — a future fourth reason compiles everywhere and silently demotes to unclassified at runtime. const CACHE_MISS_REASONS satisfies Record<CacheMissReason, true> beside the type fixes it.
• CacheMissReason isn't exported from the dialcache/redis-protocol subpath that exports RedisReadMiss — even scripts/test-package.mjs has to root-import it. One re-export line.
• Decode micro: readFrameCreatedAtMs runs twice per served tracked frame; decodedRedisFrame builds-then-spreads; the watermark-parse expression is duplicated in two branches (redis-payload.ts:139-158).
• Both legacy decoders are now zero-consumer wrappers, and this diff grows decodeTrackedRedisFrame with a strip projection. They were released in 0.22.0, so deleting is your policy call — this feat! is the natural moment.

Credit where due: the FakeRedis delegation to the real decoders kills the untracked-fence divergence from the PR 143 review, the fence-carry closes the core-side-rejection gap that PR 143 deliberately deferred, and the invalid-watermark normalization matrix and boundary tests are exactly the coverage this layer needs.

One revision pass and this is an easy approve.

[lan17]

Verdict: approve(recorded as a comment — GitHub refuses a formal review state on the author's own PR)

Re-reviewed at dc6199d (fix(metrics): preserve existing refill and decoder behavior). All three requested changes from the previous review are resolved; typecheck is clean, 653/653 unit tests pass, and a fresh sweep over the fix commit found nothing new.

Requested changes — all addressed

1. Scope / README behavior claim — fixed by reversion. The fence-carry is fully removed (DecodedRedisFrame.observedWatermarkMs, unclassifiedFrameMiss, the retained-path carry, and the validateFrameAge miss objects are gone). A 216-cell decoder differential against main shows byte-parity on the legacy decodeRedisFrame/decodeTrackedRedisFrame surfaces and zero hit/fence/throw drift on the classified decoders (only the added reason labels differ). The rewritten README claim — "Miss classification adds no new refill-fencing paths and preserves serving eligibility, invalidation, stale-recovery policy, and shadow outcomes" — is verified true: fencing occurs solely via PR 143's adapter-level watermark-miss path. This also moots the adapter-fence-trust hardening item, since frames no longer carry fences at all.
2. RedisReadMiss guard hazard — fixed via value-based guards. Both isRedisReadMiss and isRedisWatermarkMiss now treat explicitly-undefinedpayload/createdAtMs as absent, closing the misclassification for the new miss type and the PR-143-era residual on the kinded miss; probe coverage landed in the unit tests and the packed-package script (including the @ts-expect-error unbounded-reason negative). Guard value-reads are short-circuited behind the reason/kind presence checks and every classified miss is canonicalized inside validateReadResult, so no new throw path escapes unrecorded.
3. Hit-shape documentation — moot and documented. The widened hit shape is deleted; the README now states explicitly that decoded hits keep { payload, createdAtMs }.

Also verified: the unsafe-timestamp reclassification is fully reverted (doubly-corrupt frames throw DialCacheRedisPayloadEncodingError again; only the zero-baseline check remains, matching main), and the zero-timestamp carve-out landed in both the invalidation section and the metrics-table watermark_fenced row. The PR body has been updated to match the reverted scope (behavior-parity statement, corrected test count, footer back to the two real breaks).

Deferred (non-blocking, unchanged from the previous review)

Twin miss constructors (redisReadMiss / classifiedRedisReadMiss), the duplicated safe-int≥0 timestamp envelope in validObservedWatermarkMs, reason?: optionality serving a never-released shape, the non-exhaustive isCacheMissReason (a future fourth reason silently demotes to unclassified), CacheMissReason missing from the dialcache/redis-protocol exports, the local-vs-remote expired-entry taxonomy note, decode micro-items (double readFrameCreatedAtMs, duplicated watermark-parse expression), and the legacy-decoder deletion policy call. Several of these collapse together if the miss union is ever unified into one kinded type.

Good ship. The guard fix in particular is stronger than what was asked — it hardens both miss types instead of one.

[lan17]

Follow-up guidance: post-merge cleanup plan(non-blocking — the approve verdict above stands; this consolidates the deferred findings from both review rounds into an actionable plan for a single refactor: PR before 0.23 ships)

Two review rounds across #143/#146 kept finding one failure pattern: a correctness-critical invariant hand-written in more than one module, already divergent at birth (the three isRedisWatermarkMiss copies in #143 differed within one commit; the safe-int≥0 timestamp envelope has now been written three times). The cleanup principle is therefore one owner per invariant — not "no repeated lines". Some repetition here is deliberate and load-bearing; the last section lists what to leave alone.

DRY batch (descending leverage)

1. Unify the miss union into one kinded type — { kind, reason, observedWatermarkMs?: number }. Retires three deferred findings at once: the twin constructors (redisReadMiss in internal/redis-payload.ts vs classifiedRedisReadMiss in internal/redis-cache.ts — every bundled miss is currently built twice per read), the reason?: optionality that exists only for source compatibility with a shape that never shipped (no release contains #143's reason-less RedisWatermarkMiss, so missReason()'s undefined-handling at five sites serves nobody), and the composite isRedisReadMiss guard, which becomes a single kind check. Free now — none of this surface is released; expensive after adapter adoption. ~1 hour.
2. Exhaustiveness-tie the reason guard — isCacheMissReason (internal/redis-cache.ts) hand-lists the three strings with no compiler link to CacheMissReason. A future fourth reason compiles everywhere while validateReadResult silently demotes every adapter-supplied instance to unclassified — discovered in dashboards, not at build time. Fix: const CACHE_MISS_REASONS = { value_absent: true, watermark_fenced: true, unclassified: true } satisfies Record<CacheMissReason, true> beside the type, guard via Object.hasOwn (the exact pattern the tests and test-package.mjs already use).
3. One owner for the timestamp envelope — export boolean isValidRedisTimestampMs from internal/redis-payload.ts (widened to unknown) and define validObservedWatermarkMs on top of it. Prevents the envelope forking between bundled decoders and core validation of custom-adapter fences (e.g., if an upper bound is ever added so a garbage far-future watermark can't fence writes for years).
4. Move createdAtMs defaulting into the protocol — encodeRedisFrame(payload, createdAtMs?) defaulting to Date.now() internally (or one exported resolver). Today the "honor supplied value exactly, else sample" rule is a ternary copy-pasted into node-redis.ts and valkey-glide.ts and prescribed as README prose every custom adapter must transcribe; the natural mis-spelling request.createdAtMs || Date.now() silently breaks the fence-decision/stored-frame coupling at createdAtMs: 0. Make the correct behavior the only expressible one.
5. Small fry, batch opportunistically: hoist the duplicated watermarkFrame === null ? undefined : parseRedisWatermark(watermarkFrame) ?? undefined expression in decodeTrackedRedisReadResult; pass the already-read createdAtMs into decodedRedisFrame (removes the double readBigUInt64BE per served tracked hit); merge the two adjacent miss literals in getWithResolvedConfig; dedupe the README's twice-pasted "Conditional refill suppression reuses the existing tracked MGET result..." sentence (network-shape §, keep; ACL §, drop). Tests: fold the ~52-line ESM/CJS classified-decode paste in scripts/test-package.mjs into its existing shared-check mechanism (verifyPackedInvalidation precedent), and extract the thrice-copied throwing-serializer fixture.

Not DRY, but higher value than most of the above

• Meter the caller-path fence skip — putWithLayer still has three unmetered return false paths, so an active fence window is indistinguishable from a broken write path on dashboards (the same telemetry-gap shape as #141, which #143 fixed for the shadow side only). One new MetricErrorKind (fenced_refill_skipped — precedent: the advisory tracked_ttl_clamped) plus a typed 'dispatched' | 'fenced' | 'abandoned' return also removes the hidden invariant the shadow fill_fenced ternary depends on (sticky abandonment re-checked before the ternary). Extending MetricErrorKind is a type-surface change for exhaustive consumers — cheapest before 0.23, compounding after.
• Decide the legacy-decoder question — decodeRedisFrame / decodeTrackedRedisFrame are null-collapsing wrappers with zero in-repo consumers, kept for hypothetical external adapters; they drifted twice within this PR's own history before being re-pinned. Either delete them in this feat! window (they shipped in 0.22.0 but are unadopted; house policy is delete-don't-deprecate) or explicitly commit to the four-decoders-in-lockstep burden. Default-by-inaction is the worst option.
• Two one-liners: re-export CacheMissReason from dialcache/redis-protocol (it types RedisReadMiss.reason, which IS exported there; test-package.mjs currently has to root-import it), and a cross-reference comment at the deser site + one README table clause recording that local TTL expiry → value_absent vs remote logical expiry → unclassified is a deliberate evidence-based choice (and that internal RedisCacheMissReason is a disposition enum, not the metric reason) — so nobody "fixes" it later with another breaking metrics change.
• Optional hardening: make put's fence parameter a required RedisWatermarkMiss | undefined so any future fill path must decide explicitly instead of silently bypassing the fence.

Deliberate repetition — do NOT DRY these

• validateReadResult's reconstruction of adapter results is the single untrusted-input choke point, not redundant validation — keep it (with the union unification it gets simpler, not removed).
• The per-adapter label spreads ({...cacheLabels(labels), reason} in prometheus.ts/datadog.ts) are deliberate per-metric cardinality allowlists, consistent with disabled/error; prom-client rejects unknown labels, so wholesale pass-through isn't viable.
• FakeRedis's hand-rolled encodeFrame/decodeFrame test fixtures exist to construct malformed frames the real encoder refuses — that independence is the point (its read path already delegates to the real decoders, which is the right split).

Suggested packaging: items 1–4 + the one-liners as one refactor(redis): single-owner invariants for miss classification PR, the fence-skip metric as its own small feat(metrics) (it extends a public union), and the legacy-decoder deletion as a standalone commit gated on the adoption decision — all before the 0.23 release so every type-shape change rides the same pre-adoption window. Total estimated effort excluding the decoder decision: about half a day.

[lan17]

Summary

Implements #145 (replaces the stale attempt in #130).

Every DialCache miss event now says why it missed. Until now, a miss storm during a future-buffer invalidation window was indistinguishable from a cold cache or an eviction problem: the miss counter only told you that a layer missed, never whether the value was truly absent or a stored frame was being rejected by an invalidation watermark. This PR adds one required, bounded reason to the existing miss event — no new metric instrument, no new Redis command, no extra round trip, and no change to serving, refill, invalidation, stale-recovery, or shadow behavior.

What changes in your metrics

miss events now carry reason alongside the existing cacheNamespace / useCase / keyType / layer labels, on every layer (request_local, local, remote, remote_shadow):

reason	Fires when	Typical cause
value_absent	The layer had no retrievable value at all: never populated, physically expired or evicted, Redis nil, or a tracked-MGET wrong-type member (which Redis reports as nil). Request-local and process-local misses are always this.	Cold keys, physical TTL expiry, eviction pressure.
expired	A complete, supported frame with a valid, non-future timestamp was present, but its logical age against the reader's clock reached the effective remote TTL F. Covers age >= M misses and F <= age < M frames retained as stale-on-error candidates, on both caller and shadow reads.	Ordinary TTL churn on hot keys; the routine steady-state miss reason whenever staleOnErrorMaxAgeSec is configured.
watermark_fenced	A complete, supported tracked frame with a positive safe-integer timestamp was rejected because its createdAtMs was at or below a valid observed invalidation watermark. Decided before deserialization.	Reads inside an active futureBufferMs invalidation window; writer/invalidator clock skew.
unclassified	The miss is real but attributable to none of the above: unrecognized custom-adapter results (including null), short/unsupported frames, zero-stamped tracked frames, malformed watermark metadata paired with a present frame, future or invalid timestamps, and deserialization failures.	Un-migrated custom adapters; protocol-edge states.

The operational win: during an invalidation window you can now separate fence churn from genuine absence at a glance; routine TTL churn (including the stale-on-error F..M window, where every read of a retained frame is a miss) is expired rather than noise in the other buckets; and because expiry no longer lands there, a sudden unclassified plateau genuinely points at protocol-edge states (skewed clocks, corrupt frames, deserialization failures, legacy adapters) instead of hiding inside the aggregate.

Deviation from #145: the issue specified a three-value union and named unclassified as the safe default for stale-on-error retained frames "unless a separately justified reason is added". Review found that with staleOnErrorMaxAgeSec configured every read of a retained F..M frame is a miss, so unclassified would have been the routine steady-state reason for exactly the use cases that enable stale-on-error, and the plateau signal above would not hold. expired is that separately justified reason; the union is four values and the cardinality note below is 4x accordingly.

Prometheus — dialcache_miss_counter keeps its name and gains reason as a fifth label:

# Fence churn by use case during an invalidation window
sum by (use_case) (rate(dialcache_miss_counter{layer="remote", reason="watermark_fenced"}[5m]))
# Pre-existing total-miss and hit-ratio queries: aggregate reason away
sum by (cache_namespace, use_case, key_type, layer) (rate(dialcache_miss_counter[5m]))

Datadog — dialcache.miss.count keeps its name and gains the same reason tag, e.g. sum:dialcache.miss.count{reason:watermark_fenced} by {use_case}. Expect up to 4× the miss-series cardinality (four bounded reasons; only remote and remote_shadow tuples can carry every reason, since request-local and local misses are always value_absent).

Migration

• Prometheus registries: the label set changed, so registering against a registry that already holds a 4-label dialcache_miss_counter (e.g. a not-yet-upgraded sidecar library) fails loudly at construction — upgrade producers together per registry. Nothing silently mislabels.
• Mixed-fleet rollouts: old processes emit reason-less series while new ones carry reason. Total-miss and miss/request-ratio queries must sum by (...) the shared labels (example above); reason-aware dashboards should group by reason explicitly.
• Custom metrics adapters:DialCacheMetricsAdapter.miss now receives MissMetricLabels (extends the unchanged CacheMetricLabels with required reason: CacheMissReason). Adapters whose miss parameter is typed as the broader CacheMetricLabels compile unchanged and may ignore the field; exact label snapshots, exhaustive Records over reasons, and adapters that reject or forward unknown fields must add it.
• Custom Redis adapters:RedisReadResult is now DecodedRedisFrame | RedisReadMiss { kind: "miss", reason, observedWatermarkMs? }. null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed; use decodeRedisReadResult (untracked) / decodeTrackedRedisReadResult (tracked) and the exported isRedisReadMiss guard. An un-migrated adapter still fails open at runtime (null or any unrecognized result is an unclassified miss with a normal refill) but loses reason precision and fenced-refill suppression until it returns the typed miss.

How classification works

Bundled decoders return one miss shape, RedisReadMiss { kind: "miss", reason, observedWatermarkMs? }, attaching observedWatermarkMs only when the same atomic tracked snapshot carried a valid numeric watermark. Cause and fence are deliberately independent: Redis nil is decisive evidence of absence, so an absent value reports value_absent while still carrying the observed watermark that lets PR 143's two-sample admission skip a known-fenced refill — only a complete supported frame actually rejected by the watermark reports watermark_fenced.

Core treats the adapter boundary as untrusted and normalizes every result once, at one choke point: kind: "miss" is the only discriminator; invalid or missing reasons become unclassified, a watermark_fenced claim without a valid tracked fence is demoted to unclassified, invalid fences (NaN, negatives, non-integers, untracked keys) are dropped, and null, undefined, or any other unrecognized value is an unclassified miss with a normal refill. Objects without kind: "miss" are frame candidates and go through the existing timestamp and serializer validation.

Decoded hits keep the existing { payload, createdAtMs } shape. Core-side rejections never acquire a refill fence: logical-age rejections emit expired, future timestamps and deserialization failures emit unclassified; conditional refill suppression remains exactly PR 143's adapter-level watermark-miss path, byte-compatible with main (differential-verified across the decode input matrix).

Breaking change

DialCacheMetricsAdapter.miss now receives MissMetricLabels, and first-party miss metrics require a reason label/tag. RedisReadResult is now DecodedRedisFrame | RedisReadMiss; null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed, and bundled Redis adapters return RedisReadMiss for every semantic miss.

Under the pre-1.0 release policy, this is a minor release.

Validation

• Node.js 22.22.0: corepack pnpm check
  • typecheck
  • 656 unit tests with coverage thresholds
  • ESM/CJS build and declaration generation
  • packed-package type and runtime checks
• Node.js 22.22.0: corepack pnpm test:integration
  • 143 passed
  • 2 expected local GLIDE Cluster skips because announced container IPs are not host-routable
• Review follow-up (fix(metrics): preserve existing refill and decoder behavior) removed the fence-carry scope creep and restored decoder/refill parity with main, differential-verified across the decode input matrix

BREAKING CHANGE: DialCacheMetricsAdapter.miss now receives MissMetricLabels and first-party miss metrics require a reason label/tag; RedisReadResult is now DecodedRedisFrame | RedisReadMiss ({ kind: "miss", reason, observedWatermarkMs? }); null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed, and bundled Redis adapters return RedisReadMiss for every semantic miss.

[lan17]

Verdict: request changes(recorded as a comment — GitHub refuses a requested-changes review on the author's own PR)

Reviewed at max effort (10 finder angles, adversarial verification, differential decode matrices up to 280 cells against main, plus a gap sweep). Typecheck and all 645 unit tests pass at 83e8a7c; the classification logic itself held up everywhere it was attacked: fence boundaries exact, zero-watermark/zero-timestamp edges probe-verified, no unbounded label can reach a metrics adapter, Prometheus re-registration fails loudly rather than mislabeling.

Requesting changes on three items. Each is cheap now and compounds after merge, because this PR freezes both the migration documentation and the public custom-adapter contract.

1. Align the docs with the fence extension (README.md:981, README.md:724, README.md:~485)

Threading observedWatermarkMs through decoded hit frames makes this more than a metrics migration: refills after core-side rejections (retained stale-on-error candidates, logical-age expiry, future-dated frames, deserialization failures) are now watermark-fenced — your own tests pin write suppression at candidate = watermark+0 on the retained path, fill_fenced where filled was emitted, and core-supplied createdAtMs on writes that never carried it. The behavior is good (verification showed every suppressed write would have been permanently unreadable, and the retained-path skip preserves the stale-on-error candidate the old refill destroyed) — but README line 981 still says this migration "does not change cache serving, refill, invalidation, stale recovery, or shadow-verdict behavior", which is now false, and issue #145 scoped behavior changes out. The PR body has been updated with a "Behavior changes" section; the README needs the matching fix:

• Rewrite the line-981 claim to scope it to what actually holds (serving verdicts, invalidation, stale-recovery classification, shadow match/mismatch/superseded verdicts).
• Line 724: "createdAtMs <= watermark is watermark_fenced" needs the zero-timestamp carve-out (a complete frame stamped 0 under a valid watermark classifies unclassified, probe-verified).

2. Give RedisReadMiss a discriminant (src/redis-client.ts:92)

RedisReadMiss has no kind, so isRedisReadMiss falls back to in-presence checks — reintroducing the exact hazard PR 143 added kind: "watermark_miss" to kill. Probe-verified: { reason: "value_absent", payload: undefined, createdAtMs: undefined } type-checks for strict consumers without exactOptionalPropertyTypes (tsc exit 0) and is misrouted as a frame at runtime, silently degrading every one of that adapter's declared reasons to unclassified — the feature defeated with no signal. Adding kind: "read_miss" pre-merge is free; after adapters adopt, it's another breaking change.

Stronger option worth 30 minutes while you're here: one discriminated miss type { kind; reason; observedWatermarkMs?: number }. That single change also collapses the twin constructors (redisReadMiss in redis-payload.ts:205 vs classifiedRedisReadMiss in redis-cache.ts:660), deletes the never-released-compat reason?: optionality (redis-client.ts:108 — no tag contains PR 143, so the optionality serves zero consumers) and its missReason() ceremony, and halves the 4× fence ternaries in getWithResolvedConfig.

3. Document the hit-shape widening (README Serialization section + migration list)

Bundled tracked hits now return { payload, createdAtMs, observedWatermarkMs } whenever a valid watermark exists (including 0). Your own exact-shape tests had to change (test/node-redis.test.ts, test/valkey-glide.test.ts) — consumers' will too. The README still describes DecodedRedisFrame as payload + createdAtMs only, and the migration list omits it. One sentence plus one bullet. (The PR body and BREAKING CHANGE footer now carry it.)

---

Non-blocking, comment-tier (take or leave; several vanish if you take the unified miss type):

• Corrupt-timestamp tracked frames are now silently reclassified at decode (redis-payload.ts:186): the legacy decodeTrackedRedisFrame returns null where main returned a frame or threw DialCacheRedisPayloadEncodingError (18 differential cells), the doubly-corrupt class loses its alertable error signal, and tracked vs untracked decoders now disagree on the same corrupt frame. Document as intended hardening or align the two.
• unclassifiedFrameMiss (redis-cache.ts:676) trusts an adapter-attached hit fence with envelope-only validation; a one-line observedWatermarkMs < frame.createdAtMs cross-check restores the pre-PR blast radius for contract-violating adapters.
• Taxonomy coherence: local TTL expiry reports value_absent while the same logical condition remotely reports unclassified; and the internal RedisCacheMissReason vs public CacheMissReason enums collide undocumented at the deser site (redis-cache.ts:184). Evidence-based and defensible — record the rationale in a comment/README clause so it reads as deliberate.
• validObservedWatermarkMs (redis-cache.ts:680) re-duplicates the safe-int≥0 envelope of isValidRedisTimestampMs (same finding as PR 143's isValidRedisWatermarkMiss, reincarnated) and runs twice per miss.
• isCacheMissReason (redis-cache.ts:686) has no exhaustiveness tie to the union — a future fourth reason compiles everywhere and silently demotes to unclassified at runtime. const CACHE_MISS_REASONS satisfies Record<CacheMissReason, true> beside the type fixes it.
• CacheMissReason isn't exported from the dialcache/redis-protocol subpath that exports RedisReadMiss — even scripts/test-package.mjs has to root-import it. One re-export line.
• Decode micro: readFrameCreatedAtMs runs twice per served tracked frame; decodedRedisFrame builds-then-spreads; the watermark-parse expression is duplicated in two branches (redis-payload.ts:139-158).
• Both legacy decoders are now zero-consumer wrappers, and this diff grows decodeTrackedRedisFrame with a strip projection. They were released in 0.22.0, so deleting is your policy call — this feat! is the natural moment.

Credit where due: the FakeRedis delegation to the real decoders kills the untracked-fence divergence from the PR 143 review, the fence-carry closes the core-side-rejection gap that PR 143 deliberately deferred, and the invalid-watermark normalization matrix and boundary tests are exactly the coverage this layer needs.

One revision pass and this is an easy approve.

[lan17]

Verdict: approve(recorded as a comment — GitHub refuses a formal review state on the author's own PR)

Re-reviewed at dc6199d (fix(metrics): preserve existing refill and decoder behavior). All three requested changes from the previous review are resolved; typecheck is clean, 653/653 unit tests pass, and a fresh sweep over the fix commit found nothing new.

Requested changes — all addressed

1. Scope / README behavior claim — fixed by reversion. The fence-carry is fully removed (DecodedRedisFrame.observedWatermarkMs, unclassifiedFrameMiss, the retained-path carry, and the validateFrameAge miss objects are gone). A 216-cell decoder differential against main shows byte-parity on the legacy decodeRedisFrame/decodeTrackedRedisFrame surfaces and zero hit/fence/throw drift on the classified decoders (only the added reason labels differ). The rewritten README claim — "Miss classification adds no new refill-fencing paths and preserves serving eligibility, invalidation, stale-recovery policy, and shadow outcomes" — is verified true: fencing occurs solely via PR 143's adapter-level watermark-miss path. This also moots the adapter-fence-trust hardening item, since frames no longer carry fences at all.
2. RedisReadMiss guard hazard — fixed via value-based guards. Both isRedisReadMiss and isRedisWatermarkMiss now treat explicitly-undefinedpayload/createdAtMs as absent, closing the misclassification for the new miss type and the PR-143-era residual on the kinded miss; probe coverage landed in the unit tests and the packed-package script (including the @ts-expect-error unbounded-reason negative). Guard value-reads are short-circuited behind the reason/kind presence checks and every classified miss is canonicalized inside validateReadResult, so no new throw path escapes unrecorded.
3. Hit-shape documentation — moot and documented. The widened hit shape is deleted; the README now states explicitly that decoded hits keep { payload, createdAtMs }.

Also verified: the unsafe-timestamp reclassification is fully reverted (doubly-corrupt frames throw DialCacheRedisPayloadEncodingError again; only the zero-baseline check remains, matching main), and the zero-timestamp carve-out landed in both the invalidation section and the metrics-table watermark_fenced row. The PR body has been updated to match the reverted scope (behavior-parity statement, corrected test count, footer back to the two real breaks).

Deferred (non-blocking, unchanged from the previous review)

Twin miss constructors (redisReadMiss / classifiedRedisReadMiss), the duplicated safe-int≥0 timestamp envelope in validObservedWatermarkMs, reason?: optionality serving a never-released shape, the non-exhaustive isCacheMissReason (a future fourth reason silently demotes to unclassified), CacheMissReason missing from the dialcache/redis-protocol exports, the local-vs-remote expired-entry taxonomy note, decode micro-items (double readFrameCreatedAtMs, duplicated watermark-parse expression), and the legacy-decoder deletion policy call. Several of these collapse together if the miss union is ever unified into one kinded type.

Good ship. The guard fix in particular is stronger than what was asked — it hardens both miss types instead of one.

[lan17]

Follow-up guidance: post-merge cleanup plan(non-blocking — the approve verdict above stands; this consolidates the deferred findings from both review rounds into an actionable plan for a single refactor: PR before 0.23 ships)

Two review rounds across #143/#146 kept finding one failure pattern: a correctness-critical invariant hand-written in more than one module, already divergent at birth (the three isRedisWatermarkMiss copies in #143 differed within one commit; the safe-int≥0 timestamp envelope has now been written three times). The cleanup principle is therefore one owner per invariant — not "no repeated lines". Some repetition here is deliberate and load-bearing; the last section lists what to leave alone.

DRY batch (descending leverage)

1. Unify the miss union into one kinded type — { kind, reason, observedWatermarkMs?: number }. Retires three deferred findings at once: the twin constructors (redisReadMiss in internal/redis-payload.ts vs classifiedRedisReadMiss in internal/redis-cache.ts — every bundled miss is currently built twice per read), the reason?: optionality that exists only for source compatibility with a shape that never shipped (no release contains #143's reason-less RedisWatermarkMiss, so missReason()'s undefined-handling at five sites serves nobody), and the composite isRedisReadMiss guard, which becomes a single kind check. Free now — none of this surface is released; expensive after adapter adoption. ~1 hour.
2. Exhaustiveness-tie the reason guard — isCacheMissReason (internal/redis-cache.ts) hand-lists the three strings with no compiler link to CacheMissReason. A future fourth reason compiles everywhere while validateReadResult silently demotes every adapter-supplied instance to unclassified — discovered in dashboards, not at build time. Fix: const CACHE_MISS_REASONS = { value_absent: true, watermark_fenced: true, unclassified: true } satisfies Record<CacheMissReason, true> beside the type, guard via Object.hasOwn (the exact pattern the tests and test-package.mjs already use).
3. One owner for the timestamp envelope — export boolean isValidRedisTimestampMs from internal/redis-payload.ts (widened to unknown) and define validObservedWatermarkMs on top of it. Prevents the envelope forking between bundled decoders and core validation of custom-adapter fences (e.g., if an upper bound is ever added so a garbage far-future watermark can't fence writes for years).
4. Move createdAtMs defaulting into the protocol — encodeRedisFrame(payload, createdAtMs?) defaulting to Date.now() internally (or one exported resolver). Today the "honor supplied value exactly, else sample" rule is a ternary copy-pasted into node-redis.ts and valkey-glide.ts and prescribed as README prose every custom adapter must transcribe; the natural mis-spelling request.createdAtMs || Date.now() silently breaks the fence-decision/stored-frame coupling at createdAtMs: 0. Make the correct behavior the only expressible one.
5. Small fry, batch opportunistically: hoist the duplicated watermarkFrame === null ? undefined : parseRedisWatermark(watermarkFrame) ?? undefined expression in decodeTrackedRedisReadResult; pass the already-read createdAtMs into decodedRedisFrame (removes the double readBigUInt64BE per served tracked hit); merge the two adjacent miss literals in getWithResolvedConfig; dedupe the README's twice-pasted "Conditional refill suppression reuses the existing tracked MGET result..." sentence (network-shape §, keep; ACL §, drop). Tests: fold the ~52-line ESM/CJS classified-decode paste in scripts/test-package.mjs into its existing shared-check mechanism (verifyPackedInvalidation precedent), and extract the thrice-copied throwing-serializer fixture.

Not DRY, but higher value than most of the above

• Meter the caller-path fence skip — putWithLayer still has three unmetered return false paths, so an active fence window is indistinguishable from a broken write path on dashboards (the same telemetry-gap shape as #141, which #143 fixed for the shadow side only). One new MetricErrorKind (fenced_refill_skipped — precedent: the advisory tracked_ttl_clamped) plus a typed 'dispatched' | 'fenced' | 'abandoned' return also removes the hidden invariant the shadow fill_fenced ternary depends on (sticky abandonment re-checked before the ternary). Extending MetricErrorKind is a type-surface change for exhaustive consumers — cheapest before 0.23, compounding after.
• Decide the legacy-decoder question — decodeRedisFrame / decodeTrackedRedisFrame are null-collapsing wrappers with zero in-repo consumers, kept for hypothetical external adapters; they drifted twice within this PR's own history before being re-pinned. Either delete them in this feat! window (they shipped in 0.22.0 but are unadopted; house policy is delete-don't-deprecate) or explicitly commit to the four-decoders-in-lockstep burden. Default-by-inaction is the worst option.
• Two one-liners: re-export CacheMissReason from dialcache/redis-protocol (it types RedisReadMiss.reason, which IS exported there; test-package.mjs currently has to root-import it), and a cross-reference comment at the deser site + one README table clause recording that local TTL expiry → value_absent vs remote logical expiry → unclassified is a deliberate evidence-based choice (and that internal RedisCacheMissReason is a disposition enum, not the metric reason) — so nobody "fixes" it later with another breaking metrics change.
• Optional hardening: make put's fence parameter a required RedisWatermarkMiss | undefined so any future fill path must decide explicitly instead of silently bypassing the fence.

Deliberate repetition — do NOT DRY these

• validateReadResult's reconstruction of adapter results is the single untrusted-input choke point, not redundant validation — keep it (with the union unification it gets simpler, not removed).
• The per-adapter label spreads ({...cacheLabels(labels), reason} in prometheus.ts/datadog.ts) are deliberate per-metric cardinality allowlists, consistent with disabled/error; prom-client rejects unknown labels, so wholesale pass-through isn't viable.
• FakeRedis's hand-rolled encodeFrame/decodeFrame test fixtures exist to construct malformed frames the real encoder refuses — that independence is the point (its read path already delegates to the real decoders, which is the right split).

Suggested packaging: items 1–4 + the one-liners as one refactor(redis): single-owner invariants for miss classification PR, the fence-skip metric as its own small feat(metrics) (it extends a public union), and the legacy-decoder deletion as a standalone commit gated on the adoption decision — all before the 0.23 release so every type-shape change rides the same pre-adoption window. Total estimated effort excluding the decoder decision: about half a day.

[lan17]

Summary

Implements #145 (replaces the stale attempt in #130).

Every DialCache miss event now says why it missed. Until now, a miss storm during a future-buffer invalidation window was indistinguishable from a cold cache or an eviction problem: the miss counter only told you that a layer missed, never whether the value was truly absent or a stored frame was being rejected by an invalidation watermark. This PR adds one required, bounded reason to the existing miss event — no new metric instrument, no new Redis command, no extra round trip, and no change to serving, refill, invalidation, stale-recovery, or shadow behavior.

What changes in your metrics

miss events now carry reason alongside the existing cacheNamespace / useCase / keyType / layer labels, on every layer (request_local, local, remote, remote_shadow):

reason	Fires when	Typical cause
value_absent	The layer had no retrievable value at all: never populated, physically expired or evicted, Redis nil, or a tracked-MGET wrong-type member (which Redis reports as nil). Request-local and process-local misses are always this.	Cold keys, physical TTL expiry, eviction pressure.
expired	A complete, supported frame with a valid, non-future timestamp was present, but its logical age against the reader's clock reached the effective remote TTL F. Covers age >= M misses and F <= age < M frames retained as stale-on-error candidates, on both caller and shadow reads.	Ordinary TTL churn on hot keys; the routine steady-state miss reason whenever staleOnErrorMaxAgeSec is configured.
watermark_fenced	A complete, supported tracked frame with a positive safe-integer timestamp was rejected because its createdAtMs was at or below a valid observed invalidation watermark. Decided before deserialization.	Reads inside an active futureBufferMs invalidation window; writer/invalidator clock skew.
unclassified	The miss is real but attributable to none of the above: unrecognized custom-adapter results (including null), short/unsupported frames, zero-stamped tracked frames, malformed watermark metadata paired with a present frame, future or invalid timestamps, and deserialization failures.	Un-migrated custom adapters; protocol-edge states.

The operational win: during an invalidation window you can now separate fence churn from genuine absence at a glance; routine TTL churn (including the stale-on-error F..M window, where every read of a retained frame is a miss) is expired rather than noise in the other buckets; and because expiry no longer lands there, a sudden unclassified plateau genuinely points at protocol-edge states (skewed clocks, corrupt frames, deserialization failures, legacy adapters) instead of hiding inside the aggregate.

Deviation from #145: the issue specified a three-value union and named unclassified as the safe default for stale-on-error retained frames "unless a separately justified reason is added". Review found that with staleOnErrorMaxAgeSec configured every read of a retained F..M frame is a miss, so unclassified would have been the routine steady-state reason for exactly the use cases that enable stale-on-error, and the plateau signal above would not hold. expired is that separately justified reason; the union is four values and the cardinality note below is 4x accordingly.

Prometheus — dialcache_miss_counter keeps its name and gains reason as a fifth label:

# Fence churn by use case during an invalidation window
sum by (use_case) (rate(dialcache_miss_counter{layer="remote", reason="watermark_fenced"}[5m]))
# Pre-existing total-miss and hit-ratio queries: aggregate reason away
sum by (cache_namespace, use_case, key_type, layer) (rate(dialcache_miss_counter[5m]))

Datadog — dialcache.miss.count keeps its name and gains the same reason tag, e.g. sum:dialcache.miss.count{reason:watermark_fenced} by {use_case}. Expect up to 4× the miss-series cardinality (four bounded reasons; only remote and remote_shadow tuples can carry every reason, since request-local and local misses are always value_absent).

Migration

• Prometheus registries: the label set changed, so registering against a registry that already holds a 4-label dialcache_miss_counter (e.g. a not-yet-upgraded sidecar library) fails loudly at construction — upgrade producers together per registry. Nothing silently mislabels.
• Mixed-fleet rollouts: old processes emit reason-less series while new ones carry reason. Total-miss and miss/request-ratio queries must sum by (...) the shared labels (example above); reason-aware dashboards should group by reason explicitly.
• Custom metrics adapters:DialCacheMetricsAdapter.miss now receives MissMetricLabels (extends the unchanged CacheMetricLabels with required reason: CacheMissReason). Adapters whose miss parameter is typed as the broader CacheMetricLabels compile unchanged and may ignore the field; exact label snapshots, exhaustive Records over reasons, and adapters that reject or forward unknown fields must add it.
• Custom Redis adapters:RedisReadResult is now DecodedRedisFrame | RedisReadMiss { kind: "miss", reason, observedWatermarkMs? }. null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed; use decodeRedisReadResult (untracked) / decodeTrackedRedisReadResult (tracked) and the exported isRedisReadMiss guard. An un-migrated adapter still fails open at runtime (null or any unrecognized result is an unclassified miss with a normal refill) but loses reason precision and fenced-refill suppression until it returns the typed miss.

How classification works

Bundled decoders return one miss shape, RedisReadMiss { kind: "miss", reason, observedWatermarkMs? }, attaching observedWatermarkMs only when the same atomic tracked snapshot carried a valid numeric watermark. Cause and fence are deliberately independent: Redis nil is decisive evidence of absence, so an absent value reports value_absent while still carrying the observed watermark that lets PR 143's two-sample admission skip a known-fenced refill — only a complete supported frame actually rejected by the watermark reports watermark_fenced.

Core treats the adapter boundary as untrusted and normalizes every result once, at one choke point: kind: "miss" is the only discriminator; invalid or missing reasons become unclassified, a watermark_fenced claim without a valid tracked fence is demoted to unclassified, invalid fences (NaN, negatives, non-integers, untracked keys) are dropped, and null, undefined, or any other unrecognized value is an unclassified miss with a normal refill. Objects without kind: "miss" are frame candidates and go through the existing timestamp and serializer validation.

Decoded hits keep the existing { payload, createdAtMs } shape. Core-side rejections never acquire a refill fence: logical-age rejections emit expired, future timestamps and deserialization failures emit unclassified; conditional refill suppression remains exactly PR 143's adapter-level watermark-miss path, byte-compatible with main (differential-verified across the decode input matrix).

Breaking change

DialCacheMetricsAdapter.miss now receives MissMetricLabels, and first-party miss metrics require a reason label/tag. RedisReadResult is now DecodedRedisFrame | RedisReadMiss; null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed, and bundled Redis adapters return RedisReadMiss for every semantic miss.

Under the pre-1.0 release policy, this is a minor release.

Validation

• Node.js 22.22.0: corepack pnpm check
  • typecheck
  • 656 unit tests with coverage thresholds
  • ESM/CJS build and declaration generation
  • packed-package type and runtime checks
• Node.js 22.22.0: corepack pnpm test:integration
  • 143 passed
  • 2 expected local GLIDE Cluster skips because announced container IPs are not host-routable
• Review follow-up (fix(metrics): preserve existing refill and decoder behavior) removed the fence-carry scope creep and restored decoder/refill parity with main, differential-verified across the decode input matrix

BREAKING CHANGE: DialCacheMetricsAdapter.miss now receives MissMetricLabels and first-party miss metrics require a reason label/tag; RedisReadResult is now DecodedRedisFrame | RedisReadMiss ({ kind: "miss", reason, observedWatermarkMs? }); null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed, and bundled Redis adapters return RedisReadMiss for every semantic miss.

[lan17]

Verdict: request changes(recorded as a comment — GitHub refuses a requested-changes review on the author's own PR)

Reviewed at max effort (10 finder angles, adversarial verification, differential decode matrices up to 280 cells against main, plus a gap sweep). Typecheck and all 645 unit tests pass at 83e8a7c; the classification logic itself held up everywhere it was attacked: fence boundaries exact, zero-watermark/zero-timestamp edges probe-verified, no unbounded label can reach a metrics adapter, Prometheus re-registration fails loudly rather than mislabeling.

Requesting changes on three items. Each is cheap now and compounds after merge, because this PR freezes both the migration documentation and the public custom-adapter contract.

1. Align the docs with the fence extension (README.md:981, README.md:724, README.md:~485)

Threading observedWatermarkMs through decoded hit frames makes this more than a metrics migration: refills after core-side rejections (retained stale-on-error candidates, logical-age expiry, future-dated frames, deserialization failures) are now watermark-fenced — your own tests pin write suppression at candidate = watermark+0 on the retained path, fill_fenced where filled was emitted, and core-supplied createdAtMs on writes that never carried it. The behavior is good (verification showed every suppressed write would have been permanently unreadable, and the retained-path skip preserves the stale-on-error candidate the old refill destroyed) — but README line 981 still says this migration "does not change cache serving, refill, invalidation, stale recovery, or shadow-verdict behavior", which is now false, and issue #145 scoped behavior changes out. The PR body has been updated with a "Behavior changes" section; the README needs the matching fix:

• Rewrite the line-981 claim to scope it to what actually holds (serving verdicts, invalidation, stale-recovery classification, shadow match/mismatch/superseded verdicts).
• Line 724: "createdAtMs <= watermark is watermark_fenced" needs the zero-timestamp carve-out (a complete frame stamped 0 under a valid watermark classifies unclassified, probe-verified).

2. Give RedisReadMiss a discriminant (src/redis-client.ts:92)

RedisReadMiss has no kind, so isRedisReadMiss falls back to in-presence checks — reintroducing the exact hazard PR 143 added kind: "watermark_miss" to kill. Probe-verified: { reason: "value_absent", payload: undefined, createdAtMs: undefined } type-checks for strict consumers without exactOptionalPropertyTypes (tsc exit 0) and is misrouted as a frame at runtime, silently degrading every one of that adapter's declared reasons to unclassified — the feature defeated with no signal. Adding kind: "read_miss" pre-merge is free; after adapters adopt, it's another breaking change.

Stronger option worth 30 minutes while you're here: one discriminated miss type { kind; reason; observedWatermarkMs?: number }. That single change also collapses the twin constructors (redisReadMiss in redis-payload.ts:205 vs classifiedRedisReadMiss in redis-cache.ts:660), deletes the never-released-compat reason?: optionality (redis-client.ts:108 — no tag contains PR 143, so the optionality serves zero consumers) and its missReason() ceremony, and halves the 4× fence ternaries in getWithResolvedConfig.

3. Document the hit-shape widening (README Serialization section + migration list)

Bundled tracked hits now return { payload, createdAtMs, observedWatermarkMs } whenever a valid watermark exists (including 0). Your own exact-shape tests had to change (test/node-redis.test.ts, test/valkey-glide.test.ts) — consumers' will too. The README still describes DecodedRedisFrame as payload + createdAtMs only, and the migration list omits it. One sentence plus one bullet. (The PR body and BREAKING CHANGE footer now carry it.)

---

Non-blocking, comment-tier (take or leave; several vanish if you take the unified miss type):

• Corrupt-timestamp tracked frames are now silently reclassified at decode (redis-payload.ts:186): the legacy decodeTrackedRedisFrame returns null where main returned a frame or threw DialCacheRedisPayloadEncodingError (18 differential cells), the doubly-corrupt class loses its alertable error signal, and tracked vs untracked decoders now disagree on the same corrupt frame. Document as intended hardening or align the two.
• unclassifiedFrameMiss (redis-cache.ts:676) trusts an adapter-attached hit fence with envelope-only validation; a one-line observedWatermarkMs < frame.createdAtMs cross-check restores the pre-PR blast radius for contract-violating adapters.
• Taxonomy coherence: local TTL expiry reports value_absent while the same logical condition remotely reports unclassified; and the internal RedisCacheMissReason vs public CacheMissReason enums collide undocumented at the deser site (redis-cache.ts:184). Evidence-based and defensible — record the rationale in a comment/README clause so it reads as deliberate.
• validObservedWatermarkMs (redis-cache.ts:680) re-duplicates the safe-int≥0 envelope of isValidRedisTimestampMs (same finding as PR 143's isValidRedisWatermarkMiss, reincarnated) and runs twice per miss.
• isCacheMissReason (redis-cache.ts:686) has no exhaustiveness tie to the union — a future fourth reason compiles everywhere and silently demotes to unclassified at runtime. const CACHE_MISS_REASONS satisfies Record<CacheMissReason, true> beside the type fixes it.
• CacheMissReason isn't exported from the dialcache/redis-protocol subpath that exports RedisReadMiss — even scripts/test-package.mjs has to root-import it. One re-export line.
• Decode micro: readFrameCreatedAtMs runs twice per served tracked frame; decodedRedisFrame builds-then-spreads; the watermark-parse expression is duplicated in two branches (redis-payload.ts:139-158).
• Both legacy decoders are now zero-consumer wrappers, and this diff grows decodeTrackedRedisFrame with a strip projection. They were released in 0.22.0, so deleting is your policy call — this feat! is the natural moment.

Credit where due: the FakeRedis delegation to the real decoders kills the untracked-fence divergence from the PR 143 review, the fence-carry closes the core-side-rejection gap that PR 143 deliberately deferred, and the invalid-watermark normalization matrix and boundary tests are exactly the coverage this layer needs.

One revision pass and this is an easy approve.

[lan17]

Verdict: approve(recorded as a comment — GitHub refuses a formal review state on the author's own PR)

Re-reviewed at dc6199d (fix(metrics): preserve existing refill and decoder behavior). All three requested changes from the previous review are resolved; typecheck is clean, 653/653 unit tests pass, and a fresh sweep over the fix commit found nothing new.

Requested changes — all addressed

1. Scope / README behavior claim — fixed by reversion. The fence-carry is fully removed (DecodedRedisFrame.observedWatermarkMs, unclassifiedFrameMiss, the retained-path carry, and the validateFrameAge miss objects are gone). A 216-cell decoder differential against main shows byte-parity on the legacy decodeRedisFrame/decodeTrackedRedisFrame surfaces and zero hit/fence/throw drift on the classified decoders (only the added reason labels differ). The rewritten README claim — "Miss classification adds no new refill-fencing paths and preserves serving eligibility, invalidation, stale-recovery policy, and shadow outcomes" — is verified true: fencing occurs solely via PR 143's adapter-level watermark-miss path. This also moots the adapter-fence-trust hardening item, since frames no longer carry fences at all.
2. RedisReadMiss guard hazard — fixed via value-based guards. Both isRedisReadMiss and isRedisWatermarkMiss now treat explicitly-undefinedpayload/createdAtMs as absent, closing the misclassification for the new miss type and the PR-143-era residual on the kinded miss; probe coverage landed in the unit tests and the packed-package script (including the @ts-expect-error unbounded-reason negative). Guard value-reads are short-circuited behind the reason/kind presence checks and every classified miss is canonicalized inside validateReadResult, so no new throw path escapes unrecorded.
3. Hit-shape documentation — moot and documented. The widened hit shape is deleted; the README now states explicitly that decoded hits keep { payload, createdAtMs }.

Also verified: the unsafe-timestamp reclassification is fully reverted (doubly-corrupt frames throw DialCacheRedisPayloadEncodingError again; only the zero-baseline check remains, matching main), and the zero-timestamp carve-out landed in both the invalidation section and the metrics-table watermark_fenced row. The PR body has been updated to match the reverted scope (behavior-parity statement, corrected test count, footer back to the two real breaks).

Deferred (non-blocking, unchanged from the previous review)

Twin miss constructors (redisReadMiss / classifiedRedisReadMiss), the duplicated safe-int≥0 timestamp envelope in validObservedWatermarkMs, reason?: optionality serving a never-released shape, the non-exhaustive isCacheMissReason (a future fourth reason silently demotes to unclassified), CacheMissReason missing from the dialcache/redis-protocol exports, the local-vs-remote expired-entry taxonomy note, decode micro-items (double readFrameCreatedAtMs, duplicated watermark-parse expression), and the legacy-decoder deletion policy call. Several of these collapse together if the miss union is ever unified into one kinded type.

Good ship. The guard fix in particular is stronger than what was asked — it hardens both miss types instead of one.

[lan17]

Follow-up guidance: post-merge cleanup plan(non-blocking — the approve verdict above stands; this consolidates the deferred findings from both review rounds into an actionable plan for a single refactor: PR before 0.23 ships)

Two review rounds across #143/#146 kept finding one failure pattern: a correctness-critical invariant hand-written in more than one module, already divergent at birth (the three isRedisWatermarkMiss copies in #143 differed within one commit; the safe-int≥0 timestamp envelope has now been written three times). The cleanup principle is therefore one owner per invariant — not "no repeated lines". Some repetition here is deliberate and load-bearing; the last section lists what to leave alone.

DRY batch (descending leverage)

1. Unify the miss union into one kinded type — { kind, reason, observedWatermarkMs?: number }. Retires three deferred findings at once: the twin constructors (redisReadMiss in internal/redis-payload.ts vs classifiedRedisReadMiss in internal/redis-cache.ts — every bundled miss is currently built twice per read), the reason?: optionality that exists only for source compatibility with a shape that never shipped (no release contains #143's reason-less RedisWatermarkMiss, so missReason()'s undefined-handling at five sites serves nobody), and the composite isRedisReadMiss guard, which becomes a single kind check. Free now — none of this surface is released; expensive after adapter adoption. ~1 hour.
2. Exhaustiveness-tie the reason guard — isCacheMissReason (internal/redis-cache.ts) hand-lists the three strings with no compiler link to CacheMissReason. A future fourth reason compiles everywhere while validateReadResult silently demotes every adapter-supplied instance to unclassified — discovered in dashboards, not at build time. Fix: const CACHE_MISS_REASONS = { value_absent: true, watermark_fenced: true, unclassified: true } satisfies Record<CacheMissReason, true> beside the type, guard via Object.hasOwn (the exact pattern the tests and test-package.mjs already use).
3. One owner for the timestamp envelope — export boolean isValidRedisTimestampMs from internal/redis-payload.ts (widened to unknown) and define validObservedWatermarkMs on top of it. Prevents the envelope forking between bundled decoders and core validation of custom-adapter fences (e.g., if an upper bound is ever added so a garbage far-future watermark can't fence writes for years).
4. Move createdAtMs defaulting into the protocol — encodeRedisFrame(payload, createdAtMs?) defaulting to Date.now() internally (or one exported resolver). Today the "honor supplied value exactly, else sample" rule is a ternary copy-pasted into node-redis.ts and valkey-glide.ts and prescribed as README prose every custom adapter must transcribe; the natural mis-spelling request.createdAtMs || Date.now() silently breaks the fence-decision/stored-frame coupling at createdAtMs: 0. Make the correct behavior the only expressible one.
5. Small fry, batch opportunistically: hoist the duplicated watermarkFrame === null ? undefined : parseRedisWatermark(watermarkFrame) ?? undefined expression in decodeTrackedRedisReadResult; pass the already-read createdAtMs into decodedRedisFrame (removes the double readBigUInt64BE per served tracked hit); merge the two adjacent miss literals in getWithResolvedConfig; dedupe the README's twice-pasted "Conditional refill suppression reuses the existing tracked MGET result..." sentence (network-shape §, keep; ACL §, drop). Tests: fold the ~52-line ESM/CJS classified-decode paste in scripts/test-package.mjs into its existing shared-check mechanism (verifyPackedInvalidation precedent), and extract the thrice-copied throwing-serializer fixture.

Not DRY, but higher value than most of the above

• Meter the caller-path fence skip — putWithLayer still has three unmetered return false paths, so an active fence window is indistinguishable from a broken write path on dashboards (the same telemetry-gap shape as #141, which #143 fixed for the shadow side only). One new MetricErrorKind (fenced_refill_skipped — precedent: the advisory tracked_ttl_clamped) plus a typed 'dispatched' | 'fenced' | 'abandoned' return also removes the hidden invariant the shadow fill_fenced ternary depends on (sticky abandonment re-checked before the ternary). Extending MetricErrorKind is a type-surface change for exhaustive consumers — cheapest before 0.23, compounding after.
• Decide the legacy-decoder question — decodeRedisFrame / decodeTrackedRedisFrame are null-collapsing wrappers with zero in-repo consumers, kept for hypothetical external adapters; they drifted twice within this PR's own history before being re-pinned. Either delete them in this feat! window (they shipped in 0.22.0 but are unadopted; house policy is delete-don't-deprecate) or explicitly commit to the four-decoders-in-lockstep burden. Default-by-inaction is the worst option.
• Two one-liners: re-export CacheMissReason from dialcache/redis-protocol (it types RedisReadMiss.reason, which IS exported there; test-package.mjs currently has to root-import it), and a cross-reference comment at the deser site + one README table clause recording that local TTL expiry → value_absent vs remote logical expiry → unclassified is a deliberate evidence-based choice (and that internal RedisCacheMissReason is a disposition enum, not the metric reason) — so nobody "fixes" it later with another breaking metrics change.
• Optional hardening: make put's fence parameter a required RedisWatermarkMiss | undefined so any future fill path must decide explicitly instead of silently bypassing the fence.

Deliberate repetition — do NOT DRY these

• validateReadResult's reconstruction of adapter results is the single untrusted-input choke point, not redundant validation — keep it (with the union unification it gets simpler, not removed).
• The per-adapter label spreads ({...cacheLabels(labels), reason} in prometheus.ts/datadog.ts) are deliberate per-metric cardinality allowlists, consistent with disabled/error; prom-client rejects unknown labels, so wholesale pass-through isn't viable.
• FakeRedis's hand-rolled encodeFrame/decodeFrame test fixtures exist to construct malformed frames the real encoder refuses — that independence is the point (its read path already delegates to the real decoders, which is the right split).

Suggested packaging: items 1–4 + the one-liners as one refactor(redis): single-owner invariants for miss classification PR, the fence-skip metric as its own small feat(metrics) (it extends a public union), and the legacy-decoder deletion as a standalone commit gated on the adoption decision — all before the 0.23 release so every type-shape change rides the same pre-adoption window. Total estimated effort excluding the decoder decision: about half a day.

[lan17]

Summary

Implements #145 (replaces the stale attempt in #130).

Every DialCache miss event now says why it missed. Until now, a miss storm during a future-buffer invalidation window was indistinguishable from a cold cache or an eviction problem: the miss counter only told you that a layer missed, never whether the value was truly absent or a stored frame was being rejected by an invalidation watermark. This PR adds one required, bounded reason to the existing miss event — no new metric instrument, no new Redis command, no extra round trip, and no change to serving, refill, invalidation, stale-recovery, or shadow behavior.

What changes in your metrics

miss events now carry reason alongside the existing cacheNamespace / useCase / keyType / layer labels, on every layer (request_local, local, remote, remote_shadow):

reason	Fires when	Typical cause
value_absent	The layer had no retrievable value at all: never populated, physically expired or evicted, Redis nil, or a tracked-MGET wrong-type member (which Redis reports as nil). Request-local and process-local misses are always this.	Cold keys, physical TTL expiry, eviction pressure.
expired	A complete, supported frame with a valid, non-future timestamp was present, but its logical age against the reader's clock reached the effective remote TTL F. Covers age >= M misses and F <= age < M frames retained as stale-on-error candidates, on both caller and shadow reads.	Ordinary TTL churn on hot keys; the routine steady-state miss reason whenever staleOnErrorMaxAgeSec is configured.
watermark_fenced	A complete, supported tracked frame with a positive safe-integer timestamp was rejected because its createdAtMs was at or below a valid observed invalidation watermark. Decided before deserialization.	Reads inside an active futureBufferMs invalidation window; writer/invalidator clock skew.
unclassified	The miss is real but attributable to none of the above: unrecognized custom-adapter results (including null), short/unsupported frames, zero-stamped tracked frames, malformed watermark metadata paired with a present frame, future or invalid timestamps, and deserialization failures.	Un-migrated custom adapters; protocol-edge states.

The operational win: during an invalidation window you can now separate fence churn from genuine absence at a glance; routine TTL churn (including the stale-on-error F..M window, where every read of a retained frame is a miss) is expired rather than noise in the other buckets; and because expiry no longer lands there, a sudden unclassified plateau genuinely points at protocol-edge states (skewed clocks, corrupt frames, deserialization failures, legacy adapters) instead of hiding inside the aggregate.

Deviation from #145: the issue specified a three-value union and named unclassified as the safe default for stale-on-error retained frames "unless a separately justified reason is added". Review found that with staleOnErrorMaxAgeSec configured every read of a retained F..M frame is a miss, so unclassified would have been the routine steady-state reason for exactly the use cases that enable stale-on-error, and the plateau signal above would not hold. expired is that separately justified reason; the union is four values and the cardinality note below is 4x accordingly.

Prometheus — dialcache_miss_counter keeps its name and gains reason as a fifth label:

# Fence churn by use case during an invalidation window
sum by (use_case) (rate(dialcache_miss_counter{layer="remote", reason="watermark_fenced"}[5m]))
# Pre-existing total-miss and hit-ratio queries: aggregate reason away
sum by (cache_namespace, use_case, key_type, layer) (rate(dialcache_miss_counter[5m]))

Datadog — dialcache.miss.count keeps its name and gains the same reason tag, e.g. sum:dialcache.miss.count{reason:watermark_fenced} by {use_case}. Expect up to 4× the miss-series cardinality (four bounded reasons; only remote and remote_shadow tuples can carry every reason, since request-local and local misses are always value_absent).

Migration

• Prometheus registries: the label set changed, so registering against a registry that already holds a 4-label dialcache_miss_counter (e.g. a not-yet-upgraded sidecar library) fails loudly at construction — upgrade producers together per registry. Nothing silently mislabels.
• Mixed-fleet rollouts: old processes emit reason-less series while new ones carry reason. Total-miss and miss/request-ratio queries must sum by (...) the shared labels (example above); reason-aware dashboards should group by reason explicitly.
• Custom metrics adapters:DialCacheMetricsAdapter.miss now receives MissMetricLabels (extends the unchanged CacheMetricLabels with required reason: CacheMissReason). Adapters whose miss parameter is typed as the broader CacheMetricLabels compile unchanged and may ignore the field; exact label snapshots, exhaustive Records over reasons, and adapters that reject or forward unknown fields must add it.
• Custom Redis adapters:RedisReadResult is now DecodedRedisFrame | RedisReadMiss { kind: "miss", reason, observedWatermarkMs? }. null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed; use decodeRedisReadResult (untracked) / decodeTrackedRedisReadResult (tracked) and the exported isRedisReadMiss guard. An un-migrated adapter still fails open at runtime (null or any unrecognized result is an unclassified miss with a normal refill) but loses reason precision and fenced-refill suppression until it returns the typed miss.

How classification works

Bundled decoders return one miss shape, RedisReadMiss { kind: "miss", reason, observedWatermarkMs? }, attaching observedWatermarkMs only when the same atomic tracked snapshot carried a valid numeric watermark. Cause and fence are deliberately independent: Redis nil is decisive evidence of absence, so an absent value reports value_absent while still carrying the observed watermark that lets PR 143's two-sample admission skip a known-fenced refill — only a complete supported frame actually rejected by the watermark reports watermark_fenced.

Core treats the adapter boundary as untrusted and normalizes every result once, at one choke point: kind: "miss" is the only discriminator; invalid or missing reasons become unclassified, a watermark_fenced claim without a valid tracked fence is demoted to unclassified, invalid fences (NaN, negatives, non-integers, untracked keys) are dropped, and null, undefined, or any other unrecognized value is an unclassified miss with a normal refill. Objects without kind: "miss" are frame candidates and go through the existing timestamp and serializer validation.

Decoded hits keep the existing { payload, createdAtMs } shape. Core-side rejections never acquire a refill fence: logical-age rejections emit expired, future timestamps and deserialization failures emit unclassified; conditional refill suppression remains exactly PR 143's adapter-level watermark-miss path, byte-compatible with main (differential-verified across the decode input matrix).

Breaking change

DialCacheMetricsAdapter.miss now receives MissMetricLabels, and first-party miss metrics require a reason label/tag. RedisReadResult is now DecodedRedisFrame | RedisReadMiss; null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed, and bundled Redis adapters return RedisReadMiss for every semantic miss.

Under the pre-1.0 release policy, this is a minor release.

Validation

• Node.js 22.22.0: corepack pnpm check
  • typecheck
  • 656 unit tests with coverage thresholds
  • ESM/CJS build and declaration generation
  • packed-package type and runtime checks
• Node.js 22.22.0: corepack pnpm test:integration
  • 143 passed
  • 2 expected local GLIDE Cluster skips because announced container IPs are not host-routable
• Review follow-up (fix(metrics): preserve existing refill and decoder behavior) removed the fence-carry scope creep and restored decoder/refill parity with main, differential-verified across the decode input matrix

BREAKING CHANGE: DialCacheMetricsAdapter.miss now receives MissMetricLabels and first-party miss metrics require a reason label/tag; RedisReadResult is now DecodedRedisFrame | RedisReadMiss ({ kind: "miss", reason, observedWatermarkMs? }); null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed, and bundled Redis adapters return RedisReadMiss for every semantic miss.

[lan17]

Verdict: request changes(recorded as a comment — GitHub refuses a requested-changes review on the author's own PR)

Reviewed at max effort (10 finder angles, adversarial verification, differential decode matrices up to 280 cells against main, plus a gap sweep). Typecheck and all 645 unit tests pass at 83e8a7c; the classification logic itself held up everywhere it was attacked: fence boundaries exact, zero-watermark/zero-timestamp edges probe-verified, no unbounded label can reach a metrics adapter, Prometheus re-registration fails loudly rather than mislabeling.

Requesting changes on three items. Each is cheap now and compounds after merge, because this PR freezes both the migration documentation and the public custom-adapter contract.

1. Align the docs with the fence extension (README.md:981, README.md:724, README.md:~485)

Threading observedWatermarkMs through decoded hit frames makes this more than a metrics migration: refills after core-side rejections (retained stale-on-error candidates, logical-age expiry, future-dated frames, deserialization failures) are now watermark-fenced — your own tests pin write suppression at candidate = watermark+0 on the retained path, fill_fenced where filled was emitted, and core-supplied createdAtMs on writes that never carried it. The behavior is good (verification showed every suppressed write would have been permanently unreadable, and the retained-path skip preserves the stale-on-error candidate the old refill destroyed) — but README line 981 still says this migration "does not change cache serving, refill, invalidation, stale recovery, or shadow-verdict behavior", which is now false, and issue #145 scoped behavior changes out. The PR body has been updated with a "Behavior changes" section; the README needs the matching fix:

• Rewrite the line-981 claim to scope it to what actually holds (serving verdicts, invalidation, stale-recovery classification, shadow match/mismatch/superseded verdicts).
• Line 724: "createdAtMs <= watermark is watermark_fenced" needs the zero-timestamp carve-out (a complete frame stamped 0 under a valid watermark classifies unclassified, probe-verified).

2. Give RedisReadMiss a discriminant (src/redis-client.ts:92)

RedisReadMiss has no kind, so isRedisReadMiss falls back to in-presence checks — reintroducing the exact hazard PR 143 added kind: "watermark_miss" to kill. Probe-verified: { reason: "value_absent", payload: undefined, createdAtMs: undefined } type-checks for strict consumers without exactOptionalPropertyTypes (tsc exit 0) and is misrouted as a frame at runtime, silently degrading every one of that adapter's declared reasons to unclassified — the feature defeated with no signal. Adding kind: "read_miss" pre-merge is free; after adapters adopt, it's another breaking change.

Stronger option worth 30 minutes while you're here: one discriminated miss type { kind; reason; observedWatermarkMs?: number }. That single change also collapses the twin constructors (redisReadMiss in redis-payload.ts:205 vs classifiedRedisReadMiss in redis-cache.ts:660), deletes the never-released-compat reason?: optionality (redis-client.ts:108 — no tag contains PR 143, so the optionality serves zero consumers) and its missReason() ceremony, and halves the 4× fence ternaries in getWithResolvedConfig.

3. Document the hit-shape widening (README Serialization section + migration list)

Bundled tracked hits now return { payload, createdAtMs, observedWatermarkMs } whenever a valid watermark exists (including 0). Your own exact-shape tests had to change (test/node-redis.test.ts, test/valkey-glide.test.ts) — consumers' will too. The README still describes DecodedRedisFrame as payload + createdAtMs only, and the migration list omits it. One sentence plus one bullet. (The PR body and BREAKING CHANGE footer now carry it.)

---

Non-blocking, comment-tier (take or leave; several vanish if you take the unified miss type):

• Corrupt-timestamp tracked frames are now silently reclassified at decode (redis-payload.ts:186): the legacy decodeTrackedRedisFrame returns null where main returned a frame or threw DialCacheRedisPayloadEncodingError (18 differential cells), the doubly-corrupt class loses its alertable error signal, and tracked vs untracked decoders now disagree on the same corrupt frame. Document as intended hardening or align the two.
• unclassifiedFrameMiss (redis-cache.ts:676) trusts an adapter-attached hit fence with envelope-only validation; a one-line observedWatermarkMs < frame.createdAtMs cross-check restores the pre-PR blast radius for contract-violating adapters.
• Taxonomy coherence: local TTL expiry reports value_absent while the same logical condition remotely reports unclassified; and the internal RedisCacheMissReason vs public CacheMissReason enums collide undocumented at the deser site (redis-cache.ts:184). Evidence-based and defensible — record the rationale in a comment/README clause so it reads as deliberate.
• validObservedWatermarkMs (redis-cache.ts:680) re-duplicates the safe-int≥0 envelope of isValidRedisTimestampMs (same finding as PR 143's isValidRedisWatermarkMiss, reincarnated) and runs twice per miss.
• isCacheMissReason (redis-cache.ts:686) has no exhaustiveness tie to the union — a future fourth reason compiles everywhere and silently demotes to unclassified at runtime. const CACHE_MISS_REASONS satisfies Record<CacheMissReason, true> beside the type fixes it.
• CacheMissReason isn't exported from the dialcache/redis-protocol subpath that exports RedisReadMiss — even scripts/test-package.mjs has to root-import it. One re-export line.
• Decode micro: readFrameCreatedAtMs runs twice per served tracked frame; decodedRedisFrame builds-then-spreads; the watermark-parse expression is duplicated in two branches (redis-payload.ts:139-158).
• Both legacy decoders are now zero-consumer wrappers, and this diff grows decodeTrackedRedisFrame with a strip projection. They were released in 0.22.0, so deleting is your policy call — this feat! is the natural moment.

Credit where due: the FakeRedis delegation to the real decoders kills the untracked-fence divergence from the PR 143 review, the fence-carry closes the core-side-rejection gap that PR 143 deliberately deferred, and the invalid-watermark normalization matrix and boundary tests are exactly the coverage this layer needs.

One revision pass and this is an easy approve.

[lan17]

Verdict: approve(recorded as a comment — GitHub refuses a formal review state on the author's own PR)

Re-reviewed at dc6199d (fix(metrics): preserve existing refill and decoder behavior). All three requested changes from the previous review are resolved; typecheck is clean, 653/653 unit tests pass, and a fresh sweep over the fix commit found nothing new.

Requested changes — all addressed

1. Scope / README behavior claim — fixed by reversion. The fence-carry is fully removed (DecodedRedisFrame.observedWatermarkMs, unclassifiedFrameMiss, the retained-path carry, and the validateFrameAge miss objects are gone). A 216-cell decoder differential against main shows byte-parity on the legacy decodeRedisFrame/decodeTrackedRedisFrame surfaces and zero hit/fence/throw drift on the classified decoders (only the added reason labels differ). The rewritten README claim — "Miss classification adds no new refill-fencing paths and preserves serving eligibility, invalidation, stale-recovery policy, and shadow outcomes" — is verified true: fencing occurs solely via PR 143's adapter-level watermark-miss path. This also moots the adapter-fence-trust hardening item, since frames no longer carry fences at all.
2. RedisReadMiss guard hazard — fixed via value-based guards. Both isRedisReadMiss and isRedisWatermarkMiss now treat explicitly-undefinedpayload/createdAtMs as absent, closing the misclassification for the new miss type and the PR-143-era residual on the kinded miss; probe coverage landed in the unit tests and the packed-package script (including the @ts-expect-error unbounded-reason negative). Guard value-reads are short-circuited behind the reason/kind presence checks and every classified miss is canonicalized inside validateReadResult, so no new throw path escapes unrecorded.
3. Hit-shape documentation — moot and documented. The widened hit shape is deleted; the README now states explicitly that decoded hits keep { payload, createdAtMs }.

Also verified: the unsafe-timestamp reclassification is fully reverted (doubly-corrupt frames throw DialCacheRedisPayloadEncodingError again; only the zero-baseline check remains, matching main), and the zero-timestamp carve-out landed in both the invalidation section and the metrics-table watermark_fenced row. The PR body has been updated to match the reverted scope (behavior-parity statement, corrected test count, footer back to the two real breaks).

Deferred (non-blocking, unchanged from the previous review)

Twin miss constructors (redisReadMiss / classifiedRedisReadMiss), the duplicated safe-int≥0 timestamp envelope in validObservedWatermarkMs, reason?: optionality serving a never-released shape, the non-exhaustive isCacheMissReason (a future fourth reason silently demotes to unclassified), CacheMissReason missing from the dialcache/redis-protocol exports, the local-vs-remote expired-entry taxonomy note, decode micro-items (double readFrameCreatedAtMs, duplicated watermark-parse expression), and the legacy-decoder deletion policy call. Several of these collapse together if the miss union is ever unified into one kinded type.

Good ship. The guard fix in particular is stronger than what was asked — it hardens both miss types instead of one.

[lan17]

Follow-up guidance: post-merge cleanup plan(non-blocking — the approve verdict above stands; this consolidates the deferred findings from both review rounds into an actionable plan for a single refactor: PR before 0.23 ships)

Two review rounds across #143/#146 kept finding one failure pattern: a correctness-critical invariant hand-written in more than one module, already divergent at birth (the three isRedisWatermarkMiss copies in #143 differed within one commit; the safe-int≥0 timestamp envelope has now been written three times). The cleanup principle is therefore one owner per invariant — not "no repeated lines". Some repetition here is deliberate and load-bearing; the last section lists what to leave alone.

DRY batch (descending leverage)

1. Unify the miss union into one kinded type — { kind, reason, observedWatermarkMs?: number }. Retires three deferred findings at once: the twin constructors (redisReadMiss in internal/redis-payload.ts vs classifiedRedisReadMiss in internal/redis-cache.ts — every bundled miss is currently built twice per read), the reason?: optionality that exists only for source compatibility with a shape that never shipped (no release contains #143's reason-less RedisWatermarkMiss, so missReason()'s undefined-handling at five sites serves nobody), and the composite isRedisReadMiss guard, which becomes a single kind check. Free now — none of this surface is released; expensive after adapter adoption. ~1 hour.
2. Exhaustiveness-tie the reason guard — isCacheMissReason (internal/redis-cache.ts) hand-lists the three strings with no compiler link to CacheMissReason. A future fourth reason compiles everywhere while validateReadResult silently demotes every adapter-supplied instance to unclassified — discovered in dashboards, not at build time. Fix: const CACHE_MISS_REASONS = { value_absent: true, watermark_fenced: true, unclassified: true } satisfies Record<CacheMissReason, true> beside the type, guard via Object.hasOwn (the exact pattern the tests and test-package.mjs already use).
3. One owner for the timestamp envelope — export boolean isValidRedisTimestampMs from internal/redis-payload.ts (widened to unknown) and define validObservedWatermarkMs on top of it. Prevents the envelope forking between bundled decoders and core validation of custom-adapter fences (e.g., if an upper bound is ever added so a garbage far-future watermark can't fence writes for years).
4. Move createdAtMs defaulting into the protocol — encodeRedisFrame(payload, createdAtMs?) defaulting to Date.now() internally (or one exported resolver). Today the "honor supplied value exactly, else sample" rule is a ternary copy-pasted into node-redis.ts and valkey-glide.ts and prescribed as README prose every custom adapter must transcribe; the natural mis-spelling request.createdAtMs || Date.now() silently breaks the fence-decision/stored-frame coupling at createdAtMs: 0. Make the correct behavior the only expressible one.
5. Small fry, batch opportunistically: hoist the duplicated watermarkFrame === null ? undefined : parseRedisWatermark(watermarkFrame) ?? undefined expression in decodeTrackedRedisReadResult; pass the already-read createdAtMs into decodedRedisFrame (removes the double readBigUInt64BE per served tracked hit); merge the two adjacent miss literals in getWithResolvedConfig; dedupe the README's twice-pasted "Conditional refill suppression reuses the existing tracked MGET result..." sentence (network-shape §, keep; ACL §, drop). Tests: fold the ~52-line ESM/CJS classified-decode paste in scripts/test-package.mjs into its existing shared-check mechanism (verifyPackedInvalidation precedent), and extract the thrice-copied throwing-serializer fixture.

Not DRY, but higher value than most of the above

• Meter the caller-path fence skip — putWithLayer still has three unmetered return false paths, so an active fence window is indistinguishable from a broken write path on dashboards (the same telemetry-gap shape as #141, which #143 fixed for the shadow side only). One new MetricErrorKind (fenced_refill_skipped — precedent: the advisory tracked_ttl_clamped) plus a typed 'dispatched' | 'fenced' | 'abandoned' return also removes the hidden invariant the shadow fill_fenced ternary depends on (sticky abandonment re-checked before the ternary). Extending MetricErrorKind is a type-surface change for exhaustive consumers — cheapest before 0.23, compounding after.
• Decide the legacy-decoder question — decodeRedisFrame / decodeTrackedRedisFrame are null-collapsing wrappers with zero in-repo consumers, kept for hypothetical external adapters; they drifted twice within this PR's own history before being re-pinned. Either delete them in this feat! window (they shipped in 0.22.0 but are unadopted; house policy is delete-don't-deprecate) or explicitly commit to the four-decoders-in-lockstep burden. Default-by-inaction is the worst option.
• Two one-liners: re-export CacheMissReason from dialcache/redis-protocol (it types RedisReadMiss.reason, which IS exported there; test-package.mjs currently has to root-import it), and a cross-reference comment at the deser site + one README table clause recording that local TTL expiry → value_absent vs remote logical expiry → unclassified is a deliberate evidence-based choice (and that internal RedisCacheMissReason is a disposition enum, not the metric reason) — so nobody "fixes" it later with another breaking metrics change.
• Optional hardening: make put's fence parameter a required RedisWatermarkMiss | undefined so any future fill path must decide explicitly instead of silently bypassing the fence.

Deliberate repetition — do NOT DRY these

• validateReadResult's reconstruction of adapter results is the single untrusted-input choke point, not redundant validation — keep it (with the union unification it gets simpler, not removed).
• The per-adapter label spreads ({...cacheLabels(labels), reason} in prometheus.ts/datadog.ts) are deliberate per-metric cardinality allowlists, consistent with disabled/error; prom-client rejects unknown labels, so wholesale pass-through isn't viable.
• FakeRedis's hand-rolled encodeFrame/decodeFrame test fixtures exist to construct malformed frames the real encoder refuses — that independence is the point (its read path already delegates to the real decoders, which is the right split).

Suggested packaging: items 1–4 + the one-liners as one refactor(redis): single-owner invariants for miss classification PR, the fence-skip metric as its own small feat(metrics) (it extends a public union), and the legacy-decoder deletion as a standalone commit gated on the adoption decision — all before the 0.23 release so every type-shape change rides the same pre-adoption window. Total estimated effort excluding the decoder decision: about half a day.

[lan17]

Summary

Implements #145 (replaces the stale attempt in #130).

Every DialCache miss event now says why it missed. Until now, a miss storm during a future-buffer invalidation window was indistinguishable from a cold cache or an eviction problem: the miss counter only told you that a layer missed, never whether the value was truly absent or a stored frame was being rejected by an invalidation watermark. This PR adds one required, bounded reason to the existing miss event — no new metric instrument, no new Redis command, no extra round trip, and no change to serving, refill, invalidation, stale-recovery, or shadow behavior.

What changes in your metrics

miss events now carry reason alongside the existing cacheNamespace / useCase / keyType / layer labels, on every layer (request_local, local, remote, remote_shadow):

reason	Fires when	Typical cause
value_absent	The layer had no retrievable value at all: never populated, physically expired or evicted, Redis nil, or a tracked-MGET wrong-type member (which Redis reports as nil). Request-local and process-local misses are always this.	Cold keys, physical TTL expiry, eviction pressure.
expired	A complete, supported frame with a valid, non-future timestamp was present, but its logical age against the reader's clock reached the effective remote TTL F. Covers age >= M misses and F <= age < M frames retained as stale-on-error candidates, on both caller and shadow reads.	Ordinary TTL churn on hot keys; the routine steady-state miss reason whenever staleOnErrorMaxAgeSec is configured.
watermark_fenced	A complete, supported tracked frame with a positive safe-integer timestamp was rejected because its createdAtMs was at or below a valid observed invalidation watermark. Decided before deserialization.	Reads inside an active futureBufferMs invalidation window; writer/invalidator clock skew.
unclassified	The miss is real but attributable to none of the above: unrecognized custom-adapter results (including null), short/unsupported frames, zero-stamped tracked frames, malformed watermark metadata paired with a present frame, future or invalid timestamps, and deserialization failures.	Un-migrated custom adapters; protocol-edge states.

The operational win: during an invalidation window you can now separate fence churn from genuine absence at a glance; routine TTL churn (including the stale-on-error F..M window, where every read of a retained frame is a miss) is expired rather than noise in the other buckets; and because expiry no longer lands there, a sudden unclassified plateau genuinely points at protocol-edge states (skewed clocks, corrupt frames, deserialization failures, legacy adapters) instead of hiding inside the aggregate.

Deviation from #145: the issue specified a three-value union and named unclassified as the safe default for stale-on-error retained frames "unless a separately justified reason is added". Review found that with staleOnErrorMaxAgeSec configured every read of a retained F..M frame is a miss, so unclassified would have been the routine steady-state reason for exactly the use cases that enable stale-on-error, and the plateau signal above would not hold. expired is that separately justified reason; the union is four values and the cardinality note below is 4x accordingly.

Prometheus — dialcache_miss_counter keeps its name and gains reason as a fifth label:

# Fence churn by use case during an invalidation window
sum by (use_case) (rate(dialcache_miss_counter{layer="remote", reason="watermark_fenced"}[5m]))
# Pre-existing total-miss and hit-ratio queries: aggregate reason away
sum by (cache_namespace, use_case, key_type, layer) (rate(dialcache_miss_counter[5m]))

Datadog — dialcache.miss.count keeps its name and gains the same reason tag, e.g. sum:dialcache.miss.count{reason:watermark_fenced} by {use_case}. Expect up to 4× the miss-series cardinality (four bounded reasons; only remote and remote_shadow tuples can carry every reason, since request-local and local misses are always value_absent).

Migration

• Prometheus registries: the label set changed, so registering against a registry that already holds a 4-label dialcache_miss_counter (e.g. a not-yet-upgraded sidecar library) fails loudly at construction — upgrade producers together per registry. Nothing silently mislabels.
• Mixed-fleet rollouts: old processes emit reason-less series while new ones carry reason. Total-miss and miss/request-ratio queries must sum by (...) the shared labels (example above); reason-aware dashboards should group by reason explicitly.
• Custom metrics adapters:DialCacheMetricsAdapter.miss now receives MissMetricLabels (extends the unchanged CacheMetricLabels with required reason: CacheMissReason). Adapters whose miss parameter is typed as the broader CacheMetricLabels compile unchanged and may ignore the field; exact label snapshots, exhaustive Records over reasons, and adapters that reject or forward unknown fields must add it.
• Custom Redis adapters:RedisReadResult is now DecodedRedisFrame | RedisReadMiss { kind: "miss", reason, observedWatermarkMs? }. null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed; use decodeRedisReadResult (untracked) / decodeTrackedRedisReadResult (tracked) and the exported isRedisReadMiss guard. An un-migrated adapter still fails open at runtime (null or any unrecognized result is an unclassified miss with a normal refill) but loses reason precision and fenced-refill suppression until it returns the typed miss.

How classification works

Bundled decoders return one miss shape, RedisReadMiss { kind: "miss", reason, observedWatermarkMs? }, attaching observedWatermarkMs only when the same atomic tracked snapshot carried a valid numeric watermark. Cause and fence are deliberately independent: Redis nil is decisive evidence of absence, so an absent value reports value_absent while still carrying the observed watermark that lets PR 143's two-sample admission skip a known-fenced refill — only a complete supported frame actually rejected by the watermark reports watermark_fenced.

Core treats the adapter boundary as untrusted and normalizes every result once, at one choke point: kind: "miss" is the only discriminator; invalid or missing reasons become unclassified, a watermark_fenced claim without a valid tracked fence is demoted to unclassified, invalid fences (NaN, negatives, non-integers, untracked keys) are dropped, and null, undefined, or any other unrecognized value is an unclassified miss with a normal refill. Objects without kind: "miss" are frame candidates and go through the existing timestamp and serializer validation.

Decoded hits keep the existing { payload, createdAtMs } shape. Core-side rejections never acquire a refill fence: logical-age rejections emit expired, future timestamps and deserialization failures emit unclassified; conditional refill suppression remains exactly PR 143's adapter-level watermark-miss path, byte-compatible with main (differential-verified across the decode input matrix).

Breaking change

DialCacheMetricsAdapter.miss now receives MissMetricLabels, and first-party miss metrics require a reason label/tag. RedisReadResult is now DecodedRedisFrame | RedisReadMiss; null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed, and bundled Redis adapters return RedisReadMiss for every semantic miss.

Under the pre-1.0 release policy, this is a minor release.

Validation

• Node.js 22.22.0: corepack pnpm check
  • typecheck
  • 656 unit tests with coverage thresholds
  • ESM/CJS build and declaration generation
  • packed-package type and runtime checks
• Node.js 22.22.0: corepack pnpm test:integration
  • 143 passed
  • 2 expected local GLIDE Cluster skips because announced container IPs are not host-routable
• Review follow-up (fix(metrics): preserve existing refill and decoder behavior) removed the fence-carry scope creep and restored decoder/refill parity with main, differential-verified across the decode input matrix

BREAKING CHANGE: DialCacheMetricsAdapter.miss now receives MissMetricLabels and first-party miss metrics require a reason label/tag; RedisReadResult is now DecodedRedisFrame | RedisReadMiss ({ kind: "miss", reason, observedWatermarkMs? }); null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed, and bundled Redis adapters return RedisReadMiss for every semantic miss.

[lan17]

Verdict: request changes(recorded as a comment — GitHub refuses a requested-changes review on the author's own PR)

Reviewed at max effort (10 finder angles, adversarial verification, differential decode matrices up to 280 cells against main, plus a gap sweep). Typecheck and all 645 unit tests pass at 83e8a7c; the classification logic itself held up everywhere it was attacked: fence boundaries exact, zero-watermark/zero-timestamp edges probe-verified, no unbounded label can reach a metrics adapter, Prometheus re-registration fails loudly rather than mislabeling.

Requesting changes on three items. Each is cheap now and compounds after merge, because this PR freezes both the migration documentation and the public custom-adapter contract.

1. Align the docs with the fence extension (README.md:981, README.md:724, README.md:~485)

Threading observedWatermarkMs through decoded hit frames makes this more than a metrics migration: refills after core-side rejections (retained stale-on-error candidates, logical-age expiry, future-dated frames, deserialization failures) are now watermark-fenced — your own tests pin write suppression at candidate = watermark+0 on the retained path, fill_fenced where filled was emitted, and core-supplied createdAtMs on writes that never carried it. The behavior is good (verification showed every suppressed write would have been permanently unreadable, and the retained-path skip preserves the stale-on-error candidate the old refill destroyed) — but README line 981 still says this migration "does not change cache serving, refill, invalidation, stale recovery, or shadow-verdict behavior", which is now false, and issue #145 scoped behavior changes out. The PR body has been updated with a "Behavior changes" section; the README needs the matching fix:

• Rewrite the line-981 claim to scope it to what actually holds (serving verdicts, invalidation, stale-recovery classification, shadow match/mismatch/superseded verdicts).
• Line 724: "createdAtMs <= watermark is watermark_fenced" needs the zero-timestamp carve-out (a complete frame stamped 0 under a valid watermark classifies unclassified, probe-verified).

2. Give RedisReadMiss a discriminant (src/redis-client.ts:92)

RedisReadMiss has no kind, so isRedisReadMiss falls back to in-presence checks — reintroducing the exact hazard PR 143 added kind: "watermark_miss" to kill. Probe-verified: { reason: "value_absent", payload: undefined, createdAtMs: undefined } type-checks for strict consumers without exactOptionalPropertyTypes (tsc exit 0) and is misrouted as a frame at runtime, silently degrading every one of that adapter's declared reasons to unclassified — the feature defeated with no signal. Adding kind: "read_miss" pre-merge is free; after adapters adopt, it's another breaking change.

Stronger option worth 30 minutes while you're here: one discriminated miss type { kind; reason; observedWatermarkMs?: number }. That single change also collapses the twin constructors (redisReadMiss in redis-payload.ts:205 vs classifiedRedisReadMiss in redis-cache.ts:660), deletes the never-released-compat reason?: optionality (redis-client.ts:108 — no tag contains PR 143, so the optionality serves zero consumers) and its missReason() ceremony, and halves the 4× fence ternaries in getWithResolvedConfig.

3. Document the hit-shape widening (README Serialization section + migration list)

Bundled tracked hits now return { payload, createdAtMs, observedWatermarkMs } whenever a valid watermark exists (including 0). Your own exact-shape tests had to change (test/node-redis.test.ts, test/valkey-glide.test.ts) — consumers' will too. The README still describes DecodedRedisFrame as payload + createdAtMs only, and the migration list omits it. One sentence plus one bullet. (The PR body and BREAKING CHANGE footer now carry it.)

---

Non-blocking, comment-tier (take or leave; several vanish if you take the unified miss type):

• Corrupt-timestamp tracked frames are now silently reclassified at decode (redis-payload.ts:186): the legacy decodeTrackedRedisFrame returns null where main returned a frame or threw DialCacheRedisPayloadEncodingError (18 differential cells), the doubly-corrupt class loses its alertable error signal, and tracked vs untracked decoders now disagree on the same corrupt frame. Document as intended hardening or align the two.
• unclassifiedFrameMiss (redis-cache.ts:676) trusts an adapter-attached hit fence with envelope-only validation; a one-line observedWatermarkMs < frame.createdAtMs cross-check restores the pre-PR blast radius for contract-violating adapters.
• Taxonomy coherence: local TTL expiry reports value_absent while the same logical condition remotely reports unclassified; and the internal RedisCacheMissReason vs public CacheMissReason enums collide undocumented at the deser site (redis-cache.ts:184). Evidence-based and defensible — record the rationale in a comment/README clause so it reads as deliberate.
• validObservedWatermarkMs (redis-cache.ts:680) re-duplicates the safe-int≥0 envelope of isValidRedisTimestampMs (same finding as PR 143's isValidRedisWatermarkMiss, reincarnated) and runs twice per miss.
• isCacheMissReason (redis-cache.ts:686) has no exhaustiveness tie to the union — a future fourth reason compiles everywhere and silently demotes to unclassified at runtime. const CACHE_MISS_REASONS satisfies Record<CacheMissReason, true> beside the type fixes it.
• CacheMissReason isn't exported from the dialcache/redis-protocol subpath that exports RedisReadMiss — even scripts/test-package.mjs has to root-import it. One re-export line.
• Decode micro: readFrameCreatedAtMs runs twice per served tracked frame; decodedRedisFrame builds-then-spreads; the watermark-parse expression is duplicated in two branches (redis-payload.ts:139-158).
• Both legacy decoders are now zero-consumer wrappers, and this diff grows decodeTrackedRedisFrame with a strip projection. They were released in 0.22.0, so deleting is your policy call — this feat! is the natural moment.

Credit where due: the FakeRedis delegation to the real decoders kills the untracked-fence divergence from the PR 143 review, the fence-carry closes the core-side-rejection gap that PR 143 deliberately deferred, and the invalid-watermark normalization matrix and boundary tests are exactly the coverage this layer needs.

One revision pass and this is an easy approve.

[lan17]

Verdict: approve(recorded as a comment — GitHub refuses a formal review state on the author's own PR)

Re-reviewed at dc6199d (fix(metrics): preserve existing refill and decoder behavior). All three requested changes from the previous review are resolved; typecheck is clean, 653/653 unit tests pass, and a fresh sweep over the fix commit found nothing new.

Requested changes — all addressed

1. Scope / README behavior claim — fixed by reversion. The fence-carry is fully removed (DecodedRedisFrame.observedWatermarkMs, unclassifiedFrameMiss, the retained-path carry, and the validateFrameAge miss objects are gone). A 216-cell decoder differential against main shows byte-parity on the legacy decodeRedisFrame/decodeTrackedRedisFrame surfaces and zero hit/fence/throw drift on the classified decoders (only the added reason labels differ). The rewritten README claim — "Miss classification adds no new refill-fencing paths and preserves serving eligibility, invalidation, stale-recovery policy, and shadow outcomes" — is verified true: fencing occurs solely via PR 143's adapter-level watermark-miss path. This also moots the adapter-fence-trust hardening item, since frames no longer carry fences at all.
2. RedisReadMiss guard hazard — fixed via value-based guards. Both isRedisReadMiss and isRedisWatermarkMiss now treat explicitly-undefinedpayload/createdAtMs as absent, closing the misclassification for the new miss type and the PR-143-era residual on the kinded miss; probe coverage landed in the unit tests and the packed-package script (including the @ts-expect-error unbounded-reason negative). Guard value-reads are short-circuited behind the reason/kind presence checks and every classified miss is canonicalized inside validateReadResult, so no new throw path escapes unrecorded.
3. Hit-shape documentation — moot and documented. The widened hit shape is deleted; the README now states explicitly that decoded hits keep { payload, createdAtMs }.

Also verified: the unsafe-timestamp reclassification is fully reverted (doubly-corrupt frames throw DialCacheRedisPayloadEncodingError again; only the zero-baseline check remains, matching main), and the zero-timestamp carve-out landed in both the invalidation section and the metrics-table watermark_fenced row. The PR body has been updated to match the reverted scope (behavior-parity statement, corrected test count, footer back to the two real breaks).

Deferred (non-blocking, unchanged from the previous review)

Twin miss constructors (redisReadMiss / classifiedRedisReadMiss), the duplicated safe-int≥0 timestamp envelope in validObservedWatermarkMs, reason?: optionality serving a never-released shape, the non-exhaustive isCacheMissReason (a future fourth reason silently demotes to unclassified), CacheMissReason missing from the dialcache/redis-protocol exports, the local-vs-remote expired-entry taxonomy note, decode micro-items (double readFrameCreatedAtMs, duplicated watermark-parse expression), and the legacy-decoder deletion policy call. Several of these collapse together if the miss union is ever unified into one kinded type.

Good ship. The guard fix in particular is stronger than what was asked — it hardens both miss types instead of one.

[lan17]

Follow-up guidance: post-merge cleanup plan(non-blocking — the approve verdict above stands; this consolidates the deferred findings from both review rounds into an actionable plan for a single refactor: PR before 0.23 ships)

Two review rounds across #143/#146 kept finding one failure pattern: a correctness-critical invariant hand-written in more than one module, already divergent at birth (the three isRedisWatermarkMiss copies in #143 differed within one commit; the safe-int≥0 timestamp envelope has now been written three times). The cleanup principle is therefore one owner per invariant — not "no repeated lines". Some repetition here is deliberate and load-bearing; the last section lists what to leave alone.

DRY batch (descending leverage)

1. Unify the miss union into one kinded type — { kind, reason, observedWatermarkMs?: number }. Retires three deferred findings at once: the twin constructors (redisReadMiss in internal/redis-payload.ts vs classifiedRedisReadMiss in internal/redis-cache.ts — every bundled miss is currently built twice per read), the reason?: optionality that exists only for source compatibility with a shape that never shipped (no release contains #143's reason-less RedisWatermarkMiss, so missReason()'s undefined-handling at five sites serves nobody), and the composite isRedisReadMiss guard, which becomes a single kind check. Free now — none of this surface is released; expensive after adapter adoption. ~1 hour.
2. Exhaustiveness-tie the reason guard — isCacheMissReason (internal/redis-cache.ts) hand-lists the three strings with no compiler link to CacheMissReason. A future fourth reason compiles everywhere while validateReadResult silently demotes every adapter-supplied instance to unclassified — discovered in dashboards, not at build time. Fix: const CACHE_MISS_REASONS = { value_absent: true, watermark_fenced: true, unclassified: true } satisfies Record<CacheMissReason, true> beside the type, guard via Object.hasOwn (the exact pattern the tests and test-package.mjs already use).
3. One owner for the timestamp envelope — export boolean isValidRedisTimestampMs from internal/redis-payload.ts (widened to unknown) and define validObservedWatermarkMs on top of it. Prevents the envelope forking between bundled decoders and core validation of custom-adapter fences (e.g., if an upper bound is ever added so a garbage far-future watermark can't fence writes for years).
4. Move createdAtMs defaulting into the protocol — encodeRedisFrame(payload, createdAtMs?) defaulting to Date.now() internally (or one exported resolver). Today the "honor supplied value exactly, else sample" rule is a ternary copy-pasted into node-redis.ts and valkey-glide.ts and prescribed as README prose every custom adapter must transcribe; the natural mis-spelling request.createdAtMs || Date.now() silently breaks the fence-decision/stored-frame coupling at createdAtMs: 0. Make the correct behavior the only expressible one.
5. Small fry, batch opportunistically: hoist the duplicated watermarkFrame === null ? undefined : parseRedisWatermark(watermarkFrame) ?? undefined expression in decodeTrackedRedisReadResult; pass the already-read createdAtMs into decodedRedisFrame (removes the double readBigUInt64BE per served tracked hit); merge the two adjacent miss literals in getWithResolvedConfig; dedupe the README's twice-pasted "Conditional refill suppression reuses the existing tracked MGET result..." sentence (network-shape §, keep; ACL §, drop). Tests: fold the ~52-line ESM/CJS classified-decode paste in scripts/test-package.mjs into its existing shared-check mechanism (verifyPackedInvalidation precedent), and extract the thrice-copied throwing-serializer fixture.

Not DRY, but higher value than most of the above

• Meter the caller-path fence skip — putWithLayer still has three unmetered return false paths, so an active fence window is indistinguishable from a broken write path on dashboards (the same telemetry-gap shape as #141, which #143 fixed for the shadow side only). One new MetricErrorKind (fenced_refill_skipped — precedent: the advisory tracked_ttl_clamped) plus a typed 'dispatched' | 'fenced' | 'abandoned' return also removes the hidden invariant the shadow fill_fenced ternary depends on (sticky abandonment re-checked before the ternary). Extending MetricErrorKind is a type-surface change for exhaustive consumers — cheapest before 0.23, compounding after.
• Decide the legacy-decoder question — decodeRedisFrame / decodeTrackedRedisFrame are null-collapsing wrappers with zero in-repo consumers, kept for hypothetical external adapters; they drifted twice within this PR's own history before being re-pinned. Either delete them in this feat! window (they shipped in 0.22.0 but are unadopted; house policy is delete-don't-deprecate) or explicitly commit to the four-decoders-in-lockstep burden. Default-by-inaction is the worst option.
• Two one-liners: re-export CacheMissReason from dialcache/redis-protocol (it types RedisReadMiss.reason, which IS exported there; test-package.mjs currently has to root-import it), and a cross-reference comment at the deser site + one README table clause recording that local TTL expiry → value_absent vs remote logical expiry → unclassified is a deliberate evidence-based choice (and that internal RedisCacheMissReason is a disposition enum, not the metric reason) — so nobody "fixes" it later with another breaking metrics change.
• Optional hardening: make put's fence parameter a required RedisWatermarkMiss | undefined so any future fill path must decide explicitly instead of silently bypassing the fence.

Deliberate repetition — do NOT DRY these

• validateReadResult's reconstruction of adapter results is the single untrusted-input choke point, not redundant validation — keep it (with the union unification it gets simpler, not removed).
• The per-adapter label spreads ({...cacheLabels(labels), reason} in prometheus.ts/datadog.ts) are deliberate per-metric cardinality allowlists, consistent with disabled/error; prom-client rejects unknown labels, so wholesale pass-through isn't viable.
• FakeRedis's hand-rolled encodeFrame/decodeFrame test fixtures exist to construct malformed frames the real encoder refuses — that independence is the point (its read path already delegates to the real decoders, which is the right split).

Suggested packaging: items 1–4 + the one-liners as one refactor(redis): single-owner invariants for miss classification PR, the fence-skip metric as its own small feat(metrics) (it extends a public union), and the legacy-decoder deletion as a standalone commit gated on the adoption decision — all before the 0.23 release so every type-shape change rides the same pre-adoption window. Total estimated effort excluding the decoder decision: about half a day.

[lan17]

Summary

Implements #145 (replaces the stale attempt in #130).

Every DialCache miss event now says why it missed. Until now, a miss storm during a future-buffer invalidation window was indistinguishable from a cold cache or an eviction problem: the miss counter only told you that a layer missed, never whether the value was truly absent or a stored frame was being rejected by an invalidation watermark. This PR adds one required, bounded reason to the existing miss event — no new metric instrument, no new Redis command, no extra round trip, and no change to serving, refill, invalidation, stale-recovery, or shadow behavior.

What changes in your metrics

miss events now carry reason alongside the existing cacheNamespace / useCase / keyType / layer labels, on every layer (request_local, local, remote, remote_shadow):

reason	Fires when	Typical cause
value_absent	The layer had no retrievable value at all: never populated, physically expired or evicted, Redis nil, or a tracked-MGET wrong-type member (which Redis reports as nil). Request-local and process-local misses are always this.	Cold keys, physical TTL expiry, eviction pressure.
expired	A complete, supported frame with a valid, non-future timestamp was present, but its logical age against the reader's clock reached the effective remote TTL F. Covers age >= M misses and F <= age < M frames retained as stale-on-error candidates, on both caller and shadow reads.	Ordinary TTL churn on hot keys; the routine steady-state miss reason whenever staleOnErrorMaxAgeSec is configured.
watermark_fenced	A complete, supported tracked frame with a positive safe-integer timestamp was rejected because its createdAtMs was at or below a valid observed invalidation watermark. Decided before deserialization.	Reads inside an active futureBufferMs invalidation window; writer/invalidator clock skew.
unclassified	The miss is real but attributable to none of the above: unrecognized custom-adapter results (including null), short/unsupported frames, zero-stamped tracked frames, malformed watermark metadata paired with a present frame, future or invalid timestamps, and deserialization failures.	Un-migrated custom adapters; protocol-edge states.

The operational win: during an invalidation window you can now separate fence churn from genuine absence at a glance; routine TTL churn (including the stale-on-error F..M window, where every read of a retained frame is a miss) is expired rather than noise in the other buckets; and because expiry no longer lands there, a sudden unclassified plateau genuinely points at protocol-edge states (skewed clocks, corrupt frames, deserialization failures, legacy adapters) instead of hiding inside the aggregate.

Deviation from #145: the issue specified a three-value union and named unclassified as the safe default for stale-on-error retained frames "unless a separately justified reason is added". Review found that with staleOnErrorMaxAgeSec configured every read of a retained F..M frame is a miss, so unclassified would have been the routine steady-state reason for exactly the use cases that enable stale-on-error, and the plateau signal above would not hold. expired is that separately justified reason; the union is four values and the cardinality note below is 4x accordingly.

Prometheus — dialcache_miss_counter keeps its name and gains reason as a fifth label:

# Fence churn by use case during an invalidation window
sum by (use_case) (rate(dialcache_miss_counter{layer="remote", reason="watermark_fenced"}[5m]))
# Pre-existing total-miss and hit-ratio queries: aggregate reason away
sum by (cache_namespace, use_case, key_type, layer) (rate(dialcache_miss_counter[5m]))

Datadog — dialcache.miss.count keeps its name and gains the same reason tag, e.g. sum:dialcache.miss.count{reason:watermark_fenced} by {use_case}. Expect up to 4× the miss-series cardinality (four bounded reasons; only remote and remote_shadow tuples can carry every reason, since request-local and local misses are always value_absent).

Migration

• Prometheus registries: the label set changed, so registering against a registry that already holds a 4-label dialcache_miss_counter (e.g. a not-yet-upgraded sidecar library) fails loudly at construction — upgrade producers together per registry. Nothing silently mislabels.
• Mixed-fleet rollouts: old processes emit reason-less series while new ones carry reason. Total-miss and miss/request-ratio queries must sum by (...) the shared labels (example above); reason-aware dashboards should group by reason explicitly.
• Custom metrics adapters:DialCacheMetricsAdapter.miss now receives MissMetricLabels (extends the unchanged CacheMetricLabels with required reason: CacheMissReason). Adapters whose miss parameter is typed as the broader CacheMetricLabels compile unchanged and may ignore the field; exact label snapshots, exhaustive Records over reasons, and adapters that reject or forward unknown fields must add it.
• Custom Redis adapters:RedisReadResult is now DecodedRedisFrame | RedisReadMiss { kind: "miss", reason, observedWatermarkMs? }. null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed; use decodeRedisReadResult (untracked) / decodeTrackedRedisReadResult (tracked) and the exported isRedisReadMiss guard. An un-migrated adapter still fails open at runtime (null or any unrecognized result is an unclassified miss with a normal refill) but loses reason precision and fenced-refill suppression until it returns the typed miss.

How classification works

Bundled decoders return one miss shape, RedisReadMiss { kind: "miss", reason, observedWatermarkMs? }, attaching observedWatermarkMs only when the same atomic tracked snapshot carried a valid numeric watermark. Cause and fence are deliberately independent: Redis nil is decisive evidence of absence, so an absent value reports value_absent while still carrying the observed watermark that lets PR 143's two-sample admission skip a known-fenced refill — only a complete supported frame actually rejected by the watermark reports watermark_fenced.

Core treats the adapter boundary as untrusted and normalizes every result once, at one choke point: kind: "miss" is the only discriminator; invalid or missing reasons become unclassified, a watermark_fenced claim without a valid tracked fence is demoted to unclassified, invalid fences (NaN, negatives, non-integers, untracked keys) are dropped, and null, undefined, or any other unrecognized value is an unclassified miss with a normal refill. Objects without kind: "miss" are frame candidates and go through the existing timestamp and serializer validation.

Decoded hits keep the existing { payload, createdAtMs } shape. Core-side rejections never acquire a refill fence: logical-age rejections emit expired, future timestamps and deserialization failures emit unclassified; conditional refill suppression remains exactly PR 143's adapter-level watermark-miss path, byte-compatible with main (differential-verified across the decode input matrix).

Breaking change

DialCacheMetricsAdapter.miss now receives MissMetricLabels, and first-party miss metrics require a reason label/tag. RedisReadResult is now DecodedRedisFrame | RedisReadMiss; null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed, and bundled Redis adapters return RedisReadMiss for every semantic miss.

Under the pre-1.0 release policy, this is a minor release.

Validation

• Node.js 22.22.0: corepack pnpm check
  • typecheck
  • 656 unit tests with coverage thresholds
  • ESM/CJS build and declaration generation
  • packed-package type and runtime checks
• Node.js 22.22.0: corepack pnpm test:integration
  • 143 passed
  • 2 expected local GLIDE Cluster skips because announced container IPs are not host-routable
• Review follow-up (fix(metrics): preserve existing refill and decoder behavior) removed the fence-carry scope creep and restored decoder/refill parity with main, differential-verified across the decode input matrix

BREAKING CHANGE: DialCacheMetricsAdapter.miss now receives MissMetricLabels and first-party miss metrics require a reason label/tag; RedisReadResult is now DecodedRedisFrame | RedisReadMiss ({ kind: "miss", reason, observedWatermarkMs? }); null, RedisWatermarkMiss, decodeRedisFrame, and decodeTrackedRedisFrame are removed, and bundled Redis adapters return RedisReadMiss for every semantic miss.

[lan17]

Verdict: request changes(recorded as a comment — GitHub refuses a requested-changes review on the author's own PR)

Reviewed at max effort (10 finder angles, adversarial verification, differential decode matrices up to 280 cells against main, plus a gap sweep). Typecheck and all 645 unit tests pass at 83e8a7c; the classification logic itself held up everywhere it was attacked: fence boundaries exact, zero-watermark/zero-timestamp edges probe-verified, no unbounded label can reach a metrics adapter, Prometheus re-registration fails loudly rather than mislabeling.

Requesting changes on three items. Each is cheap now and compounds after merge, because this PR freezes both the migration documentation and the public custom-adapter contract.

1. Align the docs with the fence extension (README.md:981, README.md:724, README.md:~485)

Threading observedWatermarkMs through decoded hit frames makes this more than a metrics migration: refills after core-side rejections (retained stale-on-error candidates, logical-age expiry, future-dated frames, deserialization failures) are now watermark-fenced — your own tests pin write suppression at candidate = watermark+0 on the retained path, fill_fenced where filled was emitted, and core-supplied createdAtMs on writes that never carried it. The behavior is good (verification showed every suppressed write would have been permanently unreadable, and the retained-path skip preserves the stale-on-error candidate the old refill destroyed) — but README line 981 still says this migration "does not change cache serving, refill, invalidation, stale recovery, or shadow-verdict behavior", which is now false, and issue #145 scoped behavior changes out. The PR body has been updated with a "Behavior changes" section; the README needs the matching fix:

• Rewrite the line-981 claim to scope it to what actually holds (serving verdicts, invalidation, stale-recovery classification, shadow match/mismatch/superseded verdicts).
• Line 724: "createdAtMs <= watermark is watermark_fenced" needs the zero-timestamp carve-out (a complete frame stamped 0 under a valid watermark classifies unclassified, probe-verified).

2. Give RedisReadMiss a discriminant (src/redis-client.ts:92)

RedisReadMiss has no kind, so isRedisReadMiss falls back to in-presence checks — reintroducing the exact hazard PR 143 added kind: "watermark_miss" to kill. Probe-verified: { reason: "value_absent", payload: undefined, createdAtMs: undefined } type-checks for strict consumers without exactOptionalPropertyTypes (tsc exit 0) and is misrouted as a frame at runtime, silently degrading every one of that adapter's declared reasons to unclassified — the feature defeated with no signal. Adding kind: "read_miss" pre-merge is free; after adapters adopt, it's another breaking change.

Stronger option worth 30 minutes while you're here: one discriminated miss type { kind; reason; observedWatermarkMs?: number }. That single change also collapses the twin constructors (redisReadMiss in redis-payload.ts:205 vs classifiedRedisReadMiss in redis-cache.ts:660), deletes the never-released-compat reason?: optionality (redis-client.ts:108 — no tag contains PR 143, so the optionality serves zero consumers) and its missReason() ceremony, and halves the 4× fence ternaries in getWithResolvedConfig.

3. Document the hit-shape widening (README Serialization section + migration list)

Bundled tracked hits now return { payload, createdAtMs, observedWatermarkMs } whenever a valid watermark exists (including 0). Your own exact-shape tests had to change (test/node-redis.test.ts, test/valkey-glide.test.ts) — consumers' will too. The README still describes DecodedRedisFrame as payload + createdAtMs only, and the migration list omits it. One sentence plus one bullet. (The PR body and BREAKING CHANGE footer now carry it.)

---

Non-blocking, comment-tier (take or leave; several vanish if you take the unified miss type):

• Corrupt-timestamp tracked frames are now silently reclassified at decode (redis-payload.ts:186): the legacy decodeTrackedRedisFrame returns null where main returned a frame or threw DialCacheRedisPayloadEncodingError (18 differential cells), the doubly-corrupt class loses its alertable error signal, and tracked vs untracked decoders now disagree on the same corrupt frame. Document as intended hardening or align the two.
• unclassifiedFrameMiss (redis-cache.ts:676) trusts an adapter-attached hit fence with envelope-only validation; a one-line observedWatermarkMs < frame.createdAtMs cross-check restores the pre-PR blast radius for contract-violating adapters.
• Taxonomy coherence: local TTL expiry reports value_absent while the same logical condition remotely reports unclassified; and the internal RedisCacheMissReason vs public CacheMissReason enums collide undocumented at the deser site (redis-cache.ts:184). Evidence-based and defensible — record the rationale in a comment/README clause so it reads as deliberate.
• validObservedWatermarkMs (redis-cache.ts:680) re-duplicates the safe-int≥0 envelope of isValidRedisTimestampMs (same finding as PR 143's isValidRedisWatermarkMiss, reincarnated) and runs twice per miss.
• isCacheMissReason (redis-cache.ts:686) has no exhaustiveness tie to the union — a future fourth reason compiles everywhere and silently demotes to unclassified at runtime. const CACHE_MISS_REASONS satisfies Record<CacheMissReason, true> beside the type fixes it.
• CacheMissReason isn't exported from the dialcache/redis-protocol subpath that exports RedisReadMiss — even scripts/test-package.mjs has to root-import it. One re-export line.
• Decode micro: readFrameCreatedAtMs runs twice per served tracked frame; decodedRedisFrame builds-then-spreads; the watermark-parse expression is duplicated in two branches (redis-payload.ts:139-158).
• Both legacy decoders are now zero-consumer wrappers, and this diff grows decodeTrackedRedisFrame with a strip projection. They were released in 0.22.0, so deleting is your policy call — this feat! is the natural moment.

Credit where due: the FakeRedis delegation to the real decoders kills the untracked-fence divergence from the PR 143 review, the fence-carry closes the core-side-rejection gap that PR 143 deliberately deferred, and the invalid-watermark normalization matrix and boundary tests are exactly the coverage this layer needs.

One revision pass and this is an easy approve.

[lan17]

Verdict: approve(recorded as a comment — GitHub refuses a formal review state on the author's own PR)

Re-reviewed at dc6199d (fix(metrics): preserve existing refill and decoder behavior). All three requested changes from the previous review are resolved; typecheck is clean, 653/653 unit tests pass, and a fresh sweep over the fix commit found nothing new.

Requested changes — all addressed

1. Scope / README behavior claim — fixed by reversion. The fence-carry is fully removed (DecodedRedisFrame.observedWatermarkMs, unclassifiedFrameMiss, the retained-path carry, and the validateFrameAge miss objects are gone). A 216-cell decoder differential against main shows byte-parity on the legacy decodeRedisFrame/decodeTrackedRedisFrame surfaces and zero hit/fence/throw drift on the classified decoders (only the added reason labels differ). The rewritten README claim — "Miss classification adds no new refill-fencing paths and preserves serving eligibility, invalidation, stale-recovery policy, and shadow outcomes" — is verified true: fencing occurs solely via PR 143's adapter-level watermark-miss path. This also moots the adapter-fence-trust hardening item, since frames no longer carry fences at all.
2. RedisReadMiss guard hazard — fixed via value-based guards. Both isRedisReadMiss and isRedisWatermarkMiss now treat explicitly-undefinedpayload/createdAtMs as absent, closing the misclassification for the new miss type and the PR-143-era residual on the kinded miss; probe coverage landed in the unit tests and the packed-package script (including the @ts-expect-error unbounded-reason negative). Guard value-reads are short-circuited behind the reason/kind presence checks and every classified miss is canonicalized inside validateReadResult, so no new throw path escapes unrecorded.
3. Hit-shape documentation — moot and documented. The widened hit shape is deleted; the README now states explicitly that decoded hits keep { payload, createdAtMs }.

Also verified: the unsafe-timestamp reclassification is fully reverted (doubly-corrupt frames throw DialCacheRedisPayloadEncodingError again; only the zero-baseline check remains, matching main), and the zero-timestamp carve-out landed in both the invalidation section and the metrics-table watermark_fenced row. The PR body has been updated to match the reverted scope (behavior-parity statement, corrected test count, footer back to the two real breaks).

Deferred (non-blocking, unchanged from the previous review)

Twin miss constructors (redisReadMiss / classifiedRedisReadMiss), the duplicated safe-int≥0 timestamp envelope in validObservedWatermarkMs, reason?: optionality serving a never-released shape, the non-exhaustive isCacheMissReason (a future fourth reason silently demotes to unclassified), CacheMissReason missing from the dialcache/redis-protocol exports, the local-vs-remote expired-entry taxonomy note, decode micro-items (double readFrameCreatedAtMs, duplicated watermark-parse expression), and the legacy-decoder deletion policy call. Several of these collapse together if the miss union is ever unified into one kinded type.

Good ship. The guard fix in particular is stronger than what was asked — it hardens both miss types instead of one.

[lan17]

Follow-up guidance: post-merge cleanup plan(non-blocking — the approve verdict above stands; this consolidates the deferred findings from both review rounds into an actionable plan for a single refactor: PR before 0.23 ships)

Two review rounds across #143/#146 kept finding one failure pattern: a correctness-critical invariant hand-written in more than one module, already divergent at birth (the three isRedisWatermarkMiss copies in #143 differed within one commit; the safe-int≥0 timestamp envelope has now been written three times). The cleanup principle is therefore one owner per invariant — not "no repeated lines". Some repetition here is deliberate and load-bearing; the last section lists what to leave alone.

DRY batch (descending leverage)

1. Unify the miss union into one kinded type — { kind, reason, observedWatermarkMs?: number }. Retires three deferred findings at once: the twin constructors (redisReadMiss in internal/redis-payload.ts vs classifiedRedisReadMiss in internal/redis-cache.ts — every bundled miss is currently built twice per read), the reason?: optionality that exists only for source compatibility with a shape that never shipped (no release contains #143's reason-less RedisWatermarkMiss, so missReason()'s undefined-handling at five sites serves nobody), and the composite isRedisReadMiss guard, which becomes a single kind check. Free now — none of this surface is released; expensive after adapter adoption. ~1 hour.
2. Exhaustiveness-tie the reason guard — isCacheMissReason (internal/redis-cache.ts) hand-lists the three strings with no compiler link to CacheMissReason. A future fourth reason compiles everywhere while validateReadResult silently demotes every adapter-supplied instance to unclassified — discovered in dashboards, not at build time. Fix: const CACHE_MISS_REASONS = { value_absent: true, watermark_fenced: true, unclassified: true } satisfies Record<CacheMissReason, true> beside the type, guard via Object.hasOwn (the exact pattern the tests and test-package.mjs already use).
3. One owner for the timestamp envelope — export boolean isValidRedisTimestampMs from internal/redis-payload.ts (widened to unknown) and define validObservedWatermarkMs on top of it. Prevents the envelope forking between bundled decoders and core validation of custom-adapter fences (e.g., if an upper bound is ever added so a garbage far-future watermark can't fence writes for years).
4. Move createdAtMs defaulting into the protocol — encodeRedisFrame(payload, createdAtMs?) defaulting to Date.now() internally (or one exported resolver). Today the "honor supplied value exactly, else sample" rule is a ternary copy-pasted into node-redis.ts and valkey-glide.ts and prescribed as README prose every custom adapter must transcribe; the natural mis-spelling request.createdAtMs || Date.now() silently breaks the fence-decision/stored-frame coupling at createdAtMs: 0. Make the correct behavior the only expressible one.
5. Small fry, batch opportunistically: hoist the duplicated watermarkFrame === null ? undefined : parseRedisWatermark(watermarkFrame) ?? undefined expression in decodeTrackedRedisReadResult; pass the already-read createdAtMs into decodedRedisFrame (removes the double readBigUInt64BE per served tracked hit); merge the two adjacent miss literals in getWithResolvedConfig; dedupe the README's twice-pasted "Conditional refill suppression reuses the existing tracked MGET result..." sentence (network-shape §, keep; ACL §, drop). Tests: fold the ~52-line ESM/CJS classified-decode paste in scripts/test-package.mjs into its existing shared-check mechanism (verifyPackedInvalidation precedent), and extract the thrice-copied throwing-serializer fixture.

Not DRY, but higher value than most of the above

• Meter the caller-path fence skip — putWithLayer still has three unmetered return false paths, so an active fence window is indistinguishable from a broken write path on dashboards (the same telemetry-gap shape as #141, which #143 fixed for the shadow side only). One new MetricErrorKind (fenced_refill_skipped — precedent: the advisory tracked_ttl_clamped) plus a typed 'dispatched' | 'fenced' | 'abandoned' return also removes the hidden invariant the shadow fill_fenced ternary depends on (sticky abandonment re-checked before the ternary). Extending MetricErrorKind is a type-surface change for exhaustive consumers — cheapest before 0.23, compounding after.
• Decide the legacy-decoder question — decodeRedisFrame / decodeTrackedRedisFrame are null-collapsing wrappers with zero in-repo consumers, kept for hypothetical external adapters; they drifted twice within this PR's own history before being re-pinned. Either delete them in this feat! window (they shipped in 0.22.0 but are unadopted; house policy is delete-don't-deprecate) or explicitly commit to the four-decoders-in-lockstep burden. Default-by-inaction is the worst option.
• Two one-liners: re-export CacheMissReason from dialcache/redis-protocol (it types RedisReadMiss.reason, which IS exported there; test-package.mjs currently has to root-import it), and a cross-reference comment at the deser site + one README table clause recording that local TTL expiry → value_absent vs remote logical expiry → unclassified is a deliberate evidence-based choice (and that internal RedisCacheMissReason is a disposition enum, not the metric reason) — so nobody "fixes" it later with another breaking metrics change.
• Optional hardening: make put's fence parameter a required RedisWatermarkMiss | undefined so any future fill path must decide explicitly instead of silently bypassing the fence.

Deliberate repetition — do NOT DRY these

• validateReadResult's reconstruction of adapter results is the single untrusted-input choke point, not redundant validation — keep it (with the union unification it gets simpler, not removed).
• The per-adapter label spreads ({...cacheLabels(labels), reason} in prometheus.ts/datadog.ts) are deliberate per-metric cardinality allowlists, consistent with disabled/error; prom-client rejects unknown labels, so wholesale pass-through isn't viable.
• FakeRedis's hand-rolled encodeFrame/decodeFrame test fixtures exist to construct malformed frames the real encoder refuses — that independence is the point (its read path already delegates to the real decoders, which is the right split).

Suggested packaging: items 1–4 + the one-liners as one refactor(redis): single-owner invariants for miss classification PR, the fence-skip metric as its own small feat(metrics) (it extends a public union), and the legacy-decoder deletion as a standalone commit gated on the adoption decision — all before the 0.23 release so every type-shape change rides the same pre-adoption window. Total estimated effort excluding the decoder decision: about half a day.