4.8 KiB
Code review — NEO-144 (E7M4-01 contract catalog + schemas + CI)
Date: 2026-06-20
Scope: Branch NEO-144-e7m4-01-contracttemplate-catalog-schemas-ci — commits 53122f9…2afd29e (6 commits)
Base: main
Verdict
Approve with nits
Summary
NEO-144 delivers the E7M4-01 content slice: contract-template and contract-seed JSON Schemas, a single-row prototype_contract_templates.json catalog, and ~320 lines of CI gates in validate_content.py (schema validation, roster/freeze, encounter/item/skill/faction cross-refs, band-1 economy caps). The work mirrors established NEO-133/NEO-124 quest and faction patterns, stays infrastructure-only (no server loader), and updates decomposition docs, content/README.md, and CT.M1 consistently. Risk is low: no runtime behavior change until NEO-145; the primary surface is python3 scripts/validate_content.py, which passes locally along with 823 server tests.
Documentation checked
| Document | Alignment |
|---|---|
docs/plans/NEO-144-implementation-plan.md |
Matches — schemas, catalog, gates, docs, and out-of-scope boundaries (no server/Godot) are all shipped; AC checklist marked complete. |
docs/plans/E7M4-pre-production-backlog.md (E7M4-01) |
Matches — acceptance criteria satisfied; client counterpart correctly none. |
docs/decomposition/modules/E7_M4_ContractMissionGenerator.md |
Matches — freeze table, band policy, and CI landed note present. |
docs/decomposition/modules/CT_M1_ContentValidationPipeline.md |
Matches — contract-catalog PR gate paragraph added. |
docs/decomposition/modules/documentation_and_implementation_alignment.md |
Matches — E7.M4 row updated to In Progress with E7M4-01 catalog landed. |
docs/decomposition/modules/module_dependency_register.md |
Matches — E7.M4 status and NEO-144 spine note updated. |
docs/decomposition/modules/client_server_authority.md |
N/A — content-only; no authority boundary change. |
| Full-stack epic decomposition | N/A — infrastructure-only; plan explicitly defers playable loop to NEO-154. |
Register/tracking tables were updated in this branch; no further alignment edits required for merge.
Blocking issues
None.
Suggestions
-
Freeze gate ignoresDone. Freeze table includesreputationGrantstoday —_prototype_e7m4_template_freeze_gatecompares bundles via_normalize_completion_bundle(item + skill XP only), same as E7M2 quest bundles. Prototype row has no rep grants, so this is fine for E7M4-01. When rep grants land on contract templates (or a second template is added), extend normalization akin to_normalize_e7m3_completion_bundleand expandPROTOTYPE_E7M4_TEMPLATE_FREEZEso silent rep drift cannot pass CI.reputationGrants: []; freeze gate uses_normalize_e7m3_completion_bundle. -
Done.contract-seed.schema.jsonis not exercised in CI — intentional per plan (NEO-147+). When HTTP DTO wiring starts, add at least a schema parse/registry smoke check (or a tiny fixture) so$refbreakage is caught before server integration._contract_seed_schema_smoke_gatevalidates a minimal fixture invalidate_content.py. -
Port band-cap constants to C# in NEO-145/NEO-147 —Done. Comment onPROTOTYPE_E7M4_BAND_CAPSin Python should stay name-aligned with the future serverContractEconomyValidation(plan already flags this); call it out in NEO-145 kickoff so drift does not reappear at issue time.PROTOTYPE_E7M4_BAND_CAPS+ NEO-145 kickoff note in plan open questions.
Nits
-
Nit: The implementation plan §5 namesDone. Plan §5 updated toPROTOTYPE_E7M4_BAND1_*constants; shipped code usesPROTOTYPE_E7M4_BAND_CAPSkeyed by band — the dict form is better for E7M4-07; consider a one-line reconciliation in the plan (cosmetic only; plan reconciliation section already describes shipped shape).PROTOTYPE_E7M4_BAND_CAPS. -
Nit:Done. Cross-ref and band-cap gates iterate all loaded catalog rows._prototype_e7m4_cross_ref_gateiterates onlyPROTOTYPE_E7M4_TEMPLATE_IDS. Correct for the single-template roster; generalize to all catalog rows when the roster gate relaxes in a later story.
Verification
Commands run during review (all green):
python3 scripts/validate_content.py
dotnet test
Author should also confirm band-cap negative AC manually (plan documents this; not committed):
# Temporarily set scrap_metal_bulk quantity to 11 in prototype_contract_templates.json
python3 scripts/validate_content.py # expect non-zero exit
Optional before merge: skim PR diff for doc-only commits (E7M4-pre-production-backlog.md, epic alignment) — no functional concern, but keeps reviewer focus on the three implementation commits (58d2c32, 7ecc874, 2afd29e).