codeant-ai-for-open-source[bot] commented on code in PR #38345:
URL: https://github.com/apache/superset/pull/38345#discussion_r2884609119
##########
superset-frontend/plugins/plugin-chart-echarts/src/Histogram/transformProps.ts:
##########
@@ -169,9 +170,10 @@ export default function transformProps(
},
yAxis: {
...defaultYAxis,
+ ...(yAxisLogScale ? { min: 1 } : {}),
Review Comment:
**Suggestion:** When both normalization and log scale are enabled, forcing
the y-axis minimum to 1 will likely make all normalized values (which are
typically between 0 and 1) fall below the visible axis range, causing the
histogram bars to disappear or render incorrectly; the min override should not
apply in the normalized case. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Normalized histograms on log scale appear empty or distorted.
- ⚠️ Users misinterpret data when combining normalize and log scale.
```
</details>
```suggestion
...(yAxisLogScale && !normalize ? { min: 1 } : {}),
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. A histogram chart using the ECharts histogram plugin is rendered, which
calls
`transformProps()` from
`superset-frontend/plugins/plugin-chart-echarts/src/Histogram/transformProps.ts`
to build
`echartOptions` (see default export at top of this file).
2. `transformProps()` receives `chartProps.formData` where both `normalize`
and
`yAxisLogScale` are true (destructured at lines 55–67), a scenario reachable
from the
chart control panel where both options can be enabled simultaneously.
3. The series data for the histogram is built in `barSeries` (lines 93–119)
from the query
result; when `normalize` is enabled, these bar heights represent normalized
frequencies
and are typically between 0 and 1 for most bins.
4. The y-axis configuration at line 171–180 sets `type: 'log'` when
`yAxisLogScale` is
true and, due to the existing line 173, also sets `min: 1`; as a result, any
bar values
below 1 (the common case for normalized data) fall below the axis minimum
and are
effectively hidden or rendered at the bottom, causing the normalized
log-scale histogram
to appear empty or severely distorted. Removing the `min: 1` override
specifically for
`normalize === true` avoids this issue while preserving the protection
against
zero/negative values in non-normalized log charts.
```
</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/Histogram/transformProps.ts
**Line:** 173:173
**Comment:**
*Logic Error: When both normalization and log scale are enabled,
forcing the y-axis minimum to 1 will likely make all normalized values (which
are typically between 0 and 1) fall below the visible axis range, causing the
histogram bars to disappear or render incorrectly; the min override should not
apply in the normalized case.
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%2F38345&comment_hash=69a1949d79adcf02d7bf4a307303a22453377921476f28fb2e1e4fc5c7452c15&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38345&comment_hash=69a1949d79adcf02d7bf4a307303a22453377921476f28fb2e1e4fc5c7452c15&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]
