codeant-ai-for-open-source[bot] commented on PR #37027: URL: https://github.com/apache/superset/pull/37027#issuecomment-3731835878
## 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/37027/files#diff-028e07b18b264f0cfd26c72df29e3ab4d079b3da6a42bd1d9c2e902ebee09293R125-R133'><strong>Geohash polygon coords</strong></a><br>The geohash decoding builds polygon vertices using values from decode_bbox, but the min/max lat/lon indices are mixed for two vertices (NW/SE). This will produce an incorrectly shaped / flipped polygon. Verify decode_bbox return order and build vertices in consistent [lon, lat] order: SW, NW, NE, SE, SW.<br> - [ ] <a href='https://github.com/apache/superset/pull/37027/files#diff-028e07b18b264f0cfd26c72df29e3ab4d079b3da6a42bd1d9c2e902ebee09293R176-R178'><strong>Debug log present</strong></a><br>There's a console.log of `chartProps` left in the transform function. This can leak large objects to client logs and harm performance; remove before merging.<br> - [ ] <a href='https://github.com/apache/superset/pull/37027/files#diff-033bc3956cc357516a15a61af6410a39e130bc05c907f7d8bede788f84f09d71R72-R101'><strong>Hardcoded expectations</strong></a><br>The test expects exactly 4 geohashes and 5 corners each (total 20). If source data or decoding changes (closed ring omitted/added), this will break. Prefer asserting features count and per-feature polygon length explicitly so failures are clearer and less brittle.<br> - [ ] <a href='https://github.com/apache/superset/pull/37027/files#diff-033bc3956cc357516a15a61af6410a39e130bc05c907f7d8bede788f84f09d71R72-R309'><strong>Missing negative cases</strong></a><br>There's no test coverage for invalid or null geohash values (e.g., empty string, null, malformed geohash). Add tests to ensure transformProps handles invalid geohashes gracefully (skips or returns no polygon) to avoid runtime errors.<br> - [ ] <a href='https://github.com/apache/superset/pull/37027/files#diff-033bc3956cc357516a15a61af6410a39e130bc05c907f7d8bede788f84f09d71R292-R309'><strong>Flaky assertion</strong></a><br>The test flattens polygons using optional chaining (p?.polygon). If any feature has undefined polygon, flatMap will include undefined entries and make the length assertion flaky. Use a deterministic mapping that returns an array (e.g. p?.polygon ?? []) or assert per-feature polygon lengths.<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]
