ci: download the wiki stylesheet once per snapshot run by Rathoz · Pull Request #8063 · Liquipedia/Lua-Modules

Rathoz · 2026-09-07T12:51:53Z

Summary

The snapshot template linked liquipedia.net's load.php bundle directly, so each of the GoldenTest cases downloaded the same pinned bundle again in its own browser. Now it is fetched once per run into lua/output/css/lakeside.css, next to the compiled main.css, and the template points at that local copy. Each run still fetches fresh, so there is no cache to invalidate.

How did you test this change?

Pipeline

The snapshot template linked liquipedia.net's load.php bundle directly, so every one of the 17 GoldenTest cases spawned its own browser and downloaded the same pinned bundle again. That is 17 downloads per run and 17 chances for a network hiccup to fail the job. Fetch it once up front instead, into lua/output/css/lakeside.css next to the compiled main.css, and point the template at that local copy. Every run still fetches a fresh copy, so there is no cache to invalidate. Every non-data URL in the bundle is root-relative (the FontAwesome webfonts, the Liquipedia logos, the magnify-clip icons). Served from disk those would resolve against the local filesystem, so the script rewrites them back to liquipedia.net. Without that the fonts fail, which snapshot.mjs catches, and the SVG logos silently do not render, which it does not. The output is named after the theme, since lakesideview is only the current production skin, while the script itself stays generic. GoldenTest is a no-op unless UPDATE_SNAPSHOTS is set, so the fetch hangs off update-snapshots rather than lua-test, keeping a plain busted run offline. That means the workflow has to call update-snapshots instead of setting UPDATE_SNAPSHOTS around lua-test, or CI would stop fetching and every snapshot would fail on a missing stylesheet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot

🟡 Changes recommended

The new CI network fetch currently has no explicit timeout and has brittle error formatting, which can lead to hangs or secondary failures when reporting fetch errors.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR improves visual snapshot update performance by downloading Liquipedia’s Lakeside skin stylesheet once per snapshot run (into lua/output/css/lakeside.css) and switching the snapshot HTML template to reference that local file, avoiding repeated network fetches per GoldenTest.

Changes:

Add a one-time-per-run stylesheet fetch step (fetch-remote-css.mjs) and wire it into npm run update-snapshots.
Update the snapshot HTML template to load the locally saved lakeside.css instead of liquipedia.net/commons/load.php.
Adjust the “Update Visual Snapshots” GitHub Actions workflow to call npm run update-snapshots.

File summaries

File	Description
package.json	Adds `fetch:remote-css` script and runs it as part of `update-snapshots`.
lua/spec/helpers/template.html	Switches Lakeside stylesheet link to a local file under `./css/`.
fetch-remote-css.mjs	New script that downloads the remote Lakeside CSS and rewrites root-relative `url()`s.
.github/workflows/update visual snapshots.yml	Updates CI job to run `npm run update-snapshots` instead of env-wrapping `lua-test`.

Review details

Files reviewed: 4/4 changed files
Comments generated: 1
Review effort level: Lite

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

undici gives up after five minutes of its own accord, which is a long time for a CI job to sit on a stalled connection. Abort after thirty seconds and say so. AbortSignal.timeout starts counting at creation, so it can fire while the 519KB body is still streaming, after fetch() has already resolved. Reading the body therefore has to sit under the same guard as the request, or the rejection escapes uncaught. Both now live in one function with one message. Format the thrown value defensively while we are here. fetch() rejects with a TypeError or DOMException, both of which carry a message, so this is belt-and-braces rather than a fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A download does not need a node script, and curl's --max-time covers the whole transfer rather than starting a clock at request time, so the abort that could fire mid-body while the 519KB response was still streaming is no longer something to work around. Downloads to a temporary file rather than piping into sed. sh has no pipefail, so a pipe would exit 0 with a truncated stylesheet on a failed transfer, and an empty stylesheet still loads, meaning snapshot.mjs would screenshot an unstyled page rather than refuse it. Errors now leave the previous copy in place and stop the chain before busted runs. The sed handles the quoted form too. The bundle uses both url(/x) and url("/x"), and rewriting only the bare form leaves four broken references. Kept as a file rather than a package.json one-liner. Inline, the same command is 512 characters of nested quote escaping, and the reason the download is not a pipe stops being sayable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Three sentences where one does. Also fixes the template still pointing at fetch-remote-css.mjs, and the temp file comment, which claimed a pipe would pass unnoticed. That was true of the same command inlined in package.json, run by sh, but this script sets pipefail. What the temp file still buys is the previous copy surviving a failed transfer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Where the rest of the tooling lives, generate_changelog_local.sh included. Dropped the exec bit to match its neighbours, since package.json invokes it through bash either way. The output path is relative to the repository root, so the cd that makes the script work from any directory now has to climb out of scripts/. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot

🟡 Changes recommended

The new fetch script can still clobber an existing lakeside.css on sed failure due to non-atomic output writing, which undermines the intended safety mechanism.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 4/4 changed files
Comments generated: 1
Review effort level: Lite

The snapshot job failed on a 429. load.php rate limits CI's shared address now and then, having answered four earlier runs on this branch fine, so back off and retry rather than fall over. Also ask for the gzipped copy, which is 81KB against 519KB. Pipe into sed and mv the result into place, rather than redirecting over the output. The shell truncates a redirect target before sed runs, so the previous copy was still destroyed if sed failed, which is what the temporary file was there to prevent. pipefail covers the curl end of the pipeline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A backoff does not help against an address that was already blocked on the first call, and the retry was not what turned the snapshot job green, since the 429 passed on its own. Rerunning the job is the answer. Dropping the temporary file with it. set -e stops the chain before busted, and the next attempt downloads again, so no one ever reads a half written stylesheet and there is nothing for the atomic replace to protect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot AI lite review requested due to automatic review settings September 7, 2026 12:51

Rathoz requested review from a team as code owners September 7, 2026 12:51

Copilot started reviewing on behalf of Rathoz September 7, 2026 12:52

View session

hjpalpha approved these changes Sep 7, 2026

View reviewed changes

Copilot AI reviewed Sep 7, 2026

View reviewed changes

Comment thread fetch-remote-css.mjs Outdated