# NEO-92 — Implementation plan

## Story reference

| Field | Value |
|--------|--------|
| **Key** | NEO-92 |
| **Title** | E5M2-06: AggroRule engine + `IThreatStateStore` |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-92/e5m2-06-aggrorule-engine-ithreatstatestore |
| **Module** | [E5.M2 — NpcAiAndBehaviorProfiles](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md) · Epic 5 Slice 2 · backlog **E5M2-06** |
| **Branch** | `NEO-92` |
| **Precursor** | [NEO-91](https://linear.app/neon-sprawl/issue/NEO-91) — `PrototypeNpcRegistry` + combat-target migration (**Done** on `main`) |
| **Pattern** | [NEO-80](https://linear.app/neon-sprawl/issue/NEO-80) — in-memory session store + DI; [NEO-82](https://linear.app/neon-sprawl/issue/NEO-82) — cast-layer player correlation |
| **Blocks** | [NEO-93](https://linear.app/neon-sprawl/issue/NEO-93) — NPC behavior state machine (needs aggro holder) |
| **Client counterpart** | None for this slice — threat projection lands in [NEO-94](https://linear.app/neon-sprawl/issue/NEO-94) (`npc-runtime-snapshot`); Bruno + automated tests verify server behavior |

## Kickoff clarifications

| Topic | Question | Agent recommendation | Answer |
|--------|----------|----------------------|--------|
| **Aggro acquire hook** | Where to run acquire after a damaging cast? | **`AbilityCastApi` after successful `TryResolve` when `DamageDealt > 0`** — player id at cast layer (NEO-30); keep `CombatOperations` engine-pure. | **User:** `AbilityCastApi`. |
| **Leash clear trigger** | When to invoke `TryClearOnLeash` at runtime? | **Player move (`POST /move` + `/move-stream`) and before acquire on cast** — deterministic clear without NEO-93 snapshot poll. | **User:** move + cast. |
| **Re-aggro model** | Proximity auto-aggro vs first-hit in NEO-92? | **First damaging cast only** — E5M2-06 AC; defer proximity auto-aggro to NEO-93. | **User:** first-hit only. |
| **Holder semantics** | Behavior when NPC already has a holder? | **No-op** — first hit wins until leash clear (prototype single-player). | **User:** no-op. |

## Goal, scope, and out-of-scope

**Goal:** Deterministic aggro — NPC acquires a **holder** (`playerId`) on the **first damaging player cast**; holder **clears** when that player leaves the behavior catalog **`leashRadius`** from the NPC world anchor.

**In scope (from Linear + [E5M2-06](E5M2-prototype-backlog.md#e5m2-06--aggrorule-engine--ithreatstatestore)):**

- **`IThreatStateStore`** + in-memory implementation keyed by NPC instance id.
- **`AggroOperations.TryAcquire`** / **`TryClearOnLeash`** using player **`PositionState`**, **`PrototypeNpcRegistry`** anchors, and **`INpcBehaviorDefinitionRegistry`** `leashRadius`.
- Hook acquire from **`AbilityCastApi`** after successful combat resolve with **`DamageDealt > 0`**.
- Wire leash clear on **`PositionStateApi`** move accept paths and before acquire on cast.
- Extend dev **`combat-targets-fixture`** to reset threat rows (Bruno isolation).
- Unit + integration tests (AAA).

**Out of scope (from Linear + backlog):**

- NPC attack loop, telegraphs, behavior state machine ([NEO-93](https://linear.app/neon-sprawl/issue/NEO-93)).
- HTTP DTO projection of threat state ([NEO-94](https://linear.app/neon-sprawl/issue/NEO-94)).
- Proximity auto-aggro inside `aggroRadius` without a damaging cast.
- Godot / client HUD ([NEO-97](https://linear.app/neon-sprawl/issue/NEO-97)).
- `docs/manual-qa/NEO-92.md` — server-only; Bruno + automated tests (NEO-91 pattern).

## Acceptance criteria checklist

- [x] First damaging cast on NPC sets aggro holder to casting player id.
- [x] Holder clears when player leaves `leashRadius` (deterministic horizontal distance check).
- [x] Re-aggro inside radius after clear follows same first-hit rule (no proximity auto-aggro).

## Technical approach

1. **Threat store (`Game/Npc/`)**
   - **`ThreatStateSnapshot`**: `AggroHolderPlayerId` (`string?`, lowercase trimmed route id or `null` when empty).
   - **`IThreatStateStore`**: `TryGet(npcInstanceId, out snapshot)`, `TrySetHolder(npcInstanceId, playerId?)`, `TryClearHolder(npcInstanceId)` (or single `TrySetHolder` with null).
   - **`InMemoryThreatStateStore`**: thread-safe lazy rows for known **`PrototypeNpcRegistry`** ids only; unknown ids fail `TryGet`/`TrySet`.
   - **`ThreatStateServiceCollectionExtensions.AddThreatStateStore`**: register in-memory singleton (prototype session store — no Postgres).

2. **`AggroOperations` (static, `Game/Npc/`)**
   - **`TryAcquire(npcInstanceId, playerId, threatStore)`** → `bool` (true when holder was empty and now set):
     - Normalize ids (trim + lowercase).
     - Gate npc id via **`PrototypeNpcRegistry.TryGet`**.
     - If current holder non-empty → **no-op**, return `false`.
     - Else set holder to `playerId`, return `true`.
   - **`TryClearOnLeashForPlayer(playerId, positions, behaviorRegistry, threatStore)`**:
     - Load player position via **`IPositionStateStore.TryGetPosition`**; no-op if missing.
     - For each prototype NPC instance id, if holder == `playerId`:
       - Resolve behavior def → **`LeashRadius`**.
       - Compare **`HorizontalReach.HorizontalDistance`** (player X/Z vs NPC anchor X/Z).
       - Clear holder when distance **>** `leashRadius` (inclusive boundary still leashed — mirrors lock-radius tests).
   - **`TryClearOnLeashForNpc(...)`** (optional internal helper for unit tests): single-NPC variant.

3. **Cast hook (`AbilityCastApi`)**
   - Inject **`IThreatStateStore`**.
   - After combat success and **before** cooldown commit:
     - If **`combatResult.DamageDealt > 0`**: call **`TryClearOnLeashForPlayer`** then **`TryAcquire(lookupKey, id.Trim(), threatStore)`**.
   - Zero-damage abilities (guard/dash) skip acquire.

4. **Move hook (`PositionStateApi`)**
   - Inject **`IThreatStateStore`** + **`INpcBehaviorDefinitionRegistry`** on **`POST /move`** and **`POST /move-stream`** success paths.
   - After **`TryApplyMoveTarget`** / stream apply succeeds, call **`TryClearOnLeashForPlayer`** for route `id`.

5. **Dev fixture (`CombatTargetFixtureApi`)**
   - Inject **`IThreatStateStore`**; after HP reset loop, clear all prototype NPC threat holders (fresh Bruno runs).

6. **DI wiring**
   - Call **`AddThreatStateStore()`** from **`Program.cs`** (or co-locate with NPC catalog extension).
   - No change to **`CombatOperations`** signature — aggro stays outside the combat engine.

7. **Docs**
   - **`server/README.md`** — brief threat/aggro section: first-hit acquire, leash clear triggers, fixture reset, no HTTP read yet (NEO-94).
   - Reconcile [E5M2-prototype-backlog.md](E5M2-prototype-backlog.md) E5M2-06 checkboxes when landed.

### Leash radii (frozen catalog, for test expectations)

| NPC instance | Behavior def | `leashRadius` | Anchor (X, Z) |
|--------------|--------------|---------------|---------------|
| `prototype_npc_melee` | `prototype_melee_pressure` | 16.0 | (-3, -3) |
| `prototype_npc_ranged` | `prototype_ranged_control` | 20.0 | (3, 3) |
| `prototype_npc_elite` | `prototype_elite_mini_boss` | 18.0 | (0, 0) |

**Example (melee):** dev player at origin `(0, 0, 0)` is ~4.24 m from melee anchor — inside leash 16. Move to `(20, 0, 20)` → distance ~32.5 > 16 → holder clears.

## Files to add

| Path | Purpose |
|------|---------|
| `server/NeonSprawl.Server/Game/Npc/ThreatStateSnapshot.cs` | Immutable aggro holder snapshot for one NPC instance. |
| `server/NeonSprawl.Server/Game/Npc/IThreatStateStore.cs` | Store contract keyed by NPC instance id. |
| `server/NeonSprawl.Server/Game/Npc/InMemoryThreatStateStore.cs` | Thread-safe in-memory prototype implementation. |
| `server/NeonSprawl.Server/Game/Npc/ThreatStateServiceCollectionExtensions.cs` | DI registration for `IThreatStateStore`. |
| `server/NeonSprawl.Server/Game/Npc/AggroOperations.cs` | `TryAcquire` + `TryClearOnLeashForPlayer` deterministic ops. |
| `server/NeonSprawl.Server.Tests/Game/Npc/AggroOperationsTests.cs` | AAA unit tests: acquire, no-op when held, leash clear, re-acquire after clear. |
| `server/NeonSprawl.Server.Tests/Game/Npc/InMemoryThreatStateStoreTests.cs` | AAA store tests: lazy rows, unknown id, clear/set. |
| `server/NeonSprawl.Server.Tests/Game/Npc/AggroThreatIntegrationTests.cs` | HTTP integration: cast acquire + move beyond leash clears holder via injected store. |
| `docs/plans/NEO-92-implementation-plan.md` | This plan. |

## Files to modify

| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Program.cs` | Register `AddThreatStateStore()`. |
| `server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs` | Inject threat store; leash clear + acquire after damaging resolve. |
| `server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs` | Leash clear on successful move + move-stream. |
| `server/NeonSprawl.Server/Game/Combat/CombatTargetFixtureApi.cs` | Reset threat holders in dev fixture alongside HP reset. |
| `server/README.md` | Document aggro acquire/clear behavior and fixture reset. |
| `server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs` | Assert holder set after pulse cast; zero-damage guard skips acquire. |
| `server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs` | Move beyond leash clears aggro holder (cast setup helper). |
| `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | Threat store landed note when complete. |
| `docs/plans/E5M2-prototype-backlog.md` | E5M2-06 acceptance checkboxes + landed note when complete. |

## Tests

| File | Coverage |
|------|----------|
| `AggroOperationsTests.cs` | First acquire sets holder; second acquire no-op; clear when distance > leashRadius; no clear when inside; re-acquire after clear on next TryAcquire. |
| `InMemoryThreatStateStoreTests.cs` | Unknown npc id fails; set/get/clear; thread-safe spot check if trivial. |
| `AggroThreatIntegrationTests.cs` | End-to-end: bind pulse → lock melee → cast → store has `dev-local-1`; move far → holder cleared; cast again → re-acquire. |
| `AbilityCastApiTests.cs` | Damaging cast sets holder (via store resolve from factory); guard cast leaves holder empty. |
| `MoveCommandApiTests.cs` | After aggro via cast, validated move outside leash clears holder. |

No Bruno changes required for AC (internal store not on wire); optional Bruno note in README only. No `docs/manual-qa/NEO-92.md`.

## Open questions / risks

| Question / risk | Agent recommendation | Status |
|-----------------|---------------------|--------|
| **Linear blockedBy NEO-91** | Proceed — NEO-91 **Done** on `main`. | **adopted** |
| **No HTTP read surface** | Accept — NEO-94 projects holder; tests inject `IThreatStateStore` from factory. | **adopted** |
| **Leash vs lock radius confusion** | Use catalog **`leashRadius`** for aggro clear; **`LockRadius` (6.0)** remains targeting-only. | **adopted** |
| **Multi-player holder steal** | Out of prototype scope; no-op when holder set (kickoff). | **adopted** |
| **Proximity re-aggro in NEO-93** | NEO-92 first-hit only; state machine may add radius checks later — do not implement proximity acquire here. | **deferred** |

## Reconciliation (implementation)

- **`IThreatStateStore`**, **`InMemoryThreatStateStore`**, and **`AggroOperations`** landed in `Game/Npc/`.
- Acquire wired in **`AbilityCastApi`** after damaging resolve; leash clear on **`PositionStateApi`** move paths and before acquire.
- Dev **`combat-targets-fixture`** clears threat holders alongside HP reset.
- Unit + integration tests cover acquire, leash clear, re-acquire, guard skip, and fixture reset.