codeant-ai-for-open-source[bot] commented on code in PR #39922: URL: https://github.com/apache/superset/pull/39922#discussion_r3398599146
########## superset/mcp_service/chart/plugins/big_number.py: ########## @@ -0,0 +1,247 @@ +# 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. + +"""Big number chart type plugin.""" + +from __future__ import annotations + +from typing import Any + +from superset.mcp_service.chart.chart_utils import ( + _big_number_chart_what, + _summarize_filters, + is_column_truly_temporal, + map_big_number_config, +) +from superset.mcp_service.chart.plugin import BaseChartPlugin +from superset.mcp_service.chart.schemas import BigNumberChartConfig, ColumnRef +from superset.mcp_service.chart.validation.dataset_validator import DatasetValidator +from superset.mcp_service.common.error_schemas import ChartGenerationError + + +class BigNumberChartPlugin(BaseChartPlugin): + """Plugin for big_number chart type.""" + + chart_type = "big_number" + display_name = "Big Number" + native_viz_types = { + "big_number": "Big Number with Trendline", + "big_number_total": "Big Number", + } + + def pre_validate( + self, + config: dict[str, Any], + ) -> ChartGenerationError | None: + if "metric" not in config: + return ChartGenerationError( + error_type="missing_metric", + message="Big Number chart missing required field: metric", + details=( + "Big Number charts require a 'metric' field " + "specifying the value to display" + ), + suggestions=[ + "Add 'metric' with name and aggregate: " + "{'name': 'revenue', 'aggregate': 'SUM'}", + "The aggregate function is required (SUM, COUNT, AVG, MIN, MAX)", + "Example: {'chart_type': 'big_number', " + "'metric': {'name': 'sales', 'aggregate': 'SUM'}}", + ], + error_code="MISSING_BIG_NUMBER_METRIC", + ) + + metric = config.get("metric", {}) + if not isinstance(metric, dict): + return ChartGenerationError( + error_type="invalid_metric_type", + message="Big Number metric must be a dict with 'name' and 'aggregate'", + details=( + f"The 'metric' field must be an object, got {type(metric).__name__}" + ), + suggestions=[ + "Use a dict: {'name': 'col', 'aggregate': 'SUM'}", + "Valid aggregates: SUM, COUNT, AVG, MIN, MAX", + ], + error_code="INVALID_BIG_NUMBER_METRIC_TYPE", + ) + if metric.get("sql_expression"): + label = metric.get("label") + if not isinstance(label, str) or not label.strip(): + return ChartGenerationError( + error_type="missing_sql_metric_label", + message="SQL expression metrics require a non-empty 'label'", + details=( + "When using a custom SQL expression as the Big Number metric, " + "a human-readable 'label' string is required so Superset can " + "display the metric name." + ), + suggestions=[ + "Add 'label': e.g. {'sql_expression': 'SUM(a)/SUM(b)', " + "'label': 'Conversion Rate'}", + "The label must be a non-empty string", + ], + error_code="MISSING_SQL_METRIC_LABEL", + ) + elif not metric.get("aggregate") and not metric.get("saved_metric"): + return ChartGenerationError( + error_type="missing_metric_aggregate", + message=( + "Big Number metric must include an aggregate function " + "or reference a saved metric" + ), + details=( + "The metric must have an 'aggregate' field or 'saved_metric': true" + ), + suggestions=[ + "Add 'aggregate': {'name': 'col', 'aggregate': 'SUM'}", + "Or use a saved metric: {'name': 'metric', 'saved_metric': true}", + "Valid aggregates: SUM, COUNT, AVG, MIN, MAX", + ], + error_code="MISSING_BIG_NUMBER_AGGREGATE", + ) + + show_trendline = config.get("show_trendline", False) + temporal_column = config.get("temporal_column") + if show_trendline and not temporal_column: + return ChartGenerationError( + error_type="missing_temporal_column", + message="Trendline requires a temporal column", + details=( + "When 'show_trendline' is True, " + "a 'temporal_column' must be specified" + ), + suggestions=[ + "Add 'temporal_column': 'date_column_name'", + "Or set 'show_trendline': false for number only", + "Use get_dataset_info to find temporal columns", + ], + error_code="MISSING_TEMPORAL_COLUMN", + ) + + return None + + def extract_column_refs(self, config: Any) -> list[ColumnRef]: + if not isinstance(config, BigNumberChartConfig): + return [] + refs: list[ColumnRef] = [config.metric] + # temporal_column is a str field, not a ColumnRef — validate it exists + if config.temporal_column: + refs.append(ColumnRef(name=config.temporal_column)) + if config.filters: + for f in config.filters: + refs.append(ColumnRef(name=f.column)) + return refs + + def to_form_data( + self, config: Any, dataset_id: int | str | None = None + ) -> dict[str, Any]: + return map_big_number_config(config) + + def post_map_validate( + self, + config: Any, + form_data: dict[str, Any], + dataset_id: int | str | None = None, + ) -> ChartGenerationError | None: + """Verify the trendline temporal column is a real temporal SQL type. + + This check was previously baked into map_config_to_form_data() in + chart_utils.py as a special case. Moving it here keeps the dispatcher + clean and makes the constraint explicit and discoverable. + """ + if not isinstance(config, BigNumberChartConfig): + return None + if not (config.show_trendline and config.temporal_column): + return None + + if not is_column_truly_temporal(config.temporal_column, dataset_id): + return ChartGenerationError( + error_type="non_temporal_trendline_column", + message=( + f"Big Number trendline requires a temporal SQL column; " + f"'{config.temporal_column}' is not temporal." + ), + details=( + f"Column '{config.temporal_column}' does not have a temporal " + f"SQL type (DATE, DATETIME, TIMESTAMP). The trendline requires " + f"a true temporal column for DATE_TRUNC to work." + ), + suggestions=[ + "Use get_dataset_info to find columns with temporal SQL types", + "Set 'show_trendline': false to use any column as the metric", + "If the column contains dates stored as integers, " + "consider casting it in a virtual dataset", + ], + error_code="NON_TEMPORAL_TRENDLINE_COLUMN", + ) + + return None + + def generate_name(self, config: Any, dataset_name: str | None = None) -> str: Review Comment: **Suggestion:** Add a docstring describing how the chart title is derived and how filtering context affects the generated name. [custom_rule] **Severity Level:** Minor ⚠️ <details> <summary><b>Why it matters? 🤔 </b></summary> The method is newly added and has no docstring. This is a direct violation of the rule requiring new functions and classes to be documented inline. </details> [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=db89da8698734b8da1a56152afa372bb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=db89da8698734b8da1a56152afa372bb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset/mcp_service/chart/plugins/big_number.py **Line:** 195:195 **Comment:** *Custom Rule: Add a docstring describing how the chart title is derived and how filtering context affects the generated name. 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%2F39922&comment_hash=c53f91555f4c31242aa40c4c1d5ca8ddcc775239598b0a917405dd54bd4389f1&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39922&comment_hash=c53f91555f4c31242aa40c4c1d5ca8ddcc775239598b0a917405dd54bd4389f1&reaction=dislike'>👎</a> ########## superset/mcp_service/chart/plugins/xy.py: ########## @@ -0,0 +1,198 @@ +# 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. + +"""XY chart type plugin (line, bar, area, scatter).""" + +from __future__ import annotations + +import logging +from typing import Any + +from superset.mcp_service.chart.chart_utils import ( + _xy_chart_context, + _xy_chart_what, + map_xy_config, +) +from superset.mcp_service.chart.plugin import BaseChartPlugin +from superset.mcp_service.chart.schemas import ColumnRef, XYChartConfig +from superset.mcp_service.chart.validation.dataset_validator import DatasetValidator +from superset.mcp_service.chart.validation.runtime.cardinality_validator import ( + CardinalityValidator, +) +from superset.mcp_service.chart.validation.runtime.format_validator import ( + FormatTypeValidator, +) +from superset.mcp_service.common.error_schemas import ChartGenerationError + +logger = logging.getLogger(__name__) + + +class XYChartPlugin(BaseChartPlugin): + """Plugin for xy chart type (line, bar, area, scatter).""" + + chart_type = "xy" + display_name = "Line / Bar / Area / Scatter Chart" + native_viz_types = { + "echarts_timeseries_line": "Line Chart", + "echarts_timeseries_bar": "Bar Chart", + "echarts_area": "Area Chart", + "echarts_timeseries_scatter": "Scatter Plot", + } + + def pre_validate( + self, + config: dict[str, Any], + ) -> ChartGenerationError | None: + # x is optional — defaults to dataset's main_dttm_col in map_xy_config + if not config.get("y") and not config.get("metrics"): + return ChartGenerationError( + error_type="missing_xy_fields", + message="XY chart missing required field: 'y' (Y-axis metrics)", + details=( + "XY charts require Y-axis (metrics) specifications. " + "X-axis is optional and defaults to the dataset's primary " + "datetime column when omitted." + ), + suggestions=[ + "Add 'y' field: [{'name': 'metric_column', 'aggregate': 'SUM'}]", + "Example: {'chart_type': 'xy', 'x': {'name': 'date'}, " + "'y': [{'name': 'sales', 'aggregate': 'SUM'}]}", + ], + error_code="MISSING_XY_FIELDS", + ) + + if not isinstance(config.get("y", []), list): + return ChartGenerationError( + error_type="invalid_y_format", + message="Y-axis must be a list of metrics", + details="The 'y' field must be an array of metric specifications", + suggestions=[ + "Wrap Y-axis metric in array: 'y': [{'name': 'column', " + "'aggregate': 'SUM'}]", + "Multiple metrics supported: 'y': [metric1, metric2, ...]", + ], + error_code="INVALID_Y_FORMAT", + ) + + return None + + def extract_column_refs(self, config: Any) -> list[ColumnRef]: + if not isinstance(config, XYChartConfig): + return [] + refs: list[ColumnRef] = [] + if config.x is not None: + refs.append(config.x) + refs.extend(config.y) + if config.group_by: + refs.extend(config.group_by) + if config.filters: + for f in config.filters: + refs.append(ColumnRef(name=f.column)) + return refs + + def to_form_data( + self, config: Any, dataset_id: int | str | None = None + ) -> dict[str, Any]: + return map_xy_config(config, dataset_id=dataset_id) + + def normalize_column_refs(self, config: Any, dataset_context: Any) -> Any: Review Comment: **Suggestion:** Add a docstring documenting the normalization flow and the expected types of both inputs and normalized output. [custom_rule] **Severity Level:** Minor ⚠️ <details> <summary><b>Why it matters? 🤔 </b></summary> This is a newly added method with no docstring, which violates the rule requiring docstrings for new Python functions and classes. </details> [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5c704ac70cb14b39963bf1134d2be19a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=5c704ac70cb14b39963bf1134d2be19a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset/mcp_service/chart/plugins/xy.py **Line:** 112:112 **Comment:** *Custom Rule: Add a docstring documenting the normalization flow and the expected types of both inputs and normalized output. 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%2F39922&comment_hash=98dae0bf0db487481a26d391311ed6b9ae9642ad2c4979aabd668f0c04582f4f&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39922&comment_hash=98dae0bf0db487481a26d391311ed6b9ae9642ad2c4979aabd668f0c04582f4f&reaction=dislike'>👎</a> ########## superset/mcp_service/chart/plugins/table.py: ########## @@ -0,0 +1,132 @@ +# 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. + +"""Table chart type plugin.""" + +from __future__ import annotations + +from typing import Any + +from superset.mcp_service.chart.chart_utils import ( + _summarize_filters, + _table_chart_what, + map_table_config, +) +from superset.mcp_service.chart.plugin import BaseChartPlugin +from superset.mcp_service.chart.schemas import ColumnRef, TableChartConfig +from superset.mcp_service.chart.validation.dataset_validator import DatasetValidator +from superset.mcp_service.common.error_schemas import ChartGenerationError + + +class TableChartPlugin(BaseChartPlugin): + """Plugin for table chart type.""" + + chart_type = "table" + display_name = "Table" + native_viz_types = { + "table": "Table", + "ag-grid-table": "Interactive Table", + } + + def pre_validate( + self, + config: dict[str, Any], + ) -> ChartGenerationError | None: + if not config.get("columns"): + return ChartGenerationError( + error_type="missing_columns", + message="Table chart missing required field: columns", + details=( + "Table charts require a 'columns' array to specify which " + "columns to display" + ), + suggestions=[ + "Add 'columns' field with array of column specifications", + "Example: 'columns': [{'name': 'product'}, {'name': 'sales', " + "'aggregate': 'SUM'}]", + "Each column can have optional 'aggregate' for metrics", + ], + error_code="MISSING_COLUMNS", + ) + + if not isinstance(config.get("columns", []), list): + return ChartGenerationError( + error_type="invalid_columns_format", + message="Columns must be a list", + details="The 'columns' field must be an array of column specifications", + suggestions=[ + "Ensure columns is an array: 'columns': [...]", + "Each column should be an object with 'name' field", + ], + error_code="INVALID_COLUMNS_FORMAT", + ) + + return None + + def extract_column_refs(self, config: Any) -> list[ColumnRef]: + if not isinstance(config, TableChartConfig): + return [] + refs: list[ColumnRef] = list(config.columns) + if config.filters: + for f in config.filters: + refs.append(ColumnRef(name=f.column)) + return refs + + def to_form_data( + self, config: Any, dataset_id: int | str | None = None + ) -> dict[str, Any]: Review Comment: **Suggestion:** Add a method docstring that states how chart configuration is converted into form data and how optional dataset identifiers are handled. [custom_rule] **Severity Level:** Minor ⚠️ <details> <summary><b>Why it matters? 🤔 </b></summary> This is a newly added method without a docstring. That matches the custom rule violation for newly introduced Python functions lacking inline documentation. </details> [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9ae9e23308ae44459cd489a0059e4b25&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9ae9e23308ae44459cd489a0059e4b25&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset/mcp_service/chart/plugins/table.py **Line:** 89:91 **Comment:** *Custom Rule: Add a method docstring that states how chart configuration is converted into form data and how optional dataset identifiers are handled. 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%2F39922&comment_hash=b5c8a9ac0c07f7d9420cd04653a14dd5275da04c1a78db213c2cbfe70249cc20&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39922&comment_hash=b5c8a9ac0c07f7d9420cd04653a14dd5275da04c1a78db213c2cbfe70249cc20&reaction=dislike'>👎</a> ########## superset/mcp_service/chart/plugin.py: ########## @@ -0,0 +1,263 @@ +# 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. + +""" +ChartTypePlugin protocol and BaseChartPlugin base class. + +Each chart type owns its pre-validation, column extraction, form_data mapping, +and post-map validation in a single plugin class. This eliminates the previous +pattern of 4 separate dispatch points (schema_validator.py, dataset_validator.py, +chart_utils.py, pipeline.py) that had to be updated in sync whenever a new chart +type was added. +""" + +from __future__ import annotations + +from typing import Any, Protocol, runtime_checkable + +from superset.mcp_service.chart.schemas import ColumnRef +from superset.mcp_service.common.error_schemas import ChartGenerationError + + +@runtime_checkable +class ChartTypePlugin(Protocol): + """ + Protocol that every chart-type plugin must satisfy. + + Implementing all eight methods in a single class guarantees that adding a + new chart type requires only one new file — the plugin — rather than edits + across multiple separate files. + """ + + #: Discriminator value matching ChartConfig's chart_type field. + chart_type: str + + #: Human-readable name shown to users (e.g. "Line / Bar / Area / Scatter"). + display_name: str + + #: Maps every Superset-internal viz_type this plugin can produce to a + #: user-facing display name, e.g. {"echarts_timeseries_line": "Line Chart"}. + #: Used by the registry to resolve display names for existing charts without + #: needing a separate JSON mapping file. + native_viz_types: dict[str, str] + + def pre_validate( + self, + config: dict[str, Any], + ) -> ChartGenerationError | None: + """ + Early validation of the raw config dict before Pydantic parsing. + + Called by SchemaValidator before attempting to parse the request. + Should check that required top-level keys are present and well-typed. + + Returns None if valid, ChartGenerationError if invalid. + """ + ... + + def extract_column_refs( + self, + config: Any, + ) -> list[ColumnRef]: + """ + Extract all column references from a parsed chart config. + + Called by DatasetValidator to validate that all referenced columns exist + in the dataset. Must cover every field that holds a column name, + including filters. + + Returns a list of ColumnRef objects (may be empty). + """ + ... + + def to_form_data( + self, + config: Any, + dataset_id: int | str | None = None, + ) -> dict[str, Any]: + """ + Map a parsed chart config to Superset's internal form_data dict. + + Replaces the if/elif chain in chart_utils.map_config_to_form_data(). + + Returns a Superset form_data dict ready for caching and rendering. + """ + ... + + def post_map_validate( + self, + config: Any, + form_data: dict[str, Any], + dataset_id: int | str | None = None, + ) -> ChartGenerationError | None: + """ + Validate the mapped form_data after to_form_data() runs. + + Use this for cross-field constraints that can only be checked once + form_data is assembled (e.g. BigNumber trendline requires a temporal + column whose type must be verified against the dataset). + + Returns None if valid, ChartGenerationError if invalid. + """ + ... + + def normalize_column_refs( + self, + config: Any, + dataset_context: Any, + ) -> Any: + """ + Return a new config with column names normalized to canonical dataset casing. + + Called by DatasetValidator.normalize_column_names(). The default + implementation (in BaseChartPlugin) returns the config unchanged; plugins + with column fields override this to fix case sensitivity mismatches. + + Returns a new config object (or the original if no normalization needed). + """ + ... + + def get_runtime_warnings( + self, + config: Any, + dataset_id: int | str, + ) -> list[str]: + """ + Return chart-type-specific runtime warnings (performance, compatibility). + + Called by RuntimeValidator to collect per-type warnings. Warnings are + informational only — they never block chart generation. The default + implementation returns an empty list; plugins override this to emit + chart-type-specific warnings (e.g. XY cardinality checks). + + Returns a list of warning message strings (may be empty). + """ + ... + + def generate_name( + self, + config: Any, + dataset_name: str | None = None, + ) -> str: + """ + Return a descriptive chart name for the given config. + + Called by chart_utils.generate_chart_name(). The name should follow + the standard format conventions documented in that function. Plugins + that do not override this return the generic fallback "Chart". + """ + ... + + def resolve_viz_type(self, config: Any) -> str: + """ + Return the Superset-internal viz_type string for this config. + + Called by chart_utils._resolve_viz_type(). The returned string must + match a registered Superset viz plugin (e.g. "echarts_timeseries_line"). + Plugins that do not override this return "unknown". + """ + ... + + def schema_error_hint(self) -> ChartGenerationError | None: + """ + Return a user-friendly error for Pydantic discriminated-union parse failures. + + Called by SchemaValidator when Pydantic cannot parse the config union and + the chart_type is known. Returning None falls back to the generic error. + """ + ... + + +class BaseChartPlugin: + """ + Base class providing sensible defaults for all ChartTypePlugin methods. + + Concrete plugins extend this and override only what they need. Default + implementations: ``pre_validate`` → None (valid), ``extract_column_refs`` → [], + ``post_map_validate`` → None, ``normalize_column_refs`` → config unchanged, + ``get_runtime_warnings`` → [], ``generate_name`` → "Chart", + ``resolve_viz_type`` → "unknown", ``schema_error_hint`` → None. + ``to_form_data`` raises ``NotImplementedError`` and must be overridden. + """ + + chart_type: str = "" + display_name: str = "" + # Class-level dict shared across all subclasses that don't override it. + # Subclasses MUST override this as a class attribute (not mutate in place) + # to avoid corrupting the shared empty-dict default for other plugins. + native_viz_types: dict[str, str] = {} + + def pre_validate( + self, + config: dict[str, Any], + ) -> ChartGenerationError | None: + return None + + def extract_column_refs( + self, + config: Any, + ) -> list[ColumnRef]: + return [] Review Comment: **Suggestion:** Add a method docstring that explains what is extracted and why the default implementation returns an empty list. [custom_rule] **Severity Level:** Minor ⚠️ <details> <summary><b>Why it matters? 🤔 </b></summary> This newly added method lacks a docstring in the final file. Since the rule requires new Python functions and classes to include docstrings, the suggestion correctly identifies a violation. </details> [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=82dd324f7a06483e9ae3d7005d63313f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=82dd324f7a06483e9ae3d7005d63313f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset/mcp_service/chart/plugin.py **Line:** 210:214 **Comment:** *Custom Rule: Add a method docstring that explains what is extracted and why the default implementation returns an empty list. 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%2F39922&comment_hash=502005f7c1f08174a2d1be240e8426f6bb67a4f8291b3909ce1de496a30cd7d1&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39922&comment_hash=502005f7c1f08174a2d1be240e8426f6bb67a4f8291b3909ce1de496a30cd7d1&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]
