76 lines
5.5 KiB
Markdown
76 lines
5.5 KiB
Markdown
# 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.
|