neon-sprawl/docs/reviews/2026-05-24-NEO-67.md

Code review — NEO-67 recipe definition registry + DI

Date: 2026-05-24
Scope: Branch NEO-67-recipe-definition-registry-di · commits 7188c5b–81b0e3f vs origin/main
Base: origin/main

Verdict

Approve with nits

Summary

NEO-67 delivers E3M2-03: a thin IRecipeDefinitionRegistry / RecipeDefinitionRegistry adapter over the NEO-66 startup-loaded RecipeDefinitionCatalog, registered in DI alongside the catalog singleton. The interface mirrors IItemDefinitionRegistry (NEO-52): TryGetDefinition(string? recipeId, …) and GetDefinitionsInIdOrder() with ordinal id ordering. Seven AAA unit/host tests pass; docs (plan, E3M2 backlog, E3.M2 snapshot, alignment register, server/README.md) are updated. No HTTP, craft engine, or client work — correctly deferred to NEO-68+. Risk is very low: pure adapter + DI wiring over immutable catalog data; no behavior change at boot beyond an extra singleton registration.

Documentation checked

Document	Result
docs/plans/NEO-67-implementation-plan.md	Matches — interface, adapter, DI extension, comment/README touches, acceptance checklist complete; HTTP/craft/loader changes correctly out of scope.
docs/plans/E3M2-prototype-backlog.md	Matches — E3M2-03 acceptance checkbox + landed note.
docs/decomposition/modules/E3_M2_RefinementAndRecipeExecution.md	Matches — implementation snapshot references NEO-67 registry.
docs/decomposition/modules/documentation_and_implementation_alignment.md	Matches — E3.M2 row updated with NEO-67 landed note; next backlog item NEO-68.
docs/decomposition/modules/module_dependency_register.md	Partially matches — table Status still Planned for E3.M2 (same pattern as post–NEO-66); optional bump when Slice 3 server spine completes.
docs/decomposition/modules/client_server_authority.md	N/A — server DI/read surface only; no client mutation.
server/README.md	Matches — recipe catalog section documents IRecipeDefinitionRegistry as preferred lookup surface.
Manual QA	N/A — plan documents skip rationale: automated unit/host tests (same as NEO-52 / NEO-59); no player-visible HTTP.

Register/tracking: alignment table E3.M2 In Progress with NEO-67 note is correct.

Blocking issues

None.

Suggestions

None.

Nits

• Nit: RecipeDefinitionRegistry.TryGetDefinition reads catalog.ById directly instead of delegating to catalog.TryGetRecipe — consistent with ItemDefinitionRegistry (NEO-52), but both bypass the catalog’s public lookup helper; harmless for immutable data. Done. Registry now delegates to catalog.TryGetRecipe.

• Nit: GetDefinitionsInIdOrder() allocates a new sorted list on every call — acceptable for prototype HTTP/craft volume; optional future optimization is to cache the ordered list at catalog/registry construction time if hot-path profiling warrants it (same as item registry). Done. Ordered list built once in RecipeDefinitionRegistry constructor.

• Nit: Host_ShouldResolveRegistryFromDi_WhenStartupSucceeds asserts a known id and enumeration count but not an unknown id in the host context — unit tests cover unknown/null; host test could add one line for symmetry with E3M2-03 acceptance wording (“distinguishable from valid ids in tests”). Done. Host test asserts unknown recipeId returns false without throw.

Verification

cd /home/don/neon-sprawl/server
dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj --filter "FullyQualifiedName~RecipeDefinitionRegistryTests"
dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj

Manual (optional):

• Start server from repo root; confirm existing NEO-66 Information log (recipe count 8) unchanged — registry does not alter boot semantics.