fix(doctor): report unsupported video encodings and missing schemas separately from read-failed - #465
Open
Sagar-024 wants to merge 1 commit into
Open
Conversation
…eparately from read-failed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(doctor): report unsupported video encodings and missing schemas separately from read-failed
Closes #460.
What changed
Doctor's video check previously ran inside one broad
tryblock. That meant two failures that weren't actually corruption problems ended up being reported asread-failed, along with the misleading "corrupt chunk or bad CRC?" assertion.Unsupported video encodings
_resolve_video_decoderraises when neither supported decoder factory (ROS2 or protobuf) can handle a channel'smessage_encoding.That isn't evidence of corrupted bytes or a machine codec problem. It simply means the file uses an encoding that isn't supported by our current decoder set.
This now reports as
video-encoding-unsupported, including the topic name and a detail message making it clear that the file bytes themselves are not implicated. The resolver now raises the typedVideoEncodingUnsupported, which remains aValueErrorsubclass.Missing schemas
The existing in-loop
schema is Nonecheck was effectively dead code.There are two reasons:
KeyErrorduring iteration before the loop body can reach that check.The missing-schema case is now handled in a summary pre-pass instead. Any channel that references a schema ID not present in the file reports
channel-schema-missing.schema_id == 0remains valid and means the channel is schemaless.Channels with missing schemas are excluded from the read pass using the reader's
topicsfilter, which keepsread-failedreserved for actual read and CRC failures.Severity
Both findings remain ERROR.
Doctor is certifying episode conformance. If a video topic uses an encoding our decoder set cannot handle, the episode can't be fully certified because downstream readers rely on that same decoder support.
Making this a WARNING would allow Doctor to certify a file whose video was never actually checked.
The distinction is reflected in the finding codes and messages, which now describe what actually happened. The severity represents "cannot certify," consistent with findings such as
no-statisticsandno-chunk-indexes.Behavior notes
Tests
23 passedintests/test_doctor.py, including two new real-path tests with no stubs:video-encoding-unsupportedand neverread-failed.99) alongside a healthy sibling reportschannel-schema-missingand neverread-failed, while the sibling's video checks still run.Mutation checks also confirmed the new coverage: removing either the pre-pass or the encoding catch makes exactly the two new tests fail. Restoring them brings everything back to green.
ruff check,ruff format, andtyare all clean.