aminghadersohi commented on PR #43572:
URL: https://github.com/apache/superset/pull/43572#issuecomment-5658716727

   Rechecked **edb7e9ff01c4**: SQL-expression save-mode reporting and 
saved/transient Bubble renderer dispatch still work; **43 targeted tests pass** 
and current required CI is green. The new sort precedence introduces one 
public-path regression:
   
   For native Bubble form-data with `orderby=[size]`, `order_desc=False`, the 
[preview 
strategies](https://github.com/apache/superset/blob/edb7e9ff01c47842aea0d6af6238facee6587f50/superset/mcp_service/chart/tool/get_chart_preview.py#L449-L457)
 and [saved-data 
fallback](https://github.com/apache/superset/blob/edb7e9ff01c47842aea0d6af6238facee6587f50/superset/mcp_service/chart/tool/get_chart_data.py#L609-L617)
 pass **hardcoded `order_desc=True`**, not a user-requested override. The 
changed helper now lets that override the saved chart.
   
   Captured query dictionaries with DB execution mocked:
   - Compile/cached-data path: `orderby=[(size, True)]` — ascending, matching 
Explore.
   - Saved/transient preview and saved-data fallback: `orderby=[(size, False)]` 
— descending.
   - Running the previous helper with the same fallback arguments gives 
ascending.
   
   With `row_limit=1` on the saved-data fallback, this selects the largest 
bubble instead of the requested smallest; it changes the selected data, not 
just display ordering.
   
   Keeping explicit-argument precedence is reasonable, but please update these 
callers to forward the chart's saved direction (or omit the override for 
Bubble). Add a regression through the preview strategy and saved-data fallback 
with `order_desc=False`; the new helper-only test currently verifies the 
override without checking whether callers meant to override the chart.
   
   No contributor code changed; these were isolated source/unit-harness checks, 
not a live database/browser run.
   


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