codeant-ai-for-open-source[bot] commented on code in PR #39857:
URL: https://github.com/apache/superset/pull/39857#discussion_r3649830178
##########
tests/integration_tests/charts/commands_tests.py:
##########
@@ -761,3 +764,737 @@ def test_fave_unfave_chart_command_non_owner(self,
mock_find_by_id):
finally:
if example_chart.datasource:
self.revoke_role_access_to_table("Gamma",
example_chart.datasource)
+
+
+class TestExportChartsAnnotationLayers(SupersetTestCase):
+ """Tests for annotation layer handling in chart export."""
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_native_annotation_uuid_replacement(self, mock_g):
+ """Test that NATIVE annotation layer IDs are replaced with UUIDs on
export."""
+ mock_g.user = security_manager.find_user("admin")
+
+ # Create an annotation layer
+ ann_layer = AnnotationLayer(name="Test Layer")
+ db.session.add(ann_layer)
+ db.session.commit()
+
+ # Get a chart and set params with a NATIVE annotation referencing the
layer
+ chart = db.session.query(Slice).filter_by(slice_name="Energy
Sankey").one()
+ original_params = json.loads(chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Layer 1",
+ "sourceType": "NATIVE",
+ "value": ann_layer.id,
+ "show": True,
+ "style": "solid",
+ }
+ ]
+ chart.params = json.dumps(original_params)
+ db.session.commit()
Review Comment:
**Suggestion:** The test permanently modifies the shared `Energy Sankey`
chart's persisted `params` and never restores the original value. If the test
suite reuses this fixture or the test fails before cleanup, later tests can
observe the injected annotation configuration and become order-dependent.
Restore `chart.params` in a `finally` block or isolate the chart mutation in a
transaction. [state inconsistency]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Export tests can observe altered Energy Sankey parameters.
- ⚠️ Test outcomes can depend on execution order.
- ⚠️ Failed tests can leave persistent fixture state behind.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run
`TestExportChartsAnnotationLayers.test_export_chart_no_annotation_layers` in
`tests/integration_tests/charts/commands_tests.py:1002`, which obtains the
shared `Energy
Sankey` chart at line 1006.
2. The test removes `annotation_layers` from the chart parameters at lines
1008-1010 and
commits the change at line 1011; the original parameters are not restored.
3. Execute a later test that uses the same fixture chart, such as
`test_export_chart_native_annotation_uuid_replacement` at line 774 or
`test_export_chart_query_context_annotation_uuid_replacement` at line 1030.
Those tests
load `Energy Sankey` and read its persisted parameters.
4. If the fixture/database is reused, those tests observe the modified
parameters rather
than the fixture's original state. If an assertion fails before line 1027,
the test also
leaves its database changes behind, increasing order dependence.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3d3da461131547959c2b46a2b93c0258&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=3d3da461131547959c2b46a2b93c0258&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/integration_tests/charts/commands_tests.py
**Line:** 786:796
**Comment:**
*State Inconsistency: The test permanently modifies the shared `Energy
Sankey` chart's persisted `params` and never restores the original value. If
the test suite reuses this fixture or the test fails before cleanup, later
tests can observe the injected annotation configuration and become
order-dependent. Restore `chart.params` in a `finally` block or isolate the
chart mutation in a transaction.
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%2F39857&comment_hash=60d5e388ee770ca0a78dbfff98fa5daea3f778ffe65850c5e41a2bcad4937f0c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39857&comment_hash=60d5e388ee770ca0a78dbfff98fa5daea3f778ffe65850c5e41a2bcad4937f0c&reaction=dislike'>👎</a>
##########
tests/integration_tests/charts/commands_tests.py:
##########
@@ -761,3 +764,737 @@ def test_fave_unfave_chart_command_non_owner(self,
mock_find_by_id):
finally:
if example_chart.datasource:
self.revoke_role_access_to_table("Gamma",
example_chart.datasource)
+
+
+class TestExportChartsAnnotationLayers(SupersetTestCase):
+ """Tests for annotation layer handling in chart export."""
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_native_annotation_uuid_replacement(self, mock_g):
+ """Test that NATIVE annotation layer IDs are replaced with UUIDs on
export."""
+ mock_g.user = security_manager.find_user("admin")
+
+ # Create an annotation layer
+ ann_layer = AnnotationLayer(name="Test Layer")
+ db.session.add(ann_layer)
+ db.session.commit()
+
+ # Get a chart and set params with a NATIVE annotation referencing the
layer
+ chart = db.session.query(Slice).filter_by(slice_name="Energy
Sankey").one()
+ original_params = json.loads(chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Layer 1",
+ "sourceType": "NATIVE",
+ "value": ann_layer.id,
+ "show": True,
+ "style": "solid",
+ }
+ ]
+ chart.params = json.dumps(original_params)
+ db.session.commit()
+
+ command = ExportChartsCommand([chart.id])
+ contents = dict(command.run())
+
+ chart_key = f"charts/Energy_Sankey_{chart.id}.yaml"
+ chart_yaml = yaml.safe_load(contents[chart_key]())
+ layers = chart_yaml["params"]["annotation_layers"]
+ assert len(layers) == 1
+ assert layers[0]["value"] == str(ann_layer.uuid)
+ assert layers[0]["sourceType"] == "NATIVE"
+
+ # Verify annotation layer YAML is included in export
+ ann_layer_key = [k for k in contents if
k.startswith("annotation_layers/")]
+ assert len(ann_layer_key) == 1
+
+ # Clean up
+ db.session.query(Annotation).filter_by(layer_id=ann_layer.id).delete()
+ db.session.delete(ann_layer)
+ db.session.commit()
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_table_annotation_uuid_replacement(self, mock_g):
+ """Test that table/line annotation chart IDs are replaced with
UUIDs."""
+ mock_g.user = security_manager.find_user("admin")
+
+ # Get two charts: one main, one referenced as annotation source
+ charts = (
+ db.session.query(Slice)
+ .filter(Slice.slice_name.in_(["Energy Sankey", "Heatmap"]))
+ .all()
+ )
+ main_chart = next(c for c in charts if c.slice_name == "Energy Sankey")
+ ref_chart = next(c for c in charts if c.slice_name == "Heatmap")
+
+ # Set params with a table annotation referencing the other chart
+ original_params = json.loads(main_chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Table Annotation",
+ "sourceType": "table",
+ "value": ref_chart.id,
+ "show": True,
+ "style": "solid",
+ }
+ ]
+ main_chart.params = json.dumps(original_params)
+ db.session.commit()
Review Comment:
**Suggestion:** This test mutates the shared `Energy Sankey` chart's
`params` without restoring the original configuration. The mutation remains in
the database after the test, so subsequent tests using the same chart can
inherit the synthetic annotation layer, especially when an assertion aborts
before any cleanup. [state inconsistency]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Later export tests can inherit a table annotation.
- ⚠️ Shared chart fixture state becomes order-dependent.
- ⚠️ Failed tests can contaminate subsequent integration tests.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run
`TestExportChartsAnnotationLayers.test_export_chart_table_annotation_uuid_replacement`
at
`tests/integration_tests/charts/commands_tests.py:819`; it loads the shared
`Energy
Sankey` and `Heatmap` charts at lines 823-830.
2. The test writes a synthetic table annotation referencing `Heatmap` into
`Energy
Sankey.params` at lines 834-843 and commits it at line 844.
3. The export assertion completes at lines 846-858, but no code restores
`main_chart.params` afterward.
4. Run another test using `Energy Sankey`, such as
`test_export_chart_no_annotation_layers` at line 1002. Its expectation that
the chart has
no annotation layers can be affected by the previously committed table
annotation, and an
earlier assertion failure leaves the mutation committed as well.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b46775df7df147b792141a616a61d7c3&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=b46775df7df147b792141a616a61d7c3&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/integration_tests/charts/commands_tests.py
**Line:** 834:844
**Comment:**
*State Inconsistency: This test mutates the shared `Energy Sankey`
chart's `params` without restoring the original configuration. The mutation
remains in the database after the test, so subsequent tests using the same
chart can inherit the synthetic annotation layer, especially when an assertion
aborts before any cleanup.
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%2F39857&comment_hash=9a49c216538778a30b4a888af802b4fb5f2762ae38428a12d8c1fa08de169a6c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39857&comment_hash=9a49c216538778a30b4a888af802b4fb5f2762ae38428a12d8c1fa08de169a6c&reaction=dislike'>👎</a>
##########
tests/integration_tests/charts/commands_tests.py:
##########
@@ -761,3 +764,737 @@ def test_fave_unfave_chart_command_non_owner(self,
mock_find_by_id):
finally:
if example_chart.datasource:
self.revoke_role_access_to_table("Gamma",
example_chart.datasource)
+
+
+class TestExportChartsAnnotationLayers(SupersetTestCase):
+ """Tests for annotation layer handling in chart export."""
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_native_annotation_uuid_replacement(self, mock_g):
+ """Test that NATIVE annotation layer IDs are replaced with UUIDs on
export."""
+ mock_g.user = security_manager.find_user("admin")
+
+ # Create an annotation layer
+ ann_layer = AnnotationLayer(name="Test Layer")
+ db.session.add(ann_layer)
+ db.session.commit()
+
+ # Get a chart and set params with a NATIVE annotation referencing the
layer
+ chart = db.session.query(Slice).filter_by(slice_name="Energy
Sankey").one()
+ original_params = json.loads(chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Layer 1",
+ "sourceType": "NATIVE",
+ "value": ann_layer.id,
+ "show": True,
+ "style": "solid",
+ }
+ ]
+ chart.params = json.dumps(original_params)
+ db.session.commit()
+
+ command = ExportChartsCommand([chart.id])
+ contents = dict(command.run())
+
+ chart_key = f"charts/Energy_Sankey_{chart.id}.yaml"
+ chart_yaml = yaml.safe_load(contents[chart_key]())
+ layers = chart_yaml["params"]["annotation_layers"]
+ assert len(layers) == 1
+ assert layers[0]["value"] == str(ann_layer.uuid)
+ assert layers[0]["sourceType"] == "NATIVE"
+
+ # Verify annotation layer YAML is included in export
+ ann_layer_key = [k for k in contents if
k.startswith("annotation_layers/")]
+ assert len(ann_layer_key) == 1
+
+ # Clean up
+ db.session.query(Annotation).filter_by(layer_id=ann_layer.id).delete()
+ db.session.delete(ann_layer)
+ db.session.commit()
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_table_annotation_uuid_replacement(self, mock_g):
+ """Test that table/line annotation chart IDs are replaced with
UUIDs."""
+ mock_g.user = security_manager.find_user("admin")
+
+ # Get two charts: one main, one referenced as annotation source
+ charts = (
+ db.session.query(Slice)
+ .filter(Slice.slice_name.in_(["Energy Sankey", "Heatmap"]))
+ .all()
+ )
+ main_chart = next(c for c in charts if c.slice_name == "Energy Sankey")
+ ref_chart = next(c for c in charts if c.slice_name == "Heatmap")
+
+ # Set params with a table annotation referencing the other chart
+ original_params = json.loads(main_chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Table Annotation",
+ "sourceType": "table",
+ "value": ref_chart.id,
+ "show": True,
+ "style": "solid",
+ }
+ ]
+ main_chart.params = json.dumps(original_params)
+ db.session.commit()
+
+ command = ExportChartsCommand([main_chart.id])
+ contents = dict(command.run())
+
+ chart_key = f"charts/Energy_Sankey_{main_chart.id}.yaml"
+ chart_yaml = yaml.safe_load(contents[chart_key]())
+ layers = chart_yaml["params"]["annotation_layers"]
+ assert len(layers) == 1
+ assert layers[0]["value"] == str(ref_chart.uuid)
+ assert layers[0]["sourceType"] == "table"
+
+ # The referenced chart should also be exported
+ ref_chart_key = f"charts/Heatmap_{ref_chart.id}.yaml"
+ assert ref_chart_key in contents
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_line_annotation_uuid_replacement(self, mock_g):
+ """Test that line sourceType annotation chart IDs are replaced with
UUIDs."""
+ mock_g.user = security_manager.find_user("admin")
+
+ charts = (
+ db.session.query(Slice)
+ .filter(Slice.slice_name.in_(["Energy Sankey", "Heatmap"]))
+ .all()
+ )
+ main_chart = next(c for c in charts if c.slice_name == "Energy Sankey")
+ ref_chart = next(c for c in charts if c.slice_name == "Heatmap")
+
+ original_params = json.loads(main_chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Line Annotation",
+ "sourceType": "line",
+ "value": ref_chart.id,
+ "show": True,
+ "style": "dashed",
+ }
+ ]
+ main_chart.params = json.dumps(original_params)
+ db.session.commit()
+
+ command = ExportChartsCommand([main_chart.id])
+ contents = dict(command.run())
+
+ chart_key = f"charts/Energy_Sankey_{main_chart.id}.yaml"
+ chart_yaml = yaml.safe_load(contents[chart_key]())
+ layers = chart_yaml["params"]["annotation_layers"]
+ assert len(layers) == 1
+ assert layers[0]["value"] == str(ref_chart.uuid)
+ assert layers[0]["sourceType"] == "line"
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_formula_annotation_unchanged(self, mock_g):
+ """Test that FORMULA annotations are not modified during export."""
+ mock_g.user = security_manager.find_user("admin")
+
+ chart = db.session.query(Slice).filter_by(slice_name="Energy
Sankey").one()
+ original_params = json.loads(chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Formula",
+ "sourceType": "FORMULA",
+ "value": "sin(x)",
+ "show": True,
+ "style": "solid",
+ }
+ ]
+ chart.params = json.dumps(original_params)
+ db.session.commit()
Review Comment:
**Suggestion:** The formula test commits a modified `Energy Sankey` chart
but never restores its original parameters. This contaminates the shared
fixture for later tests and leaves the database in a different state even when
the test passes. [state inconsistency]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Later exports can contain an unintended formula annotation.
- ⚠️ Fixture-dependent tests may become order-sensitive.
- ⚠️ Persistent test data differs from the expected baseline.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run
`TestExportChartsAnnotationLayers.test_export_chart_formula_annotation_unchanged`
at `tests/integration_tests/charts/commands_tests.py:899`; it loads the
shared `Energy
Sankey` chart at line 903.
2. The test adds a `FORMULA` annotation with value `sin(x)` at lines 905-914
and commits
the chart at line 915.
3. The export assertion runs at lines 917-925, but there is no restoration of
`chart.params` after the assertion.
4. Run another test that expects the fixture's original `Energy Sankey`
parameters, such
as `test_export_chart_no_annotation_layers` at line 1002. A reused database
exposes the
committed formula annotation, and any failure before the assertion completes
also skips
cleanup.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=77afe1efaf3049549ae0237c2eea1276&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=77afe1efaf3049549ae0237c2eea1276&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/integration_tests/charts/commands_tests.py
**Line:** 905:915
**Comment:**
*State Inconsistency: The formula test commits a modified `Energy
Sankey` chart but never restores its original parameters. This contaminates the
shared fixture for later tests and leaves the database in a different state
even when the test passes.
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%2F39857&comment_hash=b471ded7cc3837a4d96f37630f9c76fe09cf549ac2cc3815c46463f885e8148b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39857&comment_hash=b471ded7cc3837a4d96f37630f9c76fe09cf549ac2cc3815c46463f885e8148b&reaction=dislike'>👎</a>
##########
tests/integration_tests/charts/commands_tests.py:
##########
@@ -761,3 +764,737 @@ def test_fave_unfave_chart_command_non_owner(self,
mock_find_by_id):
finally:
if example_chart.datasource:
self.revoke_role_access_to_table("Gamma",
example_chart.datasource)
+
+
+class TestExportChartsAnnotationLayers(SupersetTestCase):
+ """Tests for annotation layer handling in chart export."""
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_native_annotation_uuid_replacement(self, mock_g):
+ """Test that NATIVE annotation layer IDs are replaced with UUIDs on
export."""
+ mock_g.user = security_manager.find_user("admin")
+
+ # Create an annotation layer
+ ann_layer = AnnotationLayer(name="Test Layer")
+ db.session.add(ann_layer)
+ db.session.commit()
+
+ # Get a chart and set params with a NATIVE annotation referencing the
layer
+ chart = db.session.query(Slice).filter_by(slice_name="Energy
Sankey").one()
+ original_params = json.loads(chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Layer 1",
+ "sourceType": "NATIVE",
+ "value": ann_layer.id,
+ "show": True,
+ "style": "solid",
+ }
+ ]
+ chart.params = json.dumps(original_params)
+ db.session.commit()
+
+ command = ExportChartsCommand([chart.id])
+ contents = dict(command.run())
+
+ chart_key = f"charts/Energy_Sankey_{chart.id}.yaml"
+ chart_yaml = yaml.safe_load(contents[chart_key]())
+ layers = chart_yaml["params"]["annotation_layers"]
+ assert len(layers) == 1
+ assert layers[0]["value"] == str(ann_layer.uuid)
+ assert layers[0]["sourceType"] == "NATIVE"
+
+ # Verify annotation layer YAML is included in export
+ ann_layer_key = [k for k in contents if
k.startswith("annotation_layers/")]
+ assert len(ann_layer_key) == 1
+
+ # Clean up
+ db.session.query(Annotation).filter_by(layer_id=ann_layer.id).delete()
+ db.session.delete(ann_layer)
+ db.session.commit()
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_table_annotation_uuid_replacement(self, mock_g):
+ """Test that table/line annotation chart IDs are replaced with
UUIDs."""
+ mock_g.user = security_manager.find_user("admin")
+
+ # Get two charts: one main, one referenced as annotation source
+ charts = (
+ db.session.query(Slice)
+ .filter(Slice.slice_name.in_(["Energy Sankey", "Heatmap"]))
+ .all()
+ )
+ main_chart = next(c for c in charts if c.slice_name == "Energy Sankey")
+ ref_chart = next(c for c in charts if c.slice_name == "Heatmap")
+
+ # Set params with a table annotation referencing the other chart
+ original_params = json.loads(main_chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Table Annotation",
+ "sourceType": "table",
+ "value": ref_chart.id,
+ "show": True,
+ "style": "solid",
+ }
+ ]
+ main_chart.params = json.dumps(original_params)
+ db.session.commit()
+
+ command = ExportChartsCommand([main_chart.id])
+ contents = dict(command.run())
+
+ chart_key = f"charts/Energy_Sankey_{main_chart.id}.yaml"
+ chart_yaml = yaml.safe_load(contents[chart_key]())
+ layers = chart_yaml["params"]["annotation_layers"]
+ assert len(layers) == 1
+ assert layers[0]["value"] == str(ref_chart.uuid)
+ assert layers[0]["sourceType"] == "table"
+
+ # The referenced chart should also be exported
+ ref_chart_key = f"charts/Heatmap_{ref_chart.id}.yaml"
+ assert ref_chart_key in contents
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_line_annotation_uuid_replacement(self, mock_g):
+ """Test that line sourceType annotation chart IDs are replaced with
UUIDs."""
+ mock_g.user = security_manager.find_user("admin")
+
+ charts = (
+ db.session.query(Slice)
+ .filter(Slice.slice_name.in_(["Energy Sankey", "Heatmap"]))
+ .all()
+ )
+ main_chart = next(c for c in charts if c.slice_name == "Energy Sankey")
+ ref_chart = next(c for c in charts if c.slice_name == "Heatmap")
+
+ original_params = json.loads(main_chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Line Annotation",
+ "sourceType": "line",
+ "value": ref_chart.id,
+ "show": True,
+ "style": "dashed",
+ }
+ ]
+ main_chart.params = json.dumps(original_params)
+ db.session.commit()
Review Comment:
**Suggestion:** This test persists a synthetic line annotation on the shared
`Energy Sankey` chart and does not restore the previous `params`. A failed or
subsequently reused test can therefore see stale annotation state and produce
misleading results. [state inconsistency]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Later chart-export tests can inherit line annotations.
- ⚠️ Results can vary with test execution order.
- ⚠️ Failed tests leave committed chart mutations.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run
`TestExportChartsAnnotationLayers.test_export_chart_line_annotation_uuid_replacement`
at
`tests/integration_tests/charts/commands_tests.py:862`; it selects `Energy
Sankey` and
`Heatmap` at lines 867-872.
2. The test assigns a synthetic `line` annotation to `Energy Sankey.params`
at lines
875-884 and commits it at line 885.
3. The test verifies the export at lines 887-895 but does not restore the
original
parameters afterward.
4. Run a later test that reads or exports `Energy Sankey`, such as
`test_export_chart_query_context_annotation_uuid_replacement` at line 1030.
With a reused
integration-test database, it can see the stale line annotation; if the
earlier assertion
fails, cleanup is skipped entirely.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1dee501c549743c286d83e14745f1b61&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=1dee501c549743c286d83e14745f1b61&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/integration_tests/charts/commands_tests.py
**Line:** 875:885
**Comment:**
*State Inconsistency: This test persists a synthetic line annotation on
the shared `Energy Sankey` chart and does not restore the previous `params`. A
failed or subsequently reused test can therefore see stale annotation state and
produce misleading results.
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%2F39857&comment_hash=c7b7e56bceecb6815d5a660186d31e7c40ea371a7e72e8e9cc233995913f27bf&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39857&comment_hash=c7b7e56bceecb6815d5a660186d31e7c40ea371a7e72e8e9cc233995913f27bf&reaction=dislike'>👎</a>
##########
tests/integration_tests/charts/commands_tests.py:
##########
@@ -761,3 +764,737 @@ def test_fave_unfave_chart_command_non_owner(self,
mock_find_by_id):
finally:
if example_chart.datasource:
self.revoke_role_access_to_table("Gamma",
example_chart.datasource)
+
+
+class TestExportChartsAnnotationLayers(SupersetTestCase):
+ """Tests for annotation layer handling in chart export."""
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_native_annotation_uuid_replacement(self, mock_g):
+ """Test that NATIVE annotation layer IDs are replaced with UUIDs on
export."""
+ mock_g.user = security_manager.find_user("admin")
+
+ # Create an annotation layer
+ ann_layer = AnnotationLayer(name="Test Layer")
+ db.session.add(ann_layer)
+ db.session.commit()
+
+ # Get a chart and set params with a NATIVE annotation referencing the
layer
+ chart = db.session.query(Slice).filter_by(slice_name="Energy
Sankey").one()
+ original_params = json.loads(chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Layer 1",
+ "sourceType": "NATIVE",
+ "value": ann_layer.id,
+ "show": True,
+ "style": "solid",
+ }
+ ]
+ chart.params = json.dumps(original_params)
+ db.session.commit()
+
+ command = ExportChartsCommand([chart.id])
+ contents = dict(command.run())
+
+ chart_key = f"charts/Energy_Sankey_{chart.id}.yaml"
+ chart_yaml = yaml.safe_load(contents[chart_key]())
+ layers = chart_yaml["params"]["annotation_layers"]
+ assert len(layers) == 1
+ assert layers[0]["value"] == str(ann_layer.uuid)
+ assert layers[0]["sourceType"] == "NATIVE"
+
+ # Verify annotation layer YAML is included in export
+ ann_layer_key = [k for k in contents if
k.startswith("annotation_layers/")]
+ assert len(ann_layer_key) == 1
+
+ # Clean up
+ db.session.query(Annotation).filter_by(layer_id=ann_layer.id).delete()
+ db.session.delete(ann_layer)
+ db.session.commit()
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_table_annotation_uuid_replacement(self, mock_g):
+ """Test that table/line annotation chart IDs are replaced with
UUIDs."""
+ mock_g.user = security_manager.find_user("admin")
+
+ # Get two charts: one main, one referenced as annotation source
+ charts = (
+ db.session.query(Slice)
+ .filter(Slice.slice_name.in_(["Energy Sankey", "Heatmap"]))
+ .all()
+ )
+ main_chart = next(c for c in charts if c.slice_name == "Energy Sankey")
+ ref_chart = next(c for c in charts if c.slice_name == "Heatmap")
+
+ # Set params with a table annotation referencing the other chart
+ original_params = json.loads(main_chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Table Annotation",
+ "sourceType": "table",
+ "value": ref_chart.id,
+ "show": True,
+ "style": "solid",
+ }
+ ]
+ main_chart.params = json.dumps(original_params)
+ db.session.commit()
+
+ command = ExportChartsCommand([main_chart.id])
+ contents = dict(command.run())
+
+ chart_key = f"charts/Energy_Sankey_{main_chart.id}.yaml"
+ chart_yaml = yaml.safe_load(contents[chart_key]())
+ layers = chart_yaml["params"]["annotation_layers"]
+ assert len(layers) == 1
+ assert layers[0]["value"] == str(ref_chart.uuid)
+ assert layers[0]["sourceType"] == "table"
+
+ # The referenced chart should also be exported
+ ref_chart_key = f"charts/Heatmap_{ref_chart.id}.yaml"
+ assert ref_chart_key in contents
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_line_annotation_uuid_replacement(self, mock_g):
+ """Test that line sourceType annotation chart IDs are replaced with
UUIDs."""
+ mock_g.user = security_manager.find_user("admin")
+
+ charts = (
+ db.session.query(Slice)
+ .filter(Slice.slice_name.in_(["Energy Sankey", "Heatmap"]))
+ .all()
+ )
+ main_chart = next(c for c in charts if c.slice_name == "Energy Sankey")
+ ref_chart = next(c for c in charts if c.slice_name == "Heatmap")
+
+ original_params = json.loads(main_chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Line Annotation",
+ "sourceType": "line",
+ "value": ref_chart.id,
+ "show": True,
+ "style": "dashed",
+ }
+ ]
+ main_chart.params = json.dumps(original_params)
+ db.session.commit()
+
+ command = ExportChartsCommand([main_chart.id])
+ contents = dict(command.run())
+
+ chart_key = f"charts/Energy_Sankey_{main_chart.id}.yaml"
+ chart_yaml = yaml.safe_load(contents[chart_key]())
+ layers = chart_yaml["params"]["annotation_layers"]
+ assert len(layers) == 1
+ assert layers[0]["value"] == str(ref_chart.uuid)
+ assert layers[0]["sourceType"] == "line"
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_formula_annotation_unchanged(self, mock_g):
+ """Test that FORMULA annotations are not modified during export."""
+ mock_g.user = security_manager.find_user("admin")
+
+ chart = db.session.query(Slice).filter_by(slice_name="Energy
Sankey").one()
+ original_params = json.loads(chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Formula",
+ "sourceType": "FORMULA",
+ "value": "sin(x)",
+ "show": True,
+ "style": "solid",
+ }
+ ]
+ chart.params = json.dumps(original_params)
+ db.session.commit()
+
+ command = ExportChartsCommand([chart.id])
+ contents = dict(command.run())
+
+ chart_key = f"charts/Energy_Sankey_{chart.id}.yaml"
+ chart_yaml = yaml.safe_load(contents[chart_key]())
+ layers = chart_yaml["params"]["annotation_layers"]
+ assert len(layers) == 1
+ assert layers[0]["value"] == "sin(x)"
+ assert layers[0]["sourceType"] == "FORMULA"
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_mixed_annotation_types(self, mock_g):
+ """Test export with NATIVE, table, and FORMULA annotations together."""
+ mock_g.user = security_manager.find_user("admin")
+
+ ann_layer = AnnotationLayer(name="Mixed Test Layer")
+ db.session.add(ann_layer)
+ db.session.commit()
+
+ charts = (
+ db.session.query(Slice)
+ .filter(Slice.slice_name.in_(["Energy Sankey", "Heatmap"]))
+ .all()
+ )
+ main_chart = next(c for c in charts if c.slice_name == "Energy Sankey")
+ ref_chart = next(c for c in charts if c.slice_name == "Heatmap")
+
+ original_params = json.loads(main_chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Native",
+ "sourceType": "NATIVE",
+ "value": ann_layer.id,
+ "show": True,
+ "style": "solid",
+ },
+ {
+ "name": "Table",
+ "sourceType": "table",
+ "value": ref_chart.id,
+ "show": True,
+ "style": "solid",
+ },
+ {
+ "name": "Formula",
+ "sourceType": "FORMULA",
+ "value": "cos(x)",
+ "show": True,
+ "style": "dashed",
+ },
+ ]
+ main_chart.params = json.dumps(original_params)
Review Comment:
**Suggestion:** The mixed-annotation test commits changes to the shared
chart's `params` and only deletes the newly created annotation layer; it never
restores the chart parameters. This leaves the table, native, and formula
annotations behind for subsequent tests and can also leave the database
contaminated if an assertion fails before cleanup. [state inconsistency]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Later exports can reference a deleted annotation layer.
- ⚠️ Shared chart parameters become contaminated.
- ⚠️ Cleanup failures can leave multiple database records.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run
`TestExportChartsAnnotationLayers.test_export_chart_mixed_annotation_types` at
`tests/integration_tests/charts/commands_tests.py:929`; it selects the
shared `Energy
Sankey` and `Heatmap` charts at lines 937-943 and creates an annotation
layer at lines
933-935.
2. The test replaces `Energy Sankey.params` with native, table, and formula
annotations at
lines 945-969 and commits the mutation at line 970.
3. The export assertions run at lines 972-993, then cleanup only deletes the
newly created
`AnnotationLayer` at lines 995-998; it does not restore `main_chart.params`.
4. Run a later test using `Energy Sankey`, such as
`test_export_chart_no_annotation_layers` at line 1002. The chart can still
reference the
deleted annotation-layer ID and retain the table/formula entries, while
assertion failures
before cleanup leave both the chart and annotation layer persisted.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5cea2bfdebd74d1886ad2d969db082ef&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=5cea2bfdebd74d1886ad2d969db082ef&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/integration_tests/charts/commands_tests.py
**Line:** 945:969
**Comment:**
*State Inconsistency: The mixed-annotation test commits changes to the
shared chart's `params` and only deletes the newly created annotation layer; it
never restores the chart parameters. This leaves the table, native, and formula
annotations behind for subsequent tests and can also leave the database
contaminated if an assertion fails before cleanup.
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%2F39857&comment_hash=dcdc179d3b7859302b10a6f399d7f24ca6bdd19d15717a359b7ff7b53c833778&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39857&comment_hash=dcdc179d3b7859302b10a6f399d7f24ca6bdd19d15717a359b7ff7b53c833778&reaction=dislike'>👎</a>
##########
tests/integration_tests/charts/commands_tests.py:
##########
@@ -761,3 +764,737 @@ def test_fave_unfave_chart_command_non_owner(self,
mock_find_by_id):
finally:
if example_chart.datasource:
self.revoke_role_access_to_table("Gamma",
example_chart.datasource)
+
+
+class TestExportChartsAnnotationLayers(SupersetTestCase):
+ """Tests for annotation layer handling in chart export."""
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_native_annotation_uuid_replacement(self, mock_g):
+ """Test that NATIVE annotation layer IDs are replaced with UUIDs on
export."""
+ mock_g.user = security_manager.find_user("admin")
+
+ # Create an annotation layer
+ ann_layer = AnnotationLayer(name="Test Layer")
+ db.session.add(ann_layer)
+ db.session.commit()
+
+ # Get a chart and set params with a NATIVE annotation referencing the
layer
+ chart = db.session.query(Slice).filter_by(slice_name="Energy
Sankey").one()
+ original_params = json.loads(chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Layer 1",
+ "sourceType": "NATIVE",
+ "value": ann_layer.id,
+ "show": True,
+ "style": "solid",
+ }
+ ]
+ chart.params = json.dumps(original_params)
+ db.session.commit()
+
+ command = ExportChartsCommand([chart.id])
+ contents = dict(command.run())
+
+ chart_key = f"charts/Energy_Sankey_{chart.id}.yaml"
+ chart_yaml = yaml.safe_load(contents[chart_key]())
+ layers = chart_yaml["params"]["annotation_layers"]
+ assert len(layers) == 1
+ assert layers[0]["value"] == str(ann_layer.uuid)
+ assert layers[0]["sourceType"] == "NATIVE"
+
+ # Verify annotation layer YAML is included in export
+ ann_layer_key = [k for k in contents if
k.startswith("annotation_layers/")]
+ assert len(ann_layer_key) == 1
+
+ # Clean up
+ db.session.query(Annotation).filter_by(layer_id=ann_layer.id).delete()
+ db.session.delete(ann_layer)
+ db.session.commit()
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_table_annotation_uuid_replacement(self, mock_g):
+ """Test that table/line annotation chart IDs are replaced with
UUIDs."""
+ mock_g.user = security_manager.find_user("admin")
+
+ # Get two charts: one main, one referenced as annotation source
+ charts = (
+ db.session.query(Slice)
+ .filter(Slice.slice_name.in_(["Energy Sankey", "Heatmap"]))
+ .all()
+ )
+ main_chart = next(c for c in charts if c.slice_name == "Energy Sankey")
+ ref_chart = next(c for c in charts if c.slice_name == "Heatmap")
+
+ # Set params with a table annotation referencing the other chart
+ original_params = json.loads(main_chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Table Annotation",
+ "sourceType": "table",
+ "value": ref_chart.id,
+ "show": True,
+ "style": "solid",
+ }
+ ]
+ main_chart.params = json.dumps(original_params)
+ db.session.commit()
+
+ command = ExportChartsCommand([main_chart.id])
+ contents = dict(command.run())
+
+ chart_key = f"charts/Energy_Sankey_{main_chart.id}.yaml"
+ chart_yaml = yaml.safe_load(contents[chart_key]())
+ layers = chart_yaml["params"]["annotation_layers"]
+ assert len(layers) == 1
+ assert layers[0]["value"] == str(ref_chart.uuid)
+ assert layers[0]["sourceType"] == "table"
+
+ # The referenced chart should also be exported
+ ref_chart_key = f"charts/Heatmap_{ref_chart.id}.yaml"
+ assert ref_chart_key in contents
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_line_annotation_uuid_replacement(self, mock_g):
+ """Test that line sourceType annotation chart IDs are replaced with
UUIDs."""
+ mock_g.user = security_manager.find_user("admin")
+
+ charts = (
+ db.session.query(Slice)
+ .filter(Slice.slice_name.in_(["Energy Sankey", "Heatmap"]))
+ .all()
+ )
+ main_chart = next(c for c in charts if c.slice_name == "Energy Sankey")
+ ref_chart = next(c for c in charts if c.slice_name == "Heatmap")
+
+ original_params = json.loads(main_chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Line Annotation",
+ "sourceType": "line",
+ "value": ref_chart.id,
+ "show": True,
+ "style": "dashed",
+ }
+ ]
+ main_chart.params = json.dumps(original_params)
+ db.session.commit()
+
+ command = ExportChartsCommand([main_chart.id])
+ contents = dict(command.run())
+
+ chart_key = f"charts/Energy_Sankey_{main_chart.id}.yaml"
+ chart_yaml = yaml.safe_load(contents[chart_key]())
+ layers = chart_yaml["params"]["annotation_layers"]
+ assert len(layers) == 1
+ assert layers[0]["value"] == str(ref_chart.uuid)
+ assert layers[0]["sourceType"] == "line"
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_formula_annotation_unchanged(self, mock_g):
+ """Test that FORMULA annotations are not modified during export."""
+ mock_g.user = security_manager.find_user("admin")
+
+ chart = db.session.query(Slice).filter_by(slice_name="Energy
Sankey").one()
+ original_params = json.loads(chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Formula",
+ "sourceType": "FORMULA",
+ "value": "sin(x)",
+ "show": True,
+ "style": "solid",
+ }
+ ]
+ chart.params = json.dumps(original_params)
+ db.session.commit()
+
+ command = ExportChartsCommand([chart.id])
+ contents = dict(command.run())
+
+ chart_key = f"charts/Energy_Sankey_{chart.id}.yaml"
+ chart_yaml = yaml.safe_load(contents[chart_key]())
+ layers = chart_yaml["params"]["annotation_layers"]
+ assert len(layers) == 1
+ assert layers[0]["value"] == "sin(x)"
+ assert layers[0]["sourceType"] == "FORMULA"
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_mixed_annotation_types(self, mock_g):
+ """Test export with NATIVE, table, and FORMULA annotations together."""
+ mock_g.user = security_manager.find_user("admin")
+
+ ann_layer = AnnotationLayer(name="Mixed Test Layer")
+ db.session.add(ann_layer)
+ db.session.commit()
+
+ charts = (
+ db.session.query(Slice)
+ .filter(Slice.slice_name.in_(["Energy Sankey", "Heatmap"]))
+ .all()
+ )
+ main_chart = next(c for c in charts if c.slice_name == "Energy Sankey")
+ ref_chart = next(c for c in charts if c.slice_name == "Heatmap")
+
+ original_params = json.loads(main_chart.params or "{}")
+ original_params["annotation_layers"] = [
+ {
+ "name": "Native",
+ "sourceType": "NATIVE",
+ "value": ann_layer.id,
+ "show": True,
+ "style": "solid",
+ },
+ {
+ "name": "Table",
+ "sourceType": "table",
+ "value": ref_chart.id,
+ "show": True,
+ "style": "solid",
+ },
+ {
+ "name": "Formula",
+ "sourceType": "FORMULA",
+ "value": "cos(x)",
+ "show": True,
+ "style": "dashed",
+ },
+ ]
+ main_chart.params = json.dumps(original_params)
+ db.session.commit()
+
+ command = ExportChartsCommand([main_chart.id])
+ contents = dict(command.run())
+
+ chart_key = f"charts/Energy_Sankey_{main_chart.id}.yaml"
+ chart_yaml = yaml.safe_load(contents[chart_key]())
+ layers = chart_yaml["params"]["annotation_layers"]
+ assert len(layers) == 3
+
+ # NATIVE → UUID of annotation layer
+ assert layers[0]["value"] == str(ann_layer.uuid)
+ # table → UUID of referenced chart
+ assert layers[1]["value"] == str(ref_chart.uuid)
+ # FORMULA → unchanged string
+ assert layers[2]["value"] == "cos(x)"
+
+ # Annotation layer YAML should be in the export
+ ann_keys = [k for k in contents if k.startswith("annotation_layers/")]
+ assert len(ann_keys) == 1
+
+ # Referenced chart should be in the export
+ ref_chart_key = f"charts/Heatmap_{ref_chart.id}.yaml"
+ assert ref_chart_key in contents
+
+ # Clean up
+ db.session.query(Annotation).filter_by(layer_id=ann_layer.id).delete()
+ db.session.delete(ann_layer)
+ db.session.commit()
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_no_annotation_layers(self, mock_g):
+ """Test that charts without annotation_layers export normally."""
+ mock_g.user = security_manager.find_user("admin")
+
+ chart = db.session.query(Slice).filter_by(slice_name="Energy
Sankey").one()
+ # Ensure no annotation_layers in params
+ original_params = json.loads(chart.params or "{}")
+ original_params.pop("annotation_layers", None)
+ chart.params = json.dumps(original_params)
+ db.session.commit()
+
+ command = ExportChartsCommand([chart.id])
+ contents = dict(command.run())
+
+ chart_key = f"charts/Energy_Sankey_{chart.id}.yaml"
+ chart_yaml = yaml.safe_load(contents[chart_key]())
+ # No annotation_layers key or empty list — either is fine
+ ann_layers = chart_yaml.get("params", {}).get("annotation_layers", [])
+ assert ann_layers == [] or "annotation_layers" not in chart_yaml.get(
+ "params", {}
+ )
+
+ # No annotation_layers/ files in export
+ ann_keys = [k for k in contents if k.startswith("annotation_layers/")]
+ assert len(ann_keys) == 0
+
+ @patch("superset.security.manager.g")
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_export_chart_query_context_annotation_uuid_replacement(self,
mock_g):
+ """Test that annotation IDs in query_context are replaced with
UUIDs."""
+ mock_g.user = security_manager.find_user("admin")
+
+ ann_layer = AnnotationLayer(name="QC Test Layer")
+ db.session.add(ann_layer)
+ db.session.commit()
+
+ charts = (
+ db.session.query(Slice)
+ .filter(Slice.slice_name.in_(["Energy Sankey", "Heatmap"]))
+ .all()
+ )
+ main_chart = next(c for c in charts if c.slice_name == "Energy Sankey")
+ ref_chart = next(c for c in charts if c.slice_name == "Heatmap")
+
+ annotations = [
+ {
+ "name": "Native",
+ "sourceType": "NATIVE",
+ "value": ann_layer.id,
+ "show": True,
+ },
+ {
+ "name": "Table",
+ "sourceType": "table",
+ "value": ref_chart.id,
+ "show": True,
+ },
+ ]
+
+ original_params = json.loads(main_chart.params or "{}")
+ original_params["annotation_layers"] = deepcopy(annotations)
+ main_chart.params = json.dumps(original_params)
+
+ query_context = {
+ "datasource": {"id": 1, "type": "table"},
+ "queries": [{"annotation_layers": deepcopy(annotations)}],
+ "form_data": {"annotation_layers": deepcopy(annotations)},
+ }
+ main_chart.query_context = json.dumps(query_context)
+ db.session.commit()
Review Comment:
**Suggestion:** The query-context test persists both modified `params` and
`query_context` on the shared `Energy Sankey` chart, but its cleanup only
removes the newly created annotation layer. Neither field is restored, so later
tests can consume stale query-context annotation IDs and chart parameters.
[state inconsistency]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Later chart exports can contain stale query-context annotations.
- ⚠️ Query execution tests may use contaminated chart metadata.
- ⚠️ Shared integration-test state becomes order-dependent.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run
`TestExportChartsAnnotationLayers.test_export_chart_query_context_annotation_uuid_replacement`
at `tests/integration_tests/charts/commands_tests.py:1030`; it loads `Energy
Sankey` and
`Heatmap` at lines 1038-1044.
2. The test modifies `main_chart.params` at lines 1061-1063 and assigns
annotation layers
under both `queries` and `form_data` in `query_context` at lines 1065-1069.
3. Both fields are committed at line 1071, and the export assertions inspect
them at lines
1073-1088.
4. Cleanup at lines 1090-1093 deletes only the newly created annotation
layer. It does not
restore either field, so later tests reading or exporting `Energy Sankey`
can consume
stale query-context IDs; failures before cleanup leave the changes committed.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=55fb52e5ee7e46f2aa04fc0c13576e0f&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=55fb52e5ee7e46f2aa04fc0c13576e0f&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/integration_tests/charts/commands_tests.py
**Line:** 1061:1071
**Comment:**
*State Inconsistency: The query-context test persists both modified
`params` and `query_context` on the shared `Energy Sankey` chart, but its
cleanup only removes the newly created annotation layer. Neither field is
restored, so later tests can consume stale query-context annotation IDs and
chart parameters.
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%2F39857&comment_hash=151d901ea5ab9b00cff28919bc356ba4eb1cc17ca0bb56d52aeef84b5a2419dd&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39857&comment_hash=151d901ea5ab9b00cff28919bc356ba4eb1cc17ca0bb56d52aeef84b5a2419dd&reaction=dislike'>👎</a>
##########
superset/commands/chart/importers/v1/__init__.py:
##########
@@ -44,12 +51,33 @@ class ImportChartsCommand(ImportModelsCommand):
model_name = "chart"
prefix = "charts/"
schemas: dict[str, Schema] = {
+ "annotation_layers/": ImportV1AnnotationLayerSchema(),
"charts/": ImportV1ChartSchema(),
"datasets/": ImportV1DatasetSchema(),
"databases/": ImportV1DatabaseSchema(),
}
import_error = ChartImportError
+ @staticmethod
+ def _import_chart_with_tags(
+ config: dict[str, Any],
+ overwrite: bool,
+ contents: dict[str, Any],
+ imported_chart_ids: dict[str, int],
+ annotation_layer_ids: dict[str, int] | None = None,
+ ) -> None:
+ chart = import_chart(
+ config,
+ overwrite=overwrite,
+ annotation_layer_ids=annotation_layer_ids,
+ chart_ids=imported_chart_ids,
+ )
+ imported_chart_ids[str(chart.uuid)] = chart.id
+
+ if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
+ if "tags" in config:
+ import_tag(config["tags"], contents, chart.id, "chart",
db.session)
Review Comment:
**Suggestion:** `feature_flag_manager` is referenced without being imported
or defined in this module. Any chart import that reaches the tagging block will
raise `NameError` after the chart is imported, causing the import command to
fail. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Chart and dashboard imports can fail after chart creation.
- ❌ Import completion is blocked when the tagging check executes.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start a Superset instance with the PR code and invoke the existing chart
import flow
through `ImportChartsCommand._import()` at
`superset/commands/chart/importers/v1/__init__.py:83`, such as dashboard
import through
`superset/dashboards/api.py:2219`.
2. Include a valid exported chart configuration so
`_import_chart_with_tags()` at
`superset/commands/chart/importers/v1/__init__.py:62` is called.
3. After `import_chart()` returns at line 69, execution reaches
`feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM")` at line 77.
4. `feature_flag_manager` is not imported or defined in
`superset/commands/chart/importers/v1/__init__.py`, so Python raises
`NameError` and the
chart import command fails before completing its tagging handling.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2f3c02176d454f23817f4cd26f4e2d4b&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=2f3c02176d454f23817f4cd26f4e2d4b&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/chart/importers/v1/__init__.py
**Line:** 77:79
**Comment:**
*Possible Bug: `feature_flag_manager` is referenced without being
imported or defined in this module. Any chart import that reaches the tagging
block will raise `NameError` after the chart is imported, causing the import
command to fail.
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%2F39857&comment_hash=cfd5cff734ae07203ee792e9b361f544b2c69a85fbe5231615d66714c7e1f597&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39857&comment_hash=cfd5cff734ae07203ee792e9b361f544b2c69a85fbe5231615d66714c7e1f597&reaction=dislike'>👎</a>
##########
superset/commands/chart/importers/v1/__init__.py:
##########
@@ -58,11 +86,18 @@ def _import(
contents: dict[str, Any] | None = None,
) -> None:
contents = {} if contents is None else contents
- # discover datasets associated with charts
+ # discover datasets and referenced chart UUIDs from chart configs
dataset_uuids: set[str] = set()
+ referenced_chart_uuids: set[str] = set()
for file_name, config in configs.items():
if file_name.startswith("charts/"):
dataset_uuids.add(config["dataset_uuid"])
+ for annotation in config.get("params",
{}).get("annotation_layers", []):
+ if annotation.get("sourceType") in (
+ "table",
+ "line",
+ ) and isinstance(annotation.get("value"), str):
+ referenced_chart_uuids.add(annotation["value"])
Review Comment:
**Suggestion:** Dependency discovery only scans `params.annotation_layers`,
while `_resolve_query_context_annotations` also resolves annotation references
inside `query_context`. Charts referenced only from query context will not be
placed in the dependency-first order, so newly imported charts may be processed
before their referenced charts and their query-context references can remain
unresolved. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Query-context chart annotations can remain unresolved after import.
- ⚠️ Charts using query-context annotations may lose annotation behavior.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Export charts through `ExportChartsCommand` at
`superset/commands/chart/export.py:46`
where a chart's annotation configuration includes a chart reference inside
serialized
`query_context.annotation_layers`, but not inside `params.annotation_layers`.
2. Import the export through `ImportChartsCommand._import()` at
`superset/commands/chart/importers/v1/__init__.py:83` with the dependent
chart listed
before its referenced chart in `configs`.
3. The dependency scan at lines 92-100 examines only `config.get("params",
{}).get("annotation_layers", [])`, so it does not add the query-context
chart UUID to
`referenced_chart_uuids`.
4. The dependent chart is therefore placed in `dependent_chart_configs` and
can be passed
to `_import_chart_with_tags()` at line 163 before the referenced chart has
been added to
`imported_chart_ids`; `_resolve_query_context_annotations()` at
`superset/commands/chart/importers/v1/utils.py:145` cannot resolve that UUID
at this point
and leaves the reference unresolved.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ee0305cfc12f41e9bb8e3602d94b58e1&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=ee0305cfc12f41e9bb8e3602d94b58e1&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/chart/importers/v1/__init__.py
**Line:** 92:100
**Comment:**
*Logic Error: Dependency discovery only scans
`params.annotation_layers`, while `_resolve_query_context_annotations` also
resolves annotation references inside `query_context`. Charts referenced only
from query context will not be placed in the dependency-first order, so newly
imported charts may be processed before their referenced charts and their
query-context references can remain unresolved.
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%2F39857&comment_hash=96cf58f3cb98898f60944e23d9a5758db71bf60c708e384fd415ffb2df4e3b7c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39857&comment_hash=96cf58f3cb98898f60944e23d9a5758db71bf60c708e384fd415ffb2df4e3b7c&reaction=dislike'>👎</a>
##########
superset/commands/chart/importers/v1/__init__.py:
##########
@@ -88,6 +123,16 @@ def _import(
dataset = import_dataset(config, overwrite=False)
datasets[str(dataset.uuid)] = dataset
+ # import annotation layers before charts so UUID→ID maps are ready
+ annotation_layer_ids: dict[str, int] = {}
+ for file_name, config in configs.items():
+ if file_name.startswith("annotation_layers/"):
+ layer = import_annotation_layer(config, overwrite=False)
Review Comment:
**Suggestion:** The annotation-layer import ignores the command's
`overwrite` value by always passing `overwrite=False`. Re-importing an export
with overwrite enabled will therefore retain stale annotation layers and
annotations instead of updating them. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Re-imported dashboards retain stale annotation-layer configuration.
- ⚠️ Chart contextual annotations may differ from exported source state.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Export a chart or dashboard containing an annotation layer through
`ExportChartsCommand`, which is called by `superset/dashboards/api.py:1449`
and includes
an `annotation_layers/` configuration.
2. Change the annotation layer's event, interval, or SQL configuration in
the source
system, then import the export through the existing dashboard import entry
point
`superset/dashboards/api.py:2219` with `overwrite=True`.
3. `ImportChartsCommand._import()` at
`superset/commands/chart/importers/v1/__init__.py:83` reaches the
annotation-layer loop at
lines 128-131 and calls `import_annotation_layer(config, overwrite=False)`.
4. The annotation-layer importer at
`superset/commands/annotation_layer/importers/v1/utils.py:25` therefore uses
non-overwrite
semantics, so an existing layer with the same UUID is retained instead of
receiving the
updated exported configuration; the chart's resolved annotation reference
points to stale
data.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f27ec413eefc4a62bc2411ead118e0f8&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=f27ec413eefc4a62bc2411ead118e0f8&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/chart/importers/v1/__init__.py
**Line:** 128:130
**Comment:**
*Logic Error: The annotation-layer import ignores the command's
`overwrite` value by always passing `overwrite=False`. Re-importing an export
with overwrite enabled will therefore retain stale annotation layers and
annotations instead of updating them.
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%2F39857&comment_hash=b9e5bbb6faef2f7b58758e8f1f2181c207f56abb9c724f4b8b9ec5ba5414b183&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39857&comment_hash=b9e5bbb6faef2f7b58758e8f1f2181c207f56abb9c724f4b8b9ec5ba5414b183&reaction=dislike'>👎</a>
##########
superset/commands/chart/importers/v1/utils.py:
##########
@@ -24,24 +25,148 @@
from superset.commands.importers.v1.utils import find_existing_for_import
from superset.migrations.shared.migrate_viz import processors
from superset.migrations.shared.migrate_viz.base import MigrateViz
+from superset.models.annotations import AnnotationLayer
from superset.models.slice import Slice
from superset.utils import json
from superset.utils.core import AnnotationType, get_user
+logger = logging.getLogger(__name__)
-def filter_chart_annotations(chart_config: dict[str, Any]) -> None:
+
+def topological_sort_charts(
+ chart_configs: list[dict[str, Any]],
+) -> list[dict[str, Any]]:
+ """Sort charts so that annotation dependencies are imported first.
+
+ Handles multi-level dependencies (A→B→C) by iteratively resolving
+ charts whose in-batch dependencies are already satisfied.
"""
- Mutating the chart's config params to keep only the annotations of
- type FORMULA.
- TODO:
- handle annotation dependencies on either other charts or
- annotation layers objects.
+ if len(chart_configs) <= 1:
+ return chart_configs
+
+ batch_uuids = {c["uuid"] for c in chart_configs}
+ sorted_refs: list[dict[str, Any]] = []
+ remaining = list(chart_configs)
+ resolved: set[str] = set()
+ while remaining:
+ next_remaining = []
+ for c in remaining:
+ unmet = {
+ ann["value"]
+ for ann in c.get("params", {}).get("annotation_layers", [])
+ if ann.get("sourceType") in ("table", "line")
+ and isinstance(ann.get("value"), str)
+ and ann["value"] in batch_uuids - resolved
+ }
+ if not unmet:
+ sorted_refs.append(c)
+ resolved.add(c["uuid"])
+ else:
+ next_remaining.append(c)
+ if len(next_remaining) == len(remaining):
+ logger.warning(
+ "Circular annotation dependency detected for charts: %s — "
+ "these charts may have unresolved annotation references after
import.",
+ [c["uuid"] for c in next_remaining],
+ )
+ sorted_refs.extend(next_remaining)
+ break
+ remaining = next_remaining
+ return sorted_refs
+
+
+def _resolve_uuid_to_id(
+ uuid_value: str,
+ id_map: dict[str, int] | None,
+ model: type,
+) -> int | None:
+ """Resolve a UUID to a local integer ID using a map or DB fallback."""
+ if id_map and uuid_value in id_map:
+ return id_map[uuid_value]
+ obj = db.session.query(model).filter_by(uuid=uuid_value).first()
+ return obj.id if obj else None
+
+
+def filter_chart_annotations(
+ chart_config: dict[str, Any],
+ annotation_layer_ids: dict[str, int] | None = None,
+ chart_ids: dict[str, int] | None = None,
+) -> None:
+ """
+ Resolve annotation references from exported UUIDs to local integer IDs.
+ - FORMULA: kept unchanged (no DB reference)
+ - NATIVE: UUID resolved to AnnotationLayer.id
+ - table/line: UUID resolved to referenced Chart.id
+ Annotations whose references cannot be resolved are dropped.
"""
params = chart_config.get("params", {})
- als = params.get("annotation_layers", [])
- params["annotation_layers"] = [
- al for al in als if al.get("annotationType") == AnnotationType.FORMULA
- ]
+ annotation_layers = params.get("annotation_layers", [])
+ resolved_annotations: list[dict[str, Any]] = []
+ for annotation in annotation_layers:
+ source_type = annotation.get("sourceType")
+ value = annotation.get("value")
+
+ if annotation.get("annotationType") == AnnotationType.FORMULA:
+ resolved_annotations.append(annotation)
+ elif source_type == "NATIVE" and isinstance(value, str):
+ layer_id = _resolve_uuid_to_id(value, annotation_layer_ids,
AnnotationLayer)
+ if layer_id is not None:
+ annotation["value"] = layer_id
+ resolved_annotations.append(annotation)
+ elif source_type in ("table", "line") and isinstance(value, str):
+ ref_chart_id = _resolve_uuid_to_id(value, chart_ids, Slice)
+ if ref_chart_id is not None:
+ annotation["value"] = ref_chart_id
+ resolved_annotations.append(annotation)
+ params["annotation_layers"] = resolved_annotations
+
+
+def _resolve_annotation_list(
+ annotations: list[dict[str, Any]],
+ annotation_layer_ids: dict[str, int] | None,
+ chart_ids: dict[str, int] | None,
+) -> None:
+ """Resolve UUID values to integer IDs in-place for an annotation list."""
+ for annotation in annotations:
+ source_type = annotation.get("sourceType")
+ value = annotation.get("value")
+ if not isinstance(value, str):
+ continue
+ if source_type == "NATIVE":
+ layer_id = _resolve_uuid_to_id(value, annotation_layer_ids,
AnnotationLayer)
+ if layer_id is not None:
+ annotation["value"] = layer_id
+ elif source_type in ("table", "line"):
+ ref_chart_id = _resolve_uuid_to_id(value, chart_ids, Slice)
+ if ref_chart_id is not None:
+ annotation["value"] = ref_chart_id
Review Comment:
**Suggestion:** Unresolved query-context annotation references are left
unchanged. When a referenced layer or chart is not present in the destination,
its exported UUID remains in a field that expects a local integer ID, causing
the imported chart to use an invalid annotation reference instead of dropping
or otherwise handling it consistently with `filter_chart_annotations`.
[possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Imported query contexts retain unusable annotation references.
- ⚠️ Queries using missing chart or layer annotations may fail or render
incorrectly.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Import a chart export through `ImportChartsCommand._import()` at
`superset/commands/chart/importers/v1/__init__.py:83` where `query_context`
contains an
annotation-layer reference and the corresponding annotation layer or chart
is absent from
the import package and destination database.
2. `import_chart()` at `superset/commands/chart/importers/v1/utils.py:261`
calls
`_resolve_query_context_annotations()` at line 325.
3. `_resolve_annotation_list()` at lines 131-142 attempts to resolve the
UUID through
`_resolve_uuid_to_id()` at line 140; that lookup returns `None` when the
referenced
`Slice` is unavailable.
4. Because there is no `else` branch removing the annotation, its exported
UUID remains in
`query_context`, which is serialized at line 167 and persisted with the
chart even though
downstream annotation handling expects a local integer ID.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8fff8cb8224c4e40be6bddffa84a2f86&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=8fff8cb8224c4e40be6bddffa84a2f86&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/chart/importers/v1/utils.py
**Line:** 131:142
**Comment:**
*Possible Bug: Unresolved query-context annotation references are left
unchanged. When a referenced layer or chart is not present in the destination,
its exported UUID remains in a field that expects a local integer ID, causing
the imported chart to use an invalid annotation reference instead of dropping
or otherwise handling it consistently with `filter_chart_annotations`.
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%2F39857&comment_hash=1b3b8d5791aa36eba5a47a6dad0381de7d691312eb98c6ad05234ee2e743fbb6&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39857&comment_hash=1b3b8d5791aa36eba5a47a6dad0381de7d691312eb98c6ad05234ee2e743fbb6&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]
