diff --git a/cmd/root/eval.go b/cmd/root/eval.go index 487c4b9353..2ec3fc0dd1 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,11 +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 (/.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) @@ -149,5 +164,41 @@ func (f *evalFlags) runEvalCommand(cmd *cobra.Command, args []string) (commandEr fmt.Fprintf(teeOut, "Log: %s\n", logPath) - return evalErr + // 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 f.checkBaseline(teeOut, run) +} + +// 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, err := evaluation.Compare(baseline, run, f.regressionTolerance) + if err != nil { + return err + } + evaluation.PrintComparison(out, comparison) + + if comparison.Regressed { + return errors.New("evaluation regressed against baseline") + } + return nil } diff --git a/cmd/root/eval_baseline_test.go b/cmd/root/eval_baseline_test.go new file mode 100644 index 0000000000..efa4b017a1 --- /dev/null +++ b/cmd/root/eval_baseline_test.go @@ -0,0 +1,127 @@ +package root + +import ( + "bytes" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "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.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() + + 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() + + f := &evalFlags{baseline: saveBaseline(t, sizeRun(true))} + 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() + + 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") +} + +// 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() + + 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 + err := f.checkBaseline(&buf, sizeRun(false)) + + 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) { + 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") +} + +// 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/docs/features/cli/index.md b/docs/features/cli/index.md index 688823172f..3cb25e3607 100644 --- a/docs/features/cli/index.md +++ b/docs/features/cli/index.md @@ -468,6 +468,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 diff --git a/pkg/evaluation/baseline.go b/pkg/evaluation/baseline.go new file mode 100644 index 0000000000..e9046989ff --- /dev/null +++ b/pkg/evaluation/baseline.go @@ -0,0 +1,403 @@ +package evaluation + +import ( + "cmp" + "encoding/json" + "errors" + "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 [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 +// 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"` +} + +// metricsOfSummary derives [Metrics] from a run summary. +func metricsOfSummary(s Summary) Metrics { + 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 +} + +// 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. +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 { + Eval string `json:"eval"` + // 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"` +} + +// 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 output RunOutput + if err := json.Unmarshal(data, &output); err != nil { + return nil, fmt.Errorf("parsing baseline %q: %w", path, err) + } + + 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. +// +// 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. 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: metricsOfSummary(baseline.Summary), + Current: MetricsOf(current), + Tolerance: tolerance, + } + + gating := 0 + 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) + gating++ + } + + 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) + 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{ + Name: "total cost", + Baseline: c.Baseline.TotalCost, + Current: c.Current.TotalCost, + Delta: c.Current.TotalCost - c.Baseline.TotalCost, + Informational: true, + }) + + 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, nil +} + +// 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 +} + +// 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) + } + + 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(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 + } + 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.Eval, b.Eval) + }) + return changes +} + +// 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 + } + 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 { + switch { + case !present: + return "absent" + case passed: + return "pass" + default: + return "fail" + } +} + +// 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 { + _, failures := r.checkResults() + return len(failures) == 0 +} + +// 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.Eval, 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") +} diff --git a/pkg/evaluation/baseline_test.go b/pkg/evaluation/baseline_test.go new file mode 100644 index 0000000000..3163909275 --- /dev/null +++ b/pkg/evaluation/baseline_test.go @@ -0,0 +1,389 @@ +package evaluation + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "testing" + + "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. +// 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 +} + +// 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 +} + +// 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() + + 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 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() + + path := filepath.Join(t.TempDir(), "not-a-run.json") + require.NoError(t, os.WriteFile(path, []byte(`{"name":"not-an-eval-run"}`), 0o600)) + + _, 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() + + baseline := saveAndLoad(t, newRun(sizeResult("a", true), sizeResult("b", true))) + + 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) +} + +func TestCompare_QualityDropRegresses(t *testing.T) { + t.Parallel() + + baseline := saveAndLoad(t, newRun(sizeResult("a", true), sizeResult("b", true))) + + got, err := Compare(baseline, newRun(sizeResult("a", true), sizeResult("b", false)), 0) + require.NoError(t, err) + 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) +} + +// 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() + + 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 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() + + baseline := saveAndLoad(t, newRun(toolResult("a", 1.0))) + + got, err := Compare(baseline, newRun(toolResult("a", 0.9)), 0.2) + require.NoError(t, err) + + 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.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() + + baseline := saveAndLoad(t, newRun(sizeResult("a", false), sizeResult("b", false))) + + got, err := Compare(baseline, newRun(sizeResult("a", true), sizeResult("b", true)), 0) + require.NoError(t, err) + assert.False(t, got.Regressed) + for _, ch := range got.Changes { + assert.False(t, ch.Regressed, "fail → pass must never gate") + } +} + +func TestCompare_FailureRateClimbRegresses(t *testing.T) { + t.Parallel() + + baseline := saveAndLoad(t, newRun(sizeResult("a", true), sizeResult("b", true))) + + 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) +} + +// Cost is reported but must never gate: a price change is not a quality change. +func TestCompare_CostIsInformationalOnly(t *testing.T) { + t.Parallel() + + 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, 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 + 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) +} + +func TestCompare_AddedAndRemovedEvalsDoNotGateOnTheirOwn(t *testing.T) { + t.Parallel() + + baseline := saveAndLoad(t, newRun(sizeResult("a", true), sizeResult("gone", true))) + + got, err := Compare(baseline, newRun(sizeResult("a", true), sizeResult("new", true)), 0) + require.NoError(t, err) + + byKey := map[string]EvalChange{} + for _, ch := range got.Changes { + byKey[ch.Eval] = ch + } + 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) +} + +// 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() + + baseline := saveAndLoad(t, newRun(sizeResult("a", true))) + + 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") +} + +func TestCompare_RepeatedEvalCollapsesPessimistically(t *testing.T) { + t.Parallel() + + baseline := saveAndLoad(t, newRun(sizeResult("a", true))) + + // 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) + + 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 TestCompare_RegressionsAreListedFirst(t *testing.T) { + t.Parallel() + + baseline := saveAndLoad(t, newRun(sizeResult("aaa", true), sizeResult("zzz", true))) + + got, err := Compare(baseline, + newRun(sizeResult("aaa", true), sizeResult("zzz", false), sizeResult("bbb", true)), 0) + require.NoError(t, err) + + 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 TestMetricsOf_AbsentMetricsAreFlaggedNotZero(t *testing.T) { + t.Parallel() + + 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) + + assert.Equal(t, Metrics{}, MetricsOf(nil), "a nil run yields zero metrics, not a panic") +} + +func TestPrintComparison(t *testing.T) { + t.Parallel() + + 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, c) + 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() + 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, c) + assert.Contains(t, buf.String(), "No regression against baseline") + }) +} + +func TestComparison_IsJSONSerializable(t *testing.T) { + t.Parallel() + + 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) + + var round Comparison + require.NoError(t, json.Unmarshal(data, &round)) + assert.True(t, round.Regressed) + assert.InDelta(t, 0.05, round.Tolerance, 1e-9) +}