diff --git a/src/coder_eval/cli/run_command.py b/src/coder_eval/cli/run_command.py index de12513e..b35366ca 100644 --- a/src/coder_eval/cli/run_command.py +++ b/src/coder_eval/cli/run_command.py @@ -583,9 +583,13 @@ async def _run_with_experiment( default_experiment = experiment # fall back to custom as its own baseline # Resolve tasks through experiment layer (applies all 5 config layers). - # Layer-5 override failures (invalid -D value/path, sdk_options on a - # non-claude agent, the agent.type guard) and duplicate-task-id checks raise - # ValueError here; surface them as a clean CLI error instead of a traceback. + # Global failures raise ValueError here — duplicate task IDs, early-stop + # arming, or an invocation error that trips every task identically (bad + # --type / -D value, repeats over the cap) — and we surface them as a clean + # CLI error instead of a traceback. Per-task config-resolution failures + # (e.g. sdk_options on a non-claude agent) among otherwise-resolvable tasks + # are NOT raised: resolve_all_tasks isolates them into `skipped` so one + # incompatible task can't abort the whole suite. try: resolved, skipped = resolve_all_tasks( task_files=all_task_files, diff --git a/src/coder_eval/orchestration/experiment.py b/src/coder_eval/orchestration/experiment.py index 0962c362..3f55a890 100644 --- a/src/coder_eval/orchestration/experiment.py +++ b/src/coder_eval/orchestration/experiment.py @@ -552,10 +552,18 @@ def resolve_all_tasks( Also handles tag filtering and unique task ID validation. Task YAMLs that fail to load (YAML parse error, Pydantic validation, - dataset expansion error) are recorded in the returned ``skipped`` list - and excluded from the resolved set rather than aborting the suite. The - caller surfaces ``skipped`` in the run summary so the failure is loud - but recoverable. + dataset expansion error) are recorded in the returned ``skipped`` list and + excluded from the resolved set rather than aborting the suite. + + Per-task config-resolution failures (a task whose own YAML is incompatible + with the resolved run — e.g. Claude-only ``sdk_options`` surviving a + ``--type codex`` override, which ``CodexAgentConfig`` forbids) are likewise + demoted to ``skipped`` — but only when other tasks resolve. If EVERY task + that reaches resolution fails, the cause is a global invocation error (a bad + ``--type`` / ``-D`` value, repeats over the cap) rather than a per-task + incompatibility, so it is re-raised and aborts the run. Early-stop arming + errors always propagate. The caller surfaces ``skipped`` in the run summary + so per-task failures are loud but recoverable. Args: task_files: Paths to task YAML files. @@ -572,10 +580,18 @@ def resolve_all_tasks( Raises: ValueError: If duplicate task IDs are found after resolution. """ - from .early_stop import validate_early_stop + from .early_stop import EarlyStopConfigError, validate_early_stop resolved: list[ResolvedTask] = [] skipped: list[SkippedTask] = [] + # Per-task config-resolution failures are collected here rather than raised + # inline. After the loop they are demoted to ``skipped`` — UNLESS every task + # that reached resolution failed, which signals a global invocation error + # (bad --type, an invalid -D value, repeats over the cap) that trips every + # task identically rather than a per-task incompatibility; that case is + # re-raised so the CLI aborts cleanly instead of producing an empty run. + resolution_errors: list[tuple[Path, Exception]] = [] + attempted = 0 # Resolve variant-level initial_prompt_file paths before the main loop exp_dir = experiment_file.parent if experiment_file is not None else None @@ -623,50 +639,89 @@ def resolve_all_tasks( skipped.append(SkippedTask(path=str(task_file), reason=reason)) continue - for expanded_task in expanded_tasks: - for variant in experiment.variants: - # Apply layers 1-4 (default → experiment-defaults → task → variant) + resolve repeats - resolved_task, lineage, effective_repeats = resolve_task_for_variant( - default_experiment, expanded_task, experiment, variant, config - ) + # Isolate layer 1-5 config resolution per task file. A task whose own + # YAML config is incompatible with the resolved run — e.g. Claude-only + # agent fields (`sdk_options`) surviving a `--type codex` override, which + # `CodexAgentConfig` forbids (`extra="forbid"`) — raises here. Without + # isolation that single task aborts the entire coder-eval run. Buffer the + # file's resolved tasks and commit them only once the whole file + # resolves, so a mid-file failure discards this file's fan-out as a unit + # (mirroring the load/expand isolation above) rather than leaving a + # partial, lopsided fan-out behind. + attempted += 1 + file_resolved: list[ResolvedTask] = [] + try: + for expanded_task in expanded_tasks: + for variant in experiment.variants: + # Apply layers 1-4 (default → experiment-defaults → task → variant) + resolve repeats + resolved_task, lineage, effective_repeats = resolve_task_for_variant( + default_experiment, expanded_task, experiment, variant, config + ) - # Resolve file paths injected by variant overrides - resolve_task_files(resolved_task, task_file, experiment_file) - - # Apply prompt mutations or overrides (between file resolution and CLI overrides) - _apply_prompt_overrides(resolved_task, experiment, variant, lineage) - - # Apply layer 5 (CLI overrides) - _apply_cli_overrides(resolved_task, config, lineage) - - # Early-stop guardrails: run once the task is fully resolved (all 5 - # layers merged, incl. -D run_limits.stop_early). No-op unless armed; - # a bad arming raises EarlyStopConfigError (a ValueError) which the - # run path converts to a clean CLI error. - validate_early_stop(resolved_task) - - # Fan-out: simulation n_trials takes precedence over experiment repeats - # when simulation is active; otherwise use experiment-level repeats. - sim = resolved_task.simulation - n_trials = sim.n_trials if (sim is not None and sim.enabled) else 1 - fan_count = n_trials if n_trials > 1 else effective_repeats - for rep in range(fan_count): - resolved.append( - ResolvedTask( - task=resolved_task, - task_file=task_file, - run_dir=build_task_run_dir( - config.run_dir, - variant.variant_id, - resolved_task.task_id, + # Resolve file paths injected by variant overrides + resolve_task_files(resolved_task, task_file, experiment_file) + + # Apply prompt mutations or overrides (between file resolution and CLI overrides) + _apply_prompt_overrides(resolved_task, experiment, variant, lineage) + + # Apply layer 5 (CLI overrides) + _apply_cli_overrides(resolved_task, config, lineage) + + # Early-stop guardrails: run once the task is fully resolved (all 5 + # layers merged, incl. -D run_limits.stop_early). No-op unless armed; + # a bad arming raises EarlyStopConfigError (a ValueError). + validate_early_stop(resolved_task) + + # Fan-out: simulation n_trials takes precedence over experiment repeats + # when simulation is active; otherwise use experiment-level repeats. + sim = resolved_task.simulation + n_trials = sim.n_trials if (sim is not None and sim.enabled) else 1 + fan_count = n_trials if n_trials > 1 else effective_repeats + for rep in range(fan_count): + file_resolved.append( + ResolvedTask( + task=resolved_task, + task_file=task_file, + run_dir=build_task_run_dir( + config.run_dir, + variant.variant_id, + resolved_task.task_id, + replicate_index=rep, + ), + variant_id=variant.variant_id, replicate_index=rep, - ), - variant_id=variant.variant_id, - replicate_index=rep, - source_yaml=source_yaml, - config_lineage=dict(lineage), + source_yaml=source_yaml, + config_lineage=dict(lineage), + ) ) - ) + # Early-stop arming errors are a deliberate hard stop: they always + # propagate (never demoted to skipped) so a misarmed run fails loudly. + except EarlyStopConfigError: + raise + # Narrow set, matching the load/expand block above: config-resolution + # and IO failures are collected (decided after the loop, below); + # AttributeError / TypeError / ImportError still crash loudly as + # regressions. Pydantic ValidationError is a ValueError subclass in v2, + # so it's covered. + except (FileNotFoundError, OSError, ValueError, yaml.YAMLError) as exc: + resolution_errors.append((task_file, exc)) + continue + + resolved.extend(file_resolved) + + # Decide the fate of collected per-task resolution failures. If EVERY task + # that reached resolution failed, the cause is almost certainly a global + # invocation error (a bad --type / -D value, repeats over the cap) that + # trips every task identically — re-raise the first so the CLI aborts + # cleanly with its own message instead of silently producing an empty run. + # Otherwise the failures are per-task incompatibilities: demote them to + # ``skipped`` and run the tasks that did resolve. + if resolution_errors and len(resolution_errors) == attempted: + raise resolution_errors[0][1] + for err_file, exc in resolution_errors: + reason = f"{type(exc).__name__}: {exc}"[:500] + logger.warning("Skipping task file %s — config resolution failed: %s", err_file, reason) + skipped.append(SkippedTask(path=str(err_file), reason=reason)) # Filter by tags if config.include_tags or config.exclude_tags: diff --git a/tests/test_dataset_expansion.py b/tests/test_dataset_expansion.py index fb741d63..894cb9a2 100644 --- a/tests/test_dataset_expansion.py +++ b/tests/test_dataset_expansion.py @@ -726,3 +726,106 @@ def test_duplicate_detection_still_catches_true_dupes(self, tmp_path: Path) -> N with pytest.raises(ValueError, match="Duplicate task IDs"): resolve_all_tasks([task_file, task_file2], experiment, experiment, config) + + def test_config_resolution_failure_isolated_to_skipped(self, tmp_path: Path) -> None: + """A per-task layer-5 resolution failure skips just that task, not the suite. + + `--type codex` (config.agent_type) rewrites a task whose YAML carries the + Claude-only `sdk_options` into a CodexAgentConfig, which forbids that field + (`extra="forbid"`). The incompatibility raises during layer-5 resolution; + the task must land in `skipped` while a sibling still resolves — rather than + aborting the whole coder-eval run (as it did before this isolation). + """ + from coder_eval.orchestration.config import BatchRunConfig + from coder_eval.orchestration.experiment import resolve_all_tasks + + good = { + "task_id": "plain-task", + "description": "d", + "initial_prompt": "p", + "sandbox": {"driver": "tempdir"}, + "success_criteria": [{"type": "file_exists", "path": "f.py", "description": "d"}], + } + # Valid as claude-code (sdk_options is Claude-only); becomes invalid only + # once --type codex rewrites the agent kind at layer 5. + claude_only = { + **good, + "task_id": "claude-only-task", + "agent": {"type": "claude-code", "sdk_options": {"effort": "high"}}, + } + good_file = tmp_path / "good.yaml" + good_file.write_text(yaml.dump(good)) + bad_file = tmp_path / "claude_only.yaml" + bad_file.write_text(yaml.dump(claude_only)) + + experiment = ExperimentDefinition( + experiment_id="default", + defaults=ExperimentDefaults(agent={"type": "claude-code"}), + variants=[ExperimentVariant(variant_id="v1")], + ) + # --type codex applied to every task (layer 5, highest precedence). + config = BatchRunConfig(run_dir=tmp_path / "runs", agent_type="codex") + + resolved, skipped = resolve_all_tasks([good_file, bad_file], experiment, experiment, config) + + # Sibling resolved; the incompatible task is isolated (no raise). + assert [rt.task.task_id for rt in resolved] == ["plain-task"] + assert len(skipped) == 1 + assert skipped[0].path == str(bad_file) + assert "sdk_options" in skipped[0].reason + + def test_config_resolution_failure_skips_whole_file_no_partial_fanout(self, tmp_path: Path) -> None: + """A variant that fails resolution rolls back its whole file's fan-out. + + `bad.yaml` carries Claude-only `sdk_options`: it resolves cleanly under + the claude-code variant but fails under the codex variant (which forbids + the field). Because a file's resolved tasks are buffered and committed as + a unit, the clean claude-code entry is rolled back with the failing codex + one — the file is skipped whole, never left as a lopsided partial fan-out + that would skew an A/B comparison. Sibling `good.yaml` (no sdk_options) + resolves under both variants, so the failure is isolated, not a global + abort. + """ + from coder_eval.orchestration.config import BatchRunConfig + from coder_eval.orchestration.experiment import resolve_all_tasks + + def _task(task_id: str, agent: dict[str, Any] | None = None) -> dict[str, Any]: + data: dict[str, Any] = { + "task_id": task_id, + "description": "d", + "initial_prompt": "p", + "sandbox": {"driver": "tempdir"}, + "success_criteria": [{"type": "file_exists", "path": "f.py", "description": "d"}], + } + if agent is not None: + data["agent"] = agent + return data + + good_file = tmp_path / "good.yaml" + good_file.write_text(yaml.dump(_task("good-task"))) + bad_file = tmp_path / "bad.yaml" + # Claude-only sdk_options: valid under the claude-code variant, forbidden + # once the codex variant rewrites the agent kind. + bad_file.write_text( + yaml.dump(_task("bad-task", agent={"type": "claude-code", "sdk_options": {"effort": "high"}})) + ) + + experiment = ExperimentDefinition( + experiment_id="exp", + defaults=ExperimentDefaults(agent={"type": "claude-code"}), + variants=[ + ExperimentVariant(variant_id="as-claude"), + ExperimentVariant(variant_id="as-codex", agent={"type": "codex"}), + ], + ) + config = BatchRunConfig(run_dir=tmp_path / "runs") + + resolved, skipped = resolve_all_tasks([good_file, bad_file], experiment, experiment, config) + + # good.yaml resolves for BOTH variants; bad.yaml is skipped whole — its + # clean claude-code entry is rolled back with the failing codex one. + assert {rt.task.task_id for rt in resolved} == {"good-task"} + assert sorted(rt.variant_id for rt in resolved) == ["as-claude", "as-codex"] + assert len(skipped) == 1 + assert skipped[0].path == str(bad_file) + assert "sdk_options" in skipped[0].reason