Uh oh!
There was an error while loading. Please reload this page.
fix(data ingest): preflight & message polish — schema-type validation, friendly missing-label, help copy, clearer errors (#213 #214 #215 #76) - #233
Merged
Conversation
…chema types Four small ingest/preflight polish tickets, one coherent change. The data INGESTOR owns validation; the CLI only PREVIEWS it (Principle 6), so every new rule here mirrors an authoritative ingestor grammar rather than inventing a CLI-only one. - #215: derive the "Supports N tasks" count from the task registry (push.SupportedCategoryIDs) instead of a stale hardcoded "9", and fix the text example to show sequences/ for masked_language_modeling (its primary_subdir per layout.v1.json), both read from the vendored contract so they can't drift again. - #214: a tabular task with no --label-column now gets the friendly flag-naming message (listing the CSV's columns) via a targeted pre-check, instead of the opaque label-oneOf schema dump. Only the missing case is intercepted; other schema errors still surface. - #213: validate each --schema TYPE token locally against the ingestor's REAL accepted set (mirrors database.py::_get_sqlalchemy_type, di#349) so a bogus type (e.g. age:BANANA) is caught before the upload — on both the --schema flag path (ParseSchema) and `data validate`. Also fixes ParseSchema's comma-split so DECIMAL(p,s)/NUMERIC(p,s) — types the ingestor accepts — parse as one entry instead of being torn apart. - #76: (a) `data delete ""` gives a delete-appropriate positional-arg message, not the ingest-path "set --name"; (b) --number-of-keypoints distinguishes unset ("requires") from an explicit non-positive value ("must be a positive integer (got N)") via cmd.Flags().Changed; (c) suppress the redundant parent-level "label: got object, want string" oneOf type-noise when a specific label.policy error is present. Tests added/updated for each fix across internal/cli, internal/push, and internal/schema. go build/vet/test/gofmt all clean; coverage floors hold (internal/cli 74.4%). Closes#213#214#215#76 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 12, 2026
Closed
saqlainsyed007
approved these changes
Jul 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Four small, independent polish fixes to the
data ingestlocal gate + its error messages, bundled into one review. Each closes a backlog ticket from the 2026-07-11 triage; all four sit in the same files (data.go/push/schema) and share one review + CI cycle.#213 — validate
--schemaTYPE tokens locally. A bogus type (age:BANANA) used to pass every local check and only fail in-cluster.ParseSchemanow validates the base type against the ingestor's real accepted set —acceptedSQLBaseTypesis a byte-exact mirror of data-ingestorsdatabase.py::_get_sqlalchemy_type(di#349, the source of truth), withsqlBaseTypemirroring the ingestor's upper/strip/split-(extraction. Also closes the same false-green on thedata validateYAML path (reusing the exportedValidateSchemaTypeso the two paths can't diverge) and fixes a pre-existingParseSchemacomma-split bug that toreDECIMAL(p,s)/NUMERIC(p,s)— types the ingestor accepts.#214 — name the missing
--label-column. Omitting--label-columnon a tabular task used to dump a crypticlabel: got string, want object …oneOf schema error. A targeted pre-check now returns a friendly exit-2 message naming--label-columnand listing the inferred columns. Only the missing case is intercepted — a present-but-wrong label still flows toCheckLabelColumn, and all other schema errors still surface normally.#215 — derived help copy. The ingest help hardcoded "Supports 9 tasks" (stale) and put the MLM example under a
texts/folder while the wired layout needssequences/. Both are now derived — the count frompush.SupportedCategoryIDs()(renders 15) and the text-family sidecar from the vendored layout contract — so they can't drift again.#76 — clearer validation messages.
data delete ""pointed at--name, a flagdeletedoesn't have (it takes a positional arg). Now: "pass it as an argument:tracebloc data delete <dataset>".--number-of-keypoints 0(or negative) claimed you hadn't passed the flag. Now distinguishes unset ("requires …") from set-but-invalid ("must be a positive integer (got 0)"), viacmd.Flags().Changed.--label-policyleakedlabel: got object, want stringabove the reallabel.policy: value must be one of …. The generic oneOf type-noise is now suppressed when a more specific child error exists.Deliberately NOT included: #77
#77 ("task-irrelevant flags silently ignored") was in the triage bundle but is dropped here on purpose: PR #224 (merged to develop 2026-07-10) already hard-rejects misapplied task flags with exit 2, so the original "silent" bug is gone. Whether to keep that hard-reject or soften it to a warn (Principle 6: the CLI drops the flag from the synthesized spec, so the ingestor would accept the run) is a design decision parked for @LukasWodka — this PR leaves #224's behavior untouched.
Tests / gates
TestSuppressOneOfTypeNoisestill pins the filter, so coverage isn't lost.go build ./.../go vet ./...OK;gofmt -lclean;go test ./...green;coverage-floor.sh: internal/cli 74.4% ≥ 68%, internal/submit 76.1% ≥ 72%.Closes#213#214#215#76.
🤖 Generated with Claude Code
Note
Low Risk
CLI-only validation and messaging in preflight paths; behavior mirrors existing ingestor rules and is covered by new tests, with no auth or cluster workflow changes.
Overview
This PR bundles local preflight and error-message polish for
data ingest,data validate, anddata delete—no change to cluster staging or submit behavior.Schema types (#213) —
--schemaand YAMLschemablocks now reject unsupported SQL base types using the same vocabulary as the ingestor (ValidateSchemaType/acceptedSQLBaseTypes).data validateaddsschemaTypeViolationsso bogus types likeBANANAfail locally instead of after upload.ParseSchemaalso splits on commas only outside parentheses soDECIMAL(10,2)stays one entry.Tabular missing label (#214) — Omitting
--label-columnon tabular/time-series tasks returns a short message naming the flag and listing inferred CSV columns, instead of a rawlabeloneOf schema dump.Ingest help (#215) — Task count and text sidecar names in
data ingestlong help come fromSupportedCategoryIDs()andTextSidecarDir(e.g.sequences/for masked language modeling), not hardcoded copy.Misc (#76) —
data delete ""points at the positional argument; keypoint errors distinguish unset--number-of-keypointsfrom an explicit invalid value viaChangedFlags; schema validation suppresses parent-levelgot X, want YoneOf noise when a nested path (e.g.label.policy) already reports the real issue.Reviewed by Cursor Bugbot for commit ff72f4b. Bugbot is set up for automated code reviews on this repo. Configure here.