NEO-71: add implementation plan for craft telemetry hook sites

pull/105/head
VinPropane 2026-05-24 18:22:10 -04:00
parent 1be4a7eed8
commit 194078dd3b
1 changed files with 115 additions and 0 deletions

View File

@ -0,0 +1,115 @@
# NEO-71 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-71 |
| **Title** | E3.M2: item_crafted / craft_failed telemetry hook sites |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-71/e3m2-item-crafted-craft-failed-telemetry-hook-sites |
| **Module** | [E3.M2 — RefinementAndRecipeExecution](../decomposition/modules/E3_M2_RefinementAndRecipeExecution.md) · Epic 3 Slice 3 · backlog **E3M2-07** |
| **Branch** | `NEO-71-item-crafted-craft-failed-telemetry-hooks` |
| **Blocked by** | [NEO-69](https://linear.app/neon-sprawl/issue/NEO-69) — `CraftOperations.TryCraft` + `CraftResult` (**Done** on `main`); [NEO-70](https://linear.app/neon-sprawl/issue/NEO-70) — craft HTTP + Bruno spine (**Done** on `main`; may parallel). |
| **Pattern** | [NEO-64](https://linear.app/neon-sprawl/issue/NEO-64) — gather telemetry hooks in engine; [NEO-56](https://linear.app/neon-sprawl/issue/NEO-56) — centralized **`Deny`** hook for structured denies |
| **Client counterpart** | None — comment-only server engine story; craft UI is [NEO-74](https://linear.app/neon-sprawl/issue/NEO-74) (Slice 5). |
## Kickoff clarifications
| Topic | Question | Agent recommendation | Answer |
|--------|----------|----------------------|--------|
| **Manual QA doc** | Add `docs/manual-qa/NEO-71.md`? User asked whether anything is testable in the UI. | **No** — no Godot/UI surface in this story (comment-only server hooks; craft UI is NEO-74). Verify via code review + `dotnet test` + optional Bruno craft regression (NEO-70 waiver for E3.M2 server-only). | **User:** No UI to test; **no manual QA doc** (NEO-70 pattern). |
| **Runtime behavior** | Comments-only vs dev `ILogger`? | **Comments-only**`TODO(E9.M1)`; no production logging (NEO-49/56/64 precedent). | **Adopted** (repo precedent). |
| **Hook anchor** | Engine only vs engine + `PlayerCraftApi`? | **Engine only** in **`CraftOperations.TryCraft`** — HTTP delegates to engine; no duplicate API-layer hooks (NEO-56/64 precedent; Linear out-of-scope). | **Adopted** (repo precedent). |
| **`craft_failed` placement** | Central **`Deny`** vs inline at each early return? | **Central `Deny`** — one hook site covers all **`CraftReasonCodes`** (NEO-56 **`PlayerInventoryOperations.Deny`** pattern). | **Adopted** (repo precedent). |
| **`craft_failed` scope** | All structured denies vs pre-commit rule denies only? | **All** **`Deny`** outcomes including store-missing and post-mutation rollback paths — stable **`reasonCode`** on every craft failure (NEO-56 all-deny precedent). | **Adopted** (repo precedent). |
| **`time-to-first-craft`** | Separate hook in prototype? | **No separate hook** — derived metric from **`item_crafted`** + session context at E9.M1 (E3M2-07 backlog note). | **Adopted** (backlog default). |
## Goal, scope, and out-of-scope
**Goal:** Document and place **comment-only hook sites** on the server-authoritative craft path for future E9.M1 catalog events **`item_crafted`** and **`craft_failed`**. Align with Epic 3 Slice 3 telemetry vocabulary ([epic_03 — Slice 3](../decomposition/epics/epic_03_crafting_economy.md#epic-3-slice-3)).
**In scope (from Linear + [E3M2-07](E3M2-prototype-backlog.md#e3m2-07--item_crafted--craft_failed-telemetry-hook-sites)):**
- Hook placement in **`CraftOperations.TryCraft`** (success + structured deny via **`Deny`**).
- **`TODO(E9.M1)`** comments; no production logging.
- **`server/README.md`** craft telemetry subsection; [E3_M2 module](../decomposition/modules/E3_M2_RefinementAndRecipeExecution.md) hook note.
- E3M2 backlog + alignment register updates when implementation completes.
**Out of scope (from Linear):**
- Telemetry ingest, dashboards (**E9.M1**).
- `ILogger` / metrics / dev-only log lines.
- Duplicate hook comments in **`PlayerCraftApi`** (engine-only precedent).
- Godot craft UI — **[NEO-74](https://linear.app/neon-sprawl/issue/NEO-74)**.
- **`docs/manual-qa/NEO-71.md`** (kickoff decision — no player-visible or UI verification).
## Acceptance criteria checklist
- [ ] Hook sites documented in code and `server/README.md`.
- [ ] Matches Epic 3 Slice 3 telemetry vocabulary (`item_crafted`, `craft_failed` with reason; **`time-to-first-craft`** noted as derived, no separate hook).
## Technical approach
1. **Authoritative surface:** **`CraftOperations.TryCraft`** — all craft side effects (recipe resolve → inventory pre-flight → input removal → output grant → refine XP) funnel here; **`PlayerCraftApi`** delegates without duplicating hooks.
2. **Hook site A — `item_crafted`:** Immediately before the successful **`CraftResult`** return (after refine XP grant succeeds):
- Comment block names future E9.M1 event **`item_crafted`**.
- **`TODO(E9.M1): catalog emit`** (NEO-64 / NEO-56 style).
- Planned payload fields: `playerId`, `recipeId`, `quantity` (batch), `inputsConsumed`, `outputsGranted`, `xpGrantSummary` (`skillId`, `amount`, `sourceKind`).
3. **Hook site B — `craft_failed`:** In private **`Deny(string reasonCode)`** (centralized, NEO-56 pattern):
- Comment block names future E9.M1 event **`craft_failed`**.
- **`TODO(E9.M1): catalog emit`** — on every structured craft deny with stable **`reasonCode`** from **`CraftReasonCodes`**.
- Planned payload fields: `playerId`, `recipeId`, `quantity`, `reasonCode`; note **`playerId` / `recipeId` / `quantity`** are in enclosing **`TryCraft`** scope (same pattern as NEO-56 **`Deny`** noting fields at call sites).
4. **Cross-references (minimal):**
- Update **`CraftOperations`** class summary → NEO-71 telemetry hook anchor.
- **`CraftResult.cs`** summary already points to **`TryCraft`** — confirm wording after hooks land.
- **`server/README.md`:** new **NEO-71 craft telemetry hooks** subsection under craft engine (replace “Telemetry hook sites are **NEO-71**” deferral with landed doc).
- **`E3_M2_RefinementAndRecipeExecution.md`:** NEO-71 landed line; remove “Telemetry hooks deferred to NEO-71”.
- **`documentation_and_implementation_alignment.md`** + **`module_dependency_register.md`:** E3.M2 row — E3M2-07 complete; module **Ready** when all Slice 3 backlog lands.
- **`E3M2-prototype-backlog.md`:** E3M2-07 checkboxes + landed note.
5. **NEO-56 interaction:** Successful craft output grants already trigger **`item_created`** inside **`PlayerInventoryOperations.TryAddStack`**; **`item_crafted`** is the craft-funnel catalog event at the orchestration layer (analogous to **`resource_gathered`** + **`item_created`** on gather).
6. **`time-to-first-craft`:** Document in README that E9.M1 derives this from first **`item_crafted`** per player/session — no prototype hook.
## Files to add
| Path | Purpose |
|------|---------|
| `docs/plans/NEO-71-implementation-plan.md` | This plan. |
## Files to modify
| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Game/Crafting/CraftOperations.cs` | **NEO-71:** comment-only **`item_crafted`** hook on success path + **`craft_failed`** hook in **`Deny`**. |
| `server/NeonSprawl.Server/Game/Crafting/CraftResult.cs` | Confirm type summary cross-references NEO-71 hook anchor (wording tweak if needed). |
| `server/README.md` | Document craft telemetry hook placement (NEO-71); remove deferral note. |
| `docs/decomposition/modules/E3_M2_RefinementAndRecipeExecution.md` | Implementation snapshot: NEO-71 telemetry hooks landed; Slice 3 telemetry vocabulary. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E3.M2 row: NEO-71 hook sites. |
| `docs/decomposition/modules/module_dependency_register.md` | E3.M2 note — E3M2-07 complete; module status **Ready**. |
| `docs/plans/E3M2-prototype-backlog.md` | E3M2-07 checkboxes + landed note; Slice 3 server spine complete. |
## 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 **`CraftOperationsTests`** + **`PlayerCraftApiTests`** unchanged). Optional Bruno: `bruno/neon-sprawl-server/craft/` spine against running dev server (NEO-70; behavior unchanged). |
## Open questions / risks
| Question / risk | Agent recommendation | Status |
|-----------------|----------------------|--------|
| **Overlap with NEO-56 `item_created`** | Keep both — **`item_created`** at inventory layer; **`item_crafted`** at craft orchestration (E9.M1 can correlate). | **adopted** |
| **No UI / manual QA doc** | User confirmed no Godot verification; README + code review + existing test suite sufficient (NEO-70 E3.M2 server-only pattern). | **adopted** (kickoff) |
| **Slice 3 module closure** | Landing NEO-71 completes E3.M2 prototype **server** backlog; client loop remains NEO-72NEO-75 (Slice 5). | **adopted** |
## Decisions (kickoff)
- **No `docs/manual-qa/NEO-71.md`** — no UI surface; server comment-only (user confirmed via UI question).
- **Engine-only hooks** in **`CraftOperations`**; no **`PlayerCraftApi`** duplicates.
- **Central `Deny` for `craft_failed`** — all **`CraftReasonCodes`** paths.
- **Comments + `TODO(E9.M1)` only** — no ingest or logging.
- **`time-to-first-craft`** — derived at E9.M1; no separate prototype hook.