feat(ingest): guided mode always asks, pre-filled — never skips a question by LukasWodka · Pull Request #505 · tracebloc/cli

LukasWodka · 2026-08-14T13:41:57Z

Closes #509. Follow-ups filed as #504 (prompt labels + answer recall) and tracebloc/backend#1990 (derive the task→questions mapping from ingest.v1.json).

What went wrong

A user answered two questions of the guided flow and then got:

Step 1 of 4 · Do you want to ingest training or test data?
? test
Step 2 of 4 · Please name the dataset.
? testing12344
Error: no such file or directory: "data" — check the path to your dataset

The path check was correct — they'd passed data as the positional argument and had no such folder. The problem is that step 3 never asked. It was guarded by if a.LocalPath == "", so a supplied path skipped the question and went straight to validating it.

Every step was guarded that way: intent, name, path, task, and the per-task extras.

Why "skip what was supplied" is the wrong rule

For a name or a task, what you passed is still true next run. For a path it isn't — data moves, and the path is the one answer that silently stops being true. The result is a dead end: the user is sitting at a prompt, ready to answer, and gets a hard error instead of the question that would fix it in two seconds. Anyone with muscle memory for tb data ingest ./data hits it the moment their data moves.

Rather than special-case the path:

In guided mode, every question relevant to the chosen task is asked. A value that arrived on the command line becomes that question's default — Enter accepts it — never a reason to skip. In non-interactive mode (--no-input, no TTY, --output-json) nothing is ever asked and flags/positionals are obeyed exactly as before.

The task gate survives, one level down: which questions apply still depends on the task chosen at step 4 (self-supervised text has no label and no extras). That gate is about relevance, not about what the user typed.

Non-interactive is untouched — runInteractive is only reached on a TTY without --no-input/--output-json. This matters because the positional is the only way to pass a path (there is no --path flag), so scripts and CI depend on it.

Details worth a reviewer's attention

An explicit --task now settles the family directly instead of skipping the picker. Scoping the list to the sniffed family would let the two disagree — a tabular task against a folder that sniffs image — and leave the user's own answer missing from its own default.
pickTask only honours a pre-selection that is actually in the family's list. Survey would otherwise render a default the user cannot see, and an Enter on it would be unexplainable.
The review + confirm is now unconditional. It was gated on "did we prompt anything", which can only ever be true now; keeping the condition would read as a choice while having none.
TaskSet is removed, not left threaded through unused — it was only ever read to decide the skip.

Test plan

make check and make check-all green — vet, full test suite, fmt, file budget, style guard, tool pins, deadcode, coverage.

The two tests that specified the old rule are inverted, not deleted — they were the specification, so they now specify the new one:

TestRunInteractive_ExplicitTaskSkipsSniff → …_ExplicitTaskStillAsks (asks, pre-selects, still no sniff)
TestRunInteractive_SkipsProvidedValues → …_AsksEvenWhenFullySpecified

New: …_SuppliedValuesArePrefilled (a supplied value survives as the default — the regression that would make "always ask" mean "always retype"), plus …_UnsuppliedDefaultsUnchanged and …_LabelPolicyDefaultUnchanged.

Those last two exist for a specific reason: three defaults ("train", "bucket", "time") moved from inline arguments into variables so a supplied value can take their place, which drops them from zz-all-strings.golden. The golden diff shows that. Rather than lose the guarantee, it's replaced with a stronger behavioural one.

The catalog's own description asserted "Passing flags … skips the matching questions", which this makes false — updated in the same change.

🤖 Generated with Claude Code

Note

Medium Risk
Touches the primary interactive ingest path and task/flag consistency; flag-only and CI behavior stay gated off TTY/--no-input, but guided UX and edge cases around task changes need careful review.

Overview
Guided ingest (#509) on a TTY no longer skips steps when flags or a positional path are already set. Each question still runs, with command-line values as the default (Enter keeps them). Non-interactive paths (--no-input, no TTY, --output-json) are unchanged. TaskSet / the taskSet argument are removed.

Task picker: --task no longer bypasses the picker; it picks the family from the supplied task and pre-selects that option (so family and default stay consistent when layout sniff disagrees). Review and confirm always run in guided mode.

Safety: New task_scope.go shares misapplied-flag rejection with rejectMisappliedTaskValues and dropValuesLeftBehindByATaskChange after a real task change in the picker (so e.g. --time-column does not linger on a new task). Misapplied flags on the CLI are still rejected; unknown --task values are not treated as a “change.” defaultInOptions guards Select defaults so typos do not crash survey on a real terminal.

Tests and copy-catalog goldens document “pre-fill, don’t skip” instead of “flags skip questions.”

^{Reviewed by Cursor Bugbot for commit a494c67. Bugbot is set up for automated code reviews on this repo. Configure here.}

…stion A user hit "no such file or directory: data" after answering two questions of the guided flow. Not a defect in the check: they had passed `data` as the positional path, and step 3 is guarded by `if a.LocalPath == ""`, so the flow skipped "Where is your data?" and went straight to validating a path that was never asked about. Every step was guarded that way — intent, name, path, task, and the per-task extras. Skipping is defensible for a name or a task: what you passed is still true on the next run. It is not defensible for a PATH, which is the one answer that silently stops being true when data moves. The result was a dead end: the user sitting at a prompt, ready to answer, getting a hard error instead of the question that would have fixed it in two seconds. Rather than special-case the path, the rule is now uniform and simple: In guided mode, every question relevant to the chosen task is asked. A value that arrived on the command line becomes that question's DEFAULT — Enter accepts it — never a reason to skip. In non-interactive mode (--no-input, no TTY, --output-json) nothing is ever asked and flags/positionals are obeyed exactly as before. The task gate survives, one level down: which questions apply still depends on the task chosen at step 4 (self-supervised text has no label and no extras). That gate is about relevance, not about what the user typed. Details worth knowing: - An explicit --task now settles the FAMILY directly instead of skipping the picker. Scoping the list to the sniffed family instead would let the two disagree — a tabular task against a folder that sniffs image — and leave the user's own answer missing from its own default. - pickTask only honours a pre-selection that is actually in the family's list; survey would otherwise render a default the user cannot see. - The review + confirm is now unconditional. It was gated on "did we prompt anything", which can only ever be true now; keeping it would be a condition that reads as a choice while having none. - TaskSet is removed rather than left threaded through unused. Three defaults ("train", "bucket", "time") moved from inline arguments into variables so a supplied value can take their place, which drops them from the zz-all-strings index. Replaced with something stronger: tests that assert the fallbacks behaviourally. The two tests that specified the old rule are inverted rather than deleted — they were the specification, so they now specify the new one. Refs #711 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…lumn Guided ingest pre-fills survey.Select prompts with command-line values, but survey aborts when a Default is not one of the Options ("default value ... not found in options"). A typo'd --intent or --label-policy therefore crashed the prompt on a real TTY, while the fake prompter ignored Default so the tests stayed green. Add a shared defaultInOptions guard — the check pickTask already applied — and route intent, label-policy and the label-column pick through it; an unknown supplied value now falls back to the sensible default and the question is still asked. pickTask is unified onto the same helper. Also stop the label-column step from skipping when --label-column was supplied: like every other value under #711 it now pre-fills the header-backed picker instead of bypassing it, so a wrong or mistyped column can be corrected at the prompt the same way a stale path can. Make fakePrompter.Select honour survey's default-in-options contract so the crash class can no longer pass in tests, and add mutation-proven coverage. Fixes two Cursor Bugbot findings on #505. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

LukasWodka · 2026-08-14T14:25:08Z

bugbot run

Bugbot, and it is the guided flow contradicting its own promise. `pickTask` wrote the chosen task and left every task-scoped value from the ORIGINAL --task on the spec. Start a run as tracebloc data ingest ./d --task time_to_event_prediction --time-column t pick tabular_classification at the prompt, and TimeColumn rides through: no prompt asks about it (it is time_to_event_prediction-only), Review shows it anyway, and the run then dies AFTER the confirm with exitBadInput blaming a flag the user just spent a prompt walking away from. #711's whole claim is that the answers on screen are the run; a value no question asked about and no answer can reach is not one of them. Same for --label-policy, --number-of-keypoints, --target-size and --min-size. The reset is one call after the picker. What it needed was a place to read the scopes from, and there wasn't one — the misapplied-flag guard held them as five inline conditions plus a two-element loop. A second hand-written copy in the guided flow is the failure mode this codebase keeps finding: both copies pass their own tests while disagreeing with each other, and the disagreement surfaces as an error message about a flag that no longer applies. So the scopes move to task_scope.go as one table, and both callers read it: rejectMisappliedTaskValues (unchanged behaviour) and dropOutOfScopeTaskValues (new). Adding a task-scoped flag is now one row, and it cannot be added to the guard while being forgotten in the reset. Each message is written out whole rather than composed from a flag name and a fragment, so zz-all-strings.golden shows a reviewer the exact sentence a user sees. That is the only golden change: "%s is image tasks only…" becomes the two concrete --target-size / --min-size lines. Tests: the four values are gone after a task change and the label column just answered survives; nothing is cleared when the task is UNCHANGED (a blanket wipe passes the first test and silently discards flags the user meant); every row rejects out-of-scope and names its own flag, with the counterexample task taken from push.SupportedCategoryIDs() rather than hand-picked; and for every supported task, clearing satisfies the guard — which is what makes the shared predicate worth sharing. Mutation-proved both ways: dropping the reset call reddens with all five values surviving; clearing unconditionally reddens the unchanged-task test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

LukasWodka · 2026-08-14T14:59:32Z

bugbot run

cursor

✅ Bugbot reviewed your changes and found no new issues!

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

^{Reviewed by Cursor Bugbot for commit 5e6184e. Configure here.}

LukasWodka · 2026-08-14T15:05:37Z

bugbot run

Bugbot, on my own previous commit, and it is the more interesting half of the bug. `dropOutOfScopeTaskValues` cleared everything the PICKED task doesn't use — which also cleared a flag that was misapplied on the command line, before any picking happened. So tracebloc data ingest ./d --task tabular_classification --time-column t then Enter on the pre-selected task, silently ignored --time-column and continued, while the identical invocation under --no-input exits 2. Guided mode quietly meaning something different from the flags it echoes is worse than the stale-value bug it replaced: the first version lost an answer the user could see, this one loses one they cannot. "Out of scope for the new task" and "left behind by the change" are different sets, and only the second is the reset's business. A flag nobody walked away from is the guard's to reject. So the reset now takes the task the user ARRIVED with and clears only what was in scope for that and is not in scope for what they chose. A run with no --task supplied is not a change either — there was no task to move away from — so every value stands or falls on the guard. My comment claiming an unchanged task clears nothing was false in exactly this case, which is the kind of comment worth deleting rather than correcting: it described the intent while the code did something else. Tests: the misapplied flag survives the guided flow and is rejected, both with --task supplied and without. The stale-value tests now start from a LEGAL state — every value in scope for the supplied task — because starting from an illegal one tests the guard, not the reset; a second case covers the image family, where the dropped value is a number and two neighbouring flags must survive. TestClearingAlwaysSatisfiesTheGuard is replaced by the stronger property it was reaching for: from any legal starting state, a change to ANY other supported task leaves a state the guard accepts — both sides built from the table, so a scope widened later is exercised without editing the test. Mutation-proved both ways: clearing everything out of scope reddens the two misapplied-flag tests; removing the reset reddens the two stale-value ones. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

LukasWodka · 2026-08-14T15:16:31Z

bugbot run

cursor

✅ Bugbot reviewed your changes and found no new issues!

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

^{Reviewed by Cursor Bugbot for commit 7afa9a2. Configure here.}

He asked what happens when the user changes task A -> B and the flag was invalid for both, and offered the weaker rule (`!inScope(to) && !inScope(from)` as the DROP condition) as an option. The stricter answer is already what the code does, and it is the right one: the flag was wrong when they typed it, and no answer they gave walks away from it, so it must be rejected exactly as --no-input rejects it. A re-pick is not blanket permission to drop. `v.inScope(from)` in the clear condition is what makes that true — but nothing tested it, so it was true by accident. Mutation-proved: relaxing to "clear anything out of scope for the picked task" reddens this and nothing else, which is precisely the case that was untested. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

LukasWodka · 2026-08-14T15:26:23Z

bugbot run

LukasWodka · 2026-08-14T15:28:04Z

bugbot run

Bugbot, and it is right — this is the third distinct way the same reset has swallowed a value, and the worst of them, because it loses TWO. Every `inScope` predicate answers from the registry, so an unknown id lands on the DEFAULT side of each one: `!SelfSupervisedText("tabular_clasification")` is true because the lookup misses, not because that task uses a label column. `dropValuesLeftBehindByATaskChange` treated any non-empty `from` as a real prior task, so tracebloc data ingest ./d --task tabular_clasification --label-column churned then picking masked_language_modeling read --label-column as "in scope before, out of scope now" and cleared it — leaving rejectMisappliedTaskValues nothing to object to. The typo is lost in the same breath, and that half is worth stating: the guided flow runs BEFORE the category gate (data_ingest_local.go:103 vs :165), and the picker has already overwritten Category with a valid id by then, so the gate never sees the typo either. The run proceeds as though the user typed neither flag. Under --no-input the identical command line exits 2 on the unrecognized task. Guided mode quietly meaning something other than the flags it echoes is the exact failure this helper was added to prevent. An unknown `from` is therefore the same case as no --task at all: nothing was walked away from, so clear nothing and let the guard speak. `push.IsKnown`, not `IsCLISupported` — a known-but-unsupported task is still a task the user declared, and the gate has its own message for that. Mutation-proved: dropping the `IsKnown` guard reddens TestGuided_ATypoedTaskIsNotATaskChange with "the reset cleared it on behalf of a task that does not exist", and nothing else. Anchor asserted rather than assumed. gofmt/go vet clean, full package green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

LukasWodka · 2026-08-14T15:40:00Z

bugbot run

cursor

✅ Bugbot reviewed your changes and found no new issues!

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

^{Reviewed by Cursor Bugbot for commit 69460af. Configure here.}

`misspell` is a golangci-lint gate here, and "tabular_clasification" is a word it knows how to correct — 4 hits across the helper's comment and the test, so my own previous commit went red on a gate the fix had nothing to do with. The test needs a `--task` the registry does not recognize; it does not need a MISSPELLED one. "tabular_classifier" is a plausible wrong name a user would actually type, spelled correctly, and exercises the identical path (`push.IsKnown` false → every `inScope` defaults to true). Renamed to `TestGuided_AnUnrecognizedTaskIsNotATaskChange` so the name describes what is being tested rather than one way of arriving at it, and the comment now says the spelling is deliberate so nobody reintroduces a typo for flavour. Re-proved after the rename: dropping the `IsKnown` guard still reddens it with "the reset cleared it on behalf of a task that does not exist". gofmt clean, `misspell -error ./internal` clean, package green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The comments and commit trail referenced #711, which does not exist in this repo — I carried the number over from an unrelated client PR, and the later review commits propagated it into three more comments. Filed cli#509 with the actual defect (a supplied path skips its question, so a dataset that moved dead-ends on a path the user was never asked for) and corrected every reference. No behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

LukasWodka · 2026-08-14T21:19:33Z

Non-TTY audit: no regression — verified structurally and empirically

The risk worth checking on a "always asks" change is that a path which used to run unattended now blocks on a prompt in CI, a script, or a pipe. It does not. Two independent checks:

1. The gate is untouched by this PR. interactive := !noInput && !outputJSON && isInteractiveTTY() (data_ingest_cmd.go:203) does not appear in the diff — only the comments around it moved. isInteractiveTTY requires both stdin and stdout to be terminals, and runInteractive is reached only under if a.Interactive && a.Prompter != nil (data_ingest_local.go:102). Every question this PR un-skips lives inside runInteractive, so none of them is reachable off a TTY. I also grepped for prompting added outside that block on the ingest path — there is none.

2. Ran the built binary. All four cases exit immediately; none waits for input (20s ceiling, stdin at /dev/null and again through a pipe):

invocation	result
fully flagged, `--dry-run`	rc=2 in 3s — real validation error
same, stdin piped	rc=2 in 1s
`data ingest` with no arguments at all (worst case)	rc=3 in 1s
path omitted	rc=3 in 1s

The no-argument case is the one that would hang if the gate were wrong, and it exits with a message that names the escape hatch: "pass it as an argument, or run on a terminal without --no-input for guided prompts."

make check is green on this branch (vet, full tests, fmt, file-budget, style, tool pins).

One real behaviour change, worth stating explicitly

It is not the non-TTY case, but it is adjacent and this PR does own it. Under a pty with every value already supplied:

$ script -q /dev/null tb data ingest ./data --name ds --task tabular_classification \
    --label-column label --intent train --dry-run
Step 1 of 4 · Do you want to ingest training or test data?   ← now stops here

Before this change that ran straight through; now it asks four questions and a confirm. That is exactly the PR's stated intent, and --no-input is a working escape hatch (same command with --no-input exits in 1s, no prompting). The group it affects is TTY-attached automation — a Makefile target, a wrapper script, or a tmux pane where stdin and stdout are still the terminal. Those are not covered by the non-TTY guard, by construction.

Not a blocker and I have not changed anything for it: the behaviour is deliberate, documented in the PR body, and the escape hatch is in --help. Flagging it only because --no-input is currently discoverable only from --help — the guided flow itself never mentions it. If a user is surprised by the new prompts, nothing on screen tells them how to turn them off. Worth a follow-up if anyone hits it; not worth touching a green PR for.

…e-fill (#513) defaultInOptions matches exactly, so a --label-column differing from the CSV header only in case failed the match and fell through to defaultLabelChoice -- the FIRST column when nothing is named "label". Enter then accepted that wrong column silently. Until #505 a supplied --label-column skipped the prompt entirely and its spelling was kept verbatim, so it never had to agree with the header. Now that the question is always asked, the mismatch became reachable. canonicalHeader resolves the supplied value to the header's own spelling before the guard. Resolving there rather than loosening defaultInOptions keeps the case-insensitivity where the options are user data -- defaultInOptions also guards --intent and --label-policy against fixed vocabularies, which should stay exact. A value matching nothing is returned unchanged, so a genuine typo is still caught rather than case-folded into a hit. Mutation-proved: reverting the canonicalHeader call reddens both new rows with "Income" -> "age" and "CHURNED" -> "age", the defect verbatim. Found by Bugbot on release-train promotion PR cli#511. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

LukasWodka self-assigned this Aug 14, 2026