Repository navigation
Conversation
Eight renderer call sites built Intl.RelativeTimeFormat with an
undefined locale, which resolves to the Chromium/OS locale. With the UI
language set to English on a Korean-locale machine, relative timestamps
('그저께', '5일 전') rendered in Korean while every other label was
English — the Tasks UPDATED column, GitHub/Linear/Jira item views,
account usage rows, and workspace-space labels were all affected.
Relative times are language words, not just date shapes, so they follow
the configured UI language now. A shared locale-keyed cached getter
replaces the per-module formatter constants: it preserves the zero
per-row allocation behavior TaskPage relied on while staying correct
across runtime language switches, which the module-scope constants
could not.
Intentionally unchanged: Intl.DateTimeFormat call sites — numeric
date/time shapes following the OS region is conventional.
📝 WalkthroughWalkthroughAdds cached, locale-aware relative-time helpers with minute/hour/day formatting and invalid-date fallbacks. Renderer timestamp displays across issue, pull request, activity, account, and workspace scan views now delegate to the shared helpers. Tests cover locale selection, formatter caching, runtime language changes, fallback behavior, relative-time phrasing, and deterministic workspace scan-time output. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/components/GitHubItemDialog.tsx (1)
342-359: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winExtract duplicated relative time formatting to a shared utility.
This
formatRelativeTimehelper logic is duplicated across multiple component files (and several others in the codebase context, such asPullRequestPage.tsxandTaskPage.tsx). Consolidate this logic into a single shared utility to adhere to the DRY principle and simplify future maintenance.
src/renderer/src/components/GitHubItemDialog.tsx#L342-L359: Extract this function tosrc/renderer/src/i18n/relative-time-format.ts(or similar utility file) and export it.src/renderer/src/components/JiraIssueWorkspace.tsx#L60-L74: Replace this local implementation by importing the new shared utility.src/renderer/src/components/LinearItemDrawer.tsx#L104-L121: Replace this local implementation by importing the new shared utility.
🧹 Nitpick comments (1)
src/renderer/src/components/status-bar/workspace-space-format.ts (1)
46-56: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStore the formatter instance in a local variable.
getUiRelativeTimeFormatter()is called in every conditional branch. Although the underlying instance is cached, extracting it to a single variable at the top of the logic improves readability and aligns with the implementation pattern in other files.♻️ Proposed refactor
- if (Math.abs(diffMinutes) < 60) { - return getUiRelativeTimeFormatter().format(diffMinutes, 'minute') - } - - const diffHours = Math.round(diffMinutes / 60) - if (Math.abs(diffHours) < 24) { - return getUiRelativeTimeFormatter().format(diffHours, 'hour') - } - - const diffDays = Math.round(diffHours / 24) - return getUiRelativeTimeFormatter().format(diffDays, 'day') + const formatter = getUiRelativeTimeFormatter() + if (Math.abs(diffMinutes) < 60) { + return formatter.format(diffMinutes, 'minute') + } + + const diffHours = Math.round(diffMinutes / 60) + if (Math.abs(diffHours) < 24) { + return formatter.format(diffHours, 'hour') + } + + const diffDays = Math.round(diffHours / 24) + return formatter.format(diffDays, 'day')
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 30921f54-5140-4cfe-b6be-5fdf6f966243
📒 Files selected for processing (12)
src/renderer/src/components/GitHubItemDialog.tsxsrc/renderer/src/components/JiraIssueWorkspace.tsxsrc/renderer/src/components/LinearItemDrawer.tsxsrc/renderer/src/components/PullRequestPage.tsxsrc/renderer/src/components/TaskPage.tsxsrc/renderer/src/components/activity/ActivityPrototypePage.tsxsrc/renderer/src/components/linear-issue-workspace-text.tssrc/renderer/src/components/settings/AccountsPane.tsxsrc/renderer/src/components/status-bar/workspace-space-format.test.tssrc/renderer/src/components/status-bar/workspace-space-format.tssrc/renderer/src/i18n/relative-time-format.test.tssrc/renderer/src/i18n/relative-time-format.ts
Address CodeRabbit review: vi.mock factories are hoisted above const initializers, so the mock object moves into vi.hoisted (matching the repo's existing pattern), and functions that format multiple units now grab the cached formatter once instead of per branch.
…red helpers Eight call sites each carried an identical minute/hour/day branching copy around getUiRelativeTimeFormatter. Centralize that into formatUiRelativeTime (signed ms delta) and formatUiRelativeTimeFromDate (string input + "recently" fallback) so the locale-aware branching lives in one place. AccountsPane's formatMiniMaxRelativeRefresh stays bespoke — it has a translated "just now" branch and a past-clamped, asymmetric rounding convention that the symmetric helper would subtly shift at half-unit boundaries. Net -65 lines; behavior unchanged for the eight consolidated sites.
AmethystLiang
left a comment
There was a problem hiding this comment.
Thank you for this, @moseoh — really clean, well-scoped work. 🙏
The approach is the right one, and I want to be explicit about why since it's a subtle call: Intl.RelativeTimeFormat emits language words ("2 days ago" / "그저께"), so it should follow the configured UI language, while Intl.DateTimeFormat emits numeric date shapes that conventionally track the OS region — leaving those on undefined is correct, not an oversight. This matches the direction #7188 already set (UI language wins over OS language). English labels sitting next to Korean 5일 전 is unambiguously the bug.
I also verified the sweep is genuinely complete: the other relative-time surfaces I found (WorkspaceCleanupDialog, pr-comment-time.ts, ReviewNotesSendMenuContent, automation-page-parts) hardcode English strings like ${hours}h ago rather than constructing Intl.RelativeTimeFormat, so they don't have the OS-locale bug — that's a separate "never localized" gap, out of scope here. Every RelativeTimeFormat(undefined) site was migrated.
The one thing I acted on: the fix left eight near-identical copies of the minute/hour/day branching around getUiRelativeTimeFormatter. I pushed a follow-up commit (9266eb8) that centralizes them into two shared helpers — formatUiRelativeTime(diffMs) and formatUiRelativeTimeFromDate(input, fallback?) — net −65 lines, behavior unchanged, with added unit tests. I deliberately left AccountsPane.formatMiniMaxRelativeRefresh bespoke: it has a translated "just now" branch and a past-clamped, asymmetric rounding convention that the symmetric helper would shift by one at exact half-unit boundaries, so folding it in wasn't behavior-preserving.
Typecheck, oxlint/oxfmt, and the touched suites (relative-time-format, workspace-space-format, task-page-*, AccountsPane, ActivityPrototypePage — 117 tests) all pass locally.
Follow-up worth a separate issue, not this PR: the hardcoded-English Xh ago sites above should eventually go through the same locale-aware path.
Approving. Thanks again for the careful diagnosis and the before/after screenshots.
|
Really enjoying Orca — thanks for building it, and thanks for the detailed review and cleanup! |
|
Thanks for this, and sorry it sat so long. The bug is real: those relative times were following the OS locale instead of the configured UI language.
Closing this in favor of that branch once it merges — the fix and the credit are yours. |
Eight renderer call sites built Intl.RelativeTimeFormat(undefined, ...), which resolves to the OS locale, so relative timestamps rendered in Korean on a Korean-locale machine even with the UI language set to English. Unlike DateTimeFormat, RelativeTimeFormat emits language words, so it must follow the UI language. Rebased onto main: the formatter now resolves through getIntlLocale() (#12105) rather than i18n.resolvedLanguage, so the synthetic plugin<hex> resource language cannot reach the constructor and throw. GitHubItemDialog and PullRequestPage no longer own their formatter — main moved it into work-item-state-presentation, which is converted instead, along with the newly added site there. Co-authored-by: moseoh <azqazq195@gmail.com> Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Co-authored-by: Orca <help@stably.ai>
|
Merged into Closing this in favour of that. Thank you for the fix, and sorry it took as long as it did to get through. |
Summary
With the UI language explicitly set to English on a Korean-locale machine, relative timestamps still render in Korean — the Tasks issue list shows
그저께/5일 전/34일 전in the UPDATED column while every other label is English.Eight renderer call sites built
new Intl.RelativeTimeFormat(undefined, { numeric: 'auto' }); theundefinedlocale resolves to the Chromium/OS locale, not the configured UI language. Affected surfaces: the Tasks UPDATED column, GitHub/Linear/Jira item views, PR page timestamps, account usage rows, workspace-space labels, and the activity page.Unlike
Intl.DateTimeFormat(where following the OS region for numeric date shapes is conventional — those call sites are intentionally unchanged),RelativeTimeFormatemits language words ("2 days ago" / "그저께"), so it must follow the UI language — consistent with the direction already established when #7188 made the configured UI language win over the OS language.The fix replaces the per-module formatter constants with one shared locale-keyed cached getter (
getUiRelativeTimeFormatter). This keeps the zero per-row allocation behaviorTaskPagerelied on, while staying correct across runtime language switches — which module-scope constants could not, since they froze the locale at import time.Screenshots
Testing
pnpm lint/pnpm typecheckpnpm test— new unit tests for the helper (UI-language formatting, cache reuse, runtime language switch, resolvedLanguage fallback) plus all touched-area suites (TaskPage/task-page-cache, AccountsPane, status-bar, activity — 350+ tests) pass; full-run failures are the workstation's pre-existing environment-dependent ones, identical onmainpnpm buildAI Review Report
Intl.RelativeTimeFormatconstruction (9 sites including two found beyond the initially reported one) and confirmed noundefined-locale constructions remain.en/zh/ko/ja/es) and the pseudo-localization locale (en-XA) are valid BCP-47 tags, so the constructor cannot throw; the "System" language resolves to a concrete locale before reaching i18n.workspace-space-format.test.ts); it now pins'en', which also makes it deterministic on non-English machines.Security Audit
No new inputs, IPC, or command execution — the change only threads the app's existing i18n locale string into
Intl.RelativeTimeFormatconstruction.Notes
Intl.DateTimeFormat(undefined, ...)call sites are deliberately out of scope: numeric date/time shapes following the OS region is defensible and changing them is a broader product decision. If maintainers want those unified too, the same locale-keyed getter pattern extends naturally.ELI5
Relative times (“5 days ago”) followed the OS language even when the UI was set to English. They now follow your configured Orca language everywhere those timestamps appear.