codeant-ai-for-open-source[bot] commented on code in PR #40588:
URL: https://github.com/apache/superset/pull/40588#discussion_r3449989634
##########
superset/commands/dashboard/export.py:
##########
@@ -105,6 +106,172 @@ def append_charts(position: dict[str, Any], charts:
set[Slice]) -> dict[str, Any
return position
+# Bound the derived id to the largest positive signed 32-bit integer
+# (2**31 - 1) so it stays 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` uses modulo reduction of UUIDs into a
31-bit space, which is not collision-safe. If two charts in the same dashboard
collide, they will get the same stabilized ID and the remap code will silently
merge/overwrite chart-keyed metadata entries (`filter_scopes`,
`chart_configuration`, etc.), causing incorrect filter scope wiring on import.
Use a collision-free per-dashboard mapping (for example, deterministically
assign unique integers from sorted chart UUIDs) instead of lossy hashing.
[logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dashboard export merges filter_scopes entries for colliding chart IDs.
- ❌ chart_configuration for one chart lost in exported YAML bundle.
- ⚠️ Imported dashboard filters mis-target charts when collision occurs.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Inspect `stable_chart_id` in
`superset/commands/dashboard/export.py:116-127`, which
derives an integer from a UUID via `(uuid_module.UUID(chart_uuid).int %
_STABLE_CHART_ID_MODULO) + 1` with `_STABLE_CHART_ID_MODULO = 2_147_483_647`
at line 113,
mathematically guaranteeing that multiple distinct UUIDs map to the same
stabilized
integer id.
2. Consider a dashboard whose `position_json` contains at least two distinct
`CHART-*`
nodes with different `meta.uuid` values (the normal case) and with integer
`meta.chartId`
values (env‑local primary keys), as in the single-chart fixture built in
`_export_with_chart` at
`tests/unit_tests/commands/dashboard/export_test.py:22-52`, but
extended to two charts.
3. Call `ExportDashboardsCommand._file_content` (in
`superset/commands/dashboard/export.py:285-377`) on such a dashboard; inside
it
`_stabilize_chart_ids(payload)` is invoked at lines 352-355, which builds
`id_map` and
then rewrites metadata structures like `filter_scopes`, `expanded_slices`,
`default_filters`, and `chart_configuration` using comprehensions keyed by
the stabilized
ids (see lines 205-209, 220-224, 232-237, and 262-272).
4. If two charts' UUIDs happen to satisfy `stable_chart_id(uuid1) ==
stable_chart_id(uuid2)`, then when `_stabilize_chart_ids` re-keys
`metadata["filter_scopes"]`, `metadata["expanded_slices"]`,
`default_filters` (JSON
object), or `metadata["chart_configuration"]` by `str(new_key)`, the second
entry with the
same `new_key` overwrites the first, so on the exported YAML bundle one
chart's
filter/visibility configuration is silently dropped or merged, leading to
incorrect filter
wiring upon import via the normal dashboard export/import flow.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=71ceb2a1fea44ccf9d9cfeccaed5430c&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=71ceb2a1fea44ccf9d9cfeccaed5430c&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:** 113:127
**Comment:**
*Logic Error: `stable_chart_id` uses modulo reduction of UUIDs into a
31-bit space, which is not collision-safe. If two charts in the same dashboard
collide, they will get the same stabilized ID and the remap code will silently
merge/overwrite chart-keyed metadata entries (`filter_scopes`,
`chart_configuration`, etc.), causing incorrect filter scope wiring on import.
Use a collision-free per-dashboard mapping (for example, deterministically
assign unique integers from sorted chart UUIDs) instead of lossy hashing.
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=994479928119079938fe319e7929039d23855dbb534abfe294c2959df751769b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40588&comment_hash=994479928119079938fe319e7929039d23855dbb534abfe294c2959df751769b&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]
