codeant-ai-for-open-source[bot] commented on code in PR #38416:
URL: https://github.com/apache/superset/pull/38416#discussion_r2931292560
##########
superset/mcp_service/dashboard/tool/list_dashboards.py:
##########
@@ -155,3 +196,83 @@ def _serialize_dashboard(
return result.model_dump(
mode="json", context={"select_columns": columns_to_filter}
)
+
+
+def _list_dashboards_by_popularity(
+ request: ListDashboardsRequest,
+ dao_class: Any,
+ serializer: Callable[..., dict[str, Any] | None],
+ all_columns: list[str],
+ ctx: Context,
+) -> DashboardList:
+ """Two-pass listing: sort all matching dashboards by popularity score."""
+ from superset.mcp_service.common.schema_discovery import (
+ DASHBOARD_SORTABLE_COLUMNS,
+ )
+
+ sorted_ids, scores, total_count = get_popularity_sorted_ids(
+ compute_fn=compute_dashboard_popularity,
+ dao_class=dao_class,
+ filters=request.filters,
+ search=request.search,
+ search_columns=DASHBOARD_SEARCH_COLUMNS,
+ order_direction=request.order_direction,
+ )
+
+ # Apply pagination to sorted IDs
+ page = max(request.page - 1, 0)
+ page_size = request.page_size
+ start = page * page_size
+ end = start + page_size
+
+ # Fetch full models for page IDs
+ if page_ids := sorted_ids[start:end]:
+ items = dao_class.find_by_ids(page_ids)
+ id_to_item = {item.id: item for item in items}
+ ordered_items = [id_to_item[pid] for pid in page_ids if pid in
id_to_item]
+ else:
+ ordered_items = []
+
+ # Serialize - preserve the original request columns for response filtering
+ columns_requested = request.select_columns or DEFAULT_DASHBOARD_COLUMNS
Review Comment:
**Suggestion:** When sorting by popularity without explicit
`select_columns`, the response metadata and field filtering omit
`popularity_score`, so clients cannot see the score they explicitly asked to
sort by. Include `popularity_score` in the default requested columns for the
popularity-sort path. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Popularity-sorted dashboard responses hide computed score field.
- ⚠️ MCP clients cannot display numeric ranking evidence.
```
</details>
```suggestion
columns_requested = request.select_columns or (
DEFAULT_DASHBOARD_COLUMNS + ["popularity_score"]
)
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Invoke MCP tool `list_dashboards` with `order_column="popularity_score"`
and no
`select_columns` (real entrypoint is `list_dashboards()` at
`superset/mcp_service/dashboard/tool/list_dashboards.py:82`, registered via
`@tool` at
line 80; same invocation pattern is exercised in tests via
`Client.call_tool(...)` at
`tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_tools.py:102-106`).
2. Request enters popularity branch (`if request.order_column ==
"popularity_score"`) at
`list_dashboards.py:133-137`, then `_list_dashboards_by_popularity()` sets
`columns_requested` to defaults only (`line 237`) and still
computes/attaches popularity
(`lines 241-242`, `250`).
3. Back in `list_dashboards()`, response filtering uses `columns_to_filter =
original_select_columns or result.columns_requested` (`line 190`), so with
no explicit
`select_columns`, filter list is default columns without `popularity_score`.
4. `DashboardInfo` serializer drops fields not in `select_columns` context
(`superset/mcp_service/dashboard/schemas.py:375-380`), so `popularity_score`
is removed
from each dashboard despite being computed and used for sorting.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/dashboard/tool/list_dashboards.py
**Line:** 237:237
**Comment:**
*Logic Error: When sorting by popularity without explicit
`select_columns`, the response metadata and field filtering omit
`popularity_score`, so clients cannot see the score they explicitly asked to
sort by. Include `popularity_score` in the default requested columns for the
popularity-sort path.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38416&comment_hash=d61634f7f3a5c4e872ab09524459f8c7b74bde943c47274937fcf71caa2c8c37&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38416&comment_hash=d61634f7f3a5c4e872ab09524459f8c7b74bde943c47274937fcf71caa2c8c37&reaction=dislike'>👎</a>
--
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]
