codeant-ai-for-open-source[bot] commented on code in PR #38126:
URL: https://github.com/apache/superset/pull/38126#discussion_r3651469209
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts:
##########
@@ -69,6 +69,31 @@ import {
TIMESERIES_CONSTANTS,
} from '../constants';
+function parseTimeShiftToMs(timeShift?: string | null): number {
+ if (!timeShift) return 0;
+
+ const match = timeShift
+ .trim()
+
.match(/^(-?\d+(?:\.\d+)?)\s*(second|minute|hour|day|week|month|year)s?$/i);
+
+ if (!match) return 0;
+
+ const value = Number(match[1]);
+ const unit = match[2].toLowerCase();
+
+ const MS: Record<string, number> = {
+ second: 1000,
+ minute: 60 * 1000,
+ hour: 60 * 60 * 1000,
+ day: 24 * 60 * 60 * 1000,
+ week: 7 * 24 * 60 * 60 * 1000,
+ month: 30 * 24 * 60 * 60 * 1000,
+ year: 365 * 24 * 60 * 60 * 1000,
+ };
+
+ return value * (MS[unit] ?? 0);
Review Comment:
**Suggestion:** Using fixed millisecond durations for `month` and `year`
does not implement calendar-based time shifts. A one-month shift from January
31 will land in March rather than February, and the result varies incorrectly
across month lengths and leap years. Apply calendar-aware date arithmetic for
month and year shifts instead of converting them to 30 and 365 days. [logic
error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Timeseries annotations shift incorrectly across month boundaries.
- ⚠️ Leap-year annotation alignment can differ by one or more days.
- ⚠️ Shifted annotation lines may not align with chart data.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a Timeseries annotation layer with `overrides.time_shift` set
to a calendar
value such as `1 month`, and provide an annotation record whose first field
is a timestamp
on January 31; `transformTimeseriesAnnotation()` accepts the layer and
annotation data at
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts:702`.
2. During transformation, `transformTimeseriesAnnotation()` reads
`layer.overrides.time_shift` and passes it to `parseTimeShiftToMs()` at
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts:714`.
3. `parseTimeShiftToMs()` matches `1 month` and uses the fixed `30 * 24 * 60
* 60 * 1000`
value at
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts:84-94`.
4. The annotation record is converted to milliseconds and shifted by that
fixed duration
at
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts:720-731`;
January 31 therefore moves to March 2 in a non-leap year rather than the
expected February
calendar position, causing the generated annotation series at lines 734-737
to be
misaligned.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c66b48090f79411b9acbecea67d9977c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c66b48090f79411b9acbecea67d9977c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<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/Timeseries/transformers.ts
**Line:** 84:94
**Comment:**
*Logic Error: Using fixed millisecond durations for `month` and `year`
does not implement calendar-based time shifts. A one-month shift from January
31 will land in March rather than February, and the result varies incorrectly
across month lengths and leap years. Apply calendar-aware date arithmetic for
month and year shifts instead of converting them to 30 and 365 days.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38126&comment_hash=c03c7a5b805553a6e54924e74c03707f1208b865f8e2daae4208b4bf23bee78a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38126&comment_hash=c03c7a5b805553a6e54924e74c03707f1208b865f8e2daae4208b4bf23bee78a&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]
