fix(i18n): resolve plugin-pack plurals with the declared locale

[smwbev]

Summary

Prototype for #12106, option (1) — routing plural resolution through the locale a plugin pack declares. @nwparker asked for one and said (1) is fine "if perf isn't weird"; it is not, and the measurement below turned out to favour the change for a reason worth reading.

A plugin language pack registers its catalog under the synthetic resource language from pluginLanguageResourceId() — plugin followed by fixed-width hex — because i18next parses punctuation in language tags during resource lookup. That tag carries no locale, so Intl.PluralRules cannot be built from it and i18next falls back to a two-form English rule:

code	CLDR categories	1	2	5	21
ru-RU	one/few/many/other	one	few	many	one
plugin<hex>	one/other	one	other	other	other

For a Russian pack that renders 5 сессии where the language needs 5 сессий — few and many are different words, and no catalog content can express the difference while selection happens on the synthetic tag.

What changed

The pack already declares the locale its translations were written for. plugin-plural-locale.ts feeds that declared locale to the plural resolver, and only to the resolver — resource lookup keeps running on the synthetic tag exactly as before:

const base = resolver.getRule.bind(resolver)
resolver.getRule = (code, options) => base(declaredLocales.get(code) ?? code, options)

setRendererPluginLanguagePacks() refreshes the mapping whenever packs change and clears the rule cache, so a pack that changes its declared locale is followed rather than stuck on the previous one. Every count-based call site benefits at once; no call site changes.

Perf: it gets faster, and the reason is a bug in the fallback path

Same instance, same key, t() with count, 200,000 calls after a 2,000-call warmup:

	µs/call
before	15.249
after	3.580

That is not noise, and the change does no extra work — it removes work. PluralResolver.getRule caches its result at the end of the method, but the fallback for a code with no - or _ returns early:

if (!code.match(/-|_/)) return dummyRule;   // never reaches the cache write

A synthetic plugin<hex> tag has no separator, so it takes that branch on every call: a fresh new Intl.PluralRules() over a 90-character invalid tag, a thrown RangeError, caught, discarded. Measured per getRule:

code	µs	cached?
ru-RU	0.129	yes
en	0.143	yes
plugin<hex>	3.549	no

Mapping the code to a real locale before the lookup puts it back on the cached path. So the answer to "is perf weird" is that it was already weird, and this removes a throw-per-call from the hot path.

(Worth a separate upstream report to i18next, since any consumer with a non-BCP-47 resource language hits it. Happy to file that if you agree with the reading.)

Testing

• pnpm lint
• pnpm typecheck
• pnpm test
• pnpm build
• Added or updated high-quality tests that would catch regressions

plugin-plural-locale.test.ts covers four things: that Russian forms appear only after the pack declares its locale (asserting the wrong values first, so the test fails if the fix silently stops working); that resource lookup still resolves on the synthetic tag; that a pack with a missing or self-referential locale is ignored; and that a pack changing its declared locale is followed, including back to no mapping.

Verified against the real 11,742-string smwbev.russian catalog as well — with _few/_many present it renders 2 темы / 5 тем where it previously rendered the _other form for both.

Full suite on 2548b816c: 45,359 passed, 24 failures across 9 files. Eight of those files reproduce on a clean upstream/main checkout on this machine — omp-shell-wrapper.node-pty, ssh-posix-command-wrapper, both terminal-ime-*, automation-schedules, external-automation-jobs-file, headless-emulator-fidelity.fuzz, terminal-snapshot-color-parity. The ninth, transcript-watch, flaked once under full-suite load on a file-watcher timing assertion and passes four out of four in isolation on this branch.

AI Review Report

• Cross-platform: pure JS string mapping over a Map, no paths, no shell, no platform APIs, no Electron surface. Intl.PluralRules availability is unchanged — the code that consults it is untouched; only the tag handed to it differs.
• Scope check: the wrapper is installed once per resolver instance and is a no-op for every code that is not a registered plugin resource language, so built-in en/es/ja/ko/zh take the identical path they take today. That is asserted by the "keeps resource lookup on the synthetic tag" case.
• Flagged during development: the first benchmark compared a plugin-language call against an English one and reported a 4× speedup, which was an artifact of comparing two different lookups. Re-measured before/after on the same instance and key; the mechanism above is what the honest measurement exposed.
• Correctness of the reach-in: services.pluralResolver is public API surface on the instance, but the wrapper is deliberately narrow — it rewrites the argument, never the rule, so i18next keeps owning selection semantics.

Security Audit

• Input handling: the declared locale comes from the plugin manifest, which is already parsed and validated before registration; it is used only as an argument to Intl.PluralRules, which rejects malformed tags by throwing — the same behavior the synthetic tag hits today, and it degrades to the current two-form rule rather than failing the render.
• No new surface: no IPC, no filesystem, no network, no dependencies. Catalog parsing and the protected-copy rules are untouched.
• Cache safety: the rule cache is keyed by cleaned code; clearing it on pack change prevents a stale rule from a previously registered pack being reused.

Notes

This is option (1) from #12106 in isolation, so nothing regresses if it is rejected: with the mapping absent, behavior is exactly today's. Option (2) — moving the seven === 1 ? … : … call sites to count — only becomes worthwhile on top of this, and is intentionally not bundled here so the two can be judged separately.

[coderabbitai]

Important

Review skipped

We couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting @coderabbitai full review.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds locale canonicalization for plugin declarations before Intl validation. Adds plural-locale mapping for synthetic plugin resource languages and applies it after resource registration. Tests cover fallback pluralization, locale normalization, invalid locale rejection, synthetic resource lookup, locale changes, and mapping removal.

---

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the primary change: using declared plugin-pack locales for plural resolution.
Description check	✅ Passed	The description covers the change, testing, AI review, security audit, and notes, but omits the required Screenshots section.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/renderer/src/i18n/plugin-plural-locale.ts (1)

4-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Shorten the module comment.

Keep one concise comment that states why plural resolution uses the declared locale while resource lookup uses the synthetic language. The current block restates implementation details.

As per coding guidelines, comments must be concise, limited to non-obvious information, and preferably one line.

Source: Coding guidelines

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2f7335c8-82a8-4867-a51e-e310e85db824

📥 Commits

Reviewing files that changed from the base of the PR and between 2548b81 and b20e09c533df4e6ce883f5541aa7ecd327a3f263.

📒 Files selected for processing (3)

• src/renderer/src/i18n/i18n.ts
• src/renderer/src/i18n/plugin-plural-locale.test.ts
• src/renderer/src/i18n/plugin-plural-locale.ts

[smwbev]

Addressed in 188ca64bb.

Canonicalizing alone turned out not to be enough. Intl.getCanonicalLocales() does reject malformed tags such as ru_RU or not_a_locale, but a well-formed yet unknown tag (xx, tlh) passes it, and Intl.PluralRules then silently falls back to the host locale — on my machine new Intl.PluralRules('xx').resolvedOptions().locale is ru-RU. A pack with a typo would pluralize by whatever language the user's computer runs, which its author could never reproduce.

So the declared tag is now canonicalized (underscores folded first, so a ru_RU slip still lands on ru-RU) and kept only when Intl.PluralRules.supportedLocalesOf() reports real data for it. Anything unresolvable leaves the lookup empty, so the resolver stays on the synthetic plugin<hex> tag and the pack degrades to i18next's default instead of borrowing the host's rules.

Tests cover both directions: ru_RU and RU-ru resolve to ru-RU, while not_a_locale, xx and tlh register no mapping and leave rendering on the default forms. Module comment shortened as suggested.

[coderabbitai]

🧹 Nitpick comments (1)

src/renderer/src/i18n/pack-locale.ts (1)

1-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Condense the locale rationale comments.

Keep the non-obvious rationale in one short comment at each site.

• src/renderer/src/i18n/pack-locale.ts#L1-L14: replace the multi-paragraph block with one sentence that states canonicalization occurs before caller-specific Intl support checks.
• src/renderer/src/i18n/intl-locale.test.ts#L1-L9: replace the multi-paragraph block with one sentence that states ICU support is stubbed for deterministic tests.

As per coding guidelines, comments must be concise, limited to non-obvious information, and preferably one line.

Source: Coding guidelines

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7a07260b-b824-4f64-b421-ffa85a191c0f

📥 Commits

Reviewing files that changed from the base of the PR and between 188ca64bb09f32d36999540ebe004f3982d3f7c3 and 6e9e88961e1b204a0ca2e1e31c798acc5c7ace2f.

📒 Files selected for processing (5)

• src/renderer/src/i18n/i18n.ts
• src/renderer/src/i18n/intl-locale.test.ts
• src/renderer/src/i18n/pack-locale.ts
• src/renderer/src/i18n/plugin-plural-locale.test.ts
• src/renderer/src/i18n/plugin-plural-locale.ts

🚧 Files skipped from review as they are similar to previous changes (3)

• src/renderer/src/i18n/i18n.ts
• src/renderer/src/i18n/plugin-plural-locale.ts
• src/renderer/src/i18n/plugin-plural-locale.test.ts

[smwbev]

Scope note: while rebasing this branch onto current main I found that getIntlLocale() — which landed via #12514 — reads the same author-supplied field and trips over the same tags, so a pack declaring ru_RU silently formats its dates in English.

That fix and the shared canonicalizePackLocale() helper are now open separately as #12533, since the date bug is real regardless of whether this prototype is the approach you want. The top commit here is the same change; reviewing #12533 first leaves this PR as a two-file diff, and once it merges I'll rebase so the duplicate disappears.

[smwbev]

@nwparker — this and #12533 have no reviewer, and the reason looks like a gap worth fixing rather than something to ping around each time.

CODEOWNERS routes /src/renderer/src/i18n/locales/ and the localization scripts, but not /src/renderer/src/i18n/ itself. So a PR touching a catalog file gets auto-assigned, while a PR touching i18n.ts — the code that decides which catalog and which locale rules apply — gets nobody. That is the third time in a week: #12105 sat unassigned until it was raised manually, #12455 needed a direct request, and now these two.

If you agree the whole directory belongs to the same owner, one line does it:

/src/renderer/src/i18n/ @brennanb2025

Happy to send that as a one-line PR if you'd rather have it in the queue than in a comment.

On the two PRs themselves, both are green and the split is deliberate:

• fix(plugins): reject locale ids Intl cannot parse #12533 is the standalone bug: getIntlLocale() reads the pack's declared locale straight from the manifest, so ru_RU — an ordinary slip — makes Intl throw and the pack renders English dates with no log line. Three files, real regardless of what happens to the prototype.
• fix(i18n): resolve plugin-pack plurals with the declared locale #12505 is the i18n: languages with more than two plural forms cannot be translated #12106 prototype you said you were up for. It sits on top of fix(plugins): reject locale ids Intl cannot parse #12533; reviewing that one first leaves this as a two-file diff, and I'll rebase so the shared commit disappears.

No rush on either — mainly flagging that the routing gap is what's keeping them invisible, not the content.

[smwbev]

Heads-up on the red check here, since it is misleading: the PR Checks failure is an expiry, not a test failure.

Run 34408566433 was created 9 Sep 21:44 and concluded failure on 9 Oct 21:47 — exactly 30 days later, with zero jobs executed. It never ran; it sat awaiting the manual approval that fork PRs need, and GitHub expired it. The same thing has happened to every one of these branches, each at the 30-day mark:

PR	run created	expired	jobs run
#12505	9 Sep	9 Oct	0
#12533	8 Aug	7 Sep	0
#14121	13 Aug	12 Sep	0
#14127	17 Aug	16 Sep	0

So the red mark carries no information about the code — but it does make these PRs look broken to anyone scanning the queue, which seemed worth stating explicitly rather than leaving to inference.

What I can confirm locally for this branch at b2b17c73, run on 9 Sep after rebasing onto then-current main:

• vitest on intl-locale.test.ts + plugin-plural-locale.test.ts — 13 passed
• pnpm typecheck — exit 0
• pnpm lint — exit 0, including verify:localization-catalog, verify:localization-runtime-catalog, verify:localization-extraction and verify:localization-coverage

That rebase resolved one conflict: upstream replaced the eagerly bundled English catalog with en-runtime-required.json, so I took upstream's import and kept canonicalizePackLocale and applyPluginPluralLocales, both still used at their call sites. All four branches are MERGEABLE / mergeStateStatus: CLEAN as of today.

The one thing that would help: a single "Approve and run" on these four would let CI actually execute, and the checks would then say something real either way. If any of them is unwanted, closing it is just as useful to me — #13020 was closed as superseded by #13118 last month and that was the right call, no complaints from me.