NEO-88: Add implementation plan for NPC behavior catalog load.

Kickoff plan mirrors NEO-77 fail-fast loader pattern with CI-parity
validation for the three frozen E5M2 behavior defs; registry deferred to NEO-89.
pull/125/head
VinPropane 2026-05-25 17:06:23 -04:00
parent b9e4aebb20
commit 7730bde353
1 changed files with 130 additions and 0 deletions

View File

@ -0,0 +1,130 @@
# NEO-88 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-88 |
| **Title** | E5M2-02: Server NPC behavior catalog load (fail-fast) |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-88/e5m2-02-server-npc-behavior-catalog-load-fail-fast |
| **Module** | [E5.M2 — NpcAiAndBehaviorProfiles](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md) · Epic 5 Slice 2 · backlog **E5M2-02** |
| **Branch** | `NEO-88-e5m2-server-npc-behavior-catalog-load` |
| **Precursor** | [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) — frozen three-behavior catalog + CI gates (**Done** on `main`) |
| **Pattern** | [NEO-77](https://linear.app/neon-sprawl/issue/NEO-77) — fail-fast catalog loader + `ContentPathsOptions` + eager `Program.cs` resolve; strict split before registry ticket |
| **Blocks** | [NEO-89](https://linear.app/neon-sprawl/issue/NEO-89) — `INpcBehaviorDefinitionRegistry` + DI (E5M2-03) |
## Kickoff clarifications
| Topic | Question | Agent recommendation | Answer |
|--------|----------|----------------------|--------|
| **NEO-88 vs NEO-89 (DI scope)** | Ship `INpcBehaviorDefinitionRegistry` on this story, or only load + `NpcBehaviorDefinitionCatalog`? | **Strict split** — NEO-88: loader + `NpcBehaviorDefinitionCatalog` + eager fail-fast boot; [NEO-89](https://linear.app/neon-sprawl/issue/NEO-89) adds injectable `INpcBehaviorDefinitionRegistry` (E5M2-03). Rationale: E5M2 backlog separates E5M2-02/03; mirrors NEO-77/NEO-79. | **User:** `NpcBehaviorDefinitionCatalog` singleton only (kickoff). |
| **Startup failure tests** | Loader unit tests only, or also `WebApplicationFactory` host boot failure? | **Both** — mirror `AbilityDefinitionCatalogLoaderTests` (NEO-77). | **User:** loader AAA + host boot tests (kickoff). |
| **Runtime validation** | In-process C# vs subprocess `validate_content.py`? | **In-process C#** mirroring `scripts/validate_content.py` (Draft 2020-12 + E5M2 three-id gate + leash > aggro numeric gate). Rationale: NEO-77 precedent; no cross-catalog refs in Slice 2. | **Adopted** — NEO-77 precedent; no separate question. |
| **Client counterpart** | Godot issue for this story? | **None** — server-only load; player-visible work starts at E5M2-04+ / [NEO-90](https://linear.app/neon-sprawl/issue/NEO-90) / [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98). | **Adopted** (E5M2-02 out of scope). |
## Goal, scope, and out-of-scope
**Goal:** On host startup, load all `content/npc-behaviors/*_npc_behaviors.json` catalogs (validated in CI per [NEO-87](NEO-87-implementation-plan.md)), build an in-memory **`NpcBehaviorDefinitionCatalog`**, and **refuse to listen** when any file is missing, malformed, has duplicate `id`, fails the prototype E5M2 three-id gate, or violates the leash > aggro numeric invariant.
**In scope (from Linear + [E5M2-02](E5M2-prototype-backlog.md#e5m2-02--server-npc-behavior-catalog-load-fail-fast)):**
- Loader + catalog types under `server/NeonSprawl.Server/Game/Npc/`.
- Configurable npc-behaviors directory and schema path (`ContentPathsOptions` extension).
- CI-parity validation: `schemaVersion`, row schema (`npc-behavior-def`), duplicate `id`, E5M2 three-id allowlist gate, **`leashRadius > aggroRadius`** per row.
- DI registration of **`NpcBehaviorDefinitionCatalog`** singleton; eager resolve in `Program.cs`.
- Unit + host startup tests (AAA).
- `server/README.md` section.
**Out of scope (from Linear):**
- **`INpcBehaviorDefinitionRegistry`** / DI registry (**NEO-89**, E5M2-03).
- HTTP projection (**NEO-90**, E5M2-04), NPC runtime tick, dummy migration (E5M2-05+).
## Acceptance criteria checklist
- [ ] Server refuses boot when NPC behavior catalog invalid (missing dir, no `*_npc_behaviors.json`, bad JSON, `schemaVersion` ≠ 1, schema violation, duplicate `id`, E5M2 gate failure, leash ≤ aggro).
- [ ] `NpcBehaviorDefinitionCatalog` resolves prototype rows by behavior `id` (e.g. `prototype_melee_pressure`) with all schema fields on the row DTO.
- [ ] Unit tests (AAA): loader happy path + failure modes; host resolves catalog on success; host fails startup on invalid catalog directory.
- [ ] Success log includes **behavior count**, **catalog directory**, and **file count** (Information).
## Technical approach
1. **`Game/Npc/` row DTO** — `NpcBehaviorDefRow` (`Id`, `DisplayName`, `ArchetypeKind`, `MaxHp`, `AggroRadius`, `LeashRadius`, `TelegraphWindupSeconds`, `AttackDamage`, `AttackCooldownSeconds`). `ArchetypeKind` stored as `string` matching schema enum values (mirror `AbilityDefRow.AbilityKind` pattern).
2. **`NpcBehaviorDefinitionCatalogLoader.Load(npcBehaviorsDirectory, schemaPath, logger)`** — single-pass load mirroring `validate_content.py` `_validate_npc_behavior_catalogs` + `_prototype_e5m2_npc_behavior_gate` + `_prototype_e5m2_npc_behavior_numeric_gate`:
- Enumerate `*_npc_behaviors.json` (top-level only), ordered by path.
- Build **JsonSchema.Net** evaluator from `npc-behavior-def.schema.json`.
- Per file: parse JSON; require `schemaVersion == 1` and top-level `npcBehaviors` array.
- Per row: validate against schema; on clean rows track `id`; build `NpcBehaviorDefRow`; reject duplicate `id` across files.
- Run **`PrototypeE5M2NpcBehaviorCatalogRules.TryGetE5M2GateError`** (mirror `PROTOTYPE_E5M2_NPC_BEHAVIOR_IDS` / `_prototype_e5m2_npc_behavior_gate`).
- Run **`PrototypeE5M2NpcBehaviorCatalogRules.TryGetNumericGateError`** (mirror `_prototype_e5m2_npc_behavior_numeric_gate`: `leashRadius > aggroRadius`).
- Throw **`InvalidOperationException`** with aggregated, sorted messages prefixed `NPC behavior catalog validation failed:`.
3. **`NpcBehaviorDefinitionCatalog`** — immutable snapshot: `NpcBehaviorsDirectory`, `ById`, `DistinctBehaviorCount`, `CatalogJsonFileCount`; `TryGetBehavior(id, out NpcBehaviorDefRow?)` for lookups.
4. **`NpcBehaviorCatalogPathResolution`** — `TryDiscoverNpcBehaviorsDirectory`, `ResolveNpcBehaviorsDirectory`, `ResolveNpcBehaviorDefSchemaPath` (parent `content/schemas/` layout when unset).
5. **`PrototypeE5M2NpcBehaviorCatalogRules`** — frozen three behavior ids; sync comment to `scripts/validate_content.py` `PROTOTYPE_E5M2_NPC_BEHAVIOR_IDS`. Ids: `prototype_melee_pressure`, `prototype_ranged_control`, `prototype_elite_mini_boss` (same set as [E5.M2 freeze table](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md#prototype-slice-2-freeze-e5m2-01)).
6. **`NpcBehaviorCatalogServiceCollectionExtensions.AddNpcBehaviorDefinitionCatalog`** — bind `ContentPathsOptions`; register **`NpcBehaviorDefinitionCatalog`** singleton factory that resolves paths and calls loader. No dependency on ability/item catalogs (Slice 2 behavior rows have no cross-ref fields).
7. **`Program.cs`** — `AddNpcBehaviorDefinitionCatalog(configuration)` alongside existing catalog registrations; after `Build()`, `GetRequiredService<NpcBehaviorDefinitionCatalog>()` with other eager catalog resolves.
8. **Tests** — temp-dir fixtures copying repo `npc-behavior-def.schema.json`; valid three-behavior JSON matching [prototype_npc_behaviors.json](../../content/npc-behaviors/prototype_npc_behaviors.json); negative cases (duplicate id, wrong/missing E5M2 roster, bad numeric fields, empty `displayName`, `schemaVersion` ≠ 1, missing dir/schema, no `*_npc_behaviors.json`, invalid JSON, `leashRadius <= aggroRadius`); `InMemoryWebApplicationFactory` host test + `WebApplicationFactory` with empty npc-behaviors dir fails startup.
9. **Test factories** — pin `Content:NpcBehaviorsDirectory` in `InMemoryWebApplicationFactory` and other `WebApplicationFactory` subclasses that already pin recipes/items/abilities (same pattern as NEO-77).
## Files to add
| Path | Purpose |
|------|---------|
| `server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefRow.cs` | Load-time `NpcBehaviorDef` row DTO. |
| `server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionCatalog.cs` | In-memory catalog + `TryGetBehavior`. |
| `server/NeonSprawl.Server/Game/Npc/NpcBehaviorDefinitionCatalogLoader.cs` | Disk I/O, schema validation, E5M2 gates. |
| `server/NeonSprawl.Server/Game/Npc/NpcBehaviorCatalogPathResolution.cs` | Directory/schema discovery and config resolution. |
| `server/NeonSprawl.Server/Game/Npc/PrototypeE5M2NpcBehaviorCatalogRules.cs` | Frozen three ids + numeric gate (sync comment to Python). |
| `server/NeonSprawl.Server/Game/Npc/NpcBehaviorCatalogServiceCollectionExtensions.cs` | `AddNpcBehaviorDefinitionCatalog` DI registration. |
| `server/NeonSprawl.Server.Tests/Game/Npc/NpcBehaviorCatalogTestPaths.cs` | Repo `content/npc-behaviors` + schema discovery for tests. |
| `server/NeonSprawl.Server.Tests/Game/Npc/NpcBehaviorDefinitionCatalogLoaderTests.cs` | AAA loader + host startup tests. |
| `docs/plans/NEO-88-implementation-plan.md` | This plan. |
## Files to modify
| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Game/Skills/ContentPathsOptions.cs` | Add `NpcBehaviorsDirectory`, `NpcBehaviorDefSchemaPath` under `Content` section. |
| `server/NeonSprawl.Server/Program.cs` | Register and eagerly resolve `NpcBehaviorDefinitionCatalog` at startup. |
| `server/NeonSprawl.Server/appsettings.json` | Optional empty `Content:NpcBehaviorsDirectory` / schema path keys for override documentation (mirror other catalogs). |
| `server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs` | Pin `Content:NpcBehaviorsDirectory` to discovered repo path so existing tests keep booting. |
| `server/NeonSprawl.Server.Tests/Game/PositionState/PostgresWebApplicationFactory.cs` | Pin npc-behaviors directory (same as recipes/items/abilities). |
| `server/NeonSprawl.Server.Tests/Game/Interaction/SalvageActivityDeniedRegistryWebApplicationFactory.cs` | Pin npc-behaviors directory. |
| `server/NeonSprawl.Server.Tests/Game/Skills/RefineActivityDeniedRegistryWebApplicationFactory.cs` | Pin npc-behaviors directory. |
| `server/NeonSprawl.Server.Tests/Game/Skills/MissionRewardDeniedRegistryWebApplicationFactory.cs` | Pin npc-behaviors directory. |
| `server/README.md` | New **NPC behavior catalog (`content/npc-behaviors`, NEO-88)** section (config keys, discovery, fail-fast, E5M2 parity with CI). |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E5.M2 row — note NEO-88 server load when implementation completes. |
| `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | Status line — E5M2-02 server load landed when complete. |
| `docs/plans/E5M2-prototype-backlog.md` | E5M2-02 checkboxes when implementation completes. |
## Tests
| Test file | What it covers |
|-----------|----------------|
| `server/NeonSprawl.Server.Tests/Game/Npc/NpcBehaviorDefinitionCatalogLoaderTests.cs` | **Unit:** valid three-behavior prototype catalog loads; `npcBehaviors` not array; schema violation (zero `aggroRadius`, empty `displayName`, invalid `maxHp`); duplicate `id` across files; E5M2 gate failure (missing id + extra/wrong id); numeric gate failure (`leashRadius <= aggroRadius`); `schemaVersion` ≠ 1; missing dir/schema; no `*_npc_behaviors.json` files; invalid JSON. **Host:** `InMemoryWebApplicationFactory` + `/health` + DI `NpcBehaviorDefinitionCatalog` count 3; `WebApplicationFactory` with empty npc-behaviors dir fails startup with actionable message. |
**Manual QA:** Skipped — no player-visible HTTP; acceptance is fully covered by AAA loader/host tests above.
## Open questions / risks
| Question / risk | Agent recommendation | Status |
|-----------------|----------------------|--------|
| **Constants drift vs Python** | Comment on `PrototypeE5M2NpcBehaviorCatalogRules` pointing at `scripts/validate_content.py` `PROTOTYPE_E5M2_NPC_BEHAVIOR_IDS` / gate helpers. | **adopted** |
| **Test cwd** | `InMemoryWebApplicationFactory` must set `Content:NpcBehaviorsDirectory` explicitly (do not rely on discovery alone in CI). | **adopted** |
| **Registry consumers** | Runtime NPC logic uses `INpcBehaviorDefinitionRegistry` in NEO-89+; this story only exposes `NpcBehaviorDefinitionCatalog` for boot + direct test lookup. | **adopted** |
| **Alpha/beta dummies still active** | Expected — catalog lands before E5M2-05 migrates combat targets to NPC instance ids. | **adopted** |
| **Numeric gate not in JSON Schema** | Enforce in C# gate class (same as Python `_prototype_e5m2_npc_behavior_numeric_gate`); schema only requires strictly positive radii. | **adopted** |
## Decisions (kickoff)
- **`NpcBehaviorDefinitionCatalog` singleton only** — defer `INpcBehaviorDefinitionRegistry` to NEO-89 (user confirmed kickoff).
- **In-process validation** + **loader and host boot tests** per NEO-77 precedent.
- **No cross-catalog checks** — behavior rows have no ability/item refs in Slice 2; loader has no catalog dependencies.