Skip to content

feat(data ingest): wire the 5 text tasks (#182) - #209

Merged
saadqbal merged 3 commits into
developfrom
feat/182-wire-text-tasks
Jul 10, 2026
Merged

feat(data ingest): wire the 5 text tasks (#182)#209
saadqbal merged 3 commits into
developfrom
feat/182-wire-text-tasks

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Wires the five CLI-pending text tasks into data ingest — token classification, sentence-pair classification, causal language modeling, seq2seq, and embeddings — so the whole text family is pushable, not just text classification and masked language modeling. Discovery, staging, spec-building, the interactive picker, and preflight all learn these tasks by reading the vendored layout contract (layout.v1.json), never by forking the ingestor's rules (RFC-0002 Principle 6). Data stays on-prem throughout; only metadata syncs.

This PR also folds in the verified review findings on the branch.

What changed

  • layout contract vendoredinternal/schema/layout.v1.json + layout_contract.go mirror data-ingestors' per-task on-disk layout. sync-schema.sh now vendors both ingest.v1.json and layout.v1.json from the pinned ref, drift-checked in CI.
  • enforced record formats — sentence-pair (text_a<TAB>text_b) and embeddings (anchor<TAB>positive[<TAB>negative]) get a local structural check before staging, so a malformed file fails with a clear message instead of after the upload. The check is scoped to the files labels.csv actually references — mirroring the ingestor's TabSeparatedRecordValidator manifest walk — so a stray unreferenced .txt never blocks a layout the cluster would accept.
  • label preflight parity — the text-family label-column and diversity previews now cover every supervised text task, gated on registry flags (!SelfSupervisedText for the label-column existence check, IsClassification for diversity) rather than a hardcoded text_classification. Token classification correctly skips the diversity check — the ingestor runs BIOLabelValidator on it, not LabelDiversityValidator.
  • self-supervised vs supervised splitbuildText emits label for exactly the supervised text tasks, keyed on the registry's SelfSupervised flag. TextSidecarDir is now read from the contract's primary_subdir instead of a hardcoded MLM special-case.
  • sync-schema.sh hardening — signal-safe temp-file cleanup (EXIT/INT/TERM), and a failed write now returns non-zero instead of a false "wrote" + exit 0.

Test plan

  • go build ./... — clean
  • go test ./... — all green (added TestDiscoverText_EnforcedRecordFormat_IgnoresUnreferenced, TestPreflightDataset_TextLabelParity, plus the existing phase-4 discovery/record-format/spec tests)
  • gofmt -l . — empty
  • go vet ./... — clean
  • scripts/sync-schema.sh --check — no drift against the pinned ref; write-failure path verified to exit non-zero

Layout-contract fields consumed

The CLI reads these from layout.v1.json so discovery/staging stays a verified mirror:

  • primary_subdir — the text-file subdir to stage into (texts/, or sequences/ for MLM), via TextSidecarDir
  • record_format (fields, min_fields, separator, enforced) — the per-.txt structure and whether the ingestor rejects a malformed file, via RecordFormatFor / ValidateTextRecord
  • manifest.has_label_column — pinned against the registry's supervised/self-supervised split in layout_contract_test.go

Stacking

Was stacked on the #180b data-first-inversion work (#198). That has since squash-merged into develop, so this targets develop directly and is standalone — no ordering constraint left.

Deferred: semantic segmentation

Semantic segmentation stays CLI-pending on purpose. di#136 shipped the mask sidecar, but the ingestor doesn't yet populate the mask_id link column the contract requires, and the training-side sign-off is tracked in backend#816. The registry keeps it schema-recognized-but-unsupported with that note, so the push gate explains why. Wire it once #816 lands.

Part of #182.


Note

Medium Risk
Broad changes to ingest discovery, spec emission, and preflight across the text family; correctness depends on staying in sync with the vendored layout contract, though tests and CI drift checks mitigate that.

Overview
Enables data ingest for the five previously CLI-pending text tasks — token classification, sentence-pair classification, causal LM, seq2seq, and embeddings — so the full text family is pushable except semantic segmentation (still blocked on ingestor mask_id + backend#816).

Discovery, spec building (buildText labels via SelfSupervised), preflight, and the interactive picker all follow the vendored layout.v1.json contract (synced with ingest.v1.json in scripts/sync-schema.sh) instead of hardcoded per-task rules. TextSidecarDir now comes from the contract’s primary_subdir; a new IsClassification registry flag drives label-diversity preflight so token classification skips diversity (BIO tags) while supervised class tasks do not.

Local validation before upload: enforced tab-separated shapes for sentence-pair and embeddings mirror the ingestor’s TabSeparatedRecordValidator, only on files referenced in labels.csv (stray .txt files do not fail discovery). Shared CSV helpers (openCSVReader, matchColumnIndex) dedupe BOM-aware reads and column matching.

Tests and registry expectations were updated (14 supported categories; image picker still shows semantic segmentation as “not yet in the CLI”).

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

@LukasWodka
LukasWodkaforce-pushed the feat/182-wire-text-tasks branch from 76e6250 to bcea12fCompareJuly 9, 2026 19:06

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit bcea12f. Configure here.

Comment threadscripts/sync-schema.sh Outdated
LukasWodkaand others added 2 commits July 10, 2026 09:45
Wire token_classification, sentence_pair_classification,
causal_language_modeling, seq2seq, and embeddings so the family-scoped
picker offers them under "Available now" — the ingestor, schema, and
backend already support them; CLI-side discovery/staging was the only gap.
The CLI now MIRRORS data-ingestors' machine-readable layout contract
(di#347/#353) rather than forking its layout rules (RFC-0002 Principle 6):
- Vendor layout.v1.json into internal/schema/ + embed it; extend
scripts/sync-schema.sh to sync + drift-check both contract files. Bump
scripts/.data-ingestors-ref to the #353 merge commit (3c63d9a), which
keeps ingest.v1.json byte-identical (no unrelated drift).
- internal/push/layout_contract.go parses the contract and drives the
ENFORCED record-format checks (sentence_pair: text_a<TAB>text_b;
embeddings: anchor<TAB>positive[<TAB>negative]) at discovery, mirroring
the ingestor's TabSeparatedRecordValidator. Unenforced formats (seq2seq,
causal LM) accept raw text, so the mirror doesn't reject them.
- Fix the self-supervised flags for seq2seq + embeddings (no label
question), and emit the label for the supervised text tasks.
- Flip the 5 CLISupported=true; a drift test pins the Go registry's
family/label/subdir facts against the vendored contract.
semantic_segmentation stays CLI-pending: it awaits the ingestor's mask_id
link column + training sign-off (backend#816).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…exactly
Verified review findings on the text-task wiring, fixed minimally so the CLI
never forks a fact the layout contract / ingestor owns (RFC-0002 Principle 6):
- text.go: the enforced record-format check (sentence_pair, embeddings) now
runs only over the .txt files labels.csv references, mirroring the ingestor's
TabSeparatedRecordValidator manifest walk — a stray unreferenced .txt no
longer fails discovery on a layout the cluster accepts.
- preflight.go: the text-family label preflight was hardcoded to
text_classification; gate the label-column check on !SelfSupervisedText and
the diversity check on IsClassification, so sentence_pair_classification and
token_classification get the right previews (token_classification is NOT
is_classification, so it skips diversity — the ingestor runs BIOLabelValidator
instead). Adds IsClassification to the registry, mirroring the ingestor's
ModalitySpec.is_classification.
- category.go: derive TextSidecarDir from the contract's primary_subdir instead
of a hardcoded MLM special-case; refresh the SelfSupervised docs to cover
seq2seq/embeddings (target from the record's paired fields, not the text).
- data.go: refresh the stale text-branch comment (7 tasks, supervised split).
- sync-schema.sh: restore signal-safe temp-file cleanup (EXIT/INT/TERM, not a
RETURN trap) and make a failed write in write mode return non-zero instead of
a false "wrote" + exit 0 under the `if ! sync_one` errexit suspension.
Part of #182.
Co-Authored-By: Claude Opus 4.8 <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.

Careful PR — the registry/contract/flag refactor mirrors the ingestor faithfully (verified SelfSupervisedhas_label_column, IsClassification, TextSidecarDir, buildText against data-ingestors). Findings below are mostly in the new record-format machinery, where the enforced check can silently no-op. Two candidates I checked turned out fine: text tasks resolve the filename by a namedfilename column (not positional like the image path), and token_classification correctly requires --label-column (BIOLabelValidator). #1 and #2 are the ones worth acting on.

Comment threadinternal/push/text.go Outdated
Comment threadinternal/push/text.go Outdated
Comment threadinternal/push/text.go Outdated
Comment threadinternal/push/text.go Outdated
Comment threadinternal/push/layout_contract.go Outdated
Comment threadscripts/sync-schema.sh
Comment threadinternal/push/category.go Outdated
Comment threadinternal/push/text.go Outdated
Comment threadinternal/push/text.go
Comment threadinternal/push/layout_contract.go Outdated
…d files, not reconstructed names (#209)
Address Asad's review on #209. The enforced text-record check reconstructed
"<stem>.txt" from the manifest instead of matching the files actually
discovered on disk, so it fail-opened in several ways. Mirror the ingestor's
TabSeparatedRecordValidator exactly (RFC-0002 Principle 6):
- Match each manifest filename against the discovered sidecar basenames
(case-insensitive on basename and stem), so a row "a" resolves to texts/a.text
when the ingestor's configured extension is .text — no more hardcoded ".txt".
- Require the filename column locally: a manifest without it now errors clearly
(mirrors the ingestor's "Missing required column: filename") instead of
silently validating nothing.
- Read the manifest with LazyQuotes so a row pandas tolerates (an unescaped
quote) is read here too, not silently dropped and left unvalidated.
- Drive the field-count error message off the contract separator (sepLabel),
so a future non-tab task isn't misdescribed as "tab-separated".
- TextSidecarDir now fails loud on a text category missing from the vendored
contract — that can only be a vendoring/drift bug, not a runtime condition.
- sync-schema.sh checks curl's exit explicitly (a 404 was misdiagnosed as
"not valid JSON" under the set -e-suspending `if ! sync_one`) and adds
--tlsv1.2 to match the rest of the repo.
- Dedupe: shared matchColumnIndex (column resolve) and openCSVReader
(BOM-stripping CSV reader) helpers; slices.Contains over hand-rolled
containsInt.
Tests: .text extension validated, missing filename column errors, case-
mismatched basename validated, pandas-tolerable/Go-strict row read, contract-
driven message.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

Thanks for the careful review, @saadqbal — all 11 addressed in a98e598. The core fix reworks the enforced text-record check to match the actual discovered files (mirroring the ingestor's manifest walk) instead of reconstructing <stem>.txt, which closes all four fail-open paths at once.

Correctness (the record-format cluster):

  1. Configured extensionvalidateTextRecords now indexes discovered files by lowercased basename + stem and resolves each manifest value against them; row a matches texts/a.text. No more hardcoded .txt.
  2. Missing filename column — now a local hard error for the enforced tasks ("labels.csv has no filename column …"), mirroring the ingestor's rejection instead of silently validating nothing.
  3. Case-insensitive basename — folded via Phase 1: embed ingest.v1.json + tracebloc ingest validate #1 (both maps + lookups lowercase); A.txta.txt validates.
  4. Tolerant manifest readLazyQuotes=true (+ FieldsPerRecord=-1) so a pandas-tolerable row Go's strict reader would reject is read + validated, not dropped.

Robustness / convention:
5. layout_contract field-count message is now contract-driven (sepLabel()/shape()), no hardcoded "tab-separated"; pinning test updated.
6. sync-schema.sh captures curl's exit explicitly — a 404 now says "failed to fetch … (curl exited N)" instead of misdiagnosing "not valid JSON".
7. --tlsv1.2 added to the fetch (matches install.sh).
8. TextSidecarDir's unreachable fallback now panics with a drift message instead of silently returning "texts" (guarded by the mirror test + sync-schema --check).

Reuse:
9. Shared matchColumnIndex (exact → case-insensitive-trimmed) — one copy, used by CheckLabelColumn + the filename resolve.
10. Shared openCSVReader (open + BOM-strip + FieldsPerRecord=-1) — deduped across the 5 copies (ReadCSVHeader, CheckHasDataRows, CrossCheckLabels, readLabelColumnValues, and the new manifest reader). LazyQuotes stays opt-in per caller.
11. slices.Contains replaces containsInt.

New tests: TestDiscoverText_ConfiguredExtension / _MissingFilenameColumn / _CaseMismatchedBasename / _TolerantManifestRow. go build/test/vet/gofmt/sync-schema --check all green.

One thing I did NOT change (flagging for transparency): there's no text-family missing-file cross-check — a text row naming a file absent from disk is caught by the ingestor ("text file not found") but not in CLI preflight, whereas the image family has CrossCheckLabels. That's pre-existing (not introduced here) and out of this PR's scope; it likely belongs with the #1009 parity-harness work. Happy to file a follow-up if you'd like it tracked.

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