codeant-ai-for-open-source[bot] commented on PR #35525:
URL: https://github.com/apache/superset/pull/35525#issuecomment-3706939708

   ## Nitpicks 🔍
   
   <table>
   <tr><td>🔒&nbsp;<strong>No security issues identified</strong></td></tr>
   <tr><td>⚡&nbsp;<strong>Recommended areas for review</strong><br><br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/35525/files#diff-d16e97921b691cfdf07753db19d61cd58d480a88428b53821addf023d736ec74R127-R146'><strong>Coordinate
 Unit Mismatch</strong></a><br>computeClusters builds a bbox by 
subtracting/adding an "offset" derived from width/height and small constants. 
Those offsets are in pixel-space yet are applied directly to longitude/latitude 
values. This likely mixes units (pixels vs geographic degrees) and can produce 
incorrect cluster bounding boxes. Verify intended behavior and, if pixel 
buffering is required, convert pixel offsets to geographic coordinates using 
WebMercatorViewport.project/unproject.<br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/35525/files#diff-4ba87e36aa24b7b146413e4834ea349ea37f8d5521b51f8db04b3e6d3fd19179R318-R326'><strong>Division
 by zero / NaN guard</strong></a><br>scaledRadius is computed as (clusterLabel 
/ maxLabel) ** 0.5 * radius without guarding against maxLabel === 0 or maxLabel 
being NaN. If maxLabel is 0 or NaN this yields NaN and breaks drawing. Add 
checks to avoid dividing by zero and provide sensible fallback.<br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/35525/files#diff-2ed115f015e65c4c37fad4a94a7f9090df1ecf54ec8772d497ebc2f2cd1c714eR20-R48'><strong>Duplicate
 ambient declarations</strong></a><br>Many projects already have global asset 
module declarations (e.g., in a root `global.d.ts`). Adding per-plugin 
declarations may create duplicate-identifier errors or confusing overlap if 
another declaration for the same extensions exists elsewhere. Verify this file 
won't be included twice or move declarations to a single shared typings 
file.<br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/35525/files#diff-d16e97921b691cfdf07753db19d61cd58d480a88428b53821addf023d736ec74R105-R124'><strong>Recompute
 Trigger Logic</strong></a><br>componentDidUpdate only checks reference 
equality of `clusterer` and a rounded zoom change to decide recomputation. 
Changes to `bounds`, `width`, `height`, or to the internal data of `clusterer` 
(same instance) won't trigger recompute. This can lead to stale clusters. 
Consider adding explicit checks for bounds/size changes or a more reliable 
change signal from the clusterer.<br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/35525/files#diff-ee863f4cc2d5d2249cc3d883e907bfce78156835592a2e2b33e4943b0ad7454cR112-R116'><strong>Early
 return shape</strong></a><br>When `mapboxColor` validation fails the function 
calls `onError` and returns `{}`. Upstream consumers may assume some properties 
(e.g., `onViewportChange`, `rgb`, `pointRadius`) exist. Ensure callers handle 
an empty returned object or consider returning a minimally safe shape to avoid 
runtime undefined access.<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]

Reply via email to