Make --changed-only plural rescue format-aware

src/commands/pull.ts

@@ -4,6 +4,7 @@ import { findTranslationFiles } from '../utils/files.js';
 import { isGitAvailable, getChangedKeysForProject } from '../utils/git-changes.js';
 import { checkAuth as defaultCheckAuth } from '../utils/auth.js';
 import { PLURAL_SUFFIX_REGEX, extractBaseKeys } from '../utils/po-utils.js';
+import { splitPluralKey, yamlPluralGroupBasesFromKeys } from '../utils/translation-utils.js';
 import chalk from 'chalk';
 import type { Updates } from '../utils/sync-service.js';
 import type {
@@ -59,6 +60,10 @@ interface PullResult {
 function filterUpdatesByKeys(updates: Updates, changedKeys: Set<string>): Updates {
   // Extract base keys from plural forms in changedKeys (only relevant for .po files)
   const baseChangedKeys = extractBaseKeys(changedKeys);
+  // PLURAL_SUFFIX_REGEX is gettext-only, so a YAML/JSON group (`count.one` /
+  // `count.few`) needs the CLDR-category bases among the changed keys, or the
+  // categories the backend generated come back and get dropped here.
+  const yamlPluralBases = yamlPluralGroupBasesFromKeys(changedKeys);
 
   return {
     updates: {
@@ -75,7 +80,11 @@ function filterUpdatesByKeys(updates: Updates, changedKeys: Set<string>): Update
                 }
                 // Include if base key changed (for plurals)
                 const baseKey = t.key.replace(PLURAL_SUFFIX_REGEX, '');
-                return baseKey !== t.key && baseChangedKeys.has(baseKey);
+                if (baseKey !== t.key && baseChangedKeys.has(baseKey)) {
+                  return true;
+                }
+                const parsed = splitPluralKey(t.key);
+                return parsed !== null && yamlPluralBases.has(parsed.base);
               })
             }))
             .filter(lang => lang.translations.length > 0)

src/utils/git-changes.ts

@@ -4,6 +4,7 @@ import nodePath from 'path';
 import chalk from 'chalk';
 import type { TranslationFile, ProjectConfig, KeyIdentifier } from '../types/index.js';
 import type { MissingLocaleEntry } from './translation-utils.js';
+import { splitPluralKey, yamlPluralGroupBasesFromKeys } from './translation-utils.js';
 import { parseFile, flattenTranslations, extractLocaleFromPath } from './files.js';
 import { PLURAL_SUFFIX_REGEX, extractBaseKeys } from './po-utils.js';
 import { recordUnreadableFile } from './unreadable-files.js';
@@ -910,6 +911,23 @@ export function getRemovedKeysManifestForFinalize(
   return Object.fromEntries(perFile);
 }
 
+/**
+ * A missing key is a changed plural variant when its group had any category
+ * change in the same file. Two suffix conventions exist: gettext's
+ * `__plural_N` and the CLDR category YAML/JSON nests under a base.
+ */
+function isChangedPluralVariant(
+  key: string,
+  baseChangedKeys: Set<string>,
+  yamlPluralBases: Set<string>
+): boolean {
+  const gettextBase = key.replace(PLURAL_SUFFIX_REGEX, '');
+  if (gettextBase !== key && baseChangedKeys.has(gettextBase)) return true;
+
+  const parsed = splitPluralKey(key);
+  return parsed !== null && yamlPluralBases.has(parsed.base);
+}
+
 /**
  * Filter missing translations by changed keys, scoped per source file.
  *
@@ -933,15 +951,21 @@ function filterMissing(
   // Pre-compute the base-key set (plural-stripped) per file. Cheap, and
   // keeps the inner loop free of repeated regex work.
   const baseChangedKeysByFile = new Map<string, Set<string>>();
+  // The gettext regex above only matches `__plural_N`, so a YAML group
+  // (`count.one` / `count.few`) needs its own base set: the CLDR-category
+  // bases among the changed keys of that same file.
+  const yamlPluralBasesByFile = new Map<string, Set<string>>();
   for (const [filePath, keys] of changedKeysByFile.entries()) {
     baseChangedKeysByFile.set(filePath, extractBaseKeys(keys));
+    yamlPluralBasesByFile.set(filePath, yamlPluralGroupBasesFromKeys(keys));
   }
 
   for (const [localeKey, entry] of Object.entries(missingByLocale)) {
     const changedKeysForThisFile = changedKeysByFile.get(entry.path);
     if (!changedKeysForThisFile) continue;
 
     const baseChangedKeysForThisFile = baseChangedKeysByFile.get(entry.path) ?? new Set<string>();
+    const yamlPluralBasesForThisFile = yamlPluralBasesByFile.get(entry.path) ?? new Set<string>();
 
     const filteredKeys: Record<string, any> = {};
     let count = 0;
@@ -950,13 +974,9 @@ function filterMissing(
       if (changedKeysForThisFile.has(key)) {
         filteredKeys[key] = details;
         count++;
-      } else {
-        // Plural variant: include only if base key changed IN THIS FILE.
-        const baseKey = key.replace(PLURAL_SUFFIX_REGEX, '');
-        if (baseKey !== key && baseChangedKeysForThisFile.has(baseKey)) {
-          filteredKeys[key] = details;
-          count++;
-        }
+      } else if (isChangedPluralVariant(key, baseChangedKeysForThisFile, yamlPluralBasesForThisFile)) {
+        filteredKeys[key] = details;
+        count++;
       }
     }
 

src/utils/translation-utils.ts

@@ -107,7 +107,7 @@ function extractPrimitiveValue(value: unknown): TranslationPrimitiveValue {
  */
 const CLDR_CATEGORIES = ['zero', 'one', 'two', 'few', 'many', 'other'];
 
-function splitPluralKey(key: string): { base: string; category: string } | null {
+export function splitPluralKey(key: string): { base: string; category: string } | null {
   const lastDot = key.lastIndexOf('.');
   if (lastDot === -1) return null;
   const category = key.slice(lastDot + 1);
@@ -155,6 +155,12 @@ function yamlPluralGroupBases(sourceKeys: Record<string, any>): Set<string> {
   return bases;
 }
 
+// The same rule over a bare key set, for callers (--changed-only filtering)
+// that hold changed key names rather than a source map.
+export function yamlPluralGroupBasesFromKeys(keys: Iterable<string>): Set<string> {
+  return yamlPluralGroupBases(Object.fromEntries([...keys].map((key) => [key, true])));
+}
+
 // The same rule read from the parsed YAML tree, which unlike flattened keys
 // can tell a nested `foo: { one:, other: }` from literal `"foo.one"` keys.
 export function yamlPluralGroupBasesFromTree(node: unknown, prefix = ''): Set<string> {

tests/commands/pull.test.js

@@ -412,6 +412,115 @@ describe('pull command', () => {
       expect(translations.map(t => t.key)).toEqual(['item', 'item__plural_1']);
     });
 
+    it('should keep YAML plural categories the backend generated for the locale', async () => {
+      // The source group only has one/other; Polish also renders few/many.
+      // Those come back from the server and must survive the filter.
+      mockGitUtils.isGitAvailable.mockReturnValue(true);
+      mockGitUtils.getChangedKeysForProject.mockReturnValue(
+        new Set(['milestone_count.one', 'milestone_count.other'])
+      );
+
+      const mockUpdates = {
+        updates: {
+          files: [
+            {
+              path: 'config/locales/pl.yml',
+              languages: [{
+                code: 'pl',
+                translations: [
+                  { key: 'milestone_count.one', value: '1 kamien' },
+                  { key: 'milestone_count.few', value: '%{count} kamienie' },
+                  { key: 'milestone_count.many', value: '%{count} kamieni' },
+                  { key: 'milestone_count.other', value: '%{count} kamienia' },
+                  { key: 'other.key', value: 'inny' }
+                ]
+              }]
+            }
+          ],
+          deleted_keys: []
+        }
+      };
+
+      mockSyncService.checkForUpdates.mockResolvedValue({
+        hasUpdates: true,
+        updates: mockUpdates
+      });
+
+      mockSyncService.applyUpdates.mockResolvedValue({
+        totalUpdates: 4,
+        totalDeleted: 0
+      });
+
+      const deps = createPullDeps({
+        gitUtils: mockGitUtils,
+        configUtils: mockConfigUtils,
+        fileUtils: mockFileUtils
+      });
+
+      await pull({ changedOnly: true }, deps);
+
+      const actualUpdates = mockSyncService.applyUpdates.mock.calls[0][0];
+      const translations = actualUpdates.updates.files[0].languages[0].translations;
+
+      expect(translations.map(t => t.key)).toEqual([
+        'milestone_count.one',
+        'milestone_count.few',
+        'milestone_count.many',
+        'milestone_count.other'
+      ]);
+    });
+
+    it('should not pull a lone dotted key that only looks like a plural category', async () => {
+      mockGitUtils.isGitAvailable.mockReturnValue(true);
+      mockGitUtils.getChangedKeysForProject.mockReturnValue(
+        new Set(['greeting', 'errors.other'])
+      );
+
+      const mockUpdates = {
+        updates: {
+          files: [
+            {
+              path: 'config/locales/pl.yml',
+              languages: [{
+                code: 'pl',
+                translations: [
+                  { key: 'greeting', value: 'Czesc' },
+                  { key: 'errors.other', value: 'Blad' },
+                  { key: 'errors.detail', value: 'Szczegoly' }
+                ]
+              }]
+            }
+          ],
+          deleted_keys: []
+        }
+      };
+
+      mockSyncService.checkForUpdates.mockResolvedValue({
+        hasUpdates: true,
+        updates: mockUpdates
+      });
+
+      mockSyncService.applyUpdates.mockResolvedValue({
+        totalUpdates: 2,
+        totalDeleted: 0
+      });
+
+      const deps = createPullDeps({
+        gitUtils: mockGitUtils,
+        configUtils: mockConfigUtils,
+        fileUtils: mockFileUtils
+      });
+
+      await pull({ changedOnly: true }, deps);
+
+      const actualUpdates = mockSyncService.applyUpdates.mock.calls[0][0];
+      const translations = actualUpdates.updates.files[0].languages[0].translations;
+
+      // errors.other has no sibling category among the changed keys, so
+      // errors.detail must not ride along with it.
+      expect(translations.map(t => t.key)).toEqual(['greeting', 'errors.other']);
+    });
+
     it('should handle deleted keys filtering', async () => {
       mockGitUtils.isGitAvailable.mockReturnValue(true);
       mockGitUtils.getChangedKeysForProject.mockReturnValue(

tests/utils/changed-only-plural-synthesis.test.ts

@@ -0,0 +1,147 @@
+import { describe, it, expect, beforeEach, afterEach } from '@jest/globals';
+import { execSync } from 'child_process';
+import { mkdtempSync, writeFileSync, readFileSync, rmSync } from 'fs';
+import os from 'os';
+import path from 'path';
+import { filterByGitChanges } from '../../src/utils/git-changes.js';
+import { findMissingTranslationsByLocale } from '../../src/utils/translation-utils.js';
+import type { ProjectConfig, TranslationFile } from '../../src/types/index.js';
+
+/**
+ * Composes plural-category synthesis (#636) with --changed-only filtering.
+ *
+ * The two were correct in isolation but not together: synthesis produced the
+ * categories the locale needs and the filter threw them away, because its
+ * plural rescue only understood gettext's `__plural_N` suffix.
+ */
+describe('--changed-only over synthesised plural categories', () => {
+  let repo: string;
+  let originalCwd: string;
+
+  const config = {
+    schemaVersion: '1.0',
+    projectId: 'test',
+    sourceLocale: 'en',
+    outputLocales: ['pl'],
+    translationFiles: { paths: ['config/locales/'], baseBranch: 'HEAD' },
+    localePluralCategories: { pl: ['one', 'few', 'many', 'other'] },
+    lastSyncedAt: null
+  } as unknown as ProjectConfig;
+
+  const sourcePath = 'config/locales/en.yml';
+  const targetPath = 'config/locales/pl.yml';
+
+  function asTranslationFile(filePath: string, locale: string): TranslationFile {
+    return {
+      path: filePath,
+      format: 'yml',
+      locale,
+      content: readFileSync(filePath).toString('base64')
+    } as TranslationFile;
+  }
+
+  function commitBase(files: Record<string, string>) {
+    for (const [filePath, body] of Object.entries(files)) {
+      execSync(`mkdir -p ${path.dirname(filePath)}`);
+      writeFileSync(filePath, body);
+    }
+    execSync('git add .');
+    execSync('git -c user.email=t@t -c user.name=t -c commit.gpgsign=false commit -qm base');
+  }
+
+  function write(files: Record<string, string>) {
+    for (const [filePath, body] of Object.entries(files)) {
+      execSync(`mkdir -p ${path.dirname(filePath)}`);
+      writeFileSync(filePath, body);
+    }
+  }
+
+  function synthesiseThenFilter() {
+    const sourceFiles = [asTranslationFile(sourcePath, 'en')];
+    const { missing } = findMissingTranslationsByLocale(
+      sourceFiles,
+      { pl: [asTranslationFile(targetPath, 'pl')] },
+      config as any,
+      false,
+      { log: () => {} }
+    );
+
+    const beforeFilter = Object.keys(missing[`pl:${sourcePath}`]?.keys ?? {}).sort();
+    const filtered = filterByGitChanges(sourceFiles, missing, config, false);
+    const afterFilter = Object.keys(filtered?.[`pl:${sourcePath}`]?.keys ?? {}).sort();
+
+    return { beforeFilter, afterFilter };
+  }
+
+  beforeEach(() => {
+    originalCwd = process.cwd();
+    repo = mkdtempSync(path.join(os.tmpdir(), 'changed-only-plurals-'));
+    process.chdir(repo);
+    execSync('git init -q');
+  });
+
+  afterEach(() => {
+    process.chdir(originalCwd);
+    rmSync(repo, { recursive: true, force: true });
+  });
+
+  it('keeps every synthesised category when a plural group is added in the PR', () => {
+    commitBase({
+      [sourcePath]: 'en:\n  greeting: Hello\n',
+      [targetPath]: 'pl:\n  greeting: Czesc\n'
+    });
+
+    write({
+      [sourcePath]: 'en:\n  greeting: Hello\n  milestone_count:\n    one: 1 milestone\n    other: "%{count} milestones"\n',
+      [targetPath]: 'pl:\n  greeting: Czesc\n'
+    });
+
+    const { beforeFilter, afterFilter } = synthesiseThenFilter();
+
+    expect(beforeFilter).toEqual([
+      'milestone_count.few',
+      'milestone_count.many',
+      'milestone_count.one',
+      'milestone_count.other'
+    ]);
+    expect(afterFilter).toEqual(beforeFilter);
+  });
+
+  it('keeps few/many when the source text is reworded and one/other are already translated', () => {
+    commitBase({
+      [sourcePath]: 'en:\n  milestone_count:\n    one: 1 milestone\n    other: "%{count} milestones"\n',
+      [targetPath]: 'pl:\n  milestone_count:\n    one: 1 kamien\n    other: "%{count} kamieni"\n'
+    });
+
+    write({
+      [sourcePath]: 'en:\n  milestone_count:\n    one: 1 milestone reached\n    other: "%{count} milestones reached"\n'
+    });
+
+    const { beforeFilter, afterFilter } = synthesiseThenFilter();
+
+    // one/other are already present in pl.yml, so synthesis asks only for the
+    // categories Polish adds. Before the fix the filter dropped both and
+    // translate reported "No changed keys need translation".
+    expect(beforeFilter).toEqual(['milestone_count.few', 'milestone_count.many']);
+    expect(afterFilter).toEqual(['milestone_count.few', 'milestone_count.many']);
+  });
+
+  it('does not rescue a lone dotted key that only looks like a plural category', () => {
+    commitBase({
+      [sourcePath]: 'en:\n  greeting: Hello\n  errors:\n    other: Something went wrong\n    detail: Details\n',
+      [targetPath]: 'pl:\n  greeting: Czesc\n  errors:\n    other: Blad\n    detail: Szczegoly\n'
+    });
+
+    // greeting is reworded, and both it and errors.other are untranslated in pl.
+    write({
+      [sourcePath]: 'en:\n  greeting: Hi there\n  errors:\n    other: Something went wrong\n    detail: Details\n',
+      [targetPath]: 'pl:\n  errors:\n    detail: Szczegoly\n'
+    });
+
+    const { afterFilter } = synthesiseThenFilter();
+
+    // `errors.other` has no sibling CLDR category, so it is not a plural group:
+    // changing `greeting` must not pull it in.
+    expect(afterFilter).toEqual(['greeting']);
+  });
+});