Repository navigation
ci: build the docsite once, run its browser contracts on that build, keep docsite-test as the required join - #9877
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
PR Analysis Report🔎 Preview ·
|
| Before | After | Change | |
|---|---|---|---|
| Job-minutes per PR update | 80.9 | 97.9 | +16.9 |
| Runner pool, runner-minutes a day | 31,409 | 37,969 | +6,561 (+21%) |
| Average concurrent jobs | 21.8 | 26.4 | +4.6 |
| Time to green (median → this run) | 26.8 min | 16.1 min | −10.7 min |
| Runs a day | 388 | 388 | 0 |
Changed jobs: docsite-browser added: standard · docsite-build added: standard · docsite-test changed: standard · pr-interaction changed: standard · pr-interaction-webkit changed: standard
Not measured (the jobs that use these keep their recent minutes above): .github/scripts/ci-admission.test.mjs, .github/scripts/ci-docsite-browser.test.mjs, .github/scripts/ci-duration-guard.test.mjs, .github/scripts/ci-lab-only-routing.test.mjs, .github/scripts/ci-test-routing.test.mjs, .github/scripts/ci-tooling-routing.test.mjs, .github/scripts/playwright-legs.mjs, .github/scripts/playwright-legs.test.mjs, .github/scripts/shadcn-registry-ci.test.mjs, .github/scripts/spec-owner-workflow.test.mjs.
Flags: runner-pool usage +21%.
Assumptions: before is each job's average over the last 20 pull-request runs; after keeps that for unchanged jobs and uses this run's minutes for added or changed ones, weighted by how often each changed job ran in those runs; runs a day averages the seven whole days before today; time to green compares this run with the median of those runs that ran the same jobs.
Generated by PR Enrichment workflow | View full report
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes. The split runs green end to end on this head, but the new browser-contract skip covers more than AST-030 allows.
Decide whether the browser contracts apply(.github/workflows/ci.yml:639) reads only thepathsregex, so a change limited tointernal/interaction-harness/,internal/a11y-spec/,apps/storybook/.storybook/or a test file outside a package (such as.github/scripts/*.test.mjs) skips the docsite browser contracts although the trusted classifier reportsfull_suite=trueand AST-030 FR13 runs every owner's full suite for shared infrastructure; gate the step ontouches == 'true' || full_suite == 'true'and add a test that such a change runs the contracts.
Two small follow-ups: the comments at ci.yml:570-571 and ci.yml:576-578 (and the test title) should name everything the regex skips, and playwright-legs.mjs:229 still prints the old interaction-legs.mjs usage. The split also makes docsite-test report later (about 105 s on this run; the description estimates three minutes), which FR19 asks reviewers to weigh.
[Automated review]
| run: (set +e; pnpm -F @astryxdesign/docsite test > docsite-data.log 2>&1; echo $? > docsite-data.exit.tmp; mv docsite-data.exit.tmp docsite-data.exit) & | ||
| - name: Decide whether the browser contracts apply | ||
| id: browser | ||
| run: echo "applies=${{ (steps.scope.outputs.spec_only != 'true' && steps.scope.outputs.tooling_only != 'true' && steps.scope.outputs.lab_only != 'true') && steps.scope.outputs.touches == 'true' }}" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
This decision reads only the paths regex (line 590), never the trusted classifier's full_suite. A change limited to internal/interaction-harness/, internal/a11y-spec/, apps/storybook/.storybook/ or a test file outside a package (.github/scripts/*.test.mjs) therefore skips the docsite browser contracts, though the classifier marks it shared infrastructure and AST-030 FR13 runs every owner's full suite for it. Gating on both keeps every skip the description claims for component tests, stories, spec records and changesets:
run: echo "applies=${{ (steps.scope.outputs.spec_only != 'true' && steps.scope.outputs.tooling_only != 'true' && steps.scope.outputs.lab_only != 'true') && (steps.scope.outputs.touches == 'true' || steps.scope.outputs.full_suite == 'true') }}" >> "$GITHUB_OUTPUT"A test that a full_suite change runs the contracts would cover the fail-closed edge (FR9).
There was a problem hiding this comment.
Requesting changes. The two new commits slim the uploaded docsite build soundly, but the browser-contract skip still covers more than AST-030 allows.
Decide whether the browser contracts apply(.github/workflows/ci.yml:639) still reads only thepathsregex, so a change limited tointernal/interaction-harness/,internal/a11y-spec/,apps/storybook/.storybook/or a test file outside a package (such as.github/scripts/*.test.mjs) skips the docsite browser contracts although the trusted classifier reportsfull_suite=trueand AST-030 FR13 runs every owner's full suite for shared infrastructure; gate the step ontouches == 'true' || full_suite == 'true'and add a test that such a change runs the contracts.
Still open and non-blocking: the comments at ci.yml:570-571 and ci.yml:576-578 and the test title should name everything the regex skips, and playwright-legs.mjs:229 still prints the old interaction-legs.mjs usage. On this head the docsite jobs did not run because heavy CI was deferred, and the lint failure is in a DateInput spec this pull request does not touch.
[Automated review]
There was a problem hiding this comment.
Requesting changes: two issues block this head.
Decide whether the browser contracts apply(.github/workflows/ci.yml:641) still reads only thepathsregex, so a change limited tointernal/interaction-harness/,internal/a11y-spec/,apps/storybook/.storybook/or a shared test file such as.github/scripts/*.test.mjspassesdocsite-testwithout the docsite browser contracts, although the trusted classifier reportsfull_suite=truefor these paths and AST-030 FR13 runs every owner's full suite for shared infrastructure; gating ontouches == 'true' || full_suite == 'true'and adding a test for a harness-only change fixes it.- The merge of
mainrewroteinternal/interaction-harness/src/busy.ts:910toif (f.t >= window.from) {break;}, which fails the requiredlintcheck's formatting step; restoremain's line or runpnpm exec prettier --write internal/interaction-harness/src/busy.ts.
While you are there, the skip comments at ci.yml:572-573 and ci.yml:578-580 and the interaction-legs.mjs usage string at playwright-legs.mjs:229 are out of date.
[Automated review]
| let before: (typeof valued)[number] | undefined; | ||
| for (const f of valued) { | ||
| if (f.t >= window.from) break; | ||
| if (f.t >= window.from) {break;} |
There was a problem hiding this comment.
This line came in with the merge of main (where it reads if (f.t >= window.from) break;) and fails the required lint check's Prettier step. Restore main's line, or run pnpm exec prettier --write internal/interaction-harness/src/busy.ts, which puts break; on its own line inside the block.
b40c063 to
fde9596
Compare
There was a problem hiding this comment.
Requesting changes: one small fix remains.
- The description's "Also" section says this pull request rewrites
internal/interaction-harness/src/busy.tsto fix alintfailure from a merge ofmain, but this head has no such merge, leavesbusy.tsunchanged (line 910 matchesmain), and passeslint, so please delete that section so the description matches the change.
Both earlier blockers are resolved: the browser-contract decision now also runs on the classifier's full_suite, and the busy.ts edit is gone. The split itself conforms to AST-030 (FR7, FR11, FR13, FR16, FR17, FR20, FR26), and its workflow tests cover the join and the skip. CI on this head deferred the heavy suites, so docsite-build and docsite-browser still need their run before merge.
[Automated review]
There was a problem hiding this comment.
Approving: splitting docsite-test into a build job and a browser job behind the same required join conforms to AST-030 (FR7, FR11, FR13), and every unknown or skipped state fails the join.
CI's own run at this head built the docsite once, matched the build's commit in the browser leg and passed the browser contracts, and the description now matches the change.
- Non-blocking: the "Playground preview isolation" section of
CONTRIBUTING.mdstill says the browser-contract step runs indocsite-test; it now runs indocsite-browser. - The pull-request comment flags +16% runner-pool usage against the description's estimate of about +40 job-minutes a day; that cost is a maintainer's call.
[Automated review]
There was a problem hiding this comment.
Approving: splitting docsite-test into docsite-build, docsite-browser and a required join conforms to AST-030 (FR7, FR11, FR13), and the join fails on every skipped, cancelled or unknown state. Required CI passed on this head: the docsite was built once, the browser contracts passed against that build, and docsite-test, test, build and lint are green. Nothing further is required.
- Non-blocking: the build upload in
.github/workflows/ci.yml(lines 670-684) leaves outapps/docsite/public/shadcn/, the generated ShadCN registry thatnext startserves, although the comment above it says the upload carries everythingnext startreads; add the directory or narrow the comment. - The pull-request comment flags +17% runner-pool usage against the description's estimate of about +40 job-minutes a day; that cost is a maintainer's call.
[Automated review]
| apps/docsite/src/themes/astryx.css | ||
| apps/docsite/public/playground-types.json | ||
| apps/docsite/public/monaco/ | ||
| apps/docsite/public/vendor/ |
There was a problem hiding this comment.
Non-blocking: the docsite build also writes apps/docsite/public/shadcn/ (the generated ShadCN registry, apps/docsite/scripts/generate-data.mjs:2750-2752), which next start serves. Adding it here keeps the comment above accurate and lets a future browser contract fetch a registry item; otherwise narrow the comment.
There was a problem hiding this comment.
Approving: splitting docsite-test into docsite-build, docsite-browser and a required join conforms to AST-030 (FR7, FR11, FR13), and the join fails on every skipped, cancelled or unknown state. Required CI has now run on this head and passed: the docsite was built once, all 38 browser contract tests passed against that build, and docsite-test, test, build and lint are green.
- Non-blocking: the header of
.github/scripts/shadcn-registry-ci.test.mjs(line 8) still says the production gate runs indocsite-test; it now runs indocsite-build. - Miss: non-blocking, the comment above
test-registry-e2ein.github/workflows/ci.yml(lines 1200-1201) says the same and needs the same one-word fix. - Non-blocking: the description gives the build artifact as 62 MB; on this head, which also uploads the ShadCN registry, it is 70 MB.
- The pull-request comment flags +19% runner-pool usage against the description's estimate of about +40 job-minutes a day; that cost is a maintainer's call.
[Automated review]
There was a problem hiding this comment.
Approving: splitting docsite-test into docsite-build, docsite-browser and a required docsite-test join conforms to AST-030 (FR7, FR11, FR13), and the join passes only on a passed build with passed browser contracts or with none to run, failing on a failed, cancelled or skipped leg and on an unknown scope. Required CI has run on this head and passed, with the browser contracts run against the uploaded build of the same commit.
- Non-blocking: re-running only a failed
docsite-browserjob more than a day after its build fails at the download, because the build artifact is kept one day; re-running all jobs recovers. - The pull-request comment flags +19% runner-pool usage against the description's estimate of about +40 job-minutes a day; weighing that cost is a maintainer's call.
[Automated review]
…s on that build in docsite-browser, and keep docsite-test as the required join docsite-build builds the packages and the docsite once, runs the data tests and the production registry gate, and uploads the build (about 70 MB: the server and static output, generated data and theme, the ShadCN registry and the playground's vendored assets; no compile cache, traces, source maps or file traces) stamped with its commit. docsite-browser downloads that build, fails unless it is of the commit it checked out, and runs the browser contracts as a one-leg matrix planned by recorded time. docsite-test stays the required context as a join that fails on a failed, cancelled or skipped leg. The contracts skip only for component tests, stories, spec records and changesets that are not shared infrastructure (full_suite). The interaction planner becomes playwright-legs.mjs for all three leg lanes.
265350c to
e3bd076
Compare
There was a problem hiding this comment.
Approved pending CI: a re-run of all jobs is in progress on this head; the previous run passed every required check, including all docsite browser contracts against the uploaded build of the same commit.
Splitting docsite-test into docsite-build, docsite-browser and a required docsite-test join conforms to AST-030 (FR7, FR11, FR13), and the join passes only on a passed build with passed browser contracts or with none to run. Nothing further is needed from you.
[Automated review]
There was a problem hiding this comment.
Approved pending CI: the first CI attempt on this head deferred the heavy suites, and a re-run of all jobs is now in progress, so build, test and docsite-test have not reported on this head yet.
Splitting docsite-test into docsite-build, docsite-browser and a required docsite-test join conforms to AST-030 (FR7, FR11, FR13), the join passes only on a passed build with passed browser contracts or with none to run, and the merge of main leaves this pull request's own changes untouched. Nothing further is needed from you.
[Automated review]
Why
docsite-testbuilds the docsite, runs its data tests and then its browser contracts in one 15-minute job. On recent green runs the job took 737 s (median; p90 760 s): docsite build 332 s, browser contracts 265 s, setup and package builds about 140 s, checkout 9 s. That leaves about two and a half minutes of slack, so any slow checkout or busy runner cancels it at the limit (#9795, #9725), and the required check goes red without a failing test.What changes
docsite-testbecomes three jobs. The docsite is still built once.docsite-builddoes whatdocsite-testdid up to the build. It validates spec records on the spec-only lane, builds the packages and the docsite, runs the data tests and the hidden production registry gate. When the browser contracts apply, it stamps the build with its commit (.next/BUILD_COMMIT) and uploads whatnext startreads: the build without its compile cache, the generated data and theme, and the playground's vendored assets (one-day retention).docsite-browserdownloads that build. It fails unless the build's commit is the commit it checked out, and it never builds. It runs the browser contracts as a planner-driven matrix of one leg:playwright-legs.mjssplits the spec files by recorded time, so a second leg is a one-line change.docsite-teststays the required context (FR7) as a join on a slim runner. It keeps the outside-contributor, CI-is-full and deferred-heavy-CI stops and the Lab-only notice. It passes only when the build passed and the browser leg passed, or when the build's scope found no browser contract to run. A failed, cancelled or skipped leg, a failed build or an unknown scope fails it.__tests__/file or a*.test/*.specfile in any package, CLI included), a story, a spec record or a changeset, and the trusted classifier does not call the change shared infrastructure. A harness, a11y-spec, Storybook-config or shared-test change setsfull_suiteand runs them, as for every owner (FR13). So does a change toapps/docsite/**(its own e2e specs and blog posts included), any non-test source, a theme, a README, a lockfile or the workflow. An undecidable scope runs them too.playwright-legs.mjs, with.github/playwright-leg-durations.json. It now takes a config, an optional project and a times key, and records logs from a suite with one unnamed project. The interaction legs pass the same config and project as before.Measured on this pull request's run
Measured on main with the docs page-weight fix (#9907) merged in:
docsite-testdocsite-builddocsite-browserCI impact
ubuntu-latestfor the build and the browser leg; the join runs onubuntu-slim.docsite-buildreplacesdocsite-test), so admission's pull-requestpeakstays 16. No Actions cache is used (ci-no-dependency-cache.test.mjsstill holds).Tests
ci-docsite-browser.test.mjsis rewritten for the three jobs. It covers:playwright-legs.test.mjsadds the unnamed-project log reader and the docsite times within one leg's budget.docsite-testis a join), ci-lab-only-routing, ci-test-routing, ci-tooling-routing, shadcn-registry-ci, spec-owner-workflow and the duration-split guard.