diff --git a/docs/reviews/2026-05-28-NEO-94.md b/docs/reviews/2026-05-28-NEO-94.md index 7463165..2828276 100644 --- a/docs/reviews/2026-05-28-NEO-94.md +++ b/docs/reviews/2026-05-28-NEO-94.md @@ -4,9 +4,11 @@ **Scope:** Branch `NEO-94-npc-runtime-snapshot-get` vs `origin/main` — full rewrite in `a68d8aa` (+ plan `5442910`) **Base:** `origin/main` +**Follow-up:** Suggestions addressed in `798c601`; re-verified 2026-05-28 (NEO-94 tests 4/4 pass). + ## Verdict -**Approve with nits** +**Approve** ## Summary @@ -29,7 +31,7 @@ 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.~~ **Done.** +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.~~ **Done.** (`798c601`) 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): @@ -39,12 +41,12 @@ None. // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); var body = await response.Content.ReadFromJsonAsync(); - ```~~ **Done.** Aggro + telegraph tests fixed; decreasing-windup test also moves all **`ReadFromJsonAsync`** calls to **Assert**. + ```~~ **Done.** Aggro + telegraph tests fixed; decreasing-windup test also moves all **`ReadFromJsonAsync`** calls to **Assert** (`798c601`). ## 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. +- ~~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.~~ **Done.** **`ReadFromJsonAsync`** for both polls now in **Assert**; first poll GET remains in **Arrange** as setup (`798c601`). ## Verification