69 lines
4.7 KiB
Markdown
69 lines
4.7 KiB
Markdown
# 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`
|
||
|
||
## Verdict
|
||
|
||
**Request changes** — one documentation corruption in the alignment register (blocking); implementation otherwise 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` | **Conflicts** — E7.M2 row **References** column corrupted (stray `\|ion-plan.md)…` fragment); see blocking issue **1**. |
|
||
| `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
|
||
|
||
1. **Corrupted E7.M2 row in `documentation_and_implementation_alignment.md`** — line 62 ends the References column with a stray fragment:
|
||
|
||
```
|
||
… [NEO-43](../../plans/NEO-43-implementation-plan.md) |ion-plan.md), label **`E7.M2`** on NEO-124–NEO-132 |
|
||
```
|
||
|
||
This breaks the markdown table and truncates the intended link list. Replace with a single clean References cell, e.g.:
|
||
|
||
```
|
||
| [E7M2-prototype-backlog.md](../../plans/E7M2-prototype-backlog.md), [E7_M2](E7_M2_RewardAndUnlockRouter.md), [NEO-124](../../plans/NEO-124-implementation-plan.md), [NEO-43](../../plans/NEO-43-implementation-plan.md), label **`E7.M2`** on NEO-124–NEO-132 |
|
||
```
|
||
|
||
## Suggestions
|
||
|
||
1. **Spot-check negative CI paths before merge** — Plan lists manual failure cases; run at least one each (typo `itemId`, wrong XP amount, skill missing `mission_reward`) to confirm gate messages are actionable. Not required for approve once doc fix lands, but cheap confidence.
|
||
|
||
2. **NEO-125 sync comment** — `PROTOTYPE_E7M2_COMPLETION_BUNDLES` already 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_gate` skips malformed grant dicts silently (same pattern as other E7M1 gates); schema validation already rejects them — consistent, no change needed.
|
||
- Nit: `quest-reward-bundle.schema.json` `anyOf` allows `itemGrants: []` when `skillXpGrants` is non-empty; catalog correctly omits empty `itemGrants` per kickoff decision.
|
||
|
||
## Verification
|
||
|
||
```bash
|
||
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 `amount` to **24** → confirm freeze gate error.
|
||
- Temporarily remove `mission_reward` from a skill's `allowedXpSourceKinds` → 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.
|