# Code review — NEO-130 (E7M2-07)

**Date:** 2026-06-07  
**Scope:** Branch `NEO-130-reward-telemetry-hooks` vs `origin/main` — commits `7493a6b` … `c461a82`  
**Base:** `origin/main`  
**Follow-up:** Review suggestions/nits addressed in a subsequent commit on the same branch.

## Verdict

**Approve with nits**

## Summary

NEO-130 adds **comment-only** E9.M1 telemetry hook sites to **`RewardRouterOperations.TryDeliverQuestCompletion`** for Epic 7 Slice 2 events **`reward_delivery`** (after first-time **`IRewardDeliveryStore.TryRecord`** success) and **`unlock_granted`** (stub region after item + skill XP apply, before **`RewardDeliveryEvent`** construction). Hook placement correctly excludes idempotent **`TryGet`** replay, deny paths, and **`TryRecord`** race-loser rollback. No runtime ingest, **`ILogger`**, wire contract, or API changes. Documentation matches NEO-121/NEO-49 precedent: implementation plan reconciliation, **`server/README.md`** event table, E7.M2 module snapshot, alignment register, E7M2-07 backlog checkboxes, **`RewardDeliveryEvent`** xml cross-ref, and **`docs/manual-qa/NEO-130.md`**. Infrastructure-only slice with no client counterpart. Risk is negligible.

## Documentation checked

| Path | Result |
|------|--------|
| `docs/plans/NEO-130-implementation-plan.md` | **Matches** — kickoff timing decisions adopted; acceptance checklist checked; reconciliation accurate. |
| `docs/plans/E7M2-prototype-backlog.md` (E7M2-07) | **Matches** — acceptance checkbox checked; landed note + README link. |
| `docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md` | **Matches** — NEO-130 telemetry hook snapshot added under implementation anchor. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E7M2-07 / NEO-130 landed note appended; row stays **Planned** until NEO-132 capstone. |
| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — no register row change required (detail lives in alignment note). |
| `server/README.md` | **Matches** — Reward telemetry hooks subsection with event table, delegation note, and **`perk_unlock`** distinction. |
| `docs/manual-qa/NEO-130.md` | **Matches** — comment-only verification checklist; optional regression/Bruno steps. |
| Full-stack epic decomposition | **N/A** — infrastructure-only; plan and backlog explicitly document no client counterpart. |

**Register / tracking:** E7.M2 correctly remains **Planned** until client capstone NEO-132; E7M2-07 completion noted in alignment register and module snapshot only — no further status change required.

## Blocking issues

(none)

## Suggestions

1. ~~**E9.M1 `unlock_granted` wiring note** — The stub sits before **`TryRecord`**, so a future literal emit at this line would also run on the race-rollback path (grants applied then reverted). When **`UnlockGrant`** rows land, place per-row emits inside the apply loop and/or gate on durable commit (mirror **`reward_delivery`** after **`TryRecord`** success). The stub comment already says “once per applied UnlockGrant row”; consider adding one line that durable emit should not fire on rollback paths when E9.M1 implements the loop.~~ **Done.** Stub comment now gates E9.M1 durable emit to apply-loop and/or post-**`TryRecord`** paths only (not race rollback).

## Nits

- ~~Nit: **`unlock_granted`** stub runs even for empty bundles (no item/skill rows) — correct for a region marker; E9.M1 loop will simply emit nothing until unlock rows exist in content.~~ **Done.** Acknowledged; no change.

- ~~Nit: Plan technical approach lists class-summary update on **`RewardRouterOperations`**; shipped in xml summary — reconciliation bullet could mention it for parity with other stories (cosmetic doc only).~~ **Done.** Reconciliation bullet updated.

## Verification

```bash
# Router-focused subset (14 passed at review time)
dotnet test NeonSprawl.sln --filter "FullyQualifiedName~RewardRouterOperations"

# Full regression (comments-only; no new tests)
dotnet test NeonSprawl.sln
```

Manual (per `docs/manual-qa/NEO-130.md`):

- Walk **`RewardRouterOperations.cs`** hook comment blocks; confirm **`TODO(E9.M1)`**, no **`ILogger`**, and no duplicate hooks in **`QuestStateOperations`**, **`QuestAcceptApi`**, or **`QuestProgressApi`**.
- Optional Bruno sanity in `bruno/neon-sprawl-server/quest-progress/` — gather intro complete + health after reward wiring unchanged.