From db65d546ae81999e1331493462393e08149054b3 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Thu, 9 Jul 2026 16:15:32 +0200 Subject: [PATCH 1/3] fix(data ingest): address #197 code-review findings - interactive picker no longer passes an empty default to survey.Select after --task's default was dropped (crashed "omit --task to pick interactively" on a TTY); falls back to the first task. - rename the guided name prompt "Destination table name" -> "Dataset name" to match --name / the Review + summary labels (finish the #180 rename). - --name flag help now states the leading letter/underscore rule the tightened validator enforces. - README quickstart: --split train -> --intent train (--split was removed; --intent stayed canonical). Co-Authored-By: Claude Opus 4.8 --- README.md | 2 +- internal/cli/data.go | 2 +- internal/cli/interactive.go | 14 +++++++++++--- internal/cli/interactive_test.go | 8 ++++---- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 9dd71ff6..57d0ac8f 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,7 @@ irm https://github.com/tracebloc/cli/releases/latest/download/install.ps1 | iex tracebloc data ingest ./my-data \ --name cats_dogs_train \ --task image_classification \ - --split train \ + --intent train \ --label-column label ``` diff --git a/internal/cli/data.go b/internal/cli/data.go index b0b1a37b..a15872b5 100644 --- a/internal/cli/data.go +++ b/internal/cli/data.go @@ -260,7 +260,7 @@ Exit codes: // schema validator catches missing/empty values with the canonical // JSON-pointer-anchored error. cmd.Flags().StringVar(&name, "name", "", - "a name for this dataset (letters, digits, underscore) — you'll reference it by this name when you start a training run") + "a name for this dataset — start with a letter or underscore, then letters/digits/underscores — you'll reference it by this name when you start a training run") cmd.Flags().StringVar(&tableAlias, "table", "", "deprecated alias for --name") _ = cmd.Flags().MarkHidden("table") diff --git a/internal/cli/interactive.go b/internal/cli/interactive.go index 3b561008..81b33886 100644 --- a/internal/cli/interactive.go +++ b/internal/cli/interactive.go @@ -127,8 +127,16 @@ func runInteractive(p *ui.Printer, pr prompter, a *runDataIngestArgs, taskSet bo if !taskSet { p.PromptHint("What kind of task your data is for — this drives how it's validated and loaded.") + // The --task default was dropped (#180a), so a.Spec.Category is "" here. + // survey.Select rejects a default that isn't one of the options, so fall + // back to the first task rather than passing "" (which errors out the + // whole "omit --task to pick interactively" flow). + def := a.Spec.Category + if def == "" && len(promptCategories) > 0 { + def = promptCategories[0] + } ans, err := pr.Select("Task", "what kind of data this is", - promptCategories, a.Spec.Category) + promptCategories, def) if err != nil { return err } @@ -138,8 +146,8 @@ func runInteractive(p *ui.Printer, pr prompter, a *runDataIngestArgs, taskSet bo if a.Spec.Table == "" { p.PromptHint("Names the table created on the cluster (and its folder on the shared storage). Letters, digits, underscores only. e.g. churn_train") - ans, err := pr.Input("Destination table name", - "MySQL identifier + PVC subdir; letters, digits, underscore only", "", + ans, err := pr.Input("Dataset name", + "MySQL identifier + PVC subdir; start with a letter or underscore, then letters, digits, underscore", "", push.ValidateTableName) if err != nil { return err diff --git a/internal/cli/interactive_test.go b/internal/cli/interactive_test.go index 18b24d40..6a6e056b 100644 --- a/internal/cli/interactive_test.go +++ b/internal/cli/interactive_test.go @@ -57,7 +57,7 @@ func TestRunInteractive_FillsAllWhenEmpty(t *testing.T) { f := &fakePrompter{answers: map[string]string{ "Path to your dataset directory": "./data", "Task": "tabular_classification", - "Destination table name": "churn_train", + "Dataset name": "churn_train", "Is this training or test data?": "test", "Label column": "churned", }} @@ -90,7 +90,7 @@ func TestRunInteractive_FillsAllWhenEmpty(t *testing.T) { func TestRunInteractive_ShowsExampleHints(t *testing.T) { f := &fakePrompter{answers: map[string]string{ "Path to your dataset directory": "./d", - "Destination table name": "churn_train", + "Dataset name": "churn_train", }} a := &runDataIngestArgs{Spec: push.SpecArgs{Category: "tabular_regression"}} @@ -192,7 +192,7 @@ func TestRunInteractive_Cancel(t *testing.T) { // label column, so it must not be prompted. func TestRunInteractive_MLMSkipsLabel(t *testing.T) { f := &fakePrompter{answers: map[string]string{ - "Destination table name": "mlm_train", + "Dataset name": "mlm_train", "Is this training or test data?": "train", }} a := &runDataIngestArgs{ @@ -215,7 +215,7 @@ func TestRunInteractive_MLMSkipsLabel(t *testing.T) { // TestRunInteractive_RejectsBadTable: the table prompt runs // push.ValidateTableName, so an unsafe name surfaces as an error. func TestRunInteractive_RejectsBadTable(t *testing.T) { - f := &fakePrompter{answers: map[string]string{"Destination table name": "../bad"}} + f := &fakePrompter{answers: map[string]string{"Dataset name": "../bad"}} a := &runDataIngestArgs{ LocalPath: "./data", Spec: push.SpecArgs{Category: "image_classification", Intent: "train", LabelColumn: "label"}, From 7ae4701e5afe92d2d085ab5c89af876df93b91ed Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Thu, 9 Jul 2026 20:25:21 +0500 Subject: [PATCH 2/3] fix(data ingest): finish the name-prompt rename + drop dead picker seed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review follow-ups on #199: - The PromptHint above the "Dataset name" prompt still described "the table created on the cluster (and its folder on the shared storage). Letters, digits, underscores only" — it leaked the k8s/MySQL ceremony the rename was removing and stated the old rule (implying a leading digit is allowed, which ValidateTableName rejects). Reworded to plain language matching the flag help + validator. - The picker's default was seeded from a.Spec.Category, which is always "" in the !taskSet branch, so the len-guarded fallback always fired. Seed promptCategories[0] directly — the list is a fixed non-empty package var backed by the hardcoded category registry. go build ./..., go vet, gofmt -l clean; go test ./internal/cli/... ./internal/push/... green. Co-Authored-By: Claude Opus 4.8 --- internal/cli/interactive.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/internal/cli/interactive.go b/internal/cli/interactive.go index 81b33886..7789db16 100644 --- a/internal/cli/interactive.go +++ b/internal/cli/interactive.go @@ -127,16 +127,12 @@ func runInteractive(p *ui.Printer, pr prompter, a *runDataIngestArgs, taskSet bo if !taskSet { p.PromptHint("What kind of task your data is for — this drives how it's validated and loaded.") - // The --task default was dropped (#180a), so a.Spec.Category is "" here. - // survey.Select rejects a default that isn't one of the options, so fall - // back to the first task rather than passing "" (which errors out the - // whole "omit --task to pick interactively" flow). - def := a.Spec.Category - if def == "" && len(promptCategories) > 0 { - def = promptCategories[0] - } + // The --task default was dropped (#180a), so the task is always + // unset in this branch. survey.Select rejects a default that isn't + // one of the options, so seed the first task rather than "" (which + // errors out the whole "omit --task to pick interactively" flow). ans, err := pr.Select("Task", "what kind of data this is", - promptCategories, def) + promptCategories, promptCategories[0]) if err != nil { return err } @@ -145,7 +141,7 @@ func runInteractive(p *ui.Printer, pr prompter, a *runDataIngestArgs, taskSet bo } if a.Spec.Table == "" { - p.PromptHint("Names the table created on the cluster (and its folder on the shared storage). Letters, digits, underscores only. e.g. churn_train") + p.PromptHint("A name for this dataset — start with a letter or underscore, then letters, digits, underscores. e.g. churn_train") ans, err := pr.Input("Dataset name", "MySQL identifier + PVC subdir; start with a letter or underscore, then letters, digits, underscore", "", push.ValidateTableName) From 4497efb95d5b3676c388a740548d4d90ae35bcf9 Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Thu, 9 Jul 2026 20:33:35 +0500 Subject: [PATCH 3/3] test(data ingest): cover the picker's default-seed path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #197 fix seeds the task picker with promptCategories[0] when the task is unset, since survey.Select rejects an empty default. No test drove taskSet=false with an empty category, so a reseed to "" would have kept the suite green while crashing a real terminal. This case omits the "Task" answer so the fake returns the seeded default, then asserts the category landed on promptCategories[0] — verified to fail if the seed regresses to "". Co-Authored-By: Claude Opus 4.8 --- internal/cli/interactive_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/internal/cli/interactive_test.go b/internal/cli/interactive_test.go index 6a6e056b..4a694a0a 100644 --- a/internal/cli/interactive_test.go +++ b/internal/cli/interactive_test.go @@ -83,6 +83,30 @@ func TestRunInteractive_FillsAllWhenEmpty(t *testing.T) { } } +// TestRunInteractive_PickerSeedsFirstTaskWhenUnset: with the task unset +// (taskSet=false) and no category on a, the picker must seed a valid +// option as its default. Dropping --task's old image_classification +// default (#180a) left an empty seed, which real survey.Select rejects +// ("default value \"\" not found in options"), crashing the headline +// "omit --task to pick interactively" flow. Omitting the "Task" answer +// makes the fake return the seeded default verbatim, so asserting the +// category landed on promptCategories[0] locks the seed in — a reseed +// to "" would fail this even though the fake itself doesn't validate. +func TestRunInteractive_PickerSeedsFirstTaskWhenUnset(t *testing.T) { + f := &fakePrompter{answers: map[string]string{ + "Dataset name": "churn_train", + }} + a := &runDataIngestArgs{LocalPath: "./data"} // Category deliberately left "" + + if err := runInteractive(discardPrinter(), f, a, false /*taskSet*/); err != nil { + t.Fatalf("runInteractive: %v", err) + } + if a.Spec.Category != promptCategories[0] { + t.Errorf("Category = %q, want the seeded first task %q", + a.Spec.Category, promptCategories[0]) + } +} + // TestRunInteractive_ShowsExampleHints: each input prompt is preceded // by a visible hint with an example, so the guided flow teaches as it // goes. Drives runInteractive with a real (buffer-backed) Printer and