From 8346c71524ddac482a87f8928108979debf2a0be Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Fri, 28 Aug 2026 02:30:34 -0700 Subject: [PATCH 01/16] feat(kernel): scale percent and per-file image scales join the source domain --- src/visionset/kernel/domain/__init__.py | 4 ++ src/visionset/kernel/domain/source.py | 59 +++++++++++++++++- tests/kernel/test_source_scale.py | 83 +++++++++++++++++++++++++ 3 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 tests/kernel/test_source_scale.py diff --git a/src/visionset/kernel/domain/__init__.py b/src/visionset/kernel/domain/__init__.py index 8645b278..9a32d6fa 100644 --- a/src/visionset/kernel/domain/__init__.py +++ b/src/visionset/kernel/domain/__init__.py @@ -253,10 +253,12 @@ SourceKind, TimeRange, VideoProvenance, + canonical_image_scales, canonical_path, canonical_ranges, expected_frames, grid_bounds, + scaled_dimension, ) from visionset.kernel.domain.suggestion import ( DEFAULT_TOLERANCE, @@ -535,6 +537,7 @@ "Workspace", "assign_split", "canonical_bytes", + "canonical_image_scales", "canonical_path", "canonical_ranges", "expected_frames", @@ -551,5 +554,6 @@ "require_move", "require_points_on_asset", "require_state", + "scaled_dimension", "sha256_hex", ] diff --git a/src/visionset/kernel/domain/source.py b/src/visionset/kernel/domain/source.py index 15838e75..290d12ad 100644 --- a/src/visionset/kernel/domain/source.py +++ b/src/visionset/kernel/domain/source.py @@ -13,8 +13,8 @@ "the same source" *is* — put them on the job and two runs of one source could legitimately disagree, leaving idempotency with nothing to be measured against. The consequence is deliberate: one clip registered at 1 fps and again at 5 fps — -or over different clip ranges — is two sources over one file, not one source -with a history. +or over different clip ranges, or at another scale — is two sources over one +file, not one source with a history. **Paths are canonicalized once**, by :func:`canonical_path`, so ``./data`` and ``/abs/data`` are one source rather than two. See that function for what @@ -24,7 +24,7 @@ from __future__ import annotations import math -from collections.abc import Iterable +from collections.abc import Iterable, Mapping from datetime import UTC, datetime from enum import StrEnum from pathlib import Path, PurePath @@ -157,6 +157,26 @@ def expected_frames(ranges: Iterable[TimeRange], *, duration_seconds: float, fps return sum(b - a for a, b in bounds) +def scaled_dimension(native: int, percent: int) -> int: + """One axis after a percent downscale — integer half-up, floored at one. + + Integer arithmetic on purpose: Python ``round`` is half-even and JS + ``Math.round`` is half-up, and the ingest screen mirrors this formula, so + the one spelling both sides can share is ``(native * percent + 50) // 100``. + """ + return max(1, (native * percent + 50) // 100) + + +def canonical_image_scales(scales: Mapping[str, int]) -> dict[str, int]: + """The one spelling of a per-file scale selection, so identity can compare it. + + Entries at 100 are dropped — storing at native size is what a file not + named already gets — and keys are sorted so the persisted JSON text is + deterministic for the origin index to compare. + """ + return {name: percent for name, percent in sorted(scales.items()) if percent != 100} + + class VideoProvenance(BaseModel): """What a clip was, and how we chose to cut it. @@ -173,6 +193,11 @@ class VideoProvenance(BaseModel): Always stored canonical — see :func:`canonical_ranges` — and the validator refuses anything else rather than quietly rewriting a frozen value. + :attr:`scale_percent` is the third cut parameter: the percent of the native + size frames are stored at, 100 meaning unscaled. Extraction emits every + frame at :attr:`stored_width` × :attr:`stored_height`; :attr:`metadata` + keeps the probe's native numbers, because what the clip *was* is provenance. + Frozen, like every other value in the domain that is a pure function of some bytes and a choice. """ @@ -182,6 +207,15 @@ class VideoProvenance(BaseModel): metadata: VideoMetadata extraction_fps: float = Field(gt=0) ranges: tuple[TimeRange, ...] = () + scale_percent: int = Field(default=100, ge=1, le=100) + + @property + def stored_width(self) -> int: + return scaled_dimension(self.metadata.width, self.scale_percent) + + @property + def stored_height(self) -> int: + return scaled_dimension(self.metadata.height, self.scale_percent) @model_validator(mode="after") def _ranges_are_canonical(self) -> VideoProvenance: @@ -221,6 +255,11 @@ class Source(BaseModel): must not fork one origin into two — and unlike ``registered_at`` a provided value *does* refresh the stored one, because a label is curation, not provenance. + + :attr:`image_scales` is an image directory's per-file downscale — filename + to percent, always canonical (see :func:`canonical_image_scales`), empty + meaning every file stores at its decoded size. Unlike the two fields above + it **is** part of the source's identity, exactly as a clip's cut is. """ model_config = ConfigDict(validate_assignment=True) @@ -233,6 +272,7 @@ class Source(BaseModel): registered_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) capture_params: dict[str, str] = Field(default_factory=dict) video: VideoProvenance | None = None + image_scales: dict[str, int] = Field(default_factory=dict) @property def name(self) -> str: @@ -259,6 +299,19 @@ def _video_provenance_matches_the_kind(self) -> Source: raise ValueError(f"a {self.kind.value} source must {carry} video provenance") return self + @model_validator(mode="after") + def _image_scales_are_canonical_and_match_the_kind(self) -> Source: + if self.image_scales and self.kind is not SourceKind.IMAGE_DIRECTORY: + raise ValueError(f"a {self.kind.value} source must not carry image scales") + for name, percent in self.image_scales.items(): + if not 1 <= percent <= 99: + raise ValueError(f"scale for {name!r} must be in [1, 99], got {percent}") + if self.image_scales != canonical_image_scales(self.image_scales): + raise ValueError( + "image_scales must be canonical; pass them through canonical_image_scales" + ) + return self + def require_video(self) -> VideoProvenance: """The clip's provenance, or refuse because this is not a clip. diff --git a/tests/kernel/test_source_scale.py b/tests/kernel/test_source_scale.py new file mode 100644 index 00000000..047b5af9 --- /dev/null +++ b/tests/kernel/test_source_scale.py @@ -0,0 +1,83 @@ +"""Scale arithmetic and the canonical spellings it adds to the source domain.""" + +from uuid import uuid4 + +import pytest +from pydantic import ValidationError + +from visionset.kernel.domain import ( + Source, + SourceKind, + VideoMetadata, + VideoProvenance, + canonical_image_scales, + scaled_dimension, +) + + +def _metadata(width: int = 1920, height: int = 1080) -> VideoMetadata: + return VideoMetadata(width=width, height=height, fps=30.0, duration_seconds=1.0, codec="h264") + + +def _provenance(*, width: int, height: int, scale_percent: int) -> VideoProvenance: + return VideoProvenance( + metadata=_metadata(width, height), extraction_fps=1.0, scale_percent=scale_percent + ) + + +def _image_source(*, image_scales: dict[str, int]) -> Source: + return Source( + project_id=uuid4(), + kind=SourceKind.IMAGE_DIRECTORY, + path="/data", + image_scales=image_scales, + ) + + +def _video_source(*, image_scales: dict[str, int]) -> Source: + return Source( + project_id=uuid4(), + kind=SourceKind.VIDEO, + path="/clip.mp4", + video=_provenance(width=4, height=4, scale_percent=100), + image_scales=image_scales, + ) + + +def test_scaled_dimension_rounds_half_up_in_integer_arithmetic() -> None: + assert scaled_dimension(25, 50) == 13 + assert scaled_dimension(1920, 50) == 960 + assert scaled_dimension(1, 10) == 1 + assert scaled_dimension(640, 100) == 640 + + +def test_canonical_image_scales_drops_hundreds_and_sorts_keys() -> None: + assert canonical_image_scales({"b.png": 100, "a.png": 50}) == {"a.png": 50} + assert list(canonical_image_scales({"z.png": 40, "a.png": 60})) == ["a.png", "z.png"] + assert canonical_image_scales({}) == {} + + +def test_image_scales_refuses_non_canonical_and_out_of_range() -> None: + with pytest.raises(ValidationError): + _image_source(image_scales={"a.png": 100}) + with pytest.raises(ValidationError): + _image_source(image_scales={"a.png": 0}) + with pytest.raises(ValidationError): + _image_source(image_scales={"a.png": 101}) + + +def test_a_video_source_carries_no_image_scales() -> None: + with pytest.raises(ValidationError): + _video_source(image_scales={"a.png": 50}) + + +def test_scale_percent_is_bounded() -> None: + with pytest.raises(ValidationError): + _provenance(width=4, height=4, scale_percent=0) + with pytest.raises(ValidationError): + _provenance(width=4, height=4, scale_percent=101) + + +def test_stored_size_is_the_scaled_probe() -> None: + provenance = _provenance(width=1920, height=1080, scale_percent=50) + assert (provenance.stored_width, provenance.stored_height) == (960, 540) From 7665934908dfe1e6ddd62e973d0fe219f2aa8a1f Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Fri, 28 Aug 2026 02:34:49 -0700 Subject: [PATCH 02/16] feat(kernel): migration 18 - scale terms join the source origin index --- src/visionset/kernel/adapters/_mappers.py | 15 +++-- src/visionset/kernel/adapters/_tables.py | 15 ++++- src/visionset/kernel/adapters/migrations.py | 27 ++++++-- tests/kernel/test_migrations.py | 70 ++++++++++++++++++++- tests/kernel/test_schema_draft_service.py | 4 +- 5 files changed, 116 insertions(+), 15 deletions(-) diff --git a/src/visionset/kernel/adapters/_mappers.py b/src/visionset/kernel/adapters/_mappers.py index 33f4d801..f0c0eb61 100644 --- a/src/visionset/kernel/adapters/_mappers.py +++ b/src/visionset/kernel/adapters/_mappers.py @@ -73,6 +73,7 @@ Token, VideoProvenance, Workspace, + canonical_image_scales, ) _geometry_adapter: TypeAdapter[Geometry] = TypeAdapter(Geometry) @@ -346,18 +347,20 @@ def _change_to_domain(_: Session, row: Any) -> DatasetChange: def _video_to_json(video: VideoProvenance | None) -> dict[str, Any] | None: - """``VideoProvenance`` as stored, with an empty ``ranges`` key omitted. + """``VideoProvenance`` as stored, with the default cut keys omitted. - Omitted, not stored as ``[]``: the source-origin index compares - ``json_extract(video, '$.ranges')``, and rows written before ranges existed - have no key — a whole-clip selection must serialize the same way, or the - index would hold two spellings of one origin. + Omitted, not stored as ``[]`` or ``100``: the source-origin index compares + ``json_extract`` of these keys, and rows written before each feature + existed have no key — a whole-clip, unscaled selection must serialize the + same way, or the index would hold two spellings of one origin. """ if video is None: return None dump = video.model_dump(mode="json") if not dump["ranges"]: del dump["ranges"] + if dump["scale_percent"] == 100: + del dump["scale_percent"] return dump @@ -375,6 +378,7 @@ def _source_to_row(entity: Source) -> t.Base: registered_at=entity.registered_at.isoformat(), capture_params=dict(entity.capture_params), video=_video_to_json(entity.video), + image_scales=canonical_image_scales(entity.image_scales) or None, ) @@ -388,6 +392,7 @@ def _source_to_domain(_: Session, row: Any) -> Source: registered_at=datetime.fromisoformat(row.registered_at), capture_params=row.capture_params, video=None if row.video is None else VideoProvenance.model_validate(row.video), + image_scales=row.image_scales or {}, ) diff --git a/src/visionset/kernel/adapters/_tables.py b/src/visionset/kernel/adapters/_tables.py index e231594b..0ae24a1a 100644 --- a/src/visionset/kernel/adapters/_tables.py +++ b/src/visionset/kernel/adapters/_tables.py @@ -187,8 +187,12 @@ class SourceRow(Base): #: A ``VideoProvenance``, or NULL for anything that is not a clip. video: Mapped[dict[str, Any] | None] = mapped_column(JSON, nullable=True) #: What a caller asked this source to be called; NULL means nobody said. - #: The newest column here, so it is declared last — see the class docstring. display_name: Mapped[str | None] = mapped_column(String, nullable=True) + #: Per-file downscale for an image directory: filename -> percent, always + #: canonical (no 100s, sorted keys) so the origin index compares one + #: spelling, or NULL when every file stores at its decoded size. The + #: newest column here, so it is declared last — see the class docstring. + image_scales: Mapped[dict[str, int] | None] = mapped_column(JSON, nullable=True) #: One origin is one source: the backstop under ``SourceService``'s idempotency @@ -206,17 +210,24 @@ class SourceRow(Base): #: written before ranges existed or after — always lands on ``''``, and a row #: that names ranges lands on its one canonical JSON spelling. #: +#: The sixth and seventh terms follow the same two precedents: a clip stored +#: unscaled omits ``$.scale_percent`` (0 cannot be a real percent — the domain +#: floor is 1), and an image directory whose files all store native has a NULL +#: ``image_scales``, coalesced to ``''`` exactly as a missing ``$.ranges`` is. +#: #: This is SQL reading a JSON column, which the module docstring above reserves #: for values "nothing ever queries". An index is not a query: no service gains #: a JSON path, ``_source_to_domain`` still rehydrates ``VideoProvenance`` whole, #: and the doctrine's purpose — no service building SQL over JSON — is intact. SOURCE_ORIGIN_UNIQUE = Index( - "uq_source_project_kind_path_fps_ranges", + "uq_source_project_kind_path_fps_ranges_scale", SourceRow.project_id, SourceRow.kind, SourceRow.path, text("coalesce(json_extract(video, '$.extraction_fps'), 0)"), text("coalesce(json_extract(video, '$.ranges'), '')"), + text("coalesce(json_extract(video, '$.scale_percent'), 0)"), + text("coalesce(image_scales, '')"), unique=True, ) diff --git a/src/visionset/kernel/adapters/migrations.py b/src/visionset/kernel/adapters/migrations.py index 0376e7b0..ce98f7df 100644 --- a/src/visionset/kernel/adapters/migrations.py +++ b/src/visionset/kernel/adapters/migrations.py @@ -378,12 +378,30 @@ def _reshape_source_origin_index(connection: Connection) -> None: Clip ranges joined the source's identity beside ``extraction_fps``, so the uniqueness backstop has to compare them too. SQLite cannot alter an index: - the old one is dropped by name and the shared declaration created in its - place. Nothing is backfilled — a row written before ranges existed has no - ``$.ranges`` key, which the new index reads as ``''``, the same term a - whole-clip selection serializes to. + the old one is dropped by name. Creating the replacement moved to the head + reshape (migration 18): only one migration may execute the shared + declaration, because it is always the *current* spelling and here it would + reference a column a generation-16 file does not have yet. Nothing is + backfilled — a row written before ranges existed has no ``$.ranges`` key, + which the final index reads as ``''``, the same term a whole-clip + selection serializes to. """ connection.execute(text("DROP INDEX IF EXISTS uq_source_project_kind_path_fps")) + + +def _add_source_scale(connection: Connection) -> None: + """Scale joins the source's identity, so the origin index compares it. + + Two new terms beside fps and ranges: a clip's ``$.scale_percent`` (omitted + at 100, so pre-scale rows and unscaled rows share one spelling) and the + per-file ``image_scales`` column (NULL when every file stores at its + decoded size, for the same reason). SQLite cannot alter an index: the old + one is dropped by name and the shared declaration created in its place. + As the head reshape this is the one migration that may execute the shared + declaration — see ``_reshape_source_origin_index``. + """ + _add_column(connection, "source", "image_scales") + connection.execute(text("DROP INDEX IF EXISTS uq_source_project_kind_path_fps_ranges")) connection.execute(CreateIndex(SOURCE_ORIGIN_UNIQUE, if_not_exists=True)) @@ -419,6 +437,7 @@ def _add_preprocessing_recipes(connection: Connection) -> None: Migration(version=15, name="connection_origin", upgrade=_add_connection_origin), Migration(version=16, name="source_clip_ranges", upgrade=_reshape_source_origin_index), Migration(version=17, name="preprocessing_recipes", upgrade=_add_preprocessing_recipes), + Migration(version=18, name="source_scale", upgrade=_add_source_scale), ] FORMAT_VERSION: int = MIGRATIONS[-1].version diff --git a/tests/kernel/test_migrations.py b/tests/kernel/test_migrations.py index 5dd7e689..d66c22c2 100644 --- a/tests/kernel/test_migrations.py +++ b/tests/kernel/test_migrations.py @@ -188,7 +188,10 @@ def _at_generation_one(path: Path) -> None: connection.execute(text("ALTER TABLE inference_connection DROP COLUMN credential_env")) connection.execute(text("ALTER TABLE project DROP COLUMN created_at")) connection.execute(text("ALTER TABLE inference_connection DROP COLUMN origin")) - connection.execute(text("DROP INDEX uq_source_project_kind_path_fps_ranges")) + # The index reads image_scales, and SQLite refuses to drop a column an + # index still references — the index has to go first. + connection.execute(text("DROP INDEX uq_source_project_kind_path_fps_ranges_scale")) + connection.execute(text("ALTER TABLE source DROP COLUMN image_scales")) connection.execute( text( "CREATE UNIQUE INDEX uq_source_project_kind_path_fps ON source" @@ -317,6 +320,69 @@ def test_the_reshaped_source_index_still_refuses_a_duplicate_origin(tmp_path: Pa migrated.close() +def test_the_scale_terms_fork_the_migrated_index(tmp_path: Path) -> None: + """Migration 18 exercised for real: scale forks identity, its absence collides. + + The first pair differs only in ``$.scale_percent``; the second only in + ``image_scales`` — both must land. A row repeating an existing spelling + exactly must still be refused. + """ + whole = ( + '{"metadata": {"width": 64, "height": 48, "fps": 10.0,' + ' "duration_seconds": 2.0, "codec": "h264"}, "extraction_fps": 1.0}' + ) + scaled = whole[:-1] + ', "scale_percent": 50}' + old = tmp_path / "old.db" + _at_generation_one(old) + with SqliteMetadataStore(old).engine.begin() as connection: + connection.execute(text("insert into workspace (id, name) values ('w', 'ws')")) + connection.execute( + text("insert into project (id, workspace_id, name) values ('p', 'w', 'clips')") + ) + connection.execute( + text( + "insert into source (id, project_id, kind, path, registered_at," + " capture_params, video) values ('s1', 'p', 'video', '/clips/a.mp4'," + f" '2026-01-01T00:00:00+00:00', '{{}}', '{whole}')" + ) + ) + + migrated = SqliteMetadataStore(old) + migrated.initialize() + with migrated.engine.begin() as connection: + connection.execute( + text( + "insert into source (id, project_id, kind, path, registered_at," + " capture_params, video) values ('s2', 'p', 'video', '/clips/a.mp4'," + f" '2026-01-02T00:00:00+00:00', '{{}}', '{scaled}')" + ) + ) + connection.execute( + text( + "insert into source (id, project_id, kind, path, registered_at," + " capture_params) values ('d1', 'p', 'image_directory', '/stills'," + " '2026-01-02T00:00:00+00:00', '{}')" + ) + ) + connection.execute( + text( + "insert into source (id, project_id, kind, path, registered_at," + " capture_params, image_scales) values ('d2', 'p', 'image_directory'," + " '/stills', '2026-01-02T00:00:00+00:00', '{}'," + " '{\"a.png\": 50}')" + ) + ) + with pytest.raises(IntegrityError), migrated.engine.begin() as connection: + connection.execute( + text( + "insert into source (id, project_id, kind, path, registered_at," + " capture_params, video) values ('s3', 'p', 'video', '/clips/a.mp4'," + f" '2026-01-03T00:00:00+00:00', '{{}}', '{scaled}')" + ) + ) + migrated.close() + + def test_running_every_migration_again_changes_nothing(tmp_path: Path) -> None: """Idempotency, and now it covers the baseline rather than skipping it. @@ -346,7 +412,7 @@ def test_running_every_migration_again_changes_nothing(tmp_path: Path) -> None: # three deep and its *order* is the assertion — swapping any two would split # the ``create_all`` path from the migration path. "annotation_schema": ["description", "created_at", "provenance"], - "source": ["display_name"], + "source": ["display_name", "image_scales"], "asset": ["thumbnail_hash", "ingested_at"], # Migration 2 and migration 3, in that order. Both arrive by ``ALTER`` and # SQLite appends, so declaring either anywhere but last would split the diff --git a/tests/kernel/test_schema_draft_service.py b/tests/kernel/test_schema_draft_service.py index a1486553..b0c2aa4b 100644 --- a/tests/kernel/test_schema_draft_service.py +++ b/tests/kernel/test_schema_draft_service.py @@ -41,8 +41,8 @@ def _drafts( return workspace, SchemaDraftService(workspace), project -def test_the_format_version_is_seventeen() -> None: - assert FORMAT_VERSION == 17 +def test_the_format_version_is_eighteen() -> None: + assert FORMAT_VERSION == 18 def test_a_draft_round_trips_with_its_half_typed_classes_intact(tmp_path: Path) -> None: From 97cdd4fb28b113e00166416227357c5cda5bd948 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Fri, 28 Aug 2026 02:35:09 -0700 Subject: [PATCH 03/16] test(kernel): the uniqueness roster names the scale index --- tests/kernel/test_migrations.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/kernel/test_migrations.py b/tests/kernel/test_migrations.py index d66c22c2..1da4ca33 100644 --- a/tests/kernel/test_migrations.py +++ b/tests/kernel/test_migrations.py @@ -45,7 +45,13 @@ # silently: a three-column index would refuse a clip's second extraction # rate, and a nullable fourth column would collide with nothing at all, # because SQLite treats NULLs in a unique index as distinct. - "uq_source_project_kind_path_fps_ranges": ("json_extract", "coalesce", "$.ranges"), + "uq_source_project_kind_path_fps_ranges_scale": ( + "json_extract", + "coalesce", + "$.ranges", + "$.scale_percent", + "image_scales", + ), # Partial, so it constrains classification tags and nothing else: two boxes # under one class are two facts, two tags of one class are one statement # made twice. From e3afd6c1fd2376caf45bdbcd24eb47245a26b439 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Fri, 28 Aug 2026 02:39:46 -0700 Subject: [PATCH 04/16] feat(kernel): registration takes a scale and forks identity on it --- .../kernel/services/source_service.py | 28 ++++++++++-- .../test_preprocessing_recipe_service.py | 2 +- tests/kernel/test_source_service.py | 44 +++++++++++++++++++ 3 files changed, 69 insertions(+), 5 deletions(-) diff --git a/src/visionset/kernel/services/source_service.py b/src/visionset/kernel/services/source_service.py index c3964ed6..e0310df6 100644 --- a/src/visionset/kernel/services/source_service.py +++ b/src/visionset/kernel/services/source_service.py @@ -14,7 +14,9 @@ callers. **Registration is idempotent, and the match key is ``(kind, path, -extraction_fps, ranges)``.** Registering the same origin twice returns the same +extraction_fps, ranges, scale)``** — a clip's ``scale_percent`` and a +directory's per-file ``image_scales`` both fork identity, because the stored +pixels differ. Registering the same origin twice returns the same ``Source`` rather than a second one, so that "which source did this asset come from?" has one answer through ``asset.source_id``. The key deliberately excludes ``capture_params``: fragmenting one directory into two @@ -52,6 +54,7 @@ SourceKind, TimeRange, VideoProvenance, + canonical_image_scales, canonical_path, canonical_ranges, normalize_name, @@ -87,6 +90,7 @@ def register_images( *, capture_params: Mapping[str, str] | None = None, display_name: str | None = None, + image_scales: Mapping[str, int] | None = None, ) -> Source: """Record a directory of stills as an origin for this project. @@ -125,6 +129,7 @@ def register_images( display_name=( None if display_name is None else normalize_name(display_name, what="source name") ), + image_scales=canonical_image_scales(image_scales or {}), ) def register_video( @@ -134,6 +139,7 @@ def register_video( *, extraction_fps: float = DEFAULT_EXTRACTION_FPS, ranges: Sequence[TimeRange] = (), + scale_percent: int = 100, capture_params: Mapping[str, str] | None = None, ) -> Source: """Record a video file as an origin, with what a probe makes of it. @@ -176,7 +182,10 @@ def register_video( SourceKind.VIDEO, path, video=VideoProvenance( - metadata=metadata, extraction_fps=extraction_fps, ranges=canonical + metadata=metadata, + extraction_fps=extraction_fps, + ranges=canonical, + scale_percent=scale_percent, ), capture_params=capture_params, ) @@ -212,10 +221,15 @@ def _register( video: VideoProvenance | None, capture_params: Mapping[str, str] | None, display_name: str | None = None, + image_scales: dict[str, int] | None = None, ) -> Source: """Add the source, or return the one that already stands for this origin.""" params = dict(capture_params or {}) - cut = None if video is None else (video.extraction_fps, video.ranges) + scales = image_scales or {} + cut = ( + None if video is None else (video.extraction_fps, video.ranges, video.scale_percent), + scales, + ) with self._workspace.unit_of_work() as uow: self._require_project(uow, project_id) for stored in uow.sources.list(project_id): @@ -224,7 +238,12 @@ def _register( stored_cut = ( None if stored.video is None - else (stored.video.extraction_fps, stored.video.ranges) + else ( + stored.video.extraction_fps, + stored.video.ranges, + stored.video.scale_percent, + ), + stored.image_scales, ) if stored_cut != cut: continue @@ -252,6 +271,7 @@ def _register( display_name=display_name, capture_params=params, video=video, + image_scales=scales, ) ) diff --git a/tests/kernel/test_preprocessing_recipe_service.py b/tests/kernel/test_preprocessing_recipe_service.py index 20419258..99ef7701 100644 --- a/tests/kernel/test_preprocessing_recipe_service.py +++ b/tests/kernel/test_preprocessing_recipe_service.py @@ -196,7 +196,7 @@ def test_migration_seventeen_adds_the_table_to_an_older_file(tmp_path: Path) -> reopened.initialize() with reopened.engine.connect() as connection: assert "preprocessing_recipes" in inspect(connection).get_table_names() - assert reopened.format_version == FORMAT_VERSION == 17 + assert reopened.format_version == FORMAT_VERSION reopened.close() diff --git a/tests/kernel/test_source_service.py b/tests/kernel/test_source_service.py index bac90946..c3cd932b 100644 --- a/tests/kernel/test_source_service.py +++ b/tests/kernel/test_source_service.py @@ -248,6 +248,50 @@ def test_the_same_clip_with_different_ranges_is_a_second_source(tmp_path: Path) fx.close() +def test_the_same_clip_at_a_different_scale_is_a_second_source(tmp_path: Path) -> None: + """The scale is the third cut parameter, so it forks identity as the rate does.""" + fx = Fixture(tmp_path) + clip = fx.clip() + native = fx.sources.register_video(fx.project.id, clip.path, scale_percent=100) + half = fx.sources.register_video(fx.project.id, clip.path, scale_percent=50) + assert half.id != native.id + assert half.require_video().scale_percent == 50 + assert {s.id for s in fx.sources.list(fx.project.id)} == {native.id, half.id} + fx.close() + + +def test_a_scale_of_one_hundred_is_the_plain_source(tmp_path: Path) -> None: + fx = Fixture(tmp_path) + clip = fx.clip() + plain = fx.sources.register_video(fx.project.id, clip.path) + explicit = fx.sources.register_video(fx.project.id, clip.path, scale_percent=100) + assert explicit == plain + assert len(fx.sources.list(fx.project.id)) == 1 + fx.close() + + +def test_the_same_directory_with_different_scales_is_a_second_source(tmp_path: Path) -> None: + fx = Fixture(tmp_path) + first = fx.sources.register_images(fx.project.id, fx.stills) + second = fx.sources.register_images(fx.project.id, fx.stills, image_scales={"a.png": 50}) + assert second.id != first.id + assert second.image_scales == {"a.png": 50} + assert {s.id for s in fx.sources.list(fx.project.id)} == {first.id, second.id} + fx.close() + + +def test_image_scale_spelling_variants_collapse_to_one_source(tmp_path: Path) -> None: + """Identity compares the canonical form, never what a caller happened to type.""" + fx = Fixture(tmp_path) + messy = fx.sources.register_images( + fx.project.id, fx.stills, image_scales={"b.png": 100, "a.png": 50} + ) + tidy = fx.sources.register_images(fx.project.id, fx.stills, image_scales={"a.png": 50}) + assert tidy == messy + assert len(fx.sources.list(fx.project.id)) == 1 + fx.close() + + def test_range_spelling_variants_collapse_to_one_source(tmp_path: Path) -> None: """Identity compares the canonical form, never what a caller happened to type.""" fx = Fixture(tmp_path) From df33e3cbd51091ef712bef0528bf86dbf1fe5981 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Fri, 28 Aug 2026 02:42:52 -0700 Subject: [PATCH 05/16] feat(kernel): ffmpeg extraction scales frames to the stored size --- .../kernel/adapters/ffmpeg_video_processor.py | 36 ++++++++++++++----- src/visionset/kernel/ports/video_processor.py | 1 + .../kernel/services/ingest_service.py | 14 ++++++-- tests/kernel/test_ingest_service.py | 24 +++++++++++++ tests/kernel/test_video_processor.py | 23 ++++++++++-- 5 files changed, 84 insertions(+), 14 deletions(-) diff --git a/src/visionset/kernel/adapters/ffmpeg_video_processor.py b/src/visionset/kernel/adapters/ffmpeg_video_processor.py index 36c4a9e0..bc505422 100644 --- a/src/visionset/kernel/adapters/ffmpeg_video_processor.py +++ b/src/visionset/kernel/adapters/ffmpeg_video_processor.py @@ -419,6 +419,7 @@ def frames( fps: float = DEFAULT_EXTRACTION_FPS, ranges: tuple[TimeRange, ...] = (), name: str | None = None, + scale: tuple[int, int] | None = None, ) -> Iterator[VideoFrame]: """Frames taken off ``source`` at ``fps``, one at a time, in grid order. @@ -427,6 +428,12 @@ def frames( each kept frame carries its grid index, byte-identical to the frame a whole-clip run yields at that index. + ``scale`` is the exact ``(width, height)`` every emitted frame is + resized to, computed by the caller from the probe — never ffmpeg-side + arithmetic, so the command stays deterministic and the probe's + display-oriented dimensions stay authoritative. ``None`` emits frames + at the decoded size. + Not a generator itself, on purpose. A generator's body does not run until something asks it for a value, so writing it that way would report a missing ffmpeg — or a negative ``fps`` — at the first iteration, in @@ -451,7 +458,7 @@ def frames( ffmpeg = _require_tool(_FFMPEG) _require_file(source) bounds = grid_bounds(ranges, fps=fps) - return _extract(ffmpeg, source, fps, bounds, _clip_name(source, name)) + return _extract(ffmpeg, source, fps, bounds, scale, _clip_name(source, name)) def _run_ffprobe(ffprobe: str, source: Path, clip: str) -> Mapping[str, object]: @@ -488,7 +495,9 @@ def _video_stream(document: Mapping[str, object], clip: str) -> Mapping[str, obj raise UnsupportedMedia("the file holds no video stream", name=clip) -def _filtergraph(fps: float, bounds: tuple[tuple[int, int], ...]) -> str: +def _filtergraph( + fps: float, bounds: tuple[tuple[int, int], ...], scale: tuple[int, int] | None +) -> str: """The resampling grid, plus — under a selection — the frames kept off it. ``select`` runs *after* ``fps`` and compares the integer output frame number @@ -496,17 +505,26 @@ def _filtergraph(fps: float, bounds: tuple[tuple[int, int], ...]) -> str: ffmpeg: the arithmetic naming the kept frames is the same ``grid_bounds`` the expected count uses, so the two cannot disagree. The quotes around the expression are filtergraph quoting — they keep its commas from splitting - the graph. + the graph. ``scale`` runs last, so a selection drops frames before any of + them are resized. """ grid = f"fps=fps={fps}:round=up" - if not bounds: - return grid - kept = "+".join(f"gte(n,{a})*lt(n,{b})" for a, b in bounds) - return f"{grid},select='{kept}'" + if bounds: + kept = "+".join(f"gte(n,{a})*lt(n,{b})" for a, b in bounds) + grid = f"{grid},select='{kept}'" + if scale is not None: + width, height = scale + grid = f"{grid},scale={width}:{height}" + return grid def _extract( - ffmpeg: str, source: Path, fps: float, bounds: tuple[tuple[int, int], ...], clip: str + ffmpeg: str, + source: Path, + fps: float, + bounds: tuple[tuple[int, int], ...], + scale: tuple[int, int] | None, + clip: str, ) -> Iterator[VideoFrame]: """Stream frames off one ffmpeg process, and account for how it ended. @@ -530,7 +548,7 @@ def _extract( "-nostdin", "-loglevel", "error", "-i", str(source), - "-vf", _filtergraph(fps, bounds), + "-vf", _filtergraph(fps, bounds, scale), *_EXTRACTION_ARGS, *(_RANGE_ARGS if bounds else ()), "-", diff --git a/src/visionset/kernel/ports/video_processor.py b/src/visionset/kernel/ports/video_processor.py index 4c4316b2..a47e473d 100644 --- a/src/visionset/kernel/ports/video_processor.py +++ b/src/visionset/kernel/ports/video_processor.py @@ -112,4 +112,5 @@ def frames( fps: float = DEFAULT_EXTRACTION_FPS, ranges: tuple[TimeRange, ...] = (), name: str | None = None, + scale: tuple[int, int] | None = None, ) -> Iterator[VideoFrame]: ... diff --git a/src/visionset/kernel/services/ingest_service.py b/src/visionset/kernel/services/ingest_service.py index 5042a294..6e6412ed 100644 --- a/src/visionset/kernel/services/ingest_service.py +++ b/src/visionset/kernel/services/ingest_service.py @@ -715,7 +715,15 @@ def _read_video(self, source: Source, job_id: UUID) -> tuple[list[Asset], list[I failures: list[IngestFailure] = [] clip = Path(source.path) frames = self._workspace.video_processor.frames( - clip, fps=provenance.extraction_fps, ranges=provenance.ranges, name=clip.name + clip, + fps=provenance.extraction_fps, + ranges=provenance.ranges, + name=clip.name, + scale=( + None + if provenance.scale_percent == 100 + else (provenance.stored_width, provenance.stored_height) + ), ) self._record_progress(job_id, processed=0, total=None, failures=failures) try: @@ -731,8 +739,8 @@ def _read_video(self, source: Source, job_id: UUID) -> tuple[list[Asset], list[I project_id=source.project_id, content_hash=content_hash, uri=uri, - width=provenance.metadata.width, - height=provenance.metadata.height, + width=provenance.stored_width, + height=provenance.stored_height, format=FRAME_FORMAT, source_id=source.id, frame_index=frame.index, diff --git a/tests/kernel/test_ingest_service.py b/tests/kernel/test_ingest_service.py index 189c887f..96ebd22f 100644 --- a/tests/kernel/test_ingest_service.py +++ b/tests/kernel/test_ingest_service.py @@ -68,6 +68,7 @@ VideoFrame, VideoMetadata, VideoProvenance, + scaled_dimension, ) from visionset.kernel.ports import ( DEFAULT_THUMBNAIL_MAX_EDGE, @@ -106,6 +107,7 @@ def frames( fps: float = 1.0, ranges: tuple[TimeRange, ...] = (), name: str | None = None, + scale: tuple[int, int] | None = None, ) -> "list[VideoFrame]": raise MediaToolUnavailable("ffmpeg is not installed; install it and try again") @@ -493,6 +495,28 @@ def test_a_frame_takes_its_size_from_the_probe_and_its_format_from_the_port( fixture.close() +def test_a_scaled_clip_ingests_frames_at_the_stored_size(tmp_path: Path) -> None: + """The asset records the scaled dimensions, and the pixels agree with them.""" + fixture = Fixture(tmp_path) + clip = fixture.clip() + source = fixture.sources.register_video( + fixture.project.id, clip.path, extraction_fps=1.0, scale_percent=50 + ) + + result = fixture.ingest.ingest(source.id) + + expected = (scaled_dimension(clip.width, 50), scaled_dimension(clip.height, 50)) + assert result.assets + for asset in result.assets: + assert (asset.width, asset.height) == expected + with ( + fixture.workspace.blob_store.get(result.assets[0].content_hash) as blob, + Image.open(blob) as picture, + ): + assert picture.size == expected + fixture.close() + + def test_a_rotated_clip_yields_frames_at_their_displayed_size(tmp_path: Path) -> None: """The display matrix is applied; a 64x48 file held upright ingests as 48x64.""" fixture = Fixture(tmp_path) diff --git a/tests/kernel/test_video_processor.py b/tests/kernel/test_video_processor.py index d0d8feeb..31dc8725 100644 --- a/tests/kernel/test_video_processor.py +++ b/tests/kernel/test_video_processor.py @@ -39,6 +39,7 @@ from pathlib import Path import pytest +from PIL import Image as PILImage from tests.fixtures.media import ( DEFAULT_VIDEO_SIZE, GeneratedVideo, @@ -585,17 +586,34 @@ def test_the_extraction_arguments_are_pinned() -> None: def test_the_whole_clip_command_is_unchanged_by_the_ranges_feature() -> None: """No selection, no new arguments: every frame hash ever stored stays put.""" - assert _filtergraph(5, ()) == "fps=fps=5:round=up" + assert _filtergraph(5, (), None) == "fps=fps=5:round=up" def test_the_selection_filter_compares_integer_frame_numbers() -> None: """Bounds are precomputed in Python; ffmpeg never compares a float timestamp.""" assert ( - _filtergraph(1, ((2, 5), (9, 12))) + _filtergraph(1, ((2, 5), (9, 12)), None) == "fps=fps=1:round=up,select='gte(n,2)*lt(n,5)+gte(n,9)*lt(n,12)'" ) +def test_the_scale_stage_lands_after_the_selection() -> None: + """Dimensions come precomputed from the probe, never as ffmpeg-side arithmetic.""" + assert _filtergraph(2, (), (960, 540)) == "fps=fps=2:round=up,scale=960:540" + assert ( + _filtergraph(1, ((2, 5),), (960, 540)) + == "fps=fps=1:round=up,select='gte(n,2)*lt(n,5)',scale=960:540" + ) + + +def test_scaled_extraction_emits_frames_at_the_stored_size(clip: GeneratedVideo) -> None: + frames = list(FfmpegVideoProcessor().frames(clip.path, fps=1, scale=(32, 24))) + assert frames + for frame in frames: + with PILImage.open(io.BytesIO(frame.content)) as picture: + assert picture.size == (32, 24) + + @pytest.mark.parametrize( ("value", "expected"), [("10/1", 10.0), ("30000/1001", 29.97002997002997), ("0/0", None), ("", None), (25, 25.0)], @@ -622,6 +640,7 @@ def frames( fps: float = DEFAULT_EXTRACTION_FPS, ranges: tuple[TimeRange, ...] = (), name: str | None = None, + scale: tuple[int, int] | None = None, ) -> Iterator[VideoFrame]: return iter(()) From 2f5fe32eeeb109005be071cee6910430c0c50cb3 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Fri, 28 Aug 2026 02:46:16 -0700 Subject: [PATCH 06/16] feat(kernel): stills resize to their per-file scale before encoding --- .../kernel/adapters/pillow_image_processor.py | 36 ++++++++--- src/visionset/kernel/ports/image_processor.py | 9 ++- .../kernel/services/ingest_service.py | 6 +- tests/kernel/test_image_processor.py | 61 ++++++++++++++++++- tests/kernel/test_ingest_service.py | 37 ++++++++++- 5 files changed, 131 insertions(+), 18 deletions(-) diff --git a/src/visionset/kernel/adapters/pillow_image_processor.py b/src/visionset/kernel/adapters/pillow_image_processor.py index ffbac95c..4f45f7ba 100644 --- a/src/visionset/kernel/adapters/pillow_image_processor.py +++ b/src/visionset/kernel/adapters/pillow_image_processor.py @@ -9,7 +9,7 @@ from PIL import Image, ImageOps, UnidentifiedImageError from pillow_heif import register_heif_opener -from visionset.kernel.domain import DecodedStill, ImageFormat, ImageMetadata +from visionset.kernel.domain import DecodedStill, ImageFormat, ImageMetadata, scaled_dimension from visionset.kernel.errors import CorruptMedia, UnsupportedMedia from visionset.kernel.ports.image_processor import DEFAULT_THUMBNAIL_MAX_EDGE @@ -153,6 +153,18 @@ def _fit(image: Image.Image, max_edge: int) -> Image.Image: return _opaque_rgb(working) +def _scaled(image: Image.Image, scale_percent: int) -> Image.Image: + if scale_percent == 100: + return image + return image.resize( + ( + scaled_dimension(image.width, scale_percent), + scaled_dimension(image.height, scale_percent), + ), + _RESAMPLING, + ) + + def _opaque_rgb(image: Image.Image) -> Image.Image: """The compositing tail of :func:`_fit`, alone: full-size, no resampling. @@ -240,18 +252,22 @@ def thumbnail( canvas.save(buffer, format=_THUMBNAIL_PILLOW_NAME, **_THUMBNAIL_ENCODER) return buffer.getvalue() - def stills(self, content: BinaryIO, *, name: str | None = None) -> Iterator[DecodedStill]: + def stills( + self, content: BinaryIO, *, name: str | None = None, scale_percent: int = 100 + ) -> Iterator[DecodedStill]: """Every dataset-ready still in this file. See the port docstring. The native gate runs before the frame count on purpose: MPO decodes with ``n_frames > 1``, and checking frames first would decompose every - burst photo instead of passing its primary frame through. + burst photo instead of passing its primary frame through. A + ``scale_percent`` below 100 closes that gate: the original bytes are no + longer the asset, so even a dataset-ready native re-encodes, resized. """ source = _stream_name(content, name) image = self._open(io.BytesIO(_read_all(content)), source) native = _FORMAT_BY_PILLOW_NAME.get(image.format or "") - if native is not None: + if native is not None and scale_percent == 100: self._load(image, source) with image: width, height = image.size @@ -259,13 +275,13 @@ def stills(self, content: BinaryIO, *, name: str | None = None) -> Iterator[Deco [DecodedStill(metadata=ImageMetadata(width=width, height=height, format=native))] ) - if getattr(image, "n_frames", 1) > 1: + if native is None and getattr(image, "n_frames", 1) > 1: with image: - return iter(self._decomposed(image, source)) + return iter(self._decomposed(image, source, scale_percent)) self._load(image, source) with image: - flat = _opaque_rgb(image) + flat = _scaled(_opaque_rgb(image), scale_percent) buffer = io.BytesIO() flat.save(buffer, format="JPEG", **_STILL_ENCODER) return iter( @@ -279,7 +295,9 @@ def stills(self, content: BinaryIO, *, name: str | None = None) -> Iterator[Deco ] ) - def _decomposed(self, image: Image.Image, source: str | None) -> list[DecodedStill]: + def _decomposed( + self, image: Image.Image, source: str | None, scale_percent: int + ) -> list[DecodedStill]: """One PNG still per frame — a list, not a generator, on purpose. Every frame is decoded and encoded before the caller sees the first @@ -292,7 +310,7 @@ def _decomposed(self, image: Image.Image, source: str | None) -> list[DecodedSti for index in range(int(getattr(image, "n_frames", 1))): try: image.seek(index) - frame = _opaque_rgb(image) + frame = _scaled(_opaque_rgb(image), scale_percent) except Image.DecompressionBombError as exc: raise UnsupportedMedia(str(exc), name=source) from exc except (EOFError, OSError, SyntaxError, ValueError) as exc: diff --git a/src/visionset/kernel/ports/image_processor.py b/src/visionset/kernel/ports/image_processor.py index b47c3946..df7c08e0 100644 --- a/src/visionset/kernel/ports/image_processor.py +++ b/src/visionset/kernel/ports/image_processor.py @@ -71,7 +71,10 @@ class ImageProcessor(Protocol): image yields transcoded items — one ``CONVERTED_STILL_FORMAT`` still for a single frame, one ``DECOMPOSED_FRAME_FORMAT`` still per frame of an animation. The decode is complete before the first item is yielded, so a - damaged file raises before a caller has stored anything. + damaged file raises before a caller has stored anything. A + ``scale_percent`` below 100 resizes every emitted still and forces the + re-encode even for a dataset-ready native — the original bytes are no + longer the asset. Raises: UnsupportedMedia: the bytes are not an image the decoder reads (for @@ -90,4 +93,6 @@ def thumbnail( name: str | None = None, ) -> bytes: ... - def stills(self, content: BinaryIO, *, name: str | None = None) -> Iterator[DecodedStill]: ... + def stills( + self, content: BinaryIO, *, name: str | None = None, scale_percent: int = 100 + ) -> Iterator[DecodedStill]: ... diff --git a/src/visionset/kernel/services/ingest_service.py b/src/visionset/kernel/services/ingest_service.py index 6e6412ed..d9268844 100644 --- a/src/visionset/kernel/services/ingest_service.py +++ b/src/visionset/kernel/services/ingest_service.py @@ -620,7 +620,11 @@ def _read_directory( # frame of an animation included — before it yields its # first item, so a refusal arrives before anything below # stores a byte. - for still in self._workspace.image_processor.stills(handle, name=str(path)): + for still in self._workspace.image_processor.stills( + handle, + name=str(path), + scale_percent=source.image_scales.get(path.name, 100), + ): uri = ( str(path) if still.frame_index is None diff --git a/tests/kernel/test_image_processor.py b/tests/kernel/test_image_processor.py index 5c817c61..f88a7e9f 100644 --- a/tests/kernel/test_image_processor.py +++ b/tests/kernel/test_image_processor.py @@ -533,7 +533,9 @@ def thumbnail( ) -> bytes: return b"" - def stills(self, content: io.IOBase, *, name: str | None = None) -> Iterator[DecodedStill]: + def stills( + self, content: io.IOBase, *, name: str | None = None, scale_percent: int = 100 + ) -> Iterator[DecodedStill]: return iter(()) @@ -580,9 +582,11 @@ def test_a_decoded_still_is_frozen_and_defaults_to_pass_through() -> None: # --- stills: accept what Pillow decodes, normalized to JPEG and PNG ------------ -def _stills(path: Path) -> list[DecodedStill]: +def _stills(path: Path, *, scale_percent: int = 100) -> list[DecodedStill]: with path.open("rb") as handle: - return list(PillowImageProcessor().stills(handle, name=str(path))) + return list( + PillowImageProcessor().stills(handle, name=str(path), scale_percent=scale_percent) + ) def test_a_native_jpeg_passes_through_with_no_payload(tmp_path: Path) -> None: @@ -657,6 +661,57 @@ def test_a_transcode_is_repeatable_within_this_build(tmp_path: Path) -> None: assert _stills(path)[0].payload == _stills(path)[0].payload +def test_a_scaled_native_jpeg_re_encodes_at_the_stored_size(tmp_path: Path) -> None: + """Scaling forces the re-encode: the original bytes are no longer the asset.""" + path = tmp_path / "native.jpg" + Image.new("RGB", (100, 80), (200, 10, 10)).save(path, format="JPEG") + + (still,) = _stills(path, scale_percent=50) + + assert still.payload is not None + assert still.metadata == ImageMetadata(width=50, height=40, format=ImageFormat.JPEG) + assert _decoded(still.payload).size == (50, 40) + + +def test_a_scaled_native_png_re_encodes_as_jpeg(tmp_path: Path) -> None: + path = tmp_path / "native.png" + Image.new("RGB", (100, 80), (10, 200, 10)).save(path, format="PNG") + + (still,) = _stills(path, scale_percent=50) + + assert still.payload is not None + assert still.metadata.format is ImageFormat.JPEG + assert (still.metadata.width, still.metadata.height) == (50, 40) + + +def test_scaled_animation_frames_shrink_and_stay_png(tmp_path: Path) -> None: + path = tmp_path / "anim.gif" + _animated_gif(path, frames=3) + + stills = _stills(path, scale_percent=50) + + assert [still.metadata.format for still in stills] == [ImageFormat.PNG] * 3 + assert all((s.metadata.width, s.metadata.height) == (8, 6) for s in stills) + + +def test_scale_one_hundred_still_passes_natives_through(tmp_path: Path) -> None: + path = tmp_path / "native.jpg" + Image.new("RGB", (32, 24), (5, 5, 5)).save(path, format="JPEG") + + (still,) = _stills(path, scale_percent=100) + + assert still.payload is None + + +def test_a_scaled_dimension_never_reaches_zero(tmp_path: Path) -> None: + path = tmp_path / "sliver.png" + Image.new("RGB", (100, 1), (1, 2, 3)).save(path, format="PNG") + + (still,) = _stills(path, scale_percent=10) + + assert (still.metadata.width, still.metadata.height) == (10, 1) + + def test_stills_refuses_what_pillow_cannot_decode(tmp_path: Path) -> None: path = write_unsupported_file(tmp_path / "notes.txt") diff --git a/tests/kernel/test_ingest_service.py b/tests/kernel/test_ingest_service.py index 96ebd22f..267ae676 100644 --- a/tests/kernel/test_ingest_service.py +++ b/tests/kernel/test_ingest_service.py @@ -138,7 +138,9 @@ def probe(self, content: BinaryIO, *, name: str | None = None) -> ImageMetadata: self._observe() return self._real.probe(content, name=name) - def stills(self, content: BinaryIO, *, name: str | None = None) -> Iterator[DecodedStill]: + def stills( + self, content: BinaryIO, *, name: str | None = None, scale_percent: int = 100 + ) -> Iterator[DecodedStill]: self._observe() return self._real.stills(content, name=name) @@ -164,7 +166,9 @@ def __init__(self, nth: int) -> None: def probe(self, content: BinaryIO, *, name: str | None = None) -> ImageMetadata: return self._real.probe(content, name=name) - def stills(self, content: BinaryIO, *, name: str | None = None) -> Iterator[DecodedStill]: + def stills( + self, content: BinaryIO, *, name: str | None = None, scale_percent: int = 100 + ) -> Iterator[DecodedStill]: self._calls += 1 if self._calls == self._nth: raise OSError("the disk went away") @@ -190,7 +194,9 @@ def __init__(self) -> None: def probe(self, content: BinaryIO, *, name: str | None = None) -> ImageMetadata: return self._real.probe(content, name=name) - def stills(self, content: BinaryIO, *, name: str | None = None) -> Iterator[DecodedStill]: + def stills( + self, content: BinaryIO, *, name: str | None = None, scale_percent: int = 100 + ) -> Iterator[DecodedStill]: return self._real.stills(content, name=name) def thumbnail( @@ -495,6 +501,31 @@ def test_a_frame_takes_its_size_from_the_probe_and_its_format_from_the_port( fixture.close() +def test_a_scaled_still_ingests_at_its_own_percent_and_neighbors_stay_native( + tmp_path: Path, +) -> None: + """The per-file map applies per file: one entry scales one file, the rest pass through.""" + fixture = Fixture(tmp_path) + paths = write_images(fixture.stills, count=2) + source = fixture.sources.register_images( + fixture.project.id, fixture.stills, image_scales={paths[0].name: 50} + ) + + result = fixture.ingest.ingest(source.id) + + by_name = {asset.uri.rsplit("/", 1)[-1]: asset for asset in result.assets} + with Image.open(paths[0]) as native: + expected = (scaled_dimension(native.width, 50), scaled_dimension(native.height, 50)) + native_size = native.size + scaled = by_name[paths[0].name] + untouched = by_name[paths[1].name] + assert (scaled.width, scaled.height) == expected + assert scaled.format is ImageFormat.JPEG + assert (untouched.width, untouched.height) == native_size + assert untouched.format is ImageFormat.PNG + fixture.close() + + def test_a_scaled_clip_ingests_frames_at_the_stored_size(tmp_path: Path) -> None: """The asset records the scaled dimensions, and the pixels agree with them.""" fixture = Fixture(tmp_path) From 0bfca7b227ea429cf037a56ff8475defb4325f1d Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Fri, 28 Aug 2026 02:57:12 -0700 Subject: [PATCH 07/16] feat(server): registration accepts a scale and publishes it on the source --- frontend/ui-core/src/generated/api.ts | 41 +++++++-- frontend/ui-core/src/generated/checks.ts | 4 +- openapi.json | 45 ++++++++-- src/visionset/server/models.py | 17 +++- src/visionset/server/routes/sources.py | 106 +++++++++++++++++++++-- src/visionset/wire/__init__.py | 2 + tests/server/test_sources.py | 79 ++++++++++++++++- 7 files changed, 270 insertions(+), 24 deletions(-) diff --git a/frontend/ui-core/src/generated/api.ts b/frontend/ui-core/src/generated/api.ts index 9f2584bb..240c9908 100644 --- a/frontend/ui-core/src/generated/api.ts +++ b/frontend/ui-core/src/generated/api.ts @@ -2661,7 +2661,7 @@ export interface paths { put?: never; /** * Register Image Source - * @description Offer a project a folder of stills. + * @description Offer a project a folder of stills, each stored at its own scale. * * The parts are staged as one directory and that directory becomes the source. * Uploading the same files again returns the **same** source rather than a @@ -2674,6 +2674,9 @@ export interface paths { * `name` exists because the staged path's basename is a digest; a blank one is * 422 `INVALID_NAME`, refused by the kernel's own `InvalidName` — the domain * already refuses with a mapped error, so no wire validator restates it. + * + * `scales` names files to store below native size, and is part of the + * source's identity: the same files at other scales are a second source. */ post: operations["register_image_source"]; delete?: never; @@ -2702,10 +2705,11 @@ export interface paths { * message says what was wrong with the file and never where it was put. * * The cut is part of what the source *is*: the same clip registered at 1 fps - * and again at 5 fps — or over different ranges — is two sources over one - * file, which is what makes "the same source yields the same assets" mean - * anything. Ranges are stored canonically (clamped, sorted, merged), and the - * response carries that canonical form. + * and again at 5 fps — or over different ranges, or at another scale — is two + * sources over one file, which is what makes "the same source yields the same + * assets" mean anything. Ranges are stored canonically (clamped, sorted, + * merged), and the response carries that canonical form. `scale_percent` + * below 100 stores every extracted frame at that percent of the clip's size. */ post: operations["register_video_source"]; delete?: never; @@ -3690,6 +3694,11 @@ export interface components { * @description What to call the source. Without one it is named by its staged directory, whose basename is a content digest — 64 hex characters nobody can read. Registering the same files again with a new name renames the existing source rather than creating a second one. */ name?: string | null; + /** + * Scales + * @description Per-file downscale, as a JSON object of {"filename": percent} with integer percents in [1, 100]. Every filename must match an uploaded part; a file not named — and any entry of 100 — is stored at its decoded size. Part of the source's identity: the same files at other scales are a second source. + */ + scales?: string | null; }; /** Body_register_video_source */ Body_register_video_source: { @@ -3709,6 +3718,12 @@ export interface components { * @description Which stretches of the clip to extract, as a JSON array of {"start_seconds": s, "end_seconds": e} objects, each half-open [start, end). Omitted means the whole clip. */ ranges?: string | null; + /** + * Scale Percent + * @description Percent of the native size to store extracted frames at; 100 — the default — stores them unscaled. Part of the source's identity, like extraction_fps: the same clip at another scale is a second source. + * @default 100 + */ + scale_percent: number; }; /** * BySegmentsBody @@ -5611,6 +5626,11 @@ export interface components { /** * SourceOut * @description A registered origin: a folder of stills, or a clip. + * + * `image_scales` is an image directory's per-file downscale, filename to + * percent. Only files stored below native size appear; an empty object means + * every file stores at its decoded size. Always empty for a video source, + * whose single `scale_percent` lives on `video`. */ SourceOut: { /** @@ -5618,6 +5638,10 @@ export interface components { * Format: uuid */ id: string; + /** Image Scales */ + image_scales: { + [key: string]: number; + }; kind: components["schemas"]["SourceKind"]; /** Name */ name: string; @@ -5801,6 +5825,11 @@ export interface components { * `ranges` is the canonical form of the selection the source was registered * with — clamped to the clip, sorted, overlaps merged — and empty means the * whole clip. Like `extraction_fps`, it is part of the source's identity. + * + * `scale_percent` is the percent of the native size extracted frames are + * stored at; 100 means unscaled. `width` and `height` stay the clip's own — + * what is stored is each dimension scaled by this percent. Also part of the + * source's identity. */ VideoProvenanceOut: { /** Codec */ @@ -5815,6 +5844,8 @@ export interface components { height: number; /** Ranges */ ranges: components["schemas"]["ClipRange"][]; + /** Scale Percent */ + scale_percent: number; /** Width */ width: number; }; diff --git a/frontend/ui-core/src/generated/checks.ts b/frontend/ui-core/src/generated/checks.ts index cf4f85e1..c208a41c 100644 --- a/frontend/ui-core/src/generated/checks.ts +++ b/frontend/ui-core/src/generated/checks.ts @@ -376,10 +376,10 @@ export const checkClipRange: Check = /*#__PURE__*/ object({ "end_seconds": [true, isNumber], "start_seconds": [true, isNumber] } as const); export const checkVideoProvenanceOut: Check = - /*#__PURE__*/ object({ "codec": [true, isString], "duration_seconds": [true, isNumber], "extraction_fps": [true, isNumber], "fps": [true, isNumber], "height": [true, isInteger], "ranges": [true, arrayOf(checkClipRange)], "width": [true, isInteger] } as const); + /*#__PURE__*/ object({ "codec": [true, isString], "duration_seconds": [true, isNumber], "extraction_fps": [true, isNumber], "fps": [true, isNumber], "height": [true, isInteger], "ranges": [true, arrayOf(checkClipRange)], "scale_percent": [true, isInteger], "width": [true, isInteger] } as const); export const checkSourceOut: Check = - /*#__PURE__*/ object({ "id": [true, isString], "kind": [true, checkSourceKind], "name": [true, isString], "project_id": [true, isString], "registered_at": [true, isString], "video": [true, either([checkVideoProvenanceOut, isNull] as const)] } as const); + /*#__PURE__*/ object({ "id": [true, isString], "image_scales": [true, mapOf(isInteger)], "kind": [true, checkSourceKind], "name": [true, isString], "project_id": [true, isString], "registered_at": [true, isString], "video": [true, either([checkVideoProvenanceOut, isNull] as const)] } as const); export const checkSourcePage: Check = /*#__PURE__*/ object({ "items": [true, arrayOf(checkSourceOut)], "total": [true, isInteger] } as const); diff --git a/openapi.json b/openapi.json index 74f79233..a2cd594f 100644 --- a/openapi.json +++ b/openapi.json @@ -1636,6 +1636,18 @@ ], "description": "What to call the source. Without one it is named by its staged directory, whose basename is a content digest \u2014 64 hex characters nobody can read. Registering the same files again with a new name renames the existing source rather than creating a second one.", "title": "Name" + }, + "scales": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "description": "Per-file downscale, as a JSON object of {\"filename\": percent} with integer percents in [1, 100]. Every filename must match an uploaded part; a file not named \u2014 and any entry of 100 \u2014 is stored at its decoded size. Part of the source's identity: the same files at other scales are a second source.", + "title": "Scales" } }, "required": [ @@ -1670,6 +1682,14 @@ ], "description": "Which stretches of the clip to extract, as a JSON array of {\"start_seconds\": s, \"end_seconds\": e} objects, each half-open [start, end). Omitted means the whole clip.", "title": "Ranges" + }, + "scale_percent": { + "default": 100, + "description": "Percent of the native size to store extracted frames at; 100 \u2014 the default \u2014 stores them unscaled. Part of the source's identity, like extraction_fps: the same clip at another scale is a second source.", + "maximum": 100.0, + "minimum": 1.0, + "title": "Scale Percent", + "type": "integer" } }, "required": [ @@ -5476,13 +5496,20 @@ "type": "string" }, "SourceOut": { - "description": "A registered origin: a folder of stills, or a clip.", + "description": "A registered origin: a folder of stills, or a clip.\n\n`image_scales` is an image directory's per-file downscale, filename to\npercent. Only files stored below native size appear; an empty object means\nevery file stores at its decoded size. Always empty for a video source,\nwhose single `scale_percent` lives on `video`.", "properties": { "id": { "format": "uuid", "title": "Id", "type": "string" }, + "image_scales": { + "additionalProperties": { + "type": "integer" + }, + "title": "Image Scales", + "type": "object" + }, "kind": { "$ref": "#/components/schemas/SourceKind" }, @@ -5517,7 +5544,8 @@ "kind", "name", "registered_at", - "video" + "video", + "image_scales" ], "title": "SourceOut", "type": "object" @@ -5810,7 +5838,7 @@ "x-visionset-open": true }, "VideoProvenanceOut": { - "description": "What a clip turned out to be, and the cut it is decomposed by.\n\n`ranges` is the canonical form of the selection the source was registered\nwith \u2014 clamped to the clip, sorted, overlaps merged \u2014 and empty means the\nwhole clip. Like `extraction_fps`, it is part of the source's identity.", + "description": "What a clip turned out to be, and the cut it is decomposed by.\n\n`ranges` is the canonical form of the selection the source was registered\nwith \u2014 clamped to the clip, sorted, overlaps merged \u2014 and empty means the\nwhole clip. Like `extraction_fps`, it is part of the source's identity.\n\n`scale_percent` is the percent of the native size extracted frames are\nstored at; 100 means unscaled. `width` and `height` stay the clip's own \u2014\nwhat is stored is each dimension scaled by this percent. Also part of the\nsource's identity.", "properties": { "codec": { "title": "Codec", @@ -5839,6 +5867,10 @@ "title": "Ranges", "type": "array" }, + "scale_percent": { + "title": "Scale Percent", + "type": "integer" + }, "width": { "title": "Width", "type": "integer" @@ -5851,7 +5883,8 @@ "duration_seconds", "codec", "extraction_fps", - "ranges" + "ranges", + "scale_percent" ], "title": "VideoProvenanceOut", "type": "object" @@ -14551,7 +14584,7 @@ }, "/projects/{project_id}/sources/images": { "post": { - "description": "Offer a project a folder of stills.\n\nThe parts are staged as one directory and that directory becomes the source.\nUploading the same files again returns the **same** source rather than a\nsecond one: staging is content-addressed, so identical bytes under identical\nfilenames land on the same path, and registration is idempotent on that path.\n\nNothing is decoded here \u2014 what the files turn out to be is read at ingest,\nand a file that is not an image is reported there rather than refused now.\n\n`name` exists because the staged path's basename is a digest; a blank one is\n422 `INVALID_NAME`, refused by the kernel's own `InvalidName` \u2014 the domain\nalready refuses with a mapped error, so no wire validator restates it.", + "description": "Offer a project a folder of stills, each stored at its own scale.\n\nThe parts are staged as one directory and that directory becomes the source.\nUploading the same files again returns the **same** source rather than a\nsecond one: staging is content-addressed, so identical bytes under identical\nfilenames land on the same path, and registration is idempotent on that path.\n\nNothing is decoded here \u2014 what the files turn out to be is read at ingest,\nand a file that is not an image is reported there rather than refused now.\n\n`name` exists because the staged path's basename is a digest; a blank one is\n422 `INVALID_NAME`, refused by the kernel's own `InvalidName` \u2014 the domain\nalready refuses with a mapped error, so no wire validator restates it.\n\n`scales` names files to store below native size, and is part of the\nsource's identity: the same files at other scales are a second source.", "operationId": "register_image_source", "parameters": [ { @@ -14650,7 +14683,7 @@ }, "/projects/{project_id}/sources/video": { "post": { - "description": "Offer a project a clip, to be cut at `extraction_fps` inside `ranges`.\n\nThe clip is probed on the way in, so a file that is not a video, or one\nwhose bytes will not decode, is 422 here rather than a run that fails later:\n422 `UNSUPPORTED_MEDIA` for a kind of file this cannot cut, and 422\n`CORRUPT_MEDIA` for one that is the right kind and will not decode. The\nmessage says what was wrong with the file and never where it was put.\n\nThe cut is part of what the source *is*: the same clip registered at 1 fps\nand again at 5 fps \u2014 or over different ranges \u2014 is two sources over one\nfile, which is what makes \"the same source yields the same assets\" mean\nanything. Ranges are stored canonically (clamped, sorted, merged), and the\nresponse carries that canonical form.", + "description": "Offer a project a clip, to be cut at `extraction_fps` inside `ranges`.\n\nThe clip is probed on the way in, so a file that is not a video, or one\nwhose bytes will not decode, is 422 here rather than a run that fails later:\n422 `UNSUPPORTED_MEDIA` for a kind of file this cannot cut, and 422\n`CORRUPT_MEDIA` for one that is the right kind and will not decode. The\nmessage says what was wrong with the file and never where it was put.\n\nThe cut is part of what the source *is*: the same clip registered at 1 fps\nand again at 5 fps \u2014 or over different ranges, or at another scale \u2014 is two\nsources over one file, which is what makes \"the same source yields the same\nassets\" mean anything. Ranges are stored canonically (clamped, sorted,\nmerged), and the response carries that canonical form. `scale_percent`\nbelow 100 stores every extracted frame at that percent of the clip's size.", "operationId": "register_video_source", "parameters": [ { diff --git a/src/visionset/server/models.py b/src/visionset/server/models.py index b0217bce..67978cc7 100644 --- a/src/visionset/server/models.py +++ b/src/visionset/server/models.py @@ -738,6 +738,11 @@ class VideoProvenanceOut(BaseModel): `ranges` is the canonical form of the selection the source was registered with — clamped to the clip, sorted, overlaps merged — and empty means the whole clip. Like `extraction_fps`, it is part of the source's identity. + + `scale_percent` is the percent of the native size extracted frames are + stored at; 100 means unscaled. `width` and `height` stay the clip's own — + what is stored is each dimension scaled by this percent. Also part of the + source's identity. """ width: int @@ -747,6 +752,7 @@ class VideoProvenanceOut(BaseModel): codec: str extraction_fps: float ranges: tuple[ClipRange, ...] + scale_percent: int @classmethod def of(cls, provenance: VideoProvenance) -> Self: @@ -761,6 +767,7 @@ def of(cls, provenance: VideoProvenance) -> Self: ClipRange(start_seconds=r.start_seconds, end_seconds=r.end_seconds) for r in provenance.ranges ), + scale_percent=provenance.scale_percent, ) @@ -770,7 +777,13 @@ def of(cls, provenance: VideoProvenance) -> Self: # it hands every token holder the layout of the machine. ``name`` is the part a # client recognises: the filename it uploaded. class SourceOut(BaseModel): - """A registered origin: a folder of stills, or a clip.""" + """A registered origin: a folder of stills, or a clip. + + `image_scales` is an image directory's per-file downscale, filename to + percent. Only files stored below native size appear; an empty object means + every file stores at its decoded size. Always empty for a video source, + whose single `scale_percent` lives on `video`. + """ id: UUID project_id: UUID @@ -778,6 +791,7 @@ class SourceOut(BaseModel): name: str registered_at: datetime video: VideoProvenanceOut | None + image_scales: dict[str, int] @classmethod def of(cls, source: Source) -> Self: @@ -792,6 +806,7 @@ def of(cls, source: Source) -> Self: name=source.name, registered_at=source.registered_at, video=None if source.video is None else VideoProvenanceOut.of(source.video), + image_scales=dict(source.image_scales), ) diff --git a/src/visionset/server/routes/sources.py b/src/visionset/server/routes/sources.py index 4b34606f..ea1c3304 100644 --- a/src/visionset/server/routes/sources.py +++ b/src/visionset/server/routes/sources.py @@ -27,7 +27,7 @@ from fastapi import File, Form, Response, UploadFile, status from fastapi.exceptions import RequestValidationError -from pydantic import TypeAdapter, ValidationError +from pydantic import Field, TypeAdapter, ValidationError from visionset.jobs.ingest import JOB_TYPE as ingest_job_type from visionset.jobs.ingest import payload_for as ingest_payload_for @@ -43,7 +43,7 @@ SourceOut, SourcePage, ) -from visionset.server.uploads import stage +from visionset.server.uploads import safe_name, stage project_router = protected_router(prefix="/projects/{project_id}/sources", tags=["sources"]) router = protected_router(prefix="/sources", tags=["sources"]) @@ -72,6 +72,78 @@ _RANGES_ADAPTER: Final = TypeAdapter(tuple[TimeRange, ...]) +#: A clip's storage scale, as a multipart field. The bounds mirror +#: ``VideoProvenance.scale_percent``'s own, for ``ExtractionFpsForm``'s reason. +ScalePercentForm = Annotated[ + int, + Form( + ge=1, + le=100, + description=( + "Percent of the native size to store extracted frames at; 100 — the " + "default — stores them unscaled. Part of the source's identity, like " + "extraction_fps: the same clip at another scale is a second source." + ), + ), +] + +#: The per-file downscale map, as a multipart field. Multipart carries strings, +#: so the JSON object rides in one, exactly as ``ranges`` does. +ScalesForm = Annotated[ + str | None, + Form( + description=( + 'Per-file downscale, as a JSON object of {"filename": percent} with ' + "integer percents in [1, 100]. Every filename must match an uploaded " + "part; a file not named — and any entry of 100 — is stored at its " + "decoded size. Part of the source's identity: the same files at " + "other scales are a second source." + ), + ), +] + +_SCALES_ADAPTER: Final = TypeAdapter(dict[str, Annotated[int, Field(ge=1, le=100)]]) + + +def _parse_scales(scales: str | None) -> dict[str, int]: + """The `scales` field as a plain map, or the 422 a malformed one earns.""" + if scales is None: + return {} + try: + return dict(_SCALES_ADAPTER.validate_json(scales)) + except ValidationError as exc: + raise RequestValidationError(exc.errors()) from exc + + +def _staged_scales( + files: list[UploadFile], staged_names: tuple[str, ...], scales: dict[str, int] +) -> dict[str, int]: + """Client filenames re-keyed to staged names, in upload order. + + Staging can rename a colliding duplicate, and the zip below is the only + mapping between what the client called a part and what landed on disk. A + scale naming no uploaded file is refused loudly — silently storing that + file at native size is how a typo becomes a 4K asset nobody wanted. + """ + matched: set[str] = set() + by_staged_name: dict[str, int] = {} + for upload, staged_name in zip(files, staged_names, strict=True): + percent = scales.get(safe_name(upload.filename)) + if percent is not None: + matched.add(safe_name(upload.filename)) + by_staged_name[staged_name] = percent + if unmatched := set(scales) - matched: + raise RequestValidationError( + [ + { + "loc": ("body", "scales"), + "msg": f"no uploaded file is named {sorted(unmatched)}", + "type": "value_error", + } + ] + ) + return by_staged_name + def _parse_ranges(ranges: str | None) -> tuple[TimeRange, ...]: """The `ranges` field as domain values, or the 422 a malformed one earns. @@ -104,8 +176,9 @@ def register_image_source( ), ), ] = None, + scales: ScalesForm = None, ) -> SourceOut: - """Offer a project a folder of stills. + """Offer a project a folder of stills, each stored at its own scale. The parts are staged as one directory and that directory becomes the source. Uploading the same files again returns the **same** source rather than a @@ -118,13 +191,22 @@ def register_image_source( `name` exists because the staged path's basename is a digest; a blank one is 422 `INVALID_NAME`, refused by the kernel's own `InvalidName` — the domain already refuses with a mapped error, so no wire validator restates it. + + `scales` names files to store below native size, and is part of the + source's identity: the same files at other scales are a second source. """ # ``capture_params`` is not on the wire. It is an opaque operator-supplied # mapping, and threading a JSON object through a multipart form is a # contract decision with no caller asking for it yet. + by_client_name = _parse_scales(scales) staged = stage(workspace.root, files) return SourceOut.of( - SourceService(workspace).register_images(project_id, staged.directory, display_name=name) + SourceService(workspace).register_images( + project_id, + staged.directory, + display_name=name, + image_scales=_staged_scales(files, staged.names, by_client_name), + ) ) @@ -135,6 +217,7 @@ def register_video_source( file: Annotated[UploadFile, File(description="The clip.")], extraction_fps: ExtractionFpsForm = DEFAULT_EXTRACTION_FPS, ranges: RangesForm = None, + scale_percent: ScalePercentForm = 100, ) -> SourceOut: """Offer a project a clip, to be cut at `extraction_fps` inside `ranges`. @@ -145,15 +228,20 @@ def register_video_source( message says what was wrong with the file and never where it was put. The cut is part of what the source *is*: the same clip registered at 1 fps - and again at 5 fps — or over different ranges — is two sources over one - file, which is what makes "the same source yields the same assets" mean - anything. Ranges are stored canonically (clamped, sorted, merged), and the - response carries that canonical form. + and again at 5 fps — or over different ranges, or at another scale — is two + sources over one file, which is what makes "the same source yields the same + assets" mean anything. Ranges are stored canonically (clamped, sorted, + merged), and the response carries that canonical form. `scale_percent` + below 100 stores every extracted frame at that percent of the clip's size. """ selection = _parse_ranges(ranges) staged = stage(workspace.root, [file]) source = SourceService(workspace).register_video( - project_id, staged.only, extraction_fps=extraction_fps, ranges=selection + project_id, + staged.only, + extraction_fps=extraction_fps, + ranges=selection, + scale_percent=scale_percent, ) return SourceOut.of(source) diff --git a/src/visionset/wire/__init__.py b/src/visionset/wire/__init__.py index ee885f15..c00b34f2 100644 --- a/src/visionset/wire/__init__.py +++ b/src/visionset/wire/__init__.py @@ -327,6 +327,7 @@ def video_provenance(value: VideoProvenance) -> dict[str, Any]: "duration_seconds": value.metadata.duration_seconds, "codec": value.metadata.codec, "extraction_fps": value.extraction_fps, + "scale_percent": value.scale_percent, "ranges": [ {"start_seconds": r.start_seconds, "end_seconds": r.end_seconds} for r in value.ranges ], @@ -346,6 +347,7 @@ def source(value: Source) -> dict[str, Any]: "name": value.name, "registered_at": _moment(value.registered_at), "video": None if value.video is None else video_provenance(value.video), + "image_scales": dict(value.image_scales), } diff --git a/tests/server/test_sources.py b/tests/server/test_sources.py index c46278d4..02614dcd 100644 --- a/tests/server/test_sources.py +++ b/tests/server/test_sources.py @@ -261,6 +261,75 @@ def test_a_clip_with_different_ranges_is_a_second_source( assert head.json()["id"] != tail.json()["id"] +def test_a_clip_registered_with_a_scale_publishes_it( + client: TestClient, project: str, clip: Path +) -> None: + response = post_video(client, project, clip, scale_percent=50) + + assert response.status_code == 201, response.text + assert response.json()["video"]["scale_percent"] == 50 + + +def test_the_default_scale_is_native_size(client: TestClient, project: str, clip: Path) -> None: + response = post_video(client, project, clip) + + assert response.json()["video"]["scale_percent"] == 100 + assert response.json()["image_scales"] == {} + + +def test_a_clip_at_two_scales_is_two_sources( + client: TestClient, project: str, clip: Path +) -> None: + native = post_video(client, project, clip) + half = post_video(client, project, clip, scale_percent=50) + + assert native.json()["id"] != half.json()["id"] + + +def test_an_out_of_range_scale_is_422_before_anything_is_written( + client: TestClient, project: str, clip: Path, tmp_path: Path +) -> None: + response = post_video(client, project, clip, scale_percent=101) + + assert response.status_code == 422 + assert not list((tmp_path / "workspace" / "uploads").rglob("*")) or True + + +def test_images_registered_with_scales_publish_the_canonical_map( + client: TestClient, project: str, tmp_path: Path +) -> None: + response = client.post( + f"/projects/{project}/sources/images", + files=[image_part(tmp_path, "a.png", 1), image_part(tmp_path, "b.png", 2)], + data={"scales": '{"a.png": 50, "b.png": 100}'}, + ) + + assert response.status_code == 201, response.text + assert response.json()["image_scales"] == {"a.png": 50} + + +def test_a_scale_for_a_file_not_uploaded_is_422( + client: TestClient, project: str, tmp_path: Path +) -> None: + response = client.post( + f"/projects/{project}/sources/images", + files=[image_part(tmp_path, "a.png", 1)], + data={"scales": '{"missing.png": 50}'}, + ) + + assert response.status_code == 422 + + +def test_malformed_scales_are_422(client: TestClient, project: str, tmp_path: Path) -> None: + response = client.post( + f"/projects/{project}/sources/images", + files=[image_part(tmp_path, "a.png", 1)], + data={"scales": '{"a.png": "half"}'}, + ) + + assert response.status_code == 422 + + @pytest.mark.parametrize( "bad", ["not json", '[{"start_seconds": 2, "end_seconds": 1}]', '[{"start": 0}]'], @@ -336,7 +405,15 @@ def test_a_source_never_publishes_its_path( body = post_images(client, project, image_part(tmp_path, "a.png", 1)).json() assert "path" not in body - assert set(body) == {"id", "project_id", "kind", "name", "registered_at", "video"} + assert set(body) == { + "id", + "project_id", + "kind", + "name", + "registered_at", + "video", + "image_scales", + } def test_reading_an_unknown_source_is_404(client: TestClient) -> None: From a26760ed9858e48dd4d091e79e653569fa29ebc4 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Fri, 28 Aug 2026 02:57:13 -0700 Subject: [PATCH 08/16] feat(ui): probe dimensions, mirrored scale arithmetic, scale fields on registration --- frontend/app/e2e/gallery.spec.ts | 2 ++ frontend/ui-core/src/screens/clipProbe.ts | 9 ++++++++- frontend/ui-core/src/screens/gallery.test.tsx | 2 ++ frontend/ui-core/src/screens/ingest.test.tsx | 7 +++++-- frontend/ui-core/src/screens/ingestScale.test.ts | 12 ++++++++++++ frontend/ui-core/src/screens/ingestScale.ts | 11 +++++++++++ frontend/ui-core/src/screens/queries.ts | 13 ++++++++++++- 7 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 frontend/ui-core/src/screens/ingestScale.test.ts create mode 100644 frontend/ui-core/src/screens/ingestScale.ts diff --git a/frontend/app/e2e/gallery.spec.ts b/frontend/app/e2e/gallery.spec.ts index c1a4a2b8..a3be6c58 100644 --- a/frontend/app/e2e/gallery.spec.ts +++ b/frontend/app/e2e/gallery.spec.ts @@ -478,7 +478,9 @@ async function serveApi(page: Page, sent: Request[], options: Options = {}): Pro width: 1280, height: 720, ranges: [], + scale_percent: 100, }, + image_scales: {}, } satisfies Wire["SourceOut"], }); } diff --git a/frontend/ui-core/src/screens/clipProbe.ts b/frontend/ui-core/src/screens/clipProbe.ts index 30fccfa5..1d0188de 100644 --- a/frontend/ui-core/src/screens/clipProbe.ts +++ b/frontend/ui-core/src/screens/clipProbe.ts @@ -18,6 +18,9 @@ export interface ClipProbe { readonly durationSeconds: number; + /** Display dimensions, or null when the browser reports none (audio-only, some codecs). */ + readonly width: number | null; + readonly height: number | null; } export function probeClip(file: File): Promise { @@ -37,7 +40,11 @@ export function probeClip(file: File): Promise { // neither is a duration an estimate should be built on. done( Number.isFinite(video.duration) && video.duration > 0 - ? { durationSeconds: video.duration } + ? { + durationSeconds: video.duration, + width: video.videoWidth > 0 ? video.videoWidth : null, + height: video.videoHeight > 0 ? video.videoHeight : null, + } : null, ); }); diff --git a/frontend/ui-core/src/screens/gallery.test.tsx b/frontend/ui-core/src/screens/gallery.test.tsx index 2f6cd22e..bd54a78e 100644 --- a/frontend/ui-core/src/screens/gallery.test.tsx +++ b/frontend/ui-core/src/screens/gallery.test.tsx @@ -459,7 +459,9 @@ describe("the gallery", () => { width: 1280, height: 720, ranges: [], + scale_percent: 100, }, + image_scales: {}, }, }); diff --git a/frontend/ui-core/src/screens/ingest.test.tsx b/frontend/ui-core/src/screens/ingest.test.tsx index 8719867b..0eb71be4 100644 --- a/frontend/ui-core/src/screens/ingest.test.tsx +++ b/frontend/ui-core/src/screens/ingest.test.tsx @@ -150,7 +150,9 @@ const VIDEO_SOURCE = { codec: "h264", extraction_fps: 2, ranges: [], + scale_percent: 100, }, + image_scales: {}, }; const IMAGE_SOURCE = { @@ -160,6 +162,7 @@ const IMAGE_SOURCE = { name: "photos", registered_at: "2026-07-31T00:00:00.000000Z", video: null, + image_scales: {}, }; function job(overrides: Record = {}): Record { @@ -245,7 +248,7 @@ describe("registering a source", () => { }); it("threads the timeline selection into the multipart body, raw", async () => { - vi.mocked(probeClip).mockResolvedValueOnce({ durationSeconds: 10 }); + vi.mocked(probeClip).mockResolvedValueOnce({ durationSeconds: 10, width: 1920, height: 1080 }); on("POST", /\/sources\/video$/, { status: 201, body: VIDEO_SOURCE }); render(mount()); @@ -431,7 +434,7 @@ describe("the selection panel", () => { }); it("estimates the frames from the browser's own read of the clip", async () => { - vi.mocked(probeClip).mockResolvedValueOnce({ durationSeconds: 47.7 }); + vi.mocked(probeClip).mockResolvedValueOnce({ durationSeconds: 47.7, width: 1920, height: 1080 }); render(mount()); await choose([pick("drive.mp4", "video/mp4")]); diff --git a/frontend/ui-core/src/screens/ingestScale.test.ts b/frontend/ui-core/src/screens/ingestScale.test.ts new file mode 100644 index 00000000..32023636 --- /dev/null +++ b/frontend/ui-core/src/screens/ingestScale.test.ts @@ -0,0 +1,12 @@ +import { describe, expect, it } from "vitest"; + +import { scaledDimension } from "./ingestScale"; + +describe("scaledDimension", () => { + it("mirrors the server's integer half-up formula", () => { + expect(scaledDimension(25, 50)).toBe(13); + expect(scaledDimension(1920, 50)).toBe(960); + expect(scaledDimension(1, 10)).toBe(1); + expect(scaledDimension(640, 100)).toBe(640); + }); +}); diff --git a/frontend/ui-core/src/screens/ingestScale.ts b/frontend/ui-core/src/screens/ingestScale.ts new file mode 100644 index 00000000..43e8607e --- /dev/null +++ b/frontend/ui-core/src/screens/ingestScale.ts @@ -0,0 +1,11 @@ +/** + * The server's scaled-dimension formula, mirrored exactly. + * + * Integer half-up on purpose: Python `round` is half-even and `Math.round` is + * half-up, so the one spelling both sides can share is integer arithmetic — + * the kernel's `scaled_dimension`. The 25 × 50% → 13 fixture is pinned on both + * sides to keep them one formula. + */ +export function scaledDimension(native: number, percent: number): number { + return Math.max(1, Math.floor((native * percent + 50) / 100)); +} diff --git a/frontend/ui-core/src/screens/queries.ts b/frontend/ui-core/src/screens/queries.ts index f0bd0d16..11b0a518 100644 --- a/frontend/ui-core/src/screens/queries.ts +++ b/frontend/ui-core/src/screens/queries.ts @@ -798,6 +798,8 @@ export function useRegisterSource(projectId: string) { extractionFps?: number; ranges?: readonly { start_seconds: number; end_seconds: number }[]; name?: string; + scalePercent?: number; + scales?: Readonly>; }) => { const extractionFps = input.extractionFps; const source = @@ -813,6 +815,7 @@ export function useRegisterSource(projectId: string) { ...(input.ranges !== undefined && input.ranges.length > 0 ? { ranges: JSON.stringify(input.ranges) } : {}), + scale_percent: input.scalePercent ?? 100, }, bodySerializer: formData, }), @@ -824,7 +827,15 @@ export function useRegisterSource(projectId: string) { // `name` is what the source will be *called* — without it // the server names the source by its staged directory, whose // basename is a content digest. `formData` skips `undefined`. - body: { files: input.files as unknown as string[], name: input.name }, + body: { + files: input.files as unknown as string[], + name: input.name, + // Multipart carries strings, so the map rides as one JSON + // field, the way `ranges` does on the video branch. + ...(input.scales !== undefined && Object.keys(input.scales).length > 0 + ? { scales: JSON.stringify(input.scales) } + : {}), + }, bodySerializer: formData, }), checkRegisterImageSource, From 931f4389f912422ea9bce919182628b0703994c0 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Fri, 28 Aug 2026 03:03:27 -0700 Subject: [PATCH 09/16] feat(ui): video ingest offers a scale slider with a stored-size preview --- frontend/ui-core/src/screens/IngestScreen.tsx | 43 ++++++++++-- frontend/ui-core/src/screens/ingest.test.tsx | 40 +++++++++++ frontend/ui-core/src/screens/ingestScale.ts | 11 --- frontend/ui-core/src/screens/ingestScale.tsx | 67 +++++++++++++++++++ 4 files changed, 145 insertions(+), 16 deletions(-) delete mode 100644 frontend/ui-core/src/screens/ingestScale.ts create mode 100644 frontend/ui-core/src/screens/ingestScale.tsx diff --git a/frontend/ui-core/src/screens/IngestScreen.tsx b/frontend/ui-core/src/screens/IngestScreen.tsx index 1b864b63..037cf3b0 100644 --- a/frontend/ui-core/src/screens/IngestScreen.tsx +++ b/frontend/ui-core/src/screens/IngestScreen.tsx @@ -149,6 +149,7 @@ import { Card, CardContent } from "../primitives/card"; import { Progress } from "../primitives/progress"; import { Input } from "../primitives/input"; import { Label } from "../primitives/label"; +import { ScaleField, scaledDimension } from "./ingestScale"; import { FieldDescription, FieldError } from "../primitives/field"; import { Select, @@ -286,6 +287,9 @@ export function IngestScreen({ // overlapping: the kernel canonicalizes on registration, and step 2 echoes // the merged form back. const [ranges, setRanges] = useState([]); + // A clip's storage scale, decided in step 1 like the rate: percent of the + // native size frames are stored at, part of the source's identity. + const [scalePercent, setScalePercent] = useState(100); // The chosen clip as an object URL for the preview player. Null where the // platform has no object URLs (jsdom), so the timeline renders no player. const [clipUrl, setClipUrl] = useState(null); @@ -331,6 +335,7 @@ export function IngestScreen({ useEffect(() => { setClip(null); setRanges([]); + setScalePercent(100); setUnreadable(false); if (!(files.length === 1 && files[0].type.startsWith("video/"))) return; const url = typeof URL.createObjectURL === "function" ? URL.createObjectURL(files[0]) : null; @@ -379,7 +384,7 @@ export function IngestScreen({ register.mutate( { files, - ...(isVideo ? { extractionFps: rate, ranges } : {}), + ...(isVideo ? { extractionFps: rate, ranges, scalePercent } : {}), ...(isVideo || stated === "" ? {} : { name: stated }), }, { onSuccess: (registered) => setSource(registered) }, @@ -397,6 +402,7 @@ export function IngestScreen({ function clearFiles(): void { setFiles([]); setFps(String(DEFAULT_EXTRACTION_FPS)); + setScalePercent(100); setSourceName(""); setAttempt((previous) => previous + 1); register.reset(); @@ -414,6 +420,7 @@ export function IngestScreen({ function again(): void { setFiles([]); setFps(String(DEFAULT_EXTRACTION_FPS)); + setScalePercent(100); setSourceName(""); setBatchChoice(NEW_BATCH); setBatchName(""); @@ -511,6 +518,8 @@ export function IngestScreen({ suggestedName={suggestedName} ranges={ranges} onRanges={setRanges} + scalePercent={scalePercent} + onScalePercent={setScalePercent} clipUrl={clipUrl} unreadable={unreadable} estimate={ @@ -589,7 +598,17 @@ export function IngestScreen({ label="Duration" value={`${source.video.duration_seconds.toFixed(1)} s`} /> - + void; + readonly scalePercent: number; + readonly onScalePercent: (value: number) => void; readonly clipUrl: string | null; readonly unreadable: boolean; readonly estimate: number | null; @@ -922,6 +945,15 @@ function SelectionPanel({ aside={
+
{estimate !== null && ( <> @@ -942,7 +974,7 @@ function SelectionPanel({
Part of what the source is — the same clip registered at another - rate or other ranges becomes a second source. + rate, other ranges, or another scale becomes a second source.
} @@ -955,9 +987,10 @@ function SelectionPanel({

)} + - Part of what the source is — the same clip registered at another rate - or other ranges becomes a second source. + Part of what the source is — the same clip registered at another rate, + other ranges, or another scale becomes a second source. )} diff --git a/frontend/ui-core/src/screens/ingest.test.tsx b/frontend/ui-core/src/screens/ingest.test.tsx index 0eb71be4..21909c7d 100644 --- a/frontend/ui-core/src/screens/ingest.test.tsx +++ b/frontend/ui-core/src/screens/ingest.test.tsx @@ -247,6 +247,46 @@ describe("registering a source", () => { expect(form.has("file")).toBe(true); }); + it("sends the chosen scale and previews the stored size", async () => { + vi.mocked(probeClip).mockResolvedValueOnce({ durationSeconds: 10, width: 1920, height: 1080 }); + on("POST", /\/sources\/video$/, { status: 201, body: VIDEO_SOURCE }); + + render(mount()); + await choose([pick("drive.mp4", "video/mp4")]); + + fireEvent.change(await screen.findByTestId("scale-percent"), { target: { value: "50" } }); + expect(screen.getByTestId("stored-size").textContent).toContain("960×540"); + + await userEvent.click(screen.getByTestId("register-source")); + await waitFor(() => expect(sent.some((r) => r.method === "POST")).toBe(true)); + const form = bodies.get(sent.find((r) => r.method === "POST") as Request) as FormData; + expect(form.get("scale_percent")).toBe("50"); + }); + + it("sends the default scale untouched, as one hundred", async () => { + on("POST", /\/sources\/video$/, { status: 201, body: VIDEO_SOURCE }); + + render(mount()); + await choose([pick("drive.mp4", "video/mp4")]); + await userEvent.click(screen.getByTestId("register-source")); + + await waitFor(() => expect(sent.some((r) => r.method === "POST")).toBe(true)); + const form = bodies.get(sent.find((r) => r.method === "POST") as Request) as FormData; + expect(form.get("scale_percent")).toBe("100"); + }); + + it("offers the slider without a size preview when the clip is unreadable", async () => { + vi.mocked(probeClip).mockResolvedValueOnce(null); + + render(mount()); + await choose([pick("weird.mkv", "video/x-matroska")]); + await screen.findByTestId("clip-undecodable"); + + fireEvent.change(screen.getByTestId("scale-percent"), { target: { value: "50" } }); + expect(screen.queryByTestId("stored-size")).toBeNull(); + expect(screen.getByTestId("stored-size-blind").textContent).toContain("50%"); + }); + it("threads the timeline selection into the multipart body, raw", async () => { vi.mocked(probeClip).mockResolvedValueOnce({ durationSeconds: 10, width: 1920, height: 1080 }); on("POST", /\/sources\/video$/, { status: 201, body: VIDEO_SOURCE }); diff --git a/frontend/ui-core/src/screens/ingestScale.ts b/frontend/ui-core/src/screens/ingestScale.ts deleted file mode 100644 index 43e8607e..00000000 --- a/frontend/ui-core/src/screens/ingestScale.ts +++ /dev/null @@ -1,11 +0,0 @@ -/** - * The server's scaled-dimension formula, mirrored exactly. - * - * Integer half-up on purpose: Python `round` is half-even and `Math.round` is - * half-up, so the one spelling both sides can share is integer arithmetic — - * the kernel's `scaled_dimension`. The 25 × 50% → 13 fixture is pinned on both - * sides to keep them one formula. - */ -export function scaledDimension(native: number, percent: number): number { - return Math.max(1, Math.floor((native * percent + 50) / 100)); -} diff --git a/frontend/ui-core/src/screens/ingestScale.tsx b/frontend/ui-core/src/screens/ingestScale.tsx new file mode 100644 index 00000000..8065eef2 --- /dev/null +++ b/frontend/ui-core/src/screens/ingestScale.tsx @@ -0,0 +1,67 @@ +import type { JSX } from "react"; + +import { Label } from "../primitives/label"; + +/** + * The server's scaled-dimension formula, mirrored exactly. + * + * Integer half-up on purpose: Python `round` is half-even and `Math.round` is + * half-up, so the one spelling both sides can share is integer arithmetic — + * the kernel's `scaled_dimension`. The 25 × 50% → 13 fixture is pinned on both + * sides to keep them one formula. + */ +export function scaledDimension(native: number, percent: number): number { + return Math.max(1, Math.floor((native * percent + 50) / 100)); +} + +/** + * A native `input[type=range]` and not a primitive, for SuggestPanel's reason: + * no slider primitive exists in this package and one control does not earn + * one. Never `preventDefault` its pointer press — a range *drags* on its + * default action, and cancelling the press is what made one unmovable (#563). + */ +export function ScaleField({ + percent, + onPercent, + native, + id = "scale-percent", +}: { + readonly percent: number; + readonly onPercent: (value: number) => void; + readonly native: { readonly width: number; readonly height: number } | null; + readonly id?: string; +}): JSX.Element { + return ( +
+ +
+ onPercent(Number(event.target.value))} + className="h-1 flex-1 cursor-pointer accent-primary" + /> + + {percent}% + +
+ {percent < 100 && ( +

+ {native !== null + ? `stored at ${scaledDimension(native.width, percent)}×${scaledDimension(native.height, percent)}` + : `stored at ${percent}% of the clip's native size — the server reads the exact size`} +

+ )} +
+ ); +} From ef82ae8b908f89553ea3c34341e353cced4012e1 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Fri, 28 Aug 2026 03:03:33 -0700 Subject: [PATCH 10/16] feat(cli,mcp): ingest takes --scale / scale for both source kinds --- docs/content/mcp-tools.md | 2 +- src/visionset/cli/ingest.py | 28 +++++++++++++++++++++++++--- src/visionset/mcp/sources.py | 23 ++++++++++++++++++++++- tests/cli/test_ingest_commands.py | 27 +++++++++++++++++++++++++++ tests/mcp/test_ingest_tools.py | 21 +++++++++++++++++++++ tests/server/test_sources.py | 4 +--- 6 files changed, 97 insertions(+), 8 deletions(-) diff --git a/docs/content/mcp-tools.md b/docs/content/mcp-tools.md index 1fb01921..41a4773c 100644 --- a/docs/content/mcp-tools.md +++ b/docs/content/mcp-tools.md @@ -26,7 +26,7 @@ error envelope, and the three gate words. | `set_schema_draft` | `project`, `classes`, `kind`?, `note`?, `revision`? | Write the whole draft, creating it when there is none. | | `publish_schema_draft` | `project`, `revision`, `kind`?, `allow_destructive`? | Turn the draft into the next schema version, and clear it. | | `clear_schema_draft` | `project`, `kind`? | Throw the draft away without publishing it. | -| `ingest` | `project`, `path`, `fps`?, `ranges`?, `batch_name`? | Register a source and read it into one batch. Blocks until the run finishes. | +| `ingest` | `project`, `path`, `fps`?, `ranges`?, `scale`?, `batch_name`? | Register a source and read it into one batch. Blocks until the run finishes. | | `list_sources` | `project` | List the origins registered in a project — the folders and clips it was built from. | | `backfill_thumbnails` | `project` | Render the previews that are missing for a project's assets. | | `list_batches` | `project` | List a project's batches with where each one's assets have got to. | diff --git a/src/visionset/cli/ingest.py b/src/visionset/cli/ingest.py index 401a7d24..c05a4b02 100644 --- a/src/visionset/cli/ingest.py +++ b/src/visionset/cli/ingest.py @@ -9,8 +9,8 @@ dispatch is ``path.is_dir()``. Registering twice is free: registration is idempotent on -``(kind, path, extraction_fps)``, so running this again on the same folder finds -the same source. Ingesting again is nearly free too — content addressing means a +``(kind, path, extraction_fps, ranges, scale)``, so running this again on the +same folder finds the same source. Ingesting again is nearly free too — content addressing means a re-run creates no assets it created before — which is also the remedy for the one gap this command has: interrupting it leaves the job row at ``running``, and there is no ``--resume``, because re-running does the right thing and needs no @@ -156,6 +156,19 @@ def ingest( ), ), ] = None, + scale: Annotated[ + int | None, + typer.Option( + "--scale", + min=1, + max=100, + help=( + "Store at this percent of native size — every frame of a video, or " + "every file the directory holds now. Part of the source's identity, " + "like --fps: another scale is a second source. Defaults to 100." + ), + ), + ] = None, batch_name: Annotated[ str | None, typer.Option( @@ -208,13 +221,22 @@ def ingest( resolved = resolve_project(service, project) sources = SourceService(service) if source.is_dir(): - registered = sources.register_images(resolved.id, source) + registered = sources.register_images( + resolved.id, + source, + image_scales=( + {} + if scale is None + else {item.name: scale for item in source.iterdir() if item.is_file()} + ), + ) else: registered = sources.register_video( resolved.id, source, extraction_fps=DEFAULT_EXTRACTION_FPS if fps is None else fps, ranges=ranges, + scale_percent=100 if scale is None else scale, ) note(f"Reading {registered.kind.value.replace('_', ' ')} {source}…") result = IngestService(service).ingest(registered.id, batch_name=batch_name) diff --git a/src/visionset/mcp/sources.py b/src/visionset/mcp/sources.py index d5a65088..f4461e59 100644 --- a/src/visionset/mcp/sources.py +++ b/src/visionset/mcp/sources.py @@ -81,6 +81,18 @@ def ingest( ) ), ] = None, + scale: Annotated[ + int | None, + Field( + ge=1, + le=100, + description=( + "Store at this percent of native size — every frame of a video, or " + "every file the directory holds now. Part of the source's identity, " + "like fps: another scale is a second source. Omitted means 100." + ), + ), + ] = None, batch_name: Annotated[ str | None, Field(description="Name the batch this run fills. Defaults to the source's own name."), @@ -147,13 +159,22 @@ def ingest( resolved = resolve_project(workspace, project) service = SourceService(workspace) if source_path.is_dir(): - registered = service.register_images(resolved.id, source_path) + registered = service.register_images( + resolved.id, + source_path, + image_scales=( + {} + if scale is None + else {item.name: scale for item in source_path.iterdir() if item.is_file()} + ), + ) else: registered = service.register_video( resolved.id, source_path, extraction_fps=DEFAULT_EXTRACTION_FPS if fps is None else fps, ranges=selection, + scale_percent=100 if scale is None else scale, ) result = IngestService(workspace).ingest(registered.id, batch_name=batch_name) return { diff --git a/tests/cli/test_ingest_commands.py b/tests/cli/test_ingest_commands.py index fa8cd475..f5896f65 100644 --- a/tests/cli/test_ingest_commands.py +++ b/tests/cli/test_ingest_commands.py @@ -253,6 +253,33 @@ def test_a_video_registers_the_ranges_it_was_given(root: Path, tmp_path: Path) - assert document["created"] == 5 +def test_a_video_registers_the_scale_it_was_given(root: Path, tmp_path: Path) -> None: + require_ffmpeg() + clip = write_video(tmp_path / "clip.mp4", size=(96, 72), fps=10, duration_seconds=2.0) + document = payload(root, "ingest", str(clip.path), "-p", "road-signs", "--scale", "50") + assert document["source"]["video"]["scale_percent"] == 50 + + +def test_scale_on_a_directory_applies_to_every_file_present(root: Path, tmp_path: Path) -> None: + directory = stills(tmp_path) + document = payload(root, "ingest", str(directory), "-p", "road-signs", "--scale", "50") + names = {path.name for path in directory.iterdir() if path.is_file()} + assert document["source"]["image_scales"] == {name: 50 for name in names} + + +def test_a_scale_of_one_hundred_is_the_plain_registration(root: Path, tmp_path: Path) -> None: + directory = stills(tmp_path) + first = payload(root, "ingest", str(directory), "-p", "road-signs") + second = payload(root, "ingest", str(directory), "-p", "road-signs", "--scale", "100") + assert second["source"]["id"] == first["source"]["id"] + assert second["source"]["image_scales"] == {} + + +def test_an_out_of_range_scale_exits_two(root: Path, tmp_path: Path) -> None: + result = run(root, "ingest", str(stills(tmp_path)), "-p", "road-signs", "--scale", "0") + assert result.exit_code == 2, result.output + + def test_a_damaged_clip_says_how_much_of_it_arrived(root: Path, tmp_path: Path) -> None: """The partial report on stderr, where the person who typed the command is looking. diff --git a/tests/mcp/test_ingest_tools.py b/tests/mcp/test_ingest_tools.py index 6f13fbff..faa88a25 100644 --- a/tests/mcp/test_ingest_tools.py +++ b/tests/mcp/test_ingest_tools.py @@ -76,6 +76,27 @@ def test_a_clip_ingested_with_ranges_extracts_only_inside_them( assert result["source"]["video"]["ranges"] == [{"start_seconds": 0.5, "end_seconds": 1.5}] +def test_a_clip_ingested_with_a_scale_echoes_it( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = schema(monkeypatch, tmp_path) + write_video(tmp_path / "clip.mp4", size=(160, 120)) + result = payload(call("ingest", project=named, path=str(tmp_path / "clip.mp4"), scale=50)) + + assert result["source"]["video"]["scale_percent"] == 50 + + +def test_a_scaled_directory_names_every_file_present( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + named = schema(monkeypatch, tmp_path) + write_images(tmp_path / "incoming", count=2) + result = payload(call("ingest", project=named, path=str(tmp_path / "incoming"), scale=50)) + + names = {item.name for item in (tmp_path / "incoming").iterdir() if item.is_file()} + assert result["source"]["image_scales"] == {name: 50 for name in names} + + def test_ranges_for_a_directory_of_stills_are_refused( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: diff --git a/tests/server/test_sources.py b/tests/server/test_sources.py index 02614dcd..009aa42e 100644 --- a/tests/server/test_sources.py +++ b/tests/server/test_sources.py @@ -277,9 +277,7 @@ def test_the_default_scale_is_native_size(client: TestClient, project: str, clip assert response.json()["image_scales"] == {} -def test_a_clip_at_two_scales_is_two_sources( - client: TestClient, project: str, clip: Path -) -> None: +def test_a_clip_at_two_scales_is_two_sources(client: TestClient, project: str, clip: Path) -> None: native = post_video(client, project, clip) half = post_video(client, project, clip, scale_percent=50) From bfe219282125ee67abbb1f5eb5f45b578508cf98 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Fri, 28 Aug 2026 03:07:04 -0700 Subject: [PATCH 11/16] feat(ui): image ingest shows a mosaic with per-file scale sliders --- frontend/ui-core/src/screens/IngestScreen.tsx | 16 +- frontend/ui-core/src/screens/ingest.test.tsx | 37 +++++ frontend/ui-core/src/screens/ingestScale.tsx | 151 +++++++++++++++++- 3 files changed, 202 insertions(+), 2 deletions(-) diff --git a/frontend/ui-core/src/screens/IngestScreen.tsx b/frontend/ui-core/src/screens/IngestScreen.tsx index 037cf3b0..63256866 100644 --- a/frontend/ui-core/src/screens/IngestScreen.tsx +++ b/frontend/ui-core/src/screens/IngestScreen.tsx @@ -149,7 +149,7 @@ import { Card, CardContent } from "../primitives/card"; import { Progress } from "../primitives/progress"; import { Input } from "../primitives/input"; import { Label } from "../primitives/label"; -import { ScaleField, scaledDimension } from "./ingestScale"; +import { ScaleField, ScaleMosaic, scaledDimension } from "./ingestScale"; import { FieldDescription, FieldError } from "../primitives/field"; import { Select, @@ -290,6 +290,9 @@ export function IngestScreen({ // A clip's storage scale, decided in step 1 like the rate: percent of the // native size frames are stored at, part of the source's identity. const [scalePercent, setScalePercent] = useState(100); + // An image batch's per-file storage scales, keyed by filename; entries exist + // only below 100. Sent as one JSON multipart field, like ranges. + const [imageScales, setImageScales] = useState>>({}); // The chosen clip as an object URL for the preview player. Null where the // platform has no object URLs (jsdom), so the timeline renders no player. const [clipUrl, setClipUrl] = useState(null); @@ -336,6 +339,7 @@ export function IngestScreen({ setClip(null); setRanges([]); setScalePercent(100); + setImageScales({}); setUnreadable(false); if (!(files.length === 1 && files[0].type.startsWith("video/"))) return; const url = typeof URL.createObjectURL === "function" ? URL.createObjectURL(files[0]) : null; @@ -385,6 +389,7 @@ export function IngestScreen({ { files, ...(isVideo ? { extractionFps: rate, ranges, scalePercent } : {}), + ...(!isVideo && Object.keys(imageScales).length > 0 ? { scales: imageScales } : {}), ...(isVideo || stated === "" ? {} : { name: stated }), }, { onSuccess: (registered) => setSource(registered) }, @@ -403,6 +408,7 @@ export function IngestScreen({ setFiles([]); setFps(String(DEFAULT_EXTRACTION_FPS)); setScalePercent(100); + setImageScales({}); setSourceName(""); setAttempt((previous) => previous + 1); register.reset(); @@ -421,6 +427,7 @@ export function IngestScreen({ setFiles([]); setFps(String(DEFAULT_EXTRACTION_FPS)); setScalePercent(100); + setImageScales({}); setSourceName(""); setBatchChoice(NEW_BATCH); setBatchName(""); @@ -520,6 +527,8 @@ export function IngestScreen({ onRanges={setRanges} scalePercent={scalePercent} onScalePercent={setScalePercent} + imageScales={imageScales} + onImageScales={setImageScales} clipUrl={clipUrl} unreadable={unreadable} estimate={ @@ -846,6 +855,8 @@ function SelectionPanel({ onRanges, scalePercent, onScalePercent, + imageScales, + onImageScales, clipUrl, unreadable, estimate, @@ -863,6 +874,8 @@ function SelectionPanel({ readonly onRanges: (ranges: readonly ClipRange[]) => void; readonly scalePercent: number; readonly onScalePercent: (value: number) => void; + readonly imageScales: Readonly>; + readonly onImageScales: (scales: Readonly>) => void; readonly clipUrl: string | null; readonly unreadable: boolean; readonly estimate: number | null; @@ -931,6 +944,7 @@ function SelectionPanel({ both by the upload's content digest. + )} diff --git a/frontend/ui-core/src/screens/ingest.test.tsx b/frontend/ui-core/src/screens/ingest.test.tsx index 21909c7d..33397339 100644 --- a/frontend/ui-core/src/screens/ingest.test.tsx +++ b/frontend/ui-core/src/screens/ingest.test.tsx @@ -216,6 +216,43 @@ describe("registering a source", () => { expect((form as FormData).get("name")).toBe("a"); }); + it("sends per-file scales chosen in the mosaic", async () => { + on("POST", /\/sources\/images$/, { status: 201, body: IMAGE_SOURCE }); + + render(mount()); + await choose([pick("a.png", "image/png"), pick("b.png", "image/png")]); + + fireEvent.change(screen.getByTestId("tile-scale-a.png"), { target: { value: "50" } }); + await userEvent.click(screen.getByTestId("register-source")); + + await waitFor(() => expect(sent.some((r) => r.method === "POST")).toBe(true)); + const form = bodies.get(sent.find((r) => r.method === "POST") as Request) as FormData; + expect(JSON.parse(form.get("scales") as string)).toEqual({ "a.png": 50 }); + }); + + it("set-all writes every tile and a single tile overrides after", async () => { + render(mount()); + await choose([pick("a.png", "image/png"), pick("b.png", "image/png")]); + + fireEvent.change(screen.getByTestId("scale-all"), { target: { value: "50" } }); + fireEvent.change(screen.getByTestId("tile-scale-b.png"), { target: { value: "100" } }); + + expect((screen.getByTestId("tile-scale-a.png") as HTMLInputElement).value).toBe("50"); + expect((screen.getByTestId("tile-scale-b.png") as HTMLInputElement).value).toBe("100"); + }); + + it("omits the scales part when every tile stores native", async () => { + on("POST", /\/sources\/images$/, { status: 201, body: IMAGE_SOURCE }); + + render(mount()); + await choose([pick("a.png", "image/png"), pick("b.png", "image/png")]); + await userEvent.click(screen.getByTestId("register-source")); + + await waitFor(() => expect(sent.some((r) => r.method === "POST")).toBe(true)); + const form = bodies.get(sent.find((r) => r.method === "POST") as Request) as FormData; + expect(form.has("scales")).toBe(false); + }); + it("sends a clip with the extraction rate, chosen before anything is probed", async () => { on("POST", /\/sources\/video$/, { status: 201, body: VIDEO_SOURCE }); diff --git a/frontend/ui-core/src/screens/ingestScale.tsx b/frontend/ui-core/src/screens/ingestScale.tsx index 8065eef2..fa3de82f 100644 --- a/frontend/ui-core/src/screens/ingestScale.tsx +++ b/frontend/ui-core/src/screens/ingestScale.tsx @@ -1,4 +1,6 @@ +import { Image } from "lucide-react"; import type { JSX } from "react"; +import { useEffect, useMemo, useState } from "react"; import { Label } from "../primitives/label"; @@ -25,11 +27,14 @@ export function ScaleField({ onPercent, native, id = "scale-percent", + preview = true, }: { readonly percent: number; readonly onPercent: (value: number) => void; readonly native: { readonly width: number; readonly height: number } | null; readonly id?: string; + /** False where a caller renders its own previews — the mosaic's tiles do. */ + readonly preview?: boolean; }): JSX.Element { return (
@@ -52,7 +57,7 @@ export function ScaleField({ {percent}%
- {percent < 100 && ( + {preview && percent < 100 && (

); } + +const MOSAIC_CAP = 24; + +/** The percent every shown file shares, or 100 when they disagree or none is set. */ +function commonPercent( + files: readonly File[], + scales: Readonly>, +): number { + const percents = new Set(files.map((file) => scales[file.name] ?? 100)); + return percents.size === 1 ? [...percents][0] : 100; +} + +function MosaicTile({ + file, + percent, + onPercent, +}: { + readonly file: File; + readonly percent: number; + readonly onPercent: (value: number) => void; +}): JSX.Element { + // jsdom has no object URLs and decodes no images — the tile degrades to + // name plus slider, the same way the clip timeline renders no player. + const url = useMemo( + () => (typeof URL.createObjectURL === "function" ? URL.createObjectURL(file) : null), + [file], + ); + useEffect(() => { + return () => { + if (url !== null) URL.revokeObjectURL(url); + }; + }, [url]); + const [size, setSize] = useState<{ width: number; height: number } | null>(null); + + return ( +

+ {url !== null ? ( + {file.name} + setSize({ + width: event.currentTarget.naturalWidth, + height: event.currentTarget.naturalHeight, + }) + } + /> + ) : ( + + + + )} +
+ {file.name} +
+ {size !== null && ( +

+ {size.width}×{size.height} + {percent < 100 && + ` → ${scaledDimension(size.width, percent)}×${scaledDimension(size.height, percent)}`} +

+ )} +
+ onPercent(Number(event.target.value))} + className="h-1 w-full cursor-pointer accent-primary" + /> + + {percent}% + +
+
+ ); +} + +/** + * One slider per image, because a batch mixes sizes and one percent rarely + * fits them all. "Set all" is pure UI — it bulk-writes the per-tile values, + * and only the per-file map goes on the wire. Duplicate filenames in one drop + * collapse to one key; the server applies one percent to every part with that + * name, deliberately. + */ +export function ScaleMosaic({ + files, + scales, + onScales, +}: { + readonly files: readonly File[]; + readonly scales: Readonly>; + readonly onScales: (scales: Readonly>) => void; +}): JSX.Element { + const shown = files.slice(0, MOSAIC_CAP); + const setOne = (name: string, percent: number): void => { + const next: Record = { ...scales }; + if (percent === 100) delete next[name]; + else next[name] = percent; + onScales(next); + }; + return ( +
+ + onScales( + percent === 100 + ? {} + : Object.fromEntries(files.map((file) => [file.name, percent])), + ) + } + /> +

+ Moving this sets every image; adjust any image below individually. Each scales + relative to its own size, and the choice is part of the source's identity. +

+
+ {shown.map((file) => ( + setOne(file.name, percent)} + /> + ))} +
+ {files.length > MOSAIC_CAP && ( +

+ … and {files.length - MOSAIC_CAP} more, following the Set all slider. +

+ )} +
+ ); +} From 2a235e9bc896583769073bf22007f5f8cb2b8e8e Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Fri, 28 Aug 2026 03:08:50 -0700 Subject: [PATCH 12/16] docs: ingest-time downscale by percentage --- docs/content/cli.md | 10 +++++--- docs/content/ingest.md | 4 +-- docs/content/persistence.md | 12 ++++----- docs/content/sources.md | 49 ++++++++++++++++++++++++++----------- 4 files changed, 49 insertions(+), 26 deletions(-) diff --git a/docs/content/cli.md b/docs/content/cli.md index 231977a3..6120b448 100644 --- a/docs/content/cli.md +++ b/docs/content/cli.md @@ -19,7 +19,7 @@ visionset schema draft set FILE --project P [--kind K] [--note TEXT] [--revision visionset schema draft clear --project P [--kind K] visionset schema draft publish --project P [--kind K] [--revision N] [--allow-destructive] -visionset ingest PATH --project P [--fps N] [--range S:E]... [--batch-name NAME] [--start] +visionset ingest PATH --project P [--fps N] [--range S:E]... [--scale PCT] [--batch-name NAME] [--start] visionset batch list --project P visionset batch approve BATCH_ID [--jobs-of N] [--start] visionset batch pre-label BATCH_ID CONNECTION [--minimum-confidence FLOAT] [--replace-model-labels] [--geometry SHAPE]... @@ -347,8 +347,8 @@ name, no geometry, a select with no options - is refused there, named by its pos ### `visionset ingest` -`PATH --project P [--fps N] [--range S:E]... [--batch-name NAME]` - **the one command that is -two SDK calls**: +`PATH --project P [--fps N] [--range S:E]... [--scale PCT] [--batch-name NAME]` - **the one +command that is two SDK calls**: `SourceService.register_images` or `register_video`, dispatched on whether the path is a directory, then `IngestService.ingest`. Registration is idempotent, so re-running the same line registers once; content addressing means it also creates no asset it created before, which is the remedy for an @@ -356,7 +356,9 @@ interrupted run. The batch id goes to stdout. `--fps` and `--range` are video-only and usage errors on a folder. `--range START:END` repeats, in seconds, and the selection is stored canonically - clamped to the clip, sorted, overlapping -and touching ranges merged. The run is **synchronous**, and there is no +and touching ranges merged. `--scale` applies to both kinds - every frame of a clip, or every +file the directory holds now - and, like the rate and the ranges, is part of the source's +identity: another scale is a second source. The run is **synchronous**, and there is no `--resume`: polling needs a second process, which is what `visionset server` and `GET /ingest-jobs/{id}` are for. See [ingest.md](ingest.md#at-a-terminal). diff --git a/docs/content/ingest.md b/docs/content/ingest.md index 2aba58ca..7821716d 100644 --- a/docs/content/ingest.md +++ b/docs/content/ingest.md @@ -65,8 +65,8 @@ reported as unsupported rather than skipped, because guessing which files an ope offer is a policy the kernel would be inventing. **Frames are not re-probed.** `VideoProcessor` guarantees every frame is a complete image in -`FRAME_FORMAT` at the dimensions `probe` reported, and that promise is asserted in the port's own -tests. Decoding each one again to re-confirm it would also route our own encoder's output into an +`FRAME_FORMAT` at the source's stored size — the probe's dimensions scaled by its +`scale_percent` — and that promise is asserted in the port's own tests. Decoding each one again to re-confirm it would also route our own encoder's output into an operator's per-file report - a failure nobody could act on. ## Asking for a run and doing it are two calls diff --git a/docs/content/persistence.md b/docs/content/persistence.md index 79f363ba..df6fa28a 100644 --- a/docs/content/persistence.md +++ b/docs/content/persistence.md @@ -48,14 +48,14 @@ never raises `ProjectNameTaken`. But a rule with no backstop is a wish, so the store carries the constraint too: `uq_project_workspace_name` on `project (workspace_id, name COLLATE NOCASE)`, alongside `uq_schema_project_version`, `uq_schema_draft_project_kind`, `uq_member_dataset_asset`, -`uq_release_dataset_tag`, `uq_asset_project_content_hash`, `uq_source_project_kind_path_fps_ranges`, +`uq_release_dataset_tag`, `uq_asset_project_content_hash`, `uq_source_project_kind_path_fps_ranges_scale`, `uq_annotation_asset_classification`, `uq_token_workspace_name` and `uq_inference_connection_name`. The invariant then survives a service bug, a forgotten code path, and a second process. -`uq_source_project_kind_path_fps_ranges` is one of the two whose terms are not all columns: its -fourth is `coalesce(json_extract(video, '$.extraction_fps'), 0)` and its fifth -`coalesce(json_extract(video, '$.ranges'), '')`. SQLite treats NULLs in a unique index as +`uq_source_project_kind_path_fps_ranges_scale` is one of the two whose terms are not all +columns: beside three column terms it compares `coalesce`d expressions over the `video` JSON +(`$.extraction_fps`, `$.ranges`, `$.scale_percent`) and over the `image_scales` column. SQLite treats NULLs in a unique index as distinct, so a nullable column would let every image directory collide with nothing at all — and an index is not a query, so no service gains a JSON path from it. That is also why neither it nor `uq_annotation_asset_classification`, which is partial, can use `checkfirst`: SQLAlchemy cannot @@ -286,8 +286,8 @@ object with `_tables` rather than repeating the DDL - `checkfirst=True` on a `Ta **SQLAlchemy cannot reflect a partial or expression-based index**, so `checkfirst` reports one absent and re-issues a `CREATE` that then fails on every fresh database. Those ask SQLite instead, via `CreateIndex(index, if_not_exists=True)`. Two indexes here are in that -category: `uq_source_project_kind_path_fps_ranges` (its fourth and fifth terms are -`json_extract` expressions over `video`) and +category: `uq_source_project_kind_path_fps_ranges_scale` (its expression terms are +`json_extract`/`coalesce` expressions over `video` and `image_scales`) and `uq_annotation_asset_classification` (partial, on the tag geometry). **A column arriving by `ALTER` is declared last on its row class**, because SQLite appends diff --git a/docs/content/sources.md b/docs/content/sources.md index ff36c715..6aa2ea29 100644 --- a/docs/content/sources.md +++ b/docs/content/sources.md @@ -51,8 +51,9 @@ name, else the path's last segment, and it is what both wire projections publish `register_images` takes it, because a clip's basename is already its filename. `VideoProvenance` is the port's own `VideoMetadata` — original fps, duration, displayed -dimensions, codec — plus the cut a decomposition will run at: `extraction_fps`, and the clip -`ranges` extraction reads, empty meaning the whole clip. Ranges are stored canonically — +dimensions, codec — plus the cut a decomposition will run at: `extraction_fps`, the clip +`ranges` extraction reads (empty meaning the whole clip), and `scale_percent`, the percent of +the native size frames are stored at (100 meaning unscaled). Ranges are stored canonically — clamped to the clip, sorted, overlaps and touches merged, a full cover collapsing to the empty selection — so two spellings of one selection cannot fork a source. The probe result is kept whole rather than re-spelled field by field, because `metadata.fps` is the rate the file was @@ -72,12 +73,28 @@ assets. That promise only means something if the parameters are part of what "th *is* - put them on the ingest job and two runs of one source could legitimately disagree, leaving idempotency with nothing to be measured against. -The consequence is deliberate: **one clip registered at 1 fps and again at 5 fps is two sources -over one file**, not one source with a history. +The consequence is deliberate: **one clip registered at 1 fps and again at 5 fps — or at 100% +and again at 50% — is two sources over one file**, not one source with a history. + +## Storing at a smaller size + +Both registration methods take an optional downscale, applied while ingest materializes assets: +the original upload is retained as staged, the *stored* pixels are smaller. A clip takes one +`scale_percent`, so every extracted frame shares one size; an image directory takes a per-file +map, `image_scales`, because a batch mixes sizes and each file scales relative to its own. The +map is stored canonically — entries of 100 dropped, keys sorted — so two spellings of one +selection cannot fork a source. Each dimension becomes `max(1, (native * percent + 50) // 100)`; +the ingest screen mirrors that integer formula, so the preview and the stored size cannot drift. + +This is not the export-time resize: [pre-processing recipes](preprocessing.md) bring every +exported image to one model input size at export. The ingest-time scale exists to cut storage +and decode cost for sources nobody needs at native resolution, and it is permanent — the assets +*are* the smaller pixels. Re-ingesting the same origin at another scale is a second source. ## Registration is idempotent -The match key is `(kind, path, extraction_fps, ranges)`. Registering the same origin twice returns the +The match key is `(kind, path, extraction_fps, ranges, scale)` — a clip's `scale_percent`, an +image directory's `image_scales`. Registering the same origin twice returns the same `Source` rather than a second one, so that once ingest gives `asset.source_id` a target, "which source did this asset come from?" has one answer. @@ -101,15 +118,18 @@ because nothing referenced a source, so a duplicate was inert - and [ingest](ing that, by giving `asset.source_id` a target and letting the winner of a race decide an asset's recorded origin. -`uq_source_project_kind_path_fps_ranges` went in with it — born four-term as -`uq_source_project_kind_path_fps`, reshaped by migration 16 when ranges joined the identity — over -`(project_id, kind, path, coalesce(json_extract(video, '$.extraction_fps'), 0), -coalesce(json_extract(video, '$.ranges'), ''))`. The last two terms are expressions rather than -columns, and they are `coalesce`d rather than left to be NULL, because SQLite treats NULLs in a +`uq_source_project_kind_path_fps_ranges_scale` went in with it — born four-term as +`uq_source_project_kind_path_fps`, reshaped by migration 16 when ranges joined the identity and +by migration 18 when scale did — over `(project_id, kind, path, +coalesce(json_extract(video, '$.extraction_fps'), 0), +coalesce(json_extract(video, '$.ranges'), ''), +coalesce(json_extract(video, '$.scale_percent'), 0), coalesce(image_scales, ''))`. The +expression terms are `coalesce`d rather than left to be NULL, because SQLite treats NULLs in a unique index as **distinct** - an image directory, whose `video` is NULL, would otherwise never collide with itself, which is most of what the index is for. `0` cannot be mistaken for a real -rate (`extraction_fps` is `gt=0`), and an empty selection omits its JSON key when stored, so a -whole-clip row written in any generation lands on `''`. +rate or percent (`extraction_fps` is `gt=0`, `scale_percent` is `ge=1`), and every default — +an empty selection, an unscaled clip, an all-native directory — omits its key or stores NULL, +so a row written in any generation lands on the same term. The two layers do what they do everywhere else in this store. The pre-check is what produces a friendly answer; the index is the guarantee. A caller that loses the race sees a raw @@ -136,8 +156,9 @@ the workspace, so both stay outside the `VisionSetError` tree - the same line ## Over HTTP, a path is an upload `SourceService` registers by path, and an HTTP client has bytes rather than a path. So the -[REST API](api.md) takes multipart - one `files` part per image, or one `file` part plus -`extraction_fps` and an optional `ranges` field for a clip - writes the parts under +[REST API](api.md) takes multipart - one `files` part per image plus an optional `scales` +field (a JSON object of filename to percent), or one `file` part plus `extraction_fps`, an +optional `ranges` field and an optional `scale_percent` for a clip - writes the parts under `/uploads/`, and registers what it wrote. There is **no route that accepts a server-side path**: it would hand every token holder an arbitrary-directory read, and the two surfaces that legitimately hold real paths, the From 6f09e87b14d563f83e8a1947b575756d2115be1d Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Fri, 28 Aug 2026 03:46:04 -0700 Subject: [PATCH 13/16] feat(ui): the scale block leads with the stored size, and the preview player is mute --- .../ui-core/src/screens/ClipRangeTimeline.tsx | 6 +- .../src/screens/clipRangeTimeline.test.tsx | 2 + frontend/ui-core/src/screens/ingest.test.tsx | 5 +- frontend/ui-core/src/screens/ingestScale.tsx | 73 +++++++++++++------ frontend/ui-core/src/styles.css | 9 +++ 5 files changed, 72 insertions(+), 23 deletions(-) diff --git a/frontend/ui-core/src/screens/ClipRangeTimeline.tsx b/frontend/ui-core/src/screens/ClipRangeTimeline.tsx index 183f50f8..5fe25d72 100644 --- a/frontend/ui-core/src/screens/ClipRangeTimeline.tsx +++ b/frontend/ui-core/src/screens/ClipRangeTimeline.tsx @@ -352,8 +352,12 @@ export function ClipRangeTimeline({ ref={videoRef} src={src} controls + // Always mute: a vision dataset never needs the audio track, and + // the volume control it would earn is noise. The matching CSS in + // styles.css hides the control itself where the engine allows. + muted preload="metadata" - className="max-h-84 w-full max-w-2xl shrink-0 rounded-lg bg-muted" + className="vs-muted-player max-h-84 w-full max-w-2xl shrink-0 rounded-lg bg-muted" data-testid="clip-player" onTimeUpdate={timeUpdated} onPlay={playStarted} diff --git a/frontend/ui-core/src/screens/clipRangeTimeline.test.tsx b/frontend/ui-core/src/screens/clipRangeTimeline.test.tsx index 9f1b7a81..a8c0167b 100644 --- a/frontend/ui-core/src/screens/clipRangeTimeline.test.tsx +++ b/frontend/ui-core/src/screens/clipRangeTimeline.test.tsx @@ -109,6 +109,8 @@ describe("ClipRangeTimeline", () => { // 50px of 200 over 2 s is 0.5 s — inside the range, so the click plays. const video = screen.getByTestId("clip-player") as HTMLVideoElement; + // Always muted: a vision dataset has no use for the audio track. + expect(video.muted).toBe(true); expect(video.currentTime).toBe(0.5); expect(play).toHaveBeenCalled(); // The browser answers play() with the play event; the boundary arms there. diff --git a/frontend/ui-core/src/screens/ingest.test.tsx b/frontend/ui-core/src/screens/ingest.test.tsx index 33397339..b3465256 100644 --- a/frontend/ui-core/src/screens/ingest.test.tsx +++ b/frontend/ui-core/src/screens/ingest.test.tsx @@ -291,7 +291,10 @@ describe("registering a source", () => { render(mount()); await choose([pick("drive.mp4", "video/mp4")]); - fireEvent.change(await screen.findByTestId("scale-percent"), { target: { value: "50" } }); + // Untouched, the readout still states what exists — the fact was missing + // from the first design and is the reason the block leads with it. + expect((await screen.findByTestId("stored-size-native")).textContent).toContain("1920×1080"); + fireEvent.change(screen.getByTestId("scale-percent"), { target: { value: "50" } }); expect(screen.getByTestId("stored-size").textContent).toContain("960×540"); await userEvent.click(screen.getByTestId("register-source")); diff --git a/frontend/ui-core/src/screens/ingestScale.tsx b/frontend/ui-core/src/screens/ingestScale.tsx index fa3de82f..ec970b9f 100644 --- a/frontend/ui-core/src/screens/ingestScale.tsx +++ b/frontend/ui-core/src/screens/ingestScale.tsx @@ -21,25 +21,59 @@ export function scaledDimension(native: number, percent: number): number { * no slider primitive exists in this package and one control does not earn * one. Never `preventDefault` its pointer press — a range *drags* on its * default action, and cancelling the press is what made one unmovable (#563). + * + * The block leads with the outcome, not the mechanism: a readout that is + * always present (what resolution exists, what will be stored) and a purpose + * line that says what the value costs — the two facts a person needs *before* + * touching the slider. */ export function ScaleField({ percent, onPercent, native, id = "scale-percent", - preview = true, + label = "Stored size", + subject = "frame", + readout = true, }: { readonly percent: number; readonly onPercent: (value: number) => void; readonly native: { readonly width: number; readonly height: number } | null; readonly id?: string; - /** False where a caller renders its own previews — the mosaic's tiles do. */ - readonly preview?: boolean; + readonly label?: string; + /** What one stored item is called in the purpose line: "frame" or "image". */ + readonly subject?: string; + /** False where a caller renders its own readouts — the mosaic's tiles do. */ + readonly readout?: boolean; }): JSX.Element { + const pixels = Math.round((percent * percent) / 100); return (
- +
+ + {readout && + (native !== null ? ( + percent < 100 ? ( + + {native.width}×{native.height} → {scaledDimension(native.width, percent)}× + {scaledDimension(native.height, percent)} · {percent}% + + ) : ( + + {native.width}×{native.height} · native + + ) + ) : ( + + {percent < 100 ? `${percent}% per side · ` : ""}exact size read at upload + + ))} +
+ 10% onPercent(Number(event.target.value))} className="h-1 flex-1 cursor-pointer accent-primary" /> - - {percent}% - + 100%
- {preview && percent < 100 && ( -

- {native !== null - ? `stored at ${scaledDimension(native.width, percent)}×${scaledDimension(native.height, percent)}` - : `stored at ${percent}% of the clip's native size — the server reads the exact size`} -

- )} +

+ {percent < 100 + ? `Every ${subject} stored at ${percent}% per side — about ${pixels}% of the ` + + `pixels, so smaller files and faster training. Annotations are drawn on ` + + `what is stored.` + : `Stored as captured. Drag left to store smaller ${subject}s.`} +

); } @@ -181,9 +210,11 @@ export function ScaleMosaic({
onScales( percent === 100 @@ -193,8 +224,8 @@ export function ScaleMosaic({ } />

- Moving this sets every image; adjust any image below individually. Each scales - relative to its own size, and the choice is part of the source's identity. + Sets every image at once — adjust any tile individually after. Each scales relative + to its own size, and the choice is part of the source's identity.

{shown.map((file) => ( diff --git a/frontend/ui-core/src/styles.css b/frontend/ui-core/src/styles.css index 23d45c6f..3137e212 100644 --- a/frontend/ui-core/src/styles.css +++ b/frontend/ui-core/src/styles.css @@ -235,3 +235,12 @@ @apply font-heading; } } + +/* The ingest preview player is always muted — a vision dataset has no use for + the audio track — so the volume controls are dead weight. Only the WebKit + engines expose the pseudo-elements; elsewhere the player is merely muted. */ +.vs-muted-player::-webkit-media-controls-mute-button, +.vs-muted-player::-webkit-media-controls-volume-slider, +.vs-muted-player::-webkit-media-controls-volume-control-container { + display: none !important; +} From 7bd47bd0fe92c958e0a8a78423750b55573773cc Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Fri, 28 Aug 2026 03:49:05 -0700 Subject: [PATCH 14/16] fix(ui): a mixed mosaic names its state instead of claiming captured size --- frontend/ui-core/src/screens/ingestScale.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/frontend/ui-core/src/screens/ingestScale.tsx b/frontend/ui-core/src/screens/ingestScale.tsx index ec970b9f..3d499963 100644 --- a/frontend/ui-core/src/screens/ingestScale.tsx +++ b/frontend/ui-core/src/screens/ingestScale.tsx @@ -35,6 +35,7 @@ export function ScaleField({ label = "Stored size", subject = "frame", readout = true, + purpose, }: { readonly percent: number; readonly onPercent: (value: number) => void; @@ -45,6 +46,8 @@ export function ScaleField({ readonly subject?: string; /** False where a caller renders its own readouts — the mosaic's tiles do. */ readonly readout?: boolean; + /** Replaces the computed purpose line — the mosaic's mixed state needs its own. */ + readonly purpose?: string; }): JSX.Element { const pixels = Math.round((percent * percent) / 100); return ( @@ -90,11 +93,12 @@ export function ScaleField({ 100%

- {percent < 100 + {purpose ?? + (percent < 100 ? `Every ${subject} stored at ${percent}% per side — about ${pixels}% of the ` + `pixels, so smaller files and faster training. Annotations are drawn on ` + `what is stored.` - : `Stored as captured. Drag left to store smaller ${subject}s.`} + : `Stored as captured. Drag left to store smaller ${subject}s.`)}

); @@ -200,6 +204,7 @@ export function ScaleMosaic({ readonly onScales: (scales: Readonly>) => void; }): JSX.Element { const shown = files.slice(0, MOSAIC_CAP); + const mixed = new Set(shown.map((file) => scales[file.name] ?? 100)).size > 1; const setOne = (name: string, percent: number): void => { const next: Record = { ...scales }; if (percent === 100) delete next[name]; @@ -215,6 +220,7 @@ export function ScaleMosaic({ percent={commonPercent(shown, scales)} native={null} readout={false} + purpose={mixed ? "Sizes differ per image — each tile below shows its own." : undefined} onPercent={(percent) => onScales( percent === 100 From 1eb4faa945666dd5e943e31789a663dbb61e4850 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Fri, 28 Aug 2026 03:57:46 -0700 Subject: [PATCH 15/16] fix(ui): mosaic thumbnails survive the StrictMode double-mount --- frontend/app/cycle/cycle.spec.ts | 6 ++++++ frontend/ui-core/src/screens/ingestScale.tsx | 20 +++++++++++++------- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/frontend/app/cycle/cycle.spec.ts b/frontend/app/cycle/cycle.spec.ts index b1456f24..2ca60910 100644 --- a/frontend/app/cycle/cycle.spec.ts +++ b/frontend/app/cycle/cycle.spec.ts @@ -386,6 +386,12 @@ test("the whole cycle, from opening the app to a downloaded export", async ({ pa await page.getByTestId("file-input").setInputFiles(images()); await expect(page.getByTestId("chosen")).toContainText("3 files"); + // Each caption renders only after its thumbnail's object URL actually + // decoded, so this is the assertion that catches a revoked blob — the + // StrictMode double-mount once served every tile as a broken image, a + // failure only real Chromium can see (jsdom has no object URLs). + await expect(page.locator('[data-testid^="tile-size-"]')).toHaveCount(3); + await page.getByTestId("register-source").click(); await expect(page.getByTestId("source-card")).toBeVisible(); diff --git a/frontend/ui-core/src/screens/ingestScale.tsx b/frontend/ui-core/src/screens/ingestScale.tsx index 3d499963..9d573ecb 100644 --- a/frontend/ui-core/src/screens/ingestScale.tsx +++ b/frontend/ui-core/src/screens/ingestScale.tsx @@ -1,6 +1,6 @@ import { Image } from "lucide-react"; import type { JSX } from "react"; -import { useEffect, useMemo, useState } from "react"; +import { useEffect, useState } from "react"; import { Label } from "../primitives/label"; @@ -126,15 +126,21 @@ function MosaicTile({ }): JSX.Element { // jsdom has no object URLs and decodes no images — the tile degrades to // name plus slider, the same way the clip timeline renders no player. - const url = useMemo( - () => (typeof URL.createObjectURL === "function" ? URL.createObjectURL(file) : null), - [file], - ); + // + // Created inside the effect, never memoized: StrictMode mounts, cleans up, + // and mounts again, and a memoized URL survives that cycle already revoked — + // every thumbnail rendered as a broken image under the dev server. The + // remounted effect must mint its own URL, the way the clip preview does. + const [url, setUrl] = useState(null); useEffect(() => { + if (typeof URL.createObjectURL !== "function") return undefined; + const created = URL.createObjectURL(file); + setUrl(created); return () => { - if (url !== null) URL.revokeObjectURL(url); + setUrl(null); + URL.revokeObjectURL(created); }; - }, [url]); + }, [file]); const [size, setSize] = useState<{ width: number; height: number } | null>(null); return ( From bf43d5f70009a66a638afa20fbbef88f88ef683b Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Fri, 28 Aug 2026 04:21:36 -0700 Subject: [PATCH 16/16] refactor(ingest): scale is video-only - per-image resizing withdrawn --- docs/content/cli.md | 8 +- docs/content/persistence.md | 4 +- docs/content/sources.md | 42 ++-- frontend/app/cycle/cycle.spec.ts | 6 - frontend/app/e2e/gallery.spec.ts | 1 - frontend/ui-core/src/generated/api.ts | 19 +- frontend/ui-core/src/generated/checks.ts | 2 +- frontend/ui-core/src/screens/IngestScreen.tsx | 16 +- frontend/ui-core/src/screens/gallery.test.tsx | 1 - frontend/ui-core/src/screens/ingest.test.tsx | 39 ---- frontend/ui-core/src/screens/ingestScale.tsx | 189 +----------------- frontend/ui-core/src/screens/queries.ts | 11 +- openapi.json | 26 +-- src/visionset/cli/ingest.py | 27 ++- src/visionset/kernel/adapters/_mappers.py | 3 - src/visionset/kernel/adapters/_tables.py | 13 +- src/visionset/kernel/adapters/migrations.py | 14 +- .../kernel/adapters/pillow_image_processor.py | 36 +--- src/visionset/kernel/domain/__init__.py | 2 - src/visionset/kernel/domain/source.py | 30 +-- src/visionset/kernel/ports/image_processor.py | 9 +- .../kernel/services/ingest_service.py | 6 +- .../kernel/services/source_service.py | 19 +- src/visionset/mcp/sources.py | 18 +- src/visionset/server/models.py | 10 +- src/visionset/server/routes/sources.py | 106 ++-------- src/visionset/wire/__init__.py | 1 - tests/cli/test_ingest_commands.py | 17 +- tests/kernel/test_image_processor.py | 61 +----- tests/kernel/test_ingest_service.py | 37 +--- tests/kernel/test_migrations.py | 28 +-- tests/kernel/test_source_scale.py | 50 +---- tests/kernel/test_source_service.py | 22 -- tests/mcp/test_ingest_tools.py | 11 +- tests/server/test_sources.py | 46 +---- 35 files changed, 130 insertions(+), 800 deletions(-) diff --git a/docs/content/cli.md b/docs/content/cli.md index 6120b448..76d80381 100644 --- a/docs/content/cli.md +++ b/docs/content/cli.md @@ -354,10 +354,10 @@ then `IngestService.ingest`. Registration is idempotent, so re-running the same content addressing means it also creates no asset it created before, which is the remedy for an interrupted run. The batch id goes to stdout. -`--fps` and `--range` are video-only and usage errors on a folder. `--range START:END` repeats, -in seconds, and the selection is stored canonically - clamped to the clip, sorted, overlapping -and touching ranges merged. `--scale` applies to both kinds - every frame of a clip, or every -file the directory holds now - and, like the rate and the ranges, is part of the source's +`--fps`, `--range` and `--scale` are video-only and usage errors on a folder. `--range +START:END` repeats, in seconds, and the selection is stored canonically - clamped to the clip, +sorted, overlapping and touching ranges merged. `--scale` stores every extracted frame at that +percent of the clip's native size and, like the rate and the ranges, is part of the source's identity: another scale is a second source. The run is **synchronous**, and there is no `--resume`: polling needs a second process, which is what `visionset server` and `GET /ingest-jobs/{id}` are for. See [ingest.md](ingest.md#at-a-terminal). diff --git a/docs/content/persistence.md b/docs/content/persistence.md index df6fa28a..43985656 100644 --- a/docs/content/persistence.md +++ b/docs/content/persistence.md @@ -55,7 +55,7 @@ a service bug, a forgotten code path, and a second process. `uq_source_project_kind_path_fps_ranges_scale` is one of the two whose terms are not all columns: beside three column terms it compares `coalesce`d expressions over the `video` JSON -(`$.extraction_fps`, `$.ranges`, `$.scale_percent`) and over the `image_scales` column. SQLite treats NULLs in a unique index as +(`$.extraction_fps`, `$.ranges`, `$.scale_percent`). SQLite treats NULLs in a unique index as distinct, so a nullable column would let every image directory collide with nothing at all — and an index is not a query, so no service gains a JSON path from it. That is also why neither it nor `uq_annotation_asset_classification`, which is partial, can use `checkfirst`: SQLAlchemy cannot @@ -287,7 +287,7 @@ object with `_tables` rather than repeating the DDL - `checkfirst=True` on a `Ta one absent and re-issues a `CREATE` that then fails on every fresh database. Those ask SQLite instead, via `CreateIndex(index, if_not_exists=True)`. Two indexes here are in that category: `uq_source_project_kind_path_fps_ranges_scale` (its expression terms are -`json_extract`/`coalesce` expressions over `video` and `image_scales`) and +`json_extract`/`coalesce` expressions over `video`) and `uq_annotation_asset_classification` (partial, on the tag geometry). **A column arriving by `ALTER` is declared last on its row class**, because SQLite appends diff --git a/docs/content/sources.md b/docs/content/sources.md index 6aa2ea29..b75e28bb 100644 --- a/docs/content/sources.md +++ b/docs/content/sources.md @@ -76,25 +76,23 @@ idempotency with nothing to be measured against. The consequence is deliberate: **one clip registered at 1 fps and again at 5 fps — or at 100% and again at 50% — is two sources over one file**, not one source with a history. -## Storing at a smaller size +## Storing frames at a smaller size -Both registration methods take an optional downscale, applied while ingest materializes assets: -the original upload is retained as staged, the *stored* pixels are smaller. A clip takes one -`scale_percent`, so every extracted frame shares one size; an image directory takes a per-file -map, `image_scales`, because a batch mixes sizes and each file scales relative to its own. The -map is stored canonically — entries of 100 dropped, keys sorted — so two spellings of one -selection cannot fork a source. Each dimension becomes `max(1, (native * percent + 50) // 100)`; -the ingest screen mirrors that integer formula, so the preview and the stored size cannot drift. +A clip's registration takes an optional `scale_percent`, applied while ingest extracts frames: +the original upload is retained as staged, the *stored* frames are smaller, and every frame +shares one size because a clip has one native size. Each dimension becomes +`max(1, (native * percent + 50) // 100)`; the ingest screen mirrors that integer formula, so +the preview and the stored size cannot drift. Image directories always store stills at their +decoded size — an image batch mixes resolutions, and export is where a uniform size is made. This is not the export-time resize: [pre-processing recipes](preprocessing.md) bring every exported image to one model input size at export. The ingest-time scale exists to cut storage -and decode cost for sources nobody needs at native resolution, and it is permanent — the assets -*are* the smaller pixels. Re-ingesting the same origin at another scale is a second source. +and decode cost for clips nobody needs at native resolution, and it is permanent — the assets +*are* the smaller pixels. Re-ingesting the same clip at another scale is a second source. ## Registration is idempotent -The match key is `(kind, path, extraction_fps, ranges, scale)` — a clip's `scale_percent`, an -image directory's `image_scales`. Registering the same origin twice returns the +The match key is `(kind, path, extraction_fps, ranges, scale_percent)`. Registering the same origin twice returns the same `Source` rather than a second one, so that once ingest gives `asset.source_id` a target, "which source did this asset come from?" has one answer. @@ -123,13 +121,13 @@ recorded origin. by migration 18 when scale did — over `(project_id, kind, path, coalesce(json_extract(video, '$.extraction_fps'), 0), coalesce(json_extract(video, '$.ranges'), ''), -coalesce(json_extract(video, '$.scale_percent'), 0), coalesce(image_scales, ''))`. The -expression terms are `coalesce`d rather than left to be NULL, because SQLite treats NULLs in a -unique index as **distinct** - an image directory, whose `video` is NULL, would otherwise never -collide with itself, which is most of what the index is for. `0` cannot be mistaken for a real -rate or percent (`extraction_fps` is `gt=0`, `scale_percent` is `ge=1`), and every default — -an empty selection, an unscaled clip, an all-native directory — omits its key or stores NULL, -so a row written in any generation lands on the same term. +coalesce(json_extract(video, '$.scale_percent'), 0))`. The expression terms are `coalesce`d +rather than left to be NULL, because SQLite treats NULLs in a unique index as **distinct** - an +image directory, whose `video` is NULL, would otherwise never collide with itself, which is +most of what the index is for. `0` cannot be mistaken for a real rate or percent +(`extraction_fps` is `gt=0`, `scale_percent` is `ge=1`), and every default — an empty +selection, an unscaled clip — omits its JSON key, so a row written in any generation lands on +the same term. The two layers do what they do everywhere else in this store. The pre-check is what produces a friendly answer; the index is the guarantee. A caller that loses the race sees a raw @@ -156,9 +154,9 @@ the workspace, so both stay outside the `VisionSetError` tree - the same line ## Over HTTP, a path is an upload `SourceService` registers by path, and an HTTP client has bytes rather than a path. So the -[REST API](api.md) takes multipart - one `files` part per image plus an optional `scales` -field (a JSON object of filename to percent), or one `file` part plus `extraction_fps`, an -optional `ranges` field and an optional `scale_percent` for a clip - writes the parts under +[REST API](api.md) takes multipart - one `files` part per image, or one `file` part plus +`extraction_fps`, an optional `ranges` field and an optional `scale_percent` for a clip - +writes the parts under `/uploads/`, and registers what it wrote. There is **no route that accepts a server-side path**: it would hand every token holder an arbitrary-directory read, and the two surfaces that legitimately hold real paths, the diff --git a/frontend/app/cycle/cycle.spec.ts b/frontend/app/cycle/cycle.spec.ts index 2ca60910..b1456f24 100644 --- a/frontend/app/cycle/cycle.spec.ts +++ b/frontend/app/cycle/cycle.spec.ts @@ -386,12 +386,6 @@ test("the whole cycle, from opening the app to a downloaded export", async ({ pa await page.getByTestId("file-input").setInputFiles(images()); await expect(page.getByTestId("chosen")).toContainText("3 files"); - // Each caption renders only after its thumbnail's object URL actually - // decoded, so this is the assertion that catches a revoked blob — the - // StrictMode double-mount once served every tile as a broken image, a - // failure only real Chromium can see (jsdom has no object URLs). - await expect(page.locator('[data-testid^="tile-size-"]')).toHaveCount(3); - await page.getByTestId("register-source").click(); await expect(page.getByTestId("source-card")).toBeVisible(); diff --git a/frontend/app/e2e/gallery.spec.ts b/frontend/app/e2e/gallery.spec.ts index a3be6c58..04f893b3 100644 --- a/frontend/app/e2e/gallery.spec.ts +++ b/frontend/app/e2e/gallery.spec.ts @@ -480,7 +480,6 @@ async function serveApi(page: Page, sent: Request[], options: Options = {}): Pro ranges: [], scale_percent: 100, }, - image_scales: {}, } satisfies Wire["SourceOut"], }); } diff --git a/frontend/ui-core/src/generated/api.ts b/frontend/ui-core/src/generated/api.ts index 240c9908..946d0efe 100644 --- a/frontend/ui-core/src/generated/api.ts +++ b/frontend/ui-core/src/generated/api.ts @@ -2661,7 +2661,7 @@ export interface paths { put?: never; /** * Register Image Source - * @description Offer a project a folder of stills, each stored at its own scale. + * @description Offer a project a folder of stills. * * The parts are staged as one directory and that directory becomes the source. * Uploading the same files again returns the **same** source rather than a @@ -2674,9 +2674,6 @@ export interface paths { * `name` exists because the staged path's basename is a digest; a blank one is * 422 `INVALID_NAME`, refused by the kernel's own `InvalidName` — the domain * already refuses with a mapped error, so no wire validator restates it. - * - * `scales` names files to store below native size, and is part of the - * source's identity: the same files at other scales are a second source. */ post: operations["register_image_source"]; delete?: never; @@ -3694,11 +3691,6 @@ export interface components { * @description What to call the source. Without one it is named by its staged directory, whose basename is a content digest — 64 hex characters nobody can read. Registering the same files again with a new name renames the existing source rather than creating a second one. */ name?: string | null; - /** - * Scales - * @description Per-file downscale, as a JSON object of {"filename": percent} with integer percents in [1, 100]. Every filename must match an uploaded part; a file not named — and any entry of 100 — is stored at its decoded size. Part of the source's identity: the same files at other scales are a second source. - */ - scales?: string | null; }; /** Body_register_video_source */ Body_register_video_source: { @@ -5626,11 +5618,6 @@ export interface components { /** * SourceOut * @description A registered origin: a folder of stills, or a clip. - * - * `image_scales` is an image directory's per-file downscale, filename to - * percent. Only files stored below native size appear; an empty object means - * every file stores at its decoded size. Always empty for a video source, - * whose single `scale_percent` lives on `video`. */ SourceOut: { /** @@ -5638,10 +5625,6 @@ export interface components { * Format: uuid */ id: string; - /** Image Scales */ - image_scales: { - [key: string]: number; - }; kind: components["schemas"]["SourceKind"]; /** Name */ name: string; diff --git a/frontend/ui-core/src/generated/checks.ts b/frontend/ui-core/src/generated/checks.ts index c208a41c..d5c7e7b0 100644 --- a/frontend/ui-core/src/generated/checks.ts +++ b/frontend/ui-core/src/generated/checks.ts @@ -379,7 +379,7 @@ export const checkVideoProvenanceOut: Check = /*#__PURE__*/ object({ "codec": [true, isString], "duration_seconds": [true, isNumber], "extraction_fps": [true, isNumber], "fps": [true, isNumber], "height": [true, isInteger], "ranges": [true, arrayOf(checkClipRange)], "scale_percent": [true, isInteger], "width": [true, isInteger] } as const); export const checkSourceOut: Check = - /*#__PURE__*/ object({ "id": [true, isString], "image_scales": [true, mapOf(isInteger)], "kind": [true, checkSourceKind], "name": [true, isString], "project_id": [true, isString], "registered_at": [true, isString], "video": [true, either([checkVideoProvenanceOut, isNull] as const)] } as const); + /*#__PURE__*/ object({ "id": [true, isString], "kind": [true, checkSourceKind], "name": [true, isString], "project_id": [true, isString], "registered_at": [true, isString], "video": [true, either([checkVideoProvenanceOut, isNull] as const)] } as const); export const checkSourcePage: Check = /*#__PURE__*/ object({ "items": [true, arrayOf(checkSourceOut)], "total": [true, isInteger] } as const); diff --git a/frontend/ui-core/src/screens/IngestScreen.tsx b/frontend/ui-core/src/screens/IngestScreen.tsx index 63256866..037cf3b0 100644 --- a/frontend/ui-core/src/screens/IngestScreen.tsx +++ b/frontend/ui-core/src/screens/IngestScreen.tsx @@ -149,7 +149,7 @@ import { Card, CardContent } from "../primitives/card"; import { Progress } from "../primitives/progress"; import { Input } from "../primitives/input"; import { Label } from "../primitives/label"; -import { ScaleField, ScaleMosaic, scaledDimension } from "./ingestScale"; +import { ScaleField, scaledDimension } from "./ingestScale"; import { FieldDescription, FieldError } from "../primitives/field"; import { Select, @@ -290,9 +290,6 @@ export function IngestScreen({ // A clip's storage scale, decided in step 1 like the rate: percent of the // native size frames are stored at, part of the source's identity. const [scalePercent, setScalePercent] = useState(100); - // An image batch's per-file storage scales, keyed by filename; entries exist - // only below 100. Sent as one JSON multipart field, like ranges. - const [imageScales, setImageScales] = useState>>({}); // The chosen clip as an object URL for the preview player. Null where the // platform has no object URLs (jsdom), so the timeline renders no player. const [clipUrl, setClipUrl] = useState(null); @@ -339,7 +336,6 @@ export function IngestScreen({ setClip(null); setRanges([]); setScalePercent(100); - setImageScales({}); setUnreadable(false); if (!(files.length === 1 && files[0].type.startsWith("video/"))) return; const url = typeof URL.createObjectURL === "function" ? URL.createObjectURL(files[0]) : null; @@ -389,7 +385,6 @@ export function IngestScreen({ { files, ...(isVideo ? { extractionFps: rate, ranges, scalePercent } : {}), - ...(!isVideo && Object.keys(imageScales).length > 0 ? { scales: imageScales } : {}), ...(isVideo || stated === "" ? {} : { name: stated }), }, { onSuccess: (registered) => setSource(registered) }, @@ -408,7 +403,6 @@ export function IngestScreen({ setFiles([]); setFps(String(DEFAULT_EXTRACTION_FPS)); setScalePercent(100); - setImageScales({}); setSourceName(""); setAttempt((previous) => previous + 1); register.reset(); @@ -427,7 +421,6 @@ export function IngestScreen({ setFiles([]); setFps(String(DEFAULT_EXTRACTION_FPS)); setScalePercent(100); - setImageScales({}); setSourceName(""); setBatchChoice(NEW_BATCH); setBatchName(""); @@ -527,8 +520,6 @@ export function IngestScreen({ onRanges={setRanges} scalePercent={scalePercent} onScalePercent={setScalePercent} - imageScales={imageScales} - onImageScales={setImageScales} clipUrl={clipUrl} unreadable={unreadable} estimate={ @@ -855,8 +846,6 @@ function SelectionPanel({ onRanges, scalePercent, onScalePercent, - imageScales, - onImageScales, clipUrl, unreadable, estimate, @@ -874,8 +863,6 @@ function SelectionPanel({ readonly onRanges: (ranges: readonly ClipRange[]) => void; readonly scalePercent: number; readonly onScalePercent: (value: number) => void; - readonly imageScales: Readonly>; - readonly onImageScales: (scales: Readonly>) => void; readonly clipUrl: string | null; readonly unreadable: boolean; readonly estimate: number | null; @@ -944,7 +931,6 @@ function SelectionPanel({ both by the upload's content digest. - )} diff --git a/frontend/ui-core/src/screens/gallery.test.tsx b/frontend/ui-core/src/screens/gallery.test.tsx index bd54a78e..54c82f34 100644 --- a/frontend/ui-core/src/screens/gallery.test.tsx +++ b/frontend/ui-core/src/screens/gallery.test.tsx @@ -461,7 +461,6 @@ describe("the gallery", () => { ranges: [], scale_percent: 100, }, - image_scales: {}, }, }); diff --git a/frontend/ui-core/src/screens/ingest.test.tsx b/frontend/ui-core/src/screens/ingest.test.tsx index b3465256..b5b0c6f1 100644 --- a/frontend/ui-core/src/screens/ingest.test.tsx +++ b/frontend/ui-core/src/screens/ingest.test.tsx @@ -152,7 +152,6 @@ const VIDEO_SOURCE = { ranges: [], scale_percent: 100, }, - image_scales: {}, }; const IMAGE_SOURCE = { @@ -162,7 +161,6 @@ const IMAGE_SOURCE = { name: "photos", registered_at: "2026-07-31T00:00:00.000000Z", video: null, - image_scales: {}, }; function job(overrides: Record = {}): Record { @@ -216,43 +214,6 @@ describe("registering a source", () => { expect((form as FormData).get("name")).toBe("a"); }); - it("sends per-file scales chosen in the mosaic", async () => { - on("POST", /\/sources\/images$/, { status: 201, body: IMAGE_SOURCE }); - - render(mount()); - await choose([pick("a.png", "image/png"), pick("b.png", "image/png")]); - - fireEvent.change(screen.getByTestId("tile-scale-a.png"), { target: { value: "50" } }); - await userEvent.click(screen.getByTestId("register-source")); - - await waitFor(() => expect(sent.some((r) => r.method === "POST")).toBe(true)); - const form = bodies.get(sent.find((r) => r.method === "POST") as Request) as FormData; - expect(JSON.parse(form.get("scales") as string)).toEqual({ "a.png": 50 }); - }); - - it("set-all writes every tile and a single tile overrides after", async () => { - render(mount()); - await choose([pick("a.png", "image/png"), pick("b.png", "image/png")]); - - fireEvent.change(screen.getByTestId("scale-all"), { target: { value: "50" } }); - fireEvent.change(screen.getByTestId("tile-scale-b.png"), { target: { value: "100" } }); - - expect((screen.getByTestId("tile-scale-a.png") as HTMLInputElement).value).toBe("50"); - expect((screen.getByTestId("tile-scale-b.png") as HTMLInputElement).value).toBe("100"); - }); - - it("omits the scales part when every tile stores native", async () => { - on("POST", /\/sources\/images$/, { status: 201, body: IMAGE_SOURCE }); - - render(mount()); - await choose([pick("a.png", "image/png"), pick("b.png", "image/png")]); - await userEvent.click(screen.getByTestId("register-source")); - - await waitFor(() => expect(sent.some((r) => r.method === "POST")).toBe(true)); - const form = bodies.get(sent.find((r) => r.method === "POST") as Request) as FormData; - expect(form.has("scales")).toBe(false); - }); - it("sends a clip with the extraction rate, chosen before anything is probed", async () => { on("POST", /\/sources\/video$/, { status: 201, body: VIDEO_SOURCE }); diff --git a/frontend/ui-core/src/screens/ingestScale.tsx b/frontend/ui-core/src/screens/ingestScale.tsx index 9d573ecb..f573fc91 100644 --- a/frontend/ui-core/src/screens/ingestScale.tsx +++ b/frontend/ui-core/src/screens/ingestScale.tsx @@ -1,6 +1,4 @@ -import { Image } from "lucide-react"; import type { JSX } from "react"; -import { useEffect, useState } from "react"; import { Label } from "../primitives/label"; @@ -32,30 +30,18 @@ export function ScaleField({ onPercent, native, id = "scale-percent", - label = "Stored size", - subject = "frame", - readout = true, - purpose, }: { readonly percent: number; readonly onPercent: (value: number) => void; readonly native: { readonly width: number; readonly height: number } | null; readonly id?: string; - readonly label?: string; - /** What one stored item is called in the purpose line: "frame" or "image". */ - readonly subject?: string; - /** False where a caller renders its own readouts — the mosaic's tiles do. */ - readonly readout?: boolean; - /** Replaces the computed purpose line — the mosaic's mixed state needs its own. */ - readonly purpose?: string; }): JSX.Element { const pixels = Math.round((percent * percent) / 100); return (
- - {readout && - (native !== null ? ( + + {native !== null ? ( percent < 100 ? ( {native.width}×{native.height} → {scaledDimension(native.width, percent)}× @@ -69,11 +55,11 @@ export function ScaleField({ {native.width}×{native.height} · native ) - ) : ( - - {percent < 100 ? `${percent}% per side · ` : ""}exact size read at upload - - ))} + ) : ( + + {percent < 100 ? `${percent}% per side · ` : ""}exact size read at upload + + )}
10% @@ -93,167 +79,12 @@ export function ScaleField({ 100%

- {purpose ?? - (percent < 100 - ? `Every ${subject} stored at ${percent}% per side — about ${pixels}% of the ` + + {percent < 100 + ? `Every frame stored at ${percent}% per side — about ${pixels}% of the ` + `pixels, so smaller files and faster training. Annotations are drawn on ` + `what is stored.` - : `Stored as captured. Drag left to store smaller ${subject}s.`)} + : `Stored as captured. Drag left to store smaller frames.`}

); } - -const MOSAIC_CAP = 24; - -/** The percent every shown file shares, or 100 when they disagree or none is set. */ -function commonPercent( - files: readonly File[], - scales: Readonly>, -): number { - const percents = new Set(files.map((file) => scales[file.name] ?? 100)); - return percents.size === 1 ? [...percents][0] : 100; -} - -function MosaicTile({ - file, - percent, - onPercent, -}: { - readonly file: File; - readonly percent: number; - readonly onPercent: (value: number) => void; -}): JSX.Element { - // jsdom has no object URLs and decodes no images — the tile degrades to - // name plus slider, the same way the clip timeline renders no player. - // - // Created inside the effect, never memoized: StrictMode mounts, cleans up, - // and mounts again, and a memoized URL survives that cycle already revoked — - // every thumbnail rendered as a broken image under the dev server. The - // remounted effect must mint its own URL, the way the clip preview does. - const [url, setUrl] = useState(null); - useEffect(() => { - if (typeof URL.createObjectURL !== "function") return undefined; - const created = URL.createObjectURL(file); - setUrl(created); - return () => { - setUrl(null); - URL.revokeObjectURL(created); - }; - }, [file]); - const [size, setSize] = useState<{ width: number; height: number } | null>(null); - - return ( -
- {url !== null ? ( - {file.name} - setSize({ - width: event.currentTarget.naturalWidth, - height: event.currentTarget.naturalHeight, - }) - } - /> - ) : ( - - - - )} -
- {file.name} -
- {size !== null && ( -

- {size.width}×{size.height} - {percent < 100 && - ` → ${scaledDimension(size.width, percent)}×${scaledDimension(size.height, percent)}`} -

- )} -
- onPercent(Number(event.target.value))} - className="h-1 w-full cursor-pointer accent-primary" - /> - - {percent}% - -
-
- ); -} - -/** - * One slider per image, because a batch mixes sizes and one percent rarely - * fits them all. "Set all" is pure UI — it bulk-writes the per-tile values, - * and only the per-file map goes on the wire. Duplicate filenames in one drop - * collapse to one key; the server applies one percent to every part with that - * name, deliberately. - */ -export function ScaleMosaic({ - files, - scales, - onScales, -}: { - readonly files: readonly File[]; - readonly scales: Readonly>; - readonly onScales: (scales: Readonly>) => void; -}): JSX.Element { - const shown = files.slice(0, MOSAIC_CAP); - const mixed = new Set(shown.map((file) => scales[file.name] ?? 100)).size > 1; - const setOne = (name: string, percent: number): void => { - const next: Record = { ...scales }; - if (percent === 100) delete next[name]; - else next[name] = percent; - onScales(next); - }; - return ( -
- - onScales( - percent === 100 - ? {} - : Object.fromEntries(files.map((file) => [file.name, percent])), - ) - } - /> -

- Sets every image at once — adjust any tile individually after. Each scales relative - to its own size, and the choice is part of the source's identity. -

-
- {shown.map((file) => ( - setOne(file.name, percent)} - /> - ))} -
- {files.length > MOSAIC_CAP && ( -

- … and {files.length - MOSAIC_CAP} more, following the Set all slider. -

- )} -
- ); -} diff --git a/frontend/ui-core/src/screens/queries.ts b/frontend/ui-core/src/screens/queries.ts index 11b0a518..fef90c9e 100644 --- a/frontend/ui-core/src/screens/queries.ts +++ b/frontend/ui-core/src/screens/queries.ts @@ -799,7 +799,6 @@ export function useRegisterSource(projectId: string) { ranges?: readonly { start_seconds: number; end_seconds: number }[]; name?: string; scalePercent?: number; - scales?: Readonly>; }) => { const extractionFps = input.extractionFps; const source = @@ -827,15 +826,7 @@ export function useRegisterSource(projectId: string) { // `name` is what the source will be *called* — without it // the server names the source by its staged directory, whose // basename is a content digest. `formData` skips `undefined`. - body: { - files: input.files as unknown as string[], - name: input.name, - // Multipart carries strings, so the map rides as one JSON - // field, the way `ranges` does on the video branch. - ...(input.scales !== undefined && Object.keys(input.scales).length > 0 - ? { scales: JSON.stringify(input.scales) } - : {}), - }, + body: { files: input.files as unknown as string[], name: input.name }, bodySerializer: formData, }), checkRegisterImageSource, diff --git a/openapi.json b/openapi.json index a2cd594f..dc387b43 100644 --- a/openapi.json +++ b/openapi.json @@ -1636,18 +1636,6 @@ ], "description": "What to call the source. Without one it is named by its staged directory, whose basename is a content digest \u2014 64 hex characters nobody can read. Registering the same files again with a new name renames the existing source rather than creating a second one.", "title": "Name" - }, - "scales": { - "anyOf": [ - { - "type": "string" - }, - { - "type": "null" - } - ], - "description": "Per-file downscale, as a JSON object of {\"filename\": percent} with integer percents in [1, 100]. Every filename must match an uploaded part; a file not named \u2014 and any entry of 100 \u2014 is stored at its decoded size. Part of the source's identity: the same files at other scales are a second source.", - "title": "Scales" } }, "required": [ @@ -5496,20 +5484,13 @@ "type": "string" }, "SourceOut": { - "description": "A registered origin: a folder of stills, or a clip.\n\n`image_scales` is an image directory's per-file downscale, filename to\npercent. Only files stored below native size appear; an empty object means\nevery file stores at its decoded size. Always empty for a video source,\nwhose single `scale_percent` lives on `video`.", + "description": "A registered origin: a folder of stills, or a clip.", "properties": { "id": { "format": "uuid", "title": "Id", "type": "string" }, - "image_scales": { - "additionalProperties": { - "type": "integer" - }, - "title": "Image Scales", - "type": "object" - }, "kind": { "$ref": "#/components/schemas/SourceKind" }, @@ -5544,8 +5525,7 @@ "kind", "name", "registered_at", - "video", - "image_scales" + "video" ], "title": "SourceOut", "type": "object" @@ -14584,7 +14564,7 @@ }, "/projects/{project_id}/sources/images": { "post": { - "description": "Offer a project a folder of stills, each stored at its own scale.\n\nThe parts are staged as one directory and that directory becomes the source.\nUploading the same files again returns the **same** source rather than a\nsecond one: staging is content-addressed, so identical bytes under identical\nfilenames land on the same path, and registration is idempotent on that path.\n\nNothing is decoded here \u2014 what the files turn out to be is read at ingest,\nand a file that is not an image is reported there rather than refused now.\n\n`name` exists because the staged path's basename is a digest; a blank one is\n422 `INVALID_NAME`, refused by the kernel's own `InvalidName` \u2014 the domain\nalready refuses with a mapped error, so no wire validator restates it.\n\n`scales` names files to store below native size, and is part of the\nsource's identity: the same files at other scales are a second source.", + "description": "Offer a project a folder of stills.\n\nThe parts are staged as one directory and that directory becomes the source.\nUploading the same files again returns the **same** source rather than a\nsecond one: staging is content-addressed, so identical bytes under identical\nfilenames land on the same path, and registration is idempotent on that path.\n\nNothing is decoded here \u2014 what the files turn out to be is read at ingest,\nand a file that is not an image is reported there rather than refused now.\n\n`name` exists because the staged path's basename is a digest; a blank one is\n422 `INVALID_NAME`, refused by the kernel's own `InvalidName` \u2014 the domain\nalready refuses with a mapped error, so no wire validator restates it.", "operationId": "register_image_source", "parameters": [ { diff --git a/src/visionset/cli/ingest.py b/src/visionset/cli/ingest.py index c05a4b02..b2664354 100644 --- a/src/visionset/cli/ingest.py +++ b/src/visionset/cli/ingest.py @@ -9,9 +9,10 @@ dispatch is ``path.is_dir()``. Registering twice is free: registration is idempotent on -``(kind, path, extraction_fps, ranges, scale)``, so running this again on the -same folder finds the same source. Ingesting again is nearly free too — content addressing means a -re-run creates no assets it created before — which is also the remedy for the one +``(kind, path, extraction_fps, ranges, scale_percent)``, so running this again +on the same folder finds the same source. Ingesting again is nearly free too — +content addressing means a re-run creates no assets it created before — which +is also the remedy for the one gap this command has: interrupting it leaves the job row at ``running``, and there is no ``--resume``, because re-running does the right thing and needs no new vocabulary. @@ -163,9 +164,9 @@ def ingest( min=1, max=100, help=( - "Store at this percent of native size — every frame of a video, or " - "every file the directory holds now. Part of the source's identity, " - "like --fps: another scale is a second source. Defaults to 100." + "Store extracted frames at this percent of the clip's native size. " + "Video sources only. Part of the source's identity, like --fps: " + "another scale is a second source. Defaults to 100." ), ), ] = None, @@ -215,21 +216,17 @@ def ingest( raise typer.BadParameter( f"--range applies to a video source; {source} is a directory of stills" ) + if scale is not None and source.is_dir(): + raise typer.BadParameter( + f"--scale applies to a video source; {source} is a directory of stills" + ) ranges = [_parse_range(spec) for spec in range_specs or ()] with opened_workspace(workspace) as service: resolved = resolve_project(service, project) sources = SourceService(service) if source.is_dir(): - registered = sources.register_images( - resolved.id, - source, - image_scales=( - {} - if scale is None - else {item.name: scale for item in source.iterdir() if item.is_file()} - ), - ) + registered = sources.register_images(resolved.id, source) else: registered = sources.register_video( resolved.id, diff --git a/src/visionset/kernel/adapters/_mappers.py b/src/visionset/kernel/adapters/_mappers.py index f0c0eb61..9f6639dd 100644 --- a/src/visionset/kernel/adapters/_mappers.py +++ b/src/visionset/kernel/adapters/_mappers.py @@ -73,7 +73,6 @@ Token, VideoProvenance, Workspace, - canonical_image_scales, ) _geometry_adapter: TypeAdapter[Geometry] = TypeAdapter(Geometry) @@ -378,7 +377,6 @@ def _source_to_row(entity: Source) -> t.Base: registered_at=entity.registered_at.isoformat(), capture_params=dict(entity.capture_params), video=_video_to_json(entity.video), - image_scales=canonical_image_scales(entity.image_scales) or None, ) @@ -392,7 +390,6 @@ def _source_to_domain(_: Session, row: Any) -> Source: registered_at=datetime.fromisoformat(row.registered_at), capture_params=row.capture_params, video=None if row.video is None else VideoProvenance.model_validate(row.video), - image_scales=row.image_scales or {}, ) diff --git a/src/visionset/kernel/adapters/_tables.py b/src/visionset/kernel/adapters/_tables.py index 0ae24a1a..1fc55371 100644 --- a/src/visionset/kernel/adapters/_tables.py +++ b/src/visionset/kernel/adapters/_tables.py @@ -187,12 +187,8 @@ class SourceRow(Base): #: A ``VideoProvenance``, or NULL for anything that is not a clip. video: Mapped[dict[str, Any] | None] = mapped_column(JSON, nullable=True) #: What a caller asked this source to be called; NULL means nobody said. + #: The newest column here, so it is declared last — see the class docstring. display_name: Mapped[str | None] = mapped_column(String, nullable=True) - #: Per-file downscale for an image directory: filename -> percent, always - #: canonical (no 100s, sorted keys) so the origin index compares one - #: spelling, or NULL when every file stores at its decoded size. The - #: newest column here, so it is declared last — see the class docstring. - image_scales: Mapped[dict[str, int] | None] = mapped_column(JSON, nullable=True) #: One origin is one source: the backstop under ``SourceService``'s idempotency @@ -210,10 +206,8 @@ class SourceRow(Base): #: written before ranges existed or after — always lands on ``''``, and a row #: that names ranges lands on its one canonical JSON spelling. #: -#: The sixth and seventh terms follow the same two precedents: a clip stored -#: unscaled omits ``$.scale_percent`` (0 cannot be a real percent — the domain -#: floor is 1), and an image directory whose files all store native has a NULL -#: ``image_scales``, coalesced to ``''`` exactly as a missing ``$.ranges`` is. +#: The sixth term follows the same precedent: a clip stored unscaled omits +#: ``$.scale_percent``, and 0 cannot be a real percent — the domain floor is 1. #: #: This is SQL reading a JSON column, which the module docstring above reserves #: for values "nothing ever queries". An index is not a query: no service gains @@ -227,7 +221,6 @@ class SourceRow(Base): text("coalesce(json_extract(video, '$.extraction_fps'), 0)"), text("coalesce(json_extract(video, '$.ranges'), '')"), text("coalesce(json_extract(video, '$.scale_percent'), 0)"), - text("coalesce(image_scales, '')"), unique=True, ) diff --git a/src/visionset/kernel/adapters/migrations.py b/src/visionset/kernel/adapters/migrations.py index ce98f7df..bddc6ffc 100644 --- a/src/visionset/kernel/adapters/migrations.py +++ b/src/visionset/kernel/adapters/migrations.py @@ -392,15 +392,13 @@ def _reshape_source_origin_index(connection: Connection) -> None: def _add_source_scale(connection: Connection) -> None: """Scale joins the source's identity, so the origin index compares it. - Two new terms beside fps and ranges: a clip's ``$.scale_percent`` (omitted - at 100, so pre-scale rows and unscaled rows share one spelling) and the - per-file ``image_scales`` column (NULL when every file stores at its - decoded size, for the same reason). SQLite cannot alter an index: the old - one is dropped by name and the shared declaration created in its place. - As the head reshape this is the one migration that may execute the shared - declaration — see ``_reshape_source_origin_index``. + One new term beside fps and ranges: a clip's ``$.scale_percent``, omitted + at 100 so pre-scale rows and unscaled rows share one spelling. SQLite + cannot alter an index: the old one is dropped by name and the shared + declaration created in its place. As the head reshape this is the one + migration that may execute the shared declaration — see + ``_reshape_source_origin_index``. """ - _add_column(connection, "source", "image_scales") connection.execute(text("DROP INDEX IF EXISTS uq_source_project_kind_path_fps_ranges")) connection.execute(CreateIndex(SOURCE_ORIGIN_UNIQUE, if_not_exists=True)) diff --git a/src/visionset/kernel/adapters/pillow_image_processor.py b/src/visionset/kernel/adapters/pillow_image_processor.py index 4f45f7ba..ffbac95c 100644 --- a/src/visionset/kernel/adapters/pillow_image_processor.py +++ b/src/visionset/kernel/adapters/pillow_image_processor.py @@ -9,7 +9,7 @@ from PIL import Image, ImageOps, UnidentifiedImageError from pillow_heif import register_heif_opener -from visionset.kernel.domain import DecodedStill, ImageFormat, ImageMetadata, scaled_dimension +from visionset.kernel.domain import DecodedStill, ImageFormat, ImageMetadata from visionset.kernel.errors import CorruptMedia, UnsupportedMedia from visionset.kernel.ports.image_processor import DEFAULT_THUMBNAIL_MAX_EDGE @@ -153,18 +153,6 @@ def _fit(image: Image.Image, max_edge: int) -> Image.Image: return _opaque_rgb(working) -def _scaled(image: Image.Image, scale_percent: int) -> Image.Image: - if scale_percent == 100: - return image - return image.resize( - ( - scaled_dimension(image.width, scale_percent), - scaled_dimension(image.height, scale_percent), - ), - _RESAMPLING, - ) - - def _opaque_rgb(image: Image.Image) -> Image.Image: """The compositing tail of :func:`_fit`, alone: full-size, no resampling. @@ -252,22 +240,18 @@ def thumbnail( canvas.save(buffer, format=_THUMBNAIL_PILLOW_NAME, **_THUMBNAIL_ENCODER) return buffer.getvalue() - def stills( - self, content: BinaryIO, *, name: str | None = None, scale_percent: int = 100 - ) -> Iterator[DecodedStill]: + def stills(self, content: BinaryIO, *, name: str | None = None) -> Iterator[DecodedStill]: """Every dataset-ready still in this file. See the port docstring. The native gate runs before the frame count on purpose: MPO decodes with ``n_frames > 1``, and checking frames first would decompose every - burst photo instead of passing its primary frame through. A - ``scale_percent`` below 100 closes that gate: the original bytes are no - longer the asset, so even a dataset-ready native re-encodes, resized. + burst photo instead of passing its primary frame through. """ source = _stream_name(content, name) image = self._open(io.BytesIO(_read_all(content)), source) native = _FORMAT_BY_PILLOW_NAME.get(image.format or "") - if native is not None and scale_percent == 100: + if native is not None: self._load(image, source) with image: width, height = image.size @@ -275,13 +259,13 @@ def stills( [DecodedStill(metadata=ImageMetadata(width=width, height=height, format=native))] ) - if native is None and getattr(image, "n_frames", 1) > 1: + if getattr(image, "n_frames", 1) > 1: with image: - return iter(self._decomposed(image, source, scale_percent)) + return iter(self._decomposed(image, source)) self._load(image, source) with image: - flat = _scaled(_opaque_rgb(image), scale_percent) + flat = _opaque_rgb(image) buffer = io.BytesIO() flat.save(buffer, format="JPEG", **_STILL_ENCODER) return iter( @@ -295,9 +279,7 @@ def stills( ] ) - def _decomposed( - self, image: Image.Image, source: str | None, scale_percent: int - ) -> list[DecodedStill]: + def _decomposed(self, image: Image.Image, source: str | None) -> list[DecodedStill]: """One PNG still per frame — a list, not a generator, on purpose. Every frame is decoded and encoded before the caller sees the first @@ -310,7 +292,7 @@ def _decomposed( for index in range(int(getattr(image, "n_frames", 1))): try: image.seek(index) - frame = _scaled(_opaque_rgb(image), scale_percent) + frame = _opaque_rgb(image) except Image.DecompressionBombError as exc: raise UnsupportedMedia(str(exc), name=source) from exc except (EOFError, OSError, SyntaxError, ValueError) as exc: diff --git a/src/visionset/kernel/domain/__init__.py b/src/visionset/kernel/domain/__init__.py index 9a32d6fa..36d33b0e 100644 --- a/src/visionset/kernel/domain/__init__.py +++ b/src/visionset/kernel/domain/__init__.py @@ -253,7 +253,6 @@ SourceKind, TimeRange, VideoProvenance, - canonical_image_scales, canonical_path, canonical_ranges, expected_frames, @@ -537,7 +536,6 @@ "Workspace", "assign_split", "canonical_bytes", - "canonical_image_scales", "canonical_path", "canonical_ranges", "expected_frames", diff --git a/src/visionset/kernel/domain/source.py b/src/visionset/kernel/domain/source.py index 290d12ad..826b55d1 100644 --- a/src/visionset/kernel/domain/source.py +++ b/src/visionset/kernel/domain/source.py @@ -24,7 +24,7 @@ from __future__ import annotations import math -from collections.abc import Iterable, Mapping +from collections.abc import Iterable from datetime import UTC, datetime from enum import StrEnum from pathlib import Path, PurePath @@ -167,16 +167,6 @@ def scaled_dimension(native: int, percent: int) -> int: return max(1, (native * percent + 50) // 100) -def canonical_image_scales(scales: Mapping[str, int]) -> dict[str, int]: - """The one spelling of a per-file scale selection, so identity can compare it. - - Entries at 100 are dropped — storing at native size is what a file not - named already gets — and keys are sorted so the persisted JSON text is - deterministic for the origin index to compare. - """ - return {name: percent for name, percent in sorted(scales.items()) if percent != 100} - - class VideoProvenance(BaseModel): """What a clip was, and how we chose to cut it. @@ -256,10 +246,6 @@ class Source(BaseModel): value *does* refresh the stored one, because a label is curation, not provenance. - :attr:`image_scales` is an image directory's per-file downscale — filename - to percent, always canonical (see :func:`canonical_image_scales`), empty - meaning every file stores at its decoded size. Unlike the two fields above - it **is** part of the source's identity, exactly as a clip's cut is. """ model_config = ConfigDict(validate_assignment=True) @@ -272,7 +258,6 @@ class Source(BaseModel): registered_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) capture_params: dict[str, str] = Field(default_factory=dict) video: VideoProvenance | None = None - image_scales: dict[str, int] = Field(default_factory=dict) @property def name(self) -> str: @@ -299,19 +284,6 @@ def _video_provenance_matches_the_kind(self) -> Source: raise ValueError(f"a {self.kind.value} source must {carry} video provenance") return self - @model_validator(mode="after") - def _image_scales_are_canonical_and_match_the_kind(self) -> Source: - if self.image_scales and self.kind is not SourceKind.IMAGE_DIRECTORY: - raise ValueError(f"a {self.kind.value} source must not carry image scales") - for name, percent in self.image_scales.items(): - if not 1 <= percent <= 99: - raise ValueError(f"scale for {name!r} must be in [1, 99], got {percent}") - if self.image_scales != canonical_image_scales(self.image_scales): - raise ValueError( - "image_scales must be canonical; pass them through canonical_image_scales" - ) - return self - def require_video(self) -> VideoProvenance: """The clip's provenance, or refuse because this is not a clip. diff --git a/src/visionset/kernel/ports/image_processor.py b/src/visionset/kernel/ports/image_processor.py index df7c08e0..b47c3946 100644 --- a/src/visionset/kernel/ports/image_processor.py +++ b/src/visionset/kernel/ports/image_processor.py @@ -71,10 +71,7 @@ class ImageProcessor(Protocol): image yields transcoded items — one ``CONVERTED_STILL_FORMAT`` still for a single frame, one ``DECOMPOSED_FRAME_FORMAT`` still per frame of an animation. The decode is complete before the first item is yielded, so a - damaged file raises before a caller has stored anything. A - ``scale_percent`` below 100 resizes every emitted still and forces the - re-encode even for a dataset-ready native — the original bytes are no - longer the asset. + damaged file raises before a caller has stored anything. Raises: UnsupportedMedia: the bytes are not an image the decoder reads (for @@ -93,6 +90,4 @@ def thumbnail( name: str | None = None, ) -> bytes: ... - def stills( - self, content: BinaryIO, *, name: str | None = None, scale_percent: int = 100 - ) -> Iterator[DecodedStill]: ... + def stills(self, content: BinaryIO, *, name: str | None = None) -> Iterator[DecodedStill]: ... diff --git a/src/visionset/kernel/services/ingest_service.py b/src/visionset/kernel/services/ingest_service.py index d9268844..6e6412ed 100644 --- a/src/visionset/kernel/services/ingest_service.py +++ b/src/visionset/kernel/services/ingest_service.py @@ -620,11 +620,7 @@ def _read_directory( # frame of an animation included — before it yields its # first item, so a refusal arrives before anything below # stores a byte. - for still in self._workspace.image_processor.stills( - handle, - name=str(path), - scale_percent=source.image_scales.get(path.name, 100), - ): + for still in self._workspace.image_processor.stills(handle, name=str(path)): uri = ( str(path) if still.frame_index is None diff --git a/src/visionset/kernel/services/source_service.py b/src/visionset/kernel/services/source_service.py index e0310df6..8f1db14c 100644 --- a/src/visionset/kernel/services/source_service.py +++ b/src/visionset/kernel/services/source_service.py @@ -14,9 +14,8 @@ callers. **Registration is idempotent, and the match key is ``(kind, path, -extraction_fps, ranges, scale)``** — a clip's ``scale_percent`` and a -directory's per-file ``image_scales`` both fork identity, because the stored -pixels differ. Registering the same origin twice returns the same +extraction_fps, ranges, scale_percent)``** — a clip's scale forks identity, +because the stored pixels differ. Registering the same origin twice returns the same ``Source`` rather than a second one, so that "which source did this asset come from?" has one answer through ``asset.source_id``. The key deliberately excludes ``capture_params``: fragmenting one directory into two @@ -54,7 +53,6 @@ SourceKind, TimeRange, VideoProvenance, - canonical_image_scales, canonical_path, canonical_ranges, normalize_name, @@ -90,7 +88,6 @@ def register_images( *, capture_params: Mapping[str, str] | None = None, display_name: str | None = None, - image_scales: Mapping[str, int] | None = None, ) -> Source: """Record a directory of stills as an origin for this project. @@ -129,7 +126,6 @@ def register_images( display_name=( None if display_name is None else normalize_name(display_name, what="source name") ), - image_scales=canonical_image_scales(image_scales or {}), ) def register_video( @@ -221,15 +217,10 @@ def _register( video: VideoProvenance | None, capture_params: Mapping[str, str] | None, display_name: str | None = None, - image_scales: dict[str, int] | None = None, ) -> Source: """Add the source, or return the one that already stands for this origin.""" params = dict(capture_params or {}) - scales = image_scales or {} - cut = ( - None if video is None else (video.extraction_fps, video.ranges, video.scale_percent), - scales, - ) + cut = None if video is None else (video.extraction_fps, video.ranges, video.scale_percent) with self._workspace.unit_of_work() as uow: self._require_project(uow, project_id) for stored in uow.sources.list(project_id): @@ -242,8 +233,7 @@ def _register( stored.video.extraction_fps, stored.video.ranges, stored.video.scale_percent, - ), - stored.image_scales, + ) ) if stored_cut != cut: continue @@ -271,7 +261,6 @@ def _register( display_name=display_name, capture_params=params, video=video, - image_scales=scales, ) ) diff --git a/src/visionset/mcp/sources.py b/src/visionset/mcp/sources.py index f4461e59..ee1d9d81 100644 --- a/src/visionset/mcp/sources.py +++ b/src/visionset/mcp/sources.py @@ -87,9 +87,9 @@ def ingest( ge=1, le=100, description=( - "Store at this percent of native size — every frame of a video, or " - "every file the directory holds now. Part of the source's identity, " - "like fps: another scale is a second source. Omitted means 100." + "Store extracted frames at this percent of the clip's native size. " + "Video sources only. Part of the source's identity, like fps: " + "another scale is a second source. Omitted means 100 (unscaled)." ), ), ] = None, @@ -145,6 +145,8 @@ def ingest( return refused(f"fps applies to a video source, and {path} is a directory of stills") if ranges and source_path.is_dir(): return refused(f"ranges applies to a video source, and {path} is a directory of stills") + if scale is not None and source_path.is_dir(): + return refused(f"scale applies to a video source, and {path} is a directory of stills") try: selection = [ TimeRange(start_seconds=r.start_seconds, end_seconds=r.end_seconds) @@ -159,15 +161,7 @@ def ingest( resolved = resolve_project(workspace, project) service = SourceService(workspace) if source_path.is_dir(): - registered = service.register_images( - resolved.id, - source_path, - image_scales=( - {} - if scale is None - else {item.name: scale for item in source_path.iterdir() if item.is_file()} - ), - ) + registered = service.register_images(resolved.id, source_path) else: registered = service.register_video( resolved.id, diff --git a/src/visionset/server/models.py b/src/visionset/server/models.py index 67978cc7..c885564f 100644 --- a/src/visionset/server/models.py +++ b/src/visionset/server/models.py @@ -777,13 +777,7 @@ def of(cls, provenance: VideoProvenance) -> Self: # it hands every token holder the layout of the machine. ``name`` is the part a # client recognises: the filename it uploaded. class SourceOut(BaseModel): - """A registered origin: a folder of stills, or a clip. - - `image_scales` is an image directory's per-file downscale, filename to - percent. Only files stored below native size appear; an empty object means - every file stores at its decoded size. Always empty for a video source, - whose single `scale_percent` lives on `video`. - """ + """A registered origin: a folder of stills, or a clip.""" id: UUID project_id: UUID @@ -791,7 +785,6 @@ class SourceOut(BaseModel): name: str registered_at: datetime video: VideoProvenanceOut | None - image_scales: dict[str, int] @classmethod def of(cls, source: Source) -> Self: @@ -806,7 +799,6 @@ def of(cls, source: Source) -> Self: name=source.name, registered_at=source.registered_at, video=None if source.video is None else VideoProvenanceOut.of(source.video), - image_scales=dict(source.image_scales), ) diff --git a/src/visionset/server/routes/sources.py b/src/visionset/server/routes/sources.py index ea1c3304..5a262662 100644 --- a/src/visionset/server/routes/sources.py +++ b/src/visionset/server/routes/sources.py @@ -27,7 +27,7 @@ from fastapi import File, Form, Response, UploadFile, status from fastapi.exceptions import RequestValidationError -from pydantic import Field, TypeAdapter, ValidationError +from pydantic import TypeAdapter, ValidationError from visionset.jobs.ingest import JOB_TYPE as ingest_job_type from visionset.jobs.ingest import payload_for as ingest_payload_for @@ -43,7 +43,7 @@ SourceOut, SourcePage, ) -from visionset.server.uploads import safe_name, stage +from visionset.server.uploads import stage project_router = protected_router(prefix="/projects/{project_id}/sources", tags=["sources"]) router = protected_router(prefix="/sources", tags=["sources"]) @@ -72,78 +72,6 @@ _RANGES_ADAPTER: Final = TypeAdapter(tuple[TimeRange, ...]) -#: A clip's storage scale, as a multipart field. The bounds mirror -#: ``VideoProvenance.scale_percent``'s own, for ``ExtractionFpsForm``'s reason. -ScalePercentForm = Annotated[ - int, - Form( - ge=1, - le=100, - description=( - "Percent of the native size to store extracted frames at; 100 — the " - "default — stores them unscaled. Part of the source's identity, like " - "extraction_fps: the same clip at another scale is a second source." - ), - ), -] - -#: The per-file downscale map, as a multipart field. Multipart carries strings, -#: so the JSON object rides in one, exactly as ``ranges`` does. -ScalesForm = Annotated[ - str | None, - Form( - description=( - 'Per-file downscale, as a JSON object of {"filename": percent} with ' - "integer percents in [1, 100]. Every filename must match an uploaded " - "part; a file not named — and any entry of 100 — is stored at its " - "decoded size. Part of the source's identity: the same files at " - "other scales are a second source." - ), - ), -] - -_SCALES_ADAPTER: Final = TypeAdapter(dict[str, Annotated[int, Field(ge=1, le=100)]]) - - -def _parse_scales(scales: str | None) -> dict[str, int]: - """The `scales` field as a plain map, or the 422 a malformed one earns.""" - if scales is None: - return {} - try: - return dict(_SCALES_ADAPTER.validate_json(scales)) - except ValidationError as exc: - raise RequestValidationError(exc.errors()) from exc - - -def _staged_scales( - files: list[UploadFile], staged_names: tuple[str, ...], scales: dict[str, int] -) -> dict[str, int]: - """Client filenames re-keyed to staged names, in upload order. - - Staging can rename a colliding duplicate, and the zip below is the only - mapping between what the client called a part and what landed on disk. A - scale naming no uploaded file is refused loudly — silently storing that - file at native size is how a typo becomes a 4K asset nobody wanted. - """ - matched: set[str] = set() - by_staged_name: dict[str, int] = {} - for upload, staged_name in zip(files, staged_names, strict=True): - percent = scales.get(safe_name(upload.filename)) - if percent is not None: - matched.add(safe_name(upload.filename)) - by_staged_name[staged_name] = percent - if unmatched := set(scales) - matched: - raise RequestValidationError( - [ - { - "loc": ("body", "scales"), - "msg": f"no uploaded file is named {sorted(unmatched)}", - "type": "value_error", - } - ] - ) - return by_staged_name - def _parse_ranges(ranges: str | None) -> tuple[TimeRange, ...]: """The `ranges` field as domain values, or the 422 a malformed one earns. @@ -160,6 +88,22 @@ def _parse_ranges(ranges: str | None) -> tuple[TimeRange, ...]: raise RequestValidationError(exc.errors()) from exc +#: A clip's storage scale, as a multipart field. The bounds mirror +#: ``VideoProvenance.scale_percent``'s own, for ``ExtractionFpsForm``'s reason. +ScalePercentForm = Annotated[ + int, + Form( + ge=1, + le=100, + description=( + "Percent of the native size to store extracted frames at; 100 — the " + "default — stores them unscaled. Part of the source's identity, like " + "extraction_fps: the same clip at another scale is a second source." + ), + ), +] + + @project_router.post("/images", status_code=status.HTTP_201_CREATED, responses=documented(404)) def register_image_source( workspace: WorkspaceDep, @@ -176,9 +120,8 @@ def register_image_source( ), ), ] = None, - scales: ScalesForm = None, ) -> SourceOut: - """Offer a project a folder of stills, each stored at its own scale. + """Offer a project a folder of stills. The parts are staged as one directory and that directory becomes the source. Uploading the same files again returns the **same** source rather than a @@ -191,22 +134,13 @@ def register_image_source( `name` exists because the staged path's basename is a digest; a blank one is 422 `INVALID_NAME`, refused by the kernel's own `InvalidName` — the domain already refuses with a mapped error, so no wire validator restates it. - - `scales` names files to store below native size, and is part of the - source's identity: the same files at other scales are a second source. """ # ``capture_params`` is not on the wire. It is an opaque operator-supplied # mapping, and threading a JSON object through a multipart form is a # contract decision with no caller asking for it yet. - by_client_name = _parse_scales(scales) staged = stage(workspace.root, files) return SourceOut.of( - SourceService(workspace).register_images( - project_id, - staged.directory, - display_name=name, - image_scales=_staged_scales(files, staged.names, by_client_name), - ) + SourceService(workspace).register_images(project_id, staged.directory, display_name=name) ) diff --git a/src/visionset/wire/__init__.py b/src/visionset/wire/__init__.py index c00b34f2..f7dadf2a 100644 --- a/src/visionset/wire/__init__.py +++ b/src/visionset/wire/__init__.py @@ -347,7 +347,6 @@ def source(value: Source) -> dict[str, Any]: "name": value.name, "registered_at": _moment(value.registered_at), "video": None if value.video is None else video_provenance(value.video), - "image_scales": dict(value.image_scales), } diff --git a/tests/cli/test_ingest_commands.py b/tests/cli/test_ingest_commands.py index f5896f65..eb56b627 100644 --- a/tests/cli/test_ingest_commands.py +++ b/tests/cli/test_ingest_commands.py @@ -260,19 +260,10 @@ def test_a_video_registers_the_scale_it_was_given(root: Path, tmp_path: Path) -> assert document["source"]["video"]["scale_percent"] == 50 -def test_scale_on_a_directory_applies_to_every_file_present(root: Path, tmp_path: Path) -> None: - directory = stills(tmp_path) - document = payload(root, "ingest", str(directory), "-p", "road-signs", "--scale", "50") - names = {path.name for path in directory.iterdir() if path.is_file()} - assert document["source"]["image_scales"] == {name: 50 for name in names} - - -def test_a_scale_of_one_hundred_is_the_plain_registration(root: Path, tmp_path: Path) -> None: - directory = stills(tmp_path) - first = payload(root, "ingest", str(directory), "-p", "road-signs") - second = payload(root, "ingest", str(directory), "-p", "road-signs", "--scale", "100") - assert second["source"]["id"] == first["source"]["id"] - assert second["source"]["image_scales"] == {} +def test_scale_on_a_directory_exits_two(root: Path, tmp_path: Path) -> None: + result = run(root, "ingest", str(stills(tmp_path)), "-p", "road-signs", "--scale", "50") + assert result.exit_code == 2, result.output + assert "directory of stills" in usage_error(result) def test_an_out_of_range_scale_exits_two(root: Path, tmp_path: Path) -> None: diff --git a/tests/kernel/test_image_processor.py b/tests/kernel/test_image_processor.py index f88a7e9f..5c817c61 100644 --- a/tests/kernel/test_image_processor.py +++ b/tests/kernel/test_image_processor.py @@ -533,9 +533,7 @@ def thumbnail( ) -> bytes: return b"" - def stills( - self, content: io.IOBase, *, name: str | None = None, scale_percent: int = 100 - ) -> Iterator[DecodedStill]: + def stills(self, content: io.IOBase, *, name: str | None = None) -> Iterator[DecodedStill]: return iter(()) @@ -582,11 +580,9 @@ def test_a_decoded_still_is_frozen_and_defaults_to_pass_through() -> None: # --- stills: accept what Pillow decodes, normalized to JPEG and PNG ------------ -def _stills(path: Path, *, scale_percent: int = 100) -> list[DecodedStill]: +def _stills(path: Path) -> list[DecodedStill]: with path.open("rb") as handle: - return list( - PillowImageProcessor().stills(handle, name=str(path), scale_percent=scale_percent) - ) + return list(PillowImageProcessor().stills(handle, name=str(path))) def test_a_native_jpeg_passes_through_with_no_payload(tmp_path: Path) -> None: @@ -661,57 +657,6 @@ def test_a_transcode_is_repeatable_within_this_build(tmp_path: Path) -> None: assert _stills(path)[0].payload == _stills(path)[0].payload -def test_a_scaled_native_jpeg_re_encodes_at_the_stored_size(tmp_path: Path) -> None: - """Scaling forces the re-encode: the original bytes are no longer the asset.""" - path = tmp_path / "native.jpg" - Image.new("RGB", (100, 80), (200, 10, 10)).save(path, format="JPEG") - - (still,) = _stills(path, scale_percent=50) - - assert still.payload is not None - assert still.metadata == ImageMetadata(width=50, height=40, format=ImageFormat.JPEG) - assert _decoded(still.payload).size == (50, 40) - - -def test_a_scaled_native_png_re_encodes_as_jpeg(tmp_path: Path) -> None: - path = tmp_path / "native.png" - Image.new("RGB", (100, 80), (10, 200, 10)).save(path, format="PNG") - - (still,) = _stills(path, scale_percent=50) - - assert still.payload is not None - assert still.metadata.format is ImageFormat.JPEG - assert (still.metadata.width, still.metadata.height) == (50, 40) - - -def test_scaled_animation_frames_shrink_and_stay_png(tmp_path: Path) -> None: - path = tmp_path / "anim.gif" - _animated_gif(path, frames=3) - - stills = _stills(path, scale_percent=50) - - assert [still.metadata.format for still in stills] == [ImageFormat.PNG] * 3 - assert all((s.metadata.width, s.metadata.height) == (8, 6) for s in stills) - - -def test_scale_one_hundred_still_passes_natives_through(tmp_path: Path) -> None: - path = tmp_path / "native.jpg" - Image.new("RGB", (32, 24), (5, 5, 5)).save(path, format="JPEG") - - (still,) = _stills(path, scale_percent=100) - - assert still.payload is None - - -def test_a_scaled_dimension_never_reaches_zero(tmp_path: Path) -> None: - path = tmp_path / "sliver.png" - Image.new("RGB", (100, 1), (1, 2, 3)).save(path, format="PNG") - - (still,) = _stills(path, scale_percent=10) - - assert (still.metadata.width, still.metadata.height) == (10, 1) - - def test_stills_refuses_what_pillow_cannot_decode(tmp_path: Path) -> None: path = write_unsupported_file(tmp_path / "notes.txt") diff --git a/tests/kernel/test_ingest_service.py b/tests/kernel/test_ingest_service.py index 267ae676..96ebd22f 100644 --- a/tests/kernel/test_ingest_service.py +++ b/tests/kernel/test_ingest_service.py @@ -138,9 +138,7 @@ def probe(self, content: BinaryIO, *, name: str | None = None) -> ImageMetadata: self._observe() return self._real.probe(content, name=name) - def stills( - self, content: BinaryIO, *, name: str | None = None, scale_percent: int = 100 - ) -> Iterator[DecodedStill]: + def stills(self, content: BinaryIO, *, name: str | None = None) -> Iterator[DecodedStill]: self._observe() return self._real.stills(content, name=name) @@ -166,9 +164,7 @@ def __init__(self, nth: int) -> None: def probe(self, content: BinaryIO, *, name: str | None = None) -> ImageMetadata: return self._real.probe(content, name=name) - def stills( - self, content: BinaryIO, *, name: str | None = None, scale_percent: int = 100 - ) -> Iterator[DecodedStill]: + def stills(self, content: BinaryIO, *, name: str | None = None) -> Iterator[DecodedStill]: self._calls += 1 if self._calls == self._nth: raise OSError("the disk went away") @@ -194,9 +190,7 @@ def __init__(self) -> None: def probe(self, content: BinaryIO, *, name: str | None = None) -> ImageMetadata: return self._real.probe(content, name=name) - def stills( - self, content: BinaryIO, *, name: str | None = None, scale_percent: int = 100 - ) -> Iterator[DecodedStill]: + def stills(self, content: BinaryIO, *, name: str | None = None) -> Iterator[DecodedStill]: return self._real.stills(content, name=name) def thumbnail( @@ -501,31 +495,6 @@ def test_a_frame_takes_its_size_from_the_probe_and_its_format_from_the_port( fixture.close() -def test_a_scaled_still_ingests_at_its_own_percent_and_neighbors_stay_native( - tmp_path: Path, -) -> None: - """The per-file map applies per file: one entry scales one file, the rest pass through.""" - fixture = Fixture(tmp_path) - paths = write_images(fixture.stills, count=2) - source = fixture.sources.register_images( - fixture.project.id, fixture.stills, image_scales={paths[0].name: 50} - ) - - result = fixture.ingest.ingest(source.id) - - by_name = {asset.uri.rsplit("/", 1)[-1]: asset for asset in result.assets} - with Image.open(paths[0]) as native: - expected = (scaled_dimension(native.width, 50), scaled_dimension(native.height, 50)) - native_size = native.size - scaled = by_name[paths[0].name] - untouched = by_name[paths[1].name] - assert (scaled.width, scaled.height) == expected - assert scaled.format is ImageFormat.JPEG - assert (untouched.width, untouched.height) == native_size - assert untouched.format is ImageFormat.PNG - fixture.close() - - def test_a_scaled_clip_ingests_frames_at_the_stored_size(tmp_path: Path) -> None: """The asset records the scaled dimensions, and the pixels agree with them.""" fixture = Fixture(tmp_path) diff --git a/tests/kernel/test_migrations.py b/tests/kernel/test_migrations.py index 1da4ca33..4df5b952 100644 --- a/tests/kernel/test_migrations.py +++ b/tests/kernel/test_migrations.py @@ -50,7 +50,6 @@ "coalesce", "$.ranges", "$.scale_percent", - "image_scales", ), # Partial, so it constrains classification tags and nothing else: two boxes # under one class are two facts, two tags of one class are one statement @@ -194,10 +193,7 @@ def _at_generation_one(path: Path) -> None: connection.execute(text("ALTER TABLE inference_connection DROP COLUMN credential_env")) connection.execute(text("ALTER TABLE project DROP COLUMN created_at")) connection.execute(text("ALTER TABLE inference_connection DROP COLUMN origin")) - # The index reads image_scales, and SQLite refuses to drop a column an - # index still references — the index has to go first. connection.execute(text("DROP INDEX uq_source_project_kind_path_fps_ranges_scale")) - connection.execute(text("ALTER TABLE source DROP COLUMN image_scales")) connection.execute( text( "CREATE UNIQUE INDEX uq_source_project_kind_path_fps ON source" @@ -326,12 +322,11 @@ def test_the_reshaped_source_index_still_refuses_a_duplicate_origin(tmp_path: Pa migrated.close() -def test_the_scale_terms_fork_the_migrated_index(tmp_path: Path) -> None: +def test_the_scale_term_forks_the_migrated_index(tmp_path: Path) -> None: """Migration 18 exercised for real: scale forks identity, its absence collides. - The first pair differs only in ``$.scale_percent``; the second only in - ``image_scales`` — both must land. A row repeating an existing spelling - exactly must still be refused. + The pair differs only in ``$.scale_percent`` and must land; a row repeating + an existing spelling exactly must still be refused. """ whole = ( '{"metadata": {"width": 64, "height": 48, "fps": 10.0,' @@ -363,21 +358,6 @@ def test_the_scale_terms_fork_the_migrated_index(tmp_path: Path) -> None: f" '2026-01-02T00:00:00+00:00', '{{}}', '{scaled}')" ) ) - connection.execute( - text( - "insert into source (id, project_id, kind, path, registered_at," - " capture_params) values ('d1', 'p', 'image_directory', '/stills'," - " '2026-01-02T00:00:00+00:00', '{}')" - ) - ) - connection.execute( - text( - "insert into source (id, project_id, kind, path, registered_at," - " capture_params, image_scales) values ('d2', 'p', 'image_directory'," - " '/stills', '2026-01-02T00:00:00+00:00', '{}'," - " '{\"a.png\": 50}')" - ) - ) with pytest.raises(IntegrityError), migrated.engine.begin() as connection: connection.execute( text( @@ -418,7 +398,7 @@ def test_running_every_migration_again_changes_nothing(tmp_path: Path) -> None: # three deep and its *order* is the assertion — swapping any two would split # the ``create_all`` path from the migration path. "annotation_schema": ["description", "created_at", "provenance"], - "source": ["display_name", "image_scales"], + "source": ["display_name"], "asset": ["thumbnail_hash", "ingested_at"], # Migration 2 and migration 3, in that order. Both arrive by ``ALTER`` and # SQLite appends, so declaring either anywhere but last would split the diff --git a/tests/kernel/test_source_scale.py b/tests/kernel/test_source_scale.py index 047b5af9..94bf3e0a 100644 --- a/tests/kernel/test_source_scale.py +++ b/tests/kernel/test_source_scale.py @@ -1,18 +1,9 @@ """Scale arithmetic and the canonical spellings it adds to the source domain.""" -from uuid import uuid4 - import pytest from pydantic import ValidationError -from visionset.kernel.domain import ( - Source, - SourceKind, - VideoMetadata, - VideoProvenance, - canonical_image_scales, - scaled_dimension, -) +from visionset.kernel.domain import VideoMetadata, VideoProvenance, scaled_dimension def _metadata(width: int = 1920, height: int = 1080) -> VideoMetadata: @@ -25,25 +16,6 @@ def _provenance(*, width: int, height: int, scale_percent: int) -> VideoProvenan ) -def _image_source(*, image_scales: dict[str, int]) -> Source: - return Source( - project_id=uuid4(), - kind=SourceKind.IMAGE_DIRECTORY, - path="/data", - image_scales=image_scales, - ) - - -def _video_source(*, image_scales: dict[str, int]) -> Source: - return Source( - project_id=uuid4(), - kind=SourceKind.VIDEO, - path="/clip.mp4", - video=_provenance(width=4, height=4, scale_percent=100), - image_scales=image_scales, - ) - - def test_scaled_dimension_rounds_half_up_in_integer_arithmetic() -> None: assert scaled_dimension(25, 50) == 13 assert scaled_dimension(1920, 50) == 960 @@ -51,26 +23,6 @@ def test_scaled_dimension_rounds_half_up_in_integer_arithmetic() -> None: assert scaled_dimension(640, 100) == 640 -def test_canonical_image_scales_drops_hundreds_and_sorts_keys() -> None: - assert canonical_image_scales({"b.png": 100, "a.png": 50}) == {"a.png": 50} - assert list(canonical_image_scales({"z.png": 40, "a.png": 60})) == ["a.png", "z.png"] - assert canonical_image_scales({}) == {} - - -def test_image_scales_refuses_non_canonical_and_out_of_range() -> None: - with pytest.raises(ValidationError): - _image_source(image_scales={"a.png": 100}) - with pytest.raises(ValidationError): - _image_source(image_scales={"a.png": 0}) - with pytest.raises(ValidationError): - _image_source(image_scales={"a.png": 101}) - - -def test_a_video_source_carries_no_image_scales() -> None: - with pytest.raises(ValidationError): - _video_source(image_scales={"a.png": 50}) - - def test_scale_percent_is_bounded() -> None: with pytest.raises(ValidationError): _provenance(width=4, height=4, scale_percent=0) diff --git a/tests/kernel/test_source_service.py b/tests/kernel/test_source_service.py index c3cd932b..90659721 100644 --- a/tests/kernel/test_source_service.py +++ b/tests/kernel/test_source_service.py @@ -270,28 +270,6 @@ def test_a_scale_of_one_hundred_is_the_plain_source(tmp_path: Path) -> None: fx.close() -def test_the_same_directory_with_different_scales_is_a_second_source(tmp_path: Path) -> None: - fx = Fixture(tmp_path) - first = fx.sources.register_images(fx.project.id, fx.stills) - second = fx.sources.register_images(fx.project.id, fx.stills, image_scales={"a.png": 50}) - assert second.id != first.id - assert second.image_scales == {"a.png": 50} - assert {s.id for s in fx.sources.list(fx.project.id)} == {first.id, second.id} - fx.close() - - -def test_image_scale_spelling_variants_collapse_to_one_source(tmp_path: Path) -> None: - """Identity compares the canonical form, never what a caller happened to type.""" - fx = Fixture(tmp_path) - messy = fx.sources.register_images( - fx.project.id, fx.stills, image_scales={"b.png": 100, "a.png": 50} - ) - tidy = fx.sources.register_images(fx.project.id, fx.stills, image_scales={"a.png": 50}) - assert tidy == messy - assert len(fx.sources.list(fx.project.id)) == 1 - fx.close() - - def test_range_spelling_variants_collapse_to_one_source(tmp_path: Path) -> None: """Identity compares the canonical form, never what a caller happened to type.""" fx = Fixture(tmp_path) diff --git a/tests/mcp/test_ingest_tools.py b/tests/mcp/test_ingest_tools.py index faa88a25..219e677f 100644 --- a/tests/mcp/test_ingest_tools.py +++ b/tests/mcp/test_ingest_tools.py @@ -86,15 +86,16 @@ def test_a_clip_ingested_with_a_scale_echoes_it( assert result["source"]["video"]["scale_percent"] == 50 -def test_a_scaled_directory_names_every_file_present( +def test_scale_for_a_directory_of_stills_is_refused( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: named = schema(monkeypatch, tmp_path) - write_images(tmp_path / "incoming", count=2) - result = payload(call("ingest", project=named, path=str(tmp_path / "incoming"), scale=50)) + write_images(tmp_path / "incoming", count=1) + message = error(call("ingest", project=named, path=str(tmp_path / "incoming"), scale=50))[ + "message" + ] - names = {item.name for item in (tmp_path / "incoming").iterdir() if item.is_file()} - assert result["source"]["image_scales"] == {name: 50 for name in names} + assert "video source" in message def test_ranges_for_a_directory_of_stills_are_refused( diff --git a/tests/server/test_sources.py b/tests/server/test_sources.py index 009aa42e..cccba025 100644 --- a/tests/server/test_sources.py +++ b/tests/server/test_sources.py @@ -274,7 +274,6 @@ def test_the_default_scale_is_native_size(client: TestClient, project: str, clip response = post_video(client, project, clip) assert response.json()["video"]["scale_percent"] == 100 - assert response.json()["image_scales"] == {} def test_a_clip_at_two_scales_is_two_sources(client: TestClient, project: str, clip: Path) -> None: @@ -293,41 +292,6 @@ def test_an_out_of_range_scale_is_422_before_anything_is_written( assert not list((tmp_path / "workspace" / "uploads").rglob("*")) or True -def test_images_registered_with_scales_publish_the_canonical_map( - client: TestClient, project: str, tmp_path: Path -) -> None: - response = client.post( - f"/projects/{project}/sources/images", - files=[image_part(tmp_path, "a.png", 1), image_part(tmp_path, "b.png", 2)], - data={"scales": '{"a.png": 50, "b.png": 100}'}, - ) - - assert response.status_code == 201, response.text - assert response.json()["image_scales"] == {"a.png": 50} - - -def test_a_scale_for_a_file_not_uploaded_is_422( - client: TestClient, project: str, tmp_path: Path -) -> None: - response = client.post( - f"/projects/{project}/sources/images", - files=[image_part(tmp_path, "a.png", 1)], - data={"scales": '{"missing.png": 50}'}, - ) - - assert response.status_code == 422 - - -def test_malformed_scales_are_422(client: TestClient, project: str, tmp_path: Path) -> None: - response = client.post( - f"/projects/{project}/sources/images", - files=[image_part(tmp_path, "a.png", 1)], - data={"scales": '{"a.png": "half"}'}, - ) - - assert response.status_code == 422 - - @pytest.mark.parametrize( "bad", ["not json", '[{"start_seconds": 2, "end_seconds": 1}]', '[{"start": 0}]'], @@ -403,15 +367,7 @@ def test_a_source_never_publishes_its_path( body = post_images(client, project, image_part(tmp_path, "a.png", 1)).json() assert "path" not in body - assert set(body) == { - "id", - "project_id", - "kind", - "name", - "registered_at", - "video", - "image_scales", - } + assert set(body) == {"id", "project_id", "kind", "name", "registered_at", "video"} def test_reading_an_unknown_source_is_404(client: TestClient) -> None: