Stop five Windows jobs rebuilding the same frontend on every commit by danielhanchen · Pull Request #9387 · unslothai/unsloth · GitHub
Skip to content

Stop five Windows jobs rebuilding the same frontend on every commit - #9387

Merged
danielhanchen merged 2 commits into
mainfrom
win-fe-dist-composite
Aug 20, 2026
Merged

Stop five Windows jobs rebuilding the same frontend on every commit#9387
danielhanchen merged 2 commits into
mainfrom
win-fe-dist-composite

Conversation

@danielhanchen

Copy link
Copy Markdown
Member

The Linux dist cache from #9375 is proven on main: Cache hit for: fe-dist-Linux-<hash>, frontend up to date, zero building frontend lines, and the install step 71.5s -> 34.6s. Windows pays the same cost and never had the cache.

Measured on current main, Windows Unsloth API CI: [72s] building frontend... -> [168s] frontend built = 96s of a ~257s install (37%).

Scope: 5 job definitions / 7 job runs, ~625s per commit

Two candidates were excluded on evidence, not judgement:

  • applocker-denied-launcher runs install.ps1 --no-torch without --local, so install.ps1:5978 sets STUDIO_LOCAL_INSTALL=0, setup.ps1:3523 takes the $IsPipInstall branch, and run 32363473232 logs frontend bundled (pip install). It never builds a frontend, so a cache there saves 0s and only spends budget. Pinned so nobody completes the set later.
  • startup-profile-ci.yml has pull_request + workflow_dispatch and no push, so it is a pure consumer. It restores on all three OSes but does not save. Its frontend build is 167s on Windows and 38s on macOS.

Two blockers this had to clear

1. An empty dist is worse than no dist, and this repo already shipped that bug. Four workflow files carry a warning citing run 25546676715: pre-creating an empty studio/frontend/dist trips setup.ps1's mtime check into "up to date", and Studio then 500s on GET / with FileNotFoundError: dist\\index.html. The Linux [ ! -d dist ] guard passes an empty directory. Windows has less backstop: update-idempotency only requests /api/health, never GET /. So a hit is not trusted on cache-hit == 'true' alone.

2. Re-dating goes through pwsh, not touch. setup.ps1:3527 reads (Get-Item $DistDir).LastWriteTime; the step assigns that exact property through that exact cmdlet. Git Bash's touch on an NTFS directory reaches the same field through MSYS2's utimensat and backup-semantics handle, which probably works -- and "probably" is the wrong standard when the failure is a reported hit with a rebuild behind it.

Composite pair, not inline

Both approaches were built. The inline version (5 workflows, 422 lines of YAML, the key at six sites) was rejected by the agent that built it: the hazard motivating inline -- uses: ./... resolving from GITHUB_WORKSPACE -- is absent here (no adopting job checks out with path:) and is already guarded repo-wide. Against that, six copies of a key means drift is caught rather than prevented, and "safe because a test catches it" is strictly weaker than one definition. This repo already uses the same split for pip-cache-restore / pip-cache-save, for the identical reason: the save must come after a step the caller owns.

Verified

  • 32/32 mutations caught, each naming the right test.
  • 96 passed across test_frontend_dist_cache, test_uv_cache_discipline, test_cache_budget_discipline. lint_workflow_triggers.py OK across 41 files.
  • Two guards were found weak by mutation and fixed: both were satisfied by the comments quoting the strings they grep for. Script-body assertions now strip comment lines first, and the shapes changed too (read-back asserted as read-then-compare-then-fail; reuse assertion per-branch rather than counting exit 1s).
  • Rebased onto Stop the dist cache key hashing 457 files the rebuild check never reads #9380, so the single key definition carries !studio/frontend/tests/** and !studio/frontend/scripts/**, and the guard that keeps those negations in place was carried across the merge and re-mutation-tested.

Not yet run on a Windows runner. The pwsh argument writes the same .NET property setup.ps1 reads and the setter semantics were proven locally, but the evidence you would want -- Cache hit for: fe-dist-Windows-<hash> followed by frontend up to date and zero building frontend lines -- needs a push to main to populate the first entry.

@danielhanchen
danielhanchen merged commit 4880b78 into main Aug 20, 2026
46 of 49 checks passed
@danielhanchen
danielhanchen deleted the win-fe-dist-composite branch August 20, 2026 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant