From f97da0ddfc5cb19b8edf705ed9047a3f662b8c0e Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 28 Sep 2026 12:59:53 +0200 Subject: [PATCH] conformance: judge the ruby/spec sweep per file, in Go (#710) The frozen scalar floor cannot be calibrated. A spec file that fails to load carries tens of examples -- one flip was measured moving the total by 43 on a documentation-only commit -- so the floor needs a margin wider than the largest file. But a gate with a 43-example margin is blind to every regression smaller than 43, and the margin actually in place is 6, which is why the lane has been both too loose and too tight at once. The number it reports is also unattributable. A drop of 43 is one file not loading, or 43 specs regressing. Those need opposite responses -- rerun versus bisect -- and the total cannot tell them apart. A per-file baseline needs no margin at all. Each file is judged against its own recorded count, so one example lost in one file is visible, while a file that failed to load is reported as a load failure and never summed into the same number. The total is still printed, as a derived summary rather than as the thing being gated. This lands the judge and its tests. It is deliberately NOT yet wired into run.sh: the baseline has to be measured on an unloaded machine, and three conformance waves are running on this one right now. A pass/fail count is corrupted under load through per-file timeouts -- that is exactly the failure this tool exists to attribute -- so measuring the baseline now would bake the load into the gate. The switch-over, with the measured baseline, is a follow-up. Written in Go rather than added to the shell because this is the judging half, where a silent failure reads as a clean result. The tool refuses input it cannot read: an empty sweep, an unrecognised record, a pass count that is not a number, and the same file twice are each exit 2, distinct from exit 1 for a real regression, so a broken ratchet cannot be mistaken for a failing one. Seventeen cases, each differing only in the sweep fed to one fixed baseline, so the verdict provably depends on the input. Among them the case the scalar floor cannot see at all: one file loses an example while another gains nineteen, the total RISES, and the ratchet still fails and names the file. Co-Authored-By: Claude Opus 5 --- .../conformance/rubyspec/cmd/ratchet/main.go | 277 ++++++++++++++++++ .../rubyspec/cmd/ratchet/main_test.go | 181 ++++++++++++ 2 files changed, 458 insertions(+) create mode 100644 scripts/conformance/rubyspec/cmd/ratchet/main.go create mode 100644 scripts/conformance/rubyspec/cmd/ratchet/main_test.go diff --git a/scripts/conformance/rubyspec/cmd/ratchet/main.go b/scripts/conformance/rubyspec/cmd/ratchet/main.go new file mode 100644 index 00000000..05350f11 --- /dev/null +++ b/scripts/conformance/rubyspec/cmd/ratchet/main.go @@ -0,0 +1,277 @@ +// Command ratchet judges one ruby/spec sweep against a per-file baseline. +// +// It replaces a single frozen scalar. The scalar could not work: a spec file +// that fails to load carries tens of examples -- one flip was measured moving +// the total by 43 on a documentation-only commit -- so the floor needed a margin +// wider than the largest file, and a gate with a 43-example margin is blind to +// every regression smaller than 43. Worse, the number it reported was +// unattributable: a drop of 43 is one file not loading, or 43 specs regressing, +// and those need opposite responses. +// +// A per-file baseline needs no margin at all. Each file is judged against its +// own recorded count, so a real regression of one example in one file is visible +// while a file that failed to load is reported as a load failure and not summed +// into the same number. The total is still printed, but as a derived summary +// rather than as the thing being gated. +// +// It reads the sweep's TSV on stdin or from -results: +// +// OK\t\t +// FILEFAIL\t +// +// and the baseline from -baseline, in the same shape minus the counts it does +// not need. Exit 0 means no file went backwards. +package main + +import ( + "bufio" + "errors" + "flag" + "fmt" + "io" + "os" + "sort" + "strconv" + "strings" +) + +// state is what the baseline records for one file, and what a sweep measures. +// A file that did not load is not the same as a file that loaded and passed +// zero examples: the first is an infrastructure fact, the second a conformance +// one, and collapsing them is how the scalar floor became unattributable. +type state struct { + loaded bool + pass int +} + +func (s state) String() string { + if !s.loaded { + return "did not load" + } + return fmt.Sprintf("%d passing", s.pass) +} + +// parse reads the sweep/baseline TSV. An unrecognised verb is an error rather +// than a skipped line: a sweep whose format drifted must not be read as a clean +// result, which is the failure mode this whole tool exists to prevent. +func parse(r io.Reader, what string) (map[string]state, error) { + out := make(map[string]state) + sc := bufio.NewScanner(r) + sc.Buffer(make([]byte, 0, 64*1024), 4*1024*1024) + for n := 1; sc.Scan(); n++ { + line := strings.TrimRight(sc.Text(), "\r") + if line == "" || strings.HasPrefix(line, "#") { + continue + } + f := strings.Split(line, "\t") + switch { + case f[0] == "OK" && len(f) == 3: + p, err := strconv.Atoi(f[2]) + if err != nil { + return nil, fmt.Errorf("%s line %d: pass count %q is not a number", what, n, f[2]) + } + if prev, dup := out[f[1]]; dup { + return nil, fmt.Errorf("%s line %d: %s appears twice (was %s)", what, n, f[1], prev) + } + out[f[1]] = state{loaded: true, pass: p} + case f[0] == "FILEFAIL" && len(f) == 2: + if prev, dup := out[f[1]]; dup { + return nil, fmt.Errorf("%s line %d: %s appears twice (was %s)", what, n, f[1], prev) + } + out[f[1]] = state{} + default: + return nil, fmt.Errorf("%s line %d: unrecognised record %q", what, n, line) + } + } + if err := sc.Err(); err != nil { + return nil, fmt.Errorf("reading %s: %w", what, err) + } + if len(out) == 0 { + return nil, fmt.Errorf("%s is empty -- a sweep that measured nothing must not read as a clean result", what) + } + return out, nil +} + +// write emits a baseline in the same format it reads, sorted, so a regenerated +// baseline diffs cleanly against its predecessor. +func write(w io.Writer, m map[string]state, header string) error { + paths := make([]string, 0, len(m)) + for p := range m { + paths = append(paths, p) + } + sort.Strings(paths) + b := bufio.NewWriter(w) + fmt.Fprint(b, header) + for _, p := range paths { + if s := m[p]; s.loaded { + fmt.Fprintf(b, "OK\t%s\t%d\n", p, s.pass) + } else { + fmt.Fprintf(b, "FILEFAIL\t%s\n", p) + } + } + return b.Flush() +} + +// verdict is the whole comparison, computed before anything is printed so the +// summary cannot disagree with the detail. +type verdict struct { + regressed []string // loaded, but fewer passing than the baseline + unloaded []string // baseline says it loaded; this sweep says it did not + missing []string // in the baseline, absent from the sweep entirely + improved []string // more passing than the baseline + nowLoading []string // baseline says it did not load; this sweep says it does + added []string // in the sweep, absent from the baseline (corpus bump) + base, got int // derived totals, reported but not gated +} + +func (v *verdict) bad() bool { + return len(v.regressed) > 0 || len(v.unloaded) > 0 || len(v.missing) > 0 +} + +func compare(base, got map[string]state) *verdict { + v := &verdict{} + for p, b := range base { + v.base += b.pass + g, present := got[p] + switch { + case !present: + v.missing = append(v.missing, p) + case b.loaded && !g.loaded: + v.unloaded = append(v.unloaded, fmt.Sprintf("%s (baseline: %s)", p, b)) + case g.loaded && g.pass < b.pass: + v.regressed = append(v.regressed, fmt.Sprintf("%s: %d passing, baseline %d (-%d)", p, g.pass, b.pass, b.pass-g.pass)) + case g.loaded && g.pass > b.pass: + if !b.loaded { + v.nowLoading = append(v.nowLoading, fmt.Sprintf("%s: now loads, %d passing", p, g.pass)) + } else { + v.improved = append(v.improved, fmt.Sprintf("%s: %d passing, baseline %d (+%d)", p, g.pass, b.pass, g.pass-b.pass)) + } + } + } + for p, g := range got { + v.got += g.pass + if _, present := base[p]; !present { + v.added = append(v.added, fmt.Sprintf("%s: %s", p, g)) + } + } + for _, s := range [][]string{v.regressed, v.unloaded, v.missing, v.improved, v.nowLoading, v.added} { + sort.Strings(s) + } + return v +} + +// section prints a named list, or nothing at all when it is empty. The count is +// always in the heading: a heading without one invites the reader to guess. +func section(w io.Writer, title string, items []string, limit int) { + if len(items) == 0 { + return + } + fmt.Fprintf(w, "\n-- %s (%d) --\n", title, len(items)) + for i, s := range items { + if limit > 0 && i == limit { + fmt.Fprintf(w, " ... and %d more\n", len(items)-limit) + break + } + fmt.Fprintf(w, " %s\n", s) + } +} + +// errRegression is the one failure that is not an error in the tool: the sweep +// was read correctly and the answer is no. main maps it to exit 1, distinct from +// exit 2 for a tool or input failure, so a broken ratchet cannot be mistaken for +// a failing one. It is a sentinel rather than an os.Exit inside run so that the +// verdict is reachable from a test. +var errRegression = errors.New("regression") + +func main() { + err := run(os.Args[1:], os.Stdin, os.Stdout) + switch { + case err == nil: + return + case errors.Is(err, errRegression): + os.Exit(1) + default: + fmt.Fprintf(os.Stderr, "ratchet: %v\n", err) + os.Exit(2) + } +} + +func run(args []string, stdin io.Reader, stdout io.Writer) error { + fs := flag.NewFlagSet("ratchet", flag.ContinueOnError) + fs.SetOutput(stdout) + baseline := fs.String("baseline", "", "per-file baseline TSV (required)") + results := fs.String("results", "", "sweep results TSV (default: stdin)") + update := fs.Bool("update", false, "rewrite the baseline from this sweep and exit 0") + limit := fs.Int("limit", 40, "cap each printed section at N entries (0 = no cap)") + if err := fs.Parse(args); err != nil { + return err + } + if *baseline == "" { + return fmt.Errorf("-baseline is required") + } + + var src io.Reader = stdin + if *results != "" { + f, err := os.Open(*results) + if err != nil { + return err + } + defer f.Close() + src = f + } + got, err := parse(src, "sweep results") + if err != nil { + return err + } + + if *update { + const header = "# ruby/spec per-file baseline. Regenerate with UPDATE_BASELINE=1 scripts/conformance/rubyspec/run.sh.\n" + + "# One record per spec file: OKpathpassing, or FILEFAILpath.\n" + + "# Judged per file, so no margin is needed and every move is attributable.\n" + f, err := os.Create(*baseline) + if err != nil { + return err + } + if err := write(f, got, header); err != nil { + f.Close() + return err + } + if err := f.Close(); err != nil { + return err + } + fmt.Fprintf(stdout, "baseline rewritten: %d files\n", len(got)) + return nil + } + + bf, err := os.Open(*baseline) + if err != nil { + return err + } + defer bf.Close() + base, err := parse(bf, "baseline") + if err != nil { + return err + } + + v := compare(base, got) + fmt.Fprintf(stdout, "files: %d measured, %d in baseline\n", len(got), len(base)) + fmt.Fprintf(stdout, "passing examples: %d (baseline %d, %+d)\n", v.got, v.base, v.got-v.base) + + section(stdout, "REGRESSED", v.regressed, *limit) + section(stdout, "FAILED TO LOAD (the baseline says they load)", v.unloaded, *limit) + section(stdout, "MISSING FROM THE SWEEP ENTIRELY", v.missing, *limit) + section(stdout, "improved", v.improved, *limit) + section(stdout, "now loading", v.nowLoading, *limit) + section(stdout, "new files, not gated", v.added, *limit) + + if v.bad() { + fmt.Fprintf(stdout, "\n::error::ruby/spec ratchet REGRESSION in %d file(s) -- named above\n", + len(v.regressed)+len(v.unloaded)+len(v.missing)) + return errRegression + } + if n := len(v.improved) + len(v.nowLoading); n > 0 { + fmt.Fprintf(stdout, "\n%d file(s) improved -- rerun with UPDATE_BASELINE=1 to lock it in.\n", n) + } + fmt.Fprintln(stdout, "\nruby/spec ratchet OK -- no file went backwards") + return nil +} diff --git a/scripts/conformance/rubyspec/cmd/ratchet/main_test.go b/scripts/conformance/rubyspec/cmd/ratchet/main_test.go new file mode 100644 index 00000000..e5e13d86 --- /dev/null +++ b/scripts/conformance/rubyspec/cmd/ratchet/main_test.go @@ -0,0 +1,181 @@ +package main + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" +) + +const baseline3 = "OK\tcore/a_spec.rb\t10\nOK\tcore/b_spec.rb\t20\nFILEFAIL\tcore/c_spec.rb\n" + +// judge runs the tool over a sweep against a fixed three-file baseline and +// returns its output and error, so every case below differs only in its input. +func judge(t *testing.T, sweep string, extra ...string) (string, error) { + t.Helper() + dir := t.TempDir() + bp := filepath.Join(dir, "BASELINE") + if err := os.WriteFile(bp, []byte(baseline3), 0o644); err != nil { + t.Fatal(err) + } + var out strings.Builder + err := run(append([]string{"-baseline", bp}, extra...), strings.NewReader(sweep), &out) + return out.String(), err +} + +// TestRatchetVerdicts is the whole point of the tool: the answer must depend on +// the input. Each case is one shape of move, and the pairs that differ by one +// example exist because a scalar floor with a margin wide enough for a whole +// file could not see them at all. +func TestRatchetVerdicts(t *testing.T) { + for _, tc := range []struct { + name string + sweep string + wantBad bool + want []string + absent []string + }{ + { + name: "unchanged", + sweep: baseline3, + want: []string{"no file went backwards", "passing examples: 30 (baseline 30, +0)"}, + }, + { + name: "one example lost in one file", + sweep: "OK\tcore/a_spec.rb\t9\nOK\tcore/b_spec.rb\t20\nFILEFAIL\tcore/c_spec.rb\n", + wantBad: true, + want: []string{"REGRESSED (1)", "core/a_spec.rb: 9 passing, baseline 10 (-1)"}, + }, + { + // The case the scalar floor was blind to: one file gains more than + // another loses, so the total RISES while a file went backwards. + name: "a regression hidden under a larger gain", + sweep: "OK\tcore/a_spec.rb\t9\nOK\tcore/b_spec.rb\t40\nFILEFAIL\tcore/c_spec.rb\n", + wantBad: true, + want: []string{"REGRESSED (1)", "core/a_spec.rb: 9 passing, baseline 10 (-1)", "passing examples: 49 (baseline 30, +19)"}, + }, + { + name: "a file stops loading", + sweep: "FILEFAIL\tcore/a_spec.rb\nOK\tcore/b_spec.rb\t20\nFILEFAIL\tcore/c_spec.rb\n", + wantBad: true, + want: []string{"FAILED TO LOAD", "core/a_spec.rb (baseline: 10 passing)"}, + // A load failure must not be reported as a conformance regression: + // the two need opposite responses. + absent: []string{"REGRESSED"}, + }, + { + name: "a file vanishes from the sweep", + sweep: "OK\tcore/a_spec.rb\t10\nOK\tcore/b_spec.rb\t20\n", + wantBad: true, + want: []string{"MISSING FROM THE SWEEP ENTIRELY", "core/c_spec.rb"}, + }, + { + name: "improvement is reported, not gated", + sweep: "OK\tcore/a_spec.rb\t11\nOK\tcore/b_spec.rb\t20\nFILEFAIL\tcore/c_spec.rb\n", + want: []string{"improved (1)", "core/a_spec.rb: 11 passing, baseline 10 (+1)", "UPDATE_BASELINE=1"}, + }, + { + // A file the baseline records as not loading, now loading, is an + // improvement and not a "regressed to fewer than zero". + name: "a file starts loading", + sweep: "OK\tcore/a_spec.rb\t10\nOK\tcore/b_spec.rb\t20\nOK\tcore/c_spec.rb\t5\n", + want: []string{"now loading (1)", "core/c_spec.rb: now loads, 5 passing"}, + }, + { + name: "a new file is reported and not gated", + sweep: baseline3 + "OK\tcore/d_spec.rb\t7\n", + want: []string{"new files, not gated (1)", "core/d_spec.rb: 7 passing"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + out, err := judge(t, tc.sweep) + if bad := errors.Is(err, errRegression); bad != tc.wantBad { + t.Errorf("regression = %v, want %v (err=%v)\n%s", bad, tc.wantBad, err, out) + } + if err != nil && !errors.Is(err, errRegression) { + t.Fatalf("unexpected tool error: %v\n%s", err, out) + } + for _, w := range tc.want { + if !strings.Contains(out, w) { + t.Errorf("output does not contain %q\n%s", w, out) + } + } + for _, a := range tc.absent { + if strings.Contains(out, a) { + t.Errorf("output should not contain %q\n%s", a, out) + } + } + }) + } +} + +// TestRatchetRejectsUnreadableInput: a sweep this tool cannot read must not be +// reported as a clean machine. Silence is the failure mode the whole tool exists +// to prevent, so every one of these is exit 2, distinct from a regression. +func TestRatchetRejectsUnreadableInput(t *testing.T) { + for _, tc := range []struct{ name, sweep, want string }{ + {"empty", "", "measured nothing"}, + {"unknown verb", "MAYBE\tcore/a_spec.rb\t1\n", "unrecognised record"}, + {"OK without a count", "OK\tcore/a_spec.rb\n", "unrecognised record"}, + {"count is not a number", "OK\tcore/a_spec.rb\tmany\n", "is not a number"}, + {"the same file twice", "OK\tcore/a_spec.rb\t1\nOK\tcore/a_spec.rb\t2\n", "appears twice"}, + } { + t.Run(tc.name, func(t *testing.T) { + out, err := judge(t, tc.sweep) + if err == nil || errors.Is(err, errRegression) { + t.Fatalf("want a tool error, got %v\n%s", err, out) + } + if !strings.Contains(err.Error(), tc.want) { + t.Errorf("error %q does not mention %q", err, tc.want) + } + }) + } +} + +// TestRatchetUpdateRoundTrips: -update must write a baseline this tool reads +// back identically, and sorted, so a regenerated baseline diffs cleanly. +func TestRatchetUpdateRoundTrips(t *testing.T) { + dir := t.TempDir() + bp := filepath.Join(dir, "BASELINE") + if err := os.WriteFile(bp, []byte(baseline3), 0o644); err != nil { + t.Fatal(err) + } + sweep := "OK\tcore/b_spec.rb\t20\nFILEFAIL\tcore/c_spec.rb\nOK\tcore/a_spec.rb\t11\n" + var out strings.Builder + if err := run([]string{"-baseline", bp, "-update"}, strings.NewReader(sweep), &out); err != nil { + t.Fatalf("update: %v", err) + } + got, err := os.ReadFile(bp) + if err != nil { + t.Fatal(err) + } + body := got + for len(body) > 0 && body[0] == '#' { + i := strings.IndexByte(string(body), '\n') + body = body[i+1:] + } + const want = "OK\tcore/a_spec.rb\t11\nOK\tcore/b_spec.rb\t20\nFILEFAIL\tcore/c_spec.rb\n" + if string(body) != want { + t.Errorf("baseline written as %q, want %q (sorted, same shape it reads)", body, want) + } + // And the written baseline now accepts the sweep it was written from. + var out2 strings.Builder + if err := run([]string{"-baseline", bp}, strings.NewReader(sweep), &out2); err != nil { + t.Errorf("the regenerated baseline rejects its own sweep: %v\n%s", err, out2.String()) + } +} + +// TestRatchetMissingBaselineIsAnError: pointing at a baseline that is not there +// must fail loudly. A tool that treats an absent baseline as "nothing to beat" +// would pass forever. +func TestRatchetMissingBaselineIsAnError(t *testing.T) { + var out strings.Builder + err := run([]string{"-baseline", filepath.Join(t.TempDir(), "nope")}, strings.NewReader(baseline3), &out) + if err == nil || errors.Is(err, errRegression) { + t.Fatalf("want a tool error for a missing baseline, got %v", err) + } + if err := run(nil, strings.NewReader(baseline3), &out); err == nil { + t.Error("want an error when -baseline is omitted entirely") + } +}