Skip to content

test(parity): value-level label parity — catch accept/accept-with-divergent-data (#1009 P0)#186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
saadqbal merged 1 commit into from
Jul 8, 2026
Merged

test(parity): value-level label parity — catch accept/accept-with-divergent-data (#1009 P0) #186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
61 changes: 60 additions & 1 deletion internal/push/parity_golden_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,11 +4,13 @@ import (
"encoding/json"
"os"
"path/filepath"
"slices"
"sort"
"testing"
)

// The validator-parity harness (backend#828 P3). Two assertions per case:
// The validator-parity harness (backend#828 P3; value-level from backend#1009).
// Per case:
//
// 1. the Go preflight's verdict matches the manifest's cli_verdict —
// pins the CLI side;
Expand All@@ -17,6 +19,11 @@ import (
// manifest's ingestor_verdict — so when the ingestor's rules change,
// regenerating the goldens fails this test until the manifest (and,
// where needed, the Go preview) is consciously updated.
// 3. for cases flagged value_parity, the Go preview's VALUE-level read of
// the label column (resolved header + row count + class set) equals the
// REAL ingestor's — the only assertion that catches accept/accept with
// divergent stored data (data-ingestors #340: a case-/whitespace-
// mismatched label passes both verdicts, then reads null in-cluster).
//
// Deliberate divergences (the CLI previewing read-/transfer-time failures
// the ingestor's preflight can't see) are explicit in the manifest, never
Expand All@@ -32,6 +39,7 @@ type parityCase struct {
Schema map[string]string `json:"schema"`
CLIVerdict string `json:"cli_verdict"`
IngestorVerdict string `json:"ingestor_verdict"`
ValueParity bool `json:"value_parity"`
Note string `json:"note"`
}

Expand All@@ -45,6 +53,11 @@ func TestValidatorParity(t *testing.T) {
Verdicts map[string]struct {
Verdict string `json:"verdict"`
Errors []string `json:"errors"`
Values *struct {
Resolved string `json:"resolved_label"`
RowCount int `json:"row_count"`
Classes []string `json:"classes"`
} `json:"values"`
} `json:"verdicts"`
}
mustLoad(t, filepath.Join("testdata", "parity", "goldens.json"), &goldens)
Expand All@@ -64,10 +77,56 @@ func TestValidatorParity(t *testing.T) {
if got != c.CLIVerdict {
t.Errorf("Go preflight = %q, manifest expects %q (note: %s)", got, c.CLIVerdict, c.Note)
}

if !c.ValueParity {
return
}
// Value-level parity (backend#1009): the Go preview must read the
// SAME label header, row count, and class set the real ingestor
// does. Catches accept/accept-with-divergent-label (#340).
if golden.Values == nil {
t.Fatalf("case %s is value_parity but goldens.json has no values — "+
"regenerate with scripts/gen-validator-goldens.py against a data-ingestors "+
"checkout that includes the #340 label-resolution fix", c.Name)
}
gv := goLabelValues(t, c)
if gv.Resolved != golden.Values.Resolved {
t.Errorf("resolved label: Go preview = %q, ingestor golden = %q "+
"(the read paths resolve the label column differently — #340 class)",
gv.Resolved, golden.Values.Resolved)
}
if gv.RowCount != golden.Values.RowCount {
t.Errorf("row count: Go preview = %d, ingestor golden = %d", gv.RowCount, golden.Values.RowCount)
}
if !slices.Equal(gv.Classes, golden.Values.Classes) {
t.Errorf("class set: Go preview = %v, ingestor golden = %v", gv.Classes, golden.Values.Classes)
}
})
}
}

// goLabelValues runs the Go preview's value-level label read for a case,
// deriving the NA-drop / numeric-collapse flags from the label's schema type
// exactly as PreflightDataset does — so the value comparison uses the same
// read semantics the production preflight would.
func goLabelValues(t *testing.T, c parityCase) LabelReadValues {
t.Helper()
csvPath := filepath.Join("testdata", "parity", "cases", c.Name, c.CSV)
schema := c.Schema
if IsTabular(c.Category) && len(schema) == 0 {
if sch, _, _, err := InferSchema(csvPath); err == nil {
schema = sch
}
}
dropNA, collapse := false, false
if IsTabular(c.Category) {
sqlType, inSchema := labelSchemaType(schema, c.LabelColumn)
dropNA = inSchema
collapse = !(inSchema && isStringSQLType(sqlType))
}
return ReadLabelValues(csvPath, c.LabelColumn, dropNA, collapse)
}

// runGoPreflight runs THE production dispatch (push.PreflightDataset) over
// the case — the same code path runDataIngest executes, so a check deleted
// or rewired in production fails parity here.
Expand Down
82 changes: 64 additions & 18 deletions internal/push/preflight.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -383,9 +383,54 @@ func TruncateList(items []string, max int) string {
// 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 {
v := readLabelColumnValues(csvPath, labelColumn, dropNASentinels, collapseNumeric)
// Benign-skip when the column is absent (that's CheckLabelColumn's
// diagnostic) or an unreadable file (another check's) — both leave Found
// false. Two or more classes is diverse enough.
if !v.Found || len(v.Classes) >= 2 {
return nil
}
return fmt.Errorf(
"the label column %q has %d distinct value(s) — a classification dataset needs at "+
"least 2 classes. The cluster rejects this after the upload; check the labels and re-run.",
labelColumn, len(v.Classes))
}

// LabelReadValues is the value-level view of a label column: the header the
// read path RESOLVES the configured name to (case/whitespace-insensitively —
// the ingestor's rule), the sorted distinct classes the ingestor counts, and
// the data-row count. It is what the value-level parity harness pins, so a
// preview that says "N rows, K classes" cannot silently diverge from what the
// ingestor actually reads — the accept/accept-with-divergent-label class the
// verdict-only harness is blind to (data-ingestors #340).
type LabelReadValues struct {
Resolved string `json:"resolved_label"`
Classes []string `json:"classes"`
RowCount int `json:"row_count"`
Found bool `json:"-"`
}

// ReadLabelValues is the exported value-level read used by the parity harness
// (and, later, the RFC-0002 "check your data" preview). It shares the exact
// read/resolve/NA/collapse rules with CheckLabelDiversity via
// readLabelColumnValues, so the value preview and the diversity verdict cannot
// drift from each other.
func ReadLabelValues(csvPath, labelColumn string, dropNASentinels, collapseNumeric bool) LabelReadValues {
return readLabelColumnValues(csvPath, labelColumn, dropNASentinels, collapseNumeric)
}

// readLabelColumnValues reads csvPath's label column once and returns its
// value-level view. The column is resolved exactly, then case/whitespace-
// insensitively (mirroring the ingestor's resolve_column rule); each row value
// is whitespace-trimmed; NA sentinels are dropped and numeric values collapsed
// per the caller's flags (see CheckLabelDiversity's doc for how those mirror
// the ingestor's per-column read). Unlike the previous early-exit diversity
// scan, this reads the whole column to build the full class set + row count —
// one scan now backs both the diversity verdict and the value-level preview.
func readLabelColumnValues(csvPath, labelColumn string, dropNASentinels, collapseNumeric bool) LabelReadValues {
f, err := os.Open(csvPath)
if err != nil {
return nil // unreadable file is another check's diagnostic
return LabelReadValues{} // Found=false: unreadable file is another check's diagnostic
}
defer func() { _ = f.Close() }()
br := bufio.NewReader(f)
Expand All@@ -396,34 +441,39 @@ func CheckLabelDiversity(csvPath, labelColumn string, dropNASentinels, collapseN
r.FieldsPerRecord = -1
header, err := r.Read()
if err != nil {
return nil
return LabelReadValues{}
}
col:= -1
col, resolved := -1, ""
for i, c := range header {
if strings.TrimSpace(c) == labelColumn {
col= i
col, resolved = i, strings.TrimSpace(c)
break
}
}
if col == -1 {
want := strings.ToLower(strings.TrimSpace(labelColumn))
for i, c := range header {
if strings.ToLower(strings.TrimSpace(c)) == want {
col= i
col, resolved = i, strings.TrimSpace(c)
break
}
}
}
if col == -1 {
return nil // benign-skip, like the ingestor
return LabelReadValues{} // Found=false — benignskip, like the ingestor
}
distinct := map[string]bool{}
rowCount := 0
for {
rec, err := r.Read()
if errors.Is(err, io.EOF) {
break
}
if err != nil || len(rec) <= col {
if err != nil {
continue
}
rowCount++
if len(rec) <= col {
continue
}
v := strings.TrimSpace(rec[col])
Expand All@@ -435,22 +485,18 @@ func CheckLabelDiversity(csvPath, labelColumn string, dropNASentinels, collapseN
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 {
v = strconv.FormatFloat(f, 'g', -1, 64)
if fv, err := strconv.ParseFloat(v, 64); err == nil {
v = strconv.FormatFloat(fv, 'g', -1, 64)
}
}
distinct[v] = true
if len(distinct) >= 2 {
return nil
}
}
if len(distinct) >= 2 {
return nil
classes := make([]string, 0, len(distinct))
for k := range distinct {
classes = append(classes, k)
}
return fmt.Errorf(
"the label column %q has %d distinct value(s) — a classification dataset needs at "+
"least 2 classes. The cluster rejects this after the upload; check the labels and re-run.",
labelColumn, len(distinct))
sort.Strings(classes)
return LabelReadValues{Resolved: resolved, Classes: classes, RowCount: rowCount, Found: true}
}

// knownMediaExtensions mirrors the ingestor's FileExtension.get_all_extensions
Expand Down
27 changes: 18 additions & 9 deletions internal/push/testdata/parity/cases.json
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,7 +7,8 @@
"csv": "data.csv",
"label_column": "label",
"cli_verdict": "accept",
"ingestor_verdict": "accept"
"ingestor_verdict": "accept",
"value_parity": true
},
{
"name": "tabular-dup-header",
Expand DownExpand Up@@ -56,7 +57,8 @@
8
],
"cli_verdict": "accept",
"ingestor_verdict": "accept"
"ingestor_verdict": "accept",
"value_parity": true
},
{
"name": "imgc-bom-labels",
Expand All@@ -70,7 +72,8 @@
],
"cli_verdict": "accept",
"ingestor_verdict": "accept",
"note": "pandas strips the BOM in-cluster \u2014 the CLI must NOT reject what the cluster accepts (cli#71 parity)"
"note": "pandas strips the BOM in-cluster \u2014 the CLI must NOT reject what the cluster accepts (cli#71 parity)",
"value_parity": true
},
{
"name": "imgc-label-missing",
Expand DownExpand Up@@ -98,7 +101,8 @@
],
"cli_verdict": "accept",
"ingestor_verdict": "accept",
"note": "the ingestor's _match_column is case-insensitive + trimmed \u2014 the CLI must be exactly as loose"
"note": "the ingestor's _match_column is case-insensitive + trimmed \u2014 the CLI must be exactly as loose",
"value_parity": true
},
{
"name": "imgc-zero-byte",
Expand DownExpand Up@@ -196,7 +200,8 @@
],
"cli_verdict": "accept",
"ingestor_verdict": "accept",
"note": "non-square [W,H]=[8,4]: pins the target_size ORIENTATION end-to-end \u2014 an [H,W] swap on emit (the pre-P3 bug) flips this to reject in-cluster"
"note": "non-square [W,H]=[8,4]: pins the target_size ORIENTATION end-to-end \u2014 an [H,W] swap on emit (the pre-P3 bug) flips this to reject in-cluster",
"value_parity": true
},
{
"name": "imgc-nonsquare-swapped",
Expand DownExpand Up@@ -238,7 +243,8 @@
],
"cli_verdict": "accept",
"ingestor_verdict": "accept",
"note": "empty-string label IS a class for image categories (keep_default_na=False): the CLI must not be stricter"
"note": "empty-string label IS a class for image categories (keep_default_na=False): the CLI must not be stricter",
"value_parity": true
},
{
"name": "imgc-dotted-stem",
Expand All@@ -252,7 +258,8 @@
],
"cli_verdict": "accept",
"ingestor_verdict": "accept",
"note": "row 'photo.2024' resolves to photo.2024.jpg via _has_extension (dotted stems are not extensions) \u2014 the CLI's cross-check must mirror, not reject"
"note": "row 'photo.2024' resolves to photo.2024.jpg via _has_extension (dotted stems are not extensions) \u2014 the CLI's cross-check must mirror, not reject",
"value_parity": true
},
{
"name": "tabular-na-labels",
Expand DownExpand Up@@ -280,7 +287,8 @@
"extension": ".txt",
"cli_verdict": "accept",
"ingestor_verdict": "accept",
"note": "pins the text-family dispatch"
"note": "pins the text-family dispatch",
"value_parity": true
},
{
"name": "tabular-varchar-numeric-labels",
Expand All@@ -293,7 +301,8 @@
},
"cli_verdict": "accept",
"ingestor_verdict": "accept",
"note": "labels '1' vs '1.0' under a VARCHAR label: the ingestor pins dtype=str (no numeric collapse) so they are 2 classes \u2014 pins #152's schema-type-aware collapse (the earlier blanket collapse falsely rejected this)"
"note": "labels '1' vs '1.0' under a VARCHAR label: the ingestor pins dtype=str (no numeric collapse) so they are 2 classes \u2014 pins #152's schema-type-aware collapse (the earlier blanket collapse falsely rejected this)",
"value_parity": true
},
{
"name": "tabular-float-numeric-labels",
Expand Down
Loading
Loading