codeant-ai-for-open-source[bot] commented on code in PR #40129:
URL: https://github.com/apache/superset/pull/40129#discussion_r3397904594
##########
superset/charts/filters.py:
##########
@@ -180,3 +181,40 @@ def apply(self, query: Query, value: Any) -> Query:
FavStar.user_id == get_user_id(),
)
)
+
+
+class ChartDeletedStateFilter( # pylint: disable=too-few-public-methods
+ BaseDeletedStateFilter
+):
+ """Rison filter for the GET list that exposes soft-deleted charts.
+
+ Soft-deleted rows are additionally scoped to the **restore audience**: only
+ the chart's owners (or admins) may enumerate them. This mirrors
+ ``RestoreChartCommand``'s ``raise_for_ownership`` check, so a read-access
+ non-owner (who can see the chart via datasource access) cannot list
+ soft-deleted charts they could never restore. Live rows are unaffected —
+ they keep their normal ``ChartFilter`` visibility. The ownership scoping is
+ part of the cross-entity deleted-state contract: only the restore audience
+ may enumerate soft-deleted rows (kept consistent with the deleted-state
+ filters in the dashboard and dataset soft-delete rollouts).
+ """
+
+ arg_name = "chart_deleted_state"
+ model = Slice
+
+ def apply(self, query: Query, value: Any) -> Query:
+ query = super().apply(query, value)
+ normalized = str(value).lower().strip() if value is not None else ""
+ if normalized not in {"include", "only"} or
security_manager.is_admin():
+ return query
+
+ # Non-admins may only see soft-deleted charts they own. ``any()`` emits
+ # an EXISTS subquery so it composes with the base access filter without
+ # producing duplicate rows from a join.
+ owned = Slice.owners.any(security_manager.user_model.id ==
get_user_id())
+ if normalized == "only":
+ # ``super().apply`` already restricted to ``deleted_at IS NOT
NULL``.
+ return query.filter(owned)
+ # ``include``: keep all live rows (normal access) and add only the
+ # soft-deleted rows this user owns.
+ return query.filter(or_(Slice.deleted_at.is_(None), owned))
Review Comment:
**Suggestion:** Add a method docstring to this newly introduced function so
the new API behavior is documented inline, consistent with the requirement that
new functions/classes include docstrings. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly introduced Python method in the added class, and it has no
docstring. The custom rule requires new Python functions and classes to include
docstrings, so the suggestion correctly identifies a real violation.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=71d2dbe2dce24768bdef0f3add70feda&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=71d2dbe2dce24768bdef0f3add70feda&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/charts/filters.py
**Line:** 205:220
**Comment:**
*Custom Rule: Add a method docstring to this newly introduced function
so the new API behavior is documented inline, consistent with the requirement
that new functions/classes include docstrings.
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%2F40129&comment_hash=f7f56580c432bfcf25f228a5fbbf0888eccd481fd283a251b99de4a2d100335a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40129&comment_hash=f7f56580c432bfcf25f228a5fbbf0888eccd481fd283a251b99de4a2d100335a&reaction=dislike'>👎</a>
##########
superset/migrations/versions/2026-05-08_12-00_7c4a8d09ca37_add_deleted_at_to_slices.py:
##########
@@ -0,0 +1,52 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Add deleted_at column and index to slices for soft-delete.
+
+Adds a nullable ``deleted_at`` column and an index on it to the
+``slices`` table to support soft deletion of charts. Companion to
+the ``SoftDeleteMixin`` infrastructure shipped in PR #39977.
+
+Revision ID: 7c4a8d09ca37
+Revises: 31dae2559c05
+Create Date: 2026-05-08 12:00:00.000000
+"""
+
+from sqlalchemy import Column, DateTime
+
+from superset.migrations.shared.utils import (
+ add_columns,
+ create_index,
+ drop_columns,
+ drop_index,
+)
+
+# revision identifiers, used by Alembic.
+revision = "7c4a8d09ca37"
+down_revision = "31dae2559c05"
+
+TABLE_NAME = "slices"
+INDEX_NAME = f"ix_{TABLE_NAME}_deleted_at"
+
+
+def upgrade() -> None:
+ add_columns(TABLE_NAME, Column("deleted_at", DateTime(), nullable=True))
+ create_index(TABLE_NAME, INDEX_NAME, ["deleted_at"])
Review Comment:
**Suggestion:** Add an inline docstring to this new function describing the
migration step it applies so the function is self-documented per the project
rule. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The function is newly added in this file and has no docstring. The custom
rule requires newly added Python functions and classes to include docstrings,
so this is a real violation.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=47b5fd8e0ef6414aa91f3fd1de1b983b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=47b5fd8e0ef6414aa91f3fd1de1b983b&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/migrations/versions/2026-05-08_12-00_7c4a8d09ca37_add_deleted_at_to_slices.py
**Line:** 45:47
**Comment:**
*Custom Rule: Add an inline docstring to this new function describing
the migration step it applies so the function is self-documented per the
project 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%2F40129&comment_hash=720a13f73499e78b1363b916137c388ac1054440683b2912f1c354e919a2410a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40129&comment_hash=720a13f73499e78b1363b916137c388ac1054440683b2912f1c354e919a2410a&reaction=dislike'>👎</a>
##########
tests/unit_tests/migrations/test_add_deleted_at_to_slices.py:
##########
@@ -0,0 +1,186 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Tests for migration ``7c4a8d09ca37_add_deleted_at_to_slices``.
+
+Runs the migration's ``upgrade()`` and ``downgrade()`` against an
+in-memory SQLite engine with a real Alembic ``Operations`` context.
+The behaviour being pinned is the operator-facing contract documented
+in ``UPDATING.md``: ``downgrade()`` reverses the schema but does not
+hard-delete or otherwise mutate rows that were soft-deleted before
+the migration was reversed — those rows survive the downgrade and
+become visible to any code path that no longer applies the
+soft-delete visibility filter.
+"""
+
+from __future__ import annotations
+
+from datetime import datetime
+from importlib import import_module
+
+import pytest
+from alembic.migration import MigrationContext
+from alembic.operations import Operations
+from sqlalchemy import (
+ Column,
+ create_engine,
+ insert,
+ inspect,
+ Integer,
+ MetaData,
+ select,
+ String,
+ Table,
+)
+from sqlalchemy.engine import Engine
+
+migration = import_module(
+ "superset.migrations.versions."
+ "2026-05-08_12-00_7c4a8d09ca37_add_deleted_at_to_slices"
+)
+
+TABLE_NAME = migration.TABLE_NAME # "slices"
+INDEX_NAME = migration.INDEX_NAME # "ix_slices_deleted_at"
+
+
[email protected]
+def engine() -> Engine:
+ """In-memory SQLite seeded with a minimal pre-migration ``slices`` table.
+
+ The real ``slices`` table has many columns; the migration only touches
+ ``deleted_at`` and its index, so only the columns that participate in
+ the test are seeded.
+ """
+ engine = create_engine("sqlite:///:memory:")
+ md = MetaData()
+ Table(
+ TABLE_NAME,
+ md,
+ Column("id", Integer, primary_key=True),
+ Column("slice_name", String(250), nullable=False),
+ )
+ md.create_all(engine)
+ return engine
+
+
+def _columns(engine: Engine) -> set[str]:
+ return {col["name"] for col in inspect(engine).get_columns(TABLE_NAME)}
+
+
+def _indexes(engine: Engine) -> set[str]:
Review Comment:
**Suggestion:** Add a concise docstring to this newly added helper function
describing that it returns the index names for the target table. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly added helper function has no docstring. Since the rule requires
newly added Python functions to include docstrings, the suggestion identifies a
genuine violation.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d2690bcdebe14b8a8ee2c0c220dfe242&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d2690bcdebe14b8a8ee2c0c220dfe242&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/migrations/test_add_deleted_at_to_slices.py
**Line:** 83:83
**Comment:**
*Custom Rule: Add a concise docstring to this newly added helper
function describing that it returns the index names for the target table.
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%2F40129&comment_hash=4de8450bf70e944de5ee1695973bc1fc0180ed1402e682c7c1bb793aa7873613&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40129&comment_hash=4de8450bf70e944de5ee1695973bc1fc0180ed1402e682c7c1bb793aa7873613&reaction=dislike'>👎</a>
##########
tests/unit_tests/migrations/test_add_deleted_at_to_slices.py:
##########
@@ -0,0 +1,186 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Tests for migration ``7c4a8d09ca37_add_deleted_at_to_slices``.
+
+Runs the migration's ``upgrade()`` and ``downgrade()`` against an
+in-memory SQLite engine with a real Alembic ``Operations`` context.
+The behaviour being pinned is the operator-facing contract documented
+in ``UPDATING.md``: ``downgrade()`` reverses the schema but does not
+hard-delete or otherwise mutate rows that were soft-deleted before
+the migration was reversed — those rows survive the downgrade and
+become visible to any code path that no longer applies the
+soft-delete visibility filter.
+"""
+
+from __future__ import annotations
+
+from datetime import datetime
+from importlib import import_module
+
+import pytest
+from alembic.migration import MigrationContext
+from alembic.operations import Operations
+from sqlalchemy import (
+ Column,
+ create_engine,
+ insert,
+ inspect,
+ Integer,
+ MetaData,
+ select,
+ String,
+ Table,
+)
+from sqlalchemy.engine import Engine
+
+migration = import_module(
+ "superset.migrations.versions."
+ "2026-05-08_12-00_7c4a8d09ca37_add_deleted_at_to_slices"
+)
+
+TABLE_NAME = migration.TABLE_NAME # "slices"
+INDEX_NAME = migration.INDEX_NAME # "ix_slices_deleted_at"
+
+
[email protected]
+def engine() -> Engine:
+ """In-memory SQLite seeded with a minimal pre-migration ``slices`` table.
+
+ The real ``slices`` table has many columns; the migration only touches
+ ``deleted_at`` and its index, so only the columns that participate in
+ the test are seeded.
+ """
+ engine = create_engine("sqlite:///:memory:")
+ md = MetaData()
+ Table(
+ TABLE_NAME,
+ md,
+ Column("id", Integer, primary_key=True),
+ Column("slice_name", String(250), nullable=False),
+ )
+ md.create_all(engine)
+ return engine
+
+
+def _columns(engine: Engine) -> set[str]:
+ return {col["name"] for col in inspect(engine).get_columns(TABLE_NAME)}
+
+
+def _indexes(engine: Engine) -> set[str]:
+ return {ix["name"] for ix in inspect(engine).get_indexes(TABLE_NAME)}
+
+
+def test_upgrade_adds_deleted_at_column_and_index(engine: Engine) -> None:
Review Comment:
**Suggestion:** Add a docstring to this new test function to describe the
upgrade behavior it verifies. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added test function and it does not have a docstring in the
final file state. That violates the rule requiring new functions to be
documented inline.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=fb4ae302ed744d289c415f1573e1aea1&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=fb4ae302ed744d289c415f1573e1aea1&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/migrations/test_add_deleted_at_to_slices.py
**Line:** 87:87
**Comment:**
*Custom Rule: Add a docstring to this new test function to describe the
upgrade behavior it verifies.
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%2F40129&comment_hash=28f41eedc3446a60b5ec4dec3692a2e2f6980b04831cf3a4eb0b4e8869398fff&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40129&comment_hash=28f41eedc3446a60b5ec4dec3692a2e2f6980b04831cf3a4eb0b4e8869398fff&reaction=dislike'>👎</a>
##########
tests/unit_tests/migrations/test_add_deleted_at_to_slices.py:
##########
@@ -0,0 +1,186 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Tests for migration ``7c4a8d09ca37_add_deleted_at_to_slices``.
+
+Runs the migration's ``upgrade()`` and ``downgrade()`` against an
+in-memory SQLite engine with a real Alembic ``Operations`` context.
+The behaviour being pinned is the operator-facing contract documented
+in ``UPDATING.md``: ``downgrade()`` reverses the schema but does not
+hard-delete or otherwise mutate rows that were soft-deleted before
+the migration was reversed — those rows survive the downgrade and
+become visible to any code path that no longer applies the
+soft-delete visibility filter.
+"""
+
+from __future__ import annotations
+
+from datetime import datetime
+from importlib import import_module
+
+import pytest
+from alembic.migration import MigrationContext
+from alembic.operations import Operations
+from sqlalchemy import (
+ Column,
+ create_engine,
+ insert,
+ inspect,
+ Integer,
+ MetaData,
+ select,
+ String,
+ Table,
+)
+from sqlalchemy.engine import Engine
+
+migration = import_module(
+ "superset.migrations.versions."
+ "2026-05-08_12-00_7c4a8d09ca37_add_deleted_at_to_slices"
+)
+
+TABLE_NAME = migration.TABLE_NAME # "slices"
+INDEX_NAME = migration.INDEX_NAME # "ix_slices_deleted_at"
+
+
[email protected]
+def engine() -> Engine:
+ """In-memory SQLite seeded with a minimal pre-migration ``slices`` table.
+
+ The real ``slices`` table has many columns; the migration only touches
+ ``deleted_at`` and its index, so only the columns that participate in
+ the test are seeded.
+ """
+ engine = create_engine("sqlite:///:memory:")
+ md = MetaData()
+ Table(
+ TABLE_NAME,
+ md,
+ Column("id", Integer, primary_key=True),
+ Column("slice_name", String(250), nullable=False),
+ )
+ md.create_all(engine)
+ return engine
+
+
+def _columns(engine: Engine) -> set[str]:
+ return {col["name"] for col in inspect(engine).get_columns(TABLE_NAME)}
+
+
+def _indexes(engine: Engine) -> set[str]:
+ return {ix["name"] for ix in inspect(engine).get_indexes(TABLE_NAME)}
+
+
+def test_upgrade_adds_deleted_at_column_and_index(engine: Engine) -> None:
+ with engine.connect() as conn:
+ ctx = MigrationContext.configure(conn)
+ with Operations.context(ctx):
+ migration.upgrade()
+
+ assert "deleted_at" in _columns(engine), "upgrade() must add the
deleted_at column"
+ assert INDEX_NAME in _indexes(engine), (
+ "upgrade() must create the supporting index on deleted_at"
+ )
+
+
+def test_downgrade_drops_deleted_at_column_and_index(engine: Engine) -> None:
+ with engine.connect() as conn:
+ ctx = MigrationContext.configure(conn)
+ with Operations.context(ctx):
+ migration.upgrade()
+ migration.downgrade()
+
+ assert "deleted_at" not in _columns(engine), (
+ "downgrade() must drop the deleted_at column"
+ )
+ assert INDEX_NAME not in _indexes(engine), (
+ "downgrade() must drop the supporting index"
+ )
+
+
+def test_downgrade_preserves_soft_deleted_row_data(engine: Engine) -> None:
+ """Pin the operator-facing contract from ``UPDATING.md``: rows that
+ were soft-deleted before the migration is reversed survive the
+ downgrade. The ``deleted_at`` column is gone, so those rows are
+ indistinguishable from live rows to any code path that no longer
+ applies the visibility filter — operators must decide on
+ hard-delete, restore, or rename BEFORE downgrading. See the
+ "Rollback note" in ``UPDATING.md``.
+ """
+ with engine.connect() as conn:
+ ctx = MigrationContext.configure(conn)
+ with Operations.context(ctx):
+ migration.upgrade()
+
+ slices = Table(TABLE_NAME, MetaData(), autoload_with=conn)
+ conn.execute(
+ insert(slices).values(
+ [
+ {
+ "id": 1,
+ "slice_name": "live_chart",
+ "deleted_at": None,
+ },
+ {
+ "id": 2,
+ "slice_name": "archived_chart",
+ "deleted_at": datetime(2026, 1, 1, 12, 0, 0),
+ },
+ ]
+ )
+ )
+
+ migration.downgrade()
+
+ slices_after = Table(TABLE_NAME, MetaData(), autoload_with=conn)
+ rows = conn.execute(
+ select(slices_after).order_by(slices_after.c.id)
+ ).fetchall()
+
+ assert [(r.id, r.slice_name) for r in rows] == [
+ (1, "live_chart"),
+ (2, "archived_chart"),
+ ], (
+ "downgrade() must not delete or mutate row data — soft-deleted "
+ "rows become indistinguishable from live rows but they remain"
+ )
+ assert "deleted_at" not in {
+ c["name"] for c in inspect(engine).get_columns(TABLE_NAME)
+ }
+
+
+def test_upgrade_is_idempotent(engine: Engine) -> None:
Review Comment:
**Suggestion:** Add a docstring to this new test function describing the
idempotency expectation for repeated upgrade execution. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added test function and it does not include a docstring in
the final file state. That matches the custom rule violation for new functions
lacking docstrings.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=da91f100569a433faaa613418293d8e2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=da91f100569a433faaa613418293d8e2&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/migrations/test_add_deleted_at_to_slices.py
**Line:** 165:165
**Comment:**
*Custom Rule: Add a docstring to this new test function describing the
idempotency expectation for repeated upgrade execution.
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%2F40129&comment_hash=9e3a6c7df8fa9a26593752aea24252deceeafd76cf253ce4adceb4dd0ba33d3f&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40129&comment_hash=9e3a6c7df8fa9a26593752aea24252deceeafd76cf253ce4adceb4dd0ba33d3f&reaction=dislike'>👎</a>
##########
tests/unit_tests/migrations/test_add_deleted_at_to_slices.py:
##########
@@ -0,0 +1,186 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Tests for migration ``7c4a8d09ca37_add_deleted_at_to_slices``.
+
+Runs the migration's ``upgrade()`` and ``downgrade()`` against an
+in-memory SQLite engine with a real Alembic ``Operations`` context.
+The behaviour being pinned is the operator-facing contract documented
+in ``UPDATING.md``: ``downgrade()`` reverses the schema but does not
+hard-delete or otherwise mutate rows that were soft-deleted before
+the migration was reversed — those rows survive the downgrade and
+become visible to any code path that no longer applies the
+soft-delete visibility filter.
+"""
+
+from __future__ import annotations
+
+from datetime import datetime
+from importlib import import_module
+
+import pytest
+from alembic.migration import MigrationContext
+from alembic.operations import Operations
+from sqlalchemy import (
+ Column,
+ create_engine,
+ insert,
+ inspect,
+ Integer,
+ MetaData,
+ select,
+ String,
+ Table,
+)
+from sqlalchemy.engine import Engine
+
+migration = import_module(
+ "superset.migrations.versions."
+ "2026-05-08_12-00_7c4a8d09ca37_add_deleted_at_to_slices"
+)
+
+TABLE_NAME = migration.TABLE_NAME # "slices"
+INDEX_NAME = migration.INDEX_NAME # "ix_slices_deleted_at"
+
+
[email protected]
+def engine() -> Engine:
+ """In-memory SQLite seeded with a minimal pre-migration ``slices`` table.
+
+ The real ``slices`` table has many columns; the migration only touches
+ ``deleted_at`` and its index, so only the columns that participate in
+ the test are seeded.
+ """
+ engine = create_engine("sqlite:///:memory:")
+ md = MetaData()
+ Table(
+ TABLE_NAME,
+ md,
+ Column("id", Integer, primary_key=True),
+ Column("slice_name", String(250), nullable=False),
+ )
+ md.create_all(engine)
+ return engine
+
+
+def _columns(engine: Engine) -> set[str]:
+ return {col["name"] for col in inspect(engine).get_columns(TABLE_NAME)}
+
+
+def _indexes(engine: Engine) -> set[str]:
+ return {ix["name"] for ix in inspect(engine).get_indexes(TABLE_NAME)}
+
+
+def test_upgrade_adds_deleted_at_column_and_index(engine: Engine) -> None:
+ with engine.connect() as conn:
+ ctx = MigrationContext.configure(conn)
+ with Operations.context(ctx):
+ migration.upgrade()
+
+ assert "deleted_at" in _columns(engine), "upgrade() must add the
deleted_at column"
+ assert INDEX_NAME in _indexes(engine), (
+ "upgrade() must create the supporting index on deleted_at"
+ )
+
+
+def test_downgrade_drops_deleted_at_column_and_index(engine: Engine) -> None:
Review Comment:
**Suggestion:** Add a docstring to this new test function clarifying that it
validates downgrade removal of both schema artifacts. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly added test function has no docstring. The custom rule explicitly
flags new Python functions without docstrings, so the violation is real.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=167126cade7d47f79f2b0f675162633e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=167126cade7d47f79f2b0f675162633e&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/migrations/test_add_deleted_at_to_slices.py
**Line:** 99:99
**Comment:**
*Custom Rule: Add a docstring to this new test function clarifying that
it validates downgrade removal of both schema artifacts.
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%2F40129&comment_hash=ff1fb8662357da65f4406aa96eff861abe0ffbfb76f42dc1fd328b08bc817a13&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40129&comment_hash=ff1fb8662357da65f4406aa96eff861abe0ffbfb76f42dc1fd328b08bc817a13&reaction=dislike'>👎</a>
##########
tests/unit_tests/migrations/test_add_deleted_at_to_slices.py:
##########
@@ -0,0 +1,186 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Tests for migration ``7c4a8d09ca37_add_deleted_at_to_slices``.
+
+Runs the migration's ``upgrade()`` and ``downgrade()`` against an
+in-memory SQLite engine with a real Alembic ``Operations`` context.
+The behaviour being pinned is the operator-facing contract documented
+in ``UPDATING.md``: ``downgrade()`` reverses the schema but does not
+hard-delete or otherwise mutate rows that were soft-deleted before
+the migration was reversed — those rows survive the downgrade and
+become visible to any code path that no longer applies the
+soft-delete visibility filter.
+"""
+
+from __future__ import annotations
+
+from datetime import datetime
+from importlib import import_module
+
+import pytest
+from alembic.migration import MigrationContext
+from alembic.operations import Operations
+from sqlalchemy import (
+ Column,
+ create_engine,
+ insert,
+ inspect,
+ Integer,
+ MetaData,
+ select,
+ String,
+ Table,
+)
+from sqlalchemy.engine import Engine
+
+migration = import_module(
+ "superset.migrations.versions."
+ "2026-05-08_12-00_7c4a8d09ca37_add_deleted_at_to_slices"
+)
+
+TABLE_NAME = migration.TABLE_NAME # "slices"
+INDEX_NAME = migration.INDEX_NAME # "ix_slices_deleted_at"
+
+
[email protected]
+def engine() -> Engine:
+ """In-memory SQLite seeded with a minimal pre-migration ``slices`` table.
+
+ The real ``slices`` table has many columns; the migration only touches
+ ``deleted_at`` and its index, so only the columns that participate in
+ the test are seeded.
+ """
+ engine = create_engine("sqlite:///:memory:")
+ md = MetaData()
+ Table(
+ TABLE_NAME,
+ md,
+ Column("id", Integer, primary_key=True),
+ Column("slice_name", String(250), nullable=False),
+ )
+ md.create_all(engine)
+ return engine
+
+
+def _columns(engine: Engine) -> set[str]:
Review Comment:
**Suggestion:** Add a short docstring to this newly added helper function
explaining that it returns the current column names for the target table.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added Python helper function and it has no docstring in the
final file state. The custom rule requires new functions to be documented
inline, so this is a real violation.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=73d71f04d793461888c781a7286e033f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=73d71f04d793461888c781a7286e033f&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/migrations/test_add_deleted_at_to_slices.py
**Line:** 79:79
**Comment:**
*Custom Rule: Add a short docstring to this newly added helper function
explaining that it returns the current column names for the target table.
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%2F40129&comment_hash=1f8dd6ecc83953402dcffc7d689a3c4937eab9c657439b4b4e02dd58cf8e92c4&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40129&comment_hash=1f8dd6ecc83953402dcffc7d689a3c4937eab9c657439b4b4e02dd58cf8e92c4&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]
