The branch review approved the slice with no blocking findings. These
are the substantive non-blocking ones.
N1, the one no per-task review could see: every seeded monster rolls
silverMin/silverMax = 0 (R7), so the victory screen showed "Silber +0"
after every fight in the shipped game. Three tasks were each correct in
isolation -- the mechanism stays, the values are zero, the field still
exists -- and the composite was wrong. Now conditional, with a test.
This does not contradict R16: R16 deleted the XP block because the
field ceased to exist, leaving nothing to hide. Silver still exists and
can be non-zero, so a conditional is the right tool.
N2: world.store.ts still said a combat "granted XP and silver". Same
false-fact-in-a-comment defect fixed in b5bcd50, one file over.
N4: resolveReputationRank threw a TypeError on negative input, since
findIndex returns -1 and REPUTATION_RANKS[-1] is undefined. Unreachable
today, but grantReputation is public and accepts any number.
N5/N6: grantReputation resolved factions without the enabled filter the
read path applies, so a disabled faction could accumulate invisible
reputation -- "disabled" was not actually a kill switch. The dense read
also had no ORDER BY, so the list could reorder between requests.
N8: design 13 requires silver stay 0 for every seeded monster; only two
of four were pinned. Re-adding silver to the others would have shipped
silently.
N9: spec 36's "a normal kill grants no Renown" had no test. It was
structurally guaranteed but unasserted -- now locked down against a
later slice wiring renown into combat.
N10: an impossible renown: 0 fixture, and RENOWN_MIN exported but never
used to clamp the floor.
API 268/268, web 230/230, API build zero errors.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Design ruling R7 abolishes XP as a concept and says the column goes with
it, but no task in the plan actually dropped it -- the plan only dropped
characters.experience and combat_rewards.experience_granted. Task 9
removed experienceReward from the seed literals, leaving
monster_definitions.experience_reward as a NOT NULL column with no
default that nothing supplies. The first monster insert against a real
database would have failed on a constraint violation.
No suite here could have caught it: none of them connect to Postgres.
Drops the column in the slice migration (which has never been run, so
amending it in place is correct rather than stacking a second one),
removes the entity field, and clears the three test fixtures that still
set it. silver_min/silver_max deliberately stay -- spec 15 keeps a
direct currency drop available as a lore-valid exception, and XP has no
such carve-out.
Also retargets the seed idempotency test off renown: 1, which is the
seed's own default and so could not distinguish "preserved" from
"reset to default".
NOTE ON SCOPE: this commit also absorbs a Prettier reformatting pass
that was already sitting uncommitted in the working tree, which is why
it touches ~59 files. That churn is purely cosmetic line-rewrapping --
verified by inspection, and the suite is green at 267/267 with the build
at exactly the 3 expected errors owned by Tasks 10 and 11. The repo is
not Prettier-clean at baseline (119 files still flagged), so this was a
partial run by an earlier step, not a deliberate repo-wide format.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
