Skip to content

fix(platform): restore i18n keys lost to orphan detector false positives#1642

Merged
yannickmonney merged 2 commits into
mainfrom
fix/i18n-orphan-detector-false-positives
Apr 27, 2026
Merged

fix(platform): restore i18n keys lost to orphan detector false positives#1642
yannickmonney merged 2 commits into
mainfrom
fix/i18n-orphan-detector-false-positives

Conversation

@yannickmonney
Copy link
Copy Markdown
Contributor

@yannickmonney yannickmonney commented Apr 27, 2026

Summary

PR #1632 introduced an orphan-key test that wrongly classified 109 in-use translations as unused and stripped them from en/de/fr.json. This PR hardens the detector and restores the lost keys.

The detector previously only recognised translation aliases destructured directly from useT(...) in the current file. It missed:

  • Callback aliases({ tTables }) => tTables('headers.product') where the alias arrives via a destructured callback parameter (use-products-table-config.tsx, use-customers-table-config.tsx, use-vendors-table-config.tsx).
  • Custom-hook returnsuse-sso-config-form.ts does const { t } = useT('settings') and returns t; consumers like sso-config-dialog.tsx destructure it from the hook return without a local useT(...).
  • Lookup tables — keys stored in *Key: properties, keys: { ... } blocks, as const casts, or const records named *_I18N_KEY (e.g. CATEGORY_I18N_KEY in sanitize-chat-error.ts).
  • Ternaries inside t(...)t(isDisabled ? 'disabled' : 'noMembership') in dashboard/$id.tsx.
  • *Key() functions returning translation-key suffixes consumed via t(fn(args)) (statusLabelKey in todo-list-card.tsx).

Detector changes — services/platform/lib/i18n/messages-usage.test.ts

  • Per-file namespace search for dotted suffixes — 'headers.product' in a file binding tables resolves to tables.headers.product.
  • tFoo heuristictTables, tCustomers, etc. register as aliases for the corresponding namespace when called, even without useT(...) in the same file. Skipped for ambiguous names like tEntity.
  • Single-segment lookup-table candidatesxxxKey: property values, keys: { ... } blocks, as const casts, and *_I18N_KEY-named const records. Per-file namespace resolution by default; global fallback only when the file has no namespace bound (cross-file consumer pattern).
  • Ternary-aware call-body capturet(...) calls now scan the whole argument body (one level of nested parens) for string literals.
  • *Key function body parsing — header regex + brace-counting walk to extract the true function body and find every return '...' literal inside.
  • TFunction / parameter-t fallback — files importing TFunction or declaring a t: (key: string) => string parameter trigger global namespace search for their dotted literals.

The per-file restriction prevents the previous over-permissive matching from re-appearing — coincidental literals like 'pii.blocked' (a discriminator code in chat governance) no longer resurrect genuinely orphan translations such as governance.pii.blocked.

Restoration

Restored 109 keys to en.json, de.json, fr.json by reading their values from the pre-#1632 commit. Examples:

  • tables.headers.{product,description,email,scanned,stock,website,actions,created,title,message} and tables.cells.noEmail
  • All 24 of settings.integrations.sso.* consumed by sso-config-dialog.tsx via the hook pattern
  • chat.modelSelector.tags.* (6 keys) consumed via labelKey: lookup
  • chat.errorHint* (10 keys) and chat.errorGeneratingDescription consumed via CATEGORY_I18N_KEY map
  • customers.delete{Confirmation,Error,Warning}, vendors.delete{Confirmation,Error}, websites.toast.deleteError consumed via useDeleteDialogTranslations
  • todoList.status* (5 keys) consumed via the statusLabelKey function
  • accessDenied.noMembership consumed via a ternary
  • auth.forcedChange.{description,descriptionAdminSet}, conversations.priority.*, conversations.category.*, documents.sourceType.*, governance.defaultModels.*ConflictWarning*, governance.moderationProvider.preset*, twoFactor.{grace,lowBackupCodes}.title{One,Other}, and the *.import.errorCodes.* clusters

Verification on the remaining 961 deletions

I ran four independent audits to confirm nothing else is in use:

  1. Full-key literal grep ('<full.key>') across all source — 0 hits.
  2. Namespace-suffix literal grep — 8 hits, all confirmed false positives where the actual key lives under a different namespace.
  3. Aggressive cross-file scanner with the new ternary support — 0 remaining suspicious matches.
  4. Manual spot-check of suspicious clusters (automations.execution.*, automations.monitoring.*, chat.share.*, auth.signup/login/oauth2.*, governance.tabs.*, governance.modelAccess.{allowlist,blocklist}, documents.rag.*, mcpServers.oauth2.*, settings.integrations.{circuly,gmail,outlook,protel,shopify}.*, etc.) — none referenced at runtime; all stale entries from features that were renamed or removed.

Pre-PR checklist

  • Ran bun run check (format, lint, typecheck, all tests) — 21/21 tasks pass.
  • Updated services/platform/messages/{en,de,fr}.json.
  • Updated docs/{,de/,fr/} for every user-visible change — N/A (restores translations the runtime already expects; no UI/text changes).
  • Ran bun run --filter @tale/docs lint — N/A.
  • Updated README.md, README.de.md, README.fr.md — N/A.

Test plan

  • bunx vitest run lib/i18n/ — 8/8 (orphan-key test passes against the restored JSON).
  • bunx tsc --noEmit — clean.
  • npm run lint --workspace=@tale/platform — 0 warnings, 0 errors.
  • bun run check — all 21 workspace tasks succeed.

Summary by CodeRabbit

  • Documentation

    • Expanded UI translations across English, German, and French with new labels and messaging for authentication flows, team management, integrations, model configuration, data import/export, and security features.
  • Tests

    • Improved i18n key usage analysis to better detect and validate translation key patterns.

The orphan-key test introduced in #1632 only recognised translation
aliases destructured directly from `useT(...)` in the current file, so
109 in-use keys consumed via callback parameters, custom-hook returns,
lookup tables, ternaries, or `*Key()` helpers were classified as unused
and stripped from en/de/fr.json.

Hardens the detector with:
- per-file namespace search for dotted suffixes (`'headers.product'`
  inside a file binding `tables` resolves to `tables.headers.product`),
- `tFoo` heuristic that registers callback / parameter aliases,
- single-segment lookup-table candidates (`*Key:` properties, `keys: {
  ... }` blocks, `as const` casts, `*_I18N_KEY` records, `*Key()`
  function returns),
- ternary-aware call-body capture for `t(cond ? 'a' : 'b')`,
- bare-`t` and `TFunction` cross-file fallback for files consuming
  translation keys via passed-in functions.

The per-file restriction prevents the previous over-permissive matching
from re-appearing — coincidental literals like `'pii.blocked'` (a
discriminator code in chat governance) no longer resurrect genuinely
orphan translations such as `governance.pii.blocked`.

Verified against all 1063 keys removed in #1632: 109 restored as
genuinely used, the remaining 961 confirmed orphan.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 27, 2026

📝 Walkthrough

Walkthrough

This PR expands i18n message coverage across English, German, and French locales with new UI labels and strings for table operations, password flows, team management, Microsoft Entra SSO configuration, API key management, conversation filtering, import/export workflows, two-factor authentication, and moderation provider presets. Concurrently, the i18n test utility is enhanced to detect translation key usage through sophisticated pattern matching, including inferred namespace aliases from function calls and indirect string key resolution from constants and type-annotated properties, with cross-file context awareness.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change—restoring 109 i18n keys and hardening the orphan detector to eliminate false positives.
Description check ✅ Passed The description is comprehensive, well-structured, and follows the template with detailed explanation of the problem, solution, restoration details, and verification. All pre-merge checklist items are addressed with clear tick marks or N/A justifications.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/i18n-orphan-detector-false-positives

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@services/platform/lib/i18n/messages-usage.test.ts`:
- Around line 209-213: Add a one-line clarifying comment above the dynamic regex
construction for callBodyRe and innerLiteralRe: note that callBodyRe's inner
pattern uses mutually-exclusive alternatives (?:[^()]|\([^()]*\)) so nested
quantifiers do not produce catastrophic backtracking and that the interpolated
alias is constrained to \w-only captures (no injection risk); also add a short
note that innerLiteralRe is intentionally greedy to capture all quoted tokens
within the call body (which may produce false positives that are later filtered
by allFlatKeys). This comment should be placed next to the callBodyRe and
innerLiteralRe declarations so future readers understand the safety and the
trade-off.
- Around line 352-370: The custom body scanner around
KEY_FUNCTION_HEADER_RE/RETURN_LITERAL_RE is vulnerable because the brace counter
(the loop using bodyStart/depth/i over content) does not skip string literals,
template literals, or comments, causing mis-counts; update the scanner in
messages-usage.test.ts to, when scanning from bodyStart, recognize and skip over
single-quoted, double-quoted and backtick template literal ranges (including
escaped chars and ${...} nested expressions), skip // line comments and /* block
comments, and also ignore regex literals if feasible, so that '{' and '}' inside
those constructs do not affect depth; while here, extend KEY_FUNCTION_HEADER_RE
to also detect arrow-function forms of *Key (e.g., identifiers followed by = or
const/let var patterns with =>) and broaden RETURN_LITERAL_RE (or post-process
matched body) to detect simple ternary return patterns so returned literal
branches are captured.

In `@services/platform/messages/en.json`:
- Around line 2822-2823: The file defines bilingual keys titleOne/titleOther for
lowBackupCodes and grace which hardcodes a one/other plural split; replace these
two-key patterns with a single ICU plural key (e.g. "title") using the {count,
plural, ...} form in services/platform/messages/en.json and update call sites
that currently choose between 'titleOne' and 'titleOther' (the conditional like
count === 1 ? 'titleOne' : 'titleOther') to request the single ICU plural key
with the count argument so the i18n library can select the correct plural form;
make the same replacement for the other occurrence mentioned (lines for
lowBackupCodes and grace).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9f6d7baf-a863-46dc-958e-dab281ed9d70

📥 Commits

Reviewing files that changed from the base of the PR and between be2eb56 and 32e0d49.

📒 Files selected for processing (4)
  • services/platform/lib/i18n/messages-usage.test.ts
  • services/platform/messages/de.json
  • services/platform/messages/en.json
  • services/platform/messages/fr.json

Comment thread services/platform/lib/i18n/messages-usage.test.ts
Comment thread services/platform/lib/i18n/messages-usage.test.ts
Comment thread services/platform/messages/en.json
@yannickmonney yannickmonney merged commit 7da7cf3 into main Apr 27, 2026
17 checks passed
@yannickmonney yannickmonney deleted the fix/i18n-orphan-detector-false-positives branch April 27, 2026 21:43
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.

1 participant