feat(kernel): thumbnails cached at ingest, with a backfill for what predates them (#21)

docs/ingest.md

@@ -182,6 +182,65 @@ A missing ffmpeg is not a file's fault at all. `MediaToolUnavailable` is recorde
 `failed`, and it is re-raised, which is precisely why it sits outside the `MediaError` family.
 One broken machine is not five thousand broken files.
 
+## A preview per asset, and it is allowed to fail
+
+Every item also gets a thumbnail, stored content-addressed beside its content and named by
+`asset.thumbnail_hash`. The M5 gallery is the reason: drawing a grid of hundreds of tiles by
+decoding full-resolution images at request time is the wrong shape, and the cost is naturally
+amortized here, where the bytes are already open and already decoded once.
+
+**A thumbnail hash is a cache key, not an identity.** It is absent from every release manifest,
+`ReleaseService.verify` never recomputes it, and two machines may legitimately hold different
+preview bytes for one image — [media.md](media.md) has the determinism argument. Losing every
+thumbnail blob loses only the time to render them again.
+
+Everything else follows from that sentence.
+
+**A preview that will not render is not an `IngestFailure`.** That error means "this file did not
+become an asset, so fix the file"; here the asset exists, its bytes are stored and nothing was
+lost. So the hash stays NULL and the run carries on with no entry in the report. Putting it there
+would tell an operator that data was lost when it was not, and would bury real loss under it. The
+NULL *is* the record, which is why nothing is logged: it is exactly what the backfill looks for.
+
+**Frames get previews too.** The frames of a clip are deliberately never re-probed — the port
+guarantees each one, and re-decoding would put our own encoder's output into an operator's
+report. That argument is about metadata a caller reads back as fact, and a preview is reported to
+nobody, so it does not carry over. A gallery with tiles for stills and blanks for frames would be
+the worse outcome.
+
+**One edge, one cache.** `DEFAULT_THUMBNAIL_MAX_EDGE` is pinned at the port and is not a
+parameter on ingest or on the backfill. A per-call edge would fork the cache into variants
+nothing can tell apart from a hash, and the column holds one pointer.
+
+**A deduplicated asset has its NULL filled, and a value is never replaced.** Origin fields record
+the first sighting and are never rewritten; a preview is not provenance, so filling an empty one
+from whoever first held the bytes is not a rewrite. That is what makes re-ingesting a source
+enough to give assets written before the cache existed their previews.
+
+### The backfill
+
+`IngestService.backfill_thumbnails(project_id)` renders a preview for every asset in a project
+that has none — the remedy for all three things a NULL can mean, and idempotent, so a second pass
+over a healthy project examines nothing.
+
+It reads the **blob**, never `asset.uri`: that path may be gone, renamed, or on another machine,
+while `blob_store.get(asset.content_hash)` is what the workspace actually holds.
+
+Three phases, and the rendering is in none of them — the same rule as a run. The first
+transaction collects ids and hashes, the rendering happens outside any transaction, and the last
+re-reads each asset before writing so an ingest that filled a preview meanwhile is not clobbered.
+
+It reports rather than raises, on `ReleaseService.verify`'s terms: someone repairing a damaged
+workspace needs the list, and one asset nobody can render must not abandon the other five
+thousand. `ThumbnailBackfill` keeps `missing` (the content blob is gone — damage a preview pass
+cannot repair) apart from `unreadable` (the bytes are there and will not decode). The second
+reuses `IngestFailure` because the `UNSUPPORTED`/`CORRUPT` split says exactly the right thing
+about stored bytes; the first does not, because `IngestFailureKind` answers "what is wrong with
+this file" and a blob that is not there is not a file.
+
+There is no progress to poll: a backfill has no `IngestJob` row. If that is ever wanted it is a
+task of its own, not a flag on this one.
+
 ## The target batch
 
 With no `batch_id`, the run creates a draft named `batch_name` or, failing that, after the
@@ -194,8 +253,6 @@ order for a directory and frame order for a clip.
 
 ## What is deliberately not here yet
 
-- **No thumbnails.** Generating one per asset at ingest and recording `asset.thumbnail_hash` is
-  #21, for the M5 gallery.
 - **No background execution.** A run is synchronous and in-process. The service API is shaped so
   that moving it behind a queue changes the caller's waiting, not its vocabulary — which is why
   a job is created `pending` and why progress is read off the row rather than off a callback.

docs/media.md

@@ -208,6 +208,10 @@ a verification pass recomputing one — source-content hashes are reproducible,
 hashes are not, and conflating the two is how a "verified" release starts failing on a different
 machine.
 
+`asset.thumbnail_hash` is where that cache key is recorded, and it holds itself to every word of
+this paragraph: it is absent from `Manifest`, `ReleaseService.verify` does not touch it, and a
+NULL there is an ordinary state rather than a fault. See [ingest.md](ingest.md).
+
 ## Refusals
 
 Two errors under one `MediaError` base, so an ingest can catch the family once, record the
@@ -331,14 +335,19 @@ Neither library needs an import-linter change. The contracts forbid *frameworks*
 kernel — FastAPI, Typer, MCP, uvicorn — not third-party libraries, and ffmpeg is reached through
 `subprocess` and is not an import at all.
 
-## What is deliberately not here yet
+## Everything on this page now has a caller
+
+Three things used to be listed here as not built yet, and none of them is. `Source` came off the
+list with registration, which records a clip's original rate and the decomposition parameters
+chosen for it, built on `VideoMetadata` exactly as anticipated — see [sources.md](sources.md).
+The `Asset` fields came off it with [ingest](ingest.md): what a probe reported is now stored as
+`asset.format`, and a frame's `index`/`timestamp` land on the asset as
+`frame_index`/`frame_timestamp` beside the source it was cut from.
 
-- **No thumbnail write.** `thumbnail()` hands back bytes. Storing them content-addressed and
-  recording an `asset.thumbnail_hash` is the thumbnail-cache task, for the M5 gallery.
+The thumbnail write came off it last. `thumbnail()` still only hands back bytes — storing them is
+not the port's job — but those bytes now go into the blob store during the ingest loop, on both
+paths, and the hash lands on `asset.thumbnail_hash`. The cache-key-not-identity rule above is
+what that column is built on, and `IngestService.backfill_thumbnails` is how an asset that
+predates it, or one whose preview would not render, gets caught up. See [ingest.md](ingest.md).
 
-Two things used to be on that list and no longer are. `Source` came off it with registration,
-which records a clip's original rate and the decomposition parameters chosen for it, built on
-`VideoMetadata` exactly as anticipated — see [sources.md](sources.md). The `Asset` fields came off
-it with [ingest](ingest.md): what a probe reported is now stored as `asset.format`, and a frame's
-`index`/`timestamp` land on the asset as `frame_index`/`frame_timestamp` beside the source it was
-cut from. Both ports are called from exactly one place, and that is where.
+Both ports are called from exactly one place, and that is where.

docs/persistence.md

@@ -82,7 +82,7 @@ the operation and not the individual write.
 | --- | --- | --- |
 | Relations that get mutated element by element | Child table — `batch_asset`, `annotation_job_asset` | Membership and per-asset progress are edited one row at a time and queried from the asset side. `batch_asset.position` preserves order. |
 | Immutable nested values | JSON column — `annotation_schema.classes`, `annotation.geometry`, `annotation.attributes`, `release.split` | A schema version must rehydrate byte-identical, and nothing queries a single `LabelClass` in SQL. Child tables would only add ordering columns. |
-| An immutable value too large for a row | The **blob store**, with the row keeping its hash — `release.manifest_hash` | A release manifest lists every asset and every label; megabytes of it in a column would have to be read just to list a dataset's releases. Content-addressed, so it is verifiable and two identical releases share one document. See [releases.md](releases.md). |
+| An immutable value too large for a row | The **blob store**, with the row keeping its hash — `release.manifest_hash`, `asset.thumbnail_hash` | A release manifest lists every asset and every label; megabytes of it in a column would have to be read just to list a dataset's releases. Content-addressed, so it is verifiable and two identical releases share one document. See [releases.md](releases.md). A thumbnail is the same storage decision for a different reason: it is a *cache*, so the row keeps a pointer that may be NULL and losing the bytes costs only the time to render them again. |
 | Timestamps | TEXT holding ISO-8601 **with offset** | SQLite's `DATETIME` storage drops the timezone. Domain timestamps are timezone-aware UTC and a naive value is rejected at construction. |
 
 Foreign keys are declared `ON DELETE CASCADE` — and the store issues
@@ -116,8 +116,9 @@ MIGRATIONS: list[Migration] = [
     Migration(version=7, name="source_provenance", upgrade=...),
     Migration(version=8, name="ingest_pipeline", upgrade=...),
     Migration(version=9, name="ingest_job_progress", upgrade=...),
+    Migration(version=10, name="asset_thumbnail", upgrade=...),
 ]
-FORMAT_VERSION: int = MIGRATIONS[-1].version  # 9
+FORMAT_VERSION: int = MIGRATIONS[-1].version  # 10
 ```
 
 `initialize()` reads the version stamped in `_visionset_meta` and runs whatever is
@@ -221,12 +222,22 @@ which is exactly what `0` and `[]` say; NULL is what a run that named no batch m
 child table on the criteria above: a per-file report is an immutable value read whole, and
 nothing queries a single failed file in SQL.
 
+Migration 010 is plainer still: one nullable column, `asset.thumbnail_hash`, pointing at a cached
+preview in the blob store. No foreign key, so 008's "a column carrying a key cannot arrive by
+`ALTER` at all" limit does not bite, and no data pre-check to make — the column is a *cache*, so
+NULL is not a legacy value something has to tolerate but the ordinary state of an asset nobody
+has rendered a preview for yet. `IngestService.backfill_thumbnails` reads exactly that state.
+
 The fresh-versus-migrated test is only as strong as how far back
 `_downgrade_to_version_one` walks, so every migration added there needs its undo added too.
 Migrations 006 and 007 are the two places that undo cannot borrow its DDL from `_tables`,
 because `_tables` no longer describes the shape it is restoring. Migration 009 is the one
 place that needs no undo of its own: its columns live on `ingest_job`, which 008's undo rebuilds
 from scratch, so restoring the generation-1 shape removes them along with everything else.
+Migration 010 gets no such ride and has its own `DROP COLUMN` line — `asset` is only ever
+altered, for the reasons 008 gives, so nothing later rebuilds it. The compensation is that 010's
+real `ALTER` runs on the way back up from generation 1, which is why it needs no generation twin
+of `test_migration_nine_alters_a_table_migration_eight_rebuilt`.
 
 `format_version` here is the *database* generation. Validating the on-disk workspace layout
 around it — directories, the blob-store root, what makes a directory a workspace at all —

src/visionset/kernel/adapters/_tables.py

@@ -224,11 +224,11 @@ class AssetRow(Base):
     uri: Mapped[str] = mapped_column(String, nullable=False)
     width: Mapped[int | None] = mapped_column(Integer, nullable=True)
     height: Mapped[int | None] = mapped_column(Integer, nullable=True)
-    # The four below arrive by ``ALTER TABLE`` in migration 8 and are therefore
-    # declared last, in the order that migration adds them. #21's
-    # ``thumbnail_hash`` goes after them, for the same reason.
+    # The five below arrive by ``ALTER TABLE`` and are therefore declared last,
+    # in the order the migrations add them: four in migration 8, then
+    # ``thumbnail_hash`` in migration 10.
     #
-    # All four are nullable, and that is not a shortcut. ``asset`` could not be
+    # All are nullable, and that is not a shortcut. ``asset`` could not be
     # rebuilt the way migrations 6 and 7 rebuilt their tables: four tables carry
     # ``ON DELETE CASCADE`` keys into it (``batch_asset``, ``annotation``,
     # ``dataset_member``, ``annotation_job_asset``), and under
@@ -258,6 +258,18 @@ class AssetRow(Base):
     frame_index: Mapped[int | None] = mapped_column(Integer, nullable=True)
     #: Seconds into the clip. The locator that survives a different rate.
     frame_timestamp: Mapped[float | None] = mapped_column(Float, nullable=True)
+    #: A cached preview in the blob store, added by migration 10.
+    #:
+    #: Nullable for a reason the four above do not share: this one is a *cache*,
+    #: so NULL is the ordinary state rather than a legacy one. It means "no
+    #: preview yet" whether the row predates migration 10, holds bytes that will
+    #: not render, or simply has not been reached — and
+    #: ``IngestService.backfill_thumbnails`` reads it to find all three.
+    #:
+    #: Unindexed, and deliberately: the one query over it walks a project's
+    #: assets and filters in Python, which is the shape ``Repository.list``
+    #: already has. No foreign key either — it names a blob, not a row.
+    thumbnail_hash: Mapped[str | None] = mapped_column(String, nullable=True)
 
 
 #: The same bytes are the same asset: the backstop under the ingest pipeline's

src/visionset/kernel/adapters/migrations.py

@@ -377,6 +377,40 @@ def _add_ingest_progress_and_report(connection: Connection) -> None:
         _add_column(connection, cast(Column[object], column))
 
 
+def _add_asset_thumbnail(connection: Connection) -> None:
+    """Give an asset somewhere to point at its cached preview.
+
+    The plainest migration in the file, and every way it is plain is an
+    argument rather than an omission. One column, so there is no ordering
+    question between siblings. No foreign key — it names a blob, not a row — so
+    migration 8's limit, that a column carrying a key cannot arrive by ``ALTER``
+    at all, does not bite. Nothing to refuse and nothing to rebuild: ``asset``
+    has four ``ON DELETE CASCADE`` children, and under ``PRAGMA foreign_keys =
+    ON`` dropping it would take them silently.
+
+    No data pre-check, and here that is easier to claim than it was for
+    migration 9: the column is a *cache*, so NULL is not a legacy value that
+    something has to tolerate but the ordinary state of an asset nobody has
+    rendered a preview for yet. ``IngestService.backfill_thumbnails`` reads
+    exactly that state and is the remedy for it.
+
+    Idempotent the way 3 to 5 and 9 are — the inspector check inside
+    ``_add_column`` *is* the ``checkfirst``, because migration 1 is
+    ``create_all`` of current metadata.
+
+    Unlike migration 9, this one **needs its own undo** in the tests'
+    ``_downgrade_to_version_one``. Migration 9's columns rode back on migration
+    8's rebuild of ``ingest_job``; ``asset`` is only ever altered, so nothing
+    later removes this column on the way to generation 1. The flip side is that
+    the walk back to generation 1 does exercise this ``ALTER`` for real, which
+    is why there is no generation-9 schema twin of
+    ``test_migration_nine_alters_a_table_migration_eight_rebuilt``.
+    """
+    # ``.c`` is typed as the generic column collection; the entry is a real
+    # ``Column``, which is what ``CreateColumn`` needs.
+    _add_column(connection, cast(Column[object], AssetRow.__table__.c.thumbnail_hash))
+
+
 MIGRATIONS: list[Migration] = [
     Migration(version=1, name="initial_schema", upgrade=_create_initial_schema),
     Migration(
@@ -419,6 +453,11 @@ def _add_ingest_progress_and_report(connection: Connection) -> None:
         name="ingest_job_progress",
         upgrade=_add_ingest_progress_and_report,
     ),
+    Migration(
+        version=10,
+        name="asset_thumbnail",
+        upgrade=_add_asset_thumbnail,
+    ),
 ]
 
 FORMAT_VERSION: int = MIGRATIONS[-1].version

src/visionset/kernel/domain/__init__.py

@@ -39,6 +39,7 @@
     IngestJob,
     IngestResult,
     IngestState,
+    ThumbnailBackfill,
 )
 from visionset.kernel.domain.media import (
     ImageFormat,
@@ -163,6 +164,7 @@
     "SplitAssignment",
     "SplitRecipe",
     "TaskGroup",
+    "ThumbnailBackfill",
     "VideoFrame",
     "VideoMetadata",
     "VideoProvenance",

src/visionset/kernel/domain/asset.py

@@ -5,7 +5,7 @@
 from typing import Literal
 from uuid import UUID, uuid4
 
-from pydantic import BaseModel, Field, field_validator, model_validator
+from pydantic import BaseModel, Field, ValidationInfo, field_validator, model_validator
 
 from visionset.kernel.domain.media import ImageFormat
 
@@ -33,6 +33,9 @@ class Asset(BaseModel):
     clip. ``format`` and ``source_id`` are absent on a row written before the
     ingest pipeline existed, and no default could be honest: the store cannot
     invent a format nobody probed.
+
+    ``thumbnail_hash`` is optional for a different reason again, and it is the
+    one field here that is **not** provenance — see its own note below.
     """
 
     id: UUID = Field(default_factory=uuid4)
@@ -50,12 +53,34 @@ class Asset(BaseModel):
     frame_index: int | None = Field(default=None, ge=0)
     #: Seconds into the clip — the locator that survives a re-decomposition.
     frame_timestamp: float | None = Field(default=None, ge=0)
+    #: A cached preview in the blob store, or NULL when none has been rendered.
+    #:
+    #: **A cache key, not an identity**, which is the whole reason this can sit
+    #: beside the provenance fields without being one. It never enters a release
+    #: manifest and ``ReleaseService.verify`` never recomputes it: two machines
+    #: may hold different thumbnail bytes for one image, because determinism is
+    #: promised within a Pillow build rather than across them. Losing every
+    #: thumbnail blob loses only the CPU time to render them again.
+    #:
+    #: NULL therefore has one meaning with three causes — an asset written
+    #: before the cache existed, one whose bytes would not render, or one a run
+    #: has not reached yet. ``IngestService.backfill_thumbnails`` is the remedy
+    #: for all three, and reads exactly this state. Declared last, after the
+    #: columns migration 8 added, because it arrives by ``ALTER TABLE`` too.
+    thumbnail_hash: str | None = None
 
-    @field_validator("content_hash")
+    @field_validator("content_hash", "thumbnail_hash")
     @classmethod
-    def _content_hash_is_sha256_hex(cls, value: str) -> str:
-        if not _SHA256_HEX.fullmatch(value):
-            raise ValueError("content_hash must be 64 lowercase hex chars (SHA-256)")
+    def _is_sha256_hex(cls, value: str | None, info: ValidationInfo) -> str | None:
+        """Both fields name a blob, so both are checked by the one rule.
+
+        A second regex for the second field is how the two drift apart. ``None``
+        passes through because ``thumbnail_hash`` is optional and
+        ``content_hash`` is not — pydantic has already refused a missing one by
+        the time a validator runs.
+        """
+        if value is not None and not _SHA256_HEX.fullmatch(value):
+            raise ValueError(f"{info.field_name} must be 64 lowercase hex chars (SHA-256)")
         return value
 
     @model_validator(mode="after")