codeant-ai-for-open-source[bot] commented on code in PR #40960:
URL: https://github.com/apache/superset/pull/40960#discussion_r3501391058
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -1717,3 +1717,217 @@ def dashboard_layout_serializer(dashboard: "Dashboard")
-> DashboardLayout:
has_layout=bool(position_json_str),
)
)
+
+
+# ---------------------------------------------------------------------------
+# manage_native_filters schemas
+# ---------------------------------------------------------------------------
+
+
+class BaseNewFilterSpec(BaseModel):
+ """Common fields shared by all new native filter specs."""
+
+ name: str = Field(..., min_length=1, description="Filter display name")
+ description: str = Field("", description="Optional filter description")
+ scope_chart_ids: List[int] | None = Field(
+ None,
+ description=(
+ "Chart IDs this filter should apply to. When omitted the filter "
+ "applies to all charts on the dashboard. All IDs must belong to "
+ "charts that are on the dashboard."
+ ),
+ )
+
+
+class FilterSelectSpec(BaseNewFilterSpec):
+ """Spec for a new dropdown (filter_select) native filter."""
+
+ filter_type: Literal["filter_select"] = Field(
+ ..., description="Discriminator - must be 'filter_select'"
+ )
+ dataset_id: int = Field(..., description="ID of the dataset to filter on")
+ column: str = Field(
+ ..., min_length=1, description="Name of the dataset column to filter
on"
+ )
+ multi_select: bool = Field(
+ True, description="Allow selecting multiple values (default True)"
+ )
+ default_to_first_item: bool = Field(
+ False, description="Default the filter to the first item in the list"
+ )
+ enable_empty_filter: bool = Field(
+ False, description="Require a value before the filter is applied"
+ )
Review Comment:
**Suggestion:** The `enable_empty_filter` field description is inverted: it
currently says the flag “Require[s] a value before the filter is applied,” but
this option enables empty-filter behavior. This mismatch can cause callers
(including LLM tool users) to send the opposite value and get incorrect runtime
behavior. Update the description to clearly state that enabling it allows
applying the filter with an empty/unset value. [comment mismatch]
<details>
<summary><b>Severity Level:</b> Minor 🧹</summary>
```mdx
⚠️ Suggestion addresses non-issue; description matches frontend semantics.
⚠️ No misconfiguration risk; enable_empty_filter already acts as required.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open `superset/mcp_service/dashboard/schemas.py` and inspect
`FilterSelectSpec` around
lines 1742-1760; at line 1758 the field `enable_empty_filter: bool =
Field(False,
description="Require a value before the filter is applied")` defines the
schema
description for MCP callers.
2. Open the MCP tool implementation at
`superset/mcp_service/dashboard/tool/manage_native_filters.py` and inspect
`_build_new_filter_config` around lines 130-140; here
`controlValues["enableEmptyFilter"]
= spec.enable_empty_filter` shows the backend passes the boolean straight
through to the
dashboard’s `controlValues.enableEmptyFilter` key without inversion.
3. Inspect the frontend native filter configuration logic in
`superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/state.ts`
lines 56-65; `const enableEmptyFilter =
!!formFilter?.controlValues?.enableEmptyFilter`
and `const required = enableEmptyFilter && !defaultToFirstItem;` then
`setIsRequired(required)` demonstrate that a true `enableEmptyFilter` flag
is treated as
“required” in the UI state, matching the description “Require a value before
the filter is
applied”.
4. Inspect
`superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts`
lines 278-288; it maps `isRequired:
formInputs.controlValues?.enableEmptyFilter || false`,
again treating `enableEmptyFilter` as the “required” flag, confirming that
the current
schema description is aligned with actual usage and the suggestion’s claim
that it
“enables empty-filter behavior” is not supported by the code, making this a
non-issue.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=80b0248c898548799120f0b0dab32f6f&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=80b0248c898548799120f0b0dab32f6f&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/dashboard/schemas.py
**Line:** 1758:1760
**Comment:**
*Comment Mismatch: The `enable_empty_filter` field description is
inverted: it currently says the flag “Require[s] a value before the filter is
applied,” but this option enables empty-filter behavior. This mismatch can
cause callers (including LLM tool users) to send the opposite value and get
incorrect runtime behavior. Update the description to clearly state that
enabling it allows applying the filter with an empty/unset value.
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%2F40960&comment_hash=2b8c7a817ddd20b69e97b5aaa94b555237b22b344d504bda5c6d36851a8493bd&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40960&comment_hash=2b8c7a817ddd20b69e97b5aaa94b555237b22b344d504bda5c6d36851a8493bd&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]
