codeant-ai-for-open-source[bot] commented on PR #36896: URL: https://github.com/apache/superset/pull/36896#issuecomment-3709150312
## Nitpicks 🔍 <table> <tr><td>🔒 <strong>No security issues identified</strong></td></tr> <tr><td>⚡ <strong>Recommended areas for review</strong><br><br> - [ ] <a href='https://github.com/apache/superset/pull/36896/files#diff-5f5c22612f270fe5718a3d200d291ec22bc1ee0bd576ef16f36e76d286cddb44R167-R265'><strong>stackGroup vs timeCompare</strong></a><br>When `stackGroup` is provided the code currently uses it as the entire stack id, bypassing the existing time-compare logic in `getTimeCompareStackId`. That can cause time-shifted series (timeCompare) to collapse into the same stack inside a group, producing incorrect stacking for time-comparison series. Verify that timeCompare stacks remain separated when stackGroup is used (or intentionally merge), and ensure stack ids remain unique when both grouping and timeCompare are present.<br> - [ ] <a href='https://github.com/apache/superset/pull/36896/files#diff-5f5c22612f270fe5718a3d200d291ec22bc1ee0bd576ef16f36e76d286cddb44R140-R165'><strong>data item shape handling</strong></a><br>`transformNegativeLabelsPosition` assumes series data values are arrays and destructures them directly. ECharts data items can also be objects with a `value` field or additional properties. The function may drop existing item properties for object-shaped data items. Ensure the function preserves other data-item properties and correctly extracts the numeric axis value from both array and object-shaped entries.<br> - [ ] <a href='https://github.com/apache/superset/pull/36896/files#diff-2672bc046f1a56f67dd661b1ace7c639da51ad78c4e442b96445c60924621c4eR514-R518'><strong>Possible Bug (stackGroup lookup — series B)</strong></a><br>The stackGroup value for series from the second query (rawSeriesB) looks up labelMapB using `seriesName`, but `seriesName` for B is constructed as `${seriesEntry} (1)`. If `label_map` keys do not include the " (1)" suffix, the lookup will fail and fallback to `entryName.split(', ')[0]`, potentially producing incorrect grouping/stacking. Verify label_map keys and consider using the original series key (without the " (1)" suffix) to derive the stackGroup.<br> - [ ] <a href='https://github.com/apache/superset/pull/36896/files#diff-2672bc046f1a56f67dd661b1ace7c639da51ad78c4e442b96445c60924621c4eR298-R306'><strong>Potential Code Smell</strong></a><br>`customFormattersSecondary` is built from `[...ensureIsArray(metrics), ...ensureIsArray(metricsB)]` (same as the primary custom formatters). This may be intentional, but if secondary axis formatters should only reflect metricsB, building formatters using both arrays is wasteful and could cause unexpected formatter selection. Confirm intended behavior.<br> </td></tr> </table> -- 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]
