feat: extend sandboxes monitoring charts to support up to 90 days#350
feat: extend sandboxes monitoring charts to support up to 90 days#350devin-ai-integration[bot] wants to merge 3 commits into
Conversation
- Add 90d preset to TIME_OPTIONS in time-picker constants - Add 90d entry to TIME_RANGES in timeframe utilities - Increase MAX_DAYS_AGO from 31 to 91 days in constants, use-timeframe hook - Update validation error messages to reflect 91-day limit - Update Peak Concurrent Sandboxes header to show 90-day max - Update header metric query to fetch 90 days of data Co-Authored-By: ben@e2b.dev <ben@e2b.dev>
devin-ai-integration
Bot
requested review from
ben-fornefeld and
drankou
as code owners
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| const { teamSlug } = await params | ||
|
|
||
| const end = Date.now() | ||
| const start = end - (MAX_DAYS_AGO - 60_000) // 1 minute margin to avoid validation errors | ||
| const start = end - (90 * 24 * 60 * 60 * 1000 - 60_000) // 90 days minus 1 minute margin to avoid validation errors | ||
|
|
There was a problem hiding this comment.
🔴 Mock data generator at src/configs/mock-data.ts:1087-1102 still hardcodes a 30-day clamp (const thirtyDaysAgo = now - 30 * 24 * 60 * 60 * 1000; if (startMs < thirtyDaysAgo) startMs = thirtyDaysAgo). This PR's new 90-day query in MaxConcurrentSandboxes (header.tsx:162) and the "Last 90 days" preset now silently truncate to 30 days when USE_MOCK_DATA is on, so the "(90-day max)" card and the 90d chart on the Vercel preview deployments listed in this PR are computed over only the last 30 days. Bump the clamp to match MAX_DAYS_AGO (≥91 days) and update the stale "past 30 days from now" docstring above the function.
Extended reasoning...
What the bug is. generateMockTeamMetrics in src/configs/mock-data.ts:1087-1102 clamps any requested start time forward to no more than 30 days ago:
const now = Date.now()
const thirtyDaysAgo = now - 30 * 24 * 60 * 60 * 1000
// Clamp start time to no earlier than 30 days ago
if (startMs < thirtyDaysAgo) {
startMs = thirtyDaysAgo
}It is exported as both MOCK_TEAM_METRICS_DATA (line 1277) and via MOCK_TEAM_METRICS_MAX_DATA (lines 1282-1311, which calls generateMockTeamMetrics then computes the max). Both get-team-metrics.ts and get-team-metrics-max.ts dispatch to these mock paths when USE_MOCK_DATA is set (src/configs/flags.ts:8-10: VERCEL_ENV !== 'production' && NEXT_PUBLIC_MOCK_DATA === '1').
Code path this PR triggers. header.tsx:162 now computes start = end - (90 * 24 * 60 * 60 * 1000 - 60_000) and calls getTeamMetricsMax({ ..., metric: 'concurrent_sandboxes' }). In mock mode that routes to MOCK_TEAM_METRICS_MAX_DATA(start, end, 'concurrent_sandboxes') → generateMockTeamMetrics(start, end), which silently rewrites startMs to now - 30d. The "max" returned is therefore the max over the last 30 days, but header.tsx:88 labels the card (90-day max). The same issue affects the new 90d preset in TIME_OPTIONS (constants.ts) / TIME_RANGES (timeframe.ts): the 90-day chart paints only the most-recent 30 days.
Why existing code doesn't prevent it. The clamp is silent — no warning is returned and no callers inspect the actual range covered. Pre-PR this was a 1-day gap (MAX_DAYS_AGO was 31d vs the clamp at 30d), so it was effectively invisible. This PR widens MAX_DAYS_AGO to 91 days and adds a 90-day preset, opening a 60-day gap between what the UI promises and what the mock returns. The function docstring at mock-data.ts:1082-1086 ("Can generate data for the past 30 days from now") is now stale and will mislead the next reader.
Impact. Production is unaffected — USE_MOCK_DATA is gated to non-prod by flags.ts:8-10. But the PR explicitly links Vercel preview deployments (web-git-devin-1780334031-90d-timepicker.e2b-preview.dev) for QA, and those are exactly where mock data is on. A reviewer toggling mock mode to verify this PR's headline feature will see a "90-day max" card and a "Last 90 days" chart that quietly only reflect 30 days of data, defeating the purpose of preview verification.
Step-by-step proof. Assume now = Date.now() = T and USE_MOCK_DATA = true.
MaxConcurrentSandboxescomputesend = T,start = T - (90d - 60s)≈T - 90d.- It calls
getTeamMetricsMax({ startDate: T - 90d, endDate: T, metric: 'concurrent_sandboxes' }). - Mock branch invokes
MOCK_TEAM_METRICS_MAX_DATA(T - 90d, T, 'concurrent_sandboxes')→generateMockTeamMetrics(T - 90d, T). - Inside the function:
thirtyDaysAgo = T - 30d. SincestartMs (T - 90d) < thirtyDaysAgo (T - 30d),startMsis reassigned toT - 30d. - Metrics are generated only between
T - 30dandT. The max returned is the max over those 30 days, not 90. - The card renders "(90-day max)" above a value computed from 30 days of mock data. Same logic applies for
MOCK_TEAM_METRICS_DATAwhen a user picks "Last 90 days" in the time picker.
How to fix. Change the clamp to MAX_DAYS_AGO (or at least 91 * 24 * 60 * 60 * 1000) and update the docstring at mock-data.ts:1082-1086 to reflect the new horizon. One-line change, bundles cleanly with this PR.
There was a problem hiding this comment.
Addressed in 8de383e — the mock data generator's clamp was updated from 30 days to 90 days, and the "past 30 days from now" docstring was updated to "past 90 days from now".
| const now = Date.now() | ||
| const startTimestamp = startDateTime.getTime() | ||
|
|
||
| // validate start date is not more than 31 days ago | ||
| // validate start date is not more than 91 days ago | ||
| if (startTimestamp < now - MAX_DAYS_AGO) { | ||
| ctx.addIssue({ | ||
| code: z.ZodIssueCode.custom, | ||
| message: 'Start date cannot be more than 31 days ago', | ||
| message: 'Start date cannot be more than 91 days ago', | ||
| path: ['startDate'], | ||
| }) | ||
| return |
There was a problem hiding this comment.
🟡 The validation error messages now reference "91 days" (Start date cannot be more than 91 days ago / Date range cannot exceed 91 days) but the UI advertises 90 days everywhere — the new "Last 90 days" preset in TIME_OPTIONS/TIME_RANGES and the header card's "(90-day max)" label. A user who selects "Last 90 days" and nudges the start a touch earlier in the custom picker will see an error referencing a 91-day limit the UI never promised. The same drift propagates server-side because get-team-metrics.ts:29, get-team-metrics-max.ts:32, schemas.ts:18,36, and app/api/teams/[teamSlug]/metrics/types.ts:18,35 interpolate MAX_DAYS_AGO / 86_400_000 (= 91) into their error text. Either hardcode "90 days" in user-facing messages and keep MAX_DAYS_AGO as the internal clock-skew buffer, or tighten MAX_DAYS_AGO to 90 days and add a separate small buffer constant (the existing CLOCK_SKEW_TOLERANCE = 60_000 already covers clock skew).
Extended reasoning...
What the bug is
The PR introduces a "Last 90 days" preset and updates the header card to read "(90-day max)" — the user-visible product surface promises 90 days. Internally, however, MAX_DAYS_AGO is set to 91 days (apparently a 1-day safety buffer over the advertised 90), and the validation error strings in validation.ts were rewritten to literally say 91 days:
// src/features/dashboard/sandboxes/monitoring/time-picker/validation.ts:46
message: 'Start date cannot be more than 91 days ago',
// :111
message: 'Date range cannot exceed 91 days',The same 91 leaks into server-side error toasts because several schemas build their messages by dividing MAX_DAYS_AGO by 86_400_000:
src/core/server/functions/sandboxes/get-team-metrics.ts:29and:46src/core/server/functions/sandboxes/get-team-metrics-max.ts:32and:50src/core/modules/sandboxes/schemas.ts:18and:36src/app/api/teams/[teamSlug]/metrics/types.ts:18and:35
All of those now interpolate 91 into user-facing error messages.
How it manifests — step-by-step proof
- User opens the monitoring page and sees the new "(90-day max)" caption on the Peak Concurrent Sandboxes card and "Last 90 days" as the largest preset.
- User clicks "Last 90 days" —
setTimeRange('90d')is called, which computesnow - 90 * 24 * 60 * 60 * 1000and writes the resulting start/end to the URL. - User opens the custom picker to refine the start (say, scrolls it back by an hour). The form's
superRefinechecksstartTimestamp < now - MAX_DAYS_AGO.MAX_DAYS_AGOis now 91 days, so the picker happily accepts up to 91 days back — but if the user moves the start more than 1 day before the original 90-day preset (still well within what "90-day max" suggests should be allowed/rejected boundary), they'll first see no error. - As soon as the user drags slightly past 91 days (or types a date that ends up >91 days ago after combining), they get the toast: "Start date cannot be more than 91 days ago." That number was never advertised. The same happens with the "Date range cannot exceed 91 days" message if the end is also adjusted.
- The server-side validators have the same
91baked into their template-literal error text, so any path that hits the API directly (or any case where client-side validation is bypassed/raced) surfaces "91 days" to the user as well.
Why existing code doesn't prevent it
The PR deliberately left MAX_DAYS_AGO = 91 * 24 * 60 * 60 * 1000 as a 1-day safety buffer (mirrored by the 60_000 ms margin in header.tsx:162 for the same reason). That buffer is sensible internally, but it leaks because (a) the new copy in validation.ts hard-codes the buffered number into the human-readable string, and (b) the server-side schemas compute the user-facing copy directly from MAX_DAYS_AGO. There's no separate "advertised max" constant to drive UX copy independently.
Impact
UX polish, not a functional bug — the picker still works and the data still loads. But a user who reads "(90-day max)" and then sees "91 days" in a validation toast is left wondering which is correct, and the inconsistency surfaces an implementation detail (the clock-skew buffer) as if it were a product limit. It's more visually jarring at 90/91 than at the previous 30/31 because the larger numbers are read more carefully.
How to fix
Two clean options:
- Keep the 91-day internal buffer, fix the copy. Change
validation.ts:46and:111to say'90 days'instead of'91 days', and change the server-side templates inget-team-metrics.ts,get-team-metrics-max.ts,schemas.ts, andmetrics/types.tsto interpolate a separateADVERTISED_MAX_DAYS = 90constant (or just hardcode90) instead ofMAX_DAYS_AGO / 86_400_000. - Tighten
MAX_DAYS_AGOto 90 days and rely on the existingCLOCK_SKEW_TOLERANCE = 60 * 1000(already imported intovalidation.ts) for the buffer. Theheader.tsxquery already subtracts a separate60_000margin, so it would continue to work.
Option 1 is the smaller change and keeps the existing buffer semantics. The pre-existing 30/31 split shows the pattern was already in the codebase, but the larger number makes it more noticeable now.
There was a problem hiding this comment.
Addressed in 8de383e — all user-facing validation error messages now hardcode "90 days" instead of interpolating MAX_DAYS_AGO / 86_400_000 (which yielded 91). This applies to validation.ts, get-team-metrics.ts, get-team-metrics-max.ts, schemas.ts, and types.ts. MAX_DAYS_AGO stays at 91 days as an internal buffer so the "Last 90 days" preset never hits the validation boundary.
… clamp - Hardcode '90 days' in validation error messages instead of interpolating MAX_DAYS_AGO / 86_400_000 (which yields 91, mismatching the UI) - Update mock data generator to clamp at 90 days instead of 30 days - Affects: validation.ts, get-team-metrics.ts, get-team-metrics-max.ts, schemas.ts, types.ts, mock-data.ts Co-Authored-By: ben@e2b.dev <ben@e2b.dev>
|
devin comment on the bot comments please and resolve them if they are addressed |
|
|
||
| // constraints | ||
| export const MAX_DAYS_AGO = 31 * 24 * 60 * 60 * 1000 // 31 days in ms | ||
| export const MAX_DAYS_AGO = 91 * 24 * 60 * 60 * 1000 // 91 days in ms |
There was a problem hiding this comment.
why is this 91 days and not 90?
There was a problem hiding this comment.
It's a 1-day buffer so the "Last 90 days" preset never hits the validation boundary. The preset calculates 90 * 24 * 60 * 60 * 1000 ms from Date.now(), but by the time the request reaches validation, a few seconds/ms have elapsed — if MAX_DAYS_AGO were exactly 90 days, the start timestamp could fail the start >= now - MAX_DAYS_AGO check due to clock drift between preset calculation and validation.
The original code used the same pattern: MAX_DAYS_AGO was 31 days while the largest preset was "Last 30 days".
Happy to change it to exactly 90 days if you'd prefer — the existing CLOCK_SKEW_TOLERANCE = 60_000 could cover that gap instead, though it's currently only used for future-date checks, not the max-age check.
| @@ -32,7 +32,7 @@ export const TeamMetricsRequestSchema = z | |||
| return data.end - data.start <= MAX_DAYS_AGO | |||
| }, | |||
| { | |||
| message: `Date range cannot exceed ${MAX_DAYS_AGO / (1000 * 60 * 60 * 24)} days`, | |||
| message: 'Date range cannot exceed 90 days', | |||
There was a problem hiding this comment.
why did we make this a string literal instead of dynamically inferred from the actual max?
There was a problem hiding this comment.
Because MAX_DAYS_AGO is 91 days (the 1-day buffer mentioned in the other thread), so MAX_DAYS_AGO / 86_400_000 evaluates to 91 — but the user-facing message should say "90 days" to match what the UI advertises. Hardcoding avoids that mismatch.
That said, if you'd prefer to keep it dynamic, the cleaner fix would be to change MAX_DAYS_AGO to exactly 90 days and rely on a separate small tolerance for the buffer. Want me to go that route instead?
There was a problem hiding this comment.
same question for all other removals of the constant that was in use in this pr. we should make sure we don't infer the logic inline but use the correct constant
There was a problem hiding this comment.
Makes sense — I'll change MAX_DAYS_AGO to exactly 90 days, add a separate small buffer for the validation boundary, and restore the dynamic interpolation from the constant in all error messages.
There was a problem hiding this comment.
yes go that route you outlined
There was a problem hiding this comment.
Pushed in 543fbf8 — MAX_DAYS_AGO is now exactly 90 days, with a separate MAX_DAYS_AGO_BUFFER (90d + 60s) used only in the validation checks. All error messages dynamically interpolate from MAX_DAYS_AGO again. CI is green.
- Change MAX_DAYS_AGO to exactly 90 days (was 91) - Add MAX_DAYS_AGO_BUFFER (MAX_DAYS_AGO + 60s) for validation checks - Restore dynamic interpolation from MAX_DAYS_AGO in all error messages - Re-add MAX_DAYS_AGO import in header.tsx instead of inline constant - validation.ts uses MAX_DAYS_AGO_BUFFER for checks to prevent preset race conditions while keeping error messages accurate Co-Authored-By: ben@e2b.dev <ben@e2b.dev>
1 participant
Summary
Extends the sandboxes monitoring view to support up to 90 days of historical data, previously limited to 31 days.
Changes:
90dpreset toTIME_OPTIONS(time-picker popover) andTIME_RANGES(small text button bar)MAX_DAYS_AGOchanged from31 * 24hto exactly90 * 24hintime-picker/constants.tsand theuse-timeframehookMAX_DAYS_AGO_BUFFER(MAX_DAYS_AGO + 60s) used invalidation.tschecks to prevent preset race conditions (time elapsed between preset calculation and validation)MAX_DAYS_AGOconstant (e.g.`Date range cannot exceed ${MAX_DAYS_AGO / 86_400_000} days`)MAX_DAYS_AGOconstantgenerateMockTeamMetricsto clamp at 90 days instead of 30Backend check: Verified the infra API's
ValidateRangehas no max-age restriction, and the ClickHouse TTL was already extended to 90 days in migration20250822155059_extend_team_metrics_ttl.sql.Link to Devin session: https://app.devin.ai/sessions/cdae9cd475134911a0147bdf0e0df424