bito-code-review[bot] commented on code in PR #43468: URL: https://github.com/apache/superset/pull/43468#discussion_r3845445037
########## superset/examples/usa_births_names/charts/Trends.yaml: ########## @@ -23,7 +23,7 @@ params: compare_lag: '10' compare_suffix: o10Y granularity_sqla: ds - groupby: + columns: Review Comment: <div> <div id="suggestion"> <div id="issue"><b>Broken chart series grouping</b></div> <div id="fix"> Changing `groupby` to `columns` will break chart functionality. The frontend `buildQuery.ts` reads `formData.groupby` (line 44) to determine series grouping via `series_columns: groupby` (line 95). The `columns` field serves a different purpose (raw query column selection). While `security/manager.py` (lines 1550-1551) treats them as equivalent for access control, they are NOT interchangeable for query execution semantics. </div> </div> <small><i>Code Review Run #d0487c</i></small> </div> --- Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>) - [ ] Yes, avoid them ########## superset/examples/usa_births_names/charts/Genders.yaml: ########## @@ -23,7 +23,7 @@ params: compare_lag: '10' compare_suffix: o10Y granularity_sqla: ds - groupby: + columns: Review Comment: <div> <div id="suggestion"> <div id="issue"><b>CWE-20: Wrong chart parameter</b></div> <div id="fix"> The Pie chart plugin (`plugin-chart-echarts/src/Pie/controlPanel.tsx` line 54) explicitly requires the `groupby` parameter, not `columns`. All 5 other pie chart examples in the codebase (Pie.yaml, Work_Location_Preference.yaml, etc.) use `groupby`. The `columns` parameter belongs to Table/Pivot Table charts, not Pie. This change will cause the Genders chart to not group by `gender` at runtime. (See also: [CWE-20](https://cwe.mitre.org/data/definitions/20.html)) </div> </div> <small><i>Code Review Run #d0487c</i></small> </div> --- Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>) - [ ] Yes, avoid them -- 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]
