check lanes re-certify a decayed canonical episode: post-sync reads never validate chunk CRCs

[Sagar-024]

Version or commit

ff1e218 (current main)

Environment

Ubuntu 24.04 (WSL2), Python 3.12, x86_64

Summary

Every post-sync read of a canonical episode runs with chunk CRC validation off. Episode._reader opens the file through open_reader(path) (src/hflow/episode.py:311), whose default is validate_crcs=False (src/hflow/reader.py:296), and every check consumes that reader. So the META lane, the relabel lane, and the online re-check flow re-read a canonical episode that has decayed on disk since sync and re-certify it: checks stamp measured findings over bytes that fail the file's own integrity stamp. The reader docstring justifies the default with "already identified by content hash", but that hash proves the source at sync time, not the state of the file at re-read time. #426 and #462 established the opposite rule for sources: file existence is not proof the bytes are good.

Measured repro

On current main. Flip the stored uncompressed_crc of the first chunk of a published canonical episode (real-world header rot: the payload decompresses fine, the bytes no longer match the file's own stamp), then run the production re-check lane:

import io, sys
from pathlib import Path
sys.path.insert(0, "tests")
import hflow
from hflow.testing import SyntheticEpisodeSpec, synthesize_episode
from mcap.reader import make_reader
from mcap.stream_reader import CRCValidationError
from hflow.steps import Stage

tmp = Path("/tmp/check-crc-repro")
tmp.mkdir(parents=True)

source = synthesize_episode(tmp / "source.mcap", SyntheticEpisodeSpec(duration_s=4.0))
app = hflow.App("emi", data_root=tmp / "data")
report = app.process(source, verbose=False)
clean = [(c.check.name, c.status.value) for c in report.checks]
print("healthy canonical:", clean)

canonical = sorted(tmp.rglob("*.canonical.mcap"))[0]
data = bytearray(canonical.read_bytes())
summary = make_reader(io.BytesIO(bytes(data))).get_summary()
crc_offset = summary.chunk_indexes[0].chunk_start_offset + 33  # chunk: opcode(1) len(8) start(8) end(8) usize(8) crc(4)
data[crc_offset] ^= 0x01
canonical.write_bytes(bytes(data))

# A CRC-validating read refuses the file:
with canonical.open("rb") as stream:
    try:
        for _ in make_reader(stream, validate_crcs=True).iter_messages(log_time_order=False):
            pass
    except CRCValidationError as error:
        print("provably damaged:", error)

# The check lanes do not:
report2 = app.process(source, stages=[Stage.META], verbose=False)
print([(c.check.name, c.status.value) for c in report2.checks])

Observed on ff1e218: the validated read raises CRCValidationError: crc validation failed in Chunk, expected: 3695898951, calculated: 3695898950, and the META lane then stamps identical measured findings for every check. Zero findings mention integrity. The damaged episode is re-certified.

Why it matters

The catalog's check rows are the evidence downstream curation and export trust; default_dataset_sql selects on status plus settled checks. Re-certifying a decayed episode puts fresh, confident QC rows over bytes the file itself says are wrong, and the dataset ships with them. The two worst shapes:

• Payload damage that decompresses silently (zstd does not always refuse) is measured as if intact. Only the chunk CRC knows, and the check lane never asks.
• Payload damage that zstd refuses crashes the whole META task with a raw ZstdError: decompression error, no diagnosed finding, no per-episode isolation; a batch run loses the episode to an undiagnosed exception instead of a finding.

#426 fixed this species on the resume path, #462 on the primary read. The post-sync reads are the remaining link in the same chain, and they are the reads users repeat most.

Proposed fix

One line, at the trust boundary that already exists:

# src/hflow/episode.py:311
return open_reader(self.path, validate_crcs=True)

The check rides on decompression that was already happening, so the added cost is a CRC pass over bytes already in memory; #462's commit message carries the same measurement argument. The reader docstring's own rule ("pass True only when reading a source that has not been trusted yet") supports it: a file that has been sitting on disk since its last sync is an untrusted source again. If the maintainer prefers keeping Episode unrestricted, the narrower cut is validating at the META lane entry in stage_execution; the episode-default fix is the smaller diff and the honest boundary.

Definition of done

• A canonical episode whose stored chunk CRC no longer matches is refused by the check lanes with a diagnosed error, not re-certified
• A healthy canonical runs the check lanes unchanged
• The repro above becomes the pinning test
• Mutation: revert the one line, confirm the test goes red, restore, green
• Existing check and stage tests unaffected

Scope note

No change to exit codes, the catalog schema, sync semantics, or any check's logic. One read flag plus one test.

[kstonekuan]

The core holds: checks read canonicals with validation off (episode.py:311, reader.py:296), so a decayed file gets re-certified.

Two corrections. Per-episode isolation already exists (app.py:2490, stage_execution.py:317), so a ZstdError does not take down the batch.

And that is why the one-line fix does not reach your definition of done: with validate_crcs=True on Episode._reader, every check errors and app.py:2490 records each as infrastructure, never a quality outcome. You get N tracebacks and no diagnosis.

Validate once where the lane opens the episode and refuse there with a named reason. CRCValidationError subclasses ValueError, not McapError.

Labelled advanced. Yours if you want it.

[Sagar-024]

on to these will start working on these now .....

[Sagar-024]

PR up: #502. Front-door guard covering all three lanes (META, relabel, production re-check), one named diagnosis, exit code 1 in the runtime-finding family. Cache carries a size+mtime witness; scope note from the issue is superseded by the front-door shape.