fix(data ingest): preflight & message polish — schema-type validation, friendly missing-label, help copy, clearer errors (#213 #214 #215 #76)

internal/cli/data.go

@@ -9,6 +9,7 @@ import (
 	"k8s.io/client-go/kubernetes"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 
 	"github.com/spf13/cobra"
@@ -143,12 +144,17 @@ func newDataIngestCmd() *cobra.Command {
  Use: "ingest <dataset>",
  Aliases: []string{"push"},
  Short: "Ingest a local dataset into your workspace",
- Long: `Ingests a local dataset into your workspace's storage,
+ // The task COUNT and the text-family subdir names are derived from the
+ // registry / vendored layout contract (push.SupportedCategoryIDs +
+ // push.TextSidecarDir) rather than hardcoded, so the help can't drift
+ // from what the CLI actually supports (cli#215): the count used to read
+ // a stale "9", and the text example showed texts/ for every text task
+ // even though masked_language_modeling stages into sequences/.
+ Long: fmt.Sprintf(`Ingests a local dataset into your workspace's storage,
 submits the ingestion run, and follows it to completion (streaming
 progress + the final summary). Your data never leaves your own
-infrastructure. Supports 9 tasks (image classification,
-object/keypoint detection, text classification, masked language
-modeling, and the tabular / time-series family); pick one with --task.
+infrastructure. Supports %[1]d tasks across the image, text, and
+tabular / time-series families; pick one with --task.
 
 <dataset> is the data itself. What it looks like depends on the task:
 
@@ -170,11 +176,11 @@ modeling, and the tabular / time-series family); pick one with --task.
  ...
 
  text (classification, masked language modeling) — a folder with
- labels.csv + a texts/ subfolder:
+ labels.csv + a %[2]s/ subfolder (masked language modeling uses %[3]s/):
 
  reviews/
  labels.csv (required)
-texts/ (required)
+%[2]s/ (required — %[3]s/ for masked language modeling)
  001.txt
  ...
 
@@ -205,6 +211,9 @@ Exit codes:
  8 jobs-manager rejected the submit (4xx/5xx other than auth)
  9 ingestion Job exited non-zero, or completed with row-level
  failures the summary panel reports`,
+ len(push.SupportedCategoryIDs()),
+ push.TextSidecarDir("text_classification"),
+ push.TextSidecarDir("masked_language_modeling")),
  Args: cobra.MaximumNArgs(1),
  RunE: func(cmd *cobra.Command, args []string) error {
  var localPath string
@@ -229,6 +238,14 @@ Exit codes:
  // unset task now drives the picker (TTY) or a clear error
  // (non-interactive), never a silent image assumption.
  taskSet := cmd.Flags().Changed("task") || cmd.Flags().Changed("category")
+ // Record whether --number-of-keypoints was explicitly passed, so
+ // the keypoint set-vs-unset message (#76b) can distinguish an
+ // explicit zero value from an unset flag (both look like the Go
+ // zero value in the spec).
+ changedFlags := map[string]bool{}
+ if cmd.Flags().Changed("number-of-keypoints") {
+ changedFlags["number-of-keypoints"] = true
+ }
  // Guided mode: on a terminal (and unless --no-input), prompt
  // for whatever's still missing. Off a TTY / with --no-input,
  // prompter stays nil and runDataIngest keeps flag-only
@@ -272,6 +289,7 @@ Exit codes:
  Interactive: interactive,
  Prompter: pr,
  TaskSet: taskSet,
+ ChangedFlags: changedFlags,
  OutputJSON: outputJSON,
  JSONOut: jsonOut,
  })
@@ -381,6 +399,15 @@ type runDataIngestArgs struct {
 	Prompter prompter
 	TaskSet bool
 
+	// ChangedFlags records which CLI flags were EXPLICITLY set
+	// (cmd.Flags().Changed), decoupling "was it passed" from "is its value
+	// non-zero" — the value alone can't tell `--number-of-keypoints 0` (an
+	// explicit, invalid value) from an unset flag (also 0). The RunE
+	// populates it for --number-of-keypoints; the keypoint set-vs-unset
+	// diagnostic (#76b) reads it. Nil in direct-construction tests that
+	// don't exercise that path.
+	ChangedFlags map[string]bool
+
 	// OutputJSON routes human output to stderr and emits a JSON result
 	// to JSONOut (stdout); set together by the RunE in --output-json
 	// mode (which also forces non-interactive).
@@ -394,6 +421,17 @@ type runDataIngestArgs struct {
 	ImageDigest string
 }
 
+// sortedKeys returns m's keys in sorted order — used to list a CSV's inferred
+// columns in the friendly missing-label message (#214) deterministically.
+func sortedKeys(m map[string]string) []string {
+	keys := make([]string, 0, len(m))
+	for k := range m {
+ keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	return keys
+}
+
 // expandHome expands a leading ~ (current user or ~user) to a home
 // directory, leaving every other path untouched. It's the CLI-local
 // name for the shared pathutil.ExpandHome; cluster.expandPath resolves
@@ -713,8 +751,18 @@ collaborators can train against that table without ever seeing the raw files.`))
 	case push.IsImage(a.Spec.Category):
  // keypoint_detection needs --number-of-keypoints (dataset-
  // specific, no default). Catch it here with an actionable
- // message rather than letting the ingestor fail mid-run.
+ // message rather than letting the ingestor fail mid-run. Split
+ // UNSET from SET-BUT-INVALID (#76b): a Go 0 could mean either, so
+ // key on whether the flag was passed (ChangedFlags). Unset → the
+ // "requires" nudge; set to a non-positive value → name the bad
+ // value so the user sees exactly what was rejected.
  if a.Spec.Category == "keypoint_detection" && a.Spec.NumberOfKeypoints <= 0 {
+ if a.ChangedFlags["number-of-keypoints"] {
+ return &exitError{code: 2, err: fmt.Errorf(
+ "--number-of-keypoints must be a positive integer (got %d); "+
+ "it's the number of keypoints per sample (e.g. 17 for COCO pose)",
+ a.Spec.NumberOfKeypoints)}
+ }
  return &exitError{code: 2, err: errors.New(
  "keypoint_detection requires --number-of-keypoints (e.g. " +
  "--number-of-keypoints 17); it's dataset-specific and has no default")}
@@ -777,6 +825,24 @@ collaborators can train against that table without ever seeing the raw files.`))
  // the registry's SelfSupervised flag (not a hardcoded id).
 	}
 
+	// 3b. Friendly missing-label pre-check (#214). Every tabular / time-series
+	// task carries a label column (layout contract has_label_column=true for
+	// the whole family). With no --label-column the synthesized spec's
+	// `label` is an empty string, which trips the schema's label oneOf and
+	// the raw validation below dumps an opaque "got object, want string" /
+	// "minLength" pair. Intercept ONLY that specific missing case here — a
+	// label that's present-but-not-in-the-CSV still flows to
+	// runLocalPreflight's CheckLabelColumn, and every other schema error
+	// still reaches the dump — and name the flag to fix instead.
+	if push.IsTabular(a.Spec.Category) && a.Spec.LabelColumn == "" {
+ msg := "this task needs a label column, but --label-column wasn't set — " +
+ "pass --label-column with the name of the target column in your data CSV"
+ if cols := sortedKeys(a.Spec.Schema); len(cols) > 0 {
+ msg += " (columns: " + strings.Join(cols, ", ") + ")"
+ }
+ return &exitError{code: 2, err: errors.New(msg)}
+	}
+
 	// 4. Synthesize the spec from flags + validate against schema.
 	// Catches "bad category", "missing intent" etc. BEFORE we
 	// touch the cluster. The error formatter is the same one

internal/cli/data_delete.go

@@ -105,6 +105,15 @@ func runDataDelete(ctx context.Context, a runDataDeleteArgs) error {
 the cluster and deletes the dataset's files on the shared storage. It can't be
 undone — re-ingesting the data is the only way back.`)
 
+	// 0. Empty-arg guard with a DELETE-appropriate message (#76a). delete
+	// takes the dataset as a POSITIONAL arg, so an empty one must not fall
+	// through to ValidateTableName's "set --name" text (that flag belongs to
+	// `data ingest`, not here). ExactArgs(1) still accepts an explicit "".
+	if a.Table == "" {
+ return &exitError{code: 2, err: errors.New(
+ "dataset name is required — pass it as an argument: tracebloc data delete <dataset>")}
+	}
+
 	// 1. Validate the name before we build any PVC path from it
 	// (push.PlanTeardown panics on an unsafe name by design).
 	if err := push.ValidateTableName(a.Table); err != nil {

internal/cli/data_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/tracebloc/cli/internal/cluster"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"testing"
 )
@@ -661,3 +662,106 @@ func TestDataIngest_ScopedFlag_OnCorrectTask_NotRejected(t *testing.T) {
  t.Fatal("--number-of-keypoints on keypoint_detection must not be rejected as a wrong-task flag")
 	}
 }
+
+// TestDataIngestHelp_DerivedTaskCountAndMLMSubdir pins cli#215: the task COUNT
+// in the ingest help is derived from the registry (not a stale hardcoded "9"),
+// and the text example reflects the layout contract — masked_language_modeling
+// stages into sequences/, not texts/.
+func TestDataIngestHelp_DerivedTaskCountAndMLMSubdir(t *testing.T) {
+	long := newDataIngestCmd().Long
+	n := len(push.SupportedCategoryIDs())
+	want := "Supports " + strconv.Itoa(n) + " tasks"
+	if !strings.Contains(long, want) {
+ t.Errorf("help should derive the task count (%q); got:\n%s", want, long)
+	}
+	if n != 9 && strings.Contains(long, "Supports 9 tasks") {
+ t.Error("help still carries the stale hardcoded 'Supports 9 tasks'")
+	}
+	if !strings.Contains(long, "sequences/") {
+ t.Errorf("help example should show sequences/ for masked language modeling; got:\n%s", long)
+	}
+}
+
+// TestDataIngest_TabularMissingLabel_Friendly pins cli#214: a tabular task with
+// no --label-column gets the friendly flag-naming message (listing the CSV's
+// columns), NOT the raw label-oneOf schema dump.
+func TestDataIngest_TabularMissingLabel_Friendly(t *testing.T) {
+	root := tabularDir(t)
+	var buf bytes.Buffer
+	err := runDataIngest(context.Background(), &buf, &buf, runDataIngestArgs{
+ LocalPath: root,
+ Spec: push.SpecArgs{Table: "t", Category: "tabular_classification", Intent: "train"},
+ Printer: ui.New(&buf, ui.WithColor(false)),
+	})
+	var ee *exitError
+	if !errors.As(err, &ee) || ee.Code() != 2 {
+ t.Fatalf("err = %v, want *exitError code 2", err)
+	}
+	msg := ee.Error()
+	if !strings.Contains(msg, "--label-column") {
+ t.Errorf("message should name --label-column; got: %q", msg)
+	}
+	for _, noise := range []string{"want string", "want object", "failed schema validation", "oneOf"} {
+ if strings.Contains(msg, noise) {
+ t.Errorf("message should be friendly, not the raw schema dump (contained %q); got: %q", noise, msg)
+ }
+	}
+	if !strings.Contains(msg, "churned") {
+ t.Errorf("message should list the CSV columns to guide the fix; got: %q", msg)
+	}
+}
+
+// TestDataDelete_EmptyArg_Friendly pins cli#76a: `data delete ""` (a positional
+// arg) must give a delete-appropriate message pointing at the argument, NOT the
+// shared ValidateTableName "--name" text (that flag belongs to `data ingest`).
+func TestDataDelete_EmptyArg_Friendly(t *testing.T) {
+	var buf bytes.Buffer
+	err := runDataDelete(context.Background(), runDataDeleteArgs{
+ Table: "",
+ Printer: ui.New(&buf, ui.WithColor(false)),
+	})
+	var ee *exitError
+	if !errors.As(err, &ee) || ee.Code() != 2 {
+ t.Fatalf("err = %v, want *exitError code 2", err)
+	}
+	msg := ee.Error()
+	if !strings.Contains(msg, "tracebloc data delete <dataset>") {
+ t.Errorf("delete empty-arg message should point at the positional arg; got: %q", msg)
+	}
+	if strings.Contains(msg, "--name") {
+ t.Errorf("delete path must NOT point at --name; got: %q", msg)
+	}
+}
+
+// TestDataIngest_KeypointNumberSetVsUnset pins cli#76b: --number-of-keypoints
+// unset gets the "requires" nudge, while an explicit non-positive value gets a
+// distinct "must be a positive integer (got N)" message that names the value —
+// the two cases can't be told apart by the Go zero value alone.
+func TestDataIngest_KeypointNumberSetVsUnset(t *testing.T) {
+	root := imgcLayout(t)
+	run := func(changed map[string]bool, n int) *exitError {
+ t.Helper()
+ var buf bytes.Buffer
+ err := runDataIngest(context.Background(), &buf, &buf, runDataIngestArgs{
+ LocalPath: root,
+ Spec: push.SpecArgs{
+ Table: "t", Category: "keypoint_detection", Intent: "train",
+ LabelColumn: "label", NumberOfKeypoints: n,
+ },
+ ChangedFlags: changed,
+ Printer: ui.New(&buf, ui.WithColor(false)),
+ })
+ var ee *exitError
+ if !errors.As(err, &ee) {
+ t.Fatalf("expected *exitError, got %v", err)
+ }
+ return ee
+	}
+	if ee := run(nil, 0); ee.Code() != 2 || !strings.Contains(ee.Error(), "requires --number-of-keypoints") {
+ t.Errorf("unset: got code=%d msg=%q, want exit 2 + 'requires --number-of-keypoints'", ee.Code(), ee.Error())
+	}
+	if ee := run(map[string]bool{"number-of-keypoints": true}, 0); ee.Code() != 2 ||
+ !strings.Contains(ee.Error(), "must be a positive integer (got 0)") {
+ t.Errorf("explicit 0: got code=%d msg=%q, want exit 2 + 'must be a positive integer (got 0)'", ee.Code(), ee.Error())
+	}
+}

internal/cli/ingest.go

@@ -3,10 +3,12 @@ package cli
 import (
 	"fmt"
 	"os"
+	"sort"
 	"strings"
 
 	"github.com/spf13/cobra"
 
+	"github.com/tracebloc/cli/internal/push"
 	"github.com/tracebloc/cli/internal/schema"
 )
 
@@ -91,11 +93,19 @@ func runIngestValidate(cmd *cobra.Command, args []string) error {
  return &exitError{code: 2, err: fmt.Errorf("loading embedded schema: %w", err)}
 	}
 
-_, violations, parseErr := v.ValidateYAML(body)
+doc, violations, parseErr := v.ValidateYAML(body)
 	if parseErr != nil {
  return &exitError{code: 3, err: fmt.Errorf("%s: %w", path, parseErr)}
 	}
 
+	// The jsonschema types every `schema` value as a bare string, so a bogus
+	// SQL type (e.g. {age: BANANA}) passes it — a false green that only fails
+	// in-cluster at CREATE TABLE. `data validate` is a local preview of what
+	// the cluster accepts, so mirror the ingestor's accepted-type set here too
+	// (cli#213), reusing the SAME push.ValidateSchemaType the --schema flag
+	// path uses so the two can't diverge.
+	violations = append(violations, schemaTypeViolations(doc)...)
+
 	if len(violations) == 0 {
  // Explicit discard: Fprintf returns an error when the
  // underlying writer fails (closed pipe, etc.). For the
@@ -119,6 +129,42 @@ func runIngestValidate(cmd *cobra.Command, args []string) error {
 	return &exitError{code: 2, err: nil} // err==nil so cobra doesn't print "Error: ..." on top
 }
 
+// schemaTypeViolations previews the ingestor's accepted-SQL-type check over a
+// parsed ingest doc's `schema` block (cli#213). It returns one ValidationError
+// per column whose declared type the ingestor would reject, anchored at
+// "schema.<column>" so it renders in the same JSON-pointer format as every
+// other violation. A missing / non-map schema, or a non-string value, is left
+// to the jsonschema layer — this only adds the type-vocabulary check the
+// schema itself can't express.
+func schemaTypeViolations(doc map[string]any) []schema.ValidationError {
+	if doc == nil {
+ return nil
+	}
+	sch, ok := doc["schema"].(map[string]any)
+	if !ok {
+ return nil
+	}
+	cols := make([]string, 0, len(sch))
+	for col := range sch {
+ cols = append(cols, col)
+	}
+	sort.Strings(cols) // deterministic order (FormatErrors re-sorts, but keep it stable)
+	var out []schema.ValidationError
+	for _, col := range cols {
+ typ, ok := sch[col].(string)
+ if !ok {
+ continue
+ }
+ if err := push.ValidateSchemaType(col, typ); err != nil {
+ out = append(out, schema.ValidationError{
+ Path: "schema." + col,
+ Message: err.Error(),
+ })
+ }
+	}
+	return out
+}
+
 func plural(n int) string {
 	if n == 1 {
  return ""