fix(review): validate stored review evidence once per startup

Startup used to run the full review validation twice (the early guard and
the install) and the annotation group's row checks inside each, so every
evidence document was parsed and re-encoded twice. The early guard is now
catalog-only (a review group without annotation tables), and the install
validates the group once, trusting the annotation group it has just
validated. The evidence predicate no longer pattern-scans every document in
SQL, because the streamed canonical pass proves it, and the codec is built
once.

Measured on a database with 40,000 items of 4 KiB evidence: startup open
went from 7.7 s to 2.5 s. A test pins one evidence pass per reopen.
Restore and verify_database_file keep the full validation.

database/models.py

@@ -18,7 +18,11 @@ from typing import Optional, Set, Tuple
 from .library_schema import library_schema_error, migrate_library_state
 from .image_favorites import image_favorites_schema_error, migrate_image_favorites
 from .annotation_schema import annotation_schema_error, migrate_annotation_state
-from .review_schema import migrate_review_state, review_schema_error
+from .review_schema import (
+    migrate_review_state,
+    review_schema_error,
+    review_without_annotations_error,
+)
 from .saved_views import migrate_saved_views, saved_views_schema_error
 from .execution_history_schema import (
     history_schema_error,
@@ -305,8 +309,9 @@ class PromptModel:
         migrate_image_favorites(conn)
         migrate_saved_views(conn)
         # A review group without its annotation group is refused before the
-        # annotation install could make it look valid.
-        review_error = review_schema_error(conn)
+        # annotation install could make it look valid (catalog only; the rows
+        # are validated once, in migrate_review_state).
+        review_error = review_without_annotations_error(conn)
         if review_error:
             raise sqlite3.DatabaseError(review_error)
         migrate_annotation_state(conn)

database/review_schema.py

@@ -33,7 +33,10 @@ import re
 import sqlite3
 from typing import Any, Optional
 
-from .annotation_schema import BYTES_TABLE, annotation_schema_error
+from .annotation_schema import BYTES_TABLE
+from .annotation_schema import TABLES as ANNOTATION_TABLES
+from .annotation_schema import _catalog as _annotation_catalog
+from .annotation_schema import annotation_schema_error
 
 SCHEMA_VERSION = 1
 METADATA_TABLE = "media_review_metadata"
@@ -159,6 +162,13 @@ def _reject_constant(name: str) -> Any:
     raise _NonCanonical(f"JSON constant {name} is not allowed")
 
 
+# Built once: validation re-encodes every stored document at startup.
+_ENCODER = json.JSONEncoder(
+    sort_keys=True, separators=(",", ":"), ensure_ascii=True, allow_nan=False
+)
+_DECODER = json.JSONDecoder(parse_constant=_reject_constant)
+
+
 def canonical_evidence(value: Any) -> str:
     """Serialize one evidence object canonically; ``ValueError`` when invalid.
 
@@ -168,13 +178,7 @@ def canonical_evidence(value: Any) -> str:
     if not isinstance(value, dict):
         raise ValueError("Review evidence must be a JSON object")
     try:
-        text = json.dumps(
-            value,
-            sort_keys=True,
-            separators=(",", ":"),
-            ensure_ascii=True,
-            allow_nan=False,
-        )
+        text = _ENCODER.encode(value)
     except (TypeError, RecursionError) as error:
         raise ValueError(f"Review evidence is not serializable: {error}") from None
     if len(text) > MAX_EVIDENCE_BYTES:
@@ -191,7 +195,7 @@ def _is_canonical(text: Any) -> bool:
     if not isinstance(text, str):
         return False
     try:
-        value = json.loads(text, parse_constant=_reject_constant)
+        value = _DECODER.decode(text)
         return canonical_evidence(value) == text
     except (ValueError, RecursionError):
         return False
@@ -228,12 +232,12 @@ def _hex64(column: str) -> str:
 
 
 def _evidence(column: str) -> str:
-    # Printable ASCII only is a cheap necessary condition for canonical text;
-    # the streamed pass below proves the rest.
+    # Type and size only: the streamed pass below proves the text canonical
+    # (which implies printable ASCII), so a pattern scan here would read every
+    # document a second time.
     return (
         f"typeof({column}) = 'text' "
-        f"AND length(CAST({column} AS BLOB)) BETWEEN 2 AND {MAX_EVIDENCE_BYTES} "
-        f"AND CAST({column} AS BLOB) NOT GLOB '*[^ -~]*'"
+        f"AND length(CAST({column} AS BLOB)) BETWEEN 2 AND {MAX_EVIDENCE_BYTES}"
     )
 
 
@@ -356,25 +360,55 @@ def _value_error(conn: sqlite3.Connection) -> Optional[str]:
     return _evidence_error(conn)
 
 
+def review_without_annotations_error(conn: sqlite3.Connection) -> Optional[str]:
+    """Catalog only: a review group present without its annotation tables.
+
+    Startup asks this before the annotation group installs, which would make
+    a review group copied without it look valid. It reads no rows; the full
+    validation runs once in :func:`migrate_review_state`.
+    """
+    if not _catalog(conn):
+        return None
+    present = {
+        name for kind, name, _sql in _annotation_catalog(conn) if kind == "table"
+    }
+    if present != set(ANNOTATION_TABLES):
+        return (
+            "Review queue schema requires a valid image annotation schema: "
+            "Missing image annotation schema"
+        )
+    return None
+
+
 def review_schema_error(
-    conn: sqlite3.Connection, *, allow_missing: bool = True
+    conn: sqlite3.Connection,
+    *,
+    allow_missing: bool = True,
+    annotations_valid: bool = False,
 ) -> Optional[str]:
     """Reject partial or unsupported review schemas and invalid rows.
 
     Reads only: nothing is ever repaired. A database without any of the
     group's objects is acceptable for backups that predate it unless
     ``allow_missing`` is false. A present group requires a present, valid
-    image annotation group.
+    image annotation group; ``annotations_valid`` says the caller has just
+    validated it (startup's annotation install), so only its presence is
+    checked again.
     """
     objects = _catalog(conn)
     if not objects:
         return None if allow_missing else "Missing review queue schema"
-    annotation_error = annotation_schema_error(conn, allow_missing=False)
+    if annotations_valid:
+        annotation_error = review_without_annotations_error(conn)
+    else:
+        annotation_error = annotation_schema_error(conn, allow_missing=False)
+        if annotation_error:
+            annotation_error = (
+                "Review queue schema requires a valid image annotation schema: "
+                + annotation_error
+            )
     if annotation_error:
-        return (
-            "Review queue schema requires a valid image annotation schema: "
-            + annotation_error
-        )
+        return annotation_error
     present = {name for kind, name, _sql in objects if kind == "table"}
     if present != set(TABLES):
         return "Incomplete review queue schema"
@@ -387,13 +421,14 @@ def review_schema_error(
 def migrate_review_state(conn: sqlite3.Connection) -> None:
     """Install the group atomically; validate before and after creating it.
 
-    Runs after ``migrate_annotation_state``: the group is only valid beside
-    the annotation group it cites.
+    Runs after ``migrate_annotation_state`` (which has just validated the
+    annotation group): the group is only valid beside the annotation group it
+    cites. A present group is validated once, evidence included.
     """
     conn.execute("SAVEPOINT media_review_schema")
     try:
         installed = bool(_catalog(conn))
-        error = review_schema_error(conn)
+        error = review_schema_error(conn, annotations_valid=True)
         if error:
             raise sqlite3.DatabaseError(error)
         if installed:
@@ -410,7 +445,7 @@ def migrate_review_state(conn: sqlite3.Connection) -> None:
             "(singleton_id, schema_version, review_index_revision) VALUES (1, ?, 0)",
             (SCHEMA_VERSION,),
         )
-        error = review_schema_error(conn, allow_missing=False)
+        error = review_schema_error(conn, allow_missing=False, annotations_valid=True)
         if error:
             raise sqlite3.DatabaseError(error)
         conn.execute("RELEASE media_review_schema")

tests/test_review_schema.py

@@ -330,6 +330,43 @@ class ReviewSchemaTests(unittest.TestCase):
                 conn.execute(f"DELETE FROM {ITEMS}")
             self.assertEqual(table_rows(conn, TABLES), snapshot)
 
+    def test_startup_validates_stored_evidence_once(self):
+        """A reopen streams the evidence once and the annotation group twice
+        (its own install), never again for the review group (I3)."""
+        self.require_module()
+        import database.annotation_schema as annotation_module
+        import database.review_schema as review_module
+
+        model = self.model()
+        with model.get_connection() as conn:
+            insert_group_rows(conn)
+        model.close_all()
+        for _ in range(2):
+            PromptModel.reset_schema_cache()
+            with (
+                patch.object(
+                    review_module,
+                    "_evidence_error",
+                    wraps=review_module._evidence_error,
+                ) as evidence,
+                patch.object(
+                    review_module,
+                    "annotation_schema_error",
+                    wraps=review_module.annotation_schema_error,
+                ) as nested,
+                patch.object(
+                    annotation_module,
+                    "_value_error",
+                    wraps=annotation_module._value_error,
+                ) as annotation_rows,
+            ):
+                model = self.model()
+                model.get_connection()
+            self.assertEqual(evidence.call_count, 1)
+            self.assertEqual(nested.call_count, 0)
+            self.assertEqual(annotation_rows.call_count, 2)
+            model.close_all()
+
     def test_old_database_without_group_migrates_without_touching_legacy_rows(self):
         self.require_module()
         for label, dropped in (