bito-code-review[bot] commented on code in PR #38416:
URL: https://github.com/apache/superset/pull/38416#discussion_r2933372228
##########
superset/mcp_service/dataset/tool/list_datasets.py:
##########
@@ -151,10 +194,11 @@ def _serialize_dataset(
)
)
- # Apply field filtering via serialization context
- # Always use columns_requested (either explicit select_columns or
defaults)
- # This triggers DatasetInfo._filter_fields_by_context for each dataset
- columns_to_filter = result.columns_requested
+ # Build response filtering from the original request columns when the
+ # user explicitly specified select_columns (to avoid leaking internally
+ # injected columns like 'id'). Fall back to result.columns_requested
+ # (which holds DAO defaults) when no explicit columns were requested.
+ columns_to_filter = original_select_columns or result.columns_requested
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Logic Bug: Popularity Score Response Filtering</b></div>
<div id="fix">
When order_column is 'popularity_score' and it's not in the original
select_columns, the current logic uses original_select_columns for filtering,
which excludes popularity_score from the response. However, the code comment in
_list_datasets_by_popularity indicates it should be included so clients can see
the sort key. Change to: columns_to_filter = result.columns_requested if
request.order_column == 'popularity_score' else original_select_columns or
result.columns_requested
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- a/superset/mcp_service/dataset/tool/list_datasets.py
+++ b/superset/mcp_service/dataset/tool/list_datasets.py
@@ -197,7 +197,7 @@
# Build response filtering from the original request columns when
the
# user explicitly specified select_columns (to avoid leaking
internally
# injected columns like 'id'). Fall back to
result.columns_requested
# (which holds DAO defaults) when no explicit columns were
requested.
- columns_to_filter = original_select_columns or
result.columns_requested
+ columns_to_filter = result.columns_requested if
request.order_column == "popularity_score" else original_select_columns or
result.columns_requested
await ctx.debug(
"Applying field filtering via serialization context:
columns=%s"
% (columns_to_filter,)
)
```
</div>
</details>
</div>
<small><i>Code Review Run #a054b2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
