Hebbian-Robotics / Hebbian-Robotics/hflow

[Good First Issue]: LeRobot importer crashes with a bare KeyError when a meta/episodes tree entry has no "path"

Open Beginner friendly
#544 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
269
Forks
150
Avg merge
8h 18m
Merged PRs (30d)
246

Description

**Version or commit:**

Current main

**Environment:**

Ubuntu 24.04 (WSL2), Python 3.12, x86_64

**Minimal reproduction:**

1. Monkeypatch `prep._hf_tree` to return a list containing an entry without a `"path"` key: `[{"type": "file", "size": 4096}]`.
2. Call `import_lerobot_dataset` with any valid repo_id.
3. Observe the crash at `src/hflow/importers/lerobot.py:760`.

**Expected behavior:**

The importer should raise a clean, descriptive `ValueError` naming the `repo_id` and the missing field, matching the codebase's established boundary-refusal pattern. It should not crash with a raw `KeyError`.

**Actual behavior:**

The importer crashes with a raw stack trace:

```
File "src/hflow/importers/lerobot.py", line 760, in _ensure_source_archive
if entry.get("type") == "file" and entry["path"].endswith(".parquet"):
KeyError: 'path'
```

**Additional context:**

Root cause: `_hf_tree` (line 513) deliberately tolerates entries without a string `"path"` using `.get("path")`, but the episode-parquet loop at line 760 assumes the key exists and uses raw subscript access (`entry["path"]`).

Why it matters: Every other consumer of the same tree response guards this gap. `_fetch_info_json` uses `.get("path")` (line 553), and `_episode_metadata_cache_path` (lines 594-617) raises a clean `ValueError` for unsafe paths. This is the single unhardened site in the file.

Fix direction: At the top of the loop, replace `entry["path"]` with a guarded read. Validate `tree_path = entry.get("path")` is a non-empty string, else raise a `ValueError` matching the module's phrasing (e.g., `"Hugging Face tree response for {repo_id} lists an entry with no usable 'path'"`). No new exception types needed.

Definition of done:

- Add a test in `tests/test_lerobot_metadata_refusals.py` following the existing pattern (stub `_hf_repo_info` + `_fetch_info_json`, monkeypatch `prep._hf_tree`).
- Assert `pytest.raises(ValueError, match=...)` on the path-less entry.
- Ensure `_assert_no_dataset_output(output_dir)` confirms nothing was published.

Contributor guide

Open the contributing guide

Research direction

Start in src/hflow/importers/lerobot.py at _ensure_source_archive around line 760, and compare its tree-entry handling with _hf_tree and _fetch_info_json. Follow the existing refusal tests in tests/test_lerobot_metadata_refusals.py, then run the targeted test to confirm a path-less entry raises the expected ValueError and _assert_no_dataset_output confirms nothing was published.

Written by the indexing model from the issue text.

Assessment

Tech stack
huggingface, python
Domain
data-engineering, testing
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.