feat(snapshot): record table and copied-asset integrity in format.json (#397)

[VARUN3WARE]

Summary

• Keep dataset snapshot format_version at 1 and add delivery integrity to format.json: each required Parquet table gets path / size_bytes / sha256.
• In copy mode, every local file under assets/ gets the same receipt fields inline (no separate asset manifest); references mode leaves assets empty and does not fetch remote media.
• Add a content_id over the normalized inventory so a deleted member is detectable later without a second format change.
• Document what is and is not guaranteed; no public verifier / CLI / verify-on-export (deferred, same split as prepared-manifest.json records how many episodes were converted but not which ones, so a corrupt or missing episode cannot be detected from the delivery #379).

Closes #397

Test plan

• uv run ruff check
• uv run ruff format --check
• uv run ty check
• uv run pytest tests/test_dataset_snapshot.py -q (16 passed)
• uv run pytest -q (1529 passed, 6 skipped)

Notes

• Follows the accepted direction on Dataset snapshots cannot verify table or copied-media integrity after transfer #397: marker only; verifier purely additive later.
• Inventory content_id width matches prepared-manifest episode content_id from feat(import): per-episode receipts in prepared-manifest.json #389 for a familiar receipt shape.

[kstonekuan]

Holding this one, and the reason is a shape change rather than anything wrong with the work. The receipt content is right: per-file sha256, the inventory content_id that makes a deleted member visible, and references mode leaving assets empty rather than fetching remote media to hash it. Gate is clean here too, 16 passed.

tables changed type under an unchanged format version. It was a name-to-filename mapping and is now a name-to-record mapping:

-            "tables": {
-                "samples": _SAMPLES_TABLE_FILE_NAME,        # "samples.parquet", a string
+        tables[table_name] = _file_integrity_record(file_name, absolute_path)   # a dict

So format_marker["tables"]["samples"] returns "samples.parquet" before this and {"path": ..., "size_bytes": ..., "sha256": ...} after, with format_version still 1. Main's own documentation calls that section the Format contract and says the marker "names each table", so the mapping is published, not incidental. Nothing in the repo reads it besides the tests you updated, but external readers are the entire audience for a snapshot: #397's premise is that this artifact moves independently of the catalog.

This one is partly on me. My direction said added keys are backward compatible and that adding under v1 beats a version break. That was true of adding, and I did not say the obvious corollary: not redefining a field that is already there. Reading it back, it invited exactly this.

The fix, and the shape is yours to pick. Put the integrity block under its own key and leave tables alone:

"tables":    { "samples": "samples.parquet", ... },
"integrity": { "tables": { "samples": { "path": ..., "size_bytes": ..., "sha256": ... } },
               "assets": [ ... ],
               "content_id": "..." }

That keeps the whole thing genuinely additive, which makes your doc sentence about backward compatibility true rather than nearly true, and it keeps the verifier purely a reader. Nesting the per-table receipts under integrity.tables rather than flattening them also leaves room for a future non-table member without another reshuffle.

Two smaller things while you are in there.

_build_snapshot_integrity_marker_fields hashes every copied asset, so copy mode now reads every media byte a second time on export. For a snapshot with real video that is a visible slowdown and nobody has costed it. It is very likely worth paying, but say so in the PR with a rough number rather than leaving it for someone to discover.

The 16-hex truncation on content_id deserves a sentence. Matching #389's width is a reasonable instinct for a familiar shape, but that value identifies an episode while this one is the delivery's integrity digest, and #397 cited Croissant's full-SHA-256 recommendation. The per-file hashes are full length so the exposure is limited to the set digest, and 64 bits is ample against corruption while being weak against deliberate tampering. Either keep it and say which threat it covers, or make it full length; I lean toward full length, since nothing here needs it to be short.

The FileNotFoundError when a required table is missing from staging is fine and I would keep it: staging always writes all seven, so it is an invariant check rather than an error path, and failing loudly there beats publishing a marker that promises a file nobody wrote.

Nothing else needs changing, and none of the test work is wasted; the assertions move to a new path rather than being rewritten.

[VARUN3WARE]

Thanks, reshaped it so this stays additive under format v1:

• Restored tables as the name→filename string map
• Moved receipts under integrity.tables / integrity.assets / integrity.content_id
• Inventory content_id is now full-length SHA-256 (same threat model as the per-file hashes)
• Copy mode still re-reads each copied asset once after the copy to hash it; called out in the docs
  Ready for another look.

[kstonekuan]

The reshape is right. tables is a string map again, so a v1 reader that already knows the format keeps working, and everything new sits under one key it can ignore.

Validated on your branch against current main: full gate clean, 1585 passed / 6 skipped. I went after the one thing that worried me, which is that the new tests recompute their expected digests with the same helpers the implementation uses. That shape usually cannot notice a helper that stops covering something, because both sides move together. Yours does:

• Receipts stop carrying sha256, or stop carrying size_bytes: two tests red.
• Assets left out of the inventory, or the assets/ walk skipped: red.
• Inventory digest reduced to paths only, contents ignored: test_dataset_snapshot_marker_integrity_detects_post_export_mutations red on its own.

That last one is the one that counts, and it works because you mutate the file on disk and let the helper recompute. A contents-blind digest then matches when the test says it must not.

Pushed one commit, docs(snapshot): say the receipt is not a tamper defence. Three changes:

• The _inventory_content_id docstring said the digest covers "deliberate set tampering". It cannot: it travels unsigned inside the format.json it describes, so anyone who edits a table can recompute it to match. What it does give is corruption and accidental loss, which is worth having and is what the tests demonstrate. Left the reasoning in the docstring and added the same caveat to the how-to, since someone deciding whether to trust a delivered snapshot will read the docs, not the source.
• snapshot_module.shutil.copytree in the new test to a plain shutil.copytree. The two existing uses reach through the module because they monkeypatch it; yours does not, and the idiom reads as required when it is copied without that reason.

Nice work on the second pass.