NEO-60: add code review document

docs/reviews/2026-05-24-NEO-60.md

@@ -0,0 +1,56 @@
+# Code review — NEO-60 GET world resource-node-definitions + Bruno
+
+**Date:** 2026-05-24  
+**Scope:** Branch `NEO-60-e3m1-get-world-resource-node-definitions-bruno` · commits `78dbf1c`–`97f8f31` vs `origin/main`  
+**Base:** `origin/main`
+
+## Verdict
+
+**Approve with nits**
+
+## Summary
+
+NEO-60 adds **`GET /game/world/resource-node-definitions`**: a versioned read-only JSON projection (`schemaVersion` **1**, **`nodes`**) backed by **`IResourceNodeDefinitionRegistry`**, mirroring the NEO-53 **`ItemDefinitionsWorldApi`** pattern. Each row maps node metadata plus a nested **`yield`** object joined via **`TryGetYield`**; missing yields throw **`InvalidOperationException`** (programmer error per plan, post NEO-58 load validation). Implementation is thin (API mapper, DTOs, `Program.cs` wiring), integration tests use full AAA and assert the frozen four ids in ordinal order with alpha/delta spot-checks, and Bruno/manual QA/README/alignment docs match the plan. Risk is low: no auth change, no second catalog source, no instance depletion or inventory mutation.
+
+## Documentation checked
+
+| Document | Result |
+|----------|--------|
+| [`docs/plans/NEO-60-implementation-plan.md`](../plans/NEO-60-implementation-plan.md) | **Matches** — route, nested `yield`, registry-only injection, frozen four order, Bruno/manual QA/README, acceptance checklist complete. |
+| [`docs/plans/E3M1-prototype-backlog.md`](../plans/E3M1-prototype-backlog.md) | **Matches** — E3M1-04 acceptance items flipped with NEO-60 landed note. |
+| [`docs/decomposition/modules/E3_M1_ResourceNodeAndGatherLoop.md`](../decomposition/modules/E3_M1_ResourceNodeAndGatherLoop.md) | **Matches** — NEO-60 HTTP bullet under **Related implementation slices**; read-only definition catalog aligns with module **Purpose** / **Responsibilities** (no instance depletion on this route). |
+| [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) | **Matches** — E3.M1 row notes NEO-60 landed with plan, manual QA, README, Bruno links; **Still planned** correctly lists gather engine / depletion work. |
+| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Partially matches** — E3.M1 **In Progress** status is correct; **E3.M1 note** footnote still describes Slice 2 generically and does not mention NEO-57–NEO-60 landed slices (alignment table is updated; register footnote optional parity — see Suggestions). |
+| [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **Matches** — read-only world catalog projection; server remains source of truth; no client-trusted mutation. |
+| [`docs/manual-qa/NEO-60.md`](../manual-qa/NEO-60.md) | **Matches** — curl, schema v1, four ids in order, spot-checks aligned with automated tests and Bruno. |
+| [`server/README.md`](../../server/README.md) | **Matches** — Resource node definitions section with GET path, curl example, plan/manual QA/Bruno links. |
+
+Register/tracking: E3.M1 alignment row update is appropriate; module **Status** stays **In Progress** until gather engine / depletion slices (NEO-61+) land.
+
+## Blocking issues
+
+None.
+
+## Suggestions
+
+1. **Dependency register footnote (optional)** — Extend the **E3.M1 note** in [`module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) to mention **NEO-57–NEO-60** landed work (catalog, load, registry, HTTP read model) so the register matches the alignment table — same pattern as the NEO-53 review suggestion for E3.M3.
+
+## Nits
+
+- Nit: Bruno **`nodes are ascending by id (ordinal)`** block omits the **StringComparer.Ordinal** comment present in [`Get skill definitions.bru`](../../bruno/neon-sprawl-server/skill-definitions/Get%20skill%20definitions.bru) and post-review [`Get item definitions.bru`](../../bruno/neon-sprawl-server/item-definitions/Get%20item%20definitions.bru); harmless for ASCII snake_case ids but copy the comment for parity if touching the file.
+
+- Nit: **`ResourceNodeDefinitionsWorldApi`** allocates a `List<ResourceNodeDefinitionJson>` and calls **`TryGetYield`** per row on each request — necessary given the registry API shape; fine at four rows (same pattern as sibling definition endpoints).
+
+- Nit: Bruno **`frozen prototype four is present`** is redundant given **`frozen prototype four matches registry id order`** asserts the exact sequence — consistent with NEO-53 Bruno layout; no action required.
+
+## Verification
+
+```bash
+cd server
+dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj --filter "FullyQualifiedName~ResourceNodeDefinitionsWorldApi"
+# Passed! Failed: 0, Passed: 1, Skipped: 0
+
+dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj
+```
+
+Manual: [`docs/manual-qa/NEO-60.md`](../manual-qa/NEO-60.md) and `bruno/neon-sprawl-server/resource-node-definitions/Get resource node definitions.bru` against dev `baseUrl`.