Skip to content

Require Selenium for browser-owned Components regressions - #69064

Open
PureWeen wants to merge 7 commits into
mainfrom
pureween-create-trusted-selenium-pr
Open

Require Selenium for browser-owned Components regressions#69064
PureWeen wants to merge 7 commits into
mainfrom
pureween-create-trusted-selenium-pr

Conversation

@PureWeen

@PureWeen PureWeen commented Sep 4, 2026

Copy link
Copy Markdown
Member

Summary

This implements Ilona's maintainer feedback on #68748 and adds a focused deterministic browser-to-Selenium workflow:

  • Keeps the permanent boundary: user-visible behavior that depends on real browser layout, observers, scheduling, or event ordering requires C# Selenium coverage under src/Components/test/E2ETest.
  • For a browser regression or reproduced browser failure, adds the operational sequence: reproduce with Playwright, distill the failure to the smallest temporary JavaScript diagnostic assertion in the real page, and repeat the unchanged scenario a recorded bounded number of times. New-feature validation without a failure skips this diagnostic/repeat loop.
  • Calls a reproduction deterministic only when every sampled run fails the same assertion for the same reason. Ten runs is a reasonable default confidence sample when no stronger repository evidence exists, not a magical count or proof across environments.
  • Sends nondeterministic results back to producer/precondition/timing investigation or a faithful owning lower boundary. It uses an explicit test-controlled gate such as TaskCompletionSource only when deterministic managed or intermediate ordering must be controlled; it does not stabilize races with an arbitrary fixed sleep.
  • Requires the permanent Selenium scenario to preserve the validated browser scenario and final observable. Browser.True/Browser.Equal (WebDriverWait) is sufficient when the final observable can be awaited directly; an explicit gate is not ceremonial. Existing bounded polling helpers may delay between checks, so this is not a syntactic ban on every Thread.Sleep.
  • Clarifies the split boundary: deterministic JavaScript manager, timer-state, structural DOM, and validation-engine contracts may remain in Jest/jsdom when that module contract is the final observable. Those tests do not prove real browser reachability, scheduling, layout, or user-visible behavior.
  • Defines the nearest-opposite control as the closest scenario that must remain green and makes an adjacent same-producer control conditional when meaningful.
  • Keeps temporary diagnostic JavaScript outside the production change and removes it after Selenium supersedes the regression scenario.
  • Removes brittle output-not-matches graders and moves semantic denials/permissions to prompt-judge rubrics. Remaining static graders check only broad surface signals; five-field decisions are judged for substantive case-specific values rather than placeholder labels.

This PR changes contributor guidance, the existing interactive validation skill, and its eval lane only; it does not change production or test implementation code.

Validation

  • .github/skills/validate-blazor-feature passes strict skill lint.
  • eng/skill-evals/run.ps1 Validate -Eval eng/skill-evals/validate-blazor-feature/eval.vally.yaml passes repository layout checks, Vally 0.14 strict lint/schema validation, and the model-free experiment dry-run.
  • eng/skill-evals/run.ps1 Lint -Eval eng/skill-evals/validate-blazor-feature/eval.vally.yaml passes the pinned Vally 0.14 strict lint.
  • git diff --check passes.
  • Independent skeptical reviews covered overfitting, feature-versus-regression scope, the JS lower-boundary split, timing/sleep guidance, diagnostic-probe cleanup, fixed-count wording, control proportionality, and Vally false positives/negatives. Their concrete findings were fixed; the final read-only review reported no remaining high-confidence issues.

Skill-creator comparison

The pre-change skill was snapshotted outside the repository and compared with the revised workflow across four paired Claude Sonnet 5 cases: deterministic browser handoff, nondeterministic producer investigation, arbitrary sleep rejection, and manager-contract versus browser-reachability selection.

  • Revised skill: 95% mean assertion pass rate.
  • Pre-change snapshot: 65% mean assertion pass rate.
  • Delta: +30 percentage points.
  • Exactly one run per configuration was used. Timing and token measurements were unavailable, so no performance conclusion is claimed.
  • The lower-boundary split case was non-discriminating because both versions selected the correct split. Most improvement came from the new determinism and sleep/wait guidance.
  • The benchmark metadata and static review viewer were regenerated to reflect the one-run scope accurately.

Bounded model-bearing Vally evidence and limitations

A focused baseline-versus-skilled run used Claude Opus 4.6 for three trials each across four workflow cases (12 trajectories per variant):

  • Skilled aggregate: 90.0%, above the 80% threshold; all four stimuli passed 3/3.
  • Baseline aggregate: 59.2%, below threshold.
  • Pairwise comparison: skilled won 8, tied 4, and lost 0 of 12 trials; mean score +0.42 with 95% CI [+0.17, +0.67].
  • Grader pass/fail flips favored skilled +9/-0, exact McNemar p=0.004.
  • The sleep case passed 3/3 by selecting an observable WebDriver wait without a ceremonial TaskCompletionSource, which is the intended policy. The model did not invoke the skill for those three straightforward sleep prompts, but the behavior was correct; the other nine skilled trials invoked validate-blazor-feature.
  • This is a bounded focused suite, not the repository's full five-run standard eval. No full standard baseline-versus-skilled suite was run, so the evidence is reported at its exact scope.

Copilot AI added 7 commits August 25, 2026 15:40
Document the permanent regression boundary, align the interactive validation skill handoff, and add focused Vally coverage for browser and lower-boundary test selection.

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>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: e2b46a36-410e-48dc-8ce7-cf07dab0bc96
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: e2b46a36-410e-48dc-8ce7-cf07dab0bc96
Copilot AI lite review requested due to automatic review settings September 4, 2026 18:18
@PureWeen
PureWeen requested review from a team and wtgodbe as code owners September 4, 2026 18:18
@github-actions github-actions Bot added area-blazor Includes: Blazor, Razor Components area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework labels Sep 4, 2026
@PureWeen

PureWeen commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

/evaluate 1d6c41b

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Hey @dotnet/aspnet-build, looks like this PR is something you want to take a look at.

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.

Copilot review overview

🟢 Approval recommended

The changes are scoped to guidance/skill/eval content, and the new instructions reference existing repo paths and mechanisms consistently without introducing correctness or integration risks.

Review tier: Lite
Findings: None

What changed in this PR

This PR updates contributor guidance and the validate-blazor-feature skill workflow to require real-browser (Selenium) coverage for browser-owned Components regressions, and to formalize a deterministic Playwright → diagnostic JS probe → Selenium handoff process (including a five-field “boundary” record) before selecting permanent coverage.

Changes:

  • Expanded src/Components/AGENTS.md implementation workflow with deterministic reproduction guidance and a “Permanent regression test boundary” policy (including the Selenium/Jest split and synchronization guidance).
  • Updated .github/skills/validate-blazor-feature/SKILL.md to incorporate the deterministic probe + boundary-recording workflow and clarify the permanent-test handoff expectations.
  • Extended eng/skill-evals/validate-blazor-feature/eval.vally.yaml with new stimuli/rubrics covering the new boundary and determinism policies, and adjusted scoring weights.
File Description
src/​Components/​AGENTS.md Adds explicit determinism workflow and a normative permanent regression test boundary (Selenium vs Jest/.test.ts guidance).
eng/​skill-evals/​validate-blazor-feature/​eval.vally.yaml Updates eval weighting and adds new stimuli/rubrics to enforce the revised policy and decision-making.
.github/​skills/​validate-blazor-feature/​SKILL.md Aligns skill instructions with the new deterministic reproduction + handoff + boundary-record workflow.

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

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Smoke skill evaluation passed for validate-blazor-feature at 1d6c41b. Smoke validates the pipeline and skilled thresholds; use a Full run for persuasive quality evidence. View the workflow run and artifacts.

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

Labels

area-blazor Includes: Blazor, Razor Components area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants