feat: Version history UI for Explore and Dashboard#40988
Draft
kgabryje wants to merge 114 commits into
Draft
Conversation
Replace synthetic id INTEGER PRIMARY KEY with composite PRIMARY KEY (fk1, fk2) on the eight pure-junction tables: dashboard_roles, dashboard_slices, dashboard_user, report_schedule_user, rls_filter_roles, rls_filter_tables, slice_user, sqlatable_user. The redundant UNIQUE(fk1, fk2) on dashboard_slices and report_schedule_user is dropped (subsumed by the new PK). Migration handles dialect quirks: copy_from for tables with pre-existing UNIQUE (so SQLite's anonymous-constraint reflection doesn't matter), wrapped- subquery dedupe for MySQL (ERROR 1093), sa.Identity(always=False) on downgrade to backfill the restored id column without NOT NULL violations, and distinct PK names per direction (pk_<table> on upgrade, <table>_pkey on downgrade) to avoid round-trip index-name collisions on Postgres. ORM Table() definitions updated to match. UPDATING.md entry added with operator runbook (BI-tool impact, pre-flight inventory queries, dedupe-row- loss notice, pg_dump workaround, FK-NOT-NULL downgrade asymmetry note). Tests: 8 schema-shape assertions (post-upgrade), 8 duplicate-rejection unit tests, 8 distinct-pair sanity tests, 1 round-trip + idempotency test (in-memory SQLite via Alembic MigrationContext). Continuum-restore verification against the new shape is out of scope for this PR; it is the responsibility of the versioning epic (sc-103156). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two cleanups from PR review: 1. ``dashboard_roles.dashboard_id`` was created nullable in revision e11ccdd12658 but was missing from ``TABLES_WITH_NULLABLE_FKS``. A production database with a stray NULL ``dashboard_id`` row would have failed the PK-add with a cryptic constraint violation. Fix by running the NULL-FK cleanup on every affected table — it is a no-op DELETE on tables whose FK columns are already NOT NULL, and it eliminates the risk of further drift in the hardcoded set. ``dashboard_roles`` is added to the documentation set; the runtime now does not consult it. 2. The unit-test parent-table name for ``rls_filter_roles`` and ``rls_filter_tables`` was ``rls_filter`` (does not exist) instead of the real parent ``row_level_security_filters``. Test passes either way (the in-memory FK is self-consistent), but the parameter is now accurate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four operator-experience improvements from the second review pass: 1. ``TABLES_WITH_NULLABLE_FKS`` is now explicitly documented as an informational set that is not consulted at runtime; the comment explains the previous ``dashboard_roles`` omission was the bug that motivated the always-run cleanup. 2. ``_delete_null_fk_rows`` docstring updated to match the "always run" semantics (was still claiming "called only on tables in TABLES_WITH_NULLABLE_FKS"). 3. ``_check_no_external_fks_to_id`` now documents its scope limitation: ``Inspector.get_table_names()`` returns the default schema only, so cross-schema FKs in non-standard multi-schema PostgreSQL deployments would not be caught. The single-schema case (Superset's documented deployment) is fully covered. 4. ``_dedupe_by_min_id`` now logs a sample of up to 10 discarded ``(fk1, fk2, id)`` tuples at WARN before deletion, so operators can audit which rows the ``MIN(id)`` policy drops. The keep- original policy is correct in practice but discards later re-grants on ownership tables; the sample makes that visible. 5. ``UPDATING.md`` documents the upgrade/downgrade primary-key name divergence (``pk_<table>`` vs ``<table>_pkey``) so operators using schema-comparison tools don't mistake it for migration drift. No schema or runtime-behaviour changes. All 44 migration tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address Beto's review comments on apache#39859: replace ``sa.text(f"...")`` SQL construction in the three pre-flight helpers (``_delete_null_fk_rows``, ``_dedupe_by_min_id``, ``_assert_no_duplicates``) with SQLAlchemy core constructs (``sa.delete``, ``sa.select``, ``sa.func``, ``.subquery()``, ``.notin_()``). A small ``_table_clause()`` helper builds a lightweight ``TableClause`` exposing the columns the queries reference; the three helpers consume it. Removes all ``# noqa: S608`` comments — they are no longer needed because there is no string-interpolated SQL. Verified the compiled SQL is identical on Postgres, MySQL, and SQLite, including the MySQL ERROR 1093 workaround (the inner aggregation is wrapped in a derived table via ``.subquery()``, producing ``... NOT IN (SELECT keep_id FROM (SELECT min(id) ...) AS keep_min)``). Also drops the redundant ``f`` prefix on the two non-interpolating lines of the ``_check_no_external_fks_to_id`` error message. 44 migration tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI test-mysql failed with: MySQLdb.OperationalError: (1826, "Duplicate foreign key constraint name 'fk_dashboard_slices_slice_id_slices'") Root cause: MySQL scopes foreign-key constraint names per-database, not per-table (PostgreSQL and SQLite scope per-table). The ``batch_alter_table(... recreate="always", copy_from=...)`` path used for ``dashboard_slices`` and ``report_schedule_user`` builds ``_alembic_tmp_<table>`` carrying the original FK names from ``copy_from`` while the original table still holds those names — MySQL rejects the temp-table creation with ERROR 1826. Fix: on MySQL only, drop the original FK constraints by name before the ``batch_alter_table`` runs. The ``copy_from`` re-creates them on the rebuilt table with their original names, so the post-migration shape is unchanged. On PostgreSQL and SQLite the original code path still runs unchanged. Local SQLite tests (44 passed, 1 skipped) still pass; CI will validate on MySQL. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two MySQL-only failures in the downgrade path, found by running the full migration history against a fresh MySQL 8 container: 1. ``MySQLdb.OperationalError: (1553, "Cannot drop index 'PRIMARY': needed in a foreign key constraint")``. InnoDB uses the composite PK index to back the FK on the leftmost column. The downgrade tried to drop the composite PK before dropping the FKs, orphaning the FK's backing index. PostgreSQL and SQLite create separate indexes for FK columns and don't trip on this. 2. ``Field 'id' doesn't have a default value`` on subsequent INSERT. ``sa.Identity(always=False)`` only emits ``AUTO_INCREMENT`` on MySQL when the column is created with ``primary_key=True`` — our portable path adds the column first then creates the PK separately, so MySQL leaves the column without auto-generation. Existing rows would all collide on id=0; future inserts fail because no default. Postgres' ``GENERATED BY DEFAULT AS IDENTITY`` and SQLite's ``INTEGER PRIMARY KEY`` rowid alias don't have this gap. Fix: extract ``_downgrade_mysql_table()`` that emits the canonical MySQL idiom — drop FKs, then a single ALTER combining ``DROP PRIMARY KEY, ADD COLUMN id INT NOT NULL AUTO_INCREMENT, ADD PRIMARY KEY (id)`` (which backfills existing rows with sequential ids and preserves AUTO_INCREMENT), restore the redundant UNIQUE on the 2 tables that originally had it, and re-add the FKs with their original names. Postgres and SQLite keep the existing portable ``batch_alter_table`` path. Raw SQL is unavoidable for the combined-ALTER form; per the constitution it's allowed for dialect-specific DDL with no SQLA equivalent, with triple-quoted strings for legibility. Verified end-to-end: upgrade → downgrade → upgrade against a fresh MySQL 8 container with INSERT-without-id sanity check showing the restored ``id`` column auto-increments correctly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Found by running fresh-install + round-trip against a real SQLite DB: 6 of the 8 affected tables had FK columns that were originally declared nullable. PostgreSQL and MySQL implicitly promote the constituent columns of an ``ALTER TABLE ... ADD PRIMARY KEY`` to ``NOT NULL``; SQLite does not (it's a long-standing SQLite quirk — only ``INTEGER PRIMARY KEY`` enforces NOT NULL on a composite-PK column). Result: a fresh SQLite install would accept ``INSERT INTO dashboard_slices (NULL, 5)`` despite both columns being part of the composite PK. Our integration tests previously masked this: the test fixture seeds columns with ``nullable=False``, so the post-upgrade NOT NULL assertion passed regardless of whether the migration enforced it. Fix: add explicit ``batch_op.alter_column(fk, nullable=False)`` for both FK columns inside the per-table batch_alter_table block. On PostgreSQL and MySQL this is a no-op (PK already implies NOT NULL); on SQLite it adds the missing NOT NULL declaration so a fresh install matches the data-model.md "After" contract. Verified end-to-end: - Postgres + MySQL: column shape unchanged (still NOT NULL) - SQLite fresh install + round-trip: all 8 tables now have NOT NULL on FK columns, ``INSERT (NULL, 5)`` correctly rejected with IntegrityError on dashboard_slices, dashboard_user, sqlatable_user Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI cypress + playwright shards were red with: ERROR [flask_migrate] Error: Multiple head revisions are present for given argument 'head' The recent rebase onto master pulled in ``33d7e0e21daa_add_semantic_layers_and_views.py`` (from PR apache#37815, "semantic layer extension"), which had been authored against ``ce6bd21901ab`` as its parent — the same parent our migration referenced. After the rebase both migrations point at ``ce6bd21901ab``, producing two heads and breaking ``flask db upgrade head`` for any downstream consumer (CI's Cypress / Playwright shards spin up a real Superset instance via ``superset db upgrade``, which is why those shards failed first; the integration shards run against a precomputed schema and didn't surface this). Fix: chain our migration after the semantic-layer migration by pointing ``down_revision`` at ``33d7e0e21daa``. The chain is now linear: ... → ce6bd21901ab → 33d7e0e21daa (semantic layers) → 2bee73611e32 (composite PK, this PR) Verified with ``superset db heads`` (returns single head ``2bee73611e32``) and the local migration test suite (44 passed, 1 skipped). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…105349) Add a "Sizing the maintenance window on PostgreSQL" sub-section to the operator runbook. The simple per-table COUNT/duplicate/NULL queries that were already there are dialect-portable but only count rows; operators on PostgreSQL with large deployments need to characterize the migration's runtime cost before scheduling it. Adds four diagnostic queries: - Per-table size, row count (from pg_class.reltuples), and which migration path each table will take (recreate-rewrite vs direct ALTER). Sizes the work concretely. - Aggregated duplicate-row roll-up: dup_groups + total rows_dropped per table. Replaces eight separate per-table queries with one consolidated result for audit/dump-before-apply decisions. - External-FK pre-flight check (the same one the migration runs at upgrade time and aborts on). Lets operators surface any blocking external reference ahead of the maintenance window. Should be empty on a stock install. - Lock-window estimate for the two full-rewrite tables, using pg_relation_size and a conservative 100 MB/s rewrite throughput assumption. The other six use direct ALTER and are dominated by composite-index build time (seconds for low-millions-of-rows tables). Prompted by reviewer feedback on apache#39859 from a large deployment asking how to size the maintenance window. The original pre-flight queries are kept for cross-dialect operators (MySQL, SQLite) since the new queries use PostgreSQL-specific catalog views. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…349) Mirror of the PostgreSQL diagnostic queries added in 1114877, adapted for MySQL/InnoDB. One important difference: InnoDB rebuilds the clustered index on every PK change, so all eight tables undergo a full table rebuild on MySQL — not just the two that go through the explicit ``recreate="always"`` path. The lock-window estimate query is updated to cover all eight rather than just two, and the "migration_path" column makes the rebuild expectation explicit ("direct ALTER (still rebuilds InnoDB clustered index)"). Other notes: - ``information_schema.TABLES.TABLE_ROWS`` is an InnoDB estimate, analogous to PostgreSQL's ``reltuples``; documented inline. - ``KEY_COLUMN_USAGE`` carries both sides of the FK in a single row on MySQL, so the external-FK pre-flight check is simpler than the PostgreSQL version (no joins between three views). - The aggregated dedupe query is portable standard SQL; included verbatim for copy-paste convenience. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds ``docker-compose-mysql.yml``, a compose-override file that swaps the default Postgres metadata DB for MySQL 8 with one extra ``-f`` flag: docker compose -f docker-compose.yml -f docker-compose-mysql.yml up Useful for evaluating dialect-specific behaviour (e.g., the runtime cost of DDL migrations on a deployment whose production metadata DB is MySQL — the question raised by review feedback on this PR). Mirrors the connection settings used by CI's ``test-mysql`` shard: ``mysql+mysqldb`` dialect, charset ``utf8mb4`` with binary_prefix. Host port defaults to 13306 (configurable via ``DATABASE_PORT_MYSQL``) to avoid colliding with a native MySQL install on 3306. A separate volume (``db_home_mysql``) keeps MySQL data isolated from the Postgres ``db_home`` volume, so switching between the two with ``-f`` flag toggles doesn't corrupt either side. The Postgres-specific init scripts under ``docker/docker-entrypoint-initdb.d/`` are not mounted on the MySQL service (they are postgres-only). Examples / cypress fixtures still load via ``superset-init``'s post-startup steps, which run ``superset load-examples`` against whichever metadata DB is in use. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fix two follow-on issues reported when starting the dev stack with docker-compose-mysql.yml: 1. ``superset-init`` step 4 (load-examples) fails with ``MySQLdb.OperationalError: (2002, "Can't connect to server on 'db'")`` because the analytics-examples DB connection inherits ``EXAMPLES_PORT=5432`` (Postgres port) from ``docker/.env``. The override flipped ``DATABASE_DIALECT`` to ``mysql+mysqldb`` but left the EXAMPLES_* group on Postgres defaults, so the URI became ``mysql+mysqldb://examples:examples@db:5432/examples`` — MySQL container has no listener on 5432. Fix: add ``EXAMPLES_HOST/PORT/DB/USER/PASSWORD`` and a complete ``SUPERSET__SQLALCHEMY_EXAMPLES_URI`` to the ``mysql-env`` anchor. 2. The Postgres init scripts under ``docker/docker-entrypoint-initdb.d/`` (``cypress-init.sh``, ``examples-init.sh``) get mounted on the MySQL container too — compose merges volume lists. They invoke ``psql`` which doesn't exist in the MySQL image, abort with ``psql: command not found``, and prevent the ``examples`` DB from being created. Fix: add a MySQL-specific init script ``docker/mysql-init/examples-init.sql`` that creates the ``examples`` database and user, and mount it at ``/docker-entrypoint-initdb.d`` in the override. Compose's later-takes-precedence rule on duplicate volume targets displaces the Postgres init dir, so the MySQL container only sees the MySQL-compatible script. (Used a plain duplicate-target mount rather than the ``!override`` tag because pre-commit's ``check-yaml`` doesn't recognize Compose's custom YAML tags.) Recovery for an existing failed MySQL stack: ``docker compose -f docker-compose.yml -f docker-compose-mysql.yml down``, then ``docker volume rm superset_db_home_mysql`` (so the new init script runs on the next fresh boot), then ``up`` again. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add ``scripts/seed_junction_load.py``, a backend-agnostic script that
bulk-inserts synthetic parent rows (dashboards, slices, users, roles,
tables, dbs) and many-to-many junction rows for the four largest
association tables targeted by the composite-PK migration:
``dashboard_slices``, ``slice_user``, ``dashboard_user``,
``dashboard_roles``.
Designed for measuring migration runtime at varying scales — run with
a series of size flags (100K / 1M / 5M / 10M for the target table)
and time the migration at each scale to verify the predicted
``O(N log N)`` extrapolation against real numbers.
Properties:
- **Reproducible**: deterministic cross-product walk through parent IDs
produces a stable pair sequence; re-running is replayable.
- **Idempotent**: re-running with the same target is a no-op; with a
higher target, only new rows are added.
- **Backend-agnostic**: connects via Superset's standard ``DATABASE_*``
env vars (or ``SUPERSET__SQLALCHEMY_DATABASE_URI``). Branches on
dialect for ``BINARY(16)`` vs ``UUID`` vs TEXT/BLOB UUID columns.
- **Batched**: bulk INSERT 10K rows per statement.
- **Per-phase timing**: logs elapsed wall time for the parents phase,
the junctions phase as a whole, and per junction-table.
- **Avoidance set**: loads existing junction pairs into a Python set
so re-runs on top of pre-existing data don't collide on the
uniqueness constraint.
Usage (inside the Superset container):
docker exec superset-superset-1 \\
/app/.venv/bin/python /app/scripts/seed_junction_load.py \\
--dashboard-slices 1000000
Defaults target a "large multi-team install" shape: 1M
``dashboard_slices``, 100K each ``slice_user`` / ``dashboard_user``,
10K ``dashboard_roles``. Override per-table via flags.
Tested locally on MySQL (the user's current eval stack):
- 200/100/100/50 row mini-run produced expected counts.
- Re-running at the same target is a no-op (idempotent).
- ``--dry-run`` plans without writing.
Junction tables not yet covered (``sqlatable_user``, ``rls_filter_*``,
``report_schedule_user``) are typically small in production and
require additional parent seeding (RLS filters, report schedules)
that wasn't worth the scope here. Adding them is straightforward by
extending ``JUNCTIONS`` and writing the corresponding parent seeder.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extends the stress-test seed script with an optional duplicate-row
injection step, used to measure the empirical cost of the migration's
``_dedupe_by_min_id`` phase.
Usage: after running the normal seed at a given scale, add
``--dirty-duplicates-pct 5`` (or any non-zero value) to inject that
percentage of duplicate ``(fk1, fk2)`` rows into each non-UNIQUE
junction (slice_user, dashboard_user, dashboard_roles —
dashboard_slices is skipped because its UNIQUE constraint, present
both pre- and post-migration, rejects duplicates).
Pre-condition: requires the DB to be at the pre-migration revision
(33d7e0e21daa). The post-migration composite PK rejects duplicates,
so attempting to inject on the upgraded schema errors out.
Empirical result on MySQL @ 10M dashboard_slices + ~2.1M other
junction rows + 105K injected duplicates (5% on the 3 non-UNIQUE
tables):
Upgrade time: 1m 36s vs clean baseline 1m 37s
→ dedupe cost is within measurement noise; the table-scan that
the migration already performs dominates whether or not
duplicates exist.
This empirically confirms what the cost-model predicted: the
``_dedupe_by_min_id`` GROUP BY scan is the dominant cost of that
phase, and the actual per-duplicate DELETE is negligible.
NULL-FK injection deliberately skipped — would require altering the
six non-UNIQUE FK columns from NOT NULL back to nullable (the
migration's downgrade keeps them NOT NULL by design), which adds
per-backend ALTER complexity for a code path that's structurally
identical in cost shape (DELETE WHERE col IS NULL is the same scan
shape as the dedupe scan).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…5349) Justin Park (@justinpark) reported on apache#39859: MySQLdb.OperationalError: (1832, "Cannot change column 'dashboard_id': used in a foreign key constraint 'fk_dashboard_roles_dashboard_id_dashboards'") Root cause: ``batch_op.alter_column(fk1, nullable=False)`` for the six non-UNIQUE association tables emits ``ALTER COLUMN`` on a column that participates in an FK constraint. MySQL 8 rejects this with ERROR 1832 when the table has data — even when the change is just ``NULL`` → ``NOT NULL`` and the column is already part of a freshly-added composite primary key (which InnoDB has just made implicitly NOT NULL anyway). The error fires on populated tables only; CI's ``test-mysql`` shard runs against empty tables and so didn't catch this, while a real production-shaped install does. The ``alter_column`` was only ever needed for SQLite, where composite ``PRIMARY KEY`` does not promote constituent columns to ``NOT NULL`` (a long-standing SQLite quirk — only ``INTEGER PRIMARY KEY`` does). PostgreSQL and MySQL implicitly promote PK columns to ``NOT NULL`` as part of ``ADD PRIMARY KEY``, so the explicit step is unnecessary on both — and on MySQL it's actively broken on populated tables. Fix: extract the ``alter_column`` pair into a helper ``_enforce_not_null_for_sqlite()`` that no-ops on Postgres and MySQL. Both branches of the per-table upgrade (the ``recreate="always"`` path for the two UNIQUE-bearing tables, and the direct-ALTER path for the other six) now call the helper instead of inlining the ``alter_column``. Verified end-to-end: downgrade-then-upgrade against MySQL with ~12M total junction rows (10M dashboard_slices + 1M each slice_user/dashboard_user + 100K dashboard_roles) completes in 1m 39s with no ERROR 1832. The 44 in-memory SQLite tests still pass. Considered Justin's alternative (drop FKs on MySQL across all eight tables, unifying the two branches) but rejected as more invasive — it would require capturing FK metadata and explicitly re-creating the FKs for the six non-recreate tables, since they don't go through the ``copy_from`` path that re-creates FKs automatically. The SQLite-only approach is more targeted: it removes the operation that MySQL rejects rather than working around the rejection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three improvements from @aminghadersohi's review on apache#39859: 1. **`fk["name"]` unguarded in ``_downgrade_mysql_table`` re-add loop** The drop loop gates on ``if fk_name := fk.get("name"):`` but the re-add loop accessed ``fk["name"]`` unconditionally in an f-string. MySQL/InnoDB always assigns FK names, so this branch was defensive, but the asymmetry was confusing. Symmetrized via ``continue`` at the top of the re-add loop. 2. **``ondelete`` whitelist before raw-SQL interpolation** The value comes from MySQL's ``information_schema`` (not user input), but interpolating a reflected string into raw SQL without a guard left a "what if an unexpected value appears" footgun. Added ``_VALID_ONDELETE_ACTIONS`` (the four SQL-standard actions) and a ``RuntimeError`` when an unexpected value is reflected. 3. **Direct ALTER on PostgreSQL for tables with pre-existing UNIQUE** ``recreate="always"`` is dialect-agnostic — on PostgreSQL it triggers ``CREATE TABLE AS SELECT → DROP → RENAME`` holding ``ACCESS EXCLUSIVE`` for the full table-copy duration. For a multi-million-row ``dashboard_slices``, that lock window can be noticeable. The reflected UNIQUE constraint has a stable name on PostgreSQL (default ``<table>_<cols>_key`` convention), so dropping it directly and then running structural change as direct ALTER avoids the copy entirely. The reflected UNIQUE name is wrapped in a new ``_drop_redundant_unique_by_name()`` helper. Postgres takes the direct path; MySQL keeps ``recreate="always"`` because InnoDB binds FKs to the UNIQUE's underlying index for back-reference (``DROP CONSTRAINT`` on the UNIQUE there raises ``ERROR 1553``); SQLite keeps ``recreate="always"`` because unnamed UNIQUEs reflect with ``name=None`` and can't be dropped by name. Verified end-to-end: downgrade-then-upgrade against MySQL with ~12M total junction rows seeded completes in ~1m 41s (within the range of the prior measurements). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Belt-and-braces invariant: ``t.name`` is interpolated as a
backtick-quoted identifier into the ALTER statements emitted by
``_downgrade_mysql_table``. The values originate from
``AFFECTED_TABLES`` (a module-level literal), so SQL injection is
already structurally precluded at the call site. Adding an explicit
``allowed = {a.name for a in AFFECTED_TABLES}`` membership check
makes that invariant load-bearing rather than implicit — a future
refactor that loosens the call-site can't slip past review.
Surfaced during a downstream SQLAlchemy review on the entity-versioning
branch that stacks on top of this one; lifted onto sc-105349 because
the patch is properly scoped to this branch's composite-PK migration.
Pin SQLAlchemy-Continuum for the validity-strategy shadow tables that back FR-016..FR-021 (entity version history). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Single hand-written migration creating the schema backing sc-103156
entity versioning. Replaces three iterative spike-phase migrations
(56cd24c07170, e1f3c5a7b9d0, f7a2b3c4d5e6) so downstream operators see
one migration to apply or reverse and one review surface. Hash retained
from the original first migration so anyone still tracking the chain
by that hash lands on the same logical change set.
Tables created (eight total): version_transaction (audit log keyed by
Continuum's per-flush transaction id, plus a Postgres-only id_seq),
version_changes (field-level diff log), three parent shadow tables
(dashboards_version / slices_version / tables_version), and three
child shadow tables (table_columns_version / sql_metrics_version /
dashboard_slices_version). Downgrade drops all eight in FK-reverse
order plus the Postgres sequence.
Primary key choice. version_transaction.id and version_changes.id are
BigInteger autoincrement — a deliberate carveout from the project's
UUID-PK convention. 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.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
UUIDMixin gains string-value coercion so uuid setter accepts both str and UUID forms. Audit columns are pinned when force-flagging the parent dirty so onupdate hooks don't fire spuriously; the flag_modified-suppresses-onupdate invariant is locked in by test. Slice, Dashboard, SqlaTable, TableColumn, and SqlMetric carry __versioned__ declarations so SQLAlchemy-Continuum builds shadow classes for each. The slices exclude is dropped from Dashboard.__versioned__ so dashboard_slices membership is captured in the timeline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Field-level diff (Shape B leaf-level recursive walk) with per-field depth caps; scalar_fields_for discovers versioned columns automatically so new columns are picked up without editing the diff module; diff_slice_params kind-classifies JSON-blob changes (filter / metric / time_range / color_palette / dimension / field). Adds versioning/queries.py (per-version row construction for the API response shape), versioning/utils.py (read_row_outside_flush + small shared helpers), versioning/schemas.py (Marshmallow response schemas), and the package __init__.py. Unit tests for the diff engine live in tests/unit_tests/versioning/test_diff.py and cover both scalar-field and JSON-blob walking. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
VersionTransactionFactory.create_class appends user_id and action_kind columns onto the Continuum-built Table via append_column + add_property because Continuum does not propagate __versioned__-config columns onto the version table. Without this the runtime attribute access (tx_tbl.c.action_kind) fails and the change-record listener cannot stamp the transaction-scope action_kind. Baseline listener captures pre-existing rows on first save so the timeline isn't missing transaction-0; uses session.no_autoflush around the live-row reads to avoid the implicit flush that would otherwise push the in-progress save before its own pre-state is captured. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…bing
Two session events cooperate: before_flush reads pre-save scalar state
from the DB via raw SQL inside session.no_autoflush, calls the diff
engine, and buffers per-entity ChangeRecords on session.info.
after_flush drains the buffer, resolves the current Continuum
transaction id, and bulk-inserts one version_changes row per record
with a monotonic sequence number. Records accumulated across multiple
before_flush calls within one transaction share the same
transaction_id and contiguous sequence numbers.
Three-dimension schema. The version_changes row carries kind (content
category, per record) and operation (verb, per record). The
transaction-scope action_kind ("restore" / "import" / "clone" / NULL)
is stamped onto version_transaction.action_kind via sa.update() —
dialect-portable through the SQLAlchemy core compiler's identifier
quoting, not f-string SQL. Commands declare the avenue by writing
session.info[ACTION_KIND_KEY] immediately before db.session.commit().
Cleanup. The listener pops the action_kind key after stamping
(primary lifecycle); an after_rollback listener pops it again as a
safety net so a long-lived session cannot inherit a stale action_kind
into the next transaction. Regression test
test_action_kind_dropped_on_rollback pins this down.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wire init_versioning() into the app boot sequence so Continuum's make_versioned() runs after model imports but before the session listeners are registered. Adds versioning config keys (retention windows, feature gates), and list_versions / get_version / restore_version permission map entries to the route-method permission table so the REST endpoints flow through the standard FAB check. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
VersionDAO exposes list_versions, get_version, and restore_version with row-level ownership filtering (T056). Restore primitives execute the Continuum revert under a single transactional boundary so a partial restore cannot land in the database; child collections are restored alongside their parent. ETag helper module emits stable headers for GET /versions/<uuid>/ keyed by entity uuid and transaction id so clients can cache snapshots across replicas. dataset DAO touches accommodate restore-aware refresh so the dataset's columns/metrics reload after a version revert. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
BaseRestoreVersionCommand defines the workflow for a non-destructive version restore on one entity. Subclasses declare the model class plus the three entity-specific exception classes (not_found / forbidden / failed); each subclass decorates run() with @transaction(on_error=...) so the transactional commit boundary maps to the right HTTP-level error. Each command stamps session.info[ACTION_KIND_KEY] = "restore" before db.session.commit() so the change-record listener writes version_transaction.action_kind = "restore" on the resulting Continuum transaction. Reuses the resource's existing can_write permission; workspace admins can list and restore any entity. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three endpoints per resource type (chart, dashboard, dataset) under
the existing API surface:
GET /api/v1/{resource}/<uuid>/versions/
GET /api/v1/{resource}/<uuid>/versions/<version_uuid>/
POST /api/v1/{resource}/<uuid>/versions/<version_uuid>/restore
<version_uuid> is a deterministic UUIDv5 derived from the entity's
uuid and the Continuum transaction id — stable across replicas and
retention pruning. Authorisation reuses can_write on the resource.
Existing copy / duplicate / import / asset-import commands stamp
session.info[ACTION_KIND_KEY] before commit ("clone" / "import") so
the timeline reads "Cloned from <source>" or "Imported from <file>"
instead of "Created". Method-scoped imports of ACTION_KIND_KEY carry a
one-line justification noting the versioning bootstrap defer pattern
documented in changes.py.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Daily beat task prunes version_transaction rows older than the configured retention window and cascades to the shadow tables and version_changes via the ON DELETE CASCADE FKs declared in the versioning migration. Retention window is config-driven so operators can tune per-deployment without code changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per-resource version-history tests (list / get / restore round-trips) for chart, dashboard, and dataset. SkipUnmodifiedPlugin integration coverage verifies that ordinary saves that do not change any versioned column do not produce a new Continuum transaction (FR-021 budget). Perf-validation harness pins down the save-path overhead budget on dashboards with many charts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Documents the new endpoints, the version response shape (changes array, version_uuid derivation, action_kind transaction scope), and the no-frontend-UI-in-this-release expectation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 12, 2026
Version-history UI feedback (PR apache#40988): action_kind='restore' carried no pointer to WHICH version was restored, so 'Restored to X from [date]' couldn't be rendered from API data. Implements the __meta__ headline convention: the restore command sets ACTION_META_KEY on session.info alongside ACTION_KIND_KEY; the change-record listener pops it on its first firing for the transaction and prepends a synthetic record (kind='__meta__', path=['__meta__','restore'], to_value={version_uuid, version_number}) at sequence 0. Same lifecycle as action_kind: popped on use, cleaned up on commit/rollback. The convention generalizes to import/clone headlines later. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 12, 2026
…ave marker, search Backend fixes for the client workarounds in PR apache#40988 (grep: 'backend workaround'): - __meta__ accommodation: add '__meta__' to ACTIVITY_CHANGE_KINDS and render 'restored to version N' summaries for the restore headline records the backend now emits; convert the long-standing AV-015 xfail into a passing assertion on the headline (the strict xfail had been masking an endpoint 404 — fixed separately as the restore-uuid bug). - first_tracked_save marker: every activity record says whether its transaction is the entity's FIRST tracked save (first op=1 after the retroactive baseline). A legacy chart's first Explore save replays ~74 params-normalization deltas; clients collapse such transactions instead of rendering each as a user edit. Matched on (id, uuid) against the live row so id reuse after a hard delete can't inherit a dead entity's history. - Server-side search: ?q= on /activity/ filters the FULL history (summary, entity name, kind, path, values, case-insensitive) before pagination, so count reflects matches — the panel's client-side search only covered loaded pages. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 12, 2026
…ave marker, search Backend fixes for the client workarounds in PR apache#40988 (grep: 'backend workaround'): - __meta__ accommodation: add '__meta__' to ACTIVITY_CHANGE_KINDS and render 'restored to version N' summaries for the restore headline records the backend now emits; convert the long-standing AV-015 xfail into a passing assertion on the headline (the strict xfail had been masking an endpoint 404 — fixed separately as the restore-uuid bug). - first_tracked_save marker: every activity record says whether its transaction is the entity's FIRST tracked save (first op=1 after the retroactive baseline). A legacy chart's first Explore save replays ~74 params-normalization deltas; clients collapse such transactions instead of rendering each as a user edit. Matched on (id, uuid) against the live row so id reuse after a hard delete can't inherit a dead entity's history. - Server-side search: ?q= on /activity/ filters the FULL history (summary, entity name, kind, path, values, case-insensitive) before pagination, so count reflects matches — the panel's client-side search only covered loaded pages. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 12, 2026
Multi-lens review of the headline work (PR apache#40988 feedback): - build_action_headline() is the single owner of the record shape; the restore command no longer hand-rolls the dict (three-module string contract). - operation='announce' (new vocabulary member): a headline announces an action, it doesn't edit a field. The verb no longer rides in path — path is pure navigation per the ChangeRecord contract; renderers join kind='__meta__' with the transaction's action_kind. - The injector peeks instead of popping when the buffer is empty, and runs after the child-record append: a headline-only buffer no longer defeats the multi-flush short-circuit (which would have silently dropped a later flush's records once import/clone headlines arrive). - _BUFFER_KEY is cleared on the processed-tx early return and popped in both after_commit/after_rollback cleanups — buffered records can no longer leak into the next transaction under the wrong tx id. - Module docstring re-synced with the session keys it manages. - Test hygiene: the perm-rewrite probe restores the perm trio in a finally (security-routing state on a persistent DB), and the foreign-uuid test un-plants the shadow row it corrupts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 12, 2026
…ave marker, search Backend fixes for the client workarounds in PR apache#40988 (grep: 'backend workaround'): - __meta__ accommodation: add '__meta__' to ACTIVITY_CHANGE_KINDS and render 'restored to version N' summaries for the restore headline records the backend now emits; convert the long-standing AV-015 xfail into a passing assertion on the headline (the strict xfail had been masking an endpoint 404 — fixed separately as the restore-uuid bug). - first_tracked_save marker: every activity record says whether its transaction is the entity's FIRST tracked save (first op=1 after the retroactive baseline). A legacy chart's first Explore save replays ~74 params-normalization deltas; clients collapse such transactions instead of rendering each as a user edit. Matched on (id, uuid) against the live row so id reuse after a hard delete can't inherit a dead entity's history. - Server-side search: ?q= on /activity/ filters the FULL history (summary, entity name, kind, path, values, case-insensitive) before pagination, so count reflects matches — the panel's client-side search only covered loaded pages. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 12, 2026
…y, docs Multi-lens review follow-ups on the PR apache#40988 feedback work: - __meta__ headlines now render on SELF records (their primary surface — 'restored to version N' was previously built only for related records and the entity's own stream got summary=''), include the entity name on related streams, and dispatch on the transaction's action_kind instead of a verb smuggled into path. - operation vocabulary grows an honest member: headline records are 'announce', not a fudged 'edit'. - ?q= search matches values in their JSON form (the text clients render — 'false'/'null'/double-quoted keys) and no longer collapses falsy values (False/0) to unsearchable empty strings. - first_tracked_save documents its (id, uuid) live-row matching: always false for hard-deleted entities. - get_activity's count contract names the post-q semantics; UPDATING.md's activity section documents q, first_tracked_save, and the __meta__/announce record class. - Unit tests pin the headline summary branches and the falsy/JSON search semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
@kgabryje Four of the five items are addressed on the backend stack; one comes with an honest caveat:
|
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 15, 2026
…ave marker, search Backend fixes for the client workarounds in PR apache#40988 (grep: 'backend workaround'): - __meta__ accommodation: add '__meta__' to ACTIVITY_CHANGE_KINDS and render 'restored to version N' summaries for the restore headline records the backend now emits; convert the long-standing AV-015 xfail into a passing assertion on the headline (the strict xfail had been masking an endpoint 404 — fixed separately as the restore-uuid bug). - first_tracked_save marker: every activity record says whether its transaction is the entity's FIRST tracked save (first op=1 after the retroactive baseline). A legacy chart's first Explore save replays ~74 params-normalization deltas; clients collapse such transactions instead of rendering each as a user edit. Matched on (id, uuid) against the live row so id reuse after a hard delete can't inherit a dead entity's history. - Server-side search: ?q= on /activity/ filters the FULL history (summary, entity name, kind, path, values, case-insensitive) before pagination, so count reflects matches — the panel's client-side search only covered loaded pages. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 15, 2026
…y, docs Multi-lens review follow-ups on the PR apache#40988 feedback work: - __meta__ headlines now render on SELF records (their primary surface — 'restored to version N' was previously built only for related records and the entity's own stream got summary=''), include the entity name on related streams, and dispatch on the transaction's action_kind instead of a verb smuggled into path. - operation vocabulary grows an honest member: headline records are 'announce', not a fudged 'edit'. - ?q= search matches values in their JSON form (the text clients render — 'false'/'null'/double-quoted keys) and no longer collapses falsy values (False/0) to unsearchable empty strings. - first_tracked_save documents its (id, uuid) live-row matching: always false for hard-deleted entities. - get_activity's count contract names the post-q semantics; UPDATING.md's activity section documents q, first_tracked_save, and the __meta__/announce record class. - Unit tests pin the headline summary branches and the falsy/JSON search semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 15, 2026
Version-history UI feedback (PR apache#40988): system-managed writes polluted the activity timeline with phantom rows the client had to suppress. - Exclude the perm-string class (perm / schema_perm / catalog_perm) from Slice and SqlaTable versioning: bulk permission maintenance rewrites these across many entities, producing phantom transactions (10 'Chart updated' rows for one user save). They're derived security state, not user content — excluding them means Continuum sees no versioned change, so the phantom transactions disappear entirely (records, version rows, and shadow churn). Restore also can no longer resurrect stale permission strings. Shadow DDL columns dropped from the unmerged migration. - Add shared_label_colors to DASHBOARD_JSON_METADATA_AUDIT_KEYS: the DAO rewrites it when a dashboard is merely viewed, producing phantom 'Properties updated' records. The view-time write itself is a separate round-trip-asymmetry issue (cf. apache#39706); this stops the change-record noise regardless. Lets the frontend revert the NOISE_PATHS workaround for these paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 15, 2026
Version-history UI feedback (PR apache#40988): a single dataset save produced two Continuum transactions stamped the same second (update + refresh each committed), rendered as two 'Dataset updated' rows for one user action. Wrap UpdateDatasetCommand + RefreshDatasetCommand in one @transaction at the API layer — the decorator nests via g.in_transaction, so the commands' own decorators pass through and the wrapper commits once. The pass-through also skips the commands' on_error conversion, so it's replicated inline. This simplifies the earlier attribution fix: with a single transaction the post-commit version reads and the ETag are correct by construction. Intended semantic change: a failed refresh now rolls back the whole save instead of leaving the update applied with the refresh lost. If the reporter's repro involved two separate HTTP requests rather than the override_columns path, that half is client-side; the regression test pins the backend path to one transaction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 15, 2026
Version-history UI feedback (PR apache#40988): action_kind='restore' carried no pointer to WHICH version was restored, so 'Restored to X from [date]' couldn't be rendered from API data. Implements the __meta__ headline convention: the restore command sets ACTION_META_KEY on session.info alongside ACTION_KIND_KEY; the change-record listener pops it on its first firing for the transaction and prepends a synthetic record (kind='__meta__', path=['__meta__','restore'], to_value={version_uuid, version_number}) at sequence 0. Same lifecycle as action_kind: popped on use, cleaned up on commit/rollback. The convention generalizes to import/clone headlines later. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 15, 2026
Multi-lens review of the headline work (PR apache#40988 feedback): - build_action_headline() is the single owner of the record shape; the restore command no longer hand-rolls the dict (three-module string contract). - operation='announce' (new vocabulary member): a headline announces an action, it doesn't edit a field. The verb no longer rides in path — path is pure navigation per the ChangeRecord contract; renderers join kind='__meta__' with the transaction's action_kind. - The injector peeks instead of popping when the buffer is empty, and runs after the child-record append: a headline-only buffer no longer defeats the multi-flush short-circuit (which would have silently dropped a later flush's records once import/clone headlines arrive). - _BUFFER_KEY is cleared on the processed-tx early return and popped in both after_commit/after_rollback cleanups — buffered records can no longer leak into the next transaction under the wrong tx id. - Module docstring re-synced with the session keys it manages. - Test hygiene: the perm-rewrite probe restores the perm trio in a finally (security-routing state on a persistent DB), and the foreign-uuid test un-plants the shadow row it corrupts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 15, 2026
…ave marker, search Backend fixes for the client workarounds in PR apache#40988 (grep: 'backend workaround'): - __meta__ accommodation: add '__meta__' to ACTIVITY_CHANGE_KINDS and render 'restored to version N' summaries for the restore headline records the backend now emits; convert the long-standing AV-015 xfail into a passing assertion on the headline (the strict xfail had been masking an endpoint 404 — fixed separately as the restore-uuid bug). - first_tracked_save marker: every activity record says whether its transaction is the entity's FIRST tracked save (first op=1 after the retroactive baseline). A legacy chart's first Explore save replays ~74 params-normalization deltas; clients collapse such transactions instead of rendering each as a user edit. Matched on (id, uuid) against the live row so id reuse after a hard delete can't inherit a dead entity's history. - Server-side search: ?q= on /activity/ filters the FULL history (summary, entity name, kind, path, values, case-insensitive) before pagination, so count reflects matches — the panel's client-side search only covered loaded pages. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 15, 2026
…y, docs Multi-lens review follow-ups on the PR apache#40988 feedback work: - __meta__ headlines now render on SELF records (their primary surface — 'restored to version N' was previously built only for related records and the entity's own stream got summary=''), include the entity name on related streams, and dispatch on the transaction's action_kind instead of a verb smuggled into path. - operation vocabulary grows an honest member: headline records are 'announce', not a fudged 'edit'. - ?q= search matches values in their JSON form (the text clients render — 'false'/'null'/double-quoted keys) and no longer collapses falsy values (False/0) to unsearchable empty strings. - first_tracked_save documents its (id, uuid) live-row matching: always false for hard-deleted entities. - get_activity's count contract names the post-q semantics; UPDATING.md's activity section documents q, first_tracked_save, and the __meta__/announce record class. - Unit tests pin the headline summary branches and the falsy/JSON search semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 15, 2026
Version-history UI feedback (PR apache#40988): system-managed writes polluted the activity timeline with phantom rows the client had to suppress. - Exclude the perm-string class (perm / schema_perm / catalog_perm) from Slice and SqlaTable versioning: bulk permission maintenance rewrites these across many entities, producing phantom transactions (10 'Chart updated' rows for one user save). They're derived security state, not user content — excluding them means Continuum sees no versioned change, so the phantom transactions disappear entirely (records, version rows, and shadow churn). Restore also can no longer resurrect stale permission strings. Shadow DDL columns dropped from the unmerged migration. - Add shared_label_colors to DASHBOARD_JSON_METADATA_AUDIT_KEYS: the DAO rewrites it when a dashboard is merely viewed, producing phantom 'Properties updated' records. The view-time write itself is a separate round-trip-asymmetry issue (cf. apache#39706); this stops the change-record noise regardless. Lets the frontend revert the NOISE_PATHS workaround for these paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 15, 2026
Version-history UI feedback (PR apache#40988): a single dataset save produced two Continuum transactions stamped the same second (update + refresh each committed), rendered as two 'Dataset updated' rows for one user action. Wrap UpdateDatasetCommand + RefreshDatasetCommand in one @transaction at the API layer — the decorator nests via g.in_transaction, so the commands' own decorators pass through and the wrapper commits once. The pass-through also skips the commands' on_error conversion, so it's replicated inline. This simplifies the earlier attribution fix: with a single transaction the post-commit version reads and the ETag are correct by construction. Intended semantic change: a failed refresh now rolls back the whole save instead of leaving the update applied with the refresh lost. If the reporter's repro involved two separate HTTP requests rather than the override_columns path, that half is client-side; the regression test pins the backend path to one transaction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 15, 2026
Version-history UI feedback (PR apache#40988): action_kind='restore' carried no pointer to WHICH version was restored, so 'Restored to X from [date]' couldn't be rendered from API data. Implements the __meta__ headline convention: the restore command sets ACTION_META_KEY on session.info alongside ACTION_KIND_KEY; the change-record listener pops it on its first firing for the transaction and prepends a synthetic record (kind='__meta__', path=['__meta__','restore'], to_value={version_uuid, version_number}) at sequence 0. Same lifecycle as action_kind: popped on use, cleaned up on commit/rollback. The convention generalizes to import/clone headlines later. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 15, 2026
Multi-lens review of the headline work (PR apache#40988 feedback): - build_action_headline() is the single owner of the record shape; the restore command no longer hand-rolls the dict (three-module string contract). - operation='announce' (new vocabulary member): a headline announces an action, it doesn't edit a field. The verb no longer rides in path — path is pure navigation per the ChangeRecord contract; renderers join kind='__meta__' with the transaction's action_kind. - The injector peeks instead of popping when the buffer is empty, and runs after the child-record append: a headline-only buffer no longer defeats the multi-flush short-circuit (which would have silently dropped a later flush's records once import/clone headlines arrive). - _BUFFER_KEY is cleared on the processed-tx early return and popped in both after_commit/after_rollback cleanups — buffered records can no longer leak into the next transaction under the wrong tx id. - Module docstring re-synced with the session keys it manages. - Test hygiene: the perm-rewrite probe restores the perm trio in a finally (security-routing state on a persistent DB), and the foreign-uuid test un-plants the shadow row it corrupts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 15, 2026
…ave marker, search Backend fixes for the client workarounds in PR apache#40988 (grep: 'backend workaround'): - __meta__ accommodation: add '__meta__' to ACTIVITY_CHANGE_KINDS and render 'restored to version N' summaries for the restore headline records the backend now emits; convert the long-standing AV-015 xfail into a passing assertion on the headline (the strict xfail had been masking an endpoint 404 — fixed separately as the restore-uuid bug). - first_tracked_save marker: every activity record says whether its transaction is the entity's FIRST tracked save (first op=1 after the retroactive baseline). A legacy chart's first Explore save replays ~74 params-normalization deltas; clients collapse such transactions instead of rendering each as a user edit. Matched on (id, uuid) against the live row so id reuse after a hard delete can't inherit a dead entity's history. - Server-side search: ?q= on /activity/ filters the FULL history (summary, entity name, kind, path, values, case-insensitive) before pagination, so count reflects matches — the panel's client-side search only covered loaded pages. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 15, 2026
…y, docs Multi-lens review follow-ups on the PR apache#40988 feedback work: - __meta__ headlines now render on SELF records (their primary surface — 'restored to version N' was previously built only for related records and the entity's own stream got summary=''), include the entity name on related streams, and dispatch on the transaction's action_kind instead of a verb smuggled into path. - operation vocabulary grows an honest member: headline records are 'announce', not a fudged 'edit'. - ?q= search matches values in their JSON form (the text clients render — 'false'/'null'/double-quoted keys) and no longer collapses falsy values (False/0) to unsearchable empty strings. - first_tracked_save documents its (id, uuid) live-row matching: always false for hard-deleted entities. - get_activity's count contract names the post-q semantics; UPDATING.md's activity section documents q, first_tracked_save, and the __meta__/announce record class. - Unit tests pin the headline summary branches and the falsy/JSON search semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 16, 2026
…ave marker, search Backend fixes for the client workarounds in PR apache#40988 (grep: 'backend workaround'): - __meta__ accommodation: add '__meta__' to ACTIVITY_CHANGE_KINDS and render 'restored to version N' summaries for the restore headline records the backend now emits; convert the long-standing AV-015 xfail into a passing assertion on the headline (the strict xfail had been masking an endpoint 404 — fixed separately as the restore-uuid bug). - first_tracked_save marker: every activity record says whether its transaction is the entity's FIRST tracked save (first op=1 after the retroactive baseline). A legacy chart's first Explore save replays ~74 params-normalization deltas; clients collapse such transactions instead of rendering each as a user edit. Matched on (id, uuid) against the live row so id reuse after a hard delete can't inherit a dead entity's history. - Server-side search: ?q= on /activity/ filters the FULL history (summary, entity name, kind, path, values, case-insensitive) before pagination, so count reflects matches — the panel's client-side search only covered loaded pages. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 16, 2026
…y, docs Multi-lens review follow-ups on the PR apache#40988 feedback work: - __meta__ headlines now render on SELF records (their primary surface — 'restored to version N' was previously built only for related records and the entity's own stream got summary=''), include the entity name on related streams, and dispatch on the transaction's action_kind instead of a verb smuggled into path. - operation vocabulary grows an honest member: headline records are 'announce', not a fudged 'edit'. - ?q= search matches values in their JSON form (the text clients render — 'false'/'null'/double-quoted keys) and no longer collapses falsy values (False/0) to unsearchable empty strings. - first_tracked_save documents its (id, uuid) live-row matching: always false for hard-deleted entities. - get_activity's count contract names the post-q semantics; UPDATING.md's activity section documents q, first_tracked_save, and the __meta__/announce record class. - Unit tests pin the headline summary branches and the falsy/JSON search semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 16, 2026
Version-history UI feedback (PR apache#40988): system-managed writes polluted the activity timeline with phantom rows the client had to suppress. - Exclude the perm-string class (perm / schema_perm / catalog_perm) from Slice and SqlaTable versioning: bulk permission maintenance rewrites these across many entities, producing phantom transactions (10 'Chart updated' rows for one user save). They're derived security state, not user content — excluding them means Continuum sees no versioned change, so the phantom transactions disappear entirely (records, version rows, and shadow churn). Restore also can no longer resurrect stale permission strings. Shadow DDL columns dropped from the unmerged migration. - Add shared_label_colors to DASHBOARD_JSON_METADATA_AUDIT_KEYS: the DAO rewrites it when a dashboard is merely viewed, producing phantom 'Properties updated' records. The view-time write itself is a separate round-trip-asymmetry issue (cf. apache#39706); this stops the change-record noise regardless. Lets the frontend revert the NOISE_PATHS workaround for these paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 16, 2026
Version-history UI feedback (PR apache#40988): a single dataset save produced two Continuum transactions stamped the same second (update + refresh each committed), rendered as two 'Dataset updated' rows for one user action. Wrap UpdateDatasetCommand + RefreshDatasetCommand in one @transaction at the API layer — the decorator nests via g.in_transaction, so the commands' own decorators pass through and the wrapper commits once. The pass-through also skips the commands' on_error conversion, so it's replicated inline. This simplifies the earlier attribution fix: with a single transaction the post-commit version reads and the ETag are correct by construction. Intended semantic change: a failed refresh now rolls back the whole save instead of leaving the update applied with the refresh lost. If the reporter's repro involved two separate HTTP requests rather than the override_columns path, that half is client-side; the regression test pins the backend path to one transaction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 16, 2026
Version-history UI feedback (PR apache#40988): action_kind='restore' carried no pointer to WHICH version was restored, so 'Restored to X from [date]' couldn't be rendered from API data. Implements the __meta__ headline convention: the restore command sets ACTION_META_KEY on session.info alongside ACTION_KIND_KEY; the change-record listener pops it on its first firing for the transaction and prepends a synthetic record (kind='__meta__', path=['__meta__','restore'], to_value={version_uuid, version_number}) at sequence 0. Same lifecycle as action_kind: popped on use, cleaned up on commit/rollback. The convention generalizes to import/clone headlines later. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 16, 2026
Multi-lens review of the headline work (PR apache#40988 feedback): - build_action_headline() is the single owner of the record shape; the restore command no longer hand-rolls the dict (three-module string contract). - operation='announce' (new vocabulary member): a headline announces an action, it doesn't edit a field. The verb no longer rides in path — path is pure navigation per the ChangeRecord contract; renderers join kind='__meta__' with the transaction's action_kind. - The injector peeks instead of popping when the buffer is empty, and runs after the child-record append: a headline-only buffer no longer defeats the multi-flush short-circuit (which would have silently dropped a later flush's records once import/clone headlines arrive). - _BUFFER_KEY is cleared on the processed-tx early return and popped in both after_commit/after_rollback cleanups — buffered records can no longer leak into the next transaction under the wrong tx id. - Module docstring re-synced with the session keys it manages. - Test hygiene: the perm-rewrite probe restores the perm trio in a finally (security-routing state on a persistent DB), and the foreign-uuid test un-plants the shadow row it corrupts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 16, 2026
…ave marker, search Backend fixes for the client workarounds in PR apache#40988 (grep: 'backend workaround'): - __meta__ accommodation: add '__meta__' to ACTIVITY_CHANGE_KINDS and render 'restored to version N' summaries for the restore headline records the backend now emits; convert the long-standing AV-015 xfail into a passing assertion on the headline (the strict xfail had been masking an endpoint 404 — fixed separately as the restore-uuid bug). - first_tracked_save marker: every activity record says whether its transaction is the entity's FIRST tracked save (first op=1 after the retroactive baseline). A legacy chart's first Explore save replays ~74 params-normalization deltas; clients collapse such transactions instead of rendering each as a user edit. Matched on (id, uuid) against the live row so id reuse after a hard delete can't inherit a dead entity's history. - Server-side search: ?q= on /activity/ filters the FULL history (summary, entity name, kind, path, values, case-insensitive) before pagination, so count reflects matches — the panel's client-side search only covered loaded pages. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mikebridge
pushed a commit
to mikebridge/superset
that referenced
this pull request
Jun 16, 2026
…y, docs Multi-lens review follow-ups on the PR apache#40988 feedback work: - __meta__ headlines now render on SELF records (their primary surface — 'restored to version N' was previously built only for related records and the entity's own stream got summary=''), include the entity name on related streams, and dispatch on the transaction's action_kind instead of a verb smuggled into path. - operation vocabulary grows an honest member: headline records are 'announce', not a fudged 'edit'. - ?q= search matches values in their JSON form (the text clients render — 'false'/'null'/double-quoted keys) and no longer collapses falsy values (False/0) to unsearchable empty strings. - first_tracked_save documents its (id, uuid) live-row matching: always false for hard-deleted entities. - get_activity's count contract names the post-q semantics; UPDATING.md's activity section documents q, first_tracked_save, and the __meta__/announce record class. - Unit tests pin the headline summary branches and the falsy/JSON search semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
71355883b9onmikebridge:sc-107283-versioning-activity-view(the activity-view follow-up to the versioning backend PR #39603). Most commits in the diff belong to that backend work — this PR's own changes are ONLY the last 7 commits (71355883b9..931c4df51b, all prefixed(version-history)). It will be rebased ontomasteronce the backend lands. Kept as draft until then and until the manual browser walkthrough is complete.SUMMARY
Adds a version history UI for charts (Explore) and dashboards, entirely behind a new
VERSION_HISTORYfeature flag (default off).?version_history=1). Shows an activity timeline with client-side search, an include filter (All changes / This chart·dashboard only / Related items only), and "Load more" pagination.SuperChartthat never touches the live Explore redux state; dashboards re-hydrate from the snapshot (live charts) and restore the live state on close. A banner shows the previewed version with Restore / Open as new / Close actions; page actions are disabled while previewing.POST .../versions/{uuid}/restore, success/failure toasts, and in-place rehydration of the page afterwards (Explore re-fetches the explore payload and re-queries; Dashboard re-hydrates fresh data).BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Will be added during the manual walkthrough (PR is draft until then).
TESTING INSTRUCTIONS
VERSION_HISTORYfeature flag (requires the stacked versioning backend).Automated coverage:
npm run typepasses with zero errors.ADDITIONAL INFORMATION
VERSION_HISTORYCC @mikebridge