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
43 changes: 43 additions & 0 deletions internal/cli/coverage_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -194,6 +194,49 @@ func TestRunDatasetPush_OutputJSONEarlyFailureEmitsJSON(t *testing.T) {
}
}

// TestRunDataIngest_ImageOnlyFlagsRejectedOnNonImage: --target-size and
// --min-size describe image resolution, so on a tabular/text task they
// must fail fast (exit 2) with a clear message rather than being parsed
// only inside the image branch — where the value, even a malformed one,
// was silently dropped (#206 review).
func TestRunDataIngest_ImageOnlyFlagsRejectedOnNonImage(t *testing.T) {
cases := []struct {
name string
mutate func(*runDataIngestArgs)
}{
{"target-size on tabular", func(a *runDataIngestArgs) { a.TargetSizeFlag = "64x64" }},
{"min-size on tabular", func(a *runDataIngestArgs) { a.MinSizeFlag = "32x32" }},
{"malformed min-size on tabular", func(a *runDataIngestArgs) { a.MinSizeFlag = "garbage" }},
}
// A real, existing path so the earlier dataset-path stat passes and
// the image-only guard is what actually fires (the path check runs
// before the guard).
dir := t.TempDir()
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
var human bytes.Buffer
a := runDataIngestArgs{
LocalPath: dir,
Spec: push.SpecArgs{
Table: "t", Category: "tabular_classification",
Intent: "train", LabelColumn: "y",
},
Printer: ui.New(&human, ui.WithColor(false)),
}
c.mutate(&a)
err := runDataIngest(context.Background(), &human, &human, a)

var ee *exitError
if !errors.As(err, &ee) || ee.Code() != 2 {
t.Fatalf("err = %v, want *exitError code 2", err)
}
if !strings.Contains(ee.Error(), "image tasks only") {
t.Errorf("error should explain the flag is image-only; got: %v", ee)
}
})
}
}

// TestExpandHome covers the #37 fix: a leading ~ / ~/… resolves under
// $HOME, while relative, absolute, and empty paths pass through
// untouched (the case that bit the interactive prompt — the shell
Expand Down
45 changes: 43 additions & 2 deletions internal/cli/data.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -103,6 +103,7 @@ func newDataIngestCmd() *cobra.Command {
intent string
labelColumn string
targetSize string
minSize string
schemaFlag string
labelPolicy string
timeColumn string
Expand DownExpand Up@@ -231,6 +232,7 @@ Exit codes:
NumberOfKeypoints: numberOfKeypoints,
},
TargetSizeFlag: targetSize,
MinSizeFlag: minSize,
SchemaFlag: schemaFlag,
DryRun: dryRun,
Overwrite: overwrite,
Expand DownExpand Up@@ -276,8 +278,13 @@ Exit codes:
cmd.Flags().StringVar(&labelColumn, "label-column", "",
"name of the label/target column (in labels.csv for image tasks, in the data CSV for tabular)")
cmd.Flags().StringVar(&targetSize, "target-size", "",
"image tasks only: resolution as WxH (e.g. 512x512). Default: auto-detected from the first image. "+
"All images must share this resolution — the ingestor validates it, it does not resize.")
"image tasks only: the resolution your images already are, as WxH (e.g. 512x512). tracebloc never "+
"resizes — it checks every image is exactly this size and rejects any that differ. Default: "+
"read from your first image.")
cmd.Flags().StringVar(&minSize, "min-size", "",
"image tasks only: reject images smaller than WxH before the ingest (e.g. 64x64). Set it to the "+
"smallest size your model can train on — raise or lower it freely. Default: unset (no local "+
"size check).")
cmd.Flags().StringVar(&schemaFlag, "schema", "",
"tabular/time-series only: column types as col:TYPE,col:TYPE (e.g. age:INT,price:FLOAT). "+
"Default: inferred from the CSV (INT/FLOAT/VARCHAR).")
Expand DownExpand Up@@ -326,6 +333,7 @@ type runDataIngestArgs struct {
Namespace string
Spec push.SpecArgs
TargetSizeFlag string // raw --target-size; resolved after Discover (image)
MinSizeFlag string // raw --min-size; resolved after Discover (image) — #348 floor override
SchemaFlag string // raw --schema; resolved or inferred after Discover (tabular)
DryRun bool
Overwrite bool
Expand DownExpand Up@@ -510,6 +518,24 @@ collaborators can train against that table without ever seeing the raw files.`))
a.Spec.Category, push.SupportedCategoriesList())}
}

// Image-only flags. --target-size / --min-size describe image
// resolution, so they're meaningless on a tabular / text task.
// Reject them explicitly here: without this guard they'd be parsed
// only inside the image branch below, so on a non-image task the
// value — even a malformed one — was silently dropped with no error.
if !push.IsImage(a.Spec.Category) {
for _, f := range []struct{ name, val string }{
{"--target-size", a.TargetSizeFlag},
{"--min-size", a.MinSizeFlag},
} {
if f.val != "" {
return &exitError{code: 2, err: fmt.Errorf(
"%s is image tasks only; it doesn't apply to task %q",
f.name, a.Spec.Category)}
}
}
}

// 3. Walk the local directory FIRST (local "fail fast"), dispatched
// by category family. Image categories expect labels.csv +
// images/; tabular / time-series categories expect a single
Expand DownExpand Up@@ -616,6 +642,21 @@ collaborators can train against that table without ever seeing the raw files.`))
"resolution mismatch.\n", derr)
}
}
// Minimum-size floor override (#348): plumb an explicit --min-size to
// spec.file_options.min_size. When unset, no spec field is emitted, so
// the ingestor applies its own default (none on the deployed
// v0.5.7/v0.6.0; 32x32 on develop post-#348) — and the local preview
// applies NO floor either (PreflightDataset only previews the floor
// when --min-size is set, so it never rejects an ingest the live
// cluster accepts). The below-floor reject is previewed in
// runLocalPreflight (ValidateImages).
if a.MinSizeFlag != "" {
w, h, perr := push.ParseMinSize(a.MinSizeFlag)
if perr != nil {
return &exitError{code: 2, err: perr}
}
a.Spec.MinSize = []int{w, h}
}
// Extension: every image must share one type, and the spec tells
// the cluster which one to validate against (file_options.extension).
// Without this the ingestor checked its .jpeg convention default and
Expand Down
12 changes: 9 additions & 3 deletions internal/cli/interactive.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -307,9 +307,9 @@ func promptCategorySpecific(p *ui.Printer, pr prompter, a *runDataIngestArgs) (b
prompted = true
}
if a.TargetSizeFlag == "" {
p.PromptHint("All images must share one resolution; the ingestor checks it (it won't resize). Blank = auto-detect from the first image. e.g. 224x224")
ans, err := pr.Input("Image resolution as WxH (blank = auto-detect from the first image)",
"all images must share it; the ingestor validates, it doesn't resize", "",
p.PromptHint("The resolution your images already are. tracebloc never resizes — it checks every image is exactly this size and rejects any that differ. Blank = read it from your first image. e.g. 224x224")
ans, err := pr.Input("Image resolution as WxH (blank = read it from your first image)",
"the size your images already are; tracebloc checks it, it never resizes", "",
validateOptionalTargetSize)
if err != nil {
return prompted, err
Expand DownExpand Up@@ -402,6 +402,12 @@ func renderReview(p *ui.Printer, a *runDataIngestArgs) {
case push.IsImage(a.Spec.Category):
p.Field("resolution", "auto-detect")
}
// Only shown when set — --min-size is opt-in with no local default,
// so there's nothing to echo otherwise. Surfacing it lets a mistyped
// floor (e.g. 640x640 for 64x64) be caught at the confirm gate.
if a.MinSizeFlag != "" {
p.Field("min size", a.MinSizeFlag)
}
switch {
case a.SchemaFlag != "":
p.Field("schema", a.SchemaFlag)
Expand Down
23 changes: 19 additions & 4 deletions internal/push/detect.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -44,28 +44,43 @@ func DetectImageSize(path string) (width, height int, err error) {
// height]. Accepts "WxH" (the documented form, e.g. "512x512") and
// "W,H" as a convenience. Both dimensions must be positive integers.
func ParseTargetSize(s string) (width, height int, err error) {
return parseWxH("target size", s)
}

// ParseMinSize parses a --min-size flag value into [width, height],
// the same WxH grammar as --target-size (#183). It plumbs to
// spec.file_options.min_size, the ingestor's minimum-image-size floor
// override (data-ingestors #348).
func ParseMinSize(s string) (width, height int, err error) {
return parseWxH("min size", s)
}

// parseWxH parses a "WxH" (or "W,H") dimension pair, using kind in
// error messages so callers surface "target size …" / "min size …"
// verbatim. Both dimensions must be positive integers.
func parseWxH(kind, s string) (width, height int, err error) {
sep := "x"
if strings.Contains(s, ",") {
sep = ","
}
parts := strings.Split(s, sep)
if len(parts) != 2 {
return 0, 0, fmt.Errorf(
"target size %q must be WxH (e.g. 512x512)", s)
"%s %q must be WxH (e.g. 512x512)", kind, s)
}
width, err = strconv.Atoi(strings.TrimSpace(parts[0]))
if err != nil {
return 0, 0, fmt.Errorf(
"target size %q: width is not an integer: %w", s, err)
"%s %q: width is not an integer: %w", kind, s, err)
}
height, err = strconv.Atoi(strings.TrimSpace(parts[1]))
if err != nil {
return 0, 0, fmt.Errorf(
"target size %q: height is not an integer: %w", s, err)
"%s %q: height is not an integer: %w", kind, s, err)
}
if width <= 0 || height <= 0 {
return 0, 0, fmt.Errorf(
"target size %q: width and height must both be positive", s)
"%s %q: width and height must both be positive", kind, s)
}
return width, height, nil
}
2 changes: 2 additions & 0 deletions internal/push/parity_golden_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -36,6 +36,7 @@ type parityCase struct {
LabelColumn string `json:"label_column"`
Extension string `json:"extension"`
TargetSize []int `json:"target_size"`
MinSize []int `json:"min_size"`
Schema map[string]string `json:"schema"`
CLIVerdict string `json:"cli_verdict"`
IngestorVerdict string `json:"ingestor_verdict"`
Expand DownExpand Up@@ -143,6 +144,7 @@ func runGoPreflight(t *testing.T, c parityCase) string {
LabelColumn: c.LabelColumn,
Extension: c.Extension,
TargetSize: c.TargetSize,
MinSize: c.MinSize,
}
if IsTabular(c.Category) {
// Mirror runDataIngest: an explicit schema (the --schema flow) wins,
Expand Down
58 changes: 50 additions & 8 deletions internal/push/preflight.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -188,18 +188,30 @@ func CheckHasDataRows(path string) error {

// ValidateImages previews the ingestor's ImageResolutionValidator
// (image_validator.py): it opens EVERY image (header-only decode — cheap)
// and rejects zero-byte files, undecodable files, and any image whose
// resolution differs from the expected size (exact equality, zero
// tolerance — the ingestor validates, it does not resize). Previously the
// CLI decoded only the first image, so a single odd-sized or corrupt file
// failed in-cluster after the full upload (cli#72b/c).
// and rejects zero-byte files, undecodable files, images below the
// minimum-size floor, and any image whose resolution differs from the
// expected size (exact equality, zero tolerance — the ingestor validates,
// it does not resize). Previously the CLI decoded only the first image, so
// a single odd-sized or corrupt file failed in-cluster after the full
// upload (cli#72b/c).
//
// expectedW/expectedH of 0 skips the resolution comparison (the caller
// couldn't establish a target size — the ingestor would then auto-detect
// from its first file, which the CLI's detection already mirrors).
func ValidateImages(images []string, expectedW, expectedH int) error {
//
// minW/minH is the minimum-size floor (#348), mirroring the ingestor's
// _meets_min_size: an image is too small when EITHER side is below the
// floor; an image exactly at the floor passes. 0/0 disables the floor.
// PreflightDataset passes a non-zero floor ONLY when the customer set
// --min-size — it does NOT default to MinImageSize, because the deployed
// ingestor has no floor yet (see the PreflightDataset image branch), so a
// default block would reject an ingest the live cluster accepts. The
// too-small check takes precedence over the resolution mismatch, exactly
// as data-ingestors #348 returns the too_small error before the
// target_size uniformity error.
func ValidateImages(images []string, expectedW, expectedH, minW, minH int) error {
const maxListed = 5
var broken, mismatched []string
var broken, tooSmall, mismatched []string
for _, path := range images {
name := filepath.Base(path)
f, err := os.Open(path)
Expand All@@ -217,11 +229,24 @@ func ValidateImages(images []string, expectedW, expectedH int) error {
}
continue
}
if minW > 0 && minH > 0 && (cfg.Width < minW || cfg.Height < minH) {
tooSmall = append(tooSmall,
fmt.Sprintf("%s (%dx%d)", name, cfg.Width, cfg.Height))
}
if expectedW > 0 && expectedH > 0 && (cfg.Width != expectedW || cfg.Height != expectedH) {
mismatched = append(mismatched,
fmt.Sprintf("%s (%dx%d)", name, cfg.Width, cfg.Height))
}
}
// Floor first: an image below the minimum size simply can't be trained
// on, so it's the most fundamental, actionable failure — data-ingestors
// #348 returns it ahead of the uniformity / target_size mismatch.
if len(tooSmall) > 0 {
return fmt.Errorf(
"%d image(s) are smaller than the %dx%d minimum you set with --min-size: %s. "+
"Provide larger images, or lower the floor with --min-size, then re-run.",
len(tooSmall), minW, minH, TruncateList(tooSmall, maxListed))
}
if len(broken) > 0 {
return fmt.Errorf(
"%d image(s) can't be ingested: %s. The cluster rejects these after the upload — "+
Expand DownExpand Up@@ -688,7 +713,24 @@ func PreflightDataset(spec SpecArgs, layout *LocalLayout) (notes []string, probl
if len(spec.TargetSize) == 2 {
expW, expH = spec.TargetSize[0], spec.TargetSize[1]
}
if err := ValidateImages(layout.Images, expW, expH); err != nil {
// Minimum-size floor (#348). The floor lives in data-ingestors only
// on develop (di#348/#356); the DEPLOYED ingestor (v0.5.7/v0.6.0) has
// no floor and ingests small images fine. So the preview must NOT
// apply the 32x32 default on its own — a default block would reject an
// ingest the live cluster accepts, the inverse of the tabular-BOM
// block (whose reject mirrors a real deployed rejection). Apply the
// floor ONLY when the customer explicitly set --min-size (spec.MinSize)
// — their own declared requirement, honored locally regardless of the
// cluster. Once di#348 reaches prod, default this to MinImageSize and
// flip the imgc-too-small parity case so the floor is previewed by
// default. The emit side already matches: it omits file_options.min_size
// when unset, letting whichever ingestor is deployed apply its own
// default (none today; MinImageSize post-#348).
minW, minH := 0, 0
if len(spec.MinSize) == 2 {
minW, minH = spec.MinSize[0], spec.MinSize[1]
}
if err := ValidateImages(layout.Images, expW, expH, minW, minH); err != nil {
return nil, dataProblem(err)
}
if err := CheckHasDataRows(layout.LabelsCSV); err != nil {
Expand Down
59 changes: 54 additions & 5 deletions internal/push/preflight_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -127,28 +127,77 @@ func TestValidateImages(t *testing.T) {
zero := write("zero.png", nil)
corrupt := write("corrupt.png", []byte("not an image at all"))

if err := ValidateImages([]string{good}, 8, 8); err != nil {
// minW/minH of 0 disables the floor so these decode/mismatch cases
// exercise the same behavior as before the #348 floor landed (the
// 8x8 / 4x4 fixtures are below the real 32x32 default).
if err := ValidateImages([]string{good}, 8, 8, 0, 0); err != nil {
t.Errorf("valid image rejected: %v", err)
}
if err := ValidateImages([]string{good, zero}, 8, 8); err == nil {
if err := ValidateImages([]string{good, zero}, 8, 8, 0, 0); err == nil {
t.Fatal("zero-byte image must be rejected (cli#72b)")
} else if !strings.Contains(err.Error(), "0 bytes") {
t.Errorf("zero-byte diagnosis missing: %v", err)
}
if err := ValidateImages([]string{good, corrupt}, 8, 8); err == nil {
if err := ValidateImages([]string{good, corrupt}, 8, 8, 0, 0); err == nil {
t.Fatal("corrupt image must be rejected (cli#72b)")
}
if err := ValidateImages([]string{good, odd}, 8, 8); err == nil {
if err := ValidateImages([]string{good, odd}, 8, 8, 0, 0); err == nil {
t.Fatal("resolution mismatch must be rejected (cli#72c — the ingestor validates, it does not resize)")
} else if !strings.Contains(err.Error(), "4x4") || !strings.Contains(err.Error(), "8x8") {
t.Errorf("mismatch error must show both sizes: %v", err)
}
// 0x0 expectation skips the resolution comparison entirely.
if err := ValidateImages([]string{good, odd}, 0, 0); err != nil {
if err := ValidateImages([]string{good, odd}, 0, 0, 0, 0); err != nil {
t.Errorf("no expected size → no resolution rejection: %v", err)
}
}

// TestValidateImagesMinSize covers the #348 minimum-size floor preview:
// an image below the floor is rejected (naming the file, its dimensions,
// and the floor); an image exactly at the floor passes; the floor takes
// precedence over a target_size mismatch; and it mirrors the ingestor's
// default (push.MinImageSize).
func TestValidateImagesMinSize(t *testing.T) {
dir := t.TempDir()
write := func(name string, w, h int) string {
p := filepath.Join(dir, name)
if err := os.WriteFile(p, pngBytes(t, w, h), 0o644); err != nil {
t.Fatal(err)
}
return p
}
minW, minH := MinImageSize[0], MinImageSize[1] // 32x32, mirrors data-ingestors #348

atFloor := write("at_floor.png", minW, minH)
aboveFloor := write("above.png", minW+16, minH+16)
belowW := write("below_w.png", minW-1, minH) // one side under → too small
tiny := write("tiny.png", 8, 8) // both sides under

// At or above the floor passes (exact-floor image is accepted).
if err := ValidateImages([]string{atFloor, aboveFloor}, 0, 0, minW, minH); err != nil {
t.Errorf("at/above-floor images rejected: %v", err)
}
// One side below the floor → rejected, naming the file, its size, and the floor.
err := ValidateImages([]string{atFloor, belowW}, 0, 0, minW, minH)
if err == nil {
t.Fatal("below-floor image must be rejected (#348)")
}
for _, want := range []string{"below_w.png", "31x32", "32x32", "min-size"} {
if !strings.Contains(err.Error(), want) {
t.Errorf("too-small error missing %q: %v", want, err)
}
}
// The floor takes precedence over a target_size mismatch: tiny is both
// below the floor AND != the 64x64 target, but the too-small message wins.
err = ValidateImages([]string{tiny}, 64, 64, minW, minH)
if err == nil {
t.Fatal("tiny image must be rejected")
}
if !strings.Contains(err.Error(), "minimum") {
t.Errorf("floor must take precedence over the mismatch message: %v", err)
}
}

func TestCrossCheckLabels(t *testing.T) {
dir := t.TempDir()
imgs := filepath.Join(dir, "images")
Expand Down
Loading
Loading