Repository navigation
fix: classify transform content refusals as source-unsupported, not infrastructure - #288
Conversation
…nfrastructure Adds SourceNotConforming(ValueError) raised by the transform's content refusals (non-conforming pass-through video, raw-image schema, unsupported compressed-image format, derived-topic collision) and a new IngestFailureKind.SOURCE_UNSUPPORTED, so classify_ingest_failure no longer files these under infrastructure -- which would otherwise retry a batch that cannot succeed. McapError still classifies as source-unreadable and SourceNotFound still classifies as source-missing. Fixes Hebbian-Robotics#203
|
👋 Hi @aleks-drozy — 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! 🙌 |
…review - CATALOG.md: rows written before this change keep whatever kind the ingest binary assigned at write time; source-unsupported only appears on rows written after the cutover, and error_type/message remain the ground truth regardless. - SourceNotConforming docstring: state explicitly that isinstance-checking this type is the only path into SOURCE_UNSUPPORTED, so a stray ValueError from a new refusal falls back to infrastructure rather than drifting in.
kstonekuan
left a comment
There was a problem hiding this comment.
LGTM, merging. You answered both questions before I asked them again, and the "only way in" argument is the right one: one isinstance check, no string matching, bare ValueError falls back to infrastructure. The cutover note in CATALOG.md is what an operator draining an old ledger actually needs.
Validated on your branch: ruff check, ruff format --check, ty check clean, 1252 passed / 6 skipped. Your 39 failures were the pinned ffmpeg download 404ing in your sandbox, nothing to do with the change.
I also checked the tests can detect a regression, since a type-based classifier is easy to break silently. Reverted the raw-image site to NotImplementedError and test_raw_image_schema_classifies_as_source_unsupported failed. Good, that's the property that matters.
One gap, not blocking. transform.py:510 refuses a camera topic that mixes jpeg and png with a bare ValueError, so it still lands in infrastructure, while the unsupported-format refusal one line later is now source-unsupported:
mixes image formats -> infrastructure
unsupported single format -> source-unsupported
Same channel, same kind of complaint about its contents, two different kinds. Not in the issue's list of four so you were right to scope it out, but it's the one place the new vocabulary is inconsistent. I'll file it.
Summary
classify_ingest_failure()now files a transform's content refusal (non-conforming pass-through video, a raw-image schema, an unsupported compressed-image format, or a derived-topic collision) under a newIngestFailureKind.SOURCE_UNSUPPORTED, instead of the catch-allinfrastructurebucket.SourceNotFoundstill classifies assource-missingand anyMcapErrorstill classifies assource-unreadable.Why
Per #203:
infrastructureis what a retry is for, but a content refusal fails identically on every retry, so an operator draining the ledger by kind retries a batch that cannot succeed, and a converter author debugging their own output is told the fault is on our side. This implements shape (1)+(2) from the issue, which the issue itself notes compose: a dedicatedSourceNotConforming(ValueError)exception, raised at the transform's content-refusal sites (previously plainValueError/NotImplementedErrorwith the same messages), classified into the newsource-unsupportedkind.docs/CATALOG.mdanddocs/how-to/write-a-converter.mdare updated to document the new kind; the write-a-converter guidance that told converter authors to bypassfailure_kindentirely is rewritten now that the kind is actionable.Scope was kept to exactly the four refusal categories the issue names. Left untouched (still plain
ValueError, stillinfrastructure): transform config validation (crf/gop_seconds/chunk_size), decoder/encoder-resolution failures, "mixes image formats", and "derived topics must be unique" (that one is caller-config, not source content).Validation
Ran inside WSL2/Ubuntu on the Linux filesystem, per CONTRIBUTING.md:
Every failure/error in the full run traces to
PinnedDownloadError/FfmpegNotFoundError— the pinned ffmpeg build's release URL 404s in this sandbox with no network path to it — confirmed unrelated to this change by running one such failing test in isolation and inspecting the traceback (src/hflow/ffmpeg/_binary.py:184). No failure touches a file this PR modifies.Checklist
uv run ruff check --fix,uv run ruff format, anduv run ty check.Investigated and implemented with AI assistance (Claude Code — Claude Fable 5 for the initial repo/issue investigation, Claude Sonnet 5 for the implementation and verification), under my direction and review; every change and the WSL2 test run above were reviewed by me before opening this PR.