codeant-ai-for-open-source[bot] commented on code in PR #38017:
URL: https://github.com/apache/superset/pull/38017#discussion_r2834552019
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts:
##########
@@ -29,13 +29,96 @@ import {
SMART_DATE_ID,
SMART_DATE_VERBOSE_ID,
TimeFormatter,
+ TimeGranularity,
ValueFormatter,
} from '@superset-ui/core';
export const getSmartDateDetailedFormatter = () =>
getTimeFormatter(SMART_DATE_DETAILED_ID);
-export const getSmartDateFormatter = () => getTimeFormatter(SMART_DATE_ID);
+export const getSmartDateFormatter = (timeGrain?: string) => {
+ const baseFormatter = getTimeFormatter(SMART_DATE_ID);
+
+ // If no time grain provided, use the standard smart date formatter
+ if (!timeGrain) {
+ return baseFormatter;
+ }
+
+ // Create a wrapper that normalizes dates based on time grain
+ return new TimeFormatter({
+ id: SMART_DATE_ID,
+ label: baseFormatter.label,
+ formatFunc: (date: Date) => {
+ // Create a normalized date based on time grain to ensure consistent
smart formatting
+ const normalizedDate = new Date(date);
+
+ // Always remove milliseconds to prevent .XXXms format
+ normalizedDate.setMilliseconds(0);
+
+ // For all time grains, normalize using UTC methods to avoid timezone
issues
+ if (timeGrain === TimeGranularity.YEAR) {
+ // Set to January 1st at midnight UTC - smart formatter will show year
+ const year = normalizedDate.getUTCFullYear();
+ const cleanDate = new Date(Date.UTC(year, 0, 1, 0, 0, 0, 0));
+ return baseFormatter(cleanDate);
+ } else if (timeGrain === TimeGranularity.QUARTER) {
+ // Set to first month of quarter, first day, midnight UTC
+ const year = normalizedDate.getUTCFullYear();
+ const month = normalizedDate.getUTCMonth();
+ const quarterStartMonth = Math.floor(month / 3) * 3;
+ const cleanDate = new Date(
+ Date.UTC(year, quarterStartMonth, 1, 0, 0, 0, 0),
+ );
+ return baseFormatter(cleanDate);
+ } else if (timeGrain === TimeGranularity.MONTH) {
+ // Set to first of month at midnight UTC - smart formatter will show
month name or year
+ const year = normalizedDate.getUTCFullYear();
+ const month = normalizedDate.getUTCMonth();
+ const cleanDate = new Date(Date.UTC(year, month, 1, 0, 0, 0, 0));
+ return baseFormatter(cleanDate);
+ } else if (
+ timeGrain === TimeGranularity.WEEK ||
+ timeGrain === TimeGranularity.WEEK_STARTING_SUNDAY ||
+ timeGrain === TimeGranularity.WEEK_STARTING_MONDAY ||
+ timeGrain === TimeGranularity.WEEK_ENDING_SATURDAY ||
+ timeGrain === TimeGranularity.WEEK_ENDING_SUNDAY
+ ) {
+ // Set to midnight UTC, keep the day
+ const year = normalizedDate.getUTCFullYear();
+ const month = normalizedDate.getUTCMonth();
+ const day = normalizedDate.getUTCDate();
+ const cleanDate = new Date(Date.UTC(year, month, day, 0, 0, 0, 0));
+ return baseFormatter(cleanDate);
+ } else if (timeGrain === TimeGranularity.DAY || timeGrain ===
TimeGranularity.DATE) {
+ // Set to midnight UTC
+ const year = normalizedDate.getUTCFullYear();
+ const month = normalizedDate.getUTCMonth();
+ const day = normalizedDate.getUTCDate();
+ const cleanDate = new Date(Date.UTC(year, month, day, 0, 0, 0, 0));
+ return baseFormatter(cleanDate);
+ } else if (
+ timeGrain === TimeGranularity.HOUR ||
+ timeGrain === TimeGranularity.THIRTY_MINUTES ||
+ timeGrain === TimeGranularity.FIFTEEN_MINUTES ||
+ timeGrain === TimeGranularity.TEN_MINUTES ||
+ timeGrain === TimeGranularity.FIVE_MINUTES ||
+ timeGrain === TimeGranularity.MINUTE ||
+ timeGrain === TimeGranularity.SECOND
+ ) {
Review Comment:
**Suggestion:** Grouping all sub-hour time grains (e.g., 30/15/5 minutes,
seconds) into the same branch as the hourly grain and normalizing them to the
top of the hour zeros out their minute/second components, so distinct time
buckets within the same hour produce identical formatted labels and
misrepresent the actual bucket boundaries. Restrict this normalization to the
pure hourly grain so that finer-grained buckets preserve their minute/second
resolution while still having milliseconds stripped earlier in the function.
[logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Timeseries chart x-axis mislabels sub-hour time buckets.
- ⚠️ Users may misinterpret 30/15/5-minute trend boundaries.
- ⚠️ Time-based analyses lose clarity at finer granularity.
```
</details>
```suggestion
} else if (timeGrain === TimeGranularity.HOUR) {
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In Superset, create or open a Timeseries ECharts chart (handled by
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:117-211`),
using a temporal column on the x-axis so `xAxisDataType ===
GenericDataType.Temporal` (see
lines 250-252).
2. In the chart controls, set **Time Grain** to a sub-hour grain such as `30
minutes` so
that `timeGrainSqla` is a sub-hour `TimeGranularity` value like
`TimeGranularity.THIRTY_MINUTES` (`transformProps.ts:181`), and set **X-Axis
Time Format**
to "Adaptive formatting" (maps to `SMART_DATE_ID`, passed as
`xAxisTimeFormat` at
`transformProps.ts:186,199`).
3. On render, `transformProps` computes the x-axis formatter for temporal
data as
`getXAxisFormatter(xAxisTimeFormat, timeGrainSqla)` at
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:559-566`.
With `xAxisTimeFormat = SMART_DATE_ID`, `getXAxisFormatter` returns
`getSmartDateFormatter(timeGrainSqla)` at
`superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts:172-178`.
4. Inside `getSmartDateFormatter`'s `formatFunc` (`formatters.ts:51-118`),
when
`timeGrainSqla` is any of `TimeGranularity.THIRTY_MINUTES`,
`FIFTEEN_MINUTES`,
`TEN_MINUTES`, `FIVE_MINUTES`, `MINUTE`, or `SECOND`, the current branch at
`formatters.ts:99-115` executes, normalizing each bucket's `Date` to the
*top of the hour*
(minutes and seconds set to `0`) before passing it to the base SMART_DATE
formatter. As a
result, distinct 30-minute (or finer) buckets within the same hour (e.g.,
`10:00` and
`10:30`) produce identical axis labels like `"2025-03-15 10:00"`, causing
duplicate/misleading x-axis labels in the rendered ECharts Timeseries chart.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts
**Line:** 99:107
**Comment:**
*Logic Error: Grouping all sub-hour time grains (e.g., 30/15/5 minutes,
seconds) into the same branch as the hourly grain and normalizing them to the
top of the hour zeros out their minute/second components, so distinct time
buckets within the same hour produce identical formatted labels and
misrepresent the actual bucket boundaries. Restrict this normalization to the
pure hourly grain so that finer-grained buckets preserve their minute/second
resolution while still having milliseconds stripped earlier in the function.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38017&comment_hash=532d02d2c727a0b7515a306bd99d2a2c28f0082e77fbdc0e9bfb22d15ac93aac&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38017&comment_hash=532d02d2c727a0b7515a306bd99d2a2c28f0082e77fbdc0e9bfb22d15ac93aac&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
