From 8a3ae8652e77222c538627f5bc1b502e51178acb Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 16 Mar 2026 22:26:07 +0000
Subject: [PATCH 1/4] Initial plan


From 935e8a626ea66b9665f97062be6c5537015915ec Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 16 Mar 2026 23:25:53 +0000
Subject: [PATCH 2/4] Changes before error encountered

Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
---
 scripts/check_alembic_migrations.py | 214 ++++++++++++++++++++++++++++
 1 file changed, 214 insertions(+)
 create mode 100644 scripts/check_alembic_migrations.py

diff --git a/scripts/check_alembic_migrations.py b/scripts/check_alembic_migrations.py
new file mode 100644
index 00000000..08f85b45
--- /dev/null
+++ b/scripts/check_alembic_migrations.py
@@ -0,0 +1,214 @@
+#!/usr/bin/env python3
+"""Validate Alembic migration chain integrity.
+
+This script checks the migration files in ``migrations/versions/`` for
+common problems that arise when multiple feature branches add migrations
+in parallel and then get merged into *main*.
+
+Checks performed
+~~~~~~~~~~~~~~~~
+1. **Multiple heads** – more than one migration without a child means the
+   chain has diverged and a merge migration is needed.
+2. **Broken down-revision references** – a migration points to a
+   ``down_revision`` that does not exist.
+3. **Duplicate revision IDs** – two files declare the same ``revision``.
+4. **Revision / filename mismatch** – the ``revision`` variable inside a
+   file does not match the stem of the filename (minus the numeric
+   prefix).
+
+Exit codes
+~~~~~~~~~~
+* **0** – all checks passed.
+* **1** – one or more problems detected (details printed to *stderr*).
+* **2** – unexpected runtime error.
+
+Usage::
+
+    python scripts/check_alembic_migrations.py          # from repo root
+    python scripts/check_alembic_migrations.py --verbose # extra detail
+"""
+
+from __future__ import annotations
+
+import argparse
+import ast
+import re
+import sys
+from pathlib import Path
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+_REVISION_RE = re.compile(r'^revision\s*(?::\s*str\s*)?=\s*["\'](.+?)["\']', re.MULTILINE)
+_DOWN_REV_RE = re.compile(
+    r'^down_revision\s*(?::\s*Union\[str,\s*(?:None|tuple)\]\s*)?=\s*(.+)',
+    re.MULTILINE,
+)
+
+
+def _parse_down_revision(raw: str) -> list[str] | None:
+    """Parse a ``down_revision`` value into a list of parent revisions.
+
+    Returns ``None`` for the root migration (``down_revision = None``).
+    Returns a list with one or more strings otherwise.  Tuples are
+    returned for merge migrations (e.g. ``("017_a", "017_b")``).
+    """
+    raw = raw.strip().rstrip("#").strip()
+    # Handle inline comments
+    if "#" in raw:
+        raw = raw[: raw.index("#")].strip()
+    try:
+        value = ast.literal_eval(raw)
+    except (ValueError, SyntaxError):
+        return [raw.strip("\"' ")]
+
+    if value is None:
+        return None
+    if isinstance(value, str):
+        return [value]
+    if isinstance(value, (tuple, list)):
+        return [str(v) for v in value]
+    return [str(value)]
+
+
+def _parse_migration(path: Path) -> dict | None:
+    """Extract ``revision`` and ``down_revision`` from a migration file."""
+    text = path.read_text(encoding="utf-8")
+
+    rev_match = _REVISION_RE.search(text)
+    down_match = _DOWN_REV_RE.search(text)
+
+    if not rev_match:
+        return None  # not a valid migration file
+
+    revision = rev_match.group(1)
+    down_revision = _parse_down_revision(down_match.group(1)) if down_match else None
+
+    return {
+        "path": path,
+        "revision": revision,
+        "down_revision": down_revision,
+    }
+
+
+# ---------------------------------------------------------------------------
+# Checks
+# ---------------------------------------------------------------------------
+
+
+def check_migrations(versions_dir: Path, *, verbose: bool = False) -> list[str]:
+    """Run all migration-chain checks and return a list of error messages."""
+    errors: list[str] = []
+
+    # Collect all migrations ------------------------------------------------
+    migrations: dict[str, dict] = {}
+    py_files = sorted(versions_dir.glob("*.py"))
+    if not py_files:
+        errors.append(f"No migration files found in {versions_dir}")
+        return errors
+
+    for path in py_files:
+        if path.name == "__init__.py":
+            continue
+        info = _parse_migration(path)
+        if info is None:
+            if verbose:
+                print(f"  SKIP {path.name} (no revision found)", file=sys.stderr)
+            continue
+        rev = info["revision"]
+
+        # Check 1 – duplicate revision IDs
+        if rev in migrations:
+            errors.append(
+                f"Duplicate revision '{rev}' in:\n"
+                f"  - {migrations[rev]['path'].name}\n"
+                f"  - {path.name}"
+            )
+        else:
+            migrations[rev] = info
+
+        if verbose:
+            parents = info["down_revision"] or ["(root)"]
+            print(f"  {rev}  ←  {', '.join(parents)}", file=sys.stderr)
+
+    # Build child map -------------------------------------------------------
+    all_revisions = set(migrations.keys())
+    children: dict[str, list[str]] = {rev: [] for rev in all_revisions}
+
+    for rev, info in migrations.items():
+        parents = info["down_revision"]
+        if parents is None:
+            continue
+        for parent in parents:
+            # Check 2 – broken down_revision references
+            if parent not in all_revisions:
+                errors.append(
+                    f"Broken chain: '{rev}' ({info['path'].name}) references "
+                    f"down_revision '{parent}' which does not exist."
+                )
+            else:
+                children[parent].append(rev)
+
+    # Check 3 – multiple heads (revisions with no children) -----------------
+    heads = [rev for rev, kids in children.items() if not kids]
+    if len(heads) > 1:
+        head_details = "\n".join(f"  - {h} ({migrations[h]['path'].name})" for h in sorted(heads))
+        errors.append(
+            f"Multiple migration heads detected ({len(heads)}).  "
+            f"Create a merge migration to resolve:\n{head_details}\n\n"
+            f"  Fix: alembic merge heads -m \"merge_parallel_branches\""
+        )
+
+    # Check 4 – revision / filename consistency -----------------------------
+    for rev, info in migrations.items():
+        stem = info["path"].stem  # e.g. "017_add_pipelines"
+        if rev != stem:
+            errors.append(
+                f"Filename mismatch: file '{info['path'].name}' declares "
+                f"revision='{rev}' but filename stem is '{stem}'."
+            )
+
+    return errors
+
+
+# ---------------------------------------------------------------------------
+# CLI entry-point
+# ---------------------------------------------------------------------------
+
+
+def main(argv: list[str] | None = None) -> int:
+    """CLI entry-point.  Returns 0 on success, 1 on failure, 2 on error."""
+    parser = argparse.ArgumentParser(description="Check Alembic migration chain integrity.")
+    parser.add_argument(
+        "--versions-dir",
+        type=Path,
+        default=Path("migrations/versions"),
+        help="Path to Alembic versions directory (default: migrations/versions)",
+    )
+    parser.add_argument("--verbose", "-v", action="store_true", help="Print extra diagnostic info")
+    args = parser.parse_args(argv)
+
+    if not args.versions_dir.is_dir():
+        print(f"ERROR: versions directory not found: {args.versions_dir}", file=sys.stderr)
+        return 2
+
+    if args.verbose:
+        print("Scanning migrations…", file=sys.stderr)
+
+    errors = check_migrations(args.versions_dir, verbose=args.verbose)
+
+    if errors:
+        print(f"\n{'=' * 60}", file=sys.stderr)
+        print(f"  Migration chain problems found: {len(errors)}", file=sys.stderr)
+        print(f"{'=' * 60}\n", file=sys.stderr)
+        for i, err in enumerate(errors, 1):
+            print(f"  [{i}] {err}\n", file=sys.stderr)
+        return 1
+
+    print("✓ Migration chain is valid.", file=sys.stderr)
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

From aa49fa3ae6ae0bfbb0f9efa6304cf109eca1c402 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Tue, 17 Mar 2026 09:18:11 +0000
Subject: [PATCH 3/4] feat(db): add migration chain CI validation, pre-commit
 hook, script template, docs, and tests

Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
---
 .github/workflows/ci.yml               |  14 +-
 .pre-commit-config.yaml                |  10 +
 docs/DatabaseConfiguration.md          |  21 ++
 docs/MigrationWorkflow.md              | 358 +++++++++++++++++++++++++
 migrations/script.py.mako              |  40 +++
 scripts/check_alembic_migrations.py    |  10 +-
 tests/test_check_alembic_migrations.py | 196 ++++++++++++++
 7 files changed, 641 insertions(+), 8 deletions(-)
 create mode 100644 docs/MigrationWorkflow.md
 create mode 100644 migrations/script.py.mako
 create mode 100644 tests/test_check_alembic_migrations.py

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 0c206c46..45777958 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -44,6 +44,18 @@ jobs:
       - run: ruff check app/ tests/
       - run: ruff format --check app/ tests/
 
+  migration-chain:
+    name: Alembic Migration Chain Check
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: "3.11"
+      - name: Validate migration chain
+        run: python scripts/check_alembic_migrations.py
+
   html-lint:
     name: HTML Accessibility Lint
     runs-on: ubuntu-latest
@@ -138,7 +150,7 @@ jobs:
   build:
     name: Build & Push Docker Image
     runs-on: ubuntu-latest
-    needs: [run-tests, mypy, dependency-scan, html-lint]
+    needs: [run-tests, mypy, dependency-scan, html-lint, migration-chain]
     if: github.event_name == 'push'
     steps:
       - name: Checkout Code
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 1416b15c..773edaa9 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -48,6 +48,16 @@ repos:
             .env.demo
           )$
 
+  # Alembic migration chain validation
+  - repo: local
+    hooks:
+      - id: check-alembic-migrations
+        name: Check Alembic migration chain
+        entry: python scripts/check_alembic_migrations.py
+        language: python
+        pass_filenames: false
+        files: ^migrations/versions/.*\.py$
+
   # Conventional commits validation
   - repo: https://github.com/compilerla/conventional-pre-commit
     rev: v3.0.0
diff --git a/docs/DatabaseConfiguration.md b/docs/DatabaseConfiguration.md
index 9eb1fece..4930820f 100644
--- a/docs/DatabaseConfiguration.md
+++ b/docs/DatabaseConfiguration.md
@@ -287,6 +287,27 @@ alembic revision --autogenerate -m "describe your change"
 
 Review the generated file in `migrations/versions/` before applying it.
 
+> **Tip:** For detailed guidance on naming conventions, idempotent patterns, parallel-branch workflows, and resolving merge conflicts, see the [Migration Workflow Guide](MigrationWorkflow.md).
+
+### Validating the Migration Chain
+
+A CI check and pre-commit hook validate that the migration chain has no broken
+references, duplicate revisions, or diverged heads.  Run the check locally:
+
+```bash
+python scripts/check_alembic_migrations.py
+python scripts/check_alembic_migrations.py --verbose   # extra detail
+```
+
+If you see **"Multiple migration heads detected"**, two branches added
+migrations from the same parent.  Create a merge migration:
+
+```bash
+alembic merge heads -m "merge_parallel_branches"
+```
+
+For a complete walk-through, see the [Migration Workflow Guide](MigrationWorkflow.md).
+
 ### Automating Migrations in Docker Compose
 
 Add a short-lived `migrate` service that runs before the API and Worker:
diff --git a/docs/MigrationWorkflow.md b/docs/MigrationWorkflow.md
new file mode 100644
index 00000000..9a3603f6
--- /dev/null
+++ b/docs/MigrationWorkflow.md
@@ -0,0 +1,358 @@
+# Migration Workflow
+
+This guide explains how to create, test, and merge Alembic database migrations in DocuElevate — especially when **multiple feature branches** add migrations in parallel.
+
+## Table of Contents
+
+- [Quick Reference](#quick-reference)
+- [Creating a New Migration](#creating-a-new-migration)
+- [Migration Naming Convention](#migration-naming-convention)
+- [Idempotent Migration Patterns](#idempotent-migration-patterns)
+- [Parallel Branch Development](#parallel-branch-development)
+- [Resolving Migration Conflicts](#resolving-migration-conflicts)
+- [CI Validation](#ci-validation)
+- [Pre-commit Hook](#pre-commit-hook)
+- [Troubleshooting](#troubleshooting)
+
+---
+
+## Quick Reference
+
+```bash
+# Create a new migration after editing app/models.py
+alembic revision --autogenerate -m "add_foobar_column"
+
+# Apply all pending migrations
+alembic upgrade head
+
+# Check current database version
+alembic current
+
+# View migration history
+alembic history --verbose
+
+# Detect multiple heads (diverged branches)
+alembic heads
+
+# Create a merge migration to resolve multiple heads
+alembic merge heads -m "merge_parallel_branches"
+
+# Validate migration chain integrity (CI script)
+python scripts/check_alembic_migrations.py
+python scripts/check_alembic_migrations.py --verbose
+```
+
+---
+
+## Creating a New Migration
+
+1. **Edit `app/models.py`** — add or modify SQLAlchemy model classes.
+
+2. **Generate the migration** from the repo root:
+
+   ```bash
+   alembic revision --autogenerate -m "add_my_new_table"
+   ```
+
+   Alembic uses the `migrations/script.py.mako` template to generate the file.  The template includes inline comments about idempotent patterns — read them.
+
+3. **Rename the file** to follow the [naming convention](#migration-naming-convention):
+
+   ```bash
+   # Alembic generates a hash-based name by default.
+   # Rename to the sequential numbering scheme:
+   mv migrations/versions/<hash>_add_my_new_table.py \
+      migrations/versions/037_add_my_new_table.py
+   ```
+
+   Update the `revision` variable inside the file to match:
+
+   ```python
+   revision: str = "037_add_my_new_table"
+   ```
+
+4. **Review the generated code** — autogenerate is helpful but not perfect.  Check:
+   - Are new tables and columns detected correctly?
+   - Does the `downgrade()` reverse all changes?
+   - Are SQLite-incompatible operations wrapped in `batch_alter_table()`?
+
+5. **Test the migration** against a fresh database:
+
+   ```bash
+   # Apply
+   alembic upgrade head
+
+   # Rollback
+   alembic downgrade -1
+
+   # Re-apply
+   alembic upgrade head
+   ```
+
+6. **Run the chain validation**:
+
+   ```bash
+   python scripts/check_alembic_migrations.py
+   ```
+
+---
+
+## Migration Naming Convention
+
+All migration files follow a **sequential numeric prefix** scheme:
+
+```
+NNN_short_description.py
+```
+
+| Component | Rule |
+|-----------|------|
+| `NNN` | Three-digit zero-padded number, incrementing from the previous migration |
+| `short_description` | Lowercase snake_case summary of the change |
+
+The **`revision`** variable inside the file **must match the filename stem** exactly:
+
+```python
+# File: migrations/versions/037_add_classification_rules.py
+revision: str = "037_add_classification_rules"
+down_revision: Union[str, None] = "036_add_document_translation_fields"
+```
+
+The CI check (`scripts/check_alembic_migrations.py`) enforces this consistency.
+
+---
+
+## Idempotent Migration Patterns
+
+Migrations should be **idempotent** — safe to run even if the change already exists.  This is critical for SQLite compatibility and for recovering from partial failures.
+
+### Add a Column (only if missing)
+
+```python
+def upgrade() -> None:
+    conn = op.get_bind()
+    inspector = sa.inspect(conn)
+
+    if "my_table" in inspector.get_table_names():
+        existing = {c["name"] for c in inspector.get_columns("my_table")}
+        if "new_col" not in existing:
+            with op.batch_alter_table("my_table") as batch_op:
+                batch_op.add_column(sa.Column("new_col", sa.String(128), nullable=True))
+```
+
+### Create a Table (only if missing)
+
+```python
+def upgrade() -> None:
+    conn = op.get_bind()
+    inspector = sa.inspect(conn)
+
+    if "new_table" not in inspector.get_table_names():
+        op.create_table(
+            "new_table",
+            sa.Column("id", sa.Integer(), primary_key=True),
+            sa.Column("name", sa.String(255), nullable=False),
+        )
+```
+
+### Drop a Column (only if present)
+
+```python
+def downgrade() -> None:
+    conn = op.get_bind()
+    inspector = sa.inspect(conn)
+
+    if "my_table" in inspector.get_table_names():
+        existing = {c["name"] for c in inspector.get_columns("my_table")}
+        if "new_col" in existing:
+            with op.batch_alter_table("my_table") as batch_op:
+                batch_op.drop_column("new_col")
+```
+
+### Use `batch_alter_table` for SQLite
+
+SQLite does not support `ALTER TABLE DROP COLUMN` or `ALTER TABLE RENAME COLUMN` natively.  Alembic's `batch_alter_table` context manager works around this by recreating the table:
+
+```python
+with op.batch_alter_table("users") as batch_op:
+    batch_op.add_column(sa.Column("phone", sa.String(20), nullable=True))
+    batch_op.drop_column("fax")
+```
+
+---
+
+## Parallel Branch Development
+
+When two feature branches both add migrations from the same parent, the migration chain **diverges** into multiple heads.  This is normal and expected — Alembic supports it — but the heads must be merged before the code reaches `main`.
+
+### Example
+
+```
+main:   001 → 002 → 003
+                         ↘ Branch A: 004_add_widgets
+                         ↘ Branch B: 004_add_gadgets   ← two heads!
+```
+
+### How to Avoid Conflicts
+
+1. **Coordinate** — if two developers are both adding migrations, assign different sequence numbers (e.g., `037_` and `038_`).  Even if both depend on `036_`, different numbers prevent filename collisions.
+
+2. **Rebase early** — before opening a PR, rebase your branch onto the latest `main`:
+
+   ```bash
+   git fetch origin main
+   git rebase origin/main
+   ```
+
+   If `main` now has a new migration `037_*`, renumber yours to `038_*` and update `down_revision` to point at `037_*`.
+
+3. **Check for multiple heads** locally:
+
+   ```bash
+   python scripts/check_alembic_migrations.py
+   # or
+   alembic heads
+   ```
+
+---
+
+## Resolving Migration Conflicts
+
+If your PR's CI check reports **"Multiple migration heads detected"**, follow these steps:
+
+### Step 1 — Update Your Branch
+
+```bash
+git fetch origin main
+git merge origin/main
+# or
+git rebase origin/main
+```
+
+### Step 2 — Check Heads
+
+```bash
+python scripts/check_alembic_migrations.py --verbose
+```
+
+The output lists the conflicting heads.
+
+### Step 3 — Create a Merge Migration
+
+```bash
+alembic merge heads -m "merge_parallel_branches"
+```
+
+This generates a new migration with **two parents** (a merge point):
+
+```python
+down_revision = ("037_add_widgets", "037_add_gadgets")
+```
+
+### Step 4 — Rename and Validate
+
+Rename the merge migration to the next sequence number:
+
+```bash
+mv migrations/versions/<hash>_merge_parallel_branches.py \
+   migrations/versions/038_merge_parallel_branches.py
+```
+
+Update the `revision` inside to match, then validate:
+
+```bash
+python scripts/check_alembic_migrations.py
+```
+
+### Step 5 — Test
+
+```bash
+alembic upgrade head
+alembic downgrade -1
+alembic upgrade head
+```
+
+---
+
+## CI Validation
+
+The CI pipeline (`.github/workflows/ci.yml`) includes a **migration-chain** job that runs:
+
+```bash
+python scripts/check_alembic_migrations.py
+```
+
+This script checks for:
+
+| Check | Description |
+|-------|-------------|
+| Multiple heads | Diverged migration chains that need a merge migration |
+| Broken references | A `down_revision` that points to a non-existent revision |
+| Duplicate revisions | Two files declaring the same `revision` identifier |
+| Filename mismatches | The `revision` variable doesn't match the filename stem |
+
+The job runs in Stage 1 (fast-fail gates) alongside lint checks.  If it fails, the build is blocked until the migration chain is fixed.
+
+---
+
+## Pre-commit Hook
+
+A local pre-commit hook is configured in `.pre-commit-config.yaml` that runs the same check whenever you commit a change to `migrations/versions/`:
+
+```yaml
+- repo: local
+  hooks:
+    - id: check-alembic-migrations
+      name: Check Alembic migration chain
+      entry: python scripts/check_alembic_migrations.py
+      language: python
+      pass_filenames: false
+      files: ^migrations/versions/.*\.py$
+```
+
+Install the hook:
+
+```bash
+pip install pre-commit
+pre-commit install
+```
+
+---
+
+## Troubleshooting
+
+### "Multiple migration heads detected"
+
+See [Resolving Migration Conflicts](#resolving-migration-conflicts) above.
+
+### "Broken chain: revision X references down_revision Y which does not exist"
+
+You removed or renamed a migration that another migration depends on.  Either restore the missing file or update the dependent migration's `down_revision`.
+
+### "Filename mismatch: file declares revision=X but filename stem is Y"
+
+The `revision` string inside the Python file must match the filename (without `.py`).  Rename the file or update the variable.
+
+### "relation already exists" when running `alembic upgrade head`
+
+The database has a table that a pending migration tries to create.  Stamp the current state:
+
+```bash
+alembic stamp head
+```
+
+### Autogenerate doesn't detect my changes
+
+Ensure all models are imported in `migrations/env.py`.  The `from app.models import ...` block at the top must include your new model class.
+
+### SQLite "no such column" after downgrade
+
+SQLite has limited `ALTER TABLE` support.  Always use `op.batch_alter_table()` for column operations on existing tables.
+
+---
+
+## Further Reading
+
+- [Alembic Tutorial](https://alembic.sqlalchemy.org/en/latest/tutorial.html)
+- [Alembic Branch / Merge](https://alembic.sqlalchemy.org/en/latest/branches.html)
+- [Database Configuration Guide](DatabaseConfiguration.md)
diff --git a/migrations/script.py.mako b/migrations/script.py.mako
new file mode 100644
index 00000000..fb8a9c55
--- /dev/null
+++ b/migrations/script.py.mako
@@ -0,0 +1,40 @@
+"""${message}
+
+Revision ID: ${up_revision}
+Revises: ${down_revision | comma,n}
+Create Date: ${create_date}
+
+"""
+
+from typing import Union
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision: str = ${repr(up_revision)}
+down_revision: Union[str, None] = ${repr(down_revision)}
+depends_on: Union[str, None] = None
+
+
+def upgrade() -> None:
+    """${message}."""
+    # Use ``op.batch_alter_table()`` for SQLite compatibility.
+    # Always check whether the table/column already exists before altering
+    # to keep migrations idempotent (safe to re-run).
+    #
+    # Example – add a column only if it is missing:
+    #
+    #     conn = op.get_bind()
+    #     inspector = sa.inspect(conn)
+    #     if "my_table" in inspector.get_table_names():
+    #         existing = {c["name"] for c in inspector.get_columns("my_table")}
+    #         if "new_col" not in existing:
+    #             with op.batch_alter_table("my_table") as batch_op:
+    #                 batch_op.add_column(sa.Column("new_col", sa.String(128), nullable=True))
+    ${upgrades if upgrades else "pass"}
+
+
+def downgrade() -> None:
+    """Reverse ${message}."""
+    ${downgrades if downgrades else "pass"}
diff --git a/scripts/check_alembic_migrations.py b/scripts/check_alembic_migrations.py
index 08f85b45..dee4050a 100644
--- a/scripts/check_alembic_migrations.py
+++ b/scripts/check_alembic_migrations.py
@@ -42,7 +42,7 @@ from pathlib import Path
 
 _REVISION_RE = re.compile(r'^revision\s*(?::\s*str\s*)?=\s*["\'](.+?)["\']', re.MULTILINE)
 _DOWN_REV_RE = re.compile(
-    r'^down_revision\s*(?::\s*Union\[str,\s*(?:None|tuple)\]\s*)?=\s*(.+)',
+    r"^down_revision\s*(?::\s*Union\[str,\s*(?:None|tuple)\]\s*)?=\s*(.+)",
     re.MULTILINE,
 )
 
@@ -120,11 +120,7 @@ def check_migrations(versions_dir: Path, *, verbose: bool = False) -> list[str]:
 
         # Check 1 – duplicate revision IDs
         if rev in migrations:
-            errors.append(
-                f"Duplicate revision '{rev}' in:\n"
-                f"  - {migrations[rev]['path'].name}\n"
-                f"  - {path.name}"
-            )
+            errors.append(f"Duplicate revision '{rev}' in:\n  - {migrations[rev]['path'].name}\n  - {path.name}")
         else:
             migrations[rev] = info
 
@@ -157,7 +153,7 @@ def check_migrations(versions_dir: Path, *, verbose: bool = False) -> list[str]:
         errors.append(
             f"Multiple migration heads detected ({len(heads)}).  "
             f"Create a merge migration to resolve:\n{head_details}\n\n"
-            f"  Fix: alembic merge heads -m \"merge_parallel_branches\""
+            f'  Fix: alembic merge heads -m "merge_parallel_branches"'
         )
 
     # Check 4 – revision / filename consistency -----------------------------
diff --git a/tests/test_check_alembic_migrations.py b/tests/test_check_alembic_migrations.py
new file mode 100644
index 00000000..611bf647
--- /dev/null
+++ b/tests/test_check_alembic_migrations.py
@@ -0,0 +1,196 @@
+"""Tests for scripts/check_alembic_migrations.py."""
+
+# The script lives outside of the ``app`` package, so we import it by path.
+import importlib.util
+import textwrap
+from pathlib import Path
+
+import pytest
+
+_SCRIPT = Path(__file__).resolve().parent.parent / "scripts" / "check_alembic_migrations.py"
+_spec = importlib.util.spec_from_file_location("check_alembic_migrations", _SCRIPT)
+assert _spec and _spec.loader
+_mod = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(_mod)  # type: ignore[union-attr]
+
+check_migrations = _mod.check_migrations
+main = _mod.main
+
+
+# ---------------------------------------------------------------------------
+# Fixtures
+# ---------------------------------------------------------------------------
+
+
+def _write_migration(directory: Path, filename: str, revision: str, down_revision: str | None) -> Path:
+    """Helper to create a minimal migration file."""
+    if down_revision is None:
+        down_rev_str = "None"
+    elif isinstance(down_revision, tuple):
+        down_rev_str = repr(down_revision)
+    else:
+        down_rev_str = f'"{down_revision}"'
+
+    content = textwrap.dedent(f'''\
+        """Test migration."""
+        from typing import Union
+        revision: str = "{revision}"
+        down_revision: Union[str, None] = {down_rev_str}
+        depends_on: Union[str, None] = None
+        def upgrade() -> None:
+            pass
+        def downgrade() -> None:
+            pass
+    ''')
+    path = directory / filename
+    path.write_text(content)
+    return path
+
+
+@pytest.fixture
+def versions_dir(tmp_path: Path) -> Path:
+    """Return a temporary versions directory."""
+    d = tmp_path / "versions"
+    d.mkdir()
+    return d
+
+
+# ---------------------------------------------------------------------------
+# Tests
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.unit
+class TestCheckMigrations:
+    """Tests for the check_migrations function."""
+
+    def test_valid_linear_chain(self, versions_dir: Path) -> None:
+        """A simple linear chain should pass with no errors."""
+        _write_migration(versions_dir, "001_initial.py", "001_initial", None)
+        _write_migration(versions_dir, "002_add_col.py", "002_add_col", "001_initial")
+        _write_migration(versions_dir, "003_add_table.py", "003_add_table", "002_add_col")
+
+        errors = check_migrations(versions_dir)
+        assert errors == []
+
+    def test_valid_merge_migration(self, versions_dir: Path) -> None:
+        """A chain with a merge point should pass."""
+        _write_migration(versions_dir, "001_base.py", "001_base", None)
+        _write_migration(versions_dir, "002_a.py", "002_a", "001_base")
+        _write_migration(versions_dir, "002_b.py", "002_b", "001_base")
+
+        # Merge file with tuple down_revision
+        content = textwrap.dedent('''\
+            """Merge."""
+            from typing import Union
+            revision: str = "003_merge"
+            down_revision: Union[str, tuple] = ("002_a", "002_b")
+            depends_on: Union[str, None] = None
+            def upgrade() -> None:
+                pass
+            def downgrade() -> None:
+                pass
+        ''')
+        (versions_dir / "003_merge.py").write_text(content)
+
+        errors = check_migrations(versions_dir)
+        assert errors == []
+
+    def test_multiple_heads_detected(self, versions_dir: Path) -> None:
+        """Two unmerged branches should report multiple heads."""
+        _write_migration(versions_dir, "001_base.py", "001_base", None)
+        _write_migration(versions_dir, "002_a.py", "002_a", "001_base")
+        _write_migration(versions_dir, "002_b.py", "002_b", "001_base")
+
+        errors = check_migrations(versions_dir)
+        assert len(errors) == 1
+        assert "Multiple migration heads" in errors[0]
+        assert "002_a" in errors[0]
+        assert "002_b" in errors[0]
+
+    def test_broken_down_revision(self, versions_dir: Path) -> None:
+        """A migration pointing to a non-existent parent should be flagged."""
+        _write_migration(versions_dir, "001_base.py", "001_base", None)
+        _write_migration(versions_dir, "002_orphan.py", "002_orphan", "NONEXISTENT")
+
+        errors = check_migrations(versions_dir)
+        assert any("Broken chain" in e for e in errors)
+        assert any("NONEXISTENT" in e for e in errors)
+
+    def test_duplicate_revision(self, versions_dir: Path) -> None:
+        """Two files declaring the same revision should be flagged."""
+        _write_migration(versions_dir, "001_base.py", "001_base", None)
+        _write_migration(versions_dir, "002_first.py", "002_dup", "001_base")
+        _write_migration(versions_dir, "002_second.py", "002_dup", "001_base")
+
+        errors = check_migrations(versions_dir)
+        assert any("Duplicate revision" in e for e in errors)
+
+    def test_filename_mismatch(self, versions_dir: Path) -> None:
+        """A file whose revision doesn't match its filename should be flagged."""
+        _write_migration(versions_dir, "001_base.py", "001_base", None)
+        # filename stem is "002_wrong_name" but revision says "002_correct_name"
+        _write_migration(versions_dir, "002_wrong_name.py", "002_correct_name", "001_base")
+
+        errors = check_migrations(versions_dir)
+        assert any("Filename mismatch" in e for e in errors)
+
+    def test_empty_directory(self, versions_dir: Path) -> None:
+        """An empty versions directory should report an error."""
+        errors = check_migrations(versions_dir)
+        assert len(errors) == 1
+        assert "No migration files found" in errors[0]
+
+    def test_init_py_is_skipped(self, versions_dir: Path) -> None:
+        """__init__.py files should be ignored."""
+        (versions_dir / "__init__.py").write_text("")
+        _write_migration(versions_dir, "001_base.py", "001_base", None)
+
+        errors = check_migrations(versions_dir)
+        assert errors == []
+
+    def test_non_migration_file_skipped(self, versions_dir: Path) -> None:
+        """A .py file without a revision variable should be silently skipped."""
+        (versions_dir / "helper.py").write_text("# just a helper\nx = 1\n")
+        _write_migration(versions_dir, "001_base.py", "001_base", None)
+
+        errors = check_migrations(versions_dir)
+        assert errors == []
+
+
+@pytest.mark.unit
+class TestMainCLI:
+    """Tests for the CLI entry-point."""
+
+    def test_success_returns_zero(self, versions_dir: Path) -> None:
+        """Valid chain should exit 0."""
+        _write_migration(versions_dir, "001_base.py", "001_base", None)
+        rc = main(["--versions-dir", str(versions_dir)])
+        assert rc == 0
+
+    def test_failure_returns_one(self, versions_dir: Path) -> None:
+        """Invalid chain should exit 1."""
+        _write_migration(versions_dir, "001_base.py", "001_base", None)
+        _write_migration(versions_dir, "002_a.py", "002_a", "001_base")
+        _write_migration(versions_dir, "002_b.py", "002_b", "001_base")
+
+        rc = main(["--versions-dir", str(versions_dir)])
+        assert rc == 1
+
+    def test_missing_directory_returns_two(self, tmp_path: Path) -> None:
+        """Non-existent versions directory should exit 2."""
+        rc = main(["--versions-dir", str(tmp_path / "does_not_exist")])
+        assert rc == 2
+
+    def test_verbose_flag(self, versions_dir: Path) -> None:
+        """The --verbose flag should not crash."""
+        _write_migration(versions_dir, "001_base.py", "001_base", None)
+        rc = main(["--versions-dir", str(versions_dir), "--verbose"])
+        assert rc == 0
+
+    def test_real_migrations(self) -> None:
+        """Smoke test against the actual project migrations."""
+        real_dir = Path(__file__).resolve().parent.parent / "migrations" / "versions"
+        if real_dir.is_dir():
+            rc = main(["--versions-dir", str(real_dir)])
+            assert rc == 0

From a007b4fd98ed327a9874082766e0f3b8c32a1a68 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Tue, 17 Mar 2026 09:21:11 +0000
Subject: [PATCH 4/4] fix(db): address code review feedback - fix comment
 stripping, type hints, test skip, and docs

Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
---
 docs/MigrationWorkflow.md              | 40 ++++++++++++++------------
 scripts/check_alembic_migrations.py    |  4 +--
 tests/test_check_alembic_migrations.py | 11 ++++---
 3 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/docs/MigrationWorkflow.md b/docs/MigrationWorkflow.md
index 9a3603f6..0bb3e862 100644
--- a/docs/MigrationWorkflow.md
+++ b/docs/MigrationWorkflow.md
@@ -48,35 +48,37 @@ python scripts/check_alembic_migrations.py --verbose
 
 1. **Edit `app/models.py`** — add or modify SQLAlchemy model classes.
 
-2. **Generate the migration** from the repo root:
+2. **Generate the migration** from the repo root.  Use `--rev-id` to set the
+   revision identifier directly (avoids renaming afterwards):
+
+   ```bash
+   alembic revision --autogenerate --rev-id 037_add_my_new_table -m "add my new table"
+   ```
+
+   This creates `migrations/versions/037_add_my_new_table_add_my_new_table.py`
+   with `revision = "037_add_my_new_table"`.  Rename the file to match:
+
+   ```bash
+   mv migrations/versions/037_add_my_new_table_add_my_new_table.py \
+      migrations/versions/037_add_my_new_table.py
+   ```
+
+   Alternatively, generate with the default hash and then rename:
 
    ```bash
    alembic revision --autogenerate -m "add_my_new_table"
+   # Rename: mv migrations/versions/<hash>_add_my_new_table.py migrations/versions/037_add_my_new_table.py
+   # Update revision inside the file to match the filename stem.
    ```
 
    Alembic uses the `migrations/script.py.mako` template to generate the file.  The template includes inline comments about idempotent patterns — read them.
 
-3. **Rename the file** to follow the [naming convention](#migration-naming-convention):
-
-   ```bash
-   # Alembic generates a hash-based name by default.
-   # Rename to the sequential numbering scheme:
-   mv migrations/versions/<hash>_add_my_new_table.py \
-      migrations/versions/037_add_my_new_table.py
-   ```
-
-   Update the `revision` variable inside the file to match:
-
-   ```python
-   revision: str = "037_add_my_new_table"
-   ```
-
-4. **Review the generated code** — autogenerate is helpful but not perfect.  Check:
+3. **Review the generated code** — autogenerate is helpful but not perfect.  Check:
    - Are new tables and columns detected correctly?
    - Does the `downgrade()` reverse all changes?
    - Are SQLite-incompatible operations wrapped in `batch_alter_table()`?
 
-5. **Test the migration** against a fresh database:
+4. **Test the migration** against a fresh database:
 
    ```bash
    # Apply
@@ -89,7 +91,7 @@ python scripts/check_alembic_migrations.py --verbose
    alembic upgrade head
    ```
 
-6. **Run the chain validation**:
+5. **Run the chain validation**:
 
    ```bash
    python scripts/check_alembic_migrations.py
diff --git a/scripts/check_alembic_migrations.py b/scripts/check_alembic_migrations.py
index dee4050a..c399aef5 100644
--- a/scripts/check_alembic_migrations.py
+++ b/scripts/check_alembic_migrations.py
@@ -54,8 +54,8 @@ def _parse_down_revision(raw: str) -> list[str] | None:
     Returns a list with one or more strings otherwise.  Tuples are
     returned for merge migrations (e.g. ``("017_a", "017_b")``).
     """
-    raw = raw.strip().rstrip("#").strip()
-    # Handle inline comments
+    # Strip inline comments (e.g. ``None  # type: ignore``)
+    raw = raw.strip()
     if "#" in raw:
         raw = raw[: raw.index("#")].strip()
     try:
diff --git a/tests/test_check_alembic_migrations.py b/tests/test_check_alembic_migrations.py
index 611bf647..252d648b 100644
--- a/tests/test_check_alembic_migrations.py
+++ b/tests/test_check_alembic_migrations.py
@@ -22,7 +22,9 @@ main = _mod.main
 # ---------------------------------------------------------------------------
 
 
-def _write_migration(directory: Path, filename: str, revision: str, down_revision: str | None) -> Path:
+def _write_migration(
+    directory: Path, filename: str, revision: str, down_revision: str | tuple[str, ...] | None
+) -> Path:
     """Helper to create a minimal migration file."""
     if down_revision is None:
         down_rev_str = "None"
@@ -191,6 +193,7 @@ class TestMainCLI:
     def test_real_migrations(self) -> None:
         """Smoke test against the actual project migrations."""
         real_dir = Path(__file__).resolve().parent.parent / "migrations" / "versions"
-        if real_dir.is_dir():
-            rc = main(["--versions-dir", str(real_dir)])
-            assert rc == 0
+        if not real_dir.is_dir():
+            pytest.skip("migrations/versions directory not found in working tree")
+        rc = main(["--versions-dir", str(real_dir)])
+        assert rc == 0