Skip to content

Fix language picker localization#1895

Merged
steipete merged 3 commits into
steipete:mainfrom
Zihao-Qi:fix-language-picker-system
Jul 5, 2026
Merged

Fix language picker localization#1895
steipete merged 3 commits into
steipete:mainfrom
Zihao-Qi:fix-language-picker-system

Conversation

@Zihao-Qi

@Zihao-Qi Zihao-Qi commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Keep every language-picker option readable in its own stable/native language, independent of the current UI language.
  • Make System follow macOS language preferences instead of inheriting a stale CodexBar override.
  • Cache per-language bundle lookups used by the picker.
  • Preserve unrelated external AppleLanguages overrides; migrate only the exact legacy override CodexBar previously owned.

Why

The old picker localized language names into the currently selected language. A mistaken selection could therefore make recovery difficult. Explicit app choices were also mirrored into AppleLanguages, so returning to System could retain the previous CodexBar language.

The picker now uses stable native names. CodexBar keeps its explicit choice in appLanguage, and startup removes the old app-domain AppleLanguages value only when it exactly matches that stored choice. System mode and unrelated external overrides remain untouched.

Maintainer improvements

  • Merged current main before validation.
  • Replaced unconditional startup deletion with an ownership-aware migration guard.
  • Added regressions for System mode, matching legacy cleanup, and unrelated external overrides.
  • Used readable Swift Testing sentence names.

Validation

  • swift test --filter 'LocalizationLanguageCatalogTests|PreferencesPaneSmokeTests' — 34 tests passed.
  • make check — 0 format/lint violations.
  • make test — all 47 shards passed.
  • Full-branch autoreview — no actionable findings.
  • Fresh ad-hoc debug bundle, isolated temporary bundle identifier:
    • System mode + external German AppleLanguages retained the override and rendered German menus (Ablage, Bearbeiten, Darstellung, Fenster, Hilfe).
    • Japanese appLanguage + matching legacy AppleLanguages removed the legacy key and rendered Japanese menus (ファイル, 編集, 表示, ウインドウ, ヘルプ).

Screenshots

Before After
Language picker before Language picker after

@clawsweeper

clawsweeper Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge. Reviewed July 5, 2026, 12:04 AM ET / 04:04 UTC.

Summary
The PR makes language picker options render in stable native languages, changes AppleLanguages migration/persistence, caches per-language bundles, and adds localization regression tests.

Reproducibility: yes. Current-main source shows picker labels resolve through the active app language and explicit selections write AppleLanguages, while the inspected screenshots show the before/after picker behavior.

Review metrics: 2 noteworthy metrics.

  • Patch Surface: 4 production files, 2 test files; +208/-44. The diff is focused, but it spans both visible picker UI and persisted language-setting behavior.
  • AppleLanguages Paths: 2 production call sites changed. Startup migration and the app-language setter both change upgrade behavior that CI alone does not settle.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Update remaining visible date formatters affected by AppleLanguages removal to use codexBarLocalizedLocale() for explicit app languages.
  • [P2] Add a focused regression covering explicit app language with a different system locale, then rerun the localization/settings tests.

Mantis proof suggestion
A short native-app proof would help confirm the repaired explicit-language and System-mode behavior after the formatter fix. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: verify explicit Japanese language keeps picker labels readable and user-facing dates localized after AppleLanguages is cleared.

Risk before merge

  • [P1] Merging as-is can produce mixed-language UI for users with an explicit CodexBar language because surrounding strings use L(...) while remaining .current date formatters follow macOS after AppleLanguages is cleared.
  • [P1] The AppleLanguages migration is compatibility-sensitive for existing users, so maintainers should explicitly accept CodexBar's custom localization resolver as the source of truth once visible formatters are covered.

Maintainer options:

  1. Localize remaining formatters before merge (recommended)
    Update the remaining visible date formatting paths affected by AppleLanguages removal to use the selected CodexBar locale and add a focused regression test.
  2. Restore a compatibility fallback
    If process-wide localization remains part of the app-language contract, keep or replace the explicit-language fallback instead of clearing AppleLanguages unconditionally for all visible formatting.
  3. Accept mixed-locale edge cases
    Maintainers can merge as-is only if they deliberately accept that some dates may remain in the macOS language after explicit app-language selection.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Update user-visible DateFormatter/FormattedStyle paths affected by removing AppleLanguages to use codexBarLocalizedLocale() for explicit app languages, add focused tests for an explicit Japanese app language with a different system locale, and leave System mode plus unrelated external AppleLanguages preservation unchanged.

Next step before merge

  • [P2] A narrow automated repair can address the concrete formatter regression on the PR branch without deciding a broader product direction.

Security
Cleared: The diff is limited to Swift localization/settings code and focused tests; no concrete security or supply-chain concern was found.

Review findings

  • [P2] Preserve explicit-language date formatting — Sources/CodexBar/SettingsStore+Defaults.swift:739
Review details

Best possible solution:

Keep the native picker-label approach, but before merge either move remaining user-facing Foundation formatters to codexBarLocalizedLocale() for explicit app languages or preserve an equivalent compatibility path while keeping System mode clean.

Do we have a high-confidence way to reproduce the issue?

Yes. Current-main source shows picker labels resolve through the active app language and explicit selections write AppleLanguages, while the inspected screenshots show the before/after picker behavior.

Is this the best way to solve the issue?

No as submitted. Per-option native labels are the right narrow fix, but removing AppleLanguages before moving remaining visible formatters to codexBarLocalizedLocale() leaves a merge-blocking mixed-locale regression.

Full review comments:

  • [P2] Preserve explicit-language date formatting — Sources/CodexBar/SettingsStore+Defaults.swift:739
    After this setter stops writing AppleLanguages, explicit app-language selection no longer changes Locale.current, but visible formatters still use .current in subscription renewal notes and the About pane build timestamp. That means a user can select Japanese, relaunch, and see Japanese strings with month/day names from macOS instead; move those paths to codexBarLocalizedLocale() or keep an equivalent compatibility fallback. This was visible in an earlier ClawSweeper cycle and I am raising it late after the focused formatter audit.
    Confidence: 0.88
    Late finding: first raised on code an earlier review cycle already covered.

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 68f5f9156a05.

Label changes

Label changes:

  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (screenshot): The inspected before/after screenshots directly show the picker changing from active-language labels to stable native-language labels in the real macOS app UI.
  • remove status: 👀 ready for maintainer look: Current PR status label is status: ⏳ waiting on author.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

  • P2: The PR addresses a real localization recovery problem with limited blast radius, but still needs one compatibility repair before merge.
  • merge-risk: 🚨 compatibility: Changing AppleLanguages persistence can alter existing users' localized date and UI behavior after upgrade.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (screenshot): The inspected before/after screenshots directly show the picker changing from active-language labels to stable native-language labels in the real macOS app UI.
  • proof: sufficient: Contributor real behavior proof is sufficient. The inspected before/after screenshots directly show the picker changing from active-language labels to stable native-language labels in the real macOS app UI.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The inspected before/after screenshots directly show the picker changing from active-language labels to stable native-language labels in the real macOS app UI.
Evidence reviewed

Acceptance criteria:

  • [P1] swift test --filter 'LocalizationLanguageCatalogTests|PreferencesPaneSmokeTests'.
  • [P1] make check.
  • [P1] make test.

What I checked:

Likely related people:

  • steipete: Current-main blame points the affected picker, localization cache, and language persistence code to the v0.39.0 import and later settings work, and this handle also authored the latest PR branch adjustment preserving external overrides. (role: recent area contributor; confidence: high; commits: e437044c32ba, 17347d847b15, 9470a3298866; files: Sources/CodexBar/PreferencesGeneralPane.swift, Sources/CodexBar/SettingsStore+Defaults.swift, Sources/CodexBar/Localization.swift)
  • Yuxin-Qiao: Merged localization work from this handle added and maintained several native language catalogs that the picker now depends on. (role: adjacent localization contributor; confidence: medium; commits: c303b67cca1e, 3b5f5234c9fa, 9d197036cfba; files: Sources/CodexBar/Resources, Tests/CodexBarTests/LocalizationLanguageCatalogTests.swift)
  • Zihao-Qi: Beyond authoring this PR, local main history shows this handle recently changed the settings window area that contains the language picker. (role: recent settings contributor; confidence: medium; commits: 17347d847b15; files: Sources/CodexBar/PreferencesGeneralPane.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (4 earlier review cycles)
  • reviewed 2026-07-04T15:23:58.552Z sha cbc82e6 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-04T15:28:39.466Z sha cbc82e6 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-04T15:33:12.937Z sha cbc82e6 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T03:52:57.682Z sha 9470a32 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 4, 2026
@Zihao-Qi Zihao-Qi marked this pull request as ready for review July 4, 2026 15:25
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 4, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9470a32988

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

UserDefaults.standard.set(stored, forKey: "appLanguage")
}
UserDefaults.standard.set([stored], forKey: "AppleLanguages")
UserDefaults.standard.removeObject(forKey: "AppleLanguages")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve explicit-language date formatting

When the user selects an explicit CodexBar language while macOS remains in another language, this removes the app-level AppleLanguages override without first moving all visible Foundation formatters to codexBarLocalizedLocale(). There are still user-facing dates formatted with .current (for example subscription renewal notes in MenuCardView+ModelHelpers.subscriptionDateString and the About pane build timestamp), so after a relaunch the surrounding strings use the chosen app language but embedded month/day names fall back to the macOS language.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 5, 2026
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Exact head 9470a32988663b222941c501099cc8c3da66380c verified: focused language migration suite (34 tests), make check, full 47-shard make test, isolated live app proof for externally managed German and app-selected Japanese, and all required CI checks green. The maintainer rewrite preserves external AppleLanguages overrides and only removes the exact legacy override owned by CodexBar. Thank you, @Zihao-Qi!

@steipete steipete merged commit ec83fcb into steipete:main Jul 5, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants