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-25-NEO-87.md

5.2 KiB
Raw Blame History

Code review — NEO-87 prototype NpcBehaviorDef catalog + CI

Date: 2026-05-25
Scope: Branch NEO-87-e5m2-npc-behavior-catalog-schemas-ci · commits 846da90–6fd9694 vs origin/main
Base: origin/main

Verdict

Request changes

Summary

NEO-87 lands E5M2-01 content-only work: npc-behavior-def.schema.json, NPC behavior validation in scripts/validate_content.py (mirroring NEO-76 ability pattern), and thorough doc updates (E5.M2 freeze table, content/README.md, CT.M1, alignment register, backlog checkboxes). Schema shape, CI gate constants, and Python validation helpers align with the kickoff plan. However, content/npc-behaviors/prototype_npc_behaviors.json ships only two of three frozen rows — prototype_elite_mini_boss is missing — so validate_content.py exits non-zero and PR gate would fail. Docs and the implementation plan mark E5M2-01 as landed with all acceptance criteria checked; that is premature until the catalog is complete.

Documentation checked

Document Result
docs/plans/NEO-87-implementation-plan.md Conflicts — reconciliation and AC claim three-row catalog shipped; file on disk has two rows. Kickoff table, schema spec, and CI approach otherwise match.
docs/plans/E5M2-prototype-backlog.md · E5M2-01 Conflicts — AC checked and “Landed” note present while CI fails on missing third id.
docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md Partially matches — freeze table documents all three rows (including elite stats); catalog file does not. CI rules line and status update match intent.
docs/decomposition/modules/CT_M1_ContentValidationPipeline.md Matches — NPC behavior PR gate paragraph added.
docs/decomposition/modules/documentation_and_implementation_alignment.md · E5.M2 Conflicts — row states NEO-87 landed while three-id gate fails.
docs/decomposition/modules/module_dependency_register.md Conflicts — E5.M2 note cites NEO-87 landed before catalog passes CI.
content/README.md Matches — npc-behaviors/ row + E5 Slice 2 freeze paragraph (ids and rules).
docs/decomposition/modules/client_server_authority.md N/A — no runtime authority changes; content-only.
Full-stack epic decomposition Matches — E5M2-01 is server/content + CI only; kickoff explicitly defers Godot to E5M2-04+ / NEO-97 / NEO-98. No docs/manual-qa/NEO-87.md required.

Blocking issues

  1. Missing third catalog row — CI fails. content/npc-behaviors/prototype_npc_behaviors.json contains only prototype_melee_pressure and prototype_ranged_control. The E5M2 three-id gate requires prototype_elite_mini_boss as well. Running validation:

    python3 -m venv .venv-content && .venv-content/bin/pip install -r scripts/requirements-content.txt
.venv-content/bin/python scripts/validate_content.py

    exits non-zero with:

    error: prototype E5M2 expects exactly npc behavior ids ['prototype_elite_mini_boss', 'prototype_melee_pressure', 'prototype_ranged_control'], got ['prototype_melee_pressure', 'prototype_ranged_control']

    Add the elite row per the kickoff freeze table (maxHp 200, aggroRadius 8.0, leashRadius 18.0, telegraphWindupSeconds 2.5, attackDamage 25, attackCooldownSeconds 5.0, archetypeKind elite_mini_boss).

Suggestions

  1. Defer “landed” doc claims until CI is green — After fixing the catalog, re-run validation and confirm plan reconciliation, E5M2-01 backlog “Landed” note, alignment register, and module register accurately reflect a passing gate (same pattern as NEO-76 review follow-up).

  2. Optional: one-row-per-archetypeKind gate — The three-id frozenset already implies one row per archetype for E5M2-01. If the roster grows in a follow-up issue, consider an explicit archetype coverage check; not required for this slice.

Nits

  • Nit: Commit subject “ship prototype … catalog” reads complete while the catalog file is incomplete — minor messaging; resolved when elite row lands.

Verification

# From repo root
python3 -m venv .venv-content && .venv-content/bin/pip install -r scripts/requirements-content.txt
.venv-content/bin/python scripts/validate_content.py
# Expect: content OK … 1 npc behavior catalog file(s) … 3 unique npc behavior id(s)

# Negative smokes (optional, after fix)
# — duplicate id across rows → non-zero exit
# — remove one frozen id → gate error
# — leashRadius <= aggroRadius → numeric gate error

PR gate (.github/workflows/pr-gate.yml) runs the same script; no additional workflow changes needed for this story.