Repository navigation
fix(transform): classify mixed image formats as source-unsupported - #337
Conversation
|
👋 Hi @aadetya — thank you so much for your first contribution to HFlow! A maintainer will review your pull request as soon as possible. In the meantime:
💡 Tip: one open pull request per contributor at a time. Issues with an assignee are taken; everything else is fair game. We are excited to have you here and appreciate your help making the project better! 🙌 |
kstonekuan
left a comment
There was a problem hiding this comment.
LGTM, merging. Welcome.
This closes the gap exactly as measured. When I filed #331 the two adjacent refusals disagreed:
mixes image formats -> infrastructure
unsupported single format -> source-unsupported
On your branch:
mixes image formats -> source-unsupported
unsupported single format -> source-unsupported
Validated: ruff check, ruff format --check, ty check clean, 1320 passed / 6 skipped. Reverted your raise back to a plain ValueError and test_mixed_compressed_image_formats_classify_as_source_unsupported failed, so the test holds the classification rather than just describing it.
Driving it through write_canonical_episode with a real two-message MCAP instead of constructing the exception directly is what the four tests next to it do, and it's the difference between pinning the classifier and pinning the raise site as well. Your comment on the payload bytes is right too: the format consistency check runs before anything decodes the image, which is why a placeholder works there.
You also updated both places that enumerate the refusal categories, the SourceNotConforming docstring and the converter guide. I checked docs/CATALOG.md and it lists kinds rather than categories, so there was correctly nothing to do there.
Summary
source-unsupported.Closes #331
Why
#203 established that transform failures caused by a recording’s contents should not be classified as
infrastructure, since retrying the same recording cannot resolve them. #288 implemented that distinction by introducingSourceNotConformingand mapping it tosource-unsupportedfor the four refusal cases in scope.During review of #288, the mixed-image-format path was identified as the remaining inconsistency. It still raised a plain
ValueError, causingclassify_ingest_failureto treat it asinfrastructure, even though the failure comes from the source recording.This change brings that path in line with the classification introduced in #288. The existing error message remains unchanged.
Mixed-format inputs still fail before an output file is written, so no behavior version bump is needed.
Validation
uv sync --locked --all-extrasuv run ruff check --fix— passeduv run ruff format— passeduv run ty check— passeduv run pytest -q tests/test_ingest_ledger.py— 9 passeduv run pytest -q— 1315 passed, 11 skippedChecklist
uv run ruff check --fix,uv run ruff format, anduv run ty check.