feat: add comprehensive validation and error handling to TreeGrid

Add robust input validation and improve error messages for TreeGrid:

- Validate invalid child references with descriptive error messages
- Add input validation in __init__ for:
  - Grid dimensions (must be positive integers)
  - node_class (must be TreeNode subclass)
  - tree_structure (cannot be empty)
  - Node positions (must be within grid bounds)
  - Required 'position' key in node data
- Extract magic numbers to class constants:
  - CONNECTION_CURVE_RADIUS = 0.15
  - CONNECTION_LINE_WIDTH = 2
  - CONNECTION_LINE_COLOR = "black"
- Add 6 comprehensive validation tests with realistic retail data:
  - test_invalid_child_reference
  - test_invalid_grid_dimensions
  - test_invalid_node_class
  - test_empty_tree_structure
  - test_missing_position_key
  - test_out_of_bounds_position

All 33 tests passing with 82% coverage (improved from 79%).

Addresses code review issues #1, #2, #4, and #5 from PR #357.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

Author: mvanwyk

pyretailscience/plots/tree_diagram.py

@@ -494,6 +494,11 @@ def render(self, ax: Axes) -> None:
 class TreeGrid:
     """Grid-based tree diagram renderer with configurable node types."""
 
+    # Connection styling constants
+    CONNECTION_CURVE_RADIUS = 0.15
+    CONNECTION_LINE_WIDTH = 2
+    CONNECTION_LINE_COLOR = "black"
+
     def __init__(
         self,
         tree_structure: dict[str, dict],
@@ -516,7 +521,26 @@ def __init__(
             horizontal_spacing: Horizontal spacing between columns. If None, automatically calculated as
                 node_width - 1.0 overlap for compact layout.
 
+        Raises:
+            ValueError: If grid dimensions are not positive, if tree_structure is empty,
+                or if node positions are out of bounds.
+            TypeError: If node_class is not a TreeNode subclass.
+
         """
+        # Validate grid dimensions
+        if num_rows <= 0 or num_cols <= 0:
+            error_msg = f"Grid dimensions must be positive: num_rows={num_rows}, num_cols={num_cols}"
+            raise ValueError(error_msg)
+
+        # Validate node_class is a TreeNode subclass
+        if not issubclass(node_class, TreeNode):
+            error_msg = f"node_class must be a TreeNode subclass, got {node_class}"
+            raise TypeError(error_msg)
+
+        # Validate tree_structure is not empty
+        if not tree_structure:
+            raise ValueError("tree_structure cannot be empty")
+
         self.tree_structure = tree_structure
         self.num_rows = num_rows
         self.num_cols = num_cols
@@ -530,6 +554,20 @@ def __init__(
         self.vertical_spacing = vertical_spacing if vertical_spacing is not None else self.node_height + 0.6
         self.horizontal_spacing = horizontal_spacing if horizontal_spacing is not None else self.node_width - 1.0
 
+        # Validate positions are within grid bounds
+        for node_id, node_data in tree_structure.items():
+            if "position" not in node_data:
+                error_msg = f"Node '{node_id}' is missing required 'position' key"
+                raise ValueError(error_msg)
+
+            col_idx, row_idx = node_data["position"]
+            if not (0 <= col_idx < num_cols):
+                error_msg = f"Node '{node_id}' column index {col_idx} is out of bounds [0, {num_cols})"
+                raise ValueError(error_msg)
+            if not (0 <= row_idx < num_rows):
+                error_msg = f"Node '{node_id}' row index {row_idx} is out of bounds [0, {num_rows})"
+                raise ValueError(error_msg)
+
         # Generate row and column positions
         self.row = {i: i * self.vertical_spacing for i in range(num_rows)}
         self.col = {i: i * self.horizontal_spacing for i in range(num_cols)}
@@ -585,6 +623,13 @@ def render(self, ax: Axes | None = None) -> Axes:
             if "children" in node_data and len(node_data["children"]) > 0:
                 parent = node_centers[node_id]
                 for child_id in node_data["children"]:
+                    if child_id not in node_centers:
+                        available_nodes = list(node_centers.keys())
+                        error_msg = (
+                            f"Child node '{child_id}' referenced by '{node_id}' not found in tree_structure. "
+                            f"Available nodes: {available_nodes}"
+                        )
+                        raise ValueError(error_msg)
                     child = node_centers[child_id]
                     self._draw_connection(
                         ax=ax,
@@ -633,10 +678,10 @@ def _draw_connection(ax: Axes, x1: float, y1: float, x2: float, y2: float) -> No
             y2: Y-coordinate of second point.
 
         """
-        # Connection styling constants
-        curve_radius = 0.15
-        line_width = 2
-        line_color = "black"
+        # Use class constants for connection styling
+        curve_radius = TreeGrid.CONNECTION_CURVE_RADIUS
+        line_width = TreeGrid.CONNECTION_LINE_WIDTH
+        line_color = TreeGrid.CONNECTION_LINE_COLOR
 
         mid_y = (y1 + y2) / 2
         curve_sign = 1 if x2 > x1 else -1

tests/plots/test_tree_diagram.py

@@ -466,3 +466,151 @@ def test_wide_tree(self):
         num_connections = 5
         expected_patches = num_nodes * patches_per_node + num_connections
         assert len(ax.patches) == expected_patches
+
+    def test_invalid_child_reference(self):
+        """Test that referencing a non-existent child raises ValueError."""
+        tree_structure = {
+            "total_revenue": {
+                "header": "Total Revenue",
+                "percent": 15.3,
+                "value1": "£1.2M",
+                "value2": "£1.04M",
+                "position": (0, 0),
+                "children": ["nonexistent_child"],
+            },
+        }
+
+        grid = TreeGrid(
+            tree_structure=tree_structure,
+            num_rows=1,
+            num_cols=1,
+            node_class=SimpleTreeNode,
+        )
+
+        with pytest.raises(ValueError, match="not found in tree_structure"):
+            grid.render()
+
+    def test_invalid_grid_dimensions(self):
+        """Test that invalid grid dimensions raise ValueError."""
+        tree_structure = {
+            "total_revenue": {
+                "header": "Total Revenue",
+                "percent": 8.5,
+                "value1": "£850K",
+                "value2": "£784K",
+                "position": (0, 0),
+                "children": [],
+            },
+        }
+
+        # Test negative rows
+        with pytest.raises(ValueError, match="Grid dimensions must be positive"):
+            TreeGrid(
+                tree_structure=tree_structure,
+                num_rows=-1,
+                num_cols=1,
+                node_class=SimpleTreeNode,
+            )
+
+        # Test zero columns
+        with pytest.raises(ValueError, match="Grid dimensions must be positive"):
+            TreeGrid(
+                tree_structure=tree_structure,
+                num_rows=1,
+                num_cols=0,
+                node_class=SimpleTreeNode,
+            )
+
+    def test_invalid_node_class(self):
+        """Test that invalid node_class raises TypeError."""
+        tree_structure = {
+            "customer_count": {
+                "header": "Customer Count",
+                "percent": 12.4,
+                "value1": "25,450",
+                "value2": "22,640",
+                "position": (0, 0),
+                "children": [],
+            },
+        }
+
+        with pytest.raises(TypeError, match="must be a TreeNode subclass"):
+            TreeGrid(
+                tree_structure=tree_structure,
+                num_rows=1,
+                num_cols=1,
+                node_class=str,  # Not a TreeNode subclass
+            )
+
+    def test_empty_tree_structure(self):
+        """Test that empty tree_structure raises ValueError."""
+        with pytest.raises(ValueError, match="tree_structure cannot be empty"):
+            TreeGrid(
+                tree_structure={},
+                num_rows=1,
+                num_cols=1,
+                node_class=SimpleTreeNode,
+            )
+
+    def test_missing_position_key(self):
+        """Test that missing position key raises ValueError."""
+        tree_structure = {
+            "avg_basket": {
+                "header": "Average Basket Value",
+                "percent": 6.2,
+                "value1": "£45.80",
+                "value2": "£43.12",
+                # Missing 'position' key
+                "children": [],
+            },
+        }
+
+        with pytest.raises(ValueError, match="missing required 'position' key"):
+            TreeGrid(
+                tree_structure=tree_structure,
+                num_rows=1,
+                num_cols=1,
+                node_class=SimpleTreeNode,
+            )
+
+    def test_out_of_bounds_position(self):
+        """Test that out of bounds positions raise ValueError."""
+        # Test column out of bounds (trying to use column 1 when only column 0 exists)
+        tree_structure = {
+            "transaction_freq": {
+                "header": "Transaction Frequency",
+                "percent": 4.8,
+                "value1": "3.2",
+                "value2": "3.05",
+                "position": (1, 0),  # Column 1 is out of bounds for 1 column grid (0-indexed)
+                "children": [],
+            },
+        }
+
+        with pytest.raises(ValueError, match="column index .* is out of bounds"):
+            TreeGrid(
+                tree_structure=tree_structure,
+                num_rows=1,
+                num_cols=1,
+                node_class=SimpleTreeNode,
+            )
+
+        # Test row out of bounds (trying to use row 1 when only row 0 exists)
+        tree_structure = {
+            "items_per_basket": {
+                "header": "Items per Basket",
+                "percent": -2.3,
+                "value1": "4.8",
+                "value2": "4.9",
+                "position": (0, 1),  # Row 1 is out of bounds for 1 row grid (0-indexed)
+                "children": [],
+            },
+        }
+
+        with pytest.raises(ValueError, match="row index .* is out of bounds"):
+            TreeGrid(
+                tree_structure=tree_structure,
+                num_rows=1,
+                num_cols=1,
+                node_class=SimpleTreeNode,
+            )