aminghadersohi commented on code in PR #38416:
URL: https://github.com/apache/superset/pull/38416#discussion_r2931934573
##########
superset/mcp_service/common/schema_discovery.py:
##########
@@ -449,6 +465,13 @@ def get_columns_from_model(
"charts": ColumnMetadata(
name="charts", description="Charts in dashboard", type="list",
is_default=False
),
+ "popularity_score": ColumnMetadata(
+ name="popularity_score",
+ description="Popularity score based on views, favorites, chart count, "
+ "and recency",
Review Comment:
Valid. Updated the dashboard `popularity_score` description to include
certification and published status bonuses. Fixed in the latest commit.
##########
superset/mcp_service/dataset/schemas.py:
##########
@@ -137,6 +137,12 @@ class DatasetInfo(BaseModel):
is_favorite: bool | None = Field(
None, description="Whether this dataset is favorited by the current
user"
)
+ popularity_score: float | None = Field(
+ None,
+ description="Popularity score based on chart usage, certification, "
+ "and recency. Request via select_columns=['popularity_score'] or sort "
+ "via order_column='popularity_score'.",
+ )
model_config = ConfigDict(
Review Comment:
Valid. Added `compute_dataset_popularity()` call in `get_dataset_info` so
single-dataset retrievals now populate `popularity_score`. Fixed in the latest
commit.
##########
superset/mcp_service/dashboard/tool/list_dashboards.py:
##########
@@ -108,34 +124,53 @@ def _serialize_dashboard(
"""Serialize dashboard object (field filtering handled by
model_serializer)."""
return serialize_dashboard_object(obj)
- tool = ModelListCore(
- dao_class=DashboardDAO,
- output_schema=DashboardInfo,
- item_serializer=_serialize_dashboard,
- filter_type=DashboardFilter,
- default_columns=DEFAULT_DASHBOARD_COLUMNS,
- search_columns=[
- "dashboard_title",
- "slug",
- "uuid",
- ],
- list_field_name="dashboards",
- output_list_schema=DashboardList,
- all_columns=all_columns,
- sortable_columns=DASHBOARD_SORTABLE_COLUMNS,
- logger=logger,
- )
-
- with event_logger.log_context(action="mcp.list_dashboards.query"):
- result = tool.run_tool(
- filters=request.filters,
- search=request.search,
- select_columns=request.select_columns,
- order_column=request.order_column,
- order_direction=request.order_direction,
- page=max(request.page - 1, 0),
- page_size=request.page_size,
+ # Two-pass approach when sorting by popularity_score
+ if request.order_column == "popularity_score":
+ with
event_logger.log_context(action="mcp.list_dashboards.popularity_sort"):
+ result = _list_dashboards_by_popularity(
+ request, DashboardDAO, _serialize_dashboard, all_columns, ctx
+ )
+ else:
+ list_core = ModelListCore(
+ dao_class=DashboardDAO,
+ output_schema=DashboardInfo,
+ item_serializer=_serialize_dashboard,
+ filter_type=DashboardFilter,
+ default_columns=DEFAULT_DASHBOARD_COLUMNS,
+ search_columns=DASHBOARD_SEARCH_COLUMNS,
+ list_field_name="dashboards",
+ output_list_schema=DashboardList,
+ all_columns=all_columns,
+ sortable_columns=DASHBOARD_SORTABLE_COLUMNS,
+ logger=logger,
)
+
+ # Strip computed fields before passing to DAO query
+ dao_columns = request.select_columns
+ if dao_columns:
+ dao_columns = [c for c in dao_columns if c != "popularity_score"]
+ # Ensure id is loaded when popularity_score was requested
+ # (scores are keyed by id)
+ if "popularity_score" in request.select_columns and "id" not in
dao_columns:
+ dao_columns = ["id"] + dao_columns
+
+ with event_logger.log_context(action="mcp.list_dashboards.query"):
+ result = list_core.run_tool(
+ filters=request.filters,
+ search=request.search,
+ select_columns=dao_columns,
+ order_column=request.order_column,
+ order_direction=request.order_direction,
+ page=max(request.page - 1, 0),
+ page_size=request.page_size,
+ )
+
+ # Attach popularity scores if requested in select_columns
Review Comment:
Valid. When sorting by popularity_score, the response now includes
`popularity_score` in `columns_requested` even if the user didn't explicitly
request it, so clients can see the sort key. Applied to all three list tools.
Fixed in the latest commit.
##########
superset/mcp_service/chart/tool/list_charts.py:
##########
@@ -71,7 +92,7 @@ async def list_charts(request: ListChartsRequest, ctx:
Context) -> ChartList:
modified time.
Sortable columns for order_column: id, slice_name, viz_type,
- datasource_name, description, changed_on, created_on
+ datasource_name, description, changed_on, created_on, popularity_score
Review Comment:
Valid. Removed `datasource_name` from the docstring since it's not in
`CHART_SORTABLE_COLUMNS`. Fixed in the latest commit.
##########
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:
Valid. When sorting by popularity without explicit `select_columns`,
`popularity_score` is now automatically included in `columns_requested` so it
appears in the response. Applied to all three list tools. Fixed in the latest
commit.
--
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]
