# Code review — NEO-102 (E5M3-03)

**Date:** 2026-05-31  
**Scope:** Branch `NEO-102-injectable-encounter-reward-registries-di` vs `d7c2684` (merge-base on `main`) — commits `93eca82` … `734876c`  
**Base:** `d7c2684` (main at branch point, post NEO-101 merge)

## Verdict

**Approve with nits**

## Summary

NEO-102 adds thin **`IEncounterDefinitionRegistry`** and **`IRewardTableDefinitionRegistry`** adapters over the NEO-101 startup-loaded catalogs, mirroring the established NEO-79 **`AbilityDefinitionRegistry`** pattern: `TryGetDefinition`, `TryNormalizeKnown` (trim + lowercase + fail-closed lookup), and cached `GetDefinitionsInIdOrder`. DI registers both singletons in `AddEncounterAndRewardCatalogs` after catalog loaders; no `Program.cs` change (catalogs remain the eager boot gate). Eighteen AAA unit + host DI tests pass locally. Docs (implementation plan, alignment register, E5M3 backlog, E5.M3 module snapshot, `server/README.md`) are updated. Risk is low: server-only registry surface with no HTTP or game call-site migration yet (correctly deferred to NEO-103+).

## Documentation checked

| Path | Result |
|------|--------|
| `docs/plans/NEO-102-implementation-plan.md` | **Matches** — interfaces, adapters, DI, tests, README, alignment register; acceptance checklist checked; reconciliation section accurate. |
| `docs/plans/E5M3-prototype-backlog.md` (E5M3-03) | **Matches** — acceptance criteria checked; landed note cites plan and 18 tests. |
| `docs/decomposition/modules/E5_M3_EncounterAndRewardTables.md` | **Matches** — Summary **Status** notes **NEO-102 registries landed**; **Implementation snapshot** paragraph added. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E5.M3 row notes **NEO-102** registries; runtime NEO-103+ still outstanding. |
| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E5.M3 note cites **NEO-102** registry landing; runtime NEO-103+ still outstanding. |
| Full-stack epic decomposition | **N/A** — plan explicitly excludes client counterpart until NEO-110 / NEO-111. |
| `server/README.md` | **Matches** — encounter + reward-table sections document registries as preferred lookup surface. |

## Blocking issues

None.

## Suggestions

1. ~~**Sync `module_dependency_register.md` E5.M3 note** — NEO-101 review added **NEO-101 landed** to the register note; this branch updates alignment + module pages but leaves the register saying **NEO-102+ not started**. Add **NEO-102 landed:** injectable **`IEncounterDefinitionRegistry`** + **`IRewardTableDefinitionRegistry`** + DI, mirroring the alignment register row, so the three tracking surfaces stay consistent.~~ **Done.** E5.M3 note updated; runtime NEO-103+ still outstanding.

## Nits

- Nit: `EncounterDefinitionRegistryTests.Host_ShouldResolveRegistriesFromDi_WhenStartupSucceeds` resolves **both** registries (per plan), while `RewardTableDefinitionRegistryTests.Host_ShouldResolveRegistryFromDi_WhenStartupSucceeds` repeats reward-table assertions. Harmless overlap with NEO-79’s single-registry host test; optional consolidation later.

- ~~Nit: `TryGetDefinition` is case-sensitive (delegates to ordinal catalog keys); only `TryNormalizeKnown` lowercases. Consistent with **`AbilityDefinitionRegistry`** — document for NEO-103 HTTP callers if they accept mixed-case query params.~~ **Done.** `server/README.md` encounter + reward-table sections note case-sensitive `TryGetDefinition` and wire validation via `TryNormalizeKnown`.

## Verification

```bash
# NEO-102-focused (18 tests — all passed locally)
dotnet test NeonSprawl.sln --filter "FullyQualifiedName~EncounterDefinitionRegistry|FullyQualifiedName~RewardTableDefinitionRegistry"

# Broader encounter slice (includes NEO-101 catalog tests)
dotnet test NeonSprawl.sln --filter "FullyQualifiedName~Encounters"

# CI parity
python3 scripts/validate_content.py
```

**Local run:** 18/18 registry tests passed.