Skip to content

feat(i18n): port check-keys.py + add phase 6 unit tests - #327

Merged
krisarmstrong merged 2 commits into
mainfrom
feat/i18n-check-keys-and-tests
May 27, 2026
Merged

feat(i18n): port check-keys.py + add phase 6 unit tests#327
krisarmstrong merged 2 commits into
mainfrom
feat/i18n-check-keys-and-tests

Conversation

@krisarmstrong

Copy link
Copy Markdown
Collaborator

Summary

Cross-repo backfill from niac-go#723 (check-keys.py) and niac-go#724
(Phase 6 test coverage). Brings stem to parity with niac.

What lands

  • `scripts/i18n/check-keys.py` — read-only static analysis that
    cross-references `t()` calls in `ui/src` against committed EN
    locale JSON. Catches typo'd key names and dead JSON keys.
    Identical to the niac script; added `--ratchet` flag so newly-
    added repos can absorb it without an immediate cleanup burden
    (stem currently has 48 t() calls referencing non-existent keys —
    pre-existing bugs masked by i18next's fallback semantics, queued
    for cleanup).

  • `scripts/i18n/validate.sh` — pulled in the niac improvements:

    • `check_key_usage` step delegating to check-keys.py with ratchet
      propagation
    • Word-boundary `\\bt\\(` regex fix for the fallback-pattern check
      so identifiers ending in `t` (`headers.set('Accept',
      'application/json')`) don't false-match.
  • `ui/vitest.config.ts` — adds `@locales` path alias so
    `i18n/index.ts` can be imported under test, plus dedupe entries
    for i18next/react-i18next/react to encourage module deduplication.

  • `ui/src/hooks/useLocale.test.ts` — 3 unit tests for the BCP-47
    normalization (`en` → `en-US`, `es` → `es-ES`, unknown → raw
    passthrough). Uses `vi.mock('react-i18next')` because vitest's
    jsdom + Vite's module resolution otherwise produces separate
    i18next instances for direct imports vs react-i18next's chain,
    breaking React Context subscription assertions.

  • `ui/src/i18n/i18n.test.ts` — i18next configuration smoke + key
    resolution + `` sync (WCAG 3.1.1/3.1.2). 11 assertions
    covering declared langs (en/es), declared namespaces (9), default
    ns = common, every namespace has EN+ES bundles loaded, top-level

    • pluralized key resolution, plus
      `document.documentElement.lang` update on language change.

Test plan

  • `vitest run` — 136 passed (124 prior + 12 new)
  • `./scripts/i18n/validate.sh --ratchet` OK with 4 warnings
    (3 unchanged + 48 new t() call errors demoted to warning by
    check-keys.py `--ratchet`)
  • CI green for `Frontend (TypeScript)` + `i18n Validation` jobs

Out of scope

  • E2E spec for language switching deferred to a separate PR — stem's
    UI shell pattern differs enough from niac's that the spec needs
    fresh markers, and this PR is already at a reasonable size.
  • The 48 t() calls referencing missing keys (wrong-namespace prefix
    bugs, e.g. `t('common:settings.interface.title')` when the key
    lives in `settings.json`). i18next's fallback shows the key as
    English text so the bugs are invisible to users — but ES
    translations don't resolve. Will land in a follow-up batch.

Cross-repo backfill from niac-go#723 (check-keys.py) and niac-go#724
(Phase 6 test coverage). Brings stem to parity with niac.

What lands:

- scripts/i18n/check-keys.py — read-only static analysis that
  cross-references t() calls in ui/src against committed EN locale
  JSON. Catches typo'd key names and dead JSON keys. Identical to
  the niac script; added --ratchet flag so newly-added repos can
  absorb it without an immediate cleanup burden (stem currently has
  48 t() calls referencing non-existent keys — pre-existing bugs
  masked by i18next's fallback semantics, queued for cleanup).
- scripts/i18n/validate.sh — pulled in the niac improvements:
  (a) check_key_usage step delegating to check-keys.py with ratchet
      propagation
  (b) word-boundary \\bt\\( regex fix for the fallback-pattern check
      so identifiers ending in `t` (headers.set('Accept',
      'application/json')) don't false-match
- ui/vitest.config.ts — adds @locales path alias so i18n/index.ts
  can be imported under test, plus dedupe entries for i18next/
  react-i18next/react to encourage module deduplication.
- ui/src/hooks/useLocale.test.ts — 3 unit tests for the BCP-47
  normalization (en->en-US, es->es-ES, unknown->raw passthrough).
  Uses vi.mock('react-i18next') because vitest's jsdom + Vite's
  module resolution otherwise produces separate i18next instances
  for direct imports vs react-i18next's chain, breaking React
  Context subscription assertions.
- ui/src/i18n/i18n.test.ts — i18next configuration smoke + key
  resolution + <html lang> sync (WCAG 3.1.1/3.1.2). 11 assertions
  covering declared langs (en/es), declared namespaces (9), default
  ns = common, every namespace has EN+ES bundles loaded, top-level/
  pluralized key resolution, plus document.documentElement.lang
  update on language change.

Validation: vitest run 136 passed (124 prior + 12 new); validator
--ratchet OK with 4 warnings (3 unchanged + 48 new t() call errors
demoted to warning by check-keys.py --ratchet — queued for cleanup).

E2E spec for language switching deferred to a separate PR — stem's
UI shell pattern differs enough from niac's that the spec needs
fresh markers, and this PR is already at a reasonable size.

Cleanup follow-up: 48 t() calls in stem reference keys that don't
exist in the EN locale (most are wrong-namespace prefixes like
t('common:settings.interface.title') when the key actually lives in
settings.json). i18next's fallback shows the key as English text,
so the bugs are invisible to users — but ES translations don't
resolve. Will land in a follow-up batch.
@krisarmstrong
krisarmstrong enabled auto-merge (squash) May 27, 2026 14:07
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@github-actions

Copy link
Copy Markdown
Contributor

License Compliance Report

All dependencies pass license compliance checks

Go Dependencies

  • Unknown: 31 package(s)
  • MIT: 26 package(s)
  • BSD-3-Clause: 16 package(s)
  • Apache-2.0: 11 package(s)
  • BSD-2-Clause: 1 package(s)

npm Dependencies

See full report in workflow artifacts

Allowed Licenses: MIT, Apache-2.0, BSD-*, ISC, CC0-1.0, MPL-2.0
Forbidden: GPL, AGPL, SSPL (strong copyleft)

`local args=()` + `${args[@]}` errors on macOS bash 3.2 under
`set -u`. Use `local extra=""` + `$extra` instead — works on both
bash 3.2 and 5.x. Same patch already in seed PR #1203.

Detected when running ./scripts/i18n/validate.sh --ratchet locally
on macOS; the validator FAILED instead of demoting the
check_key_usage errors as intended by --ratchet.

No behavior change in CI (Linux runners use bash 5.x where
${empty[@]} expands fine); fixes local-dev workflow.
@github-actions

Copy link
Copy Markdown
Contributor

License Compliance Report

All dependencies pass license compliance checks

Go Dependencies

  • Unknown: 31 package(s)
  • MIT: 26 package(s)
  • BSD-3-Clause: 16 package(s)
  • Apache-2.0: 11 package(s)
  • BSD-2-Clause: 1 package(s)

npm Dependencies

See full report in workflow artifacts

Allowed Licenses: MIT, Apache-2.0, BSD-*, ISC, CC0-1.0, MPL-2.0
Forbidden: GPL, AGPL, SSPL (strong copyleft)

@krisarmstrong
krisarmstrong merged commit 85a3382 into main May 27, 2026
26 of 28 checks passed
@krisarmstrong
krisarmstrong deleted the feat/i18n-check-keys-and-tests branch May 27, 2026 14:26
krisarmstrong added a commit that referenced this pull request May 27, 2026
)

Stem had 48 t() call sites referencing keys that didn't exist in
the EN locale JSON. i18next's fallback semantics (second arg to t())
showed the key as English text so users didn't see broken UI — but
ES translations never resolved, leaving the Spanish version
permanently showing English for these labels.

Surfaced by check-keys.py landing via PR #327; cleanup of the
backlog so the --ratchet flag can eventually be dropped from CI.

Two-pronged fix:

(1) Namespace prefix corrections (24 call sites) — keys actually
exist in settings.json but t() calls referenced them via the
default common namespace. Fixed by changing literal call sites to
use the settings: cross-namespace prefix:

  t('settings.interface.title', 'Interface')
    → t('settings:interface.title', 'Interface')

Affected: SettingsDrawer, HeaderInterfaceSelector, InterfaceSection,
ModeSection, ReflectorSection, RFC2544Section, RFC2889Section,
RFC6349Section, Y1564Section, Y1731Section, MEFSection,
TrafficGenSection, TSNSection.

(2) Missing keys added to locale JSON (24 keys + ES translations):

common.json:
- accessibility.{closeSettings, openHistory, refreshInterfaces,
  selectProfile}
- interface.{networkInterfaces, noInterfaces, select}
- profile.{current, manage, noProfiles, select}
- tooltips.header.{refresh, history, help, settings, logout}
- status.{clickToReconnect, tapToReconnect, tapToReconnectHint}

settings.json:
- viewModuleHint, viewStandardHint
- tests.{rfc2544, rfc2889, rfc6349, y1564, y1731, mef, trafficgen,
  tsn}.title (8 test category headings)

setup.json:
- buttons.copyTooltip

ES translations follow I18N_TRANSLATION_MEMORY.md (formal usted,
sentence case, glossary terms preserved verbatim — Reflector, MEF,
RFC numbers, Y.1564 etc. stay English).

Validation: tsc clean; biome clean; vitest 136 passed; check-keys
strict mode now passes (was 48 errors, now 0); validator strict
still fails on the pre-existing 355 fallback-pattern check (separate
cleanup) and i18next 26.2 → 26.3 version bump (PR #317).

Drop the --ratchet flag from .github/workflows/ci.yml's
check_key_usage invocation in a separate PR once the related
fallback-pattern cleanup also lands.

Co-authored-by: Kris Armstrong <kris.armstrong@icloud.com>
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