You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/07/22 06:33:51 UTC
[PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
rusackas opened a new pull request, #42300:
URL: https://github.com/apache/superset/pull/42300
### SUMMARY
When Y axis truncation is on and a value falls outside the configured min/max, ECharts was dropping that point (and the line segments around it) instead of clipping it at the boundary. This clamps series values to the configured Y axis bounds before handing them to ECharts, so a spike gets a flat top instead of a gap. Covers Line, Area, Smooth, Step and Bar series in the shared Timeseries chart.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
See the screenshots on #27449, this reproduces the same "expected" flat-top behavior described there.
### TESTING INSTRUCTIONS
Added a test that pins the bug: a line series with a value of 1000 and Y axis bounds `[0, 10]` used to drop that point; now the point stays in the series data, clamped to 10. Ran via `npx jest plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts`.
Manually: create a line/area/bar chart with a big outlier, turn on "Truncate Y Axis" with a max below the outlier, confirm the line still connects through a flat top rather than a gap.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #27449
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #42300:
URL: https://github.com/apache/superset/pull/42300#issuecomment-5274775575
@aminghadersohi thanks for the deep pass. Added the annotation-skip and horizontal (valueIndex 0) clamp tests you flagged, and noted the tooltip/label value change and gap-to-pinned-boundary shift in `UPDATING.md`.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42300:
URL: https://github.com/apache/superset/pull/42300#discussion_r3680950064
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -724,6 +724,57 @@ export default function transformProps(
}
}
+ // Whenever a Y axis bound is defined, whether explicitly configured or
+ // derived above from the data, clamp series values to those bounds
+ // instead of leaving raw out-of-range values in place. ECharts axis
+ // clipping can otherwise drop an out-of-bounds point (and the line
+ // segments around it) entirely rather than truncating it at the
+ // boundary (see https://github.com/apache/superset/issues/27449).
+ if (yAxisMin !== undefined || yAxisMax !== undefined) {
+ const valueIndex = isHorizontal ? 0 : 1;
+ type AxisValue = string | number | null | undefined;
+ type AxisPoint = AxisValue[];
+ const clampAxisValue = (value: AxisValue): AxisValue => {
+ if (typeof value !== 'number' || Number.isNaN(value)) return value;
+ let clamped = value;
+ if (yAxisMin !== undefined) clamped = Math.max(clamped, yAxisMin);
+ if (yAxisMax !== undefined) clamped = Math.min(clamped, yAxisMax);
Review Comment:
**Suggestion:** Stacked series are clamped independently before ECharts applies stacking. For example, two segments valued at 8 with a maximum of 10 remain 8 each, but their cumulative stack reaches 16 and is still outside the axis; conversely, clamping individual segments can alter stack totals and percentage calculations. The bounds need to be applied to the rendered cumulative stack geometry or this rewrite must be limited to unstacked series. [logic error]
<details>
<summary><b>Severity Level:</b> Critical π¨</summary>
```mdx
- β Stacked area and bar geometry remains incorrectly clipped.
- β οΈ Segment totals and percentages can reflect altered values.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=824838fa4c2b4e778b337123658ed842&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=824838fa4c2b4e778b337123658ed842&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/transformProps.ts
**Line:** 740:741
**Comment:**
*Logic Error: Stacked series are clamped independently before ECharts applies stacking. For example, two segments valued at 8 with a maximum of 10 remain 8 each, but their cumulative stack reaches 16 and is still outside the axis; conversely, clamping individual segments can alter stack totals and percentage calculations. The bounds need to be applied to the rendered cumulative stack geometry or this rewrite must be limited to unstacked series.
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%2F42300&comment_hash=b02505244211ad97156745a677996241d43156070e292e2588dc80c9a9dc8a6e&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42300&comment_hash=b02505244211ad97156745a677996241d43156070e292e2588dc80c9a9dc8a6e&reaction=dislike'>π</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -724,6 +724,57 @@ export default function transformProps(
}
}
+ // Whenever a Y axis bound is defined, whether explicitly configured or
+ // derived above from the data, clamp series values to those bounds
+ // instead of leaving raw out-of-range values in place. ECharts axis
+ // clipping can otherwise drop an out-of-bounds point (and the line
+ // segments around it) entirely rather than truncating it at the
+ // boundary (see https://github.com/apache/superset/issues/27449).
+ if (yAxisMin !== undefined || yAxisMax !== undefined) {
+ const valueIndex = isHorizontal ? 0 : 1;
+ type AxisValue = string | number | null | undefined;
+ type AxisPoint = AxisValue[];
+ const clampAxisValue = (value: AxisValue): AxisValue => {
+ if (typeof value !== 'number' || Number.isNaN(value)) return value;
+ let clamped = value;
+ if (yAxisMin !== undefined) clamped = Math.max(clamped, yAxisMin);
+ if (yAxisMax !== undefined) clamped = Math.min(clamped, yAxisMax);
+ return clamped;
+ };
+ const clampPoint = (point: AxisPoint): AxisPoint => {
+ const newPoint = [...point];
+ newPoint[valueIndex] = clampAxisValue(newPoint[valueIndex]);
+ return newPoint;
+ };
+ series.forEach(s => {
+ if (!Array.isArray(s.data)) return;
Review Comment:
**Suggestion:** The loop includes timeseries annotation series, whose ordinary `[x, y]` points are appended to `series` before this block. An annotation value outside the chart bounds is therefore permanently rewritten to the bound, changing the annotation's location instead of merely clipping the observation series. Restrict clamping to query-derived observation series and leave annotation series unchanged. [logic error]
<details>
<summary><b>Severity Level:</b> Major β οΈ</summary>
```mdx
- β Formula annotations can appear at incorrect Y coordinates.
- β οΈ Timeseries annotation locations and tooltips become inaccurate.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6cb4480938ff4f5b86258a693dfb1710&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=6cb4480938ff4f5b86258a693dfb1710&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/transformProps.ts
**Line:** 749:750
**Comment:**
*Logic Error: The loop includes timeseries annotation series, whose ordinary `[x, y]` points are appended to `series` before this block. An annotation value outside the chart bounds is therefore permanently rewritten to the bound, changing the annotation's location instead of merely clipping the observation series. Restrict clamping to query-derived observation series and leave annotation series unchanged.
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%2F42300&comment_hash=0a3920dc1e4c689c929e434f51d992a7939566a4ca37b8f060d1a8821b7e8d1a&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42300&comment_hash=0a3920dc1e4c689c929e434f51d992a7939566a4ca37b8f060d1a8821b7e8d1a&reaction=dislike'>π</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -724,6 +724,57 @@ export default function transformProps(
}
}
+ // Whenever a Y axis bound is defined, whether explicitly configured or
+ // derived above from the data, clamp series values to those bounds
+ // instead of leaving raw out-of-range values in place. ECharts axis
+ // clipping can otherwise drop an out-of-bounds point (and the line
+ // segments around it) entirely rather than truncating it at the
+ // boundary (see https://github.com/apache/superset/issues/27449).
+ if (yAxisMin !== undefined || yAxisMax !== undefined) {
+ const valueIndex = isHorizontal ? 0 : 1;
+ type AxisValue = string | number | null | undefined;
+ type AxisPoint = AxisValue[];
+ const clampAxisValue = (value: AxisValue): AxisValue => {
+ if (typeof value !== 'number' || Number.isNaN(value)) return value;
+ let clamped = value;
+ if (yAxisMin !== undefined) clamped = Math.max(clamped, yAxisMin);
+ if (yAxisMax !== undefined) clamped = Math.min(clamped, yAxisMax);
+ return clamped;
+ };
+ const clampPoint = (point: AxisPoint): AxisPoint => {
+ const newPoint = [...point];
+ newPoint[valueIndex] = clampAxisValue(newPoint[valueIndex]);
+ return newPoint;
+ };
+ series.forEach(s => {
+ if (!Array.isArray(s.data)) return;
+ const clampedData = (
+ s.data as (AxisPoint | Record<string, unknown>)[]
+ ).map(point => {
+ if (Array.isArray(point)) {
+ return clampPoint(point);
+ }
+ // Some series paths (e.g. colorByPrimaryAxis, or negative bar
+ // label positioning) wrap the tuple in an object of the shape
+ // `{ value: [x, y], ... }` instead of passing the tuple
+ // directly; clamp the wrapped tuple in place so those points
+ // aren't skipped and left to be dropped by ECharts axis clipping.
+ if (
+ point &&
+ typeof point === 'object' &&
+ Array.isArray((point as { value?: unknown }).value)
+ ) {
+ return {
+ ...point,
+ value: clampPoint((point as { value: AxisPoint }).value),
+ };
+ }
+ return point;
+ });
+ s.data = clampedData as typeof s.data;
Review Comment:
**Suggestion:** This mutates the values used by ECharts tooltips and labels, so an original observation such as `1000` is exposed to the formatter as `10` when the configured maximum is `10`. The chart should preserve the source value for tooltip and label content while using a separate clipped value for rendering, or explicitly retain the original value in the data item. [api mismatch]
<details>
<summary><b>Severity Level:</b> Critical π¨</summary>
```mdx
- β Tooltips report truncated values as source observations.
- β οΈ Data labels and stacked totals can become misleading.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2d955762601043b1877b23194f838b35&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=2d955762601043b1877b23194f838b35&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/transformProps.ts
**Line:** 773:774
**Comment:**
*Api Mismatch: This mutates the values used by ECharts tooltips and labels, so an original observation such as `1000` is exposed to the formatter as `10` when the configured maximum is `10`. The chart should preserve the source value for tooltip and label content while using a separate clipped value for rendering, or explicitly retain the original value in the data item.
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%2F42300&comment_hash=cb51c398f177fc688f55c4ca73d395d161044cbced2cfc1256317a604a100b6d&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42300&comment_hash=cb51c398f177fc688f55c4ca73d395d161044cbced2cfc1256317a604a100b6d&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #42300:
URL: https://github.com/apache/superset/pull/42300#discussion_r3680919964
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -724,6 +724,41 @@ export default function transformProps(
}
}
+ // Whenever a Y axis bound is defined, whether explicitly configured or
+ // derived above from the data, clamp series values to those bounds
+ // instead of leaving raw out-of-range values in place. ECharts axis
+ // clipping can otherwise drop an out-of-bounds point (and the line
+ // segments around it) entirely rather than truncating it at the
+ // boundary (see https://github.com/apache/superset/issues/27449).
+ if (yAxisMin !== undefined || yAxisMax !== undefined) {
+ const valueIndex = isHorizontal ? 0 : 1;
+ const clampAxisValue = (
+ value: string | number | null | undefined,
+ ): string | number | null | undefined => {
+ if (typeof value !== 'number' || Number.isNaN(value)) return value;
+ let clamped = value;
+ if (yAxisMin !== undefined) clamped = Math.max(clamped, yAxisMin);
+ if (yAxisMax !== undefined) clamped = Math.min(clamped, yAxisMax);
+ return clamped;
+ };
+ series.forEach(s => {
+ if (!Array.isArray(s.data)) return;
+ const clampedData = (
+ s.data as (string | number | null | undefined)[][]
+ ).map(point => {
+ if (Array.isArray(point)) {
+ const newPoint = [...point];
+ newPoint[valueIndex] = clampAxisValue(newPoint[valueIndex]);
+ return newPoint;
+ }
+ return point;
+ });
+ // Matches the existing pattern used elsewhere in this file for
+ // narrowing the broad, union-typed ECharts `SeriesOption.data` field.
+ (s as any).data = clampedData;
+ });
Review Comment:
Good catch, fixed. colorByPrimaryAxis (and the negative-bar label wrapping) puts points in `{ value: [x, y], ... }` objects instead of bare tuples, so those were skipping the clamp entirely. Extended it to handle both shapes and dropped the `any` cast while I was in there.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #42300:
URL: https://github.com/apache/superset/pull/42300#issuecomment-5048487062
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/42300?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 65.15%. Comparing base ([`788165b`](https://app.codecov.io/gh/apache/superset/commit/788165b5bbb68a544506119edd5e1773fe73b79b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`c22bae4`](https://app.codecov.io/gh/apache/superset/commit/c22bae420c1b07261ce9e283ff027082f5890205?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 7 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #42300 +/- ##
=======================================
Coverage 65.15% 65.15%
=======================================
Files 2789 2789
Lines 157543 157562 +19
Branches 35864 35871 +7
=======================================
+ Hits 102644 102657 +13
- Misses 52931 52937 +6
Partials 1968 1968
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/42300/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/42300/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `71.07% <100.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/42300?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42300:
URL: https://github.com/apache/superset/pull/42300#issuecomment-5269341657
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #42300:
URL: https://github.com/apache/superset/pull/42300#discussion_r3629017215
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -724,6 +724,40 @@ export default function transformProps(
}
}
+ // When explicit Y axis bounds are set, clamp series values to those bounds
+ // instead of leaving raw out-of-range values in place. ECharts axis
+ // clipping can otherwise drop an out-of-bounds point (and the line
+ // segments around it) entirely rather than truncating it at the
+ // boundary (see https://github.com/apache/superset/issues/27449).
+ if (yAxisMin !== undefined || yAxisMax !== undefined) {
Review Comment:
Fair point, the comment overstated it β bounds get set from derived data too (shouldCalculateDataBounds, log axis floor, etc.), not just explicit user config. Reworded to cover both.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42300:
URL: https://github.com/apache/superset/pull/42300#discussion_r3628032294
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -724,6 +724,40 @@ export default function transformProps(
}
}
+ // When explicit Y axis bounds are set, clamp series values to those bounds
+ // instead of leaving raw out-of-range values in place. ECharts axis
+ // clipping can otherwise drop an out-of-bounds point (and the line
+ // segments around it) entirely rather than truncating it at the
+ // boundary (see https://github.com/apache/superset/issues/27449).
+ if (yAxisMin !== undefined || yAxisMax !== undefined) {
Review Comment:
**Suggestion:** The new comment says clamping happens only when explicit bounds are set, but the condition triggers for any defined bound value, including non-explicit values assigned earlier in transform logic; this mismatch makes the behavior easy to misread and can lead to unintended clamping in future edits. Update the comment or narrow the condition so implementation and description match. [comment mismatch]
<details>
<summary><b>Severity Level:</b> Minor π§Ή</summary>
```mdx
- β οΈ Code comment misstates clamping trigger condition.
- β οΈ Possible minor developer confusion when maintaining code.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. Configure a Timeseries chart without explicit Y axis min or max, so bounds are derived
automatically in `transformProps` (see lines 714β724 where `yAxisMax` and `yAxisMin` can
be set from `effectiveDataMax` and `dataMin` when undefined).
2. Render the chart, causing `transformProps` in
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts` to run
and compute `yAxisMin`/`yAxisMax` based on data even though the user did not set explicit
bounds.
3. At lines 727β732, the clamping block guarded by `if (yAxisMin !== undefined || yAxisMax
!== undefined)` executes because these derived bounds are defined, regardless of whether
they were explicitly configured by the user.
4. The runtime behavior (clamping whenever any axis bounds exist) is consistent and
intentional for avoiding dropped points, but the comment specifically says βWhen explicit
Y axis bounds are set,β which does not match the broader condition; the discrepancy is
limited to developer-facing documentation and does not change chart output.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=92681971b81c4623afc4a4d479bce141&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=92681971b81c4623afc4a4d479bce141&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/transformProps.ts
**Line:** 727:732
**Comment:**
*Comment Mismatch: The new comment says clamping happens only when explicit bounds are set, but the condition triggers for any defined bound value, including non-explicit values assigned earlier in transform logic; this mismatch makes the behavior easy to misread and can lead to unintended clamping in future edits. Update the comment or narrow the condition so implementation and description match.
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%2F42300&comment_hash=955d60f6bcc71e063fad46204995dfb0e7f3cf00cbb7624a2103a0f95564d782&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42300&comment_hash=955d60f6bcc71e063fad46204995dfb0e7f3cf00cbb7624a2103a0f95564d782&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #42300:
URL: https://github.com/apache/superset/pull/42300#discussion_r3685631623
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -724,6 +724,57 @@ export default function transformProps(
}
}
+ // Whenever a Y axis bound is defined, whether explicitly configured or
+ // derived above from the data, clamp series values to those bounds
+ // instead of leaving raw out-of-range values in place. ECharts axis
+ // clipping can otherwise drop an out-of-bounds point (and the line
+ // segments around it) entirely rather than truncating it at the
+ // boundary (see https://github.com/apache/superset/issues/27449).
+ if (yAxisMin !== undefined || yAxisMax !== undefined) {
+ const valueIndex = isHorizontal ? 0 : 1;
+ type AxisValue = string | number | null | undefined;
+ type AxisPoint = AxisValue[];
+ const clampAxisValue = (value: AxisValue): AxisValue => {
+ if (typeof value !== 'number' || Number.isNaN(value)) return value;
+ let clamped = value;
+ if (yAxisMin !== undefined) clamped = Math.max(clamped, yAxisMin);
+ if (yAxisMax !== undefined) clamped = Math.min(clamped, yAxisMax);
+ return clamped;
+ };
+ const clampPoint = (point: AxisPoint): AxisPoint => {
+ const newPoint = [...point];
+ newPoint[valueIndex] = clampAxisValue(newPoint[valueIndex]);
+ return newPoint;
+ };
+ series.forEach(s => {
+ if (!Array.isArray(s.data)) return;
Review Comment:
Good catch, fixed. Annotation series were getting pushed into `series` before the clamp loop ran, so their configured values were getting rewritten to the axis bounds along with the real observation data. Snapshotted the series count before annotations get appended and skip clamping anything past that index now.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -724,6 +724,57 @@ export default function transformProps(
}
}
+ // Whenever a Y axis bound is defined, whether explicitly configured or
+ // derived above from the data, clamp series values to those bounds
+ // instead of leaving raw out-of-range values in place. ECharts axis
+ // clipping can otherwise drop an out-of-bounds point (and the line
+ // segments around it) entirely rather than truncating it at the
+ // boundary (see https://github.com/apache/superset/issues/27449).
+ if (yAxisMin !== undefined || yAxisMax !== undefined) {
+ const valueIndex = isHorizontal ? 0 : 1;
+ type AxisValue = string | number | null | undefined;
+ type AxisPoint = AxisValue[];
+ const clampAxisValue = (value: AxisValue): AxisValue => {
+ if (typeof value !== 'number' || Number.isNaN(value)) return value;
+ let clamped = value;
+ if (yAxisMin !== undefined) clamped = Math.max(clamped, yAxisMin);
+ if (yAxisMax !== undefined) clamped = Math.min(clamped, yAxisMax);
Review Comment:
I don't think this changes behavior here. The clamp only touches a value that individually exceeds the bound, so in your 8+8/max=10 example neither segment gets touched, same as before this PR. ECharts clips bar rendering to the grid area by default regardless of this, so an overflowing stack was already visually truncated at the boundary, not dropped. And the auto-calculated bounds path already accounts for the stacked total via `stackedTotalMax`.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -724,6 +724,57 @@ export default function transformProps(
}
}
+ // Whenever a Y axis bound is defined, whether explicitly configured or
+ // derived above from the data, clamp series values to those bounds
+ // instead of leaving raw out-of-range values in place. ECharts axis
+ // clipping can otherwise drop an out-of-bounds point (and the line
+ // segments around it) entirely rather than truncating it at the
+ // boundary (see https://github.com/apache/superset/issues/27449).
+ if (yAxisMin !== undefined || yAxisMax !== undefined) {
+ const valueIndex = isHorizontal ? 0 : 1;
+ type AxisValue = string | number | null | undefined;
+ type AxisPoint = AxisValue[];
+ const clampAxisValue = (value: AxisValue): AxisValue => {
+ if (typeof value !== 'number' || Number.isNaN(value)) return value;
+ let clamped = value;
+ if (yAxisMin !== undefined) clamped = Math.max(clamped, yAxisMin);
+ if (yAxisMax !== undefined) clamped = Math.min(clamped, yAxisMax);
+ return clamped;
+ };
+ const clampPoint = (point: AxisPoint): AxisPoint => {
+ const newPoint = [...point];
+ newPoint[valueIndex] = clampAxisValue(newPoint[valueIndex]);
+ return newPoint;
+ };
+ series.forEach(s => {
+ if (!Array.isArray(s.data)) return;
+ const clampedData = (
+ s.data as (AxisPoint | Record<string, unknown>)[]
+ ).map(point => {
+ if (Array.isArray(point)) {
+ return clampPoint(point);
+ }
+ // Some series paths (e.g. colorByPrimaryAxis, or negative bar
+ // label positioning) wrap the tuple in an object of the shape
+ // `{ value: [x, y], ... }` instead of passing the tuple
+ // directly; clamp the wrapped tuple in place so those points
+ // aren't skipped and left to be dropped by ECharts axis clipping.
+ if (
+ point &&
+ typeof point === 'object' &&
+ Array.isArray((point as { value?: unknown }).value)
+ ) {
+ return {
+ ...point,
+ value: clampPoint((point as { value: AxisPoint }).value),
+ };
+ }
+ return point;
+ });
+ s.data = clampedData as typeof s.data;
Review Comment:
Fair point, this does mutate the same data ECharts reads for tooltips and labels, so a clamped point's tooltip shows the truncated value instead of the real observation. Keeping the original value available to the formatters while only clamping the render position would be the right fix, but that touches how values flow through several of these formatters and I'd rather think it through than bolt something on here.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #42300:
URL: https://github.com/apache/superset/pull/42300
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #42300:
URL: https://github.com/apache/superset/pull/42300#issuecomment-6274677209
I raised a new issue to continue this, I have an ongoing problem with the first issue that Amin raised initially.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42300:
URL: https://github.com/apache/superset/pull/42300#issuecomment-5129844234
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42300:
URL: https://github.com/apache/superset/pull/42300#discussion_r3685634347
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -724,6 +724,57 @@ export default function transformProps(
}
}
+ // Whenever a Y axis bound is defined, whether explicitly configured or
+ // derived above from the data, clamp series values to those bounds
+ // instead of leaving raw out-of-range values in place. ECharts axis
+ // clipping can otherwise drop an out-of-bounds point (and the line
+ // segments around it) entirely rather than truncating it at the
+ // boundary (see https://github.com/apache/superset/issues/27449).
+ if (yAxisMin !== undefined || yAxisMax !== undefined) {
+ const valueIndex = isHorizontal ? 0 : 1;
+ type AxisValue = string | number | null | undefined;
+ type AxisPoint = AxisValue[];
+ const clampAxisValue = (value: AxisValue): AxisValue => {
+ if (typeof value !== 'number' || Number.isNaN(value)) return value;
+ let clamped = value;
+ if (yAxisMin !== undefined) clamped = Math.max(clamped, yAxisMin);
+ if (yAxisMax !== undefined) clamped = Math.min(clamped, yAxisMax);
Review Comment:
β
**Customized review instruction saved!**
**Instruction:**
> Do not flag the axis-value clamping in timeseries chart transformations as a stacked-series logic error when ECharts handles visual clipping and auto-calculated bounds account for stacked totals.
**Applied to:**
- `superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/**`
---
π‘ *To manage or update this instruction, visit: [CodeAnt AI Settings](https://app.codeant.ai/org/settings/learnings)*
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42300:
URL: https://github.com/apache/superset/pull/42300#issuecomment-5042717817
<!-- Bito Reply -->
The flagged issue is correct. The code comment states that clamping occurs "When explicit Y axis bounds are set," but the implementation triggers clamping whenever `yAxisMin` or `yAxisMax` are defined, which includes cases where they are derived automatically from the data. To resolve this, the comment should be updated to reflect that clamping occurs whenever Y axis bounds are defined, regardless of whether they were explicitly set by the user or derived automatically.
**superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts**
```
// When Y axis bounds are defined, clamp series values to those bounds
// instead of leaving raw out-of-range values in place. ECharts axis
// clipping can otherwise drop an out-of-bounds point (and the line
// segments around it) entirely rather than truncating it at the
// boundary (see https://github.com/apache/superset/issues/27449).
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42300:
URL: https://github.com/apache/superset/pull/42300#issuecomment-5045666588
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42300:
URL: https://github.com/apache/superset/pull/42300#issuecomment-5042677207
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42300:
URL: https://github.com/apache/superset/pull/42300#issuecomment-5275957409
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #42300:
URL: https://github.com/apache/superset/pull/42300#discussion_r3679057256
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -724,6 +724,41 @@ export default function transformProps(
}
}
+ // Whenever a Y axis bound is defined, whether explicitly configured or
+ // derived above from the data, clamp series values to those bounds
+ // instead of leaving raw out-of-range values in place. ECharts axis
+ // clipping can otherwise drop an out-of-bounds point (and the line
+ // segments around it) entirely rather than truncating it at the
+ // boundary (see https://github.com/apache/superset/issues/27449).
+ if (yAxisMin !== undefined || yAxisMax !== undefined) {
+ const valueIndex = isHorizontal ? 0 : 1;
+ const clampAxisValue = (
+ value: string | number | null | undefined,
+ ): string | number | null | undefined => {
+ if (typeof value !== 'number' || Number.isNaN(value)) return value;
+ let clamped = value;
+ if (yAxisMin !== undefined) clamped = Math.max(clamped, yAxisMin);
+ if (yAxisMax !== undefined) clamped = Math.min(clamped, yAxisMax);
+ return clamped;
+ };
+ series.forEach(s => {
+ if (!Array.isArray(s.data)) return;
+ const clampedData = (
+ s.data as (string | number | null | undefined)[][]
+ ).map(point => {
+ if (Array.isArray(point)) {
+ const newPoint = [...point];
+ newPoint[valueIndex] = clampAxisValue(newPoint[valueIndex]);
+ return newPoint;
+ }
+ return point;
+ });
+ // Matches the existing pattern used elsewhere in this file for
+ // narrowing the broad, union-typed ECharts `SeriesOption.data` field.
+ (s as any).data = clampedData;
+ });
Review Comment:
The clamping logic only adjusts tuple points (e.g. `[x, y]`). Some series paths (e.g. `colorByPrimaryAxis`) use object data items like `{ value: [x, y], itemStyle: ... }`, which this code will skip, so out-of-range values can still be dropped by ECharts in those modes. It also relies on `(s as any).data` to assign back. Consider clamping both tuple points and `{ value: [...] }` points and assign back with a typed cast instead of `any`.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42300:
URL: https://github.com/apache/superset/pull/42300#issuecomment-5136922766
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(echarts): truncate values at Y axis bounds instead of dropping them [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #42300:
URL: https://github.com/apache/superset/pull/42300#issuecomment-5128092633
### <span aria-hidden="true">β
</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">π¨</span> Latest commit | 93b4076eed133980e865219f617ba305ea381447 |
|<span aria-hidden="true">π</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a6b00d870efea0007b819c5 |
|<span aria-hidden="true">π</span> Deploy Preview | [https://deploy-preview-42300--superset-docs-preview.netlify.app](https://deploy-preview-42300--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">π±</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">π€</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DctLDoMgEADQu8zaSaeNBe0NeowBxkgCSvg0msa7y-5t3h9S3mOq8AEYwGTe7Nq9-OPhS2mCLz2OM9bcNstV8OTDF7SBY8IfhyalNycp7OfX9ahYGSI3aZJFmIi0mZ6zfcN1Aw) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-42300--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org