From 39a7e514c9f2ed1e6cf2e9cf7e7e88a50c2a4a5d Mon Sep 17 00:00:00 2001 From: dayland Date: Thu, 24 Sep 2026 16:42:27 +0100 Subject: [PATCH 1/2] Add validated PR Review definitions Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6a73db23-7e6d-4e87-a162-f7f16374bc44 --- .github/workflows/pr-review-evaluation.yml | 13 +- CATEGORIES.md | 6 + docs/code-review-details.md | 28 +++- docs/code-review.md | 18 ++- src/bcbench/agent/pr_review/agent.py | 30 ++-- src/bcbench/agent/pr_review/definitions.py | 173 +++++++++++++++++++++ src/bcbench/agent/pr_review/metrics.py | 8 + src/bcbench/commands/evaluate.py | 3 + src/bcbench/results/codereview.py | 49 +++++- src/bcbench/results/leaderboard.py | 22 ++- src/bcbench/types.py | 6 + tests/test_pr_review_agent.py | 1 + tests/test_pr_review_definitions.py | 101 ++++++++++++ tests/test_pr_review_metrics_reporting.py | 66 ++++++++ tests/test_review_runners.py | 1 + tests/test_review_workflows.py | 17 +- 16 files changed, 515 insertions(+), 27 deletions(-) create mode 100644 src/bcbench/agent/pr_review/definitions.py create mode 100644 tests/test_pr_review_definitions.py diff --git a/.github/workflows/pr-review-evaluation.yml b/.github/workflows/pr-review-evaluation.yml index f0c3a9460..34f11d088 100644 --- a/.github/workflows/pr-review-evaluation.yml +++ b/.github/workflows/pr-review-evaluation.yml @@ -18,6 +18,14 @@ on: - "gpt-5.6-luna" - "gpt-5.3-codex" - "mai-code-1.1-flash" + definition-id: + description: "Registered PR Review definition (unclassified runs are never published)" + required: false + default: "pr-review-production-sol-luna-serial-v1" + type: choice + options: + - "pr-review-production-sol-luna-serial-v1" + - "unclassified" leaf-model: description: "Model requested for domain leaf agents" required: false @@ -168,6 +176,7 @@ jobs: uv run bcbench evaluate pr-review "${{ matrix.entry }}" ` --model "${{ inputs.model }}" ` + --definition-id "${{ inputs.definition-id }}" ` --engine-path "${{ steps.install-harnesses.outputs.bc-alagents-path }}" ` --repo-path "${{ steps.setup-env.outputs.repo_path }}" ` --cli-version "${{ steps.install-harnesses.outputs.copilot-version }}" ` @@ -195,7 +204,7 @@ jobs: agent: "BC PR Review" # Scores only the modified entries, so publishing it to Braintrust/Kusto or the # leaderboard would record a partial corpus as a benchmark result. - mock: ${{ inputs.test-run || inputs.modified-only || inputs.engine-sha != '' || inputs.entries != '' }} + mock: ${{ inputs.test-run || inputs.modified-only || inputs.engine-sha != '' || inputs.definition-id != 'pr-review-production-sol-luna-serial-v1' || inputs.entries != '' }} category: code-review git-ref: ${{ inputs.git-ref || github.ref_name }} benchmark-commit: ${{ github.sha }} @@ -219,4 +228,4 @@ jobs: repeat: ${{ inputs.repeat }} existing-tag: ${{ needs.pin-commit.outputs.tag-name }} workflow-inputs: | - {"model": "${{ inputs.model }}", "leaf-model": "${{ inputs.leaf-model }}", "leaf-execution": "${{ inputs.leaf-execution }}", "max-leaf-concurrency": "${{ inputs.max-leaf-concurrency }}", "engine-sha": ${{ toJSON(inputs.engine-sha) }}, "test-run": "${{ inputs.test-run }}", "entries": ${{ toJSON(inputs.entries) }}, "git-ref": "${{ inputs.git-ref || github.ref_name }}"} + {"model": "${{ inputs.model }}", "definition-id": "${{ inputs.definition-id }}", "leaf-model": "${{ inputs.leaf-model }}", "leaf-execution": "${{ inputs.leaf-execution }}", "max-leaf-concurrency": "${{ inputs.max-leaf-concurrency }}", "engine-sha": ${{ toJSON(inputs.engine-sha) }}, "test-run": "${{ inputs.test-run }}", "entries": ${{ toJSON(inputs.entries) }}, "git-ref": "${{ inputs.git-ref || github.ref_name }}"} diff --git a/CATEGORIES.md b/CATEGORIES.md index 006a3af1d..8c541b83d 100644 --- a/CATEGORIES.md +++ b/CATEGORIES.md @@ -23,6 +23,12 @@ Start with `EvaluationCategory` in [src/bcbench/types.py](src/bcbench/types.py). Keep dataset entry classes and result classes focused on typed data. Put category-specific behavior in the pipeline. +### Named definitions + +Standard single-model agents are identified by the shared `model` field. A composed runner may additionally expose a category-owned named definition when its category can validate the complete configuration from runtime evidence. Keep that optional `definition_id` and display name in the category's result, summary, and aggregate models; do not add a universal profile or definition object to shared base result, summary, aggregate, or metric schemas. + +The `code-review` category demonstrates this boundary for BC PR Review. Its release-owned registry accepts a definition only when root and leaf models, scheduling, engine and CLI pins, BCQuality provenance, and recorded review policy all match. The normal [Code Review dashboard](docs/code-review.md) displays the definition when present and otherwise falls back to `model`; the [Advanced Metrics view](docs/code-review-details.md) is the detailed provenance surface. Other categories, including BCal scenarios and checkpointed bug-fix lifecycles, may opt in later through their own category-owned metadata without changing shared schemas. + ## Checklist Use the existing implementations as examples: `bug-fix` and `test-generation` for execution-based categories, `code-review` and `nl2al` for judge-based ones. diff --git a/docs/code-review-details.md b/docs/code-review-details.md index 731ee800f..7a4345793 100644 --- a/docs/code-review-details.md +++ b/docs/code-review-details.md @@ -49,7 +49,9 @@ Diagnostic averages use only tasks that reported the metric. Cross-run averages Agent - Model + Root model + Definition + Definition ID Configuration Models (leaf → root) Leaf Model @@ -99,6 +101,10 @@ Diagnostic averages use only tasks that reported the metric. Cross-run averages Copilot CLI BC-ALAgents BCQuality + Leaf model + Leaf scheduling + Review policy + BCQuality source snapshot @@ -107,6 +113,8 @@ Diagnostic averages use only tasks that reported the metric. Cross-run averages {{ agg.agent_name }} {{ agg.model }} + {{ agg.definition_name | default: agg.model }} + {% if agg.definition_id %}{{ agg.definition_id }}{% else %}—{% endif %} {% if agg.experiment %}{{ agg.experiment | jsonify }}{% else %}Baseline{% endif %} {% if agg.models and agg.models.size > 0 %}{{ agg.models | join: " → " }}{% else %}—{% endif %} {% if agg.leaf_model %}{{ agg.leaf_model }}{% else %}—{% endif %} @@ -156,6 +164,10 @@ Diagnostic averages use only tasks that reported the metric. Cross-run averages {% if agg.copilot_cli_version %}{{ agg.copilot_cli_version }}{% else %}—{% endif %} {% if agg.bc_alagents_commit %}{{ agg.bc_alagents_commit | slice: 0, 8 }}{% else %}—{% endif %} {% if agg.bcquality_commit %}{{ agg.bcquality_commit | slice: 0, 8 }}{% if agg.bcquality_version %} ({{ agg.bcquality_version }}){% endif %}{% else %}—{% endif %} + {% if agg.leaf_model %}{{ agg.leaf_model }}{% else %}—{% endif %} + {% if agg.leaf_execution %}{{ agg.leaf_execution }} ({{ agg.max_leaf_concurrency }}){% else %}—{% endif %} + {% if agg.minimum_severity %}knowledge ≥ {{ agg.minimum_severity }}; agent ≥ {{ agg.agent_minimum_severity }}; {{ agg.cli_timeout_minutes }}m{% else %}—{% endif %} + {% if agg.bcquality_source_snapshot %}{{ agg.bcquality_source_snapshot }}{% else %}—{% endif %} {% endfor %} @@ -174,7 +186,9 @@ Diagnostic averages use only tasks that reported the metric. Cross-run averages Run Agent - Model + Root model + Definition + Definition ID Configuration Models (leaf → root) Leaf Model @@ -230,6 +244,10 @@ Diagnostic averages use only tasks that reported the metric. Cross-run averages Copilot CLI BC-ALAgents BCQuality + Leaf model + Leaf scheduling + Review policy + BCQuality source snapshot @@ -239,6 +257,8 @@ Diagnostic averages use only tasks that reported the metric. Cross-run averages {% if run.github_run_id %}{{ run.github_run_id }}{% else %}—{% endif %} {{ run.agent_name }} {{ run.model }} + {{ run.definition_name | default: run.model }} + {% if run.definition_id %}{{ run.definition_id }}{% else %}—{% endif %} {% if run.experiment %}{{ run.experiment | jsonify }}{% else %}Baseline{% endif %} {% if run.models and run.models.size > 0 %}{{ run.models | join: " → " }}{% else %}—{% endif %} {% if run.leaf_model %}{{ run.leaf_model }}{% else %}—{% endif %} @@ -294,6 +314,10 @@ Diagnostic averages use only tasks that reported the metric. Cross-run averages {% if run.copilot_cli_version %}{{ run.copilot_cli_version }}{% else %}—{% endif %} {% if run.bc_alagents_commit %}{{ run.bc_alagents_commit | slice: 0, 8 }}{% else %}—{% endif %} {% if run.bcquality_commit %}{{ run.bcquality_commit | slice: 0, 8 }}{% if run.bcquality_version %} ({{ run.bcquality_version }}){% endif %}{% else %}—{% endif %} + {% if run.leaf_model %}{{ run.leaf_model }}{% else %}—{% endif %} + {% if run.leaf_execution %}{{ run.leaf_execution }} ({{ run.max_leaf_concurrency }}){% else %}—{% endif %} + {% if run.minimum_severity %}knowledge ≥ {{ run.minimum_severity }}; agent ≥ {{ run.agent_minimum_severity }}; {{ run.cli_timeout_minutes }}m{% else %}—{% endif %} + {% if run.bcquality_source_snapshot %}{{ run.bcquality_source_snapshot }}{% else %}—{% endif %} {% endfor %} diff --git a/docs/code-review.md b/docs/code-review.md index e1522f29f..80fe836ec 100644 --- a/docs/code-review.md +++ b/docs/code-review.md @@ -63,6 +63,8 @@ Aggregate telemetry must identify the expected CLI version and root/leaf model s Version `0.16.0` separates these PR Review routing and usage-eligibility changes from earlier benchmark results. Its default harness pins are Copilot CLI `1.0.88` (PR Review uses the telemetry-compatible `1.0.83`), Claude Code `2.1.281`, and BC-ALAgents `03239afe611a3eff490002eba0e4098b48099ba3`. The `run pr-review` and `evaluate pr-review` commands default to a Sol root and Luna leaves, matching the workflow. +When a run exactly matches a release-owned PR Review definition, the default dashboard displays that **Definition** name. Standard single-model agents, historical rows, and PR Review rows with no recognized definition keep their existing model display instead. Historical data is never rewritten or guessed; only validated runtime evidence can produce a definition. The v1 registry's BCQuality commit and source snapshot are taken from the successful [evaluation-identity run 36001653480](https://github.com/microsoft/BC-Bench/actions/runs/36001653480) manifest artifact. The [Advanced Metrics view](code-review-details.html) shows the definition ID alongside the underlying root/leaf, scheduling, policy, engine, CLI, and BCQuality provenance. + Unavailable AI credits remain `null` in bceval exports; observed zero remains zero. The pinned bc-eval 0.6.1 consumer requires numeric prompt/completion tokens, so its existing zero fallbacks for missing tokens remain unchanged. Use the original per-entry result metrics, not bceval token fields, to distinguish unknown usage from measured zero. ## Production BC PR Review Baseline @@ -72,7 +74,7 @@ Unavailable AI credits remain `null` in bceval exports; observed zero remains ze Agent - Model + Definition Micro F1 (95% CI) Precision Recall @@ -88,7 +90,7 @@ Unavailable AI credits remain `null` in bceval exports; observed zero remains ze {% if agg.experiment == null or agg.experiment.is_experiment == false %} {{ agg.agent_name }} - {{ agg.model }} + {{ agg.definition_name | default: agg.model }} {{ agg.f1 | times: 100.0 | round: 1 }}%{% if agg.f1_ci_low %} ({{ agg.f1_ci_low | times: 100.0 | round: 1 }}-{{ agg.f1_ci_high | times: 100.0 | round: 1 }}%){% endif %} {{ agg.precision | times: 100.0 | round: 1 }}% {{ agg.recall | times: 100.0 | round: 1 }}% @@ -111,7 +113,7 @@ Unavailable AI credits remain `null` in bceval exports; observed zero remains ze Agent - Model + Definition Avg Time Avg Prompt Tokens Avg Completion Tokens @@ -127,7 +129,7 @@ Unavailable AI credits remain `null` in bceval exports; observed zero remains ze {% if agg.experiment == null or agg.experiment.is_experiment == false %} {{ agg.agent_name }} - {{ agg.model }} + {{ agg.definition_name | default: agg.model }} {{ agg.average_duration | round: 1 }}s {% if agg.average_prompt_tokens != null %}{{ agg.average_prompt_tokens | round: 0 }}{% else %}—{% endif %} {% if agg.average_completion_tokens != null %}{{ agg.average_completion_tokens | round: 0 }}{% else %}—{% endif %} @@ -153,7 +155,7 @@ These historical rows evaluate the generic Copilot or Claude runners against the Agent - Model + Definition Micro F1 Precision Recall @@ -166,7 +168,7 @@ These historical rows evaluate the generic Copilot or Claude runners against the {% for agg in legacy_results %} {{ agg.agent_name }} - {{ agg.model }} + {{ agg.definition_name | default: agg.model }} {{ agg.f1 | times: 100.0 | round: 1 }}% {{ agg.precision | times: 100.0 | round: 1 }}% {{ agg.recall | times: 100.0 | round: 1 }}% @@ -194,7 +196,7 @@ For detailed aggregate and per-run quality, performance, configuration, and usag Variant Agent - Model + Definition Micro F1 (95% CI) Macro F1 (95% CI) Precision @@ -212,7 +214,7 @@ For detailed aggregate and per-run quality, performance, configuration, and usag {%- if agg.experiment.custom_instructions -%}Inline knowledge (pre-#8700){%- else -%}Other{%- endif -%} {{ agg.agent_name }} - {{ agg.model }} + {{ agg.definition_name | default: agg.model }} {{ agg.f1 | times: 100.0 | round: 1 }}%{% if agg.f1_ci_low %} ({{ agg.f1_ci_low | times: 100.0 | round: 1 }}-{{ agg.f1_ci_high | times: 100.0 | round: 1 }}%){% endif %} {{ agg.macro_f1 | times: 100.0 | round: 1 }}%{% if agg.macro_f1_ci_low %} ({{ agg.macro_f1_ci_low | times: 100.0 | round: 1 }}-{{ agg.macro_f1_ci_high | times: 100.0 | round: 1 }}%){% endif %} {{ agg.precision | times: 100.0 | round: 1 }}% diff --git a/src/bcbench/agent/pr_review/agent.py b/src/bcbench/agent/pr_review/agent.py index 91ced4dcf..742e986af 100644 --- a/src/bcbench/agent/pr_review/agent.py +++ b/src/bcbench/agent/pr_review/agent.py @@ -24,6 +24,7 @@ from bcbench_core.git import commit_changes, has_changes, init_repo from bcbench.agent.copilot.cli import get_copilot_version +from bcbench.agent.pr_review.definitions import validate_requested_pr_review_definition 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.agent.pr_review.run_manifest import RUN_MANIFEST_FILE_NAME, load_run_manifest, validate_run_manifest @@ -183,6 +184,7 @@ def run_pr_review_agent( leaf_execution: Literal["serial", "parallel"] = "serial", max_leaf_concurrency: int = 4, cli_timeout_minutes: int = 30, + definition_id: str | None = None, ) -> tuple[PRReviewMetrics, ExperimentConfiguration]: """Run the engine's complete local review pipeline and write review.json. @@ -221,6 +223,15 @@ def run_pr_review_agent( raise AgentError("PR Review max leaf concurrency must be a positive integer.") if cli_timeout_minutes < 0: raise AgentError("PR Review CLI timeout minutes must be a non-negative integer.") + requested_definition = validate_requested_pr_review_definition( + definition_id, + engine_commit=agent_version, + copilot_cli_version=cli_version, + root_model=model, + leaf_model=leaf_model, + leaf_execution=leaf_execution, + max_leaf_concurrency=max_leaf_concurrency, + ) engine = engine_root / "agents" / "ALReviewAgent" / "scripts" / "Invoke-CopilotPRReview.ps1" env = { @@ -239,6 +250,7 @@ def run_pr_review_agent( "COPILOT_REVIEW_LEAF_EXECUTION": leaf_execution, "COPILOT_REVIEW_MAX_LEAF_CONCURRENCY": str(max_leaf_concurrency), "COPILOT_REVIEW_CLI_TIMEOUT_MINUTES": str(cli_timeout_minutes), + "MINIMUM_SEVERITY": settings["min_severity"], "AGENT_MINIMUM_SEVERITY": severity, } @@ -282,13 +294,13 @@ def run_pr_review_agent( logger.exception("Unexpected error running engine review") raise else: - return ( - build_pr_review_metrics( - output_dir, - bcquality_root, - time.monotonic() - start, - manifest=manifest, - engine_root=engine_root, - ), - config, + metrics = build_pr_review_metrics( + output_dir, + bcquality_root, + time.monotonic() - start, + manifest=manifest, + engine_root=engine_root, ) + if requested_definition is not None and metrics.definition_id != requested_definition.id: + raise AgentError(f"Validated engine evidence did not resolve to requested BC PR Review definition {requested_definition.id!r}.") + return metrics, config diff --git a/src/bcbench/agent/pr_review/definitions.py b/src/bcbench/agent/pr_review/definitions.py new file mode 100644 index 000000000..96de66ab1 --- /dev/null +++ b/src/bcbench/agent/pr_review/definitions.py @@ -0,0 +1,173 @@ +from pydantic import BaseModel, ConfigDict, Field + +from bcbench.agent.pr_review.run_manifest import RunManifest +from bcbench.exceptions import AgentError + +UNCLASSIFIED_PR_REVIEW_DEFINITION_ID = "unclassified" + + +class PRReviewDefinition(BaseModel): + """A release-owned, fully validated BC PR Review configuration.""" + + model_config = ConfigDict(extra="forbid", frozen=True, strict=True) + + id: str = Field(pattern=r"^[a-z0-9]+(?:-[a-z0-9]+)*$") + display_name: str + root_model: str + leaf_model: str + leaf_execution: str + max_leaf_concurrency: int = Field(ge=1) + engine_commit: str = Field(pattern=r"^[0-9a-f]{40}$") + copilot_cli_version: str + bcquality_repository: str + bcquality_commit: str = Field(pattern=r"^[0-9a-f]{40}$") + bcquality_source_snapshot: str = Field(pattern=r"^[0-9a-f]{64}$") + cli_timeout_minutes: int = Field(ge=0) + minimum_severity: str + agent_minimum_severity: str + review_source: str + + def runtime_mismatches( + self, + *, + engine_commit: str, + copilot_cli_version: str, + root_model: str, + leaf_model: str, + leaf_execution: str, + max_leaf_concurrency: int, + bcquality_repository: str | None = None, + bcquality_commit: str | None = None, + bcquality_source_snapshot: str | None = None, + cli_timeout_minutes: int | None = None, + minimum_severity: str | None = None, + agent_minimum_severity: str | None = None, + review_source: str | None = None, + ) -> list[str]: + expected_values = { + "engine_commit": self.engine_commit, + "copilot_cli_version": self.copilot_cli_version, + "root_model": self.root_model, + "leaf_model": self.leaf_model, + "leaf_execution": self.leaf_execution, + "max_leaf_concurrency": self.max_leaf_concurrency, + } + actual_values = { + "engine_commit": engine_commit, + "copilot_cli_version": copilot_cli_version, + "root_model": root_model, + "leaf_model": leaf_model, + "leaf_execution": leaf_execution, + "max_leaf_concurrency": max_leaf_concurrency, + } + optional_values = { + "bcquality_repository": (bcquality_repository, self.bcquality_repository), + "bcquality_commit": (bcquality_commit, self.bcquality_commit), + "bcquality_source_snapshot": (bcquality_source_snapshot, self.bcquality_source_snapshot), + "cli_timeout_minutes": (cli_timeout_minutes, self.cli_timeout_minutes), + "minimum_severity": (minimum_severity, self.minimum_severity), + "agent_minimum_severity": (agent_minimum_severity, self.agent_minimum_severity), + "review_source": (review_source, self.review_source), + } + mismatches = [f"{name}={actual_values[name]!r} (expected {expected!r})" for name, expected in expected_values.items() if actual_values[name] != expected] + mismatches.extend(f"{name}={actual!r} (expected {expected!r})" for name, (actual, expected) in optional_values.items() if actual is not None and actual != expected) + return mismatches + + def mismatches_for_manifest(self, manifest: RunManifest, *, bcquality_repository: str | None) -> list[str]: + configuration = manifest.configuration + return self.runtime_mismatches( + engine_commit=manifest.engine.commit or "", + copilot_cli_version=configuration.copilot_cli_version, + root_model=configuration.root_model, + leaf_model=configuration.leaf_model, + leaf_execution=configuration.leaf_execution, + max_leaf_concurrency=configuration.max_leaf_concurrency, + bcquality_repository=bcquality_repository, + bcquality_commit=manifest.bcquality.commit, + bcquality_source_snapshot=manifest.bcquality.source_snapshot, + cli_timeout_minutes=configuration.cli_timeout_minutes, + minimum_severity=configuration.minimum_severity, + agent_minimum_severity=configuration.agent_minimum_severity, + review_source=configuration.review_source, + ) + + +PR_REVIEW_DEFINITIONS: tuple[PRReviewDefinition, ...] = ( + PRReviewDefinition( + id="pr-review-production-sol-luna-serial-v1", + display_name="BC PR Review — Production / Sol / Luna / Serial v1", + root_model="gpt-5.6-sol", + leaf_model="gpt-5.6-luna", + leaf_execution="serial", + max_leaf_concurrency=4, + engine_commit="03239afe611a3eff490002eba0e4098b48099ba3", + copilot_cli_version="1.0.83", + bcquality_repository="microsoft/BCQuality", + bcquality_commit="b74967bc5b7a454eae19d6a1250199afd869f064", + bcquality_source_snapshot="096dca9a79cc20bb42bfd0d42d317672f39270e03532c119ac7c47084323d903", + cli_timeout_minutes=30, + minimum_severity="Medium", + agent_minimum_severity="Medium", + review_source="local", + ), +) +DEFAULT_PR_REVIEW_DEFINITION_ID = PR_REVIEW_DEFINITIONS[0].id + + +def get_pr_review_definition(definition_id: str) -> PRReviewDefinition: + for definition in PR_REVIEW_DEFINITIONS: + if definition.id == definition_id: + return definition + raise AgentError(f"Unknown BC PR Review definition {definition_id!r}. Register its complete immutable configuration before using it.") + + +def resolve_requested_pr_review_definition(definition_id: str | None) -> PRReviewDefinition | None: + if definition_id is None or definition_id == UNCLASSIFIED_PR_REVIEW_DEFINITION_ID: + return None + return get_pr_review_definition(definition_id) + + +def validate_requested_pr_review_definition( + definition_id: str | None, + *, + engine_commit: str, + copilot_cli_version: str, + root_model: str, + leaf_model: str, + leaf_execution: str, + max_leaf_concurrency: int, +) -> PRReviewDefinition | None: + definition = resolve_requested_pr_review_definition(definition_id) + if definition is None: + return None + + mismatches = definition.runtime_mismatches( + engine_commit=engine_commit, + copilot_cli_version=copilot_cli_version, + root_model=root_model, + leaf_model=leaf_model, + leaf_execution=leaf_execution, + max_leaf_concurrency=max_leaf_concurrency, + ) + if mismatches: + raise AgentError(f"Requested BC PR Review definition {definition.id!r} does not match configured inputs: {'; '.join(mismatches)}") + return definition + + +def resolve_pr_review_definition(manifest: RunManifest, *, bcquality_repository: str | None) -> PRReviewDefinition | None: + for definition in PR_REVIEW_DEFINITIONS: + if not definition.mismatches_for_manifest(manifest, bcquality_repository=bcquality_repository): + return definition + return None + + +def validate_pr_review_definition_metadata(definition_id: str | None, definition_name: str | None) -> None: + if definition_id is None and definition_name is None: + return + if definition_id is None or definition_name is None: + raise ValueError("PR Review definition_id and definition_name must be set together.") + definition = next((item for item in PR_REVIEW_DEFINITIONS if item.id == definition_id), None) + if definition is None: + raise ValueError(f"Unknown BC PR Review definition {definition_id!r}.") + if definition_name != definition.display_name: + raise ValueError(f"PR Review definition_name does not match registered definition {definition_id!r}.") diff --git a/src/bcbench/agent/pr_review/metrics.py b/src/bcbench/agent/pr_review/metrics.py index 45b33d094..76044228a 100644 --- a/src/bcbench/agent/pr_review/metrics.py +++ b/src/bcbench/agent/pr_review/metrics.py @@ -8,6 +8,7 @@ import yaml from pydantic import BaseModel, ConfigDict, Field, ValidationError, model_validator +from bcbench.agent.pr_review.definitions import resolve_pr_review_definition from bcbench.agent.pr_review.run_manifest import RunManifest from bcbench.exceptions import AgentError from bcbench.types import PRReviewMetrics @@ -237,6 +238,7 @@ def build_pr_review_metrics( raise AgentError("BCQuality checkout does not match the validated run manifest.") usage_values_available = run.usage_complete and run.malformed_records == 0 + definition = resolve_pr_review_definition(manifest, bcquality_repository=bcquality_identity[0] if bcquality_identity else None) if manifest else None return PRReviewMetrics( execution_time=execution_time, prompt_tokens=run.prompt_tokens if usage_values_available else None, @@ -267,4 +269,10 @@ def build_pr_review_metrics( max_leaf_concurrency=manifest.configuration.max_leaf_concurrency if manifest else None, bcquality_source_snapshot=manifest.bcquality.source_snapshot if manifest else None, review_process_count=len(manifest.processes) if manifest else None, + cli_timeout_minutes=manifest.configuration.cli_timeout_minutes if manifest else None, + minimum_severity=manifest.configuration.minimum_severity if manifest else None, + agent_minimum_severity=manifest.configuration.agent_minimum_severity if manifest else None, + review_source=manifest.configuration.review_source if manifest else None, + definition_id=definition.id if definition else None, + definition_name=definition.display_name if definition else None, ) diff --git a/src/bcbench/commands/evaluate.py b/src/bcbench/commands/evaluate.py index a8638c212..9218104f1 100644 --- a/src/bcbench/commands/evaluate.py +++ b/src/bcbench/commands/evaluate.py @@ -10,6 +10,7 @@ from bcbench.agent import BCalBackendConfig, get_claude_version, get_copilot_version, get_pr_review_version, run_bcal_agent, run_claude_code, run_copilot_agent, run_pr_review_agent from bcbench.agent.bcal.version import get_bcal_version, verify_bcal_version +from bcbench.agent.pr_review.definitions import DEFAULT_PR_REVIEW_DEFINITION_ID from bcbench.cli_options import ( ClaudeCodeModel, ContainerCompany, @@ -209,6 +210,7 @@ def evaluate_pr_review( leaf_execution: PRReviewLeafExecution = "serial", max_leaf_concurrency: PRReviewMaxLeafConcurrency = 4, cli_timeout_minutes: PRReviewCliTimeoutMinutes = 30, + definition_id: Annotated[str, typer.Option("--definition-id", help="Registered PR Review definition ID, or 'unclassified' for an unpublished experiment")] = DEFAULT_PR_REVIEW_DEFINITION_ID, ) -> None: """ Evaluate BC PR Review on a single code-review entry. @@ -254,6 +256,7 @@ def evaluate_pr_review( leaf_execution=leaf_execution, max_leaf_concurrency=max_leaf_concurrency, cli_timeout_minutes=cli_timeout_minutes, + definition_id=definition_id, ), ) diff --git a/src/bcbench/results/codereview.py b/src/bcbench/results/codereview.py index e4edb77e9..96bca942d 100644 --- a/src/bcbench/results/codereview.py +++ b/src/bcbench/results/codereview.py @@ -1,15 +1,16 @@ import logging from collections.abc import Hashable, Sequence -from typing import Literal, NamedTuple, Self, override +from typing import Any, Literal, NamedTuple, Self, override import numpy as np from bcbench_core.scoring import f1_score, f_beta_score, precision_recall -from pydantic import Field +from pydantic import Field, model_validator from rich.console import Group, RenderableType from rich.panel import Panel from rich.table import Table from scipy.optimize import linear_sum_assignment +from bcbench.agent.pr_review.definitions import validate_pr_review_definition_metadata from bcbench.dataset import BaseDatasetEntry, ReviewComment from bcbench.results.base import BaseEvaluationResult, JudgeScoredEvaluationResult from bcbench.results.summary import JudgeBasedEvaluationResultSummary @@ -201,6 +202,24 @@ class CodeReviewResult(JudgeScoredEvaluationResult): f_beta_05: float = Field(default=0.0, ge=0.0, le=1.0) f_beta_2: float = Field(default=0.0, ge=0.0, le=1.0) severity_mae: float = 0.0 + definition_id: str | None = None + definition_name: str | None = None + + @model_validator(mode="after") + def validate_definition(self) -> Self: + validate_pr_review_definition_metadata(self.definition_id, self.definition_name) + return self + + @classmethod + @override + def _base_fields[E: BaseDatasetEntry](cls, context: EvaluationContext[E]) -> dict[str, Any]: + fields = super()._base_fields(context) + if isinstance(context.metrics, PRReviewMetrics): + fields.update( + definition_id=context.metrics.definition_id, + definition_name=context.metrics.definition_name, + ) + return fields @classmethod def create[E: BaseDatasetEntry]( @@ -350,11 +369,22 @@ class CodeReviewResultSummary(JudgeBasedEvaluationResultSummary): token_coverage_rate: float | None = Field(default=None, ge=0.0, le=1.0) credit_coverage_rate: float | None = Field(default=None, ge=0.0, le=1.0) usage_complete_rate: float | None = Field(default=None, ge=0.0, le=1.0) + cli_timeout_minutes: int | None = Field(default=None, ge=0) + minimum_severity: Literal["Critical", "High", "Medium", "Low"] | None = None + agent_minimum_severity: Literal["Critical", "High", "Medium", "Low"] | None = None + review_source: Literal["pr", "local"] | None = None + definition_id: str | None = None + definition_name: str | None = None # Per-task F1 keyed by instance_id, retained so the leaderboard can bootstrap a confidence # interval over tasks (meaningful even for a single run) instead of only over runs. instance_results: dict[str, float] = Field(default_factory=dict) + @model_validator(mode="after") + def validate_definition(self) -> Self: + validate_pr_review_definition_metadata(self.definition_id, self.definition_name) + return self + @override def combination_key(self) -> tuple[Hashable, ...]: return ( @@ -369,6 +399,11 @@ def combination_key(self) -> tuple[Hashable, ...]: self.max_leaf_concurrency, self.bcquality_source_snapshot, self.review_process_count, + self.cli_timeout_minutes, + self.minimum_severity, + self.agent_minimum_severity, + self.review_source, + self.definition_id, ) def _performance_markdown(self) -> str: @@ -560,6 +595,10 @@ def consistent_runtime_value(name: str) -> str | int | list[str] | None: metric_counts = {name: sum(value is not None for value in metric_values(name)) for name in _PR_REVIEW_METRICS} metric_sums = {name: float(sum(value for value in metric_values(name) if value is not None)) for name in _PR_REVIEW_METRICS} + definitions = {(result.definition_id, result.definition_name) for result in code_review_results} + if len(definitions) > 1: + raise ValueError(f"Cannot summarize code review results with different definitions: {definitions}") + definition_id, definition_name = next(iter(definitions), (None, None)) return summary.model_copy( update={ @@ -613,6 +652,12 @@ def consistent_runtime_value(name: str) -> str | int | list[str] | None: "max_leaf_concurrency": consistent_runtime_value("max_leaf_concurrency"), "bcquality_source_snapshot": consistent_runtime_value("bcquality_source_snapshot"), "review_process_count": consistent_runtime_value("review_process_count"), + "cli_timeout_minutes": consistent_runtime_value("cli_timeout_minutes"), + "minimum_severity": consistent_runtime_value("minimum_severity"), + "agent_minimum_severity": consistent_runtime_value("agent_minimum_severity"), + "review_source": consistent_runtime_value("review_source"), + "definition_id": definition_id, + "definition_name": definition_name, "metric_counts": metric_counts, "metric_sums": metric_sums, } diff --git a/src/bcbench/results/leaderboard.py b/src/bcbench/results/leaderboard.py index 14615ae6b..5b5112e5c 100644 --- a/src/bcbench/results/leaderboard.py +++ b/src/bcbench/results/leaderboard.py @@ -4,12 +4,13 @@ from collections import defaultdict from collections.abc import Sequence from pathlib import Path -from typing import Any, Literal, override +from typing import Any, Literal, Self, override from bcbench_core.scoring import pass_hat_k from bcbench_core.stats import bootstrap_ci -from pydantic import BaseModel, Field, field_validator +from pydantic import BaseModel, Field, field_validator, model_validator +from bcbench.agent.pr_review.definitions import validate_pr_review_definition_metadata from bcbench.results.summary import EvaluationResultSummary, ExecutionBasedEvaluationResultSummary from bcbench.types import EvaluationCategory, ExperimentConfiguration @@ -186,6 +187,17 @@ class CodeReviewLeaderboardAggregate(JudgeBasedLeaderboardAggregate): token_coverage_rate: float | None = None credit_coverage_rate: float | None = None usage_complete_rate: float | None = None + cli_timeout_minutes: int | None = None + minimum_severity: Literal["Critical", "High", "Medium", "Low"] | None = None + agent_minimum_severity: Literal["Critical", "High", "Medium", "Low"] | None = None + review_source: Literal["pr", "local"] | None = None + definition_id: str | None = None + definition_name: str | None = None + + @model_validator(mode="after") + def validate_definition(self) -> Self: + validate_pr_review_definition_metadata(self.definition_id, self.definition_name) + return self @classmethod @override @@ -206,6 +218,12 @@ def _base_fields(cls, runs: Sequence[EvaluationResultSummary]) -> dict[str, Any] "max_leaf_concurrency": first_run.max_leaf_concurrency, "bcquality_source_snapshot": first_run.bcquality_source_snapshot, "review_process_count": first_run.review_process_count, + "cli_timeout_minutes": first_run.cli_timeout_minutes, + "minimum_severity": first_run.minimum_severity, + "agent_minimum_severity": first_run.agent_minimum_severity, + "review_source": first_run.review_source, + "definition_id": first_run.definition_id, + "definition_name": first_run.definition_name, } @classmethod diff --git a/src/bcbench/types.py b/src/bcbench/types.py index d5a8c6260..b8f04c512 100644 --- a/src/bcbench/types.py +++ b/src/bcbench/types.py @@ -165,6 +165,12 @@ class PRReviewMetrics(AgentMetrics): bcquality_version: str | None = None bcquality_source_snapshot: str | None = None review_process_count: int | None = Field(default=None, ge=1) + cli_timeout_minutes: int | None = Field(default=None, ge=0) + minimum_severity: Literal["Critical", "High", "Medium", "Low"] | None = None + agent_minimum_severity: Literal["Critical", "High", "Medium", "Low"] | None = None + review_source: Literal["pr", "local"] | None = None + definition_id: str | None = None + definition_name: str | None = None type AnyAgentMetrics = Annotated[AgentMetrics | PRReviewMetrics, Field(discriminator="kind")] diff --git a/tests/test_pr_review_agent.py b/tests/test_pr_review_agent.py index dbdd7c152..13d4c352a 100644 --- a/tests/test_pr_review_agent.py +++ b/tests/test_pr_review_agent.py @@ -303,6 +303,7 @@ def test_engine_environment_uses_target_repository_and_absolute_paths(tmp_path: assert "BCQUALITY_SHA" not in engine_env assert "BCQUALITY_REF" not in engine_env assert engine_env["GITHUB_REPOSITORY"] == "microsoft/BCApps" + assert engine_env["MINIMUM_SEVERITY"] == "Medium" assert engine_env["AGENT_MINIMUM_SEVERITY"] == "Medium" assert engine_env["COPILOT_REVIEW_CLI_VERSION"] == "1.0.83" assert engine_env["COPILOT_REVIEW_LEAF_MODEL"] == "gpt-5.4" diff --git a/tests/test_pr_review_definitions.py b/tests/test_pr_review_definitions.py new file mode 100644 index 000000000..7b97d5c1c --- /dev/null +++ b/tests/test_pr_review_definitions.py @@ -0,0 +1,101 @@ +from copy import deepcopy + +import pytest + +from bcbench.agent.pr_review.definitions import ( + DEFAULT_PR_REVIEW_DEFINITION_ID, + get_pr_review_definition, + resolve_pr_review_definition, + validate_requested_pr_review_definition, +) +from bcbench.agent.pr_review.run_manifest import RunManifest +from bcbench.exceptions import AgentError + + +def _baseline_manifest_payload() -> dict: + return { + "schema_version": 1, + "status": "completed", + "started_at": "2026-09-24T12:52:10Z", + "completed_at": "2026-09-24T13:05:08Z", + "failure_reason": None, + "engine": { + "repository": "microsoft/BC-ALAgents", + "commit": "03239afe611a3eff490002eba0e4098b48099ba3", + "agent_version": "0.1.0", + }, + "bcquality": { + "commit": "b74967bc5b7a454eae19d6a1250199afd869f064", + "source_snapshot": "096dca9a79cc20bb42bfd0d42d317672f39270e03532c119ac7c47084323d903", + }, + "configuration": { + "copilot_cli_version": "1.0.83", + "root_model": "gpt-5.6-sol", + "leaf_model": "gpt-5.6-luna", + "leaf_execution": "serial", + "max_leaf_concurrency": 4, + "cli_timeout_minutes": 30, + "minimum_severity": "Medium", + "agent_minimum_severity": "Medium", + "review_source": "local", + }, + "plan": { + "skill_id": "al-code-review", + "leaf_count": 1, + "leaf_ids": ["al-security-review"], + }, + "processes": [], + } + + +def _baseline_manifest() -> RunManifest: + return RunManifest.model_validate(_baseline_manifest_payload()) + + +def test_canonical_pr_review_manifest_resolves_baseline_definition() -> None: + definition = resolve_pr_review_definition(_baseline_manifest(), bcquality_repository="microsoft/BCQuality") + + assert definition is not None + assert definition.id == DEFAULT_PR_REVIEW_DEFINITION_ID + assert definition.display_name == "BC PR Review — Production / Sol / Luna / Serial v1" + + +@pytest.mark.parametrize( + "mutation", + [ + lambda payload: payload["configuration"].update(leaf_model="gpt-5.6-sol"), + lambda payload: payload["configuration"].update(leaf_execution="parallel"), + lambda payload: payload["configuration"].update(max_leaf_concurrency=1), + lambda payload: payload["engine"].update(commit="a" * 40), + lambda payload: payload["configuration"].update(copilot_cli_version="1.0.84"), + lambda payload: payload["bcquality"].update(commit="a" * 40), + lambda payload: payload["bcquality"].update(source_snapshot="a" * 64), + lambda payload: payload["configuration"].update(minimum_severity="High"), + ], + ids=["leaf-model", "schedule", "concurrency", "engine", "cli", "bcquality-commit", "bcquality-snapshot", "policy"], +) +def test_baseline_mismatches_are_unclassified(mutation) -> None: + payload = deepcopy(_baseline_manifest_payload()) + mutation(payload) + + definition = resolve_pr_review_definition(RunManifest.model_validate(payload), bcquality_repository="microsoft/BCQuality") + + assert definition is None + + +def test_unknown_definition_cannot_create_an_arbitrary_display_name() -> None: + with pytest.raises(AgentError, match="Unknown BC PR Review definition"): + get_pr_review_definition("my-unreviewed-profile") + + +def test_requested_definition_rejects_mismatched_configured_inputs() -> None: + with pytest.raises(AgentError, match="does not match configured inputs"): + validate_requested_pr_review_definition( + DEFAULT_PR_REVIEW_DEFINITION_ID, + engine_commit="03239afe611a3eff490002eba0e4098b48099ba3", + copilot_cli_version="1.0.83", + root_model="gpt-5.6-sol", + leaf_model="mai-code-1.1-flash", + leaf_execution="serial", + max_leaf_concurrency=4, + ) diff --git a/tests/test_pr_review_metrics_reporting.py b/tests/test_pr_review_metrics_reporting.py index 9aed34d63..65bda68f7 100644 --- a/tests/test_pr_review_metrics_reporting.py +++ b/tests/test_pr_review_metrics_reporting.py @@ -2,6 +2,7 @@ import pytest +from bcbench.agent.pr_review.definitions import DEFAULT_PR_REVIEW_DEFINITION_ID, get_pr_review_definition from bcbench.results.codereview import CodeReviewResultSummary from bcbench.results.leaderboard import CodeReviewLeaderboardAggregate, ExecutionBasedLeaderboardAggregate from bcbench.results.summary import ExecutionBasedEvaluationResultSummary @@ -40,9 +41,18 @@ def _metrics(*, duration: float, scale: int) -> PRReviewMetrics: max_leaf_concurrency=4, bcquality_source_snapshot="b" * 64, review_process_count=2, + cli_timeout_minutes=30, + minimum_severity="Medium", + agent_minimum_severity="Medium", + review_source="local", ) +def _defined_metrics(*, duration: float, scale: int) -> PRReviewMetrics: + definition = get_pr_review_definition(DEFAULT_PR_REVIEW_DEFINITION_ID) + return _metrics(duration=duration, scale=scale).model_copy(update={"definition_id": definition.id, "definition_name": definition.display_name}) + + def test_pr_review_provenance_fields_are_code_review_specific() -> None: provenance_fields = { "copilot_cli_version", @@ -55,6 +65,8 @@ def test_pr_review_provenance_fields_are_code_review_specific() -> None: "max_leaf_concurrency", "bcquality_source_snapshot", "review_process_count", + "definition_id", + "definition_name", } assert provenance_fields <= CodeReviewResultSummary.model_fields.keys() @@ -104,6 +116,10 @@ def test_summary_aggregates_public_pr_review_metrics() -> None: assert summary.max_leaf_concurrency == 4 assert summary.bcquality_source_snapshot == "b" * 64 assert summary.review_process_count == 2 + assert summary.cli_timeout_minutes == 30 + assert summary.minimum_severity == "Medium" + assert summary.agent_minimum_severity == "Medium" + assert summary.review_source == "local" assert summary.metric_counts["knowledge_used"] == 2 assert summary.metric_sums["knowledge_used"] == 15 @@ -283,6 +299,10 @@ def test_leaderboard_propagates_public_pr_review_metrics() -> None: assert aggregate.max_leaf_concurrency == 4 assert aggregate.bcquality_source_snapshot == "b" * 64 assert aggregate.review_process_count == 2 + assert aggregate.cli_timeout_minutes == 30 + assert aggregate.minimum_severity == "Medium" + assert aggregate.agent_minimum_severity == "Medium" + assert aggregate.review_source == "local" def test_github_summary_renders_only_public_performance_metrics() -> None: @@ -366,7 +386,53 @@ def test_summary_and_leaderboard_schemas_include_pr_review_diagnostics() -> None "max_leaf_concurrency", "bcquality_source_snapshot", "review_process_count", + "definition_id", + "definition_name", "metric_counts", "metric_sums", ): assert diagnostic in payload + + +def test_named_definition_is_persisted_and_part_of_code_review_combination_identity() -> None: + named = CodeReviewResultSummary.from_results( + [create_codereview_result(agent_name=AgentHarness.PR_REVIEW, metrics=_defined_metrics(duration=4.0, scale=1))], + run_id="named", + ) + legacy = CodeReviewResultSummary.from_results( + [create_codereview_result(agent_name=AgentHarness.PR_REVIEW, metrics=_metrics(duration=4.0, scale=1))], + run_id="legacy", + ) + + assert named.definition_id == DEFAULT_PR_REVIEW_DEFINITION_ID + assert named.definition_name == "BC PR Review — Production / Sol / Luna / Serial v1" + assert named.combination_key() != legacy.combination_key() + assert CodeReviewLeaderboardAggregate.from_runs([named]).definition_id == DEFAULT_PR_REVIEW_DEFINITION_ID + + with pytest.raises(ValueError, match="different combinations"): + CodeReviewLeaderboardAggregate.from_runs([named, legacy]) + + +def test_generic_code_review_rows_retain_existing_combination_behavior() -> None: + first = CodeReviewResultSummary.from_results( + [create_codereview_result(agent_name=AgentHarness.COPILOT, model="gpt-5.6-sol", metrics=AgentMetrics(execution_time=4.0))], + run_id="first", + ) + second = CodeReviewResultSummary.from_results( + [create_codereview_result(agent_name=AgentHarness.COPILOT, model="gpt-5.6-sol", metrics=AgentMetrics(execution_time=5.0))], + run_id="second", + ) + + aggregate = CodeReviewLeaderboardAggregate.from_runs([first, second]) + + assert first.definition_id is None + assert second.definition_id is None + assert first.combination_key() == second.combination_key() + assert aggregate.num_runs == 2 + + +def test_code_review_result_rejects_an_unregistered_definition_name() -> None: + invalid_metrics = _metrics(duration=4.0, scale=1).model_copy(update={"definition_id": "unregistered", "definition_name": "Arbitrary"}) + + with pytest.raises(ValueError, match="Unknown BC PR Review definition"): + create_codereview_result(agent_name=AgentHarness.PR_REVIEW, metrics=invalid_metrics) diff --git a/tests/test_review_runners.py b/tests/test_review_runners.py index 1c3be84b8..6dbbb0010 100644 --- a/tests/test_review_runners.py +++ b/tests/test_review_runners.py @@ -96,6 +96,7 @@ def test_pr_review_evaluation_is_fixed_to_runner_and_category(tmp_path: Path) -> assert agent_runner.call_args.kwargs["leaf_model"] == "gpt-5.6-luna" assert agent_runner.call_args.kwargs["engine_path"] == tmp_path assert agent_runner.call_args.kwargs["agent_version"] == "a" * 40 + assert agent_runner.call_args.kwargs["definition_id"] == "pr-review-production-sol-luna-serial-v1" get_version.assert_called_once_with(tmp_path) diff --git a/tests/test_review_workflows.py b/tests/test_review_workflows.py index 1340fc839..72457b2b5 100644 --- a/tests/test_review_workflows.py +++ b/tests/test_review_workflows.py @@ -81,6 +81,13 @@ def test_pr_review_workflow_is_fixed_to_code_review() -> None: "gpt-5.3-codex", "mai-code-1.1-flash", ] + assert inputs["definition-id"] == { + "description": "Registered PR Review definition (unclassified runs are never published)", + "required": False, + "default": "pr-review-production-sol-luna-serial-v1", + "type": "choice", + "options": ["pr-review-production-sol-luna-serial-v1", "unclassified"], + } assert inputs["leaf-model"]["default"] == "gpt-5.6-luna" assert inputs["leaf-model"]["options"] == ["gpt-5.4", "gpt-5.6-luna", "mai-code-1.1-flash"] assert inputs["leaf-execution"] == { @@ -106,6 +113,7 @@ def test_pr_review_workflow_is_fixed_to_code_review() -> None: assert "repetition-id" not in workflow for input_name in ( "model:", + "definition-id:", "leaf-model:", "leaf-execution:", "max-leaf-concurrency:", @@ -145,7 +153,7 @@ def test_engine_sha_override_is_never_published_as_a_benchmark_result() -> None: workflow = yaml.safe_load(_workflow("pr-review-evaluation.yml")) summarize = workflow["jobs"]["summarize-results"]["with"] - assert summarize["mock"] == "${{ inputs.test-run || inputs.modified-only || inputs.engine-sha != '' || inputs.entries != '' }}" + assert summarize["mock"] == "${{ inputs.test-run || inputs.modified-only || inputs.engine-sha != '' || inputs.definition-id != 'pr-review-production-sol-luna-serial-v1' || inputs.entries != '' }}" # Repeats stay available so an override can be measured over several runs. assert "inputs.engine-sha" not in workflow["jobs"]["requeue"]["if"] @@ -156,11 +164,16 @@ def test_pr_review_requeue_preserves_engine_sha(engine_sha: str) -> None: payload = workflow["jobs"]["requeue"]["with"]["workflow-inputs"] engine_expression = "${{ toJSON(inputs.engine-sha) }}" entries_expression = "${{ toJSON(inputs.entries) }}" + definition_expression = "${{ inputs.definition-id }}" assert engine_expression in payload assert entries_expression in payload - parsed = json.loads(payload.replace(engine_expression, json.dumps(engine_sha)).replace(entries_expression, json.dumps(""))) + assert definition_expression in payload + parsed = json.loads( + payload.replace(engine_expression, json.dumps(engine_sha)).replace(entries_expression, json.dumps("")).replace(definition_expression, "pr-review-production-sol-luna-serial-v1") + ) assert parsed["engine-sha"] == engine_sha + assert parsed["definition-id"] == "pr-review-production-sol-luna-serial-v1" def test_requeue_workflow_reads_inputs_from_environment() -> None: From 368e7c53574defe63bb7754efbd8965ec6de7b45 Mon Sep 17 00:00:00 2001 From: dayland Date: Thu, 22 Oct 2026 15:36:10 +0100 Subject: [PATCH 2/2] Fix named definition test typing Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6a73db23-7e6d-4e87-a162-f7f16374bc44 --- tests/test_pr_review_definitions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pr_review_definitions.py b/tests/test_pr_review_definitions.py index 7b97d5c1c..f3f8868e1 100644 --- a/tests/test_pr_review_definitions.py +++ b/tests/test_pr_review_definitions.py @@ -12,7 +12,7 @@ from bcbench.exceptions import AgentError -def _baseline_manifest_payload() -> dict: +def _baseline_manifest_payload() -> dict[str, object]: return { "schema_version": 1, "status": "completed",