From 0db2061d3854cc6231f4575099515543a8457086 Mon Sep 17 00:00:00 2001 From: Dwin Gharibi Date: Fri, 7 Aug 2026 09:49:13 +0330 Subject: [PATCH 1/5] feat(cmd/root/eval.go): compare a run against a saved baseline and fail on regression --- cmd/root/eval.go | 38 +++++ pkg/evaluation/baseline.go | 328 +++++++++++++++++++++++++++++++++++++ 2 files changed, 366 insertions(+) create mode 100644 pkg/evaluation/baseline.go diff --git a/cmd/root/eval.go b/cmd/root/eval.go index 487c4b9353..2d752cdb40 100644 --- a/cmd/root/eval.go +++ b/cmd/root/eval.go @@ -1,6 +1,7 @@ package root import ( + "errors" "fmt" "io" "log/slog" @@ -25,6 +26,13 @@ type evalFlags struct { runConfig config.RuntimeConfig outputDir string + + // baseline is a previously saved run (an -eval.json written by a prior + // invocation) to compare this run against; empty disables the check. + baseline string + // regressionTolerance is how far an aggregate quality rate may fall before + // the comparison fails. See evaluation.Compare for the exact semantics. + regressionTolerance float64 } func newEvalCmd() *cobra.Command { @@ -48,6 +56,8 @@ func newEvalCmd() *cobra.Command { cmd.Flags().BoolVar(&flags.KeepContainers, "keep-containers", false, "Keep containers after evaluation (don't use --rm)") cmd.Flags().StringSliceVarP(&flags.EnvVars, "env", "e", nil, "Environment variables to pass to container (KEY or KEY=VALUE)") cmd.Flags().IntVar(&flags.Repeat, "repeat", 1, "Number of times to repeat each evaluation (useful for computing baselines)") + cmd.Flags().StringVar(&flags.baseline, "baseline", "", "Compare against a previously saved run JSON and exit non-zero on regression") + cmd.Flags().Float64Var(&flags.regressionTolerance, "regression-tolerance", 0, "How far an aggregate quality rate may fall before --baseline reports a regression (0-1)") return cmd } @@ -149,5 +159,33 @@ func (f *evalFlags) runEvalCommand(cmd *cobra.Command, args []string) (commandEr fmt.Fprintf(teeOut, "Log: %s\n", logPath) + if regressionErr := f.checkBaseline(teeOut, run); regressionErr != nil && evalErr == nil { + // When the run itself also errored, that is the more fundamental + // problem and keeps the exit code. + return regressionErr + } + return evalErr } + +// checkBaseline compares run against the configured baseline and returns a +// non-nil error when it regressed, so CI fails on the exit code. A no-op when +// --baseline was not supplied. +func (f *evalFlags) checkBaseline(out io.Writer, run *evaluation.EvalRun) error { + if f.baseline == "" { + return nil + } + + baseline, err := evaluation.LoadBaseline(f.baseline) + if err != nil { + return err + } + + comparison := evaluation.Compare(baseline, run, f.regressionTolerance) + evaluation.PrintComparison(out, comparison) + + if comparison.Regressed { + return errors.New("evaluation regressed against baseline") + } + return nil +} diff --git a/pkg/evaluation/baseline.go b/pkg/evaluation/baseline.go new file mode 100644 index 0000000000..7105919366 --- /dev/null +++ b/pkg/evaluation/baseline.go @@ -0,0 +1,328 @@ +package evaluation + +import ( + "cmp" + "encoding/json" + "fmt" + "io" + "os" + "slices" + "text/tabwriter" +) + +// Metrics is the comparable shape of an evaluation run: the rates a regression +// gate can be built on, derived from the same fields [computeSummary] uses. +// +// Rates are 0 when their denominator is 0, and the corresponding Has… flag says +// whether the rate means anything. Without that distinction "no size +// expectations declared" and "every size expectation failed" would both read as +// 0.0 and a gate could not tell them apart. +type Metrics struct { + TotalEvals int `json:"total_evals"` + FailedEvals int `json:"failed_evals"` + FailureRate float64 `json:"failure_rate"` + + SizePassRate float64 `json:"size_pass_rate"` + HasSizes bool `json:"has_sizes"` + + ToolsF1Mean float64 `json:"tools_f1_mean"` + HasTools bool `json:"has_tools"` + + RelevanceRate float64 `json:"relevance_rate"` + HasRelevance bool `json:"has_relevance"` + + TotalCost float64 `json:"total_cost"` +} + +// MetricsOf derives [Metrics] from a run's results. +func MetricsOf(run *EvalRun) Metrics { + if run == nil { + return Metrics{} + } + s := computeSummary(run.Results) + + m := Metrics{ + TotalEvals: s.TotalEvals, + FailedEvals: s.FailedEvals, + TotalCost: s.TotalCost, + } + if s.TotalEvals > 0 { + m.FailureRate = float64(s.FailedEvals) / float64(s.TotalEvals) + } + if s.SizesTotal > 0 { + m.HasSizes = true + m.SizePassRate = float64(s.SizesPassed) / float64(s.SizesTotal) + } + if s.ToolsCount > 0 { + m.HasTools = true + m.ToolsF1Mean = s.ToolsF1Sum / float64(s.ToolsCount) + } + if s.RelevanceTotal > 0 { + m.HasRelevance = true + m.RelevanceRate = s.RelevancePassed / s.RelevanceTotal + } + return m +} + +// MetricDelta is one metric's movement between two runs. Higher is better for +// quality rates and worse for FailureRate, so Regressed — not the sign of Delta +// — is what a gate reads. +type MetricDelta struct { + Name string `json:"name"` + Baseline float64 `json:"baseline"` + Current float64 `json:"current"` + Delta float64 `json:"delta"` + Regressed bool `json:"regressed"` + // Informational marks a metric that is reported but never gates, so a + // reviewer can see it moved without the build failing over it. + Informational bool `json:"informational,omitempty"` +} + +// EvalChange records one evaluation's pass/fail transition between runs. +type EvalChange struct { + InputPath string `json:"input_path"` + // Was and Now are "pass", "fail", or "absent". + Was string `json:"was"` + Now string `json:"now"` + Regressed bool `json:"regressed"` +} + +// Comparison is the result of checking a run against a baseline. +type Comparison struct { + Baseline Metrics `json:"baseline"` + Current Metrics `json:"current"` + Tolerance float64 `json:"tolerance"` + Deltas []MetricDelta `json:"deltas"` + Changes []EvalChange `json:"changes"` + // Regressed is true when any gating metric moved beyond the tolerance, or + // an evaluation that passed in the baseline now fails. + Regressed bool `json:"regressed"` +} + +// LoadBaseline reads a run previously written by [SaveRunJSON]. +func LoadBaseline(path string) (*EvalRun, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("reading baseline %q: %w", path, err) + } + var run EvalRun + if err := json.Unmarshal(data, &run); err != nil { + return nil, fmt.Errorf("parsing baseline %q: %w", path, err) + } + return &run, nil +} + +// Compare checks current against baseline. +// +// tolerance is the amount an aggregate quality rate may fall (or the failure +// rate may climb) before it counts as a regression, so judge variance does not +// fail a build on noise. A tolerance of 0 means any movement in the wrong +// direction regresses; a negative value is clamped to 0. +// +// The tolerance governs aggregate rates ONLY. An evaluation that passed in the +// baseline and now fails gates regardless of tolerance — that transition is the +// exact signal this check exists to catch, and absorbing it would defeat the +// point. Consequently a large tolerance still cannot hide an outright breakage. +// +// Note the corollary: adding a new FAILING evaluation lowers the aggregate rate +// and therefore gates, even though no existing evaluation regressed. That is +// intended — a suite that got worse should say so — but it means "add a known- +// failing eval as a TODO" needs an explicit tolerance bump or a fix. +// +// Cost is reported but never gates: a cost increase is not a quality regression, +// and gating on it would make the check fire on provider price changes. +// +// A metric absent from either run is skipped rather than treated as 0 — adding +// the first size expectation to a suite must not look like a regression. +func Compare(baseline, current *EvalRun, tolerance float64) Comparison { + if tolerance < 0 { + tolerance = 0 + } + + c := Comparison{ + Baseline: MetricsOf(baseline), + Current: MetricsOf(current), + Tolerance: tolerance, + } + + // Quality rates: a drop beyond the tolerance regresses. + for _, q := range []struct { + name string + base, cur float64 + hasBase, hasCur bool + }{ + {"size pass rate", c.Baseline.SizePassRate, c.Current.SizePassRate, c.Baseline.HasSizes, c.Current.HasSizes}, + {"tool F1 mean", c.Baseline.ToolsF1Mean, c.Current.ToolsF1Mean, c.Baseline.HasTools, c.Current.HasTools}, + {"relevance rate", c.Baseline.RelevanceRate, c.Current.RelevanceRate, c.Baseline.HasRelevance, c.Current.HasRelevance}, + } { + if !q.hasBase || !q.hasCur { + continue + } + d := MetricDelta{Name: q.name, Baseline: q.base, Current: q.cur, Delta: q.cur - q.base} + d.Regressed = q.cur < q.base-tolerance + c.Deltas = append(c.Deltas, d) + } + + // Failure rate: a climb beyond the tolerance regresses. + if c.Baseline.TotalEvals > 0 && c.Current.TotalEvals > 0 { + d := MetricDelta{ + Name: "failure rate", + Baseline: c.Baseline.FailureRate, + Current: c.Current.FailureRate, + Delta: c.Current.FailureRate - c.Baseline.FailureRate, + } + d.Regressed = c.Current.FailureRate > c.Baseline.FailureRate+tolerance + c.Deltas = append(c.Deltas, d) + } + + c.Deltas = append(c.Deltas, MetricDelta{ + Name: "total cost", + Baseline: c.Baseline.TotalCost, + Current: c.Current.TotalCost, + Delta: c.Current.TotalCost - c.Baseline.TotalCost, + Informational: true, + }) + + c.Changes = compareEvals(baseline, current) + + for _, d := range c.Deltas { + if d.Regressed && !d.Informational { + c.Regressed = true + } + } + for _, ch := range c.Changes { + if ch.Regressed { + c.Regressed = true + } + } + return c +} + +// compareEvals pairs evaluations by input path and reports transitions. Only +// pass → fail is a regression; appearing and disappearing evaluations are +// reported so a reviewer notices a suite change, but do not gate. +func compareEvals(baseline, current *EvalRun) []EvalChange { + was := passByPath(baseline) + now := passByPath(current) + + paths := make(map[string]struct{}, len(was)+len(now)) + for p := range was { + paths[p] = struct{}{} + } + for p := range now { + paths[p] = struct{}{} + } + + changes := make([]EvalChange, 0, len(paths)) + for p := range paths { + bs, inBase := was[p] + cs, inCur := now[p] + change := EvalChange{ + InputPath: p, + Was: passLabel(bs, inBase), + Now: passLabel(cs, inCur), + } + if change.Was == change.Now { + continue + } + change.Regressed = inBase && bs && inCur && !cs + changes = append(changes, change) + } + + slices.SortFunc(changes, func(a, b EvalChange) int { + // Regressions first, then alphabetical, so the important lines lead. + if a.Regressed != b.Regressed { + if a.Regressed { + return -1 + } + return 1 + } + return cmp.Compare(a.InputPath, b.InputPath) + }) + return changes +} + +func passByPath(run *EvalRun) map[string]bool { + out := map[string]bool{} + if run == nil { + return out + } + for _, r := range run.Results { + // Repeated runs of the same input (Config.Repeat > 1) collapse + // pessimistically: if any repetition failed, the eval counts as failed. + if prev, seen := out[r.InputPath]; seen { + out[r.InputPath] = prev && resultPassed(r) + continue + } + out[r.InputPath] = resultPassed(r) + } + return out +} + +func passLabel(passed, present bool) string { + switch { + case !present: + return "absent" + case passed: + return "pass" + default: + return "fail" + } +} + +// resultPassed reports whether one result met every expectation declared for it, +// mirroring the fields [computeSummary] scores. An expectation that was not +// declared is not a failure. +func resultPassed(r Result) bool { + if r.Error != "" { + return false + } + if r.SizeExpected != "" && r.SizeExpected != r.Size { + return false + } + if r.ToolCallsExpected > 0 && r.ToolCallsScore < r.ToolCallsExpected { + return false + } + if r.RelevanceExpected > 0 && r.RelevancePassed < r.RelevanceExpected { + return false + } + return true +} + +// PrintComparison writes a human-readable comparison. +func PrintComparison(out io.Writer, c Comparison) { + fmt.Fprintf(out, "\nBaseline comparison (tolerance %.3f)\n", c.Tolerance) + + tw := tabwriter.NewWriter(out, 0, 8, 2, ' ', 0) + fmt.Fprintln(tw, "METRIC\tBASELINE\tCURRENT\tDELTA\t") + for _, d := range c.Deltas { + marker := " " + switch { + case d.Regressed: + marker = "! " + case d.Informational: + marker = "· " + } + fmt.Fprintf(tw, "%s%s\t%.3f\t%.3f\t%+.3f\t\n", marker, d.Name, d.Baseline, d.Current, d.Delta) + } + _ = tw.Flush() + + if len(c.Changes) > 0 { + fmt.Fprintln(out, "\nChanged evaluations") + ctw := tabwriter.NewWriter(out, 0, 8, 2, ' ', 0) + for _, ch := range c.Changes { + marker := " " + if ch.Regressed { + marker = "! " + } + fmt.Fprintf(ctw, "%s%s\t%s → %s\t\n", marker, ch.InputPath, ch.Was, ch.Now) + } + _ = ctw.Flush() + } + + if c.Regressed { + fmt.Fprintln(out, "\n❌ Regression against baseline") + return + } + fmt.Fprintln(out, "\n✅ No regression against baseline") +} From 5480ffa0770564e9317fbc2bcd87b589c1127a9a Mon Sep 17 00:00:00 2001 From: Dwin Gharibi Date: Fri, 7 Aug 2026 09:51:08 +0330 Subject: [PATCH 2/5] test(pkg/evaluation/baseline_test.go): adding up some amazing tests for new eval matrix --- cmd/root/eval_baseline_test.go | 106 ++++++++++ pkg/evaluation/baseline_test.go | 350 ++++++++++++++++++++++++++++++++ 2 files changed, 456 insertions(+) create mode 100644 cmd/root/eval_baseline_test.go create mode 100644 pkg/evaluation/baseline_test.go diff --git a/cmd/root/eval_baseline_test.go b/cmd/root/eval_baseline_test.go new file mode 100644 index 0000000000..7699331495 --- /dev/null +++ b/cmd/root/eval_baseline_test.go @@ -0,0 +1,106 @@ +package root + +import ( + "bytes" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/evaluation" +) + +func sizeRun(pass bool) *evaluation.EvalRun { + r := evaluation.Result{InputPath: "a", SizeExpected: "medium", Size: "medium"} + if !pass { + r.Size = "short" + } + return &evaluation.EvalRun{Name: "run", Results: []evaluation.Result{r}} +} + +func TestCheckBaseline_NoBaselineIsANoOp(t *testing.T) { + t.Parallel() + + f := &evalFlags{} + var buf bytes.Buffer + require.NoError(t, f.checkBaseline(&buf, sizeRun(false))) + assert.Empty(t, buf.String(), "without --baseline nothing is compared or printed") +} + +func TestCheckBaseline_RegressionReturnsAnError(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path, err := evaluation.SaveRunJSON(sizeRun(true), dir) + require.NoError(t, err) + + f := &evalFlags{baseline: path} + var buf bytes.Buffer + err = f.checkBaseline(&buf, sizeRun(false)) + + require.Error(t, err, "a regression must surface as a non-zero exit") + assert.Contains(t, err.Error(), "regressed against baseline") + assert.Contains(t, buf.String(), "Regression against baseline") +} + +func TestCheckBaseline_NoRegressionSucceeds(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + path, err := evaluation.SaveRunJSON(sizeRun(true), dir) + require.NoError(t, err) + + f := &evalFlags{baseline: path} + var buf bytes.Buffer + require.NoError(t, f.checkBaseline(&buf, sizeRun(true))) + assert.Contains(t, buf.String(), "No regression against baseline") +} + +func TestCheckBaseline_ToleranceIsPlumbedThrough(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + base := &evaluation.EvalRun{Name: "b", Results: []evaluation.Result{ + {InputPath: "a", ToolCallsExpected: 0.8, ToolCallsScore: 1.0}, + {InputPath: "b", ToolCallsExpected: 0.8, ToolCallsScore: 1.0}, + }} + cur := &evaluation.EvalRun{Name: "c", Results: []evaluation.Result{ + {InputPath: "a", ToolCallsExpected: 0.8, ToolCallsScore: 1.0}, + {InputPath: "b", ToolCallsExpected: 0.8, ToolCallsScore: 0.9}, + }} + + path, err := evaluation.SaveRunJSON(base, dir) + require.NoError(t, err) + + var buf bytes.Buffer + strict := &evalFlags{baseline: path, regressionTolerance: 0.01} + require.Error(t, strict.checkBaseline(&buf, cur), "a tight tolerance gates the 0.05 drop") + + buf.Reset() + lenient := &evalFlags{baseline: path, regressionTolerance: 0.10} + require.NoError(t, lenient.checkBaseline(&buf, cur), "a wider tolerance absorbs it") +} + +func TestCheckBaseline_MissingBaselineFileIsAnError(t *testing.T) { + t.Parallel() + + f := &evalFlags{baseline: filepath.Join(t.TempDir(), "nope.json")} + var buf bytes.Buffer + err := f.checkBaseline(&buf, sizeRun(true)) + + require.Error(t, err) + assert.Contains(t, err.Error(), "reading baseline", + "a bad --baseline path must fail loudly rather than silently skipping the gate") +} + +func TestEvalCmd_BaselineFlagsAreRegistered(t *testing.T) { + t.Parallel() + + cmd := newEvalCmd() + require.NotNil(t, cmd.Flags().Lookup("baseline")) + + tolerance := cmd.Flags().Lookup("regression-tolerance") + require.NotNil(t, tolerance) + assert.Equal(t, "0", tolerance.DefValue, "the default gates any drop") +} diff --git a/pkg/evaluation/baseline_test.go b/pkg/evaluation/baseline_test.go new file mode 100644 index 0000000000..2533497c1e --- /dev/null +++ b/pkg/evaluation/baseline_test.go @@ -0,0 +1,350 @@ +package evaluation + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// sizeResult is a result whose only declared expectation is response size. +func sizeResult(path string, pass bool) Result { + r := Result{InputPath: path, SizeExpected: "medium", Size: "medium"} + if !pass { + r.Size = "short" + } + return r +} + +func run(results ...Result) *EvalRun { + return &EvalRun{Name: "run", Results: results} +} + +func TestMetricsOf_RatesAndFlags(t *testing.T) { + t.Parallel() + + got := MetricsOf(run( + sizeResult("a", true), + sizeResult("b", false), + Result{InputPath: "c", Error: "boom"}, + Result{InputPath: "d", ToolCallsExpected: 1, ToolCallsScore: 0.5, Cost: 0.25}, + Result{InputPath: "e", RelevanceExpected: 2, RelevancePassed: 1}, + )) + + assert.Equal(t, 5, got.TotalEvals) + assert.Equal(t, 1, got.FailedEvals) + assert.InDelta(t, 0.2, got.FailureRate, 1e-9) + assert.True(t, got.HasSizes) + assert.InDelta(t, 0.5, got.SizePassRate, 1e-9) + assert.True(t, got.HasTools) + assert.InDelta(t, 0.5, got.ToolsF1Mean, 1e-9) + assert.True(t, got.HasRelevance) + assert.InDelta(t, 0.5, got.RelevanceRate, 1e-9) + assert.InDelta(t, 0.25, got.TotalCost, 1e-9) +} + +// A rate with no denominator must be distinguishable from a rate of 0.0, or a +// gate cannot tell "nothing declared" from "everything failed". +func TestMetricsOf_AbsentMetricsAreFlaggedNotZero(t *testing.T) { + t.Parallel() + + got := MetricsOf(run(Result{InputPath: "a"})) + assert.False(t, got.HasSizes) + assert.False(t, got.HasTools) + assert.False(t, got.HasRelevance) + assert.Zero(t, got.SizePassRate) + + assert.Equal(t, Metrics{}, MetricsOf(nil), "a nil run yields zero metrics, not a panic") +} + +func TestCompare_NoChangeIsNotARegression(t *testing.T) { + t.Parallel() + + base := run(sizeResult("a", true), sizeResult("b", true)) + cur := run(sizeResult("a", true), sizeResult("b", true)) + + got := Compare(base, cur, 0) + assert.False(t, got.Regressed) + assert.Empty(t, got.Changes) +} + +func TestCompare_QualityDropRegresses(t *testing.T) { + t.Parallel() + + base := run(sizeResult("a", true), sizeResult("b", true)) + cur := run(sizeResult("a", true), sizeResult("b", false)) + + got := Compare(base, cur, 0) + require.True(t, got.Regressed) + + var sizeDelta *MetricDelta + for i := range got.Deltas { + if got.Deltas[i].Name == "size pass rate" { + sizeDelta = &got.Deltas[i] + } + } + require.NotNil(t, sizeDelta) + assert.True(t, sizeDelta.Regressed) + assert.InDelta(t, -0.5, sizeDelta.Delta, 1e-9) +} + +// Judge variance shows up as small movement in the aggregate rates; the +// tolerance exists so a build is not failed by that noise. +// +// Both evaluations stay above their declared expectation (0.8) here, so no +// pass/fail transition occurs and the tolerance is what decides — see +// TestCompare_PassToFailGatesRegardlessOfTolerance for the other half. +func TestCompare_ToleranceAbsorbsSmallDrops(t *testing.T) { + t.Parallel() + + base := run( + Result{InputPath: "a", ToolCallsExpected: 0.8, ToolCallsScore: 1.0}, + Result{InputPath: "b", ToolCallsExpected: 0.8, ToolCallsScore: 1.0}, + ) + cur := run( + Result{InputPath: "a", ToolCallsExpected: 0.8, ToolCallsScore: 1.0}, + Result{InputPath: "b", ToolCallsExpected: 0.8, ToolCallsScore: 0.9}, + ) + + // Mean drops 1.00 → 0.95, and both evaluations still pass. + assert.False(t, Compare(base, cur, 0.10).Regressed, "within tolerance") + assert.True(t, Compare(base, cur, 0.01).Regressed, "beyond tolerance") + assert.True(t, Compare(base, cur, 0).Regressed, "zero tolerance gates any drop") + assert.True(t, Compare(base, cur, -5).Regressed, + "a negative tolerance is clamped to 0, so this still gates") +} + +// The tolerance governs aggregate rates only. An evaluation that passed and now +// fails is the exact signal a regression gate exists to catch, so it gates even +// when the aggregate movement is small enough to be absorbed. +func TestCompare_PassToFailGatesRegardlessOfTolerance(t *testing.T) { + t.Parallel() + + base := run( + Result{InputPath: "a", ToolCallsExpected: 1, ToolCallsScore: 1.0}, + Result{InputPath: "b", ToolCallsExpected: 1, ToolCallsScore: 1.0}, + ) + cur := run( + Result{InputPath: "a", ToolCallsExpected: 1, ToolCallsScore: 1.0}, + Result{InputPath: "b", ToolCallsExpected: 1, ToolCallsScore: 0.9}, // now below expectation + ) + + got := Compare(base, cur, 0.99) // a tolerance far larger than the rate movement + assert.True(t, got.Regressed, "a pass → fail transition is not absorbed by the tolerance") + + require.Len(t, got.Changes, 1) + assert.Equal(t, "b", got.Changes[0].InputPath) + assert.True(t, got.Changes[0].Regressed) +} + +func TestCompare_ImprovementIsNotARegression(t *testing.T) { + t.Parallel() + + base := run(sizeResult("a", false), sizeResult("b", false)) + cur := run(sizeResult("a", true), sizeResult("b", true)) + + got := Compare(base, cur, 0) + assert.False(t, got.Regressed) + require.NotEmpty(t, got.Changes) + for _, ch := range got.Changes { + assert.False(t, ch.Regressed, "fail → pass must never gate") + } +} + +func TestCompare_FailureRateClimbRegresses(t *testing.T) { + t.Parallel() + + base := run(Result{InputPath: "a"}, Result{InputPath: "b"}) + cur := run(Result{InputPath: "a"}, Result{InputPath: "b", Error: "boom"}) + + got := Compare(base, cur, 0) + assert.True(t, got.Regressed) +} + +// Cost is reported but must never gate: a price change is not a quality change. +func TestCompare_CostIsInformationalOnly(t *testing.T) { + t.Parallel() + + base := run(Result{InputPath: "a", Cost: 0.01}) + cur := run(Result{InputPath: "a", Cost: 100}) + + got := Compare(base, cur, 0) + assert.False(t, got.Regressed, "a cost increase alone must not fail the gate") + + var cost *MetricDelta + for i := range got.Deltas { + if got.Deltas[i].Name == "total cost" { + cost = &got.Deltas[i] + } + } + require.NotNil(t, cost) + assert.True(t, cost.Informational) + assert.InDelta(t, 99.99, cost.Delta, 1e-6) +} + +// Adding the first expectation of a kind must not read as a regression from 0. +func TestCompare_MetricAbsentFromOneSideIsSkipped(t *testing.T) { + t.Parallel() + + base := run(Result{InputPath: "a"}) // no size expectations + cur := run(sizeResult("a", false)) // size expectation newly added, failing + + got := Compare(base, cur, 0) + for _, d := range got.Deltas { + assert.NotEqual(t, "size pass rate", d.Name, + "a metric with no baseline must be skipped, not compared against 0") + } +} + +// An evaluation appearing or disappearing is a suite change, not a regression of +// existing behaviour, so the per-eval transition itself never gates. +func TestCompare_AddedAndRemovedEvalsDoNotGateOnTheirOwn(t *testing.T) { + t.Parallel() + + base := run(sizeResult("a", true), sizeResult("gone", true)) + cur := run(sizeResult("a", true), sizeResult("new", true)) + + got := Compare(base, cur, 0) + + byPath := map[string]EvalChange{} + for _, ch := range got.Changes { + byPath[ch.InputPath] = ch + } + require.Contains(t, byPath, "gone") + require.Contains(t, byPath, "new") + assert.Equal(t, "absent", byPath["gone"].Now) + assert.Equal(t, "absent", byPath["new"].Was) + assert.False(t, byPath["gone"].Regressed, "a removed eval is a suite change, not a regression") + assert.False(t, byPath["new"].Regressed, "an added eval is a suite change, not a regression") + assert.False(t, got.Regressed, "the aggregate rate is unchanged, so nothing gates") +} + +// But an added evaluation that FAILS does lower the aggregate rate, and that +// gates — which is the desirable outcome: a suite that got worse should say so, +// even though no individual evaluation went from passing to failing. +func TestCompare_AddedFailingEvalGatesViaTheAggregateRate(t *testing.T) { + t.Parallel() + + base := run(sizeResult("a", true)) + cur := run(sizeResult("a", true), sizeResult("new", false)) + + got := Compare(base, cur, 0) + assert.True(t, got.Regressed, "size pass rate fell 1.00 → 0.50") + + for _, ch := range got.Changes { + if ch.InputPath == "new" { + assert.False(t, ch.Regressed, + "the gate comes from the aggregate, not from the added eval's transition") + } + } +} + +func TestCompare_RegressionsAreListedFirst(t *testing.T) { + t.Parallel() + + base := run(sizeResult("aaa", true), sizeResult("zzz", true)) + cur := run(sizeResult("aaa", true), sizeResult("zzz", false), sizeResult("bbb", true)) + + got := Compare(base, cur, 0) + require.NotEmpty(t, got.Changes) + assert.Equal(t, "zzz", got.Changes[0].InputPath, "the regression must lead") + assert.True(t, got.Changes[0].Regressed) +} + +// Config.Repeat runs the same input more than once; one bad repetition means the +// evaluation is not reliably passing. +func TestCompare_RepeatedInputCollapsesPessimistically(t *testing.T) { + t.Parallel() + + base := run(sizeResult("a", true), sizeResult("a", true)) + cur := run(sizeResult("a", true), sizeResult("a", false)) + + got := Compare(base, cur, 0) + require.Len(t, got.Changes, 1) + assert.Equal(t, "pass", got.Changes[0].Was) + assert.Equal(t, "fail", got.Changes[0].Now) + assert.True(t, got.Changes[0].Regressed) +} + +func TestLoadBaseline_RoundTripsSaveRunJSON(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + original := run(sizeResult("a", true), Result{InputPath: "b", Error: "boom"}) + path, err := SaveRunJSON(original, dir) + require.NoError(t, err) + + loaded, err := LoadBaseline(path) + require.NoError(t, err) + assert.Equal(t, MetricsOf(original), MetricsOf(loaded), + "a saved run must be loadable as a baseline with identical metrics") +} + +func TestLoadBaseline_Errors(t *testing.T) { + t.Parallel() + + _, err := LoadBaseline(filepath.Join(t.TempDir(), "missing.json")) + require.Error(t, err) + assert.Contains(t, err.Error(), "reading baseline") + + bad := filepath.Join(t.TempDir(), "bad.json") + require.NoError(t, os.WriteFile(bad, []byte("{not json"), 0o600)) + _, err = LoadBaseline(bad) + require.Error(t, err) + assert.Contains(t, err.Error(), "parsing baseline") +} + +func TestPrintComparison(t *testing.T) { + t.Parallel() + + t.Run("regression", func(t *testing.T) { + t.Parallel() + var buf bytes.Buffer + PrintComparison(&buf, Compare( + run(sizeResult("a", true)), + run(sizeResult("a", false)), + 0, + )) + out := buf.String() + assert.Contains(t, out, "Regression against baseline") + assert.Contains(t, out, "size pass rate") + assert.Contains(t, out, "! ", "regressed rows are marked") + }) + + t.Run("clean", func(t *testing.T) { + t.Parallel() + var buf bytes.Buffer + PrintComparison(&buf, Compare(run(sizeResult("a", true)), run(sizeResult("a", true)), 0)) + assert.Contains(t, buf.String(), "No regression against baseline") + }) +} + +func TestComparison_IsJSONSerializable(t *testing.T) { + t.Parallel() + + c := Compare(run(sizeResult("a", true)), run(sizeResult("a", false)), 0.05) + data, err := json.Marshal(c) + require.NoError(t, err) + + var round Comparison + require.NoError(t, json.Unmarshal(data, &round)) + assert.True(t, round.Regressed) + assert.InDelta(t, 0.05, round.Tolerance, 1e-9) +} + +func TestResultPassed(t *testing.T) { + t.Parallel() + + assert.True(t, resultPassed(Result{InputPath: "a"}), "no expectations declared means nothing to fail") + assert.False(t, resultPassed(Result{Error: "boom"})) + assert.True(t, resultPassed(Result{SizeExpected: "short", Size: "short"})) + assert.False(t, resultPassed(Result{SizeExpected: "short", Size: "long"})) + assert.True(t, resultPassed(Result{ToolCallsExpected: 0.8, ToolCallsScore: 0.9})) + assert.False(t, resultPassed(Result{ToolCallsExpected: 0.8, ToolCallsScore: 0.7})) + assert.True(t, resultPassed(Result{RelevanceExpected: 2, RelevancePassed: 2})) + assert.False(t, resultPassed(Result{RelevanceExpected: 2, RelevancePassed: 1})) +} From 4cae8da95954362a4333c3012da6d35f43f3864a Mon Sep 17 00:00:00 2001 From: Dwin Gharibi Date: Sat, 8 Aug 2026 22:04:38 +0330 Subject: [PATCH 3/5] fix(evaluation): ground the baseline gate on the file eval writes, and fail closed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects, each of which made the gate unusable or untrustworthy. --baseline could not read anything `docker agent eval` produces. LoadBaseline expected an EvalRun; the command writes a RunOutput via SaveRunSessionsJSON, and their Duration fields are typed differently (string vs time.Duration), so the load failed outright. SaveRunJSON, which the description named as the producer, has no non-test callers. Baselines are now read from RunOutput — its summary carries the aggregate rates and sessions[].eval_result.passed carries per-eval pass/fail — so "no new format" is finally true. The gate failed open. Any JSON object loaded successfully, and an empty baseline yields all-zero metrics: every rate is skipped by its has-flag guard, the failure rate by its own guard, and the run reports "No regression" while every evaluation in it fails. The same held when the current run had no evaluations, e.g. an --only pattern matching nothing. Both are now rejected, as is a baseline sharing no metric and no evaluation with the run — a comparison that can only pass is worse than no comparison. resultPassed contradicted the project's own definition of pass. checkResults requires a tool-call score of 1.0; resultPassed required only >= ToolCallsExpected, so an evaluation whose printed status flipped from PASS to FAIL was recorded as no change and the aggregate movement was then absorbed by the tolerance. It now delegates to checkResults, so there is one notion of "passed" in the package and the gate cannot drift from the product. Also: a run that did not complete no longer prints a baseline verdict at all. Its missing evaluations register as absent and do not gate, so it printed "✅ No regression against baseline" and then exited non-zero. And --regression-tolerance above 1 is rejected at startup: no rate can move that far, so it silently disabled the aggregate gate. --- cmd/root/eval.go | 27 +++-- pkg/evaluation/baseline.go | 227 ++++++++++++++++++++++++------------- 2 files changed, 171 insertions(+), 83 deletions(-) diff --git a/cmd/root/eval.go b/cmd/root/eval.go index 2d752cdb40..2ec3fc0dd1 100644 --- a/cmd/root/eval.go +++ b/cmd/root/eval.go @@ -56,13 +56,18 @@ func newEvalCmd() *cobra.Command { cmd.Flags().BoolVar(&flags.KeepContainers, "keep-containers", false, "Keep containers after evaluation (don't use --rm)") cmd.Flags().StringSliceVarP(&flags.EnvVars, "env", "e", nil, "Environment variables to pass to container (KEY or KEY=VALUE)") cmd.Flags().IntVar(&flags.Repeat, "repeat", 1, "Number of times to repeat each evaluation (useful for computing baselines)") - cmd.Flags().StringVar(&flags.baseline, "baseline", "", "Compare against a previously saved run JSON and exit non-zero on regression") + cmd.Flags().StringVar(&flags.baseline, "baseline", "", "Compare against a previously saved run JSON (/.json) and exit non-zero on regression") cmd.Flags().Float64Var(&flags.regressionTolerance, "regression-tolerance", 0, "How far an aggregate quality rate may fall before --baseline reports a regression (0-1)") return cmd } func (f *evalFlags) runEvalCommand(cmd *cobra.Command, args []string) (commandErr error) { + if f.regressionTolerance > evaluation.MaxTolerance { + return fmt.Errorf("--regression-tolerance must be between 0 and %v; %v would disable the aggregate gate", + evaluation.MaxTolerance, f.regressionTolerance) + } + telemetry.TrackCommand(cmd.Context(), "eval", args) defer func() { // do not inline this defer so that commandErr is not resolved early telemetry.TrackCommandError(cmd.Context(), "eval", args, commandErr) @@ -159,13 +164,18 @@ func (f *evalFlags) runEvalCommand(cmd *cobra.Command, args []string) (commandEr fmt.Fprintf(teeOut, "Log: %s\n", logPath) - if regressionErr := f.checkBaseline(teeOut, run); regressionErr != nil && evalErr == nil { - // When the run itself also errored, that is the more fundamental - // problem and keeps the exit code. - return regressionErr + // Only compare a run that completed. A partial run's missing evaluations + // register as "absent" and do not gate, so comparing would print + // "✅ No regression against baseline" for a broken run and then exit + // non-zero — contradictory, and the reassuring half is the one people read. + if evalErr != nil { + if f.baseline != "" { + fmt.Fprintln(teeOut, "\nSkipping baseline comparison: the run did not complete.") + } + return evalErr } - return evalErr + return f.checkBaseline(teeOut, run) } // checkBaseline compares run against the configured baseline and returns a @@ -181,7 +191,10 @@ func (f *evalFlags) checkBaseline(out io.Writer, run *evaluation.EvalRun) error return err } - comparison := evaluation.Compare(baseline, run, f.regressionTolerance) + comparison, err := evaluation.Compare(baseline, run, f.regressionTolerance) + if err != nil { + return err + } evaluation.PrintComparison(out, comparison) if comparison.Regressed { diff --git a/pkg/evaluation/baseline.go b/pkg/evaluation/baseline.go index 7105919366..e9046989ff 100644 --- a/pkg/evaluation/baseline.go +++ b/pkg/evaluation/baseline.go @@ -3,6 +3,7 @@ package evaluation import ( "cmp" "encoding/json" + "errors" "fmt" "io" "os" @@ -11,7 +12,7 @@ import ( ) // Metrics is the comparable shape of an evaluation run: the rates a regression -// gate can be built on, derived from the same fields [computeSummary] uses. +// gate can be built on, derived from the same [Summary] the run prints. // // Rates are 0 when their denominator is 0, and the corresponding Has… flag says // whether the rate means anything. Without that distinction "no size @@ -34,13 +35,8 @@ type Metrics struct { TotalCost float64 `json:"total_cost"` } -// MetricsOf derives [Metrics] from a run's results. -func MetricsOf(run *EvalRun) Metrics { - if run == nil { - return Metrics{} - } - s := computeSummary(run.Results) - +// metricsOfSummary derives [Metrics] from a run summary. +func metricsOfSummary(s Summary) Metrics { m := Metrics{ TotalEvals: s.TotalEvals, FailedEvals: s.FailedEvals, @@ -64,6 +60,26 @@ func MetricsOf(run *EvalRun) Metrics { return m } +// MetricsOf derives [Metrics] from a run's results. +func MetricsOf(run *EvalRun) Metrics { + if run == nil { + return Metrics{} + } + return metricsOfSummary(computeSummary(run.Results)) +} + +// Baseline is a previously saved run, reduced to what a regression gate needs. +// +// It is loaded from the JSON the eval command actually writes — a [RunOutput] +// from SaveRunSessionsJSON — rather than from [EvalRun], which has no producer +// outside tests and whose Duration field is typed incompatibly. +type Baseline struct { + Name string + Summary Summary + // Passed maps an evaluation's key (see evalKey) to whether it passed. + Passed map[string]bool +} + // MetricDelta is one metric's movement between two runs. Higher is better for // quality rates and worse for FailureRate, so Regressed — not the sign of Delta // — is what a gate reads. @@ -80,7 +96,7 @@ type MetricDelta struct { // EvalChange records one evaluation's pass/fail transition between runs. type EvalChange struct { - InputPath string `json:"input_path"` + Eval string `json:"eval"` // Was and Now are "pass", "fail", or "absent". Was string `json:"was"` Now string `json:"now"` @@ -99,17 +115,57 @@ type Comparison struct { Regressed bool `json:"regressed"` } -// LoadBaseline reads a run previously written by [SaveRunJSON]. -func LoadBaseline(path string) (*EvalRun, error) { +// MaxTolerance is the largest accepted --regression-tolerance. A rate cannot +// move by more than 1.0, so anything above it silently disables the aggregate +// gate — better rejected at startup than discovered when a regression sails +// through. +const MaxTolerance = 1.0 + +// ErrNoBaselineEvals reports a baseline carrying no evaluations. A gate built on +// one would compare against all-zero metrics and pass unconditionally. +var ErrNoBaselineEvals = errors.New("baseline contains no evaluations") + +// ErrNoCurrentEvals reports a run that produced no evaluations — an --only +// pattern that matched nothing, for instance. There is nothing to gate on. +var ErrNoCurrentEvals = errors.New("run produced no evaluations to compare") + +// ErrNothingComparable reports a baseline and run with no metric and no +// evaluation in common, so the comparison could only ever pass. +var ErrNothingComparable = errors.New("baseline and run share no metric or evaluation to compare") + +// LoadBaseline reads the run JSON the eval command writes. +// +// A file that parses but carries no evaluations is rejected rather than treated +// as an empty baseline: every rate would be skipped by the has-flag guards and +// the gate would report success while every evaluation in the current run +// failed. A gate must fail closed. +func LoadBaseline(path string) (*Baseline, error) { data, err := os.ReadFile(path) if err != nil { return nil, fmt.Errorf("reading baseline %q: %w", path, err) } - var run EvalRun - if err := json.Unmarshal(data, &run); err != nil { + + var output RunOutput + if err := json.Unmarshal(data, &output); err != nil { return nil, fmt.Errorf("parsing baseline %q: %w", path, err) } - return &run, nil + + baseline := &Baseline{ + Name: output.Name, + Summary: output.Summary, + Passed: make(map[string]bool, len(output.Sessions)), + } + for _, sess := range output.Sessions { + if sess == nil || sess.EvalResult == nil { + continue + } + baseline.Passed[sessionEvalKey(sess.InputID, sess.Title)] = sess.EvalResult.Passed + } + + if baseline.Summary.TotalEvals == 0 && len(baseline.Passed) == 0 { + return nil, fmt.Errorf("%w: %q is not an evaluation run written by `docker agent eval`", ErrNoBaselineEvals, path) + } + return baseline, nil } // Compare checks current against baseline. @@ -133,19 +189,25 @@ func LoadBaseline(path string) (*EvalRun, error) { // and gating on it would make the check fire on provider price changes. // // A metric absent from either run is skipped rather than treated as 0 — adding -// the first size expectation to a suite must not look like a regression. -func Compare(baseline, current *EvalRun, tolerance float64) Comparison { - if tolerance < 0 { - tolerance = 0 +// the first size expectation to a suite must not look like a regression. If that +// leaves nothing to gate on and no evaluation in common, an error is returned: +// a comparison that can only ever pass is worse than no comparison. +func Compare(baseline *Baseline, current *EvalRun, tolerance float64) (Comparison, error) { + if baseline == nil { + return Comparison{}, ErrNoBaselineEvals + } + if current == nil || len(current.Results) == 0 { + return Comparison{}, ErrNoCurrentEvals } + tolerance = max(tolerance, 0) c := Comparison{ - Baseline: MetricsOf(baseline), + Baseline: metricsOfSummary(baseline.Summary), Current: MetricsOf(current), Tolerance: tolerance, } - // Quality rates: a drop beyond the tolerance regresses. + gating := 0 for _, q := range []struct { name string base, cur float64 @@ -161,9 +223,9 @@ func Compare(baseline, current *EvalRun, tolerance float64) Comparison { d := MetricDelta{Name: q.name, Baseline: q.base, Current: q.cur, Delta: q.cur - q.base} d.Regressed = q.cur < q.base-tolerance c.Deltas = append(c.Deltas, d) + gating++ } - // Failure rate: a climb beyond the tolerance regresses. if c.Baseline.TotalEvals > 0 && c.Current.TotalEvals > 0 { d := MetricDelta{ Name: "failure rate", @@ -173,6 +235,15 @@ func Compare(baseline, current *EvalRun, tolerance float64) Comparison { } d.Regressed = c.Current.FailureRate > c.Baseline.FailureRate+tolerance c.Deltas = append(c.Deltas, d) + gating++ + } + + c.Changes = compareEvals(baseline, current) + + // Nothing comparable: no shared metric and no shared evaluation. Reporting + // "no regression" here would be a gate that cannot fail. + if gating == 0 && !anyShared(baseline, current) { + return Comparison{}, ErrNothingComparable } c.Deltas = append(c.Deltas, MetricDelta{ @@ -183,8 +254,6 @@ func Compare(baseline, current *EvalRun, tolerance float64) Comparison { Informational: true, }) - c.Changes = compareEvals(baseline, current) - for _, d := range c.Deltas { if d.Regressed && !d.Informational { c.Regressed = true @@ -195,33 +264,48 @@ func Compare(baseline, current *EvalRun, tolerance float64) Comparison { c.Regressed = true } } - return c + return c, nil } -// compareEvals pairs evaluations by input path and reports transitions. Only -// pass → fail is a regression; appearing and disappearing evaluations are -// reported so a reviewer notices a suite change, but do not gate. -func compareEvals(baseline, current *EvalRun) []EvalChange { - was := passByPath(baseline) - now := passByPath(current) +// anyShared reports whether the two runs have an evaluation in common. +func anyShared(baseline *Baseline, current *EvalRun) bool { + for _, r := range current.Results { + if _, ok := baseline.Passed[resultEvalKey(r)]; ok { + return true + } + } + return false +} - paths := make(map[string]struct{}, len(was)+len(now)) - for p := range was { - paths[p] = struct{}{} +// compareEvals pairs evaluations by key and reports transitions. Only pass → +// fail is a regression; appearing and disappearing evaluations are reported so a +// reviewer notices a suite change, but do not gate. +func compareEvals(baseline *Baseline, current *EvalRun) []EvalChange { + now := map[string]bool{} + for _, r := range current.Results { + key := resultEvalKey(r) + // Repeated runs of the same evaluation collapse pessimistically: if any + // repetition failed, it counts as failed. A flaky pass is not a pass. + if prev, seen := now[key]; seen { + now[key] = prev && resultPassed(r) + continue + } + now[key] = resultPassed(r) } - for p := range now { - paths[p] = struct{}{} + + keys := make(map[string]struct{}, len(baseline.Passed)+len(now)) + for k := range baseline.Passed { + keys[k] = struct{}{} + } + for k := range now { + keys[k] = struct{}{} } - changes := make([]EvalChange, 0, len(paths)) - for p := range paths { - bs, inBase := was[p] - cs, inCur := now[p] - change := EvalChange{ - InputPath: p, - Was: passLabel(bs, inBase), - Now: passLabel(cs, inCur), - } + changes := make([]EvalChange, 0, len(keys)) + for k := range keys { + bs, inBase := baseline.Passed[k] + cs, inCur := now[k] + change := EvalChange{Eval: k, Was: passLabel(bs, inBase), Now: passLabel(cs, inCur)} if change.Was == change.Now { continue } @@ -237,26 +321,25 @@ func compareEvals(baseline, current *EvalRun) []EvalChange { } return 1 } - return cmp.Compare(a.InputPath, b.InputPath) + return cmp.Compare(a.Eval, b.Eval) }) return changes } -func passByPath(run *EvalRun) map[string]bool { - out := map[string]bool{} - if run == nil { - return out +// resultEvalKey identifies an evaluation the same way [sessionEvalKey] does for +// the baseline side, so the two runs pair up. The session's InputID is preferred +// because it is the caller-supplied correlation ID; the display title is the +// fallback and carries the "#N" suffix that distinguishes repetitions. +func resultEvalKey(r Result) string { + var inputID string + if r.Session != nil { + inputID = r.Session.InputID } - for _, r := range run.Results { - // Repeated runs of the same input (Config.Repeat > 1) collapse - // pessimistically: if any repetition failed, the eval counts as failed. - if prev, seen := out[r.InputPath]; seen { - out[r.InputPath] = prev && resultPassed(r) - continue - } - out[r.InputPath] = resultPassed(r) - } - return out + return sessionEvalKey(inputID, cmp.Or(r.Title, r.InputPath)) +} + +func sessionEvalKey(inputID, title string) string { + return cmp.Or(inputID, title) } func passLabel(passed, present bool) string { @@ -270,23 +353,15 @@ func passLabel(passed, present bool) string { } } -// resultPassed reports whether one result met every expectation declared for it, -// mirroring the fields [computeSummary] scores. An expectation that was not -// declared is not a failure. +// resultPassed reports whether one result met every expectation declared for it. +// +// It delegates to [Result.checkResults] — the same function that decides the +// PASS/FAIL the eval command prints and the `passed` flag written into the saved +// run. A second definition here would let the gate drift from the product: an +// evaluation whose printed status flipped would be recorded as no change. func resultPassed(r Result) bool { - if r.Error != "" { - return false - } - if r.SizeExpected != "" && r.SizeExpected != r.Size { - return false - } - if r.ToolCallsExpected > 0 && r.ToolCallsScore < r.ToolCallsExpected { - return false - } - if r.RelevanceExpected > 0 && r.RelevancePassed < r.RelevanceExpected { - return false - } - return true + _, failures := r.checkResults() + return len(failures) == 0 } // PrintComparison writes a human-readable comparison. @@ -315,7 +390,7 @@ func PrintComparison(out io.Writer, c Comparison) { if ch.Regressed { marker = "! " } - fmt.Fprintf(ctw, "%s%s\t%s → %s\t\n", marker, ch.InputPath, ch.Was, ch.Now) + fmt.Fprintf(ctw, "%s%s\t%s → %s\t\n", marker, ch.Eval, ch.Was, ch.Now) } _ = ctw.Flush() } From 1ff99fd16756e2338429c4c956a5c9218d0a04b8 Mon Sep 17 00:00:00 2001 From: Dwin Gharibi Date: Sat, 8 Aug 2026 22:04:38 +0330 Subject: [PATCH 4/5] test(evaluation): exercise the real baseline file format Tests now write a run with SaveRunSessionsJSON and read it back through LoadBaseline, so the round trip the command actually performs is covered rather than one that no caller uses. Adds the fail-closed cases (foreign baseline, empty run), pins that resultPassed agrees with checkResults for every expectation kind, and pins the specific divergence the old definition hid: a tool score dropping below 1.0 flips a printed PASS to FAIL and must gate even inside the tolerance. --- cmd/root/eval_baseline_test.go | 81 ++++--- pkg/evaluation/baseline_test.go | 401 ++++++++++++++++++-------------- 2 files changed, 271 insertions(+), 211 deletions(-) diff --git a/cmd/root/eval_baseline_test.go b/cmd/root/eval_baseline_test.go index 7699331495..efa4b017a1 100644 --- a/cmd/root/eval_baseline_test.go +++ b/cmd/root/eval_baseline_test.go @@ -2,6 +2,7 @@ package root import ( "bytes" + "os" "path/filepath" "testing" @@ -9,16 +10,33 @@ import ( "github.com/stretchr/testify/require" "github.com/docker/docker-agent/pkg/evaluation" + "github.com/docker/docker-agent/pkg/session" ) +// sizeRun builds a run in the shape the eval command produces, including the +// session that carries the saved pass/fail flag. func sizeRun(pass bool) *evaluation.EvalRun { - r := evaluation.Result{InputPath: "a", SizeExpected: "medium", Size: "medium"} + r := evaluation.Result{ + InputPath: "a.json", + Title: "a", + SizeExpected: "medium", + Size: "medium", + Session: &session.Session{Title: "a"}, + } if !pass { r.Size = "short" } return &evaluation.EvalRun{Name: "run", Results: []evaluation.Result{r}} } +// saveBaseline writes a run exactly as `docker agent eval` does. +func saveBaseline(t *testing.T, run *evaluation.EvalRun) string { + t.Helper() + path, err := evaluation.SaveRunSessionsJSON(run, t.TempDir()) + require.NoError(t, err) + return path +} + func TestCheckBaseline_NoBaselineIsANoOp(t *testing.T) { t.Parallel() @@ -31,13 +49,9 @@ func TestCheckBaseline_NoBaselineIsANoOp(t *testing.T) { func TestCheckBaseline_RegressionReturnsAnError(t *testing.T) { t.Parallel() - dir := t.TempDir() - path, err := evaluation.SaveRunJSON(sizeRun(true), dir) - require.NoError(t, err) - - f := &evalFlags{baseline: path} + f := &evalFlags{baseline: saveBaseline(t, sizeRun(true))} var buf bytes.Buffer - err = f.checkBaseline(&buf, sizeRun(false)) + err := f.checkBaseline(&buf, sizeRun(false)) require.Error(t, err, "a regression must surface as a non-zero exit") assert.Contains(t, err.Error(), "regressed against baseline") @@ -47,39 +61,27 @@ func TestCheckBaseline_RegressionReturnsAnError(t *testing.T) { func TestCheckBaseline_NoRegressionSucceeds(t *testing.T) { t.Parallel() - dir := t.TempDir() - path, err := evaluation.SaveRunJSON(sizeRun(true), dir) - require.NoError(t, err) - - f := &evalFlags{baseline: path} + f := &evalFlags{baseline: saveBaseline(t, sizeRun(true))} var buf bytes.Buffer require.NoError(t, f.checkBaseline(&buf, sizeRun(true))) assert.Contains(t, buf.String(), "No regression against baseline") } -func TestCheckBaseline_ToleranceIsPlumbedThrough(t *testing.T) { +// The gate must fail closed rather than reporting success against a baseline it +// cannot actually compare with. +func TestCheckBaseline_FailsClosedOnAnUnusableBaseline(t *testing.T) { t.Parallel() - dir := t.TempDir() - base := &evaluation.EvalRun{Name: "b", Results: []evaluation.Result{ - {InputPath: "a", ToolCallsExpected: 0.8, ToolCallsScore: 1.0}, - {InputPath: "b", ToolCallsExpected: 0.8, ToolCallsScore: 1.0}, - }} - cur := &evaluation.EvalRun{Name: "c", Results: []evaluation.Result{ - {InputPath: "a", ToolCallsExpected: 0.8, ToolCallsScore: 1.0}, - {InputPath: "b", ToolCallsExpected: 0.8, ToolCallsScore: 0.9}, - }} - - path, err := evaluation.SaveRunJSON(base, dir) - require.NoError(t, err) + path := filepath.Join(t.TempDir(), "not-a-run.json") + require.NoError(t, os.WriteFile(path, []byte(`{"name":"not-an-eval-run"}`), 0o600)) + f := &evalFlags{baseline: path} var buf bytes.Buffer - strict := &evalFlags{baseline: path, regressionTolerance: 0.01} - require.Error(t, strict.checkBaseline(&buf, cur), "a tight tolerance gates the 0.05 drop") + err := f.checkBaseline(&buf, sizeRun(false)) - buf.Reset() - lenient := &evalFlags{baseline: path, regressionTolerance: 0.10} - require.NoError(t, lenient.checkBaseline(&buf, cur), "a wider tolerance absorbs it") + require.ErrorIs(t, err, evaluation.ErrNoBaselineEvals) + assert.NotContains(t, buf.String(), "No regression", + "an unusable baseline must never print a reassuring verdict") } func TestCheckBaseline_MissingBaselineFileIsAnError(t *testing.T) { @@ -104,3 +106,22 @@ func TestEvalCmd_BaselineFlagsAreRegistered(t *testing.T) { require.NotNil(t, tolerance) assert.Equal(t, "0", tolerance.DefValue, "the default gates any drop") } + +// A tolerance above 1 cannot be met by any rate movement, so it silently +// disables the aggregate gate. Rejecting it is a startup error, not a surprise +// discovered when a regression sails through. +func TestEvalCmd_RejectsTooLargeTolerance(t *testing.T) { + t.Parallel() + + cmd := newEvalCmd() + require.NoError(t, cmd.Flags().Set("regression-tolerance", "10")) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + err := cmd.Args(cmd, []string{"agent.yaml"}) + require.NoError(t, err) + + err = cmd.RunE(cmd, []string{"agent.yaml"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "--regression-tolerance must be between 0 and 1") +} diff --git a/pkg/evaluation/baseline_test.go b/pkg/evaluation/baseline_test.go index 2533497c1e..3163909275 100644 --- a/pkg/evaluation/baseline_test.go +++ b/pkg/evaluation/baseline_test.go @@ -9,65 +9,134 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/session" ) // sizeResult is a result whose only declared expectation is response size. -func sizeResult(path string, pass bool) Result { - r := Result{InputPath: path, SizeExpected: "medium", Size: "medium"} +// Session is populated because that is what SaveRunSessionsJSON serialises, and +// what LoadBaseline reads back. +func sizeResult(title string, pass bool) Result { + r := Result{ + InputPath: title + ".json", + Title: title, + SizeExpected: "medium", + Size: "medium", + Session: &session.Session{Title: title}, + } if !pass { + r.Session.Title = title r.Size = "short" } return r } -func run(results ...Result) *EvalRun { - return &EvalRun{Name: "run", Results: results} +// toolResult declares a tool-call expectation. checkResults requires a score of +// 1.0 to pass, regardless of the declared expectation value. +func toolResult(title string, score float64) Result { + return Result{ + InputPath: title + ".json", + Title: title, + ToolCallsExpected: 1, + ToolCallsScore: score, + Session: &session.Session{Title: title}, + } +} + +func newRun(results ...Result) *EvalRun { + run := &EvalRun{Name: "run", Results: results} + run.Summary = computeSummary(run.Results) + return run } -func TestMetricsOf_RatesAndFlags(t *testing.T) { +// saveAndLoad writes a run exactly as the eval command does and reads it back as +// a baseline, so every test exercises the real file format. +func saveAndLoad(t *testing.T, run *EvalRun) *Baseline { + t.Helper() + for i := range run.Results { + populateEvalResult(&run.Results[i]) + } + path, err := SaveRunSessionsJSON(run, t.TempDir()) + require.NoError(t, err) + + baseline, err := LoadBaseline(path) + require.NoError(t, err) + return baseline +} + +// The file the eval command writes is a RunOutput, not an EvalRun. Loading the +// wrong shape failed outright on Duration (string vs time.Duration), which meant +// --baseline could not read anything the tool produced. +func TestLoadBaseline_ReadsWhatTheEvalCommandWrites(t *testing.T) { t.Parallel() - got := MetricsOf(run( - sizeResult("a", true), - sizeResult("b", false), - Result{InputPath: "c", Error: "boom"}, - Result{InputPath: "d", ToolCallsExpected: 1, ToolCallsScore: 0.5, Cost: 0.25}, - Result{InputPath: "e", RelevanceExpected: 2, RelevancePassed: 1}, - )) - - assert.Equal(t, 5, got.TotalEvals) - assert.Equal(t, 1, got.FailedEvals) - assert.InDelta(t, 0.2, got.FailureRate, 1e-9) - assert.True(t, got.HasSizes) - assert.InDelta(t, 0.5, got.SizePassRate, 1e-9) - assert.True(t, got.HasTools) - assert.InDelta(t, 0.5, got.ToolsF1Mean, 1e-9) - assert.True(t, got.HasRelevance) - assert.InDelta(t, 0.5, got.RelevanceRate, 1e-9) - assert.InDelta(t, 0.25, got.TotalCost, 1e-9) + run := newRun(sizeResult("a", true), sizeResult("b", false)) + for i := range run.Results { + populateEvalResult(&run.Results[i]) + } + path, err := SaveRunSessionsJSON(run, t.TempDir()) + require.NoError(t, err) + + baseline, err := LoadBaseline(path) + require.NoError(t, err) + + assert.Equal(t, 2, baseline.Summary.TotalEvals) + assert.Equal(t, map[string]bool{"a": true, "b": false}, baseline.Passed) } -// A rate with no denominator must be distinguishable from a rate of 0.0, or a -// gate cannot tell "nothing declared" from "everything failed". -func TestMetricsOf_AbsentMetricsAreFlaggedNotZero(t *testing.T) { +// A gate must fail closed: a file that parses but carries no evaluations would +// compare against all-zero metrics and pass unconditionally. +func TestLoadBaseline_RejectsAFileThatIsNotAnEvalRun(t *testing.T) { t.Parallel() - got := MetricsOf(run(Result{InputPath: "a"})) - assert.False(t, got.HasSizes) - assert.False(t, got.HasTools) - assert.False(t, got.HasRelevance) - assert.Zero(t, got.SizePassRate) + path := filepath.Join(t.TempDir(), "not-a-run.json") + require.NoError(t, os.WriteFile(path, []byte(`{"name":"not-an-eval-run"}`), 0o600)) - assert.Equal(t, Metrics{}, MetricsOf(nil), "a nil run yields zero metrics, not a panic") + _, err := LoadBaseline(path) + require.ErrorIs(t, err, ErrNoBaselineEvals) +} + +func TestLoadBaseline_Errors(t *testing.T) { + t.Parallel() + + _, err := LoadBaseline(filepath.Join(t.TempDir(), "missing.json")) + require.Error(t, err) + assert.Contains(t, err.Error(), "reading baseline") + + bad := filepath.Join(t.TempDir(), "bad.json") + require.NoError(t, os.WriteFile(bad, []byte("{not json"), 0o600)) + _, err = LoadBaseline(bad) + require.Error(t, err) + assert.Contains(t, err.Error(), "parsing baseline") +} + +// A run with no evaluations (an --only pattern that matched nothing) has nothing +// to gate on and must not report success. +func TestCompare_RejectsAnEmptyCurrentRun(t *testing.T) { + t.Parallel() + + baseline := saveAndLoad(t, newRun(sizeResult("a", true))) + + _, err := Compare(baseline, newRun(), 0) + require.ErrorIs(t, err, ErrNoCurrentEvals) + + _, err = Compare(baseline, nil, 0) + require.ErrorIs(t, err, ErrNoCurrentEvals) +} + +func TestCompare_RejectsANilBaseline(t *testing.T) { + t.Parallel() + _, err := Compare(nil, newRun(sizeResult("a", true)), 0) + require.ErrorIs(t, err, ErrNoBaselineEvals) } func TestCompare_NoChangeIsNotARegression(t *testing.T) { t.Parallel() - base := run(sizeResult("a", true), sizeResult("b", true)) - cur := run(sizeResult("a", true), sizeResult("b", true)) + baseline := saveAndLoad(t, newRun(sizeResult("a", true), sizeResult("b", true))) - got := Compare(base, cur, 0) + got, err := Compare(baseline, newRun(sizeResult("a", true), sizeResult("b", true)), 0) + require.NoError(t, err) assert.False(t, got.Regressed) assert.Empty(t, got.Changes) } @@ -75,10 +144,10 @@ func TestCompare_NoChangeIsNotARegression(t *testing.T) { func TestCompare_QualityDropRegresses(t *testing.T) { t.Parallel() - base := run(sizeResult("a", true), sizeResult("b", true)) - cur := run(sizeResult("a", true), sizeResult("b", false)) + baseline := saveAndLoad(t, newRun(sizeResult("a", true), sizeResult("b", true))) - got := Compare(base, cur, 0) + got, err := Compare(baseline, newRun(sizeResult("a", true), sizeResult("b", false)), 0) + require.NoError(t, err) require.True(t, got.Regressed) var sizeDelta *MetricDelta @@ -92,64 +161,72 @@ func TestCompare_QualityDropRegresses(t *testing.T) { assert.InDelta(t, -0.5, sizeDelta.Delta, 1e-9) } -// Judge variance shows up as small movement in the aggregate rates; the -// tolerance exists so a build is not failed by that noise. -// -// Both evaluations stay above their declared expectation (0.8) here, so no -// pass/fail transition occurs and the tolerance is what decides — see -// TestCompare_PassToFailGatesRegardlessOfTolerance for the other half. -func TestCompare_ToleranceAbsorbsSmallDrops(t *testing.T) { +// resultPassed must agree with what the run prints and saves. checkResults +// requires a tool-call score of 1.0; a second definition keyed on +// ToolCallsExpected would call 0.9 a pass and record a printed FAIL as no change. +func TestResultPassed_MatchesCheckResults(t *testing.T) { t.Parallel() - base := run( - Result{InputPath: "a", ToolCallsExpected: 0.8, ToolCallsScore: 1.0}, - Result{InputPath: "b", ToolCallsExpected: 0.8, ToolCallsScore: 1.0}, - ) - cur := run( - Result{InputPath: "a", ToolCallsExpected: 0.8, ToolCallsScore: 1.0}, - Result{InputPath: "b", ToolCallsExpected: 0.8, ToolCallsScore: 0.9}, - ) - - // Mean drops 1.00 → 0.95, and both evaluations still pass. - assert.False(t, Compare(base, cur, 0.10).Regressed, "within tolerance") - assert.True(t, Compare(base, cur, 0.01).Regressed, "beyond tolerance") - assert.True(t, Compare(base, cur, 0).Regressed, "zero tolerance gates any drop") - assert.True(t, Compare(base, cur, -5).Regressed, - "a negative tolerance is clamped to 0, so this still gates") + for _, r := range []Result{ + {Title: "none"}, + {Title: "err", Error: "boom"}, + {Title: "size-ok", SizeExpected: "short", Size: "short"}, + {Title: "size-bad", SizeExpected: "short", Size: "long"}, + {Title: "tool-perfect", ToolCallsExpected: 0.8, ToolCallsScore: 1.0}, + {Title: "tool-short", ToolCallsExpected: 0.8, ToolCallsScore: 0.9}, + {Title: "rel-ok", RelevanceExpected: 2, RelevancePassed: 2}, + {Title: "rel-bad", RelevanceExpected: 2, RelevancePassed: 1}, + } { + _, failures := r.checkResults() + assert.Equalf(t, len(failures) == 0, resultPassed(r), + "resultPassed disagrees with checkResults for %q (failures=%v)", r.Title, failures) + } } -// The tolerance governs aggregate rates only. An evaluation that passed and now -// fails is the exact signal a regression gate exists to catch, so it gates even -// when the aggregate movement is small enough to be absorbed. -func TestCompare_PassToFailGatesRegardlessOfTolerance(t *testing.T) { +// The specific divergence the old definition hid: a printed PASS → FAIL flip +// that was recorded as no change and then absorbed by the tolerance. +func TestCompare_ToolScoreDropBelowOneIsARegression(t *testing.T) { t.Parallel() - base := run( - Result{InputPath: "a", ToolCallsExpected: 1, ToolCallsScore: 1.0}, - Result{InputPath: "b", ToolCallsExpected: 1, ToolCallsScore: 1.0}, - ) - cur := run( - Result{InputPath: "a", ToolCallsExpected: 1, ToolCallsScore: 1.0}, - Result{InputPath: "b", ToolCallsExpected: 1, ToolCallsScore: 0.9}, // now below expectation - ) + baseline := saveAndLoad(t, newRun(toolResult("a", 1.0))) - got := Compare(base, cur, 0.99) // a tolerance far larger than the rate movement - assert.True(t, got.Regressed, "a pass → fail transition is not absorbed by the tolerance") + got, err := Compare(baseline, newRun(toolResult("a", 0.9)), 0.2) + require.NoError(t, err) require.Len(t, got.Changes, 1) - assert.Equal(t, "b", got.Changes[0].InputPath) - assert.True(t, got.Changes[0].Regressed) + assert.Equal(t, "pass", got.Changes[0].Was) + assert.Equal(t, "fail", got.Changes[0].Now) + assert.True(t, got.Regressed, "a printed pass → fail must gate even inside the tolerance") +} + +// Judge variance shows up as small movement in the aggregate rates; the +// tolerance exists so a build is not failed by that noise. Both evaluations stay +// passing here, so only the aggregate moves. +func TestCompare_ToleranceAbsorbsSmallDrops(t *testing.T) { + t.Parallel() + + baseline := saveAndLoad(t, newRun(toolResult("a", 1.0), toolResult("b", 1.0))) + current := newRun(toolResult("a", 1.0), toolResult("b", 1.0)) + // Nudge the aggregate without flipping a pass: F1 mean 1.00 → 0.95 is not + // expressible while both pass, so assert the no-movement case instead. + got, err := Compare(baseline, current, 0) + require.NoError(t, err) + assert.False(t, got.Regressed) + + // A real drop below 1.0 flips the eval and gates regardless of tolerance. + got, err = Compare(baseline, newRun(toolResult("a", 1.0), toolResult("b", 0.99)), MaxTolerance) + require.NoError(t, err) + assert.True(t, got.Regressed) } func TestCompare_ImprovementIsNotARegression(t *testing.T) { t.Parallel() - base := run(sizeResult("a", false), sizeResult("b", false)) - cur := run(sizeResult("a", true), sizeResult("b", true)) + baseline := saveAndLoad(t, newRun(sizeResult("a", false), sizeResult("b", false))) - got := Compare(base, cur, 0) + got, err := Compare(baseline, newRun(sizeResult("a", true), sizeResult("b", true)), 0) + require.NoError(t, err) assert.False(t, got.Regressed) - require.NotEmpty(t, got.Changes) for _, ch := range got.Changes { assert.False(t, ch.Regressed, "fail → pass must never gate") } @@ -158,10 +235,13 @@ func TestCompare_ImprovementIsNotARegression(t *testing.T) { func TestCompare_FailureRateClimbRegresses(t *testing.T) { t.Parallel() - base := run(Result{InputPath: "a"}, Result{InputPath: "b"}) - cur := run(Result{InputPath: "a"}, Result{InputPath: "b", Error: "boom"}) + baseline := saveAndLoad(t, newRun(sizeResult("a", true), sizeResult("b", true))) - got := Compare(base, cur, 0) + current := newRun(sizeResult("a", true), Result{ + InputPath: "b.json", Title: "b", Error: "boom", Session: &session.Session{Title: "b"}, + }) + got, err := Compare(baseline, current, 0) + require.NoError(t, err) assert.True(t, got.Regressed) } @@ -169,10 +249,15 @@ func TestCompare_FailureRateClimbRegresses(t *testing.T) { func TestCompare_CostIsInformationalOnly(t *testing.T) { t.Parallel() - base := run(Result{InputPath: "a", Cost: 0.01}) - cur := run(Result{InputPath: "a", Cost: 100}) + baseline := saveAndLoad(t, newRun(Result{ + InputPath: "a.json", Title: "a", Cost: 0.01, Session: &session.Session{Title: "a"}, + })) + current := newRun(Result{ + InputPath: "a.json", Title: "a", Cost: 100, Session: &session.Session{Title: "a"}, + }) - got := Compare(base, cur, 0) + got, err := Compare(baseline, current, 0) + require.NoError(t, err) assert.False(t, got.Regressed, "a cost increase alone must not fail the gate") var cost *MetricDelta @@ -186,116 +271,76 @@ func TestCompare_CostIsInformationalOnly(t *testing.T) { assert.InDelta(t, 99.99, cost.Delta, 1e-6) } -// Adding the first expectation of a kind must not read as a regression from 0. -func TestCompare_MetricAbsentFromOneSideIsSkipped(t *testing.T) { - t.Parallel() - - base := run(Result{InputPath: "a"}) // no size expectations - cur := run(sizeResult("a", false)) // size expectation newly added, failing - - got := Compare(base, cur, 0) - for _, d := range got.Deltas { - assert.NotEqual(t, "size pass rate", d.Name, - "a metric with no baseline must be skipped, not compared against 0") - } -} - -// An evaluation appearing or disappearing is a suite change, not a regression of -// existing behaviour, so the per-eval transition itself never gates. func TestCompare_AddedAndRemovedEvalsDoNotGateOnTheirOwn(t *testing.T) { t.Parallel() - base := run(sizeResult("a", true), sizeResult("gone", true)) - cur := run(sizeResult("a", true), sizeResult("new", true)) + baseline := saveAndLoad(t, newRun(sizeResult("a", true), sizeResult("gone", true))) - got := Compare(base, cur, 0) + got, err := Compare(baseline, newRun(sizeResult("a", true), sizeResult("new", true)), 0) + require.NoError(t, err) - byPath := map[string]EvalChange{} + byKey := map[string]EvalChange{} for _, ch := range got.Changes { - byPath[ch.InputPath] = ch + byKey[ch.Eval] = ch } - require.Contains(t, byPath, "gone") - require.Contains(t, byPath, "new") - assert.Equal(t, "absent", byPath["gone"].Now) - assert.Equal(t, "absent", byPath["new"].Was) - assert.False(t, byPath["gone"].Regressed, "a removed eval is a suite change, not a regression") - assert.False(t, byPath["new"].Regressed, "an added eval is a suite change, not a regression") - assert.False(t, got.Regressed, "the aggregate rate is unchanged, so nothing gates") + require.Contains(t, byKey, "gone") + require.Contains(t, byKey, "new") + assert.False(t, byKey["gone"].Regressed, "a removed eval is a suite change, not a regression") + assert.False(t, byKey["new"].Regressed, "an added eval is a suite change, not a regression") + assert.False(t, got.Regressed) } -// But an added evaluation that FAILS does lower the aggregate rate, and that -// gates — which is the desirable outcome: a suite that got worse should say so, -// even though no individual evaluation went from passing to failing. +// An added failing evaluation lowers the aggregate rate, and that gates — which +// is desirable: a suite that got worse should say so. func TestCompare_AddedFailingEvalGatesViaTheAggregateRate(t *testing.T) { t.Parallel() - base := run(sizeResult("a", true)) - cur := run(sizeResult("a", true), sizeResult("new", false)) + baseline := saveAndLoad(t, newRun(sizeResult("a", true))) - got := Compare(base, cur, 0) + got, err := Compare(baseline, newRun(sizeResult("a", true), sizeResult("new", false)), 0) + require.NoError(t, err) assert.True(t, got.Regressed, "size pass rate fell 1.00 → 0.50") - - for _, ch := range got.Changes { - if ch.InputPath == "new" { - assert.False(t, ch.Regressed, - "the gate comes from the aggregate, not from the added eval's transition") - } - } } -func TestCompare_RegressionsAreListedFirst(t *testing.T) { +func TestCompare_RepeatedEvalCollapsesPessimistically(t *testing.T) { t.Parallel() - base := run(sizeResult("aaa", true), sizeResult("zzz", true)) - cur := run(sizeResult("aaa", true), sizeResult("zzz", false), sizeResult("bbb", true)) - - got := Compare(base, cur, 0) - require.NotEmpty(t, got.Changes) - assert.Equal(t, "zzz", got.Changes[0].InputPath, "the regression must lead") - assert.True(t, got.Changes[0].Regressed) -} - -// Config.Repeat runs the same input more than once; one bad repetition means the -// evaluation is not reliably passing. -func TestCompare_RepeatedInputCollapsesPessimistically(t *testing.T) { - t.Parallel() + baseline := saveAndLoad(t, newRun(sizeResult("a", true))) - base := run(sizeResult("a", true), sizeResult("a", true)) - cur := run(sizeResult("a", true), sizeResult("a", false)) + // Two repetitions under one key, one of which failed. + got, err := Compare(baseline, newRun(sizeResult("a", true), sizeResult("a", false)), 0) + require.NoError(t, err) - got := Compare(base, cur, 0) require.Len(t, got.Changes, 1) assert.Equal(t, "pass", got.Changes[0].Was) assert.Equal(t, "fail", got.Changes[0].Now) assert.True(t, got.Changes[0].Regressed) } -func TestLoadBaseline_RoundTripsSaveRunJSON(t *testing.T) { +func TestCompare_RegressionsAreListedFirst(t *testing.T) { t.Parallel() - dir := t.TempDir() - original := run(sizeResult("a", true), Result{InputPath: "b", Error: "boom"}) - path, err := SaveRunJSON(original, dir) - require.NoError(t, err) + baseline := saveAndLoad(t, newRun(sizeResult("aaa", true), sizeResult("zzz", true))) - loaded, err := LoadBaseline(path) + got, err := Compare(baseline, + newRun(sizeResult("aaa", true), sizeResult("zzz", false), sizeResult("bbb", true)), 0) require.NoError(t, err) - assert.Equal(t, MetricsOf(original), MetricsOf(loaded), - "a saved run must be loadable as a baseline with identical metrics") + + require.NotEmpty(t, got.Changes) + assert.Equal(t, "zzz", got.Changes[0].Eval, "the regression must lead") + assert.True(t, got.Changes[0].Regressed) } -func TestLoadBaseline_Errors(t *testing.T) { +func TestMetricsOf_AbsentMetricsAreFlaggedNotZero(t *testing.T) { t.Parallel() - _, err := LoadBaseline(filepath.Join(t.TempDir(), "missing.json")) - require.Error(t, err) - assert.Contains(t, err.Error(), "reading baseline") + got := MetricsOf(newRun(Result{InputPath: "a", Title: "a"})) + assert.False(t, got.HasSizes) + assert.False(t, got.HasTools) + assert.False(t, got.HasRelevance) + assert.Zero(t, got.SizePassRate) - bad := filepath.Join(t.TempDir(), "bad.json") - require.NoError(t, os.WriteFile(bad, []byte("{not json"), 0o600)) - _, err = LoadBaseline(bad) - require.Error(t, err) - assert.Contains(t, err.Error(), "parsing baseline") + assert.Equal(t, Metrics{}, MetricsOf(nil), "a nil run yields zero metrics, not a panic") } func TestPrintComparison(t *testing.T) { @@ -303,12 +348,12 @@ func TestPrintComparison(t *testing.T) { t.Run("regression", func(t *testing.T) { t.Parallel() + baseline := saveAndLoad(t, newRun(sizeResult("a", true))) + c, err := Compare(baseline, newRun(sizeResult("a", false)), 0) + require.NoError(t, err) + var buf bytes.Buffer - PrintComparison(&buf, Compare( - run(sizeResult("a", true)), - run(sizeResult("a", false)), - 0, - )) + PrintComparison(&buf, c) out := buf.String() assert.Contains(t, out, "Regression against baseline") assert.Contains(t, out, "size pass rate") @@ -317,8 +362,12 @@ func TestPrintComparison(t *testing.T) { t.Run("clean", func(t *testing.T) { t.Parallel() + baseline := saveAndLoad(t, newRun(sizeResult("a", true))) + c, err := Compare(baseline, newRun(sizeResult("a", true)), 0) + require.NoError(t, err) + var buf bytes.Buffer - PrintComparison(&buf, Compare(run(sizeResult("a", true)), run(sizeResult("a", true)), 0)) + PrintComparison(&buf, c) assert.Contains(t, buf.String(), "No regression against baseline") }) } @@ -326,7 +375,10 @@ func TestPrintComparison(t *testing.T) { func TestComparison_IsJSONSerializable(t *testing.T) { t.Parallel() - c := Compare(run(sizeResult("a", true)), run(sizeResult("a", false)), 0.05) + baseline := saveAndLoad(t, newRun(sizeResult("a", true))) + c, err := Compare(baseline, newRun(sizeResult("a", false)), 0.05) + require.NoError(t, err) + data, err := json.Marshal(c) require.NoError(t, err) @@ -335,16 +387,3 @@ func TestComparison_IsJSONSerializable(t *testing.T) { assert.True(t, round.Regressed) assert.InDelta(t, 0.05, round.Tolerance, 1e-9) } - -func TestResultPassed(t *testing.T) { - t.Parallel() - - assert.True(t, resultPassed(Result{InputPath: "a"}), "no expectations declared means nothing to fail") - assert.False(t, resultPassed(Result{Error: "boom"})) - assert.True(t, resultPassed(Result{SizeExpected: "short", Size: "short"})) - assert.False(t, resultPassed(Result{SizeExpected: "short", Size: "long"})) - assert.True(t, resultPassed(Result{ToolCallsExpected: 0.8, ToolCallsScore: 0.9})) - assert.False(t, resultPassed(Result{ToolCallsExpected: 0.8, ToolCallsScore: 0.7})) - assert.True(t, resultPassed(Result{RelevanceExpected: 2, RelevancePassed: 2})) - assert.False(t, resultPassed(Result{RelevanceExpected: 2, RelevancePassed: 1})) -} From c72713ec682d2cc517448ef48695012e4cfd147c Mon Sep 17 00:00:00 2001 From: Dwin Gharibi Date: Sat, 8 Aug 2026 22:04:38 +0330 Subject: [PATCH 5/5] docs(evaluation): document the regression gate and its two flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both flag tables enumerated every eval flag except the new two. Adds them, plus the four rules that decide a verdict — tolerance governs aggregates only, a pass to fail always gates, cost never gates, and an added failing evaluation gates via the aggregate — which previously lived only in a doc comment where someone wiring up CI would not see them. --- docs/features/cli/index.md | 2 ++ docs/features/evaluation/index.md | 33 +++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/docs/features/cli/index.md b/docs/features/cli/index.md index 23cac6ecfe..d0e4b80da8 100644 --- a/docs/features/cli/index.md +++ b/docs/features/cli/index.md @@ -466,6 +466,8 @@ $ docker agent eval | [|./evals] [flags] | `--keep-containers` | `false` | Keep containers after evaluation (don't remove with `--rm`) | | `-e, --env` | (none) | Environment variables to pass to container (`KEY` or `KEY=VALUE`, repeatable) | | `--repeat ` | `1` | Number of times to repeat each evaluation (useful for computing baselines) | +| `--baseline ` | (none) | Compare against a previously saved run JSON (`/.json`) and exit non-zero on regression | +| `--regression-tolerance ` | `0` | How far an aggregate quality rate may fall before `--baseline` reports a regression (0–1) | All [runtime configuration flags](#runtime-configuration-flags) are also accepted. diff --git a/docs/features/evaluation/index.md b/docs/features/evaluation/index.md index 03c36a9fc1..e9c6fed4ee 100644 --- a/docs/features/evaluation/index.md +++ b/docs/features/evaluation/index.md @@ -168,6 +168,39 @@ $ docker agent eval | [|./evals] | `--keep-containers` | `false` | Keep containers after evaluation (don't remove with `--rm`) | | `-e, --env` | (none) | Environment variables to pass to container (`KEY` or `KEY=VALUE`) | | `--repeat` | `1` | Number of times to repeat each evaluation (useful for computing baselines) | +| `--baseline` | (none) | Compare against a previously saved run JSON and exit non-zero on regression (see [Regression gate](#regression-gate)) | +| `--regression-tolerance` | `0` | How far an aggregate quality rate may fall before `--baseline` reports a regression (0–1) | + +### Regression gate + +`--baseline` compares the run against a previous one and exits non-zero when +quality regressed, so an eval suite can gate CI: + +```console +$ docker agent eval ./agent.yaml --baseline results/2026-08-01-run.json +``` + +The baseline is the run JSON written by a previous invocation — +`/.json` — so there is no separate artifact to produce. + +Four rules decide the verdict, and they are worth knowing before wiring this +into CI: + +- **The tolerance governs aggregate rates only.** An LLM judge does not return + the same score twice, so without a tolerance the gate flaps. `--regression-tolerance 0.05` + lets an aggregate rate fall five points before it counts. +- **An evaluation that passed and now fails always gates**, regardless of the + tolerance. That transition is the signal the gate exists to catch, so it is + never absorbed. +- **Cost is reported but never gates.** A provider price change is not a quality + regression. +- **An added *failing* evaluation gates** via the aggregate rate, even though no + existing evaluation regressed. A suite that got worse should say so — but it + means committing a known-failing eval needs a tolerance bump or a fix. + +A baseline that carries no evaluations, or a run that produced none (an +`--only` pattern that matched nothing), is rejected rather than reported as +passing: a gate that cannot fail is worse than no gate. ### Provider Credentials