Skip to content

fix(preflight): resolve labels.csv filename by column name, not position - #207

Merged
LukasWodka merged 2 commits into
developfrom
fix/preflight-filename-by-name
Jul 10, 2026
Merged

fix(preflight): resolve labels.csv filename by column name, not position#207
LukasWodka merged 2 commits into
developfrom
fix/preflight-filename-by-name

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

CrossCheckLabels (the image_classification preflight that verifies each labels.csv row's image exists under images/) read the filename from rec[0] — the first column, positionally. But the ingestor reads it by name: record.get("filename") over the header-keyed record (file_transfer.py / record_processor.py), order-independent, and every template's labels.csv names the column filename.

So a label,filename header (filename present but not first — a layout the cluster ingests cleanly) made the CLI read the label value ("cat") as the filename, miss every row, and reject with exit 3 ("N labels.csv row(s) reference images that aren't in images/") — a false reject whose message also misleadingly lists label values as missing images. Violated the "never stricter than the ingestor" preflight contract.

(Found by an adversarial bug-hunt; verified against the ingestor's actual read path + templates.)

Fix

Resolve the filename column by name — exact filename first, then case-insensitive + whitespace-trimmed (the ingestor's _match_column rule) — and read that column's index. Falls back to column 0 when there's no filename-ish column (a malformed layout the ingestor fails on regardless — not this check's job to diagnose); short/ragged rows without that cell are skipped.

Tests

  • Existing test's unrealistic image_id,label fixture corrected to the real filename,label.
  • TestCrossCheckLabels_FilenameColumnNotFirstlabel,Filename (not first, mixed case) resolves correctly and does not read labels as filenames (this fails against the old rec[0]).
  • TestFilenameColIndex — first / not-first / case / whitespace / no-filename fallback.

Full go test ./... green; gofmt -s / errcheck / ineffassign / misspell clean.

Type

Bug fix (preflight false-reject; same parity class as #340) · cli · HIGH.


Note

Low Risk
Scoped to CLI preflight parity for CrossCheckLabels; behavior is tightened to match ingestor rules and is covered by new unit tests, with no upload or cluster path changes.

Overview
Fixes false preflight rejects for image_classification when labels.csv does not put the image key in the first column. CrossCheckLabels used to read rec[0]; it now picks the image column the same way the ingestor does: filename by name (exact, then case/whitespace-insensitive), else data_id, with column 0 only when neither exists.

That restores the never stricter than the ingestor contract for layouts like label,filename or label,data_id, where the old check treated label values as filenames and failed with exit 3. Short rows without a filename cell are skipped instead of mis-read.

Tests cover non-first filename, data_id fallback, and imageFileColIndex header matching; the main fixture uses a realistic filename,label header.

Reviewed by Cursor Bugbot for commit 21258c9. Bugbot is set up for automated code reviews on this repo. Configure here.

CrossCheckLabels (image_classification preflight) read the image filename from
rec[0] — the first column, positionally. But the ingestor reads it BY NAME:
record.get("filename") over the header-keyed record (file_transfer.py /
record_processor.py), order-independent, and every template's labels.csv names
the column "filename". So a `label,filename` header (filename present but not
first — a layout the cluster ingests cleanly) made the CLI read the LABEL value
("cat") as the filename, miss every row, and reject with exit 3 ("N labels.csv
row(s) reference images that aren't in images/") — a false reject that also
misleadingly listed label values as missing images. Violated the "never stricter
than the ingestor" preflight contract.
Fix: resolve the filename column by name — exact "filename" first, then
case-insensitive + whitespace-trimmed (the ingestor's _match_column rule) — and
read that column's index. Falls back to column 0 when there's no filename-ish
column (a malformed layout the ingestor fails on regardless; not this check's job
to diagnose). Short/ragged rows without that cell are skipped.
Tests: the existing test's unrealistic `image_id,label` fixture corrected to the
real `filename,label`; new TestCrossCheckLabels_FilenameColumnNotFirst pins that
`label,Filename` (not first, mixed case) resolves correctly and does NOT read
labels as filenames (fails against the old rec[0]); TestFilenameColIndex covers
first/not-first/case/whitespace/fallback. Full suite green; gofmt -s / errcheck /
ineffassign / misspell clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@saadqbalsaadqbal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good fix — reading by column name is right. One gap though: the ingestor resolves filename OR data_id, and we only handle filename. Details inline.

Comment threadinternal/push/preflight.go
Comment threadinternal/push/preflight.go
…ke the ingestor
Addresses @saadqbal's #207 review. The ingestor resolves the image file key as
`filename` else `data_id` (image_paths / image_loader), position-independent for
both — so a valid `label,data_id` CSV (no filename column) was falling back to
index 0 (the label column) and false-rejecting (exit 3) a dataset the cluster
ingests cleanly.
imageFileColIndex (renamed from filenameColIndex) now matches filename-else-data_id
with the same exact-then-case-insensitive-trimmed rule; filename wins when both are
present; it falls back to 0 only when NEITHER exists (a labels.csv the ingestor
rejects at validate_data anyway). Added a label,data_id CrossCheckLabels regression
test + data_id cases to the index test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

Thanks @saadqbal — fixed in 21258c9.

filename-else-data_id (the gap).imageFileColIndex (renamed from filenameColIndex) now mirrors the ingestor's precedence: filename if present, else data_id, each matched exact-then-case-insensitive-trimmed, filename winning when both resolve. So a valid label,data_id CSV resolves the data_id column instead of falling back to index 0 and false-rejecting. Fallback to 0 stays only for the neither-column case (which the ingestor rejects at validate_data regardless — as you noted, fine to leave). Added TestCrossCheckLabels_DataIDColumn (the label,data_id regression) plus data_id / Data_ID / filename-wins cases to the index test.

Truncated-row skip (preflight.go:290) — left, on purpose. A row too short to reach the image-file cell references no image, so there's nothing for CrossCheckLabels (an image-existence check) to verify. The record's own invalidity is IngestableRecordsValidator's concern — the ingestor pads it to NaN and fails that record with a clear per-record error (the exit-9 path), which is a different check than "do the referenced images exist." Flagging it here would mean inventing a synthetic entry in the filename list or a signature change, for a ragged-CSV edge the cluster already reports cleanly. Happy to add a ragged-row guard if you'd rather it fail locally too — your call.

go build / go test ./... / gofmt / go vet green.

@LukasWodka
LukasWodka merged commit 8c06ab8 into developJul 10, 2026
16 checks passed
@LukasWodka
LukasWodka deleted the fix/preflight-filename-by-name branch July 10, 2026 11:17
LukasWodka added a commit that referenced this pull request Jul 21, 2026
…#373)
Asad's review of #373:
- data_id (comment): confirmed against the ingestor source that a data_id-only
image manifest fails at TRANSFER, so this is not a false-reject. The ingest
copy reads record.get("filename") case-sensitively (data-ingestors
record_processor.py:279 -> file_transfer.py:179) with no data_id fallback;
the filename-vs-data_id resolver #207 cited
(prepare_classification_pytorch_image_df) lives in tracebloc-client and runs
at TRAINING time over the already-ingested table, not the ingest copy.
Documented this in the imageFileColIndex doc comment.
- Filename case variant: CheckImageFilenameColumn / imageFileColIndex now match
EXACTLY (trimmed, case-sensitive), mirroring the transfer read, so a
Filename/FILENAME header is rejected up front with a "lowercase it" message
instead of being green-lit into the same doomed upload. Adds the
imgc-uppercase-filename-col parity case (ingestor preflight accepts, CLI
rejects) + unit coverage.
- Decode cost: moved the cheap manifest/header checks (incl.
CheckImageFilenameColumn) AHEAD of the per-image ValidateImages decode, so a
structurally doomed manifest fails before we open a single image.
Removes the now-contradicted TestCrossCheckLabels_FilenameColumnNotFirst
(mixed-case), superseded by the case-sensitive FilenameNotFirst. Goldens
regenerated against the pin (sync-validator-goldens.sh --check in sync).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka added a commit that referenced this pull request Jul 21, 2026
…#371) (#373)
* fix(push): reject image datasets with no "filename" column in dry-run (#371)
Image `data ingest` preflight green-lit a labels.csv whose file column
wasn't named `filename` (e.g. `image_id`): imageFileColIndex fell back to
column 0, read the label values as filenames, found the files, and passed.
The cluster then reads each image via a case-sensitive record.get("filename")
at transfer time (data-ingestors file_transfer.py:179), finds no key, and
fails every row post-upload with "No filename found in record" (exit 9).
The ingestor's own preflight (validate_data) doesn't catch this — it only
surfaces at transfer — so this was a genuine accept-locally / fail-after-upload
parity gap, invisible to the golden harness (goldens are generated from
validate_data, which accepts image_id,label).
- Enforce the contract's requires_filename_column up front for the image
family (CheckImageFilenameColumn), mirroring the text family's check.
- Drop imageFileColIndex's data_id false-accept and column-0 fallback — the
ingestor never aliases another column to filename, so a data_id-only
manifest fails at transfer identically to image_id.
- Correct all 17 imgc-* parity fixtures to `filename`, add imgc-no-filename-col
to pin the divergence (ingestor preflight accepts, CLI rejects the doomed
upload), regenerate goldens against the pin, fix unit-test CSVs + help text.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(push): address review — case-sensitive filename + fail-fast order (#373)
Asad's review of #373:
- data_id (comment): confirmed against the ingestor source that a data_id-only
image manifest fails at TRANSFER, so this is not a false-reject. The ingest
copy reads record.get("filename") case-sensitively (data-ingestors
record_processor.py:279 -> file_transfer.py:179) with no data_id fallback;
the filename-vs-data_id resolver #207 cited
(prepare_classification_pytorch_image_df) lives in tracebloc-client and runs
at TRAINING time over the already-ingested table, not the ingest copy.
Documented this in the imageFileColIndex doc comment.
- Filename case variant: CheckImageFilenameColumn / imageFileColIndex now match
EXACTLY (trimmed, case-sensitive), mirroring the transfer read, so a
Filename/FILENAME header is rejected up front with a "lowercase it" message
instead of being green-lit into the same doomed upload. Adds the
imgc-uppercase-filename-col parity case (ingestor preflight accepts, CLI
rejects) + unit coverage.
- Decode cost: moved the cheap manifest/header checks (incl.
CheckImageFilenameColumn) AHEAD of the per-image ValidateImages decode, so a
structurally doomed manifest fails before we open a single image.
Removes the now-contradicted TestCrossCheckLabels_FilenameColumnNotFirst
(mixed-case), superseded by the case-sensitive FilenameNotFirst. Goldens
regenerated against the pin (sync-validator-goldens.sh --check in sync).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* style(push): trim preflight.go doc comments under the line budget (#373)
6f0f96b pushed internal/push/preflight.go to 1714 lines, over the 1700-line
file-budget ratchet (the failing Lint job). Consolidated the overlapping
filename-column doc comments — the full rationale (case-sensitive transfer
read, data_id-is-training-time, source citations) now lives once in
imageFileColIndex, with CheckImageFilenameColumn referencing it — and merged
the reorder note. 1698 lines now; comments only, no behavior change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for freeto 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

@LukasWodka@saadqbal