From 72b98a627cbf1ed00c93043c74ccc0e3d04dbd7c Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 7 Jun 2026 12:28:11 -0400 Subject: [PATCH] NEO-121: Add kickoff implementation plan for quest telemetry hooks. --- docs/plans/NEO-121-implementation-plan.md | 116 ++++++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 docs/plans/NEO-121-implementation-plan.md diff --git a/docs/plans/NEO-121-implementation-plan.md b/docs/plans/NEO-121-implementation-plan.md new file mode 100644 index 0000000..b0d9908 --- /dev/null +++ b/docs/plans/NEO-121-implementation-plan.md @@ -0,0 +1,116 @@ +# NEO-121 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-121 | +| **Title** | E7M1-10: Quest telemetry hook sites (comment-only) | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-121/e7m1-10-quest-telemetry-hook-sites-comment-only | +| **Module** | [E7.M1 — QuestStateMachine](../decomposition/modules/E7_M1_QuestStateMachine.md) · Epic 7 Slice 1 · backlog **E7M1-10** | +| **Branch** | `NEO-121-quest-telemetry-hooks` | +| **Precursor** | [NEO-117](https://linear.app/neon-sprawl/issue/NEO-117) — `QuestStateOperations` (**landed on `main`**); [NEO-118](https://linear.app/neon-sprawl/issue/NEO-118) — `QuestObjectiveWiring` (**landed on `main`**); [NEO-119](https://linear.app/neon-sprawl/issue/NEO-119) / [NEO-120](https://linear.app/neon-sprawl/issue/NEO-120) — quest progress GET + accept POST (**landed on `main`**) | +| **Pattern** | [NEO-64](NEO-64-implementation-plan.md) / [NEO-56](NEO-56-implementation-plan.md) / [NEO-49](NEO-49-implementation-plan.md) — comment-only engine anchors + `TODO(E9.M1)`; engine-only (no duplicate API-layer hooks) | +| **Client counterpart** | none (infrastructure-only per [E7M1-10](E7M1-prototype-backlog.md#e7m1-10--quest-telemetry-hook-sites-comment-only)) | + +## Kickoff clarifications + +**No clarifications needed.** [E7M1-10](E7M1-prototype-backlog.md#e7m1-10--quest-telemetry-hook-sites-comment-only) backlog, Linear AC, landed quest operations/wiring (NEO-117/118), and comment-only telemetry precedents (NEO-49/56/64/71) settle scope: + +- **Runtime behavior:** comments-only + `TODO(E9.M1)` — no production ingest, no `ILogger` (NEO-49/56 precedent). +- **Hook anchor:** **`QuestStateOperations`** only for **`quest_start`**, **`quest_step_complete`**, **`quest_complete`** — HTTP accept POST and objective wiring delegate to the engine; no duplicate API-layer or wiring-layer hooks (NEO-56 engine-only precedent). **`QuestObjectiveWiring`** triggers step advance/complete by calling **`QuestStateOperations`**; hooks live at mutation commit sites. +- **`quest_start` timing:** after successful **`TryActivate`** in **`TryAccept`** (first activation only; deny paths excluded). +- **`quest_step_complete` timing:** after successful **`TryAdvanceStep`** store commit (covers wiring-driven multi-step chains — one emit per committed advance). +- **`quest_complete` timing:** after first-time **`TryMarkComplete`** store commit; **not** on idempotent replay when row is already **`completed`** (NEO-117 idempotency + NEO-49 “idempotent reevaluation produces none” precedent). +- **Optional deny hook:** include **`quest_accept_denied`** comment in **`TryAccept`** **`Deny`** returns with stable **`reasonCode`** (NEO-56 **`inventory_transfer_denied`** pattern; backlog lists as optional — adopt for accept funnel parity). +- **Manual QA doc:** **`docs/manual-qa/NEO-121.md`** — NEO-64/49 comment-only verification pattern (no behavior/API change). + +## Goal, scope, and out-of-scope + +**Goal:** Document and place **comment-only hook sites** on the server-authoritative quest state path for future E9.M1 catalog events **`quest_start`**, **`quest_step_complete`**, and **`quest_complete`**. Align with Epic 7 Slice 1 telemetry vocabulary ([E7_M1 module](../decomposition/modules/E7_M1_QuestStateMachine.md#risks-and-telemetry)). + +**In scope (from Linear + [E7M1-10](E7M1-prototype-backlog.md#e7m1-10--quest-telemetry-hook-sites-comment-only)):** + +- Hook sites in **`QuestStateOperations`** at accept, step advance, and complete commit paths. +- Reserved deny hook **`quest_accept_denied`** (comment-only) on **`TryAccept`** deny paths. +- **`server/README.md`** quest telemetry subsection. +- **`E7_M1_QuestStateMachine.md`** implementation snapshot note. +- **`docs/manual-qa/NEO-121.md`** — verify comments + no behavior change. +- E7M1 backlog + alignment register updates when implementation completes. + +**Out of scope (from Linear + backlog):** + +- E9.M1 ingest pipeline; runtime telemetry emit. +- `ILogger` / metrics / dev-only log lines. +- Duplicate hook comments in **`QuestAcceptApi`**, **`QuestProgressApi`**, or **`QuestObjectiveWiring`** (engine-only precedent). +- Client HUD ([NEO-122](https://linear.app/neon-sprawl/issue/NEO-122)). + +## Acceptance criteria checklist + +- [ ] Hook sites exist at accept, step complete, quest complete paths. +- [ ] Epic 7 Slice 1 telemetry vocabulary documented in `server/README.md`. + +## Technical approach + +1. **Authoritative surface:** **`QuestStateOperations`** — all quest lifecycle mutations funnel here. **`QuestObjectiveWiring`** calls **`TryAdvanceStep`** / **`TryMarkComplete`**; **`QuestAcceptApi`** calls **`TryAccept`**. + +2. **Hook site A — `quest_start`:** In **`TryAccept`**, immediately after **`progressStore.TryActivate`** returns **`true`** (before objective wiring refresh): + - Comment block names future E9.M1 event **`quest_start`**. + - **`TODO(E9.M1): catalog emit`** (NEO-49 style). + - Planned payload fields: `playerId`, `questId`, `currentStepIndex` (0), optional `schemaVersion`. + +3. **Hook site B — `quest_step_complete`:** In **`TryAdvanceStep`**, immediately after **`progressStore.TryAdvanceStep`** returns **`true`**: + - Comment block names future E9.M1 event **`quest_step_complete`**. + - **`TODO(E9.M1): catalog emit`** — emit once per committed step advance (chain quests may emit multiple in one wiring pass). + - Planned payload fields: `playerId`, `questId`, `completedStepIndex` (prior index), `newStepIndex`, optional step id from catalog at E9.M1 wiring time. + +4. **Hook site C — `quest_complete`:** In **`TryMarkComplete`**, immediately after **`progressStore.TryMarkComplete`** returns **`true`** on the first-time completion path (after the early idempotent **`completed`** return): + - Comment block names future E9.M1 event **`quest_complete`**. + - **`TODO(E9.M1): catalog emit`** — not on idempotent replay. + - Planned payload fields: `playerId`, `questId`, `completedAt`, optional total step count. + +5. **Hook site D (optional) — `quest_accept_denied`:** On **`TryAccept`** deny returns (central **`Deny`** helper and/or each deny branch before return): + - Comment block names future E9.M1 event **`quest_accept_denied`**. + - **`TODO(E9.M1): catalog emit`** with stable **`reasonCode`** (`unknown_quest`, `prerequisite_incomplete`, `already_active`, `already_completed`, etc.). + - Planned payload fields: `playerId`, `questId`, `reasonCode`. + +6. **Cross-references (minimal):** + - Update **`QuestStateOperations`** class summary → NEO-121 telemetry hook anchor. + - **`server/README.md`:** new **Quest telemetry hooks (NEO-121)** subsection after quest accept POST (NEO-120). + - **`E7_M1_QuestStateMachine.md`:** NEO-121 landed line under Risks and telemetry. + - **`documentation_and_implementation_alignment.md`** + **`module_dependency_register.md`:** E7.M1 row — E7M1-10 when complete. + - **`E7M1-prototype-backlog.md`:** E7M1-10 checkboxes + landed note. + +7. **Accept + instant wiring:** **`TryAccept`** may trigger **`quest_start`**, then wiring may emit **`quest_step_complete`** / **`quest_complete`** in the same call stack — order is preserved by code flow (activate → wiring → advance/complete). + +## Files to add + +| Path | Purpose | +|------|---------| +| `docs/manual-qa/NEO-121.md` | Manual QA: verify comment blocks + no behavior/API change. | +| `docs/plans/NEO-121-implementation-plan.md` | This plan. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs` | **NEO-121:** comment-only **`quest_start`**, **`quest_step_complete`**, **`quest_complete`**, **`quest_accept_denied`** hook sites at mutation/deny commit paths. | +| `server/README.md` | Document quest telemetry hook placement and Epic 7 Slice 1 vocabulary (NEO-121). | +| `docs/decomposition/modules/E7_M1_QuestStateMachine.md` | Implementation snapshot: NEO-121 telemetry hooks landed. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E7.M1 row: NEO-121 hook sites. | +| `docs/decomposition/modules/module_dependency_register.md` | E7.M1 note — E7M1-10 complete when story lands. | +| `docs/plans/E7M1-prototype-backlog.md` | E7M1-10 checkboxes + landed note. | + +## Tests + +| Test file | What it covers | +|-----------|----------------| +| *(none added or changed)* | Comments-only change; no behavior or wire contract change. Regression: `dotnet test NeonSprawl.sln` (existing **`QuestStateOperationsTests`**, **`QuestObjectiveWiringTests`**, quest HTTP integration tests unchanged). Manual verification: **`docs/manual-qa/NEO-121.md`**. | + +## Open questions / risks + +| Question / risk | Agent recommendation | Status | +|-----------------|----------------------|--------| +| **Duplicate emits on accept + wiring same tick** | Expected — **`quest_start`** then zero or more **`quest_step_complete`** / **`quest_complete`**; E9.M1 can correlate by session/time or sample funnels. | **adopted** | +| **`quest_accept_denied` scope** | Include on all **`TryAccept`** deny paths with **`reasonCode`** (NEO-56 deny pattern). | **adopted** | +| **Module Ready vs In Progress** | Leave E7.M1 **In Progress** until client capstones NEO-122/123 land; only note E7M1-10 complete in register. | **adopted** |