Output polish: nine paper cuts across formatters and CLI

ROADMAP.md

@@ -177,8 +177,8 @@ Removed from the old "already correct" list — disproven by this review:
 - Exits 0/1 only (`cmd/ci-snitch/main.go:36-40`). A `--fail-on regression|failure-rate>N` mode makes ci-snitch usable as a CI gate; pairs with F7.
 - **Files:** `cmd/ci-snitch/`, docs
 
-### U7. Output polish batch [S total]
-- `fmtDur` renders 2.5h as "150m" (`internal/output/helpers.go:49-60`); `|` unescaped in markdown/LLM tables (`markdown.go:38`, `llm.go:126`); `"diagnostics": null` in JSON when empty (`internal/analyze/engine.go:23`) — emit `[]`; `Diagnostic.String()` drops the wrapped `Err` (`internal/diag/diag.go:37-42`); `compactResult` comment claims it drops outliers but filters only changepoints (`llm.go:240-243`); `-q` quiet mode; flag-value completion for `--format`; reject `--since 0d`/future dates before fetching.
+### U7. Output polish batch [S total] ✅ done
+- Shipped 2026-07-15: hours render as "2h30m"; `|` escaped in markdown/LLM table names; JSON emits `"diagnostics": []`/`"findings": []` instead of null (fixed at the formatter boundary); `Diagnostic.String()` shows the wrapped cause; percentile displays floor (no more "slower than 100% of runs"); `-q/--quiet` silences stderr entirely; `--format` shell completion; `--since 0d`/future dates rejected before any API call; `compactResult` comment matches the code.
 
 ---
 

cmd/ci-snitch/analyze.go

@@ -29,6 +29,7 @@ func newAnalyzeCmd() *cobra.Command {
 		noCache         bool
 		includeFailures bool
 		verbose         bool
+		quiet           bool
 	)
 
 	cmd := &cobra.Command{
@@ -79,6 +80,9 @@ If no repository is specified, detects the GitHub remote from the current direct
 			}
 
 			prog := output.NewProgress()
+			if quiet {
+				prog = output.NewProgressQuiet()
+			}
 			prog.Log("Snitching on %s", repo)
 
 			// Open store
@@ -123,7 +127,9 @@ If no repository is specified, detects the GitHub remote from the current direct
 			}
 
 			// Blank line before output
-			_, _ = fmt.Fprintln(os.Stderr)
+			if !quiet {
+				_, _ = fmt.Fprintln(os.Stderr)
+			}
 
 			// Output
 			formatStart := time.Now()
@@ -144,6 +150,10 @@ If no repository is specified, detects the GitHub remote from the current direct
 	cmd.Flags().BoolVar(&noCache, "no-cache", false, "bypass local cache, fetch fresh data")
 	cmd.Flags().BoolVar(&includeFailures, "include-failures", false, "include failed runs in analysis")
 	cmd.Flags().BoolVarP(&verbose, "verbose", "v", false, "verbose output (show fetch details)")
+	cmd.Flags().BoolVarP(&quiet, "quiet", "q", false, "suppress progress and diagnostic output on stderr")
+
+	_ = cmd.RegisterFlagCompletionFunc("format", cobra.FixedCompletions(
+		[]string{"table", "json", "markdown", "llm"}, cobra.ShellCompDirectiveNoFileComp))
 
 	return cmd
 }
@@ -172,7 +182,7 @@ var sinceRe = regexp.MustCompile(`^(\d+)(d|w|mo)$`)
 func parseSinceFrom(s string, now time.Time) (time.Time, error) {
 	// Try absolute date first
 	if t, err := time.Parse("2006-01-02", s); err == nil {
-		return t, nil
+		return t, validateSincePast(t, now)
 	}
 
 	m := sinceRe.FindStringSubmatch(s)
@@ -181,13 +191,25 @@ func parseSinceFrom(s string, now time.Time) (time.Time, error) {
 	}
 
 	n, _ := strconv.Atoi(m[1]) // regex guarantees digits
+	var t time.Time
 	switch m[2] {
 	case "d":
-		return now.AddDate(0, 0, -n), nil
+		t = now.AddDate(0, 0, -n)
 	case "w":
-		return now.AddDate(0, 0, -n*7), nil
+		t = now.AddDate(0, 0, -n*7)
 	case "mo":
-		return now.AddDate(0, -n, 0), nil
+		t = now.AddDate(0, -n, 0)
+	default:
+		return time.Time{}, fmt.Errorf("unrecognized format %q", s)
+	}
+	return t, validateSincePast(t, now)
+}
+
+// validateSincePast rejects windows that cannot contain any runs ("0d",
+// future dates) before they burn an API round trip.
+func validateSincePast(t, now time.Time) error {
+	if !t.Before(now) {
+		return fmt.Errorf("--since must be in the past, got %s", t.Format("2006-01-02"))
 	}
-	return time.Time{}, fmt.Errorf("unrecognized format %q", s)
+	return nil
 }

cmd/ci-snitch/analyze_test.go

@@ -31,6 +31,8 @@ func TestParseSinceFrom(t *testing.T) {
 		{name: "months", input: "3mo", want: now.AddDate(0, -3, 0)},
 		{name: "single day", input: "1d", want: now.AddDate(0, 0, -1)},
 		{name: "single month", input: "1mo", want: now.AddDate(0, -1, 0)},
+		{name: "zero days", input: "0d", wantErr: "in the past"},
+		{name: "future date", input: "2026-06-01", wantErr: "in the past"}, // "now" in this test is 2026-04-15
 		{name: "too short", input: "x", wantErr: "unrecognized format"},
 		{name: "empty", input: "", wantErr: "unrecognized format"},
 		{name: "bad suffix", input: "5y", wantErr: "unrecognized format"},

internal/analyze/outliers.go

@@ -97,7 +97,7 @@ func (o OutlierAnalyzer) Analyze(_ context.Context, ac *AnalysisContext) ([]Find
 				Severity: severityFromPercentile(out.Percentile),
 				Title:    fmt.Sprintf("Slow run in %q", wfName),
 				Description: fmt.Sprintf("Run took %s (p%.0f — slower than %.0f%% of runs)",
-					d.Duration().Round(time.Second), out.Percentile, out.Percentile),
+					d.Duration().Round(time.Second), math.Floor(out.Percentile), math.Floor(out.Percentile)),
 				Detail: OutlierDetail{
 					RunID:        d.Run.ID,
 					CommitSHA:    d.Run.HeadSHA,
@@ -149,7 +149,7 @@ func (o OutlierAnalyzer) Analyze(_ context.Context, ac *AnalysisContext) ([]Find
 				Severity: severityFromPercentile(out.Percentile),
 				Title:    fmt.Sprintf("Slow job %q in %q", job.Name, wfName),
 				Description: fmt.Sprintf("Job took %s (p%.0f — slower than %.0f%% of runs)",
-					job.Duration().Round(time.Second), out.Percentile, out.Percentile),
+					job.Duration().Round(time.Second), math.Floor(out.Percentile), math.Floor(out.Percentile)),
 				Detail: OutlierDetail{
 					RunID:        d.Run.ID,
 					CommitSHA:    d.Run.HeadSHA,

internal/diag/diag.go

@@ -1,7 +1,10 @@
 // Package diag provides a unified diagnostic type for non-fatal issues.
 package diag
 
-import "fmt"
+import (
+	"fmt"
+	"strings"
+)
 
 // Severity indicates how important a diagnostic is.
 type Severity string
@@ -35,10 +38,16 @@ type Diagnostic struct {
 }
 
 func (d Diagnostic) String() string {
+	msg := d.Message
+	// The wrapped cause is the actionable part of a failure — show it unless
+	// the message already embeds it.
+	if d.Err != nil && !strings.Contains(msg, d.Err.Error()) {
+		msg += ": " + d.Err.Error()
+	}
 	if d.Scope != "" {
-		return fmt.Sprintf("[%s] %s: %s", d.Severity, d.Scope, d.Message)
+		return fmt.Sprintf("[%s] %s: %s", d.Severity, d.Scope, msg)
 	}
-	return fmt.Sprintf("[%s] %s", d.Severity, d.Message)
+	return fmt.Sprintf("[%s] %s", d.Severity, msg)
 }
 
 // New creates a Diagnostic with the given severity, kind, scope, and message.

internal/diag/diag_test.go

@@ -0,0 +1,23 @@
+package diag
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestDiagnosticString(t *testing.T) {
+	d := New(Warn, KindNetwork, "run-5", "failed to fetch")
+	assert.Equal(t, "[warn] run-5: failed to fetch", d.String())
+
+	noScope := New(Info, KindPreprocess, "", "deduplicated 3 runs")
+	assert.Equal(t, "[info] deduplicated 3 runs", noScope.String())
+}
+
+func TestDiagnosticString_IncludesCause(t *testing.T) {
+	// The wrapped error is the actionable part of a cache/network failure;
+	// dropping it left "failed to cache 5 runs" with no why.
+	d := Errorf(KindCache, "CI", errors.New("disk full"), "failed to cache %d runs", 5)
+	assert.Contains(t, d.String(), "disk full", "the cause must be visible")
+}

internal/output/formatter_test.go

@@ -302,6 +302,76 @@ func TestLLMFormatter_RawOutputWritesJSONFile(t *testing.T) {
 		"the briefing should point the LLM at the raw file")
 }
 
+func TestFmtDur_HoursRenderAsHours(t *testing.T) {
+	assert.Equal(t, "2h30m", fmtDur(dur(150*time.Minute)), "a 2.5h p95 rendered as '150m'")
+	assert.Equal(t, "1h0m", fmtDur(dur(time.Hour)))
+	assert.Equal(t, "5m30s", fmtDur(dur(5*time.Minute+30*time.Second)))
+	assert.Equal(t, "45s", fmtDur(dur(45*time.Second)))
+}
+
+func TestJSONFormatter_EmptyDiagnosticsIsArray(t *testing.T) {
+	var buf bytes.Buffer
+	err := JSONFormatter{}.Format(&buf, &analyze.AnalysisResult{})
+	require.NoError(t, err)
+	assert.Contains(t, buf.String(), `"diagnostics": []`,
+		`null breaks jq '.diagnostics[]' consumers`)
+}
+
+func TestMarkdownFormatter_EscapesPipesInNames(t *testing.T) {
+	result := &analyze.AnalysisResult{
+		Findings: []analyze.Finding{{
+			Type: analyze.TypeOutlier, Severity: analyze.SeverityWarning,
+			Detail: analyze.OutlierGroupDetail{
+				WorkflowName: "build|test", JobName: "job|name", Count: 2,
+				WorstDuration: dur(10 * time.Minute), WorstPercentile: 97,
+				WorstCommitSHA: "aabbccdd", MaxSeverity: analyze.SeverityWarning,
+			},
+		}},
+	}
+	var buf bytes.Buffer
+	require.NoError(t, MarkdownFormatter{}.Format(&buf, result))
+	assert.Contains(t, buf.String(), `build\|test`,
+		"an unescaped pipe in a name breaks the markdown table")
+	assert.NotContains(t, buf.String(), "| build|test |")
+}
+
+func TestMarkdownFormatter_PercentileDisplayFloors(t *testing.T) {
+	result := &analyze.AnalysisResult{
+		Findings: []analyze.Finding{{
+			Type: analyze.TypeOutlier, Severity: analyze.SeverityWarning,
+			Detail: analyze.OutlierGroupDetail{
+				WorkflowName: "CI", JobName: "build", Count: 1,
+				WorstDuration: dur(10 * time.Minute), WorstPercentile: 99.8,
+				WorstCommitSHA: "aabbccdd", MaxSeverity: analyze.SeverityWarning,
+			},
+		}},
+	}
+	var buf bytes.Buffer
+	require.NoError(t, MarkdownFormatter{}.Format(&buf, result))
+	assert.Contains(t, buf.String(), "p99")
+	assert.NotContains(t, buf.String(), "p100",
+		`"slower than 100% of runs" includes the run itself`)
+}
+
+func TestProgress_QuietDiscardsEverything(t *testing.T) {
+	r, w, err := os.Pipe()
+	require.NoError(t, err)
+	old := os.Stderr
+	os.Stderr = w
+	defer func() { os.Stderr = old }()
+
+	p := NewProgressQuiet()
+	p.Status("working")
+	p.Log("noise")
+	p.Done()
+	require.NoError(t, w.Close())
+	os.Stderr = old
+
+	out, err := io.ReadAll(r)
+	require.NoError(t, err)
+	assert.Empty(t, string(out), "quiet mode must write nothing to stderr")
+}
+
 func TestProgress_NonTTYWritesLines(t *testing.T) {
 	// Capture stderr through a pipe: exercises the real constructor's
 	// non-TTY branch (plain lines, no ANSI clearing).

internal/output/helpers.go

@@ -2,6 +2,8 @@ package output
 
 import (
 	"fmt"
+	"math"
+	"strings"
 	"time"
 
 	"github.com/vertti/ci-snitch/internal/analyze"
@@ -45,12 +47,18 @@ func groupByType(findings []analyze.Finding) groupedFindings {
 	return g
 }
 
-// fmtDur formats a duration as a compact human-readable string (e.g. "5m30s").
+// fmtDur formats a duration as a compact human-readable string (e.g. "5m30s",
+// "2h30m" — a 2.5-hour p95 as "150m" is unreadable).
 func fmtDur(ad analyze.Duration) string {
 	d := ad.Std().Round(time.Second)
 	if d < time.Minute {
 		return fmt.Sprintf("%ds", int(d.Seconds()))
 	}
+	if d >= time.Hour {
+		h := int(d.Hours())
+		m := int(d.Minutes()) % 60
+		return fmt.Sprintf("%dh%dm", h, m)
+	}
 	m := int(d.Minutes())
 	s := int(d.Seconds()) % 60
 	if s == 0 {
@@ -59,6 +67,17 @@ func fmtDur(ad analyze.Duration) string {
 	return fmt.Sprintf("%dm%02ds", m, s)
 }
 
+// fmtPercentile floors for display: "p100 — slower than 100% of runs" would
+// include the run itself.
+func fmtPercentile(p float64) string {
+	return fmt.Sprintf("p%.0f", math.Floor(p))
+}
+
+// escMD escapes pipes so names can't break markdown table rows.
+func escMD(s string) string {
+	return strings.ReplaceAll(s, "|", `\|`)
+}
+
 // fmtTotalTime formats a duration as hours and minutes (e.g. "2h30m").
 func fmtTotalTime(ad analyze.Duration) string {
 	d := ad.Std()

internal/output/json.go

@@ -5,14 +5,24 @@ import (
 	"io"
 
 	"github.com/vertti/ci-snitch/internal/analyze"
+	"github.com/vertti/ci-snitch/internal/diag"
 )
 
 // JSONFormatter outputs results as indented JSON.
 type JSONFormatter struct{}
 
 // Format implements Formatter.
 func (JSONFormatter) Format(w io.Writer, result *analyze.AnalysisResult) error {
+	// Nil slices marshal to null, which breaks `jq '.diagnostics[]'` and
+	// similar consumers; emit [] instead. Shallow copy — don't mutate input.
+	out := *result
+	if out.Diagnostics == nil {
+		out.Diagnostics = []diag.Diagnostic{}
+	}
+	if out.Findings == nil {
+		out.Findings = []analyze.Finding{}
+	}
 	enc := json.NewEncoder(w)
 	enc.SetIndent("", "  ")
-	return enc.Encode(result)
+	return enc.Encode(&out)
 }

internal/output/llm.go

@@ -124,7 +124,7 @@ func llmWriteSummaryTable(w io.Writer, summaries []analyze.Finding) {
 			queueStr = fmtDur(d.Queue.Median)
 		}
 		_, _ = fmt.Fprintf(w, "| %s | %d | %s | %s | %s | %s | %s |\n",
-			d.Workflow, d.Stats.TotalRuns,
+			escMD(d.Workflow), d.Stats.TotalRuns,
 			fmtDur(d.Stats.Median), fmtDur(d.Stats.P95),
 			queueStr,
 			fmtTotalTime(d.Stats.TotalTime), d.Stats.VolatilityLabel)
@@ -238,8 +238,9 @@ func writeJSONFile(path string, result *analyze.AnalysisResult) error {
 }
 
 // compactResult strips noise from the analysis result for LLM consumption.
-// Drops oscillating/minor changepoints and low-severity outliers that inflate
-// the JSON from ~54k tokens to ~5k without adding actionable information.
+// Drops oscillating/minor changepoints, which inflate the JSON from ~54k
+// tokens to ~5k without adding actionable information (other finding types
+// pass through unchanged).
 func compactResult(result *analyze.AnalysisResult) analyze.AnalysisResult {
 	var filtered []analyze.Finding
 	for _, f := range result.Findings {

internal/output/markdown.go

@@ -36,7 +36,7 @@ func (MarkdownFormatter) Format(w io.Writer, result *analyze.AnalysisResult) err
 			_, _ = fmt.Fprintln(w, "|-----|------|--------|-----|-----|-----|")
 			for _, job := range d.Jobs {
 				_, _ = fmt.Fprintf(w, "| %s | %d | %s | %s | %s | %s |\n",
-					job.Name, job.Stats.TotalRuns,
+					escMD(job.Name), job.Stats.TotalRuns,
 					fmtDur(job.Stats.Median), fmtDur(job.Stats.P95),
 					fmtDur(job.Stats.Min), fmtDur(job.Stats.Max))
 			}
@@ -81,12 +81,12 @@ func (MarkdownFormatter) Format(w io.Writer, result *analyze.AnalysisResult) err
 			if !ok {
 				continue
 			}
-			subject := d.WorkflowName
+			subject := escMD(d.WorkflowName)
 			if d.JobName != "" {
-				subject += " / " + d.JobName
+				subject += " / " + escMD(d.JobName)
 			}
-			_, _ = fmt.Fprintf(w, "| %s | %s | %d | %s | p%.0f | `%s` |\n",
-				d.MaxSeverity, subject, d.Count, fmtDur(d.WorstDuration), d.WorstPercentile, truncSHA(d.WorstCommitSHA))
+			_, _ = fmt.Fprintf(w, "| %s | %s | %d | %s | %s | `%s` |\n",
+				d.MaxSeverity, subject, d.Count, fmtDur(d.WorstDuration), fmtPercentile(d.WorstPercentile), truncSHA(d.WorstCommitSHA))
 		}
 		_, _ = fmt.Fprintln(w)
 	}

internal/output/progress.go

@@ -30,6 +30,11 @@ func NewProgress() *Progress {
 	}
 }
 
+// NewProgressQuiet returns a Progress that discards everything (--quiet).
+func NewProgressQuiet() *Progress {
+	return &Progress{w: io.Discard}
+}
+
 // Status writes a transient status line that will be overwritten by the next call.
 // On non-TTY, each status is printed on its own line.
 func (p *Progress) Status(format string, args ...any) {

internal/output/table.go

@@ -553,11 +553,11 @@ func writeOutlierTable(w io.Writer, findings []analyze.Finding) error {
 		if d.Count == 1 {
 			countStr = "  "
 		}
-		_, _ = fmt.Fprintf(w, "  %s %-*s  %s%-3s%s %s%-8s%s %sp%.0f%s  %s%s%s\n",
+		_, _ = fmt.Fprintf(w, "  %s %-*s  %s%-3s%s %s%-8s%s %s%s%s  %s%s%s\n",
 			severityDot(d.MaxSeverity), maxSubject, subject,
 			bold, countStr, reset,
 			durColor, fmtDur(d.WorstDuration), reset,
-			dim, d.WorstPercentile, reset,
+			dim, fmtPercentile(d.WorstPercentile), reset,
 			dim, truncSHA(d.WorstCommitSHA), reset)
 	}
 	_, _ = fmt.Fprintln(w)