NEO-150: Add code review for economy validation at issue

docs/reviews/2026-06-28-NEO-150.md

@@ -0,0 +1,64 @@
+# Code review — NEO-150 economy validation at issue time
+
+**Date:** 2026-06-28  
+**Scope:** Branch `NEO-150-e7m4-07-economy-validation-lint-at-issue-time` — commits `039542a`..`d5de4e8` (3 commits)  
+**Base:** `origin/main`  
+**Issue:** [NEO-150](https://linear.app/neon-sprawl/issue/NEO-150) — E7M4-07 economy validation lint at issue time
+
+## Verdict
+
+**Approve with nits**
+
+## Summary
+
+This branch adds defense-in-depth economy validation at contract issue time: `ContractEconomyValidation.TryValidateTemplate` delegates to existing `PrototypeE7M4ContractCatalogRules` band-cap and cross-ref helpers, wired into `ContractGeneratorOperations.TryIssue` after template selection and before persist. Two new reason codes (`economy_cap_exceeded`, `invalid_reward_bundle`) surface stable deny semantics for downstream HTTP (NEO-151) and client copy (NEO-153).
+
+The implementation matches the adopted kickoff plan: registry parameters on `TryIssue`, split reason codes, full loader-parity cross-ref scope, and no HTTP/client scope creep. All `TryIssue` call sites are updated; unit and integration tests cover happy path, over-cap deny, and unknown bundle ref deny. Full suite passes (911 tests). Risk is low — logic reuses proven CI/startup gates; main residual risk is per-issue registry snapshot allocation cost, acceptable at prototype scale.
+
+## Documentation checked
+
+| Document | Result |
+|----------|--------|
+| `docs/plans/NEO-150-implementation-plan.md` | **Matches** — AC checklist marked complete; shipped reconciliation aligns with diff |
+| `docs/decomposition/modules/E7_M4_ContractMissionGenerator.md` | **Matches** — NEO-150 economy validation note + E6.M4/E3.M5 parity stub added |
+| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E7.M4 row updated with E7M4-07 / NEO-150 |
+| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E7.M4 note references NEO-150 |
+| `server/README.md` | **Matches** — issue-time economy validation paragraph + reason-code table extended |
+| `docs/plans/E7M4-pre-production-backlog.md` | **Not re-read in diff** — plan cites it; backlog intent satisfied by slice scope |
+
+Register/tracking table updates are already included in this branch.
+
+## Blocking issues
+
+None.
+
+## Suggestions
+
+1. **Deduplicate prototype registry test fixtures** — `ContractEconomyValidationTests.CreatePrototypeRegistries()` and `ContractGeneratorOperationsTests.CreatePrototypeCatalogRegistries()` are nearly identical (~50 lines). Consider a shared internal test helper (e.g. `ContractPrototypeTestFixtures`) to avoid drift when prototype catalog ids change.
+
+2. **Broaden deny-path test matrix (optional)** — Current tests exercise item qty over cap and unknown item id only. Band-cap and cross-ref helpers also cover skill XP, reputation grants, unknown encounter, and disallowed `mission_reward` skill sources (covered at loader level in `ContractTemplateCatalogLoaderTests`). One additional `[Theory]` or a skill-Xp-over-cap case in `ContractEconomyValidationTests` would document parity without much cost.
+
+3. **NEO-151 call-site checklist** — When HTTP lands, confirm the production DI path passes the same four registries already wired in integration tests. No code change needed here; worth a one-line note in NEO-151 plan.
+
+## Nits
+
+- Nit: `ContractEconomyValidation.TryValidateTemplate` does not null-check `template`; safe today because `TrySelectTemplate` only returns success with a non-null template, but `ArgumentNullException.ThrowIfNull(template)` would mirror the registry guards.
+- Nit: Issue-time validation order is band-cap → cross-ref; startup loader runs cross-ref → band-cap. Intentional per plan (cap failures get `economy_cap_exceeded`), but worth keeping in mind if both gates ever fail on the same row during debugging.
+- Nit: Per-issue allocation of encounter/faction hash sets and skill dictionary is fine for prototype; revisit only if issue volume becomes hot-path.
+
+## Verification
+
+Commands run during review:
+
+```bash
+dotnet test NeonSprawl.sln --filter "FullyQualifiedName~ContractEconomyValidation|FullyQualifiedName~ContractGeneratorOperations"
+dotnet test NeonSprawl.sln
+```
+
+Results: 20 filtered tests passed; **911/911** full suite passed.
+
+Author pre-merge checklist:
+
+- [x] `dotnet test NeonSprawl.sln` green
+- [ ] Merge to `main` and proceed to NEO-151 (HTTP) with extended `TryIssue` signature
+- [ ] No Godot manual QA required (server policy per plan)