Make lanes.size safe to drop: defer reads + server_default for writes (PP-4506)

[dbernstein]

Description

Follow-up to #3424 (merged), which removed all lane-size maintenance but left size / size_by_entrypoint eagerly mapped on the Lane model. This is release-1 of the online-migration sequence to remove those columns: stop using them. The column drop is a stacked follow-up (release-2), gated on this shipping first.

"Stop using" an ORM column has two sides, and both must be handled before the column can be dropped safely:

• Reads. SQLAlchemy eagerly SELECTs every mapped column, so simply "keeping them mapped" (as Migrate custom-list entry updates to Celery and remove lane-size maintenance (PP-4506) #3424 did) does not stop the current code from reading them. Both columns are marked deferred(), which excludes them from the default SELECT.
• Writes. SQLAlchemy also emits any column with a Python-side default= in every INSERT. size is NOT NULL with default=0, so it was still written on every Lane insert — and lanes are created at runtime (create_default_lanes: reset lanes / configure a library / create a custom lane). A follow-up migration that dropped the column would then break the still-running previous release's inserts (column "size" does not exist). This PR moves the default into the database (ALTER COLUMN size SET DEFAULT 0) and removes the Python-side default=0. With a server_default and no Python default, SQLAlchemy omits the column from INSERTs and Postgres fills in 0, so the eventual drop is write-safe. size_by_entrypoint is nullable with no default, so it was already omitted from inserts and needs no write-side change.

The SET DEFAULT migration is catalog-only in Postgres (instant, no table rewrite) and backwards-compatible: the previous release still writes size=0 explicitly, harmlessly overriding the new default, and still reads size (the column is untouched). The columns stay in the model and the database, so test_model_definitions_match_ddl still passes and the previous release keeps working.

Once a release containing this change has shipped, the follow-up PR can drop both columns without breaking the then-previous release.

Motivation and Context

JIRA: PP-4506

Completes the "stop using" step of the online-migration sequence for removing the lane-size columns (release-1). The column drop is release-2, gated on this shipping first, and is tracked by the cleanup migration label.

How Has This Been Tested?

• tests/migration/test_alembic_builtin_tests.py passes, including test_model_definitions_match_ddl (model matches the migrated schema, with compare_server_default=True) and the up/down migration round-trip.
• mypy clean on the Lane model.
• Verified against a NOT NULL column with the same config that, with a server_default and no Python default, SQLAlchemy omits the column from INSERTs entirely (and the insert still succeeds after the column is dropped), whereas the previous deferred-only / Python-default=0 version emitted it on every insert.

Checklist

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

[claude]

Claude finished @jonathangreen's task in 2m 19s —— View job

---

Review — PR #3481

• Read changed files & surrounding context
• Verify migration chain / down_revision
• Check deferred() reads + server_default writes reasoning
• Check for remaining size usages in code
• Post review

Summary

This is a clean, correct implementation of release-1 of the online-migration sequence. The reasoning in the PR body holds up under scrutiny: marking both columns deferred() removes them from the default SELECT (read-side), and moving size's default from a Python default=0 to a DB server_default=text("0") makes SQLAlchemy omit the NOT NULL column from INSERTs while Postgres fills in 0 (write-side), so the release-2 drop won't break this release's still-running inserts. I confirmed the migration chain is valid (down_revision = "a6c85605404c" is the current head and nothing else revises it, with no release-2 drop present yet), no code path reads Lane.size / Lane.size_by_entrypoint (the other .size hits are CustomList / Pagination / PIL.Image), the SET DEFAULT is catalog-only and backwards-compatible, and the downgrade's server_default=None correctly emits DROP DEFAULT — matching the idiom already used in 20251023_6212e80c0fab and 20250626_bc471d8a83fb. size_by_entrypoint is nullable with no default, so it is already omitted from inserts and correctly needs no write-side change. Only one very minor nit below.

Details

Nit: src/palace/manager/sqlalchemy/model/lane.py:121

Switching to deferred(...) dropped the Mapped[int] annotation that size previously carried, leaving it untyped unlike the other typed columns in the model. Since the column is unread and slated for removal next release this is harmless, but if you want to keep the annotation it composes fine with deferred: size: Mapped[int] = deferred(Column(Integer, nullable=False, server_default=text("0"))).

circulation/src/palace/manager/sqlalchemy/model/lane.py

Line 121 in 13b75b6

size = deferred(Column(Integer, nullable=False, server_default=text("0")))

[greptile-apps]

Greptile Summary

This PR implements release-1 of a two-step online migration to remove the deprecated lanes.size and lanes.size_by_entrypoint columns. It stops both reads (via deferred()) and writes (via moving size's default from Python-side to a server_default) so that a follow-up release can safely drop the columns without breaking the still-running previous release.

• Both size and size_by_entrypoint are marked deferred() on the Lane model, excluding them from default SELECT queries.
• A catalog-only ALTER COLUMN size SET DEFAULT 0 migration moves the NOT NULL default into the database, allowing SQLAlchemy to omit the column from INSERTs entirely while keeping existing rows valid.
• CLAUDE.md is updated with the canonical pattern for this two-phase column-removal strategy.

Confidence Score: 5/5

Safe to merge — the migration is catalog-only (instant, no table rewrite), fully reversible, and backwards-compatible with the previous release.

Both the model change and the migration are mechanically correct: deferred() stops eager reads, server_default replaces the Python-side default so the ORM omits the column from inserts, and the ALTER COLUMN SET DEFAULT migration is a no-lock, instant Postgres catalog operation. No other code in the codebase references size or size_by_entrypoint on the Lane model.

No files require special attention.

Important Files Changed

Filename	Overview
src/palace/manager/sqlalchemy/model/lane.py	Correctly wraps both deprecated columns in deferred() and replaces the Python-side default=0 on size with a server_default; no other code in the codebase references these columns directly.
alembic/versions/20260715_dbeb42224c1b_add_server_default_to_lanes_size.py	Adds server_default=sa.text('0') to lanes.size in upgrade and removes it in downgrade; catalog-only operation in Postgres, correct and reversible.
CLAUDE.md	Adds clear documentation for the two-phase ORM column removal pattern (deferred reads + server_default writes), consistent with the migration strategy this PR implements.

_{Reviews (6): Last reviewed commit: "Some small human comment cleanup" | Re-trigger Greptile}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.46%. Comparing base (65b15bf) to head (13b75b6).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3481   +/-   ##
=======================================
  Coverage   93.46%   93.46%           
=======================================
  Files         512      512           
  Lines       46611    46611           
  Branches     6352     6352           
=======================================
+ Hits        43566    43567    +1     
+ Misses       1969     1968    -1     
  Partials     1076     1076           

☔ 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.

[jonathangreen]

This looks good.

I pushed a couple commits to clean up some AI code review comments here. And I'm going to merge this one once all the tests pass, since @dbernstein is out on vacation and it would be nice to get this into this release.