codeant-ai-for-open-source[bot] commented on code in PR #38513:
URL: https://github.com/apache/superset/pull/38513#discussion_r2928233455
##########
superset/views/core.py:
##########
@@ -206,6 +211,24 @@ def generate_json(
payload = viz_obj.get_payload()
return self.send_data_payload_response(viz_obj, payload)
+ @staticmethod
+ def _generate_xlsx(viz_obj: BaseViz) -> FlaskResponse:
+ import pandas as pd
+
+ from superset.utils.excel import apply_column_types, df_to_excel
+
+ payload = viz_obj.get_df_payload()
+ df = payload.get("df")
+ if df is None:
+ df = pd.DataFrame()
+ coltypes: list[GenericDataType] = []
+ else:
+ coltypes = payload.get("coltypes") or []
+ if coltypes:
+ df = apply_column_types(df, coltypes)
+ xlsx_data = df_to_excel(df, index=False)
Review Comment:
**Suggestion:** XLSX export always forces `index=False`, which drops
meaningful non-default indexes from the exported data. This causes silent data
loss for charts where the index carries dimension values, so index inclusion
should mirror existing CSV/XLSX behavior by including non-RangeIndex indexes.
[logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Legacy XLSX export inconsistent with legacy CSV behavior.
- ⚠️ Non-RangeIndex values can be omitted from spreadsheets.
- ⚠️ Export parity diverges from `/api/v1/chart/data` path.
```
</details>
```suggestion
include_index = not isinstance(df.index, pd.RangeIndex)
xlsx_data = df_to_excel(df, index=include_index)
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Trigger legacy XLSX export through `exportChart` path
(`superset-frontend/src/explore/exploreUtils/index.ts:365-383`) so backend
reaches
`_generate_xlsx` in `superset/views/core.py:215-230`.
2. `_generate_xlsx` always calls `df_to_excel(df, index=False)` at
`core.py:229`, so
DataFrame index is always dropped.
3. Codebase shows intended behavior is conditional index retention: legacy
CSV uses
`include_index = not isinstance(df.index, pd.RangeIndex)` in
`superset/viz.py:685-689`.
4. New API chart exports follow the same rule for XLSX/CSV in
`superset/common/query_context_processor.py:21-37`; therefore this legacy
XLSX branch is
inconsistent and drops non-RangeIndex values that other export paths
preserve.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/views/core.py
**Line:** 229:229
**Comment:**
*Logic Error: XLSX export always forces `index=False`, which drops
meaningful non-default indexes from the exported data. This causes silent data
loss for charts where the index carries dimension values, so index inclusion
should mirror existing CSV/XLSX behavior by including non-RangeIndex indexes.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38513&comment_hash=c366838174bfce9d9591a2ec53e94b4409fb8a4193b68192b6530fd7aae852f9&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38513&comment_hash=c366838174bfce9d9591a2ec53e94b4409fb8a4193b68192b6530fd7aae852f9&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]
