NEO-93: Reconcile backlog and review doc after code review follow-up.

Co-authored-by: Cursor <cursoragent@cursor.com>
pull/131/head
VinPropane 2026-05-27 23:08:34 -04:00
parent d3f178b5fb
commit 4fcbb99881
3 changed files with 13 additions and 10 deletions

View File

@ -231,7 +231,7 @@ Working backlog for **Epic 5 — Slice 2** ([NPC archetypes and telegraphs](../d
**In scope** **In scope**
- `NpcRuntimeOperations.AdvanceAll` (or per-id) called from snapshot GET handler with monotonic clock delta cap. - `NpcRuntimeOperations.AdvanceAll` (or per-id) — engine + DI only in NEO-93; [NEO-94](https://linear.app/neon-sprawl/issue/NEO-94) invokes from **`GET /game/world/npc-runtime-snapshot`** with monotonic clock delta cap.
- State transitions emit internal events for telemetry hooks (E5M2-10). - State transitions emit internal events for telemetry hooks (E5M2-10).
- Unit tests with injected clock for deterministic transitions. - Unit tests with injected clock for deterministic transitions.
@ -244,6 +244,7 @@ Working backlog for **Epic 5 — Slice 2** ([NPC archetypes and telegraphs](../d
- [x] Aggro'd NPC enters telegraph after `attackCooldownSeconds` elapses from last attack. - [x] Aggro'd NPC enters telegraph after `attackCooldownSeconds` elapses from last attack.
- [x] Windup duration matches `telegraphWindupSeconds` from behavior def. - [x] Windup duration matches `telegraphWindupSeconds` from behavior def.
- [x] Idle NPC with no aggro holder does not telegraph. - [x] Idle NPC with no aggro holder does not telegraph.
- [x] Full transition chain **`idle``aggro``telegraph_windup``attack_execute``recover`** reachable with holder set (deterministic injected clock).
**Landed ([NEO-93](https://linear.app/neon-sprawl/issue/NEO-93)):** **`INpcRuntimeStateStore`** + **`NpcRuntimeOperations.AdvanceAll`** — catalog-driven state machine, lazy tick with delta cap; plan [NEO-93-implementation-plan.md](NEO-93-implementation-plan.md). **Landed ([NEO-93](https://linear.app/neon-sprawl/issue/NEO-93)):** **`INpcRuntimeStateStore`** + **`NpcRuntimeOperations.AdvanceAll`** — catalog-driven state machine, lazy tick with delta cap; plan [NEO-93-implementation-plan.md](NEO-93-implementation-plan.md).

View File

@ -61,7 +61,7 @@
- Holder-only idle→aggro sync from **`IThreatStateStore`**; no proximity auto-aggro. - Holder-only idle→aggro sync from **`IThreatStateStore`**; no proximity auto-aggro.
- **`attack_execute`** chains instantly to **`recover`** within the same advance step (no player damage — NEO-95). - **`attack_execute`** chains instantly to **`recover`** within the same advance step (no player damage — NEO-95).
- Dev **`combat-targets-fixture`** resets runtime rows alongside HP + threat. - Dev **`combat-targets-fixture`** resets runtime rows alongside HP + threat.
- **17** NEO-93-focused tests green (`NpcRuntimeOperationsTests`, `InMemoryNpcRuntimeStateStoreTests`, fixture extension). - **22** NEO-93-focused tests green (`NpcRuntimeOperationsTests`, `InMemoryNpcRuntimeStateStoreTests`, fixture extension).
## Technical approach ## Technical approach

View File

@ -4,20 +4,22 @@
**Scope:** Branch `NEO-93-npc-behavior-state-machine-lazy-tick` vs `origin/main` — NPC runtime state machine, lazy tick advance, tests, and docs **Scope:** Branch `NEO-93-npc-behavior-state-machine-lazy-tick` vs `origin/main` — NPC runtime state machine, lazy tick advance, tests, and docs
**Base:** `origin/main` **Base:** `origin/main`
**Follow-up:** Code review suggestions addressed (backlog in-scope/AC alignment, AAA test layout, single-call multi-phase test, plan test count, windup→recover write simplification).
## Verdict ## Verdict
**Approve with nits** **Approve with nits**
## Summary ## Summary
This branch delivers the E5M2-07 server spine: `INpcRuntimeStateStore`, thread-safe in-memory rows, and `NpcRuntimeOperations.AdvanceAll` driving the prototype **`idle``aggro``telegraph_windup``attack_execute``recover`** loop from catalog timings and `IThreatStateStore` holders. Kickoff decisions are honored — holder-only aggro sync, no HTTP wire, instant `attack_execute` stub, comment-only telemetry hooks, and dev fixture reset alongside HP + threat. Tests cover the acceptance paths (cooldown, windup, full melee cycle, holder clear, archetype spot checks, delta cap) plus store and fixture integration; **18** focused tests passed locally. Risk is low for merge; remaining items are doc parity and test-layout polish. This branch delivers the E5M2-07 server spine: `INpcRuntimeStateStore`, thread-safe in-memory rows, and `NpcRuntimeOperations.AdvanceAll` driving the prototype **`idle``aggro``telegraph_windup``attack_execute``recover`** loop from catalog timings and `IThreatStateStore` holders. Kickoff decisions are honored — holder-only aggro sync, no HTTP wire, instant `attack_execute` stub, comment-only telemetry hooks, and dev fixture reset alongside HP + threat. Tests cover the acceptance paths (cooldown, windup, full melee cycle, single-call multi-phase advance, holder clear, archetype spot checks, delta cap) plus store and fixture integration; **22** focused tests passed locally.
## Documentation checked ## Documentation checked
| Path | Result | | Path | Result |
|------|--------| |------|--------|
| `docs/plans/NEO-93-implementation-plan.md` | **Matches** — store, ops, state machine, fixture, DI, tests, and reconciliation section align with the diff. | | `docs/plans/NEO-93-implementation-plan.md` | **Matches** — store, ops, state machine, fixture, DI, tests, and reconciliation section align with the diff. |
| `docs/plans/E5M2-prototype-backlog.md` (E5M2-07) | **Partially matches** — AC checkboxes and landed note updated; **In scope** still says advance is “called from snapshot GET handler” (NEO-94 wires that; NEO-93 is engine-only). Plans fourth AC (full transition chain) is not mirrored in backlog AC list. | | `docs/plans/E5M2-prototype-backlog.md` (E5M2-07) | **Matches** — AC checkboxes (including full-chain), landed note, and engine-only in-scope bullet align with kickoff and implementation. |
| `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | **Matches** — E5M2-07 landed note and runtime state machine section present. | | `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | **Matches** — E5M2-07 landed note and runtime state machine section present. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E5.M2 row includes **NEO-93 landed** note and plan/README links. | | `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E5.M2 row includes **NEO-93 landed** note and plan/README links. |
| `docs/decomposition/modules/client_server_authority.md` | **Matches** — server-owned behavior state; no client projection in this slice (NEO-94 deferred). | | `docs/decomposition/modules/client_server_authority.md` | **Matches** — server-owned behavior state; no client projection in this slice (NEO-94 deferred). |
@ -29,15 +31,15 @@ None.
## Suggestions ## Suggestions
1. **Align E5M2-07 backlog “In scope” bullet** — Change “called from snapshot GET handler” to **engine + DI only in NEO-93; NEO-94 invokes `AdvanceAll` from `GET /game/world/npc-runtime-snapshot`** so the backlog matches the adopted kickoff decision and the implementation plan. 1. ~~**Align E5M2-07 backlog “In scope” bullet** — Change “called from snapshot GET handler” to **engine + DI only in NEO-93; NEO-94 invokes `AdvanceAll` from `GET /game/world/npc-runtime-snapshot`** so the backlog matches the adopted kickoff decision and the implementation plan.~~ **Done.** E5M2-07 in-scope bullet updated.
2. **Add full-chain AC to E5M2-07 backlog** — The implementation plan checks “full transition chain reachable”; add the same checkbox to the backlog AC list for traceability with Linear/E5M2-07. 2. ~~**Add full-chain AC to E5M2-07 backlog** — The implementation plan checks “full transition chain reachable”; add the same checkbox to the backlog AC list for traceability with Linear/E5M2-07.~~ **Done.** Fourth AC checkbox added to E5M2-07 backlog.
3. **AAA layout in new tests** — Several `NpcRuntimeOperationsTests` methods call `TryGet` in **Act** (e.g. `AdvanceAll_ShouldCompleteMeleeWindupDuration_BeforeAttackExecute` performs a mid-Act read and a second `Advance`). Move verification reads to **Assert** per [csharp-style](../../.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert). Non-blocking but worth fixing before NEO-94 adds more tests on this file. 3. ~~**AAA layout in new tests** — Several `NpcRuntimeOperationsTests` methods call `TryGet` in **Act** (e.g. `AdvanceAll_ShouldCompleteMeleeWindupDuration_BeforeAttackExecute` performs a mid-Act read and a second `Advance`). Move verification reads to **Assert** per [csharp-style](../../.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert). Non-blocking but worth fixing before NEO-94 adds more tests on this file.~~ **Done.** `TryGet` moved to Assert; windup test split into two facts.
4. **Optional: single-call multi-phase test** — Add one test that advances from aggro entry to second telegraph in **one** `AdvanceAll` call (e.g. `T0``T0+7.5`) to lock the inner `while (cursor < windowEnd)` multi-step loop independently of sequential polls. 4. ~~**Optional: single-call multi-phase test** — Add one test that advances from aggro entry to second telegraph in **one** `AdvanceAll` call (e.g. `T0` → `T0+7.5`) to lock the inner `while (cursor < windowEnd)` multi-step loop independently of sequential polls.~~ **Done.** `AdvanceAll_ShouldRunFullMeleeCycle_ToSecondTelegraph_InSingleAdvanceCall` added.
## Nits ## Nits
- Nit: `TelegraphWindup` case writes `AttackExecute` then immediately overwrites with `Recover`; the intermediate write is never observable — harmless but could be a single `Recover` write until NEO-95 needs a visible execute phase. - ~~Nit: `TelegraphWindup` case writes `AttackExecute` then immediately overwrites with `Recover`; the intermediate write is never observable — harmless but could be a single `Recover` write until NEO-95 needs a visible execute phase.~~ **Done.** Windup completion writes `Recover` directly with comment for logical `attack_execute`.
- Nit: Plan reconciliation cites **17** NEO-93-focused tests; local filter run reports **18** (theory counts as two cases) — trivial count drift. - ~~Nit: Plan reconciliation cites **17** NEO-93-focused tests; local filter run reports **18** (theory counts as two cases) — trivial count drift.~~ **Done.** Plan reconciliation updated to **18**.
- Nit: `LastAdvancedUtc` is an unlocked property on the store (same pattern as typical session singletons); acceptable for prototype poll-driven advance. - Nit: `LastAdvancedUtc` is an unlocked property on the store (same pattern as typical session singletons); acceptable for prototype poll-driven advance.
## Verification ## Verification