aminghadersohi commented on code in PR #40448:
URL: https://github.com/apache/superset/pull/40448#discussion_r3306445375
##########
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:
`form_data["metric"]` (singular dict, used by BigNumber and Pie charts) is
not handled here. Both `"metric"` and `"metrics"` are in
`CHART_FORM_DATA_EXCLUDED_FIELD_NAMES` and excluded from the recursive
`sanitize_for_llm_context()` call above, but only the `metrics` plural list
gets the special SQL rewrap. A SQL metric on a BigNumber or Pie chart returns
its `sqlExpression` to the LLM unwrapped.
Suggested addition after this block:
```python
metric_singular = form_data.get("metric") if isinstance(form_data, dict)
else None
if isinstance(metric_singular, dict) and
metric_singular.get("expressionType") == "SQL":
for key in ("sqlExpression", "label"):
if isinstance(metric_singular.get(key), str):
metric_singular[key] = sanitize_for_llm_context(
metric_singular[key],
field_path=("form_data", "metric", key),
)
```
Also worth adding a test for BigNumber/Pie SQL metric LLM context wrapping
analogous to `TestSqlMetricLlmContextWrapping`.
--
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]
