Skip to content

main - > Enhance data ingest with flexible input, schema confirmation, and task updates - #219

Merged
saadqbal merged 22 commits into
mainfrom
develop
Jul 10, 2026
Merged

main - > Enhance data ingest with flexible input, schema confirmation, and task updates#219
saadqbal merged 22 commits into
mainfrom
develop

Conversation

@saadqbal

@saadqbalsaadqbal commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Note

Medium Risk
Touches the full ingest preflight/staging/submit path, interactive flows, and machine offboard revoke semantics across many task types; behavior is heavily tested but regressions could affect data uploads or credential lifecycle.

Overview
Large data ingest refresh aligned with RFC-0002: user-facing --name / --task (hidden deprecated --table / --category), workspace wording, a 3-step happy path, spinners on long waits, and cluster release/PVC detail only under --verbose. Guided mode is data-first (intent → name → path → family sniff → family-scoped task list → label pick from CSV headers).

Coverage and correctness: CLI-supported tasks rise to 15 of 16 (additional text modalities, time_series_classification, etc.; semantic_segmentation still pending). Layout/rules are driven by vendored layout.v1.json (RecordFormat validation, grouping trait, TextSidecarDir from contract). Fail-fast guards add early path checks, --min-size, rejection of task-mismatched flags, default --intent=train, and no default --task. Tabular schema inference emits richer warnings and always ships inferred schema in the spec.

Smaller but important:login rejects unknown env strings via IsKnownEnv so typos do not silently target prod. delete continues local teardown when server revoke fails (except 401/426), with honest success/warn copy. Shared pathutil.ExpandHome supports ~user paths. README examples use the new flags and task count.

Reviewed by Cursor Bugbot for commit e4d5ce5. Bugbot is set up for automated code reviews on this repo. Configure here.

LukasWodkaand others added 16 commits July 9, 2026 18:12
feat(data ingest): plain-language copy + hide k8s ceremony + progress on every wait (#179)
…/--split (#180a) (#197)
* feat(data ingest): rename --category/--table/--intent to --task/--name/--split (#180a)
RFC-0002 §6 flag renames — the mechanical half of #180 (the data-first
flow inversion is #180b, stacked separately). Every old flag stays on as
a HIDDEN deprecated alias so existing scripts don't break; the wire/spec
field names (category/table/intent) are unchanged.
- --category → --task, and DROP the old image_classification default:
omitting the task now drives the interactive picker (TTY) or a clear
"which task? pass --task" error (non-interactive), never a silent
image assumption.
- --table → --name (the wire field stays "table").
- --intent → --split, default "train"; the prompt is reworded to
"Is this training or test data?".
- Reworded the interactive prompts, the Review labels, the pre-flight
"Ingest settings" labels, all help/error copy, and the README example
to the new names.
Behavior is otherwise identical — exit codes, guards, and the
destination-exists (exit 6) check are unchanged.
Tests: the canonical flags work; each old flag still resolves via its
hidden alias (canonical wins on conflict); omitting --task errors
clearly off a TTY; omitting --split defaults to train.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(data ingest): keep --intent canonical + mirror the ingestor's table-name rule
Reviewer feedback on #180a:
- Revert the --intent→--split rename. --intent stays the canonical
flag; --split is dropped entirely. The --category→--task and
--table→--name renames (with hidden deprecated aliases) are unchanged.
Omitting --intent still defaults to train (RFC-0002 §5); the wire/spec
field stays "intent".
- Tighten table-name validation to mirror the ingestor (the source of
truth): names must start with a letter or underscore
(^[A-Za-z_][A-Za-z0-9_]*$), matching data-ingestors'
validators/table_name_validator.py. The old pattern was looser and
let leading-digit / all-digit names ("123", "1data") through the
CLI only for the cluster to reject them post-upload.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…195)
masked_language_modeling required a tokenizer.json at the dataset root,
staged it as an ExtraFile, and displayed it in the local summary. This
was a false gate: the ingestor never read it, the ingest.v1 schema never
required it, and the training side deliberately stopped staging a dataset
tokenizer in #805 (it diverged the vocab and broke weight averaging). The
tokenizer is the collaborator's, supplied at model upload.
- DiscoverText: remove the MLM tokenizer branch. An MLM dataset with just
the text layout is now accepted; a stray tokenizer.json is ignored, not
an error (it is simply not walked).
- LocalLayout.ExtraFiles was populated solely by this branch, so remove it
(struct field, FileCount, the stream.go packaging loop, and the data.go
local-summary display) rather than leave dead, always-empty plumbing.
- Fix the now-false "the ingestor reads it" doc comments.
- Tests: replace TestDiscoverText_MLM_RequiresTokenizer with
TestDiscoverText_MLM_NoTokenizer (accepted without; stray one ignored).
RFC-0002 §12.11. CLI-only, low risk.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ker (#180b) (#198)
* feat(data ingest): data-first flow inversion + family-scoped task picker (#180b)
Invert the guided ingest flow to data-first (RFC-0002 §12.1) and rework
task selection so the user is never shown the flat 15-task wall.
- Prompt order is now intent → name → path → task → task-specific →
review. Name is asked with an example in the hint (not auto-filled).
- After the path, the family is sniffed from the layout and echoed
("Found a CSV table — this is tabular data."). The sniff is a HINT, not
a lock (§5.1): an ambiguous layout asks the family plainly, an explicit
--task skips the sniff entirely, and non-interactive runs never sniff.
SniffFamily mirrors the Discover* layout markers — it validates nothing,
so it can't fork or replace the real walk.
- Family-scoped task picker (§7): only that family's tasks are offered,
rendered "Display — one-liner · task_id", split into Available now vs a
greyed "Not yet in the CLI" (with the reason). The registry Label field
is finally wired up, plus the locked glosses (time_to_event_prediction →
"Survival analysis", masked_language_modeling → "fill-mask", seq2seq →
"translation / summarization") and a per-task blurb.
- Label question (§8) now SELECTs over the real CSV header row (exact
match kills the data-ingestors#340 case-mismatch silent-null-label
class), worded per task (class vs value-to-predict), skipped for
self-supervised text; falls back to free text if the header can't be
read. --number-of-keypoints / --time-column unchanged.
- Promptable overwrite (folded): a pre-existing destination table is a
y/N prompt in interactive mode (yes → replace, no → clean exit 0), still
a hard exit 6 non-interactively; a reused --idempotency-key falls
through to exit 6 to preserve the data-loss guard.
- Copy polish (folded from #179): data/ingest help says "workspace" and
"ingest", not "client" and "stage". Aliases unchanged.
Exit codes / classify / reclaim matrix and --output-json keys unchanged.
Closes#180
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(data ingest): keep Kubernetes out of the happy-path copy (#180b)
The ingest command's Long help described the normal successful flow in
Kubernetes terms ("submits an ingestion run to jobs-manager, and watches
the ingestor Job") - the exact ceremony RFC-0002 section 6 keeps off the
happy path. Reword it to plain language plus the on-prem reassurance. The
exit-code section and the --detach reconnect hint still name
jobs-manager/kubectl: those are failure/detach diagnostics, which section 6
explicitly permits.
Also fix the exit-6 duplicate-table error to say "in this workspace", not
"in this client" - matching the workspace wording used everywhere else in
the flow.
Copy-only; no behavior change. Tests + gofmt + vet green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(data ingest): make the family sniff mirror the discovery walk (#180b)
SniffFamily + previewLabelCSVPath were an over-permissive fork of the real
discovery walk. Align them to what Discover / DiscoverText / DiscoverTabular
actually accept so a confident sniff can never promise a layout the walk
refuses:
- Match the marker dirs (images/, texts/, sequences/) and labels.csv with
the literal, case-sensitive names the walk joins + Lstats. A mis-cased
"Images/" is no longer read as confident image. Only the .csv extension
match stays case-insensitive (mirrors DiscoverTabular's EqualFold).
- Require BOTH labels.csv AND the subdir before claiming confident image /
text (mirror Discover / DiscoverText); the echo now only names files that
were actually found.
- Require a directory for tabular: a bare .csv file is no longer confident
(DiscoverTabular rejects bare files; that's cli#181).
- previewLabelCSVPath now reuses DiscoverTabular's single-CSV rule via a
shared findSingleCSV helper, so a multi-CSV directory errors (caller falls
back to free text) instead of silently reading the alphabetical first.
Interactive flow:
- Path prompt copy is folder-oriented ("the folder holding it") — the walk
can't take a bare file yet, so don't promise one.
- Path prompt gets a validator that rejects an empty/whitespace answer, so a
bare Enter can't sniff (and ingest) the current working directory.
- Drop the dead famPrompted store in resolveFamily (pickTask sets prompted
unconditionally right after).
Also removes the unused FamilyOf accessor.
Tests pin: mis-cased dir not confident, missing labels.csv not confident
image/text, bare .csv not confident tabular, empty path rejected, multi-CSV
matches the walk.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(data ingest): sniff confident tabular only on exactly one CSV (#180b)
The family sniff claimed confident tabular whenever a directory held at
least one CSV, but DiscoverTabular's findSingleCSV requires exactly one —
so a two-CSV directory got echoed "this is tabular data" and then the walk
rejected it. That made the sniff more permissive than the walk it mirrors,
breaking the sniff's own contract ("never claims more than the matching
Discover* would accept"). Gate the tabular case on csvCount == 1 so
sniff-confident-tabular holds only for directories the walk would accept
structurally; multi-CSV directories now fall through to the plain family
question. Adds a regression test asserting both the ambiguous sniff and the
walk rejection on the same input.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(cli): single source of truth for the Family<->noun mapping (#200)
The Family<->noun mapping had three hand-maintained copies that had to
stay in sync by hand: push.FamilyNoun (Family->noun), interactive.go's
familyFromNoun (noun->Family), and resolveFamily's literal
[]string{"tabular","image","text"} picker options + "tabular" default.
Consolidate to one ordered familyNounTable in internal/push. FamilyNoun,
the new exported FamilyFromNoun, and FamilyNouns() (picker options +
default) all derive from it. resolveFamily now takes its Select options
and default from push.FamilyNouns() and its reverse lookup from
push.FamilyFromNoun; the local familyFromNoun is gone.
Behavior identical: same order (tabular, image, text), same default
(tabular). build/vet/tests green.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix(data ingest): #180b review — path trim, self-supervised registry flag, honest text sniff echo (#201)
- Trim the interactive path answer before storing it. validateDatasetPath
only trims to check for emptiness, so a pasted " ~/data" (stray space)
otherwise survived, defeated expandHome (first char isn't '~'), and made
filepath.Abs prepend cwd — the family sniff / label-header preview then
read a path that doesn't exist (and silently fell back to free-text label
entry, the data-ingestors#340 class this feature exists to prevent).
- Make SelfSupervised a CategorySpec registry field (set on MLM + CLM) and
have SelfSupervisedText read it, so a new self-supervised task can't be
added without deciding whether it needs a label column — was a hardcoded
two-id switch decoupled from the registry.
- Soften the confident text sniff echo to "looks like": texts/ and
sequences/ map to DIFFERENT tasks (TextSidecarDir), so a family-level
echo must not imply the task the user then picks will load. The walk
stays the authoritative check.
- Drop the unreachable empty-slice branch in defaultLabelChoice (its only
caller already guards len(headers) > 0).
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix(data ingest): don't sniff a mis-cased media folder + labels.csv as confident tabular (#203) (#204)
A directory with labels.csv plus a mis-cased media folder (Images/,
Texts/, Sequences/) fell through SniffFamily's confident-tabular branch:
the subdir switch is case-sensitive so hasImages/hasText stayed false,
labels.csv counted as the one CSV, and the flow confidently echoed
"tabular", skipped the family question, and offered only tabular tasks.
DiscoverTabular then ignores the subdir, reads labels.csv as the single
CSV, and SUCCEEDS — silently ingesting an image/text dataset as a
standalone table with the media files dropped and no error.
Detect a subdir whose name matches a marker (images/texts/sequences)
case-insensitively but not exactly, and bail to ambiguous so the flow
asks the family plainly. Narrow: an unrelated subdir (backup/, raw/)
still sniffs confident tabular, matching DiscoverTabular which ignores
it. The confident image/text branches still require an EXACT match,
mirroring the walk's literal os.Lstat.
Strengthened the two mis-cased tests to assert the sniff is not confident
AT ALL (they previously only checked "not confident image/text" and so
missed the tabular masquerade), plus a case pinning that an unrelated
subdir stays confident tabular.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Asad Iqbal (Saadi) <asad.dsoft@gmail.com>
…/label (#199)
* fix(data ingest): address #197 code-review findings
- interactive picker no longer passes an empty default to survey.Select
after --task's default was dropped (crashed "omit --task to pick
interactively" on a TTY); falls back to the first task.
- rename the guided name prompt "Destination table name" -> "Dataset name"
to match --name / the Review + summary labels (finish the #180 rename).
- --name flag help now states the leading letter/underscore rule the
tightened validator enforces.
- README quickstart: --split train -> --intent train (--split was removed;
--intent stayed canonical).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(data ingest): finish the name-prompt rename + drop dead picker seed
Two review follow-ups on #199:
- The PromptHint above the "Dataset name" prompt still described "the
table created on the cluster (and its folder on the shared storage).
Letters, digits, underscores only" — it leaked the k8s/MySQL ceremony
the rename was removing and stated the old rule (implying a leading
digit is allowed, which ValidateTableName rejects). Reworded to plain
language matching the flag help + validator.
- The picker's default was seeded from a.Spec.Category, which is always
"" in the !taskSet branch, so the len-guarded fallback always fired.
Seed promptCategories[0] directly — the list is a fixed non-empty
package var backed by the hardcoded category registry.
go build ./..., go vet, gofmt -l clean; go test ./internal/cli/...
./internal/push/... green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(data ingest): cover the picker's default-seed path
The #197 fix seeds the task picker with promptCategories[0] when the
task is unset, since survey.Select rejects an empty default. No test
drove taskSet=false with an empty category, so a reseed to "" would
have kept the suite green while crashing a real terminal. This case
omits the "Task" answer so the fake returns the seeded default, then
asserts the category landed on promptCategories[0] — verified to fail
if the seed regresses to "".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Asad Iqbal <asad.dsoft@gmail.com>
…202)
* feat(data ingest): accept a bare .csv + per-family help + path fixes (#181)
RFC-0002 phase 3: flexible file-or-folder input, clearer instructions, and
two path bugs.
- Accept a bare .csv for the tabular / time-series family. DiscoverTabular
now takes either a directory (its existing exactly-one-CSV rule) or a
bare .csv file — both resolve to the SAME staged layout (the CSV staged
as the one labels.csv under the dataset), so the ingestor's contract is
unchanged. This is a CLI-side input convenience only; no data-ingestors
change. Media/label families (image, text) stay directory-only and error
clearly ("… is not a directory").
- Restore the sniff + copy #180b softened: SniffFamily is confident-tabular
for a bare .csv again (mirroring the now file-or-folder-capable walk), and
the interactive path prompt goes back to "Where is your data? (file or
folder)" with a matching hint.
- Per-family layout help: the ingest Long help + the data group help now
show tabular (a .csv OR a folder with one .csv), image (labels.csv +
images/), and text (labels.csv + texts/) instead of only image.
- Rename the positional arg <local-path> → <dataset> in Use/help.
- expandHome now expands "~user" / "~user/…" via user.Lookup, not just
"~" / "~/…"; an unknown/unlookupable user is left literal so the path
check reports it plainly.
- Check path existence BEFORE spec/schema/family validation, so a typo'd
path fails on the path (exit 3, "no such file or directory") instead of a
confusing downstream error (e.g. the task gate).
Tests: bare .csv discovers + stages identically to a one-CSV dir; bare
non-.csv and image/text bare files rejected; sniff confident-tabular on a
bare .csv; "file or folder" copy restored; ~user expansion; nonexistent
path beats the task gate with exit 3; <dataset> arg name.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(data ingest): address #181 review findings — expandHome edge cases + path-check ordering
Three verified review findings on the #181 flexible-input work:
- expandHome mangled a ~user whose passwd home field is blank: user.Lookup
succeeded with an empty HomeDir, so filepath.Join("", rest) produced a
relative path instead of the documented clear literal-path error. Guard
u.HomeDir == "" the same way the current-user branch already guards an
empty $HOME.
- The "path existence FIRST" invariant held only on the flag-only route.
In guided mode runInteractive sniffed the family and previewed the label
header at a non-existent path before the 0b guard ever fired, so the user
answered the whole questionnaire against a bad path. Extract the guard
into statDatasetPath and run it in runInteractive before the family
sniff; a typed exitError from a guided step now propagates unwrapped
(clean message, not "interactive setup: ...").
- expandHome (data ingest) and cluster.expandPath had drifted: #181 added
~user resolution to only one, so `--kubeconfig ~alice/.kube/config`
silently treated alice as a subdir of the current user's home. Consolidate
both into a shared internal/pathutil.ExpandHome (the promote-to-pathutil
plan the old comments kept deferring) so ~-expansion is identical across
every subcommand.
Coverage floor unaffected (internal/cli 73.6% >= 68%). New pathutil package
carries its own contract tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(data ingest): sniff/walk symlink parity + shared isCSV + stale test key
Review polish on the #181 flexible-input work:
- SniffFamily no longer confidently sniffs a symlinked .csv as tabular.
os.Stat follows the link, but DiscoverTabular Lstats + rejectSymlinks
the CSV, so a symlinked .csv is a layout the walk REFUSES. The sniff
would lock the guided flow to tabular and then hard-fail on the walk's
symlink guard — breaking SniffFamily's "never claims more than the
matching Discover* would accept" contract. The bare-file branch now
Lstats and stays ambiguous for a symlink, so sniff and walk agree.
- Extract isCSV(name) and route all four .csv-extension checks through
it (findSingleCSV, DiscoverTabular's bare-file branch, and both
SniffFamily sites). The sniff and walk bare-file rules were copy-pasted
case-fold checks the comments insist must stay in lockstep; one helper
removes the drift risk.
- Fix a stale fakePrompter key in TestRunInteractive_TrimsPath: the path
prompt was renamed to "(file or folder)" everywhere except this one
answer-map key, which still read "(the folder holding it)", so the fake
returned "" and the test failed with "a dataset path is required".
Tests: new SniffFamily case asserts sniff + walk both refuse a symlinked
.csv. go build ./... && go test ./... && gofmt -l . && go vet ./... green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Asad Iqbal <asad.dsoft@gmail.com>
…#183) (#206)
Image expected-size reframe + local min-size floor preview (gated on the deployed ingestor floor), plus Asad's image-only-flag guard: --target-size/--min-size now hard-error (exit 2) on non-image tasks. Reviewed and approved by @saadqbal, who also authored the flag-guard commits.
Closes#183.
Co-authored-by: saadqbal <saadqbal@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* feat(data ingest): wire the 5 CLI-pending text tasks (RFC-0002 phase 4)
Wire token_classification, sentence_pair_classification,
causal_language_modeling, seq2seq, and embeddings so the family-scoped
picker offers them under "Available now" — the ingestor, schema, and
backend already support them; CLI-side discovery/staging was the only gap.
The CLI now MIRRORS data-ingestors' machine-readable layout contract
(di#347/#353) rather than forking its layout rules (RFC-0002 Principle 6):
- Vendor layout.v1.json into internal/schema/ + embed it; extend
scripts/sync-schema.sh to sync + drift-check both contract files. Bump
scripts/.data-ingestors-ref to the #353 merge commit (3c63d9a), which
keeps ingest.v1.json byte-identical (no unrelated drift).
- internal/push/layout_contract.go parses the contract and drives the
ENFORCED record-format checks (sentence_pair: text_a<TAB>text_b;
embeddings: anchor<TAB>positive[<TAB>negative]) at discovery, mirroring
the ingestor's TabSeparatedRecordValidator. Unenforced formats (seq2seq,
causal LM) accept raw text, so the mirror doesn't reject them.
- Fix the self-supervised flags for seq2seq + embeddings (no label
question), and emit the label for the supervised text tasks.
- Flip the 5 CLISupported=true; a drift test pins the Go registry's
family/label/subdir facts against the vendored contract.
semantic_segmentation stays CLI-pending: it awaits the ingestor's mask_id
link column + training sign-off (backend#816).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(data ingest): address #182 review findings — mirror the ingestor exactly
Verified review findings on the text-task wiring, fixed minimally so the CLI
never forks a fact the layout contract / ingestor owns (RFC-0002 Principle 6):
- text.go: the enforced record-format check (sentence_pair, embeddings) now
runs only over the .txt files labels.csv references, mirroring the ingestor's
TabSeparatedRecordValidator manifest walk — a stray unreferenced .txt no
longer fails discovery on a layout the cluster accepts.
- preflight.go: the text-family label preflight was hardcoded to
text_classification; gate the label-column check on !SelfSupervisedText and
the diversity check on IsClassification, so sentence_pair_classification and
token_classification get the right previews (token_classification is NOT
is_classification, so it skips diversity — the ingestor runs BIOLabelValidator
instead). Adds IsClassification to the registry, mirroring the ingestor's
ModalitySpec.is_classification.
- category.go: derive TextSidecarDir from the contract's primary_subdir instead
of a hardcoded MLM special-case; refresh the SelfSupervised docs to cover
seq2seq/embeddings (target from the record's paired fields, not the text).
- data.go: refresh the stale text-branch comment (7 tasks, supervised split).
- sync-schema.sh: restore signal-safe temp-file cleanup (EXIT/INT/TERM, not a
RETURN trap) and make a failed write in write mode return non-zero instead of
a false "wrote" + exit 0 under the `if ! sync_one` errexit suspension.
Part of #182.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(data ingest): make the enforced text-record check match discovered files, not reconstructed names (#209)
Address Asad's review on #209. The enforced text-record check reconstructed
"<stem>.txt" from the manifest instead of matching the files actually
discovered on disk, so it fail-opened in several ways. Mirror the ingestor's
TabSeparatedRecordValidator exactly (RFC-0002 Principle 6):
- Match each manifest filename against the discovered sidecar basenames
(case-insensitive on basename and stem), so a row "a" resolves to texts/a.text
when the ingestor's configured extension is .text — no more hardcoded ".txt".
- Require the filename column locally: a manifest without it now errors clearly
(mirrors the ingestor's "Missing required column: filename") instead of
silently validating nothing.
- Read the manifest with LazyQuotes so a row pandas tolerates (an unescaped
quote) is read here too, not silently dropped and left unvalidated.
- Drive the field-count error message off the contract separator (sepLabel),
so a future non-tab task isn't misdescribed as "tab-separated".
- TextSidecarDir now fails loud on a text category missing from the vendored
contract — that can only be a vendoring/drift bug, not a runtime condition.
- sync-schema.sh checks curl's exit explicitly (a 404 was misdiagnosed as
"not valid JSON" under the set -e-suspending `if ! sync_one`) and adds
--tlsv1.2 to match the rest of the repo.
- Dedupe: shared matchColumnIndex (column resolve) and openCSVReader
(BOM-stripping CSV reader) helpers; slices.Contains over hand-rolled
containsInt.
Tests: .text extension validated, missing filename column errors, case-
mismatched basename validated, pandas-tolerable/Go-strict row read, contract-
driven message.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* feat(data ingest): confirm the inferred tabular schema (#185)
Mirror data-ingestors' di#349 tabular type-inference in the Go CLI
(InferSchema → *SchemaInference), show the inferred col:TYPE for the 4
tabular/time tasks, let the user confirm/amend, and EMIT the confirmed
schema explicitly as spec.schema — so the ingestor uses the CLI's
(di#349-correct) types regardless of its deployed version. Leading-zero
codes stay VARCHAR (the #349 fix), id-like INT and empty-in-sample
columns are surfaced in the confirm step.
Pinned to di#349's value->type contract via a venv-free Go parity test
(testdata/schema_inference_parity.json, 21 cases). Inference is a pure-Go
mirror — no schema-ref bump needed (the min_size/efaeb07 drag-in belongs
to #183), so no validator-goldens regen.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(data ingest): address #185 review findings
- Mixed-timezone DATETIME parity fix: a tabular column of RFC3339 values
with non-uniform UTC offsets (or a mix of tz-aware and tz-naive) is now
routed to VARCHAR, matching the ingestor's schema_inference._infer_datetime
(pd.to_datetime(format="mixed") returns None on mixed timezones, di#349).
Previously each offset token parsed individually via time.RFC3339 and the
column was typed DATETIME, so the emitted spec.schema was a tz-naive
DATETIME that silently dropped the per-row offset in-cluster. Pinned with
TestInferColumnType_TimezoneParity (verified against di#349 / pandas 3.0.3);
the shared ASCII-only parity fixture is left untouched — di#349 carries no
tz case, so adding one would diverge the vendored contract.
- Remove dead API surface built for an interactive confirm/amend step that
is not part of this PR: SerializeSchema, the InferredColumn type, and
SchemaInference.Columns had zero callers/readers repo-wide.
- Correct comments that described that unbuilt step as if it existed
(data.go schema branch; the InferSchema / SchemaInference docs): the CLI
infers the schema mirroring di#349 and EMITS it explicitly; the interactive
prompt captures an optional --schema override; risky columns are surfaced
as warnings.
- Refresh the stale --schema flag help to the full inferred type set
(INT/BIGINT/FLOAT/BOOLEAN/DATE/DATETIME/VARCHAR(n)).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…iguous, not confident tabular (#203) (#211)
* fix(data ingest): warn instead of silently ingesting a mis-cased media folder as a table (#203)
A dataset with labels.csv plus a mis-cased media folder (Images/, Texts/,
Sequences/) already stays ambiguous rather than sniffing confident tabular
(#198's miscasedMarker guard) — but the ambiguous path gave no clue why. So
the user saw only a blind "which is it?" prompt with no hint that their
folder was just mis-cased.
Carry a plain-language hint on the ambiguous FamilySniff for that case,
naming the folder and the lowercase form it should have (e.g. "Images" →
"images"), and surface it via ui.Printer.Warnf in resolveFamily before the
family question. Reuses the same marker set the case-sensitive walk keys on
(markerFold, formerly isMarkerFold, now returns the canonical marker).
Extend TestSniffFamily to cover all three mis-cased markers (adds the
Sequences/ case), assert each stays ambiguous AND carries a hint naming the
rename, and pin that an unrelated subdir stays confident tabular with no hint.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(data ingest): mis-cased media sniff tracks the walk per-filesystem (#203)
Address the code-review findings on the mis-cased-media guard.
- Cross-platform false positive: the guard flagged a mis-cased "Images/"
next to labels.csv as ambiguous and told the user to rename it — but on
a case-insensitive filesystem (macOS APFS, Windows) the walk's own
os.Lstat(<dir>/images) resolves that folder, so Discover already accepts
the layout. The CLI preflight runs on the user's own machine, so a valid
layout was reported broken with an unnecessary rename instruction. Now
the sniff probes the literal lowercase marker path the walk keys on
(markerResolves): if it resolves, treat the folder as the real marker
(confident media, no hint); only when it doesn't resolve (case-sensitive
FS, e.g. Linux) is it the genuine #203 footgun — stay ambiguous + hint.
- Test coverage: add TestResolveFamily_SurfacesMiscasedHint, pinning that
resolveFamily surfaces the advisory hint through the printer before
asking the family plainly (the PR's headline behavior, previously
exercised by no test). FS-aware, so on case-sensitive CI deleting the
Warnf branch fails it.
- Make the SniffFamily mis-cased test FS-aware to match the new behavior,
and drop the dead `&& s.Confident` assertion (unreachable after the
preceding not-confident guard).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…canner buffer (bug-hunt MED) (#208)
* fix(auth): reject unknown --env/$CLIENT_ENV at login instead of silently using prod
BaseURL falls unknown/typo env values back to prod (a lenient library
default), so `login --env staging` or `CLIENT_ENV=prd` silently targeted
production AND persisted it as the active session env for every later
command — the class behind earlier dev-vs-prod confusion.
login PICKS and persists the session env, so a typo must fail there. Add
api.IsKnownEnv (dev/stg/prod, case-insensitive) and validate the resolved
env at runLogin entry, before any network call. ResolveEnv still maps
empty->prod, so the no-flag default is unaffected. BaseURL's unknown->prod
fallback is deliberately unchanged (TestBaseURL asserts it).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(submit): raise log-scanner buffer to 16 MB so tqdm progress can't force a false exit 9
tqdm (a data-ingestors dep) redraws its progress bar with \r and no \n,
so a whole ingestion phase's redraws are one newline-delimited "line" that
grows for the life of the run. Past 1 MB the display scanner returned
bufio.ErrTooLong, cutting the log stream mid-run; a still-running Job then
couldn't be confirmed terminal in the 30s finalJobStatus poll, so watch
returned a false exit 9 on a healthy large ingestion — exactly the case the
1h JobWatchTimeout targets.
The parser is fed via the TeeReader, not the scanner, so this cap only ever
bounded the DISPLAY line and never the verdict. Raise it to 16 MB (clears a
fast ~10/s hour of redraws with headroom; the 1h cap bounds accumulation).
The buffer grows on demand, so ordinary log lines still cost 64 KB.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(submit): drain past ErrTooLong so a giant tqdm line can't force a false exit 9 (review)
Addresses @saadqbal's review on #208: the 16 MB buffer bump only MOVED the
false-exit-9 threshold, it didn't close it. The tee is pulled only by the
DISPLAY scanner, so when a line trips ErrTooLong the scan loop exits, the tee
stops being read, and the parser never sees the rest of the stream (the closing
banner) → streamFailed && outcome==Unknown → a false exit 9 on a healthy run.
A long enough single '\r'-line (> the buffer) still breaks it.
Class-level fix (his suggestion): keep draining past ErrTooLong. Extracted the
display/parse loop into streamDisplayAndParse; on ErrTooLong it drains the rest
of the stream THROUGH the tee (io.Copy to io.Discard) so the parser still sees
the banner, and it is NOT fatal — the Job status poll is the verdict's source of
truth. Genuine read failures (network drop, ctx cancel) still propagate.
Corrected the now-wrong "cap never affects the verdict" comment; kept 16 MB as a
generous display headroom (the drain is the correctness guarantee).
New tests (the #3 no-test gap Asad noted): an oversized '\r'-line + the real
ingestor banner → the parser still resolves the summary (no false exit 9); and
a genuine read error still propagates.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(push): pin Inf/NaN → not FLOAT against the di#349 inference
#208's fix#2 (demote Inf/NaN columns to VARCHAR) is superseded by #185/#210:
the di#349 floatRE grammar pre-screens the token before ParseFloat, so
"inf"/"Infinity"/"NaN" already fall through to VARCHAR. Dropped the redundant
production change on rebase; kept the intent as a regression test, since no
parity-fixture case covers non-finite.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…205)
* fix(delete): don't brick local teardown when the server-side revoke isn't a 403
runDelete treated the credential revoke as a hard gate: only HTTP 403 was
special-cased (ask-an-admin); every OTHER RevokeClient error returned
exitError{code:1} and short-circuited the entire offboard — helm uninstall, k3d
teardown, image prune, ~/.tracebloc wipe, and self-remove never ran. So a stale/
wrong-account active-client pointer (account-scoped 404), a backend predating the
/edge-device/<id>/revoke route (404), or a transient network/5xx blip left the
machine fully installed with no escape (--force only skips the online guard).
That contradicts the same function's online-guard, which already treats
5xx/429/network as "warn and continue — the teardown is the real gate," and its
own comment anticipating a 404 revoke.
Fix: non-403 revoke failures now warn and continue into the (offline-capable)
local teardown, mirroring the online guard. 403 still routes to ask-an-admin
(unchanged, still tested). "Revoked …" is printed only on actual success; the
warn path says the credential may still be live (revoke from the dashboard; the
orphan reaper backend#970 sweeps a never-torn-down record later).
Test: TestDelete_RevokeNon403_ContinuesTeardown (404 revoke → teardown still runs,
honest warning, no false "revoked" claim). Existing TestDelete_RevokeForbidden
(403 → ask-an-admin) unchanged. Full suite green; gofmt -s / errcheck /
ineffassign clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(delete): honest offboard summary + treat 426/401 revoke as terminal
Addresses @saadqbal's #205 review:
- 426 revoke now fails fast with the upgrade prompt (mirrors the pre-offboard
guard) instead of warn-and-continue into a teardown the backend can't process.
- 401 revoke now fails fast to re-login. The --force footgun: with --force the
online guard is skipped, so an expired session used to silently tear the machine
down while leaving a live credential; now it aborts to sign-in.
- Track whether the server-side revoke actually succeeded; the closing summary is
honest on BOTH axes (revoke status x teardown status) and no longer claims the
credential is revoked / the machine disconnected when the revoke failed.
- Fix the now-false "credential is already revoked" comment on the teardown block.
- Tests: 404 + no-namespace honest closing (the degraded+revoke-failed case the
old test never reached), plus 426 and 401 revoke fail-fast.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ion (#207)
* fix(preflight): resolve labels.csv filename by column name, not position
CrossCheckLabels (image_classification preflight) read the image filename from
rec[0] — the first column, positionally. But the ingestor reads it BY NAME:
record.get("filename") over the header-keyed record (file_transfer.py /
record_processor.py), order-independent, and every template's labels.csv names
the column "filename". So a `label,filename` header (filename present but not
first — a layout the cluster ingests cleanly) made the CLI read the LABEL value
("cat") as the filename, miss every row, and reject with exit 3 ("N labels.csv
row(s) reference images that aren't in images/") — a false reject that also
misleadingly listed label values as missing images. Violated the "never stricter
than the ingestor" preflight contract.
Fix: resolve the filename column by name — exact "filename" first, then
case-insensitive + whitespace-trimmed (the ingestor's _match_column rule) — and
read that column's index. Falls back to column 0 when there's no filename-ish
column (a malformed layout the ingestor fails on regardless; not this check's job
to diagnose). Short/ragged rows without that cell are skipped.
Tests: the existing test's unrealistic `image_id,label` fixture corrected to the
real `filename,label`; new TestCrossCheckLabels_FilenameColumnNotFirst pins that
`label,Filename` (not first, mixed case) resolves correctly and does NOT read
labels as filenames (fails against the old rec[0]); TestFilenameColIndex covers
first/not-first/case/whitespace/fallback. Full suite green; gofmt -s / errcheck /
ineffassign / misspell clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(preflight): resolve the image column as filename else data_id, like the ingestor
Addresses @saadqbal's #207 review. The ingestor resolves the image file key as
`filename` else `data_id` (image_paths / image_loader), position-independent for
both — so a valid `label,data_id` CSV (no filename column) was falling back to
index 0 (the label column) and false-rejecting (exit 3) a dataset the cluster
ingests cleanly.
imageFileColIndex (renamed from filenameColIndex) now matches filename-else-data_id
with the same exact-then-case-insensitive-trimmed rule; filename wins when both are
present; it falls back to 0 only when NEITHER exists (a labels.csv the ingestor
rejects at validate_data anyway). Added a label,data_id CrossCheckLabels regression
test + data_id cases to the index test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#216)
Wire the sequence-grouped time-series task end to end on the CLI side:
- category registry: time_series_classification, FamilyTabular,
CLISupported, IsClassification (mirrors the ingestor registry's
is_classification=True). Not RegressionClass: real class labels use
the plain string label form, no label.policy. --task help and the
interactive picker derive from the registry, so both pick it up
without further edits. No spec.go / flag / prompt changes — the
sequence_id / timestamp column names are fixed (Decision-2).
- re-vendor ingest.v1.json + layout.v1.json from the WS1 branch
(data-ingestors#359) and bump scripts/.data-ingestors-ref to its
head SHA so sync-schema.sh --check stays green. The registry<->schema
parity tests pin the new enum both ways.
- layout contract: parse the new grouping trait (Decision-4) and expose
GroupingFor; grouped preflight behaviour gates on the trait, never on
a category id.
- preflight: extend the label-diversity gate from a hardcoded
tabular_classification to IsClassification, and add the cheap local
sequence checks — CheckSequenceSchemaColumns (fixed sequence_id /
timestamp columns declared in the schema, previewing the ingest.v1
conditional) and CheckSequenceRows (no null/empty sequence ids,
previewing SequenceGroupValidator; also yields the sequence count,
echoed as a note since the platform counts sequences, not rows —
Decision-3).
- parity harness: 6 new tsc-* cases with goldens generated from the
REAL WS1 validators (existing goldens unchanged). tsc-label-flip and
tsc-unsorted-timestamp document the two deliberate divergences (no
local whole-group label-constancy / per-group order preview yet).
- README: category listing updated (15 of 16 supported).
Part of backend#1054 (WS2, backend#1057). Requires data-ingestors#359
in the same release window (T16) — the vendored schema comes from that
branch.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…us (review) (#212)
Adversarial review of the earlier drain fix caught a vacuous test:
bufio.Scanner's token cap is max(maxLine, cap(initialBuf)), so passing a 64 KB
initial buffer with maxLine=1024 silently capped at 64 KB — the test's ~7.5 KB
"oversized" line never tripped bufio.ErrTooLong, so the drain branch was never
exercised (the test still passed with the drain deleted).
Fix the helper so maxLine is authoritative: clamp the initial buffer to maxLine
(64 KB otherwise). Production passes 16 MB, so the clamp is a no-op there — the
initial buffer stays 64 KB and grows on demand exactly as before. The test now
uses maxLine=4096 with a >4 KB line (guarded against future vacuity), which
genuinely trips ErrTooLong. Verified by mutation: removing the drain now makes
the test FAIL (InsertedRecords=0, the false exit 9), and restoring it passes.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Closes the T16 coupled release opened by #216: the vendored schema was
pinned to the WS1 PR-branch head pre-merge; data-ingestors#359 is now
merged, so the pin moves to the merge commit on develop
(7b4ecac21ee491998ea4252daace9a5af6a1cb4a). sync-schema.sh confirms
both vendored files (ingest.v1.json, layout.v1.json) are byte-identical
to the merge commit - no vendored content changes, ref-only.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@saadqbalsaadqbal self-assigned this Jul 10, 2026
Comment threadinternal/push/text.go
@saadqbal

Copy link
Copy Markdown
CollaboratorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

1 issue from previous review remains unresolved.

Fix All in Cursor

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 2a5e0e6. Configure here.

@saadqbal

Copy link
Copy Markdown
CollaboratorAuthor

Bugbot's finding here is addressed in #221 (targets develop, so it joins this promote). One correction: the "skips bad CSV rows" framing isn't reproducible — LazyQuotes + FieldsPerRecord=-1 parse every malformed shape cleanly (like pandas). The real issue is the continue swallowing a mid-walk read error into a partial referenced set (fail-open, + a latent infinite loop on a persistent error). #221 makes it fail closed, matching CrossCheckLabels.

…reflight) (#221)
manifestReferencedTextNames skipped a failed CSV read with `continue`,
returning a partial referenced set — so the text files named by the
unread rows would silently escape the enforced record-format check
(local fail-open), and a *persistent* read error would spin the loop
forever. The image mirror-check (CrossCheckLabels) already aborts on the
same read error; make the enforced-text path match and fail closed.
The trigger is I/O, not malformed CSV content: openCSVReader sets
LazyQuotes + FieldsPerRecord=-1, so every bad CSV shape parses cleanly
(like pandas) — the branch is only reachable via a genuine read failure.
The parse loop is split into referencedTextNames(*csv.Reader) so that
branch can be exercised with an injected failing reader.
Addresses the Bugbot finding on #219.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Comment threadinternal/push/preflight.go
Comment threadinternal/push/preflight.go Outdated
@saadqbal

Copy link
Copy Markdown
CollaboratorAuthor

The two follow-up findings (readLabelColumnValues :509, CheckSequenceRows :750) are fixed in #222 — same class as #221 (continue swallowing a mid-read error). Label diversity now fails closed (partial column no longer under-counts → no false reject/pass); sequence scan surfaces the read error instead of skipping the tail. CrossCheckLabels + the tabular scan already did this, so internal/push is now uniform. Both split into reader cores so the branch is actually tested; parity harness confirms the parsing is unchanged.

…preflight (#222)
Two more mirror-checks swallowed a non-EOF csv.Reader.Read error with
`continue`, same class as #221:
- readLabelColumnValues (CheckLabelDiversity / ReadLabelValues): a
mid-read failure returned a PARTIAL class set with Found=true —
under-counting classes could false-reject good data or pass a bad
file. Now returns an error; the diversity GATE fails closed, while the
value PREVIEW (ReadLabelValues) degrades to Found=false.
- CheckSequenceRows: a mid-read failure skipped the unread tail, so
null/missing sequence ids there never surfaced locally. Now fails
closed.
CrossCheckLabels and the tabular schema scan already abort on the same
error; this brings the whole package to uniform fail-closed behavior.
The trigger is I/O, not malformed CSV (LazyQuotes + FieldsPerRecord=-1
tolerate every bad shape). Each scan is split into a reader-based core
(labelColumnValuesFrom / sequenceScanFrom) so the branch is exercised
with an injected failing reader; parsing is unchanged and the value
parity harness validates the relocation.
Addresses the two follow-up Bugbot findings on #219.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f05d01d. Configure here.

Comment threadinternal/push/preview.go Outdated
saadqbaland others added 4 commits July 10, 2026 21:32
…kers) (#223)
* fix(push): SniffFamily mirrors the walk for symlinked/mis-cased/lone markers
SniffFamily inferred layout markers from the directory listing (e.type/
IsDir + a case-sensitive name compare), which diverged from the walk's
os.Lstat-based, symlink-rejecting, case-following resolution — so several
layouts sniffed "confident tabular" and got silently ingested as a table when
the walk would have refused them or the data was actually image/text:
- A symlinked images/ / texts/ / sequences/ (IsDir()==false) fell through to
confident tabular → an image/text dataset ingested as a table, media dropped.
- A plain file named images/… did the same.
- A lone labels.csv with no media dir sniffed confident tabular, though
labels.csv is the image/text manifest name (usually an incomplete media set).
- labels.csv was matched case-sensitively while the walk resolves it
case-insensitively on macOS/Windows (under-claimed there).
- A dir whose only CSV was a symlink was counted, though DiscoverTabular
rejects a symlinked CSV.
Now media markers are probed with markerResolves (the walk's literal-lowercase
Lstat), labels.csv with a new regularFileResolves (regular-file Lstat, so it
tracks the walk's case behavior and rejects a symlink/dir), symlinked CSVs are
skipped from the count, any marker-named entry the walk can't use is flagged
ambiguous with a fix hint, and a lone labels.csv stays ambiguous. Generalises
the mis-cased-marker fix (#203) to its symlink/file/lone-manifest siblings and
resolves the open Bugbot finding on #219.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(cli): match the generalized sniff hint wording (fix vs rename)
The mis-cased-marker hint became a general badMarker hint covering symlinks
and files too, where "rename" isn't the right verb — the message now says
"fix it and ingest again". Update the resolveFamily consumer test's assertion
to match (it fired on the case-sensitive-FS branch in Linux CI).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(push): count symlinked CSVs like findSingleCSV in the sniff
Bugbot follow-up on #223: SniffFamily skipped symlinked .csv entries from
csvCount, but findSingleCSV counts every non-dir .csv (symlinks included)
toward its exactly-one rule. So a directory with one regular CSV plus a
symlinked CSV sniffed confident tabular while DiscoverTabular rejects it as
multi-CSV. Count symlinks too, and track csvSymlink so a lone symlinked CSV
(which DiscoverTabular rejectSymlinks) stays ambiguous rather than confident.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…but-ok jobs (#224)
* fix(cli,submit): reject misapplied task flags; don't false-fail slow-but-ok jobs
Five findings from a full adversarial review of the v0.8.0 payload:
cli/data.go — task-scoped flags (--schema, --label-policy, --time-column,
--number-of-keypoints, and --label-column on self-supervised text) were read
only inside their one category branch, so passing one on a task that doesn't
use it silently dropped the value and the user's intent — despite the help
text saying each is scoped. Reject them explicitly (exit 2), mirroring the
existing --target-size/--min-size guard and spec.go's build gates.
cli/interactive.go — pickTask mapped the chosen display name back to a task ID
through a DisplayName→ID map; two tasks sharing a display name would collide
and return the wrong ID. Map by the offered option's position instead.
submit/watch.go —
* finalJobStatus timing out on a slow apiserver returned Unknown → a false
exit 9 even when the ingest actually succeeded. Fall back to the Pod phase
(Succeeded/Failed) before giving up. The 30s bound is now a package var so
the fallback is unit-testable.
* the most-recent-Pod selection tied on CreationTimestamp's 1s granularity;
a same-second backoffLimit retry could leave the watch on the old Failed
Pod. Break the tie toward the live/succeeded Pod.
Tests added for every case (wrong-task rejection + a correct-task negative
control; the pod-phase fallback; the same-second tie-break).
Addresses review findings on #219 (C1/C2/D1/D2 + the --label-column echo).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(submit): re-select the current pod in the Unknown-status fallback
Bugbot follow-up on #224: the pod-phase fallback read the podName captured at
the start of the watch. waitForJobPod can return an early Failed Pod while a
backoffLimit retry is still Pending, so classifying off that stale name could
report Failed for a run that ultimately succeeds. Re-list Pods and re-select
the current most-recent useful one (extracted as mostRecentUsefulPod, shared
with waitForJobPod) so the fallback reflects the retry's real outcome.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…e-reject (#225)
Root cause behind the read-error findings: openCSVReader set FieldsPerRecord=-1
but NOT LazyQuotes, so a bare/unescaped quote in a row — which pandas (the
ingestor) tolerates — made csv.Reader.Read return a parse error. Every preflight
mirror-check (CrossCheckLabels, readLabelColumnValues, CheckSequenceRows,
CheckHasDataRows, ReadCSVHeader) therefore diverged from the ingestor in the
inverse direction: it REJECTED a layout the cluster ingests cleanly.
It also meant the fail-closed change earlier in this PR was reachable via a
bare-quote row, turning a fail-open into a false-reject. Setting LazyQuotes
centrally in openCSVReader (and in InferSchema's own reader, which doesn't use
it) makes the readers pandas-faithful; the remaining non-EOF Read error is then
only ever genuine I/O, so failing closed on it is correct.
Added a parity guard test (bare-quote CSV → read like pandas, not rejected).
The full value/verdict parity harness still passes, confirming LazyQuotes does
not over-accept relative to the ingestor.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…226) (#227)
SummaryParser.Feed accumulated bytes into p.buf until a '\n'. The
display path caps an oversized tqdm line via displayLineMax (16 MB) in
streamDisplayAndParse and drains the rest, but the drained bytes still
flow through the TeeReader into Feed — so an ingestor emitting many MB
of '\r'-redraws with no '\n' for the life of a (up to 1h) run grew
p.buf without the display-side cap ever applying to the parser.
Bound buf at parserLineMax (= displayLineMax; same package, referenced
directly so the two paths can't drift). When the partial newline-less
line passes the ceiling, drop it and enter drop-until-newline mode so
the oversized line's tail is discarded too rather than parsed as a
spurious fresh line; FlushLine honors the same state at EOF. A real
banner line is tens of bytes, so newline-less content past 16 MB can
never be one — dropping is safe and the parser still recovers to parse
the closing banner once a '\n' finally lands.
Adds a white-box test feeding a >2x-parserLineMax newline-less flood,
asserting buf stays bounded and a real banner after the flood parses.
Deferred finding D3 from the v0.8.0 review (#220). Low severity.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@saadqbalsaadqbal added the skip-fr-gate Maintainer override: bypass the FR gate on a develop→main promote (audited) label Jul 10, 2026
@saadqbal

Copy link
Copy Markdown
CollaboratorAuthor

Applied skip-fr-gate (maintainer override) — shipping past the FR gate without the full staging cycle, same as v0.4.0. Payload was hardened via the review sweep (all Bugbot findings on the ingest rework resolved across #221#225 + #227), not a staging deploy, so this is a deliberate audited bypass rather than a claim that every card reached Ready-for-prod.

@saadqbal
saadqbal merged commit b7fbdcc into mainJul 10, 2026
32 of 33 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-fr-gateMaintainer override: bypass the FR gate on a develop→main promote (audited)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@saadqbal@LukasWodka