aminghadersohi commented on PR #43943: URL: https://github.com/apache/superset/pull/43943#issuecomment-5575896351
Addressed [this review](https://github.com/apache/superset/pull/43943#pullrequestreview-5135167297) in `f17d7f9e81e5638946a7c9dbb74680d4011e54d5`. - **Mixed NULL/NaN/non-finite dials:** query normalization preserves finite groups and skips invalid values without mutating cached results. Nonempty all-invalid output still errors; malformed envelopes/aliases and query failures remain errors. Genuinely empty results retain the existing no-data behavior. Applied to both get-data query paths, all preview formats, and generate/update compile checks. Gauge compilation samples its configured dial limit (up to 10), so the first two empty groups cannot hide a later valid dial. - **Faceted pointer:** endpoint origins use the same child dimensions as the facet unit. The prior regression checked length but missed origin; the new actual Vega-Lite 5/Vega 5 scenegraph test compares each needle with its own arc center, and verifies visibility, length, angles, and multiple cells across 12 specs. - **Native `"No filter"`:** treated as a neutral dashboard binding, not a conflicting restriction. Real conflicting ranges still reject. - **Config-less rebind guard:** restating the same dataset is accepted; a different Gauge dataset still requires a complete config. Public FastMCP tests cover both. - **Duplicate `NO_TIME_RANGE`:** generated neutral bindings self-heal on updates, including omitted temporal controls; user-authored ranges survive. - **Dead helper:** removed `_preserve_previous_adhoc_filters` and its unreachable helper-only tests. Coverage targets the shared merge and actual cached-update path instead. - **Assertion gaps:** added returned-data/row-count assertions, saved-versus-preview role assertions, and real generate/update compile regressions rather than mocking successful compilation. The offered low/nit batch was not posted separately; all named areas above were inspected and addressed directly. No additional review is requested. The four existing resolved threads remain resolved; this review has no new inline thread to resolve. Evidence: **20 fail-before cases** against starting head `2e014b6`; final **1,772 affected Python tests**, **21 frontend Gauge tests**, actual rendered-spec checks, branch-wide/staged pre-commit (including mypy), compileall, and optimized-mode validation passed. **All 13 required GitHub checks passed on this exact head**, including [unit tests](https://github.com/apache/superset/actions/runs/34162287144/job/101871425929), [PostgreSQL integration](https://github.com/apache/superset/actions/runs/34162287384/job/101870458443), [Playwright](https://github.com/apache/superset/actions/runs/34162287178/job/101869846479), and [pre-commit](https://github.com/apache/superset/actions/runs/34162287169/job/101866399639). -- 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]
