From 9adc87c9e19389ba4b8906c5e5e2298fc64411a3 Mon Sep 17 00:00:00 2001 From: ctan Date: Sun, 20 Sep 2026 17:05:36 +0200 Subject: [PATCH] fix(loaders): coerce combatant level with Pydantic-consistent rules Feat resolution read level with an isinstance(int) check and fell back to 1, while Pydantic accepts numeric strings and lossless floats: a level-4 sheet authored as "4" got Toughness for level 1. _parse_level now runs the value through TypeAdapter(int) (same lax coercion as the model) and raises CombatantLoadError for values the model would reject. --- src/pf1e_simulator/loaders/combatant.py | 24 +++++++++++++++++++++--- tests/test_combatant_loader.py | 22 ++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/pf1e_simulator/loaders/combatant.py b/src/pf1e_simulator/loaders/combatant.py index 9716113..4b013b3 100644 --- a/src/pf1e_simulator/loaders/combatant.py +++ b/src/pf1e_simulator/loaders/combatant.py @@ -19,7 +19,7 @@ import logging import re from typing import TYPE_CHECKING -from pydantic import ValidationError +from pydantic import TypeAdapter, ValidationError if TYPE_CHECKING: from pathlib import Path @@ -187,6 +187,25 @@ class CombatantLoadError(Exception): self.reason = reason +_LEVEL_ADAPTER: TypeAdapter[int] = TypeAdapter(int) + + +def _parse_level(raw_level: Json, path: Path) -> int: + """Coerce the JSON ``level`` with the same lax rules Pydantic applies. + + Feat resolution (Toughness) needs the level before model validation; + reading it with a bare ``isinstance(int)`` check disagreed with + Pydantic, which also accepts numeric strings ("4") and lossless + floats (4.0) -- the feat then silently used level 1 on a level-4 + sheet. A value Pydantic would reject raises CombatantLoadError here. + """ + try: + return _LEVEL_ADAPTER.validate_python(raw_level) + except ValidationError as exc: + msg = f"level must be an integer, got {raw_level!r}" + raise CombatantLoadError(path, msg) from exc + + def _resolve_features( raw_features: list[Json], level: int, path: Path ) -> list[AbilitySpec]: @@ -301,8 +320,7 @@ def load_combatant(path: Path) -> Combatant: # Resolve features (list of strings → list of AbilitySpec). A present # but non-list value (e.g. a bare string) used to be popped and dropped # silently, so the combatant lost every feat; it is now a data error. - raw_level = data.get("level") - level = raw_level if isinstance(raw_level, int) and not isinstance(raw_level, bool) else 1 + level = _parse_level(data.get("level", 1), path) had_features = "features" in data raw_features = data.pop("features", None) if had_features and not isinstance(raw_features, list): diff --git a/tests/test_combatant_loader.py b/tests/test_combatant_loader.py index 4b1ebe6..d71af16 100644 --- a/tests/test_combatant_loader.py +++ b/tests/test_combatant_loader.py @@ -146,6 +146,28 @@ class TestFeatures: c = load_combatant(_write(tmp_path, data)) assert c.features[0].effects == toughness(7).effects + def test_toughness_numeric_string_level(self, tmp_path: Path) -> None: + data = _minimal_payload(level="4", features=["Toughness"]) + c = load_combatant(_write(tmp_path, data)) + assert c.level == 4 + assert c.features[0].effects == toughness(4).effects + + def test_toughness_lossless_float_level(self, tmp_path: Path) -> None: + data = _minimal_payload(level=4.0, features=["Toughness"]) + c = load_combatant(_write(tmp_path, data)) + assert c.level == 4 + assert c.features[0].effects == toughness(4).effects + + def test_non_numeric_level_raises(self, tmp_path: Path) -> None: + data = _minimal_payload(level="abc", features=["Toughness"]) + with pytest.raises(CombatantLoadError, match="level must be an integer"): + load_combatant(_write(tmp_path, data)) + + def test_fractional_level_raises(self, tmp_path: Path) -> None: + data = _minimal_payload(level=4.5, features=["Toughness"]) + with pytest.raises(CombatantLoadError, match="level must be an integer"): + load_combatant(_write(tmp_path, data)) + def test_level_default_one(self, tmp_path: Path) -> None: data = _minimal_payload(level=1, features=["Toughness"]) c = load_combatant(_write(tmp_path, data))