codeant-ai-for-open-source[bot] commented on PR #36644: URL: https://github.com/apache/superset/pull/36644#issuecomment-3702889434
## 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/36644/files#diff-f6bef8dec637a227cfa8419c7ff4df853f7acf41d666035cbe0489bff55af5e5R64-R69'><strong>Missing guards / unsafe non-null assertion</strong></a><br>The code uses a non-null assertion when retrieving command contributions and then accesses `command.icon` and `command.command` without runtime checks. If `getCommandContribution` returns undefined or returns a command missing fields, this will throw at runtime. Consider adding guards and fallbacks so missing/invalid contributions don't crash the UI.<br> - [ ] <a href='https://github.com/apache/superset/pull/36644/files#diff-d5127e3fd6e0df25bef3a93a82c006c88f465fc0969a97a9a6636b274ee4d6fcR59-R61'><strong>Missing guard on dispatch</strong></a><br>`onDbChange` calls `dispatch(queryEditorSetDb(queryEditor, dbId))` without verifying `queryEditor` is defined. Other handlers (catalog/schema) guard the dispatch. If `queryEditor` can be undefined this may dispatch an action with an invalid payload and cause reducers or middleware to misbehave.<br> - [ ] <a href='https://github.com/apache/superset/pull/36644/files#diff-d5127e3fd6e0df25bef3a93a82c006c88f465fc0969a97a9a6636b274ee4d6fcR81-R86'><strong>Potential type mismatch for DB list</strong></a><br>`handleDbList` is typed to accept a single `DatabaseObject` but immediately calls `setDatabases(result)` which (elsewhere) expects the full databases collection (likely a map or array). The parameter type may be incorrect and cause incorrect runtime behavior or wrong shape in the store.<br> - [ ] <a href='https://github.com/apache/superset/pull/36644/files#diff-0253e8c09081e3be4b9e5643ceb26e302557bd650ba2257efd1afcfc9e3f548eR260-R270'><strong>Debounced callback cleanup</strong></a><br>The debounced callback `setQueryEditorAndSaveSqlWithDebounce` is created with lodash `debounce` inside a `useMemo` but there is no cancellation/cleanup when the component unmounts. This can lead to stale executions or memory leaks if the debounced function fires after the component has unmounted or after queryEditor changed.<br> - [ ] <a href='https://github.com/apache/superset/pull/36644/files#diff-0253e8c09081e3be4b9e5643ceb26e302557bd650ba2257efd1afcfc9e3f548eR200-R220'><strong>Hotkeys not unbound</strong></a><br>Hotkeys bound with `Mousetrap.bind(...)` in an effect are not unbound when the component is no longer active or on cleanup. If `isActive` toggles multiple times or the component is remounted, duplicate bindings can accumulate and result in double-triggered commands.<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]
