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>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](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)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](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]

Reply via email to