aminghadersohi commented on code in PR #45504:
URL: https://github.com/apache/superset/pull/45504#discussion_r4332094251
##########
superset/mcp_service/dashboard/tool/get_dashboard_data.py:
##########
@@ -132,6 +132,17 @@ def _summarize_chart_data(
chart_id=result.chart_id,
chart_name=result.chart_name,
chart_type=result.chart_type,
+ fetch_row_limit=request.fetch_row_limit,
+ sample_rows_limit=request.sample_rows,
+ completeness_note=(
+ "Histogram bins may be computed from input rows capped by
fetch_row_limit. "
+ "A single output row and truncated=false do not establish
full-population "
+ "coverage or a defect in the saved chart. Verify the saved
configuration "
+ "and source aggregates before making a distribution claim."
+ if result.chart_type in {"histogram", "histogram_v2"}
+ else "sample_data contains example rows only; use row_count,
total_rows "
Review Comment:
Fixed the remaining timeseries/groupby gap in
b17c76ffcd5961222f57ab39bc4bd21c46b4a369 using the operations on every executed
query, rather than extending the chart-type list. Saved and rebuilt JSON
results carry private pipeline metadata through preflight. For potentially
row-reducing/replacing pipelines, affected query layers report
`total_rows=null`, conservative `truncated=true`, and the input
`fetch_row_limit`. The note explicitly distinguishes unverified input coverage
from observed truncation. Row-preserving pipelines retain ordinary completeness
checks; legacy distribution results without metadata retain their warning.
Regression: `test_timeseries_groupby_pivot_does_not_claim_complete_input`
executes the generated timeseries/groupby pipeline on 500 input rows, producing
50 output rows. Both saved and rebuilt cases fail at
377a9082300e23a02daf89e95da20b38daa8f5be (`total_rows=50`, rather than unknown)
and pass after. Command: `PYTHONPATH=.:superset-core/src pytest -q -m 'not
manual and not llm_compliance'
tests/unit_tests/mcp_service/dashboard/tool/test_get_dashboard_data.py -k
timeseries_groupby`. The final dashboard/DAO run reports `90 passed`;
branch-file pre-commit, including mypy, passes. The full MCP run under
repository-pinned FastMCP 3.4.7 reports `11031 passed, 4 skipped`: `python -m
pytest -q -n 8 -m 'not manual and not llm_compliance'
tests/unit_tests/mcp_service`. Tests additionally cover each registry
classification, custom operations, mixed-query layers, empty leading layers,
and metadata staying off the wire.
Siblings checked:
- superset/mcp_service/chart/tool/get_chart_data.py — fixed in
b17c76ffcd5961222f57ab39bc4bd21c46b4a369: saved and rebuilt execution paths
carry all query pipelines, not only the first.
- superset/mcp_service/dashboard/tool/get_dashboard_data.py — fixed in
b17c76ffcd5961222f57ab39bc4bd21c46b4a369: single-query, selected primary, and
every mixed-query layer use the operation classification.
- superset/utils/pandas_postprocessing/aggregate.py — fixed in
b17c76ffcd5961222f57ab39bc4bd21c46b4a369: dashboard coverage is unverified
because aggregate groups rows; the operation itself is unchanged.
- superset/utils/pandas_postprocessing/boxplot.py — fixed in
b17c76ffcd5961222f57ab39bc4bd21c46b4a369: boxplot reduces rows to group
statistics; conservative dashboard coverage applies.
- superset/utils/pandas_postprocessing/histogram.py — fixed in
b17c76ffcd5961222f57ab39bc4bd21c46b4a369: histogram reduces rows to bin counts;
conservative dashboard coverage applies.
- superset/utils/pandas_postprocessing/pivot.py — fixed in
b17c76ffcd5961222f57ab39bc4bd21c46b4a369: pivot aggregates duplicate indexes
and moves dimensions into columns; conservative dashboard coverage applies.
- superset/utils/pandas_postprocessing/resample.py — fixed in
b17c76ffcd5961222f57ab39bc4bd21c46b4a369: resample can reduce rows into coarser
time bins; conservative dashboard coverage applies.
- superset/utils/pandas_postprocessing/rolling.py — fixed in
b17c76ffcd5961222f57ab39bc4bd21c46b4a369: rolling can slice leading rows with
min_periods; conservative dashboard coverage applies.
- superset/utils/pandas_postprocessing/prophet.py — fixed in
b17c76ffcd5961222f57ab39bc4bd21c46b4a369: prophet reconstructs the time index,
so output counts cannot establish input coverage.
- superset/utils/pandas_postprocessing/compare.py — not affected: compare
changes comparison columns, preserving input rows.
- superset/utils/pandas_postprocessing/contribution.py — not affected:
contribution computes cell shares, preserving input rows.
- superset/utils/pandas_postprocessing/cum.py — not affected: cum computes
cumulative values, preserving input rows.
- superset/utils/pandas_postprocessing/diff.py — not affected: diff retains
rows, including missing leading values.
- superset/utils/pandas_postprocessing/flatten.py — not affected: flatten
changes labels and resets the index without removing rows.
- superset/utils/pandas_postprocessing/geography.py — not affected:
geodetic_parse appends coordinate columns, preserving rows.
- superset/utils/pandas_postprocessing/geography.py — not affected:
geohash_decode appends coordinate columns, preserving rows.
- superset/utils/pandas_postprocessing/geography.py — not affected:
geohash_encode appends geohash columns, preserving rows.
- superset/utils/pandas_postprocessing/rank.py — not affected: rank assigns
values without removing rows.
- superset/utils/pandas_postprocessing/rename.py — not affected: rename
changes column labels without removing rows.
- superset/utils/pandas_postprocessing/select.py — not affected: select
selects/excludes/renames columns only, preserving rows.
- superset/utils/pandas_postprocessing/sort.py — not affected: sort reorders
rows without removing them.
- superset/utils/pandas_postprocessing/__init__.py — fixed in
b17c76ffcd5961222f57ab39bc4bd21c46b4a369: all 19 registered built-ins are
pinned by the classification test; unknown/custom/future operations default to
unverified coverage. escape_separator/unescape_separator are internal string
helpers, not registered DataFrame operations.
--
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]
