neon-sprawl/docs/plans/NEO-77-implementation-plan.md

140 lines
13 KiB
Markdown

# NEO-77 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-77 |
| **Title** | E5M1-02: Server ability catalog load (fail-fast) |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-77/e5m1-02-server-ability-catalog-load-fail-fast |
| **Module** | [E5.M1 — CombatRulesEngine](../decomposition/modules/E5_M1_CombatRulesEngine.md) · Epic 5 Slice 1 · backlog **E5M1-02** |
| **Branch** | `NEO-77-e5m1-server-ability-catalog-load` |
| **Precursor** | [NEO-76](https://linear.app/neon-sprawl/issue/NEO-76) — frozen four-ability catalog + CI gates (**Done** on `main`) |
| **Pattern** | [NEO-51](https://linear.app/neon-sprawl/issue/NEO-51) / [NEO-66](https://linear.app/neon-sprawl/issue/NEO-66) — fail-fast catalog loader + `ContentPathsOptions` + eager `Program.cs` resolve; strict split before registry ticket |
| **Blocks** | [NEO-79](https://linear.app/neon-sprawl/issue/NEO-79) — `IAbilityDefinitionRegistry` + DI (E5M1-03) |
## Kickoff clarifications
| Topic | Question | Agent recommendation | Answer |
|--------|----------|----------------------|--------|
| **NEO-77 vs NEO-79 (DI scope)** | Ship `IAbilityDefinitionRegistry` on this story, or only load + `AbilityDefinitionCatalog`? | **Strict split** — NEO-77: loader + `AbilityDefinitionCatalog` + eager fail-fast boot; [NEO-79](https://linear.app/neon-sprawl/issue/NEO-79) adds injectable `IAbilityDefinitionRegistry` (E5M1-03). Rationale: E5M1 backlog separates E5M1-02/03; mirrors NEO-66/NEO-67. | **User:** `AbilityDefinitionCatalog` singleton only (kickoff). |
| **Startup failure tests** | Loader unit tests only, or also `WebApplicationFactory` host boot failure? | **Both** — mirror `ItemDefinitionCatalogLoaderTests` / `RecipeDefinitionCatalogLoaderTests`. | **User:** loader AAA + host boot tests (kickoff). |
| **`PrototypeAbilityRegistry`** | Migrate hotbar/cast allowlist reads this story? | **Leave unchanged** — E5M1-03 explicitly migrates call sites; NEO-77 is load-only. | **User:** unchanged (kickoff). |
| **Runtime validation** | In-process C# vs subprocess `validate_content.py`? | **In-process C#** mirroring `scripts/validate_content.py` (Draft 2020-12 + E5M1 four-id gate). Rationale: NEO-51 / NEO-66 precedent; no cross-catalog refs in Slice 1. | **Adopted** — NEO-51 precedent; no separate question. |
| **Lookup API on NEO-77** | How does “catalog resolves `AbilityDef` by `id`” land without NEO-79? | **`AbilityDefinitionCatalog.ById`** (read-only map) + `TryGetAbility(string id, out AbilityDefRow row)`; row carries damage + cooldown fields for NEO-79 consumers. | **Adopted** — NEO-66 `RecipeDefinitionCatalog` precedent. |
## Goal, scope, and out-of-scope
**Goal:** On host startup, load all `content/abilities/*_abilities.json` catalogs (validated in CI per [NEO-76](NEO-76-implementation-plan.md)), build an in-memory **`AbilityDefinitionCatalog`**, and **refuse to listen** when any file is missing, malformed, has duplicate `id`, or fails the prototype E5M1 four-id gate.
**In scope (from Linear + [E5M1-02](E5M1-prototype-backlog.md#e5m1-02--server-ability-catalog-load-fail-fast)):**
- Loader + catalog types under `server/NeonSprawl.Server/Game/Combat/`.
- Configurable abilities directory and schema path (`ContentPathsOptions` extension).
- CI-parity validation: `schemaVersion`, row schema (`ability-def`), duplicate `id`, E5M1 four-id allowlist gate.
- DI registration of **`AbilityDefinitionCatalog`** singleton; eager resolve in `Program.cs`.
- Unit + host startup tests (AAA).
- `server/README.md` section.
**Out of scope (from Linear):**
- **`IAbilityDefinitionRegistry`** / hotbar-cast migration from **`PrototypeAbilityRegistry`** (**NEO-79**, E5M1-03).
- HTTP projection (**NEO-78**, E5M1-04), combat resolution (**NEO-81+**).
## Acceptance criteria checklist
- [x] Server refuses boot when ability catalog invalid (missing dir, no `*_abilities.json`, bad JSON, `schemaVersion` ≠ 1, schema violation, duplicate `id`, E5M1 gate failure).
- [x] `AbilityDefinitionCatalog` resolves prototype rows by ability `id` (e.g. `prototype_pulse`, `prototype_burst`) with `baseDamage`, `cooldownSeconds`, optional `abilityKind`.
- [x] Unit tests (AAA): loader happy path + failure modes; host resolves catalog on success; host fails startup on invalid catalog directory.
- [x] Success log includes **ability count**, **catalog directory**, and **file count** (Information).
## Implementation reconciliation (shipped)
- **Loader:** [`AbilityDefinitionCatalogLoader`](../../server/NeonSprawl.Server/Game/Combat/AbilityDefinitionCatalogLoader.cs) — CI-parity validation via JsonSchema.Net + E5M1 gate.
- **Catalog:** [`AbilityDefinitionCatalog`](../../server/NeonSprawl.Server/Game/Combat/AbilityDefinitionCatalog.cs) + [`AbilityDefRow`](../../server/NeonSprawl.Server/Game/Combat/AbilityDefRow.cs) — `TryGetAbility` lookup by id.
- **Gate rules:** [`PrototypeE5M1AbilityCatalogRules`](../../server/NeonSprawl.Server/Game/Combat/PrototypeE5M1AbilityCatalogRules.cs) — sync comment to `scripts/validate_content.py` `PROTOTYPE_E5M1_ABILITY_IDS`.
- **DI / startup:** [`AbilityCatalogServiceCollectionExtensions`](../../server/NeonSprawl.Server/Game/Combat/AbilityCatalogServiceCollectionExtensions.cs) + eager resolve in [`Program.cs`](../../server/NeonSprawl.Server/Program.cs).
- **Tests:** [`AbilityDefinitionCatalogLoaderTests`](../../server/NeonSprawl.Server.Tests/Game/Combat/AbilityDefinitionCatalogLoaderTests.cs) — 14 AAA cases (loader + host).
- **Docs:** `server/README.md` ability catalog section; E5.M1 module doc + alignment register + E5M1 backlog E5M1-02 checkboxes.
- **Unchanged:** [`PrototypeAbilityRegistry`](../../server/NeonSprawl.Server/Game/AbilityInput/PrototypeAbilityRegistry.cs) — migration deferred to NEO-79.
## Technical approach
1. **`Game/Combat/` row DTO** — `AbilityDefRow` (`Id`, `DisplayName`, `BaseDamage`, `CooldownSeconds`, optional `AbilityKind` enum or string).
2. **`AbilityDefinitionCatalogLoader.Load(abilitiesDirectory, schemaPath, logger)`** — single-pass load mirroring `validate_content.py` `_validate_ability_catalogs` + `_prototype_e5m1_ability_gate`:
- Enumerate `*_abilities.json` (top-level only), ordered by path.
- Build **JsonSchema.Net** evaluator from `ability-def.schema.json`.
- Per file: parse JSON; require `schemaVersion == 1` and top-level `abilities` array.
- Per row: validate against schema; on clean rows track `id`; build `AbilityDefRow`; reject duplicate `id` across files.
- Run **`PrototypeE5M1AbilityCatalogRules.TryGetE5M1GateError`** (mirror `PROTOTYPE_E5M1_ABILITY_IDS` / `_prototype_e5m1_ability_gate` in `scripts/validate_content.py`).
- Throw **`InvalidOperationException`** with aggregated, sorted messages prefixed `Ability catalog validation failed:`.
3. **`AbilityDefinitionCatalog`** — immutable snapshot: `AbilitiesDirectory`, `ById`, `DistinctAbilityCount`, `CatalogJsonFileCount`; `TryGetAbility(id, out AbilityDefRow?)` for lookups.
4. **`AbilityCatalogPathResolution`** — `TryDiscoverAbilitiesDirectory`, `ResolveAbilitiesDirectory`, `ResolveAbilityDefSchemaPath` (parent `content/schemas/` layout when unset).
5. **`PrototypeE5M1AbilityCatalogRules`** — frozen four ability ids; sync comment to `scripts/validate_content.py` `PROTOTYPE_E5M1_ABILITY_IDS` / `_prototype_e5m1_ability_gate`. Ids: `prototype_pulse`, `prototype_guard`, `prototype_dash`, `prototype_burst` (same set as [`PrototypeAbilityRegistry`](../../server/NeonSprawl.Server/Game/AbilityInput/PrototypeAbilityRegistry.cs) — unchanged this story).
6. **`AbilityCatalogServiceCollectionExtensions.AddAbilityDefinitionCatalog`** — bind `ContentPathsOptions`; register **`AbilityDefinitionCatalog`** singleton factory that resolves paths and calls loader. No dependency on item/skill catalogs (Slice 1 abilities have no cross-ref fields).
7. **`Program.cs`** — `AddAbilityDefinitionCatalog(configuration)` alongside existing catalog registrations; after `Build()`, `GetRequiredService<AbilityDefinitionCatalog>()` with other eager catalog resolves.
8. **Tests** — temp-dir fixtures copying repo `ability-def.schema.json`; valid four-ability JSON matching [prototype_abilities.json](../../content/abilities/prototype_abilities.json); negative cases (duplicate id, wrong/missing E5M1 roster, bad `baseDamage` / `cooldownSeconds`, empty `displayName`, `schemaVersion` ≠ 1, missing dir/schema, no `*_abilities.json`, invalid JSON); `InMemoryWebApplicationFactory` host test + `WebApplicationFactory` with empty abilities dir fails startup.
9. **Test factories** — pin `Content:AbilitiesDirectory` in `InMemoryWebApplicationFactory` and other `WebApplicationFactory` subclasses that already pin recipes/items (same pattern as NEO-66).
## Files to add
| Path | Purpose |
|------|---------|
| `server/NeonSprawl.Server/Game/Combat/AbilityDefRow.cs` | Load-time `AbilityDef` row DTO. |
| `server/NeonSprawl.Server/Game/Combat/AbilityDefinitionCatalog.cs` | In-memory catalog + `TryGetAbility`. |
| `server/NeonSprawl.Server/Game/Combat/AbilityDefinitionCatalogLoader.cs` | Disk I/O, schema validation, E5M1 gate. |
| `server/NeonSprawl.Server/Game/Combat/AbilityCatalogPathResolution.cs` | Directory/schema discovery and config resolution. |
| `server/NeonSprawl.Server/Game/Combat/PrototypeE5M1AbilityCatalogRules.cs` | Frozen four ids gate (sync comment to Python). |
| `server/NeonSprawl.Server/Game/Combat/AbilityCatalogServiceCollectionExtensions.cs` | `AddAbilityDefinitionCatalog` DI registration. |
| `server/NeonSprawl.Server.Tests/Game/Combat/AbilityCatalogTestPaths.cs` | Repo `content/abilities` + schema discovery for tests. |
| `server/NeonSprawl.Server.Tests/Game/Combat/AbilityDefinitionCatalogLoaderTests.cs` | AAA loader + host startup tests. |
| `docs/plans/NEO-77-implementation-plan.md` | This plan. |
## Files to modify
| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Game/Skills/ContentPathsOptions.cs` | Add `AbilitiesDirectory`, `AbilityDefSchemaPath` under `Content` section. |
| `server/NeonSprawl.Server/Program.cs` | Register and eagerly resolve `AbilityDefinitionCatalog` at startup. |
| `server/NeonSprawl.Server/appsettings.json` | Optional empty `Content:AbilitiesDirectory` / schema path keys for override documentation (mirror other catalogs). |
| `server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs` | Pin `Content:AbilitiesDirectory` to discovered repo path so existing tests keep booting. |
| `server/NeonSprawl.Server.Tests/Game/PositionState/PostgresWebApplicationFactory.cs` | Pin abilities directory (same as recipes/items). |
| `server/NeonSprawl.Server.Tests/Game/Interaction/SalvageActivityDeniedRegistryWebApplicationFactory.cs` | Pin abilities directory. |
| `server/NeonSprawl.Server.Tests/Game/Skills/RefineActivityDeniedRegistryWebApplicationFactory.cs` | Pin abilities directory. |
| `server/NeonSprawl.Server.Tests/Game/Skills/MissionRewardDeniedRegistryWebApplicationFactory.cs` | Pin abilities directory. |
| `server/README.md` | New **Ability catalog (`content/abilities`, NEO-77)** section (config keys, discovery, fail-fast, E5M1 parity with CI). |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E5.M1 row — note NEO-77 server load when implementation completes. |
| `docs/decomposition/modules/E5_M1_CombatRulesEngine.md` | Status line — E5M1-02 server load landed when complete. |
| `docs/plans/E5M1-prototype-backlog.md` | E5M1-02 checkboxes when implementation completes. |
## Tests
| Test file | What it covers |
|-----------|----------------|
| `server/NeonSprawl.Server.Tests/Game/Combat/AbilityDefinitionCatalogLoaderTests.cs` | **Unit:** valid four-ability prototype catalog loads; `abilities` not array; schema violation (negative `baseDamage`, zero `cooldownSeconds`, empty `displayName`); duplicate `id` across files; E5M1 gate failure (missing id + extra/wrong id); `schemaVersion` ≠ 1; missing dir/schema; no `*_abilities.json` files; invalid JSON. **Host:** `InMemoryWebApplicationFactory` + `/health` + DI `AbilityDefinitionCatalog` count 4; `WebApplicationFactory` with empty abilities 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 `PrototypeE5M1AbilityCatalogRules` pointing at `scripts/validate_content.py` `PROTOTYPE_E5M1_ABILITY_IDS` / `_prototype_e5m1_ability_gate`. | **adopted** |
| **Test cwd** | `InMemoryWebApplicationFactory` must set `Content:AbilitiesDirectory` explicitly (do not rely on discovery alone in CI). | **adopted** |
| **Registry consumers** | Hotbar/cast/combat use `IAbilityDefinitionRegistry` in NEO-79+; this story only exposes `AbilityDefinitionCatalog` for boot + direct test lookup. **`PrototypeAbilityRegistry` unchanged.** | **adopted** |
| **Global cooldown still active** | Expected — catalog lands before E5M1-07 wires per-ability `cooldownSeconds`; load does not change runtime cast behavior. | **adopted** |
## Decisions (kickoff)
- **`AbilityDefinitionCatalog` singleton only** — defer `IAbilityDefinitionRegistry` and `PrototypeAbilityRegistry` migration to NEO-79 (user confirmed kickoff).
- **In-process validation** + **loader and host boot tests** per NEO-51 / NEO-66 precedent.
- **No cross-catalog checks** — ability rows have no item/skill refs in Slice 1; loader has no catalog dependencies.