fix(summary): align CLI success/failure with the ingestor — inserted-based rate + full has_failures

internal/cli/coverage_test.go

@@ -99,7 +99,7 @@ func TestClassifyPushOutcome(t *testing.T) {
 		wantStat string
 		wantCode int
 	}{
-		{"clean", &submit.Result{Submit: resp, Watch: &submit.WatchResult{Outcome: submit.JobOutcomeSucceeded, Summary: &submit.Summary{TotalRecords: 10, InsertedRecords: 10}}}, nil, "succeeded", 0},
+		{"clean", &submit.Result{Submit: resp, Watch: &submit.WatchResult{Outcome: submit.JobOutcomeSucceeded, Summary: &submit.Summary{TotalRecords: 10, InsertedRecords: 10, APISentRecords: 10}}}, nil, "succeeded", 0},
 		{"partial", &submit.Result{Submit: resp, Watch: &submit.WatchResult{Outcome: submit.JobOutcomeSucceeded, Summary: &submit.Summary{TotalRecords: 10, InsertedRecords: 7, FailedRecords: 3}}}, nil, "completed_with_failures", 9},
 		{"failed", &submit.Result{Submit: resp, Watch: &submit.WatchResult{Outcome: submit.JobOutcomeFailed}}, nil, "failed", 9},
 		{"unknown", &submit.Result{Submit: resp, Watch: &submit.WatchResult{Outcome: submit.JobOutcomeUnknown}}, nil, "unknown", 9},

internal/cli/ingestion_run_test.go

@@ -25,7 +25,11 @@ import (
 func succeededResult() *submit.Result {
 	return &submit.Result{
 		Submit: &submit.SubmitResponse{Namespace: "tracebloc", JobName: "ingestor-x"},
-		Watch:  &submit.WatchResult{Outcome: submit.JobOutcomeSucceeded, Summary: &submit.Summary{TotalRecords: 2, InsertedRecords: 2}},
+		// A genuinely clean run: every stage counter equal (total == inserted
+		// == api_sent). APISentRecords must be set too — the ingestor-aligned
+		// HasFailures() treats api_sent < inserted as a partial, so omitting it
+		// (defaulting to 0) would misclassify this "succeeded" row as a failure.
+		Watch: &submit.WatchResult{Outcome: submit.JobOutcomeSucceeded, Summary: &submit.Summary{TotalRecords: 2, InsertedRecords: 2, APISentRecords: 2}},
 	}
 }
 

internal/submit/summary.go

@@ -69,26 +69,41 @@ type Summary struct {
 	FailedRecords int64
 }
 
-// HasFailures returns true if any failure-class counter is non-zero.
-// Used by the orchestrator to decide which exit code to return
-// (success: 0, ingest-with-failures: non-zero) and how to color
-// the rendered panel.
+// HasFailures returns true if any non-trivial failure occurred. It MIRRORS
+// the ingestor's IngestionSummary.has_failures EXACTLY (data-ingestors
+// ingestors/base.py) — DB insert short of total, API short of inserted, a
+// file-transfer or processing drop (skipped), or a hard failure — so the CLI's
+// exit code + staging-reclaim gate agree with the ingestor's own "completed
+// successfully" banner. The narrower prior version (only FailedRecords /
+// FileTransferFailures) reported success and reclaimed the staged source on a
+// run that silently SKIPPED rows or inserted fewer than total — silent data
+// loss that then deletes the user's only copy. Every counter this reads is
+// emitted unconditionally by the ingestor banner + parsed above, so the
+// inserted<total / api_sent<inserted clauses can't false-positive on a clean
+// run (where all counters are equal).
 func (s *Summary) HasFailures() bool {
 	if s == nil {
 		return false
 	}
-	return s.FileTransferFailures > 0 || s.FailedRecords > 0
+	return s.FailedRecords > 0 ||
+		s.FileTransferFailures > 0 ||
+		s.SkippedRecords > 0 ||
+		s.InsertedRecords < s.TotalRecords ||
+		s.APISentRecords < s.InsertedRecords
 }
 
-// SuccessRate returns a 0-100 percentage for the panel header.
-// Defined as ProcessedRecords / TotalRecords; returns 0 when
-// TotalRecords is 0 to avoid divide-by-zero in early-failure
-// banners.
+// SuccessRate returns a 0-100 percentage for the panel header. Defined as
+// InsertedRecords / TotalRecords — matching the ingestor's own banner
+// (reporting.py: inserted_records / total_records), since InsertedRecords (rows
+// that actually landed in MySQL) is the metric that matters for training, and
+// ProcessedRecords (passed validation) is a superset that OVERSTATED success
+// when rows validated but failed to insert. Returns 0 when TotalRecords is 0 to
+// avoid divide-by-zero in early-failure banners.
 func (s *Summary) SuccessRate() float64 {
 	if s == nil || s.TotalRecords == 0 {
 		return 0
 	}
-	return float64(s.ProcessedRecords) / float64(s.TotalRecords) * 100
+	return float64(s.InsertedRecords) / float64(s.TotalRecords) * 100
 }
 
 // ansiCodeRE matches the ANSI SGR (Select Graphic Rendition)
@@ -337,10 +352,21 @@ func RenderSummary(p *ui.Printer, s *Summary) {
 	headline := fmt.Sprintf("ingested %s of %s records (%.1f%%)",
 		commaSep(s.InsertedRecords), commaSep(s.TotalRecords), s.SuccessRate())
 	switch {
-	case s.HasFailures():
+	case s.FailedRecords > 0 || s.FileTransferFailures > 0:
+		// Hard failures: rows errored at DB insert or file transfer.
 		p.Errorf("Ingestion completed with failures — %s", headline)
-	case s.SkippedRecords > 0:
-		p.Warnf("Ingestion completed with skips — %s", headline)
+	case s.HasFailures():
+		// No hard failure, but not clean: rows skipped, or fewer inserted/
+		// synced than the ingestor saw. Exit-coded as not-clean (HasFailures),
+		// but colored distinctly from a hard failure. Word it by which soft
+		// shortfall actually occurred — "skips" only when rows were skipped;
+		// an insert/API shortfall with zero skips is a partial result, not a
+		// skip, and mislabeling it reads as a validator drop.
+		if s.SkippedRecords > 0 {
+			p.Warnf("Ingestion completed with skips — %s", headline)
+		} else {
+			p.Warnf("Ingestion completed partially — %s", headline)
+		}
 	default:
 		p.Successf("Ingestion complete — %s", headline)
 	}

internal/submit/summary_test.go

@@ -71,20 +71,24 @@ func TestSummaryParser_RealBannerEndToEnd(t *testing.T) {
 // that the orchestrator uses to choose between success exit code
 // (0) and ingest-failure exit code (9).
 func TestSummaryParser_HasFailures(t *testing.T) {
+	// Mirrors the ingestor's IngestionSummary.has_failures exactly.
 	cases := []struct {
 		name string
 		s    *Summary
 		want bool
 	}{
 		{"nil", nil, false},
-		{"all zero", &Summary{TotalRecords: 100, ProcessedRecords: 100}, false},
-		{"file transfer failures", &Summary{FileTransferFailures: 1}, true},
-		{"failed records", &Summary{FailedRecords: 1}, true},
-		{"both", &Summary{FileTransferFailures: 1, FailedRecords: 1}, true},
-		// Skipped records are NOT failures — they're rows that
-		// validators rejected. The customer wants to see the
-		// count but it doesn't change the exit code.
-		{"skipped is not failure", &Summary{SkippedRecords: 100}, false},
+		// A genuinely clean run: every counter equal, nothing skipped/failed.
+		{"clean", &Summary{TotalRecords: 100, ProcessedRecords: 100, InsertedRecords: 100, APISentRecords: 100}, false},
+		{"file transfer failures", &Summary{TotalRecords: 1, InsertedRecords: 1, APISentRecords: 1, FileTransferFailures: 1}, true},
+		{"failed records", &Summary{TotalRecords: 1, InsertedRecords: 1, APISentRecords: 1, FailedRecords: 1}, true},
+		// Skipped rows ARE a failure — a dropped row is silent data loss
+		// (#234); the ingestor counts it, so the CLI must too (was the bug).
+		{"skipped is a failure", &Summary{TotalRecords: 100, InsertedRecords: 100, APISentRecords: 100, SkippedRecords: 5}, true},
+		// Fewer rows in MySQL than the ingestor saw → partial run.
+		{"inserted < total", &Summary{TotalRecords: 100, ProcessedRecords: 100, InsertedRecords: 99, APISentRecords: 99}, true},
+		// Rows in MySQL but the central catalog got fewer.
+		{"api_sent < inserted", &Summary{TotalRecords: 100, InsertedRecords: 100, APISentRecords: 99}, true},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
@@ -99,15 +103,19 @@ func TestSummaryParser_HasFailures(t *testing.T) {
 // rendered panel's "Success rate: XX%" line. Divide-by-zero on
 // empty banner is the critical edge case.
 func TestSummaryParser_SuccessRate(t *testing.T) {
+	// Rate is INSERTED/total (matches the ingestor banner), not processed/total.
 	cases := []struct {
 		name string
 		s    *Summary
 		want float64
 	}{
 		{"nil", nil, 0},
 		{"empty banner", &Summary{}, 0},
-		{"100%", &Summary{TotalRecords: 100, ProcessedRecords: 100}, 100},
-		{"50%", &Summary{TotalRecords: 100, ProcessedRecords: 50}, 50},
+		{"100%", &Summary{TotalRecords: 100, ProcessedRecords: 100, InsertedRecords: 100}, 100},
+		{"50%", &Summary{TotalRecords: 100, InsertedRecords: 50}, 50},
+		// The overstatement the fix closes: all rows validated (processed=100)
+		// but only 70 landed in MySQL → 70%, not the old 100%.
+		{"processed overstates: inserted<processed", &Summary{TotalRecords: 100, ProcessedRecords: 100, InsertedRecords: 70}, 70},
 		{"all failed", &Summary{TotalRecords: 100}, 0},
 	}
 	for _, c := range cases {
@@ -233,9 +241,13 @@ func TestRenderSummary_OutcomeHeadline(t *testing.T) {
 		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"},
+		{"clean", &Summary{TotalRecords: 10, ProcessedRecords: 10, InsertedRecords: 10, APISentRecords: 10}, "complete —"},
+		{"skips", &Summary{TotalRecords: 10, ProcessedRecords: 8, InsertedRecords: 8, APISentRecords: 8, SkippedRecords: 2}, "skips"},
+		// Soft shortfall with ZERO skips: inserted < total (and api_sent <
+		// inserted) but nothing was skipped. Must NOT be labeled "skips" — it's
+		// a partial result, not a validator drop. (Bugbot #193.)
+		{"partial, no skips", &Summary{TotalRecords: 10, ProcessedRecords: 10, InsertedRecords: 9, APISentRecords: 8}, "partially"},
+		{"failures", &Summary{TotalRecords: 10, ProcessedRecords: 7, InsertedRecords: 7, APISentRecords: 7, FailedRecords: 3}, "failures"},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {