codeant-ai-for-open-source[bot] commented on code in PR #42810:
URL: https://github.com/apache/superset/pull/42810#discussion_r3723269869
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/utilities.ts:
##########
@@ -814,7 +836,16 @@ const baseAggregatorTemplates = {
return acc;
}
- return this.inner.value() / acc;
+ // A DB-computed rollup value can legitimately be a real SQL NULL
+ // (e.g. AVG over an empty group). `null / acc` coerces to `0` in
+ // JS, which would render a measured "0.0%" for a value that
+ // should stay blank, same as it does in "Actual values" mode.
+ const numerator = this.inner.value();
+ if (numerator === null) {
+ return null;
+ }
+
+ return numerator / acc;
},
Review Comment:
**Suggestion:** The denominator can be an existing aggregator whose inner
value is SQL `null`; the new guard only checks whether the inner aggregator
exists. In that case this expression coerces `null` to zero, producing
`Infinity` or `NaN` for nonzero or zero numerators instead of returning the
null/blank value contract. Check `acc === null` before dividing. [null pointer]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Fraction cells can display Infinity or NaN.
- ⚠️ Null denominator rollups render invalid percentage output.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d8a4ad0bce664378885e1b9ec93e9881&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=d8a4ad0bce664378885e1b9ec93e9881&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-pivot-table/src/react-pivottable/utilities.ts
**Line:** 849:849
**Comment:**
*Null Pointer: The denominator can be an existing aggregator whose
inner value is SQL `null`; the new guard only checks whether the inner
aggregator exists. In that case this expression coerces `null` to zero,
producing `Infinity` or `NaN` for nonzero or zero numerators instead of
returning the null/blank value contract. Check `acc === null` before dividing.
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%2F42810&comment_hash=38d23daa7a4eb02fe1512047b3f3fb7aa30f42b4f1f4b7788e6cd15a22e28a99&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42810&comment_hash=38d23daa7a4eb02fe1512047b3f3fb7aa30f42b4f1f4b7788e6cd15a22e28a99&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/utilities.ts:
##########
@@ -805,7 +824,10 @@ const baseAggregatorTemplates = {
}
}
const denominatorAggregator = data.getAggregator(selRow, selCol);
- if (!denominatorAggregator.inner) {
+ if (
+ !denominatorAggregator.inner ||
+ denominatorAggregator.metricCollision
Review Comment:
**Suggestion:** The result of `getAggregator` is statically typed as
`Aggregator`, but `Aggregator` does not declare `metricCollision`. Accessing
this newly added property causes a TypeScript property-access error during type
checking; extend the aggregator contract or narrow the returned value before
reading it. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Frontend TypeScript validation reports an aggregator property error.
- ⚠️ Plugin builds cannot reliably pass the repository typecheck.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e0ee3070153d44e59861e4aa44c27398&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=e0ee3070153d44e59861e4aa44c27398&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-pivot-table/src/react-pivottable/utilities.ts
**Line:** 829:829
**Comment:**
*Type Error: The result of `getAggregator` is statically typed as
`Aggregator`, but `Aggregator` does not declare `metricCollision`. Accessing
this newly added property causes a TypeScript property-access error during type
checking; extend the aggregator contract or narrow the returned value before
reading it.
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%2F42810&comment_hash=7135821584e80bc468b9dbb17543b57280d94760f131001e2e6bf451df8479ae&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42810&comment_hash=7135821584e80bc468b9dbb17543b57280d94760f131001e2e6bf451df8479ae&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]
