diff --git a/docs/reviews/2026-06-20-NEO-144.md b/docs/reviews/2026-06-20-NEO-144.md new file mode 100644 index 0000000..43eb03e --- /dev/null +++ b/docs/reviews/2026-06-20-NEO-144.md @@ -0,0 +1,64 @@ +# 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 + +1. **Freeze gate ignores `reputationGrants` today** — `_prototype_e7m4_template_freeze_gate` compares 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_bundle` and expand `PROTOTYPE_E7M4_TEMPLATE_FREEZE` so silent rep drift cannot pass CI. + +2. **`contract-seed.schema.json` is 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 `$ref` breakage is caught before server integration. + +3. **Port band-cap constants to C# in NEO-145/NEO-147** — `PROTOTYPE_E7M4_BAND_CAPS` in Python should stay name-aligned with the future server `ContractEconomyValidation` (plan already flags this); call it out in NEO-145 kickoff so drift does not reappear at issue time. + +## Nits + +- Nit: The implementation plan §5 names `PROTOTYPE_E7M4_BAND1_*` constants; shipped code uses `PROTOTYPE_E7M4_BAND_CAPS` keyed 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). + +- Nit: `_prototype_e7m4_cross_ref_gate` iterates only `PROTOTYPE_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): + +```bash +python3 scripts/validate_content.py +dotnet test +``` + +Author should also confirm band-cap negative AC manually (plan documents this; not committed): + +```bash +# 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`).