fix(db): autogenerate keeps new FKs; enum rendering matches the model; nested import errors surface; doctor uses filtered sessions

CHANGELOG.md

@@ -73,6 +73,18 @@ All notable changes to this project are documented in this file. The format is b
 
   A single-tenant install with nothing bound keeps running the last two. See
   `docs/framework/multi-tenancy.md`.
+- Alembic autogenerate now emits newly modeled foreign keys on existing tables
+  while preserving migration-only reflected foreign keys (#812). A foreign key
+  with the same columns and target but changed options (`ondelete`, `deferrable`)
+  is emitted as a drop plus a create; on SQLite wrap those ops in `op.batch_alter_table(...)` by hand.
+- **Behaviour change:** StrEnum migration rendering preserves configured labels and enum storage options,
+  keeping runtime reads/writes and generated constraints consistent; a plain
+  `StrEnum` field now renders SQLAlchemy's member-name labels, matching what the
+  model binds (#813).
+- Metadata discovery propagates missing dependencies inside models modules instead
+  of silently producing an incomplete schema (#814).
+- **Behaviour change:** doctor data compatibility checks use framework DB sessions with soft-delete and
+  strict tenant filters; installation-wide checks must opt into `all_tenants()` (#815).
 - **Behaviour change:** `LocaleMiddleware` never picks a language the browser
   sends with `q=0` ("not acceptable"), including through a regional tag such
   as `es-MX;q=0` and when `q` follows another parameter or is spelled `Q`;

docs/database/migrations.md

@@ -2,6 +2,23 @@
 
 All migrations live in `host/migrations/versions/` — **not** in module packages. Alembic runs from the repo root via `host/alembic.ini` and shares the app's `.env` / `SM_DATABASE_URL`.
 
+Autogenerate's `make_include_object()` filter preserves foreign keys declared
+only in migrations by ignoring unmatched **reflected** constraints. Newly modeled
+foreign keys on existing tables still generate `CreateForeignKeyOp`; set
+`ignore_unmodeled_fks=False` to allow dropping database-only constraints (#812).
+A foreign key with the same columns and target but different options (`ondelete`,
+`deferrable`) is emitted as a drop plus a create; one with other columns or another
+target is a different constraint and stays preserved. SQLite cannot `ALTER` constraints: wrap such ops in
+`op.batch_alter_table(...)` by hand before applying the revision there.
+
+For `sa.Enum` columns backed by Python `StrEnum`, generated revisions freeze the
+column's actual configured labels and preserve `name`, `native_enum`,
+`create_constraint`, `length` and configured `values_callable` behavior without
+importing application enums/callables. Plain enum fields store member names
+(e.g. `READY`); to store values (e.g. `ready`), configure the **runtime** type with
+`values_callable=lambda e: [m.value for m in e]`. Custom callable labels are frozen
+in the revision. Existing revisions are not rewritten (#813).
+
 ## Why centralized?
 
 - **Dependency ordering is global.** If `invoices` depends on `orders_order.id`, their migrations must order correctly. One shared Alembic history, with `down_revision` links between modules, enforces this.
@@ -31,6 +48,14 @@ first runs defer checks until the wizard applies migrations. No automatic
 credential deletion happens at startup. See the [scaffolded host upgrade
 guide](../guide/upgrading.md) for the operator workflow and token sign-out risk.
 
+Doctor uses the DB session factory and listeners used at boot: soft-deleted
+rows are hidden and tenant queries are scoped. Because doctor has no host settings,
+it always enforces strict tenant isolation, which is stricter than a single-tenant
+boot: a check that passes doctor also passes boot. A check must use `tenant_context(id)`
+for one tenant or explicitly use `all_tenants()` / `execution_options(all_tenants=True)`
+for an installation-wide check. Add `execution_options(include_deleted=True)`
+only when the check also needs trashed rows. These opt-ins work at boot too (#815).
+
 ## The real graph
 
 The history is **not** a single linear chain. It has several roots and several heads, and `make migrate` runs `upgrade heads` for that reason. On `main` today:
@@ -174,7 +199,7 @@ context.configure(
 
 - `target_metadata` — union of every module's `MetaData`.
 - `include_object` — accepts only tables present in `target_metadata`, so autogenerate never diffs system tables (`alembic_version`) or any host-owned tables outside the module system.
-- `render_item` — collapses SQLModel's `AutoString` to `sa.String` and renders `StrEnum` columns with `values_callable` so generated migrations are importable.
+- `render_item` — collapses SQLModel's `AutoString` to `sa.String` and renders `StrEnum` columns with their configured labels and storage options (no application imports in revisions, #813).
 - `process_revision_directives` — re-emits expression-based (functional) indexes that autogenerate silently drops under SQLite.
 
 ## Boot-time migration check

docs/database/per-module-base.md

@@ -34,15 +34,15 @@ target_metadata = build_module_metadata()
 
 This function iterates every discovered module, imports its `models` submodule (if one exists), and unions all the per-module `MetaData`s into one. Autogenerate then diffs the DB against that union.
 
-If a module has no `models.py`, it contributes nothing — fine. If a module has a `models.py` that doesn't import, the union fails — fix the import.
+If a module has no `models.py` (or its parent package is absent), it contributes nothing — fine. If an existing `models.py` fails to import a dependency, the union fails with that import error — fix the import. Only `ModuleNotFoundError` naming the models module itself or one of its parents is skipped; missing sibling modules and dependencies are errors (#814).
 
 ## `make_include_object()`
 
 Alembic's `include_object` callback filters which tables `autogenerate` considers. `host/migrations/env.py` calls `make_include_object(target_metadata)` from `simple_module_db` to:
 
 - **Include** only tables present in the combined module `MetaData` (the allowlist is `{t.name for t in metadata.tables.values()}`).
 - **Exclude** everything else — tables owned by the Alembic runtime (`alembic_version`) and any host-owned tables that aren't part of an installed module.
-- Optionally skip unmodeled foreign-key constraints (`ignore_unmodeled_fks=True` by default) so migration-level cross-module FKs aren't dropped on every autogen run.
+- Skip unmatched reflected foreign-key constraints (`ignore_unmodeled_fks=True` by default) so migration-level cross-module FKs aren't dropped on every autogen run. Newly modeled FKs on existing tables are still included (#812).
 
 If you write a one-off host-level table that autogenerate shouldn't track, keep it out of the module metadata — `make_include_object` already excludes anything not in the allowlist.
 

docs/framework/lifecycle.md

@@ -368,8 +368,9 @@ Use stable semantic ids and inspect actual schema/data state so hosts with
 independently ported revisions pass. Checks run before module DB-settings
 hydration and `on_startup`; errors refuse boot outside development. A never-
 migrated first run defers checks until migrations have been applied. Doctor
-reports the same issues as `SM033`. Checks must never clear credentials or apply
-migrations; see [Migrations](../database/migrations.md).
+reports the same issues as `SM033`, through the same filtered sessions
+(tenant and soft-delete rules in [Migrations](../database/migrations.md)).
+Checks must never clear credentials or apply migrations.
 
 ## `on_startup()` / `on_shutdown()`
 

docs/module-authoring.md

@@ -376,8 +376,9 @@ generated file against this checklist before treating it as portable
    `sa.Enum(Status)` the stored label is the member *name*
    (`server_default="PUBLISH"`), not its value (`"publish"`). SQLite accepts
    either; PostgreSQL rejects the value with `invalid input value for enum`.
-   (A `StrEnum` rendered with `values_callable`, which is what `render_item`
-   emits, stores the *value* — match the default to whichever the column uses.)
+   (A `StrEnum` column whose runtime type sets `values_callable` stores the
+   *value*; `render_item` freezes whichever labels the runtime type uses, so
+   match the default to the column's labels.)
 
 Prove it on PostgreSQL before merging. Create a disposable database once
 (`createdb sm_migrations_check` on the shared dev Postgres), then:

framework/db/simple_module_db/_migration_render.py

@@ -18,10 +18,10 @@ def render_item(type_, obj, autogen_context):
       to ``sa.DateTime(timezone=True)`` so migrations don't need to
       ``import sqlmodel`` (GH #440). Any other ``sqlmodel`` type still gets
       the import added, so the migration stays importable.
-    * Renders Python ``StrEnum`` columns with ``values_callable`` so the
-      Postgres enum labels match the lowercase ``StrEnum`` values rather than
-      SQLAlchemy's default of using uppercase attribute names. This means raw
-      SQL like ``WHERE status = 'ready'`` actually works against the live DB.
+    * Renders Python ``StrEnum`` columns using their configured labels and
+      storage options, without importing application enums into revisions.
+      Value-based labels require ``values_callable`` on the runtime type too;
+      plain enum columns keep SQLAlchemy's member-name labels.
     * Renders ``true()``/``false()`` server defaults as ``sa.true()``/``sa.false()``
       instead of the dialect-specific ``sa.text('1')``/``sa.text('0')`` (GH #342).
     * Renders ``LTree``/``TSVector`` through ``simple_module_db.types`` (with
@@ -63,10 +63,19 @@ def render_item(type_, obj, autogen_context):
     if isinstance(obj, sa.Enum):
         python_type = getattr(obj, "enum_class", None)
         if isinstance(python_type, type) and issubclass(python_type, StrEnum):
-            name = getattr(obj, "name", None) or python_type.__name__.lower()
-            members = ", ".join(repr(m.value) for m in python_type)
-            return (
-                f"sa.Enum({members}, name={name!r}, values_callable=lambda e: [m.value for m in e])"
-            )
+            members = ", ".join(repr(label) for label in obj.enums)
+            options = [f"name={obj.name!r}"]
+            if not obj.native_enum:
+                options.append("native_enum=False")
+            if obj.create_constraint:
+                options.append("create_constraint=True")
+            if obj.length != max((len(label) for label in obj.enums), default=0):
+                options.append(f"length={obj.length!r}")
+            if obj.values_callable is not None:
+                # Freeze custom labels; revisions must not import application callables.
+                values = [member.value for member in python_type]
+                callable_body = "[m.value for m in e]" if obj.enums == values else repr(obj.enums)
+                options.append(f"values_callable=lambda e: {callable_body}")
+            return f"sa.Enum({members}, {', '.join(options)})"
 
     return False  # let alembic use its default rendering

framework/db/simple_module_db/data_compatibility.py

@@ -7,19 +7,29 @@
 from simple_module_core.data_compatibility import collect_data_compatibility
 from simple_module_core.diagnostics import Diagnostic, DiagnosticLevel
 from simple_module_core.module import ModuleBase
-from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine
+
+from simple_module_db.listeners import register_listeners
+from simple_module_db.session import init_db
 
 DATA_COMPATIBILITY_CODE = "SM033"
 
 
 async def database_compatibility_diagnostics(
     modules: Sequence[ModuleBase], database_url: str
 ) -> list[Diagnostic]:
+    state = None
     try:
         if not _database_exists(database_url):
             return []  # Nothing migrated yet; opening it would create an empty SQLite file.
-        engine = create_async_engine(database_url)
+        # Read-only diagnostics: never switch a legacy SQLite file to WAL.
+        state = init_db(database_url, sqlite_wal=False)
+        # Doctor has no host settings: checks must explicitly opt into all tenants.
+        state.tenant_strict = True
+        register_listeners(state)
+        engine = state.engine
     except Exception:
+        if state is not None:
+            await state.engine.dispose()
         # A malformed or non-async URL must be a finding, not a traceback; the URL can
         # carry credentials, so it is not echoed.
         return [_problem(DiagnosticLevel.WARNING, "Could not open the database URL.")]
@@ -37,7 +47,7 @@ async def database_compatibility_diagnostics(
         if not migrated:
             return []
         try:
-            async with async_sessionmaker(engine)() as db:
+            async with state.session_factory() as db:
                 issues = await collect_data_compatibility(modules, db)
         except Exception as exc:
             # A check that crashes is a bug to fix, not a pass: doctor must fail.

framework/db/simple_module_db/migrations.py

@@ -17,13 +17,14 @@
 import importlib
 import logging
 from collections.abc import Callable, Sequence
-from typing import Literal
+from typing import Any, Literal
 
 from alembic.operations.ops import CreateIndexOp, CreateTableOp, DropIndexOp, DropTableOp
 from simple_module_core import ModuleBase
 from simple_module_core.diagnostics._expression_index import index_is_expression_based
 from simple_module_core.discovery import discover_modules, get_module_package_name
 from sqlalchemy import Index, MetaData
+from sqlalchemy.exc import NoReferenceError
 from sqlalchemy.schema import SchemaItem
 
 from simple_module_db._migration_render import render_item  # noqa: F401  (re-exported)
@@ -70,9 +71,14 @@ def build_module_metadata(modules: Sequence[ModuleBase] | None = None) -> MetaDa
 
     for mod in modules:
         pkg = get_module_package_name(mod)
+        models_module = f"{pkg}.models"
         try:
-            importlib.import_module(f"{pkg}.models")
-        except ModuleNotFoundError:
+            importlib.import_module(models_module)
+        except ModuleNotFoundError as exc:
+            if exc.name is None or not (
+                exc.name == models_module or models_module.startswith(f"{exc.name}.")
+            ):
+                raise
             logger.debug("No models submodule for module '%s' (pkg=%s)", mod.meta.name, pkg)
 
     combined = MetaData()
@@ -82,6 +88,32 @@ def build_module_metadata(modules: Sequence[ModuleBase] | None = None) -> MetaDa
     return combined
 
 
+def _fk_signature(fk: Any) -> tuple[str, frozenset[tuple[str, str]]] | None:
+    """Referred table and (local column, referred column) pairs of a foreign key."""
+    try:
+        elements = list(fk.elements)
+        referred = elements[0].column.table.fullname
+    except (AttributeError, IndexError, LookupError, NoReferenceError):
+        return None  # unresolvable target: never treat it as a match
+    return referred, frozenset((e.parent.name, e.column.name) for e in elements)
+
+
+def _fk_replaced_in_model(metadata: MetaData, reflected_fk: SchemaItem) -> bool:
+    """Whether the model declares the same foreign key with different options.
+
+    A changed option (``ondelete``, ``deferrable``...) makes Alembic report the old
+    constraint as removed and the new one as added. Hiding only the removal would
+    leave the database with two foreign keys on the same columns. A key that differs
+    in its columns or its target is a different constraint and stays preserved.
+    """
+    table = getattr(reflected_fk, "table", None)
+    modeled = metadata.tables.get(table.fullname) if table is not None else None
+    signature = _fk_signature(reflected_fk)
+    if modeled is None or signature is None:
+        return False
+    return any(_fk_signature(fk) == signature for fk in modeled.foreign_key_constraints)
+
+
 def make_include_object(
     metadata: MetaData,
     *,
@@ -96,8 +128,8 @@ def make_include_object(
     table added by the host developer outside the module system).
 
     :param ignore_unmodeled_fks: When ``True`` (default), foreign-key
-        constraints that exist in the live DB but are absent from the SQLModel
-        metadata are skipped instead of being emitted as ``op.drop_constraint``
+        constraints that exist in the live DB (reflected) but are absent from the
+        SQLModel metadata are skipped instead of being emitted as ``op.drop_constraint``
         in the autogenerated migration. The framework's recommended pattern
         declares cross-module FKs at the migration level only (no Python-level
         relationship between modules), so those constraints look "unmodeled" to
@@ -115,7 +147,13 @@ def include_object(
     ) -> bool:
         if type_ == "table":
             return name in allowlist
-        if ignore_unmodeled_fks and type_ == "foreign_key_constraint" and compare_to is None:
+        if (
+            ignore_unmodeled_fks
+            and type_ == "foreign_key_constraint"
+            and reflected
+            and compare_to is None
+            and not _fk_replaced_in_model(metadata, object)
+        ):
             return False
         parent_table = getattr(object, "table", None)
         if parent_table is not None: