Skip to content

Build strict: enable -W in ci.yml and publish.yml#14

Merged
mmcky merged 2 commits into
mainfrom
enable-strict-build
Jul 21, 2026
Merged

Build strict: enable -W in ci.yml and publish.yml#14
mmcky merged 2 commits into
mainfrom
enable-strict-build

Conversation

@mmcky

@mmcky mmcky commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Both builds carried # TODO: Re-enable -W flag once all lectures are translated and warnings are resolved. That precondition is already met — this edition has 26 of 26 lectures with a _toc.yml byte-identical to source, so there are no untranslated lectures left to produce dangling cross-references. The comment was stale.

Why strict is the right default for a translated edition

A translated edition carries the same code as its English source, so a warning here that does not appear upstream is evidence of a translation defect, not noise. That makes -W a translation-quality signal rather than mere build hygiene.

It is also the last line of defence against a class of silent engine corruption. action-translation#118 (a resync wrapped an entire 1,465-line document in a code fence and mis-derived its title from a Python tuple-unpacking line) and #119 (a {raw} jupyter directive argument stripped to {raw}) both reported success at run time, looked plausible in review, and surfaced weeks later only because a downstream repo built with -n -W. Without the flag this edition would merge that class of damage green.

Estate context

Before this PR, strict-build coverage was 3 of 5 editions — the three zh-cn editions had -n -W, while .fa and .fr did not. This closes .fr; the .fa half is in QuantEcon/lecture-python-programming.fa#137.

Verification

This PR is its own test. The change is empirical: CI here is what tells us whether the edition builds clean under -W. If warnings appear, each is either a translation defect (fix it) or a genuine false positive (waive it deliberately) — either outcome is information worth having before merge.

Part of Stage 1 of the translation program work plan. See QuantEcon/project-translation#9.

🤖 Generated with Claude Code

Both builds carried `# TODO: Re-enable -W flag once all lectures are
translated and warnings are resolved`. The precondition is already met:
this edition has 26 of 26 lectures with a `_toc.yml` identical to
source, so there are no untranslated lectures left to produce dangling
cross-references.

The case for strict is stronger than build hygiene. A translated
edition carries the same code as its English source, so a warning here
that does not appear upstream is evidence of a translation defect, not
noise — which makes -W a translation-quality signal.

It is also the last line of defence against a class of silent engine
corruption. QuantEcon/action-translation#118 (a resync wrapped an
entire 1,465-line document in a code fence and mis-derived its title
from a Python tuple-unpacking line) and #119 (a `{raw} jupyter`
directive argument stripped to `{raw}`) both reported success at run
time, looked plausible in review, and surfaced weeks later only
because a downstream repo built with -n -W. Without the flag this
edition would merge that class of damage green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 21, 2026 00:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR makes the French edition’s CI and publish workflows treat Sphinx/Jupyter Book warnings as errors by enabling the -W flag during jb build, ensuring translation-induced warnings fail the build.

Changes:

  • Enable strict warning handling (-W) in the HTML build step for CI.
  • Enable strict warning handling (-W) in the HTML build step for the publish-to-Pages workflow.
  • Remove the stale TODO comment about re-enabling -W.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
.github/workflows/ci.yml Adds -W to the jb build command so PR builds fail on warnings.
.github/workflows/publish.yml Adds -W to the jb build command so release/tag builds fail on warnings before publishing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Three lectures — autodiff, jax_intro and numpy_vs_numba_vs_jax — carry
`{include} _admonition/gpu.md`, but the file was never seeded into this
edition. Every other edition has it; this one did not:

    lecture-python-programming        lectures/_admonition/gpu.md
    lecture-python-programming.fa     lectures/_admonition/gpu.md
    lecture-python-programming.zh-cn  lectures/_admonition/gpu.md
    lecture-python-programming.fr     (absent)

So those three lectures have been building with a broken include and
publishing without the GPU admonition. The build reported success
throughout, because this repo built without -W.

Found by enabling -W in the same PR, which is the argument for the flag
in one example: three CRITICAL "file not found" directive errors that
had been live and invisible. This is the shared-asset class described in
QuantEcon/action-translation#117 — sync moves lecture files, so an asset
that lives outside that set is never carried across.

Translated per the French glossary (Lecture -> Cours) and typeset with
U+00A0 before the colon and inside the guillemets, matching the
convention applyTypography() enforces for prose directives — admonition
is on its PROSE_DIRECTIVES list, so the file is in scope for it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mmcky

mmcky commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

The -W build failed, and it found a live defect on the first run. Three CRITICAL errors, all the same missing file:

lectures/autodiff.md:36               CRITICAL: Directive "include": file not found: lectures/_admonition/gpu.md
lectures/jax_intro.md:50              CRITICAL: Directive "include": file not found: lectures/_admonition/gpu.md
lectures/numpy_vs_numba_vs_jax.md:67  CRITICAL: Directive "include": file not found: lectures/_admonition/gpu.md

lectures/_admonition/gpu.md was never seeded into this edition, though three lectures include it. Every other edition has it:

Repo _admonition/gpu.md
lecture-python-programming
lecture-python-programming.fa ✅ (untranslated — see below)
lecture-python-programming.zh-cn ✅ translated
lecture-python-programming.fr absent

So those three lectures have been building with a broken include and publishing without the GPU admonition, silently, for as long as they have existed here — because this repo built without -W. This is exactly the failure mode the flag exists to prevent, caught within minutes of enabling it.

It is also the shared-asset class described in action-translation#117: sync moves lecture files, so an asset living outside that set is never carried across. Worth noting on that issue that _admonition/ joins quant-econ.bib as a known instance.

Fixed in 1db95bf — the file is added, translated per the French glossary (Lecture → Cours) and typeset with U+00A0 before the colon and inside the guillemets, matching what applyTypography() enforces (admonition is on its PROSE_DIRECTIVES list, so this file is in its scope).

Separate finding, not fixed here: .fa's copy of gpu.md is present but still entirely in English. It is not a build error, so -W will not catch it — it needs a translation pass. Filing that separately rather than widening this PR.

@github-actions

Copy link
Copy Markdown

📖 Netlify Preview Ready!

Preview URL: https://pr-14--verdant-toffee-3261a8.netlify.app

Commit: 1db95bf


Build Info

@mmcky
mmcky merged commit 4c092ed into main Jul 21, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants