diff --git a/bruno/neon-sprawl-server/ability-cast/Post cast happy.bru b/bruno/neon-sprawl-server/ability-cast/Post cast happy.bru index 2dfbbaa..e076b31 100644 --- a/bruno/neon-sprawl-server/ability-cast/Post cast happy.bru +++ b/bruno/neon-sprawl-server/ability-cast/Post cast happy.bru @@ -21,6 +21,7 @@ body:json { tests { test("status 200", function () { + // NEO-30 review follow-up: cast JSON contract unchanged (server comments only). expect(res.getStatus()).to.equal(200); }); diff --git a/docs/decomposition/modules/module_dependency_register.md b/docs/decomposition/modules/module_dependency_register.md index fee350a..61515a9 100644 --- a/docs/decomposition/modules/module_dependency_register.md +++ b/docs/decomposition/modules/module_dependency_register.md @@ -21,7 +21,7 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen **E1.M3 note:** Decomposition expanded in [E1_M3_InteractionAndTargetingLayer.md](E1_M3_InteractionAndTargetingLayer.md) (E1.M1 vs E1.M3 boundary, contract sketches, Slice 3 alignment). **Partial (NEO-23):** versioned JSON v1 **`PlayerTargetStateResponse`** + **`POST …/target/select`** under `server/NeonSprawl.Server/Game/Targeting/` ([NEO-23](../../plans/NEO-23-implementation-plan.md); [server README — Targeting](../../../server/README.md#targeting-neo-23)) — throwaway HTTP spike per [contracts.md](contracts.md). **Partial (NEO-25):** versioned **`GET /game/world/interactables`** + `InteractablesListDtos` / `InteractablesWorldApi` in `Game/Interaction/`; Godot `interactables_catalog_client.gd` + `interactable_world_builder.gd` ([NEO-25](../../plans/NEO-25-implementation-plan.md); [server README — Interactable descriptors](../../../server/README.md#interactable-descriptors-neo-25)). **NEO-26 (prototype `SelectionEvent`):** Godot **`TargetSelectionClient.selection_event`** on **`lockedTargetId`** changes ([NEO-26](../../plans/NEO-26-implementation-plan.md)). **Follow-on / in progress:** richer multi-consumer **`InteractableDescriptor`** targeting on later Linear issues. **Precursor (E1.M1):** **`InteractionRequest`** + horizontal reach ([NEO-9](../../plans/NEO-9-implementation-plan.md); `Game/Interaction/`, `Game/World/HorizontalReach.cs`). -**E1.M4 note:** Vertical-slice decomposition is defined in [E1M4-prototype-backlog.md](../../plans/E1M4-prototype-backlog.md) with story slugs **E1M4-01** through **E1M4-05** (loadout contract, cast path, accept/deny UX, cooldown sync, telemetry hooks). **NEO-29** started implementation with `HotbarLoadout` v1 server APIs + persistence wiring and client hydration; **NEO-31** added the prototype cast POST + client digit-key path and resolver tests — module status remains **In Progress**; see [E1_M4_AbilityInputScaffold.md](E1_M4_AbilityInputScaffold.md), [NEO-29 plan](../../plans/NEO-29-implementation-plan.md), and [NEO-31 plan](../../plans/NEO-31-implementation-plan.md). +**E1.M4 note:** Vertical-slice decomposition is defined in [E1M4-prototype-backlog.md](../../plans/E1M4-prototype-backlog.md) with story slugs **E1M4-01** through **E1M4-05** (loadout contract, cast path, accept/deny UX, cooldown sync, telemetry hooks). **NEO-29** started implementation with `HotbarLoadout` v1 server APIs + persistence wiring and client hydration; **NEO-31** added the prototype cast POST + client digit-key path and resolver tests; **NEO-28** added server target lock + range gates and client cast feedback UX; **NEO-30** documented Epic 1 Slice 3 cast-funnel telemetry hook sites (comment-only, `AbilityCastApi`) — module status remains **In Progress**; see [E1_M4_AbilityInputScaffold.md](E1_M4_AbilityInputScaffold.md), [NEO-29 plan](../../plans/NEO-29-implementation-plan.md), [NEO-31 plan](../../plans/NEO-31-implementation-plan.md), [NEO-28 plan](../../plans/NEO-28-implementation-plan.md), and [NEO-30 plan](../../plans/NEO-30-implementation-plan.md). ### Epic 2 — Skills and Progression Framework diff --git a/docs/reviews/2026-04-30-NEO-30.md b/docs/reviews/2026-04-30-NEO-30.md index e9306c9..5659da2 100644 --- a/docs/reviews/2026-04-30-NEO-30.md +++ b/docs/reviews/2026-04-30-NEO-30.md @@ -6,6 +6,8 @@ **Base:** `origin/main` +**Follow-up:** Suggestions and nits below are **done** (strikethrough + **Done.**); register row and XML doc tweaks landed in the same change-set. + ## Verdict **Approve** @@ -21,12 +23,12 @@ The branch delivers NEO-30 as agreed: **comment-only** telemetry hook sites on t | `docs/plans/NEO-30-implementation-plan.md` | **Matches** — kickoff decisions (comment-only server hooks, no new server logging), file list, acceptance criteria, and technical approach align with the diff. | | `docs/decomposition/modules/E1_M4_AbilityInputScaffold.md` | **Matches** — snapshot and E1M4-05 row cite NEO-30, hook anchors, and E9.M1 deferral. | | `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E1.M4 row documents NEO-30 telemetry hook sites. | -| `docs/decomposition/modules/module_dependency_register.md` | **Partially matches** — E1.M4 remains **In Progress** (correct); the long **E1.M4 note** still summarizes NEO-29/31/28 but does not mention NEO-30 (optional discoverability; not a plan requirement). | +| `docs/decomposition/modules/module_dependency_register.md` | **Matches** (post-review) — E1.M4 note now includes NEO-28 + NEO-30 with plan links. | | `docs/decomposition/modules/E9_M1_TelemetryEventSchema.md` | **N/A** — deferred per plan; `TODO(E9.M1)` in code matches. | | `docs/decomposition/modules/client_server_authority.md` / `contracts.md` | **Matches** — no change to authority or wire contracts; comments reinforce server as authoritative deny/reason source. | | `docs/manual-qa/NEO-30.md` | **Matches** — concrete checks for comments, 400/404 exclusion, and shared NEO-28/31 vocabulary. | -**Register / tracking table:** No mandatory row edit beyond what landed in `documentation_and_implementation_alignment.md`; optional follow-up is a one-line NEO-30 mention in the register’s E1.M4 narrative note for consistency. +**Register / tracking table:** E1.M4 narrative note updated to include NEO-30 (and NEO-28) with plan links — see **Suggestions** resolution above. ## Blocking issues @@ -34,13 +36,13 @@ The branch delivers NEO-30 as agreed: **comment-only** telemetry hook sites on t ## Suggestions -1. **Module dependency register (optional):** Add **NEO-30** to the **E1.M4 note** paragraph in `docs/decomposition/modules/module_dependency_register.md` so readers who start from the register see hook-site documentation alongside NEO-29/31/28 — mirrors the alignment table and avoids “only in alignment doc” drift. +1. ~~**Module dependency register (optional):** Add **NEO-30** to the **E1.M4 note** paragraph in `docs/decomposition/modules/module_dependency_register.md` so readers who start from the register see hook-site documentation alongside NEO-29/31/28 — mirrors the alignment table and avoids “only in alignment doc” drift.~~ **Done.** E1.M4 note now includes **NEO-28** and **NEO-30** with plan links (`module_dependency_register.md`). ## Nits -1. **Nit:** `AbilityCastApi` class `` uses `AbilityCastResponse.reasonCode` (JSON casing). C# readers may expect `ReasonCode`; harmless for intent. +1. ~~**Nit:** `AbilityCastApi` class `` uses `AbilityCastResponse.reasonCode` (JSON casing). C# readers may expect `ReasonCode`; harmless for intent.~~ **Done.** Remarks now use `` and note wire JSON `reasonCode` (`AbilityCastApi.cs`). -2. **Nit:** `AbilityCastApiTests` `` targets an extension method; some doc generators are picky. If XML doc warnings appear in CI later, switch to a fully qualified `cref` or drop the `see` to the containing type only. +2. ~~**Nit:** `AbilityCastApiTests` `` targets an extension method; some doc generators are picky. If XML doc warnings appear in CI later, switch to a fully qualified `cref` or drop the `see` to the containing type only.~~ **Done.** `` targets `AbilityCastApi` only; extension name in plain `MapAbilityCastApi` (`AbilityCastApiTests.cs`). ## Verification diff --git a/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs b/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs index 5086aec..41eaee9 100644 --- a/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs @@ -12,7 +12,7 @@ namespace NeonSprawl.Server.Tests.Game.AbilityInput; /// /// NEO-30: every JSON deny ( false) asserts a non-whitespace /// — matches Slice 3 ability_cast_denied payload expectations; -/// authoritative hook-site comments live on . +/// authoritative hook-site comments live on (cast route: MapAbilityCastApi extension in the same source file). /// public sealed class AbilityCastApiTests { diff --git a/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs b/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs index 75bffd4..f4d7da4 100644 --- a/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs +++ b/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs @@ -7,7 +7,7 @@ namespace NeonSprawl.Server.Game.AbilityInput; /// Maps prototype ability cast POST (NEO-31 + NEO-28 target authority). /// /// Epic 1 Slice 3 product telemetry names ability_cast_requested and ability_cast_denied -/// (see AbilityCastResponse.reasonCode on denies) — hook sites are marked inline below. +/// (on JSON denies, — wire JSON reasonCode) — hook sites are marked inline below. /// TODO(E9.M1): emit cataloged events (payload: route player id, slotIndex, abilityId, targetId; on deny include reasonCode). /// HTTP 400/404 here are outside the v1 cast deny contract (no JSON AbilityCastResponse). ///