codeant-ai-for-open-source[bot] commented on code in PR #44393: URL: https://github.com/apache/superset/pull/44393#discussion_r4053931587
########## superset/mcp_service/chart/datasource_resolver.py: ########## @@ -0,0 +1,279 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Resolve authorized semantic chart targets and validate saved references.""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any, cast, TYPE_CHECKING +from uuid import UUID + +from superset.daos.datasource import DatasourceDAO +from superset.daos.exceptions import DatasourceNotFound, DatasourceValueIsIncorrect +from superset.exceptions import SupersetSecurityException +from superset.mcp_service.chart.schemas import ChartConfig, ColumnRef +from superset.mcp_service.chart.validation.dataset_validator import DatasetValidator +from superset.mcp_service.common.error_schemas import ( + ChartGenerationError, + DatasetContext, +) +from superset.utils import json +from superset.utils.core import DatasourceType + +if TYPE_CHECKING: + from superset.semantic_layers.models import SemanticView + +SEMANTIC_VIEW_ADHOC_ERROR: str = "semantic_view_adhoc_not_supported" +VIEW_NOT_FOUND_ERROR: str = "view_not_found" + + +@dataclass(frozen=True) +class ChartDatasource: + """Carry an authorized semantic view and its family-qualified identity.""" + + explorable: SemanticView + datasource_type: DatasourceType + id: int + name: str + + @property + def form_data_datasource(self) -> str: + """Return the source identity expected by Explore.""" + return f"{self.id}__{self.datasource_type.value}" + + @property + def explore_url_path(self) -> str: + """Return the basic Explore path for the resolved source.""" + return ( + f"/explore/?datasource_type={self.datasource_type.value}" + f"&datasource_id={self.id}" + ) + + +def resolve_semantic_view(view_id: int | str | UUID) -> ChartDatasource | None: + """Resolve and authorize a view before exposing metadata.""" + # The registry's selected model is SemanticView; no cross-family fallback. + try: + view: SemanticView = cast( + "SemanticView", + DatasourceDAO.get_datasource( + DatasourceType.SEMANTIC_VIEW, + str(view_id) if isinstance(view_id, UUID) else view_id, + ), + ) + view.raise_for_access() + except (DatasourceNotFound, DatasourceValueIsIncorrect, SupersetSecurityException): + return None + + return ChartDatasource( + explorable=view, + datasource_type=DatasourceType.SEMANTIC_VIEW, + id=view.id, + name=view.name, + ) + + +def build_context_from_explorable(target: ChartDatasource) -> DatasetContext: + """Adapt authorized host metadata without assuming table columns.""" + view: SemanticView = target.explorable + return DatasetContext( + id=target.id, + table_name=target.name, + schema=None, + database_name="", + available_columns=[ + { + "name": column.column_name, + "type": column.type, + "is_temporal": column.is_dttm, + "is_numeric": False, + } + for column in view.columns + ], + available_metrics=[ + { + "name": metric.metric_name, + "expression": metric.expression, + "description": metric.description, + } + for metric in view.metrics + ], + ) + + +def view_not_found_error(view_id: int | str | UUID) -> ChartGenerationError: + """Describe a missing or inaccessible view without protected metadata.""" + return ChartGenerationError( + error_type=VIEW_NOT_FOUND_ERROR, + error_code="MCP_SEMANTIC_VIEW_NOT_FOUND", + message=f"Semantic view not found: {view_id}.", + details=( + "No matching semantic view is visible to you; " + "view and dataset IDs are unrelated." + ), + suggestions=[ + "Use the view_id returned by list_metrics for the semantic view", + "Check that you have access to the semantic view", + ], + ) + + +def validate_semantic_view_config( + config: ChartConfig, target: ChartDatasource +) -> tuple[bool, ChartGenerationError | None, DatasetContext]: + """Reject ad-hoc expressions and validate saved names against the view.""" + context: DatasetContext = build_context_from_explorable(target) + ref: ColumnRef + for ref in DatasetValidator._extract_column_references(config): + if ref.aggregate or ref.sql_expression: + expression: str = ( + f"{ref.aggregate}({ref.name})" + if ref.aggregate + else str(ref.sql_expression) + ) + return ( + False, + ChartGenerationError( + error_type=SEMANTIC_VIEW_ADHOC_ERROR, + error_code="MCP_SEMANTIC_VIEW_ADHOC_NOT_SUPPORTED", + message=( + f"Ad-hoc expression '{expression}' is not supported " + "on semantic views." + ), + details="Use the view's predefined metrics and dimensions.", + suggestions=[ + 'Use {"name": "<metric>", "saved_metric": true}', + "Use get_compatible_dimensions to select dimensions", + "Saved metrics: " + + ", ".join( + metric["name"] for metric in context.available_metrics[:15] + ), + ], + ), + context, + ) + valid: bool + error: ChartGenerationError | None + valid, error = DatasetValidator.validate_against_dataset( + config, target.id, dataset_context=context + ) + return valid, error, context + + +def validate_semantic_view_form_data( + form_data: dict[str, Any], target: ChartDatasource +) -> ChartGenerationError | None: + """Validate retained query roles before a semantic rebind or preview.""" + context: DatasetContext = build_context_from_explorable(target) + metrics: set[str] = {metric["name"] for metric in context.available_metrics} + columns: set[str] = {column["name"] for column in context.available_columns} + filter_error: ChartGenerationError | None = _validate_semantic_filters(form_data) + if filter_error is not None: + return filter_error + metric_values: list[object] = list(form_data.get("metrics") or []) + [ + form_data[key] + for key in ("metric", "secondary_metric", "timeseries_limit_metric") + if form_data.get(key) is not None + ] + if any( + not isinstance(value, str) or value not in metrics for value in metric_values + ): + return ChartGenerationError( + error_type=SEMANTIC_VIEW_ADHOC_ERROR, + error_code="MCP_SEMANTIC_VIEW_ADHOC_NOT_SUPPORTED", + message="The chart must use saved metrics from the selected view.", + details="A retained metric is not a saved metric on the selected view.", + suggestions=["Provide a complete configuration valid for the target view"], + ) + dimensions: list[object] = [ + dimension + for key in ("groupby", "groupby_b", "columns", "all_columns") + for dimension in form_data.get(key) or [] + ] + [form_data[key] for key in ("x_axis", "granularity_sqla") if form_data.get(key)] Review Comment: **Suggestion:** When retained Explore data stores `groupby` or another column role as a scalar string, this iterates its characters and rejects valid semantic charts. **Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Type error` [](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b841153c8bcf4347b84ec390e927acc3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b841153c8bcf4347b84ec390e927acc3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) <details> <summary><b>Prompt for AI Agent ๐ค </b></summary> ```mdx This is a comment left during a code review. **Path:** superset/mcp_service/chart/datasource_resolver.py **Line:** 202:206 **Comment:** *Type Error: When retained Explore data stores `groupby` or another column role as a scalar string, this iterates its characters and rejects valid semantic charts. 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. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44393&comment_hash=206d50de7c89fc865ce11e1a811c03dcdbddc6d139a0d81b8a950884f5b61ab9&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44393&comment_hash=206d50de7c89fc865ce11e1a811c03dcdbddc6d139a0d81b8a950884f5b61ab9&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]
