feat(compute): per-lecture live compute via an enable_live_compute site option - #195
feat(compute): per-lecture live compute via an enable_live_compute site option#195mmcky wants to merge 4 commits into
Conversation
`project.thebe` turns in-page compute on for a whole project, but Pyodide cannot run every lecture (numba and JAX do not import), so a `live_compute` option, declared in template.yml, gates the control per page. Resolution is the page's `site:` value, then the site-wide option, then on: absent means today's behaviour, so existing projects change nothing and a series adopts the flag by marking known-incompatible lectures false; a series that would rather certify lectures one at a time sets it false site-wide and opts pages in. The gate is broad. Page.tsx turns the resolved value into the `optionOverrideFn` of ComputeOptionsProvider, which flips `compute.enabled` and with it the toolbar toggle, the error tray and the execute scope in one place: "not compatible" means nothing on the page should try to run. The resolver is a pure module (app/liveCompute.ts) with node unit tests. The RTL fixture carries the Playwright cases -- project thebe on, `live_compute: false` site-wide, notebook.ipynb inheriting it and notebook-live.ipynb opting back in via notebook metadata -- because its only snapshot is the landing page, so the extra pages move no baseline; the main fixture's existing toggle test is the no-flag case. Closes #114 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
🎭 Visual regression results
Details
Skipped testsmobile-chrome › theme.spec.ts › QuantEcon theme — visual regression › without JavaScript › drawer-opens-without-javascript |
|
@DrDrij this is not a parity feature -- so I will leave this open and do a detailed review of this next week. I have ideas about the yaml headers and will make some adjustments. |
There was a problem hiding this comment.
🔵 Needs a closer look
It changes compute-surface gating for every lecture page in the upcoming all-or-nothing cutover, and the load-bearing upstream contract (optionOverrideFn returning undefined ⇒ compute.enabled === false) could not be independently verified in this environment.
Pull request overview
This PR adds a per-lecture live_compute site option (closes #114) that gates the in-page Thebe/Pyodide compute control on a per-page basis. project.thebe still enables compute for the whole project, but because Pyodide cannot run every lecture (numba, JAX, etc.), a page or the site can now turn the control off. It builds on #194, which made declared page-level site: keys survive validation, and slots into the #147 book-theme parity cutover as migration-readiness work.
Changes:
- New pure resolver
app/liveCompute.ts(resolveLiveCompute): page value → site value → defaulttrue, with lenient string parsing; wired intoPage.tsxasComputeOptionsProvider'soptionOverrideFn(a memoised callback that returnsundefinedto disable the whole compute surface — the broad gate). - Declares
live_computeas a boolean option intemplate.yml, types it inapp/types.ts, and documents it inREADME.md/CHANGELOG.md. - Adds 5 unit tests plus a
live-compute-per-pagevisual test with two new RTL-fixture notebooks (inherit-off vs opt-in) and fixture/README wiring.
File summaries
| File | Description |
|---|---|
app/liveCompute.ts |
New precedence resolver (page → site → on) with lenient boolean parsing |
app/components/Page.tsx |
Wires resolved value into ComputeOptionsProvider.optionOverrideFn, memoised |
app/types.ts |
Adds live_compute?: boolean to TemplateOptions |
template.yml |
Declares the live_compute boolean option so it reaches the theme |
tests/unit/live-compute.test.mjs |
Unit coverage for precedence and string spellings |
tests/visual/theme.spec.ts |
live-compute-per-page desktop test for inherit-off and opt-in |
tests/visual/fixture-rtl/* |
New notebooks + myst.yml.in carrying the per-lecture gate cases |
README.md |
"Per-lecture live compute" subsection and site-options row |
CHANGELOG.md |
Unreleased/Added entry (contains an incorrect phase label) |
tests/visual/README.md |
Notes the RTL fixture now carries the gate cases |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ict: keep both entries)
`live_compute` read as a setting that might take a mode value, and a name with "browser" or "pyodide" in it was considered and rejected because the gate wraps whichever backend `project.thebe` configures (lite, binder or server). `enable_live_compute` keeps the `enable_` prefix the option table already uses (`enable_rtl`), so it is visibly a boolean, and keeps "live compute", the term the README, changelog and header control already use. Also documents the option in the docs/ feature reference that landed in main via #197, and drops the "Phase 4" label from the changelog entry: this is not a parity-plan item (mmcky, PR #195 discussion), and Copilot noted that Phase 4 is internationalisation in any case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
mmcky
changed the title
The changelog entry for #114 merged without conflict into the now-frozen 2.7.0 section; moved it back under Unreleased, since this ships in the release after 2.7.0. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
quantecon-services
left a comment
quantecon-services
left a comment
There was a problem hiding this comment.
Review: per-lecture live compute (enable_live_compute)
Verdict: technically ready to merge. Nothing blocking. Two non-blocking nits inline, and one design constraint worth settling before the planned YAML-header adjustments.
Verified locally (head 17a87f8)
npm ci, thennpm run test:unit(38 pass, including the 5 new resolver tests) andtsc --noEmit(clean).- The upstream contract Copilot could not verify holds. In the installed
@myst-theme/jupyter1.3.0,ComputeOptionsProvidercomputesenabled: !!optionsWithOverrides, so anoptionOverrideFnreturningundefineddoes disable compute. ItsuseMemoalso depends on the override function's identity, so theuseCallbackwrap inPage.tsxis necessary, not just tidy. - A benefit the description does not mention: the override also blanks
compute.thebe, soThebeLoaderAndServergetsloadThebeLite: falseanduseJupyterLite: falseon a gated page. Gated lectures never download the Thebe or Pyodide bundle at all. Worth one line in the README /docs/notebooks.md. - CI is green on all four checks; the 13 visual skips are the expected mobile-only exclusions.
Design constraint: keep the per-page key under site:
Since the YAML-header shape is going to be revisited: myst-frontmatter's validatePageFrontmatterKeys drops unknown top-level keys from page frontmatter, and site is the only declared pass-through to template options. A flat top-level enable_live_compute: in a lecture's frontmatter (or in notebook metadata) would be silently discarded before it reaches the theme. Any reshaping needs to stay inside the site: block unless it comes with an upstream change or a plugin transform that lifts the key into site.
Other non-blocking notes
- Static-build coverage.
live-compute-per-pageruns against themyst startserver only; thestatic-chromeproject does not exercise the gate. Page frontmatter reaches the static page JSON by the same path and the existing toggle tests share the gap, so this is consistent rather than a defect. - Flags are backend-specific.
falsemeans "does not run under Pyodide". If a series later switchesproject.thebetobinder:orserver:, those flags become wrong in the permissive direction. The naming decision already acknowledges this; the lecture-repo certification job should record results per backend. - Inline: the lenient
yes/no/on/offparsing is unreachable behind the CLI's boolean validation, and the twoas anycasts inPage.tsxcan go with a narrower parameter type on the resolver.
Generated by Claude Code
| function asBoolean(value: unknown): boolean | undefined { | ||
| if (typeof value === 'boolean') return value; | ||
| if (typeof value === 'string') { | ||
| const v = value.trim().toLowerCase(); | ||
| if (v === 'true' || v === 'yes' || v === 'on') return true; | ||
| if (v === 'false' || v === 'no' || v === 'off') return false; | ||
| } |
There was a problem hiding this comment.
Nit, non-blocking: the yes/no/on/off branches are unreachable in practice. The option is declared type: boolean in template.yml, and the CLI's validateBoolean (simple-validators) accepts only true/false (case-insensitive) and rejects everything else with a validation error before the theme sees it. The page-level site: block goes through the same validation (that is what #194 relies on), so a hand-edited enable_live_compute: yes is dropped upstream, not parsed here.
Either trim asBoolean to true/false and drop the matching assertions in tests/unit/live-compute.test.mjs, or keep it and note in the comment that it only matters for a value that bypasses template validation.
Generated by Claude Code
| // broad gate, since "not compatible" means nothing on the page should run. | ||
| // Memoised: the provider recomputes its options whenever this identity | ||
| // changes. | ||
| const liveCompute = resolveLiveCompute(pageDesign as any, siteDesign as any); |
There was a problem hiding this comment.
Nit, non-blocking: both as any casts go away if resolveLiveCompute takes Partial<TemplateOptions> | undefined (or { enable_live_compute?: unknown } | undefined) instead of Record<string, unknown>. TemplateOptions has no index signature, which is the only reason the cast is needed. No behaviour change, and asBoolean already accepts unknown.
Generated by Claude Code
Closes #114. Ships in the release after v2.7.0 (the changelog entry is under Unreleased; main was merged in after the v2.7.0 tag).
Summary
project.thebeturns in-page live compute on for a whole project, but Pyodide cannot run every lecture (numba and JAX do not import). This PR adds a per-lecture switch: a boolean site option,enable_live_compute, declared intemplate.ymlso the MyST CLI passes it through at both the site and page level. It builds on #194, which made a page-levelsite:block keep its other declared keys.Configuration
Thebe stays the outer switch: without
project.thebethere is no compute anywhere and the new option does nothing.Site-wide default goes under
site.optionsinmyst.yml. This example is the certify-one-lecture-at-a-time story: everything is off unless a page opts in.Leaving the option out of
myst.ymlgives the other story: every page is on, and only the incompatible lectures are marked off.Per-lecture value goes under
site:in the lecture's frontmatter:For an
.ipynblecture the same key goes in the notebook-level metadata:Resolution (in
app/liveCompute.ts, a pure module): page value, then site value, then on.site.enable_live_computesite.options.enable_live_computeinmyst.ymlproject.thebeis configured, exactly as todayDecisions
Name. First shipped as
live_compute, renamed after review. Theenable_prefix is the one the option table already uses (enable_rtl), so the key is visibly a boolean, and "live compute" is the term the README, changelog and header control already use. Names containing "browser" or "pyodide" were rejected because the gate wraps whichever backendproject.thebeconfigures (lite, binder or server), so they would go stale with #26 or #154.Default when absent: on. Existing projects change nothing, and a series adopts the flag incrementally by marking known-incompatible lectures
false. The inverse story (site-widefalse, opt pages in) is available without a schema change.Gate: broad.
Page.tsxturns the resolved value into theoptionOverrideFnofComputeOptionsProvider; returningundefinedmakes the provider reportenabled: false(verified against the installed@myst-theme/jupyter, which computesenabled: !!optionsWithOverrides). That takes the toolbar toggle, the error tray and the execute scope down together: "not compatible" means nothing on the page should try to run. Zero upstream changes.Certification stays in the lecture repos. The flag should be written from a per-lecture pass/fail record under the Pyodide kernel and re-run when package usage changes. The theme cannot do that.
Placement of the control is unchanged; per the 2026-09-10 decision on #128, that review continues as design iteration under #187. This is not a parity-plan item: the book theme has one project-wide Thebe flag with no per-page control.
Tests
live-compute-per-page(desktop-chrome) on the RTL fixture, which now hasproject.thebeandenable_live_compute: falsesite-wide:/notebookshows no control and an empty compute slot;/notebook-live(metadatasite: {enable_live_compute: true}) shows the toggle in the header slot.live-compute-toggletest is the no-flag case, andlive-compute-toggle-absent-without-thebestill provesproject.thebeis the outer gate.Docs
README gains a "Per-lecture live compute" subsection under Live compute and a row in the site-options table. The
docs/feature reference from #197 gets the same row inconfiguration.mdand a matching subsection innotebooks.md. CHANGELOG entry under Unreleased, Added.🤖 Generated with Claude Code