bito-code-review[bot] commented on code in PR #36458:
URL: https://github.com/apache/superset/pull/36458#discussion_r2666615701
##########
superset/mcp_service/mcp_core.py:
##########
@@ -473,34 +489,110 @@ def _generate_instance_info(self) -> BaseModel:
raise
-class ModelGetAvailableFiltersCore(BaseCore, Generic[S]):
+class ModelGetSchemaCore(BaseCore, Generic[S]):
"""
- Generic tool for retrieving available filterable columns and operators for
a
- model. Used for get_dataset_available_filters, get_chart_available_filters,
- get_dashboard_available_filters, etc.
+ Generic tool for retrieving comprehensive schema metadata for a model type.
+
+ Provides unified schema discovery for list tools:
+ - select_columns: All columns available for selection
+ - filter_columns: Filterable columns with their operators
+ - sortable_columns: Columns valid for order_column
+ - default_columns: Columns returned when select_columns not specified
+ - search_columns: Columns searched by the search parameter
+ - default_sort: Default column for sorting
+ - default_sort_direction: Default sort direction ("asc" or "desc")
+
+ Replaces the individual get_*_available_filters tools with a unified
approach.
"""
def __init__(
self,
+ model_type: Literal["chart", "dataset", "dashboard"],
dao_class: Type[BaseDAO[Any]],
output_schema: Type[S],
+ select_columns: List[Any],
+ sortable_columns: List[str],
+ default_columns: List[str],
+ search_columns: List[str],
+ default_sort: str = "changed_on",
+ default_sort_direction: Literal["asc", "desc"] = "desc",
logger: logging.Logger | None = None,
) -> None:
+ """
+ Initialize the schema discovery core.
+
+ Args:
+ model_type: The type of model (chart, dataset, dashboard)
+ dao_class: The DAO class to query for filter columns
+ output_schema: Pydantic schema for the response (e.g.,
ModelSchemaInfo)
+ select_columns: Column metadata (List[ColumnMetadata] or similar)
+ sortable_columns: Column names that support sorting
+ default_columns: Column names returned by default
+ search_columns: Column names used for text search
+ default_sort: Default sort column
+ default_sort_direction: Default sort direction
+ logger: Optional logger instance
+ """
super().__init__(logger)
+ self.model_type = model_type
self.dao_class = dao_class
self.output_schema = output_schema
+ self.select_columns = select_columns
+ self.sortable_columns = sortable_columns
+ self.default_columns = default_columns
+ self.search_columns = search_columns
+ self.default_sort = default_sort
+ self.default_sort_direction = default_sort_direction
- def run_tool(self) -> S:
+ def _get_filter_columns(self) -> Dict[str, List[str]]:
+ """Get filterable columns and operators from the DAO."""
try:
filterable = self.dao_class.get_filterable_columns_and_operators()
- # Ensure column_operators is a plain dict, not a custom type
- column_operators = dict(filterable)
- response = self.output_schema(column_operators=column_operators)
+ # Defensive handling: ensure we have a valid mapping
+ if filterable is None:
+ return {}
+ # Convert to dict safely - handle both dict and dict-like objects
+ if isinstance(filterable, dict):
+ return dict(filterable)
+ # Try to convert mapping-like objects
+ try:
+ return dict(filterable)
+ except (TypeError, ValueError):
+ self._log_warning(
+ f"Unexpected filter columns type for {self.model_type}: "
+ f"{type(filterable)}"
+ )
+ return {}
+ except Exception as e:
+ self._log_warning(
+ f"Failed to get filter columns for {self.model_type}: {e}"
+ )
+ return {}
+
+ def run_tool(self) -> S:
+ """Execute schema discovery and return comprehensive schema info."""
+ try:
+ filter_columns = self._get_filter_columns()
+
+ response = self.output_schema(
+ model_type=self.model_type,
+ select_columns=self.select_columns,
+ filter_columns=filter_columns,
+ sortable_columns=self.sortable_columns,
+ default_select=self.default_columns,
+ default_sort=self.default_sort,
+ default_sort_direction=self.default_sort_direction,
+ search_columns=self.search_columns,
+ )
+
+ select_count = len(self.select_columns) if self.select_columns
else 0
self._log_info(
- f"Successfully retrieved available filters for "
- f"{self.dao_class.__class__.__name__}"
+ f"Successfully retrieved schema for {self.model_type}: "
+ f"{select_count} select columns, "
+ f"{len(filter_columns)} filter columns, "
+ f"{len(self.sortable_columns)} sortable columns"
)
return response
except Exception as e:
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Blind exception catch too broad</b></div>
<div id="fix">
Replace bare `Exception` catch with specific exception types (e.g.,
`AttributeError`, `TypeError`) to avoid catching unexpected errors.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
except (AttributeError, TypeError, ValueError) as e:
````
</div>
</details>
</div>
<small><i>Code Review Run #1b43c3</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]
