gabotorresruiz commented on code in PR #40448:
URL: https://github.com/apache/superset/pull/40448#discussion_r3311089881
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -444,6 +445,25 @@ def sanitize_chart_info_for_llm_context(chart_info:
ChartInfo) -> ChartInfo:
| frozenset({"cache_key", "database", "database_name",
"schema"})
),
)
+ # ``metrics`` is in the bulk exclusion list (SIMPLE-metric content is
+ # bounded). SQL-metric adhoc dicts carry LLM-controlled strings that
+ # still need ``<UNTRUSTED-CONTENT>`` wrapping.
+ form_data = payload.get("form_data")
+ metrics = form_data.get("metrics") if isinstance(form_data, dict) else
None
+ if isinstance(metrics, list):
+ for index, metric in enumerate(metrics):
+ if isinstance(metric, dict) and metric.get("expressionType")
== "SQL":
+ for key in ("sqlExpression", "label"):
+ if isinstance(metric.get(key), str):
+ metric[key] = sanitize_for_llm_context(
+ metric[key],
+ field_path=(
+ "form_data",
+ "metrics",
+ str(index),
+ key,
+ ),
+ )
Review Comment:
Hey @aminghadersohi Good catch, thanks! I've fixed this by applying your
suggested patch verbatim, plus a
`TestSqlMetricLlmContextWrapping.test_singular_sql_metric_is_wrapped` that
mirrors the existing plural-metrics test.
--
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]
