Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions internal/cli/dataset.go
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,7 @@ func runDatasetPush(ctx context.Context, out, errOut io.Writer, a runDatasetPush
ImageDigest: a.ImageDigest,
Detach: a.Detach,
Out: out,
Printer: a.Printer,
})
if err != nil {
switch {
Expand Down
17 changes: 14 additions & 3 deletions internal/submit/submit.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"io"

"k8s.io/client-go/kubernetes"

"github.com/tracebloc/cli/internal/ui"
)

// Options bundles every dependency Run needs. The CLI builds one
Expand Down Expand Up @@ -47,9 +49,14 @@ type Options struct {

// Out is the customer-facing log stream. Submit writes the
// 201 announcement here, then either streams the Pod's logs
// to it (live watch) or prints the Job name (detach). The
// rendered summary panel also goes here.
// to it (live watch) or prints the Job name (detach).
Out io.Writer

// Printer renders the final ingestion summary (RenderSummary).
// nil is fine — Run falls back to ui.New(Out), so callers that
// don't thread the --plain decision still get a sensible
// (auto-detected) rendering.
Printer *ui.Printer
}

// Result is what Run reports back to the CLI orchestrator.
Expand Down Expand Up @@ -162,8 +169,12 @@ func Run(ctx context.Context, opts Options) (*Result, error) {
// Both Succeeded and Failed paths print it — on Failed, the
// banner tells the customer what got partially through.
if wr.Summary != nil {
p := opts.Printer
if p == nil {
p = ui.New(opts.Out)
}
_, _ = fmt.Fprintln(opts.Out)
_, _ = fmt.Fprint(opts.Out, RenderPanel(wr.Summary))
RenderSummary(p, wr.Summary)
}

return &Result{Submit: resp, Watch: wr}, nil
Expand Down
69 changes: 36 additions & 33 deletions internal/submit/summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"regexp"
"strconv"
"strings"

"github.com/tracebloc/cli/internal/ui"
)

// Summary is the parsed contents of the ingestor's 📊 INGESTION
Expand Down Expand Up @@ -318,45 +320,46 @@ func (p *SummaryParser) Result() *Summary {
return p.summary
}

// RenderPanel returns a multi-line, customer-facing rendering of
// the Summary for display in the orchestrator's success/failure
// message. Format:
//
// ┌─ Ingestion summary ──────────────────────────┐
// │ Ingestor ID: <id> │
// │ Total records: 1,234 │
// │ Inserted: 1,200 │
// │ Skipped: 4 │
// │ File transfer failures: 0 │
// │ DB-insert failures: 30 │
// │ Success rate: 97.2% │
// └──────────────────────────────────────────────┘
// RenderSummary prints the installer-style ingestion summary through
// p: an outcome-colored headline (green when clean, yellow on skips,
// red on failures), the per-stage counts as Section/Field rows, and a
// short "what's next". It replaces the old box-drawing panel —
// Section/Field is plain-ASCII friendly, so no Unicode-box fallback is
// needed (that was the v0.2 TODO).
//
// Uses box-drawing characters for visual structure. Plain ASCII
// fallback could be added in v0.2 for terminals that don't render
// Unicode (rare on modern OS X/Linux/Windows-Terminal).
func RenderPanel(s *Summary) string {
// No-op on a nil summary: an early failure (OOM before the ingestor
// printed its banner) produces no Summary, so the orchestrator can
// call this unconditionally.
func RenderSummary(p *ui.Printer, s *Summary) {
if s == nil {
return ""
return
}
const labelWidth = 26
var b strings.Builder
b.WriteString("┌─ Ingestion summary ──────────────────────────┐\n")
row := func(label, value string) {
fmt.Fprintf(&b, "│ %-*s %s\n", labelWidth, label, value)
headline := fmt.Sprintf("ingested %s of %s records (%.1f%%)",
commaSep(s.InsertedRecords), commaSep(s.TotalRecords), s.SuccessRate())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Headline percent mismatches ingested counts

Medium Severity

The new headline reports ingested using InsertedRecords and TotalRecords, but the parenthetical rate comes from SuccessRate(), which is ProcessedRecords / TotalRecords. When those counters differ—or ProcessedRecords is unset—the shown percentage no longer matches the ingested fraction (e.g. 8 of 10 with 0.0%).

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit be91b60. Configure here.

switch {
case s.HasFailures():
p.Errorf("Ingestion completed with failures — %s", headline)
case s.SkippedRecords > 0:
p.Warnf("Ingestion completed with skips — %s", headline)
default:
p.Successf("Ingestion complete — %s", headline)
}

p.Section("Ingestion summary")
if s.IngestorID != "" {
row("Ingestor ID:", s.IngestorID)
p.Field("ingestor ID", s.IngestorID)
}
row("Total records:", commaSep(s.TotalRecords))
row("Inserted:", commaSep(s.InsertedRecords))
row("Sent to API:", commaSep(s.APISentRecords))
row("Skipped:", commaSep(s.SkippedRecords))
row("File transfer failures:", commaSep(s.FileTransferFailures))
row("DB-insert failures:", commaSep(s.FailedRecords))
row("Success rate:", fmt.Sprintf("%.1f%%", s.SuccessRate()))
b.WriteString("└──────────────────────────────────────────────┘\n")
return b.String()
p.Field("total records", commaSep(s.TotalRecords))
p.Field("inserted", commaSep(s.InsertedRecords))
p.Field("sent to API", commaSep(s.APISentRecords))
p.Field("skipped", commaSep(s.SkippedRecords))
p.Field("file failures", commaSep(s.FileTransferFailures))
p.Field("DB failures", commaSep(s.FailedRecords))
p.Field("success rate", fmt.Sprintf("%.1f%%", s.SuccessRate()))

p.Section("What's next")
p.Infof("View it in the dashboard: https://ai.tracebloc.io")
p.Hintf("The table is staged and ready for training jobs.")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Training hint shown after failures

Low Severity

RenderSummary always prints the “What's next” hint that the table is ready for training jobs, including when HasFailures() drove a failure headline. That message can contradict a run that reported DB or file transfer failures.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit be91b60. Configure here.

}

// commaSep formats an int64 with thousands-separator commas to
Expand Down
58 changes: 44 additions & 14 deletions internal/submit/summary_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package submit

import (
"bytes"
"strings"
"testing"

"github.com/tracebloc/cli/internal/ui"
)

// realIngestorBanner mirrors what
Expand Down Expand Up @@ -182,10 +185,10 @@ func TestStripANSI(t *testing.T) {
}
}

// TestRenderPanel_BasicShape: the panel rendering is what the
// customer sees on success; pin a few key lines so a refactor
// breaks the test rather than silently producing weird output.
func TestRenderPanel_BasicShape(t *testing.T) {
// TestRenderSummary_BasicShape pins the key facts the customer sees;
// a refactor that drops one breaks the test rather than silently
// producing weird output. Color off so we assert plain text.
func TestRenderSummary_BasicShape(t *testing.T) {
s := &Summary{
IngestorID: "run-abc",
TotalRecords: 1234567,
Expand All @@ -195,26 +198,53 @@ func TestRenderPanel_BasicShape(t *testing.T) {
FileTransferFailures: 30,
FailedRecords: 5,
}
got := RenderPanel(s)
var buf bytes.Buffer
RenderSummary(ui.New(&buf, ui.WithColor(false)), s)
got := buf.String()
for _, want := range []string{
"Ingestion summary",
"run-abc",
"1,234,567", // commaSep formatting
"1,200,000", // commaSep formatting
"30", // file transfer failures
"DB-insert failures:",
"1,200,000",
"30", // file failures
} {
if !strings.Contains(got, want) {
t.Errorf("RenderPanel missing %q in:\n%s", want, got)
t.Errorf("RenderSummary missing %q in:\n%s", want, got)
}
}
}

// TestRenderPanel_Nil: nil summary returns empty string so the
// orchestrator can blind-print without a guard.
func TestRenderPanel_Nil(t *testing.T) {
if got := RenderPanel(nil); got != "" {
t.Errorf("RenderPanel(nil) = %q, want empty", got)
// TestRenderSummary_Nil: a nil summary writes nothing, so the
// orchestrator can call it unconditionally.
func TestRenderSummary_Nil(t *testing.T) {
var buf bytes.Buffer
RenderSummary(ui.New(&buf, ui.WithColor(false)), nil)
if buf.Len() != 0 {
t.Errorf("RenderSummary(nil) wrote %q, want nothing", buf.String())
}
}

// TestRenderSummary_OutcomeHeadline: the headline reflects the outcome
// derived from the counts — clean / skips / failures. Table-driven,
// one sub-test per row via t.Run.
func TestRenderSummary_OutcomeHeadline(t *testing.T) {
cases := []struct {
name string
s *Summary
want string
}{
{"clean", &Summary{TotalRecords: 10, ProcessedRecords: 10, InsertedRecords: 10}, "complete —"},
{"skips", &Summary{TotalRecords: 10, InsertedRecords: 8, SkippedRecords: 2}, "skips"},
{"failures", &Summary{TotalRecords: 10, InsertedRecords: 7, FailedRecords: 3}, "failures"},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
var buf bytes.Buffer
RenderSummary(ui.New(&buf, ui.WithColor(false)), c.s)
if !strings.Contains(buf.String(), c.want) {
t.Errorf("headline missing %q in:\n%s", c.want, buf.String())
}
})
}
}

Expand Down
Loading