From 9656ae820ae8a5cd8a9b1b0ca3d0b9234a589697 Mon Sep 17 00:00:00 2001
From: ctan <ctan@home.fr>
Date: Sun, 20 Sep 2026 16:58:57 +0200
Subject: [PATCH] fix(loaders): malformed spell_slots key raises
 CombatantLoadError, not bare ValueError

int(k) on a non-numeric JSON key escaped the loader as an unhandled
ValueError traceback (CLI exit 1) instead of the documented exit code 2.
Route the conversion through _parse_spell_slot_keys which wraps it in a
CombatantLoadError with a clear message. Audited the other loader-side
int()/conversions: foundry.py guards every int() with isinstance/isdigit
checks and combatant.py has no other unguarded conversion.
---
 src/pf1e_simulator/loaders/combatant.py | 21 ++++++++++++++++++++-
 tests/test_combatant_loader.py          | 19 +++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/src/pf1e_simulator/loaders/combatant.py b/src/pf1e_simulator/loaders/combatant.py
index dec2f60..55aa41f 100644
--- a/src/pf1e_simulator/loaders/combatant.py
+++ b/src/pf1e_simulator/loaders/combatant.py
@@ -221,6 +221,25 @@ def _merge_resource_grants(
     return merged
 
 
+def _parse_spell_slot_keys(raw_slots: dict[str, Json], path: Path) -> dict[int, Json]:
+    """Convert JSON ``spell_slots`` string keys to ints, failing cleanly.
+
+    JSON object keys are always strings; the ``Combatant.spell_slots`` field
+    is ``dict[int, int]``.  A non-numeric key (e.g. ``"one": 4``) is a data
+    error and must surface as a ``CombatantLoadError`` (CLI exit code 2),
+    never as a bare ``ValueError`` traceback escaping the loader.
+    """
+    parsed: dict[int, Json] = {}
+    for key, value in raw_slots.items():
+        try:
+            slot_level = int(key)
+        except ValueError as exc:
+            msg = f"spell_slots keys must be integer spell levels, got {key!r}"
+            raise CombatantLoadError(path, msg) from exc
+        parsed[slot_level] = value
+    return parsed
+
+
 def _auto_attack_ids(data: dict[str, Json], monster_id: str) -> None:
     """Auto-assign attack ids from file stem if missing."""
     attacks = data.get("attacks")
@@ -284,7 +303,7 @@ def load_combatant(path: Path) -> Combatant:
 
     raw_slots = data.pop("spell_slots", None)
     if isinstance(raw_slots, dict):
-        validate_payload["spell_slots"] = {int(k): v for k, v in raw_slots.items()}
+        validate_payload["spell_slots"] = _parse_spell_slot_keys(raw_slots, path)
 
     try:
         return Combatant.model_validate(validate_payload)
diff --git a/tests/test_combatant_loader.py b/tests/test_combatant_loader.py
index ea14670..0aa3cc2 100644
--- a/tests/test_combatant_loader.py
+++ b/tests/test_combatant_loader.py
@@ -5,6 +5,8 @@ from __future__ import annotations
 import json
 from typing import TYPE_CHECKING
 
+import pytest
+
 if TYPE_CHECKING:
     from pathlib import Path
 
@@ -175,6 +177,23 @@ class TestSpells:
         assert c.spells == []
 
 
+class TestSpellSlots:
+    def test_numeric_string_keys_converted(self, tmp_path: Path) -> None:
+        data = _minimal_payload(spell_slots={"1": 4, "2": 3})
+        c = load_combatant(_write(tmp_path, data))
+        assert c.spell_slots == {1: 4, 2: 3}
+
+    def test_non_numeric_key_raises_load_error(self, tmp_path: Path) -> None:
+        data = _minimal_payload(spell_slots={"one": 4})
+        with pytest.raises(CombatantLoadError, match="spell_slots keys must be integer"):
+            load_combatant(_write(tmp_path, data))
+
+    def test_float_key_raises_load_error(self, tmp_path: Path) -> None:
+        data = _minimal_payload(spell_slots={"1.5": 4})
+        with pytest.raises(CombatantLoadError, match="spell_slots keys must be integer"):
+            load_combatant(_write(tmp_path, data))
+
+
 class TestBackwardCompatAliases:
     def test_monster_load_error_is_combatant_load_error(self) -> None:
         assert MonsterLoadError is CombatantLoadError