Uh oh!
There was an error while loading. Please reload this page.
fix(data ingest): #180b review follow-ups — path trim, self-supervised registry flag, honest text sniff - #201
Merged
saadqbal merged 1 commit intoJul 9, 2026
Conversation
…flag, honest text sniff echo - Trim the interactive path answer before storing it. validateDatasetPath only trims to check for emptiness, so a pasted " ~/data" (stray space) otherwise survived, defeated expandHome (first char isn't '~'), and made filepath.Abs prepend cwd — the family sniff / label-header preview then read a path that doesn't exist (and silently fell back to free-text label entry, the data-ingestors#340 class this feature exists to prevent). - Make SelfSupervised a CategorySpec registry field (set on MLM + CLM) and have SelfSupervisedText read it, so a new self-supervised task can't be added without deciding whether it needs a label column — was a hardcoded two-id switch decoupled from the registry. - Soften the confident text sniff echo to "looks like": texts/ and sequences/ map to DIFFERENT tasks (TextSidecarDir), so a family-level echo must not imply the task the user then picks will load. The walk stays the authoritative check. - Drop the unreachable empty-slice branch in defaultLabelChoice (its only caller already guards len(headers) > 0). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
approved these changes
Jul 9, 2026
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka added a commit
that referenced
this pull request
Jul 9, 2026
…ker (#180b) (#198) * feat(data ingest): data-first flow inversion + family-scoped task picker (#180b) Invert the guided ingest flow to data-first (RFC-0002 §12.1) and rework task selection so the user is never shown the flat 15-task wall. - Prompt order is now intent → name → path → task → task-specific → review. Name is asked with an example in the hint (not auto-filled). - After the path, the family is sniffed from the layout and echoed ("Found a CSV table — this is tabular data."). The sniff is a HINT, not a lock (§5.1): an ambiguous layout asks the family plainly, an explicit --task skips the sniff entirely, and non-interactive runs never sniff. SniffFamily mirrors the Discover* layout markers — it validates nothing, so it can't fork or replace the real walk. - Family-scoped task picker (§7): only that family's tasks are offered, rendered "Display — one-liner · task_id", split into Available now vs a greyed "Not yet in the CLI" (with the reason). The registry Label field is finally wired up, plus the locked glosses (time_to_event_prediction → "Survival analysis", masked_language_modeling → "fill-mask", seq2seq → "translation / summarization") and a per-task blurb. - Label question (§8) now SELECTs over the real CSV header row (exact match kills the data-ingestors#340 case-mismatch silent-null-label class), worded per task (class vs value-to-predict), skipped for self-supervised text; falls back to free text if the header can't be read. --number-of-keypoints / --time-column unchanged. - Promptable overwrite (folded): a pre-existing destination table is a y/N prompt in interactive mode (yes → replace, no → clean exit 0), still a hard exit 6 non-interactively; a reused --idempotency-key falls through to exit 6 to preserve the data-loss guard. - Copy polish (folded from #179): data/ingest help says "workspace" and "ingest", not "client" and "stage". Aliases unchanged. Exit codes / classify / reclaim matrix and --output-json keys unchanged. Closes#180 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(data ingest): keep Kubernetes out of the happy-path copy (#180b) The ingest command's Long help described the normal successful flow in Kubernetes terms ("submits an ingestion run to jobs-manager, and watches the ingestor Job") - the exact ceremony RFC-0002 section 6 keeps off the happy path. Reword it to plain language plus the on-prem reassurance. The exit-code section and the --detach reconnect hint still name jobs-manager/kubectl: those are failure/detach diagnostics, which section 6 explicitly permits. Also fix the exit-6 duplicate-table error to say "in this workspace", not "in this client" - matching the workspace wording used everywhere else in the flow. Copy-only; no behavior change. Tests + gofmt + vet green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(data ingest): make the family sniff mirror the discovery walk (#180b) SniffFamily + previewLabelCSVPath were an over-permissive fork of the real discovery walk. Align them to what Discover / DiscoverText / DiscoverTabular actually accept so a confident sniff can never promise a layout the walk refuses: - Match the marker dirs (images/, texts/, sequences/) and labels.csv with the literal, case-sensitive names the walk joins + Lstats. A mis-cased "Images/" is no longer read as confident image. Only the .csv extension match stays case-insensitive (mirrors DiscoverTabular's EqualFold). - Require BOTH labels.csv AND the subdir before claiming confident image / text (mirror Discover / DiscoverText); the echo now only names files that were actually found. - Require a directory for tabular: a bare .csv file is no longer confident (DiscoverTabular rejects bare files; that's cli#181). - previewLabelCSVPath now reuses DiscoverTabular's single-CSV rule via a shared findSingleCSV helper, so a multi-CSV directory errors (caller falls back to free text) instead of silently reading the alphabetical first. Interactive flow: - Path prompt copy is folder-oriented ("the folder holding it") — the walk can't take a bare file yet, so don't promise one. - Path prompt gets a validator that rejects an empty/whitespace answer, so a bare Enter can't sniff (and ingest) the current working directory. - Drop the dead famPrompted store in resolveFamily (pickTask sets prompted unconditionally right after). Also removes the unused FamilyOf accessor. Tests pin: mis-cased dir not confident, missing labels.csv not confident image/text, bare .csv not confident tabular, empty path rejected, multi-CSV matches the walk. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(data ingest): sniff confident tabular only on exactly one CSV (#180b) The family sniff claimed confident tabular whenever a directory held at least one CSV, but DiscoverTabular's findSingleCSV requires exactly one — so a two-CSV directory got echoed "this is tabular data" and then the walk rejected it. That made the sniff more permissive than the walk it mirrors, breaking the sniff's own contract ("never claims more than the matching Discover* would accept"). Gate the tabular case on csvCount == 1 so sniff-confident-tabular holds only for directories the walk would accept structurally; multi-CSV directories now fall through to the plain family question. Adds a regression test asserting both the ambiguous sniff and the walk rejection on the same input. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(cli): single source of truth for the Family<->noun mapping (#200) The Family<->noun mapping had three hand-maintained copies that had to stay in sync by hand: push.FamilyNoun (Family->noun), interactive.go's familyFromNoun (noun->Family), and resolveFamily's literal []string{"tabular","image","text"} picker options + "tabular" default. Consolidate to one ordered familyNounTable in internal/push. FamilyNoun, the new exported FamilyFromNoun, and FamilyNouns() (picker options + default) all derive from it. resolveFamily now takes its Select options and default from push.FamilyNouns() and its reverse lookup from push.FamilyFromNoun; the local familyFromNoun is gone. Behavior identical: same order (tabular, image, text), same default (tabular). build/vet/tests green. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix(data ingest): #180b review — path trim, self-supervised registry flag, honest text sniff echo (#201) - Trim the interactive path answer before storing it. validateDatasetPath only trims to check for emptiness, so a pasted " ~/data" (stray space) otherwise survived, defeated expandHome (first char isn't '~'), and made filepath.Abs prepend cwd — the family sniff / label-header preview then read a path that doesn't exist (and silently fell back to free-text label entry, the data-ingestors#340 class this feature exists to prevent). - Make SelfSupervised a CategorySpec registry field (set on MLM + CLM) and have SelfSupervisedText read it, so a new self-supervised task can't be added without deciding whether it needs a label column — was a hardcoded two-id switch decoupled from the registry. - Soften the confident text sniff echo to "looks like": texts/ and sequences/ map to DIFFERENT tasks (TextSidecarDir), so a family-level echo must not imply the task the user then picks will load. The walk stays the authoritative check. - Drop the unreachable empty-slice branch in defaultLabelChoice (its only caller already guards len(headers) > 0). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix(data ingest): don't sniff a mis-cased media folder + labels.csv as confident tabular (#203) (#204) A directory with labels.csv plus a mis-cased media folder (Images/, Texts/, Sequences/) fell through SniffFamily's confident-tabular branch: the subdir switch is case-sensitive so hasImages/hasText stayed false, labels.csv counted as the one CSV, and the flow confidently echoed "tabular", skipped the family question, and offered only tabular tasks. DiscoverTabular then ignores the subdir, reads labels.csv as the single CSV, and SUCCEEDS — silently ingesting an image/text dataset as a standalone table with the media files dropped and no error. Detect a subdir whose name matches a marker (images/texts/sequences) case-insensitively but not exactly, and bail to ambiguous so the flow asks the family plainly. Narrow: an unrelated subdir (backup/, raw/) still sniffs confident tabular, matching DiscoverTabular which ignores it. The confident image/text branches still require an EXACT match, mirroring the walk's literal os.Lstat. Strengthened the two mis-cased tests to assert the sniff is not confident AT ALL (they previously only checked "not confident image/text" and so missed the tabular masquerade), plus a case pinning that an unrelated subdir stays confident tabular. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Asad Iqbal (Saadi) <asad.dsoft@gmail.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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Stacked on #198 (feat/180b) — merge into that branch. Addresses four findings from the code review of #198.
What changed
Path answer is trimmed before it's stored (
interactive.go).validateDatasetPathonly trims to check for emptiness; the raw answer was stored, so a pasted" ~/data"(stray leading/trailing space) survived, defeatedexpandHome(first char isn't~), andfilepath.Absprepended cwd. The sniff / label-header preview then read a nonexistent path — and silently fell back to free-text label entry, re-opening the data-ingestors#340 case-mismatch class this feature exists to close. New regression testTestRunInteractive_TrimsPath.SelfSupervisedis now aCategorySpecregistry field (category.go), set onmasked_language_modeling+causal_language_modeling;SelfSupervisedTextreads it instead of a hardcoded two-id switch. A new self-supervised task can no longer be added without deciding whether it needs a label column — the same disciplineRegressionClass/CLISupportedalready enforce.Confident text sniff echo softened to "looks like" (
preview.go).texts/andsequences/map to different tasks viaTextSidecarDir(texts/ → classification, sequences/ → MLM), so a family-level echo must not imply the task the user then picks will load. TheDiscover*walk stays the authoritative check. (Image/tabular echoes are unchanged — those layouts map cleanly to their walks.)Dropped the unreachable empty-slice branch in
defaultLabelChoice— its only caller already guardslen(headers) > 0.Rolls up under the #180 ticket (review follow-ups on #180b, not a new task).
Tests
gofmt -lclean,go build ./...,go vet ./...,go test ./...— all green.🤖 Generated with Claude Code
Note
Low Risk
Small CLI UX and registry-metadata changes with targeted tests; no auth, security, or ingest pipeline behavior changes beyond fixing path handling and copy.
Overview
Follow-ups on the interactive data-ingest flow: path answers are trimmed before
LocalPathis set so pasted whitespace cannot breakexpandHomeand send family sniff / label-header preview to a wrong path (with a regression test).Self-supervised text tasks are modeled via a new
CategorySpec.SelfSupervisedflag on MLM/CLM;SelfSupervisedTextreads the registry instead of a hardcoded id list. The confident text family sniff echo now says "looks like text data" becausetexts/vssequences/imply different tasks, not a guaranteed match for the task the user picks.Minor cleanup: unreachable empty-header branch removed from
defaultLabelChoice.Reviewed by Cursor Bugbot for commit f54e06c. Bugbot is set up for automated code reviews on this repo. Configure here.