diff --git a/docs/decomposition/modules/module_dependency_register.md b/docs/decomposition/modules/module_dependency_register.md index 2a619ca..13d8f9d 100644 --- a/docs/decomposition/modules/module_dependency_register.md +++ b/docs/decomposition/modules/module_dependency_register.md @@ -36,7 +36,7 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen **E2.M2 note:** Epic 2 Slice 2 — [NEO-37](https://linear.app/neon-sprawl/issue/NEO-37) (progression snapshot **GET**) and [NEO-38](https://linear.app/neon-sprawl/issue/NEO-38) (grant **POST**, persistence, level-up payloads); label **`E2.M2`**. Module doc [E2_M2_XpAwardAndLevelEngine.md](E2_M2_XpAwardAndLevelEngine.md). Slice 3 integration: [NEO-41](https://linear.app/neon-sprawl/issue/NEO-41)–[NEO-43](https://linear.app/neon-sprawl/issue/NEO-43) landed; **NEO-44** (gig XP, Epic 5) remains backlog. Keep aligned with [documentation and implementation alignment](documentation_and_implementation_alignment.md). -**E2.M3 note:** Epic 2 Slice 4 — [NEO-45](https://linear.app/neon-sprawl/issue/NEO-45) → [NEO-49](https://linear.app/neon-sprawl/issue/NEO-49); label **`E2.M3`**. **NEO-45 landed:** prototype **`salvage`** mastery catalog + schema + CI (`content/mastery/`, `scripts/validate_content.py`); register row **In Progress**. Flagship track; dependency order and story text in [E2_M3_MasteryAndPerkUnlocks.md](E2_M3_MasteryAndPerkUnlocks.md) and [E2M3-pre-production-backlog.md](../../plans/E2M3-pre-production-backlog.md). **NEO-47** blocked by [NEO-46](https://linear.app/neon-sprawl/issue/NEO-46) and [NEO-38](https://linear.app/neon-sprawl/issue/NEO-38) (skill level source). +**E2.M3 note:** Epic 2 Slice 4 — [NEO-45](https://linear.app/neon-sprawl/issue/NEO-45) → [NEO-49](https://linear.app/neon-sprawl/issue/NEO-49); label **`E2.M3`**. **NEO-45 landed:** prototype **`salvage`** mastery catalog + schema + CI. **NEO-46 landed:** fail-fast server mastery catalog load. **NEO-47 landed:** `PerkUnlockEngine`, `IPlayerPerkStateStore` + **`V004`**, level-up hook in skill XP grants. **NEO-49 landed:** comment-only **`perk_unlock`** telemetry hook in `PerkUnlockEngine.TryUnlockPerks`. **Still planned:** perk HTTP + Bruno ([NEO-48](https://linear.app/neon-sprawl/issue/NEO-48)). Flagship track; see [E2_M3_MasteryAndPerkUnlocks.md](E2_M3_MasteryAndPerkUnlocks.md), [E2M3-pre-production-backlog.md](../../plans/E2M3-pre-production-backlog.md), and [documentation_and_implementation_alignment.md](documentation_and_implementation_alignment.md) E2.M3 row. ### Epic 3 — Crafting Economy diff --git a/docs/plans/NEO-49-implementation-plan.md b/docs/plans/NEO-49-implementation-plan.md index 51cb019..971ded9 100644 --- a/docs/plans/NEO-49-implementation-plan.md +++ b/docs/plans/NEO-49-implementation-plan.md @@ -86,6 +86,12 @@ **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. +## Decisions (post-review) + +| Topic | Decision | +|--------|----------| +| **E9.M1 `source` field** | Path-auto unlocks triggered inside **`TrySelectBranch`** (via `ApplyPathAutoTiersAtOrBelowLevel` at current level) currently use **`PerkUnlockSource.LevelUp`**, not `BranchPick`. When wiring E9.M1 ingest, product/catalog owners should confirm whether retroactive path-auto at pick time stays **`LevelUp`** or gains a distinct catalog value; hook comments list `BranchPick \| LevelUp` as implemented today. | + ## 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`. +**None** for NEO-49 scope. 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`. diff --git a/docs/reviews/2026-05-17-NEO-49.md b/docs/reviews/2026-05-17-NEO-49.md index d897b8a..3763a9b 100644 --- a/docs/reviews/2026-05-17-NEO-49.md +++ b/docs/reviews/2026-05-17-NEO-49.md @@ -2,7 +2,8 @@ **Date:** 2026-05-17 **Scope:** Branch `NEO-49-perk-unlock-telemetry-hook-sites` · commits `f3e48f4`–`38ef54c` vs `main` (includes unrelated `0d63965` chore: `.vscodecsdt/` gitignore) -**Base:** `main` +**Base:** `main` +**Follow-up:** Suggestions and actionable nits below are **done** (strikethrough + **Done.**). ## Verdict @@ -22,7 +23,7 @@ NEO-49 adds a **comments-only** E9.M1 **`perk_unlock`** hook anchor in `PerkUnlo | [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) | **Matches** — E2.M3 row cites NEO-49 hook + manual QA. | | [`docs/decomposition/epics/epic_02_skills_and_progression.md`](../decomposition/epics/epic_02_skills_and_progression.md) | **Matches** — Slice 4 lists `perk_unlock` telemetry hook; implementation is comment-only until E9.M1. | | [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **N/A** — no client or authority boundary change. | -| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Partially matches** — E2.M3 note still says **NEO-47 blocked by NEO-46** (stale vs `main`); alignment doc is current. Optional register refresh (see Suggestions). | +| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Matches** — E2.M3 note refreshed (NEO-45 → NEO-49 landed; NEO-48 still planned). | Register/tracking: alignment table and E2.M3 module updated; register footnote not touched in this branch. @@ -32,12 +33,12 @@ None. ## Suggestions -1. **Refresh `module_dependency_register` E2.M3 note** — The footnote still ends with “**NEO-47** blocked by NEO-46…” while NEO-46/47/49 are landed on this branch’s doc trail. Mirror the E2.M3 row in `documentation_and_implementation_alignment.md` (NEO-46 → NEO-47 → NEO-49) so the register does not contradict merge-ready `main` + this PR. -2. **E9.M1 `source` semantics** — NEO-47 re-review noted path-auto unlocks triggered from `TrySelectBranch` may carry `PerkUnlockSource.LevelUp`. The hook comment already lists `BranchPick | LevelUp`; when wiring ingest, document whether retroactive path-auto from branch pick should remain `LevelUp` or gain a distinct catalog value (product/E9.M1 decision, not blocking NEO-49). +1. ~~**Refresh `module_dependency_register` E2.M3 note** — The footnote still ends with “**NEO-47** blocked by NEO-46…” while NEO-46/47/49 are landed on this branch’s doc trail. Mirror the E2.M3 row in `documentation_and_implementation_alignment.md` (NEO-46 → NEO-47 → NEO-49) so the register does not contradict merge-ready `main` + this PR.~~ **Done.** `module_dependency_register.md` E2.M3 note updated. +2. ~~**E9.M1 `source` semantics** — NEO-47 re-review noted path-auto unlocks triggered from `TrySelectBranch` may carry `PerkUnlockSource.LevelUp`. The hook comment already lists `BranchPick | LevelUp`; when wiring ingest, document whether retroactive path-auto from branch pick should remain `LevelUp` or gain a distinct catalog value (product/E9.M1 decision, not blocking NEO-49).~~ **Done.** Documented in [NEO-49 plan](../plans/NEO-49-implementation-plan.md) **Decisions (post-review)**. ## Nits -- Nit: `PerkUnlockEvent.cs` XML summary sits on **`PerkUnlockSource`** enum, not the **`PerkUnlockEvent`** record—manual QA says “Open `PerkUnlockEvent.cs`”; moving the summary to the record (or duplicating a one-liner on the record) would match reader intent. +- ~~Nit: `PerkUnlockEvent.cs` XML summary sits on **`PerkUnlockSource`** enum, not the **`PerkUnlockEvent`** record—manual QA says “Open `PerkUnlockEvent.cs`”; moving the summary to the record (or duplicating a one-liner on the record) would match reader intent.~~ **Done.** Hook-site summary on `PerkUnlockEvent` record; enum has its own one-line summary. - Nit: Branch includes chore commit `0d63965` (`.vscodecsdt/` gitignore)—harmless; already isolated if reviewers prefer a cherry-pick-only view of NEO-49 commits. ## Verification diff --git a/server/NeonSprawl.Server/Game/Mastery/PerkUnlockEvent.cs b/server/NeonSprawl.Server/Game/Mastery/PerkUnlockEvent.cs index 3d28bda..4b4d44b 100644 --- a/server/NeonSprawl.Server/Game/Mastery/PerkUnlockEvent.cs +++ b/server/NeonSprawl.Server/Game/Mastery/PerkUnlockEvent.cs @@ -1,12 +1,13 @@ namespace NeonSprawl.Server.Game.Mastery; -/// Internal event when a perk becomes unlocked (NEO-47). E9.M1 perk_unlock hook site: TryUnlockPerks (NEO-49). +/// How the unlock was triggered (branch-pick tier perks vs path-auto after level-up). public enum PerkUnlockSource { BranchPick, LevelUp, } +/// Internal event when a perk becomes unlocked (NEO-47). E9.M1 perk_unlock hook site: TryUnlockPerks (NEO-49). /// Chosen branch when relevant; null for edge cases without a branch context. public sealed record PerkUnlockEvent( string PlayerId,