diff --git a/.github/workflows/shard-timings-refresh.yml b/.github/workflows/shard-timings-refresh.yml new file mode 100644 index 0000000000..2b2274d322 --- /dev/null +++ b/.github/workflows/shard-timings-refresh.yml @@ -0,0 +1,604 @@ +# Regenerate scripts/test-shard-timings.json on a timer, and open the PR. +# +# ══════════════════════════════════════════════════════════════════════════════ +# WHY THIS EXISTS: THE BALANCING INPUT WAS THE ONLY PART OF THE LOOP WITH NO +# CLOCK ON IT. (#16464) +# ══════════════════════════════════════════════════════════════════════════════ +# +# scripts/test-shard-timings.json is the per-package duration dataset the Test +# Core split is binned from. It is GENERATED — scripts/measure-test-shard- +# timings.mjs turns a green run's six turbo summaries into it — but until now it +# was generated only when a person remembered to. It went 13 days without a +# refresh while ~700 test files were added, and the shard it mis-weighted was +# killed by the job wall twelve times in one day and ejected from the merge queue +# twice (#16173). +# +# The rot is one-directional and silent, which is what makes a timer the fix +# rather than more discipline: suites only get slower, the table stays put, and +# the shard that drifted heavy reads as balanced on paper right up to the moment +# it is killed — and a killed shard produces NO measurement while the rollup +# reads green. +# +# WHAT THIS WORKFLOW IS NOT ALLOWED TO DO, AND WHY EACH ONE IS LOAD-BEARING +# ------------------------------------------------------------------------ +# ⛔ It never hand-edits the dataset. Every byte it commits came out of the +# generator. A hand-touched number wearing a generated file's `provenance` +# is worse than a stale one: the staleness is at least visible in +# `measuredAt`. +# ⛔ It never touches `timeout-minutes`, the shard matrix, MAX_SHARD_OVER_MEAN, +# MAX_MEASURED_OVER_PREDICTED or FILE_SHARDED_PACKAGES. The partitioner's +# own header calls raising a bound "the one move that cannot be right", and +# its pin 3c is built to red on exactly that. If an honest refresh breaches +# a bound, that breach is the REPORT, not a problem to be tuned away — it is +# quoted verbatim into the PR body and a human decides. See "THE PINS" below. +# ⛔ It never opens a PR when the regenerated file is byte-identical. A weekly +# no-op PR trains everyone to ignore this PR. +# +# HOW IT GETS ITS INPUTS, AND WHY IT RUNS HERE RATHER THAN IN AN AGENT WORKTREE +# ---------------------------------------------------------------------------- +# The documented refresh path downloads the six `test-core-run-summary-N-of-6` +# artifacts of a green run. `GET /actions/artifacts/{id}/zip` redirects to +# `productionresultssa*.blob.core.windows.net`, and every agent container's +# egress policy answers 403 to CONNECT for that host — reproduced four ways, on +# two artifacts across two runs, on the pre-signed URL as well as the API one, on +# two separate days and containers (#16222, and again on #16173). So the +# documented path cannot be walked from a dev seat at all. +# +# A GitHub-hosted runner reaches that host natively — it is the same host +# `actions/download-artifact` uses. Running the refresh here is therefore not a +# workaround for the denial, it is the removal of the channel from the loop: the +# regeneration happens where the data already is, on a timer, and no seat needs +# reachability it does not have. +# +# CHOOSING THE RUNS IS THE HARD PART — see scripts/ci/select-shard-timings-run.mjs +# ------------------------------------------------------------------------------ +# "The newest run" is wrong three different ways here (cancelled runs, cache +# replays, expired artifacts), and a refresh built on the wrong run is worse than +# no refresh because it stamps a fresh `measuredAt` on numbers nobody measured. +# That script carries the argument and the self-test; this file only drives it. +# +# ⚠ AND IT IS "RUNS", PLURAL, WHICH THE DOCUMENTED PROCEDURE DOES NOT SAY. +# Measured on this lane's first live run (34083991141): of the seven retained +# green push runs on main, the BEST measured 52 of the 71 packages the committed +# dataset holds, and the others measured 2, 3, 13, 18, 22 and 49. All seven were +# partial cache replays. That follows from the cache design rather than from luck +# — turbo's key is namespaced per shard and only main pushes write it, so a +# package whose inputs have not changed is a HIT, and the generator refuses hits +# rather than recording a replayed ~0.1s window as a suite's cost. "Download six +# artifacts from any green run" therefore measures a SLICE of the workspace. +# +# So the regeneration step accumulates runs, each under its own `--run ` +# group — the grouping #16473 added, which sums a sliced package's slices within +# a run and medians the per-run sums across runs. A package seen in three of the +# accumulated runs gets the median of three observations, which is the property +# the dataset's own merge rule always claimed and could not previously deliver. +# +# ⚠ AND ACCUMULATION ALONE STILL FALLS SHORT, so the pass MERGES rather than +# replaces. Measured on the same live runs: the union of all seven retained runs +# reaches 57 of 71 packages and converges there, because 14 packages are cache +# HITs in every one of them. Those are carried at their previous weights through +# `--merge-into`, and the carry is sound for one specific reason — a cache HIT is +# not missing data, it is POSITIVE EVIDENCE that the package's inputs are +# unchanged since the run whose output was replayed, so its last measured weight +# still describes it. Every carried package is named in the dataset's +# `carriedOver` list, and a package absent for any OTHER reason is not carried at +# all: it drops out and the coverage check names it as a refusal. The file's +# invariant is preserved exactly — every number in it is a real measurement of +# code as it stands, never an estimate. +# +# THE PINS, AND THE ONE THING A MACHINE MUST NOT DECIDE +# ---------------------------------------------------- +# partition-test-shards.mjs `--self-test` grades the dataset against the +# acceptance bound. On a refresh that makes a package heavier than any split can +# bin, it reds BY DESIGN and names the remedy (raise the slice count — never the +# bound), and its pin 3c additionally demands a decision the day the CLI comes +# back under the bound on its own. Both are judgement, so this workflow REPORTS +# the verdict and never acts on it: the self-test is run on the refreshed file +# and its output — pass or fail, verbatim — goes into the PR body. The PR is +# opened either way, because a refusal to open it would lose the measurement, +# which is the exact silent-rot failure this workflow exists to end. + +name: Shard Timings Refresh + +on: + schedule: + # Monday 05:30 UTC. After the nightly lanes, and early enough in the week + # that the PR is in front of someone before the week's merge volume builds. + - cron: '30 5 * * 1' + workflow_dispatch: + # Changes to this lane get exercised before they merge — the same posture + # half-state-patrol.yml and required-set-patrol.yml keep, and the reason a + # scheduled-only lane is not an option: `dispatch-gates.mjs` reds when a gate + # family is reachable ONLY on a schedule, because a family no PR-time event + # reaches appears on no card's gate list and is therefore graded by nobody + # until the next sweep. + # + # On a `pull_request` run everything executes — selection, download, + # regeneration, the coverage check, the byte comparison and the partitioner's + # verdict — so the transport and the flags are proven on a real runner rather + # than argued about. The WRITE is what is skipped: no branch is pushed, no PR + # is opened, no label is written, and the body that would have been posted is + # rendered to the run's step summary instead. + pull_request: + paths: + - '.github/workflows/shard-timings-refresh.yml' + - 'scripts/ci/select-shard-timings-run.mjs' + - 'scripts/measure-test-shard-timings.mjs' + +# Read-only at the top level; the one job widens to exactly what it writes. +permissions: + contents: read + +# One refresh at a time. A second run while the first is mid-push would race on +# the same bot branch; the newer inputs are the better ones, so the in-flight run +# yields. +concurrency: + # Scoped BY REF, not one global group. Two scheduled refreshes cannot overlap + # (they would race on the same bot branch, and the newer inputs are the better + # ones, so the in-flight run yields) — but two PRs touching this lane are + # unrelated runs, and a single global group would have each cancel the other's + # dry run and report a cancellation as though the lane were busy. + group: shard-timings-refresh-${{ github.ref }} + cancel-in-progress: true + +jobs: + refresh: + name: Regenerate the shard-timings dataset + runs-on: ubuntu-latest + timeout-minutes: 30 + permissions: + # Declaring any `permissions:` block drops every scope not listed, so all + # three are spelled even though only two are writes. + contents: write # push the bot branch + pull-requests: write # open the PR and add its label + actions: read # list runs, read jobs, download run-summary artifacts + steps: + - name: Checkout repository + uses: actions/checkout@v7 + with: + # A PAT when one exists, the Actions token otherwise — the same choice + # cut-rc.yml makes, for the same reason, and the difference is stated + # in the PR body rather than left to be discovered: a PR opened with + # the Actions `GITHUB_TOKEN` starts NO workflow runs (GitHub's + # recursion guard), so with the fallback credential the refresh PR + # arrives with no CI of its own until someone pushes to it or reopens + # it. With the PAT its checks run immediately. + token: ${{ secrets.RELEASE_PUSH_TOKEN || github.token }} + + - name: Setup Node.js + uses: actions/setup-node@v7 + with: + node-version: '22' + + - name: Setup pnpm + uses: ./.github/actions/setup-pnpm + + - name: Get pnpm store directory + shell: bash + run: echo "STORE_PATH=$(pnpm store path --silent)" >> $GITHUB_ENV + + # Restore-only: scheduled runs read main's store cache; the per-push + # workflows own saving it. Same shape as coverage-nightly.yml. + - name: Restore pnpm cache + uses: actions/cache/restore@v6 + with: + path: ${{ env.STORE_PATH }} + key: ${{ runner.os }}-pnpm-store-v3-${{ hashFiles('**/pnpm-lock.yaml') }} + restore-keys: | + ${{ runner.os }}-pnpm-store-v3- + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + # Verify the instruments BEFORE trusting their output. Both scripts carry a + # battery floor, so this also catches the case where their assertions + # stopped running — which would otherwise let a wrong dataset through a + # green-looking pipeline. + - name: Self-test the generator and the run selector + run: | + # A COLLECTOR, not a bare sequence. Under `bash -e` the first non-zero + # exit aborts the step, so a plain `a && b` list leaves the second + # self-test neither green nor red -- and this step exists precisely to + # say which instrument is broken before the dataset is trusted. + # ⛔ Never let the collector swallow the exit code: a green step over a + # red self-test looks identical to success. + failed="" + run_self_test() { + echo "-- $*" + if "$@"; then + echo "PASS $*" + else + echo "FAIL $*" + failed="${failed} $*"$'\n' + fi + return 0 + } + run_self_test node scripts/measure-test-shard-timings.mjs --self-test + run_self_test node scripts/ci/select-shard-timings-run.mjs --self-test + if [ -n "$failed" ]; then + echo "" + echo "Shard-timings instrument self-tests — the following FAILED:" + printf "%s" "$failed" + exit 1 + fi + echo "Shard-timings instrument self-tests — both ran and passed" + + # The package list the coverage check judges against. `turbo ls` is + # experimental, so the reader asserts its payload loudly rather than + # defaulting around it (an empty list would make every package look deleted + # and every coverage check pass). + - name: List the workspace + run: pnpm exec turbo ls --output=json > "$RUNNER_TEMP/turbo-ls.json" + + - name: Choose a green, uncensored, un-replayed run + id: select + env: + GITHUB_TOKEN: ${{ github.token }} + run: | + node scripts/ci/select-shard-timings-run.mjs --candidates --limit 15 \ + > "$RUNNER_TEMP/candidates.json" + echo "Eligible runs, newest first:" + node -e ' + const runs = JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP + "/candidates.json", "utf8")); + for (const r of runs) console.log(` ${r.run_id} ${r.created_at} ${r.head_sha.slice(0, 10)}`); + ' + + # ACCUMULATE runs until the workspace is covered — do not look for one run + # that covers it, because there is no such run. + # + # MEASURED on the first live run of this lane (run 34083991141), and it is + # the fact that shapes this step: of the seven retained green push runs on + # main, the BEST measured 52 of the 71 packages the committed dataset + # holds, and the rest measured 2, 3, 13, 18, 22 and 49. Every one of them + # was a partial cache replay. That is not bad luck, it is the cache design: + # turbo's key is namespaced per shard and only main pushes write it, so a + # package whose inputs have not changed is a HIT — and the generator + # refuses hits rather than recording a replayed ~0.1s window as a suite's + # cost. "Download six artifacts from any green run" therefore measures a + # SLICE of the workspace, never all of it. + # + # So runs are accumulated. Each contributes its six summaries under its own + # `--run ` group, which is exactly the grouping #16473 added: slices are + # summed within a run and the per-run sums are medianed across runs, so a + # package measured by three of these runs gets a median of three + # observations rather than whichever run happened to be read last. Feeding + # several runs WITHOUT that grouping is refused by the generator by name. + # + # The loop stops at the first accumulation that covers the workspace, so a + # quiet week costs one run's download and a busy one costs a few. + - name: Regenerate the dataset, accumulating runs until the workspace is covered + id: generate + env: + GITHUB_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + WORK="$RUNNER_TEMP/refresh" + mkdir -p "$WORK" + CHOSEN='' + ACCEPTED=() + + RUN_COUNT=$(node -e 'console.log(JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP + "/candidates.json", "utf8")).length)') + for i in $(seq 0 $((RUN_COUNT - 1))); do + RUN_ID=$(node -e 'const r=JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP+"/candidates.json","utf8"))[Number(process.argv[1])];console.log(r.run_id)' "$i") + echo "::group::Candidate run $RUN_ID" + SUMDIR="$WORK/$RUN_ID" + rm -rf "$SUMDIR"; mkdir -p "$SUMDIR" + + OK=1 + for N in 1 2 3 4 5 6; do + ART=$(node -e 'const r=JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP+"/candidates.json","utf8"))[Number(process.argv[1])];console.log(r.artifact_ids[process.argv[2]])' "$i" "$N") + # curl drops the Authorization header across the redirect to the + # blob host, which is correct: that URL is pre-signed and the + # header would be rejected. + if ! curl -sSL --fail -H "Authorization: Bearer $GITHUB_TOKEN" \ + -H "Accept: application/vnd.github+json" \ + "https://api.github.com/repos/$GITHUB_REPOSITORY/actions/artifacts/$ART/zip" \ + -o "$SUMDIR/$N.zip"; then + echo "::warning::Could not download artifact $ART (shard $N) of run $RUN_ID; skipping this run." + OK=0; break + fi + unzip -qo "$SUMDIR/$N.zip" -d "$SUMDIR/$N" + done + if [ "$OK" -ne 1 ]; then echo "::endgroup::"; continue; fi + + # `find` rather than a fixed-depth glob: `upload-artifact` with + # `path: .turbo/runs/` puts the summaries at the artifact root, but a + # shape assumption here would silently collect NOTHING and hand the + # generator an empty argument list, which reads like a refusal for + # the wrong reason. + find "$SUMDIR" -name '*.json' -type f > "$SUMDIR/summaries.txt" + SUMMARY_COUNT=$(wc -l < "$SUMDIR/summaries.txt") + echo "Collected $SUMMARY_COUNT run summary file(s) from run $RUN_ID." + if [ "$SUMMARY_COUNT" -eq 0 ]; then + echo "::warning::Run $RUN_ID's artifacts contained no run-summary JSON; skipping this run." + echo "::endgroup::"; continue + fi + + ACCEPTED+=("$RUN_ID") + + # The argument list is REBUILT from the accepted set every round + # rather than appended to, so a run the generator refuses can be + # dropped cleanly instead of poisoning every later attempt. Each run + # is fenced by its own `--run `; an array, not `xargs`, because a + # split invocation would run the generator twice and the second would + # overwrite the first's output with a partial dataset. + ARGS=() + for R in "${ACCEPTED[@]}"; do + ARGS+=( --run "$R" ) + mapfile -t RUN_FILES < "$WORK/$R/summaries.txt" + ARGS+=( "${RUN_FILES[@]}" ) + done + + # `--merge-into` the committed dataset, because no retained run set + # measures the whole workspace (see the header). A package this pass + # did not measure keeps its previous weight ONLY when a turbo cache + # HIT witnesses that its inputs are unchanged; anything else is left + # out for the coverage check below to name. The workflow still writes + # no byte the generator did not emit — the merge happens inside it. + if ! node scripts/measure-test-shard-timings.mjs "${ARGS[@]}" \ + --merge-into scripts/test-shard-timings.json \ + --out "$WORK/refreshed.json"; then + echo "::warning::The generator refused the set including run $RUN_ID; dropping that run and continuing." + unset 'ACCEPTED[-1]' + echo "::endgroup::"; continue + fi + + echo "Accumulated ${#ACCEPTED[@]} run(s): ${ACCEPTED[*]}" + if node scripts/ci/select-shard-timings-run.mjs --check-coverage \ + --committed scripts/test-shard-timings.json \ + --refreshed "$WORK/refreshed.json" \ + --workspace "$RUNNER_TEMP/turbo-ls.json" \ + --exclude @objectstack/dogfood; then + CHOSEN="$RUN_ID" + echo "::endgroup::" + break + fi + echo "::endgroup::" + done + + if [ -z "$CHOSEN" ]; then + echo "::error::The ${#ACCEPTED[@]} eligible run(s) on main, accumulated and merged with the committed dataset, still leave a package that HAD a measured weight with neither a fresh measurement nor a turbo cache HIT to witness that it is unchanged — the shortfall above names them. Each would drop to the test-file-count ESTIMATE, which is the silent degradation this lane exists to prevent. NOTHING was regenerated and no PR was opened: this is a refusal, not a quiet success. A package that merely went unmeasured is NOT this error — that case is carried on its cache-hit witness — so a shortfall here means a suite failed, a package was renamed or removed, or its slices could not be assembled in any run." + exit 1 + fi + # Candidates arrive newest-first, so ACCEPTED[0] is the most recent run + # in the set and its commit is the one the refresh is dated from. The + # full list travels alongside it: every run in it contributed + # measurements, and the PR body names them all rather than implying one. + NEWEST="${ACCEPTED[0]}" + echo "run_id=$NEWEST" >> "$GITHUB_OUTPUT" + echo "runs=${ACCEPTED[*]}" >> "$GITHUB_OUTPUT" + echo "run_count=${#ACCEPTED[@]}" >> "$GITHUB_OUTPUT" + echo "head_sha=$(node -e 'const rs=JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP+"/candidates.json","utf8"));console.log(rs.find(r=>String(r.run_id)===process.argv[1]).head_sha)' "$NEWEST")" >> "$GITHUB_OUTPUT" + + # Byte comparison, and it decides everything downstream. `cmp -s` rather + # than a diff of parsed JSON: the committed artefact is the file, so the + # file is what has to differ for a PR to be worth anyone's attention. + - name: Compare against the committed dataset + id: compare + run: | + if cmp -s scripts/test-shard-timings.json "$RUNNER_TEMP/refresh/refreshed.json"; then + echo "changed=false" >> "$GITHUB_OUTPUT" + echo "The regenerated dataset is BYTE-IDENTICAL to the committed one; no PR." | tee -a "$GITHUB_STEP_SUMMARY" + else + echo "changed=true" >> "$GITHUB_OUTPUT" + fi + + # The before/after halves of ruling 3, taken from the partitioner's OWN + # verdict line rather than recomputed here — a second implementation of the + # binning would be a second answer to grade against. + # + # ⛔ The AFTER leg's non-zero exit is CAPTURED, not propagated, and that is + # not leniency: a red there is the partitioner refusing an honest + # measurement, which is information the PR must carry rather than a reason + # to withhold the PR. `set +e` around that one command, with `$?` read + # immediately and before any pipe, is what keeps the verdict readable + # without letting the step's own status swallow it; the text goes into the + # body either way and the PR's own lint job grades it again. + - name: Predicted bins, before and after + id: bins + if: steps.compare.outputs.changed == 'true' + run: | + node scripts/partition-test-shards.mjs --self-test > "$RUNNER_TEMP/bins-before.txt" 2>&1 || true + cp "$RUNNER_TEMP/refresh/refreshed.json" scripts/test-shard-timings.json + set +e + node scripts/partition-test-shards.mjs --self-test > "$RUNNER_TEMP/bins-after.txt" 2>&1 + echo "partitioner_exit=$?" >> "$GITHUB_OUTPUT" + set -e + echo "BEFORE: $(cat "$RUNNER_TEMP/bins-before.txt")" + echo "AFTER: $(cat "$RUNNER_TEMP/bins-after.txt")" + + # Composition is separated from the WRITE on purpose: a `pull_request` run + # of this lane must exercise the body-building — the shard durations, the + # bins, the conditional blocks — without pushing anything. Both the write + # step and the dry-run step below consume this file. + - name: Compose the pull request body + if: steps.compare.outputs.changed == 'true' + env: + RUN_ID: ${{ steps.generate.outputs.run_id }} + RUNS: ${{ steps.generate.outputs.runs }} + RUN_COUNT: ${{ steps.generate.outputs.run_count }} + HEAD_SHA: ${{ steps.generate.outputs.head_sha }} + PARTITIONER_EXIT: ${{ steps.bins.outputs.partitioner_exit }} + USED_PAT: ${{ secrets.RELEASE_PUSH_TOKEN != '' }} + run: | + set -euo pipefail + + # Read out of the generated dataset itself rather than recomputed, so the + # sentence in the PR cannot drift from the file it describes. + CARRY_LINE=$(node -e ' + const d = JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP + "/refresh/refreshed.json", "utf8")); + const carried = d.carriedOver ?? []; + const total = Object.keys(d.packages).length; + const fresh = total - carried.length; + if (carried.length === 0) { + console.log(`All ${total} package weights were measured in these runs; nothing was carried.`); + } else { + console.log( + `${total} package weights: ${fresh} measured in these runs, and ${carried.length} carried ` + + `forward at their previous values because a turbo cache HIT witnessed that their inputs are ` + + `unchanged (so the old number still describes them). Carried: ${carried.join(", ")}.` + ); + } + ') + + SHARDS=$(node -e ' + const rs = JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP + "/candidates.json", "utf8")); + const r = rs.find((x) => String(x.run_id) === process.env.RUN_ID); + const s = r.shard_seconds ?? {}; + console.log([1,2,3,4,5,6].map((n) => (s[n] == null ? "?" : `${n}: ${s[n]}s`)).join(" | ")); + ') + + { + echo "Refreshes \`scripts/test-shard-timings.json\`, the balancing input for the Test Core" + echo "shard split. Opened automatically by \`.github/workflows/shard-timings-refresh.yml\`." + echo "Every byte came out of \`scripts/measure-test-shard-timings.mjs\`; nothing here was" + echo "hand-edited, and no bound, timeout or matrix entry was touched." + echo + echo "## Source" + echo + echo "Measured across $RUN_COUNT accumulated run(s). No single green run measures the whole" + echo "workspace — turbo's cache is namespaced per shard and only main pushes write it, so a" + echo "package whose inputs have not changed is a HIT and the generator refuses hits rather" + echo "than recording a replay as a duration. Runs are therefore accumulated, each fenced by" + echo "its own \`--run\` group, until every package the committed dataset holds is measured" + echo "again; a package seen in several of them gets the median of those observations." + echo + for R in $RUNS; do + echo "- https://github.com/$GITHUB_REPOSITORY/actions/runs/$R" + done + echo + echo "- Newest run in the set: \`$RUN_ID\`, commit \`$HEAD_SHA\` — the date this refresh carries." + echo "- Every run above had all six \`Test Core (N/6)\` jobs conclude \`success\` with its six" + echo " run-summary artifacts still retained; runs that were cancelled, failed or had lost" + echo " their artifacts were rejected by name in the log before any of these were used." + echo + echo "$CARRY_LINE" + echo + echo "⚠️ This PR references #16173 and #16222 but does NOT carry a closing keyword for them," + echo "because a weekly lane cannot know which cards a given run ought to retire. If this is" + echo "the first refresh to land, retire those two by hand as part of merging it." + echo + echo "## Measured per-shard suite time on the newest run in the set" + echo + echo "\`\`\`" + echo "$SHARDS" + echo "\`\`\`" + echo + echo "## Predicted bins, before and after" + echo + echo "\`\`\`" + echo "BEFORE $(cat "$RUNNER_TEMP/bins-before.txt")" + echo "AFTER $(cat "$RUNNER_TEMP/bins-after.txt")" + echo "\`\`\`" + echo + if [ "${PARTITIONER_EXIT:-0}" != "0" ]; then + echo "## The partitioner's own pins RED on this refresh — read this before merging" + echo + echo "This is the designed behaviour, not a defect in the refresh: the acceptance bound is" + echo "a ratio, and a package that has grown past what any six-way split can bin makes the" + echo "pins fail with the arithmetic in the message. The remedy the partitioner names is to" + echo "raise the file-level slice count for that package — ⛔ never to raise the bound, and" + echo "⛔ never to hand-edit this dataset. This workflow deliberately does neither: it" + echo "reports and stops, because both are decisions." + echo + fi + if [ "$USED_PAT" != "true" ]; then + echo "## No checks will start on this PR by themselves" + echo + echo "It was opened with the Actions \`GITHUB_TOKEN\`, and GitHub's recursion guard means a" + echo "PR opened that way triggers no workflow runs. Push any commit to the branch, or close" + echo "and reopen the PR, to start CI." + echo + fi + # ⛔ No closing keyword is emitted here, in any form. GitHub's + # parser matches `fixes`/`closes`/`resolves` plus a number and + # ignores every negation around them, so even a sentence saying a + # card is NOT closed would close it. This PR is the workflow's + # OUTPUT; the cards about the workflow are referenced only. + echo "Refs #16464, #16173, #16222." + } > "$RUNNER_TEMP/pr-body.md" + echo "Composed a $(wc -l < "$RUNNER_TEMP/pr-body.md")-line pull request body." + + # The WRITE. Skipped on a `pull_request` run of this lane: a PR that only + # edits this workflow must never push a bot branch or open a second PR. + - name: Push the refresh branch and open the pull request + if: steps.compare.outputs.changed == 'true' && github.event_name != 'pull_request' + env: + GH_TOKEN: ${{ secrets.RELEASE_PUSH_TOKEN || github.token }} + RUN_ID: ${{ steps.generate.outputs.run_id }} + HEAD_SHA: ${{ steps.generate.outputs.head_sha }} + run: | + set -euo pipefail + BRANCH="claude/shard-timings-refresh-$RUN_ID" + git config user.name 'github-actions[bot]' + git config user.email '41898282+github-actions[bot]@users.noreply.github.com' + git switch -c "$BRANCH" + git add scripts/test-shard-timings.json + # Separate -m flags rather than one embedded newline: a YAML-indented + # heredoc would carry its own leading whitespace into the message body. + git commit \ + -m "chore(ci): refresh the Test Core shard-timings dataset" \ + -m "Regenerated by .github/workflows/shard-timings-refresh.yml from the test-core-run-summary artifacts of $RUN_COUNT accumulated run(s) ($RUNS), newest $RUN_ID at $HEAD_SHA. Generated, never hand-edited." + git push origin "$BRANCH" + + PR_URL=$(gh pr create --base main --head "$BRANCH" \ + --title "chore(ci): refresh the Test Core shard-timings dataset" \ + --body-file "$RUNNER_TEMP/pr-body.md") + echo "Opened $PR_URL" | tee -a "$GITHUB_STEP_SUMMARY" + + # ADDITIVE label write only. A whole-set PUT replaces the PR's labels + # and destroys any that land in between — measured on this repo, one + # second wide (see pr-automation.yml's header). POST names only what it + # adds, so no interleaving can lose another writer's label. + PR_NUMBER=$(gh pr view "$PR_URL" --json number --jq .number) + gh api --method POST "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/labels" \ + -f "labels[]=skip-changeset" > /dev/null + + # Read back, because an additive write is necessary and not sufficient: + # a concurrent whole-set PUT from another workflow can still strip the + # label after a successful POST. `skip-changeset` is this PR's exemption + # from the changeset gate — it publishes nothing — so losing it turns + # the gate red on a PR that legitimately has no changeset. + if gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/labels" --jq '.[].name' \ + | grep -qxF 'skip-changeset'; then + echo "skip-changeset confirmed on PR #$PR_NUMBER." + else + echo "::warning::skip-changeset did not survive the write on PR #$PR_NUMBER (a concurrent whole-set label PUT strips it). Re-applying once." + gh api --method POST "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/labels" \ + -f "labels[]=skip-changeset" > /dev/null + gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/labels" --jq '.[].name' \ + | grep -qxF 'skip-changeset' \ + || echo "::error::skip-changeset is still absent from PR #$PR_NUMBER; the changeset gate will demand a changeset this PR legitimately has none of. Apply the label by hand." + fi + + # The dry-run half of the `pull_request` posture. Everything above ran for + # real; this renders the body that WOULD have been posted, so a reviewer of + # a change to this lane sees the actual output rather than the diff of the + # code that produces it. + - name: Dry run — the pull request this would have opened + if: steps.compare.outputs.changed == 'true' && github.event_name == 'pull_request' + run: | + { + echo "### Shard timings: dry run (no branch pushed, no PR opened, no label written)" + echo + echo "This is a \`pull_request\` run of the refresh lane itself. The run selection, the" + echo "artifact download, the regeneration, the coverage check and the partitioner's verdict" + echo "all executed for real; only the write was skipped. The body below is what a scheduled" + echo "run would have posted." + echo + echo "---" + echo + cat "$RUNNER_TEMP/pr-body.md" + } >> "$GITHUB_STEP_SUMMARY" + + - name: Say what happened when nothing changed + if: steps.compare.outputs.changed == 'false' + run: | + { + echo "### Shard timings: byte-identical, no PR" + echo + echo "Run \`${{ steps.generate.outputs.run_id }}\` (\`${{ steps.generate.outputs.head_sha }}\`)" + echo "regenerated \`scripts/test-shard-timings.json\` byte-for-byte identically to the" + echo "committed file, so there is nothing to open a pull request about. The dataset is" + echo "current, and this is the loop working rather than the loop skipping." + } >> "$GITHUB_STEP_SUMMARY" diff --git a/scripts/check-self-test-wired.mjs b/scripts/check-self-test-wired.mjs index cfca4ddcb0..db95b1025a 100644 --- a/scripts/check-self-test-wired.mjs +++ b/scripts/check-self-test-wired.mjs @@ -193,9 +193,25 @@ const ROOT_DIR_WATCH_HINTS = ['scripts/**/*.mjs', 'scripts/**/*.mts', 'scripts/* * Measured after this change over the live corpus: 186 named paths, ZERO of * which lack a file on disk (before: exactly one, the phantom above). The * `live corpus` battery holds both halves of that reading. + * + * ## The RIGHT boundary, and the phantom it mints without one (#16464) + * + * The extension alternation needs the same treatment the left edge got, for the + * same reason and with the same failure shape. `js` is a PREFIX of `json`, so + * without a right boundary a workflow line naming `scripts/test-shard- + * timings.json` -- a data file, not a script -- matched as far as + * `scripts/test-shard-timings.js` and minted exactly the phantom key the left + * boundary was added to abolish: a path with no file behind it, silent in both + * directions. + * + * It stayed invisible only because no workflow had yet named a `.json` under + * `scripts/`; the first one to do it (the shard-timings refresh) reddened the + * `live corpus` battery. `(?![\w-])` is the spelling already used on + * `--self-test` two groups along, so both boundaries on this pattern now read + * the same way: a match must end where the token ends. */ const INVOCATION_RE = - /(?] +// node scripts/ci/select-shard-timings-run.mjs --check-coverage \ +// --committed --refreshed --workspace \ +// [--exclude ]... +// node scripts/ci/select-shard-timings-run.mjs --self-test +// +// `--candidates` needs GITHUB_TOKEN and GITHUB_REPOSITORY in the environment and +// prints a JSON array, newest first. `--check-coverage` judges MEASURED UNION +// CARRIED against the workspace and exits non-zero when a package that HAD a +// measured weight has neither -- it names workspace packages that were never +// measured too, but those are a report rather than a refusal, because the +// partitioner already estimated them and this refresh did not change that. + +import { readFileSync } from 'node:fs'; +import process from 'node:process'; + +import { isEntrypoint } from '../invoked-as.mjs'; + +// The Test Core shard count. Spelled here as the number of jobs and artifacts a +// complete run must present; partition-test-shards.mjs owns the split itself and +// its own self-test reads ci.yml back, so this is a reader's expectation rather +// than a second declaration of the split. +export const SHARD_COUNT = 6; + +const API = 'https://api.github.com'; + +// `Test Core (3/6)` -> 3. Anchored on both ends: a job merely CONTAINING that +// text (a future "Test Core (3/6) rerun") is not this job, and reading it as one +// would let a run qualify on a job that never ran the suite. +export function testCoreShard(jobName, shardCount = SHARD_COUNT) { + const m = /^Test Core \((\d+)\/(\d+)\)$/.exec(String(jobName ?? '').trim()); + if (!m) return null; + if (Number(m[2]) !== shardCount) return null; + const index = Number(m[1]); + return index >= 1 && index <= shardCount ? index : null; +} + +// `test-core-run-summary-3-of-6` -> 3, by the same anchoring argument. +export function summaryArtifactShard(artifactName, shardCount = SHARD_COUNT) { + const m = /^test-core-run-summary-(\d+)-of-(\d+)$/.exec(String(artifactName ?? '').trim()); + if (!m) return null; + if (Number(m[2]) !== shardCount) return null; + const index = Number(m[1]); + return index >= 1 && index <= shardCount ? index : null; +} + +// How long a shard's suite actually ran, from the job's own step window. This +// is the tighter reading than job wall-clock -- setup, install and cache restore +// are not the suite -- and it is what the refresh PR quotes as "per-shard +// durations" so a reviewer can compare measured against predicted without +// opening the run. +export const TEST_STEP_NAME = "Run this shard's tests"; +export function testStepSeconds(job, stepName = TEST_STEP_NAME) { + const step = (job?.steps ?? []).find((s) => s?.name === stepName); + if (!step?.started_at || !step?.completed_at) return null; + const seconds = (Date.parse(step.completed_at) - Date.parse(step.started_at)) / 1000; + return Number.isFinite(seconds) && seconds >= 0 ? seconds : null; +} + +// Is this run a complete, uncensored measurement? Both halves are required and +// they fail for different reasons, so both are named separately. +// +// ⛔ `conclusion === 'success'` is the test, NOT `status === 'completed'`: a +// cancelled job is completed too, and a cancelled Test Core shard is exactly the +// censored run this refuses. A job that is still running is likewise not a +// success -- it may yet fail -- so anything other than the literal string is a +// rejection. +export function runIsEligible({ jobs, artifacts }, shardCount = SHARD_COUNT) { + const reasons = []; + + const byShard = new Map(); + for (const job of jobs ?? []) { + const shard = testCoreShard(job?.name, shardCount); + if (shard === null) continue; + // A re-run leaves several job records for one shard; the run qualifies if + // ANY attempt of that shard concluded success, which is the same rule the + // artifacts follow (the successful attempt is the one that uploaded). + if (job?.conclusion === 'success') byShard.set(shard, job); + } + const missingJobs = []; + for (let i = 1; i <= shardCount; i++) if (!byShard.has(i)) missingJobs.push(`${i}/${shardCount}`); + if (missingJobs.length > 0) { + reasons.push(`Test Core ${missingJobs.join(', ')} did not conclude success (cancelled, failed or never ran)`); + } + + const artifactByShard = new Map(); + for (const artifact of artifacts ?? []) { + const shard = summaryArtifactShard(artifact?.name, shardCount); + if (shard === null) continue; + // An expired artifact is a 410 at download time, so it is not an input. + if (artifact?.expired === true) continue; + if (!artifactByShard.has(shard)) artifactByShard.set(shard, artifact.id); + } + const missingArtifacts = []; + for (let i = 1; i <= shardCount; i++) if (!artifactByShard.has(i)) missingArtifacts.push(`${i}-of-${shardCount}`); + if (missingArtifacts.length > 0) { + reasons.push(`run summary artifact(s) ${missingArtifacts.join(', ')} are missing or expired`); + } + + const shardSeconds = {}; + for (const [shard, job] of [...byShard.entries()].sort((a, b) => a[0] - b[0])) { + shardSeconds[shard] = testStepSeconds(job); + } + + return { + eligible: reasons.length === 0, + reasons, + artifactIds: Object.fromEntries([...artifactByShard.entries()].sort((a, b) => a[0] - b[0])), + shardSeconds, + }; +} + +// The half that catches a cache replay, an incomplete slice set, or any other +// reason a run "succeeded" without measuring much (#16464 hazard 2 above). +// +// The judgement is deliberately about MEASUREMENT and not about duration: a +// package the committed dataset measured, that the workspace still contains, and +// that the refreshed dataset does NOT measure, has silently been demoted to the +// test-file-count estimate. That is the direction nobody notices, so it is the +// direction this refuses in. Comparing against the live workspace is what keeps +// a package legitimately deleted from the monorepo from blocking every future +// refresh -- it is gone from `workspace`, so it is not required. +// Coverage is judged on MEASURED UNION CARRIED, which after a `--merge-into` +// pass is simply the refreshed dataset's own key set: the generator has already +// folded in every package a cache HIT witnessed as unchanged, and refused to +// fold in anything else. Two outcomes are reported separately because they are +// different facts and only one of them is a regression: +// +// `lost` — the package HAD a measured weight, this refresh neither measured +// it nor found a cache HIT to witness it, so it would drop to a +// test-file-count ESTIMATE. That is the silent degradation this +// whole lane exists to prevent, so it is a REFUSAL, by name. +// +// `neverMeasured` — the package is in the workspace and has no measured +// weight before OR after: a new package, or one the dataset has +// never covered. The partitioner already estimates it from its +// test-file count and this refresh changed nothing about it, so it +// cannot be a regression — but it is NAMED rather than passed over +// in silence, because "estimated" must never be something a reader +// has to infer from an absence. +export function coverageReport({ committed, refreshed, workspace, exclude = [] }) { + const excluded = new Set(exclude); + const inWorkspace = new Set(workspace); + const priorPackages = committed?.packages ?? {}; + const covered = new Set(Object.keys(refreshed?.packages ?? {})); + const carried = new Set(refreshed?.carriedOver ?? []); + + const required = Object.keys(priorPackages).filter( + (name) => inWorkspace.has(name) && !excluded.has(name) + ); + const lost = required.filter((name) => !covered.has(name)).sort((a, b) => a.localeCompare(b, 'en')); + + const neverMeasured = [...inWorkspace] + .filter((name) => !excluded.has(name) && !covered.has(name) && !Object.hasOwn(priorPackages, name)) + .sort((a, b) => a.localeCompare(b, 'en')); + + const gained = [...covered] + .filter((name) => !Object.hasOwn(priorPackages, name)) + .sort((a, b) => a.localeCompare(b, 'en')); + + return { + ok: lost.length === 0, + lost, + neverMeasured, + gained, + carriedCount: carried.size, + freshCount: covered.size - carried.size, + requiredCount: required.length, + measuredCount: covered.size, + }; +} + +// `turbo ls --output=json` -> package names. The payload shape is asserted +// loudly for the same reason partition-test-shards.mjs asserts it: `turbo ls` is +// marked experimental, and a silently-empty package list here would make every +// package look deleted and every coverage check pass. +export function workspaceNames(parsed) { + const items = parsed?.packages?.items; + if (!Array.isArray(items)) { + throw new Error( + 'workspace: expected `turbo ls --output=json` output with a {packages:{items:[...]}} array' + ); + } + const names = items.map((item) => item?.name).filter((n) => typeof n === 'string' && n.length > 0); + if (names.length === 0) throw new Error('workspace: `turbo ls` listed no packages'); + return names; +} + +// --------------------------------------------------------------------------- +// The network half. Thin on purpose: every judgement above is a pure function +// with a self-test, and what is left here is paging and shape-reading. +// --------------------------------------------------------------------------- + +async function api(pathname, { token, fetchImpl = fetch }) { + const response = await fetchImpl(`${API}${pathname}`, { + headers: { + accept: 'application/vnd.github+json', + authorization: `Bearer ${token}`, + 'x-github-api-version': '2022-11-28', + }, + }); + if (!response.ok) { + throw new Error(`GET ${pathname} -> HTTP ${response.status} ${response.statusText}`); + } + return response.json(); +} + +// Newest first, bounded. `limit` is how many runs are EXAMINED, not how many +// come back: the answer is often the fourth or fifth run, and stopping at the +// first eligible one would hide the fact that the newest four were replays. +export async function listCandidates({ + repo, + token, + workflow = 'ci.yml', + shardCount = SHARD_COUNT, + limit = 12, + fetchImpl = fetch, +} = {}) { + const runs = await api( + `/repos/${repo}/actions/workflows/${workflow}/runs` + + `?event=push&branch=main&status=completed&per_page=${limit}`, + { token, fetchImpl } + ); + const examined = []; + for (const run of runs?.workflow_runs ?? []) { + const [jobsPayload, artifactsPayload] = await Promise.all([ + api(`/repos/${repo}/actions/runs/${run.id}/jobs?per_page=100`, { token, fetchImpl }), + api(`/repos/${repo}/actions/runs/${run.id}/artifacts?per_page=100`, { token, fetchImpl }), + ]); + const verdict = runIsEligible( + { jobs: jobsPayload?.jobs, artifacts: artifactsPayload?.artifacts }, + shardCount + ); + examined.push({ + run_id: run.id, + head_sha: run.head_sha, + created_at: run.created_at, + html_url: run.html_url, + eligible: verdict.eligible, + reasons: verdict.reasons, + artifact_ids: verdict.artifactIds, + shard_seconds: verdict.shardSeconds, + }); + } + return examined; +} + +// --------------------------------------------------------------------------- +// -- The self-test's own battery roster and floor --------------------------- +// +// Same shape as the two scripts this one serves: what is pinned is the +// registered NAMES, and the count is a FLOOR -- a battery below it means cases +// stopped running, and the remedy is to find what stopped registering, never to +// lower the number. +const SELF_TEST_BATTERIES = Object.freeze({ + 'select-shard-timings-run self-test': 35, +}); +const SELF_TEST_BATTERY_FLOOR = 1; +const UNATTRIBUTED_BATTERY = '(no battery open)'; + +// Returned by `selfTest()` only after its verdict is printed, so a `return` that +// leaves the function early cannot report as a pass. +const SELF_TEST_VERDICT = 'select-shard-timings-run self-test reached its verdict'; + +function selfTest() { + const batterySeen = new Map(); + let openBattery = null; + const battery = (name) => { + openBattery = name; + }; + const check = (fn) => { + const b = openBattery ?? UNATTRIBUTED_BATTERY; + batterySeen.set(b, (batterySeen.get(b) ?? 0) + 1); + fn(); + }; + const threw = (fn) => { + try { + fn(); + return false; + } catch { + return true; + } + }; + + battery('select-shard-timings-run self-test'); + + // -- The two name matchers. Both are anchored, and the pins that matter are + // the NEAR MISSES: a matcher that also accepts a rerun job or a + // differently-sharded artifact would qualify a run on evidence from a run + // that is not this one. + check(() => { + if (testCoreShard('Test Core (3/6)') !== 3) throw new Error('job matcher: the plain shape did not parse'); + }); + check(() => { + if (testCoreShard('Test Core (1/6)') !== 1 || testCoreShard('Test Core (6/6)') !== 6) { + throw new Error('job matcher: an endpoint shard did not parse'); + } + }); + check(() => { + if (testCoreShard('Test Core (0/6)') !== null || testCoreShard('Test Core (7/6)') !== null) { + throw new Error('job matcher: an out-of-range shard index was accepted'); + } + }); + check(() => { + if (testCoreShard('Test Core (3/6) rerun') !== null) { + throw new Error('job matcher: a job merely CONTAINING the shape was accepted'); + } + }); + check(() => { + if (testCoreShard('Dogfood (3/6)') !== null) throw new Error('job matcher: a different job was accepted'); + }); + check(() => { + if (testCoreShard('Test Core (3/8)') !== null) { + throw new Error('job matcher: a job from a different shard count was accepted'); + } + }); + check(() => { + if (testCoreShard(undefined) !== null || testCoreShard('') !== null) { + throw new Error('job matcher: a missing name was not rejected'); + } + }); + check(() => { + if (summaryArtifactShard('test-core-run-summary-4-of-6') !== 4) { + throw new Error('artifact matcher: the plain shape did not parse'); + } + }); + check(() => { + if (summaryArtifactShard('test-core-run-summary-4-of-8') !== null) { + throw new Error('artifact matcher: a different shard count was accepted'); + } + }); + check(() => { + if (summaryArtifactShard('test-core-run-summary-4-of-6-retry') !== null) { + throw new Error('artifact matcher: a longer name was accepted'); + } + }); + + // -- Eligibility. The fixtures are built from the two rejection shapes this + // workflow actually meets: a cancelled shard, and an expired artifact. + const jobs = (conclusions) => + conclusions.map((conclusion, i) => ({ name: `Test Core (${i + 1}/6)`, conclusion, status: 'completed' })); + const artifacts = (present, expired = []) => + present.map((n) => ({ id: 1000 + n, name: `test-core-run-summary-${n}-of-6`, expired: expired.includes(n) })); + const allSix = ['success', 'success', 'success', 'success', 'success', 'success']; + + check(() => { + const v = runIsEligible({ jobs: jobs(allSix), artifacts: artifacts([1, 2, 3, 4, 5, 6]) }); + if (!v.eligible) throw new Error(`eligibility: a complete run was rejected (${v.reasons.join('; ')})`); + }); + check(() => { + const v = runIsEligible({ jobs: jobs(allSix), artifacts: artifacts([1, 2, 3, 4, 5, 6]) }); + if (v.artifactIds['5'] !== 1005) throw new Error(`eligibility: artifact ids were not returned per shard (${JSON.stringify(v.artifactIds)})`); + }); + check(() => { + const cancelled = [...allSix]; + cancelled[2] = 'cancelled'; + const v = runIsEligible({ jobs: jobs(cancelled), artifacts: artifacts([1, 2, 3, 4, 5, 6]) }); + if (v.eligible) throw new Error('eligibility: a run with a CANCELLED shard was accepted'); + if (!v.reasons.some((r) => r.includes('3/6'))) throw new Error(`eligibility: the cancelled shard was not named (${v.reasons.join('; ')})`); + }); + check(() => { + // A cancelled shard that still uploaded its artifact is the exact censored + // shape: "the artifacts exist" must not be enough on its own. + const cancelled = [...allSix]; + cancelled[0] = 'cancelled'; + const v = runIsEligible({ jobs: jobs(cancelled), artifacts: artifacts([1, 2, 3, 4, 5, 6]) }); + if (v.eligible) throw new Error('eligibility: a censored run passed because its artifacts were complete'); + }); + check(() => { + const stillRunning = [...allSix]; + stillRunning[4] = null; + const v = runIsEligible({ jobs: jobs(stillRunning), artifacts: artifacts([1, 2, 3, 4, 5, 6]) }); + if (v.eligible) throw new Error('eligibility: a shard with no conclusion was read as a success'); + }); + check(() => { + const v = runIsEligible({ jobs: jobs(allSix), artifacts: artifacts([1, 2, 3, 4, 5]) }); + if (v.eligible) throw new Error('eligibility: a run missing an artifact was accepted'); + if (!v.reasons.some((r) => r.includes('6-of-6'))) throw new Error(`eligibility: the missing artifact was not named (${v.reasons.join('; ')})`); + }); + check(() => { + const v = runIsEligible({ jobs: jobs(allSix), artifacts: artifacts([1, 2, 3, 4, 5, 6], [2]) }); + if (v.eligible) throw new Error('eligibility: an EXPIRED artifact was counted as retained'); + }); + check(() => { + // A re-run leaves a failed attempt beside the successful one. The shard + // qualifies on the attempt that succeeded, which is also the one that + // uploaded the artifact. + const withRetry = [...jobs(allSix), { name: 'Test Core (2/6)', conclusion: 'failure', status: 'completed' }]; + const v = runIsEligible({ jobs: withRetry, artifacts: artifacts([1, 2, 3, 4, 5, 6]) }); + if (!v.eligible) throw new Error(`eligibility: a re-run's earlier failed attempt disqualified the shard (${v.reasons.join('; ')})`); + }); + + // -- The per-shard duration the PR body quotes. Read from the suite step's + // own window, and ABSENT rather than zero when the step never ran -- a + // missing duration reported as 0s would read as an instant suite, which is + // the cache-replay shape this file exists to catch. + check(() => { + const job = { + steps: [ + { name: 'Install dependencies', started_at: '2026-09-07T00:00:00Z', completed_at: '2026-09-07T00:00:30Z' }, + { name: TEST_STEP_NAME, started_at: '2026-09-07T00:01:00Z', completed_at: '2026-09-07T00:10:20Z' }, + ], + }; + if (testStepSeconds(job) !== 560) throw new Error(`step window: got ${testStepSeconds(job)}, expected 560`); + }); + check(() => { + if (testStepSeconds({ steps: [{ name: 'Something else', started_at: 'x', completed_at: 'y' }] }) !== null) { + throw new Error('step window: a different step was measured as the suite'); + } + }); + check(() => { + if (testStepSeconds({ steps: [{ name: TEST_STEP_NAME, started_at: '2026-09-07T00:00:00Z' }] }) !== null) { + throw new Error('step window: an unfinished step was reported as a duration'); + } + }); + check(() => { + if (testStepSeconds({}) !== null || testStepSeconds(undefined) !== null) { + throw new Error('step window: a job with no steps was not rejected'); + } + }); + + // -- Coverage, the cache-replay catcher. The control leg first: an identical + // package set must pass, so the rejection below is about the LOSS and not + // about the comparison being broken. + const committed = { packages: { a: 10, b: 20, c: 30 } }; + const ws = ['a', 'b', 'c']; + check(() => { + const r = coverageReport({ committed, refreshed: { packages: { a: 11, b: 21, c: 31 } }, workspace: ws }); + if (!r.ok) throw new Error(`coverage: a complete refresh was rejected (${r.lost.join(', ')})`); + }); + check(() => { + const r = coverageReport({ committed, refreshed: { packages: { a: 11 } }, workspace: ws }); + if (r.ok) throw new Error('coverage: a refresh that lost two thirds of the workspace was accepted'); + if (r.lost.join(',') !== 'b,c') throw new Error(`coverage: the lost packages were not named (${r.lost.join(',')})`); + }); + check(() => { + // A package deleted from the monorepo is NOT a loss -- otherwise one + // deletion blocks every refresh from then on. + const r = coverageReport({ committed, refreshed: { packages: { a: 11, b: 21 } }, workspace: ['a', 'b'] }); + if (!r.ok) throw new Error(`coverage: a package removed from the workspace was counted as lost (${r.lost.join(',')})`); + }); + check(() => { + // ci.yml excludes dogfood from Test Core, so it is absent by construction. + const r = coverageReport({ + committed: { packages: { a: 10, '@objectstack/dogfood': 99 } }, + refreshed: { packages: { a: 11 } }, + workspace: ['a', 'ptsc/dogfood'.replace('ptsc/', '@objectstack/')], + exclude: ['@objectstack/dogfood'], + }); + if (!r.ok) throw new Error(`coverage: an excluded package was required (${r.lost.join(',')})`); + }); + check(() => { + const r = coverageReport({ committed, refreshed: { packages: { a: 1, b: 2, c: 3, d: 4 } }, workspace: [...ws, 'd'] }); + if (!r.ok || r.gained.join(',') !== 'd') throw new Error(`coverage: a newly measured package was not reported (${r.gained.join(',')})`); + }); + + // -- Coverage under the MERGE (#16464). After a `--merge-into` pass the + // refreshed dataset already holds the carried weights, so coverage is + // judged on measured UNION carried; what the cases below separate is the + // two ways a package can be missing, because only one of them is a + // regression. + check(() => { + // A carried package COUNTS as covered — it has a real weight, witnessed + // unchanged by a cache hit — so a refresh that measured only `a` and + // carried `b` and `c` is complete, not short. + const r = coverageReport({ + committed, + refreshed: { packages: { a: 11, b: 20, c: 30 }, carriedOver: ['b', 'c'] }, + workspace: ws, + }); + if (!r.ok) throw new Error(`coverage: carried packages were not counted as covered (${r.lost.join(', ')})`); + if (r.carriedCount !== 2 || r.freshCount !== 1) { + throw new Error(`coverage: the carried/fresh split is wrong (carried ${r.carriedCount}, fresh ${r.freshCount})`); + } + }); + check(() => { + // The regression that still refuses: `c` had a weight and is in NEITHER set. + const r = coverageReport({ + committed, + refreshed: { packages: { a: 11, b: 20 }, carriedOver: ['b'] }, + workspace: ws, + }); + if (r.ok) throw new Error('coverage: a package that lost its measured weight was accepted'); + if (r.lost.join(',') !== 'c') throw new Error(`coverage: the lost package was not named (${r.lost.join(',')})`); + }); + check(() => { + // A workspace package that NEVER had a weight is named but is not a + // refusal: the partitioner already estimated it and this refresh changed + // nothing about it. + const r = coverageReport({ + committed, + refreshed: { packages: { a: 11, b: 20, c: 30 }, carriedOver: [] }, + workspace: [...ws, 'brand-new'], + }); + if (!r.ok) throw new Error(`coverage: a never-measured package was treated as a regression (${r.lost.join(',')})`); + if (r.neverMeasured.join(',') !== 'brand-new') { + throw new Error(`coverage: the never-measured package was not named (${r.neverMeasured.join(',')})`); + } + }); + check(() => { + // …and it is not confused with a carried one. + const r = coverageReport({ + committed, + refreshed: { packages: { a: 11, b: 20, c: 30 }, carriedOver: ['c'] }, + workspace: [...ws, 'brand-new'], + }); + if (r.neverMeasured.includes('c') || r.carriedCount !== 1) { + throw new Error(`coverage: carried and never-measured were conflated (never ${r.neverMeasured.join(',')}, carried ${r.carriedCount})`); + } + }); + check(() => { + // A dataset with no carriedOver key at all (a plain replace) still reads. + const r = coverageReport({ committed, refreshed: { packages: { a: 1, b: 2, c: 3 } }, workspace: ws }); + if (!r.ok || r.carriedCount !== 0) throw new Error('coverage: a dataset without carriedOver was misread'); + }); + + // -- The workspace reader refuses a shape it cannot trust, rather than + // returning an empty list that would make every package look deleted. + check(() => { + if (workspaceNames({ packages: { items: [{ name: 'a' }, { name: 'b' }] } }).join(',') !== 'a,b') { + throw new Error('workspace: a valid turbo ls payload did not parse'); + } + }); + check(() => { + if (!threw(() => workspaceNames({ packages: {} }))) throw new Error('workspace: a payload with no items array was accepted'); + }); + check(() => { + if (!threw(() => workspaceNames({ packages: { items: [] } }))) throw new Error('workspace: an EMPTY package list was accepted'); + }); + + // -- The floor: every declared battery ran, and ran its cases. Evaluated + // before the verdict, so the success line can only be printed by a run in + // which the set of batteries that registered EQUALS the set declared. + const floorFailures = []; + const declared = Object.keys(SELF_TEST_BATTERIES); + if (declared.length < SELF_TEST_BATTERY_FLOOR) { + floorFailures.push( + `SELF_TEST_BATTERIES declares ${declared.length} batteries, below the pinned ${SELF_TEST_BATTERY_FLOOR}.` + ); + } + for (const [name, count] of batterySeen) { + if (declared.includes(name)) continue; + floorFailures.push(`battery "${name}" registered ${count} case(s) but is not declared.`); + } + for (const name of declared) { + const count = batterySeen.get(name) ?? 0; + if (count >= SELF_TEST_BATTERIES[name]) continue; + floorFailures.push( + count === 0 + ? `battery "${name}" DID NOT RUN -- 0 cases registered, ${SELF_TEST_BATTERIES[name]} pinned.` + : `battery "${name}" registered ${count} case(s), below its pinned floor of ${SELF_TEST_BATTERIES[name]}.` + ); + } + if (floorFailures.length > 0) { + throw new Error( + `select-shard-timings-run self-test floor (${floorFailures.length} breach(es)):\n` + + floorFailures.map((f) => ` - ${f}`).join('\n') + + '\n A battery at or below its floor means cases STOPPED RUNNING -- the battery is the bug, ' + + 'not the number.' + ); + } + + console.log('select-shard-timings-run: self-test OK'); + return SELF_TEST_VERDICT; +} + +function readJson(file) { + return JSON.parse(readFileSync(file, 'utf8')); +} + +async function main() { + const argv = process.argv.slice(2); + + if (argv.includes('--self-test')) { + if (selfTest() !== SELF_TEST_VERDICT) { + console.error( + '\nx select-shard-timings-run self-test: selfTest() returned without reaching its verdict,\n' + + 'so no success line was printed. Exiting 0 here would report a self-test that never\n' + + 'finished as a self-test that passed.\n' + ); + process.exit(1); + } + return; + } + + if (argv.includes('--check-coverage')) { + const value = (flag) => { + const i = argv.indexOf(flag); + return i === -1 ? null : argv[i + 1]; + }; + const exclude = []; + for (let i = 0; i < argv.length; i++) if (argv[i] === '--exclude') exclude.push(argv[i + 1]); + const committed = readJson(value('--committed')); + const refreshed = readJson(value('--refreshed')); + const workspace = workspaceNames(readJson(value('--workspace'))); + const report = coverageReport({ committed, refreshed, workspace, exclude }); + // Named whichever way the verdict goes: a package the partitioner estimates + // must never be something a reader infers from an absence. + if (report.neverMeasured.length > 0) { + console.error( + `select-shard-timings-run: ${report.neverMeasured.length} workspace package(s) have no measured ` + + `weight before or after this refresh and are ESTIMATED by the partitioner from their ` + + `test-file count: ${report.neverMeasured.join(', ')}. Not a regression — this refresh did not ` + + 'change their standing — but they are named rather than passed over, because an estimate that ' + + 'reads as a measurement is this dataset\'s signature hazard.' + ); + } + if (!report.ok) { + console.error( + `select-shard-timings-run: COVERAGE SHORTFALL -- ${report.lost.length} package(s) HAD a measured ` + + 'weight and this refresh neither re-measured them nor found a turbo cache HIT to witness that ' + + `they are unchanged: ${report.lost.join(', ')}. Each would drop to the test-file-count ` + + 'ESTIMATE, which is the silent degradation this lane exists to prevent, so this is a refusal. ' + + 'Not a verdict on any one run: no single run measures everything, so the caller adds the next ' + + 'older run and asks again.' + ); + process.exit(1); + } + console.error( + `select-shard-timings-run: coverage OK -- ${report.measuredCount} package(s) covered ` + + `(${report.freshCount} measured in these runs, ${report.carriedCount} carried on a cache-hit ` + + `witness), ${report.requiredCount} required, ${report.gained.length} newly measured` + + `${report.gained.length > 0 ? ` (${report.gained.join(', ')})` : ''}.` + ); + return; + } + + if (argv.includes('--candidates')) { + const token = process.env.GITHUB_TOKEN; + const repo = process.env.GITHUB_REPOSITORY; + if (!token || !repo) throw new Error('--candidates needs GITHUB_TOKEN and GITHUB_REPOSITORY in the environment'); + const limitAt = argv.indexOf('--limit'); + const limit = limitAt === -1 ? 12 : Number(argv[limitAt + 1]); + const examined = await listCandidates({ repo, token, limit }); + for (const run of examined) { + console.error( + run.eligible + ? ` ELIGIBLE ${run.run_id} ${run.created_at} ${String(run.head_sha).slice(0, 10)}` + : ` rejected ${run.run_id} ${run.created_at} ${run.reasons.join('; ')}` + ); + } + const eligible = examined.filter((r) => r.eligible); + if (eligible.length === 0) { + console.error( + `select-shard-timings-run: NO ELIGIBLE RUN among the ${examined.length} most recent completed push ` + + 'runs on main. Every one was censored, failed, or has lost its run-summary artifacts to the ' + + '1-day retention window. This is a refusal, not a no-op: nothing was regenerated.' + ); + process.exit(1); + } + console.log(JSON.stringify(eligible, null, 2)); + return; + } + + console.error( + 'usage: select-shard-timings-run.mjs --candidates [--limit ]\n' + + ' select-shard-timings-run.mjs --check-coverage --committed --refreshed --workspace [--exclude ]...\n' + + ' select-shard-timings-run.mjs --self-test' + ); + process.exit(1); +} + +// Exports bindings, so an import for those exports alone must run nothing. +if (isEntrypoint(import.meta.url)) { + await main(); +} diff --git a/scripts/measure-test-shard-timings.mjs b/scripts/measure-test-shard-timings.mjs index 2867931ed7..61b1b71827 100644 --- a/scripts/measure-test-shard-timings.mjs +++ b/scripts/measure-test-shard-timings.mjs @@ -27,11 +27,38 @@ // // From CI, no special run needed. Every Test Core shard passes --summarize // and, on merge_group builds, uploads `.turbo/runs/` as the -// `test-core-run-summary--of-6` artifact. Download all six from any -// green queue build and: -// node scripts/measure-test-shard-timings.mjs /*.json \ +// `test-core-run-summary--of-6` artifact. +// +// ⚠ ONE RUN IS NOT ENOUGH, AND THIS SENTENCE USED TO SAY IT WAS. It read +// "download all six from any green queue build and re-run the generator", +// which is optimistic in a way nobody had measured until the refresh lane ran +// for real: a single green run yields between 2 and 52 of the ~71 measurable +// packages, depending on nothing but how warm that run's turbo cache was, and +// the union of every retained green run converges around 57. The rest are +// cache HITs, which this file refuses (see the CACHED TASKS rule below). So +// the honest procedure is: +// +// node scripts/measure-test-shard-timings.mjs \ +// --run /*.json \ +// --run /*.json \ +// --merge-into scripts/test-shard-timings.json \ // --out scripts/test-shard-timings.json // +// `--run` fences each run: slices are summed within a run and the per-run sums +// medianed across runs, so a file-sharded package gets the same median +// treatment as every other package (#16473); undeclared multi-run input is +// REFUSED rather than resolved by guessing. +// +// `--merge-into` is what makes a partial measurement sound. A package this +// pass did not measure keeps its previous weight -- but ONLY when a cache HIT +// witnesses that its inputs are unchanged, which is the evidence that the old +// number still describes it. Those packages are named in `carriedOver`. +// Anything absent for any other reason is simply not in the output, so the +// caller can name it rather than estimate it. +// +// `.github/workflows/shard-timings-refresh.yml` does all of this on a weekly +// timer and opens the PR. +// // Locally, on a 4-vCPU box (the hosted runner's shape): // pnpm exec turbo run build // pnpm exec turbo run test --concurrency=4 --summarize \ @@ -50,6 +77,8 @@ // // Usage: // node scripts/measure-test-shard-timings.mjs ... [--out ] +// node scripts/measure-test-shard-timings.mjs --run ... \ +// --run ... [--merge-into ] [--out ] // node scripts/measure-test-shard-timings.mjs --self-test import { existsSync, readFileSync, readdirSync, writeFileSync } from 'node:fs'; @@ -181,7 +210,7 @@ export function fallbackRate(measured, fileCounts) { return Math.round(median(rates) * 1000) / 1000; } -export function buildDataset({ perSummary, fileCounts, provenance }) { +export function buildDataset({ perSummary, fileCounts, provenance, carryFrom = null }) { const bySample = new Map(); const cachedNames = new Set(); const push = (name, seconds) => { @@ -193,10 +222,42 @@ export function buildDataset({ perSummary, fileCounts, provenance }) { // summaries, and the whole-package cost this dataset records is their SUM -- // not their median, which is the rule for repeat measurements of one package // and would write 1/n of the truth here. So slices are held back and summed - // per (package, slice count) set; the sum then enters the median pool as ONE - // sample, which keeps the two rules composable when several runs are fed in. + // per (run, package, slice count) set; the sum then enters the median pool as + // ONE sample per run, which is exactly what an unsliced package contributes + // per run, so the two rules compose instead of competing. + // + // ⛔ THE SUM IS ONLY MEANINGFUL WITHIN ONE RUN (#16473). The order is + // load-bearing and there is only one correct one: SUM the slices a single run + // produced, then MEDIAN those per-run sums across runs. Keyed by (package, + // slice count, index) ALONE -- which is what this ledger used to be -- every + // run collapses into one entry and BOTH halves break at once, silently: + // + // * the innermost value is overwritten by each successive summary carrying + // that index, so the package takes whichever run was read LAST no matter + // how many are fed. Measured on three runs giving 400/600/1000s: 1000 + // recorded, median 600. Every OTHER package in the same refresh gets its + // median, so the one package the slicing machinery exists for -- the + // heaviest suite in the workspace -- is the single least robust reading + // in the file, with no line of output saying so. + // * a set completed from slices of DIFFERENT runs is summed as though it + // were one measurement. Run B's 1/2=300 and 2/2=300 with run C's 1/2=500 + // recorded 800s -- a duration no run observed -- and + // `skippedIncompleteSlices` stayed EMPTY, because from that ledger's point + // of view the set IS complete. It just is not from one run. + // + // Both are this file's own signature hazard (a wrong number that reads exactly + // like a right one) surviving on the axis of WHICH RUN, on the package whose + // mis-weighting killed a shard twelve times in a day. + // + // The grouping key comes from the CALLER, because a run summary carries no run + // identifier to infer one from: the six artifacts of one CI run are named and + // fetched together, so the caller is the only actor that knows which is which. + // An entry with no `run` joins one implicit group -- correct for the ordinary + // single-run refresh, which is what the scheduled workflow feeds -- and the + // duplicate refusal below is what keeps that default honest when more than one + // run is fed without declaring itself. const sliceLedger = new Map(); - for (const { samples, skippedCached, slices } of perSummary) { + for (const { samples, skippedCached, slices, run = null } of perSummary) { for (const n of skippedCached) cachedNames.add(n); for (const [name, seconds] of samples) { const slice = slices?.get(name) ?? null; @@ -204,30 +265,59 @@ export function buildDataset({ perSummary, fileCounts, provenance }) { push(name, seconds); continue; } - if (!sliceLedger.has(name)) sliceLedger.set(name, new Map()); - const byCount = sliceLedger.get(name); + if (!sliceLedger.has(run)) sliceLedger.set(run, new Map()); + const byName = sliceLedger.get(run); + if (!byName.has(name)) byName.set(name, new Map()); + const byCount = byName.get(name); if (!byCount.has(slice.count)) byCount.set(slice.count, new Map()); - byCount.get(slice.count).set(slice.index, seconds); + const seen = byCount.get(slice.count); + // ⛔ REFUSE, never take the last. One run runs each slice exactly once, so + // a second window for the same (package, slice) means these summaries come + // from different runs sharing one group -- which is the ambiguity that + // produced both numbers above. Taking either value, or their sum, records + // a weight assembled from runs that never happened together. + if (seen.has(slice.index)) { + throw new Error( + `${name} slice ${slice.index}/${slice.count} was measured twice in ` + + `${run === null ? 'this summary set' : `run ${run}`} (${seen.get(slice.index)}s and ` + + `${seconds}s). A run runs each slice exactly once, so these summaries come from ` + + 'DIFFERENT runs. Group them with `--run ` before each run\'s summaries: slices ' + + 'are summed WITHIN a run and the per-run sums are medianed ACROSS runs. Refusing to ' + + 'pick one, which would record the last run read rather than a median (#16473).' + ); + } + seen.set(slice.index, seconds); } } - // ⛔ An INCOMPLETE slice set is not summed. Summing 1 of 2 slices would record - // half a suite as the whole of it -- a wrong number that reads exactly like a - // right one, which is the hazard this file's cache rule already refuses in the - // other direction. The package instead drops out of `packages` entirely and is - // ESTIMATED from its test-file count like any unmeasured package, and it is - // named in the dataset so a refresh built on a partial artifact set is visible - // in the file rather than inferred from the split going strange later. + // ⛔ An INCOMPLETE slice set is not summed, and completeness is judged WITHIN + // ONE RUN. Summing 1 of 2 slices would record half a suite as the whole of it + // -- a wrong number that reads exactly like a right one, which is the hazard + // this file's cache rule already refuses in the other direction. Borrowing the + // missing slice from ANOTHER run to complete the set is the same wrong number + // wearing a complete set's clothes (#16473), so a run that cannot assemble the + // package on its own contributes nothing rather than something spliced. The + // package drops out of that run's sample and, if no run could assemble it, out + // of `packages` entirely -- ESTIMATED from its test-file count like any + // unmeasured package -- and every partial set is named, with its run, so a + // refresh built on a partial artifact set is visible in the file rather than + // inferred from the split going strange three weeks later. const incompleteSlices = []; - for (const [name, byCount] of sliceLedger) { - for (const [count, seen] of byCount) { - if (seen.size === count) { - push(name, [...seen.values()].reduce((a, b) => a + b, 0)); - continue; + for (const [run, byName] of sliceLedger) { + for (const [name, byCount] of byName) { + for (const [count, seen] of byCount) { + if (seen.size === count) { + push(name, [...seen.values()].reduce((a, b) => a + b, 0)); + continue; + } + const missing = []; + for (let i = 1; i <= count; i++) if (!seen.has(i)) missing.push(`${i}/${count}`); + incompleteSlices.push( + run === null + ? `${name} (missing ${missing.join(', ')})` + : `${name} in run ${run} (missing ${missing.join(', ')})` + ); } - const missing = []; - for (let i = 1; i <= count; i++) if (!seen.has(i)) missing.push(`${i}/${count}`); - incompleteSlices.push(`${name} (missing ${missing.join(', ')})`); } } incompleteSlices.sort((a, b) => a.localeCompare(b, 'en')); @@ -242,6 +332,44 @@ export function buildDataset({ perSummary, fileCounts, provenance }) { const measured = new Map( [...bySample.entries()].map(([name, values]) => [name, Math.round(median(values) * 100) / 100]) ); + + // ⛔ A CARRIED WEIGHT NEEDS A CACHE HIT AS ITS WITNESS. NOTHING ELSE CARRIES. + // + // Measured on the first two live runs of the refresh lane: NO set of retained + // green runs measures the whole workspace. The best single run covered 52 of + // the 71 packages the dataset holds, the accumulation of all seven converged + // at 57, and the last 14 were turbo cache HITs in every one of them. That is + // the cache design rather than luck -- the key is namespaced per shard and + // only main pushes write it, so a package whose inputs have not changed is a + // HIT, and this file refuses hits rather than recording a replayed ~0.1s + // window as a suite's cost. + // + // So "regenerate" cannot mean "replace". It means MERGE, and the merge is + // sound for one specific reason: a cache HIT is not missing data, it is + // POSITIVE EVIDENCE that the package's inputs are unchanged since the run + // whose output was replayed. Its last measured weight therefore still + // describes it -- the number is not stale, and turbo is the witness. Carrying + // it forward preserves this file's whole invariant: every number in it is a + // real measurement of code as it stands, never an estimate. + // + // The witness requirement is what keeps that from becoming a licence to keep + // anything. A package absent from the summaries for ANY OTHER reason -- it + // never ran, its suite failed, its slices could not be assembled -- has no + // evidence behind it, so it is NOT carried. It drops out and the caller's + // coverage check names it. `skippedAsCached` is exactly the witnessed set, so + // the carry reads it rather than inventing a second classification. + const carriedOver = []; + if (carryFrom) { + for (const name of Object.keys(carryFrom).sort((a, b) => a.localeCompare(b, 'en'))) { + if (measured.has(name)) continue; + if (!cachedNames.has(name)) continue; + const seconds = carryFrom[name]; + if (typeof seconds !== 'number' || !(seconds >= 0)) continue; + measured.set(name, seconds); + carriedOver.push(name); + } + } + const packages = {}; for (const name of [...measured.keys()].sort((a, b) => a.localeCompare(b, 'en'))) { packages[name] = measured.get(name); @@ -250,12 +378,22 @@ export function buildDataset({ perSummary, fileCounts, provenance }) { note: 'GENERATED by scripts/measure-test-shard-timings.mjs -- do not hand-edit. Per-package ' + '`turbo run test` durations in seconds, the balancing input for the Test Core shard ' + - 'split (scripts/partition-test-shards.mjs). See `provenance.refresh` to regenerate.', + 'split (scripts/partition-test-shards.mjs). See `provenance.refresh` to regenerate. ' + + 'Every weight is a real measurement: the ones in `carriedOver` were measured by an earlier ' + + 'refresh and re-confirmed unchanged by a turbo cache HIT in this one.', provenance, secondsPerTestFileFallback: fallbackRate(measured, fileCounts), packages, skippedAsCached: [...cachedNames].sort((a, b) => a.localeCompare(b, 'en')), skippedIncompleteSlices: incompleteSlices, + // Beside `skippedAsCached` rather than folded into a per-package + // `measuredAt`, and the choice is forced by the reader: partition-test- + // shards.mjs reads `packages` as name -> NUMBER (`timings.packages[name] / + // sliceCount`) and never opens `provenance` at all, so per-package dates + // would mean changing that shape and every consumer of it. A single + // `provenance.measuredAt` paired with this list carries the same + // information and needs no change in the partitioner. + carriedOver, }; } @@ -287,7 +425,7 @@ export function buildDataset({ perSummary, fileCounts, provenance }) { // must not red. A battery BELOW its floor means cases stopped running; the // remedy is to find what stopped registering, never to lower the number. const SELF_TEST_BATTERIES = Object.freeze({ - 'measure-test-shard-timings self-test': 34, + 'measure-test-shard-timings self-test': 50, }); // DELETING an entry silences that battery's floor exactly as effectively as @@ -539,8 +677,9 @@ function selfTest() { } }); - // The two merge rules compose: sum WITHIN a run, median ACROSS runs. - const twoRuns = buildDataset({ + // One run's two slices sum to the whole package. This is the SUM half only -- + // it says nothing about the median half, which is what the block below pins. + const oneRunTwoSlices = buildDataset({ perSummary: [ samplesFromSummary(summary([slicedTask('cli', 0, 500_000, 1, 2)]), 'r1a'), samplesFromSummary(summary([slicedTask('cli', 0, 500_000, 2, 2)]), 'r1b'), @@ -550,8 +689,238 @@ function selfTest() { provenance: {}, }); check(() => { - if (twoRuns.packages.cli !== 1000) { - throw new Error(`slice: a 2-slice set summed to ${twoRuns.packages.cli}, expected 1000`); + if (oneRunTwoSlices.packages.cli !== 1000) { + throw new Error(`slice: a 2-slice set summed to ${oneRunTwoSlices.packages.cli}, expected 1000`); + } + }); + + // The two merge rules COMPOSE, and in one order only (#16473): sum the slices + // WITHIN a run, then median those per-run sums ACROSS runs. Every case below + // failed before the ledger was keyed by run, and each fails in a different + // direction, so none of them can be satisfied by accident: + // + // * the sliced median: last-wins answered 1000 where the median is 600 + // * the control: proves the median rule was alive the whole time, so the + // sliced path alone was bypassing it -- without this leg a broken median + // would look like a broken slice rule + // * the splice: 800s assembled from two different runs, with the file's own + // `skippedIncompleteSlices` guard silent because the set looked complete + // * the refusal: the guard that makes the UNDECLARED default safe, so the + // ordinary single-run call needs no ceremony and a multi-run one cannot + // quietly do the wrong thing + const fromRun = (run, tasks, label) => ({ ...samplesFromSummary(summary(tasks), label), run }); + + // Three runs of a 2-way sliced package: 200+200, 300+300, 500+500 -> the runs + // measured 400, 600 and 1000s, so the package's weight is the median 600. The + // unsliced control rides in the same dataset, fed 100/300/500 across the same + // three runs, and must answer its own median 300. + const threeRuns = buildDataset({ + perSummary: [ + fromRun('A', [slicedTask('cli', 0, 200_000, 1, 2)], 'a1'), + fromRun('A', [slicedTask('cli', 0, 200_000, 2, 2), testTask('ctl', 0, 100_000)], 'a2'), + fromRun('B', [slicedTask('cli', 0, 300_000, 1, 2)], 'b1'), + fromRun('B', [slicedTask('cli', 0, 300_000, 2, 2), testTask('ctl', 0, 300_000)], 'b2'), + fromRun('C', [slicedTask('cli', 0, 500_000, 1, 2)], 'c1'), + fromRun('C', [slicedTask('cli', 0, 500_000, 2, 2), testTask('ctl', 0, 500_000)], 'c2'), + ], + fileCounts: new Map([['cli', 300], ['ctl', 100]]), + provenance: {}, + }); + check(() => { + if (threeRuns.packages.cli !== 600) { + throw new Error( + `slice: three runs measuring 400/600/1000s recorded ${threeRuns.packages.cli}, expected the ` + + 'median 600 (1000 is the last run read -- the #16473 last-wins ledger)' + ); + } + }); + check(() => { + if (threeRuns.packages.ctl !== 300) { + throw new Error( + `slice: the unsliced control recorded ${threeRuns.packages.ctl}, expected its median 300 -- ` + + 'the median rule itself is broken, not just the sliced path' + ); + } + }); + check(() => { + if (threeRuns.skippedIncompleteSlices.length !== 0) { + throw new Error( + `slice: every run assembled a complete set, but ${threeRuns.skippedIncompleteSlices.join('; ')} ` + + 'was reported incomplete' + ); + } + }); + + // The cross-run splice. Run B is complete (300+300); run C fed only 1/2. The + // ledger must NOT reach into run B for run C's missing slice: the answer is + // run B's 600 alone, and run C is named as the partial set it is. Keyed + // without a run this recorded 800s -- a duration no run observed -- and named + // nothing. + const spliced = buildDataset({ + perSummary: [ + fromRun('B', [slicedTask('cli', 0, 300_000, 1, 2)], 'b1'), + fromRun('B', [slicedTask('cli', 0, 300_000, 2, 2)], 'b2'), + fromRun('C', [slicedTask('cli', 0, 500_000, 1, 2), testTask('ctl', 0, 10_000)], 'c1'), + ], + fileCounts: new Map([['cli', 300], ['ctl', 100]]), + provenance: {}, + }); + check(() => { + if (spliced.packages.cli !== 600) { + throw new Error( + `slice: a set completed ACROSS runs recorded ${spliced.packages.cli}, expected run B's own 600 ` + + '(800 is B 1/2 + B 2/2 + C 1/2 spliced -- a weight no run measured)' + ); + } + }); + check(() => { + if (!spliced.skippedIncompleteSlices.some((s) => s.includes('cli') && s.includes('run C') && s.includes('2/2'))) { + throw new Error( + 'slice: run C could not assemble the package and was not named in skippedIncompleteSlices ' + + `(${spliced.skippedIncompleteSlices.join('; ') || 'empty'})` + ); + } + }); + + // The refusal that makes the undeclared default honest. Two runs' slices fed + // as one group is not resolvable -- the summaries carry no run id of their own + // -- so it is a named error, never the last value read. + check(() => { + if (!threw(() => + buildDataset({ + perSummary: [ + samplesFromSummary(summary([slicedTask('cli', 0, 300_000, 1, 2)]), 'x1'), + samplesFromSummary(summary([slicedTask('cli', 0, 500_000, 1, 2)]), 'x2'), + ], + fileCounts: new Map([['cli', 300]]), + provenance: {}, + }) + )) { + throw new Error('slice: two runs fed as one group were resolved by last-wins instead of refused'); + } + }); + check(() => { + let message = ''; + try { + buildDataset({ + perSummary: [ + samplesFromSummary(summary([slicedTask('cli', 0, 300_000, 2, 2)]), 'x1'), + samplesFromSummary(summary([slicedTask('cli', 0, 500_000, 2, 2)]), 'x2'), + ], + fileCounts: new Map([['cli', 300]]), + provenance: {}, + }); + } catch (error) { + message = String(error?.message ?? ''); + } + // The remedy has to be IN the refusal: a caller who hits this is holding + // several runs' artifacts and needs to be told the flag, not just told no. + if (!message.includes('--run') || !message.includes('cli slice 2/2')) { + throw new Error(`slice: the duplicate-slice refusal does not name the slice and the remedy (${message})`); + } + }); + + // The MERGE, and the witness rule that bounds it (#16464). No retained run set + // measures the whole workspace, so a refresh that REPLACED the dataset would + // demote every cache-hit package to a test-file-count estimate. Carrying is + // sound only because a cache HIT is evidence the package is unchanged, so the + // cases below pin the carry AND its boundary: what has a witness carries, what + // does not is simply absent for the caller to name. + const priorDataset = { a: 10, cached: 500, vanished: 700 }; + const mergeSummary = summary([ + testTask('a', 0, 12_000), + { taskId: 'cached#test', task: 'test', package: 'cached', cache: { status: 'HIT' }, execution: { startTime: 0, endTime: 90, exitCode: 0 } }, + ]); + const mergedSet = buildDataset({ + perSummary: [samplesFromSummary(mergeSummary, 'm')], + fileCounts: new Map([['a', 6], ['cached', 250], ['vanished', 300]]), + provenance: {}, + carryFrom: priorDataset, + }); + + // 1. A CARRIED package: weight unchanged, and named. + check(() => { + if (mergedSet.packages.cached !== 500) { + throw new Error(`merge: a cache-hit package was not carried at its previous weight (got ${mergedSet.packages.cached}, expected 500)`); + } + }); + check(() => { + if (!mergedSet.carriedOver.includes('cached')) { + throw new Error(`merge: the carried package was not named in carriedOver (${mergedSet.carriedOver.join(', ') || 'empty'})`); + } + }); + // 2. A freshly MEASURED package takes the new number, and is NOT called carried. + check(() => { + if (mergedSet.packages.a !== 12) throw new Error(`merge: a measured package did not take its new weight (${mergedSet.packages.a})`); + }); + check(() => { + if (mergedSet.carriedOver.includes('a')) throw new Error('merge: a package measured in this pass was reported as carried'); + }); + // 3. HIT-WITNESSED CARRY versus ABSENT: `vanished` is in the prior dataset but + // appears in NO summary, so nothing witnesses that it is unchanged. It must + // NOT be carried — this is the case that separates a merge from "keep + // whatever was there", and without it the carry would launder a stale + // number for a package that may have been deleted, renamed, or gone red. + check(() => { + if (Object.hasOwn(mergedSet.packages, 'vanished')) { + throw new Error(`merge: a package with no cache-hit witness was carried anyway (${mergedSet.packages.vanished})`); + } + }); + check(() => { + if (mergedSet.carriedOver.includes('vanished')) throw new Error('merge: an unwitnessed package was named as carried'); + }); + // 4. No carryFrom at all is the plain replace, and reports an empty list + // rather than omitting the field — an absent key and "nothing was carried" + // must not read the same way to the caller's coverage check. + check(() => { + const plain = buildDataset({ + perSummary: [samplesFromSummary(mergeSummary, 'm')], + fileCounts: new Map([['a', 6]]), + provenance: {}, + }); + if (!Array.isArray(plain.carriedOver) || plain.carriedOver.length !== 0) { + throw new Error(`merge: a run with no --merge-into did not report an empty carriedOver (${JSON.stringify(plain.carriedOver)})`); + } + }); + // 5. The merge does NOT rescue a run that measured nothing: an all-cached pass + // still refuses, so "everything carried" can never masquerade as a refresh. + check(() => { + if (!threw(() => + buildDataset({ + perSummary: [samplesFromSummary(summary([testTask('a', 0, 40, 'HIT')]), 'f')], + fileCounts: new Map([['a', 6]]), + provenance: {}, + carryFrom: priorDataset, + }) + )) { + throw new Error('merge: a pass that measured NOTHING produced a dataset out of carried weights alone'); + } + }); + // 6. MONOTONE ACCUMULATION: feeding a second run measures more, and a package + // measured by the newer run stops being carried and takes its real number. + check(() => { + const oneRun = buildDataset({ + perSummary: [{ ...samplesFromSummary(mergeSummary, 'm'), run: 'r1' }], + fileCounts: new Map([['a', 6], ['cached', 250]]), + provenance: {}, + carryFrom: priorDataset, + }); + const twoRuns = buildDataset({ + perSummary: [ + { ...samplesFromSummary(mergeSummary, 'm'), run: 'r1' }, + { ...samplesFromSummary(summary([testTask('cached', 0, 480_000)]), 'n'), run: 'r2' }, + ], + fileCounts: new Map([['a', 6], ['cached', 250]]), + provenance: {}, + carryFrom: priorDataset, + }); + if (oneRun.carriedOver.length !== 1 || twoRuns.carriedOver.length !== 0) { + throw new Error( + `merge: accumulation is not monotone — one run carried ${oneRun.carriedOver.length}, two carried ${twoRuns.carriedOver.length} (expected 1 then 0)` + ); + } + if (twoRuns.packages.cached !== 480) { + throw new Error(`merge: the second run measured the package but it kept its carried weight (${twoRuns.packages.cached})`); } }); @@ -690,42 +1059,102 @@ function main() { return; } let out = DEFAULT_OUT; + let mergeInto = null; + // Each input carries the run it belongs to (#16473). `--run ` opens a + // group and every summary AFTER it belongs to that run, so one CI run's six + // artifacts are named together the way they are fetched together. Summaries + // before any `--run` share one implicit group, which is the ordinary + // single-run refresh; feeding two runs that way is not silently averaged or + // last-won, it is refused by buildDataset with `--run` named as the remedy. const inputs = []; + let currentRun = null; for (let i = 0; i < argv.length; i++) { if (argv[i] === '--out') out = path.resolve(argv[++i]); - else if (argv[i].startsWith('--')) throw new Error(`unrecognized argument: ${argv[i]}`); - else inputs.push(argv[i]); + else if (argv[i] === '--merge-into') { + const value = argv[++i]; + if (typeof value !== 'string' || value.length === 0 || value.startsWith('--')) { + throw new Error('--merge-into needs the path of the dataset to carry unchanged weights from'); + } + mergeInto = path.resolve(value); + } else if (argv[i] === '--run') { + const value = argv[++i]; + if (typeof value !== 'string' || value.length === 0 || value.startsWith('--')) { + throw new Error('--run needs a run identifier (the id of the CI run whose summaries follow it)'); + } + currentRun = value; + } else if (argv[i].startsWith('--')) throw new Error(`unrecognized argument: ${argv[i]}`); + else inputs.push({ file: argv[i], run: currentRun }); } if (inputs.length === 0) { - console.error('usage: measure-test-shard-timings.mjs ... [--out ]'); + console.error( + 'usage: measure-test-shard-timings.mjs [--run ] ... ' + + '[--merge-into ] [--out ]' + ); process.exit(1); } const perSummary = []; - for (const input of inputs) { - perSummary.push(samplesFromSummary(JSON.parse(readFileSync(input, 'utf8')), input)); + for (const { file, run } of inputs) { + perSummary.push({ ...samplesFromSummary(JSON.parse(readFileSync(file, 'utf8')), file), run }); } - const fileCounts = new Map(); - for (const { samples } of perSummary) { - for (const name of samples.keys()) { - if (fileCounts.has(name)) continue; - const dir = packageDirForName(name); - fileCounts.set(name, dir ? countTestFiles(dir) : 0); + // The prior dataset, when a merge was asked for. Read before the file counts, + // because a carried package needs a count too -- it votes on the fallback rate + // exactly like a freshly measured one, which is what keeps that rate derived + // from the numbers actually in the file rather than from a subset of them. + let carryFrom = null; + if (mergeInto !== null) { + const prior = JSON.parse(readFileSync(mergeInto, 'utf8')); + if (!prior || typeof prior.packages !== 'object' || prior.packages === null) { + throw new Error( + `--merge-into ${path.relative(REPO_ROOT, mergeInto)}: expected a dataset with a {packages:{...}} ` + + 'map to carry unchanged weights from. Refusing to merge into a shape this did not write.' + ); } + carryFrom = prior.packages; + } + + const fileCounts = new Map(); + const needCount = new Set(); + for (const { samples } of perSummary) for (const name of samples.keys()) needCount.add(name); + if (carryFrom) for (const name of Object.keys(carryFrom)) needCount.add(name); + for (const name of needCount) { + const dir = packageDirForName(name); + fileCounts.set(name, dir ? countTestFiles(dir) : 0); } + const declaredRuns = [...new Set(inputs.map((i) => i.run).filter((r) => r !== null))]; const dataset = buildDataset({ perSummary, fileCounts, + carryFrom, provenance: { measuredAt: new Date().toISOString().slice(0, 10), - summaries: inputs.map((i) => path.basename(i)), - mergeRule: 'median across summaries', + summaries: inputs.map((i) => path.basename(i.file)), + ...(declaredRuns.length > 0 ? { runs: declaredRuns } : {}), + // Stated as the two composed rules it actually is (#16473). "median across + // summaries" was true only of unsliced packages: a file-sharded package's + // summaries are PARTS of one measurement, not repeats of it, so they are + // summed within their run first and only the per-run sums are medianed. + mergeRule: + 'median across runs; a file-sharded package is summed from its slices WITHIN one run ' + + 'first, and a run that cannot assemble every slice contributes no sample for it; a package ' + + 'not measured in this pass keeps its previous weight ONLY when a turbo cache HIT witnesses ' + + 'that its inputs are unchanged, and every such package is named in `carriedOver`', + // `measuredAt` is the date of THIS pass, and it dates the measured + // weights. The carried ones were measured earlier and re-confirmed + // unchanged by a cache HIT today; `carriedOver` names them, which is why a + // single date is enough and per-package dates are not needed. See the note + // on `carriedOver` in buildDataset for why the reader forces that choice. refresh: - 'node scripts/measure-test-shard-timings.mjs ... --out scripts/test-shard-timings.json ' + - '(summaries: the `test-core-run-summary--of-6` artifacts of any green merge_group run, or a local ' + - '`pnpm exec turbo run test --concurrency=4 --summarize`)', + 'node scripts/measure-test-shard-timings.mjs [--run ] ... ' + + '--merge-into scripts/test-shard-timings.json --out scripts/test-shard-timings.json ' + + '(summaries: the `test-core-run-summary--of-6` artifacts of any green run, or a local ' + + '`pnpm exec turbo run test --concurrency=4 --summarize`. ⚠ ONE RUN COVERS ONLY 2-52 of ~71 ' + + 'packages depending on cache warmth, so feed SEVERAL runs -- a `--run ` before each ' + + 'run\'s summaries is REQUIRED, so a sliced package is assembled per run and then medianed ' + + 'like every other package -- and `--merge-into` to carry the cache-hit remainder. ' + + '`.github/workflows/shard-timings-refresh.yml` does all of this weekly.)', }, }); writeFileSync(out, `${JSON.stringify(dataset, null, 2)}\n`);