Issue 17774 - Usage - Local - Show data from midnight to midnight of selected dates for browser time zone (AI assisted)#19357
Conversation
| Type.Union([Type.Literal("utc"), Type.Literal("gateway"), Type.Literal("specific")]), | ||
| ), | ||
| /** UTC offset to use when mode is `specific` (for example, UTC-4 or UTC+5:30). */ | ||
| utcOffset: Type.Optional(Type.String({ pattern: "^UTC[+-]\\d{1,2}(?::[0-5]\\d)?$" })), |
There was a problem hiding this comment.
Schema pattern is more permissive than the parser
The utcOffset pattern ^UTC[+-]\\d{1,2}(?::[0-5]\\d)?$ accepts UTC+14:30 (14 hours, 30 minutes), which passes schema validation for sessions.usage. However, parseUtcOffsetToMinutes in usage.ts explicitly rejects this value via if (hours === 14 && minutes !== 0) and silently falls back to UTC mode instead.
A client sending { mode: "specific", utcOffset: "UTC+14:30" } would receive no error but actually get UTC-scoped results. Tightening the regex to disallow :30 when hours equal 14 would surface this as a validation error rather than a silent fallback:
| utcOffset: Type.Optional(Type.String({ pattern: "^UTC[+-]\\d{1,2}(?::[0-5]\\d)?$" })), | |
| utcOffset: Type.Optional(Type.String({ pattern: "^UTC[+-](?:1[0-3]|\\d)(?::[0-5]\\d)?$|^UTC[+-]14(?::00)?$" })), |
Alternatively, a simpler approach: since this is an edge case (UTC+14:30 is not a real timezone), this can be left as-is if silent fallback is acceptable.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/protocol/schema/sessions.ts
Line: 122:122
Comment:
**Schema pattern is more permissive than the parser**
The `utcOffset` pattern `^UTC[+-]\\d{1,2}(?::[0-5]\\d)?$` accepts `UTC+14:30` (14 hours, 30 minutes), which passes schema validation for `sessions.usage`. However, `parseUtcOffsetToMinutes` in `usage.ts` explicitly rejects this value via `if (hours === 14 && minutes !== 0)` and silently falls back to UTC mode instead.
A client sending `{ mode: "specific", utcOffset: "UTC+14:30" }` would receive no error but actually get UTC-scoped results. Tightening the regex to disallow `:30` when hours equal 14 would surface this as a validation error rather than a silent fallback:
```suggestion
utcOffset: Type.Optional(Type.String({ pattern: "^UTC[+-](?:1[0-3]|\\d)(?::[0-5]\\d)?$|^UTC[+-]14(?::00)?$" })),
```
Alternatively, a simpler approach: since this is an edge case (UTC+14:30 is not a real timezone), this can be left as-is if silent fallback is acceptable.
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (1)
Since the entire point of this PR is to make the "Local" / "UTC" selection meaningful, switching between them should trigger a reload: Prompt To Fix With AIThis is a comment left during a code review.
Path: ui/src/ui/app-render-usage-tab.ts
Line: 74:76
Comment:
**Timezone change does not reload data**
`onTimeZoneChange` updates `state.usageTimeZone` but does not call `loadUsage`. This PR makes `usageTimeZone` actually affect the API request (choosing between `mode: "specific"` vs `mode: "utc"`), so switching the timezone dropdown will leave the displayed data stale — using the old timezone interpretation — until the user manually clicks Refresh or changes a date.
Since the entire point of this PR is to make the "Local" / "UTC" selection meaningful, switching between them should trigger a reload:
```suggestion
onTimeZoneChange: (zone) => {
state.usageTimeZone = zone;
void loadUsage(state);
},
```
How can I resolve this? If you propose a fix, please make it concise. |
What no? That is not the entire point of this PR. The point of this PR is to make the UI + gateway capable of selecting a full day in a local timezone, at all (it can't now). The existing UI functionality of not loading anything until Refresh is clicked is a completely orthognal issue that can be changed or not changed. |
1aa5c50 to
cc47a45
Compare
1b609b7 to
afc1068
Compare
Co-authored-by: Tak Hoffman <[email protected]>
a672de2 to
61426f4
Compare
|
PR #19357 - Issue 17774 - Usage - Local - Show data from midnight to midnight of selected dates for browser time zone (AI assisted) (#19357) Merged via squash.
Thanks @huntharo! |
|
Thank you @Takhoffman ! |
…selected dates for browser time zone (AI assisted) (openclaw#19357) thanks @huntharo Verified: - pnpm install --frozen-lockfile - pnpm build - pnpm check - pnpm test:macmini (override approved by Tak for this run; local baseline failures outside PR scope) Co-authored-by: huntharo <[email protected]> Co-authored-by: Tak Hoffman <[email protected]>
…selected dates for browser time zone (AI assisted) (openclaw#19357) thanks @huntharo Verified: - pnpm install --frozen-lockfile - pnpm build - pnpm check - pnpm test:macmini (override approved by Tak for this run; local baseline failures outside PR scope) Co-authored-by: huntharo <[email protected]> Co-authored-by: Tak Hoffman <[email protected]>
…selected dates for browser time zone (AI assisted) (openclaw#19357) thanks @huntharo Verified: - pnpm install --frozen-lockfile - pnpm build - pnpm check - pnpm test:macmini (override approved by Tak for this run; local baseline failures outside PR scope) Co-authored-by: huntharo <[email protected]> Co-authored-by: Tak Hoffman <[email protected]>
…selected dates for browser time zone (AI assisted) (openclaw#19357) thanks @huntharo Verified: - pnpm install --frozen-lockfile - pnpm build - pnpm check - pnpm test:macmini (override approved by Tak for this run; local baseline failures outside PR scope) Co-authored-by: huntharo <[email protected]> Co-authored-by: Tak Hoffman <[email protected]>
…selected dates for browser time zone (AI assisted) (openclaw#19357) thanks @huntharo Verified: - pnpm install --frozen-lockfile - pnpm build - pnpm check - pnpm test:macmini (override approved by Tak for this run; local baseline failures outside PR scope) Co-authored-by: huntharo <[email protected]> Co-authored-by: Tak Hoffman <[email protected]>
Summary
Describe the problem and fix in 2–5 bullets:
Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
Security Impact (required)
NoNoNo(new params on Usage gateway websocket request - I don't think this is what is meant?)NoNoYes, explain risk + mitigation:Repro + Verification
Environment
Steps
Expected
Actual
Evidence
Attach at least one:
After - Modified UI + Modified Gateway - Changes to correct data for local date
After - Modified UI + Unmodified Gateway - Preserves current behavior / no errors / backwards compatible
After - Unmodified UI + Modified Gateway - Preserves current behavior / no errors / params not passed
Before - Only Today Selected with Local
Before - Today + Tomorrow Selected with Local
Human Verification (required)
What you personally verified (not just CI), and how:
Compatibility / Migration
YesNoNoFailure Recovery (if this breaks)
Risks and Mitigations
List only real risks for this PR. Add/remove entries as needed. If none, write
None.NoneGreptile Summary
This PR fixes a real user-facing bug where the Usage tab would stop showing current-day data after midnight UTC for users in time zones that trail the UTC date change (e.g., NYC after 7 PM, San Francisco after 4 PM). The fix adds
modeandutcOffsetrequest parameters to thesessions.usageandusage.costgateway endpoints, allowing the server to interpret date boundaries in the client's local timezone rather than always using UTC midnight.Key changes:
sessions.ts,usage.ts): New optionalmode(utc/gateway/specific) andutcOffsetfields on theSessionsUsageParamsSchema. Helper functionsparseUtcOffsetToMinutes,resolveDateInterpretation,getTodayStartMshandle timezone-aware date math. Backward compatible — missing params default to UTC behavior.usage.ts):UsageStategains ausageTimeZone: "local" | "utc"field.loadUsagenow callsbuildDateInterpretationParamsand spreadsmode/utcOffsetinto both API calls.toErrorMessageimproves error serialization.One logic bug: the
onTimeZoneChangehandler updatesstate.usageTimeZonebut does not callloadUsage, so the displayed data remains stale until the user manually refreshes. Since this PR makes the timezone selection functionally meaningful for the first time, a reload should be triggered on change.One minor style issue: the
utcOffsetJSON Schema regex permitsUTC+14:30, which passes validation but is silently rejected byparseUtcOffsetToMinutesand falls back to UTC mode without any error.Confidence Score: 3/5
loadUsagecall, so the UI will display stale data with the old timezone interpretation until the user manually refreshes. This directly undermines the intended UX of the fix. There is also a minor schema/parser inconsistency forUTC+14:30(non-existent timezone), but that is a low-risk edge case. Score reflects that the fix works correctly once a load is triggered, but the missing automatic reload on timezone change is a real behavioral issue.ui/src/ui/app-render-usage-tab.ts—onTimeZoneChangeshould callloadUsageafter updating state.Last reviewed commit: 3944740
(2/5) Greptile learns from your feedback when you react with thumbs up/down!