diff --git a/.github/actions/run-tests/action.yml b/.github/actions/run-tests/action.yml index a6258a3ab..3d3dd50ca 100644 --- a/.github/actions/run-tests/action.yml +++ b/.github/actions/run-tests/action.yml @@ -81,6 +81,31 @@ runs: --results-directory ./test-results/ /p:CI=true + # Neither of these needs a server, and between them they took about four seconds - but nothing ran + # them until now, so a namespace rename silently broke all 18 SER309 tests and nobody found out. + - name: RESPite.Tests + shell: bash + run: >- + dotnet test tests/RESPite.Tests/RESPite.Tests.csproj + --no-build + -c Release + -f net10.0 + --logger trx + --logger "GitHubActions;summary-include-passed=false;summary-include-skipped=false" + --results-directory ./test-results/ + /p:CI=true + + - name: StackExchange.Redis.Build.Tests + shell: bash + run: >- + dotnet test tests/StackExchange.Redis.Build.Tests/StackExchange.Redis.Build.Tests.csproj + --no-build + -c Release + --logger trx + --logger "GitHubActions;summary-include-passed=false;summary-include-skipped=false" + --results-directory ./test-results/ + /p:CI=true + - uses: dorny/test-reporter@v3 continue-on-error: true if: success() || failure() diff --git a/.gitignore b/.gitignore index e8b097e22..9395fd75e 100644 --- a/.gitignore +++ b/.gitignore @@ -34,3 +34,6 @@ BenchmarkDotNet.Artifacts/ # local planning/design notes (not for publication) planning/ + +# agent worktrees (see .claude/); these are separate checkouts, never repo content +.claude/worktrees/ diff --git a/Directory.Build.props b/Directory.Build.props index fbb5c39f5..403c8b8d0 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -1,4 +1,4 @@ - + 2.0.0 2014 - $([System.DateTime]::Now.Year) Stack Exchange, Inc. @@ -10,9 +10,11 @@ true $(MSBuildThisFileDirectory)Shared.ruleset NETSDK1069 - - $(NoWarn);NU5105;NU1507;SER004;SER005;SER009;SER010 + + $(NoWarn);NU5105;NU1507;SER004;SER005;SER009;SER010;SER012 https://github.com/StackExchange/StackExchange.Redis/releases https://seredis.dev/ MIT diff --git a/design/interpolated-resp-writer.md b/design/interpolated-resp-writer.md new file mode 100644 index 000000000..0c07f2698 --- /dev/null +++ b/design/interpolated-resp-writer.md @@ -0,0 +1,3422 @@ +# Interpolated-string RESP writer + +**Exploratory notes — ideas, not decisions.** Nothing here is agreed or committed to; it is a log of +what was tried, what was verified empirically, what seems to follow, and what is still open. Treat +recommendations as "this looked right at the time", not as a plan of record. A working spike lives in `src/StackExchange.Redis/Interpolated/` with unit tests +in `tests/StackExchange.Redis.Tests/InterpolatedWriterUnitTests.cs`; everything there is `internal`, so +there is no public API commitment yet. + +The idea: let command construction read as + +```csharp +Render($"{cmd}{key}{value}", handler); +``` + +where `$"..."` binds to a custom interpolated string handler that writes RESP directly, rather than +building a `Message` + argument array. The handler is pure formatting: it runs on the caller's +thread, ahead of the critical section, and the bytes it produces double as the client-side cache +key before anything touches the muxer core. It is never near a connection. + +This is intended to **replace the writer half of the `marc/respite` v3 PoC spike** — its manual +`RespWriter` + `RespOperationBuilder`. The execution API around it (`RespContext`, cancellation, +`RespContextDatabase`) is good and carries over unchanged. See §8. + +--- + +## 1. Is the handler pattern usable down-level? + +**Spec:** [Improved Interpolated Strings](https://github.com/dotnet/csharplang/blob/main/proposals/csharp-10.0/improved-interpolated-strings.md) +(C# 10 feature spec). Quoted below where it settles a question; the empirical checks agree with it +throughout. Notably it imposes **no requirement that the attributes come from corelib** — they are +recognised by name — which is what makes the polyfill legitimate rather than a trick that happens to work. + +**Yes, with no runtime support at all.** Interpolated string handlers are 100% compiler lowering, +and the marker attributes are matched *by full name*, so declaring them `internal` in our own source +works exactly like the existing `SkipLocalsInit` (`src/RESPite/Shared/SkipLocalsInit.cs`) and +`IsExternalInit` (`src/StackExchange.Redis/FrameworkShims.IsExternalInit.cs`) shims. +`LangVersion 14` is already set repo-wide in `Directory.Build.props`. + +Verified by compiling across `netstandard2.0` / `net472` / `net8.0`: + +| Feature | Down-level | Notes | +| --- | --- | --- | +| `[InterpolatedStringHandler]` on a `ref struct` | works | polyfill the attribute in source | +| `[InterpolatedStringHandlerArgument(nameof(x))]` | works | caller's `stackalloc` buffer reaches the ctor | +| `[InterpolatedStringHandlerArgument("")]` | works | `""` passes the **receiver** (`this`) | +| `[InterpolatedStringHandlerArgument("", nameof(cmd))]` | works | receiver *and* a parameter | +| `out bool shouldAppend` conditional ctor | works | | +| `bool`-returning `Append*` | works | compiler emits a short-circuiting `&&` chain | +| Use inside an `async` method | works | fails only if a hole itself contains `await` (CS8850) | +| `u8` literals (`"..."u8`) | works | **no polyfill needed**; needs only `ReadOnlySpan` | +| C# 14 extension members (`extension(...) { }`) | works | also pure lowering | +| `[OverloadResolutionPriority]` | works | polyfill the attribute in source | +| `scoped` on span params | works | `ScopedRefAttribute` is compiler-synthesized | +| `System.Index` / `System.Range` | works, but **`internal` only** | public polyfill would collide on newer TFMs; unusable in public API | + +`net461` was not tested (no reference assemblies to hand) but uses the same compiler path; the only +dependency is `ReadOnlySpan`, which RESPite already has there via `System.Memory`. + +The spec text behind three of those rows, since they shape the design elsewhere: + +- **Constructor** — *"The first two arguments are integer constants, representing the literal length of + `i`, and the number of interpolation components in `i`, respectively."* Extra parameters come from + `InterpolatedStringHandlerArgumentAttribute`, and the trailing `out bool` is optional: *"If no + applicable constructors were found, step 3 is retried, removing the final `bool` parameter."* +- **Short-circuiting** — *"If `Fax` returns a `bool`, the result is logically anded with all preceding + `Fax` calls."* So `bool` returns genuinely stop later holes being evaluated (§2.4 rejects this for the + disabled-command case, which must throw rather than quietly truncate). +- **`AppendFormatted` shapes** — the value by itself; plus an `int alignment` when the hole carries + `,N`; plus a `string format` when it carries `:F`. See §2.3 for why the `format` route was not used. + +**`InlineArray` is the one thing that is *not* polyfillable** — it needs .NET 8+ runtime layout +support, and down-level the attribute is inert, silently giving a one-element struct. Use +`stackalloc` at the call site or `fixed` buffers instead. + +--- + +## 2. Shape + +### 2.1 Literals become arguments (was: rejected, except a single space) + +> **Superseded.** Literal text is no longer discarded, and SER309 is a **warning**, not an error. What +> follows records why rejection looked right first; the reasoning that replaced it is here. + +**What changed.** `AppendLiteral` tokenizes on whitespace. If nothing has been written yet, the first +token is the **command** - parsed against the known set, mapped through `CommandMap`, framed verbatim if +unrecognised. Every later token is an ordinary **value** argument, UTF-8 encoded straight into the frame. +Whitespace-only literals still contribute nothing, so `$"{cmd} {key} {value}"` is unchanged. + +So `$"SET {key} {value}"` renders byte-identically to `$"{RedisCommand.SET}{key}{value}"`, and +`$"COMMAND INFO {name.Command()}"` works. + +**Why this is better than rejecting.** The rejected form did exactly what it looked like; refusing to +compile it bought correctness we did not actually need. Working-but-slower beats not-working, and the +warning still points at the faster spelling. + +**The fixer forks on accessibility, not on preference.** A *leading* literal is the command, so it gets a +different fix from a token in any other position - the same positional rule the writer applies at runtime, +so the fix and the behaviour cannot disagree: + +| | offered | +| --- | --- | +| leading, `RedisCommand` reachable | `RedisCommand.SET` - no parse, and a typo is a compile error | +| leading, not reachable | a `static readonly RespCommand` field, `"SET".Command(preform: true)` | +| anywhere else | the existing `[Resp]` fragment fix | + +`RedisCommand` is internal, so this library's own code takes the first row and everyone else takes the +second. The field uses `preform: true` **because it is a static**: a no-op for a command the library knows, +since the command map already holds its bytes, and a real saving for a module command, where the bytes are +then built once rather than per call. + +That external half is what the code-fix tests actually exercise, since the harness compiles against the +public surface with no `InternalsVisibleTo` - so even `SET` gets the field there, which is exactly right. + +**Splitting on whitespace gets container commands right for free.** `$"CONFIG GET {name}"` yields three +arguments, with `CONFIG` mapped and `GET` not - which is precisely how `CommandMap` behaves, since it maps +container verbs only. That was not designed for; it fell out. + +**What it costs, and what it does not.** + +- Each token is parsed and encoded **per call**, where a `[Resp]` fragment or a `RespCommand` resolves + once. That is the whole content of the warning. +- `AppendLiteral` fast-paths empty and a single space before entering the tokenizer, so the recommended + spelling pays nothing for the readable one existing. The tokenizer is `[MethodImpl(NoInlining)]`, the + same split as `MessageWriter`'s fallbacks and for the same codegen reason. +- Nothing allocates: the split is index arithmetic over the literal, and the encode is pointer-based + straight into the frame buffer - `Encoding.GetByteCount(ReadOnlySpan)` does not exist on + netstandard2.0 or net461, but the `char*` overloads do. +- **A literal token is never a key.** Key-ness comes from the hole type, so routing and invalidation are + unaffected by any of this. +- `*N` is no longer derivable from `formattedCount` for this form, which is fine: the count was only ever + an optimisation hint, since `Compose` plus `AppendFormatted` already defeat it. + +--- + +#### Original reasoning: literals rejected, except a single space + +Every part of the command must be a hole, with one exception: a **single space**, which is discarded. + +```csharp +ctx.Execute(RedisCommand.SET, $"{key} {value}") // ok - the space is discarded +ctx.Render($"SET {key} {value}") // rejected - "SET " is not a separator +ctx.Render($"{cmd} {key}") // rejected - two spaces +``` + +The space earns its place on readability: `$"{RedisCommand.SET} {key} {value}"` mirrors how the command +is written everywhere else, for ~1.4 ns per space (measured below). + +**Why reject the rest:** with no literal segments, the compiler-supplied `formattedCount` *is* the argument +count, as a compile-time constant — so `*N\r\n` can be written in the constructor with no counting +and no back-fill. + +**Verified non-hazard:** C# 10 makes an all-constant interpolated string a *constant expression*, so +`$"{"GET"}{"mykey"}"` could in principle have been folded to a single literal and routed to +`AppendLiteral`, silently corrupting the frame. It is not — the handler conversion wins and each hole +stays a hole. Confirmed both at runtime and by the fact that it compiles against a handler that has +no `AppendLiteral` at all. + +**The compiler always passes a `string`, never u8.** The spec is explicit, and one rule accounts for +every case below: + +> The argument list `Al` is constructed with one value parameter of type `string`. Traditional method +> invocation resolution is performed with method group `Ml` and argument list `Al`. + +So the argument is *always* a `string`, and binding is then ordinary overload resolution. Verified across +the three plausible overload shapes: + +| `AppendLiteral` overload | Binds to a literal segment? | +| --- | --- | +| `string` | yes — this is what the compiler passes | +| `ReadOnlySpan` | yes, via the implicit `string` → span conversion | +| `ReadOnlySpan` | **no** — `CS1503: cannot convert from 'string' to 'System.ReadOnlySpan'` | + +All three follow from the rule: `string`→`ReadOnlySpan` is an applicable conversion, `string`→ +`ReadOnlySpan` is not, and there is no step at which the compiler would UTF8-encode. The +`ReadOnlySpan` form buys nothing (the argument is a constant `string` either way), and the absence +of a `u8` route is one more reason to ban literals rather than encode them at runtime. + +("Traditional method invocation resolution" is also why the ban holds: see below.) + +**Enforcement is the analyzer's, exclusively. `AppendLiteral` is a no-op with no check at all**, so the +JIT eliminates the call. That is not laziness — a runtime check would buy nothing the analyzer does not, +because *discarding a literal is benign in the way that matters*: the frame stays **well-formed**, with +an argument missing. Literals never contributed to `*N`, so the header remains correct; the server sees +a wrong command and errors, or does the wrong thing, and the connection is unaffected. + +Compare `RespFragment` (§9.1), where bad bytes desync the connection for every *subsequent* command. The +principle running through both: **guard strength proportional to blast radius** — an analyzer error here, +an analyzer error *plus* a generator-emitted `#error` there. Ignoring the analyzer here is user error with +local consequences; ignoring it there corrupts other people's commands. + +An earlier revision marked +`AppendLiteral(string)` as `[Obsolete(..., error: true)]`, which made *any* literal a compile error — +strictly stronger, but incompatible with allowing the space. Two findings from that revision, recorded +because they bear on the alternative: + +- *Omitting* `AppendLiteral` also rejects literals, but produces a confusing pair of diagnostics + (`CS1061` plus a bogus `CS8941` "does not return void or bool") where `[Obsolete]` gives one error + carrying our own message. +- The obsolete ban did **not** leak: with both an obsolete `AppendLiteral(string)` and a non-obsolete + `AppendLiteral(ReadOnlySpan)`, the obsolete one still won (`CS0619`), because the exact `string` + match beats the span conversion and `[Obsolete]` applies after resolution. + +Allowing the space trades that compile-time guarantee for readability, so the analyzer (§7) has to carry +the rule instead — in particular because **two spaces look exactly like one** on the page, and a runtime +throw arrives at the worst possible moment. It also has to reject leading and trailing spaces, which +satisfy "exactly one space" but are not separators. + +#### Why the space costs nothing + +**It does not cost the `*N` constant**, which is the main justification for rejecting literals. Spaces are literal +segments, not holes, so `formattedCount` is unchanged; only `literalLength` moves, which merely nudges +the buffer size hint. Measured: + +``` +tight : args=3 literals=0 formattedCount=3 literalLength=0 +spaced : args=3 literals=2 formattedCount=3 literalLength=2 +``` + +**It costs about 1.4 ns per space** — less than expected, but not nothing, and notably *not* zero even +with an empty `AppendLiteral`. The `Separators` benchmark renders the same four-argument command spaced +and unspaced: + +``` +Separators_None 62.62 ns 1.00 +Separators_Spaced 66.87 ns 1.07 +``` + +Consistent across both jobs with low deviation, so the `ldstr` and the call are not being fully +eliminated despite the method body being empty. That is ~7% of the render, and a far smaller share of the +operation around it — but "the JIT will nuke it" turned out to be optimistic, and the figure is recorded +rather than assumed. + +Both spellings render byte-identically, since the space is discarded, so cache identity (§6.2) is +unaffected. + +The cost is the compile-time guarantee, discussed above: enforcement rests entirely on the analyzer. +Formatters are a third hazard alongside the two already noted — nothing stops a tool normalising +whitespace inside an interpolated string. + +#### Inline tokens: rejected, with a fixer + +`$"{key} nx {val} withsave"` reads well, and `nx`/`withsave` are arguments rather than separators — so the +question is whether literal runs should be split into tokens. **No.** They stay rejected, and the analyzer +carries the ergonomics instead: a diagnostic on the offending literal plus a **code fixer** that rewrites +it to the declared form. + +``` +$"{key} nx {val}" -> fix -> $"{key} {RespLiterals.Nx} {val}" +``` + +Two fixes, since the token may not be declared yet: + +- *"Use `RespLiterals.Nx`"* when a matching `[Resp]` declaration exists. +- *"Declare `Nx` and use it"* when it does not — the fixer adds the partial property, and the generator + fills in the body. + +You type it the natural way and take the fix; the committed code is the strict form. The ergonomic gap +closes at authoring time, which is where it is actually felt. + +**Why not make it work at runtime.** It is achievable: `AppendLiteral(" nx ")` could split on spaces and +resolve each token, and the resolution need not be a lazy dictionary — this repo already generates +precisely that lookup as a hash-dispatched switch, `[AsciiHash(CaseSensitive = false)] static partial bool +TryParseCI(...)`, which is allocation-free, lock-free, and case-insensitive (so `nx` in source would still +yield the canonical `NX` bytes). The cost is not the lookup: + +- **`*N` stops being a compile-time constant.** Inline tokens are literal segments, not holes, so + `formattedCount` is no longer the argument count — and the loss applies to every call site using the + sugar, not only the complex ones. +- **It is a second mechanism** to document, analyze and explain, alongside `RespFragment`. + +`{Nx}` costs a declaration; `nx` costs an invariant. + +**The analyzer and the generator are one piece of work**, not two: both are driven by the set of `[Resp]` +declarations. The generator emits the fragment bodies from them; the analyzer validates literals against +the same set, and the fixer needs it to know which member to offer. + +**Non-interpolated strings do *not* bind to the handler.** If a `string` overload exists alongside, +`Write(buf, "plain literal")` silently takes it while `Write(buf, $"GET {key}")` takes the handler. +Either don't provide a `string` overload, or accept that callers must write `$"PING"`. + +### 2.2 The overload set is closed — permanently + +Extension `AppendFormatted` methods **do not bind**. Verified three ways, all rejected in a hole +while compiling fine as ordinary calls: + +```csharp +public static void AppendFormatted(this ref H h, Geo v) // no +public static void AppendFormatted(this H h, Vec v) // no +extension(ref H h) { public void AppendFormatted(Blob v) } // no (C# 14 extension block) +``` + +Re-verified on the current compiler against the real `RespRequestBuilder`, and the third case below is +the one that settles it — it is not "an instance member wins", it is that **extension lookup never runs**: + +| setup | `$"{x}"` | +|---|---| +| classic `this ref` extension, other instance members present | CS0315 against the *instance* generic | +| C# 14 `extension(ref H h)` block | CS0315 against the *instance* generic | +| handler whose only member is `AppendFormatted(int)`, extension takes `Geo` | **CS1503, "cannot convert from 'Geo' to 'int'"** | + +The third row is the proof. Ordinary C# consults extensions when no instance method is applicable; here +none was applicable and the compiler still bound to the instance member and failed the conversion. (That +is also where the "CS1503 names an arbitrary overload" cost below comes from.) + +**Positive control:** the very same extension methods, in the same file with the same usings, compile and +*run* as ordinary calls — `cmd.AppendFormatted(new Geo())`. So they are genuinely in scope and valid; the +lowering simply does not look at them. + +So nobody — not a consumer, not another assembly here — can extend the hole vocabulary after the fact +**by adding a method**. `IRespArgument` (below) is the sanctioned way back in, and being an interface on +the *argument* rather than a method on the *handler* is exactly why it works. + +**Consequence for layering (§9.5):** a `RedisCommand` hole can only ever be served by an instance member +of the handler type. `RedisCommand` is an `enum`, so it cannot implement `IRespArgument` either. Whatever +assembly declares the handler type must therefore know about `RedisCommand` — which is why the handler +cannot simply move to RESPite, and why the split is by *layer* (a RESPite `RespWriter` held by value +inside the SE.Redis handler) rather than by relocation. + +Consequences: + +- Prefer a **few correct funnels over an enumeration**. Adding overloads later is additive and safe; + removing or retyping them is breaking (AGENTS.md). Ship the minimum set. +- The funnels: `RedisCommand`, `RedisKey`, `RedisValue`, `Resp.Raw`. +- **Do not define an *unconstrained* `AppendFormatted`.** A generic catch-all is an exact match by + inference, so it beats any overload needing a conversion — anything not explicitly declared silently + falls into a `ToString()` path and goes on the wire wrong. Omitting it makes those compile errors + instead. + +**The constrained form is the exception, and is now implemented:** + +```csharp +public void AppendFormatted(T value) where T : IRespArgument +``` + +The constraint is the whole difference. A type that does not implement the interface is **not +applicable**, so the undeclared cases still fail to compile — and the diagnostic gets *better*, not +worse: `CS0315` naming `IRespArgument` and what to do about it, where the closed overload set produced a +`CS1503` naming an arbitrary member (the "analyzer candidate" this bullet used to end with is +consequently no longer needed). + +Overload resolution measured on the three cases that decide whether it is safe: + +| both applicable | winner | verdict | +|---|---|---| +| dedicated non-generic overload vs. the generic | **non-generic** | wanted; built-ins keep their own rendering | +| implicit conversion to `RedisValue` vs. the generic | **generic** | wanted; opting in beats an incidental conversion | +| type implementing nothing (`Guid`) | *neither* — CS0315 | wanted; the protection above survives | + +A `struct` implementer is a constrained call, so **nothing boxes** — pinned by an allocation test +asserting exactly zero. + +**An implementer cannot miscount.** It writes by calling the handler's own `AppendFormatted` methods, +which maintain `_args`/`_argIndex`, so there is no separately declared token count to drift from what was +actually written. Contrast `RespFragment.ArgCount`, which is an assertion taken on trust — a wrong one +corrupts the `*N` header and misframes the *next* command on the connection. Writing nothing is legal and +means "no argument". + +This is the argument-level counterpart to §9.4: the context is the extension point for *commands*, and +`IRespArgument` is the extension point for *argument types*. Without it, `NRedisStack` could add commands +but could not add a type that appears in one. + +#### Format specifiers: a second, unrelated interface + +`IRespFormattableArgument.WriteTo(scoped ref RespRequestBuilder, string? format)` handles `$"{x:fmt}"`, +behind its own `AppendFormatted(T, string?)` overload. + +It deliberately does **not** derive from `IRespArgument`, so the three combinations are three different +contracts, each enforced by the compiler — measured, both directions: + +| implements | `$"{x}"` | `$"{x:fmt}"` | +|---|---|---| +| `IRespArgument` only | writes | **CS0315**, naming `IRespFormattableArgument` | +| `IRespFormattableArgument` only | **CS0315**, naming `IRespArgument` | writes | +| both | plain form | format form | + +The middle row is the reason for the split rather than a single interface: it makes the format +**mandatory**, which is how a type with no safe default forces the caller to choose — the same move §2.3 +wanted when a bare `$"{ttl}"` would have to guess between `EX` and `PX`. A single interface cannot +express it, and a default interface method cannot fake it: DIMs need runtime support that `net461` and +`netstandard2.0` do not have. + +A type implementing both is unambiguous because the overloads differ in **arity** — the `:` in the hole +decides, not overload betterness, so none of the resolution subtleties above apply. + +*Rough edge:* in the middle row the message reads "no boxing conversion from X to `IRespArgument`", which +is accurate but does not say *"you must supply a format"*. Analyzer candidate. + +#### Alignment: never + +There is no `int alignment` overload and there must not be one. RESP is length-prefixed binary, so +`$"{key,10}"` would pad the payload and send a **different key**, silently — the one failure mode where +the wire bytes change and nothing complains. It is `CS1739` ("does not have a parameter named +'alignment'") today, and pinned by a reflection test asserting no `AppendFormatted` parameter is named +`alignment`, because a compile error cannot be asserted directly and "add it for symmetry with the format +overload" is the plausible way it gets broken. +- With no catch-all, `RedisValue`'s existing implicit conversions cover `string`, `int`, `byte[]` + etc. for free. + +A clean rule for the two byte-ish funnels: + +- **`Resp.Raw` = "I already framed this"** → explicit, because the compiler can't check the claim. +- **`RedisValue` = "you frame this"** → implicit is safe, because the framing is ours. + +Note `Resp.Raw` is a `ref struct`, so it is *structurally* incapable of falling into a generic +catch-all even if one were added later. + +### 2.3 `Resp.Raw` and `u8` + +Pre-framed fragments enter as `Resp.Raw`, a `readonly ref struct` over `ReadOnlySpan` plus an +`ArgCount`. Reuse goes through static **properties** (a `ReadOnlySpan` cannot be a field): + +```csharp +public static Raw Ex => "$2\r\nEX\r\n"u8.Resp(); +public static Raw ExpireSeconds300 => "$2\r\nEX\r\n$3\r\n300\r\n"u8.Resp(2); +``` + +Zero allocation; inlines to an RVA load. + +`ArgCount` is the reason the wrapper exists rather than a bare `ReadOnlySpan`: **a raw fragment +can be more than one bulk string**, so without it `formattedCount` stops equalling the argument count +and the constant `*N` header silently breaks. Measured: a 2-arg fragment in a 3-hole interpolation +yields 4 args. + +An implicit `ReadOnlySpan` → `Raw` conversion was tried and rejected: it re-opens the hole it +was meant to close, because *any* span — including a runtime `byte[]` payload — then claims to be a +framed fragment. The analyzer can catch bad *literals*, but the conversion's new risk is non-literal +spans, which is exactly what it cannot see. `.Resp()` costs seven characters and is the whole +assertion. + +Ship `Resp()` and `Resp(int argCount)` as **separate overloads**, not one optional parameter — +adding an optional parameter later is a binary break (AGENTS.md). + +#### Authoring raw fragments: generated partial properties + +Rather than hand-writing `u8` blobs, declare the fragment and let a generator emit it: + +```csharp +// what the author writes +[Resp] private static partial Resp Ex { get; } // token inferred: "EX" +[Resp("foo", "bar")] private static partial Resp FooBar { get; } // two tokens, ArgCount 2 + +// what the generator emits +private static partial Resp Ex => new("$2\r\nEX\r\n"u8); +private static partial Resp FooBar => new("$3\r\nFOO\r\n$3\r\nBAR\r\n"u8, 2); +``` + +Partial properties are C# 13, and `LangVersion 14` is repo-wide; verified compiling on +`netstandard2.0`/`net472`/`net8.0`, since like everything else here they are pure compiler lowering. + +**Multi-token fragments are not hypothetical — they are the dominant shape.** Container commands, whose +first argument is a fixed subcommand token, account for roughly 140 call sites in `src/`: `CONFIG` (22), +`CLIENT` (22), `SCRIPT` (16), `XGROUP` (14), `PUBSUB` (13), `OBJECT` (12), `LATENCY` (12), `CLUSTER` +(11), `SLOWLOG` (10), `MEMORY` (10), `XINFO` (8). `CLIENT SETINFO LIB-NAME` is three tokens; `MAXLEN ~` +in `XADD`/`XTRIM` is two. So `ArgCount` is load-bearing rather than defensive — without it the handler's +argument count silently disagrees with the frame. + +**This is an existing pattern in the tree, not a new one.** `AsciiHashGenerator` already does it with +partial *classes*: + +```csharp +[AsciiHash("__keyspace@")] +private static partial class KeyspaceChannelPrefix { } // generator emits .HashCS and .U8 +``` + +Partial properties are simply the tidier shape — one member rather than a nested type. Two conventions +from `AsciiHashAttribute` transfer directly: the token is **inferred from the member name** unless the +attribute overrides it, and the attribute is `[Conditional("DEBUG")]` so it evaporates from shipped +metadata while the generator still sees it in source. + +##### Casing + +**Default to upper-case; the attribute gives verbatim control.** Counting the tokens `RedisLiterals` +actually sends: **138 upper-case, 31 lower-case, 0 mixed**, out of 165. So an inferred token — one with +no attribute, taken from the member name — should be upper-cased, which is right ~84% of the time and +matches what `CommandMap` already does to command names for the canonicality reason in §6.3. + +The lower-case minority is not arbitrary, which is why a single rule is not enough: those tokens are +**values rather than keywords**. `yes`/`no`, `lib-name`/`lib-ver`, `replica`/`slave`/`sentinel`/`pubsub`, +config parameter names such as `databases`/`timeout`, the geo units `km`/`mi`/`ft`/`m`, and the markers +`#`/`-`/`+`/`*`. + +So: **a token given in the attribute is used verbatim.** One rule, no extra flag, and it handles the case +that forces the issue — a single fragment containing both: + +```csharp +[Resp("SETINFO", "lib-name")] // keyword upper, attribute name lower - as sent today +``` + +`CLIENT SETINFO lib-name` is the shape the library sends now; emitting `LIB-NAME` would be a gratuitous +change to the wire format. Worth noting only because a generator that upper-cased everything +unconditionally would make exactly that mistake silently. + +**It largely removes the need for the raw-fragment analyzer rules (§7.1-3).** Those exist to validate +hand-written `u8`: framing, matching length prefixes, uppercase tokens. A generator emits all three +correctly *by construction* — there is nothing left to check. The rule becomes "don't hand-write these" +rather than "validate what you hand-wrote", which is both easier to enforce and impossible to get subtly +wrong. It also settles the canonicality requirement from §6.3 at the source: the generator uppercases, +as `AsciiHash` already does. + +The optional `argCount` defaulting to 1 is fine here, despite the binary-compat rule against optional +parameters — that rule is about *shipped public* API, and if the constructor stays internal with +`.Resp()` as the public factory (§2.3), the generated call site is inside the assembly. + +#### Format specifiers: wrong for "raw", right for units + +Two different uses, with opposite answers. + +**Dropped — `{blob:R}` to mark a hole as pre-framed.** This is an assertion about the argument's +*nature*, which a type expresses better. It was tried and works, but the specifier is only checked at +runtime (`{x:r}` or `{x:Raw}` compiles and falls through silently), it cannot carry `ArgCount`, and once +raw fragments are a distinct type (`RespFragment`) the marker is redundant. + +**Kept — `{ttl:s}` to choose an encoding the type cannot determine.** `TimeSpan` has no single correct +RESP encoding: `EXPIRE`/`EX` want seconds, `PEXPIRE`/`PX` want milliseconds. Likewise `DateTime` for +`EXPIREAT` versus `PEXPIREAT`, and `bool` for `0`/`1` versus the `yes`/`no` that `CONFIG SET` takes +(`RedisLiterals.yes`/`no` already exist). This is what format specifiers are *for*. + +**The specifier can be made mandatory, by the compiler.** Declare only +`AppendFormatted(TimeSpan, string format)` and omit the one-argument overload, and `$"{ttl}"` fails to +compile (`CS1503`). So for a type with no safe default the unit is *required*, enforced by overload +resolution rather than by the analyzer — which answers the objection that sank `:R`: the dangerous case +is not a mistyped specifier but an absent one, and absence is a build error. Types that do have a safe +default (`int`, `RedisValue`) simply keep their one-argument overload and are unaffected. Verified: + +``` +{ttl:s} -> 300 {ttl:ms} -> 300000 {ttl} -> does not compile +{42} -> 42 {true} -> 1 {true:yn} -> yes +``` + +**The catch is that the unit is coupled to the command**, and the compiler cannot see that. Today's code +picks both together — `useSeconds = milliseconds % 1000 == 0`, then `HEXPIRE` versus `HPEXPIRE` +(`RedisDatabase.cs:447-449`). So `$"{RedisCommand.PEXPIRE} {key} {ttl:s}"` would compile and be wrong. +That is an analyzer rule, and a new *kind* of rule: relating a specifier to the value of another hole. +Tractable, because the command is normally a literal at the call site, but more involved than the +per-hole checks in §7. + +A mistyped-but-present specifier still falls through to a runtime `FormatException`, so the analyzer +should also pin the valid set per type. + +### 2.4 `RedisCommand` and CommandMap + +The command must be a `RedisCommand` so it routes through `CommandMap` (renaming/disabling per server +type). The plumbing already exists: `CommandMap.GetResp(command)` returns a **pre-encoded RESP +bulk-string fragment** (`$6\r\nLRANGE\r\n`) from one shared ~3k buffer, already uppercased +(`CommandMap.cs:225`, built at `CommandMap.cs:248-270`). So `AppendFormatted(RedisCommand)` is a +lookup and a blit. + +Two consequences: + +1. **It forces the receiver-passing form.** `CommandMap` is per-`ConfigurationOptions` and resolved + at runtime, so no static lookup is possible. +2. **It rules out the `bool` short-circuit pattern.** A disabled command must *throw*, not return + `false` — returning `false` would abandon the remaining holes and emit a truncated frame. Use + `void` Append methods. + +--- + +## 3. Two call shapes + +### 3.1 Argument form — preferred + +```csharp +public Resp Begin(RedisCommand command, + [InterpolatedStringHandlerArgument("", nameof(command))] ref Resp handler) => handler; + +using var r = writer.Begin(RedisCommand.SET, $"{key}{value}"); +``` + +`("", nameof(command))` passes **both** the receiver and the `command` parameter into the constructor, +so the CommandMap, the command and the target are all known up front. + +### 3.2 Conversion form + +```csharp +using Resp r = $"{RedisCommand.SET}{key}{value}"; +``` + +This is an interpolated string *conversion*, not an argument, so `[InterpolatedStringHandlerArgument]` +does not apply and only the `(int literalLength, int formattedCount)` ctor runs. The handler therefore +loses the receiver, which means: + +- it must own a pooled buffer; +- the CommandMap has to arrive at `Close(map)`; +- the prologue must be a **fixed worst-case** reservation, since `map.MaxRespLength` is not available + yet. (Hit as an `ArgumentOutOfRangeException` while building this.) Not a real cost: `*N` is ≤12 + bytes and command names are bounded. + +Prefer the argument form. Its terseness advantage is small and the receiver is the thing you need. + +### 3.3 The receiver, and a context object + +`[InterpolatedStringHandlerArgument("")]` passes the **receiver** of the call into the handler's +constructor — per the spec, *"The empty string is matched to the receiver of `M1`."* Verified working in +every shape that matters — concrete receiver, receiver via an +interface, implicit `this` from inside the type, an `object`-typed ctor parameter, and extension +methods (where the receiver is the first parameter, so `nameof(db)` rather than `""`). + +**Rule:** the receiver's *static type at the call site* must be convertible to the ctor parameter +type. Declaring `Execute` on `IDatabase` therefore forces the ctor to accept `IDatabase`. + +That is a problem, because `CommandMap` is not reachable from there — it is not on +`IConnectionMultiplexer` and is not public API at all; `IDatabase` reaches only +`IConnectionMultiplexer Multiplexer` (`IRedisAsync.cs:14`). A downcast would work inside the +assembly but **breaks every `IDatabase` mock**, and breaks it during command *construction*, in the +caller's frame, before the mock's `Execute` is reached. + +**Resolution: a dedicated context type as the receiver** — `ctx.Render($"...")` — carrying: + +| Shared per multiplexer | Varies per instance | +| --- | --- | +| CommandMap, buffer manager, client-side cache, `ServerType` | `KeyPrefix`, database index | + +`ServerType` matters: `HashSlot` short-circuits to `NoSlot` for standalone +(`ServerSelectionStrategy.cs:101-102`), so without it the handler computes CRC16 over every key for +standalone deployments that never use the result. + +That granularity is one context per *(multiplexer, db, prefix)* — what `RedisDatabase` already has — +so cache one per database instance rather than allocating per command. + +**Class or struct:** a context that *stores* all of the above wants to be a class, since a five- or +six-field struct is copied into the handler on every command. But it does not have to store them — +see §8, where the `marc/respite` spike keeps `RespContext` to four fields and derives `CommandMap` +through the connection. That factoring is better and keeps a `readonly struct` viable. + +**Cost: the context type must be public.** The accessibility chain is forced, and verified +cross-assembly — an `internal` handler constructor fails at the consumer call site with +`CS1729: does not contain a constructor that takes 3 arguments`, because it is the *consumer's* +lowered code that constructs the handler. Public ctor therefore implies a public parameter type. + +Its **members can all be internal**, though: a `public sealed class` whose `CommandMap`/`KeyPrefix`/ +`BufferManager` are internal works cross-assembly and gives consumers a name they can neither +construct nor read from. Verified. That is a small commitment, but a permanent one, so it belongs in +`PublicAPI.Shipped.txt` deliberately rather than being noticed at pack time. + +If the context is unavailable for some path, command resolution can be **deferred** instead — the +handler stores the `RedisCommand` and the concrete implementation resolves it at `Close`/`Execute`. +That is the same mechanism §3.2 already needs, and it keeps mocks working. + +#### How big is it, and should `.Context` be a field? + +Measured: **`RespContext` is 64 bytes**, which is past the point where the JIT keeps a struct in registers, +so a by-value copy is a real one. That prompted the question of whether the grouping structs should expose +their context as a public *field* rather than a property, to avoid a copy on `strings.Context`. + +**No — but the measurement points at something better.** The 64 bytes break down as four references (32), +a `CancellationToken` (8), `Database` + `ServerType` (8), and **`RedisChannel ChannelPrefix` (16)**. A +quarter of every context copy is a channel prefix that only pub/sub uses and that the `Strings`, `Hashes` +and every other data-type group never touch. + +So the fix is to **shrink the thing being copied**, not to dodge one copy of it. **Done:** `ChannelPrefix` +moved into the services slot that already existed for optional capabilities (§6.7), taking the context +from **64 bytes to 48** - and later to **40**, when cancellation moved from context state to a per-call +argument and took its `CancellationToken` field with it - a quarter off *every* copy, including the ones inside `Send` on the hot path, +rather than only the rare external `.Context` read. Resolving it now costs a type test, paid only by code +that actually writes a channel. + +**The slot became a chain to make this work.** One service was enough while the cache was the only one; +two are not. A `ServiceLink` holds a service plus whatever was already there, and adding **prepends** - so +the most recent of a type wins by lookup order. That removes two pieces of code rather than adding them: +"replace" needs none, because a later add shadows an earlier one; and "remove" needs none, because a veto +entry shadows a lookup without emptying the chain. An array would have to be copied on every add; a link is +one allocation, immutable, and shared by every context clone. + +> **Correction.** This paragraph originally read "*and 'remove' needs none, because setting a prefix back to +> `default` shadows it with an empty one that reads as absent*" - treating free replacement and free removal +> as a virtue of the mechanism, and wiring the channel prefix straight onto it. For a *capability* like the +> cache that is right, and `WithoutCache()` is exactly that shape. For a **prefix** it is a bug, and it +> shipped as one: `WithChannelPrefix(a).WithChannelPrefix(b)` gave `b`, and `WithChannelPrefix(default)` +> escaped the prefix entirely. Both halves of keyspace isolation have to behave the same way, and the other +> half already composed - it folds via `WithPrefix` (§8.2), matching `DatabaseExtensions.WithKeyPrefix`, +> which detects an already-prefixed database and re-wraps the *inner* one with the two prefixes joined. +> The reason is the same one that made the slot compose rather than assign in the first place: a context is +> handed down through code that does not know what its caller applied, so a library reaching for its own +> channel namespace would silently cancel the tenant isolation above it, with a well-formed frame going to +> the wrong channel and nothing to see afterwards. **Rule: capabilities may be vetoed; prefixes only ever +> compose, and there is no escape** - you cannot un-prefix a `RedisKey` or unwrap a decorator either. +> (The null-prefix early-out that remains is an allocation saving, not the mechanism: composing nothing onto +> the existing bytes already yields the existing bytes. Confirmed by mutation, since a guard that looks +> load-bearing and is not is exactly what a later edit gets wrong.) +> +> **And then the names followed the semantics.** Fixing the behaviour left `With*` still saying the wrong +> thing: `With` reads as "the result differs in this respect", which invites "so the second call wins" - the +> exact misreading that had just been a bug. On the context they are now **`AppendKeyPrefix`** and +> **`AppendChannelPrefix`**, which say what happens when there is already a prefix and leave no room for the +> other guess. `Append` and not `Prepend` because the *new* prefix lands nearest the key: +> `AppendKeyPrefix("a").AppendKeyPrefix("b")` sends `k` as `abk`. Everything else on the context keeps `With*`, +> which is now a real distinction rather than a habit: `WithDatabase`/`WithServerType` replace a value, +> `WithCache`/`WithScriptCache` rebind a capability by shadowing, and only the prefixes accumulate. +> `DatabaseExtensions.WithKeyPrefix` on `IDatabase` is **shipped and stays** - renaming it would be a source +> and binary break on a widely-used API to fix a name, which is not a trade worth making. + +It is allocated per context *configuration* and never per command - and only from the second service +onwards, since a context with exactly one keeps the bare object and never sees the chain at all. + +Against the public field specifically: the JIT inlines a trivial getter, so partial uses like +`strings.Context.Database` are usually forwarded anyway; and a public field locks the representation, +which is precisely what the paragraph above wants to change. (Style is not the objection - +`.editorconfig` sets SA1401 to `silent`, so public fields are allowed here.) + +### 3.4 The context is not a new idea — it is `MessageWriter`'s parameter list + +Long term this replaces `MessageWriter`, and that is the clearest way to see what the context is for: +`MessageWriter`'s constructor **already takes it**. + +```csharp +public MessageWriter(byte[]? channelPrefix, CommandMap? map, IBufferWriter writer) +``` + +`TestHarness` — already `[Experimental]`, already built for "render RESP and inspect the bytes" — goes +one further and carries all three prefixes: + +```csharp +public class TestHarness(CommandMap? commandMap = null, RedisChannel channelPrefix = default, RedisKey keyPrefix = default) +``` + +So the context is that triple plus the routing and cancellation state (`Database`, `ServerType`, +`CancellationToken`). `TestHarness` is the closest thing to a prototype already in the tree. + +**The two prefixes reach the wire by different routes today**, which is the asymmetry the context is +meant to end: + +| | How it is applied today | Conditional? | +| --- | --- | --- | +| `ChannelPrefix` | writer state, applied at write time (`MessageWriter.cs:77`) | yes — skipped when `channel.IgnoreChannelPrefix` | +| `KeyPrefix` | rides on the `RedisKey` itself, put there upstream by the `KeyPrefixed*` decorators | no | + +`TestHarness` mirrors that split exactly — it hands `ChannelPrefix` to the `MessageWriter` but simulates +the decorator for keys by rewriting the arguments (`TestHarness.cs:133`). + +`IgnoreChannelPrefix` is not incidental: keyspace and keyevent notification channels are server-generated +names and opt out (`RedisChannel.cs:336`, `:422`), so `AppendFormatted(RedisChannel)` has to honour it +rather than prefixing unconditionally. The read side already strips the channel prefix +(`PhysicalConnection.Read.cs:817`); keys never got the equivalent, which is §8.4's read-half problem. + +### 3.5 A struct context must be valid in its `default` state + +If the context is a `struct`, `new RespContext()` binds the **implicit parameterless constructor** that +zeroes every field — *not* an all-optional-arguments constructor, however tempting that looks. So no +field may be assumed non-null, and `CommandMap` has to fall back to `CommandMap.Default` on read. + +Found the hard way: every test in the spike threw `NullReferenceException` at the first +`AppendFormatted(RedisCommand)`. It fails at the first command rather than at construction, which is the +wrong end to debug from. + +--- + +## 4. Deferred composition + +For conditional arguments: + +```csharp +using var r = writer.Begin(RedisCommand.SET, $"{key}{value}"); +if (withTtl) { r.AppendFormatted("EX"); r.AppendFormatted(300); } +if (withNx) r.AppendFormatted("$2\r\nNX\r\n"u8.Resp()); +var span = r.Close(); // back-fills *N into the reserved prologue, right-aligned +``` + +Verified output (parsed back with RESPite's `RespReader`, `DemandEnd()` enforcing exact consumption): + +``` +*6|$3|SET|$5|mykey|$7|myvalue|$2|EX|$3|300|$2|NX| +*5|$3|SET|$5|mykey|$7|myvalue|$2|EX|$3|300| +*2|$3|GET|$5|mykey| +``` + +Also passing: empty bulk string, multi-byte UTF-8, a 5000-byte payload forcing a pool regrow *mid-build* +(after the prologue is reserved), negative integers, and 22 args forcing a two-digit `*NN` header. + +**The trade:** conditional appends make the total arg count runtime-only, so the compile-time-constant +header is lost. Keep the single-expression path alongside for fixed-arity commands, where `*N` stays +constant. + +`using` works on both forms, and matters here: arbitrary user code sits between construction and +`Close()`, so a throw in that window leaks the rented buffer. + +**Gotcha:** `using var` cannot be passed by `ref` (CS1657). Mark resolution members `readonly` so `in` +works, or callers are forced into `try`/`finally`. + +### 4.1 `Compose` / `Render(ref cmd)` — the shape for optional arguments + +> **`cmd.Append($"…")`.** A conditional fragment is now written the same way as the command itself: +> ```csharp +> var cmd = ctx.Compose($"{RedisCommand.SET}{key}{value}"); +> if (withTtl) cmd.Append($"{RespLiterals.EX}{ttl}"); +> using var frame = ctx.Render(ref cmd); +> ``` +> rather than a sequence of `AppendFormatted` calls whose order is the caller's to keep straight. +> +> **It moves the command rather than referring to it.** The obvious design — a handler holding +> `ref RespRequestBuilder` and forwarding each call — does not compile on **any** target: *CS9050, a ref +> field cannot refer to a ref struct*. That is a language rule, not a down-level runtime gap, so narrowing +> the target frameworks would not have helped. (netfx adds CS9064 on top, but it is not the blocker.) +> +> So the command is copied into the handler, appended to, and assigned back — **including when a growth +> inside the window swapped the array**, which is the case that distinguishes a move from a share, and has +> its own test. +> +> **The move is completed at both ends.** The constructor resets the source to `default` after copying it, +> and `Append` resets the handler after assigning it back, so exactly one copy owns the pooled array at +> any instant. Without the first reset the command spends the append window as a second owner, still +> pointing at an array that a growth may already have returned to the pool; the reset makes an escape from +> that window — an exception mid-fragment — leave an empty command rather than a live-looking one. +> +> `default` rather than merely clearing the buffer, because `_hasCommand` goes false with it: every path +> off a moved-from handler is then a clean throw naming the problem (`Complete`, every `AppendFormatted`) +> or a no-op (`Dispose`, so no double return to the pool), and none of them is a `NullReferenceException`. +> This is the ownership-transfer idiom `Complete` already used for handing the buffer to a frame. +> +> That reset is also the reason the constructor parameter is `ref` rather than `in`. `in` compiles — the +> constructor genuinely only reads — and it was briefly the signature on those grounds; writing the move +> down properly made `ref` the accurate one. Both mutants (dropping either reset) are caught by test. +> +> **There is only one handler type**, which is what makes this safe rather than merely neat. A separate +> proxy type would need its `AppendFormatted` overloads kept in step with the command handler's - and the +> failure would be quiet, since adding one there without adding it here just makes `cmd.Append($"{x}")` +> stop compiling, with nothing to say why it works in the command and not in the append. The handler for +> an append simply **is** the command handler, so an append accepts exactly what the command does by +> construction. (That guard was written, as a reflection test, before the single-type version replaced the +> need for it.) +> +> **Optional arguments did not need `Append` after all.** `Append` was built for `if (cond) cmd.Append(...)`, +> and it is still the right tool for a fragment whose *presence* is a branch in the caller's own logic. But +> an argument that knows it might be absent can just say so: `AppendFormatted(Expiration)` and +> `AppendFormatted(ValueCondition)` write between zero and three tokens, so the whole of SET is +> +> ```csharp +> $"{RedisCommand.SET}{key}{value}{when}{expiry}" +> ``` +> +> with no branch at all. That is the first place the design pays for itself against the existing code rather +> than merely matching it: `RedisDatabase.GetStringSetMessage` is a ~17-branch decision tree, and most of +> those branches are not about Redis — they pick between fixed-arity `Message.Create` overloads, one branch +> per token count. Arity is free here, so they evaporate. +> +> Order is the documented grammar, `SET key value [NX|XX|IFEQ cmp] [GET] [EX s|...|KEEPTTL]`, i.e. condition +> before expiration. Redis parses the tail as an order-insensitive loop — which is how the legacy builder +> gets away with emitting `EX n XX` — but other RESP servers need not be as forgiving. +> +> **The new surface emits canonical `SET` only**, where the legacy builder also reaches for `SETNX`, +> `SETEX`, `PSETEX` and `DEL`. `SETEX`/`PSETEX` are pure arity relics with identical semantics and reply. +> `SETNX` is **not** a relic — it answers `:1`/`:0` where `SET ... NX` answers `+OK`/nil — so collapsing it +> is a real, deliberate divergence: `SET ... NX` has been available since 2.6.12, and one reply shape beats +> two. +> +> **The operand tokens have one home.** `Expiration.OperandResp` and `ValueCondition.KeywordResp` hold the +> mode/keyword selection, and each writer does only its own plumbing around them, so the `MessageWriter` +> path and the handler path cannot disagree about what an `Expiration` *means*. Pinned by a test that +> renders the same command through both writers and compares bytes, across the whole matrix. +> +> Two things make it legal, both worth knowing because the errors are opaque: +> `Append` is an **extension** with an explicit `ref` parameter rather than an instance method, because as +> an instance method the compiler must pass `ref this` into the handler's constructor and then refuses the +> call (CS8350/CS8352); and that parameter is **`scoped`**, which is how "this reference does not escape" +> is said. The call site is identical either way. + + + +Implemented in the spike (§9): + +```csharp +var cmd = ctx.Compose($"{RedisCommand.SET}{key}{value}"); +if (withTtl) { cmd.AppendFormatted(ex); cmd.AppendFormatted(ttl); } +using var frame = ctx.Render(ref cmd); +``` + +`Compose` carries `[InterpolatedStringHandlerArgument("")]` and simply returns the handler. + +Three initializer forms exist, all returning the builder: + +| Form | When | +| --- | --- | +| `ctx.Compose($"{cmd}{a}{b}")` | command as the first hole | +| `ctx.Compose(cmd, $"{a}{b}")` | **preferred** — command as a real argument (§3.1), so the map is consulted before the rent | +| `ctx.Compose(cmd, argHint)` | no interpolated part at all, for a fully dynamic argument list | + +The last is for the variadic case — `DEL` over a runtime-sized key array, where there is no fixed prefix +to interpolate: + +```csharp +var cmd = ctx.Compose(RedisCommand.DEL, keys.Length); +foreach (var key in keys) cmd.AppendFormatted(key); +using var frame = ctx.Render(ref cmd); +``` + +`argHint` only sizes the initial rent; it is not a promise, and appending more simply grows the buffer. +**`Render(ref cmd)` needs no new overload** — and could not have one, since the attribute does not change +the signature: it binds to the same `Execute`, because the interpolated-string-handler conversion applies +only when the argument *is* an interpolated string. Passing a real variable by `ref` is an ordinary +argument and the attribute is ignored. Verified. + +The trade is the one from §4: the argument count is only known at `Complete`, so `*N` is back-filled +rather than a compile-time constant. `ComposedHeaderGrowsWithLateArguments` pins the case that would +otherwise be silently wrong — three arguments at the call site, twelve by execution, so a compile-time +`*3` would have framed a corrupt command. + +Keys appended after the interpolation still track and route normally +(`ComposedKeysStillTrackAndRoute`): the second key of an `SMOVE` arrives via `AppendFormatted` and is +still marked and folded into the slot. + +**Ownership:** `Execute` takes the buffer on success, so there is nothing to dispose afterwards. But the +window between `Compose` and `Execute` is arbitrary user code, and CS1657 means the handler cannot be +held in a `using` while also being passed by `ref` — so a throwing window needs `try`/`finally` calling +`Dispose`, not `using`. + +--- + +## 5. Key and slot accumulation + +Needed for routing (cluster slot) and client-side cache invalidation. + +### 5.1 Routing is free + +Slot folding is O(1) state — one `int` — using the same logic as +`ServerSelectionStrategy.CombineSlot` (`ServerSelectionStrategy.cs:272`). No key storage at any arity. + +Better still, the handler can fold the slot over **the bytes it just wrote**, rather than +re-materializing the key. `RedisKey.CopyTo(Span)`/`TotalLength()` write straight into the +output buffer; `GetHashSlot` today has to copy the key into a separate scratch buffer purely to hash +it (`ServerSelectionStrategy.cs:67-92`). + +Keyspace-isolation prefixes must be applied **before** both the write and the slot. + +Verified: + +``` +zero keys slot=NoSlot keys=0 +one key slot=10778 keys=1 [user:1] +three keys, shared hashtag slot=4574 keys=3 [{u1}:name, {u1}:age, {u1}:email] +three keys, cross-slot slot=MultipleSlots keys=3 [alpha, beta, gamma] +prefix: slot(user:1)=10778 slot(tenant7:user:1)=11022 (differ) +``` + +### 5.2 Key marks — an MSB-discriminated `ulong` + +Measurement: testing a bitmap costs **nothing** — recovering one key of N and recovering all N are +indistinguishable. The entire cost is the RESP walk (~40–100 ns for typical small frames, ~400 ns at +65 args), versus single-digit ns for a direct slice from a stored offset. So the axis that matters is +**scan vs no-scan**, not bitmap vs offsets. + +``` +MSB clear → [ offset_b:31 | offset_a:31 ] 0, 1 or 2 keys, resolved with NO scan +MSB set → bitmap of arg indices 3+ keys, scan required +zero → no keys +``` + +- Offsets are **byte offsets of the `$` of the fragment**. No length needs storing — `$3\r\n` is + self-describing. +- 31 bits is ample (`proto-max-bulk-len` caps at 512 MB). +- Zero is a free sentinel for an empty slot: offset 0 can never be a key, because the frame starts + `*N\r\n`. +- Two slots is worth it — two-key commands are common (`SMOVE`, `RENAME`, `LMOVE`, `COPY`, + `ZRANGESTORE`, `BITOP`, `SINTERSTORE`). +- Beyond arg 63: rented `long[]`. Rare, and those are the variadic bulk commands already allocating. + +**Offsets must be buffer-absolute, not frame-relative.** `Close()` right-aligns `*N` into the reserved +prologue, so the *frame* start moves with the digit count of N. A frame-relative offset is then off by +one byte in the two-digit case — and one byte before a `$` is still inside the previous fragment's +CRLF, so you often parse *something plausible* and silently register the wrong key. Verified: with a +15-arg command (`*15`), buffer-absolute offsets still resolve correctly. + +Verified: + +``` +PASS single key, 1-digit arg count keys(no scan)=[user:1] +PASS two keys keys(no scan)=[src:set, dst:set] +PASS single key, 2-digit count (frame start moved) keys(no scan)=[user:1] + three keys -> scan required (as designed) +PASS zero keys keys(no scan)=[] +``` + +**Do not expose `System.Range` in the key-resolution API.** `Index`/`Range` polyfill fine down-level +(the compiler matches them by name), but the polyfill must be `internal` — public would collide with +the real types on newer TFMs. A public method taking `Span` then fails with +`CS0051: Inconsistent accessibility`. Use a purpose-built `(offset, length)` struct. + +**~~Open seam~~ — resolved.** The framing above ("there aren't spare bits to carry both") is true of the +*frame*, which must stay 64 bits. It is not true of the *writer*: `RespRequestBuilder` is a `ref struct` on +the stack with no size pressure, so it maintains **both** representations as it writes — the two offsets +and a full argument-index bitmap — and `Complete()` publishes whichever fits. Nothing needs re-deriving, +because nothing is discarded any more. + +The old code overwrote the two offsets with a bare `OverflowFlag` on the third key, so keys 1–3 were +recorded in *neither* form and the frame could report nothing at all; the bits it then set for keys 4+ were +never read by anything. `TryGetKeys` returning −1 was the only honest answer available to it. + +Now: + +| Keys | Encoding | Recovery | +| --- | --- | --- | +| 0 | zero | — | +| 1–2 | two 31-bit byte offsets | O(1), no scan | +| 3+ | `OverflowFlag` \| bitmap of argument indices | walk the frame, mapping index → range | + +The walk is length-prefixed skipping over `*N\r\n` + N bulk strings — no `RespReader`, no allocation. + +**The limitation that remains, and is inherent to a 64-bit field:** bit 63 is the mode flag and argument 0 +is always the command, leaving bits 1–62, so **a key at argument index above 62 cannot be recorded**. That +case sets bit 0 as a "truncated" marker and `KeyCount`/`TryGetKeys` report **−1** — deliberately *not* a +partial list, because a caller tracking keys for invalidation would believe a partial list was complete and +would cache something it could never invalidate. `RespClientCache.TryBeginFill` declines such frames. + +Going beyond 62 would mean heap-allocating the key list per frame, which costs an allocation on every +multi-key command to serve a case that is rare and already enormous. Not worth it unless something real +turns up. + +**Rejected:** falling back to "treat every argument as a key". Over-invalidation is safe by protocol — the +server does it deliberately when its tracking table overflows — but this would register *value* bytes as +tracked keys, polluting the key table and inviting spurious invalidation from unrelated keys that happen to +match a value. Safe, but it degrades the cache in a way that is hard to observe. + +--- + +## 6. The frame as cache key + +Because the rendered bytes are the client-side cache key, **canonicality is a correctness property**, +not tidiness: two logically identical commands must render byte-identical or you get duplicate entries +and phantom misses. `CommandMap` already uppercases command names (`AsciiHash.ToUpper`, +`CommandMap.cs:270`); that has to hold for every fragment. + +`Resp.Raw` is the hole — an opaque blob bypasses every normalisation: + +```csharp +$"{cmd}{key}{"$2\r\nex\r\n"u8.Resp()}" // lowercase +$"{cmd}{key}{(RedisValue)"EX"}" // uppercase +``` + +Same command, two cache entries, forever. Silent, so this is the highest-value analyzer rule. + +Keyspace prefixes fall out correctly for free — applied before the write, so tenants cannot collide. + +### 5.3 `RespCommand`: resolved once, usable in either position + +`"FT.SEARCH".Command()` parses, validates and (optionally) frames a command name once. What it stores +depends on whether this library knows the name, and that split is a correctness requirement rather than an +optimisation: + +| | stored | why | +| --- | --- | --- | +| known (`"GET"`) | the `RedisCommand` | `CommandMap` is per-context and may rename **or disable** it; the map already holds the bytes | +| unknown, casual | the `string` | inline use encodes straight into the frame buffer - preforming would allocate an array to copy from and discard | +| unknown, `preform: true` | framed `byte[]` | a `static readonly` field pays once, then every use is a `memcpy` | + +**`preform` has no effect on a known command.** `CommandMap` stores every mapped name as a pre-framed RESP +fragment already — *"ready to throw directly into the stream"* — so the bytes are preformed per map, which +is the only place they can be: the map is what decides them. + +**Preforming an unknown command is safe**, and the reason is worth knowing: `CommandMap` is built by +walking the `RedisCommand` enum, so an override keyed on a name that does not parse — `FT.SEARCH`, +`JSON.GET` — is **silently ignored**. Nothing could rename it, so there is nothing to defer to. (That is +also a gap: module commands cannot be renamed or disabled client-side at all, while a server-side +`rename-command` on one works fine and is undetectable. Orthogonal, but more visible once module commands +are first-class.) + +A `u8` overload takes the name as bytes, so generated code and `static readonly` fields need no `string`: +`TryParseCI` matches on bytes directly, so even a known command needs no transcoding. + +**Position decides the meaning, and the bytes are identical.** First, it is the command; later, it is an +argument that names one — `$"{command}{Info}{target.Command()}"`. That second case is not a curiosity: a +server knows a renamed command **only by its new name**, so `COMMAND INFO HGET` returns nothing where +`HGET` was renamed, and you must pass the mapped spelling. Taking it from the map is the only way to get +it right, which is exactly what appending a `RespCommand` does. + +Validation happens at resolution, not on the wire: a name carrying CR, LF or a space would desynchronise +the connection for every subsequent command — the `SER011` hazard — so it is rejected once, where it is +free. The framing itself is ours, which is what distinguishes this from a hand-built fragment. + +### 6.1 Three incremental folds + +All O(1) state, all during the write, none needing a second pass: + +| State | Purpose | +| --- | --- | +| `int _slot` | routing | +| `ulong _keyMarks` | key resolution for invalidation | +| rolling hash | cache probe | + +A lookup is then hash → bucket → `SequenceEqual`, with no walk unless a real collision. + +### 6.2 The database number is not in the frame + +`SELECT` is a separate command on the connection, so `GET foo` on database 0 and database 3 render +**byte-identically**. Cache identity is therefore `(frame, database)`, never the frame alone. + +This is obvious once stated and very easy to overlook, precisely because everything *else* that affects +identity is already in the bytes — the key prefix, a renamed command from the `CommandMap`, every +argument — so the frame feels self-sufficient. The context already carries `Database`, so the fix is to +fold it into the hash alongside the bytes; the cost is remembering to. + +Pinned by `DatabaseIsNotPartOfTheRenderedFrame`. + +Two neighbours, which resolve differently. + +**The multiplexer**, if a process talks to more than one deployment: free by scoping, assuming the cache +is per-multiplexer. Worth not hoisting it somewhere more shared without revisiting. + +**The protocol version is not an identity input**, despite RESP2 and RESP3 response shapes differing. +It is negotiated per `PhysicalConnection` (`SetProtocol`, `PhysicalConnection.cs:372`, propagated to the +bridge; `ServerEndPoint.cs:148` reads it back from the interactive connection), so mixed protocols +within one multiplexer are reachable *simultaneously* — a cluster mid-upgrade, or a primary and replica +at different versions. + +That is still not a reason to key on it. Since the key is `(frame, database)`, both shapes collide on +the same entry: there is exactly one, holding whichever protocol wrote it last, and any reader parses it +because the result processors have to be shape-tolerant anyway — which RESP3 support requires of them +generally. No duplicate entries, no hit-rate cost; the protocol simply does not participate. + +### 6.3 Caching the result: blob by default, value by exception + +`HybridCache` is the model worth copying — see +[Reuse objects](https://learn.microsoft.com/aspnet/core/performance/caching/hybrid?view=aspnetcore-10.0#reuse-objects). + +Its default is that every retrieval deserializes, so each concurrent caller gets a **separate instance**. +That is deliberate: it preserves the `IDistributedCache` behaviour most callers are migrating from, so +adopting `HybridCache` cannot introduce concurrency bugs. (`string` and `byte[]` are handled internally; +everything else goes through a serializer.) Reuse is opt-in, and requires **both**: + +- the type is `sealed`, and +- the type carries `[ImmutableObject(true)]`. + +Applied here, the default is to cache the raw RESP response bytes and re-run the `ResultProcessor`. +That is safe for any `T`, and it is the same property that makes §6.2 work: the processor is the single +place that tolerates RESP2 versus RESP3, so a cached blob is readable whichever shape it holds. + +Value-caching is then the optimisation — and **the `HybridCache` opt-in does not port directly**. +`RedisValue` and `RedisKey` are `readonly struct`s, so the `sealed` half is free, but +`[ImmutableObject(true)]` would be untrue of them, and a type-level attribute cannot express why: for +`RedisValue` the answer depends on the *value*, specifically its `StorageType`. + +| `StorageType` | Backing | Safe to cache by value? | +| --- | --- | --- | +| `Null`, `Int64`, `UInt64`, `Double` | the overlapped field | yes — self-contained | +| `String` | a `string` | yes — immutable | +| `ShortBlob` | 1-8 bytes inline in the overlapped field | yes — self-contained | +| `ByteArray` | a `byte[]` | **no** — see below | +| `MemoryManager`, `Sequence` | memory owned elsewhere | **no** — see below | + +**`ByteArray` aliases.** The `byte[]` conversion returns the **internal array** in exactly one case — +`StorageType.ByteArray` where the value spans the whole array (`RedisValue.cs:1198-1200`; `RedisKey` +does the same via `TryGetSimpleBuffer`). Every other branch copies. So a caller can take that array, +mutate it, and poison every other holder of the same cached instance. + +**`MemoryManager`/`Sequence` are a different hazard.** These reference memory the `RedisValue` does not +own, which may be a pooled lease that is later recycled. Unsafe to *retain* — unless the lease is pinned, +which is exactly what §6.4 has the cache entry doing. See "windows, not copies" below: under pinning +these stop being a hazard and become the preferred representation. + +So the eligibility test wants to be a value-oriented predicate over `StorageType`, not a type-level +marker — cheap to evaluate, but evaluated per value. The alternative is to copy on the way in for the +unsafe kinds, which costs an allocation exactly where value-caching was supposed to save one. + +#### Idea: never hand the cache an exact-size array + +The aliasing branch fires only on `_index is 0 && _length == arr.Length`, so ensuring a cached value is +never backed by an exactly-sized array forces the copying branch instead. That is structural rather than +incidental — `byte[]` cannot express a partial view, so the operator *has* to copy — and it is close to +free, because a pooled rent is over-sized by construction. + +It defends one route, though, not the invariant: + +| Route | Over-sizing defends? | Why | +| --- | --- | --- | +| `(byte[])` | **yes** | `byte[]` cannot be a partial view, so the operator must copy | +| `(ReadOnlyMemory)` | no | returns `new ReadOnlyMemory(arr, _index, _length)` at any size (`RedisValue.cs:1353`) | +| `(ReadOnlySequence)` | no | delegates to the above | + +`ReadOnlyMemory` is read-only only by convention: `MemoryMarshal.AsMemory` makes it mutable in one +call, with no `unsafe`. Whether that counts is a judgement call — reaching for `MemoryMarshal` to mutate +someone else's read-only memory is arguably "you broke it, you own it", and on that reading over-sizing +does close the practical *mutation* surface. + +The *lifetime* half is not a judgement call: a well-behaved caller can hold the returned +`ReadOnlyMemory` past eviction, or past the pooled array being recycled, with no misuse at all. +That is §6.4 again, and over-sizing does nothing for it. + +**Better variant: start at index 1.** Breaking the `_index is 0` half instead costs exactly one byte and +is a property of how the `RedisValue` is *constructed* — fully under our control — rather than depending +on the allocator having over-sized the array. + +Note it is **not** already true for response-derived values. `RedisValue.FromRaw` copies anything over +`MaxInlineBytes` (8) into an exactly-sized array at index 0: + +```csharp +internal static RedisValue FromRaw(ReadOnlySpan bytes) +{ + if (bytes.IsEmpty) return EmptyString; + if (bytes.Length <= MaxInlineBytes) return new RedisValue(bytes); // inline + return bytes.ToArray(); // exact-size, index 0 +} +``` + +So every response payload over 8 bytes is exactly the aliasing case — which also means +`byte[] blob = db.StringGet(key)` is **zero-copy today**. Applying index-1 blanket in `FromRaw` would +turn that common pattern into a copy per call: a real pessimisation, not a free byte. + +Applied at *cache insert* rather than universally, though, it is better than an eager defensive copy, +because **it makes the copy lazy**: a cache hit that never asks for `byte[]` pays nothing, and one that +does pays exactly the copy it needed for safety anyway. + +The `ReadOnlyMemory` caveat above is unchanged either way — that conversion windows into the array +at any index. + +#### Better still: windows, not copies — and the hack disappears + +If the cache entry pins the buffer (§6.4), a cached value need not be copied out of it *at all*. The +payload is a slice of the frame, sitting between `$len\r\n` and the trailing `\r\n` — so a `RedisValue` +constructed over that buffer has `_index > 0` **and** `_length < arr.Length`. Both halves of the aliasing +condition fail on their own, because the trim was required regardless. No deliberate off-by-one, no +deliberate over-allocation, nothing to explain to a future reader. + +The machinery already exists: the `ReadOnlyMemory` constructor takes exactly this shape +(`MemoryMarshal.TryGetArray` → `_index = segment.Offset; _length = segment.Count; _obj = segment.Array`), +and values of 8 bytes or fewer still go inline as a self-contained `ShortBlob`, so the small case has no +coupling at all. + +This also collapses the blob-versus-value distinction for blob-shaped payloads: a `RedisValue` windowed +onto the cached buffer *is* both. Re-materialising it is an offset computation rather than a parse, so +the "re-run the parser" default costs almost nothing — and the immutability question that motivated +value-caching does not arise, because nothing was ever copied out to alias. + +What remains is lifetime, and it is sharper rather than softer: the handed-out value now points *into* +the entry's lease, so a caller holding a `RedisValue` across eviction is looking at recycled memory. Note +the conversions are on our side here — `(byte[])` and `(string)` both copy out of a windowed value — so +the danger is narrowly a caller who retains the `RedisValue` itself and materialises later. + +That is the same §6.4 problem, but concentrated in one place (the entry's lease) rather than spread +across copies, which is probably where you want it. + +Array returns are common across this API, and they are exactly the poisoning hazard that blob-by-default +exists to prevent, so the default matters more here than it might elsewhere. + +Nothing in the repo is annotated for this today — no `[ImmutableObject]`, no `HybridCache` reference. + +#### The same trick, already anticipated + +`HybridCache`'s own cache-key guidance recommends writing the key as an interpolated string *inline at +the call site*: + +> Notice that the inline interpolated string syntax (`$"..."` [...]) is directly inside the +> `GetOrCreateAsync` call. This syntax is recommended when using `HybridCache`, as it allows for planned +> future improvements that bypass the need to allocate a `string` for the key in many scenarios. + +That is this document's technique, in the public guidance of the library whose caching model §6.3 is +copying: keep the interpolation at the call site so a handler can consume the parts without ever +materialising a `string`. Worth knowing that the shape is already established rather than novel. + +### 6.4 Buffer ownership + +A pooled buffer **must not** be retained as a dictionary key without ownership transfer — `ArrayPool` +reuse would mutate live cache keys, and the failure mode is wrong data served from cache, not a crash. + +Resolution: the cache entry **pins the lease** for its lifetime; the buffer is returned on eviction. + +Two requirements: + +- **Dispose must be neuterable.** The transfer decision comes late (only after dispatch do you know + whether the response is cacheable), so the consumer's `using` is correct on every path *except* the + one where ownership moved. Precedent exists: `MemoryTrackedPool.MemoryManager.Dispose` does + `Interlocked.Exchange(ref array, null)` and only returns if it won (`MemoryTrackedPool.cs:58`). + `TransferOwnership()` wins that exchange first. +- **Accept permanent rounding slack.** A pooled lease is power-of-two sized, so a 130-byte frame pins + a 256-byte array for the entry's life — roughly a third overhead on retained bytes. Minor, and a + custom chunk pool with buckets fitted to the real frame distribution would tighten it. + +#### Implemented: `RespRequest` / `RespPayload` (see `InterpolatedWriterCacheKeyTests`) + +Two corrections to the sketch above, both found by building it. + +**Reference counting, not ownership transfer.** The neuterable-`Dispose`-plus-`TransferOwnership` design +makes every holder reason about whether ownership moved, and the answer is only known after dispatch. A +count gives every holder one rule — *whoever retains, releases*. The primitive already existed: +`RefCountedBuffer` (`src/RESPite/Buffers/RefCountedBuffer.cs`), which backs `RespResult`. It is a +`MemoryManager` specifically so every `Span`/`Memory` access routes through one liveness check, and +its `TryAddRef` is already increment-if-non-zero, with the same rationale this needs: + +> a reservation racing the final release must fail rather than resurrect a buffer that has already gone +> back to the pool + +So the read side is `TryGetValue(key, out payload) && payload.TryRetain()`, then `try`/`finally` with +`Release()` — success means *found* **and** *count incremented from non-zero*; a zero count is a miss, not +an error. (`MemoryTrackedPool` is the same idea but is behind `#if TRACK_MEMORY`, which is defined +nowhere — it is not a live facility.) + +**A lookup must not need ownership.** `Detach()` transfers the frame's buffer into a lease, and that lease +is an object: **48 bytes per call, measured**. On a cache *hit* — the common case — the caller never wanted +the buffer, so that is a per-lookup allocation buying nothing, which is precisely the cost this design +exists to remove. Hence `AsLookupKey()`, which borrows the frame's array with no lease and no allocation; +`Detach()` is for the miss path, where ownership is actually wanted. + +The two are the same struct, distinguished by `IsOwned`, and the safety property falls out: a borrowed key +**cannot be retained**, so the documented store idiom (retain, then add) cannot express "put a pooled array +into the cache and then hand it back to the pool". That is the §6.4 hazard made unreachable rather than +merely documented. + +Measured: a steady-state cache hit — render, probe, retain, read, release — allocates **zero** bytes. + +### 6.5 Exception paths + +The lowering puts construction and all `Append` calls in the *caller's* frame, before `Execute` is +entered: + +```csharp +var h = new Resp(...); // rents here +h.AppendFormatted(cmd); // can throw: disabled command +h.AppendFormatted(key); // can throw: a hole's property getter +Execute(h, handler); // consumer's using/try-finally only starts HERE +``` + +So on a throw in that window there is no handler for the consumer to dispose. Dropping the buffer is +the only available behaviour, and that is **accepted, not merely tolerated**: `DefaultInterpolatedStringHandler` +does exactly the same — it rents from `ArrayPool.Shared` and abandons the rental if an +interpolation throws, because the compiler emits no `try`/`finally` around the append sequence. Broken +usage dumping an incomplete buffer is the established behaviour of the pattern. + +The same precedent settles a second wart in the `Compose` path: the handler cannot be held by `using`, +because a `using` variable cannot be passed by `ref` (CS1657), so a throwing window between `Compose` and +`Execute` needs try/finally. `DefaultInterpolatedStringHandler` has exactly this shape and exactly this +limitation; it is a property of the pattern rather than of this design. + +It is also harmless here: `MemoryTrackedPool` is a thin wrapper over `ArrayPool.Shared` +(`MemoryTrackedPool.cs:34`) with no outstanding-rental tracking and no budget, so a dropped buffer is +simply garbage. + +Two notes: + +- **Validate the command before renting** where it is free to do so. A CommandMap-disabled command is + both the most likely throw here and the most likely to *repeat*, being configuration-driven. The + command-as-argument form (§4.1) gets this for nothing, since `command` reaches the constructor. This is + a tidiness win rather than a correctness one — see the `DefaultInterpolatedStringHandler` precedent + above — so it is not worth contorting the API for. +- **A bounded custom pool would invalidate this.** Dropping into `ArrayPool.Shared` is free because + Shared doesn't track; dropping a chunk from a bounded free-list permanently removes capacity and + silently degrades to allocating every time. `CycleBuffer.AppendOrRecycle(segment, maxDepth: 2)` shows + the bounded pattern is idiomatic here, so this needs care. Keep any dedicated pool unbounded — + allocate on miss, return opportunistically. + +--- + +### 6.6 Invalidation: two tables, not a cross-index + +Verified against the protocol first: an invalidation message carries **an array of key names and nothing +else** — no timestamp, no version, no epoch. A `null` in its place means `FLUSHALL`/`FLUSHDB`. Two further +properties shape the design more than the missing time does: + +- **False invalidations are normal.** The server's invalidation table is bounded; when it fills it evicts + by *pretending a key was modified*. Over-invalidation is routine traffic, so invalidation must be cheap + and correctness must never depend on it being precise. +- **Tracking ignores the database.** *"There is a single keys namespace, not divided by database numbers"* — + writing `foo` in db 3 invalidates a cached `foo` in db 2. + +**The structure.** Two independent lookups rather than one cross-indexed structure: + +| | key | value | +| --- | --- | --- | +| Table 1 — `RespClientCache` | rendered frame **+ database** | payload + the generations its keys had at send time | +| Table 2 — `RespKeyTable` | Redis key bytes, **no database** | a generation | + +A server invalidation touches *only* table 2: one hash, one stamp. It never enumerates cache entries, which +is the whole point — under `BCAST` we are told about every key touched on the server and almost none are +ours. The database asymmetry above is protocol-faithful and looks like a bug; it is commented as such. + +**Generations are global monotonic tickets, not per-key counters.** This is what makes removal and reuse +safe. A per-key counter restarting at zero can collide with a ticket a cached entry recorded before the key +was invalidated, and that entry would then validate against a key that had in fact changed. + +**Entries hold the key's node directly**, so validating a hit is a dereference and a `long` compare — table +2 is never re-hashed on the hot path. The price is one invariant: *a node that leaves table 2 must be +stamped invalid first*, or entries still pointing at it would never learn. Both that invariant and the +in-flight check below are pinned by mutation-tested cases. + +**The fill race is the reason any of this needs ordering.** An invalidation can land between send and +reply, and the server will not repeat it — it dropped the key from its table when it fired. Caching that +reply leaves *permanently* stale data. `TryBeginFill` captures generations at **send** time and +`TryComplete` refuses if they moved, which is the documented "caching-in-progress placeholder" without a +placeholder. + +Everything fails closed: an unresolvable key, a frame whose keys cannot be enumerated, a generation that +moved — all are misses. In particular a frame whose keys cannot be named is **refused outright**, since an entry that cannot be +invalidated must not be cached. Since §5.2's seam was closed that means only one thing: a key at argument +index above 62. `MGET` over three keys caches and invalidates on any of them; `MGET` over seventy does not +cache at all. + +Measured (`ClientCacheBenchmarks`): `OnInvalidate` is **~5-6 ns, zero allocation, flat from 1 to 100,000 +cached keys** — about 170M invalidations/sec on one thread, for both hits and misses. + +### 6.7 Sending through the cache, and what kind of cache this is + +The obvious hand-written shape is **wrong**, and not in a way a careful caller can fix: + +```csharp +if (!cache.TryGet(req, out resp)) +{ + resp = Execute(req); + cache.Add(req, resp); // an invalidation between these two lines is lost forever +} +``` + +By the time `Add` runs there is nothing left to compare against, so an invalidation that arrived during +`Execute` cannot be detected — and the server will not repeat it, having dropped the key from its table +when it fired. The result is a *permanently* stale entry. That is why the orchestration has to own the +call: it captures generations before it sends, so the completion can see that the world moved. It is not +sugar; **it is the only shape that is correct by construction**, and the explicit +`TryBeginFill`/`TryComplete` pair is for callers who need to interleave their own dispatch. + +A response that arrives after an invalidation is still *returned* — it is a legitimate answer for a read +that raced a write, and the caller would have got it anyway without a cache — it is simply not stored. + +**The cache is a participant in the send, not the entry point.** An earlier shape had the cache own the +call (`cache.GetOrExecute(...)`, with the command supplying its own `Execute`) — since removed. That was +backwards twice +over: a cache that calls the executor has to sit above dispatch and know how to send, and a *command* has +no business knowing how to send itself. Inverting it gives the shape the library already has — an executor +that sends, and a handler that is exactly the `ResultProcessor` role: + +```csharp +executor.Send(ref request, handler); // no cache +executor.Send(ref request, handler, cache); // with cache +``` + +Caching becomes one extra argument rather than a different API, so turning it on does not mean rewriting +call sites, and "no cache" is an ordinary case rather than a missing one. + +**Neither side of the executor is a span, and neither is a `byte[]`.** This is not a detail — it is what +makes the contract usable at all: + +- A **span request** cannot cross an `await`, so it rules out async; and it cannot be parked in a backlog + for a resend after a reconnect, so it rules out retries even when synchronous. +- A **`byte[]` reply** allocates on every call, which is the cost this design exists to remove. + +So both sides are pooled and reference-counted: `RespRequest` in, `RespPayload` out. The executor takes its +own reference with `TryRetain` if it needs the bytes past the call; the caller releases theirs either way. +`IRespHandler.Parse` *does* take a span, correctly — parsing is synchronous and runs inside the retained +window. + +This is also why `RespCacheKey` became **`RespRequest`**: the bytes about to be sent and the cache key are +the same object, and the request role is the primary one. + +`SendAsync` is deliberately **not** an `async` method. `async` forbids `ref` parameters, and the frame must +be consumed by reference so a caller's copy cannot be disposed twice; so the probe and hand-off are +synchronous and only the awaiting tail is a separate `async` method. **A cache hit therefore completes +synchronously and allocates nothing** — no state machine, no `Task`. + +`TryComplete` takes the payload rather than the bytes, so a cached reply is **shared with the caller, not +copied**: it is already in a pooled reference-counted buffer, and copying it to cache it would be waste. + +**The whole pattern is three members.** `IRespExecutor.Send`/`SendAsync`, +`IRespHandler.Parse(ReadOnlySpan)`, and the extension pair carrying all the orchestration — +so the ordering rule that makes caching safe lives in exactly one place we own, instead of being exposed to +every caller. (`IRespExecutor.Database` is data, not behaviour.) + +One method rather than two overloads, because **the cached path *is* the uncached path plus a probe and a +commit**: a request that cannot be cached — or a caller with no cache — falls through to the same tail +rather than duplicating it. `TryBeginFill` deliberately leaves the frame owned when it declines, which is +what makes that fall-through work. The optional parameter is acceptable only because this is experimental; +adding one to a shipped method is a binary break, so a shipping version would want overloads for headroom. + +That orchestration internalises three lifetimes, in descending order of how easy each is to get wrong: + +| | Handled by | +| --- | --- | +| Generations captured **before** the send | `Send` sequences the capture and the send itself | +| Payload retained across the parse, released in a `finally` | `Parse` is called inside the window | +| The request frame consumed on **every** path | `ref RespFrame`, neutered whether it became a key or not | + +**A command is one expression.** `SendAsync` takes the interpolated string directly - the `ref` is implied, +exactly as `Execute` already does - with **flags before the handler** so the handler can be omitted: + +```csharp +public ValueTask Get(RedisKey key, CommandFlags flags = CommandFlags.None) + => ctx.SendAsync($"{RedisCommand.GET}{key}", flags.WithRetryCategory(CommandRetryReadOnly)); +``` + +An omitted handler is resolved from `TResult` (`RespHandlers.Inbuilt`), with a throw naming the type if +there is none - at the call site, not when a reply arrives. `TResult` must be explicit, because C# infers +type arguments from arguments and never from a return type. + +**Flags are cumulative, and that is a correctness point rather than a style one.** The category must come +from `WithRetryCategory` at the call site, *not* from the parameter's default value. With +`flags = CommandRetryReadOnly` as a default, a caller passing `CommandFlags.FireAndForget` would silently +**replace** the category with nothing - losing both the retry semantics and, now, cacheability. People +expect flags to add. `WithRetryCategory` is first-wins, so an explicitly named category still beats ours. +`CommandFlagsExtensions` became public for this: an external command surface cannot express a default +category without it. Pinned by tests, since the failure is silent. + +None of the three is visible at the call site, which reduces to: + +```csharp +var req = ctx.Render($"{RedisCommand.GET}{key}"); +return executor.Send(ref req, handler, cache); // no using, nothing to release, nothing to order +``` + +Executor and handler are passed as interfaces, so hold **one instance of each and reuse them** — a `struct` +implementation would box per call. Reused instances allocate nothing per request. + +**Read-through, and no write path at all.** `Send` with a cache makes the cache own the fetch, which is +read-through; the raw `TryGet` + `TryBeginFill` pair is cache-aside. Neither write-through nor write-behind +applies, because **writes never go through this cache**. Coherence comes from the server telling us what +changed, which puts this closer to hardware cache coherence than to the application-caching taxonomy: we +hold no dirty state and never write back. + +Two consequences worth stating: + +- **Updating a cached value on write is not an option**, even in principle. Entries are keyed by rendered + frame and hold a response *frame*, so "write through" would mean synthesising what `GET foo` will return + after a `SET foo bar` — possible only for trivial commands and wrong in general. Invalidation is the only + sound answer. +- **`NOLOOP` reintroduces a write-side hook.** It suppresses invalidations for keys this connection + modified, so under `NOLOOP` the write path *must* call `OnInvalidate` itself. That is a write-through + concern in a design that otherwise has none, and it is the one place the "no write path" story breaks. + +Pinning also **keeps the key offsets valid**: buffer-absolute offsets stay resolvable for the entry's +whole lifetime, so keys can be recovered lazily from a cached entry without re-rendering. Copying +would have forced rebasing them by the frame-start delta — the same off-by-a-few-bytes hazard as §5.2, +reintroduced at a second site. + +### 6.8 Transition: reusing `Message` rather than rewriting the command surface + +`RedisDatabase` builds a `Message` and pairs it with a `ResultProcessor`. Those are **the same two +halves as the new API** — a request that renders itself, and something that turns a reply into a result — +so the existing command surface can feed the new pipeline without being rewritten. `RespFrameWriter` is a +working demonstration (`MessageToRespFrameTests`). + +| New API | Existing equivalent | +| --- | --- | +| the rendered request | `Message` + `MessageWriter` | +| `IRespHandler.Parse` | `ResultProcessor.SetResultCore(..., ref RespReader)` | +| cluster slot | `Message.GetHashSlot` — already computed, so **nothing to fold during the write** | +| argument count | already in the `*N\r\n` header the writer emits | +| key prefixes, channel prefix, command map | already applied by `MessageWriter` | + +**What bytes cannot supply is which arguments were keys**, which is why this is a writer and not a post-pass +over a rendered frame — §5.2's finding applies directly. The saving grace is that `MessageWriter` kept the +distinction at the call site: `Write(in RedisKey)` is a separate overload from `WriteBulkString(in +RedisValue)`. So the whole integration is **one hook** — `Write(in RedisKey)` reports the current offset — +plus an `IBufferWriter` that accumulates and packs the marks. + +Notes from building it: + +- `MessageWriter` is a `readonly ref struct`, so it cannot accumulate marks itself. The recorder is a + reference to the target writer, resolved **once per message** in the constructor (`writer as + RespFrameWriter`), so the per-key cost is a null check on an already-loaded field. +- **Cost: below the noise floor.** A/B on `SET key value`: 66.96 ns with the hook, 68.77 ns without — the + hooked build measured *faster*, which is proof the difference is run-to-run variance rather than signal. + So the cost is bounded below ~3%, not that it is zero. +- Offsets suffice for ≤2 keys; beyond that the frame's encoding is argument *indices*, which the recorder + derives by walking the finished frame once — off any hot path, and the same walk `TryGetKeys` does in + reverse. +- Both routes render **byte-identically**, pinned by a test. That is a correctness property, not tidiness: + the frame is the cache key, so two routes that disagreed would cache the same logical command twice. + +#### The other direction: a frame becomes a message + +`RespMessageExecutor` sends a pre-rendered frame through the existing pipeline - connection selection, +backlog, multiplexing, failover, all untouched. Two small pieces: a `Message` whose `WriteImpl` is a blit, +and a `ResultProcessor` that captures the raw reply (overriding `SetResult` rather than `SetResultCore`, so +it runs before `MovePastBof()` consumes the prefix bytes the capture needs). + +**One message type covers every pre-formatted command.** The library has **75 `WriteImpl` overrides across +20 files**, and they exist purely because each command shape writes itself differently; once the bytes +arrive already framed, there is one shape. That is the clearest single measure of what moving formatting +upstream buys - and it is scaffolding rather than a destination, since the `Message` machinery is expected +to go away entirely in favour of execution life-cycle state. + +This is what made `RespEndToEndTests` possible: `target.Strings.Set/Get` against a real server, with the +legacy API cross-checking that the bytes landed. + +**The minimal run now needs no wiring at all**, because `RedisDatabase.Context` is real: + +```csharp +var db = conn.GetDatabase(); +await db.Strings.Set(key, "marc"); +var value = await db.Strings.Get(key); +``` + +No cast, no executor, no context construction. The context is built once per database and cached - the +executor is a per-database object, and minting one per property access would allocate on a path meant not +to. `RedisBase.Context` still throws, so `IServer` and `ISubscriber` are untouched; `RedisDatabase` hides +it with `new`, which means the **interface mapping** must land on the derived member - if it ever landed on +the base, every extension member would throw, since they all reach the context through `IRespTarget`. That +is asserted rather than assumed. Before it, everything was validated against fakes - which +proves the shape but never that a server accepts the bytes, since only framing was ever in question. + +Still open for a real transition: a cacheability predicate (Redis excludes `FT.*`, probabilistic and +time-series types, and non-deterministic commands such as `HRANDFIELD`/`ZRANDMEMBER`/`HSCAN`), and running +a `ResultProcessor` against a cached payload — it takes `ref RespReader`, which `RespPayload.GetReader()` +supplies, but it also wants a `PhysicalConnection` and `Message` for error context. + +### 6.9 Cacheability: gate on the retry category, fail closed + +Cacheability cannot be a list of command names. `FT.*` is not in this library at all — it lives in +NRedisStack, reaching the server through `Execute`/`ExecuteAsync` — so any rule expressed as "these +commands are excluded" is unenforceable for exactly the commands most likely to be wrong. + +The flags already model this. The retry category is a 5-bit severity ladder in `CommandFlags` +(`Message.MaskRetryCategory`, bits 13–17), and `Message.UserSelectableFlags` **already includes it**, so an +external surface can declare a category today with no new API. So the gate is: + +```csharp +var category = flags & Message.MaskRetryCategory; +return category != 0 && category <= CommandFlags.CommandRetryReadOnly; +``` + +**Both halves matter.** Zero means "nobody declared one", and zero sorts *below* `CommandRetryReadOnly` on +the ladder — so a naive `<=` would read "nobody said" as "safe to cache", which is precisely backwards for +commands this library does not define. Undeclared must mean uncacheable. That is pinned by a test, because +it is the one that fails open if written carelessly. + +`flags` is therefore **not optional** on `Send`/`SendAsync`. Every `IDatabase` method in this library +already carries flags; whether a command may be cached is a property of the command, and the caller has to +say. + +**Read-only is necessary, not sufficient**, and this is a gate rather than the whole test. The exclusions +are *our* commands, so they belong in command metadata rather than in flags — a compile-time property of +our own enum should not be pushed onto every call site. + +**The metadata table already exists.** `CommandFlags.Category.cs` has a per-command `switch` supplying the +default retry category; a cacheability answer wants to sit beside it, not in a new structure. Same kind of +fact about the same enum. + +**The list is longer than first recorded** (found in review; every entry verified to return +`CommandRetryReadOnly` from that table, so all of them pass the gate today): + +| | why caching is wrong | +| --- | --- | +| `SRANDMEMBER`, `HRANDFIELD`, `ZRANDMEMBER` | non-deterministic: a cached "random" answer stops being random | +| `SCAN`, `HSCAN`, `SSCAN`, `ZSCAN` | cursor state; a cached page is meaningless | +| **`TTL`, `PTTL`** | **time-dependent**: the answer changes with the clock, with no key write, so *nothing ever invalidates it*. The same failure class as a keyless command — permanently wrong, not briefly | +| **`TOUCH`** | **the side effect is the point**: it bumps LRU/LFU state, and a cache hit skips that entirely, so the command silently stops doing its job | +| **`PFCOUNT`** | **a read that writes**: it caches the computed cardinality back into the HLL header, so a cache hit skips a real mutation | + +`TOUCH` is worth dwelling on, because the codebase already contains the evidence that the two axes +diverge. Its entry in the category table reads: + +> `case RedisCommand.TOUCH: // technically bumps LRU/LFU state, but that's not a "real" side effect worth blocking retries over` + +Correct for retry, and exactly wrong for caching. That comment is the clearest single argument that +cacheability cannot be read off the retry category. + +**One I questioned rather than accepted, and it was settled: `DUMP` stays cacheable.** It is correctly +invalidated — the payload is a deterministic function of the value, and the key is tracked, so a write +invalidates it like any other read. The case against was benefit rather than correctness: large payloads, +rarely re-read. That is not what this list is for. Everything else here would be *wrong* to cache; `DUMP` +would merely be *unrewarding*, and mixing the two makes the list harder to trust, because a reader could no +longer assume an entry means "unsafe". + +Two things settled it. `CacheOptions.MaxPayloadBytes` now refuses oversized replies generically, so the +size worry needs no per-command rule; and `NoClientCache` is exactly the control for a caller who knows +their `DUMP` is one-shot, which a bulk migration does. The public `Keys.Dump` documentation says so, rather +than leaving the caller to work it out. + +A caveat was drafted here about `DUMP`'s payload encoding the value's *internal* representation (listpack +versus skiplist, and so on), on the theory that identical logical content could serialise differently. It +was wrong, and is recorded as wrong because it is the kind of worry that looks prudent: the payload is +**opaque**, and its only guarantee is that `RESTORE` reconstructs a semantically equivalent value. The +bytes are not stable across server versions either, so a caller comparing two payloads is already outside +the contract. Whether the cached bytes match what the server would produce today is therefore not a +question anyone is entitled to ask - only whether they still restore to the right *value*, which is the +ordinary invalidation question and is answered the ordinary way. + +#### Opt-out, not opt-in + +The caller-facing control is **`CommandFlags.NoClientCache`** (bit 19), and caching is otherwise on by +default for anything that clears the gates. Opt-in was considered and rejected: it would mean touching +every `IDatabase` method, and a single omission makes the feature silently do nothing. + +The worry that argued for opt-in was an external command that is read-only, keyed, and *not* tracked by +the server — it would be cached and never invalidated. On inspection that population is close to empty: + +- `FT.*` takes an **index name, not a keyspace key**, so it is keyless and the rule above already refuses + it. (This was my counter-example, and it was simply wrong.) +- Probabilistic and time-series types (`BF.*`, `TS.*`) are keyed on *real* keyspace keys, so tracking and + invalidation work normally. The Redis docs exclude them because *"these types are designed to be updated + frequently, which means caching has little or no benefit"* — an efficiency argument, not a correctness + one, and precisely what an opt-out is for. + +What remains is a third party who writes their own module, enables client-side caching, declares a +read-only retry category, and whose module reads are not registered for invalidation by the server. Note +that doing *nothing* is already safe: an undeclared category is uncacheable, so the failure needs a +positive act of mis-declaration. And caching is globally opt-in in the first place. Treating that as caller +error is consistent with how this same enum already treats retry categories, where mis-declaring gets you +duplicate writes on a reconnect — a worse outcome that we already trust callers to avoid. + +`NoClientCache` suppresses the **probe as well as the store**: opting out has to mean the caller does not +receive a cached answer either, not merely that this reply is not kept. + +#### Why not a new rung on the retry ladder + +Tempting — it is a numeric range with gaps — but no: + +- **The caller wins on the ladder.** `WithCategory` is explicit: *"if the user has already specified a + category, that wins."* So opting out of caching via the category would *replace* the retry category, and + a caller suppressing caching on a churny value would silently change reconnect behaviour. +- **Inserting above `ReadOnly` breaks every `<=`.** A "read-only but uncacheable" rung reads as more + severe, so retry policies testing `<= CommandRetryReadOnly` would stop retrying it: a caching annotation + causing a retry regression. Inserting *below* avoids that but forces recategorising every read-only + command and leaves `ReadOnly` meaning "not cacheable". +- **The codebase already decided this.** `CommandServerSpecific` sits outside the ladder because it is + *"an orthogonal flag, not part of the `<=`-comparable severity ladder"*. Same shape, same answer. The + ladder orders one axis — is it safe to send again; cacheability asks another — will invalidation tell me + when this changes. + +#### Scripts: cacheable by default, opt out explicitly + +`EVAL_RO` and `EVALSHA_RO` also default to `CommandRetryReadOnly`, so they pass the gate today. They are +**not** simply another row above, because **cacheability is a property of the script, not of the command +name**. Two `EVAL_RO` calls can differ entirely: one deterministic and perfectly cacheable, the next +reading `TIME` or `RANDOMKEY`. The library cannot know, and a blanket "scripts are excluded" throws away +the cacheable majority to catch the minority. + +**Resolved: cacheable by default, and opting out is the caller's job.** The reasoning is stronger than +"the caller knows best", which on its own would be a weak default: + +- **`EVAL`/`EVALSHA` are excluded *by default*** — they default to `CommandRetryWriteAccumulating`, well + above read-only. So the population defaulting to cacheable is not "scripts"; it is *only* scripts where + the caller deliberately chose the `_RO` variant. +- **`_RO` is server-enforced**, not a convention: a script that attempts a write under it errors. So the + caller has already made a declaration, and the server has already verified half of it. Defaulting those + to cacheable is a much smaller step than defaulting *scripts* to cacheable. + +**A caller can still make a plain `EVAL` cacheable, and that is intended.** `WithDefaultCategory` is a +no-op when a category was already supplied, and `MaskRetryCategory` is in `Message.UserSelectableFlags` - +so passing `CommandRetryReadOnly` with an `EVAL` is honoured. That is not a hole: it is the same single +rule the whole gate rests on, *the declared retry category, whoever declared it*, applied without a +special case for scripts. Someone who declares a writing script read-only has already broken retry +semantics more severely than caching. + +**The risk worth documenting is not non-determinism — it is undeclared key access.** Invalidation tracks +the keys the script declares; a script that reads a key it did not declare in `KEYS[]` is not tracked +against that key, so it goes stale silently and stays stale. Declaring keys properly is already mandatory +in cluster, so the guidance aligns with existing good practice rather than adding a new rule. A +read-only script that also reads `TIME` or `RANDOMKEY` is possible but much rarer, and is the caller's to +notice. + +**Rejected: detecting it by inspecting the script.** A regex - or anything short of a Lua parser - loses +to computed command names, `pcall`, and string concatenation. A detector that is right most of the time is +worse than a clear rule, because people trust it; and the failure it would miss is the silent, durable +kind. + +#### Diagnosability + +The failure this design can still produce is silent and durable: something wrongly cached serves stale data +forever, with no error and no log. So the fill path keeps four counters — `Stored`, `RefusedByFlags`, +`RefusedNoKeys`, `RefusedRaced`. They are incremented only on a miss, which has already paid for a round +trip, so a cache hit costs nothing. "Why is this stale?" and "why is nothing being cached?" should both be +answerable without a debugger. + +**Keyless commands are never cached.** Found by building this: `AllValid` over an empty dependency list is +vacuously `true`, so a keyless entry was valid *for the life of the process* — not even a flush cleared it, +since `OnFlush` stamps key nodes and there were none. Server-assisted invalidation only ever reports keys, +so a command with no keys can never be invalidated by anything. `TIME`, `PING`, `RANDOMKEY`, `INFO` would +all have been permanently stale. This also removes a slice of the non-deterministic problem for free, since +several of those commands are keyless anyway. + +--- + +### 6.11 Request combining: instrument first + +`HybridCache` collapses concurrent misses onto one in-flight operation: the first caller installs a +placeholder carrying a `TaskCompletionSource`, later callers join it, and all complete or fail together. +It is the standard answer to a cache stampede. Whether it earns its complexity *here* is a different +question, because the economics are not the same. + +**A miss costs far less here.** A `HybridCache` miss invokes an arbitrary factory — a database query, an +HTTP call, hundreds of milliseconds. A miss here is a Redis round trip on an already-multiplexed +connection: a thousand concurrent misses become a thousand pipelined commands and a thousand O(1) server +lookups. That is not a stampede in the damaging sense, so the default answer is "not needed". + +**Two cases flip it, and the first is self-inflicted.** Redis requires the client to drop its whole cache +when a connection is lost (§6.6), so every hot key re-fetches simultaneously — precisely when the +connection has just been re-established. And for large values, a thousand concurrent 1MB misses is a +gigabyte of network and a thousand pooled buffers to produce one entry. + +#### Cancellation makes this a now-decision, not a later one + +v4 adds cancellation — `Send`/`SendAsync` take a `CancellationToken` per call — which changes the shape. +(It was originally going to be context state, `WithCancellation(token)`; that is wrong because a token's +lifetime is the *operation's*, not the connection's, and a context is a value built once and reused, so a +stashed one would carry a token belonging to a long-finished request.) +The naive implementation is then *actively wrong*: passing the first caller's token to the shared send +means one caller's cancellation aborts everyone who joined. The shared send must use a **cache-owned** +token, with each waiter observing its own independently. So if cancellation is arriving anyway, this +wants deciding alongside it rather than retrofitted around it. + +**Last-man-standing is not a correctness requirement here**, though — and not because cancellation is +absent, but because **the cache is a stakeholder independent of the callers**. In `HybridCache`, if every +caller cancels, the work is pointless; there is nobody left who wants it. Here, completing the fill +populates a shared cache that later callers will hit, so it has standalone value. Let the fill complete +and commit it, and let each waiter observe its own token. Withdrawing the command when the last waiter +leaves *and* it has not yet been sent is then an optimisation, not a requirement — which removes the part +that is genuinely awkward in `HybridCache`. + +#### A tolerance to state, and a cheap mitigation + +Combining can hand a joiner data **older than an independent read would have given it**. If the leader +sends at T0, a write lands at T0.5, and a joiner arrives at T1, the joiner receives pre-write data for a +request that began strictly *after* the write — where its own request would have seen the write. That is +transient rather than permanent, so it is tolerable, but it should be a stated tolerance rather than an +accident. + +The mitigation is nearly free: **do not join a fill whose generations have already been stamped invalid.** +The leader's dependencies are right there, so a joiner arriving after an invalidation simply sends its +own request. + +#### Constraints for whenever it is built + +- `NoClientCache` callers must never join — they asked not to participate in cache machinery at all. +- The in-flight table is keyed by `(frame, database)`, like table 1. +- The `TaskCompletionSource` is allocated only on a miss, so the zero-allocation hit is unaffected. +- Per-waiter cancellation wants `Task.WaitAsync`, which does not exist on `netstandard2.0`/`net461`; that + needs a linked-TCS polyfill on down-level targets. + +#### The agreed cancellation model + +Settled: **the request completes or fails by itself, including caching; a caller's cancellation applies +only to that caller's await.** No extra token, no waiter tracking, no last-man-standing. `HybridCache` +needs all of that because it fronts arbitrary external systems whose work has no value once nobody is +waiting; here the fill populates a shared cache, so it is worth finishing regardless of who is still +listening. + +### 6.12 Errors are not cached; nulls are + +`TryComplete` refuses a reply whose first **content element** is an error — simple (`-`) or, in RESP3, +bulk (`!`). + +The invariant that makes this cache sound is that a reply is **a function of the keys it depends on**, and +that the server will tell us when those change. An error need not be: it can come from server +configuration, cluster topology, ACLs, memory pressure or a module's own state, none of which key +invalidation covers — so nothing would ever evict it. Caching one turns a **transient failure into a +permanent one**, which is the same class of bug as caching a keyless command (§6.9). + +`-WRONGTYPE` genuinely *is* a function of the key and would be invalidated correctly, but separating those +cases needs per-code knowledge for something that should be rare — and if errors are not rare, caching +them hides the problem instead of solving it. Hence `RefusedError`: a non-trivial count is itself worth +investigating. + +**A null is a value, not a failure**, in all three spellings — `$-1` (RESP2 null bulk), `*-1` (RESP2 null +array) and `_` (RESP3). Redis tracks every key *"mentioned in the context of a read-only command"*, +whether or not it exists, so creating the key invalidates the entry. **Negative caching therefore works, +and works correctly** — which is unusual enough to be worth stating. + +#### Why this cannot be a first-byte test + +The obvious implementation — look at `response[0]` — is **wrong**, and wrong in the way that survives +testing. RESP3 permits attribute metadata (`|`) ahead of a value, and **nothing in the specification +exempts errors or nulls from carrying it**. So `|1\r\n$6\r\nttl-ms\r\n:1000\r\n-ERR …` begins with +`|`, passes a leading-byte check, and gets cached as though it were data — a permanently cached error, +which is exactly the failure §6.12 exists to prevent. + +No server is known to emit attributes today. That is precisely why it would go unnoticed: the bug is +latent until a server, a proxy, or a future protocol revision starts using a feature the protocol already +allows. + +The fix is not to parse every reply, though. **Attributes are the only construct that can precede a +value**, so if the first byte is not `|` then it *is* the first content element's prefix, and the cheap +test is **exact, not approximate**: + +```csharp +return (RespPrefix)response[0] switch +{ + RespPrefix.Attribute => IsCacheableBehindAttributes(response), // NoInlining + RespPrefix.SimpleError or RespPrefix.BulkError => false, + _ => true, +}; +``` + +Protocol parsing is reserved for the branch that needs it, which — no server emitting attributes today — +is in practice never taken. The slow path is `[MethodImpl(NoInlining)]` for the same reason the +`MessageWriter` fallbacks are: `RespReader` is a sizeable `ref struct`, and constructing one in a cold +branch changes codegen for the whole method. + +On that path, `RespReader.TryMoveNext(checkError: false)` skips attributes and lands on the first content +element, and `RespReader.IsError` classifies it. `checkError: false` matters — the default overload +*throws* on an error, which is the very thing being detected. A reply with no content element at all +(metadata only, or empty) is refused too: unclassifiable fails closed. + +Both branches are pinned: the tests fail against a first-byte-only implementation, and they fail again if +the attribute path stops classifying. + +#### Decision: measure first + +Not built. `RespClientCache.RedundantFills` counts fills that completed only to find the same request +already cached — two or more callers missing on the same request concurrently, which is exactly what +combining would have collapsed. It costs one increment on an already-cold path and needs no in-flight +table, so it does not presuppose the design it is evaluating. Expect near zero for ordinary traffic and a +spike after a flush. + +### 6.10 Decision log + +What was chosen, what was rejected, and why. Several of these were reversed during implementation; the +reversals are the useful part. + +| Decision | Rejected alternative | Why | +| --- | --- | --- | +| Reference counting (`RefCountedBuffer`) | Neuterable `Dispose` + `TransferOwnership`, as §6.4 originally sketched | Transfer makes every holder reason about whether ownership moved, and the answer is only known after dispatch. A count gives one rule: whoever retains, releases. | +| `AsLookupKey()` borrows for the probe | Always `Detach()` | `Detach` allocates a lease — **48 bytes, measured** — and on a cache *hit* the caller never wanted the buffer. The split also makes storing a borrowed key unexpressible, since a borrowed key cannot be retained. | +| Global monotonic generation tickets | Per-key counters | A per-key counter restarting at zero collides with a ticket an entry recorded before invalidation, so the entry validates against a key that *did* change. | +| Two independent tables | `key → set of entries` cross-index | The set must be maintained on every insert and eviction, an N-key entry lives in N sets, and a hot key's set can be a large fraction of the cache — so invalidation is O(entries), not O(1). | +| Entries hold the key's `Node` directly | Re-look-up table 2 per hit | Validation becomes a dereference and a compare, with no hashing on the hot path. Cost: a node leaving table 2 must be stamped invalid *first*, or entries pointing at it never learn. | +| Executor owns the send; cache is a participant | `cache.GetOrExecute(...)` | A cache that calls the executor must sit above dispatch and know how to send; and a *command* has no business knowing how to send itself. Splitting yields `IRespExecutor` + `IRespHandler`, which are `Message` + `ResultProcessor`. | +| One `Send` with an optional cache | Two overloads | The cached path *is* the uncached path plus a probe and a commit, so an uncacheable request falls through to the same tail instead of duplicating it. | +| `RespRequest` / `RespPayload` on both sides | `ReadOnlySpan` in, `byte[]` out | A span cannot cross an `await` **or be parked in a backlog for a resend** — so it rules out async *and* retries even synchronously. `byte[]` allocates per call. | +| `SendAsync` is not an `async` method | Plain `async` | `async` forbids `ref` parameters, and the frame must be consumed by reference. Keeping the probe synchronous also makes a cache hit complete with **no state machine and no `Task`**. | +| `TryComplete` takes the payload | `TryComplete` takes the bytes | The reply is already in a pooled reference-counted buffer; copying it to cache it is waste. | +| Caching is **opt-out** (`NoClientCache`) | Opt-in | Opt-in means touching every `IDatabase` method, and one omission makes the feature silently do nothing. The population that argued for opt-in turned out to be nearly empty — see below. | +| A separate flag bit | A new rung on the retry ladder | `WithCategory` says the caller's category wins, so opting out of caching would *replace* the retry category and change reconnect behaviour. A rung above `ReadOnly` also reads as more severe, so `<= ReadOnly` retry policies would stop retrying it. `CommandServerSpecific` sits outside the ladder for exactly this reason. | +| Non-determinism lives in command metadata | A `CommandFlags` bit | `SRANDMEMBER`/`SCAN`/`HRANDFIELD` are compile-time properties of our own enum; pushing them onto every call site is burden without benefit. | +| Keyless requests are never cached | Cache them | Invalidation only ever reports **keys**, so a keyless entry is vacuously valid for the life of the process — not even a flush clears it. Found by building it, not by reasoning. | +| Handler maintains **both** key-mark forms | Re-derive arg indices on promotion | §5.2 assumed there were no spare bits — true of the *frame*, false of the writer, which is a stack `ref struct` with no size pressure. | +| >62 arguments reports "cannot report keys" | Report the first 62 | A partial list is worse than none: a caller tracking keys for invalidation would believe it complete and cache something it can never invalidate. | +| Fast byte test, `RespReader` only behind an attribute | Parse every reply | Attributes are the only thing that can precede a value, so a non-`\|` first byte *is* the content prefix - the cheap test is exact, and parsing is reserved for a branch that is in practice never taken (§6.12). | +| Classify the reply with `RespReader` | Test `response[0]` | RESP3 attributes may precede any value, and nothing exempts errors from carrying them - so a first-byte test caches an error hidden behind metadata. Latent today because no server emits attributes, which is what makes it dangerous (§6.12). | +| `EVAL_RO`/`EVALSHA_RO` cacheable by default | Exclude all scripts; or detect by inspecting the script | `EVAL`/`EVALSHA` are excluded *by default*, so this applies only where the caller chose the `_RO` variant *and the server enforces it*. An explicit `CommandRetryReadOnly` on a plain `EVAL` is honoured, deliberately - one rule, no script special case. Inspecting the script loses to computed command names and `pcall` (§6.9). | +| Exclusions live in command metadata, beside the retry category | A `CommandFlags` bit | `CommandFlags.Category.cs` already classifies per enum value; cacheability is the same kind of fact about the same enum, and a flag would burden call sites with something we know (§6.9). | +| Errors never cached; nulls always | Cache errors too, or treat null as a miss | A cached reply must be a function of the tracked keys; an error need not be, so nothing would evict it and a transient failure becomes permanent. A null *is* a function of the key, and Redis tracks keys that do not exist, so negative caching is correct (§6.12). | +| Cancellation applies only to the caller's await | HybridCache's extra token + waiter tracking | The fill populates a shared cache, so it has value once nobody is waiting - unlike an arbitrary external system, where it does not (§6.11). | +| Request combining deferred, with a counter | Build it now | A miss is a round trip on a multiplexed connection, not an arbitrary factory call, so the stampede economics differ by orders of magnitude. `RedundantFills` measures whether it is real without presupposing the design (§6.11). | +| >62 arguments declines to cache | "Treat every argument as a key" | Over-invalidation is safe by protocol, but this registers *value* bytes as tracked keys, polluting the key table and inviting spurious invalidation from unrelated keys that happen to match a value. Safe, and invisibly degrading. | + +**One reversal worth recording explicitly.** The case for opt-in rested on "an external command that is +read-only, keyed, and untracked" — with `FT.SEARCH` as the example. That was wrong: `FT.*` takes an *index +name*, not a keyspace key, so it is keyless and already refused. The keyed module commands (`JSON.GET`, +`TS.RANGE`, `BF.EXISTS`) operate on real keys the server does track, and the Redis docs exclude the +probabilistic and time-series families on **efficiency** grounds — *"designed to be updated frequently, +which means caching has little or no benefit"* — which is exactly what an opt-out is for. With the +counter-example gone, the argument went with it. + +**A race that is not a defect.** Validation is not atomic across an entry's keys: validate A, an +invalidation for A lands, validate B, serve. The read could have completed a microsecond earlier and been +equally correct, so either outcome is a legitimate observation. It is bounded to reads that overlap the +invalidation, and everything after it is correct. Recorded as a deliberate tolerance rather than something +to fix. + +### 6.13 `CLIENT TRACKING`: the mode, decided + +Everything below was checked against a live Redis 8.9.241 rather than inferred; the error text is quoted +from the server. + +**RESP3 only; no `REDIRECT`.** Not merely because two connections are more work — the redirected model has +a race the single-connection model cannot have. The invalidation can arrive *before* the reply it +invalidates, so a client caches a value it has already been told to drop, and the documented workaround is +to write a placeholder entry before sending and refuse the fill if it disappears. We happen to implement +exactly that already (`TryBeginFill` + `Dependency.AllValid` + stamp-before-drop, §6.6), so we would +survive it — but there is no reason to pay for a protocol that requires it. When RESP3 is unavailable, +client-side caching must **refuse loudly**, not silently degrade into a cache nothing invalidates. + +**`BCAST`, with the empty prefix by default.** + +``` +CLIENT TRACKING on PREFIX foo → ERR PREFIX option requires BCAST mode to be enabled +CLIENT TRACKING on BCAST PREFIX foo PREFIX foob → ERR Prefix 'foo' overlaps with another provided + prefix 'foob'. Prefixes for a single client must + not overlap. +``` + +Multiple prefixes are an OR; no prefix under `BCAST` means the empty prefix, i.e. every key. + +**`PREFIX` does not map onto `AppendKeyPrefix`, and this is the trap worth recording.** Prefixes are +connection-global, must not overlap, and cannot be removed individually ("to remove all prefixes, disable +and re-enable tracking"). Context key-prefixes routinely *nest* — `app:` and `app:users:` — which is +precisely the rejected case, and a context going out of scope has no way to deregister. So the prefix set +is an explicit connection-level tuning knob, never derived per-context. Registration is O(N²) and server +CPU scales with prefix count. + +The honest cost of `BCAST` with the empty prefix is a push for every key modified by anyone. Client-side +that is cheap — `OnInvalidate` is ~5-6ns and allocation-free, which is exactly why it was measured that way +(§6.6) — but the network cost is real, and is the reason `PREFIX` exists at all. + +**`OPTIN`/`OPTOUT` are therefore off the table.** + +``` +CLIENT TRACKING on BCAST OPTIN → ERR OPTIN and OPTOUT are not compatible with BCAST +CLIENT TRACKING on; CLIENT CACHING yes → ERR CLIENT CACHING YES is only valid when tracking is enabled + in OPTIN mode. +CLIENT TRACKING on; CLIENT CACHING no → ERR CLIENT CACHING NO is only valid when tracking is enabled + in OPTOUT mode. +``` + +Note the default mode is *not* `OPTOUT`: both track everything, but only `OPTOUT` unlocks per-command +exclusion, and the default mode has no escape hatch at all. Choosing `BCAST` removes the question. We lose +little: `CommandFlags.NoClientCache` already opts out client-side at zero protocol cost, and the only thing +a server-side opt-out buys is invalidation-table memory — which under `BCAST` is zero. + +*If we ever went default-mode:* `OPTIN` needs a positive flag (`CommandFlags.ClientCache`) and a +`CLIENT CACHING yes` pipelined immediately ahead of each command, with two traps — it applies to **all** +commands in a following `MULTI`, and to **all** commands executed by a following Lua script. + +**Invalidate locally on every write. Do not enable `NOLOOP`.** Two separate decisions that look like one. + +Local invalidation of the keys a write touches is a strict improvement, independent of `NOLOOP`: +over-invalidating is always safe (§6.6 accepts false invalidations by design), the frame already carries +key marks so it costs almost nothing, and it closes the window between our write landing and the push +coming back. Keyless flushes map to `OnFlush()`. + +`NOLOOP` is a different matter, and the server documentation is unusually blunt about why: + +> "With tracking in the default mode, the server removes the key from the invalidation table when the key +> is modified. If the connection that modified the key is using `NOLOOP`, Redis suppresses the invalidation +> message to that connection, **but the key is still no longer tracked for that connection after the +> write.**" + +So in default mode, `NOLOOP` without exact local invalidation is not "briefly stale" — it is +**permanently** stale: we keep the entry, the server has stopped tracking it, and a *third party's* later +write produces no message for us either. And "exact" is the problem: any command whose key set we +under-declare — `EVAL`/`EVALSHA` with computed keys, anything whose key spec we do not model — lands in +that case. Under `BCAST` there is no invalidation table and the hazard does not arise, which is another +point in `BCAST`'s favour, but it stays a later optimisation rather than part of the first cut. + +#### The invalidation push, on the wire + +Captured from a live server rather than read off a page, because the exact shape is what the integration +turns on: + +``` +third-party SET >2\r\n$10\r\ninvalidate\r\n*1\r\n$9\r\nprobe:key\r\n +MSET m:1 m:2 m:3 >2\r\n$10\r\ninvalidate\r\n*3\r\n$3\r\nm:1\r\n$3\r\nm:2\r\n$3\r\nm:3\r\n +FLUSHDB >2\r\n$10\r\ninvalidate\r\n_\r\n +our own SET +OK\r\n followed by the same push (NOLOOP off; reply first, then push) +``` + +- The second element is **an array of keys, or a RESP3 null** — *never* a string. That is exactly why the + existing pipeline drops these: its push handling expects pub/sub shape (`message` / channel / payload, + all strings), so an invalidation fails the "second element is a string" test and is discarded. +- **One push can name several keys.** A handler that reads only the first leaves entries live. +- **Key expiry invalidates too**, not only explicit writes — confirmed by watching a `PX 150` key. +- `BCAST` really does report keys we never read, and `PREFIX` filters exactly as documented. +- **A flush cannot be scoped away.** `PREFIX want:` correctly ignores a write to `other:key`, but a + `FLUSHDB` on an entirely different database still arrives as `>2 invalidate _`. A flush names no keys, so + there is nothing for a prefix to filter and nothing to attribute to a database. One `FLUSHDB` by anyone, + anywhere, empties every tracking client's cache. That is correct, and worth knowing before someone + reasons that a narrow prefix bounds their exposure - it bounds key traffic, not flushes. + +**Telling an invalidation from a pub/sub delivery is the discrimination the real pipeline needs to add**, +and getting it backwards is not a no-op in either direction: a channel name read as a key list, or a +delivery handed back as somebody's reply. + +And "push means out-of-band" is too simple, in a way that is genuinely unpleasant. +`subscribe`/`unsubscribe` confirmations are typed as pushes but **can be** the reply to a command — *can +be*, not *are*. The same shape arrives unsolicited (`RESET`, server-side teardown, shard migration), and +`UNSUBSCRIBE` with no arguments answers **one** command with **N** pushes, or none at all if there were no +subscriptions. So the correlation is not one-to-one, and **cannot be decided by inspecting the frame** — it +needs subscription state. + +The library already has this: `PhysicalConnection.OutOfBandResult` has a third value, `MatchToCommand`, +for exactly that case, and its policy for anything unknown is worth copying verbatim: + +> *"a RESP3 push frame is out-of-band by definition; if we don't recognize it (newer server, or a feature +> we don't implement) we drop it - matching it to a pending command would desynchronize the entire response +> stream"* + +Which is also precisely why invalidations are dropped today: they are simply not recognised. + +#### What the production integration actually needs — done + +Two changes, both small, and both as predicted: + +1. **`PushKind` gains `[AsciiHash("invalidate")] Invalidate`** (`PhysicalConnection.Read.cs`). One line; the + generator does the parsing. +2. **It must be handled *before* the channel gate.** The existing flow does: + + ```csharp + if (kind is PushKind.None || !TryMoveNextString(ref reader)) return OutOfBandResult.NotRecognized; + ``` + + `TryMoveNextString` requires the second element to be an inline `BulkString`/`SimpleString`, because for + pub/sub the second element is always the channel. An invalidation's second element is an **array** or a + **null**, so adding the enum member alone changes nothing — it would still fall out here as + `NotRecognized` and be dropped. Invalidation has to branch off ahead of that gate and return `Handled`. + +Proven end to end by `RespTrackingTests` against a real server, through a dedicated RESP3 `BCAST` +connection (`TrackingExecutor`): a third party's write evicts what we cached, non-ASCII key bytes match, +one push clears several entries, a flush clears everything, and a pub/sub delivery on the same connection +disturbs nothing. All four parsing steps are mutation-tested — the discriminator check initially survived +its mutant, which is what prompted the pub/sub test. + +Both landed as written. `OnInvalidate` sits immediately after the `kind` is decoded and before +`TryMoveNextString`, and always returns `Handled` — including when there is no cache at all, because an +invalidation is never the reply to anything we sent, so letting it fall through to command matching would +hand it to whoever happened to be at the front of the queue. Its three unreadable cases (a payload that is +not an aggregate, a streaming aggregate, a key whose bytes we cannot see contiguously) all **over-flush** +rather than guess: we already know something changed, and the same judgement is made on disconnect. + +#### When invalidations actually arrive, measured + +Against Redis 8.9.241, RESP3, `BCAST PREFIX`, reading raw bytes off one socket so the order on the wire +*is* the answer: + +| What was sent | What came back, in order | +| --- | --- | +| `SET k v` (self-tracked) | `+OK`, **then** `>2 invalidate [k]` | +| `MSET a b c` | `+OK`, then **one** push: `>2 invalidate [a, b, c]` | +| `SET k1` + `SET k2`, pipelined | `+OK`, `+OK`, then **one** push: `>2 invalidate [k1, k2]` | +| `SET k v` then `GET k`, pipelined | `+OK`, `$2 v`, **then** the invalidation | +| `DEL k` | `:1`, then `>2 invalidate [k]` | +| `FLUSHDB` | `>2 invalidate _` **then** `+OK` | +| `SET k v PX 100`, then wait, then read it | **nothing** - no push at all, before or after the expiry | + +Four things follow, and none of them were obvious. + +**Invalidations trail their replies, and are accumulated across the write cycle rather than emitted per +command.** Two pipelined `SET`s produced a single two-key push after *both* `+OK`s - so the batching unit +is not the command, it is whatever the server flushes in one go. + +**Which means a self-invalidation can never protect read-your-own-writes.** The `SET`/`GET` row is the +proof: the reply to a read issued *after* the write still precedes the notification about it. A cache +holding a stale entry for `k` would answer that `GET` from cache, and the correction arrives afterwards. +`RespClientCache.OnLocalWrite` is therefore not an optimisation or a latency shortcut - it is the *only* +mechanism that can close that window, because the server's own message is late by construction. It +currently has no caller in `src`, which makes wiring it a correctness item rather than a nicety. + +**`FLUSHDB` is the exception that a fake will get wrong.** Its `invalidate null` is emitted *before* its +own `+OK`, where every key invalidation comes after. So "accumulate and fan out after the reply" is right +for writes and wrong for flushes. + +**A write whose key marks overflow stamps its arguments, not the whole cache.** A frame can only mark keys +up to argument 62, so a large `MSET` or `DEL` reports "I have keys but cannot tell you which" - and the +first version of the local-write hook answered that with `OnFlush`. That is correct and far too blunt: a +bulk write would destroy an unrelated hot cache every time. Reporting a *subset* is forbidden - it is why +the frame refuses to report one at all - but the arguments are a **superset** of the keys, and stamping a +superset is sound. The cost is one needless miss for any value that happens to equal a cached key, on a +command that already named enough keys to overflow the bitmap. + +Note this only ever applies to **writes**. A 100-key `MGET` is a read, so it never reaches this path; it is +simply not cached, by the same keyless rule, because a frame that cannot name its keys cannot be +invalidated either. + +**Expiry was not announced at all** - not passively after the TTL passed, and not even when a subsequent +read forced the deletion. Whatever the documented behaviour, an entry whose only end is expiry may get no +notification, which is the case `CachePolicy.TimeToLive` exists to bound. This is the evidence for that +argument, which until now rested on reasoning. + +#### Where the cache lives + +A cache needs an owner before a push has anywhere to go, and until now there wasn't one: the cache was a +free-standing object that tests built and attached per context with `WithCache`, which is exactly what +blocked this. It now hangs off the **multiplexer** — created in the constructor when +`ConfigurationOptions.ClientCache` names a policy, never replaced (so a reader can take it without a lock), +and handed to each `RedisDatabase`'s context as it is built. + +Per-multiplexer and not per-database is forced, not chosen (§6.14): tracking is per *connection*, and a +connection belongs to the multiplexer. A cache per database would have to be found from here anyway when a +push landed, and a cache per context would be handed the invalidations of a connection it does not own. + +`OnConnectionFailed` flushes it as its very first act — before the disposed check and before the handler +dispatch, and **synchronously** rather than via `CompleteAsWorker`, because queueing it leaves a window in +which we would answer from a cache we already know is suspect. + +The policy is deliberately **not** part of the connection string. A policy is a set of durations and +correctness choices rather than a name, and round-tripping it through text invites it to be configured by +someone who has not read what `InvalidationGracePeriod` actually permits. `null` — no cache — is the only +safe default: a cache changes what a read can return, and nobody should acquire that by upgrading. + +`RespCacheInvalidationTests` proves the whole path through the real client, with `CLIENT TRACKING` still +issued by hand: a write from a second connection evicts what the multiplexer cached, while a key outside +the `PREFIX` filter keeps serving the old value — which is the cache proving it was in the path at all, +since without it that read would have gone to the server and come back changed. A `FLUSHDB` on a dedicated +database empties everything, including that unfiltered key. Three mutations are caught: never matching +`PushKind.Invalidate`, ignoring the null payload, and detaching the cache from the context. + +One thing the test had to learn: under `BCAST` the server announces a matching key to every tracking client +the moment *anyone* writes it, including the test's own setup write on the other connection. If that push +overtakes the reply being filled from, the fill is refused as raced — correctly, since storing it would +cache a value the server has already said is wrong. A test that primes an entry immediately after writing +it therefore has to be prepared to ask twice. + +What is still missing is the negotiation: nothing yet sends `CLIENT TRACKING` on our behalf, so a caller who +sets `ClientCache` and stops there gets a cache that fills, expires on TTL, and is never invalidated. That +is the next item, and it must refuse loudly without RESP3 rather than quietly behave this way. + +#### A prefix list is also a statement about what may be cached + +The test above originally had a second key, outside the `PREFIX`, and asserted that it **kept serving the +old value** — offered as proof that the cache was in the path at all, since without a cache that read would +have gone to the server and come back changed. It did prove that. It also enshrined a bug. + +Under `BCAST` the server announces only keys matching a prefix. An entry whose key matches none of them has +nothing that will ever say it is wrong: it is served until `TimeToLive` alone retires it. That is the +`RefusedNoKeys` argument reached from the other side — there the request declared nothing to depend on, +here it declared something the server was never asked to watch — and it deserves the same answer. + +So `CachePolicy.Prefixes` now carries the list, and `TryBeginFill` refuses anything outside it, counted as +`RefusedNotTracked`. Four decisions inside that: + +- **Every key, not any.** The entry depends on all of its keys, so one untracked key makes the whole reply + uninvalidatable. `MGET tracked untracked` is not "mostly fine". +- **Bytes, not characters**, compared against the key as written to the wire. That is the only comparison + that means anything: the server matches the bytes it received and names those bytes back, so a context + key-prefix or keyspace isolation is already baked in by the time the cache sees it. +- **Empty list means everything**; an empty *string* is rejected. `""` matches every key, so accepting one + would silently turn a deliberately narrow list into a total one — a cache that looks scoped and is not. +- **Overlap is rejected at construction**, because `CLIENT TRACKING` rejects it at the handshake. Better + the failure lands where the mistake was made. + +The list lives on the policy rather than being derived from anything, for the reason already recorded in +this section: prefixes are connection-global, must not overlap, and cannot be removed individually, whereas +context key-prefixes routinely nest. Declaring it once means the set the cache will admit and the set the +server agreed to announce are **one set** — and when negotiation lands, the `PREFIX` arguments come from +here rather than from a second list that could drift. + +The honest cost: narrowing the prefix list narrows the cache. That is the trade — broadcasting everything +means being told about every key every client touches, and scoping it down buys quiet by only caching +what is in scope. Which is the right way round: the alternative was caching things nobody would ever +correct. + +One thing this does **not** change: a flush is still unfilterable, so `invalidate null` must still be +honoured. It is simply no longer observable through an out-of-prefix entry, because there are none. + +A note on the verification: the empty-prefix rule initially survived its mutant. The test spelled the case +as `["app:", ""]`, which the *overlap* rule catches first — every string starts with `""` — so deleting the +empty check changed nothing. A lone `[""]` is the case that matters, and it now asserts on the message. + +### 6.14 Global cache, contextual TTL + +**The cache is global.** Two facts force it. Tracking is per-*connection* (by client id), and the server +keeps *"a single keys namespace, not divided by database numbers"* — a change to `foo` in db 3 invalidates +`foo` cached from db 2, which §6.6 already handles (`InvalidationCrossesDatabases`). + +- In **default mode**, only the connection that *read* a key is told about it. So every connection serving + cacheable reads needs tracking on, and a push arriving on one connection must evict entries populated via + another — entries are keyed by (frame, database), not by connection. +- In **`BCAST` mode**, invalidations reach every client subscribed to the prefix regardless of who read, so + **one** tracking connection serves the whole multiplexer. + +Either way the cache is global; `BCAST` merely makes it clean — one subscription, one push stream, no +per-connection bookkeeping, no duplicate invalidations. So `WithCache` means "participate, or not" (plus +substitution in tests), **not** "bring your own": two different caches over one multiplexer is not +supportable, because the invalidation stream has exactly one destination. + +**The TTL is not global.** How stale a caller will tolerate is a per-caller policy, not a property of the +connection — and it is the one piece of cache configuration that *cannot* be added to the existing +surface, because `IDatabase.StringGet` cannot grow a parameter without a binary break (AGENTS.md). On the +context it is free and reaches every command without touching a signature, which is §9.4 paying off again. + +**It must be applied on read, not stamped on store.** The entry is shared, so the fill timestamp goes with +the entry and `TryGet` takes a maximum age from the *reading* context. One entry serves any number of +contexts with different tolerances; stamping at store time would force identical replies to be cached once +per distinct TTL. + +Two things still to settle: + +- **Where it sits on the context.** A `TimeSpan` field pushes `RespContext` past its 48 bytes; the service + slot keeps it there at the cost of a chain walk per cache read. That is the same trade `ChannelPrefix` + was measured for (§3.3), so measure rather than guess — noting this one is on the *hit* path, where the + alternative is a network round trip. +- **There must be a default, and it must be finite.** There is no such thing as "no TTL policy" - the + absence of a TTL is a policy, and it is `TTL = infinity`. An entry that is never invalidated and never + expires is **permanently** stale, which is strictly worse than being briefly over-stale. The server + documentation says so directly: *"Putting a max TTL on every key is a good idea, even if it has no TTL. + This protects against bugs or connection issues that would make the client have old data in the local + copy."* + + Note which way the argument runs. "Invalidation delivery is not wired yet, so a number would be + arbitrary" is exactly backwards: while delivery is unwired, the TTL is the **only** freshness mechanism + there is, so the backstop matters more, not less. + + **Proposed default: 60 seconds**, and it is a safety bound rather than a tuning knob. What it protects + against is a missed invalidation - a connection blip we did not notice, or a bug. Flushing on disconnect + (still unwired) plus keepalive detects a real disconnect within seconds to tens of seconds, so a minute + is comfortably longer than detection while still bounding the damage when detection itself fails. + + **A second tier is worth considering**: the right default depends on whether invalidation is actually + live. With `CLIENT TRACKING` confirmed, the TTL is a backstop against rare losses and can be generous. + Without it, the cache is a stale-data generator with a timer, and the honest options are a much shorter + bound or refusing to cache at all. Either way the client should be loud about which regime it is in, + rather than the difference being invisible. + +**Still unwired, and both come straight from the same documentation:** losing the connection must flush the +cache (`OnFlush()` exists; nothing calls it on disconnect), and there is no TTL of any kind today. + + +### 6.15 Stampedes: single-flight, and stale-while-revalidate + +Reported from the field (Microsoft, on HybridCache): expiry and invalidation both produce **stampedes** — +the moment an entry goes, every concurrent reader of a hot key misses at once and they all hit the server +together. + +**We already measure this and do nothing about it.** `RedundantFills` counts exactly these collisions, and +its test says so in as many words: *"two callers miss on the same request and both go to the server - +exactly what request combining would have collapsed into one round trip"*. So the counter is a meter, not a +mitigation. + +There are **two** mechanisms, and they are usually conflated: + +- **Single-flight** — N concurrent misses on the same request become one round trip with N waiters. The + direct fix. +- **Stale-while-revalidate (SWR)** — serve the old value while a refresh runs, so the window in which a + stampede is even possible mostly stops existing. + +They are not independent: if N readers cross the refresh threshold together, the *background refresh* is +itself a stampede. So SWR's "refresh once" is single-flight wearing a different hat, and single-flight is +the thing to build first — it stands alone, and everything else needs it. + +#### Single-flight, and why sharing a reply is sound + +A waiter attaching to an in-flight request gets that request's reply. The justification is an ordering one: +the leader sent at T0, the waiter attached at T > T0, the reply lands at T1 > T. Had the waiter sent its +own request at T, it would have been answered at about T1 as well. **No linearisation the waiter can +observe distinguishes the two**, so the shared reply is a legitimate answer to its read. + +That argument has exactly one hole, and it is the same hole as everywhere else in this design: +**read-your-own-writes**. If the waiter (or anything else in this process) wrote the key after T0, the +leader's in-flight reply predates the write, and returning it is observably wrong — that gets reported as +corruption, not as staleness. + +The fix reuses machinery that already exists. Writes invalidate locally (§6.13), which bumps the key's +generation; the leader records the generation it sent at, and a waiter may attach **only if the dependency +generations still match**. That is `Dependency.AllValid` (§6.6) — the same invariant that decides whether a +fill may be *stored* decides whether a waiter may *attach*. If it fails, the waiter simply goes its own +way. + +Sharing the reply is also already safe for lifetime: entries hold a refcounted blob lease, so each waiter +takes its own reference and the reply outlives the leader. + +#### Stale-while-revalidate on expiry + +Falls straight out of the TTL work in §6.14 — two thresholds instead of one, both contextual and applied on +read: + +| age | behaviour | +|---|---| +| `< soft` | fresh hit | +| `soft ≤ age < hard` | **serve stale**, and trigger a refresh, once | +| `≥ hard` | miss | + +The once-only flag lives on the *shared entry* while the thresholds are *per-context*. That is right rather +than a compromise: whoever crosses their own soft bar first triggers a refresh everyone benefits from. The +flag must clear on failure as well as success, with backoff — otherwise one failing server leaves the entry +pinned stale until hard expiry. + +#### Built: expiry-based SWR + +`CachePolicy.RefreshAfter` is the soft threshold; `TimeToLive` remains the hard one. A read between them +is **served** and **claims a refresh**, which runs on the thread pool while the caller already has an +answer. + +**The refresh needs nothing configured.** The cache key *is* the request, so refreshing means re-sending +it — no factory, no captured state, nothing of the caller's retained, and handler-agnostic because the +cache stores raw bytes. That is the thing `HybridCache`'s `(TState, Func)` shape exists to +work around, and it falls out of §6 rather than being designed. + +**Off by default** (`RefreshAfter = Zero`). Serving a value already known to be old is a choice about +correctness, not a tuning knob, so it is made rather than inherited. A threshold beyond the lifetime means +the same thing as off — it could never be crossed. + +Three things this needed that were not obvious until it ran: + +- **The claim must be atomic with the lookup.** `TryGet` decides "this is ageing" *and* "you are the one + who will fix it" in one step, and tells exactly one caller. Deciding those separately lets every reader + past the threshold decide both, which is the stampede again in different clothing. The flag lives on the + shared entry while the thresholds are per-context, so whoever crosses their own bar first starts a + refresh everyone benefits from. +- **A refresh must REPLACE, not add.** `TryComplete` used `TryAdd`, which is right for a first fill — + losing that race means somebody answered the same question first and their answer is as good — but makes + every refresh a silent no-op that still counts as a redundant fill. It now swaps in place when the fill + says it replaces. +- **Swap in place rather than remove-then-add.** `TryRemove` hands back the *value* but not the stored + *key*, and the key holds a retained request of its own; removing would strand that reference, and + disposing our own copy instead would release the wrong one. `TryUpdate` leaves the dictionary's key + untouched, so only the superseded reply needs releasing — which it does, and there is a test watching the + reference count to prove it. + +A refresh takes **no in-flight registration**: it is not something anybody should wait for. The entry is +still being served, so a concurrent miss for the same request is asking a different question — it has +nothing, and should fetch rather than queue behind a nicety. + +`Refreshes` counts them. Read it against `Expired`: refreshes rising while expiries stay near zero is the +shape you want — entries renewed before anyone had to wait. Expiries rising alongside means the window is +too narrow to cover the fetch. + +#### Stale-while-revalidate on invalidation + +Also possible, under "you cannot prove the order, so any order is valid" — but **only for third-party +writes**. An invalidation we caused ourselves is not a race, it is a fact, and serving through it breaks +read-your-own-writes. + +The carve-out needs no new bookkeeping: + +- **local invalidation** (we wrote it, on any connection — the cache is global, §6.14) → **hard drop** +- **server push** → eligible for the soft window + +and because the local invalidation happens *before* the echoed push arrives, our own write's entry is +already gone when that push lands. Ordering does the work. + +**Measure the window from first notice, not from the invalidation.** Measuring from the invalidation means +a timestamp on the key node in table 2 — eight more bytes per node, on the `OnInvalidate` path that is +currently ~5-6ns and allocation-free, which is not a path to disturb for this. First-notice needs one field +on the entry, and is the better semantic anyway: the window is "how long we serve stale while a refresh is +in flight", which is a fact about refresh latency, not about when somebody else wrote. + +#### The refresh needs no factory, because we already hold the request + +Most caches cannot refresh themselves. The key is an opaque string, so the cache must be *handed* a way to +recompute the value — and `HybridCache` pays a real price for that: its `(TState, Func)` +shape exists specifically to avoid a lambda allocation per call, which is awkward to use and keeps +arbitrary caller objects alive for as long as the operation is pending. For a *background* refresh it is +worse again, because the state and the delegate would have to be retained on the entry, pinning user +objects inside the cache for as long as the entry lives. + +None of that applies here, and it falls straight out of §6: **the cache key IS the rendered request**. To +refresh an entry we re-send its own key. No factory, no captured state, no delegate, nothing of the +caller's retained — the only thing held is the pooled buffer the cache already owns, and `Detach` preserves +the original `CommandFlags` so the refresh goes out exactly as the original did. + +It is also **handler-agnostic**: the cache stores the raw reply and parsing happens per-caller, so a +refresh does not need to know what anybody intended to turn the bytes into. That is what makes a background +refresh a few lines rather than a design. + +#### Built: flushing on disconnect + +`cache.FlushOnDisconnect(multiplexer)` subscribes to `ConnectionFailed` and empties the cache; the returned +`IDisposable` unsubscribes. + +**Not a tidy-up.** Server-assisted invalidation only works while somebody is listening. Anything that +changes during a disconnect is never announced - the server forgets a client it has lost - so an entry that +survives the gap is wrong, with nothing left in the system that will ever say so. There is a test asserting +exactly that failure without the hook, because "it would be stale" is much less convincing than watching it +happen. + +It flushes on **any** connection failure rather than reasoning about whether that particular connection was +carrying invalidations. Over-flushing costs a round trip per key; under-flushing serves wrong data with no +bound on how long for, and that is the direction this design errs in everywhere else. + +It does **not** cover a connection that has failed and nobody has noticed - no event is raised for a socket +that is quietly dead. That is what `CachePolicy.TimeToLive` is for, and is the concrete reason it must never +be infinite (§6.14). The two are a pair: the hook handles detected failure, the lifetime bounds undetected +failure. + +Explicit for now because the cache hangs off a context rather than being owned by the multiplexer. When +`GetDatabase()` returns a cache-aware database this becomes part of constructing one - which is the right +end state, because a cache nobody remembered to wire up is a cache that goes quietly wrong. + +#### Built: invalidation as a grace period + +`CachePolicy.InvalidationGracePeriod` is the other half, and the more valuable one: age is staggered across +entries, but an **invalidation lands for every reader of a key at the same instant**. That is the thundering +herd exactly, and time-based smoothing cannot touch it, because the trigger was not time. + +**It is a grace period, not a licence.** It moves the entry's hard expiry to "now plus this", measured from +the **invalidation**. The case worth protecting is a key under constant access, where the herd forms +immediately; a key nobody is reading should just expire, and does - nobody arrives inside the window, so +nothing is served and it goes on the next sweep. + +**The clock starts at the invalidation, not at first notice** - and the first version had this wrong. First +notice is cheaper (it needs no timestamp on the invalidation path) but it means a key invalidated an hour +ago is served stale to whoever reads it next, which is the opposite of the intent: the point is to bridge a +burst, not to resurrect something nobody wanted. There is a test for a key nobody reads, and the mutant that +restores the first-notice version fails it. + +The cost of getting it right is a `Stopwatch.GetTimestamp()` in `OnInvalidate`, which is otherwise a few +nanoseconds wide and sees **every** key the server mentions under `BCAST`. So it is **conditional**: the +stamp is taken only when the policy has asked for a grace period, and the default path is unchanged. + +**Read-your-own-writes is enforced structurally**, not by convention. `OnLocalWrite` marks the key node with +a monotonic ticket, and an entry is refused the grace if any key it depends on carries a local-write ticket +newer than the generation it recorded. "No observer can prove the order" excuses serving through *somebody +else's* write; it says nothing about ours, and handing a caller back the value they just replaced is +reported as corruption, not staleness. Monotonic because our own write echoes back from the server as an +ordinary invalidation - the fact that we wrote it has to survive that, and there is a test for the two +arriving in that order. + +The window is also the **cap**: on a hot-written key every refresh is invalidated before it can be stored, +so an unbounded one would serve stale for ever. The test for this has to make the refresh *fail*, otherwise +a successful refresh heals the entry and the assertion passes whether or not there is a cap - which is how +the first version of it silently proved nothing. + +`ServedStale` counts answers that were knowingly out of date - its own counter rather than folded into hits, +because a deployment should be able to see how many it served without reading the configuration to work out +whether it could have. + +#### Risks to design for, not discover + +- **Compounding staleness on a hot-written key.** Every refresh is invalidated in flight, `AllValid` + correctly refuses the store, and the entry serves stale indefinitely. Needs an absolute cap — consecutive + stale serves, or a wall-clock bound from first notice — after which it is a real miss regardless. +- **The refresh's store failing is the normal case** under that write pressure, not an edge case; it is the + path the once-only flag has to handle. +- **The two defaults differ.** Expiry-SWR is a reasonable default. Invalidation-SWR is deliberately serving + data the server has *told* us is wrong, and should be explicit, per-context, and named so that choosing it + is a decision rather than an inheritance. + +#### Built: single-flight + +Implemented, with SWR still to come. `TryAwaitInFlight` lets a miss wait on a request already in flight; +`TryBeginFill` registers the leader; the registration is released on **every** path, including a send that +throws — which previously leaked the key silently and would now also hang every waiter. + +Three ordering constraints, each of which was a bug first: + +- **Unregister while the key is alive.** The key is the dictionary key, so removing it hashes and compares + the buffer — and completing a fill disposes that key. Folding unregistration into the same `finally` as + the publish threw `ObjectDisposedException` on `RefCountedBuffer`. +- **Publish after the store, unregister before it.** A waiter wakes and re-probes, so it must not be woken + before there is anything to find; and it must not be able to re-attach to a registration whose reply has + already arrived. A caller landing in the gap between the two finds neither and sends for itself — a missed + coalescing opportunity, not a wrong answer. +- **Waiters are woken on refusals too**, not only on success. They then miss and fetch for themselves, + which is what they would have done anyway. + +**Sync callers do not coalesce.** Waiting on another caller's `Task` from a synchronous method is the +sync-over-async problem this design avoids elsewhere, so `Send` still issues its own request. Deliberate, +given sync is deprioritised; it closes when the executor grows a synchronous wait. + +`Coalesced` counts the stampedes that did not happen, and is the counterpart to `RedundantFills`. The two +together are the useful signal: coalesced rising while redundant stays flat is the shape you want. + +#### Built: CachePolicy and a finite lifetime + +`CachePolicy` is a sealed class held once by the cache - deployment-level configuration, not a per-call +argument. `RespContext.WithMaxCacheAge` is the single per-call knob, because freshness tolerance is the one +thing that genuinely varies by caller *and* the one thing that could never be added to `IDatabase` without a +binary break. + +Three properties that took a wrong turn first and are worth stating flatly: + +- **The default lifetime is finite** (one minute). See §6.14: the absence of a lifetime is not the absence + of a policy, it is `TTL = infinity`. +- **Age is checked on read, never stamped on store.** One entry serves callers with different tolerances; + a test asserts exactly that (`OneEntryServesCallersWithDifferentTolerances` - one send, one entry, two + contexts). +- **A context narrows, never widens.** The effective limit is `Min(policy, caller)`, so a caller can ask for + fresher but never for staler than the deployment allows. + +Clock is `Stopwatch.GetTimestamp()`: `Environment.TickCount64` does not exist on `net461`/`netstandard2.0`, +and the 32-bit `TickCount` wraps every ~49 days. + +`Expired` counts hits refused for age, and is deliberately separate from invalidation - nobody told us the +entry was wrong, we just stopped trusting it. A high count against `Stored` means either the lifetime is +shorter than the data's useful life, or invalidation is doing nothing for you. + +#### Consequence for the context + +Soft window, hard TTL, invalidation-SWR on/off, staleness cap — four knobs, all contextual for the reasons +in §6.14. Growing `RespContext` field-by-field past its 48 bytes for those is the wrong shape; they want a +single small `CacheOptions` in the service slot, which is the pattern `ChannelPrefix` already set (§3.3). + + +### 6.16 `RespResult`, and why a span cannot share a buffer + +`RespResult` is registered as a built-in result type, so any command can come back undecoded: +`ctx.SendAsync($"...")`. That matters most for **other people's** commands — a library like +NRedisStack reaches the server through the escape hatch and wants the reply, not a shape this library +happens to model. Registering one handler lights the whole surface up without enumerating a single command. + +Keep two things apart: this is the *mechanism*, not permission. Whether a given reply may be **cached** is +still per-command, and the server does not track the `FT.*` family for invalidation at all. + +#### The finding: `Parse(ReadOnlySpan)` structurally cannot share + +`ReadLease` already implements the sharing we want, and picks between two strategies: + +```csharp +if (reader.TryReservePayload(out var reservation)) // contiguous, and the buffer is known + return Lease.Create(reservation.Owner, reservation.Offset, reservation.Length); +// otherwise rent and copy +``` + +The first branch needs the reader to know **which buffer the bytes live in** — which is why +`RespResult.Read()` constructs `new RespReader(buffer.GetSpan(), buffer)`, passing the buffer as a reader +*service*. + +A `ReadOnlySpan` cannot carry that. So every handler on the interpolated surface copies, whatever it +returns, and this is not a quality-of-implementation problem — it is the signature. It shows up concretely +in the `Lease` handler, which builds `new RespReader(response)` with no service and therefore always +takes the copy branch, where the connection path shares. + +**So `Parse(ref RespReader)` is not merely about composability.** If the executor constructs the reader with +the buffer attached, one change unlocks three things: composable handlers (§2.2's argument), `ReadLease` +sharing instead of copying, and a `RespResult` that reserves rather than captures. + +#### ...but sharing is safe per *topology*, not per *type* + +The reason the connection path can hand out a **mutable** `Lease` into its own reply buffer is that a +reply is **single-owner**: whoever received it owns it, and a lease into it is transitively theirs. The +existing comment even accepts the consequence — a small payload pins the whole reply, "deliberate, and +cheaper than the copy". + +A **cache entry is multi-owner**. The same move there lets one caller mutate bytes another will read, and +`Lease.ArraySegment` hands out the underlying pooled array, so it is not even bounded by the entry. + +So the reader's buffer service is precisely the switch: attach it for a fresh reply, withhold it for a +cache hit. Per-type rules do not work, because the *same* type is safe in one topology and not the other. + +`RespResult` is the exception that proves it: its public surface is read-only readers, so it is safe in +**both** topologies, and it holds the **raw frame** with interpretation deferred to `RespReader` — which +also makes it immune to the streaming case that makes a byte-lease conditional (a chunked scalar has to be +assembled to be handed over as bytes; it does not have to be assembled to be *stored*). + +#### Built: `RespResult` shares the buffer + +The blit is gone on the path that matters most. `RespResult.Share(buffer, offset, length)` takes a +reference instead of renting and copying, and the handler reaches it through an **internal** +`IRespPayloadHandler` — because a `ReadOnlySpan` cannot carry buffer identity (see below), and +judging when retaining is safe is a privilege the library keeps rather than an option it offers. + +Three things this needed: + +- **`RespResult` had to learn to be a window.** `RawSpan` was `_buffer.GetSpan()` — buffer *is* frame — + whereas a cached `RespPayload` is `(lease, offset, length)` inside a larger one. It now carries the + offset and length, and `Read`/`ReadScalar` slice accordingly while still passing the buffer as a reader + service, which is what lets a lease taken from the reply reserve against it in turn. +- **One dispatch point.** The executor's `Parse` helper tests for the payload-aware shape once; every reply + already funnelled through it. **Except the two cache-hit paths, which called `handler.Parse(hit.Span)` + directly** — so the first version shared on a fresh reply and quietly copied on a cache hit, which is + precisely backwards. Caught by asserting the reference count rather than the bytes. +- **A fallback that does not resurrect.** `TryAddRef` is increment-if-nonzero, so losing the race against + the final release yields `null` and the handler copies. Winning it by any other means would hand back a + buffer already on its way to the pool. + +Why this is safe where a byte lease would not be: `RespResult` exposes only `RespReader`, so nothing can +write through it. And sharing a *cache entry* pins nothing extra — the entry holds that buffer for its own +lifetime regardless — so this is the case where sharing is both safe and free. + +#### Decided: share internally, copy on the way out + +The interpolated surface **always copies what it hands to a caller**. Not a retreat — three reasons, and the +first is the one that is easy to miss: + +- **Sharing pins.** `ReadLease`'s own comment accepts that "a small payload can pin the whole reply - + deliberate, and cheaper than the copy". That trade is fine for a reply about to be dropped. With a cache + it is much worse: entries live for *minutes*, so a caller keeping one field of a large multi-key reply + holds the entire buffer for the life of the entry. Copying a small value out is not only safer, it is + **smaller**. +- **The economics were never there.** The cache's win is skipping a network round trip, not skipping a + memcpy of a typically-tiny reply. +- **It dissolves the whole topology problem.** No `IReadOnlyPayloadReservationProvider`, no "is this buffer + shared" flag on `RespReader`, no retiring `ReadLease` by removing its `this`, and therefore no source break + on a shipped API. + +**"Always copy" applies to the *mutable* outgoing type, not to every outgoing type.** Two types, two +different reasons to be safe: + +| outgoing type | strategy | safe because | +|---|---|---| +| `Lease` | always copies | the caller owns bytes nobody else can reach | +| `ReadOnlyLease` | shares when it can | nobody can write through it | + +So `Lease` becomes correct again — it was only ever wrong when shared — **and** `ReadOnlyLease` +earns its existence by being the one that may share. Collapsing both into "copy" throws away the only +reason to have the second type. + +**The pinning argument flips by topology, and in favour of sharing where it matters.** Sharing a *fresh +reply* pins a buffer that would otherwise be recycled at once — a real cost, and the one `ReadLease`'s +comment is about. Sharing a *cache entry* pins **nothing extra**: the cache is holding that buffer for the +entry's lifetime whatever the caller does. So a read-only lease over a cache hit is safe *and* free, which +is the case the whole exercise is about. + +**And the machinery collapses.** No "is this buffer shared" flag on `RespReader`, no second reservation +interface: the mutable path simply **never calls `TryReservePayload`**, and the read-only path does. The +*type* decides, at compile time, with no topology reasoning at any call site. `TryReservePayload` and +`IPayloadReservationProvider` stay alive serving the read-only path, so there is no dead-code cascade +either. + +Streaming behaves as it does today: a read-only lease shares when the payload is one contiguous run and +copies when it is chunked — exactly the two branches `ReadLease` already has, and exactly what `Lease`'s +dual backing (`T[]` *or* `IMemoryOwner`) exists to express. + +The rule, in one line: **share what cannot be written; copy what can.** The cache also shares the reply +payload with itself (`TryComplete` retains rather than copies, §6.3). + +#### Built: `ReadOnlyLease`, and retiring the mutable spelling + +`RespReaderExtensions.ReadLease` keeps its name, signature and containing type — so anything already +compiled against it still binds — and merely stops being an extension method, gaining `[Obsolete]` +explaining where to go. `RespReaderLeaseExtensions.ReadLease` takes over the call site, returning +`ReadOnlyLease` and sharing where it can. The old one now **always copies**. + +That is a **source** break for callers who named the type or wrote through the result — precisely the +callers for whom sharing would have been unsafe, which is why a compile error is the right way for them to +find out. It is recorded as an explicit `*REMOVED*` line in the public API files rather than left to be +discovered, so the break appears in the API diff. + +`ReadOnlyLease` deliberately has **no `ArraySegment`**. `Lease` has one and it hands out the +underlying array, which for shared memory is a way to reach outside the lease entirely. `DecodeString` and +`AsStream` still need an array, and get one internally via `MemoryMarshal.TryGetArray` — the library may do +what it will not offer callers by default. A caller determined to reach the array through `MemoryMarshal` +themselves can, and that is their responsibility: it is an explicit escape hatch, not an accident. + +Sharing **pins**, and that is the remaining cost: the lease holds the whole reply alive. +`ReadOnlyLease.ToArray()` is the way out when a small value must outlive a large reply. For a *cached* +reply the pinning is free, because the entry holds that buffer anyway. + +The working queue lives in `design/interpolated-resp-writer.queue.md`. + + +## 7. Analyzer rules + +The analyzer **does** reach consumers: `StackExchange.Redis.csproj:83-100` packs both +`eng/StackExchange.Redis.Build` and `eng/StackExchange.Redis.CodeFixes` into `analyzers/dotnet/cs`, +with hard errors at pack time if either is missing. (AGENTS.md describes that project as "Not shipped", +which is now inaccurate.) + +**`.Resp()` is permitted only on literals.** That makes every rule below decidable, and it does not +break the static-property reuse pattern — `.Resp()` is still on a literal at the declaration site, and +the call site is a plain property read. + +For this to be enforced rather than advisory, `Raw`'s constructor must be `internal` with `.Resp()` as +the only public factory. Internal construction stays available for `CommandMap.GetResp` and +per-connection preambles. + +Rules: + +1. **Raw framing** — `$`, digits, CRLF, payload of exactly that length, CRLF; repeated exactly + `ArgCount` times, consuming the whole blob. +2. **Canonical casing** — token payloads uppercase. The cache-key correctness rule; the only one whose + violation is silent. +3. **Receiver is `u8`, not `string`** — otherwise a runtime encode was silently paid. +4. **No raw string literals for RESP blobs** — `.gitattributes` is `* text=auto`, so a `"""..."""` + blob bakes in platform-native line endings (LF on Linux, CRLF on Windows). Require `\r\n` escapes. +5. **No keys inside a `Raw`** — invisible to the handler, so they would never be registered for + invalidation or counted for routing. +6. **Shape** — at least one hole; first hole is a `RedisCommand`. +7. **Inline literal tokens** — reject, with a fixer offering `{RespLiterals.Nx}`. See §2.1. **Implemented** + as `SER309` (`RespInterpolationAnalyzer` + `RespLiteralCodeFixProvider`). +8. Possibly: a better diagnostic than `CS1503` for an unsupported hole type. + +Rules 1–3 have mechanical code fixes, which is presumably what the CodeFixes assembly is for. + +--- + +## 8. Prior art: the `marc/respite` spike + +`origin/marc/respite` is the v3 PoC spike — a tip (nothing contains it), 117 commits ahead of `main`, +370 files under `src/`, last commit `2026-03-06` *"support WriteMode"*. Siblings from the same push: +`marc/localwriter` (2026-03-06) and `marc/resp-reader` (2026-03-13). `marc/respite-weekend` +(2025-09-22) is a sub-branch that merged into it, not the spike itself. + +Projects: `RESP.Core`, `RESPite`, `RESPite.Redis`, `RESPite.StackExchange.Redis`, +`RESPite.Benchmark`, alongside `StackExchange.Redis`. + +**This document replaces the spike's *writer* half only.** The execution API around it is good and +carries over. + +### 8.1 What carries over + +`RespContext` — a `readonly struct` of four fields: + +```csharp +private readonly RespConnection _connection; +public readonly CancellationToken CancellationToken; +private readonly int _database; +private readonly RespContextFlags _flags; + +public RespCommandMap CommandMap => _connection.NonDefaultCommandMap ?? RespCommandMap.Default; +``` + +- `With*` clone surface — `WithCancellationToken`, `WithDatabase`, `WithConnection`, `WithFlags`, + `With(db, flags)`, `With(db, flags, mask)`, `ConfigureAwait` — each copying the struct and assigning + through `Unsafe.AsRef(in clone._x)`. +- Cancellation: `WithCombineCancellationToken` / `WithCombineTimeout` / `WithCombine` return a + `Lifetime : IDisposable` owning the linked CTS, with the no-op fast paths handled (uncancellable + token, already-equal token, no existing token to link). +- `RespContextFlags` is bit-aligned with `CommandFlags`, so `RespContextDatabase.Context(flags)` is a + cast plus a mask — no mapping table. +- `RespContextDatabase` implements `IDatabase` over an `IRespContextSource`, split across the usual + per-type partials. + +**This is the context object §3.3 arrives at independently**, and its factoring is better than what +§3.3 first proposed: it *derives* `CommandMap` from the connection rather than storing it, which is +what keeps it to four fields and makes the `With*` clone pattern affordable. Adopt that — store the +connection, derive the rest — rather than a class holding CommandMap + prefix + buffer manager + +cache + `ServerType`. + +### 8.2 What the handler replaces + +The spike's writer is a manual builder: `RespOperationBuilder` from +`RespContextExtensions.Command()`, then `Send`/`SendAsync`/`CreateOperation`, with the frame +assembled by hand through `RespWriter`. Three specific things get better: + +| Spike | Handler | +| --- | --- | +| `WriteCommand(command, args)` — caller states the arg count | `formattedCount` is a compile-time constant | +| `WriteKey(...)` — a plain alias for `WriteBulkString`, no behaviour | `AppendFormatted(RedisKey)` — prefix, slot fold, key marks | +| `public RespCommandMap? CommandMap { get; set; }` on the writer | supplied via the receiver; immutable, cannot be forgotten | + +The first is the substantive one. A hand-maintained argument count can disagree with the writes that +follow it, which is exactly the failure `RenderedArgs.ThrowArgCountMismatch` exists to catch on `main` +— a runtime check for something the handler makes unrepresentable. + +The second matters because `WriteKey` is where routing and invalidation have to attach. The spike cut +the seam in the right place and left it empty. + +### 8.3 The key-prefix gap + +The spike has `RespConfiguration.KeyPrefix` (`ReadOnlySpan`, settable as `string` or `byte[]` +on the builder) *and* distinct `WriteKey` overloads on `RespWriter` — but nothing connects them; the +prefix is never applied, and `KeyspaceIsolation/KeyPrefixed*.cs` is still present on the branch. + +So "the execution API that replaces `KeyPrefixedDatabase`" is the *pattern* — a cheap value-type +context threaded through with `With*` clones, instead of a decorator object per prefix — rather than +something finished. Wiring it is part of this work: `AppendFormatted(RedisKey)` applies the prefix +before both the write and the slot (§5.1). + +--- + +### 8.4 Key prefixes: both mechanisms, permanently + +The two prefix mechanisms coexist for good — the context's prefix, and the prefix a `RedisKey` already +carries from a `KeyPrefixed*` decorator. That is awkward conceptually but free in the writer: +`RedisKey.WithPrefix` only allocates in its *"two prefixes; darn"* branch because it must hand back a +`RedisKey`; the handler needs only the combined *bytes*, and `TotalLength()`/`CopyTo()` already include +the key's own prefix. So writing the context prefix immediately ahead of them composes both with no +intermediate object. Verified: **0 bytes allocated over 128 renders** with both prefixes in play. + +The context normalises its key prefix to bytes once at construction, so a string-backed prefix does not +re-convert per command. `AppendKeyPrefix` still composes eagerly via `WithPrefix`, but that is once per +context clone, not per command. + +The two mechanisms stay distinguishable as *values* (`RedisKey.Equals` compares the carried prefix) but +render byte-identically — pinned by `BothPrefixMechanismsRenderIdenticalBytes`. That is what lets the +rendered frame serve as a cache key (§6): cache identity must come off the frame, never off the key +object. + +#### The new `KeyPrefixedDatabase` + +The write half collapses to `localCtx = downstreamCtx.AppendKeyPrefix(prefix)` with no per-method +overrides — roughly 2600 lines of forwarding in `KeyspaceIsolation/` become one context clone. + +**The read half is the open part**, and today it is largely unhandled rather than merely imperfect: + +- `KeyRandom`/`KeyRandomAsync` **throw** `NotSupportedException`, documented in the `WithKeyPrefix` + remarks (`DatabaseExtension.cs`). +- Script and `Execute` results carry prefixed keys — seven `// TODO: ... might make sense to 'unprefix'` + sites, and the public docs state the caveat. +- RESP APIs opt out deliberately: `// note the Resp API explicitly doesn't unprefix keys`. +- Multi-key pop results (`ListPopResult` from `ListLeftPopAsync(RedisKey[], long)`) forward unstripped, + not even TODO'd. + +The decorator *cannot* fix this: it sits above the database with no hook into result processing, so +stripping would mean wrapping every return value. A prefix on the context, threaded through to the +result processor, makes it one concern in one place. `ChannelPrefix` already has exactly this shape on +the read side (`PhysicalConnection.Read.cs:817`); keys never got the equivalent. + +Two constraints on doing it: + +- **Strip at the API boundary, never in the cache or routing layer.** Invalidation pushes carry key + names, and the cache is keyed on rendered frames, which contain *prefixed* keys — so an invalidation + in prefixed form matches as-is. Stripping earlier would silently stop invalidations matching: stale + reads, no error. +- **Stripping must be conditional.** A script can return a key it built itself that never carried the + prefix, so it has to be "starts with the prefix → strip, else leave", and documented as such. + +Commands that return key names, and so need this: `RANDOMKEY`, `KEYS`, `SCAN`, the blocking and multi +pops (`BLPOP`/`BRPOP`/`LMPOP`/`ZMPOP`/`BZPOPMIN`/`BZPOPMAX`), `XREAD`/`XREADGROUP` stream names, +keyspace notifications, and script/`Execute` results. + +## 9. The spike in this repo + +A working spike. The surface is public but gated behind `SER010`/`SER011` — see §9.1. + +| File | What it is | +| --- | --- | +| `src/StackExchange.Redis/FrameworkShims.InterpolatedStringHandler.cs` | the attribute polyfill (§1), same shape as the `IsExternalInit` shim | +| `src/StackExchange.Redis/Interpolated/RespContext.cs` | CommandMap, KeyPrefix, ChannelPrefix, Database, ServerType, CancellationToken; `With*` clones; `Execute` | +| `src/StackExchange.Redis/Interpolated/RespRequestBuilder.cs` | renders the frame, folds the slot, marks keys | +| `src/StackExchange.Redis/Interpolated/RespFrame.cs` | rendered frame + slot + key marks + `KeyRange` | +| `tests/StackExchange.Redis.Tests/InterpolatedWriterUnitTests.cs` | 41 tests | +| `src/StackExchange.Redis/Interpolated/RespFragment.cs` | pre-framed token runs + the `[Resp]` marker | +| `tests/StackExchange.Redis.Tests/InterpolatedWriterDemo.cs` | 7 worked examples, each asserting the exact frame | +| `tests/StackExchange.Redis.Tests/InterpolatedWriterFragmentTests.cs` | 16 tests; declarations only - the generator supplies the bodies | +| `src/StackExchange.Redis/Interpolated/RespRequest.cs` | the rendered request; also the cache key (§6.4) | +| `src/StackExchange.Redis/Interpolated/RespPayload.cs` | a reply as a pooled, reference-counted blob (§6.4) | +| `src/StackExchange.Redis/Interpolated/RespClientCache.cs` | table 1: `(request, db)` to payload + generations (§6.6) | +| `src/StackExchange.Redis/Interpolated/RespKeyTable.cs` | table 2: key bytes to a generation; the only thing invalidation touches (§6.6) | +| `src/StackExchange.Redis/Interpolated/RespExecutor.cs` | `IRespExecutor`, `IRespHandler`, and the `Send` orchestration (§6.7) | +| `src/StackExchange.Redis/Interpolated/RespFrameWriter.cs` | renders an existing `Message` into a `RespFrame` (§6.8) | +| `tests/StackExchange.Redis.Tests/InterpolatedWriterCacheKeyTests.cs` | 12 tests: zero-alloc hits, use-after-release, concurrent readers vs eviction | +| `tests/StackExchange.Redis.Tests/RespClientCacheTests.cs` | 44 tests: invalidation, the in-flight race, flag gates, counters | +| `tests/StackExchange.Redis.Tests/MessageToRespFrameTests.cs` | 7 tests: existing `Message` objects through the new pipeline | +| `tests/StackExchange.Redis.Tests/InterpolatedWriterCapacityTests.cs` | buffer arithmetic asserted directly, where pool slack cannot mask it | +| `tests/StackExchange.Redis.Benchmarks/ClientCacheBenchmarks.cs` | `OnInvalidate` under a broadcasting flood | + +Green on net10.0 and net8.0 (58 tests); net481 compiles; `-c Release /p:CI=true /p:RunAnalyzers=true` +clean. + +`InterpolatedWriterDemo` is the readable end-to-end example — each case asserts the exact rendered frame, +so it doubles as documentation of what the shapes produce: + +``` +FixedArity *2|$3|GET|$6|user:1| slot=10778 keys=user:1 +KeyAndValue *3|$3|SET|$6|user:1|$4|marc| slot=10778 keys=user:1 +KeyspaceIsolation *2|$3|GET|$9|t7:user:1| slot=13865 keys=t7:user:1 +OptionalArguments *5|$3|SET|$6|user:1|$4|marc|$2|EX|$3|300| slot=10778 keys=user:1 +VariadicWithSharedHashTag *4|$3|DEL|$5|{u}:a|$5|{u}:b|$5|{u}:c| slot=11826 keys= +CrossSlotIsDetected *3|$3|DEL|$5|alpha|$4|beta| slot=MULTI keys=alpha,beta +ChannelPrefix *3|$7|PUBLISH|$8|app:news|$2|hi| slot=5631 keys= +``` + +What the tests pin, grouped by the section they belong to: + +- **Literals (§2.1)** — `SingleSpacesAreAllowedAndDiscarded` (spaced and unspaced render identical + bytes), `OtherLiteralsAreRejected` (two spaces, a hyphen, a leading command name). +- **Framing** — `RendersCommandKeyAndValue`, `RendersExactBytes`, `MultiByteAndEmptyPayloadsRoundTrip`, + `LargePayloadForcesBufferGrowthMidBuild` (forces a pool regrow *after* the prologue is reserved). +- **Header back-fill (§4)** — `HeaderBackfillIsRightAligned`, theory over 1/9/10/120 extra arguments, so + the frame start moves as `*N` gains digits; it also asserts the key offset survives that. +- **CommandMap (§2.4)** — `CommandMapRenamesAreApplied`, `DisabledCommandThrows`, `CommandMustComeFirst`. +- **Prefixes (§3.4, §8.4)** — `KeyPrefixIsAppliedToTheWire`, `KeyPrefixComposesWithAKeyThatAlreadyHasOne`, + `NestedAppendKeyPrefixComposes`, `BothPrefixMechanismsRenderIdenticalBytes`, + `ComposingBothPrefixMechanismsDoesNotAllocate`, `ChannelPrefixIsApplied`, + `ChannelPrefixIsSkippedWhenTheChannelOptsOut`. +- **Keys and routing (§5)** — `NoKeysMeansNoSlotAndNoMarks`, `OneAndTwoKeysResolveWithoutScanning`, + `ThreeKeysFallBackToScanning`, `StandaloneSkipsSlotComputation`, + `ClusterFoldsTheSlotFromTheWrittenBytes`, `SharedHashTagGivesOneSlot`, `CrossSlotKeysAreDetected`, + `SlotIsComputedFromThePrefixedKey`. +- **Cache identity (§6.2)** — `DatabaseIsNotPartOfTheRenderedFrame`. +- **Cancellation (§3.3)** — `CancellationIsObservedAndTheBufferIsReturned`, + `CancellationTokenFlowsThroughWithClones`. +- **Deferred composition (§4.1)** — `ComposeThenConditionallyAppend` (theory over the four + ttl/nx combinations), `ComposedKeysStillTrackAndRoute`, `ComposedHeaderGrowsWithLateArguments`, + `ComposeWithCommandArgument`, `ExecuteWithCommandArgument`, `ComposeWithNoInterpolationAtAll`, + `DisabledCommandThrowsFromBothInitializerForms`. + +**What the spike does not do:** it stops at "the right bytes were rendered, and we know which arguments +were keys". Nothing dispatches, nothing caches, and the read half (§8.4) is untouched — so the claim +that the context can be threaded through to result processing is design, not demonstration. + +--- + +### 9.1 Public API, and the `RedisCommand` problem + +The surface is public but **experimental**, under two diagnostic IDs: + +| | | +| --- | --- | +| `SER010` | the feature as a whole; in the repo-wide `NoWarn`, so internal use is quiet | +| `SER011` | **hand-constructing a `RespFragment`**; deliberately NOT in `NoWarn` | + +`SER011` exists because nothing validates the bytes handed to `new RespFragment(...)`: a length prefix +that disagrees with its payload, a missing CRLF, or an `ArgCount` that does not match the `$` runs will +corrupt the connection for every command that follows, with the first symptom appearing somewhere +unrelated. So the ctor is gated, and *generated* code suppresses it at the emit site and nowhere wider: + +```csharp +#pragma warning disable SER011 // this half stands in for the generator +internal static partial RespFragment EX => new("$2\r\nEX\r\n"u8); +#pragma warning restore SER011 +``` + +Verified that it fires unsuppressed, as an error carrying the docs link +(`https://seredis.dev/exp/SER011`). + +**Making it unsuppressible is possible; the question is whether it should be.** Two mechanisms: an +analyzer rule tagged `WellKnownDiagnosticTags.NotConfigurable`, or a generator that detects a +hand-written construction and emits `#error`. The second is the stronger, and is genuinely absolute — +verified that `#error` survives both a blanket `#pragma warning disable` and a targeted +`#pragma warning disable CS1029`: + +``` +error CS1029: #error: 'RespFragment constructed by hand at Foo.cs(12,34); see https://seredis.dev/exp/SER011' +``` + +Two objections that look like blockers are not: + +- *"It points at generated source rather than the call site."* The **message** carries whatever text the + generator puts in it, including the offending file and position — as above. +- *"The generated-code exemption is spoofable."* It is not: the generator knows precisely which + constructions are its own, because it emitted them. That is identity, not an `` header + check. + +**Do both.** They are not alternatives — they do different jobs, and the weaknesses cancel: + +| | Job | Suppressible | +| --- | --- | --- | +| `[Experimental(SER011)]` | code fix — "convert to a `[Resp]` partial property" | yes | +| generator emits `#error` | the build fails regardless | no | + +The generator does not need to know whether the diagnostic was suppressed; it emits `#error` for every +construction it did not emit itself. So suppressing `SER011` removes the squiggle and changes nothing +about the outcome, which is honest — the suppression stops claiming to achieve something it does not. + +**`#error` is not stuck at "right here":** `#line` redirects it, so the generator can report *at the +offending call site*, in a file it never wrote. Both forms verified, reporting into a `Consumer.cs` that +does not exist: + +```csharp +#line 42 "Consumer.cs" // -> Consumer.cs(42,8) +#line (12, 34) - (12, 58) 1 "Consumer.cs" // -> Consumer.cs(12,40), the C# 10 span form +#error RespFragment constructed by hand; see https://seredis.dev/exp/SER011 +#line default +``` + +The span form is the one added in C# 10 for generators. Note the mapped column derives from the physical +column in the *generated* file adjusted by the `charOffset` argument — asking for column 34 gave 40 — so +landing it exactly means laying the emitted line out deliberately, not merely stating the offsets. + +Since generators run in the IDE, this gives a live squiggle in the right place, which leaves the code fix +as the analyzer's only unique contribution. + +**This only works with a sanctioned escape hatch, and it has to exist first.** A +`RespFragment.CreateValidated(bytes, argCount)` that checks framing at runtime, which the generator +ignores. Then "you cannot hand-roll a fragment" is a true statement with a supported answer for the +startup-built case from §2.3, rather than a dead end. Without it, the absolute block is the hostile +version. + +The one remaining tension is with this repo's own bar for an error. `Diagnostics.cs` reserves +`DiagnosticSeverity.Error` for code that *cannot work*, and a hand-built fragment with correct bytes +works. The counter-argument, which carries here: **the blast radius is not the caller's own code.** +Malformed RESP desyncs the connection for every *subsequent* command, so the damage is unbounded and +lands somewhere unrelated. That is a different class of hazard from "you get a wrong answer", and it is +what justifies the asymmetry. +- **It forecloses the legitimate case** in §2.3 unless `CreateValidated` ships alongside it — which is + why that is a precondition rather than a nicety. + +Not yet built: the generator does not exist (§2.3 spikes the pattern with both halves hand-written), so +the `#error` half is a design, not a demonstration. What *was* verified is the part it depends on — that +a `#error` anywhere in the compilation cannot be suppressed. + +Until then, keeping `SER011` out of `NoWarn` is the proportionate friction: blanket-disabling means +adding it to the csproj where review sees it, and per-site suppression means a pragma naming `SER011`. Blanket-disabling it means +adding it to the csproj, where review sees it; per-site suppression means a pragma naming `SER011`, which +makes `git grep SER011` an exact inventory of every hand-rolled fragment in a codebase. Auditable beats +absent, for a failure mode whose whole problem is invisibility. If more teeth are wanted, the +proportionate lever is a rule that flags a *project-wide* suppression while leaving per-site pragmas +alone. + +One portability note: `ExperimentalAttribute.Message` is .NET 9+, so a custom message cannot be used +while this targets net461 through net10.0. The explanation lives in `docs/exp/SER011.md`, which +`UrlFormat` links to — which is the existing convention here anyway. + +**The blocker was `RedisCommand`, which is `internal`.** The whole design routes commands through it for +`CommandMap` aliasing, but the public surface deliberately exposes commands as typed methods or +`Execute(string command, ...)`; making that enum public would be a large, permanent commitment to an +implementation detail. + +Resolved by giving public callers a **string overload that speculatively parses**, which is exactly what +`Execute(string)` already does (`RedisDatabase.cs:6149-6155`): + +```csharp +if (!RedisCommandMetadata.TryParseCI(adhocCommand, out knownCommand)) + knownCommand = RedisCommand.UNKNOWN; +``` + +So a recognised name still gets command-map aliasing and disabling; anything unrecognised is framed +verbatim. `RedisCommand` stays internal, and those overloads stay internal alongside it. Behaviour is +consistent with the existing ad-hoc command path rather than a second set of rules. + +--- + +### 9.2 Measured against the existing writer + +`InterpolatedWriterBenchmarks` compares rendering the same command three ways, formatting only — no +server, no dispatch — all writing into the same pre-allocated `IBufferWriter`: + +| Method | What it is | Mean | Allocated | Ratio | +| --- | --- | ---: | ---: | ---: | +| `KeyValue_Message` | `Message.Create` + `WriteTo` — the typed path `db.StringSet` uses | 63.97 ns | 136 B | 1.00 | +| `KeyValue_Adhoc` | `ExecuteMessage` over `object[]` — what `Execute(string, ...)` does | 89.69 ns | 152 B | 1.40 | +| `KeyValue_Interpolated` | this | **41.72 ns** | **0 B** | **0.65** | +| `Expiry_Message` | four arguments, typed path | 90.91 ns | 168 B | 1.00 | +| `Expiry_Interpolated` | four arguments, this | **69.83 ns** | **0 B** | **0.77** | + +So roughly **a third faster than the typed path and twice as fast as the ad-hoc string path**, with no +managed allocation where both existing paths allocate 136-168 bytes per command. The ad-hoc row is the +relevant comparison for the public string overload (§9.1), since that is what it competes with. + +Two honest qualifications: + +- **The comparison is conservative on time.** The interpolated path renders into a rented buffer and then + copies into the target; `Message` writes straight through. Removing that copy would widen the gap. +- **The 0 B will not survive dispatch.** `Message` allocates partly because it is *retained* for the + response. A dispatching implementation needs per-command state too, so the end-to-end delta will not + stay 136 B → 0. What the zero does establish is that *formatting itself* need not allocate, which is + the half this replaces. + +--- + +### 9.3 The generator, the analyzer and the fixer + +Built, so the authoring story is no longer hand-waved: + +| | | +| --- | --- | +| `RespFragmentGenerator` | implements `[Resp]` partial properties — framing, length prefixes, casing and `ArgCount` by construction | +| `RespInterpolationAnalyzer` | `SER309`: literal text in a RESP command is discarded, not sent. **Error** | +| `RespLiteralCodeFixProvider` | rewrites `$"{key} nx"` to `$"{key} {RespLiterals.Nx}"` | +| `RespFragment.CreateValidated` | the sanctioned runtime route: checks framing and the argument count | +| `SER351` | a `[Resp]` declaration the generator cannot implement, rather than skipping it in silence | + +The fragment tests now declare only the properties; the generator supplies the bodies, and the exact-frame +assertions pass unchanged — which is the real check, since it means `EX` was inferred and upper-cased, +`lib-name` came through verbatim, and `SETINFO lib-name` counted as two arguments. + +The emitted file suppresses `SER010` and `SER011` at source and nowhere wider, which is the pattern §9.1 +describes: the generator is the sanctioned construction site. + +`CreateValidated` is the precondition §9.1 names for ever making hand-construction impossible: it walks the +bytes, checks every `$len\r\n…\r\n` and that the count matches, and throws otherwise. Not gated, because the +check is the point — the cost is irrelevant when it runs once at startup, and it is the difference between a +mistake that throws at the call and one that desyncs the connection somewhere unrelated. Eight malformed +shapes are covered by tests, each of which would otherwise have corrupted the stream. + +**The `#error` half is deliberately NOT built.** The generator could detect a hand-written +`new RespFragment(...)` and emit `#error`, and §9.1 records how — but that forecloses the escape hatch, so +`CreateValidated` had to exist first. Whether to take the next step is a judgement about how hostile to be, +which is worth making deliberately rather than as a side effect of me being on a roll. + +**Both fixes exist.** When a matching declaration is in source, use it; when none is, declare it in the type +containing the call site. That is not an obviously right home, but it is the only one that needs no guessing, +and moving it afterwards is trivial — so the strict form costs a keystroke rather than a lookup. The fix adds +`partial` to the host type when it is missing, and includes the attribute argument only when inference would +not reproduce the token (`nx` needs none; `lib-ver` does). + +Word boundaries can only come from separators, so `withsave` becomes `Withsave`, not `WithSave`. Guessing +where words divide would need a dictionary and would be wrong often enough to be worse. + +A run of several tokens is still left alone, having no single answer. + +#### `using static` closes most of the remaining gap + +`using static` imports the fragments, so the declared form reads very close to the inline one it replaces: + +```csharp +using static RespLiterals; +... +ctx.Execute(RedisCommand.SET, $"{key} {value} {Nx} {Ex} {300}"); // vs. "... nx ex 300" +``` + +Verified. The difference is braces and a capital letter — which is a much weaker case for ever supporting +inline tokens than it looked when §2.1 weighed it. + +Notes from building it, in case they bite again: + +- The analyzer project targets `netstandard2.0` against Roslyn 4.3, so: no records (no `IsExternalInit`), + and `LanguageVersion.CSharp11` has to come from the existing `LanguageVersions` shim. +- Detection is by **converted type** on the interpolated string rather than + `OperationKind.InterpolatedStringHandlerCreation`, which keeps it working against that Roslyn floor. +- `ToMinimalDisplayString` on a *property* includes its type, yielding `RespFragment RespLiterals.Nx`; build + the name from the containing type instead. +- The code-fix test harness runs analyzers, not generators, so its sources spell out both halves — and a fix + that *declares* a fragment leaves the fixed code legitimately reporting `CS9248`, which needed a verifier + overload carrying fixed-state diagnostics. Note `StateInheritanceMode.Explicit` is the wrong tool there: it + drops the inherited references too, and the fixed state stops seeing the library at all. +- **Generator diagnostics have no test harness here.** `SER350` never had one either; the project references + `Analyzer.Testing` and `CodeFix.Testing` but not `SourceGenerators.Testing`. `SER351` was verified by + compiling a deliberately-bad declaration and reading the output, which is weaker than the other rules' + coverage and is worth closing if more generator diagnostics arrive. + +--- + +### 9.4 The context as the extension point + +The intended shape is a context (`db`, key prefix, cancellation, executor) reached from `IDatabase`/ +`IServer` via one new member, with the command surface hanging off it as **extension members**: + +```csharp +ctx.Strings.Set(key, value) => ctx.Execute(RedisCommands.Set, $"{key}{value}") +``` + +Three reasons, in the order different audiences feel them. + +**1. Discoverability.** `IDatabase` is a flat surface of several hundred methods; IntelliSense on `db.` is +not a navigable list, it is a wall. `ctx.Strings.`, `ctx.Hashes.`, `ctx.Sets.`, `ctx.Streams.` groups the +surface the way Redis documents itself — by data type — so the shape of the API teaches it. This is the +benefit an ordinary caller notices first, and on its own it would probably justify the change. + +**2. Module libraries become first class.** NRedisStack today reaches the server through +`db.Execute("FT.SEARCH", …)` or a parallel set of its own interfaces. Extension members over a shared +context mean `ctx.Search.Query(...)` composes exactly like `ctx.Strings.Set(...)` — same cancellation, +same key prefix, same cache participation, no wrapper interface and no forked surface. Module commands +also then arrive through the same `Send`, so they inherit the §6.9 cacheability gates automatically +instead of needing a parallel opt-out story. + +**3. It is the last break.** Adding the member is a breaking change, and adding to `IDatabase` has been +standard practice here, so the cost is familiar rather than novel. The difference is that this one ends +the sequence: once a context exists, every subsequent addition is an extension member and breaks nobody. +The one break buys the end of breaks. + +**That guarantee rests on a discipline, not on the type system.** The first "just this once" method added +to `IDatabase` after the context exists spends the break for nothing. Worth writing down, and eventually +worth an analyzer — this repo already gates hand-built fragments behind `SER011` on the same reasoning, +that the blast radius is not the author's own code. + +#### What the prototype found + +Built as `IRespTarget` + `RespStrings` + `RespSurface` (`RespSurfaceTests`), with a fake executor +underneath. `target.Strings.Set(key, value)` and `.Get(key)` work end to end, through the cache, with +`AppendKeyPrefix` as a context clone and no per-method forwarding. + +- **Extension members compile on every target**, `net461` and `netstandard2.0` included. They are compiler + lowering, like the interpolated handler itself, so the down-level story that made §1 work holds here too. + This was the main risk and it is gone. +- **Each extension member costs TWO `PublicAPI` entries** — the `extension(...)` form *and* the lowered + static (`RespSurface.get_Strings(IRespTarget)`). So "extension members are free to add" is true for + source and binary compatibility, but not for API tracking: the surface still grows, and the lowered + names are part of it. Worth knowing before the surface is hundreds of commands. +- **A plain wrapper is enough.** `RespStrings` holds one `RespContext` field, which makes it + layout-identical by construction — the wrapper *is* the pun, enforced by the compiler, with no `Unsafe` + and no `ref readonly`. Both entry points work: `target.Strings` and `context.Strings`. +- **`Context` returning by value costs nothing visible** and keeps every command `async`-usable, settling + item (1) below. +- **A missing executor throws rather than silently doing nothing** — worth pinning early, because a + `default(RespContext)` is valid by design (§3.5) and would otherwise render a frame and drop it. + +**And one bug the prototype exposed.** `RefusedByFlags` was unreachable in real use: the orchestration +skips the cache entirely when flags forbid caching — it does not probe and then decline — so +`TryBeginFill` was never reached and never counted. A diagnostic that reads zero because nothing asks it +looks like evidence, which is worse than not having it. The flag decision now goes through +`cache.PermitsCaching(flags)`, so the cache observes every refusal without probing anything it has been +told to leave alone. + +#### Plugged in + +`IRedis` now inherits `IRespTarget`, so `IDatabase`, `IServer` and `ISubscriber` all carry `.Context` from +**one** interface edit. The blast radius inside the library was four types, which is smaller than it +sounds: + +| | | +| --- | --- | +| `RedisBase` | throws - covers `RedisDatabase`, `RedisServer`, `RedisSubscriber` | +| `MultiGroupDatabase`, `MultiGroupSubscriber` | throw | +| `KeyPrefixedDatabase` | **implemented**: `Inner.Context.AppendKeyPrefix(Prefix)` | +| `RespDatabase` (new) | the minimal one that actually works | + +`KeyPrefixedDatabase` is worth calling out: that single line is the whole write half of what the class +otherwise does by forwarding ~2600 lines of overrides. It throws today only because its inner target does. + +**`IRespExecutor` is now internal.** Dispatch is an implementation concern; the public surface is the +context plus extension members. That keeps the executor chain - retry, and whatever follows - reshapeable +without it being a breaking change, and it is why `RespContext.Executor` and `WithExecutor` are internal +too. + +**`RespDatabase` has no command methods**, which is the point rather than an omission: `Set`, `Get` and +everything after are extension members over the context, so the type does not grow as the surface does. + +Connection-backed types throw for now. Wiring a rendered frame through the existing message pipeline is +separate work, and nothing here needs to wait for it. + +#### Four things to settle before building it + +1. ~~**`ref readonly` and `async` do not mix.**~~ **Settled: by value.** A `ref readonly` local cannot cross an `await`, and the + command surface is `ValueTask`-first — so the context is copied into the state machine anyway and the + `ref` buys nothing on the only path that matters. At roughly four registers, copy it: return by value, + take `in` on parameters. And keep `RespContext` a `readonly struct`; making it a `ref struct` to "make + it cheap" would make it unusable in the very methods it exists for. + +2. ~~**`RespRequest` must carry its own metadata first.**~~ **Done.** `Detach(flags)` and + `AsLookupKey(flags)` now carry the key marks, slot, argument count and flags, and the request exposes + `KeyCount`/`TryGetKeys`/`GetKey`/`Slot`/`ArgCount`/`Flags`. The mark-resolution logic moved to statics + on `RespFrame` so both types answer identically rather than by duplicated code. + + Note the division of labour this exposes: **routing needs the slot and nothing else** — one `int`, + already folded during the write and gated on `ServerType == Cluster`, since CRC16 over every key is the + expensive part. Only *caching* needs the key marks, which are a few field writes and so are not gated. + The cheap thing is unconditional, the expensive thing is conditional; that asymmetry is deliberate. + + The fold still covers **every** key rather than just the first, because it is not only producing a + routing value: it detects cross-slot, which is a correctness check in cluster. First-key-only would + yield a plausible slot for a command that must be rejected outright. + + Identity deliberately ignores all of it: `Equals`/`GetHashCode` remain the rendered bytes alone, so two + callers issuing the same command with different `CommandFlags` share a cache entry. Pinned by a test. + +3. ~~**A retry executor needs the flags.**~~ **Done, with (2)** — `RespRequest.Flags` carries the retry + category. Retry otherwise fits well: it must hold the preformed payload across attempts, which is + exactly what `RespRequest.TryRetain` is for. + +4. **Decorator order is silent and load-bearing.** `cache(retry(raw))`: a hit must not traverse retry + logic, and a retry must not re-probe a cache it already missed. Nothing in the type system says so, so + it wants a test. + +#### Why the cache is not an executor decorator + +Tempting, because `Send`'s `cache` parameter and `RespContext.Cache` would both vanish. Two reasons not to: + +- **The executor contract deals in OWNED requests** — it has to, because a backlog or resend may need the + bytes past the call, which is what the reference count is for. A cache decorator therefore receives an + already-detached request, so **every call pays for ownership, including hits** — the 48 bytes that + `AsLookupKey` exists to avoid, and the zero-allocation hit with it. Lazy upgrade does not rescue it: a + borrowed request points at the frame's pooled array, and minting a lease from it would give two owners + that both return it to the pool. +- **It would pin the executor to returning raw bytes forever.** The cache stores blobs, so a caching + decorator in the chain forecloses any later move toward executors that return processed results. + +So the layering is deliberate: the **frame level** decides whether to form and send at all — probing, +generation capture, ownership transfer — and the **executor chain** operates on a formed, owned request. +Retry belongs in the chain because it resends the same bytes; caching belongs above it because it decides +whether bytes are needed. + +#### Services, not fields + +The orchestration takes **`in RespContext`** rather than a cache and a cancellation token. A cache is then +just a service the context happens to carry, and `WithCache` is sugar over `WithServices`. + +The slot follows `RespReader`'s: one `object?` that either *is* the requested service — the common case, a +type test — or is an `IServiceProvider` for things the context knows nothing about. That buys +**extensibility with no new fields**, so a capability arriving later costs no API change and no growth in +the struct. Given the whole point of §9.4 is to stop adding members, adding a member per capability would +have been a poor start. + +The executor stays a real field: it is required on every call, where the cache is optional. + +**Cache and retry are still not an either/or between "executor" and "context".** A retry decorator *is* an +executor; installing it is a `With` on the context. Behaviour composes in the chain, configuration on the +context. + +**`GetDatabase()` becomes the secondary API.** Long term the primary entry point returns the new root +interface rather than `IDatabase`; for now it can simply be `NewThing() => GetDatabase()`, since +`IDatabase` implements it. That keeps the transition a rename rather than a fork, and means the "last +break" is spent once at the interface rather than again at the entry point. + +**Keep `IRespExecutor.Send` (the synchronous member).** Driving sync as +`AsTask().GetAwaiter().GetResult()` blocks a pool thread for a whole round trip, which today's sync path +deliberately avoids and which a large constituency depends on. Fine for a spike; but keeping the sync +member means a real sync path can arrive later without reshaping the API, and it costs nothing now. + +### 9.5 Layering: why this stays in SE.Redis for now + +**Decision: it stays. Not moving `RespRequestBuilder` to RESPite.** + +The prize would be real — RESPite already owns `RespReader`, and a matching writer would let anything +build RESP commands with pooled buffers, key marks and slot folding, with no Redis semantics attached. +The route looked available too: give the writer an `AppendKey(ReadOnlySpan)` primitive, let +`RedisKey` reach it through `IRespArgument`, and key reporting flows down a layer while `RedisKey` stays +up here. + +**What stops it is the command, not the key.** A `RedisCommand` hole can only ever be served by an +instance member of the handler type (§2.2 — extension lookup never runs for the handler pattern, and the +CS1503 case proves it), and `RedisCommand` is an `enum`, so `IRespArgument` is closed to it as well. +Whatever assembly declares the handler must therefore know about `RedisCommand`. `RespCommand` does not +rescue it either: it *holds* a `RedisCommand`, and resolves through `RespContext.ResolveCommand`, i.e. +`CommandMap` — renames, disabled commands, per-server-type maps. That is policy, not protocol. + +A key is *bytes*; a command is *a lookup*. Bytes hand down a layer cleanly. A lookup drags its policy +with it. + +**The shape that would work, when it is worth doing:** split the type, do not relocate it. RESPite owns a +`RespWriter` — buffer rental, bulk framing, the `*N` back-fill, argument counters, key marks, slot folding +— exposing primitives only (`AppendBulk`, `AppendKey(prefix, body)`, a pre-framed form, `Complete`). +SE.Redis keeps `RespRequestBuilder` as the `[InterpolatedStringHandler]`, holding a `RespWriter` **by +value** and owning the whole hole vocabulary. Verified to compile and run: a `ref struct` may contain +another `ref struct` by value, and the outer type's `AppendFormatted` members bind normally while +delegating the writing inward. (CS9050 bars a ref *field* to a ref struct; by-value containment is fine.) + +Two things move with it whenever that happens: + +- `AppendKey` needs a **two-span** form, `(prefix, body)`. Today the context prefix and any prefix the key + already carries from a `KeyPrefixed*` decorator are written straight into the frame rather than + concatenated, specifically to avoid the allocation `RedisKey.WithPrefix` would cost. +- `FoldSlot` calls `ServerSelectionStrategy.GetClusterSlot` — CRC16 over the key bytes. Standard Redis + Cluster, so it belongs in the lower layer anyway. + +**Why not now:** it is a pure refactor with no behavioural change, across a spike that is still growing; +moving files today churns everything in flight for nothing. Revisit when the surface stops moving, or +when something outside this repo actually wants to write RESP commands — whichever comes first. + +**Rejected along the way:** "never put a command in a hole, always `Compose(RedisCommand.SET, $"...")`". +That form is already preferred (§6.5 — resolution happens before the buffer is rented, so a disabled +command drops nothing on the floor), but it does not rescue the move: `COMMAND INFO `, +`COMMAND DOCS` and `ACL` rules need a command *as an argument*, which is a hole by definition. + + +## 10. Open questions + +- **Should a `RedisChannel` fold into the same slot as keys?** The spike folds it unconditionally, which + suits sharded pub/sub (`SPUBLISH`) but is meaningless for plain `PUBLISH`, where the channel does not + route by slot. `RedisChannel` carries a `KeyRouted` option (`Subscription.cs:83`) that presumably ought + to gate it, and sharing one `_slot` field between keys and channels conflates two different things. + Visible in the worked example as `ChannelPrefix` reporting `slot=5631` for a plain `PUBLISH`. +- **`Raw` multi-arg and the bit cursor.** A fragment with `ArgCount > 1` advances `_argIndex` by its arg + count, which keeps the key-mark bitmap aligned — but `Raw` still cannot itself contain a key. Either + forbid that (rule 5) or have `Raw` carry its own bitmap to shift and OR in. +- **Single-arg vs multi-arg `Raw`.** Restricting `Raw` to exactly one bulk string keeps `*N` a + compile-time constant; allowing multi-arg costs runtime counting. Possibly two types. +- **A runtime-validating `Raw` factory** for fragments assembled once at startup from config — the one + legitimate case the literal-only rule closes off. +- **Public API commitment.** A public method taking the handler forces the handler type public, putting + every `AppendFormatted` overload into `PublicAPI.Unshipped.txt` permanently. Can the interpolated + surface start internal (RESPite-side, used by SE.Redis) to buy room to iterate? §9.4 argues the opposite + direction for the *command* surface - one member on `IDatabase`, everything else extension members - + so these want reconciling. +- **Should the handler be a `ref struct`?** It holds only a `byte[]`. Ref struct prevents capture, + copying and double-dispose, which is why it is right — but it also blocks `using var` + `ref` (§4) + and any async retention. +Added while building the cache (§6.6-6.9): + +- **Command metadata for cacheability.** Read-only and keyed, so the flag gates pass them today: + non-deterministic (`SRANDMEMBER`, `HRANDFIELD`, `ZRANDMEMBER`), cursor-based + (`SCAN`/`HSCAN`/`SSCAN`/`ZSCAN`), time-dependent (`TTL`, `PTTL`), side-effecting (`TOUCH`, `PFCOUNT`). + Wants a per-command fact beside the retry category in `CommandFlags.Category.cs`, *not* a `CommandFlags` + bit — see §6.9. `DUMP` was also proposed; I would challenge it, since it looks correctly invalidated, so + that is a benefit call rather than a safety one. +- ~~**Scripts (`EVAL_RO`/`EVALSHA_RO`) are unresolved.**~~ **Settled:** cacheable by default, caller opts + out with `NoClientCache`. `EVAL`/`EVALSHA` are excluded by default, `_RO` is server-enforced, + so the default applies to a narrow, self-declared population. The risk to document is *undeclared key + access*, not non-determinism — see §6.9. +- **Do module reads register for invalidation?** If the server tracks keys only for core command + dispatch, a keyed module read would be cached and never invalidated. Unresolved by the docs and worth + five minutes against a real server with a module loaded; it decides whether §6.9's opt-out story needs + a caveat for module authors. +- **Nothing turns tracking on.** There is no `CLIENT TRACKING` support, and the RESP3 `invalidate` push is + actively dropped — `PushKind` has no member for it, *and* `OnOutOfBand` requires the second element to + be an inline string, which an invalidate push's key array is not. Both need changing. RESP2 `REDIRECT` + already delivers invalidations via pub/sub today (`Issue2507`). +- **Replies are copied into the cache.** `RespPayload.Create` copies; the real executor should share the + reply frame's own lease via a reservation, as `RespResult` already does. +- **Running a `ResultProcessor` over a cached payload.** It takes `ref RespReader`, which + `RespPayload.GetReader()` supplies, but also wants a `PhysicalConnection` and `Message` for error + context — so it needs a synthetic context or a narrower interface (§6.8). +- **Bounding the cache.** Invalidated entries linger until `Sweep`, and the key table grows with distinct + keys seen. Both need a size bound; both fail closed, so bounding is safe (§6.6). + +- **Static key bitmaps.** For fixed-arity commands the key positions are statically known, so the JIT + may constant-fold the bitmap when the Append chain inlines. Not to be designed around, but the + structure permits it and an analyzer could emit the constant if it matters. + +--- + +## 11. Verification log + +Everything above marked "verified" was compiled and, where runtime behaviour was in question, run — +first in throwaway scratch projects multi-targeting `netstandard2.0;net472;net8.0` against the real +`src/RESPite` and `src/StackExchange.Redis`, and then in the in-repo spike of §9. + +Generated frames were validated by parsing them back with RESPite's own `RespReader`, including +`DemandEnd()` so the frame must be exactly consumed — over- and under-run both fail. + +Hash slots were checked against published `CLUSTER KEYSLOT` values: `foo`→12182, `bar`→5061, +`hello`→866, `somekey`→11058, `""`→0, all passing, with hash-tag handling matching +`ServerSelectionStrategy.GetClusterSlot` (first `{`, first `}` after it, non-empty between). + +**Not verified:** nothing was run against a live server — these are local bytes only, which is the +right contract given where this sits, but it means only *framing* is proven, not that any particular +argument combination is semantically acceptable to a server. `net461` was not compiled. +Micro-benchmark numbers in §5.2 are indicative order-of-magnitude only, not a controlled benchmark. diff --git a/design/interpolated-resp-writer.queue.md b/design/interpolated-resp-writer.queue.md new file mode 100644 index 000000000..db5fda6b8 --- /dev/null +++ b/design/interpolated-resp-writer.queue.md @@ -0,0 +1,3158 @@ +# Queue — interpolated RESP writer / client-side caching + +Working list for the `marc/interpolated-writer-design` branch. Kept separate from +`interpolated-resp-writer.md` so it can be edited without conflicting with the design notes, which are +long and appended to constantly. + +**Convention:** items move to Done with the commit that closed them. Anything removed rather than done gets +a line saying why, because "we decided not to" is worth as much as "we did". + +--- + +## Framing: this is V4, and the two implementations never run in parallel + +Decided 2026-09-15, and it changes how several items below should be read. The spike is not a parallel +alternative that might ship - it is the next major version's core, and the old implementation goes. + +Four consequences, none of them cosmetic: + +1. **The old `IDatabase` surface still exists; the old *implementation* does not.** Binary compatibility is + paramount here, so the signatures stay and are served by the new core, and people are *led* to the new + surface rather than pushed. That makes the internal `...Array` siblings **permanent fixtures**, not + scaffolding, and their doc comments now say so. + + **`Message` stays too**, and for a better reason than inertia: it is abstract over exactly two members, + `ArgCount` and `WriteImpl`. Everything else it carries - db, flags, command, slot, status, timeouts, + result pairing, high-integrity, profiling - is concrete shared bookkeeping. So the core is already + pluggable at precisely the rendering step, which is the only step the frame path wanted to replace, and + a new `IMessage` would only re-spell a seam that is already two members wide. What V4 changes is not the + abstraction but the population: 36 `Message` subclasses exist mainly to implement `WriteImpl` for one + command each, and every command that moves to the writer makes one of them redundant. The end state is a + **deletion**, not a reconciliation. The innards may evolve once sync is no longer a requirement; that is + deferred, and it is a smaller cut than it first looked. + + `ArgCount` shrinks with them. Its consumers are the `REDIS_MAX_ARGS` guard, composite forwarding, and + per-command arithmetic that lives in the subclasses being deleted - so it stops being something each + command must compute correctly and becomes a field the frame already holds, having counted while + writing. That is the `IRespArgument`-over-`RespFragment` argument again: a count taken during the write + cannot disagree with the bytes. + +2. **`Fallback()` must reach zero.** `TransitionalDatabase` currently delegates transactions to + `RedisDatabase`. With nothing to delegate to, `MULTI`/`WATCH`/`HIMPORT`/`EVALSHA` stop being acceptable + permanent residue and become **release blockers**. Deferring them is a temporary state with a mandatory + exit, not an end state. + +3. **The `Message` layer is load-bearing, and a second write path is not merely unattractive - it is + incoherent.** The backlog is one `ConcurrentQueue` per bridge, and `FrameMessage : Message`, so + frames *already* participate in ordering, backlog replay, retry accounting and the reconnect handshake. + Two owners of one socket's write path cannot share any of those. So "build a raw executor beside the + shim" is not an option at all, and the shim is not a hack to escape - it is what lets frames have those + properties for free. + + **Which means the vexing commands need composition, not a new path.** Both mechanisms already exist in + that layer: `IMultiMessage` (one logical message expanding into several, written as a unit - this is how + `TransactionMessage` does MULTI/EXEC today) and write-lock injection (a connection-local preamble decided + once the connection is known - this is how `HashImport` does PREPARE). What the frame surface lacks is a + frame-shaped participant in each. That is a far smaller and strictly additive piece of work than a raw + executor, and none of MOVED/ASK, backlog, timeouts, retry, profiling or high-integrity has to be + reimplemented, because we never leave the path that already has them. + +4. **`[Experimental]` is a staging label, not a hedge.** "We can change it later because it is + experimental" stops being true. Public shapes - lease returns, group names, `Keys` over `Keyspace` - + are effectively permanent from here, which raises the value of settling them now and lowers the value of + leaving options open. SER352's 354 members stop being a progress bar and become a release gate, which + is what the Release-build warning was asked for in the first place. + +--- + +## Now + +- [ ] **Where a cache hit's ~125ns goes — profiled 2026-09-18** (`CacheHitSendBenchmarks`, in-process, + zero allocation throughout). Measured **subtractively**: start from the full call and remove one + layer at a time, so every row is a complete operation whose result is consumed. + + | removed | ns | the layer costs | + |---|---|---| + | nothing (`db.Strings.GetAsync`) | 125.9 | — | + | the group accessor | 116.4 | **~9** | + | + the `ValueTask` | 117.8 | **~0** (inside noise) | + | + parsing the reply into `RedisValue` | 83.7 | **~33** | + | everything but render + hash + probe | 71.0 | **~13** serve-the-hit | + | *(render + hash + dictionary probe)* | **71.0** | **56% of the total** | + + **So the two places worth looking are the probe (56%) and the reply parse (26%)**, and the layers + people would suspect - the group sugar, the `ValueTask` - are 7% and nothing. Inside the probe, + `TryGet` takes an **interlocked** `TryRetain` and then re-checks validity afterwards, deliberately + (an invalidation between check and retain would let one stale read through); a lock-prefixed RMW is + typically 10-20ns, so that is plausibly a third of it, and it is there for correctness. + + **A build-up decomposition of this path does not work, and the way it fails is worth recording.** + Measuring the stages in isolation reported *render alone* at 39ns and *render + hashing 20 bytes* at + 30ns - impossible, reproducible three times. Isolating it: reading any field of the frame appeared to + cost 7.5ns while hashing appeared free. It is dead-code elimination - `Render` inlines, so the JIT + deletes whatever a stage does not consume. Only rows whose result is fully consumed mean anything + here. + +- [x] **The command groups no longer hand their context back — DONE, 2026-09-18.** Marc: *"I was + expecting all of those to become internal readonly fields"*. All fourteen - `RespStrings`, + `RespBitmaps`, `RespHashes`, `RespKeys`, `RespKeyspace`, `RespLists`, `RespSets`, `RespSortedSets`, + `RespStreams`, `RespScripts`, `RespArrays`, `RespGeospatial`, `RespHyperLogLog`, `RespVectorSets` - + swapped `private readonly RespContext _context` plus `public RespContext Context => _context` for a + single `internal readonly RespContext Context`. Fourteen lines out of the public API, and not one + command method changed: the field kept the name the methods already used. + + **Nothing outside the assembly wanted it.** Checked before removing: no test, toy or doc reaches for + `group.Context`, and `docs/Extending.md`'s level-3 sample uses the *extender's own* group type, whose + `Context` is theirs to make public. + + **On whether we need a way back.** Marc, thinking aloud, suggested the groups could implement + `IRespTarget` explicitly with a `GetContext() where T : IRespTarget` helper. Recommend not, unless + a need actually turns up: `IRespTarget.Context` is public, so implementing it would re-expose the + naked context with a cast in front of it - the same thing the typed contexts just stopped doing. If + a need does turn up, the consistent answer is the one already taken there, a + `public static explicit operator RespContext(RespStrings)`, which is additive and costs nothing now. + + `RespContextConversionTests.NoGroupExposesItsContext` sweeps every group by reflection rather than + naming them, because the failure that matters is a *new* group being added with the old public + property - which nothing at compile time would catch, and which would reopen the hole on one type + only. + +- [x] **`IRespTarget.Raw` is now `Context`, hidden by the derived interfaces — DONE, 2026-09-18.** + Marc: *"I really don't like the public Raw"*. Renaming the base member and letting + `IRespKeyspaceTarget`/`IRespServerTarget` hide it with `new` means `db.Context` is the typed context, + and the plain one only turns up for a caller who has deliberately taken an `IRespTarget`. + + **`RedisBase.GetContext()` is what made it tractable.** With the hiding, every implementer owes two + members that differ only in return type and neither can override the other; one `protected virtual` + builder is what both of them call, so a subclass says how its context is made once and the two + cannot drift. Every implementer took the same shape - `RedisDatabase`, `RedisServer`, `KeyPrefixed`, + `TransitionalDatabase`, `MultiGroupDatabase`, `RetryDatabase`/`RetryTransaction`, the subscribers. + + The messy half Marc predicted - internal code doing `.Raw.Something()` on what it wraps - is an + **internal** extension property `Raw` on `IRespTarget` that is literally `=> target.Context`. + Internal rather than public on purpose: making it public would put `Raw` back on every target and on + the typed contexts, which is the thing being removed. + + Earlier the same day, and what led here: the typed contexts' own `Raw` became an internal field plus + an **explicit** conversion operator. `RespContextConversionTests` pins both halves - explicit rather + than implicit, and the `new` hiding - because an implicit conversion or a lost `new` would still + compile every call site that exists today. + +- [ ] **Streams, batch 1 of 4 — DONE, 2026-09-18: 106 -> 80.** Marc: *"let's try to get streams done"*, + and *"some of the overloads may be reducable on a fresh clean API"*. The scalar-reply commands are + moved: `XADD`, `XNACK`, `XACKDEL`, `XCFGSET`. + + **Eight `StreamAdd` overloads became two.** `StreamAddOptions` already holds every setting the + shipped overloads spell out positionally, so the new surface takes it and the rest is the adapter's + problem. `LegacyStreamAddOptions` is shared with the classic path rather than copied - it + deliberately does not validate, and only the options-carrying overloads call `ThrowIfInvalid`, an + asymmetry that would have been easy to lose in a reimplementation. + + Likewise `XACKDEL` has only the span form here, where `IDatabase` also has a single-id one: the + server is told `IDS 1` either way, so the adapter passes a one-element span. + + `NameValueEntry` now implements `IRespArgument` (explicitly, so no public API line), which is what + lets `$"{RedisCommand.XADD}{key}{fields}"` unroll a whole run without a loop at the call site. + + **`RespSurfaceStreamsParityTests` is the real check.** Asserting strings I typed would only prove I + read the old code the way I wrote the new code; instead it drives the shipped `Message` builders + through `MessageWriter` and compares against the interpolated writer, over all 13 `XADD` option + combinations plus every `XNACK`/`XACKDEL`/`XCFGSET` shape. 39 cases, byte-identical. Three classic + builders went from `private` to `internal` to make that possible, which is a fair price. + + The first version of that file reported "requires a scalar element" rather than a diff, because the + fake replied `*0` to everything and the parse failed before the comparison - so each command now + gets a reply its handler accepts. + + Remaining: `XCLAIM`/`XAUTOCLAIM`, `XPENDING` x2, `XREAD`/`XREADGROUP`, `XINFO` x3. + +- [x] **BUG (shipped): `XREADGROUP CLAIM` sent a non-integer for a fractional `TimeSpan` — FIXED, 2026-09-18.** Found while + moving the stream reads, 2026-09-18; Marc asked for it to be logged rather than folded into that work. + + Both writers - `MultiStreamReadGroupCommandMessage` (`RedisDatabase.cs:4567`) and + `SingleStreamReadGroupCommandMessage` (`:5386`) - do + `writer.WriteBulkString(claimMinIdleTime.Value.TotalMilliseconds)`. `TotalMilliseconds` is a + **double**, so: + + - `TimeSpan.FromMilliseconds(5000)` renders `CLAIM 5000` - fine, which is why nothing has noticed; + - `TimeSpan.FromMilliseconds(1500.5)` renders `CLAIM 1500.5`, and the server wants an integer there, + so it answers `ERR value is not an integer or out of range`. + + `XCLAIM`/`XAUTOCLAIM` are unaffected: their `IDatabase` signatures take `long minIdleTimeInMs`, so no + double is ever involved. It is specific to `XREADGROUP`, where the parameter is a `TimeSpan`, and any + caller passing sub-millisecond precision - `TimeSpan.FromSeconds(1.0005)`, say - hits it. + + Both paths now truncate, and `RespSurfaceStreamsParityTests.ReadGroupClaimIsWholeMilliseconds` + asserts both that they agree and that what they agree on is sendable. + + **The real fix is the hole, not the cast.** Marc: *"we should be able to use something like + `{minIdleTime:ms}` - if we can't, we've screwed up somewhere"*. We could not: `TimeSpan` is a BCL + type and can never implement `IRespArgument`, and nothing implemented `IRespFormattableArgument` + either, so every site was writing `(long)x.TotalMilliseconds` by hand - four of them on the new + surface - and the shipped writers were passing the `double` straight through. + `RespRequestBuilder.AppendFormatted(TimeSpan, string?)` now takes the unit, with a `TimeSpan?` twin + that writes nothing when null. + + Two properties fall out. A unit-less `$"{ttl}"` **does not compile** - CS0315, no conversion to + `IRespArgument` - so a duration cannot reach the wire without someone naming the unit. And an + unrecognised unit throws: they are our tokens, not `TimeSpan`'s, and a duration in the wrong unit is + still a *valid* command, so the call site is the only place it can be caught. + + Not affected, and worth writing down because it is the same question asked of the commands that + really do offer both: `EX`/`PX` down-levelling already happens, inside `Expiration`, which truncates + to whole milliseconds and then drops to seconds when `millis % 1000 == 0`. Every TTL parameter on + the new surface is an `Expiration` rather than a `TimeSpan`, so that choice is made by the type that + writes the operand instead of at each call site - which is why there is no `{x:s}` anywhere in + `src`. The `{x:ms}` holes are all commands whose unit the protocol fixes. + +- [ ] **BUG (shipped): multi-stream `XREAD` accepts `StreamPosition.NewMessages`; single-stream refuses + it.** Found 2026-09-18; Marc: *"log that, we should come back to it"*. + + `StreamPosition.Resolve` throws for `NewMessages` under `RedisCommand.XREAD` - correct, since `$` + means "entries added after this call blocks" and `IDatabase.StreamRead` does not block - and maps it + to `>` under `XREADGROUP`. But `MultiStreamReadCommandMessage.WriteImpl` resolves its positions with + `StreamPosition.Resolve(..., RedisCommand.XREADGROUP)` while the command being written is `XREAD`. So + the single-key overload throws and the multi-key one sends `>` to a command that has no consumer + group, which the server then rejects. + + Reproduced as-is by the new surface, and both halves are pinned by + `RespSurfaceStreamsParityTests.ReadRefusesNewMessagesButReadGroupDoesNot`, so whichever way this is + settled the test says what changed. + +- [ ] **A commit on this branch deleted 77 files that the main merge had just brought in.** Found + 2026-09-18 while looking for `RedisValue.EqualityComparer`, which Marc expected to be here. + + `8648b429` - *"Merge main (3.3.0) into the V4 spike"* - brought main in correctly; the file was + present at that commit. The **next** commit, `8c8863f0` *"Review feedback: IRespArgument removes the + bespoke execute path"*, is single-parent, changed 176 files **+2,142 / -18,388**, and deleted 77 of + them under a message about something else. It reads as a bad conflict resolution rather than intent. + + Lost: `RedisValue.EqualityComparer.cs` (#3230), `Maintenance/` x7 (#3191 - *main's own tip*), + `Configuration/RedisCloud`+`RedisEnterprise` providers, the whole `StackExchange.Redis.FaultInjector.Tests` + project (32 files), `toys/MaintenanceSoak` and `MaintenanceWatch`, `eng/public-api.py`, + `docs/exp/SER013.md`. + + **Merging main does not fix it**, and this is worth knowing before anyone tries: `origin/main` + (`dc915bbc`) is an **ancestor** of this branch - 0 commits on main we lack, 336 of ours it lacks - so + `git merge origin/main` reports "Already up to date". The content has to be restored from + `8648b429`, which is the post-merge state including whatever conflict resolution was already done: + + ``` + git checkout 8648b429 -- $(git diff --diff-filter=D --name-only 8648b429 HEAD) + ``` + + **A false alarm to record too**, because it cost time: a text diff of `PublicAPI.Shipped.txt` against + main showed "328 entries missing", which is wrong. Our side carries 247 lines main lacks, reading + `[SER007]abstract StackExchange.Redis.Availability...` - the `Availability`/`Maintenance` API is not + gone, it has been re-annotated as experimental, which changes the line text. Compare annotated API + files by symbol, not by line. + +- [x] **`RedisValue.EqualityComparer` is back — DONE, 2026-09-18.** Marc: *"if you can make it available + without breaking main: we should do that - merge hell"*. Restored surgically rather than by taking + all 77 files back: the file itself (**byte-identical to main**, so there is nothing to reconcile + later), the one-line coupling it needs - `RedisValue` becomes `partial` - its five + `PublicAPI.Shipped.txt` entries inserted at main's own position so the diff is five lines rather than + a reordering, and its tests and benchmark. `System.IO.Hashing` was still referenced, so nothing else + was needed. + + It matters beyond tidiness: `EqualityComparer.Binary` hashes with seeded **XxHash3**, where the cache + key's current hash is a serial DJB-style loop over 8-byte chunks with a carried accumulator - about + 36 dependent iterations for a 291-byte frame. The probe is **31%** of the cache-hit path and grows + with key size (32ns at 8 bytes, 61ns at 256), which makes it a five-times better target than the 6% + rent the stackalloc idea was aiming at. Measure before changing the cache's hash, though: the frame + hash is also the identity used for equality, so it is correctness-sensitive. + +- [x] **XxHash3 for the cache key: measured, and NOT worth doing — 2026-09-18.** Marc asked for early + benchmarks before committing to it: the cache key could use `XxHash3.HashToUInt64` directly - one + static, no comparer needed - folded 64->32. `FrameHashBenchmarks` compares it against the current + hash over the frame sizes actually observed (a GET frame is about `25 + key`): + + | frame | current | XxHash3 seeded | XxHash3 unseeded | + |---|---|---|---| + | 41 | **2.30** | 4.15 | 4.14 | + | 98 | **6.09** | 6.84 | 6.81 | + | 130 | 8.17 | 7.55 | **7.52** | + | 163 | 10.92 | 9.11 | **9.15** | + | 291 | 21.71 | 20.60 | **12.09** | + + **The current hash wins below ~130 bytes** - XxHash3 carries a fixed setup cost a short frame never + amortises - and the crossover is around a 100-byte key. Past it the seeded saving is 1-2ns against a + 158-176ns cache hit: under 1%. A straight swap is a regression for typical keys. + + **Seeding is not free, and its cost depends on length.** At 291 bytes seeded is 20.6ns where unseeded + is 12.1ns. XXH3 re-derives its 192-byte secret from a custom seed on every call for long inputs, + where the unseeded path uses the precomputed default. Worth knowing before seeding a hash on any hot + path. + + **And hashing is a smaller part of the probe than earlier notes here implied** - correcting that: at + an 8-byte key the probe is 32ns of which the hash is 2.3ns (~7%); at 256 bytes it is 61ns of 21.7ns + (~36%). Eliminating hashing outright caps at ~12% of a hit, and only for large keys. The probe's cost + is mostly the dictionary lookup, `SequenceEqual`, and the interlocked `TryRetain`/re-check - which is + where to look if the 31% is worth attacking. + +- [ ] **Hot-path measurement before the stackalloc/alt-lookup work — 2026-09-18.** Marc: *"definitely + measure first... what the overhead of the lease rent and return is"*. `CacheHitSendBenchmarks` is now + parameterised by key size, because the answer depends on it entirely. + + **The initial rent does not depend on the key.** It is `HeaderMax + 64 + literalLength + + (formattedCount * 24)` - an estimate that counts *holes, not content* - which for `$"{GET}{key}"` is + 126 bytes whatever the key is. A GET frame needs about `25 + key`, so: + + | key bytes | frame needs | vs the 126-byte rent | + |---|---|---| + | 8 | 41 | fits | + | 64 | 98 | fits | + | 96 | 130 | **grows** | + | 128 | 163 | **grows** | + | 256 | 291 | **grows** | + + So at the sizes Marc called the common hot path - 128, 256 - **every cache hit already does two + rents, a copy and two returns**. + + **Attribution at a 128-byte key** (short job, in-process; full hit = 158.6ns): + + | | ns | share | + |---|---|---| + | render (incl. rent+grow) | 59 | 37% | + | cache probe | 49 | 31% | + | reply parse | 37 | 23% | + | group layer | 7 | 4% | + | ValueTask | ~0 | ~0 | + + **Rent+return in isolation is 7.5ns, flat across key size** - about 6% of the hit path, roughly double + where it grows. That is the honest ceiling on what deferring the buffer can save, and it is a lot + less than the 56% "render+hash+probe" bucket recorded earlier, which was three things in a trenchcoat. + + **The cheap experiment beat the expensive plan.** Widening the slack from 64 to 192 - one character - + took the 128-byte case from 158.6ns to **143.7ns (-9.4%)**, render -15%. At 256 it changed nothing + (still grows: needs 291). At 8 and 64 bytes it was within noise. Reverted, not committed: it trades a + 256-byte bucket for every command against a 128-byte one, which is a pool-footprint decision that + wants its own measurement. + + **What this means for the idea.** The stackalloc is *more* attractive than the 6% suggests, because a + 256-byte stack buffer skips **both** rents and the copy for the common case, not just the first rent. + But the alternate-lookup half does not reduce the probe (49ns, 31%): hashing and `SequenceEqual` over + the frame bytes is inherent, and grows with key size (32ns at 8 bytes, 61ns at 256). What it buys is + the ability to probe *without materialising a frame at all* - which is only reachable if the + stackalloc render exists. The two are coupled; neither pays alone. + + Order suggested: settle the capacity estimate first (one line, measurable, no TFM gating), then + re-measure, then decide whether the remaining rent justifies the ref-struct surgery. + +- [x] **Scans: the dual API, DONE for every cursor scan — 2026-09-18.** Marc's shape: a *raw* value-task cursor + API returning a lease, and a utility `IAsyncEnumerable` **on top of** it rather than beside it, so + the cursor loop exists once. Done for sets; hashes (with and without values) and sorted sets are the + same shape with a different element projection, and `VectorSetRangeEnumerate` is not a cursor scan + at all. + + - `RespScanPage` - cursor plus `ReadOnlyLease`, disposable. `IsComplete` is `Cursor == 0` + and is documented as *not* "the page was empty", because that is the scan bug everyone writes once. + - `ScanPageAsync` - one page, for a caller who wants to checkpoint or bound work per tick. + - `ScanAsync` - the sequence, and an `IScanningCursor`, so an interrupted scan reports where it + reached and a later one resumes. `Cursor` is the **active** page's, as the shipped interface + specifies, not the pending one. + - All four: `SSCAN`, `HSCAN`, `HSCAN NOVALUES`, `ZSCAN`. The pair-shaped replies (`HSCAN`, `ZSCAN`) + reuse the same `ValuePairInterleavedProcessorBase` shapes the classic path uses, which is also what + copes with RESP3 turning some of those replies jagged. + + **The enumerator is hand-written, and that is forced.** It has to implement `IScanningCursor`, and a + compiler-generated async iterator cannot implement an interface of ours - which also means + `[EnumeratorCancellation]` would be a no-op here (CS8424) and is absent. The token still arrives, + through the interface method, which is what `WithCancellation` passes anyway; the attribute only ever + automated getting it somewhere this code already is. + + **Two tokens, combined once.** The enumerator's and the one given to `ScanAsync`. If they are the + same token - which happens the moment anyone writes `ScanAsync(key, cancellationToken: ct) + .WithCancellation(ct)` - they are used directly rather than linked: `CreateLinkedTokenSource` + allocates a source and registers on *each* side, so one cancellation walks the chain twice for + nothing. `CancellationToken` compares by its underlying source, so the check is exact, and it catches + both-`None` for free. + + **Cancellation lands between pages, and the reason is worth knowing.** `RespExecutor` does not ignore + a live token - it *throws* `NotImplementedException`, because the pipeline cannot cancel a request in + flight. So the token is checked before each fetch and `default` is passed to the send. For a scan + that is the granularity that matters: what a caller stops is the loop, and one page is bounded work. + The delegate still carries the token, so this becomes a one-word change when the pipeline can honour + one. Found by a test, not by reading: the first version propagated the token and every cancellation + test failed with `NotImplementedException`. + + **The shipped contract is stronger than it looks, and a test caught me assuming otherwise.** + `HashTests.ScanAsync` enumerates `HashScan` as an `IAsyncEnumerable` *and* `HashScanAsync` as an + `IEnumerable` - so which method produced a scan cannot decide which interfaces work. My first + design made the sync face optional, and those inherited tests failed with "this scan is asynchronous + only". `RespScanEnumerable` now carries a fetcher for **each** face, and the sync one is a real + synchronous `Send` rather than a blocking wait on the async one - so offering both is honest rather + than sync-over-async. The transitional adapter hands the one object to both members of each pair, + exactly as `CursorEnumerable` always has. + + `VectorSetRangeEnumerate` stays deferred, and is the only member of the family left. It is not a + cursor scan: it is keyset pagination over `VRANGE`, and the shipped code says it avoids "scan" naming + "in case a VSCAN command is added later". It is in this family only because it happens to return + `IEnumerable` and so falls in the same skipped bucket. + + Also hoisted `OptionalValue` into `Protocol/` while doing this - `SSCAN`'s `MATCH` had the same + `$0`-instead-of-omitted bug `XPENDING`'s consumer had, caught the same way, and writing it a third + time was clearly next. + +- [ ] **PROPOSAL: `RespContext` should be a sealed class, memoising the hot pieces.** Marc, 2026-09-18: + *"all our flavoured contexts are really just a reference with an accent"*. + + Today it is a 40-byte `readonly struct` with six fields - `_services`, `_commandMap`, `_keyPrefix`, + `Executor`, `ServerType`, `_database` - and two of the things the send path needs most are **not** + fields at all: `Cache` and `MaxCacheAgeTicks` are `TryGetService` lookups, consulted on *every* send. + A lookup walks a `ServiceLink` chain calling `Type.IsInstanceOfType` per node - a reflection type + test, not an `is T` the JIT turns into a cast check - and a database context built by the multiplexer + has three services in that chain. `Database` is likewise `Executor?.Database ?? _database`, an + interface call per read. + + **The design case is strong on its own**: `??=` replaces the `_haveContext` bool in `RedisDatabase` + and `RedisServer`; the fourteen group structs shrink from 40 bytes to 8, which matters because they + are copied on every `this in` extension call; and `KeyPrefixed.GetContext()` - which today rebuilds + `Inner.Raw.AppendKeyPrefix(Prefix)` on *every access* - becomes memoisable. + + **Costs to weigh**: every wither allocates, so `KeyPrefixed` must memoise rather than rebuild or it + regresses from free to an allocation per call; `new RespContext()` is a valid sentinel today and used + widely in tests, so a class needs a `Default` or null discipline; and identity/equality semantics + change. + + **I could not measure the service-lookup cost and should not pretend otherwise.** Two microbenchmarks + gave sub-nanosecond figures for a chain of reflection type tests - about one and a half cycles, so + not a measurement of anything - and the second *inverted*, reporting three services faster than none. + Defeating the hoist with an indexed array did not fix it. This is the third time today a + microbenchmark of a piece of this path has misled (the `in`/ref-accessor experiment measured 7.7x + faster in isolation and 8% *slower* in the real path). The only trustworthy route is to prototype the + memoisation and re-run `CacheHitSendBenchmarks`, which measures a whole operation. + +- [ ] **`TransitionalDatabase`: 71 unimplemented members — status, 2026-09-18.** Was 106 generated at the + start of the day, now **58** generated plus **13 hand-written scans that SER352 cannot see**. Marc + spotted the gap: *"make sure we add scans to the list, because I think we're cheating on that"*. + + **Why the scans are invisible.** `[AutoDatabase]`'s `SkipMethod` drops every `IEnumerable` and + `IAsyncEnumerable` member, because deferred execution does not fit capture-and-replay. So they + are written by hand in `TransitionalDatabase.Scans.cs`, where they forward to the test fallback and + throw without one - exactly like a generated stub. The generator has no way to tell a real + implementation from a forwarding throw; to it they are simply "declared". **So every SER352 number + quoted in this file has been short by 13.** + + Fixing the generator is not on: it cannot know. `TransitionalScanGapTests` is the tripwire in the one + place that can see it - it sweeps the enumerable-returning members by reflection, asserts each still + throws, and pins the count, so implementing one forces the number to be updated. + + | family | members | what is left | + |---|---|---| + | Streams | 18 | multi-stream `XREAD`/`XREADGROUP`, and `XINFO` STREAM/GROUPS/CONSUMERS | + | **Scans** | **13** | `HashScan` x3, `SetScan` x3, `SortedSetScan` x3, `HashScanNoValues` x2, `VectorSetRangeEnumerate` x2 - *uncounted by SER352* | + | Scripts | 12 | `ScriptEvaluate`, `ScriptEvaluateReadOnly` | + | Locks | 8 | `LockTake`, `LockRelease`, `LockQuery`, `LockExtend` | + | Execute | 4 | the legacy `Execute(string, object[])` pair | + | Keys | 4 | `KeyMigrate`, `KeyRestore` | + | one-offs | 12 | `ArrayGrep`, `DebugObject`, `HashImport`, `Ping`, `Publish`, `StringGetWithExpiry` | + + The scans are also the only family that needs a **shape** decision rather than a translation: the old + surface returns `IEnumerable`/`IAsyncEnumerable` over a cursor, and what the context surface + should answer - the same, or something that owns its pages the way `ReadOnlyLease` does - has not + been settled. That is the reason they were deferred, and it has not gone away. + +- [x] **The `Interpolated/` folder is gone — DONE, 2026-09-18.** Marc: *"we shouldn't have anything left + in there by the end of this"*. The namespace went several commits ago; the folder name was the last + thing still claiming this is a spike about interpolated strings. Pure file moves - every file was + already in the right namespace - into `Contexts/` (the contexts, `RespSurface`, `RespExecutor`, + `RespConnectionExtensions`, `IRespServerFeatures`), `Protocol/` (`RespLiterals`, + `IRespPreambleGate`, `ScriptLoadGate`), `Caching/` (`RespKeyTable` - invalidation is its only job) + and `Transitional/` (`RespMessageExecutor` and the 17 `TransitionalDatabase` parts). + + `Contexts/` rather than the project root, so the new surface reads as siblings - `Contexts/`, + `Groups/`, `Protocol/`, `Caching/`, `Downlevel/`. Easy to flatten later if the root is preferred. + +- [x] **The SER309 code fix had been dead for months — FIXED, 2026-09-18.** Found by grepping for + surviving `Interpolated` references. `RespLiteralCodeFixProvider` resolves three types by metadata + name and all three still said `StackExchange.Redis.Interpolated.*`, so every lookup returned null + and the fix silently offered nothing. Exactly the failure mode `RespInterpolationAnalyzer`'s own + comment describes - *"rename the type and this analyzer stops reporting rather than stops + compiling"* - and that comment claimed `RespInterpolationAnalyzerTests` pinned the names. **No such + test has ever existed.** + + **The `Build.Tests` "18 pre-existing failures" were this, not a separate problem.** All 18 were + SER309/SER309CodeFix: 8 because the fixtures' sample code called `RespContext.Raw`, which the + context refactor removed (a context *is* the raw thing now), and 10 because the fixtures declare + `[Resp] RespFragment` members and only imported `StackExchange.Redis`. Repointed at the public + `RespContext.Render(string, ref RespRequestBuilder)` and given the `.Protocol` using: **174/174 + green.** + + **And CI never ran them.** It runs `StackExchange.Redis.Tests` only - `RESPite.Tests` and + `Build.Tests` are built by `Build.csproj` and then never executed, which is the whole reason this + could rot. Both are now steps in `.github/actions/run-tests`; together they take about four seconds + and neither needs a server. + +- [x] **`RespCommand`, `RespCommands`, `RespHandlers` and `IRespHandler` moved to + `StackExchange.Redis.Protocol` — DONE, 2026-09-18.** Marc asked whether those, plus `RespExecutor`, + `RespContext` and `IRespTarget`, belong in `.Protocol`. The answer split rather than being uniform. + + **Moved**, because they are named only when writing a command: `RespCommand`/`RespCommands` - + `"SUBSTR".Command(preform: true)` is request-building, which is exactly what `docs/Extending.md` + already defines `.Protocol` as - and `RespHandlers`/`IRespHandler`, which are reply-parsing and + only named for a shape the defaults do not cover. The latter also fixes a real inconsistency: + `RespReplyHandler` was already in `.Protocol` *implementing an interface in the root*. + + **Stayed**, because they are the primary API: `RespContext`, `IRespTarget` and kin (Marc's own + earlier ruling - the context pieces are the root), and `RespExecutor`, because `SendAsync` is the + verb on the primary type and is level 2 in the docs. `db.SendAsync($"...")` names no + protocol type at all, since the interpolated string is *lowered* into a builder rather than written + as one - so the ad-hoc path still needs no extra `using`, which is the whole point of level 2. + + Six files needed the new using; everything else already had it. `docs/Extending.md` had gone stale + in the doing: its level-3 sample uses `RespCommand`, so the two-using preamble no longer compiled, + and the prose still said the `.Protocol` import "shows up later". + +- [x] **The script cache is seeded from the multiplexer — DONE, 2026-09-18.** Marc asked what the + logical scope is, and whether the endpoint or bridge is the place to hook it. + + **It is the multiplexer, and the endpoint would be wrong.** An entry is the bytes of + `SCRIPT LOAD ` plus the script's SHA, which are pure functions of the script text and the + `CommandMap`. The map is fixed for a multiplexer's life and cannot be changed from outside it - only + the internal `RespContext(CommandMap?, ...)` constructor takes one, and only the parameterless + constructor is public. So every endpoint of one multiplexer renders byte-identical entries: an + endpoint- or bridge-scoped cache would be N copies of the same arrays and N renders of the same + script, buying nothing. + + **The per-endpoint half already exists, and is already in the right place.** Whether a given server + holds a script is `ServerEndPoint.IsScriptLoaded`, consulted at write time by `ScriptLoadGate` and + reset by `FlushScriptCache` and the `RunId` check. That is the only part that can go stale, which is + exactly why this part never invalidates - the type's own remarks said so, and they are what settles + the scope question. + + So: `ConnectionMultiplexer.ScriptCache`, alongside `ClientCache` and for the same stated reason, + attached in `RedisDatabase.Raw` and `RedisServer.Raw`. Unconditional rather than config-gated, + because an entry cannot be wrong and an unused one is an empty dictionary; + `CommandFlags.NoScriptCache` is what keeps generated-per-call scripts out. + + `RespScriptCacheWiringTests` pins presence *and* scope - shared across databases and the server + context, not shared across multiplexers - because presence alone would not have caught the original + gap either. + +- [x] **`RespScriptCache` is internal — DONE, 2026-09-18.** Marc: *"internal, I think"*. The type, plus + `RespContext.ScriptCache` and `WithScriptCache` on all three contexts: nine lines out of + `PublicAPI.Unshipped.txt`. + + **This is safe because the absent case was already the designed one.** `Scripts.Methods` reads + `context.ScriptCache` and, when it is null, renders the `SCRIPT LOAD` preamble afresh - its own + comment calls that "correct and wasteful". So an external caller loses an optimisation, not a + behaviour. + + **It also exposed that nothing ever wired one up** - the only writer was that `WithScriptCache`, so + every real application took the wasteful branch. Now seeded from the multiplexer; see below. + +- [x] **Caching into `StackExchange.Redis.Caching`, and `CacheTrackingMode.Default` — DONE, 2026-09-18.** + Six files moved out of the root: `CacheOptions`, `CachePolicy`, `CacheTrackingMode`, + `RespScriptCache` (public) and `RespClientCache`, `RespCacheConnectionExtensions` (already internal). + `.Caching` rather than `.Cache` because a namespace is a category, not a thing - and it leaves the + name `Cache` free. + + `CacheTrackingMode` gained `Default = 0`, pushing `Broadcast` to 1 and `PerKey` to 2. Zero was + `Broadcast`, which is the thing this type's own remarks argue against: it says an unset value is a + considered choice, and makes "asked for broadcasting" indistinguishable from "never thought about + it", so the library could never change its mind without silently overriding the first caller. One + internal `CacheOptions.ResolvedTrackingMode` maps `Default` to the answer; both consumers read that + rather than comparing against `Broadcast`, so the choice lives in exactly one place. + + Two stale docs fell out: `Broadcast` claimed to be "The default" while `PerKey`'s summary opened + "Default mode:" (it meant *Redis's* default) - directly contradictory, sitting three lines apart. + +- [x] **`RespSurface.Downlevel.cs` was not part of `RespSurface` — FIXED, 2026-09-18.** It holds + `RespGroups`, in namespace `StackExchange.Redis.Downlevel`, and the csproj nested it under + `RespSurface.cs` in the IDE via `Interpolated\RespSurface.*.cs`. Now `Downlevel/RespGroups.cs` with + that `DependentUpon` line deleted (it matched nothing else). Its remarks still told down-level + consumers to import `StackExchange.Redis.Interpolated`, which has not existed for several commits. + + Kept the class name `RespGroups` rather than Marc's suggested `DownlevelExtensions`: the namespace + already says "downlevel", so `Downlevel.RespGroups` says what it holds where + `Downlevel.DownlevelExtensions` repeats itself. Trivial to overrule. + +- [x] **Throw helpers on `RespRequestBuilder` — DONE, 2026-09-18.** Marc asked for the house shape: + a method-local `static Throw()` marked `[MethodImpl(NoInlining)]`, with the guard itself inlined. + Seven guards moved. Measured, Release net10.0, IL bytes: `DemandCommand` 20 -> 15, `Complete` + 219 -> 169, `AppendFormatted(RespCommand)` 214 -> 204, `AppendFormatted(RedisCommand)` 111 -> 106, + `AppendFormatted(ReadOnlySpan)` 81 -> 76. + + **Two of my claims did not survive measurement.** I said the in-loop null check had "the most to + gain"; it was exactly IL-neutral, because passing `nameof(value)` leaves the `ldstr` at the call + site. Making the helper parameterless recovered the 5 bytes. And `Complete` held a verbatim copy of + `ThrowTooManyArguments`'s four lines, which is most of its -50. + + **Should the helper `Dispose()` too?** Marc asked; yes, and not theoretically. The + `(literalLength, formattedCount, context)` constructor rents the buffer and *then* sets + `_hasCommand = false`, so `$"{key}{value}"` with no command reaches `DemandCommand` with a live + rented array - and no `finally` is generated around an interpolated string, so nothing else ever + returns it. `InterpolatedThrowHelperTests` pins all five throwing guards by priming the pool's + per-thread slot and checking the array comes back, with a negative control so the harness cannot + pass vacuously. + + **Local functions can still reach `Dispose`.** My first pass made them private instance methods for + exactly that reason; Marc pointed out that a `static` local can take `scoped ref RespRequestBuilder + @this` - the same `ref this` the `IRespArgument` calls already pass - which restores the house idiom + *and* recovers `nameof` on the enclosing parameters. Measured identical IL at every call site, so it + is free. The two helpers with more than one caller stay methods. + +- [x] **`default(RespCommand)` framed a command called `NONE` — FIXED, 2026-09-18.** Found by the throw + helper tests, not by looking for it. `IsEmpty` tested `_command == RedisCommand.UNKNOWN`, which no + constructible value satisfies - both `UNKNOWN` constructors set `_resp` or `_name` - and which + `default` does not satisfy either, because an unassigned `RedisCommand` is `NONE`, which is 0. + + So the property was `false` for the only empty value there is, the guard in + `AppendFormatted(RespCommand)` never fired, and `$"{default(RespCommand)}{key}"` rendered + `*2|$4|NONE|$1|k|` - a command literally named `NONE`, sent to the server, which answers with an + unknown-command error pointing nowhere near the cause. `RespCommandTests` now pins both halves. + +- [x] **A bare `string` in a hole now binds — DONE, 2026-09-18.** Found while writing the test above: + `$"{cmd}{key}{"x"}"` did not compile. `RedisKey`, `RedisValue` and `RedisChannel` all convert + implicitly from `string` and none is better, so it was CS0121 naming two of the three arbitrarily. + Marc: *"maybe we should just add an explicit string overload"* / *"to be clear: we should assume + non-key"*. + + `AppendFormatted(string? value)` is an exact match, and a standard conversion beats every + user-defined one, so it wins outright - `OverloadResolutionPriority` was considered and is not + needed. It forwards to the `RedisValue` overload, so a string is a plain argument: no key prefix, + no slot, no invalidation. A key living in a `string` must still be cast. + + `docs/Extending.md` already *asserted* this behaviour ("the trap to know about: `$"{someString}"` + binds to the **`RedisValue`** overload") while the code refused to compile it - the doc described + the design and the implementation had not caught up. `InterpolatedStringHoleTests` pins all four + facts now, null included (`$0`, an empty argument, not an absent one). + +- [x] **The `ReadOnlyMemory` execute is gone from the public surface — DONE, 2026-09-18.** + Marc: *"this should not exist - that signature is only needed for the old code; our new Execute API + will use the RespRequestBuilder"*. `RespContext.ExecuteAsync(string, ReadOnlyMemory, + CommandFlags)` is now `internal`, and `RespSurface.ExecuteAsync(this IRespTarget, ...)` is deleted + outright - two lines out of `PublicAPI.Unshipped.txt`. The only callers that need the collection + shape are `IDatabase.ExecuteResp`/`ExecuteRespAsync`, which live inside the assembly. + + The new ad-hoc path is the interpolated builder, and `RespAdHocExecuteTests. + TheInterpolatedFormRendersTheSameRequest` pins that it is not a downgrade: the collection form and + `$"{SomeCommand}{key}{(RedisValue)"x"}"` render byte-identical requests and mark the same key. So the + removal takes away the allocation and the wrapping, not a capability. `docs/Extending.md` level 2 was + rewritten to match - it described the removed method. + +- [x] **`RespContext.Render(string, ReadOnlySpan)` no longer hand-rolls the loop — + DONE, 2026-09-18.** It `foreach`ed and branched on `arg.IsKey`, which was the key/value decision + made in a second place. Now that `RedisKeyOrValue` implements `IRespArgument`, the whole body is + `handler.AppendFormatted(args)` - the existing `AppendFormatted(ReadOnlySpan) where T : + IRespArgument` does the unroll and asks each argument to write itself. Marc spotted it; the loop + predates the interface. + +- [x] **Typed contexts — GREEN, 2026-09-18.** Build and tests pass in Debug and Release; + 7,669 + 1,886 tests, no failures. + + **The shape.** A context carries what can differ from upstream - key prefix, database, services - + so the groups hang off the *context* (`extension(in RespDatabaseContext context)`) and a target + forwards to its own (`db.Strings => db.Context.Strings`). `IRespTarget.Context` became `Raw`; + `IRespKeyspaceTarget.Context` is a `RespDatabaseContext`, `IRespServerTarget.Context` a + `RespServerContext`, each derived from `Raw` by its implementor. The typed contexts implement + `IRespTarget` and nothing narrower: they do carry shared state, so the ad-hoc escape hatch reaches + them, but they are not keyspace targets, which is what stops a context being its own `Context`. + + **What the review changed.** `RedisKeyOrValue : IRespArgument`, so a span of them writes through + the ordinary interpolated form and the bespoke execute path is gone from the public surface. + `Render` came off the typed contexts - it returns a frame, not a context, so there is no chain to + preserve and it is plumbing: `Raw.Render`. Marc's catches, both right. + + **Two deliberately left.** `Database` on `RespDatabaseContext` is still a property reading through + to the shared state rather than a field; doing it properly means the frame carries the database, + which then deletes `IRespExecutor.Database` instead of syncing two copies - flagged for eyeballs + before building. And whether `Raw` needs a public name at all depends on demoting `RespContext` to + embedded shared state, which is the next question rather than this one. + + **A note on method, because it cost hours.** The test pass went 232 -> 0 by fixing *helper + signatures and declarations*, not call sites: retype the thing that produces a context and the + hundreds of `ctx.Strings.…` lines below it never move. Every blanket regex over call sites made it + worse and had to be undone - twice, once turning ten files of legacy `IDatabase.ExecuteAsync` into + `db.Raw.ExecuteAsync`. Fixing precisely the positions the compiler reports is reliable; guessing at + receivers by name is not. + +- [ ] **Nothing should extend a naked `RespContext` - HALF DONE 2026-09-17.** Marc: *"the .Lists etc + extension properties (and fallback methods) should be against the semantic-carrying structs... it + may even be that we don't even need the naked struct"*. A context on its own is routing and + configuration; it cannot say whether `Strings` or `Keyspace` is a sensible thing to offer, so it + should not be offering either. + + **Done so far:** `RespDatabaseContext` (was the `RespDatabase` class) and `RespServerContext` are + now `readonly struct`s that wrap a context and add exactly one thing - which kind it is - the same + shape the groups themselves use. `conn.GetDatabaseContext()` / `GetServerContext()` return them. + + **The accessors had to go generic first, and the reason is measurable.** They bound to the + interface, and a struct receiver passed to an interface parameter **boxes**: measured at 24 bytes + per `db.Strings`, 24,000 bytes per 1000 calls. `extension(TTarget target) where TTarget : + IRespKeyspaceTarget` is a constrained call instead - **0 bytes** - and still binds from an + interface-typed variable, so nothing that worked stopped working. The same change applies to the + down-level method shims. `RespDownlevelShimTests` had to learn about it: it matched accessors by + parameter type, which a generic parameter is not, so it found nothing and failed loudly rather than + passing vacuously - which is what its `Assert.NotEmpty` was put there for. + + **Still to do:** delete the `extension(in RespContext)` accessors and their down-level twins, and + move the ~361 live `ctx.Strings.…` call sites onto a typed context. Most of those are tests that + take a context from a per-file helper, so changing the helper's return type moves the call sites + with it; the ones that break are where the same local is used for both `ctx.Strings` and + `ctx.SendAsync`, which now needs `.Context`. + + **Then the open question Marc raised:** whether `RespContext` needs to be public at all, if both + typed contexts expose the executor in a common way for `Execute`-style patterns. It is 108 public + API lines and is what `IRespTarget.Context` returns, so that is a separate decision. + +- [ ] **`StackExchange.Redis.Build.Tests` is red, and CI does not run it. Found 2026-09-17**, while + retiring `SER010`; **pre-existing**, confirmed by stashing the change and re-running - 18 of 174 + fail on `HEAD` too. The analyzer samples call `ctx.Execute("SET", $"...")`, and `RespContext` has no + `Execute`: the public surface has `ExecuteAsync(string, ReadOnlyMemory, ...)` and + `Render`. The samples are stale against a reshape this spike did to the send API, and every failure + is the same `CS1061` knocking the expected diagnostic column out. + + **The reason nobody noticed is the more useful half**: `.github/workflows/CI.yml` runs + `tests/StackExchange.Redis.Tests` only. The analyzer's own tests - the thing that decides whether a + shipped analyzer reports correctly - are built by CI and never executed. Fixing the samples is + mechanical; adding the project to the CI test step is what stops it happening again, and should + probably come first so the fix is what turns the light green. + +- [x] **`SER010` retired entirely — 4.0 ships this as supported API.** Not "the attribute comes off the + groups": the whole experiment is over, so the ID joins `SER002`/`SER003`/`SER006` in the reserved + list, 67 `[Experimental]` attributes are gone, 612 `[SER010]` prefixes come off the public-API + files, and `SER010` leaves the repo-wide `NoWarn` - which is the part that proves it, since the + solution now builds with nothing suppressing it anywhere. + + **Two things fell out of removing the gate.** An experimental symbol used *inside* another + experimental symbol is exempt from the diagnostic, so `RespFragment.Parse` had been silently + building a fragment through the `SER011`-guarded constructor; it now says so, scoped to the one + statement, with the justification that the validation immediately above it is precisely what the + guard exists to demand. And `SA1506` started firing on doc comments that were followed by a comment + block: the attribute used to break the run, and without it a blank line inside the block reads as a + blank line after the documentation. + + **`SER011` stays, and it is not the same kind of marker**: it guards hand-constructing a + `RespFragment`, which is a speed bump on a sharp tool rather than a maturity gate - the surface + being supported does not make unchecked bytes safer. + + **`SER012` is now the open question, and it is not independent.** Measured, not guessed: taking it + out of `NoWarn` produces **1,174 diagnostics**, because `RespValue` is the element type of the + leases the groups return - `db.Sets.MembersAsync` hands back `ReadOnlyLease`. A + supported surface whose return types are experimental is not a coherent position, so this needs + answering before 4.0, and "retire it too" is the only answer that leaves the groups usable. + +- [x] **`*Async` suffixes on the new surface.** MSFT review, 2026-09-15; done the same day. Checked whether the suffix would + be redundant: the surface is async-only (22 `ValueTask` returns in Strings alone, zero sync twins), so + the *compiler* never needs it - but the justification is the reader, not the compiler, and + `db.Strings.Get(key)` sitting beside `db.StringGetAsync(key)` makes you check a return type to know + what you are holding. Mechanical across 11 groups, free while SER010 is experimental, expensive after. + Agreed, and done in the same pass as the cancellation change below, since both touch every signature. + +- [ ] **`CancellationToken` moved off the context onto the call - HALF DONE, found 2026-09-16.** + Marc: *"RangeAsync doesn't carry cancellationtoken - did we lose that somewhere? other victims?"* + Checked: **zero** `CancellationToken` references across `RespSurface.*.cs` and `Groups/*.cs`, and + zero on `RespContext`. So it is not a regression - the token came *off* the context as recorded, and + went *onto* `Send`/`SendAsync`, but never reached a single one of the ~256 public group methods. + Every one of them, uniformly, not a few stragglers. + + **The entry below argues from a signature that does not exist** - `db.Strings.GetAsync(key, flags, + token)` - which is how a half-finished change came to be ticked off. Left unticked now. + + **The deadline is not "now", it is before SER010 comes off.** Adding an optional parameter to an + existing method is a binary break (AGENTS.md), so after the experimental attributes are removed + every one of those methods needs a permanent overload instead. While experimental it is free. That + puts it in the same set as the `.Interpolated` namespace: cheap now, impossible later, and worth + deciding together rather than one at a time. + + **CORRECTION - my "what can cancellation even mean here" objection was wrong.** I argued that a RESP + request cannot be recalled, so the only honest cancellation is *stop waiting*, which would desync + unless the pipeline tracked the abandoned reply. Marc: *"the cancellation isn't about recall; it is + about (1) intercepting unsent things - retries, moves, etc, and (2) allowing the caller to get about + their day, whatever happens to the task."* + + Both are real, and neither desyncs: + + 1. **Unsent work is genuinely cancellable** - the backlog, a retry, a re-dispatch after MOVED/ASK. + Nothing has gone to the server, so cancelling is cancelling, not a euphemism. + 2. **Abandoning the await costs the connection nothing.** The pipeline already matches replies to + requests - it has to - so the reply still arrives, is still parsed, and the result is simply + dropped with nobody waiting. My desync claim confused "nobody is awaiting this" with "nobody is + reading the socket". + + **And it is already built:** Marc has cancellation working fully in the unmerged v3 spike. So this is + not an open design question with an uncertain answer - it is a known implementation waiting to be + merged, which is why putting the parameter on the signatures now is the right call rather than a + promise we might not keep. + + **Done for Streams as the template, 2026-09-16:** all 14 group methods take + `CancellationToken cancellationToken = default` as the last parameter and thread it to the send. + Until the pipeline lands, an already-cancelled token is honoured and a merely-cancellable one throws + `NotImplementedException` naming the reason - the resolution this queue already reached for + `Send`/`SendAsync`, now reaching the surface it was always meant to reach. The other 8 groups follow. + +- [x] **(the rejected idea, kept)** Recorded here + because one idea was raised and rejected with evidence: having the **interpolated string handler** + take the token via `[InterpolatedStringHandlerArgument]`, so `ThrowIfCancellationRequested` runs + before anything is formatted. + + It works mechanically, but forces the token **before** the interpolated string at every call site - + `CS8950`, "Reorder the arguments to move 'cancellationToken' before 'handler'" - which is the opposite + of the convention being asked for. Note the declaration compiles fine and only the *call* fails + (`CS8947` warns at the declaration), so "it compiled" is not evidence here. + + Rejected because the surface does not need it: `db.Strings.GetAsync(key, flags, token)` has no + interpolated parameter at the call site - the interpolation is inside the method body, where a + pre-format check is free and needs no voodoo. The tension only ever applied to the ad-hoc + `SendAsync($"...", flags, handler, token)` escape hatch, and there it would trade the convention for + one rent-and-dispose on a call that is about to throw anyway. + +- [x] **(superseded) Move `CancellationToken` off the context.** MSFT review; done 2026-09-15. + Conventional, per-call, and it shrinks the context (see the sizing item). **But it is two decisions, + not one:** the token currently *cancels nothing*. `RespMessageExecutor.SendAsync` says so - "the + existing pipeline has no cancellation; the token is observed by the caller's await". On a context that + reads as configuration; on every method signature it reads as a promise. So either wire it into the + pipeline or document plainly that it is observed at the await - do not ship per-call tokens that do + not cancel. + + **Resolved by taking neither branch.** A token that cannot cancel is not documented away and not + silently observed: `Send`/`SendAsync` take one, and a token that *can* be cancelled throws + `NotImplementedException` naming the reason. An already-cancelled token is honoured, because that one + genuinely can be - and it recycles the command first, so the obvious no-op does not leak a pooled + buffer. Wiring it into the pipeline is the real fix and belongs with the `Message` refactor. + +- [ ] **Degraded state: may the cache prefer stale to offline?** MSFT review. Today it does the opposite, + and deliberately: `OnConnectionFailed` calls `ClientCache.OnFlush()` *first*, synchronously, because + "server-assisted invalidation only works while we are listening, so anything that changed during the + gap is never announced and an entry that survives it is stale with nothing left in the system that + will ever say so". + + That reasoning is about an **unannounced** gap. A planned maintenance is different in kind, and the + library already has the signal: `AzureNotificationType` gives `NodeMaintenanceScheduled` -> + `NodeMaintenanceStarting` (~20s) -> `NodeMaintenanceStart` (<5s) -> ended. The window is announced on + both sides and therefore **bounded**, which is the property an unplanned drop lacks. + + **Suggested framing: the flush is not skipped, it is deferred to the end of the announced window.** + During a signalled window, serve stale knowingly rather than going offline; when the window closes, + flush and rebuild. That keeps the invariant - nothing survives an un-listened gap indefinitely - while + buying the availability the review is asking for. Requirements: opt-in, a bounded maximum stale age + distinct from TTL, and no path by which an entry outlives the window. Stale-while-revalidate and grace + periods already exist to build on. + + Open: whether an unsignalled drop *during* a signalled window reverts to flush-immediately (probably + yes - the signal said what would happen, and this is not it). + +- [x] **Down-level consumers: a `Downlevel` namespace of method-shims. Investigated and built 2026-09-15.** + + The commands are already classic `this in` extension methods, so they bind everywhere. Only the + **group accessors** (`db.Strings`) are extension-block properties, and those need **C# 14**. Shipped: + the properties stay in `StackExchange.Redis.Interpolated` always, and method-shims (`db.Strings()`) + live in an opt-in `StackExchange.Redis.Interpolated.Downlevel.RespGroups` - 22 of them, one per group + per receiver (`IRespKeyspaceTarget` and `in RespContext`). + + **Measured across four real toolchains** - not by pinning `LangVersion`, which is not the same thing + (see the warning below). Each built a consumer with the shim *and* the property both in scope: + + | toolchain | langver | result | + | --- | --- | --- | + | Mono msbuild 16.10, net472 | 7.3 | succeeded | + | .NET SDK 6.0.428, netstandard2.0 | 10 | succeeded | + | .NET SDK 8.0.425, net8.0 | 12 | succeeded | + | .NET SDK 11 preview, net10.0 | 14 | **CS9339**, ambiguous | + + So: **extension-block metadata is inert to a down-level compiler** - not merely unusable, invisible. + Having the properties always in scope costs those consumers nothing. + + The three failure modes are all compile-time, all actionable, and none can misbehave at run time - + both spellings construct the same value over the same context: + + - up-level importing `Downlevel` -> `CS9339`, naming both members. Up-level implies a modern SDK, so + an analyzer can always catch this one. + - down-level *without* `Downlevel` -> `CS1061`, which already ends "are you missing a using directive + or an assembly reference?". **The native message is the fix**, so the analyzer is a nicety here + rather than load-bearing - which is the answer to "does our analyzer even load on an old SDK". + - down-level *with* `Downlevel`, writing `db.Strings` -> `CS0119` "is a method", i.e. add the parens. + + **`[OverloadResolutionPriority]` does not help** - tested. `CS9339` is extension *member lookup* + between a property and a method group, which never reaches overload resolution. + + **No reshuffle was needed** - an earlier draft of this entry claimed one, and the probes disproved it. + Only the *shims* move to their own namespace; the accessors, the commands and the shared types + (`RespStrings` and friends) all stay where they are. A down-level consumer imports **both** namespaces + - which is exactly the combination all four toolchains above compiled - and only an up-level consumer + must leave `Downlevel` alone. So the cost per new command group is one shim line per receiver, not a + public move. + + **Generating the shims was raised and declined** (2026-09-15), so it does not get re-proposed: + **generators cost build time on every consumer build**, and analyzers already account for ~40% of a + clean build here - which is why they are limited to one TFM. Paying that on every build to save + writing one line per group is the wrong trade. + + The shims are written by hand and **kept honest by a unit test** rather than a generator: + `RespDownlevelShimTests` asserts that every group accessor on each receiver has a matching `Downlevel` + shim, and that a shim composes with the commands that hang off it. Same shape as + `RespTargetSplitTests.NoGroupBindsToTheBareTarget` - the rule is enforced, the build stays fast, and a + missing shim fails a test rather than silently shipping. (Verified by mutation: deleting one shim + *and* its API entry - deleting only the shim does not compile, so it proves nothing - fails exactly + that one test.) + + **Do not validate this by pinning `LangVersion`.** A modern compiler at `/langversion:12` reports + `CS9202`+`CS9339` where a *real* C# 12 compiler succeeds: it still sees the metadata and then refuses + the feature, where an old compiler never sees it. The emulation is stricter than reality, so anyone + reproducing it that way will find failures no real consumer has. (Noted because the first round of + evidence here was exactly that mistake - and worse, a `Microsoft.Net.Compilers.Toolset` pin meant to + give a genuine old compiler silently never engaged, so the results were the modern compiler all + along. Docker images of the real SDKs are the honest instrument.) + + Probe artefact worth knowing: the net472 consumer needs `Microsoft.Bcl.AsyncInterfaces` and + `System.Memory` at the library's pinned versions to bind `ValueTask`; real consumers get those + transitively, but a netfx consumer pinning older ones hits `CS1705` before any of this matters. + +- [ ] **`WATCH`/`MULTI` is BLOCKED on the `Message` refactor — do not start it first.** The measurement is + taken (`0ac297fe`): a condition makes `ExecuteAsync` block the *calling* thread for two round trips + (sync 508ms vs 2ms without), because the expansion is enumerated inside a sync `WriteMessageInsideLock` + and waits there on `Monitor.Wait`. The target is "release the thread, keep the connection reserved", + and the reservation half is already expressible: `_singleWriter` is an `AwaitableMutex`, not + thread-affine, so it can be held across an `await`. + + **What blocks it is the completion, and that is the refactor's to give.** The pulse goes away when + `Message` moves onto a poolable core with `IValueTaskSource` - which *is* an awaitable completion, + correctly armed, for free. Building an awaitable pulse-replacement now would entrench the very thing + being deleted, including its awkward "arm the monitor before sending so the pulse cannot be missed" + property, which would then have to be un-entrenched. + + Left to do once the refactor lands: an async expansion for `TransactionMessage` (the other four + implementors never wait), an async `WriteMessageInsideLock` for the two async call sites of four - the + sync write and the backlog drain stay as they are, and a sync `Execute()` caller has a thread to block + by definition. Re-measure against the 508ms. + + **Rejected, and worth not re-deriving:** doing the condition check *before* taking the write lock, so + nothing has to await inside it. `WATCH` and the conditions are sent before `MULTI` anyway, so it looks + free - but `EXEC`/`DISCARD`/`UNWATCH` are connection-global, so another transaction completing on the + same connection between our `WATCH` and our `MULTI` would silently clear our watch. The lock is what + makes the watch mean anything. + +- [ ] **The `IMultiMessage` map is complete, and finishing it found a bug.** There are exactly five: + `TransactionMessage`, `ScriptEvalMessage`, `ScriptEvaluateMessage`, `StringGetWithExpiryMessage`, + `FramePairMessage` - plus `HashImport`, which is *not* one (its preamble is injected by the bridge). + All six are now pinned by `MultiMessageInTransactionTests`; the file previously covered five of them + and read as covering all, because the two script paths look like duplicates and only one was tested. + + **The gap was load-bearing.** `ScriptEvalMessage.WriteImpl` branched on two cases where there are + three: a hash it resolved, a hash the **caller** supplied, and a body. The middle case fell into the + last, which is unreachable only while the expansion always runs - and inside a transaction it never + does, because `QueuedMessage` is not an `IMultiMessage` and never asks. So + `tran.ScriptEvaluateRespAsync(someSha1, ...)` sent `EVAL `, and the server tried to + compile it as Lua: *"ERR Error compiling script (new function)"* instead of `NOSCRIPT`. A misleading + error, and not the command the caller asked for. `ScriptEvaluateMessage` does not have the bug + because it keeps the caller's hash in its own `hexHash` field and checks that first. + + Fixed, and the same fix covers the non-transaction triggers (`NoScriptCache`, or a `CommandMap` with + `SCRIPT` disabled), which reach the same branch. **The lesson is the one `ScriptLoadPairingTests` + already recorded**: these two classes carry separate copies of one rule, so a test that exercises + either one alone proves nothing about the other. + +- [ ] **Three probes, one per layer: `EVALSHA`, `MULTI`, `HIMPORT`.** These look like three awkward + commands and are better understood as three *different seams*, which is why doing all three settles + the question and doing one does not. + + - **`EVALSHA` is composition too, not frame-level** - revised, and it is the better answer. The + original plan was a frame carrying an **alternate rendering**, recovering a `NOSCRIPT` by + re-spelling itself as `EVAL