Skip to content

fix(lerobot): read episode metadata from every v3 shard (fixes #293) - #383

Merged
kstonekuan merged 1 commit into
Hebbian-Robotics:mainfrom
akshatpatel64:akshat/lerobot-metadata-shards
Sep 4, 2026
Merged

fix(lerobot): read episode metadata from every v3 shard (fixes #293)#383
kstonekuan merged 1 commit into
Hebbian-Robotics:mainfrom
akshatpatel64:akshat/lerobot-metadata-shards

Conversation

@akshatpatel64

Copy link
Copy Markdown
Contributor

Summary

Multi-shard LeRobot Dataset v3 repositories now import every episode with its own video window and its own data. Before this change, two things went wrong once meta/episodes had more than one parquet file:

  • Video columns and video windows were read from the first shard only (lerobot.py:459 and :482 on main). Every episode in a later shard converted with no window, so the whole chunk video was transcoded in place of its slice. With real frame counts that shows up as a frame count mismatch error; when the counts happen to agree, the wrong footage is published.
  • Shards were cached under their basename (:413). A second chunk directory that reuses file-000.parquet looked already downloaded and was skipped, and the first shard was read twice. Two episodes became four published files, one of them stamped source_episode_index=1 with episode 0's task and data, and prepared-manifest.json counted four.

Measured on lerobot/droid_1.0.1 (v3.0, 95,658 episodes, seven metadata shards, revision 0eabc778), running the first two shards through _ensure_source_archive: 14,152 of 28,305 episodes had no video window on main, 0 with this change. I measured two shards, not seven; every episode outside shard 0 is affected, so the full corpus would lose roughly 81,500 windows.

Why

Both defects come from the same assumption, that meta/episodes is one file. The fix removes the assumption in the two places it lived:

  • _episode_metadata_cache_path keeps the tree path below meta/episodes in the cache instead of flattening to the basename. The tree listing is remote input, so an entry that is absolute, contains .., or sits outside meta/episodes/ is refused with an error that names the repository, matching the other Hugging Face boundary errors in this module.
  • Episode rows, video columns, and video windows come from one read_parquet([...], union_by_name=true) relation over every shard. That also drops the per-file loop and the post-hoc sort.
  • CONVERTER_VERSION moves to lerobot-converter-v5, following fix(lerobot): separate video cache files by index #353. Multi-shard corpora previously published episodes with the wrong window or the wrong source episode under the same output names, so those outputs should not share an identity with corrected ones. TRANSFORM_BEHAVIOR_VERSION does not move: the transform still writes identical bytes for identical input. This does re-mint content IDs for single-shard re-imports such as pusht; if you would rather keep v4 and treat the old multi-shard outputs as simply wrong under it, I will change that.
  • One sentence in docs/how-to/import-lerobot-v3.md.

Tests: test_index_discovery_reads_every_metadata_shard is parametrized over both shard layouts (distinct basenames in one chunk, which is droid's shape, and the same basename in the next chunk). It asserts each shard downloads once to its own cache entry and is reused on a second discovery, that episode indexes come back as [0, 1, 2, 3], and that every episode carries its own per-camera window. On main the first layout fails on the missing window and the second on the skipped download, so each defect is pinned on its own. test_episode_metadata_cache_path_refuses_entries_outside_the_metadata_tree covers the trust boundary.

Alternatives I did not take: running the video-window query per shard works but leaves two places that must agree on the shard list. Deduplicating episode_rows by episode_index would hide the collision instead of fixing it and would mask a genuinely malformed corpus. Reusing hflow.storage._validated_relative_key for the path check would work, but its message talks about storage keys. Stale flattened cache files from earlier runs are left alone; they are small and no longer read.

Validation

uv run ruff check --fix                                   # All checks passed!
uv run ruff format                                        # 212 files already formatted
uv run ty check                                           # All checks passed!
uv run pytest -q tests/test_lerobot_converter.py          # 44 passed
uv run --python 3.14 --locked pytest -q                   # 1 failed, 1453 passed, 11 skipped
uv run --python 3.11 --locked pytest -q                   # 1 failed, 1453 passed, 11 skipped

The one failure is pre-existing and unrelated: test_preview_stats_render_timestamps_in_utc_on_a_non_utc_host asserts "-04" not in bound_value and fails on the fourth day of any month because the date itself contains -04. It fails the same way on main today (1 failed, 1447 passed, 11 skipped). Happy to send a one-line fix separately once this PR is closed out, given the one-open-PR rule.

Checklist

  • I added or updated outcome-focused tests for changed business logic.
  • I updated documentation for changed behavior, flags, formats, or requirements.
  • I ran uv run ruff check --fix, uv run ruff format, and uv run ty check.
  • I ran the relevant pytest suite.
  • I did not add recordings, generated media, credentials, private URLs, or runtime artifacts.
  • I preserved stored-data compatibility or documented an explicit version change (CONVERTER_VERSION v4 to v5; canonical format and TRANSFORM_BEHAVIOR_VERSION unchanged).

…n-Robotics#293)

Dataset v3 splits meta/episodes into chunk-XXX/file-YYY.parquet shards by
size. The importer cached each shard under its basename, so a second chunk
directory reusing file-000.parquet looked already downloaded and its
episodes vanished while the first shard was read twice. It also discovered
video columns and video windows from the first shard only, so every episode
in a later shard converted without its window and fell back to the whole
chunk video.

Keep the tree path below meta/episodes in the cache, refusing entries that
would escape it, and read episode rows, video columns, and video windows
from one relation over every shard. Bump CONVERTER_VERSION to v5 so outputs
prepared under the old behavior do not share an identity with corrected
ones.

Measured on lerobot/droid_1.0.1 (v3.0, 95,658 episodes, seven metadata
shards) with its first two shards: 14,152 of 28,305 episodes had no video
window before, 0 after.
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

👋 Hi @akshatpatel64 — thank you so much for your first contribution to HFlow!

A maintainer will review your pull request as soon as possible. In the meantime:

💡 Tip: one open pull request per contributor at a time. Issues with an assignee are taken; everything else is fair game.

We are excited to have you here and appreciate your help making the project better! 🙌

@akshatpatel64

Copy link
Copy Markdown
Contributor Author

CI note: both checks jobs fail on one pre-existing test, test_preview_stats_render_timestamps_in_utc_on_a_non_utc_host. It asserts "-04" not in the bound value, and the runner's date is 2026-09-04, so the date string itself trips it. It fails the same way on main today and passes on any day that is not the 4th. Everything else is green: 1458 passed on 3.11 and 3.14. I kept this PR to #293; if you would rather I fold a one-line fix for that assertion in here instead of a follow-up, say so and I will push it.

@kstonekuan kstonekuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, merging. Welcome, and measuring the defect on droid_1.0.1 before fixing it is the right way to open a first PR.

The parametrization is doing real work rather than decorating the test. Each defect fails on its own shard layout:

mutation                                   fails
cache keyed by basename again              [chunk-001/file-000.parquet] only
episode rows from the first shard only     both layouts
video windows from the first shard only    both layouts
traversal refusal removed                  all four refusal cases

The first line is the point: the collision only exists when a later chunk reuses a basename, so a test built solely on droid's shape (distinct names in one chunk) would have missed it entirely. Keeping the tree path below meta/episodes instead of flattening is the fix for the cause rather than the symptom.

Keep v5. You asked whether to bump or treat the old multi-shard outputs as wrong under v4. Bump. A version constant cannot be conditional on how many shards a corpus happens to have, so the choice is between re-minting single-shard IDs that did not need it and letting a corrected output share an identity with a wrong one. The first costs a re-conversion; the second means a cache hit can serve footage from the wrong episode, which is the bug you are fixing. TRANSFORM_BEHAVIOR_VERSION staying put is right for the same reason: the transform's bytes did not change.

One observation, not a request. Moving column discovery onto the union relation is untested: pointing it back at the first shard leaves the whole suite green, because both shards in the fixture carry the same cameras. Real v3 shards are size-splits of one table and share a schema, so this is defensive rather than load-bearing, and I would not add a fixture asymmetry just to pin it. Worth knowing it is not covered.

Separately, converter_version is not asserted anywhere in the suite, so nothing catches an accidental change to a string that is part of episode identity. That is ours, not yours, and predates this.

Your date-sensitive failure report was right and it was ours: "-04" not in bound_value also matches the day in 2026-09-04 and the month in any April date. Fixed in #385, which credits you and @victorwon2001 for catching it independently. No need to send the one-liner.

Gate on the merged result: ruff check, ruff format --check, ty check clean, 1469 passed / 6 skipped, with the same #385 failure and nothing else.

Closes #293. If you want another, #191 is a good fit for what you have just been through: a reproducible real-camera LeRobot import through quality, curate, and export, and nobody is on it. The other thing worth your time, given you already have droid_1.0.1 in hand, is pointing HFlow at a real corpus and reporting what breaks, what is slow, or what is awkward. #287 came out of exactly that and it is more useful to us than any issue we could write ourselves.

@kstonekuan
kstonekuan merged commit 623f9f6 into Hebbian-Robotics:main Sep 4, 2026
6 of 8 checks passed
kstonekuan added a commit that referenced this pull request Sep 4, 2026
…hes the date (#385)

test_preview_stats_render_timestamps_in_utc_on_a_non_utc_host asserted
"-04" not in the rendered bound to catch the host's -04:00 offset. The
substring also matches the day in 2026-09-04 and the month in any April
date, so the test failed on the 4th of every month and through all of
April regardless of the code under test. It is failing on main today.

The bounds are now compared to the row timestamps and to the wall clock
instead. That drops the date collision and covers more: a render that
shifted the clock into the host zone while still stamping +00:00 passed
both the old substring check and a rows-to-stats comparison on its own,
because rows and stats go through the same projection and shift together.

Reported independently by @akshatpatel64 (#383) and @victorwon2001 (#384).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants