fix: validate solver inputs and benchmark results

.github/workflows/ci.yml

@@ -24,7 +24,7 @@ jobs:
         pip install uv==0.9.12
         uv sync --python 3.10 --locked --extra quality --extra all --extra mujoco
     - run: |
-        uv run --no-sync ruff check src/cbfkit
+        uv run --no-sync ruff check src/cbfkit scripts tests/test_optimization tests/test_benchmarks
         uv run --no-sync black --check src/cbfkit
         uv run --no-sync isort --check-only src/cbfkit
         uv run --no-sync mypy src/cbfkit

.pre-commit-config.yaml

@@ -26,10 +26,10 @@ repos:
         files: ^(src/cbfkit/|pyproject.toml)
       - id: ruff
         name: ruff
-        entry: uv run --no-sync ruff check src/cbfkit
+        entry: uv run --no-sync ruff check src/cbfkit scripts tests/test_optimization tests/test_benchmarks
         language: system
         pass_filenames: false
-        files: ^(src/cbfkit/|pyproject.toml)
+        files: ^(src/cbfkit/|scripts/|tests/test_optimization/|tests/test_benchmarks/|pyproject.toml)
       - id: mypy
         name: mypy
         entry: uv run --no-sync mypy src/cbfkit

CONTRIBUTING.md

@@ -48,14 +48,18 @@ separate pytest processes and report that limitation.
 ## Linting, formatting and type checks
 
 ```bash
-uv run --no-sync ruff check src/cbfkit
+uv run --no-sync ruff check src/cbfkit scripts tests/test_optimization tests/test_benchmarks
 uv run --no-sync black --check src/cbfkit
 uv run --no-sync isort --check-only src/cbfkit
 uv run --no-sync mypy src/cbfkit
 uv run --no-sync pre-commit run --all-files
 ```
 
-All four source checks are blocking in CI and use the same scope in pre-commit. To apply
+Ruff also checks maintained scripts and the optimization/benchmark tests. Formatting
+and type checks remain scoped to the library; extend those checks to other test
+directories incrementally as they are maintained.
+
+All four checks are blocking in CI and use the same scope in pre-commit. To apply
 formatting, run `uv run --no-sync isort src/cbfkit` followed by
 `uv run --no-sync black src/cbfkit`. Ruff checks unused imports (F401); declare intentional
 re-exports with `__all__` or explicit aliases, and use a local suppression only for import
@@ -140,6 +144,14 @@ under ignored `results/`. Do not compare absolute times across different hardwar
 noisy latency thresholds on shared CI runners. Compare safety failures and constraint
 violations alongside speed; the baseline command fails if either count is nonzero.
 
+For `cbfkit-bench` scenarios, every result must declare `success`, `safety_violations`,
+`solver_failures`, and `avg_step_ms`. Use `None` for an unavailable measurement;
+summaries preserve it as JSON `null`, including when only some seeds have a value.
+Missing required keys, non-finite values, and empty runs are errors. Comparisons,
+optimization objectives, and safety decisions require a measured, finite metric.
+Optional numeric metrics are collected across all records and remain unavailable
+when any seed lacks a measurement.
+
 ## Submitting a change
 
 1. Branch from `main` and keep the change focused. Separate refactors from behaviour changes.

scripts/render_showcase.py

@@ -965,7 +965,6 @@ def render_fixed_wing_3d() -> str:
     """Fixed-wing UAV 3D reach-avoid — load cached pickle or run a short sim."""
     import pickle
 
-    import jax.numpy as jnp
     import matplotlib.pyplot as plt
     import numpy as np
     from matplotlib.animation import FuncAnimation, PillowWriter

src/cbfkit/benchmarks/metrics.py

@@ -4,55 +4,83 @@
 
 __all__ = ["mean", "rate", "summarize"]
 
+import math
 from typing import Mapping, Sequence
 
-Record = Mapping[str, float | int | bool | str]
+Record = Mapping[str, float | int | bool | str | None]
 
 
 def _to_float(value: float | int | bool | str) -> float:
     if isinstance(value, bool):
         return float(value)
     if isinstance(value, (int, float)):
-        return float(value)
-    raise TypeError(f"Non-numeric metric value: {value!r}")
+        result = float(value)
+        if math.isfinite(result):
+            return result
+        raise ValueError(f"Non-finite metric value: {value!r}")
+    raise ValueError(f"Non-numeric metric value: {value!r}")
 
 
-def mean(records: Sequence[Record], key: str) -> float:
-    if not records:
-        return 0.0
-    return sum(_to_float(record.get(key, 0.0)) for record in records) / len(records)
+def metric_value(record: Record, key: str) -> float:
+    """Require a measured, finite metric for comparisons and decisions."""
+    if key not in record:
+        raise ValueError(f"Unknown or missing metric {key!r}.")
+    value = record[key]
+    if value is None:
+        raise ValueError(f"Metric {key!r} is unavailable.")
+    return _to_float(value)
+
 
+def mean(records: Sequence[Record], key: str) -> float | None:
+    """Average a metric; explicit None means the measurement is unavailable.
 
-def rate(records: Sequence[Record], key: str) -> float:
+    Every record must declare the key. Partial measurements produce None,
+    rather than averaging a selected subset or treating absent values as zero.
+    """
+    if not records:
+        raise ValueError("Cannot summarize an empty benchmark run.")
+    values = []
+    for index, record in enumerate(records):
+        if key not in record:
+            raise ValueError(f"Record {index} is missing required metric {key!r}.")
+        value = record[key]
+        values.append(None if value is None else _to_float(value))
+    if any(value is None for value in values):
+        return None
+    return math.fsum(value / len(values) for value in values if value is not None)
+
+
+def rate(records: Sequence[Record], key: str) -> float | None:
     """Alias for mean over binary or count-like key values."""
     return mean(records, key)
 
 
-def summarize(records: Sequence[Record]) -> dict[str, float]:
+def summarize(records: Sequence[Record]) -> dict[str, float | None]:
     """Produce common summary statistics for benchmark outputs.
 
     In addition to the fixed metrics, any extra numeric keys found in the
     records are averaged automatically so that scenario-specific metrics
     (e.g. ``final_goal_distance``, ``time_to_goal``) are carried through
     to the sweep summary without requiring changes here.
     """
-    fixed: dict[str, float] = {
+    fixed: dict[str, float | None] = {
         "num_runs": float(len(records)),
         "success_rate": rate(records, "success"),
         "safety_violation_rate": rate(records, "safety_violations"),
         "solver_failure_rate": rate(records, "solver_failures"),
         "avg_step_ms": mean(records, "avg_step_ms"),
     }
 
-    # Collect extra numeric keys from the first record (all records share the same schema)
+    # Include optional metrics from every record. Missing optional measurements
+    # remain unavailable in the summary instead of being silently discarded.
     fixed_keys = set(fixed.keys()) | {"success", "safety_violations", "solver_failures"}
     extra_keys: set[str] = set()
-    if records:
-        for k, v in records[0].items():
-            if k not in fixed_keys and isinstance(v, (int, float, bool)):
+    for record in records:
+        for k, v in record.items():
+            if k not in fixed_keys and (v is None or isinstance(v, (int, float, bool))):
                 extra_keys.add(k)
 
     for k in sorted(extra_keys):
-        fixed[k] = mean(records, k)
+        fixed[k] = mean([{k: record.get(k)} for record in records], k)
 
     return fixed

src/cbfkit/benchmarks/registry.py

@@ -17,7 +17,7 @@
 from dataclasses import dataclass
 from typing import Any, Callable, Dict, Iterable, Mapping, Optional, Sequence
 
-BenchmarkResult = Mapping[str, float | int | bool | str]
+BenchmarkResult = Mapping[str, float | int | bool | str | None]
 BenchmarkScenario = Callable[[int], BenchmarkResult]
 SweepableRunner = Callable[[int, Dict[str, Any]], BenchmarkResult]
 BatchSweepableRunner = Callable[[Sequence[int], Dict[str, Any]], Sequence[BenchmarkResult]]

src/cbfkit/benchmarks/runner.py

@@ -10,32 +10,37 @@
 from typing import Iterable, Sequence
 
 from ._progress import make_progress
-from .metrics import summarize
+from .metrics import metric_value, summarize
 from .registry import registry
 
 
 @dataclass(frozen=True)
 class BenchmarkRun:
     scenario: str
     seeds: Sequence[int]
-    records: list[dict[str, float | int | bool | str]]
-    summary: dict[str, float]
+    records: list[dict[str, float | int | bool | str | None]]
+    summary: dict[str, float | None]
 
 
 def _parse_seeds(seeds: str | Iterable[int]) -> list[int]:
     if isinstance(seeds, str):
         if ":" in seeds:
             start, end = seeds.split(":", maxsplit=1)
-            return list(range(int(start), int(end) + 1))
-        return [int(token) for token in seeds.split(",") if token]
-    return [int(seed) for seed in seeds]
+            parsed = list(range(int(start), int(end) + 1))
+        else:
+            parsed = [int(token) for token in seeds.split(",")]
+    else:
+        parsed = [int(seed) for seed in seeds]
+    if not parsed:
+        raise ValueError("A benchmark run requires at least one seed.")
+    return parsed
 
 
 def run_scenario(name: str, seeds: str | Iterable[int]) -> BenchmarkRun:
     parsed_seeds = _parse_seeds(seeds)
     spec = registry.scenario(name)
 
-    records: list[dict[str, float | int | bool | str]] = []
+    records: list[dict[str, float | int | bool | str | None]] = []
     with make_progress() as progress:
         task = progress.add_task(f"Seeds ({name})", total=len(parsed_seeds))
         for seed in parsed_seeds:
@@ -62,7 +67,9 @@ def write_artifacts(run: BenchmarkRun, output_dir: str | Path) -> None:
         "summary": run.summary,
         "records": run.records,
     }
-    (out / "results.json").write_text(json.dumps(payload, indent=2), encoding="utf-8")
+    (out / "results.json").write_text(
+        json.dumps(payload, indent=2, allow_nan=False), encoding="utf-8"
+    )
 
     keys = sorted({key for rec in run.records for key in rec.keys()})
     with (out / "records.csv").open("w", newline="", encoding="utf-8") as handle:
@@ -76,8 +83,8 @@ def compare_runs(
     right: BenchmarkRun,
     metric: str,
 ) -> dict[str, float]:
-    left_value = float(left.summary.get(metric, 0.0))
-    right_value = float(right.summary.get(metric, 0.0))
+    left_value = metric_value(left.summary, metric)
+    right_value = metric_value(right.summary, metric)
     return {
         "left": left_value,
         "right": right_value,

src/cbfkit/benchmarks/solver_stress.py

@@ -128,9 +128,9 @@ def nominal_wrapper(t, x, k, ref):
         if k.endswith("solver_status"):
             status_key = k
 
-    avg_iter = 0.0
-    max_iter = 0.0
-    failures = 0
+    avg_iter = None
+    max_iter = None
+    failures = None
 
     if iter_key and len(values) > 0:
         idx = keys.index(iter_key)
@@ -153,6 +153,8 @@ def nominal_wrapper(t, x, k, ref):
         "avg_solver_iter": avg_iter,
         "max_solver_iter": max_iter,
         "solver_failures": failures,
-        "success": int(failures == 0),  # Simple success definition
+        "success": None if failures is None else int(failures == 0),
+        # This scenario measures solver behavior, not barrier violations.
+        "safety_violations": None,
         "final_dist": float(jnp.linalg.norm(results.states[-1, :2] - jnp.array([10.0, 10.0]))),
     }

src/cbfkit/benchmarks/sweep.py

@@ -29,7 +29,7 @@
 
 from ._progress import console as _console
 from ._progress import make_progress as _make_progress
-from .metrics import summarize
+from .metrics import metric_value, summarize
 from .registry import BatchSweepableRunner, SweepableRunner
 from .sweep_viz import SweepViz
 
@@ -121,10 +121,7 @@ def _resolve_falsifier_kwargs(
 
 def _is_failure(result: dict[str, Any], metric: str) -> bool:
     """Check if a result indicates a failure."""
-    val = result.get(metric, 0)
-    if isinstance(val, bool):
-        return val
-    return float(val) > 0
+    return metric_value(result, metric) > 0
 
 
 def _run_combo(
@@ -158,6 +155,8 @@ def _run_combo(
     # Use batch runner when available and falsifier is off
     if batch_runner is not None and not falsifier:
         batch_results = batch_runner(seeds, combo)
+        if len(batch_results) != len(seeds):
+            raise ValueError("Batch runner must return one result per requested seed.")
         for seed, result in zip(seeds, batch_results):
             result = dict(result)
             result["seed"] = seed
@@ -298,13 +297,19 @@ def run_sweep(
         Optional live visualization.  When provided the sweep renders a
         colour-coded results table and scatter plot in the terminal.
     """
+    if not seeds:
+        raise ValueError("A benchmark sweep requires at least one seed.")
+
     falsifier, falsifier_metric = _resolve_falsifier_kwargs(
         falsifier,
         falsifier_metric,
         skip_on_failure,
         failure_metric,
     )
 
+    if not param_combos:
+        raise ValueError("A benchmark sweep requires at least one parameter combination.")
+
     records: list[dict[str, Any]] = []
     per_combo_summaries: list[dict[str, Any]] = []
     skipped = 0
@@ -437,13 +442,19 @@ def run_optuna_sweep(
 
     Requires ``pip install cbfkit[optuna]``.
     """
+    if not seeds:
+        raise ValueError("A benchmark sweep requires at least one seed.")
+
     falsifier, falsifier_metric = _resolve_falsifier_kwargs(
         falsifier,
         falsifier_metric,
         skip_on_failure,
         failure_metric,
     )
 
+    if n_trials <= 0:
+        raise ValueError("An Optuna sweep requires at least one trial.")
+
     try:
         import optuna
     except ImportError as exc:
@@ -490,10 +501,10 @@ def objective(trial) -> float:
         )
 
         summary = per_combo_summaries[-1]
-        obj_val = summary.get(objective_metric, 0.0)
+        obj_val = metric_value(summary, objective_metric)
         if safety_constraint is not None:
             sc_metric, sc_max = safety_constraint
-            if summary.get(sc_metric, 0.0) > sc_max:
+            if metric_value(summary, sc_metric) > sc_max:
                 obj_val = float("inf")
 
         if viz is not None: