diff --git a/CHANGELOG.md b/CHANGELOG.md index 657ad93a..f72bb785 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- Add `skill-up compare ...` for offline + comparison with self-contained HTML, shareable Markdown, text, and JSON + reports, an overview and case matrix across all runs, per-candidate baseline + comparisons, input paths as default labels, explicit label + overrides, and optional regression and token-growth gates. HTML extends the + existing report with a comparison tab and run selector, retaining the original + case details and leaving existing report output unchanged. Comparison supports + selecting any two input files and reviewing aligned prompts, responses, and + grades side by side without changing CI gate references. Color-code pass rates, + case statuses, and metric changes, with cost/time direction handled separately. + Document primary-result selection for cross-file comparisons; use canonical + primary cases in the interactive viewer, and keep dropdowns and multi-run + tables readable with padding, wrapping, and horizontal scrolling. Align the + Compare controls, collapse secondary explanations, and omit empty Benchmark + tabs in comparison mode. - Expose the framework-owned `SKILL_UP_VARIANT` execution group to setup and subsequent runtime commands across none, Docker, and OpenSandbox; reject the reserved name in `environment.env`. - Add opt-in `judge.output.mode: file` for agent judges, with per-attempt JSON templates, strict validation, isolated correction retries, and result artifacts. @@ -21,6 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 DeepSeek Harness packages to 0.1.5-rc.3 while retaining the required cordis 4.0.2. ### Fixed +- Preserve multi-turn post-condition failure reasons in the HTML cross-run case comparison when judging was not reached. - Use patched Vite/esbuild dependencies for the documentation site while retaining VitePress 1.x. - Remove overflow-prone runtime kwargs preallocation while preserving diff --git a/README.md b/README.md index 96542523..ee7bd15c 100644 --- a/README.md +++ b/README.md @@ -408,6 +408,7 @@ skill-up import ./evals/evals.json --output ./evals | `skill-up run [path]` | Run evaluation cases and produce reports | | `skill-up validate [path]` | Validate `eval.yaml` and case files | | `skill-up list-cases [path]` | List all cases referenced by the config | +| `skill-up compare ...` | Compare multiple finished runs; export HTML, Markdown, JSON or text | | `skill-up report ` | Generate reports from a previous run | | `skill-up import ` | Import Anthropic `evals.json` to YAML cases | | `skill-up debug judge ` | Debug judge module with a JSON input | diff --git a/README.zh.md b/README.zh.md index a1e82a94..3ca319fd 100644 --- a/README.zh.md +++ b/README.zh.md @@ -315,6 +315,7 @@ Engine 和凭据。它与旧式 `judge.model` 互斥，后者复用被测 Agent | `skill-up run [path]` | 运行评测用例并生成报告 | | `skill-up validate [path]` | 校验 `eval.yaml` 和用例文件 | | `skill-up list-cases [path]` | 列出配置引用的所有用例 | +| `skill-up compare ...` | 离线对比多个评测结果，支持 HTML、Markdown、JSON 和文本 | | `skill-up report ` | 从已有结果生成报告 | | `skill-up import ` | 将 Anthropic `evals.json` 导入为 YAML 用例 | | `skill-up debug judge ` | 使用 JSON 输入调试 judge 模块 | diff --git a/docs/guide/cli-reference.md b/docs/guide/cli-reference.md index b028e2b0..59a9f144 100644 --- a/docs/guide/cli-reference.md +++ b/docs/guide/cli-reference.md @@ -234,7 +234,141 @@ skill-up report result.json --format html --output-dir ./reports | `html` | `report.html` | Human-readable visualization; open in a browser | | `markdown` | `report.md` | Concise Markdown summary for CI logs and GitHub PR comments | -`markdown` is intentionally limited to `skill-up report` as an offline conversion format for an existing `result.json`. It is not currently accepted by `skill-up run --format`, `skill-up debug report --format`, `skill-up debug judge --report`, or `eval.yaml report.formats`. +For single-run reports, `markdown` is limited to `skill-up report` as an offline conversion format for an existing `result.json`. It is not currently accepted by `skill-up run --format`, `skill-up debug report --format`, `skill-up debug judge --report`, or `eval.yaml report.formats`. + +--- + +## skill-up compare + +Compare two or more existing canonical `result.json` (or equivalent `report.json`) +files without running evaluations or calling any Agent or Judge. + +```bash +skill-up compare ... [flags] +``` + +The first file is the reference for deltas and gates. Every remaining file is +compared independently against it, preserving argument order. This comparison +role does not determine its display name. Any failed comparison makes the +command fail; inputs are never compared as a sequential chain. + +### Flags + +| Flag | Default | Description | +|---|---|---| +| `--format` | `text` | `text`, `json`, `html`, or `markdown` | +| `--output` | stdout | Write the comparison to this file; its parent directory must exist | +| `--baseline-label` | automatic | Override the first run's display label | +| `--candidate-label` | automatic | Override later runs' display labels in input order; repeat once per later input or omit the flag | +| `--fail-on-regression` | `false` | Return a non-zero exit code for any shared case moving from PASS to another status | +| `--max-token-increase-percent` | disabled | Maximum permitted increase in run-level total tokens (finite, non-negative percentage) | + +### Examples + +```bash +# Compare several finished runs in one self-contained HTML report +skill-up compare baseline/result.json skill-a/result.json skill-b/result.json \ + --format html --output comparison.html + +# Two-run comparisons use the same command and report structure +skill-up compare old/result.json new/result.json --format html --output pair.html + +# Markdown tables for PR reviews and CI artifacts +skill-up compare old/result.json new/result.json --format markdown --output comparison.md \ + --baseline-label "Released Skill" --candidate-label "Proposed Skill" + +# Explicit labels for multiple candidates (repeatable, commas remain literal) +skill-up compare baseline.json a.json b.json --format markdown --output variants.md \ + --candidate-label "Skill A" --candidate-label "Skill B" + +# Produce a report even if the regression gate fails +skill-up compare old/result.json new/result.json --format html --output comparison.html \ + --fail-on-regression --max-token-increase-percent 20 + +# Stable machine-readable output +skill-up compare old/result.json new/result.json --format json +``` + +All formats use the same comparison and gates. Reports include run metrics, +metadata, original source paths, a side-by-side overview of all runs, a case +status matrix across the union of case IDs, and detailed fixed, regressed, changed, +unchanged, added, and removed cases for each candidate against the baseline. +The case matrix marks cases absent from a run explicitly; JSON uses null matrix +cells. Matrix outcomes follow input order, and baseline-only cases are retained. +JSON always uses the same top-level `baseline`, `runs`, `case_matrix`, +`comparisons`, and `gates` structure, including for exactly two input files. +`comparisons` contains one baseline comparison per candidate, with individual +gate results; top-level `gates` aggregates failures and identifies the candidate. +All runs have a one-based numeric ID (`runs[].source.id`) assigned in input +order, including the reference. These IDs are also retained in each comparison's +source identity; display labels do not change them. + +Default display labels are the input paths exactly as supplied. Relative paths +remain relative (including a leading `./` or `..`); absolute paths remain absolute. +Directory names are not shortened, and numeric IDs are not used as display labels. +The source path is shown once, rather than repeated below the label. + +Explicit label flags override path-based display labels. The first run is never +automatically named `Baseline`, nor are later runs automatically named `Candidate`. +The JSON keys `baseline` and `candidate` describe comparison roles only. + +HTML extends the existing evaluation report with a Comparison tab and a run +selector. In Comparison, the Reference and Compared selectors can choose any two +inputs, in either direction. The selected pair shows metrics, metadata, coverage, +and case transitions calculated with the same Go comparison logic. These controls +do not change the first-input reference used by CLI/CI gates or text/JSON/Markdown. +The case-response button opens the reused case viewer inside Comparison, showing +both selected files' primary case results side by side: prompts, full responses +(including recorded turns), grades and evidence. Case IDs absent on either side +are explicitly marked absent. Cross-file comparison is read-only; normal Run +details preserve each file's own with/without-Skill pairing and feedback. +Pair summaries are pre-rendered for every ordered pair, so HTML size grows with +the number of run pairs. No Agent or Judge is called when switching pairs. Selecting a run reuses the original Cases and Benchmark views, including +prompts, responses, formal grades, and evidence. The existing single-run and +with/without-Skill report presentation is unchanged. Comparison-mode feedback is +isolated per run and exported locally, without using the single-run feedback API. +HTML embeds its styles and filtering script, with +no external assets or network requests; all cases remain readable without +JavaScript. Markdown includes tables suitable for GitHub PR descriptions. +Source result artifacts are preserved; output may not overwrite any input. + +Cases align by case ID using the existing primary-result rule: prefer +`with_skill`, then a non-baseline configuration, falling back to `without_skill` +only when no other result for that case is available. In the usual with/without +benchmark, cross-file comparison therefore compares **each revision's with_skill +result**, not all variants against one another. This same selection rule drives +metrics, the case matrix, transitions, and the interactive side-by-side viewer. +Each file's internal with/without binding remains in Run details. The page keeps +this rule in a collapsed explanation; the full selection semantics are documented +here. Benchmark is the original single-run statistics view (including with/without +deltas when recorded), and is shown in comparison mode only when the selected +run actually contains benchmark data. The selected +configuration is exposed in the matrix and transition tables. + +For more than three inputs, overview/matrix columns retain readable widths and +scroll horizontally with the first column pinned. Selectors wrap to new rows +on narrow screens and reserve spacing for the dropdown arrow; the pair viewer +always compares two selected inputs, regardless of the total input count. +Benchmark reference entries do not double-count case counts or pass rates. +Configuration identity is retained in case tables. Shared cases flag prompt changes +when both prompts were recorded; otherwise the prompt comparison is unknown. +Requested, applied, and observed agent configurations are compared when recorded. Total tokens are the recorded +run-level total across all configurations; input/output tokens sum primary cases. +Duration is run wall time, and pass-rate changes are percentage points. + +Added and removed cases are shown separately, with shared-case coverage. Aggregate +changes include each run's full primary case set, so different case sets are not +paired samples. A change between two non-PASS statuses is classified as changed, +not fixed or regressed. Recorded zero tokens remain zero; missing token metrics +are `unknown` in human-readable reports and `null` in JSON, including deltas. +The token gate fails when the total-token measurement is unknown, or when a zero +baseline increases to a positive value. + +Reports compare recorded outcomes only and do not regrade scores. Missing +provenance is unknown; matching case IDs do not establish identical prompts, +suites, Skill revisions, or judge/rubric definitions. Review those conditions +before interpreting a change as an improvement. Gates are opt-in, apply equally +to every output format, and write the report before returning a gate failure. --- diff --git a/internal/cli/compare.go b/internal/cli/compare.go new file mode 100644 index 00000000..0a2932b8 --- /dev/null +++ b/internal/cli/compare.go @@ -0,0 +1,240 @@ +package cli + +import ( + "encoding/json" + "errors" + "fmt" + "math" + "os" + "path/filepath" + "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" +) + +const jsonFormat = "json" + +var compareCmd = &cobra.Command{ + Use: "compare ...", + Short: "Compare two or more offline evaluation results", + Long: `Compare existing results without running an Agent or Judge. + +The first input is the reference for changes and gates. Display labels use +the input paths exactly as supplied, unless explicitly overridden. + +Examples: + skill-up compare run-a/result.json run-b/result.json run-c/result.json --format html --output compare.html + skill-up compare old.json new.json --format html --output compare.html + skill-up compare old.json new.json --format markdown --output compare.md + skill-up compare old.json new.json --format json --fail-on-regression`, + Args: cobra.MinimumNArgs(2), + RunE: runCompare, +} + +func init() { + configureCompareFlags(compareCmd) +} + +func configureCompareFlags(cmd *cobra.Command) { + cmd.Flags().String("format", "text", "Output format: text, json, html, markdown") + cmd.Flags().String("output", "", "Write report to a file instead of stdout") + cmd.Flags().String("baseline-label", "", "Override the first run's automatic display label") + cmd.Flags().StringArray("candidate-label", nil, "Override later runs' automatic labels; repeat once per later input, or omit") + cmd.Flags().Bool("fail-on-regression", false, "Fail when any case regresses") + cmd.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) + } + switch format { + case "text", jsonFormat, "html", "markdown": + default: + return fmt.Errorf("unsupported compare format %q; supported formats: text, json, html, markdown", format) + } + + if len(args) < 2 { + return errors.New("compare requires at least two result files") + } + runs, err := loadBuildSet(cmd, args) + 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, err := comparepkg.BuildSet(runs, options) + if err != nil { + return err + } + data, err := comparepkg.RenderSet(result, format) + if err != nil { + return err + } + output, _ := cmd.Flags().GetString("output") + if err := writeComparison(cmd, args, output, data); err != nil { + return err + } + + if !result.Gates.Passed { + return fmt.Errorf("comparison gates failed: %v", result.Gates.Failures) + } + return nil +} + +func loadBuildSet(cmd *cobra.Command, args []string) ([]comparepkg.RunInput, error) { + labels := append([]string(nil), args...) + baselineLabel, _ := cmd.Flags().GetString("baseline-label") + if baselineLabel != "" { + labels[0] = baselineLabel + } + candidateLabels, _ := cmd.Flags().GetStringArray("candidate-label") + if len(candidateLabels) != 0 && len(candidateLabels) != len(args)-1 { + return nil, fmt.Errorf("--candidate-label must be omitted or supplied once per candidate: got %d labels for %d candidates", len(candidateLabels), len(args)-1) + } + runs := make([]comparepkg.RunInput, 0, len(args)) + for i, path := range args { + role := "old reference" + if i > 0 { + role = fmt.Sprintf("new input %d", i+1) + if len(candidateLabels) > 0 && candidateLabels[i-1] != "" { + labels[i] = candidateLabels[i-1] + } + } + input, missing, err := loadCompareInput(role, path) + if err != nil { + return nil, err + } + runs = append(runs, comparepkg.RunInput{Source: comparepkg.Source{Label: labels[i], Path: path}, Input: input, MissingMetrics: missing}) + } + return runs, nil +} + +func writeComparison(cmd *cobra.Command, inputs []string, output string, data []byte) error { + if output == "" { + if _, err := cmd.OutOrStdout().Write(data); err != nil { + return fmt.Errorf("write comparison: %w", err) + } + return nil + } + outPath, err := filepath.Abs(output) + if err != nil { + return fmt.Errorf("resolve output path: %w", err) + } + outInfo, statErr := os.Stat(output) + if statErr != nil && !os.IsNotExist(statErr) { + return fmt.Errorf("inspect output: %w", statErr) + } + for _, input := range inputs { + inPath, err := filepath.Abs(input) + if err != nil { + return fmt.Errorf("resolve input path: %w", err) + } + inInfo, err := os.Stat(input) + if err != nil { + return fmt.Errorf("inspect input: %w", err) + } + if outPath == inPath || (outInfo != nil && os.SameFile(outInfo, inInfo)) { + return errors.New("comparison output must not overwrite a source result") + } + } + if err := os.WriteFile(output, data, 0o600); err != nil { + return fmt.Errorf("write comparison %q: %w", output, err) + } + 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, []string, error) { + data, err := os.ReadFile(path) + if err != nil { + return report.Input{}, nil, 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{}, nil, fmt.Errorf("parse %s result %q: %w", role, path, err) + } + if err := validateCompareInput(input); err != nil { + return report.Input{}, nil, fmt.Errorf("validate %s result %q: %w", role, path, err) + } + return input, comparepkg.MissingMetrics(data, 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") + } + if input.EndTime.Before(input.StartTime) || input.TotalTokens < 0 { + return errors.New("invalid run duration or total tokens") + } + primary := input.PrimaryCaseResults() + if len(primary) == 0 { + return errors.New("no primary case results") + } + seen := make(map[[2]string]bool) + for _, result := range input.CaseResults { + key := [2]string{result.CaseID, result.Configuration} + if seen[key] { + return fmt.Errorf("duplicate case ID %q for configuration %q", result.CaseID, result.Configuration) + } + seen[key] = true + if err := validateCompareCase(result); err != nil { + return err + } + } + return nil +} + +func validateCompareCase(result report.CaseResult) error { + if result.DurationMs < 0 || result.InputTokens < 0 || result.OutputTokens < 0 { + return fmt.Errorf("case %q has negative metrics", result.CaseID) + } + 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 00000000..712da1c7 --- /dev/null +++ b/internal/cli/compare_test.go @@ -0,0 +1,524 @@ +package cli + +import ( + "bytes" + "encoding/json" + "math" + "os" + "path/filepath" + "strconv" + "strings" + "testing" + + "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" +) + +// newCompareCmd creates an isolated cobra command wired to runCompare for testing. +func newCompareCmd() *cobra.Command { + cmd := &cobra.Command{RunE: runCompare} + configureCompareFlags(cmd) + 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 { + Runs json.RawMessage `json:"runs"` + Comparisons []json.RawMessage `json:"comparisons"` + Gates json.RawMessage `json:"gates"` + } + if err := json.Unmarshal(out.Bytes(), &result); err != nil { + t.Fatalf("unmarshal compare output: %v", err) + } + if result.Runs == nil || len(result.Comparisons) != 1 || 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 +} + +func TestRunCompareShareableFilesSurviveGateFailure(t *testing.T) { + t.Parallel() + for _, format := range []string{"html", "markdown"} { + t.Run(format, func(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)) + original, err := os.ReadFile(oldPath) + if err != nil { + t.Fatal(err) + } + output := filepath.Join(dir, "comparison."+format) + cmd := newCompareCmd() + var stdout bytes.Buffer + cmd.SetOut(&stdout) + for flag, value := range map[string]string{"format": format, "output": output, "fail-on-regression": "true", "baseline-label": "Before", "candidate-label": "After"} { + if err := cmd.Flags().Set(flag, value); err != nil { + t.Fatal(err) + } + } + if err := runCompare(cmd, []string{oldPath, newPath}); err == nil || !strings.Contains(err.Error(), "comparison gates failed") { + t.Fatalf("expected gate failure: %v", err) + } + data, err := os.ReadFile(output) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{"Before", "After", "Regressed (1)", "1 case(s) regressed"} { + if !strings.Contains(string(data), want) { + t.Errorf("%s missing %q", format, want) + } + } + if stdout.Len() != 0 { + t.Fatalf("file output leaked into stdout: %s", stdout.String()) + } + retained, err := os.ReadFile(oldPath) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(original, retained) { + t.Fatal("source artifact modified") + } + }) + } +} + +func TestRunCompareProtectsSourceArtifacts(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)) + linked := filepath.Join(dir, "linked.json") + if err := os.Link(oldPath, linked); err != nil { + t.Fatal(err) + } + for _, output := range []string{oldPath, newPath, linked} { + cmd := newCompareCmd() + if err := cmd.Flags().Set("output", output); err != nil { + t.Fatal(err) + } + if err := runCompare(cmd, []string{oldPath, newPath}); err == nil || !strings.Contains(err.Error(), "must not overwrite") { + t.Fatalf("output %q: %v", output, err) + } + } +} + +func TestRootCommandIncludesCompare(t *testing.T) { + command, _, err := rootCmd.Find([]string{"compare"}) + if err != nil { + t.Fatal(err) + } + if command != compareCmd { + t.Fatal("compare command not registered") + } +} + +func TestRunCompareMultipleInputsAndLabels(t *testing.T) { + t.Parallel() + dir := t.TempDir() + paths := []string{ + writeCompareResultJSON(t, dir, "baseline.json", makeReportInput(judge.StatusPass)), + writeCompareResultJSON(t, dir, "same.json", makeReportInput(judge.StatusPass)), + writeCompareResultJSON(t, dir, "regressed.json", makeReportInput(judge.StatusFail)), + } + for _, format := range []string{"json", "text", "html", "markdown"} { + t.Run(format, func(t *testing.T) { + t.Parallel() + cmd := newCompareCmd() + var out bytes.Buffer + cmd.SetOut(&out) + for flag, value := range map[string]string{"format": format, "fail-on-regression": "true"} { + if err := cmd.Flags().Set(flag, value); err != nil { + t.Fatal(err) + } + } + for _, label := range []string{"Same Skill", "Revised Skill"} { + if err := cmd.Flags().Set("candidate-label", label); err != nil { + t.Fatal(err) + } + } + if err := runCompare(cmd, paths); err == nil || !strings.Contains(err.Error(), "Revised Skill") { + t.Fatalf("later candidate gate not enforced: %v", err) + } + for _, want := range []string{"Same Skill", "Revised Skill", "regressed.json"} { + if !strings.Contains(out.String(), want) { + t.Errorf("%s output lost %q", format, want) + } + } + if format == "json" { + assertMultipleCompareJSON(t, out.Bytes()) + } + }) + } +} + +func TestRunCompareValidatesEveryCandidateBeforeWriting(t *testing.T) { + t.Parallel() + dir := t.TempDir() + valid := writeCompareResultJSON(t, dir, "valid.json", makeReportInput(judge.StatusPass)) + missing := filepath.Join(dir, "missing-third.json") + cmd := newCompareCmd() + var out bytes.Buffer + cmd.SetOut(&out) + if err := runCompare(cmd, []string{valid, valid, missing}); err == nil || !strings.Contains(err.Error(), missing) { + t.Fatalf("later invalid input ignored: %v", err) + } + if out.Len() != 0 { + t.Fatal("partial report emitted for invalid inputs") + } + if err := cmd.Flags().Set("candidate-label", "only-one-label"); err != nil { + t.Fatal(err) + } + if err := runCompare(cmd, []string{valid, valid, valid}); err == nil || !strings.Contains(err.Error(), "2 candidates") { + t.Fatalf("ambiguous label count accepted: %v", err) + } +} + +func TestRunCompareProtectsLaterCandidate(t *testing.T) { + t.Parallel() + dir := t.TempDir() + base := writeCompareResultJSON(t, dir, "baseline.json", makeReportInput(judge.StatusPass)) + later := writeCompareResultJSON(t, dir, "later.json", makeReportInput(judge.StatusFail)) + cmd := newCompareCmd() + if err := cmd.Flags().Set("output", later); err != nil { + t.Fatal(err) + } + if err := runCompare(cmd, []string{base, base, later}); err == nil || !strings.Contains(err.Error(), "must not overwrite") { + t.Fatalf("later candidate can be overwritten: %v", err) + } +} + +func assertMultipleCompareJSON(t *testing.T, data []byte) { + t.Helper() + var set comparepkg.Set + if err := json.Unmarshal(data, &set); err != nil { + t.Fatal(err) + } + if len(set.Runs) != 3 || len(set.Comparisons) != 2 || !set.Comparisons[0].Gates.Passed || set.Comparisons[1].Gates.Passed { + t.Fatalf("wrong per-candidate gates: %+v", set) + } +} + +func TestAutomaticLabelsPropagateToReportsAndRunIdentity(t *testing.T) { + t.Parallel() + root := t.TempDir() + var paths []string + for _, name := range []string{"alpha", "beta", "gamma"} { + dir := filepath.Join(root, name) + if err := os.Mkdir(dir, 0o700); err != nil { + t.Fatal(err) + } + paths = append(paths, writeCompareResultJSON(t, dir, "result.json", makeReportInput(judge.StatusPass))) + } + current, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + relative, err := filepath.Rel(current, paths[0]) + if err != nil { + t.Fatal(err) + } + paths[0] = "." + string(filepath.Separator) + relative + for _, format := range []string{"json", "html", "markdown", "text"} { + cmd := newCompareCmd() + var out bytes.Buffer + cmd.SetOut(&out) + if err := cmd.Flags().Set("format", format); err != nil { + t.Fatal(err) + } + if err := runCompare(cmd, paths); err != nil { + t.Fatal(err) + } + if strings.Contains(out.String(), "Candidate 1") || strings.Contains(out.String(), "Baseline") { + t.Errorf("%s still uses role-based display labels", format) + } + if format == jsonFormat { + assertPathRunIdentity(t, out.Bytes(), paths) + } + } +} + +func assertPathRunIdentity(t *testing.T, data []byte, paths []string) { + t.Helper() + var set comparepkg.Set + if err := json.Unmarshal(data, &set); err != nil { + t.Fatal(err) + } + for i, path := range paths { + if source := set.Runs[i].Source; source.ID != i+1 || source.Label != path || source.Path != path { + t.Fatalf("wrong run identity: %+v", source) + } + } + if set.Baseline != set.Runs[0].Source || set.Comparisons[1].Candidate != set.Runs[2].Source { + t.Fatal("reference or comparison lost run identity") + } +} diff --git a/internal/cli/root.go b/internal/cli/root.go index 33819030..ab8e7ea7 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/compare/compare.go b/internal/compare/compare.go new file mode 100644 index 00000000..096d00b5 --- /dev/null +++ b/internal/compare/compare.go @@ -0,0 +1,276 @@ +// Package compare compares two offline skill-up report results. +package compare + +import ( + "encoding/json" + "slices" + "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 + OldMissingMetrics []string + NewMissingMetrics []string +} + +// Result is the stable comparison result used by text and JSON output. +type Result struct { + Baseline Source `json:"baseline"` + Candidate Source `json:"candidate"` + Coverage Coverage `json:"coverage"` + Metadata MetadataDiff `json:"metadata"` + Run RunComparison `json:"run"` + Cases CaseTransitions `json:"cases"` + Gates GateResult `json:"gates"` +} + +// Source identifies an original result artifact independently of its configuration. +type Source struct { + ID int `json:"id,omitempty"` + Label string `json:"label"` + Path string `json:"path"` +} + +// Coverage describes the shared case set, without treating added or removed cases as pairs. +type Coverage struct { + Shared int `json:"shared"` + Added int `json:"added"` + Removed int `json:"removed"` +} + +// MetadataDiff records old and new run metadata. +type MetadataDiff struct { + RequestedConfiguration FieldDiff[string] `json:"requested_configuration"` + AppliedConfiguration FieldDiff[string] `json:"applied_configuration"` + ObservedConfiguration FieldDiff[string] `json:"observed_configuration"` + 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 { + Missing []string `json:"-"` + 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"` +} + +// MarshalJSON preserves unknown measurements as null rather than measured zero. +func (m RunMetrics) MarshalJSON() ([]byte, error) { + type metrics RunMetrics + data, err := json.Marshal(metrics(m)) + if err != nil { + return nil, err + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(data, &fields); err != nil { + return nil, err + } + for _, name := range m.Missing { + fields[name] = json.RawMessage("null") + } + return json.Marshal(fields) +} + +// 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 { + PromptChanged *bool `json:"prompt_changed"` + OldConfiguration string `json:"old_configuration,omitempty"` + NewConfiguration string `json:"new_configuration,omitempty"` + 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) + oldMetrics.Missing = options.OldMissingMetrics + newMetrics.Missing = options.NewMissingMetrics + result := Result{ + Baseline: Source{Label: "Baseline"}, + Candidate: Source{Label: "Candidate"}, + Metadata: metadataDiff(oldInput, newInput), + Run: RunComparison{ + Old: oldMetrics, + New: newMetrics, + Delta: diffRunMetrics(oldMetrics, newMetrics), + }, + Cases: compareCases(oldInput.PrimaryCaseResults(), newInput.PrimaryCaseResults()), + } + result.Coverage = Coverage{ + Shared: oldMetrics.CaseCount - len(result.Cases.Removed), + Added: len(result.Cases.Added), Removed: len(result.Cases.Removed), + } + 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{ + Missing: slices.Concat(oldMetrics.Missing, newMetrics.Missing), + 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{ + RequestedConfiguration: fieldDiff(configurationText(oldInput.RequestedConfiguration), configurationText(newInput.RequestedConfiguration)), + AppliedConfiguration: fieldDiff(configurationText(oldInput.AppliedConfiguration), configurationText(newInput.AppliedConfiguration)), + ObservedConfiguration: fieldDiff(configurationText(oldInput.ObservedConfiguration), configurationText(newInput.ObservedConfiguration)), + 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 configurationText(configuration *report.AgentConfiguration) string { + if configuration == nil { + return "" + } + data, err := json.Marshal(configuration) + if err != nil { + return "" + } + return string(data) +} + +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{ + OldConfiguration: oldCase.Configuration, + CaseID: oldCase.CaseID, OldTitle: oldCase.Title, OldStatus: oldCase.Status, + }) + continue + } + + transition := CaseTransition{ + OldConfiguration: oldCase.Configuration, NewConfiguration: newCase.Configuration, + CaseID: oldCase.CaseID, OldTitle: oldCase.Title, NewTitle: newCase.Title, + OldStatus: oldCase.Status, NewStatus: newCase.Status, + } + if oldCase.Prompt != "" && newCase.Prompt != "" { + changed := oldCase.Prompt != newCase.Prompt + transition.PromptChanged = &changed + } + 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{ + NewConfiguration: newCase.Configuration, + 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 00000000..aeb3bab1 --- /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 pp)", + "Total tokens (all configurations): 100 -> 140 (+40)", + "Metadata differences", + "Model: 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 00000000..c6dfeea9 --- /dev/null +++ b/internal/compare/gates.go @@ -0,0 +1,34 @@ +package compare + +import ( + "fmt" + "slices" +) + +// 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 slices.Contains(result.Run.Old.Missing, "total_tokens") || slices.Contains(result.Run.New.Missing, "total_tokens"): + failures = append(failures, "token increase cannot be evaluated: total tokens is unknown") + 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/input.go b/internal/compare/input.go new file mode 100644 index 00000000..a4afaa33 --- /dev/null +++ b/internal/compare/input.go @@ -0,0 +1,47 @@ +package compare + +import ( + "encoding/json" + + "github.com/alibaba/skill-up/internal/report" +) + +// MissingMetrics identifies absent measurements in an already validated JSON input. +// Unmarshalling into report.Input alone would turn absent measurements into zero. +func MissingMetrics(data []byte, input report.Input) []string { + var raw struct { + TotalTokens *int `json:"total_tokens"` + Cases []struct { + CaseID string `json:"case_id"` + Configuration string `json:"configuration"` + Input *int `json:"input_tokens"` + Output *int `json:"output_tokens"` + } `json:"case_results"` + } + if err := json.Unmarshal(data, &raw); err != nil { + return []string{"total_tokens", "input_tokens", "output_tokens", "duration_ms"} + } + var missing []string + if raw.TotalTokens == nil { + missing = append(missing, "total_tokens") + } + selected := make(map[string]string) + for _, cr := range input.PrimaryCaseResults() { + selected[cr.CaseID] = cr.Configuration + } + inputKnown, outputKnown := true, true + for _, cr := range raw.Cases { + if selected[cr.CaseID] != cr.Configuration { + continue + } + inputKnown = inputKnown && cr.Input != nil + outputKnown = outputKnown && cr.Output != nil + } + if !inputKnown { + missing = append(missing, "input_tokens") + } + if !outputKnown { + missing = append(missing, "output_tokens") + } + return missing +} diff --git a/internal/compare/render.go b/internal/compare/render.go new file mode 100644 index 00000000..d020249a --- /dev/null +++ b/internal/compare/render.go @@ -0,0 +1,282 @@ +package compare + +import ( + "bytes" + _ "embed" + "encoding/json" + "fmt" + "html" + "html/template" + "math" + "slices" + "strconv" + "strings" + "time" + + "github.com/alibaba/skill-up/internal/report" +) + +const ( + unknownValue = "unknown" + comparisonNeutralClass = "comparison-neutral" + jsonFormat = "json" +) + +//go:embed templates/compare.html +var htmlSource string + +//go:embed templates/set.html +var setHTMLSource string + +var comparisonTemplate = template.Must(template.New("comparison").Funcs(template.FuncMap{"promptChange": promptChange, "statusClass": comparisonStatusClass, "metricClass": comparisonMetricClass, "deltaClass": comparisonDeltaClass, "changeClass": comparisonChangeClass}).Parse(htmlSource + setHTMLSource)) + +type ( + comparisonRow struct{ Name, Old, New, Delta string } + caseGroup struct { + Name string + Cases []CaseTransition + } +) + +type comparisonView struct { + Interactive bool + Result Result + Metrics []comparisonRow + Metadata []comparisonRow + Groups []caseGroup +} + +// Render emits the same comparison in text, JSON, HTML, or Markdown form. +func Render(result Result, format string) ([]byte, error) { + switch format { + case "text": + return []byte(RenderText(result)), nil + case jsonFormat: + data, err := json.MarshalIndent(result, "", " ") + if err != nil { + return nil, fmt.Errorf("encode comparison: %w", err) + } + return append(data, '\n'), nil + case "markdown": + return []byte(RenderMarkdown(result)), nil + case "html": + var b bytes.Buffer + if err := comparisonTemplate.ExecuteTemplate(&b, "comparison-body", newComparisonView(result)); err != nil { + return nil, fmt.Errorf("render HTML comparison: %w", err) + } + return report.RenderComparisonHTML([]report.ComparisonRun{{Label: result.Baseline.Label}}, template.HTML(b.String())) //nolint:gosec // html/template escaped all source content. + default: + return nil, fmt.Errorf("unsupported comparison format %q", format) + } +} + +func newComparisonView(result Result) comparisonView { + old, candidate, delta := result.Run.Old, result.Run.New, result.Run.Delta + view := comparisonView{ + Result: result, + Metrics: []comparisonRow{ + {"Pass rate", fmt.Sprintf("%.2f%%", old.PassRate*100), fmt.Sprintf("%.2f%%", candidate.PassRate*100), fmt.Sprintf("%+.2f pp", delta.PassRate*100)}, + {"Cases", strconv.Itoa(old.CaseCount), strconv.Itoa(candidate.CaseCount), fmt.Sprintf("%+d", delta.CaseCount)}, + {"Passed", strconv.Itoa(old.PassCount), strconv.Itoa(candidate.PassCount), fmt.Sprintf("%+d", delta.PassCount)}, + metricRow("Total tokens (all configurations)", "total_tokens", int64(old.TotalTokens), int64(candidate.TotalTokens), result.Run), + metricRow("Input tokens (primary cases)", "input_tokens", int64(old.InputTokens), int64(candidate.InputTokens), result.Run), + metricRow("Output tokens (primary cases)", "output_tokens", int64(old.OutputTokens), int64(candidate.OutputTokens), result.Run), + metricRow("Run duration (ms)", "duration_ms", old.DurationMs, candidate.DurationMs, result.Run), + }, + Groups: []caseGroup{ + {"Regressed", result.Cases.Regressed}, + {"Fixed", result.Cases.Fixed}, + {"Changed", result.Cases.Changed}, + {"Added", result.Cases.Added}, + {"Removed", result.Cases.Removed}, + {"Unchanged", result.Cases.Unchanged}, + }, + } + m := result.Metadata + view.Metadata = []comparisonRow{ + metadataRow("Skill", m.SkillName), metadataRow("Schema version", m.SchemaVersion), + metadataRow("Engine", m.EngineName), metadataRow("Model", m.ModelName), + metadataRow("Start time", fieldDiff(formatTime(m.StartTime.Old), formatTime(m.StartTime.New))), + metadataRow("End time", fieldDiff(formatTime(m.EndTime.Old), formatTime(m.EndTime.New))), + metadataRow("Requested configuration", m.RequestedConfiguration), + metadataRow("Applied configuration", m.AppliedConfiguration), + metadataRow("Observed configuration", m.ObservedConfiguration), + } + return view +} + +func promptChange(changed *bool) string { + if changed == nil { + return unknownValue + } + if *changed { + return "changed" + } + return "unchanged" +} + +func formatTime(t time.Time) string { + if t.IsZero() { + return unknownValue + } + return t.Format(time.RFC3339Nano) +} + +func metricRow(label, key string, old, candidate int64, run RunComparison) comparisonRow { + row := comparisonRow{label, strconv.FormatInt(old, 10), strconv.FormatInt(candidate, 10), fmt.Sprintf("%+d", candidate-old)} + if slices.Contains(run.Old.Missing, key) { + row.Old, row.Delta = unknownValue, unknownValue + } + if slices.Contains(run.New.Missing, key) { + row.New, row.Delta = unknownValue, unknownValue + } + return row +} + +func metadataRow(label string, diff FieldDiff[string]) comparisonRow { + row := comparisonRow{Name: label, Old: knownText(diff.Old), New: knownText(diff.New), Delta: "unchanged"} + if diff.Changed { + row.Delta = "changed" + } + if diff.Old == "" || diff.New == "" { + row.Delta = unknownValue + } + return row +} + +func knownText(value string) string { + if value == "" { + return unknownValue + } + return value +} + +// RenderMarkdown creates a shareable comparison for PR descriptions and CI artifacts. +func RenderMarkdown(result Result) string { + view := newComparisonView(result) + var b strings.Builder + b.WriteString("# Evaluation comparison\n\n") + fmt.Fprintf(&b, "**%s** → **%s**\n\n", markdownCell(result.Baseline.Label), markdownCell(result.Candidate.Label)) + fmt.Fprintf(&b, "Sources: %s → %s\n\n", markdownCell(result.Baseline.Path), markdownCell(result.Candidate.Path)) + fmt.Fprintf(&b, "Shared cases: **%d**; added: **%d**; removed: **%d**.\n\n", result.Coverage.Shared, result.Coverage.Added, result.Coverage.Removed) + fmt.Fprintf(&b, "Comparison coverage: %d / %d reference cases; %d / %d compared cases.\n\n", result.Coverage.Shared, result.Run.Old.CaseCount, result.Coverage.Shared, result.Run.New.CaseCount) + if result.Coverage.Added+result.Coverage.Removed > 0 { + b.WriteString("> Different case sets: run-level deltas include unpaired cases. Only shared case IDs are classified as fixed or regressed.\n\n") + } + writeMarkdownRows(&b, "Run summary", view.Metrics, result) + writeMarkdownRows(&b, "Metadata differences", view.Metadata, result) + b.WriteString("Missing provenance is unknown. Matching case IDs do not prove identical prompts, suites, Skill revisions, or judge/rubric definitions. Scores are not regraded.\n\n") + b.WriteString("## Case transitions\n\n") + for _, group := range view.Groups { + writeMarkdownGroup(&b, group) + } + b.WriteString("## Gates\n\n") + if result.Gates.Passed { + b.WriteString("Passed (configured checks only).\n") + } else { + b.WriteString("Failed.\n\n") + for _, failure := range result.Gates.Failures { + fmt.Fprintf(&b, "- %s\n", markdownCell(failure)) + } + } + return b.String() +} + +func writeMarkdownRows(b *strings.Builder, heading string, rows []comparisonRow, result Result) { + fmt.Fprintf(b, "## %s\n\n| Metric | %s | %s | Change |\n|---|---|---|---|\n", heading, markdownCell(result.Baseline.Label), markdownCell(result.Candidate.Label)) + for _, row := range rows { + fmt.Fprintf(b, "| %s | %s | %s | %s |\n", markdownCell(row.Name), markdownCell(row.Old), markdownCell(row.New), markdownCell(row.Delta)) + } + b.WriteByte('\n') +} + +func writeMarkdownGroup(b *strings.Builder, group caseGroup) { + fmt.Fprintf(b, "### %s (%d)\n\n", group.Name, len(group.Cases)) + if len(group.Cases) == 0 { + b.WriteString("None.\n\n") + return + } + b.WriteString("| Case ID | Reference title | Compared title | Reference status | Compared status | Reference configuration | Compared configuration | Prompt |\n|---|---|---|---|---|---|---|---|\n") + for _, cr := range group.Cases { + fmt.Fprintf(b, "| %s | %s | %s | %s | %s | %s | %s | %s |\n", markdownCell(cr.CaseID), markdownCell(cr.OldTitle), markdownCell(cr.NewTitle), markdownCell(string(cr.OldStatus)), markdownCell(string(cr.NewStatus)), markdownCell(cr.OldConfiguration), markdownCell(cr.NewConfiguration), promptChange(cr.PromptChanged)) + } + b.WriteByte('\n') +} + +func markdownCell(value string) string { + // Encode all Markdown control characters, pipes, and line breaks so source + // titles cannot add markup, links, or rows to a shared report. + value = html.EscapeString(value) + value = strings.NewReplacer("\r", " ", "\n", " ", "\\", "\", "|", "|", "`", "`", "*", "*", "_", "_", "[", "[", "]", "]").Replace(value) + if strings.TrimSpace(value) == "" { + return "—" + } + return value +} + +func comparisonStatusClass(value any) string { + switch strings.ToUpper(fmt.Sprint(value)) { + case "PASS": + return "status-badge sb-pass comparison-status" + case "FAIL": + return "status-badge sb-fail comparison-status" + case "ERROR": + return "status-badge sb-error comparison-status" + case "SKIP": + return "status-badge sb-skip comparison-status" + default: + return "status-badge comparison-absent" + } +} + +func comparisonNumber(value string) (float64, bool) { + fields := strings.Fields(strings.TrimSuffix(value, "%")) + if len(fields) == 0 { + return 0, false + } + number, err := strconv.ParseFloat(fields[0], 64) + return number, err == nil && !math.IsNaN(number) && !math.IsInf(number, 0) +} + +func comparisonMetricClass(name, value string) string { + if value == unknownValue { + return comparisonNeutralClass + } + if name != "Pass rate" { + return "" + } + number, known := comparisonNumber(value) + if !known { + return comparisonNeutralClass + } + if number == 100 { + return "comparison-positive" + } + if number == 0 { + return "comparison-negative" + } + return "comparison-warning" +} + +func comparisonDeltaClass(name, value string) string { + number, known := comparisonNumber(value) + if !known || number == 0 { + return comparisonNeutralClass + } + if strings.Contains(name, "tokens") || strings.Contains(name, "duration") { + number = -number + } else if name != "Pass rate" && name != "Passed" { + return "comparison-changed" + } + if number > 0 { + return "comparison-positive" + } + return "comparison-negative" +} + +func comparisonChangeClass(value string) string { + if value == "changed" { + return "comparison-changed" + } + return comparisonNeutralClass +} diff --git a/internal/compare/render_test.go b/internal/compare/render_test.go new file mode 100644 index 00000000..f8892d58 --- /dev/null +++ b/internal/compare/render_test.go @@ -0,0 +1,169 @@ +package compare + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/alibaba/skill-up/internal/judge" + "github.com/alibaba/skill-up/internal/report" +) + +func TestShareableReportsIncludeTransitionsAndCoverage(t *testing.T) { + t.Parallel() + oldInput, newInput := compareFixture() + oldInput.CaseResults = append(oldInput.CaseResults, report.CaseResult{CaseID: "removed", Status: judge.StatusPass}) + newInput.CaseResults[0].Status = judge.StatusFail + newInput.CaseResults = append(newInput.CaseResults, report.CaseResult{CaseID: "added", Status: judge.StatusPass}) + result := Compare(oldInput, newInput, Options{FailOnRegression: true}) + result.Baseline = Source{Label: "Previous Skill", Path: "old.json"} + result.Candidate = Source{Label: "Revised Skill", Path: "new.json"} + for _, format := range []string{"html", "markdown"} { + t.Run(format, func(t *testing.T) { + t.Parallel() + data, err := Render(result, format) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{"Previous Skill", "Revised Skill", "old.json", "new.json", "Run summary", "Metadata differences", "Regressed (1)", "Fixed (1)", "Added (1)", "Removed (1)", "Different case sets", "1 case(s) regressed", "unknown"} { + if !strings.Contains(string(data), want) { + t.Errorf("%s missing %q", format, want) + } + } + }) + } + if result.Coverage != (Coverage{Shared: 2, Added: 1, Removed: 1}) { + t.Fatalf("coverage = %+v", result.Coverage) + } +} + +func TestReportsEscapeUntrustedSourceText(t *testing.T) { + t.Parallel() + input := report.Input{CaseResults: []report.CaseResult{{CaseID: "case", Title: "\n| [link](https://example.com) `code` \\|", Status: judge.StatusPass}}} + result := Compare(input, input, Options{}) + result.Baseline.Label = "" + for _, format := range []string{"html", "markdown"} { + data, err := Render(result, format) + if err != nil { + t.Fatal(err) + } + text := string(data) + for _, unsafe := range []string{"", Input: input}} + output, err := RenderComparisonHTML(runs, template.HTML("

Comparison marker

")) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{"Comparison marker", "comparison-run", "original prompt", "original response", "reference response", "Formal Grades", "Your Feedback", "panel-benchmark"} { + if !strings.Contains(string(output), want) { + t.Errorf("original report feature missing: %s", want) + } + } + if strings.Contains(string(output), "