fix(loaders): non-list features and blank/non-string entries fail cleanly

features was popped and only re-added when it was a list, so a bare
string (or dict/null) loaded with zero feats - silent data loss. Now a
present non-list features raises CombatantLoadError, and the list
resolution adopts an accept-string-or-error contract: non-string or
blank entries raise instead of being silently dropped.

src/pf1e_simulator/loaders/combatant.py

@@ -187,13 +187,24 @@ class CombatantLoadError(Exception):
         self.reason = reason
 
 
-def _resolve_features(raw_features: list[Json], level: int) -> list[AbilitySpec]:
-    """Resolve a list of feat name strings to AbilitySpec objects."""
-    return [
-        _resolve_feat(item, level)
-        for item in raw_features
-        if isinstance(item, str) and item.strip()
-    ]
+def _resolve_features(
+    raw_features: list[Json], level: int, path: Path
+) -> list[AbilitySpec]:
+    """Resolve a list of feat name strings to AbilitySpec objects.
+
+    Data contract (accept-string-or-error): every entry must be a
+    non-blank string.  A non-string entry (``42``, ``None``, a nested
+    list) or a blank/whitespace-only string is a data error and raises
+    ``CombatantLoadError`` -- nothing is silently dropped, so a feat can
+    never disappear from a sheet without the loader failing loudly.
+    """
+    specs: list[AbilitySpec] = []
+    for item in raw_features:
+        if not isinstance(item, str) or not item.strip():
+            msg = f"features entries must be non-empty feat name strings, got {item!r}"
+            raise CombatantLoadError(path, msg)
+        specs.append(_resolve_feat(item, level))
+    return specs
 
 
 def _merge_resource_grants(
@@ -287,13 +298,19 @@ def load_combatant(path: Path) -> Combatant:
 
     _auto_attack_ids(data, monster_id)
 
-    # Resolve features (list of strings → list of AbilitySpec)
+    # 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
+    had_features = "features" in data
     raw_features = data.pop("features", None)
+    if had_features and not isinstance(raw_features, list):
+        msg = f"features must be a list of feat name strings, got {raw_features!r}"
+        raise CombatantLoadError(path, msg)
     validate_payload: dict[str, object] = dict(data)
     if isinstance(raw_features, list):
-        specs = _resolve_features(raw_features, level)
+        specs = _resolve_features(raw_features, level, path)
         validate_payload["features"] = specs
         granted = _merge_resource_grants(data.get("resources"), specs)
         if granted:

tests/test_combatant_loader.py

@@ -76,22 +76,24 @@ class TestResolveFeat:
 
 
 class TestResolveFeatures:
-    def test_filters_empty_and_whitespace(self) -> None:
-        result = _resolve_features(["Power Attack", "", "  ", "Dodge"], 1)
-        assert len(result) == 2
-        assert result[0] is POWER_ATTACK
-        assert result[1] is DODGE
+    def test_blank_entry_raises(self, tmp_path: Path) -> None:
+        with pytest.raises(CombatantLoadError, match="non-empty feat name strings"):
+            _resolve_features(["Power Attack", "", "  ", "Dodge"], 1, tmp_path)
 
-    def test_filters_non_strings(self) -> None:
+    def test_non_string_entry_raises(self, tmp_path: Path) -> None:
         raw: list[Json] = ["Iron Will", 42, None, True]
-        result = _resolve_features(raw, 1)
-        assert len(result) == 1
+        with pytest.raises(CombatantLoadError, match="non-empty feat name strings"):
+            _resolve_features(raw, 1, tmp_path)
 
-    def test_empty_list(self) -> None:
-        assert _resolve_features([], 1) == []
+    def test_valid_strings_resolve(self, tmp_path: Path) -> None:
+        result = _resolve_features(["Power Attack", "Dodge"], 1, tmp_path)
+        assert result == [POWER_ATTACK, DODGE]
 
-    def test_all_unknown(self) -> None:
-        result = _resolve_features(["Feat A", "Feat B"], 1)
+    def test_empty_list(self, tmp_path: Path) -> None:
+        assert _resolve_features([], 1, tmp_path) == []
+
+    def test_all_unknown(self, tmp_path: Path) -> None:
+        result = _resolve_features(["Feat A", "Feat B"], 1, tmp_path)
         assert len(result) == 2
         assert all(f.effects == () for f in result)
 
@@ -159,6 +161,26 @@ class TestFeatures:
         c = load_combatant(_write(tmp_path, data))
         assert c.features == []
 
+    def test_string_features_raises(self, tmp_path: Path) -> None:
+        data = _minimal_payload(features="Toughness")
+        with pytest.raises(CombatantLoadError, match="features must be a list"):
+            load_combatant(_write(tmp_path, data))
+
+    def test_dict_features_raises(self, tmp_path: Path) -> None:
+        data = _minimal_payload(features={"a": "Toughness"})
+        with pytest.raises(CombatantLoadError, match="features must be a list"):
+            load_combatant(_write(tmp_path, data))
+
+    def test_null_features_raises(self, tmp_path: Path) -> None:
+        data = _minimal_payload(features=None)
+        with pytest.raises(CombatantLoadError, match="features must be a list"):
+            load_combatant(_write(tmp_path, data))
+
+    def test_non_string_entry_raises(self, tmp_path: Path) -> None:
+        data = _minimal_payload(features=["Toughness", 42])
+        with pytest.raises(CombatantLoadError, match="non-empty feat name strings"):
+            load_combatant(_write(tmp_path, data))
+
 
 class TestSpells:
     def test_spells_passed_through(self, tmp_path: Path) -> None: