codeant-ai-for-open-source[bot] commented on code in PR #40681:
URL: https://github.com/apache/superset/pull/40681#discussion_r3605353670
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts:
##########
@@ -100,20 +100,23 @@ const processComparisonTotals = (
return totals;
}
const transformedTotals: DataRecord = {};
+ const mainLabel = t('Main');
totals.map((totalRecord: DataRecord) =>
Object.keys(totalRecord).forEach(key => {
if (totalRecord[key] !== undefined && !key.includes(comparisonSuffix)) {
- transformedTotals[`Main ${key}`] =
- parseInt(transformedTotals[`Main ${key}`]?.toString() || '0', 10) +
- parseInt(totalRecord[key]?.toString() || '0', 10);
+ transformedTotals[`${mainLabel} ${key}`] =
+ parseInt(
+ transformedTotals[`${mainLabel} ${key}`]?.toString() || '0',
+ 10,
+ ) + parseInt(totalRecord[key]?.toString() || '0', 10);
Review Comment:
**Suggestion:** The new total accumulation for the localized main comparison
column uses integer parsing, which truncates decimal metric values (for
example, 12.7 becomes 12) and produces incorrect totals and deltas in
comparison mode. Use non-integer numeric parsing (or direct numeric coercion)
so fractional metrics are preserved. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
❌ Table V2 comparison totals wrong for fractional metrics.
⚠️ Comparison deltas miscomputed using truncated main totals.
⚠️ Time comparison summary row displays inconsistent aggregate values.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Create or use a Table V2 (AG Grid) chart that uses the
`AgGridTableChartPlugin` defined
in
`superset-frontend/plugins/plugin-chart-ag-grid-table/src/index.ts:15-26`, with
a
numeric metric that can produce fractional totals (for example an average or
ratio metric
returning values like 12.7).
2. Ensure the chart is in aggregate query mode (`query_mode ===
QueryMode.Aggregate`) and
`show_totals` is enabled so that `buildQuery` adds an aggregate totals
query, as
implemented in
`superset-frontend/plugins/plugin-chart-ag-grid-table/src/buildQuery.ts:618-69`,
where it
builds a SUM totals query and returns `totalQuery?.data` with numeric
(potentially
fractional) totals.
3. Enable time comparison (`time_compare` and `comparison_type ===
ComparisonType.Values`)
so `transformProps` in
`superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts:519-523`
sets
`isUsingTimeComparison` to true and calls
`processComparisonTotals(comparisonSuffix,
totalQuery?.data)` at `transformProps.ts:756-759` when computing the
`totals` object used
for the summary row.
4. Observe execution in `processComparisonTotals` at
`transformProps.ts:95-49`, where for
each totals record it accumulates main comparison totals via
`transformedTotals[`${mainLabel} ${key}`] =
parseInt(transformedTotals[`${mainLabel}
${key}`]?.toString() || '0', 10) + parseInt(totalRecord[key]?.toString() ||
'0', 10);`
(lines 107-111 in the PR hunk). For a fractional total like 12.7 this
coerces to the
string `"12.7"` and then `parseInt` truncates it to `12`, so the stored main
total and the
resulting value/percent differences passed to `calculateDifferences` are
based on
truncated integers rather than the true fractional totals, causing the
displayed
comparison totals row in the Table V2 chart to show incorrect values for
fractional
metrics.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4c58721b54234ad995d898970b9f7075&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=4c58721b54234ad995d898970b9f7075&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-ag-grid-table/src/transformProps.ts
**Line:** 107:111
**Comment:**
*Logic Error: The new total accumulation for the localized main
comparison column uses integer parsing, which truncates decimal metric values
(for example, 12.7 becomes 12) and produces incorrect totals and deltas in
comparison mode. Use non-integer numeric parsing (or direct numeric coercion)
so fractional metrics are preserved.
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%2F40681&comment_hash=ccdf53af1dd7be660e194a933ed5536dc31121aba56420f151d33ebb8be2da65&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40681&comment_hash=ccdf53af1dd7be660e194a933ed5536dc31121aba56420f151d33ebb8be2da65&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]