diff --git a/.github/actions/install-agent-harnesses/action.yml b/.github/actions/install-agent-harnesses/action.yml index 8b7a408f6..c9ff94c19 100644 --- a/.github/actions/install-agent-harnesses/action.yml +++ b/.github/actions/install-agent-harnesses/action.yml @@ -19,7 +19,7 @@ runs: id: engine-sha shell: pwsh env: - ENGINE_SHA: ${{ inputs.engine-sha || 'ecf8e31759d6ddd6d78e3a0b7836b40134368009' }} + ENGINE_SHA: ${{ inputs.engine-sha || 'fdc02d7020632795810057500d62cff2a61513d7' }} run: | if ($env:ENGINE_SHA -notmatch '\A[0-9a-fA-F]{40}\z') { throw "engine-sha must be a full 40-character hexadecimal commit SHA." diff --git a/docs/code-review-details.md b/docs/code-review-details.md new file mode 100644 index 000000000..5d2477485 --- /dev/null +++ b/docs/code-review-details.md @@ -0,0 +1,280 @@ +--- +layout: default +title: Code Review Advanced Metrics - BC-Bench +--- + + + +# Code Review Advanced Metrics + +This view exposes every quality, performance, configuration, and usage metric persisted in the public code-review leaderboard data. The [default leaderboard](code-review.html) keeps only the headline metrics. + +Diagnostic averages use only tasks that reported the metric. Coverage columns show what share of tasks contributed complete token or credit telemetry. + +## Aggregate metrics + +{% if site.data.code-review.aggregate and site.data.code-review.aggregate.size > 0 %} +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + {% assign aggregate_results = site.data.code-review.aggregate | sort: "f1" | reverse %} + {% for agg in aggregate_results %} + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + {% assign engine_version = agg.agent_version | default: agg.bc_alagents_commit %} + + + + {% endfor %} + +
AgentModelConfigurationRunsTasksMicro PrecisionMicro RecallMicro F1Micro F1 95% CIMicro F0.5Micro F2Macro PrecisionMacro RecallMacro F1Macro F1 95% CIMacro F0.5Macro F2Valid OutputAvg TimeAvg Prompt TokensAvg Completion TokensAvg Total TokensAvg AI CreditsToken CoverageCredit CoverageUsage CompleteAvg Cached TokensAvg Cache Creation TokensAvg Reasoning TokensAvg API CallsAvg Failed API CallsAvg Calls With UsageAvg Malformed RecordsAvg Articles RetainedAvg Articles PrunedAvg Articles Used in FindingsAvg Articles SuppressedAvg Sub-skills ExecutedAvg Sub-skills SkippedJudgeBC-BenchCopilot CLIBC-ALAgentsBCQuality
{{ agg.agent_name }}{{ agg.model }}{% if agg.experiment %}{{ agg.experiment | jsonify }}{% else %}Baseline{% endif %}{{ agg.num_runs }}{{ agg.total }}{{ agg.precision | times: 100.0 | round: 1 }}%{{ agg.recall | times: 100.0 | round: 1 }}%{{ agg.f1 | times: 100.0 | round: 1 }}%{% if agg.f1_ci_low != null %}{{ agg.f1_ci_low | times: 100.0 | round: 1 }}-{{ agg.f1_ci_high | times: 100.0 | round: 1 }}%{% else %}—{% endif %}{{ agg.f_beta_05 | times: 100.0 | round: 1 }}%{{ agg.f_beta_2 | times: 100.0 | round: 1 }}%{{ agg.macro_precision | times: 100.0 | round: 1 }}%{{ agg.macro_recall | times: 100.0 | round: 1 }}%{{ agg.macro_f1 | times: 100.0 | round: 1 }}%{% if agg.macro_f1_ci_low != null %}{{ agg.macro_f1_ci_low | times: 100.0 | round: 1 }}-{{ agg.macro_f1_ci_high | times: 100.0 | round: 1 }}%{% else %}—{% endif %}{{ agg.macro_f_beta_05 | times: 100.0 | round: 1 }}%{{ agg.macro_f_beta_2 | times: 100.0 | round: 1 }}%{% if agg.valid_review_output_rate != null %}{{ agg.valid_review_output_rate | times: 100.0 | round: 1 }}%{% else %}—{% endif %}{{ 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 %}{% if agg.average_total_tokens != null %}{{ agg.average_total_tokens | round: 0 }}{% else %}—{% endif %}{% if agg.average_ai_credits != null %}{{ agg.average_ai_credits | round: 4 }}{% else %}—{% endif %}{% if agg.token_coverage_rate != null %}{{ agg.token_coverage_rate | times: 100.0 | round: 1 }}%{% else %}—{% endif %}{% if agg.credit_coverage_rate != null %}{{ agg.credit_coverage_rate | times: 100.0 | round: 1 }}%{% else %}—{% endif %}{% if agg.usage_complete_rate != null %}{{ agg.usage_complete_rate | times: 100.0 | round: 1 }}%{% else %}—{% endif %}{% if agg.average_cached_tokens != null %}{{ agg.average_cached_tokens | round: 0 }}{% else %}—{% endif %}{% if agg.average_cache_creation_tokens != null %}{{ agg.average_cache_creation_tokens | round: 0 }}{% else %}—{% endif %}{% if agg.average_reasoning_tokens != null %}{{ agg.average_reasoning_tokens | round: 0 }}{% else %}—{% endif %}{% if agg.average_api_calls != null %}{{ agg.average_api_calls | round: 1 }}{% else %}—{% endif %}{% if agg.average_failed_api_calls != null %}{{ agg.average_failed_api_calls | round: 1 }}{% else %}—{% endif %}{% if agg.average_usage_api_calls != null %}{{ agg.average_usage_api_calls | round: 1 }}{% else %}—{% endif %}{% if agg.average_malformed_records != null %}{{ agg.average_malformed_records | round: 1 }}{% else %}—{% endif %}{% if agg.average_knowledge_files != null %}{{ agg.average_knowledge_files | round: 1 }}{% else %}—{% endif %}{% if agg.average_knowledge_pruned != null %}{{ agg.average_knowledge_pruned | round: 1 }}{% else %}—{% endif %}{% if agg.average_knowledge_used != null %}{{ agg.average_knowledge_used | round: 1 }}{% else %}—{% endif %}{% if agg.average_knowledge_suppressed != null %}{{ agg.average_knowledge_suppressed | round: 1 }}{% else %}—{% endif %}{% if agg.average_sub_skills_executed != null %}{{ agg.average_sub_skills_executed | round: 1 }}{% else %}—{% endif %}{% if agg.average_sub_skills_skipped != null %}{{ agg.average_sub_skills_skipped | round: 1 }}{% else %}—{% endif %}{{ agg.judge_model }}{{ agg.benchmark_version }}{% if agg.copilot_cli_version %}{{ agg.copilot_cli_version }}{% else %}—{% endif %}{% if agg.agent_name == "BC PR Review" and engine_version %}{{ engine_version | 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 %}
+
+{% else %} +

No aggregate results available.

+{% endif %} + +## Individual run metrics + +{% if site.data.code-review.runs and site.data.code-review.runs.size > 0 %} +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + {% assign run_results = site.data.code-review.runs | sort: "date" | reverse %} + {% for run in run_results %} + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + {% assign engine_version = run.agent_version | default: run.bc_alagents_commit %} + + + + {% endfor %} + +
RunAgentModelConfigurationDateTasksGeneratedExpectedMatchedIncorrectMissedIgnoredMicro PrecisionMicro RecallMicro F1Micro F0.5Micro F2Macro PrecisionMacro RecallMacro F1Macro F0.5Macro F2Severity MAEValid OutputAvg TimeAvg LLM TimeAvg Prompt TokensAvg Completion TokensAvg Total TokensAvg AI CreditsToken CoverageCredit CoverageUsage CompleteAvg Cached TokensAvg Cache Creation TokensAvg Reasoning TokensAvg API CallsAvg Failed API CallsAvg Calls With UsageAvg Malformed RecordsAvg Articles RetainedAvg Articles PrunedAvg Articles Used in FindingsAvg Articles SuppressedAvg Sub-skills ExecutedAvg Sub-skills SkippedJudgeBC-BenchCopilot CLIBC-ALAgentsBCQuality
{% if run.github_run_id %}{{ run.github_run_id }}{% else %}—{% endif %}{{ run.agent_name }}{{ run.model }}{% if run.experiment %}{{ run.experiment | jsonify }}{% else %}Baseline{% endif %}{{ run.date }}{{ run.total }}{{ run.generated_comment_count }}{{ run.expected_comment_count }}{{ run.matched_comment_count }}{{ run.incorrect_comment_count }}{{ run.missed_comment_count }}{{ run.ignored_matched_comment_count }}{{ run.precision | times: 100.0 | round: 1 }}%{{ run.recall | times: 100.0 | round: 1 }}%{{ run.f1 | times: 100.0 | round: 1 }}%{{ run.f_beta_05 | times: 100.0 | round: 1 }}%{{ run.f_beta_2 | times: 100.0 | round: 1 }}%{{ run.macro_precision | times: 100.0 | round: 1 }}%{{ run.macro_recall | times: 100.0 | round: 1 }}%{{ run.macro_f1 | times: 100.0 | round: 1 }}%{{ run.macro_f_beta_05 | times: 100.0 | round: 1 }}%{{ run.macro_f_beta_2 | times: 100.0 | round: 1 }}%{{ run.severity_mae | round: 3 }}{% if run.valid_review_output_rate != null %}{{ run.valid_review_output_rate | times: 100.0 | round: 1 }}%{% else %}—{% endif %}{{ run.average_duration | round: 1 }}s{% if run.average_llm_duration != null %}{{ run.average_llm_duration | round: 1 }}s{% else %}—{% endif %}{% if run.average_prompt_tokens != null %}{{ run.average_prompt_tokens | round: 0 }}{% else %}—{% endif %}{% if run.average_completion_tokens != null %}{{ run.average_completion_tokens | round: 0 }}{% else %}—{% endif %}{% if run.average_total_tokens != null %}{{ run.average_total_tokens | round: 0 }}{% else %}—{% endif %}{% if run.average_ai_credits != null %}{{ run.average_ai_credits | round: 4 }}{% else %}—{% endif %}{% if run.token_coverage_rate != null %}{{ run.token_coverage_rate | times: 100.0 | round: 1 }}%{% else %}—{% endif %}{% if run.credit_coverage_rate != null %}{{ run.credit_coverage_rate | times: 100.0 | round: 1 }}%{% else %}—{% endif %}{% if run.usage_complete_rate != null %}{{ run.usage_complete_rate | times: 100.0 | round: 1 }}%{% else %}—{% endif %}{% if run.average_cached_tokens != null %}{{ run.average_cached_tokens | round: 0 }}{% else %}—{% endif %}{% if run.average_cache_creation_tokens != null %}{{ run.average_cache_creation_tokens | round: 0 }}{% else %}—{% endif %}{% if run.average_reasoning_tokens != null %}{{ run.average_reasoning_tokens | round: 0 }}{% else %}—{% endif %}{% if run.average_api_calls != null %}{{ run.average_api_calls | round: 1 }}{% else %}—{% endif %}{% if run.average_failed_api_calls != null %}{{ run.average_failed_api_calls | round: 1 }}{% else %}—{% endif %}{% if run.average_usage_api_calls != null %}{{ run.average_usage_api_calls | round: 1 }}{% else %}—{% endif %}{% if run.average_malformed_records != null %}{{ run.average_malformed_records | round: 1 }}{% else %}—{% endif %}{% if run.average_knowledge_files != null %}{{ run.average_knowledge_files | round: 1 }}{% else %}—{% endif %}{% if run.average_knowledge_pruned != null %}{{ run.average_knowledge_pruned | round: 1 }}{% else %}—{% endif %}{% if run.average_knowledge_used != null %}{{ run.average_knowledge_used | round: 1 }}{% else %}—{% endif %}{% if run.average_knowledge_suppressed != null %}{{ run.average_knowledge_suppressed | round: 1 }}{% else %}—{% endif %}{% if run.average_sub_skills_executed != null %}{{ run.average_sub_skills_executed | round: 1 }}{% else %}—{% endif %}{% if run.average_sub_skills_skipped != null %}{{ run.average_sub_skills_skipped | round: 1 }}{% else %}—{% endif %}{{ run.judge_model }}{{ run.benchmark_version }}{% if run.copilot_cli_version %}{{ run.copilot_cli_version }}{% else %}—{% endif %}{% if run.agent_name == "BC PR Review" and engine_version %}{{ engine_version | 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 %}
+
+{% else %} +

No individual run results available.

+{% endif %} + +[← Back to Code Review](code-review.html) diff --git a/docs/code-review.md b/docs/code-review.md index 8d7c92057..9eaa6a46d 100644 --- a/docs/code-review.md +++ b/docs/code-review.md @@ -47,11 +47,11 @@ The `pr-review` workflow also accepts an `engine-sha` input — a full 40-charac Either way, hold everything else fixed: the benchmark version, the model, the Copilot CLI version the engine uses internally, and the configured minimum severity. Locally, `bcbench evaluate pr-review --engine-path ` requires a clean engine checkout and uses the configured severity; use `bcbench run pr-review` for dirty-checkout smoke tests or `--min-severity` overrides. -BC PR Review records wall-clock duration, prompt/completion/total tokens, and exact AI credits. Usage values come from the engine's strictly validated schema-v1 `_run-metrics.json`, never from console transcripts. API-call details, knowledge-filter counts, token subcategories, completeness diagnostics, and producer metadata remain in that raw artifact rather than being promoted into BC-Bench result and leaderboard schemas. +BC PR Review records wall-clock duration, prompt/completion/total tokens, and exact AI credits. Usage values come from the engine's strictly validated schema-v1 `_run-metrics.json`, never from console transcripts. API-call details, knowledge-filter counts, token subcategories, completeness diagnostics, and producer metadata are retained for the [Advanced Metrics view](code-review-details.html). Unavailable AI credits remain `null` in bceval exports; observed zero remains zero. The pinned bc-eval 0.3.14 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. -## Baseline Leaderboard +## Production BC PR Review Baseline {% if site.data.code-review.aggregate and site.data.code-review.aggregate.size > 0 %} @@ -64,11 +64,12 @@ Unavailable AI credits remain `null` in bceval exports; observed zero remains ze - + - {% assign sorted_results = site.data.code-review.aggregate | sort: "f1" | reverse %} + {% assign production_results = site.data.code-review.aggregate | where: "agent_name", "BC PR Review" %} + {% assign sorted_results = production_results | sort: "f1" | reverse %} {% for agg in sorted_results %} {% if agg.experiment == null or agg.experiment.is_experiment == false %} @@ -89,7 +90,7 @@ Unavailable AI credits remain `null` in bceval exports; observed zero remains ze

No results available yet. Check back soon!

{% endif %} -## Performance Leaderboard +## Production BC PR Review Performance {% if site.data.code-review.aggregate and site.data.code-review.aggregate.size > 0 %}
Recall Valid Output Avg TimeVerVersion
@@ -102,11 +103,12 @@ Unavailable AI credits remain `null` in bceval exports; observed zero remains ze - + - {% assign performance_results = site.data.code-review.aggregate | sort: "average_duration" %} + {% assign production_results = site.data.code-review.aggregate | where: "agent_name", "BC PR Review" %} + {% assign performance_results = production_results | sort: "average_duration" %} {% for agg in performance_results %} @@ -125,12 +127,49 @@ Unavailable AI credits remain `null` in bceval exports; observed zero remains ze

No performance results available yet. Check back soon!

{% endif %} +## Legacy Direct-Agent Results + +These historical rows evaluate the generic Copilot or Claude runners against the same code-review dataset and scorer. They are retained for reference but are not directly comparable to the production `BC-Bench -> BC-ALAgents -> BCQuality` pipeline above. + +{% assign legacy_results = site.data.code-review.aggregate | where_exp: "agg", "agg.agent_name != 'BC PR Review'" %} +{% if legacy_results and legacy_results.size > 0 %} +
Avg Completion Tokens Avg Total Tokens Avg AI CreditsVerVersion
{{ agg.agent_name }}
+ + + + + + + + + + + + + {% assign legacy_results = legacy_results | sort: "f1" | reverse %} + {% for agg in legacy_results %} + + + + + + + + + + {% endfor %} + +
AgentModelMicro F1PrecisionRecallAvg TimeBC-Bench
{{ agg.agent_name }}{{ agg.model }}{{ agg.f1 | times: 100.0 | round: 1 }}%{{ agg.precision | times: 100.0 | round: 1 }}%{{ agg.recall | times: 100.0 | round: 1 }}%{{ agg.average_duration | round: 1 }}s{{ agg.benchmark_version }}
+{% endif %} + ## Experiment Leaderboard -Compares review-knowledge configurations for the same model (see the Baseline Leaderboard above for the plain agent): +Compares review-knowledge configurations for the same runner and model. Runner identity remains visible because historical direct-agent experiments are not comparable to production BC PR Review experiments. - **Inline knowledge (pre-#8700)** — the review checklists BCApps shipped inline before adopting BCQuality, injected as custom instructions. +The default tables follow the shared BC-Bench dashboard convention and show the benchmark version. The generic `agent_version` field identifies the evaluated harness revision. For detailed aggregate and per-run quality, performance, configuration, usage, and transitive BCQuality lineage, open the [Advanced Metrics view](code-review-details.html). + {% assign experiment_rows = site.data.code-review.aggregate | where_exp: "agg", "agg.experiment != null" %} {% assign experiment_rows = experiment_rows | where_exp: "agg", "agg.experiment.is_experiment != false" %} {% if experiment_rows and experiment_rows.size > 0 %} @@ -146,7 +185,7 @@ Compares review-knowledge configurations for the same model (see the Baseline Le Recall Valid Output Avg Time - Ver + Version diff --git a/src/bcbench/agent/pr_review/agent.py b/src/bcbench/agent/pr_review/agent.py index 76c04de85..3ac3d335c 100644 --- a/src/bcbench/agent/pr_review/agent.py +++ b/src/bcbench/agent/pr_review/agent.py @@ -235,4 +235,4 @@ def run_pr_review_agent( logger.exception("Unexpected error running engine review") raise else: - return build_pr_review_metrics(output_dir, time.monotonic() - start), config + return build_pr_review_metrics(output_dir, bcquality_root, time.monotonic() - start, engine_root), config diff --git a/src/bcbench/agent/pr_review/metrics.py b/src/bcbench/agent/pr_review/metrics.py index 0d3204697..9adf8a7c6 100644 --- a/src/bcbench/agent/pr_review/metrics.py +++ b/src/bcbench/agent/pr_review/metrics.py @@ -1,17 +1,48 @@ import json +import re +import subprocess +from functools import cache from pathlib import Path from typing import Annotated, Literal +import yaml from pydantic import BaseModel, ConfigDict, Field, ValidationError, model_validator from bcbench.exceptions import AgentError +from bcbench.logger import get_logger from bcbench.types import PRReviewMetrics +logger = get_logger(__name__) + RUN_METRICS_FILE_NAME = "_run-metrics.json" +FILTER_REPORT_FILE_NAME = "_filter-report.json" +FINDINGS_FILE_NAME = "al-code-review-findings.json" +_KNOWLEDGE_LAYERS = {"microsoft", "community", "custom"} _NonNegativeInt = Annotated[int, Field(ge=0)] _NonNegativeFloat = Annotated[float, Field(ge=0)] +class _FilterRemoval(BaseModel): + model_config = ConfigDict(extra="ignore", frozen=True) + + kind: Literal["knowledge", "skill"] + + +class _FilterReport(BaseModel): + model_config = ConfigDict(extra="ignore", frozen=True) + + removed: list[_FilterRemoval] + + +class _EngineDiagnostics(BaseModel): + model_config = ConfigDict(frozen=True) + + knowledge_used: _NonNegativeInt + knowledge_suppressed: _NonNegativeInt + sub_skills_executed: _NonNegativeInt | None + sub_skills_skipped: _NonNegativeInt + + class _RunMetrics(BaseModel): model_config = ConfigDict(extra="forbid", frozen=True, strict=True) @@ -76,10 +107,118 @@ def _load_run_metrics(path: Path) -> _RunMetrics: raise AgentError(f"Engine run metrics artifact {path} does not satisfy schema version 1: {exc}") from exc -def build_pr_review_metrics(output_dir: Path, execution_time: float) -> PRReviewMetrics: +def _load_filter_report(path: Path) -> _FilterReport | None: + if not path.exists(): + return None + try: + payload = json.loads(path.read_text(encoding="utf-8-sig")) + except (json.JSONDecodeError, OSError) as exc: + raise AgentError(f"Could not read BCQuality filter report {path}: {exc}") from exc + try: + return _FilterReport.model_validate(payload) + except ValidationError as exc: + raise AgentError(f"BCQuality filter report {path} has an invalid shape: {exc}") from exc + + +@cache +def _count_available_knowledge(bcquality_root: Path) -> int: + def is_knowledge_file(path: Path) -> bool: + parts = path.relative_to(bcquality_root).parts + return len(parts) >= 3 and parts[0].lower() in _KNOWLEDGE_LAYERS and parts[1].lower() == "knowledge" + + return sum(1 for path in bcquality_root.rglob("*.md") if path.is_file() and is_knowledge_file(path)) + + +def _normalize_knowledge_reference(path: str) -> str | None: + parts = path.replace("\\", "/").strip("/").split("/") + if any(part in {"", ".."} for part in parts) or not parts[-1].lower().endswith(".md"): + return None + for index in range(len(parts) - 2): + if parts[index].lower() in _KNOWLEDGE_LAYERS and parts[index + 1].lower() == "knowledge": + return "/".join(parts[index:]).lower() + return None + + +def _load_engine_diagnostics(path: Path) -> _EngineDiagnostics: + if not path.exists(): + return _EngineDiagnostics(knowledge_used=0, knowledge_suppressed=0, sub_skills_executed=None, sub_skills_skipped=0) + try: + payload = json.loads(path.read_text(encoding="utf-8-sig")) + except (json.JSONDecodeError, OSError) as exc: + raise AgentError(f"Could not read engine findings artifact {path}: {exc}") from exc + if not isinstance(payload, dict): + raise AgentError(f"Engine findings artifact {path} must contain a JSON object.") + + cited: set[str] = set() + for collection_name in ("findings", "subResults"): + collection = payload.get(collection_name, []) + if not isinstance(collection, list): + continue + for item in collection: + if not isinstance(item, dict): + continue + references = item.get("references", []) + if not isinstance(references, list): + continue + for reference in references: + if isinstance(reference, dict) and isinstance(reference.get("path"), str): + normalized = _normalize_knowledge_reference(reference["path"]) + if normalized is not None: + cited.add(normalized) + + sub_results = payload.get("subResults") + skipped_sub_skills = payload.get("skippedSubSkills", []) + suppressed = payload.get("suppressed", []) + return _EngineDiagnostics( + knowledge_used=len(cited), + knowledge_suppressed=len(suppressed) if isinstance(suppressed, list) else 0, + sub_skills_executed=len(sub_results) if isinstance(sub_results, list) else None, + sub_skills_skipped=len(skipped_sub_skills) if isinstance(skipped_sub_skills, list) else 0, + ) + + +def _load_bcquality_identity(engine_root: Path, bcquality_root: Path) -> tuple[str, str, str] | None: + config_path = engine_root / "agents" / "ALReviewAgent" / "bcquality.config.yaml" + try: + config = yaml.safe_load(config_path.read_text(encoding="utf-8")) + bcquality = config["bcquality"] + repository = bcquality["repo"] + version = bcquality["version"] + except (OSError, TypeError, KeyError, yaml.YAMLError) as exc: + logger.warning(f"BCQuality provenance unavailable from {config_path}: {exc}") + return None + if not isinstance(repository, str) or not isinstance(version, str): + logger.warning(f"BCQuality provenance unavailable: {config_path} must contain string repo and version values.") + return None + repository = re.sub(r"\.git$", "", re.sub(r"^(https://github\.com/|git@github\.com:)", "", repository)) + if re.fullmatch(r"[a-zA-Z0-9_-]+/[a-zA-Z0-9_-]+", repository) is None: + logger.warning(f"BCQuality provenance unavailable: invalid repository {repository!r} in {config_path}.") + return None + try: + commit = subprocess.run( + ["git", "-C", str(bcquality_root), "rev-parse", "HEAD"], + capture_output=True, + text=True, + encoding="utf-8", + timeout=30, + check=True, + ).stdout.strip() + except (OSError, subprocess.SubprocessError) as exc: + logger.warning(f"BCQuality provenance unavailable from checkout {bcquality_root}: {exc}") + return None + if re.fullmatch(r"[0-9a-fA-F]{40}", commit) is None: + logger.warning(f"BCQuality provenance unavailable: checkout at {bcquality_root} returned invalid commit {commit!r}.") + return None + return repository, commit, version + + +def build_pr_review_metrics(output_dir: Path, bcquality_root: Path, execution_time: float, engine_root: Path | None = None) -> PRReviewMetrics: run = _load_run_metrics(output_dir / RUN_METRICS_FILE_NAME) if run.metrics_source == "not-applicable": raise AgentError("Engine metrics were not applicable. BC-Bench code-review entries must contain AL changes.") + report = _load_filter_report(bcquality_root / FILTER_REPORT_FILE_NAME) + engine = _load_engine_diagnostics(output_dir / FINDINGS_FILE_NAME) + bcquality_identity = _load_bcquality_identity(engine_root, bcquality_root) if engine_root is not None else None usage_values_available = run.malformed_records == 0 token_values_available = usage_values_available and run.usage_complete return PRReviewMetrics( @@ -96,5 +235,14 @@ def build_pr_review_metrics(output_dir: Path, execution_time: float) -> PRReview usage_api_calls=run.usage_api_calls, usage_complete=run.usage_complete, malformed_records=run.malformed_records, + knowledge_files=_count_available_knowledge(bcquality_root.resolve()), + knowledge_pruned=sum(1 for item in report.removed if item.kind == "knowledge") if report is not None else None, + knowledge_used=engine.knowledge_used, + knowledge_suppressed=engine.knowledge_suppressed, + sub_skills_executed=engine.sub_skills_executed, + sub_skills_skipped=engine.sub_skills_skipped, copilot_cli_version=run.cli_version, + bcquality_repository=bcquality_identity[0] if bcquality_identity else None, + bcquality_commit=bcquality_identity[1] if bcquality_identity else None, + bcquality_version=bcquality_identity[2] if bcquality_identity else None, ) diff --git a/src/bcbench/results/codereview.py b/src/bcbench/results/codereview.py index 4570f8d7a..5684828ec 100644 --- a/src/bcbench/results/codereview.py +++ b/src/bcbench/results/codereview.py @@ -314,6 +314,22 @@ class CodeReviewResultSummary(JudgeBasedEvaluationResultSummary): valid_review_output_rate: float = Field(default=0.0, ge=0.0, le=1.0) average_total_tokens: float | None = None + average_cached_tokens: float | None = None + average_cache_creation_tokens: float | None = None + average_reasoning_tokens: float | None = None + average_api_calls: float | None = None + average_failed_api_calls: float | None = None + average_usage_api_calls: float | None = None + average_malformed_records: float | None = None + average_knowledge_files: float | None = None + average_knowledge_pruned: float | None = None + average_knowledge_used: float | None = None + average_knowledge_suppressed: float | None = None + average_sub_skills_executed: float | None = None + average_sub_skills_skipped: float | None = None + 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) # 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. @@ -488,6 +504,9 @@ def average_metric(values: Sequence[int | float | None]) -> float | None: available = [value for value in values if value is not None] return sum(available) / len(available) if available else None + def metric_values(name: str) -> list[int | float | None]: + return [getattr(result.metrics, name, None) if result.metrics else None for result in code_review_results] + return summary.model_copy( update={ "generated_comment_count": generated_total, @@ -509,9 +528,25 @@ def average_metric(values: Sequence[int | float | None]) -> float | None: "severity_mae": round(severity_mae, 3), "valid_review_output_rate": round(valid_output_rate, 3), "instance_results": {r.instance_id: round(r.f1, 6) for r in code_review_results}, - "average_prompt_tokens": average_metric([result.metrics.prompt_tokens if result.metrics else None for result in code_review_results]), - "average_completion_tokens": average_metric([result.metrics.completion_tokens if result.metrics else None for result in code_review_results]), - "average_total_tokens": average_metric([result.metrics.total_tokens if result.metrics else None for result in code_review_results]), - "average_ai_credits": average_metric([result.metrics.ai_credits if result.metrics else None for result in code_review_results]), + "average_prompt_tokens": average_metric(metric_values("prompt_tokens")), + "average_completion_tokens": average_metric(metric_values("completion_tokens")), + "average_total_tokens": average_metric(metric_values("total_tokens")), + "average_ai_credits": average_metric(metric_values("ai_credits")), + "average_cached_tokens": average_metric(metric_values("cached_tokens")), + "average_cache_creation_tokens": average_metric(metric_values("cache_creation_tokens")), + "average_reasoning_tokens": average_metric(metric_values("reasoning_tokens")), + "average_api_calls": average_metric(metric_values("api_calls")), + "average_failed_api_calls": average_metric(metric_values("failed_api_calls")), + "average_usage_api_calls": average_metric(metric_values("usage_api_calls")), + "average_malformed_records": average_metric(metric_values("malformed_records")), + "average_knowledge_files": average_metric(metric_values("knowledge_files")), + "average_knowledge_pruned": average_metric(metric_values("knowledge_pruned")), + "average_knowledge_used": average_metric(metric_values("knowledge_used")), + "average_knowledge_suppressed": average_metric(metric_values("knowledge_suppressed")), + "average_sub_skills_executed": average_metric(metric_values("sub_skills_executed")), + "average_sub_skills_skipped": average_metric(metric_values("sub_skills_skipped")), + "token_coverage_rate": sum(value is not None for value in metric_values("total_tokens")) / total_results, + "credit_coverage_rate": sum(value is not None for value in metric_values("ai_credits")) / total_results, + "usage_complete_rate": sum(value is True for value in metric_values("usage_complete")) / total_results, } ) diff --git a/src/bcbench/results/leaderboard.py b/src/bcbench/results/leaderboard.py index d7596eb67..8bf9a1c42 100644 --- a/src/bcbench/results/leaderboard.py +++ b/src/bcbench/results/leaderboard.py @@ -5,11 +5,11 @@ from pathlib import Path from typing import Any -from pydantic import BaseModel, field_validator +from pydantic import BaseModel, field_validator, model_validator from bcbench.logger import get_logger from bcbench.results.metrics import bootstrap_ci, pass_hat_k -from bcbench.results.summary import EvaluationResultSummary, ExecutionBasedEvaluationResultSummary +from bcbench.results.summary import EvaluationResultSummary, ExecutionBasedEvaluationResultSummary, restore_legacy_pr_review_agent_version from bcbench.types import EvaluationCategory, ExperimentConfiguration logger = get_logger(__name__) @@ -35,6 +35,15 @@ class LeaderboardAggregate(BaseModel, ABC): average_duration: float benchmark_version: str + copilot_cli_version: str | None = None + bcquality_repository: str | None = None + bcquality_commit: str | None = None + bcquality_version: str | None = None + + @model_validator(mode="before") + @classmethod + def restore_legacy_agent_version(cls, payload: object) -> object: + return restore_legacy_pr_review_agent_version(payload) @staticmethod def _validate_consistent_runs(runs: Sequence[EvaluationResultSummary]) -> None: @@ -61,6 +70,10 @@ def _base_fields(cls, runs: Sequence[EvaluationResultSummary]) -> dict[str, Any] "num_runs": len(runs), "average_duration": sum(durations) / len(durations) if durations else 0.0, "benchmark_version": first_run.benchmark_version, + "copilot_cli_version": first_run.copilot_cli_version, + "bcquality_repository": first_run.bcquality_repository, + "bcquality_commit": first_run.bcquality_commit, + "bcquality_version": first_run.bcquality_version, } @classmethod @@ -154,6 +167,22 @@ class CodeReviewLeaderboardAggregate(JudgeBasedLeaderboardAggregate): average_completion_tokens: float | None = None average_total_tokens: float | None = None average_ai_credits: float | None = None + average_cached_tokens: float | None = None + average_cache_creation_tokens: float | None = None + average_reasoning_tokens: float | None = None + average_api_calls: float | None = None + average_failed_api_calls: float | None = None + average_usage_api_calls: float | None = None + average_malformed_records: float | None = None + average_knowledge_files: float | None = None + average_knowledge_pruned: float | None = None + average_knowledge_used: float | None = None + average_knowledge_suppressed: float | None = None + average_sub_skills_executed: float | None = None + average_sub_skills_skipped: float | None = None + token_coverage_rate: float | None = None + credit_coverage_rate: float | None = None + usage_complete_rate: float | None = None @classmethod def from_runs(cls, runs: Sequence[EvaluationResultSummary]) -> "CodeReviewLeaderboardAggregate": @@ -201,6 +230,22 @@ def mean_metric(values: Sequence[float | None]) -> float | None: "average_completion_tokens": mean_metric([run.average_completion_tokens for run in cr_runs]), "average_total_tokens": mean_metric([run.average_total_tokens for run in cr_runs]), "average_ai_credits": mean_metric([run.average_ai_credits for run in cr_runs]), + "average_cached_tokens": mean_metric([run.average_cached_tokens for run in cr_runs]), + "average_cache_creation_tokens": mean_metric([run.average_cache_creation_tokens for run in cr_runs]), + "average_reasoning_tokens": mean_metric([run.average_reasoning_tokens for run in cr_runs]), + "average_api_calls": mean_metric([run.average_api_calls for run in cr_runs]), + "average_failed_api_calls": mean_metric([run.average_failed_api_calls for run in cr_runs]), + "average_usage_api_calls": mean_metric([run.average_usage_api_calls for run in cr_runs]), + "average_malformed_records": mean_metric([run.average_malformed_records for run in cr_runs]), + "average_knowledge_files": mean_metric([run.average_knowledge_files for run in cr_runs]), + "average_knowledge_pruned": mean_metric([run.average_knowledge_pruned for run in cr_runs]), + "average_knowledge_used": mean_metric([run.average_knowledge_used for run in cr_runs]), + "average_knowledge_suppressed": mean_metric([run.average_knowledge_suppressed for run in cr_runs]), + "average_sub_skills_executed": mean_metric([run.average_sub_skills_executed for run in cr_runs]), + "average_sub_skills_skipped": mean_metric([run.average_sub_skills_skipped for run in cr_runs]), + "token_coverage_rate": mean_metric([run.token_coverage_rate for run in cr_runs]), + "credit_coverage_rate": mean_metric([run.credit_coverage_rate for run in cr_runs]), + "usage_complete_rate": mean_metric([run.usage_complete_rate for run in cr_runs]), } ) diff --git a/src/bcbench/results/summary.py b/src/bcbench/results/summary.py index fcaee4662..d75c43250 100644 --- a/src/bcbench/results/summary.py +++ b/src/bcbench/results/summary.py @@ -6,9 +6,9 @@ from datetime import UTC, date, datetime from importlib.metadata import PackageNotFoundError, version from pathlib import Path -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, cast -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, model_validator from bcbench.logger import get_logger from bcbench.results.base import BaseEvaluationResult @@ -20,6 +20,18 @@ logger = get_logger(__name__) +def restore_legacy_pr_review_agent_version(payload: object) -> object: + if not isinstance(payload, dict): + return payload + data = cast(dict[str, Any], payload) + if data.get("agent_version") or data.get("agent_name") != "BC PR Review": + return payload + legacy_version = data.get("bc_alagents_commit") + if not isinstance(legacy_version, str) or not legacy_version: + return payload + return {**data, "agent_version": legacy_version} + + def get_benchmark_version() -> str: pyproject_path = Path(__file__).parent.parent.parent.parent / "pyproject.toml" if not pyproject_path.exists(): @@ -59,6 +71,15 @@ class EvaluationResultSummary(BaseModel, ABC): experiment: ExperimentConfiguration | None = None benchmark_version: str + copilot_cli_version: str | None = None + bcquality_repository: str | None = None + bcquality_commit: str | None = None + bcquality_version: str | None = None + + @model_validator(mode="before") + @classmethod + def restore_legacy_agent_version(cls, payload: object) -> object: + return restore_legacy_pr_review_agent_version(payload) @abstractmethod def render_github_metrics_markdown(self) -> str: @@ -86,6 +107,19 @@ def _base_fields(cls, results: Sequence[BaseEvaluationResult], run_id: str) -> d first_result = results[0] experiment = first_result.experiment if first_result.experiment and not first_result.experiment.is_empty() else None + def consistent_metric_value(name: str) -> str | None: + values: set[str] = set() + for result in results: + value = getattr(result.metrics, name, None) if result.metrics else None + if value is not None: + if not isinstance(value, str): + raise TypeError(f"Expected {name} to be a string, got {type(value).__name__}") + values.add(value) + if len(values) > 1: + logger.warning(f"Results contain inconsistent {name} values; omitting provenance: {values}") + return None + return next(iter(values), None) + return { "total": len(results), "date": datetime.now(UTC).date(), @@ -102,6 +136,10 @@ def _base_fields(cls, results: Sequence[BaseEvaluationResult], run_id: str) -> d "github_run_id": run_id, "experiment": experiment, "benchmark_version": get_benchmark_version(), + "copilot_cli_version": consistent_metric_value("copilot_cli_version"), + "bcquality_repository": consistent_metric_value("bcquality_repository"), + "bcquality_commit": consistent_metric_value("bcquality_commit"), + "bcquality_version": consistent_metric_value("bcquality_version"), } @classmethod diff --git a/src/bcbench/types.py b/src/bcbench/types.py index 7e810fd4a..bcfea2d8a 100644 --- a/src/bcbench/types.py +++ b/src/bcbench/types.py @@ -102,7 +102,16 @@ class PRReviewMetrics(AgentMetrics): usage_api_calls: int | None = Field(default=None, ge=0) usage_complete: bool | None = None malformed_records: int | None = Field(default=None, ge=0) + knowledge_files: int | None = Field(default=None, ge=0) + knowledge_pruned: int | None = Field(default=None, ge=0) + knowledge_used: int | None = Field(default=None, ge=0) + knowledge_suppressed: int | None = Field(default=None, ge=0) + sub_skills_executed: int | None = Field(default=None, ge=0) + sub_skills_skipped: int | None = Field(default=None, ge=0) copilot_cli_version: str | None = None + bcquality_repository: RepoSlug | None = None + bcquality_commit: CommitSha | None = None + bcquality_version: str | None = None type AnyAgentMetrics = Annotated[AgentMetrics | PRReviewMetrics, Field(discriminator="kind")] diff --git a/tests/test_agent_version_results.py b/tests/test_agent_version_results.py index 6c8b2ac7d..f438f9b48 100644 --- a/tests/test_agent_version_results.py +++ b/tests/test_agent_version_results.py @@ -54,6 +54,25 @@ def test_summary_uses_artifact_version_not_environment(monkeypatch: pytest.Monke assert EvaluationResultSummary.from_results([result], "run").agent_version == "1.2.3" +def test_legacy_pr_review_identity_loads_as_agent_version() -> None: + summary = EvaluationResultSummary.from_results( + [create_codereview_result(agent_name=AgentHarness.PR_REVIEW)], + "run", + ) + payload = summary.to_dict() + payload["agent_version"] = None + payload["bc_alagents_commit"] = "a" * 40 + + restored = EvaluationResultSummary.from_json(payload) + aggregate_payload = LeaderboardAggregate.from_runs([summary]).model_dump(mode="json") + aggregate_payload["agent_version"] = None + aggregate_payload["bc_alagents_commit"] = "a" * 40 + restored_aggregate = LeaderboardAggregate.from_json(aggregate_payload) + + assert restored.agent_version == "a" * 40 + assert restored_aggregate.agent_version == "a" * 40 + + def test_repeated_versions_aggregate_and_different_versions_stay_separate(tmp_path: Path) -> None: for index, version in enumerate(["a" * 40, "b" * 40, "a" * 40]): result = create_codereview_result(agent_name=AgentHarness.PR_REVIEW).model_copy(update={"agent_version": version}) diff --git a/tests/test_evaluation_summary.py b/tests/test_evaluation_summary.py index fba2cc97b..8d97c3b5b 100644 --- a/tests/test_evaluation_summary.py +++ b/tests/test_evaluation_summary.py @@ -131,6 +131,33 @@ def test_from_results_creates_correct_summary(self, sample_results): assert summary.github_run_id == "test_run_123" assert summary.date == datetime.now(UTC).date() + def test_from_results_records_runtime_provenance_from_metrics(self, sample_results): + metrics = sample_results[0].metrics.model_copy( + update={ + "copilot_cli_version": "1.0.82", + "bcquality_repository": "microsoft/BCQuality", + "bcquality_commit": "2" * 40, + "bcquality_version": "1.6", + } + ) + results = [result.model_copy(update={"metrics": metrics}) for result in sample_results] + + summary = ExecutionBasedEvaluationResultSummary.from_results(results, run_id="test_run_123") + + assert summary.copilot_cli_version == "1.0.82" + assert summary.bcquality_repository == "microsoft/BCQuality" + assert summary.bcquality_commit == "2" * 40 + assert summary.bcquality_version == "1.6" + + def test_from_results_omits_inconsistent_runtime_provenance(self, sample_results, caplog): + first = sample_results[0].model_copy(update={"metrics": sample_results[0].metrics.model_copy(update={"bcquality_commit": "1" * 40})}) + second = sample_results[1].model_copy(update={"metrics": sample_results[1].metrics.model_copy(update={"bcquality_commit": "2" * 40})}) + + summary = ExecutionBasedEvaluationResultSummary.from_results([first, second], run_id="test_run_123") + + assert summary.bcquality_commit is None + assert "inconsistent bcquality_commit" in caplog.text + def test_from_results_calculates_averages_correctly(self, sample_results): summary = ExecutionBasedEvaluationResultSummary.from_results(sample_results, run_id="test_run_123") @@ -830,6 +857,38 @@ def test_aggregate_rejects_runs_from_different_combinations(self): with pytest.raises(ValueError, match="different combinations"): LeaderboardAggregate.from_runs([run1, run2]) + def test_transitive_provenance_does_not_replace_agent_version_grouping(self): + run1 = ExecutionBasedEvaluationResultSummary.from_results( + [create_bugfix_result(instance_id="test__1", resolved=True)], + run_id="run_1", + ).model_copy(update={"bcquality_commit": "1" * 40}) + run2 = run1.model_copy(update={"github_run_id": "run_2", "bcquality_commit": "2" * 40}) + + assert run1.combination_key() == run2.combination_key() + + def test_aggregate_includes_advanced_provenance(self): + from bcbench.results.leaderboard import LeaderboardAggregate + + run = ExecutionBasedEvaluationResultSummary.from_results( + [create_bugfix_result(instance_id="test__1", resolved=True)], + run_id="run_1", + ).model_copy( + update={ + "agent_version": "1" * 40, + "copilot_cli_version": "1.0.82", + "bcquality_repository": "microsoft/BCQuality", + "bcquality_commit": "2" * 40, + "bcquality_version": "1.6", + } + ) + + aggregate = LeaderboardAggregate.from_runs([run]) + + assert aggregate.agent_version == "1" * 40 + assert aggregate.copilot_cli_version == "1.0.82" + assert aggregate.bcquality_commit == "2" * 40 + assert aggregate.bcquality_version == "1.6" + def test_aggregate_rejects_runs_with_different_judge_models(self): from bcbench.results.leaderboard import LeaderboardAggregate from bcbench.results.summary import EvaluationResultSummary diff --git a/tests/test_pr_review_agent.py b/tests/test_pr_review_agent.py index 664bdd007..0eccb58ba 100644 --- a/tests/test_pr_review_agent.py +++ b/tests/test_pr_review_agent.py @@ -159,6 +159,10 @@ def test_engine_environment_uses_target_repository_and_absolute_paths(tmp_path: patch("bcbench.agent.pr_review.agent._commit_patch_as_head"), patch("bcbench.agent.pr_review.agent._init_trusted_workspace", return_value=tmp_path / "trusted"), patch("bcbench.agent.pr_review.agent._prepare_bcquality_root", return_value=bcquality_root) as prepare_bcquality, + patch( + "bcbench.agent.pr_review.metrics._load_bcquality_identity", + return_value=("microsoft/BCQuality", "a" * 40, "1.6"), + ), patch("bcbench.agent.pr_review.agent._write_review_json", return_value=0), 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, @@ -180,6 +184,9 @@ def test_engine_environment_uses_target_repository_and_absolute_paths(tmp_path: assert metrics.ai_credits == 0.25 assert metrics.api_calls == 2 assert metrics.copilot_cli_version == "1.0.81-0" + assert metrics.bcquality_repository == "microsoft/BCQuality" + assert metrics.bcquality_commit == "a" * 40 + assert metrics.bcquality_version == "1.6" assert config.is_empty() resolve_engine.assert_called_once_with(tmp_path / "engine") prepare_bcquality.assert_called_once_with(tmp_path / "engine", "pwsh", (tmp_path / "output" / "bcquality").resolve()) diff --git a/tests/test_pr_review_metrics.py b/tests/test_pr_review_metrics.py index cfa6c75a4..8b80f8939 100644 --- a/tests/test_pr_review_metrics.py +++ b/tests/test_pr_review_metrics.py @@ -1,10 +1,11 @@ import json +import subprocess from pathlib import Path from unittest.mock import patch import pytest -from bcbench.agent.pr_review.metrics import RUN_METRICS_FILE_NAME, build_pr_review_metrics +from bcbench.agent.pr_review.metrics import FILTER_REPORT_FILE_NAME, RUN_METRICS_FILE_NAME, _count_available_knowledge, build_pr_review_metrics from bcbench.dataset.codereview import CodeReviewEntry from bcbench.exceptions import AgentError from bcbench.results.bceval_export import write_bceval_results @@ -38,12 +39,13 @@ def _run_metrics(**overrides: object) -> dict[str, object]: def _write_run_metrics(root: Path, **overrides: object) -> None: (root / RUN_METRICS_FILE_NAME).write_text(json.dumps(_run_metrics(**overrides)), encoding="utf-8") + (root / FILTER_REPORT_FILE_NAME).write_text(json.dumps({"removed": []}), encoding="utf-8") def test_build_metrics_promotes_public_performance_metrics(tmp_path: Path) -> None: _write_run_metrics(tmp_path) - metrics = build_pr_review_metrics(tmp_path, execution_time=12.5) + metrics = build_pr_review_metrics(tmp_path, tmp_path, execution_time=12.5) assert isinstance(metrics, PRReviewMetrics) assert metrics.kind == "pr-review" @@ -60,6 +62,12 @@ def test_build_metrics_promotes_public_performance_metrics(tmp_path: Path) -> No assert metrics.usage_api_calls == 2 assert metrics.usage_complete is True assert metrics.malformed_records == 0 + assert metrics.knowledge_files == 0 + assert metrics.knowledge_pruned == 0 + assert metrics.knowledge_used == 0 + assert metrics.knowledge_suppressed == 0 + assert metrics.sub_skills_executed is None + assert metrics.sub_skills_skipped == 0 assert metrics.copilot_cli_version == "1.0.81-0" @@ -76,7 +84,7 @@ def test_legal_null_optional_fields_and_multiple_models_are_accepted(tmp_path: P models=["gpt-5.4-mini", "gpt-5.6-sol"], ) - metrics = build_pr_review_metrics(tmp_path, execution_time=2.0) + metrics = build_pr_review_metrics(tmp_path, tmp_path, execution_time=2.0) assert metrics.ai_credits is None assert metrics.total_tokens == 178 @@ -105,7 +113,7 @@ def test_valid_engine_metrics_without_billing_preserve_unknown_credits_through_e malformed_records=0, ) - metrics = build_pr_review_metrics(tmp_path, execution_time=2.5) + metrics = build_pr_review_metrics(tmp_path, tmp_path, execution_time=2.5) assert metrics.ai_credits is None assert metrics.total_tokens == (178 if has_token_usage else None) result = create_codereview_result(agent_name=AgentHarness.PR_REVIEW, metrics=metrics) @@ -139,12 +147,17 @@ def test_malformed_records_suppress_all_usage_metrics(tmp_path: Path) -> None: malformed_records=3, ) - metrics = build_pr_review_metrics(tmp_path, execution_time=2.0) + metrics = build_pr_review_metrics(tmp_path, tmp_path, execution_time=2.0) assert metrics.prompt_tokens is None assert metrics.completion_tokens is None assert metrics.total_tokens is None assert metrics.ai_credits is None + assert metrics.api_calls == 2 + assert metrics.failed_api_calls == 1 + assert metrics.usage_api_calls == 1 + assert metrics.usage_complete is False + assert metrics.malformed_records == 3 def test_incomplete_usage_suppresses_tokens_but_preserves_exact_credits(tmp_path: Path) -> None: @@ -159,7 +172,7 @@ def test_incomplete_usage_suppresses_tokens_but_preserves_exact_credits(tmp_path malformed_records=0, ) - metrics = build_pr_review_metrics(tmp_path, execution_time=2.0) + metrics = build_pr_review_metrics(tmp_path, tmp_path, execution_time=2.0) assert metrics.prompt_tokens is None assert metrics.completion_tokens is None @@ -169,14 +182,32 @@ def test_incomplete_usage_suppresses_tokens_but_preserves_exact_credits(tmp_path def test_missing_run_metrics_raises(tmp_path: Path) -> None: with pytest.raises(AgentError, match="run metrics artifact not found"): - build_pr_review_metrics(tmp_path, execution_time=1.0) + build_pr_review_metrics(tmp_path, tmp_path, execution_time=1.0) + + +def test_missing_filter_report_preserves_other_diagnostics(tmp_path: Path) -> None: + _write_run_metrics(tmp_path) + (tmp_path / FILTER_REPORT_FILE_NAME).unlink() + + metrics = build_pr_review_metrics(tmp_path, tmp_path, execution_time=1.0) + + assert metrics.prompt_tokens == 150 + assert metrics.knowledge_pruned is None + + +def test_invalid_filter_report_still_raises(tmp_path: Path) -> None: + _write_run_metrics(tmp_path) + (tmp_path / FILTER_REPORT_FILE_NAME).write_text("not json", encoding="utf-8") + + with pytest.raises(AgentError, match="Could not read BCQuality filter report"): + build_pr_review_metrics(tmp_path, tmp_path, execution_time=1.0) def test_invalid_run_metrics_json_raises(tmp_path: Path) -> None: (tmp_path / RUN_METRICS_FILE_NAME).write_text("not json", encoding="utf-8") with pytest.raises(AgentError, match="Could not read engine run metrics artifact"): - build_pr_review_metrics(tmp_path, execution_time=1.0) + build_pr_review_metrics(tmp_path, tmp_path, execution_time=1.0) @pytest.mark.parametrize( @@ -197,7 +228,7 @@ def test_invalid_run_metrics_contract_raises(tmp_path: Path, overrides: dict[str _write_run_metrics(tmp_path, **overrides) with pytest.raises(AgentError, match="does not satisfy schema version 1"): - build_pr_review_metrics(tmp_path, execution_time=1.0) + build_pr_review_metrics(tmp_path, tmp_path, execution_time=1.0) def test_missing_run_metrics_key_raises(tmp_path: Path) -> None: @@ -206,7 +237,7 @@ def test_missing_run_metrics_key_raises(tmp_path: Path) -> None: (tmp_path / RUN_METRICS_FILE_NAME).write_text(json.dumps(payload), encoding="utf-8") with pytest.raises(AgentError, match="does not satisfy schema version 1"): - build_pr_review_metrics(tmp_path, execution_time=1.0) + build_pr_review_metrics(tmp_path, tmp_path, execution_time=1.0) def test_not_applicable_zero_shape_fails_evaluation(tmp_path: Path) -> None: @@ -232,7 +263,7 @@ def test_not_applicable_zero_shape_fails_evaluation(tmp_path: Path) -> None: ) with pytest.raises(AgentError, match="must contain AL changes"): - build_pr_review_metrics(tmp_path, execution_time=0.25) + build_pr_review_metrics(tmp_path, tmp_path, execution_time=0.25) @pytest.mark.parametrize( @@ -272,4 +303,99 @@ def test_not_applicable_rejects_noncanonical_shape(tmp_path: Path, field: str, v _write_run_metrics(tmp_path, **not_applicable) with pytest.raises(AgentError, match="not-applicable metrics have invalid fields"): - build_pr_review_metrics(tmp_path, execution_time=1.0) + build_pr_review_metrics(tmp_path, tmp_path, execution_time=1.0) + + +def test_engine_diagnostics_count_knowledge_and_sub_skills(tmp_path: Path) -> None: + _count_available_knowledge.cache_clear() + _write_run_metrics(tmp_path) + (tmp_path / "microsoft" / "knowledge").mkdir(parents=True) + (tmp_path / "community" / "knowledge").mkdir(parents=True) + (tmp_path / "custom" / "skills").mkdir(parents=True) + (tmp_path / "microsoft" / "knowledge" / "one.md").write_text("one", encoding="utf-8") + (tmp_path / "community" / "knowledge" / "two.md").write_text("two", encoding="utf-8") + (tmp_path / "custom" / "skills" / "not-knowledge.md").write_text("skill", encoding="utf-8") + (tmp_path / FILTER_REPORT_FILE_NAME).write_text( + json.dumps({"removed": [{"kind": "knowledge"}, {"kind": "knowledge"}, {"kind": "skill"}]}), + encoding="utf-8", + ) + (tmp_path / "al-code-review-findings.json").write_text( + json.dumps( + { + "findings": [ + { + "references": [ + {"path": "microsoft/knowledge/one.md"}, + {"path": "C:/checkout/bcquality/microsoft/knowledge/one.md"}, + {"path": "microsoft/skills/not-knowledge.md"}, + {"path": "microsoft/knowledge/../skills/not-knowledge.md"}, + {"path": ""}, + ] + } + ], + "subResults": [ + { + "references": [ + {"path": "community/knowledge/two.md"}, + {"path": "MICROSOFT\\KNOWLEDGE\\ONE.MD"}, + ] + } + ], + "skippedSubSkills": [{"name": "one"}, {"name": "two"}], + "suppressed": [{"path": "custom/knowledge/three.md"}], + } + ), + encoding="utf-8", + ) + + metrics = build_pr_review_metrics(tmp_path, tmp_path, execution_time=1.0) + + assert metrics.knowledge_files == 2 + assert metrics.knowledge_pruned == 2 + assert metrics.knowledge_used == 2 + assert metrics.knowledge_suppressed == 1 + assert metrics.sub_skills_executed == 1 + assert metrics.sub_skills_skipped == 2 + + +def test_available_knowledge_count_is_cached_per_checkout(tmp_path: Path) -> None: + _count_available_knowledge.cache_clear() + knowledge_root = tmp_path / "microsoft" / "knowledge" + knowledge_root.mkdir(parents=True) + (knowledge_root / "one.md").write_text("one", encoding="utf-8") + + assert _count_available_knowledge(tmp_path.resolve()) == 1 + (knowledge_root / "two.md").write_text("two", encoding="utf-8") + assert _count_available_knowledge(tmp_path.resolve()) == 1 + + +def test_runtime_provenance_is_derived_from_metrics_and_checkout(tmp_path: Path) -> None: + _write_run_metrics(tmp_path, cli_version="1.0.82") + engine_root = tmp_path / "engine" + config = engine_root / "agents" / "ALReviewAgent" / "bcquality.config.yaml" + config.parent.mkdir(parents=True) + config.write_text( + "bcquality:\n repo: https://github.com/microsoft/BCQuality.git\n ref: " + "a" * 40 + '\n version: "1.6"\n', + encoding="utf-8", + ) + + completed = subprocess.CompletedProcess(args=["git"], returncode=0, stdout="b" * 40 + "\n", stderr="") + with patch("bcbench.agent.pr_review.metrics.subprocess.run", return_value=completed): + metrics = build_pr_review_metrics(tmp_path, tmp_path, execution_time=1.0, engine_root=engine_root) + + assert metrics.copilot_cli_version == "1.0.82" + assert metrics.bcquality_repository == "microsoft/BCQuality" + assert metrics.bcquality_commit == "b" * 40 + assert metrics.bcquality_version == "1.6" + + +def test_unavailable_runtime_provenance_does_not_fail_metrics(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: + _write_run_metrics(tmp_path, cli_version="1.0.82") + + metrics = build_pr_review_metrics(tmp_path, tmp_path, execution_time=1.0, engine_root=tmp_path / "missing-engine") + + assert metrics.copilot_cli_version == "1.0.82" + assert metrics.bcquality_repository is None + assert metrics.bcquality_commit is None + assert metrics.bcquality_version is None + assert "BCQuality provenance unavailable" in caplog.text diff --git a/tests/test_pr_review_metrics_reporting.py b/tests/test_pr_review_metrics_reporting.py index 738f72e95..9afe02816 100644 --- a/tests/test_pr_review_metrics_reporting.py +++ b/tests/test_pr_review_metrics_reporting.py @@ -2,25 +2,43 @@ from bcbench.results.codereview import CodeReviewResultSummary from bcbench.results.leaderboard import CodeReviewLeaderboardAggregate -from bcbench.types import AgentMetrics +from bcbench.types import AgentHarness, AgentMetrics, PRReviewMetrics from tests.conftest import create_codereview_result -def _metrics(*, duration: float, scale: int) -> AgentMetrics: - return AgentMetrics( +def _metrics(*, duration: float, scale: int) -> PRReviewMetrics: + return PRReviewMetrics( execution_time=duration, prompt_tokens=900 * scale, completion_tokens=100 * scale, total_tokens=1000 * scale, ai_credits=0.5 * scale, + cached_tokens=100 * scale, + cache_creation_tokens=50 * scale, + reasoning_tokens=25 * scale, + api_calls=10 * scale, + failed_api_calls=scale - 1, + usage_api_calls=9 * scale, + usage_complete=True, + malformed_records=0, + knowledge_files=40 * scale, + knowledge_pruned=20 * scale, + knowledge_used=5 * scale, + knowledge_suppressed=2 * scale, + sub_skills_executed=3 * scale, + sub_skills_skipped=scale, + copilot_cli_version="1.0.82", + bcquality_repository="microsoft/BCQuality", + bcquality_commit="a" * 40, + bcquality_version="1.6", ) def test_summary_aggregates_public_pr_review_metrics() -> None: summary = CodeReviewResultSummary.from_results( [ - create_codereview_result(instance_id="proj__review-1", metrics=_metrics(duration=4.0, scale=1)), - create_codereview_result(instance_id="proj__review-2", metrics=_metrics(duration=6.0, scale=2)), + create_codereview_result(instance_id="proj__review-1", agent_name=AgentHarness.PR_REVIEW, metrics=_metrics(duration=4.0, scale=1)), + create_codereview_result(instance_id="proj__review-2", agent_name=AgentHarness.PR_REVIEW, metrics=_metrics(duration=6.0, scale=2)), ], run_id="run", ) @@ -30,7 +48,27 @@ def test_summary_aggregates_public_pr_review_metrics() -> None: assert summary.average_completion_tokens == 150 assert summary.average_total_tokens == 1500 assert summary.average_ai_credits == 0.75 + assert summary.average_cached_tokens == 150 + assert summary.average_cache_creation_tokens == 75 + assert summary.average_reasoning_tokens == 37.5 + assert summary.average_api_calls == 15 + assert summary.average_failed_api_calls == 0.5 + assert summary.average_usage_api_calls == 13.5 + assert summary.average_malformed_records == 0 + assert summary.average_knowledge_files == 60 + assert summary.average_knowledge_pruned == 30 + assert summary.average_knowledge_used == 7.5 + assert summary.average_knowledge_suppressed == 3 + assert summary.average_sub_skills_executed == 4.5 + assert summary.average_sub_skills_skipped == 1.5 + assert summary.token_coverage_rate == 1.0 + assert summary.credit_coverage_rate == 1.0 + assert summary.usage_complete_rate == 1.0 assert summary.valid_review_output_rate == 1.0 + assert summary.copilot_cli_version == "1.0.82" + assert summary.bcquality_repository == "microsoft/BCQuality" + assert summary.bcquality_commit == "a" * 40 + assert summary.bcquality_version == "1.6" def test_summary_preserves_unavailable_usage_as_none() -> None: @@ -45,15 +83,40 @@ def test_summary_preserves_unavailable_usage_as_none() -> None: assert serialized["average_completion_tokens"] is None assert serialized["average_total_tokens"] is None assert serialized["average_ai_credits"] is None + assert serialized["token_coverage_rate"] == 0.0 + assert serialized["credit_coverage_rate"] == 0.0 + assert serialized["usage_complete_rate"] == 0.0 + + +def test_aggregate_preserves_missing_legacy_coverage_as_none() -> None: + summary = CodeReviewResultSummary.model_validate( + { + "run_id": "legacy", + "agent_name": "GitHub Copilot", + "model": "legacy-model", + "category": "code-review", + "benchmark_version": "0.7.0", + "date": "2026-01-01T00:00:00Z", + "total": 1, + "average_duration": 1.0, + "judge_model": "legacy-judge", + } + ) + + aggregate = CodeReviewLeaderboardAggregate.from_runs([summary]) + + assert aggregate.token_coverage_rate is None + assert aggregate.credit_coverage_rate is None + assert aggregate.usage_complete_rate is None def test_leaderboard_propagates_public_pr_review_metrics() -> None: first = CodeReviewResultSummary.from_results( - [create_codereview_result(instance_id="proj__review-1", metrics=_metrics(duration=4.0, scale=1))], + [create_codereview_result(instance_id="proj__review-1", agent_name=AgentHarness.PR_REVIEW, metrics=_metrics(duration=4.0, scale=1))], run_id="one", ) second = CodeReviewResultSummary.from_results( - [create_codereview_result(instance_id="proj__review-1", metrics=_metrics(duration=6.0, scale=2))], + [create_codereview_result(instance_id="proj__review-1", agent_name=AgentHarness.PR_REVIEW, metrics=_metrics(duration=6.0, scale=2))], run_id="two", ) @@ -64,12 +127,26 @@ def test_leaderboard_propagates_public_pr_review_metrics() -> None: assert aggregate.average_completion_tokens == 150 assert aggregate.average_total_tokens == 1500 assert aggregate.average_ai_credits == 0.75 + assert aggregate.average_api_calls == 15 + assert aggregate.average_knowledge_files == 60 + assert aggregate.average_knowledge_pruned == 30 + assert aggregate.average_knowledge_used == 7.5 + assert aggregate.average_knowledge_suppressed == 3 + assert aggregate.average_sub_skills_executed == 4.5 + assert aggregate.average_sub_skills_skipped == 1.5 + assert aggregate.token_coverage_rate == 1.0 + assert aggregate.credit_coverage_rate == 1.0 + assert aggregate.usage_complete_rate == 1.0 assert aggregate.valid_review_output_rate == 1.0 + assert aggregate.copilot_cli_version == "1.0.82" + assert aggregate.bcquality_repository == "microsoft/BCQuality" + assert aggregate.bcquality_commit == "a" * 40 + assert aggregate.bcquality_version == "1.6" def test_github_summary_renders_only_public_performance_metrics() -> None: summary = CodeReviewResultSummary.from_results( - [create_codereview_result(instance_id="proj__review-1", metrics=_metrics(duration=4.0, scale=1))], + [create_codereview_result(instance_id="proj__review-1", agent_name=AgentHarness.PR_REVIEW, metrics=_metrics(duration=4.0, scale=1))], run_id="run", ) @@ -84,8 +161,8 @@ def test_github_summary_renders_only_public_performance_metrics() -> None: assert diagnostic not in markdown -def test_result_json_excludes_raw_only_diagnostics(tmp_path) -> None: - result = create_codereview_result(metrics=_metrics(duration=4.0, scale=1)) +def test_result_json_persists_pr_review_diagnostics(tmp_path) -> None: + result = create_codereview_result(agent_name=AgentHarness.PR_REVIEW, metrics=_metrics(duration=4.0, scale=1)) result.save(tmp_path, "results.jsonl") saved_metrics = json.loads((tmp_path / "results.jsonl").read_text(encoding="utf-8"))["metrics"] @@ -94,22 +171,25 @@ def test_result_json_excludes_raw_only_diagnostics(tmp_path) -> None: assert saved_metrics["completion_tokens"] == 100 assert saved_metrics["total_tokens"] == 1000 assert saved_metrics["ai_credits"] == 0.5 - for diagnostic in ( - "cached_tokens", - "cache_creation_tokens", - "reasoning_tokens", - "failed_api_calls", - "usage_api_calls", - "premium_requests", - "usage_complete", - "malformed_records", - ): - assert diagnostic not in saved_metrics - - -def test_summary_and_leaderboard_schemas_exclude_raw_only_diagnostics() -> None: + assert saved_metrics["cached_tokens"] == 100 + assert saved_metrics["cache_creation_tokens"] == 50 + assert saved_metrics["reasoning_tokens"] == 25 + assert saved_metrics["api_calls"] == 10 + assert saved_metrics["failed_api_calls"] == 0 + assert saved_metrics["usage_api_calls"] == 9 + assert saved_metrics["usage_complete"] is True + assert saved_metrics["malformed_records"] == 0 + assert saved_metrics["knowledge_files"] == 40 + assert saved_metrics["knowledge_pruned"] == 20 + assert saved_metrics["knowledge_used"] == 5 + assert saved_metrics["knowledge_suppressed"] == 2 + assert saved_metrics["sub_skills_executed"] == 3 + assert saved_metrics["sub_skills_skipped"] == 1 + + +def test_summary_and_leaderboard_schemas_include_pr_review_diagnostics() -> None: summary = CodeReviewResultSummary.from_results( - [create_codereview_result(metrics=_metrics(duration=4.0, scale=1))], + [create_codereview_result(agent_name=AgentHarness.PR_REVIEW, metrics=_metrics(duration=4.0, scale=1))], run_id="run", ) aggregate = CodeReviewLeaderboardAggregate.from_runs([summary]) @@ -122,10 +202,15 @@ def test_summary_and_leaderboard_schemas_exclude_raw_only_diagnostics() -> None: "average_api_calls", "average_failed_api_calls", "average_usage_api_calls", - "average_premium_requests", - "structured_usage_complete_rate", + "usage_complete_rate", "average_malformed_records", "average_knowledge_files", "average_knowledge_pruned", + "average_knowledge_used", + "average_knowledge_suppressed", + "average_sub_skills_executed", + "average_sub_skills_skipped", + "token_coverage_rate", + "credit_coverage_rate", ): - assert diagnostic not in payload + assert diagnostic in payload diff --git a/tests/test_review_workflows.py b/tests/test_review_workflows.py index f1f072283..d9cf0744a 100644 --- a/tests/test_review_workflows.py +++ b/tests/test_review_workflows.py @@ -9,8 +9,9 @@ WORKFLOWS = Path(__file__).parents[1] / ".github" / "workflows" ACTIONS = Path(__file__).parents[1] / ".github" / "actions" +DOCS = Path(__file__).parents[1] / "docs" AGENT_CONFIG = Path(__file__).parents[1] / "src" / "bcbench" / "agent" / "shared" / "config.yaml" -DEFAULT_ENGINE_SHA = "ecf8e31759d6ddd6d78e3a0b7836b40134368009" +DEFAULT_ENGINE_SHA = "fdc02d7020632795810057500d62cff2a61513d7" PWSH = shutil.which("pwsh") @@ -130,7 +131,7 @@ def test_agent_harness_action_pins_published_copilot_version() -> None: assert "@github/copilot@1.0.82" in action -def test_agent_harness_action_pins_and_exports_bc_alagents() -> None: +def test_agent_harness_action_pins_engine_without_exporting_transitive_identity() -> None: action = (ACTIONS / "install-agent-harnesses" / "action.yml").read_text(encoding="utf-8") config = yaml.safe_load(action) validation = next(step for step in config["runs"]["steps"] if step.get("id") == "engine-sha") @@ -149,6 +150,36 @@ def test_agent_harness_action_pins_and_exports_bc_alagents() -> None: assert set(config["outputs"]) == {"bc-alagents-path"} +def test_shared_summary_workflow_has_no_pr_review_provenance_inputs() -> None: + workflow = _workflow("pr-review-evaluation.yml") + summary_workflow = _workflow("summarize-results.yml") + + for field in ( + "benchmark-commit", + "copilot-cli-version", + "bcquality-repository", + "bcquality-commit", + "bcquality-version", + ): + assert field not in workflow + assert field not in summary_workflow + assert "bc-alagents-commit" not in workflow + assert "bc-alagents-commit" not in summary_workflow + + +def test_transitive_provenance_is_only_displayed_on_advanced_dashboard() -> None: + dashboard = (DOCS / "code-review.md").read_text(encoding="utf-8") + advanced = (DOCS / "code-review-details.md").read_text(encoding="utf-8") + + assert "Evaluation Stack" not in dashboard + assert "bcquality_commit" not in dashboard + assert "copilot_cli_version" not in dashboard + assert "Version" in dashboard + assert "agent_version" in advanced + assert "bcquality_commit" in advanced + assert "copilot_cli_version" in advanced + + @pytest.mark.skipif(PWSH is None, reason="PowerShell is required to test the composite action script") @pytest.mark.parametrize( ("engine_sha", "valid"),