Skip to content

[Maintainer] i18n: locale registry, formatting and first locales - #1148

Merged
Aymericr merged 24 commits into
mainfrom
cursor/i18n-registry-ae7f
Oct 27, 2026
Merged

Aymericr merged 24 commits into
mainfrom
cursor/i18n-registry-ae7f

Conversation

@Aymericr

@Aymericr Aymericr commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

What does this PR do?

Links #621 and delivers the first maintainable localization slice:

  • uses Lingui 6 core behind Pascal’s experimental registry facade, preserving owner namespaces, ordered host/plugin overrides (including English), locale fallback, dev-only missing-message warnings, and isolated request/editor scopes
  • retains client scopes only while their React provider is mounted, so SSR scopes are collectible and Strict Mode effect replay can safely detach and reattach them
  • uses plain i18n._({ id, message, values }) runtime descriptors in published packages—no Lingui macros, compiler plugin, or host build step
  • registers the compiled English catalog synchronously from the published editor entry so production interpolation works for package consumers; hosts opt into the other bundled packs
  • replaces the custom message/plural engine and JSON catalogs with ICU MessageFormat, PO source catalogs, and committed Lingui-compiled English, zh-CN, fr, es, and ja catalogs for the existing 85 messages
  • resolves standalone/iframe ?lang= on the server and hydrates with the same request-local locale; <Editor locale> is also initialized before first render and runtime switching remains supported without catalog registration resetting the active locale
  • keeps locale-aware number/length formatting and decimal input parsing separate from the unit system, including scoped decimal parsing in sliders, stair/metric controls, floor-plan dimensions/reference scales/measurement extrusion, and cabinet placement dimensions
  • converts the viewer controls/header/floor-plan/stage surfaces plus the Build panel Rooms section and Advanced heading
  • routes missing/stale translation fill through lazily loaded Vercel AI SDK structured output with configurable OpenAI/Anthropic providers; ICU, IDs, placeholders, attributed/self-closing tags, plural categories, and glossary terms are validated, multiline review notes are escaped, and every AI result is written fuzzy with Review: required
  • treats defaults extracted from code as the canonical English source: i18n:check reports drifted IDs without writing files, while i18n:extract overwrites only English, preserves target translations, and marks changed targets fuzzy/review-required; stale entries are omitted from compiled target packs so runtime falls back to current English until review
  • keeps i18n:check key-free, read-only, and strict for PO/extraction/compiled drift, completeness, stale source hashes, placeholders, plurals, glossary rules, and human-review status; source line shifts do not alter PO files
  • rejects prototype-mutating catalog keys and builds registry accumulators without object prototypes
  • includes [Bugs] Editor proxy: harden middleware matcher #1156’s self-host proxy matcher/exclusions unchanged, applies locale forwarding only after the auth gate, and checks the matcher against the real public/ and app/ top-level folders
  • runs the standalone i18n-tool and proxy regressions from the root CI test command
  • verifies scene serialization is identical across locales; exports, MCP/agent text, catalog content, and developer errors remain untranslated

The design follows the proposals from @luis-caf and @volkankurt2. #1022 can rebase onto this registry and contribute its broader zh-CN surface coverage.

Runtime dependency footprint

@pascal-app/core adds only two direct Lingui runtime dependencies, both pinned to 6.10.1:

  • @lingui/core is the message runtime used for catalog activation, fallback, ICU interpolation, and plural selection.
  • @lingui/message-utils is loaded only when a host calls asynchronous registerMessages with raw ICU strings, so those packs can be validated and compiled atomically. Bundled packs use precompiled messages and do not load it on the rendering path.

@lingui/react was removed: Pascal’s own context and useSyncExternalStore provider are sufficient for the published plain-runtime API. CLI, PO, AI SDK, provider, and Zod packages remain root development tooling; the AI modules load only for i18n:translate.

Proxy / security scope

This branch merges main at 5ec20e77 and preserves #1156’s matcher and static exclusion list exactly. Relative to main, apps/editor/proxy.ts only imports the locale helpers and replaces the successful-gate NextResponse.next() with a request-header copy that overwrites x-pascal-locale from canonicalized ?lang=. gateRequest(request) still runs first and any redirect/error returns immediately. This forwarding is required because Next App Router layouts cannot read searchParams; without it, the root layout cannot choose the locale during SSR and the first paint would disagree with hydration. The change does not add an auth bypass, alter open paths, or trust a client-supplied locale header.

The root test script executes apps/editor/proxy.test.ts, retaining #1156’s dotted-route and static-asset matcher cases alongside locale canonicalization. A filesystem-backed regression now requires every top-level public/ folder to be excluded and every non-shadowed top-level app/ folder to remain matched.

Deferred follow-ups

A follow-up issue should cover the intentionally deferred deeper placeholder/select validation, per-locale lazy entry points, inherited parent-catalog plural rules such as pt used by pt-PT, and client-side ?lang navigation without a reload. None expands this first slice beyond its current 85 messages.

How to test

  1. Run bun run i18n:check; all bundled PO catalogs and compiled modules should be synchronized and complete without an API key, and the command must leave them unchanged.
  2. Run bun test scripts/i18n.test.ts packages/core/src/i18n packages/editor/src/i18n packages/editor/src/components/ui/controls/slider-control.test.tsx packages/editor/src/components/ui/controls/metric-control.test.ts packages/nodes/src/cabinet/__tests__/dimension-input.test.ts apps/editor/proxy.test.ts.
  3. Run bun run check, bun run check-types, and bun run ci for the complete gate.
  4. Run bun dev, then compare the editor with no lang, ?lang=fr, and ?lang=ja. The selected locale should be present in the server-rendered first paint and remain unchanged through hydration and later pack registration.
  5. In French, edit slider, metric/stair, floor-plan, and cabinet fields with comma decimals; they should parse in the provider’s locale while English comma behavior stays unchanged. In particular, ambiguous 1,000 means 1 in French and 1000 in English.
  6. To exercise AI fill without credentials, run bun test scripts/i18n.test.ts; the provider path is mocked and validates that output remains flagged for human review.

Screenshots / screen recording

The screenshots are served from a dedicated non-merged asset branch so they are public to all contributors without adding binaries to main.

English (default)

Pascal editor in English

French

Pascal editor in French

Japanese

Pascal editor in Japanese

French decimal input verification

Pascal editor accepting French decimal input

Checklist

  • I've tested this locally with bun dev
  • My code follows the existing code style (run bun check to verify)
  • I've updated relevant documentation (if applicable)
  • This PR targets the main branch

Credits: @luis-caf for the inventory and extensibility contract, and @volkankurt2 for the agreed four-point host design.

Open in Web Open in Cursor 

Note

Medium Risk
Touches request middleware and SSR/hydration locale wiring plus broad editor input parsing; auth gating in proxy is unchanged but every allowed request now gets a canonicalized locale header.

Reviewed by Cursor Bugbot for commit 1dbbd10. Bugbot is set up for automated code reviews on this repo. Configure here.

Co-authored-by: Aymeric Rabot <Aymericr@users.noreply.github.com>
@Aymericr Aymericr added the bot:maintainer label Oct 26, 2026 — with Cursor
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-26T20:26:17.452575Z 6eb4972 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6eb497206d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1967 to +1968
useClientLayoutEffect(() => {
if (props.locale) setLocale(props.locale)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Trigger formatted readouts when the locale prop changes

When an already-mounted <Editor> receives a new locale, its descendants first render with the previous global locale and this layout effect changes only the registry afterward. Existing numeric readouts call formatNumber imperatively rather than subscribing through useTranslate—several are also memoized or cached—so area, volume, and length labels can remain in the previous locale until unrelated state changes. Apply the locale through a subscribed provider/snapshot, or otherwise ensure formatter consumers rerender after this mutation.

Useful? React with 👍 / 👎.

Comment thread packages/core/src/i18n/registry.ts Outdated
Comment on lines +35 to +36
const language = new Intl.Locale(exact).language
return language === exact ? [exact] : [exact, language]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Retain script subtags in the locale fallback chain

For locales containing both script and region subtags, such as zh-Hant-HK or sr-Latn-RS, this reduces the fallback directly to zh or sr. A registered zh-Hant/sr-Latn catalog is therefore skipped and the UI falls back to a generic language catalog or English. Build progressive parent fallbacks (zh-Hant-HK → zh-Hant → zh) rather than keeping only Intl.Locale.language.

Useful? React with 👍 / 👎.

: null
const bare = Number(text)
const stored = parsed ?? (text.trim() && Number.isFinite(bare) ? toStored(bare) : null)
const bare = parseLocalizedNumber(text)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve exponent notation in plain numeric controls

For a SliderControl without a lingo-recognized unit—such as the many percentage or unitless controls—input like 1e2 previously passed through Number(text) as 100, but parseLocalizedNumber rejects it because its regex accepts only decimal notation. This silently removes scientific-number input from those controls while dimension controls still accept it; extend the localized parser to accept a localized mantissa followed by an exponent.

Useful? React with 👍 / 👎.

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread apps/editor/app/client-bootstrap.tsx Outdated
Comment thread packages/core/src/i18n/format.ts
Comment thread packages/editor/src/components/viewer/floorplan-preview.tsx Outdated

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread packages/editor/src/components/ui/controls/slider-control.tsx Outdated

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread packages/editor/src/components/ui/controls/slider-control.tsx
Comment thread packages/editor/src/components/viewer/floorplan-preview.tsx Outdated

Copy link
Copy Markdown
Member Author

Tooling cross-review at c93ff63: CHANGES REQUESTED

Required CI is green on this head (quality, build, cli-smoke, Changed paths). Locally bun run i18n:check passes in about 12s and needs no API key. The CI step itself is fine: no new secrets or permissions, and translate never runs in CI.

P1

  • lingui.config.ts:22 lineNumbers: true makes the CI gate fail on unrelated edits. I added a single comment line to the top of viewer-stage-switcher.tsx, and i18n:check failed with all 5 PO files "not synchronized". Any line shift in a file that has a message would fail CI, and parallel PRs would conflict on PO references. Use lineNumbers: false (keep origins), or drop origins from the drift comparison.
  • packages/editor/src/components/ui/controls/slider-control.tsx:273 parseLocalizedNumber(text) falls back to getLocale(), which is defaultI18n (registry.ts:219). Standalone ?lang=fr and <Editor locale> use a scoped createI18n, and sync only runs default to scope (client-bootstrap.tsx:43). So 2,75 doesn't parse, which breaks test step 5. I confirmed Bugbot's High finding on this head. Pass the scoped locale through.
  • Bugbot still has 4 unresolved findings, 2 of them on this head. Resolve them or reply to each.

P2

  • scripts/i18n.ts:574-578 check runs extract, annotate and compile in place, so a local i18n:check or bun run ci rewrites catalogs. Restore the snapshot after comparing, or say in the README that the check writes files.
  • scripts/i18n.ts:5-8 imports ai, @ai-sdk/* and zod statically, so every CI check loads them. Lazy-import them inside configuredProvider().
  • @lingui/core is pinned 6.10.1 in packages/editor/package.json:34 but ^6.10.1 in packages/core/package.json:161, and the root devDeps mix both. Pick one.
  • Install footprint: @lingui/core and @lingui/react depend on @lingui/babel-plugin-lingui-macro, which brings @lingui/conf, jest-validate and jiti into the @pascal-app/core install tree. It's acceptable, but worth knowing.
  • AGENTS.md:32 still lists what bun run ci runs as "check, skills:validate, check-types, test, build". Add i18n:check and a one-line pointer to i18n/README.md. The README is otherwise lean and accurate.

Security scope: apps/editor/proxy.ts:21-26 changes the self-host auth gate. Gating still returns first, and headers.set overwrites any x-pascal-locale sent by the client, so it looks safe. Under the fleet rule, auth-touching changes still need Security's written approval naming the head SHA. I found no changes to payments, secrets, user data or schema.

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread apps/editor/app/client-bootstrap.tsx

@Aymericr Aymericr left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer (bot) review, Codex round: two local Codex passes on head 10415155 (gpt-6-astra, xhigh). Pass 1 found the issues and pass 2 re-checked each one against the branch code and the Lingui 6.10.1 and pofile-ts sources, dropping the false positives. Only confirmed findings are listed. The slider locale bug and the stock Lingui context bypass were already fixed by f91ad4c and 1041515.

P1, to fix before merge

  1. SSR scope leak, and Strict Mode detaches scopes. packages/core/src/i18n/registry.ts:105 adds every PascalI18n to the module-level scopes set, and the only cleanup is in effects (apps/editor/app/client-bootstrap.tsx:47, packages/editor/src/components/editor/index.tsx:1977-1978). Effects never run during SSR, so each server render keeps its scope and both Lingui instances alive, and every later registerCompiledMessages refreshes all of them. In Strict Mode, the cleanup replay calls destroy() on a scope that stays mounted, so packs registered later no longer refresh it. Fix: don't register globally in the constructor. Attach and detach through a reversible subscription, and let server scopes be garbage-collected.
  2. Registering any pack resets the chosen language. apps/editor/app/client-bootstrap.tsx:43-44 calls i18n.setLocale(defaultI18n.getLocale()) on every defaultI18n notification, and catalog refreshes fire it too. With ?lang=fr, defaultI18n stays en, so registering any host or plugin pack switches the page back to English (Bugbot reported the same thing). Also, setLocale('en') from French is a no-op because the default instance already reports en. Fix: give the app one locale owner, or separate locale-change events from catalog-refresh events.
  3. Package consumers lose English interpolation in production. Neither the @pascal-app/editor entry nor <Editor> (packages/editor/src/components/editor/index.tsx:1976) registers the compiled English catalog. Only the standalone app calls registerBundledLocales() (apps/editor/app/layout.tsx:11, client-bootstrap.tsx:16). Without a compiled message, production Lingui 6 returns the raw literal: a probe got Hello {name} instead of Hello Ada. That affects the existing labels that use {lead}, {mode} and {number}. Fix: register compiled English synchronously from the editor entry or <Editor>, and document the same requirement for host and plugin fallback messages.
  4. MetricControl still parses numbers with the global locale. packages/editor/src/components/ui/controls/metric-control.tsx:234 calls parseMeasurement without the scoped locale, so a French editor commits 1,000 as 1000 in the stair dimension fields. Fix: pass useLocale() through parsing, hints and the callback dependencies, as the slider fix did. The remaining floor-plan and cabinet parser callers need the same change.

P2, fine as follow-ups
5. packages/core/src/i18n/format.ts:80: an unsupported but well-formed locale (?lang=zz) makes Intl.NumberFormat use the runtime default, so server and client can disagree (2.75 vs 2,75). Pass [locale, 'en'] instead.
6. scripts/i18n.ts:460: model-written reviewNotes go into a PO comment, and pofile-ts only prefixes the first line, so a multiline note can inject a msgid/msgstr entry. The current check catches it (obsolete ID, missing review metadata), but every line should be prefixed and the serialized catalog validated before writing.
7. scripts/i18n.ts:148: placeholder validation accepts renumbered Lingui tags (<0>→<1>), broken nesting, removed selects and selectors without other. Compare markup and selector structure recursively.
8. packages/editor/src/i18n/locales.ts:2: all five catalogs are imported statically and registered together, with no per-locale entry points. Keep English synchronous and lazy-load the other languages.
9. packages/core/src/i18n/registry.ts:150: parent-locale messages are merged into the requested locale's Lingui instance, so a pt plural inherited by pt-PT uses pt-PT plural rules. Keep the locale of the catalog that supplied each message.
10. package.json:20: scripts/i18n.test.ts and apps/editor/proxy.test.ts aren't run by CI's test command. Add them.
11. apps/editor/app/layout.tsx:36: the locale is read only by the root layout, so a client-side navigation that changes ?lang= keeps the old language until a reload. Sync from useSearchParams() or document that changing language needs a full navigation.

Verified OK: the first SSR render for a supported ?lang= matches across the proxy header, <html lang> and the client provider. The pt-BR → pt → en fallback, script fallback, English overrides and namespacing work. No macros are used in published code, so hosts need no Babel/SWC plugin or lingui compile (apart from #3). All 85 IDs are explicit and match in all five catalogs. The translate script passes keys only to the SDK config, never to prompts, logs or catalogs, and marks AI output fuzzy with Review: required. One version of Lingui resolves in the lockfile.

Verdict: not mergeable as is. It's mergeable once the four P1s are fixed, and the P2s can go in follow-ups.

Copy link
Copy Markdown
Member Author

Tooling cross-review at 0293d90: CHANGES REQUESTED

The earlier findings are fixed on this head. I checked each one locally:

  • CI: quality, build, cli-smoke, Changed paths and Bugbot are all green at this SHA.
  • Line shifts: with lineNumbers: false, adding unrelated lines to viewer-stage-switcher.tsx and viewer-controls-bar.tsx passes i18n:check (about 12s).
  • Check-only: i18n:check writes nothing now (git status is clean). A new message ID fails it as it should.
  • Locale parse fix: slider, metric, measurement, floor-plan and cabinet parsing all take the scoped useLocale(). Scopes are released through retain(), and subscribeLocale stops pack registration from resetting the language.
  • P2s: the AI SDK and zod load lazily, and the AGENTS.md line is lean and accurate. The [Maintainer] i18n follow-ups from the #1148 review #1155 items are real P2s.
  • proxy.ts: the locale header is still set only after gateRequest returns null, and set overwrites any header the client sends. Still safe.

P1, still blocking: editing the English text of an existing message is silently ignored. I changed viewer-controls-bar.tsx:540 from message: 'Top view' to 'Top view X'. Both i18n:extract and i18n:check passed and left en/messages.po unchanged. Lingui keeps the existing source-locale msgstr, and the runtime prefers the compiled en catalog, so the copy change never ships. The other locales are never marked stale either, which defeats the Source-Hash check the README promises. Fix: have extract and check overwrite or compare the source-locale catalog against the code's message (for example with Lingui's --overwrite), and add a test that edits a message.

Once that's fixed I expect to approve. The merge will still need Security's written approval at the final SHA because of apps/editor/proxy.ts.

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread packages/editor/src/components/ui/controls/metric-control.tsx

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1dbbd10. Configure here.

Comment thread packages/editor/src/components/editor/floorplan-panel.tsx

Copy link
Copy Markdown
Member Author

Tooling cross-review at 1dbbd10: APPROVE

What I checked locally on this head:

  • CI: quality, build, cli-smoke and Changed paths are all green at this SHA, so the main merge ([Bugs] Editor proxy: harden middleware matcher #1156) didn't break CI or i18n. Bugbot is neutral.
  • Edited message: changing 'Top view' to 'Top view X' fails i18n:check, with source drift in en and the entry stale in all 4 other locales. i18n:extract updates the English catalog, marks the other locales fuzzy / Review: required, and leaves them out of their compiled packs. At runtime fr and ja show Top view X, while untouched entries stay translated.
  • Removed message: check fails as obsolete in all 5 locales. After extract the entry is gone from every PO and compiled file, and check passes.
  • Line shift: check still passes after adding unrelated lines.
  • Check-only: i18n:check writes no files. scripts/i18n.test.ts and proxy.test.ts pass (20/20).
  • proxy.ts: against main, the PR still only sets x-pascal-locale after gateRequest returns null. The matcher change comes from main.

No P0 or P1 problems in the new code.

P2 follow-ups:

  • Bugbot's two Medium findings: metric-control.tsx:250 and the floorplan-panel.tsx:2468 labels still show English decimals.
  • bun run test now runs the two script tests outside the turbo cache, about 66s locally.

Merging still needs Security's written approval at this SHA because of apps/editor/proxy.ts.

@volkankurt2

Copy link
Copy Markdown

Late host test, now that this has landed. I checked it against Forehome, a Next 16 app that embeds <Editor> and keeps its own Turkish/English UI with the locale in a cookie.

What I did

  • Merged the PR head onto our fork. The only conflicts were in our own apps/editor layout/bootstrap. The host passes its cookie locale to createI18n({ locale }) under PascalI18nProvider.
  • Registered a Turkish host pack covering all 85 messages through registerMessages('tr', 'core' | 'editor' | 'viewer', …). The IDs match the catalog exactly.
  • check-types and the production build pass. In the browser, the converted surfaces switch: Rooms and Walls show as Odalar and Duvarlar in tr and stay English in en. There are no console errors in either locale.
  • formatLength(3.1, 'm', { locale: 'tr' }) gives 3,1 m, and comma input parses. Turning grouping off in fields is right for Turkish, where . is the grouping separator.

One gap (still on main at 1dbbd10)

  • apps/editor/components/viewer-toolbar.tsx repeats the strings that viewer-controls-bar.tsx now translates, but it is not converted itself. It still has literal Display settings, Camera, Shadows, Render, Edges, Scene theme, Units, Grid, Walkthrough, Preview mode, and the wall modes Full height, Cutaway, Low and Translucent. scene-loader.tsx uses it, so the standalone editor page shows these in English for every bundled locale. Could it go into the first follow-up PR?

A note for hosts

  • registerMessages is async because it compiles lazily. A host pack registered at module load is not in place for the very first render, and the registry refresh brings it in afterwards. That worked for us because the editor mounts client-side. A line in i18n/README.md would help hosts that server-render a converted surface: either await the registration or use registerCompiledMessages.

I'll follow up with the tr catalog as its own PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants