feat(seo): emit a canonical link, and base-URL-aware head links - #227
Merged
Merged
Conversation
Every page now carries `<link rel="canonical">`, built from `site_url` as the Sphinx lecture sites build theirs from `html.baseurl`. Two rules apply, and `og:url` is built by the same function so the two cannot disagree: - the home page's canonical is the site root. With a base URL, the export renders the root index.html by requesting the index slug, so the page's render-time path is that slug -- and the slug's own URL is not served, so naming it would point every home page at a 404; - every URL takes the trailing-slash form, which is what the export writes and what a host redirects the slashless form to, so no canonical names a redirect. Nothing is emitted without `site_url`, as Sphinx emits nothing without `html_baseurl`. The page path is stripped of the base before the base is re-applied: the browser router has no basename, so on the client the location already carries it. Two head links assumed the domain root and 404ed on a site served under a sub-path, which is how lecture-wasm is deployed. The favicon and `/myst-theme.css` now carry the base URL. Neither can come from a route's `links()`, which takes no arguments in Remix 1.17 while BASE_URL reaches the app only through the root loader, so the local Document emits them; the root route keeps a root-absolute icon as the fallback for the error boundary upstream renders with its own Document. `og:image` is made absolute against the site origin, which a social scraper needs to fetch it at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZLpDVYu1YBZHQfwkRRJj7
- On a site whose projects carry a slug, every project home page was canonicalised onto a URL the build never writes. The home test compared the bare index slug against the whole path, which can only match a project at the site root; with a base URL the export renders a project's index.html by requesting `<projectSlug>/<indexSlug>`, so the test failed and the page named `<projectSlug>/<indexSlug>/` -- a 404 -- as its canonical. The project slug is now part of the test, and such a page resolves to the project root, which is where the build serves it. - A page whose own slug equalled the base-URL segment was canonicalised onto the home page. The base was stripped on an exact match as well as on a `<base>/` prefix, and at render time the path carries no base at all, so `/notebook` under `BASE_URL=/notebook` looked like the site root. Only a real prefix is stripped now, which leaves the client path (which does carry the base) resolving to the same URL. - `absoluteImage` corrupted a protocol-relative image URL: `//host/x.png` begins with a slash but names its own host, and the site origin was prefixed to it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZLpDVYu1YBZHQfwkRRJj7
Contributor
|
Contributor
🎭 Visual regression results
Details
Skipped testsmobile-chrome › theme.spec.ts › QuantEcon theme — visual regression › without JavaScript › drawer-opens-without-javascript |
There was a problem hiding this comment.
🟡 Changes recommended
Document.tsx uses baseurl verbatim in href construction, which can generate double slashes when BASE_URL has a trailing slash (e.g. /lecture-wasm/).
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR implements issue #207 by making SEO head output consistent and correct across normal and BASE_URL (sub-path) static builds, adding a canonical URL signal and ensuring critical head links resolve under sub-path hosting.
Changes:
- Add a
pageUrl()builder and emit<link rel="canonical">on pages whensite_urlis set, keeping it aligned withog:url. - Make
og:imageabsolute (when possible) and harden image URL handling (including protocol-relative URLs). - Move base-URL-sensitive head links (favicon +
/myst-theme.css) into the localDocumentwhereBASE_URLis available, and extend unit/visual tests plus docs/changelog accordingly.
File summaries
| File | Description |
|---|---|
| tests/visual/theme.spec.ts | Adds visual assertions for trailing-slash og:url, canonical link presence, and the no-site_url behavior. |
| tests/unit/seo.test.mjs | Adds unit coverage for canonical/page URL construction, base URL edge cases, and absolute image handling. |
| template.yml | Updates site_url option description to include canonical + absolute og:image behavior. |
| README.md | Documents canonical/og:url coupling, trailing-slash canonical form, and unset site_url behavior. |
| docs/configuration.md | Updates site_url docs to reflect canonical link + og:url. |
| CHANGELOG.md | Records Added/Fixed entries for canonical link and base-URL-aware head links. |
| app/seo.ts | Introduces pageUrl, canonicalLink, absoluteImage; updates socialMetaTags to use url and absolute images. |
| app/routes/$.tsx | Computes a public url via pageUrl, uses it for og:url, and emits canonical link. |
| app/routes/_index.tsx | Same as article route: derives url, uses it for og:url, and emits canonical link. |
| app/root.tsx | Removes /myst-theme.css from links() (now emitted by Document); documents error-boundary icon fallback rationale. |
| app/components/Document.tsx | Emits base-URL-aware favicon and /myst-theme.css from the local Document where baseurl is available. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
mmcky
added a commit
that referenced
this pull request
Sep 12, 2026
The Site URL section read as though `site_url` is what makes the theme emit a canonical link. `site_url` has existed since 2.7.0, but no page carries a canonical link on main at all: that is #207, implemented by #227, which is still open. Without this note the section describes a feature the theme does not have, and it would only become true if this branch merged after that one -- a doc should not depend on merge order to be accurate. The wording follows the two callouts already in the file, which flag QuantEcon/mystmd#108 for the notebook header setting and QuantEcon/mystmd#113 for build-time redirects. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mmcky
added a commit
that referenced
this pull request
Sep 12, 2026
…#226) * docs: add a migration checklist for repos moving off the Sphinx theme A lecture repository leaving `quantecon-book-theme` has to set up several things its Sphinx build handled another way, and no page listed them as steps. Three came with the Sphinx theme and needed no per-repo setup at all; the rest are configured today under `_config.yml` keys this theme does not read. `myst init` carries none of them across: it reads the analytics ID only from the `html` section, takes `project.github` only from a top-level `repository.url`, and never reads `sphinx.config`, which is where the lecture configs keep most of these. The page also carries the table of Sphinx theme options with no counterpart here, each with its reason, and `docs/index.md` now points its "where a Sphinx option has no counterpart" promise at that table rather than at the individual pages. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZLpDVYu1YBZHQfwkRRJj7 * docs: leave the no-counterpart sentence to the docs review The migration page's table is where that promise now points, but the sentence also carries the PLAN.md reference the docs review rewrites in the same paragraph. Changing it in both places would only collide, so this keeps to the Pages-table row its own acceptance criteria name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZLpDVYu1YBZHQfwkRRJj7 * docs(migrating): mark the canonical link as not shipped yet The Site URL section read as though `site_url` is what makes the theme emit a canonical link. `site_url` has existed since 2.7.0, but no page carries a canonical link on main at all: that is #207, implemented by #227, which is still open. Without this note the section describes a feature the theme does not have, and it would only become true if this branch merged after that one -- a doc should not depend on merge order to be accurate. The wording follows the two callouts already in the file, which flag QuantEcon/mystmd#108 for the notebook header setting and QuantEcon/mystmd#113 for build-time redirects. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Matt McKay <mmcky@users.noreply.github.com>
#221, #222, #224, #225 and #226 all landed entries under `## [Unreleased]` while this was open. This branch adds one `### Added` entry and one `### Fixed` entry, so each folds into the matching section rather than opening a second: Added (default site footer, `{raw}` rewriter, migration checklist, this canonical link), Changed (the breaking launch change), Fixed (output image centring, these base-URL head links). Both entries also gain their PR link beside the issue link, which is what CONTRIBUTING.md:225-227 asks for. `README.md`, `docs/configuration.md`, `template.yml` and `tests/visual/theme.spec.ts` auto-merged. Each was checked rather than assumed: both sides' content is present in all four. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A trailing slash on `BASE_URL` doubled the separator in every href built
from it. `Document` builds `${baseurl}/favicon.ico` and
`${baseurl}/myst-theme.css`, so `/lecture-wasm/` produced
`/lecture-wasm//favicon.ico` -- and the same unnormalised value reaches
`BaseUrlProvider`, so every link `withBaseurl()` builds carried it too.
Whoever deploys the site writes `BASE_URL` by hand, so both spellings
arrive.
`normalizeBaseurl` trims it and strips trailing slashes once, in the root
loader where the value enters, and `pageUrl` calls it instead of
repeating the rule -- so the canonical URL and the head links cannot
disagree about the shape of the base.
The helper lives in `app/seo.ts` rather than `root.tsx` because a `.tsx`
module cannot be imported by the unit suite: Node strips types but not
JSX. Mutation-tested: with the trailing-slash strip removed the new test
fails, and it passes again with the strip restored.
Also drops the migration doc's "Not shipped yet" note for the canonical
link, which this branch is what ships.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mmcky
added a commit
that referenced
this pull request
Sep 12, 2026
#221, #222, #224, #225, #226 and #227 all landed entries under `## [Unreleased]` while this was open. This branch adds one `### Added` entry, so it folds into that list rather than opening a second section, and it gains its PR link beside the issue link, which is what CONTRIBUTING.md:225-227 asks for. `docs/layout.md`, `styles/quantecon.css` and `tests/visual/theme.spec.ts` auto-merged. Each was checked rather than assumed: the branch's h4 indent rule sits beside main's output-image centring rule, the branch's outline assertions beside main's footer, launch, centring and canonical tests, and the branch's h2-to-h4 documentation beside main's default-footer section. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mmcky
added a commit
that referenced
this pull request
Sep 12, 2026
#221, #222, #224, #225, #226, #227 and #228 all landed `[Unreleased]` entries while this was open. This branch adds a `### Fixed` entry, so it folds into main's existing Fixed section (output image centring, base-URL head links) rather than opening a second one, and it gains its PR link beside the issue link per CONTRIBUTING.md:225-227. `docs/configuration.md`, `docs/index.md` and `docs/layout.md` auto-merged, and each was checked rather than assumed: layout.md holds this branch's Header and Widths rewrites beside #228's h2-to-h4 outline text and #221's default-footer section; index.md holds #226's Pages-table row beside this branch's rewritten closing paragraph; configuration.md holds the rows from all four PRs that touched its table. Co-Authored-By: Claude Opus 5 (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.
Closes #207, implementing the 2026-09-11 decision with the widened scope.
Problem, re-verified on main @ d7ec6cd
No page emitted a
<link rel="canonical">at all, and on aBASE_URLbuild the favicon and/myst-theme.csshrefs resolved to the domain root — which is why lecture-wasm's pages link tohttps://quantecon.github.io/favicon.ico(404) while the icon is served at/lecture-wasm/favicon.ico.Change
site_url.og:urlis built by the same function (pageUrl), so the two cannot drift apart.index.htmlby requesting the index slug, so the render-time path is that slug — and the slug's own URL is not served.site_url, as Sphinx emits nothing withouthtml.baseurl./myst-theme.csscarry the base URL. Neither can come from a route'slinks()(no arguments in Remix 1.17, andBASE_URLreaches the app only via the root loader), so the localDocumentemits them.og:imageabsolute against the site origin.Verified on real static builds
BASE_URL=/lecture-wasm myst build --html, inspecting the generated HTML:https://example.org/lecture-wasm/intro/directory existsmyst-theme.csshref includes the base, file existsog:urlequals the canonicalsite_urlunset → no canonical, noog:urlAdversarial review, and what it found
Four lenses (correctness, static builds/BASE_URL, error pages/hydration, SEO semantics), each finding refuted by two independent skeptics. 4 findings survived, reducing to three distinct defects — all fixed in the second commit:
slug:, every project home page was canonicalised onto a URL the build never writes. My home test compared the bare index slug against the whole path, which can only match a project at the site root. I reproduced it: a two-project site built withBASE_URL=/lecture-wasmwritesalpha/index.htmlbut emittedhttps://example.org/lecture-wasm/alpha/index/— and/lecture-wasm/alpha/index/404s. The project slug is now part of the test, and such a page resolves to the project root. Re-verified on the same fixture:alpha/andbeta/, both of which the build writes.<base>/prefix, and at render time the path carries no base — so/notebookunderBASE_URL=/notebooklooked like the site root. Only a real prefix is stripped now; verified that the page gets/notebook/notebook/while the home still gets/notebook/, and that the client path (which does carry the base) resolves to the same URL.absoluteImagecorrupted a protocol-relative URL://assets.example.org/og.pngbegins with a slash but names its own host, and the origin was prefixed to it.Six findings were verified and refuted — including four claiming the hreflang alternates now contradict the canonical. Worth noting since it was raised repeatedly and rejected each time:
hreflangLinksbuilds per-edition URLs from thelanguagesoption, which is a separate concern from the canonical's own origin, and the skeptics could not show a real disagreement on a built page.Not silently capped: 11 further findings were low severity or over the five-per-lens verification cap and were not put through refutation.
Tests
tests/unit/seo.test.mjs— 42 green, including one test per defect above: the project-home case, the base-equals-slug collision in both path forms, and the protocol-relative image. Plussocial-metanow asserts the canonical link and the home page's root canonical, and a newno-canonical-without-site-urlcovers the unset case.No pixels move
Only
<head>content changes. The full suite shows the same locally-drifting baselines that drift on unmodified main, and nothing else — no baseline refresh needed,-darwinincluded.One nuance worth your eye
On a
BASE_URLsite an error page's icon link is still the root-absolute/favicon.ico, so it still 404s there. The criterion is only that error pages declare an icon link, and they do. Fixing it properly means the rootErrorBoundary— which is upstream'sAppErrorBoundary, rendering upstream's ownDocumentwith no base URL — so it is an upstream change, not one this PR can make. Say the word and I'll file it.🤖 Generated with Claude Code
https://claude.ai/code/session_01UZLpDVYu1YBZHQfwkRRJj7
Generated by Claude Code