codeant-ai-for-open-source[bot] commented on PR #36819: URL: https://github.com/apache/superset/pull/36819#issuecomment-3688509497
## 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/36819/files#diff-d987d4addee4ccb4cccae5bb4376bdf7915776d6e8631f9a777abfa98c08c9a0R383-R396'><strong>Division-by-zero risk</strong></a><br>For the alignPositiveNegative case you compute the max absolute value via `d3Max(nums.map(Math.abs))` and cast to ValueRange. If the max is 0 (all zeros) this will later produce division by zero in `cellWidth` (value / maxValue). Add a guard to avoid returning a range with maxValue 0 or ensure `cellWidth` handles maxValue === 0 safely.<br> - [ ] <a href='https://github.com/apache/superset/pull/36819/files#diff-d987d4addee4ccb4cccae5bb4376bdf7915776d6e8631f9a777abfa98c08c9a0R383-R396'><strong>NaN / Infinity handling</strong></a><br>The new value filtering uses `typeof value === 'number'`, which includes `NaN` and `Infinity`. If those values exist, d3 calculations and subsequent percent/width math can produce `NaN` or `Infinity` and lead to invalid CSS widths or unexpected rendering. Consider filtering for finite numbers only.<br> - [ ] <a href='https://github.com/apache/superset/pull/36819/files#diff-ef3f268ef885e00851ad170f12c7dc714d86fc036fed86a88d64bc8bf2100985R70-R78'><strong>Numeric edge cases</strong></a><br>The new filter accepts any JavaScript `number` values but doesn't exclude NaN or Infinity. NaN/Infinity are of type 'number' and can break Math.abs, d3Max, and downstream visualizations or cause unexpected ranges.<br> - [ ] <a href='https://github.com/apache/superset/pull/36819/files#diff-ef3f268ef885e00851ad170f12c7dc714d86fc036fed86a88d64bc8bf2100985R73-R76'><strong>d3 return type assumptions</strong></a><br>The code casts d3Max/d3Extent results to `ValueRange` without an explicit non-null / fallback check. Although you check `nums.length > 0`, defensive handling (or explicit non-null assertions) would make the intent clearer and avoid subtle runtime issues if d3 functions ever return undefined for edge inputs.<br> - [ ] <a href='https://github.com/apache/superset/pull/36819/files#diff-d987d4addee4ccb4cccae5bb4376bdf7915776d6e8631f9a777abfa98c08c9a0R955-R968'><strong>Type narrowing for rendering</strong></a><br>The cell bar CSS now checks `typeof value === 'number'` before rendering — good to avoid BigInt errors. Confirm you also exclude NaN/Infinity at render-time (e.g., via `Number.isFinite`) to avoid creating invalid CSS values.<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]
