NEO-96: add E5M2-10 NPC telemetry hook sites implementation plan

pull/135/head
VinPropane 2026-05-29 20:47:21 -04:00
parent 5994e58bb7
commit dedd9a9c18
1 changed files with 129 additions and 0 deletions

View File

@ -0,0 +1,129 @@
# NEO-96 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-96 |
| **Title** | E5M2-10: Slice 2 NPC telemetry hook sites |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-96/e5m2-10-slice-2-npc-telemetry-hook-sites |
| **Module** | [E5.M2 — NpcAiAndBehaviorProfiles](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md) · Epic 5 Slice 2 · backlog **E5M2-10** |
| **Branch** | `NEO-96-e5m2-10-npc-telemetry-hooks` |
| **Precursor** | [NEO-93](https://linear.app/neon-sprawl/issue/NEO-93) — NPC runtime state machine + lazy tick (**Done** on `main`; left `(NEO-96)` placeholder comments) |
| **Pattern** | [NEO-64](https://linear.app/neon-sprawl/issue/NEO-64) / [NEO-84](https://linear.app/neon-sprawl/issue/NEO-84) — comment-only engine anchors + `TODO(E9.M1): catalog emit` + planned payload fields |
| **Client counterpart** | None — server-only comment hooks; Godot poll/HUD is [NEO-97](https://linear.app/neon-sprawl/issue/NEO-97) |
## Kickoff clarifications
| Topic | Question | Agent recommendation | Answer |
|--------|----------|----------------------|--------|
| **Runtime behavior** | Comments-only vs dev `ILogger`? | **Comments-only**`TODO(E9.M1)`; no production logging (NEO-64/NEO-84 precedent). | **Adopted** (repo precedent; not asked). |
| **Hook anchor** | Engine only vs engine + snapshot HTTP API? | **Engine only** in **`NpcRuntimeOperations`** — snapshot GET calls `AdvanceAll`; no duplicate API-layer hooks (NEO-56/NEO-64 precedent). | **Adopted** (repo precedent; not asked). |
| **`telegraph_fired` timing** | At telegraph start vs windup complete / attack resolve? | **At each `telegraph_windup` entry** — aggro→telegraph and recover→telegraph (E5M2 backlog: “at telegraph start”). | **Adopted** (backlog AC). |
| **`npc_state_transition` scope** | Every `WriteState` commit including holder clear → idle? | **Yes** — emit anchor on every committed state change, including **`WriteIdle`** when aggro holder clears. | **Adopted** (backlog: “on state transition commit”). |
| **Manual QA doc** | `docs/manual-qa/NEO-96.md`? | **No** — no client-facing change; NEO-93 server-only pattern (NEO-64 added manual QA for gather hooks with broader doc surface). | **User:** no manual QA — not client-facing. |
## Goal, scope, and out-of-scope
**Goal:** Formalize **comment-only** E9.M1 telemetry hook sites for Epic 5 Slice 2 product events **`telegraph_fired`** and **`npc_state_transition`** on the server-authoritative NPC runtime path.
**In scope (from Linear + [E5M2-10](E5M2-prototype-backlog.md#e5m2-10--slice-2-npc-telemetry-hook-sites)):**
- Replace NEO-93 `(NEO-96)` stub comments in **`NpcRuntimeOperations`** with full NEO-84-style blocks: event name, **`TODO(E9.M1): catalog emit`**, planned payload fields.
- **`telegraph_fired`** at each **`telegraph_windup`** entry (telegraph start).
- **`npc_state_transition`** at every state commit (idle↔aggro, phase transitions, holder clear → idle).
- **`server/README.md`** NPC telemetry subsection + [E5_M2 module](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md) landed note.
- E5M2 backlog acceptance checkboxes + alignment register bump when complete.
**Out of scope (from Linear + backlog):**
- Production telemetry ingest, dashboards (**E9.M1**).
- `ILogger` / metrics / dev-only log lines.
- Godot / client hooks ([NEO-97](https://linear.app/neon-sprawl/issue/NEO-97)).
- Duplicate hook comments in **`NpcRuntimeSnapshotWorldApi`** or **`NpcAttackOperations`** (engine orchestration layer only).
- `docs/manual-qa/NEO-96.md` (kickoff decision).
## Acceptance criteria checklist
- [ ] **`TODO(E9.M1)`** markers present at **`telegraph_fired`** and **`npc_state_transition`** hook sites in NPC runtime path.
- [ ] Hook names match [epic_05 Slice 2](../decomposition/epics/epic_05_pve_combat.md#slice-2---npc-archetypes-and-telegraphs) vocabulary (`telegraph_fired`, `npc_state_transition`).
- [ ] No runtime behavior change beyond comments / doc cross-links.
## Technical approach
### Authoritative surface
**`NpcRuntimeOperations.AdvanceAll` → `AdvanceOne`** — all prototype NPC state transitions and telegraph scheduling funnel here. **`GET /game/world/npc-runtime-snapshot`** invokes **`AdvanceAll`** before read ([NEO-94](https://linear.app/neon-sprawl/issue/NEO-94)); no HTTP-layer hooks.
### Hook site A — `telegraph_fired`
Immediately after **`WriteState(..., TelegraphWindup, ...)`** (both paths):
1. **aggro → telegraph_windup** — cooldown elapsed while holder set.
2. **recover → telegraph_windup** — post-attack cooldown elapsed while holder set.
Comment block names future E9.M1 event **`telegraph_fired`**. **`TODO(E9.M1): catalog emit`** — once per telegraph start (each windup cycle).
Planned payload fields: `npcInstanceId`, `telegraphId`, `behaviorDefId`, `archetypeKind`, `aggroHolderPlayerId`, `windupStartedUtc`, `telegraphWindupSeconds`.
### Hook site B — `npc_state_transition`
Immediately after every **`WriteState`** / **`WriteIdle`** commit:
| Transition | `fromState``toState` |
|------------|-------------------------|
| Holder set while idle | `idle``aggro` |
| Cooldown complete | `aggro``telegraph_windup` |
| Windup complete | `telegraph_windup``recover` (logical attack; no visible `attack_execute` phase) |
| Defensive `AttackExecute` branch | `attack_execute``recover` |
| Recover cooldown complete | `recover``telegraph_windup` |
| Holder cleared / fixture reset | any → `idle` |
Comment block names future E9.M1 event **`npc_state_transition`**. **`TODO(E9.M1): catalog emit`** — once per committed transition.
Planned payload fields: `npcInstanceId`, `behaviorDefId`, `fromState`, `toState`, `phaseStartedUtc`, optional `aggroHolderPlayerId`, optional `telegraphId` when entering/leaving windup.
**Note:** At recover→telegraph, emit **both** `npc_state_transition` and `telegraph_fired` (state commit then telegraph start) — analogous to gather success + depletion hooks sharing a path but naming distinct catalog events.
### Cross-references
1. **`NpcRuntimeOperations.cs`** — extend class summary with NEO-96 telemetry hook anchor (NEO-84 style).
2. **`server/README.md`** — new **NEO-96 NPC telemetry hooks** subsection under NPC runtime behavior (after NEO-93 block).
3. **`E5_M2_NpcAiAndBehaviorProfiles.md`** — NEO-96 landed line when complete.
4. **`E5M2-prototype-backlog.md`** — E5M2-10 checkboxes + landed note.
5. **`documentation_and_implementation_alignment.md`** — E5.M2 row bump when E5M2-10 lands.
### NEO-93 placeholder replacement
Replace terse `// telemetry: … (NEO-96)` stubs with full `--- Telemetry hook site (NEO-96): … ---` blocks matching **`CombatOperations`** / **`GatherOperations`** formatting.
## Files to add
| Path | Purpose |
|------|---------|
| `docs/plans/NEO-96-implementation-plan.md` | This plan. |
## Files to modify
| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Game/Npc/NpcRuntimeOperations.cs` | NEO-96: formalize `telegraph_fired` + `npc_state_transition` comment-only hook sites at transition/telegraph anchors; extend class summary. |
| `server/README.md` | Document NPC runtime telemetry hook sites + payload notes; link plan and epic vocabulary. |
| `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | NEO-96 landed note under Related implementation slices. |
| `docs/plans/E5M2-prototype-backlog.md` | E5M2-10 acceptance checkboxes + landed note when complete. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E5.M2 alignment register — E5M2-10 complete when shipped. |
## Tests
| File | Coverage |
|------|----------|
| *(none — no new or changed test files)* | Comment-only change; existing **`NpcRuntimeOperationsTests`** continue to verify state machine behavior. No runtime/API contract change to assert. |
## Open questions / risks
| Question / risk | Agent recommendation | Status |
|-----------------|---------------------|--------|
| **Linear blockedBy NEO-93** | Proceed — **Done** on `main`; placeholders already in place. | **adopted** |
| **Dual emit on recover→telegraph** | Accept — distinct catalog events at same code point (state transition + telegraph start). | **adopted** |
| **No manual QA doc** | Accept per kickoff — server-only comments; verify via code review + existing unit tests green. | **adopted** (kickoff) |
| **No new unit tests for comments** | Accept — NEO-64/NEO-84 did not add comment-assertion tests; behavior unchanged. | **adopted** |