test(lerobot): pin prepared-manifest and the converter version it carries - #388
Conversation
…rries prepared-manifest.json was asserted only for existence, so every field was unpinned - including converter_version, which is part of episode identity rather than decoration. The manifest is now parsed and asserted field by field (schema_version, dataset repo_id/revision/license, camera_keys, episodes_converted, converter_version), converter_version is checked against prep.CONVERTER_VERSION rather than a literal string, and the same version is asserted in the canonical episode's episode/v1 and source-provenance/v1 metadata records. Each of the three writers was neutered in turn and the matching test failed; bumping the constant value alone still passes. Closes Hebbian-Robotics#386 Signed-off-by: ayam04 <ayam04@users.noreply.github.com>
write_canonical_episode is stubbed to a copy, so the test pins what the importer writes rather than what survives canonicalization. Both transform-side mutations that would drop these records leave it green. Recorded in the docstring with the two routes the real transform takes, since episode/v1 is skipped by the verbatim copy loop and rewritten.
kstonekuan
left a comment
kstonekuan
left a comment
There was a problem hiding this comment.
LGTM, merging. Your RED/GREEN matrix reproduces exactly, all four rows:
manifest converter_version dropped ..._returns_local_uris_and_keeps_cache_beside_landing
episode/v1 dropped ..._reaches_the_canonical_episode_provenance
source-provenance/v1 dropped ..._reaches_the_canonical_episode_provenance
constant bumped alone 45 passed
Asserting the manifest as a whole dict rather than field by field is the better choice and I would not have asked for it. It pins absence as well as presence, so a field appearing is a failure too. That matters immediately: #389 adds per-episode entries and moves schema_version to 3, so this test is what will make that a deliberate change rather than a silent one.
I pushed one fixup, 74afbe2, and it is a docstring rather than a behavior change. The test does not exercise canonicalization, so it pins what the importer writes rather than what survives the transform. Both of these leave it green:
canonicalization stops carrying episode/v1 through 45 passed
canonicalization stops copying unknown records verbatim 45 passed
Stubbing there is right and it is the sibling tests' idiom, so this is a boundary worth naming rather than closing. One correction to your reasoning while it is on the record: the passthrough is not verbatim for both records. transform.py:884-886 skips episode/v1 and provenance/v1 before copying, so source-provenance/v1 rides the verbatim loop only because its name is not provenance/v1, and episode/v1 is rebuilt at :889-890 from a dict copy taken at :787. Both survive today, by two different routes, and neither route is covered here.
Gate on the merged result: ruff check, ruff format --check, ty check clean, 1490 passed / 6 skipped. Your five failures were environment. #385 fixed the non-UTC-host one about three hours after you opened this, and the ffmpeg filter_script and module-form CLI failures are WSL, not you.
Closes #386. One ask, and it is an invitation rather than a rule: good first issue exists to give newcomers somewhere to start, and at twenty-odd PRs you are well past needing it. I filed #386 eight minutes before this PR appeared, which is faster than any newcomer could have got to it. Please take from the advanced list instead: https://github.com/Hebbian-Robotics/hflow/issues?q=is%3Aissue+is%3Aopen+label%3Aadvanced
Given you wrote the v3 generalization in #230 and the export path in #273, the thing most worth your time is not an issue we wrote. Point HFlow at a real corpus (Egocentric-10K or Egocentric-100K on Hugging Face) and tell us what breaks, what is slow, or what is awkward. #287 came out of exactly that and it has been worth more than anything we would have thought to file.
.gitignore had no trailing newline, so the added entry concatenated onto **/node_modules/* and produced **/node_modules/*.zcode/. That stopped ignoring node_modules entirely: git check-ignore reported packages/hflow-server/ui/node_modules as tracked. Both manifest tests build their receipts inside the convert stub, so dropping content_id or size_bytes from the real _convert_single_episode left the suite green. The real-conversion test now checks the receipt against the object it published. Also updates the two Hebbian-Robotics#388 assertions the schema-3 bump moves, which is what that whole-dict pin exists to force.
* feat(import): per-episode receipts in prepared-manifest.json prepared-manifest.json (schema version 3) now carries an episodes receipt list: every delivered episode's published URI, its content_episode_id content address, and its byte size. A recipient of a prepared corpus can check the delivery against the manifest without re-running the import, and a truncated, missing, or swapped episode is detectable from the delivery itself. The content id is taken inside _convert_single_episode while the canonical file is still on local disk, before storage.publish: for a bucket root, reading the hash back from the published object means downloading our own upload. content_episode_id stays the single hashing implementation. The v2 top-level keys are unchanged, so old readers keep working, and the entries carry everything a future verify command needs, keeping verify purely additive (#379 direction). Documented in docs/how-to/import-lerobot-v3.md. Refs #379 * chore: ignore local .zcode scratch tooling * fix(lerobot): restore the node_modules ignore and pin the real receipt .gitignore had no trailing newline, so the added entry concatenated onto **/node_modules/* and produced **/node_modules/*.zcode/. That stopped ignoring node_modules entirely: git check-ignore reported packages/hflow-server/ui/node_modules as tracked. Both manifest tests build their receipts inside the convert stub, so dropping content_id or size_bytes from the real _convert_single_episode left the suite green. The real-conversion test now checks the receipt against the object it published. Also updates the two #388 assertions the schema-3 bump moves, which is what that whole-dict pin exists to force. --------- Co-authored-by: Kingston <kingstonkuan@u.nus.edu> Co-authored-by: Kingston <kingston@hebbianrobotics.com>
2 participants
Fixes #386
prepared-manifest.jsonwas asserted only for existence (is_file()), so every field was unpinned — includingconverter_version, which is part of episode identity rather than decoration (the v4→v5 move in #383 is exactly the case that needed a net).What changed
tests/test_lerobot_converter.py(+82):test_import_returns_local_uris_and_keeps_cache_beside_landing, the manifest is parsed and asserted as a full dict:schema_version,dataset(repo_id/revision/license),camera_keys,episodes_converted, andconverter_version— the last againstprep.CONVERTER_VERSION, not a literal copy, so it survives legitimate version moves.test_converter_version_reaches_the_canonical_episode_provenanceruns the real_convert_single_episodemcap writer (network/ffmpeg stubbed, canonicalization stubbed with a copy exactly like the sibling converter tests, since fake access units are not parseable video: the staged→canonical metadata passthrough is verbatim attransform.py:884-886) and assertsconverter_versionin both theepisode/v1andsource-provenance/v1records of the landed episode.RED/GREEN matrix (DoD 4)
Neutering each of the three writers in turn and running the tests:
converter_version(lerobot.py:739)test_import_returns_local_uris_and_keeps_cache_beside_landingFAILSepisode/v1metadata (lerobot.py:1022)test_converter_version_reaches_the_canonical_episode_provenanceFAILSsource-provenance/v1metadata (lerobot.py:1028)test_converter_version_reaches_the_canonical_episode_provenanceFAILSv5→v5-testbump)Validation
uv run pytest tests/test_lerobot_converter.py→ 45 passeduv run ruff check/ruff format --check/ty check→ cleanuv run pytest -q(full suite) → 1466 passed, 6 skipped, 5 failed — all reproduced on a clean main worktree (stash-proven: none touch this change):test_cli.pymodule-form ×3,test_end_to_end.py::test_canonical_episode_extracts_exact_source_frame_indices(the known WSL ffmpeg filter_script failure), andpackages/hflow-servernon-UTC-host TZ test. Environment-only, zero overlap with this diff.