diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index d3dc6f4b1..a8ef3c4a4 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -28,6 +28,12 @@ BC-Bench is category-based and designed to grow over time. It currently has two - Prefer high-order functions like map, filter, reduce over loops - Prefer immutable data structures where possible +### 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. + +`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. + ### Readable code over documentation or comments Function names should be self-explanatory. Do NOT add docstrings to functions unless absolutely necessary. When a docstring is necessary, keep it short and use Google style. Include only useful sections such as `Args:` and `Returns:`; skip details that are obvious from names and type hints. diff --git a/pyproject.toml b/pyproject.toml index 4023d8ccd..12b3126f5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -89,6 +89,9 @@ extend-select = [ "RSE", # flake8-raise: drop redundant parentheses on bare exception raises "PGH", # pygrep-hooks: require codes on noqa/type-ignore comments (PGH003/PGH004) "S307", # flake8-bandit: ban eval (was PGH001, which ruff removed in favour of this rule) + "DTZ", # flake8-datetimez: require timezone-aware datetime usage + "NPY", # NumPy-specific correctness and modernization checks + "PIE", # flake8-pie: miscellaneous correctness and simplification checks ] ignore = [ @@ -98,6 +101,9 @@ ignore = [ "TRY003", # raise-vanilla-args: forces a custom exception class for every error message (96 sites) ] +[tool.ruff.lint.flake8-tidy-imports] +ban-relative-imports = "all" + [tool.ruff.lint.flake8-tidy-imports.banned-api] # Force sandboxed rendering: "jinja2.Template".msg = "Use jinja2.sandbox.SandboxedEnvironment instead; the bare Template is not sandboxed." diff --git a/src/bcbench/commands/evaluate.py b/src/bcbench/commands/evaluate.py index 6aa1dcbc7..101b8b92c 100644 --- a/src/bcbench/commands/evaluate.py +++ b/src/bcbench/commands/evaluate.py @@ -1,6 +1,5 @@ import random import shutil -from collections.abc import Callable from pathlib import Path from typing import Annotated, cast @@ -22,7 +21,7 @@ ) from bcbench.config import get_config from bcbench.dataset import BaseDatasetEntry, NL2ALEntry -from bcbench.evaluate import EvaluationPipeline +from bcbench.evaluate import AgentRunner, EvaluationPipeline from bcbench.evaluate.codereview_judge_calibration import run_calibration from bcbench.logger import get_logger from bcbench.results import BaseEvaluationResult, CodeReviewResult, ExecutionBasedEvaluationResult, JudgeBasedEvaluationResult @@ -335,7 +334,7 @@ def setup_workspace(self, entry: BaseDatasetEntry, repo_path: Path) -> None: def setup(self, context: EvaluationContext[BaseDatasetEntry]) -> None: logger.info("Mock pipeline: Skipping setup") - def run_agent(self, context: EvaluationContext[BaseDatasetEntry], agent_runner: Callable) -> None: + def run_agent(self, context: EvaluationContext[BaseDatasetEntry], agent_runner: AgentRunner[BaseDatasetEntry]) -> None: """Generate random agent metrics and experiment configuration.""" logger.info("Mock pipeline: Generating random metrics and experiment configuration") diff --git a/src/bcbench/evaluate/__init__.py b/src/bcbench/evaluate/__init__.py index fd15bd584..c9c63ee90 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 EvaluationPipeline +from bcbench.evaluate.base import AgentRunner, EvaluationPipeline from bcbench.evaluate.bugfix import BugFixPipeline from bcbench.evaluate.codereview import CodeReviewPipeline from bcbench.evaluate.ext_request_advisor import ExtRequestAdvisorPipeline @@ -10,6 +10,7 @@ from bcbench.evaluate.testgeneration import TestGenerationPipeline __all__ = [ + "AgentRunner", "BugFixPipeline", "CodeReviewPipeline", "EvaluationPipeline", diff --git a/src/bcbench/evaluate/base.py b/src/bcbench/evaluate/base.py index bdd1542ae..2eaf427bd 100644 --- a/src/bcbench/evaluate/base.py +++ b/src/bcbench/evaluate/base.py @@ -1,8 +1,8 @@ from __future__ import annotations from abc import ABC, abstractmethod -from collections.abc import Callable from pathlib import Path +from typing import Protocol from bcbench.config import get_config from bcbench.dataset import BaseDatasetEntry @@ -14,7 +14,11 @@ logger = get_logger(__name__) _config = get_config() -__all__ = ["EvaluationPipeline"] +__all__ = ["AgentRunner", "EvaluationPipeline"] + + +class AgentRunner[E: BaseDatasetEntry](Protocol): + def __call__(self, context: EvaluationContext[E], /) -> tuple[AgentMetrics | None, ExperimentConfiguration | None]: ... class EvaluationPipeline[E: BaseDatasetEntry](ABC): @@ -45,7 +49,7 @@ def setup(self, context: EvaluationContext[E]) -> None: raise NotImplementedError @abstractmethod - def run_agent(self, context: EvaluationContext[E], agent_runner: Callable) -> None: + def run_agent(self, context: EvaluationContext[E], agent_runner: AgentRunner[E]) -> None: """Run the agent and capture metrics. Args: @@ -74,7 +78,7 @@ def evaluate(self, context: EvaluationContext[E]) -> None: def execute( self, context: EvaluationContext[E], - agent_runner: Callable[[EvaluationContext[E]], tuple[AgentMetrics | None, ExperimentConfiguration | None]], + agent_runner: AgentRunner[E], ) -> None: """Template method orchestrating the evaluation flow. diff --git a/src/bcbench/evaluate/bugfix.py b/src/bcbench/evaluate/bugfix.py index 625319b18..acc1c9838 100644 --- a/src/bcbench/evaluate/bugfix.py +++ b/src/bcbench/evaluate/bugfix.py @@ -1,8 +1,7 @@ -from collections.abc import Callable from pathlib import Path from bcbench.dataset import BugFixEntry -from bcbench.evaluate.base import EvaluationPipeline +from bcbench.evaluate.base import AgentRunner, EvaluationPipeline from bcbench.exceptions import BuildError, TestExecutionError from bcbench.github_actions import github_log_group from bcbench.logger import get_logger @@ -46,7 +45,7 @@ def setup(self, context: EvaluationContext[BugFixEntry]) -> None: copy_problem_statement_folder(context.entry, context.repo_path) set_runtime_version(context.repo_path, context.entry.project_paths) - def run_agent(self, context: EvaluationContext[BugFixEntry], agent_runner: Callable) -> None: + def run_agent(self, context: EvaluationContext[BugFixEntry], agent_runner: AgentRunner[BugFixEntry]) -> None: with github_log_group(f"{context.agent_name} -- Entry: {context.entry.instance_id}"): context.metrics, context.experiment = agent_runner(context) diff --git a/src/bcbench/evaluate/codereview.py b/src/bcbench/evaluate/codereview.py index 62c167c64..fefbdc05a 100644 --- a/src/bcbench/evaluate/codereview.py +++ b/src/bcbench/evaluate/codereview.py @@ -1,9 +1,8 @@ import subprocess -from collections.abc import Callable from pathlib import Path from bcbench.dataset.codereview import CodeReviewEntry, ReviewComment -from bcbench.evaluate.base import EvaluationPipeline +from bcbench.evaluate.base import AgentRunner, 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 @@ -50,7 +49,7 @@ def setup_workspace(self, entry: CodeReviewEntry, repo_path: Path) -> None: def setup(self, context: EvaluationContext[CodeReviewEntry]) -> None: self.setup_workspace(context.entry, context.repo_path) - def run_agent(self, context: EvaluationContext[CodeReviewEntry], agent_runner: Callable) -> None: + def run_agent(self, context: EvaluationContext[CodeReviewEntry], agent_runner: AgentRunner[CodeReviewEntry]) -> None: with github_log_group(f"{context.agent_name} -- Entry: {context.entry.instance_id}"): context.metrics, context.experiment = agent_runner(context) diff --git a/src/bcbench/evaluate/ext_request_advisor.py b/src/bcbench/evaluate/ext_request_advisor.py index c8ecddcf9..4ce32f92d 100644 --- a/src/bcbench/evaluate/ext_request_advisor.py +++ b/src/bcbench/evaluate/ext_request_advisor.py @@ -1,8 +1,7 @@ -from collections.abc import Callable from pathlib import Path from bcbench.dataset import ExtRequestAdvisorEntry -from bcbench.evaluate.base import EvaluationPipeline +from bcbench.evaluate.base import AgentRunner, EvaluationPipeline from bcbench.github_actions import github_log_group from bcbench.logger import get_logger from bcbench.operations import setup_repo_prebuild @@ -26,7 +25,7 @@ def setup_workspace(self, entry: ExtRequestAdvisorEntry, repo_path: Path) -> Non def setup(self, context: EvaluationContext[ExtRequestAdvisorEntry]) -> None: self.setup_workspace(context.entry, context.repo_path) - def run_agent(self, context: EvaluationContext[ExtRequestAdvisorEntry], agent_runner: Callable) -> None: + def run_agent(self, context: EvaluationContext[ExtRequestAdvisorEntry], agent_runner: AgentRunner[ExtRequestAdvisorEntry]) -> None: with github_log_group(f"{context.agent_name} -- Entry: {context.entry.instance_id}"): context.metrics, context.experiment = agent_runner(context) diff --git a/src/bcbench/evaluate/ext_request_implement.py b/src/bcbench/evaluate/ext_request_implement.py index de4a04d22..23b4f40b6 100644 --- a/src/bcbench/evaluate/ext_request_implement.py +++ b/src/bcbench/evaluate/ext_request_implement.py @@ -1,8 +1,7 @@ -from collections.abc import Callable from pathlib import Path from bcbench.dataset import ExtRequestImplementEntry -from bcbench.evaluate.base import EvaluationPipeline +from bcbench.evaluate.base import AgentRunner, EvaluationPipeline from bcbench.exceptions import EmptyDiffError from bcbench.github_actions import github_log_group from bcbench.logger import get_logger @@ -31,7 +30,7 @@ def setup_workspace(self, entry: ExtRequestImplementEntry, repo_path: Path) -> N def setup(self, context: EvaluationContext[ExtRequestImplementEntry]) -> None: self.setup_workspace(context.entry, context.repo_path) - def run_agent(self, context: EvaluationContext[ExtRequestImplementEntry], agent_runner: Callable) -> None: + def run_agent(self, context: EvaluationContext[ExtRequestImplementEntry], agent_runner: AgentRunner[ExtRequestImplementEntry]) -> None: with github_log_group(f"{context.agent_name} -- Entry: {context.entry.instance_id}"): context.metrics, context.experiment = agent_runner(context) diff --git a/src/bcbench/evaluate/ext_request_triage.py b/src/bcbench/evaluate/ext_request_triage.py index 2a3f7b7cb..f2539cdfc 100644 --- a/src/bcbench/evaluate/ext_request_triage.py +++ b/src/bcbench/evaluate/ext_request_triage.py @@ -8,11 +8,10 @@ `expected` checklist. """ -from collections.abc import Callable from pathlib import Path from bcbench.dataset import ExtRequestTriageEntry -from bcbench.evaluate.base import EvaluationPipeline +from bcbench.evaluate.base import AgentRunner, EvaluationPipeline from bcbench.github_actions import github_log_group from bcbench.logger import get_logger from bcbench.operations import setup_repo_prebuild @@ -36,7 +35,7 @@ def setup_workspace(self, entry: ExtRequestTriageEntry, repo_path: Path) -> None def setup(self, context: EvaluationContext[ExtRequestTriageEntry]) -> None: self.setup_workspace(context.entry, context.repo_path) - def run_agent(self, context: EvaluationContext[ExtRequestTriageEntry], agent_runner: Callable) -> None: + def run_agent(self, context: EvaluationContext[ExtRequestTriageEntry], agent_runner: AgentRunner[ExtRequestTriageEntry]) -> None: with github_log_group(f"{context.agent_name} -- Entry: {context.entry.instance_id}"): context.metrics, context.experiment = agent_runner(context) diff --git a/src/bcbench/evaluate/nl2al.py b/src/bcbench/evaluate/nl2al.py index cc704a204..157bf4edf 100644 --- a/src/bcbench/evaluate/nl2al.py +++ b/src/bcbench/evaluate/nl2al.py @@ -1,10 +1,9 @@ import os import subprocess -from collections.abc import Callable from pathlib import Path from bcbench.dataset import NL2ALEntry -from bcbench.evaluate.base import EvaluationPipeline +from bcbench.evaluate.base import AgentRunner, EvaluationPipeline from bcbench.exceptions import EmptyDiffError from bcbench.github_actions import github_log_group from bcbench.logger import get_logger @@ -52,7 +51,7 @@ def setup_workspace(self, entry: NL2ALEntry, repo_path: Path) -> None: def setup(self, context: EvaluationContext[NL2ALEntry]) -> None: self.setup_workspace(context.entry, context.repo_path) - def run_agent(self, context: EvaluationContext[NL2ALEntry], agent_runner: Callable) -> None: + def run_agent(self, context: EvaluationContext[NL2ALEntry], agent_runner: AgentRunner[NL2ALEntry]) -> None: for attempt in range(1, _EMPTY_DIFF_MAX_ATTEMPTS + 1): with github_log_group(f"{context.agent_name} -- Entry: {context.entry.instance_id} (attempt {attempt}/{_EMPTY_DIFF_MAX_ATTEMPTS})"): context.metrics, context.experiment = agent_runner(context) diff --git a/src/bcbench/evaluate/testgeneration.py b/src/bcbench/evaluate/testgeneration.py index 7c2af2034..966db099a 100644 --- a/src/bcbench/evaluate/testgeneration.py +++ b/src/bcbench/evaluate/testgeneration.py @@ -1,4 +1,3 @@ -from collections.abc import Callable from pathlib import Path import yaml @@ -6,7 +5,7 @@ from bcbench.collection.patch_utils import extract_file_paths_from_patch from bcbench.config import get_config from bcbench.dataset import TestEntry, TestGenEntry -from bcbench.evaluate.base import EvaluationPipeline +from bcbench.evaluate.base import AgentRunner, EvaluationPipeline from bcbench.exceptions import BuildError, NoTestsExtractedError, TestExecutionError from bcbench.github_actions import github_log_group from bcbench.logger import get_logger @@ -78,7 +77,7 @@ def setup(self, context: EvaluationContext[TestGenEntry]) -> None: self._apply_input_postbuild(context.entry, context.repo_path) set_runtime_version(context.repo_path, context.entry.project_paths) - def run_agent(self, context: EvaluationContext[TestGenEntry], agent_runner: Callable) -> None: + def run_agent(self, context: EvaluationContext[TestGenEntry], agent_runner: AgentRunner[TestGenEntry]) -> None: with github_log_group(f"{context.agent_name} -- Entry: {context.entry.instance_id}"): context.metrics, context.experiment = agent_runner(context) diff --git a/tests/test_evaluate_pipeline.py b/tests/test_evaluate_pipeline.py index a1fc8324a..13ce7eeec 100644 --- a/tests/test_evaluate_pipeline.py +++ b/tests/test_evaluate_pipeline.py @@ -1,7 +1,6 @@ """Tests for EvaluationPipeline.execute() template-method orchestration.""" import json -from collections.abc import Callable from pathlib import Path from unittest.mock import patch @@ -10,7 +9,7 @@ from bcbench.commands.evaluate import MockEvaluationPipeline from bcbench.config import get_config from bcbench.dataset import BaseDatasetEntry, BugFixEntry -from bcbench.evaluate.base import EvaluationPipeline +from bcbench.evaluate.base import AgentRunner, EvaluationPipeline from bcbench.exceptions import AgentTimeoutError from bcbench.results.base import BaseEvaluationResult from bcbench.types import AgentMetrics, EvaluationCategory, EvaluationContext, ExperimentConfiguration @@ -31,7 +30,7 @@ def setup_workspace(self, entry: BugFixEntry, repo_path: Path) -> None: def setup(self, context: EvaluationContext[BugFixEntry]) -> None: self.setup_called = True - def run_agent(self, context: EvaluationContext[BugFixEntry], agent_runner: Callable) -> None: + def run_agent(self, context: EvaluationContext[BugFixEntry], agent_runner: AgentRunner[BugFixEntry]) -> None: self.run_agent_called = True if self.raise_in_run_agent is not None: raise self.raise_in_run_agent