codeant-ai-for-open-source[bot] commented on code in PR #40003:
URL: https://github.com/apache/superset/pull/40003#discussion_r3456234755
##########
superset-frontend/src/dashboard/components/nativeFilters/selectors.ts:
##########
@@ -141,9 +141,20 @@ const selectIndicatorsForChartFromFilter = (
}));
};
+const getQueryFilterMetadata = (
Review Comment:
**Suggestion:** Replace the `any` type on the new helper parameter with a
concrete chart/query response type (or a narrowed reusable interface) so filter
metadata access is type-safe. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The modified TypeScript code introduces `chart: any` in a new helper, which
directly violates the rule against using `any` in new or changed TS/TSX code. A
concrete chart/query response type or narrower reusable interface should be
used instead.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9b41b84095fe4e01aa47a62f709c85e1&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=9b41b84095fe4e01aa47a62f709c85e1&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-frontend/src/dashboard/components/nativeFilters/selectors.ts
**Line:** 144:144
**Comment:**
*Custom Rule: Replace the `any` type on the new helper parameter with a
concrete chart/query response type (or a narrowed reusable interface) so filter
metadata access is type-safe.
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%2F40003&comment_hash=80fd26ac3b0d75b100ba1606c37fd5dac3bfe45a66b35a366eed8490c6c692ef&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40003&comment_hash=80fd26ac3b0d75b100ba1606c37fd5dac3bfe45a66b35a366eed8490c6c692ef&reaction=dislike'>👎</a>
##########
tests/integration_tests/viz_tests.py:
##########
@@ -1849,6 +1849,79 @@ def test_get_data_filters_none_data_slices(self,
mock_db_session, mock_viz_types
assert len(result["slices"]) == 1
assert result["slices"][0] == slice_1.data
+ @with_config({"MAPBOX_API_KEY": "test_key"})
+ @patch("superset.viz.viz_types")
+ @patch("superset.db.session")
+ def test_get_payload_includes_subslice_filter_metadata(
+ self,
+ mock_db_session,
+ mock_viz_types,
+ ):
Review Comment:
**Suggestion:** Add full type hints to this newly added test method,
including annotations for injected mock parameters and an explicit `-> None`
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 in a changed hunk, and it omits
type hints for its injected mock parameters as well as an explicit return
annotation. That matches the custom rule requiring new Python code to be fully
typed.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ebeee65183e24ec58a24942704e166a2&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=ebeee65183e24ec58a24942704e166a2&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/integration_tests/viz_tests.py
**Line:** 1855:1859
**Comment:**
*Custom Rule: Add full type hints to this newly added test method,
including annotations for injected mock parameters and an explicit `-> None`
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%2F40003&comment_hash=6d99b34abdc9299bbd4eaa9bf5734f06b3dd1796a31f49bc6d9cd68b445bdd90&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40003&comment_hash=6d99b34abdc9299bbd4eaa9bf5734f06b3dd1796a31f49bc6d9cd68b445bdd90&reaction=dislike'>👎</a>
##########
superset/viz.py:
##########
@@ -1626,6 +1626,27 @@ class DeckGLMultiLayer(BaseViz):
is_timeseries = False
credits = '<a href="https://uber.github.io/deck.gl/";>deck.gl</a>'
+ @staticmethod
+ def _merge_filter_metadata(
+ *filter_groups: list[dict[str, Any]] | None,
+ ) -> list[dict[str, Any]]:
+ merged_filters: list[dict[str, Any]] = []
+ seen_filters: set[str] = set()
Review Comment:
**Suggestion:** Add a short docstring to this new helper method describing
that it merges multiple filter metadata lists while de-duplicating entries.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly introduced Python method and it does not include a docstring.
The custom rule requires new functions and classes to be documented inline,
so
the suggestion correctly identifies a real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b521066d8a13402a9c60be1ebcf28253&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=b521066d8a13402a9c60be1ebcf28253&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/viz.py
**Line:** 1629:1634
**Comment:**
*Custom Rule: Add a short docstring to this new helper method
describing that it merges multiple filter metadata lists while de-duplicating
entries.
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%2F40003&comment_hash=c7a032ddd705f42756fbbb7403c662cf123745087145add1f8b9aa200c043f35&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40003&comment_hash=c7a032ddd705f42756fbbb7403c662cf123745087145add1f8b9aa200c043f35&reaction=dislike'>👎</a>
##########
superset/viz.py:
##########
@@ -1755,6 +1787,19 @@ def get_data(self, df: pd.DataFrame) -> VizData:
"slices": [slc.data for slc in slices if slc.data is not None],
}
+ @deprecated(deprecated_in="3.0")
+ def get_payload(self, query_obj: QueryObjectDict | None = None) ->
VizPayload:
+ payload = super().get_payload(query_obj)
+ payload["applied_filters"] = self._merge_filter_metadata(
+ payload.get("applied_filters"),
+ self.applied_filters,
+ )
+ payload["rejected_filters"] = self._merge_filter_metadata(
+ payload.get("rejected_filters"),
+ self.rejected_filters,
+ )
+ return payload
Review Comment:
**Suggestion:** Add a docstring to this newly introduced method to explain
that it extends the base payload by merging layer-level applied and rejected
filter metadata. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added Python method and it has no docstring.
The custom rule says new functions and classes should be documented inline,
so
this is a genuine violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7cc59b445f084cd48c4e165638a6ca0d&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=7cc59b445f084cd48c4e165638a6ca0d&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/viz.py
**Line:** 1790:1801
**Comment:**
*Custom Rule: Add a docstring to this newly introduced method to
explain that it extends the base payload by merging layer-level applied and
rejected filter metadata.
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%2F40003&comment_hash=a7bb1a598fdbb5efb9d1d1a3879084901e876e12e95eb96e885a28274c80acf4&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40003&comment_hash=a7bb1a598fdbb5efb9d1d1a3879084901e876e12e95eb96e885a28274c80acf4&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]
