neon-sprawl/docs/reviews/2026-05-31-NEO-102.md

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 (see Suggestion 1). Partially matches — E5.M3 note still ends with NEO-102+ not started.
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

# 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.