tokenizers: TryDecode(Span<char>) zero-allocation overload + IncrementalDetokenizer adoption

[jamesburton]

Summary

Addresses an item from #121. Adds a TryDecode(Span<char>) zero-allocation overload to the tokenizer interface and adopts it in IncrementalDetokenizer, removing string allocations from the steady-state streaming detokenize path.

Test results

• New: TryDecodeZeroAllocTests (334 lines).
• 10 files changed, 791 insertions / 44 deletions.
• Bit-exact equivalence to the allocating overload is asserted.

Notes for review

Single-commit perf fix on main. The allocating Decode is preserved for backward compatibility.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a zero-allocation decode path to the tokenizer API and wires it through the BPE encodings and incremental detokenization flow to reduce allocations and improve per-token stop-check performance.

Changes:

• Introduces ITokenizer.TryDecode(...) and implements it for SentencePiece and GPT-2/tiktoken BPE decode paths.
• Refactors IncrementalDetokenizer to decode into pooled buffers and adds deterministic cleanup via IDisposable.
• Adds unit tests and a BenchmarkDotNet benchmark to validate parity and reduced allocations.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/DotLLM.Tests.Unit/Tokenizers/TryDecodeZeroAllocTests.cs	New parity + allocation-focused tests for TryDecode.
tests/DotLLM.Tests.Unit/Engine/IncrementalDetokenizerTests.cs	Adds allocation regression coverage for IncrementalDetokenizer.Append().
src/DotLLM.Tokenizers/ITokenizer.cs	Adds TryDecode default interface method and docs.
src/DotLLM.Tokenizers/Bpe/SentencePieceEncoding.cs	Implements span-based zero-alloc SentencePiece decode.
src/DotLLM.Tokenizers/Bpe/Gpt2TiktokenEncoding.cs	Adds atomic span-based decode and factors byte materialization helper.
src/DotLLM.Tokenizers/Bpe/BpeTokenizer.cs	Overrides ITokenizer.TryDecode to delegate to encoding.
src/DotLLM.Tokenizers/Bpe/BpeCore.cs	Extends encoding interface and adds TryFlushByteBuffer helper.
src/DotLLM.Engine/TextGenerator.cs	Ensures IncrementalDetokenizer is disposed even on early exits/cancellation.
src/DotLLM.Engine/IncrementalDetokenizer.cs	Moves window/tail decoding to pooled char buffers; adds IDisposable.
benchmarks/DotLLM.Benchmarks/Tokenizers/DetokenizerAllocBenchmark.cs	Adds allocation benchmark comparing Decode vs TryDecode.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jamesburton]

Thanks for the review — all six comments assessed, pushed as 3a5d34e.

Applied

1. TextGenerator.cs:148 / :492 — pooled buffer stranded if the detokenizer constructor throws. Fair; the constructor rents from the pool itself, so it is exactly the call most likely to fail after stopScratch is live. Both the sync and streaming paths now declare IncrementalDetokenizer? detok = null outside the try and construct it as the first statement inside, with detok?.Dispose() in the finally. That keeps the existing single try/finally (no re-indentation of the generation loop) and compiles warning-free under Nullable enable — the definite-assignment flow analysis carries the non-null state through the whole body.

2. IncrementalDetokenizer.cs:209 — pooled char[] returned without clearing. Agreed, and cheaper than the tradeoff framing suggests. Dispose() runs once per generation, not per token, and the grow-and-re-rent paths converge logarithmically — so Return(clearArray: true) at all four sites costs nothing on the hot path. Also applied to the two stopScratch returns in TextGenerator, which hold decoded output for the same reason. No measurable change to the zero-allocation assertions (Array.Clear does not allocate).

3. DetokenizerAllocBenchmark.cs:55 — ignored TryDecode result. Valid. Rather than adding a branch to the measured loop, GlobalSetup now asserts that _scratch fits both decodes and throws otherwise. The inputs are fixed per run, so this catches the failure case completely while leaving the benchmark bodies byte-for-byte as measured.

4. TryDecodeZeroAllocTests.cs:286 — strict delta == 0 may be flaky. Taking your "small upper bound" option, with a bound chosen so nothing is given up: 1000 bytes across 1000 warmed calls. The minimum managed object size is 24 bytes, so staying under one byte per call mathematically rules out any per-call allocation, while absorbing one-off runtime bookkeeping. The allocating Decode baseline costs tens of bytes per call here, so the assertion still discriminates by well over an order of magnitude.

Answered rather than changed

5. IncrementalDetokenizerTests.cs:220 — allocation-ceiling sensitivity. Half applied. This assertion is already a tolerant ceiling (2 KB / 200 calls = 10 bytes per call, versus a pre-PR baseline of ~20+), so loosening it further would start eroding its ability to catch the regression it exists for. I did take the pre-sizing suggestion: the detokenizer is now constructed with initialCapacity: 4096 against ~232 chars of produced text, so StringBuilder chunk growth cannot contribute to the measurement at all and the remaining headroom is pure noise margin. Threshold kept as-is.

Verification — dotnet test --filter "FullyQualifiedName~TryDecode|FullyQualifiedName~Detokenizer": 25 passed, 0 failed. DotLLM.Engine and DotLLM.Benchmarks both build with 0 warnings.