NEO-130: Address review feedback on unlock_granted stub.
Document E9.M1 durable-emit gating for rollback paths; reconcile plan bullet for class summary; strike through resolved review items.pull/169/head
parent
471d4b61a7
commit
1333da2938
|
|
@ -50,7 +50,7 @@
|
|||
|
||||
## Implementation reconciliation (shipped)
|
||||
|
||||
- **Engine anchor:** **`RewardRouterOperations.TryDeliverQuestCompletion`** — two comment-only hook sites with **`TODO(E9.M1)`** (no runtime emit).
|
||||
- **Engine anchor:** **`RewardRouterOperations.TryDeliverQuestCompletion`** — two comment-only hook sites with **`TODO(E9.M1)`** (no runtime emit); class xml summary cites NEO-130 telemetry anchor.
|
||||
- **`reward_delivery`:** after **`deliveryStore.TryRecord`** returns **`true`** on first-time delivery.
|
||||
- **`unlock_granted`:** stub after item + skill XP apply loops, before delivery event build — future **`UnlockGrant`** apply region; prototype bundles have no unlock rows.
|
||||
- **Docs:** `server/README.md` reward telemetry subsection; E7.M2 module + alignment register + E7M2 backlog updated.
|
||||
|
|
|
|||
|
|
@ -2,7 +2,8 @@
|
|||
|
||||
**Date:** 2026-06-07
|
||||
**Scope:** Branch `NEO-130-reward-telemetry-hooks` vs `origin/main` — commits `7493a6b` … `c461a82`
|
||||
**Base:** `origin/main`
|
||||
**Base:** `origin/main`
|
||||
**Follow-up:** Review suggestions/nits addressed in a subsequent commit on the same branch.
|
||||
|
||||
## Verdict
|
||||
|
||||
|
|
@ -33,13 +34,13 @@ NEO-130 adds **comment-only** E9.M1 telemetry hook sites to **`RewardRouterOpera
|
|||
|
||||
## 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.
|
||||
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.
|
||||
- ~~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).
|
||||
- ~~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
|
||||
|
||||
|
|
|
|||
|
|
@ -140,6 +140,8 @@ public static class RewardRouterOperations
|
|||
// Prototype completionRewardBundle has item + skill XP rows only; no runtime unlock apply in Slice 2.
|
||||
// Planned payload fields: playerId, questId, unlock id / kind — finalize at E9.M1 wiring time.
|
||||
// Distinct from `perk_unlock` (PerkUnlockEngine, NEO-49) which may fire as a side effect of skill XP grants.
|
||||
// E9.M1 wiring: place per-row emits inside the apply loop and/or after TryRecord success only — not on
|
||||
// TryRecord race-rollback paths (grants applied then reverted below); mirror reward_delivery gating.
|
||||
|
||||
var deliveredAt = timeProvider.GetUtcNow();
|
||||
var deliveryEvent = new RewardDeliveryEvent(
|
||||
|
|
|
|||
Loading…
Reference in New Issue