From 52900448459094cd562a568776672ee8ce1dbdc8 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 17 May 2026 20:06:52 -0400 Subject: [PATCH] NEO-48: add implementation plan for perk-state HTTP Kickoff plan for GET/POST /game/players/{id}/perk-state, Bruno, and API tests; records user choices on route path and unlockedEvents on success. --- docs/plans/NEO-48-implementation-plan.md | 124 +++++++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 docs/plans/NEO-48-implementation-plan.md diff --git a/docs/plans/NEO-48-implementation-plan.md b/docs/plans/NEO-48-implementation-plan.md new file mode 100644 index 0000000..6be9695 --- /dev/null +++ b/docs/plans/NEO-48-implementation-plan.md @@ -0,0 +1,124 @@ +# NEO-48 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-48 | +| **Title** | E2.M3: Perk state GET + branch selection POST + Bruno | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-48/e2m3-perk-state-get-branch-selection-post-bruno | +| **Module** | [E2.M3 — MasteryAndPerkUnlocks](../decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md) · Epic 2 Slice 4 | +| **Branch** | `NEO-48-perk-state-get-branch-selection-post-bruno` | +| **Blocked by** | [NEO-47](https://linear.app/neon-sprawl/issue/NEO-47) — **Done** on `main` (`PerkUnlockEngine`, `IPlayerPerkStateStore`, `PerkUnlockReasonCodes`). | + +## Kickoff clarifications + +| Topic | Question | Resolution | +|--------|----------|------------| +| **POST URL** | Same resource path as GET vs dedicated sub-path? | **User:** **`POST /game/players/{id}/perk-state`** (same as GET; mirrors skill-progression / hotbar-loadout). | +| **POST success body** | Include unlock events from `PerkUnlockEngine`? | **User:** **Yes** — `selected: true` + full **`perkState`** snapshot + **`unlockedEvents[]`** (mirrors NEO-38 `levelUps` on grant). | +| **GET JSON shape** | Nested maps vs arrays? | **Agent (repo pattern):** serialize branch picks as a **`branchPicks`** array of `{ skillId, picks: [{ tierIndex, branchId }] }` (ordinal sort by `skillId`; picks sorted by `tierIndex`). **`unlockedPerkIds`** as a sorted string array. Internal `PerkStateSnapshot` mapping only — no rename of persisted fields. | +| **Deny envelope** | Field names for success/deny? | **Agent (NEO-38 mirror):** HTTP **200** with **`selected`** (`true`/`false`), optional **`reasonCode`** (stable codes from `PerkUnlockReasonCodes`), and authoritative **`perkState`** on both paths. **400** for missing/wrong `schemaVersion`; **404** when player absent from position state (same gate as skill-progression GET). | +| **Telemetry at HTTP** | Duplicate NEO-49 hook comments on POST? | **No** — [NEO-49 plan](NEO-49-implementation-plan.md): engine `TryUnlockPerks` is the authoritative hook; HTTP calls `TrySelectBranch` and surfaces returned `Events` only in JSON. | + +## Goal, scope, and out-of-scope + +**Goal:** Versioned HTTP **GET** read model and **POST** branch selection for per-player perk state, delegating all rules to **`PerkUnlockEngine`** from NEO-47. Match NEO-37/NEO-38 versioning, player gate, and structured deny patterns. + +**In scope (from Linear):** + +- **`GET /game/players/{id}/perk-state`** — `schemaVersion` **1** JSON: branch picks + unlocked perk ids. +- **`POST /game/players/{id}/perk-state`** — commit one branch pick; structured success/deny; stable **`reasonCode`** values from engine. +- Bruno under **`bruno/neon-sprawl-server/perk-state/`** (GET + POST happy + deny). +- **`server/README.md`** section; API integration tests (AAA). +- **`docs/manual-qa/NEO-48.md`** during implementation (repo convention for HTTP stories). + +**Out of scope (from Linear):** + +- Godot client HUD. +- Applying perk `effectKind` in gather/craft. +- New telemetry hooks (NEO-49 complete). +- Respec; gig perks. + +## Acceptance criteria checklist + +- [ ] `GET /game/players/{id}/perk-state` returns versioned JSON v1. +- [ ] `POST` branch selection applies NEO-47 unlock rules; structured denies on invalid tier/branch. +- [ ] Bruno collection `bruno/neon-sprawl-server/perk-state/` (GET + POST + deny). +- [ ] `server/README.md` section for perk-state endpoints. +- [ ] Unit/integration tests (AAA) for API happy/deny paths. + +## Technical approach + +1. **Routes (`Game/Mastery/PerkStateApi.cs`):** + - **`GET /game/players/{id}/perk-state`** — trim id; **404** if `!IPositionStateStore.TryGetPosition`; else **200** + `PerkStateSnapshotResponse` built from `IPlayerPerkStateStore.GetSnapshot`. + - **`POST /game/players/{id}/perk-state`** — body `PerkBranchSelectRequest` (`schemaVersion` **1**, `skillId`, `tierIndex`, `branchId`). **400** if body null or `schemaVersion != 1`. **404** same player gate. Call **`perkUnlockEngine.TrySelectBranch(trimmedId, skillId, tierIndex, branchId)`**; map outcome to `PerkBranchSelectResponse`. + +2. **Snapshot builder:** `PerkStateApi.BuildSnapshot(playerId, IPlayerPerkStateStore)` maps `PerkStateSnapshot` → JSON DTO: + - **`branchPicks`:** one entry per skill with any picks; each **`picks`** row has `tierIndex` + `branchId`. + - **`unlockedPerkIds`:** sorted ordinal list from set. + - Preserve **`schemaVersion` 1**, **`playerId`**. + +3. **POST success (`selected: true`):** + - **`perkState`:** same shape as GET body (without re-wrapping `playerId` if shared type — use shared `PerkStateSnapshotResponse` or embed identical fields). + - **`unlockedEvents`:** map each `PerkUnlockEvent` → `{ perkId, skillId, tierIndex, branchId, source }` where **`source`** is JSON string **`branch_pick`** | **`level_up`** (snake_case; maps `PerkUnlockSource.BranchPick` / `LevelUp`). Empty array when tier-1 pick unlocks no perks yet. + +4. **POST deny (`selected: false`):** + - **`reasonCode`:** pass through engine `ReasonCode` (`unknown_track`, `unknown_tier`, `unknown_branch`, `level_too_low`, `branch_already_chosen`, `tier_branch_not_selectable`, `player_not_found`). + - **`perkState`:** authoritative snapshot from outcome (unchanged on deny except where engine returns updated snapshot — use `outcome.Snapshot` always). + - **`unlockedEvents`:** empty array on deny. + +5. **Wire-up:** `app.MapPerkStateApi()` in **`Program.cs`** after mastery/perk DI is available (`PerkUnlockEngine` already registered in `AddPerkStateStore`). No new store or engine logic in this story. + +6. **Bruno / README / manual QA:** + - Sequence: optional **`POST …/skill-progression`** grant to reach salvage level 2, then **POST perk-state** tier-1 pick, **GET** verifies pick; deny example: second pick or `level_too_low` without grant. + - Document paths, schema fields, reason codes, and link to this plan + **`docs/manual-qa/NEO-48.md`**. + +7. **Module doc (implementation batch):** Update [E2_M3](../decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md) NEO-47 handoff line to note HTTP landed; refresh [documentation_and_implementation_alignment.md](../decomposition/modules/documentation_and_implementation_alignment.md) E2.M3 row when complete. + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Mastery/PerkStateDtos.cs` | `PerkStateSnapshotResponse`, `PerkBranchPickTrackJson`, `PerkBranchPickRowJson`, `PerkBranchSelectRequest`, `PerkBranchSelectResponse`, `PerkUnlockedEventJson`. | +| `server/NeonSprawl.Server/Game/Mastery/PerkStateApi.cs` | `MapPerkStateApi`, `BuildSnapshot`, GET/POST handlers. | +| `server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs` | In-memory host: GET empty defaults; POST tier-1 happy after XP grant; deny `level_too_low` / `branch_already_chosen`; GET reflects POST; **404** / **400** gates. | +| `bruno/neon-sprawl-server/perk-state/folder.bru` | Collection folder metadata. | +| `bruno/neon-sprawl-server/perk-state/Get perk state.bru` | GET smoke + schema assertions. | +| `bruno/neon-sprawl-server/perk-state/Post branch select tier1.bru` | Happy path (docs: grant salvage XP first if needed). | +| `bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru` | Deny without level gate. | +| `docs/manual-qa/NEO-48.md` | Curl/Bruno checklist for GET/POST/deny. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Program.cs` | Register **`MapPerkStateApi()`**. | +| `server/README.md` | New **Perk state (NEO-48)** section: GET/POST paths, schema v1, `selected`/`reasonCode`/`unlockedEvents`, reason code table (link `PerkUnlockReasonCodes`), Bruno path, manual QA link. | +| `docs/decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md` | Note NEO-48 HTTP landed (replace “HTTP is NEO-48” pending wording in NEO-47 handoff). | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E2.M3 row: NEO-48 perk-state HTTP. | +| `docs/decomposition/modules/module_dependency_register.md` | E2.M3 note: NEO-48 planned → landed when shipped. | + +## Tests + +| Test file | What it covers | +|-----------|----------------| +| `PerkStateApiTests.cs` | **AAA** integration via `InMemoryWebApplicationFactory`: **GET** returns v1 empty state for dev player; **POST** after `POST …/skill-progression` grant (100 XP → level 2) selects `scrap_efficiency` tier 1 → `selected: true`, pick in **GET**; **POST** deny `level_too_low` without grant; **POST** deny `branch_already_chosen` on second pick; **404** unknown player; **400** bad `schemaVersion`. Optional: **POST** at level 4+ includes non-empty **`unlockedEvents`** when tier-2 perk unlocks on same request (path-auto). | +| Existing `PerkUnlockEngineTests.cs` | No changes required — engine rules remain covered; API tests are thin HTTP mapping. | + +Bruno + **`docs/manual-qa/NEO-48.md`** supplement automated coverage. Postgres persistence of picks is already covered by **`PerkStatePersistenceIntegrationTests`** (NEO-47); no new Postgres API test unless a regression gap appears during implementation. + +## Open questions / risks + +- **Field name `selected` vs `granted`:** Chosen to distinguish perk branch pick from XP grant; document clearly in README to avoid client confusion. +- **Large `unlockedEvents` batches:** Prototype catalog emits at most a handful per request; no pagination needed. +- **Concurrent POST picks:** Same as NEO-47 store semantics; no new transactional requirement for this story. + +## Decisions + +| Decision | Rationale | +|----------|-----------| +| POST on same path as GET | User choice; matches skill-progression / hotbar. | +| Include `unlockedEvents` on success | User choice; parallels `levelUps`; supports client/QA without parsing perk id diffs. | +| Branch picks as sorted arrays in JSON | Stable serialization; consistent with skill-progression “unordered by contract, sorted for bytes” pattern. | +| Reuse NEO-47 `reasonCode` strings verbatim | README + tests + Bruno share one vocabulary with engine. |