Repository: reviewdog/reviewdog (language: go)

Issue:
github-pr-check fails to complete if there are too many findings
Updating the check run fails with the following error message:

```
UpdateCheckRun failed: {"message":"Invalid request. Only 65535 characters are allowed; 66618 were supplied.","documentation_url":"https://docs.github.com/rest/reference/checks#update-a-check-run"}
```

At some point there was an attempt to fix this in fe3996e1bd87c07ce52a705453e517c9e1153531, but it seems like the change was not sufficient.

Currently, the summary is limited to 150 findings, but if the text for the individual findings is very long, this can still be too much.
https://github.com/reviewdog/reviewdog/blob/abcd1e0f45b73cd4d5c624b5e64870556d8631b0/doghouse/server/doghouse.go#L20-L26
https://github.com/reviewdog/reviewdog/blob/abcd1e0f45b73cd4d5c624b5e64870556d8631b0/doghouse/server/doghouse.go#L216-L231
Maybe a better approach would be to stop appending after the byte threshold is reached:

```golang
func (ch *Checker) summaryFindings(name string, checks []*filter.FilteredDiagnostic) []string {
	var lines []string
	lines = append(lines, "<details>")
	lines = append(lines, fmt.Sprintf("<summary>%s (%d)</summary>", name, len(checks)))
	lines = append(lines, "")
	for _, c := range checks {
		nextLine := githubutils.LinkedMarkdownDiagnostic(ch.req.Owner, ch.req.Repo, ch.req.SHA, c.Diagnostic)
		if len(lines) + len(nextLine) >= 65535 {
			lines = append(lines, "... (Too many findings. Dropped some findings)")
			break
		}
		lines = append(lines, nextLine)
	}
	lines = append(lines, "</details>")
	return lines
}
``` 

I think it's also worth considering, if the `text` field of the output wouldn't be a better place for all the findings. In the example from the GitHub docs[^1], the summary is much shorter.

```json
  "output": {
    "title": "Mighty Readme report",
    "summary": "There are 0 failures, 2 warnings, and 1 notice.",
    "text": "You may have some misspelled words on lines 2 and 4. You also may want to add a section in your README about how to install your app.",
    "annotations_count": 2,
    "annotations_url": "https://api.github.com/repos/github/hello-world/check-runs/4/annotations"
  },
```

[^1]: https://docs.github.com/en/rest/checks/runs#create-a-check-run


Discussion / hints:
I'd be willing to open a PR with the proposed fix if there are no objections from a maintainer.

Fix patch:
diff --git a/doghouse/server/doghouse.go b/doghouse/server/doghouse.go
index c85479a8f7..aa43a20fe6 100644
--- a/doghouse/server/doghouse.go
+++ b/doghouse/server/doghouse.go
@@ -23,7 +23,7 @@ import (
 //  https://api.github.com/repos/easymotion/vim-easymotion/check-runs: 422
 //  Invalid request.
 //  Only 65535 characters are allowed; 250684 were supplied. []
-const maxFilteredFinding = 150
+const maxAllowedSize = 65535
 
 // > The Checks API limits the number of annotations to a maximum of 50 per API
 // > request.
@@ -196,8 +196,9 @@ func (ch *Checker) reqAnnotationLevel() string {
 
 func (ch *Checker) summary(checks []*filter.FilteredDiagnostic) string {
 	var lines []string
+	var usedBytes int
 	lines = append(lines, "reported by [reviewdog](https://github.com/reviewdog/reviewdog) :dog:")
-
+	usedBytes += len(lines[0]) + 1
 	var findings []*filter.FilteredDiagnostic
 	var filteredFindings []*filter.FilteredDiagnostic
 	for _, c := range checks {
@@ -207,27 +208,39 @@ func (ch *Checker) summary(checks []*filter.FilteredDiagnostic) string {
 			filteredFindings = append(filteredFindings, c)
 		}
 	}
-	lines = append(lines, ch.summaryFindings("Findings", findings)...)
-	lines = append(lines, ch.summaryFindings("Filtered Findings", filteredFindings)...)
 
+	findingMsgs, usedBytes := ch.summaryFindings("Findings", usedBytes, findings)
+	lines = append(lines, findingMsgs...)
+	filteredFindingsMsgs, _ := ch.summaryFindings("Filtered Findings", usedBytes, filteredFindings)
+	lines = append(lines, filteredFindingsMsgs...)
 	return strings.Join(lines, "\n")
 }
 
-func (ch *Checker) summaryFindings(name string, checks []*filter.FilteredDiagnostic) []string {
+func (ch *Checker) summaryFindings(name string, usedBytes int, checks []*filter.FilteredDiagnostic) ([]string, int) {
 	var lines []string
-	lines = append(lines, "<details>")
-	lines = append(lines, fmt.Sprintf("<summary>%s (%d)</summary>", name, len(checks)))
-	lines = append(lines, "")
-	for i, c := range checks {
-		if i >= maxFilteredFinding {
-			lines = append(lines, "... (Too many findings. Dropped some findings)")
+	lines = append(lines, fmt.Sprintf("<details>\n<summary>%s (%d)</summary>\n", name, len(checks)))
+	if len(lines[0])+1+usedBytes > maxAllowedSize {
+		// bail out if we're already over the limit
+		return nil, usedBytes
+	}
+	usedBytes += len(lines[0]) + 1
+	for _, c := range checks {
+		nextLine := githubutils.LinkedMarkdownDiagnostic(ch.req.Owner, ch.req.Repo, ch.req.SHA, c.Diagnostic)
+		// existing lines + newline + closing details tag must be smaller than the max allowed size
+		if usedBytes+len(nextLine)+1+10 >= maxAllowedSize {
+			cutoffMsg := "... (Too many findings. Dropped some findings)"
+			if usedBytes+len(cutoffMsg)+1+10 <= maxAllowedSize {
+				lines = append(lines, cutoffMsg)
+				usedBytes += len(cutoffMsg) + 1
+			}
 			break
 		}
-		lines = append(lines, githubutils.LinkedMarkdownDiagnostic(
-			ch.req.Owner, ch.req.Repo, ch.req.SHA, c.Diagnostic))
+		lines = append(lines, nextLine)
+		usedBytes += len(nextLine) + 1
 	}
 	lines = append(lines, "</details>")
-	return lines
+	usedBytes += 10 + 1
+	return lines, usedBytes
 }
 
 func (ch *Checker) toCheckRunAnnotation(c *filter.FilteredDiagnostic) *github.CheckRunAnnotation {


Test patch:
diff --git a/doghouse/server/doghouse_test.go b/doghouse/server/doghouse_test.go
index 279808ed40..859670efa6 100644
--- a/doghouse/server/doghouse_test.go
+++ b/doghouse/server/doghouse_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"net/http"
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -663,3 +664,23 @@ func TestCheck_fail_check_with_403(t *testing.T) {
 		t.Error("resp.CheckedResults should not be nil")
 	}
 }
+func TestCheck_too_many_findings_cut_off_correctly(t *testing.T) {
+	checker := &Checker{req: &doghouse.CheckRequest{}}
+
+	var diagnostics []*filter.FilteredDiagnostic
+	for i := 0; i < 1000; i++ {
+		diagnostics = append(diagnostics, &filter.FilteredDiagnostic{
+			ShouldReport: true,
+			Diagnostic: &rdf.Diagnostic{
+				Message: "this is a pretty long test message that will lead to overshooting the maximum allowed size",
+			},
+		})
+	}
+	summaryText := checker.summary(diagnostics)
+	if len(summaryText) > maxAllowedSize {
+		t.Errorf("summary text is %d bytes long, but the maximum allowed size is %d", len(summaryText), maxAllowedSize)
+	}
+	if !strings.Contains(summaryText, "... (Too many findings. Dropped some findings)\n</details>") {
+		t.Error("summary text was not cut off correctly")
+	}
+}