fix: validate chunk CRCs when reading canonical episodes after sync - #477
Closed
mulkakhileshmj wants to merge 1 commit into
Closed
mulkakhileshmj wants to merge 1 commit into
mulkakhileshmj wants to merge 1 commit into
Conversation
Episode._reader opened canonical files with CRC validation off, so the META, relabel and re-check lanes could re-certify an episode that had decayed on disk after sync. The checks stamped fresh measured findings over bytes that no longer match the file's own integrity stamp. Episode._reader now opens with validate_crcs=True. On a damaged episode the data reading checks record a diagnosed CRCValidationError per episode instead of fresh evidence, so the catalog never certifies the file. A healthy episode runs the check lanes unchanged, and the CRC pass rides on decompression that was already happening. The open_reader docstring now states the actual rule, and a regression test flips a canonical chunk's stored CRC and asserts the META lane refuses it with the CRC diagnosis while a healthy control stays green. Closes Hebbian-Robotics#474
|
👋 Hi @mulkakhileshmj — thanks for the contribution! To keep starter issues available You already have #476 open, so this one is being closed automatically. |
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.
Closes #474.
Problem
Every post-sync read of a canonical episode ran with chunk CRC validation off.
Episode._readeropened the file throughopen_reader(path), whose default isvalidate_crcs=False, and the META lane, the relabel lane, and the online re-check flow all consume that reader. A canonical episode that decayed on disk after sync was therefore re-read and re-certified: checks stamped freshmeasuredfindings over bytes that fail the file's own integrity stamp, and those rows are what curation and export trust.Fix
Episode._readernow opens withvalidate_crcs=True. On a damaged episode every data-reading check records a diagnosedCRCValidationErrorper episode instead of fresh evidence, so the catalog never certifies the file anddefault_dataset_sqlnever ships it. A healthy canonical runs the check lanes unchanged, and the CRC pass rides on decompression that was already happening (#462 carries the cost measurement).I audited the other
open_readercall sites:app.py(sync-reuse witness),importers/lerobot.py, andruntime/_templates.pyread only summary and metadata records, which chunk CRCs cannot vouch for, so they keep the default. Theopen_readerdocstring now states that rule instead of the "already identified by content hash" claim the issue debunked.No
TRANSFORM_BEHAVIOR_VERSIONbump: the transform writes identical bytes, only reads changed.Coverage
New regression test in
tests/test_processing_regressions.pyfollowing the issue's repro: process a synthesized episode, flip one bit of the first chunk's storeduncompressed_crc(helper added totests/reuse_test_helpers.pybeside the existing corruption helper), and assert the META re-run reportscontent_digestasERRORwith the CRC diagnosis and stamps no fresh measurements, while a healthy control run stays fullyMEASURED.Validation
Run on WSL2 Ubuntu, Python 3.12:
Mutation check per the issue's definition of done: with the one-line fix reverted the pinning test fails; restored, it passes. The touched test file also passes under Python 3.11.