Drop coveragerecords and equivalentscoveragerecords tables (PP-4653)

[dbernstein]

Description

Stacked follow-up to #3520 ("Retire the CoverageProvider machinery"). Now that no running code reads or writes the coverage tables, this PR removes the CoverageRecord and (already-dormant) EquivalencyCoverageRecord models, the coverage_records relationships on Identifier/DataSource/Collection, and drops the coveragerecords and equivalentscoveragerecords tables along with their shared coverage_status enum.

It also removes the now-orphaned BaseCoverageRecord mixin. Its only remaining user was the workcoveragerecords-removal migration (01b1e464a9d1), which imported it solely for the coverage_status enum values; that migration is now self-contained (the enum values are inlined), so the mixin can go. coverage.py is left containing only the Timestamp model.

Timestamp and its separate service_type enum are retained.

Motivation and Context

JIRA (PP-4653)

This is the "release 2" half of the CoverageProvider retirement. Per the online-migration / N-1 rule, the tables could not be dropped in the same release that stopped using them (#3520) — N-1 app servers still wrote to them during a rolling deploy.

⚠️ Draft until the release containing #3520 has shipped. Merging earlier would break N-1 webservers that still write to these tables.

How Has This Been Tested?

• mypy and ruff pass on all changed files.
• The migration round-trips on a real Postgres: upgrade drops both tables and the coverage_status enum (leaving timestamps / service_type intact); downgrade recreates both tables with identical indexes, constraints, and foreign keys; a second upgrade drops them again.
• The full tests/migration/ suite passes under tox -e py312-docker (incl. pytest-alembic single-head / up-down-consistency / model-matches-DDL checks and a new drop test), plus the Timestamp tests. The 01b1e464a9d1 down-migration still recreates workcoveragerecords correctly with its inlined enum.

Checklist

• I have updated the documentation accordingly.
• All new and existing tests passed.

🤖 Generated with Claude Code

[claude]

Claude finished @dbernstein's task in 3m 55s —— View job

---

Claude Code Review

• Read the new drop migration (20260624_5ca948100902)
• Review the 01b1e464a9d1 migration change (inlined enum)
• Review model deletions (coverage.py, collection.py, datasource.py, identifier.py)
• Review test/fixture deletions
• Verify N-1 safety & no dangling references to dropped symbols
• Post review

Summary

This is a clean, well-scoped "release 2" deletion. I verified there are no dangling references left behind: no CoverageRecord / EquivalencyCoverageRecord / BaseCoverageRecord usages remain in src/ or tests/, the coverage_records relationships are gone from all three owning models, and the .coverage_record() test fixture has no remaining callers. The only other code that imports from model.coverage now imports Timestamp (which is retained), so nothing breaks. The coverage_status enum is genuinely orphaned after this PR (its only users were coveragerecords, equivalentscoveragerecords, and the already-dropped workcoveragerecords), so dropping the type is correct. Inlining the enum literals in 01b1e464a9d1 is also necessary — that migration could no longer import BaseCoverageRecord once it's deleted — and the inlined values match the original. The migration chains cleanly off the single head a6c85605404c, and the create_type=False + explicit .create()/.drop() enum handling is the right pattern. I have one minor test-coverage suggestion.

Details

Minor: tests/migration/test_20260624_5ca948100902_drop_coverage_tables.py:26-38

The test's docstring says the migration "drops … the shared coverage_status enum" on upgrade and recreates it on downgrade, but the assertions only check the two table names — the enum lifecycle (the most novel/risky part of this migration, since it's a shared type managed with create_type=False) is never verified. A regression that left the enum orphaned on upgrade, or failed to recreate it on downgrade, would pass this test. Consider asserting the type's presence/absence directly, e.g.:

def enum_exists(engine) -> bool:
    with engine.connect() as conn:
        return conn.execute(
            sa.text("SELECT 1 FROM pg_type WHERE typname = 'coverage_status'")
        ).scalar() is not None

# after migrate_down_one(): assert enum_exists(alembic_engine)
# after migrate_up_one():   assert not enum_exists(alembic_engine)

circulation/tests/migration/test_20260624_5ca948100902_drop_coverage_tables.py

Lines 26 to 38 in e5361af

	tables = set(inspect(alembic_engine).get_table_names())
	assert "coveragerecords" in tables
	assert "equivalentscoveragerecords" in tables
	# Apply the drop.
	alembic_runner.migrate_up_one()
	assert alembic_runner.current == REVISION
	tables = set(inspect(alembic_engine).get_table_names())
	assert "coveragerecords" not in tables
	assert "equivalentscoveragerecords" not in tables
	# The unrelated timestamps table is left in place.
	assert "timestamps" in tables

• chore/drop-coverage-tables

[greptile-apps]

Greptile Summary

This PR completes the second half of the CoverageProvider retirement by dropping the now-unused coveragerecords and equivalentscoveragerecords tables and their shared coverage_status Postgres enum, following the project's N-1 online-migration convention correctly.

• Removes BaseCoverageRecord, CoverageRecord, and EquivalencyCoverageRecord model classes along with their ORM relationships on Identifier, DataSource, and Collection; Timestamp and the service_type enum are retained untouched.
• Adds a new Alembic migration (5ca948100902) with a carefully ordered upgrade (drop indexes → drop tables → drop enum) and a matching downgrade that faithfully recreates both tables, all FK constraints, partial indexes, and the enum; the older 01b1e464a9d1 migration is patched to inline the four coverage_status enum strings directly so it no longer imports the now-deleted BaseCoverageRecord.
• Cleans up all corresponding tests, fixtures, and imports with no dangling references remaining in src/ or tests/.

Confidence Score: 5/5

Safe to merge once the release containing PR #3520 has shipped; the PR itself contains no logic — only schema cleanup and the deletion of dormant models.

Every changed file is a removal: dead model classes, orphaned ORM relationships, and their tests. The Alembic migration correctly sequences drops (indexes → tables → enum) and the downgrade recreates both tables with all FK constraints and partial indexes in the right order. No running code references the dropped objects, and a grep over the full src/ and tests/ tree confirms zero remaining imports of CoverageRecord, BaseCoverageRecord, or EquivalencyCoverageRecord. The older 01b1e464a9d1 migration is self-contained after inlining the enum values. The migration test validates the full round-trip.

No files require special attention.

Important Files Changed

Filename	Overview
alembic/versions/20260624_5ca948100902_drop_coveragerecords_and_.py	New migration that drops coveragerecords and equivalentscoveragerecords tables and the coverage_status enum; upgrade and downgrade ordering is correct, enum is dropped after tables and created before tables, FK constraints and partial indexes are faithfully reproduced in the downgrade.
alembic/versions/20250528_01b1e464a9d1_remove_work_coverage_table.py	Removes the import of BaseCoverageRecord (now deleted) and inlines the four enum string values and the enum name directly; the inlined values match what the original class defined.
src/palace/manager/sqlalchemy/model/coverage.py	Strips BaseCoverageRecord, CoverageRecord, and EquivalencyCoverageRecord; retains only Timestamp and its service_type enum; the remaining TYPE_CHECKING import of Collection is still needed by Timestamp.collection.
tests/migration/test_20260624_5ca948100902_drop_coverage_tables.py	New migration test that validates the round-trip: steps down so tables exist, then back up to verify both coverage tables are dropped while timestamps is untouched.
tests/fixtures/database.py	Removes the coverage_record() fixture helper and its import of CoverageRecord; no callers remain in the test suite.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph upgrade ["upgrade() — 5ca948100902"]
        A[Drop equivalentscoveragerecords indexes x4] --> B[Drop equivalentscoveragerecords table]
        B --> C[Drop coveragerecords indexes x6]
        C --> D[Drop coveragerecords table]
        D --> E[Drop coverage_status enum]
    end

    subgraph downgrade ["downgrade() — 5ca948100902"]
        F[Create coverage_status enum] --> G[Create coveragerecords table + constraints]
        G --> H[Create coveragerecords indexes x6]
        H --> I[Create equivalentscoveragerecords table + constraints]
        I --> J[Create equivalentscoveragerecords indexes x4]
    end

    subgraph retained ["Retained — unaffected"]
        K[timestamps table]
        L[service_type enum]
        M[Timestamp model]
    end

    E -. "timestamps / service_type untouched" .-> K

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    subgraph upgrade ["upgrade() — 5ca948100902"]
        A[Drop equivalentscoveragerecords indexes x4] --> B[Drop equivalentscoveragerecords table]
        B --> C[Drop coveragerecords indexes x6]
        C --> D[Drop coveragerecords table]
        D --> E[Drop coverage_status enum]
    end

    subgraph downgrade ["downgrade() — 5ca948100902"]
        F[Create coverage_status enum] --> G[Create coveragerecords table + constraints]
        G --> H[Create coveragerecords indexes x6]
        H --> I[Create equivalentscoveragerecords table + constraints]
        I --> J[Create equivalentscoveragerecords indexes x4]
    end

    subgraph retained ["Retained — unaffected"]
        K[timestamps table]
        L[service_type enum]
        M[Timestamp model]
    end

    E -. "timestamps / service_type untouched" .-> K

Loading

_{Reviews (1): Last reviewed commit: "Remove the now-orphaned BaseCoverageReco..." | Re-trigger Greptile}