neon-sprawl/docs/reviews/2026-05-24-NEO-57.md

4.5 KiB
Raw Permalink Blame History

Code review — NEO-57 prototype resource node catalogs + CI

Date: 2026-05-24
Scope: Branch NEO-57-e3m1-prototype-resourcenodedef-yield-schemas-ci · commits 17daf9aca1a15f vs origin/main (decomposition merge + plan + implementation + review follow-ups)
Base: origin/main

Verdict

Approve

Summary

NEO-57 lands content-only Slice 2 gather spine: two JSON Schemas (resource-node-def, resource-yield-row), four frozen ResourceNodeDef rows and matching yield rows under content/resource-nodes/, and extended scripts/validate_content.py with duplicate nodeDefId detection, frozen four-id / four-lens gates, yield↔node and yield↔item cross-checks, and scrap_metal_bulk-only yield item gate. The implementation mirrors the NEO-50 item-catalog pattern (row schema + schemaVersion envelope + prototype allowlists). Docs (E3.M1 module freeze, CT.M1 PR gate, alignment table, content/README.md, E3M1 backlog) align with the implementation plan. No server, client, or Bruno changes — appropriate for E3M1-01. Risk is low; CI script is the primary regression surface.

Documentation checked

Document Result
docs/plans/NEO-57-implementation-plan.md Matches — two-file layout, maxGathers, differentiated quantities 1/2/3/5, acceptance checklist complete; no server scope.
docs/plans/E3M1-prototype-backlog.md Matches — E3M1-01 acceptance checked; yield row schema name aligned (post-review).
docs/decomposition/modules/E3_M1_ResourceNodeAndGatherLoop.md Matches — Slice 2 freeze table, designer note, contracts, E1.M3 boundary; content ids match JSON.
docs/decomposition/modules/CT_M1_ContentValidationPipeline.md Matches — NEO-57 PR gate paragraph for resource-node + yield validation.
docs/decomposition/modules/documentation_and_implementation_alignment.md Matches — E3.M1 row notes NEO-57 content + CI; server load still planned.
docs/decomposition/modules/module_dependency_register.md Matches — E3.M1 note for NEO-57NEO-64 Slice 2 backlog.
content/README.md Matchesresource-nodes/ table row + Slice 2 freeze pointer.
docs/decomposition/modules/client_server_authority.md N/A — no runtime authority change this story (content + CI only).

Register/tracking: alignment table and CT.M1 updated; E3.M1 module status reflects Slice 2 backlog in progress.

Blocking issues

None.

Suggestions

None (resolved in ca1a15f and follow-up backlog edit).

  1. E3M1 backlog checkboxes — In E3M1-prototype-backlog.md § E3M1-01, flip acceptance criteria to [x] (or add a “NEO-57 landed” note) so the backlog matches NEO-57-implementation-plan.md. Done.

  2. Backlog schema name — Update E3M1-01 bullet from resource-yield-table.schema.json to resource-yield-row.schema.json (kickoff chose row schema + prototype_resource_yields.json). Done.

  3. Backlog yield file wording — Remove “or embedded yields — plan decides” from E3M1-01 in-scope list; two-file layout is landed. Done.

Nits

  • Nit: _prototype_slice2_resource_node_gate enforces lens set equality, not a per-lens duplicate error message; duplicate gatherLens on two nodes still fails with a clear set-mismatch line — acceptable for prototype.

  • Nit: CI does not freeze yield quantities (1/2/3/5) — only schema quantity >= 1 and scrap_metal_bulk only; intentional per plan (differentiation is content, not gate).

  • Nit: No docs/manual-qa/NEO-57.md — fine for content-only story; manual negative cases live in the implementation plan Tests table.

Verification

pip install -r scripts/requirements-content.txt
python3 scripts/validate_content.py

Manual negatives (from plan): duplicate nodeDefId; remove a frozen id; invalid gatherLens; unknown yield itemId; duplicate yield nodeDefId; non-scrap_metal_bulk yield item — each should exit non-zero.

PR gate: .github/workflows/pr-gate.yml already invokes validate_content.py (no workflow change required).