aminghadersohi commented on code in PR #43737:
URL: https://github.com/apache/superset/pull/43737#discussion_r3944682674
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -469,11 +446,288 @@ def _is_nan(value: Any) -> bool:
return False
+def _gantt_metric_label(metric: Any) -> str | None:
+ """Resolve a native metric result key like frontend ``getMetricLabel``."""
+ if isinstance(metric, str) and metric:
+ return metric
+ if not isinstance(metric, dict) or not 0 < len(metric) <= 20:
+ return None
+
+ label = metric.get("label")
+ if label:
+ return label if isinstance(label, str) else None
+ if label not in (None, ""):
+ return None
+
+ expression_type = metric.get("expressionType")
+ if expression_type == "SIMPLE":
+ aggregate = metric.get("aggregate")
+ column = metric.get("column")
+ if (
+ not isinstance(aggregate, str)
+ or not aggregate
+ or len(aggregate) > 100
+ or not isinstance(column, dict)
+ or not 0 < len(column) <= 50
+ ):
+ return None
+ column_name = column.get("columnName") or column.get("column_name")
+ if not isinstance(column_name, str) or not column_name:
+ return None
+ return f"{aggregate}({column_name})"
+ if expression_type == "SQL":
+ sql_expression = metric.get("sqlExpression")
+ if (
+ isinstance(sql_expression, str)
+ and sql_expression
+ and len(sql_expression) <= 2000
+ ):
+ return sql_expression
+ return None
+
+
+def _gantt_result_field(field: Any, _field_name: str) -> str | None:
+ """Resolve the query-result key using the frontend getColumnLabel
contract."""
+ if isinstance(field, str) and field:
+ return field
+ if not isinstance(field, dict) or not 0 < len(field) <= 20:
+ return None
+ label = field.get("label") or field.get("sqlExpression")
+ return label if isinstance(label, str) and label else None
Review Comment:
Addressed in 1614ed40238e204ee2007ec619411a4fa0e6a841. The shared Gantt
result-field resolver accepts column_name physical-column objects (while
preserving label/sqlExpression precedence), matching the backend query-field
inputs. Parametrized coverage tests start/end/category/series objects plus
tooltip physical columns. Exact-head chart suite: 1,598 passed; focused suite:
321 passed; full PR-range MyPy/Ruff/Pylint/pre-commit passed.
##########
superset/mcp_service/chart/tool/update_chart.py:
##########
@@ -234,8 +235,10 @@ def _build_update_payload(
parsed_config, dataset_id=effective_dataset_id
)
new_form_data.pop("_mcp_warnings", None)
- merge_table_column_config(_get_existing_form_data(chart),
new_form_data)
- merge_interactive_pivot_ui_config(_get_existing_form_data(chart),
new_form_data)
+ existing_form_data = _get_existing_form_data(chart)
+ merge_table_column_config(existing_form_data, new_form_data)
+ merge_interactive_pivot_ui_config(existing_form_data, new_form_data)
+ merge_gantt_ui_config(existing_form_data, new_form_data)
Review Comment:
Addressed in 1614ed40238e204ee2007ec619411a4fa0e6a841. Saved Gantt updates
and their preview now share omitted-filter preservation with cached previews
through preserve_previous_adhoc_filters. Explicit filters, including [],
replace old filters; marked temporal bindings are regenerated, and inactive
native placeholders are replaced. Unsupported merged state still fails
validation rather than being discarded. Regressions assert saved/preview
equality AND retention/replacement semantics, plus changed temporal subject
preservation of unrelated predicates. Exact-head chart suite: 1,598 passed;
full PR-range MyPy/Ruff/Pylint/pre-commit passed.
--
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]
