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-31-NEO-112.md

4.8 KiB
Raw Blame History

Code review — NEO-112 (E7M1-01)

Date: 2026-05-31
Scope: Branch NEO-112-e7m1-prototype-questdef-catalog-schemas-ci vs 47105cc (merge-base on main) — commits be1dae3 … 765075f
Base: 47105cc (main at branch point, post NEO-111 merge)

Follow-up: Suggestions and the prerequisites nit below are done (strikethrough + Done.).

Verdict

Approve with nits

Summary

NEO-112 delivers the Epic 7 Slice 1 content foundation: three JSON Schemas (quest-def, quest-step-def, quest-objective-def), a four-row prototype_quests.json catalog aligned to the E7.M1 freeze table, and ~300 lines of CI gates in validate_content.py (four-id allowlist, acyclic prerequisites, item/recipe/encounter cross-refs, operator-chain terminal token gate, duplicate quest/step/objective ids). Documentation is thorough—implementation plan reconciliation, CT.M1 paragraph, content/README.md, alignment register, and the full E7M1 prototype backlog land together. No server, C#, or Godot changes (correct for E7M1-01). Risk is low: pattern mirrors NEO-100/NEO-87; happy-path validation passes locally.

Documentation checked

Path Result
docs/plans/NEO-112-implementation-plan.md Matches — acceptance checklist checked; reconciliation section accurate.
docs/plans/E7M1-prototype-backlog.md (E7M1-01) Matches — AC checked; scope/out-of-scope correct; client counterpart none.
docs/decomposition/modules/E7_M1_QuestStateMachine.md Matches — freeze table, CI enforcement line, and Status note NEO-112 catalog landed / runtime from NEO-113.
docs/decomposition/modules/CT_M1_ContentValidationPipeline.md Matches — quest catalog PR gate paragraph describes shipped validators.
docs/decomposition/modules/documentation_and_implementation_alignment.md Matches — E7.M1 row notes NEO-112 catalog; register stays Planned until NEO-113.
docs/decomposition/modules/module_dependency_register.md Matches — E7.M1 note references NEO-112 catalog landed; register Planned until NEO-113 server load.
content/README.md Matches — quests/ table row + E7M1 prototype paragraph.
docs/decomposition/epics/epic_07_quest_faction.md Matches — Slice 1 backlog link + Godot DoD clarification.
Full-stack epic decomposition N/A — infrastructure-only story; no client counterpart required until NEO-122/NEO-123.

Blocking issues

(none)

Suggestions

  1. Refresh module_dependency_register.md E7.M1 note — Change “Register row stays Planned until NEO-112 lands” to “until NEO-113 server load lands” so it matches documentation_and_implementation_alignment.md after this merge. Done. — module_dependency_register.md E7.M1 note updated.

  2. Clarify E7.M1 module Status line — Replace “implementation not started” with wording that E7M1-01 catalog (NEO-112) is landed and runtime work starts at NEO-113. Done. — E7_M1_QuestStateMachine.md Status row updated.

Nits

  • Nit: E7M1 kickoff table row “Onboarding order: gather → refine → combat → chain” oversimplifies prerequisites — prototype_quest_combat_intro has no prereq (parallel path); freeze table and catalog are correct. Done. — E7M1-prototype-backlog.md kickoff Prerequisites row clarified.

  • Nit: quest-objective-def.schema.json lists all kind-specific fields in properties plus oneOf branches, so schema validation may accept wrong-field-at-wrong-kind (e.g. recipeId on encounter_complete) if both are present — plan adopted this tradeoff; CI cross-ref gate still catches unknown ids.

  • Nit: Duplicate step/objective ids are enforced within a quest only, not globally across quests — acceptable for prototype; E7M1-04 HTTP projection should namespace by quest id.

  • Nit: Branch bundles the full E7M1 backlog decomposition (~390 lines) with E7M1-01 implementation — large diff but intentional and documented.

Verification

# Happy path (primary regression signal — also runs in pr-gate.yml)
python3 -m pip install -r scripts/requirements-content.txt
python3 scripts/validate_content.py

# Manual negative cases (from NEO-112 plan)
# 1. Duplicate quest id across or within files → non-zero exit
# 2. Remove a frozen quest id from catalog → four-id gate error
# 3. Unknown itemId/recipeId/encounterId in objective → cross-ref error
# 4. Cyclic prerequisiteQuestIds → prerequisite gate error
# 5. Unknown prerequisite id → prerequisite gate error
# 6. Chain terminal step not inventory_has_item contract_handoff_token ×1 → chain gate error
# 7. Invalid kind or missing required field per kind → schema error

Reviewer note: Happy path executed successfully in review environment (content OK … 4 unique quest id(s)).