chore(comments): code comments explain the code, not its history

.github/workflows/ci.yml

@@ -105,7 +105,7 @@ jobs:
         run: make build-theme
       - name: Install Playwright WebKit
         run: npx playwright install --with-deps webkit
-      - name: Run FOUC guard (QuantEcon/quantecon-theme-src#66)
+      - name: Run FOUC guard
         run: npm run test:fouc
         env:
           THEME_TEMPLATE: ${{ github.workspace }}/.deploy/quantecon-theme

.github/workflows/preview.yml

@@ -1,16 +1,15 @@
 # PR preview on GitHub Pages — self-contained (GITHUB_TOKEN only, no org secrets).
 #
 # Builds a real lecture site (QuantEcon/lecture-python-programming) with the
-# PR's theme via a static `myst build --html` — the production export path the
-# Playwright harness (live `myst start`) never exercises — and deploys it to
-# the gh-pages branch under pr-preview/pr-<n>/. A sticky PR comment links the
-# preview; closing the PR tears the subdirectory down.
+# PR's theme via a static `myst build --html` — the production export path,
+# here run on real lecture content rather than the Playwright fixture — and
+# deploys it to the gh-pages branch under pr-preview/pr-<n>/. A sticky PR
+# comment links the preview; closing the PR tears the subdirectory down.
 #
 # The content repo is a legacy Jupyter Book (_config.yml/_toc.yml): `myst init`
-# performs the JB→MyST upgrade at build time, so the preview doubles as a
-# continuous migration-readiness check for the lecture content (PLAN.md open
-# question 4). Content is cloned at full depth so the git-metadata plugin
-# (plugins/git-metadata.mjs, PR #83) renders real per-page history.
+# performs the JB→MyST upgrade at build time, so every preview also exercises
+# that upgrade on real lecture content. Content is cloned at full depth so the
+# git-metadata plugin (plugins/git-metadata.mjs) renders real per-page history.
 name: PR Preview
 
 on:
@@ -76,8 +75,7 @@ jobs:
           test -f myst.yml
           yq -i '.site.template = env(THEME_DIR)' myst.yml
           yq -i '.project.github = env(CONTENT_GITHUB)' myst.yml
-          # The git-metadata plugin ships with the theme from PR #83 onward;
-          # guard so previews still build on branches that predate it.
+          # Guarded so the preview still builds on a branch without the plugin.
           if [ -f "$PLUGIN" ]; then
             yq -i '.project.plugins = [env(PLUGIN)]' myst.yml
           fi

.github/workflows/release.yml

@@ -1,6 +1,6 @@
 name: Release
 
-# Tag-triggered build → zip → GitHub Release pipeline (PLAN.md Phase 0, #71).
+# Tag-triggered build → zip → GitHub Release pipeline.
 #
 # Pushing a `vX.Y.Z` tag builds the theme, assembles the same bundle that
 # `make build-theme` ships (adapted from upstream jupyter-book/myst-theme's
@@ -10,7 +10,7 @@ name: Release
 #
 # The zip contains:
 #   build/, public/                 (from `npm run prod:build`)
-#   template.yml                    (version stamped from package.json — #71)
+#   template.yml                    (version stamped from package.json)
 #   server.js, package.json, LICENSE, README.md, thumbnail.png, qe-logo.png
 #                                   (from template/, with VERSION substituted)
 #   package-lock.json               (generated; node_modules is NOT shipped)
@@ -19,10 +19,6 @@ name: Release
 # Guards (both fail the release):
 #   - the tag must match `package.json` `version`
 #   - CHANGELOG.md must have a `## [X.Y.Z]` section — it becomes the release notes
-#
-# This replaces the Changesets release.yml (removed in #72). The legacy
-# `make deploy` push to QuantEcon/quantecon-theme remains only until consumers
-# are repointed to pinned release URLs (PLAN.md Phase 0 consumer migration).
 
 on:
   push:
@@ -88,8 +84,8 @@ jobs:
       # the published artifact.
       - run: npm run test:unit
       - name: Build
-        # esbuild occasionally deadlocks (QuantEcon/quantecon-theme-src — known
-        # flake); time out fast so the run can simply be retried.
+        # esbuild occasionally deadlocks: the build hangs rather than failing.
+        # Time out fast so the run can simply be retried.
         timeout-minutes: 20
         run: npm run prod:build
 
@@ -103,7 +99,7 @@ jobs:
           cp -r build "$DIST/build"
           cp CHANGELOG.md "$DIST/CHANGELOG.md"
           # Stamp both version fields from package.json so the shipped bundle
-          # cannot drift (#71).
+          # cannot drift.
           sed -i "s/VERSION/$VERSION/g" "$DIST/package.json"
           sed -E "s/^version: .*/version: $VERSION/" template.yml > "$DIST/template.yml"
           # Lockfile for reproducible consumer installs, without shipping node_modules.

.npmrc

@@ -1,8 +1,8 @@
 # @myst-theme/site (1.3.x) declares a peer dependency on @remix-run/* "^1.19",
 # but this theme deliberately pins Remix to ~1.17.0 to match upstream
-# (jupyter-book/myst-theme themes are still on ~1.17.0). Remix 1.19's client
+# (jupyter-book/myst-theme's themes are on ~1.17.0). Remix 1.19's client
 # hard-reloads when window.__remixContext.url is undefined — which it always is
 # under the mystmd CLI's SSR — producing an infinite reload loop ("flashing")
-# and breaking the in-page outline. 1.17 hydrates gracefully, like v1.1.1.
+# and breaking the in-page outline. 1.17 hydrates gracefully.
 # legacy-peer-deps lets npm install/ci accept the intentional version mismatch.
 legacy-peer-deps=true

CONTRIBUTING.md

@@ -149,6 +149,64 @@ Sphinx-era repos), but the theme treats both styles identically. Making gated
 cells genuinely root-level in the AST would be an upstream mystmd transform
 change, not a theme or content fix.
 
+## Code comments
+
+Comments explain the code as it stands: what it does, and why it is written
+that way when the code alone does not say. How it came to be written belongs
+in git — the commit message, the PR and `CHANGELOG.md` — where it stays
+accurate; a comment that narrates a project phase goes stale once the work is
+done.
+
+This applies to comments in code — including the comments in YAML, shell and
+fixture files. Prose documentation (`README.md`, `docs/`, `PLAN.md`, the test
+suite's own `README.md`) is written for a reader who wants the project's
+history, and keeps it.
+
+- **No project framing.** No phase numbers, `PLAN.md` items or milestones, and
+  no issue number used as a label for the work that produced the code.
+  `git log -L` traces any line back through the PRs that shaped it.
+- **Link an issue only when the thread carries more than the comment can.**
+  An upstream bug being worked around, a field a fork adds, the regression a
+  test exists to catch, the measurements a claim rests on, an open decision
+  that will change the code. State the point itself in the comment as well, so
+  the comment stands on its own if the link rots.
+- **Reasons, not comparisons.** "Matches the Sphinx build" on its own does not
+  tell the next reader what breaks if the value changes. Give the reason: the
+  measurement, the contrast ratio, the layout constraint.
+
+  A constraint from another system *is* a reason, and naming it is not project
+  framing — this theme reproduces the deployed Sphinx lecture sites, so for a
+  value derived by measuring them, that derivation is the reason and belongs in
+  the comment: `/* 14.4px: the lecture builds' .8rem of an 18px root */`. What
+  the rule rules out is the bare comparison that leaves the number unexplained.
+
+  Code ported from another project gets one line of provenance where the port
+  starts.
+
+### Matching the lecture builds, and when not to
+
+Most values in `styles/quantecon.css` are measurements of the deployed Sphinx
+lecture sites, and most inherited colours come from `quantecon-book-theme`'s
+own palettes. The rule the theme follows is: **match them, except where
+matching them would fail WCAG.** Where it diverges it says so, with the
+measurement, so the next reader does not "restore parity" and reintroduce the
+problem. The existing divergences are the model:
+
+- the outline's resting entries stay undimmed at 10.3:1, where the Sphinx panel
+  composites its own to 4.47:1;
+- the outline's active entry carries weight and a rule as well as colour, so the
+  state is not colour-only (WCAG 1.4.1);
+- the dark footer link is blue-300, because the inherited `#0072bc` composites
+  to 2.2:1 at the footer's opacity;
+- content is a flat 18px off a `rem` root, rather than the Sphinx builds'
+  px root, which is a WCAG 1.4.4 problem.
+
+Changing an inherited value for any *other* reason is a two-theme decision, not
+a fix this repo makes alone — it would reintroduce a difference against the live
+sites. Open an issue instead (see #172 for the code palette, #201 for the
+footer). When you compute a contrast ratio, composite any `opacity` on the
+element first: the declared colour is not what the reader sees.
+
 ## Commit Convention
 
 We use conventional commits:

Makefile

@@ -2,8 +2,6 @@
 
 # Releases ship via the tag-triggered GitHub Release workflow
 # (.github/workflows/release.yml) — see CONTRIBUTING.md "Releases".
-# The old `make deploy` flow (pushing the bundle to the now-archived
-# QuantEcon/quantecon-theme build repo) was retired with it.
 
 THEME = quantecon-theme
 VERSION = $(shell cat package.json | jq -r '.version')

app/backend/loaders.server.ts

@@ -134,11 +134,10 @@ export async function getMystSearchJson(): Promise<MystSearchIndex | null> {
 }
 
 /**
- * The QuantEcon lectures favicon (byte-identical to the Sphinx sites'
- * `_static/lectures-favicon.ico`), served when no `favicon` option is set.
+ * The QuantEcon lectures favicon, served when no `favicon` option is set.
  * It lives under public/logos/ rather than at public/favicon.ico: a static
- * file at that path is served ahead of the `[favicon.ico]` route, which is
- * why the `favicon` site option never took effect before #173.
+ * file at that path is served ahead of the `[favicon.ico]` route, so the
+ * `favicon` site option would never take effect.
  */
 const DEFAULT_FAVICON = "public/logos/lectures-favicon.png";
 

app/components/Document.tsx

@@ -34,13 +34,14 @@ import classNames from 'classnames';
  *
  * No Radix `DirectionProvider` is needed: the floating positioning under
  * dropdown menus and tooltips reads the computed CSS `direction`, which
- * `dir` on <html> sets, so they mirror on their own. (A provider was tried
- * and found inert: the theme's copy of `@radix-ui/react-direction` was a
- * different instance from the one the menu and select packages pin, so its
- * context never reached them.)
+ * `dir` on <html> sets, so they mirror on their own. (A provider would not
+ * reach them anyway: in this lockfile `@radix-ui/react-menu`,
+ * `react-roving-focus` and `react-select` each nest their own copy of
+ * `@radix-ui/react-direction`, so a context from any other copy is invisible
+ * to them.)
  *
  * Everything else is upstream's, unchanged, so a future upstream `lang` /
- * `dir` prop (see UPSTREAM-CANDIDATES.yml) lets this file be deleted again.
+ * `dir` prop (see UPSTREAM-CANDIDATES.yml) lets this file be deleted.
  * Kept in step with @myst-theme/site 1.3.0.
  */
 export function Document({

app/components/Outline.tsx

@@ -18,18 +18,17 @@ export function BackToTop() {
             }
           )}
         >
-          {/* Label matches the Sphinx build's back-to-top button exactly:
-              U+2191 + space + "Top". `aria-label` is deliberately kept -- the
-              Sphinx original leans on `title` alone, which screen readers
-              announce inconsistently, so the visible label can shorten without
-              the accessible name going with it.
+          {/* Visible label: U+2191 + space + "Top". `aria-label` is kept
+              deliberately -- `title` alone is announced inconsistently by
+              screen readers, so the visible label can stay short without the
+              accessible name going with it.
               A plain fragment link, not the provider `Link`: this control is
-              rendered server-side, and the provider resolved `#top` against
-              the un-slashed SSR pathname while the outline (built after mount)
-              resolved against the slashed one -- so on the deployed lecture
-              sites "Top" was the one in-page control that left the document
-              (301 + full reload, #186). A bare `href="#top"` is what the Sphinx
-              build renders and never needs path resolution. */}
+              rendered server-side, where the provider would resolve `#top`
+              against the un-slashed SSR pathname rather than the slashed one
+              the outline (built after mount) sees -- so on a static host that
+              redirects to the slashed URL, "Top" would leave the document
+              (301 + full reload) instead of scrolling. A bare `href="#top"`
+              never needs path resolution. */}
           <a href="#top" title="Back to top" aria-label="Back to top">
             ↑ Top
           </a>
@@ -40,34 +39,28 @@ export function BackToTop() {
 }
 
 /**
- * The right-hand "On this page" panel (#182). Pinned, scroll-tracking and
- * nested, matching the Sphinx build's `sticky_contents` panel:
+ * The right-hand "On this page" panel -- pinned, scroll-tracking and nested:
  *
- *  - The current entry is chosen by `useActiveHeading`, a port of the Sphinx
- *    build's scrollspy rule (the last heading whose top has passed 120px
- *    from the viewport top; the final one within 50px of the page bottom),
- *    rather than by `useHeaders`' own `activeId`. That id is computed from an
- *    IntersectionObserver pass against the theme's navbar offset, and in this
- *    theme the hook reads that offset as 0 (measured on the fixture: the
- *    topmost heading in the upper third wins, even one hidden under the
- *    navbar), so the deployed sites' rule is both more predictable and the
- *    parity target.
+ *  - The current entry is chosen by `useActiveHeading` (the last heading whose
+ *    top has passed 120px from the viewport top; the final one within 50px of
+ *    the page bottom) rather than by `useHeaders`' own `activeId`. That id is
+ *    computed from an IntersectionObserver pass against the theme's navbar
+ *    offset, and in this theme the hook reads that offset as 0 (measured on the
+ *    fixture: the topmost heading in the upper third wins, even one hidden
+ *    under the navbar), so a fixed scroll offset is the more predictable rule.
  *  - Pinning is `position: fixed` in styles/quantecon.css (`.qe-outline`), not
  *    `sticky`: the wrapper is `self-start`, so a sticky child would have zero
  *    travel, and the grid declares no rows for it to span. `left`/`right` stay
  *    `auto`, so the panel keeps its static position in the margin track.
- *  - h3 entries nest under their h2 and collapse to the active branch, as the
- *    Sphinx panel does under `contents_autoexpand` (on by default there and
- *    on every lecture site): at the top of the page only the sections show;
- *    scrolling into a section expands its subsections, the current one
- *    (section or subsection) is marked, and the parent of a current
- *    subsection is expanded but not marked. Past `max-height` the panel
- *    scrolls internally.
+ *  - h3 entries nest under their h2 and collapse to the active branch: at the
+ *    top of the page only the sections show; scrolling into a section expands
+ *    its subsections, the current one (section or subsection) is marked, and
+ *    the parent of a current subsection is expanded but not marked. Past
+ *    `max-height` the panel scrolls internally.
  *  - Enumerators come from the heading itself (`span.select-none`, "3.1"),
- *    plus the period the h1 and the Sphinx panel use -- never a number
- *    computed from the list index, which was wrong as soon as h3s existed.
- *  - The logo sits below the list and above "Powered by", where the Sphinx
- *    panel keeps it (decided on review of #196); smaller, per #96.
+ *    plus the period the h1 uses -- never a number computed from the list
+ *    index, which miscounts as soon as h3s are listed.
+ *  - The logo sits below the list and above "Powered by".
  */
 export function Outline({
   containerClassName,
@@ -81,8 +74,8 @@ export function Outline({
   const { headings } = useHeaders('main h2, main h3', 3);
   const currentId = useActiveHeading(headings);
   const tree = nest(headings);
-  // Sphinx's autoexpand: the current item's own sub-list, and every ancestor
-  // of the current item, are expanded; nothing else is.
+  // The current item's own sub-list, and every ancestor of the current item,
+  // are expanded; nothing else is.
   const expandedId = tree.find(
     (branch) => branch.id === currentId || branch.children.some((c) => c.id === currentId)
   )?.id;
@@ -162,8 +155,8 @@ function Entry({
   currentId?: string;
   Link: ReturnType<typeof useLinkProvider>;
 }) {
-  // The heading's own enumerator ("3.1"), plus the period the h1 and the
-  // Sphinx panel use; the bare title when the project sets no numbering.
+  // The heading's own enumerator ("3.1"), plus the period the h1 uses; the
+  // bare title when the project sets no numbering.
   const enumerator = heading.element.querySelector('span.select-none')?.textContent?.trim();
   return (
     <Link to={`#${heading.id}`} aria-current={heading.id === currentId ? 'location' : undefined}>
@@ -172,17 +165,17 @@ function Entry({
   );
 }
 
-/** Sphinx's `scrollspy.js` constants: a section is current once its heading
- *  has passed this many px from the top; the last section is forced current
- *  this close to the page bottom. */
+/** From quantecon-book-theme's `scrollspy.js`, as is `useActiveHeading`: a
+ *  section is current once its heading has passed this many px from the top;
+ *  the last section is forced current this close to the page bottom. */
 const ACTIVATION_OFFSET_PX = 120;
 const BOTTOM_THRESHOLD_PX = 50;
 
 /**
- * The id of the heading the reader is in, by the Sphinx rule: the last heading
- * whose top is at or above the activation line, or the last heading of all
- * once the page is scrolled to its bottom. Nothing is current above the first
- * heading. Recomputed on scroll and resize, one read per animation frame.
+ * The id of the heading the reader is in: the last heading whose top is at or
+ * above the activation line, or the last heading of all once the page is
+ * scrolled to its bottom. Nothing is current above the first heading.
+ * Recomputed on scroll and resize, one read per animation frame.
  */
 function useActiveHeading(headings: { id: string; element: HTMLElement }[]) {
   const [currentId, setCurrentId] = useState<string | undefined>(undefined);

app/components/PageContent.tsx

@@ -37,9 +37,9 @@ export const PageContent = React.memo(function ({ article }: { article: PageLoad
   const keywords = article.frontmatter?.keywords ?? [];
   const parts = extractKnownParts(tree, article.frontmatter?.parts);
   const projectParts = config?.parts ?? {};
-  // Translator credit (#143): the site option, overridden per page under
-  // `site:` in the page's frontmatter (replace, never merge; an explicit empty
-  // value suppresses). See app/i18n.ts for the rules and the string form.
+  // Translator credit: the site option, overridden per page under `site:` in
+  // the page's frontmatter (replace, never merge; an explicit empty value
+  // suppresses). See app/i18n.ts for the rules and the string form.
   const siteOptions = ((config as any)?.options ?? {}) as Record<string, unknown>;
   const pageOptions = ((article.frontmatter as any)?.site ?? undefined) as
     | Record<string, unknown>