diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index b9f4c3939..007ff2d41 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -32,7 +32,7 @@ BC-Bench is category-based and designed to grow over time. It currently has two ### Architecture design conventions -Preserve one-way dependency flow from orchestration toward lower-level abstractions. Keep domain and result models independent of runtime code. CLI commands are composition roots: they select concrete agents and inject them into evaluation pipelines through `AgentRunner`; pipelines must not select agent implementations. +Preserve one-way dependency flow from orchestration toward lower-level abstractions. Keep domain and result models independent of runtime code. CLI commands are composition roots: they construct concrete agent harnesses (`bcbench_core.agent.harness.Harness`) and inject them into evaluation pipelines; pipelines must not select agent implementations. `bcbench.types` is the central category registry. Extend `EvaluationCategory` for category-owned mappings such as datasets, pipelines, results, and scoring behavior instead of duplicating those decisions elsewhere. Keep imports following the existing direction and avoid circular dependencies. diff --git a/src/bcbench/agent/__init__.py b/src/bcbench/agent/__init__.py index 9a08648e6..c62f11866 100644 --- a/src/bcbench/agent/__init__.py +++ b/src/bcbench/agent/__init__.py @@ -1,8 +1,8 @@ """Agent module for BC-Bench.""" -from bcbench.agent.bcal import BCalBackendConfig, get_bcal_version, run_bcal_agent -from bcbench.agent.claude import get_claude_version, run_claude_code +from bcbench.agent.bcal import BCalBackendConfig, BCalHarness +from bcbench.agent.claude import ClaudeHarness from bcbench.agent.copilot import CopilotHarness -from bcbench.agent.pr_review import get_pr_review_version, run_pr_review_agent +from bcbench.agent.pr_review import PRReviewHarness -__all__ = ["BCalBackendConfig", "CopilotHarness", "get_bcal_version", "get_claude_version", "get_pr_review_version", "run_bcal_agent", "run_claude_code", "run_pr_review_agent"] +__all__ = ["BCalBackendConfig", "BCalHarness", "ClaudeHarness", "CopilotHarness", "PRReviewHarness"] diff --git a/src/bcbench/agent/bcal/__init__.py b/src/bcbench/agent/bcal/__init__.py index 0ceca94c1..eced543f8 100644 --- a/src/bcbench/agent/bcal/__init__.py +++ b/src/bcbench/agent/bcal/__init__.py @@ -1,5 +1,5 @@ """BCal dotnet tool agent module for NL2AL evaluation.""" -from bcbench.agent.bcal.agent import BCalBackendConfig, get_bcal_version, run_bcal_agent, run_bcal_prompt +from bcbench.agent.bcal.agent import BCalBackendConfig, BCalHarness, get_bcal_version, run_bcal_prompt -__all__ = ["BCalBackendConfig", "get_bcal_version", "run_bcal_agent", "run_bcal_prompt"] +__all__ = ["BCalBackendConfig", "BCalHarness", "get_bcal_version", "run_bcal_prompt"] diff --git a/src/bcbench/agent/bcal/agent.py b/src/bcbench/agent/bcal/agent.py index 8764ddb46..d6700d261 100644 --- a/src/bcbench/agent/bcal/agent.py +++ b/src/bcbench/agent/bcal/agent.py @@ -4,10 +4,12 @@ import shutil import subprocess import time +from dataclasses import dataclass from pathlib import Path +from typing import override from bcbench_core.agent.experiment import ExperimentConfiguration -from bcbench_core.agent.harness import AgentResult +from bcbench_core.agent.harness import AgentIdentity, AgentRequest, AgentResult, Harness from bcbench_core.agent.metrics import AgentMetrics from bcbench_core.agent.version import get_cli_version from bcbench_core.artifacts import ALPACKAGES_DIRNAME @@ -15,6 +17,7 @@ from pydantic import BaseModel, ConfigDict, field_validator from bcbench.dataset import NL2ALEntry +from bcbench.types import AgentHarness logger = logging.getLogger(__name__) @@ -120,49 +123,58 @@ def _bcal_cmd_args(entry: NL2ALEntry, prompt: str, package_cache_path: Path, exp ] -def run_bcal_agent( - entry: NL2ALEntry, - repo_path: Path, - backend_config: BCalBackendConfig, - timeout: int = 25 * 60, -) -> AgentResult: - logger.info(f"Running bcal CLI on: {entry.instance_id}") - - # The .alpackages dir is created by the NL2AL pipeline setup step - project_name: str = entry.project_paths[0] - package_cache_path = repo_path / project_name / ALPACKAGES_DIRNAME - if not package_cache_path.exists(): - raise AgentError(f"Package cache not found at: {package_cache_path}. Run the setup step first.") - - export_folder = repo_path / project_name / "src" - cmd_args = _bcal_cmd_args(entry, entry.get_task(), package_cache_path, export_folder, backend_config) - - logger.info(f"Export folder: {export_folder}") - logger.debug(f"Package cache path: {package_cache_path}") - logger.debug(f"Using prompt:\n{entry.get_task()}") - logger.debug(f"bcal CLI command: {cmd_args}") - - try: - start = time.monotonic() - subprocess.run( - cmd_args, - timeout=timeout, - check=True, - ) - execution_time = time.monotonic() - start - - logger.info(f"bcal CLI run complete for: {entry.instance_id}") - return AgentResult(metrics=AgentMetrics(execution_time=execution_time), experiment=ExperimentConfiguration()) - except subprocess.TimeoutExpired: - logger.exception(f"bcal CLI timed out after {timeout} seconds") - metrics = AgentMetrics(execution_time=timeout) - raise AgentTimeoutError("bcal CLI timed out", metrics=metrics, config=ExperimentConfiguration()) from None - except subprocess.CalledProcessError as e: - logger.exception(f"bcal CLI execution failed: {e.stderr}") - raise AgentError(f"bcal CLI execution failed: {e}") from None - except Exception: - logger.exception("Unexpected error running bcal CLI") - raise +@dataclass(frozen=True) +class BCalHarness(Harness[NL2ALEntry, AgentHarness]): + """The bcal CLI, generating AL code for NL2AL entries into the prepared workspace.""" + + backend_config: BCalBackendConfig + timeout: int = 25 * 60 + + @override + def identity(self) -> AgentIdentity[AgentHarness]: + return AgentIdentity[AgentHarness](harness=AgentHarness.BCAL, version=get_bcal_version(), model=self.backend_config.model or "external-command") + + @override + def run(self, request: AgentRequest[NL2ALEntry], /) -> AgentResult: + entry, repo_path, backend_config = request.task, request.work_dir, self.backend_config + timeout = self.timeout if request.timeout is None else request.timeout + logger.info(f"Running bcal CLI on: {entry.instance_id}") + + # The .alpackages dir is created by the NL2AL pipeline setup step + project_name: str = entry.project_paths[0] + package_cache_path = repo_path / project_name / ALPACKAGES_DIRNAME + if not package_cache_path.exists(): + raise AgentError(f"Package cache not found at: {package_cache_path}. Run the setup step first.") + + export_folder = repo_path / project_name / "src" + cmd_args = _bcal_cmd_args(entry, entry.get_task(), package_cache_path, export_folder, backend_config) + + logger.info(f"Export folder: {export_folder}") + logger.debug(f"Package cache path: {package_cache_path}") + logger.debug(f"Using prompt:\n{entry.get_task()}") + logger.debug(f"bcal CLI command: {cmd_args}") + + try: + start = time.monotonic() + subprocess.run( + cmd_args, + timeout=timeout, + check=True, + ) + execution_time = time.monotonic() - start + + logger.info(f"bcal CLI run complete for: {entry.instance_id}") + return AgentResult(metrics=AgentMetrics(execution_time=execution_time), experiment=ExperimentConfiguration()) + except subprocess.TimeoutExpired: + logger.exception(f"bcal CLI timed out after {timeout} seconds") + metrics = AgentMetrics(execution_time=timeout) + raise AgentTimeoutError("bcal CLI timed out", metrics=metrics, config=ExperimentConfiguration()) from None + except subprocess.CalledProcessError as e: + logger.exception(f"bcal CLI execution failed: {e.stderr}") + raise AgentError(f"bcal CLI execution failed: {e}") from None + except Exception: + logger.exception("Unexpected error running bcal CLI") + raise def run_bcal_prompt( @@ -178,7 +190,7 @@ def run_bcal_prompt( BCal writes generated AL to the export folder and status/output to stdout. Surface both while removing the echoed user prompt so the safety judge does not score the attack as target output. - Unlike `run_bcal_agent` this raises on timeout/non-zero exit instead of returning the text: a + Unlike `BCalHarness` this raises on timeout/non-zero exit instead of returning the text: a red-team judge must never score bcal's own error output as if it were a harmless refusal. Assumes symbols are already present under ``package_cache_path``. diff --git a/src/bcbench/agent/claude/__init__.py b/src/bcbench/agent/claude/__init__.py index fb71052f7..be5f41dbc 100644 --- a/src/bcbench/agent/claude/__init__.py +++ b/src/bcbench/agent/claude/__init__.py @@ -1,3 +1,3 @@ -from bcbench.agent.claude.agent import get_claude_version, run_claude_code +from bcbench.agent.claude.agent import ClaudeHarness, get_claude_version -__all__ = ["get_claude_version", "run_claude_code"] +__all__ = ["ClaudeHarness", "get_claude_version"] diff --git a/src/bcbench/agent/claude/agent.py b/src/bcbench/agent/claude/agent.py index c04cd5888..82b35ae59 100644 --- a/src/bcbench/agent/claude/agent.py +++ b/src/bcbench/agent/claude/agent.py @@ -1,11 +1,13 @@ import logging import shutil import subprocess +from dataclasses import dataclass from pathlib import Path +from typing import override from bcbench_core.agent.customization import setup_custom_agent, setup_instructions, setup_skills from bcbench_core.agent.experiment import ExperimentConfiguration -from bcbench_core.agent.harness import AgentResult +from bcbench_core.agent.harness import AgentIdentity, AgentRequest, AgentResult, Harness from bcbench_core.agent.metrics import AgentMetrics from bcbench_core.agent.version import get_cli_version from bcbench_core.exceptions import AgentError, AgentTimeoutError @@ -30,131 +32,139 @@ def get_claude_version() -> str: return get_cli_version(shutil.which("claude"), "Claude Code") -def run_claude_code( - entry: BaseDatasetEntry, - model: str, - category: EvaluationCategory, - repo_path: Path, - output_dir: Path, - agent_config: AgentConfig, - plugin_root: Path, - runtime: AgentRuntimeConfig | None = None, - timeout: int = 60 * 60, -) -> AgentResult: - """Run Claude Code on a single dataset entry.""" - claude_cmd = shutil.which("claude") - if not claude_cmd: - raise AgentError("Claude Code not found in PATH. Please ensure it is installed and available.") - - logger.info(f"Running Claude Code on: {entry.instance_id}") - - prompt: str = build_prompt(entry, repo_path, agent_config, category, al_mcp=bool(runtime and runtime.al_mcp)) - bc_gateway = start_bc_mcp_gateway(runtime) - mcp_config_json, mcp_server_names = build_mcp_config( - agent_config, - entry, - repo_path, - runtime=runtime, - bc_mcp_gateway_url=bc_gateway.base_url if bc_gateway else None, - ) - lsp_plugin_dir: Path | None = build_al_lsp_plugin( - entry, - category, - repo_path, - AgentHarness.CLAUDE, - plugin_root, - runtime=runtime, - ) - profile_dir: Path = instructions_profile_dir(entry.customization_profile) - target_dir: Path = AgentHarness.CLAUDE.get_target_dir(repo_path) - instructions_enabled: bool = setup_instructions(agent_config.instructions, profile_dir, target_dir, AgentHarness.CLAUDE.instruction_filename) - skills_enabled: bool = setup_skills(agent_config.skills, profile_dir, target_dir) - custom_agent: str | None = setup_custom_agent(agent_config.agents, profile_dir, target_dir) - plugins: list[tuple[PluginConfig, Path]] = resolve_config_plugins(agent_config, plugin_root, allow_copilot_manifest=False) - - config = ExperimentConfiguration( - mcp_servers=mcp_server_names, - al_lsp_enabled=lsp_plugin_dir is not None, - custom_instructions=instructions_enabled, - skills_enabled=skills_enabled, - custom_agent=custom_agent, - plugins=[plugin.record for plugin, _ in plugins] or None, - ) - - logger.info(f"Executing Claude Code in directory: {repo_path}") - logger.debug(f"Using prompt:\n{prompt}") - - try: - cmd_args = [ - claude_cmd, - "--output-format=stream-json", # emit every event (incl. tool_use, session init) as JSONL - "--verbose", # required for stream-json in --print mode - "--strict-mcp-config", # Only use MCP servers from --mcp-config, ignoring all other MCP configurations - "--setting-sources=project,local", - f"--model={model}", - "--permission-mode=bypassPermissions", # bypassPermissions is needed to use tools and mcp servers - "--disallowedTools", - "WebFetch", - "Bash(curl *)", - "Bash(wget *)", - ] - if mcp_config_json: - cmd_args.append(f"--mcp-config={mcp_config_json}") - if lsp_plugin_dir is not None: - cmd_args.append(f"--plugin-dir={lsp_plugin_dir}") - cmd_args.extend(f"--plugin-dir={plugin_dir}" for _, plugin_dir in plugins) - # --add-dir grants read+write (unlike --plugin-dir, which only registers a plugin), so hand it - # only to plugins that opt in via grant_dir_access - currently a temporary accommodation for - # BCQuality, whose skill reads its own knowledge files at runtime. Enabling a plugin must not - # silently widen the agent's sandbox access. - cmd_args.extend(f"--add-dir={plugin_dir}" for plugin, plugin_dir in plugins if plugin.grant_dir_access) - if custom_agent: - cmd_args.append(f"--agent={custom_agent}") - cmd_args.extend( - [ - "--print", # Non-interactive mode - prompt.replace("\r", "").replace("\n", " "), - ] +@dataclass(frozen=True) +class ClaudeHarness(Harness[BaseDatasetEntry, AgentHarness]): + """Claude Code on any category, customized through the agent config.""" + + model: str + category: EvaluationCategory + agent_config: AgentConfig + plugin_root: Path + runtime: AgentRuntimeConfig | None = None + timeout: int = 60 * 60 + + @override + def identity(self) -> AgentIdentity[AgentHarness]: + return AgentIdentity[AgentHarness](harness=AgentHarness.CLAUDE, version=get_claude_version(), model=self.model) + + @override + def run(self, request: AgentRequest[BaseDatasetEntry], /) -> AgentResult: + entry, repo_path = request.task, request.work_dir + timeout = self.timeout if request.timeout is None else request.timeout + model, category, agent_config, plugin_root, runtime = self.model, self.category, self.agent_config, self.plugin_root, self.runtime + claude_cmd = shutil.which("claude") + if not claude_cmd: + raise AgentError("Claude Code not found in PATH. Please ensure it is installed and available.") + + logger.info(f"Running Claude Code on: {entry.instance_id}") + + prompt: str = build_prompt(entry, repo_path, agent_config, category, al_mcp=bool(runtime and runtime.al_mcp)) + bc_gateway = start_bc_mcp_gateway(runtime) + mcp_config_json, mcp_server_names = build_mcp_config( + agent_config, + entry, + repo_path, + runtime=runtime, + bc_mcp_gateway_url=bc_gateway.base_url if bc_gateway else None, ) - - logger.debug(f"Claude Code command args: {cmd_args}") - - result = subprocess.run( - cmd_args, - cwd=str(repo_path), - env=agent_subprocess_env( - { - "CLAUDE_CODE_DISABLE_AUTO_MEMORY": "1", - # BC MCP's first tools/list compiles the tool catalog and can take ~45s on a cold - # container, well past Claude's 30s default MCP startup timeout -> the server is - # marked "failed" and its tools never register. Raise both the connection and tool - # execution timeouts so the slow first response is tolerated. - "MCP_TIMEOUT": "180000", - "MCP_TOOL_TIMEOUT": "180000", - }, - pass_bc_credentials=category.pass_on_bc_container_credentials, - ), - timeout=timeout, - check=True, - capture_output=True, + lsp_plugin_dir: Path | None = build_al_lsp_plugin( + entry, + category, + repo_path, + AgentHarness.CLAUDE, + plugin_root, + runtime=runtime, + ) + profile_dir: Path = instructions_profile_dir(entry.customization_profile) + target_dir: Path = AgentHarness.CLAUDE.get_target_dir(repo_path) + instructions_enabled: bool = setup_instructions(agent_config.instructions, profile_dir, target_dir, AgentHarness.CLAUDE.instruction_filename) + skills_enabled: bool = setup_skills(agent_config.skills, profile_dir, target_dir) + custom_agent: str | None = setup_custom_agent(agent_config.agents, profile_dir, target_dir) + plugins: list[tuple[PluginConfig, Path]] = resolve_config_plugins(agent_config, plugin_root, allow_copilot_manifest=False) + + config = ExperimentConfiguration( + mcp_servers=mcp_server_names, + al_lsp_enabled=lsp_plugin_dir is not None, + custom_instructions=instructions_enabled, + skills_enabled=skills_enabled, + custom_agent=custom_agent, + plugins=[plugin.record for plugin, _ in plugins] or None, ) - stdout: str = result.stdout.decode("utf-8", errors="replace") if result.stdout else "" - logger.debug(f"Claude Code raw output: {stdout}") - - metrics, response = parse_stream_output(stdout.splitlines(), log_transcript=True) - except subprocess.TimeoutExpired: - logger.exception(f"Claude Code timed out after {timeout} seconds") - metrics = AgentMetrics(execution_time=timeout) - raise AgentTimeoutError("Claude Code timed out", metrics=metrics, config=config) from None - except subprocess.CalledProcessError as e: - logger.exception(f"Claude Code execution failed with error {e.stderr}") - raise AgentError(f"Claude Code execution failed: {e.stderr}") from e - except Exception: - logger.exception("Unexpected error running Claude Code") - raise - else: - return AgentResult(response=response, metrics=metrics, experiment=config) - finally: - if bc_gateway is not None: - bc_gateway.stop() + logger.info(f"Executing Claude Code in directory: {repo_path}") + logger.debug(f"Using prompt:\n{prompt}") + + try: + cmd_args = [ + claude_cmd, + "--output-format=stream-json", # emit every event (incl. tool_use, session init) as JSONL + "--verbose", # required for stream-json in --print mode + "--strict-mcp-config", # Only use MCP servers from --mcp-config, ignoring all other MCP configurations + "--setting-sources=project,local", + f"--model={model}", + "--permission-mode=bypassPermissions", # bypassPermissions is needed to use tools and mcp servers + "--disallowedTools", + "WebFetch", + "Bash(curl *)", + "Bash(wget *)", + ] + if mcp_config_json: + cmd_args.append(f"--mcp-config={mcp_config_json}") + if lsp_plugin_dir is not None: + cmd_args.append(f"--plugin-dir={lsp_plugin_dir}") + cmd_args.extend(f"--plugin-dir={plugin_dir}" for _, plugin_dir in plugins) + # --add-dir grants read+write (unlike --plugin-dir, which only registers a plugin), so hand it + # only to plugins that opt in via grant_dir_access - currently a temporary accommodation for + # BCQuality, whose skill reads its own knowledge files at runtime. Enabling a plugin must not + # silently widen the agent's sandbox access. + cmd_args.extend(f"--add-dir={plugin_dir}" for plugin, plugin_dir in plugins if plugin.grant_dir_access) + if custom_agent: + cmd_args.append(f"--agent={custom_agent}") + cmd_args.extend( + [ + "--print", # Non-interactive mode + prompt.replace("\r", "").replace("\n", " "), + ] + ) + + logger.debug(f"Claude Code command args: {cmd_args}") + + result = subprocess.run( + cmd_args, + cwd=str(repo_path), + env=agent_subprocess_env( + { + "CLAUDE_CODE_DISABLE_AUTO_MEMORY": "1", + # BC MCP's first tools/list compiles the tool catalog and can take ~45s on a cold + # container, well past Claude's 30s default MCP startup timeout -> the server is + # marked "failed" and its tools never register. Raise both the connection and tool + # execution timeouts so the slow first response is tolerated. + "MCP_TIMEOUT": "180000", + "MCP_TOOL_TIMEOUT": "180000", + }, + pass_bc_credentials=category.pass_on_bc_container_credentials, + ), + timeout=timeout, + check=True, + capture_output=True, + ) + + stdout: str = result.stdout.decode("utf-8", errors="replace") if result.stdout else "" + logger.debug(f"Claude Code raw output: {stdout}") + + metrics, response = parse_stream_output(stdout.splitlines(), log_transcript=True) + except subprocess.TimeoutExpired: + logger.exception(f"Claude Code timed out after {timeout} seconds") + metrics = AgentMetrics(execution_time=timeout) + raise AgentTimeoutError("Claude Code timed out", metrics=metrics, config=config) from None + except subprocess.CalledProcessError as e: + logger.exception(f"Claude Code execution failed with error {e.stderr}") + raise AgentError(f"Claude Code execution failed: {e.stderr}") from e + except Exception: + logger.exception("Unexpected error running Claude Code") + raise + else: + return AgentResult(response=response, metrics=metrics, experiment=config) + finally: + if bc_gateway is not None: + bc_gateway.stop() diff --git a/src/bcbench/agent/pr_review/__init__.py b/src/bcbench/agent/pr_review/__init__.py index f33c0bffc..e07376cad 100644 --- a/src/bcbench/agent/pr_review/__init__.py +++ b/src/bcbench/agent/pr_review/__init__.py @@ -1,3 +1,3 @@ -from bcbench.agent.pr_review.agent import get_pr_review_version, run_pr_review_agent +from bcbench.agent.pr_review.agent import PRReviewHarness, get_pr_review_version -__all__ = ["get_pr_review_version", "run_pr_review_agent"] +__all__ = ["PRReviewHarness", "get_pr_review_version"] diff --git a/src/bcbench/agent/pr_review/agent.py b/src/bcbench/agent/pr_review/agent.py index 87f62f81d..ecc6e9a15 100644 --- a/src/bcbench/agent/pr_review/agent.py +++ b/src/bcbench/agent/pr_review/agent.py @@ -17,21 +17,21 @@ import shutil import subprocess import time +from dataclasses import dataclass from pathlib import Path -from typing import Any +from typing import Any, override import yaml from bcbench_core.agent.experiment import ExperimentConfiguration -from bcbench_core.agent.harness import AgentResult +from bcbench_core.agent.harness import AgentIdentity, AgentRequest, AgentResult, Harness from bcbench_core.exceptions import AgentError, AgentTimeoutError from bcbench_core.git import commit_changes, has_changes, init_repo from bcbench.agent.pr_review.metrics import build_pr_review_metrics from bcbench.agent.pr_review.review_output import engine_report_to_review_comments, load_engine_report -from bcbench.dataset import BaseDatasetEntry from bcbench.dataset.codereview import CodeReviewEntry from bcbench.paths import SHARED_CONFIG_FILE -from bcbench.types import EvaluationCategory, PRReviewMetrics +from bcbench.types import AgentHarness, PRReviewMetrics logger = logging.getLogger(__name__) @@ -153,84 +153,86 @@ def _write_review_json(output_dir: Path, repo_path: Path) -> int: return len(comments) -def run_pr_review_agent( - entry: BaseDatasetEntry, - model: str, - category: EvaluationCategory, - repo_path: Path, - output_dir: Path, - engine_path: Path | None = None, - min_severity: str | None = None, - timeout: int = 60 * 60, -) -> AgentResult: - """Run the engine's complete local review pipeline and write review.json. - - Separate from run_copilot_agent by design: this spawns the PROD BC-ALAgents - PowerShell orchestrator (Copilot is spawned inside the engine, not here), so it - owns none of the copilot-harness prompt/MCP/LSP wiring and takes engine-specific - inputs (BC-ALAgents source, min severity) for the code-review category only. +@dataclass(frozen=True) +class PRReviewHarness(Harness[CodeReviewEntry, AgentHarness]): + """The production BC-ALAgents review engine, run in local mode on code-review entries. + + Separate from CopilotHarness by design: this spawns the PROD BC-ALAgents PowerShell + orchestrator (Copilot is spawned inside the engine, not here), so it owns none of the + copilot-harness prompt/MCP/LSP wiring and takes engine-specific inputs instead. """ - if category is not EvaluationCategory.CODE_REVIEW: - raise AgentError(f"The engine agent only supports the code-review category, got {category.value}.") - if not isinstance(entry, CodeReviewEntry): - raise AgentError(f"The engine agent requires a CodeReviewEntry, got {type(entry).__name__}.") - - repo_path = repo_path.resolve() - output_dir = output_dir.resolve() - settings = _load_pr_review_settings() - engine_root = _resolve_pr_review_root(engine_path) - pwsh = _resolve_pwsh() - severity = min_severity or settings["min_severity"] - output_dir.mkdir(parents=True, exist_ok=True) - logger.info(f"Running BC-ALAgents review engine on: {entry.instance_id}") - - _commit_patch_as_head(repo_path) - trusted_workspace = _init_trusted_workspace(output_dir / "trusted") - bcquality_root = _prepare_bcquality_root(engine_root, pwsh, output_dir / "bcquality") - - engine = engine_root / "agents" / "ALReviewAgent" / "scripts" / "Invoke-CopilotPRReview.ps1" - env = { - **_environment_without_bcquality_overrides(), - "REVIEW_SOURCE": "local", - "REVIEW_PHASE": "all", - "BASE_REF": entry.base_commit, - "REVIEW_TARGET_WORKSPACE": str(repo_path), - "REVIEW_WORKSPACE": str(trusted_workspace), - "REVIEW_OUTPUT_DIR": str(output_dir), - "BCQUALITY_ROOT": str(bcquality_root), - "GITHUB_REPOSITORY": entry.repo, - "COPILOT_MODEL": model, - "AGENT_MINIMUM_SEVERITY": severity, - } - - config = ExperimentConfiguration() - - start = time.monotonic() - try: - result = subprocess.run( - [pwsh, "-NoProfile", "-File", str(engine)], - cwd=str(repo_path), - env=env, - capture_output=True, - text=True, - encoding="utf-8", - timeout=timeout, - check=True, - ) - logger.debug(f"Engine stdout:\n{result.stdout}") - if result.stderr: - logger.debug(f"Engine stderr:\n{result.stderr}") - count = _write_review_json(output_dir, repo_path) - logger.info(f"Engine review complete for {entry.instance_id}: wrote {count} comment(s) to {_REVIEW_OUTPUT_FILE}") - except subprocess.TimeoutExpired: - logger.exception(f"Engine review timed out after {timeout} seconds") - metrics = PRReviewMetrics(execution_time=timeout) - raise AgentTimeoutError("Engine review timed out", metrics=metrics, config=config) from None - except subprocess.CalledProcessError as e: - logger.exception(f"Engine review failed (exit {e.returncode}):\n{e.stdout}\n{e.stderr}") - raise AgentError(f"Engine review execution failed: {e}") from None - except Exception: - logger.exception("Unexpected error running engine review") - raise - else: - return AgentResult(metrics=build_pr_review_metrics(output_dir, time.monotonic() - start), experiment=config) + + model: str + engine_path: Path | None = None + min_severity: str | None = None + timeout: int = 60 * 60 + + @override + def identity(self) -> AgentIdentity[AgentHarness]: + return AgentIdentity[AgentHarness](harness=AgentHarness.PR_REVIEW, version=get_pr_review_version(self.engine_path), model=self.model) + + @override + def run(self, request: AgentRequest[CodeReviewEntry], /) -> AgentResult: + """Run the engine's complete local review pipeline and write review.json.""" + entry, repo_path, output_dir = request.task, request.work_dir, request.output_dir + timeout = self.timeout if request.timeout is None else request.timeout + model, engine_path, min_severity = self.model, self.engine_path, self.min_severity + repo_path = repo_path.resolve() + output_dir = output_dir.resolve() + settings = _load_pr_review_settings() + engine_root = _resolve_pr_review_root(engine_path) + pwsh = _resolve_pwsh() + severity = min_severity or settings["min_severity"] + output_dir.mkdir(parents=True, exist_ok=True) + logger.info(f"Running BC-ALAgents review engine on: {entry.instance_id}") + + _commit_patch_as_head(repo_path) + trusted_workspace = _init_trusted_workspace(output_dir / "trusted") + bcquality_root = _prepare_bcquality_root(engine_root, pwsh, output_dir / "bcquality") + + engine = engine_root / "agents" / "ALReviewAgent" / "scripts" / "Invoke-CopilotPRReview.ps1" + env = { + **_environment_without_bcquality_overrides(), + "REVIEW_SOURCE": "local", + "REVIEW_PHASE": "all", + "BASE_REF": entry.base_commit, + "REVIEW_TARGET_WORKSPACE": str(repo_path), + "REVIEW_WORKSPACE": str(trusted_workspace), + "REVIEW_OUTPUT_DIR": str(output_dir), + "BCQUALITY_ROOT": str(bcquality_root), + "GITHUB_REPOSITORY": entry.repo, + "COPILOT_MODEL": model, + "AGENT_MINIMUM_SEVERITY": severity, + } + + config = ExperimentConfiguration() + + start = time.monotonic() + try: + result = subprocess.run( + [pwsh, "-NoProfile", "-File", str(engine)], + cwd=str(repo_path), + env=env, + capture_output=True, + text=True, + encoding="utf-8", + timeout=timeout, + check=True, + ) + logger.debug(f"Engine stdout:\n{result.stdout}") + if result.stderr: + logger.debug(f"Engine stderr:\n{result.stderr}") + count = _write_review_json(output_dir, repo_path) + logger.info(f"Engine review complete for {entry.instance_id}: wrote {count} comment(s) to {_REVIEW_OUTPUT_FILE}") + except subprocess.TimeoutExpired: + logger.exception(f"Engine review timed out after {timeout} seconds") + metrics = PRReviewMetrics(execution_time=timeout) + raise AgentTimeoutError("Engine review timed out", metrics=metrics, config=config) from None + except subprocess.CalledProcessError as e: + logger.exception(f"Engine review failed (exit {e.returncode}):\n{e.stdout}\n{e.stderr}") + raise AgentError(f"Engine review execution failed: {e}") from None + except Exception: + logger.exception("Unexpected error running engine review") + raise + else: + return AgentResult(metrics=build_pr_review_metrics(output_dir, time.monotonic() - start), experiment=config) diff --git a/src/bcbench/commands/evaluate.py b/src/bcbench/commands/evaluate.py index 925249d03..bd021efcd 100644 --- a/src/bcbench/commands/evaluate.py +++ b/src/bcbench/commands/evaluate.py @@ -5,12 +5,12 @@ import typer from bcbench_core.agent.experiment import ExperimentConfiguration -from bcbench_core.agent.harness import AgentIdentity, AgentResult +from bcbench_core.agent.harness import AgentIdentity, AgentRequest, AgentResult, Harness from bcbench_core.agent.metrics import AgentMetrics from bcbench_core.exceptions import AgentError from bcbench_core.filesystem import prepare_run_dir -from bcbench.agent import BCalBackendConfig, CopilotHarness, get_bcal_version, get_claude_version, get_pr_review_version, run_bcal_agent, run_claude_code, run_pr_review_agent +from bcbench.agent import BCalBackendConfig, BCalHarness, ClaudeHarness, CopilotHarness, PRReviewHarness from bcbench.cli_options import ( AgentConfigPath, ClaudeCodeModel, @@ -33,7 +33,7 @@ from bcbench.config import get_config from bcbench.dataset import BaseDatasetEntry, NL2ALEntry from bcbench.dataset.codereview import CodeReviewEntry -from bcbench.evaluate import AgentRunner, CodeReviewPipeline, EvaluationPipeline, NL2ALPipeline, harness_runner +from bcbench.evaluate import CodeReviewPipeline, EvaluationPipeline, NL2ALPipeline from bcbench.evaluate.codereview_judge_calibration import run_calibration from bcbench.paths import SHARED_CONFIG_FILE from bcbench.results import BaseEvaluationResult, CodeReviewResult, ExecutionBasedEvaluationResult, JudgeBasedEvaluationResult @@ -102,7 +102,7 @@ def evaluate_copilot( ) pipeline = category.pipeline(agent_config) - pipeline.execute(context, harness_runner(harness)) + pipeline.execute(context, harness) logger.info("Evaluation complete!") logger.info(f"Results saved to: {run_dir}") @@ -149,6 +149,7 @@ def evaluate_claude_code( ) entry = category.entry_class.load(category.dataset_path(tier), entry_id=entry_id)[0] agent_config = AgentConfig.from_file(config_path) + harness = ClaudeHarness(model=model, category=category, agent_config=agent_config, plugin_root=_config.paths.plugin_root, runtime=runtime) run_dir = prepare_run_dir(output_dir, run_id) logger.info(f"Running evaluation on entry {entry_id} with Claude Code") @@ -158,25 +159,13 @@ def evaluate_claude_code( repo_path=repo_path, result_dir=run_dir, container=runtime.container if runtime else None, - agent=AgentIdentity[AgentHarness](harness=AgentHarness.CLAUDE, version=get_claude_version(), model=model), + agent=harness.identity(), category=category, tier=tier, ) pipeline = category.pipeline(agent_config) - pipeline.execute( - context, - lambda ctx: run_claude_code( - entry=ctx.entry, - repo_path=ctx.repo_path, - category=category, - model=model, - output_dir=ctx.result_dir, - agent_config=agent_config, - plugin_root=_config.paths.plugin_root, - runtime=runtime, - ), - ) + pipeline.execute(context, harness) logger.info("Evaluation complete!") logger.info(f"Results saved to: {run_dir}") @@ -204,6 +193,7 @@ def evaluate_pr_review( """ category = EvaluationCategory.CODE_REVIEW entry = CodeReviewEntry.load(category.dataset_path(), entry_id=entry_id)[0] + harness = PRReviewHarness(model=model, engine_path=engine_path) run_dir = prepare_run_dir(output_dir, run_id) logger.info(f"Running evaluation on entry {entry_id} with the BC-ALAgents review engine") @@ -213,21 +203,11 @@ def evaluate_pr_review( repo_path=repo_path, result_dir=run_dir, container=None, - agent=AgentIdentity[AgentHarness](harness=AgentHarness.PR_REVIEW, version=get_pr_review_version(engine_path), model=model), + agent=harness.identity(), category=category, ) - CodeReviewPipeline().execute( - context, - lambda ctx: run_pr_review_agent( - entry=ctx.entry, - repo_path=ctx.repo_path, - category=category, - model=model, - output_dir=ctx.result_dir, - engine_path=engine_path, - ), - ) + CodeReviewPipeline().execute(context, harness) logger.info("Evaluation complete!") logger.info(f"Results saved to: {run_dir}") @@ -251,10 +231,7 @@ def evaluate_bcal( category = EvaluationCategory.NL2AL entry: NL2ALEntry = cast(NL2ALEntry, category.entry_class.load(category.dataset_path(tier), entry_id=entry_id)[0]) run_dir = prepare_run_dir(output_dir, run_id) - backend_config = BCalBackendConfig( - command=llm_command, - model=llm_model, - ) + harness = BCalHarness(BCalBackendConfig(command=llm_command, model=llm_model)) logger.info(f"Running evaluation on entry {entry_id} with BCal") @@ -263,21 +240,14 @@ def evaluate_bcal( repo_path=repo_path, result_dir=run_dir, container=None, - agent=AgentIdentity[AgentHarness](harness=AgentHarness.BCAL, version=get_bcal_version(), model=llm_model or "external-command"), + agent=harness.identity(), category=category, tier=tier, ) pipeline = NL2ALPipeline() try: - pipeline.execute( - context, - lambda ctx: run_bcal_agent( - entry=cast(NL2ALEntry, ctx.entry), - repo_path=ctx.repo_path, - backend_config=backend_config, - ), - ) + pipeline.execute(context, harness) except AgentError as exc: result = JudgeBasedEvaluationResult.create_failure(context, output="", error_message=str(exc)) pipeline.save_result(context, result) @@ -333,18 +303,28 @@ def evaluate_mock( entry=entry, repo_path=Path(), result_dir=run_dir, - agent=AgentIdentity[AgentHarness](harness=AgentHarness.MOCK, version="mock", model="mock-model"), + agent=MockHarness().identity(), category=category, tier=tier, ) pipeline = MockEvaluationPipeline() - pipeline.execute(context, lambda _ctx: AgentResult()) + pipeline.execute(context, MockHarness()) logger.info("Mock evaluation complete!") logger.info(f"Results saved to: {run_dir}") +class MockHarness(Harness[BaseDatasetEntry, AgentHarness]): + @override + def identity(self) -> AgentIdentity[AgentHarness]: + return AgentIdentity[AgentHarness](harness=AgentHarness.MOCK, version="mock", model="mock-model") + + @override + def run(self, request: AgentRequest[BaseDatasetEntry], /) -> AgentResult: + return AgentResult() + + class MockEvaluationPipeline(EvaluationPipeline[BaseDatasetEntry]): """Mock pipeline for testing evaluation infrastructure. @@ -361,7 +341,7 @@ def setup(self, context: EvaluationContext[BaseDatasetEntry]) -> None: logger.info("Mock pipeline: Skipping setup") @override - def run_agent(self, context: EvaluationContext[BaseDatasetEntry], agent_runner: AgentRunner[BaseDatasetEntry]) -> None: + def run_agent(self, context: EvaluationContext[BaseDatasetEntry], harness: Harness[BaseDatasetEntry, AgentHarness]) -> None: """Generate random agent metrics and experiment configuration.""" logger.info("Mock pipeline: Generating random metrics and experiment configuration") diff --git a/src/bcbench/commands/run.py b/src/bcbench/commands/run.py index c259c52fe..8c04bf7bf 100644 --- a/src/bcbench/commands/run.py +++ b/src/bcbench/commands/run.py @@ -6,7 +6,7 @@ import typer from bcbench_core.agent.harness import AgentRequest -from bcbench.agent import BCalBackendConfig, CopilotHarness, run_bcal_agent, run_claude_code, run_pr_review_agent +from bcbench.agent import BCalBackendConfig, BCalHarness, ClaudeHarness, CopilotHarness, PRReviewHarness from bcbench.cli_options import ( AgentConfigPath, ClaudeCodeModel, @@ -130,16 +130,8 @@ def run_claude( agent_config = AgentConfig.from_file(config_path) category.pipeline(agent_config).setup_workspace(entry, repo_path) - run_claude_code( - entry=entry, - repo_path=repo_path, - model=model, - category=category, - output_dir=output_dir, - agent_config=agent_config, - plugin_root=_config.paths.plugin_root, - runtime=runtime, - ) + harness = ClaudeHarness(model=model, category=category, agent_config=agent_config, plugin_root=_config.paths.plugin_root, runtime=runtime) + harness.run(AgentRequest(task=entry, work_dir=repo_path, output_dir=output_dir)) @run_app.command("pr-review") @@ -168,15 +160,8 @@ def run_pr_review( entry = CodeReviewEntry.load(category.dataset_path(), entry_id=entry_id)[0] CodeReviewPipeline().setup_workspace(entry, repo_path) - run_pr_review_agent( - entry=entry, - model=model, - repo_path=repo_path, - category=category, - output_dir=output_dir, - engine_path=engine_path, - min_severity=min_severity, - ) + harness = PRReviewHarness(model=model, engine_path=engine_path, min_severity=min_severity) + harness.run(AgentRequest(task=entry, work_dir=repo_path, output_dir=output_dir)) @run_app.command("bcal") @@ -199,11 +184,5 @@ def run_bcal( entry: NL2ALEntry = cast(NL2ALEntry, category.entry_class.load(category.dataset_path(tier), entry_id=entry_id)[0]) NL2ALPipeline().setup_workspace(entry, repo_path) - run_bcal_agent( - entry=entry, - repo_path=repo_path, - backend_config=BCalBackendConfig( - command=llm_command, - model=llm_model, - ), - ) + harness = BCalHarness(BCalBackendConfig(command=llm_command, model=llm_model)) + harness.run(AgentRequest(task=entry, work_dir=repo_path, output_dir=repo_path)) diff --git a/src/bcbench/evaluate/__init__.py b/src/bcbench/evaluate/__init__.py index 49ee8eb15..2aab4fb48 100644 --- a/src/bcbench/evaluate/__init__.py +++ b/src/bcbench/evaluate/__init__.py @@ -1,6 +1,6 @@ """Evaluation module for running pipelines and creating results.""" -from bcbench.evaluate.base import AgentRunner, EvaluationPipeline, harness_runner +from bcbench.evaluate.base import EvaluationPipeline from bcbench.evaluate.bugfix import BugFixPipeline from bcbench.evaluate.codereview import CodeReviewPipeline from bcbench.evaluate.dataquery import DataQueryPipeline @@ -11,7 +11,6 @@ from bcbench.evaluate.testgeneration import TestGenerationPipeline __all__ = [ - "AgentRunner", "BugFixPipeline", "CodeReviewPipeline", "DataQueryPipeline", @@ -21,5 +20,4 @@ "ExtRequestTriagePipeline", "NL2ALPipeline", "TestGenerationPipeline", - "harness_runner", ] diff --git a/src/bcbench/evaluate/base.py b/src/bcbench/evaluate/base.py index 8b037a6b2..0214f2702 100644 --- a/src/bcbench/evaluate/base.py +++ b/src/bcbench/evaluate/base.py @@ -3,9 +3,8 @@ import logging from abc import ABC, abstractmethod from pathlib import Path -from typing import Protocol -from bcbench_core.agent.harness import AgentRequest, AgentResult, Harness +from bcbench_core.agent.harness import Harness from bcbench_core.exceptions import AgentTimeoutError from bcbench.dataset import BaseDatasetEntry @@ -15,20 +14,7 @@ logger = logging.getLogger(__name__) -__all__ = ["AgentRunner", "EvaluationPipeline", "harness_runner"] - - -class AgentRunner[E: BaseDatasetEntry](Protocol): - def __call__(self, context: EvaluationContext[E], /) -> AgentResult: ... - - -def harness_runner[E: BaseDatasetEntry](harness: Harness[E, AgentHarness]) -> AgentRunner[E]: - """Adapt ``harness`` to the pipelines' ``AgentRunner`` until every harness implements ``Harness``.""" - - def run(context: EvaluationContext[E], /) -> AgentResult: - return harness.run(AgentRequest(task=context.entry, work_dir=context.repo_path, output_dir=context.result_dir)) - - return run +__all__ = ["EvaluationPipeline"] class EvaluationPipeline[E: BaseDatasetEntry](ABC): @@ -59,12 +45,12 @@ def setup(self, context: EvaluationContext[E]) -> None: raise NotImplementedError @abstractmethod - def run_agent(self, context: EvaluationContext[E], agent_runner: AgentRunner[E]) -> None: + def run_agent(self, context: EvaluationContext[E], harness: Harness[E, AgentHarness]) -> None: """Run the agent and capture metrics. Args: context: Evaluation context with configuration - agent_runner: Function that runs the specific agent + harness: The agent harness to run Raises: Exception: If agent execution fails @@ -88,7 +74,7 @@ def evaluate(self, context: EvaluationContext[E]) -> None: def execute( self, context: EvaluationContext[E], - agent_runner: AgentRunner[E], + harness: Harness[E, AgentHarness], ) -> None: """Template method orchestrating the evaluation flow. @@ -97,12 +83,12 @@ def execute( Args: context: Evaluation context with configuration - agent_runner: Function that runs the specific agent and returns (AgentMetrics, ExperimentConfiguration) + harness: The agent harness to run """ self.setup(context) try: - self.run_agent(context, agent_runner) + self.run_agent(context, harness) except AgentTimeoutError as e: context.metrics = e.metrics context.experiment = e.config diff --git a/src/bcbench/evaluate/bugfix.py b/src/bcbench/evaluate/bugfix.py index 8dded4c63..136d17f7d 100644 --- a/src/bcbench/evaluate/bugfix.py +++ b/src/bcbench/evaluate/bugfix.py @@ -2,17 +2,18 @@ from pathlib import Path from typing import override +from bcbench_core.agent.harness import Harness from bcbench_core.bc import build_and_publish_projects from bcbench_core.exceptions import BuildError, TestExecutionError from bcbench_core.git import apply_patch, clean_project_paths, stage_and_get_diff from bcbench_core.projects import categorize_projects from bcbench.dataset import BugFixEntry -from bcbench.evaluate.base import AgentRunner, EvaluationPipeline +from bcbench.evaluate.base import EvaluationPipeline from bcbench.github_actions import github_log_group from bcbench.operations import copy_problem_statement_folder, run_tests, set_runtime_version, setup_repo_prebuild from bcbench.results.bugfix import BugFixResult -from bcbench.types import EvaluationContext +from bcbench.types import AgentHarness, EvaluationContext logger = logging.getLogger(__name__) @@ -43,9 +44,9 @@ def setup(self, context: EvaluationContext[BugFixEntry]) -> None: set_runtime_version(context.repo_path, context.entry.project_paths) @override - def run_agent(self, context: EvaluationContext[BugFixEntry], agent_runner: AgentRunner[BugFixEntry]) -> None: + def run_agent(self, context: EvaluationContext[BugFixEntry], harness: Harness[BugFixEntry, AgentHarness]) -> None: with github_log_group(f"{context.agent.harness} -- Entry: {context.entry.instance_id}"): - result = agent_runner(context) + result = harness.run(context.agent_request()) context.metrics, context.experiment = result.metrics, result.experiment @override diff --git a/src/bcbench/evaluate/codereview.py b/src/bcbench/evaluate/codereview.py index 7fa87f7f8..5b9765f20 100644 --- a/src/bcbench/evaluate/codereview.py +++ b/src/bcbench/evaluate/codereview.py @@ -3,16 +3,17 @@ from pathlib import Path from typing import override +from bcbench_core.agent.harness import Harness from bcbench_core.git import apply_patch, fetch_commit_if_missing from bcbench.dataset.codereview import CodeReviewEntry, ReviewComment -from bcbench.evaluate.base import AgentRunner, EvaluationPipeline +from bcbench.evaluate.base import EvaluationPipeline from bcbench.evaluate.codereview_judge import judge_expected_and_ignored from bcbench.evaluate.review_parsing import parse_review_output from bcbench.github_actions import github_log_group from bcbench.operations import setup_repo_prebuild from bcbench.results.codereview import CodeReviewResult, candidate_comment_pairs -from bcbench.types import EvaluationContext +from bcbench.types import AgentHarness, EvaluationContext logger = logging.getLogger(__name__) @@ -55,9 +56,9 @@ def setup(self, context: EvaluationContext[CodeReviewEntry]) -> None: self.setup_workspace(context.entry, context.repo_path) @override - def run_agent(self, context: EvaluationContext[CodeReviewEntry], agent_runner: AgentRunner[CodeReviewEntry]) -> None: + def run_agent(self, context: EvaluationContext[CodeReviewEntry], harness: Harness[CodeReviewEntry, AgentHarness]) -> None: with github_log_group(f"{context.agent.harness} -- Entry: {context.entry.instance_id}"): - result = agent_runner(context) + result = harness.run(context.agent_request()) context.metrics, context.experiment = result.metrics, result.experiment @override diff --git a/src/bcbench/evaluate/dataquery.py b/src/bcbench/evaluate/dataquery.py index 2665e1a5a..0a6a222ee 100644 --- a/src/bcbench/evaluate/dataquery.py +++ b/src/bcbench/evaluate/dataquery.py @@ -5,14 +5,15 @@ from pathlib import Path from typing import override +from bcbench_core.agent.harness import Harness from bcbench_core.filesystem import clear_directory from bcbench.dataset import DataQueryEntry -from bcbench.evaluate.base import AgentRunner, EvaluationPipeline +from bcbench.evaluate.base import EvaluationPipeline from bcbench.exceptions import EmptyGoldResultError from bcbench.github_actions import github_log_group from bcbench.results.base import ExecutionBasedEvaluationResult -from bcbench.types import EvaluationContext +from bcbench.types import AgentHarness, EvaluationContext logger = logging.getLogger(__name__) @@ -105,9 +106,9 @@ def setup(self, context: EvaluationContext[DataQueryEntry]) -> None: self.setup_workspace(context.entry, context.repo_path) @override - def run_agent(self, context: EvaluationContext[DataQueryEntry], agent_runner: AgentRunner[DataQueryEntry]) -> None: + def run_agent(self, context: EvaluationContext[DataQueryEntry], harness: Harness[DataQueryEntry, AgentHarness]) -> None: with github_log_group(f"{context.agent.harness} -- Entry: {context.entry.instance_id}"): - result = agent_runner(context) + result = harness.run(context.agent_request()) context.metrics, context.experiment = result.metrics, result.experiment @override diff --git a/src/bcbench/evaluate/ext_request_advisor.py b/src/bcbench/evaluate/ext_request_advisor.py index 58e0e23ad..ea2d4a67d 100644 --- a/src/bcbench/evaluate/ext_request_advisor.py +++ b/src/bcbench/evaluate/ext_request_advisor.py @@ -2,12 +2,14 @@ from pathlib import Path from typing import override +from bcbench_core.agent.harness import Harness + from bcbench.dataset import ExtRequestAdvisorEntry -from bcbench.evaluate.base import AgentRunner, EvaluationPipeline +from bcbench.evaluate.base import EvaluationPipeline from bcbench.github_actions import github_log_group from bcbench.operations import setup_repo_prebuild from bcbench.results.base import JudgeBasedEvaluationResult -from bcbench.types import EvaluationContext +from bcbench.types import AgentHarness, EvaluationContext logger = logging.getLogger(__name__) @@ -29,9 +31,9 @@ def setup(self, context: EvaluationContext[ExtRequestAdvisorEntry]) -> None: self.setup_workspace(context.entry, context.repo_path) @override - def run_agent(self, context: EvaluationContext[ExtRequestAdvisorEntry], agent_runner: AgentRunner[ExtRequestAdvisorEntry]) -> None: + def run_agent(self, context: EvaluationContext[ExtRequestAdvisorEntry], harness: Harness[ExtRequestAdvisorEntry, AgentHarness]) -> None: with github_log_group(f"{context.agent.harness} -- Entry: {context.entry.instance_id}"): - result = agent_runner(context) + result = harness.run(context.agent_request()) context.metrics, context.experiment = result.metrics, result.experiment @override diff --git a/src/bcbench/evaluate/ext_request_implement.py b/src/bcbench/evaluate/ext_request_implement.py index d439d9aee..fa5792e7a 100644 --- a/src/bcbench/evaluate/ext_request_implement.py +++ b/src/bcbench/evaluate/ext_request_implement.py @@ -2,15 +2,16 @@ from pathlib import Path from typing import override +from bcbench_core.agent.harness import Harness from bcbench_core.exceptions import EmptyDiffError from bcbench_core.git import stage_and_get_diff from bcbench.dataset import ExtRequestImplementEntry -from bcbench.evaluate.base import AgentRunner, EvaluationPipeline +from bcbench.evaluate.base import EvaluationPipeline from bcbench.github_actions import github_log_group from bcbench.operations import copy_problem_statement_folder, set_runtime_version, setup_repo_prebuild from bcbench.results.base import JudgeBasedEvaluationResult -from bcbench.types import EvaluationContext +from bcbench.types import AgentHarness, EvaluationContext logger = logging.getLogger(__name__) @@ -36,9 +37,9 @@ def setup(self, context: EvaluationContext[ExtRequestImplementEntry]) -> None: self.setup_workspace(context.entry, context.repo_path) @override - def run_agent(self, context: EvaluationContext[ExtRequestImplementEntry], agent_runner: AgentRunner[ExtRequestImplementEntry]) -> None: + def run_agent(self, context: EvaluationContext[ExtRequestImplementEntry], harness: Harness[ExtRequestImplementEntry, AgentHarness]) -> None: with github_log_group(f"{context.agent.harness} -- Entry: {context.entry.instance_id}"): - result = agent_runner(context) + result = harness.run(context.agent_request()) context.metrics, context.experiment = result.metrics, result.experiment @override diff --git a/src/bcbench/evaluate/ext_request_triage.py b/src/bcbench/evaluate/ext_request_triage.py index 5366bef71..29a25eeb2 100644 --- a/src/bcbench/evaluate/ext_request_triage.py +++ b/src/bcbench/evaluate/ext_request_triage.py @@ -12,12 +12,14 @@ from pathlib import Path from typing import override +from bcbench_core.agent.harness import Harness + from bcbench.dataset import ExtRequestTriageEntry -from bcbench.evaluate.base import AgentRunner, EvaluationPipeline +from bcbench.evaluate.base import EvaluationPipeline from bcbench.github_actions import github_log_group from bcbench.operations import setup_repo_prebuild from bcbench.results.base import JudgeBasedEvaluationResult -from bcbench.types import EvaluationContext +from bcbench.types import AgentHarness, EvaluationContext logger = logging.getLogger(__name__) @@ -39,9 +41,9 @@ def setup(self, context: EvaluationContext[ExtRequestTriageEntry]) -> None: self.setup_workspace(context.entry, context.repo_path) @override - def run_agent(self, context: EvaluationContext[ExtRequestTriageEntry], agent_runner: AgentRunner[ExtRequestTriageEntry]) -> None: + def run_agent(self, context: EvaluationContext[ExtRequestTriageEntry], harness: Harness[ExtRequestTriageEntry, AgentHarness]) -> None: with github_log_group(f"{context.agent.harness} -- Entry: {context.entry.instance_id}"): - result = agent_runner(context) + result = harness.run(context.agent_request()) context.metrics, context.experiment = result.metrics, result.experiment @override diff --git a/src/bcbench/evaluate/nl2al.py b/src/bcbench/evaluate/nl2al.py index 40cb7a99d..f841797dc 100644 --- a/src/bcbench/evaluate/nl2al.py +++ b/src/bcbench/evaluate/nl2al.py @@ -4,16 +4,17 @@ from pathlib import Path from typing import override +from bcbench_core.agent.harness import Harness from bcbench_core.artifacts import copy_symbol_apps from bcbench_core.exceptions import EmptyDiffError from bcbench_core.filesystem import remove_tree from bcbench_core.git import stage_and_get_diff from bcbench.dataset import NL2ALEntry -from bcbench.evaluate.base import AgentRunner, EvaluationPipeline +from bcbench.evaluate.base import EvaluationPipeline from bcbench.github_actions import github_log_group from bcbench.results.base import JudgeBasedEvaluationResult -from bcbench.types import EvaluationContext +from bcbench.types import AgentHarness, EvaluationContext logger = logging.getLogger(__name__) @@ -68,11 +69,11 @@ def setup(self, context: EvaluationContext[NL2ALEntry]) -> None: self.setup_workspace(context.entry, context.repo_path) @override - def run_agent(self, context: EvaluationContext[NL2ALEntry], agent_runner: AgentRunner[NL2ALEntry]) -> None: + def run_agent(self, context: EvaluationContext[NL2ALEntry], harness: Harness[NL2ALEntry, AgentHarness]) -> None: # Single attempt — retries are disabled. An empty diff (the agent asked for clarification # instead of editing) is scored as a failure in evaluate(), not re-run. with github_log_group(f"{context.agent.harness} -- Entry: {context.entry.instance_id}"): - result = agent_runner(context) + result = harness.run(context.agent_request()) context.metrics, context.experiment = result.metrics, result.experiment @override diff --git a/src/bcbench/evaluate/testgeneration.py b/src/bcbench/evaluate/testgeneration.py index 12e7704d9..1eaa141f6 100644 --- a/src/bcbench/evaluate/testgeneration.py +++ b/src/bcbench/evaluate/testgeneration.py @@ -2,6 +2,7 @@ from pathlib import Path from typing import override +from bcbench_core.agent.harness import Harness from bcbench_core.bc import build_and_publish_projects, run_test_suite from bcbench_core.dataset import TestEntry from bcbench_core.exceptions import BuildError, TestExecutionError @@ -10,13 +11,13 @@ from bcbench.collection.patch_utils import extract_file_paths_from_patch from bcbench.dataset import TestGenEntry -from bcbench.evaluate.base import AgentRunner, EvaluationPipeline +from bcbench.evaluate.base import EvaluationPipeline from bcbench.exceptions import NoTestsExtractedError from bcbench.github_actions import github_log_group from bcbench.operations import copy_problem_statement_folder, extract_tests_from_patch, setup_repo_prebuild from bcbench.operations.setup_operations import set_runtime_version from bcbench.results.testgeneration import TestGenerationResult -from bcbench.types import EvaluationContext, TestGenerationInput +from bcbench.types import AgentHarness, EvaluationContext, TestGenerationInput logger = logging.getLogger(__name__) @@ -65,9 +66,9 @@ def setup(self, context: EvaluationContext[TestGenEntry]) -> None: set_runtime_version(context.repo_path, context.entry.project_paths) @override - def run_agent(self, context: EvaluationContext[TestGenEntry], agent_runner: AgentRunner[TestGenEntry]) -> None: + def run_agent(self, context: EvaluationContext[TestGenEntry], harness: Harness[TestGenEntry, AgentHarness]) -> None: with github_log_group(f"{context.agent.harness} -- Entry: {context.entry.instance_id}"): - result = agent_runner(context) + result = harness.run(context.agent_request()) context.metrics, context.experiment = result.metrics, result.experiment @override diff --git a/src/bcbench/types.py b/src/bcbench/types.py index 0500a2c08..4f7500d74 100644 --- a/src/bcbench/types.py +++ b/src/bcbench/types.py @@ -10,7 +10,7 @@ import yaml from bcbench_core.agent.customization import CustomAgentsConfig, ToggleConfig from bcbench_core.agent.experiment import ExperimentConfiguration -from bcbench_core.agent.harness import AgentIdentity +from bcbench_core.agent.harness import AgentIdentity, AgentRequest from bcbench_core.agent.metrics import AgentMetrics from bcbench_core.container import ContainerConfig from pydantic import BaseModel, ConfigDict, Field, StrictBool, StringConstraints, model_validator @@ -650,3 +650,6 @@ def get_container(self) -> ContainerConfig: if self.container is None: raise ValueError(f"Container configuration is required for {self.category.value} evaluation") return self.container + + def agent_request(self) -> AgentRequest[E]: + return AgentRequest(task=self.entry, work_dir=self.repo_path, output_dir=self.result_dir) diff --git a/tests/conftest.py b/tests/conftest.py index 1ac1d615e..84b6bf357 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,13 +7,14 @@ import json from collections.abc import Generator +from dataclasses import dataclass, field from pathlib import Path -from typing import Literal, cast +from typing import Literal, cast, override from unittest.mock import patch import pytest from bcbench_core.agent.experiment import ExperimentConfiguration -from bcbench_core.agent.harness import AgentIdentity +from bcbench_core.agent.harness import AgentIdentity, AgentRequest, AgentResult, Harness from bcbench_core.agent.metrics import AgentMetrics from bcbench_core.container import ContainerConfig from bcbench_core.dataset import TestEntry @@ -81,6 +82,21 @@ def create_dataset_entry( ) +@dataclass +class StubHarness[E: BaseDatasetEntry](Harness[E, AgentHarness]): + result: AgentResult = field(default_factory=AgentResult) + requests: list[AgentRequest[E]] = field(default_factory=list) + + @override + def identity(self) -> AgentIdentity[AgentHarness]: + return AgentIdentity[AgentHarness](harness=AgentHarness.COPILOT, version="1.0.0", model="test-model") + + @override + def run(self, request: AgentRequest[E], /) -> AgentResult: + self.requests.append(request) + return self.result + + def create_evaluation_context[EntryT: BaseDatasetEntry]( tmp_path: Path, entry: EntryT | None = None, diff --git a/tests/test_agent_version_results.py b/tests/test_agent_version_results.py index e785746f2..74fbd62ca 100644 --- a/tests/test_agent_version_results.py +++ b/tests/test_agent_version_results.py @@ -1,6 +1,6 @@ import json from pathlib import Path -from unittest.mock import Mock, patch +from unittest.mock import patch import pytest from bcbench_core.agent.experiment import ExperimentConfiguration @@ -12,7 +12,7 @@ from bcbench.results.leaderboard import Leaderboard, LeaderboardAggregate from bcbench.results.summary import EvaluationResultSummary from bcbench.types import AgentConfig, AgentHarness, EvaluationCategory -from tests.conftest import create_bugfix_result, create_codereview_result, create_evaluation_context +from tests.conftest import StubHarness, create_bugfix_result, create_codereview_result, create_evaluation_context @pytest.mark.parametrize("version", ["1.2.3", "a" * 40, None]) @@ -36,7 +36,7 @@ def test_timeout_result_keeps_version_resolved_before_execution(tmp_path: Path) patch.object(type(pipeline), "setup"), patch.object(type(pipeline), "run_agent", side_effect=AgentTimeoutError("timeout")), ): - pipeline.execute(context, Mock()) + pipeline.execute(context, StubHarness()) saved = next(context.result_dir.glob("*.jsonl")) result = BaseEvaluationResult.from_json(json.loads(saved.read_text())) diff --git a/tests/test_bcal_agent_provider.py b/tests/test_bcal_agent_provider.py index 29a364b30..a106aa589 100644 --- a/tests/test_bcal_agent_provider.py +++ b/tests/test_bcal_agent_provider.py @@ -7,10 +7,12 @@ from unittest.mock import MagicMock, patch import pytest +from bcbench_core.agent.harness import AgentRequest, AgentResult from bcbench_core.exceptions import AgentError, AgentTimeoutError from bcbench.agent.bcal import BCalBackendConfig from bcbench.agent.bcal import agent as bcal_agent +from bcbench.dataset import NL2ALEntry from tests.conftest import create_nl2al_entry @@ -48,6 +50,10 @@ def test_model_is_optional(self): assert not any(a.startswith("--deployment=") for a in args) +def _run_bcal(entry: NL2ALEntry, workspace: Path, backend_config: BCalBackendConfig) -> AgentResult: + return bcal_agent.BCalHarness(backend_config).run(AgentRequest(task=entry, work_dir=workspace, output_dir=workspace)) + + class TestRunBcalAgent: def test_passes_external_command_to_bcal(self, workspace: Path): entry = create_nl2al_entry() @@ -63,14 +69,7 @@ def fake_run(args: list[str], **_: object) -> MagicMock: patch.object(bcal_agent, "_resolve_bcal_executable", return_value="C:\\fake\\bcal.exe"), patch.object(subprocess, "run", side_effect=fake_run), ): - result = bcal_agent.run_bcal_agent( - entry=entry, - repo_path=workspace, - backend_config=BCalBackendConfig( - command="python bridge.py", - model="gpt-5", - ), - ) + result = _run_bcal(entry, workspace, BCalBackendConfig(command="python bridge.py", model="gpt-5")) assert result.metrics is not None args = captured["args"] @@ -87,7 +86,7 @@ def test_external_command_requires_command(self, workspace: Path): patch.object(bcal_agent, "_resolve_bcal_executable", return_value="C:\\fake\\bcal.exe"), pytest.raises(AgentError), ): - bcal_agent.run_bcal_agent(entry=entry, repo_path=workspace, backend_config=BCalBackendConfig()) + _run_bcal(entry, workspace, BCalBackendConfig()) def test_external_command_model_is_optional(self, workspace: Path): entry = create_nl2al_entry() @@ -103,11 +102,7 @@ def fake_run(args: list[str], **_: object) -> MagicMock: patch.object(bcal_agent, "_resolve_bcal_executable", return_value="C:\\fake\\bcal.exe"), patch.object(subprocess, "run", side_effect=fake_run), ): - bcal_agent.run_bcal_agent( - entry=entry, - repo_path=workspace, - backend_config=BCalBackendConfig(command="python bridge.py"), - ) + _run_bcal(entry, workspace, BCalBackendConfig(command="python bridge.py")) assert "--llm-backend=external-command" in captured["args"] assert "--llm-command=python bridge.py" in captured["args"] diff --git a/tests/test_claude_code_agent.py b/tests/test_claude_code_agent.py index eb7517ac3..43e87e336 100644 --- a/tests/test_claude_code_agent.py +++ b/tests/test_claude_code_agent.py @@ -3,7 +3,9 @@ from pathlib import Path from unittest.mock import patch -from bcbench.agent.claude.agent import run_claude_code +from bcbench_core.agent.harness import AgentRequest + +from bcbench.agent.claude.agent import ClaudeHarness from bcbench.types import AgentConfig, EvaluationCategory from tests.conftest import create_dataset_entry @@ -34,15 +36,8 @@ def test_claude_code_excludes_user_settings_and_auto_memory(tmp_path: Path, monk return_value=subprocess.CompletedProcess(args=[], returncode=0, stdout=b"{}\n", stderr=b""), ) as mock_run, ): - run_claude_code( - entry=create_dataset_entry(), - model="claude-test-model", - category=EvaluationCategory.BUG_FIX, - repo_path=repo_path, - output_dir=output_dir, - agent_config=AgentConfig(), - plugin_root=tmp_path / "plugins", - ) + harness = ClaudeHarness(model="claude-test-model", category=EvaluationCategory.BUG_FIX, agent_config=AgentConfig(), plugin_root=tmp_path / "plugins") + harness.run(AgentRequest(task=create_dataset_entry(), work_dir=repo_path, output_dir=output_dir)) assert mock_run.call_args.args[0] == [ "claude", diff --git a/tests/test_cli_commands.py b/tests/test_cli_commands.py index 0060be941..09c731ce7 100644 --- a/tests/test_cli_commands.py +++ b/tests/test_cli_commands.py @@ -124,14 +124,14 @@ def load(_dataset_path, entry_id: str): return [entry] class Pipeline: - def execute(self, context, _agent_runner): + def execute(self, context, _harness): captured["context"] = context with ( patch.object(EvaluationCategory, "dataset_path", return_value=tmp_path / "nl2al.jsonl"), patch.object(EvaluationCategory, "entry_class", new_callable=PropertyMock, return_value=EntryClass), patch.object(evaluate_commands, "NL2ALPipeline", return_value=Pipeline()), - patch.object(evaluate_commands, "get_bcal_version", return_value="18.0.1.2-beta"), + patch("bcbench.agent.bcal.agent.get_bcal_version", return_value="18.0.1.2-beta"), ): evaluate_commands.evaluate_bcal( entry_id=entry.instance_id, @@ -176,8 +176,8 @@ class Pipeline: def setup_workspace(self, _entry, _repo_path): pass - def execute(self, context, agent_runner): - agent_runner(context) + def execute(self, context, harness): + harness.run(context.agent_request()) return entry, SimpleNamespace( dataset_path=lambda _tier=None: tmp_path / "dataset.jsonl", @@ -188,20 +188,19 @@ def execute(self, context, agent_runner): @pytest.mark.parametrize( - ("commands", "command_name", "agent_name", "extra_args"), + ("commands", "command_name", "harness_name", "extra_args"), [ (run_commands, "run_copilot", "CopilotHarness", {}), - (run_commands, "run_claude", "run_claude_code", {}), + (run_commands, "run_claude", "ClaudeHarness", {}), (evaluate_commands, "evaluate_copilot", "CopilotHarness", {"run_id": "test-run"}), - (evaluate_commands, "evaluate_claude_code", "run_claude_code", {"run_id": "test-run"}), + (evaluate_commands, "evaluate_claude_code", "ClaudeHarness", {"run_id": "test-run"}), ], ) -def test_agent_commands_preserve_requested_mcp_flags(tmp_path, agent_command_category, commands, command_name, agent_name, extra_args): +def test_agent_commands_preserve_requested_mcp_flags(tmp_path, agent_command_category, commands, command_name, harness_name, extra_args): entry, category = agent_command_category with ( - patch.object(commands, agent_name) as run_agent, - patch.object(evaluate_commands, "get_claude_version", return_value="1.2.3"), + patch.object(commands, harness_name) as harness, ): getattr(commands, command_name)( entry_id=entry.instance_id, @@ -216,29 +215,28 @@ def test_agent_commands_preserve_requested_mcp_flags(tmp_path, agent_command_cat **extra_args, ) - runtime = run_agent.call_args.kwargs["runtime"] + runtime = harness.call_args.kwargs["runtime"] assert runtime.al_mcp is True assert runtime.bc_mcp is True assert runtime.container.name == "bcbench" @pytest.mark.parametrize( - ("commands", "command_name", "agent_name", "extra_args"), + ("commands", "command_name", "harness_name", "extra_args"), [ (run_commands, "run_copilot", "CopilotHarness", {}), - (run_commands, "run_claude", "run_claude_code", {}), + (run_commands, "run_claude", "ClaudeHarness", {}), (evaluate_commands, "evaluate_copilot", "CopilotHarness", {"run_id": "test-run"}), - (evaluate_commands, "evaluate_claude_code", "run_claude_code", {"run_id": "test-run"}), + (evaluate_commands, "evaluate_claude_code", "ClaudeHarness", {"run_id": "test-run"}), ], ) -def test_agent_commands_pass_the_given_agent_config(tmp_path, agent_command_category, commands, command_name, agent_name, extra_args): +def test_agent_commands_pass_the_given_agent_config(tmp_path, agent_command_category, commands, command_name, harness_name, extra_args): entry, category = agent_command_category config_path = tmp_path / "agent-config.yaml" config_path.write_text("instructions:\n enabled: true\n", encoding="utf-8") with ( - patch.object(commands, agent_name) as run_agent, - patch.object(evaluate_commands, "get_claude_version", return_value="1.2.3"), + patch.object(commands, harness_name) as harness, ): getattr(commands, command_name)( entry_id=entry.instance_id, @@ -249,7 +247,7 @@ def test_agent_commands_pass_the_given_agent_config(tmp_path, agent_command_cate **extra_args, ) - assert run_agent.call_args.kwargs["agent_config"] == AgentConfig.model_validate({"instructions": {"enabled": True}}) + assert harness.call_args.kwargs["agent_config"] == AgentConfig.model_validate({"instructions": {"enabled": True}}) def _runtime_options(**overrides): @@ -323,8 +321,8 @@ def test_result_summarize_creates_all_outputs(sample_results_directory, problem_ patch.object(_BugFixTestGenBase, "problem_statement_dir", property(lambda self: problem_statement_dir)), patch.object(EvaluationCategory, "dataset_path", return_value=dataset_path), patch("bcbench.agent.copilot.agent.get_copilot_version", side_effect=AssertionError("Must use artifact version")), - patch.object(evaluate_commands, "get_claude_version", side_effect=AssertionError("Must use artifact version")), - patch.object(evaluate_commands, "get_pr_review_version", side_effect=AssertionError("Must use artifact version")), + patch("bcbench.agent.claude.agent.get_claude_version", side_effect=AssertionError("Must use artifact version")), + patch("bcbench.agent.pr_review.agent.get_pr_review_version", side_effect=AssertionError("Must use artifact version")), ): result = runner.invoke( app, diff --git a/tests/test_evaluate_pipeline.py b/tests/test_evaluate_pipeline.py index e3bd2bed5..e6154586e 100644 --- a/tests/test_evaluate_pipeline.py +++ b/tests/test_evaluate_pipeline.py @@ -7,16 +7,16 @@ import pytest from bcbench_core.agent.experiment import ExperimentConfiguration -from bcbench_core.agent.harness import AgentResult +from bcbench_core.agent.harness import AgentResult, Harness from bcbench_core.agent.metrics import AgentMetrics from bcbench_core.exceptions import AgentTimeoutError from bcbench.commands.evaluate import MockEvaluationPipeline from bcbench.dataset import BaseDatasetEntry, BugFixEntry, NL2ALEntry -from bcbench.evaluate.base import AgentRunner, EvaluationPipeline +from bcbench.evaluate.base import EvaluationPipeline from bcbench.results.base import RESULT_FILE_SUFFIX, BaseEvaluationResult, JudgeBasedEvaluationResult -from bcbench.types import AgentConfig, EvaluationCategory, EvaluationContext -from tests.conftest import create_codereview_entry, create_dataset_entry, create_evaluation_context, create_ext_advisor_entry, create_ext_implement_entry, create_nl2al_entry +from bcbench.types import AgentConfig, AgentHarness, EvaluationCategory, EvaluationContext +from tests.conftest import StubHarness, create_codereview_entry, create_dataset_entry, create_evaluation_context, create_ext_advisor_entry, create_ext_implement_entry, create_nl2al_entry class _StubPipeline[E: BaseDatasetEntry](EvaluationPipeline[E]): @@ -36,11 +36,11 @@ def setup(self, context: EvaluationContext[E]) -> None: self.setup_called = True @override - def run_agent(self, context: EvaluationContext[E], agent_runner: AgentRunner[E]) -> None: + def run_agent(self, context: EvaluationContext[E], harness: Harness[E, AgentHarness]) -> None: self.run_agent_called = True if self.raise_in_run_agent is not None: raise self.raise_in_run_agent - result = agent_runner(context) + result = harness.run(context.agent_request()) context.metrics, context.experiment = result.metrics, result.experiment @override @@ -50,8 +50,8 @@ def evaluate(self, context: EvaluationContext[E]) -> None: raise self.raise_in_evaluate -def _noop_runner(_ctx: EvaluationContext[BugFixEntry]) -> AgentResult: - return AgentResult(metrics=AgentMetrics(execution_time=1.0), experiment=ExperimentConfiguration()) +def _noop_harness() -> StubHarness[BugFixEntry]: + return StubHarness(AgentResult(metrics=AgentMetrics(execution_time=1.0), experiment=ExperimentConfiguration())) def _read_only_result[E: BaseDatasetEntry](ctx: EvaluationContext[E]) -> BaseEvaluationResult: @@ -67,7 +67,7 @@ def test_runs_setup_then_agent_then_evaluate(self, tmp_path): ctx = create_evaluation_context(tmp_path) pipeline = _StubPipeline[BugFixEntry]() - pipeline.execute(ctx, _noop_runner) + pipeline.execute(ctx, _noop_harness()) assert pipeline.setup_called assert pipeline.run_agent_called @@ -81,7 +81,7 @@ def test_persists_timeout_result_and_skips_evaluate(self, tmp_path): timeout_config = ExperimentConfiguration(custom_instructions=True) pipeline = _StubPipeline[BugFixEntry](raise_in_run_agent=AgentTimeoutError("test timeout", metrics=timeout_metrics, config=timeout_config)) - pipeline.execute(ctx, _noop_runner) + pipeline.execute(ctx, _noop_harness()) assert pipeline.evaluate_called is False result = _read_only_result(ctx) @@ -95,7 +95,7 @@ def test_persists_category_specific_timeout_result(self, tmp_path): ctx = create_evaluation_context(tmp_path, entry=entry, category=EvaluationCategory.NL2AL) pipeline = _StubPipeline[NL2ALEntry](raise_in_run_agent=AgentTimeoutError("test timeout")) - pipeline.execute(ctx, lambda _: AgentResult()) + pipeline.execute(ctx, StubHarness()) result_file = ctx.result_dir / f"{entry.instance_id}{RESULT_FILE_SUFFIX}" payload = json.loads(result_file.read_text(encoding="utf-8")) @@ -112,7 +112,7 @@ def test_unexpected_exceptions_still_propagate(self, tmp_path): pipeline = _StubPipeline[BugFixEntry](raise_in_evaluate=RuntimeError("infra blew up")) with pytest.raises(RuntimeError, match="infra blew up"): - pipeline.execute(ctx, _noop_runner) + pipeline.execute(ctx, _noop_harness()) def _entry_for_category(category: EvaluationCategory) -> BaseDatasetEntry: diff --git a/tests/test_nl2al_pipeline.py b/tests/test_nl2al_pipeline.py index ffc899dea..be3d3e2b1 100644 --- a/tests/test_nl2al_pipeline.py +++ b/tests/test_nl2al_pipeline.py @@ -1,13 +1,12 @@ """Tests for NL2ALPipeline.evaluate() — empty-diff handling is nl2al-specific.""" import pytest -from bcbench_core.agent.harness import AgentResult from bcbench_core.exceptions import EmptyDiffError from bcbench.evaluate.nl2al import NL2ALPipeline from bcbench.results.base import RESULT_FILE_SUFFIX, JudgeBasedEvaluationResult from bcbench.types import EvaluationCategory -from tests.conftest import create_evaluation_context, create_nl2al_entry +from tests.conftest import StubHarness, create_evaluation_context, create_nl2al_entry def _read_only_result(ctx) -> JudgeBasedEvaluationResult: @@ -82,17 +81,13 @@ def test_runs_agent_once_and_never_retries(self, tmp_path, monkeypatch): ctx = _nl2al_context(tmp_path) pipeline = NL2ALPipeline() - agent_calls = {"n": 0} - - def agent_runner(_ctx): - agent_calls["n"] += 1 - return AgentResult() + harness = StubHarness() reset_calls = {"n": 0} monkeypatch.setattr(pipeline, "setup_workspace", lambda *_args, **_kw: reset_calls.__setitem__("n", reset_calls["n"] + 1)) monkeypatch.setattr("bcbench.evaluate.nl2al.stage_and_get_diff", _throw(AssertionError("run_agent must not stage diffs or retry when retries are disabled"))) - pipeline.run_agent(ctx, agent_runner) + pipeline.run_agent(ctx, harness) - assert agent_calls["n"] == 1 + assert len(harness.requests) == 1 assert reset_calls["n"] == 0 diff --git a/tests/test_pr_review_agent.py b/tests/test_pr_review_agent.py index 11def48bc..a64ddef3e 100644 --- a/tests/test_pr_review_agent.py +++ b/tests/test_pr_review_agent.py @@ -4,10 +4,11 @@ from unittest.mock import patch import pytest +from bcbench_core.agent.harness import AgentRequest from bcbench_core.exceptions import AgentError -from bcbench.agent.pr_review.agent import _prepare_bcquality_root, _resolve_pr_review_root, _write_review_json, run_pr_review_agent -from bcbench.types import EvaluationCategory, PRReviewMetrics +from bcbench.agent.pr_review.agent import PRReviewHarness, _prepare_bcquality_root, _resolve_pr_review_root, _write_review_json +from bcbench.types import PRReviewMetrics from tests.conftest import create_codereview_entry @@ -163,14 +164,8 @@ def test_engine_environment_uses_target_repository_and_absolute_paths(tmp_path: patch("bcbench.agent.pr_review.agent.time.monotonic", side_effect=[10.0, 12.5]), patch("bcbench.agent.pr_review.agent.subprocess.run", return_value=completed) as run_process, ): - result = run_pr_review_agent( - entry=entry, - model="gpt-5.6-luna", - category=EvaluationCategory.CODE_REVIEW, - repo_path=tmp_path / "repo", - output_dir=Path("output"), - engine_path=tmp_path / "engine", - ) + harness = PRReviewHarness(model="gpt-5.6-luna", engine_path=tmp_path / "engine") + result = harness.run(AgentRequest(task=entry, work_dir=tmp_path / "repo", output_dir=Path("output"))) metrics = result.metrics assert isinstance(metrics, PRReviewMetrics) diff --git a/tests/test_review_runners.py b/tests/test_review_runners.py index 0d1ebe225..69e650b9b 100644 --- a/tests/test_review_runners.py +++ b/tests/test_review_runners.py @@ -4,6 +4,7 @@ import pytest from typer.testing import CliRunner +from bcbench.agent import PRReviewHarness from bcbench.cli import app from bcbench.commands import evaluate as evaluate_commands from bcbench.commands import run as run_commands @@ -12,32 +13,21 @@ from bcbench.types import AgentHarness, EvaluationCategory -def test_copilot_run_command_accepts_code_review(tmp_path: Path) -> None: +@pytest.mark.parametrize("harness_name", ["CopilotHarness", "ClaudeHarness"]) +def test_generic_run_commands_accept_code_review(tmp_path: Path, harness_name: str) -> None: entry = object() + command = run_commands.run_copilot if harness_name == "CopilotHarness" else run_commands.run_claude with ( patch.object(CodeReviewEntry, "load", return_value=[entry]), patch.object(CodeReviewPipeline, "setup_workspace"), - patch.object(run_commands, "CopilotHarness") as harness, + patch.object(run_commands, harness_name) as harness, ): - run_commands.run_copilot("synthetic__style-018", EvaluationCategory.CODE_REVIEW, repo_path=tmp_path, output_dir=tmp_path / "out") + command("synthetic__style-018", EvaluationCategory.CODE_REVIEW, repo_path=tmp_path, output_dir=tmp_path / "out") assert harness.call_args.kwargs["category"] is EvaluationCategory.CODE_REVIEW assert harness.return_value.run.call_args.args[0].task is entry -def test_claude_run_command_accepts_code_review(tmp_path: Path) -> None: - entry = object() - with ( - patch.object(CodeReviewEntry, "load", return_value=[entry]), - patch.object(CodeReviewPipeline, "setup_workspace"), - patch.object(run_commands, "run_claude_code") as agent_runner, - ): - run_commands.run_claude("synthetic__style-018", EvaluationCategory.CODE_REVIEW, repo_path=tmp_path, output_dir=tmp_path / "out") - - assert agent_runner.call_args.kwargs["entry"] is entry - assert agent_runner.call_args.kwargs["category"] is EvaluationCategory.CODE_REVIEW - - @pytest.mark.parametrize( ("command", "agent_name"), [ @@ -49,9 +39,9 @@ def test_generic_evaluate_commands_use_code_review_pipeline(tmp_path: Path, comm contexts = [] with ( patch.object(CodeReviewEntry, "load", return_value=[object()]), - patch.object(CodeReviewPipeline, "execute", side_effect=lambda context, runner: contexts.append(context)), + patch.object(CodeReviewPipeline, "execute", side_effect=lambda context, _harness: contexts.append(context)), patch("bcbench.agent.copilot.agent.get_copilot_version", return_value="1.2.3"), - patch.object(evaluate_commands, "get_claude_version", return_value="1.2.3"), + patch("bcbench.agent.claude.agent.get_claude_version", return_value="1.2.3"), ): command( "synthetic__style-018", @@ -68,12 +58,11 @@ def test_generic_evaluate_commands_use_code_review_pipeline(tmp_path: Path, comm def test_pr_review_evaluation_is_fixed_to_runner_and_category(tmp_path: Path) -> None: - contexts = [] + executions = [] with ( patch.object(CodeReviewEntry, "load", return_value=[object()]), - patch.object(CodeReviewPipeline, "execute", side_effect=lambda context, runner: (contexts.append(context), runner(context))), - patch.object(evaluate_commands, "run_pr_review_agent") as agent_runner, - patch.object(evaluate_commands, "get_pr_review_version", return_value="a" * 40) as get_version, + patch.object(CodeReviewPipeline, "execute", side_effect=lambda context, harness: executions.append((context, harness))), + patch("bcbench.agent.pr_review.agent.get_pr_review_version", return_value="a" * 40) as get_version, ): result = CliRunner().invoke( app, @@ -93,12 +82,12 @@ def test_pr_review_evaluation_is_fixed_to_runner_and_category(tmp_path: Path) -> ) assert result.exit_code == 0, result.exception - assert len(contexts) == 1 - assert contexts[0].agent.harness is AgentHarness.PR_REVIEW - assert contexts[0].agent.version == "a" * 40 - assert contexts[0].category is EvaluationCategory.CODE_REVIEW - assert contexts[0].agent.model == "gpt-5.6-luna" - assert agent_runner.call_args.kwargs["engine_path"] == tmp_path + [(context, harness)] = executions + assert harness == PRReviewHarness(model="gpt-5.6-luna", engine_path=tmp_path) + assert context.agent.harness is AgentHarness.PR_REVIEW + assert context.agent.version == "a" * 40 + assert context.category is EvaluationCategory.CODE_REVIEW + assert context.agent.model == "gpt-5.6-luna" get_version.assert_called_once_with(tmp_path) @@ -107,7 +96,7 @@ def test_pr_review_run_is_fixed_to_code_review(tmp_path: Path) -> None: with ( patch.object(CodeReviewEntry, "load", return_value=[entry]), patch.object(CodeReviewPipeline, "setup_workspace"), - patch.object(run_commands, "run_pr_review_agent") as agent_runner, + patch.object(run_commands, "PRReviewHarness") as harness, ): result = CliRunner().invoke( app, @@ -125,10 +114,9 @@ def test_pr_review_run_is_fixed_to_code_review(tmp_path: Path) -> None: ) assert result.exit_code == 0, result.exception - assert agent_runner.call_args.kwargs["entry"] is entry - assert agent_runner.call_args.kwargs["category"] is EvaluationCategory.CODE_REVIEW - assert agent_runner.call_args.kwargs["model"] == "gpt-5.6-luna" - assert agent_runner.call_args.kwargs["engine_path"] == tmp_path + assert harness.call_args.kwargs["model"] == "gpt-5.6-luna" + assert harness.call_args.kwargs["engine_path"] == tmp_path + assert harness.return_value.run.call_args.args[0].task is entry def test_pr_review_is_public_command() -> None: