NEO-94: add code review for rewritten snapshot GET branch

docs/reviews/2026-05-28-NEO-94.md

@@ -0,0 +1,64 @@
+# Code review — NEO-94 (E5M2-08)
+
+**Date:** 2026-05-28  
+**Scope:** Branch `NEO-94-npc-runtime-snapshot-get` vs `origin/main` — full rewrite in `a68d8aa` (+ plan `5442910`)  
+**Base:** `origin/main`
+
+## Verdict
+
+**Approve with nits**
+
+## Summary
+
+The rewritten branch delivers E5M2-08 cleanly: **`GET /game/world/npc-runtime-snapshot`** invokes **`NpcRuntimeOperations.AdvanceAll`** on poll, then projects all three **`PrototypeNpcRegistry`** rows with **`behaviorDefId`**, **`state`**, **`aggroHolderPlayerId`**, and optional nested **`activeTelegraph`** (including server-computed **`windupRemainingSeconds`**). The implementation mirrors **`CooldownSnapshotApi`** / **`CombatTargetsWorldApi`** conventions — `internal static BuildSnapshot`, fail-fast registry lookups, versioned envelope with **`serverTimeUtc`**, route registered in **`Program.cs`**. Four AAA integration tests cover idle baseline, cast→aggro, advance→telegraph, and decreasing windup; Bruno adds happy-path + telegraph smoke with documented **`sleep(3100)`**. Build succeeds; NEO-94-filtered tests pass (**4/4**). **`NpcRuntimeStateSnapshot`** is unchanged — projection reads from registry + threat store at HTTP boundary, as planned.
+
+## Documentation checked
+
+| Path | Result |
+|------|--------|
+| `docs/plans/NEO-94-implementation-plan.md` | **Matches** — AC checkboxes marked done; handler flow, wire shape, lazy tick, and file list align with the diff. |
+| `docs/plans/E5M2-prototype-backlog.md` (E5M2-08) | **Matches** — AC checked, landed note present. |
+| `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | **Matches** — E5M2-08 landed in status table; NEO-94 HTTP section + README/Bruno cross-links. |
+| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Partially matches** — E5.M2 row still ends at **NEO-93**; should add **NEO-94 landed** note per plan. |
+| `docs/decomposition/modules/client_server_authority.md` | **Matches** — server-owned runtime + telegraph timing; client poll-only (NEO-97 deferred). |
+| `server/README.md` | **Matches** — threat/runtime HTTP read rows updated; dedicated NEO-94 section with field table and curl. |
+
+## Blocking issues
+
+None.
+
+## Suggestions
+
+1. **Update `documentation_and_implementation_alignment.md`** — Append **NEO-94 landed** to the E5.M2 tracking row (snapshot GET, Bruno folder, plan/README links), matching NEO-90–NEO-93 entries. Listed in the plan “Files to modify” but not yet in the diff.
+
+2. **AAA layout in two integration tests** — `GetNpcRuntimeSnapshot_ShouldShowAggro_OnMeleeAfterDamagingCast` and `GetNpcRuntimeSnapshot_ShouldShowTelegraph_AfterAttackCooldownElapses` call **`ReadFromJsonAsync`** in **Act**. Move status/body reads to **Assert** to match **`CombatTargetsWorldApiTests`** and [csharp-style](../../.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert):
+
+   ```csharp
+   // Act
+   var response = await client.GetAsync("/game/world/npc-runtime-snapshot");
+   // Assert
+   Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+   var body = await response.Content.ReadFromJsonAsync<NpcRuntimeSnapshotResponse>();
+   ```
+
+## Nits
+
+- Nit: **`ActiveTelegraph`** / **`AggroHolderPlayerId`** could use **`[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]`** for leaner JSON (optional; explicit `null` is fine for client parsers).
+- Nit: **`GetNpcRuntimeSnapshot_ShouldDecreaseWindupRemaining_AcrossPolls`** captures **`firstBody`** during **Arrange** (baseline for comparison). Acceptable for a two-poll delta test; alternatively split into two facts for stricter AAA.
+
+## Verification
+
+```bash
+# Build + NEO-94 tests (passed 2026-05-28)
+dotnet build NeonSprawl.sln
+dotnet test server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj \
+  --filter "FullyQualifiedName~NpcRuntimeSnapshot"
+
+# Full server suite (Postgres persistence tests require local DB; 12 env failures unrelated to NEO-94)
+dotnet test server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj
+
+# Bruno (CI or local dev server)
+# bruno/neon-sprawl-server/npc-runtime-snapshot/
+```
+
+Manual: run **`Get snapshot after cast telegraph.bru`** against dev server — melee should reach **`telegraph_windup`** with **`activeTelegraph.windupRemainingSeconds > 0`** after the 3.1 s wait.