Skip to content

Fix shortcut recorder app-language mismatch - #3190

Merged
steipete merged 2 commits into
steipete:mainfrom
endless7:fix/shortcut-recorder-app-language
Aug 25, 2026
Merged

Fix shortcut recorder app-language mismatch#3190
steipete merged 2 commits into
steipete:mainfrom
endless7:fix/shortcut-recorder-app-language

Conversation

@endless7

Copy link
Copy Markdown
Contributor

Summary

  • make the keyboard shortcut recorder follow CodexBar's selected app language instead of the macOS system language
  • keep both the idle and active recording prompts synchronized with the app language
  • add recorder prompt translations for all 23 supported app languages and cover English/Simplified Chinese transitions

Why

When CodexBar is explicitly set to English on a Mac using Simplified Chinese, the General settings pane mixes English app labels with the recorder's Chinese placeholder. The recorder comes from KeyboardShortcuts, whose resource bundle follows the system locale independently of CodexBar's in-app language override.

The wrapper now reapplies CodexBar-localized placeholders during the AppKit editing lifecycle without changing AppleLanguages or the dependency.

Validation

  • swift test --filter KeyboardShortcutsBundleTests — 3 tests passed
  • make check — locale checks, SwiftFormat, and SwiftLint passed with 0 violations
  • make test — 930 selections in 78 groups; 78 first-pass successes, 0 failures, 0 retries, 0 timeouts

@clawsweeper

clawsweeper Bot commented Aug 25, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. 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 Aug 25, 2026
@clawsweeper

clawsweeper Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed August 25, 2026, 9:26 AM ET / 13:26 UTC.

ClawSweeper review

What this changes

The PR makes the menu-shortcut recorder’s idle and recording prompts follow CodexBar’s selected in-app language, with 23 locale updates and focused coverage.

Merge readiness

Blocked until real behavior proof from a real setup is added - 3 items remain

Keep open: this is a narrow, source-aligned fix with no blocking correctness defect found, but the external PR needs real after-fix macOS UI proof before merge.

Likely related people: @steipete (high confidence) and @Yuxin-Qiao (medium confidence).

Priority: P3
Reviewed head: 1b0bb7ff553ee286aee0bd02712b679b9009018b

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The patch is focused and source-consistent, but its real behavior proof is currently limited to tests.
Proof confidence 🧂 unranked krab (1/6) Needs real behavior proof before merge: The reported tests are useful supplemental evidence, but no after-fix run of the macOS settings UI is attached; add a redacted screenshot or recording with the selected app language differing from macOS, and request @clawsweeper re-review if the body update is not picked up. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The reported tests are useful supplemental evidence, but no after-fix run of the macOS settings UI is attached; add a redacted screenshot or recording with the selected app language differing from macOS, and request @clawsweeper re-review if the body update is not picked up. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 5 items Current-main baseline: Current main creates the dependency recorder and only assigns its shortcut name; it does not replace the dependency-localized placeholder text.
PR implementation: The branch attaches a coordinator on recorder creation and SwiftUI updates, then applies CodexBar-localized idle or recording text in response to the AppKit editing lifecycle.
Locale coverage: All 23 checked-in app locale files contain both new recorder prompt keys.
Findings None None.
Security None None.

Live Verification

Command: ./Scripts/compile_and_run.sh

Result: FAIL (failed) — execution before step 1 run: sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.23.0.tgz

sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.23.0.tgz

Assertions:

  • FAIL expect_output: OK: CodexBar is running.

How this fits together

CodexBar’s General settings embeds a KeyboardShortcuts AppKit recorder for the global menu shortcut. CodexBar’s own localization override drives surrounding settings text, while the dependency otherwise follows macOS’s system locale.

flowchart LR
A[Selected app language] --> B[CodexBar localization]
B --> C[General settings]
D[System-locale shortcut recorder] --> C
C --> E[Prompt coordinator]
E --> F[Localized idle and recording prompts]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The reported tests are useful supplemental evidence, but no after-fix run of the macOS settings UI is attached; add a redacted screenshot or recording with the selected app language differing from macOS, and request @clawsweeper re-review if the body update is not picked up. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Resolve merge risk (P1) - The new test manually posts editing notifications, so it does not prove the real KeyboardShortcuts editing lifecycle and notification ordering in the macOS settings UI.
  • Complete next step (P2) - No mechanical repair is identified; the remaining merge gate is contributor-supplied real UI evidence.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Localization surface 23 locale files updated; 2 prompt keys each The fix covers every checked-in app language rather than falling back to the dependency’s system-locale text.
Production versus test delta production +97/-1, tests +32 Most production growth is the required locale coverage; the behavioral change is confined to one settings wrapper.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Retain the narrow coordinator approach and add redacted after-fix proof showing CodexBar set to a language different from macOS before merge.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Retain the narrow coordinator approach and add redacted after-fix proof showing CodexBar set to a language different from macOS before merge.

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

Yes—source reproducibly shows main leaving the dependency recorder’s system-localized placeholder intact while CodexBar independently resolves its selected app language; the reported mismatch follows when those languages differ.

Is this the best way to solve the issue?

Yes—the coordinator changes only the two dependency prompt strings at the existing AppKit lifecycle boundary, avoiding a global language override or dependency fork.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • add P3: This is a focused settings-localization fix with limited user impact and no evidence of broader failure.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The reported tests are useful supplemental evidence, but no after-fix run of the macOS settings UI is attached; add a redacted screenshot or recording with the selected app language differing from macOS, and request @clawsweeper re-review if the body update is not picked up. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P3: This is a focused settings-localization fix with limited user impact and no evidence of broader failure.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The reported tests are useful supplemental evidence, but no after-fix run of the macOS settings UI is attached; add a redacted screenshot or recording with the selected app language differing from macOS, and request @clawsweeper re-review if the body update is not picked up. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

Likely related people:

  • steipete: History ties the General settings redesign and selected-language behavior to this area. (role: settings-surface author and recent area contributor; confidence: high; commits: 9b13a7bae72f, a01bf8c9751c; files: Sources/CodexBar/PreferencesComponents.swift, Sources/CodexBar/PreferencesView.swift, Sources/CodexBar/Localization.swift)
  • Yuxin-Qiao: Recent localization changes and localization-bundle caching work are directly adjacent to the app-language resolution used by this patch. (role: recent localization contributor; confidence: medium; commits: deb5efdc0d72, 1bd85494c6d6; files: Sources/CodexBar/Localization.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Add redacted after-fix settings evidence showing both idle and active recorder prompts under an app-language/system-language mismatch.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

Co-authored-by: JiaJian <33014143+endless7@users.noreply.github.com>

@steipete steipete left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Reviewed all supported locale resources and verified localized recorder prompts survive dependency writes during both recording lifecycle transitions.

@clawsweeper

clawsweeper Bot commented Aug 25, 2026

Copy link
Copy Markdown

ClawSweeper status: review started.

I am starting a fresh review of this pull request: Fix shortcut recorder app-language mismatch This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

@steipete
steipete merged commit 54e2714 into steipete:main Aug 25, 2026
9 checks passed
@steipete

Copy link
Copy Markdown
Owner

Landed after verifying all 23 app languages, lifecycle regressions for dependency prompt overwrites, zero lint violations, all 930 local test selections with zero retries or timeouts, and successful exact-head workflow 32883623677.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants