mikebridge commented on PR #45105:
URL: https://github.com/apache/superset/pull/45105#issuecomment-6079039628

   Thanks for the detailed review. Pushed the follow-up in 
80b95a528f30af3c3b16541f7290220859f18e39.
   
   **Finding 1 (MCP provenance):** Fixed at the shared point: 
`split_adhoc_filters_into_base_filters` preserves each adhoc filter's `isExtra` 
when rebuilding `{col, op, val}`. This covers the saved-query merge and 
unsaved/fallback builders used by chart data, SQL and previews, without tagging 
chart-owned filters. The regression covers saved, table and timeseries paths, 
pairing dashboard metric filters with chart-owned metric filters that remain 
refused.
   
   **Finding 2 (badge on all dashboards):** Agreed this is a visible change 
beyond semantic views. Before/after screenshots of SQL dashboard 
cross-filtering across datasets and the UX decision on badge behavior will 
follow separately; this follow-up does not change that behavior.
   
   **Finding 3 (unset filters):** Good catch. Applied `column && hasValue && 
rejectedColumns?.has(column)` and added the two-filters/same-column regression: 
the valued filter is Incompatible and the unset one stays Unset.
   
   **Nits:** Added the `UPDATING.md` cache warm-up note. Independently checked 
that absent/false provenance preserves chart-owned SQL filter payloads and 
cache keys, while dashboard provenance changes the key. The `TEMPORAL_RANGE` 
reporting edge case remains a separate follow-up. Repeated metadata lookups are 
unchanged; we could hoist them separately if they prove costly. Thanks also for 
checking the bot's double-count claim.
   
   Verification: the previously failing real-builder probe now passes; focused 
Python checks had 759 passes and the known local timezone failure; 67 focused 
frontend tests and changed-file hooks passed. No browser screenshots or 
live-provider validation are claimed here.
   


-- 
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]

Reply via email to