Skip to content

feat(i18n): add per-instance language selection mechanism#1442

Open
thomasbeaudry wants to merge 1 commit into
mainfrom
feat/spanish-mechanism
Open

feat(i18n): add per-instance language selection mechanism#1442
thomasbeaudry wants to merge 1 commit into
mainfrom
feat/spanish-mechanism

Conversation

@thomasbeaudry

Copy link
Copy Markdown
Collaborator

Summary

  • Add es to the Language type in runtime-core and libui UserConfig.LanguageOptions
  • Add activeLanguages to SetupState schema, Prisma model, and API service
  • Replace the staged Save workflow in the admin settings page with autosave
  • Add a "Languages" card to admin settings with checkboxes to toggle active languages
  • Derive LanguageToggle options from activeLanguages in Navbar and Sidebar (hide toggle when only one language is active)
  • Gateway: detect language from ?lang= query param
  • Fix pre-existing type errors in branding/login/session code caused by the broader Language union

No translated strings are included — missing translations fall back to defaultLanguage (English).

Depends on DouglasNeuroInformatics/libui#107 for libui's own Spanish translations. The libui version bump should be done when that PR is merged and released.

Test plan

  • TypeScript type-check passes (0 new errors introduced; 5 pre-existing errors fixed)
  • Manual test: toggle languages in admin settings, verify navbar/sidebar reflect changes
  • Manual test: gateway respects ?lang=es param

🤖 Generated with Claude Code

Adds the mechanism for supporting additional UI languages (e.g. Spanish)
without shipping any translations. Deployments choose which languages are
active; the language toggle and instrument rendering derive from that set.

- Store activeLanguages in SetupState (default ['en', 'fr']); expose via the
  setup API and admin settings, which autosaves language changes.
- Augment libui's UserConfig.LanguageOptions with es in the web and gateway
  apps so the UI can resolve to Spanish; instrument content types are left at
  en|fr (partial translations are accepted), and sites that pass the resolved
  UI language into instrument APIs narrow it back to a supported language.
- Derive Navbar/Sidebar language toggle options from activeLanguages and hide
  the toggle when only one language is active.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@gdevenyi gdevenyi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review — per-instance language selection

The mechanism is the right shape: store the active set on SetupState, derive the toggle options from it, hide the toggle when only one language is active. Hiding rather than disabling the toggle is a nice touch, and disabling the last remaining checkbox is the correct guard.

Two things make this hard to accept as-is: the perimeter isn't validated, and the type widening was absorbed by casting at ~8 call sites rather than fixed once.

Blocking

  1. activeLanguages: z.array(z.string()) validates nothing. PATCH /v1/setup {"activeLanguages": ["klingon"]} is accepted and persisted. Object.entries(ALL_LANGUAGES).filter(([key]) => activeLanguages.includes(key)) then yields {}, Object.keys(...).length > 1 is false, and the language toggle disappears from the navbar and sidebar for every user on the instance, with no way to restore it through the UI. AGENTS.md is explicit here — strict data validation at the boundary, and fail loudly on an undeclared policy. This should be z.array($Language).min(1) with $Language = z.enum(['en', 'es', 'fr']) exported as the single source of truth.

  2. The casts move the problem instead of solving it. Widening Language to include es broke indexing into the { en, fr } objects in $BrandingConfig and GroupSettings, and the fix was as undefined | { [key: string]: string } at four spots in LoginBrandingPanel, one in login.tsx, one in StartSessionForm, plus resolvedLanguage === 'en' || resolvedLanguage === 'fr' ? resolvedLanguage : 'en' repeated five times. AGENTS.md: "no casting at call sites", "shape is never repeated", "concentrate the cost". The actual defect is that those schema fields are typed { en: string; fr: string } instead of being keyed by Language. Fixing them once in packages/schemas makes every one of these call sites type-check unmodified. (#1439 introduces a $LocalizedString that is close to what's needed — worth converging on one, in schemas/core.)

  3. No tests. AGENTS.md requires unit tests and e2e tests for new changes; this has neither, and both manual boxes in the test plan are unchecked. Minimum viable set: a $UpdateSetupStateData schema test for the accepted/rejected language arrays, a SetupService test that activeLanguages round-trips (the pattern is right there in #1441's setup.service.spec.ts), and an e2e that deactivates a language and asserts the toggle disappears.

Should fix

  1. A failed autosave still reports "All changes saved"onSettled runs on error. Same issue as #1441; see inline.

  2. Deactivating a language strands users who had it selected. i18n.changeLanguage is called only for the admin flipping the checkbox. Every other user whose persisted language was just removed keeps rendering in it, and the toggle no longer offers a way out. Needs a reconciliation on app load: if the resolved language isn't in activeLanguages, switch to the first active one.

  3. ['en', 'fr'] is hardcoded as the default in four places here (setup.service.ts, Navbar, Sidebar, settings.tsx) and five more in #1439. If SetupService guarantees a non-empty array — which it already tries to — no client needs a fallback at all.

  4. The gateway carries a fifth hardcoded language list and doesn't consult activeLanguages, so ?lang=es works there even when Spanish is deactivated instance-wide.

  5. ALL_LANGUAGES: { [key: string]: string } — a loose record over a closed key set, which AGENTS.md rules out and which is what forces the ?? code fallbacks in #1439's consumers.

  6. The libui bump isn't here. The PR notes this depends on libui#107 for libui's own Spanish strings. Until that lands and is released, selecting Español leaves every libui-owned string (form submit/reset buttons, search placeholder, date pickers, notification titles) in English. Should this merge before the bump, or wait?

Coordination with the other open PRs

Conflict Files
#1442 × #1441 apps/web/src/routes/_app/admin/settings.tsx (both replace the staged-Save block with incompatible autosave layouts — #1441 adds a Settings section and SettingSection wrappers, this adds a Languages card) and apps/web/src/components/SaveStatus.tsx (add/add)
#1442 × #1439 apps/web/src/components/SaveStatus.tsx (add/add); apps/web/src/utils/languages.ts, activeLanguages in schema.prisma / setup.service.ts / $SetupState are duplicated verbatim between the two

Where the two autosave implementations differ, #1441's is correct: it destructures mutate (referentially stable) and depends on [mutate], whereas this one depends on [updateSetupStateMutation], which React Query returns fresh every render.

});

const $SetupState = z.object({
activeLanguages: z.array(z.string()).optional(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

z.array(z.string()) accepts any string, and this is the perimeter — $UpdateSetupStateData is what UpdateSetupStateDto validates the PATCH body against.

PATCH /v1/setup {"activeLanguages": ["klingon"]} is stored as-is. Navbar/Sidebar then compute Object.entries(ALL_LANGUAGES).filter(([key]) => activeLanguages.includes(key)){}Object.keys(...).length > 1 is false → the language toggle vanishes for every user, and the admin settings page shows all three boxes unchecked with no way back through the UI. [] is likewise accepted here and only papered over by a ?.length check in the service.

AGENTS.md: "Strict data validation at the boundary, trusting inside" and "Correctness is structural, not vigilant".

const LANGUAGES = ['en', 'es', 'fr'] as const;
const $Language = z.enum(LANGUAGES);
const $ActiveLanguages = z.array($Language).min(1);

Exporting LANGUAGES/$Language also gives ALL_LANGUAGES, the gateway's VALID_LANGUAGES, and #1439's MAIL_LANGUAGE one thing to derive from instead of four independent copies.

(data: Parameters<typeof updateSetupStateMutation.mutate>[0]) => {
setSaveState('saving');
updateSetupStateMutation.mutate(data, {
onSettled: () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

onSettled runs on failure too, so a rejected save still shows the green check and "All changes saved". Because useUpdateSetupStateMutation doesn't disable the default error notification, the user gets a red error toast and a green "saved" pill simultaneously — and for the language checkboxes specifically, the box stays flipped even though nothing was stored.

onSuccess for the saved state, onError for a visible failure (and revert the optimistic checkbox).

Same issue in #1441, which rewrites this same block.

};
});
},
[updateSetupStateMutation]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

React Query returns a new mutation result object on every render — only mutate/mutateAsync are referentially stable. Depending on [updateSetupStateMutation] means this useCallback re-creates autosave every render, so the memo does nothing, and it becomes a real bug the moment autosave is used in an effect dependency array.

#1441 gets this right in the same file:

// `mutate` is referentially stable across renders, so callbacks that depend on `autosave` stay stable too.
const { mutate } = updateSetupStateMutation;
const autosave = useCallback((data) => { ... }, [mutate]);

Worth adopting that version wholesale when these two branches are reconciled.

: activeLanguages.filter((l) => l !== code);
if (updated.length > 0) {
autosave({ activeLanguages: updated });
if (!updated.includes(i18n.resolvedLanguage)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two problems with switching the language here.

It only moves the admin. Every other user whose persisted language was just deactivated keeps rendering in it — t() still resolves their strings, and the toggle no longer lists it, so they can't switch away either. There's no reconciliation anywhere on app load. Something like this in the _app route (or wherever setupStateQuery is first read) would close it:

if (!activeLanguages.includes(i18n.resolvedLanguage)) i18n.changeLanguage(activeLanguages[0]);

updated[0] is arbitrary. [...activeLanguages, code] appends, so the array's order reflects the sequence of toggles, not any canonical order. Deactivating English can drop the admin into Spanish or French depending on click history. Sorting against the canonical LANGUAGES tuple (or just preferring 'en' when present) makes it deterministic.

Minor: the if (updated.length > 0) guard silently does nothing when it fails. The disabled={isLastActive} prop already prevents that path, so the guard is either dead or masking a case where it isn't — worth deciding which.

// older $BrandingConfig will silently drop newer branding fields on read.
const branding = $BrandingConfig.nullable().safeParse(savedOptions?.branding ?? null);
return {
activeLanguages: savedOptions?.activeLanguages?.length ? savedOptions.activeLanguages : ['en', 'fr'],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the first of four copies of ['en', 'fr'] in this PR (also Navbar, Sidebar, settings.tsx) and #1439 adds five more. AGENTS.md: "One source of truth; everything derived."

Export DEFAULT_ACTIVE_LANGUAGES from schemas/setup and use it here. Then, since this method already guarantees a non-empty array, $SetupState.activeLanguages can drop .optional() and every client-side ?? ['en', 'fr'] disappears — the clients stop needing to know the default exists.

createdAt DateTime @default(now()) @db.Date
updatedAt DateTime @updatedAt @db.Date
id String @id @default(auto()) @map("_id") @db.ObjectId
activeLanguages String[]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A required scalar list added to a model with existing documents. #1441 adds its SetupState field as Int?; this one has neither ? (not permitted on lists) nor @default([]).

Prisma's MongoDB connector should return [] for a list field absent from a stored document, so this probably works — but "probably" is doing real work in a field that gates the language toggle for the whole instance. @default([]) costs nothing and removes the question for every instance created before this deploys.

Also worth noting initApp creates SetupState without this field, so the very first document an instance writes exercises exactly that path.

}
}

const VALID_LANGUAGES = ['en', 'es', 'fr'];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Third hardcoded language list in this PR (after ALL_LANGUAGES and LanguageOptions), and #1439 adds a fourth (MAIL_LANGUAGE). Import the tuple from schemas and derive.

Two behavioural notes:

  1. The gateway ignores activeLanguages. ?lang=es renders Spanish even on an instance where Spanish has been deactivated. Since the emailed assignment links in feat(mail): add outgoing email, group email templates, and assignment emails #1439 are built as ${assignment.url}?lang=${language}, that's reachable in practice.

  2. SSR/hydration. detectLanguage() reads window.location.search at module scope; the guard makes the server always resolve 'en' while the client can resolve 'es'. Worth confirming the first paint doesn't flash English or trip a hydration mismatch — a ?lang= link is precisely the case where the two disagree.

Minor: the two /* eslint-disable */ lines at the top disable those rules for the whole file rather than the declare module block that needs them.

setInstrument(
translateInstrument(
instrument,
resolvedLanguage === 'en' || resolvedLanguage === 'fr' ? resolvedLanguage : 'en'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

resolvedLanguage === 'en' || resolvedLanguage === 'fr' ? resolvedLanguage : 'en' appears five times in this PR — here, useInstrumentInfoQuery, group/manage.tsx, InteractiveContent, and useInterpretedInstrument. AGENTS.md: "Shape is never repeated."

One named helper says what it means and gives you a single place to change the policy:

/** Instruments are authored in en/fr only; other UI languages fall back to English. */
export const toInstrumentLanguage = (language: Language): 'en' | 'fr' =>
  language === 'fr' ? 'fr' : 'en';

The policy itself is also worth surfacing: a user who has selected Español gets instruments silently rendered in English with no indication that a translation is missing. That's defensible, but right now it's an implicit consequence of five scattered ternaries rather than a stated decision.

const instanceDetails = branding?.instanceDetails?.[lang]?.trim() || null;
/* eslint-disable @typescript-eslint/prefer-nullish-coalescing -- blank strings must fall back, so `||` not `??` */
const instanceName =
(branding?.instanceName as undefined | { [key: string]: string })?.[lang]?.trim() || DEFAULT_INSTANCE_NAME;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These casts (four in this file, plus login.tsx and StartSessionForm.tsx) are the symptom, not the fix. AGENTS.md: "no casting at call sites" and "Concentrate the cost: complex type machinery belongs in a small number of utilities."

The root cause is that $BrandingConfig types instanceName/instanceTagline/instanceDetails and resourceLinks[].label as literal { en, fr } objects, so they can't be indexed once Language widens. Keying them by Language in packages/schemas fixes all six sites without touching them:

const $LocalizedString = z.partialRecord($Language, z.string().nullish());

That also makes tl = (obj) => obj[lang] ?? obj.en ?? '' unnecessary and — importantly — makes adding a fourth language a compile-time task rather than a runtime fallback to ''. Today the cast means a missing language silently renders as an empty instance name.

Note #1439 adds a $LocalizedString in schemas/mail for the same reason; one shared definition in schemas/core would serve both PRs.


const handleChangeLanguageEvent = useCallback(
(event: CustomEvent<Language>) => {
(event: CustomEvent<string>) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Widening a react-core event contract from CustomEvent<Language> to CustomEvent<string> to work around a downstream type error loses the checking for every other consumer of the changeLanguage event — including instrument authors, since this is the interactive-task bridge.

The runtime guard on the next line already narrows to 'en' | 'fr', so the type can stay Language and the guard keeps doing its job. If the incompatibility is with document.addEventListener's global CustomEventMap (which is where the widening pressure comes from), that map is the thing to update — not this handler's signature.

While here: console.error on an unsupported language is invisible to the participant, who just sees the language not change. Given AGENTS.md's "fail loudly on an undeclared policy", a user-visible signal (or not offering the option at all) would be better.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants