fix(bench): interleave baseline and current samples

.github/workflows/bench.yml

@@ -69,61 +69,56 @@ jobs:
           echo "PLATFORM=$PLATFORM" >> "$GITHUB_ENV"
 
       - name: Run benchmarks
+        if: github.event_name != 'pull_request'
         run: |
           go test -bench=. -benchmem -count=6 -run='^$' -timeout=15m \
             ${{ env.BENCH_PKGS }} | tee bench-current.txt
 
+      # ── PR: benchmark base and PR code on the SAME RUNNER, interleaved.
+      #    Apple Silicon GitHub Actions VMs show 30-50% variance across runner
+      #    instances, so a stored baseline is unusable.  Running both versions
+      #    on one runner fixes that, but running them as two consecutive blocks
+      #    leaves a second failure mode: if the runner drifts between the two
+      #    blocks, every benchmark in the later block shifts at once and reads
+      #    as a uniform package-wide regression.  Interleaving the samples puts
+      #    both versions in the same time window, so drift cancels out.
+      - name: Run interleaved benchmarks (same-runner baseline)
+        if: github.event_name == 'pull_request'
+        run: |
+          set +e
+          bash scripts/bench-interleaved.sh \
+            "${{ github.event.pull_request.base.sha }}" 6 \
+            bench-baseline.txt bench-current.txt ${{ env.BENCH_PKGS }}
+          status=$?
+          set -e
+          # Exit 3 means nothing on the base commit is comparable.
+          if [ "$status" -eq 3 ]; then
+            echo "no_baseline=true" >> "$GITHUB_ENV"
+          elif [ "$status" -ne 0 ]; then
+            exit "$status"
+          fi
+
       - name: Upload benchmark results
         uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
         with:
           name: bench-${{ env.PLATFORM }}-${{ github.sha }}
           path: bench-current.txt
           retention-days: 90
 
-      # ── PR: run main benchmarks on the SAME RUNNER to eliminate cross-machine
-      #    variance.  Apple Silicon GitHub Actions VMs show 30-50% variance across
-      #    different runner instances, making stored-baseline comparisons unreliable.
-      #    By benchmarking both main and PR code on the same machine we keep a
-      #    tight threshold and detect real regressions without false positives.
-      - name: Run main benchmarks (same-runner baseline)
-        if: github.event_name == 'pull_request'
-        run: |
-          BASE_SHA="${{ github.event.pull_request.base.sha }}"
-          PR_REF="$(git rev-parse HEAD)"
-          echo "Checking out base ($BASE_SHA) for same-runner comparison…"
-          git checkout "$BASE_SHA" --quiet
-
-          # Only benchmark packages that exist on the base branch , the PR may
-          # introduce new packages that have no main-branch counterpart.
-          VALID_PKGS=""
-          for pkg in $BENCH_PKGS; do
-            dir="${pkg#./}"
-            dir="${dir%/}"
-            if [ -d "$dir" ]; then
-              VALID_PKGS="$VALID_PKGS $pkg"
-            fi
-          done
-
-          if [ -z "$VALID_PKGS" ]; then
-            echo "No benchmark packages found on base , skipping comparison."
-            echo "no_baseline=true" >> "$GITHUB_ENV"
-          else
-            echo "Benchmarking on base: $VALID_PKGS"
-            go test -bench=. -benchmem -count=6 -run='^$' -timeout=15m \
-              $VALID_PKGS | tee bench-baseline.txt
-          fi
-
-          # Return to PR code.
-          git checkout "$PR_REF" --quiet
-
       - name: Compare vs same-runner baseline
         if: github.event_name == 'pull_request' && env.no_baseline != 'true'
         run: |
           benchstat bench-baseline.txt bench-current.txt > benchstat-diff.txt
           cat benchstat-diff.txt
-          bash scripts/bench-regress.sh bench-baseline.txt bench-current.txt ${{ matrix.time_threshold }} 10 || {
+          set +e
+          bash scripts/bench-regress.sh bench-baseline.txt bench-current.txt \
+            ${{ matrix.time_threshold }} 10 > bench-regressions.txt 2>&1
+          status=$?
+          set -e
+          cat bench-regressions.txt
+          if [ "$status" -ne 0 ]; then
             echo "regression=true" >> "$GITHUB_ENV"
-          }
+          fi
 
       - name: Build PR comment body
         if: github.event_name == 'pull_request' && env.no_baseline != 'true'
@@ -143,6 +138,22 @@ jobs:
             if [ "${{ env.regression }}" = "true" ]; then
               echo "> [!CAUTION]"
               echo "> Benchmarks regressed by more than ${{ matrix.time_threshold }}% on \`${{ env.PLATFORM }}\`."
+              echo ""
+              echo "Flagged:"
+              echo ""
+              echo '```'
+              cat bench-regressions.txt
+              echo '```'
+              echo ""
+              echo "Samples are interleaved between base and PR code, so runner drift"
+              echo "should cancel out.  If the flagged benchmarks live in packages this"
+              echo "PR cannot reach, or a whole package moved together, treat the result"
+              echo "as suspect and confirm locally with:"
+              echo ""
+              echo '```'
+              echo "go test -bench=<name> -count=10 -run='^\$' <pkg>   # on base, then on the PR"
+              echo "benchstat base.txt pr.txt"
+              echo '```'
             else
               echo "> [!NOTE]"
               echo "> No significant regressions on \`${{ env.PLATFORM }}\` (threshold: ${{ matrix.time_threshold }}%, same-runner comparison)."

scripts/bench-interleaved.sh

@@ -0,0 +1,114 @@
+#!/usr/bin/env bash
+# bench-interleaved.sh — sample baseline and current benchmarks in alternating
+# order so runner drift cannot masquerade as a code regression.
+#
+# Why: running every benchmark for one version and then every benchmark for the
+# other puts the two groups in separate time windows.  Anything that changes the
+# runner between those windows (cold caches, CPU frequency ramp, a noisy
+# neighbour on a shared VM) shifts one whole group in one direction.  benchstat
+# then reports a large, uniform, statistically significant "regression" across
+# entire packages, including packages the change never touched.  Tight variance
+# and a small p-value do not rule this out: the comparison controls for
+# runner-to-runner variance, but not for one runner drifting over time.
+#
+# This script compiles both versions up front, then alternates between them one
+# round at a time, so both are sampled across the same window and drift applies
+# to both equally.  The version that runs first alternates each round, so
+# neither systematically absorbs any per-round warm-up cost.
+#
+# Usage: bench-interleaved.sh <base_ref> <rounds> <baseline_out> <current_out> <pkg>...
+
+set -euo pipefail
+
+BASE_REF="${1:?Usage: bench-interleaved.sh <base_ref> <rounds> <baseline_out> <current_out> <pkg>...}"
+ROUNDS="${2:?missing rounds}"
+BASELINE_OUT="${3:?missing baseline output path}"
+CURRENT_OUT="${4:?missing current output path}"
+shift 4
+
+if [ "$#" -eq 0 ]; then
+  echo "error: no benchmark packages given" >&2
+  exit 2
+fi
+PKGS=("$@")
+
+BIN_DIR="$(mktemp -d)"
+CURRENT_REF="$(git rev-parse HEAD)"
+
+# Always return the worktree to the commit under test, even on failure, so a
+# crash here cannot leave the checkout parked on the base commit.
+cleanup() {
+  git checkout "$CURRENT_REF" --quiet 2>/dev/null || true
+  rm -rf "$BIN_DIR"
+}
+trap cleanup EXIT
+
+# ./internal/panels/filetree/ -> internal_panels_filetree
+pkg_slug() {
+  local p="${1#./}"
+  p="${p%/}"
+  echo "${p//\//_}"
+}
+
+compile() {
+  local pkg="$1" dest="$2"
+  # A package with no test files compiles successfully but emits no binary,
+  # so the caller checks for the file rather than trusting the exit code.
+  go test -c -o "$dest" "$pkg" >/dev/null 2>&1 || return 1
+  [ -f "$dest" ]
+}
+
+mkdir -p "$BIN_DIR/current" "$BIN_DIR/baseline"
+
+echo "Compiling current benchmarks ($CURRENT_REF)…"
+for pkg in "${PKGS[@]}"; do
+  slug="$(pkg_slug "$pkg")"
+  if ! compile "$pkg" "$BIN_DIR/current/$slug.test"; then
+    echo "error: failed to build benchmarks for $pkg" >&2
+    exit 1
+  fi
+done
+
+echo "Compiling baseline benchmarks ($BASE_REF)…"
+git checkout "$BASE_REF" --quiet
+VALID=()
+for pkg in "${PKGS[@]}"; do
+  slug="$(pkg_slug "$pkg")"
+  # The PR may add packages that have no counterpart on the base commit.
+  if compile "$pkg" "$BIN_DIR/baseline/$slug.test"; then
+    VALID+=("$pkg")
+  else
+    echo "  skipping $pkg (absent or has no benchmarks on base)"
+  fi
+done
+git checkout "$CURRENT_REF" --quiet
+
+# Create the outputs before any early exit, so callers that upload them as
+# artifacts always find a file.
+: >"$BASELINE_OUT"
+: >"$CURRENT_OUT"
+
+if [ "${#VALID[@]}" -eq 0 ]; then
+  echo "No benchmark packages are comparable against the base commit."
+  exit 3
+fi
+
+run_one() {
+  "$1" -test.bench=. -test.benchmem -test.count=1 -test.run='^$' -test.timeout=15m >>"$2"
+}
+
+for ((round = 1; round <= ROUNDS; round++)); do
+  echo "Round $round/$ROUNDS…"
+  for pkg in "${VALID[@]}"; do
+    slug="$(pkg_slug "$pkg")"
+    if ((round % 2 == 1)); then
+      run_one "$BIN_DIR/baseline/$slug.test" "$BASELINE_OUT"
+      run_one "$BIN_DIR/current/$slug.test" "$CURRENT_OUT"
+    else
+      run_one "$BIN_DIR/current/$slug.test" "$CURRENT_OUT"
+      run_one "$BIN_DIR/baseline/$slug.test" "$BASELINE_OUT"
+    fi
+  done
+done
+
+echo "Collected $ROUNDS interleaved samples for ${#VALID[@]} package(s)."