fix(magic): normalize spell-database keys so kebab-case lookups resolve#53
Merged
fix(magic): normalize spell-database keys so kebab-case lookups resolve#53
Conversation
📝 WalkthroughWalkthroughThis change canonicalizes spell lookup keys (lowercase, remove spaces/hyphens/underscores), indexes each spell under normalized name and id, makes lookups delimiter-insensitive, deduplicates iteration results, and updates SPELL_COUNT to reflect unique Spell objects. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Spells were registered under name.toLowerCase() (e.g. "fire bolt") while
callers passed kebab-case ids ("fire-bolt"). getSpell did the same
normalization on the read side, so every kebab-case lookup returned
undefined and surfaced as "Unknown spell" — even though the spell was
in the database.
- Introduce normalizeSpellKey: strips whitespace/hyphens/underscores and
lowercases, accepting "Fire Bolt" / "fire-bolt" / "fire_bolt" /
"FIREBOLT" / "firebolt" interchangeably.
- registerSpell indexes each spell under both the spaced name and the
kebab-case id so any sensible call resolves.
- getSpell and spellExists use the same normalizer.
- SPELL_COUNT switches to deduped Set count.
- Add focused regression tests covering the live-test failure cases:
fire-bolt, magic-missile, sacred-flame, eldritch-blast, cure-wounds,
hold-person, spiritual-weapon, bless.
Closes #41
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/engine/magic/spell-database.ts (1)
395-413:⚠️ Potential issue | 🟡 MinorHaste is level 3, not level 5.
The spell is placed under the "5TH LEVEL SPELLS" section comment but
level: 3is correct per D&D 5e SRD. Either move this spell registration to the 3rd-level section (after Counterspell) or update the section comment. Same applies toFlyimmediately below.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/engine/magic/spell-database.ts` around lines 395 - 413, The Haste registration (registerSpell with id: 'haste') is correctly set to level: 3 but resides under a "5TH LEVEL SPELLS" section; move the registerSpell block for id 'haste' (and likewise the subsequent Fly registration) into the 3rd-level spell section (e.g., after Counterspell) or update the surrounding section comment to accurately reflect 3rd-level spells so the section header matches the spell.level values; ensure you update only the comment/position and not the level field on the registerSpell entries.
🧹 Nitpick comments (1)
tests/engine/magic/spell-database.test.ts (1)
38-41: Consider adding edge-case tests for malformed input.The unknown-spell test is good, but consider adding tests for edge cases like empty strings or whitespace-only input to ensure
normalizeSpellKeyhandles them gracefully without throwing.💡 Optional edge-case tests
it('handles empty and whitespace-only input gracefully', () => { expect(getSpell('')).toBeUndefined(); expect(getSpell(' ')).toBeUndefined(); expect(spellExists('')).toBe(false); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/engine/magic/spell-database.test.ts` around lines 38 - 41, Add tests for malformed inputs (empty string and whitespace-only) to ensure getSpell and spellExists behave gracefully and that normalizeSpellKey doesn't throw; specifically, add assertions like expect(getSpell('')).toBeUndefined(), expect(getSpell(' ')).toBeUndefined(), and expect(spellExists('')).toBe(false). If normalizeSpellKey currently throws or returns unexpected values for blank/whitespace input, update it to trim input and handle empty result (e.g., return a safe normal form or undefined lookup) so these new tests pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/engine/magic/spell-database.ts`:
- Around line 29-30: SPELL_DATABASE is inserting each spell twice
(normalizeSpellKey(spell.name) and normalizeSpellKey(spell.id)), causing
duplicates when iterating via SPELL_DATABASE.values(); update the iteration
helpers getSpellsByLevel and getSpellsForClass to dedupe results by tracking
seen spell IDs (e.g., create a local Set seen and skip any spell whose spell.id
is already in seen) before pushing into the results, ensuring SPELL_COUNT can
remain as-is and normalizeSpellKey/SPELL_DATABASE inserts need not be changed.
---
Outside diff comments:
In `@src/engine/magic/spell-database.ts`:
- Around line 395-413: The Haste registration (registerSpell with id: 'haste')
is correctly set to level: 3 but resides under a "5TH LEVEL SPELLS" section;
move the registerSpell block for id 'haste' (and likewise the subsequent Fly
registration) into the 3rd-level spell section (e.g., after Counterspell) or
update the surrounding section comment to accurately reflect 3rd-level spells so
the section header matches the spell.level values; ensure you update only the
comment/position and not the level field on the registerSpell entries.
---
Nitpick comments:
In `@tests/engine/magic/spell-database.test.ts`:
- Around line 38-41: Add tests for malformed inputs (empty string and
whitespace-only) to ensure getSpell and spellExists behave gracefully and that
normalizeSpellKey doesn't throw; specifically, add assertions like
expect(getSpell('')).toBeUndefined(), expect(getSpell(' ')).toBeUndefined(),
and expect(spellExists('')).toBe(false). If normalizeSpellKey currently throws
or returns unexpected values for blank/whitespace input, update it to trim input
and handle empty result (e.g., return a safe normal form or undefined lookup) so
these new tests pass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c63508e7-5907-4dd6-a18b-635007c01751
📒 Files selected for processing (2)
src/engine/magic/spell-database.tstests/engine/magic/spell-database.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87db8807d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }; | ||
| SPELL_DATABASE.set(spell.name.toLowerCase(), spell); | ||
| SPELL_DATABASE.set(normalizeSpellKey(spell.name), spell); | ||
| SPELL_DATABASE.set(normalizeSpellKey(spell.id), spell); |
There was a problem hiding this comment.
Deduplicate results when adding secondary spell keys
Adding a second Map entry for each spell can cause duplicate results from getSpellsByLevel/getSpellsForClass, because both functions iterate SPELL_DATABASE.values() without deduplication. This appears when normalizeSpellKey(spell.name) and normalizeSpellKey(spell.id) differ (for example, punctuation differences such as apostrophes), so one logical spell can be emitted multiple times and skew selection/count logic in callers.
Useful? React with 👍 / 👎.
Reviewers (PR #53) flagged that registering each spell under both spell.name and spell.id keys causes SPELL_DATABASE.values() to yield duplicates. getSpellsByLevel() and getSpellsForClass() iterated those values directly, returning each spell twice. Adds a uniqueSpells() helper that wraps the iteration in a Set, and routes both helpers (and getCantrips by extension) through it. Regression tests assert no duplicates in level/class queries. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/engine/magic/spell-database.ts (1)
29-30: Add collision protection when writing normalized keys.
SPELL_DATABASE.set(...)currently overwrites silently if two different spells normalize to the same key. Guarding this inregisterSpellwill prevent hard-to-debug data loss later.Suggested hardening
function registerSpell(input: SpellInput): void { const spell: Spell = { ...input, ritual: input.ritual ?? false }; - SPELL_DATABASE.set(normalizeSpellKey(spell.name), spell); - SPELL_DATABASE.set(normalizeSpellKey(spell.id), spell); + const keys = [normalizeSpellKey(spell.name), normalizeSpellKey(spell.id)]; + for (const key of keys) { + const existing = SPELL_DATABASE.get(key); + if (existing && existing.id !== spell.id) { + throw new Error(`Spell key collision for "${key}": "${existing.id}" vs "${spell.id}"`); + } + SPELL_DATABASE.set(key, spell); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/engine/magic/spell-database.ts` around lines 29 - 30, SPELL_DATABASE.set calls using normalizeSpellKey(spell.name) and normalizeSpellKey(spell.id) can silently overwrite an existing entry; modify the registerSpell logic so before calling SPELL_DATABASE.set you check SPELL_DATABASE.has(key) and compare the existing entry's id (or reference) to the incoming spell, and if they differ either throw an error or log and skip the set to avoid collision; apply this check for both normalizeSpellKey(spell.name) and normalizeSpellKey(spell.id) and include clear diagnostic text mentioning the colliding key and the two spell ids.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/engine/magic/spell-database.ts`:
- Around line 587-595: The spell lookup uses normalizeSpellKey for reads but the
database population is inserting keys that aren't normalized, so kebab-case IDs
like "scorching-ray", "healing-word", and "web" never get registered; fix the
database population to store normalized keys by calling normalizeSpellKey when
inserting into SPELL_DATABASE (wherever SPELL_DATABASE.set or the initial
registration/population occurs) so that SPELL_DATABASE keys match the
normalization used by get (normalizeSpellKey(name)) and spellExists(name).
---
Nitpick comments:
In `@src/engine/magic/spell-database.ts`:
- Around line 29-30: SPELL_DATABASE.set calls using
normalizeSpellKey(spell.name) and normalizeSpellKey(spell.id) can silently
overwrite an existing entry; modify the registerSpell logic so before calling
SPELL_DATABASE.set you check SPELL_DATABASE.has(key) and compare the existing
entry's id (or reference) to the incoming spell, and if they differ either throw
an error or log and skip the set to avoid collision; apply this check for both
normalizeSpellKey(spell.name) and normalizeSpellKey(spell.id) and include clear
diagnostic text mentioning the colliding key and the two spell ids.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8c9bdfe-bf9d-4134-b0f1-15029c9754fb
📒 Files selected for processing (2)
src/engine/magic/spell-database.tstests/engine/magic/spell-database.test.ts
✅ Files skipped from review due to trivial changes (1)
- tests/engine/magic/spell-database.test.ts
… list Reviewers (PR #53) flagged that the issue-#41 acceptance criteria listed 10 baseline spells and three were still missing: - Scorching Ray (L2 evocation, sorcerer/wizard, 6d6 fire +2d6/upcast) - Healing Word (L1 evocation, bard/cleric/druid, bonus action 1d4) - Web (L2 conjuration, sorcerer/wizard, 20ft cube concentration restrain) All three are now registered with the same dual-key normalization the rest of the catalog uses. Lookup tests extended to cover them via both canonical name and kebab-case id. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/engine/magic/spell-database.ts (1)
756-758:SPELL_COUNTcan drift from the exported map.This value is frozen at module load, but
SPELL_DATABASEis exported as a mutableMap. Any later registration or test mutation makes the count stale. A helper likegetSpellCount()would keep this invariant accurate.Suggested direction
-// Export total unique-spell count for tests (each spell may be indexed under -// multiple keys, so dedupe before counting). -export const SPELL_COUNT = new Set(SPELL_DATABASE.values()).size; +// Compute total unique-spell count on demand so it stays aligned with the map. +export function getSpellCount(): number { + return uniqueSpells().length; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/engine/magic/spell-database.ts` around lines 756 - 758, SPELL_COUNT is computed once at module load and can drift because SPELL_DATABASE is a mutable Map; replace the exported constant with a runtime getter (e.g., export function getSpellCount() or an exported getter named SPELL_COUNT) that computes and returns new Set(SPELL_DATABASE.values()).size on each call so the count always reflects the current SPELL_DATABASE contents; update any tests/imports to call the function/getter instead of using the frozen constant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/engine/magic/spell-database.ts`:
- Around line 242-313: Add cast-path tests in spellcasting.test.ts for the three
new spells: for 'healing-word' (id: healing-word) write a test that uses the
existing castSpell helper to target a creature and assert HP increases by 1d4 +
spellcasting modifier (and that upcasting (slot >1) increases healing by +1d4
per slot via the upcastBonus); for 'scorching-ray' (id: scorching-ray) write a
test that casts the spell and asserts it produces three separate attack/damage
instances of 2d6 fire each (and that upcasting grants extra ray(s) per
upcastBonus); and for 'web' (id: web) write a test that casts on an area and
asserts affected creatures must make a Dexterity save and failing results in the
RESTRAINED condition being applied (and concentration/duration behavior as
appropriate). Use the same test helpers and assertions used by existing spell
tests (e.g., castSpell, getSpellById, checking resulting HP/damage events and
conditions) to mirror style and sizing.
---
Nitpick comments:
In `@src/engine/magic/spell-database.ts`:
- Around line 756-758: SPELL_COUNT is computed once at module load and can drift
because SPELL_DATABASE is a mutable Map; replace the exported constant with a
runtime getter (e.g., export function getSpellCount() or an exported getter
named SPELL_COUNT) that computes and returns new
Set(SPELL_DATABASE.values()).size on each call so the count always reflects the
current SPELL_DATABASE contents; update any tests/imports to call the
function/getter instead of using the frozen constant.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 49459343-b1b9-47c6-847a-d4a91a2320d5
📒 Files selected for processing (2)
src/engine/magic/spell-database.tstests/engine/magic/spell-database.test.ts
✅ Files skipped from review due to trivial changes (1)
- tests/engine/magic/spell-database.test.ts
Comment on lines
+242
to
+313
| registerSpell({ | ||
| id: 'healing-word', | ||
| name: 'Healing Word', | ||
| level: 1, | ||
| school: 'evocation', | ||
| castingTime: 'bonus_action', | ||
| range: 60, | ||
| components: { verbal: true, somatic: false, material: false }, | ||
| duration: 'Instantaneous', | ||
| concentration: false, | ||
| description: 'A creature of your choice within range regains 1d4 + your spellcasting ability modifier.', | ||
| higherLevels: 'Healing increases by 1d4 for each slot level above 1st.', | ||
| classes: ['bard', 'cleric', 'druid'], | ||
| targetType: 'creature', | ||
| effects: [{ | ||
| type: 'healing', | ||
| dice: '1d4', | ||
| upcastBonus: { dice: '1d4', perLevel: 1 } | ||
| }], | ||
| autoHit: false | ||
| }); | ||
|
|
||
| // ============================================================================ | ||
| // 2ND LEVEL SPELLS | ||
| // ============================================================================ | ||
|
|
||
| registerSpell({ | ||
| id: 'scorching-ray', | ||
| name: 'Scorching Ray', | ||
| level: 2, | ||
| school: 'evocation', | ||
| castingTime: 'action', | ||
| range: 120, | ||
| components: { verbal: true, somatic: true, material: false }, | ||
| duration: 'Instantaneous', | ||
| concentration: false, | ||
| description: 'You create three rays of fire and hurl them at targets within range. Each ray deals 2d6 fire on a hit.', | ||
| higherLevels: 'You create one additional ray for each slot level above 2nd.', | ||
| classes: ['sorcerer', 'wizard'], | ||
| targetType: 'creature', | ||
| effects: [{ | ||
| type: 'damage', | ||
| dice: '6d6', | ||
| damageType: 'fire', | ||
| saveType: 'none', | ||
| upcastBonus: { dice: '2d6', perLevel: 1 } | ||
| }], | ||
| autoHit: false | ||
| }); | ||
|
|
||
| registerSpell({ | ||
| id: 'web', | ||
| name: 'Web', | ||
| level: 2, | ||
| school: 'conjuration', | ||
| castingTime: 'action', | ||
| range: 60, | ||
| components: { verbal: true, somatic: true, material: true, materialDescription: 'a bit of spiderweb' }, | ||
| duration: 'Concentration, up to 1 hour', | ||
| concentration: true, | ||
| description: 'You conjure a mass of thick, sticky webbing in a 20-foot cube. Creatures starting their turn in the webs or entering them on their turn must make a Dexterity saving throw or become restrained.', | ||
| classes: ['sorcerer', 'wizard'], | ||
| targetType: 'area', | ||
| areaOfEffect: { shape: 'cube', size: 20 }, | ||
| effects: [{ | ||
| type: 'debuff', | ||
| saveType: 'dexterity', | ||
| saveEffect: 'none', | ||
| conditions: ['RESTRAINED'] | ||
| }], | ||
| autoHit: false | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for spell in healing-word scorching-ray web; do
echo "=== $spell ==="
rg -n -C2 --glob 'tests/**/*.ts' "\"$spell\"|'$spell'" tests || true
doneRepository: Mnehmos/mnehmos.rpg.mcp
Length of output: 1487
🏁 Script executed:
# Search for cast or casting related tests
find tests -name '*.test.ts' -o -name '*.spec.ts' | head -20Repository: Mnehmos/mnehmos.rpg.mcp
Length of output: 766
🏁 Script executed:
# Search for spell-casting or cast-related test patterns
rg -l 'cast|spell.*effect|apply.*spell' tests/ --type ts | head -20Repository: Mnehmos/mnehmos.rpg.mcp
Length of output: 547
🏁 Script executed:
# Search for these specific spell references in all test files, not just by name
rg -n 'healing-word|scorching-ray' tests/ --type ts -B2 -A5Repository: Mnehmos/mnehmos.rpg.mcp
Length of output: 899
🏁 Script executed:
# Check test file structure - how many test files exist
find tests -name '*.test.ts' -o -name '*.spec.ts' | wc -lRepository: Mnehmos/mnehmos.rpg.mcp
Length of output: 70
🏁 Script executed:
# Check the contents of spellcasting.test.ts
cat -n tests/server/spellcasting.test.ts | head -100Repository: Mnehmos/mnehmos.rpg.mcp
Length of output: 4488
🏁 Script executed:
# Search specifically in spellcasting.test.ts for references to our three spells
rg -n 'healing-word|scorching-ray|web' tests/server/spellcasting.test.ts -B3 -A3Repository: Mnehmos/mnehmos.rpg.mcp
Length of output: 49
🏁 Script executed:
# Check what spells are actually tested in spellcasting.test.ts
rg -n "id:.*'|spell.*id" tests/server/spellcasting.test.ts | head -30Repository: Mnehmos/mnehmos.rpg.mcp
Length of output: 2970
🏁 Script executed:
# Search all test files for these spell IDs in cast/casting contexts
rg -n 'spellId.*healing-word|spellId.*scorching-ray|spellId.*web|"healing-word"|"scorching-ray"' tests/ --type ts -B2 -A2Repository: Mnehmos/mnehmos.rpg.mcp
Length of output: 49
🏁 Script executed:
# Check the full spell-database.test.ts to understand what's actually being tested
wc -l tests/engine/magic/spell-database.test.tsRepository: Mnehmos/mnehmos.rpg.mcp
Length of output: 111
🏁 Script executed:
# Get the context around the spell lookup tests in spell-database.test.ts
sed -n '20,60p' tests/engine/magic/spell-database.test.tsRepository: Mnehmos/mnehmos.rpg.mcp
Length of output: 2071
🏁 Script executed:
# Search for cast-related test descriptions across all test files
rg -n "it.*cast|describe.*cast|it.*spell.*effect|it.*healing|it.*damage" tests/ --type ts | grep -i "healing-word\|scorching-ray\|web" || echo "No cast tests found for these spells"Repository: Mnehmos/mnehmos.rpg.mcp
Length of output: 170
🏁 Script executed:
# Check if there are any references to issue `#41` in the codebase
rg -n "issue.*41|#41" . --type ts --type md -B2 -A2Repository: Mnehmos/mnehmos.rpg.mcp
Length of output: 1144
🏁 Script executed:
# View the entire spell-database.test.ts file to see all tests
cat -n tests/engine/magic/spell-database.test.tsRepository: Mnehmos/mnehmos.rpg.mcp
Length of output: 3725
🏁 Script executed:
# Search for any cast action tests that might use spell IDs generically
rg -n "castSpell|executeSpell|executeAction.*spell|spellAction" tests/ --type ts | head -20Repository: Mnehmos/mnehmos.rpg.mcp
Length of output: 2523
Add cast-path tests for the three newly registered spells.
The three spells—healing-word, scorching-ray, and web—currently appear only in the lookup/dedup regression test (spell-database.test.ts), confirming they can be resolved by name and kebab-id variants. However, they lack spell-casting tests that verify their effects and mechanics (as other spells like Magic Missile, Fireball, and Cure Wounds have in spellcasting.test.ts). Add at least one cast-oriented test per spell to validate healing amounts, damage rolls, and condition applications.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/engine/magic/spell-database.ts` around lines 242 - 313, Add cast-path
tests in spellcasting.test.ts for the three new spells: for 'healing-word' (id:
healing-word) write a test that uses the existing castSpell helper to target a
creature and assert HP increases by 1d4 + spellcasting modifier (and that
upcasting (slot >1) increases healing by +1d4 per slot via the upcastBonus); for
'scorching-ray' (id: scorching-ray) write a test that casts the spell and
asserts it produces three separate attack/damage instances of 2d6 fire each (and
that upcasting grants extra ray(s) per upcastBonus); and for 'web' (id: web)
write a test that casts on an area and asserts affected creatures must make a
Dexterity save and failing results in the RESTRAINED condition being applied
(and concentration/duration behavior as appropriate). Use the same test helpers
and assertions used by existing spell tests (e.g., castSpell, getSpellById,
checking resulting HP/damage events and conditions) to mirror style and sizing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The spell catalog already contained
Fire Bolt,Magic Missile,Sacred Flame, etc., butgetSpell("fire-bolt")returnedundefined. Root cause: the map was keyed byname.toLowerCase()("fire bolt") andgetSpelldid the same normalization, so kebab-case ids never matched. Surfaced asUnknown spell: <name>for almost every common spell.Fix
normalizeSpellKeycollapses whitespace/hyphens/underscores and lowercases.getSpell/spellExistsuse the same normalizer, so"Fire Bolt" / "fire-bolt" / "fire_bolt" / "FIREBOLT" / "firebolt"all resolve to the same record.Test plan
tests/engine/magic/spell-database.test.ts) covers the eight spells the live test couldn't find.Closes #41
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests