NEO-98: address code review — Esc regression step and AC parity

2026-05-30 17:05:30 -04:00 · 2026-05-30 17:05:30 -04:00 · b03128a3b0
parent 817ed26304
commit b03128a3b0
5 changed files with 16 additions and 13 deletions
--- a/client/README.md
+++ b/client/README.md
@ -172,11 +172,11 @@ Epic 5 Slice 2 capstone — aggro, readable telegraph, incoming NPC damage, and
 | Step | Input / trigger | HUD / server outcome |
 |------|-----------------|----------------------|
 | Boot | Godot **F5** + fresh server | Hotbar slot 0 binds **`prototype_pulse`**; player combat HP **100/100** when poll starts |
-| Melee lock | **Tab** on **`prototype_npc_melee`** | Marker brightens; **`CombatTargetHpLabel`** **`100/100`** |
+| Melee lock | **Tab** on **`prototype_npc_melee`** | Marker brightens; **`CombatTargetHpLabel`** **`Target HP: prototype_npc_melee 100/100`** |
 | Melee aggro | **1** (damaging cast) | **`NpcStateLabel`** → **`aggro`**; combat poll ~1 Hz |
 | Melee telegraph | Wait **≥ 3 s** | **`telegraph_windup`** + **`TelegraphLabel`** countdown |
 | Melee hit | Wait **≥ 4.5 s** from first cast | **`PlayerCombatHpLabel`** → **`85/100`** (15 damage) |
-| Melee defeat | **1** ×4 total (cooldown between) | **`CombatTargetHpLabel`** **`0/100 (defeated)`** |
+| Melee defeat | **1** ×4 total (cooldown between) | **`CombatTargetHpLabel`** includes **`0/100 (defeated)`** |
 | Ranged | Lock + cast ×4; wait **≥ 4 s** for telegraph | Defeat at **80** HP; telegraph visible before damage |
 | Elite | Lock + cast ×8; wait **≥ 5 s** for telegraph | Defeat at **200** HP; telegraph visible before damage |

--- a/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md
+++ b/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md
@ -8,7 +8,7 @@
 | **Epic** | [Epic 5 — PvE Combat](../epics/epic_05_pve_combat.md) |
 | **Stage target** | Prototype |
 | **Status** | Ready — Slice 2 backlog [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md): **E5M2-01** [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) catalog + CI landed · **E5M2-02** [NEO-88](https://linear.app/neon-sprawl/issue/NEO-88) server load landed · **E5M2-03** [NEO-89](https://linear.app/neon-sprawl/issue/NEO-89) registry + DI landed · **E5M2-04** [NEO-90](https://linear.app/neon-sprawl/issue/NEO-90) behavior-definitions GET landed · **E5M2-05** [NEO-91](https://linear.app/neon-sprawl/issue/NEO-91) NPC instance registry + combat-target migration landed · **E5M2-06** [NEO-92](https://linear.app/neon-sprawl/issue/NEO-92) aggro rule + threat store landed · **E5M2-07** [NEO-93](https://linear.app/neon-sprawl/issue/NEO-93) NPC runtime state machine landed · **E5M2-08** [NEO-94](https://linear.app/neon-sprawl/issue/NEO-94) runtime snapshot GET landed · **E5M2-09** [NEO-95](https://linear.app/neon-sprawl/issue/NEO-95) NPC attack resolve + player combat HP landed · **E5M2-10** [NEO-96](https://linear.app/neon-sprawl/issue/NEO-96) NPC runtime telemetry hooks landed · **E5M2-11** [NEO-97](https://linear.app/neon-sprawl/issue/NEO-97) client telegraph HUD landed · **E5M2-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98) playable capstone landed |
-| **Linear** | Label **`E5.M2`** · [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md) **E5M2-01** [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) · **E5M2-02** [NEO-88](https://linear.app/neon-sprawl/issue/NEO-88) · **E5M2-03** [NEO-89](https://linear.app/neon-sprawl/issue/NEO-89) · **E5M2-04** [NEO-90](https://linear.app/neon-sprawl/issue/NEO-90) · **E5M2-05** [NEO-91](https://linear.app/neon-sprawl/issue/NEO-91) → **E5M2-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98) |
+| **Linear** | Label **`E5.M2`** · [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md) **E5M2-01** [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) · **E5M2-02** [NEO-88](https://linear.app/neon-sprawl/issue/NEO-88) · **E5M2-03** [NEO-89](https://linear.app/neon-sprawl/issue/NEO-89) · **E5M2-04** [NEO-90](https://linear.app/neon-sprawl/issue/NEO-90) · **E5M2-05** [NEO-91](https://linear.app/neon-sprawl/issue/NEO-91) · **E5M2-06** [NEO-92](https://linear.app/neon-sprawl/issue/NEO-92) · **E5M2-07** [NEO-93](https://linear.app/neon-sprawl/issue/NEO-93) · **E5M2-08** [NEO-94](https://linear.app/neon-sprawl/issue/NEO-94) · **E5M2-09** [NEO-95](https://linear.app/neon-sprawl/issue/NEO-95) · **E5M2-10** [NEO-96](https://linear.app/neon-sprawl/issue/NEO-96) · **E5M2-11** [NEO-97](https://linear.app/neon-sprawl/issue/NEO-97) · **E5M2-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98) |

 ## Purpose

--- a/docs/manual-qa/NEO-98.md
+++ b/docs/manual-qa/NEO-98.md
@ -13,6 +13,7 @@
 - **Fresh dev player:** stop any running server, then start a new instance so in-memory **`dev-local-1`** NPC HP, aggro/runtime rows, and player combat HP reset.
 - **No Bruno/curl seeding** for this checklist — hotbar **`prototype_pulse`** bind is automatic (NEO-85 dev bootstrap).
 - NEO-97 telegraph HUD landed on `main` (PR #136).
+- **`CombatTargetHpLabel`** uses NEO-85 format **`Target HP: {id} {current}/{max}`** (e.g. **`Target HP: prototype_npc_melee 100/100`**) — match HP substring or full line.

 ## Archetype reference

@ -44,7 +45,7 @@ Player casts **`prototype_pulse`** (**25** damage, ~**3 s** cooldown).
 ### Melee — full telegraph + player HP spine

 3. Walk to orange **Melee** marker west of spawn **`(-3, -3)`**.
-4. Press **Tab** until **`prototype_npc_melee`** is locked with **`Validity: ok`** — marker **brightens**; **`CombatTargetHpLabel`** shows **`100/100`**.
+4. Press **Tab** until **`prototype_npc_melee`** is locked with **`Validity: ok`** — marker **brightens**; **`CombatTargetHpLabel`** shows **`Target HP: prototype_npc_melee 100/100`** (or **`…`** briefly until GET completes).
 5. Press **1** (damaging cast): **`CastFeedbackLabel`** shows damage; **`NpcStateLabel`** eventually shows **`NPC state: Melee → aggro`** (~1 Hz poll).
 6. **`PlayerCombatHpLabel`** shows **`Player HP: 100/100`** once combat poll starts.
 7. Wait **≥ 3 s** from first cast: **`NpcStateLabel`** shows **`telegraph_windup`**; **`TelegraphLabel`** shows countdown (e.g. **`Telegraph: Melee … · ~1.x s (melee)`**) ticking between polls.
@ -68,8 +69,9 @@ Player casts **`prototype_pulse`** (**25** damage, ~**3 s** cooldown).

 ### Regression

-19. Cast without lock still shows **`ability_cast_denied: invalid_target`** on **`CastFeedbackLabel`** (NEO-28).
-20. **`CombatTargetHpLabel`** + cooldown HUD still refresh after accepted casts (NEO-85 / NEO-32).
+19. Press **Esc** to clear lock: **`CombatTargetHpLabel`** shows **`Target HP: — (Tab → elite/melee/ranged near spawn)`** (or equivalent no-lock line).
+20. Cast without lock: **`CastFeedbackLabel`** shows **`ability_cast_denied: invalid_target`** (NEO-28).
+21. **`CombatTargetHpLabel`** + cooldown HUD still refresh after accepted casts (NEO-85 / NEO-32) — re-lock any NPC and press **1** once to confirm.

 ## Notes

@ -80,7 +82,7 @@ Player casts **`prototype_pulse`** (**25** damage, ~**3 s** cooldown).

 ## Acceptance

- [ ] Steps 1–20 completable in one session without Bruno/curl.
+- [ ] Steps 1–21 completable in one session without Bruno/curl.
 - [ ] Melee telegraph countdown readable **before** player HP drops to **85/100** (steps 7–8).
 - [ ] All three archetypes show **`defeated`** in **`CombatTargetHpLabel`** (steps 9, 14, 18).
 - [ ] Aggro on first damaging cast feels deterministic (no silent failures).
--- a/docs/plans/E5M2-prototype-backlog.md
+++ b/docs/plans/E5M2-prototype-backlog.md
@ -377,6 +377,7 @@ Working backlog for **Epic 5 — Slice 2** ([NPC archetypes and telegraphs](../d

 - [x] Human completes script with server + client; telegraph timing readable before damage.
 - [x] Epic 5 Slice 2 AC: telegraph timing matches UI feedback; aggro rules feel deterministic in manual QA.
+- [x] All three archetypes defeated in one session; **`CombatTargetHpLabel`** shows defeated for each (manual QA steps 9, 14, 18).
 - [x] Re-read [epic_05 Definition of Done](../decomposition/epics/epic_05_pve_combat.md#definition-of-done) for prototype minimums.

 **Landed ([NEO-98](https://linear.app/neon-sprawl/issue/NEO-98)):** playable NPC telegraph combat capstone — [`NEO-98` manual QA](../manual-qa/NEO-98.md); `client/README.md` end-to-end NPC telegraph combat loop section; plan [NEO-98-implementation-plan.md](NEO-98-implementation-plan.md). **Epic 5 Slice 2 client capstone complete.**
--- a/docs/reviews/2026-05-30-NEO-98.md
+++ b/docs/reviews/2026-05-30-NEO-98.md
@ -17,7 +17,7 @@ NEO-98 closes **E5M2-12** and Epic 5 Slice 2 client acceptance with a **docs-onl
 | Path | Result |
 |------|--------|
 | `docs/plans/NEO-98-implementation-plan.md` | **Matches** — kickoff decisions (docs-primary, fresh restart, Melee→Ranged→Elite, melee full / ranged+elite light, NEO-97 as component regression), scope, file list, and reconciliation align with the diff; no integration fixes shipped. |
-| `docs/plans/E5M2-prototype-backlog.md` (E5M2-12) | **Partially matches** — landed note and three AC items checked; plan’s fourth AC (“all three archetypes defeated”) is absent from backlog AC list (see Suggestions). |
+| `docs/plans/E5M2-prototype-backlog.md` (E5M2-12) | **Matches** — landed note and four AC items checked (includes three-archetype defeat). |
 | `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | **Matches** — Status **Ready** with E5M2-11 + E5M2-12 landed; capstone cross-link + manual QA pointer added. |
 | `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E5.M2 row **Ready**, **NEO-98 landed**, Slice 2 client capstone complete. |
 | `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E5.M2 note summarizes NEO-92–NEO-98 and marks Slice 2 complete. |
@ -32,15 +32,15 @@ None.

 ## Suggestions

-1. **Regression step — clear lock before `invalid_target` check** — After step 18 the tester is still locked on defeated **`prototype_npc_elite`**; step 19 (“cast without lock”) will hit **`target_defeated`**, not **`invalid_target`**, unless lock is cleared first. Add an **Esc** step (mirror [NEO-86 manual QA](../manual-qa/NEO-86.md) steps 8–10) before the NEO-28 regression assertion.
+1. ~~**Regression step — clear lock before `invalid_target` check** — After step 18 the tester is still locked on defeated **`prototype_npc_elite`**; step 19 (“cast without lock”) will hit **`target_defeated`**, not **`invalid_target`**, unless lock is cleared first. Add an **Esc** step (mirror [NEO-86 manual QA](../manual-qa/NEO-86.md) steps 8–10) before the NEO-28 regression assertion.~~ **Done.** Manual QA step 19 **Esc** clear; NEO-28 check is step 20.

-2. **E5M2-12 backlog AC parity** — [NEO-98 implementation plan](../plans/NEO-98-implementation-plan.md) lists a fourth AC (“All three archetypes defeated… **`CombatTargetHpLabel`** shows defeated for each”); [E5M2-prototype-backlog.md](../plans/E5M2-prototype-backlog.md) E5M2-12 section omits it. Add the checkbox for traceability with the plan and manual QA steps 9/14/18.
+2. ~~**E5M2-12 backlog AC parity** — [NEO-98 implementation plan](../plans/NEO-98-implementation-plan.md) lists a fourth AC (“All three archetypes defeated… **`CombatTargetHpLabel`** shows defeated for each”); [E5M2-prototype-backlog.md](../plans/E5M2-prototype-backlog.md) E5M2-12 section omits it. Add the checkbox for traceability with the plan and manual QA steps 9/14/18.~~ **Done.**

 ## Nits

- Nit: **E5.M2 Summary `Linear` row** still abbreviates **`E5M2-05 … → E5M2-12`** while the **Status** row now lists E5M2-06–E5M2-11 inline — optional consistency pass.
+- ~~Nit: **E5.M2 Summary `Linear` row** still abbreviates **`E5M2-05 … → E5M2-12`** while the **Status** row now lists E5M2-06–E5M2-11 inline — optional consistency pass.~~ **Done.** Linear row lists E5M2-01–E5M2-12 inline.

- Nit: Manual QA / README use shorthand **`100/100`** on **`CombatTargetHpLabel`**; NEO-85 format includes **`Target HP: {id}`** prefix — testers should match substring, not exact label text (or tighten one example step to full format).
+- ~~Nit: Manual QA / README use shorthand **`100/100`** on **`CombatTargetHpLabel`**; NEO-85 format includes **`Target HP: {id}`** prefix — testers should match substring, not exact label text (or tighten one example step to full format).~~ **Done.** Preconditions note + step 4 full format; README flow table updated.

 - Nit: Plan / backlog AC boxes are **`[x]`** while **`docs/manual-qa/NEO-98.md` Acceptance** stays **`[ ]`** until a human run — intentional per NEO-86; keep that distinction when pasting PR text.

@ -51,6 +51,6 @@ None.
 cd server/NeonSprawl.Server.Tests && dotnet test --filter "FullyQualifiedName~NpcRuntime"
 ```

-**Manual (required before merge):** [`docs/manual-qa/NEO-98.md`](../manual-qa/NEO-98.md) — fresh server restart, Godot **F5**, Melee steps 3–10 (telegraph visible **before** **85/100**), Ranged + Elite defeat with telegraph visible, regression steps 19–20.
+**Manual (required before merge):** [`docs/manual-qa/NEO-98.md`](../manual-qa/NEO-98.md) — fresh server restart, Godot **F5**, Melee steps 3–10 (telegraph visible **before** **85/100**), Ranged + Elite defeat with telegraph visible, regression steps 19–21 (**Esc** before NEO-28 deny).

 **Optional component regression:** [`docs/manual-qa/NEO-97.md`](../manual-qa/NEO-97.md) — elite incoming-threat row when locking a different NPC while elite holds aggro.