codeant-ai-for-open-source[bot] commented on code in PR #42490:
URL: https://github.com/apache/superset/pull/42490#discussion_r3675317003
##########
superset/mcp_service/chart/tool/update_chart_preview.py:
##########
@@ -102,6 +102,59 @@ def _get_previous_form_data(form_data_key: str) ->
dict[str, Any] | None:
return None
+def _preserve_previous_adhoc_filters(
+ new_form_data: dict[str, Any], previous_form_data: dict[str, Any]
+) -> None:
+ """Preserve cached filters without dropping mapper-generated bindings."""
+ previous_filters = previous_form_data.get("adhoc_filters")
+ if not isinstance(previous_filters, list) or not previous_filters:
+ return
+
+ generated_filters = new_form_data.get("adhoc_filters", [])
+ generated_temporal_subjects = {
+ filter_.get("subject")
+ for filter_ in generated_filters
+ if isinstance(filter_, dict) and filter_.get("operator") ==
"TEMPORAL_RANGE"
+ }
+ previous_temporal_subjects = {
+ filter_.get("subject")
+ for filter_ in previous_filters
+ if isinstance(filter_, dict) and filter_.get("operator") ==
"TEMPORAL_RANGE"
+ }
+ replace_temporal_binding = (
+ bool(generated_temporal_subjects)
+ and generated_temporal_subjects != previous_temporal_subjects
+ )
Review Comment:
**Suggestion:** When the new mapping produces no `TEMPORAL_RANGE` filter,
`replace_temporal_binding` remains false, so cached temporal filters are
retained unchanged. This leaves a stale dashboard time binding when the new
chart has no valid temporal subject, such as when the dataset no longer has a
`main_dttm_col`; compile-time validation only checks that the cached column
exists, not that it is still the intended temporal binding. Remove stale cached
temporal filters whenever the new configuration intentionally has no generated
temporal binding. [stale reference]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Updated charts retain stale dashboard time-filter subjects.
- ⚠️ Dashboard time ranges may filter an unintended temporal column.
- ⚠️ Preview and saved chart behavior can diverge from new configuration.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=437de76350ec4c02944d9ce8a70e108b&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=437de76350ec4c02944d9ce8a70e108b&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/tool/update_chart_preview.py
**Line:** 124:127
**Comment:**
*Stale Reference: When the new mapping produces no `TEMPORAL_RANGE`
filter, `replace_temporal_binding` remains false, so cached temporal filters
are retained unchanged. This leaves a stale dashboard time binding when the new
chart has no valid temporal subject, such as when the dataset no longer has a
`main_dttm_col`; compile-time validation only checks that the cached column
exists, not that it is still the intended temporal binding. Remove stale cached
temporal filters whenever the new configuration intentionally has no generated
temporal binding.
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%2F42490&comment_hash=268805849d308aa6cc2a08e7122590a481b344dcb77a3a6cc8d7d83a4138ad79&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42490&comment_hash=268805849d308aa6cc2a08e7122590a481b344dcb77a3a6cc8d7d83a4138ad79&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]
