cli: emoji welcome + screen copy catalog (golden)#385
Merged
Conversation
Two changes from Lukas's review of the real `tb` output. 1. Emoji are welcome. Reverses the "no emoji" rule — the 👋 greeting, 💚 sign-off, 🚀 sent, and the traffic-light/⚠ status glyphs are intentional brand warmth. The guard's emoji check is removed (it now enforces only hardcoded brand colour + the 'workspace' term); STYLE.md documents emoji as welcome. 2. testdata/screens.golden — a copy catalog. TestScreensGolden renders every home-view state (+ data list) through the real renderers, colour off, into one committed file, so the exact wording + spacing can be reviewed WITHOUT deploying — read the file, or the diff on any PR that changes copy. The test fails on drift; regenerate with TB_UPDATE_GOLDEN=1 go test -run TestScreensGolden. (It already surfaces one inconsistency: `data list` empty-state says `tracebloc data ingest` while the rest says `tb` — a separate fix.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@BugBot run |
Lukas: the home-only catalog wasn't enough — needs every possible string a user
could see. The catalog now has three parts, all in testdata/screens.golden:
A. COMMANDS — walks NewRootCmd and captures the `--help` of every command
(24 of them): all Short/Long descriptions + flag help, exactly as printed.
B. SCREENS — the stateful views rendered plain (13): home in all 7 states,
data list empty/populated/--all, client-create review, offboard summaries.
C. MESSAGES — a harvested, deduped, sorted index of every user-facing string
literal in internal/cli, submit, push, doctor, cluster (501 of them). This
catches error paths + the flows not rendered above (ingest validation,
login, resources) so nothing a user can see is missing.
1497 lines total. Read the file (or the PR diff) to review any copy without
deploying; the test fails on drift, regenerate with TB_UPDATE_GOLDEN=1.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@BugBot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ 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 5ed4f88. Configure here.
Lukas: the wording was right but the layout wasn't — the catalog inserted blank lines the terminal doesn't show. Now each entry is `$ <command>` followed by the BYTE-EXACT output — line breaks, tabs, leading/trailing blanks all as printed: - Help is captured through the real `--help` flag path (SetArgs+Execute, exactly what the binary runs), not c.Help(). - Screens render through the real renderers (same code the binary runs). - No framing, no inserted whitespace: the only blank lines in the file are the outputs' own. `$ tracebloc --help` now starts its output on the very next line, and consecutive entries sit back-to-back like a real terminal session. The MESSAGE INDEX appendix stays (raw templates for search). Regenerate with TB_UPDATE_GOLDEN=1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@BugBot run |
Lukas: `tb data ingest` output wasn't findable. Two fixes:
- Add the ingest pre-flight `Review` screen (renderReview) to the SCREENS section.
- Replace the first-arg regex message-harvest with a go/ast walk that captures
EVERY string argument to a Printer method or error constructor — so labels that
aren't the first arg (Step("…","…") titles, MenuRow descriptions, Field values)
are now included. E.g. "Copy into your secure environment" (an ingest Step
label) was missing before; it's in the index now.
Honest limit: the assembled step-by-step RUN transcript of a live flow (ingest
steps + progress, login device flow) still isn't a single rendered screen — those
need the flow driven with a mocked cluster. But every STRING they print is now in
the MESSAGE INDEX, and the review screen is rendered. Can add driven flow
transcripts next if wanted.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@BugBot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 567b26c. Configure here.
…iles) Per Lukas's approved structure: replace the monolith screens.golden with testdata/golden/, one file per command, ordered by how often a user hits it. Each file leads with what you see when you RUN the command (byte-exact, every state/path), and ends with its --help. This commit lands the generator + the flagship files: - 00-home.golden — tb / tracebloc, all 7 states + --help - 02-data-list.golden — empty / populated / --all + --help - zz-all-strings.golden — every user-facing string (AST index), completeness backstop Adding a command file is now one entry in the generator. Coming next: 01-ingest (driven run), 03-data-delete, 04-resources, 05-doctor, 06-delete, 07-login, the rest, and the installer folder in the client repo. (The catalog already caught drift: the root --help still says "your client" / "the cluster" instead of "secure environment" — a copy fix to do separately.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds a golden file for every remaining command, in the order a user hits
them: 01-data-ingest, 03-data-delete, 04-resources, 05-doctor, 06-delete,
07-login, 08-client, 09-cluster, 10-version. Each leads with the byte-exact
screens we can render deterministically (ingest review, doctor rollup via the
real summarizeDoctor/renderHealth/doctorVerdict, delete + client review) and
ends with the command's --help. Flow-only copy (ingest steps, login device
flow, confirmations, and the launcher-dependent failure remedies) can't be a
stable screen, so it stays in the zz-all-strings backstop, which each file
points to.
Strengthens the backstop harvest to also catch fmt.Sprintf templates and the
text/remedy fields of healthLine{} / doctor.Result{} literals — user-facing
copy that never passes through a Printer call and so was missing before
(417 -> 556 strings; e.g. every "Not connected — …" / "Not ready — …" line).
Fixes the build: the prior commit left internal/cli/screens_golden_test.go in
the tree alongside copy_catalog_test.go, so harvestMessages was defined twice
(the deletion never got staged). This commit stages the deletion.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5656193. Configure here.
01-data-ingest now shows the full guided flow, not just the review. A
catalog double for the prompter seam prints each question the way the
terminal does ("? <question> <answer>") while the REAL runInteractive
drives it, so the transcript is every prompt in order — intro, the core
questions, the family sniff/echo, the task picker, the task-specific
questions, the review, and the confirm. Driven for two families (tabular +
image) so the task-specific questions (schema; image resolution) are
visible. The temp data dir is normalised to ~/datasets/hospital.
Also harvests the prompter labels + help text (Input/Select/Confirm) into
the backstop — prompt copy that never passes through a Printer call and so
was missing before (556 -> 589 strings; e.g. "which split this data is",
"How many keypoints", "Override the column types", "bucket bins the
target"). This covers the prompt copy for flows not driven as a screen
(client create, delete).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
Reworks `tb data ingest`'s interactive setup per the approved copy pass: - Intro trimmed to one line + the specific data-prep docs link; drops the "Let's set up… / Press Enter…" preamble. - Each question is now a dominant "Step N of 5 · <question>" header with any supporting line beneath it and an answer-only prompt (surveyPrompter gains a `bare` mode; the CLI owns the question text via the new PromptStep). - Plainer wording: "Do you want to ingest training or test data?", "Please name the dataset.", "Where is your data?" (+ per-modality file/folder examples), "What kind of machine learning task is this data for?" (airy list, task IDs), "Which column holds the label?" (+ examples), and a schema step reframed to "Column types — infer or override" instead of the cryptic blank prompt. - The task picker lists task IDs + blurbs (the blurb carries what the old gloss said); DisplayName/Gloss metadata is retained (allowlisted, still tested). - The invalid-table-name error drops the MySQL/PVC/path-traversal internals for a one-line "use letters, digits, underscores…". Tests + the copy catalog (01-data-ingest.golden) updated to the new wording. Execution/summary redesign (clean 3 steps, single summary) follows separately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
Applies one consistent spacing rule to the whole guided flow (STYLE.md
"Guided-prompt spacing"): header → blank → [supporting text → blank] → `?`
prompt, with the answer-belongs-to-it result (the sniff echo) attached to the
answer. Every step, the task-specific refinements, and the confirm now sit the
same distance below their question — tabular and image alike.
- The confirm renders as its own "Proceed with the ingest?" header + a bare
"? Yes" (surveyPrompter.Confirm honours bare), matching the step questions.
- Intro copy: "Ingest datasets to your secure environment." / "For help: …".
Regenerated 01-data-ingest.golden; the schema step ("Column types") now also
carries the blank-before-prompt for consistency.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
01-data-ingest now drives one task per family — tabular, image, AND text — so the text path (and the full text task list: classification, masked/causal LM, seq2seq, token/sentence-pair classification, embeddings) is reviewable. Each flow now shows a self-consistent example path (~/data/patients, ~/data/xray, ~/data/reviews) instead of a shared placeholder. Self-supervised text (which skips the label step) stays in the backstop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
The catalog showed only the guided Q&A — the execution (Check → Copy → Validate + the summary) was missing, which is what a user sees on every real run. Now 01-data-ingest also renders the run: the three step headers, the "Local dataset" / "Ingest settings" blocks (printLocalSummary), and the final summary + "What's next" (submit.RenderSummary) — the real renderers, so it's drift-caught. Rendering the CLI's OWN screens excludes the raw ingestor stream (MySQL waits, the 📊 banner, the doubled-word validator lines) — that's the engine's stdout, not CLI copy — so the catalog shows the clean view. Adds a coverage guard (mustRender): each command's primary path must appear as a rendered screen, so a dropped/half-rendered flow fails the test instead of silently vanishing (which is how the execution went missing in the first place). Every user-facing string is already guaranteed by the zz-all-strings AST harvest; this guards the SCREENS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
Two execution-copy fixes from the redesign review: - Ingestion summary is lean: the clean-run headline already says "X of Y records (N%)", so the summary drops "sent to API", the zero-value failure counters, and the redundant success-rate line. Everything below total/inserted is now a SHORTFALL, shown only when it happened — incl. "sent to API" gated on api_sent < inserted (the one failure mode the headline can't show), so no silent data-loss signal is hidden. - The step-1 "Ingest settings" block is suppressed on the guided path: the pre-confirm Review already showed name/task/intent/label, so repeating them was pure duplication. The flag-only path (no Review) still shows it once. Catalog regenerated; the run now reads clean. Step-format unification (the run's "Step N/3" vs the questionnaire's "Step N of 5 ·") is deferred — Step mirrors the installer's bash step(), so it's a coordinated CLI+installer change, not a CLI-only tweak. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b900d52. Configure here.
Three verified bugbot findings on PR #385: - Blank overwrite confirm (High): surveyPrompter.Confirm applied `bare` via message(), so the label was dropped. The overwrite-replace confirm in existingTableAction fires during the cluster phase with no header of its own, rendering a label-less `? (y/N)` before a destructive replace. Confirm now always keeps its label (matching its documented contract); drop the now-redundant Section header before "Proceed with the ingest?". - Settings hidden without Review (Medium): printLocalSummary gated the "Ingest settings" block on `Prompter == nil`, but a fully flagged run on a TTY sets a prompter yet prompts nothing, so it skipped the Review AND dropped settings that non-interactive runs still show. Gate on a new ReviewShown flag (set only when the Review actually renders) instead. - PromptStep omitted from string harvest (Medium): harvestMessages missed the new PromptStep method, so copy that only appears there (e.g. "What kind of machine learning task is this data for?") never landed in zz-all-strings.golden and the completeness backstop couldn't catch drift. Regenerated the golden catalog; catalogPrompter.Confirm now echoes its label to match the non-bare surveyPrompter. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses the remaining bugbot finding (#3, "Step total wrong without label"): self-supervised text (MLM/CLM) has no label column, so the guided flow showed "Step 4 of 5" and then jumped straight to Review with no fifth step. Root cause: the label column was numbered as a universal "step 5 of 5", but it isn't universal — it's task-specific, exactly like keypoints (image) or schema (tabular), none of which are numbered. Fix at that level: the four core steps (intent → name → path → task) are the only numbered ones ("of 4"), and the label joins the other task-dependent inputs as an unnumbered Section header. The count is now honest for every task, including the ones that have no label. Regenerated the golden catalog + updated the catalog's required-copy assertions and screen description to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
approved these changes
Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to the merged style enforcement (#368), from Lukas's review of the real
tboutput. Two things:1. Emoji are welcome
Reverses the "no emoji" rule — the
👋greeting,💚sign-off,🚀sent, and the traffic-light /⚠status glyphs are intentional brand warmth. The guard's emoji check is removed (it now enforces only hardcoded brand colour + theworkspaceterm); STYLE.md documents emoji as welcome.2. A screen copy catalog —
internal/cli/testdata/screens.goldenThe recurring pain: you can't see the CLI's wording + spacing without deploying. Now
TestScreensGoldenrenders every home-view state (Online / running / starting / offline / no-env / not-signed-in) +data listthrough the real renderers (colour off), into one committed file. Read that file — or the diff on any PR that changes copy — instead of deploying. The test fails on drift; regenerate with:It already earns its keep: the catalog shows the
data listempty-state saystracebloc data ingestwhile everything else saystb— an inconsistency to fix separately.Test plan
TB_UPDATE_GOLDEN=1regenerates; the plaingo testverifies (drift guard).go build ./...,go test, gofmt, staticcheck, deadcode, file-budget, and the style guard all clean.🤖 Generated with Claude Code
Note
Low Risk
Mostly user-facing CLI copy, golden tests, and style docs; ingest logic changes are presentation and deduplication, not cluster or auth behaviour.
Overview
Style & guard: Emoji are now documented as intentional brand warmth;
check-style.shno longer bans status emoji (only hardcoded brand colours andworkspace). Guided-prompt spacing is spelled out inSTYLE.md.tb data ingestUX: The questionnaire usesStep N of 4 · …headers via newPromptStep,baresurvey prompts so?lines show answers only, refreshed copy, task IDs in the picker (not display names), andReviewShownso the pre-confirm Review suppresses duplicate Ingest settings inprintLocalSummary.Copy catalog:
TestCopyCatalogwrites per-command golden files undertestdata/golden/(home states, ingest transcripts,--help, pluszz-all-strings.goldenfrom AST harvest) with drift checks andTB_UPDATE_GOLDEN=1regeneration.Other copy/output: Shorter ingest intro; cleaner table-name validation message; ingestion summary hides zero shortfall fields; home/catalog goldens include 👋/💚 welcome copy.
Reviewed by Cursor Bugbot for commit 6bf1328. Bugbot is set up for automated code reviews on this repo. Configure here.