From 9c2ab5ae4de4191e318fe74ec19c426d1935bc46 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 31 May 2026 22:36:26 -0400 Subject: [PATCH] =?UTF-8?q?NEO-113:=20address=20code=20review=20=E2=80=94?= =?UTF-8?q?=20docs,=20schema=20and=20chain=20terminal=20tests.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../modules/E7_M1_QuestStateMachine.md | 4 +- docs/plans/NEO-113-implementation-plan.md | 5 +- docs/reviews/2026-05-31-NEO-113.md | 14 ++--- .../QuestDefinitionCatalogLoaderTests.cs | 61 +++++++++++++++++++ 4 files changed, 74 insertions(+), 10 deletions(-) diff --git a/docs/decomposition/modules/E7_M1_QuestStateMachine.md b/docs/decomposition/modules/E7_M1_QuestStateMachine.md index c639563..7cfefd3 100644 --- a/docs/decomposition/modules/E7_M1_QuestStateMachine.md +++ b/docs/decomposition/modules/E7_M1_QuestStateMachine.md @@ -7,7 +7,7 @@ | **Module ID** | E7.M1 | | **Epic** | [Epic 7 — Quest / Faction](../epics/epic_07_quest_faction.md) | | **Stage target** | Prototype | -| **Status** | Planned — Slice 1 backlog [E7M1-prototype-backlog.md](../../plans/E7M1-prototype-backlog.md): **E7M1-01** [NEO-112](https://linear.app/neon-sprawl/issue/NEO-112) catalog **landed** (schemas + CI); runtime from **E7M1-02** [NEO-113](https://linear.app/neon-sprawl/issue/NEO-113) → capstone **E7M1-12** [NEO-123](https://linear.app/neon-sprawl/issue/NEO-123) | +| **Status** | Planned — Slice 1 backlog [E7M1-prototype-backlog.md](../../plans/E7M1-prototype-backlog.md): **E7M1-01** [NEO-112](https://linear.app/neon-sprawl/issue/NEO-112) catalog **landed** (schemas + CI); **E7M1-02** [NEO-113](https://linear.app/neon-sprawl/issue/NEO-113) server load **landed**; registry + runtime from **E7M1-03** [NEO-114](https://linear.app/neon-sprawl/issue/NEO-114) → capstone **E7M1-12** [NEO-123](https://linear.app/neon-sprawl/issue/NEO-123) | | **Linear** | Label **`E7.M1`** · [E7M1-prototype-backlog.md](../../plans/E7M1-prototype-backlog.md) | ## Purpose @@ -78,6 +78,8 @@ The **first shipped quest spine** is **frozen** for prototype tuning until a del **CI enforcement (NEO-112):** `scripts/validate_content.py` requires **exactly** these four quest ids; objective cross-refs to frozen item/recipe/encounter catalogs; acyclic **`prerequisiteQuestIds`**; chain quest terminal step uses **`contract_handoff_token`** from [E5.M3 encounter loot](E5_M3_EncounterAndRewardTables.md#prototype-slice-3-freeze-e5m3-01). +**Server load (NEO-113):** Host fail-fast load of `content/quests/*_quests.json` with the same gates — [server README — Quest catalog](../../../server/README.md#quest-catalog-contentquests-neo-113). + **Reward policy (Slice 1):** Quest completion updates **`QuestStepState` only** — no duplicate item/XP grants. Gather/craft/encounter paths keep existing payouts; [E7.M2](E7_M2_RewardAndUnlockRouter.md) adds **`QuestRewardBundle`** apply in Slice 2. ## Risks and telemetry diff --git a/docs/plans/NEO-113-implementation-plan.md b/docs/plans/NEO-113-implementation-plan.md index 684d204..d093187 100644 --- a/docs/plans/NEO-113-implementation-plan.md +++ b/docs/plans/NEO-113-implementation-plan.md @@ -51,8 +51,9 @@ - **Catalog:** `QuestDefinitionCatalog` singleton; eager resolve in `Program.cs` after encounter catalogs. - **DI:** `AddQuestDefinitionCatalog` — loads after item, recipe, and encounter catalogs for cross-refs. - **Config:** `ContentPathsOptions` quest path keys; `InMemoryWebApplicationFactory` pins repo `content/quests`. -- **Tests:** 18 AAA tests in `QuestDefinitionCatalogLoaderTests` (loader + host boot). -- **Docs:** `server/README.md` quest catalog section; alignment register E7.M1 row updated. +- **Tests:** 20 AAA tests in `QuestDefinitionCatalogLoaderTests` (loader + host boot, incl. row schema violation + chain terminal quantity). +- **Docs:** `server/README.md` quest catalog section; alignment register E7.M1 row updated; `E7_M1_QuestStateMachine.md` Status + server-load line. +- **Bruno:** `bruno/neon-sprawl-server/quest-catalog/Health after quest catalog load.bru` (boot smoke; no quest HTTP in this story). ## Technical approach diff --git a/docs/reviews/2026-05-31-NEO-113.md b/docs/reviews/2026-05-31-NEO-113.md index a61e6ef..fe4c9d0 100644 --- a/docs/reviews/2026-05-31-NEO-113.md +++ b/docs/reviews/2026-05-31-NEO-113.md @@ -18,7 +18,7 @@ NEO-113 delivers fail-fast server startup loading for `content/quests/*_quests.j |------|--------| | `docs/plans/NEO-113-implementation-plan.md` | **Matches** — kickoff decisions adopted; acceptance checklist checked; reconciliation accurate. | | `docs/plans/E7M1-prototype-backlog.md` (E7M1-02) | **Matches** — AC checked; client counterpart none. | -| `docs/decomposition/modules/E7_M1_QuestStateMachine.md` | **Partially matches** — freeze table and CI enforcement line correct; **Status** still reads “runtime from E7M1-02 NEO-113” without noting server load **landed** (see Suggestion 1). | +| `docs/decomposition/modules/E7_M1_QuestStateMachine.md` | **Matches** — freeze table, CI enforcement + server-load line, Status notes NEO-113 landed. | | `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E7.M1 row notes NEO-113 server load; register stays Planned until NEO-114. | | `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E7.M1 note updated for NEO-113 server load. | | `server/README.md` | **Matches** — quest catalog section (config keys, discovery, fail-fast, load order, NEO-114 forward reference). | @@ -30,15 +30,15 @@ NEO-113 delivers fail-fast server startup loading for `content/quests/*_quests.j ## Suggestions -1. **Refresh `E7_M1_QuestStateMachine.md` Status line** — After merge, note that **E7M1-02 / NEO-113** server load is **landed** and runtime/registry work continues from **NEO-114** (mirror the alignment register wording). Optionally add a one-line “server load (NEO-113)” bullet under **CI enforcement** pointing at `server/README.md#quest-catalog-contentquests-neo-113`. +1. ~~**Refresh `E7_M1_QuestStateMachine.md` Status line** — After merge, note that **E7M1-02 / NEO-113** server load is **landed** and runtime/registry work continues from **NEO-114** (mirror the alignment register wording). Optionally add a one-line “server load (NEO-113)” bullet under **CI enforcement** pointing at `server/README.md#quest-catalog-contentquests-neo-113`.~~ **Done.** -2. **Add `Load_ShouldThrow_WhenRowViolatesSchema` test** — The implementation plan and NEO-101 precedent (`EncounterDefinitionCatalogLoaderTests.Load_ShouldThrow_WhenRowViolatesSchema`) call for a row-level schema failure case (e.g. empty `displayName` or missing required objective field). Other negative paths are well covered; this is the main gap vs the plan’s test table. +2. ~~**Add `Load_ShouldThrow_WhenRowViolatesSchema` test** — The implementation plan and NEO-101 precedent (`EncounterDefinitionCatalogLoaderTests.Load_ShouldThrow_WhenRowViolatesSchema`) call for a row-level schema failure case (e.g. empty `displayName` or missing required objective field). Other negative paths are well covered; this is the main gap vs the plan’s test table.~~ **Done.** -3. **Chain terminal quantity negative test** — Plan lists “wrong chain terminal objective (kind / item / quantity)”; kind and item are covered; adding a quantity ≠ 1 case would close the loop on `TryGetChainTerminalGateError`. +3. ~~**Chain terminal quantity negative test** — Plan lists “wrong chain terminal objective (kind / item / quantity)”; kind and item are covered; adding a quantity ≠ 1 case would close the loop on `TryGetChainTerminalGateError`.~~ **Done.** ## Nits -- Nit: Bruno `bruno/neon-sprawl-server/quest-catalog/` is not listed in the implementation plan file table — harmless smoke; consider a one-line mention in plan reconciliation if you want docs to mirror NEO-103/NEO-78 Bruno folders. +- ~~Nit: Bruno `bruno/neon-sprawl-server/quest-catalog/` is not listed in the implementation plan file table — harmless smoke; consider a one-line mention in plan reconciliation if you want docs to mirror NEO-103/NEO-78 Bruno folders.~~ **Done.** — plan reconciliation lists Bruno boot smoke. - Nit: E7M1 four-id gate error strings differ slightly from Python (`'id'` quoted list vs `repr` list) — acceptable; both are actionable. @@ -47,7 +47,7 @@ NEO-113 delivers fail-fast server startup loading for `content/quests/*_quests.j ## Verification ```bash -# Quest catalog tests (18 — primary signal for this story) +# Quest catalog tests (20 — primary signal for this story) cd server dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj \ --filter "FullyQualifiedName~QuestDefinitionCatalogLoaderTests" @@ -60,4 +60,4 @@ python3 -m pip install -r scripts/requirements-content.txt python3 scripts/validate_content.py ``` -**Reviewer note:** All 18 `QuestDefinitionCatalogLoaderTests` passed locally. Full server suite: 602 passed; 12 Postgres/docker integration tests skipped/failed in this environment (no Docker daemon) — unrelated to NEO-113. +**Reviewer note:** All 20 `QuestDefinitionCatalogLoaderTests` passed locally after review follow-up. Full server suite: 602 passed; 12 Postgres/docker integration tests skipped/failed in this environment (no Docker daemon) — unrelated to NEO-113. diff --git a/server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionCatalogLoaderTests.cs b/server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionCatalogLoaderTests.cs index 2737484..03595ae 100644 --- a/server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionCatalogLoaderTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionCatalogLoaderTests.cs @@ -264,6 +264,46 @@ public class QuestDefinitionCatalogLoaderTests Assert.Contains("expected schemaVersion 1", ioe.Message, StringComparison.Ordinal); } + [Fact] + public void Load_ShouldThrow_WhenRowViolatesSchema() + { + // Arrange + var (_, questsDir, defSchemaPath, stepSchemaPath, objectiveSchemaPath) = CreateTempContentLayout(); + const string bad = """ + { + "schemaVersion": 1, + "quests": [ + { + "id": "prototype_quest_gather_intro", + "displayName": "", + "prerequisiteQuestIds": [], + "steps": [ + { + "id": "gather_intro_step_salvage", + "displayName": "Gather scrap metal", + "objectives": [ + { + "id": "gather_intro_obj_scrap", + "kind": "gather_item", + "itemId": "scrap_metal_bulk", + "quantity": 3 + } + ] + } + ] + } + ] + } + """; + File.WriteAllText(Path.Combine(questsDir, "bad_quests.json"), bad, Encoding.UTF8); + // Act + var ex = Record.Exception(() => LoadCatalog(questsDir, defSchemaPath, stepSchemaPath, objectiveSchemaPath)); + // Assert + var ioe = Assert.IsType(ex); + Assert.Contains("bad_quests.json", ioe.Message, StringComparison.Ordinal); + Assert.Contains("Quest catalog validation failed", ioe.Message, StringComparison.Ordinal); + } + [Fact] public void Load_ShouldThrow_WhenDuplicateQuestIdAcrossFiles() { @@ -506,6 +546,27 @@ public class QuestDefinitionCatalogLoaderTests Assert.Contains("terminal itemId must be 'contract_handoff_token'", ioe.Message, StringComparison.Ordinal); } + [Fact] + public void Load_ShouldThrow_WhenChainTerminalQuantityIsWrong() + { + // Arrange + var (_, questsDir, defSchemaPath, stepSchemaPath, objectiveSchemaPath) = CreateTempContentLayout(); + var root = JsonNode.Parse(ValidPrototypeCatalogJson) as JsonObject + ?? throw new InvalidOperationException("expected object"); + var chain = GetQuestRow(root, "prototype_quest_operator_chain"); + var steps = chain["steps"] as JsonArray ?? throw new InvalidOperationException("expected steps"); + var lastStep = steps[^1] as JsonObject ?? throw new InvalidOperationException("expected step"); + var objectives = lastStep["objectives"] as JsonArray ?? throw new InvalidOperationException("expected objectives"); + var obj = objectives[0] as JsonObject ?? throw new InvalidOperationException("expected objective"); + obj["quantity"] = 2; + WriteCatalog(questsDir, root.ToJsonString()); + // Act + var ex = Record.Exception(() => LoadCatalog(questsDir, defSchemaPath, stepSchemaPath, objectiveSchemaPath)); + // Assert + var ioe = Assert.IsType(ex); + Assert.Contains("terminal quantity must be 1", ioe.Message, StringComparison.Ordinal); + } + [Fact] public void Load_ShouldThrow_WhenMissingSchemaFile() {