codeant-ai-for-open-source[bot] commented on code in PR #37525:
URL: https://github.com/apache/superset/pull/37525#discussion_r2736996344
##########
superset-frontend/packages/superset-ui-core/src/time-format/types.ts:
##########
@@ -35,7 +35,11 @@ export const TimeGranularity = {
FIFTEEN_MINUTES: 'PT15M',
THIRTY_MINUTES: 'PT30M',
HOUR: 'PT1H',
+ THREE_HOURS: 'PT3H',
+ SIX_HOURS: 'PT6H',
+ TWELVE_HOURS_6AM: 'PT12H_6AM',
Review Comment:
**Suggestion:** The value `'PT12H_6AM'` is not a valid ISO8601 duration or
an ISO8601 interval anchor; code that parses or normalizes time-grain strings
expects either a plain duration (e.g., `PT12H`) or a start-anchor/period
interval (e.g., `1969-12-31T06:00:00Z/PT12H`). As written, the parser will
likely fail or treat this string as an unknown grain. Replace it with a proper
anchored interval string that includes a timestamp and a period (UTC/Z) so
rounding/binning logic works correctly. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Chart aggregations mis-bucketed for 12-hour-at-06:00 queries.
- ⚠️ Backend `builtin_time_grains` lookup fails matching strings.
- ⚠️ Time-grain dropdown selection yields incorrect query behavior.
```
</details>
```suggestion
TWELVE_HOURS_6AM: '1969-12-31T06:00:00Z/PT12H',
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Inspect the frontend constant at
superset-frontend/packages/superset-ui-core/src/time-format/types.ts:40 where
TWELVE_HOURS_6AM is defined as 'PT12H_6AM'.
2. Select the corresponding 12-hour (6AM anchored) granularity in the UI
(the frontend
serializes the chosen TimeGranularity value into the query payload sent to
the backend).
3. The backend mapping for time grains is defined in
incubator-superset/superset/db_engine_specs/base.py (search for
`builtin_time_grains` as
hinted in the file comment). The backend lookup compares the incoming
time_grain string
against known ISO8601 durations/anchored intervals.
4. Because the frontend sends 'PT12H_6AM' (nonstandard), the backend's
`builtin_time_grains` will not match it, causing the query to fall back or
be treated as
an unknown grain — resulting in incorrect aggregation/binning for charts
(visualizations
showing wrong buckets or unexpected results).
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/time-format/types.ts
**Line:** 40:40
**Comment:**
*Logic Error: The value `'PT12H_6AM'` is not a valid ISO8601 duration
or an ISO8601 interval anchor; code that parses or normalizes time-grain
strings expects either a plain duration (e.g., `PT12H`) or a
start-anchor/period interval (e.g., `1969-12-31T06:00:00Z/PT12H`). As written,
the parser will likely fail or treat this string as an unknown grain. Replace
it with a proper anchored interval string that includes a timestamp and a
period (UTC/Z) so rounding/binning logic works correctly.
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>
##########
superset-frontend/packages/superset-ui-core/src/time-format/types.ts:
##########
@@ -35,7 +35,11 @@ export const TimeGranularity = {
FIFTEEN_MINUTES: 'PT15M',
THIRTY_MINUTES: 'PT30M',
HOUR: 'PT1H',
+ THREE_HOURS: 'PT3H',
+ SIX_HOURS: 'PT6H',
+ TWELVE_HOURS_6AM: 'PT12H_6AM',
DAY: 'P1D',
+ DAY_6AM: 'P1D_6AM',
Review Comment:
**Suggestion:** The value `'P1D_6AM'` is not a valid ISO8601 day interval or
anchored interval; consumers that expect an ISO8601 interval (start/date +
`/P1D`) will not recognize this token and rounding/aggregation by day-at-6AM
will fail. Use a proper anchored interval with a UTC timestamp and `/P1D` to
represent "days starting at 06:00 UTC". [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Daily aggregations misaligned relative to 06:00 boundary.
- ⚠️ Backend granularity mapping fails matching frontend token.
- ⚠️ Visualizations show incorrect daily buckets.
```
</details>
```suggestion
DAY_6AM: '1969-12-31T06:00:00Z/P1D',
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open superset-frontend/packages/superset-ui-core/src/time-format/types.ts
and observe
DAY_6AM defined at line 42 as 'P1D_6AM'.
2. In the UI, choose "day starting 06:00" granularity (the frontend will
send the
TimeGranularity value as the time_grain parameter in the query payload).
3. Backend logic that interprets time_grain values is maintained in
incubator-superset/superset/db_engine_specs/base.py (`builtin_time_grains`);
it expects
ISO8601 intervals or duration/anchor formats (e.g.,
'1969-12-31T06:00:00Z/P1D' or 'P1D').
4. Backend receives 'P1D_6AM', does not find a matching builtin_time_grains
entry, and
either rejects the granularity or uses an incorrect fallback — resulting in
daily
aggregations that do not start at 06:00 as intended.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/time-format/types.ts
**Line:** 42:42
**Comment:**
*Logic Error: The value `'P1D_6AM'` is not a valid ISO8601 day interval
or anchored interval; consumers that expect an ISO8601 interval (start/date +
`/P1D`) will not recognize this token and rounding/aggregation by day-at-6AM
will fail. Use a proper anchored interval with a UTC timestamp and `/P1D` to
represent "days starting at 06:00 UTC".
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>
--
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]
