You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "aminghadersohi (via GitHub)" <gi...@apache.org> on 2026/08/29 04:22:49 UTC

[PR] fix(mcp): surface rejected filter columns in get_chart_sql [superset]

aminghadersohi opened a new pull request, #43680:
URL: https://github.com/apache/superset/pull/43680

   ## Why
   
   `get_chart_data` fails closed when a filter names a column the dataset does not have: the
   datasource drops the predicate during query construction and the tool returns a
   `ValidationError` rather than unfiltered rows. `get_chart_sql` accepts the same
   `extra_form_data` filters but had no equivalent check, so it returned the unfiltered SQL as a
   success — presenting it as the SQL for filters that were never applied.
   
   The check could not simply be copied across, because the signal was not available: the `query`
   result type discards the `applied_filter_columns` / `rejected_filter_columns` that the
   datasource has already computed while building the statement, keeping only the SQL string. So
   anyone asking for just the SQL — the MCP tool, and the "View query" modal — had no way to tell
   that a filter had been dropped.
   
   ## What
   
   Carry the filter status through the `query` result type using the same `applied_filters` /
   `rejected_filters` entries the data-bearing results already expose, then have `get_chart_sql`
   fail closed on a rejected request filter, matching `get_chart_data`.
   
   `_get_query` now prefers `get_query_str_extended`, which returns that status alongside the SQL;
   datasources that do not implement it (e.g. semantic layers) keep the plain string form.
   `QueryStringExtended.full_sql` holds the prequery-joining logic that `ExploreMixin.get_query_str`
   already had, so there is one definition of it rather than two.
   
   The three filter-column helpers move from `get_chart_data` into `chart_helpers` so both tools
   share one implementation instead of a copy each.
   
   ## Blast radius
   
   `_get_query` is the `query` result type used by the chart-data API and the "View query" modal.
   The change is additive — two extra keys on that payload — and the SQL string itself is produced
   by the same code path as before (`ExploreMixin.get_query_str` was already
   `get_query_str_extended` plus a join). Behavior changes only for `get_chart_sql`, which now
   errors instead of returning unfiltered SQL. No change to query construction or execution.
   
   ## How to test
   
   - `tests/unit_tests/common/test_query_actions.py::test_get_query_surfaces_rejected_filter_columns`
     — the `query` payload reports the rejected column.
   - `..._falls_back_when_datasource_has_no_extended_form` — datasources without the extended form
     keep the old shape.
   - `tests/unit_tests/mcp_service/chart/tool/test_get_chart_sql.py::TestRejectedFilterColumnsAreSurfaced`
     — rejected request filter returns `ValidationError`; a stale filter the caller did not ask for
     does not fail the request; no rejections still returns SQL.
   
   Verified by reverting the `get_chart_sql` change with the tests in place: the three new cases
   fail, the rest pass. Suites run: `tests/unit_tests/mcp_service/`, `tests/unit_tests/common/`,
   `tests/unit_tests/charts/` — 1363 passed locally, plus the wider run clean apart from
   `test_tools_call_health_check_over_real_asgi_transport`, which fails identically on an unmodified
   master.
   
   Three tests in `test_get_data_command.py` mocked `get_query_str` to raise a parse error; they now
   mock `get_query_str_extended`, since that is the method that raises. The assertions are unchanged.
   
   ## Risk & rollback
   
   Low. The new failure mode is an explicit error where the previous behavior was a misleading
   success. A caller depending on unfiltered SQL being returned for an unknown column would see a
   `ValidationError` instead; that response was incorrect. Straight revert if needed.
   
   ## Review guidance
   
   Read `_get_query` in `superset/common/query_actions.py` first — that is the only change outside
   the MCP service, and the `getattr` fallback is the hunk worth scrutinising. Then the guard in
   `_extract_sql_from_result`, which both `get_chart_sql` paths funnel through.
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(mcp): surface rejected filter columns in get_chart_sql [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #43680:
URL: https://github.com/apache/superset/pull/43680#discussion_r3897250519


##########
superset/mcp_service/chart/chart_helpers.py:
##########
@@ -62,6 +62,68 @@ class ChartNotOnDashboardError(ValueError):
     """Raised when a chart is not part of the given dashboard's slices."""
 
 
+def requested_filter_columns(extra_form_data: dict[str, Any] | None) -> set[str]:
+    """Return simple column names explicitly requested through extra form data."""
+    if not extra_form_data:
+        return set()
+
+    columns: set[str] = set()
+    for filter_ in extra_form_data.get("filters", []):
+        if isinstance(filter_, dict) and isinstance(column := filter_.get("col"), str):
+            columns.add(column)
+    for filter_ in extra_form_data.get("adhoc_filters", []):
+        if (
+            isinstance(filter_, dict)
+            and filter_.get("expressionType") == "SIMPLE"
+            and isinstance(column := filter_.get("subject"), str)
+        ):
+            columns.add(column)
+    return columns
+
+
+def rejected_columns_in_query(query: Any) -> set[str]:
+    """Return the rejected filter column names reported by one query payload.
+
+    Query construction reports dropped filters as ``rejected_filters`` entries
+    (``{"reason": ..., "column": ...}``), the shape every consumer of a
+    chart-data or query payload sees. The raw ``rejected_filter_columns`` list
+    is still accepted for payloads captured before that conversion.
+    """
+    if not isinstance(query, dict):
+        return set()
+
+    columns = {
+        column
+        for entry in query.get("rejected_filters", [])
+        if isinstance(entry, dict) and isinstance(column := entry.get("column"), str)
+    }

Review Comment:
   Fixed in c4ad918604. QUERY payloads now preserve the datasource-only rejected column list, and the MCP helper prefers that list over the combined temporal/datasource status. This avoids same-name collisions without hiding real dataset-column rejections. I also added collision coverage.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(mcp): surface rejected filter columns in get_chart_sql [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #43680:
URL: https://github.com/apache/superset/pull/43680#discussion_r3896499493


##########
superset/mcp_service/chart/chart_helpers.py:
##########
@@ -62,6 +62,68 @@ class ChartNotOnDashboardError(ValueError):
     """Raised when a chart is not part of the given dashboard's slices."""
 
 
+def requested_filter_columns(extra_form_data: dict[str, Any] | None) -> set[str]:
+    """Return simple column names explicitly requested through extra form data."""
+    if not extra_form_data:
+        return set()
+
+    columns: set[str] = set()
+    for filter_ in extra_form_data.get("filters", []):
+        if isinstance(filter_, dict) and isinstance(column := filter_.get("col"), str):
+            columns.add(column)
+    for filter_ in extra_form_data.get("adhoc_filters", []):
+        if (
+            isinstance(filter_, dict)
+            and filter_.get("expressionType") == "SIMPLE"
+            and isinstance(column := filter_.get("subject"), str)
+        ):
+            columns.add(column)
+    return columns
+
+
+def rejected_columns_in_query(query: Any) -> set[str]:
+    """Return the rejected filter column names reported by one query payload.
+
+    Query construction reports dropped filters as ``rejected_filters`` entries
+    (``{"reason": ..., "column": ...}``), the shape every consumer of a
+    chart-data or query payload sees. The raw ``rejected_filter_columns`` list
+    is still accepted for payloads captured before that conversion.
+    """
+    if not isinstance(query, dict):
+        return set()
+
+    columns = {
+        column
+        for entry in query.get("rejected_filters", [])
+        if isinstance(entry, dict) and isinstance(column := entry.get("column"), str)
+    }

Review Comment:
   **Suggestion:** This treats rejected temporal metadata such as `__time_range` as a rejected dataset column, so a legitimate request filter with that name can incorrectly return `ValidationError`. [incorrect condition logic]
   
   **Assessment:** 🟠 `Major` · 🔁 `Occurrence: Rarely`
   
   [![Use CodeAnt Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4e22ad97ceda46479f3335ba835da8c2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=4e22ad97ceda46479f3335ba835da8c2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset/mcp_service/chart/chart_helpers.py
   **Line:** 95:99
   **Comment:**
   	*Incorrect Condition Logic: This treats rejected temporal metadata such as `__time_range` as a rejected dataset column, so a legitimate request filter with that name can incorrectly return `ValidationError`.
   
   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%2F43680&comment_hash=34da85f3e7205d0ecea8f5dd10909dbdf82cc6e186d266d228d4dcd7f2fcfbcf&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43680&comment_hash=34da85f3e7205d0ecea8f5dd10909dbdf82cc6e186d266d228d4dcd7f2fcfbcf&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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(mcp): surface rejected filter columns in get_chart_sql [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43680:
URL: https://github.com/apache/superset/pull/43680#issuecomment-5483565097

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers an incremental AI Review.
   - `/review full` - Manually triggers a full AI Review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(mcp): surface rejected filter columns in get_chart_sql [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on PR #43680:
URL: https://github.com/apache/superset/pull/43680#issuecomment-5482737216

   Addressed the review feedback in `c4ad918604` and rebased onto current `master`:
   
   - preserve datasource rejection origin in QUERY payloads so temporal pseudo-filter rejections cannot collide with same-named request filters
   - added the requested same-name `__time_col` collision regression test
   - normalized nullable `filters` / `adhoc_filters` before iteration
   
   Local verification: **198 passed** across the MCP chart data/SQL and query-actions suites; targeted pre-commit hooks all pass.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(mcp): surface rejected filter columns in get_chart_sql [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi merged PR #43680:
URL: https://github.com/apache/superset/pull/43680


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(mcp): surface rejected filter columns in get_chart_sql [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on PR #43680:
URL: https://github.com/apache/superset/pull/43680#issuecomment-5535826392

   Addressed both follow-up suggestions in `f3ede8eb76`:
   
   - data-bearing/materialized results now preserve `rejected_filter_columns` provenance too, preventing the same temporal pseudo-filter collision in `get_chart_data`
   - `_materialize_full_payload` now uses the shared `_filter_status` producer, removing the duplicate status-building logic
   - producer tests assert the carrier for both QUERY and materialized payloads
   
   Also rebased onto current `master`. Local verification: **199 passed**; targeted pre-commit hooks pass.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(mcp): surface rejected filter columns in get_chart_sql [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on PR #43680:
URL: https://github.com/apache/superset/pull/43680#issuecomment-5481471534

   @richardfogaca — assigning you as reviewer here.
   
   **Why you:** you approved #38700 (the PR that introduced `get_chart_sql`) and you've been across the recent chart-data/query-object work (#43520, #43081), so this sits squarely in territory you already know.
   
   **Worth scrutinising:**
   - `_get_query` in `superset/common/query_actions.py` — the `getattr(..., "get_query_str_extended", None)` fallback, and the two new keys on the `query` result payload (also consumed by the "View query" modal, not just MCP).
   - `QueryStringExtended.full_sql` in `superset/models/helpers.py` — prequery-joining logic moved here from `ExploreMixin.get_query_str`; confirm the two definitions really were equivalent.
   - The new fail-closed guard in `get_chart_sql`: it's an intentional behavior change (previously returned unfiltered SQL as a success). The PR body calls it out explicitly.
   
   Thanks!
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(mcp): surface rejected filter columns in get_chart_sql [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43680:
URL: https://github.com/apache/superset/pull/43680#issuecomment-5481525053

   <!-- Bito Reply -->
   The flagged issue is correct. The current implementation of `rejected_columns_in_query` in `superset/mcp_service/chart/chart_helpers.py` indiscriminately collects all columns listed in `rejected_filters`, including temporal metadata like `__time_range`, which are not actual dataset columns. When these are compared against the requested filters, they can trigger a `ValidationError` even if the user did not explicitly request them as filters.
   
   To resolve this, you should filter the rejected columns to ensure only those that are actually part of the dataset or explicitly requested are considered, or ensure that temporal metadata is excluded from the rejection check.
   
   Would you like me to implement a fix for this issue? I can also check the rest of the PR for other comments if you would like to address them as well.
   
   **superset/mcp_service/chart/chart_helpers.py**
   ```
   def rejected_columns_in_query(query: Any) -> set[str]:
       # ... existing logic ...
       columns = {
           column
           for entry in query.get("rejected_filters", [])
           if isinstance(entry, dict) 
           and isinstance(column := entry.get("column"), str)
           and not column.startswith("__")  # Proposed fix: exclude temporal metadata
       }
       # ... rest of function ...
   ```


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(mcp): surface rejected filter columns in get_chart_sql [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43680:
URL: https://github.com/apache/superset/pull/43680#issuecomment-5537507134

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers an incremental AI Review.
   - `/review full` - Manually triggers a full AI Review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(mcp): surface rejected filter columns in get_chart_sql [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #43680:
URL: https://github.com/apache/superset/pull/43680#issuecomment-5460403975

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/43680?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `32.50000%` with `27 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 66.32%. Comparing base ([`1c8d58a`](https://app.codecov.io/gh/apache/superset/commit/1c8d58a77bda36f892cc27298ed87ded43e6ef9f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`8bf1df0`](https://app.codecov.io/gh/apache/superset/commit/8bf1df0e4a1ed2713039927c371fa8386a0bd255?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/43680?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [superset/mcp\_service/chart/chart\_helpers.py](https://app.codecov.io/gh/apache/superset/pull/43680?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fchart%2Fchart_helpers.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvY2hhcnQvY2hhcnRfaGVscGVycy5weQ==) | 13.04% | [20 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/43680?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [superset/mcp\_service/chart/tool/get\_chart\_sql.py](https://app.codecov.io/gh/apache/superset/pull/43680?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fchart%2Ftool%2Fget_chart_sql.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvY2hhcnQvdG9vbC9nZXRfY2hhcnRfc3FsLnB5) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/43680?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [superset/common/query\_actions.py](https://app.codecov.io/gh/apache/superset/pull/43680?src=pr&el=tree&filepath=superset%2Fcommon%2Fquery_actions.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X2FjdGlvbnMucHk=) | 75.00% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/superset/pull/43680?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [superset/mcp\_service/chart/tool/get\_chart\_data.py](https://app.codecov.io/gh/apache/superset/pull/43680?src=pr&el=tree&filepath=superset%2Fmcp_service%2Fchart%2Ftool%2Fget_chart_data.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvbWNwX3NlcnZpY2UvY2hhcnQvdG9vbC9nZXRfY2hhcnRfZGF0YS5weQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/43680?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   > :exclamation:  There is a different number of reports uploaded between BASE (1c8d58a) and HEAD (8bf1df0). Click for more details.
   > 
   > <details><summary>HEAD has 23 uploads less than BASE</summary>
   >
   >| Flag | BASE (1c8d58a) | HEAD (8bf1df0) |
   >|------|------|------|
   >|python|19|3|
   >|presto|3|1|
   >|hive|3|1|
   >|postgres|4|1|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #43680       +/-   ##
   ===========================================
   - Coverage   79.22%   66.32%   -12.90%     
   ===========================================
     Files        2879     2879               
     Lines      166087   166098       +11     
     Branches    38420    38422        +2     
   ===========================================
   - Hits       131587   110170    -21417     
   - Misses      32017    53684    +21667     
   + Partials     2483     2244      -239     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/43680/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/43680/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.91% <15.00%> (-0.01%)` | :arrow_down: |
   | [mysql](https://app.codecov.io/gh/apache/superset/pull/43680/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [postgres](https://app.codecov.io/gh/apache/superset/pull/43680/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.67% <32.50%> (-0.01%)` | :arrow_down: |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/43680/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.82% <30.00%> (-0.01%)` | :arrow_down: |
   | [python](https://app.codecov.io/gh/apache/superset/pull/43680/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.76% <32.50%> (-25.99%)` | :arrow_down: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/43680/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [unit](https://app.codecov.io/gh/apache/superset/pull/43680/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/43680?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix(mcp): surface rejected filter columns in get_chart_sql [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43680:
URL: https://github.com/apache/superset/pull/43680#issuecomment-5460309822

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org