Repository navigation
A transform refusal about the recording is filed as an infrastructure failure #203
Description
What happens
classify_ingest_failure (src/hflow/ingest_ledger.py:80) sorts an ingest failure into one of
three kinds. It returns source-missing for SourceNotFound, source-unreadable for any
McapError, and infrastructure for everything else.
Everything else includes every refusal the transform makes about the file's contents:
- non-conforming pass-through video (
formatis not h264, more than one picture per message, a
keyframe without SPS and PPS, a channel starting mid-GOP) - a raw-image schema (
sensor_msgs/msg/Image,foxglove.RawImage) - an unsupported compressed-image format
- a derived topic colliding with a source topic
Each of those is a ValueError naming exactly what is wrong with the recording, and each is
filed under a kind that says the problem is with the machine.
Why it matters
The infrastructure kind is what a retry is for. A corrupt or unsupported recording will fail
identically on every retry, so an operator draining the ledger by kind retries a batch that
cannot succeed, and a converter author debugging their output is told the fault is on our side.
docs/how-to/write-a-converter.md (added in #202) currently routes around this by telling
converter authors to read error_type and message and ignore failure_kind. That is good
advice for today and a poor place to leave it: the column exists so nobody has to parse
messages.
Why it is not just a missing branch
classify_ingest_failure's own docstring says the classification is a heuristic, stored beside
the verbatim error_type and message so a wrong guess stays visible. That design is
deliberate and I think worth keeping. The question is whether the heuristic can be improved without
becoming a list of exception messages, which is the thing it was written to avoid.
Three shapes, and the choice is not obvious:
- A dedicated exception type for source-content refusals. The transform raises something
likeSourceNotConforming(ValueError)for the refusals above, and the classifier tests for
it the way it already tests forSourceNotFound. Most precise, and it means the transform
states the classification at the point that knows it. Costs a new public exception name and
touching every raise site. - A fourth kind,
source-unsupported, distinguishing "this recording is not something v1
can ingest" from "this recording is not readable at all". Useful to an operator, since the
first is a converter bug and the second is a corrupt file. Needs the same reclassification
work as (1) to route anything into it. - Leave the kinds and document them. Defensible, and it is where docs: document input MCAP converter contract #202 left things, but it
makesfailure_kinda column nobody can act on.
(1) and (2) compose: a new exception type could route to a new kind.
Definition of done
Whichever shape is chosen: a transform refusal about the source file's contents is not
classified infrastructure, an McapError still classifies as source-unreadable, and
SourceNotFound still classifies as source-missing. A test per refusal category above pins
the kind it lands in. docs/how-to/write-a-converter.md's paragraph about reading error_type
instead of failure_kind is rewritten or removed to match. If a new kind is added,
docs/CATALOG.md's ingest-failure section documents it.
Validation
uv sync --locked --all-extras
uv run ruff check --fix
uv run ruff format
uv run ty check
uv run pytest -q
Run plain uv run pytest -q from the repo root: pytest tests alone misses
packages/hflow-server/tests. Markdown changes also want the lychee command from
CONTRIBUTING.md.
Notes
- Not a good first issue: it adds vocabulary to a stored column, so the shape wants agreeing
first. - Surfaced by @tunglambk while writing docs: document input MCAP converter contract #202, whose page documents the current behaviour
accurately. Nothing there needs reverting.
Activity
aleks-drozy commentedon Aug 30, 2026
Hi, I'd like to work on this — planning to add a
SourceNotConformingexception and a newsource-unsupportedclassification kind, mapped inclassify_ingest_failure()and excluded from retry logic, per the direction in the issue. Will follow up with a PR.kstonekuan commentedon Aug 30, 2026
Assigned, go ahead.
Two things to settle before the code, since both are easier now than after.
failure_kindis a stored column, so a fourth value splits the corpus: rows written before your change keepinfrastructurefor refusals that'd now besource-unsupported, and nothing rewrites them. Say in the PR what an operator draining an old ledger should expect, and whether anything needs to say so out loud.The other is the boundary. The issue lists four refusals that belong on the recording, not the machine, and the classifier has to tell them apart from a genuine
ValueErrorraised by a bug in our own code. A new exception type is the right instrument. What matters is that raising it is the only way into the new kind, so a strayValueErrorcan't drift in later. Say how you got that.Prove the classification with a test that fails if a refusal goes back to
infrastructure, and keep the existing three kinds behaving exactly as they do.aleks-drozy commentedon Aug 30, 2026
Opened as a draft: #288
docs: address cutover and boundary questions from Hebbian-Robotics#203 …
aleks-drozy commentedon Aug 31, 2026
Thanks — answering both before the review, and pushed a follow-up commit (78b59c1) covering them in the PR:
Old ledger rows: they don't get reclassified.
ingest_failuresis append-only, so a refusal recorded asinfrastructureby an ingest binary from before this change staysinfrastructureforever — nothing rewrites past rows. An operator draining an old ledger by kind should expect pre-cutoverinfrastructurerows to be a mix of genuine infra trouble and content refusals;error_type/messageare the ground truth for any individual row regardless offailure_kind, which is exactly why they're kept verbatim beside it. Only rows written by a binary that already raisesSourceNotConformingcan carrysource-unsupported. Spelled this out indocs/CATALOG.md.Boundary / the only way in:
classify_ingest_failureroutes toSOURCE_UNSUPPORTEDvia a singleisinstance(error, SourceNotConforming)check — no string-matching, no other predicate. So a future content refusal that's raised as a bareValueError(a bug, or someone forgetting to use the new type) falls back toinfrastructure, the safe default, rather than silently landing insource-unsupportedby accident. Made this explicit inSourceNotConforming's docstring so it's discoverable at the raise site, not just in the classifier.Test that fails if a refusal regresses to infrastructure:
tests/test_ingest_ledger.pyhas one test per refusal category (unsupported compressed-image format, non-conforming pass-through video, raw-image schema, derived-topic collision) that triggers the real raise site — not a synthetic exception — and assertsclassify_ingest_failure(caught_error) == IngestFailureKind.SOURCE_UNSUPPORTED. If any of those raise sites reverted to a bareValueError, the exception type check would fail and so would the classification assertion. The existing three kinds are covered by their own tests in the same file (SourceNotFound→source-missing,McapError→source-unreadable, unrecognized →infrastructure) and are unchanged.PR is #288 (draft), ready for review.
kstonekuan commentedon Aug 31, 2026
Resolved by #288.
fix: classify transform content refusals as source-unsupported, not i…