4.8 KiB
Code review — NEO-124 (E7M2-01)
Date: 2026-06-07
Scope: Branch NEO-124-e7m2-questrewardbundle-schema-quest-catalog-extension-ci vs origin/main — commits 85ead01 … c8e7449
Base: origin/main
Follow-up: Alignment register E7.M2 References column repaired; negative CI spot-checks run (see Verification).
Verdict
Approve with nits — blocking documentation corruption fixed; implementation matches plan.
Summary
NEO-124 locks the completionRewardBundle content contract for Epic 7 Slice 2: two new JSON Schemas (quest-reward-bundle, quest-skill-xp-grant), optional completionRewardBundle on quest-def, four frozen quest rows in prototype_quests.json, and three CI gates in validate_content.py (presence, freeze-table match with normalized sort, item/skill cross-refs + mission_reward allowlist). The work follows the NEO-112 content-only pattern, extends the quest $ref registry correctly, and documents the freeze in E7.M2, CT.M1, content/README.md, and the implementation plan. python3 scripts/validate_content.py passes locally. Risk is low for runtime (no server/client changes), but the corrupted alignment-table row should be fixed before merge.
Documentation checked
| Path | Result |
|---|---|
docs/plans/NEO-124-implementation-plan.md |
Matches — kickoff decisions (camelCase keys, omit sourceKind), acceptance criteria checked, reconciliation accurate. |
docs/plans/E7M2-prototype-backlog.md (E7M2-01) |
Matches — AC checkboxes complete; infrastructure-only client counterpart noted. |
docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md |
Matches — Slice 2 freeze table, camelCase keys, CI enforcement line. |
docs/decomposition/modules/CT_M1_ContentValidationPipeline.md |
Matches — quest completion bundle PR-gate bullet added. |
content/README.md |
Matches — E7 Slice 2 paragraph with cross-ref rules and links. |
docs/decomposition/modules/documentation_and_implementation_alignment.md |
|
docs/decomposition/modules/module_dependency_register.md |
Matches — E7.M2 note between table rows (same pattern as E7.M1); row stays Planned until NEO-132. |
docs/decomposition/epics/epic_07_quest_faction.md |
Matches — Slice 2 backlog link + Godot DoD line for NEO-132. |
| Full-stack epic decomposition | N/A — E7M2-01 is infrastructure-only; no client counterpart required. |
Register / tracking: E7.M2 correctly remains Planned until capstone NEO-132; alignment row content is otherwise appropriate once corruption is fixed.
Blocking issues
Corrupted E7.M2 row inDone. Replaced References cell with clean link list per review suggestion; table renders as one row.documentation_and_implementation_alignment.md— line 62 ends the References column with a stray fragment:
Suggestions
-
Spot-check negative CI paths before merge — Plan lists manual failure cases; run at least one each (typoDone. Ran three temporary mutations (restored after each): operator chain item typo → freeze gate exit 1; gather intro amount 24 → freeze gate exit 1; salvage withoutitemId, wrong XP amount, skill missingmission_reward) to confirm gate messages are actionable.mission_reward→ allowlist cross-ref exit 1. Note: item typo is caught by freeze gate before item cross-ref (gate order: presence → freeze → cross-ref). -
NEO-125 sync comment —
PROTOTYPE_E7M2_COMPLETION_BUNDLESalready points at E7.M2 freeze; when server loader lands, mirror the constant in C# with an explicit cross-link comment (plan risk row already tracks this).
Nits
- Nit:
_prototype_e7m2_completion_bundle_cross_ref_gateskips malformed grant dicts silently (same pattern as other E7M1 gates); schema validation already rejects them — consistent, no change needed. - Nit:
quest-reward-bundle.schema.jsonanyOfallowsitemGrants: []whenskillXpGrantsis non-empty; catalog correctly omits emptyitemGrantsper kickoff decision.
Verification
python3 scripts/validate_content.py
Optional negative checks (expect non-zero exit + stderr message):
- Temporarily typo
survey_drone_kit→ confirm item cross-ref error. - Temporarily set gather intro
amountto 24 → confirm freeze gate error. - Temporarily remove
mission_rewardfrom a skill'sallowedXpSourceKinds→ confirm allowlist error.
After fixing blocking issue 1, re-read the E7.M2 alignment row in a Markdown preview to confirm the table renders as one row.
Follow-up verification: python3 scripts/validate_content.py passes on clean tree; negative spot-checks above executed in follow-up (see Suggestions 1).