Skip to content

fix(ui): localize the Astryx chrome adopted since #1795 - #2202

Merged
Astro-Han merged 7 commits into
apache:mainfrom
GabrielDrapor:fix/astryx-chat-i18n
Aug 5, 2026
Merged

fix(ui): localize the Astryx chrome adopted since #1795#2202
Astro-Han merged 7 commits into
apache:mainfrom
GabrielDrapor:fix/astryx-chat-i18n

Conversation

@GabrielDrapor

@GabrielDrapor GabrielDrapor commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Problem

The scroll-to-bottom pill in the transcript reads "New messages" in a Chinese UI.

#1795 moved the chat surfaces onto Astryx ChatLayout and deleted the label={copy.jumpLatest} that fed Maka's own scroll-to-bottom button. ChatLayout supplies its own label from t('@astryx.chatLayout.newMessages'), but no matching entry was added to astryxMessageOverrides — and Astryx ships only en, fr-FR and pseudo catalogs. That file's own rule is that a slice adopting a new Astryx surface registers its @astryx.* keys in the same PR; this one did not, so the copy silently fell back to English.

The same gap covers every other Astryx surface adopted without its copy, not just the pill.

Audit

Compared all 219 shipped Astryx keys against the components Maka actually renders. Reachability by import is misleading — the top-level barrel re-export makes all 219 keys look live — so the filter is JSX usage: ChatLayout, Banner (25 call sites), Selector (15), TextInput (21), SideNav, TabList, Table, CommandPalette, DateTimeInput, Thumbnail, Token, MultiSelector, MoreMenu, DropdownMenu, Breadcrumbs, AppShellplus Lightbox, reached through useLightbox in chat-turn.tsx (image preview), which a JSX-tag scan misses.

82 rendered keys had no override (78 from the JSX sweep + 4 Lightbox keys found after the guard's premise fell). This adds them, reusing existing catalogue entries where the string is identical rather than duplicating (form.clear, form.selectPlaceholder, primitives.close, primitives.loading).

Deliberately excluded as dead config: @astryx.dateInput.dialogLabel and @astryx.dateInput.placeholderDateTimeInput references neither, and no standalone DateInput is rendered.

The guard that let it through

markdown-body.test.ts asserted that no override key matches a ban list of surfaces Maka supposedly never renders. Both entries had rotted: chat stopped being true at #1795 (and the guard then actively blocked this fix), and lightbox was never verified — useLightbox renders it. A ban list keyed to "what we render today" goes stale silently, so it is removed entirely; presence tests pin the live surfaces (chat chrome + Lightbox) instead, asserting each key exists and holds no Latin letters.

Also fixed while here (review findings)

  • P1 — refactor(ui): unify streaming (live) and committed answer onto one render path #642 streaming fallback leaked an English aria-label: the rare zero-turns streaming path rendered a bare ChatMessage, resolving Astryx's shipped Message from {sender}. It now reuses LocalizedChatMessage with the same assistantAriaLabel as the normal path; a regression test pins the exact trigger (wait indicators with zero turns — a liveTurn projects a real turn and takes the localized path instead).
  • Off-barrel: the new catalogue lives in an unexported astryx-copy.ts instead of expanding SharedUiCopy (which export *s through the barrel). The never-applied en mirror is dropped rather than left to drift.
  • The e2e spec that located the quote token remove button by /^Remove / now uses its localized name /^移除/.

Copy notes

  • chatLayout.newMessages跳到最新消息 (kept from the pre-refactor(ui): migrate chat surfaces to Astryx layout #1795 wording users know, over a semantically tidier rename).
  • commandPalette.resultCount: the zh string keeps {count, number} formatting and drops the English plural branch — Chinese does not inflect for number, so a plural clause would be dead config. ICU formatting applies to overrides (resolve.ts runs the formatter), so all {placeholder}s are preserved.
  • token.remove → 移除{label}, composerDrawer.label → 附加内容 (purpose-named; the English "Items" is too generic to transliterate).

Validation

  • @maka/ui suite — 325/325 pass; typecheck and biome lint clean
  • Regression tests verified to fail against the unfixed code (both the aria-label leak and a deleted/untranslated override)
  • quote-companion e2e passes locally; full monorepo build clean

Co-Authored-By: Claude noreply@anthropic.com

https://claude.ai/code/session_01Ac5rv6WUKWtMZgQb5QPN16

GabrielDrapor and others added 6 commits August 5, 2026 12:18
apache#1795 moved the chat surfaces onto Astryx ChatLayout and deleted the
`label={copy.jumpLatest}` that fed Maka's own scroll-to-bottom button,
but never registered the replacement `@astryx.*` keys. Astryx ships no
zh catalog, so the pill has been reading "New messages" in a Chinese UI
ever since — and the same gap covers every other Astryx surface adopted
without its copy.

Audited all 219 shipped Astryx keys against the components Maka actually
renders (JSX usage, not imports — the barrel re-export makes every
component look reachable). 79 rendered keys had no override; this adds
them, reusing existing entries where the string is identical rather than
duplicating (form.clear, primitives.close/loading, selectPlaceholder).

Excluded as dead config per this file's own rule: dateInput.dialogLabel
and dateInput.placeholder — DateTimeInput references neither, and no
standalone DateInput is rendered.

The guard test listed `chat` as a surface Maka does not render, which is
what let the regression through; it now guards lightbox only, and a new
test pins the chat chrome.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ac5rv6WUKWtMZgQb5QPN16
Astryx expands the pill only when new messages arrived, so the label
reads as a notice rather than an action — but the copy this replaced
(apache#1194's `jumpLatest`, dropped by apache#1795) said 跳到最新消息, and keeping
the wording users already know beats a semantically tidier rename.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ac5rv6WUKWtMZgQb5QPN16
`messages[key] ?? ''` coalesced a deleted override to the empty string,
which holds no Latin letters and satisfied the translation check on its
own — so the loop caught an English value but not a removed key.

Verified both ways: deleting the send override now fails with 'missing
override', and reverting its copy to English fails with 'untranslated'.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ac5rv6WUKWtMZgQb5QPN16
The dead-config guard's surviving entry was as stale as the one apache#1795
invalidated: chat-turn.tsx opens image previews through useLightbox,
which renders Astryx's Lightbox — with four English strings — while a
JSX-tag scan for <Lightbox> reports the component unrendered.

Add the four overrides (close reuses primitives.close) and retire the
ban-list guard entirely: a list keyed to "what we render today" rots
silently, and both of its entries had. Live surfaces are pinned by
presence tests instead.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ac5rv6WUKWtMZgQb5QPN16
The token remove button read 'Remove {label}' only because Astryx's
Token had no zh override; now that @astryx.token.remove is localized the
accessible name is 移除…, and the /^Remove / locator times out. The spec
already addresses every other control by its Chinese name.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ac5rv6WUKWtMZgQb5QPN16
…fallback

Review findings on apache#2202, all verified before fixing:

- The astryx catalogue expanded SharedUiCopy, which `export *`s through
  the package barrel — violating the README's off-barrel convention for
  symbols with no cross-package consumer. It now lives in astryx-copy.ts,
  deliberately unexported.
- The en mirror existed only as a diffable reference and was never
  applied (astryxMessageOverrides returns undefined for en); dropped
  rather than left to drift against upstream.
- `a` renamed to `astryx`; duplicated presence/Latin-letter assertions
  extracted into assertChineseAstryxOverrides.
- P1: the apache#642 streaming fallback rendered a bare ChatMessage, which
  resolves Astryx's shipped 'Message from {sender}' — an English
  aria-label in a Chinese tree. It now reuses LocalizedChatMessage with
  the same assistantAriaLabel as the TurnView path. The regression test
  pins the exact trigger: wait indicators with zero turns — passing a
  liveTurn instead projects a real turn and takes the localized path,
  which is how the first draft of the test passed against the bug.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ac5rv6WUKWtMZgQb5QPN16
@Astro-Han

Copy link
Copy Markdown
Contributor

Thorough fix — the zh catalog reads naturally, the E2E matcher change is the test-side half of the fix (suite runs zh; /^Remove / was enshrining the leak). Notes from an independent pass:

  • P1 — one live key slipped through: @astryx.chatMessage.messageFrom isn't overridden; the streaming fallback at chat-view.tsx:539 renders a bare <ChatMessage sender="assistant">, so zh users still get "Message from assistant" in the a11y tree. Wrapping it in LocalizedChatMessage (or adding the key) closes the same bug class this PR fixes — it's a small change, would be great to land before merge.
  • P2 — ~8 overrides look dead: chatSendButton.send/stop (composer.tsx:1526 supplies its own send button), appShell.mobileNavigation (app-shell.tsx:2107 hasToggle:false), table.filter.*/tableFiltering.filterByColumn/table.noData (only rowHeader plugin mounted; empty states short-circuit). Worth pruning per the file's own rule — and the new test pins chatSendButton.send, so swap it for a live chat key (e.g. chatToolCalls.error).
  • P2 — the other ~66 keys are unpinned: tests cover only chat + lightbox. A one-loop whole-map sweep (presence + no Latin, stripping {…} segments) plus an ICU placeholder-parity and key-membership check would make future slices auditable and catch silent runtime throwers.
  • P2 (suggestion) — newMessages reads as an action: '跳到最新消息' translates the click; the original is passive pill text — '查看新消息' fits better.
  • P3 — comment fix: "a deleted key would coalesce to ''" isn't how resolution works — a missing override falls back to the shipped en catalog (English → Latin), so the Latin-free check would already fail. Worth rewording.

Only the P1 blocks merge (and it's a few lines); everything else can be follow-ups. Nice work otherwise! 🚢

Maintainer review on apache#2202, each claim verified before acting:

- chatSendButton.send/stop: ChatComposerInput never renders
  ChatSendButton — composer.tsx supplies its own send button. Pruned;
  the pinned test now anchors on chatToolCalls.error instead.
- appShell.mobileNavigation: gated on shouldShowAutoToggle, and Maka
  passes breakpoint 'none' + hasToggle false. Pruned.
- table.noData / table.filter.* / tableFiltering.filterByColumn: the
  usage table mounts only the rowHeader plugin and renders EmptyState
  before Table when rows are empty. Pruned.
- New whole-map sweep: every override must target a key Astryx ships,
  hold no Latin outside {…}, and carry the same top-level ICU arguments
  as the en default — depth-aware extraction, since plural branch text
  ({result}) reads like an argument to a naive regex and would flag
  every zh string that drops an inapplicable plural.
- Presence-helper comment reworded: the check reads the override map
  directly, so a deleted entry yields '' here; at runtime the same miss
  falls back to Astryx's shipped en catalog.

Sweep red-validated both ways: a misspelled key fails membership, an
English value fails the Latin check.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ac5rv6WUKWtMZgQb5QPN16
@GabrielDrapor

Copy link
Copy Markdown
Contributor Author

Thanks for the pass — all five verified and addressed.

  • P1 (messageFrom): our pushes crossed — 3901e14 (a few minutes before your comment) wraps the refactor(ui): unify streaming (live) and committed answer onto one render path #642 fallback in LocalizedChatMessage with the same assistantAriaLabel as the TurnView path. The regression test pins the exact trigger you'd need to reproduce it: wait indicators with zero turns — passing a liveTurn projects a real turn and takes the localized path, which is how the first draft of the test passed against the bug.
  • P2 (dead overrides): all three claims check out — ChatComposerInput never renders ChatSendButton; mobileNav is breakpoint: 'none', hasToggle: false so shouldShowAutoToggle can't fire; the usage table mounts only the rowHeader plugin and renders EmptyState before Table on empty rows. All 8 pruned in 47d74dd; the pinned test now anchors on chatToolCalls.error as suggested.
  • P2 (unpinned keys): added a whole-map sweep — every override must target a key Astryx actually ships, hold no Latin outside {…} segments, and carry the same top-level ICU arguments as the en default. Argument extraction is brace-depth-aware: plural branch text ({result}) reads like an argument to a naive regex and would flag every zh string that legitimately drops an inapplicable plural. Red-validated both ways (misspelled key → membership failure; English value → Latin failure).
  • P3 (comment): reworded — the check reads the override map directly (no catalog resolution), so a deleted entry yields '' at the assertion site; your point that the runtime miss resolves to the shipped en catalog is now stated alongside it.
  • newMessages wording: kept 跳到最新消息 deliberately — it's the pre-refactor(ui): migrate chat surfaces to Astryx layout #1795 label users already know, and the author prefers the continuity. No objection to revisiting in a follow-up if you feel strongly.

Suite at 326/326, lint clean.

@Astro-Han

Copy link
Copy Markdown
Contributor

Verified all the fixes on the current head — P1 (LocalizedChatMessage on the #642 fallback, regression test passes), all 8 dead overrides pruned, and the whole-map sweep (membership + no-Latin + ICU-arg parity) is a nice upgrade over my suggestion. @maka/ui build and the pinned tests pass locally, and CI is green. LGTM — merging. 🚢

@Astro-Han
Astro-Han merged commit e8bacf7 into apache:main Aug 5, 2026
12 checks passed
@GabrielDrapor
GabrielDrapor deleted the fix/astryx-chat-i18n branch August 5, 2026 12:27
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