BUG-HUNT: [migration] d0_001_changeset_artifacts downgrade drops tables without dropping named indexes — violates downgrade symmetry #6655

Open
opened 2026-04-09 22:46:57 +00:00 by HAL9000 · 0 comments
Owner

Bug Report: [migration] d0_001_changeset_artifacts Downgrade Drops Tables Without Explicitly Dropping Indexes

Severity Assessment

  • Impact: On most database backends (SQLite, PostgreSQL, MySQL), DROP TABLE automatically removes associated indexes, so this works in practice. However, it violates the principle of downgrade symmetry — every create_index in upgrade() should have a corresponding drop_index in downgrade(). In some edge cases (e.g., shared/global indexes, certain PostgreSQL configurations, or future refactors that split the drop_table into ALTER TABLE operations), the missing drop_index calls could cause errors. It also makes the migration harder to reason about and audit.
  • Likelihood: Low for immediate production failures; Medium for future refactor hazards.
  • Priority: Medium

Location

  • File: alembic/versions/d0_001_changeset_artifacts.py
  • Function: downgrade()
  • Lines: ~100–104

Description

The upgrade() function creates 6 named indexes across two tables:

# upgrade() creates:
op.create_index("ix_changeset_entries_changeset_id", "changeset_entries", ["changeset_id"])
op.create_index("ix_changeset_entries_plan_id", "changeset_entries", ["plan_id"])
op.create_index("ix_changeset_entries_path", "changeset_entries", ["path"])

op.create_index("ix_tool_invocations_changeset_id", "tool_invocations", ["changeset_id"])
op.create_index("ix_tool_invocations_plan_id", "tool_invocations", ["plan_id"])
op.create_index("ix_tool_invocations_tool_name", "tool_invocations", ["tool_name"])

The downgrade() function drops the tables without dropping the indexes first:

def downgrade() -> None:
    """Drop changeset_entries and tool_invocations tables."""
    op.drop_table("tool_invocations")    # ← drops 3 indexes implicitly
    op.drop_table("changeset_entries")   # ← drops 3 indexes implicitly

Compare with well-formed migrations in the same codebase (e.g., 001_initial_schema.py, a5_003_spec_aligned_actions.py) which explicitly drop each index before dropping the table:

# Correct pattern from 001_initial_schema.py:
op.drop_index(op.f("ix_changes_plan_id"), table_name="changes")
op.drop_index(op.f("ix_changes_applied"), table_name="changes")
op.drop_table("changes")

The inconsistency means:

  1. Alembic's --autogenerate may detect schema drift (finding "extra" indexes if it compares metadata to an in-progress downgrade).
  2. Future refactors that change drop_table to individual alter_column or partial reversal logic will break silently.
  3. Code review and auditing tools that verify downgrade symmetry will flag this migration.

Expected Behavior

downgrade() should explicitly drop all 6 named indexes before dropping the tables, mirroring the upgrade operations in reverse:

def downgrade() -> None:
    """Drop changeset_entries and tool_invocations tables."""
    op.drop_index("ix_tool_invocations_tool_name", table_name="tool_invocations")
    op.drop_index("ix_tool_invocations_plan_id", table_name="tool_invocations")
    op.drop_index("ix_tool_invocations_changeset_id", table_name="tool_invocations")
    op.drop_table("tool_invocations")

    op.drop_index("ix_changeset_entries_path", table_name="changeset_entries")
    op.drop_index("ix_changeset_entries_plan_id", table_name="changeset_entries")
    op.drop_index("ix_changeset_entries_changeset_id", table_name="changeset_entries")
    op.drop_table("changeset_entries")

Actual Behavior

Indexes are silently dropped as a side effect of DROP TABLE, but no explicit drop_index calls appear in downgrade().

Suggested Fix

Add the 6 explicit op.drop_index() calls shown above to downgrade() before the op.drop_table() calls.

Category

consistency / migration

TDD Note

After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: @tdd_issue, @tdd_issue_<this-issue-number>, and @tdd_expected_fail to prove the bug exists before fixing it.


Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: bug-hunter

## Bug Report: [migration] `d0_001_changeset_artifacts` Downgrade Drops Tables Without Explicitly Dropping Indexes ### Severity Assessment - **Impact**: On most database backends (SQLite, PostgreSQL, MySQL), `DROP TABLE` automatically removes associated indexes, so this works in practice. However, it violates the principle of downgrade symmetry — every `create_index` in `upgrade()` should have a corresponding `drop_index` in `downgrade()`. In some edge cases (e.g., shared/global indexes, certain PostgreSQL configurations, or future refactors that split the `drop_table` into `ALTER TABLE` operations), the missing `drop_index` calls could cause errors. It also makes the migration harder to reason about and audit. - **Likelihood**: Low for immediate production failures; Medium for future refactor hazards. - **Priority**: Medium ### Location - **File**: `alembic/versions/d0_001_changeset_artifacts.py` - **Function**: `downgrade()` - **Lines**: ~100–104 ### Description The `upgrade()` function creates **6 named indexes** across two tables: ```python # upgrade() creates: op.create_index("ix_changeset_entries_changeset_id", "changeset_entries", ["changeset_id"]) op.create_index("ix_changeset_entries_plan_id", "changeset_entries", ["plan_id"]) op.create_index("ix_changeset_entries_path", "changeset_entries", ["path"]) op.create_index("ix_tool_invocations_changeset_id", "tool_invocations", ["changeset_id"]) op.create_index("ix_tool_invocations_plan_id", "tool_invocations", ["plan_id"]) op.create_index("ix_tool_invocations_tool_name", "tool_invocations", ["tool_name"]) ``` The `downgrade()` function drops the tables **without dropping the indexes first**: ```python def downgrade() -> None: """Drop changeset_entries and tool_invocations tables.""" op.drop_table("tool_invocations") # ← drops 3 indexes implicitly op.drop_table("changeset_entries") # ← drops 3 indexes implicitly ``` Compare with well-formed migrations in the same codebase (e.g., `001_initial_schema.py`, `a5_003_spec_aligned_actions.py`) which explicitly drop each index before dropping the table: ```python # Correct pattern from 001_initial_schema.py: op.drop_index(op.f("ix_changes_plan_id"), table_name="changes") op.drop_index(op.f("ix_changes_applied"), table_name="changes") op.drop_table("changes") ``` The inconsistency means: 1. Alembic's `--autogenerate` may detect schema drift (finding "extra" indexes if it compares metadata to an in-progress downgrade). 2. Future refactors that change `drop_table` to individual `alter_column` or partial reversal logic will break silently. 3. Code review and auditing tools that verify downgrade symmetry will flag this migration. ### Expected Behavior `downgrade()` should explicitly drop all 6 named indexes before dropping the tables, mirroring the upgrade operations in reverse: ```python def downgrade() -> None: """Drop changeset_entries and tool_invocations tables.""" op.drop_index("ix_tool_invocations_tool_name", table_name="tool_invocations") op.drop_index("ix_tool_invocations_plan_id", table_name="tool_invocations") op.drop_index("ix_tool_invocations_changeset_id", table_name="tool_invocations") op.drop_table("tool_invocations") op.drop_index("ix_changeset_entries_path", table_name="changeset_entries") op.drop_index("ix_changeset_entries_plan_id", table_name="changeset_entries") op.drop_index("ix_changeset_entries_changeset_id", table_name="changeset_entries") op.drop_table("changeset_entries") ``` ### Actual Behavior Indexes are silently dropped as a side effect of `DROP TABLE`, but no explicit `drop_index` calls appear in `downgrade()`. ### Suggested Fix Add the 6 explicit `op.drop_index()` calls shown above to `downgrade()` before the `op.drop_table()` calls. ### Category `consistency` / `migration` ### TDD Note After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: `@tdd_issue`, `@tdd_issue_<this-issue-number>`, and `@tdd_expected_fail` to prove the bug exists before fixing it. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: bug-hunter
HAL9000 added this to the v3.2.0 milestone 2026-04-09 23:04:01 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core#6655
No description provided.