diff --git a/internal/cli/data.go b/internal/cli/data.go index 74790d2e..812839df 100644 --- a/internal/cli/data.go +++ b/internal/cli/data.go @@ -559,12 +559,12 @@ collaborators can train against that table without ever seeing the raw files.`)) case push.IsCLISupported(a.Spec.Category): // supported case push.IsKnown(a.Spec.Category): - // A recognized category data ingest doesn't implement yet — image - // (semantic_segmentation) or text (causal_language_modeling, seq2seq, - // …). Routed here (not the default branch) so the - // user gets the registry's per-category pending-support reason, not a - // misleading "unrecognized category". Supported categories were already - // caught above, so IsKnown here means known-but-unsupported. + // A recognized category data ingest doesn't implement yet — today just + // semantic_segmentation (awaiting the ingestor's mask_id link column + + // training sign-off, backend#816). Routed here (not the default branch) + // so the user gets the registry's per-category pending-support reason, + // not a misleading "unrecognized category". Supported categories were + // already caught above, so IsKnown here means known-but-unsupported. spec, _ := push.Lookup(a.Spec.Category) return &exitError{code: 2, err: fmt.Errorf( "task %q isn't supported by the CLI yet (%s). Supported tasks: %s.", @@ -724,9 +724,13 @@ collaborators can train against that table without ever seeing the raw files.`)) } a.Spec.Extension = ext default: - // Text family: no extra per-category resolution. The label (for - // text_classification) comes straight from --label-column; - // masked_language_modeling needs neither a label nor a schema. + // Text family: no extra per-category resolution. The supervised text + // tasks (text_classification, token_classification, + // sentence_pair_classification) carry a label straight from + // --label-column; the self-supervised ones (masked/causal language + // modeling, seq2seq, embeddings) need neither a label nor a schema. + // buildText emits the label for exactly the supervised set, keyed on + // the registry's SelfSupervised flag (not a hardcoded id). } // 4. Synthesize the spec from flags + validate against schema. diff --git a/internal/cli/data_test.go b/internal/cli/data_test.go index 387ab464..1ca119c1 100644 --- a/internal/cli/data_test.go +++ b/internal/cli/data_test.go @@ -94,7 +94,7 @@ func execDataIngest(t *testing.T, args []string) (exitCode int, stdout, stderr s func TestDataIngest_UnsupportedCategory_ExitsTwo(t *testing.T) { root := imgcLayout(t) for _, badCategory := range []string{ - "semantic_segmentation", // known but blocked on the ingestor (data-ingestors#136) + "semantic_segmentation", // known but pending (awaiting mask_id + training sign-off, backend#816) "instance_segmentation", // dead — removed from the registry (#1005), now unrecognized "definitely-not-a-category", // nonsense; gate catches this too } { @@ -114,11 +114,11 @@ func TestDataIngest_UnsupportedCategory_ExitsTwo(t *testing.T) { } // TestDataIngest_KnownUnsupportedCategory_PendingNote pins the Bugbot fix -// (v0.4.0 RC): a registry-known but CLI-unsupported NON-image category -// (causal_language_modeling) must get the registry's pending-support note, not -// the misleading "isn't a recognized task category" message. execDataIngest -// discards the error and SilenceErrors swallows it, so run the command here and -// inspect the returned error directly. +// (v0.4.0 RC): a registry-known but CLI-unsupported category +// (semantic_segmentation — the sole remaining one after phase 4) must get the +// registry's pending-support note, not the misleading "isn't a recognized task +// category" message. execDataIngest discards the error and SilenceErrors +// swallows it, so run the command here and inspect the returned error directly. func TestDataIngest_KnownUnsupportedCategory_PendingNote(t *testing.T) { root := imgcLayout(t) rootCmd := NewRootCmd(BuildInfo{Version: "test"}) @@ -126,7 +126,7 @@ func TestDataIngest_KnownUnsupportedCategory_PendingNote(t *testing.T) { rootCmd.SetErr(&bytes.Buffer{}) rootCmd.SetArgs([]string{"data", "ingest", "--kubeconfig=/tmp/tracebloc-cli-test-nonexistent-" + t.Name(), - root, "--name=t1", "--task=causal_language_modeling", + root, "--name=t1", "--task=semantic_segmentation", "--intent=train", "--label-column=label"}) err := rootCmd.Execute() if err == nil { diff --git a/internal/cli/interactive_test.go b/internal/cli/interactive_test.go index c15e35a0..dc844eec 100644 --- a/internal/cli/interactive_test.go +++ b/internal/cli/interactive_test.go @@ -240,13 +240,13 @@ func TestRunInteractive_ExplicitTaskSkipsSniff(t *testing.T) { } // TestPickTask_FamilyScoped: the picker offers only the given family's -// tasks, wires the friendly display names + the locked glosses, and lists -// the not-yet-supported ones (greyed, with a reason) — never the other -// families' tasks. +// tasks, wires the friendly display names + the locked glosses — never the +// other families' tasks. After RFC-0002 phase 4 every text task is wired, so +// the text picker has no "Not yet in the CLI" section at all. func TestPickTask_FamilyScoped(t *testing.T) { - // Text family: fill-mask (gloss) is available; seq2seq - // (translation / summarization, gloss) + token_classification are - // pending; image/tabular tasks must not appear. + // Text family: all tasks are available now — fill-mask (gloss), + // classification, the two structured-pair tasks, and the two seq tasks; + // image/tabular tasks must not appear. f := &fakePrompter{answers: map[string]string{"Which task?": "Text classification"}} var buf bytes.Buffer p := ui.New(&buf, ui.WithColor(false)) @@ -260,17 +260,22 @@ func TestPickTask_FamilyScoped(t *testing.T) { out := buf.String() for _, want := range []string{ "Tasks for text data", - "fill-mask", // MLM gloss (available) - "Text classification", // label - "Not yet in the CLI:", // pending header - "translation / summarization", // seq2seq gloss (pending) - "token_classification", // pending id - "schema-recognized", // an UnsupportedNote fragment + "Available now:", + "fill-mask", // MLM gloss (available) + "Text classification", // label + "translation / summarization", // seq2seq gloss (now available) + "token_classification", // now available + "sentence_pair_classification", // now available + "Embeddings", // now available } { if !strings.Contains(out, want) { t.Errorf("picker output missing %q:\n%s", want, out) } } + // Every text task is wired now — no pending section. + if strings.Contains(out, "Not yet in the CLI:") { + t.Errorf("text picker should have no pending section now:\n%s", out) + } // Other families must not leak in. for _, unwanted := range []string{"Image classification", "Tabular classification", "Survival analysis"} { if strings.Contains(out, unwanted) { @@ -279,6 +284,30 @@ func TestPickTask_FamilyScoped(t *testing.T) { } } +// TestPickTask_ImagePending: semantic_segmentation is the sole remaining +// CLI-pending task, so the image picker still renders a greyed "Not yet in the +// CLI" section with its backend#816 reason. +func TestPickTask_ImagePending(t *testing.T) { + f := &fakePrompter{answers: map[string]string{"Which task?": "Image classification"}} + var buf bytes.Buffer + p := ui.New(&buf, ui.WithColor(false)) + if _, err := pickTask(p, f, push.FamilyImage); err != nil { + t.Fatalf("pickTask: %v", err) + } + out := buf.String() + for _, want := range []string{ + "Available now:", + "Image classification", + "Not yet in the CLI:", + "semantic_segmentation", + "backend#816", // the UnsupportedNote reason + } { + if !strings.Contains(out, want) { + t.Errorf("image picker missing %q:\n%s", want, out) + } + } +} + // TestPickTask_TabularGloss: the tabular picker shows the survival-analysis // gloss for time_to_event_prediction and can select it back to its id. func TestPickTask_TabularGloss(t *testing.T) { diff --git a/internal/push/category.go b/internal/push/category.go index 9edec1f8..a01d5dd3 100644 --- a/internal/push/category.go +++ b/internal/push/category.go @@ -1,6 +1,9 @@ package push -import "strings" +import ( + "fmt" + "strings" +) // CategorySpec is the single source of truth for one task category's // CLI-relevant rules. It mirrors data-ingestors' @@ -36,12 +39,26 @@ type CategorySpec struct { // never ships to the central backend by default. RegressionClass bool // SelfSupervised marks text categories that train without an explicit - // label column — the target is derived from the text itself (MLM masks - // tokens; CLM predicts the next token), so the interactive flow skips - // the "which column is the label?" question. A registry fact rather - // than a hardcoded id list so a new self-supervised task can't be added - // without deciding this (SelfSupervisedText reads it). + // label column — no `label` travels in labels.csv. For MLM/CLM the target + // is derived from the text itself (mask a token; predict the next one); for + // seq2seq and embeddings it comes from the record's own paired fields + // (source→target, anchor/positive/negative). Either way there's no label + // column, so the interactive flow skips the "which column is the label?" + // question. A registry fact rather than a hardcoded id list so a new + // self-supervised task can't be added without deciding this + // (SelfSupervisedText reads it). Mirrors the ingestor registry's + // is_self_supervised (data-ingestors modalities/registry.py). SelfSupervised bool + // IsClassification marks categories the ingestor treats as classification + // (registry ModalitySpec.is_classification) — the ones whose validator + // chain gets a LabelDiversityValidator, so the dataset needs >= 2 distinct + // labels. Mirrors the ingestor exactly: the image family + + // text_classification + sentence_pair_classification + tabular_classification + // are true; token_classification is NOT (its labels are BIO tag sequences, + // checked by BIOLabelValidator, not class labels), nor are the regression / + // self-supervised tasks. The label-diversity preflight reads it so it can't + // drift from the ingestor's wiring. + IsClassification bool // CLISupported reports whether `dataset push` implements the category // today. semantic_segmentation is known (the schema defines it) but // not yet pushable. @@ -76,17 +93,17 @@ const ( // nor carry an extra the ingestor won't accept (the instance_segmentation // half-ingest class — data-ingestors #240/#99, #1005). var categoryRegistry = []CategorySpec{ - {ID: "image_classification", Family: FamilyImage, Label: "Image classification", CLISupported: true, + {ID: "image_classification", Family: FamilyImage, Label: "Image classification", CLISupported: true, IsClassification: true, Blurb: "sort images into classes"}, - {ID: "object_detection", Family: FamilyImage, Label: "Object detection", CLISupported: true, + {ID: "object_detection", Family: FamilyImage, Label: "Object detection", CLISupported: true, IsClassification: true, Blurb: "draw boxes around objects in an image"}, - {ID: "keypoint_detection", Family: FamilyImage, Label: "Keypoint detection", CLISupported: true, + {ID: "keypoint_detection", Family: FamilyImage, Label: "Keypoint detection", CLISupported: true, IsClassification: true, Blurb: "locate landmark points on an image (e.g. pose)"}, - {ID: "text_classification", Family: FamilyText, Label: "Text classification", CLISupported: true, + {ID: "text_classification", Family: FamilyText, Label: "Text classification", CLISupported: true, IsClassification: true, Blurb: "sort text snippets into classes"}, {ID: "masked_language_modeling", Family: FamilyText, Label: "Masked language modeling", Gloss: "fill-mask", CLISupported: true, SelfSupervised: true, Blurb: "predict masked-out words — no labels needed"}, - {ID: "tabular_classification", Family: FamilyTabular, Label: "Tabular classification", CLISupported: true, + {ID: "tabular_classification", Family: FamilyTabular, Label: "Tabular classification", CLISupported: true, IsClassification: true, Blurb: "predict a class from table columns"}, {ID: "tabular_regression", Family: FamilyTabular, Label: "Tabular regression", RegressionClass: true, CLISupported: true, Blurb: "predict a number from table columns"}, @@ -94,24 +111,23 @@ var categoryRegistry = []CategorySpec{ Blurb: "predict future values from past ones"}, {ID: "time_to_event_prediction", Family: FamilyTabular, Label: "Time-to-event prediction", Gloss: "Survival analysis", RegressionClass: true, CLISupported: true, Blurb: "predict how long until an event happens"}, - {ID: "semantic_segmentation", Family: FamilyImage, Label: "Semantic segmentation", CLISupported: false, + {ID: "causal_language_modeling", Family: FamilyText, Label: "Causal language modeling", CLISupported: true, SelfSupervised: true, + Blurb: "predict the next word in a sequence"}, + {ID: "seq2seq", Family: FamilyText, Label: "Sequence-to-sequence", Gloss: "translation / summarization", CLISupported: true, SelfSupervised: true, + Blurb: "map an input sequence to an output one"}, + {ID: "token_classification", Family: FamilyText, Label: "Token classification", CLISupported: true, + Blurb: "label each word in a sequence"}, + {ID: "sentence_pair_classification", Family: FamilyText, Label: "Sentence-pair classification", CLISupported: true, IsClassification: true, + Blurb: "label how two texts relate"}, + {ID: "embeddings", Family: FamilyText, Label: "Embeddings", CLISupported: true, SelfSupervised: true, + Blurb: "learn vector representations from text pairs"}, + // semantic_segmentation stays CLI-pending: di#136 (mask sidecar) shipped, + // but the ingestor doesn't yet populate the mask_id link column the + // contract requires, and the training-side sign-off is tracked in + // backend#816. Wire it once those land (RFC-0002 phase 4 follow-up). + {ID: "semantic_segmentation", Family: FamilyImage, Label: "Semantic segmentation", CLISupported: false, IsClassification: true, Blurb: "label every pixel in an image", - UnsupportedNote: "blocked on the ingestor's mask-sidecar support (data-ingestors#136)"}, - {ID: "causal_language_modeling", Family: FamilyText, Label: "Causal language modeling", CLISupported: false, SelfSupervised: true, - Blurb: "predict the next word in a sequence", - UnsupportedNote: "schema-recognized (data-ingestors#805); `tracebloc ingest` discover/build for its raw-.txt / prompt\\tcompletion `texts` layout is pending"}, - {ID: "seq2seq", Family: FamilyText, Label: "Sequence-to-sequence", Gloss: "translation / summarization", CLISupported: false, - Blurb: "map an input sequence to an output one", - UnsupportedNote: "schema-recognized; `tracebloc ingest` discover/build for its raw-.txt / source\\ttarget `texts` layout is pending"}, - {ID: "token_classification", Family: FamilyText, Label: "Token classification", CLISupported: false, - Blurb: "label each word in a sequence", - UnsupportedNote: "schema-recognized; the CLI doesn't stage its per-token-label `texts` layout yet"}, - {ID: "sentence_pair_classification", Family: FamilyText, Label: "Sentence-pair classification", CLISupported: false, - Blurb: "label how two texts relate", - UnsupportedNote: "schema-recognized; `tracebloc ingest` discover/build for its raw-.txt / text_a\\ttext_b `texts` layout is pending"}, - {ID: "embeddings", Family: FamilyText, Label: "Embeddings", CLISupported: false, - Blurb: "learn vector representations from text pairs", - UnsupportedNote: "schema-recognized; `tracebloc ingest` discover/build for its raw-.txt / anchor\\tpositive[\\tnegative] `texts` layout is pending"}, + UnsupportedNote: "schema-recognized; awaiting the ingestor's mask_id link column + training sign-off (backend#816)"}, } // categoryByID indexes the registry for O(1) lookup, built once. @@ -206,12 +222,13 @@ func FamilyNouns() []string { } // SelfSupervisedText reports whether a text category trains without an -// explicit label column — the target is derived from the text itself, so -// the CLI skips the "which column is the label?" question. MLM masks -// tokens; CLM predicts the next token; neither reads a labels column. The -// answer is the registry's SelfSupervised flag, so a new self-supervised -// task is handled the moment it's added to the registry — not when someone -// remembers to edit this function. +// explicit label column, so the CLI skips the "which column is the label?" +// question. MLM/CLM derive the target from the text itself (mask a token; +// predict the next one); seq2seq and embeddings derive it from the record's +// own paired fields (source→target, anchor/positive/negative) — none reads a +// labels column. The answer is the registry's SelfSupervised flag, so a new +// self-supervised task is handled the moment it's added to the registry — not +// when someone remembers to edit this function. func SelfSupervisedText(category string) bool { c, ok := categoryByID[category] return ok && c.SelfSupervised @@ -249,6 +266,14 @@ func IsText(category string) bool { // therefore needs label.policy (object label form). func IsRegressionClass(category string) bool { return categoryByID[category].RegressionClass } +// IsClassification reports whether the ingestor treats category as a +// classification task (registry ModalitySpec.is_classification) — i.e. its +// validator chain includes LabelDiversityValidator. The label-diversity +// preflight gates on this so the CLI mirrors the ingestor's wiring rather than +// hardcoding a category id (which is exactly how the text-family preflight +// drifted when it only knew text_classification). +func IsClassification(category string) bool { return categoryByID[category].IsClassification } + // SupportedCategoryIDs returns the ids `dataset push` supports, in display // order. Used to build the --task help, the interactive picker, and // the accept-gate's "Supported:" lists from one place. @@ -275,13 +300,27 @@ func AllCategoryIDs() []string { // and gate error messages. func SupportedCategoriesList() string { return strings.Join(SupportedCategoryIDs(), ", ") } -// TextSidecarDir returns the sidecar directory name a text category -// expects: "sequences" for masked_language_modeling, "texts" for -// text_classification. (Used both as the local subdir to stage and the -// spec field to emit.) +// TextSidecarDir returns the sidecar directory name a text category expects +// ("sequences" for masked_language_modeling, "texts" for every other text +// task). Used both as the local subdir to stage and the spec field to emit. +// +// The value is READ from the vendored layout contract's primary_subdir — the +// ingestor owns this fact (data-ingestors registry ModalitySpec.file_subdir), +// so the CLI mirrors it rather than keeping a Go fork of the same rule +// (RFC-0002 Principle 6). +// +// A text category with no primary_subdir in the contract is a vendoring/drift +// bug, not a runtime condition: every text task pins one and +// TestTextSidecarDirMirrorsContract enforces the registry and contract agree. +// Silently falling back to "texts" would stage files into a directory the +// ingestor never reads, so fail loud instead — the only way here is a broken +// vendored contract, which scripts/sync-schema.sh --check catches in CI. func TextSidecarDir(category string) string { - if category == "masked_language_modeling" { - return "sequences" + if layout, ok := LayoutFor(category); ok && layout.PrimarySubdir != nil { + return *layout.PrimarySubdir } - return "texts" + panic(fmt.Sprintf( + "text category %q has no primary_subdir in the vendored layout contract — "+ + "the Go registry has drifted from layout.v1.json; re-run scripts/sync-schema.sh", + category)) } diff --git a/internal/push/category_registry_test.go b/internal/push/category_registry_test.go index 3548a381..7e7c0f43 100644 --- a/internal/push/category_registry_test.go +++ b/internal/push/category_registry_test.go @@ -37,17 +37,21 @@ func TestRegistryKnownCategories(t *testing.T) { func TestSupportedCategories(t *testing.T) { got := SupportedCategoryIDs() - if len(got) != 9 { - t.Fatalf("SupportedCategoryIDs() len = %d, want 9: %v", len(got), got) + // RFC-0002 phase 4 wired the 5 text tasks (token/sentence-pair + // classification, causal LM, seq2seq, embeddings), so 14 of the 15 + // categories are pushable; only semantic_segmentation remains pending. + if len(got) != 14 { + t.Fatalf("SupportedCategoryIDs() len = %d, want 14: %v", len(got), got) } for _, id := range got { if !IsCLISupported(id) { t.Errorf("SupportedCategoryIDs returned %q but IsCLISupported is false", id) } } - // semantic_segmentation + the self-supervised text categories (CLM, seq2seq) - // + token_classification are known but not yet pushable, and must explain why. - for _, id := range []string{"semantic_segmentation", "causal_language_modeling", "seq2seq", "token_classification", "sentence_pair_classification", "embeddings"} { + // semantic_segmentation is the sole known-but-not-yet-pushable category + // (awaiting the ingestor's mask_id link column + training sign-off, + // backend#816); it must stay gated out and explain why. + for _, id := range []string{"semantic_segmentation"} { if !IsKnown(id) { t.Errorf("%s should be known", id) } @@ -58,6 +62,16 @@ func TestSupportedCategories(t *testing.T) { t.Errorf("%s should carry an UnsupportedNote", id) } } + // The 5 newly-wired text tasks must now be pushable AND carry no stale + // pending note (the picker only greys out categories with a note). + for _, id := range []string{"token_classification", "sentence_pair_classification", "causal_language_modeling", "seq2seq", "embeddings"} { + if !IsCLISupported(id) { + t.Errorf("%s should be CLI-supported after phase 4", id) + } + if spec, _ := Lookup(id); spec.UnsupportedNote != "" { + t.Errorf("%s is supported but still carries an UnsupportedNote: %q", id, spec.UnsupportedNote) + } + } } func TestPredicatesDeriveFromRegistry(t *testing.T) { diff --git a/internal/push/layout_contract.go b/internal/push/layout_contract.go new file mode 100644 index 00000000..ca28af8a --- /dev/null +++ b/internal/push/layout_contract.go @@ -0,0 +1,191 @@ +package push + +import ( + "encoding/json" + "fmt" + "slices" + "strings" + + "github.com/tracebloc/cli/internal/schema" +) + +// The per-task local dataset-layout contract, mirrored from data-ingestors' +// tracebloc_ingestor/schema/layout.v1.json (data-ingestors#347/#353), vendored +// into internal/schema/ and drift-checked by scripts/sync-schema.sh. +// +// The ingestor is the source of truth for what a task's local dataset looks +// like on disk. The CLI reads this contract so its discovery + staging is a +// VERIFIED MIRROR of the ingestor's rules rather than a Go fork of them +// (RFC-0002 Principle 6). Two things drive real behaviour here: +// +// - RecordFormat — the structure inside each .txt for the structured text +// tasks. For the ENFORCED formats (sentence_pair_classification, +// embeddings) the CLI rejects a malformed file before staging, exactly as +// the ingestor's TabSeparatedRecordValidator would in-cluster. +// - The manifest/family/subdir facts — pinned against the Go category +// registry by layout_contract_test.go, so category.go can't silently drift +// from the ingestor's truth. + +// LayoutContract is the top-level shape of layout.v1.json. +type LayoutContract struct { + Version string `json:"version"` + Tasks map[string]TaskLayout `json:"tasks"` +} + +// TaskLayout is one task's on-disk layout. +type TaskLayout struct { + Family string `json:"family"` // image | text | tabular + Manifest ManifestLayout `json:"manifest"` + PrimarySubdir *string `json:"primary_subdir"` // images | texts | sequences | null + Sidecars []SidecarSpec `json:"sidecars"` + RecordFormat *RecordFormat `json:"record_format"` // structured-text tasks only +} + +// ManifestLayout describes the task's manifest CSV. +type ManifestLayout struct { + Kind string `json:"kind"` // labels_csv | data_csv + RequiresFilenameColumn bool `json:"requires_filename_column"` + HasLabelColumn bool `json:"has_label_column"` +} + +// SidecarSpec is an extra per-row directory a file-bearing task needs beyond +// its primary subdir (object_detection's annotations/, semseg's masks/). +type SidecarSpec struct { + Subdir string `json:"subdir"` + Glob string `json:"glob"` + Required bool `json:"required"` + LinkColumn *string `json:"link_column"` // manifest column linking a row to its sidecar; null = paired by filename stem +} + +// RecordFormat is the structure inside each .txt for the structured text +// tasks. Fields are the ordered field names separated by Separator; MinFields +// is the fewest that must be present (embeddings accepts an optional trailing +// negative, so Fields=(anchor,positive,negative) with MinFields=2). Enforced +// is true only when a structural validator rejects a malformed file in-cluster +// (sentence_pair, embeddings); false marks a documented convention the +// ingestor does NOT reject (seq2seq, causal LM accept raw free text), so a +// mirror must not reject it either. +type RecordFormat struct { + Separator string `json:"separator"` + Fields []string `json:"fields"` + MinFields int `json:"min_fields"` + Enforced bool `json:"enforced"` +} + +// layoutContract is the parsed embedded contract. Parsed once at package init; +// a parse failure means the vendored JSON is broken (a build/vendoring bug CI +// catches via sync-schema.sh --check), so we fail loudly rather than limp on. +var layoutContract = mustLoadLayoutContract() + +func mustLoadLayoutContract() *LayoutContract { + var c LayoutContract + if err := json.Unmarshal(schema.LayoutV1Bytes, &c); err != nil { + panic(fmt.Sprintf("parsing embedded layout.v1.json: %v", err)) + } + return &c +} + +// LayoutFor returns the layout contract for a task category and whether it is +// present in the contract. +func LayoutFor(category string) (TaskLayout, bool) { + t, ok := layoutContract.Tasks[category] + return t, ok +} + +// RecordFormatFor returns the record format for a text category and whether it +// declares one. Tasks without a structured .txt shape (text_classification, +// token_classification, MLM) return false. +func RecordFormatFor(category string) (RecordFormat, bool) { + t, ok := layoutContract.Tasks[category] + if !ok || t.RecordFormat == nil { + return RecordFormat{}, false + } + return *t.RecordFormat, true +} + +// AllowedFieldCounts is the set of field counts a valid record may have — +// MinFields..len(Fields), inclusive. Mirrors the ingestor's +// TabSeparatedRecordValidator.ALLOWED_FIELD_COUNTS (sentence_pair: {2}; +// embeddings: {2, 3}). +func (rf RecordFormat) AllowedFieldCounts() []int { + var out []int + for n := rf.MinFields; n <= len(rf.Fields); n++ { + out = append(out, n) + } + return out +} + +// sepLabel renders the separator for an error message — a literal tab becomes +// "" so the message is readable in a terminal. +func (rf RecordFormat) sepLabel() string { + if rf.Separator == "\t" { + return "" + } + return rf.Separator +} + +// shape renders the canonical record shape, e.g. "text_atext_b" or +// "anchorpositivenegative". +func (rf RecordFormat) shape() string { + return strings.Join(rf.Fields, rf.sepLabel()) +} + +// countPhrase renders the allowed field-count clause: "exactly 2" for a single +// allowed count, "2 or 3" for a range. +func (rf RecordFormat) countPhrase() string { + counts := rf.AllowedFieldCounts() + if len(counts) == 1 { + return fmt.Sprintf("exactly %d", counts[0]) + } + parts := make([]string, len(counts)) + for i, n := range counts { + parts[i] = fmt.Sprintf("%d", n) + } + return strings.Join(parts, " or ") +} + +// ValidateTextRecord mirrors the ingestor's TabSeparatedRecordValidator +// per-file structural check for the ENFORCED record-format text tasks +// (sentence_pair_classification, embeddings): the file must be a single line +// of MinFields..len(Fields) non-empty separator-delimited fields. +// +// For unenforced formats (causal_language_modeling, seq2seq) it returns nil — +// the ingestor accepts raw free text for those, so a mirror must not reject it +// (RFC-0002 Principle 6). An empty / whitespace-only file also returns nil: the +// ingestor leaves that to its TextContentValidator (which warns), so rejecting +// it here would diverge. +func ValidateTextRecord(rf RecordFormat, content string) error { + if !rf.Enforced { + return nil + } + // Drop only surrounding blank lines / trailing newline — NOT interior + // separators, so a leading/trailing empty field is still caught below. + record := strings.Trim(content, "\r\n") + if strings.TrimSpace(record) == "" { + return nil + } + // One record per file: a surviving interior line break means several + // records were crammed in (or a field holds a newline) — ambiguous. + if strings.ContainsAny(record, "\r\n") { + return fmt.Errorf( + "expected a single %s record but the file spans multiple lines. "+ + "Put one %s per .txt", rf.shape(), rf.shape()) + } + parts := strings.Split(record, rf.Separator) + if !slices.Contains(rf.AllowedFieldCounts(), len(parts)) { + // Separator comes from the contract (sepLabel renders a tab as ""), + // so a future non-tab task isn't misdescribed as "tab-separated". + return fmt.Errorf( + "expected %s %s-separated fields (%s), found %d. "+ + "Separate each field with exactly one %s", + rf.countPhrase(), rf.sepLabel(), rf.shape(), len(parts), rf.sepLabel()) + } + for i, p := range parts { + if strings.TrimSpace(p) == "" { + return fmt.Errorf( + "field %d is empty — every field (%s) must be non-empty", + i+1, strings.Join(rf.Fields[:len(parts)], ", ")) + } + } + return nil +} diff --git a/internal/push/layout_contract_test.go b/internal/push/layout_contract_test.go new file mode 100644 index 00000000..0a5809ba --- /dev/null +++ b/internal/push/layout_contract_test.go @@ -0,0 +1,173 @@ +package push + +import "testing" + +// These tests pin the Go category registry as a VERIFIED MIRROR of the +// vendored layout contract (internal/schema/layout.v1.json), so category.go +// cannot silently drift from the ingestor's on-disk truth (RFC-0002 +// Principle 6). The contract itself is drift-checked against data-ingestors by +// scripts/sync-schema.sh, so this ties the registry transitively to upstream. + +// familyFromContract maps the contract's family string to the CLI Family enum. +func familyFromContract(t *testing.T, s string) Family { + t.Helper() + switch s { + case "image": + return FamilyImage + case "text": + return FamilyText + case "tabular": + return FamilyTabular + default: + t.Fatalf("unknown contract family %q", s) + return 0 + } +} + +// TestRegistryMirrorsLayoutContract: for every category the Go registry knows, +// the layout contract must agree on family and on the label-column fact, and +// vice versa (every contract task must be a known category). This is the +// single guard that keeps the hand-maintained registry honest against the +// machine-readable contract. +func TestRegistryMirrorsLayoutContract(t *testing.T) { + // Registry ⊆ contract, with agreeing facts. + for _, c := range categoryRegistry { + layout, ok := LayoutFor(c.ID) + if !ok { + t.Errorf("category %q is in the registry but missing from layout.v1.json", c.ID) + continue + } + if want := familyFromContract(t, layout.Family); c.Family != want { + t.Errorf("%s: registry Family = %d, contract says %q (%d)", c.ID, c.Family, layout.Family, want) + } + // SelfSupervised (no label question) is the inverse of the contract's + // has_label_column, for EVERY category — image/tabular carry a label + // and are not self-supervised; the self-supervised text tasks carry + // none. This is the fact spec.buildText + the interactive label prompt + // both key off, so pinning it here catches a mis-set flag. + if c.SelfSupervised == layout.Manifest.HasLabelColumn { + t.Errorf("%s: registry SelfSupervised = %v but contract has_label_column = %v (must be opposite)", + c.ID, c.SelfSupervised, layout.Manifest.HasLabelColumn) + } + } + + // Contract ⊆ registry: no task in the contract is unknown to the CLI. + for id := range layoutContract.Tasks { + if !IsKnown(id) { + t.Errorf("layout.v1.json task %q is not a known CLI category", id) + } + } +} + +// TestTextSidecarDirMirrorsContract: TextSidecarDir must return exactly the +// contract's primary_subdir for every text task — the directory the CLI stages +// into has to be the one the ingestor reads (texts/ for every text task but +// MLM, which uses sequences/). +func TestTextSidecarDirMirrorsContract(t *testing.T) { + for _, c := range categoryRegistry { + if c.Family != FamilyText { + continue + } + layout, ok := LayoutFor(c.ID) + if !ok || layout.PrimarySubdir == nil { + t.Fatalf("%s: text task missing a primary_subdir in the contract", c.ID) + } + if got := TextSidecarDir(c.ID); got != *layout.PrimarySubdir { + t.Errorf("%s: TextSidecarDir = %q, contract primary_subdir = %q", c.ID, got, *layout.PrimarySubdir) + } + } +} + +// TestRecordFormatFor_Contract pins the record-format facts the CLI enforces +// against the contract: the two enforced structured tasks and the two +// unenforced conventions, plus the derived allowed field counts. +func TestRecordFormatFor_Contract(t *testing.T) { + cases := []struct { + category string + wantPresent bool + wantEnforced bool + wantCounts []int + }{ + {"sentence_pair_classification", true, true, []int{2}}, + {"embeddings", true, true, []int{2, 3}}, + {"seq2seq", true, false, []int{1, 2}}, + {"causal_language_modeling", true, false, []int{1, 2}}, + {"token_classification", false, false, nil}, // no structured record + {"text_classification", false, false, nil}, + {"image_classification", false, false, nil}, + } + for _, tc := range cases { + t.Run(tc.category, func(t *testing.T) { + rf, ok := RecordFormatFor(tc.category) + if ok != tc.wantPresent { + t.Fatalf("RecordFormatFor(%s) present = %v, want %v", tc.category, ok, tc.wantPresent) + } + if !ok { + return + } + if rf.Enforced != tc.wantEnforced { + t.Errorf("%s: Enforced = %v, want %v", tc.category, rf.Enforced, tc.wantEnforced) + } + got := rf.AllowedFieldCounts() + if len(got) != len(tc.wantCounts) { + t.Fatalf("%s: AllowedFieldCounts = %v, want %v", tc.category, got, tc.wantCounts) + } + for i := range got { + if got[i] != tc.wantCounts[i] { + t.Errorf("%s: AllowedFieldCounts = %v, want %v", tc.category, got, tc.wantCounts) + } + } + }) + } +} + +// TestValidateTextRecord mirrors the ingestor's TabSeparatedRecordValidator +// cases: enforced tasks reject the wrong field count / empty fields / multiple +// lines, accept a well-formed record, and never reject on an unenforced format. +func TestValidateTextRecord(t *testing.T) { + sp, _ := RecordFormatFor("sentence_pair_classification") + emb, _ := RecordFormatFor("embeddings") + s2s, _ := RecordFormatFor("seq2seq") + + // Well-formed records pass. + if err := ValidateTextRecord(sp, "left side\tright side"); err != nil { + t.Errorf("valid sentence pair rejected: %v", err) + } + if err := ValidateTextRecord(emb, "anchor\tpositive"); err != nil { + t.Errorf("valid embeddings pair rejected: %v", err) + } + if err := ValidateTextRecord(emb, "anchor\tpositive\tnegative"); err != nil { + t.Errorf("valid embeddings triplet rejected: %v", err) + } + // A trailing newline is stripped, not an error. + if err := ValidateTextRecord(sp, "left\tright\n"); err != nil { + t.Errorf("trailing newline should be tolerated: %v", err) + } + + // Malformed records fail. + if err := ValidateTextRecord(sp, "no tab here"); err == nil { + t.Error("sentence pair with 1 field should fail") + } + if err := ValidateTextRecord(sp, "a\tb\tc"); err == nil { + t.Error("sentence pair with 3 fields should fail") + } + if err := ValidateTextRecord(emb, "only one"); err == nil { + t.Error("embeddings with 1 field should fail") + } + if err := ValidateTextRecord(sp, "left\t"); err == nil { + t.Error("empty trailing field should fail") + } + if err := ValidateTextRecord(sp, "l1\tr1\nl2\tr2"); err == nil { + t.Error("multi-line record should fail") + } + + // Unenforced format never rejects, even malformed-looking content. + if err := ValidateTextRecord(s2s, "just raw text no tab"); err != nil { + t.Errorf("unenforced seq2seq should accept raw text: %v", err) + } + // An empty / whitespace-only file is the TextContentValidator's job, not + // this structural check — it must pass here (no double reporting). + if err := ValidateTextRecord(sp, " \n"); err != nil { + t.Errorf("empty file should be tolerated by the structural check: %v", err) + } +} diff --git a/internal/push/preflight.go b/internal/push/preflight.go index 4b884d04..bbdb617f 100644 --- a/internal/push/preflight.go +++ b/internal/push/preflight.go @@ -31,6 +31,47 @@ import ( // utf8BOM is the byte-order mark Excel's "CSV UTF-8" export prepends. var utf8BOM = []byte{0xEF, 0xBB, 0xBF} +// openCSVReader opens path for row-walking with any UTF-8 BOM stripped, the one +// idiom the pandas-backed checks share (cli#71): pandas strips the BOM even +// under encoding="utf-8", so a BOM'd file must read as if it had none or the +// CLI would reject what the cluster accepts. FieldsPerRecord is -1 so a ragged +// row is a per-row concern, not an abort. The caller closes the returned +// Closer. A caller that must read the rows pandas tolerates (an unescaped +// quote) sets r.LazyQuotes = true before its first Read. +func openCSVReader(path string) (*csv.Reader, io.Closer, error) { + f, err := os.Open(path) + if err != nil { + return nil, nil, err + } + br := bufio.NewReader(f) + if head, _ := br.Peek(3); bytes.Equal(head, utf8BOM) { + _, _ = br.Discard(3) + } + r := csv.NewReader(br) + r.FieldsPerRecord = -1 + return r, f, nil +} + +// matchColumnIndex returns the index of the header column matching want — an +// exact match first, then case-insensitively with surrounding whitespace +// stripped (the ingestor's resolve_column / _match_column rule). Returns -1 +// when absent. Shared by the label-column and filename-column checks so the +// ingestor's single resolve rule has a single Go copy. +func matchColumnIndex(header []string, want string) int { + for i, c := range header { + if c == want { + return i + } + } + wl := strings.ToLower(strings.TrimSpace(want)) + for i, c := range header { + if strings.ToLower(strings.TrimSpace(c)) == wl { + return i + } + } + return -1 +} + // HasBOM reports whether the file starts with a UTF-8 BOM. func HasBOM(path string) (bool, error) { f, err := os.Open(path) @@ -56,16 +97,11 @@ func HasBOM(path string) (bool, error) { // accepts. The one in-cluster path that does NOT strip it is the tabular // schema probe — see CheckTabularBOM. func ReadCSVHeader(path string) ([]string, error) { - f, err := os.Open(path) + r, closer, err := openCSVReader(path) if err != nil { return nil, err } - defer func() { _ = f.Close() }() - br := bufio.NewReader(f) - if head, _ := br.Peek(3); bytes.Equal(head, utf8BOM) { - _, _ = br.Discard(3) - } - r := csv.NewReader(br) + defer func() { _ = closer.Close() }() header, err := r.Read() if err != nil { if errors.Is(err, io.EOF) { @@ -110,16 +146,8 @@ func CheckTabularBOM(path string) error { // must stay this loose or the CLI would reject datasets the cluster // accepts). func CheckLabelColumn(header []string, labelColumn, csvName string) error { - for _, c := range header { - if c == labelColumn { - return nil - } - } - want := strings.ToLower(strings.TrimSpace(labelColumn)) - for _, c := range header { - if strings.ToLower(strings.TrimSpace(c)) == want { - return nil - } + if matchColumnIndex(header, labelColumn) >= 0 { + return nil } return fmt.Errorf( "label column %q isn't in %s's header (columns: %s). Pass --label-column with one of "+ @@ -158,17 +186,11 @@ func CheckDuplicateHeaders(header []string, csvName string) error { // category): a header-only CSV has zero ingestable records and is // rejected in-cluster before any table is created. func CheckHasDataRows(path string) error { - f, err := os.Open(path) + r, closer, err := openCSVReader(path) if err != nil { return fmt.Errorf("reading %s: %w", filepath.Base(path), err) } - defer func() { _ = f.Close() }() - br := bufio.NewReader(f) - if head, _ := br.Peek(3); bytes.Equal(head, utf8BOM) { - _, _ = br.Discard(3) - } - r := csv.NewReader(br) - r.FieldsPerRecord = -1 // row-shape problems are someone else's diagnostic + defer func() { _ = closer.Close() }() if _, err := r.Read(); err != nil { if errors.Is(err, io.EOF) { return fmt.Errorf("%s is empty — add a header and at least one data row, then re-run", filepath.Base(path)) @@ -275,17 +297,11 @@ func ValidateImages(images []string, expectedW, expectedH, minW, minH int) error // filenameColumn is the CSV's first column (the ingestor reads filenames // positionally from the id column of labels.csv). func CrossCheckLabels(csvPath string, images []string, extension string) (missing []string, orphans []string, err error) { - f, err := os.Open(csvPath) + r, closer, err := openCSVReader(csvPath) if err != nil { return nil, nil, fmt.Errorf("reading %s: %w", filepath.Base(csvPath), err) } - defer func() { _ = f.Close() }() - br := bufio.NewReader(f) - if head, _ := br.Peek(3); bytes.Equal(head, utf8BOM) { - _, _ = br.Discard(3) - } - r := csv.NewReader(br) - r.FieldsPerRecord = -1 + defer func() { _ = closer.Close() }() present := make(map[string]bool, len(images)) for _, img := range images { @@ -453,17 +469,11 @@ func ReadLabelValues(csvPath, labelColumn string, dropNASentinels, collapseNumer // 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) + r, closer, err := openCSVReader(csvPath) if err != nil { return LabelReadValues{} // Found=false: unreadable file is another check's diagnostic } - defer func() { _ = f.Close() }() - br := bufio.NewReader(f) - if head, _ := br.Peek(3); bytes.Equal(head, utf8BOM) { - _, _ = br.Discard(3) - } - r := csv.NewReader(br) - r.FieldsPerRecord = -1 + defer func() { _ = closer.Close() }() header, err := r.Read() if err != nil { return LabelReadValues{} @@ -797,14 +807,28 @@ func PreflightDataset(spec SpecArgs, layout *LocalLayout) (notes []string, probl if err := CheckDuplicateHeaders(header, "labels.csv"); err != nil { return nil, dataProblem(err) } - if spec.Category == "text_classification" { + // Every SUPERVISED text task carries a label column the ingestor + // requires present — text_classification & sentence_pair_classification + // via LabelColumnValidator, token_classification via BIOLabelValidator — + // so preview the header for all of them. Gating on !SelfSupervisedText + // mirrors buildText's label emission, so a typo'd --label-column fails + // locally (exit 2) instead of after the full upload. The self-supervised + // tasks (MLM, CLM, seq2seq, embeddings) carry no label. + if !SelfSupervisedText(spec.Category) { if err := CheckLabelColumn(header, spec.LabelColumn, "labels.csv"); err != nil { return nil, &PreflightProblem{Err: err, BadFlag: true} } - // 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) + // LabelDiversityValidator is wired only for the is_classification + // text tasks (text_classification, sentence_pair_classification), NOT + // token_classification — its BIO tag sequences aren't class labels, + // so the ingestor runs BIOLabelValidator instead and never checks + // label diversity. Mirror that exactly (Principle 6): gate on + // IsClassification. Text labels are read untyped (like image), so no + // NA drop and no numeric collapse. + if IsClassification(spec.Category) { + 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 38ff928f..b3bc11c7 100644 --- a/internal/push/preflight_test.go +++ b/internal/push/preflight_test.go @@ -301,3 +301,50 @@ func TestCheckLabelDiversitySchemaTypeDispatch(t *testing.T) { t.Error("FLOAT label should collapse '1'/'1.0' and be rejected") } } + +// TestPreflightDataset_TextLabelParity locks the text-family label preflight to +// the ingestor's wiring across ALL supervised text tasks (not just +// text_classification, which is how the gate drifted when #182 wired the rest): +// - a missing label column fails locally (BadFlag) for every supervised text +// task — LabelColumnValidator for text/sentence_pair, BIOLabelValidator for +// token_classification — instead of uploading and failing in-cluster; +// - a single-class label is rejected for the is_classification text tasks +// (LabelDiversityValidator), but token_classification (BIO tag sequences, +// is_classification=false) must NOT trigger diversity — the ingestor never +// runs it, so neither may the CLI. +func TestPreflightDataset_TextLabelParity(t *testing.T) { + writeLayout := func(t *testing.T, content string) *LocalLayout { + t.Helper() + dir := t.TempDir() + p := filepath.Join(dir, "labels.csv") + if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + return &LocalLayout{Root: dir, LabelsCSV: p} + } + + for _, cat := range []string{"text_classification", "sentence_pair_classification", "token_classification"} { + layout := writeLayout(t, "filename,text\na.txt,foo\nb.txt,bar\n") + _, problem := PreflightDataset(SpecArgs{Category: cat, LabelColumn: "label"}, layout) + if problem == nil { + t.Errorf("%s: a missing label column should fail preflight before upload", cat) + continue + } + if !problem.BadFlag { + t.Errorf("%s: a missing label column should be a BadFlag (settings) problem, got %v", cat, problem.Err) + } + } + + single := "filename,label\na.txt,x\nb.txt,x\n" + for _, cat := range []string{"text_classification", "sentence_pair_classification"} { + layout := writeLayout(t, single) + if _, problem := PreflightDataset(SpecArgs{Category: cat, LabelColumn: "label"}, layout); problem == nil { + t.Errorf("%s: a single-class label should be rejected (classification needs >=2 classes)", cat) + } + } + layout := writeLayout(t, single) + if _, problem := PreflightDataset(SpecArgs{Category: "token_classification", LabelColumn: "label"}, layout); problem != nil { + t.Errorf("token_classification: a single-value label must NOT trigger the diversity check "+ + "(BIO labels aren't class labels; the ingestor runs BIOLabelValidator, not LabelDiversity): %v", problem.Err) + } +} diff --git a/internal/push/preview_test.go b/internal/push/preview_test.go index dba65827..ae6bcd1c 100644 --- a/internal/push/preview_test.go +++ b/internal/push/preview_test.go @@ -293,12 +293,16 @@ func TestCategoriesByFamily(t *testing.T) { } func TestSelfSupervisedText(t *testing.T) { - for _, id := range []string{"masked_language_modeling", "causal_language_modeling"} { + // The self-supervised text tasks have no label column (MLM/CLM predict from + // the text itself; seq2seq/embeddings derive their target from the record + // structure), so the interactive flow skips the label question for them. + for _, id := range []string{"masked_language_modeling", "causal_language_modeling", "seq2seq", "embeddings"} { if !SelfSupervisedText(id) { t.Errorf("%s should be self-supervised", id) } } - for _, id := range []string{"text_classification", "tabular_regression", "image_classification"} { + // The SUPERVISED text tasks carry a label column and must still be asked. + for _, id := range []string{"text_classification", "token_classification", "sentence_pair_classification", "tabular_regression", "image_classification"} { if SelfSupervisedText(id) { t.Errorf("%s should not be self-supervised", id) } diff --git a/internal/push/spec.go b/internal/push/spec.go index 73dafcd6..6d7f3f6d 100644 --- a/internal/push/spec.go +++ b/internal/push/spec.go @@ -268,15 +268,22 @@ func (a SpecArgs) Build() map[string]any { } // buildText fills in the text-family fields: the text-file sidecar -// directory (texts/ for text_classification, sequences/ for -// masked_language_modeling) and the label. masked_language_modeling -// has NO label (the schema doesn't require one for it). +// directory (texts/ for every text task except masked_language_modeling, +// which uses sequences/) and, for the SUPERVISED text tasks, the label. +// +// The self-supervised text tasks (masked/causal language modeling, seq2seq, +// embeddings) carry NO label column — their target is derived from the text +// itself, and the schema does not require `label` for them. The supervised +// ones (text_classification, token_classification, sentence_pair_classification) +// do, so the label is emitted for exactly those — driven by the registry's +// SelfSupervised flag (mirrored against the layout contract's has_label_column) +// rather than a hardcoded id, so a new task can't be wired without deciding it. func (a SpecArgs) buildText(spec map[string]any, prefix string) { dir := TextSidecarDir(a.Category) // Trailing slash matches the directory-glob convention the // ingestor uses for sidecar dirs. spec[dir] = path.Join(prefix, dir) + "/" - if a.Category == "text_classification" { + if !SelfSupervisedText(a.Category) { spec["label"] = a.LabelColumn } } diff --git a/internal/push/text.go b/internal/push/text.go index 4d9048b4..2c7a9b34 100644 --- a/internal/push/text.go +++ b/internal/push/text.go @@ -3,6 +3,7 @@ package push import ( "errors" "fmt" + "io" "os" "path/filepath" "strings" @@ -84,6 +85,26 @@ func DiscoverText(category, rootDir string) (*LocalLayout, error) { layout.Sidecars[dirName] = files layout.TotalBytes += sidecarBytes + // Structured-text tasks whose .txt shape the ingestor ENFORCES + // (sentence_pair_classification: text_atext_b; embeddings: + // anchorpositive[negative]) get the same per-file structural + // check here, so a malformed layout fails locally with a clear message + // instead of after the full stage. The rule comes from the vendored + // layout contract, not hardcoded — the CLI mirrors the ingestor's + // TabSeparatedRecordValidator (RFC-0002 Principle 6). Unenforced formats + // (seq2seq, causal LM) accept raw text and are not checked. + // + // The check is scoped to the files the manifest actually references, NOT + // every .txt in the dir: the ingestor's validator walks labels.csv rows and + // only opens the file each row names, so an unreferenced stray .txt (a + // README, a scratch draft) must not fail discovery — the ingestor would + // accept the dataset. + if rf, ok := RecordFormatFor(category); ok && rf.Enforced { + if err := validateTextRecords(labelsPath, dirName, files, rf); err != nil { + return nil, err + } + } + if layout.TotalBytes > MaxTotalBytes { return nil, fmt.Errorf( "dataset is %s, exceeds v0.1 cap of %s. For larger datasets, the "+ @@ -93,6 +114,122 @@ func DiscoverText(category, rootDir string) (*LocalLayout, error) { return layout, nil } +// validateTextRecords runs the enforced record-format check over the +// manifest-referenced text files in dirName, mirroring the ingestor's per-file +// TabSeparatedRecordValidator: it walks labels.csv rows (not the directory) and +// checks the file each row names. Only files a row references are checked — the +// ingestor never opens a file no row references, so validating a stray +// unreferenced .txt would reject a layout the ingestor accepts (RFC-0002 +// Principle 6). The first malformed file fails discovery with a message naming +// the offending file (relative to the dataset root, e.g. "texts/bad.txt"), so +// the fix is obvious without reaching the cluster. +// +// Each manifest value is matched against the files actually discovered on disk, +// not a reconstructed ".txt": the ingestor appends the CONFIGURED +// extension (.txt or .text — file_options.extension), so a row "a" must match +// texts/a.text when that is what is on disk. Matching is case-insensitive on +// both the basename and the stem so "A.txt" in the manifest still resolves to +// a.txt on disk (fail-open otherwise). +func validateTextRecords(csvPath, dirName string, files []string, rf RecordFormat) error { + referenced, err := manifestReferencedTextNames(csvPath) + if err != nil { + return err + } + + // Index the discovered files by lowercased basename and by lowercased stem, + // so a manifest value resolves to the file the ingestor would open whether + // or not it carries the extension, and regardless of case. + byBase := make(map[string]string, len(files)) + byStem := make(map[string]string, len(files)) + for _, f := range files { + base := filepath.Base(f) + byBase[strings.ToLower(base)] = f + stem := strings.TrimSuffix(base, filepath.Ext(base)) + byStem[strings.ToLower(stem)] = f + } + + for name := range referenced { + // Mirror file_transfer._has_extension: a value that already ends in a + // known extension names the file directly; otherwise the ingestor + // appends its configured extension, so match on the stem. + var path string + if hasKnownExtension(name) { + path = byBase[strings.ToLower(name)] + } else { + path = byStem[strings.ToLower(name)] + } + if path == "" { + continue // manifest names a file not on disk — a missing-file check's job, not ours + } + content, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("reading %s: %w", filepath.Join(dirName, filepath.Base(path)), err) + } + if verr := ValidateTextRecord(rf, string(content)); verr != nil { + return fmt.Errorf("%s: %w", filepath.Join(dirName, filepath.Base(path)), verr) + } + } + return nil +} + +// manifestReferencedTextNames returns the set of raw text-file values the +// manifest (labels.csv) references — the "filename" column, trimmed, dropping +// blanks — mirroring the ingestor's TabSeparatedRecordValidator manifest walk. +// The values are returned unresolved (no extension appended); the caller +// matches them against the discovered files, since only there does it know +// which extension is actually on disk. +// +// The filename column is REQUIRED: the ingestor's validator rejects a manifest +// without it ("Missing required column: filename"), so — for the enforced tasks +// that call this — surface that locally rather than validating nothing (which +// would fail-open post-upload). An empty CSV yields an empty set (its own +// emptiness is another check's diagnostic). The CSV is read with LazyQuotes so +// a row pandas tolerates (an unescaped quote) is read here too, not silently +// dropped — its filename would otherwise never be validated. +func manifestReferencedTextNames(csvPath string) (map[string]struct{}, error) { + r, closer, err := openCSVReader(csvPath) + if err != nil { + return nil, fmt.Errorf("reading labels.csv: %w", err) + } + defer func() { _ = closer.Close() }() + r.LazyQuotes = true // read the rows pandas would, don't drop them + + header, err := r.Read() + if err != nil { + if errors.Is(err, io.EOF) { + return map[string]struct{}{}, nil // empty CSV — another check's diagnostic + } + return nil, fmt.Errorf("reading labels.csv: %w", err) + } + col := matchColumnIndex(header, "filename") + if col < 0 { + return nil, fmt.Errorf( + "labels.csv has no \"filename\" column (columns: %s) — the ingestor matches each "+ + "row to its text file by that column and rejects a manifest without it. "+ + "Add a filename column and re-run.", + strings.Join(header, ", ")) + } + referenced := map[string]struct{}{} + for { + rec, err := r.Read() + if errors.Is(err, io.EOF) { + break + } + if err != nil { + continue // a row even LazyQuotes can't read is another check's diagnostic + } + if col >= len(rec) { + continue + } + name := strings.TrimSpace(rec[col]) + if name == "" { + continue + } + referenced[name] = struct{}{} + } + return referenced, nil +} + // discoverSidecarFiles walks / (non-recursive) for files // whose extension is in exts, rejecting symlinks and enforcing the // single-file cap. Returns the absolute paths + their total size. A diff --git a/internal/push/text_test.go b/internal/push/text_test.go index 9556ce62..4c228f57 100644 --- a/internal/push/text_test.go +++ b/internal/push/text_test.go @@ -107,6 +107,241 @@ func TestDiscoverText_MissingSidecarDir(t *testing.T) { } } +// mkStructuredTextDir builds a text dataset whose texts/ files carry the given +// contents (filename → body). labels.csv lists each file; hasLabel adds a +// label column (supervised tasks) so the CSV mirrors what the ingestor reads. +func mkStructuredTextDir(t *testing.T, hasLabel bool, files map[string]string) string { + t.Helper() + dir := t.TempDir() + header := "filename\n" + if hasLabel { + header = "filename,label\n" + } + csv := header + sub := filepath.Join(dir, "texts") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatal(err) + } + for name, body := range files { + if hasLabel { + csv += name + ",x\n" + } else { + csv += name + "\n" + } + if err := os.WriteFile(filepath.Join(sub, name), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + writeFile(t, dir, "labels.csv", csv) + return dir +} + +// TestDiscoverText_AllPhase4Tasks: each of the 5 newly-wired text tasks +// discovers its texts/ layout and stages labels.csv + the text files, with a +// valid fixture per the layout contract (enforced formats get a well-formed +// record; unenforced ones get raw text). +func TestDiscoverText_AllPhase4Tasks(t *testing.T) { + cases := []struct { + category string + hasLabel bool + body string + }{ + {"token_classification", true, "the\tDET\ncat\tNOUN"}, // per-token; no enforced record_format + {"sentence_pair_classification", true, "a rose is red\tit is a flower"}, // text_atext_b (enforced) + {"causal_language_modeling", false, "just some raw pretraining text"}, // unenforced + {"seq2seq", false, "bonjour le monde\thello world"}, // sourcetarget (unenforced) + {"embeddings", false, "query\tpositive doc\thard negative"}, // anchorpositivenegative (enforced) + } + for _, tc := range cases { + t.Run(tc.category, func(t *testing.T) { + dir := mkStructuredTextDir(t, tc.hasLabel, map[string]string{"a.txt": tc.body}) + layout, err := DiscoverText(tc.category, dir) + if err != nil { + t.Fatalf("DiscoverText(%s): %v", tc.category, err) + } + if len(layout.Sidecars["texts"]) != 1 { + t.Errorf("texts files = %d, want 1", len(layout.Sidecars["texts"])) + } + if got := layout.FileCount(); got != 2 { // labels.csv + 1 text + t.Errorf("FileCount = %d, want 2", got) + } + }) + } +} + +// TestDiscoverText_EnforcedRecordFormat_Reject: the ENFORCED formats +// (sentence_pair_classification, embeddings) reject a malformed .txt at +// discovery, with a message that names the file and the expected shape — +// mirroring the ingestor's TabSeparatedRecordValidator. The UNENFORCED formats +// (seq2seq, causal LM) accept the same raw content, so a mirror must not reject. +func TestDiscoverText_EnforcedRecordFormat_Reject(t *testing.T) { + // A single field, no tab: malformed for the enforced tasks. + rawNoTab := map[string]string{"bad.txt": "one blob of prose with no tab"} + + for _, category := range []string{"sentence_pair_classification", "embeddings"} { + t.Run(category+"_rejects", func(t *testing.T) { + hasLabel := !SelfSupervisedText(category) + dir := mkStructuredTextDir(t, hasLabel, rawNoTab) + _, err := DiscoverText(category, dir) + if err == nil { + t.Fatalf("DiscoverText(%s) accepted a malformed record", category) + } + // The separator label comes from the contract (sepLabel renders a + // tab as ""), not a hardcoded "tab-separated" literal. + for _, want := range []string{"bad.txt", "-separated fields"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error missing %q: %v", want, err) + } + } + }) + } + + // The unenforced tasks accept the very same raw file. + for _, category := range []string{"seq2seq", "causal_language_modeling"} { + t.Run(category+"_accepts_raw", func(t *testing.T) { + dir := mkStructuredTextDir(t, false, rawNoTab) + if _, err := DiscoverText(category, dir); err != nil { + t.Errorf("DiscoverText(%s) rejected raw text it should accept: %v", category, err) + } + }) + } +} + +// TestDiscoverText_EnforcedRecordFormat_IgnoresUnreferenced: the enforced +// record-format check runs only over the files labels.csv references, mirroring +// the ingestor's manifest walk (TabSeparatedRecordValidator iterates the CSV +// rows, not the directory). A stray unreferenced .txt in texts/ — a README, a +// scratch draft with no tab — must NOT fail discovery: the ingestor never opens +// a file no row names, so rejecting it would block a layout the cluster accepts +// (RFC-0002 Principle 6). +func TestDiscoverText_EnforcedRecordFormat_IgnoresUnreferenced(t *testing.T) { + for _, category := range []string{"sentence_pair_classification", "embeddings"} { + t.Run(category, func(t *testing.T) { + hasLabel := !SelfSupervisedText(category) + // a.txt is a well-formed 2-field record AND is referenced by + // labels.csv; notes.txt is prose with no tab and is NOT referenced. + dir := mkStructuredTextDir(t, hasLabel, map[string]string{"a.txt": "left side\tright side"}) + stray := filepath.Join(dir, "texts", "notes.txt") + if err := os.WriteFile(stray, []byte("just some prose with no tab"), 0o644); err != nil { + t.Fatal(err) + } + if _, err := DiscoverText(category, dir); err != nil { + t.Errorf("DiscoverText(%s) rejected a dataset with an unreferenced stray .txt "+ + "the ingestor would accept: %v", category, err) + } + }) + } +} + +// TestDiscoverText_SentencePair_WrongFieldCount: sentence_pair requires exactly +// 2 fields — a 3-field record is rejected, whereas embeddings accepts 2 or 3. +func TestDiscoverText_SentencePair_WrongFieldCount(t *testing.T) { + three := map[string]string{"a.txt": "one\ttwo\tthree"} + + dir := mkStructuredTextDir(t, true, three) + if _, err := DiscoverText("sentence_pair_classification", dir); err == nil { + t.Error("sentence_pair_classification should reject a 3-field record") + } + + dir2 := mkStructuredTextDir(t, false, three) + if _, err := DiscoverText("embeddings", dir2); err != nil { + t.Errorf("embeddings should accept a 3-field triplet: %v", err) + } +} + +// TestDiscoverText_ConfiguredExtension: the ingestor appends the CONFIGURED +// extension (.txt OR .text — file_options.extension), so the enforced check +// must match a manifest value against the file actually on disk, not a +// reconstructed ".txt". A row "a" resolves to texts/a.text when that is +// what exists — a malformed a.text is rejected, a well-formed one passes. +func TestDiscoverText_ConfiguredExtension(t *testing.T) { + mk := func(t *testing.T, body string) string { + t.Helper() + dir := t.TempDir() + writeFile(t, dir, "labels.csv", "filename\na\n") // embeddings: no label; value carries no extension + sub := filepath.Join(dir, "texts") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(sub, "a.text"), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + return dir + } + + if _, err := DiscoverText("embeddings", mk(t, "one blob no tab")); err == nil { + t.Fatal("malformed a.text should be rejected via the configured .text extension") + } else if !strings.Contains(err.Error(), "a.text") { + t.Errorf("error should name the on-disk file a.text: %v", err) + } + + if _, err := DiscoverText("embeddings", mk(t, "anchor\tpositive")); err != nil { + t.Errorf("well-formed a.text rejected: %v", err) + } +} + +// TestDiscoverText_MissingFilenameColumn: the ingestor's validator rejects a +// manifest with no filename column ("Missing required column: filename"); the +// enforced check surfaces that locally instead of validating nothing (which +// would fail-open post-upload). +func TestDiscoverText_MissingFilenameColumn(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "labels.csv", "id\nrow1\n") // no filename column + sub := filepath.Join(dir, "texts") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(sub, "a.txt"), []byte("anchor\tpositive"), 0o644); err != nil { + t.Fatal(err) + } + _, err := DiscoverText("embeddings", dir) + if err == nil { + t.Fatal("a manifest with no filename column should be rejected locally") + } + if !strings.Contains(err.Error(), "filename") { + t.Errorf("error should name the missing filename column: %v", err) + } +} + +// TestDiscoverText_CaseMismatchedBasename: a manifest value "A.txt" must resolve +// to the on-disk a.txt case-insensitively — otherwise the file goes unchecked +// (fail-open). The malformed a.txt is therefore still rejected. +func TestDiscoverText_CaseMismatchedBasename(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "labels.csv", "filename,label\nA.txt,x\n") + sub := filepath.Join(dir, "texts") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(sub, "a.txt"), []byte("one blob no tab"), 0o644); err != nil { + t.Fatal(err) + } + if _, err := DiscoverText("sentence_pair_classification", dir); err == nil { + t.Fatal("case-mismatched manifest value A.txt should still match a.txt and reject the malformed file") + } +} + +// TestDiscoverText_TolerantManifestRow: a row Go's strict csv.Reader rejects but +// pandas tolerates (an unescaped quote) must still be read — LazyQuotes — so its +// filename is validated, not silently dropped. Here the tolerated row names a +// malformed a.txt, which must be rejected; without LazyQuotes the row (and its +// file) would be skipped and discovery would fail-open. +func TestDiscoverText_TolerantManifestRow(t *testing.T) { + dir := t.TempDir() + // The unescaped " in the label field trips Go's strict reader; pandas reads it. + writeFile(t, dir, "labels.csv", "filename,label\na.txt,he said \"hi\"\n") + sub := filepath.Join(dir, "texts") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(sub, "a.txt"), []byte("one blob no tab"), 0o644); err != nil { + t.Fatal(err) + } + if _, err := DiscoverText("sentence_pair_classification", dir); err == nil { + t.Fatal("a pandas-tolerable row must be read (LazyQuotes) so its file is validated") + } +} + // TestBuild_Text_PassesSchema: the text Build branch emits the right // sidecar field (texts vs sequences), a label for text_classification // but NOT for masked_language_modeling, never an images field, and a @@ -149,4 +384,26 @@ func TestBuild_Text_PassesSchema(t *testing.T) { check("masked_language_modeling", SpecArgs{ Table: "t_mlm", Category: "masked_language_modeling", Intent: "train", }, "sequences", false) + + // Phase 4: supervised text tasks emit a label under texts/; the + // self-supervised ones emit none. Each must still be schema-valid. + check("token_classification", SpecArgs{ + Table: "t_tok", Category: "token_classification", Intent: "train", LabelColumn: "label", + }, "texts", true) + + check("sentence_pair_classification", SpecArgs{ + Table: "t_sp", Category: "sentence_pair_classification", Intent: "train", LabelColumn: "label", + }, "texts", true) + + check("causal_language_modeling", SpecArgs{ + Table: "t_clm", Category: "causal_language_modeling", Intent: "train", + }, "texts", false) + + check("seq2seq", SpecArgs{ + Table: "t_s2s", Category: "seq2seq", Intent: "train", + }, "texts", false) + + check("embeddings", SpecArgs{ + Table: "t_emb", Category: "embeddings", Intent: "train", + }, "texts", false) } diff --git a/internal/schema/embed.go b/internal/schema/embed.go index 9fa8f871..37229fad 100644 --- a/internal/schema/embed.go +++ b/internal/schema/embed.go @@ -28,3 +28,18 @@ import _ "embed" // //go:embed ingest.v1.json var V1Bytes []byte + +// LayoutV1Bytes is the raw JSON of the per-task dataset-layout contract +// (layout.v1.json), vendored from tracebloc/data-ingestors at build time via +// scripts/sync-schema.sh (data-ingestors#347/#353). +// +// The ingestor is the source of truth for what a task's local dataset looks +// like on disk — the manifest CSV, whether it carries a label column, the +// primary file subdir, extra sidecar dirs, and the in-.txt record format for +// the structured text tasks. Embedding it here lets the CLI's discovery + +// staging be a VERIFIED MIRROR of that contract (RFC-0002 Principle 6) rather +// than re-implementing the layout rules in Go — drift is caught at build time +// by the same sync-schema.sh --check the ingest schema uses. +// +//go:embed layout.v1.json +var LayoutV1Bytes []byte diff --git a/internal/schema/layout.v1.json b/internal/schema/layout.v1.json new file mode 100644 index 00000000..2ecdc199 --- /dev/null +++ b/internal/schema/layout.v1.json @@ -0,0 +1,217 @@ +{ + "tasks": { + "causal_language_modeling": { + "family": "text", + "manifest": { + "has_label_column": false, + "kind": "labels_csv", + "requires_filename_column": true + }, + "primary_subdir": "texts", + "record_format": { + "enforced": false, + "fields": [ + "prompt", + "completion" + ], + "min_fields": 1, + "separator": "\t" + }, + "sidecars": [] + }, + "embeddings": { + "family": "text", + "manifest": { + "has_label_column": false, + "kind": "labels_csv", + "requires_filename_column": true + }, + "primary_subdir": "texts", + "record_format": { + "enforced": true, + "fields": [ + "anchor", + "positive", + "negative" + ], + "min_fields": 2, + "separator": "\t" + }, + "sidecars": [] + }, + "image_classification": { + "family": "image", + "manifest": { + "has_label_column": true, + "kind": "labels_csv", + "requires_filename_column": true + }, + "primary_subdir": "images", + "record_format": null, + "sidecars": [] + }, + "keypoint_detection": { + "family": "image", + "manifest": { + "has_label_column": true, + "kind": "labels_csv", + "requires_filename_column": true + }, + "primary_subdir": "images", + "record_format": null, + "sidecars": [] + }, + "masked_language_modeling": { + "family": "text", + "manifest": { + "has_label_column": false, + "kind": "labels_csv", + "requires_filename_column": true + }, + "primary_subdir": "sequences", + "record_format": null, + "sidecars": [] + }, + "object_detection": { + "family": "image", + "manifest": { + "has_label_column": true, + "kind": "labels_csv", + "requires_filename_column": true + }, + "primary_subdir": "images", + "record_format": null, + "sidecars": [ + { + "glob": "*.xml", + "link_column": null, + "required": true, + "subdir": "annotations" + } + ] + }, + "semantic_segmentation": { + "family": "image", + "manifest": { + "has_label_column": true, + "kind": "labels_csv", + "requires_filename_column": true + }, + "primary_subdir": "images", + "record_format": null, + "sidecars": [ + { + "glob": "*.png", + "link_column": "mask_id", + "required": true, + "subdir": "masks" + } + ] + }, + "sentence_pair_classification": { + "family": "text", + "manifest": { + "has_label_column": true, + "kind": "labels_csv", + "requires_filename_column": true + }, + "primary_subdir": "texts", + "record_format": { + "enforced": true, + "fields": [ + "text_a", + "text_b" + ], + "min_fields": 2, + "separator": "\t" + }, + "sidecars": [] + }, + "seq2seq": { + "family": "text", + "manifest": { + "has_label_column": false, + "kind": "labels_csv", + "requires_filename_column": true + }, + "primary_subdir": "texts", + "record_format": { + "enforced": false, + "fields": [ + "source", + "target" + ], + "min_fields": 1, + "separator": "\t" + }, + "sidecars": [] + }, + "tabular_classification": { + "family": "tabular", + "manifest": { + "has_label_column": true, + "kind": "data_csv", + "requires_filename_column": false + }, + "primary_subdir": null, + "record_format": null, + "sidecars": [] + }, + "tabular_regression": { + "family": "tabular", + "manifest": { + "has_label_column": true, + "kind": "data_csv", + "requires_filename_column": false + }, + "primary_subdir": null, + "record_format": null, + "sidecars": [] + }, + "text_classification": { + "family": "text", + "manifest": { + "has_label_column": true, + "kind": "labels_csv", + "requires_filename_column": true + }, + "primary_subdir": "texts", + "record_format": null, + "sidecars": [] + }, + "time_series_forecasting": { + "family": "tabular", + "manifest": { + "has_label_column": true, + "kind": "data_csv", + "requires_filename_column": false + }, + "primary_subdir": null, + "record_format": null, + "sidecars": [] + }, + "time_to_event_prediction": { + "family": "tabular", + "manifest": { + "has_label_column": true, + "kind": "data_csv", + "requires_filename_column": false + }, + "primary_subdir": null, + "record_format": null, + "sidecars": [] + }, + "token_classification": { + "family": "text", + "manifest": { + "has_label_column": true, + "kind": "labels_csv", + "requires_filename_column": true + }, + "primary_subdir": "texts", + "record_format": null, + "sidecars": [] + } + }, + "version": "1" +} diff --git a/scripts/sync-schema.sh b/scripts/sync-schema.sh index d4b42a4c..1fa12e5c 100755 --- a/scripts/sync-schema.sh +++ b/scripts/sync-schema.sh @@ -1,11 +1,17 @@ #!/usr/bin/env bash -# Sync ingest.v1.json from tracebloc/data-ingestors into the CLI's -# embedded copy at internal/schema/ingest.v1.json. +# Sync the CLI's embedded contract files from tracebloc/data-ingestors: # -# The CLI validates locally using this schema. Drift between the +# - ingest.v1.json — the ingest-config JSON Schema the CLI validates against +# - layout.v1.json — the per-task dataset-layout contract (data-ingestors +# #347/#353) the CLI mirrors for discovery + staging +# +# both under internal/schema/. +# +# The CLI validates locally using these files. Drift between the # CLI's copy and data-ingestors' canonical source is a real # correctness hazard — a customer's YAML could pass `tracebloc ingest -# validate` locally but be rejected by jobs-manager (or vice versa). +# validate` locally but be rejected by jobs-manager (or vice versa), or the +# CLI could stage a layout the ingestor rejects. # # Run this script when bumping the schema version. CI invokes it in # check-mode (`--check`) to fail builds that have drifted without @@ -16,11 +22,12 @@ # scripts/sync-schema.sh --check # verify in-tree copy matches upstream; exit non-zero on drift # # Env knobs: -# SCHEMA_SOURCE_URL override the upstream URL (default: built from the -# pinned ref below) +# SCHEMA_SOURCE_URL override the upstream URL for ingest.v1.json (default: +# built from the pinned ref below) # DATA_INGESTORS_REF override the data-ingestors ref (default: the pinned # SHA in scripts/.data-ingestors-ref, else master) -# SCHEMA_OUT override the in-tree destination (default: internal/schema/ingest.v1.json) +# SCHEMA_OUT override the in-tree destination for ingest.v1.json +# (default: internal/schema/ingest.v1.json) # # The ref is PINNED (scripts/.data-ingestors-ref), not a floating branch, so an # unrelated upstream commit doesn't red every open CLI PR — adopting upstream @@ -54,61 +61,118 @@ if ! printf '%s' "$DATA_INGESTORS_REF" | grep -qE '^[A-Za-z0-9][A-Za-z0-9._/-]*$ exit 2 fi -readonly DEFAULT_URL="https://raw.githubusercontent.com/tracebloc/data-ingestors/${DATA_INGESTORS_REF}/tracebloc_ingestor/schema/ingest.v1.json" +readonly UPSTREAM_BASE="https://raw.githubusercontent.com/tracebloc/data-ingestors/${DATA_INGESTORS_REF}/tracebloc_ingestor/schema" + +readonly DEFAULT_URL="${UPSTREAM_BASE}/ingest.v1.json" readonly DEFAULT_OUT="internal/schema/ingest.v1.json" +# ingest.v1.json keeps its historical env overrides; layout.v1.json is derived +# from the pinned ref. Each entry is "URL|OUT". SCHEMA_SOURCE_URL="${SCHEMA_SOURCE_URL:-$DEFAULT_URL}" SCHEMA_OUT="${SCHEMA_OUT:-$DEFAULT_OUT}" +FILES=( + "${SCHEMA_SOURCE_URL}|${SCHEMA_OUT}" + "${UPSTREAM_BASE}/layout.v1.json|internal/schema/layout.v1.json" +) CHECK_MODE=false if [[ "${1:-}" == "--check" ]]; then CHECK_MODE=true fi -# Stage the fetched schema in a temp file so a half-failed curl doesn't -# leave a truncated file in the repo. -tmp=$(mktemp) -trap 'rm -f "$tmp"' EXIT +# Track every temp file we stage so a single top-level trap can clean them all +# up — on normal exit AND on a signal-driven one (SIGINT/SIGTERM during the curl +# fetch or json.tool validation). The pre-refactor code used a top-level EXIT +# trap; a per-function RETURN trap alone would leak the temp file when the run +# is killed mid-fetch, so keep cleanup at the top level. +_tmpfiles=() +cleanup_tmpfiles() { + # Guard the expansion: under `set -u`, "${arr[@]}" on an empty array is an + # unbound-variable error in bash < 4.4 (macOS ships 3.2). + [[ ${#_tmpfiles[@]} -eq 0 ]] && return 0 + local f + for f in "${_tmpfiles[@]}"; do + rm -f "$f" + done +} +trap cleanup_tmpfiles EXIT INT TERM -echo "==> fetching $SCHEMA_SOURCE_URL" -curl -fsSL "$SCHEMA_SOURCE_URL" -o "$tmp" +# sync_one fetches one upstream file and either checks it against the in-tree +# copy (--check) or writes it. Returns non-zero on drift / missing file in +# check mode. Each fetch is staged in its own temp file so a half-failed curl +# never leaves a truncated file in the repo. +sync_one() { + local url="$1" out="$2" + local tmp + tmp=$(mktemp) + _tmpfiles+=("$tmp") -# Make sure what came back is valid JSON before we trust it. -if ! python3 -m json.tool < "$tmp" > /dev/null 2>&1; then - echo "error: upstream response is not valid JSON" >&2 - echo "first 200 bytes of response:" >&2 - head -c 200 "$tmp" >&2 - exit 2 -fi + echo "==> fetching $url" + # sync_one is called as `if ! sync_one ...`, which suspends `set -e` for the + # whole body — so a failed curl (e.g. a 404) would otherwise fall through and + # be misdiagnosed as "not valid JSON" on the empty temp file. Check curl's + # exit explicitly and report the real fetch failure. --tlsv1.2 matches every + # other curl in the repo (scripts/install.sh). + curl -fsSL --tlsv1.2 "$url" -o "$tmp" + local curl_rc=$? + if [[ $curl_rc -ne 0 ]]; then + echo "error: failed to fetch $url (curl exited $curl_rc)" >&2 + return "$curl_rc" + fi -mkdir -p "$(dirname "$SCHEMA_OUT")" + # Make sure what came back is valid JSON before we trust it. + if ! python3 -m json.tool < "$tmp" > /dev/null 2>&1; then + echo "error: upstream response is not valid JSON ($url)" >&2 + echo "first 200 bytes of response:" >&2 + head -c 200 "$tmp" >&2 + return 2 + fi -if $CHECK_MODE; then - if [[ ! -f "$SCHEMA_OUT" ]]; then - echo "error: $SCHEMA_OUT does not exist" >&2 - echo "run \`scripts/sync-schema.sh\` (without --check) to seed it." >&2 - exit 1 + mkdir -p "$(dirname "$out")" + + if $CHECK_MODE; then + if [[ ! -f "$out" ]]; then + echo "error: $out does not exist" >&2 + echo "run \`scripts/sync-schema.sh\` (without --check) to seed it." >&2 + return 1 + fi + if ! diff -q "$tmp" "$out" >/dev/null; then + echo "error: $out has drifted from upstream." >&2 + echo "diff (upstream → in-tree):" >&2 + diff -u "$out" "$tmp" | head -40 >&2 || true + echo >&2 + echo "to fix, bump scripts/.data-ingestors-ref if needed, run \`scripts/sync-schema.sh\`, and commit the result." >&2 + return 1 + fi + echo "==> $out matches upstream — no drift" + return 0 fi - if ! diff -q "$tmp" "$SCHEMA_OUT" >/dev/null; then - echo "error: $SCHEMA_OUT has drifted from upstream." >&2 - echo "diff (upstream → in-tree):" >&2 - diff -u "$SCHEMA_OUT" "$tmp" | head -40 >&2 || true - echo >&2 - echo "to fix, run \`scripts/sync-schema.sh\` and commit the result." >&2 - exit 1 + + # Write mode. Only touch the destination if the content actually changed, so + # re-running the script on an already-current file produces no mtime churn. + if [[ -f "$out" ]] && diff -q "$tmp" "$out" >/dev/null; then + echo "==> $out already matches upstream — no change" + return 0 fi - echo "==> $SCHEMA_OUT matches upstream — no drift" - exit 0 -fi -# Write mode. Only touch the destination if the content actually -# changed, so re-running the script on an already-current schema -# produces no file-mtime churn. -if [[ -f "$SCHEMA_OUT" ]] && diff -q "$tmp" "$SCHEMA_OUT" >/dev/null; then - echo "==> $SCHEMA_OUT already matches upstream — no change" - exit 0 -fi + # Check the write explicitly: sync_one is called as `if ! sync_one ...`, which + # suspends `set -e` for the whole function body, so a failed cp (unwritable + # dir, full disk) would otherwise fall through to the "wrote" line and return + # 0 — a false success that lets a stale vendored file get committed. + if ! cp "$tmp" "$out"; then + echo "error: failed to write $out (check directory permissions / disk space)" >&2 + return 1 + fi + echo "==> wrote $out ($(wc -c < "$out" | tr -d ' ') bytes)" + return 0 +} -mv "$tmp" "$SCHEMA_OUT" -trap - EXIT # the temp file is now in place; don't try to rm it -echo "==> wrote $SCHEMA_OUT ($(wc -c < "$SCHEMA_OUT" | tr -d ' ') bytes)" +rc=0 +for entry in "${FILES[@]}"; do + url="${entry%%|*}" + out="${entry##*|}" + if ! sync_one "$url" "$out"; then + rc=1 + fi +done +exit "$rc"