Add CardDefinition validation for required fields (Issue #2)

- Add model_validator to enforce card-type-specific required fields
- Pokemon: require hp (positive), stage, pokemon_type
- Pokemon Stage 1/2 and VMAX/VSTAR: require evolves_from
- Trainer: require trainer_type
- Energy: require energy_type (auto-fills energy_provides)
- Update all test fixtures to include required fields
- Mark Issue #2 as FIXED in SYSTEM_REVIEW.md

765 tests passing

backend/SYSTEM_REVIEW.md

@@ -3,16 +3,16 @@
 **Date:** 2026-01-25  
 **Updated:** 2026-01-26  
 **Reviewed By:** Multi-agent system review  
-**Status:** 766 tests passing, 94% coverage  
+**Status:** 765 tests passing, 94% coverage  
 
 ---
 
 ## Executive Summary
 
-The core engine has a solid foundation with good separation of concerns, comprehensive documentation, and thorough test coverage. The review identified **8 critical issues** - **4 have been fixed** as part of the Energy/Evolution Stack refactor. The remaining 4 critical issues and several medium-priority gaps still need attention.
+The core engine has a solid foundation with good separation of concerns, comprehensive documentation, and thorough test coverage. The review identified **8 critical issues** - **5 have been fixed** as part of the Energy/Evolution Stack refactor and CardDefinition validation work. The remaining 3 critical issues and several medium-priority gaps still need attention.
 
-**Fixed:** #1, #5, #7, #8  
+**Fixed:** #1, #2, #5, #7, #8  
-**Still Open:** #2 (model validation), #3 (end_turn knockouts), #4 (win condition timing), #6 (engine knockout processing)
+**Still Open:** #3 (end_turn knockouts), #4 (win condition timing), #6 (engine knockout processing)
 
 ---
 
@@ -36,16 +36,19 @@ for zone_name in ["deck", "hand", "active", "bench", "discard", "prizes", "energ
 
 ---
 
-### 2. No Validation That Pokemon Cards Have Required Fields
+### 2. ~~No Validation That Pokemon Cards Have Required Fields~~ FIXED
 **File:** `app/core/models/card.py:163-231`  
 **Severity:** CRITICAL  
+**Status:** FIXED in commit TBD
 
-`CardDefinition` accepts Pokemon-specific fields as optional (`hp: int | None`, `stage: PokemonStage | None`), but there's no validation that Pokemon cards actually have these fields set. This could lead to runtime errors.
+Added a Pydantic `model_validator` to `CardDefinition` that enforces:
+- **Pokemon cards** require: `hp` (must be positive), `stage`, `pokemon_type`
+- **Pokemon Stage 1/2** require: `evolves_from`
+- **Pokemon VMAX/VSTAR** require: `evolves_from`
+- **Trainer cards** require: `trainer_type`
+- **Energy cards** require: `energy_type` (auto-fills `energy_provides` if empty)
 
-**Fix:** Add a Pydantic `model_validator` to ensure:
+This prevents invalid card definitions at construction time rather than runtime errors later.
-- If `card_type == POKEMON`, then `hp`, `stage`, and `pokemon_type` must be set
-- If `card_type == TRAINER`, then `trainer_type` must be set
-- If `card_type == ENERGY`, then `energy_type` must be set
 
 ---
 

backend/app/core/models/card.py

@@ -31,7 +31,7 @@ Usage:
 
 from typing import Any
 
-from pydantic import BaseModel, Field
+from pydantic import BaseModel, Field, model_validator
 
 from app.core.models.enums import (
     CardType,
@@ -230,6 +230,54 @@ class CardDefinition(BaseModel):
     set_id: str = ""
     image_url: str | None = None
 
+    @model_validator(mode="after")
+    def validate_card_type_fields(self) -> "CardDefinition":
+        """Validate that required fields are set based on card_type.
+
+        Ensures that:
+        - Pokemon cards have hp, stage, and pokemon_type
+        - Stage 1/2 and VMAX/VSTAR Pokemon have evolves_from
+        - Trainer cards have trainer_type
+        - Energy cards have energy_type (auto-fills energy_provides if empty)
+
+        Raises:
+            ValueError: If required fields are missing for the card type.
+        """
+        if self.card_type == CardType.POKEMON:
+            if self.hp is None:
+                raise ValueError("Pokemon cards must have 'hp' set")
+            if self.hp <= 0:
+                raise ValueError("Pokemon 'hp' must be positive")
+            if self.stage is None:
+                raise ValueError("Pokemon cards must have 'stage' set")
+            if self.pokemon_type is None:
+                raise ValueError("Pokemon cards must have 'pokemon_type' set")
+
+            # Evolution chain validation
+            if (
+                self.stage in (PokemonStage.STAGE_1, PokemonStage.STAGE_2)
+                and self.evolves_from is None
+            ):
+                raise ValueError(f"{self.stage.value} Pokemon must have 'evolves_from' set")
+            if (
+                self.variant in (PokemonVariant.VMAX, PokemonVariant.VSTAR)
+                and self.evolves_from is None
+            ):
+                raise ValueError(f"{self.variant.value} Pokemon must have 'evolves_from' set")
+
+        elif self.card_type == CardType.TRAINER:
+            if self.trainer_type is None:
+                raise ValueError("Trainer cards must have 'trainer_type' set")
+
+        elif self.card_type == CardType.ENERGY:
+            if self.energy_type is None:
+                raise ValueError("Energy cards must have 'energy_type' set")
+            # Auto-fill energy_provides if empty
+            if not self.energy_provides:
+                self.energy_provides = [self.energy_type]
+
+        return self
+
     def is_pokemon(self) -> bool:
         """Check if this is a Pokemon card."""
         return self.card_type == CardType.POKEMON

backend/tests/core/conftest.py

@@ -414,6 +414,7 @@ def double_colorless_energy_def() -> CardDefinition:
         id="dce_001",
         name="Double Colorless Energy",
         card_type=CardType.ENERGY,
+        energy_type=EnergyType.COLORLESS,
         energy_provides=[EnergyType.COLORLESS, EnergyType.COLORLESS],
         rarity="uncommon",
         set_id="base",

backend/tests/core/test_effects/test_handlers.py

@@ -666,50 +666,24 @@ class TestAttackDamage:
         assert not result.success
         assert "must be positive" in result.message
 
-    def test_skips_weakness_when_source_has_no_type(
+    def test_validation_prevents_pokemon_without_type(self) -> None:
-        self, game_state: GameState, rng: SeededRandom
-    ) -> None:
         """
-        Verify attack_damage skips weakness/resistance when source has no pokemon_type.
+        Verify CardDefinition validation prevents creating Pokemon without pokemon_type.
 
-        Some cards (like certain trainer-summoned tokens) might not have a type.
+        This ensures all Pokemon in the game have a type, which is required for
-        The damage should still be dealt, just without weakness/resistance calculation.
+        proper weakness/resistance calculations. The attack_damage handler still
+        has defensive code for edge cases (e.g., missing registry entries), but
+        CardDefinition validation now prevents typeless Pokemon at creation time.
         """
-        # Make source Pokemon have no type
+        with pytest.raises(ValueError, match="pokemon_type"):
-        game_state.card_registry["pikachu-001"] = CardDefinition(
+            CardDefinition(
-            id="pikachu-001",
+                id="typeless-pokemon",
-            name="Pikachu",
+                name="Typeless",
-            card_type=CardType.POKEMON,
+                card_type=CardType.POKEMON,
-            stage=PokemonStage.BASIC,
+                stage=PokemonStage.BASIC,
-            hp=60,
+                hp=60,
-            pokemon_type=None,  # No type!
+                pokemon_type=None,  # Should fail validation
-        )
+            )
-
-        # Give target weakness (which won't apply since source has no type)
-        game_state.card_registry["charmander-001"] = CardDefinition(
-            id="charmander-001",
-            name="Charmander",
-            card_type=CardType.POKEMON,
-            stage=PokemonStage.BASIC,
-            hp=70,
-            pokemon_type=EnergyType.FIRE,
-            weakness=WeaknessResistance(energy_type=EnergyType.LIGHTNING),
-        )
-
-        ctx = make_context(
-            game_state,
-            rng,
-            source_card_id="pikachu-inst",
-            params={"amount": 30},
-        )
-
-        result = resolve_effect("attack_damage", ctx)
-
-        assert result.success
-        target = game_state.players["player2"].get_active_pokemon()
-        assert target is not None
-        assert target.damage == 30  # No weakness applied, just base damage
-        assert "weakness" not in result.details  # No weakness in details
 
     def test_skips_weakness_when_source_definition_missing(
         self, game_state: GameState, rng: SeededRandom

backend/tests/core/test_engine.py

@@ -63,6 +63,7 @@ def basic_pokemon_def() -> CardDefinition:
         stage=PokemonStage.BASIC,
         variant=PokemonVariant.NORMAL,
         hp=60,
+        pokemon_type=EnergyType.LIGHTNING,
         attacks=[
             Attack(
                 name="Thunder Shock",
@@ -84,7 +85,8 @@ def strong_pokemon_def() -> CardDefinition:
         stage=PokemonStage.STAGE_1,
         variant=PokemonVariant.NORMAL,
         hp=100,
-        evolves_from="pikachu-001",
+        pokemon_type=EnergyType.LIGHTNING,
+        evolves_from="Pikachu",
         attacks=[
             Attack(
                 name="Thunder",
@@ -1083,6 +1085,7 @@ class TestEvolvePokemonAction:
             stage=PokemonStage.STAGE_1,
             variant=PokemonVariant.NORMAL,
             hp=100,
+            pokemon_type=EnergyType.LIGHTNING,
             evolves_from=basic_pokemon_def.name,  # Must match the Pokemon's name
             attacks=[
                 Attack(
@@ -1446,6 +1449,7 @@ class TestUseAbilityAction:
             stage=PokemonStage.BASIC,
             variant=PokemonVariant.NORMAL,
             hp=60,
+            pokemon_type=EnergyType.LIGHTNING,
             abilities=[
                 Ability(
                     name="Static",

backend/tests/core/test_evolution_stack.py

@@ -32,6 +32,7 @@ from app.core.models.enums import (
     PokemonStage,
     PokemonVariant,
     StatusCondition,
+    TrainerType,
     TurnPhase,
 )
 from app.core.models.game_state import GameState, PlayerState
@@ -123,7 +124,7 @@ def tool_def() -> CardDefinition:
         id="choice-band-001",
         name="Choice Band",
         card_type=CardType.TRAINER,
-        trainer_type="tool",
+        trainer_type=TrainerType.TOOL,
         effect_id="modify_damage",
         effect_params={"amount": 30, "condition": "vs_gx_ex"},
         effect_description="+30 damage against GX/EX",

backend/tests/core/test_models/test_card.py

@@ -442,6 +442,10 @@ class TestCardDefinitionPokemon:
         ]
 
         for variant, expected_points in variants_and_points:
+            # VMAX and VSTAR require evolves_from
+            evolves_from = (
+                "Pikachu V" if variant in (PokemonVariant.VMAX, PokemonVariant.VSTAR) else None
+            )
             card = CardDefinition(
                 id=f"test_{variant.value}",
                 name="Test Pokemon",
@@ -449,6 +453,8 @@ class TestCardDefinitionPokemon:
                 stage=PokemonStage.BASIC,
                 variant=variant,
                 hp=100,
+                pokemon_type=EnergyType.COLORLESS,
+                evolves_from=evolves_from,
             )
             assert card.knockout_points() == expected_points, f"Failed for {variant}"
 
@@ -466,6 +472,8 @@ class TestCardDefinitionPokemon:
             stage=PokemonStage.BASIC,
             variant=PokemonVariant.VMAX,
             hp=300,
+            pokemon_type=EnergyType.LIGHTNING,
+            evolves_from="Pikachu V",
         )
         assert vmax.requires_evolution_from_variant() is True
 
@@ -477,6 +485,8 @@ class TestCardDefinitionPokemon:
             stage=PokemonStage.BASIC,
             variant=PokemonVariant.VSTAR,
             hp=250,
+            pokemon_type=EnergyType.LIGHTNING,
+            evolves_from="Pikachu V",
         )
         assert vstar.requires_evolution_from_variant() is True
 
@@ -494,6 +504,7 @@ class TestCardDefinitionPokemon:
                 stage=PokemonStage.BASIC,
                 variant=variant,
                 hp=100,
+                pokemon_type=EnergyType.COLORLESS,
             )
             assert card.requires_evolution_from_variant() is False, f"Failed for {variant}"
 
@@ -511,6 +522,7 @@ class TestCardDefinitionPokemon:
             stage=PokemonStage.BASIC,
             variant=PokemonVariant.NORMAL,
             hp=50,
+            pokemon_type=EnergyType.COLORLESS,
         )
         basic_ex = CardDefinition(
             id="test",
@@ -519,6 +531,7 @@ class TestCardDefinitionPokemon:
             stage=PokemonStage.BASIC,
             variant=PokemonVariant.EX,
             hp=150,
+            pokemon_type=EnergyType.COLORLESS,
         )
         assert basic_normal.is_basic_pokemon() is True
         assert basic_ex.is_basic_pokemon() is True
@@ -531,6 +544,8 @@ class TestCardDefinitionPokemon:
             stage=PokemonStage.STAGE_1,
             variant=PokemonVariant.NORMAL,
             hp=80,
+            pokemon_type=EnergyType.COLORLESS,
+            evolves_from="Basic",
         )
         stage1_gx = CardDefinition(
             id="test",
@@ -539,6 +554,8 @@ class TestCardDefinitionPokemon:
             stage=PokemonStage.STAGE_1,
             variant=PokemonVariant.GX,
             hp=200,
+            pokemon_type=EnergyType.COLORLESS,
+            evolves_from="Basic",
         )
         assert stage1_normal.is_basic_pokemon() is False
         assert stage1_gx.is_basic_pokemon() is False
@@ -553,6 +570,7 @@ class TestCardDefinitionPokemon:
             card_type=CardType.POKEMON,
             stage=PokemonStage.BASIC,
             hp=50,
+            pokemon_type=EnergyType.COLORLESS,
         )
         assert pokemon.variant == PokemonVariant.NORMAL
 
@@ -649,6 +667,7 @@ class TestCardDefinitionEnergy:
             id="rainbow_energy_001",
             name="Rainbow Energy",
             card_type=CardType.ENERGY,
+            energy_type=EnergyType.COLORLESS,  # Base type for special energy
             energy_provides=list(EnergyType),  # Provides all types
             effect_id="rainbow_damage",
             effect_params={"damage_on_attach": 10},
@@ -761,12 +780,19 @@ class TestCardDefinitionHelpers:
     def test_is_pokemon(self) -> None:
         """Verify is_pokemon() returns correct value."""
         pokemon = CardDefinition(
-            id="test", name="Test", card_type=CardType.POKEMON, stage=PokemonStage.BASIC
+            id="test",
+            name="Test",
+            card_type=CardType.POKEMON,
+            stage=PokemonStage.BASIC,
+            hp=60,
+            pokemon_type=EnergyType.COLORLESS,
         )
         trainer = CardDefinition(
             id="test", name="Test", card_type=CardType.TRAINER, trainer_type=TrainerType.ITEM
         )
-        energy = CardDefinition(id="test", name="Test", card_type=CardType.ENERGY)
+        energy = CardDefinition(
+            id="test", name="Test", card_type=CardType.ENERGY, energy_type=EnergyType.COLORLESS
+        )
 
         assert pokemon.is_pokemon() is True
         assert trainer.is_pokemon() is False
@@ -775,7 +801,12 @@ class TestCardDefinitionHelpers:
     def test_is_trainer(self) -> None:
         """Verify is_trainer() returns correct value."""
         pokemon = CardDefinition(
-            id="test", name="Test", card_type=CardType.POKEMON, stage=PokemonStage.BASIC
+            id="test",
+            name="Test",
+            card_type=CardType.POKEMON,
+            stage=PokemonStage.BASIC,
+            hp=60,
+            pokemon_type=EnergyType.COLORLESS,
         )
         trainer = CardDefinition(
             id="test", name="Test", card_type=CardType.TRAINER, trainer_type=TrainerType.ITEM
@@ -786,7 +817,9 @@ class TestCardDefinitionHelpers:
 
     def test_is_energy(self) -> None:
         """Verify is_energy() returns correct value."""
-        energy = CardDefinition(id="test", name="Test", card_type=CardType.ENERGY)
+        energy = CardDefinition(
+            id="test", name="Test", card_type=CardType.ENERGY, energy_type=EnergyType.COLORLESS
+        )
 
         assert energy.is_energy() is True
 

backend/tests/core/test_turn_manager.py

@@ -50,6 +50,7 @@ def basic_pokemon_def() -> CardDefinition:
         stage=PokemonStage.BASIC,
         variant=PokemonVariant.NORMAL,
         hp=60,
+        pokemon_type=EnergyType.LIGHTNING,
     )
 
 
@@ -63,6 +64,7 @@ def ex_pokemon_def() -> CardDefinition:
         stage=PokemonStage.BASIC,
         variant=PokemonVariant.EX,
         hp=120,
+        pokemon_type=EnergyType.LIGHTNING,
     )
 
 

backend/tests/core/test_visibility.py

@@ -22,6 +22,7 @@ from app.core.models.enums import (
     GameEndReason,
     PokemonStage,
     PokemonVariant,
+    TrainerType,
     TurnPhase,
 )
 from app.core.models.game_state import ForcedAction, GameState, PlayerState
@@ -46,6 +47,7 @@ def pokemon_def() -> CardDefinition:
         stage=PokemonStage.BASIC,
         variant=PokemonVariant.NORMAL,
         hp=60,
+        pokemon_type=EnergyType.LIGHTNING,
     )
 
 
@@ -67,6 +69,7 @@ def trainer_def() -> CardDefinition:
         id="potion-001",
         name="Potion",
         card_type=CardType.TRAINER,
+        trainer_type=TrainerType.ITEM,
     )