Skip to content

Conversation

@briangregoryholmes
Copy link
Contributor

@briangregoryholmes briangregoryholmes commented Jan 15, 2026

Problem

When viewing an infinite time range (inf) without a valid smallest_time_grain, the derived activeTimeGrain can be set to TIME_GRAIN_SECOND. Since TIME_GRAIN_SECOND, TIME_GRAIN_MILLISECOND, and TIME_GRAIN_UNSPECIFIED were missing from the TIME_GRAIN record, this caused runtime failures in UI code that performed lookups (e.g., formatting, pivot chip labels).

Solution

Core fix: Add missing grain entries to TIME_GRAIN record.

Additional cleanup:

  • Replace redundant FromURLParamTimeGrainMap / ToURLParamTimeGrainMapMap with existing DateTimeUnitToV1TimeGrain / V1TimeGrainToDateTimeUnit from new-grains.ts
  • Use effectiveGrain (respects minTimeGrain) instead of raw activeTimeGrain in time series charts
  • Refactor getAdjustedFetchTime to use Luxon directly instead of legacy helper functions
  • Add formatDateTimeByGrain utility for grain-aware datetime formatting

Changes

Core fix

config.ts, types.ts

Add MILLISECOND, SECOND, UNSPECIFIED to TIME_GRAIN record and Period enum

Mapper consolidation

mappers.ts, time-state.ts, TDDHeader.svelte, selectors.ts, + 6 url-state files

Remove duplicate mappers, use V1TimeGrainToDateTimeUnit throughout

Time series

MetricsTimeSeriesCharts.svelte, timeseries-data-store.ts

Use effectiveGrain with isGrainAllowed guard

Utilities

formatter.ts, ranges/index.ts, new-grains.ts

Add formatDateTimeByGrain, simplify getAdjustedFetchTime

Tests

formatter.spec.ts

Add 12 tests for formatDateTimeByGrain


Technical debt

This is a targeted fix. The underlying issue is type fragmentation across TIME_GRAIN, V1TimeGrain, AvailableTimeGrain, Period, and TimeUnit—all of which duplicate Luxon's DateTimeUnit. A TODO comment documents the path forward: consolidate around Luxon utilities per the pattern in new-grains.ts.

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

@briangregoryholmes briangregoryholmes self-assigned this Jan 15, 2026
@briangregoryholmes briangregoryholmes changed the title fix: time grain second fix: TIME_GRAIN lookups for second/millisecond/unspecified grains Jan 15, 2026
@briangregoryholmes briangregoryholmes requested review from AdityaHegde and ericpgreen2 and removed request for ericpgreen2 January 15, 2026 01:26
@briangregoryholmes briangregoryholmes added the Type:Bug Something isn't working label Jan 15, 2026
@ericpgreen2 ericpgreen2 removed their request for review January 15, 2026 15:55
@briangregoryholmes briangregoryholmes merged commit ccf2687 into main Jan 15, 2026
13 checks passed
@briangregoryholmes briangregoryholmes deleted the bgh/fix-time-grain-second-usage branch January 15, 2026 16:31
briangregoryholmes added a commit that referenced this pull request Jan 15, 2026
)

* fix: time grain second

* cleanup

* cleanup

* cleanup

* update formatDateTimeByGrain tests

* add test cases for getAdjustedFetchTime
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch Type:Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants