Uh oh!
There was an error while loading. Please reload this page.
data commands: cleaner output — drop banner, honest empty state, hide internal tables - #370
Merged
Merged
Conversation
Remove the `tracebloc` + rule + subtitle banner block from the data commands (`data list`, `data delete`, `data ingest`). It's redundant noise — each command's Section/Para header already carries the context — and it hardcoded `tracebloc` regardless of how the user launched the CLI. For an empty `data list`, render the hint via invokedName() so it reads `tb data ingest` when the user launched via the tb alias (matching what they typed), falling back to `tracebloc` otherwise — the same mechanism `resources` already uses. Scope: data commands only. The shared ui.Printer.Banner method stays for now (still called by resources/doctor/cluster, being redesigned in parallel) and will be deleted once those land. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ListDatasets ran a bare `SELECT table_name … WHERE table_schema= 'training_test_datasets'` with no filter, so the ingestor's internal bookkeeping tables (tracebloc_ingest_runs / tracebloc_ingest_meta) leaked into `data list` as if they were user datasets and inflated the count. Filter them out at the shared ListDatasets layer so they never surface as datasets anywhere: `data list` shows only real datasets, and `data delete` can no longer target the ingestor's bookkeeping. Reserved names live next to IngestionDatabase, kept in sync with data-ingestors database.py (RUNS_TABLE / SALT_TABLE). The empty→nil listing contract is preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Follow-up to review on #370. Dropping the intro banner left three stale comments describing output that no longer prints: - data_list.go: --output-json note said "(the banner)" → header + listing - data_ingest_local.go: resolveLocalInput doc listed "the intro banner" → intro explainer - data_ingest_local.go: "Intro header: brand + …" → no brand now Comments only — no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
approved these changes
Jul 21, 2026
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka added a commit
that referenced
this pull request
Jul 21, 2026
Resolve internal/cli/data_list.go: keep the rich modality-grouped renderer (--all, per-dataset size/records/format/split, details JSON) and adopt #370's cleaner-output conventions on the shared surface — no banner, honest empty state via invokedName() + Para. Fold #370's reservedTables set into ListDatasetsDetailed's System-table detection so the ingest-run journal and ingest-meta store are hidden by default (alongside the existing data_id heuristic), consistent with how ListDatasets now filters them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Merged
LukasWodka added a commit
that referenced
this pull request
Jul 21, 2026
…375) * ui: remove the shared Banner method — no intro banner on any command Completes the banner removal started for the data commands (#370). The `tracebloc` + rule + subtitle block is gone from the last callers — `cluster info`, `resources`, `resources set` — and the shared `ui.Printer.Banner` method is deleted so nothing can reintroduce it. (`doctor` already dropped its banner in its own redesign.) Spacing: commands whose first line is a Section (`cluster info`) keep their leading blank from Section; the Stat/message-led paths (`resources` show, the `resources set` no-op / phantom-GPU notes) get an explicit Newline so every command still opens with exactly one blank line. Also dropped a now- redundant Newline in the `resources set` dry-run path (Section supplies it). Tests: drop the Banner unit-test call + its assertion; update the cluster-info test that asserted the "cluster diagnostics" subtitle; fix stale "banner" comments (incl. the ui package doc + a data-delete test). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(resources): drop the double blank before the set confirm hint (Bugbot #375) After removing the intro banner, flag-driven interactive `resources set` (no --yes/--dry-run) opened with TWO blank lines: a Newline() immediately before PromptHint, which already self-leads with a newline. The dry-run path dropped its redundant Newline() (Section supplies one) but the confirm path kept the stacked pair, so the command no longer opened with a single blank line. Remove the redundant Newline() — PromptHint's own leading newline gives the one blank the PR's spacing contract promises. Adds TestSet_ConfirmOpensWithSingleBlank (drives the flag confirm path via runSet) to pin it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * resources show: pin the single-blank opening with a test (Asad review, #375) The leading Newline() in runResourcesShow (before resolve, so a resolve-time redirect line also gets a blank) wasn't covered — every resources-show test drives renderResources directly, skipping it. Wire runResourcesShow to the resolveClusterTargetFn seam (matching the data commands) and add TestShow_OpensWithSingleBlank: it drives the outer function through a canned target and asserts it opens with exactly one blank — the mirror of TestSet_ConfirmOpensWithSingleBlank. Confirmed it fails if the Newline() is removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Jul 21, 2026
ContributorAuthor
/fr-pass — verified live on dev: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two focused cleanups to the data commands' output.
1. Drop the intro banner + honest empty-state hint
The
traceblocbanner block —— was printed at the top of every data command. It's redundant (each command's own
Section/Paraheader already carries the context) and it hardcodedtraceblocregardless of how the CLI was launched. Removed fromdata list,data delete,data ingest.For the empty
data list, the hint now usesinvokedName()so it echoes what the user actually typed:(
tbwhen launched via thetbalias,traceblocotherwise — the same helperresourcesalready uses.)2. Hide the ingestor's reserved bookkeeping tables
ListDatasetsran a bareSELECT table_name … WHERE table_schema='training_test_datasets'with no filter, so the ingestor's internal bookkeeping tables leaked intodata listas if they were user datasets and inflated the count. The ingestor (data-ingestors/tracebloc_ingestor/database.py) keeps two such tables and its own tests tag them"reserved":tracebloc_ingest_runs(RUNS_TABLE — run tracking / idempotency)tracebloc_ingest_meta(SALT_TABLE — pseudonymization salt)Now filtered out at the shared
ListDatasetslayer, so they never surface as datasets anywhere:data listshows only real datasets, anddata deletecan no longer target the ingestor's bookkeeping. Reserved names live next toIngestionDatabase(kept in sync with the ingestor). The empty→nil listing contract is preserved.Changed
data_list.go,data_delete.go,data_ingest_local.go— drop the banner; emptydata listhint viainvokedName().push/teardown.go— reserved-table name constants next toIngestionDatabase.push/list.go—filterReservedapplied inlistDatasetsWith.Scope
Banner removal is data commands only, by design. The shared
ui.Printer.Bannermethod is still called byresources/resources set/cluster/doctor(being redesigned in parallel); deleting the method + remaining call sites is a small follow-up once those land.Test plan
go build ./...,go vet,gofmt— clean.go test ./internal/push/... ./internal/cli/... ./internal/ui/...— green. New:TestFilterReserved+ a reserved-filtering case inTestListDatasetsWith;TestRenderDataList_Empty/_Itemsstill pass; theui_test.goBannerunit test is untouched (method stays).Datasets in test0721 (N); DB had 33 tables incl.tracebloc_ingest_runs,data listshowed 32 (the reserved table hidden).🤖 Generated with Claude Code
Note
Low Risk
CLI output and list filtering only; behavior change is hiding internal tables and softer headers, with tests locking the filter contract.
Overview
Data command UX drops the redundant
traceblocintro banner ondata list,data delete, anddata ingest(replaced with a leading newline where needed); ingest/delete keep their explanatory paragraphs. Emptydata listnow suggests ingest usinginvokedName()so the hint matches how the binary was launched (tbvstracebloc).Dataset listing filters ingestor-internal MySQL tables (
tracebloc_ingest_runs,tracebloc_ingest_meta) inListDatasetsviafilterReserved, with constants documented next toIngestionDatabase. That fixes inflated counts and prevents those names from appearing as deletable datasets (delete already resolves targets through the same list). Tests cover filtering and the empty→nilslice contract.Reviewed by Cursor Bugbot for commit 0ed3404. Bugbot is set up for automated code reviews on this repo. Configure here.