diff --git a/docs/reviews/2026-05-24-NEO-75.md b/docs/reviews/2026-05-24-NEO-75.md new file mode 100644 index 0000000..9553a17 --- /dev/null +++ b/docs/reviews/2026-05-24-NEO-75.md @@ -0,0 +1,60 @@ +# Code review — NEO-75 playable gather→refine→make loop (capstone) + +**Date:** 2026-05-24 +**Scope:** Branch `NEO-75-e3-playable-gather-refine-make-loop` · commits `fbae314`–`977bcc3` vs `origin/main` +**Base:** `origin/main` + +## Verdict + +**Approve with nits** — capstone docs, HUD collapse, and path updates match the adopted plan; GdUnit passes. Merge after human capstone manual QA; minor module/README doc consistency nits below. + +## Summary + +The branch completes Epic 3 Slice 5 capstone integration: collapsible **`EconomyHudSection`** wraps inventory/skills/recipe panel under a **`Economy HUD`** checkbox, **`main.gd`** `@onready` paths follow the reparent, and a focused **`prototype_economy_hud_section.gd`** script toggles body visibility. Docs deliver the zero-Bruno capstone script (`docs/manual-qa/NEO-75.md`), README end-to-end loop section, implementation plan reconciliation, alignment register updates, and E3S5-04 backlog checkboxes. No new HTTP routes or economy client logic — the gather→refine→make loop reuses NEO-72–74 wiring as intended. Risk is low: layout-only client change plus documentation; automated coverage is the HUD toggle suite only (orchestration remains manual QA per plan). + +## Documentation checked + +| Document | Result | +|----------|--------| +| [`docs/plans/NEO-75-implementation-plan.md`](../plans/NEO-75-implementation-plan.md) | **Matches** — kickoff decisions (docs-first, implement HUD collapse, fresh server session, visible XP only) reflected; acceptance checklist and reconciliation complete. | +| [`docs/plans/E3S5-client-prototype-backlog.md`](../plans/E3S5-client-prototype-backlog.md) · **E3S5-04** | **Matches** — capstone manual QA, README integration, optional HUD collapse, alignment cross-links; AC checked. | +| [`docs/decomposition/modules/E3_M2_RefinementAndRecipeExecution.md`](../decomposition/modules/E3_M2_RefinementAndRecipeExecution.md) | **Matches** — status notes NEO-74 + NEO-75 landed. | +| [`docs/decomposition/modules/E3_M1_ResourceNodeAndGatherLoop.md`](../decomposition/modules/E3_M1_ResourceNodeAndGatherLoop.md) | **Partially matches** — alignment register cites NEO-75 capstone; module **Status** row still lists Slice 2 only (no NEO-73/75 client note). | +| [`docs/decomposition/modules/E3_M3_ItemizationAndInventorySchema.md`](../decomposition/modules/E3_M3_ItemizationAndInventorySchema.md) | **Partially matches** — alignment register cites NEO-72/75; module **Status** row still lists Slice 1 server stories only. | +| [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) | **Matches** — E3.M1/M2/M3 rows note NEO-75 landed and Slice 5 client complete. | +| [`docs/decomposition/epics/epic_03_crafting_economy.md`](../decomposition/epics/epic_03_crafting_economy.md) | **Partially matches** — Slice 5 AC prose unchanged; plan listed optional epic status update when capstone ships. | +| [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **Matches** — no client-side stack math; display-only HUD collapse. | +| [`docs/manual-qa/NEO-75.md`](../manual-qa/NEO-75.md) | **Matches** — single-session script, material math, collapse step; acceptance boxes left for human sign-off (expected). | +| [`client/README.md`](../../client/README.md) | **Partially matches** — NEO-75 loop section and NEO-72/74 paths updated; NEO-73 gather subsection still references bare **`InventoryLabel`** / **`SkillProgressionLabel`** paths. | +| Full-stack epic decomposition | **Matches** — E3S5-04 **client** capstone completes NEO-72→75 chain; Godot manual QA authoritative (not Bruno-only). | + +## Blocking issues + +(none) + +## Suggestions + +1. **Add Slice 5 capstone note to `epic_03_crafting_economy.md`** — Plan §4 called for a Slice 5 completion note when AC pass; alignment register and backlog are updated but the epic page still reads as open-ended. A one-line “Slice 5 client capstone (NEO-75) landed” under Slice 5 would close the loop. + +2. **Refresh E3.M1 / E3.M3 module Status rows** — E3.M2 was updated; E3.M1 and E3.M3 module pages still show pre–Slice 5 status while the alignment register already cites NEO-73/72/75. Mirror the E3.M2 pattern (client slice landed + capstone) for doc parity. + +3. **Align NEO-73 README HUD paths** — Update the gather section to `UICanvas/EconomyHudSection/Body/InventoryLabel` and `…/SkillProgressionLabel` so all three Slice 5 README sections reference the same hierarchy after the reparent. + +## Nits + +- Nit: **`test_toggle_collapses_body`** drives collapse via direct **`button_pressed = false`** rather than simulating a user click on **`ToggleButton`** — acceptable in headless GdUnit; behavior is covered. +- Nit: **`set_expanded`** and **`_on_toggle_toggled`** duplicate visibility logic — fine at this size; only matters if collapse rules grow (e.g. persist preference). +- Nit: Capstone AC for HUD collapse is checked in the implementation plan but not listed as a separate bullet in E3S5-04 backlog acceptance criteria — optional backlog tweak for traceability. + +## Verification + +```bash +# NEO-75 GdUnit suite (from client/) +godot --headless --path . -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode \ + -a res://test/prototype_economy_hud_section_test.gd + +# Full client suite (CI parity) +godot --headless --path . -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode +``` + +Manual (required before claiming capstone complete): follow [`docs/manual-qa/NEO-75.md`](../manual-qa/NEO-75.md) — **fresh server restart**, four-node gather (**11** scrap), two **`refine_scrap_standard`**, one **`make_field_stim_mk0`**, verify **`field_stim_mk0` ≥ 1**, salvage/refine XP movement, and economy HUD collapse/expand with populated panel.