codeant-ai-for-open-source[bot] commented on PR #36270: URL: https://github.com/apache/superset/pull/36270#issuecomment-3714076359
## 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/36270/files#diff-085c29b647c4eb19021735d0db832c82a2ac4d59ff1ee3021f018e1f2088e268R51-R56'><strong>Assumes cell data is UTC</strong></a><br>The comparator assumes `cellValue` is stored in UTC and extracts UTC date parts (`getUTCFullYear`, etc.). If backend dates are plain local ISO strings (or formatted without timezone), parsing semantics differ across browsers and may cause incorrect comparisons. Validate and/or detect timezone information on `cellValue` before assuming UTC.<br> - [ ] <a href='https://github.com/apache/superset/pull/36270/files#diff-085c29b647c4eb19021735d0db832c82a2ac4d59ff1ee3021f018e1f2088e268R35-R42'><strong>Null/invalid handling behavior change</strong></a><br>The new code returns `-1` for `null` or invalid `cellValue`. That changes behavior relative to earlier code (which implicitly treated invalid values differently). Confirm this is the intended UX for filters (e.g., whether nulls should be considered "before" the filter date) and make the behavior explicit and consistent.<br> - [ ] <a href='https://github.com/apache/superset/pull/36270/files#diff-c79b9936e5acddcee32499d03e4b665107d66d3c7d13f5deb1d1e9fa91fe93aeR43-R61'><strong>Null/invalid value expectations</strong></a><br>Tests expect the comparator to return -1 for null, undefined and invalid dates. Verify this aligns with AG Grid's expected behavior in the product and that any downstream code relying on -1 treats it as "not matched" vs "before".<br> - [ ] <a href='https://github.com/apache/superset/pull/36270/files#diff-c79b9936e5acddcee32499d03e4b665107d66d3c7d13f5deb1d1e9fa91fe93aeR106-R110'><strong>ISO date parsing ambiguity</strong></a><br>The test uses `new Date('2003-10-08')` which can be interpreted differently by JS engines (some treat date-only ISO strings as UTC, others as local). This can produce inconsistent behavior across runtimes.<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]
