Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/benchflow/_utils/scoring.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 5 additions & 1 deletion src/benchflow/agents/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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."
Expand Down
18 changes: 17 additions & 1 deletion src/benchflow/cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
22 changes: 20 additions & 2 deletions src/benchflow/evaluation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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():

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Defer or rebase native YAML tasks_dir validation

This new existence check evaluates relative tasks_dir values against the current process directory during config loading. Existing Evaluation.from_yaml() callers and tests create self-contained configs outside the cwd with tasks_dir: tasks and load them to inspect/override config fields before running; those now raise YAML tasks_dir not found even though the loader previously accepted the config. Please either keep from_yaml() as a parser and defer this validation to the run path, or resolve relative paths consistently before rejecting them.

Useful? React with 👍 / 👎.

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'")

Expand Down
4 changes: 4 additions & 0 deletions src/benchflow/rollout/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
51 changes: 51 additions & 0 deletions tests/test_agent_idle_timeout_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
25 changes: 25 additions & 0 deletions tests/test_cli_edge_case_hardening.py
Original file line number Diff line number Diff line change
Expand Up @@ -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) ─────────────────────


Expand Down
24 changes: 24 additions & 0 deletions tests/test_provider_auth_detection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading