Merge pull request #74 from ViPro-Technologies/NEO-40-skill-xp-level-up-telemetry-hook-sites
NEO-40: Skill XP / level-up telemetry hook sitespull/76/head
GitHub
commit
9446bab292
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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 |
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
@ -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.
|
||||
|
|
@ -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.
|
||||
|
|
@ -6,6 +6,7 @@ using Xunit;
|
|||
|
||||
namespace NeonSprawl.Server.Tests.Game.Skills;
|
||||
|
||||
/// <summary>Regression tests for <c>POST …/skill-progression</c> (NEO-38). NEO-40 adds comment-only hook sites; JSON contract unchanged. E2.M2 implementation-alignment row documents NEO-40 landed.</summary>
|
||||
public sealed class SkillProgressionGrantApiTests
|
||||
{
|
||||
private static SkillProgressionGrantRequest Grant(
|
||||
|
|
|
|||
|
|
@ -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<SkillLevelUpJson>();
|
||||
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
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in New Issue