# Parser Span Retention Implementation Plan

> **For Codex:** REQUIRED SUB-SKILLS: Use `using-git-worktrees` to create an isolated worktree before making changes, `executeplan` to implement this plan task-by-task, and `finishing-a-development-branch` to merge the verified work back to `main` when implementation is complete.

**Goal:** Reduce parser hot-path allocations by keeping protocol fields and payloads in byte-oriented views until a caller explicitly needs materialized `string` or copied `byte[]` values.

**Architecture:** Introduce a byte-first parser representation alongside the current `ParsedCommand` contract, then migrate `NatsClient` and adjacent hot paths to consume the new representation without changing wire behavior. Preserve compatibility through an adapter layer so functional parity stays stable while allocation-heavy paths move to spans, pooled buffers, and sequence slices.

**Tech Stack:** .NET 10, C#, `System.Buffers`, `System.IO.Pipelines`, `ReadOnlySequence<byte>`, `SequenceReader<byte>`, xUnit, existing benchmark test harness.

---

## Scope Anchors
- Primary source: `src/NATS.Server/Protocol/NatsParser.cs`
- Primary consumer: `src/NATS.Server/NatsClient.cs`
- Existing parser tests: `tests/NATS.Server.Core.Tests/ParserTests.cs`
- Existing snippet/parity tests: `tests/NATS.Server.Core.Tests/Protocol/ProtocolParserSnippetGapParityTests.cs`
- Documentation to update: `Documentation/Protocol/Parser.md`
- Benchmark project: `tests/NATS.Server.Benchmark.Tests/NATS.Server.Benchmark.Tests.csproj`
- Benchmark run instructions: `tests/NATS.Server.Benchmark.Tests/README.md`
- Benchmark comparison report: `benchmarks_comparison.md`

## Task 0: Create an isolated git worktree for the parser optimization work

**Files:**
- Verify: `.worktrees/`
- Modify if needed: `.gitignore`

**Step 1: Verify the preferred worktree directory is available and ignored**
- Check that `.worktrees/` exists and is ignored by git before creating a project-local worktree.
- If `.worktrees/` is not ignored, add it to `.gitignore`, then commit that repository hygiene fix before continuing.

**Step 2: Create the feature worktree on a `codex/` branch**
- Run:
```bash
git worktree add .worktrees/codex-parser-span-retention -b codex/parser-span-retention
cd .worktrees/codex-parser-span-retention
```
- Expected: a new isolated worktree exists at `.worktrees/codex-parser-span-retention` on branch `codex/parser-span-retention`.

**Step 3: Verify the worktree starts from a clean, passing baseline**
- Run: `dotnet test tests/NATS.Server.Core.Tests/NATS.Server.Core.Tests.csproj -c Release`
- Expected: PASS. If this fails, stop and resolve or explicitly confirm whether to proceed from a failing baseline.

**Step 4: Commit any required worktree setup fix**
- Only if `.gitignore` changed, run:
```bash
git add .gitignore
git commit -m "chore: ignore local worktree directories"
```

## Task 1: Freeze parser behavior and add allocation-focused tests

**Files:**
- Modify: `tests/NATS.Server.Core.Tests/ParserTests.cs`
- Modify: `tests/NATS.Server.Core.Tests/Protocol/ProtocolParserSnippetGapParityTests.cs`
- Create: `tests/NATS.Server.Core.Tests/Protocol/ParserSpanRetentionTests.cs`

**Step 1: Add failing tests for byte-first parser behavior**
- Cover `PUB`, `HPUB`, `CONNECT`, and `INFO` with assertions that the new parser path can expose field data without forcing immediate `string` materialization.
- Add split-payload cases to prove the parser preserves pending payload state across reads.

**Step 2: Add compatibility tests for existing `ParsedCommand` behavior**
- Keep current semantics for `Type`, `Subject`, `ReplyTo`, `Queue`, `Sid`, `HeaderSize`, and `Payload`.
- Ensure malformed protocol inputs still throw `ProtocolViolationException` with existing snippets/messages.

**Step 3: Run targeted tests to verify the new tests fail first**
- Run: `dotnet test tests/NATS.Server.Core.Tests/NATS.Server.Core.Tests.csproj --filter "FullyQualifiedName~ParserTests|FullyQualifiedName~ParserSpanRetentionTests|FullyQualifiedName~ProtocolParserSnippetGapParityTests" -c Release`
- Expected: FAIL in the newly added parser span-retention tests only.

**Step 4: Commit the failing-test baseline**
- Run:
```bash
git add tests/NATS.Server.Core.Tests/ParserTests.cs tests/NATS.Server.Core.Tests/Protocol/ProtocolParserSnippetGapParityTests.cs tests/NATS.Server.Core.Tests/Protocol/ParserSpanRetentionTests.cs
git commit -m "test: lock parser span-retention behavior"
```

## Task 2: Introduce byte-oriented parser view types

**Files:**
- Create: `src/NATS.Server/Protocol/ParsedCommandView.cs`
- Modify: `src/NATS.Server/Protocol/NatsParser.cs`

**Step 1: Add a hot-path parser view contract**
- Create a `ref struct` or small `readonly struct` representation for command views that can carry:
  - operation kind
  - subject/reply/queue/SID as spans or sequence-backed views
  - payload as `ReadOnlySequence<byte>` or `ReadOnlyMemory<byte>` when contiguous
  - header size and max-messages metadata

**Step 2: Add an adapter to the current `ParsedCommand` shape**
- Keep the public/internal `ParsedCommand` entry point usable for existing consumers and tests.
- Centralize materialization so `Encoding.ASCII.GetString(...)` and `ToArray()` happen in one adapter layer instead of inside every parse branch.

**Step 3: Re-run parser tests**
- Run: `dotnet test tests/NATS.Server.Core.Tests/NATS.Server.Core.Tests.csproj --filter "FullyQualifiedName~Parser" -c Release`
- Expected: FAIL only in branches not yet migrated to the new view path.

**Step 4: Commit the parser-view scaffolding**
- Run:
```bash
git add src/NATS.Server/Protocol/ParsedCommandView.cs src/NATS.Server/Protocol/NatsParser.cs
git commit -m "feat: add byte-oriented parser view contract"
```

## Task 3: Rework control-line parsing and pending payload state

**Files:**
- Modify: `src/NATS.Server/Protocol/NatsParser.cs`

**Step 1: Remove early string materialization from control-line parsing**
- Change `ParsePub`, `ParseHPub`, `ParseSub`, `ParseUnsub`, `ParseConnect`, and `ParseInfo` to keep raw byte slices in the hot parser path.
- Replace `_pendingSubject` and `_pendingReplyTo` string fields with byte-oriented pending state.

**Step 2: Avoid unconditional payload copies**
- Update `TryReadPayload()` so single-segment payloads can flow through as borrowed memory/slices.
- Copy only when the payload is multi-segment or when the compatibility adapter explicitly requires a standalone buffer.

**Step 3: Replace repeated tiny literal allocations**
- Stop using per-call `u8.ToArray()`-style buffers for CRLF and other fixed protocol tokens inside this parser path.
- Add shared static buffers where appropriate.

**Step 4: Run targeted regression tests**
- Run: `dotnet test tests/NATS.Server.Core.Tests/NATS.Server.Core.Tests.csproj --filter "FullyQualifiedName~ParserTests|FullyQualifiedName~ProtocolParserSnippetGapParityTests" -c Release`
- Expected: PASS.

**Step 5: Commit the parser hot-path rewrite**
- Run:
```bash
git add src/NATS.Server/Protocol/NatsParser.cs
git commit -m "perf: keep parser state in bytes until materialization"
```

## Task 4: Migrate `NatsClient` to the new parser path without changing behavior

**Files:**
- Modify: `src/NATS.Server/NatsClient.cs`
- Modify: `tests/NATS.Server.Core.Tests/ParserTests.cs`
- Modify: `tests/NATS.Server.Core.Tests/Protocol/ClientProtocolGoParityTests.cs`

**Step 1: Consume parser views first, materialize only at command handling boundaries**
- Update `ProcessCommandsAsync` and any parser call sites so hot `PUB`/`HPUB` handling can read subject, reply, and payload from the byte-oriented representation.
- Keep logging/tracing behavior intact, but ensure tracing is the only reason strings are created on trace-enabled paths.

**Step 2: Preserve feature parity**
- Verify header parsing, payload size checks, connect/info handling, and slow-consumer/error behavior still match current tests.

**Step 3: Run consumer-facing protocol tests**
- Run: `dotnet test tests/NATS.Server.Core.Tests/NATS.Server.Core.Tests.csproj --filter "FullyQualifiedName~ClientProtocolGoParityTests|FullyQualifiedName~ParserTests" -c Release`
- Expected: PASS.

**Step 4: Commit the consumer migration**
- Run:
```bash
git add src/NATS.Server/NatsClient.cs tests/NATS.Server.Core.Tests/ParserTests.cs tests/NATS.Server.Core.Tests/Protocol/ClientProtocolGoParityTests.cs
git commit -m "perf: consume parser command views in client hot path"
```

## Task 5: Add benchmarks, document the change, run full verification, and refresh the benchmark comparison report

**Files:**
- Create: `tests/NATS.Server.Benchmark.Tests/Protocol/ParserHotPathBenchmarks.cs`
- Modify: `Documentation/Protocol/Parser.md`
- Modify: `benchmarks_comparison.md`

**Step 1: Add parser-focused benchmark coverage**
- Add microbenchmarks for:
  - `PING` / `PONG`
  - `PUB`
  - `HPUB`
  - split payload reads
- Capture throughput and allocation deltas before/after.

**Step 2: Update protocol documentation**
- Document the new parser view + adapter split, why strings are deferred, and where payload copying is still intentionally required.

**Step 3: Run full verification**
- Run: `dotnet test tests/NATS.Server.Core.Tests/NATS.Server.Core.Tests.csproj -c Release`
- Run: `dotnet test tests/NATS.Server.Benchmark.Tests/NATS.Server.Benchmark.Tests.csproj --filter "FullyQualifiedName~Parser" -c Release`
- Expected: PASS; benchmark output shows reduced allocations relative to the baseline run.

**Step 4: Run the full benchmark suite per the benchmark project README**
- Run:
```bash
dotnet test tests/NATS.Server.Benchmark.Tests \
  --filter "Category=Benchmark" \
  -v normal \
  --logger "console;verbosity=detailed" 2>&1 | tee /tmp/bench-output.txt
```
- Expected: benchmark comparison output is captured in `/tmp/bench-output.txt`, including the "Standard Output Messages" blocks described in `tests/NATS.Server.Benchmark.Tests/README.md`.

**Step 5: Update `benchmarks_comparison.md` with the new benchmark results**
- Extract the comparison blocks from `/tmp/bench-output.txt`.
- Update `benchmarks_comparison.md` with the latest msg/s, MB/s, ratio, and latency values.
- Update the benchmark date, environment description, Summary table, and Key Observations so they match the new run.

**Step 6: Commit the verification, docs, and benchmark report refresh**
- Run:
```bash
git add tests/NATS.Server.Benchmark.Tests/Protocol/ParserHotPathBenchmarks.cs Documentation/Protocol/Parser.md benchmarks_comparison.md
git commit -m "docs: record parser hot-path allocation strategy"
```

## Task 6: Merge the verified parser work back to `main` and clean up the worktree

**Files:**
- No source changes expected

**Step 1: Confirm the feature branch is fully verified before merge**
- Reuse the verification from Task 5. Do not merge if the core tests, parser benchmark tests, or full benchmark suite run did not complete successfully.

**Step 2: Merge `codex/parser-span-retention` back into `main`**
- Return to the primary repository worktree and run:
```bash
git checkout main
git pull
git merge codex/parser-span-retention
```
- Expected: the parser optimization commits merge cleanly into `main`.

**Step 3: Re-run verification on the merged `main` branch**
- Run: `dotnet test tests/NATS.Server.Core.Tests/NATS.Server.Core.Tests.csproj -c Release`
- Run: `dotnet test tests/NATS.Server.Benchmark.Tests/NATS.Server.Benchmark.Tests.csproj --filter "FullyQualifiedName~Parser" -c Release`
- Confirm `benchmarks_comparison.md` reflects the results captured from the Task 5 full benchmark suite.
- Expected: PASS on merged `main`.

**Step 4: Delete the feature branch and remove the worktree**
- Run:
```bash
git branch -d codex/parser-span-retention
git worktree remove .worktrees/codex-parser-span-retention
```
- Expected: only `main` remains checked out in the primary workspace, and the temporary parser worktree is removed.

## Completion Checklist
- [ ] Parser optimization work was implemented in an isolated `.worktrees/codex-parser-span-retention` worktree on branch `codex/parser-span-retention`.
- [ ] `NatsParser` no longer materializes hot-path `string` values during parse unless the compatibility adapter requests them.
- [ ] Single-segment payloads can pass through without an unconditional `byte[]` copy.
- [ ] Existing parser and protocol behavior remains green in core tests.
- [ ] Parser-focused benchmark coverage exists in `tests/NATS.Server.Benchmark.Tests/Protocol/`.
- [ ] The full benchmark suite was run using the workflow from `tests/NATS.Server.Benchmark.Tests/README.md`.
- [ ] `benchmarks_comparison.md` was updated to reflect the latest benchmark run.
- [ ] `Documentation/Protocol/Parser.md` explains the byte-first parser architecture.
- [ ] Verified parser optimization commits were merged back into `main`.