feat(cli)!: reject duplicate keys in the eval config on every entry point

docs/guide/configuration.md

@@ -4,6 +4,9 @@
 
 SiEval uses YAML files for batch evaluation. A config defines models, datasets, tasks, and runner options.
 
+A key repeated within one mapping is an error on every entry point, before
+anything starts. Keys a merge (`<<: *anchor`) brings in may still be overridden.
+
 ```yaml
 result_dir: "./outputs/my-eval"
 

sieval/cli/_config_yaml.py

@@ -0,0 +1,68 @@
+"""The one reader of an evaluation config file.
+
+Every entry point loads the config through :func:`load_eval_config`, which
+refuses a key repeated within one mapping instead of keeping the last value.
+
+AI-Generated Code - Claude Opus 5.5 (Anthropic)
+"""
+
+from pathlib import Path
+
+import yaml
+
+_MERGE_TAG = "tag:yaml.org,2002:merge"
+
+
+class DuplicateKeyError(ValueError):
+    """The config repeats a key within one mapping."""
+
+
+class _EvalConfigLoader(yaml.SafeLoader):
+    """``SafeLoader`` that records each repeated key; one instance per load."""
+
+    def __init__(self, stream: str) -> None:
+        super().__init__(stream)
+        self.duplicates: list[str] = []
+        self._checked: set[int] = set()
+
+    def flatten_mapping(self, node):
+        # flatten_mapping rewrites node.value in place, merge sources included,
+        # so check each node once, before that: keys a ``<<`` brings in may be
+        # overridden.
+        if id(node) not in self._checked:
+            self._checked.add(id(node))
+            first_line: dict[object, int] = {}
+            for key_node, _ in node.value:
+                if key_node.tag == _MERGE_TAG:
+                    continue
+                key = self.construct_object(key_node, deep=False)
+                line = key_node.start_mark.line + 1
+                try:
+                    seen = key in first_line
+                except TypeError:
+                    continue  # unhashable; SafeLoader refuses it itself
+                if seen:
+                    self.duplicates.append(
+                        f"duplicate key {key!r} at line {line} "
+                        f"(first at line {first_line[key]})"
+                    )
+                else:
+                    first_line[key] = line
+        super().flatten_mapping(node)
+
+
+def load_eval_config(path: str | Path) -> object:
+    """Parse the evaluation config at *path*, as ``yaml.safe_load`` would.
+
+    Raises:
+        DuplicateKeyError: a mapping repeats a key; names every repeat.
+        yaml.YAMLError: the file is not valid YAML.
+    """
+    loader = _EvalConfigLoader(Path(path).read_text(encoding="utf-8"))
+    try:
+        data = loader.get_single_data()
+    finally:
+        loader.dispose()
+    if loader.duplicates:
+        raise DuplicateKeyError(f"{path}: " + "; ".join(loader.duplicates))
+    return data

sieval/cli/run.py

@@ -25,6 +25,7 @@
     CliModel,
     apply_cli_model,
 )
+from sieval.cli._config_yaml import DuplicateKeyError, load_eval_config
 from sieval.cli._resume_option import (
     ResumeCommand,
     ResumeOption,
@@ -833,10 +834,13 @@ def unplanned(result: dict[str, object]) -> dict[str, object]:
     if not config_path.exists():
         return unplanned(checks())
 
-    with open(config_path) as stream:
-        loaded = yaml.safe_load(stream)
-    if not isinstance(loaded, dict):
+    try:
+        parsed = load_eval_config(config_path)
+    except (yaml.YAMLError, DuplicateKeyError):
+        return unplanned(checks())  # reported as the failed yaml_syntax check
+    if not isinstance(parsed, dict):
         raise ValueError("evaluation config must be a mapping")
+    loaded = cast(dict, parsed)
     try:
         apply_cli_model(loaded, cli_model, model)
     except ValueError:
@@ -1148,10 +1152,10 @@ async def _run_all(
     """
     configure_logging(verbose)
 
-    with open(config_path) as f:
-        config = yaml.safe_load(f)
-    if not isinstance(config, dict):
+    parsed = load_eval_config(config_path)
+    if not isinstance(parsed, dict):
         raise ValueError("evaluation config must be a mapping")
+    config = cast(dict, parsed)
     apply_cli_model(config, cli_model, model)
 
     # Path-only models skip resolve_infer_config (which reads YAML

sieval/cli/session.py

@@ -28,6 +28,7 @@
 
 from sieval import __version__
 from sieval.cli._cli_model import CliModel, apply_cli_model
+from sieval.cli._config_yaml import load_eval_config
 from sieval.cli._filter_spec import (
     VALUES_DIGEST_KEY,
     check_arg_names,
@@ -1753,12 +1754,12 @@ def __init__(
             invocation if invocation is not None else shlex.join(sys.argv)
         )
 
-        with open(self.config_path, encoding="utf-8") as f:
-            loaded_config: RootConfigDict | None = yaml.safe_load(f)
-            if loaded_config is None:
-                loaded_config = {}
-            if not isinstance(loaded_config, dict):
-                raise ValueError("Top-level YAML config must be a dictionary")
+        parsed = load_eval_config(self.config_path)
+        if parsed is None:
+            parsed = {}
+        if not isinstance(parsed, dict):
+            raise ValueError("Top-level YAML config must be a dictionary")
+        loaded_config = cast(RootConfigDict, parsed)
         if "alignment" in loaded_config:
             logger.warning(
                 "{}: the `alignment:` block was removed in 0.11 and is ignored; "

sieval/cli/validation.py

@@ -11,14 +11,15 @@
 AI-Generated Code - Claude Opus 4.6 (Anthropic)
 """
 
-from collections.abc import Callable, Mapping
+from collections.abc import Mapping
 from dataclasses import dataclass, field
 from pathlib import Path
 from typing import Any, NotRequired, TypedDict, cast
 
 import yaml
 
 from sieval.cli._cli_model import CliModel, apply_cli_model
+from sieval.cli._config_yaml import DuplicateKeyError, load_eval_config
 from sieval.cli._filter_spec import (
     check_arg_names,
     check_by,
@@ -99,42 +100,6 @@ def merge(self, other: "ValidationResult") -> None:
         self.warnings.extend(other.warnings)
 
 
-def _make_duplicate_key_checker(warnings: list[str]) -> Callable[..., dict]:
-    """Create a construct_mapping override that records duplicate keys."""
-
-    original = yaml.SafeLoader.construct_mapping
-
-    def construct_mapping(loader, node, deep=False):
-        loader.flatten_mapping(node)
-        seen: set[str] = set()
-        for key_node, _value_node in node.value:
-            key = loader.construct_object(key_node, deep=False)
-            if isinstance(key, str) and key in seen:
-                mark = key_node.start_mark
-                warnings.append(f"Duplicate key '{key}' at line {mark.line + 1}")
-            if isinstance(key, str):
-                seen.add(key)
-        return original(loader, node, deep=deep)
-
-    return construct_mapping
-
-
-def load_yaml_with_duplicate_check(content: str) -> tuple[dict, list[str]]:
-    """Parse YAML content, returning (parsed_dict, duplicate_key_warnings).
-
-    Preserves last-wins behavior (consistent with ``yaml.safe_load``).
-    """
-    warnings: list[str] = []
-    checker = _make_duplicate_key_checker(warnings)
-    # Per-call subclass to avoid shared class-level state (thread-safe)
-    loader_cls = type("_DuplicateKeyLoader", (yaml.SafeLoader,), {})
-    cast(Any, loader_cls).construct_mapping = checker
-    data = yaml.load(content, Loader=loader_cls)
-    if data is None:
-        data = {}
-    return data, warnings
-
-
 def _validate_structure(cfg: dict, result: ValidationResult) -> bool:
     """Validate top-level structure. Returns False if too broken to continue."""
     for key in cfg:
@@ -1084,29 +1049,24 @@ def run_dry_run(
 
     checks.append({"name": "file_exists", "ok": True})
 
-    # 2. YAML parse + duplicate key check
-    content = config.read_text(encoding="utf-8")
+    # 2. YAML parse; a repeated key is an error
     try:
-        cfg, dup_warnings = load_yaml_with_duplicate_check(content)
-    except yaml.YAMLError as exc:
+        parsed = load_eval_config(config)
+    except (yaml.YAMLError, DuplicateKeyError) as exc:
+        detail = (
+            str(exc)
+            if isinstance(exc, DuplicateKeyError)
+            else f"YAML syntax error: {exc}"
+        )
         checks.append(
-            {
-                "name": "yaml_syntax",
-                "ok": False,
-                "detail": f"YAML syntax error: {exc}",
-                "warnings": [],
-            }
+            {"name": "yaml_syntax", "ok": False, "detail": detail, "warnings": []}
         )
         n_err = sum(1 for c in checks if not c["ok"])
         return {"checks": checks, "n_errors": n_err, "n_warnings": 0}
+    # A non-mapping is refused by the cli_model and schema checks below.
+    cfg = {} if parsed is None else cast(dict, parsed)
 
-    checks.append(
-        {
-            "name": "yaml_syntax",
-            "ok": True,
-            "warnings": list(dup_warnings),
-        }
-    )
+    checks.append({"name": "yaml_syntax", "ok": True, "warnings": []})
 
     # 2b. The models: entry --checkpoint/--api-base becomes
     if cli_model is not None:
@@ -1116,7 +1076,7 @@ def run_dry_run(
             apply_cli_model(cfg, cli_model, model_override)
         except ValueError as exc:
             checks.append({"name": "cli_model", "ok": False, "detail": str(exc)})
-            return {"checks": checks, "n_errors": 1, "n_warnings": len(dup_warnings)}
+            return {"checks": checks, "n_errors": 1, "n_warnings": 0}
         checks.append(
             {"name": "cli_model", "ok": True, "plan": {"models": cfg["models"]}}
         )
@@ -1191,7 +1151,6 @@ def run_dry_run(
         checks.append(schema_check)
 
     n_err = sum(1 for c in checks if not c["ok"])
-    all_warnings = list(dup_warnings) + list(result.warnings)
-    n_warn = len(all_warnings)
+    n_warn = len(result.warnings)
 
     return {"checks": checks, "n_errors": n_err, "n_warnings": n_warn}