fix(zarr_writer): shard edge must be >=1 for empty annotations

[ElmoPA]

An episode with no language annotations was written with shards=(0,), which
zarr v3 rejects (chunk edge must be >= 1). The array still reads back fine, so
training is unaffected (the loader takes its key list from attrs["features"]
and addresses arrays directly), but any group enumeration -- group.keys(),
.members(), tree walks, external tooling speaking the zarr group API -- raises
"Dimension 0: integer chunk edge length must be >= 1, got 0".

chunks= already clamped with max(1, n); shards= was missed. shape=(0,) with a

=1 edge is the correct encoding of empty and writes no chunk objects either way.

Affects 2,541 already-written episodes (2,538 aria + 3 trace); those are left
as-is (metadata-only defect, no functional impact on our pipeline).

[ElmoPA]

• fix(zarr_writer): shard edge must be >=1 for empty annotations #584 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[github-actions]

Claude Code Review

Review

Summary

Empty diff was provided, so I can only review based on the PR description. The described fix — clamping the shards= argument to max(1, n) for empty annotation arrays in ZarrWriter — is a correct and minimal change.

Key concerns

• Empty diff: I can't actually verify the change. Please re-post with the diff so I can confirm the fix is applied at the right call site and matches the chunks= clamp pattern.
• No test mentioned: This is a spec-compliance bug in the zarr writer. It should have a regression test — e.g., write an episode with zero language annotations, then call zarr.open_group(...).keys() / .members() and assert no exception. Without it, this class of bug will recur the next time someone adds a variable-length feature.

Suggestions

1. Add a unit test in the ZarrWriter test module: episode with empty annotations → open group → enumerate members. This is a one-liner regression guard.
2. Audit ZarrWriter for other places that pass a size-derived value into shards= or chunks= without the max(1, n) clamp. If annotations were missed, other optional/variable-length features (e.g., empty tactile, missing wrist cam frames) may have the same bug. A single helper _edge(n) = max(1, n) used everywhere would prevent recurrence.
3. Decision on the 2,541 existing episodes is reasonable given the metadata-only impact, but please document this in the writer's docstring or a KNOWN_ISSUES note so future consumers using group enumeration (external tooling, dataset export scripts) know to expect it and can rewrite lazily if needed.
4. Confirm this doesn't perturb norm stats or any dataset hash used by training configs — should be a no-op since the on-disk chunk objects are identical (none written either way), but worth stating explicitly.

Verdict: Comment

The fix as described is correct and low-risk. Blocking on: (a) actual diff, and (b) a regression test. Once those are in, this is an Approve.

---

Reviewed by Claude · Review workflow