From 90bb0871808a8e1b1c10804f4ba1d5cc0355848d Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Thu, 27 Aug 2026 18:54:17 +0000 Subject: [PATCH] i18n(client): fill a plural branch Crowdin omitted instead of dropping the message Complete a plural category the target locale needs from the message's own `other` branch during sync, so one missing branch no longer costs the whole translation. Romanian arrived with `one` and `other` for all 156 of its plurals and rendered English for every one of them. See docs/LOCALIZATION.md. --- client/scripts/locale-export-repairs.mjs | 32 ++++++++++++ client/scripts/locale-message-validation.mjs | 51 +++++++++++++++++++ client/scripts/locale-typography.mjs | 20 -------- client/scripts/sync-crowdin-translations.mjs | 20 ++++---- .../sync-crowdin-translations.spec.mjs | 32 ++++++++++-- docs/LOCALIZATION.md | 5 +- 6 files changed, 127 insertions(+), 33 deletions(-) create mode 100644 client/scripts/locale-export-repairs.mjs delete mode 100644 client/scripts/locale-typography.mjs diff --git a/client/scripts/locale-export-repairs.mjs b/client/scripts/locale-export-repairs.mjs new file mode 100644 index 000000000..fb54c786d --- /dev/null +++ b/client/scripts/locale-export-repairs.mjs @@ -0,0 +1,32 @@ +import { completePluralCategories } from './locale-message-validation.mjs' + +// Catalogs never carry a Unicode em dash. That is a house style rule rather than a correctness one, +// and translators reach for the character constantly, so rejecting the message would discard a sound +// translation over punctuation and stall the sync until somebody edits Crowdin by hand. The English +// source writes a plain hyphen in the same place, so the sync rewrites the character instead. +export function normalizeMessageTypography(message) { + return message.replaceAll('\u2014', '-') +} + +// Repairs applied to a Crowdin export before validation. Each one keeps a translation that is sound +// apart from a mechanical defect, and each disappears on its own once Crowdin carries a clean +// message, so nothing here hides a translation problem from the pull request body. +export function findExportRepairs({ catalogs }) { + const repairs = [] + for (const [locale, catalog] of catalogs) { + if (locale === 'en') continue + + for (const [key, message] of catalog) { + const kinds = [] + const normalized = normalizeMessageTypography(message) + if (normalized !== message) kinds.push('typography') + + const completed = completePluralCategories({ locale, message: normalized }) + if (completed) kinds.push('plural categories') + + if (kinds.length > 0) repairs.push({ locale, key, message: completed ?? normalized, kinds }) + } + } + + return repairs +} diff --git a/client/scripts/locale-message-validation.mjs b/client/scripts/locale-message-validation.mjs index f7c7bb6d5..6adf7b27b 100644 --- a/client/scripts/locale-message-validation.mjs +++ b/client/scripts/locale-message-validation.mjs @@ -164,6 +164,57 @@ function pluralOptionPounds(elements) { return totals } +// A translator types the whole ICU message by hand, so a locale with more plural categories than +// English loses every plural to a missing branch: Romanian arrived with `one` and `other` for all 156 +// of its plurals and rendered English for every one of them. The branch a translator writes as +// `other` is the one a reader sees for small counts, so copying it into the missing categories keeps +// the message in its own language, and the copy disappears as soon as Crowdin carries a real branch. +function pluralElements(elements, found = []) { + for (const element of elements) { + if (element.type === TYPE.plural) found.push(element) + if (element.type === TYPE.select || element.type === TYPE.plural) { + for (const option of Object.values(element.options)) pluralElements(option.value, found) + } + if (element.type === TYPE.tag) pluralElements(element.children, found) + } + return found +} + +export function completePluralCategories({ locale, message }) { + if (!isIcuPluralMessage(message)) return null + + let elements + try { + elements = parse(message, { captureLocation: true }) + } catch { + return null + } + + const insertions = [] + for (const element of pluralElements(elements)) { + const other = element.options.other + if (!other) continue + + const missing = pluralCategories(locale, element.pluralType).filter((category) => !(category in element.options)) + if (missing.length === 0) continue + + const text = message.slice(other.location.start.offset + 1, other.location.end.offset - 1) + insertions.push({ + offset: other.location.end.offset, + text: missing.map((category) => ` ${category} {${text}}`).join(''), + }) + } + + if (insertions.length === 0) return null + + let completed = message + for (const { offset, text } of insertions.sort((first, second) => second.offset - first.offset)) { + completed = `${completed.slice(0, offset)}${text}${completed.slice(offset)}` + } + + return completed +} + export function validateSlotCountMessage({ key, locale, message }) { if (!isIcuPluralMessage(message)) return [`${locale}: ICU plural syntax required for slot count message ${key}`] diff --git a/client/scripts/locale-typography.mjs b/client/scripts/locale-typography.mjs deleted file mode 100644 index 8ffcddf84..000000000 --- a/client/scripts/locale-typography.mjs +++ /dev/null @@ -1,20 +0,0 @@ -// Catalogs never carry a Unicode em dash. That is a house style rule rather than a correctness one, -// and translators reach for the character constantly, so rejecting the message would discard a sound -// translation over punctuation and stall the sync until somebody edits Crowdin by hand. The English -// source writes a plain hyphen in the same place, so the sync rewrites the character instead. -export function normalizeMessageTypography(message) { - return message.replaceAll('\u2014', '-') -} - -export function findTypographyDrift({ catalogs }) { - const drift = [] - for (const [locale, catalog] of catalogs) { - if (locale === 'en') continue - for (const [key, message] of catalog) { - const normalized = normalizeMessageTypography(message) - if (normalized !== message) drift.push({ locale, key, message: normalized }) - } - } - - return drift -} diff --git a/client/scripts/sync-crowdin-translations.mjs b/client/scripts/sync-crowdin-translations.mjs index c78b88b82..e25c82166 100644 --- a/client/scripts/sync-crowdin-translations.mjs +++ b/client/scripts/sync-crowdin-translations.mjs @@ -7,7 +7,7 @@ import { TARGET_CATALOGS, assertCrowdinTargetConfiguration } from './locale-conf import { findInvalidTargetMessages, flattenCatalog, validateCatalogs } from './locale-catalog-validation.mjs' import { collectSourceMessageKeys } from './locale-source-keys.mjs' import { PROTECTED_SOURCE_TERMS, findProtectedTermDrift } from './locale-protected-terms.mjs' -import { findTypographyDrift } from './locale-typography.mjs' +import { findExportRepairs } from './locale-export-repairs.mjs' const API = 'https://api.crowdin.com/api/v2' const SOURCE_PATH_SUFFIX = '/client/src/locales/en.json' @@ -372,9 +372,11 @@ export function formatTranslationLossReport(losses) { return `${lines.join('\n')}\n` } -async function reportSyncIssues({ rejections, corrections, normalizations, losses, reportPath }) { - if (normalizations.length > 0) { - console.log(`Normalized punctuation in ${normalizations.length} Crowdin messages`) +async function reportSyncIssues({ rejections, corrections, repairs, losses, reportPath }) { + if (repairs.length > 0) { + const kinds = repairs.flatMap(({ kinds: repaired }) => repaired) + const counts = [...new Set(kinds)].map((kind) => `${kind}=${kinds.filter((entry) => entry === kind).length}`) + console.log(`Repaired ${repairs.length} Crowdin messages before validation: ${counts.join(' ')}`) } if (losses.length > 0) { @@ -447,13 +449,13 @@ export async function syncCrowdinTranslations({ const corrections = findProtectedTermDrift({ catalogs, terms: protectedTerms }) for (const { locale, key, source } of corrections) catalogs.get(locale).set(key, source) - const normalizations = findTypographyDrift({ catalogs }) - for (const { locale, key, message } of normalizations) catalogs.get(locale).set(key, message) + const repairs = findExportRepairs({ catalogs }) + for (const { locale, key, message } of repairs) catalogs.get(locale).set(key, message) const { slotCountKeys } = await collectMessageKeys() const rejections = findInvalidTargetMessages({ catalogs, slotCountKeys }) for (const { locale, key } of rejections) catalogs.get(locale).delete(key) - const rewrittenLocales = new Set([...rejections, ...corrections, ...normalizations].map(({ locale }) => locale)) + const rewrittenLocales = new Set([...rejections, ...corrections, ...repairs].map(({ locale }) => locale)) for (const entry of downloaded) { if (rewrittenLocales.has(entry.locale)) entry.catalog = orderedSparseCatalog(reference, catalogs.get(entry.locale)) } @@ -473,9 +475,9 @@ export async function syncCrowdinTranslations({ await Promise.all( downloaded.map(({ locale, catalog }) => writeFile(path.join(outputDirectory, `${locale}.json`), `${JSON.stringify(catalog, null, 2)}\n`)), ) - await reportSyncIssues({ rejections, corrections, normalizations, losses, reportPath }) + await reportSyncIssues({ rejections, corrections, repairs, losses, reportPath }) console.log(`Synchronized ${downloaded.length} sparse translation catalogs from Crowdin`) - return { rejections, corrections, normalizations, losses } + return { rejections, corrections, repairs, losses } } if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { diff --git a/client/scripts/sync-crowdin-translations.spec.mjs b/client/scripts/sync-crowdin-translations.spec.mjs index fba9a1eea..30613bb2b 100644 --- a/client/scripts/sync-crowdin-translations.spec.mjs +++ b/client/scripts/sync-crowdin-translations.spec.mjs @@ -404,7 +404,7 @@ describe('Crowdin translation synchronization', () => { const outputDirectory = path.join(catalogDirectory, 'output') const catalogs = new Map([[targetCatalogs[0].languageId, { common: { save: 'Ulozit \u2014 hned' } }]]) - const { rejections, losses, normalizations } = await syncCrowdinTranslations({ + const { rejections, losses, repairs } = await syncCrowdinTranslations({ token: 'secret', fetchImpl: createSynchronizationFetch({ catalogs }), catalogDirectory, @@ -416,12 +416,38 @@ describe('Crowdin translation synchronization', () => { expect(rejections).toEqual([]) expect(losses).toEqual([]) - expect(normalizations).toEqual([{ locale: targetCatalogs[0].locale, key: 'common.save', message: 'Ulozit - hned' }]) + expect(repairs).toEqual([{ locale: targetCatalogs[0].locale, key: 'common.save', message: 'Ulozit - hned', kinds: ['typography'] }]) expect(JSON.parse(await readFile(path.join(outputDirectory, `${targetCatalogs[0].locale}.json`), 'utf8'))).toEqual({ common: { save: 'Ulozit - hned' }, }) }) + it('completes a plural category the target locale needs and Crowdin omitted', async () => { + const targetCatalogs = [TARGET_CATALOGS.find(({ locale }) => locale === 'ro')] + const referenceCatalog = { books: { count: '{count, plural, one {# book} other {# books}}' } } + const catalogDirectory = await createCatalogFixture(targetCatalogs, new Map(), referenceCatalog) + const outputDirectory = path.join(catalogDirectory, 'output') + const catalogs = new Map([['ro', { books: { count: '{count, plural, one {# carte} other {# de carti}}' } }]]) + + const { rejections, repairs } = await syncCrowdinTranslations({ + token: 'secret', + fetchImpl: createSynchronizationFetch({ identifiers: ['books.count'], catalogs }), + catalogDirectory, + outputDirectory, + targetCatalogs, + assertTargetConfiguration: async () => {}, + protectedTerms: [], + }) + + expect(rejections).toEqual([]) + expect(repairs).toEqual([ + { locale: 'ro', key: 'books.count', message: expect.stringContaining('few {# de carti}'), kinds: ['plural categories'] }, + ]) + expect(JSON.parse(await readFile(path.join(outputDirectory, 'ro.json'), 'utf8'))).toEqual({ + books: { count: '{count, plural, one {# carte} other {# de carti} few {# de carti}}' }, + }) + }) + it('checks translation retention before writing any files', async () => { const targetCatalogs = TARGET_CATALOGS.slice(0, 2) const keys = Array.from({ length: 40 }, (_, index) => `key${index}`) @@ -462,7 +488,7 @@ describe('Crowdin translation synchronization', () => { assertTargetConfiguration: async () => {}, protectedTerms: [], }), - ).resolves.toEqual({ rejections: [], corrections: [], normalizations: [], losses: [] }) + ).resolves.toEqual({ rejections: [], corrections: [], repairs: [], losses: [] }) expect(JSON.parse(await readFile(path.join(outputDirectory, `${targetCatalogs[0].locale}.json`), 'utf8'))).toEqual({ common: { save: 'Save' }, }) diff --git a/docs/LOCALIZATION.md b/docs/LOCALIZATION.md index 8ac63a066..f65f8b6d0 100644 --- a/docs/LOCALIZATION.md +++ b/docs/LOCALIZATION.md @@ -113,7 +113,10 @@ The retention check compares presence, never message content. Exports request on The check guards against a truncated or empty export, not against churn. Single translations leave Crowdin constantly: a reviewer unapproves a string, or an English edit invalidates the translation attached to it. A locale may lose up to twenty-five keys, or one percent of its translated keys where that is larger, before the run fails. Losses below that limit are listed in the pull request body and the English source renders for them. Failing on each one blocked every other locale over a single message and needed a hand-typed acknowledgement that a scheduled run cannot supply, which is why the export stalled repeatedly. -A message this workflow omits, because it carries HTML or breaks its ICU contract, is reported as a rejection rather than counted as a loss. Punctuation the catalogs prohibit is rewritten rather than rejected: a Unicode em dash returned by Crowdin becomes the hyphen the English source uses, so the translation survives and the catalogs stay free of the character. Fix the em dash in Crowdin when a translator's wording depends on it. +A message this workflow omits, because it carries HTML or breaks its ICU contract, is reported as a rejection rather than counted as a loss. Two mechanical defects are repaired instead of rejected, and both disappear on their own once Crowdin carries a clean message: + +- A Unicode em dash becomes the hyphen the English source uses, so the translation survives and the catalogs stay free of the character. +- A plural branch the target locale needs and Crowdin omitted is filled from the message's own `other` branch. A translator types the whole ICU message by hand, so a locale with more categories than English loses every plural to one missing branch: Romanian arrived with `one` and `other` for all 156 of its plurals and rendered English for every one of them. The filled branch reads in the translator's language rather than English, and it is still worth fixing properly in Crowdin, because a copied branch cannot carry the form the category actually needs. A manual workflow dispatch may pass exact `locale:message.key` values in `allowed_translation_losses`, separated by commas, to exclude known removals from the limit. Unknown, misspelled, and duplicate acknowledgements fail the run, so this input cannot act as a broad bypass.