diff --git a/.changeset/inline-default-value-drift-gate-3810.md b/.changeset/inline-default-value-drift-gate-3810.md new file mode 100644 index 0000000000..d6951b2f18 --- /dev/null +++ b/.changeset/inline-default-value-drift-gate-3810.md @@ -0,0 +1,39 @@ +--- +"@object-ui/app-shell": patch +"@object-ui/plugin-detail": patch +"@object-ui/plugin-list": patch +"@object-ui/console": patch +--- + +Align 43 inline `defaultValue` strings with the `en` pack, and make the call-site gate enforce it (objectui#3810) + +`t(key, { defaultValue: 'English text' })` only renders that text when i18next +**misses** the key. Where the key exists in `packages/i18n/src/locales/en.ts` the +pack value always wins, so the inline string is dead code — and 43 of those dead +strings said something different from the sentence users actually read. + +`scripts/check-i18n-call-site-keys.mjs` (objectui#3530) now compares the two +whenever a call site carries a literal `defaultValue` for a key `en` defines, and +fails on any byte of difference. It is a hard rule with **no baseline**: the +repo-wide census measured 43 sites in 19 files out of 851 literal inline defaults, +and all 43 are aligned here, so there is no debt for a ratchet to hold. A +`defaultValue` on a key that is *not* yet in `en` stays legal — that transition +runs for months (objectui#3546) and belongs to the existing `missing-key` rule, +which keeps reporting it alone. + +Every fix moved the CALL SITE to the pack's wording. `en.ts` is untouched: its +values are what users read today, and changing one would oblige the same change in +the nine other packs (`scripts/check-i18n-en-drift.mjs`, objectui#3650). Six of the +43 differed only in an ellipsis (`...` against U+2026) — invisible in review, which +is how they survived three i18n gates that are each blind to this class by +construction. + +The visible effect is confined to hosts that render these components with **no** +`I18nProvider` and no initialised i18next instance. There, react-i18next's +not-ready `t` returns the `defaultValue`, so the inline string was the rendered +one; it now matches what a provider-backed app has always shown. Inside the +console — provider mounted — nothing users see changes. The clearest converging +examples: the workspaces screen was written as "Organizations" at nine call sites +while every user has been reading "Workspaces"; the forgot-password success line +was written as "If an account exists, a reset link has been sent." while the pack +asserts "We've sent a password reset link to {{email}}." diff --git a/apps/console/src/pages/auth/ForgotPasswordPage.tsx b/apps/console/src/pages/auth/ForgotPasswordPage.tsx index 6a243c90d5..8ca4083d5a 100644 --- a/apps/console/src/pages/auth/ForgotPasswordPage.tsx +++ b/apps/console/src/pages/auth/ForgotPasswordPage.tsx @@ -31,20 +31,20 @@ export function ForgotPasswordPage() { linkComponent={RouterLink} title={t('auth.forgotPassword.title', { defaultValue: 'Reset your password' })} description={t('auth.forgotPassword.description', { - defaultValue: "Enter your email and we'll send you a reset link.", + defaultValue: "Enter your email address and we'll send you a link to reset your password", })} labels={{ emailLabel: t('auth.forgotPassword.emailLabel', { defaultValue: 'Email' }), emailPlaceholder: t('auth.forgotPassword.emailPlaceholder', { defaultValue: 'name@example.com' }), - submitButton: t('auth.forgotPassword.submitButton', { defaultValue: 'Send reset link' }), + submitButton: t('auth.forgotPassword.submitButton', { defaultValue: 'Send Reset Link' }), submittingButton: t('auth.forgotPassword.submittingButton', { - defaultValue: 'Sending…', + defaultValue: 'Sending...', }), successTitle: t('auth.forgotPassword.successTitle', { defaultValue: 'Check your email', }), successDescription: t('auth.forgotPassword.successDescription', { - defaultValue: 'If an account exists, a reset link has been sent.', + defaultValue: "We've sent a password reset link to {{email}}. Please check your inbox.", }), backToSignInText: t('auth.forgotPassword.backToSignInText', { defaultValue: 'Back to sign in', diff --git a/apps/console/src/pages/auth/LoginPage.tsx b/apps/console/src/pages/auth/LoginPage.tsx index 09d2f91c5b..f44d513d8b 100644 --- a/apps/console/src/pages/auth/LoginPage.tsx +++ b/apps/console/src/pages/auth/LoginPage.tsx @@ -366,7 +366,7 @@ function LoginFormCard({ linkComponent={RouterLink} errorMessages={{ INVALID_EMAIL_OR_PASSWORD: t('auth.login.errors.invalidCredentials', { - defaultValue: 'Invalid email or password', + defaultValue: 'Invalid email or password. Please try again.', }), EMAIL_NOT_VERIFIED: t('auth.login.errors.emailNotVerified', { defaultValue: 'Please verify your email address before signing in.', @@ -383,7 +383,7 @@ function LoginFormCard({ passwordPlaceholder: t('auth.login.passwordPlaceholder', { defaultValue: 'Enter your password' }), forgotPasswordText: t('auth.login.forgotPasswordText', { defaultValue: 'Forgot password?' }), submitButton: t('auth.login.submitButton', { defaultValue: 'Sign In' }), - submittingButton: t('auth.login.submittingButton', { defaultValue: 'Signing in…' }), + submittingButton: t('auth.login.submittingButton', { defaultValue: 'Signing in...' }), noAccountText: t('auth.login.noAccountText', { defaultValue: "Don't have an account?" }), signUpText: t('auth.login.signUpText', { defaultValue: 'Sign up' }), phoneLabel: t('auth.login.phoneLabel', { defaultValue: 'Phone number' }), diff --git a/apps/console/src/pages/auth/RegisterPage.tsx b/apps/console/src/pages/auth/RegisterPage.tsx index 4b1df69de1..3b9cf94f9d 100644 --- a/apps/console/src/pages/auth/RegisterPage.tsx +++ b/apps/console/src/pages/auth/RegisterPage.tsx @@ -151,9 +151,9 @@ export function RegisterPage() {

{t('auth.verifyEmail.resendFailed', { - defaultValue: 'Failed to resend verification email', + defaultValue: 'Cannot resend verification email', })}

{resendError}

diff --git a/packages/app-shell/src/console/home/HomeLayout.tsx b/packages/app-shell/src/console/home/HomeLayout.tsx index a9fa2e752f..ff8f4f71e2 100644 --- a/packages/app-shell/src/console/home/HomeLayout.tsx +++ b/packages/app-shell/src/console/home/HomeLayout.tsx @@ -62,7 +62,7 @@ export function HomeLayout({ children }: HomeLayoutProps) { chat (the dock maximized; Home has no shell to dock a rail into). */} {showChatbot && ( )} diff --git a/packages/app-shell/src/console/home/HomePage.tsx b/packages/app-shell/src/console/home/HomePage.tsx index a73a97aed0..c998302a01 100644 --- a/packages/app-shell/src/console/home/HomePage.tsx +++ b/packages/app-shell/src/console/home/HomePage.tsx @@ -305,7 +305,7 @@ export function HomePage() { if (loading) { return (
-
{t('home.loading', { defaultValue: 'Loading workspace…' })}
+
{t('home.loading', { defaultValue: 'Loading workspace...' })}
); } @@ -327,12 +327,12 @@ export function HomePage() { */} {isAdmin ? ( - {t('home.welcome', { product: getRuntimeConfig().branding.productName, defaultValue: 'Welcome to {{product}}' })} + {t('home.welcome', { product: getRuntimeConfig().branding.productName, defaultValue: 'Build your business system with AI' })} {buildAvailable ? t('home.welcomeAdminDescription', { defaultValue: - 'Describe your business in one sentence — AI generates the objects, screens, APIs and agent tools. Or set things up yourself from the Administration menu on the left.', + 'Describe your business in one sentence — AI generates the objects, screens, APIs and agent tools. Or set things up yourself from the menu on the left.', }) : askAvailable ? t('home.welcomeAdminDescriptionNoBuild', { @@ -358,7 +358,7 @@ export function HomePage() { {t('home.noAppsDescription', { defaultValue: - 'There are no applications available to you yet. Please contact your workspace administrator.', + 'Your workspace is being set up — apps your admin shares with you will show up here.', })} diff --git a/packages/app-shell/src/console/home/HomeRail.tsx b/packages/app-shell/src/console/home/HomeRail.tsx index a47bdf4dce..79d93eea20 100644 --- a/packages/app-shell/src/console/home/HomeRail.tsx +++ b/packages/app-shell/src/console/home/HomeRail.tsx @@ -188,7 +188,7 @@ export function HomeContinue({ items, onOpen, t }: { items: RecentItem[]; onOpen export function HomeActivity({ items, onViewAll, t }: { items: ActivityItem[]; onViewAll: () => void; t: TFn }) { const { language } = useObjectTranslation(); return ( - + {items.length === 0 ? (

{t('layout.activityFeed.empty', { defaultValue: 'No recent activity' })} diff --git a/packages/app-shell/src/console/organizations/OrganizationsPage.tsx b/packages/app-shell/src/console/organizations/OrganizationsPage.tsx index f848754e45..2b76b2ab02 100644 --- a/packages/app-shell/src/console/organizations/OrganizationsPage.tsx +++ b/packages/app-shell/src/console/organizations/OrganizationsPage.tsx @@ -175,11 +175,11 @@ export function OrganizationsPage() {

- {t('organizations.heading', { defaultValue: 'Your Organizations' })} + {t('organizations.heading', { defaultValue: 'Your Workspaces' })}

{t('organizations.subtitle', { - defaultValue: 'Select an organization to continue, or create a new one.', + defaultValue: 'Select a workspace to continue, or create a new one.', })}

@@ -191,7 +191,7 @@ export function OrganizationsPage() { value={query} onChange={(e) => setQuery(e.target.value)} placeholder={t('organizations.searchPlaceholder', { - defaultValue: 'Search for an organization', + defaultValue: 'Search for a workspace', })} className="pl-9" data-testid="organizations-search" @@ -200,7 +200,7 @@ export function OrganizationsPage() { {canCreateOrg && ( )}
@@ -208,24 +208,24 @@ export function OrganizationsPage() { {orgList.length === 0 ? ( - {t('organizations.emptyTitle', { defaultValue: 'No organizations yet' })} + {t('organizations.emptyTitle', { defaultValue: 'No workspaces yet' })} {t('organizations.emptyDescription', { - defaultValue: 'Create your first organization to get started.', + defaultValue: 'Create your first workspace to get started.', })} {canCreateOrg && ( )} ) : filtered.length === 0 ? (
{t('organizations.noMatches', { - defaultValue: 'No organizations match your search.', + defaultValue: 'No workspaces match your search.', })}
) : ( @@ -251,7 +251,7 @@ export function OrganizationsPage() {
{org.name}
{isActive - ? t('organizations.current', { defaultValue: 'Current organization' }) + ? t('organizations.current', { defaultValue: 'Current workspace' }) : org.slug}
diff --git a/packages/app-shell/src/layout/AppHeader.tsx b/packages/app-shell/src/layout/AppHeader.tsx index 594722766b..82fb3cd7ad 100644 --- a/packages/app-shell/src/layout/AppHeader.tsx +++ b/packages/app-shell/src/layout/AppHeader.tsx @@ -749,7 +749,7 @@ export function AppHeader({ <> - {t('organizations.title', { defaultValue: 'Organizations' })} + {t('organizations.title', { defaultValue: 'Workspaces' })} )} diff --git a/packages/app-shell/src/layout/AppSidebar.tsx b/packages/app-shell/src/layout/AppSidebar.tsx index f7046d0455..02937540d6 100644 --- a/packages/app-shell/src/layout/AppSidebar.tsx +++ b/packages/app-shell/src/layout/AppSidebar.tsx @@ -644,7 +644,7 @@ export function AppSidebar({ activeAppName, onAppChange }: { activeAppName: stri - {t('layout.appSwitcher.systemConsole', { defaultValue: 'System' })} + {t('layout.appSwitcher.systemConsole', { defaultValue: 'System Console' })} diff --git a/packages/app-shell/src/layout/ChatDock.tsx b/packages/app-shell/src/layout/ChatDock.tsx index f76f38adce..95e3034f25 100644 --- a/packages/app-shell/src/layout/ChatDock.tsx +++ b/packages/app-shell/src/layout/ChatDock.tsx @@ -560,7 +560,7 @@ export function ChatDockLauncher({ onExpand, className }: ChatDockLauncherProps) onClick={onExpand} data-testid="chat-dock-launcher" aria-label={t('console.ai.dock.open', { defaultValue: 'Open assistant' })} - title={t('console.ai.dock.open', { defaultValue: 'Open assistant (⌘⇧I)' })} + title={t('console.ai.dock.open', { defaultValue: 'Open assistant' })} className={cn( 'fixed right-0 top-1/2 z-40 hidden h-16 w-7 -translate-y-1/2 rounded-l-md rounded-r-none border-r-0 bg-background shadow-md md:inline-flex', className, diff --git a/packages/app-shell/src/layout/InboxPopover.tsx b/packages/app-shell/src/layout/InboxPopover.tsx index 0ef8d0f9a1..705008eb99 100644 --- a/packages/app-shell/src/layout/InboxPopover.tsx +++ b/packages/app-shell/src/layout/InboxPopover.tsx @@ -312,7 +312,7 @@ export function InboxPopover({ - {t('sidebar.activityFeed', { defaultValue: 'Activity' })} + {t('sidebar.activityFeed', { defaultValue: 'Activity feed' })} diff --git a/packages/app-shell/src/views/InterfaceListPage.tsx b/packages/app-shell/src/views/InterfaceListPage.tsx index d9210e071f..a4e819aa4f 100644 --- a/packages/app-shell/src/views/InterfaceListPage.tsx +++ b/packages/app-shell/src/views/InterfaceListPage.tsx @@ -433,7 +433,7 @@ export function InterfaceListPage({ page, className, onConfigChange, reserveEdit
- {t('empty.objectNotFound', { defaultValue: 'Source object not found' })} + {t('empty.objectNotFound', { defaultValue: 'Object Not Found' })} {t('empty.interfacePageSourceMissing', { defaultValue: 'This interface page references "{{name}}", which is not available.', diff --git a/packages/app-shell/src/views/RecordDetailView.tsx b/packages/app-shell/src/views/RecordDetailView.tsx index 35609bf272..fa4a2ff530 100644 --- a/packages/app-shell/src/views/RecordDetailView.tsx +++ b/packages/app-shell/src/views/RecordDetailView.tsx @@ -1793,7 +1793,7 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri history: { entries: historyEntries ?? [], loading: historyLoading && historyEntries === null, - unknownUserText: t('detail.unknownUser', { defaultValue: 'Unknown user' }), + unknownUserText: t('detail.unknownUser', { defaultValue: 'Unknown' }), }, }), // Approvals tab (#3461) — only when the record actually has requests, @@ -1958,7 +1958,7 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri // because the page is not focused or running over http://). try { await navigator.clipboard.writeText(window.location.href); - toast.success(t('detail.linkCopied', { defaultValue: 'Link copied' })); + toast.success(t('detail.linkCopied', { defaultValue: 'Link copied to clipboard' })); } catch (err: any) { toast.error( t('detail.linkCopyFailed', { defaultValue: 'Failed to copy link' }) + diff --git a/packages/app-shell/src/views/ReportConfigPanel.tsx b/packages/app-shell/src/views/ReportConfigPanel.tsx index bacdc973fc..e655519620 100644 --- a/packages/app-shell/src/views/ReportConfigPanel.tsx +++ b/packages/app-shell/src/views/ReportConfigPanel.tsx @@ -140,11 +140,11 @@ export function ReportConfigPanel({ className="hidden sm:flex w-[440px] shrink-0 flex-col border-l bg-background h-full" data-testid="report-config-panel" role="complementary" - aria-label={t('report.editor.title', { defaultValue: 'Edit report' })} + aria-label={t('report.editor.title', { defaultValue: 'Title' })} >
- {t('report.editor.title', { defaultValue: 'Edit report' })} + {t('report.editor.title', { defaultValue: 'Title' })}
diff --git a/packages/plugin-list/src/components/ViewSettingsPopover.tsx b/packages/plugin-list/src/components/ViewSettingsPopover.tsx index a73a20994a..8ae5879359 100644 --- a/packages/plugin-list/src/components/ViewSettingsPopover.tsx +++ b/packages/plugin-list/src/components/ViewSettingsPopover.tsx @@ -213,7 +213,7 @@ export function ViewSettingsPopover(props: ViewSettingsPopoverProps) { {showColor && setRowColorConfig && (
setRowColorConfig(undefined) : undefined} clearLabel={t('list.clear', { defaultValue: 'Clear' })} defaultOpen={!!rowColorConfig} @@ -299,7 +299,7 @@ export function ViewSettingsPopover(props: ViewSettingsPopoverProps) { {showHideFields && hiddenFields && updateHiddenFields && (
0 ? () => updateHiddenFields(new Set()) : undefined} clearLabel={t('list.showAll', { defaultValue: 'Show all' })} diff --git a/scripts/__tests__/check-i18n-call-site-keys.test.ts b/scripts/__tests__/check-i18n-call-site-keys.test.ts index 74a7ca893e..0a89c44459 100644 --- a/scripts/__tests__/check-i18n-call-site-keys.test.ts +++ b/scripts/__tests__/check-i18n-call-site-keys.test.ts @@ -36,6 +36,14 @@ import { * calls reach i18next, 1074 reach a module-local `engine.*` table, 41 are * not translators at all. The synthetic-repo tests pin each classification * independently of what today's `main` happens to contain. + * + * 3. objectui#3810 added a third: the gate now reads `en` VALUES, and a call + * site's own inline `defaultValue` must repeat the value byte for byte + * whenever the key exists. Both halves are pinned — the extractor against + * the evaluated module (values, not only keys), and the rule's verdicts + * against synthetic repos, including the cases it deliberately declines to + * judge. That last group is the one worth reading before widening the rule: + * each abstention is a decision, not an oversight. */ const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); @@ -78,6 +86,15 @@ function leafPaths(node: unknown, prefix = ''): string[] { : [prefix]; } +/** The same walk, carrying each leaf's STRING — what the drift rule compares. */ +function leafEntries(node: unknown, prefix = ''): Array<[string, string]> { + return node !== null && typeof node === 'object' + ? Object.entries(node as Record).flatMap(([k, v]) => + leafEntries(v, prefix ? `${prefix}.${k}` : k), + ) + : [[prefix, String(node)]]; +} + /** Materialises `{ 'packages/x/src/a.tsx': '…' }` into a throwaway repo root. */ function repoWith(files: Record): string { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'check-i18n-keys-')); @@ -92,9 +109,10 @@ function repoWith(files: Record): string { /** A minimal `en` pack for the synthetic repos below. */ const EN_FIXTURE = `const en = { - common: { save: 'Save', cancel: 'Cancel' }, + common: { save: 'Save', cancel: 'Cancel', loading: 'Loading...' }, detail: { showEmptyRelated_one: '+ {{count}} empty', showEmptyRelated_other: '+ {{count}} empty' }, grid: { column: { label: 'Label', width: 'Width' } }, + confirm: { purge: 'Deleting resets it to the shipped baseline. ' + 'Continue?' }, } as const; export default en; `; @@ -388,6 +406,139 @@ export function describe(t: TranslateFn): string { return t('legacy.helper'); } }); }); +describe('an inline defaultValue on a key that EXISTS must match the en value (objectui#3810)', () => { + /** Findings of `reason`, rendered as `key: expected -> actual`. */ + function driftOf(root: string): string[] { + return analyze(root) + .findings.filter((f: { reason: string }) => f.reason === 'default-value-drift') + .map((f: { detail: string; expected: string; actual: string }) => `${f.detail}: ${f.expected} -> ${f.actual}`) + .sort(); + } + + it('is silent when the call site copies the pack value byte for byte', () => { + const root = repoWith({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = () => { const { t } = useObjectTranslation(); return t('common.save', { defaultValue: 'Save' }); }; +`, + }); + const { findings, counters } = analyze(root); + expect(findings).toEqual([]); + expect(counters.matchingDefaultValues).toBe(1); + }); + + it('reports the dead string that says something else, with both texts', () => { + const root = repoWith({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = () => { const { t } = useObjectTranslation(); return t('common.save', { defaultValue: 'Save changes' }); }; +`, + }); + expect(driftOf(root)).toEqual(['common.save: Save -> Save changes']); + }); + + it('catches a difference of one character — the ellipsis families are the whole reason', () => { + // `Loading...` (three periods) against `Loading` + U+2026 renders the same to + // a reader skimming a diff, which is how six of the 43 sites survived. Both + // sides are written as escapes here for the same reason. + const root = repoWith({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = () => { const { t } = useObjectTranslation(); return t('common.loading', { defaultValue: 'Loading\\u2026' }); }; +`, + }); + expect(driftOf(root)).toEqual(['common.loading: Loading... -> Loading\u2026']); + }); + + it('folds a concatenated en value before comparing, so a wrapped sentence is judged', () => { + const both = 'Deleting resets it to the shipped baseline. Continue?'; + const root = repoWith({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = () => { + const { t } = useObjectTranslation(); + return [t('confirm.purge', { defaultValue: '${both}' }), t('confirm.purge', { defaultValue: 'Continue?' })]; +}; +`, + }); + // The matching one is silent; only the half-sentence is reported. + expect(driftOf(root)).toEqual([`confirm.purge: ${both} -> Continue?`]); + }); + + it('leaves a key en does not define to the missing-key rule, and reports it ONCE', () => { + // The two classes must stay disjoint: a missing key with an inline default is + // objectui#3517's shape, and saying "and it drifts" about a key with no value + // would be both noise and false. + const root = repoWith({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = () => { const { t } = useObjectTranslation(); return t('common.reset', { defaultValue: 'Reset' }); }; +`, + }); + expect(analyze(root).findings.map((f: { reason: string }) => f.reason)).toEqual(['missing-key']); + }); + + it('counts rather than judges a computed default — there is no text to compare', () => { + const root = repoWith({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = (label: string) => { + const { t } = useObjectTranslation(); + return [t('common.save', { defaultValue: label }), t('common.cancel', { defaultValue: \`Go \${label}\` })]; +}; +`, + }); + const { findings, counters } = analyze(root); + expect(findings).toEqual([]); + expect(counters.computedDefaultValues).toBe(2); + expect(counters.literalDefaultValues).toBe(0); + }); + + it('counts rather than judges a plural family and a several-literal key', () => { + // `detail.showEmptyRelated` resolves through `_one`/`_other`: there is no one + // form to compare against, and picking one would be an invention. + const root = repoWith({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = (flag: boolean) => { + const { t } = useObjectTranslation(); + return [ + t('detail.showEmptyRelated', { count: 2, defaultValue: 'anything' }), + t(flag ? 'common.save' : 'common.cancel', { defaultValue: 'anything' }), + t('grid.column', { returnObjects: true, defaultValue: 'anything' }), + ]; +}; +`, + }); + const { findings, counters } = analyze(root); + expect(findings).toEqual([]); + expect(counters.unjudgedDefaultValues).toBe(3); + }); + + it('reads the same values the module loader evaluates, not just the same keys', () => { + // The key half of this pin has existed since objectui#3530; the value half + // is what the drift rule rests on. A parser that folded a concatenation + // wrongly, or dropped an escape, would accuse correct call sites. + const parsed = collectEnKeys(repoRoot); + const runtime = new Map(leafEntries(realEn)); + const disagreeing = [...runtime] + .filter(([key, value]) => parsed.values.has(key) && parsed.values.get(key) !== value) + .map(([key]) => key); + const unread = [...runtime.keys()].filter((key) => !parsed.values.has(key)); + expect(disagreeing, `${disagreeing.length} key(s) parsed to a different string`).toEqual([]); + expect(unread, 'every leaf in en today is a static string, so none should be unreadable').toEqual([]); + expect(parsed.values.size).toBeGreaterThan(2000); + }); + + it('main carries no drift, which is why this rule has no baseline', () => { + // objectui#3810 measured 43 sites in 19 files and aligned all of them in the + // same PR. A finding here is a NEW divergence: fix the call site, not the + // pack. If this ever has to be waived, that decision needs a baseline + // section and an issue — not an edit to `en.ts` to make the red go away. + expect(driftOf(repoRoot)).toEqual([]); + }); +}); + describe('the baseline is a ratchet', () => { const finding = (reason: string, detail: string) => ({ reason, detail, file: 'f.tsx', line: 1, column: 1 }); diff --git a/scripts/check-i18n-call-site-keys.mjs b/scripts/check-i18n-call-site-keys.mjs index bcfdeff5b8..1cd64aa7da 100644 --- a/scripts/check-i18n-call-site-keys.mjs +++ b/scripts/check-i18n-call-site-keys.mjs @@ -1,9 +1,12 @@ #!/usr/bin/env node /** - * Every key a component asks `t()` for must EXIST in the `en` locale pack. + * Every key a component asks `t()` for must EXIST in the `en` locale pack — and + * where the call site also writes an inline `defaultValue`, that dead string + * must say the same thing the pack does (objectui#3810). * * Run: node scripts/check-i18n-call-site-keys.mjs (also `pnpm check:i18n-keys`) - * Exit: 0 = every in-scope call-site key resolves (or is baselined), 1 = it does not + * Exit: 0 = every in-scope call-site key resolves (or is baselined) and no + * inline default contradicts its `en` value, 1 = otherwise * * ## The gap this closes (objectui#3530) * @@ -27,15 +30,23 @@ * * All three read the same ten packs, and each is blind to what the next owns: * - * - THIS gate: call site -> `en`. Does the key a component asks for exist? + * - THIS gate: call site -> `en`. Does the key a component asks for exist, and + * does the call site's own inline `defaultValue` agree with the `en` value? + * Both directions of one question — what this call site will render. * - `packages/i18n/src/__tests__/all-locales-key-parity.test.ts`: pack vs pack - * KEY SETS, plus placeholder shape. - * - `scripts/check-i18n-en-drift.mjs` (objectui#3650): the only one that reads - * VALUES, and only as an event — when an `en` string CHANGES, the nine - * translations must change in the same PR (or be waived). Neither key gate - * can see a value go stale: objectui#3582 and objectui#3625 were eight packs - * serving a retired sentence at full key parity, the second one in idiomatic - * native script that every value-shaped heuristic also reads as healthy. + * KEY SETS, plus placeholder shape. Never reads a value. + * - `scripts/check-i18n-en-drift.mjs` (objectui#3650): `en` vs the other nine + * packs, as an event — when an `en` string CHANGES, the nine translations + * must change in the same PR (or be waived). Neither pack gate can see a + * value go stale: objectui#3582 and objectui#3625 were eight packs serving a + * retired sentence at full key parity, the second one in idiomatic native + * script that every value-shaped heuristic also reads as healthy. + * + * The `default-value-drift` class below is the fourth blind spot in that + * partition, and it was blind to all three by construction (objectui#3810): + * this gate asked only whether the key existed, parity reads no values at all, + * and en-drift fires on CHANGE — and in every one of the 43 sites the `en` value + * had not moved for months. The call site was the thing that disagreed. * * ## What is IN scope, and why the answer is not "every `t(`" * @@ -78,7 +89,7 @@ * every one of them a component that was handed the metadata-admin table's `t` * by its parent, and not one of them a real finding. * - * ## Two failure classes + * ## Three failure classes * * 1. `missing-key` — a literal key with no leaf in `en`. i18next plural suffixes * (`_one`, `_other`, …) count as defining the base key, and a key passed with @@ -87,6 +98,33 @@ * whose static head matches NO `en` key at all. Then every possible expansion * is missing, whatever the substitution evaluates to. This is the only claim * about a dynamic key that is true without knowing the value. + * 3. `default-value-drift` (objectui#3810) — the key EXISTS and the call site + * still carries a literal `t(key, { defaultValue: 'other English' })` whose + * text differs from the `en` value. i18next uses `defaultValue` only on a + * miss, so with the key present the pack always wins and that string is + * structurally dead code — dead code that states, at the call site, a + * different sentence from the one users read. Two costs, both measured on + * `main`: the reader (and the AI writing the next edit) is misled — + * `ForgotPasswordPage` claimed `If an account exists, a reset link has been + * sent.` while the pack asserts `We've sent a password reset link to + * {{email}}.`, a materially different privacy claim — and on the day the key + * is renamed or dropped, rendering silently falls back to that other + * sentence, in a diff where nobody expected copy to change. + * + * Classes 1 and 3 are disjoint by construction: drift is judged only when the + * key resolves to an `en` leaf this file could read as a string, so a call + * site is never reported twice, and each report reads on its own. Deliberately + * NOT judged (counted instead): a computed default (`defaultValue: label`), a + * call whose key is dynamic or denotes several literals, a `returnObjects` + * subtree, and the plural families — `t('detail.showEmptyRelated')` resolves + * through `_one`/`_other`, and there is no single form to compare against. + * + * The rule is HARD from day one — no baseline section, unlike classes 1-2. + * That is a measurement, not an aspiration: the first full run found 43 sites + * in 19 files, all of them aligned in the same PR (objectui#3810), so there + * is no debt for a ratchet to hold. A `defaultValue` written on a key that is + * NOT yet in `en` stays legal — that transition period runs for months + * (objectui#3546) and is class 1's business, not this one's. * * ## Dynamic keys: the explicit policy * @@ -111,7 +149,8 @@ * ## The baseline * * `scripts/i18n-call-site-key-baseline.json` lists the keys already missing on - * `main` when this gate landed, each with the issue tracking its fix. It is a + * `main` when this gate landed, each with the issue tracking its fix — classes 1 + * and 2 only; class 3 has no baseline and never needed one. It is a * ratchet, not an allowlist: a key that is NOT in it fails, and an entry that no * longer fires (key added to `en`, or its last call site deleted) ALSO fails, so * the file can only shrink. Fixing the debt means adding the key to @@ -173,13 +212,40 @@ const PLURAL_SUFFIXES = ['_zero', '_one', '_two', '_few', '_many', '_other']; // ── the `en` pack ──────────────────────────────────────────────────────────── /** - * Dotted leaf paths of `packages/i18n/src/locales/en.ts`, read from its AST. + * Read a node as a static string, or return `null` if it is not one. + * + * `'a' + 'b'` counts: `en.ts` wraps one long sentence that way + * (`objectActions.resetPackageSetConfirm`), and a leaf this returns `null` for + * is a leaf the `default-value-drift` rule cannot judge — so folding the + * concatenation here is what keeps that one key inside the checked surface + * instead of silently outside it. + */ +function staticString(node, source) { + const inner = unwrapExpression(node); + if (!inner) return null; + if (ts.isStringLiteral(inner) || ts.isNoSubstitutionTemplateLiteral(inner)) return inner.text; + if (ts.isBinaryExpression(inner) && inner.operatorToken.kind === ts.SyntaxKind.PlusToken) { + const left = staticString(inner.left, source); + const right = staticString(inner.right, source); + return left === null || right === null ? null : left + right; + } + return null; +} + +/** + * Dotted leaf paths of `packages/i18n/src/locales/en.ts`, read from its AST, + * plus the leaf VALUES — the strings the app actually renders. * * Parsed rather than imported so the gate needs no build step and no TS loader. * `scripts/__tests__/check-i18n-call-site-keys.test.ts` pins this extraction - * against the real module evaluated by vitest, so the two cannot drift. + * against the real module evaluated by vitest, keys AND values, so the two + * cannot drift. + * + * `values` holds only leaves that read as a static string. Every leaf is in + * `leaves` either way: the key rules judge existence and need no value, and the + * value rule declines to judge what it could not read rather than guessing. * - * @returns {{ leaves: Set, branches: Set }} + * @returns {{ leaves: Set, branches: Set, values: Map }} */ export function collectEnKeys(root) { const file = join(root, 'packages/i18n/src/locales/en.ts'); @@ -209,6 +275,7 @@ export function collectEnKeys(root) { const leaves = new Set(); const branches = new Set(); + const values = new Map(); const walk = (object, prefix) => { for (const prop of object.properties) { if (!ts.isPropertyAssignment(prop)) { @@ -228,11 +295,13 @@ export function collectEnKeys(root) { walk(value, path); } else { leaves.add(path); + const text = staticString(value, source); + if (text !== null) values.set(path, text); } } }; walk(literal, ''); - return { leaves, branches }; + return { leaves, branches, values }; } // ── source walk ────────────────────────────────────────────────────────────── @@ -448,6 +517,30 @@ function literalKeysOf(argument, source) { return { keys, dynamic }; } +/** + * The inline `defaultValue` an options argument carries (objectui#3810). + * + * `{ present: false }` — no `defaultValue` property at all. + * `{ present: true, text }` — a static string this rule can compare. + * `{ present: true, text: null }` — written, but computed (a template with a + * substitution, a variable, a ternary). Not + * comparable, so it is counted, never failed. + */ +function inlineDefaultValue(node, source) { + for (const argument of node.arguments.slice(1)) { + const inner = unwrapExpression(argument); + if (!inner || !ts.isObjectLiteralExpression(inner)) continue; + for (const property of inner.properties) { + if (!ts.isPropertyAssignment(property)) continue; + const name = + ts.isIdentifier(property.name) || ts.isStringLiteral(property.name) ? property.name.text : null; + if (name !== 'defaultValue') continue; + return { present: true, text: staticString(property.initializer, source) }; + } + } + return { present: false, text: null }; +} + /** The literal head of a template key, i.e. everything before the first `${`. */ function staticHead(argument) { const inner = unwrapExpression(argument); @@ -464,7 +557,7 @@ function staticHead(argument) { * @returns {{ findings: Array, counters: Record, enKeyCount: number }} */ export function analyze(root) { - const { leaves, branches } = collectEnKeys(root); + const { leaves, branches, values } = collectEnKeys(root); const resolvesLeaf = (key) => leaves.has(key) || PLURAL_SUFFIXES.some((suffix) => leaves.has(key + suffix)); // Materialised once, not inside the predicate: spreading a 2.6k-entry Set per // candidate head is the shape that made `all-locales-key-parity` quadratic @@ -487,6 +580,10 @@ export function analyze(root) { skippedLocalTable: 0, skippedNotATranslator: 0, skippedMethodCall: 0, + literalDefaultValues: 0, + matchingDefaultValues: 0, + computedDefaultValues: 0, + unjudgedDefaultValues: 0, }; for (const file of collectSourceFiles(root)) { @@ -601,6 +698,32 @@ export function analyze(root) { if (resolvesLeaf(key) || (returnsObjects && branches.has(key))) counters.resolvedKeys += 1; else findings.push({ reason: 'missing-key', ...at, detail: key }); } + + // objectui#3810 — the inline default is dead code the moment the key + // exists, so it must not say something else. Deliberately narrow: one + // literal key, whose `en` leaf is a plain string. A key `en` does not + // define is `missing-key`'s territory and is NOT reported here too; + // keeping the two classifications disjoint is what lets either output + // be read on its own. + const inlineDefault = inlineDefaultValue(node, source); + if (inlineDefault.present && inlineDefault.text === null) { + counters.computedDefaultValues += 1; + } else if (inlineDefault.present) { + counters.literalDefaultValues += 1; + const key = !dynamic && keys.length === 1 ? keys[0] : null; + const enValue = key !== null && !returnsObjects ? values.get(key) : undefined; + if (enValue === undefined) { + // No single static key, or a key whose `en` leaf this parser could + // not read as a string — including the plural families, where + // there is no one form to compare against. Counted, never failed. + counters.unjudgedDefaultValues += 1; + } else if (enValue === inlineDefault.text) { + counters.matchingDefaultValues += 1; + } else { + findings.push({ reason: 'default-value-drift', ...at, detail: key, expected: enValue, actual: inlineDefault.text }); + } + } + if (dynamic) { counters.dynamicKeySites += 1; const head = staticHead(argument); @@ -681,8 +804,23 @@ const HINTS = { '`createSafeTranslation(...)` bound to a name outside the `use*Translation` /' + ' `use*Translate` / `use*T` convention. Every call through it would leave this' + ' gate\'s checked surface silently — rename it to the convention.', + 'default-value-drift': + 'The key EXISTS in `en`, so i18next serves the pack value and this inline' + + ' `defaultValue` never renders — but it says something else, which misleads every' + + ' later reader of this component and becomes the visible copy the day the key is' + + ' renamed (objectui#3810). Fix it at the CALL SITE: copy the `en` value in' + + ' byte-for-byte, ellipsis and capitalisation included. Do NOT edit' + + ' `packages/i18n/src/locales/en.ts` to match the call site — the pack value is what' + + ' users read today, and changing it makes `scripts/check-i18n-en-drift.mjs` demand' + + ' the same change in the other nine packs. If the pack value is genuinely the wrong' + + ' copy for this spot, that is a copy change in its own PR, or the call site is asking' + + ' for the wrong key.', }; +/** JSON-quoted, with every non-ASCII byte escaped — `...` vs `…` must be visible. */ +const quote = (text) => + JSON.stringify(text).replace(/[^\x20-\x7e]/g, (character) => `\\u${character.charCodeAt(0).toString(16).padStart(4, '0')}`); + const invokedDirectly = process.argv[1] && resolve(process.argv[1]) === resolve(fileURLToPath(import.meta.url)); if (invokedDirectly) { @@ -710,26 +848,55 @@ if (invokedDirectly) { `${counters.skippedLocalTable} module-local table, ${counters.skippedNotATranslator} not a translator, ` + `${counters.skippedMethodCall} method call.`, ); + console.log( + `Inline defaults: ${counters.literalDefaultValues} literal ` + + `(${counters.matchingDefaultValues} match their en value, ${counters.unjudgedDefaultValues} not comparable), ` + + `${counters.computedDefaultValues} computed (report-only).`, + ); + + // Split by class before printing: the two key classes say "en does not define + // this", the drift class says "en defines it differently", and one paragraph + // cannot honestly introduce both. + const drift = unexpected.filter((finding) => finding.reason === 'default-value-drift'); + const keyFindings = unexpected.filter((finding) => finding.reason !== 'default-value-drift'); if (unexpected.length === 0 && stale.length === 0) { - console.log(`Every in-scope call-site key resolves against the en pack (${enKeyCount} keys).`); + console.log( + `Every in-scope call-site key resolves against the en pack (${enKeyCount} keys), and every` + + ' literal inline defaultValue matches the value the pack serves.', + ); process.exit(0); } - if (unexpected.length > 0) { - const distinct = new Set(unexpected.map((f) => `${f.reason} :: ${f.detail}`)); + if (keyFindings.length > 0) { + const distinct = new Set(keyFindings.map((f) => `${f.reason} :: ${f.detail}`)); console.error( - `\n${unexpected.length} call site${unexpected.length === 1 ? '' : 's'} reference${unexpected.length === 1 ? 's' : ''} ` + + `\n${keyFindings.length} call site${keyFindings.length === 1 ? '' : 's'} reference${keyFindings.length === 1 ? 's' : ''} ` + `a key the en pack does not define (${distinct.size} distinct):`, ); - for (const finding of unexpected) { + for (const finding of keyFindings) { console.error(` ${finding.file}:${finding.line}:${finding.column} [${finding.reason}] ${finding.detail}`); } - for (const reason of Object.keys(HINTS)) { - if (unexpected.some((finding) => finding.reason === reason)) console.error(`\n${reason}: ${HINTS[reason]}`); + } + + if (drift.length > 0) { + const distinct = new Set(drift.map((finding) => finding.detail)); + console.error( + `\n${drift.length} inline defaultValue${drift.length === 1 ? '' : 's'} contradict${drift.length === 1 ? 's' : ''} ` + + `the en value of a key that EXISTS (${distinct.size} distinct key${distinct.size === 1 ? '' : 's'}) — ` + + 'the pack value is what renders, so the call site is stating a sentence nobody sees:', + ); + for (const finding of drift) { + console.error(` ${finding.file}:${finding.line}:${finding.column} [${finding.reason}] ${finding.detail}`); + console.error(` en renders: ${quote(finding.expected)}`); + console.error(` call site: ${quote(finding.actual)}`); } } + for (const reason of Object.keys(HINTS)) { + if (unexpected.some((finding) => finding.reason === reason)) console.error(`\n${reason}: ${HINTS[reason]}`); + } + if (stale.length > 0) { console.error( `\n${stale.length} baseline entr${stale.length === 1 ? 'y is' : 'ies are'} stale — the defect is gone, so the` + diff --git a/scripts/check-i18n-en-drift.mjs b/scripts/check-i18n-en-drift.mjs index e0bea426a7..093610b015 100644 --- a/scripts/check-i18n-en-drift.mjs +++ b/scripts/check-i18n-en-drift.mjs @@ -224,9 +224,10 @@ function packLiteral(source, label) { * how a long sentence is wrapped across source lines: `en`'s * `objectActions.resetPackageSetConfirm` is written that way today, and the * folded text is what i18next serves, so it is what must be compared. (The - * value extractor's counterpart in `check-i18n-call-site-keys.mjs` treats such - * a node as an opaque leaf and is right to — it only needs the KEY. This gate - * needs the text, so it has to fold.) + * counterpart in `check-i18n-call-site-keys.mjs` folds it the same way and for + * the same reason, since objectui#3810 gave that gate a value rule too: a leaf + * neither can read as a string stays a key-only leaf there, judged for + * existence and not for text.) */ function stringValueOf(node) { const value = unwrap(node);