Drop coveragerecords and equivalentscoveragerecords tables (PP-4653)

[dbernstein]

Description

Follow-up to #3744 ("Stop using the coveragerecords table"), now merged. With no running code mapping or reading the coverage tables, this PR removes the CoverageRecord and (already-dormant) EquivalencyCoverageRecord models 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.

One test from #3744 is adapted here. test_upgrade_does_not_leak_lock_timeout ran the truncate migration against the schema create_all builds from the models — which no longer contains coveragerecords once this PR deletes the class. It now steps back to that revision first, whose downgrade recreates the table. The test still earns its place after this release: a database older than the truncate revision upgrades through it and this drop in a single transaction, which is exactly the leak the lock_timeout reset guards against.

Rescoped. This PR previously also removed the coverage_records relationships and the model's tests, and was based directly on #3520. Both of those moved to #3744.

Motivation and Context

JIRA (PP-4653)

This is the final step of the CoverageProvider retirement, which became a three-release sequence rather than two:

1. Retire the CoverageProvider machinery (PP-4468) #3520 — retire the CoverageProvider machinery (the code that used the records).
2. Stop using the coveragerecords table (PP-4653) #3744 — remove the coverage_records relationships, empty the table, drop the model's tests. This is what actually stops the reads.
3. This PR — remove the models and drop the tables.

The middle step turned out to be necessary. #3520 left the models in place as "dormant", but the coverage_records relationships on Identifier/DataSource/Collection meant SQLAlchemy still SELECTed from coveragerecords on every parent delete. Running the backwards-compatibility gate against the #3520 release proved it: 30 tests failed with relation "coveragerecords" does not exist, 24 of them from relationship walks on collection and identifier deletes rather than from any direct use of the model.

The split has to fall between the relationship and the model, not around the model, because a fresh database's schema is built with create_all from the models rather than by replaying migrations — so removing the classes any earlier would drop the table out from under N-1 servers on new installs.

⚠️ Draft until the release containing #3744 has shipped. #3744 is merged but not yet released; merging this before that release goes out would break N-1 webservers, which still map these tables.

How Has This Been Tested?

• mypy clean; pre-commit clean.
• 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.
• tests/migration/, tests/manager/sqlalchemy/ and tests/manager/data_layer/ pass under tox -e py312-docker (666 tests), incl. pytest-alembic single-head / up-down-consistency / model-matches-DDL checks and a drop test. The 01b1e464a9d1 down-migration still recreates workcoveragerecords correctly with its inlined enum.
• The adapted lock_timeout test was re-checked by mutation: with the reset removed from the truncate migration it fails (assert '5s' == '0'), so it still catches the leak rather than passing vacuously.
• Backwards compatibility verified locally against main, which now contains Stop using the coveragerecords table (PP-4653) #3744 and is what N-1 will be:

  ./docker/ci/test_backwards_compatibility.sh <main>
  → 3399 passed — "The previous release works against the current schema 🎉"
  

Checklist

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

🤖 Generated with Claude Code

[claude]

Claude finished @dbernstein's task in 2m 44s —— View job

---

Code review

• Gather context (PR diff, CLAUDE.md, preceding 58ebd34c5092 migration)
• Review migration 5ca948100902
• Review changes to 01b1e464a9d1 migration
• Review model changes in coverage.py
• Review tests
• Post review

Summary

This final removal step looks correct. Outside the alembic revisions, nothing in src/ or tests/ still refers to CoverageRecord, EquivalencyCoverageRecord, BaseCoverageRecord or coverage_status. The unused Index import is gone from coverage.py. With its enum values inlined, 01b1e464a9d1 is self-contained. The downgrade matches the removed model definitions: every index including the partial unique one, the ON DELETE CASCADE FK, equivalency_id NOT NULL, and the unique constraint. The test update for 58ebd34c5092 (step back down before the TRUNCATE) is also right. What's left is deploy-time locking on the drop and one test gap.

Details

Minor: alembic/versions/20260916_5ca948100902_drop_coveragerecords_and_.py:42-81

Each DROP TABLE also takes ACCESS EXCLUSIVE on every table it points to by foreign key: identifiers, datasources, collections and equivalents. Postgres needs that lock to remove the FK triggers it installed on those tables. This only bites if, when the deploy runs, some long-running transaction (a Celery import, a report) holds any lock on one of them. In that case the drop waits with no limit, and meanwhile every new N-1 query on identifiers or collections queues behind it. The 58ebd34c5092 docstring guards TRUNCATE against exactly this stall, and these tables are much busier than coveragerecords. I'd suggest the same bound:

def upgrade() -> None:
    op.execute("SET LOCAL lock_timeout = '5s'")
    ...
    coverage_status.drop(op.get_bind(), checkfirst=False)
    # Scope the timeout to this revision; see 58ebd34c5092.
    op.execute("SET LOCAL lock_timeout = DEFAULT")

circulation/alembic/versions/20260916_5ca948100902_drop_coveragerecords_and_.py

Lines 42 to 81 in 325e5db

	def upgrade() -> None:
	op.drop_index(
	op.f("ix_equivalentscoveragerecords_equivalency_id"),
	table_name="equivalentscoveragerecords",
	)
	op.drop_index(
	op.f("ix_equivalentscoveragerecords_operation"),
	table_name="equivalentscoveragerecords",
	)
	op.drop_index(
	op.f("ix_equivalentscoveragerecords_status"),
	table_name="equivalentscoveragerecords",
	)
	op.drop_index(
	op.f("ix_equivalentscoveragerecords_timestamp"),
	table_name="equivalentscoveragerecords",
	)
	op.drop_table("equivalentscoveragerecords")
	op.drop_index(
	op.f("ix_coveragerecords_data_source_id_operation_identifier_id"),
	table_name="coveragerecords",
	)
	op.drop_index(op.f("ix_coveragerecords_exception"), table_name="coveragerecords")
	op.drop_index(
	op.f("ix_coveragerecords_identifier_id"), table_name="coveragerecords"
	)
	op.drop_index(op.f("ix_coveragerecords_status"), table_name="coveragerecords")
	op.drop_index(op.f("ix_coveragerecords_timestamp"), table_name="coveragerecords")
	op.drop_index(
	"ix_identifier_id_data_source_id_operation", table_name="coveragerecords"
	)
	op.drop_index(
	"ix_identifier_id_data_source_id_operation_collection_id",
	table_name="coveragerecords",
	)
	op.drop_table("coveragerecords")
	# The coverage_status enum was used only by the two tables just dropped.
	coverage_status.drop(op.get_bind(), checkfirst=False)

Minor: tests/migration/test_20260916_5ca948100902_drop_coverage_tables.py:34-38

The docstring says the test covers the shared coverage_status enum, but nothing checks it. If coverage_status.drop(...) were deleted, this test would still pass. So would all the enabled pytest-alembic built-ins: autogenerate doesn't compare enum types, and test_up_down_consistency only runs the downgrade from a stamped head. The type would just be left behind silently. I'd add an assertion after the upgrade, and optionally the reverse after the step down:

with alembic_engine.connect() as connection:
    assert not connection.execute(
        text("SELECT 1 FROM pg_type WHERE typname = 'coverage_status'")
    ).first()

circulation/tests/migration/test_20260916_5ca948100902_drop_coverage_tables.py

Lines 34 to 38 in 325e5db

	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

Nit: tests/migration/test_20260916_58ebd34c5092_empty_coveragerecords.py:89-91

The comment says to step back to this revision, "whose downgrade recreates it". But 58ebd34c5092's own downgrade is a pass; the table actually comes back from the next revision's downgrade (5ca948100902). Suggested wording: "Step back to this revision; undoing the next revision's drop recreates it."

circulation/tests/migration/test_20260916_58ebd34c5092_empty_coveragerecords.py

Lines 89 to 91 in 325e5db

	# The next revision drops coveragerecords, so it is absent from the schema
	# built from the current models. Step back to this revision, whose
	# downgrade recreates it, to give the TRUNCATE something to act on.

---

I didn't run the test suite here: starting a local Postgres container needed approval this job didn't have. The locking point above comes from how Postgres drops FK triggers, not from a live run.

[greptile-apps]

Confidence Score: 5/5

[Critical risk] Database migration drops two tables and an enum type.

The PR appears safe to merge once the preceding release that stops using the coverage tables has shipped.

Summary

This PR removes the two dormant coverage models, their tables, and the shared coverage_status enum. It makes the older work-coverage migration independent of the removed mixin and adds migration tests. The drop must remain sequenced after the release that stopped using the tables.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Prior release stops table use] --> B[Drop migration removes both tables]
  B --> C[Drop shared coverage_status enum]
  B --> D[Keep timestamps and service_type]

Loading

_{Reviews (5) · Last reviewed commit: "Remove the now-orphaned BaseCoverageReco..."}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.74%. Comparing base (5dde763) to head (325e5db).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3521      +/-   ##
==========================================
- Coverage   93.74%   93.74%   -0.01%     
==========================================
  Files         510      510              
  Lines       46417    46390      -27     
  Branches     6300     6300              
==========================================
- Hits        43513    43487      -26     
+ Misses       1876     1875       -1     
  Partials     1028     1028              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.