Skip to content

fix(worker): register a blob for every asset the stub produces - #1022

Open
georgi wants to merge 1 commit into
mainfrom
fix/worker-asset-blob-coverage
Open

fix(worker): register a blob for every asset the stub produces#1022
georgi wants to merge 1 commit into
mainfrom
fix/worker-asset-blob-coverage

Conversation

@georgi

@georgi georgi commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Closes the class behind #1020 (video) and #1021 (audio).

This is one defect class, not four bugs

WorkerContext covers a ref type per entry point, not per type. Any factory it does not override falls through to ProcessingContext, which with no name returns a ref carrying inline data and an empty uri. executor._extract_outputs extracts blobs only for a blob:// uri, and _serialize_asset_ref strips every raw-bytes data field at any depth. The bytes are discarded and the node reports success — 114.6 s of A40 time for an empty mp4, in the case that started this.

So every *_from_* method added to ProcessingContext from now on is a silent data-loss bug in the worker until someone remembers to override it. The coverage test below is what stops that.

Changes

Entry point Was Now
image_from_io ImageRef(data=…, uri="") blob://
image_from_url broken via image_from_io fixed by the above — see finding 1
model3d_from_io Model3DRef(data=…, uri="") blob://
audio_from_segment (no name) AudioRef(uri="memory://<uuid>", data=…) blob://
video_from_io, audio_from_io as in #1020 / #1021

Branch base. Cut from main as asked, so it also carries the video_from_io and audio_from_io overrides — without them the coverage test cannot be green here. If #1020 and #1021 merge first, rebasing shrinks this diff to image / model3d / segment plus the tests; the conflicts are adjacent method insertions in one file. Their test files are not duplicated here — the coverage test drives those funnels behaviorally.

audio_from_segment keeps the base implementation's WAV encoding and metadata; the override only re-homes the bytes when the base took its memory:// shortcut. That shortcut exists so a later in-process read can get the AudioSegment back without decoding. A worker output crosses a process boundary, so the host can never follow it — the fast path is not available to a worker at all, and the choice is between blob bytes and no bytes.

Finding 1 — image_from_url

Verified, not assumed. Its body is one line:

return await self.image_from_io(await self.download_file(url), name=name, parent_id=parent_id)

It downloads and funnels through image_from_io, so overriding image_from_io covers it. No separate override. tests/worker/test_context_stub_remaining_blob_gaps.py::test_image_from_url_funnels_through_image_from_io pins that with a stubbed download_file — no network.

Finding 2 — asset_to_data

A reader, not a producer. Not this bug, not fixed. It takes an existing AssetRef, and when that ref is a memory:// handle with no data it resolves the handle out of the in-process store and returns a copy with data filled in. It creates no new asset and no new bytes; it materializes bytes that already exist so a caller can read them.

The coverage test excludes it structurally rather than by name: a method with an AssetRef-typed parameter converts a ref, it does not produce output. Nothing is allowlisted.

The coverage test

tests/worker/test_context_stub_blob_coverage.py, two halves, because neither is enough alone:

  1. Behavioral. Enumerate every *_from_io producer, drive each with a real BytesIO, and assert it returns a blob:// uri and that the bytes come back out of executor._extract_outputs. A guard test asserts the enumeration actually found a funnel for every type in ASSET_REF_TYPES, so it cannot pass by covering nothing.
  2. Structural. Every producer on ProcessingContext — returns a type in ASSET_REF_TYPES, takes no ref parameter — must be overridden by WorkerContext or delegate (transitively, read off the source) to one that is. This is what catches a factory added tomorrow.

The structural half alone would have been worse than nothing: it follows delegation without seeing branches, so it marked audio_from_segment covered while its no-name branch dropped the audio. That is why the behavioral half exists, and the test's docstring says so.

Red / green

./.venv/bin/python -m pytest tests/worker/test_context_stub_blob_coverage.py tests/worker/test_context_stub_remaining_blob_gaps.py -q

Before (stub reverted to main, tests present):

E  AssertionError: audio_from_io returned uri '', so its bytes are dropped
E  AssertionError: image_from_io returned uri '', so its bytes are dropped
E  AssertionError: model3d_from_io returned uri '', so its bytes are dropped
E  AssertionError: video_from_io returned uri '', so its bytes are dropped
E  AssertionError: these produce an asset the worker cannot deliver — override them on WorkerContext
   so the bytes are registered as a blob: ['audio_from_base64', 'audio_from_bytes', 'audio_from_io',
   'audio_from_segment', 'image_from_io', 'image_from_url', 'model3d_from_io', 'video_from_bytes',
   'video_from_frames', 'video_from_io', 'video_from_numpy']
E  AssertionError: expected a blob uri, got ''
E  AssertionError: expected a blob uri, got ''
E  AssertionError: expected a blob uri, got ''
E  AssertionError: expected a blob uri, got 'memory://c57f0789-1b87-4fe6-b08d-6994a77aaca6'
9 failed, 1 passed in 0.09s

Every red is behavioral. The memory:// line is the audio_from_segment hole the structural check could not see.

After:

..........                                                               [100%]
10 passed in 0.06s

Whole worker suite: 225 passed in 9.25s. No network, no GPU, no model downloads — the one download is monkeypatched, and the AudioSegment is 20 ms of generated silence.

Nothing is left failing: every entry point the enumeration finds passes.

The stub context covered a ref type per entry point, not per type. Any
factory it did not override fell through to ProcessingContext, which
returns a ref with inline data and no uri; the executor extracts blobs
only from a blob:// uri and the serializer strips raw data at any depth,
so the bytes were discarded while the node reported success.

Override the remaining producers: image_from_io (which image_from_url
funnels through), model3d_from_io, and audio_from_segment's no-name
path, which returned a memory:// ref pointing at a store that lives and
dies inside the worker. Also carries the video_from_io and audio_from_io
overrides, since this branch is cut from main.

Add a coverage test with two halves: it drives every *_from_io funnel
with a real buffer and follows the bytes through the executor, and it
walks the source to require every producer to be overridden or to
delegate to one — so the next factory added to ProcessingContext cannot
ship as a silent data-loss bug.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant