feat(scripts): rewrite Sphinx {raw} blocks out of lecture sources - #222
Conversation
mystmd parses `{raw}` but renders no `raw` node, so a block's own source
reaches the reader as escaped text directly under the page title, and the
ipynb export drops it. Three shapes appear across the lecture
repositories; each is now rewritten to its mystmd-native equivalent, or
deleted where it has no reason to exist:
- the notebook logo header, in both `{raw} jupyter` and `{raw} html`
form, is deleted -- the header belongs to the ipynb export rather than
to a copy in every lecture file;
- an Our World in Data chart becomes the `{iframe}` directive;
- a table carrying colspan/rowspan keeps its HTML without the fence,
which mystmd's HTML transform renders as a real table.
Any other `{raw}` block is reported with its file and line, and nothing
is written at all.
The PR preview runs the script over the lecture content it builds, so
previews show post-cutover sources and every theme PR exercises it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UZLpDVYu1YBZHQfwkRRJj7
Each of these made the script either lose content or break its own
fail-loud guarantee, and each now has a test:
- A CRLF source was invisible. The fence pattern ended `(.*)$`, and in
JS `.` excludes CR while an unanchored `$` matches only at end of
input, so no line carrying a directive argument could match. Blocks in
such a file were neither rewritten nor reported, and the run still
exited 0. The pattern now ends `(.*?)\s*$`, and a document is split and
rejoined on its own line endings.
- Unfencing a table could swallow the Markdown after it. An HTML block
runs to the next blank line, where the fence ended the block by itself,
so a heading on the line below `</table>` rendered as literal text and
left the document with no such heading at all. Blank lines are now
guaranteed on both sides.
- The table and iframe shapes were matched on the body's opening
characters and without regard to the format, so a body that merely
began with `<table>` was unfenced whole -- carrying anything that
followed it into the page -- and a `{raw} latex` body could be treated
as HTML. Both now require `html` and a body that is the whole element.
- Recursion descended into `{code-cell}`, `{code-block}` and
`{literalinclude}`, so a `{raw}` block quoted as an example inside one
was rewritten out of the listing. Listings are now stepped over whole,
as unnamed code fences already were, and `~~~` fences are recognised.
- The main-module guard compared a realpath against an unresolved path,
so invoking the script through a symlinked directory skipped `main()`
and exited 0 having done nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UZLpDVYu1YBZHQfwkRRJj7
|
🎭 Visual regression resultsDetails
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
A test fixture uses invalid MyST directive syntax so it doesn't actually exercise the LISTING_DIRECTIVES path it claims to, and the CHANGELOG entry omits the conventional PR link.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR adds scripts/rewrite-raw-blocks.mjs, a standalone Node utility that rewrites Sphinx {raw} directives out of QuantEcon lecture Markdown sources, resolving #204 where those blocks render as escaped HTML source directly under the page title in mystmd. Rather than patching the theme's renderer, the fix normalizes the sources to mystmd-native constructs, and wires the script into preview.yml so every theme PR preview exercises the post-cutover content. The theme itself (app/, styles/, template.yml) is untouched, so no visual baselines move.
Changes:
- New script that deletes the notebook logo header (
{raw} jupyter/{raw} html), converts an Our World in Data<iframe>to the{iframe}directive, unfencescolspan/rowspantables, and fails loud (non-zero exit, nothing written) on any unrecognized{raw}block. - A 20-case unit test suite covering headers, iframe/table conversion, idempotency, nesting, CRLF handling, and code-fence quoting.
preview.ymlruns the script over lecture content betweenmyst initandmyst build, plus a CHANGELOG entry.
File summaries
| File | Description |
|---|---|
scripts/rewrite-raw-blocks.mjs |
New rewriter: fence scanner, recursion into directive bodies, header/iframe/table handling, all-or-nothing writes, symlink-safe main-module guard. |
tests/unit/rewrite-raw-blocks.test.mjs |
20 unit tests for rewriteDocument; one code-listing fixture uses invalid directive syntax that bypasses the intended path. |
.github/workflows/preview.yml |
Adds a step running the script over preview lecture sources before the static build. |
CHANGELOG.md |
Adds an Unreleased "Added" entry; links the issue but not the PR (repo convention links the PR). |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Both #221 and this branch added an `### Added` block directly under `## [Unreleased]`, so the two entries conflicted textually while agreeing in substance. Resolved by keeping both, in the order they landed: the default site footer first, then the rewriter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fixtures wrote each directive's argument inside the braces
(`{code-cell ipython3}`), which the fence pattern cannot read a name
from: `name` came back undefined, so the block was stepped over as an
unnamed code fence and the test passed without ever consulting
LISTING_DIRECTIVES. Removing `code-cell` and `code-block` from the set
left it green, so it guarded nothing for them -- only `literalinclude`,
which carries no argument, was really tested.
Spelled the way MyST spells them, with the argument after the closing
brace, the test now fails when any member is dropped. `{code}`, which
had no case at all, gets one.
Also links the PR alongside the issue in the changelog entry, which is
what CONTRIBUTING.md asks for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#221 and #222 both landed `### Added` entries under `## [Unreleased]` while this was open, which conflicted textually with this branch's `### Changed` entry while agreeing in substance. Resolved by keeping all three, ordered the way Keep a Changelog orders them: Added (the default site footer, the `{raw}` rewriter) before Changed (the breaking launch change). `tests/visual/theme.spec.ts` auto-merged as a union of both sides: the `Site footer` block from #221 and this branch's launch tests are all present, and the unit suite is unchanged from main except for this branch's own rewrite of `launch-urls.test.mjs`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#221, #222 and #224 all landed entries under `## [Unreleased]` while this was open, which conflicted textually with this branch's `### Fixed` entry while agreeing in substance. Resolved by keeping all four, in the order Keep a Changelog uses: Added (default site footer, `{raw}` rewriter), Changed (breaking launch change), then Fixed (output image centring). The Fixed entry also gains its PR link alongside the issue link, which is what CONTRIBUTING.md asks for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#221, #222, #224 and #225 all landed entries under `## [Unreleased]` while this was open. This branch's entry is `### Added` like two of theirs, so it folds into that list rather than forming a second section: Added (default site footer, `{raw}` rewriter, this checklist), then main's Changed and Fixed sections unchanged. The entry also gains its PR link alongside the issue link, which is what CONTRIBUTING.md:225-227 asks for. `docs/configuration.md` auto-merged, and was checked rather than assumed: main's launch rows from #224 and this branch's `project.description`, `project.keywords` and `project.math` rows are all present, with no row duplicated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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>
#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>
#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>
* docs: correct the user docs against what the theme actually does Six documented behaviours the theme does not have, found by the #147 review and verified against the code here: - the toolbar has no site title, and the Home link, the font-size control and the GitHub link went unmentioned; - the "On this page" outline appears from 1280px, not below `lg` (1024px): `.simple-center-grid > .col-margin` is `hidden xl:block`; - `#222` is this theme's dark ground, but it was the book theme's before 0.22.0, and the release the lecture repositories pin has a navy-charcoal palette. Which palette to use is an open design question, so the page no longer presents the two as matching; - `site.title` feeds `og:site_name` and the page-title suffix; the header shows the project title; - nothing limits an output's height, so a long output renders at full length -- the Sphinx scroll-output cell tags have no effect yet; - only `translators_label` is translatable: the names themselves are joined in English on every edition, right-to-left ones included. `docs/index.md` now points its no-counterpart promise at the migration page's table, and no longer presents PLAN.md as the live record. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZLpDVYu1YBZHQfwkRRJj7 * docs(layout): correct which controls survive below md The Header section said the full-screen toggle, the font-size control, downloads, launch and GitHub all move into an overflow menu below `md`. `MobileActionsMenu` renders only `DownloadsButton`, `LaunchButton` and `GitHubButton`; the full-screen toggle and `FontScaleListItems` are `hidden md:block` in `Toolbar.tsx` with no overflow counterpart. So all five leave the toolbar at that width, but only three reappear -- the other two are simply unavailable, which is what the section now says. 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>
Closes #204, implementing the 2026-09-11 decision: the fix leaves the theme, the sources become mystmd-native, and a scripted rewrite runs at cutover.
Per @mmcky's call, the script lives in
scripts/in this repo —preview.ymlis its first user, and the move to the lecture migration tooling stays a file move.Problem, re-verified before the work
Built
lecture-python-programming'snumpy.mdandneed_for_speed.mdagainst a build of main atd7ec6cd./numpyserves<h1>NumPy</h1>followed immediately by<p><div id="qe-notebook-header" align="right"...— the block's own source as the first thing a reader sees — and the build logsunknown format for raw content: jupytertwice. After the rewrite both are gone (2 warnings → 0).The script
scripts/rewrite-raw-blocks.mjs, run asnode scripts/rewrite-raw-blocks.mjs [path...]:{raw} jupyteror{raw} html{raw} htmlholding one<iframe>{iframe}with the src, plus:width: 100%{raw} htmlholding a<table>file:line; nothing is written anywhere and the run exits non-zeroVerified against the four real sources the issue names (
short_path.md,need_for_speed.md,simple_linear_regression.md,opt_transport.md), fetched at the commits in the issue table: 3 headers deleted, 1 iframe converted, 1 table unfenced, and a second run is a no-op.The output was then built and served: the table renders as a real table keeping
colSpan="3"androwspan="2", and the iframe as the directive's responsive box with the Our World in Data src. Nothing is escaped.Adversarial review, and what it found
I ran a four-lens review over the branch (correctness, CI/static builds/BASE_URL, accessibility, regressions), each finding independently refuted by two skeptics. 8 findings survived refutation, reducing to five distinct defects — all real, all now fixed with a test each (second commit):
(.*)$; in JS.excludes CR and an unanchored$matches only at end of input, so no line carrying a directive argument could match. A CRLF lecture file was neither rewritten nor reported, and the run still exited 0 — both halves of the fail-loud guarantee silently void.</table>, the built page had zero<h2>elements — the heading rendered as literal text and vanished from the outline. This was the worst of the five.<table>was unfenced whole, carrying whatever followed into the page; a{raw} latexbody could be treated as HTML.{code-cell}/{code-block}/{literalinclude}, so a{raw}block quoted as an example inside a listing was rewritten out of it. (~~~fences are now recognised too.)main()and exited 0 having done nothing.Not silently capped: 20 further findings were rated low severity or fell outside the five-per-lens verification cap and were not put through refutation. Four findings were verified and refuted (including "the
{iframe}has no accessible name" — the raw markup it replaces had none either, so it is a carried-over gap rather than a regression, and inventing a title for someone's chart is a content decision).Tests
tests/unit/rewrite-raw-blocks.test.mjs— 20 tests, all 51 in the suite green. Covers both header forms, longer fences, the iframe and table conversions, idempotency, an unknown block reported by line, nesting inside{exercise}, a block quoted inside a code fence, and one test per defect above.The code-listing test is mutation-verified, after Copilot found it could not fail (see Review below): deleting any single member of
LISTING_DIRECTIVESnow makes it fail, where previously deletingcode-cellorcode-blockleft it green.CI
preview.ymlruns the script over the lecture content aftermyst initand beforemyst build --html, so previews show post-cutover sources and every theme PR exercises it. A non-zero exit fails the preview rather than deploying a page with literal markup.No pixels move
The theme itself is untouched — no
app/,styles/ortemplate.ymlchanges — so no visual baseline moves,-darwinincluded. Thertl.pngbaselines in the history of this branch arrive with the merge of main described below, not from this work.Merged with main
#221 (the default site footer) landed while this was open and added its own
### Addedblock directly under## [Unreleased], which conflicted textually with this entry while agreeing in substance. Merged main in at2e97723b0, keeping both entries in the order they landed: the footer first, then the rewriter. The branch isMERGEABLE/CLEANand CI is green on the merge.Review
Copilot's two inline comments are addressed in
7aa89ac87and both threads are answered:{code-cell ipython3}), which is not MyST's syntax, soFENCE_OPENcaptured no name and the blocks took the unnamed-code-fence path —LISTING_DIRECTIVESwas never consulted. A genuine defect in the test, not a style point: I confirmed by mutation that removingcode-cellandcode-blockfrom the set left the old test passing. Fixed by spelling the fences as MyST does, and a{code} pythoncase was added becausecodeis the fourth member of the set and had no case at all.pull/NNNlink" is not accurate — nine or more end with an issue link only, including the#203entry that landed from feat(footer): render the licence notice and theme credit by default #221 — so the reply records that the file's practice is mixed and that CONTRIBUTING is what is being followed.Out of scope, flagged not done
The remaining acceptance criteria are consumer-side and outside this repo: removing the header from lecture-wasm's
short_path.md(QuantEcon/lecture-wasm#66), thejb2branch's headers andmyst.ymlsetting (QuantEcon/lecture-python-programming#363), and QuantEcon/mystmd#108 merging. The cross-repo rollout is QuantEcon/workspace-lectures#64.🤖 Generated with Claude Code