Improve /review tests: Opus engine, multi-build base diff, and readable grouped comments

[kubaflo]

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

What this changes

The /review tests Test Failure Analysis workflow classifies whether a PR's CI failures are PR-caused or unrelated. Its deterministic gate computed legsRegressedVsBase — the "Likely PR-caused" headline — by comparing each red PR leg against a single most-recent base build. Flaky UI-test legs that happened to be green on that one base build produced false "regressed-vs-base / Likely PR-caused" positives, so the generated comments carried little signal.

This PR makes the regression diff sample several recent base builds of the PR's own base branch (main or net11.0) and only calls a leg a regression when it is green across enough of them and red on none — and switches the LLM engine to Opus. It also replaces the per-test result table with a skimmable, root-cause-grouped bullet list (the deep-UI-test-analysis style) so a run with hundreds of failures collapses to a handful of readable groups inside the existing compact badge + collapsible header instead of a giant table.

Changes

• Gather-TestFailureContext.ps1 — new Get-AggregatedBaseLegMap aggregator + params RegressionBaseBuilds=5 and MinBaseGreenSamples=2. A leg is a regression only if it is green on ≥ MinBaseGreenSamples base builds and red on NONE. Per base build, a leg that failed even one attempt counts RED (a retry that later passed does not clear a base flake). New leg states flaky-on-base / succeeded-on-base-unconfirmed and new per-failure fields baseSampleCount / baseGreenCount / baseFailedCount.
• Asymmetric conservatism (preserved) — multi-build sampling widens only the false-RED (assert-a-regression) side. The test-level dismissal / false-GREEN side stays single-build-strict on purpose, so this change never turns a real failure green.
• copilot-review-tests.md / .lock.yml — engine model claude-sonnet-4.6 → claude-opus-4.8; replaced only the per-test Markdown table (an unreadable wall of text on runs with hundreds of failures) with a deep-UI-test-analysis style root-cause-grouped bullet list using subtle tokens (✗ PR-related / ℹ Uncertain / ● Unrelated). The compact badge row and the Test Failure Review: [verdict] - click to expand collapsible are kept so the collapsed comment stays a one-glance summary; the badge set now surfaces the key multi-build signal — Overall / Failures / Regressed vs base (replacing the old Platform badge) / Baseline on base. Everything detailed (verdict sentence, grouped bullets, coverage counts, this-PR + base-sampling build links, recommended action) lives inside the collapsible. img.shields.io stays in the network allowlist. Lock recompiled (gh-aw v0.80.9).
• SKILL.md / maui-ci-facts.md — document the multi-build sampling, the new states/fields, and soften the "green on base is proof" wording to require green across several base builds and red on none.

Validation

Ran the gatherer locally against PR #36478 (base main), before vs after:

	legsRegressedVsBase	Verdict
Before (single base build)	14	Not ready
After (5 base builds)	0	Needs human investigation

The ValidateDynamic* (×9), CollectionViewInfiniteScroll, and Issue17400 legs — each green on 4/5 base builds and red on 1/5 — are now correctly classified as flaky-on-base instead of PR regressions.

Notes

• Extracted as a focused, standalone change from the broader improved-reviewer work (Improve maui-copilot reviewer: gate & deep-stage reliability fixes #36473).
• CI-tooling-only change (.github/); no framework/runtime code is touched.

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36533

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36533"

[github-actions]

Skill Validation Results

@kubaflo — new skill validation results are available based on this last commit: 5f11464.
To request a fresh validation after new comments or commits, comment /evaluate-skills.

✅ Skill Validation Results — 5f11464 · Improve /review tests: Opus engine, multi-build base diff, and readable grouped comments · 2026-07-15T21:24:08Z

✅ Static Checks Passed

Skills: 20 | Eval specs linted: 7

Full lint output

── .github/skills/agentic-labeler/tests/eval.vally.yaml
npm warn deprecated prebuild-install@7.1.3: No longer maintained. Please contact the author of the relevant native addon; alternatives are available.
✔ .github/skills/agentic-labeler/tests/eval.vally.yaml is valid
── .github/skills/code-review/tests/eval.capability.vally.yaml
✔ .github/skills/code-review/tests/eval.capability.vally.yaml is valid
── .github/skills/code-review/tests/eval.vally.yaml
✔ .github/skills/code-review/tests/eval.vally.yaml is valid
── .github/skills/code-review/tests/hermeticity.vally.yaml
✔ .github/skills/code-review/tests/hermeticity.vally.yaml is valid
── .github/skills/evaluate-pr-tests/tests/eval.vally.yaml
✔ .github/skills/evaluate-pr-tests/tests/eval.vally.yaml is valid
── .github/skills/try-fix/tests/eval.vally.yaml
✔ .github/skills/try-fix/tests/eval.vally.yaml is valid
── .github/skills/verify-tests-fail-without-fix/tests/eval.vally.yaml
✔ .github/skills/verify-tests-fail-without-fix/tests/eval.vally.yaml is valid

⏭️ LLM Evaluation: Skipped

No changed skills with eval specs found.

🔍 Full results and investigation steps

[Copilot]

Pull request overview

This PR improves the /review tests “Test Failure Analysis” workflow by making the job-level base-branch regression diff less sensitive to base-branch flakiness (sampling multiple recent base builds instead of a single build), and updates the workflow engine model to claude-opus-4.8.

Changes:

• Add multi-build base-branch leg aggregation in Gather-TestFailureContext.ps1, with new parameters (RegressionBaseBuilds, MinBaseGreenSamples) and new leg states/fields to reflect base sampling confidence.
• Update the review-test-failures skill + CI facts documentation to explain the new regression attribution rules and evidence fields.
• Switch the workflow engine model to claude-opus-4.8 and regenerate the compiled lock workflow.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
.github/workflows/copilot-review-tests.md	Switch engine model to Opus and update workflow guidance text to match multi-build base sampling.
.github/workflows/copilot-review-tests.lock.yml	Regenerated compiled workflow lock with the updated model metadata/env values.
.github/skills/review-test-failures/SKILL.md	Document new multi-build regression sampling behavior and evidence fields.
.github/skills/review-test-failures/scripts/Gather-TestFailureContext.ps1	Implement multi-build base-leg aggregation and new leg states + per-failure base sampling counters.
.github/docs/maui-ci-facts.md	Update CI facts to describe multi-build leg diff semantics and new states/thresholds.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

[kubaflo]

🔍 AI-generated update (GitHub Copilot CLI, on behalf of @kubaflo).

@PureWeen — new review results are addressed based on the last commits: b89f35ff7c5 and 1ab7e5de6b7. All 14 review threads are replied and resolved. Summary of the changes:

Base-leg regression diff (Gather-TestFailureContext.ps1)

• Order-independence — the per-occurrence loop now only arms a regression on a green base occurrence when the leg hasn't already been seen red (-not $legAlsoFails), and the cross-leg veto keys off legAlsoFails. A leg that is ever red on base can no longer be reported as regressed regardless of occurrence order (verified with a red-then-green vs green-then-red simulation).
• Crash vs deterministic gating — the one-sample regression shortcut now applies only to deterministic build breaks via a new deterministicBuildError field; crash/infra signatures (native-crash, unhandled-exception, test-host-crash, no-space-left) require the full MinBaseGreenSamples.
• Canceled base builds excluded from the leg sample (incomplete timelines read as false all-green).
• Fetch depth — base builds fetched with -Top max(LookbackBuilds, RegressionBaseBuilds) so the regression diff isn't starved.
• flaky-on-base surfaced distinctly in the table (was masked as also-red); report shows per-leg max green/red across sampled base builds.

Workflow prompt (copilot-review-tests.md)

• Added a comment-format precedence note (this workflow's format overrides the skill's old table contract on the gh-aw path).
• Routed flaky-on-base to ℹ Uncertain only (removed from ● Unrelated), with deterministicAttribution taking precedence.
• Clarified the base-sampling placeholder text and brought the gate.unattributedFailures description in line with the actual indeterminate set (now includes flaky-on-base and succeeded-on-base-unconfirmed).

Accuracy follow-ups (auto-review)

• Corrected the base-leg comment ("green on at least one sampled base build and red on none", not "every sampled base build") and the emitted ceiling-reason string.

Tests — added 7 Pester cases for Get-AggregatedBaseLegMap (all-green, green-plus-red, retry-then-pass=red, inaccessible-skip, none-accessible, cache-reuse, non-positive-id). Full suite: 50 passed, 0 failed. PS1 AST-parses clean and copilot-review-tests.lock.yml was recompiled (body_hash only).

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[kubaflo]

🔍 AI-generated update (GitHub Copilot CLI, on behalf of @kubaflo).

@PureWeen — follow-up to my earlier summary. The Copilot auto-reviewer flagged a few cascading doc-consistency nuances caused by the base-leg changes; all are now fixed and every review thread is resolved (0 unresolved). Latest HEAD: b8e18acbae1.

Additional rounds since the first summary:

• gate.unattributedFailures wording — aligned across the code comment, the emitted ceiling-reason string, the workflow prompt, and maui-ci-facts.md so it lists the full indeterminate set (now including flaky-on-base and succeeded-on-base-unconfirmed). (cf880968ad0)
• Deterministic one-sample caveat — noted that a deterministic build break (crossgen/NativeAOT/linker/MSBuild) regresses on a single green base build, where the general rule needs several; added where the agent consumes the definition and softened the emitted ceiling string. (cf02703a0a9)
• No hard-coded "several" — the posted-comment template's Overall-verdict line and Base-sampling label now reference the real baseSampleCount ("N recent build(s)" / "the single readable base build") so the published comment can't state an inaccurate sample count. (b8e18acbae1)

Validation each round: PS1 AST-parses clean, Pester 50/50 pass, copilot-review-tests.lock.yml recompiled with gh aw v0.80.9 (body_hash only; actions-lock.json untouched). Ready for another look whenever you have a moment.

[PureWeen]

Adversarial re-review

Result: Needs changes. One current-head logic issue remains; see the inline comment.

Required maui-pr is skipped, so CI coverage is undetermined.

Methodology: three independent reviewers plus two adversarial dispute rounds.

[kubaflo]

🔍 AI review round — transient-error boundary fixed (5f114642406)

Addressed the latest [Logic] finding (flagged 2/2 after dispute) on Gather-TestFailureContext.ps1.

Finding: deterministicBuildError was a crash blocklist (4 crash/OOM signatures), so any other coded error XXnnnn: line — including transient NU1301 (restore/network) and MSB3021/MSB3027 (file locks) — was treated as deterministic and earned the single-green-base regression shortcut, hard-capping the verdict to Not ready off one lucky green and bypassing MinBaseGreenSamples flake protection.

Fix: deterministicBuildError now requires the signature to be both not-a-crash and not-transient, via a new Test-IsTransientBuildErrorCode — a conservative, explicit, whole-code blocklist (NU1301, MSB3021, MSB3027). It is deliberately whole-code, not prefix: NU1101/MSB4018 stay deterministic, and unknown codes still default to deterministic (compile breaks reproduce build-to-build). Transient breaks now stay subject to MinBaseGreenSamples, like a crash.

Boundary locked with 8 new Pester cases (5 drive it end-to-end through Get-BuildErrorsFromLog, per the review request):

Input	deterministicBuildError
NU1301 / MSB3021 / MSB3027	$false (transient)
CS0246	$true
NU1101 / MSB4018	$true (prefix not blanket-excluded)
native crash	$false (unchanged)

Full suite 58 passed / 0 failed; both scripts AST-parse clean.

cc @PureWeen

---

🔍 AI-generated summary (GitHub Copilot CLI, on behalf of @kubaflo).

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

[PureWeen]

Adversarial re-review

Result: No new issues found. The current transient-code boundary keeps NU1301, MSB3021, and MSB3027 out of the one-green-base shortcut, while retaining deterministic handling for the tested non-transient codes.

Test coverage: The new Pester cases execute the real build-error classification boundary for transient, deterministic, and crash signatures.

Prior review status: Existing feedback, including the prior transient-code finding and current automatic comments, was inspected without duplicating previously reported concerns.

Methodology: 3 independent reviewers with adversarial consensus + repo domain specialist.