Skip to content

feat(data ingest): rename --category/--table/--intent → --task/--name/--split (#180a) - #197

Merged
saadqbal merged 2 commits into
developfrom
feat/180a-task-split-name-renames
Jul 9, 2026
Merged

feat(data ingest): rename --category/--table/--intent → --task/--name/--split (#180a)#197
saadqbal merged 2 commits into
developfrom
feat/180a-task-split-name-renames

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Stacked on #179 — merge after #179. Base is feat/179-plain-language-ceremony, not develop; the diff shown against develop until #179 lands will include #179's commit. Retarget to develop once #179 merges.

Part of #180. This is #180a, the mechanical flag renames from RFC-0002 §6. It does notclose#180 — the data-first flow inversion + task-picker rework is #180b, stacked on top of this.

Summary

Renames the three core data ingest flags to plain, task-first names, and keeps every old flag as a hidden deprecated alias so existing scripts don't break. The wire/spec field names (category / table / intent) are unchanged — this is a CLI-surface rename only.

Old (now hidden alias)New (canonical)Notes
--category--taskold image_classification default dropped
--table--namewire field stays table
--intent--splitdefault train

What changed

  • --category--task, and the image_classification default is dropped. Omitting the task now drives the interactive picker (on a TTY) or returns a clear which task is this data for? pass --task … error (non-interactive) — never a silent image assumption.
  • --table--name. Plain-language help ("a name for this dataset — you'll reference it by this name when you start a training run"); the k8s/PVC detail is gone from the flag help.
  • --intent--split, default train. The guided prompt is reworded to "Is this training or test data?". The default is applied after the interactive block, so guided runs still ask; non-interactive runs that omit it get train.
  • 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.
  • Hidden aliases are registered with MarkHidden (absent from --help) and reconciled in RunE: the canonical flag wins; a legacy alias fills in only when the new flag wasn't passed.

Behavior is otherwise identical — exit codes, guards, and the destination-exists (exit 6) check are unchanged. No overwrite-promptability (that's #180b).

Test plan

go build ./..., go test ./..., gofmt -s -l . (empty), go vet ./..., plus errcheck / ineffassign / misspell — all green locally.

New / updated unit tests:

  • TestDataIngest_DeprecatedFlagAliases — old names resolve; a bad value via --category still reaches the task gate (aliased value flows through, not ignored).
  • TestDataIngest_OmitTask_NonInteractive_Errors — omitting --task off a TTY exits 2 with a message naming --task.
  • TestDataIngest_OmittedSplit_DefaultsToTrain — omitting --split no longer rejects; falls through to the injected bad kubeconfig (exit 3).
  • Existing exit-code + interactive tests converted to the canonical flags; prompt-answer keys updated to the new labels.

Manual smoke test against a bad kubeconfig (built binary):

  • new flags → exit 3 (spec valid, reached cluster step);
  • old flags via alias → exit 3 (identical);
  • omit --task → exit 2 with the clear --task error;
  • omit --split → exit 3 (default train applied);
  • canonical wins over alias on conflict (bad alias value ignored → exit 3);
  • deprecated aliases absent from data ingest --help.

Decisions to ratify

  1. --split default train is applied post-interactive (an if a.Spec.Intent == "" { … = "train" } fallback), not baked into the cobra flag default. This keeps the guided flow asking "training or test?" while a non-interactive omit still gets train. RFC-0002 §5.
  2. Canonical-wins-on-conflict: if both --task and --category (etc.) are passed, the new flag wins and the alias is ignored. Documented and tested.
  3. JSON --output-json keys are left as category/table/intent (wire format — renaming would break scripts, the opposite of the alias promise).
  4. Deferred copy nit (not this PR): the exit-6/exit-7 error strings still say "uploading everything" — banned per tracebloc voice ("ingest", never "upload"). Left for a copy pass so #180a stays scoped to renames; only the --table--name token was touched on those lines.

Note

Medium Risk
CLI flag and validation behavior changes can break scripts that relied on --category defaulting to images or numeric-leading --table names; hidden aliases mitigate rename risk but not the stricter name rule or missing-task error.

Overview
Renames the primary data ingest flags to --name and --task (plain-language help, pre-flight labels name / task), with hidden --table / --category aliases so scripts keep working; canonical flags win on conflict. Wire/spec fields stay table / category / intent; --output-json keys are unchanged.

--task no longer defaults to image_classification—omitting it triggers the interactive picker on a TTY or a clear exit-2 error off-TTY. --intent is still the flag name but now defaults to train after guided prompts (guided flow still asks train vs test).

User-facing copy shifts from “category/table” to “task/name” across help, errors, and interactive prompts (TaskSet replaces CategorySet).

ValidateTableName now requires [A-Za-z_][A-Za-z0-9_]*, aligned with the ingestor—leading-digit names like 123 fail at the CLI instead of after staging.

Tests cover deprecated aliases, omitted --task, and default intent; README example uses the new flag names.

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

@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

Pushed the two review fixes (ba01f9e).

A — reverted the --intent--split rename.--intent stays the canonical flag; --split is gone entirely (no hidden alias for it). The --category--task and --table--name renames are untouched, still with their hidden deprecated aliases. Dropped the splitVal reconciliation block and the extra flag registration, so the surface is just --intent. The wire/spec field stays intent, and the local summary + interactive review now label the field "intent". Renamed the test to TestDataIngest_OmittedIntent_DefaultsToTrain and swapped every --split=train for --intent=train.

Default-train decision (kept): omitting --intent still defaults to train. It's applied after the interactive block, so the guided flow still asks "training or test?" and a non-interactive run that omits it gets train without erroring (RFC-0002 §5).

B — tightened table-name validation to mirror the ingestor. The ingestor's validators/table_name_validator.py is the source of truth and requires ^[a-zA-Z_][a-zA-Z0-9_]*$ — a name must start with a letter or underscore. The CLI's tableNamePattern was looser (^[A-Za-z0-9_]+$), so it accepted leading-digit / all-digit names like 123 and 1data that the cluster then rejects post-upload. Changed the regex to ^[A-Za-z_][A-Za-z0-9_]*$, updated the doc comment and error message to state the leading letter/underscore requirement and that it mirrors the ingestor, and added TestValidateTableName_LeadingDigit (rejects 1data/123; accepts _data/Data1/chest_xrays_train). Also dropped the now-invalid 9starts_with_digit case from the accepts test.

go build ./..., go test ./... (all green), gofmt -l . (clean), and go vet ./... all pass locally.

@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 ba01f9e. Configure here.

Comment threadREADME.md
Base automatically changed from feat/179-plain-language-ceremony to developJuly 9, 2026 13:12
LukasWodkaand others added 2 commits July 9, 2026 15:29
…e/--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>
…le-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>
@LukasWodka
LukasWodkaforce-pushed the feat/180a-task-split-name-renames branch from ba01f9e to b4e5678CompareJuly 9, 2026 13:29
@saadqbal
saadqbal merged commit 4ea80cc into developJul 9, 2026
16 checks passed
@saadqbal
saadqbal deleted the feat/180a-task-split-name-renames branch July 9, 2026 14:02
saadqbal added a commit that referenced this pull request Jul 9, 2026
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>
saadqbal added a commit that referenced this pull request Jul 9, 2026
#180b (#198) landed the data-first flow inversion + family-scoped task
picker on develop, which supersedes most of this branch:
- The flat Select("Task", ...) picker — and its empty-default crash fix
(#197 finding 1/2) — is gone, replaced by resolveFamily + pickTask.
Resolved interactive.go to develop's new flow.
- The "Destination table name" → "Dataset name" rename is superseded by
develop's "What should we call this dataset?" prompt. Resolved the
test conflicts to develop's rewritten new-flow suite (my old-flow
tests, incl. the seed test, are obsolete).
Kept the still-relevant copy fixes:
- README quickstart --split → --intent (auto-merged).
- --name flag help states the leading-letter rule (auto-merged), and I
folded the same wording into develop's interactive name prompt so the
prompt and the flag help agree (finding 4 drift, otherwise reintroduced
by the merge).
gofmt -l clean; go build ./..., go vet, go test ./internal/cli/...
./internal/push/... green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal added a commit that referenced this pull request Jul 10, 2026
…/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>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@LukasWodka@saadqbal