doctor labels a healthy video corrupt when a decoder is unavailable

[Sagar-024]

Current behavior
The doctor video check reads messages inside one broad try. When the machine cannot decode a stream, meaning there is no decoder available for that video, the read raises inside the loop and is reported as:

read-failed: reading messages failed (corrupt chunk or bad CRC?)

The file is intact. Every byte validates. The machine simply cannot decode it. But the diagnostic asserts "corrupt chunk or bad CRC," which points the operator at data corruption when the real cause is a missing capability.

Controlled repro

• Take a known-good video that passes doctor on a machine with the codec.
• Make that codec unavailable on the machine.
• Run hflow doctor. The intact file reports read-failed, corrupt chunk or bad CRC, even though CRCs validate and nothing is damaged.

Why it matters
This is a dishonest diagnostic. The finding asserts corruption when the true cause is environment capability. Same honesty line as the provenance stamps: a report should say what is actually true. An operator told "corrupt chunk" will distrust a healthy file. The correct action is to install the decoder, not quarantine the data.

Proposed fix
Separate decoder resolution from the read. Catch the decoder-unavailable case at codec resolution and report it as its own finding, for example video-decoder-unavailable, naming the stream. Reserve read-failed for genuine read and CRC errors. Small diff, one test.

Definition of done

• A missing decoder reports video-decoder-unavailable, not read-failed
• A genuine corrupt or CRC-failed read still reports read-failed
• A decodable healthy video reports clean
• Mutation: make decoder resolution always fail, confirm the new finding fires and read-failed does not
• Existing doctor tests unaffected

Scope note
This only splits the decoder-unavailable case out of the broad read handler. It does not change CRC validation, the severity model, or any other doctor check. Claiming this, will build once accepted.

[kstonekuan]

Accepted. The structure is exactly as you describe: _resolve_video_decoder is called at src/hflow/doctor.py:381, inside the same try that ends at :389 with

except Exception as error:
    collector.add(
        DiagnosticLevel.ERROR,
        "read-failed",
        f"reading messages failed (corrupt chunk or bad CRC?): {error}",
    )

and the resolver's own ValueError at :193 already says the true thing ("has message encoding ... that no available decoder handles"). That accurate message is then wrapped in a sentence asserting corruption. Neither string appears in any test, so nothing holds either.

One correction to the framing, because it changes what the fix should catch. _resolve_video_decoder does not fail because a codec is missing from the machine. It builds a Ros2DecoderFactory and a ProtobufDecoderFactory and asks each whether it handles the channel's message_encoding; it raises when neither does. So the case is an encoding HFlow has no decoder factory for, which is a fact about the file's encoding paired with our supported set, not about installed codecs. Your repro as written, making a codec unavailable, would more likely fail inside decoder(payload) or _check_video_payload further down.

That distinction matters for the fix: catch the resolver's failure specifically rather than reaching for a general "decoder unavailable" concept, or you will end up asserting something the code cannot know. A name like video-encoding-unsupported is closer to what actually happened than video-decoder-unavailable.

Everything else in your definition of done stands. Two additions:

The schema ValueError at :374-376 ("has no readable schema record") is in the same broad handler and has the same problem: a file with a video channel and no schema record is a real defect, but it is not a corrupt chunk. Decide whether it joins the new finding, gets its own, or stays; just do not leave it silently mislabelled next to the one you fixed.

Say which severity the new finding carries and why. Every finding in that handler is currently ERROR. An unsupported encoding on an otherwise intact file might be one, and it might be a warning, since the file is fine and the reader is not. Argue it rather than inheriting ERROR by default.

Assigning to you. Worth doing before #376, since it is the same honesty line and much smaller.

[Sagar-024]

Corrections accepted and reworked in #465.

1. Framing: video-encoding-unsupported adopted. The resolver checks our supported set (ros2, protobuf), not machine codecs. The detail line now states that the file bytes are not implicated.
2. Schema case: measured fact, the in-loop schema raise was dead code. A channel with a dangling schema gets an empty name (never classifies as video) and the reader KeyErrors on it before our loop runs, so the raise could never fire for the state it describes. Moved to a summary pre-pass that reports channel-schema-missing and filters those topics from the read pass, reserving read-failed for genuine read errors. Schemaless channels (schema_id 0) stay legitimate and untouched.
3. Severity: stays ERROR. Doctor certifies episode conformance: a video our decoder set cannot handle means the episode cannot be certified, since every downstream reader uses the same set. A WARNING would certify a file whose video was never checked.