feat(i18n): localize ToolFamily labels (10 MessageIds)#2901
Conversation
- localization.rs: Add 10 ToolFamily* MessageId variants + ALL_MESSAGE_IDS + all 7 locales - tool_card.rs: tool_activity_label_for_name() accepts locale, uses tr() for labels - footer_ui.rs, ui.rs: thread locale to tool_activity_label_for_name() callers - Tests: 2 negative i18n tests + updated existing tests for new signatures
|
Thanks @gordonlu for taking the time to contribute. This repository is currently observing a maintainer-managed contribution gate in dry-run mode, so this pull request is staying open. When enforcement is enabled, pull requests from contributors who are not listed in Please read |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces localization support for tool family labels (such as read, patch, run, find, etc.) across multiple languages in the TUI. It updates the relevant UI and footer components to pass the current UI locale when resolving tool activity labels, and adds unit tests to verify that localized labels do not leak English values. A review comment suggests simplifying the translation logic in tool_activity_label_for_name by mapping the ToolFamily to its corresponding MessageId first, thereby reducing code duplication.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| let label = match family { | ||
| ToolFamily::Read => { | ||
| crate::localization::tr(locale, crate::localization::MessageId::ToolFamilyRead) | ||
| } | ||
| ToolFamily::Patch => { | ||
| crate::localization::tr(locale, crate::localization::MessageId::ToolFamilyPatch) | ||
| } | ||
| ToolFamily::Run => { | ||
| crate::localization::tr(locale, crate::localization::MessageId::ToolFamilyRun) | ||
| } | ||
| ToolFamily::Find => { | ||
| crate::localization::tr(locale, crate::localization::MessageId::ToolFamilyFind) | ||
| } | ||
| ToolFamily::Delegate => { | ||
| crate::localization::tr(locale, crate::localization::MessageId::ToolFamilyDelegate) | ||
| } | ||
| ToolFamily::Fanout => { | ||
| crate::localization::tr(locale, crate::localization::MessageId::ToolFamilyFanout) | ||
| } | ||
| ToolFamily::Rlm => { | ||
| crate::localization::tr(locale, crate::localization::MessageId::ToolFamilyRlm) | ||
| } | ||
| ToolFamily::Verify => { | ||
| crate::localization::tr(locale, crate::localization::MessageId::ToolFamilyVerify) | ||
| } | ||
| ToolFamily::Think => { | ||
| crate::localization::tr(locale, crate::localization::MessageId::ToolFamilyThink) | ||
| } | ||
| ToolFamily::Generic => unreachable!(), | ||
| }; | ||
| label.to_string() |
There was a problem hiding this comment.
Instead of calling crate::localization::tr(locale, ...) in every single match arm, you can map the ToolFamily to its corresponding MessageId first, and then perform a single translation call. This significantly reduces code duplication and improves maintainability.
let message_id = match family {
ToolFamily::Read => crate::localization::MessageId::ToolFamilyRead,
ToolFamily::Patch => crate::localization::MessageId::ToolFamilyPatch,
ToolFamily::Run => crate::localization::MessageId::ToolFamilyRun,
ToolFamily::Find => crate::localization::MessageId::ToolFamilyFind,
ToolFamily::Delegate => crate::localization::MessageId::ToolFamilyDelegate,
ToolFamily::Fanout => crate::localization::MessageId::ToolFamilyFanout,
ToolFamily::Rlm => crate::localization::MessageId::ToolFamilyRlm,
ToolFamily::Verify => crate::localization::MessageId::ToolFamilyVerify,
ToolFamily::Think => crate::localization::MessageId::ToolFamilyThink,
ToolFamily::Generic => unreachable!(),
};
crate::localization::tr(locale, message_id).to_string()![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let checks: &[(MessageId, &str, &str)] = &[ | ||
| (MessageId::ToolFamilyRead, "read", "đọc,读,読,读取,ler,leer"), | ||
| ( | ||
| MessageId::ToolFamilyPatch, | ||
| "patch", | ||
| "vá,補,パ,修补,corrigir,parchear", | ||
| ), | ||
| ( | ||
| MessageId::ToolFamilyRun, | ||
| "run", | ||
| "chạy,執,実,运行,executar,ejecutar", | ||
| ), | ||
| ( | ||
| MessageId::ToolFamilyFind, | ||
| "find", | ||
| "tìm,搜,検,搜索,buscar,buscar", | ||
| ), | ||
| ( | ||
| MessageId::ToolFamilyVerify, | ||
| "verify", | ||
| "xác minh,驗,検,验,verificar,verificar", | ||
| ), | ||
| ]; | ||
| for locale in [ | ||
| Locale::Ja, | ||
| Locale::ZhHans, | ||
| Locale::ZhHant, | ||
| Locale::PtBr, | ||
| Locale::Es419, | ||
| Locale::Vi, | ||
| ] { | ||
| for (id, eng, _) in checks { | ||
| let msg = tr(locale, *id); | ||
| assert!( | ||
| !msg.eq_ignore_ascii_case(eng), | ||
| "{} leaked exact English '{}' for '{:?}': {msg}", | ||
| locale.tag(), | ||
| eng, | ||
| id | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
The third element of each checks tuple (e.g., "đọc,读,読,读取,ler,leer") is bound to _ and discarded. The test only proves each translation differs from the English baseline (eq_ignore_ascii_case), so a completely incorrect translation — for example, Vietnamese ToolFamilyRead accidentally set to "sai" instead of "đọc" — would pass silently. The third string reads as if it were the expected value to verify, making the intent ambiguous. Consider either dropping the column entirely (it's documentation at best) or asserting against it per locale so the test actually validates correctness.
…MessageId mapping
|
Thanks @gordonlu — we merged #2891 and #2896 from your i18n batch tonight, which moved Since the slices all touch |
2 participants
Summary
Localize tool family verb labels (read, patch, run, find, delegate, fanout, rlm, verify, think, tool) used in tool card headers, sidebar, and footer status lines. 10 new MessageId variants covering all 7 shipped locales.
Changes
Notes
Greptile Summary
Adds localized labels for all 10
ToolFamilyvariants across 7 locales (En, Vi, ZhHant, Ja, ZhHans, PtBr, Es419), surfacing them in footer status lines and activity-detail labels.tool_activity_label_for_namenow accepts aLocaleand routes throughtr();family_labelintentionally stays English-only for card-header glyphs.localization.rsgains 10 newMessageIdvariants with complete translations for every shipped locale, including intentional loan-words (\"fanout\",\"rlm\") where no native term exists.tool_card.rsintroducesfamily_message_id(), restrictstool_display_label_for_nameto#[cfg(test)], and adds two negative i18n tests verifying non-English locales don't echo English strings.footer_ui.rsandui.rsthreadapp.ui_localethrough to allGeneric-branch call-sites.Confidence Score: 5/5
Safe to merge — all production paths correctly thread locale through to tr(), and the translation tables are complete for every shipped locale.
The change is additive: new enum variants, new translation entries, and updated call-sites. Every production call-site now passes locale correctly. The only gap is that three MessageIds (Delegate, Think, Generic) are absent from the no-English-leak test, but this does not affect runtime behaviour.
The two new tests in tool_card.rs are worth a second look: ToolFamilyDelegate, ToolFamilyThink, and ToolFamilyGeneric are missing from the tool_family_labels_localized_no_english_leak coverage loop.
Important Files Changed
Sequence Diagram
sequenceDiagram participant App participant footer_ui participant ui participant tool_card participant localization App->>footer_ui: active_tool_status_label(app) footer_ui->>footer_ui: collect_active_tool_status(cell, snapshot, app.ui_locale) footer_ui->>tool_card: tool_activity_label_for_name(name, locale) tool_card->>tool_card: tool_family_for_name(name) tool_card->>tool_card: family_message_id(family) tool_card->>localization: tr(locale, mid) localization-->>tool_card: "&'static str" tool_card-->>footer_ui: String label App->>ui: activity_cell_label(app, idx, cell) ui->>tool_card: tool_activity_label_for_name(name, app.ui_locale) tool_card->>localization: tr(locale, mid) localization-->>tool_card: "&'static str" tool_card-->>ui: String labelComments Outside Diff (2)
crates/tui/src/tui/widgets/tool_card.rs, line 102-111 (link)pubfunction with#[allow(dead_code)]creates an ambiguous API contracttool_display_label_for_nameispubbut is now only called inside the test module — the production path (tool_activity_label_for_name) was refactored to calltr()directly without delegating to it. Adding#[allow(dead_code)]to a public symbol is unusual; it signals the function has no production callers while still advertising it as part of the module API. A future contributor could call it expecting localized output and get English-only labels. Consider whether the function should bepub(crate)or scoped to#[cfg(test)], or whether it should be removed if it genuinely has no callers outside tests.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
crates/tui/src/tui/footer_ui.rs, line 362-396 (link)localeis threaded in but not used for several tool cell branchescollect_active_tool_statusnow receiveslocaleand correctly passes it totool_activity_label_for_nameforToolCell::Generic, but theExploring("read {…}"),PatchSummary("patch {…}"), andMcp("tool {…}") branches still hardcode the English verb. TheMcpbranch in particular uses the identical"tool {name}"pattern that was just localized for the Generic path. In a non-English locale the footer will show mixed-language labels: localized verbs for generic tools, English verbs for MCP tools and patch/explore cells side-by-side in the same status line. This was pre-existing behaviour, but threadinglocaleall the way in makes the remaining gaps visible. Worth a follow-up issue if out of scope here.Reviews (2): Last reviewed commit: "fixup: make tool_display_label_for_name ..." | Re-trigger Greptile