diff --git a/client/scripts/locale-source-keys.mjs b/client/scripts/locale-source-keys.mjs new file mode 100644 index 000000000..4b56631b6 --- /dev/null +++ b/client/scripts/locale-source-keys.mjs @@ -0,0 +1,42 @@ +import { readdir, readFile } from 'node:fs/promises' +import path from 'node:path' + +const scriptDirectory = import.meta.dirname ?? path.join(process.cwd(), 'scripts') +const clientRoot = path.resolve(scriptDirectory, '..') + +async function sourceFiles(directory) { + const entries = await readdir(directory, { withFileTypes: true }) + const files = await Promise.all( + entries.map(async (entry) => { + const entryPath = path.join(directory, entry.name) + if (entry.isDirectory()) { + if (entry.name === '__tests__') return [] + return sourceFiles(entryPath) + } + return entry.isFile() && /\.(ts|vue)$/.test(entry.name) && !/\.(spec|test)\.ts$/.test(entry.name) ? [entryPath] : [] + }), + ) + return files.flat() +} + +export async function collectSourceMessageKeys(sourceDirectory = path.join(clientRoot, 'src')) { + const files = await sourceFiles(sourceDirectory) + const keys = new Set() + const slotCountKeys = new Set() + + for (const file of files) { + const source = await readFile(file, 'utf8') + for (const match of source.matchAll(/(?]*\bkeypath\s*=\s*(['"])([A-Za-z0-9_.-]+)\1/g)) { + keys.add(match[2]) + } + for (const match of source.matchAll(/]*\bkeypath\s*=\s*(['"])([A-Za-z0-9_.-]+)\1/g)) { + keys.add(match[2]) + slotCountKeys.add(match[2]) + } + } + + return { keys, slotCountKeys } +} diff --git a/client/scripts/sync-crowdin-translations.mjs b/client/scripts/sync-crowdin-translations.mjs index fa7119f77..29d476fe9 100644 --- a/client/scripts/sync-crowdin-translations.mjs +++ b/client/scripts/sync-crowdin-translations.mjs @@ -5,6 +5,7 @@ import { pathToFileURL } from 'node:url' import path from 'node:path' import { TARGET_CATALOGS, assertCrowdinTargetConfiguration } from './locale-configuration.mjs' import { findInvalidTargetMessages, flattenCatalog, validateCatalogs } from './locale-catalog-validation.mjs' +import { collectSourceMessageKeys } from './locale-source-keys.mjs' const API = 'https://api.crowdin.com/api/v2' const SOURCE_PATH_SUFFIX = '/client/src/locales/en.json' @@ -260,21 +261,17 @@ export function parseAllowedTranslationLosses(value = '') { return allowed } +// Exports are built with skipUntranslatedStrings, so Crowdin drops a key entirely once it stops +// carrying a translation. Retention therefore compares presence, never message content: a translation +// that equals the English source, such as Spanish "Error" or a product name, is a real translation. 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 = [] - for (const [key, currentMessage] of current) { - const referenceMessage = reference.get(key) - if (referenceMessage === undefined || (legacyComplete && currentMessage === referenceMessage)) continue + for (const key of current.keys()) { + if (!reference.has(key) || exported.has(key)) continue - const exportedMessage = exported.get(key) - if (exportedMessage === undefined) { - 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' }) - } + const rejection = rejected.get(key) + losses.push({ locale, key, reason: rejection ? `rejected by catalog validation - ${rejection[0]}` : 'missing from Crowdin export' }) } return losses @@ -354,6 +351,7 @@ export async function syncCrowdinTranslations({ allowedLosses = new Set(), targetCatalogs = TARGET_CATALOGS, assertTargetConfiguration = assertCrowdinTargetConfiguration, + collectMessageKeys = collectSourceMessageKeys, reportPath = process.env.CROWDIN_REJECTION_REPORT || '', }) { if (!token) throw new Error('CROWDIN_TOKEN is required') @@ -388,14 +386,15 @@ 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 { slotCountKeys } = await collectMessageKeys() + const rejections = findInvalidTargetMessages({ catalogs, slotCountKeys }) 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 }) + const errors = validateCatalogs({ catalogs, slotCountKeys }) if (errors.length > 0) throw new Error(`Crowdin export validation failed:\n${errors.join('\n')}`) assertTranslationRetention({ reference: referenceMessages, diff --git a/client/scripts/sync-crowdin-translations.spec.mjs b/client/scripts/sync-crowdin-translations.spec.mjs index 27de91d60..53c4ab788 100644 --- a/client/scripts/sync-crowdin-translations.spec.mjs +++ b/client/scripts/sync-crowdin-translations.spec.mjs @@ -3,6 +3,8 @@ import { tmpdir } from 'node:os' import path from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' import { assertCrowdinTargetConfiguration, validateCrowdinTargetConfiguration } from './locale-configuration.mjs' +import { flattenCatalog, validateCatalogs } from './locale-catalog-validation.mjs' +import { collectSourceMessageKeys } from './locale-source-keys.mjs' import { TARGET_CATALOGS, assertSafeDownloadUrl, @@ -26,11 +28,11 @@ afterEach(async () => { await Promise.all(temporaryDirectories.splice(0).map((directory) => rm(directory, { recursive: true, force: true }))) }) -async function createCatalogFixture(targetCatalogs, currentCatalogs = new Map()) { +async function createCatalogFixture(targetCatalogs, currentCatalogs = new Map(), referenceCatalog = { common: { save: 'Save' } }) { const directory = await mkdtemp(path.join(tmpdir(), 'bookorbit-crowdin-sync-')) temporaryDirectories.push(directory) await mkdir(directory, { recursive: true }) - await writeFile(path.join(directory, 'en.json'), `${JSON.stringify({ common: { save: 'Save' } }, null, 2)}\n`) + await writeFile(path.join(directory, 'en.json'), `${JSON.stringify(referenceCatalog, null, 2)}\n`) await Promise.all( targetCatalogs.map(({ locale }) => writeFile(path.join(directory, `${locale}.json`), `${JSON.stringify(currentCatalogs.get(locale) ?? {}, null, 2)}\n`), @@ -84,7 +86,7 @@ describe('Crowdin translation synchronization', () => { }) }) - it('protects real translations in complete legacy catalogs without treating English fallbacks as translations', () => { + it('protects a complete catalog as strictly as a sparse one', () => { const referenceMessages = new Map([ ['common.save', 'Save'], ['common.cancel', 'Cancel'], @@ -97,6 +99,7 @@ describe('Crowdin translation synchronization', () => { expect(findTranslationLosses({ locale: 'cs', reference: referenceMessages, current, exported })).toEqual([ { locale: 'cs', key: 'common.save', reason: 'missing from Crowdin export' }, + { locale: 'cs', key: 'common.cancel', reason: 'missing from Crowdin export' }, ]) }) @@ -112,16 +115,69 @@ describe('Crowdin translation synchronization', () => { ]) }) - it('rejects translations that Crowdin replaces with English source text', () => { - const referenceMessages = new Map([['common.save', 'Save']]) - const current = new Map([['common.save', 'Uložit']]) - const exported = new Map([['common.save', 'Save']]) + it.each([ + ['es', 'bookDock.tab.error', 'Error', 'error'], + ['es', 'book.detail.readingLog.months.feb', 'Feb', 'febrero'], + ['es', 'settings.metadata.providers.fields.cookie', 'Cookie', 'galleta'], + ['fr', 'settings.metadata.fields.description', 'Description', 'Descriptif'], + ['pt', 'settings.integrations.tabs.readwise', 'Readwise', 'Leitura'], + ])('keeps %s:%s when Crowdin translates it as the English source text', (locale, key, source, currentMessage) => { + expect( + findTranslationLosses({ + locale, + reference: new Map([[key, source]]), + current: new Map([[key, currentMessage]]), + exported: new Map([[key, source]]), + }), + ).toEqual([]) + }) - expect(findTranslationLosses({ locale: 'cs', reference: referenceMessages, current, exported })).toEqual([ - { locale: 'cs', key: 'common.save', reason: 'replaced by English source text' }, + it('reports why catalog validation dropped a key instead of a bare absence', () => { + expect( + findTranslationLosses({ + locale: 'cs', + reference: new Map([['common.save', 'Save']]), + current: new Map([['common.save', 'Uložit']]), + exported: new Map(), + rejected: new Map([['common.save', ['cs: Unicode em dash is not allowed in common.save']]]), + }), + ).toEqual([ + { + locale: 'cs', + key: 'common.save', + reason: 'rejected by catalog validation - cs: Unicode em dash is not allowed in common.save', + }, ]) }) + it('ignores catalog keys that English no longer defines', () => { + expect( + findTranslationLosses({ + locale: 'cs', + reference: new Map([['common.save', 'Save']]), + current: new Map([['common.removed', 'Odstraněno']]), + exported: new Map(), + }), + ).toEqual([]) + }) + + it('needs no acknowledgement when Crowdin corrects a translation to the English source', () => { + const referenceMessages = new Map([['bookDock.tab.error', 'Error']]) + const currentCatalogs = new Map(TARGET_CATALOGS.map(({ locale }) => [locale, new Map()])) + const exportedCatalogs = new Map(TARGET_CATALOGS.map(({ locale }) => [locale, new Map([['bookDock.tab.error', 'Error']])])) + currentCatalogs.set('es', new Map([['bookDock.tab.error', 'error']])) + + expect(() => assertTranslationRetention({ reference: referenceMessages, currentCatalogs, exportedCatalogs })).not.toThrow() + expect(() => + assertTranslationRetention({ + reference: referenceMessages, + currentCatalogs, + exportedCatalogs, + allowedLosses: parseAllowedTranslationLosses('es:bookDock.tab.error'), + }), + ).toThrow('es:bookDock.tab.error - acknowledgement does not match an exported loss') + }) + it('requires exact acknowledgements for intentional translation losses', () => { const referenceMessages = new Map([['common.save', 'Save']]) const currentCatalogs = new Map(TARGET_CATALOGS.map(({ locale }) => [locale, new Map()])) @@ -154,7 +210,7 @@ describe('Crowdin translation synchronization', () => { expect(() => parseAllowedTranslationLosses('cs:common.save,cs:common.save')).toThrow('Duplicate translation loss acknowledgement') }) - it('requests untranslated-string omission and downloads the bounded export', async () => { + it('omits untranslated strings from the export so retention can read absence as loss', async () => { const fetchImpl = vi .fn() .mockResolvedValueOnce( @@ -354,4 +410,81 @@ describe('Crowdin translation synchronization', () => { ).rejects.toThrow('missing from Crowdin export') await expect(access(outputDirectory)).rejects.toThrow() }) + + it('writes a Crowdin correction that replaces a translation with the English source text', 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: 'Save' } }]]) + + await expect( + syncCrowdinTranslations({ + token: 'secret', + fetchImpl: createSynchronizationFetch({ catalogs }), + catalogDirectory, + outputDirectory, + targetCatalogs, + assertTargetConfiguration: async () => {}, + }), + ).resolves.toEqual({ rejections: [] }) + expect(JSON.parse(await readFile(path.join(outputDirectory, `${targetCatalogs[0].locale}.json`), 'utf8'))).toEqual({ + common: { save: 'Save' }, + }) + }) + + it('drops a slot count violation so the catalog it writes passes locale validation', async () => { + const key = 'tools.bulkRename.confirmDialog.body' + const referenceCatalog = { tools: { bulkRename: { confirmDialog: { body: '{count, plural, one {Rename # file} other {Rename # files}}' } } } } + const targetCatalogs = TARGET_CATALOGS.slice(0, 2) + const catalogDirectory = await createCatalogFixture(targetCatalogs, new Map(), referenceCatalog) + const outputDirectory = path.join(catalogDirectory, 'output') + const catalogs = new Map([ + [ + targetCatalogs[0].languageId, + { tools: { bulkRename: { confirmDialog: { body: '{count, plural, one {Rename # file} few {Rename # files} other {Rename files}}' } } } }, + ], + [targetCatalogs[1].languageId, {}], + ]) + + const { rejections } = await syncCrowdinTranslations({ + token: 'secret', + fetchImpl: createSynchronizationFetch({ identifiers: [key], catalogs }), + catalogDirectory, + outputDirectory, + targetCatalogs, + assertTargetConfiguration: async () => {}, + collectMessageKeys: async () => ({ keys: new Set([key]), slotCountKeys: new Set([key]) }), + }) + + expect(rejections).toEqual([ + { + locale: targetCatalogs[0].locale, + key, + errors: [`${targetCatalogs[0].locale}: slot count message ${key} must render exactly one # in every branch`], + }, + ]) + expect(JSON.parse(await readFile(path.join(outputDirectory, `${targetCatalogs[0].locale}.json`), 'utf8'))).toEqual({}) + + const written = new Map([['en', flattenCatalog(referenceCatalog)]]) + for (const { locale } of targetCatalogs) { + written.set(locale, flattenCatalog(JSON.parse(await readFile(path.join(outputDirectory, `${locale}.json`), 'utf8')))) + } + expect(validateCatalogs({ catalogs: written, slotCountKeys: new Set([key]) })).toEqual([]) + }) + + it('scans the same sources locale validation scans', async () => { + const directory = await mkdtemp(path.join(tmpdir(), 'bookorbit-locale-source-')) + temporaryDirectories.push(directory) + await mkdir(path.join(directory, '__tests__'), { recursive: true }) + await writeFile(path.join(directory, 'Widget.vue'), '') + await writeFile(path.join(directory, 'helper.ts'), "export const label = t('common.save')\n") + await writeFile(path.join(directory, 'helper.spec.ts'), "t('ignored.spec')\n") + await writeFile(path.join(directory, '__tests__', 'helper.ts'), "t('ignored.tests')\n") + + await expect(collectSourceMessageKeys(directory)).resolves.toEqual({ + keys: new Set(['tools.bulkRename.confirmDialog.body', 'common.save']), + slotCountKeys: new Set(['tools.bulkRename.confirmDialog.body']), + }) + }) }) diff --git a/client/scripts/validate-locales.mjs b/client/scripts/validate-locales.mjs index 53309ac0b..e75988438 100644 --- a/client/scripts/validate-locales.mjs +++ b/client/scripts/validate-locales.mjs @@ -3,48 +3,11 @@ import { fileURLToPath } from 'node:url' import path from 'node:path' import { SUPPORTED_LOCALES } from './locale-configuration.mjs' import { flattenCatalog, validateCatalogs } from './locale-catalog-validation.mjs' +import { collectSourceMessageKeys } from './locale-source-keys.mjs' const clientRoot = fileURLToPath(new URL('..', import.meta.url)) const localesDirectory = path.join(clientRoot, 'src/locales') -async function sourceFiles(directory) { - const entries = await readdir(directory, { withFileTypes: true }) - const files = await Promise.all( - entries.map(async (entry) => { - const entryPath = path.join(directory, entry.name) - if (entry.isDirectory()) { - if (entry.name === '__tests__') return [] - return sourceFiles(entryPath) - } - return entry.isFile() && /\.(ts|vue)$/.test(entry.name) && !/\.(spec|test)\.ts$/.test(entry.name) ? [entryPath] : [] - }), - ) - return files.flat() -} - -async function sourceMessageKeys() { - const sourceDirectory = path.join(clientRoot, 'src') - const files = await sourceFiles(sourceDirectory) - const keys = new Set() - const slotCountKeys = new Set() - - for (const file of files) { - const source = await readFile(file, 'utf8') - for (const match of source.matchAll(/(?]*\bkeypath\s*=\s*(['"])([A-Za-z0-9_.-]+)\1/g)) { - keys.add(match[2]) - } - for (const match of source.matchAll(/]*\bkeypath\s*=\s*(['"])([A-Za-z0-9_.-]+)\1/g)) { - keys.add(match[2]) - slotCountKeys.add(match[2]) - } - } - - return { keys, slotCountKeys } -} - const localeFiles = (await readdir(localesDirectory)).filter((file) => file.endsWith('.json')).sort() const expectedFiles = SUPPORTED_LOCALES.map((locale) => `${locale}.json`).sort() if (JSON.stringify(localeFiles) !== JSON.stringify(expectedFiles)) { @@ -60,7 +23,7 @@ for (const locale of SUPPORTED_LOCALES) { const reference = catalogs.get('en') if (!reference) throw new Error('English reference catalog is required') -const { keys: referencedKeys, slotCountKeys } = await sourceMessageKeys() +const { keys: referencedKeys, slotCountKeys } = await collectSourceMessageKeys() const errors = validateCatalogs({ catalogs, referencedKeys, slotCountKeys }) if (errors.length > 0) { diff --git a/docs/LOCALIZATION.md b/docs/LOCALIZATION.md index fa78a9b81..c6a86bc97 100644 --- a/docs/LOCALIZATION.md +++ b/docs/LOCALIZATION.md @@ -99,7 +99,7 @@ After an English source change reaches `main`: 2. Translators update target strings in Crowdin. 3. The scheduled or manually dispatched `Crowdin Translation Sync` workflow verifies that Crowdin's source keys match `en.json`. 4. The workflow requests only translated strings, removes the empty values Crowdin emits for untranslated nested JSON entries, and validates the resulting sparse catalogs. -5. Before writing, the workflow rejects exports that omit an existing translation or replace one with English source text. +5. Before writing, the workflow rejects exports that omit an existing translation. 6. Only after validation and retention checks pass, the workflow updates `l10n_main` and opens a pull request to `main`. 7. CI verifies that the pull request changes only the twenty-four target catalogs and runs the normal client checks. 8. A maintainer reviews and squash-merges the pull request. @@ -109,7 +109,7 @@ Deleting that branch is required, not tidiness. It guarantees that every later e Translation pull requests retain the configured `i18n(client)` title and commit format. The `i18n` commit type produces a patch release and an Internationalization release-note section. `CROWDIN_PR_TOKEN` must contain a fine-grained GitHub token with repository contents and pull-request write access. A separate token is required because pull requests created with the workflow's default `GITHUB_TOKEN` do not trigger normal pull-request workflows. -The retention check distinguishes the initial complete legacy catalogs from later sparse catalogs. In a complete legacy catalog, it protects values that differ from English and ignores copied English fallbacks. Once a catalog is sparse, every existing key is treated as an intentional translation, including technical terms that legitimately match English. +The retention check compares presence, never message content. Exports request only translated strings, so Crowdin omits a key as soon as it stops carrying a translation, and every key already present in a target catalog must come back in the export. A translation that legitimately matches the English source, such as `Error` in Spanish or a product name left untranslated, is a real translation and never counts as a loss. Reading it as one would reject the routine corrections that translators make to short labels, abbreviations, and loanwords. An intentional removal requires a manual workflow dispatch with its exact `locale:message.key` value in `allowed_translation_losses`. Separate multiple acknowledgements with commas. Unknown, misspelled, duplicate, and unused acknowledgements fail the run, so this input cannot act as a broad bypass. Scheduled runs never acknowledge translation loss automatically.