# Code review — NEO-71 craft telemetry hook sites

**Date:** 2026-05-24  
**Scope:** Branch `NEO-71-item-crafted-craft-failed-telemetry-hooks` · commits `194078d`–`2d47510` vs `origin/main`  
**Base:** `origin/main`

## Verdict

**Approve with nits** — comment-only hook sites and documentation match the adopted plan and E3M2-07 acceptance criteria; ready to merge.

## Summary

The branch adds **comment-only E9.M1 telemetry hook sites** in **`CraftOperations.TryCraft`**: **`item_crafted`** immediately before the successful return (after refine XP grant), and **`craft_failed`** in the centralized private **`Deny`** helper covering all **`CraftReasonCodes`** paths including store-missing and post-mutation rollback denies. Hook style, payload field notes, and **`TODO(E9.M1): catalog emit`** markers follow NEO-64 / NEO-56 precedent. **`PlayerCraftApi`** correctly has no duplicate hooks. Docs (implementation plan, **`server/README.md`** craft telemetry subsection, E3_M2 module snapshot, E3M2-07 backlog, alignment register, dependency register **Ready** bump) are updated. No runtime or wire contract change; existing craft tests pass. Risk is negligible.

## Documentation checked

| Document | Result |
|----------|--------|
| [`docs/plans/NEO-71-implementation-plan.md`](../plans/NEO-71-implementation-plan.md) | **Matches** — engine-only hooks, central **`Deny`**, comments-only, no manual QA doc, **`time-to-first-craft`** derived at E9.M1, acceptance checklist complete. |
| [`docs/plans/E3M2-prototype-backlog.md`](../plans/E3M2-prototype-backlog.md) · **E3M2-07** | **Matches** — acceptance criteria checked; landed note present. |
| [`docs/decomposition/modules/E3_M2_RefinementAndRecipeExecution.md`](../decomposition/modules/E3_M2_RefinementAndRecipeExecution.md) | **Matches** — NEO-71 landed line; telemetry deferral removed; module **Ready** (Slice 3 server backlog complete). |
| [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) | **Matches** — E3.M2 row notes NEO-71; E3M2-01–07 complete. |
| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Matches** — E3.M2 **Status** → **Ready**; E3M2-07 note updated. |
| [`docs/decomposition/epics/epic_03_crafting_economy.md`](../decomposition/epics/epic_03_crafting_economy.md) · Slice 3 | **Matches** — **`item_crafted`**, **`craft_failed`** with reason documented; **`time-to-first-craft`** noted as derived (no separate hook). |
| NEO-64 / NEO-56 reference — [`GatherOperations`](../../server/NeonSprawl.Server/Game/Gathering/GatherOperations.cs), [`PlayerInventoryOperations`](../../server/NeonSprawl.Server/Game/Items/PlayerInventoryOperations.cs) | **Matches** — same comment block structure, **`TODO(E9.M1)`**, central **`Deny`** for structured denies, engine-only anchor. |
| Full-stack epic decomposition | **Matches** — no client counterpart (server comment-only); NEO-74 cited as Slice 5 craft UI; no claim of player-visible prototype completion. |

Register/tracking: E3.M2 **Ready** bump is appropriate — Slice 3 server backlog E3M2-01–E3M2-07 complete per plan.

## Blocking issues

None.

## Suggestions

1. ~~**`CraftResult.cs` summary tense** — Plan item 4 asked to confirm cross-reference wording after hooks land. File was not updated; summary still reads “NEO-71 telemetry hook sites **will live** in **`TryCraft`**”. Update to present tense (e.g. “live in **`CraftOperations.TryCraft`** (success path + **`Deny`**)”) so it matches landed hooks and the class summary on **`CraftOperations`**.~~ **Done.**

## Nits

- ~~Nit: README craft telemetry subsection is a single dense paragraph; a two-bullet list mirroring the code hook sites (`item_crafted` / `craft_failed`) would match NEO-64 gather README scanability — optional only.~~ **Done.**

## Verification

```bash
cd server
dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj --filter "FullyQualifiedName~PlayerCraftApiTests|FullyQualifiedName~CraftOperationsTests"
```

**Results (2026-05-24):** **21** craft-related tests pass (`PlayerCraftApiTests` + `CraftOperationsTests`).

Optional Bruno regression (behavior unchanged): run `bruno/neon-sprawl-server/craft/Post craft gather refine make spine.bru` against a running dev server.
