Fix linkcheck false positives (release-asset input, accept 200) - #39
Merged
Merged
Conversation
lecture-dp's link checker mirrored the live site with `wget --accept html`, which fetches only HTML — so lychee reported every local asset (_static/*, _images/*) as "file not found" — and used `--accept 403,503`, which drops 200 so every working link was flagged. Check the published HTML release asset instead — a permanent tarball of the full site (HTML + assets), per QuantEcon/meta#282 lesson 2 and matching lecture-python-programming. Restore `--accept 200,403,503`, add `--root-dir` for root-relative links, and scan only top-level `_site/*.html` so the Sphinx theme partials under _static/ (raw Jinja, not real links) are skipped. Mirrors the fix in QuantEcon/continuous_time_mcs#215. See QuantEcon/meta#282 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Under `set -eo pipefail`, a transient `gh api` failure aborted the fetch step before the empty-input guard ran, so no 'release asset missing' report was created. Add `|| true` (and silence stderr) so the guard still fires. Mirrors QuantEcon/continuous_time_mcs#215 (Copilot review follow-up). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the scheduled link-check workflow to avoid false positives by checking links against the published HTML release asset (which includes site assets) instead of mirroring the live site.
Changes:
- Replace live-site
wgetmirroring with downloading/extracting the latest HTML release.tar.gzinto_site. - Restore
--accept 200and add--root-dirso lychee treats working links as valid and resolves root-relative paths correctly. - Limit lychee’s scan scope to
_site/*.htmlto avoid scanning non-page theme partials under_static/.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced Jul 7, 2026
Closed
Closed
Closed
A failed or truncated `curl | tar` was swallowed by `|| true`, potentially leaving a partial _site tree where count > 0 — so lychee would run against an incomplete site and re-introduce 'file not found' false positives. Use `curl -f` and an `if !` guard that clears _site on failure, so a bad download is treated as 'no usable asset' and reported via the guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 7, 2026
Merged
Closed
mmcky
added a commit
to QuantEcon/continuous_time_mcs
that referenced
this pull request
Jul 7, 2026
A failed or truncated `curl | tar` was swallowed by `|| true`, potentially leaving a partial _site tree where count > 0 — so lychee would run against an incomplete site and re-introduce 'file not found' false positives. Use `curl -f` and an `if !` guard that clears _site on failure, so a bad download is treated as 'no usable asset' and reported via the guard. Ports the hardening from QuantEcon/lecture-dp#39 (Copilot review follow-up). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes the same link-check false-positive bug in
lecture-dpthat was just fixed in continuous_time_mcs (QuantEcon/continuous_time_mcs#215). lecture-dp's linkcheck uses the identicalwget-mirror pattern.The bug
The link checker mirrored the live site with
wget --accept html, which fetches only HTML — none of the_static/*/_images/*assets — so lychee checked those relative links as local files and reported them all as "Cannot find file". It also used--accept 403,503, which drops200, so every working link was flagged. On continuous_time_mcs the same config produced 522 "errors", ~all false positives; lecture-dp is exposed to the same noise.The fix
Check links against the published HTML release asset — a permanent
.tar.gzof the full site (HTML plus assets) that the publish workflow already attaches to eachpublish-*release (lecture-dp-html-*.tar.gz). This is the approach in QuantEcon/meta#282 (lesson 2), used bylecture-python-programming..tar.gzinto_site(replaces thewgetmirror); no dependency on the live site.--accept 200,403,503.--root-dirso root-relative links resolve against the site root._site/*.html, so Sphinx theme partials under_static/(raw Jinja like{{ pathto(...) }}, not real links) are skipped.Validated the identical change on continuous_time_mcs: 522 → 6 errors, and the 6 are genuine (0 config false positives).
Related
🤖 Generated with Claude Code