codeant-ai-for-open-source[bot] commented on code in PR #38676:
URL: https://github.com/apache/superset/pull/38676#discussion_r2981780905
##########
superset/reports/models.py:
##########
@@ -237,62 +237,85 @@ def _generate_native_filter(
filter_type: str,
column_name: str,
values: list[Optional[str]],
- ) -> dict[str, Any]:
+ ) -> tuple[dict[str, Any], Optional[str]]:
+ """
+ Generate a native filter configuration for the given filter type.
+
+ Returns:
+ A tuple of (filter_config, warning_message). If the filter type is
+ unrecognized, returns an empty dict and a warning message.
+ """
if filter_type == "filter_time":
# For select filters, we need to use the "IN" operator
- return {
- native_filter_id or "": {
- "id": native_filter_id or "",
- "extraFormData": {"time_range": values[0]},
- "filterState": {"value": values[0]},
- "ownState": {},
- }
- }
- elif filter_type == "filter_timegrain":
- return {
- native_filter_id or "": {
- "id": native_filter_id or "",
- "extraFormData": {
- "time_grain_sqla": values[0], # grain
- },
- "filterState": {
- # "label": "30 second", # grain_label
- "value": values # grain
- },
- "ownState": {},
- }
- }
-
- elif filter_type == "filter_timecolumn":
- return {
- native_filter_id or "": {
- "extraFormData": {
- "granularity_sqla": values[0] # column_name
- },
- "filterState": {
- "value": values # column_name
- },
- }
- }
-
- elif filter_type == "filter_select":
- return {
- native_filter_id or "": {
- "id": native_filter_id or "",
- "extraFormData": {
- "filters": [
- {"col": column_name or "", "op": "IN", "val":
values or []}
- ]
- },
- "filterState": {
- "label": column_name or "",
- "validateStatus": False,
- "value": values or [],
- },
- "ownState": {},
- }
- }
- elif filter_type == "filter_range":
+ return (
+ {
+ native_filter_id or "": {
+ "id": native_filter_id or "",
+ "extraFormData": {"time_range": values[0]},
+ "filterState": {"value": values[0]},
+ "ownState": {},
+ }
+ },
+ None,
+ )
+ if filter_type == "filter_timegrain":
+ return (
+ {
+ native_filter_id or "": {
+ "id": native_filter_id or "",
+ "extraFormData": {
+ "time_grain_sqla": values[0], # grain
+ },
+ "filterState": {
+ # "label": "30 second", # grain_label
+ "value": values # grain
+ },
+ "ownState": {},
+ }
+ },
+ None,
+ )
+
+ if filter_type == "filter_timecolumn":
+ return (
+ {
+ native_filter_id or "": {
+ "extraFormData": {
+ "granularity_sqla": values[0] # column_name
Review Comment:
**Suggestion:** The time-column branch also assumes at least one value and
will throw `IndexError` for empty `filterValues`, causing the full report URL
generation to fail. Add an emptiness check before reading the first item.
[possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Time-column filter can abort screenshot generation flow.
- ⚠️ Scheduled dashboard report ends in ERROR state.
```
</details>
```suggestion
"granularity_sqla": values[0] if values else
None # column_name
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Persist a report filter payload using `filterType: "filter_timecolumn"`
with
`filterValues: []`; this passes `_validate_native_filters` list checks in
`superset/commands/report/base.py:180-190`.
2. Execute dashboard report with tabs enabled so
`BaseReportState.get_dashboard_urls()`
enters native filter URL generation
(`superset/commands/report/execute.py:266-271`).
3. `ReportSchedule.get_native_filters_params()` passes empty values to
`_generate_native_filter()` (`superset/reports/models.py:220-225`).
4. `_generate_native_filter()` time-column branch reads `values[0]` at
`superset/reports/models.py:284`, triggering `IndexError` on empty list.
5. Exception interrupts dashboard URL construction
(`superset/commands/report/execute.py:390`) and report execution is logged
as failure
(`superset/commands/report/execute.py:831-858`).
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/reports/models.py
**Line:** 284:284
**Comment:**
*Possible Bug: The time-column branch also assumes at least one value
and will throw `IndexError` for empty `filterValues`, causing the full report
URL generation to fail. Add an emptiness check before reading the first item.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38676&comment_hash=dbdd0f4c7b6c60ed1c6af60142ef24431d46f2715edc11a7af411f5142083bd0&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38676&comment_hash=dbdd0f4c7b6c60ed1c6af60142ef24431d46f2715edc11a7af411f5142083bd0&reaction=dislike'>👎</a>
##########
superset/reports/models.py:
##########
@@ -237,62 +237,85 @@ def _generate_native_filter(
filter_type: str,
column_name: str,
values: list[Optional[str]],
- ) -> dict[str, Any]:
+ ) -> tuple[dict[str, Any], Optional[str]]:
+ """
+ Generate a native filter configuration for the given filter type.
+
+ Returns:
+ A tuple of (filter_config, warning_message). If the filter type is
+ unrecognized, returns an empty dict and a warning message.
+ """
if filter_type == "filter_time":
# For select filters, we need to use the "IN" operator
- return {
- native_filter_id or "": {
- "id": native_filter_id or "",
- "extraFormData": {"time_range": values[0]},
- "filterState": {"value": values[0]},
- "ownState": {},
- }
- }
- elif filter_type == "filter_timegrain":
- return {
- native_filter_id or "": {
- "id": native_filter_id or "",
- "extraFormData": {
- "time_grain_sqla": values[0], # grain
- },
- "filterState": {
- # "label": "30 second", # grain_label
- "value": values # grain
- },
- "ownState": {},
- }
- }
-
- elif filter_type == "filter_timecolumn":
- return {
- native_filter_id or "": {
- "extraFormData": {
- "granularity_sqla": values[0] # column_name
- },
- "filterState": {
- "value": values # column_name
- },
- }
- }
-
- elif filter_type == "filter_select":
- return {
- native_filter_id or "": {
- "id": native_filter_id or "",
- "extraFormData": {
- "filters": [
- {"col": column_name or "", "op": "IN", "val":
values or []}
- ]
- },
- "filterState": {
- "label": column_name or "",
- "validateStatus": False,
- "value": values or [],
- },
- "ownState": {},
- }
- }
- elif filter_type == "filter_range":
+ return (
+ {
+ native_filter_id or "": {
+ "id": native_filter_id or "",
+ "extraFormData": {"time_range": values[0]},
+ "filterState": {"value": values[0]},
Review Comment:
**Suggestion:** Accessing the first element of the filter values without
checking for emptiness can raise `IndexError` for time filters when
`filterValues` is missing or empty. Use a guarded first-value expression so
malformed but recoverable payloads are skipped gracefully instead of crashing
report execution. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Dashboard report run fails on empty time filter.
- ⚠️ Execution log records ERROR instead of success.
```
</details>
```suggestion
"extraFormData": {
"time_range": values[0] if values else None
},
"filterState": {"value": values[0] if values else
None},
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Create/update a report schedule with `extra.dashboard.nativeFilters`
containing a valid
`nativeFilterId`, `filterType: "filter_time"`, and `filterValues: []`;
validation in
`superset/commands/report/base.py:153-190` requires list type but does not
require
non-empty values.
2. Enable tabs execution path (`ALERT_REPORT_TABS`) and run the report;
`BaseReportState.get_dashboard_urls()` calls
`self._report_schedule.get_native_filters_params()` at
`superset/commands/report/execute.py:268-271`.
3. In `ReportSchedule.get_native_filters_params()`
(`superset/reports/models.py:220-225`),
empty values are passed as `native_filter.get("filterValues") or []`, so
`_generate_native_filter(..., values=[])` is called.
4. `_generate_native_filter()` time branch accesses `values[0]` at
`superset/reports/models.py:254-255`, raising `IndexError: list index out of
range`.
5. The exception bubbles during screenshot URL generation
(`superset/commands/report/execute.py:390`) and report execution is marked
error in
`next()` catch block (`superset/commands/report/execute.py:831-858`).
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/reports/models.py
**Line:** 254:255
**Comment:**
*Possible Bug: Accessing the first element of the filter values without
checking for emptiness can raise `IndexError` for time filters when
`filterValues` is missing or empty. Use a guarded first-value expression so
malformed but recoverable payloads are skipped gracefully instead of crashing
report execution.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38676&comment_hash=f6d8feb8e4bbdb000cd4efc7fdea87a05b85567fdff749980decb01ada29c4fa&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38676&comment_hash=f6d8feb8e4bbdb000cd4efc7fdea87a05b85567fdff749980decb01ada29c4fa&reaction=dislike'>👎</a>
##########
superset/reports/models.py:
##########
@@ -303,25 +326,33 @@ def _generate_native_filter(
if max_val is not None:
filters.append({"col": column_name or "", "op": "<=", "val":
max_val})
- return {
- native_filter_id or "": {
- "id": native_filter_id or "",
- "extraFormData": {"filters": filters},
- "filterState": {
- "value": [min_val, max_val],
- "label": f"{min_val} ≤ x ≤ {max_val}"
- if min_val and max_val
- else f"x ≥ {min_val}"
- if min_val
- else f"x ≤ {max_val}"
- if max_val
- else "",
- },
- "ownState": {},
- }
- }
-
- return {}
+ return (
+ {
+ native_filter_id or "": {
+ "id": native_filter_id or "",
+ "extraFormData": {"filters": filters},
+ "filterState": {
+ "value": [min_val, max_val],
+ "label": f"{min_val} ≤ x ≤ {max_val}"
+ if min_val and max_val
+ else f"x ≥ {min_val}"
+ if min_val
+ else f"x ≤ {max_val}"
+ if max_val
Review Comment:
**Suggestion:** Using truthiness to build the range label treats valid
numeric bounds like `0` as absent, producing incorrect labels. Compare against
`None` explicitly so zero is handled as a real boundary value. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Range filter label incorrect for zero boundary values.
- ⚠️ Dashboard filter state text can mislead report viewers.
```
</details>
```suggestion
if min_val is not None and max_val is not None
else f"x ≥ {min_val}"
if min_val is not None
else f"x ≤ {max_val}"
if max_val is not None
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In the report UI flow, range inputs accept numeric zero (`InputNumber`)
and store it in
`filterValues`
(`superset-frontend/src/features/alerts/AlertReportModal.tsx:227-252`,
payload mapping at `898-913`).
2. Run report generation so backend builds native filter params; range
branch computes
`min_val`/`max_val` with `None` checks
(`superset/reports/models.py:320-321`).
3. Label construction uses truthiness checks
(`superset/reports/models.py:336-342`), so
`min_val=0` is treated as absent.
4. Generated `filterState.label` becomes incorrect (e.g., not showing lower
bound `0`)
while filter operators are still built correctly from `is not None` checks
(`superset/reports/models.py:324-327`).
5. Incorrect label is serialized into the `native_filters` URL payload
returned by
`get_native_filters_params()` (`superset/reports/models.py:230-232`) and
used in dashboard
tab URLs (`superset/commands/report/execute.py:55`).
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/reports/models.py
**Line:** 337:341
**Comment:**
*Logic Error: Using truthiness to build the range label treats valid
numeric bounds like `0` as absent, producing incorrect labels. Compare against
`None` explicitly so zero is handled as a real boundary 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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38676&comment_hash=c02ea838c56e023bd7ca1bba2076bedac6fd75b119449c4e477eb7418eedc470&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38676&comment_hash=c02ea838c56e023bd7ca1bba2076bedac6fd75b119449c4e477eb7418eedc470&reaction=dislike'>👎</a>
##########
superset/reports/models.py:
##########
@@ -237,62 +237,85 @@ def _generate_native_filter(
filter_type: str,
column_name: str,
values: list[Optional[str]],
- ) -> dict[str, Any]:
+ ) -> tuple[dict[str, Any], Optional[str]]:
+ """
+ Generate a native filter configuration for the given filter type.
+
+ Returns:
+ A tuple of (filter_config, warning_message). If the filter type is
+ unrecognized, returns an empty dict and a warning message.
+ """
if filter_type == "filter_time":
# For select filters, we need to use the "IN" operator
- return {
- native_filter_id or "": {
- "id": native_filter_id or "",
- "extraFormData": {"time_range": values[0]},
- "filterState": {"value": values[0]},
- "ownState": {},
- }
- }
- elif filter_type == "filter_timegrain":
- return {
- native_filter_id or "": {
- "id": native_filter_id or "",
- "extraFormData": {
- "time_grain_sqla": values[0], # grain
- },
- "filterState": {
- # "label": "30 second", # grain_label
- "value": values # grain
- },
- "ownState": {},
- }
- }
-
- elif filter_type == "filter_timecolumn":
- return {
- native_filter_id or "": {
- "extraFormData": {
- "granularity_sqla": values[0] # column_name
- },
- "filterState": {
- "value": values # column_name
- },
- }
- }
-
- elif filter_type == "filter_select":
- return {
- native_filter_id or "": {
- "id": native_filter_id or "",
- "extraFormData": {
- "filters": [
- {"col": column_name or "", "op": "IN", "val":
values or []}
- ]
- },
- "filterState": {
- "label": column_name or "",
- "validateStatus": False,
- "value": values or [],
- },
- "ownState": {},
- }
- }
- elif filter_type == "filter_range":
+ return (
+ {
+ native_filter_id or "": {
+ "id": native_filter_id or "",
+ "extraFormData": {"time_range": values[0]},
+ "filterState": {"value": values[0]},
+ "ownState": {},
+ }
+ },
+ None,
+ )
+ if filter_type == "filter_timegrain":
+ return (
+ {
+ native_filter_id or "": {
+ "id": native_filter_id or "",
+ "extraFormData": {
+ "time_grain_sqla": values[0], # grain
Review Comment:
**Suggestion:** The time-grain branch indexes `values[0]` unconditionally,
which raises `IndexError` if a report stores this filter with no selected
value. Guard the access and emit `None` when no value is present to keep report
generation resilient. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Time-grain native filter can crash report execution.
- ⚠️ Report recipients miss scheduled dashboard delivery.
```
</details>
```suggestion
"time_grain_sqla": values[0] if values else
None, # grain
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Save a report native filter with `filterType: "filter_timegrain"` and
`filterValues:
[]`; API-side native filter validation
(`superset/commands/report/base.py:153-190`)
accepts empty lists.
2. During report execution with tabs enabled, `get_dashboard_urls()` invokes
`get_native_filters_params()`
(`superset/commands/report/execute.py:268-271`).
3. `get_native_filters_params()` forwards `[]` into
`_generate_native_filter()` via
`native_filter.get("filterValues") or []`
(`superset/reports/models.py:224-225`).
4. In `_generate_native_filter()` time-grain branch, `values[0]` at
`superset/reports/models.py:267` raises `IndexError`.
5. The error propagates before screenshot capture and the run transitions to
ERROR
(`superset/commands/report/execute.py:390`, `831-858`).
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/reports/models.py
**Line:** 267:267
**Comment:**
*Possible Bug: The time-grain branch indexes `values[0]`
unconditionally, which raises `IndexError` if a report stores this filter with
no selected value. Guard the access and emit `None` when no value is present to
keep report generation resilient.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38676&comment_hash=6740efda567a983e48198b38faedbdd4823850b992f622f27fb5e6c97b894c0d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38676&comment_hash=6740efda567a983e48198b38faedbdd4823850b992f622f27fb5e6c97b894c0d&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]
