From 9eff914aef03ed21e1e0e982985360fab15b8b60 Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Wed, 12 Aug 2026 16:54:51 +0000 Subject: [PATCH] fix(client): stop requiring plural categories no count can select The locale validator demanded every CLDR plural category, so each new ICU plural key took the daily Crowdin sync red until a `many` branch was hand-added for Spanish and French. That branch can never render: `many` selects at a million and over in Spanish, French, Italian and Portuguese, and only for fractional counts in Czech and Slovak, while every catalog plural counts whole items well below a million and intl-messageformat falls back to `other` regardless. Require only the categories an integer count 0..1000 can select, which is the same set every integer below a million selects, and keep `other` always so the categories everyday counts reach stay mandatory. Drop a target message that still fails validation from the sparse catalog instead of failing the whole export, so one broken string can no longer block the other 23 languages for days. Report the rejections in the run log and the pull request body, and keep the retention guard fatal when a rejected message would replace a translation that is already in Git. --- .github/workflows/crowdin-translations.yml | 23 ++++++-- client/scripts/locale-catalog-validation.mjs | 39 +++++++++---- .../locale-catalog-validation.spec.mjs | 5 +- client/scripts/locale-message-validation.mjs | 11 +++- .../locale-message-validation.spec.mjs | 21 +++++++ client/scripts/sync-crowdin-translations.mjs | 56 +++++++++++++++++-- .../sync-crowdin-translations.spec.mjs | 36 +++++++++++- 7 files changed, 166 insertions(+), 25 deletions(-) diff --git a/.github/workflows/crowdin-translations.yml b/.github/workflows/crowdin-translations.yml index c69eab09f..92851f154 100644 --- a/.github/workflows/crowdin-translations.yml +++ b/.github/workflows/crowdin-translations.yml @@ -59,6 +59,7 @@ jobs: env: CROWDIN_ALLOWED_TRANSLATION_LOSSES: ${{ inputs.allowed_translation_losses }} CROWDIN_PROJECT_ID: ${{ vars.CROWDIN_PROJECT_ID }} + CROWDIN_REJECTION_REPORT: ${{ runner.temp }}/crowdin-rejections.md CROWDIN_TOKEN: ${{ secrets.CROWDIN_TOKEN }} run: pnpm --filter client sync:crowdin @@ -72,6 +73,23 @@ jobs: scripts/sync-crowdin-translations.spec.mjs src/i18n/icu.spec.ts + - name: Compose pull request body + env: + PR_BODY: ${{ runner.temp }}/crowdin-pr-body.md + REJECTION_REPORT: ${{ runner.temp }}/crowdin-rejections.md + run: | + { + echo "Automated sparse translation export from Crowdin." + echo + echo "Untranslated entries are omitted so Vue I18n falls back to the English catalog." + } > "$PR_BODY" + if [ -s "$REJECTION_REPORT" ]; then + echo >> "$PR_BODY" + cat "$REJECTION_REPORT" >> "$PR_BODY" + cat "$REJECTION_REPORT" >> "$GITHUB_STEP_SUMMARY" + echo "::warning::Crowdin returned translations that failed catalog validation; they were omitted and are listed in the pull request body." + fi + - name: Create translation pull request uses: peter-evans/create-pull-request@5f6978faf089d4d20b00c7766989d076bb2fc7f1 # v8.1.1 with: @@ -111,7 +129,4 @@ jobs: delete-branch: true base: main title: "i18n(client): sync translations from Crowdin" - body: |- - Automated sparse translation export from Crowdin. - - Untranslated entries are omitted so Vue I18n falls back to the English catalog. + body-path: ${{ runner.temp }}/crowdin-pr-body.md diff --git a/client/scripts/locale-catalog-validation.mjs b/client/scripts/locale-catalog-validation.mjs index dbb841249..d36bfb88a 100644 --- a/client/scripts/locale-catalog-validation.mjs +++ b/client/scripts/locale-catalog-validation.mjs @@ -15,6 +15,18 @@ export function flattenCatalog(value, prefix = '', output = new Map()) { return output } +function messageErrors({ key, locale, message, referenceMessage, slotCountKeys }) { + if (referenceMessage === undefined) return [`${locale}: unexpected key ${key}`] + + const errors = [] + if (message.length === 0) errors.push(`${locale}: empty message ${key}`) + if (message.includes('\u2014')) errors.push(`${locale}: Unicode em dash is not allowed in ${key}`) + if (/<[^>]+>/.test(message)) errors.push(`${locale}: HTML is not allowed in ${key}`) + errors.push(...validateLocaleMessage({ key, locale, message, referenceMessage })) + if (slotCountKeys.has(key)) errors.push(...validateSlotCountMessage({ key, locale, message })) + return errors +} + export function validateCatalogs({ catalogs, referencedKeys = new Set(), slotCountKeys = new Set() }) { const reference = catalogs.get('en') if (!reference) throw new Error('English reference catalog is required') @@ -26,18 +38,25 @@ export function validateCatalogs({ catalogs, referencedKeys = new Set(), slotCou for (const [locale, catalog] of catalogs) { for (const [key, message] of catalog) { - const referenceMessage = reference.get(key) - if (referenceMessage === undefined) { - errors.push(`${locale}: unexpected key ${key}`) - continue - } - if (message.length === 0) errors.push(`${locale}: empty message ${key}`) - if (message.includes('\u2014')) errors.push(`${locale}: Unicode em dash is not allowed in ${key}`) - if (/<[^>]+>/.test(message)) errors.push(`${locale}: HTML is not allowed in ${key}`) - errors.push(...validateLocaleMessage({ key, locale, message, referenceMessage })) - if (slotCountKeys.has(key)) errors.push(...validateSlotCountMessage({ key, locale, message })) + errors.push(...messageErrors({ key, locale, message, referenceMessage: reference.get(key), slotCountKeys })) } } return errors } + +export function findInvalidTargetMessages({ catalogs, slotCountKeys = new Set() }) { + const reference = catalogs.get('en') + if (!reference) throw new Error('English reference catalog is required') + + const invalid = [] + for (const [locale, catalog] of catalogs) { + if (locale === 'en') continue + for (const [key, message] of catalog) { + const errors = messageErrors({ key, locale, message, referenceMessage: reference.get(key), slotCountKeys }) + if (errors.length > 0) invalid.push({ locale, key, errors }) + } + } + + return invalid +} diff --git a/client/scripts/locale-catalog-validation.spec.mjs b/client/scripts/locale-catalog-validation.spec.mjs index 25f34938a..ccc6b2d71 100644 --- a/client/scripts/locale-catalog-validation.spec.mjs +++ b/client/scripts/locale-catalog-validation.spec.mjs @@ -36,9 +36,6 @@ describe('locale catalog validation', () => { books: { count: '{count, plural, one {# kniha} other {# knih}}' }, } - expect(validateCatalogs({ catalogs: catalogs(target) })).toEqual([ - 'cs: ICU plural category few missing for books.count', - 'cs: ICU plural category many missing for books.count', - ]) + expect(validateCatalogs({ catalogs: catalogs(target) })).toEqual(['cs: ICU plural category few missing for books.count']) }) }) diff --git a/client/scripts/locale-message-validation.mjs b/client/scripts/locale-message-validation.mjs index 41627b768..f7c7bb6d5 100644 --- a/client/scripts/locale-message-validation.mjs +++ b/client/scripts/locale-message-validation.mjs @@ -101,8 +101,17 @@ export function analyzeIcuMessage(message) { } } +// Every plural in the catalogs counts whole items, and a library tops out well below a million. +// CLDR still lists categories that only fractional counts (Czech and Slovak `many`) or counts of a +// million and over (Spanish, French, Italian and Portuguese `many`) can ever select, so demanding a +// translation for them rejects otherwise correct Crowdin exports for a branch that cannot render. +// Sampling 0..1000 selects exactly the categories every integer below a million selects. +const REACHABLE_COUNT_SAMPLE = Array.from({ length: 1001 }, (_, index) => index) + function pluralCategories(locale, type) { - return new Intl.PluralRules(locale, { type }).resolvedOptions().pluralCategories + const rules = new Intl.PluralRules(locale, { type }) + const reachable = new Set(REACHABLE_COUNT_SAMPLE.map((count) => rules.select(count))) + return rules.resolvedOptions().pluralCategories.filter((category) => reachable.has(category)) } function exactSelectors(selectors) { diff --git a/client/scripts/locale-message-validation.spec.mjs b/client/scripts/locale-message-validation.spec.mjs index 334cf7d41..f4fee6f2d 100644 --- a/client/scripts/locale-message-validation.spec.mjs +++ b/client/scripts/locale-message-validation.spec.mjs @@ -86,6 +86,27 @@ describe('locale message validation', () => { ]) }) + it('accepts cardinal categories that only fractional or million-scale counts select', () => { + const reference = '{count, plural, one {# book} other {# books}}' + + expect(validate('es', reference, '{count, plural, one {# libro} other {# libros}}')).toEqual([]) + expect(validate('fr', reference, '{count, plural, one {# livre} other {# livres}}')).toEqual([]) + expect(validate('cs', reference, '{count, plural, one {# kniha} few {# knihy} other {# knih}}')).toEqual([]) + expect(validate('es', reference, '{count, plural, one {# libro} many {# de libros} other {# libros}}')).toEqual([]) + }) + + it('still requires categories that everyday counts select', () => { + const reference = '{count, plural, one {# book} other {# books}}' + + expect(validate('ru', reference, '{count, plural, one {# книга} other {# книги}}')).toEqual([ + 'ru: ICU plural category few missing for books.count', + 'ru: ICU plural category many missing for books.count', + ]) + expect(validate('cs', reference, '{count, plural, one {# kniha} other {# knih}}')).toEqual([ + 'cs: ICU plural category few missing for books.count', + ]) + }) + it('reports invalid ICU and rejects legacy branches for migrated keys', () => { const reference = '{count, plural, one {One book} other {# books}}' diff --git a/client/scripts/sync-crowdin-translations.mjs b/client/scripts/sync-crowdin-translations.mjs index 2e7e4b65e..fa7119f77 100644 --- a/client/scripts/sync-crowdin-translations.mjs +++ b/client/scripts/sync-crowdin-translations.mjs @@ -4,7 +4,7 @@ import { isIP } from 'node:net' import { pathToFileURL } from 'node:url' import path from 'node:path' import { TARGET_CATALOGS, assertCrowdinTargetConfiguration } from './locale-configuration.mjs' -import { flattenCatalog, validateCatalogs } from './locale-catalog-validation.mjs' +import { findInvalidTargetMessages, flattenCatalog, validateCatalogs } from './locale-catalog-validation.mjs' const API = 'https://api.crowdin.com/api/v2' const SOURCE_PATH_SUFFIX = '/client/src/locales/en.json' @@ -260,7 +260,7 @@ export function parseAllowedTranslationLosses(value = '') { return allowed } -export function findTranslationLosses({ locale, reference, current, exported }) { +export function findTranslationLosses({ locale, reference, current, exported, rejected = new Map() }) { const legacyComplete = current.size === reference.size && [...reference.keys()].every((key) => current.has(key)) const losses = [] @@ -270,7 +270,8 @@ export function findTranslationLosses({ locale, reference, current, exported }) const exportedMessage = exported.get(key) if (exportedMessage === undefined) { - losses.push({ locale, key, reason: 'missing from Crowdin export' }) + const rejection = rejected.get(key) + losses.push({ locale, key, reason: rejection ? `rejected by catalog validation - ${rejection[0]}` : 'missing from Crowdin export' }) } else if (currentMessage !== referenceMessage && exportedMessage === referenceMessage) { losses.push({ locale, key, reason: 'replaced by English source text' }) } @@ -285,13 +286,20 @@ export function assertTranslationRetention({ exportedCatalogs, allowedLosses = new Set(), targetCatalogs = TARGET_CATALOGS, + rejections = [], }) { + const rejectedByLocale = new Map() + for (const { locale, key, errors } of rejections) { + if (!rejectedByLocale.has(locale)) rejectedByLocale.set(locale, new Map()) + rejectedByLocale.get(locale).set(key, errors) + } + const losses = [] for (const { locale } of targetCatalogs) { const current = currentCatalogs.get(locale) const exported = exportedCatalogs.get(locale) if (!current || !exported) throw new Error(`Translation retention comparison is missing the ${locale} catalog`) - losses.push(...findTranslationLosses({ locale, reference, current, exported })) + losses.push(...findTranslationLosses({ locale, reference, current, exported, rejected: rejectedByLocale.get(locale) })) } const detected = new Set(losses.map(({ locale, key }) => `${locale}:${key}`)) @@ -308,6 +316,35 @@ export function assertTranslationRetention({ throw new Error(`Crowdin export would lose existing translations:\n${details.join('\n')}`) } +const MAX_REPORTED_REJECTIONS = 50 + +export function formatRejectionReport(rejections) { + if (rejections.length === 0) return '' + + const listed = rejections.slice(0, MAX_REPORTED_REJECTIONS) + const lines = [ + `### Rejected Crowdin messages (${rejections.length})`, + '', + 'These translations did not pass catalog validation and were omitted, so the English source renders instead. Fix them in Crowdin.', + '', + ...listed.map(({ errors }) => `- ${errors[0]}`), + ] + if (rejections.length > listed.length) lines.push(`- ...and ${rejections.length - listed.length} more`) + return `${lines.join('\n')}\n` +} + +async function reportRejections(rejections, reportPath) { + if (rejections.length === 0) { + if (reportPath) await writeFile(reportPath, '') + return + } + + console.log(`Rejected ${rejections.length} invalid Crowdin messages; the English source renders instead:`) + for (const { errors } of rejections.slice(0, MAX_REPORTED_REJECTIONS)) console.log(` ${errors[0]}`) + if (rejections.length > MAX_REPORTED_REJECTIONS) console.log(` ...and ${rejections.length - MAX_REPORTED_REJECTIONS} more`) + if (reportPath) await writeFile(reportPath, formatRejectionReport(rejections)) +} + export async function syncCrowdinTranslations({ token, projectId = '912891', @@ -317,6 +354,7 @@ export async function syncCrowdinTranslations({ allowedLosses = new Set(), targetCatalogs = TARGET_CATALOGS, assertTargetConfiguration = assertCrowdinTargetConfiguration, + reportPath = process.env.CROWDIN_REJECTION_REPORT || '', }) { if (!token) throw new Error('CROWDIN_TOKEN is required') await assertTargetConfiguration() @@ -350,6 +388,13 @@ export async function syncCrowdinTranslations({ const catalogs = new Map([['en', referenceMessages]]) for (const { locale, catalog } of downloaded) catalogs.set(locale, flattenCatalog(catalog)) + const rejections = findInvalidTargetMessages({ catalogs }) + const rejectedLocales = new Set(rejections.map(({ locale }) => locale)) + for (const { locale, key } of rejections) catalogs.get(locale).delete(key) + for (const entry of downloaded) { + if (rejectedLocales.has(entry.locale)) entry.catalog = orderedSparseCatalog(reference, catalogs.get(entry.locale)) + } + const errors = validateCatalogs({ catalogs }) if (errors.length > 0) throw new Error(`Crowdin export validation failed:\n${errors.join('\n')}`) assertTranslationRetention({ @@ -358,13 +403,16 @@ export async function syncCrowdinTranslations({ exportedCatalogs: catalogs, allowedLosses, targetCatalogs, + rejections, }) await mkdir(outputDirectory, { recursive: true }) await Promise.all( downloaded.map(({ locale, catalog }) => writeFile(path.join(outputDirectory, `${locale}.json`), `${JSON.stringify(catalog, null, 2)}\n`)), ) + await reportRejections(rejections, reportPath) console.log(`Synchronized ${downloaded.length} sparse translation catalogs from Crowdin`) + return { rejections } } 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 0cfb367a2..27de91d60 100644 --- a/client/scripts/sync-crowdin-translations.spec.mjs +++ b/client/scripts/sync-crowdin-translations.spec.mjs @@ -284,10 +284,42 @@ describe('Crowdin translation synchronization', () => { await expect(access(outputDirectory)).rejects.toThrow() }) - it('validates every downloaded catalog before writing any files', async () => { + it('omits an invalid downloaded message instead of failing the whole export', async () => { const targetCatalogs = TARGET_CATALOGS.slice(0, 2) const catalogDirectory = await createCatalogFixture(targetCatalogs) const outputDirectory = path.join(catalogDirectory, 'output') + const reportPath = path.join(catalogDirectory, 'rejections.md') + const catalogs = new Map([[targetCatalogs[0].languageId, { common: { save: 'Bad \u2014 value' } }]]) + + const { rejections } = await syncCrowdinTranslations({ + token: 'secret', + fetchImpl: createSynchronizationFetch({ catalogs }), + catalogDirectory, + outputDirectory, + targetCatalogs, + reportPath, + assertTargetConfiguration: async () => {}, + }) + + expect(rejections).toEqual([ + { + locale: targetCatalogs[0].locale, + key: 'common.save', + errors: [`${targetCatalogs[0].locale}: Unicode em dash is not allowed in common.save`], + }, + ]) + expect(JSON.parse(await readFile(path.join(outputDirectory, `${targetCatalogs[0].locale}.json`), 'utf8'))).toEqual({}) + expect(JSON.parse(await readFile(path.join(outputDirectory, `${targetCatalogs[1].locale}.json`), 'utf8'))).toEqual({ + common: { save: `Translated ${targetCatalogs[1].languageId}` }, + }) + expect(await readFile(reportPath, 'utf8')).toContain('Unicode em dash is not allowed in common.save') + }) + + it('fails when a rejected message would drop an existing translation', async () => { + const targetCatalogs = TARGET_CATALOGS.slice(0, 2) + const currentCatalogs = new Map([[targetCatalogs[0].locale, { common: { save: 'Ulozit' } }]]) + const catalogDirectory = await createCatalogFixture(targetCatalogs, currentCatalogs) + const outputDirectory = path.join(catalogDirectory, 'output') const catalogs = new Map([[targetCatalogs[0].languageId, { common: { save: 'Bad \u2014 value' } }]]) await expect( @@ -299,7 +331,7 @@ describe('Crowdin translation synchronization', () => { targetCatalogs, assertTargetConfiguration: async () => {}, }), - ).rejects.toThrow('Unicode em dash is not allowed') + ).rejects.toThrow('rejected by catalog validation') await expect(access(outputDirectory)).rejects.toThrow() })