[SPEC] i18n / zh-CN locale support — product and technical spec#10990
[SPEC] i18n / zh-CN locale support — product and technical spec#10990ZacharyZcR wants to merge 6 commits into
Conversation
github-actions
Bot
added
the
external-contributor
|
I'm starting a first review of this spec-only pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
Reviewed the product and technical specs for adding a custom i18n framework with zh-CN as the first non-English locale. The specs cover the main architecture, fallback model, locale file format, and broad rollout/testing intent.
Concerns
- Locale resolution is internally inconsistent and over-broad: the product and tech specs disagree on whether non-Chinese
WARP_LANGvalues override system locale, and allzh*locales are mapped to Simplified Chinese. - The technical macro example is not type-correct against the declared
Cow<'static, str>API, which would lead to a non-compiling implementation if followed literally. - The validation plan does not yet gate the core locale-file and callsite invariants needed for a 2,700-key migration.
Security
$WARP_LOCALES_DIRintroduces an external locale-file override without specifying its trust boundary, production availability, or integrity/validation requirements.
Verdict
Found: 0 critical, 5 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| 3. If no locale can be determined, the application defaults to English (`en`). | ||
|
|
||
| 1.2. The resolved locale is matched as follows: | ||
| - Any locale starting with `"zh"` (e.g., `zh-CN`, `zh-TW`, `zh-HK`, `zh`) resolves to `zh-CN`. |
There was a problem hiding this comment.
zh* locale, including zh-TW and zh-HK, to Simplified Chinese makes Traditional Chinese users automatically see the wrong script. Limit auto-selection to zh, zh-CN, and zh-Hans*, or fall other Chinese variants back to English until their locale files exist.
|
|
||
| ### 2. Translation rendering | ||
|
|
||
| 2.1. Every user-facing text string in the Warp UI is wrapped in a translation call (`t!()` macro in Rust source). English strings serve as both the translation key and the fallback value — the lookup key is a dot-separated path into a YAML locale file. |
There was a problem hiding this comment.
💡 [SUGGESTION] This says English strings are lookup keys, but the examples use stable dot-separated keys and English strings as values. Rewrite this to distinguish keys from fallback display values so implementers do not key lookups by user-visible English copy.
|
|
||
| 5.2. To change locale, the user sets `WARP_LANG=zh-CN` (or `WARP_LANG=zh`) before launching Warp, and the change takes effect on next launch. | ||
|
|
||
| 5.3. Setting `WARP_LANG` to a non-Chinese value (or unsetting it) restores the system-locale-based default (English for non-Chinese systems). |
There was a problem hiding this comment.
WARP_LANG is the highest-priority override, WARP_LANG=en or another non-Chinese value should force English even on a Chinese system; the tech spec currently falls through to system locale instead. Specify whether non-Chinese overrides are honored or ignored.
| `load_dir` reads all `.yml`/`.yaml` files in a directory, parses them with `serde_yaml`, identifies the locale from the top-level key, and recursively flattens nested YAML into dot-separated keys via `flatten_value`. | ||
|
|
||
| **File discovery order** (non-WASM): | ||
| 1. `$WARP_LOCALES_DIR` env var (if set) |
There was a problem hiding this comment.
$WARP_LOCALES_DIR the first production load path lets any launcher-controlled environment replace strings in security-sensitive UI such as auth, billing, and permission dialogs. Restrict this override to dev/test builds or define trust, validation, and integrity requirements for external locale files.
| // Arm 2: Explicit interpolation | ||
| t!("terminal.hand_off", environment = name) | ||
| // Expands to: i18n::t("terminal.hand_off") | ||
| // .map(|v| i18n::interpolate(&v, &[("environment", format!("{}", name))])) |
There was a problem hiding this comment.
i18n::t() returns Cow<'static, str>, which has no .map() method, so the proposed macro shape would not compile. Specify a type-correct expansion for interpolation and fallback detection.
|
|
||
| ### Unit tests | ||
|
|
||
| - `warp_i18n` should have tests for: |
There was a problem hiding this comment.
PRODUCT.md: identical key sets across en.yml/zh-CN.yml, matching placeholder names, and every callsite key existing in en.yml. Add CI checks for these, otherwise missing or broken translations will silently ship behind the English fallback.
…oundary, locale integrity checks
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
Reviewed the product and technical specs for the i18n / zh-CN locale support proposal. The specs cover the intended architecture and user behavior, but several contradictions and validation gaps need to be resolved before implementation.
Concerns
- The keying model is internally inconsistent: the technical spec says English string values are canonical key identifiers while the API examples use dot-separated YAML paths.
- Locale resolution has conflicting behavior for non-Chinese
WARP_LANGvalues, and the currentzh*prefix match would map Traditional Chinese locales to Simplified Chinese. - Automated locale validation does not cover interpolation placeholder parity, which can silently break dynamic UI strings.
Security
- The locale file discovery trust boundary needs an explicit production enforcement mechanism so dev-only filesystem paths cannot be used by shipped builds to load untrusted YAML into startup parsing and UI rendering.
Verdict
Found: 1 critical, 4 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| 3. If no locale can be determined, the application defaults to English (`en`). | ||
|
|
||
| 1.2. The resolved locale is matched as follows: | ||
| - Any locale starting with `"zh"` (e.g., `zh-CN`, `zh-TW`, `zh-HK`, `zh`) resolves to `zh-CN`. |
There was a problem hiding this comment.
zh-TW and zh-HK to Simplified Chinese by prefix will automatically show the wrong script for Traditional Chinese users. Narrow this to zh-CN/zh-Hans or explicitly justify that product behavior.
|
|
||
| 5.2. To change locale, the user sets `WARP_LANG=zh-CN` (or `WARP_LANG=zh`) before launching Warp, and the change takes effect on next launch. | ||
|
|
||
| 5.3. Setting `WARP_LANG` to a non-Chinese value (or unsetting it) restores the system-locale-based default (English for non-Chinese systems). |
There was a problem hiding this comment.
WARP_LANG is specified to force en, while this says it restores the system-locale default. Clarify whether only unsetting returns to system locale, or whether non-Chinese overrides are ignored.
| `load_dir` reads all `.yml`/`.yaml` files in a directory, parses them with `serde_yaml`, identifies the locale from the top-level key, and recursively flattens nested YAML into dot-separated keys via `flatten_value`. | ||
|
|
||
| **File discovery order** (non-WASM): | ||
| 1. `$WARP_LOCALES_DIR` env var (if set). **Trust boundary:** this path is intended for development and testing only. It is not used in production builds — the bundled resources path (priority 2) is the sole source of locale files in shipped binaries. No integrity validation is performed on locale files loaded from this path. If a file is malformed (e.g., invalid YAML, wrong locale key), it is silently skipped and the next discovery path is tried. |
There was a problem hiding this comment.
$WARP_LOCALES_DIR, $CARGO_MANIFEST_DIR, and $PWD discovery are compiled out or gated from shipped builds; otherwise untrusted locale YAML can be injected into startup parsing and UI text rendering.
|
|
||
| Both files share identical structure — 94 top-level YAML sections corresponding to UI areas: `menu`, `tab`, `workspace`, `auth`, `billing`, `settings`, `ai_settings_page`, `terminal`, `shared_session`, `common`, etc. | ||
|
|
||
| Each string value in `en.yml` serves as the canonical key identifier. The `zh-CN.yml` file has the same keys with Chinese translations. |
There was a problem hiding this comment.
🚨 [CRITICAL] This conflicts with the lookup model above and the callsite examples: string values cannot be canonical key identifiers if code calls t!("common.save"). Define dot-separated YAML paths as keys and en.yml values as fallback text, or update all examples to the intended keying scheme.
| ### Locale file integrity (automated) | ||
| - Every key present in `en.yml` must have a corresponding key in `zh-CN.yml`. A script or build-time check verifies this invariant — missing keys in `zh-CN.yml` should cause a CI failure. | ||
| - Both YAML files must parse successfully as valid YAML and produce the expected top-level locale key (`en:` / `zh-CN:`). | ||
| - No orphaned keys: every key referenced by a `t!()` call in the codebase must exist in `en.yml`. A static analysis script (e.g., `rg 't!\("([^"]+)"' --only-matching | sort -u` diffed against keys extracted from `en.yml`) should be runnable locally and in CI to catch callsite-locale drift. |
There was a problem hiding this comment.
t!() call arguments, so translations cannot silently drop dynamic values or render raw placeholders.
…ompile-time security gate
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
Reviewed the product and technical specs for adding a custom Warp i18n framework with zh-CN as the first non-English locale. The specs cover the main behavior, architecture, locale file format, and validation strategy, but several ambiguities would lead to inconsistent implementation if left unresolved.
Concerns
- The translation key model is internally inconsistent: the product spec says English strings serve as keys while the technical spec uses dot-path YAML keys.
WARP_LANGprecedence and fallback semantics conflict between sections, especially for non-Chinese overrides on Chinese-system machines.- The macro-sharing plan duplicates
t!()across crates without defining how the copies stay aligned.
Security
- The release trust boundary for locale files is not consistently enforced: dev and current-working-directory discovery paths remain listed after the spec says shipped builds should only load bundled resources.
Verdict
Found: 0 critical, 3 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
|
||
| ### 2. Translation rendering | ||
|
|
||
| 2.1. Every user-facing text string in the Warp UI is wrapped in a translation call (`t!()` macro in Rust source). English strings serve as both the translation key and the fallback value — the lookup key is a dot-separated path into a YAML locale file. |
There was a problem hiding this comment.
|
|
||
| 5.2. To change locale, the user sets `WARP_LANG=zh-CN` (or `WARP_LANG=zh`) before launching Warp, and the change takes effect on next launch. | ||
|
|
||
| 5.3. Setting `WARP_LANG` to a non-Chinese value (or unsetting it) restores the system-locale-based default (English for non-Chinese systems). |
There was a problem hiding this comment.
WARP_LANG is described both as forcing en and as restoring system-locale detection; specify whether WARP_LANG=en/fr overrides to English or is ignored.
| 3. `$CARGO_MANIFEST_DIR/../resources/bundled/locales` (and up to 4 ancestor levels) | ||
| 4. `$PWD/resources/bundled/locales` |
There was a problem hiding this comment.
CARGO_MANIFEST_DIR/PWD fallbacks behind debug_assertions too or a shipped app can load locale YAML from repo/CWD-controlled paths.
|
|
||
| The match guard `value if value == key` detects when `t()` returned the key itself (translation missing). In that case, the key string is returned as `Cow::Owned` — interpolation is skipped because there is no template to interpolate into. | ||
|
|
||
| A duplicate `t!()` macro exists in `crates/onboarding/src/lib.rs` so the onboarding binary can use translations without depending on the full `app` crate. |
There was a problem hiding this comment.
💡 [SUGGESTION] Duplicating the macro will let app and onboarding drift; define a shared exported macro or require a test that asserts both expansions stay identical.
…o sync, dev-path compile gate
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This spec defines a custom i18n framework plus zh-CN as the first non-English locale. The structure covers product behavior, implementation architecture, fallback behavior, and validation, but several requirements need tightening before implementation.
Concerns
WARP_LANGbehavior is internally inconsistent for non-Chinese overrides versus system-locale fallback.- The technical spec does not define the runtime/build behavior when no locale bundle can be loaded.
- Validation focuses on key and placeholder parity, but does not require review of security-sensitive translated UI copy.
Security
- Security-sensitive surfaces such as auth, billing, sharing, and agent UI need translation validation requirements so localized text cannot weaken warnings, consent, or permission semantics.
- Debug-only arbitrary locale loading is release-gated, but the spec should still bound file size/shape to avoid poisoned local environments causing excessive startup parsing.
Verdict
Found: 0 critical, 3 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
|
||
| 5.2. To change locale, the user sets `WARP_LANG=zh-CN` (or `WARP_LANG=zh`) before launching Warp, and the change takes effect on next launch. | ||
|
|
||
| 5.3. Setting `WARP_LANG` to a non-Chinese value (or unsetting it) restores the system-locale-based default (English for non-Chinese systems). |
There was a problem hiding this comment.
WARP_LANG is specified to force en, while this says it restores the system-locale default. State separately that non-Chinese WARP_LANG forces English, and only unsetting WARP_LANG restores system-locale detection.
| return t; | ||
| } | ||
| } | ||
| // On WASM, use include_str!() instead of filesystem access |
There was a problem hiding this comment.
| ## Testing and validation | ||
|
|
||
| ### Locale file integrity (automated) | ||
| - Every key present in `en.yml` must have a corresponding key in `zh-CN.yml`. A script or build-time check verifies this invariant — missing keys in `zh-CN.yml` should cause a CI failure. |
There was a problem hiding this comment.
| | 3 | `$CARGO_MANIFEST_DIR/../resources/bundled/locales` (and up to 4 ancestor levels) | `#[cfg(debug_assertions)]` | | ||
| | 4 | `$PWD/resources/bundled/locales` | `#[cfg(debug_assertions)]` | | ||
|
|
||
| **Security:** Paths 1, 3, and 4 are compiled out of release binaries. This prevents shipped builds from loading arbitrary YAML from environment variables or the current working directory into the startup parsing and UI rendering pipeline. If a file loaded from a dev-only path is malformed (e.g., invalid YAML, wrong locale key), it is silently skipped and the next discovery path is tried — the application does not crash. |
There was a problem hiding this comment.
💡 [SUGGESTION] [SECURITY] Dev builds still load arbitrary locale YAML from env/PWD-controlled paths. Add size, file-count, and nesting limits so a poisoned local environment cannot cause unbounded startup parsing or memory use.
…ity copy review, debug file size cap
|
/oz-review |
|
This is your last |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This spec defines the initial i18n framework and zh-CN rollout with product behavior, implementation architecture, locale-file layout, and validation strategy. The scope is appropriate for a spec-only PR, but several requirements need tightening before implementation.
Concerns
- Locale resolution is internally inconsistent: the product examples make non-Chinese
WARP_LANGan explicit English override, while the technical pipeline would fall through to the system locale. - The degraded no-locale mode would render raw keys even for auth, billing, sharing, and agent-consent UI, which can leave security-sensitive prompts unreadable.
- Interpolation explicitly has no escaping or trust-boundary rule, which is unsafe if interpolated values ever enter formatted/link-capable UI.
Security
- Require meaningful English fallback text, embedded fallback data, or fail-closed behavior for security-sensitive prompts when locale files cannot load.
- Define how interpolated dynamic values are escaped or constrained before rendering in any rich/formatted UI contexts.
- Ensure the manual security-sensitive translation checklist includes the exact agent-mode consent and data-handling key prefixes.
Verdict
Found: 0 critical, 4 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| ``` | ||
|
|
||
| **Locale resolution pipeline** (`init_locale`): | ||
| 1. Check `WARP_LANG` env var → if set and starts with `"zh"`, use `zh-CN`. |
There was a problem hiding this comment.
WARP_LANG=fr should classify to en without checking the system locale, but this pipeline would fall through and select zh-CN on a Chinese OS. Specify whether non-zh WARP_LANG is an explicit English override or is ignored.
|
|
||
| **Interpolation** (`interpolate`): | ||
| - Simple `str::replace` of `{name}` placeholders with provided values. | ||
| - Not a template engine — no escaping, no positional arguments, no format specifiers. |
There was a problem hiding this comment.
|
|
||
| **Security:** Paths 1, 3, and 4 are compiled out of release binaries. This prevents shipped builds from loading arbitrary YAML from environment variables or the current working directory into the startup parsing and UI rendering pipeline. In debug builds, locale files loaded from dev-only paths are subject to a size cap (8 MB per file) to prevent intentionally large or malformed YAML from causing excessive startup parsing in poisoned local environments. If a file loaded from a dev-only path exceeds the cap or is malformed, it is silently skipped and the next discovery path is tried — the application does not crash. | ||
|
|
||
| **No-locale fallback:** If no locale file can be loaded from any discovery path (e.g., corrupt installation, missing resource directory), `init_locale()` still completes successfully. The translation map remains empty, and every `t!()` call returns its raw key string as the rendered text. The application starts and functions normally — menu items, buttons, and labels display their dot-path keys (e.g., `"menu.file"`, `"common.save"`) instead of English text. This degraded mode ensures the application never fails to launch due to missing locale data. |
There was a problem hiding this comment.
| ### Security-sensitive translation review (manual) | ||
| - Strings in security-relevant UI surfaces must be manually reviewed by a maintainer before merge. These surfaces include: auth dialogs (sign-in, sign-up, permission prompts), billing pages (pricing, plan descriptions, credit consumption warnings), sharing/permission controls (access grant text, visibility labels), and agent-mode consent prompts (data-handling disclosures, handoff confirmations). | ||
| - The reviewer verifies that Chinesetranslations preserve the legal and behavioral intent of the original English text — warnings are not weakened, consent semantics are not altered, and permission descriptions remain accurate. | ||
| - A checklist of security-sensitive keys (identified by YAML section prefix: `auth.*`, `billing.*`, `teams.*`, `shared_session.*`, `hoa_onboarding.*`) is maintained alongside the locale files for targeted review. |
There was a problem hiding this comment.
…ove fallthrough ambiguity
1 participant
Description
Product and technical specifications for adding i18n support to Warp with Simplified Chinese (zh-CN) as the first non-English locale. This PR contains only the spec documents (no code).
Closes #1194
Documents
specs/gh-1194/PRODUCT.md— User-facing behavior spec: locale detection, translation rendering, interpolation, fallback strategy, locale file format, and platform consistency requirements. Written as numbered, testable invariants.specs/gh-1194/TECH.md— Implementation plan: customwarp_i18ncrate architecture,t!()macro design, YAML loading pipeline, locale resolution logic, usage patterns in the codebase (~4,700 callsites, 2,732 keys), and testing strategy.What this covers
The i18n framework is a custom, lightweight system:
resources/bundled/locales/OnceLockWARP_LANGenv var → system locale → English fallback{key}interpolationCHANGELOG-NEW-FEATURE: Add i18n framework with Simplified Chinese (zh-CN) locale support (spec)