codeant-ai-for-open-source[bot] commented on code in PR #41076: URL: https://github.com/apache/superset/pull/41076#discussion_r3560195187
########## superset/versioning/activity/windows.py: ########## @@ -0,0 +1,108 @@ +# 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. +"""Pure window arithmetic on half-open ``[start_tx, end_tx)`` intervals. + +Extracted from the DB-touching scope resolution so that: + +* :mod:`scope` (DB-touching) can import this module at module-top. +* :mod:`queries.fetch_change_records` can import + :func:`row_within_any_window` at module-top instead of through a + lazy import that previously dodged a ``scope ↔ queries`` cycle. + +Everything here is pure Python — no DB, no Flask. ``end_tx = None`` +means "open-ended (current)" and behaves like positive infinity. +""" + +from __future__ import annotations + +from typing import Any + +from superset.versioning.activity.kinds import EntityWindows, Window + + +def intersect_windows(outer: Window, inner: Window) -> Window | None: + """Intersect two half-open ``[start_tx, end_tx)`` windows. + + Returns the clipped overlap, or ``None`` when they are disjoint. + ``end_tx = None`` means "open ended (current)" and acts like + positive infinity. Thin wrapper over :meth:`Window.intersect` — + kept as a free function so callers and tests don't have to migrate + to method form in lockstep with the dataclass promotion. + """ + return outer.intersect(inner) + + +def row_within_any_window(row: dict[str, Any], windows: list[Window]) -> bool: + """``True`` iff ``row['transaction_id']`` falls inside at least one + of *windows*. Half-open interval semantics match + :func:`intersect_windows`.""" + if not windows: + return False + tx_id = row["transaction_id"] + return any(w.contains(tx_id) for w in windows) + + +def merge_entity_windows(scope: list[EntityWindows]) -> list[EntityWindows]: + """Collapse repeated ``(api_kind, entity_id)`` entries by unioning + their window lists, and collapse overlapping/touching windows + within each entity into one. + + The OR-clause in + :func:`~superset.versioning.activity.queries.fetch_change_records` + generates one branch per (kind, id, window) tuple. Without the + within-entity union, a chart that's been attached-and-detached + many times (or that repeated fixture loads have populated the M2M + shadow for) yields a separate clause per redundant window — at + ~10 entities × ~50 windows the SQL hits SQLite's + ``SQLITE_MAX_EXPR_DEPTH`` (1000). Merging here keeps the clause + count proportional to the number of *distinct* validity intervals, + not the number of shadow rows. + """ + merged: dict[tuple[str, int], list[Window]] = {} + for api_kind, entity_id, windows in scope: + merged.setdefault((api_kind, entity_id), []).extend(windows) + return [ + (api_kind, entity_id, union_windows(windows)) + for (api_kind, entity_id), windows in merged.items() + ] + + +def union_windows(windows: list[Window]) -> list[Window]: + """Sort + merge overlapping/touching half-open intervals. + + Pure function — no DB. Touching ``[a, b)`` and ``[b, c)`` merge into + ``[a, c)``. ``end_tx = None`` (open-ended) absorbs everything to its + right. Returns a minimal disjoint cover of the input set. + """ + if not windows: + return [] + sorted_windows = sorted(windows, key=lambda w: w.start_tx) Review Comment: **Suggestion:** Add a concrete type annotation for this derived windows collection so the new function keeps explicit typing for key intermediate variables. [custom_rule] **Severity Level:** Minor 🧹 <details> <summary><b>Why it matters? ⭐ </b></summary> This intermediate collection is a relevant local variable in newly added Python code and can be explicitly annotated as list[Window]. The custom rule flags new or modified Python code that omits type hints on relevant variables that can be annotated, so the suggestion matches a real violation. </details> <details> <summary><b>Rule source 📖 </b></summary> .cursor/rules/dev-standard.mdc (line 28) </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=09a904a333c74b93972f997b2863f50b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=09a904a333c74b93972f997b2863f50b&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/versioning/activity/windows.py **Line:** 93:93 **Comment:** *Custom Rule: Add a concrete type annotation for this derived windows collection so the new function keeps explicit typing for key intermediate variables. 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%2F41076&comment_hash=fd1a252863ba262f0e948deed0a327cbffed0a3042b144b242c2648b3dbdb04b&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=fd1a252863ba262f0e948deed0a327cbffed0a3042b144b242c2648b3dbdb04b&reaction=dislike'>👎</a> ########## superset/versioning/activity/orchestrator.py: ########## @@ -0,0 +1,434 @@ +# 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. +"""Top-level orchestrator + query-param parsing + observability. + +This is the public entry point for the activity-view read path. One +function — :func:`get_activity` — dispatches on the path entity's +model class to assemble the cross-entity activity stream: + +1. ``resolve_path_entity`` (queries.py) — resolve UUID → live entity. +2. ``resolve_scope`` (scope.py) — build the related-entity window list. +3. ``fetch_change_records`` (queries.py) — pull rows from + ``version_changes`` joined with ``version_transaction`` and ``ab_user``. +4. ``filter_records_by_visibility`` (visibility.py) — silent + drop of records the requester can't read. +5. ``apply_entity_name_denormalization`` (queries.py) — resolve entity names + from the shadow row valid at each record's transaction_id. +6. ``apply_record_decoration`` (render.py) — synthesize the ActivityRecord + DTO fields and strip internal-only columns. +7. Paginate in Python over the post-filter list. + +Parameter parsing for the REST endpoints lives here too — +:func:`parse_activity_query_params` is called by the three +``/activity/`` endpoint handlers before they call ``get_activity``. +Same for the observability instrumentation: ``_phase_timer`` and +``_emit_request_shape_attributes`` emit the per-phase timing and +request-shape metrics, on the same prefix the cross-coupling test pins. +""" + +from __future__ import annotations + +import contextlib +from collections.abc import Iterator +from datetime import datetime, timezone +from typing import Any +from uuid import UUID + +from flask import Response +from flask_appbuilder import Model + +from superset.utils import json +from superset.versioning.activity.kinds import EntityWindows +from superset.versioning.activity.queries import ( + apply_entity_name_denormalization, + fetch_change_records, + first_tracked_tx, + mark_first_tracked_saves, + resolve_path_entity, +) +from superset.versioning.activity.render import apply_record_decoration +from superset.versioning.activity.scope import resolve_scope +from superset.versioning.activity.visibility import filter_records_by_visibility +from superset.versioning.api_helpers import ( + PathEntityResponseError, + resolve_endpoint_path_entity, +) + +_DEFAULT_PAGE_SIZE = 25 +_MAX_PAGE_SIZE = 200 +_VALID_INCLUDE_VALUES: frozenset[str] = frozenset({"self", "related", "all"}) + + +# Upper bound on the ``q`` search string. The search is a substring scan over +# the (already-capped) materialized record set, so this is a cheap-DoS guard, +# not a correctness limit. +_MAX_Q_LENGTH = 1024 Review Comment: **Suggestion:** Add explicit type annotations to these new module-level constants to satisfy the strict type-hinting rule for relevant variables. [custom_rule] **Severity Level:** Minor 🧹 <details> <summary><b>Why it matters? ⭐ </b></summary> These new module-level constants are plain assignments without type annotations, and they are simple scalar values that can be annotated. This matches the rule requiring type hints for relevant new or modified Python variables. </details> <details> <summary><b>Rule source 📖 </b></summary> .cursor/rules/dev-standard.mdc (line 28) </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=287864ef3be7455aa1c6c20addbada31&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=287864ef3be7455aa1c6c20addbada31&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/versioning/activity/orchestrator.py **Line:** 71:79 **Comment:** *Custom Rule: Add explicit type annotations to these new module-level constants to satisfy the strict type-hinting rule for relevant variables. 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%2F41076&comment_hash=0aa50aaad7536d989b39e89df1dc66c7e0c970d3b38cce27131041420321cd05&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=0aa50aaad7536d989b39e89df1dc66c7e0c970d3b38cce27131041420321cd05&reaction=dislike'>👎</a> ########## superset/versioning/changes/listener.py: ########## @@ -198,6 +198,29 @@ def build_action_headline( # is correctly deduped. _REGISTERED_SENTINEL = "_versioning_change_listener_registered" +#: Metric namespace for swallowed capture-path failures. The capture +#: listeners fail open (a versioning bug must never break a user's save), +#: so the read path (``activity/orchestrator``) is richly instrumented but +#: the write path historically logged-and-swallowed with no counter. Each +#: ``_incr_capture_error(stage)`` emits ``<prefix>.<stage>.error`` so a +#: systematic capture regression is alertable rather than log-grep-only. +_CAPTURE_METRIC_PREFIX = "superset.versioning.capture" Review Comment: **Suggestion:** Add an explicit type annotation to this new module-level constant to satisfy the type-hint requirement for annotatable variables. [custom_rule] **Severity Level:** Minor 🧹 <details> <summary><b>Why it matters? ⭐ </b></summary> This is a new module-level variable in Python and it is annotatable, but the code omits a type hint. That matches the llmcfg-require-python-type-hints rule. </details> <details> <summary><b>Rule source 📖 </b></summary> .cursor/rules/dev-standard.mdc (line 28) </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=aae8c1f98f8f40d9af2fd564a0dd8881&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=aae8c1f98f8f40d9af2fd564a0dd8881&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/versioning/changes/listener.py **Line:** 207:207 **Comment:** *Custom Rule: Add an explicit type annotation to this new module-level constant to satisfy the type-hint requirement for annotatable variables. 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%2F41076&comment_hash=edc8304bdf3a37835f8654736a2dde9e67411c9cf73465024bc7787c318da81e&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=edc8304bdf3a37835f8654736a2dde9e67411c9cf73465024bc7787c318da81e&reaction=dislike'>👎</a> ########## superset/versioning/activity/windows.py: ########## @@ -0,0 +1,108 @@ +# 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. +"""Pure window arithmetic on half-open ``[start_tx, end_tx)`` intervals. + +Extracted from the DB-touching scope resolution so that: + +* :mod:`scope` (DB-touching) can import this module at module-top. +* :mod:`queries.fetch_change_records` can import + :func:`row_within_any_window` at module-top instead of through a + lazy import that previously dodged a ``scope ↔ queries`` cycle. + +Everything here is pure Python — no DB, no Flask. ``end_tx = None`` +means "open-ended (current)" and behaves like positive infinity. +""" + +from __future__ import annotations + +from typing import Any + +from superset.versioning.activity.kinds import EntityWindows, Window + + +def intersect_windows(outer: Window, inner: Window) -> Window | None: + """Intersect two half-open ``[start_tx, end_tx)`` windows. + + Returns the clipped overlap, or ``None`` when they are disjoint. + ``end_tx = None`` means "open ended (current)" and acts like + positive infinity. Thin wrapper over :meth:`Window.intersect` — + kept as a free function so callers and tests don't have to migrate + to method form in lockstep with the dataclass promotion. + """ + return outer.intersect(inner) + + +def row_within_any_window(row: dict[str, Any], windows: list[Window]) -> bool: + """``True`` iff ``row['transaction_id']`` falls inside at least one + of *windows*. Half-open interval semantics match + :func:`intersect_windows`.""" + if not windows: + return False + tx_id = row["transaction_id"] Review Comment: **Suggestion:** Add an explicit type annotation for this local variable to keep relevant values typed instead of leaving them inferred from an Any-typed dictionary lookup. [custom_rule] **Severity Level:** Minor 🧹 <details> <summary><b>Why it matters? ⭐ </b></summary> The function already has precise types for its parameters, and this local value is a relevant variable derived from a dictionary lookup that can be annotated as an int. The custom rule requires type hints on relevant variables that can be annotated, so this is a real violation. </details> <details> <summary><b>Rule source 📖 </b></summary> .cursor/rules/dev-standard.mdc (line 28) </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d7e93b8dfd4e4733b5cc3343d6c2fa20&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d7e93b8dfd4e4733b5cc3343d6c2fa20&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/versioning/activity/windows.py **Line:** 55:55 **Comment:** *Custom Rule: Add an explicit type annotation for this local variable to keep relevant values typed instead of leaving them inferred from an Any-typed dictionary lookup. 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%2F41076&comment_hash=8876267b94993b9d42b8167997efe9d732813cb95f0da5095f2601c526fa6bf4&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=8876267b94993b9d42b8167997efe9d732813cb95f0da5095f2601c526fa6bf4&reaction=dislike'>👎</a> ########## tests/integration_tests/versioning/change_records_tests.py: ########## @@ -0,0 +1,715 @@ +# 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. +"""Integration tests for ``version_changes`` capture (T052, partial). + +Covers in this file: + (a) saving a chart with three field changes produces three rows + (f) baseline / INSERT transactions produce zero records *for that entity* + + unchanged-save / dashboard / params-classification cases + +Deferred: + (b) ``GET /versions/`` response includes ``changes`` array — lands with + T050 (API integration). + (c) FK cascade — exercisable in principle (the migration declares + ``ON DELETE CASCADE``) but can't be isolated in a unit-style test + because ``version_transaction`` is referenced by non-cascading FKs + from slices_version / dashboards_version / etc. Covered instead + by (d) below once it lands, and by the structural declaration in + T046's migration. + (d) retention prune drops change records alongside the pruned + version — will land when T049 extends ``VersionDAO.prune_versions`` + to include ``version_changes`` alongside the shadow-row delete. + (e) ``kind`` index query plan on Postgres — deferred to T053 perf + validation. +""" + +from __future__ import annotations + +from datetime import datetime, timedelta +from typing import Any +from uuid import uuid4 + +import pytest +import sqlalchemy as sa +from sqlalchemy_continuum import version_class + +from superset.extensions import db +from superset.models.dashboard import Dashboard +from superset.models.slice import Slice +from superset.utils import json as _json +from tests.integration_tests.base_tests import SupersetTestCase +from tests.integration_tests.fixtures.birth_names_dashboard import ( # noqa: F401 + load_birth_names_dashboard_with_slices, + load_birth_names_data, +) + +_VERSION_CHANGES = sa.table( + "version_changes", + sa.column("id"), + sa.column("transaction_id"), + sa.column("entity_kind"), + sa.column("entity_id"), + sa.column("sequence"), + sa.column("kind"), + sa.column("operation"), + sa.column("path"), + sa.column("from_value"), + sa.column("to_value"), +) + +_VERSION_TRANSACTION = sa.table( + "version_transaction", + sa.column("id"), + sa.column("issued_at"), + sa.column("user_id"), + sa.column("action_kind"), +) + + +def _action_kind_for(tx_id: int) -> str | None: + """Read the ``action_kind`` column from the version_transaction row.""" + return ( + db.session.connection() + .execute( + sa.select(_VERSION_TRANSACTION.c.action_kind).where( + _VERSION_TRANSACTION.c.id == tx_id + ) + ) + .scalar() + ) + + +def _change_rows_for( + tx_id: int, + *, + entity_kind: str | None = None, + entity_id: int | None = None, +) -> list[dict[str, Any]]: + """Raw fetch of ``version_changes`` rows for a tx + optional entity filter.""" + query = sa.select(_VERSION_CHANGES).where( + _VERSION_CHANGES.c.transaction_id == tx_id + ) + if entity_kind is not None: + query = query.where(_VERSION_CHANGES.c.entity_kind == entity_kind) + if entity_id is not None: + query = query.where(_VERSION_CHANGES.c.entity_id == entity_id) + query = query.order_by(_VERSION_CHANGES.c.sequence.asc()) + result = db.session.connection().execute(query) + return [dict(row._mapping) for row in result] + + +def _persist_fixture_state() -> None: + """Commit fixture INSERTs so the baseline row exists before the test edits. + + Without this, the test's first commit batches the fixture's pending + INSERTs with the test's UPDATE into a single Continuum transaction + and no diff records are emitted (no pre-state). + """ + db.session.commit() + + +class TestChartChangeRecords(SupersetTestCase): + """Change-record capture for chart (Slice) saves.""" + + @pytest.fixture(autouse=True) Review Comment: **Suggestion:** Add type annotations to this fixture helper method by annotating its fixture parameter and explicitly declaring a return type. [custom_rule] **Severity Level:** Minor 🧹 <details> <summary><b>Why it matters? ⭐ </b></summary> This new fixture helper omits type hints on both the `load_birth_names_dashboard_with_slices` parameter and the return type, which matches the custom rule requiring type annotations on new or modified Python functions and methods. </details> <details> <summary><b>Rule source 📖 </b></summary> .cursor/rules/dev-standard.mdc (line 28) </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=db48b054cc444d4c92297ef3e39e0383&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=db48b054cc444d4c92297ef3e39e0383&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** tests/integration_tests/versioning/change_records_tests.py **Line:** 128:128 **Comment:** *Custom Rule: Add type annotations to this fixture helper method by annotating its fixture parameter and explicitly declaring a return type. 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%2F41076&comment_hash=aa266c99ef3174fbe318c3c81cf8b2215f9a4dec81fc6a19d359e3e470de0ded&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=aa266c99ef3174fbe318c3c81cf8b2215f9a4dec81fc6a19d359e3e470de0ded&reaction=dislike'>👎</a> ########## tests/integration_tests/versioning/perf_validation_tests.py: ########## @@ -0,0 +1,416 @@ +# 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. +"""T044 — Performance validation for entity version history. + +Skipped by default. Run on demand: + + SUPERSET_PERF_VALIDATION=1 pytest \ + tests/integration_tests/versioning/perf_validation_tests.py -v -s + +Measures the three success criteria defined in the spec: + + * SC-002: version list endpoint responds in under 1 second + * SC-004: save path p95 overhead under 50 ms with Continuum tracking + on vs. off (FR-014) + +(SC-003, the restore-endpoint benchmark, lands with the restore endpoint +in a later PR.) + +The test prints a summary table suitable for pasting into the PR +description. It also asserts each target so regressions fail loudly +when the harness is re-run. +""" + +from __future__ import annotations + +import os +import statistics +import time +from typing import Any + +import pytest +import sqlalchemy as sa +from sqlalchemy_continuum import version_class, versioning_manager + +from superset.extensions import db +from superset.models.slice import Slice +from tests.integration_tests.base_tests import SupersetTestCase +from tests.integration_tests.constants import ADMIN_USERNAME +from tests.integration_tests.fixtures.birth_names_dashboard import ( # noqa: F401 + load_birth_names_dashboard_with_slices, + load_birth_names_data, +) + +SKIP_REASON = "Performance validation is manual. Set SUPERSET_PERF_VALIDATION=1 to run." + +# Thresholds from spec.md §Success Criteria. +LIST_ENDPOINT_MAX_MS = 1000 # SC-002 +SAVE_OVERHEAD_P95_MAX_MS = 50 # SC-004 + +# Activity-view thresholds (sc-107283 §Success Criteria). +ACTIVITY_ENDPOINT_P95_MAX_MS = 1500 # SC-AV-001 + + +def _save_chart_once(chart: Slice, suffix: str) -> None: + """One ORM-level save path, mimicking what ChartDAO.update does.""" + chart.slice_name = f"{chart.slice_name[:64]}_{suffix}" + db.session.commit() + + +def _timings_ms(seconds: list[float]) -> dict[str, float]: + ms = sorted(s * 1000.0 for s in seconds) + return { + "p50": statistics.median(ms), + "p95": ms[int(len(ms) * 0.95) - 1] if len(ms) >= 20 else max(ms), + "max": max(ms), + "n": len(ms), + } + + [email protected]( + not os.environ.get("SUPERSET_PERF_VALIDATION"), + reason=SKIP_REASON, +) +class PerfValidationTests(SupersetTestCase): + """Runs only when SUPERSET_PERF_VALIDATION=1 is set.""" + + @pytest.fixture(autouse=True) + def _load_data(self, load_birth_names_dashboard_with_slices: Any) -> None: # noqa: F811, PT004 + pass + + def _seed_chart_with_n_versions(self, n: int) -> Slice: + """Save a chart N times to produce N version rows.""" + chart = db.session.query(Slice).first() + assert chart is not None, "birth_names fixture should provide charts" + + for i in range(n): + _save_chart_once(chart, f"v{i}") + db.session.commit() + return chart + + def test_sc002_list_endpoint_under_1s(self) -> None: + """SC-002: list endpoint responds in under 1 second.""" + self.login(ADMIN_USERNAME) + + # Generate enough versions to exercise the retention-capped state. + chart = self._seed_chart_with_n_versions(24) + chart_uuid = str(chart.uuid) + url = f"/api/v1/chart/{chart_uuid}/versions/" + + # Warm up the endpoint once (JIT caching, mapper configuration, etc.) + self.client.get(url) + + timings: list[float] = [] + for _ in range(10): + t0 = time.perf_counter() + response = self.client.get(url) + timings.append(time.perf_counter() - t0) + assert response.status_code == 200 + + stats = _timings_ms(timings) + print( + f"\n[SC-002] GET /versions/ (24 versions) " + f"p50={stats['p50']:.1f}ms p95={stats['p95']:.1f}ms " + f"max={stats['max']:.1f}ms n={stats['n']}" + ) + assert stats["p95"] < LIST_ENDPOINT_MAX_MS, ( + f"SC-002 failed: list endpoint p95 {stats['p95']:.1f}ms " + f">= {LIST_ENDPOINT_MAX_MS}ms" + ) + + # SC-003 (restore endpoint under 3s) intentionally omitted here: the + # version-restore route (POST /<uuid>/versions/<version_uuid>/restore) + # ships in a later PR, so there is nothing to time at this point in the + # stack. Re-add this benchmark alongside the restore endpoint. + + def test_sc004_save_overhead_under_50ms(self) -> None: + """SC-004: save path p95 overhead under 50ms (FR-014). + + Toggling Continuum on and off mid-process corrupts its internal + ``units_of_work`` state and is not a reliable measurement. Instead + this test directly measures the wall-clock time spent inside the + four session-level listeners Continuum attaches to + ``sa.orm.session.Session`` — ``before_flush``, ``after_flush``, + ``after_commit``, ``after_rollback`` — plus Superset's own + baseline / snapshot / retention-prune listeners (attached to + ``db.session``). The cumulative listener time per save is the + marginal overhead version capture adds over a save with + versioning removed entirely, because without these listeners + the ORM would not execute any of that code. + + The approach: + 1. Wrap each known listener with a timing proxy that adds its + wall-clock time to a per-save accumulator. + 2. Save the same chart N times, recording each save's + accumulator value. + 3. Compute p50 / p95 of the per-save overhead. + + This matches the measurement intent of SC-004 (how much does + versioning cost per save) without the fragility of toggling + Continuum mid-test. + """ + self.login(ADMIN_USERNAME) + + chart = db.session.query(Slice).first() + assert chart is not None + + # Per-save accumulator incremented by the wrapped listeners. + acc = [0.0] + + def wrap_listener(original: Any) -> Any: + def wrapper(*args: Any, **kwargs: Any) -> Any: + t0 = time.perf_counter() + try: + return original(*args, **kwargs) + finally: + acc[0] += time.perf_counter() - t0 + + wrapper.__wrapped__ = original # type: ignore[attr-defined] + return wrapper + + # Instrument Continuum's four session listeners by detaching the + # bound method, wrapping, and re-attaching under a single-use + # listener handle we can cleanly remove on teardown. + session_target = sa.orm.session.Session + attached: list[tuple[str, Any]] = [] + for event_name, listener in list(versioning_manager.session_listeners.items()): + sa.event.remove(session_target, event_name, listener) + wrapped = wrap_listener(listener) + sa.event.listen(session_target, event_name, wrapped) + attached.append((event_name, wrapped)) + + iterations = 100 + warmup = 5 + try: + # Warmup (first baseline INSERT, JIT, cache warming). + for i in range(warmup): + _save_chart_once(chart, f"warm_{i}") + acc[0] = 0.0 + + total_timings: list[float] = [] + overhead_timings: list[float] = [] + for i in range(iterations): + acc[0] = 0.0 + t0 = time.perf_counter() + _save_chart_once(chart, f"run_{i}") + total_timings.append(time.perf_counter() - t0) + overhead_timings.append(acc[0]) + finally: + for event_name, wrapped in attached: + sa.event.remove(session_target, event_name, wrapped) + sa.event.listen( + session_target, + event_name, + wrapped.__wrapped__, + ) + + total = _timings_ms(total_timings) + overhead = _timings_ms(overhead_timings) + + ver_cls = version_class(Slice) + produced = db.session.query(ver_cls).filter(ver_cls.id == chart.id).count() + print( + f"\n[SC-004] save iterations={iterations} chart_id={chart.id} " + f"version_rows_produced={produced}" + ) + print( + f"[SC-004] full save: " + f"p50={total['p50']:.2f}ms p95={total['p95']:.2f}ms " + f"max={total['max']:.2f}ms" + ) + print( + f"[SC-004] version-cap overhead: " + f"p50={overhead['p50']:.2f}ms p95={overhead['p95']:.2f}ms " + f"max={overhead['max']:.2f}ms" + ) + + assert overhead["p95"] < SAVE_OVERHEAD_P95_MAX_MS, ( + f"SC-004 failed: version-capture p95 overhead " + f"{overhead['p95']:.2f}ms >= {SAVE_OVERHEAD_P95_MAX_MS}ms" + ) + + # ---- T045: Activity-view perf validation ----------------------------- + + def _seed_activity_history(self) -> str: + """Generate dense history on the birth_names dashboard so the + activity endpoint has something realistic to read. + + T045's spec target is "25 charts × 3 dataset windows each". The + birth_names fixture has ~12 charts on a single dataset (no + multi-dataset support without a bespoke fixture). We approximate + the load by: (a) editing many charts on the dashboard, (b) + editing the dataset's description several times, (c) editing the + dashboard's own title once. That yields ~30+ change records + spanning all three entity kinds — enough to exercise the + decoration, visibility, and impact-batch paths without needing a + multi-dataset fixture builder. Returns the dashboard UUID. + + **Why this commits without rollback** (unlike the test bodies in + ``activity_view_tests.py``): the whole point of a perf seed is + that the rows it produces have actually been persisted, so the + endpoint hit that follows reads a realistic state of the + ``version_changes`` / shadow tables. T053's + ``try/finally``+``rollback`` convention is for tests that + assert on *which records were captured*; here the seed IS the + setup, not the unit under test. The fixture's session-scoped + ``_cleanup`` removes the dashboard / slices at session teardown, + which is when the shadow rows age out too. + """ + # pylint: disable=import-outside-toplevel + from superset.connectors.sqla.models import SqlaTable + from superset.models.dashboard import Dashboard + + dashboard = ( + db.session.query(Dashboard) + .filter(Dashboard.dashboard_title.like("USA Births%")) + .first() + ) Review Comment: **Suggestion:** Add an explicit optional model type annotation to this query result variable. [custom_rule] **Severity Level:** Minor 🧹 <details> <summary><b>Why it matters? ⭐ </b></summary> The query result is assigned to a variable without an explicit type hint even though it is potentially nullable. This is a relevant variable that can be annotated, so the rule violation is present. </details> <details> <summary><b>Rule source 📖 </b></summary> .cursor/rules/dev-standard.mdc (line 28) </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=adc9129b353249fb9d9c69baa498d2be&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=adc9129b353249fb9d9c69baa498d2be&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** tests/integration_tests/versioning/perf_validation_tests.py **Line:** 277:281 **Comment:** *Custom Rule: Add an explicit optional model type annotation to this query result variable. 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%2F41076&comment_hash=30070faa7840fc835c91aa4c225f47debf8ed197646206172cf732959eed76a7&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=30070faa7840fc835c91aa4c225f47debf8ed197646206172cf732959eed76a7&reaction=dislike'>👎</a> ########## tests/integration_tests/versioning/activity_view_tests.py: ########## @@ -0,0 +1,1250 @@ +# 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. +"""Integration tests for the cross-entity activity-view API (sc-107283). + +US1 — dashboard activity stream: ``GET /api/v1/dashboard/<uuid>/activity/``. +Tests for US2 (chart activity) and US3 (dataset activity) come in later +phases. + +Per spec T053 / sc-103156 T062, every test that mutates a fixture entity +wraps the test body in ``try``/``finally`` with +``metadata_db.session.rollback()`` in the ``finally``. The rationale is +documented in the spec — Continuum captures dirty mappers during +autoflush, so leaving an instrumented attribute dirty pollutes +downstream tests via the shadow tables. +""" + +from __future__ import annotations + +from typing import Any +from uuid import uuid4 + +import pytest + +from superset.connectors.sqla.models import SqlaTable +from superset.extensions import db +from superset.models.dashboard import Dashboard +from superset.models.slice import Slice +from superset.utils import json as _json +from tests.integration_tests.base_tests import SupersetTestCase +from tests.integration_tests.constants import ( + ADMIN_USERNAME, + ALPHA_USERNAME, + GAMMA_USERNAME, +) +from tests.integration_tests.fixtures.birth_names_dashboard import ( # noqa: F401 + load_birth_names_dashboard_with_slices, + load_birth_names_data, +) + + +def _get_birth_names_dataset() -> SqlaTable: + return ( + db.session.query(SqlaTable) + .filter(SqlaTable.table_name == "birth_names") + .first() + ) + + +def _persist_fixture_state() -> None: + """Force the fixture's pending INSERTs to commit so subsequent edits + produce *new* version rows instead of being batched into the + creation transaction. Mirrors the same helper in + ``tests/integration_tests/dashboards/version_history_tests.py``. + """ + db.session.commit() + + +def _get_birth_names_dashboard() -> Dashboard: + return ( + db.session.query(Dashboard) + .filter(Dashboard.dashboard_title == "USA Births Names") + .first() + ) + + +class TestDashboardActivityView(SupersetTestCase): + """T017–T026 — ``GET /api/v1/dashboard/<uuid>/activity/`` (US1).""" + + @pytest.fixture(autouse=True) Review Comment: **Suggestion:** Add explicit type hints to this fixture method by annotating the fixture parameter and the method return type. [custom_rule] **Severity Level:** Minor 🧹 <details> <summary><b>Why it matters? ⭐ </b></summary> This fixture method omits type hints for its parameter and return type, which matches the custom rule requiring explicit Python type hints on functions/methods where they can be annotated. </details> <details> <summary><b>Rule source 📖 </b></summary> .cursor/rules/dev-standard.mdc (line 28) </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=65e5db8a3c364965bb385aff4f58a28e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=65e5db8a3c364965bb385aff4f58a28e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** tests/integration_tests/versioning/activity_view_tests.py **Line:** 83:83 **Comment:** *Custom Rule: Add explicit type hints to this fixture method by annotating the fixture parameter and the method return type. 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%2F41076&comment_hash=1405618e2b3a3e9e9de2baf99f8c5c43078e881cd6b9c46c58e5f27028ca8224&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=1405618e2b3a3e9e9de2baf99f8c5c43078e881cd6b9c46c58e5f27028ca8224&reaction=dislike'>👎</a> ########## tests/integration_tests/versioning/perf_validation_tests.py: ########## @@ -0,0 +1,416 @@ +# 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. +"""T044 — Performance validation for entity version history. + +Skipped by default. Run on demand: + + SUPERSET_PERF_VALIDATION=1 pytest \ + tests/integration_tests/versioning/perf_validation_tests.py -v -s + +Measures the three success criteria defined in the spec: + + * SC-002: version list endpoint responds in under 1 second + * SC-004: save path p95 overhead under 50 ms with Continuum tracking + on vs. off (FR-014) + +(SC-003, the restore-endpoint benchmark, lands with the restore endpoint +in a later PR.) + +The test prints a summary table suitable for pasting into the PR +description. It also asserts each target so regressions fail loudly +when the harness is re-run. +""" + +from __future__ import annotations + +import os +import statistics +import time +from typing import Any + +import pytest +import sqlalchemy as sa +from sqlalchemy_continuum import version_class, versioning_manager + +from superset.extensions import db +from superset.models.slice import Slice +from tests.integration_tests.base_tests import SupersetTestCase +from tests.integration_tests.constants import ADMIN_USERNAME +from tests.integration_tests.fixtures.birth_names_dashboard import ( # noqa: F401 + load_birth_names_dashboard_with_slices, + load_birth_names_data, +) + +SKIP_REASON = "Performance validation is manual. Set SUPERSET_PERF_VALIDATION=1 to run." + +# Thresholds from spec.md §Success Criteria. +LIST_ENDPOINT_MAX_MS = 1000 # SC-002 +SAVE_OVERHEAD_P95_MAX_MS = 50 # SC-004 + +# Activity-view thresholds (sc-107283 §Success Criteria). +ACTIVITY_ENDPOINT_P95_MAX_MS = 1500 # SC-AV-001 + + +def _save_chart_once(chart: Slice, suffix: str) -> None: + """One ORM-level save path, mimicking what ChartDAO.update does.""" + chart.slice_name = f"{chart.slice_name[:64]}_{suffix}" + db.session.commit() + + +def _timings_ms(seconds: list[float]) -> dict[str, float]: + ms = sorted(s * 1000.0 for s in seconds) + return { + "p50": statistics.median(ms), + "p95": ms[int(len(ms) * 0.95) - 1] if len(ms) >= 20 else max(ms), + "max": max(ms), + "n": len(ms), + } + + [email protected]( + not os.environ.get("SUPERSET_PERF_VALIDATION"), + reason=SKIP_REASON, +) +class PerfValidationTests(SupersetTestCase): + """Runs only when SUPERSET_PERF_VALIDATION=1 is set.""" + + @pytest.fixture(autouse=True) + def _load_data(self, load_birth_names_dashboard_with_slices: Any) -> None: # noqa: F811, PT004 + pass + + def _seed_chart_with_n_versions(self, n: int) -> Slice: + """Save a chart N times to produce N version rows.""" + chart = db.session.query(Slice).first() + assert chart is not None, "birth_names fixture should provide charts" + + for i in range(n): + _save_chart_once(chart, f"v{i}") + db.session.commit() + return chart + + def test_sc002_list_endpoint_under_1s(self) -> None: + """SC-002: list endpoint responds in under 1 second.""" + self.login(ADMIN_USERNAME) + + # Generate enough versions to exercise the retention-capped state. + chart = self._seed_chart_with_n_versions(24) + chart_uuid = str(chart.uuid) + url = f"/api/v1/chart/{chart_uuid}/versions/" + + # Warm up the endpoint once (JIT caching, mapper configuration, etc.) + self.client.get(url) + + timings: list[float] = [] + for _ in range(10): + t0 = time.perf_counter() + response = self.client.get(url) + timings.append(time.perf_counter() - t0) + assert response.status_code == 200 + + stats = _timings_ms(timings) + print( + f"\n[SC-002] GET /versions/ (24 versions) " + f"p50={stats['p50']:.1f}ms p95={stats['p95']:.1f}ms " + f"max={stats['max']:.1f}ms n={stats['n']}" + ) + assert stats["p95"] < LIST_ENDPOINT_MAX_MS, ( + f"SC-002 failed: list endpoint p95 {stats['p95']:.1f}ms " + f">= {LIST_ENDPOINT_MAX_MS}ms" + ) + + # SC-003 (restore endpoint under 3s) intentionally omitted here: the + # version-restore route (POST /<uuid>/versions/<version_uuid>/restore) + # ships in a later PR, so there is nothing to time at this point in the + # stack. Re-add this benchmark alongside the restore endpoint. + + def test_sc004_save_overhead_under_50ms(self) -> None: + """SC-004: save path p95 overhead under 50ms (FR-014). + + Toggling Continuum on and off mid-process corrupts its internal + ``units_of_work`` state and is not a reliable measurement. Instead + this test directly measures the wall-clock time spent inside the + four session-level listeners Continuum attaches to + ``sa.orm.session.Session`` — ``before_flush``, ``after_flush``, + ``after_commit``, ``after_rollback`` — plus Superset's own + baseline / snapshot / retention-prune listeners (attached to + ``db.session``). The cumulative listener time per save is the + marginal overhead version capture adds over a save with + versioning removed entirely, because without these listeners + the ORM would not execute any of that code. + + The approach: + 1. Wrap each known listener with a timing proxy that adds its + wall-clock time to a per-save accumulator. + 2. Save the same chart N times, recording each save's + accumulator value. + 3. Compute p50 / p95 of the per-save overhead. + + This matches the measurement intent of SC-004 (how much does + versioning cost per save) without the fragility of toggling + Continuum mid-test. + """ + self.login(ADMIN_USERNAME) + + chart = db.session.query(Slice).first() + assert chart is not None + + # Per-save accumulator incremented by the wrapped listeners. + acc = [0.0] Review Comment: **Suggestion:** Add an explicit type annotation for this local accumulator variable to satisfy the typing rule for relevant variables. [custom_rule] **Severity Level:** Minor 🧹 <details> <summary><b>Why it matters? ⭐ </b></summary> This local accumulator is a relevant variable that can be annotated, but it is created without a type hint. That matches the rule requiring type hints on relevant Python variables. </details> <details> <summary><b>Rule source 📖 </b></summary> .cursor/rules/dev-standard.mdc (line 28) </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e80fdd1c30f0405aa9654d16e9ec9a0d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e80fdd1c30f0405aa9654d16e9ec9a0d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** tests/integration_tests/versioning/perf_validation_tests.py **Line:** 172:172 **Comment:** *Custom Rule: Add an explicit type annotation for this local accumulator variable to satisfy the typing rule for relevant variables. 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%2F41076&comment_hash=bab4f013ce2c2ca29ea4d61d14287e16b6def7e610c8e47d5dc6f83b7dfe2ef6&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41076&comment_hash=bab4f013ce2c2ca29ea4d61d14287e16b6def7e610c8e47d5dc6f83b7dfe2ef6&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]
