codeant-ai-for-open-source[bot] commented on code in PR #39922:
URL: https://github.com/apache/superset/pull/39922#discussion_r3398668354
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -1517,25 +1555,29 @@ def reject_sql_expression_in_raw_mode(self) ->
"TableChartConfig":
@model_validator(mode="after")
def validate_unique_column_labels(self) -> "TableChartConfig":
"""Ensure all column labels are unique."""
- labels_seen = set()
+ # Key is (saved_metric, label) so a saved metric and a regular column
+ # with the same input name are not flagged as duplicates — saved
metrics
+ # resolve to their actual casing from the dataset during normalization.
+ labels_seen: dict[tuple[bool, str], str] = {}
duplicates = []
for i, col in enumerate(self.columns):
# Generate the label that will be used (same logic as
create_metric_object)
if col.sql_expression:
# SQL metrics carry a required label; use it verbatim.
- label = col.label
+ label = col.label or ""
elif col.saved_metric:
- label = col.label or col.name
+ label = col.label or col.name or ""
elif col.aggregate:
label = col.label or f"{col.aggregate}({col.name})"
else:
- label = col.label or col.name
+ label = col.label or col.name or ""
- if label in labels_seen:
+ key = (col.saved_metric, label)
+ if key in labels_seen:
duplicates.append(f"columns[{i}]: '{label}'")
else:
- labels_seen.add(label)
+ labels_seen[key] = f"columns[{i}]"
Review Comment:
**Suggestion:** The duplicate-label check now keys by `(saved_metric,
label)`, which allows the same rendered label to pass validation when one entry
is a saved metric and the other is not. Superset still treats duplicate display
labels as conflicts, so these requests can pass schema validation and fail
later during query compilation. Deduplicate by final rendered label alone
(after normalization), not by metric source type. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Table configs with mixed metrics can hit duplicate-label errors.
- ⚠️ generate_explore_link may return URLs that fail in Explore.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Construct a `TableChartConfig` (model defined in
`superset/mcp_service/chart/schemas.py:1494-1589`) with `chart_type="table"`
and `columns`
containing two entries that share the same display label but differ in
`saved_metric`,
e.g. `[ColumnRef(name="revenue", saved_metric=True),
ColumnRef(name="revenue",
aggregate="SUM", label="revenue")]`.
2. When this config is parsed (e.g. as part of
`GenerateExploreLinkRequest.config` in
`superset/mcp_service/chart/schemas.py:1979-1988` or
`GenerateChartRequest.config` in
`schemas.py:1878-1904`), the
`TableChartConfig.validate_unique_column_labels()` model
validator at `schemas.py:1555-1589` runs. It computes `label` for each
column and uses
`key = (col.saved_metric, label)` (`schemas.py:1576`), so the two columns
produce distinct
keys `(True, "revenue")` and `(False, "revenue")` and no duplicate is
recorded.
3. In `generate_explore_link`
(`superset/mcp_service/explore/tool/generate_explore_link.py:180-28`), the
accepted
`TableChartConfig` is mapped to Superset `form_data` via `map_table_config()`
(`chart_utils.py:403-499`), where the saved metric becomes a metric named
`"revenue"` and
the ad‑hoc metric uses the explicit label `"revenue"`, yielding two identical
metric/column labels in the query definition.
4. When the resulting `form_data` is later executed through Superset's query
layer (e.g.
via `_compile_chart()` in `superset/mcp_service/chart/compile.py:143-200` or
by visiting
the generated Explore URL),
`QueryObject._validate_no_have_duplicate_labels()` in
`superset/common/query_object.py:15-25` builds `all_labels =
self.metric_names +
self.column_names` and raises `QueryObjectValidationError("Duplicate
column/metric labels:
...")` because `"revenue"` appears twice, even though the schema-level
validator allowed
this configuration.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d5537f139ee84c369f017c9659663dcb&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=d5537f139ee84c369f017c9659663dcb&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/schemas.py
**Line:** 1576:1580
**Comment:**
*Incorrect Condition Logic: The duplicate-label check now keys by
`(saved_metric, label)`, which allows the same rendered label to pass
validation when one entry is a saved metric and the other is not. Superset
still treats duplicate display labels as conflicts, so these requests can pass
schema validation and fail later during query compilation. Deduplicate by final
rendered label alone (after normalization), not by metric source type.
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=e9d91ae1fe82d0fa2e483d179e47a3aa9343a8e238d5d57845cc35bcb996e5e2&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39922&comment_hash=e9d91ae1fe82d0fa2e483d179e47a3aa9343a8e238d5d57845cc35bcb996e5e2&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]
