fix(data list): text datasets showed no size (du measured disk blocks; zero == unknown) - #492
Merged
Merged
Conversation
…, and stop conflating zero with unknown `tb data list` showed no size for EVERY text dataset (text/token classification, masked + causal LM, seq2seq, sentence-pair, embeddings) while image datasets in the same namespace showed real sizes. Records, classes, extension and task were all correct; only the size was missing. Two independent defects compound: 1. MIS-MEASURED. Sizes come from `du -sk <shared>/*` in the jobs-manager pod. Without --apparent-size, du measures st_blocks, and on a bind-mounted host filesystem (Docker Desktop, and any mount that doesn't report block allocation for small files) that returns 0 for a directory of small files. Measured on a real node: mlm_train 0, text_train 0, image_train 180. Image datasets survive only because their files are big enough to occupy reported blocks; text datasets are dozens of small .txt documents, so every one measures zero. Now probes --apparent-size support once (busybox du rejects the flag) and passes it to a single du run. Deliberately NOT `du --apparent-size … || du …`: du exits non-zero if ANY path is unreadable, so that chain would silently fall back to block sizes whenever one dataset dir was inaccessible — reintroducing the bug intermittently, which is harder to diagnose than never having the flag. A test pins that the real du runs exactly once. 2. ZERO == UNKNOWN. sizeCell gated on `SizeBytes > 0`, so a dataset measured at 0 was indistinguishable from one we never managed to measure. That is why this presented as "size missing" rather than "size wrong", and why it looked like a broken lookup when the lookup was fine. DatasetInfo now carries SizeKnown: a measured zero renders 0 B, and only a failed lookup renders —. SizeKnown rather than an inverted SizeUnknown on purpose: if a producer ever forgets the flag, rendering — understates what we know, whereas the inverted field would assert a confident 0 B that is false. Tabular/time-series hid defect 1 by accident — their empty Extension routes them to DBBytes instead of du. Tests: 4 new. Verified both guards fail when their own defect is reintroduced. Existing fixtures that set a size now declare SizeKnown, since they model measured datasets; the copy-catalog golden is UNCHANGED, which is the point — measured datasets render exactly as before and only genuinely-unknown ones change. (My first pass at annotating fixtures missed the two in copy_catalog_test.go because they end in `}` not `,`; reviewing the regenerated golden caught it showing — where real sizes belong, so the golden was reverted and the fixtures fixed instead of accepting the drift.) VERSION -> 0.10.6 (0.10.5 is claimed by #487): version-bump-gate fails any PR touching internal/* while VERSION names an already-tagged release. No tag is cut here. Full suite green; gofmt -s, goimports, vet, errcheck, staticcheck clean. Closes #491 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Asked whether the size fix covered ALL text data, and checking properly found a gap I had left: the JSON view still had size_bytes with no way to tell a MEASURED zero from "couldn't measure". That is the same ambiguity the rendered table had — the one that made an entire modality read as sizeless while the lookup was fine — so fixing only the human view would have left every scripted consumer with the original bug. size_known is emitted always, never omitempty: omitting it when false would recreate exactly the ambiguity it exists to remove. Coverage confirmed while checking, rather than assumed: all seven FamilyText categories (text_classification, masked_language_modeling, causal_language_modeling, seq2seq, token_classification, sentence_pair_classification, embeddings) are fixed by one change, because the du runs once over the whole shared root — it is not per-category. Any small-file dataset of any modality is fixed with them. Test asserts a measured zero reports size_known=true with 0 bytes, an unmeasured one reports false, and the key is present in the payload. Verified it fails when the mapping is dropped. 16 packages green; gofmt + staticcheck clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
saadqbal
approved these changes
Aug 12, 2026
saadqbal
left a comment
Collaborator
There was a problem hiding this comment.
Nice, careful PR 👍 The zero-vs-unknown model holds in every path (measured/measured-zero/unmeasured file dataset, row-based, empty table), table and JSON agree via the same SizeKnown, and probing --apparent-size once instead of a || du fallback is exactly right. Tests are non-vacuous and both guards fail when their defect is reintroduced. go test ./internal/cli ./internal/push green locally. One nit, non-blocking: du -sk rounds apparent size up to KiB, so the sub-KiB "512 B" case only exists in the sizeCell unit test, never from a real measurement — fine, just noting the rendering path is exercised where du never lands it.
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
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.
Closes #491. Found from a live Windows cluster: every text dataset showed no size, while image
datasets in the same namespace showed real ones.
Root cause — two defects that compound
1. The size was mis-measured. Sizes come from
du -sk <shared>/*run inside the jobs-managerpod. Without
--apparent-size,dumeasuresst_blocks, and on a bind-mounted host filesystem(Docker Desktop, and any mount that doesn't report block allocation for small files) that returns
0 for a directory of small files. Measured on the affected node:
Image datasets survive only because their files are large enough to occupy reported blocks. Text
datasets are dozens of small
.txtdocuments, so every one of them measures zero.2. Zero and "unknown" were the same value.
sizeCellgated onSizeBytes > 0, so a datasetmeasured at 0 was indistinguishable from one the CLI never managed to measure. That's why this
presented as "the size is missing" rather than "the size is wrong" — and why it looked like a
broken lookup when the lookup was working fine.
Tabular/time-series datasets hid defect 1 by accident: their empty
Extensionroutes them toDBBytesinstead ofdu.The change
--apparent-sizeonce, then pass it to a singledurun. Deliberately notdu --apparent-size … || du …:duexits non-zero if any path is unreadable, so that chainwould silently fall back to block sizes whenever one dataset dir was inaccessible —
reintroducing this bug intermittently, which is harder to diagnose than never having the flag.
A test pins that the real
duruns exactly once.DatasetInfo.SizeKnownseparates "measured" from the value. A measured zero renders0 B;only a failed lookup renders
—.SizeKnownrather than an invertedSizeUnknownon purpose: if a producer ever forgets theflag, rendering
—understates what we know, whereas the inverted field would assert aconfident
0 Bthat is false.Verification
go test ./...— 16 packages ok. 4 new tests, and both guards verified to fail whentheir own defect is reintroduced.
before, and only genuinely-unknown ones change.
gofmt -s,goimports -local,go vet,errcheck,staticcheck -checks all,-ST1005clean.Two notes for the reviewer, in the interest of not overstating this:
copy_catalog_test.go(they end in}, not,), and the regenerated golden showed—where real sizes belong. Reviewing that diff caughtit — I reverted the golden and fixed the fixtures rather than accepting the drift. Worth knowing
that
TB_UPDATE_GOLDEN=1will happily bake in a regression if you don't read it.VERSIONto 0.10.6 is inaccurate — no bumpis in this diff. develop moved to
0.10.5via fix(interactive): show Windows path examples on Windows (client#615) #487 while I was working, andv0.10.5isn'ttagged, so
version-bump-gatepasses without one. I left the message rather than force-push acorrection.
The
duchange itself runs inside the jobs-manager container, which is where I can't reach — sothe apparent-size behaviour is reasoned from
st_sizevsst_blocksplus the node measurementsabove, not observed in that pod. Worth one
tb data liston a Windows cluster to confirm textsizes appear.
🤖 Generated with Claude Code
Note
Medium Risk
Changes the remote
ducommand run inside jobs-manager and adds a field to the data-list JSON contract. Well-covered by tests, but sizing behavior depends on containerdusupport and should be spot-checked on a Windows cluster.Overview
Fixes
#491where text datasets rendered as sizeless on bind-mounted host filesystems (e.g. Docker Desktop on Windows), while image datasets looked fine.Measurement:
duon the shared PVC now probes for--apparent-sizeand uses it when available, so sizes come fromst_sizeinstead of disk blocks that report0for small files. The realdustill runs once (no||fallback that could silently revert to block sizes).Display/API: Adds
SizeKnownso a measured0renders as0 B(and JSONsize_known: true) instead of conflating with unknown (—/ unmeasured). The same flag is exposed in--output-jsondetails.Reviewed by Cursor Bugbot for commit 3e4e10c. Bugbot is set up for automated code reviews on this repo. Configure here.