From a2c6c8201b598c7eb8582726fbf487e9c8de0934 Mon Sep 17 00:00:00 2001 From: RockChinQ Date: Sat, 13 Jun 2026 05:26:08 -0400 Subject: [PATCH] refactor(persistence): freeze legacy DB migration chain, drop dbm026 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The legacy pkg/persistence/migrations (DBMigration / dbmXXX) system now coexists with Alembic but accepts no new migrations — all new schema changes go through Alembic. - remove dbm026_llm_model_context_length (superseded by Alembic 0005_add_llm_context_length, which makes the identical change) - cap required_database_version at 25 (legacy chain dbm001-025 kept read-only to upgrade pre-existing 3.x DBs to the Alembic baseline) - add migrations/README.md documenting the freeze - document the Alembic-only policy and revision-id/idempotency rules in AGENTS.md --- AGENTS.md | 8 ++++ .../pkg/persistence/migrations/README.md | 36 ++++++++++++++++ .../dbm026_llm_model_context_length.py | 42 ------------------- src/langbot/pkg/utils/constants.py | 10 ++++- 4 files changed, 52 insertions(+), 44 deletions(-) create mode 100644 src/langbot/pkg/persistence/migrations/README.md delete mode 100644 src/langbot/pkg/persistence/migrations/dbm026_llm_model_context_length.py diff --git a/AGENTS.md b/AGENTS.md index cd6e34ed..d5dbe027 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -125,6 +125,14 @@ uv run python -m langbot.pkg.persistence.alembic_runner autogenerate "descriptio Review and edit the generated script before committing. Migrations execute automatically on startup. `autogenerate` detects schema changes (add/drop columns, tables, type changes) but **data migrations** (e.g. mutating JSON field contents) must be hand-written into the generated script. `env.py` sets `render_as_batch=True`, so SQLite's ALTER TABLE limits are handled automatically — no need to branch per database. More in the wiki ["开发配置"](https://docs.langbot.app/zh/develop/dev-config#数据库迁移). +When writing a migration, follow these rules: + +- **Revision id ≤ 32 characters.** PostgreSQL stores `alembic_version.version_num` as `varchar(32)`; a longer id raises `StringDataRightTruncationError` at runtime. Prefer short, descriptive ids like `0005_add_llm_context_length`. +- **Guard every operation against missing tables/columns.** Fresh installs build the schema via `create_all()` and then stamp the Alembic baseline, so a migration may run against a table that already has the change — or, in tests, against an empty database. Check `inspector.get_table_names()` / `inspector.get_columns(...)` before `add_column` / `drop_column`, mirroring the existing migrations. +- **Keep a single linear head.** Chain `down_revision` to the current head; do not create branches. Run the migration tests after adding one: `uv run pytest tests/integration/persistence/ -q` (the PostgreSQL test needs a running PG via `TEST_POSTGRES_URL`). + +> **Legacy migration system (deprecated — do not extend).** The old 3.x migration system under `src/langbot/pkg/persistence/migrations/` (`DBMigration` subclasses in `dbmXXX_*.py`, run from `pkg/persistence/mgr.py`) is **frozen**. Do **not** add new `dbmXXX_*.py` files. The chain is capped at `required_database_version = 25` (`pkg/utils/constants.py`); those files only exist to upgrade pre-existing 3.x databases up to the Alembic baseline and are kept read-only. All new schema changes go through Alembic. + ## Some Principles - Keep it simple, stupid. diff --git a/src/langbot/pkg/persistence/migrations/README.md b/src/langbot/pkg/persistence/migrations/README.md new file mode 100644 index 00000000..8f62f1d2 --- /dev/null +++ b/src/langbot/pkg/persistence/migrations/README.md @@ -0,0 +1,36 @@ +# Legacy migrations (DEPRECATED — do not add new files here) + +This directory holds the **legacy 3.x database migration system** +(`DBMigration` subclasses in `dbmXXX_*.py`, registered via +`@migration.migration_class(N)` and run from `pkg/persistence/mgr.py`). + +**This system is frozen. Do not add new `dbmXXX_*.py` migrations.** + +The chain is capped at version 25 (`required_database_version = 25` in +`pkg/utils/constants.py`). These files exist only to upgrade pre-existing +3.x databases up to the Alembic baseline (`0001_baseline`). Removing them +would break in-place upgrades from old installations, so they are kept +read-only. + +## All new schema changes use Alembic + +Migrations now live in `pkg/persistence/alembic/versions/`. To create one: + +```bash +uv run python -m langbot.pkg.persistence.alembic_runner autogenerate "description of your change" +``` + +(requires `data/config.yaml` to exist). Review and edit the generated +script before committing — Alembic migrations run automatically on startup +and must be idempotent and guard against missing tables (the test suite +runs them against empty databases). + +### Rules for Alembic revision ids + +- Keep the revision id **≤ 32 characters** — PostgreSQL stores + `alembic_version.version_num` as `varchar(32)` and will raise + `StringDataRightTruncationError` on overflow. +- Guard every `op` call against a missing table / missing column + (`inspector.get_table_names()` / `inspector.get_columns()`); fresh + installs create the schema via `create_all()` and stamp the baseline, + so migrations may run against tables that already match or do not exist. diff --git a/src/langbot/pkg/persistence/migrations/dbm026_llm_model_context_length.py b/src/langbot/pkg/persistence/migrations/dbm026_llm_model_context_length.py deleted file mode 100644 index 81d7031e..00000000 --- a/src/langbot/pkg/persistence/migrations/dbm026_llm_model_context_length.py +++ /dev/null @@ -1,42 +0,0 @@ -import sqlalchemy -from .. import migration - - -@migration.migration_class(26) -class DBMigrateLLMModelContextLength(migration.DBMigration): - """Add context_length column to LLM models""" - - async def upgrade(self): - columns = await self._get_columns('llm_models') - if 'context_length' not in columns: - await self.ap.persistence_mgr.execute_async( - sqlalchemy.text('ALTER TABLE llm_models ADD COLUMN context_length INTEGER') - ) - - async def downgrade(self): - columns = await self._get_columns('llm_models') - if 'context_length' not in columns: - return - - if self.ap.persistence_mgr.db.name == 'postgresql': - await self.ap.persistence_mgr.execute_async( - sqlalchemy.text('ALTER TABLE llm_models DROP COLUMN IF EXISTS context_length') - ) - else: - await self.ap.persistence_mgr.execute_async( - sqlalchemy.text('ALTER TABLE llm_models DROP COLUMN context_length') - ) - - async def _get_columns(self, table_name: str) -> set[str]: - if self.ap.persistence_mgr.db.name == 'postgresql': - result = await self.ap.persistence_mgr.execute_async( - sqlalchemy.text(""" - SELECT column_name FROM information_schema.columns - WHERE table_name = :table_name - """), - {'table_name': table_name}, - ) - return {row[0] for row in result.fetchall()} - - result = await self.ap.persistence_mgr.execute_async(sqlalchemy.text(f'PRAGMA table_info({table_name})')) - return {row[1] for row in result.fetchall()} diff --git a/src/langbot/pkg/utils/constants.py b/src/langbot/pkg/utils/constants.py index f97255ab..fb520da9 100644 --- a/src/langbot/pkg/utils/constants.py +++ b/src/langbot/pkg/utils/constants.py @@ -2,8 +2,14 @@ import langbot semantic_version = f'v{langbot.__version__}' -required_database_version = 26 -"""Tag the version of the database schema, used to check if the database needs to be migrated""" +required_database_version = 25 +"""Tag the version of the legacy (3.x) database schema migration chain. + +Frozen at 25: the legacy ``pkg/persistence/migrations`` system (DBMigration / +dbmXXX_*.py) is deprecated and no longer accepts new migrations. All schema +changes from here on are managed by Alembic (see +``pkg/persistence/alembic/versions``). This value only gates the one-time +upgrade of pre-existing 3.x databases up to the Alembic baseline.""" debug_mode = False