From f3e48f40777cdec8331a41f4ea359a6af68545e1 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 17 May 2026 19:08:54 -0400 Subject: [PATCH] NEO-49: kickoff implementation plan for perk_unlock telemetry hooks --- docs/plans/NEO-49-implementation-plan.md | 91 ++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 docs/plans/NEO-49-implementation-plan.md diff --git a/docs/plans/NEO-49-implementation-plan.md b/docs/plans/NEO-49-implementation-plan.md new file mode 100644 index 0000000..ec4e4d9 --- /dev/null +++ b/docs/plans/NEO-49-implementation-plan.md @@ -0,0 +1,91 @@ +# NEO-49 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-49 | +| **Title** | E2.M3: perk_unlock telemetry hook sites | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-49/e2m3-perk-unlock-telemetry-hook-sites | +| **Module** | [E2.M3 — MasteryAndPerkUnlocks](../decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md) · Epic 2 Slice 4 | +| **Branch** | `NEO-49-perk-unlock-telemetry-hook-sites` | +| **Blocked by** | [NEO-47](https://linear.app/neon-sprawl/issue/NEO-47) — **Done** on `main` (`PerkUnlockEngine`, `PerkUnlockEvent`). May land in parallel with [NEO-48](https://linear.app/neon-sprawl/issue/NEO-48) (perk HTTP). | + +## Kickoff clarifications + +| Topic | Question / note | Resolution | +|--------|-----------------|------------| +| **Hook runtime behavior** | Comments-only vs dev `ILogger` (same as NEO-40)? | **User:** **comments-only** — no runtime emit, no production logging. | +| **Hook anchor** | Single site in `TryUnlockPerks` vs duplicate markers at callers? | User asked for recommendation → **Agent recommendation (adopted):** **single anchor** in `PerkUnlockEngine.TryUnlockPerks` immediately after `PerkUnlockEvent` instances are built (covers branch-pick unlocks, path-auto on pick, and level-up reevaluation). **Do not** duplicate at `SkillProgressionGrantOperations` or future NEO-48 HTTP — those call the engine and already receive `Events`; E9.M1 wiring can subscribe at the engine or via a future `IPerkUnlockEventSink` (NEO-47 plan note). | +| **Branch-select without unlock** | Hook on successful `TrySelectBranch` when `Events` is empty? | **No** — catalog event is **`perk_unlock`** only when a perk id is newly persisted; tier-1 pick-without-perk is not an unlock event. | +| **Deny paths** | `perk_branch_select_denied` hooks? | **Out of scope** (Linear); only **`perk_unlock`**. | + +## Goal, scope, and out-of-scope + +**Goal:** Document and place **comment-only hook sites** on the server-authoritative perk unlock path for future E9.M1 catalog event **`perk_unlock`**. Align with Epic 2 Slice 4 telemetry vocabulary ([epic_02 — Slice 4](../decomposition/epics/epic_02_skills_and_progression.md)). + +**In scope (from Linear + E2M3-05):** + +- Hook location(s) in unlock / branch-select path documented in **code comments** with **`TODO(E9.M1)`**. +- **`server/README.md`** + [E2_M3 module](../decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md) note hook placement. +- Manual QA checklist per repo convention. + +**Out of scope (from Linear):** + +- E9.M1 catalog, ingest, dashboards. +- `ILogger` / metrics / dev-only log lines (rejected at kickoff). +- Optional `IPerkUnlockEventSink` implementation (deferred; comments-only this story). + +## Acceptance criteria checklist + +- [ ] Hook location(s) in unlock / branch-select path documented in code comments. +- [ ] `TODO(E9.M1)` alignment; no production logging calls. +- [ ] Module doc + server README note hook placement. + +## Technical approach + +1. **Authoritative unlock surface:** `PerkUnlockEngine.TryUnlockPerks` — the only place new perk ids are persisted and `PerkUnlockEvent` records are created. All paths funnel here: + - **`TrySelectBranch`** → explicit pick tier perks + `ApplyPathAutoTiersAtOrBelowLevel` (may emit zero events on tier-1-only pick). + - **`ReevaluateAfterLevelUp`** → path-auto tiers after skill level-up (called from `SkillProgressionGrantOperations` and any future callers). + +2. **Hook site — `perk_unlock`:** After building the `events` list (one `PerkUnlockEvent` per newly unlocked `perkId`), add a **comment block** before `return true`: + - Name future E9.M1 event **`perk_unlock`**. + - **`TODO(E9.M1): catalog emit`** (match NEO-30 / NEO-40 style). + - Document payload fields from `PerkUnlockEvent`: `playerId`, `perkId`, `skillId`, `tierIndex`, `branchId`, `source` (`BranchPick` | `LevelUp`). + - Note: emit **once per unlocked perk** in the batch (idempotent reevaluation produces no events when already unlocked). + +3. **Cross-references (minimal):** + - One-line pointer in `PerkUnlockEvent.cs` xml summary → hook site in `TryUnlockPerks`. + - **`server/README.md`:** new short subsection under mastery (after NEO-46 block) for perk unlock engine + **NEO-49 telemetry hook** anchor. + - **`E2_M3_MasteryAndPerkUnlocks.md`:** replace “telemetry in NEO-49” backlog wording with **NEO-49 landed** line when implementation completes. + - **`documentation_and_implementation_alignment.md`:** E2.M3 row cites NEO-49 hook sites. + +4. **NEO-48 readiness:** When perk HTTP lands, it will call `TrySelectBranch` and surface `Events`; no extra hook comments required at the API layer if the engine anchor exists. + +## Files to add + +| Path | Purpose | +|------|---------| +| `docs/manual-qa/NEO-49.md` | Manual QA: verify comment blocks + no behavior/API change. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/Mastery/PerkUnlockEngine.cs` | **NEO-49:** comment-only `perk_unlock` hook site in `TryUnlockPerks` when events are created. | +| `server/NeonSprawl.Server/Game/Mastery/PerkUnlockEvent.cs` | Cross-reference hook anchor in type summary (no behavior change). | +| `server/README.md` | Document perk unlock telemetry hook placement (NEO-49). | +| `docs/decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md` | Implementation snapshot: NEO-49 telemetry hooks landed. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E2.M3 row: NEO-49 hook sites (still no E9.M1 ingest). | + +## Tests + +| Path | Change | +|------|--------| +| **None required** | Comments-only; existing `PerkUnlockEngineTests` and grant integration tests already cover unlock paths. | + +**Manual verification:** `docs/manual-qa/NEO-49.md` — open `PerkUnlockEngine.cs`, confirm hook cites **`perk_unlock`**, **`TODO(E9.M1)`**, payload fields, and no new log statements; run `dotnet test` on server solution for regression sanity. + +## Open questions / risks + +**None.** NEO-47 blocker is complete; comments-only avoids production logging risk. If E9.M1 later needs caller context (HTTP request id), prefer `IPerkUnlockEventSink` at call sites without moving the primary hook documentation away from `TryUnlockPerks`.