codeant-ai-for-open-source[bot] commented on code in PR #41075:
URL: https://github.com/apache/superset/pull/41075#discussion_r3538005061


##########
superset/migrations/versions/2026-05-28_19-50_56cd24c07170_add_versioning_tables.py:
##########
@@ -0,0 +1,566 @@
+# 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_versioning_tables
+
+Creates the full schema backing entity versioning in a single
+migration:
+
+1. ``version_transaction`` — audit log keyed by Continuum's per-flush
+   transaction id (plus a Postgres-specific id sequence).
+2. **Parent shadow tables** mirroring each versioned entity's columns:
+   ``dashboards_version`` / ``slices_version`` / ``tables_version``.
+3. ``version_changes`` — field-level diff log keyed to a
+   ``(transaction, entity)`` pair; each row describes one atomic change
+   (one field or one child-collection element) that occurred during a
+   save.
+4. **Child shadow tables** for the collections Continuum auto-registers
+   when ``__versioned__`` is applied to ``TableColumn`` / ``SqlMetric``
+   and the ``slices`` exclude is removed from
+   ``Dashboard.__versioned__``: ``table_columns_version`` /
+   ``sql_metrics_version`` / ``dashboard_slices_version``.
+
+All shadow tables follow the validity-strategy shape (mirrored columns
++ ``transaction_id`` / ``end_transaction_id`` / ``operation_type``
+bookkeeping with FKs to ``version_transaction.id``). The current
+version row has ``end_transaction_id = NULL``.
+
+This migration replaces three iterative migrations from the spike phase
+(``56cd24c07170``, ``e1f3c5a7b9d0``, ``f7a2b3c4d5e6``) that captured the
+same schema in three steps as the feature was developed. Compacting
+gives downstream operators one migration to apply / reverse and one
+review surface. The ``revision`` hash is reused from the original first
+migration so anyone still tracking the chain by that hash lands on the
+same logical change set.
+
+Generated by hand because the current Continuum + Alembic-autogenerate
+interaction trips on the renamed ``transaction`` -> ``version_transaction``
+table key (``KeyError`` lookups in ``table_key_to_table``). Column
+inventories were sourced from the live model ``__table__`` definitions
+and ``version_class(...).__table__`` / Continuum association metadata.
+
+Primary key choice. Both ``version_transaction.id`` and
+``version_changes.id`` are ``BigInteger`` autoincrement — a deliberate
+carveout from the project's UUID-PK convention for new models (see
+``CLAUDE.md`` §"UUID Migration"). ``version_transaction`` is keyed
+externally by SQLAlchemy-Continuum via
+``nextval('version_transaction_id_seq')`` on every INSERT; matching
+that contract is required for ``versioning_manager`` to function.
+``version_changes`` follows the same shape because the user-facing
+identity is the ``(transaction_id, entity_kind, entity_id, sequence)``
+composite unique key, not the row id; the API surfaces a deterministic
+UUIDv5 ``version_uuid`` derived from ``entity.uuid`` and
+``transaction_id`` for stable external references.
+
+Revision ID: 56cd24c07170
+Revises: 3a8e6f2c1b95
+Create Date: 2026-05-28 19:50:00.000000
+
+"""
+
+from __future__ import annotations
+
+import sqlalchemy as sa
+from alembic import op
+from sqlalchemy_utils import UUIDType
+
+from superset.utils.core import MediumText
+
+revision = "56cd24c07170"
+# Stacked on the current migration head, which is downstream of the 
composite-PK
+# association-tables change (2bee73611e32). That ordering is required: the
+# Continuum shadow tables this migration creates mirror the composite-PK shape
+# of the live association tables, so composite-PK must already have run.
+down_revision = "3a8e6f2c1b95"
+
+
+def upgrade() -> None:
+    bind = op.get_bind()
+
+    # ------------------------------------------------------------------
+    # version_transaction
+    #
+    # Audit log for each versioning event. Continuum emits
+    # ``nextval('version_transaction_id_seq')`` on every INSERT, so the
+    # sequence must exist before the table on Postgres. SQLite/MySQL
+    # ignore the explicit CREATE SEQUENCE (they auto-increment natively).
+    # ------------------------------------------------------------------
+    if bind.dialect.name == "postgresql":
+        op.execute("CREATE SEQUENCE IF NOT EXISTS version_transaction_id_seq")
+
+    op.create_table(
+        "version_transaction",
+        sa.Column(
+            "id",
+            sa.BigInteger(),
+            sa.Sequence("version_transaction_id_seq"),
+            primary_key=True,
+            autoincrement=True,
+            nullable=False,
+        ),
+        sa.Column("issued_at", sa.DateTime(), nullable=True),
+        sa.Column("remote_addr", sa.String(50), nullable=True),
+        sa.Column("user_id", sa.Integer(), nullable=True),
+        # ``action_kind`` carries the high-level avenue that produced
+        # this transaction (``restore`` / ``import`` / ``clone``).
+        # ``NULL`` is the default "ordinary save" — most rows leave
+        # this empty. Commands set
+        # ``session.info["_versioning_action_kind"]`` before commit;
+        # the change-record listener stamps the value here. Parallel
+        # to ``version_changes.entity_kind`` and ``version_changes.kind``
+        # — the schema's third ``*_kind`` column, at transaction scope.
+        sa.Column("action_kind", sa.String(32), nullable=True),
+    )
+
+    if bind.dialect.name == "postgresql":
+        op.execute(
+            "ALTER SEQUENCE version_transaction_id_seq OWNED BY 
version_transaction.id"
+        )
+
+    # ------------------------------------------------------------------
+    # dashboards_version
+    # ------------------------------------------------------------------
+    op.create_table(
+        "dashboards_version",
+        sa.Column("uuid", UUIDType(binary=True), nullable=True),
+        sa.Column("id", sa.Integer(), nullable=False),
+        sa.Column("dashboard_title", sa.String(500), nullable=True),
+        # ``MediumText()`` mirrors the live column type — on MySQL plain
+        # ``TEXT`` caps at 64 KB, which large dashboards exceed; an
+        # oversized live write would then fail the shadow INSERT under
+        # ``STRICT_TRANS_TABLES`` (or silently truncate without it) and
+        # corrupt the history. Postgres ``TEXT`` is unbounded and SQLite
+        # ignores the length annotation so this is MySQL-driven.
+        sa.Column("position_json", MediumText(), nullable=True),
+        sa.Column("description", sa.Text(), nullable=True),
+        sa.Column("css", MediumText(), nullable=True),
+        sa.Column("theme_id", sa.Integer(), nullable=True),
+        sa.Column("certified_by", sa.Text(), nullable=True),
+        sa.Column("certification_details", sa.Text(), nullable=True),
+        sa.Column("json_metadata", MediumText(), nullable=True),
+        sa.Column("slug", sa.String(255), nullable=True),
+        sa.Column("published", sa.Boolean(), nullable=True),
+        sa.Column("is_managed_externally", sa.Boolean(), nullable=True),
+        sa.Column("external_url", sa.Text(), nullable=True),
+        sa.Column("transaction_id", sa.BigInteger(), nullable=False),
+        sa.Column("end_transaction_id", sa.BigInteger(), nullable=True),
+        sa.Column("operation_type", sa.SmallInteger(), nullable=False),
+        sa.PrimaryKeyConstraint("id", "transaction_id"),
+        sa.ForeignKeyConstraint(
+            ["transaction_id"],
+            ["version_transaction.id"],
+            name="fk_dashboards_version_transaction_id",
+        ),
+        sa.ForeignKeyConstraint(
+            ["end_transaction_id"],
+            ["version_transaction.id"],
+            name="fk_dashboards_version_end_transaction_id",
+        ),
+    )
+    op.create_index(
+        "ix_dashboards_version_end_transaction_id",
+        "dashboards_version",
+        ["end_transaction_id"],
+    )
+    op.create_index(
+        "ix_dashboards_version_operation_type",
+        "dashboards_version",
+        ["operation_type"],
+    )
+    op.create_index(
+        "ix_dashboards_version_transaction_id",
+        "dashboards_version",
+        ["transaction_id"],
+    )
+
+    # ------------------------------------------------------------------
+    # slices_version (Charts)
+    # ------------------------------------------------------------------
+    op.create_table(
+        "slices_version",
+        sa.Column("uuid", UUIDType(binary=True), nullable=True),
+        sa.Column("id", sa.Integer(), nullable=False),
+        sa.Column("slice_name", sa.String(250), nullable=True),
+        sa.Column("datasource_id", sa.Integer(), nullable=True),
+        sa.Column("datasource_type", sa.String(200), nullable=True),
+        sa.Column("datasource_name", sa.String(2000), nullable=True),
+        sa.Column("viz_type", sa.String(250), nullable=True),
+        sa.Column("params", MediumText(), nullable=True),
+        sa.Column("description", sa.Text(), nullable=True),
+        sa.Column("cache_timeout", sa.Integer(), nullable=True),
+        sa.Column("certified_by", sa.Text(), nullable=True),
+        sa.Column("certification_details", sa.Text(), nullable=True),
+        sa.Column("is_managed_externally", sa.Boolean(), nullable=True),
+        sa.Column("external_url", sa.Text(), nullable=True),
+        sa.Column("transaction_id", sa.BigInteger(), nullable=False),
+        sa.Column("end_transaction_id", sa.BigInteger(), nullable=True),
+        sa.Column("operation_type", sa.SmallInteger(), nullable=False),
+        sa.PrimaryKeyConstraint("id", "transaction_id"),
+        sa.ForeignKeyConstraint(
+            ["transaction_id"],
+            ["version_transaction.id"],
+            name="fk_slices_version_transaction_id",
+        ),
+        sa.ForeignKeyConstraint(
+            ["end_transaction_id"],
+            ["version_transaction.id"],
+            name="fk_slices_version_end_transaction_id",
+        ),
+    )
+    op.create_index(
+        "ix_slices_version_end_transaction_id",
+        "slices_version",
+        ["end_transaction_id"],
+    )
+    op.create_index(
+        "ix_slices_version_operation_type",
+        "slices_version",
+        ["operation_type"],
+    )
+    op.create_index(
+        "ix_slices_version_transaction_id",
+        "slices_version",
+        ["transaction_id"],
+    )
+
+    # ------------------------------------------------------------------
+    # tables_version (SqlaTable / Datasets)
+    # ------------------------------------------------------------------
+    op.create_table(
+        "tables_version",
+        sa.Column("uuid", UUIDType(binary=True), nullable=True),
+        sa.Column("id", sa.Integer(), nullable=False),
+        sa.Column("description", sa.Text(), nullable=True),
+        sa.Column("default_endpoint", sa.Text(), nullable=True),
+        sa.Column("is_featured", sa.Boolean(), nullable=True),
+        sa.Column("filter_select_enabled", sa.Boolean(), nullable=True),
+        sa.Column("offset", sa.Integer(), nullable=True),
+        sa.Column("cache_timeout", sa.Integer(), nullable=True),
+        sa.Column("params", sa.String(1000), nullable=True),
+        sa.Column("is_managed_externally", sa.Boolean(), nullable=True),
+        sa.Column("external_url", sa.Text(), nullable=True),
+        sa.Column("table_name", sa.String(250), nullable=True),
+        sa.Column("main_dttm_col", sa.String(250), nullable=True),
+        sa.Column("currency_code_column", sa.String(250), nullable=True),
+        sa.Column("database_id", sa.Integer(), nullable=True),
+        sa.Column("fetch_values_predicate", sa.Text(), nullable=True),
+        sa.Column("schema", sa.String(255), nullable=True),
+        sa.Column("catalog", sa.String(256), nullable=True),
+        sa.Column("sql", MediumText(), nullable=True),
+        sa.Column("is_sqllab_view", sa.Boolean(), nullable=True),
+        sa.Column("template_params", sa.Text(), nullable=True),
+        sa.Column("extra", sa.Text(), nullable=True),
+        sa.Column("normalize_columns", sa.Boolean(), nullable=True),
+        sa.Column("always_filter_main_dttm", sa.Boolean(), nullable=True),
+        sa.Column("folders", sa.JSON(), nullable=True),
+        sa.Column("transaction_id", sa.BigInteger(), nullable=False),
+        sa.Column("end_transaction_id", sa.BigInteger(), nullable=True),
+        sa.Column("operation_type", sa.SmallInteger(), nullable=False),

Review Comment:
   **Suggestion:** The `tables_version` shadow schema omits `deleted_at` even 
though dataset models use `SoftDeleteMixin` and their versioning config 
currently does not exclude that field; this creates a model/schema contract 
mismatch that can break Continuum inserts for dataset version rows. Include 
`deleted_at` in `tables_version` or ensure dataset versioning excludes it 
consistently before this migration is used. [api mismatch]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   ❌ Dataset update API fails when versioning capture enabled.
   ❌ Dataset history writes crash from tables_version schema mismatch.
   ⚠️ Operators cannot safely enable dataset versioning feature.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Enable versioning capture by setting `ENABLE_VERSIONING_CAPTURE = True` so
   `SupersetAppInitializer.init_versioning()` registers Continuum write 
listeners
   (superset/initialization/__init__.py:696-755).
   
   2. Apply migration
   
`superset/migrations/versions/2026-05-28_19-50_56cd24c07170_add_versioning_tables.py`,
   which defines `tables_version` without a `deleted_at` column (see
   `op.create_table("tables_version", ...)` at lines 243-271 in the PR diff).
   
   3. Observe that the dataset model `SqlaTable` inherits `SoftDeleteMixin` and 
defines
   `deleted_at` as a mapped column (superset/models/helpers.py:742-809) while 
its
   `__versioned__` config in superset/connectors/sqla/models.py:36-63 excludes 
several fields
   but not `deleted_at`, meaning the Continuum version class for `SqlaTable` 
still includes
   `deleted_at`.
   
   4. Call the dataset update endpoint `PUT /api/v1/dataset/<pk>`, which 
executes
   `DatasetsApi.put()` (superset/datasets/api.py:419-577) and 
`UpdateDatasetCommand.run()`
   (superset/commands/dataset/update.py:80-199); on commit, 
SQLAlchemy-Continuum attempts to
   insert a version row including `deleted_at` into `tables_version`, but the 
DB table
   created by the migration lacks that column, causing an INSERT error and the 
request to
   fail (typically HTTP 500).
   ```
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9dba1599eb984727b979670fd0c49fad&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9dba1599eb984727b979670fd0c49fad&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-28_19-50_56cd24c07170_add_versioning_tables.py
   **Line:** 243:271
   **Comment:**
        *Api Mismatch: The `tables_version` shadow schema omits `deleted_at` 
even though dataset models use `SoftDeleteMixin` and their versioning config 
currently does not exclude that field; this creates a model/schema contract 
mismatch that can break Continuum inserts for dataset version rows. Include 
`deleted_at` in `tables_version` or ensure dataset versioning excludes it 
consistently before this migration is used.
   
   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%2F41075&comment_hash=1d14ee8606face2bde665e0e564e8d88e917be68692729b54b5b68b8ec212326&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41075&comment_hash=1d14ee8606face2bde665e0e564e8d88e917be68692729b54b5b68b8ec212326&reaction=dislike'>👎</a>



##########
superset/models/dashboard.py:
##########
@@ -151,6 +151,32 @@ class Dashboard(CoreDashboard, SoftDeleteMixin, 
AuditMixinNullable, ImportExport
     """The dashboard object!"""
 
     __tablename__ = "dashboards"
+    # deleted_at exclusion will be added when soft delete is merged.
+    # SPIKE (full-Continuum): ``slices`` removed from
+    # the exclude list so Continuum auto-creates an association version table
+    # for ``dashboard_slices`` and ``Reverter(relations=["slices"])`` can
+    # restore chart membership. Owners / roles stay excluded — access metadata,
+    # not user-authored content (ADR-005).
+    # Audit columns (changed_on/created_on/changed_by_fk/created_by_fk) are
+    # auto-bumped by AuditMixin on every save; excluding them lets Continuum's
+    # is_modified() return False on no-op saves (e.g. owners-only edits) so we
+    # don't create empty version rows. version_transaction.user_id /
+    # issued_at preserve "who/when" without per-row duplication.
+    # deleted_at is deletion-state metadata (SoftDeleteMixin), not 
user-authored
+    # content: it is tracked by soft delete, not by content versioning. It is 
also
+    # absent from the Continuum shadow table, so leaving it in would make every
+    # capture INSERT fail once soft delete is applied to dashboards.
+    __versioned__: dict[str, Any] = {
+        "exclude": [
+            "owners",
+            "roles",
+            "changed_on",
+            "created_on",
+            "changed_by_fk",
+            "created_by_fk",
+            "deleted_at",
+        ]
+    }

Review Comment:
   **Suggestion:** The comment says `deleted_at` exclusion will be added in the 
future, but the exclusion is already present directly below; this is an 
objective comment/code contradiction that will mislead future maintainers 
during migrations and versioning troubleshooting. Update the comment to reflect 
the current behavior. [comment mismatch]
   
   <details>
   <summary><b>Severity Level:</b> Minor 🧹</summary>
   
   ```mdx
   ⚠️ Comment misstates current deleted_at exclusion behavior.
   ⚠️ Maintainers may misread dashboard versioning configuration.
   ⚠️ Suggestion improves documentation, not runtime correctness.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Open the dashboard model definition in `superset/models/dashboard.py` 
around lines
   140-219 and note that `Dashboard` inherits `SoftDeleteMixin` (line 11) and 
defines a
   `__versioned__` configuration block immediately below.
   
   2. At line 154 in the PR diff, read the comment `# deleted_at exclusion will 
be added when
   soft delete is merged.` directly above the `__versioned__` dictionary.
   
   3. Compare this comment with the `__versioned__["exclude"]` list at lines 
31-39, which
   already includes `"deleted_at"`, showing that the exclusion has been 
implemented and soft
   delete is active on dashboards.
   
   4. A maintainer troubleshooting versioning/migration behavior for dashboards 
who relies on
   this comment may incorrectly assume `deleted_at` is not yet excluded, 
leading to confusion
   or unnecessary schema/config changes even though the runtime behavior is 
already correct.
   ```
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e7e7054b8f994801876e5c944bb3511a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e7e7054b8f994801876e5c944bb3511a&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/models/dashboard.py
   **Line:** 154:179
   **Comment:**
        *Comment Mismatch: The comment says `deleted_at` exclusion will be 
added in the future, but the exclusion is already present directly below; this 
is an objective comment/code contradiction that will mislead future maintainers 
during migrations and versioning troubleshooting. Update the comment to reflect 
the current behavior.
   
   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%2F41075&comment_hash=086ad7f28716012498d2b450388e808311f0384714554fe20320b3ac23df0e6c&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41075&comment_hash=086ad7f28716012498d2b450388e808311f0384714554fe20320b3ac23df0e6c&reaction=dislike'>👎</a>



##########
superset/models/slice.py:
##########
@@ -87,6 +87,38 @@ class Slice(  # pylint: disable=too-many-public-methods
     query_context_factory: QueryContextFactory | None = None
 
     __tablename__ = "slices"
+    # query_context is excluded: it is a cached/regenerated field, not 
user-authored.
+    # deleted_at exclusion will be added when soft delete is merged.
+    # Exclude M2M association relationships: Continuum only captures FK 
columns on
+    # association INSERTs (not the auto-increment id), which breaks the NOT 
NULL PK.
+    # Ownership changes are administrative metadata, not user-authored content.
+    # Audit / save-marker columns are auto-bumped on every save. Excluding
+    # them lets Continuum's is_modified() return False on no-op saves
+    # (e.g. owners-only edits) so we don't create empty version rows.
+    # version_transaction.user_id / issued_at preserve "who/when".
+    # The perm-string class (perm / schema_perm / catalog_perm) is derived
+    # security state, not user-authored content: permission maintenance
+    # rewrites it in bulk, and versioning it produced phantom transactions
+    # flooding the activity stream (10 "Chart updated" rows for one user
+    # save — surfaced by the version-history UI). Excluding it
+    # also means a restore can't resurrect stale permission strings; the
+    # live, derived values stay authoritative.
+    __versioned__: dict[str, Any] = {
+        "exclude": [
+            "query_context",
+            "owners",
+            "dashboards",
+            "changed_on",
+            "created_on",
+            "changed_by_fk",
+            "created_by_fk",
+            "last_saved_at",
+            "last_saved_by_fk",
+            "perm",
+            "schema_perm",
+            "catalog_perm",
+        ]
+    }

Review Comment:
   **Suggestion:** `Slice` inherits `SoftDeleteMixin`, so `deleted_at` is a 
mapped column unless explicitly excluded from versioning. This exclude list 
omits `deleted_at`, but the new `slices_version` migration schema does not 
include a `deleted_at` column, so version-row inserts for charts can fail at 
runtime with a column mismatch. Add `deleted_at` to the 
`__versioned__["exclude"]` list (or add the column to the shadow table schema 
consistently). [api mismatch]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   ❌ Chart update API fails when versioning capture enabled.
   ❌ Chart history writes crash from slices_version schema mismatch.
   ⚠️ Version-history UI cannot rely on chart versions.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Enable versioning capture by setting `ENABLE_VERSIONING_CAPTURE = True` 
and calling
   `SupersetAppInitializer.init_versioning()`, which wires Continuum listeners 
and version
   classes for `Slice` (superset/initialization/__init__.py:696-755, imports at 
lines 58-60).
   
   2. Inspect the `Slice` model in `superset/models/slice.py`: it inherits 
`SoftDeleteMixin`
   (class header at lines 23-25) so it has a mapped `deleted_at` column
   (superset/models/helpers.py:742-809), but its `__versioned__` config at 
lines 31-62
   excludes several fields and relationships while omitting `"deleted_at"`, 
leaving
   `deleted_at` as a versioned attribute.
   
   3. Apply the migration
   
`superset/migrations/versions/2026-05-28_19-50_56cd24c07170_add_versioning_tables.py`,
   which creates the `slices_version` table without a `deleted_at` column (see 
the
   `op.create_table("slices_version", ...)` block around lines 190-222 in the 
PR diff).
   
   4. Call the chart update endpoint `PUT /api/v1/chart/<pk>`, which executes
   `ChartsApi.put()` (superset/charts/api.py:500-559) and 
`UpdateChartCommand.run()` to
   modify a `Slice`; on commit, SQLAlchemy-Continuum attempts to insert a 
version row
   including `deleted_at` into `slices_version`, but because the DB table lacks 
this column,
   the INSERT fails and the chart update request returns an error (typically 
HTTP 500).
   ```
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a0713ba1a9ad4865871aac12edd8eb78&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a0713ba1a9ad4865871aac12edd8eb78&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/models/slice.py
   **Line:** 106:121
   **Comment:**
        *Api Mismatch: `Slice` inherits `SoftDeleteMixin`, so `deleted_at` is a 
mapped column unless explicitly excluded from versioning. This exclude list 
omits `deleted_at`, but the new `slices_version` migration schema does not 
include a `deleted_at` column, so version-row inserts for charts can fail at 
runtime with a column mismatch. Add `deleted_at` to the 
`__versioned__["exclude"]` list (or add the column to the shadow table schema 
consistently).
   
   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%2F41075&comment_hash=45a41d8cfe6f2db6412dbed2c94b3487a4f51819e42e1dcd172ce0616ba70eed&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41075&comment_hash=45a41d8cfe6f2db6412dbed2c94b3487a4f51819e42e1dcd172ce0616ba70eed&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]

Reply via email to