fix: resolve i18n issues with locale detection and hardcoded strings#58
fix: resolve i18n issues with locale detection and hardcoded strings#58KoheiYamashita merged 7 commits intomainfrom
Conversation
…54) - Fix Android per-app locale not reaching Go backend by using Context.resources.configuration instead of Locale.getDefault() - Extract all user-facing hardcoded strings in Android UI to string resources (app, feature/chat, backend/config modules) - Add Go i18n messages for Telegram, WebSocket, and agent loop commands - Propagate locale from Telegram user language and WebSocket metadata Co-Authored-By: Claude Opus 4.6 <[email protected]>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the internationalization (i18n) capabilities of the application across both its Android frontend and Go backend. It addresses critical issues such as incorrect locale detection on Android, ensuring that per-app language settings are respected. Furthermore, it systematically replaces hardcoded user-facing strings with localized string resources in the Android application and integrates a robust i18n system into the Go backend for command responses and channel messages, providing a more consistent and localized user experience. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 effectively addresses internationalization issues by replacing hardcoded strings with localized resources across both the Android application and the Go backend. The Android-side changes correctly implement per-app locale detection for API requests. The Go backend is updated to handle locales and serve translated responses for commands. My review identifies a few areas for improvement in the Go backend, mainly concerning inconsistencies in command behavior and translations between different interfaces (agent vs. Telegram). Addressing these will enhance the consistency and maintainability of the command handling logic.
| case "models": | ||
| // TODO: Fetch available models dynamically if possible | ||
| return "Available models: glm-4.7, claude-3-5-sonnet, gpt-4o (configured in config.json/env)", true | ||
| return i18n.T(locale, "agent.cmd.list.models"), true |
There was a problem hiding this comment.
The /list models command here returns a hardcoded list of example models, which is inconsistent with the same command in the Telegram channel that shows the currently configured model. The latter behavior is more useful and should be implemented here for consistency. This also removes the need for the agent.cmd.list.models translation key.
| case "models": | |
| // TODO: Fetch available models dynamically if possible | |
| return "Available models: glm-4.7, claude-3-5-sonnet, gpt-4o (configured in config.json/env)", true | |
| return i18n.T(locale, "agent.cmd.list.models"), true | |
| case "models": | |
| return i18n.Tf(locale, "cmd.list.models", al.model), true |
| "cmd.show.model": "Current Model: %s", | ||
| "cmd.show.channel": "Current Channel: telegram", | ||
| "cmd.show.unknown": "Unknown parameter: %s. Try 'model' or 'channel'.", | ||
| "cmd.list.usage": "Usage: /list [models|channels]", | ||
| "cmd.list.models": "Configured Model: %s\n\nTo change models, update config.json", | ||
| "cmd.list.channels": "Enabled Channels:\n- %s", | ||
| "cmd.list.unknown": "Unknown parameter: %s. Try 'models' or 'channels'.", | ||
|
|
||
| // Agent loop commands (/show, /list, /switch) | ||
| // cmd.show.usage and cmd.list.usage are shared with Telegram commands | ||
| "agent.cmd.show.model": "Current model: %s", | ||
| "agent.cmd.show.channel": "Current channel: %s", | ||
| "agent.cmd.show.unknown": "Unknown show target: %s", |
There was a problem hiding this comment.
There are duplicated and inconsistent translation keys for the /show command between the agent and Telegram command handlers. For example, cmd.show.model and agent.cmd.show.model differ only in capitalization, and cmd.show.channel is hardcoded while agent.cmd.show.channel uses a format specifier.
To improve consistency and reduce duplication, these should be unified into a single set of keys (e.g., under the cmd. prefix) and used across both handlers.
For example:
- Unify on
"cmd.show.model": "Current model: %s". - Change
"cmd.show.channel"to"Current channel: %s"and usei18n.Tf(locale, "cmd.show.channel", "telegram")intelegram_commands.go.
This would make the translations more reusable and consistent.
| "cmd.list.channels": "Enabled Channels:\n- %s", | ||
| "cmd.list.unknown": "Unknown parameter: %s. Try 'models' or 'channels'.", | ||
|
|
||
| // Agent loop commands (/show, /list, /switch) | ||
| // cmd.show.usage and cmd.list.usage are shared with Telegram commands | ||
| "agent.cmd.show.model": "Current model: %s", | ||
| "agent.cmd.show.channel": "Current channel: %s", | ||
| "agent.cmd.show.unknown": "Unknown show target: %s", | ||
| "agent.cmd.list.models": "Available models: glm-4.7, claude-3-5-sonnet, gpt-4o (configured in config.json/env)", | ||
| "agent.cmd.list.no_channels": "No channels enabled", | ||
| "agent.cmd.list.channels": "Enabled channels: %s", |
There was a problem hiding this comment.
The translation keys for the /list channels command imply different output formats. cmd.list.channels is designed for a newline-separated list (which is good for chat), while agent.cmd.list.channels is for a simple string, used with comma-separation in agent/loop.go. The capitalization also differs.
For better consistency, consider unifying these. For example, you could have a single key like "cmd.list.channels": "Enabled channels: %s" and apply the specific formatting (like \n- or , ) at the call site.
WebSocket clients may send non-normalized locale values (e.g. "ja-JP", "JA") which need to be normalized before use in i18n lookups. Co-Authored-By: Claude Opus 4.6 <[email protected]>
AssistantConnectionImpl directly instantiates WebSocketClient which now requires a Context parameter for per-app locale detection. Pass Context through from AssistantService to fix the compilation error. Co-Authored-By: Claude Opus 4.6 <[email protected]>
WebSocketClient only needs Context for locale detection via resources.configuration, so applicationContext is sufficient and prevents holding a reference to the Service after onDestroy. Co-Authored-By: Claude Opus 4.6 <[email protected]>
…ffect Use stringResource() in Composable scope and pass pre-resolved strings into LaunchedEffect. Replace config_error format string with config_error_prefix for simple concatenation. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Pre-resolve string resource in Composable scope before using it inside coroutine launched from ActivityResult callback. Co-Authored-By: Claude Opus 4.6 <[email protected]>
…elp tabs - Move locale extraction before rate limit check so error messages can be localized - Add agent.rate_limited / agent.rate_limited_tool i18n keys (en/ja) - Extract hardcoded "Select" contentDescription to string resource (config_select) - Remove trailing tab characters from cmd.help values in en/ja Co-Authored-By: Claude Opus 4.6 <[email protected]>
KoheiYamashita
left a comment
KoheiYamashita
left a comment
There was a problem hiding this comment.
コードレビュー
全体としてしっかりした PR です。ロケール修正のアプローチは正しく、Koin の配線も適切で、en/ja の翻訳も完全です。
良い点
context.resources.configuration.locales[0].languageは Android 13+ のアプリごとの言語設定を正しく反映する正しいアプローチandroidContext()が適切に渡されており、AssistantServiceではapplicationContextを使って Service コンテキストのリークを防止- 全ての新規キーが en/ja 両方に存在し、翻訳も適切
- Go の i18n が
messages_status.go/messages_config.go/messages_agent.goの既存パターンに準拠
修正すべき問題
1. (中) 通知タイトルがハードコードのまま
AssistantService.kt — setContentText は文字列リソースに抽出されたのに、同じ行の setContentTitle("ClawDroid Assistant") がハードコードのまま。ブランド名であっても、この PR の趣旨に合わせて文字列リソースに抽出すべき。
2. (中) WebSocketClient のパラメータ順序
WebSocketClient.kt — context（必須・デフォルトなし）が clientType（省略可能・デフォルト "main"）の後に配置されている。Kotlin の慣例では必須パラメータを省略可能パラメータの前に置くべき。
3. (中) cmd.show.channel が "telegram" をハードコード
messages_channel.go:20 — "cmd.show.channel": "Current Channel: telegram" とハードコードされているが、agent loop 版の agent.cmd.show.channel は正しく %s フォーマットを使用。一貫性のためにパラメータ化すべき。
4. (低) Cancel 文字列キーがモジュール間で重複
config_cancel（backend/config）と action_cancel（app）が同一内容。共通文字列は core/ui モジュールへの移動を検討。
5. (低) 共有 i18n キーの暗黙的な結合
agent loop コマンドが Telegram 名前空間の cmd.show.usage / cmd.list.usage を再利用。コメントで文書化されてはいるが、将来的に分岐が必要になった場合に問題となる可能性あり。
総評
1〜3 はマージ前に修正推奨、4〜5 は任意の改善事項です。
1 participant
📝 Description
Fix multiple i18n issues where Android per-app language settings were not reaching the Go backend, and numerous user-facing strings were hardcoded instead of using string resources / i18n message catalogs.
🗣️ Type of Change
🔗 Linked Issue
Closes #54
📚 Technical Context (Skip for Docs)
Settings > Apps > Language) usescontext.resources.configuration.locales[0], but the code was usingLocale.getDefault()which returns the system-wide locale. This caused WebSocket and Config API requests to send the wrong language to the Go backend.Changes
Phase 1 — Android locale fix:
WebSocketClient.kt/ConfigApiClient.kt: injectContext, usecontext.resources.configuration.locales[0].languageinstead ofLocale.getDefault().languageAppModule.kt,ConfigModule.kt) to passandroidContext()Phase 2 — Android hardcoded string extraction:
appmodule: AssistantService accessibility dialog, notification text, CalendarPickerActivityfeature/chatmodule: VoiceModeOverlay phase labels, MessageInput content descriptions, ChatScreen title/settingsbackend/configmodule (newres/values/strings.xml): ConfigSectionDetailScreen, ConfigSectionListScreenenandjaPhase 3 — Go backend i18n:
pkg/i18n/messages_channel.gowith en/ja translations for Telegram, WebSocket, and agent loop command messagestelegram.go: propagate user locale vialocaleFromMessage()helper, add to metadatatelegram_commands.go: all command responses usei18n.T()/i18n.Tf()websocket.go: "Configuration required" → i18nloop.go: all/show,/list,/switchcommand responses → i18n🧪 Test Environment & Hardware
make checkpasses (fmt/vet/test)☑️ Checklist
🤖 Generated with Claude Code