From 178ff2b62505f84aa13831c80f16bb2c55bf9cbe Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 10 May 2026 18:48:18 -0400 Subject: [PATCH] NEO-40: add implementation plan for telemetry hook sites --- docs/plans/NEO-40-implementation-plan.md | 75 ++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 docs/plans/NEO-40-implementation-plan.md diff --git a/docs/plans/NEO-40-implementation-plan.md b/docs/plans/NEO-40-implementation-plan.md new file mode 100644 index 0000000..4067678 --- /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 + +- [ ] 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.