diff --git a/docs/plans/NEO-35-implementation-plan.md b/docs/plans/NEO-35-implementation-plan.md new file mode 100644 index 0000000..9ef4dd4 --- /dev/null +++ b/docs/plans/NEO-35-implementation-plan.md @@ -0,0 +1,77 @@ +# NEO-35 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-35 | +| **Title** | E2.M1: SkillDefinitionRegistry service + lookup tests | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-35/e2m1-skilldefinitionregistry-service-lookup-tests | + +## Kickoff clarifications + +| Topic | Question | Answer | +|--------|------------|--------| +| Scope / sequencing | Any open decisions before planning? | **None.** Linear description, [E2.M1 — SkillDefinitionRegistry](../decomposition/modules/E2_M1_SkillDefinitionRegistry.md), and shipped **NEO-34** (`SkillDefinitionCatalog`, `SkillDefRow`, loader, DI) fix scope, acceptance, and out-of-scope items. **NEO-34** is **Done**, so the dependency chain is satisfied for implementation. | +| Kickoff continuation | User asked to continue kickoff after board/relationship clarification | Proceed with this plan on branch `NEO-35-e2m1-skilldefinition-registry`; no additional product questions pending. | + +## Goal, scope, and out-of-scope + +**Goal:** Provide a small **`ISkillDefinitionRegistry`** (name as in Linear; implementation class name may be `SkillDefinitionRegistry`) **backed by** the startup-loaded **`SkillDefinitionCatalog`**: resolve by stable **`id`**, enumerate definitions, read **`category`**, **`displayName`**, and **`allowedXpSourceKinds`**. Surface is **registered in DI** so **NEO-36** (HTTP/read model) and **E2.M2** callers can depend on the interface. Document on the interface (and briefly in code where helpful) how **E2.M2** will use this for **grant validation** (`allowedXpSourceKinds` vs `XpGrantEvent.sourceKind`). + +**In scope (from Linear):** + +- C# types aligned with [`content/schemas/skill-def.schema.json`](../../content/schemas/skill-def.schema.json) for **prototype fields** (reuse or thinly wrap existing **`SkillDefRow`** from NEO-34 — avoid duplicating divergent models). +- Unit tests: known prototype ids resolve; **unknown id** returns absent / false **without throwing**; assert at least one **`allowedXpSourceKinds`** expectation on a known skill. +- XML / remarks on the public contract for **E2.M2** and future callers. + +**Out of scope (from Linear):** + +- **`UnlockRequirement`** gating logic (future slice; schema has no such field today). +- Wiring into the XP engine (**E2.M2**). + +## Acceptance criteria checklist + +- [ ] Tests: **happy path** — known prototype `id` (e.g. from existing test fixtures / `salvage` trio) resolves with expected metadata. +- [ ] Tests: **unknown id** — lookup indicates absence / `Try*` returns false **without throwing**. +- [ ] Tests: **`allowedXpSourceKinds`** — at least one assertion on a known skill’s list (per E2.M1 contract). +- [ ] **`ISkillDefinitionRegistry`** (or chosen public interface name) is **registered in DI** alongside the catalog so **NEO-36** can inject it without reaching for `SkillDefinitionCatalog` internals. + +## Technical approach + +1. **Interface:** `ISkillDefinitionRegistry` with: + - **`TryGet`** (or equivalent) for **id → definition** with non-throwing miss semantics. + - **Enumeration** of all definitions (e.g. `IEnumerable` or read-only collection) for projections. + - Document that **E2.M2** must reject XP grants when `sourceKind` ∉ `AllowedXpSourceKinds` for the target skill (behavior deferred to E2.M2; this story only exposes data). + +2. **Implementation:** `SkillDefinitionRegistry` as a **thin adapter** over **`SkillDefinitionCatalog`** (constructor-injected singleton dependency on `SkillDefinitionCatalog`). + +3. **DI:** Extend **`AddSkillDefinitionCatalog`** (or adjacent extension in the same file) to register **`ISkillDefinitionRegistry`** as singleton after / with the catalog registration. No requirement to eagerly-resolve the registry in **`Program.cs`** if **`SkillDefinitionCatalog`** is already resolved (NEO-34); optional `_ = app.Services.GetRequiredService()` only if we want symmetry — default **omit** unless a test or ordering reason appears. + +4. **Models:** Prefer exposing **`SkillDefRow`** on the interface (already matches schema fields). If a separate public DTO is needed for naming only, keep it a **record with identical fields** and map in one line — avoid a second parallel shape long-term. + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Skills/ISkillDefinitionRegistry.cs` | Public contract: lookup, enumerate, XML for E2.M2 / NEO-36. | +| `server/NeonSprawl.Server/Game/Skills/SkillDefinitionRegistry.cs` | Singleton adapter over `SkillDefinitionCatalog`. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/Skills/SkillCatalogServiceCollectionExtensions.cs` | Register `ISkillDefinitionRegistry` → `SkillDefinitionRegistry` after catalog registration. | +| `server/NeonSprawl.Server/Game/Skills/SkillDefinitionCatalog.cs` | Optional: tighten summary comment if it still says “NEO-35 may wrap” once registry exists. | + +## Tests + +| Test file | What it covers | +|-----------|------------------| +| `server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionRegistryTests.cs` | **Unit:** build a **`SkillDefinitionCatalog`** in-memory (same constructor as production) with a minimal `ById` dictionary **or** load via **`SkillDefinitionCatalogLoader`** using the existing temp-layout helper pattern from **`SkillDefinitionCatalogLoaderTests`**; assert **`TryGet`** for `salvage` (or trio) returns true and **`AllowedXpSourceKinds`** contains expected values; assert unknown id returns false and does not throw; assert enumeration count matches catalog. **AAA** labels per [csharp-style](../../.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert). | + +Optional (only if valuable for regression): one assertion in **`SkillDefinitionCatalogLoaderTests`** host test that **`ISkillDefinitionRegistry`** resolves from **`InMemoryWebApplicationFactory`** — not strictly required if unit tests + extension registration are sufficient. + +## Open questions / risks + +**None.** If **`SkillDefRow`** is later replaced by enums for category / source kinds, the registry interface can evolve in a follow-up issue without blocking NEO-36 string-based JSON projection.