From 956c3fcdc37e3d04e002e455bfeae9751197fb63 Mon Sep 17 00:00:00 2001 From: Bingran You Date: Sat, 11 Jul 2026 01:43:39 -0700 Subject: [PATCH 1/4] fix(eval): clean linear issue UX paths --- src/benchflow/_utils/scoring.py | 4 +++ src/benchflow/agents/env.py | 6 +++- src/benchflow/cli/main.py | 18 +++++++++- src/benchflow/evaluation.py | 22 ++++++++++-- src/benchflow/rollout/__init__.py | 4 +++ tests/test_agent_idle_timeout_cli.py | 51 +++++++++++++++++++++++++++ tests/test_cli_edge_case_hardening.py | 25 +++++++++++++ tests/test_provider_auth_detection.py | 24 +++++++++++++ 8 files changed, 150 insertions(+), 4 deletions(-) diff --git a/src/benchflow/_utils/scoring.py b/src/benchflow/_utils/scoring.py index b29efee34..9333bfd51 100644 --- a/src/benchflow/_utils/scoring.py +++ b/src/benchflow/_utils/scoring.py @@ -117,6 +117,10 @@ def classify_error(error: str | None) -> str | None: if not error: return None lower = error.lower() + if "missing credential" in lower or ( + "required for model" in lower and "not set" in lower + ): + return PROVIDER_AUTH if "agent idle for" in lower: return IDLE_TIMEOUT if "install failed" in lower: diff --git a/src/benchflow/agents/env.py b/src/benchflow/agents/env.py index ebf83e15c..e2fd1fda9 100644 --- a/src/benchflow/agents/env.py +++ b/src/benchflow/agents/env.py @@ -72,6 +72,10 @@ _AZURE_HOST_SUFFIXES = (".openai.azure.com", ".services.ai.azure.com") +class MissingAgentCredentialError(ValueError): + """Raised when an agent/model pair is missing a required credential.""" + + def _derive_azure_resource(agent_env: dict[str, str]) -> None: """Populate AZURE_RESOURCE from AZURE_API_ENDPOINT when not already set.""" if agent_env.get(_AZURE_RESOURCE_ENV): @@ -794,7 +798,7 @@ def resolve_agent_env( required_key, ) else: - raise ValueError( + raise MissingAgentCredentialError( f"{required_key} required for model {model!r} but not set. " "Pass it explicitly (for example via --agent-env/agent_env) " "or define it in .env." diff --git a/src/benchflow/cli/main.py b/src/benchflow/cli/main.py index 092e1617d..8870ff05f 100644 --- a/src/benchflow/cli/main.py +++ b/src/benchflow/cli/main.py @@ -557,6 +557,13 @@ def eval_run( int | None, typer.Option("--retry-attempts", help="Reserved retry-attempt override"), ] = None, + max_retries: Annotated[ + int | None, + typer.Option( + "--max-retries", + help="Alias for --retry-attempts; maximum retries per task.", + ), + ] = None, retry_concurrency: Annotated[ int | None, typer.Option("--retry-concurrency", help="Reserved retry concurrency"), @@ -592,6 +599,13 @@ def eval_run( """ _apply_dotenv_to_process_env() + if retry_attempts is not None and max_retries is not None: + print_error("--retry-attempts and --max-retries are aliases; pass only one") + raise typer.Exit(1) + retry_attempt_override = ( + retry_attempts if retry_attempts is not None else max_retries + ) + request = EvalCreateRequest( config_file=config_file, tasks_dir=tasks_dir, @@ -636,7 +650,7 @@ def eval_run( canonical_selection_out=canonical_selection_out, canonical_jobs_dir=canonical_jobs_dir, retry_policy=retry_policy, - retry_attempts=retry_attempts, + retry_attempts=retry_attempt_override, retry_concurrency=retry_concurrency, publish_hf=publish_hf, hf_prefix=hf_prefix, @@ -901,6 +915,8 @@ def _run_config_file_eval(plan: "EvalPlan") -> None: j._config.prompts = plan.eval_prompts if req.agent_idle_timeout is not None: j._config.agent_idle_timeout = plan.eval_agent_idle_timeout + if req.retry_attempts is not None: + j._config.retry.max_retries = req.retry_attempts if plan.usage_tracking_overridden: j._config.usage_tracking = j._config.usage_tracking.overlay( plan.eval_usage_tracking diff --git a/src/benchflow/evaluation.py b/src/benchflow/evaluation.py index 37fc5e1c8..994819027 100644 --- a/src/benchflow/evaluation.py +++ b/src/benchflow/evaluation.py @@ -789,10 +789,12 @@ def from_yaml(cls, path: str | Path, **kwargs) -> Evaluation: # Detect format: legacy uses "agents" + "datasets", benchflow uses "agent" if "agents" in raw or "datasets" in raw: return cls._from_legacy_yaml(raw, **kwargs) - return cls._from_native_yaml(raw, **kwargs) + return cls._from_native_yaml(raw, config_path=path, **kwargs) @classmethod - def _from_native_yaml(cls, raw: dict, **kwargs) -> Evaluation: + def _from_native_yaml( + cls, raw: dict, config_path: Path | None = None, **kwargs + ) -> Evaluation: """Parse benchflow-native YAML.""" from benchflow._utils.benchmark_repos import ( TASK_ALIASES, @@ -826,9 +828,25 @@ def _from_native_yaml(cls, raw: dict, **kwargs) -> Evaluation: elif "tasks_dir" in raw: # Legacy single-string format (backward compat). ref = raw["tasks_dir"] + if not isinstance(ref, str | Path): + raise ValueError( + f"YAML 'tasks_dir' must be a path string; got {type(ref).__name__}." + ) tasks_dir = Path(ref) if not tasks_dir.exists() and ref in TASK_ALIASES: tasks_dir = ensure_tasks(ref) + elif not tasks_dir.exists(): + hint = "" + if str(ref) == ".cache/programbench-benchflow": + hint = ( + " Generate ProgramBench tasks first with " + "`python benchmarks/programbench/run_programbench.py " + "benchmarks/programbench/programbench-gemini-flash-lite.yaml` " + "or `python -m benchmarks.programbench.main --output-dir " + ".cache/programbench-benchflow`." + ) + config_note = f" in {config_path}" if config_path is not None else "" + raise ValueError(f"YAML tasks_dir not found{config_note}: {ref}.{hint}") else: raise ValueError("YAML config must have 'source' or 'tasks_dir'") diff --git a/src/benchflow/rollout/__init__.py b/src/benchflow/rollout/__init__.py index 2567fc006..db89e259b 100644 --- a/src/benchflow/rollout/__init__.py +++ b/src/benchflow/rollout/__init__.py @@ -75,6 +75,7 @@ from benchflow._utils.scoring import classify_error as classify_error from benchflow.acp.types import McpServerSpec from benchflow.agents.credentials import upload_credential +from benchflow.agents.env import MissingAgentCredentialError from benchflow.agents.registry import AGENTS from benchflow.contracts import ( AgentProtocolError, @@ -2039,6 +2040,9 @@ async def run(self) -> RolloutResult: # placeholder during cleanup. self._error = str(e) logger.error(str(e)) + except MissingAgentCredentialError as e: + self._error = f"Missing credential: {e}" + logger.error(self._error) except Exception as e: self._error = str(e) logger.error("Run failed", exc_info=True) diff --git a/tests/test_agent_idle_timeout_cli.py b/tests/test_agent_idle_timeout_cli.py index a38891a70..ac4d2ed6a 100644 --- a/tests/test_agent_idle_timeout_cli.py +++ b/tests/test_agent_idle_timeout_cli.py @@ -74,6 +74,57 @@ def test_eval_create_reasoning_effort_lands_in_config(tmp_path: Path): assert captured["config"].reasoning_effort == "max" +def test_eval_create_max_retries_alias_lands_in_config(tmp_path: Path): + """Guards the fix for Linear ENG-169: --max-retries reaches RetryConfig.""" + tasks = _make_tasks_dir(tmp_path) + captured: dict = {} + + with _stub_evaluation_run(captured): + eval_run( + tasks_dir=tasks, + max_retries=3, + jobs_dir=str(tmp_path / "jobs"), + ) + + assert captured["config"].retry.max_retries == 3 + + +def test_eval_create_max_retries_alias_overrides_yaml_config(tmp_path: Path): + """Guards the fix for Linear ENG-169 on --config eval runs.""" + tasks = _make_tasks_dir(tmp_path) + yaml_path = tmp_path / "config.yaml" + yaml_path.write_text( + "tasks_dir: " + str(tasks) + "\nagent: oracle\nmax_retries: 1\n" + ) + captured: dict = {} + + with _stub_evaluation_run(captured): + eval_run( + config_file=yaml_path, + max_retries=4, + jobs_dir=str(tmp_path / "jobs"), + ) + + assert captured["config"].retry.max_retries == 4 + + +def test_eval_create_rejects_duplicate_retry_aliases(tmp_path: Path): + """Guards the fix for Linear ENG-169 against ambiguous retry aliases.""" + tasks = _make_tasks_dir(tmp_path) + + try: + eval_run( + tasks_dir=tasks, + retry_attempts=1, + max_retries=2, + jobs_dir=str(tmp_path / "jobs"), + ) + except typer.Exit as exc: + assert exc.exit_code == 1 + else: + raise AssertionError("expected typer.Exit for duplicate retry aliases") + + def test_eval_create_reasoning_effort_overrides_yaml_config(tmp_path: Path): """Guards SkillsBench PR #825 so CLI effort overrides YAML defaults.""" tasks = _make_tasks_dir(tmp_path) diff --git a/tests/test_cli_edge_case_hardening.py b/tests/test_cli_edge_case_hardening.py index 3bb97a04d..c88cbf7a8 100644 --- a/tests/test_cli_edge_case_hardening.py +++ b/tests/test_cli_edge_case_hardening.py @@ -230,6 +230,31 @@ def test_yaml_string_prompts_are_wrapped_in_a_list(tmp_path): assert ev._config.prompts == ["do the thing"] +def test_from_yaml_rejects_missing_tasks_dir_with_programbench_hint(tmp_path): + """Guards the fix for Linear ENG-162: missing ProgramBench tasks fail cleanly.""" + from benchflow.evaluation import Evaluation + + p = tmp_path / "programbench.yaml" + p.write_text("tasks_dir: .cache/programbench-benchflow\nagent: oracle\n") + + with pytest.raises(ValueError, match=r"run_programbench\.py"): + Evaluation.from_yaml(p) + + +def test_eval_create_config_missing_tasks_dir_no_raw_traceback(tmp_path): + """Guards the fix for Linear ENG-162 at the CLI config boundary.""" + p = tmp_path / "programbench.yaml" + p.write_text("tasks_dir: .cache/programbench-benchflow\nagent: oracle\n") + + res = runner.invoke(app, ["eval", "create", "--config", str(p)]) + + assert res.exit_code == 1 + assert "Traceback (most recent call last)" not in res.output + normalized = " ".join(res.output.split()) + assert "YAML tasks_dir not found" in normalized + assert "run_programbench.py" in res.output + + # ── markup in user input crashes error handlers (H10/M13) ───────────────────── diff --git a/tests/test_provider_auth_detection.py b/tests/test_provider_auth_detection.py index 1f745f09d..b09e92cc9 100644 --- a/tests/test_provider_auth_detection.py +++ b/tests/test_provider_auth_detection.py @@ -395,6 +395,30 @@ def spy_classify(e): assert rollout._provider_auth_status_cached == 401 +@pytest.mark.asyncio +async def test_run_missing_credential_error_is_clean_provider_auth( + tmp_path, monkeypatch, caplog +): + """Guards the fix for Linear ENG-257: missing credentials avoid tracebacks.""" + from benchflow._utils.scoring import PROVIDER_AUTH, classify_error + from benchflow.agents.env import MissingAgentCredentialError + + rollout = _auth_rollout(tmp_path, usage_source="unavailable") + rollout._rollout_dir = None + missing = MissingAgentCredentialError( + "GEMINI_API_KEY required for model 'gemini/gemini-3.1-flash' but not set." + ) + + monkeypatch.setattr(rollout, "setup", AsyncMock(side_effect=missing)) + caplog.set_level("ERROR", logger="benchflow.rollout") + + result = await rollout.run() + + assert result.error == f"Missing credential: {missing}" + assert classify_error(result.error) == PROVIDER_AUTH + assert "Run failed" not in caplog.text + + def test_enforcement_still_fires_when_no_error_and_usage_missing(tmp_path): """Negative control for FIX 1 (PR #564 / issue #546): on a clean run with no ACP error and no captured provider usage, the legitimate required-usage From 0d17cdab2a0b73373a85a61563eabf44b68d7713 Mon Sep 17 00:00:00 2001 From: Bingran You Date: Sat, 11 Jul 2026 01:44:35 -0700 Subject: [PATCH 2/4] test: name PR 918 regressions --- tests/test_agent_idle_timeout_cli.py | 6 +++--- tests/test_cli_edge_case_hardening.py | 4 ++-- tests/test_provider_auth_detection.py | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_agent_idle_timeout_cli.py b/tests/test_agent_idle_timeout_cli.py index ac4d2ed6a..0d03f1a00 100644 --- a/tests/test_agent_idle_timeout_cli.py +++ b/tests/test_agent_idle_timeout_cli.py @@ -75,7 +75,7 @@ def test_eval_create_reasoning_effort_lands_in_config(tmp_path: Path): def test_eval_create_max_retries_alias_lands_in_config(tmp_path: Path): - """Guards the fix for Linear ENG-169: --max-retries reaches RetryConfig.""" + """Guards the fix from PR #918 / Linear ENG-169: --max-retries reaches RetryConfig.""" tasks = _make_tasks_dir(tmp_path) captured: dict = {} @@ -90,7 +90,7 @@ def test_eval_create_max_retries_alias_lands_in_config(tmp_path: Path): def test_eval_create_max_retries_alias_overrides_yaml_config(tmp_path: Path): - """Guards the fix for Linear ENG-169 on --config eval runs.""" + """Guards the fix from PR #918 / Linear ENG-169 on --config eval runs.""" tasks = _make_tasks_dir(tmp_path) yaml_path = tmp_path / "config.yaml" yaml_path.write_text( @@ -109,7 +109,7 @@ def test_eval_create_max_retries_alias_overrides_yaml_config(tmp_path: Path): def test_eval_create_rejects_duplicate_retry_aliases(tmp_path: Path): - """Guards the fix for Linear ENG-169 against ambiguous retry aliases.""" + """Guards the fix from PR #918 / Linear ENG-169 against ambiguous retry aliases.""" tasks = _make_tasks_dir(tmp_path) try: diff --git a/tests/test_cli_edge_case_hardening.py b/tests/test_cli_edge_case_hardening.py index c88cbf7a8..ba52b1567 100644 --- a/tests/test_cli_edge_case_hardening.py +++ b/tests/test_cli_edge_case_hardening.py @@ -231,7 +231,7 @@ def test_yaml_string_prompts_are_wrapped_in_a_list(tmp_path): def test_from_yaml_rejects_missing_tasks_dir_with_programbench_hint(tmp_path): - """Guards the fix for Linear ENG-162: missing ProgramBench tasks fail cleanly.""" + """Guards the fix from PR #918 / Linear ENG-162: missing ProgramBench tasks fail cleanly.""" from benchflow.evaluation import Evaluation p = tmp_path / "programbench.yaml" @@ -242,7 +242,7 @@ def test_from_yaml_rejects_missing_tasks_dir_with_programbench_hint(tmp_path): def test_eval_create_config_missing_tasks_dir_no_raw_traceback(tmp_path): - """Guards the fix for Linear ENG-162 at the CLI config boundary.""" + """Guards the fix from PR #918 / Linear ENG-162 at the CLI config boundary.""" p = tmp_path / "programbench.yaml" p.write_text("tasks_dir: .cache/programbench-benchflow\nagent: oracle\n") diff --git a/tests/test_provider_auth_detection.py b/tests/test_provider_auth_detection.py index b09e92cc9..3de9c0146 100644 --- a/tests/test_provider_auth_detection.py +++ b/tests/test_provider_auth_detection.py @@ -399,7 +399,7 @@ def spy_classify(e): async def test_run_missing_credential_error_is_clean_provider_auth( tmp_path, monkeypatch, caplog ): - """Guards the fix for Linear ENG-257: missing credentials avoid tracebacks.""" + """Guards the fix from PR #918 / Linear ENG-257: missing credentials avoid tracebacks.""" from benchflow._utils.scoring import PROVIDER_AUTH, classify_error from benchflow.agents.env import MissingAgentCredentialError From 2558f22053e1b6fb69804147d040e3ab1088d66a Mon Sep 17 00:00:00 2001 From: Bingran You Date: Sat, 11 Jul 2026 01:51:31 -0700 Subject: [PATCH 3/4] fix: narrow ProgramBench tasks-dir validation --- docs/reference/cli.md | 1 + src/benchflow/evaluation.py | 20 +++++++++----------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/docs/reference/cli.md b/docs/reference/cli.md index f3a6d129e..120b00178 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -275,6 +275,7 @@ bench eval run --tasks-dir ./tasks --matrix matrix.yaml --trials 3 | `--canonical-jobs-dir` | — | Materialize selected rollout directories for trainer conversion | | `--retry-policy` | `default` | Retry policy label for reproducible eval artifacts: `default` or `unscored-only` | | `--retry-attempts` | — | Override retry attempts for the eval run | +| `--max-retries` | — | Alias for `--retry-attempts`; maximum retries per task | | `--retry-concurrency` | — | Reserved retry concurrency setting recorded in run config | | `--publish-hf` | — | Upload final eval artifacts to this Hugging Face dataset repo | | `--hf-prefix` | — | Path prefix inside the Hugging Face repo; requires `--publish-hf` | diff --git a/src/benchflow/evaluation.py b/src/benchflow/evaluation.py index 994819027..d4c86409d 100644 --- a/src/benchflow/evaluation.py +++ b/src/benchflow/evaluation.py @@ -835,18 +835,16 @@ def _from_native_yaml( tasks_dir = Path(ref) if not tasks_dir.exists() and ref in TASK_ALIASES: tasks_dir = ensure_tasks(ref) - elif not tasks_dir.exists(): - hint = "" - if str(ref) == ".cache/programbench-benchflow": - hint = ( - " Generate ProgramBench tasks first with " - "`python benchmarks/programbench/run_programbench.py " - "benchmarks/programbench/programbench-gemini-flash-lite.yaml` " - "or `python -m benchmarks.programbench.main --output-dir " - ".cache/programbench-benchflow`." - ) + elif str(ref) == ".cache/programbench-benchflow": config_note = f" in {config_path}" if config_path is not None else "" - raise ValueError(f"YAML tasks_dir not found{config_note}: {ref}.{hint}") + raise ValueError( + f"YAML tasks_dir not found{config_note}: {ref}. " + "Generate ProgramBench tasks first with " + "`python benchmarks/programbench/run_programbench.py " + "benchmarks/programbench/programbench-gemini-flash-lite.yaml` " + "or `python -m benchmarks.programbench.main --output-dir " + ".cache/programbench-benchflow`." + ) else: raise ValueError("YAML config must have 'source' or 'tasks_dir'") From ccb8284cd62e37ab898393fd74e07bbd6b805238 Mon Sep 17 00:00:00 2001 From: Bingran You Date: Sat, 11 Jul 2026 06:05:18 -0700 Subject: [PATCH 4/4] fix(eval): accept generated ProgramBench task dirs --- src/benchflow/evaluation.py | 2 +- tests/test_cli_edge_case_hardening.py | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/benchflow/evaluation.py b/src/benchflow/evaluation.py index d4c86409d..ceda266c9 100644 --- a/src/benchflow/evaluation.py +++ b/src/benchflow/evaluation.py @@ -835,7 +835,7 @@ def _from_native_yaml( tasks_dir = Path(ref) if not tasks_dir.exists() and ref in TASK_ALIASES: tasks_dir = ensure_tasks(ref) - elif str(ref) == ".cache/programbench-benchflow": + elif not tasks_dir.exists() and str(ref) == ".cache/programbench-benchflow": config_note = f" in {config_path}" if config_path is not None else "" raise ValueError( f"YAML tasks_dir not found{config_note}: {ref}. " diff --git a/tests/test_cli_edge_case_hardening.py b/tests/test_cli_edge_case_hardening.py index ba52b1567..e9863c06f 100644 --- a/tests/test_cli_edge_case_hardening.py +++ b/tests/test_cli_edge_case_hardening.py @@ -9,6 +9,7 @@ import json import os +from pathlib import Path from types import SimpleNamespace import pytest @@ -241,6 +242,21 @@ def test_from_yaml_rejects_missing_tasks_dir_with_programbench_hint(tmp_path): Evaluation.from_yaml(p) +def test_from_yaml_accepts_existing_programbench_tasks_dir(tmp_path, monkeypatch): + """Guards the fix from PR #918 / Linear ENG-162: generated ProgramBench dirs load.""" + from benchflow.evaluation import Evaluation + + (tmp_path / ".cache" / "programbench-benchflow").mkdir(parents=True) + p = tmp_path / "programbench.yaml" + p.write_text("tasks_dir: .cache/programbench-benchflow\nagent: oracle\n") + + monkeypatch.chdir(tmp_path) + + ev = Evaluation.from_yaml(p) + + assert ev._tasks_dir == Path(".cache/programbench-benchflow") + + def test_eval_create_config_missing_tasks_dir_no_raw_traceback(tmp_path): """Guards the fix from PR #918 / Linear ENG-162 at the CLI config boundary.""" p = tmp_path / "programbench.yaml"