From a3fd2777e5dd75748ab88fc409706c03aac5f7cf Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Tue, 7 Jul 2026 14:26:10 +0500 Subject: [PATCH] fix(preflight): label diversity respects the label's SQL type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CheckLabelDiversity collapsed numeric-looking labels ("1"/"1.0") for every tabular_classification dataset. The in-cluster LabelDiversityValidator pins dtype=str for string-family schema types (VARCHAR/CHAR/TEXT/STRING), so those labels stay distinct — only numeric types get pandas numeric inference (data-ingestors #252). A user-declared VARCHAR label with numeric-looking classes was wrongly rejected at preflight. Derive the drop-NA and collapse-numeric flags from the label's declared schema type at the dispatch site; keep image/text (untyped) unchanged. Adds leaf + dispatch tests. This aligns the Go side with the golden generator, which already types columns as VARCHAR. Co-Authored-By: Claude Opus 4.8 --- internal/push/preflight.go | 75 +++++++++++++++++++++++++++------ internal/push/preflight_test.go | 42 ++++++++++++++++-- 2 files changed, 102 insertions(+), 15 deletions(-) diff --git a/internal/push/preflight.go b/internal/push/preflight.go index 6d19d78..9c6a6cf 100644 --- a/internal/push/preflight.go +++ b/internal/push/preflight.go @@ -372,13 +372,17 @@ func TruncateList(items []string, max int) string { // in-cluster rejection otherwise lands after the full upload. Mirrors the // validator's benign-skip when the label column isn't found (that's // CheckLabelColumn's diagnostic, not this one's). -// tabularSchema is non-nil for tabular_classification: the label is a -// SCHEMA-TYPED column there, so pandas drops NA-sentinel values to NaN -// (not classes) and numeric inference collapses "1"/"1.0" — the preview -// mirrors both. For image/text classification the label column is read -// untyped with keep_default_na=False, so even an empty string is a real -// class and every distinct trimmed string counts. -func CheckLabelDiversity(csvPath, labelColumn string, tabularSchema bool) error { +// +// dropNASentinels and collapseNumeric mirror the ingestor's per-column +// read (LabelDiversityValidator._label_read_kwargs): for a SCHEMA-TYPED +// tabular label pandas drops NA-sentinel values (na_values) and, for +// NUMERIC types only, numeric inference collapses "1"/"1.0" — but a +// string-family type (VARCHAR/CHAR/TEXT/STRING) is pinned to dtype=str, so +// numeric-looking labels stay distinct (data-ingestors #252). Image/text +// labels are read untyped with keep_default_na=False (both flags false), so +// even an empty string is a real class and every distinct trimmed string +// counts. The caller derives the two flags from the label's schema type. +func CheckLabelDiversity(csvPath, labelColumn string, dropNASentinels, collapseNumeric bool) error { f, err := os.Open(csvPath) if err != nil { return nil // unreadable file is another check's diagnostic @@ -423,10 +427,12 @@ func CheckLabelDiversity(csvPath, labelColumn string, tabularSchema bool) error continue } v := strings.TrimSpace(rec[col]) - if tabularSchema { + if dropNASentinels { if _, isNA := naSentinels[v]; isNA { continue } + } + if collapseNumeric { // Numeric inference collapses "1" and "1.0" into one value // in-cluster; normalize the same way before counting. if f, err := strconv.ParseFloat(v, 64); err == nil { @@ -500,6 +506,39 @@ var naSentinels = map[string]struct{}{ "None": {}, "none": {}, "NaN": {}, "nan": {}, "": {}, "#N/A": {}, } +// labelSchemaType resolves the label column's declared SQL type from the +// schema, matched case- and whitespace-insensitively — mirrors the ingestor's +// LabelDiversityValidator._schema_type_for. ok is false when the label isn't +// a schema column (an untyped read, in-cluster). +func labelSchemaType(schema map[string]string, labelColumn string) (sqlType string, ok bool) { + if t, found := schema[labelColumn]; found { + return t, true + } + target := strings.ToLower(strings.TrimSpace(labelColumn)) + for k, v := range schema { + if strings.ToLower(strings.TrimSpace(k)) == target { + return v, true + } + } + return "", false +} + +// isStringSQLType reports whether an SQL type declaration is a string family +// (VARCHAR/CHAR/TEXT/STRING) — the types the ingestor pins to dtype=str, +// which suppresses pandas numeric inference on the label column. Mirrors the +// base-type check in LabelDiversityValidator._label_read_kwargs. +func isStringSQLType(sqlType string) bool { + base := strings.ToUpper(strings.TrimSpace(sqlType)) + if i := strings.IndexByte(base, '('); i >= 0 { + base = strings.TrimSpace(base[:i]) + } + switch base { + case "VARCHAR", "CHAR", "TEXT", "STRING": + return true + } + return false +} + // CheckSchemaColumns previews DataValidator's missing-schema-column probe // ("Schema columns not present in CSV: …"): every schema column must appear // in the header, compared stripped but case-SENSITIVE — exactly the probe's @@ -578,7 +617,16 @@ func PreflightDataset(spec SpecArgs, layout *LocalLayout) (notes []string, probl return nil, &PreflightProblem{Err: err, BadFlag: true} } if spec.Category == "tabular_classification" { - if err := CheckLabelDiversity(layout.LabelsCSV, spec.LabelColumn, true); err != nil { + // The label is a schema-typed column: the ingestor drops NA + // sentinels for it, and collapses numeric-looking values ONLY + // for numeric types — a VARCHAR label is pinned to dtype=str, + // keeping "1"/"1.0" distinct (data-ingestors #252). Derive both + // flags from the label's declared type so the preview doesn't + // wrongly collapse a string label and reject a diverse dataset. + sqlType, inSchema := labelSchemaType(spec.Schema, spec.LabelColumn) + dropNA := inSchema + collapseNumeric := !(inSchema && isStringSQLType(sqlType)) + if err := CheckLabelDiversity(layout.LabelsCSV, spec.LabelColumn, dropNA, collapseNumeric); err != nil { return nil, dataProblem(err) } } @@ -610,8 +658,9 @@ func PreflightDataset(spec SpecArgs, layout *LocalLayout) (notes []string, probl // LabelDiversityValidator runs in-cluster for the WHOLE image // family (is_classification covers object_detection + keypoint // too); it benign-skips when no label column resolves, and so - // does the preview. - if err := CheckLabelDiversity(layout.LabelsCSV, spec.LabelColumn, false); err != nil { + // does the preview. Image labels are read untyped, so no NA drop + // and no numeric collapse. + if err := CheckLabelDiversity(layout.LabelsCSV, spec.LabelColumn, false, false); err != nil { return nil, dataProblem(err) } switch spec.Category { @@ -664,7 +713,9 @@ func PreflightDataset(spec SpecArgs, layout *LocalLayout) (notes []string, probl if err := CheckLabelColumn(header, spec.LabelColumn, "labels.csv"); err != nil { return nil, &PreflightProblem{Err: err, BadFlag: true} } - if err := CheckLabelDiversity(layout.LabelsCSV, spec.LabelColumn, false); err != nil { + // Text labels are read untyped (like image), so no NA drop and + // no numeric collapse. + if err := CheckLabelDiversity(layout.LabelsCSV, spec.LabelColumn, false, false); err != nil { return nil, dataProblem(err) } } diff --git a/internal/push/preflight_test.go b/internal/push/preflight_test.go index 1baa772..ed546f4 100644 --- a/internal/push/preflight_test.go +++ b/internal/push/preflight_test.go @@ -202,17 +202,53 @@ func TestCheckLabelDiversity(t *testing.T) { // (stripped) label values. Discovered by the parity harness's first // run; previously an in-cluster-only, post-upload failure. two := writeTmp(t, "two.csv", []byte("id,label\na,cat\nb, cat \nc,dog\n")) - if err := CheckLabelDiversity(two, "label", false); err != nil { + if err := CheckLabelDiversity(two, "label", false, false); err != nil { t.Errorf("2 distinct labels rejected: %v", err) } one := writeTmp(t, "one.csv", []byte("id,label\na,cat\nb,cat\n")) - if err := CheckLabelDiversity(one, "label", false); err == nil { + if err := CheckLabelDiversity(one, "label", false, false); err == nil { t.Fatal("single-class dataset must be rejected") } else if !strings.Contains(err.Error(), "at least 2 classes") { t.Errorf("unexpected message: %v", err) } // benign-skip when the column is absent (that's CheckLabelColumn's job) - if err := CheckLabelDiversity(one, "nope", false); err != nil { + if err := CheckLabelDiversity(one, "nope", false, false); err != nil { t.Errorf("missing column must benign-skip like the ingestor: %v", err) } + + // Schema-type sensitivity (data-ingestors #252): "1" and "1.0" are two + // distinct classes for a string-typed (VARCHAR) label — the ingestor + // pins dtype=str and does NOT collapse them — but one class for a + // numeric label, where pandas numeric inference merges them. + numeric := writeTmp(t, "numeric.csv", []byte("id,label\na,1\nb,1.0\n")) + if err := CheckLabelDiversity(numeric, "label", true /*dropNA*/, false /*collapseNumeric*/); err != nil { + t.Errorf("VARCHAR label '1'/'1.0' must stay 2 classes (no numeric collapse): %v", err) + } + if err := CheckLabelDiversity(numeric, "label", true /*dropNA*/, true /*collapseNumeric*/); err == nil { + t.Error("numeric label '1'/'1.0' must collapse to a single class and be rejected") + } +} + +func TestCheckLabelDiversitySchemaTypeDispatch(t *testing.T) { + // PreflightDataset must derive the collapse flag from the label's schema + // type: a numeric-looking VARCHAR label is accepted (2 classes), the same + // data typed FLOAT is rejected (collapses to 1). Locks the fix at the + // dispatch level, not just the leaf function. + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "data.csv"), []byte("feat,label\nx,1\ny,1.0\n"), 0o644); err != nil { + t.Fatal(err) + } + layout := &LocalLayout{Root: dir, LabelsCSV: filepath.Join(dir, "data.csv")} + + strSpec := SpecArgs{Category: "tabular_classification", LabelColumn: "label", + Schema: map[string]string{"feat": "VARCHAR(255)", "label": "VARCHAR(10)"}} + if _, problem := PreflightDataset(strSpec, layout); problem != nil { + t.Errorf("VARCHAR label should keep '1'/'1.0' distinct and pass: %v", problem.Err) + } + + numSpec := SpecArgs{Category: "tabular_classification", LabelColumn: "label", + Schema: map[string]string{"feat": "VARCHAR(255)", "label": "FLOAT"}} + if _, problem := PreflightDataset(numSpec, layout); problem == nil { + t.Error("FLOAT label should collapse '1'/'1.0' and be rejected") + } }