fix(db,audit_log): bulk audit prefetch keeps loader criteria; ORM restores and PK changes are audited

CHANGELOG.md

@@ -53,6 +53,11 @@ All notable changes to this project are documented in this file. The format is b
   `account` or whose `azp` is another client (only `oidc` pins both).
 
 ### Fixed
+- Bulk-DML audit prefetch preserves statement options, including loader criteria,
+  so it records only the rows selected by the UPDATE or DELETE (#810).
+- ORM restores now emit `restored` audit entries with translated action labels
+  and ORM primary-key updates include the old and new key in their diffs
+  (behaviour change: both used to leave no entry) (#811).
 - Tenant scoping in `simple_module_db` now applies to more statement shapes:
   ORM bulk UPDATE by primary key (`session.execute(update(Model), rows)`),
   `update().ordered_values()` (assigning `tenant_id` raises like `.values()`),

docs/modules/audit_log.md

@@ -1,6 +1,6 @@
 # audit_log
 
-Automatic field-level audit trail for every SQLModel entity in the app. Each create / update / delete / soft-delete is captured in `audit_log_audit_entry` with the changed fields, the user who made the change, and the request correlation id — **no instrumentation required in module code**. Entities opt out, they don't opt in.
+Automatic field-level audit trail for every SQLModel entity in the app. Each create / update / delete / soft-delete / restore is captured in `audit_log_audit_entry` with the changed fields, the user who made the change, and the request correlation id — **no instrumentation required in module code**. Entities opt out, they don't opt in.
 
 ## ModuleMeta
 
@@ -37,7 +37,7 @@ audit_log = "audit_log.capture:audit_callback"
 
 `session.execute(update(Model)...)`, its `delete()` / `insert()` siblings and Core DML on `Model.__table__` never reach a flush, so a `do_orm_execute` listener (`simple_module_db.bulk_audit`) audits them instead, through the same per-engine `audit_callback` (requests and Celery workers alike):
 
-- **`UPDATE` / `DELETE`.** Before the statement runs, the listener reads the primary keys it will touch, with the same `WHERE` and the same tenant scope (soft-deleted rows included, as the write includes them). Up to 500 rows each get one entry (`bulk_updated`, `bulk_deleted`; an update that writes `is_deleted` is `bulk_soft_deleted` when the value is truthy or SQL such as `sa.true()`, else `bulk_restored`, decided per row for executemany; the statement's other written columns stay in `changes`); `changes` holds the `SET` values as `{field, new}` (a SQL expression is stored as SQL with bind placeholders). `session.execute(update(Model), [{"id": ..., ...}, ...])` takes the ids from the rows instead.
+- **`UPDATE` / `DELETE`.** Before the statement runs, the listener reads the primary keys it will touch, with the same `WHERE`, statement options (including `with_loader_criteria`) and tenant scope (soft-deleted rows included, as the write includes them). Up to 500 rows each get one entry (`bulk_updated`, `bulk_deleted`; an update that writes `is_deleted` is `bulk_soft_deleted` when the value is truthy or SQL such as `sa.true()`, else `bulk_restored`, decided per row for executemany; the statement's other written columns stay in `changes`); `changes` holds the `SET` values as `{field, new}` (a SQL expression is stored as SQL with bind placeholders). `session.execute(update(Model), [{"id": ..., ...}, ...])` takes the ids from the rows instead.
 - **More than 500 rows.** One summary entry with `entity_id` `"*"`: the `SET` values plus `where_sql` (the `WHERE` with bind placeholders, never the literal values), `rowcount` and `truncated: true`.
 - **`INSERT`.** One `bulk_inserted` summary entry per statement (`entity_id` `"*"`, the column values, or the column names for a multi-row insert).
 - **Only real targets count.** `changes` comes from the statement's `SET` / `VALUES` targets, each resolved to its model attribute (bound values read through the execute params). An execute-param key that names a column (and is not one of the statement's bind parameters) is written by SQLAlchemy, so it is a target too, for a single dict and for every executemany row; any other param key is ignored. Primary-key and `is_deleted` writes are never treated as excluded bookkeeping.
@@ -64,11 +64,12 @@ record_bulk_audit(session, Order, "bulk_updated", ids, {"status": "archived"})
 | `created` | object in `session.new` |
 | `updated` | object in `session.dirty` with real column changes (empty diffs are skipped) |
 | `deleted` | object in `session.deleted` (hard delete) |
-| `soft_deleted` | a `SoftDeleteMixin` row the soft-delete listener moved into `session.new` with `is_deleted=True` |
+| `soft_deleted` | a persistent `SoftDeleteMixin` row whose `is_deleted` changes to `True` (including a delete intercepted by the soft-delete listener) |
+| `restored` | a persistent `SoftDeleteMixin` row whose `is_deleted` changes from `True` to `False` |
 | `bulk_updated` / `bulk_deleted` / `bulk_soft_deleted` / `bulk_restored` | statement `UPDATE` / `DELETE` (see above), one per row or one `"*"` summary |
 | `bulk_inserted` | statement `INSERT`, one `"*"` summary per statement |
 
-`changes` is a list of `{field, new}` (creates) or `{field, old, new}` (updates) entries; deletes and soft-deletes carry an empty change list. Column values are normalised to JSON-safe scalars (non-primitives are stringified).
+`changes` is a list of `{field, new}` (creates) or `{field, old, new}` (updates) entries; hard deletes carry an empty change list. Soft-deletes and restores retain any other field edits made in the same flush, or carry an empty list when only the lifecycle flag changed (a `VersionedMixin` model also lists its bumped `version`). Column values are normalised to JSON-safe scalars (non-primitives are stringified).
 
 ## Opting out
 
@@ -99,7 +100,7 @@ class Cache(Base, table=True):
 
 An update whose changed columns are *all* excluded writes no audit row at all, so excluding a model's volatile columns is how to audit its meaningful transitions without one row per heartbeat. `background_tasks` does both: `OutboxMessage` (delivery bookkeeping) sets `__audit_exclude__ = True`, and `TaskExecution` excludes `heartbeat_at`, `queued_at`, `started_at`, `finished_at` and `retries`, so its creation and status changes are audited while the writes in between are not. This applies to request-path and Celery-worker writes alike.
 
-Primary-key columns and the bookkeeping fields injected by `AuditMixin` (`created_at`, `updated_at`, `created_by`, `updated_by`) and `SoftDeleteMixin` (`is_deleted`, `deleted_at`, `deleted_by`) are excluded from diffs automatically.
+Primary-key columns are excluded on create; ORM updates include their old and new values, and `entity_id` resolves to the new key after flush (#811). The bookkeeping fields injected by `AuditMixin` (`created_at`, `updated_at`, `created_by`, `updated_by`) and `SoftDeleteMixin` (`is_deleted`, `deleted_at`, `deleted_by`) are excluded from diffs automatically.
 
 ## Routes
 
@@ -146,7 +147,7 @@ from audit_log.contracts.schemas import AuditEntryRead, AuditEntryList
 | `id` | `UUID` | PK |
 | `entity_type` | `str(255)` | indexed; the model class name |
 | `entity_id` | `str(255)` | indexed; the resolved PK as a string |
-| `action` | `str(20)` | `created` / `updated` / `deleted` / `soft_deleted` |
+| `action` | `str(20)` | `created` / `updated` / `deleted` / `soft_deleted` / `restored`, plus the statement-DML actions above |
 | `changes` | JSON | list of field diffs |
 | `user_id` | `str(255) \| None` | indexed; from the request's `current_user_id` |
 | `correlation_id` | `str(255) \| None` | request correlation id |

framework/db/simple_module_db/_bulk_audit_records.py

@@ -9,8 +9,6 @@
 from simple_module_core.correlation import correlation_id
 from sqlalchemy import inspect as sa_inspect
 from sqlalchemy.orm import ORMExecuteState, Session
-from sqlalchemy.sql.elements import BindParameter
-from sqlalchemy.sql.visitors import iterate
 
 from simple_module_db.audit import AuditRecord, _serialize, format_pk
 from simple_module_db.query_filter import engine_policy
@@ -61,15 +59,6 @@ def _current_user() -> str | None:
     return current_user_id.get()
 
 
-def _where_params(stmt: Any, params: Any) -> dict[str, Any] | None:
-    """The execute params the ``WHERE`` clause binds, so the prefetch can run it."""
-    where = getattr(stmt, "whereclause", None)
-    if where is None or not isinstance(params, dict):
-        return None
-    keys = {e.key for e in iterate(where) if isinstance(e, BindParameter)}
-    return {k: v for k, v in params.items() if k in keys} or None
-
-
 def _bulk_by_pk_ids(execute_state: ORMExecuteState, cls: type) -> list[str] | None:
     """``session.execute(update(Model), [{pk..., col...}, ...])``: ids from the rows."""
     params = execute_state.parameters

framework/db/simple_module_db/audit.py

@@ -61,7 +61,7 @@ class _PendingChange:
 
     obj_ref: object
     entity_type: str
-    action: str  # "created" | "updated" | "deleted" | "soft_deleted"
+    action: str  # "created" | "updated" | "deleted" | "soft_deleted" | "restored"
     changes: list[dict[str, Any]]
     user_id: str | None
     correlation_id: str | None
@@ -116,13 +116,11 @@ def _serialize(value: Any) -> Any:
     return str(value)
 
 
-def _column_diffs(
-    obj: object, inspector: Any, excl: frozenset[str], pk_cols: set[str]
-) -> list[dict[str, Any]]:
-    """Old/new diffs for the changed, non-excluded, non-PK columns of *obj*."""
+def _column_diffs(obj: object, inspector: Any, excl: frozenset[str]) -> list[dict[str, Any]]:
+    """Old/new diffs for the changed, non-excluded columns of *obj*."""
     changes: list[dict[str, Any]] = []
     for col_name in _column_names(obj):
-        if col_name in excl or col_name in pk_cols:
+        if col_name in excl:
             continue
         hist = inspector.attrs[col_name].history
         if not hist.has_changes():
@@ -197,19 +195,15 @@ def _get_pk_cols(obj: object) -> set[str]:
         if _is_excluded(obj):
             continue
         inspector = sa_inspect(obj)
-        changes = _column_diffs(obj, inspector, _get_excluded(obj), _get_pk_cols(obj))
+        changes = _column_diffs(obj, inspector, _get_excluded(obj))
         # The soft-delete listener re-adds persistent objects as dirty. Edits made in
-        # the same flush stay on the soft_deleted record.
-        if (
-            isinstance(obj, SoftDeleteMixin)
-            and obj.is_deleted
-            and inspector.attrs.is_deleted.history.has_changes()
-        ):
+        # the same flush stay on the lifecycle record, including a restore.
+        if isinstance(obj, SoftDeleteMixin) and inspector.attrs.is_deleted.history.has_changes():
             pending.append(
                 _PendingChange(
                     obj_ref=obj,
                     entity_type=type(obj).__name__,
-                    action="soft_deleted",
+                    action="soft_deleted" if obj.is_deleted else "restored",
                     changes=changes,
                     user_id=user_id,
                     correlation_id=correlation_id,

framework/db/simple_module_db/bulk_audit.py

@@ -30,6 +30,9 @@
 from sqlalchemy import inspect as sa_inspect
 from sqlalchemy import select
 from sqlalchemy.orm import ORMExecuteState
+from sqlalchemy.orm.util import LoaderCriteriaOption
+from sqlalchemy.sql.elements import BindParameter
+from sqlalchemy.sql.visitors import iterate
 
 from simple_module_db import model_registry as registry
 from simple_module_db._bulk_audit_records import (  # noqa: F401  (re-exported)
@@ -46,7 +49,6 @@
     _current_user,
     _deliver,
     _summary,
-    _where_params,
     record_bulk_audit,
 )
 from simple_module_db.audit import _excluded_fields, format_pk
@@ -115,21 +117,47 @@ def _plan(execute_state: ORMExecuteState, cls: type) -> _Ctx | None:
     return ctx
 
 
+def _bound_params(params: Any, *clauses: Any) -> dict[str, Any] | None:
+    """The execute params the prefetch's own clauses bind (``WHERE`` and loader criteria).
+
+    Only those keys pass, so a caller's param cannot collide with a bind the SELECT
+    adds itself (its ``LIMIT``). An executemany list never reaches the prefetch.
+    """
+    if not isinstance(params, dict) or not params:
+        return None
+    keys = {
+        el.key
+        for clause in clauses
+        if clause is not None
+        for el in iterate(clause)
+        if isinstance(el, BindParameter)
+    }
+    return {k: v for k, v in params.items() if k in keys} or None
+
+
 def _prefetch_ids(execute_state: ORMExecuteState, cls: type, cap: int) -> list[str]:
     """Primary keys of the rows the statement will touch, at most ``cap + 1``."""
     stmt = execute_state.statement
     if execute_state.all_mappers:
         cols = [getattr(cls, col.key) for col in sa_inspect(cls).primary_key]
     else:
         cols = list(stmt.table.primary_key.columns)
-    query = select(*cols).limit(cap + 1)
+    # Loader criteria narrow the rows the DML touches; other options (load
+    # strategies) mean nothing on a PK-only SELECT and can refuse it.
+    criteria = [o for o in stmt._with_options if isinstance(o, LoaderCriteriaOption)]
+    query = select(*cols).options(*criteria).limit(cap + 1)
     if stmt.whereclause is not None:
         query = query.where(stmt.whereclause)
     options = {"include_deleted": True}
     if execute_state.execution_options.get(ALL_TENANTS_OPTION):
         options[ALL_TENANTS_OPTION] = True
     rows = execute_state.session.execute(
-        query.execution_options(**options), _where_params(stmt, execute_state.parameters)
+        query.execution_options(**options),
+        _bound_params(
+            execute_state.parameters,
+            stmt.whereclause,
+            *(c.where_criteria for c in criteria),
+        ),
     ).all()
     return [format_pk(tuple(row)) for row in rows]
 

framework/db/tests/test_db_audit_regressions.py

@@ -0,0 +1,104 @@
+"""Audit targets and ORM lifecycle regressions (#810, #811)."""
+
+import pytest
+from _audit_models import AuditTestItem, SoftDeleteItem
+from simple_module_core.correlation import correlation_id
+from simple_module_db.listeners import current_user_id
+from sqlalchemy import bindparam, delete, select, update
+from sqlalchemy.orm import with_loader_criteria
+
+
+@pytest.mark.parametrize("operation", [update, delete], ids=["update", "delete"])
+@pytest.mark.parametrize("value", [2, 99], ids=["one-match", "no-match"])
+async def test_bulk_audit_prefetch_preserves_loader_criteria(audited, operation, value):
+    _state, session, recorder = audited
+    before = dict((await session.execute(select(AuditTestItem.id, AuditTestItem.name))).all())
+    expected = set(
+        (await session.execute(select(AuditTestItem.id).where(AuditTestItem.value == value)))
+        .scalars()
+        .all()
+    )
+    stmt = operation(AuditTestItem).options(
+        with_loader_criteria(AuditTestItem, AuditTestItem.value == value)
+    )
+    if operation is update:
+        stmt = stmt.values(name="changed")
+    result = await session.execute(stmt)
+    after = dict((await session.execute(select(AuditTestItem.id, AuditTestItem.name))).all())
+    changed = {pk for pk, name in before.items() if after.get(pk) != name}
+    assert changed == expected
+    assert result.rowcount == len(expected)
+    assert {r.entity_id for r in recorder.records} == {str(pk) for pk in changed}
+    assert len(recorder.records) == len(changed)
+    assert all(
+        r.action == ("bulk_updated" if operation is update else "bulk_deleted")
+        for r in recorder.records
+    )
+
+
+@pytest.mark.parametrize("operation", [update, delete], ids=["update", "delete"])
+async def test_bulk_audit_loader_criteria_keeps_execute_parameters(audited, operation):
+    _state, session, recorder = audited
+    target = (
+        await session.execute(select(AuditTestItem.id).where(AuditTestItem.value == 2))
+    ).scalar_one()
+    stmt = (
+        operation(AuditTestItem)
+        .where(AuditTestItem.value >= 1)
+        .options(
+            with_loader_criteria(AuditTestItem, AuditTestItem.value == bindparam("wanted_value"))
+        )
+    )
+    if operation is update:
+        stmt = stmt.values(name="changed")
+    result = await session.execute(stmt, {"wanted_value": 2})
+    assert result.rowcount == 1
+    assert [r.entity_id for r in recorder.records] == [str(target)]
+
+
+@pytest.mark.parametrize("edit_title", [False, True])
+async def test_orm_restore_records_lifecycle_and_concurrent_edits(audited, edit_title):
+    _state, session, recorder = audited
+    item = SoftDeleteItem(title="archived", is_deleted=True)
+    session.add(item)
+    await session.flush()
+    recorder.records.clear()
+    item.is_deleted = False
+    if edit_title:
+        item.title = "restored title"
+    user_token = current_user_id.set("admin")
+    correlation_token = correlation_id.set("restore-request")
+    try:
+        await session.flush()
+    finally:
+        current_user_id.reset(user_token)
+        correlation_id.reset(correlation_token)
+    [record] = recorder.records
+    assert record.action == "restored"
+    assert record.entity_type == "SoftDeleteItem"
+    assert record.entity_id == str(item.id)
+    assert record.user_id == "admin"
+    assert record.correlation_id == "restore-request"
+    assert record.changes == (
+        [{"field": "title", "old": "archived", "new": "restored title"}] if edit_title else []
+    )
+    recorder.records.clear()
+    item.title = "later edit"
+    await session.flush()
+    assert [r.action for r in recorder.records] == ["updated"]
+
+
+async def test_orm_primary_key_update_records_old_and_new_key(audited):
+    _state, session, recorder = audited
+    item = (
+        await session.execute(select(AuditTestItem).where(AuditTestItem.value == 1))
+    ).scalar_one()
+    old_id = item.id
+    item.id = 999
+    await session.flush()
+    [record] = recorder.records
+    assert record.action == "updated"
+    assert record.entity_id == "999"
+    assert record.changes == [{"field": "id", "old": old_id, "new": 999}]
+    assert await session.get(AuditTestItem, old_id) is None
+    assert await session.get(AuditTestItem, 999) is item

modules/audit_log/audit_log/constants.py

@@ -28,6 +28,7 @@
 ACTION_UPDATED: Final = "updated"
 ACTION_DELETED: Final = "deleted"
 ACTION_SOFT_DELETED: Final = "soft_deleted"
+ACTION_RESTORED: Final = "restored"
 # Statement DML (#557); the names match ``simple_module_db.bulk_audit``.
 ACTION_BULK_INSERTED: Final = "bulk_inserted"
 ACTION_BULK_UPDATED: Final = "bulk_updated"
@@ -39,6 +40,7 @@
     ACTION_UPDATED,
     ACTION_DELETED,
     ACTION_SOFT_DELETED,
+    ACTION_RESTORED,
     ACTION_BULK_INSERTED,
     ACTION_BULK_UPDATED,
     ACTION_BULK_DELETED,

modules/audit_log/audit_log/locales/de.json

@@ -38,6 +38,7 @@
     "updated": "aktualisiert",
     "deleted": "gelöscht",
     "soft_deleted": "archiviert",
+    "restored": "wiederhergestellt",
     "bulk_inserted": "massenweise erstellt",
     "bulk_updated": "massenweise geändert",
     "bulk_deleted": "massenweise gelöscht",

modules/audit_log/audit_log/locales/en.json

@@ -38,6 +38,7 @@
     "updated": "updated",
     "deleted": "deleted",
     "soft_deleted": "archived",
+    "restored": "restored",
     "bulk_inserted": "bulk created",
     "bulk_updated": "bulk updated",
     "bulk_deleted": "bulk deleted",

modules/audit_log/audit_log/pages/components/ChangesList.tsx

@@ -20,6 +20,7 @@ const VISIBLE_LIMIT = 2;
 const NO_DIFF_ACTIONS = new Set([
   'deleted',
   'soft_deleted',
+  'restored',
   'bulk_deleted',
   'bulk_soft_deleted',
   'bulk_restored',

modules/audit_log/audit_log/pages/components/EntriesTable.tsx

@@ -37,11 +37,12 @@ export interface AuditEntryRead {
 
 // Borderless tints, lowercase values: the pill is a value in a dense table,
 // not a badge competing with the row's links for attention.
-const ACTION_PILL: Record<string, string> = {
+const ACTION_PILL: Record<(typeof ACTIONS)[number], string> = {
   created: 'bg-primary-600/10 text-primary-700',
   updated: 'bg-blue-50 text-blue-700',
   deleted: 'bg-red-50 text-red-700',
   soft_deleted: 'bg-amber-50 text-amber-700',
+  restored: 'bg-blue-50 text-blue-700',
   bulk_inserted: 'bg-primary-600/10 text-primary-700',
   bulk_updated: 'bg-blue-50 text-blue-700',
   bulk_deleted: 'bg-red-50 text-red-700',

modules/audit_log/audit_log/pages/components/FilterBar.tsx

@@ -20,6 +20,7 @@ export const ACTIONS = [
   'updated',
   'deleted',
   'soft_deleted',
+  'restored',
   'bulk_inserted',
   'bulk_updated',
   'bulk_deleted',