codeant-ai-for-open-source[bot] commented on code in PR #41531:
URL: https://github.com/apache/superset/pull/41531#discussion_r3492418524
##########
superset/tasks/cache.py:
##########
@@ -73,12 +90,18 @@ class Strategy: # pylint: disable=too-few-public-methods
"""
+ name = ""
+ uses_webdriver = True
Review Comment:
**Suggestion:** Add explicit type annotations to these new class-level
attributes so the strategy interface is fully typed. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
These new strategy class-level attributes are untyped assignments in
modified Python code, which violates the requirement to add type hints to
relevant variables that can be annotated.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b92a629f558c4f70939593b0b060114b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b92a629f558c4f70939593b0b060114b&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/tasks/cache.py
**Line:** 93:94
**Comment:**
*Custom Rule: Add explicit type annotations to these new class-level
attributes so the strategy interface 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%2F41531&comment_hash=778c84f9d22bf18364f3b79b3ebecb0d0ae11a0c16ea88325ab3becb9b903a2d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41531&comment_hash=778c84f9d22bf18364f3b79b3ebecb0d0ae11a0c16ea88325ab3becb9b903a2d&reaction=dislike'>👎</a>
##########
superset/tasks/cache.py:
##########
@@ -201,7 +224,101 @@ def get_urls(self) -> list[str]:
return urls
-strategies = [DummyStrategy, TopNDashboardsStrategy, DashboardTagsStrategy]
+class NativeFilterOptionsStrategy(Strategy): # pylint:
disable=too-few-public-methods
+ """
+ Build chart data query tasks for native filter option cache warm-up.
+ """
+
+ name = "native_filter_options"
+ uses_webdriver = False
+
+ def __init__(self, dashboard_ids: list[int]) -> None:
+ super().__init__()
+ self.dashboard_ids = dashboard_ids
+
+ def get_urls(self) -> list[str]:
+ return []
+
+ def get_tasks(self) -> list[CacheWarmupTask]:
+ tasks: list[CacheWarmupTask] = []
+
+ for dashboard_id in self.dashboard_ids:
+ skipped = 0
+ built = 0
+
+ try:
+ dashboard = DashboardDAO.find_by_id(dashboard_id)
+ if dashboard is None:
+ logger.warning(
+ "Dashboard %s not found; skipping native filter option
"
+ "cache warm-up",
+ dashboard_id,
+ )
+ continue
+
+ filter_configs = get_eligible_native_filters(dashboard)
+
+ for filter_config in filter_configs:
+ try:
+ form_data = build_native_filter_option_form_data(
+ dashboard,
+ filter_config,
+ )
+ if form_data is None:
+ skipped += 1
+ continue
+
+ query_context =
build_native_filter_option_query_context(
+ form_data
+ )
+ if query_context is None:
+ skipped += 1
+ continue
+
+ tasks.append(
+ CacheWarmupTask(
+ query_context=query_context,
+ dashboard_id=dashboard.id,
+ native_filter_id=filter_config.get("id"),
+ )
+ )
+ built += 1
+ except Exception: # noqa: BLE001
+ skipped += 1
+ logger.exception(
+ "Error building native filter option cache warm-up
"
+ "task for dashboard %s filter %s",
+ dashboard_id,
+ filter_config.get("id"),
+ )
+
+ logger.info(
+ "Dashboard %s native filter option cache warm-up: %s
filters "
+ "found, %s tasks built, %s skipped",
+ dashboard_id,
+ len(filter_configs),
+ built,
+ skipped,
+ )
+ except Exception: # noqa: BLE001
+ logger.exception(
+ "Error building native filter option cache warm-up tasks
for "
+ "dashboard %s",
+ dashboard_id,
+ )
+
+ return tasks
+
+
+strategies = [
+ DummyStrategy,
+ TopNDashboardsStrategy,
+ DashboardTagsStrategy,
+ NativeFilterOptionsStrategy,
Review Comment:
**Suggestion:** Add a concrete type annotation for this new module-level
collection so its element type is explicit. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This new module-level collection is introduced without a type annotation,
and it is a relevant variable that can be annotated under the custom Python
type-hint rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f4701d34c14447d6b8bc7527004d5784&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f4701d34c14447d6b8bc7527004d5784&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/tasks/cache.py
**Line:** 313:317
**Comment:**
*Custom Rule: Add a concrete type annotation for this new module-level
collection so its element type is explicit.
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%2F41531&comment_hash=585ba2d55ccfafcdd2e0657f65efc9820f3ef813342f8ebb3f38a91b0bea0fbe&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41531&comment_hash=585ba2d55ccfafcdd2e0657f65efc9820f3ef813342f8ebb3f38a91b0bea0fbe&reaction=dislike'>👎</a>
##########
superset/tasks/cache.py:
##########
@@ -201,7 +224,101 @@ def get_urls(self) -> list[str]:
return urls
-strategies = [DummyStrategy, TopNDashboardsStrategy, DashboardTagsStrategy]
+class NativeFilterOptionsStrategy(Strategy): # pylint:
disable=too-few-public-methods
+ """
+ Build chart data query tasks for native filter option cache warm-up.
+ """
+
+ name = "native_filter_options"
+ uses_webdriver = False
Review Comment:
**Suggestion:** Add explicit type hints to these class attributes in the new
strategy implementation to satisfy typing requirements. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
These added class attributes in the new strategy are not type annotated, so
the suggestion correctly identifies missing type hints on newly modified Python
code.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f277f10d3f1946ee9c7899e7b508628a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f277f10d3f1946ee9c7899e7b508628a&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/tasks/cache.py
**Line:** 232:233
**Comment:**
*Custom Rule: Add explicit type hints to these class attributes in the
new strategy implementation to satisfy typing requirements.
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%2F41531&comment_hash=a322fdde085baafd55997d8ee7225b9b78d2245db6bb36b6349f7ed365296ef8&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41531&comment_hash=a322fdde085baafd55997d8ee7225b9b78d2245db6bb36b6349f7ed365296ef8&reaction=dislike'>👎</a>
##########
tests/unit_tests/tasks/test_cache.py:
##########
@@ -134,3 +134,129 @@ def side_effect(url: str, _element: str) -> None:
"errors": ["http://localhost/dash/boom";],
}
driver.destroy.assert_called_once_with()
+
+
+def test_native_filter_options_strategy_returns_tasks_for_eligible_filters()
-> None:
+ """Eligible native filters are converted into cache warm-up tasks."""
+ from superset.tasks.cache import NativeFilterOptionsStrategy
+
+ dashboard = mock.MagicMock()
Review Comment:
**Suggestion:** Add an explicit type annotation for this mocked dashboard
variable to satisfy the type-hint requirement for annotatable local variables.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This new local variable is introduced without any type annotation even
though it is a clearly annotatable Python variable, so it matches the type-hint
rule for modified code.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=bb38fc6a05bd4eb6bfd7840ae757bb05&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=bb38fc6a05bd4eb6bfd7840ae757bb05&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/tasks/test_cache.py
**Line:** 143:143
**Comment:**
*Custom Rule: Add an explicit type annotation for this mocked dashboard
variable to satisfy the type-hint requirement for annotatable local variables.
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%2F41531&comment_hash=b3acefc7b4c713d50f1093b92c293f9520341dfa096340b005e8e9761f95fd24&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41531&comment_hash=b3acefc7b4c713d50f1093b92c293f9520341dfa096340b005e8e9761f95fd24&reaction=dislike'>👎</a>
##########
tests/unit_tests/tasks/test_cache.py:
##########
@@ -134,3 +134,129 @@ def side_effect(url: str, _element: str) -> None:
"errors": ["http://localhost/dash/boom";],
}
driver.destroy.assert_called_once_with()
+
+
+def test_native_filter_options_strategy_returns_tasks_for_eligible_filters()
-> None:
+ """Eligible native filters are converted into cache warm-up tasks."""
+ from superset.tasks.cache import NativeFilterOptionsStrategy
+
+ dashboard = mock.MagicMock()
+ dashboard.id = 10
+ filter_configs = [{"id": "filter-1"}, {"id": "filter-2"}]
Review Comment:
**Suggestion:** Add a concrete collection type annotation for this filter
configuration list instead of leaving it untyped. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly added list is a concrete local variable that can be annotated,
but no type hint is present, so it violates the requirement to type-hint
relevant variables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a8d2d8da0f76426cbb69afa81ad169b7&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a8d2d8da0f76426cbb69afa81ad169b7&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/tasks/test_cache.py
**Line:** 145:145
**Comment:**
*Custom Rule: Add a concrete collection type annotation for this filter
configuration list instead of leaving it untyped.
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%2F41531&comment_hash=a827b64afcf77308eb2670869d13341ea48877545a3b5126490add2da7da828f&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41531&comment_hash=a827b64afcf77308eb2670869d13341ea48877545a3b5126490add2da7da828f&reaction=dislike'>👎</a>
##########
tests/unit_tests/tasks/test_cache.py:
##########
@@ -134,3 +134,129 @@ def side_effect(url: str, _element: str) -> None:
"errors": ["http://localhost/dash/boom";],
}
driver.destroy.assert_called_once_with()
+
+
+def test_native_filter_options_strategy_returns_tasks_for_eligible_filters()
-> None:
+ """Eligible native filters are converted into cache warm-up tasks."""
+ from superset.tasks.cache import NativeFilterOptionsStrategy
+
+ dashboard = mock.MagicMock()
+ dashboard.id = 10
+ filter_configs = [{"id": "filter-1"}, {"id": "filter-2"}]
+ form_data = [{"groupby": ["country"]}, {"groupby": ["state"]}]
Review Comment:
**Suggestion:** Add an explicit type annotation for this form-data list to
comply with the requirement for annotating relevant local variables.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a new local collection with a clear structure, and it is left
unannotated even though a type hint can be added, which matches the stated rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=12a741c52f414eb9a025406ab88ffa31&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=12a741c52f414eb9a025406ab88ffa31&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/tasks/test_cache.py
**Line:** 146:146
**Comment:**
*Custom Rule: Add an explicit type annotation for this form-data list
to comply with the requirement for annotating relevant local variables.
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%2F41531&comment_hash=ad4f21751794ed999ee4e1b27afc8bedec8f0171a353aae09ce32c3f0b0327c2&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41531&comment_hash=ad4f21751794ed999ee4e1b27afc8bedec8f0171a353aae09ce32c3f0b0327c2&reaction=dislike'>👎</a>
##########
tests/unit_tests/tasks/test_native_filter_cache.py:
##########
@@ -0,0 +1,217 @@
+# 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.
+from __future__ import annotations
+
+from typing import Any
+from unittest import mock
+
+from superset.common.chart_data import ChartDataResultFormat,
ChartDataResultType
+from superset.common.query_context import QueryContext
+from superset.tasks.native_filter_cache import (
+ build_native_filter_option_form_data,
+ build_native_filter_option_query_context,
+ get_eligible_native_filters,
+)
+from superset.utils import json
+
+
+def _dashboard(json_metadata: str | None, dashboard_id: int = 1) ->
mock.MagicMock:
+ dashboard = mock.MagicMock()
Review Comment:
**Suggestion:** Add an explicit type annotation for this local helper
variable to satisfy the type-hint requirement for annotatable variables.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This local variable is assigned a mock object without any type annotation.
Under the type-hint rule, it is a new Python variable in modified code that
could be annotated explicitly.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=58cc2c41d00440f4babbd0db0c538e22&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=58cc2c41d00440f4babbd0db0c538e22&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/tasks/test_native_filter_cache.py
**Line:** 33:33
**Comment:**
*Custom Rule: Add an explicit type annotation for this local helper
variable to satisfy the type-hint requirement for annotatable variables.
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%2F41531&comment_hash=81a88df38c413f7e94321e870573ae62f9a198500765870585fe236f48f6fe4a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41531&comment_hash=81a88df38c413f7e94321e870573ae62f9a198500765870585fe236f48f6fe4a&reaction=dislike'>👎</a>
##########
tests/unit_tests/tasks/test_native_filter_cache.py:
##########
@@ -0,0 +1,217 @@
+# 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.
+from __future__ import annotations
+
+from typing import Any
+from unittest import mock
+
+from superset.common.chart_data import ChartDataResultFormat,
ChartDataResultType
+from superset.common.query_context import QueryContext
+from superset.tasks.native_filter_cache import (
+ build_native_filter_option_form_data,
+ build_native_filter_option_query_context,
+ get_eligible_native_filters,
+)
+from superset.utils import json
+
+
+def _dashboard(json_metadata: str | None, dashboard_id: int = 1) ->
mock.MagicMock:
+ dashboard = mock.MagicMock()
+ dashboard.id = dashboard_id
+ dashboard.json_metadata = json_metadata
+ return dashboard
+
+
+def _filter_config(
+ filter_id: str = "filter-1",
+ filter_type: str = "filter_select",
+ entry_type: str = "NATIVE_FILTER",
+ targets: list[dict[str, Any]] | None = None,
+) -> dict[str, Any]:
+ return {
+ "id": filter_id,
+ "filterType": filter_type,
+ "type": entry_type,
+ "targets": targets
+ if targets is not None
+ else [{"datasetId": 10, "column": {"name": "country"}}],
+ }
+
+
+def test_get_eligible_native_filters_returns_only_filter_select() -> None:
+ filter_select = _filter_config(filter_id="select")
+ metadata = {
Review Comment:
**Suggestion:** Add a concrete type hint for this dictionary variable so the
test data structure is explicitly typed. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This dictionary is a newly added local variable with a clear structure and
can be annotated, but it is not. That matches the custom type-hint requirement.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2fc842e43c3e456eb9cb288d4d4e9470&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=2fc842e43c3e456eb9cb288d4d4e9470&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/tasks/test_native_filter_cache.py
**Line:** 57:57
**Comment:**
*Custom Rule: Add a concrete type hint for this dictionary variable so
the test data structure is explicitly 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%2F41531&comment_hash=f9da2912decd499925d8cbe68053e486303031e6522216c5aa07abb1020afc6d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41531&comment_hash=f9da2912decd499925d8cbe68053e486303031e6522216c5aa07abb1020afc6d&reaction=dislike'>👎</a>
##########
tests/unit_tests/tasks/test_native_filter_cache.py:
##########
@@ -0,0 +1,217 @@
+# 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.
+from __future__ import annotations
+
+from typing import Any
+from unittest import mock
+
+from superset.common.chart_data import ChartDataResultFormat,
ChartDataResultType
+from superset.common.query_context import QueryContext
+from superset.tasks.native_filter_cache import (
+ build_native_filter_option_form_data,
+ build_native_filter_option_query_context,
+ get_eligible_native_filters,
+)
+from superset.utils import json
+
+
+def _dashboard(json_metadata: str | None, dashboard_id: int = 1) ->
mock.MagicMock:
+ dashboard = mock.MagicMock()
+ dashboard.id = dashboard_id
+ dashboard.json_metadata = json_metadata
+ return dashboard
+
+
+def _filter_config(
+ filter_id: str = "filter-1",
+ filter_type: str = "filter_select",
+ entry_type: str = "NATIVE_FILTER",
+ targets: list[dict[str, Any]] | None = None,
+) -> dict[str, Any]:
+ return {
+ "id": filter_id,
+ "filterType": filter_type,
+ "type": entry_type,
+ "targets": targets
+ if targets is not None
+ else [{"datasetId": 10, "column": {"name": "country"}}],
+ }
+
+
+def test_get_eligible_native_filters_returns_only_filter_select() -> None:
+ filter_select = _filter_config(filter_id="select")
+ metadata = {
+ "native_filter_configuration": [
+ filter_select,
+ _filter_config(filter_id="range", filter_type="filter_range"),
+ _filter_config(filter_id="divider", entry_type="DIVIDER"),
+ ]
+ }
+
+ result = get_eligible_native_filters(_dashboard(json.dumps(metadata)))
+
+ assert result == [filter_select]
+
+
+def test_get_eligible_native_filters_skips_missing_target() -> None:
+ metadata = {
+ "native_filter_configuration": [
+ _filter_config(targets=[]),
+ ]
+ }
+
+ result = get_eligible_native_filters(_dashboard(json.dumps(metadata)))
+
+ assert result == []
+
+
+def test_get_eligible_native_filters_skips_missing_column() -> None:
+ metadata = {
+ "native_filter_configuration": [
+ _filter_config(targets=[{"datasetId": 10, "column": {}}]),
+ ]
+ }
+
+ result = get_eligible_native_filters(_dashboard(json.dumps(metadata)))
+
+ assert result == []
+
+
+def test_get_eligible_native_filters_malformed_json_metadata() -> None:
+ result = get_eligible_native_filters(_dashboard("{"))
+
+ assert result == []
+
+
+def test_get_eligible_native_filters_missing_native_filter_configuration() ->
None:
+ result = get_eligible_native_filters(_dashboard(json.dumps({"label":
"dash"})))
+
+ assert result == []
+
+
+def test_build_native_filter_option_form_data_correct_shape() -> None:
+ dashboard = _dashboard(json_metadata=None, dashboard_id=42)
+ filter_config = {
+ **_filter_config(filter_id="native-filter-1"),
+ "adhocFilters": [{"expressionType": "SIMPLE"}],
+ "controlValues": {"sortAscending": False},
+ "sortMetric": "sum__value",
+ }
+
+ result = build_native_filter_option_form_data(dashboard, filter_config)
+
+ assert result == {
+ "datasource": "10__table",
+ "viz_type": "filter_select",
+ "type": "NATIVE_FILTER",
+ "native_filter_id": "native-filter-1",
+ "dashboardId": 42,
+ "groupby": ["country"],
+ "adhoc_filters": [{"expressionType": "SIMPLE"}],
+ "extra_filters": [],
+ "extra_form_data": {},
+ "metrics": ["count"],
+ "row_limit": 1000,
+ "time_range": "No filter",
+ "granularity_sqla": None,
+ "showSearch": True,
+ "sortAscending": False,
+ "sortMetric": "sum__value",
+ }
+
+
+def test_build_native_filter_option_form_data_missing_dataset_id() -> None:
+ result = build_native_filter_option_form_data(
+ _dashboard(json_metadata=None),
+ _filter_config(targets=[{"column": {"name": "country"}}]),
+ )
+
+ assert result is None
+
+
+def test_build_native_filter_option_form_data_missing_column_name() -> None:
+ result = build_native_filter_option_form_data(
+ _dashboard(json_metadata=None),
+ _filter_config(targets=[{"datasetId": 10, "column": {}}]),
+ )
+
+ assert result is None
+
+
+def test_build_native_filter_option_query_context_returns_query_context() ->
None:
+ form_data = build_native_filter_option_form_data(
+ _dashboard(json_metadata=None),
+ _filter_config(),
+ )
+ datasource = mock.MagicMock()
Review Comment:
**Suggestion:** Annotate this mocked datasource variable with its intended
interface/type to comply with the type-hint rule. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The datasource mock is assigned without an explicit type hint, and the
variable has a known intended interface in the test. This is a valid omission
under the type-hint rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e8519ef09cb34b97861beaf8b3c529d7&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e8519ef09cb34b97861beaf8b3c529d7&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/tasks/test_native_filter_cache.py
**Line:** 160:160
**Comment:**
*Custom Rule: Annotate this mocked datasource variable with its
intended interface/type to comply with the type-hint rule.
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%2F41531&comment_hash=8fc96b11d28ba7840733abe787523e2055329b4edfdafba95d88223d40a3d569&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41531&comment_hash=8fc96b11d28ba7840733abe787523e2055329b4edfdafba95d88223d40a3d569&reaction=dislike'>👎</a>
##########
tests/unit_tests/tasks/test_native_filter_cache.py:
##########
@@ -0,0 +1,217 @@
+# 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.
+from __future__ import annotations
+
+from typing import Any
+from unittest import mock
+
+from superset.common.chart_data import ChartDataResultFormat,
ChartDataResultType
+from superset.common.query_context import QueryContext
+from superset.tasks.native_filter_cache import (
+ build_native_filter_option_form_data,
+ build_native_filter_option_query_context,
+ get_eligible_native_filters,
+)
+from superset.utils import json
+
+
+def _dashboard(json_metadata: str | None, dashboard_id: int = 1) ->
mock.MagicMock:
+ dashboard = mock.MagicMock()
+ dashboard.id = dashboard_id
+ dashboard.json_metadata = json_metadata
+ return dashboard
+
+
+def _filter_config(
+ filter_id: str = "filter-1",
+ filter_type: str = "filter_select",
+ entry_type: str = "NATIVE_FILTER",
+ targets: list[dict[str, Any]] | None = None,
+) -> dict[str, Any]:
+ return {
+ "id": filter_id,
+ "filterType": filter_type,
+ "type": entry_type,
+ "targets": targets
+ if targets is not None
+ else [{"datasetId": 10, "column": {"name": "country"}}],
+ }
+
+
+def test_get_eligible_native_filters_returns_only_filter_select() -> None:
+ filter_select = _filter_config(filter_id="select")
+ metadata = {
+ "native_filter_configuration": [
+ filter_select,
+ _filter_config(filter_id="range", filter_type="filter_range"),
+ _filter_config(filter_id="divider", entry_type="DIVIDER"),
+ ]
+ }
+
+ result = get_eligible_native_filters(_dashboard(json.dumps(metadata)))
+
+ assert result == [filter_select]
+
+
+def test_get_eligible_native_filters_skips_missing_target() -> None:
+ metadata = {
+ "native_filter_configuration": [
+ _filter_config(targets=[]),
+ ]
+ }
+
+ result = get_eligible_native_filters(_dashboard(json.dumps(metadata)))
+
+ assert result == []
+
+
+def test_get_eligible_native_filters_skips_missing_column() -> None:
+ metadata = {
+ "native_filter_configuration": [
+ _filter_config(targets=[{"datasetId": 10, "column": {}}]),
+ ]
+ }
+
+ result = get_eligible_native_filters(_dashboard(json.dumps(metadata)))
+
+ assert result == []
+
+
+def test_get_eligible_native_filters_malformed_json_metadata() -> None:
+ result = get_eligible_native_filters(_dashboard("{"))
+
+ assert result == []
+
+
+def test_get_eligible_native_filters_missing_native_filter_configuration() ->
None:
+ result = get_eligible_native_filters(_dashboard(json.dumps({"label":
"dash"})))
+
+ assert result == []
+
+
+def test_build_native_filter_option_form_data_correct_shape() -> None:
+ dashboard = _dashboard(json_metadata=None, dashboard_id=42)
+ filter_config = {
+ **_filter_config(filter_id="native-filter-1"),
+ "adhocFilters": [{"expressionType": "SIMPLE"}],
+ "controlValues": {"sortAscending": False},
+ "sortMetric": "sum__value",
+ }
+
+ result = build_native_filter_option_form_data(dashboard, filter_config)
+
+ assert result == {
+ "datasource": "10__table",
+ "viz_type": "filter_select",
+ "type": "NATIVE_FILTER",
+ "native_filter_id": "native-filter-1",
+ "dashboardId": 42,
+ "groupby": ["country"],
+ "adhoc_filters": [{"expressionType": "SIMPLE"}],
+ "extra_filters": [],
+ "extra_form_data": {},
+ "metrics": ["count"],
+ "row_limit": 1000,
+ "time_range": "No filter",
+ "granularity_sqla": None,
+ "showSearch": True,
+ "sortAscending": False,
+ "sortMetric": "sum__value",
+ }
+
+
+def test_build_native_filter_option_form_data_missing_dataset_id() -> None:
+ result = build_native_filter_option_form_data(
+ _dashboard(json_metadata=None),
+ _filter_config(targets=[{"column": {"name": "country"}}]),
+ )
+
+ assert result is None
+
+
+def test_build_native_filter_option_form_data_missing_column_name() -> None:
+ result = build_native_filter_option_form_data(
+ _dashboard(json_metadata=None),
+ _filter_config(targets=[{"datasetId": 10, "column": {}}]),
+ )
+
+ assert result is None
+
+
+def test_build_native_filter_option_query_context_returns_query_context() ->
None:
+ form_data = build_native_filter_option_form_data(
+ _dashboard(json_metadata=None),
+ _filter_config(),
+ )
+ datasource = mock.MagicMock()
+ datasource.cache_timeout = None
+ datasource.type = "table"
+ datasource.data = {"id": 10}
+ datasource.uid = "10__table"
+ datasource.column_names = ["country"]
+
+ expected = QueryContext(
+ datasource=datasource,
+ queries=[],
+ slice_=None,
+ form_data=form_data,
+ result_type=ChartDataResultType.FULL,
+ result_format=ChartDataResultFormat.JSON,
+ force=False,
+ custom_cache_timeout=None,
+ cache_values={},
+ )
Review Comment:
**Suggestion:** Add an explicit type annotation for this expected object
variable instead of relying only on inference. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This test variable is created without a type annotation even though its role
as a QueryContext-like expected object is clear. That fits the custom rule
requiring type hints for annotatable variables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d2941f61e9134b44858ea378711dff5b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d2941f61e9134b44858ea378711dff5b&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/tasks/test_native_filter_cache.py
**Line:** 167:177
**Comment:**
*Custom Rule: Add an explicit type annotation for this expected object
variable instead of relying only on inference.
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%2F41531&comment_hash=d46aab65071a7a8fcea47a09fd704d6c8e89ed77322c7f3c996bad2799c5fbe5&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41531&comment_hash=d46aab65071a7a8fcea47a09fd704d6c8e89ed77322c7f3c996bad2799c5fbe5&reaction=dislike'>👎</a>
##########
tests/unit_tests/tasks/test_cache.py:
##########
@@ -134,3 +134,129 @@ def side_effect(url: str, _element: str) -> None:
"errors": ["http://localhost/dash/boom";],
}
driver.destroy.assert_called_once_with()
+
+
+def test_native_filter_options_strategy_returns_tasks_for_eligible_filters()
-> None:
+ """Eligible native filters are converted into cache warm-up tasks."""
+ from superset.tasks.cache import NativeFilterOptionsStrategy
+
+ dashboard = mock.MagicMock()
+ dashboard.id = 10
+ filter_configs = [{"id": "filter-1"}, {"id": "filter-2"}]
+ form_data = [{"groupby": ["country"]}, {"groupby": ["state"]}]
+ query_contexts = [mock.MagicMock(), mock.MagicMock()]
Review Comment:
**Suggestion:** Annotate this query-context mock list with an explicit list
element type to avoid omitted type hints. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This local list is introduced without a type annotation and can be
annotated, so the suggestion correctly identifies a type-hint omission.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=33df0371ee9a4fdeb7d2b74e0088579f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=33df0371ee9a4fdeb7d2b74e0088579f&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/tasks/test_cache.py
**Line:** 147:147
**Comment:**
*Custom Rule: Annotate this query-context mock list with an explicit
list element type to avoid omitted type hints.
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%2F41531&comment_hash=06417083f348ef6a3e46328c8f999235272ffdfe680502dd62f837d9ea6a9b42&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41531&comment_hash=06417083f348ef6a3e46328c8f999235272ffdfe680502dd62f837d9ea6a9b42&reaction=dislike'>👎</a>
##########
superset/tasks/cache.py:
##########
@@ -215,11 +332,8 @@ def cache_warmup(
"""
logger.info("Loading strategy")
- class_ = None
- for class_ in strategies:
- if class_.name == strategy_name: # type: ignore
- break
- else:
+ class_ = strategy_registry.get(strategy_name)
Review Comment:
**Suggestion:** Add a type annotation for this newly introduced local
variable to keep new logic fully type hinted. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly added local variable is not type annotated despite being a
variable in modified Python code that can be annotated, so it matches the
type-hint rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1047033fd6e24689a7927df3e946390b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=1047033fd6e24689a7927df3e946390b&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/tasks/cache.py
**Line:** 335:335
**Comment:**
*Custom Rule: Add a type annotation for this newly introduced local
variable to keep new logic fully type hinted.
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%2F41531&comment_hash=97aa5a1f56ba959ac2b77f5393d5d0c88eb455048622eb42b5d302a6a8aa4fd7&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41531&comment_hash=97aa5a1f56ba959ac2b77f5393d5d0c88eb455048622eb42b5d302a6a8aa4fd7&reaction=dislike'>👎</a>
##########
superset/tasks/native_filter_cache.py:
##########
@@ -0,0 +1,210 @@
+# 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.
+from __future__ import annotations
+
+import logging
+from typing import Any
+
+from marshmallow import ValidationError
+
+from superset.charts.schemas import ChartDataQueryContextSchema
+from superset.common.chart_data import ChartDataResultFormat,
ChartDataResultType
+from superset.common.query_context import QueryContext
+from superset.models.dashboard import Dashboard
+from superset.utils import json
+from superset.utils.core import DatasourceType
+
+logger = logging.getLogger(__name__)
+
+NATIVE_FILTER_DEFAULT_ROW_LIMIT = 1000
+NO_FILTER_TIME_RANGE = "No filter"
+
+
+def _get_filter_target(filter_config: dict[str, Any]) -> tuple[int | str, str]
| None:
+ for target in filter_config.get("targets") or []:
+ if not isinstance(target, dict):
+ continue
+
+ dataset_id = target.get("datasetId")
+ column = target.get("column") or {}
+ column_name = column.get("name") if isinstance(column, dict) else None
+
+ if dataset_id is not None and column_name:
+ return dataset_id, column_name
+
+ return None
+
+
+def get_eligible_native_filters(dashboard: Dashboard) -> list[dict[str, Any]]:
+ """Return native filter value filters eligible for option cache warm-up."""
+ if not dashboard.json_metadata:
+ logger.warning(
+ "Dashboard %s has no json_metadata; skipping native filter
warm-up",
+ dashboard.id,
+ )
+ return []
+
+ try:
+ metadata = json.loads(dashboard.json_metadata)
Review Comment:
**Suggestion:** Add an explicit type annotation for this parsed JSON
variable to satisfy the type-hint requirement for relevant variables.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a new Python statement in the added file, and the local variable
`metadata` is assigned the result of `json.loads(...)` without any type
annotation. Under the type-hint rule, this is a relevant variable that can be
annotated, so the suggestion matches a real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2fd6c640dc46443295cfc1867885e7b3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=2fd6c640dc46443295cfc1867885e7b3&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/tasks/native_filter_cache.py
**Line:** 62:62
**Comment:**
*Custom Rule: Add an explicit type annotation for this parsed JSON
variable to satisfy the type-hint requirement for relevant variables.
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%2F41531&comment_hash=0f228960bb73a6a04dccdc7ef16fe905f961df37c04c001620fa55b8c90c2799&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41531&comment_hash=0f228960bb73a6a04dccdc7ef16fe905f961df37c04c001620fa55b8c90c2799&reaction=dislike'>👎</a>
##########
tests/unit_tests/tasks/test_cache.py:
##########
@@ -134,3 +134,129 @@ def side_effect(url: str, _element: str) -> None:
"errors": ["http://localhost/dash/boom";],
}
driver.destroy.assert_called_once_with()
+
+
+def test_native_filter_options_strategy_returns_tasks_for_eligible_filters()
-> None:
+ """Eligible native filters are converted into cache warm-up tasks."""
+ from superset.tasks.cache import NativeFilterOptionsStrategy
+
+ dashboard = mock.MagicMock()
+ dashboard.id = 10
+ filter_configs = [{"id": "filter-1"}, {"id": "filter-2"}]
+ form_data = [{"groupby": ["country"]}, {"groupby": ["state"]}]
+ query_contexts = [mock.MagicMock(), mock.MagicMock()]
+
+ with (
+ mock.patch(
+ "superset.tasks.cache.DashboardDAO.find_by_id",
+ return_value=dashboard,
+ ),
+ mock.patch(
+ "superset.tasks.cache.get_eligible_native_filters",
+ return_value=filter_configs,
+ ),
+ mock.patch(
+ "superset.tasks.cache.build_native_filter_option_form_data",
+ side_effect=form_data,
+ ) as mock_build_form_data,
+ mock.patch(
+ "superset.tasks.cache.build_native_filter_option_query_context",
+ side_effect=query_contexts,
+ ) as mock_build_query_context,
+ ):
+ tasks = NativeFilterOptionsStrategy(dashboard_ids=[10]).get_tasks()
+
+ assert len(tasks) == 2
+ assert [task.query_context for task in tasks] == query_contexts
+ assert [task.dashboard_id for task in tasks] == [10, 10]
+ assert [task.native_filter_id for task in tasks] == ["filter-1",
"filter-2"]
+ assert mock_build_form_data.call_count == 2
+ assert mock_build_query_context.call_count == 2
+
+
+def test_native_filter_options_strategy_skips_missing_dashboard() -> None:
+ """Missing dashboards are skipped without raising."""
+ from superset.tasks.cache import NativeFilterOptionsStrategy
+
+ with (
+ mock.patch(
+ "superset.tasks.cache.DashboardDAO.find_by_id",
+ return_value=None,
+ ),
+ mock.patch("superset.tasks.cache.get_eligible_native_filters") as
mock_filters,
+ ):
+ tasks = NativeFilterOptionsStrategy(dashboard_ids=[10]).get_tasks()
+
+ assert tasks == []
+ mock_filters.assert_not_called()
+
+
+def test_native_filter_options_strategy_skips_when_form_data_is_none() -> None:
+ """Filters without form data are skipped without raising."""
+ from superset.tasks.cache import NativeFilterOptionsStrategy
+
+ dashboard = mock.MagicMock()
+ dashboard.id = 10
+ filter_configs = [{"id": "filter-1"}, {"id": "filter-2"}]
+ query_context = mock.MagicMock()
Review Comment:
**Suggestion:** Add a type annotation for this local query context mock
variable rather than leaving it inferred. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This added local variable is left untyped even though it is clearly
annotatable, which is a real violation of the custom type-hint requirement.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=04714a0b3d254f9f928935034c024dcb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=04714a0b3d254f9f928935034c024dcb&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/tasks/test_cache.py
**Line:** 201:201
**Comment:**
*Custom Rule: Add a type annotation for this local query context mock
variable rather than leaving it inferred.
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%2F41531&comment_hash=0d71702ec830ef1b926cd4c26af48439550e6111063be6394bd10a686c6066a5&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41531&comment_hash=0d71702ec830ef1b926cd4c26af48439550e6111063be6394bd10a686c6066a5&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
