[MikeShi42]

Summary

The app crashes on load when the browser reports a POSIX-style language such as en-US@posix. Headless Chromium does this on LANG=C.UTF-8 hosts. uPlot calls new Intl.NumberFormat(navigator.language) at import time, and Intl throws on that tag. The latest uPlot release (1.6.32) still does this.

• Patches uPlot (yarn patch) to fall back to en-US when Intl rejects the browser language, with a unit test.
• Pins the Playwright browser locale to en-US so e2e formatting doesn't depend on the host.

How to reproduce

1. On a host with LANG=C.UTF-8, run make e2e.
2. On main, global setup times out waiting for source-selector. The browser console shows RangeError: Invalid language tag: en-US@posix.
3. On this branch, setup completes. It also completes with the Playwright locale pin removed, which exercises the uPlot patch.

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
hyperdx-oss	Ready	Preview	Nov 9, 2026 2:41pm UTC
hyperdx-storybook	Ready	Preview	Nov 9, 2026 2:41pm UTC

[github-actions]

🔵 Tier 2 — Low Risk

Small, isolated change with no API route or data model modifications.

Why this tier:

• Standard feature/fix — introduces new logic or modifies core functionality

Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.
SLA: Resolve within 4 business hours.

Stats

• Production files changed: 3
• Production lines changed: 45 (+ 29 in test files, excluded from tier calculation)
• Branch: claude/e2e-playwright-locale
• Author: MikeShi42

To override this classification, remove the review/tier-2 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

[github-actions]

PR Review

✅ No issues found.

[github-actions]

Duplicate code analysis

No new duplicate code introduced by this PR.

---

jscpd compared this branch against origin/main
across app, api and common-utils. Only new clones are listed; the codebase
currently holds 110 in total (1.21%).
Run yarn dupes locally to reproduce, or yarn dupes:all for the full picture.

[github-actions]

Deep Review

Scope: PR #3595 — patch uPlot's import-time Intl.NumberFormat to fall back to en-US when the browser reports a POSIX-style locale (en-US@posix), plus Playwright locale pins and a unit test. 7 files, ~45 production lines.
Intent: Stop the app crashing on load under POSIX-style navigator.language; make e2e number/date formatting host-independent.

The uPlot patch was verified correct: the try/catch wraps exactly the throwing new Intl.NumberFormat(...) call in both the CJS and ESM dist builds, and en-US is a valid fallback tag. The new Jest test genuinely exercises the patched path under the current jsdom environment. The 4 prior PR comments are all automated bot output (Vercel, triage, jscpd, e2e) with no actionable human feedback.

✅ No critical issues found.

🟡 P2 — recommended

• packages/app/src/components/DBHeatmapChart/__tests__/uplotLocale.test.ts:20 — the sole assertion uPlot.fmtNum(1234.5) === '1,234.5' is identical whether the fallback catch branch fired or uPlot took the happy en-US path, so the test cannot prove the patched branch ran and would keep passing (false confidence) if domEnv were ever false, e.g. the test environment switched to node.
  • Fix: Assert the mocked getter was actually read, e.g. expect(languageSpy).toHaveBeenCalled(), so coverage of the catch branch fails loudly if navigator.language is never consulted.
  • _{testing, correctness}

🔵 P3 nitpicks (4)

• packages/app/src/components/DBHeatmapChart/__tests__/uplotLocale.test.ts:16 — expect(() => new Intl.NumberFormat('en-US@posix')).toThrow(RangeError) couples the test to the host ICU/V8 build; a future Node that canonicalizes rather than rejects the tag would fail this line even though the patch is correct.
  • Fix: Loosen to toThrow() or treat the line as a documented canary rather than a behavioral guarantee.
  • _{testing, correctness, kieran-typescript}
• packages/app/src/components/DBHeatmapChart/__tests__/uplotLocale.test.ts:8 — afterEach calls languageSpy.mockRestore(), but languageSpy is only assigned as the first statement inside it(); if jest.spyOn ever throws, afterEach dereferences undefined and masks the real failure.
  • Fix: Use languageSpy?.mockRestore() or type the variable as jest.SpyInstance | undefined.
  • _{kieran-typescript}
• packages/app/tests/e2e/global-setup-fullstack.ts:166 — the 'en-US' locale literal is now set in two places (playwright.config.ts use.locale and the global-setup browser.newContext) because global setup does not inherit use, so the two can silently drift if the test locale ever changes.
  • Fix: Export a single shared locale constant and import it in both files.
  • _{maintainability}
• packages/app/package.json:103 — the dependency moved from a floating range to a patch protocol pinned to exactly uplot@1.6.32, so any future uplot bump requires regenerating the patch; yarn errors loudly on a version mismatch so the trap is not silent, but the one-line upstream bug is now carried locally indefinitely.
  • Fix: Track upstreaming the fix to uplot and reference the upstream issue from the patch or changeset so future maintainers can drop the local patch.
  • _{maintainability}

---

Reviewers (6): correctness, testing, maintainability, project-standards, kieran-typescript, previous-comments.

Testing gaps:

• No positive-control test proving a valid tag (e.g. de-DE) still flows through to uPlot rather than being collapsed to en-US — i.e. nothing proves the patch did not neuter normal locale handling.
• The ESM patch hunk is not covered by tests (Jest imports the CJS build); only the CJS fallback is exercised.
• The residual crash risk assumes numFormatter is uPlot's only import-time Intl construction from navigator.language; if a future uplot version adds a module-scope Intl.DateTimeFormat(nav.language) the crash could reappear unpatched.

_{No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond}

[github-actions]

E2E Test Results

✅ All tests passed • 466 passed • 1 skipped • 1192s

Status	Count
✅ Passed	466
❌ Failed	0
⚠️ Flaky	0
⏭️ Skipped	1

Tests ran across 4 shards in parallel.

View full report →