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-65.md

5.5 KiB
Raw Permalink Blame History

Code review — NEO-65 prototype RecipeDef catalog + schemas + CI

Date: 2026-05-24
Scope: Branch NEO-65-e3m2-prototype-recipedef-catalog-schemas-ci · commits 0556d5f–b4f27b4 vs origin/main
Base: origin/main

Verdict

Approve with nits

Summary

NEO-65 delivers the E3M2-01 infrastructure slice: recipe-def / recipe-io-row JSON Schemas, eight frozen rows in content/recipes/prototype_recipes.json, and scripts/validate_content.py gates mirroring NEO-50 / NEO-57 (schema, duplicate id, exact allowlist, recipeKind coverage, item + skill cross-checks, Slice 3 refine-only contract). Docs close out the plan, CT.M1 PR-gate paragraph, E3.M2 freeze table, alignment register row, and E3M2-01 backlog checkboxes. The branch also carries Epic 3 full-stack decomposition prep (E3M2/E3S5 backlogs, .cursor/rules/full-stack-epic-decomposition.md) — appropriate context for Slice 3 but orthogonal to NEO-65 runtime risk. No server, client, or C# changes; CI script is the regression signal. Negative gates verified locally (duplicate id, unknown item, wrong skill, seven-recipe roster).

Documentation checked

Document Result
docs/plans/NEO-65-implementation-plan.md Matches — schemas, catalog, validate_content gates, doc touches, acceptance checklist complete; out-of-scope (server/HTTP) respected.
docs/plans/E3M2-prototype-backlog.md Matches — E3M2-01 checkboxes + landed note; infrastructure-only (no client counterpart required for this slug).
docs/plans/E3S5-client-prototype-backlog.md Matches — craft UI NEO-74 / capstone NEO-75 exist for player-visible Slice 3 work (full-stack policy satisfied for epic, not this ticket).
docs/decomposition/modules/E3_M2_RefinementAndRecipeExecution.md Matches — freeze table, designer note, CI rules, backlog links; header Status In Progress + NEO-65 implementation snapshot (addressed post-review).
docs/decomposition/modules/CT_M1_ContentValidationPipeline.md Matches — recipe catalog PR gate paragraph.
docs/decomposition/modules/documentation_and_implementation_alignment.md Matches — E3.M2 row In Progress with NEO-65 landed note; E3.M1/E3.M3 cross-link Slice 5 client issues.
docs/decomposition/modules/module_dependency_register.md Partially matches — E3.M2 note and E3 Slice 5 note updated; table Status column still Planned (consistent with E3.M3 pattern until slice completes — optional bump when NEO-66 lands).
content/README.md Matches — recipes/ table row + Slice 3 freeze paragraph.
docs/decomposition/modules/client_server_authority.md N/A — content/CI only.
Manual QA N/A — plan defers to CI + ad-hoc negative CLI checks (same as NEO-50 / NEO-57); no docs/manual-qa/NEO-65.md required for infrastructure-only content.

Register/tracking: alignment table E3.M2 In Progress is appropriate after E3M2-01; register row Planned until Slice 3 server stories land is acceptable if intentional.

Blocking issues

None.

Suggestions

  1. E3.M2 module doc status + snapshot — E3_M2_RefinementAndRecipeExecution.md header Status is still Planned while documentation_and_implementation_alignment.md lists In Progress. Bump header to In Progress and add an Implementation snapshot bullet for NEO-65 (catalog + CI), mirroring E3.M1/E3.M3 landed notes. Done. Header In Progress + NEO-65 snapshot bullet added.

  2. NEO-66 sync reminder — When the server loader lands, duplicate PROTOTYPE_SLICE3_RECIPE_IDS in C# with an explicit “keep in sync with validate_content.py” comment (already noted in the plan; worth repeating in the NEO-66 kickoff plan). Done. Kickoff note added under E3M2-02 in E3M2-prototype-backlog.md (NEO-66 plan not yet authored).

Nits

  • Nit: _validate_recipe_catalogs runs I/O itemId cross-check even when a duplicate id is detected (inside the if isinstance(rid, str) block but outside the else). Harmless — surfaces more errors per run; matches pragmatic CI style elsewhere.

  • Nit: Branch mixes chore full-stack policy commits with NEO-65 implementation — fine for sequencing, but PR description should call out both so reviewers know the policy/docs delta is intentional prep, not scope creep.

Verification

cd /home/don/neon-sprawl
pip install -r scripts/requirements-content.txt
python3 scripts/validate_content.py
# content OK: … 1 recipe catalog file(s), … 8 unique recipe id(s)

Manual negative cases (from plan — verified during review):

  • Duplicate recipe id → exit 1
  • Unknown itemId in inputs → exit 1
  • requiredSkillId: salvage → exit 1 (Slice 3 gate)
  • Remove one frozen id (seven recipes) → exit 1

PR gate (.github/workflows/pr-gate.yml) already invokes validate_content.py — no additional test project required for this story.