LLM format: deterministic ordering, caveats, glossary, scoped hints

ROADMAP.md

@@ -168,10 +168,8 @@ Removed from the old "already correct" list — disproven by this review:
 - Markdown renders only summaries, non-info changepoints, and outliers — no failures, cost, pipeline, runner, steps, or triage sections, all present in table/JSON (`internal/output/markdown.go`). README markets it for PR comments alongside those features. Also prerequisite for F7 (PR comment bot).
 - **Files:** `internal/output/markdown.go`, golden tests
 
-### U5. LLM format quality [S]
-- Deterministic ordering: `categoryBreakdown` sorts non-stably by count from map iteration; the max-pick over `ByConclusion` ties on map order (`internal/output/llm.go:310-313,416-423`) — output flaps between runs on identical data and will flake any golden test. Use `SortStableFunc` + name tiebreaks.
-- Add a metric glossary (volatility thresholds, persistence semantics live only in the table legend, `table.go:730-737`); narrate diagnostics in the briefing (after D3); key `buildVolatileStepIndex` by (workflow, job) not bare JobName (`llm.go:343-359`); align `[COST]` priority inclusion with the PriorityScore ≥ 50 rule used for suggestions (`llm.go:97-105`).
-- **Files:** `internal/output/llm.go`
+### U5. LLM format quality [S] ✅ done
+- Shipped 2026-07-15: deterministic ordering (category ties break lexicographically; conclusion max-pick tie-broken — both pinned by run-50-times tests); new `## Data Caveats` section narrates diagnostics and `## Glossary` defines volatility/persistence/q-value/billable semantics; volatile-step index keyed by (workflow, job); `[COST]` priorities gate on PriorityScore ≥ 50 like the suggestions.
 
 ### U6. Exit-code semantics for CI gating [M]
 - 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.

internal/output/formatter_test.go

@@ -6,13 +6,15 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/vertti/ci-snitch/internal/analyze"
+	"github.com/vertti/ci-snitch/internal/diag"
 )
 
 func dur(d time.Duration) analyze.Duration { return analyze.Duration(d) }
@@ -302,6 +304,86 @@ func TestLLMFormatter_RawOutputWritesJSONFile(t *testing.T) {
 		"the briefing should point the LLM at the raw file")
 }
 
+func TestLLM_CategoryBreakdownDeterministic(t *testing.T) {
+	// Two categories tied at the same count: map iteration order must not
+	// decide their display order (flapping output flakes golden tests and
+	// confuses diff-based consumers).
+	d := &analyze.FailureDetail{ByCategory: map[string]int{"infra": 5, "build": 5, "test": 2}}
+	first := categoryBreakdown(d)
+	for range 50 {
+		require.Equal(t, first, categoryBreakdown(d), "tied categories must have a stable order")
+	}
+	assert.Contains(t, first, "build")
+	assert.Less(t, strings.Index(first, "build"), strings.Index(first, "infra"),
+		"ties break lexicographically")
+}
+
+func TestLLM_ConclusionHintDeterministic(t *testing.T) {
+	findings := []analyze.Finding{{
+		Type: analyze.TypeFailure,
+		Detail: analyze.FailureDetail{
+			Workflow: "CI", FailureRate: 0.3,
+			ByConclusion: map[string]int{"cancelled": 3, "timed_out": 3},
+		},
+	}}
+	first := suggestFromFailures(findings)
+	for range 50 {
+		require.Equal(t, first, suggestFromFailures(findings),
+			"a tied conclusion max-pick must not flap between runs")
+	}
+}
+
+func TestLLM_VolatileStepIndexKeyedByWorkflow(t *testing.T) {
+	// Same job name in two workflows: workflow A's volatile step must not be
+	// attributed to workflow B's outliers.
+	steps := []analyze.Finding{
+		{Type: "step", Detail: analyze.StepTimingDetail{
+			WorkflowName: "A", JobName: "build",
+			Steps: []analyze.StepSummary{{Name: "docker build", Volatility: 3.8}},
+		}},
+		{Type: "step", Detail: analyze.StepTimingDetail{
+			WorkflowName: "B", JobName: "build",
+			Steps: []analyze.StepSummary{{Name: "npm install", Volatility: 2.5}},
+		}},
+	}
+	idx := buildVolatileStepIndex(steps)
+	require.Len(t, idx, 2, "same-named jobs in different workflows must not share an entry")
+	assert.Equal(t, "docker build", idx[wfJobKey{"A", "build"}].name)
+	assert.Equal(t, "npm install", idx[wfJobKey{"B", "build"}].name)
+}
+
+func TestLLM_BriefingIncludesGlossaryAndCaveats(t *testing.T) {
+	result := richTestResult()
+	result.Diagnostics = []diag.Diagnostic{
+		diag.New(diag.Warn, diag.KindPartialData, "graphql", "3 runs exceed 50 jobs; extra entries were not fetched"),
+	}
+	var buf bytes.Buffer
+	require.NoError(t, LLMFormatter{}.Format(&buf, result))
+
+	out := buf.String()
+	assert.Contains(t, out, "## Data Caveats", "an LLM consumer must see partial-data caveats")
+	assert.Contains(t, out, "extra entries were not fetched")
+	assert.Contains(t, out, "## Glossary", "volatility/persistence semantics live only in the table legend otherwise")
+	assert.Contains(t, out, "volatile")
+	assert.Contains(t, out, "persistent")
+}
+
+func TestLLM_CostPriorityRequiresMeaningfulScore(t *testing.T) {
+	// [COST] priorities included the top-3 workflows regardless of magnitude
+	// while suggestions gate on PriorityScore >= 50 — a 2-minute-a-day
+	// workflow is not a priority finding.
+	result := &analyze.AnalysisResult{
+		Findings: []analyze.Finding{{
+			Type: analyze.TypeCost, Severity: analyze.SeverityInfo,
+			Detail: analyze.CostDetail{Workflow: "tiny", DailyRate: 2, BillableMinutes: 20, PriorityScore: 4},
+		}},
+	}
+	var buf bytes.Buffer
+	require.NoError(t, LLMFormatter{}.Format(&buf, result))
+	assert.NotContains(t, buf.String(), "[COST]",
+		"a negligible-score workflow is not a priority finding")
+}
+
 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)))

internal/output/llm.go

@@ -1,6 +1,7 @@
 package output
 
 import (
+	"cmp"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -9,6 +10,7 @@ import (
 	"strings"
 
 	"github.com/vertti/ci-snitch/internal/analyze"
+	"github.com/vertti/ci-snitch/internal/diag"
 )
 
 // LLMFormatter produces structured output optimized for LLM consumption.
@@ -42,9 +44,35 @@ func (l LLMFormatter) Format(w io.Writer, result *analyze.AnalysisResult) error
 		}
 	}
 
+	llmWriteCaveats(w, result.Diagnostics)
+	llmWriteGlossary(w)
+
 	return l.writeRawData(w, result)
 }
 
+// llmWriteCaveats narrates data-quality diagnostics — an LLM acting on the
+// numbers must know when the dataset was truncated or partially fetched.
+func llmWriteCaveats(w io.Writer, diags []diag.Diagnostic) {
+	if len(diags) == 0 {
+		return
+	}
+	_, _ = fmt.Fprint(w, "\n## Data Caveats\n\n")
+	for _, d := range diags {
+		_, _ = fmt.Fprintf(w, "- %s\n", d.String())
+	}
+}
+
+func llmWriteGlossary(w io.Writer) {
+	_, _ = fmt.Fprint(w, `
+## Glossary
+
+- volatility: p95/median duration ratio per series — stable <1.3, variable 1.3-2x, spiky 2-3x, volatile >=3x
+- persistence: persistent = the shift held for the rest of the window; transient = it reverted; inconclusive = too few runs after the change to judge
+- q_value: false-discovery-rate adjusted p-value across all change points in this report; treat q > 0.05 as noise
+- billable minutes: per-job wall clock rounded up to whole minutes times the runner multiplier; self-hosted runners bill 0
+`)
+}
+
 func llmWritePriorityFindings(w io.Writer, g *groupedFindings) {
 	_, _ = fmt.Fprint(w, "## Priority Findings\n\n")
 	hasPriority := false
@@ -94,12 +122,18 @@ func llmWritePriorityFindings(w io.Writer, g *groupedFindings) {
 		_, _ = fmt.Fprint(w, "\n")
 	}
 
-	costLimit := min(3, len(g.Costs))
-	for i := range costLimit {
+	costShown := 0
+	for i := range g.Costs {
+		if costShown == 3 {
+			break
+		}
 		d, ok := g.Costs[i].Detail.(analyze.CostDetail)
-		if !ok {
+		// Same bar as the suggestions section: a negligible-score workflow
+		// is not a priority finding.
+		if !ok || d.PriorityScore < 50 {
 			continue
 		}
+		costShown++
 		hasPriority = true
 		_, _ = fmt.Fprintf(w, "- **[COST]** %s: %.0f billable mins/day (%.0f total)\n",
 			d.Workflow, d.DailyRate, d.BillableMinutes)
@@ -311,7 +345,12 @@ func categoryBreakdown(d *analyze.FailureDetail) string {
 	for name, count := range d.ByCategory {
 		cats = append(cats, catCount{name, count})
 	}
-	slices.SortFunc(cats, func(a, b catCount) int { return b.count - a.count })
+	slices.SortFunc(cats, func(a, b catCount) int {
+		if a.count != b.count {
+			return b.count - a.count
+		}
+		return cmp.Compare(a.name, b.name) // deterministic order on ties
+	})
 
 	var parts []string
 	for _, c := range cats {
@@ -341,17 +380,22 @@ type volatileStep struct {
 	volatility float64
 }
 
-func buildVolatileStepIndex(steps []analyze.Finding) map[string]volatileStep {
-	index := make(map[string]volatileStep)
+// wfJobKey scopes job lookups to their workflow — job names collide across
+// workflows constantly ("build", "test").
+type wfJobKey struct{ wf, job string }
+
+func buildVolatileStepIndex(steps []analyze.Finding) map[wfJobKey]volatileStep {
+	index := make(map[wfJobKey]volatileStep)
 	for _, f := range steps {
 		d, ok := f.Detail.(analyze.StepTimingDetail)
 		if !ok {
 			continue
 		}
+		k := wfJobKey{d.WorkflowName, d.JobName}
 		for _, st := range d.Steps {
 			if st.Volatility >= 2.0 {
-				if existing, ok := index[d.JobName]; !ok || st.Volatility > existing.volatility {
-					index[d.JobName] = volatileStep{st.Name, st.Volatility}
+				if existing, ok := index[k]; !ok || st.Volatility > existing.volatility {
+					index[k] = volatileStep{st.Name, st.Volatility}
 				}
 			}
 		}
@@ -385,7 +429,7 @@ func suggestFromCosts(findings []analyze.Finding) []string {
 	return s
 }
 
-func suggestFromOutliers(findings []analyze.Finding, volatileSteps map[string]volatileStep) []string {
+func suggestFromOutliers(findings []analyze.Finding, volatileSteps map[wfJobKey]volatileStep) []string {
 	var s []string
 	for _, f := range findings {
 		d, ok := f.Detail.(analyze.OutlierGroupDetail)
@@ -397,7 +441,7 @@ func suggestFromOutliers(findings []analyze.Finding, volatileSteps map[string]vo
 			subject = d.JobName
 		}
 		hint := "check for resource contention or flaky infrastructure"
-		if vs, ok := volatileSteps[d.JobName]; ok {
+		if vs, ok := volatileSteps[wfJobKey{d.WorkflowName, d.JobName}]; ok {
 			hint = fmt.Sprintf("step %q is %.1fx volatile and likely the cause", vs.name, vs.volatility)
 		}
 		s = append(s, fmt.Sprintf("%q has %d outliers (worst %s) -- %s",
@@ -417,7 +461,8 @@ func suggestFromFailures(findings []analyze.Finding) []string {
 		maxConclusion := ""
 		maxCount := 0
 		for c, n := range d.ByConclusion {
-			if n > maxCount {
+			// Lexicographic tie-break: map order must not decide the hint.
+			if n > maxCount || (n == maxCount && (maxConclusion == "" || c < maxConclusion)) {
 				maxCount = n
 				maxConclusion = c
 			}