Summary
scripts/fetch-wake-models.sh fetches three openWakeWord release artifacts with a bare curl and no retry. A transient failure reaching github.com/dscripka/openWakeWord/releases fails the check job outright, which fails the pull request — for a reason that has nothing to do with the change under review.
It also, unlike its sibling scripts/fetch-vad-model.sh, does not verify what it downloaded.
What exists today
MODELS=(melspectrogram.onnx embedding_model.onnx hey_jarvis_v0.1.onnx)
mkdir -p "${DESTINATION}"
for model in "${MODELS[@]}"; do
if [[ -s "${DESTINATION}/${model}" ]]; then
echo "have ${model}"
continue
fi
echo "fetching ${model}"
curl --fail --silent --show-error --location \
--output "${DESTINATION}/${model}" "${BASE}/${model}"
done
Called from .github/workflows/ci.yml in the check job, before cargo test. The comment there explains why it must not be skipped: without the models the in-process detection tests skip, "which would make the one thing worth proving about a detector the one thing CI never checks." That reasoning is sound and this issue does not propose weakening it — a fetch that fails should still fail the build. It should just not fail on the first packet lost.
Two problems, and they compound
No retry. Three sequential downloads from a third-party release CDN, any one of which failing takes down the job. fetch-vad-model.sh has the same gap, but pulls one file rather than three, so it is a third of the exposure.
No checksum. This is the sharper half, and the contrast with the sibling script makes the case better than I can. fetch-vad-model.sh carries a pinned SHA-256 and says why:
The checksum is the point of this script rather than a nicety: the failure above is invisible at load time, so the guard has to be on the bytes. A model that does not match is not scored.
The wake models have exactly that property. An ONNX file that is truncated, or an HTML error page that --fail happened not to catch, or a re-cut upstream artifact, loads and then scores wrongly — and a wake detector that reports nothing looks like a quiet room. The -s check that short-circuits the download tests only that the file is non-empty, so a bad partial download is cached and every later run reports have melspectrogram.onnx.
Combining them is what makes the retry safe: without a checksum, a retry can re-download the same corrupt bytes and report success.
Proposed change
Mirror fetch-vad-model.sh's structure, extended to three files:
- Pin a SHA-256 per model. Verified as of today against the
v0.5.1 release:
melspectrogram.onnx — ba2b0e0f8b7b875369a2c89cb13360ff53bac436f2895cced9f479fa65eb176f
embedding_model.onnx — 70d164290c1d095d1d4ee149bc5e00543250a7316b59f31d056cff7bd3075c1f
hey_jarvis_v0.1.onnx — 94a13cfe60075b132f6a472e7e462e8123ee70861bc3fb58434a73712ee0d2cb
- Reuse the existing
shasum/sha256sum fallback, so the script keeps working on both macOS and CI's Linux.
- Make the cache check
is non-empty AND matches, so a corrupt cached file is re-fetched rather than trusted.
- Retry each download a small bounded number of times with a backoff —
curl --retry covers the transient HTTP and connection cases directly. Delete and re-fetch on a checksum mismatch, and fail if it still does not match; a mismatch that survives a retry is upstream having changed, which is a human decision.
Worth considering while touching this: the two scripts would then differ only in their URL and their model list, so the shared verify-fetch-retry logic could move to one helper both source. That is a judgment call about whether two small duplicated scripts read better than one script plus a library — I lean toward extracting it, but it is not required to close this.
Acceptance criteria
Related
scripts/fetch-vad-model.sh — the pattern to follow; already checksummed, still un-retried
.github/workflows/ci.yml check job — where both run, and where a flake becomes a red pull request
Summary
scripts/fetch-wake-models.shfetches three openWakeWord release artifacts with a barecurland no retry. A transient failure reachinggithub.com/dscripka/openWakeWord/releasesfails thecheckjob outright, which fails the pull request — for a reason that has nothing to do with the change under review.It also, unlike its sibling
scripts/fetch-vad-model.sh, does not verify what it downloaded.What exists today
Called from
.github/workflows/ci.ymlin thecheckjob, beforecargo test. The comment there explains why it must not be skipped: without the models the in-process detection tests skip, "which would make the one thing worth proving about a detector the one thing CI never checks." That reasoning is sound and this issue does not propose weakening it — a fetch that fails should still fail the build. It should just not fail on the first packet lost.Two problems, and they compound
No retry. Three sequential downloads from a third-party release CDN, any one of which failing takes down the job.
fetch-vad-model.shhas the same gap, but pulls one file rather than three, so it is a third of the exposure.No checksum. This is the sharper half, and the contrast with the sibling script makes the case better than I can.
fetch-vad-model.shcarries a pinned SHA-256 and says why:The wake models have exactly that property. An ONNX file that is truncated, or an HTML error page that
--failhappened not to catch, or a re-cut upstream artifact, loads and then scores wrongly — and a wake detector that reports nothing looks like a quiet room. The-scheck that short-circuits the download tests only that the file is non-empty, so a bad partial download is cached and every later run reportshave melspectrogram.onnx.Combining them is what makes the retry safe: without a checksum, a retry can re-download the same corrupt bytes and report success.
Proposed change
Mirror
fetch-vad-model.sh's structure, extended to three files:v0.5.1release:melspectrogram.onnx—ba2b0e0f8b7b875369a2c89cb13360ff53bac436f2895cced9f479fa65eb176fembedding_model.onnx—70d164290c1d095d1d4ee149bc5e00543250a7316b59f31d056cff7bd3075c1fhey_jarvis_v0.1.onnx—94a13cfe60075b132f6a472e7e462e8123ee70861bc3fb58434a73712ee0d2cbshasum/sha256sumfallback, so the script keeps working on both macOS and CI's Linux.is non-empty AND matches, so a corrupt cached file is re-fetched rather than trusted.curl --retrycovers the transient HTTP and connection cases directly. Delete and re-fetch on a checksum mismatch, and fail if it still does not match; a mismatch that survives a retry is upstream having changed, which is a human decision.Worth considering while touching this: the two scripts would then differ only in their URL and their model list, so the shared verify-fetch-retry logic could move to one helper both source. That is a judgment call about whether two small duplicated scripts read better than one script plus a library — I lean toward extracting it, but it is not required to close this.
Acceptance criteria
have.fetch-vad-model.shgets the retry too, so the two scripts do not diverge on the half they already agree about.Related
scripts/fetch-vad-model.sh— the pattern to follow; already checksummed, still un-retried.github/workflows/ci.ymlcheckjob — where both run, and where a flake becomes a red pull request