diff --git a/CHANGELOG.md b/CHANGELOG.md index af6007e..17647ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- `skill-up compare ` compares offline + evaluation results, including case transitions and optional regression and + total-token growth gates for CI. + ## [0.9.0] - 2026-08-12 ### Fixed diff --git a/README.md b/README.md index 3cfc6b2..8001005 100644 --- a/README.md +++ b/README.md @@ -235,6 +235,7 @@ skill-up import ./evals/evals.json --output ./evals | `skill-up validate [path]` | Validate `eval.yaml` and case files | | `skill-up list-cases [path]` | List all cases referenced by the config | | `skill-up report ` | Generate reports from a previous run | +| `skill-up compare ` | Compare two offline evaluation results | | `skill-up import ` | Import Anthropic `evals.json` to YAML cases | | `skill-up debug judge ` | Debug judge module with a JSON input | | `skill-up debug report ` | Debug report module with a JSON input | diff --git a/README.zh.md b/README.zh.md index b6af190..e669666 100644 --- a/README.zh.md +++ b/README.zh.md @@ -154,6 +154,7 @@ Windows 的安装方式与已知限制请参阅 | `skill-up validate [path]` | 校验 `eval.yaml` 和用例文件 | | `skill-up list-cases [path]` | 列出配置引用的所有用例 | | `skill-up report ` | 从已有结果生成报告 | +| `skill-up compare ` | 对比两个离线评测结果 | | `skill-up import ` | 将 Anthropic `evals.json` 导入为 YAML 用例 | | `skill-up debug judge ` | 使用 JSON 输入调试 judge 模块 | | `skill-up debug report ` | 使用 JSON 输入调试 report 模块 | diff --git a/internal/cli/compare.go b/internal/cli/compare.go new file mode 100644 index 0000000..63f1829 --- /dev/null +++ b/internal/cli/compare.go @@ -0,0 +1,134 @@ +package cli + +import ( + "encoding/json" + "errors" + "fmt" + "math" + "os" + "strings" + + "github.com/spf13/cobra" + + comparepkg "github.com/alibaba/skill-up/internal/compare" + "github.com/alibaba/skill-up/internal/judge" + "github.com/alibaba/skill-up/internal/report" +) + +var compareCmd = &cobra.Command{ + Use: "compare ", + Short: "Compare two offline evaluation results", + Args: cobra.ExactArgs(2), + RunE: runCompare, +} + +func init() { + compareCmd.Flags().String("format", "text", "Output format: text, json") + compareCmd.Flags().Bool("fail-on-regression", false, "Fail when any case regresses") + compareCmd.Flags().Float64("max-token-increase-percent", 0, "Maximum allowed total token increase percentage") +} + +func runCompare(cmd *cobra.Command, args []string) error { + format, err := cmd.Flags().GetString("format") + if err != nil { + return fmt.Errorf("get format flag: %w", err) + } + if format != "text" && format != jsonFormat { + return fmt.Errorf("unsupported compare format %q; supported formats: text, json", format) + } + + oldInput, err := loadCompareInput("old", args[0]) + if err != nil { + return err + } + newInput, err := loadCompareInput("new", args[1]) + if err != nil { + return err + } + + failOnRegression, err := cmd.Flags().GetBool("fail-on-regression") + if err != nil { + return fmt.Errorf("get fail-on-regression flag: %w", err) + } + options := comparepkg.Options{FailOnRegression: failOnRegression} + maxTokenIncreasePercent, err := maxTokenIncreasePercent(cmd) + if err != nil { + return err + } + if maxTokenIncreasePercent != nil { + options.MaxTokenIncreasePercent = maxTokenIncreasePercent + } + + result := comparepkg.Compare(oldInput, newInput, options) + if format == jsonFormat { + encoder := json.NewEncoder(cmd.OutOrStdout()) + if err := encoder.Encode(result); err != nil { + return fmt.Errorf("write JSON comparison: %w", err) + } + } else if _, err := fmt.Fprint(cmd.OutOrStdout(), comparepkg.RenderText(result)); err != nil { + return fmt.Errorf("write text comparison: %w", err) + } + + if !result.Gates.Passed { + return fmt.Errorf("comparison gates failed: %v", result.Gates.Failures) + } + return nil +} + +func maxTokenIncreasePercent(cmd *cobra.Command) (*float64, error) { + if !cmd.Flags().Changed("max-token-increase-percent") { + return nil, nil + } + value, err := cmd.Flags().GetFloat64("max-token-increase-percent") + if err != nil { + return nil, fmt.Errorf("get max-token-increase-percent flag: %w", err) + } + if value < 0 || math.IsNaN(value) || math.IsInf(value, 0) { + return nil, errors.New("max-token-increase-percent must be a finite non-negative number") + } + return &value, nil +} + +func loadCompareInput(role, path string) (report.Input, error) { + data, err := os.ReadFile(path) + if err != nil { + return report.Input{}, fmt.Errorf("read %s result %q: %w", role, path, err) + } + + var input report.Input + if err := json.Unmarshal(data, &input); err != nil { + return report.Input{}, fmt.Errorf("parse %s result %q: %w", role, path, err) + } + if err := validateCompareInput(input); err != nil { + return report.Input{}, fmt.Errorf("validate %s result %q: %w", role, path, err) + } + return input, nil +} + +func validateCompareInput(input report.Input) error { + if input.SkillName == "" || input.SchemaVersion == "" || input.EngineName == "" || input.StartTime.IsZero() || input.EndTime.IsZero() { + return errors.New("missing required result metadata") + } + primary := input.PrimaryCaseResults() + if len(primary) == 0 { + return errors.New("no primary case results") + } + for _, result := range input.CaseResults { + if strings.TrimSpace(result.CaseID) == "" { + return errors.New("case result has an empty case ID") + } + if !isCompareStatus(result.Status) { + return fmt.Errorf("case %q has invalid status %q", result.CaseID, result.Status) + } + } + return nil +} + +func isCompareStatus(status judge.Status) bool { + switch status { + case judge.StatusPass, judge.StatusFail, judge.StatusSkip, judge.StatusError: + return true + default: + return false + } +} diff --git a/internal/cli/compare_test.go b/internal/cli/compare_test.go new file mode 100644 index 0000000..ed5ee62 --- /dev/null +++ b/internal/cli/compare_test.go @@ -0,0 +1,307 @@ +package cli + +import ( + "bytes" + "encoding/json" + "math" + "os" + "path/filepath" + "strconv" + "strings" + "testing" + + "github.com/spf13/cobra" + + "github.com/alibaba/skill-up/internal/judge" + "github.com/alibaba/skill-up/internal/report" +) + +// newCompareCmd creates an isolated cobra command wired to runCompare for testing. +func newCompareCmd() *cobra.Command { + cmd := &cobra.Command{RunE: runCompare} + cmd.Flags().String("format", "text", "") + cmd.Flags().Bool("fail-on-regression", false, "") + cmd.Flags().Float64("max-token-increase-percent", 0, "") + return cmd +} + +func TestRunCompare_DefaultTextFormat(t *testing.T) { + t.Parallel() + dir := t.TempDir() + oldPath := writeCompareResultJSON(t, dir, "old.json", makeReportInput(judge.StatusFail)) + newPath := writeCompareResultJSON(t, dir, "new.json", makeReportInput(judge.StatusPass)) + + cmd := newCompareCmd() + var out bytes.Buffer + cmd.SetOut(&out) + + if err := runCompare(cmd, []string{oldPath, newPath}); err != nil { + t.Fatalf("runCompare error: %v", err) + } + + for _, section := range []string{"Run summary", "Metadata differences", "Case transitions", "Gates: passed"} { + if !strings.Contains(out.String(), section) { + t.Errorf("text output should contain %q, got: %s", section, out.String()) + } + } +} + +func TestRunCompare_AllowsEmptyModelName(t *testing.T) { + t.Parallel() + dir := t.TempDir() + oldInput := makeReportInput(judge.StatusFail) + oldInput.ModelName = "" + newInput := makeReportInput(judge.StatusPass) + newInput.ModelName = "" + oldPath := writeCompareResultJSON(t, dir, "old.json", oldInput) + newPath := writeCompareResultJSON(t, dir, "new.json", newInput) + + cmd := newCompareCmd() + var out bytes.Buffer + cmd.SetOut(&out) + + if err := runCompare(cmd, []string{oldPath, newPath}); err != nil { + t.Fatalf("runCompare error: %v", err) + } + if !strings.Contains(out.String(), "Run summary") { + t.Errorf("text output should contain run summary, got: %s", out.String()) + } +} + +func TestRunCompare_JSONFormat(t *testing.T) { + t.Parallel() + dir := t.TempDir() + oldPath := writeCompareResultJSON(t, dir, "old.json", makeReportInput(judge.StatusFail)) + newPath := writeCompareResultJSON(t, dir, "new.json", makeReportInput(judge.StatusPass)) + + cmd := newCompareCmd() + var out bytes.Buffer + cmd.SetOut(&out) + if err := cmd.Flags().Set("format", "json"); err != nil { + t.Fatalf("set format: %v", err) + } + + if err := runCompare(cmd, []string{oldPath, newPath}); err != nil { + t.Fatalf("runCompare error: %v", err) + } + + var result struct { + Run json.RawMessage `json:"run"` + Cases json.RawMessage `json:"cases"` + Gates json.RawMessage `json:"gates"` + } + if err := json.Unmarshal(out.Bytes(), &result); err != nil { + t.Fatalf("unmarshal compare output: %v", err) + } + if result.Run == nil || result.Cases == nil || result.Gates == nil { + t.Fatalf("JSON output missing stable result fields: %s", out.String()) + } +} + +func TestRunCompare_RegressionGateWritesTextBeforeReturningError(t *testing.T) { + t.Parallel() + dir := t.TempDir() + oldPath := writeCompareResultJSON(t, dir, "old.json", makeReportInput(judge.StatusPass)) + newPath := writeCompareResultJSON(t, dir, "new.json", makeReportInput(judge.StatusFail)) + + cmd := newCompareCmd() + var out bytes.Buffer + cmd.SetOut(&out) + if err := cmd.Flags().Set("fail-on-regression", "true"); err != nil { + t.Fatalf("set fail-on-regression flag: %v", err) + } + + err := runCompare(cmd, []string{oldPath, newPath}) + if err == nil || !strings.Contains(err.Error(), "comparison gates failed") { + t.Fatalf("runCompare error = %v, want gate failure", err) + } + for _, want := range []string{"Run summary", "Gates: failed", "1 case(s) regressed"} { + if !strings.Contains(out.String(), want) { + t.Errorf("text output should contain %q before returning error, got: %s", want, out.String()) + } + } +} + +func TestRunCompare_TokenGateWritesJSONBeforeReturningError(t *testing.T) { + t.Parallel() + dir := t.TempDir() + oldInput := makeReportInput(judge.StatusPass) + oldInput.TotalTokens = 100 + newInput := makeReportInput(judge.StatusPass) + newInput.TotalTokens = 150 + oldPath := writeCompareResultJSON(t, dir, "old.json", oldInput) + newPath := writeCompareResultJSON(t, dir, "new.json", newInput) + + cmd := newCompareCmd() + var out bytes.Buffer + cmd.SetOut(&out) + if err := cmd.Flags().Set("format", "json"); err != nil { + t.Fatalf("set format flag: %v", err) + } + if err := cmd.Flags().Set("max-token-increase-percent", "20"); err != nil { + t.Fatalf("set max-token-increase-percent flag: %v", err) + } + + err := runCompare(cmd, []string{oldPath, newPath}) + if err == nil || !strings.Contains(err.Error(), "comparison gates failed") { + t.Fatalf("runCompare error = %v, want gate failure", err) + } + + var result struct { + Gates struct { + Passed bool `json:"passed"` + Failures []string `json:"failures"` + } `json:"gates"` + } + if err := json.Unmarshal(out.Bytes(), &result); err != nil { + t.Fatalf("unmarshal JSON output written before error: %v\noutput: %s", err, out.String()) + } + if result.Gates.Passed || len(result.Gates.Failures) != 1 || !strings.Contains(result.Gates.Failures[0], "50.00%") { + t.Fatalf("JSON gates = %#v, want failed token increase gate", result.Gates) + } +} + +func TestRunCompare_InvalidInputsIncludeRoleAndPath(t *testing.T) { + t.Parallel() + dir := t.TempDir() + validPath := writeCompareResultJSON(t, dir, "valid.json", makeReportInput(judge.StatusPass)) + invalidPath := filepath.Join(dir, "invalid.json") + if err := os.WriteFile(invalidPath, []byte("not-json"), 0o600); err != nil { + t.Fatalf("write invalid input: %v", err) + } + + tests := []struct { + name string + args []string + role string + path string + }{ + {name: "old", args: []string{invalidPath, validPath}, role: "old", path: invalidPath}, + {name: "new", args: []string{validPath, invalidPath}, role: "new", path: invalidPath}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + cmd := newCompareCmd() + cmd.SetOut(&bytes.Buffer{}) + + err := runCompare(cmd, tc.args) + if err == nil { + t.Fatal("expected invalid JSON error, got nil") + } + if !strings.Contains(err.Error(), tc.role) || !strings.Contains(err.Error(), tc.path) { + t.Errorf("error should identify %s input %q, got: %v", tc.role, tc.path, err) + } + }) + } +} + +func TestRunCompare_RejectsStructurallyInvalidInputs(t *testing.T) { + t.Parallel() + dir := t.TempDir() + validPath := writeCompareResultJSON(t, dir, "valid.json", makeReportInput(judge.StatusPass)) + + tests := []struct { + name string + input report.Input + wantError string + }{ + {name: "empty result", input: report.Input{}, wantError: "missing required result metadata"}, + { + name: "blank case ID", + input: func() report.Input { + input := makeReportInput(judge.StatusPass) + input.CaseResults = []report.CaseResult{{CaseID: " ", Status: judge.StatusPass}} + return input + }(), + wantError: "case result has an empty case ID", + }, + { + name: "invalid status", + input: func() report.Input { + input := makeReportInput(judge.StatusPass) + input.CaseResults = []report.CaseResult{{CaseID: "case-1", Status: "UNKNOWN"}} + return input + }(), + wantError: "case \"case-1\" has invalid status \"UNKNOWN\"", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + invalidPath := writeCompareResultJSON(t, dir, tc.name+".json", tc.input) + cmd := newCompareCmd() + cmd.SetOut(&bytes.Buffer{}) + + err := runCompare(cmd, []string{invalidPath, validPath}) + if err == nil { + t.Fatal("expected structural validation error, got nil") + } + if !strings.Contains(err.Error(), tc.wantError) { + t.Errorf("error = %v, want to contain %q", err, tc.wantError) + } + if !strings.Contains(err.Error(), "old") || !strings.Contains(err.Error(), invalidPath) { + t.Errorf("error should identify old input %q, got: %v", invalidPath, err) + } + }) + } +} + +func TestRunCompare_RejectsInvalidMaxTokenIncreasePercent(t *testing.T) { + t.Parallel() + dir := t.TempDir() + oldPath := writeCompareResultJSON(t, dir, "old.json", makeReportInput(judge.StatusPass)) + newPath := writeCompareResultJSON(t, dir, "new.json", makeReportInput(judge.StatusPass)) + + for _, value := range []float64{-1, math.NaN(), math.Inf(1)} { + t.Run("invalid value", func(t *testing.T) { + t.Parallel() + cmd := newCompareCmd() + cmd.SetOut(&bytes.Buffer{}) + if err := cmd.Flags().Set("max-token-increase-percent", strconv.FormatFloat(value, 'g', -1, 64)); err != nil { + t.Fatalf("set max-token-increase-percent: %v", err) + } + + err := runCompare(cmd, []string{oldPath, newPath}) + if err == nil || !strings.Contains(err.Error(), "max-token-increase-percent") { + t.Fatalf("runCompare error = %v, want invalid token limit error", err) + } + }) + } +} + +func TestRunCompare_UnsupportedFormat(t *testing.T) { + t.Parallel() + dir := t.TempDir() + oldPath := writeCompareResultJSON(t, dir, "old.json", makeReportInput(judge.StatusPass)) + newPath := writeCompareResultJSON(t, dir, "new.json", makeReportInput(judge.StatusPass)) + + cmd := newCompareCmd() + cmd.SetOut(&bytes.Buffer{}) + if err := cmd.Flags().Set("format", "csv"); err != nil { + t.Fatalf("set format: %v", err) + } + + err := runCompare(cmd, []string{oldPath, newPath}) + if err == nil { + t.Fatal("expected unsupported format error, got nil") + } + if !strings.Contains(err.Error(), "csv") { + t.Errorf("error should name unsupported format, got: %v", err) + } +} + +func writeCompareResultJSON(t *testing.T, dir, name string, input report.Input) string { + t.Helper() + data, err := json.Marshal(input) + if err != nil { + t.Fatalf("marshal report input: %v", err) + } + path := filepath.Join(dir, name) + if err := os.WriteFile(path, data, 0o600); err != nil { + t.Fatalf("write report input: %v", err) + } + return path +} diff --git a/internal/cli/root.go b/internal/cli/root.go index 3381903..ab8e7ea 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -175,6 +175,7 @@ func init() { rootCmd.AddCommand(validateCmd) rootCmd.AddCommand(listCasesCmd) rootCmd.AddCommand(reportCmd) + rootCmd.AddCommand(compareCmd) rootCmd.AddCommand(debugCmd) rootCmd.AddCommand(importCmd) } diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index 4453264..078ca6e 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -47,6 +47,16 @@ func TestExecuteArgsVersionInitializesRoot(t *testing.T) { } } +func TestRootCommandIncludesCompare(t *testing.T) { + command, _, err := rootCmd.Find([]string{"compare"}) + if err != nil { + t.Fatalf("find compare command: %v", err) + } + if command != compareCmd { + t.Fatalf("compare command = %p, want registered compare command %p", command, compareCmd) + } +} + func TestIsInitInvocation(t *testing.T) { cases := []struct { name string diff --git a/internal/cli/run.go b/internal/cli/run.go index b6d07d6..888b81c 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -33,6 +33,7 @@ import ( const ( modelFormatParts = 2 maxParallelismOverride = 256 + jsonFormat = "json" runtimeKwargFlagName = "runtime-kwarg" runtimeKwargAlias = "rk" runtimeFlagName = "runtime" @@ -103,7 +104,7 @@ func init() { runCmd.Flags().Bool("auto", false, "Auto-detect evals/ directory, preferring eval.yaml over evals.json") runCmd.Flags().StringArray("include-case-name", nil, "Include cases matching glob pattern (can be specified multiple times)") runCmd.Flags().StringArray("exclude-case-name", nil, "Exclude cases matching glob pattern (can be specified multiple times)") - runCmd.Flags().StringArray("format", nil, "Report format (json, junit, html). Can be specified multiple times. Default: json") + runCmd.Flags().StringArray("format", nil, "Report format ("+jsonFormat+", junit, html). Can be specified multiple times. Default: "+jsonFormat) runCmd.Flags().String("output-dir", "", "Directory for report/artifact outputs. Default: -workspace alongside the skill directory") runCmd.Flags().String("engine", "", "Override engine name") runCmd.Flags().String(runtimeFlagName, "", "Override environment.type (none, opensandbox, docker)") @@ -421,9 +422,9 @@ func evaluateOptionsFromFlags(cmd *cobra.Command) (runner.EvaluateOptions, error } for _, f := range formats { switch f { - case "json", "junit", "html": + case jsonFormat, "junit", "html": default: - return runner.EvaluateOptions{}, fmt.Errorf("unsupported --format %q (supported: json, junit, html)", f) + return runner.EvaluateOptions{}, fmt.Errorf("unsupported --format %q (supported: %s, junit, html)", f, jsonFormat) } } diff --git a/internal/compare/compare.go b/internal/compare/compare.go new file mode 100644 index 0000000..196568e --- /dev/null +++ b/internal/compare/compare.go @@ -0,0 +1,201 @@ +// Package compare compares two offline skill-up report results. +package compare + +import ( + "time" + + "github.com/alibaba/skill-up/internal/judge" + "github.com/alibaba/skill-up/internal/report" +) + +// Options controls compare output and gate evaluation. +type Options struct { + FailOnRegression bool + MaxRegressions *int + MaxTokenIncreasePercent *float64 +} + +// Result is the stable comparison result used by text and JSON output. +type Result struct { + Metadata MetadataDiff `json:"metadata"` + Run RunComparison `json:"run"` + Cases CaseTransitions `json:"cases"` + Gates GateResult `json:"gates"` +} + +// MetadataDiff records old and new run metadata. +type MetadataDiff struct { + SkillName FieldDiff[string] `json:"skill_name"` + SchemaVersion FieldDiff[string] `json:"schema_version"` + EngineName FieldDiff[string] `json:"engine_name"` + ModelName FieldDiff[string] `json:"model_name"` + StartTime FieldDiff[time.Time] `json:"start_time"` + EndTime FieldDiff[time.Time] `json:"end_time"` +} + +// FieldDiff stores old/new values and whether they differ. +type FieldDiff[T comparable] struct { + Old T `json:"old"` + New T `json:"new"` + Changed bool `json:"changed"` +} + +// RunComparison stores old/new/delta aggregate metrics. +type RunComparison struct { + Old RunMetrics `json:"old"` + New RunMetrics `json:"new"` + Delta RunMetrics `json:"delta"` +} + +// RunMetrics stores aggregate metrics for one result set or a delta. +type RunMetrics struct { + CaseCount int `json:"case_count"` + PassCount int `json:"pass_count"` + PassRate float64 `json:"pass_rate"` + TotalTokens int `json:"total_tokens"` + InputTokens int `json:"input_tokens"` + OutputTokens int `json:"output_tokens"` + DurationMs int64 `json:"duration_ms"` +} + +// CaseTransitions groups case status transitions. +type CaseTransitions struct { + Fixed []CaseTransition `json:"fixed"` + Regressed []CaseTransition `json:"regressed"` + Changed []CaseTransition `json:"changed"` + Unchanged []CaseTransition `json:"unchanged"` + Added []CaseTransition `json:"added"` + Removed []CaseTransition `json:"removed"` +} + +// CaseTransition describes one case movement between old and new results. +type CaseTransition struct { + CaseID string `json:"case_id"` + OldTitle string `json:"old_title,omitempty"` + NewTitle string `json:"new_title,omitempty"` + OldStatus judge.Status `json:"old_status,omitempty"` + NewStatus judge.Status `json:"new_status,omitempty"` +} + +// GateResult records whether CI gates passed and why they failed. +type GateResult struct { + Passed bool `json:"passed"` + Failures []string `json:"failures"` +} + +// Compare compares two offline report inputs. +func Compare(oldInput, newInput report.Input, options Options) Result { + oldMetrics := collectRunMetrics(oldInput) + newMetrics := collectRunMetrics(newInput) + result := Result{ + Metadata: metadataDiff(oldInput, newInput), + Run: RunComparison{ + Old: oldMetrics, + New: newMetrics, + Delta: diffRunMetrics(oldMetrics, newMetrics), + }, + Cases: compareCases(oldInput.PrimaryCaseResults(), newInput.PrimaryCaseResults()), + } + result.Gates = EvaluateGates(result, options) + return result +} + +func collectRunMetrics(input report.Input) RunMetrics { + primary := input.PrimaryCaseResults() + metrics := RunMetrics{ + CaseCount: len(primary), + TotalTokens: input.TotalTokens, + DurationMs: input.TotalDuration().Milliseconds(), + } + for _, cr := range primary { + if cr.Status == judge.StatusPass { + metrics.PassCount++ + } + metrics.InputTokens += cr.InputTokens + metrics.OutputTokens += cr.OutputTokens + } + if metrics.CaseCount > 0 { + metrics.PassRate = float64(metrics.PassCount) / float64(metrics.CaseCount) + } + return metrics +} + +func diffRunMetrics(oldMetrics, newMetrics RunMetrics) RunMetrics { + return RunMetrics{ + CaseCount: newMetrics.CaseCount - oldMetrics.CaseCount, + PassCount: newMetrics.PassCount - oldMetrics.PassCount, + PassRate: newMetrics.PassRate - oldMetrics.PassRate, + TotalTokens: newMetrics.TotalTokens - oldMetrics.TotalTokens, + InputTokens: newMetrics.InputTokens - oldMetrics.InputTokens, + OutputTokens: newMetrics.OutputTokens - oldMetrics.OutputTokens, + DurationMs: newMetrics.DurationMs - oldMetrics.DurationMs, + } +} + +func metadataDiff(oldInput, newInput report.Input) MetadataDiff { + return MetadataDiff{ + SkillName: fieldDiff(oldInput.SkillName, newInput.SkillName), + SchemaVersion: fieldDiff(oldInput.SchemaVersion, newInput.SchemaVersion), + EngineName: fieldDiff(oldInput.EngineName, newInput.EngineName), + ModelName: fieldDiff(oldInput.ModelName, newInput.ModelName), + StartTime: fieldDiff(oldInput.StartTime, newInput.StartTime), + EndTime: fieldDiff(oldInput.EndTime, newInput.EndTime), + } +} + +func fieldDiff[T comparable](oldValue, newValue T) FieldDiff[T] { + return FieldDiff[T]{Old: oldValue, New: newValue, Changed: oldValue != newValue} +} + +func compareCases(oldCases, newCases []report.CaseResult) CaseTransitions { + newByID := make(map[string]report.CaseResult, len(newCases)) + for _, newCase := range newCases { + newByID[newCase.CaseID] = newCase + } + + transitions := CaseTransitions{ + Fixed: make([]CaseTransition, 0), + Regressed: make([]CaseTransition, 0), + Changed: make([]CaseTransition, 0), + Unchanged: make([]CaseTransition, 0), + Added: make([]CaseTransition, 0), + Removed: make([]CaseTransition, 0), + } + for _, oldCase := range oldCases { + newCase, exists := newByID[oldCase.CaseID] + if !exists { + transitions.Removed = append(transitions.Removed, CaseTransition{ + CaseID: oldCase.CaseID, OldTitle: oldCase.Title, OldStatus: oldCase.Status, + }) + continue + } + + transition := CaseTransition{ + CaseID: oldCase.CaseID, OldTitle: oldCase.Title, NewTitle: newCase.Title, + OldStatus: oldCase.Status, NewStatus: newCase.Status, + } + switch { + case oldCase.Status != judge.StatusPass && newCase.Status == judge.StatusPass: + transitions.Fixed = append(transitions.Fixed, transition) + case oldCase.Status == judge.StatusPass && newCase.Status != judge.StatusPass: + transitions.Regressed = append(transitions.Regressed, transition) + case oldCase.Status == newCase.Status: + transitions.Unchanged = append(transitions.Unchanged, transition) + default: + transitions.Changed = append(transitions.Changed, transition) + } + } + + oldByID := make(map[string]struct{}, len(oldCases)) + for _, oldCase := range oldCases { + oldByID[oldCase.CaseID] = struct{}{} + } + for _, newCase := range newCases { + if _, exists := oldByID[newCase.CaseID]; !exists { + transitions.Added = append(transitions.Added, CaseTransition{ + CaseID: newCase.CaseID, NewTitle: newCase.Title, NewStatus: newCase.Status, + }) + } + } + return transitions +} diff --git a/internal/compare/compare_test.go b/internal/compare/compare_test.go new file mode 100644 index 0000000..b8c40d1 --- /dev/null +++ b/internal/compare/compare_test.go @@ -0,0 +1,282 @@ +package compare + +import ( + "encoding/json" + "math" + "reflect" + "strings" + "testing" + "time" + + "github.com/alibaba/skill-up/internal/judge" + "github.com/alibaba/skill-up/internal/report" +) + +func compareFixture() (oldInput, newInput report.Input) { + start := time.Date(2026, 8, 12, 9, 0, 0, 0, time.UTC) + oldInput = report.Input{ + SkillName: "skill-a", + SchemaVersion: "v1alpha1", + EngineName: "codex", + ModelName: "gpt-5", + StartTime: start, + EndTime: start.Add(2 * time.Minute), + TotalTokens: 100, + CaseResults: []report.CaseResult{ + {CaseID: "case-1", Title: "Case One baseline", Status: judge.StatusFail, Configuration: "without_skill", InputTokens: 10, OutputTokens: 5}, + {CaseID: "case-1", Title: "Case One", Status: judge.StatusPass, Configuration: "with_skill", InputTokens: 20, OutputTokens: 10}, + {CaseID: "case-2", Title: "Case Two", Status: judge.StatusFail, InputTokens: 30, OutputTokens: 15}, + }, + } + newInput = report.Input{ + SkillName: "skill-a", + SchemaVersion: "v1alpha1", + EngineName: "codex", + ModelName: "gpt-5.1", + StartTime: start.Add(24 * time.Hour), + EndTime: start.Add(24*time.Hour + 3*time.Minute), + TotalTokens: 140, + CaseResults: []report.CaseResult{ + {CaseID: "case-1", Title: "Case One", Status: judge.StatusPass, Configuration: "with_skill", InputTokens: 25, OutputTokens: 12}, + {CaseID: "case-2", Title: "Case Two", Status: judge.StatusPass, InputTokens: 35, OutputTokens: 18}, + }, + } + return oldInput, newInput +} + +func TestCompareRunMetricsUsePrimaryCaseResults(t *testing.T) { + t.Parallel() + oldInput, newInput := compareFixture() + + result := Compare(oldInput, newInput, Options{}) + + if result.Run.Old.CaseCount != 2 { + t.Fatalf("old case count should use primary results, got %d", result.Run.Old.CaseCount) + } + if result.Run.Old.PassCount != 1 { + t.Fatalf("old pass count should ignore without_skill baseline failure, got %d", result.Run.Old.PassCount) + } + if math.Abs(result.Run.Old.PassRate-0.5) > 0.001 { + t.Fatalf("old pass rate: want 0.5, got %f", result.Run.Old.PassRate) + } + if result.Run.Old.InputTokens != 50 || result.Run.Old.OutputTokens != 25 { + t.Fatalf("old tokens should sum primary results, got input=%d output=%d", result.Run.Old.InputTokens, result.Run.Old.OutputTokens) + } + if result.Run.Old.TotalTokens != 100 || result.Run.New.TotalTokens != 140 || result.Run.Delta.TotalTokens != 40 { + t.Fatalf("total token delta mismatch: old=%d new=%d delta=%d", result.Run.Old.TotalTokens, result.Run.New.TotalTokens, result.Run.Delta.TotalTokens) + } + if result.Run.Old.DurationMs != 120000 || result.Run.New.DurationMs != 180000 || result.Run.Delta.DurationMs != 60000 { + t.Fatalf("duration delta mismatch: old=%d new=%d delta=%d", result.Run.Old.DurationMs, result.Run.New.DurationMs, result.Run.Delta.DurationMs) + } +} + +func TestCompareClassifiesCaseTransitionsInDeterministicOrder(t *testing.T) { + t.Parallel() + oldInput := report.Input{CaseResults: []report.CaseResult{ + {CaseID: "fixed", Title: "Fixed old", Status: judge.StatusFail}, + {CaseID: "regressed", Title: "Regressed old", Status: judge.StatusPass}, + {CaseID: "status-changed", Title: "Status changed old", Status: judge.StatusError}, + {CaseID: "unchanged", Title: "Unchanged old", Status: judge.StatusError}, + {CaseID: "removed", Title: "Removed", Status: judge.StatusFail}, + }} + newInput := report.Input{CaseResults: []report.CaseResult{ + {CaseID: "regressed", Title: "Regressed new", Status: judge.StatusFail}, + {CaseID: "fixed", Title: "Fixed new", Status: judge.StatusPass}, + {CaseID: "status-changed", Title: "Status changed new", Status: judge.StatusSkip}, + {CaseID: "unchanged", Title: "Unchanged new", Status: judge.StatusError}, + {CaseID: "added-first", Title: "Added first", Status: judge.StatusPass}, + {CaseID: "added-second", Title: "Added second", Status: judge.StatusFail}, + }} + + got := Compare(oldInput, newInput, Options{}).Cases + want := CaseTransitions{ + Fixed: []CaseTransition{{CaseID: "fixed", OldTitle: "Fixed old", NewTitle: "Fixed new", OldStatus: judge.StatusFail, NewStatus: judge.StatusPass}}, + Regressed: []CaseTransition{{CaseID: "regressed", OldTitle: "Regressed old", NewTitle: "Regressed new", OldStatus: judge.StatusPass, NewStatus: judge.StatusFail}}, + Changed: []CaseTransition{{CaseID: "status-changed", OldTitle: "Status changed old", NewTitle: "Status changed new", OldStatus: judge.StatusError, NewStatus: judge.StatusSkip}}, + Unchanged: []CaseTransition{{CaseID: "unchanged", OldTitle: "Unchanged old", NewTitle: "Unchanged new", OldStatus: judge.StatusError, NewStatus: judge.StatusError}}, + Added: []CaseTransition{ + {CaseID: "added-first", NewTitle: "Added first", NewStatus: judge.StatusPass}, + {CaseID: "added-second", NewTitle: "Added second", NewStatus: judge.StatusFail}, + }, + Removed: []CaseTransition{{CaseID: "removed", OldTitle: "Removed", OldStatus: judge.StatusFail}}, + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("case transitions mismatch:\nwant: %#v\n got: %#v", want, got) + } +} + +func TestCompareIncludesMetadataDiff(t *testing.T) { + t.Parallel() + oldInput, newInput := compareFixture() + + got := Compare(oldInput, newInput, Options{}).Metadata + if got.SkillName.Changed { + t.Fatal("skill name should be unchanged") + } + if !got.ModelName.Changed || !got.StartTime.Changed || !got.EndTime.Changed { + t.Fatalf("changed metadata mismatch: model=%t start=%t end=%t", got.ModelName.Changed, got.StartTime.Changed, got.EndTime.Changed) + } +} + +func TestCompareFailsRegressionAndTokenIncreaseGates(t *testing.T) { + t.Parallel() + limit := 20.0 + oldInput := report.Input{TotalTokens: 100, CaseResults: []report.CaseResult{{CaseID: "case-1", Status: judge.StatusPass}}} + newInput := report.Input{TotalTokens: 140, CaseResults: []report.CaseResult{{CaseID: "case-1", Status: judge.StatusFail}}} + + got := Compare(oldInput, newInput, Options{FailOnRegression: true, MaxTokenIncreasePercent: &limit}).Gates + if got.Passed || len(got.Failures) != 2 { + t.Fatalf("expected both gates to fail, got %#v", got) + } +} + +func TestCompareDoesNotFailRegressionGateForChangedNonPassStatus(t *testing.T) { + t.Parallel() + oldInput := report.Input{CaseResults: []report.CaseResult{{CaseID: "case-1", Status: judge.StatusError}}} + newInput := report.Input{CaseResults: []report.CaseResult{{CaseID: "case-1", Status: judge.StatusSkip}}} + + result := Compare(oldInput, newInput, Options{FailOnRegression: true}) + if !result.Gates.Passed || len(result.Gates.Failures) != 0 { + t.Fatalf("changed non-PASS status should not fail regression gate, got %#v", result.Gates) + } + if len(result.Cases.Changed) != 1 { + t.Fatalf("changed transitions = %#v, want one ERROR -> SKIP transition", result.Cases.Changed) + } +} + +func TestCompareFailsRegressionGateAboveConfiguredMaximum(t *testing.T) { + t.Parallel() + maxRegressions := 1 + oldInput := report.Input{CaseResults: []report.CaseResult{ + {CaseID: "case-1", Status: judge.StatusPass}, + {CaseID: "case-2", Status: judge.StatusPass}, + }} + newInput := report.Input{CaseResults: []report.CaseResult{ + {CaseID: "case-1", Status: judge.StatusFail}, + {CaseID: "case-2", Status: judge.StatusError}, + }} + + got := Compare(oldInput, newInput, Options{MaxRegressions: &maxRegressions}).Gates + if got.Passed || len(got.Failures) != 1 || !strings.Contains(got.Failures[0], "2 regressions exceeds maximum 1") { + t.Fatalf("expected regression maximum gate failure, got %#v", got) + } +} + +func TestCompareFailsTokenGateWhenOldTotalTokensAreZero(t *testing.T) { + t.Parallel() + limit := 20.0 + oldInput := report.Input{TotalTokens: 0} + newInput := report.Input{TotalTokens: 1, CaseResults: []report.CaseResult{{CaseID: "case-1"}}} + + got := Compare(oldInput, newInput, Options{MaxTokenIncreasePercent: &limit}).Gates + if got.Passed || len(got.Failures) != 1 || !strings.Contains(got.Failures[0], "old total tokens is 0") { + t.Fatalf("expected zero-token gate failure, got %#v", got) + } +} + +func TestCompareTokenGateUsesTotalTokens(t *testing.T) { + t.Parallel() + limit := 10.0 + oldInput := report.Input{TotalTokens: 200, CaseResults: []report.CaseResult{ + {CaseID: "case-1"}, + {CaseID: "case-2"}, + }} + newInput := report.Input{TotalTokens: 150, CaseResults: []report.CaseResult{{CaseID: "case-1"}}} + + got := Compare(oldInput, newInput, Options{MaxTokenIncreasePercent: &limit}).Gates + if !got.Passed || len(got.Failures) != 0 { + t.Fatalf("expected total token gate to pass, got %#v", got) + } +} + +func TestRenderTextIncludesRunMetadataAndCaseTransitionSections(t *testing.T) { + t.Parallel() + oldInput, newInput := compareFixture() + result := Compare(oldInput, newInput, Options{FailOnRegression: true}) + result.Cases = CaseTransitions{ + Fixed: []CaseTransition{{CaseID: "fixed", OldStatus: judge.StatusFail, NewStatus: judge.StatusPass}}, + Regressed: []CaseTransition{{CaseID: "regressed", OldStatus: judge.StatusPass, NewStatus: judge.StatusFail}}, + Changed: []CaseTransition{{CaseID: "changed", OldStatus: judge.StatusError, NewStatus: judge.StatusSkip}}, + Unchanged: []CaseTransition{{CaseID: "unchanged", OldStatus: judge.StatusError, NewStatus: judge.StatusError}}, + Added: []CaseTransition{{CaseID: "added", NewStatus: judge.StatusPass}}, + Removed: []CaseTransition{{CaseID: "removed", OldStatus: judge.StatusFail}}, + } + result.Gates = GateResult{Passed: false, Failures: []string{"1 case(s) regressed"}} + + got := RenderText(result) + for _, want := range []string{ + "Run summary", + "pass rate: 50.00% -> 100.00% (+50.00%)", + "total tokens: 100 -> 140 (+40)", + "Metadata differences", + "model name: gpt-5 -> gpt-5.1", + "Case transitions", + "fixed (1): fixed (FAIL -> PASS)", + "regressed (1): regressed (PASS -> FAIL)", + "changed (1): changed (ERROR -> SKIP)", + "unchanged (1): unchanged (ERROR -> ERROR)", + "added (1): added (-> PASS)", + "removed (1): removed (FAIL ->)", + "Gates: failed", + "- 1 case(s) regressed", + } { + if !strings.Contains(got, want) { + t.Errorf("RenderText() missing %q:\n%s", want, got) + } + } +} + +func TestResultJSONUsesStableFields(t *testing.T) { + t.Parallel() + oldInput, newInput := compareFixture() + data, err := json.Marshal(Compare(oldInput, newInput, Options{})) + if err != nil { + t.Fatalf("marshal result: %v", err) + } + + var got map[string]any + if err := json.Unmarshal(data, &got); err != nil { + t.Fatalf("unmarshal result: %v", err) + } + for _, key := range []string{"metadata", "run", "cases", "gates"} { + if _, ok := got[key]; !ok { + t.Errorf("JSON result missing top-level %q: %s", key, data) + } + } + for _, key := range []string{"old", "new", "delta"} { + if _, ok := got["run"].(map[string]any)[key]; !ok { + t.Errorf("JSON run missing %q: %s", key, data) + } + } + for _, key := range []string{"fixed", "regressed", "changed", "unchanged", "added", "removed"} { + if _, ok := got["cases"].(map[string]any)[key]; !ok { + t.Errorf("JSON cases missing %q: %s", key, data) + } + } + for _, key := range []string{"passed", "failures"} { + if _, ok := got["gates"].(map[string]any)[key]; !ok { + t.Errorf("JSON gates missing %q: %s", key, data) + } + } +} + +func TestResultJSONUsesEmptyArraysForEmptyCaseTransitions(t *testing.T) { + t.Parallel() + input := report.Input{CaseResults: []report.CaseResult{{CaseID: "case-1", Status: judge.StatusPass}}} + data, err := json.Marshal(Compare(input, input, Options{})) + if err != nil { + t.Fatalf("marshal result: %v", err) + } + + var got struct { + Cases map[string]json.RawMessage `json:"cases"` + } + if err := json.Unmarshal(data, &got); err != nil { + t.Fatalf("unmarshal result: %v", err) + } + for _, name := range []string{"fixed", "regressed", "changed", "added", "removed"} { + if string(got.Cases[name]) != "[]" { + t.Errorf("%s transition group = %s, want []", name, got.Cases[name]) + } + } +} diff --git a/internal/compare/gates.go b/internal/compare/gates.go new file mode 100644 index 0000000..832ff51 --- /dev/null +++ b/internal/compare/gates.go @@ -0,0 +1,29 @@ +package compare + +import "fmt" + +// EvaluateGates evaluates the configured comparison gates. +func EvaluateGates(result Result, options Options) GateResult { + failures := make([]string, 0, 2) + regressions := len(result.Cases.Regressed) + if options.FailOnRegression && regressions > 0 { + failures = append(failures, fmt.Sprintf("%d case(s) regressed", regressions)) + } + if options.MaxRegressions != nil && regressions > *options.MaxRegressions { + failures = append(failures, fmt.Sprintf("%d regressions exceeds maximum %d", regressions, *options.MaxRegressions)) + } + if options.MaxTokenIncreasePercent != nil { + oldTokens := result.Run.Old.TotalTokens + newTokens := result.Run.New.TotalTokens + switch { + case oldTokens == 0 && newTokens > 0: + failures = append(failures, "token increase exceeds limit: old total tokens is 0") + case oldTokens > 0: + increasePercent := float64(newTokens-oldTokens) / float64(oldTokens) * 100 + if increasePercent > *options.MaxTokenIncreasePercent { + failures = append(failures, fmt.Sprintf("token increase %.2f%% exceeds limit %.2f%%", increasePercent, *options.MaxTokenIncreasePercent)) + } + } + } + return GateResult{Passed: len(failures) == 0, Failures: failures} +} diff --git a/internal/compare/text.go b/internal/compare/text.go new file mode 100644 index 0000000..867e684 --- /dev/null +++ b/internal/compare/text.go @@ -0,0 +1,74 @@ +package compare + +import ( + "fmt" + "strings" +) + +// RenderText renders a human-readable comparison result. +func RenderText(result Result) string { + var b strings.Builder + fmt.Fprintf(&b, "Run summary\n") + fmt.Fprintf(&b, "pass rate: %.2f%% -> %.2f%% (%+.2f%%)\n", result.Run.Old.PassRate*100, result.Run.New.PassRate*100, result.Run.Delta.PassRate*100) + fmt.Fprintf(&b, "cases: %d -> %d (%+d)\n", result.Run.Old.CaseCount, result.Run.New.CaseCount, result.Run.Delta.CaseCount) + fmt.Fprintf(&b, "passed: %d -> %d (%+d)\n", result.Run.Old.PassCount, result.Run.New.PassCount, result.Run.Delta.PassCount) + fmt.Fprintf(&b, "total tokens: %d -> %d (%+d)\n", result.Run.Old.TotalTokens, result.Run.New.TotalTokens, result.Run.Delta.TotalTokens) + fmt.Fprintf(&b, "input tokens: %d -> %d (%+d)\n", result.Run.Old.InputTokens, result.Run.New.InputTokens, result.Run.Delta.InputTokens) + fmt.Fprintf(&b, "output tokens: %d -> %d (%+d)\n", result.Run.Old.OutputTokens, result.Run.New.OutputTokens, result.Run.Delta.OutputTokens) + fmt.Fprintf(&b, "duration ms: %d -> %d (%+d)\n", result.Run.Old.DurationMs, result.Run.New.DurationMs, result.Run.Delta.DurationMs) + + b.WriteString("\nMetadata differences\n") + writeMetadataDiffs(&b, result.Metadata) + + b.WriteString("\nCase transitions\n") + writeTransitions(&b, "fixed", result.Cases.Fixed) + writeTransitions(&b, "regressed", result.Cases.Regressed) + writeTransitions(&b, "changed", result.Cases.Changed) + writeTransitions(&b, "unchanged", result.Cases.Unchanged) + writeTransitions(&b, "added", result.Cases.Added) + writeTransitions(&b, "removed", result.Cases.Removed) + + if result.Gates.Passed { + b.WriteString("\nGates: passed\n") + } else { + b.WriteString("\nGates: failed\n") + for _, failure := range result.Gates.Failures { + fmt.Fprintf(&b, "- %s\n", failure) + } + } + return b.String() +} + +func writeMetadataDiffs(b *strings.Builder, metadata MetadataDiff) { + writeChangedField(b, "skill name", metadata.SkillName) + writeChangedField(b, "schema version", metadata.SchemaVersion) + writeChangedField(b, "engine name", metadata.EngineName) + writeChangedField(b, "model name", metadata.ModelName) + writeChangedField(b, "start time", metadata.StartTime) + writeChangedField(b, "end time", metadata.EndTime) +} + +func writeChangedField[T comparable](b *strings.Builder, label string, diff FieldDiff[T]) { + if diff.Changed { + fmt.Fprintf(b, "%s: %v -> %v\n", label, diff.Old, diff.New) + } +} + +func writeTransitions(b *strings.Builder, label string, transitions []CaseTransition) { + fmt.Fprintf(b, "%s (%d):", label, len(transitions)) + for _, transition := range transitions { + fmt.Fprintf(b, " %s (%s)", transition.CaseID, transitionStatus(transition)) + } + b.WriteByte('\n') +} + +func transitionStatus(transition CaseTransition) string { + switch { + case transition.OldStatus == "": + return fmt.Sprintf("-> %s", transition.NewStatus) + case transition.NewStatus == "": + return fmt.Sprintf("%s ->", transition.OldStatus) + default: + return fmt.Sprintf("%s -> %s", transition.OldStatus, transition.NewStatus) + } +}