Skip to content

fix(app): pin locale for formatMinutes so unit (linux) isn't runner-dependent - #237

Open
alltomatos wants to merge 3 commits into
devfrom
fix/stats-format-minutes-locale
Open

alltomatos wants to merge 3 commits into
devfrom
fix/stats-format-minutes-locale

Conversation

@alltomatos

Copy link
Copy Markdown
Owner

Issue for this PR

Closes #236

Type of change

  • Bug fix

What does this PR do?

unit (linux) is currently red on dev itself (confirmed on the latest few dev commits, e.g. https://github.com/alltomatos/opencode/actions/runs/34911995346/job/104226783370), which blocks CI on every open PR regardless of what it touches — including #234.

Root cause: formatMinutes() in packages/app/src/pages/stats/stats-controller.ts calls mins.toLocaleString() with no locale argument, so its output silently depends on the process's default locale. Every other numeric/date formatter on the stats page (numFmt, compactFmt, the session date/time formatters) already threads through language.intl() explicitly — formatMinutes was the one outlier. Its unit test hardcoded the pt-BR-style output ("60.327 min", period as thousands separator) that this fork's default locale produces locally, but GitHub's ubuntu-latest runners default to a locale that renders it as "60,327 min".

Fix:

  • formatMinutes(ms, locale?) now accepts and forwards an explicit locale.
  • stats.tsx's three call sites now pass language.intl(), matching the rest of the file.
  • The unit test now pins "pt-BR" explicitly instead of relying on the process's default locale, so it's deterministic on any runner.

How did you verify your code works?

  • bun test --conditions=solid --preload ./happydom.ts src/pages/stats/stats-controller.test.ts → 4 pass, 0 fail (was 1 fail before the fix, reproduced locally by not pinning a locale).
  • tsgo -b on packages/app shows no new errors from this change (pre-existing errors are all unrelated ghostty-web module-resolution failures caused by a sandbox proxy limitation in this environment, not this diff).
  • oxlint on the 3 changed files: 0 errors, pre-existing warnings only.

Screenshots / recordings

N/A — output string format only (now correctly locale-aware instead of relying on the runtime default).

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

🤖 Generated with Claude Code

https://claude.ai/code/session_015KrFrdVEmx7apnofWUViJw


Generated by Claude Code

alltomatos and others added 3 commits September 14, 2026 23:19
…pendent

formatMinutes() called mins.toLocaleString() with no locale, so its
output silently depended on the process's default locale. Every other
formatter on the stats page (numFmt, compactFmt, the session date/time
formatters) already threads through language.intl() explicitly --
formatMinutes was the one holdout.

Its unit test hardcoded the pt-BR-style output ("60.327 min", period as
thousands separator) that this fork's default locale produces locally,
but GitHub's ubuntu-latest runners default to a locale that renders it
as "60,327 min" -- so unit (linux) fails on every PR regardless of what
it touches (confirmed failing on dev's own latest commits too, e.g.
https://github.com/alltomatos/opencode/actions/runs/34911995346).

Now formatMinutes accepts an explicit locale (passed as language.intl()
from stats.tsx, matching the rest of the file), and the test pins
"pt-BR" so it's deterministic on any runner.

Closes #236

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015KrFrdVEmx7apnofWUViJw
…placeholder text)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015KrFrdVEmx7apnofWUViJw
…der text)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015KrFrdVEmx7apnofWUViJw

Copy link
Copy Markdown
Owner Author

Confirmed this fix works: @opencode-ai/app#test passes on this PR's own CI. What surfaced next was a second, unrelated pre-existing failure on dev@opencode-ai/core#test > test/database-migration.test.ts ("declared schema has no ungenerated migrations"), caused by commit 5372b2ed3f never running the migration generator after adding a schema index. Filed a separate fix: #240. Not blocking this PR's own correctness, just noting why unit (linux) may still show red until #240 also merges.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

e2e (linux) and e2e (windows) are also failing here, but it's the same pre-existing dev regression already tracked in #238 (legacy-new-session.spec.ts:9 and session-timeline-projection.spec.ts:131, reproduced identically on dev's own head and on unrelated PRs) — not caused by this PR's locale fix.


Generated by Claude Code

alltomatos added a commit that referenced this pull request Sep 15, 2026
…ependent

`unit (linux)` was red on this PR's head, but not because of anything
this PR touches: formatMinutes() called mins.toLocaleString() with no
explicit locale, so its output silently depended on the process's
default locale. This fork's local/dev environment produces
"60.327 min" (period as thousands separator); GitHub's ubuntu-latest
runner default locale produces "60,327 min" instead, failing the
hardcoded unit test.

This is the same root cause already diagnosed and fixed on dev directly
by PR #237 (fix/stats-format-minutes-locale, closes #236) — ported
verbatim here since #237 hasn't merged yet, so this PR's CI doesn't
have to wait on it. Will no-op once dev carries #237.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SHQKcGH7ayRe8UHJ9CAPQJ
alltomatos added a commit that referenced this pull request Sep 15, 2026
…ependent

Ports PR #237 into this PR so its own unit(linux) check goes green now
instead of waiting on #237 to merge — this is the other base-branch-wide
failure on dev (see standing-down comment on this PR), not caused by
this PR's own changes. No-ops once dev carries #237.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SHQKcGH7ayRe8UHJ9CAPQJ
alltomatos added a commit that referenced this pull request Sep 15, 2026
…ependent

Ported from #237 (not yet merged to dev) to unblock this PR's own CI —
same base-branch-wide failure documented on #241. No-ops once dev carries #237.
alltomatos added a commit that referenced this pull request Sep 15, 2026
Ported from #237. Companion to the stats-controller.ts change.
alltomatos added a commit that referenced this pull request Sep 15, 2026
Ported from #237. Companion to the stats-controller.ts change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(app): formatMinutes ignores app locale, breaking CI on non-pt-BR runners

1 participant