# Code review — NEO-136 (E7M3-04)

**Date:** 2026-06-15  
**Scope:** Branch `NEO-136-e7m3-reputation-operations-apply-delta` — commits `e9abc8b` … `536f635` vs `main`  
**Follow-up:** Review findings addressed in follow-up commit (applied-delta rollback, ops id normalization, AAA nit).

**Base:** `main`

## Verdict

**Approve with nits**

## Summary

This branch lands **E7M3-04**: server-internal **`ReputationOperations.TryApplyDelta`** orchestrates standing mutation via **`IFactionStandingStore`**, append-only audit via **`IReputationDeltaStore`**, and compensating standing rollback when audit append fails. Supporting types **`ReputationApplyOutcome`** and **`ReputationApplyReasonCodes`** cover ops-level pre-flight denies and store passthrough. Ten AAA unit tests plus host DI smoke exercise happy path, clamp, unknown faction, invalid delta/id/source, audit-append rollback, non-writable player, and factory resolution. **`780`** server tests pass locally. Risk is low — infrastructure-only, no HTTP or Godot wiring (NEO-138+). One non-blocking correctness gap: compensating rollback uses `-deltaAmount` rather than the **actual applied delta** when clamping occurred, which can leave wrong standing on rare audit-append failure after a clamped apply.

## Documentation checked

| Path | Result |
|------|--------|
| `docs/plans/NEO-136-implementation-plan.md` | **Matches** — shipped ops, types, tests, README, module/alignment updates reconcile with plan; AC checklist marked complete. Plan flow step 1 cites explicit `FactionStandingIds` normalization at ops layer; implementation delegates player/faction normalization to stores (see Suggestions). |
| `docs/plans/E7M3-pre-production-backlog.md` | **Matches** — E7M3-04 scope (auditable apply orchestration, no client counterpart, no HTTP) aligns with implementation. |
| `docs/decomposition/modules/E7_M3_FactionReputationLedger.md` | **Matches** — Status line notes E7M3-04 `ReputationOperations` landed (NEO-136). |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E7.M3 row updated for E7M3-04 / NEO-136. |
| `docs/decomposition/modules/module_dependency_register.md` | **N/A change** — register row already lists `FactionStanding` / `ReputationDelta` contracts. |
| `server/README.md` | **Matches** — new ReputationOperations section documents commit order, rollback policy, pre-flight denies, and “do not call standing store alone.” |

## Blocking issues

None.

## Suggestions

1. ~~**Compensating rollback delta when clamping** — On audit append failure, rollback uses `-deltaAmount`:~~ **Done.** Rollback now uses `-appliedDelta` where `appliedDelta = mutation.NewStanding - mutation.PreviousStanding`; audit row records applied delta. Test `TryApplyDelta_ShouldRestoreClampedStanding_WhenAuditAppendFails` added.

2. ~~**Ops-layer id normalization (plan alignment)** — Plan §2 step 1 says normalize via **`FactionStandingIds`** before store calls.~~ **Done.** `TryApplyDelta` normalizes player/faction ids at the ops boundary; audit row built from normalized values.

## Nits

- ~~Nit: **`TryApplyDelta_ShouldApplyStandingAndAppendAudit_WhenHappyPath`** — `auditStore.GetDeltasForPlayer(PlayerId)` runs in the **Act** phase; per [csharp-style](../../.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert), verification reads belong in **Assert**.~~ **Done.** `GetDeltasForPlayer` moved to Assert.
- Nit: **`ReputationApplyOutcome`** on store-boundary deny returns standings from the mutation outcome (good); pre-flight **`Deny()`** zeroes standings — callers must not treat `0` as “neutral read” on `invalid_delta` / `invalid_source` denies (documented implicitly by reason codes).
- Nit: Telemetry hook comment for NEO-141 is correctly placed on the success path only — no action needed.

## Verification

Commands run during review (passed):

```bash
dotnet test NeonSprawl.sln --filter "FullyQualifiedName~ReputationOperationsTests"
dotnet test NeonSprawl.sln
```

Author should confirm PR CI green before merge. No manual QA doc required (server engine only per plan). **Intentional follow-on:** reward bundle rep wiring (NEO-138), quest accept gate eval (NEO-137), HTTP GET standing (NEO-139), Godot HUD (NEO-142+).