Skip to content

Require Selenium for browser-owned Components regressions - #68787

Closed
PureWeen wants to merge 7 commits into
dotnet:mainfrom
PureWeen:pureween-selenium-first-components-tests
Closed

Require Selenium for browser-owned Components regressions#68787
PureWeen wants to merge 7 commits into
dotnet:mainfrom
PureWeen:pureween-selenium-first-components-tests

Conversation

@PureWeen

@PureWeen PureWeen commented Aug 25, 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.13 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.13 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.

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>
Copilot AI lite review requested due to automatic review settings August 25, 2026 20:40
@PureWeen
PureWeen requested review from a team and wtgodbe as code owners August 25, 2026 20:40

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.

Pull request overview

Updates Components contributor guidance and the validate-blazor-feature skill/eval to require permanent C# Selenium coverage for browser-owned regressions (layout/geometry/observers/scrolling), and to explicitly record a 5-field “permanent regression boundary” decision before handing work off from interactive Playwright validation to permanent tests.

Changes:

  • Adds a five-field “Permanent regression test boundary” decision to src/Components/AGENTS.md, and requires Selenium E2E coverage for browser-owned behavior while disallowing Jest/.test.ts as production regression proof for the same scenario.
  • Extends the validate-blazor-feature skill content to capture the same boundary/handoff guidance without turning it into a permanent test authoring workflow.
  • Adds new Vally eval stimuli to enforce these expectations in skill evaluation.
Show a summary per file
File Description
src/Components/AGENTS.md Adds the permanent regression boundary decision and clarifies Selenium vs Jest expectations for browser-owned behavior.
eng/skill-evals/validate-blazor-feature/eval.vally.yaml Adds new eval cases that grade for selecting Selenium and rejecting Jest-only proof for browser-owned scenarios.
.github/skills/validate-blazor-feature/SKILL.md Updates the skill narrative to record the handoff/boundary decision after interactive validation and before permanent coverage.

Review details

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

  • Files reviewed: 3/3 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread eng/skill-evals/validate-blazor-feature/eval.vally.yaml Outdated
Comment thread src/Components/AGENTS.md Outdated
Copilot AI added 3 commits August 25, 2026 17:02
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
@Youssef1313 Youssef1313 added area-blazor Includes: Blazor, Razor Components area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework labels Sep 2, 2026
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: e2b46a36-410e-48dc-8ce7-cf07dab0bc96
@PureWeen

PureWeen commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

Superseded by #69064, which carries the same change from a trusted same-repository branch so the model-bearing skill evaluation can run.

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.

6 participants