From 460992d89b33ba07f3a22cb9a5b4cd83c27a768c Mon Sep 17 00:00:00 2001 From: Amin Ghadersohi Date: Tue, 12 May 2026 02:38:10 -0400 Subject: [PATCH] fix(mcp): improve not-found errors to suggest corresponding list_* tools (#39919) Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../mcp_service/chart/tool/get_chart_data.py | 11 ++++++++-- .../chart/tool/get_chart_preview.py | 21 +++++++++++++++++-- .../mcp_service/chart/tool/update_chart.py | 14 +++++++------ .../tool/add_chart_to_existing_dashboard.py | 10 +++++++-- .../dashboard/tool/generate_dashboard.py | 5 ++++- .../mcp_service/dataset/tool/query_dataset.py | 5 ++++- .../mcp_service/sql_lab/tool/execute_sql.py | 5 ++++- .../sql_lab/tool/open_sql_lab_with_context.py | 3 ++- .../tool/test_open_sql_lab_with_context.py | 3 ++- 9 files changed, 60 insertions(+), 17 deletions(-) diff --git a/superset/mcp_service/chart/tool/get_chart_data.py b/superset/mcp_service/chart/tool/get_chart_data.py index 7421ab0cdc0..a14fdcb73cb 100644 --- a/superset/mcp_service/chart/tool/get_chart_data.py +++ b/superset/mcp_service/chart/tool/get_chart_data.py @@ -46,7 +46,10 @@ from superset.mcp_service.chart.schemas import ( GetChartDataRequest, PerformanceMetadata, ) -from superset.mcp_service.utils import sanitize_for_llm_context +from superset.mcp_service.utils import ( + escape_llm_context_delimiters, + sanitize_for_llm_context, +) from superset.mcp_service.utils.cache_utils import get_cache_status_from_result from superset.mcp_service.utils.oauth2_utils import ( build_oauth2_redirect_message, @@ -199,8 +202,12 @@ async def get_chart_data( # noqa: C901 if not chart: await ctx.warning("Chart not found: identifier=%s" % (request.identifier,)) + safe_id = escape_llm_context_delimiters(str(request.identifier)[:200]) return ChartError( - error=f"No chart found with identifier: {request.identifier}", + error=( + f"No chart found with identifier: {safe_id}." + " Use list_charts to get valid chart IDs." + ), error_type="NotFound", ) diff --git a/superset/mcp_service/chart/tool/get_chart_preview.py b/superset/mcp_service/chart/tool/get_chart_preview.py index 7ab8b29473a..33281ec6fb2 100644 --- a/superset/mcp_service/chart/tool/get_chart_preview.py +++ b/superset/mcp_service/chart/tool/get_chart_preview.py @@ -47,7 +47,10 @@ from superset.mcp_service.chart.schemas import ( URLPreview, VegaLitePreview, ) -from superset.mcp_service.utils import sanitize_for_llm_context +from superset.mcp_service.utils import ( + escape_llm_context_delimiters, + sanitize_for_llm_context, +) from superset.mcp_service.utils.oauth2_utils import ( build_oauth2_redirect_message, OAUTH2_CONFIG_ERROR_MESSAGE, @@ -1201,8 +1204,22 @@ async def _get_chart_preview_internal( # noqa: C901 if not chart: await ctx.warning("Chart not found: identifier=%s" % (request.identifier,)) + is_form_data_key = ( + isinstance(request.identifier, str) + and len(request.identifier) > 8 + and not request.identifier.isdigit() + ) + if is_form_data_key: + recovery = ( + "If using a form_data_key, it may have expired — " + "use generate_explore_link to get a fresh key, " + "or use list_charts to find a saved chart by ID." + ) + else: + recovery = "Use list_charts to get valid chart IDs." + safe_id = escape_llm_context_delimiters(str(request.identifier)[:200]) return ChartError( - error=f"No chart found with identifier: {request.identifier}", + error=f"No chart found with identifier: {safe_id}. {recovery}", error_type="NotFound", ) diff --git a/superset/mcp_service/chart/tool/update_chart.py b/superset/mcp_service/chart/tool/update_chart.py index 3e3057bdd2e..9fd0986a29a 100644 --- a/superset/mcp_service/chart/tool/update_chart.py +++ b/superset/mcp_service/chart/tool/update_chart.py @@ -47,6 +47,7 @@ from superset.mcp_service.chart.schemas import ( PerformanceMetadata, UpdateChartRequest, ) +from superset.mcp_service.utils import escape_llm_context_delimiters from superset.mcp_service.utils.oauth2_utils import ( build_oauth2_redirect_message, OAUTH2_CONFIG_ERROR_MESSAGE, @@ -337,17 +338,18 @@ async def update_chart( # noqa: C901 chart = find_chart_by_identifier(request.identifier) if not chart: + safe_id = escape_llm_context_delimiters(str(request.identifier)[:200]) + not_found_msg = ( + f"No chart found with identifier: {safe_id}." + " Use list_charts to get valid chart IDs." + ) return GenerateChartResponse.model_validate( { "chart": None, "error": { "error_type": "NotFound", - "message": ( - f"No chart found with identifier: {request.identifier}" - ), - "details": ( - f"No chart found with identifier: {request.identifier}" - ), + "message": not_found_msg, + "details": not_found_msg, }, "success": False, "schema_version": "2.0", diff --git a/superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py b/superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py index 026275c9e4b..72fcd482a55 100644 --- a/superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py +++ b/superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py @@ -334,7 +334,10 @@ def _find_and_authorize_dashboard( dashboard=None, dashboard_url=None, position=None, - error=f"Dashboard with ID {dashboard_id} not found", + error=( + f"Dashboard with ID {dashboard_id} not found." + " Use list_dashboards to get valid dashboard IDs." + ), ) try: @@ -392,7 +395,10 @@ def add_chart_to_existing_dashboard( dashboard=None, dashboard_url=None, position=None, - error=f"Chart with ID {request.chart_id} not found", + error=( + f"Chart with ID {request.chart_id} not found." + " Use list_charts to get valid chart IDs." + ), ) # Validate dataset access for the chart. diff --git a/superset/mcp_service/dashboard/tool/generate_dashboard.py b/superset/mcp_service/dashboard/tool/generate_dashboard.py index 968d5ca74f2..31c038e05a7 100644 --- a/superset/mcp_service/dashboard/tool/generate_dashboard.py +++ b/superset/mcp_service/dashboard/tool/generate_dashboard.py @@ -230,7 +230,10 @@ def generate_dashboard( # noqa: C901 return GenerateDashboardResponse( dashboard=None, dashboard_url=None, - error=f"Charts not found: {list(missing_chart_ids)}", + error=( + f"Charts not found: {sorted(missing_chart_ids)}." + " Use list_charts to get valid chart IDs." + ), ) # Validate dataset access for each chart. diff --git a/superset/mcp_service/dataset/tool/query_dataset.py b/superset/mcp_service/dataset/tool/query_dataset.py index d62c7fd9d2d..da7f2d227f6 100644 --- a/superset/mcp_service/dataset/tool/query_dataset.py +++ b/superset/mcp_service/dataset/tool/query_dataset.py @@ -183,7 +183,10 @@ async def query_dataset( # noqa: C901 if dataset is None: await ctx.error("Dataset not found: identifier=%s" % (request.dataset_id,)) return DatasetError.create( - error=f"No dataset found with identifier: {request.dataset_id}", + error=( + f"No dataset found with identifier: {request.dataset_id}." + " Use list_datasets to get valid dataset IDs." + ), error_type="NotFound", ) diff --git a/superset/mcp_service/sql_lab/tool/execute_sql.py b/superset/mcp_service/sql_lab/tool/execute_sql.py index c1000320a4d..b5f01e8afa7 100644 --- a/superset/mcp_service/sql_lab/tool/execute_sql.py +++ b/superset/mcp_service/sql_lab/tool/execute_sql.py @@ -100,7 +100,10 @@ async def execute_sql(request: ExecuteSqlRequest, ctx: Context) -> ExecuteSqlRes ) return ExecuteSqlResponse( success=False, - error=f"Database with ID {request.database_id} not found", + error=( + f"Database with ID {request.database_id} not found." + " Use list_databases to get valid database IDs." + ), error_type=SupersetErrorType.DATABASE_NOT_FOUND_ERROR.value, ) diff --git a/superset/mcp_service/sql_lab/tool/open_sql_lab_with_context.py b/superset/mcp_service/sql_lab/tool/open_sql_lab_with_context.py index 1d7af0ba6f2..17485b67da3 100644 --- a/superset/mcp_service/sql_lab/tool/open_sql_lab_with_context.py +++ b/superset/mcp_service/sql_lab/tool/open_sql_lab_with_context.py @@ -103,7 +103,8 @@ def open_sql_lab_with_context( database = DatabaseDAO.find_by_id(request.database_connection_id) if not database: error_message = ( - f"Database with ID {request.database_connection_id} not found" + f"Database with ID {request.database_connection_id} not found." + " Use list_databases to get valid database IDs." ) return _sanitize_sql_lab_response_for_llm_context( SqlLabResponse( diff --git a/tests/unit_tests/mcp_service/sql_lab/tool/test_open_sql_lab_with_context.py b/tests/unit_tests/mcp_service/sql_lab/tool/test_open_sql_lab_with_context.py index f6feacc0f01..fd67fd2bff2 100644 --- a/tests/unit_tests/mcp_service/sql_lab/tool/test_open_sql_lab_with_context.py +++ b/tests/unit_tests/mcp_service/sql_lab/tool/test_open_sql_lab_with_context.py @@ -298,7 +298,8 @@ class TestOpenSqlLabWithContext: field_path=("title",), ) assert response.error == sanitize_for_llm_context( - "Database with ID 404 not found", + "Database with ID 404 not found." + " Use list_databases to get valid database IDs.", field_path=("error",), ) finally: