don
/
neon-sprawl
mirror of https://github.com/ViPro-Technologies/neon-sprawl
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity
neon-sprawl/docs/reviews/2026-05-24-NEO-58.md

5.0 KiB
Raw Permalink Blame History

Code review — NEO-58 server resource-node catalog load (fail-fast)

Date: 2026-05-24
Scope: Branch NEO-58-e3m1-server-resource-node-catalog-load-fail-fast · commits 2de699b–fdaf5b7 vs main (plan + implementation + rebase note)
Base: main

Verdict

Approve with nits

Summary

NEO-58 adds fail-fast startup loading for content/resource-nodes/*_resource_nodes.json and *_resource_yields.json under Game/Gathering/, mirroring the NEO-51 item-catalog pattern: path discovery via ContentPathsOptions, in-process JSON Schema validation, duplicate nodeDefId detection, prototype Slice 2 gates (PrototypeSlice2ResourceNodeCatalogRules synced with scripts/validate_content.py), and yield itemId cross-check against ItemDefinitionCatalog. DI registers ResourceNodeCatalog after items; Program.cs eagerly resolves it at boot. Seventeen focused loader/host tests pass with full AAA. Docs (implementation plan, E3M1 backlog, alignment table, server README) match the landed behavior. Risk is low; main drift surface is duplicated Slice 2 constants in C# vs Python (documented, same as NEO-50/NEO-57).

Documentation checked

Document Result
docs/plans/NEO-58-implementation-plan.md Matches — scope, acceptance checklist, technical approach, and file list align with the diff; checklist marked complete.
docs/plans/E3M1-prototype-backlog.md Matches — E3M1-02 acceptance flipped with NEO-58 landed note.
docs/decomposition/modules/E3_M1_ResourceNodeAndGatherLoop.md Matches — content contracts and Slice 2 freeze; server load is the planned E3M1-02 step (registry/depletion still open).
docs/decomposition/modules/documentation_and_implementation_alignment.md Matches — E3.M1 row notes NEO-58 server loader + README link.
docs/plans/NEO-57-implementation-plan.md Matches — C# loader enforces the same gates CI added in NEO-57.
docs/decomposition/modules/client_server_authority.md N/A — no runtime gather authority change; startup validation only.
docs/decomposition/modules/CT_M1_ContentValidationPipeline.md Matches — PR gate remains validate_content.py; server tests guard C#/Python Slice 2 drift.

Register/tracking: alignment table updated; no further register change required beyond this row.

Blocking issues

None.

Suggestions

  1. Plan vs Bruno — NEO-58-implementation-plan.md lists Bruno as out of scope, but bruno/neon-sprawl-server/resource-node-catalog/ adds a health smoke request. Either add a one-line note under Tests (“optional Bruno smoke”) or drop the folder to keep plan scope literal; the request itself is fine for manual boot verification. Done. Plan Tests table now documents optional Bruno health smoke (pre-commit when Program.cs changes).

  2. Explicit empty-catalog file test (optional) — Host_ShouldFailStartup_WhenCatalogDirectoryInvalid covers an empty directory; a loader unit test asserting no *_resource_nodes.json files (and yields) would mirror item-catalog negative coverage and make failure messages grep-friendly in CI logs. Done. Added Load_ShouldThrow_WhenNoResourceNodesJsonFiles and Load_ShouldThrow_WhenNoResourceYieldsJsonFiles.

Nits

  • Nit: Slice 2 lens gate checks set equality (same as Python/NEO-57); duplicate gatherLens on two nodes still fails via set mismatch — acceptable for prototype.

  • Nit: Host_ShouldResolveCatalogFromDi_WhenStartupSucceeds uses /health in Act (NEO-51 item-catalog host test does the same); strict AAA would move the HTTP call to Arrange and keep Act as GetRequiredService<ResourceNodeCatalog>() only. Done. Host test is sync; Act is GetRequiredService<ResourceNodeCatalog>() only (CreateClient() in Arrange builds the host).

  • Nit: No docs/manual-qa/NEO-58.md — consistent with plan (server boot-only, no user-facing HTTP for resource nodes); Bruno health folder is sufficient smoke.

  • Nit: C#/Python constant drift remains a maintenance coupling; sync comments on PrototypeSlice2ResourceNodeCatalogRules are present (NEO-50/NEO-57 precedent).

Verification

cd server && dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj --filter "FullyQualifiedName~ResourceNodeCatalog"
cd server && dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj
python3 scripts/validate_content.py

Manual: start server from repo root (content discovery); confirm Information log with node/yield counts; point Content__ResourceNodesDirectory at an empty dir and confirm process refuses boot with Resource node catalog validation failed.