From 56e8fb2bb87ce4020bd2677b265a91e0a826ed9a Mon Sep 17 00:00:00 2001
From: Dhiraj Barnwal <sumankumaribarnwal@gmail.com>
Date: Tue, 5 Nov 2024 11:28:01 -0800
Subject: [PATCH] fix: account for DST changes while adjusting offsets (#6034)

---
 .../timestamp-profile/TimestampBound.svelte   |  8 ++++++-
 .../timestamp-profile/TimestampDetail.svelte  | 19 ++++++++++++---
 .../TimestampMouseoverAnnotation.svelte       | 10 ++++++--
 .../pivot/pivot-column-definition.ts          | 11 +++++++--
 .../charts/TDDAlternateChart.svelte           | 12 +++++-----
 .../MetricsTimeSeriesCharts.svelte            |  5 +++-
 .../features/dashboards/time-series/utils.ts  | 23 ++++++++++++++----
 web-common/src/lib/convertTimestampPreview.ts |  6 ++++-
 web-common/src/lib/time/grains/index.ts       | 23 +++++++++++++++++-
 web-common/src/lib/time/timezone/index.ts     | 24 ++++++++++++++++---
 10 files changed, 117 insertions(+), 24 deletions(-)

diff --git a/web-common/src/components/data-graphic/compositions/timestamp-profile/TimestampBound.svelte b/web-common/src/components/data-graphic/compositions/timestamp-profile/TimestampBound.svelte
index ac793d5e182..03657e76dec 100644
--- a/web-common/src/components/data-graphic/compositions/timestamp-profile/TimestampBound.svelte
+++ b/web-common/src/components/data-graphic/compositions/timestamp-profile/TimestampBound.svelte
@@ -20,16 +20,22 @@
     datePortion,
     timePortion,
   } from "@rilldata/web-common/lib/formatters";
+  import { timeGrainToDuration } from "@rilldata/web-common/lib/time/grains";
   import { removeLocalTimezoneOffset } from "@rilldata/web-common/lib/time/timezone";
+  import type { V1TimeGrain } from "@rilldata/web-common/runtime-client";
 
   export let value: Date;
+  export let grain: V1TimeGrain;
   export let label = "value";
   export let align: "left" | "right" = "left";
 
   let valueWithoutOffset: Date | undefined;
 
   $: if (value instanceof Date)
-    valueWithoutOffset = removeLocalTimezoneOffset(value);
+    valueWithoutOffset = removeLocalTimezoneOffset(
+      value,
+      timeGrainToDuration(grain),
+    );
 </script>
 
 <Tooltip
diff --git a/web-common/src/components/data-graphic/compositions/timestamp-profile/TimestampDetail.svelte b/web-common/src/components/data-graphic/compositions/timestamp-profile/TimestampDetail.svelte
index 2d2b0cf0c24..d6decf004cb 100644
--- a/web-common/src/components/data-graphic/compositions/timestamp-profile/TimestampDetail.svelte
+++ b/web-common/src/components/data-graphic/compositions/timestamp-profile/TimestampDetail.svelte
@@ -14,8 +14,10 @@
    * a smoothed series (showing the trend) if the time series merits it.
    */
   import Tooltip from "@rilldata/web-common/components/tooltip/Tooltip.svelte";
+  import { copyToClipboard } from "@rilldata/web-common/lib/actions/copy-to-clipboard";
   import { modified } from "@rilldata/web-common/lib/actions/modified-click";
   import { guidGenerator } from "@rilldata/web-common/lib/guid";
+  import { timeGrainToDuration } from "@rilldata/web-common/lib/time/grains";
   import { removeLocalTimezoneOffset } from "@rilldata/web-common/lib/time/timezone";
   import type { V1TimeGrain } from "@rilldata/web-common/runtime-client";
   import { bisector, extent, max, min } from "d3-array";
@@ -38,7 +40,6 @@
   import TimestampProfileSummary from "./TimestampProfileSummary.svelte";
   import TimestampTooltipContent from "./TimestampTooltipContent.svelte";
   import ZoomWindow from "./ZoomWindow.svelte";
-  import { copyToClipboard } from "@rilldata/web-common/lib/actions/copy-to-clipboard";
 
   const id = guidGenerator();
 
@@ -291,6 +292,7 @@
   function shiftClick() {
     const exportedValue = `TIMESTAMP '${removeLocalTimezoneOffset(
       nearestPoint[xAccessor],
+      timeGrainToDuration(rollupTimeGrain),
     ).toISOString()}'`;
     copyToClipboard(exportedValue);
   }
@@ -422,6 +424,7 @@
       {#if $coordinates.x}
         <TimestampMouseoverAnnotation
           point={nearestPoint}
+          grain={rollupTimeGrain}
           {xAccessor}
           {yAccessor}
         />
@@ -496,8 +499,18 @@
 
   <!-- Bottom time horizon labels -->
   <div class="select-none grid grid-cols-2 space-between">
-    <TimestampBound align="left" value={zoomMinBound} label="Min" />
-    <TimestampBound align="right" value={zoomMaxBound} label="Max" />
+    <TimestampBound
+      grain={rollupTimeGrain}
+      align="left"
+      value={zoomMinBound}
+      label="Min"
+    />
+    <TimestampBound
+      grain={rollupTimeGrain}
+      align="right"
+      value={zoomMaxBound}
+      label="Max"
+    />
   </div>
 </div>
 
diff --git a/web-common/src/components/data-graphic/compositions/timestamp-profile/TimestampMouseoverAnnotation.svelte b/web-common/src/components/data-graphic/compositions/timestamp-profile/TimestampMouseoverAnnotation.svelte
index 909bd4f9b25..635a185e1d8 100644
--- a/web-common/src/components/data-graphic/compositions/timestamp-profile/TimestampMouseoverAnnotation.svelte
+++ b/web-common/src/components/data-graphic/compositions/timestamp-profile/TimestampMouseoverAnnotation.svelte
@@ -4,8 +4,10 @@
     formatInteger,
     timePortion,
   } from "@rilldata/web-common/lib/formatters";
-  import type { ScaleLinear } from "d3-scale";
+  import { timeGrainToDuration } from "@rilldata/web-common/lib/time/grains";
   import { removeLocalTimezoneOffset } from "@rilldata/web-common/lib/time/timezone";
+  import type { V1TimeGrain } from "@rilldata/web-common/runtime-client";
+  import type { ScaleLinear } from "d3-scale";
   import { getContext } from "svelte";
   import type { Writable } from "svelte/store";
   import { fly } from "svelte/transition";
@@ -25,7 +27,11 @@
   export let point;
   export let xAccessor: string;
   export let yAccessor: string;
-  $: xLabel = removeLocalTimezoneOffset(point[xAccessor]);
+  export let grain: V1TimeGrain;
+  $: xLabel = removeLocalTimezoneOffset(
+    point[xAccessor],
+    timeGrainToDuration(grain),
+  );
 </script>
 
 <g>
diff --git a/web-common/src/features/dashboards/pivot/pivot-column-definition.ts b/web-common/src/features/dashboards/pivot/pivot-column-definition.ts
index a4cb6d55f2d..3cbe90cdc1c 100644
--- a/web-common/src/features/dashboards/pivot/pivot-column-definition.ts
+++ b/web-common/src/features/dashboards/pivot/pivot-column-definition.ts
@@ -2,6 +2,7 @@ import PercentageChange from "@rilldata/web-common/components/data-types/Percent
 import { createMeasureValueFormatter } from "@rilldata/web-common/lib/number-formatting/format-measure-value";
 import { formatMeasurePercentageDifference } from "@rilldata/web-common/lib/number-formatting/percentage-formatter";
 import { TIME_GRAIN } from "@rilldata/web-common/lib/time/config";
+import { timeGrainToDuration } from "@rilldata/web-common/lib/time/grains";
 import {
   addZoneOffset,
   removeLocalTimezoneOffset,
@@ -91,7 +92,10 @@ function createColumnDefinitionForDimensions(
         ) {
           const timeGrain = getTimeGrainFromDimension(dimensionNames?.[level]);
           const dt = addZoneOffset(
-            removeLocalTimezoneOffset(new Date(value)),
+            removeLocalTimezoneOffset(
+              new Date(value),
+              timeGrainToDuration(timeGrain),
+            ),
             timeConfig?.timeZone,
           );
           const timeFormatter = timeFormat(
@@ -150,7 +154,10 @@ function formatRowDimensionValue(
     if (value === "Total") return "Total";
     const timeGrain = getTimeGrainFromDimension(dimension);
     const dt = addZoneOffset(
-      removeLocalTimezoneOffset(new Date(value)),
+      removeLocalTimezoneOffset(
+        new Date(value),
+        timeGrainToDuration(timeGrain),
+      ),
       timeConfig?.timeZone,
     );
     const timeFormatter = timeFormat(
diff --git a/web-common/src/features/dashboards/time-dimension-details/charts/TDDAlternateChart.svelte b/web-common/src/features/dashboards/time-dimension-details/charts/TDDAlternateChart.svelte
index 33b5c8d83db..58871060c83 100644
--- a/web-common/src/features/dashboards/time-dimension-details/charts/TDDAlternateChart.svelte
+++ b/web-common/src/features/dashboards/time-dimension-details/charts/TDDAlternateChart.svelte
@@ -1,5 +1,11 @@
 <script lang="ts">
   import VegaLiteRenderer from "@rilldata/web-common/features/canvas-components/render/VegaLiteRenderer.svelte";
+  import VegaRenderer from "@rilldata/web-common/features/canvas-components/render/VegaRenderer.svelte";
+  import {
+    resolveSignalField,
+    resolveSignalIntervalField,
+    resolveSignalTimeField,
+  } from "@rilldata/web-common/features/canvas-components/render/vega-signals";
   import { getStateManagers } from "@rilldata/web-common/features/dashboards/state-managers/state-managers";
   import { tableInteractionStore } from "@rilldata/web-common/features/dashboards/time-dimension-details/time-dimension-data-store";
   import type { DimensionDataItem } from "@rilldata/web-common/features/dashboards/time-series/multiple-dimension-queries";
@@ -22,12 +28,6 @@
     updateChartOnTableCellHover,
   } from "./utils";
   import { VegaSignalManager } from "./vega-signal-manager";
-  import VegaRenderer from "@rilldata/web-common/features/canvas-components/render/VegaRenderer.svelte";
-  import {
-    resolveSignalField,
-    resolveSignalTimeField,
-    resolveSignalIntervalField,
-  } from "@rilldata/web-common/features/canvas-components/render/vega-signals";
 
   export let totalsData: TimeSeriesDatum[];
   export let dimensionData: DimensionDataItem[];
diff --git a/web-common/src/features/dashboards/time-series/MetricsTimeSeriesCharts.svelte b/web-common/src/features/dashboards/time-series/MetricsTimeSeriesCharts.svelte
index 6801757e39f..632f1588e40 100644
--- a/web-common/src/features/dashboards/time-series/MetricsTimeSeriesCharts.svelte
+++ b/web-common/src/features/dashboards/time-series/MetricsTimeSeriesCharts.svelte
@@ -28,6 +28,7 @@
   } from "@rilldata/web-common/features/dashboards/time-series/timeseries-data-store";
   import { EntityStatus } from "@rilldata/web-common/features/entity-management/types";
   import { adjustOffsetForZone } from "@rilldata/web-common/lib/convertTimestampPreview";
+  import { timeGrainToDuration } from "@rilldata/web-common/lib/time/grains";
   import { getAdjustedChartTime } from "@rilldata/web-common/lib/time/ranges";
   import {
     TimeRangePreset,
@@ -144,15 +145,17 @@
   $: dimensionData = dimensionDataCopy;
 
   // FIXME: move this logic to a function + write tests.
-  $: if ($timeControlsStore.ready) {
+  $: if ($timeControlsStore.ready && interval) {
     // adjust scrub values for Javascript's timezone changes
     scrubStart = adjustOffsetForZone(
       $exploreStore?.selectedScrubRange?.start,
       $exploreStore.selectedTimezone,
+      timeGrainToDuration(interval),
     );
     scrubEnd = adjustOffsetForZone(
       $exploreStore?.selectedScrubRange?.end,
       $exploreStore.selectedTimezone,
+      timeGrainToDuration(interval),
     );
 
     const slicedData = isAllTime
diff --git a/web-common/src/features/dashboards/time-series/utils.ts b/web-common/src/features/dashboards/time-series/utils.ts
index 66ff31c59d8..dd2368c37e9 100644
--- a/web-common/src/features/dashboards/time-series/utils.ts
+++ b/web-common/src/features/dashboards/time-series/utils.ts
@@ -35,14 +35,23 @@ export function niceMeasureExtents(
   ];
 }
 
-export function toComparisonKeys(d, offsetDuration: string, zone: string) {
+export function toComparisonKeys(
+  d,
+  offsetDuration: string,
+  zone: string,
+  grainDuration: string,
+) {
   return Object.keys(d).reduce((acc, key) => {
     if (key === "records") {
       Object.entries(d.records).forEach(([key, value]) => {
         acc[`comparison.${key}`] = value;
       });
     } else if (`comparison.${key}` === "comparison.ts") {
-      acc[`comparison.${key}`] = adjustOffsetForZone(d[key], zone);
+      acc[`comparison.${key}`] = adjustOffsetForZone(
+        d[key],
+        zone,
+        grainDuration,
+      );
       acc["comparison.ts_position"] = getOffset(
         acc["comparison.ts"],
         offsetDuration,
@@ -110,7 +119,8 @@ export function prepareTimeSeries(
     if (!originalPt?.ts) {
       return emptyPt;
     }
-    const ts = adjustOffsetForZone(originalPt.ts, zone);
+    const ts = adjustOffsetForZone(originalPt.ts, zone, timeGrainDuration);
+
     if (!ts || typeof ts === "string") {
       return emptyPt;
     }
@@ -121,7 +131,12 @@ export function prepareTimeSeries(
       ts_position,
       bin: originalPt.bin,
       ...originalPt.records,
-      ...toComparisonKeys(comparisonPt || {}, offsetDuration, zone),
+      ...toComparisonKeys(
+        comparisonPt || {},
+        offsetDuration,
+        zone,
+        timeGrainDuration,
+      ),
     };
   });
 }
diff --git a/web-common/src/lib/convertTimestampPreview.ts b/web-common/src/lib/convertTimestampPreview.ts
index 09d82b08c68..8b2a0104a83 100644
--- a/web-common/src/lib/convertTimestampPreview.ts
+++ b/web-common/src/lib/convertTimestampPreview.ts
@@ -22,7 +22,11 @@ export function convertTimestampPreview(d, removeLocalTimezoneOffset = false) {
 export function adjustOffsetForZone(
   ts: Date | string | undefined,
   zone: string,
+  grainDuration: string,
 ) {
   if (!ts) return ts;
-  return addZoneOffset(remove(new Date(ts)), zone);
+
+  const removedLocalOffsetdate = remove(new Date(ts), grainDuration);
+
+  return addZoneOffset(removedLocalOffsetdate, zone);
 }
diff --git a/web-common/src/lib/time/grains/index.ts b/web-common/src/lib/time/grains/index.ts
index bbe513b7e75..736f0446048 100644
--- a/web-common/src/lib/time/grains/index.ts
+++ b/web-common/src/lib/time/grains/index.ts
@@ -7,7 +7,7 @@ import { V1TimeGrain } from "@rilldata/web-common/runtime-client";
 import { Duration } from "luxon";
 import { TIME_GRAIN } from "../config";
 import { getTimeWidth } from "../transforms";
-import type { TimeGrain } from "../types";
+import type { AvailableTimeGrain, TimeGrain } from "../types";
 
 export function unitToTimeGrain(unit: string): V1TimeGrain {
   return (
@@ -165,3 +165,24 @@ export function mapDurationToGrain(duration: string): V1TimeGrain {
   }
   return V1TimeGrain.TIME_GRAIN_UNSPECIFIED;
 }
+
+export function timeGrainToDuration(timeGrain: V1TimeGrain): string {
+  if (isAvailableTimeGrain(timeGrain)) {
+    const grainConfig = TIME_GRAIN[timeGrain];
+    return grainConfig.duration;
+  } else {
+    console.warn("Requested duration for invalid time grain: ", timeGrain);
+    // Default to 1 day if the time grain is invalid to fail gracefully
+    return "P1D";
+  }
+}
+
+export function isAvailableTimeGrain(
+  grain: V1TimeGrain,
+): grain is AvailableTimeGrain {
+  return (
+    grain !== V1TimeGrain.TIME_GRAIN_UNSPECIFIED &&
+    grain !== V1TimeGrain.TIME_GRAIN_MILLISECOND &&
+    grain !== V1TimeGrain.TIME_GRAIN_SECOND
+  );
+}
diff --git a/web-common/src/lib/time/timezone/index.ts b/web-common/src/lib/time/timezone/index.ts
index fcd48e40545..d420790b729 100644
--- a/web-common/src/lib/time/timezone/index.ts
+++ b/web-common/src/lib/time/timezone/index.ts
@@ -1,8 +1,26 @@
 import { timeZoneNameToAbbreviationMap } from "@rilldata/web-common/lib/time/timezone/abbreviationMap";
+import { getOffset } from "@rilldata/web-common/lib/time/transforms";
+import { TimeOffsetType } from "@rilldata/web-common/lib/time/types";
 import { DateTime } from "luxon";
 
-export function removeLocalTimezoneOffset(dt: Date) {
-  return new Date(dt.getTime() + dt.getTimezoneOffset() * 60000);
+function getDSToffset(start: Date, end: Date) {
+  const startDateTime = DateTime.fromJSDate(start);
+  const endDateTime = DateTime.fromMillis(end.getTime() - 1);
+
+  return startDateTime.offset - endDateTime.offset;
+}
+
+/**
+ * Removes the local timezone offset from a date.
+ *
+ * Some dates in Rill are used a range rather than a point of time.
+ * The runtime aggregates data and uses the start of a period to denote a range.
+ * For ex. 3 Nov 2024 with a daily grain would infer to [3 Nov 2024, 4 Nov 2024)
+ */
+export function removeLocalTimezoneOffset(dt: Date, grainDuration = "PT0S") {
+  const endTime = getOffset(dt, grainDuration, TimeOffsetType.ADD);
+  const dstOffset = getDSToffset(dt, endTime);
+  return new Date(dt.getTime() + (dt.getTimezoneOffset() + dstOffset) * 60000);
 }
 
 export function addZoneOffset(dt: Date, iana: string) {
@@ -31,7 +49,7 @@ export function getTimeZoneNameFromIANA(now: Date, iana: string): string {
   return DateTime.fromJSDate(now).setZone(iana).toFormat("ZZZZZ");
 }
 
-export function getAbbreviationForIANA(now: Date, iana: string): string {
+export function getAbbreviationForIANA(now: Date, iana: string) {
   const zoneName = getTimeZoneNameFromIANA(now, iana);
 
   if (zoneName in timeZoneNameToAbbreviationMap)