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

   ## 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/37065/files#diff-1bbe96474254d89b21e3b373e1c9e8a222ee892518595a69a809c5f035e5bd4eR139-R165'><strong>Behavioral
 Bug</strong></a><br>The PR removed the else branch that pushed out-of-scope 
filters into `filtersOutOfScope`
   but only added explanatory comments — there is no code that adds filters 
lacking explicit
   tab scope into `filtersOutOfScope`. As a result `filtersOutOfScope` will 
always be empty,
   likely breaking the UI that expects non-tab-scoped filters to appear under 
"Filters out of scope".<br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/37065/files#diff-1bbe96474254d89b21e3b373e1c9e8a222ee892518595a69a809c5f035e5bd4eR157-R161'><strong>Missing
 scope check / constant</strong></a><br>The comments reference `ROOT_ID` and 
rootPath checks, but the implementation does not
   check `filter.scope.rootPath` or import/define `ROOT_ID`. Ensure the code 
implements the
   intended logic: only show non-tab-scoped filters in `filtersOutOfScope` 
while completely hiding
   filters explicitly scoped to inactive tabs/sub-tabs. Also ensure dividers 
are not added.<br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/37065/files#diff-0bf03082d8856a78983ed766e4fd7dd265638d05947334a4f162243016bcca84R260-R282'><strong>Behavioral
 Impact on Other Tests</strong></a><br>Changing the behavior to completely hide 
out-of-scope tab/sub-tab filters is a behavior-level change. Verify other tests 
and consumers that previously expected filters under "filters out of scope" are 
updated accordingly — this change may cause other tests or UI code to fail if 
they relied on previous behavior.<br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/37065/files#diff-0bf03082d8856a78983ed766e4fd7dd265638d05947334a4f162243016bcca84R273-R281'><strong>Test
 Assertion Robustness</strong></a><br>The new assertions hide out-of-scope 
filters by expecting an empty "filtersOutOfScope" array and absence from 
in-scope. Consider strengthening assertions to ensure the returned tuple shape 
and contents are exactly what the production code should return (both arrays), 
reducing potential false positives or future regressions.<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