fix(db): make alembic check clean on SQLite

CHANGELOG.md

@@ -16,6 +16,7 @@ All notable changes to this project are documented in this file. The format is b
 
 ### Fixed
 
+- `alembic check` and SQLite autogenerate no longer report the `file_storage` browse index (`sa.desc(sa.column("created_at"))` instead of `sa.text("created_at DESC")`) or `users_access_token.expires_at` (now `AwareDateTime`) as changed, so the host's `alembic check` on SQLite is clean. `make ci-doctor` now runs `alembic check` against its throwaway SQLite database, and a module index written with `sa.text("col DESC")` is no longer reported as changed on SQLite (#869).
 - `background_tasks`: the Redis invalidation listener now opens the connection a publish uses (one `PING` after every successful subscribe), so the first invalidation after boot or after a Redis outage no longer spends part of its 1 s budget connecting. Under load that connect was cancelled and the message was lost; other workers kept the stale value until its TTL (#880).
 
 ### Changed

Makefile

@@ -264,6 +264,7 @@ CI_DOCTOR_DB ?= ci-doctor.db
 ci-doctor:
 	rm -f $(CI_DOCTOR_DB)
 	SM_DATABASE_URL=sqlite+aiosqlite:///./$(CI_DOCTOR_DB) $(MAKE) migrate \
+		&& SM_DATABASE_URL=sqlite+aiosqlite:///./$(CI_DOCTOR_DB) uv run --project host alembic -c host/alembic.ini check \
 		&& SM_DATABASE_URL=sqlite+aiosqlite:///./$(CI_DOCTOR_DB) $(MAKE) doctor; \
 		rc=$$?; rm -f $(CI_DOCTOR_DB)*; exit $$rc
 

docs/module-authoring.md

@@ -368,7 +368,12 @@ generated file against this checklist before treating it as portable
    (`op.create_index(..., [sa.text("lower(email)")], if_not_exists=True)`) and
    confirm with the PostgreSQL round trip below. The warning is deliberately not
    an error: it fires on every autogenerate, including unrelated revisions, and
-   the Postgres round trip is the actual gate.
+   the Postgres round trip is the actual gate. Prefer writing an ordered index column as
+   `sa.desc(sa.column("col"))` over `sa.text("col DESC")` or `sa.desc("col")`:
+   the latter two are expressions to Alembic, and while the host's include filter
+   suppresses the false SQLite change when only the ordering differs, the column
+   form needs no filter and stays out of `SM` expression-index warnings
+   ([GH #869](https://github.com/antosubash/simple_module_python/issues/869)).
 3. **Enum types on downgrade.** `op.drop_table` leaves the PostgreSQL
    `CREATE TYPE` behind, so `downgrade` then `upgrade` fails with
    `DuplicateObject`. Autogenerate never emits the drop. Finish `downgrade()`

docs/superpowers/plans/2026-11-07-sqlite-alembic-check.md

@@ -0,0 +1,44 @@
+# Plan: make `alembic check` clean on SQLite (GH #869)
+
+Design: `docs/superpowers/specs/2026-11-07-sqlite-alembic-check-design.md`.
+
+## Task 1: SQLite text-ordered index guard (independent, marked hard)
+
+Files:
+- `framework/db/simple_module_db/migrations.py`
+- new `framework/db/simple_module_db/_sqlite_index_compare.py`
+- new `framework/db/tests/test_migrations_sqlite_index_compare.py`
+
+1. Write the tests first, against in-memory SQLite through a real `EnvironmentContext` and `compare_metadata`:
+   - a mixed `text("col DESC")` index produces no diff with the filter;
+   - the same index without the filter still shows the remove/add pair;
+   - a changed column, a uniqueness change and a `lower(email)` element are each still reported;
+   - with no active migration context, the filter neither raises nor skips.
+2. Add a Postgres test that patches the migration context's dialect to Postgres and shows the guard never skips there.
+3. Implement a private module `simple_module_db/_sqlite_index_compare.py` plus a branch in `include_object`, keeping `make_include_object`'s signature unchanged.
+
+Done: the tests pass, and they fail without the guard. `migrations.py` stays under 300 lines.
+
+## Task 2: models, docs and the CI gate (independent of Task 1)
+
+Files:
+- `modules/file_storage/file_storage/models.py`
+- `modules/users/users/models/access_token.py`
+- a new test in each module's tests
+- `Makefile` (`ci-doctor`)
+- `docs/module-authoring.md`
+- `CHANGELOG.md`
+
+Steps:
+1. Write tests:
+   - `file_storage`: no `TextClause` among the browse index's elements;
+   - `users`: an `expires_at` written and read on SQLite comes back aware UTC.
+2. Change `sa.text("created_at DESC")` to `sa.desc(sa.column("created_at"))`, and `expires_at` to `sa_type=AwareDateTime()`. Keep the `TIMESTAMPAware` import, because `created_at` still uses it.
+3. In `ci-doctor`, run `alembic check` against the same throwaway database after `make migrate`.
+4. Add one docs sentence and a CHANGELOG Fixed entry citing #869.
+
+Done: `make ci-doctor` passes locally, which means the `alembic check` on SQLite is clean.
+
+## Task 3: integrate and verify (after Tasks 1 and 2)
+
+Merge the Task 1 diff, then run `make lint`, `make test-py` (with `SM_TEST_REDIS_URL`), `make test-js`, `make ci-doctor` and `make migrations-roundtrip-pg` (dev-services Postgres).

docs/superpowers/specs/2026-11-07-sqlite-alembic-check-design.md

@@ -0,0 +1,111 @@
+# Make `alembic check` clean on SQLite (GH #869)
+
+## Goal and scope
+
+On a SQLite database migrated with `upgrade heads`, `alembic check` fails. So does every SQLite
+`revision --autogenerate`, which writes the same false operations into the new revision.
+Verified on `main` (8f3b9c9b), there are two false diffs:
+
+1. **#869:** `ix_file_storage_stored_file_tenant_browse` is reported as removed and re-added.
+   - The model writes `created_at DESC` as `sa.text("created_at DESC")`.
+   - SQLite reflects it as the plain column `created_at`, and Alembic compares a column with
+     a text element.
+2. **Found while reproducing #869:** `users_access_token.expires_at` is reported as a type
+   change, `DATETIME` → `TIMESTAMPAware`.
+   - Its migration (`b4c1e7d9a025`) created `sa.DateTime(timezone=True)`, and the model
+     declares fastapi-users' `TIMESTAMPAware`, whose impl is `TIMESTAMP`.
+   - SQLite reflects `DATETIME`, so the type differs. Postgres reflects
+     `TIMESTAMP WITH TIME ZONE` for both, which is why CI never saw either diff: its only
+     `alembic check` runs on Postgres.
+
+In scope: fix both, protect downstream modules from the first, and make CI run `alembic check`
+on SQLite. Out of scope: any schema change (no migration), and the expression-index
+limitation Alembic already handles (pure expression indexes are skipped on SQLite).
+
+## Approach
+
+1. **Fix the model in `file_storage`.** Replace `sa.text("created_at DESC")` with
+   `sa.desc(sa.column("created_at"))`. (Implementation note: the plain string form
+   `sa.desc("created_at")` wraps a text label, which Alembic still treats as an
+   expression, so it did not fix the diff.)
+   - Both compile to the identical `CREATE INDEX … (tenant_id, is_deleted, created_at DESC, id)`
+     on SQLite and Postgres, so no migration is needed.
+   - Measured: `desc()` over a column compares clean on SQLite and on Postgres, and the
+     `sa.text()` form compares clean only on Postgres.
+2. **Guard for other modules in `make_include_object`** (the issue's suggestion).
+   - On SQLite only, skip comparing an index pair when one side has a text element of the
+     form `<column> [ASC|DESC]` and both sides agree on uniqueness and column names, with
+     that element read as its column.
+   - Any other difference is still reported: a different column, a real expression such as
+     `lower(email)`, or a uniqueness change.
+   - The dialect comes from `alembic.context.get_context()`. When no migration context is
+     active, the guard does nothing.
+   - Scaffolded hosts get the guard without editing `env.py`, because they already call
+     `make_include_object`.
+3. **Fix the `expires_at` type in `users`.** Declare `sa_type=AwareDateTime()`, the repo
+   convention (CLAUDE.md § Database).
+   - It renders as `sa.DateTime(timezone=True)`, which matches the migration exactly.
+   - Values still read back aware UTC.
+   - On write, AwareDateTime converts an aware non-UTC value to UTC, where `TIMESTAMPAware`
+     stored its wall time as is. Every writer passes UTC (`now_utc`, the strategy's deadline),
+     so stored values do not change.
+   - `created_at` stays `TIMESTAMPAware`: the initial migration created it as that type, so it
+     already compares clean.
+4. **CI guard.** `make ci-doctor` already migrates a throwaway SQLite database. It now also runs
+   `alembic check` against that database, so a new SQLite-only false diff fails the PR instead
+   of reaching a downstream host.
+
+Rejected:
+- **The guard alone** (no model change): it leaves the framework's own model relying on a
+  comparison that is skipped.
+- **A `compare_type` hook that treats SQLite `TIMESTAMP` and `DATETIME` as equal:** every
+  existing host would have to edit its `env.py` to get it, and the model fix needs nothing.
+- **A migration that retypes `expires_at`:** Postgres already matches, so it would be a
+  no-op there. On SQLite it would be a batch table rebuild of the auth token table for no
+  behaviour change.
+
+## Decisions made
+
+- The guard matches only a single identifier with an optional `ASC`/`DESC`, plus optional
+  `NULLS FIRST|LAST`, optionally double-quoted. Anything else in a text element is a real
+  expression and keeps today's behaviour.
+- The guard lives in the private module `simple_module_db._sqlite_index_compare`, called from
+  the existing foreign-key filter's `include_object` in `simple_module_db.migrations`.
+  `make_include_object` keeps its signature. It reuses core's `unwrap_ordering` helper.
+- `docs/module-authoring.md` (Migrations section) gets one sentence: write an ordered index
+  column as `sa.desc(sa.column("col"))`, not `sa.text("col DESC")` or `sa.desc("col")`.
+- The guard also accepts the `sa.desc("col")` label form, and SM026 counts a column wrapped
+  in `desc()`/`asc()` as a column, not as an unverifiable expression.
+- CHANGELOG `[Unreleased]` → Fixed, citing #869.
+- The `alembic check` step in `ci-doctor` runs from the repo root like `make migrate`, using
+  the same `SM_DATABASE_URL`.
+
+## Testing
+
+- **New `framework/db/tests/test_migrations_sqlite_index_compare.py`**, against an in-memory
+  SQLite database through a real `EnvironmentContext`, so `get_context()` is exercised:
+  - a mixed `text("col DESC")` index compares clean with the filter;
+  - the same index without the filter still shows the remove/add pair, which proves the test
+    reproduces the bug;
+  - a column change, a uniqueness change, and a `lower(email)` text element are all still
+    reported;
+  - called with no active migration context, the filter neither raises nor skips.
+- **Postgres:** a test patches the migration context's dialect to Postgres and confirms the
+  guard never skips there (no live Postgres needed).
+- **`file_storage`:** the model's index elements are plain columns or `desc()`, with no text
+  element.
+- **`users`:** an `expires_at` round trip on SQLite reads back aware UTC.
+- **Whole branch:** `make ci-doctor` (now including `alembic check`) passes locally, and
+  `make migrations-roundtrip-pg` passes against the dev-services Postgres, proving Postgres
+  still reports no diff.
+- Full `make lint`, `make test-py` and `make test-js`. No browser QA: nothing user-visible
+  changes.
+
+## Risks
+
+- **`alembic check` on SQLite may surface other false diffs** in modules that CI does not
+  enable. `ci-doctor` migrates every installed module, and the local run found exactly these
+  two, so the gate is clean today.
+- **A downstream module may rely on `TIMESTAMPAware`'s non-UTC write behaviour for
+  `expires_at`.** The column belongs to `users`, and nothing outside it writes the column
+  (checked by grep).

framework/core/simple_module_core/diagnostics/_expression_index.py

@@ -25,13 +25,37 @@
 logger = logging.getLogger(__name__)
 
 
-def index_is_expression_based(index: Any) -> bool:
-    """True when any element of ``index`` is an expression rather than a plain column.
+_ORDERING_MODIFIERS = frozenset({"asc_op", "desc_op", "nulls_first_op", "nulls_last_op"})
+
+
+def unwrap_ordering(expr: Any) -> Any:
+    """``expr`` without its ``desc()``/``asc()``/``nulls_first()``/``nulls_last()`` wrappers."""
+    while getattr(getattr(expr, "modifier", None), "__name__", None) in _ORDERING_MODIFIERS:
+        expr = expr.element
+    return expr
+
 
-    A plain ``Column`` is attached to its table; a ``text()`` or function
-    element is not.
+def _is_column(expr: Any) -> bool:
+    """A column, possibly wrapped in ``desc()``/``asc()``/``nulls_last()``.
+
+    SQLite reflects ``created_at DESC`` as the column ``created_at`` and Alembic
+    compares it as one (GH #869), so an ordering keeps an element a column. A
+    plain ``Column`` is attached to its table; ``sa.column("x")`` is a named,
+    non-literal column; ``text()``, ``literal_column()``, a function or a bare
+    string label are expressions.
     """
-    return any(getattr(expr, "table", None) is None for expr in index.expressions)
+    expr = unwrap_ordering(expr)
+    if hasattr(expr, "element"):
+        # Still wraps something once the ordering is gone: a label over an
+        # arbitrary expression (``lower(email).label("x")``) or ``sa.desc("col")``'s
+        # bare-string label. A column wraps nothing.
+        return False
+    return getattr(expr, "table", None) is not None or getattr(expr, "is_literal", None) is False
+
+
+def index_is_expression_based(index: Any) -> bool:
+    """True when any element of ``index`` is an expression rather than a plain column."""
+    return not all(_is_column(expr) for expr in index.expressions)
 
 
 def find_expression_indexes(tables: Iterable[Any]) -> list[tuple[str, str]]:
@@ -103,4 +127,5 @@ def check_expression_indexes_unverifiable(
     "find_expression_indexes",
     "index_is_expression_based",
     "module_all_tables",
+    "unwrap_ordering",
 ]

framework/core/tests/test_expression_index_diagnostics.py

@@ -84,3 +84,39 @@ class NoModels(ModuleBase):
         meta = ModuleMeta(name="NoModels")
 
     assert module_all_tables(NoModels()) == []
+
+
+def test_a_descending_column_is_a_column_not_an_expression():
+    """SQLite reflects ``created_at DESC`` as a column and Alembic compares it (GH #869)."""
+    md = sa.MetaData()
+    table = sa.Table(
+        "t",
+        md,
+        sa.Column("id", sa.Integer, primary_key=True),
+        sa.Column("created_at", sa.DateTime),
+        sa.Index("ix_t_named", "id", sa.desc(sa.column("created_at"))),
+    )
+    sa.Index("ix_t_bound", table.c.created_at.desc().nulls_last())
+    assert find_expression_indexes([table]) == []
+
+
+def test_text_ordering_and_literal_columns_still_count_as_expressions():
+    md = sa.MetaData()
+    table = sa.Table(
+        "t",
+        md,
+        sa.Column("id", sa.Integer, primary_key=True),
+        sa.Column("created_at", sa.DateTime),
+        sa.Index("ix_t_text", "id", sa.text("created_at DESC")),
+        sa.Index("ix_t_label", "id", sa.desc("created_at")),
+        sa.Index("ix_t_literal", sa.desc(sa.literal_column("created_at"))),
+    )
+    sa.Index("ix_t_labelled_func", sa.func.lower(table.c.id).label("x"))
+    sa.Index("ix_t_cast", sa.cast(table.c.created_at, sa.String).desc())
+    assert find_expression_indexes([table]) == [
+        ("t", "ix_t_cast"),
+        ("t", "ix_t_label"),
+        ("t", "ix_t_labelled_func"),
+        ("t", "ix_t_literal"),
+        ("t", "ix_t_text"),
+    ]

framework/db/simple_module_db/_sqlite_index_compare.py

@@ -0,0 +1,97 @@
+"""Index comparisons SQLite cannot make faithfully (GH #869).
+
+SQLite reflects ``CREATE INDEX ... (tenant_id, created_at DESC, id)`` as three
+plain columns. A model that wrote the ordered column as ``sa.text("created_at
+DESC")`` or ``sa.desc("created_at")`` holds an expression there, and Alembic
+compares the two element by element: every autogenerate on SQLite then writes a
+drop and re-create of a correct index, and ``alembic check`` fails on a correct
+schema. Postgres reflects the ordering, so it compares these faithfully.
+
+:func:`differs_only_in_sqlite_ordering` recognises exactly that pair, so
+``make_include_object`` can skip it on SQLite. Anything else still reaches
+Alembic: a different column list, a uniqueness change, or a real expression
+such as ``lower(email)``. A change of ordering alone (``DESC`` to ``ASC``) is not
+reported on SQLite, which cannot reflect ordering at all; the Postgres round
+trip (``make migrations-roundtrip-pg``) is what catches it.
+"""
+
+from __future__ import annotations
+
+import logging
+import re
+
+from alembic import context
+from simple_module_core.diagnostics._expression_index import unwrap_ordering
+from sqlalchemy import Index
+from sqlalchemy.schema import SchemaItem
+from sqlalchemy.sql.elements import ColumnClause, TextClause
+
+logger = logging.getLogger(__name__)
+
+# One identifier, optionally double-quoted, then an optional ordering.
+_ORDERED_COLUMN = re.compile(
+    r'\s*(?:"([^"]+)"|([A-Za-z_][A-Za-z0-9_]*))'
+    r"(?:\s+(?:ASC|DESC))?(?:\s+NULLS\s+(?:FIRST|LAST))?\s*",
+    re.IGNORECASE,
+)
+
+
+def _on_sqlite() -> bool:
+    """Whether the active Alembic migration context targets SQLite.
+
+    Outside a migration (no context configured) the answer is no, so the
+    caller never suppresses a comparison it cannot place.
+    """
+    try:
+        return context.get_context().dialect.name == "sqlite"
+    except Exception as exc:
+        logger.debug("No Alembic migration context, SQLite index guard off: %s", exc)
+        return False
+
+
+def _element_name(element: object) -> tuple[str | None, bool]:
+    """``(column name, written as an expression)`` for one index element.
+
+    The name is ``None`` for a real expression, which never matches.
+    """
+    element = unwrap_ordering(element)
+    if isinstance(element, TextClause):
+        match = _ORDERED_COLUMN.fullmatch(element.text)
+        return (match[1] or match[2], True) if match else (None, True)
+    if isinstance(element, ColumnClause) and not element.is_literal:
+        return element.name, False
+    # ``sa.desc("col")`` wraps a textual label reference holding the bare name.
+    label = getattr(element, "element", None)
+    if type(element).__name__ == "_textual_label_reference" and isinstance(label, str):
+        return label, True
+    return None, False
+
+
+def _is_partial(index: Index) -> bool:
+    """A partial index (``WHERE`` predicate), modelled or reflected."""
+    return any(
+        index.dialect_options[dialect].get("where") is not None
+        for dialect in ("sqlite", "postgresql")
+    )
+
+
+def differs_only_in_sqlite_ordering(left: SchemaItem, right: SchemaItem | None) -> bool:
+    """True when two indexes are the same index as far as SQLite can tell.
+
+    Requires an active SQLite migration context, equal uniqueness, no partial
+    index (``WHERE`` predicate) on either side, the same
+    column names in the same order (compared case-insensitively, as SQLite
+    compares identifiers), and at least one element written as an expression
+    (otherwise Alembic's own comparison is already right).
+    """
+    if not isinstance(left, Index) or not isinstance(right, Index) or left.unique != right.unique:
+        return False
+    if _is_partial(left) or _is_partial(right):
+        # Only columns are compared here, so a changed predicate would be hidden.
+        return False
+    if not _on_sqlite():
+        return False
+    sides = [[_element_name(e) for e in index.expressions] for index in (left, right)]
+    names = [[name.casefold() if name else None for name, _ in side] for side in sides]
+    written_as_expression = any(is_expr for side in sides for _, is_expr in side)
+    return written_as_expression and None not in names[0] and names[0] == names[1]

framework/db/simple_module_db/migrations.py

@@ -28,6 +28,7 @@
 from sqlalchemy.schema import SchemaItem
 
 from simple_module_db._migration_render import render_item  # noqa: F401  (re-exported)
+from simple_module_db._sqlite_index_compare import differs_only_in_sqlite_ordering
 from simple_module_db.base import all_module_bases
 from simple_module_db.extensions import (  # noqa: F401  (ensure_db_extensions re-exported)
     ensure_db_extensions,
@@ -147,6 +148,9 @@ def include_object(
     ) -> bool:
         if type_ == "table":
             return name in allowlist
+        if type_ == "index" and differs_only_in_sqlite_ordering(object, compare_to):
+            # SQLite reflects ``created_at DESC`` as a plain column (GH #869).
+            return False
         if (
             ignore_unmodeled_fks
             and type_ == "foreign_key_constraint"