Skip to content

fix(metrics): make minute/second date ranges resolve — and roll from now - #68081

Merged
DanielVisca merged 1 commit into
masterfrom
posthog-code/metrics-validation-subday-dates
Jul 3, 2026
Merged

fix(metrics): make minute/second date ranges resolve — and roll from now#68081
DanielVisca merged 1 commit into
masterfrom
posthog-code/metrics-validation-subday-dates

Conversation

@DanielVisca

Copy link
Copy Markdown
Contributor

Found by: metrics push-model dogfood validation (first-time-user pass over the new SDK-push pipeline; reported symptom: viewer shows "No data" for last 5/30 minutes while storage has fresh rows).

Problem

Picking Last 5 minutes or Last 30 minutes in the metrics viewer renders "No data" even when the metric has samples seconds old (verified: 5,070 rows in the window in ClickHouse while the UI showed nothing). Two stacked defects in lib/utils/dateFilters:

  1. isStringDateRegex accepts [hdwmqy] only, but dateOptionsMap defines M (minute) and s (second). The viewer's minute options emit -5M/-30M → regex miss → dateStringToDayJs returns null → the viewer's resolveDate falls back to dayjs('-30M') (invalid) → the loader silently returns [].
  2. Even parsing wouldn't have made them correct: dateStringToDayJs anchors every relative date at startOf('day'), so -30M would mean today 00:00 − 30 min = yesterday 23:30 — a ~24h window labeled "Last 30 minutes". This also means the working -1h option has been querying since yesterday 23:00 while labeled "Last 1 hour".

Changes

  • Regex unit class gains M and s (both already in dateOptionsMap and componentsToDayJs — the regex was the only gap).
  • Sub-day units (hour/minute/second) anchor at now (rolling window), matching the backend's relative_date_parse; calendar units (d/w/m/q/y) keep startOf('day') anchoring so insights behavior is untouched.

How did you test this code?

  • New frozen-time test locks both behaviors (-30M/-5M/-1h/-45s resolve to exact rolling instants). Red without the fix (1 failed / 39 passed), green with it (40/40) — the 39 pre-existing cases pin calendar-unit anchoring unchanged.
  • Empirical cross-check: the same window that renders "No data" in the viewer returns 21 per-minute points via query-metrics — confirming data presence and isolating the bug to date resolution.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

  • Part of the commissioned metrics-validation dogfood (DRI @DanielVisca): act as a first-time user of the push-model metrics product, chase every discrepancy to root cause, ship observed fixes.
  • Note for reviewers outside metrics: this touches the shared date helper; the deliberate scope decision is that only sub-day units change semantics (previously either broken or nonsensically day-anchored), calendar units are pinned by existing tests.

The metrics viewer's 'Last 5 minutes' / 'Last 30 minutes' options rendered
'No data' while the API returned rows for the same window. Two defects in the
shared relative-date helper:

- isStringDateRegex only accepted [hdwmqy], but dateOptionsMap promises
  M (minute) and s (second). '-30M' failed the regex, resolveDate returned
  null, and the viewer's loader silently returned [] — 'No data'.
- dateStringToDayJs anchored every relative date at startOf(day), so even a
  parsing '-30M' would mean 'today 00:00 minus 30 min' = yesterday 23:30.
  '-1h' has been silently showing 'since yesterday 23:00' labeled as 'Last
  1 hour'. Sub-day units now roll from now, matching the backend's
  relative_date_parse; calendar units keep their day anchoring.

New test freezes time and locks both behaviors; it fails without the fix
(minute/second strings resolve to null) and the 39 pre-existing cases pin
calendar-unit behavior unchanged.

Generated-By: PostHog Code
@DanielVisca DanielVisca self-assigned this Jul 2, 2026
@DanielVisca DanielVisca added the stamphog Request AI approval (no full review) label Jul 2, 2026

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diff itself is low-risk (regex/anchor fix with a matching test), but greptile-apps[bot] shows an in-progress review (👀) on this PR — should wait for that to finish before auto-approving.

@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Jul 2, 2026
@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "fix(metrics): make minute/second date ra..." | Re-trigger Greptile

@DanielVisca
DanielVisca enabled auto-merge (squash) July 2, 2026 23:13
@DanielVisca DanielVisca added the stamphog Request AI approval (no full review) label Jul 2, 2026

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes a real bug in shared date-parsing logic used across many scenes (date filters, app metrics, billing, audit logs) — correct-looking and tested, but no human/agent reviewer has approved or commented on this diff, only weak bot reactions.

@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Jul 2, 2026
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Bundle size — 🔺 +138 B (+0.0%)

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 81.55 MiB · 🔺 +138 B (+0.0%)

No file changed by more than 1000 B.

Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report

Eager graph — within budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
3.76 MiB · 695 files 🔺 +68 B (+0.0%) █████████░ 87.7% of 4.29 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
11.52 MiB · 2,937 files 🔺 +68 B (+0.0%) █████████░ 89.5% of 12.87 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx

Largest files eagerly shipped from src/index.tsx
Size File
275.2 KiB ../node_modules/.pnpm/posthog-js@1.396.5/node_modules/posthog-js/dist/rrweb.js
266.9 KiB ../node_modules/.pnpm/@posthog+icons@0.37.4_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
224.5 KiB src/taxonomy/core-filter-definitions-by-group.json
211.3 KiB ../node_modules/.pnpm/posthog-js@1.396.5/node_modules/posthog-js/dist/module.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
105.9 KiB src/lib/api.ts
69.7 KiB src/products.tsx
61.7 KiB src/lib/lemon-ui/icons/icons.tsx
57.2 KiB src/lib/utils/eventUsageLogic.ts
39.6 KiB src/lib/KeaDevTools.tsx
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
458.3 KiB ../node_modules/.pnpm/@posthog+brand@0.3.0_react@18.3.1/node_modules/@posthog/brand/dist/generated/hoggies/svg/driving-hogzilla.mjs
302.9 KiB ../node_modules/.pnpm/@posthog+brand@0.3.0_react@18.3.1/node_modules/@posthog/brand/dist/generated/hoggies/svg/coffee-run.mjs
275.2 KiB ../node_modules/.pnpm/posthog-js@1.396.5/node_modules/posthog-js/dist/rrweb.js
266.9 KiB ../node_modules/.pnpm/@posthog+icons@0.37.4_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
253.2 KiB ../node_modules/.pnpm/@posthog+brand@0.3.0_react@18.3.1/node_modules/@posthog/brand/dist/generated/hoggies/svg/chart-hog.mjs
239.2 KiB ../node_modules/.pnpm/@posthog+brand@0.3.0_react@18.3.1/node_modules/@posthog/brand/dist/generated/hoggies/svg/x-ray.mjs
224.5 KiB src/taxonomy/core-filter-definitions-by-group.json
211.6 KiB ../node_modules/.pnpm/@posthog+brand@0.3.0_react@18.3.1/node_modules/@posthog/brand/dist/generated/hoggies/svg/money.mjs
211.3 KiB ../node_modules/.pnpm/posthog-js@1.396.5/node_modules/posthog-js/dist/module.js
185.9 KiB ../node_modules/.pnpm/@posthog+brand@0.3.0_react@18.3.1/node_modules/@posthog/brand/dist/generated/hoggies/svg/hogpatch.mjs

Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479

Dist folder size — 🟢 -365.99 MiB (-20.0%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 1464.55 MiB · 🟢 -365.99 MiB (-20.0%)

@DanielVisca DanielVisca added the stamphog Request AI approval (no full review) label Jul 2, 2026

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes shared date-parsing logic (dateStringToDayJs) used across the app for relative date ranges; behavior change is plausible and well-tested, but no independent reviewer has approved the current head — only bot reactions, no substantive review.

@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Jul 2, 2026
@DanielVisca
DanielVisca merged commit c02d422 into master Jul 3, 2026
423 of 492 checks passed
@DanielVisca
DanielVisca deleted the posthog-code/metrics-validation-subday-dates branch July 3, 2026 00:01
@deployment-status-posthog

deployment-status-posthog Bot commented Jul 3, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-07-03 00:31 UTC Run
prod-us ✅ Deployed 2026-07-03 00:43 UTC Run
prod-eu ✅ Deployed 2026-07-03 00:44 UTC Run

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.

2 participants