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

@@ -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):
+3. **Review the generated code** — autogenerate is helpful but not perfect.  Check:
-
-   ```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:
+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

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()
+    # Strip inline comments (e.g. ``None  # type: ignore``)
-    # Handle inline comments
+    raw = raw.strip()
     if "#" in raw:
         raw = raw[: raw.index("#")].strip()
     try:

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():
+        if not real_dir.is_dir():
-            rc = main(["--versions-dir", str(real_dir)])
+            pytest.skip("migrations/versions directory not found in working tree")
-            assert rc == 0
+        rc = main(["--versions-dir", str(real_dir)])
+        assert rc == 0