From 7d3935b4b1b2e204f9969e7704e072fb0ea73e95 Mon Sep 17 00:00:00 2001 From: Arvid Andersson Date: Tue, 15 Sep 2026 00:29:55 +0200 Subject: [PATCH 1/2] Translate entries gettext marked fuzzy A fuzzy entry holds a translation msgmerge guessed from a different msgid. msgfmt leaves those out of the compiled catalog, so the app shows the source string while the file looks translated. We neither replaced the guess nor reported it, so the string stayed English. - Count a fuzzy target entry as missing, so it is sent for translation - Leave fuzzy entries out of an upload, so the guess is never recorded as a translation the server would hand back as already done - Clear the flag once every form the catalog declares has a real translation, so a partial response cannot publish a remaining guess --- src/utils/po-surgical.ts | 75 +++++++++++- src/utils/po-utils.ts | 30 ++++- src/utils/translation-utils.ts | 47 +++++++- tests/utils/po-surgical.test.js | 159 +++++++++++++++++++++++++- tests/utils/po-utils.test.js | 119 +++++++++++++++++++ tests/utils/translation-utils.test.js | 86 ++++++++++++++ 6 files changed, 503 insertions(+), 13 deletions(-) diff --git a/src/utils/po-surgical.ts b/src/utils/po-surgical.ts index 8edc359..34ee202 100644 --- a/src/utils/po-surgical.ts +++ b/src/utils/po-surgical.ts @@ -1,5 +1,5 @@ import { po } from 'gettext-parser'; -import { createUniqueKey, normalizeStringValue, parseUniqueKey, parsePoFile, PLURAL_PREFIX, extractNPlurals, MAX_PLURAL_FORMS } from './po-utils.js'; +import { createUniqueKey, normalizeStringValue, parseUniqueKey, parsePoFile, parsePoFlags, PLURAL_PREFIX, extractNPlurals, MAX_PLURAL_FORMS } from './po-utils.js'; /** * Surgical update of .po file - only modify lines that actually changed @@ -32,6 +32,7 @@ export function surgicalUpdatePoFile( // Quick check, if all translations match normalized content exactly, // return original unchanged to preserve formatting const parsed = po.parse(originalContent); + const nplurals = extractNPlurals(parsed.headers || {}); let allIdentical = true; Object.entries(parsed.translations).forEach(([context, entries]) => { @@ -42,6 +43,10 @@ export function surgicalUpdatePoFile( const contextValue = context !== '' ? context : undefined; const uniqueKey = createUniqueKey(msgid, contextValue); + if (fuzzyEntryIsFullyTranslated(entry, uniqueKey, translations, nplurals, options?.keyMappings?.[uniqueKey])) { + allIdentical = false; + } + if (translations[uniqueKey]) { const newValue = translations[uniqueKey]; const currentValue = entry.msgid_plural @@ -100,6 +105,57 @@ export function surgicalUpdatePoFile( return processLineByLine(originalContent, translations, options); } +// A fuzzy entry needs rewriting even when the translation matches the guess +// gettext made, because the flag itself is what keeps it out of the compiled +// catalog. Every existing form must be covered: clearing the flag while one +// form still holds a guess would publish that guess. +function fuzzyEntryIsFullyTranslated( + entry: any, + uniqueKey: string, + translations: Record, + nplurals: number, + mappedKey?: string +): boolean { + if (!parsePoFlags(entry.comments?.flag)?.includes('fuzzy')) return false; + + // Whitespace-only counts as empty everywhere else here, so it must not be + // enough to clear the flag either. gettext also requires msgid and msgstr to + // agree on a leading and trailing newline; a translation that breaks that + // makes the whole catalog fail to compile, so it cannot count as done. + const agreesOnEdges = (value: string, msgid: string) => + value.startsWith('\n') === msgid.startsWith('\n') && + value.endsWith('\n') === msgid.endsWith('\n'); + const filled = (key: string, msgid: string) => { + const value = translations[key]; + if (value === undefined) return false; + return normalizeStringValue(value) !== '' && agreesOnEdges(value, msgid); + }; + // Versioning re-keys the payload to the new msgid, so accept either shape. + const keyed = (suffix: string, msgid: string) => + filled(uniqueKey + suffix, msgid) || Boolean(mappedKey && filled(mappedKey + suffix, msgid)); + + if (!entry.msgid_plural) return keyed('', entry.msgid); + + const { context } = parseUniqueKey(uniqueKey); + // Form 1 can also arrive under the msgid_plural text itself, the shape the + // detection pass below accepts for catalogs written before __plural_N keys. + const msgidPluralKey = createUniqueKey(entry.msgid_plural, context); + + // Count against the header, not the slots the file happens to carry: the + // writer only fills existing slots, so a file short of its declared forms + // would otherwise lose the rest while looking fully translated. + const formCount = Math.max(entry.msgstr.length, nplurals); + for (let i = 0; i < formCount; i++) { + if (i >= entry.msgstr.length) return false; + const formMsgid = i === 0 ? entry.msgid : entry.msgid_plural; + if (i === 0 && keyed('', formMsgid)) continue; + if (keyed(`${PLURAL_PREFIX}${i}`, formMsgid)) continue; + if (i === 1 && entry.msgid_plural !== entry.msgid && filled(msgidPluralKey, formMsgid)) continue; + return false; + } + return true; +} + /** * Group plural translations together */ @@ -289,6 +345,7 @@ function processLineByLine( const lines = content.split('\n'); const result: string[] = []; const parsed = po.parse(content); + const entryNplurals = extractNPlurals(parsed.headers || {}); const changesToMake = new Map(); const translatedEntries = new Set(); // Entries receiving a non-empty translation, by their own uniqueKey const msgidChanges = new Map(); // Map old msgid → new msgid (for versioning) @@ -327,6 +384,17 @@ function processLineByLine( return; } + // A source-language write echoes the msgid back, leaves the entry alone, + // and must leave its flag alone too: clearing it would publish the guess + // still sitting in msgstr. + const isSourceLanguageEcho = translations[actualNewKey] === msgid && + options?.sourceLanguage === options?.targetLanguage; + + if (!isSourceLanguageEcho && + fuzzyEntryIsFullyTranslated(entry, uniqueKey, translations, entryNplurals, actualNewKey)) { + translatedEntries.add(uniqueKey); + } + if (translations[actualNewKey]) { const newValue = translations[actualNewKey]; @@ -343,7 +411,6 @@ function processLineByLine( if (currentValue !== normalizedNewValue || foundViaMapping) { changesToMake.set(uniqueKey, newValue); - translatedEntries.add(uniqueKey); // Also track the new key so addNewEntries doesn't add it as a duplicate if (foundViaMapping && actualNewKey !== uniqueKey) { changesToMake.set(actualNewKey, newValue); @@ -367,7 +434,6 @@ function processLineByLine( if (currentPluralValue !== normalizedNewPluralValue || foundViaMapping) { changesToMake.set(oldPluralKey, newPluralValue); - translatedEntries.add(uniqueKey); // Also track the new plural key so addNewEntries doesn't add it as a duplicate if (foundViaMapping && newPluralKey !== oldPluralKey) { changesToMake.set(newPluralKey, newPluralValue); @@ -383,7 +449,6 @@ function processLineByLine( if (currentPluralValue !== normalizedNewPluralValue) { changesToMake.set(pluralKey, translations[pluralKey]); - translatedEntries.add(uniqueKey); } } } @@ -391,7 +456,7 @@ function processLineByLine( } }); - if (changesToMake.size === 0 && entriesToRemove.size === 0) { + if (changesToMake.size === 0 && entriesToRemove.size === 0 && translatedEntries.size === 0) { // Still need to add new entries even if no existing entries need changes const newEntries = addNewEntries(translations, parsed, changesToMake, options); if (newEntries.length > 0) { diff --git a/src/utils/po-utils.ts b/src/utils/po-utils.ts index f36884e..3bc6ecf 100644 --- a/src/utils/po-utils.ts +++ b/src/utils/po-utils.ts @@ -129,7 +129,7 @@ export function normalizeReferences(reference: string | string[]): string[] { // gettext allows flags on one `#,` line or spread across several, and // gettext-parser joins the latter with a newline rather than a comma. -function parsePoFlags(flag: string | undefined): string[] | undefined { +export function parsePoFlags(flag: string | undefined): string[] | undefined { if (!flag) return undefined; const flags = flag.split(/[,\n]\s*/).map(f => f.trim()).filter(Boolean); return flags.length > 0 ? flags : undefined; @@ -218,6 +218,19 @@ export function poEntriesToApiFormat( const isSourceLanguage = options?.sourceLanguage && options?.currentLanguage && options.sourceLanguage === options.currentLanguage; + // A fuzzy msgstr is gettext's guess from a different msgid, and msgfmt leaves + // it out of the compiled catalog. Uploading it would record a translation the + // app never shows, so the entry is left out and reported missing instead. + // Only when both locales are known and differ: callers that omit them (change + // detection, flag inspection) still need every entry. + const isKnownTargetLanguage = Boolean( + options?.sourceLanguage && options?.currentLanguage && + options.sourceLanguage !== options.currentLanguage + ); + if (isKnownTargetLanguage && poFlags?.includes('fuzzy')) { + return; + } + if (entry.msgid_plural) { for (let i = 0; i < nplurals; i++) { const suffix = i === 0 ? '' : `${PLURAL_PREFIX}${i}`; @@ -361,9 +374,19 @@ export function findMissingPoTranslations( const hasMetadata = Object.keys(metadata).length > 0; + // gettext marks an entry `fuzzy` when msgmerge guessed a translation from a + // similar msgid — usually after a source string was reworded. msgfmt excludes + // fuzzy entries from the compiled .mo, so the app renders the SOURCE string: + // the key looks translated in the file while being untranslated to the user. + // Treat it as missing so the guess is replaced, matching how Weblate, + // Transifex, Lokalise and Phrase all import fuzzy as needs-review, not done. + // Read from the TARGET entry: `poFlags` above belongs to the source entry. + const targetIsFuzzy = parsePoFlags(targetEntry?.comments?.flag)?.includes('fuzzy') ?? false; + if (entry.msgid_plural) { for (let i = 0; i < targetNplurals; i++) { - const isEmpty = !targetEntry || + const isEmpty = targetIsFuzzy || + !targetEntry || !targetEntry.msgstr || !targetEntry.msgstr[i] || targetEntry.msgstr[i].trim() === ''; @@ -384,7 +407,8 @@ export function findMissingPoTranslations( } } else { // Handle regular (non-plural) entries - const isEmpty = !targetEntry || + const isEmpty = targetIsFuzzy || + !targetEntry || !targetEntry.msgstr || !targetEntry.msgstr[0] || targetEntry.msgstr[0].trim() === ''; diff --git a/src/utils/translation-utils.ts b/src/utils/translation-utils.ts index 60e3f40..7f076bc 100644 --- a/src/utils/translation-utils.ts +++ b/src/utils/translation-utils.ts @@ -5,7 +5,7 @@ import { ProjectConfig, } from '../types/index.js'; import { TranslationBatch } from './translation-processor.js'; -import { findMissingPoTranslations, createUniqueKey, PLURAL_PREFIX } from './po-utils.js'; +import { findMissingPoTranslations, createUniqueKey, PLURAL_PREFIX, PLURAL_SUFFIX_REGEX } from './po-utils.js'; import { filterKeys, RemovedKey } from './ignore-keys.js'; const POT_EXTENSION = '.pot'; @@ -425,8 +425,49 @@ export function batchKeysWithMissing( const allKeys = Object.entries(data.keys); const chunkedKeys: Array]>> = []; - for (let i = 0; i < allKeys.length; i += MAX_BATCH_SIZE) { - chunkedKeys.push(allKeys.slice(i, i + MAX_BATCH_SIZE)); + // Chunk on plural-group boundaries. A form that lands in a different batch + // than its siblings arrives as a partial payload, which leaves the entry + // half-written and, for a fuzzy entry, never clears its flag. + // Only gettext catalogs use the __plural_N suffix to mean "another form of + // the same entry"; elsewhere it is an ordinary key name. Grouped by base key + // rather than adjacency, since merging several locales can interleave a + // family's forms. A family larger than the cap is still split: the cap is a + // request-size limit, not something to bypass. + const isPoFormat = ['po', 'pot'].includes(sourceFile.format.toLowerCase()); + const groups: Array]>> = []; + + if (isPoFormat) { + const groupsByBase = new Map]>>(); + for (const entry of allKeys) { + const base = entry[0].replace(PLURAL_SUFFIX_REGEX, ''); + const group = groupsByBase.get(base); + if (group) { + group.push(entry); + } else { + groupsByBase.set(base, [entry]); + } + } + for (const group of groupsByBase.values()) { + for (let i = 0; i < group.length; i += MAX_BATCH_SIZE) { + groups.push(group.slice(i, i + MAX_BATCH_SIZE)); + } + } + } else { + for (const entry of allKeys) { + groups.push([entry]); + } + } + + let chunk: Array<[string, Record]> = []; + for (const group of groups) { + if (chunk.length > 0 && chunk.length + group.length > MAX_BATCH_SIZE) { + chunkedKeys.push(chunk); + chunk = []; + } + chunk.push(...group); + } + if (chunk.length > 0) { + chunkedKeys.push(chunk); } for (const keyChunk of chunkedKeys) { diff --git a/tests/utils/po-surgical.test.js b/tests/utils/po-surgical.test.js index e906b48..26c5da1 100644 --- a/tests/utils/po-surgical.test.js +++ b/tests/utils/po-surgical.test.js @@ -79,6 +79,161 @@ msgstr "Hallo %(name)s" expect(result).toMatch(/#, python-format\nmsgid "Hello %\(name\)s"/); }); + test('clears fuzzy even when the returned translation is identical to the guess', () => { + const original = `msgid "" +msgstr "" +"Content-Type: text/plain; charset=UTF-8\\n" + +#, fuzzy +msgid "Cancel" +msgstr "Avbryt" +`; + const result = surgicalUpdatePoFile(original, { 'Cancel': 'Avbryt' }); + + expect(result).toContain('msgstr "Avbryt"'); + expect(result).not.toContain('fuzzy'); + }); + + test('clears fuzzy on a plural when every form is returned, even if identical', () => { + const original = `msgid "" +msgstr "" +"Content-Type: text/plain; charset=UTF-8\\n" +"Plural-Forms: nplurals=2; plural=(n != 1);\\n" + +#, fuzzy +msgid "item" +msgid_plural "items" +msgstr[0] "sak" +msgstr[1] "saker" +`; + const result = surgicalUpdatePoFile(original, { 'item': 'sak', 'item__plural_1': 'saker' }); + + expect(result).not.toContain('fuzzy'); + }); + + test('does not clear fuzzy while dropping forms the header requires', () => { + const original = `msgid "" +msgstr "" +"Content-Type: text/plain; charset=UTF-8\\n" +"Plural-Forms: nplurals=4; plural=(n==1 ? 0 : n==2 ? 1 : n==3 ? 2 : 3);\\n" + +#, fuzzy +msgid "item" +msgid_plural "items" +msgstr[0] "guess one" +msgstr[1] "guess many" +`; + const result = surgicalUpdatePoFile(original, { + 'item': 'sak', + 'item__plural_1': 'saker', + 'item__plural_2': 'sakerna', + 'item__plural_3': 'sakerna4' + }); + + // Either every form the header requires is written, or the flag stays put. + // Clearing it while forms 2-3 are missing publishes an incomplete entry. + const wroteAllForms = /msgstr\[2\]/.test(result) && /msgstr\[3\]/.test(result); + const keptFuzzy = /^#,.*fuzzy/m.test(result); + expect(wroteAllForms || keptFuzzy).toBe(true); + }); + + test('clears fuzzy on a versioned entry keyed by its new msgid', () => { + const original = `msgid "" +msgstr "" +"Content-Type: text/plain; charset=UTF-8\\n" + +#, fuzzy +msgid "Old wording" +msgstr "Gammal gissning" +`; + const result = surgicalUpdatePoFile( + original, + { 'New wording': 'Ny text' }, + { keyMappings: { 'Old wording': 'New wording' } } + ); + + expect(result).toContain('msgid "New wording"'); + expect(result).toContain('msgstr "Ny text"'); + expect(result).not.toContain('fuzzy'); + }); + + test('does not clear fuzzy on a source-language entry it skips', () => { + const original = `msgid "" +msgstr "" +"Content-Type: text/plain; charset=UTF-8\\n" + +#, fuzzy +msgid "Hello" +msgstr "Old guess" +`; + // Source-language write: msgid === msgstr, so the entry is skipped and the + // stale guess stays. The flag must stay with it. + const result = surgicalUpdatePoFile( + original, + { 'Hello': 'Hello' }, + { sourceLanguage: 'en', targetLanguage: 'en' } + ); + + expect(result).toContain('#, fuzzy'); + }); + + test('does not clear fuzzy when the translation breaks msgid newline agreement', () => { + // gettext requires msgid and msgstr to agree on a leading newline. An LLM + // occasionally drops it; clearing the flag would leave an entry that looks + // done and makes the whole catalog fail to compile. + const original = `msgid "" +msgstr "" +"Content-Type: text/plain; charset=UTF-8\\n" + +#, fuzzy +msgid "" +"\\n" +" Archived this week\\n" +" " +msgstr "" +"\\n" +" Gammal gissning\\n" +" " +`; + const result = surgicalUpdatePoFile(original, { + '\n Archived this week\n ': 'Arkiverad denna vecka\n ' + }); + + expect(result).toContain('#, fuzzy'); + }); + + test('does not clear fuzzy for a whitespace-only translation', () => { + const original = `msgid "" +msgstr "" +"Content-Type: text/plain; charset=UTF-8\\n" + +#, fuzzy +msgid "Cancel" +msgstr "Avbryt" +`; + const result = surgicalUpdatePoFile(original, { 'Cancel': ' ' }); + + expect(result).toContain('#, fuzzy'); + }); + + test('keeps fuzzy on a plural when only some forms are returned', () => { + const original = `msgid "" +msgstr "" +"Content-Type: text/plain; charset=UTF-8\\n" +"Plural-Forms: nplurals=2; plural=(n != 1);\\n" + +#, fuzzy +msgid "item" +msgid_plural "items" +msgstr[0] "guess one" +msgstr[1] "guess many" +`; + const result = surgicalUpdatePoFile(original, { 'item': 'sak' }); + + expect(result).toContain('msgstr[0] "sak"'); + expect(result).toContain('#, fuzzy'); + }); + test('clears fuzzy on the owning entry when translated via a __plural_N key', () => { const original = `msgid "" msgstr "" @@ -91,7 +246,7 @@ msgid_plural "foos" msgstr[0] "guessed one" msgstr[1] "guessed many" `; - const result = surgicalUpdatePoFile(original, { 'foo__plural_1': 'saker' }); + const result = surgicalUpdatePoFile(original, { 'foo': 'sak', 'foo__plural_1': 'saker' }); expect(result).toContain('msgstr[1] "saker"'); expect(result).not.toContain('fuzzy'); @@ -109,7 +264,7 @@ msgid_plural "items" msgstr[0] "guessed one" msgstr[1] "guessed many" `; - const result = surgicalUpdatePoFile(original, { 'items': 'saker' }); + const result = surgicalUpdatePoFile(original, { 'item': 'sak', 'items': 'saker' }); expect(result).toContain('msgstr[1] "saker"'); expect(result).not.toContain('fuzzy'); diff --git a/tests/utils/po-utils.test.js b/tests/utils/po-utils.test.js index e700554..409e795 100644 --- a/tests/utils/po-utils.test.js +++ b/tests/utils/po-utils.test.js @@ -479,6 +479,48 @@ msgstr "Au revoir %(name)s" expect(result['Goodbye %(name)s'].metadata.po_flags).toEqual(['fuzzy', 'c-format']); }); + it('omits fuzzy entries from a target-language upload', () => { + const content = `msgid "" +msgstr "" +"Content-Type: text/plain; charset=UTF-8\\n" +"Plural-Forms: nplurals=2; plural=(n != 1);\\n" + +msgid "Save" +msgstr "Spara" + +#, fuzzy +msgid "Cancel" +msgstr "Guessed" + +#, fuzzy, python-format +msgid "%(n)d item" +msgid_plural "%(n)d items" +msgstr[0] "guess one" +msgstr[1] "guess many" +`; + const result = poEntriesToApiFormat(parsePoFile(content), { sourceLanguage: 'en', currentLanguage: 'sv' }); + + expect(result['Save'].value).toBe('Spara'); + expect(result['Cancel']).toBeUndefined(); + expect(result['%(n)d item']).toBeUndefined(); + expect(result['%(n)d item__plural_1']).toBeUndefined(); + }); + + it('keeps fuzzy entries when the file is the source language', () => { + const content = `msgid "" +msgstr "" +"Content-Type: text/plain; charset=UTF-8\\n" + +#, fuzzy +msgid "Cancel" +msgstr "" +`; + const result = poEntriesToApiFormat(parsePoFile(content), { sourceLanguage: 'en', currentLanguage: 'en' }); + + expect(result['Cancel'].value).toBe('Cancel'); + expect(result['Cancel'].metadata.po_flags).toEqual(['fuzzy']); + }); + it('should preserve fuzzy flag in plural forms', () => { const entries = [ { @@ -905,6 +947,83 @@ msgstr "" }); }); + // gettext marks an entry fuzzy when msgmerge guessed the translation from a + // similar msgid. msgfmt excludes fuzzy entries from the compiled .mo, so the + // app renders the source string — the key reads as translated in the file but + // is untranslated to the user. It must count as missing. + it('should treat a fuzzy target entry as missing even when msgstr is filled', () => { + const sourceContent = `msgid "" +msgstr "" + +msgid "Task overview" +msgstr "Task overview" +`; + + const targetContent = `msgid "" +msgstr "" + +#, fuzzy +msgid "Task overview" +msgstr "Uppgiftstavla" +`; + + const result = findMissingPoTranslations(sourceContent, targetContent); + + expect(result).toHaveLength(1); + expect(result[0].key).toBe('Task overview'); + expect(result[0].value).toBe('Task overview'); + expect(result[0].isPlural).toBe(false); + }); + + it('should not treat an ordinary filled target entry as missing', () => { + const sourceContent = `msgid "" +msgstr "" + +msgid "Task overview" +msgstr "Task overview" +`; + + const targetContent = `msgid "" +msgstr "" + +msgid "Task overview" +msgstr "Uppgiftstavla" +`; + + const result = findMissingPoTranslations(sourceContent, targetContent); + + expect(result).toHaveLength(0); + }); + + it('should treat every slot of a fuzzy plural entry as missing', () => { + const sourceContent = `msgid "" +msgstr "" + +msgid "%(count)d task archived" +msgid_plural "%(count)d tasks archived" +msgstr[0] "" +msgstr[1] "" +`; + + const targetContent = `msgid "" +msgstr "" +"Plural-Forms: nplurals=2; plural=(n != 1);\\n" + +#, fuzzy +msgid "%(count)d task archived" +msgid_plural "%(count)d tasks archived" +msgstr[0] "%(count)d uppgift i kolumnen" +msgstr[1] "%(count)d uppgifter i kolumnen" +`; + + const result = findMissingPoTranslations(sourceContent, targetContent); + + expect(result).toHaveLength(2); + expect(result.every(r => r.isPlural)).toBe(true); + expect(result[0].key).toBe('%(count)d task archived'); + expect(result[1].key).toBe('%(count)d task archived__plural_1'); + }); + it('should detect missing plural forms based on target language nplurals', () => { // English source (2 forms) -> Polish target (3 forms) const sourceContent = `msgid "" diff --git a/tests/utils/translation-utils.test.js b/tests/utils/translation-utils.test.js index f553b70..d9b2208 100644 --- a/tests/utils/translation-utils.test.js +++ b/tests/utils/translation-utils.test.js @@ -642,6 +642,92 @@ describe('translation-utils', () => { expect(batch.localeEntries).toContain('es:locales/en.json'); }); + it('does not group plural-looking keys in non-PO formats, and honours the batch cap', () => { + // A JSON project may legitimately have a key named foo__plural_1 that has + // nothing to do with gettext plurals. + const keys = {}; + for (let i = 0; i < 199; i++) keys[`k_${String(i).padStart(4,'0')}`] = `V ${i}`; + keys['foo'] = 'Foo'; + keys['foo__plural_1'] = 'Foo one'; + keys['foo__plural_2'] = 'Foo two'; + + const { batches, errors } = batchKeysWithMissing( + [{ path: 'locales/en.json', format: 'json' }], + { 'sv:locales/en.json': { locale: 'sv', path: 'locales/en.json', targetPath: 'locales/sv.json', keys } } + ); + expect(errors).toEqual([]); + + const sizes = batches.map(b => + Object.keys(JSON.parse(Buffer.from(b.sourceFile.content, 'base64').toString()).keys).length + ); + // JSON keys are chunked plainly: no group is held together past the cap. + expect(sizes[0]).toBe(200); + expect(Math.max(...sizes)).toBeLessThanOrEqual(200); + }); + + it('keeps plural forms together even when locales interleave them', () => { + // Two locales contribute the same plural family; merging can interleave the + // forms so they are no longer adjacent in insertion order. + const keysA = {}; + const keysB = {}; + for (let i = 0; i < 150; i++) keysA[`a_${String(i).padStart(4,'0')}`] = `A ${i}`; + keysA['item'] = 'item'; + for (let i = 0; i < 150; i++) keysB[`b_${String(i).padStart(4,'0')}`] = `B ${i}`; + keysB['item__plural_1'] = 'items'; + + const sourceFiles = [{ path: 'locales/en.po', format: 'po' }]; + const missingByLocale = { + 'sv:locales/en.po': { locale: 'sv', path: 'locales/en.po', targetPath: 'locales/sv.po', keys: keysA }, + 'nb:locales/en.po': { locale: 'nb', path: 'locales/en.po', targetPath: 'locales/nb.po', keys: keysB } + }; + + const { batches, errors } = batchKeysWithMissing(sourceFiles, missingByLocale); + expect(errors).toEqual([]); + + const batchOf = (key) => batches.findIndex(b => { + const content = JSON.parse(Buffer.from(b.sourceFile.content, 'base64').toString()); + return Object.prototype.hasOwnProperty.call(content.keys, key); + }); + + const base = batchOf('item'); + expect(base).toBeGreaterThanOrEqual(0); + expect(batchOf('item__plural_1')).toBe(base); + }); + + it('keeps plural forms of one key in the same batch', () => { + // batchKeysWithMissing chunks at a fixed 200; put the plural group across + // that boundary so a plain slice would separate its forms. + const keys = {}; + for (let i = 0; i < 199; i++) keys[`filler_${i}`] = `Filler ${i}`; + keys['item'] = 'item'; + keys['item__plural_1'] = 'items'; + keys['item__plural_2'] = 'items2'; + + const sourceFiles = [{ path: 'locales/en.po', format: 'po' }]; + const missingByLocale = { + 'sv:locales/en.po': { + locale: 'sv', + path: 'locales/en.po', + targetPath: 'locales/sv.po', + keys + } + }; + + const { batches, errors } = batchKeysWithMissing(sourceFiles, missingByLocale); + expect(errors).toEqual([]); + expect(batches.length).toBeGreaterThan(1); + + const batchOf = (key) => batches.findIndex(b => { + const content = JSON.parse(Buffer.from(b.sourceFile.content, 'base64').toString()); + return Object.prototype.hasOwnProperty.call(content.keys, key); + }); + + const base = batchOf('item'); + expect(base).toBeGreaterThanOrEqual(0); + expect(batchOf('item__plural_1')).toBe(base); + expect(batchOf('item__plural_2')).toBe(base); + }); + it('should handle missing source files', () => { const sourceFiles = [ { From 24627e98e434d145a46575e1529cc15c0bdb279b Mon Sep 17 00:00:00 2001 From: Arvid Andersson Date: Tue, 15 Sep 2026 19:20:13 +0200 Subject: [PATCH 2/2] Report a job that ran out of retries as a failure A job that exhausts its status-check budget warns and is dropped, but the language was never recorded as failed. The summary reads that empty list and prints "Translations complete" for a run that wrote nothing, and `ci` inherits it, so a GitHub Action step can pass having translated nothing. - Record the language on every exit from the retry-exhaustion branch: partial completion, never started, delayed status, and the outer catch - The existing summary and exit-code handling then work as written Co-Authored-By: Claude Opus 5 (1M context) --- src/utils/translation-processor.ts | 8 +++ tests/utils/translation-processor.test.js | 59 +++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/src/utils/translation-processor.ts b/src/utils/translation-processor.ts index 983fac1..8d836ae 100644 --- a/src/utils/translation-processor.ts +++ b/src/utils/translation-processor.ts @@ -485,6 +485,14 @@ async function processBatch( console.warn(chalk.yellow(` ❌ Job ${jobId} exceeded maximum retries (${MAX_JOB_STATUS_CHECK_ATTEMPTS}) and will be skipped.`)); } + // However it ran out of retries, the job has not delivered its + // translations. Record it, or the summary reports success for a run + // that wrote nothing. + const exhaustedLocale = jobSourceMapping[jobId]?.locale; + if (exhaustedLocale) { + stats.failedLanguages.add(exhaustedLocale); + } + pendingJobs.delete(jobId); return { jobId, status: 'failed' }; } diff --git a/tests/utils/translation-processor.test.js b/tests/utils/translation-processor.test.js index ec21799..f3befbe 100644 --- a/tests/utils/translation-processor.test.js +++ b/tests/utils/translation-processor.test.js @@ -890,6 +890,65 @@ describe('translation-processor', () => { }); }); + describe('retry exhaustion is reported as a failure (#651)', () => { + it('records the language as failed when a job never starts', async () => { + const testJobId = 'job-never-starts'; + const batches = [{ + sourceFilePath: 'locales/en.json', + sourceFile: { + path: 'locales/en.json', + format: 'json', + content: Buffer.from(JSON.stringify({ keys: { stuck: { value: 'Stuck' } } })).toString('base64') + }, + localeEntries: ['fr:locales/en.json'], + locales: ['fr'] + }]; + const missingByLocale = { + 'fr:locales/en.json': { + locale: 'fr', + path: 'locales/en.json', + targetPath: 'locales/fr.json', + keys: { stuck: { value: 'Stuck' } }, + keyCount: 1 + } + }; + + mockTranslationUtils.createTranslationJob.mockResolvedValue({ + jobs: [{ id: testJobId, language: { code: 'fr' } }] + }); + // The job is accepted but never progresses, so every poll returns pending + // until the retry budget is spent. + mockTranslationUtils.checkJobStatus.mockImplementation(async (jobId) => ( + jobId === testJobId + ? { status: 'pending', job_id: jobId, progress: { completed_keys: 0, total_keys: 1 } } + : { status: 'completed', translations: { data: {} }, language: { code: 'other' }, job_id: jobId } + )); + + jest.useFakeTimers(); + const originalSetTimeout = global.setTimeout; + global.setTimeout = jest.fn((callback) => { + if (typeof callback === 'function') callback(); + return 1; + }); + + try { + const result = await processTranslationBatches( + batches, + missingByLocale, + { projectId: 'test-project-651' }, + false, + { console: mockConsole, translationUtils: mockTranslationUtils } + ); + + expect(result.uniqueKeysTranslated.size).toBe(0); + expect(result.failedLanguages).toContain('fr'); + } finally { + global.setTimeout = originalSetTimeout; + jest.useRealTimers(); + } + }); + }); + describe('a source file split into several batches (#609)', () => { const config = { projectId: 'test-project' }; const sourcePath = 'locales/en.json';