fix(git): stop unbounded diff content growth for large files (#787)

* fix(git): stop unbounded diff content growth for large files

parseDiffContent's per-file truncation counter was declared but never
incremented, so the byte cap on diff content never triggered resulting
in performance degradation for large files.

* Fix remaining bytes

---------

Co-authored-by: Thomas Miceli <tho.miceli@gmail.com>

internal/git/commands_test.go

@@ -1,6 +1,7 @@
 package git
 
 import (
+	"bufio"
 	"github.com/stretchr/testify/require"
 	"github.com/thomiceli/opengist/internal/config"
 	"os"
@@ -189,7 +190,6 @@ func TestFork(t *testing.T) {
 	require.NoError(t, err, "Could not get files of repository")
 
 	require.Equal(t, files1, files2, "Files are not the same")
-
 }
 
 func TestTruncate(t *testing.T) {
@@ -229,6 +229,101 @@ func TestTruncate(t *testing.T) {
 	require.Equal(t, 2, len(content), "Content size is not correct")
 }
 
+func TestLogDiffTruncation(t *testing.T) {
+	SetupTest(t)
+	defer TeardownTest(t)
+
+	CommitToBare(t, "thomas", "gist1", map[string]string{
+		"my_file.txt": "A",
+	})
+
+	// Write enough lines to guarantee the diff content exceeds maxBytes
+	// (diffSize).
+	var builder strings.Builder
+	lineCount := diffSize/len("A\n") + 100 // comfortably past the threshold
+	for range lineCount {
+		builder.WriteString("A\n")
+	}
+	fullContent := builder.String()
+
+	CommitToBare(t, "thomas", "gist1", map[string]string{
+		"my_file.txt": fullContent,
+	})
+
+	// 11 is arbitrary but comfortably above the 2 commits we expect back;
+	// it just ensures GetLog isn't itself limiting the result set.
+	commits, err := GetLog("thomas", "gist1", "HEAD", 0, 11)
+	require.NoError(t, err, "Could not get log")
+	require.Len(t, commits, 2, "Commits count are not correct")
+
+	// Large-file commit: content must be truncated and bounded near
+	// maxBytes, not left to grow with the full file.
+	largeFile := commits[0].Files[0]
+	require.Len(t, commits[0].Files, 1, "Files count are not correct")
+	require.True(t, largeFile.Truncated, "Diff content should be truncated for a large file")
+	require.Less(t, len(largeFile.Content), len(fullContent),
+		"Truncated content must be smaller than the original — content should not grow indefinitely")
+	require.LessOrEqual(t, len(largeFile.Content), diffSize+len("A\n"),
+		"Truncated content should be bounded at approximately maxBytes, not just under some loose multiple of it")
+
+	// Small-file commit: sanity check that truncation doesn't kick in
+	// when it shouldn't.
+	smallFile := commits[1].Files[0]
+	require.False(t, smallFile.Truncated, "Small diff content should not be truncated")
+	require.Equal(t, "@@ -0,0 +1 @@\n+A\n\\ No newline at end of file\n",
+		smallFile.Content, "Small file content should be preserved as-is")
+}
+
+// TestParseDiffContentBudget drives parseDiffContent directly, the way parseLog
+// does, with a reader sized exactly maxBytes. Going through GetLog cannot cover
+// this: a line longer than the buffer comes back from ReadLine as a fragment and
+// is drained without ever reaching the per-line clamp, so the clamp is only
+// reachable by a long line that still fits in the buffer.
+func TestParseDiffContentBudget(t *testing.T) {
+	const maxBytes = 64
+
+	// filler emits n bytes as 2-byte lines, leaving currFileLineCount just shy
+	// of the cap so the following line lands on the budget boundary.
+	filler := func(b *strings.Builder, n int) {
+		for range n / 2 {
+			b.WriteString("A\n")
+		}
+	}
+
+	tests := []struct {
+		name     string
+		longLine int
+	}{
+		// Below the buffer size, so ReadLine returns these whole rather than as
+		// fragments. Lengths straddle the point where the old code set
+		// Truncated, which is why the flag was silently missed just under it.
+		{"long line just under maxBytes", maxBytes - 2},
+		{"long line at maxBytes-1", maxBytes - 1},
+		{"long line exactly maxBytes", maxBytes},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var b strings.Builder
+			filler(&b, maxBytes-2)
+			b.WriteString(strings.Repeat("B", tt.longLine) + "\n")
+			b.WriteString("diff --git a/x b/x\n") // ends the file's diff content
+
+			currentFile := &File{}
+			input := bufio.NewReaderSize(strings.NewReader(b.String()), maxBytes)
+			_, _, err := parseDiffContent(currentFile, maxBytes, input)
+			require.NoError(t, err, "Could not parse diff content")
+
+			// The trailing newline of the final line is appended after the
+			// budget check, so one byte of overshoot is expected.
+			require.LessOrEqual(t, len(currentFile.Content), maxBytes+1,
+				"Content must stay within the byte budget, not grow to a multiple of it")
+			require.True(t, currentFile.Truncated,
+				"Truncated must be set whenever content is clipped, otherwise the UI renders a clipped diff as complete")
+		})
+	}
+}
+
 func TestGitInitBranchNames(t *testing.T) {
 	SetupTest(t)
 	defer TeardownTest(t)

internal/git/output_parser.go

@@ -340,11 +340,12 @@ func parseDiffContent(currentFile *File, maxBytes int, input *bufio.Reader) (lin
 			}
 		}
 
-		if len(line) > maxBytes {
+		if remaining := maxBytes - currFileLineCount; len(line) > remaining {
 			currentFile.Truncated = true
-			line = line[:maxBytes]
+			line = line[:remaining]
 		}
 		currentFile.Content += line + "\n"
+		currFileLineCount += len(line) + 1
 	}
 }