NEO-113: add code review for quest catalog load branch.

docs/reviews/2026-05-31-NEO-113.md

@@ -0,0 +1,63 @@
+# Code review — NEO-113 (E7M1-02)
+
+**Date:** 2026-05-31  
+**Scope:** Branch `NEO-113-e7m1-server-quest-catalog-load-fail-fast` vs `df24281` (merge-base on `main`) — commits `cb4761b` … `1044547`  
+**Base:** `df24281` (main at branch point, post NEO-112 merge)
+
+## Verdict
+
+**Approve with nits**
+
+## Summary
+
+NEO-113 delivers fail-fast server startup loading for `content/quests/*_quests.json`, mirroring the NEO-101 / NEO-112 pattern: row DTOs, `QuestDefinitionCatalogLoader` with JsonSchema.Net validation, duplicate id gates, and `PrototypeE7M1QuestCatalogRules` for CI-parity E7M1 gates (four-id allowlist, acyclic prerequisites, item/recipe/encounter cross-refs against upstream catalogs, operator-chain terminal token). DI registers `QuestDefinitionCatalog` after item, recipe, and encounter catalogs; `Program.cs` eager-resolves before `Run()`. Eighteen AAA loader + host boot tests pass locally; documentation (implementation plan, alignment register, module register note, backlog AC, `server/README.md`) is thorough. No client work (correct). Risk is low — established catalog-load pattern with strong negative coverage; Bruno health smoke is a nice bonus.
+
+## Documentation checked
+
+| Path | Result |
+|------|--------|
+| `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/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). |
+| Full-stack epic decomposition | **N/A** — infrastructure-only; no client counterpart until NEO-122/NEO-123. |
+
+## Blocking issues
+
+(none)
+
+## 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`.
+
+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.
+
+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`.
+
+## 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: E7M1 four-id gate error strings differ slightly from Python (`'id'` quoted list vs `repr` list) — acceptable; both are actionable.
+
+- Nit: `QuestDefinitionCatalog` XML doc correctly defers callers to `IQuestDefinitionRegistry` (NEO-114) — good guardrail for the strict NEO-113/114 split.
+
+## Verification
+
+```bash
+# Quest catalog tests (18 — primary signal for this story)
+cd server
+dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj \
+  --filter "FullyQualifiedName~QuestDefinitionCatalogLoaderTests"
+
+# Full server suite (CI; requires Docker for Postgres integration tests)
+dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj
+
+# CI parity — Python content gate still passes with repo catalog
+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.