feat(i18n): deterministic integrity/typography checks and real per-language style guides - #630
Timur Tukaev (tym83) wants to merge 14 commits into
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
NOT LGTM. The new masking passes make almost half of the in-scope docs untranslatable, and the deterministic checks contradict the style guides they are supposed to enforce.
Business context: the pipeline defended URLs, bare flags, versions and per-language typography with prompt rules only; this PR makes those guarantees deterministic (link/HTML masking plus integrity and typography checks feeding the revise loop) and turns the stub style guides into real per-language contracts.
Blockers
B1: pages with [text]({{< ref ... >}}) links can no longer be translated
protect() step 4 re-stashes placeholders created by earlier passes (hack/i18n/lib.py:58). In [text]({{% ref "/x" %}}) the shortcode pass stashes the ref as SC_0, then _LINKDEST_RE stashes that token again as URL_1 whose stored value is the SC_0 placeholder. The model only ever sees URL_1, so the placeholder guard counts SC_0 at 0 in every reply and raises ProtocolError on every attempt. restore() (lib.py:475) is a single pass in insertion order and cannot unwind the nesting either, and the back-translation path (lib.restore(back_en, tr_store)) restores with no guard at all. Same mechanism for [text](<https://...>) and [id]: <https://...>, legal CommonMark, none in the corpus today.
Reproduced: protect() on [install guide]({{% ref "/docs/install" %}}) yields a store where the URL token's value is itself a placeholder, and the guard on a token-preserving reply reports the inner token at 0. Running protect() over the whole configured scope: 82 of 181 pages produce nested placeholders (]({{< ref ... >}}) occurs about 279 times in scope). Each of those pages deterministically fails every protocol attempt on every run and burns the full retry budget of model calls first.
Fix in the same layer: skip destinations that contain the placeholder marker in the _LINKDEST_RE/_REFDEF_RE substitutions, iterate restore() in reverse insertion order (later stashes can only reference earlier tokens), and add a fail-closed residual-placeholder scan after restore for the unguarded back-translation path. Round-trip tests for [t]({{< ref "x" >}}) and [t](<https://x>) belong in TestLinkDestinationMasking.
B2: the version-integrity check makes the style guides' own mandated conversions gate-fatal
_VERSION_RE (lib.py:553) counts every bare decimal in prose and demands byte-for-byte survival as a major finding, while four style guides rewritten in this same PR mandate decimal comma in prose ("3.14" to "3,14", "0.5 vCPU" to "0,5 vCPU") and the prompt fix in this PR explicitly permits prose reformatting. The German thousands rule ("10,000" to "10.000") additionally trips the invented-token minor. gate_passed requires an empty findings list (translate.py:316) and deterministic checks refire identically every round, so a page with any prose decimal quantity can never pass: it burns all revise rounds on every source change and ships -with-findings forever. This re-creates at the checker level the exact contradiction the headline commit resolves in the prompt. Also, style-guides/pt-br.md contradicts itself on consecutive lines (37 and 38: "0.5 vCPU" must become "0,5 vCPU" and must also "stay exactly as written, including when quoted in prose").
Reproduced: integrity_findings("It is 2.5 times faster.", "Es ist 2,5-mal schneller.") returns a major demanding "2.5" verbatim; integrity_findings("It runs 10,000 pods.", "Es betreibt 10.000 Pods.") returns a minor "do not invent versions". Live in scope: docs/v1.5/getting-started/deploy-app.md carries "2.5 GB" and "1.5 GB" in bare prose.
Fix: restrict _VERSION_RE to unambiguous version tokens (v-prefixed and/or three-component), or accept the localized decimal form as equivalent for bare d.d while keeping v-prefixed and three-component tokens exact; that preserves the v1.5 to v1,5 catch the PR description advertises. Align pt-br.md lines 37-38 and the prompt's "resource quantities" wording with whichever rule wins. Tests in TestIntegrityFindings: ("It is 3.14 wide.", "Es ist 3,14 breit.") must be clean while v1.5 to v1,5 stays caught.
B3: the Spanish question/exclamation rules flag correct text by construction
The rule at lib.py:617 anchors at any capitalized word, not at sentence start, so a correctly opened question that contains a capitalized brand matches from the brand onward. This corpus capitalizes Cozystack/Kubernetes/Talos in nearly every sentence, and the exclamation rule has the same defect. Any finding blocks gate_passed, so most Spanish pages with a question can never clear the gate. There is no content/es/ yet; the first full es run hits this at scale.
Reproduced: check_typography() on the correct question "¿Qué es Cozystack?" returns a finding, matching from "Cozystack?".
Fix: anchor to sentence start (string/line start or after sentence-ending punctuation). Test: the exact string above must return [].
B4: the pipeline README no longer matches the gate it documents
The per-page pipeline diagram in hack/i18n/README.md lists every gate stage (translate, back-translate, two reviewers, revise) but not the new deterministic integrity/typography stage that feeds the same findings loop, and the file inventory table has no row for lint_translations.py. A maintainer triaging a weekly PR sees findings from: integrity-check / typography-check that the pipeline's own documentation says don't exist. Add the stage to the diagram and a lint_translations.py row to the table.
Non-blocking follow-ups
_REFDEF_REmasks the first word of footnote definitions:[^1]: Some notecomes back with the first word replaced by a placeholder (reproduced), leaving it untranslated. No footnotes in the corpus today; exclude[^...]labels.- The
lint_translations.pydocstring says "this makes them enforceable on every PR", but.github/workflows/i18n-lint.ymlruns onlycheck-i18n.shandtest_i18n.py. Wire it in as an advisory step or soften the claim. - The belt-and-braces path in
translate.py:362-363writes a machine-translated body while keepingl10n: transcreate. If that currently unreachable path ever fires, machine output ships labeled as human transcreation with no disclaimer. Raising instead of writing is strictly safer. integrity_findingsdo-not-translate counting is case-sensitive substring matching; a term that is also an ordinary English word will raise majors when a generic use is legitimately translated. Word-boundary matching would cut noise that blocks the gate via the B2 mechanism._LINKDEST_REstops a destination at the first), so a balanced-paren URL (.../Foo_(bar)) is partially masked with a stray)left in prose. None in the corpus today.
| # inline: [text](/docs/install "Optional title") -> destination only | ||
| # autolink: <https://example.com> | ||
| # refdef: [id]: https://example.com | ||
| _LINKDEST_RE = re.compile(r'(?<=\])\((?P<dest><[^>]*>|[^)\s]*)(?P<title>\s+"[^"]*")?\)') |
There was a problem hiding this comment.
B1 (nested placeholders): this pass re-stashes tokens created by the shortcode and autolink passes. [text]({{% ref "/x" %}}) becomes a URL placeholder wrapping the shortcode placeholder; the model never sees the inner token, so the placeholder guard raises ProtocolError on every attempt. 82 of 181 in-scope pages hit this. restore() cannot unwind the nesting either (single pass, insertion order), and the back-translation path restores unguarded. Fix: skip destinations that contain the placeholder marker here and in _REFDEF_RE, reverse the restore() iteration order, add a residual-placeholder check after restore. Details in the review body.
|
|
||
| # Version-ish tokens: v1.5, 1.2.3, v1.2.5. Localizing the separator (1,2,3) or | ||
| # bumping a digit changes documented behaviour, so counts must match the source. | ||
| _VERSION_RE = re.compile(r"\bv?\d+\.\d+(?:\.\d+)?\b") |
There was a problem hiding this comment.
B2 (gate-fatal contradiction): this counts every bare d.d decimal in prose and demands it byte-for-byte, while the style guides in this same PR mandate "0.5 vCPU" to "0,5 vCPU" and the prompt fix permits prose reformatting. Deterministic findings refire every round and gate_passed needs an empty list, so a page with any prose decimal can never pass (docs/v1.5/getting-started/deploy-app.md has two). Restrict this to v-prefixed and/or three-component tokens, or treat the localized decimal form as equivalent for bare d.d. Details in the review body.
| (r'[“][^\n]{0,80}[”]', 'English curly quotes in German prose — use „…“'), | ||
| ], | ||
| "es": [ | ||
| (r'(?<![¿])\b[A-ZÁÉÍÓÚÑ][^.!?\n]{5,120}\?', 'question without an opening ¿'), |
There was a problem hiding this comment.
B3 (false-positive anchor): the pattern matches at any capitalized word, so the correct question "¿Qué es Cozystack?" is flagged from "Cozystack?" onward (reproduced). With brands capitalized in nearly every sentence, most es pages with a question can never clear the gate. Anchor to sentence start (string/line start or after sentence-ending punctuation); same fix for the exclamation rule.
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
NOT LGTM — the round-1 fixes hold, but one of them rebuilt the placeholder guard so a hallucinated token now becomes silent content duplication, and the transcreate change made l10n machine-read without the README following.
Blockers
B1: weakened placeholder guard accepts tokens the model was never sent
File: hack/i18n/translate.py:223
Issue: The tok in masked_src filter correctly stopped requiring inner tokens of a nested stash, but it also stopped forbidding them.
Evidence: Reproduced on this tree: protect() on a backticked shortcode stores §§SC_0§§ inside §§INLINECODE_1§§'s value; a reply carrying the legit outer token plus a hallucinated §§SC_0§§ passes the guard, reverse restore expands both, the page ships with the shortcode twice, and the final residual §§ check sees nothing. At the previous head the same reply produced a visible literal token; this diff makes the outcome invisible.
Impact: silent content duplication in a published page — the exact failure class the protocol guard exists to refuse.
Fix: one line — body.count(tok) != (1 if tok in masked_src else 0): a token the model never saw must occur zero times. Plus a TestPayloadProtocol case: a stray token the model was never sent is rejected.
B2: the front-matter table no longer tells the truth about l10n
File: hack/i18n/README.md:191
Issue: The row says l10n is read by "humans triaging review". After this PR the marker is load-bearing for machines: build_worklist skips transcreate pages, translate_page refuses to write them, check-i18n.sh downgrades their drift to a warning, and update-digests skips them.
Evidence: hack/i18n/lib.py:250 (PRESERVED_L10N_VALUES), translate.py:258-262, and is_transcreate in check-i18n.sh — all introduced or rewired by this PR; the README row predates them unchanged.
Impact: a maintainer marking or unmarking transcreate cannot learn from the README that the marker changes pipeline behavior — which is the point of the feature.
Fix: update the "Read by" cell and add one sentence describing the cycle: mark transcreate → pipeline leaves the page alone → CI warns on drift → refresh by hand → update-digests <file>.
Non-blocking follow-ups
is_transcreateincheck-i18n.shgreps^l10n:file-wide whilerecorded_l10nis bounded to front matter — same divergence class as the known file-wide^source_digest:quirk, unreachable today. Neither accepts single-quoted YAML (l10n: 'transcreate'), and the failure direction there is overwriting a transcreation with machine output. Cheap fix: bound the shell grep to the front-matter block, parse viasplit_frontmatterin Python.check_digest_freshnessprints "translation freshness: OK (N pages match)" immediately after a::warning::about a drifted transcreate page — the summary contradicts the warning it just printed.
Verified fixed from round 1, each with a repro on this tree: nested placeholder masking (protect/restore round-trips identity, 122 tests pass), decimal-comma and thousands formatting no longer trip the version check while v1.5 → v1,5 is still caught, the Spanish inverted-mark rule anchors to sentence start ("¿Qué es Cozystack?" is clean), the README documents the deterministic-checks stage and lint_translations.py, the typography lint runs in CI as an advisory step, footnote definitions are excluded from ref-def masking, and do-not-translate matching is word-bounded.
| body = body.strip() | ||
| bad = {tok: body.count(tok) for tok in store if body.count(tok) != 1} | ||
| bad = {tok: body.count(tok) for tok in store | ||
| if tok in masked_src and body.count(tok) != 1} |
There was a problem hiding this comment.
This filter correctly stopped requiring inner tokens of a nested stash, but it also stopped forbidding them: a reply carrying a hallucinated inner token (§§SC_0§§ when the model was only sent §§INLINECODE_1§§) passes the guard, reverse restore expands both, and the page ships the content twice — the residual §§ check sees nothing. One line closes it: body.count(tok) != (1 if tok in masked_src else 0) — a token the model never saw must occur zero times.
|
Aleksei Sviridkin (@lexfrei) both blockers from round 2 are addressed on B1 — placeholder guard. The guard now enforces B2 — README front-matter table. The The two non-blocking follow-ups (front-matter-bounded grep for |
myasnikovdaniil
left a comment
There was a problem hiding this comment.
Checks themselves look good. One thing about the freshness guard, inline.
| echo " hack/check-i18n.sh update-digests $f" | ||
| else | ||
| rc=1 | ||
| echo "::error::stale translation: $f" |
There was a problem hiding this comment.
This error path knows nothing about translation scope, and that turns old translations into CI blocker. Any l10n: mt page with drifted digest fails the lint, lint runs on every PR touching content/**, but the pipeline only refreshes the latest version, so content/*/docs/v1.4/getting-started/_index.md cannot be fixed by rerunning it.
Digests match right now, I checked all four languages on main, so nothing is red yet. It goes red as soon as the upstream tags workflow pushes make update-all RELEASE_TAG=v1.4.x into content/en/docs/v1.4/, and from that moment every PR in this repo is blocked until somebody translates that page by hand or deletes it. Either downgrade out of scope pages to warning like transcreate ones, or remove them.
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
NOT LGTM — both blockers from my last review are fixed, but the branch now conflicts with its base, and most of the conflicting code is the same fixes written twice, so the rebase decides what ships.
Blockers
B1: conflict with feat/i18n-pipeline
Files: hack/i18n/lib.py, hack/i18n/translate.py, hack/i18n/README.md
Issue: this branch forked the base at 12e90d5c9. The base then got cb0e2e7f4, which reimplements three things this PR already has:
- Skipping
l10n: transcreatepages:recorded_l10n/is_hand_localized/find_hand_localizedhere,l10n_mode/find_transcreatedthere, same_L10N_RE. Git merges both skips intobuild_worklistwithout a conflict, and keeping both sides of the report hunk intranslate.pygives two sections listing the same pages. - Raw HTML tag masking, with a different design. Here the whole tag is masked, one line only. The base also matches multi-line tags and leaves
alt/title/captionvalues open for translation. The two_HTMLTAG_REdefinitions merge without a conflict marker, and the later one (this PR's) silently wins. - Nested placeholder restore: reverse order here, a fixed-point loop there. Either works, keep one.
Evidence: git merge-tree --write-tree 8133286ac 2652726e9 reports content conflicts in exactly these three files. With a union resolution (both sides kept, one syntax break fixed) the suite runs 136 tests and fails one: the base's test_visible_attributes_still_translate, because this PR's tag pass masks alt="A running cluster" before the base's pass sees it.
Impact: an approval now would cover lib.py and translate.py in a form that won't be merged, and a mechanical resolution either keeps duplicate code or drops the base's attribute handling without any error.
Fix: rebase and pick one implementation of each. Keep this PR's _split_payload_response (it merges cleanly). The base version still requires every stored token, so there a correct reply for a backticked shortcode fails as "lost". Keep this PR's l10n row in the README table too.
Since the history gets rewritten anyway: this repo merges with merge commits, so commit messages land on main as written. df696750e, 97dd5fc31, c2ba45e88 and 41255d05a have Co-Authored-By: Claude <noreply@anthropic.com>, and my own commits a9af5f3c5 through 345f9983e have Assisted-By: Claude <noreply@anthropic.com>. Both should be Assisted-by: LLM. 41255d05a ("Two gaps found in review by Aleksei Sviridkin (@lexfrei)") and 005aa461a ("The round-1 fix that...") talk about the review, not the change. All of this was already there at my last review, I should have raised it then.
Checked
- Placeholder guard: the reply from my last repro (the sent
§§INLINECODE_1§§plus an unsent§§SC_0§§) is rejected as "injected". At345f9983ethe same reply was accepted and the shortcode ended up in the page twice. Reverting the guard line failstest_inner_token_the_model_never_saw_is_rejectedand nothing else. l10ndocs: the row and the new paragraph match the code. On a copy of the tree a drifted transcreate homepage warns,update-digests content/ru/_index.htmlclears only that warning, and bareupdate-digestsskips all eight transcreate pages.8133286ac: the classifier matches_docs_out_of_scopeinlib.py. With the real stamped pages, drift inv1.5fails, drift inv1.4gives four warnings and exit 0, drift indocs/_index.mdfails, and a missinglatest_version_idfails closed.- 123 tests pass,
check-i18n.shandlint_translations.pyare clean.
Non-blocking
hack/check-i18n.sh:105:latest_version_id: 'v1.5'parses as'v1.5'with the quotes, so every versioned page,v1.5included, counts as superseded. Current-version drift becomes a warning and the lint exits 0, whilelib.pyreads the same value correctly.register_version.shwrites double quotes, so it takes a hand edit, but nothing would show it. This line right afterLATEST_DOCS="$(latest_docs_version)"fails closed on any bad parse (checked on the same copy):[ -d "$CONTENT_DIR/$DEFAULT_LANG/docs/$LATEST_DOCS" ] || LATEST_DOCS=""- No test covers the classifier: with
!=flipped to=inis_superseded_docsall 123 tests stay green. The shell lint has no tests at all, so this is only a suggestion. hack/i18n/test_i18n.py:447:assertIn("injected", ...)passes with any label, because the fixed tail of the message ("dropped, duplicated or injected code") always has the word. With the label forced to "duplicated" the suite stays green."injected by the model"would check it. Same for"duplicated"intest_duplicated_placeholder_is_rejected.- The new warning says the pipeline removes superseded translations on its next run. At this PR's base
find_orphan_translationskeeps them on purpose, and only18b140aa2in the base changes that, so the message becomes true after the rebase. The same reason is also written in three comments, one is enough. - Both non-blocking items from my last review are still there, and the summary one now also follows the new warnings: four superseded warnings, then "translation freshness: OK (22 translated pages match their English source)".
- The PR body becomes the merge commit message here. It still says "91 tests pass" and doesn't mention HTML masking,
transcreatehandling or the newcheck-i18n.shwarnings.
delete_branch_on_merge is on, so merging #623 retargets this PR to main.
3854890 to
d38db12
Compare
8133286 to
a6b9c3c
Compare
|
Aleksei Sviridkin (@lexfrei) rebased onto the current I dropped the four commits that reimplemented what the base already owns, so each mechanism has a single implementation:
Kept Two spots are manual merges — worth breaking by mutation first:
123/123 tests pass; |
d38db12 to
90f80fd
Compare
Masking previously covered code, shortcodes and comments, so a URL, a bare CLI flag, a version number or a brand sitting in ordinary prose was defended by a prompt rule alone. Link destinations are now masked like any other protected span, and two deterministic checks feed the existing revise loop: - integrity_findings() compares versions, bare flags and do-not-translate terms between source and translation, catching a localized version separator or a transliterated brand. - check_typography() enforces the per-language rules the style guides state (Russian guillemets, German quotes, Spanish inverted marks, Chinese full-width punctuation, pt-PT vocabulary leaks, Devanagari digits). Both look at prose only; markup, code and link targets are exempt so the checks do not cry wolf on correct ASCII punctuation in an HTML attribute. lint_translations.py applies the same typography rules to already-published pages, where a hand edit is otherwise never re-checked. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: tym83 <6355522@gmail.com>
Each guide was 3-6 lines, yet the whole fluency and typography strategy rests on injecting them into the translate and both reviewer prompts. They are now 80-100 lines each and work as an instruction set and a review rubric: register and address form, heading conventions, a decision rule for terms outside the glossary, typography, number/date formatting, the grammar traps of translating from English into that language, calque patterns with fixes, false friends, and a reviewer checklist of the MT failure modes specific to the language. Every original decision is preserved; the guides expand around them. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: tym83 <6355522@gmail.com>
…te prompt Hard rule 5 said to keep numbers unchanged, while several style guides require a decimal comma, a different thousands separator or a different date order in prose. The model was reading two incompatible instructions. The rule now separates the two ideas it was conflating: a number's VALUE and any version or identifier are literal and must be reproduced exactly, while formatting in ordinary prose follows the language's style guide. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: tym83 <6355522@gmail.com>
The version-integrity check counted every bare decimal in prose and
demanded byte-for-byte survival, while the style guides mandate the
decimal comma there ("0.5 vCPU" -> "0,5 vCPU") and the translate
prompt explicitly permits prose reformatting. Deterministic findings
refire identically every revise round and the gate requires an empty
findings list, so a page with any prose decimal could never pass: it
burned all revise rounds on every source change and shipped
-with-findings forever. The German thousands rule (10,000 -> 10.000)
additionally tripped the invented-token check.
Enforce only unambiguous version shapes (v-prefixed or three
components); bare two-part decimals are the reviewers' job. Count
do-not-translate terms on word boundaries so a term is not demanded
back for occurrences inside larger words. Anchor the Spanish inverted
punctuation rules to sentence start: matching at any capitalized word
flagged every correctly opened question containing a brand name. Align
the translate prompt and the pt-BR guide on the same rule: quantities
in code stay literal, bare decimals in prose follow the language.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
…y lint The pipeline diagram listed every gate stage except the deterministic integrity/typography checks that feed the same findings loop, and the file table had no row for lint_translations.py — a maintainer triaging a weekly PR saw findings from checks the docs said did not exist. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
lint_translations.py promised enforcement on every PR but nothing invoked it. Advisory for now; flip to --strict per language once its backlog is clean. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Three checks contradicted the ✓ forms the guides themselves give. The Spanish inverted-mark rules matched through a mid-sentence ¿/¡, so the guide's mandated 'Si el nodo falla, ¿qué pasa?' form was flagged — and a revise round would most plausibly insert the mark at sentence start, producing exactly the form the guide marks ✗. The Russian curly-quote rule counted U+201C alone, which also CLOSES the mandated nested „лапки“, so two nested pairs on one line read as an English pair. The bare three-component version branch read localized numeric dates (24.07.2026) and period-grouped thousands (10.000.000) as invented versions, demanding the source format back in violation of the guides. Stop the Spanish span at a mid-sentence mark, require the full English “…” pair for Russian, and exempt date/thousands shapes from the invented-token report. Also note the known multi-line-tag ceiling on the HTML tag mask. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
The drift report for hand-localized pages is built on a stale source_digest, but the surrounding tooling destroyed that signal and contradicted the contract. check-i18n.sh hard-failed CI on any digest mismatch, so a drifted transcreation made every PR touching content red — and its documented fix, a wholesale update-digests, re-stamped transcreate pages too, silencing the drift report forever while the drift persisted. The report also called the refresh optional while CI treated it as a hard failure. Drifted transcreations now produce a ::warning:: instead of an error (drift is a report for a human, not a build failure), a bare update-digests skips them so the signal survives, and passing the file explicitly re-stamps one that was genuinely refreshed by hand. Report and docstring wording now match that contract. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
The prose filter stripped an image's destination but left the leading exclamation mark, so a heading ending in a CJK ideograph followed by a figure read as half-width punctuation after a Chinese character, and an inline image in Spanish prose read as an exclamation missing its opening mark. Both findings refire identically every revise round (the model cannot remove image syntax), so any affected page burned its full revise budget and shipped stamped -with-findings. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Feeding the file list through a pipe put the enumerating grep under pipefail: on a checkout with zero stamped translations it exits 1 and silently kills the script, where the previous process-substitution form exited 0. First-language bootstrap and fresh forks hit this. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
The prose filter stripped only space-free link destinations, so a titled link's "..." survived into the typography view. The Russian quote rule then flagged it, and the revise loop, following the style guide, would localize the ASCII quotes into guillemets — an invalid CommonMark title delimiter that stops the link parsing at all. The title stays deliberately translatable in the masked text, so the fix belongs in the prose filter, not the masking. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
… to update The explicit-file mode of update-digests made a new input class reachable: a hand-authored page without a source_digest line. The awk only rewrites an existing line, so such a page passed through untouched while the script still printed "updated" — and the drift report would keep listing the page forever while its printed remedy kept lying that it worked. Check for the line first and print a warning naming what is missing. Also match the transcreate marker exactly (not as a substring) and note the balanced-paren ceiling on the link-destination mask. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
The round-1 fix that stopped requiring inner tokens of a nested stash also stopped forbidding them: a reply carrying a legit outer token plus a hallucinated inner token passed the guard, reverse restore expanded both, and the page shipped the nested content twice — silent duplication the residual check can no longer see. The guard now expects a sent token exactly once and an unsent token zero times. README documents that l10n: transcreate drives the pipeline, not just review triage. Signed-off-by: tym83 <6355522@gmail.com>
…advisory check-i18n.sh no longer fails CI on a machine-translated page of a non-latest docs version. The pipeline only refreshes the latest version and removes superseded translations on its next run, so a drifted digest there is not something a PR can fix by rerunning — failing the lint blocked every unrelated PR in the repo once the upstream tags workflow pushed an old-version update. Such pages now emit a ::warning:: (like transcreations) instead of a blocking ::error::; latest-version pages remain hard errors. Signed-off-by: tym83 <6355522@gmail.com>
a6b9c3c to
1e44e85
Compare
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
Timur Tukaev (@tym83) NOT LGTM. The conflict blocker is closed, the commit message blocker is not, and the rebase made the two l10n parsers disagree.
The rebase did more than move the base. Four commits are gone, and four more changed while being carried over: 3c70b6c1, 7fd9d279, 5aebd263 and a41be36b. Nine of the fourteen commits are mine, so an approval from me would also cover my own code. Please get a second reviewer for that part too.
Closed
The conflict with feat/i18n-pipeline is resolved. Each of the three mechanisms now has one implementation: l10n_mode/find_transcreated, a single _HTMLTAG_RE, and the fixed-point restore(). _split_payload_response keeps this PR's rule, with "sent" now meaning top-level. If I revert the guard to top-level-only, test_inner_token_the_model_never_saw_is_rejected fails and nothing else does. The l10n row in the README matches the code. The superseded-version warning is also true now, because find_orphan_translations in the base removes superseded pages.
Blockers
B1: commit messages
The trailers are unchanged. 3c70b6c1, ab06fd73 and fe57bb38 still have Co-Authored-By: Claude <noreply@anthropic.com>. My nine commits, 2c4cdc90 through 5f0e4d24, still have Assisted-By: Claude <noreply@anthropic.com>. a41be36b still opens with "The round-1 fix that stopped requiring inner tokens...". This repository merges with merge commits, so these land on main as written. Each trailer should be Assisted-by: LLM, and the a41be36b body should describe the bug rather than the review round.
The rebase also left two of my messages describing changes their commits no longer make. 7fd9d279 ends with "Also note the known multi-line-tag ceiling on the HTML tag mask", but that comment went away with the dropped tag regex. The commit now adds the (?!\^) footnote exclusion to _REFDEF_RE, which the message never mentions. 5aebd263 says "Report and docstring wording now match that contract", but after the rebase it only touches check-i18n.sh.
B2: check-i18n.sh and lib.py disagree on what a transcreated page is
lib.l10n_mode in the base parses the front matter as YAML, and its comment explains why: a single-quoted l10n: 'transcreate' is valid and must be protected. is_transcreate in check-i18n.sh now only accepts the bare and double-quoted forms. At 8133286ac both sides accepted the same two forms. Now the pipeline skips a page that the shell lint treats as machine output.
I checked this on content/ru/_index.md with a changed digest. With l10n: 'transcreate', l10n_mode returns transcreate, and check-i18n.sh prints ::error::stale translation and exits 1. With l10n: "transcreate", the same drift is a warning and the exit code is 0. So a single-quoted page brings back both problems 5aebd263 fixes: CI goes red for unrelated PRs, and a bare update-digests re-stamps the page and hides the drift. l10n: transcreate # hand-written behaves the same way. No page in the tree uses these forms today. The fix is to accept them, or to read the value through lib.l10n_mode so one parser decides.
Non-blocking
- The zh-cn punctuation rule reads the
;of an HTML entity as prose.lint_translations.pyreports one finding incontent/zh-cn/_index.md: the;of→, then whitespace across stripped markup, then the next heading's社.check_typographyalso runs in the revise loop, where a finding like this fires again every round. - No test covers the footnote exclusion. With
(?!\^)removed, all 123 tests still pass, and[^1]: Footnote prosemasksFootnoteas a URL. - Still open from my last review: quoted
latest_version_idincheck-i18n.sh, no test for the superseded classifier,assertIn("injected", ...)passing on any label, and the "freshness: OK" summary printed after drift warnings. - The PR body becomes the merge commit message. It still says "91 tests pass" and "0 findings". At this head there are 123 tests and one lint finding.
At 1e44e855 I ran the suite (123 tests, all pass), check-i18n.sh (clean) and lint_translations.py (one finding, above). This PR is stacked on #623, and every file it changes, except check-i18n.sh and the new lint_translations.py, comes from #623, so it can only land after #623.
| # failure — and re-stamping it wholesale would silence the report while the | ||
| # drift persists. | ||
| is_transcreate() { | ||
| grep -m1 -E '^l10n:' "$1" 2>/dev/null | grep -qE '^l10n:[[:space:]]*"?transcreate"?[[:space:]]*$' |
There was a problem hiding this comment.
This accepts only the bare and double-quoted forms, while lib.l10n_mode parses YAML and also accepts 'transcreate' and transcreate # comment. A page in either form is skipped by the pipeline but fails here as a stale machine translation, and a bare update-digests re-stamps it. Details in B2.
Summary
Three quality improvements to the translation pipeline. No architectural change — the glossary + style-guide + back-translation + two-reviewer + digest design is kept and extended.
Stacked on
feat/i18n-pipeline, since that is where the pipeline lives.What
Link destinations are masked. Masking previously covered code, shortcodes and comments, so a URL sitting in ordinary prose (
[text](https://…)) was defended by a prompt rule alone — the model could mutate or localize it and nothing would catch it. URLs are now placeholders like any other protected span, while link text stays exposed and still gets translated.Two deterministic checks feed the existing revise loop.
integrity_findings()compares versions, bare CLI flags and do-not-translate terms between source and translation, catching a localized version separator (v1.5→v1,5), a dropped--flag, or a transliterated brand.check_typography()enforces the rules the style guides state: Russian guillemets, German„…“, Spanish inverted marks, Chinese full-width punctuation, European-Portuguese vocabulary leaking into pt-BR, Devanagari digits.Both look at prose only — markup, code, link targets and list markers are exempt, so the checks do not fire on ASCII punctuation that is correct inside an HTML attribute or a path.
The six per-language style guides were 3–6 lines each, yet the entire fluency and typography strategy rests on injecting them into the translate prompt and both reviewer prompts. They are now 80–100 lines each and work as an instruction set and a review rubric: register and address form, heading conventions, a decision rule for terms outside the glossary, typography, number/date formatting, the grammar traps of translating from English into that language, calque patterns with fixes, false friends, and a per-language checklist of MT failure modes. Every original decision is preserved and expanded around.
Fixed a live contradiction. Hard rule 5 in
prompts/translate.mdsaid to keep numbers unchanged, while several style guides require a decimal comma or a different date order in prose — the model was reading two incompatible instructions. The rule now separates a number's value (and any version or identifier: literal, reproduced character-for-character) from its formatting in prose (follows the language).lint_translations.pyapplies the typography rules to already-published pages, where a hand edit is otherwise never re-checked. Advisory by default,--strictto gate.Why
Only back-ticked code was structurally guaranteed; bare-prose commands, identifiers and link URLs were soft. On a docs site this dense with code, a silently mutated URL or flag is the most expensive failure mode available. The style guides were the second gap: the architecture already routes them into three prompts, but there was almost nothing in them to route.
Validation
Deliberately left as follow-ups
Glossary growth loop (harvesting recurring terminology findings into
glossary.yaml), aneeds_humanreviewer flag distinct fromrevise, and a severity-based soft hold on theauto-reviewedstamp.