# 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) | **Matches** — E3.M1 **In Progress** status is correct; **E3.M1 note** footnote lists NEO-57–NEO-60 landed slices (parity with alignment table). |
| [`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.~~ **Done.**

## 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.~~ **Done.**

- 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`.
