diff --git a/bruno/neon-sprawl-server/skill-progression/Post skill progression grant.bru b/bruno/neon-sprawl-server/skill-progression/Post skill progression grant.bru index 8bfa23d..aa0450f 100644 --- a/bruno/neon-sprawl-server/skill-progression/Post skill progression grant.bru +++ b/bruno/neon-sprawl-server/skill-progression/Post skill progression grant.bru @@ -6,6 +6,8 @@ meta { docs { NEO-38: single skill XP grant + allowlist + level-up metadata; see server README. + NEO-40: same contract; server adds comment-only telemetry hook markers (no request/response change). + E2.M2 tracking row in docs/decomposition/modules/documentation_and_implementation_alignment.md lists NEO-40 landed with plan + manual QA links. } post { diff --git a/docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md b/docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md index 36552b2..781f28d 100644 --- a/docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md +++ b/docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md @@ -19,7 +19,9 @@ **NEO-39 landed:** data-driven **`LevelCurve`** at `content/skills/prototype_level_curve.json` with schema `content/schemas/level-curve.schema.json`; CI validation extended in `scripts/validate_content.py`; server boot now fail-fast loads curve content and uses it for XP→level resolution in skill progression GET/POST paths (no code edit required for threshold tuning). -**Still backlog within this module:** telemetry hook sites (**NEO-40**); callers in Slice 3 (**NEO-41**–**NEO-43**). Keep this paragraph and [documentation tracking](documentation_and_implementation_alignment.md) in sync as Slice 2 stories merge. +**NEO-40 landed:** comment-only **telemetry hook sites** in `SkillProgressionSnapshotApi` (`POST …/skill-progression`) for future catalog events **`xp_grant`** and **`level_up`** ([implementation plan](../../plans/NEO-40-implementation-plan.md)); manual QA **[`NEO-40.md`](../../manual-qa/NEO-40.md)**. + +**Still backlog within this module:** callers in Slice 3 (**NEO-41**–**NEO-43**). Keep this paragraph and [documentation tracking](documentation_and_implementation_alignment.md) in sync as Slice 2 stories merge. ## Purpose diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index b9a7c91..4be96e2 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -51,7 +51,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not | E1.M3 | In Progress | **NEO-23 landed:** JSON v1 targeting read + select (`GET`/`POST …/target`, `Game/Targeting/`, [NEO-23](../../plans/NEO-23-implementation-plan.md)) — `PlayerTargetStateResponse` / soft lock / `reasonCode` denials; wire name differs from illustrative **`TargetState`** in module doc (called out in plan + README). **NEO-24 landed:** client tab-target + lock HUD synced to server ([NEO-24](../../plans/NEO-24-implementation-plan.md)) — `TargetSelectionClient` ([`client/scripts/target_selection_client.gd`](../../../client/scripts/target_selection_client.gd)), Tab / Esc bindings, `TargetLockLabel` HUD, hybrid movement-triggered refresh via new `PositionAuthorityClient.authoritative_ack` signal (fires on every server-confirmed position, boot + `move-stream` 200) with a 250 ms cooldown; manual QA in [`docs/manual-qa/NEO-24.md`](../../manual-qa/NEO-24.md). **NEO-25 landed:** versioned **`GET /game/world/interactables`** ([NEO-25](../../plans/NEO-25-implementation-plan.md)) — `InteractablesListResponse` / `InteractablesWorldApi` in `server/NeonSprawl.Server/Game/Interaction/`; Godot `interactables_catalog_client.gd` + `interactable_world_builder.gd` (fetch-driven props + glow); [server README — Interactable descriptors (NEO-25)](../../../server/README.md#interactable-descriptors-neo-25). **NEO-26 landed:** prototype **`SelectionEvent`** — **`TargetSelectionClient.selection_event`** ([NEO-26](../../plans/NEO-26-implementation-plan.md)) when **`lockedTargetId`** changes; optional **`log_selection_events`**. **NEO-27 landed:** Slice 3 telemetry hook sites documented — `selection_event` maps to product `target_changed` in `target_selection_client.gd`; server lock transition hook comments in `InMemoryPlayerTargetLockStore`; reserved `ability_cast_requested` / `ability_cast_denied` comments in `client/scripts/main.gd` (all TODO(E9.M1)); see [NEO-27](../../plans/NEO-27-implementation-plan.md) and [`docs/manual-qa/NEO-27.md`](../../manual-qa/NEO-27.md). **Follow-on / still open:** richer **`InteractableDescriptor`** consumers beyond list projection + existing `POST …/interact`; production telemetry ingest/catalog after E9.M1. **Precursor (E1.M1):** [NEO-9](../../plans/NEO-9-implementation-plan.md) `POST …/interact`. | Linear [NEO-24](https://linear.app/neon-sprawl/issue/NEO-24)–[NEO-27](https://linear.app/neon-sprawl/issue/NEO-27); [E1_M3_InteractionAndTargetingLayer.md](E1_M3_InteractionAndTargetingLayer.md); [E1M3 prototype backlog](../../plans/E1M3-prototype-backlog.md); [server README — Targeting](../../../server/README.md#targeting-neo-23), [Interactable descriptors (NEO-25)](../../../server/README.md#interactable-descriptors-neo-25), [Interaction](../../../server/README.md#interaction-neo-9) | | E1.M4 | In Progress | **NEO-29 landed:** prototype `HotbarLoadout` v1 server contract (`GET`/`POST /game/players/{id}/hotbar-loadout`) with stable deny reason codes, per-player scope, and persistence policy matching NEO-8/NS-17 (Postgres when configured, in-memory fallback otherwise). Client boot hydration is wired via `hotbar_loadout_client.gd` + `hotbar_state.gd` from `main.gd`; manual QA checklist created. **NEO-31 landed:** prototype **`POST /game/players/{id}/ability-cast`**, digit-key cast wiring from `main.gd`, `ability_cast_client.gd`, and `hotbar_cast_slot_resolver.gd` (target id from `lockedTargetId` in cached target state); GdUnit covers resolver + HTTP client; manual QA [NEO-31](../../manual-qa/NEO-31.md). **NEO-28 landed:** cast POST validates **lock + registry + range** (`invalid_target`, `out_of_range`); **`AbilityCastClient.cast_result_received`** + **`CastFeedbackLabel`** HUD line; manual QA [NEO-28](../../manual-qa/NEO-28.md). **NEO-30 landed:** Slice 3 cast funnel telemetry **hook-site comments** in [`AbilityCastApi.cs`](../../../server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs) (`ability_cast_requested` on authoritative accept, `ability_cast_denied` + non-empty `reasonCode` on each JSON deny branch); client dev hooks unchanged; manual QA [NEO-30](../../manual-qa/NEO-30.md); plan [NEO-30](../../plans/NEO-30-implementation-plan.md). **NEO-32 landed:** `GET /game/players/{id}/cooldown-snapshot` + `on_cooldown` cast deny + prototype global cooldown commit; [`cooldown_snapshot_client.gd`](../../../client/scripts/cooldown_snapshot_client.gd), [`cooldown_state.gd`](../../../client/scripts/cooldown_state.gd), **`CooldownSlotsLabel`** in [`main.gd`](../../../client/scripts/main.gd); manual QA [NEO-32](../../manual-qa/NEO-32.md); plan [NEO-32](../../plans/NEO-32-implementation-plan.md). | [NEO-29](../../plans/NEO-29-implementation-plan.md), [NEO-31](../../plans/NEO-31-implementation-plan.md), [NEO-28](../../plans/NEO-28-implementation-plan.md), [NEO-30](../../plans/NEO-30-implementation-plan.md), [NEO-32](../../plans/NEO-32-implementation-plan.md); [E1_M4_AbilityInputScaffold.md](E1_M4_AbilityInputScaffold.md); [E1M4 prototype backlog](../../plans/E1M4-prototype-backlog.md); `server/NeonSprawl.Server/Game/AbilityInput/`; [`client/scripts/hotbar_loadout_client.gd`](../../../client/scripts/hotbar_loadout_client.gd), [`client/scripts/hotbar_state.gd`](../../../client/scripts/hotbar_state.gd), [`client/scripts/ability_cast_client.gd`](../../../client/scripts/ability_cast_client.gd), [`client/scripts/hotbar_cast_slot_resolver.gd`](../../../client/scripts/hotbar_cast_slot_resolver.gd), [`client/scripts/cooldown_snapshot_client.gd`](../../../client/scripts/cooldown_snapshot_client.gd), [`client/scripts/cooldown_state.gd`](../../../client/scripts/cooldown_state.gd); [server README — Hotbar loadout](../../../server/README.md#hotbar-loadout-neo-29), [Ability cast (NEO-31)](../../../server/README.md#ability-cast-neo-31), [Cooldown snapshot (NEO-32)](../../../server/README.md#cooldown-snapshot-neo-32) | | E2.M1 | In Progress | **NEO-33 landed:** frozen prototype trio **`salvage`** / **`refine`** / **`intrusion`** in [`content/skills/prototype_skills.json`](../../../content/skills/prototype_skills.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) enforce schema, duplicate `id`, exact trio ids, and category coverage; designer note on **`allowedXpSourceKinds`** in [E2_M1](E2_M1_SkillDefinitionRegistry.md) + [`content/README.md`](../../../content/README.md). **NEO-34 landed:** fail-fast server load of `content/skills/*.json` at startup — `server/NeonSprawl.Server/Game/Skills/` ([NEO-34](../../plans/NEO-34-implementation-plan.md)); config + discovery in [server README — Skill catalog](../../../server/README.md#skill-catalog-contentskills-neo-34). **NEO-35 landed:** `ISkillDefinitionRegistry` / `SkillDefinitionRegistry` + DI ([NEO-35](../../plans/NEO-35-implementation-plan.md)). **NEO-36 landed:** versioned **`GET /game/world/skill-definitions`** ([NEO-36](../../plans/NEO-36-implementation-plan.md)) — `SkillDefinitionsWorldApi` + `SkillDefinitionsListDtos` in `server/NeonSprawl.Server/Game/Skills/`; Bruno `bruno/neon-sprawl-server/skill-definitions/`; manual QA [`NEO-36`](../../manual-qa/NEO-36.md). **E2.M2 consumer (NEO-38 landed):** grant path validates `skillId` + `sourceKind` vs catalog **`allowedXpSourceKinds`** on **`POST …/skill-progression`** ([NEO-38](../../plans/NEO-38-implementation-plan.md)); see [server README — Skill progression grant](../../../server/README.md#skill-progression-grant-neo-38) and [E2_M2](E2_M2_XpAwardAndLevelEngine.md). | [NEO-33](../../plans/NEO-33-implementation-plan.md), [NEO-34](../../plans/NEO-34-implementation-plan.md), [NEO-35](../../plans/NEO-35-implementation-plan.md), [NEO-36](../../plans/NEO-36-implementation-plan.md); [E2_M1](E2_M1_SkillDefinitionRegistry.md); [module_dependency_register.md](module_dependency_register.md) **E2.M1 note**; `server/NeonSprawl.Server/Game/Skills/`; [`docs/manual-qa/NEO-36.md`](../../manual-qa/NEO-36.md); [server README — Skill definitions (NEO-36)](../../../server/README.md#skill-definitions-neo-36) | -| E2.M2 | In Progress | **NEO-37 landed:** versioned **`GET /game/players/{id}/skill-progression`** ([NEO-37](../../plans/NEO-37-implementation-plan.md)) — read model for every registered skill; known-player gate via `IPositionStateStore`; [server README — Skill progression snapshot (NEO-37)](../../../server/README.md#skill-progression-snapshot-neo-37); manual QA [`NEO-37`](../../manual-qa/NEO-37.md). **NEO-38 landed:** **`POST`** same path — grant apply, persistence (`IPlayerSkillProgressionStore`, `V003` migration), structured denies + **`levelUps`** ([NEO-38](../../plans/NEO-38-implementation-plan.md)); manual QA [`NEO-38`](../../manual-qa/NEO-38.md); Bruno `bruno/neon-sprawl-server/skill-progression/`; [server README — Skill progression grant (NEO-38)](../../../server/README.md#skill-progression-grant-neo-38). **NEO-39 landed:** data-driven `LevelCurve` content + schema + CI validation (`*_level_curve.json`) with fail-fast startup schema checks; progression GET/POST level resolution now uses `ISkillLevelCurve` content-backed thresholds ([NEO-39](../../plans/NEO-39-implementation-plan.md)); manual QA [`NEO-39`](../../manual-qa/NEO-39.md). **Follow-on:** [NEO-40](https://linear.app/neon-sprawl/issue/NEO-40) (telemetry hook sites). Slice 3 integration — [NEO-41](https://linear.app/neon-sprawl/issue/NEO-41) / [NEO-42](https://linear.app/neon-sprawl/issue/NEO-42) / [NEO-43](https://linear.app/neon-sprawl/issue/NEO-43); [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). See [epic_02 — E2.M2 + Slice 3](../epics/epic_02_skills_and_progression.md). | [NEO-37](../../plans/NEO-37-implementation-plan.md), [NEO-38](../../plans/NEO-38-implementation-plan.md), [NEO-39](../../plans/NEO-39-implementation-plan.md), [E2_M2](E2_M2_XpAwardAndLevelEngine.md); label **`E2.M2`** on NEO-37–NEO-40, NEO-41–NEO-43 | +| E2.M2 | In Progress | **NEO-37 landed:** versioned **`GET /game/players/{id}/skill-progression`** ([NEO-37](../../plans/NEO-37-implementation-plan.md)) — read model for every registered skill; known-player gate via `IPositionStateStore`; [server README — Skill progression snapshot (NEO-37)](../../../server/README.md#skill-progression-snapshot-neo-37); manual QA [`NEO-37`](../../manual-qa/NEO-37.md). **NEO-38 landed:** **`POST`** same path — grant apply, persistence (`IPlayerSkillProgressionStore`, `V003` migration), structured denies + **`levelUps`** ([NEO-38](../../plans/NEO-38-implementation-plan.md)); manual QA [`NEO-38`](../../manual-qa/NEO-38.md); Bruno `bruno/neon-sprawl-server/skill-progression/`; [server README — Skill progression grant (NEO-38)](../../../server/README.md#skill-progression-grant-neo-38). **NEO-39 landed:** data-driven `LevelCurve` content + schema + CI validation (`*_level_curve.json`) with fail-fast startup schema checks; progression GET/POST level resolution now uses `ISkillLevelCurve` content-backed thresholds ([NEO-39](../../plans/NEO-39-implementation-plan.md)); manual QA [`NEO-39`](../../manual-qa/NEO-39.md). **NEO-40 landed:** comment-only telemetry hook sites in [`SkillProgressionSnapshotApi.cs`](../../../server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs) on **`POST …/skill-progression`** for future **`xp_grant`** / **`level_up`** ([NEO-40](../../plans/NEO-40-implementation-plan.md), [`NEO-40` manual QA](../../manual-qa/NEO-40.md)). **Slice 3 integration** — [NEO-41](https://linear.app/neon-sprawl/issue/NEO-41) / [NEO-42](https://linear.app/neon-sprawl/issue/NEO-42) / [NEO-43](https://linear.app/neon-sprawl/issue/NEO-43); [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). See [epic_02 — E2.M2 + Slice 3](../epics/epic_02_skills_and_progression.md). | [NEO-37](../../plans/NEO-37-implementation-plan.md), [NEO-38](../../plans/NEO-38-implementation-plan.md), [NEO-39](../../plans/NEO-39-implementation-plan.md), [NEO-40](../../plans/NEO-40-implementation-plan.md), [E2_M2](E2_M2_XpAwardAndLevelEngine.md); label **`E2.M2`** on NEO-37–NEO-40, NEO-41–NEO-43 | --- diff --git a/docs/manual-qa/NEO-40.md b/docs/manual-qa/NEO-40.md new file mode 100644 index 0000000..c00365e --- /dev/null +++ b/docs/manual-qa/NEO-40.md @@ -0,0 +1,32 @@ +# Manual QA — NEO-40 (progression telemetry hook sites) + +Reference: [implementation plan](../plans/NEO-40-implementation-plan.md), [NEO-38 manual QA](./NEO-38.md) (same server surface). + +## Preconditions + +- Same as NEO-38: run `NeonSprawl.Server` (`dotnet run` from `server/NeonSprawl.Server`; default `http://localhost:5253`). +- This story adds **comments only**; behavior matches NEO-38. + +```bash +BASE=http://localhost:5253 +ID=dev-local-1 +``` + +## Verify API unchanged + +1. **Grant** (same as NEO-38 happy path): + +```bash +curl -sS -i -X POST "${BASE}/game/players/${ID}/skill-progression" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"skillId":"salvage","amount":10,"sourceKind":"activity"}' +``` + +Expect **HTTP 200**, `granted` **true**, `progression` and `levelUps` present as before. + +2. **Level-up path** (optional, same as NEO-38 bonus): large grant on `intrusion` if XP crosses the curve threshold; expect `levelUps` non-empty when a boundary is crossed. + +## Code review check + +- Open `server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs` and confirm **comment blocks** mark **`xp_grant`** (after successful `TryApplyXpDelta`) and **`level_up`** (inside `nextLevel > prevLevel`), referencing **E9.M1** / no production ingest. +- Confirm there are **no** new `ILogger` or metrics calls for these events. diff --git a/docs/plans/NEO-40-implementation-plan.md b/docs/plans/NEO-40-implementation-plan.md new file mode 100644 index 0000000..3d58f26 --- /dev/null +++ b/docs/plans/NEO-40-implementation-plan.md @@ -0,0 +1,75 @@ +# NEO-40 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-40 | +| **Title** | Skill XP / level-up telemetry hook sites | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-40/skill-xp-level-up-telemetry-hook-sites | +| **Module** | [E2.M2 — XpAwardAndLevelEngine](../decomposition/modules/E2_M2_XpAwardAndLevelEngine.md) | +| **Depends on** | [NEO-38](https://linear.app/neon-sprawl/issue/NEO-38) — **Done** on `main` (authoritative grant + level-up). | +| **Related** | [NEO-39](https://linear.app/neon-sprawl/issue/NEO-39) may land in parallel (curve content); hooks stay in the same API surface. | + +## Kickoff clarifications + +| Topic | Question / note | Resolution | +|--------|-----------------|------------| +| **Hook runtime behavior** | Linear: “comments / no-op or dev-only logging **as agreed at kickoff**.” User asked for recommendation. | **Agent:** Prefer **comments-only** hook markers (no `ILogger` / no runtime emission) to eliminate any risk of misconfigured production logging. **User:** confirmed **comments only**. | + +## Goal, scope, and out-of-scope + +**Goal:** Document and place **hook sites** in the server-authoritative grant + level-up path for progression telemetry aligned with Epic 2 Slice 2: future catalog events **`xp_grant`** and **`level_up`**, plus documented intent for **time-to-first-level-up** (no code path until milestone/session data exists). **No production ingest** until **E9.M1** catalog — satisfied by **comments only** (no logging calls). + +**In scope (from Linear):** + +- Hook locations on the **successful grant** path and **level-up detection** path; same story documents them in this plan and in **code comments** referencing future event names. +- Ensure **no** high-volume or accidental production logging for these events. + +**Out of scope (from Linear + kickoff):** + +- Telemetry **catalog** / ingest (**E9.M1**). +- **`ILogger`**, metrics exporters, or dev-only log lines (explicitly rejected at kickoff in favor of comments-only). +- Persisting “first level-up” timestamps or session analytics (**time-to-first-level-up** remains design intent in comments + this plan only). + +## Acceptance criteria checklist + +- [x] Hook locations in **grant** + **level-up** path; documented here **and** in code comments referencing future **`xp_grant`** / **`level_up`** (and **time-to-first-level-up** intent where appropriate). +- [x] **No** mistaken high-volume logging to production pipelines (no new log statements for these hooks). + +## Technical approach + +1. **Primary surface:** `SkillProgressionSnapshotApi` **`POST /game/players/{id}/skill-progression`** — the only authoritative apply path after NEO-38. After validation passes and **`xpStore.TryApplyXpDelta`** succeeds, the persisted totals reflect the grant; after **`levelCurve.LevelFromTotalXp`**, compare **`prevLevel`** / **`nextLevel`** for level-up. + +2. **Hook site A — `xp_grant`:** Immediately after a **successful** `TryApplyXpDelta` (and before or alongside level derivation), add a **short comment block** naming the future **E9.M1** event **`xp_grant`**, listing payload fields that ingest will eventually need (e.g. `playerId`, `skillId`, `amount`, `sourceKind`, optional correlation ids when Slice 3 adds them). **No executable telemetry code.** + +3. **Hook site B — `level_up`:** On the branch **`nextLevel > prevLevel`**, before or inside construction of **`SkillLevelUpJson`**, add a comment block naming **`level_up`** and fields (`playerId`, `skillId`, `previousLevel`, `newLevel`, deltas). **No executable telemetry code.** + +4. **Time-to-first-level-up:** Add a **single** concise comment (near hook site B or in this plan only — prefer **one** code comment + bullet here) stating that this metric requires **first-seen** / milestone persistence or session context **not** present in NEO-40; future wiring will subscribe to **`level_up`** (or a dedicated milestone event) once catalog + storage exist. + +5. **Persistence layer:** **Do not** add duplicate hook comments inside **`IPlayerSkillProgressionStore`** implementations unless the team wants a second anchor; **prefer a single authoritative layer** (HTTP grant handler) to avoid drift. If decomposition doc should mention hook sites, add a **one-line** cross-reference in `docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md` only if that file already discusses telemetry — keep edits minimal. + +## Files to add + +| Path | Purpose | +|------|---------| +| `docs/manual-qa/NEO-40.md` | Manual QA checklist (API parity + comment review). | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs` | Insert **telemetry hook site** comments on the **successful grant** path (`xp_grant`) and **level-up** branch (`level_up`); optional one-line **time-to-first-level-up** intent. | +| `docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md` | **NEO-40 landed** line + backlog sentence updated (replaces “telemetry still backlog”). | + +## Tests + +| Path | Change | +|------|--------| +| **None required** | Comments-only markers have no runtime behavior; existing **`SkillProgressionGrantApiTests`** already cover grant + level-up paths. | + +**Manual verification:** During implementation, add **`docs/manual-qa/NEO-40.md`** per repo convention: confirm POST still behaves as today; visually verify comment blocks cite **`xp_grant`** / **`level_up`** and mention **E9.M1** / no production ingest. + +## Open questions / risks + +**None.** Blocker NEO-38 is complete; risk of production logging is avoided by **comments-only** decision. diff --git a/docs/reviews/2026-05-10-NEO-40.md b/docs/reviews/2026-05-10-NEO-40.md new file mode 100644 index 0000000..12c065d --- /dev/null +++ b/docs/reviews/2026-05-10-NEO-40.md @@ -0,0 +1,47 @@ +# Code review — NEO-40 (skill progression telemetry hook sites) + +**Date:** 2026-05-10 + +**Scope:** Branch `NEO-40-skill-xp-level-up-telemetry-hook-sites` vs `origin/main` (commits through `effd234`). + +**Base:** `origin/main` + +**Follow-up:** Suggestions and nits below are **done** (strikethrough + **Done.**). + +## Verdict + +**Approve** + +## Summary + +The branch adds comment-only telemetry hook markers on the authoritative `POST …/skill-progression` path after a successful `TryApplyXpDelta`, plus a second marker on the level-up branch. Behavior and JSON contracts are unchanged; documentation (implementation plan, manual QA, E2.M2 module note, Bruno doc string, implementation alignment table E2.M2 row) matches the comments-only kickoff decision. Server tests pass. + +## Documentation checked + +| Path | Result | +|------|--------| +| `docs/plans/NEO-40-implementation-plan.md` | **Matches** — hook sites after successful apply + on `nextLevel > prevLevel`; comments-only; no `ILogger`; time-to-first-level-up called out; single authoritative layer (HTTP handler). | +| `docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md` | **Matches** — NEO-40 landed paragraph + backlog narrowed to NEO-41–NEO-43. | +| `docs/decomposition/modules/module_dependency_register.md` | **N/A** for diff content (no table edit); E2.M2 row unchanged, still appropriate. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E2.M2 row includes **NEO-40 landed** (hook sites + plan + manual QA links); follow-on line removed. | +| `docs/decomposition/modules/contracts.md` / `client_server_authority.md` / `data_and_ops_policy.md` | **N/A** — no contract or authority change; telemetry ingest remains E9.M1 backlog, consistent with comments. | +| `docs/manual-qa/NEO-40.md` | **Matches** — API parity + comment spot-check aligns with plan. | + +~~**Register / tracking table:** After merge, update the E2.M2 cell in `documentation_and_implementation_alignment.md` to a “**NEO-40 landed:** …” bullet (and trim the old “Follow-on: NEO-40” line) so it matches `E2_M2_XpAwardAndLevelEngine.md`.~~ **Done.** (E2.M2 snapshot + Plans column updated in-repo.) + +## Blocking issues + +(none) + +## Suggestions + +1. ~~**`documentation_and_implementation_alignment.md` (E2.M2 row):** Replace the remaining “Follow-on: NEO-40” wording with a landed summary (hook sites in `SkillProgressionSnapshotApi`, link to `NEO-40` plan + manual QA), mirroring the style used for NEO-37–NEO-39 in the same cell. This satisfies the module doc’s instruction to keep that table in sync.~~ **Done.** + +## Nits + +- ~~**Nit:** The implementation plan’s technical approach for `level_up` mentions “deltas” in the payload sketch; the in-code comment lists `playerId`, `skillId`, `previousLevel`, `newLevel` but not XP/total deltas. Optional one-word addition (`prevXp`/`newXp` or total XP) if you want comment parity with the plan.~~ **Done.** (`SkillProgressionSnapshotApi` `level_up` hook comment lists `prevXp`, `newXp`.) + +## Verification + +- `dotnet test NeonSprawl.sln` — **Passed** (133 tests) on review machine. +- Optional: follow `docs/manual-qa/NEO-40.md` curl steps against a running server. diff --git a/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantApiTests.cs index bd8f174..57657a9 100644 --- a/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantApiTests.cs @@ -6,6 +6,7 @@ using Xunit; namespace NeonSprawl.Server.Tests.Game.Skills; +/// Regression tests for POST …/skill-progression (NEO-38). NEO-40 adds comment-only hook sites; JSON contract unchanged. E2.M2 implementation-alignment row documents NEO-40 landed. public sealed class SkillProgressionGrantApiTests { private static SkillProgressionGrantRequest Grant( diff --git a/server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs b/server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs index 0798e8c..65fd2a7 100644 --- a/server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs +++ b/server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs @@ -81,11 +81,20 @@ public static class SkillProgressionSnapshotApi return Results.NotFound(); } + // --- Telemetry hook site (NEO-40): future E9.M1 catalog event `xp_grant` --- + // No ingest here until the telemetry catalog exists. Planned payload fields: playerId, + // skillId, amount, sourceKind; optional correlation/request ids when Slice 3 wires callers. + var prevLevel = levelCurve.LevelFromTotalXp(prevXp); var nextLevel = levelCurve.LevelFromTotalXp(newXp); var levelUps = new List(); if (nextLevel > prevLevel) { + // --- Telemetry hook site (NEO-40): future E9.M1 catalog event `level_up` --- + // Planned fields: playerId, skillId, previousLevel, newLevel, prevXp, newXp (totals before/after this grant). No logging (comments-only). + // Time-to-first-level-up: needs first-seen / milestone persistence not present here; wire + // when catalog + storage exist (likely derived from `level_up` or a dedicated milestone). + levelUps.Add( new SkillLevelUpJson {