NEO-40: add implementation plan for telemetry hook sites

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
+
+- [ ] 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).
+- [ ] **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 |
+|------|---------|
+| **none** | Hook sites are **comment markers** on the existing NEO-38 route; no new types or files required. |
+
+## 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` | **Only if** a single sentence improves traceability from module doc → hook file; otherwise **none** (YAGNI). |
+
+## 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.