Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 43 additions & 1 deletion internal/cli/coverage_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,7 @@ import (
"encoding/json"
"errors"
"os"
"os/user"
"path/filepath"
"strings"
"testing"
Expand DownExpand Up@@ -173,7 +174,10 @@ func TestClassifyPushOutcome(t *testing.T) {
func TestRunDatasetPush_OutputJSONEarlyFailureEmitsJSON(t *testing.T) {
var jsonBuf, human bytes.Buffer
a := runDataIngestArgs{
LocalPath: "./x",
// A real path so the failure is the invalid table name (exit 2), not
// the earlier path-existence check (exit 3, #181) — this test pins the
// stdout-always-JSON contract on the table-validation failure.
LocalPath: t.TempDir(),
Spec: push.SpecArgs{Table: "../bad", Category: "image_classification", Intent: "train"},
Printer: ui.New(&human, ui.WithColor(false)),
OutputJSON: true,
Expand DownExpand Up@@ -219,6 +223,44 @@ func TestExpandHome(t *testing.T) {
}
}

// TestExpandHome_NamedUser covers the #181 ~user form: "~user" and
// "~user/…" resolve under that user's home. We look up the CURRENT user by
// name so the test doesn't depend on a fixed account existing, and compare
// against os.UserHomeDir. An unknown ~user is left literal (the path-
// existence check surfaces it), which we also pin.
func TestExpandHome_NamedUser(t *testing.T) {
u, err := user.Current()
if err != nil || u.Username == "" {
t.Skipf("no current user: %v", err)
}
// user.Lookup must resolve the same account (it can differ from
// UserHomeDir on some CI images); skip if it doesn't rather than assert
// on an environment quirk.
looked, err := user.Lookup(u.Username)
if err != nil {
t.Skipf("user.Lookup(%q) unsupported here: %v", u.Username, err)
}
home := looked.HomeDir

cases := []struct{ in, want string }{
{"~" + u.Username, home},
{"~" + u.Username + "/data", filepath.Join(home, "data")},
{"~" + u.Username + "/a/b", filepath.Join(home, "a", "b")},
}
for _, c := range cases {
if got := expandHome(c.in); got != c.want {
t.Errorf("expandHome(%q) = %q, want %q", c.in, got, c.want)
}
}

// An unknown user can't be resolved: the literal is returned unchanged so
// the downstream path-existence check reports it plainly.
const unknown = "~nsuchuser-tracebloc-181/x"
if got := expandHome(unknown); got != unknown {
t.Errorf("expandHome(%q) = %q, want it left literal", unknown, got)
}
}

// TestExitError_Methods pins the exit-code carrier: Error() surfaces
// the wrapped message (or a fallback when nil), and Code() returns the
// process exit code main() propagates.
Expand Down
109 changes: 83 additions & 26 deletions internal/cli/data.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,6 +15,7 @@ import (
"gopkg.in/yaml.v3"

"github.com/tracebloc/cli/internal/cluster"
"github.com/tracebloc/cli/internal/pathutil"
"github.com/tracebloc/cli/internal/push"
"github.com/tracebloc/cli/internal/schema"
"github.com/tracebloc/cli/internal/submit"
Expand DownExpand Up@@ -44,6 +45,11 @@ submits the ingestion run, and watches it to completion (streaming
logs + the final summary). ` + "`data validate`" + ` checks an ingest.yaml
locally first.

What a dataset looks like depends on the task:
tabular / time-series — a .csv file, or a folder with one .csv
image — a folder with labels.csv + images/
text — a folder with labels.csv + texts/

` + "`tracebloc cluster info`" + ` is the pre-flight you'd typically run
before the first ingest.`,
// A bare `tracebloc data` prints help; a mistyped subcommand errors with a
Expand All@@ -58,7 +64,7 @@ before the first ingest.`,
return cmd
}

// newDataIngestCmd implements `tracebloc data ingest <local-path>`.
// newDataIngestCmd implements `tracebloc data ingest <dataset>`.
//
// Phase 3 scope (now complete across PR-a + PR-b):
//
Expand DownExpand Up@@ -133,7 +139,7 @@ func newDataIngestCmd() *cobra.Command {
)

cmd := &cobra.Command{
Use: "ingest <local-path>",
Use: "ingest <dataset>",
Aliases: []string{"push"},
Short: "Ingest a local dataset into your workspace",
Long: `Ingests a local dataset into your workspace's storage,
Expand All@@ -143,14 +149,36 @@ infrastructure. Supports 9 tasks (image classification,
object/keypoint detection, text classification, masked language
modeling, and the tabular / time-series family); pick one with --task.

Expected local layout (image_classification shown):
<dataset> is the data itself. What it looks like depends on the task:

tabular / time-series — the dataset is a single CSV. Pass the .csv
file directly, or a folder holding exactly one .csv:

churn.csv (the .csv file itself)
or
churn/
data.csv (the one .csv in the folder)

image (classification, object/keypoint detection) — a folder with
labels.csv + an images/ subfolder:

cats_dogs/
labels.csv (required)
images/ (required)
001.jpg
...

<local-path>/
labels.csv (required)
images/ (required)
001.jpg
002.jpg
...
text (classification, masked language modeling) — a folder with
labels.csv + a texts/ subfolder:

reviews/
labels.csv (required)
texts/ (required)
001.txt
...

A bare .csv file is accepted only for the tabular / time-series family;
image and text datasets must be a folder.

Accepted image extensions: .jpg, .jpeg, or .png (case-insensitive).
All images in one dataset must share a single type — the cluster
Expand DownExpand Up@@ -358,25 +386,32 @@ type runDataIngestArgs struct {
ImageDigest string
}

// expandHome expands a leading ~ or ~/… to $HOME, leaving every other
// path (relative, absolute, empty) untouched. It mirrors
// cluster.expandPath — kept as a small local copy rather than coupling
// the data path-handling to the cluster package's internals; if a
// third caller appears, promote both to a shared pathutil.
// 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
// to the same helper, so ~-expansion is identical across subcommands
// (a --kubeconfig ~alice/... resolves alice's home just like a data
// ingest path does). See pathutil.ExpandHome for the full contract. (#181)
func expandHome(path string) string {
if path == "" || path[0] != '~' {
return path
}
home, err := os.UserHomeDir()
if err != nil {
// Can't resolve $HOME — leave it and let the downstream
// Discover* error mention the literal path, which is more
// useful than a generic failure here.
return path
return pathutil.ExpandHome(path)
}

// statDatasetPath is the "path existence FIRST" guard (#181): a typo'd
// path fails plainly on the path — a clean "no such file or directory" —
// before any family sniff, label preview, or schema work touches it.
// Both entry points call it: the flag-only path from runDataIngest's 0b
// step, and the guided path from runInteractive (before the family sniff),
// so the invariant holds on every route rather than only the flag path.
func statDatasetPath(path string) error {
if _, serr := os.Stat(path); serr != nil {
if errors.Is(serr, os.ErrNotExist) {
return &exitError{code: 3, err: fmt.Errorf(
"no such file or directory: %q — check the path to your dataset", path)}
}
return &exitError{code: 3, err: fmt.Errorf(
"can't read %q: %w", path, serr)}
}
// path[1:] is "" for "~" (→ home) and "/x" for "~/x" (→ home/x);
// filepath.Join cleans the join either way.
return filepath.Join(home, path[1:])
return nil
}

// runDataIngest is the full Phase 3 implementation: pre-flight
Expand DownExpand Up@@ -437,6 +472,14 @@ collaborators can train against that table without ever seeing the raw files.`))
a.Printer.Infof("Cancelled — nothing was ingested.")
return nil
}
// A typed exitError from a guided step (e.g. the path-existence
// guard, which runInteractive runs before the family sniff)
// already carries its own code + clean message — surface it as-is
// rather than burying it under "interactive setup:".
var ee *exitError
if errors.As(err, &ee) {
return err
}
return &exitError{code: 3, err: fmt.Errorf("interactive setup: %w", err)}
}
}
Expand All@@ -460,6 +503,20 @@ collaborators can train against that table without ever seeing the raw files.`))
// before any push.Discover* call. (#37)
a.LocalPath = expandHome(a.LocalPath)

// 0b. Path existence FIRST — before any spec / schema / family
// validation. A typo'd path should fail on the path with a plain
// "no such file or directory", not surface later as a confusing
// downstream error (e.g. the task gate asking which task the
// non-existent data is for). runInteractive runs this same guard
// before its family sniff / label preview, so the invariant holds on
// the guided route too; this re-check covers the flag-only path and
// is cheap (one stat). The family walk below stats again for its
// layout-specific diagnostics; this is only about ordering the first
// failure a customer sees. (#181)
if err := statDatasetPath(a.LocalPath); err != nil {
return err
}

// 1. Validate the table name BEFORE anything else. It's both
// the MySQL identifier and the /data/shared/<table>/ PVC
// subdirectory — an unsanitized traversal name (../../etc)
Expand Down
79 changes: 79 additions & 0 deletions internal/cli/data_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -212,6 +212,85 @@ func TestDataIngest_NonexistentLocalPath_ExitsThree(t *testing.T) {
}
}

// TestDataIngest_NonexistentPath_BeatsTaskGate: a typo'd path must fail on
// the path (exit 3), NOT on a downstream spec/family error, even when the
// task is also wrong. Pins the #181 ordering fix: path existence is checked
// before the category gate (which would otherwise exit 2 for the bad task
// and send the user chasing the wrong problem).
func TestDataIngest_NonexistentPath_BeatsTaskGate(t *testing.T) {
code, _, _ := execDataIngest(t, []string{
"/tmp/tracebloc-cli-test-no-such-dir-" + t.Name(),
"--name=t1",
"--task=definitely-not-a-task", // would be exit 2 at the task gate
"--intent=train",
})
if code != 3 {
t.Fatalf("expected exit 3 (path checked before the task gate), got %d", code)
}
}

// TestDataIngest_BareCSVFile_Accepted: a bare .csv is a valid tabular input
// (#181). It gets PAST the layout walk — proven by the "Inferred schema"
// line, which prints only after DiscoverTabular accepted the file — and then
// falls through the local checks to the injected bad kubeconfig (exit 3),
// the same fall-through a valid directory reaches.
func TestDataIngest_BareCSVFile_Accepted(t *testing.T) {
dir := t.TempDir()
csv := filepath.Join(dir, "churn.csv")
if err := os.WriteFile(csv, []byte("age,churned\n30,yes\n40,no\n"), 0o644); err != nil {
t.Fatalf("write csv: %v", err)
}
code, stdout, _ := execDataIngest(t, []string{
csv,
"--name=churn",
"--task=tabular_classification",
"--intent=train",
"--label-column=churned",
})
if code != 3 {
t.Fatalf("expected exit 3 (bare .csv accepted, then bad kubeconfig), got %d", code)
}
if !strings.Contains(stdout, "Inferred schema") {
t.Errorf("want the schema-inference line proving the bare .csv passed the walk; stdout:\n%s", stdout)
}
}

// TestDataIngest_ImageBareFile_ExitsThree: the image family is directory-only.
// A bare .csv passed as image_classification is rejected at the walk (exit 3)
// and never reaches schema inference (that's tabular-only).
func TestDataIngest_ImageBareFile_ExitsThree(t *testing.T) {
dir := t.TempDir()
csv := filepath.Join(dir, "labels.csv")
if err := os.WriteFile(csv, []byte("image_id,label\n1.jpg,c\n"), 0o644); err != nil {
t.Fatalf("write csv: %v", err)
}
code, stdout, _ := execDataIngest(t, []string{
csv,
"--name=imgs",
"--task=image_classification",
"--intent=train",
"--label-column=label",
})
if code != 3 {
t.Fatalf("expected exit 3 for a bare file passed as image, got %d", code)
}
if strings.Contains(stdout, "Inferred schema") {
t.Errorf("image walk must not run tabular schema inference on a bare file; stdout:\n%s", stdout)
}
}

// TestDataIngestCmd_UsesDatasetArgName: the positional arg is <dataset>
// (renamed from <local-path>, #181) in the command's Use string and help.
func TestDataIngestCmd_UsesDatasetArgName(t *testing.T) {
cmd := newDataIngestCmd()
if !strings.Contains(cmd.Use, "<dataset>") {
t.Errorf("Use = %q, want it to name the arg <dataset>", cmd.Use)
}
if strings.Contains(cmd.Use, "<local-path>") {
t.Errorf("Use = %q still uses the old <local-path> name", cmd.Use)
}
}

// TestDataIngest_MissingLabelsCSV_ExitsThree: most likely "real
// world" wrong-layout case — customer has images but forgot
// labels.csv. Pins the exit-code contract for the common failure
Expand Down
14 changes: 12 additions & 2 deletions internal/cli/interactive.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -136,8 +136,8 @@ func runInteractive(p *ui.Printer, pr prompter, a *runDataIngestArgs, taskSet bo

// (c) path — then detect the family from the layout and echo it back.
if a.LocalPath == "" {
p.PromptHint("The folder holding your data — a single .csv for a table, or labels.csv + an images/ folder for images. e.g. ~/datasets/churn")
ans, err := pr.Input("Where is your data? (the folder holding it)", "e.g. ./my-data", "", validateDatasetPath)
p.PromptHint("The file or folder holding your data — a single .csv for a table, or labels.csv + an images/ folder for images. e.g. ~/datasets/churn")
ans, err := pr.Input("Where is your data? (file or folder)", "e.g. ./my-data", "", validateDatasetPath)
if err != nil {
return err
}
Expand All@@ -153,6 +153,16 @@ func runInteractive(p *ui.Printer, pr prompter, a *runDataIngestArgs, taskSet bo
// the real path; runDataIngest's own expandHome then no-ops.
a.LocalPath = expandHome(a.LocalPath)

// Path existence FIRST (#181): fail plainly on a typo'd path here, before
// the family sniff / label preview below touch it — otherwise the user
// answers the whole questionnaire (family, task, label) against a path
// that doesn't exist, only to hit the hard error afterward. runDataIngest
// re-checks for the flag-only route; this keeps the invariant on the
// guided route too. The exitError propagates unwrapped (see runDataIngest).
if err := statDatasetPath(a.LocalPath); err != nil {
return err
}

// (d) task — family-scoped. An explicit --task wins and skips both the
// sniff and the picker (§5.1). Otherwise the family is sniffed from the
// layout (and echoed), or asked plainly when the layout is ambiguous,
Expand Down
Loading
Loading