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

[PR] feat(mcp): list_charts accepts `dashboards` filter [superset]

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

   ### SUMMARY
   
   `list_charts` previously restricted filter columns to `slice_name`, `viz_type`, `datasource_name`, `created_by_fk`, and `changed_by_fk`. The REST API has supported filtering by the `dashboards` relationship since the chart list endpoint shipped — it appears in `search_columns` in `superset/charts/api.py` — but the MCP schema arbitrarily restricted it.
   
   Without that filter, finding *"all charts attached to dashboard X"* via MCP required calling `get_dashboard_info` and walking the nested `charts` array, which is awkward to compose with other filters or pagination.
   
   This PR adds `dashboards` as a permitted filter column and makes the underlying DAO machinery understand it.
   
   ### BEFORE/AFTER
   
   **Before:**
   
   ```
   list_charts({filters:[{col:"dashboards", opr:"eq", value:42}]})
   → ValidationError: Input should be 'slice_name', 'viz_type',
     'datasource_name', 'created_by_fk' or 'changed_by_fk'
   ```
   
   **After:**
   
   ```
   list_charts({filters:[{col:"dashboards", opr:"eq", value:42}]})
   → {charts: [...the 5 charts on dashboard 42...]}
   ```
   
   ### CHANGES
   
   Two changes:
   
   1. **`superset/mcp_service/chart/schemas.py`** — add `"dashboards"` to the `ChartFilter.col` literal so the schema validator accepts it; update the field description so an LLM consumer knows it's there.
   
   2. **`superset/daos/base.py`** — generalize `apply_column_operators` so the `eq` operator on a many-to-many relationship column dispatches to `column.any(<related_pk> == value)` instead of trying to compare a collection to a scalar. The fix is generic and applies to every DAO that inherits `BaseDAO`, not just `Slice` — every m2m column on every DAO now supports `eq` filtering by related id.
   
   The DAO change is additive and backward-compatible: scalar columns continue using the existing scalar comparison path.
   
   ### TESTING INSTRUCTIONS
   
   ```bash
   pytest tests/unit_tests/daos/test_base_relationship_filters.py
   pytest tests/unit_tests/mcp_service/chart/tool/test_list_charts.py
   ```
   
   New tests:
   - `tests/unit_tests/daos/test_base_relationship_filters.py` — verifies the dispatch path for relationship vs scalar columns, plus the existing invalid-column behavior is preserved.
   - Two added cases in `tests/unit_tests/mcp_service/chart/tool/test_list_charts.py::TestListChartsRequestSchema` — assert the schema accepts `dashboards` and rejects unknown columns.
   
   Existing 21 `list_charts` tests still pass.
   
   ### ADDITIONAL INFORMATION
   
   - [ ] Has associated issue: _(filing alongside, will link)_
   - [ ] Required feature flags: _none_
   - [x] Changes UI: _no_
   - [x] Includes DB Migration: _no_
   - [x] Includes packaged template files: _no_
   - [x] Introduces new feature or API: _adds a previously-unavailable filter column to the existing `list_charts` MCP tool_
   - [x] Removes existing feature or API: _no_


-- 
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] feat(mcp): list_charts accepts `dashboards` filter [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 #40397:
URL: https://github.com/apache/superset/pull/40397#discussion_r3476453749


##########
tests/unit_tests/daos/test_base_relationship_filters.py:
##########
@@ -0,0 +1,203 @@
+# 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.
+
+"""Tests for BaseDAO.apply_column_operators with relationship columns.
+
+`apply_column_operators` historically only worked on scalar columns. When a
+caller asked for `{col: "<m2m_relationship>", opr: "eq", value: <id>}` the
+SQLAlchemy backend raised "Can't compare a collection to an object."
+
+The behavior now: detect the relationship attribute and dispatch to
+`column.any(<related_pk> == value)`, so callers can use the natural shape
+for both scalar columns and m2m relationships.
+"""
+
+from unittest.mock import MagicMock
+
+import pytest
+
+from superset.daos.base import BaseDAO, ColumnOperator, ColumnOperatorEnum
+from superset.models.slice import Slice
+
+
+class _SliceDAO(BaseDAO[Slice]):
+    """Tiny concrete DAO so we can exercise the BaseDAO logic against the
+    real Slice model + its m2m `dashboards` relationship without standing up
+    a full database."""
+
+    model_cls = Slice
+    filterable_relationships = frozenset({"dashboards"})
+
+
+class _SliceDAONoRelationships(BaseDAO[Slice]):
+    """Same model but with no relationships opted into discovery. Used to
+    verify the default behavior (empty ``filterable_relationships``)
+    keeps relationship columns out of the schema-discovery output."""
+
+    model_cls = Slice
+
+
+class TestApplyColumnOperatorsRelationship:
+    """`apply_column_operators` handles m2m relationship columns via .any()."""
+
+    def test_eq_on_relationship_dispatches_to_any(self):
+        """`{col: 'dashboards', opr: 'eq', value: 42}` calls .any() on the
+        relationship and compares the related model's PK to 42."""
+        mock_query = MagicMock()
+        mock_query.filter.return_value = mock_query
+
+        result = _SliceDAO.apply_column_operators(
+            mock_query,
+            [ColumnOperator(col="dashboards", opr=ColumnOperatorEnum.eq, value=42)],
+        )
+
+        assert result is mock_query
+        # We applied exactly one filter
+        assert mock_query.filter.call_count == 1
+        # And the argument is a SQLAlchemy BinaryExpression that survives
+        # being passed to .filter() — we can't easily inspect its structure
+        # without rendering SQL, but we can verify the dispatch happened
+        # without raising.
+
+    def test_eq_on_scalar_column_unchanged(self):
+        """Non-relationship columns continue using the old scalar path."""
+        mock_query = MagicMock()
+        mock_query.filter.return_value = mock_query
+
+        _SliceDAO.apply_column_operators(
+            mock_query,
+            [
+                ColumnOperator(
+                    col="slice_name", opr=ColumnOperatorEnum.eq, value="Revenue"
+                )
+            ],
+        )
+        assert mock_query.filter.call_count == 1
+
+    def test_invalid_column_still_raises(self):

Review Comment:
   **Suggestion:** Add an explicit return type annotation to this method (for example, `-> None`) to align with the rule requiring type hints on new Python methods. [custom_rule]
   
   **Severity Level:** Minor ⚠️
   <details>
   <summary><b>Why it matters? 🤔 </b></summary>
   
   This is another new test method without a return type annotation. That directly violates the requirement that newly added Python functions and methods be fully typed.
   </details>
   
   [![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=a52968a2df2e41be848f78bafa468445&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=a52968a2df2e41be848f78bafa468445&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:** tests/unit_tests/daos/test_base_relationship_filters.py
   **Line:** 91:91
   **Comment:**
   	*Custom Rule: Add an explicit return type annotation to this method (for example, `-> None`) to align with the rule requiring type hints on new Python methods.
   
   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%2F40397&comment_hash=6672ff63e7e11181c490c8080a40e154866cefe34c92c3d6ac9a48ab8b16dd71&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40397&comment_hash=6672ff63e7e11181c490c8080a40e154866cefe34c92c3d6ac9a48ab8b16dd71&reaction=dislike'>👎</a>



##########
tests/unit_tests/daos/test_base_relationship_filters.py:
##########
@@ -0,0 +1,203 @@
+# 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.
+
+"""Tests for BaseDAO.apply_column_operators with relationship columns.
+
+`apply_column_operators` historically only worked on scalar columns. When a
+caller asked for `{col: "<m2m_relationship>", opr: "eq", value: <id>}` the
+SQLAlchemy backend raised "Can't compare a collection to an object."
+
+The behavior now: detect the relationship attribute and dispatch to
+`column.any(<related_pk> == value)`, so callers can use the natural shape
+for both scalar columns and m2m relationships.
+"""
+
+from unittest.mock import MagicMock
+
+import pytest
+
+from superset.daos.base import BaseDAO, ColumnOperator, ColumnOperatorEnum
+from superset.models.slice import Slice
+
+
+class _SliceDAO(BaseDAO[Slice]):
+    """Tiny concrete DAO so we can exercise the BaseDAO logic against the
+    real Slice model + its m2m `dashboards` relationship without standing up
+    a full database."""
+
+    model_cls = Slice
+    filterable_relationships = frozenset({"dashboards"})
+
+
+class _SliceDAONoRelationships(BaseDAO[Slice]):
+    """Same model but with no relationships opted into discovery. Used to
+    verify the default behavior (empty ``filterable_relationships``)
+    keeps relationship columns out of the schema-discovery output."""
+
+    model_cls = Slice
+
+
+class TestApplyColumnOperatorsRelationship:
+    """`apply_column_operators` handles m2m relationship columns via .any()."""
+
+    def test_eq_on_relationship_dispatches_to_any(self):
+        """`{col: 'dashboards', opr: 'eq', value: 42}` calls .any() on the
+        relationship and compares the related model's PK to 42."""
+        mock_query = MagicMock()
+        mock_query.filter.return_value = mock_query
+
+        result = _SliceDAO.apply_column_operators(
+            mock_query,
+            [ColumnOperator(col="dashboards", opr=ColumnOperatorEnum.eq, value=42)],
+        )
+
+        assert result is mock_query
+        # We applied exactly one filter
+        assert mock_query.filter.call_count == 1
+        # And the argument is a SQLAlchemy BinaryExpression that survives
+        # being passed to .filter() — we can't easily inspect its structure
+        # without rendering SQL, but we can verify the dispatch happened
+        # without raising.
+
+    def test_eq_on_scalar_column_unchanged(self):

Review Comment:
   **Suggestion:** Add an explicit return type annotation to this test method (for example, `-> None`) so the new function is fully typed. [custom_rule]
   
   **Severity Level:** Minor ⚠️
   <details>
   <summary><b>Why it matters? 🤔 </b></summary>
   
   This newly added test method has no return type annotation. Since the PR adds new Python code, the typing rule applies and the omission is a genuine violation.
   </details>
   
   [![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=e0fd2fdca228478d8300b5e3d3f036ae&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=e0fd2fdca228478d8300b5e3d3f036ae&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:** tests/unit_tests/daos/test_base_relationship_filters.py
   **Line:** 76:76
   **Comment:**
   	*Custom Rule: Add an explicit return type annotation to this test method (for example, `-> None`) so the new function is fully typed.
   
   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%2F40397&comment_hash=5e1dce64ad8b5d2db7194a474c936e6ec4ca8f71852af1c546d511a1a1f61e10&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40397&comment_hash=5e1dce64ad8b5d2db7194a474c936e6ec4ca8f71852af1c546d511a1a1f61e10&reaction=dislike'>👎</a>



##########
tests/unit_tests/daos/test_base_relationship_filters.py:
##########
@@ -0,0 +1,203 @@
+# 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.
+
+"""Tests for BaseDAO.apply_column_operators with relationship columns.
+
+`apply_column_operators` historically only worked on scalar columns. When a
+caller asked for `{col: "<m2m_relationship>", opr: "eq", value: <id>}` the
+SQLAlchemy backend raised "Can't compare a collection to an object."
+
+The behavior now: detect the relationship attribute and dispatch to
+`column.any(<related_pk> == value)`, so callers can use the natural shape
+for both scalar columns and m2m relationships.
+"""
+
+from unittest.mock import MagicMock
+
+import pytest
+
+from superset.daos.base import BaseDAO, ColumnOperator, ColumnOperatorEnum
+from superset.models.slice import Slice
+
+
+class _SliceDAO(BaseDAO[Slice]):
+    """Tiny concrete DAO so we can exercise the BaseDAO logic against the
+    real Slice model + its m2m `dashboards` relationship without standing up
+    a full database."""
+
+    model_cls = Slice
+    filterable_relationships = frozenset({"dashboards"})
+
+
+class _SliceDAONoRelationships(BaseDAO[Slice]):
+    """Same model but with no relationships opted into discovery. Used to
+    verify the default behavior (empty ``filterable_relationships``)
+    keeps relationship columns out of the schema-discovery output."""
+
+    model_cls = Slice
+
+
+class TestApplyColumnOperatorsRelationship:
+    """`apply_column_operators` handles m2m relationship columns via .any()."""
+
+    def test_eq_on_relationship_dispatches_to_any(self):
+        """`{col: 'dashboards', opr: 'eq', value: 42}` calls .any() on the
+        relationship and compares the related model's PK to 42."""
+        mock_query = MagicMock()
+        mock_query.filter.return_value = mock_query
+
+        result = _SliceDAO.apply_column_operators(
+            mock_query,
+            [ColumnOperator(col="dashboards", opr=ColumnOperatorEnum.eq, value=42)],
+        )
+
+        assert result is mock_query
+        # We applied exactly one filter
+        assert mock_query.filter.call_count == 1
+        # And the argument is a SQLAlchemy BinaryExpression that survives
+        # being passed to .filter() — we can't easily inspect its structure
+        # without rendering SQL, but we can verify the dispatch happened
+        # without raising.
+
+    def test_eq_on_scalar_column_unchanged(self):
+        """Non-relationship columns continue using the old scalar path."""
+        mock_query = MagicMock()
+        mock_query.filter.return_value = mock_query
+
+        _SliceDAO.apply_column_operators(
+            mock_query,
+            [
+                ColumnOperator(
+                    col="slice_name", opr=ColumnOperatorEnum.eq, value="Revenue"
+                )
+            ],
+        )
+        assert mock_query.filter.call_count == 1
+
+    def test_invalid_column_still_raises(self):
+        """Columns that don't exist on the model raise ValueError as before."""
+        mock_query = MagicMock()
+        with pytest.raises(ValueError, match="does not exist on Slice"):
+            _SliceDAO.apply_column_operators(
+                mock_query,
+                [
+                    ColumnOperator(
+                        col="does_not_exist", opr=ColumnOperatorEnum.eq, value=1
+                    )
+                ],
+            )
+
+    @pytest.mark.parametrize(
+        "operator",
+        [
+            ColumnOperatorEnum.eq,
+            ColumnOperatorEnum.ne,
+            ColumnOperatorEnum.in_,
+            ColumnOperatorEnum.nin,
+            ColumnOperatorEnum.is_null,
+            ColumnOperatorEnum.is_not_null,
+        ],
+    )
+    def test_supported_relationship_operators_dispatch(self, operator):
+        """eq/ne/in/nin/is_null/is_not_null all dispatch to .any() variants."""
+        mock_query = MagicMock()
+        mock_query.filter.return_value = mock_query
+
+        _SliceDAO.apply_column_operators(
+            mock_query,
+            [ColumnOperator(col="dashboards", opr=operator, value=[1, 2])],
+        )
+        assert mock_query.filter.call_count == 1
+
+    @pytest.mark.parametrize(
+        "operator",
+        [
+            ColumnOperatorEnum.sw,
+            ColumnOperatorEnum.ew,
+            ColumnOperatorEnum.like,
+            ColumnOperatorEnum.ilike,
+            ColumnOperatorEnum.gt,
+            ColumnOperatorEnum.gte,
+            ColumnOperatorEnum.lt,
+            ColumnOperatorEnum.lte,
+        ],
+    )
+    def test_unsupported_relationship_operators_raise(self, operator):

Review Comment:
   **Suggestion:** Add type hints for the `operator` parameter and the method return value (for example, `-> None`) to meet the full typing requirement for new methods. [custom_rule]
   
   **Severity Level:** Minor ⚠️
   <details>
   <summary><b>Why it matters? 🤔 </b></summary>
   
   This newly added parameterized method is missing type hints for its argument and its return value. That matches the custom rule violation exactly.
   </details>
   
   [![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=0383082a96d347bf95a9915710c8bfa6&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=0383082a96d347bf95a9915710c8bfa6&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:** tests/unit_tests/daos/test_base_relationship_filters.py
   **Line:** 139:139
   **Comment:**
   	*Custom Rule: Add type hints for the `operator` parameter and the method return value (for example, `-> None`) to meet the full typing requirement for new methods.
   
   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%2F40397&comment_hash=6620b036d9064805cbc7a00f0f8ccf6fc4a4259b65d56e5abf6ed23531f03153&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40397&comment_hash=6620b036d9064805cbc7a00f0f8ccf6fc4a4259b65d56e5abf6ed23531f03153&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] feat(mcp): list_charts accepts `dashboards` filter [superset]

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

   <!-- translation-regression-bot -->
   ## ⚠️ Translation Regression Detected
   
   A source change in this PR renamed or reworded strings, invalidating existing translations (they are now `#, fuzzy`) in `fi`, `fr`, `it`, `ko`, `th`. Please resolve the affected `.po` files before merging.
   
   _Note: intentionally **deleting** a translatable string is not a regression and is not flagged here — only translations invalidated by a renamed/reworded source string are._
   
   | Language | Fuzzy before | Fuzzy after | New |
   |----------|-------------:|------------:|----:|
   | `fi` | 4815 | 4823 | +8 |
   | `fr` | 1004 | 1014 | +10 |
   | `it` | 1665 | 1667 | +2 |
   | `ko` | 1519 | 1521 | +2 |
   | `th` | 4815 | 4823 | +8 |
   
   ### How to fix
   
   **1. Install dependencies** (if not already set up):
   
   ```bash
   pip install -r superset/translations/requirements.txt
   sudo apt-get install gettext   # or: brew install gettext
   ```
   
   **2. Re-extract strings and sync `.po` files:**
   
   ```bash
   ./scripts/translations/babel_update.sh
   ```
   
   This rewrites `superset/translations/messages.pot` from the current source files and merges the changes into every `.po` file. Strings whose `msgid` changed will be marked `#, fuzzy`.
   
   **3. Resolve the fuzzy entries** in the affected language files (`fi`, `fr`, `it`, `ko`, `th`):
   
   ```bash
   grep -n '#, fuzzy' superset/translations/<lang>/LC_MESSAGES/messages.po
   ```
   
   For each fuzzy entry, either rewrite the `msgstr` to match the new string and remove the `#, fuzzy` line, or clear the `msgstr` to `""` if you cannot provide a translation.
   
   **4. Commit your changes to the `.po` files.**
   


-- 
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] feat(mcp): list_charts accepts `dashboards` filter [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 #40397:
URL: https://github.com/apache/superset/pull/40397#discussion_r3476453740


##########
tests/unit_tests/daos/test_base_relationship_filters.py:
##########
@@ -0,0 +1,203 @@
+# 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.
+
+"""Tests for BaseDAO.apply_column_operators with relationship columns.
+
+`apply_column_operators` historically only worked on scalar columns. When a
+caller asked for `{col: "<m2m_relationship>", opr: "eq", value: <id>}` the
+SQLAlchemy backend raised "Can't compare a collection to an object."
+
+The behavior now: detect the relationship attribute and dispatch to
+`column.any(<related_pk> == value)`, so callers can use the natural shape
+for both scalar columns and m2m relationships.
+"""
+
+from unittest.mock import MagicMock
+
+import pytest
+
+from superset.daos.base import BaseDAO, ColumnOperator, ColumnOperatorEnum
+from superset.models.slice import Slice
+
+
+class _SliceDAO(BaseDAO[Slice]):
+    """Tiny concrete DAO so we can exercise the BaseDAO logic against the
+    real Slice model + its m2m `dashboards` relationship without standing up
+    a full database."""
+
+    model_cls = Slice
+    filterable_relationships = frozenset({"dashboards"})
+
+
+class _SliceDAONoRelationships(BaseDAO[Slice]):
+    """Same model but with no relationships opted into discovery. Used to
+    verify the default behavior (empty ``filterable_relationships``)
+    keeps relationship columns out of the schema-discovery output."""
+
+    model_cls = Slice
+
+
+class TestApplyColumnOperatorsRelationship:
+    """`apply_column_operators` handles m2m relationship columns via .any()."""
+
+    def test_eq_on_relationship_dispatches_to_any(self):

Review Comment:
   **Suggestion:** Add an explicit return type annotation to this new test method (for example, `-> None`) to satisfy the full typing requirement for newly added Python functions. [custom_rule]
   
   **Severity Level:** Minor ⚠️
   <details>
   <summary><b>Why it matters? 🤔 </b></summary>
   
   This is a newly added Python test method and it lacks an explicit return type annotation. The rule requires new Python functions and methods to be fully typed, so this is a real violation.
   </details>
   
   [![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=f04c004e6b0c47a6bd426a6f9f00239b&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=f04c004e6b0c47a6bd426a6f9f00239b&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:** tests/unit_tests/daos/test_base_relationship_filters.py
   **Line:** 57:57
   **Comment:**
   	*Custom Rule: Add an explicit return type annotation to this new test method (for example, `-> None`) to satisfy the full typing requirement for newly added Python functions.
   
   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%2F40397&comment_hash=687e045d073d9b894b56faa216c123ee6173f89c9164cbe3f254520e0a1f568a&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40397&comment_hash=687e045d073d9b894b56faa216c123ee6173f89c9164cbe3f254520e0a1f568a&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] feat(mcp): list_charts accepts `dashboards` filter [superset]

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


-- 
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] feat(mcp): list_charts accepts `dashboards` filter [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 #40397:
URL: https://github.com/apache/superset/pull/40397#discussion_r3293884062


##########
superset/daos/base.py:
##########
@@ -499,9 +499,24 @@ def apply_column_operators(
                 )
             column = getattr(cls.model_cls, col)
             try:
-                # Always use ColumnOperatorEnum's apply method
                 operator_enum = ColumnOperatorEnum(opr)
-                query = query.filter(operator_enum.apply(column, value))
+                # Relationship attributes (many-to-one or many-to-many)
+                # can't be compared directly with scalar operators —
+                # SQLAlchemy needs `.any(...)` or `.has(...)`. Detect the
+                # collection case and dispatch to the related model's id
+                # column. This lets callers use the natural shape
+                # `{col: "<relationship>", opr: "eq", value: <id>}` to
+                # find rows whose related collection contains the given id.
+                if (
+                    operator_enum == ColumnOperatorEnum.eq
+                    and hasattr(column, "property")
+                    and isinstance(column.property, RelationshipProperty)
+                    and column.property.uselist
+                ):
+                    related_pk = inspect(column.property.mapper).primary_key[0]
+                    query = query.filter(column.any(related_pk == value))
+                else:
+                    query = query.filter(operator_enum.apply(column, value))

Review Comment:
   **Suggestion:** Relationship filters only get special handling for `eq`; any other operator on a collection relationship still falls through to the scalar operator path and will raise SQLAlchemy runtime errors (for example when a client sends `opr="in"` or `opr="sw"` with `col="dashboards"`). Add explicit handling for allowed relationship operators (or raise a clear validation error before query execution) so relationship filters cannot hit the invalid scalar comparison path. [logic error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ list_charts crashes when using non-eq dashboard filters.
   - ⚠️ MCP clients can trigger 500s with malformed relationship filters.
   - ⚠️ Any BaseDAO m2m filter misusing operators hits runtime errors.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. From an MCP client, call the `list_charts` tool defined in
   `superset/mcp_service/chart/tool/list_charts.py:14-46` with a request payload whose
   `filters` array contains an entry like `{"col": "dashboards", "opr": "in", "value": [42]}`
   (or any non-`eq` operator such as `"sw"`).
   
   2. Inside `list_charts`, the request is parsed into `ListChartsRequest` and `ChartFilter`
   objects (see `superset/mcp_service/chart/schemas.py:18-48`, where `ChartFilter.opr` is a
   `ColumnOperatorEnum` and `col` allows `"dashboards"`). The tool then constructs a
   `ModelListCore` with `dao_class=ChartDAO` and calls
   `tool.run_tool(filters=request.filters, ...)` (see
   `superset/mcp_service/chart/tool/list_charts.py:93-114`).
   
   3. `ModelListCore.run_tool` in `superset/mcp_service/mcp_core.py:260-38` forwards those
   `ChartFilter` objects to the DAO via `ChartDAO.list(column_operators=filters, ...)`.
   `ChartDAO` inherits `BaseDAO[Slice]` (`superset/daos/chart.py`) so
   `BaseDAO.apply_column_operators` is used to apply the filters.
   
   4. In `BaseDAO.apply_column_operators` (`superset/daos/base.py:18-64` and the new logic at
   lines 510-519), for the filter with `col="dashboards"` and `opr="in"`, `column` is the
   `Slice.dashboards` many-to-many relationship attribute, `operator_enum` is
   `ColumnOperatorEnum.in_`, the relationship-specific `eq` branch at lines 510-56 is
   skipped, and execution falls into the `else` branch at line 519 calling
   `operator_enum.apply(column, value)`. `ColumnOperatorEnum.in_` is mapped in
   `superset/daos/base.py:29-37` to `lambda col, val: col.in_(...)`, which is invalid for a
   collection relationship attribute; when `query.filter(column.in_(...))` is evaluated,
   SQLAlchemy raises a runtime error (e.g. an AttributeError or StatementError about
   comparing a collection to a scalar), causing the `list_charts` MCP call to fail with an
   internal error instead of returning results or a clear validation error.
   ```
   </details>
   
   [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e6f6b28fd1ca4f6bb024b8cfb8219af3&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=e6f6b28fd1ca4f6bb024b8cfb8219af3&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/daos/base.py
   **Line:** 510:519
   **Comment:**
   	*Logic Error: Relationship filters only get special handling for `eq`; any other operator on a collection relationship still falls through to the scalar operator path and will raise SQLAlchemy runtime errors (for example when a client sends `opr="in"` or `opr="sw"` with `col="dashboards"`). Add explicit handling for allowed relationship operators (or raise a clear validation error before query execution) so relationship filters cannot hit the invalid scalar comparison path.
   
   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%2F40397&comment_hash=33247a3251c46da1a0f0c3ae4dc157e0647835c4d555223bd9fddbd615fe5795&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40397&comment_hash=33247a3251c46da1a0f0c3ae4dc157e0647835c4d555223bd9fddbd615fe5795&reaction=dislike'>👎</a>



##########
superset/mcp_service/chart/schemas.py:
##########
@@ -538,12 +538,15 @@ class ChartFilter(ColumnOperator):
         "datasource_name",
         "created_by_fk",
         "changed_by_fk",
+        "dashboards",

Review Comment:
   **Suggestion:** The schema now accepts `dashboards`, but this addition is not reflected in DAO filter discovery metadata, so `get_schema(model_type='chart')` still does not advertise `dashboards` as a filter column. This creates an API contract mismatch where clients following schema discovery won't learn about the new filter. Update chart filter metadata exposure (or relationship filter discovery) so `dashboards` appears consistently in `get_schema`. [api mismatch]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ get_schema omits dashboards, hiding a supported chart filter.
   - ⚠️ MCP agents relying on discovery cannot learn dashboards filter.
   - ⚠️ Documentation in ChartFilter contradicts actual schema metadata.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Call the MCP `get_schema` tool with `{"request": {"model_type": "chart"}}`, which is
   implemented in `superset/mcp_service/system/tool/get_schema.py:66-83`. For
   `model_type="chart"`, `_get_chart_schema_core` returns a `ModelGetSchemaCore` configured
   with `dao_class=ChartDAO`, `select_columns=get_chart_columns()`, and
   `exclude_filter_columns=set(SELF_REFERENCING_FILTER_COLUMNS)`.
   
   2. In `ModelGetSchemaCore.run_tool` (`superset/mcp_service/mcp_core.py:69-83`), the method
   `_get_filter_columns` at `superset/mcp_service/mcp_core.py:35-62` calls
   `ChartDAO.get_filterable_columns_and_operators()` to build `schema_info.filter_columns`.
   
   3. `ChartDAO.get_filterable_columns_and_operators` (`superset/daos/chart.py:92-16`) simply
   delegates to `BaseDAO.get_filterable_columns_and_operators` and then augments with
   `CHART_CUSTOM_FIELDS`. `BaseDAO.get_filterable_columns_and_operators` in
   `superset/daos/base.py:526-119` inspects `mapper.columns` (real table columns) and hybrid
   properties; it does not include relationship attributes like the many-to-many
   `Slice.dashboards` relationship, so `"dashboards"` never appears in the returned mapping.
   
   4. As a result, the `get_schema` response for charts contains `schema_info.filter_columns`
   with entries for fields like `"slice_name"` and `"viz_type"`, but no `"dashboards"` key,
   while the MCP request schema `ChartFilter` in
   `superset/mcp_service/chart/schemas.py:18-40` explicitly allows `col="dashboards"` and its
   description tells clients to "Use get_schema(model_type='chart') for available filter
   columns" and mentions filtering by `'dashboards'`. This creates a contract mismatch:
   clients following the documented discovery flow (call `get_schema` to learn valid filters,
   then call `list_charts`) cannot discover the new `dashboards` filter even though the list
   tool and DAO now support it.
   ```
   </details>
   
   [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a058c1b7a028453bb51605c967517650&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=a058c1b7a028453bb51605c967517650&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:** 541:541
   **Comment:**
   	*Api Mismatch: The schema now accepts `dashboards`, but this addition is not reflected in DAO filter discovery metadata, so `get_schema(model_type='chart')` still does not advertise `dashboards` as a filter column. This creates an API contract mismatch where clients following schema discovery won't learn about the new filter. Update chart filter metadata exposure (or relationship filter discovery) so `dashboards` appears consistently in `get_schema`.
   
   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%2F40397&comment_hash=b521233f6de47e872fa18f2b9fb3f3958477c2f533a7ed9c6aeef7d3a0d6b31c&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40397&comment_hash=b521233f6de47e872fa18f2b9fb3f3958477c2f533a7ed9c6aeef7d3a0d6b31c&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] feat(mcp): list_charts accepts `dashboards` filter [superset]

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


##########
tests/unit_tests/daos/test_base_relationship_filters.py:
##########
@@ -0,0 +1,93 @@
+# 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.
+
+"""Tests for BaseDAO.apply_column_operators with relationship columns.
+
+`apply_column_operators` historically only worked on scalar columns. When a
+caller asked for `{col: "<m2m_relationship>", opr: "eq", value: <id>}` the
+SQLAlchemy backend raised "Can't compare a collection to an object."
+
+The behavior now: detect the relationship attribute and dispatch to
+`column.any(<related_pk> == value)`, so callers can use the natural shape
+for both scalar columns and m2m relationships.
+"""
+
+from unittest.mock import MagicMock
+
+import pytest
+
+from superset.daos.base import BaseDAO, ColumnOperator, ColumnOperatorEnum
+from superset.models.slice import Slice
+
+
+class _SliceDAO(BaseDAO[Slice]):
+    """Tiny concrete DAO so we can exercise the BaseDAO logic against the
+    real Slice model + its m2m `dashboards` relationship without standing up
+    a full database."""
+
+    model_cls = Slice
+
+
+class TestApplyColumnOperatorsRelationship:
+    """`apply_column_operators` handles m2m relationship columns via .any()."""
+
+    def test_eq_on_relationship_dispatches_to_any(self):
+        """`{col: 'dashboards', opr: 'eq', value: 42}` calls .any() on the
+        relationship and compares the related model's PK to 42."""
+        mock_query = MagicMock()
+        mock_query.filter.return_value = mock_query
+
+        result = _SliceDAO.apply_column_operators(
+            mock_query,
+            [ColumnOperator(col="dashboards", opr=ColumnOperatorEnum.eq, value=42)],
+        )
+
+        assert result is mock_query
+        # We applied exactly one filter
+        assert mock_query.filter.call_count == 1
+        # And the argument is a SQLAlchemy BinaryExpression that survives
+        # being passed to .filter() — we can't easily inspect its structure
+        # without rendering SQL, but we can verify the dispatch happened
+        # without raising.

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Weak test assertions</b></div>
   <div id="fix">
   
   The test only asserts `call_count == 1` without verifying the actual filter expression. Per rule 6262, tests should validate behavior directly — verify the filter receives the expected BinaryExpression with related_pk == value.
   </div>
   
   
   </div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Non-existent model attribute in test</b></div>
   <div id="fix">
   
   The `col="dashboards"` references an attribute that doesn't exist on `Slice`. The `dashboards` backref is on the `Dashboard` model (via `Dashboard.slices`), not `Slice`. This test will fail with `ValueError: Invalid filter: column 'dashboards' does not exist on Slice`.
   </div>
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #80e0f2</i></small>
   </div>
   
   ---
   Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
   - [ ] Yes, avoid them



-- 
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] feat(mcp): list_charts accepts `dashboards` filter [superset]

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

   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


Re: [PR] feat(mcp): list_charts accepts `dashboards` filter [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 #40397:
URL: https://github.com/apache/superset/pull/40397#discussion_r3476453767


##########
tests/unit_tests/mcp_service/chart/tool/test_list_charts.py:
##########
@@ -184,6 +184,29 @@ def test_filter_validation(self):
         with pytest.raises(ValueError, match="Field required"):
             ChartFilter(col="slice_name")  # Missing opr and value
 
+    def test_dashboards_filter_accepted(self):
+        """`dashboards` is a valid filter column for finding charts on a dashboard."""
+        # The filter is accepted at schema-validation time
+        f = ChartFilter(col="dashboards", opr="eq", value=42)
+        assert f.col == "dashboards"
+        assert f.opr.value == "eq"
+        assert f.value == 42
+
+        # And composes into a request like any other filter
+        request = ListChartsRequest(filters=[f])
+        assert request.filters[0].col == "dashboards"
+
+    def test_invalid_filter_column_rejected(self):

Review Comment:
   **Suggestion:** Add full type hints to this newly added test method, including an explicit return type, to comply with the typing requirement for new Python code. [custom_rule]
   
   **Severity Level:** Minor ⚠️
   <details>
   <summary><b>Why it matters? 🤔 </b></summary>
   
   This is another newly added Python test method without type hints on `self` or an explicit return type.
   Since the surrounding code is being changed, the custom rule applies and the omission is a verified violation.
   </details>
   
   [![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=c1aa04effed9451ba560268a05269c6b&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=c1aa04effed9451ba560268a05269c6b&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:** tests/unit_tests/mcp_service/chart/tool/test_list_charts.py
   **Line:** 199:199
   **Comment:**
   	*Custom Rule: Add full type hints to this newly added test method, including an explicit return type, to comply with the typing requirement for new Python code.
   
   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%2F40397&comment_hash=b23f94b5667736a477861763627c7a6d7fd5fa6856b2bf0025f02b9990905bc8&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40397&comment_hash=b23f94b5667736a477861763627c7a6d7fd5fa6856b2bf0025f02b9990905bc8&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] feat(mcp): list_charts accepts `dashboards` filter [superset]

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

   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 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] feat(mcp): list_charts accepts `dashboards` filter [superset]

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

   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 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] feat(mcp): list_charts accepts `dashboards` filter [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 #40397:
URL: https://github.com/apache/superset/pull/40397#discussion_r3476491302


##########
superset/mcp_service/chart/schemas.py:
##########
@@ -606,12 +606,17 @@ class ChartFilter(ColumnOperator):
         "datasource_name",
         "created_by_fk",
         "changed_by_fk",
+        "dashboards",

Review Comment:
   **Suggestion:** Adding `dashboards` to the allowed filter columns without constraining `opr` for that column creates a validation/runtime contract break: requests like `{col:"dashboards", opr:"sw", value:"x"}` pass Pydantic validation but later fail in DAO execution with a `ValueError` ("not supported on relationship column"). Add a column-aware validator (or a dedicated dashboards filter type) so unsupported operators for relationship filters are rejected at request-validation time instead of surfacing as runtime tool errors. [api mismatch]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ list_charts crashes on dashboards filters with unsupported operator.
   - ⚠️ MCP clients see runtime errors instead of schema validation feedback.
   - ⚠️ Mismatch between get_schema filter metadata and tool input schema.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Note that `ChartFilter` in `superset/mcp_service/chart/schemas.py:595-69` defines `col`
   as a Literal including `"dashboards"` and `opr` as `ColumnOperatorEnum`, which in
   `superset/daos/base.py:60-75` includes `"sw"` among other operators.
   
   2. Construct a filter object in Python or via MCP params: `ChartFilter(col="dashboards",
   opr="sw", value="x")` or the JSON equivalent inside `ListChartsRequest(filters=[...])` as
   defined at `superset/mcp_service/chart/schemas.py:1777-59`; Pydantic validation succeeds
   because both the column and operator are individually allowed.
   
   3. Call the MCP tool `list_charts` (entrypoint
   `superset/mcp_service/chart/tool/list_charts.py:76-118`) with that `ListChartsRequest`;
   `ModelListCore.run_tool` in `superset/mcp_service/mcp_core.py:30-80` passes the filters
   through to `ChartDAO.list`, which calls `BaseDAO.apply_column_operators` at
   `superset/daos/base.py:50-75`.
   
   4. In `apply_column_operators`, the `dashboards` attribute is detected as a collection
   relationship and dispatched to `_apply_relationship_filter`
   (`superset/daos/base.py:101-150`), which does not support `sw` and raises
   `ValueError("Operator 'sw' is not supported on relationship column 'dashboards'. Use one
   of: eq, ne, in, nin, is_null, is_not_null.")`, causing `list_charts` to fail at runtime
   instead of during request validation.
   ```
   </details>
   
   [![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=98a45eba6b4244ce8c9c2b34e416c663&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=98a45eba6b4244ce8c9c2b34e416c663&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:** 609:609
   **Comment:**
   	*Api Mismatch: Adding `dashboards` to the allowed filter columns without constraining `opr` for that column creates a validation/runtime contract break: requests like `{col:"dashboards", opr:"sw", value:"x"}` pass Pydantic validation but later fail in DAO execution with a `ValueError` ("not supported on relationship column"). Add a column-aware validator (or a dedicated dashboards filter type) so unsupported operators for relationship filters are rejected at request-validation time instead of surfacing as runtime tool errors.
   
   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%2F40397&comment_hash=f8f6a0a32f135dddf90ccfc564b99cb3c191d774782e710dc3d5b8445858655f&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40397&comment_hash=f8f6a0a32f135dddf90ccfc564b99cb3c191d774782e710dc3d5b8445858655f&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] feat(mcp): list_charts accepts `dashboards` filter [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 #40397:
URL: https://github.com/apache/superset/pull/40397#discussion_r3476453752


##########
tests/unit_tests/daos/test_base_relationship_filters.py:
##########
@@ -0,0 +1,203 @@
+# 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.
+
+"""Tests for BaseDAO.apply_column_operators with relationship columns.
+
+`apply_column_operators` historically only worked on scalar columns. When a
+caller asked for `{col: "<m2m_relationship>", opr: "eq", value: <id>}` the
+SQLAlchemy backend raised "Can't compare a collection to an object."
+
+The behavior now: detect the relationship attribute and dispatch to
+`column.any(<related_pk> == value)`, so callers can use the natural shape
+for both scalar columns and m2m relationships.
+"""
+
+from unittest.mock import MagicMock
+
+import pytest
+
+from superset.daos.base import BaseDAO, ColumnOperator, ColumnOperatorEnum
+from superset.models.slice import Slice
+
+
+class _SliceDAO(BaseDAO[Slice]):
+    """Tiny concrete DAO so we can exercise the BaseDAO logic against the
+    real Slice model + its m2m `dashboards` relationship without standing up
+    a full database."""
+
+    model_cls = Slice
+    filterable_relationships = frozenset({"dashboards"})
+
+
+class _SliceDAONoRelationships(BaseDAO[Slice]):
+    """Same model but with no relationships opted into discovery. Used to
+    verify the default behavior (empty ``filterable_relationships``)
+    keeps relationship columns out of the schema-discovery output."""
+
+    model_cls = Slice
+
+
+class TestApplyColumnOperatorsRelationship:
+    """`apply_column_operators` handles m2m relationship columns via .any()."""
+
+    def test_eq_on_relationship_dispatches_to_any(self):
+        """`{col: 'dashboards', opr: 'eq', value: 42}` calls .any() on the
+        relationship and compares the related model's PK to 42."""
+        mock_query = MagicMock()
+        mock_query.filter.return_value = mock_query
+
+        result = _SliceDAO.apply_column_operators(
+            mock_query,
+            [ColumnOperator(col="dashboards", opr=ColumnOperatorEnum.eq, value=42)],
+        )
+
+        assert result is mock_query
+        # We applied exactly one filter
+        assert mock_query.filter.call_count == 1
+        # And the argument is a SQLAlchemy BinaryExpression that survives
+        # being passed to .filter() — we can't easily inspect its structure
+        # without rendering SQL, but we can verify the dispatch happened
+        # without raising.
+
+    def test_eq_on_scalar_column_unchanged(self):
+        """Non-relationship columns continue using the old scalar path."""
+        mock_query = MagicMock()
+        mock_query.filter.return_value = mock_query
+
+        _SliceDAO.apply_column_operators(
+            mock_query,
+            [
+                ColumnOperator(
+                    col="slice_name", opr=ColumnOperatorEnum.eq, value="Revenue"
+                )
+            ],
+        )
+        assert mock_query.filter.call_count == 1
+
+    def test_invalid_column_still_raises(self):
+        """Columns that don't exist on the model raise ValueError as before."""
+        mock_query = MagicMock()
+        with pytest.raises(ValueError, match="does not exist on Slice"):
+            _SliceDAO.apply_column_operators(
+                mock_query,
+                [
+                    ColumnOperator(
+                        col="does_not_exist", opr=ColumnOperatorEnum.eq, value=1
+                    )
+                ],
+            )
+
+    @pytest.mark.parametrize(
+        "operator",
+        [
+            ColumnOperatorEnum.eq,
+            ColumnOperatorEnum.ne,
+            ColumnOperatorEnum.in_,
+            ColumnOperatorEnum.nin,
+            ColumnOperatorEnum.is_null,
+            ColumnOperatorEnum.is_not_null,
+        ],
+    )
+    def test_supported_relationship_operators_dispatch(self, operator):

Review Comment:
   **Suggestion:** Add type hints for the `operator` parameter and the return value (for example, `-> None`) so this newly added parameterized test method is fully typed. [custom_rule]
   
   **Severity Level:** Minor ⚠️
   <details>
   <summary><b>Why it matters? 🤔 </b></summary>
   
   This parameterized test method omits both the `operator` type annotation and the return type annotation. It is new Python code, so the missing type hints are a verified violation.
   </details>
   
   [![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=72bc27d768914942aae40d9d8f3ccc09&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=72bc27d768914942aae40d9d8f3ccc09&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:** tests/unit_tests/daos/test_base_relationship_filters.py
   **Line:** 115:115
   **Comment:**
   	*Custom Rule: Add type hints for the `operator` parameter and the return value (for example, `-> None`) so this newly added parameterized test method is fully typed.
   
   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%2F40397&comment_hash=0959dc463159029ab04bb6e2a087761510a9c672864bd4b2e98b11623299b22f&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40397&comment_hash=0959dc463159029ab04bb6e2a087761510a9c672864bd4b2e98b11623299b22f&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] feat(mcp): list_charts accepts `dashboards` filter [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 #40397:
URL: https://github.com/apache/superset/pull/40397#discussion_r3476453760


##########
tests/unit_tests/mcp_service/chart/tool/test_list_charts.py:
##########
@@ -184,6 +184,29 @@ def test_filter_validation(self):
         with pytest.raises(ValueError, match="Field required"):
             ChartFilter(col="slice_name")  # Missing opr and value
 
+    def test_dashboards_filter_accepted(self):

Review Comment:
   **Suggestion:** Add full type hints to this newly added test method, including an explicit return type, to satisfy the rule that new Python code must be fully typed. [custom_rule]
   
   **Severity Level:** Minor ⚠️
   <details>
   <summary><b>Why it matters? 🤔 </b></summary>
   
   This is a newly added Python test method, and it lacks both parameter annotations and an explicit return type.
   The rule requires new Python code to be fully typed, so this is a real violation.
   </details>
   
   [![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=3f424e8f59bc47ca89fd2eca08043c1a&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=3f424e8f59bc47ca89fd2eca08043c1a&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:** tests/unit_tests/mcp_service/chart/tool/test_list_charts.py
   **Line:** 187:187
   **Comment:**
   	*Custom Rule: Add full type hints to this newly added test method, including an explicit return type, to satisfy the rule that new Python code must be fully typed.
   
   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%2F40397&comment_hash=95c9ecab759578ca7adc8fe831fc0730c058dc04af5abc30ba1b5b01e63bcb5b&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40397&comment_hash=95c9ecab759578ca7adc8fe831fc0730c058dc04af5abc30ba1b5b01e63bcb5b&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] feat(mcp): list_charts accepts `dashboards` filter [superset]

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

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40397?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 `15.62500%` with `27 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 55.58%. Comparing base ([`a27ec19`](https://app.codecov.io/gh/apache/superset/commit/a27ec1923e54782f9213a1bab0acd0706b20af66?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`06041a7`](https://app.codecov.io/gh/apache/superset/commit/06041a786d0f14fdbfcd958bb9f3160d00f4fdd0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 185 commits behind head on master.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/40397?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/daos/base.py](https://app.codecov.io/gh/apache/superset/pull/40397?src=pr&el=tree&filepath=superset%2Fdaos%2Fbase.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvZGFvcy9iYXNlLnB5) | 12.90% | [27 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/40397?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 (a27ec19) and HEAD (06041a7). Click for more details.
   > 
   > <details><summary>HEAD has 95 uploads less than BASE</summary>
   >
   >| Flag | BASE (a27ec19) | HEAD (06041a7) |
   >|------|------|------|
   >|python|84|1|
   >|presto|13|1|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #40397      +/-   ##
   ==========================================
   - Coverage   64.31%   55.58%   -8.74%     
   ==========================================
     Files        2652     2652              
     Lines      144817   144848      +31     
     Branches    33419    33428       +9     
   ==========================================
   - Hits        93136    80509   -12627     
   - Misses      50017    63610   +13593     
   + Partials     1664      729     -935     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/40397/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/40397/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [mysql](https://app.codecov.io/gh/apache/superset/pull/40397/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/40397/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/40397/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.89% <15.62%> (-0.02%)` | :arrow_down: |
   | [python](https://app.codecov.io/gh/apache/superset/pull/40397/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.89% <15.62%> (-18.67%)` | :arrow_down: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/40397/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/40397/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/40397?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