Fix review findings: num_ibs bound, index error messages, frozen IndexedFamily

- Add num_ibs <= 1000 check in case_validator (matches num_patches_max)
- Fix _family_attr_error to distinguish invalid index from invalid attr
- Make IndexedFamily a frozen dataclass
- Fix overstated docstring and clarify tags field
- Add iteration consistency and zero-index error tests

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

Author: Spencer Bryngelson

toolchain/mfc/case_validator.py

@@ -560,6 +560,7 @@ def check_ibm(self):
 
         self.prohibit(ib and n <= 0, "Immersed Boundaries do not work in 1D (requires n > 0)")
         self.prohibit(ib and num_ibs <= 0, "num_ibs must be >= 1 when ib is enabled")
+        self.prohibit(ib and num_ibs > 1000, "num_ibs must be <= 1000 (num_patches_max in m_constants.fpp)")
         self.prohibit(not ib and num_ibs > 0, "num_ibs is set, but ib is not enabled")
         self.prohibit(ib_state_wrt and not ib, "ib_state_wrt requires ib to be enabled")
 

toolchain/mfc/params/definitions.py

@@ -1137,9 +1137,9 @@ def _load():
         _r(f"bub_pp%{a}", REAL, {"bubbles"}, math=sym)
 
     # patch_ib (immersed boundaries) — registered as indexed family for O(1) lookup.
-    # max_index is intentionally None (unlimited): the Fortran side allocates
-    # 1:num_ibs and the case_validator enforces num_ibs >= 1 when ib is enabled.
-    # Indices beyond num_ibs are harmless (ignored at runtime).
+    # max_index is None so the parameter registry stays compact (no enumeration).
+    # The Fortran-side upper bound (num_patches_max = 1000 in m_constants.fpp) is
+    # enforced by the case_validator check on num_ibs, not by max_index here.
     _ib_tags = {"ib"}
     _ib_attrs: Dict[str, tuple] = {}
     for a in ["geometry", "moving_ibm"]:

toolchain/mfc/params/registry.py

@@ -76,20 +76,21 @@ def _resolve_family(
     return entry if entry is not None else None
 
 
-@dataclass
+@dataclass(frozen=True)
 class IndexedFamily:
     """
     Template for an indexed parameter family like patch_ib(N)%attr.
 
-    Instead of registering patch_ib(1)%geometry through patch_ib(1000000)%geometry
-    individually (~30M entries), we store one template with the attribute definitions
-    and validate parameter names via pattern matching.
+    Instead of registering every index individually (e.g., patch_ib(1)%geometry
+    through patch_ib(N)%geometry for all attributes), we store one template and
+    validate parameter names via pattern matching.
 
     Attributes:
         base_name: Family prefix (e.g., "patch_ib")
         attrs: Mapping of attribute name to (ParamType, tags) — attribute names
                may include sub-indices like "vel(1)", "angles(3)".
-        tags: Default tags applied to all attributes in this family.
+        tags: Metadata-only tags for this family (not used in resolution;
+              per-attribute tags in ``attrs`` are what get returned).
         max_index: Upper bound on the index (1-based). None = unlimited.
     """
 

toolchain/mfc/params/validate.py

@@ -46,23 +46,37 @@
 
 def _family_attr_error(name: str) -> Optional[str]:
     """
-    Check if name matches a known family base but has an invalid attribute.
+    Diagnose why a family-pattern param was rejected by the registry.
 
-    Returns a targeted error message if so, or None if it doesn't match
+    Distinguishes three cases for known family bases:
+    - Invalid index (0 or exceeding max_index)
+    - Unknown attribute
+    - Unknown family base (returns None to let caller handle)
+
+    Returns a targeted error message, or None if name doesn't match
     any family pattern.
     """
     from .registry import _INDEXED_RE
 
     m = _INDEXED_RE.match(name)
     if m is None:
         return None
-    base, _, attr = m.groups()
+    base, idx_str, attr = m.groups()
     fam = REGISTRY.families.get(base)
     if fam is None:
         return None
-    # Known family, unknown attribute — provide targeted message
+
+    # Check if the problem is the index (attr is valid but index is out of range)
+    idx = int(idx_str)
+    if attr in fam.attrs:
+        if idx < 1:
+            return f"Invalid index {idx} for {base}: indices are 1-based (must be >= 1)"
+        if fam.max_index is not None and idx > fam.max_index:
+            return f"Index {idx} exceeds maximum ({fam.max_index}) for {base}"
+        return None  # Both attr and index look valid; shouldn't reach here
+
+    # Unknown attribute — provide targeted message
     valid = sorted(fam.attrs.keys())
-    # Truncate long lists
     if len(valid) > 8:
         shown = ", ".join(valid[:8]) + f", ... ({len(valid)} total)"
     else:

toolchain/mfc/params_tests/test_registry.py

@@ -168,6 +168,35 @@ def test_get_param_def_none_for_invalid(self):
         self.assertIsNone(reg.get_param_def("thing(1)%bogus"))
         self.assertIsNone(reg.get_param_def("unknown(1)%geom"))
 
+    def test_all_params_iteration_bounded(self):
+        """Iterating all_params should yield scalars + one example per family attr."""
+        reg = ParamRegistry()
+        reg.register(ParamDef(name="scalar1", param_type=ParamType.INT))
+        reg.register(ParamDef(name="scalar2", param_type=ParamType.REAL))
+        reg.register_family(
+            IndexedFamily(
+                base_name="thing",
+                attrs={
+                    "geom": (ParamType.INT, {"tag1"}),
+                    "vel(1)": (ParamType.REAL, {"tag1"}),
+                },
+                tags={"tag1"},
+            )
+        )
+        reg.freeze()
+
+        keys = list(reg.all_params)
+        # 2 scalars + 2 family attrs (one example each at index=1)
+        self.assertEqual(len(reg.all_params), 4)
+        self.assertEqual(len(keys), 4)
+        self.assertIn("scalar1", keys)
+        self.assertIn("scalar2", keys)
+        # Family examples use index=1
+        self.assertIn("thing(1)%geom", keys)
+        self.assertIn("thing(1)%vel(1)", keys)
+        # Arbitrary indices should NOT appear in iteration
+        self.assertNotIn("thing(42)%geom", keys)
+
     def test_all_params_contains_family(self):
         """all_params mapping should resolve family params via __contains__."""
         reg = self._make_registry_with_family()

toolchain/mfc/params_tests/test_validate.py

@@ -94,6 +94,14 @@ def test_family_valid_attr_no_error(self):
         errors = check_unknown_params(params)
         self.assertEqual(errors, [])
 
+    def test_family_zero_index_gives_index_error(self):
+        """Zero index on valid attr should report index error, not attr error."""
+        params = {"patch_ib(0)%geometry": 1}
+        errors = check_unknown_params(params)
+        self.assertEqual(len(errors), 1)
+        self.assertIn("1-based", errors[0])
+        self.assertNotIn("Unknown attribute", errors[0])
+
     @unittest.skipUnless(RAPIDFUZZ_AVAILABLE, "rapidfuzz not installed")
     def test_similar_param_suggests_correction(self):
         """Typo near valid param should suggest 'did you mean?'."""