docs(plans): record J8 W1 as shipped and correct the format assumptions - #449
Merged
Conversation
W1a/W1b/W1c are merged (#446/#447/#448), so the plan moves to In progress and gains what implementation actually found. Two corrections to the plan's own premises, both from reading real LeRobot metadata rather than the format docs: - v3.0 is the DOMINANT generation now, not the newer edge case; every canonical lerobot/* dataset on HF main is v3.0. §2 read as though v2.1 were the common case. - v2.1 ships no meta/stats.json at all — that file arrives with v3.0 — so dataset-level statistics must be FOLDED from meta/episodes_stats.jsonl. The plan did not anticipate the fold, and it turns out to be the wedge's strongest test: folding a dataset's v2.1 stats reproduces the same dataset's v3.0 stats.json across all 10 features to 2.5e-9. Also recorded: the three shape traps that only real files expose (`names` has three shapes including an object; a v3.0 column name containing a slash is flat, not nested; tasks.parquet's task-string column is named `__index_level_0__` in curated datasets but `task` in freshly recorded ones), and the parquet-go v0.25.0 pin forced by Go 1.23. W1d is scoped honestly: §4 assumed #393's menu machinery covered the Inspect handoff, but InspectTree has only root-level and panel-level context menus — the per-file row menu does not exist and IS the wedge. Two §11 decisions are amended rather than claimed: #4's staleness indicator is half-shipped (the fingerprint is stored, nothing re-stats to compare it), and #1's keyframe assumption is still unverified against real v3 video. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
physercoe
pushed a commit
that referenced
this pull request
Jul 29, 2026
Implementation review of the four overnight lanes (teleport D-7/T2 #424–#428, archgraph W1–W5 #430–#436, transcript P5 #437–#445, J8 Replay W1 #446–#449). Two findings, both fixed here; everything else verified clean — lane-by-lane verdicts recorded in the review notes. - datasets: all four write statements (register INSERT, PATCH, DELETE, digest-refresh UPDATE) ran on the multi-connection READ pool `s.db`. Every other hub write goes through `s.writeDB`, the dedicated one-connection writer pool (server.go: "ALL writes"), precisely so SQLite never sees two writer connections; on the read pool a concurrent registration and digest refresh can hit SQLITE_BUSY. Switched all four. Full hub suite green. - vram: the KV-class calibration comment overstated its two upper-bucket examples by exactly 2× — MiniMax-M2 is ~248 KB/token (2·62·8·128·2, class 'moderate', not 'high') and Gemma-3-27B ~496 KB (2·62·16·128·2, 'high'). The thresholds and tests are correct; only the prose was wrong. Comment-only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
physercoe
added a commit
that referenced
this pull request
Jul 29, 2026
The #424-#449 implementation review found the four `datasets` writes on the multi-connection read pool and moved them to `s.writeDB` (5ae5c5f). This is the same defect one file over, and the only other instance outside tests: `sessionAgentIDs`' read-repair materializes `sessions.agent_ids_json` through `s.db`. It matters more here than the write count suggests, because this write is deliberately best-effort (`_, _ =`). The whole point of the one-connection `writeDB` pool is that SQLite never sees two writer connections; on the read pool a concurrent write can return SQLITE_BUSY, and here that error is swallowed by design. So the failure mode is not a visible error — it is the archived-session cache silently never materializing, and every later digest read paying the full GROUP BY scan that #118 added this column to avoid. Co-authored-by: Ubuntu <ubuntu@ip-172-26-2-239.us-west-2.compute.internal> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
W1a/W1b/W1c are merged (#446/#447/#448), so the plan moves to In progress and gains what implementation actually found.
Two corrections to the plan's own premises
Both from reading real LeRobot metadata rather than the format docs:
lerobot/*dataset on HFmainis v3.0. §2 read as though v2.1 were the common case.meta/stats.jsonat all — that file arrives with v3.0 — so dataset-level statistics must be folded frommeta/episodes_stats.jsonl. The plan didn't anticipate the fold, and it turned out to be the wedge's strongest test: folding a dataset's v2.1 stats reproduces the same dataset's v3.0stats.jsonacross all 10 features to 2.5e-9. Different files, different formats, different decoders — no shared code below the comparison.Shape traps only real files expose
nameshas three shapes in one file:null, a list, and an object. Feature dimension therefore comes fromshape, neverlen(names).data/chunk_indexis one flat name containing a slash, not a nested group. Reading it as a path finds nothing — indistinguishable from "this dataset has no offsets".tasks.parquet's task-string column is__index_level_0__in curated datasets buttaskin freshly recorded ones.Plus the
parquet-gov0.25.0 pin forced by Go 1.23.W1d scoped honestly
§4 assumed #393's menu machinery covered the Inspect handoff.
InspectTreein fact has only root-level and panel-level context menus — the per-file row menu doesn't exist, and building it is the wedge.Two §11 decisions amended, not claimed
as of <ts>plus manual Refresh.ffmpeg -c copynear-free — still has to be checked against real v3 video at W2.Doc-only;
lint-docsclean.🤖 Generated with Claude Code