Skip to content

[ab-advisor] Add model_size A/B experiment to test-quality-sentinel #43551

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 2 commits into from
+142 −4
Closed

[ab-advisor] Add model_size A/B experiment to test-quality-sentinel #43551

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e271ccb
Initial plan
Copilot Jul 5, 2026
6542a21
Add model_size A/B experiment to test-quality-sentinel workflow
Copilot Jul 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 126 additions & 4 deletions .github/workflows/test-quality-sentinel.lock.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions .github/workflows/test-quality-sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ permissions:
copilot-requests: write
engine:
id: copilot
model: "${{ needs.activation.outputs.model_size }}"
max-continuations: 15
tools:
cli-proxy: true
Expand Down Expand Up @@ -125,6 +126,21 @@ imports:
- shared/otlp.md
features:
gh-aw-detection: true
experiments:
model_size:
variants: [small, large]
description: "Tests whether a smaller model can preserve test-review decision quality at lower cost versus a larger reasoning-capable model."
hypothesis: "H0: model-size variant does not improve review usefulness acceptance rate. H1: a larger reasoning-capable model improves review usefulness acceptance rate by >=15 percentage points without materially increasing false-positive change requests."
Comment on lines +131 to +133
metric: review_usefulness_acceptance_rate

@github-actions github-actions Bot Jul 5, 2026

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.

The primary metric review_usefulness_acceptance_rate has no instrumentation path in the compiled workflow — this experiment will never collect any primary data.

💡 Details

review_usefulness_acceptance_rate is set as the metric (primary KPI) for the experiment. However, nothing in the compiled test-quality-sentinel.lock.yml writes this value to the experiment state artifact. The framework-collected metrics available automatically are: run_success_rate, run_duration_ms, ai_credits_total, effective_tokens. All of these appear in other experiments in the repo.

A "usefulness acceptance rate" that tracks whether PR reviewers found the AI review helpful requires one of:

  • A reaction-harvesting job that polls PR reactions/comments for thumbs-up signals after the fact
  • A human-in-the-loop labelling step
  • An explicit write to the experiment state artifact from within the agent prompt

Without any of these, the experiment branch will accumulate runs with review_usefulness_acceptance_rate always null. The ab-advisor will never reach a conclusion.

Also affected: false_positive_change_request_rate (secondary metric + guardrail) has the same problem — no automatic collection mechanism — making the <=0.20 guardrail permanently inert.

Fix: Either replace the primary metric with one the framework collects automatically (e.g., run_success_rate as a proxy for now), or add a post-processing job that harvests and writes the acceptance signal. If proceeding with a manual metric, document the collection SLA.

secondary_metrics: [ai_credits_total, false_positive_change_request_rate]

@github-actions github-actions Bot Jul 5, 2026

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.

false_positive_change_request_rate appears in both secondary_metrics and guardrail_metrics — the duplication is contradictory and signals unclear intent.

💡 Details

Line 135: secondary_metrics: [ai_credits_total, false_positive_change_request_rate]
Lines 136–138:

guardrail_metrics:
  - name: false_positive_change_request_rate
    threshold: "<=0.20"

A metric should be either a passive observation (secondary) or an active gate (guardrail), not both simultaneously. If the framework processes both lists independently, the metric is evaluated under two different semantics in the same run. If it deduplicates, one entry is silently dropped — and it's unclear which takes precedence.

Fix: Remove false_positive_change_request_rate from secondary_metrics and leave it only in guardrail_metrics where the threshold is operative:

secondary_metrics: [ai_credits_total]

guardrail_metrics:
- name: false_positive_change_request_rate
threshold: "<=0.20"
- name: run_success_rate
threshold: ">=0.90"
min_samples: 70

@github-actions github-actions Bot Jul 5, 2026

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.

min_samples: 70 gives ~25% statistical power for the claimed 15pp MDE — the experiment will almost certainly produce inconclusive results.

💡 Power calculation

With min_samples: 70 and weight: [50, 50], each arm gets 35 samples. For a two-proportion z-test (alpha=0.05, two-tailed), detecting a 15 percentage-point difference (the H1 threshold stated in the hypothesis) from a baseline of 0.50 requires approximately 169 samples per arm (338 total) at 80% power.

At n=35 per arm, the achieved power is approximately 25% — meaning there is a 75% chance of incorrectly accepting H0 and shipping no model change even if the large model is genuinely 15pp better. Conversely, any positive result at this sample size is highly likely to be noise.

Fix: Raise min_samples to at least 340, or narrow the MDE to a level that is achievable within the traffic budget. If the workflow fires infrequently (e.g. ~5 PRs/day), document the expected time to reach sample size.

weight: [50, 50]
start_date: "2026-07-05"

@github-actions github-actions Bot Jul 5, 2026

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.

[/grill-with-docs] The experiment specifies metric: review_usefulness_acceptance_rate (a 0–1 proportion) but omits analysis_type. Without it, the ab-testing reporting workflow can't automatically select the correct statistical test — proportion_test (two-proportion z-test) is the appropriate choice here and is one of the four valid values in the schema.

💡 Suggested addition 
experiments:
  model_size:
    ...
    metric: review_usefulness_acceptance_rate
    analysis_type: proportion_test   # 0–1 bounded rate metric → two-proportion z-test

Without this, any automated analysis reporter defaults to an unspecified test, which may produce misleading significance results for a proportion metric.

@copilot please address this.

@github-actions github-actions Bot Jul 5, 2026

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.

[/grill-with-docs] No end_date is set. Without a time bound, the experiment runs indefinitely — if statistical significance is never reached (e.g. review acceptance data is sparse), the sentinel agent silently keeps running with 50/50 assignment forever. The ab-testing-advisor guidance recommends setting an explicit end_date to cap experiment duration.

💡 Suggested addition

Based on min_samples: 70 per variant and typical PR review throughput, estimate when 140 samples will accumulate and set that as the bound:

    start_date: "2026-07-05"
    end_date: "2026-10-05"   # 3-month cap; revisit if still underpowered

After end_date passes, pick_experiment.cjs will assign all runs to the control variant (small), providing a safe fallback.

@copilot please address this.

---

# Test Quality Sentinel 🧪
Expand Down
Loading