Repository navigation
fix(platform): localize activity log counts - #4404
Conversation
|
TALE-180 / TALE-359 fleet handoff: implementation ready for nonauthor review. PR: #4404 ActivityLogView now supplies the existing active locale to every formatNumber call: four summary cards and the category/top-actor BreakdownRow. The shared helper API remains unchanged. Verification: 19 targeted component tests passed (activity view + importing logs page, one worker); regression failed against main; scoped source/test types reported zero diagnostics; type-aware oxlint, oxfmt, diff check and platform manual register gate passed. Chromium rendered DE 1.310, FR 1 310 (U+202F) and EN 1,310 across all surfaces with real catalogs and design controls. Query boundary alone used controlled data. Baseline/fixed visual-aspect-analyzer scored 100/100 with 16 detected elements, zero defects. The standalone fixture reserves its root and waits for fonts to avoid fixture startup artifacts. Manual automation register anchor compared with 75 open PR heads using git merge-tree: 73 change that register; 22 have pre-existing register conflicts against main; this branch introduces zero new register conflicts. GitHub Actions checks are queued, consistent with the dispatch's incident 3q1yb5m7ltvb. No CI reruns requested. The local SAST wrapper skipped because its pinned scanner was not cached; CI Opengrep remains pending. Independent nonauthor review and green CI remain open. Workspace task reporting unavailable: workspace_status returned not_granted. This comment is the authorized fallback for the TALE-180/TALE-359 handoff; no task was changed directly. |
yannickmonney
left a comment
There was a problem hiding this comment.
Independent exact-head review: #4404 at 349fd2ba4367a2928d6bbad6090a00bc0ac889be
Verdict: ACCEPT. I found no blocking findings. CI is pending: Actions is stalled (incident 3q1yb5m7ltvb), and I did not rerun anything.
Reviewer: agent #3 (Claude), task TALE-934. The author is Codex #8 (run ea8f9f05), so this review is independent. I re-read the head right before posting, and it is unchanged. The merge base is dd1cb498.
Checks
-
Every count follows the active locale. All three surfaces now format their counts with
localefromuseLocale()(@tale/ui/i18n/locale-provider, the provider the app shell mounts):- the four summary cards (
totalActions,successCount,failureCount,deniedCount,activity-log-view.tsx:196–208); - the category breakdown and the top actors, which both render through
BreakdownRow(:70, also used by the loading skeleton rows).
These are all 5
formatNumbercalls in the view, and none of them still lacks a locale. - the four summary cards (
-
Rendering. Node 24.21.0 with ICU 78.3 gives de
1.310, fr1 310(U+202F, asIntlrenders it) and en1,310. The test matches those exact strings with an identity normalizer. I checked the bytes: the FR fixtures do contain U+202F. -
The shared helper is unchanged. There is no diff from the merge base to the head in
services/platform/lib/utils/format/orpackages/ui/src/i18n/, andmainhasn't touchednumber.tssince the merge base. The signature is stillformatNumber(value, locale = defaultLocale, options?). -
The register anchor doesn't collide (
git merge-tree, run against all 79 other open PR heads): -
Tests (one worker,
/opt/node/bin/node, existing dependencies hard-linked; Vite didn't refuse any asset, so no fs config was needed):activity-log-view.test.tsxplus its importeraudit-logs-page.test.tsxpass 19/19 in 52 s. -
Controls (each with the test kept at the head):
Variant New locale test View at the merge base fails at 5.240localedropped fromBreakdownRowonlyfails, 2 of 4 matches dropped from the total card only fails at 5.240dropped from the success card only fails at 2.620dropped from the failure card only fails, 3 of 4 matches dropped from the denied card only fails, 3 of 4 matches head restored 4/4 pass, clean tree The test therefore catches a missing locale at each of the five calls.
-
Lint, format and types.
oxlint 1.79.0 --type-aware --type-checkon the view and the test is clean (39 s).oxfmt 0.64.0 --checkis clean;.mdfiles are outside oxfmt's targets.
Non-blocking note: whichever of #4404 and #4384 lands second needs a small resolution
#4384 (c7079740, "show activity summary read failures") edits the same view and test.
- The view merges cleanly, and all 5 locale arguments survive the merge.
- The test file has an append/append conflict, which does not occur against
main.
I did a trial merge, review-only and aborted afterwards, and resolved it by keeping both blocks: close the count-locales describe before #4384's top-level it.
- At runtime, 5/5 tests pass.
- The type-aware check reports TS2739 at #4404's fixture (
activity-log-view.test.tsx:134): under #4384's hoisted type,read.currentis missingisErrorandrefetch. - Adding
isError: false, refetch: vi.fn()to that fixture makes the check clean.
Observations (not findings)
useLocale().localeis the provider's tag as detected. A region tag such asde-CHpassesisValidLocaleand renders1'310, which is whatIntlproduces for that tag.- Other analytics views still call
formatNumberwithout a locale (automation-summary-cards.tsx,chat-health-metrics-page.tsx,external-turns-metrics-page.tsx, …). They are outside #3635's scope; they could be a follow-up.
CI
The 21:45Z snapshot, which I read once and did not rerun:
- Workflows: 7 created at 21:20:59Z. Six are queued and SAST is in progress.
- Check runs: 5 success, 12 skipped, 1 in progress, 19 queued.
- Combined status: pending.
I did not push, merge, rerun anything or change any status. The logs, mutation-check.sh and the merge-tree table are in the TALE-934 delivery box.
349fd2b to
97613d3
Compare
6c54819 to
d44c891
Compare
d44c891 to
b303641
Compare
b303641 to
f1acf58
Compare
f1acf58 to
c866f46
Compare
Activity log counts used the number helper's English default even after the app language changed. Pass the active locale from the existing locale context to all four summary cards and the shared category/top-actor row, keeping the helper API unchanged.
Add a mounted-language-switch regression using real EN/DE/FR catalogs: German
1.310, French1 310(U+202F) and English1,310. Preserve period, empty/loading filter and parent logs-page controls, and register the automated coverage.Verified locally (LIGHT):
Closes #3635
TALE-180. Independent nonauthor review is still required. GitHub Actions was reported stalled under incident 3q1yb5m7ltvb; no CI reruns will be requested.
Current-main rebase
Replayed the previously accepted source
15cf5c050cba2c787446c33a100f87cfee60f8econto maind1373d84cd56972501403f62145ec52e6f65d44a, including the merged shared CI repair in #4625. The accepted feature payload and all current-main changes are preserved in one atomic commit. Configured commit and conflict checks pass; earlier behavioral proof remains recorded above. All seven native required checks and full merge-group validation remain required for this new source.Maintenance replay: preserves the accepted feature payload on main fd277c4, including merged #4649, #4650 and #4655. Retains the exact independently accepted one-line shared CLI inventory repair from #4654 (252f0df), pending native merge on main. The #4282 task-register union, where applicable, retains the accepted feature row and current-main rows. Existing behavioral evidence remains recorded above; no fresh full-feature/full-workspace or hosted-green claim. All seven native required contexts and full merge-group validation remain mandatory.