codeant-ai-for-open-source[bot] commented on code in PR #40588:
URL: https://github.com/apache/superset/pull/40588#discussion_r3349746732
##########
superset/commands/dashboard/export.py:
##########
@@ -105,6 +106,120 @@ def append_charts(position: dict[str, Any], charts:
set[Slice]) -> dict[str, Any
return position
+# Bound the derived id to a signed 31-bit positive integer so it stays well
+# within the range a database auto-increment primary key would occupy and
+# never collides with the sign bit. The concrete value is irrelevant — only
+# its stability across environments matters.
+_STABLE_CHART_ID_MODULO = 2_147_483_647
+
+
+def stable_chart_id(chart_uuid: str) -> int:
+ """Derive a deterministic, environment-independent integer from a chart
UUID.
+
+ The dashboard export format historically embedded ``meta.chartId`` — the
+ source environment's auto-increment primary key — inside every ``CHART-*``
+ position node (issue #32972). That integer differs between environments, so
+ re-exporting an imported dashboard produced a different bundle for the same
+ logical content. Deriving the id from the (stable) UUID instead makes the
+ export reproducible while still giving the importer an integer it can use
to
+ rewire the legacy, integer-keyed metadata references back to local IDs.
+ """
+ return (uuid_module.UUID(chart_uuid).int % _STABLE_CHART_ID_MODULO) + 1
Review Comment:
**Suggestion:** `stable_chart_id` calls `UUID(...)` without handling
malformed UUID values from persisted `position_json`. A single invalid
`meta.uuid` will raise `ValueError` and abort dashboard export. Make this path
tolerant (skip stabilization for invalid UUIDs or catch/log and continue) so
one bad chart node does not fail the whole export. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Single malformed chart UUID breaks entire dashboard export.
- ⚠️ Existing dashboards with bad UUIDs become non-exportable.
- ⚠️ Export path less robust to corrupted position_json.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Persist a dashboard whose `position_json` contains a CHART node with
`meta.uuid` set to
an invalid UUID string (e.g., by directly updating the dashboard record in
the database or
via a migration); these CHART nodes are later loaded into the export payload
by
`ExportDashboardsCommand._file_content` at
`superset/commands/dashboard/export.py:234-248`.
2. Trigger a dashboard export through the dashboards API `GET
/api/v1/dashboard/export/`
(`superset/dashboards/api.py:1245-1253`), which calls
`ExportDashboardsCommand._file_content` and builds the `payload` dict
including
`position`.
3. `_file_content` invokes `_stabilize_chart_ids(payload)` at `export.py`
hunk lines
300-303; `_stabilize_chart_ids` iterates CHART nodes and, for each with a
`meta["uuid"]`,
calls `stable_chart_id(str(chart_uuid))` at `export.py:158-161`.
4. `stable_chart_id` calls `uuid_module.UUID(chart_uuid)` at
`export.py:116-127` without
any exception handling; when `chart_uuid` is not a syntactically valid UUID,
`uuid.UUID`
raises `ValueError`, which bubbles up and aborts `_stabilize_chart_ids` and
`_file_content`, causing the dashboard export request to fail with an
unhandled exception
instead of skipping or logging the malformed node.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3d994f4625cf4cf7a66d8e2dba8bab88&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=3d994f4625cf4cf7a66d8e2dba8bab88&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/dashboard/export.py
**Line:** 116:127
**Comment:**
*Possible Bug: `stable_chart_id` calls `UUID(...)` without handling
malformed UUID values from persisted `position_json`. A single invalid
`meta.uuid` will raise `ValueError` and abort dashboard export. Make this path
tolerant (skip stabilization for invalid UUIDs or catch/log and continue) so
one bad chart node does not fail the whole export.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40588&comment_hash=f1711d15255288c9e7d80a6ccfc8bf1a8938012af10441fca2c95223b8ed60ff&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40588&comment_hash=f1711d15255288c9e7d80a6ccfc8bf1a8938012af10441fca2c95223b8ed60ff&reaction=dislike'>👎</a>
##########
superset/commands/dashboard/export.py:
##########
@@ -182,6 +297,11 @@ def _file_content(model: Dashboard) -> str:
if orphan_charts:
payload["position"] = append_charts(payload["position"],
orphan_charts)
+ # Strip env-local integer chart IDs in favor of UUID-derived ones so
the
+ # export is reproducible across environments (issue #32972). Must run
+ # after orphan charts are appended so their nodes are stabilized too.
+ _stabilize_chart_ids(payload)
Review Comment:
**Suggestion:** The stabilization logic updates `position` IDs but does not
remap cross-filter metadata (`metadata.chart_configuration` and
`metadata.global_chart_configuration`), which also stores chart IDs.
Import-side `update_cross_filter_scoping` expects these IDs to align with the
IDs found in `position`; with mismatched IDs, chart configuration entries are
skipped/dropped. Add equivalent remapping for those cross-filter structures in
`_stabilize_chart_ids`. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Cross-filter configurations disappear after dashboard export/import.
- ❌ Global cross-filter scopes no longer match intended charts.
- ⚠️ Users see inconsistent interactive cross-filter behavior.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a dashboard with cross-filtering enabled so that
`metadata.global_chart_configuration` and `metadata.chart_configuration`
contain chart IDs
(these structures are consumed on import by `update_cross_filter_scoping` at
`superset/commands/dashboard/importers/v1/utils.py:190-164` to restore
cross-filter
scoping).
2. Export the dashboard via the dashboards export API
(`superset/dashboards/api.py:1245-1253`), which calls
`ExportDashboardsCommand._file_content` at
`superset/commands/dashboard/export.py:234-260`; this function loads
`position` and
`metadata` and then calls `_stabilize_chart_ids(payload)` at `export.py`
hunk lines
300-303.
3. `_stabilize_chart_ids` rewrites each CHART node's `meta.chartId` and
remaps
`metadata["timed_refresh_immune_slices"]`, `filter_scopes`,
`expanded_slices`, and
`default_filters` using its `id_map` at `export.py:181-220`, but it never
remaps
`metadata["global_chart_configuration"]` or
`metadata["chart_configuration"]`, leaving
those structures keyed by the old env-local chart IDs while `position` and
other metadata
now use the UUID-derived IDs.
4. Import the bundle into another environment via `ImportDashboardsCommand`,
which calls
`update_id_refs` and then `update_cross_filter_scoping(fixed, id_map)` at
`utils.py:186-107` and `190-164`; `update_cross_filter_scoping` expects
`global_chart_configuration.scope.excluded`,
`global_chart_configuration.chartsInScope`,
and `metadata["chart_configuration"]` keys to match the IDs in `position`,
but because
they still carry old IDs, the lookups into `id_map` fail, excluded/scoped
lists become
empty, and chart configuration entries are skipped (see
`id_map.get(old_id_int)` at
`utils.py:138-140`), effectively dropping cross-filter configuration after
import.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=92fb35987edd43879637482421f25ac6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=92fb35987edd43879637482421f25ac6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/dashboard/export.py
**Line:** 300:303
**Comment:**
*Incomplete Implementation: The stabilization logic updates `position`
IDs but does not remap cross-filter metadata (`metadata.chart_configuration`
and `metadata.global_chart_configuration`), which also stores chart IDs.
Import-side `update_cross_filter_scoping` expects these IDs to align with the
IDs found in `position`; with mismatched IDs, chart configuration entries are
skipped/dropped. Add equivalent remapping for those cross-filter structures in
`_stabilize_chart_ids`.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40588&comment_hash=966a863dc0110a8d70f2273cc2c368295026e9e7198b4cb2aab38e7ffeaf3dd3&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40588&comment_hash=966a863dc0110a8d70f2273cc2c368295026e9e7198b4cb2aab38e7ffeaf3dd3&reaction=dislike'>👎</a>
##########
superset/commands/dashboard/export.py:
##########
@@ -105,6 +106,120 @@ def append_charts(position: dict[str, Any], charts:
set[Slice]) -> dict[str, Any
return position
+# Bound the derived id to a signed 31-bit positive integer so it stays well
+# within the range a database auto-increment primary key would occupy and
+# never collides with the sign bit. The concrete value is irrelevant — only
+# its stability across environments matters.
+_STABLE_CHART_ID_MODULO = 2_147_483_647
+
+
+def stable_chart_id(chart_uuid: str) -> int:
+ """Derive a deterministic, environment-independent integer from a chart
UUID.
+
+ The dashboard export format historically embedded ``meta.chartId`` — the
+ source environment's auto-increment primary key — inside every ``CHART-*``
+ position node (issue #32972). That integer differs between environments, so
+ re-exporting an imported dashboard produced a different bundle for the same
+ logical content. Deriving the id from the (stable) UUID instead makes the
+ export reproducible while still giving the importer an integer it can use
to
+ rewire the legacy, integer-keyed metadata references back to local IDs.
+ """
+ return (uuid_module.UUID(chart_uuid).int % _STABLE_CHART_ID_MODULO) + 1
+
+
+def _stabilize_chart_ids(payload: dict[str, Any]) -> None:
+ """Replace env-local integer chart IDs in ``payload`` with UUID-derived
ones.
+
+ Rewrites ``meta.chartId`` in every ``CHART`` position node to a value
derived
+ from ``meta.uuid`` and remaps the legacy, integer-keyed metadata references
+ (filter scopes, default filters, expanded slices, etc.) accordingly so the
+ bundle stays internally consistent. Mappings are applied defensively — a
+ reference whose source id is unknown is dropped rather than raising — so a
+ partially corrupt position never aborts the export. See ``stable_chart_id``
+ and issue #32972 for the motivation.
+ """
+ position = payload.get("position")
+ if not isinstance(position, dict):
+ return
+
+ # Map each chart's env-local integer id -> stable UUID-derived id.
+ id_map: dict[int, int] = {}
+ for node in position.values():
+ if (
+ isinstance(node, dict)
+ and node.get("type") == "CHART"
+ and isinstance(node.get("meta"), dict)
+ ):
+ meta = node["meta"]
+ chart_uuid = meta.get("uuid")
+ old_id = meta.get("chartId")
+ if chart_uuid is None:
+ continue
+ new_id = stable_chart_id(str(chart_uuid))
+ if isinstance(old_id, int):
+ id_map[old_id] = new_id
+ meta["chartId"] = new_id
Review Comment:
**Suggestion:** After rewriting `meta.chartId` to a UUID-derived value, the
code does not remap chart IDs inside `metadata.native_filter_configuration`
(`scope.excluded` and `chartsInScope`). On import, `update_id_refs` builds its
mapping from the new `position` IDs, so these still-old IDs no longer match and
get dropped, changing filter scoping behavior. Remap those native-filter ID
lists in `_stabilize_chart_ids` using the same `id_map`. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Dashboard native filter scopes change after export/import.
- ❌ Some charts unexpectedly lose or gain filter application.
- ⚠️ Cross-environment dashboard migration becomes non-reproducible.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Export an existing dashboard with native filters configured, where
`metadata.native_filter_configuration[*].scope.excluded` or `chartsInScope`
contain chart
IDs (dashboards with native filters are persisted via
`DashboardDAO.update_native_filters_config` at
`superset/daos/dashboard.py:435-441`, which
writes those IDs into `json_metadata`).
2. Call the dashboards export API `GET /api/v1/dashboard/export/`
(implemented in
`superset/dashboards/api.py:69` and `1245-1253`), which invokes
`ExportDashboardsCommand._file_content` at
`superset/commands/dashboard/export.py:234-260`.
3. Inside `_file_content`, `_stabilize_chart_ids(payload)` is called at
`export.py:41-44`
(hunk lines 300-303); `_stabilize_chart_ids` rewrites each CHART node's
`meta.chartId`
using `stable_chart_id` at `export.py:116-127` and remaps
`metadata["filter_scopes"]`,
`timed_refresh_immune_slices`, `expanded_slices`, and `default_filters` at
`export.py:181-220`, but it never touches
`metadata["native_filter_configuration"]` — IDs
in `scope.excluded`/`chartsInScope` remain the original env-local integers.
4. Import the exported bundle into another environment via
`ImportDashboardsCommand`
(wired in `superset/dashboards/api.py:72`), which calls `update_id_refs` at
`superset/commands/dashboard/importers/v1/utils.py:80-108`; `update_id_refs`
builds an
`id_map` from the stabilized `position` IDs (via `build_uuid_to_id_map` at
`utils.py:21-30`) and then remaps native filter `scope.excluded` and
`chartsInScope` using
`id_map` at `utils.py:68-85`, but because those lists still contain the old
env-local IDs
(no longer present in `id_map`), all unmatched IDs are dropped, altering
which charts are
in-scope or excluded for each native filter after import.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=38be26b18ff64fc7a0476f06a2951276&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=38be26b18ff64fc7a0476f06a2951276&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/dashboard/export.py
**Line:** 158:161
**Comment:**
*Logic Error: After rewriting `meta.chartId` to a UUID-derived value,
the code does not remap chart IDs inside `metadata.native_filter_configuration`
(`scope.excluded` and `chartsInScope`). On import, `update_id_refs` builds its
mapping from the new `position` IDs, so these still-old IDs no longer match and
get dropped, changing filter scoping behavior. Remap those native-filter ID
lists in `_stabilize_chart_ids` using the same `id_map`.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40588&comment_hash=b234397f3b5f3ca6f3285317c0c149ef45916749fb1bbcc5f8d20555239f5387&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40588&comment_hash=b234397f3b5f3ca6f3285317c0c149ef45916749fb1bbcc5f8d20555239f5387&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]
