codeant-ai-for-open-source[bot] commented on PR #36359: URL: https://github.com/apache/superset/pull/36359#issuecomment-3607113063
## 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/36359/files#diff-1945a2c286811d95a0d624378f0c0166482175ac35b2fece13a5d1ff792a2c5aR94-R98'><strong>Incorrect sort order</strong></a><br>The new comparator sorts by (aTime - bTime) which produces ascending order (oldest first). The PR's intent is to show the most recent queries at the top (descending). Verify and fix the comparator to ensure newest queries appear first.<br> - [ ] <a href='https://github.com/apache/superset/pull/36359/files#diff-1945a2c286811d95a0d624378f0c0166482175ac35b2fece13a5d1ff792a2c5aR95-R97'><strong>startDttm type handling</strong></a><br>The code assumes `startDttm` can be used directly in numeric subtraction and falls back to 0. If `startDttm` is an ISO string or another non-numeric type this can lead to incorrect sorting (NaN) or misordered null/undefined values. Ensure `startDttm` is normalized to a numeric timestamp before sorting.<br> - [ ] <a href='https://github.com/apache/superset/pull/36359/files#diff-8f485cd5ae53e1005fb1bcc12452644710bec6ccbbaa80f77a4547ce92258487R138-R244'><strong>Mock leakage between tests</strong></a><br>The feature-flag mock implementation is set per-test but only cleared with mockClear(), which resets call history but leaves the implementation. That can leak behavior across tests; prefer mockReset()/mockRestore() or a shared afterEach cleanup.<br> - [ ] <a href='https://github.com/apache/superset/pull/36359/files#diff-8f485cd5ae53e1005fb1bcc12452644710bec6ccbbaa80f77a4547ce92258487R211-R216'><strong>Broad fetch mock route</strong></a><br>The fetchMock route `glob:*/api/v1/query/?q=*` is very broad and may match unrelated requests in larger test suites. Narrow the route or assert on the request query parameters to avoid false positives.<br> - [ ] <a href='https://github.com/apache/superset/pull/36359/files#diff-8f485cd5ae53e1005fb1bcc12452644710bec6ccbbaa80f77a4547ce92258487R219-R241'><strong>Flaky test selection</strong></a><br>The test uses global text queries (e.g. getByText('443') / getAllByText('1')) which can match other elements and cause flakiness. Assertions should target specific table rows/cells to assert ordering deterministically.<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]
