From 10f9aad15d47b1ea3cd9fe00848628f207f6146e Mon Sep 17 00:00:00 2001 From: Bozhidar Batsov Date: Tue, 3 Nov 2026 14:46:06 +0200 Subject: [PATCH] Show how each client fares with the servers in the matrix The matrix page gets a second tab with a row per client, where each server gets the number of checks it passes out of the ones that would break the client, with the ones it failed on hover. A check breaks the clients whose code its refs point at (a client profile's checks point at their own client), or every client when it points at none (e.g. eval.value). Warnings break nobody. JSON reports name the clients of each result, so a report keeps working with the client profiles of the proof that made it. Clients come in the order of how many checks they depend on, which puts the ones proof knows the most about first. --- CHANGELOG.md | 4 + cmd/proof/main.go | 3 + doc/usage.md | 6 ++ internal/check/check.go | 3 + internal/checks/client_profiles.go | 21 ++++++ internal/checks/client_profiles_test.go | 56 +++++++++++++- internal/report/matrix.go | 97 ++++++++++++++++++++++--- internal/report/matrix.html | 43 ++++++++++- internal/report/report.go | 3 +- internal/report/report_test.go | 44 ++++++++++- 10 files changed, 263 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e08e44d..b1f7c4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## main (unreleased) +### Features + +- [#23](https://github.com/nrepl/proof/pull/23): A clients tab on the compatibility matrix page, with how many of the checks each client depends on every server passes. + ### Changes - [#22](https://github.com/nrepl/proof/pull/22): The bundled profiles have short names, with versions from their `version` snippets, so the servers in the compatibility matrix get columns of the same width. diff --git a/cmd/proof/main.go b/cmd/proof/main.go index ae00dd3..8525e94 100644 --- a/cmd/proof/main.go +++ b/cmd/proof/main.go @@ -217,6 +217,9 @@ func run(args []string) int { env := &check.Env{Profile: p, Addr: addr} started := time.Now() results := check.Run(env, selected, checks.WireRules()) + for i, res := range results { + results[i].Clients = checks.ClientsOf(res.Severity, res.Refs) + } outcome := check.Apply(results, p.ExpectedFailures, opts.only == "") r := report.Run{Proof: proofVersion(), Server: p.Name, ServerVersion: ver, Homepage: p.Homepage, diff --git a/doc/usage.md b/doc/usage.md index d98ef3b..9e1dcc5 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -236,6 +236,12 @@ shows why it matters and hovering over a verdict shows its details: $ proof matrix -html clojure.json my-server.json > matrix.html ``` +The page has a second tab for the clients proof knows about, with one +row per client. There each server gets the number of checks it passes out +of the ones that would break the client if they failed: the client's own +checks (e.g. `cider.connect`), the checks that link to its code, and the +ones every client depends on (e.g. `eval.value`). + > [!TIP] > proof's own CI builds such a page for all the profiles in the > repository on a daily basis. You can find it at diff --git a/internal/check/check.go b/internal/check/check.go index 9034580..58518ae 100644 --- a/internal/check/check.go +++ b/internal/check/check.go @@ -286,6 +286,9 @@ type Result struct { // Expected holds the reason from the profile's expected failures when // the check failed as expected. Expected string + // Clients names the clients that break when the server fails the + // check. + Clients []string } // Run runs checks one at a time, then grades the wire traffic they diff --git a/internal/checks/client_profiles.go b/internal/checks/client_profiles.go index 4ec91ea..4ab3927 100644 --- a/internal/checks/client_profiles.go +++ b/internal/checks/client_profiles.go @@ -231,3 +231,24 @@ func nonDictField(resp nrepl.Response, path []string) any { } return v } + +// ClientsOf names the clients that break when a server fails a check or +// rule with the given severity and refs: the ones with a client profile +// whose code the refs point at, or every one of them when the refs don't +// point at any (e.g. a value that never comes). A warning breaks nobody. +func ClientsOf(severity check.Severity, refs []check.Ref) []string { + if severity != check.Fail { + return nil + } + var cited, all []string + for _, p := range clientProfiles() { + all = append(all, p.Name) + if slices.ContainsFunc(refs, func(r check.Ref) bool { return strings.HasPrefix(r.URL, p.Code) }) { + cited = append(cited, p.Name) + } + } + if len(cited) == 0 { + return all + } + return cited +} diff --git a/internal/checks/client_profiles_test.go b/internal/checks/client_profiles_test.go index 6172a1b..85c2f73 100644 --- a/internal/checks/client_profiles_test.go +++ b/internal/checks/client_profiles_test.go @@ -2,6 +2,7 @@ package checks import ( "regexp" + "slices" "strings" "testing" @@ -20,9 +21,10 @@ func TestClientProfiles(t *testing.T) { ids[c.ID] = true } pinned := regexp.MustCompile(`^https://github\.com/[^/]+/[^/]+/blob/[0-9a-f]{40}/`) - // The links in refs.go to the same clients. + // The links in refs.go to the same clients, which is how a check knows + // the clients it breaks. bases := map[string]string{"CIDER": ciderBase, "Calva": calvaSrc, "Conjure": conjureSrc, "vim-fireplace": fireplaceBase, - "REPLy": strings.TrimSuffix(replyBase, "eval_modes/nrepl.clj")} + "REPLy": strings.TrimSuffix(replyBase, "eval_modes/nrepl.clj"), "neorepl": neoreplBase} for _, p := range clientProfiles() { if p.Name == "" || !pinned.MatchString(p.Code) { t.Errorf("%q needs a name and links pinned to a commit, got %q", p.Name, p.Code) @@ -56,6 +58,56 @@ func TestClientProfiles(t *testing.T) { } } +// A check breaks the clients whose code it links to, or all of them when +// it links to none, unless it's only a warning. Every client has checks +// of its own. +func TestChecksNameTheClientsTheyBreak(t *testing.T) { + var all []string + for _, p := range clientProfiles() { + all = append(all, p.Name) + } + clients := map[string][]string{} + var everyClient []string + note := func(id string, severity check.Severity, refs []check.Ref) { + clients[id] = ClientsOf(severity, refs) + if slices.Equal(clients[id], all) { + everyClient = append(everyClient, id) + } + } + for _, c := range All() { + note(c.ID, c.Severity, c.Refs) + } + for _, r := range WireRules() { + note(r.ID, r.Severity, r.Refs) + } + for id, want := range map[string][]string{ + "cider.connect": {"CIDER"}, + "eval.error-status": {"Calva", "CIDER", "REPLy"}, + // rebel-readline has no client profile. + "wire.error-terminal": {"REPLy"}, + "describe.versions": nil, + } { + if !slices.Equal(clients[id], want) { + t.Errorf("%s breaks %v, not %v", id, clients[id], want) + } + } + // A check gets here by linking to no client's code, which is also what + // happens when it links to a client without a profile, so each one has + // to be listed. + want := []string{"session.persistent", "eval.value", "eval.stdout", "eval.stderr", "eval.survives-error", + "eval.multiple-forms", "robust.bad-frames", "wire.bencode", "wire.dict"} + if !slices.Equal(everyClient, want) { + t.Errorf("the checks that break every client are %v, not %v", everyClient, want) + } + for _, p := range clientProfiles() { + for _, c := range p.Checks { + if !slices.Equal(clients[c.ID], []string{p.Name}) { + t.Errorf("%s breaks %v, not just %s", c.ID, clients[c.ID], p.Name) + } + } + } +} + // Replies the client can live with still get a note when they say // something went wrong. func TestClientChecksNoteErrors(t *testing.T) { diff --git a/internal/report/matrix.go b/internal/report/matrix.go index 754a59f..03b3603 100644 --- a/internal/report/matrix.go +++ b/internal/report/matrix.go @@ -6,6 +6,7 @@ import ( "fmt" "html" "io" + "maps" "os" "slices" "strings" @@ -77,8 +78,14 @@ func (t table) place(id string) int { // passed returns how many checks the server of the i-th report passed, // out of the ones that ran. func (t table) passed(i int) string { + return score(slices.Collect(maps.Values(t.results[i]))) +} + +// score says how many of the results are passes, out of the ones that +// ran. +func score(results []jsonResult) string { passed, ran := 0, 0 - for _, res := range t.results[i] { + for _, res := range results { if res.Verdict == check.Skipped { continue } @@ -90,6 +97,62 @@ func (t table) passed(i int) string { return fmt.Sprintf("%d/%d", passed, ran) } +// clients are the clients that the checks say they break, the ones the +// most checks break first. Every report has a say, as the ones made +// before proof named clients don't name any. +func (t table) clients() []string { + var names []string + counts := map[string]int{} + for _, id := range t.ids { + var named []string + for _, results := range t.results { + for _, name := range results[id].Clients { + if !slices.Contains(named, name) { + named = append(named, name) + } + } + } + for _, name := range named { + if counts[name] == 0 { + names = append(names, name) + } + counts[name]++ + } + } + slices.SortStableFunc(names, func(a, b string) int { return counts[b] - counts[a] }) + return names +} + +// client is how the server of the i-th report fares with a client: the +// score of the checks that break the client, and the ones it didn't +// pass. The verdict is the worst of theirs, or skip when none ran. +func (t table) client(i int, name string) (check.Verdict, string, []string) { + verdict := check.Pass + var ran []jsonResult + var missed []string + for _, id := range t.ids { + res, ok := t.results[i][id] + if !ok || res.Verdict == check.Skipped || !slices.Contains(res.Clients, name) { + continue + } + ran = append(ran, res) + switch res.Verdict { + case check.Failed: + verdict = check.Failed + missed = append(missed, res.ID+": "+res.Title) + case check.Errored: + if verdict != check.Failed { + verdict = check.Errored + } + missed = append(missed, res.ID+": "+res.Title+" (error)") + } + } + if len(ran) == 0 { + return check.Skipped, "skip", nil + } + return verdict, score(ran), missed +} + var cells = map[check.Verdict]string{ check.Pass: "pass", check.Failed: "**fail**", @@ -129,7 +192,7 @@ func escape(s string) string { return strings.ReplaceAll(s, "|", `\|`) } -// The page comes from matrix.html, with the table and the versions of +// The page comes from matrix.html, with the tables and the versions of // proof where its markers are. // //go:embed matrix.html @@ -137,24 +200,25 @@ var matrixPage string // HTML writes the same matrix as Matrix as a web page, with the checks // grouped like in the text report. The why of a check and the details of -// a verdict show up when hovering over them. +// a verdict show up when hovering over them. Another tab of the page has +// how each client fares with the servers. func HTML(w io.Writer, reports []Report) error { t := newTable(reports) esc := html.EscapeString - var b strings.Builder - b.WriteString("\nCheck") + var servers strings.Builder for _, r := range reports { name := esc(r.Server) if r.Homepage != "" { name = fmt.Sprintf(`%s`, esc(r.Homepage), name) } - fmt.Fprintf(&b, "%s", name) + fmt.Fprintf(&servers, "%s", name) if r.ServerVersion != "" { - fmt.Fprintf(&b, `%s`, esc(r.ServerVersion)) + fmt.Fprintf(&servers, `%s`, esc(r.ServerVersion)) } - b.WriteString("") + servers.WriteString("") } - b.WriteString("\n\n\n") + var b strings.Builder + fmt.Fprintf(&b, "\nCheck%s\n\n\n", servers.String()) group := "" for _, id := range t.ids { if g, _, _ := strings.Cut(id, "."); g != group { @@ -180,6 +244,18 @@ func HTML(w io.Writer, reports []Report) error { } b.WriteString("\n\n") + var c strings.Builder + fmt.Fprintf(&c, "\nClient%s\n\n\n", servers.String()) + for _, client := range t.clients() { + fmt.Fprintf(&c, `%s`, esc(client)) + for i := range reports { + verdict, score, missed := t.client(i, client) + fmt.Fprintf(&c, `%s`, verdict, titleAttr(strings.Join(missed, "\n")), score) + } + c.WriteString("\n") + } + c.WriteString("\n") + var proofs []string var latest time.Time for _, r := range reports { @@ -197,7 +273,8 @@ func HTML(w io.Writer, reports []Report) error { if !latest.IsZero() { made += " on " + latest.UTC().Format(time.DateOnly) } - _, err := strings.NewReplacer("\n", b.String(), "", esc(made)).WriteString(w, matrixPage) + _, err := strings.NewReplacer("\n", b.String(), "\n", c.String(), + "", esc(made)).WriteString(w, matrixPage) return err } diff --git a/internal/report/matrix.html b/internal/report/matrix.html index 72f5e3f..d91f784 100644 --- a/internal/report/matrix.html +++ b/internal/report/matrix.html @@ -46,12 +46,28 @@ font: 15px/1.5 system-ui, -apple-system, "Segoe UI", sans-serif; } a { color: var(--link); } -header, footer { max-width: 60rem; } +header, footer, .tab > p, dl { max-width: 60rem; } h1 { margin: 0 0 8px; font-size: 1.75rem; } p { margin: 0 0 12px; } dl { display: grid; grid-template-columns: auto 1fr; gap: 4px 12px; margin: 0 0 20px; } dt { font-weight: 600; } dd { margin: 0; } +nav { display: flex; gap: 4px; margin: 0 0 16px; border-bottom: 1px solid var(--line); } +nav a { + margin-bottom: -1px; + padding: 6px 14px; + border-bottom: 2px solid transparent; + color: var(--muted); + font-weight: 600; + text-decoration: none; +} +nav a:hover { color: var(--fg); } +main:not(:has(#clients:target)) a[href="#checks"], +main:has(#clients:target) a[href="#clients"] { color: var(--fg); border-bottom-color: var(--link); } +/* The checks are the first tab. The margin keeps a link to a tab from + scrolling the page. */ +.tab { display: none; scroll-margin-top: 100vh; } +.tab:target, main:not(:has(.tab:target)) #checks { display: block; } .scroll { overflow: auto; max-height: 85vh; border: 1px solid var(--line); border-radius: 6px; } table { border-collapse: collapse; width: 100%; } th, td { padding: 4px 10px; border-bottom: 1px solid var(--line); text-align: left; } @@ -97,6 +113,13 @@

nREPL Compatibility Matrix

proof, which are mostly about what the clients people use (CIDER, Calva, Conjure, vim-fireplace, REPLy and so on) need from a server.

+ +
+ +
pass
The server does what the clients need.
fail
Some client won't work properly with the server, or users lose data.
@@ -105,12 +128,26 @@

nREPL Compatibility Matrix

error
proof couldn't run the check.

Hover over a check to see why it matters, and over a verdict for the details.

-
- + +
+
+
+
+

Each cell says how many of the checks a client depends on the server +passes, out of the ones that apply to it. Those are the client's own checks +(e.g. cider.connect), which send what the client sends, the +checks that link to its code, and the ones every client depends on (e.g. +eval.value).

+

Hover over a number to see the checks the server didn't pass.

+
+ +
+
+
diff --git a/internal/report/report.go b/internal/report/report.go index e24f460..36024dd 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -141,6 +141,7 @@ type jsonResult struct { Severity string `json:"severity"` Why string `json:"why"` Refs []check.Ref `json:"refs"` + Clients []string `json:"clients,omitempty"` Details []string `json:"details,omitempty"` Notes []string `json:"notes,omitempty"` Expected string `json:"expected,omitempty"` @@ -158,7 +159,7 @@ func JSON(w io.Writer, run Run) error { for _, r := range run.Results { out.Results = append(out.Results, jsonResult{ ID: r.ID, Title: r.Title, Verdict: r.Verdict, Severity: r.Severity.String(), - Why: r.Why, Refs: r.Refs, Details: r.Details, Notes: r.Notes, Expected: r.Expected, + Why: r.Why, Refs: r.Refs, Clients: r.Clients, Details: r.Details, Notes: r.Notes, Expected: r.Expected, Millis: r.Duration.Milliseconds(), }) } diff --git a/internal/report/report_test.go b/internal/report/report_test.go index 7454024..9c08e04 100644 --- a/internal/report/report_test.go +++ b/internal/report/report_test.go @@ -4,6 +4,7 @@ import ( "bytes" "os" "path/filepath" + "slices" "strings" "testing" "time" @@ -14,9 +15,11 @@ import ( func sampleRun(server string, verdicts ...check.Verdict) Run { ids := []string{"describe.reply", "eval.value", "eval.ns"} + clients := [][]string{{"Calva", "CIDER"}, {"CIDER"}, {"CIDER", "Conjure"}} r := Run{Proof: "test", Server: server, Address: "localhost:1", Started: time.Unix(0, 0)} for i, v := range verdicts { - r.Results = append(r.Results, check.Result{ID: ids[i], Title: "title " + ids[i], Why: "why " + ids[i], Verdict: v}) + r.Results = append(r.Results, check.Result{ID: ids[i], Title: "title " + ids[i], Why: "why " + ids[i], + Clients: clients[i], Verdict: v}) } return r } @@ -82,6 +85,11 @@ func TestMatrixAsAPage(t *testing.T) { `passfail`, `skip`, `3/31/2`, + // The clients the most checks break come first. + `ClientAlpha`, + `CIDER3/31/2 +Calva1/11/1 +Conjure1/1skip`, "Made with proof test on 2026-11-03.", } { if !strings.Contains(got, want) { @@ -167,3 +175,37 @@ func TestTranscriptShowsWhoHungUp(t *testing.T) { t.Errorf("got:\n%s\nwant:\n%s", buf.String(), want) } } + +// A client fares as badly as the worst of the checks that break it. +func TestClientsGetTheWorstVerdict(t *testing.T) { + reports := []Report{{Run: Run{Server: "Alpha"}, Results: []jsonResult{ + {ID: "a.x", Title: "x", Verdict: check.Errored, Clients: []string{"CIDER"}}, + {ID: "a.y", Title: "y", Verdict: check.Pass, Clients: []string{"CIDER"}}, + {ID: "a.z", Title: "z", Verdict: check.Failed, Clients: []string{"Calva"}}, + }}} + verdict, score, missed := newTable(reports).client(0, "CIDER") + if verdict != check.Errored || score != "1/2" || !slices.Equal(missed, []string{"a.x: x (error)"}) { + t.Errorf("got %s %s %v", verdict, score, missed) + } + reports[0].Results[2].Clients = []string{"CIDER"} + verdict, score, missed = newTable(reports).client(0, "CIDER") + if verdict != check.Failed || score != "1/3" || !slices.Equal(missed, []string{"a.x: x (error)", "a.z: z"}) { + t.Errorf("got %s %s %v", verdict, score, missed) + } +} + +// Reports made before proof named clients don't hide the clients that +// later reports name. +func TestClientsFromAnyReport(t *testing.T) { + reports := []Report{ + {Run: Run{Server: "Old"}, Results: []jsonResult{{ID: "eval.value", Verdict: check.Pass}}}, + {Run: Run{Server: "New"}, Results: []jsonResult{{ID: "eval.value", Verdict: check.Pass, Clients: []string{"CIDER"}}}}, + } + tbl := newTable(reports) + if got := tbl.clients(); !slices.Equal(got, []string{"CIDER"}) { + t.Errorf("got clients %v", got) + } + if verdict, _, _ := tbl.client(0, "CIDER"); verdict != check.Skipped { + t.Errorf("a report without clients gives %s", verdict) + } +}