Fix two links that 404 on the published benchmarks page - #17
Conversation
GitHub Pages serves `docs/` as the site root, so a relative link out of it cannot resolve. Two went live with 0.6.0: ../.github/workflows/benchmark.yml -> 404 (outside the site root) benchmarks/ -> 404 (directory, no index page) Both were mine, in the page template rather than in the generated results, so they would have reappeared on every tag. They are now absolute URLs to GitHub. Everything else on the site already resolved -- Jekyll's relative-links plugin rewrites `TESTING.md` to `/TESTING.html` on its own, and `docs/benchmarks/0.6.0.json` is served as a file. The page is regenerated from the corrected template; the 0.6.0 measurements in it are untouched. The new test checks every relative link in `docs/*.md` against the filesystem: it must stay inside `docs/`, exist, and not be a directory without an index. Offline, so it costs nothing and needs no network, and it fails on the two links above against the previous template. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 909f94ec07
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| yield link, target | ||
|
|
||
|
|
||
| DOC_PAGES = sorted((REPO / "docs").glob("*.md")) |
There was a problem hiding this comment.
Check the generated benchmark page from its source template
The new check only reads the committed docs/*.md snapshots, even though benchmarks/page_template.md is the authoritative input that build_page() copies into docs/BENCHMARKS.md during the tag workflow. If that template gains another invalid relative link without simultaneously regenerating the checked-in page, this test passes and the next benchmark publication puts the broken link live—the exact recurrence this test is intended to prevent. Include a page built from the template in the checked inputs and resolve its links as though it were docs/BENCHMARKS.md.
Useful? React with 👍 / 👎.
Test Results (py3.13)969 tests 969 ✅ 3m 10s ⏱️ Results for commit 909f94e. |
Test Results (py3.12)969 tests 969 ✅ 4m 10s ⏱️ Results for commit 909f94e. |
Answering "should those md notes be on GitHub Pages?" — they are, and the page is linked from the index. But two links on it went live as 404s with 0.6.0.
What was wrong
GitHub Pages serves
docs/as the site root, so a relative link out of it cannot resolve:../.github/workflows/benchmark.ymlbenchmarks/Both were mine, and both lived in
benchmarks/page_template.mdrather than in the generated results — so they would have reappeared on every future tag, not just this one. They are now absolute GitHub URLs.What was already fine
I checked every relative link on the live site, not just these. Everything else resolves: Jekyll's
relative-linksplugin rewritesTESTING.mdto/TESTING.htmlby itself,docs/benchmarks/0.6.0.jsonis served as a file, and the four pages all render with the theme despite having no front matter.The check
test_every_relative_link_resolves_inside_the_published_sitevalidates every relative link indocs/*.mdagainst the filesystem — must stay insidedocs/, must exist, must not be a directory without an index. Offline, so no network in CI, and it fails on both links against the previous template.Note
docs/BENCHMARKS.mdis regenerated from the corrected template. The 0.6.0 measurements in it are untouched — the diff is two lines.Targets
maindirectly because the broken links are live now, and because the benchmark workflow already writes this file tomainon every tag.devshould be synced frommainafter this merges.🤖 Generated with Claude Code