Repository navigation
fix(fr): apply French typography deterministically instead of asking the model - #79
Merged
Merged
Conversation
…the model
The prompt has told the model to put a non-breaking space before ; : ! ? since
French support landed (language-config.ts:53). It doesn't. Measured on a real
Opus translation of python_by_example.md: 0 × U+00A0, 0 × U+202F, and 16 plain
spaces before high punctuation. Across the 26-lecture French seed that is ~199
occurrences.
This is a pure text transform with an exact specification, so escalating the
prompt is the wrong tool — do it in code and stop asking.
The hard part is knowing where prose ends: `a ? b : c` in a code cell,
`$\{x : x > 0\}$` in math, `key: value` in frontmatter, `(sec:intro)=` anchors
and `https://` URLs all contain the same characters. Two independent defences —
classify each line, then only substitute where the mark is followed by
whitespace or end-of-line. Line classification is a stateful walk, not a regex,
because fences nest: a ```{code-cell} inside a ```{note} is code inside prose.
Notable: ```{note} / ```{hint} / ```{exercise} bodies ARE prose and must be
typeset — an early regex version masked every fence as code and silently missed
them. Unknown directives default to code: a missed space is cosmetic, a
corrupted code cell is not.
Hooked into init BEFORE generateHeadingMap, so the heading map is derived from
the same strings that land in the body. A consequence worth knowing at review
time: js-yaml escapes U+00A0 as `\_` in double-quoted scalars, so frontmatter
heading maps now serialise as "...LLM\_?". That is valid YAML and round-trips
back to U+00A0 (verified against extractHeadingMap) — it just looks odd.
scripts/typography/apply.mjs is the repair path for repos seeded before this
existed (the French programming seed) or when a rule changes later. It carries
a safety net that refuses to write unless the only difference is spacing before
high punctuation — body compared as text, frontmatter compared as parsed YAML,
since re-serialising legitimately changes quoting.
Scope: init only. The sync path derives its heading map from translated sections
before reconstruction, and wiring typography in there correctly needs the
heading-map matching semantics pinned down first — noted as follow-up rather
than guessed at.
34 unit tests; verified over the 23 seeded French lectures that the only change
is spacing before ; : ! ? and that the transform is idempotent.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a deterministic typography post-processing pass for translated MyST Markdown, focusing on French non‑breaking spaces before ; : ! ?, and wires it into the translate init flow so heading-map generation is derived from the final (typeset) text.
Changes:
- Introduces
applyTypography()with line classification + inline masking to safely apply French punctuation spacing only in prose. - Applies typography during
translate initbefore heading-map generation. - Adds unit tests for the typography transform and a repair script to back-apply the rules to already-seeded repos.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/typography.ts |
Implements deterministic typography with MyST-aware line classification and inline masking. |
src/cli/commands/init.ts |
Applies typography to the translated document before heading-map generation in init. |
src/__tests__/typography.test.ts |
Adds tests for French typography rules and protections (code/math/frontmatter/links/etc.). |
scripts/typography/apply.mjs |
Adds a back-apply tool with safety checks to ensure only intended punctuation spacing changes occur. |
Comment on lines
+195
to
+197
| export function hasTypographyRules(language: string): boolean { | ||
| return language in RULES; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Fixed in bfae2bd — same root cause, same fix: RULES is now a Map, so this is RULES.has(language) and the two functions agree by construction rather than by remembering to guard both.
Found by auditing what the pass left behind on the real seed rather than
trusting the green tests: two plain-space occurrences survived in scipy.md that
were unmistakably prose ("Voici une solution :").
MATH_DELIM only matched a bare `$$` line. But a block can open with content:
$$ \mathbb E \max\{ S_n - K, 0 \}
\approx
$$
so the opener was missed, the closing `$$` was read as an OPENER, and every
line after it in the file was classified as math — silently suppressing all
further substitutions. Now delimiters are counted, toggling once per `$$`, so
same-line openers track correctly and a self-contained `$$ x $$` nets out.
Also reordered: code-fence handling now precedes the math check. A `$$` inside
a code cell is code, and toggling math state on it would desync everything
downstream — latent, but the same class of bug.
Blast radius on the seed was small (scipy.md only, 2 occurrences) because it
needs a same-line math opener, but the failure mode is silent and unbounded:
everything after the first such block in a file.
4 regression tests. Re-running the repair over the already-repaired seed changed
exactly scipy.md and nothing else, which is the idempotence guarantee doing its
job.
Addresses Copilot's review on #79. All five comments were valid. 1. Mixed runs were split. `Quoi?!` became `Quoi?<NBSP>!` — the space landed inside the run, which is both wrong typography and a corruption of the author's punctuation. The old `before === punct` guard only caught identical marks, so `!!` was skipped (also wrong) while `?!` was mangled. The run is now matched whole (`[;:!?]+`) and spaced once in front of the group. Fixing that removed the guard that also stopped the regex re-entering a run, and my idempotence test caught the regression: a NBSP is whitespace to `\S`, so on a second pass the anchor cannot latch onto it and scanning resumes at the `?`, splitting `?!` again. Now guarded explicitly on `before` being high punctuation. Idempotence matters here — the repair script depends on it. 2. RULES was a plain object, so the lookup walked the prototype chain. Not theoretical: applyTypography(content, 'toString') found Object.prototype.toString, called it as a rule, and returned "[object Undefined]" — silently destroying the document. `language` comes from --target-language, so it is reachable. Now a Map, which has no chain. 3. hasTypographyRules used `in`, same issue → Map.has. 4. Test import now carries the .js extension, matching parser.test.ts. 5. Added run cases (?!, !!, !?, ?!?) and prototype-key cases, per Copilot's suggestion that the behaviour be locked in. 50 typography tests (1056 total). Re-running the repair over the seeded French corpus is a clean no-op, confirming the run change disturbs nothing applied.
mmcky
added a commit
that referenced
this pull request
Jul 15, 2026
* chore: release v0.16.0 - Bump version to 0.16.0 - Promote [Unreleased] changelog to [0.16.0] - Rebuild dist-action bundle (unchanged — the version is not embedded in it) - Refresh version/test counts in README + copilot-instructions (1005 → 1056, 39 → 40 suites) Changelog accuracy, beyond the promotion: - The thinking entry described the off default as a hold pending measurement. That experiment has since concluded — adaptive thinking self-regulates to ~zero on translation (+24-32 tokens even at effort: high), so off is a settled decision. Rewritten; shipping release notes that call a settled decision provisional would mislead anyone deciding whether to override it. - Stated plainly that the typography pass added in #79 covers the CLI init path ONLY and does not change how the action syncs. Verified rather than asserted: `applyTypography` does not appear in dist-action/index.js at all. Without this, a maintainer would reasonably assume 0.16 fixed typography for their sync. - Fixed the 0.15.0 release date: it read 2025-07-14, which is impossible — it would predate 0.14.1 (2026-04-09). The tag is dated 2026-04-14. Also added `typography.ts` and `models.ts` to the architecture.md module tree. Both were missing (models.ts since #75), so bumping that doc's version stamp without them would have made it more wrong, not less. Verified: 1056 tests green, lint clean (0 errors), a fresh `npm run build` leaves dist-action byte-identical, and action.yml's claude-model default matches DEFAULT_CLAUDE_MODEL. * chore: sync package-lock version with package.json Addresses Copilot's review on #80. Valid, and longer-standing than the PR: the lockfile still declared 0.8.0, having not been version-bumped since #24 — it drifted silently through seven releases. A release PR is the right place to stop that. No downstream impact today (the package is not published — `npm view` 404s), but `private` is false, so it could be, and the metadata was simply wrong either way. Changed the two version fields directly rather than running `npm install --package-lock-only`, which re-resolves and could churn transitive dependencies inside a release PR. Diff is exactly 2 lines; `npm ci --dry-run` accepts the lockfile; build and 1056 tests still green.
mmcky
added a commit
to QuantEcon/lecture-python-programming.fr
that referenced
this pull request
Jul 15, 2026
Bulk translation of lecture-python-programming into French, seeded with QuantEcon/action-translation's `translate init`. - Model: claude-opus-4-8. Opus is used for one-time seeds because it showed zero cross-lecture terminology drift over a 5-lecture trial where Sonnet 5 drifted on 5 terms, and was correct where Sonnet was not (`mutable`, not `muable`). It costs 1.66x, which is worth it once for a corpus a native speaker then reviews. Ongoing sync stays on Sonnet 5. See QuantEcon/action-translation `.dev/decisions/D-2026-07-14-opus-for-bulk-seed.md`. - Glossary: 364 terms, including 7 programming-domain terms proposed in QuantEcon/action-translation#78 and still under native review. - French typography (non-breaking space before ; : ! ?) applied deterministically rather than via the prompt, which the model ignores. 331 inserted. See QuantEcon/action-translation#79. - 26/26 translated, 687k tokens, 34 minutes. This is a draft for review, not a finished edition.
mmcky
added a commit
that referenced
this pull request
Jul 16, 2026
…h body (#99) * fix(sync): apply typography in the sync path, typeset map with body applyTypography() ran only in the `translate init` seed path, so every sync stripped the non-breaking spaces the seed inserted and fell back on the prompt instruction #79 was opened because the model ignores. One merged sync took numba.md from 27 to 14 non-breaking spaces, invisible in review because the diff renders as unchanged text. Both sync entry points now typeset: - processFull: on the raw model output, before the heading map is derived from it — the same ordering as init.ts. - processSectionBased: on the reconstructed document, before the map is injected. Document-scoped, so drift in sections the source PR did not touch is repaired on the next sync of that file. The map is typeset alongside the body rather than after it. applyTypography skips frontmatter by design, so typesetting already-injected content would leave map values plain-spaced while body headings gained non-breaking spaces. Heading lookup is an exact string compare with no whitespace normalisation, and the positional fallback is unavailable whenever the section count changes, so that mismatch silently drops unchanged sections and retranslates modified ones from English, discarding human edits. Body, map, and title are typeset together, matching what scripts/typography/apply.mjs already did. Tests were verified to fail against both the unfixed code and the body-only variant; against the latter the added-section case reports a skipped section, which is the data loss above. See #97 Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * fix(review): use the U+00A0 escape in tests, repair CHANGELOG clause Addresses Copilot feedback on #99. The test file defined NBSP as a raw literal non-breaking space, the only such definition in the repo -- src/typography.ts and src/__tests__/typography.test.ts both spell it as an escape sequence. An invisible character is a poor definition for the constant every assertion in the suite depends on: an editor could mangle it silently, and the tests would then compare plain spaces to plain spaces and pass vacuously. Line 16 held the only literal; every fixture already interpolates ${NBSP}, so fixing the definition covers the file. Verified the change is a behaviour no-op by re-running the tests against both broken variants: 7 of 9 still fail against main's file-processor.ts, and 3 of 9 against the body-only variant. The CHANGELOG clause tried to extract the object of "ignores" out of a because-clause, which no single missing word repairs. Rewritten to match the wording used in the issue. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
7 tasks
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.
Since French support landed, the prompt has instructed the model to put a non-breaking space before
; : ! ?(language-config.ts:53). It doesn't. Measured on a real Opus translation ofpython_by_example.md: 0 × U+00A0, 0 × U+202F, and 16 plain spaces before high punctuation. Across the 26-lecture French seed that's ~199 occurrences of wrong typography in published lectures.This is a pure text transform with an exact specification. Escalating the prompt is the wrong tool — the fix is to do it in code and stop asking the model.
Where prose ends is the actual problem
The substitution is trivial. Knowing where to apply it is not:
a ? b : cin a code cell,$\{x : x > 0\}$in math,key: valuein frontmatter,(sec:intro)=anchors andhttps://URLs all contain the same characters and must never be touched.Two independent defences, either of which would catch most cases:
sec:intro,14:30and.Line classification is a stateful walk rather than a regex, because fences nest: a
{code-cell} inside a{note} is code inside prose, and no single regex gets that right.The subtlety worth reviewing:
{note},{hint},{exercise}and{solution}bodies are prose and must be typeset. An early regex version masked every fence as code and silently skipped them — that's howVos indices sont les suivants :inside a{hint}survived a first pass looking correct. Unknown directives default to code: a missed space is cosmetic, a corrupted code cell is not.Hook point
applyTypographyruns ininitbeforegenerateHeadingMap, so the heading map is derived from the same strings that land in the body rather than drifting one character away from them.One consequence a reviewer should see coming: js-yaml escapes U+00A0 as
\_in double-quoted scalars, so frontmatter heading maps now serialise as"Ne puis-je pas simplement utiliser des LLM\_?". That is valid YAML and round-trips back to U+00A0 — verified against the realextractHeadingMap— it just looks odd on first sight.Repair path
scripts/typography/apply.mjsback-applies the rules to a repo seeded before this existed (the French programming seed) or when a rule changes later. It refuses to write unless the only difference is spacing before high punctuation: the body is compared as text, the frontmatter as parsed YAML, because re-serialising legitimately changes quoting. That safety net earned its place — it caught the\_escape and the quoting change during development, both of which a naive byte-diff would have called corruption.Scope
init only. The sync path derives its heading map from translated sections before reconstruction, so wiring typography in there correctly needs the heading-map matching semantics pinned down first. I'd rather flag that than guess at it. Filed as follow-up.
U+00A0overU+202F(narrow) is deliberate: it renders reliably in both HTML and PDF and matches the existing prompt wording. It's a single constant, so a native reviewer can change the convention in one place — a reasonable question for Emile alongside the glossary round.Verification
; : ! ?, and the transform is idempotent. The 37 untouched occurrences were audited — 19 are code (z[0, :],observations : array_like, French prose inside#comments) and 18 are frontmatter heading-map values, which the repair script handles.🤖 Generated with Claude Code