codeant-ai-for-open-source[bot] commented on code in PR #40588:
URL: https://github.com/apache/superset/pull/40588#discussion_r3424832868
##########
superset/commands/dashboard/export.py:
##########
@@ -105,6 +106,166 @@ 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, native filter scopes, and
+ cross-filter/chart configuration) accordingly so the bundle stays
internally
+ consistent and the import-side id remap resolves against the same IDs.
+ Mappings are applied defensively — a reference whose source id is unknown
is
+ dropped rather than raising, and a node with a malformed ``meta.uuid`` is
+ skipped — 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
+ try:
+ new_id = stable_chart_id(str(chart_uuid))
+ except ValueError:
+ # A malformed ``meta.uuid`` (corrupt position_json) must not
+ # abort the whole export — skip stabilizing this single node
+ # and leave its existing chartId untouched.
+ logger.warning(
+ "Skipping chart id stabilization for invalid uuid %r",
+ chart_uuid,
+ )
+ continue
+ if isinstance(old_id, int):
+ id_map[old_id] = new_id
+ meta["chartId"] = new_id
+
+ if not id_map:
+ return
+
+ metadata = payload.get("metadata")
+ if not isinstance(metadata, dict):
+ return
+
+ def remap_id(old_id: Any) -> Optional[int]:
+ try:
+ return id_map.get(int(old_id))
+ except (TypeError, ValueError):
+ return None
+
+ def remap_ids(old_ids: Any) -> list[int]:
+ return [
+ new_id for old_id in old_ids if (new_id := remap_id(old_id)) is
not None
+ ]
Review Comment:
**Suggestion:** Add a docstring describing that this helper remaps a
collection of legacy IDs and filters out unresolved entries. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The function is newly added and lacks a docstring, which violates the rule
that new Python functions and classes should be documented inline.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=df02cccea77147c5995e62ae101a1a8e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=df02cccea77147c5995e62ae101a1a8e&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:** 188:191
**Comment:**
*Custom Rule: Add a docstring describing that this helper remaps a
collection of legacy IDs and filters out unresolved entries.
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=c2b75f42d781ed847d0f30570f609955fd37fbdac9134076121112dad0e13d19&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40588&comment_hash=c2b75f42d781ed847d0f30570f609955fd37fbdac9134076121112dad0e13d19&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/dashboard/export_test.py:
##########
@@ -256,6 +256,446 @@ def
test_file_content_omits_roles_field_when_dashboard_has_no_roles():
assert "roles" not in result
+def test_position_json_chart_id_leaks_env_local_integers():
Review Comment:
**Suggestion:** Add an explicit return type annotation (`-> None`) to this
new test function so the function signature is fully typed. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added test function in the changed hunk, and it omits an
explicit return type annotation. The rule requires new Python functions to be
fully typed, including return values, so this is a real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=afe5ef6468f34319bd53a40680a74bb6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=afe5ef6468f34319bd53a40680a74bb6&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:** tests/unit_tests/commands/dashboard/export_test.py
**Line:** 259:259
**Comment:**
*Custom Rule: Add an explicit return type annotation (`-> None`) to
this new test function so the function signature is fully typed.
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=425858e204d913b72ce4d646fbcf09eb5d3d8a52ce3970c8c14e5ae01c24f1bd&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40588&comment_hash=425858e204d913b72ce4d646fbcf09eb5d3d8a52ce3970c8c14e5ae01c24f1bd&reaction=dislike'>👎</a>
##########
superset/commands/dashboard/export.py:
##########
@@ -105,6 +106,166 @@ 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, native filter scopes, and
+ cross-filter/chart configuration) accordingly so the bundle stays
internally
+ consistent and the import-side id remap resolves against the same IDs.
+ Mappings are applied defensively — a reference whose source id is unknown
is
+ dropped rather than raising, and a node with a malformed ``meta.uuid`` is
+ skipped — 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
+ try:
+ new_id = stable_chart_id(str(chart_uuid))
+ except ValueError:
+ # A malformed ``meta.uuid`` (corrupt position_json) must not
+ # abort the whole export — skip stabilizing this single node
+ # and leave its existing chartId untouched.
+ logger.warning(
+ "Skipping chart id stabilization for invalid uuid %r",
+ chart_uuid,
+ )
+ continue
+ if isinstance(old_id, int):
+ id_map[old_id] = new_id
+ meta["chartId"] = new_id
+
+ if not id_map:
+ return
+
+ metadata = payload.get("metadata")
+ if not isinstance(metadata, dict):
+ return
+
+ def remap_id(old_id: Any) -> Optional[int]:
+ try:
+ return id_map.get(int(old_id))
+ except (TypeError, ValueError):
+ return None
Review Comment:
**Suggestion:** Add a short docstring to this new helper function so its
input normalization behavior and fallback semantics are documented inline.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The function is newly added and has no docstring, while the rule requires
newly added Python functions and classes to be documented inline. This is a
real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=738b30a433c744bebb6ffc11e5588a3b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=738b30a433c744bebb6ffc11e5588a3b&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:** 182:186
**Comment:**
*Custom Rule: Add a short docstring to this new helper function so its
input normalization behavior and fallback semantics are documented inline.
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=5cc9a56d3758199275b85ca52103065476c942df55b02cf3f3da60998b35d440&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40588&comment_hash=5cc9a56d3758199275b85ca52103065476c942df55b02cf3f3da60998b35d440&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/dashboard/export_test.py:
##########
@@ -256,6 +256,446 @@ def
test_file_content_omits_roles_field_when_dashboard_has_no_roles():
assert "roles" not in result
+def test_position_json_chart_id_leaks_env_local_integers():
+ """
+ Regression for #32972: dashboard export must produce stable output that
does
+ not vary with env-local integer chartIds.
+
+ The export format includes a ``meta.chartId`` field inside each ``CHART-*``
+ position entry. That integer is the database auto-increment primary key
from
+ the source environment. When the bundle is imported into a different
+ environment the importer (``update_id_refs``) rewrites those IDs to the
+ destination-env primary keys. A second export from the destination then
+ serializes the new env-local integers — the same logical chart produces
+ different ``chartId`` values in each environment.
+
+ This test asserts that two exports of the same logical dashboard are
+ identical even when the underlying chart has a different integer primary
key
+ in each environment. ``meta.uuid`` is the stable identifier that should be
+ used instead of ``meta.chartId``.
+
+ Fix target: ``superset/commands/dashboard/export.py`` (``append_charts``
+ and ``_file_content``) — strip or UUID-replace ``chartId`` so the export
+ format is environment-independent.
+ """
+ from superset.commands.dashboard.export import ExportDashboardsCommand
+
+ chart_uuid = "812bc377-ac09-475a-8d34-a63f7f087bd7"
+
+ # ------------------------------------------------------------------ #
+ # Source environment: chart has auto-increment id = 392 #
+ # ------------------------------------------------------------------ #
+ source_position = {
+ "DASHBOARD_VERSION_KEY": "v2",
+ "ROOT_ID": {"children": ["GRID_ID"], "id": "ROOT_ID", "type": "ROOT"},
+ "GRID_ID": {
+ "children": ["CHART-srcAAA"],
+ "id": "GRID_ID",
+ "parents": ["ROOT_ID"],
+ "type": "GRID",
+ },
+ "HEADER_ID": {
+ "id": "HEADER_ID",
+ "meta": {"text": "My Dashboard"},
+ "type": "HEADER",
+ },
+ "CHART-srcAAA": {
+ "children": [],
+ "id": "CHART-srcAAA",
+ "meta": {
+ "chartId": 392, # source-env integer primary key
+ "height": 20,
+ "sliceName": "My Wonderful Chart",
+ "uuid": chart_uuid,
+ "width": 4,
+ },
+ "parents": ["ROOT_ID", "GRID_ID"],
+ "type": "CHART",
+ },
+ }
+
+ src_dashboard = MagicMock()
+ src_dashboard.dashboard_title = "My Dashboard"
+ src_dashboard.theme = None
+ src_dashboard.slices = []
+ src_dashboard.tags = []
+ src_dashboard.roles = []
+ src_dashboard.export_to_dict.return_value = {
+ "position_json": json.dumps(source_position),
+ "json_metadata": json.dumps({"native_filter_configuration": []}),
+ }
+
+ with patch(
+
"superset.commands.dashboard.export.feature_flag_manager.is_feature_enabled",
+ return_value=False,
+ ):
+ first_export_content =
ExportDashboardsCommand._file_content(src_dashboard)
+
+ first_export = yaml.safe_load(first_export_content)
+ first_chart_positions = [
+ node
+ for node in first_export["position"].values()
+ if isinstance(node, dict) and node.get("type") == "CHART"
+ ]
+ assert first_chart_positions, "Export must contain at least one CHART
position node"
+ first_chart_meta = first_chart_positions[0]["meta"]
+
+ # uuid must be present — it is the stable cross-environment identifier
+ assert first_chart_meta.get("uuid") == chart_uuid, (
+ "meta.uuid must be present in exported position_json; "
+ "it is the stable identifier that survives re-import."
+ )
+
+ # ------------------------------------------------------------------ #
+ # Destination environment: same chart receives a different id = 1001 #
+ # (This is what happens after import — update_id_refs rewrites the #
+ # chartId field inside position_json to the destination-env integer.) #
+ # ------------------------------------------------------------------ #
+ dest_position = {
+ k: (
+ {
+ **v,
+ "meta": {
+ **v["meta"],
+ "chartId": 1001, # destination-env integer primary key
+ },
+ }
+ if isinstance(v, dict)
+ and v.get("type") == "CHART"
+ and v.get("meta", {}).get("uuid") == chart_uuid
+ else v
+ )
+ for k, v in source_position.items()
+ }
+
+ dest_dashboard = MagicMock()
+ dest_dashboard.dashboard_title = "My Dashboard"
+ dest_dashboard.theme = None
+ dest_dashboard.slices = []
+ dest_dashboard.tags = []
+ dest_dashboard.roles = []
+ dest_dashboard.export_to_dict.return_value = {
+ "position_json": json.dumps(dest_position),
+ "json_metadata": json.dumps({"native_filter_configuration": []}),
+ }
+
+ with patch(
+
"superset.commands.dashboard.export.feature_flag_manager.is_feature_enabled",
+ return_value=False,
+ ):
+ second_export_content =
ExportDashboardsCommand._file_content(dest_dashboard)
+
+ second_export = yaml.safe_load(second_export_content)
+ second_chart_positions = [
+ node
+ for node in second_export["position"].values()
+ if isinstance(node, dict) and node.get("type") == "CHART"
+ ]
+ assert second_chart_positions, "Second export must contain at least one
CHART node"
+ second_chart_meta = second_chart_positions[0]["meta"]
+
+ # uuid must survive the re-import round-trip unchanged
+ assert second_chart_meta.get("uuid") == chart_uuid, (
+ "meta.uuid must be stable across export → import → re-export; "
+ "UUIDs are the cross-environment identity, not integer IDs."
+ )
+
+ # ------------------------------------------------------------------ #
+ # THE REGRESSION: chartId must be stable (or absent) across envs. #
+ # #
+ # With the current (broken) implementation both assertions below #
+ # fail: #
+ # - first export emits chartId 392 (source-env integer) #
+ # - second export emits chartId 1001 (destination-env integer) #
+ # The fix should either omit chartId entirely or derive it from the #
+ # UUID so that both exports agree. #
+ # ------------------------------------------------------------------ #
+ first_chart_id = first_chart_meta.get("chartId")
+ second_chart_id = second_chart_meta.get("chartId")
+
+ assert first_chart_id == second_chart_id, (
+ f"meta.chartId must be stable across environments, but the source-env "
+ f"export produced chartId={first_chart_id!r} while the destination-env
"
+ f"export (after re-import with remapped IDs) produced "
+ f"chartId={second_chart_id!r}. "
+ "Env-local integer IDs are leaking into the export format (issue
#32972). "
+ "The fix: strip chartId from exported position_json or replace it with
a "
+ "value derived from meta.uuid so the export is
environment-independent."
+ )
+
+
+def _export_with_chart(
+ chart_uuid: str,
+ chart_id: int,
+ json_metadata: dict[str, Any],
+) -> dict[str, Any]:
+ """Export a single-chart dashboard and return the parsed YAML payload."""
+ from superset.commands.dashboard.export import ExportDashboardsCommand
+
+ position = {
+ "DASHBOARD_VERSION_KEY": "v2",
+ "ROOT_ID": {"children": ["GRID_ID"], "id": "ROOT_ID", "type": "ROOT"},
+ "GRID_ID": {
+ "children": ["CHART-aaa"],
+ "id": "GRID_ID",
+ "parents": ["ROOT_ID"],
+ "type": "GRID",
+ },
+ "CHART-aaa": {
+ "children": [],
+ "id": "CHART-aaa",
+ "meta": {
+ "chartId": chart_id,
+ "height": 20,
+ "sliceName": "Chart",
+ "uuid": chart_uuid,
+ "width": 4,
+ },
+ "parents": ["ROOT_ID", "GRID_ID"],
+ "type": "CHART",
+ },
+ }
+
+ dashboard = MagicMock()
+ dashboard.dashboard_title = "Test Dashboard"
+ dashboard.theme = None
+ dashboard.slices = []
+ dashboard.tags = []
+ dashboard.roles = []
+ dashboard.export_to_dict.return_value = {
+ "position_json": json.dumps(position),
+ "json_metadata": json.dumps(json_metadata),
+ }
+
+ with patch(
+
"superset.commands.dashboard.export.feature_flag_manager.is_feature_enabled",
+ return_value=False,
+ ):
+ content = ExportDashboardsCommand._file_content(dashboard)
+
+ return yaml.safe_load(content)
+
+
+def test_stabilize_chart_ids_skips_invalid_uuid():
+ """A malformed meta.uuid must not abort the whole dashboard export."""
+ result = _export_with_chart(
+ "not-a-valid-uuid",
+ 392,
+ {"native_filter_configuration": []},
+ )
+ chart_nodes = [
+ node
+ for node in result["position"].values()
+ if isinstance(node, dict) and node.get("type") == "CHART"
+ ]
+ # Export still succeeds; the unstabilizable node keeps its original
chartId.
+ assert chart_nodes
+ assert chart_nodes[0]["meta"]["chartId"] == 392
+
+
+def test_stabilize_chart_ids_remaps_native_filter_scope():
Review Comment:
**Suggestion:** Update this function signature to include an explicit return
type (`-> None`) so it does not omit type hints. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a new test function added by the patch and it lacks an explicit
return type hint. That matches the custom typing rule violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d771e82dbd2f42a0b73ed83b7c19ea8e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d771e82dbd2f42a0b73ed83b7c19ea8e&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:** tests/unit_tests/commands/dashboard/export_test.py
**Line:** 496:496
**Comment:**
*Custom Rule: Update this function signature to include an explicit
return type (`-> None`) so it does not omit type hints.
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=4d3c4cb8c55b553499786096776f75c20d4b7bb92bb5d63f3da701bbacedb1cd&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40588&comment_hash=4d3c4cb8c55b553499786096776f75c20d4b7bb92bb5d63f3da701bbacedb1cd&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/dashboard/export_test.py:
##########
@@ -256,6 +256,446 @@ def
test_file_content_omits_roles_field_when_dashboard_has_no_roles():
assert "roles" not in result
+def test_position_json_chart_id_leaks_env_local_integers():
+ """
+ Regression for #32972: dashboard export must produce stable output that
does
+ not vary with env-local integer chartIds.
+
+ The export format includes a ``meta.chartId`` field inside each ``CHART-*``
+ position entry. That integer is the database auto-increment primary key
from
+ the source environment. When the bundle is imported into a different
+ environment the importer (``update_id_refs``) rewrites those IDs to the
+ destination-env primary keys. A second export from the destination then
+ serializes the new env-local integers — the same logical chart produces
+ different ``chartId`` values in each environment.
+
+ This test asserts that two exports of the same logical dashboard are
+ identical even when the underlying chart has a different integer primary
key
+ in each environment. ``meta.uuid`` is the stable identifier that should be
+ used instead of ``meta.chartId``.
+
+ Fix target: ``superset/commands/dashboard/export.py`` (``append_charts``
+ and ``_file_content``) — strip or UUID-replace ``chartId`` so the export
+ format is environment-independent.
+ """
+ from superset.commands.dashboard.export import ExportDashboardsCommand
+
+ chart_uuid = "812bc377-ac09-475a-8d34-a63f7f087bd7"
+
+ # ------------------------------------------------------------------ #
+ # Source environment: chart has auto-increment id = 392 #
+ # ------------------------------------------------------------------ #
+ source_position = {
+ "DASHBOARD_VERSION_KEY": "v2",
+ "ROOT_ID": {"children": ["GRID_ID"], "id": "ROOT_ID", "type": "ROOT"},
+ "GRID_ID": {
+ "children": ["CHART-srcAAA"],
+ "id": "GRID_ID",
+ "parents": ["ROOT_ID"],
+ "type": "GRID",
+ },
+ "HEADER_ID": {
+ "id": "HEADER_ID",
+ "meta": {"text": "My Dashboard"},
+ "type": "HEADER",
+ },
+ "CHART-srcAAA": {
+ "children": [],
+ "id": "CHART-srcAAA",
+ "meta": {
+ "chartId": 392, # source-env integer primary key
+ "height": 20,
+ "sliceName": "My Wonderful Chart",
+ "uuid": chart_uuid,
+ "width": 4,
+ },
+ "parents": ["ROOT_ID", "GRID_ID"],
+ "type": "CHART",
+ },
+ }
+
+ src_dashboard = MagicMock()
+ src_dashboard.dashboard_title = "My Dashboard"
+ src_dashboard.theme = None
+ src_dashboard.slices = []
+ src_dashboard.tags = []
+ src_dashboard.roles = []
+ src_dashboard.export_to_dict.return_value = {
+ "position_json": json.dumps(source_position),
+ "json_metadata": json.dumps({"native_filter_configuration": []}),
+ }
+
+ with patch(
+
"superset.commands.dashboard.export.feature_flag_manager.is_feature_enabled",
+ return_value=False,
+ ):
+ first_export_content =
ExportDashboardsCommand._file_content(src_dashboard)
+
+ first_export = yaml.safe_load(first_export_content)
+ first_chart_positions = [
+ node
+ for node in first_export["position"].values()
+ if isinstance(node, dict) and node.get("type") == "CHART"
+ ]
+ assert first_chart_positions, "Export must contain at least one CHART
position node"
+ first_chart_meta = first_chart_positions[0]["meta"]
+
+ # uuid must be present — it is the stable cross-environment identifier
+ assert first_chart_meta.get("uuid") == chart_uuid, (
+ "meta.uuid must be present in exported position_json; "
+ "it is the stable identifier that survives re-import."
+ )
+
+ # ------------------------------------------------------------------ #
+ # Destination environment: same chart receives a different id = 1001 #
+ # (This is what happens after import — update_id_refs rewrites the #
+ # chartId field inside position_json to the destination-env integer.) #
+ # ------------------------------------------------------------------ #
+ dest_position = {
+ k: (
+ {
+ **v,
+ "meta": {
+ **v["meta"],
+ "chartId": 1001, # destination-env integer primary key
+ },
+ }
+ if isinstance(v, dict)
+ and v.get("type") == "CHART"
+ and v.get("meta", {}).get("uuid") == chart_uuid
+ else v
+ )
+ for k, v in source_position.items()
+ }
+
+ dest_dashboard = MagicMock()
+ dest_dashboard.dashboard_title = "My Dashboard"
+ dest_dashboard.theme = None
+ dest_dashboard.slices = []
+ dest_dashboard.tags = []
+ dest_dashboard.roles = []
+ dest_dashboard.export_to_dict.return_value = {
+ "position_json": json.dumps(dest_position),
+ "json_metadata": json.dumps({"native_filter_configuration": []}),
+ }
+
+ with patch(
+
"superset.commands.dashboard.export.feature_flag_manager.is_feature_enabled",
+ return_value=False,
+ ):
+ second_export_content =
ExportDashboardsCommand._file_content(dest_dashboard)
+
+ second_export = yaml.safe_load(second_export_content)
+ second_chart_positions = [
+ node
+ for node in second_export["position"].values()
+ if isinstance(node, dict) and node.get("type") == "CHART"
+ ]
+ assert second_chart_positions, "Second export must contain at least one
CHART node"
+ second_chart_meta = second_chart_positions[0]["meta"]
+
+ # uuid must survive the re-import round-trip unchanged
+ assert second_chart_meta.get("uuid") == chart_uuid, (
+ "meta.uuid must be stable across export → import → re-export; "
+ "UUIDs are the cross-environment identity, not integer IDs."
+ )
+
+ # ------------------------------------------------------------------ #
+ # THE REGRESSION: chartId must be stable (or absent) across envs. #
+ # #
+ # With the current (broken) implementation both assertions below #
+ # fail: #
+ # - first export emits chartId 392 (source-env integer) #
+ # - second export emits chartId 1001 (destination-env integer) #
+ # The fix should either omit chartId entirely or derive it from the #
+ # UUID so that both exports agree. #
+ # ------------------------------------------------------------------ #
+ first_chart_id = first_chart_meta.get("chartId")
+ second_chart_id = second_chart_meta.get("chartId")
+
+ assert first_chart_id == second_chart_id, (
+ f"meta.chartId must be stable across environments, but the source-env "
+ f"export produced chartId={first_chart_id!r} while the destination-env
"
+ f"export (after re-import with remapped IDs) produced "
+ f"chartId={second_chart_id!r}. "
+ "Env-local integer IDs are leaking into the export format (issue
#32972). "
+ "The fix: strip chartId from exported position_json or replace it with
a "
+ "value derived from meta.uuid so the export is
environment-independent."
+ )
+
+
+def _export_with_chart(
+ chart_uuid: str,
+ chart_id: int,
+ json_metadata: dict[str, Any],
+) -> dict[str, Any]:
+ """Export a single-chart dashboard and return the parsed YAML payload."""
+ from superset.commands.dashboard.export import ExportDashboardsCommand
+
+ position = {
+ "DASHBOARD_VERSION_KEY": "v2",
+ "ROOT_ID": {"children": ["GRID_ID"], "id": "ROOT_ID", "type": "ROOT"},
+ "GRID_ID": {
+ "children": ["CHART-aaa"],
+ "id": "GRID_ID",
+ "parents": ["ROOT_ID"],
+ "type": "GRID",
+ },
+ "CHART-aaa": {
+ "children": [],
+ "id": "CHART-aaa",
+ "meta": {
+ "chartId": chart_id,
+ "height": 20,
+ "sliceName": "Chart",
+ "uuid": chart_uuid,
+ "width": 4,
+ },
+ "parents": ["ROOT_ID", "GRID_ID"],
+ "type": "CHART",
+ },
+ }
+
+ dashboard = MagicMock()
+ dashboard.dashboard_title = "Test Dashboard"
+ dashboard.theme = None
+ dashboard.slices = []
+ dashboard.tags = []
+ dashboard.roles = []
+ dashboard.export_to_dict.return_value = {
+ "position_json": json.dumps(position),
+ "json_metadata": json.dumps(json_metadata),
+ }
+
+ with patch(
+
"superset.commands.dashboard.export.feature_flag_manager.is_feature_enabled",
+ return_value=False,
+ ):
+ content = ExportDashboardsCommand._file_content(dashboard)
+
+ return yaml.safe_load(content)
+
+
+def test_stabilize_chart_ids_skips_invalid_uuid():
+ """A malformed meta.uuid must not abort the whole dashboard export."""
+ result = _export_with_chart(
+ "not-a-valid-uuid",
+ 392,
+ {"native_filter_configuration": []},
+ )
+ chart_nodes = [
+ node
+ for node in result["position"].values()
+ if isinstance(node, dict) and node.get("type") == "CHART"
+ ]
+ # Export still succeeds; the unstabilizable node keeps its original
chartId.
+ assert chart_nodes
+ assert chart_nodes[0]["meta"]["chartId"] == 392
+
+
+def test_stabilize_chart_ids_remaps_native_filter_scope():
+ """Native filter scope.excluded / chartsInScope must track the stabilized
id."""
+ from superset.commands.dashboard.export import stable_chart_id
+
+ chart_uuid = "812bc377-ac09-475a-8d34-a63f7f087bd7"
+ new_id = stable_chart_id(chart_uuid)
+
+ result = _export_with_chart(
+ chart_uuid,
+ 392,
+ {
+ "native_filter_configuration": [
+ {
+ "id": "NATIVE_FILTER-1",
+ "scope": {"rootPath": ["ROOT_ID"], "excluded": [392]},
+ "chartsInScope": [392],
+ }
+ ]
+ },
+ )
+
+ native_filter = result["metadata"]["native_filter_configuration"][0]
+ assert native_filter["scope"]["excluded"] == [new_id]
+ assert native_filter["chartsInScope"] == [new_id]
+
+
+def test_stabilize_chart_ids_remaps_cross_filter_configuration():
Review Comment:
**Suggestion:** Add a return type hint (`-> None`) to this newly added test
function to comply with the typing rule. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The function is part of the newly added code and currently omits a return
type annotation. Under the stated rule, that is a genuine issue.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=cc2a8555935d433c8c36b88b28386329&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=cc2a8555935d433c8c36b88b28386329&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:** tests/unit_tests/commands/dashboard/export_test.py
**Line:** 522:522
**Comment:**
*Custom Rule: Add a return type hint (`-> None`) to this newly added
test function to comply with the typing rule.
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=31dfba1b079d31daab431bfb18cb10380c034e7e2e4a7a9c7244248d34772fa0&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40588&comment_hash=31dfba1b079d31daab431bfb18cb10380c034e7e2e4a7a9c7244248d34772fa0&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/dashboard/export_test.py:
##########
@@ -256,6 +256,446 @@ def
test_file_content_omits_roles_field_when_dashboard_has_no_roles():
assert "roles" not in result
+def test_position_json_chart_id_leaks_env_local_integers():
+ """
+ Regression for #32972: dashboard export must produce stable output that
does
+ not vary with env-local integer chartIds.
+
+ The export format includes a ``meta.chartId`` field inside each ``CHART-*``
+ position entry. That integer is the database auto-increment primary key
from
+ the source environment. When the bundle is imported into a different
+ environment the importer (``update_id_refs``) rewrites those IDs to the
+ destination-env primary keys. A second export from the destination then
+ serializes the new env-local integers — the same logical chart produces
+ different ``chartId`` values in each environment.
+
+ This test asserts that two exports of the same logical dashboard are
+ identical even when the underlying chart has a different integer primary
key
+ in each environment. ``meta.uuid`` is the stable identifier that should be
+ used instead of ``meta.chartId``.
+
+ Fix target: ``superset/commands/dashboard/export.py`` (``append_charts``
+ and ``_file_content``) — strip or UUID-replace ``chartId`` so the export
+ format is environment-independent.
+ """
+ from superset.commands.dashboard.export import ExportDashboardsCommand
+
+ chart_uuid = "812bc377-ac09-475a-8d34-a63f7f087bd7"
+
+ # ------------------------------------------------------------------ #
+ # Source environment: chart has auto-increment id = 392 #
+ # ------------------------------------------------------------------ #
+ source_position = {
+ "DASHBOARD_VERSION_KEY": "v2",
+ "ROOT_ID": {"children": ["GRID_ID"], "id": "ROOT_ID", "type": "ROOT"},
+ "GRID_ID": {
+ "children": ["CHART-srcAAA"],
+ "id": "GRID_ID",
+ "parents": ["ROOT_ID"],
+ "type": "GRID",
+ },
+ "HEADER_ID": {
+ "id": "HEADER_ID",
+ "meta": {"text": "My Dashboard"},
+ "type": "HEADER",
+ },
+ "CHART-srcAAA": {
+ "children": [],
+ "id": "CHART-srcAAA",
+ "meta": {
+ "chartId": 392, # source-env integer primary key
+ "height": 20,
+ "sliceName": "My Wonderful Chart",
+ "uuid": chart_uuid,
+ "width": 4,
+ },
+ "parents": ["ROOT_ID", "GRID_ID"],
+ "type": "CHART",
+ },
+ }
+
+ src_dashboard = MagicMock()
+ src_dashboard.dashboard_title = "My Dashboard"
+ src_dashboard.theme = None
+ src_dashboard.slices = []
+ src_dashboard.tags = []
+ src_dashboard.roles = []
+ src_dashboard.export_to_dict.return_value = {
+ "position_json": json.dumps(source_position),
+ "json_metadata": json.dumps({"native_filter_configuration": []}),
+ }
+
+ with patch(
+
"superset.commands.dashboard.export.feature_flag_manager.is_feature_enabled",
+ return_value=False,
+ ):
+ first_export_content =
ExportDashboardsCommand._file_content(src_dashboard)
+
+ first_export = yaml.safe_load(first_export_content)
+ first_chart_positions = [
+ node
+ for node in first_export["position"].values()
+ if isinstance(node, dict) and node.get("type") == "CHART"
+ ]
+ assert first_chart_positions, "Export must contain at least one CHART
position node"
+ first_chart_meta = first_chart_positions[0]["meta"]
+
+ # uuid must be present — it is the stable cross-environment identifier
+ assert first_chart_meta.get("uuid") == chart_uuid, (
+ "meta.uuid must be present in exported position_json; "
+ "it is the stable identifier that survives re-import."
+ )
+
+ # ------------------------------------------------------------------ #
+ # Destination environment: same chart receives a different id = 1001 #
+ # (This is what happens after import — update_id_refs rewrites the #
+ # chartId field inside position_json to the destination-env integer.) #
+ # ------------------------------------------------------------------ #
+ dest_position = {
+ k: (
+ {
+ **v,
+ "meta": {
+ **v["meta"],
+ "chartId": 1001, # destination-env integer primary key
+ },
+ }
+ if isinstance(v, dict)
+ and v.get("type") == "CHART"
+ and v.get("meta", {}).get("uuid") == chart_uuid
+ else v
+ )
+ for k, v in source_position.items()
+ }
+
+ dest_dashboard = MagicMock()
+ dest_dashboard.dashboard_title = "My Dashboard"
+ dest_dashboard.theme = None
+ dest_dashboard.slices = []
+ dest_dashboard.tags = []
+ dest_dashboard.roles = []
+ dest_dashboard.export_to_dict.return_value = {
+ "position_json": json.dumps(dest_position),
+ "json_metadata": json.dumps({"native_filter_configuration": []}),
+ }
+
+ with patch(
+
"superset.commands.dashboard.export.feature_flag_manager.is_feature_enabled",
+ return_value=False,
+ ):
+ second_export_content =
ExportDashboardsCommand._file_content(dest_dashboard)
+
+ second_export = yaml.safe_load(second_export_content)
+ second_chart_positions = [
+ node
+ for node in second_export["position"].values()
+ if isinstance(node, dict) and node.get("type") == "CHART"
+ ]
+ assert second_chart_positions, "Second export must contain at least one
CHART node"
+ second_chart_meta = second_chart_positions[0]["meta"]
+
+ # uuid must survive the re-import round-trip unchanged
+ assert second_chart_meta.get("uuid") == chart_uuid, (
+ "meta.uuid must be stable across export → import → re-export; "
+ "UUIDs are the cross-environment identity, not integer IDs."
+ )
+
+ # ------------------------------------------------------------------ #
+ # THE REGRESSION: chartId must be stable (or absent) across envs. #
+ # #
+ # With the current (broken) implementation both assertions below #
+ # fail: #
+ # - first export emits chartId 392 (source-env integer) #
+ # - second export emits chartId 1001 (destination-env integer) #
+ # The fix should either omit chartId entirely or derive it from the #
+ # UUID so that both exports agree. #
+ # ------------------------------------------------------------------ #
+ first_chart_id = first_chart_meta.get("chartId")
+ second_chart_id = second_chart_meta.get("chartId")
+
+ assert first_chart_id == second_chart_id, (
+ f"meta.chartId must be stable across environments, but the source-env "
+ f"export produced chartId={first_chart_id!r} while the destination-env
"
+ f"export (after re-import with remapped IDs) produced "
+ f"chartId={second_chart_id!r}. "
+ "Env-local integer IDs are leaking into the export format (issue
#32972). "
+ "The fix: strip chartId from exported position_json or replace it with
a "
+ "value derived from meta.uuid so the export is
environment-independent."
+ )
+
+
+def _export_with_chart(
+ chart_uuid: str,
+ chart_id: int,
+ json_metadata: dict[str, Any],
+) -> dict[str, Any]:
+ """Export a single-chart dashboard and return the parsed YAML payload."""
+ from superset.commands.dashboard.export import ExportDashboardsCommand
+
+ position = {
+ "DASHBOARD_VERSION_KEY": "v2",
+ "ROOT_ID": {"children": ["GRID_ID"], "id": "ROOT_ID", "type": "ROOT"},
+ "GRID_ID": {
+ "children": ["CHART-aaa"],
+ "id": "GRID_ID",
+ "parents": ["ROOT_ID"],
+ "type": "GRID",
+ },
+ "CHART-aaa": {
+ "children": [],
+ "id": "CHART-aaa",
+ "meta": {
+ "chartId": chart_id,
+ "height": 20,
+ "sliceName": "Chart",
+ "uuid": chart_uuid,
+ "width": 4,
+ },
+ "parents": ["ROOT_ID", "GRID_ID"],
+ "type": "CHART",
+ },
+ }
+
+ dashboard = MagicMock()
+ dashboard.dashboard_title = "Test Dashboard"
+ dashboard.theme = None
+ dashboard.slices = []
+ dashboard.tags = []
+ dashboard.roles = []
+ dashboard.export_to_dict.return_value = {
+ "position_json": json.dumps(position),
+ "json_metadata": json.dumps(json_metadata),
+ }
+
+ with patch(
+
"superset.commands.dashboard.export.feature_flag_manager.is_feature_enabled",
+ return_value=False,
+ ):
+ content = ExportDashboardsCommand._file_content(dashboard)
+
+ return yaml.safe_load(content)
+
+
+def test_stabilize_chart_ids_skips_invalid_uuid():
Review Comment:
**Suggestion:** Add an explicit return type annotation (`-> None`) to this
new test function to satisfy the full type-hint requirement for new Python
code. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This function is newly introduced in the diff and has no return annotation.
Since the rule says new Python code should be fully typed, the missing `->
None` is a valid violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a01db3806a494fc6ab628767211c13f6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a01db3806a494fc6ab628767211c13f6&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:** tests/unit_tests/commands/dashboard/export_test.py
**Line:** 479:479
**Comment:**
*Custom Rule: Add an explicit return type annotation (`-> None`) to
this new test function to satisfy the full type-hint requirement for new Python
code.
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=0d72e405cb25d8a2a85ec668d5f50c3116e11eacf1e40f2473f6ca09595b42e8&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40588&comment_hash=0d72e405cb25d8a2a85ec668d5f50c3116e11eacf1e40f2473f6ca09595b42e8&reaction=dislike'>👎</a>
##########
tests/unit_tests/commands/dashboard/export_test.py:
##########
@@ -256,6 +256,446 @@ def
test_file_content_omits_roles_field_when_dashboard_has_no_roles():
assert "roles" not in result
+def test_position_json_chart_id_leaks_env_local_integers():
+ """
+ Regression for #32972: dashboard export must produce stable output that
does
+ not vary with env-local integer chartIds.
+
+ The export format includes a ``meta.chartId`` field inside each ``CHART-*``
+ position entry. That integer is the database auto-increment primary key
from
+ the source environment. When the bundle is imported into a different
+ environment the importer (``update_id_refs``) rewrites those IDs to the
+ destination-env primary keys. A second export from the destination then
+ serializes the new env-local integers — the same logical chart produces
+ different ``chartId`` values in each environment.
+
+ This test asserts that two exports of the same logical dashboard are
+ identical even when the underlying chart has a different integer primary
key
+ in each environment. ``meta.uuid`` is the stable identifier that should be
+ used instead of ``meta.chartId``.
+
+ Fix target: ``superset/commands/dashboard/export.py`` (``append_charts``
+ and ``_file_content``) — strip or UUID-replace ``chartId`` so the export
+ format is environment-independent.
+ """
+ from superset.commands.dashboard.export import ExportDashboardsCommand
+
+ chart_uuid = "812bc377-ac09-475a-8d34-a63f7f087bd7"
+
+ # ------------------------------------------------------------------ #
+ # Source environment: chart has auto-increment id = 392 #
+ # ------------------------------------------------------------------ #
+ source_position = {
+ "DASHBOARD_VERSION_KEY": "v2",
+ "ROOT_ID": {"children": ["GRID_ID"], "id": "ROOT_ID", "type": "ROOT"},
+ "GRID_ID": {
+ "children": ["CHART-srcAAA"],
+ "id": "GRID_ID",
+ "parents": ["ROOT_ID"],
+ "type": "GRID",
+ },
+ "HEADER_ID": {
+ "id": "HEADER_ID",
+ "meta": {"text": "My Dashboard"},
+ "type": "HEADER",
+ },
+ "CHART-srcAAA": {
+ "children": [],
+ "id": "CHART-srcAAA",
+ "meta": {
+ "chartId": 392, # source-env integer primary key
+ "height": 20,
+ "sliceName": "My Wonderful Chart",
+ "uuid": chart_uuid,
+ "width": 4,
+ },
+ "parents": ["ROOT_ID", "GRID_ID"],
+ "type": "CHART",
+ },
+ }
+
+ src_dashboard = MagicMock()
+ src_dashboard.dashboard_title = "My Dashboard"
+ src_dashboard.theme = None
+ src_dashboard.slices = []
+ src_dashboard.tags = []
+ src_dashboard.roles = []
+ src_dashboard.export_to_dict.return_value = {
+ "position_json": json.dumps(source_position),
+ "json_metadata": json.dumps({"native_filter_configuration": []}),
+ }
+
+ with patch(
+
"superset.commands.dashboard.export.feature_flag_manager.is_feature_enabled",
+ return_value=False,
+ ):
+ first_export_content =
ExportDashboardsCommand._file_content(src_dashboard)
+
+ first_export = yaml.safe_load(first_export_content)
+ first_chart_positions = [
+ node
+ for node in first_export["position"].values()
+ if isinstance(node, dict) and node.get("type") == "CHART"
+ ]
+ assert first_chart_positions, "Export must contain at least one CHART
position node"
+ first_chart_meta = first_chart_positions[0]["meta"]
+
+ # uuid must be present — it is the stable cross-environment identifier
+ assert first_chart_meta.get("uuid") == chart_uuid, (
+ "meta.uuid must be present in exported position_json; "
+ "it is the stable identifier that survives re-import."
+ )
+
+ # ------------------------------------------------------------------ #
+ # Destination environment: same chart receives a different id = 1001 #
+ # (This is what happens after import — update_id_refs rewrites the #
+ # chartId field inside position_json to the destination-env integer.) #
+ # ------------------------------------------------------------------ #
+ dest_position = {
+ k: (
+ {
+ **v,
+ "meta": {
+ **v["meta"],
+ "chartId": 1001, # destination-env integer primary key
+ },
+ }
+ if isinstance(v, dict)
+ and v.get("type") == "CHART"
+ and v.get("meta", {}).get("uuid") == chart_uuid
+ else v
+ )
+ for k, v in source_position.items()
+ }
+
+ dest_dashboard = MagicMock()
+ dest_dashboard.dashboard_title = "My Dashboard"
+ dest_dashboard.theme = None
+ dest_dashboard.slices = []
+ dest_dashboard.tags = []
+ dest_dashboard.roles = []
+ dest_dashboard.export_to_dict.return_value = {
+ "position_json": json.dumps(dest_position),
+ "json_metadata": json.dumps({"native_filter_configuration": []}),
+ }
+
+ with patch(
+
"superset.commands.dashboard.export.feature_flag_manager.is_feature_enabled",
+ return_value=False,
+ ):
+ second_export_content =
ExportDashboardsCommand._file_content(dest_dashboard)
+
+ second_export = yaml.safe_load(second_export_content)
+ second_chart_positions = [
+ node
+ for node in second_export["position"].values()
+ if isinstance(node, dict) and node.get("type") == "CHART"
+ ]
+ assert second_chart_positions, "Second export must contain at least one
CHART node"
+ second_chart_meta = second_chart_positions[0]["meta"]
+
+ # uuid must survive the re-import round-trip unchanged
+ assert second_chart_meta.get("uuid") == chart_uuid, (
+ "meta.uuid must be stable across export → import → re-export; "
+ "UUIDs are the cross-environment identity, not integer IDs."
+ )
+
+ # ------------------------------------------------------------------ #
+ # THE REGRESSION: chartId must be stable (or absent) across envs. #
+ # #
+ # With the current (broken) implementation both assertions below #
+ # fail: #
+ # - first export emits chartId 392 (source-env integer) #
+ # - second export emits chartId 1001 (destination-env integer) #
+ # The fix should either omit chartId entirely or derive it from the #
+ # UUID so that both exports agree. #
+ # ------------------------------------------------------------------ #
+ first_chart_id = first_chart_meta.get("chartId")
+ second_chart_id = second_chart_meta.get("chartId")
+
+ assert first_chart_id == second_chart_id, (
+ f"meta.chartId must be stable across environments, but the source-env "
+ f"export produced chartId={first_chart_id!r} while the destination-env
"
+ f"export (after re-import with remapped IDs) produced "
+ f"chartId={second_chart_id!r}. "
+ "Env-local integer IDs are leaking into the export format (issue
#32972). "
+ "The fix: strip chartId from exported position_json or replace it with
a "
+ "value derived from meta.uuid so the export is
environment-independent."
+ )
+
+
+def _export_with_chart(
+ chart_uuid: str,
+ chart_id: int,
+ json_metadata: dict[str, Any],
+) -> dict[str, Any]:
+ """Export a single-chart dashboard and return the parsed YAML payload."""
+ from superset.commands.dashboard.export import ExportDashboardsCommand
+
+ position = {
+ "DASHBOARD_VERSION_KEY": "v2",
+ "ROOT_ID": {"children": ["GRID_ID"], "id": "ROOT_ID", "type": "ROOT"},
+ "GRID_ID": {
+ "children": ["CHART-aaa"],
+ "id": "GRID_ID",
+ "parents": ["ROOT_ID"],
+ "type": "GRID",
+ },
+ "CHART-aaa": {
+ "children": [],
+ "id": "CHART-aaa",
+ "meta": {
+ "chartId": chart_id,
+ "height": 20,
+ "sliceName": "Chart",
+ "uuid": chart_uuid,
+ "width": 4,
+ },
+ "parents": ["ROOT_ID", "GRID_ID"],
+ "type": "CHART",
+ },
+ }
+
+ dashboard = MagicMock()
+ dashboard.dashboard_title = "Test Dashboard"
+ dashboard.theme = None
+ dashboard.slices = []
+ dashboard.tags = []
+ dashboard.roles = []
+ dashboard.export_to_dict.return_value = {
+ "position_json": json.dumps(position),
+ "json_metadata": json.dumps(json_metadata),
+ }
+
+ with patch(
+
"superset.commands.dashboard.export.feature_flag_manager.is_feature_enabled",
+ return_value=False,
+ ):
+ content = ExportDashboardsCommand._file_content(dashboard)
+
+ return yaml.safe_load(content)
+
+
+def test_stabilize_chart_ids_skips_invalid_uuid():
+ """A malformed meta.uuid must not abort the whole dashboard export."""
+ result = _export_with_chart(
+ "not-a-valid-uuid",
+ 392,
+ {"native_filter_configuration": []},
+ )
+ chart_nodes = [
+ node
+ for node in result["position"].values()
+ if isinstance(node, dict) and node.get("type") == "CHART"
+ ]
+ # Export still succeeds; the unstabilizable node keeps its original
chartId.
+ assert chart_nodes
+ assert chart_nodes[0]["meta"]["chartId"] == 392
+
+
+def test_stabilize_chart_ids_remaps_native_filter_scope():
+ """Native filter scope.excluded / chartsInScope must track the stabilized
id."""
+ from superset.commands.dashboard.export import stable_chart_id
+
+ chart_uuid = "812bc377-ac09-475a-8d34-a63f7f087bd7"
+ new_id = stable_chart_id(chart_uuid)
+
+ result = _export_with_chart(
+ chart_uuid,
+ 392,
+ {
+ "native_filter_configuration": [
+ {
+ "id": "NATIVE_FILTER-1",
+ "scope": {"rootPath": ["ROOT_ID"], "excluded": [392]},
+ "chartsInScope": [392],
+ }
+ ]
+ },
+ )
+
+ native_filter = result["metadata"]["native_filter_configuration"][0]
+ assert native_filter["scope"]["excluded"] == [new_id]
+ assert native_filter["chartsInScope"] == [new_id]
+
+
+def test_stabilize_chart_ids_remaps_cross_filter_configuration():
+ """global_chart_configuration and chart_configuration must be remapped."""
+ from superset.commands.dashboard.export import stable_chart_id
+
+ chart_uuid = "812bc377-ac09-475a-8d34-a63f7f087bd7"
+ new_id = stable_chart_id(chart_uuid)
+
+ result = _export_with_chart(
+ chart_uuid,
+ 392,
+ {
+ "native_filter_configuration": [],
+ "global_chart_configuration": {
+ "scope": {"rootPath": ["ROOT_ID"], "excluded": [392]},
+ "chartsInScope": [392],
+ },
+ "chart_configuration": {
+ "392": {
+ "id": 392,
+ "crossFilters": {
+ "scope": {"rootPath": ["ROOT_ID"], "excluded": [392]},
+ "chartsInScope": [392],
+ },
+ }
+ },
+ },
+ )
+
+ metadata = result["metadata"]
+ assert metadata["global_chart_configuration"]["scope"]["excluded"] ==
[new_id]
+ assert metadata["global_chart_configuration"]["chartsInScope"] == [new_id]
+
+ # chart_configuration is re-keyed and its inner id / scopes are remapped.
+ assert str(new_id) in metadata["chart_configuration"]
+ chart_config = metadata["chart_configuration"][str(new_id)]
+ assert chart_config["id"] == new_id
+ assert chart_config["crossFilters"]["scope"]["excluded"] == [new_id]
+ assert chart_config["crossFilters"]["chartsInScope"] == [new_id]
+
+
+def test_stable_chart_id_is_deterministic_and_in_range():
Review Comment:
**Suggestion:** Include an explicit return annotation (`-> None`) in this
new function definition so it does not omit return typing. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly added test function does not include an explicit `-> None` return
annotation. Because the PR introduces new Python code, the missing type hint
violates the custom rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4cf800924e0c42af82ff5258e77da505&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=4cf800924e0c42af82ff5258e77da505&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:** tests/unit_tests/commands/dashboard/export_test.py
**Line:** 562:562
**Comment:**
*Custom Rule: Include an explicit return annotation (`-> None`) in this
new function definition so it does not omit return typing.
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=8ee03a081ac677afb0f70934e8a107967667c02246dc5f90edd3027e8f3a8250&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40588&comment_hash=8ee03a081ac677afb0f70934e8a107967667c02246dc5f90edd3027e8f3a8250&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]
