NEO-152: Add code review for contract telemetry hook sites.

docs/reviews/2026-06-28-NEO-152.md

@@ -0,0 +1,56 @@
+# Code review — NEO-152 contract telemetry hook sites (comment-only)
+
+**Date:** 2026-06-28  
+**Scope:** Branch `NEO-152-e7m4-09-contract-telemetry-hook-sites-comment-only` — commits `6e5ea78`..`0d089a6` (2 commits)  
+**Base:** `main`  
+**Issue:** [NEO-152](https://linear.app/neon-sprawl/issue/NEO-152) — E7M4-09 contract telemetry hook sites (comment-only)
+
+## Verdict
+
+**Approve with nits**
+
+## Summary
+
+This branch anchors future E9.M1 catalog events on the server-authoritative contract path with comment-only hook sites and no runtime behavior change. **`contract_issued`** and economy-deny **`reward_anomaly`** land in **`ContractGeneratorOperations.TryIssue`**; **`contract_complete`** extends the NEO-149 partial block to full NEO-121 style; shared-router **`reward_anomaly`** documents partial-apply rollback on **`CompensatingRevertAll`**, and the NEO-130 **`reward_delivery`** comment is updated for contract **`SourceKind`**.
+
+Hook placement follows NEO-121/130/141 precedents (engine-only, **`TODO(E9.M1)`**, no API-layer duplicates). Docs (implementation plan, README subsection, E7.M4 module, alignment register, E7M4 backlog) are updated. Full suite passes (**933** tests). Risk is negligible — comments only; no wire or ingest change.
+
+## Documentation checked
+
+| Document | Result |
+|----------|--------|
+| `docs/plans/NEO-152-implementation-plan.md` | **Matches** — AC checklist complete; shipped reconciliation aligns with diff; kickoff decisions (engine-only, no duplicate `reward_delivery`, economy deny at orchestrator) reflected in code |
+| `docs/plans/E7M4-pre-production-backlog.md` | **Matches** — E7M4-09 AC checked; landed note + README/plan links |
+| `docs/decomposition/modules/E7_M4_ContractMissionGenerator.md` | **Matches** — NEO-152 telemetry hooks line added after NEO-151 HTTP |
+| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Partially matches** — E7.M4 row includes E7M4-09 / NEO-152 body text; trailing plan link list still jumps NEO-151 → label range without explicit **`NEO-152` plan** link (nit) |
+| `docs/decomposition/modules/module_dependency_register.md` | **Not re-read in diff** — no register change required; E7.M4 already tracked |
+| `server/README.md` | **Matches** — **Contract telemetry hooks (NEO-152)** subsection with event table + NEO-130 cross-link |
+| Client counterpart (NEO-153) | **N/A** — correctly out of scope; plan does not claim player-visible slice complete |
+
+Register/tracking table updates are included in this branch.
+
+## Blocking issues
+
+None.
+
+## Suggestions
+
+1. **Add `NEO-152` plan link to alignment register footer** — The E7.M4 row body cites E7M4-09 / NEO-152, but the pipe-separated trailing refs list `[NEO-144]` … `[NEO-151]` then `label E7.M4 on NEO-144–NEO-154` without `[NEO-152 plan]`. Add for parity with adjacent landed stories.
+
+2. **Note method-level vs call-site rollback anchor in plan** — Plan §4 C2 and the files table say stub comments at **`CompensatingRevertAll` invocations** in **`TryDeliverBundle`**; implementation uses a **single block on `CompensatingRevertAll`** (four call sites). Functionally equivalent and DRY (similar to NEO-121 central **`Deny`**); optional one-line note in plan **Implementation reconciliation** that the adopted anchor is the private method.
+
+## Nits
+
+- Nit: **`reward_anomaly`** at economy validation deny is semantically a lint/cap deny rather than a delivery rollback; naming matches backlog/NEO-150 deferral — no change needed, but E9.M1 wiring should treat **`reasonCode`** distinctly from router rollback anomalies.
+- Nit: Plan **§1 Current state** table still describes pre-ship “no hook” state; **Implementation reconciliation** is authoritative — optional cleanup of §1 on merge to past tense.
+
+## Verification
+
+```bash
+dotnet test NeonSprawl.sln
+```
+
+Manual (optional — comment-only):
+
+- Open `ContractGeneratorOperations.cs`, `ContractCompletionOperations.cs`, and `RewardRouterOperations.cs`; confirm **`TODO(E9.M1)`** markers, planned payload fields, and no new log/ingest calls.
+- Confirm **`ContractIssueApi`** / **`ContractListApi`** have no NEO-152 hooks (engine-only).