NEO-115: add code review for quest definitions GET API.

docs/reviews/2026-06-03-NEO-115.md

@@ -0,0 +1,63 @@
+# Code review — NEO-115 (E7M1-04)
+
+**Date:** 2026-06-03  
+**Scope:** Branch `NEO-115-get-world-quest-definitions` vs `origin/main` — commits `a5be355` … `b3f90de`  
+**Base:** `origin/main`
+
+## Verdict
+
+**Approve with nits**
+
+## Summary
+
+NEO-115 adds **`GET /game/world/quest-definitions`**, a versioned read-only projection of the frozen prototype quest catalog via **`IQuestDefinitionRegistry`**. Implementation mirrors the established encounter/item world-definition pattern: `QuestDefinitionsWorldApi` maps registry rows to flat objective DTOs with **`WhenWritingNull`** on unused fields, wired in **`Program.cs`** after encounter definitions. One AAA integration test covers schema v1, four quests in ordinal id order, and spot-checks for gather, refine prerequisite, combat, and operator-chain terminal objectives. Bruno folder validates the same frozen rows and confirms null-key omission on a gather objective via strict `eql`. Documentation (implementation plan, backlog AC, E7.M1 snapshot, alignment register, `server/README.md`) is updated. Server-only infrastructure with optional client consumer NEO-122 — correct scope. Risk is low.
+
+## Documentation checked
+
+| Path | Result |
+|------|--------|
+| `docs/plans/NEO-115-implementation-plan.md` | **Matches** — kickoff decisions adopted; acceptance checklist checked; reconciliation accurate. |
+| `docs/plans/E7M1-prototype-backlog.md` (E7M1-04) | **Matches** — AC checked; in/out of scope aligned with shipped route. |
+| `docs/decomposition/modules/E7_M1_QuestStateMachine.md` | **Matches** — HTTP read model bullet documents NEO-115 route + Bruno. |
+| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E7.M1 row notes NEO-115 HTTP projection; register stays **Planned** until E7M1-05+ runtime. |
+| `docs/decomposition/modules/module_dependency_register.md` | **Partially matches** — E7.M1 note still ends at NEO-114 registry; alignment doc covers NEO-115 (see Suggestion 2). |
+| `server/README.md` | **Matches** — quest definitions section with curl, registry note, plan/Bruno links. |
+| Full-stack epic decomposition | **N/A** — server-only; client counterpart NEO-122 explicitly deferred. |
+
+## Blocking issues
+
+(none)
+
+## Suggestions
+
+1. **Reuse frozen id ordering constant in tests** — Plan adopted reusing **`PrototypeE7M1QuestCatalogRules`** for ids. `QuestDefinitionsWorldApiTests` defines a parallel **`FrozenQuestIdsInOrdinalOrder`** array instead of deriving from **`ExpectedQuestIds.Order(StringComparer.Ordinal)`** (as **`QuestDefinitionRegistryTests`** already does). Consolidating avoids drift if the roster changes.
+
+2. **Optional: refresh `module_dependency_register` E7.M1 note** — E7.M1 module page and alignment register mention NEO-115; the register table note still stops at NEO-114. A one-line append (“**E7M1-04 / NEO-115** HTTP read landed”) would keep the dependency register consistent with NEO-114 follow-up style.
+
+## Nits
+
+- Nit: C# integration test asserts **`refine_intro`** prerequisite only, not the **`craft_recipe`** objective shape; Bruno likewise skips refine/combat spot-checks. Adequate for prototype freeze given gather + operator-chain + combat coverage, but a single **`craft_recipe`** assert would close the objective-kind loop in C#.
+
+- Nit: **`MapObjective`** default switch arm copies all optional fields for unknown kinds — unreachable for validated catalog content but reasonable defensive fallback.
+
+- Nit: Single large integration test (78 lines of asserts) follows the encounter world API test style; acceptable for a frozen four-quest roster.
+
+## Verification
+
+```bash
+# Quest definitions world API (primary signal for this story)
+cd server
+dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj \
+  --filter "FullyQualifiedName~QuestDefinitionsWorldApiTests"
+
+# Quest registry regression (NEO-114)
+dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj \
+  --filter "FullyQualifiedName~QuestDefinitionRegistryTests"
+
+# Full server suite (CI)
+dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj
+```
+
+**Reviewer note:** `QuestDefinitionsWorldApiTests` (1 test) passed locally during review.
+
+**Manual / CI Bruno:** Run `bruno/neon-sprawl-server/quest-definitions/Get quest definitions.bru` against a dev server on port 5253.