Skip to content

Validate deterministic PR review model routing - #881

Open
dayland wants to merge 33 commits into
mainfrom
experiment/live-pr-review-leaf-models
Open

dayland wants to merge 33 commits into
mainfrom
experiment/live-pr-review-leaf-models

Conversation

@dayland

@dayland dayland commented Sep 14, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • pin the default deterministic BC PR Review engine to 03239afe611a3eff490002eba0e4098b48099ba3 (Fix checkout-free post-phase BCQuality resolution BC-ALAgents#73), while retaining an explicit full-SHA override only for unpublished experiment runs
  • update the shared generic GitHub Copilot CLI harness to 1.0.88 and Claude Code to 2.1.281
  • keep deterministic PR Review on its explicitly paired Copilot CLI 1.0.83: the pinned engine validates its OpenTelemetry CLI-version field, which current generic CLI 1.0.88 does not emit
  • validate deterministic root/leaf model routing, serial or bounded-parallel scheduling, exact engine/BCQuality provenance, ordered plan/process coverage, and complete per-process telemetry before scoring
  • recognize the engine's automatically create branch to update leaderboard #70 partial recovery manifest state but reject it from benchmark scoring, so incomplete review coverage cannot contaminate quality or token comparisons
  • add GPT-6 Astra as a curated generic Copilot/contamination frontier baseline and Claude Opus 5.5 as a curated premium Claude baseline; retain existing models for historical comparability

Why

Prompt-driven child orchestration cannot provide trustworthy model-placement or cost evidence. BC-Bench must verify the production engine's mechanically bound root/leaf models and manifest before accepting a run. The latest BC-ALAgents revision also includes the checkout-free post-phase publishing correction, while preserving generation-phase BCQuality provenance validation.

The initial evaluation-identity smoke run exposed a real contract mismatch: CLI 1.0.88 produced no telemetry CLI version, so #73 correctly rejected it. This PR isolates the supported 1.0.83 engine/telemetry pair to PR Review rather than weakening deterministic validation or holding generic Copilot evaluations back from 1.0.88.

Validation

  • focused deterministic harness suite: 165 passed
  • full suite: 1,067 passed, 2 skipped, 1 deselected
  • action and workflow YAML parsing, Ruff lint/format, git diff --check, and pre-commit hooks passed
  • ty check completed with one pre-existing unused-ignore warning in untouched src/bcbench/redteam.py
  • the initial evaluation-identity run caught the 1.0.88 PR Review telemetry incompatibility before results were scored or published; a corrected default Sol-root/Luna-leaf smoke run is now dispatched from the paired-CLI head

Dependencies

Initial model scope

The PR Review baseline continues to exclude Claude and Gemini models while deterministic model routing is rebaselined. Root choices are GPT-5.4, GPT-5.6 Sol/Terra/Luna, GPT-5.3 Codex, and MAI Code 1.1 Flash. Leaf choices remain GPT-5.4, GPT-5.6 Luna, and MAI Code 1.1 Flash. GPT-5.6 Sol remains the default root and GPT-5.6 Luna the default leaf.

GPT-6 Astra is a generic Copilot and contamination frontier candidate, but is intentionally not offered to the PR Review engine until a telemetry-compatible engine/CLI pair exists. Claude Fable 5.1 is deliberately excluded because it has no clear coding-benchmark role beyond the existing Sonnet/Opus/Haiku coverage. The fixed code-review judge remains gpt-5.3-codex to avoid an unrelated scoring-methodology change.

Follow-up

After the test run passes, rerun the complete PR Review benchmark with entries, root model, leaf model, scheduling, revisions, and repetitions fixed. Treat token consumption as the primary performance measure, with quality metrics as guardrails. A BCQuality contract-bearing immutable release is still needed so external consumers need not pin raw commits.

dayland-ms and others added 18 commits September 2, 2026 15:01
Pin the latest BC-ALAgents and BCQuality main snapshot, update the installable Copilot CLI, add Gemini 3.7 Flash and Grok 4.6, and bump BC-Bench to 0.10.0.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 57308020-6803-4e62-b08e-57fa875bff89
Update the harness pin to the immutable BC-ALAgents main commit produced by microsoft/BC-ALAgents#60.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 57308020-6803-4e62-b08e-57fa875bff89
Limit the model refresh to Gemini 3.7 Flash and retain the existing provider set.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 57308020-6803-4e62-b08e-57fa875bff89
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 57308020-6803-4e62-b08e-57fa875bff89
Co-authored-by: Sun Haoran <haoransun@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 57308020-6803-4e62-b08e-57fa875bff89
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 57308020-6803-4e62-b08e-57fa875bff89
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
Preserve the pinned Copilot CLI, BC-ALAgents revision, serial scheduling, focused entries, concurrent repetitions, and observed incomplete usage telemetry.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
Pin the experimental workflow to the fail-closed BC-ALAgents engine so root-only and substituted-model runs cannot be mistaken for leaf-model evidence.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
Use the engine revision that rejects any model outside the configured root and leaf pair during leaf-model experiments.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
Retain the strict telemetry allowlist while removing the orchestration-changing prompt prohibition.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
Validate immutable engine and BCQuality revisions, exact model routing, scheduling, and complete process telemetry before accepting benchmark results.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
Pin only the review engine in BC-Bench and validate BCQuality as engine-produced provenance rather than a benchmark-controlled revision.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
@dayland

dayland commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the BCQuality ownership boundary:

  • BC-Bench now pins only BC-ALAgents (bab2863f14a1f5c474179c136e2c9775cc836fa6).
  • BC-Bench no longer resolves or passes BCQUALITY_SHA, and no longer compares the manifest to a benchmark-owned expected BCQuality commit.
  • The pinned BC-ALAgents revision resolves BCQuality from its own configuration, derives the commit from that checkout, and records it in _run-manifest.json as provenance.
  • BC-Bench still persists that engine-produced revision for reproducibility, but does not select or override it.

Validation: 84 focused tests and the full BC-Bench suite (1,074 passed) completed successfully. Live run 34850610549 also proved the new ownership path: without BC-Bench supplying a SHA, BC-ALAgents recorded its own commit bab2863f... and BCQuality 25accf... plus the source snapshot. The run later failed closed at leaf 14 because Luna omitted the required suppressed property; that is unrelated to dependency resolution and is correctly excluded from evidence.

Limit baseline workflow choices to OpenAI and Microsoft model families and use GPT-5.4 as the initial root default.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
@dayland

dayland commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator Author

Updated the initial experiment scope: Claude and Gemini are no longer selectable in the #881 PR Review workflow. The root and leaf defaults are now GPT-5.4, with choices limited to OpenAI/Microsoft model families for the first deterministic baseline. Workflow tests assert the exact allowed lists so excluded providers cannot be reintroduced accidentally.

Pass the known BC-ALAgents commit into the runner and remove the redundant checkout revision lookup.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
@dayland

dayland commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator Author

Simplified the engine revision flow as discussed: _checkout_commit() has been removed. evaluate pr-review now resolves get_pr_review_version() once, stores that SHA in EvaluationContext, and passes the same value into run_pr_review_agent() for manifest validation. run pr-review uses the same resolver with require_clean=False so dirty-checkout smoke testing remains supported. Validation: 79 focused tests, configured type checking, and the full suite (1,075 passed).

Start deterministic validation with GPT-5.6 Sol at the root and GPT-5.6 Luna for review leaves.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
@dayland

dayland commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator Author

Updated the initial defaults to GPT-5.6 Sol for root consolidation and GPT-5.6 Luna for all review leaves. The allowed model scope remains OpenAI/Microsoft only.

@dayland
dayland marked this pull request as ready for review September 14, 2026 14:19
@haoranpb
Sun Haoran (haoranpb) requested review from Wenjie Fan (gggdttt) and a balanced review from Copilot and removed request for Wenjie Fan (gggdttt) September 17, 2026 05:35
Copilot stopped reviewing on behalf of Sun Haoran (haoranpb) due to an error September 17, 2026 05:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note

Copilot was unable to run its full agentic suite in this review.

Pull request overview

Adds deterministic, provenance-validated PR Review evaluations by pinning the engine + Copilot CLI, introducing a run-manifest schema/validator, and expanding metrics + workflow inputs to support leaf scheduling and focused runs.

Changes:

  • Introduces _run-manifest.json schema + validation and wires it into the PR Review agent runtime.
  • Expands PR Review metrics to include model telemetry, scheduling configuration, and BCQuality provenance; updates persistence/reporting behavior.
  • Updates the GitHub Actions workflow/action pins and inputs (leaf model/execution/concurrency, repetition-id, entries) and strengthens related tests.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/test_review_workflows.py Updates workflow pin assertions and validates new workflow_dispatch inputs and requeue payload behavior.
tests/test_review_runners.py Ensures runner receives and propagates agent_version for PR review runs/evaluations.
tests/test_pr_review_run_manifest.py Adds unit tests for loading/validating the new PR review run manifest schema.
tests/test_pr_review_metrics_reporting.py Updates metrics JSON expectations and adds PR review metrics typing usage.
tests/test_pr_review_metrics.py Adjusts PR review metrics semantics and adds validation tests against a manifest-backed run.
tests/test_pr_review_agent.py Adds manifest fixture writing, new env requirements, and asserts expanded metrics/env behavior.
tests/test_agent_versions.py Adds coverage for resolving engine commit even when the engine checkout is dirty (smoke runs).
src/bcbench/types.py Extends PRReviewMetrics with model list, premium requests, scheduling config, and provenance fields.
src/bcbench/commands/run.py Plumbs agent_version into run pr-review using require_clean=False.
src/bcbench/commands/evaluate.py Computes PR review engine version once and passes it consistently to context and runner.
src/bcbench/cli_options.py Adds gpt-5.4 to the allowed Copilot model literals.
src/bcbench/agent/pr_review/run_manifest.py New manifest models + loader + validator enforcing pinned/ordered/deterministic runtime.
src/bcbench/agent/pr_review/metrics.py Adds optional manifest cross-checking and broadens which metrics are promoted when available.
src/bcbench/agent/pr_review/agent.py Requires leaf scheduling env, validates manifest, pins CLI version, and returns manifest-aware metrics.
docs/code-review.md Documents deterministic leaf scheduling, manifest validation, and expanded metric/provenance recording.
.github/workflows/pr-review-evaluation.yml Adds new workflow inputs, concurrency keying, focused entry runs, and uploads run manifest artifacts.
.github/actions/install-agent-harnesses/action.yml Updates pinned engine SHA and GitHub Copilot CLI version.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/bcbench/agent/pr_review/agent.py Outdated
Comment thread src/bcbench/agent/pr_review/run_manifest.py Outdated
Comment thread tests/test_review_workflows.py Outdated
Comment thread tests/test_review_workflows.py
Comment thread tests/test_review_workflows.py Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Comment thread src/bcbench/agent/pr_review/agent.py Outdated
Comment thread src/bcbench/agent/pr_review/run_manifest.py
Comment thread .github/workflows/pr-review-evaluation.yml Outdated
Comment thread src/bcbench/agent/pr_review/run_manifest.py
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

@haoranpb Sun Haoran (haoranpb) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Mostly the Code Review Environment Variable consumption, the rest looks good

Comment thread .github/workflows/pr-review-evaluation.yml Outdated
Comment thread src/bcbench/agent/pr_review/agent.py Outdated
Comment thread src/bcbench/types.py Outdated
Comment thread .github/actions/install-agent-harnesses/action.yml
Comment thread .github/actions/install-agent-harnesses/action.yml
Comment thread .github/workflows/pr-review-evaluation.yml
@dayland
dayland added this pull request to stack #898 September 25, 2026 14:54
Comment thread src/bcbench/agent/pr_review/run_manifest.py
Comment thread src/bcbench/agent/pr_review/metrics.py Outdated
Comment thread .github/workflows/pr-review-evaluation.yml
Comment thread src/bcbench/agent/pr_review/agent.py Outdated
Comment thread .github/workflows/pr-review-evaluation.yml
Comment thread src/bcbench/agent/pr_review/agent.py
Comment thread .github/actions/install-agent-harnesses/action.yml
Comment thread src/bcbench/agent/pr_review/run_manifest.py
Comment thread src/bcbench/agent/pr_review/agent.py Outdated
@github-code-quality

github-code-quality Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Code coverage overview

Languages: Python

Python / code-coverage/pytest

main 7c98b75 experiment/live-pr-r... 0e9284b +/-
88% 88% 0%
View coverage of the 6 most impacted files.
File main 7c98b75 experiment/live-pr-r... 0e9284b +/-
src/bcbench/types.py 92% 92% 0%
src/bcbench/com...nds/evaluate.py 78% 78% 0%
src/bcbench/age...view/metrics.py 100% 100% 0%
src/bcbench/cli_options.py 100% 100% 0%
src/bcbench/age...review/agent.py 77% 78% +1%
src/bcbench/age...run_manifest.py 0% 94% +94%

ℹ️ Calculations represent line coverage. Results may change as calculations are updated.


Updated October 22, 2026 14:54 UTC

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment thread src/bcbench/agent/pr_review/agent.py
Comment thread src/bcbench/agent/pr_review/metrics.py Outdated
Comment thread src/bcbench/agent/pr_review/run_manifest.py
Comment thread .github/actions/install-agent-harnesses/action.yml
Comment thread src/bcbench/cli_options.py
Comment thread src/bcbench/commands/evaluate.py
Comment thread src/bcbench/commands/run.py
dayland-ms and others added 5 commits October 22, 2026 15:10
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 76eb42c4-2acd-4f9c-a898-f4a44f9d46f7
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@dayland
dayland force-pushed the experiment/live-pr-review-leaf-models branch from 46b3a5d to f9d79bb Compare October 22, 2026 14:18
Require aggregate CLI/model identity, exclude incomplete usage from cost comparisons, align PR Review choices and defaults with its workflow, and bump BC-Bench to 0.16.0.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants