b1b81df fix(diff): omit a file over the byte budget of earlier files as budget · ermite

M internal/adapters/gitcli/diff.go +1 -0


@@ -145,6 +145,7 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di
145	145		e.file.Omitted = why
146	146		default:
147	147		before := pr.budget
	148	+	pr.budget.StartFile()
148	149		hunks, _, why, err := pr.file(sections, pr.budget.FileLines())
149	150		if err != nil {
150	151		return domain.Diff{}, domain.DiffStats{}, err


@@ -145,6 +145,7 @@ func readDiff(r *bufio.Reader, limits domain.DiffLimits) (domain.Diff, domain.Di
145	e.file.Omitted = why	145		e.file.Omitted = why
146	default:	146		default:
147	before := pr.budget	147		before := pr.budget
		148	+	pr.budget.StartFile()
148	hunks, _, why, err := pr.file(sections, pr.budget.FileLines())	149		hunks, _, why, err := pr.file(sections, pr.budget.FileLines())
149	if err != nil {	150		if err != nil {
150	return domain.Diff{}, domain.DiffStats{}, err	151		return domain.Diff{}, domain.DiffStats{}, err

M internal/domain/budget.go +13 -3


@@ -53,10 +53,12 @@ func (b *CommitObjectBudget) Charge(id ObjectID, size int64) {
53	53		type DiffBudget struct {
54	54		limits DiffLimits
55	55		linesLeft, bytesLeft int
	56	+	// fileBytes is bytesLeft when the current file started (StartFile).
	57	+	fileBytes int
56	58		}
57	59
58	60		func NewDiffBudget(limits DiffLimits) DiffBudget {
59		-	return DiffBudget{limits: limits, linesLeft: limits.MaxTotalLines, bytesLeft: limits.MaxTotalBytes}
	61	+	return DiffBudget{limits: limits, linesLeft: limits.MaxTotalLines, bytesLeft: limits.MaxTotalBytes, fileBytes: limits.MaxTotalBytes}
60	62		}
61	63
62	64		func (b DiffBudget) Window(files int) (kept, from, to int, truncated bool) {

@@ -103,10 +105,16 @@ func (b DiffBudget) FileLines() int {
103	105		return n
104	106		}
105	107
	108	+	// StartFile marks the start of a file's lines, for ChargeLine to tell a file
	109	+	// over the byte limit on its own from one that met the bytes of earlier files.
	110	+	func (b *DiffBudget) StartFile() { b.fileBytes = b.bytesLeft }
	111	+
106	112		// ChargeLine accounts for decoded text bytes and, for additions or deletions,
107	113		// one changed line. Context consumes bytes only. It returns OmitNone when the
108		-	// line fits, OmitBudget when the changed lines ran out and OmitUnavailable when
109		-	// the bytes ran out.
	114	+	// line fits and OmitBudget when the changed lines ran out. When the bytes run
	115	+	// out it returns OmitBudget if files read before this one (since StartFile)
	116	+	// spent some of them, as this file may still fit on its own, and
	117	+	// OmitUnavailable if this file alone is over MaxTotalBytes.
110	118		func (b *DiffBudget) ChargeLine(bytes int, changed bool) FileOmission {
111	119		if changed && b.limits.MaxTotalLines > 0 {
112	120		b.linesLeft--

@@ -115,6 +123,8 @@ func (b *DiffBudget) ChargeLine(bytes int, changed bool) FileOmission {
115	123		switch {
116	124		case b.limits.MaxTotalLines > 0 && b.linesLeft < 0:
117	125		return OmitBudget
	126	+	case b.bytesLeft < 0 && b.fileBytes < b.limits.MaxTotalBytes:
	127	+	return OmitBudget
118	128		case b.bytesLeft < 0:
119	129		return OmitUnavailable
120	130		}

 type DiffBudget struct {
 	limits               DiffLimits
 	linesLeft, bytesLeft int
+	// fileBytes is bytesLeft when the current file started (StartFile).
+	fileBytes int
 }
 func NewDiffBudget(limits DiffLimits) DiffBudget {
-	return DiffBudget{limits: limits, linesLeft: limits.MaxTotalLines, bytesLeft: limits.MaxTotalBytes}
+	return DiffBudget{limits: limits, linesLeft: limits.MaxTotalLines, bytesLeft: limits.MaxTotalBytes, fileBytes: limits.MaxTotalBytes}
 }
 func (b DiffBudget) Window(files int) (kept, from, to int, truncated bool) {
 	return n
 }
+// StartFile marks the start of a file's lines, for ChargeLine to tell a file
+// over the byte limit on its own from one that met the bytes of earlier files.
+func (b *DiffBudget) StartFile() { b.fileBytes = b.bytesLeft }
 // ChargeLine accounts for decoded text bytes and, for additions or deletions,
 // one changed line. Context consumes bytes only. It returns OmitNone when the
-// line fits, OmitBudget when the changed lines ran out and OmitUnavailable when
+// line fits and OmitBudget when the changed lines ran out. When the bytes run
-// the bytes ran out.
+// out it returns OmitBudget if files read before this one (since StartFile)
+// spent some of them, as this file may still fit on its own, and
+// OmitUnavailable if this file alone is over MaxTotalBytes.
 func (b *DiffBudget) ChargeLine(bytes int, changed bool) FileOmission {
 	if changed && b.limits.MaxTotalLines > 0 {
 		b.linesLeft--
 	switch {
 	case b.limits.MaxTotalLines > 0 && b.linesLeft < 0:
 		return OmitBudget
+	case b.bytesLeft < 0 && b.fileBytes < b.limits.MaxTotalBytes:
+		return OmitBudget
 	case b.bytesLeft < 0:
 		return OmitUnavailable
 	}

M internal/domain/budget_test.go +21 -0


@@ -190,6 +190,27 @@ func TestOneFileDiffLimits(t *testing.T) {
190	190		if l.From != 3 \|\| l.To != 4 \|\| l.MaxFileLines != 0 \|\| l.MaxTotalLines != 0 \|\| l.MaxFiles != 0 \|\| l.MaxTotalBytes != DefaultDiffLimits().MaxTotalBytes {
191	191		t.Fatalf("got %+v", l)
192	192		}
	193	+	// A negative index (an unchecked IndexFunc) selects no file, not the whole diff.
	194	+	if _, from, to, _ := NewDiffBudget(OneFileDiffLimits(-1)).Window(10); from < to {
	195	+	t.Fatalf("OneFileDiffLimits(-1) selects files [%d, %d)", from, to)
	196	+	}
	197	+	}
	198	+
	199	+	func TestDiffBudgetBytesOfEarlierFiles(t *testing.T) {
	200	+	b := NewDiffBudget(DiffLimits{MaxTotalBytes: 10})
	201	+	b.StartFile()
	202	+	if b.ChargeLine(4, true) != OmitNone {
	203	+	t.Fatal("the first file fits")
	204	+	}
	205	+	b.StartFile()
	206	+	if got := b.ChargeLine(7, true); got != OmitBudget {
	207	+	t.Fatalf("a file meeting the bytes of earlier files: got %v, want OmitBudget", got)
	208	+	}
	209	+	alone := NewDiffBudget(DiffLimits{MaxTotalBytes: 10})
	210	+	alone.StartFile()
	211	+	if got := alone.ChargeLine(11, true); got != OmitUnavailable {
	212	+	t.Fatalf("a file over the bytes on its own: got %v, want OmitUnavailable", got)
	213	+	}
193	214		}
194	215
195	216		func TestDiffBudgetContext(t *testing.T) {


@@ -190,6 +190,27 @@ func TestOneFileDiffLimits(t *testing.T) {
190	if l.From != 3 \|\| l.To != 4 \|\| l.MaxFileLines != 0 \|\| l.MaxTotalLines != 0 \|\| l.MaxFiles != 0 \|\| l.MaxTotalBytes != DefaultDiffLimits().MaxTotalBytes {	190		if l.From != 3 \|\| l.To != 4 \|\| l.MaxFileLines != 0 \|\| l.MaxTotalLines != 0 \|\| l.MaxFiles != 0 \|\| l.MaxTotalBytes != DefaultDiffLimits().MaxTotalBytes {
191	t.Fatalf("got %+v", l)	191		t.Fatalf("got %+v", l)
192	}	192		}
		193	+	// A negative index (an unchecked IndexFunc) selects no file, not the whole diff.
		194	+	if _, from, to, _ := NewDiffBudget(OneFileDiffLimits(-1)).Window(10); from < to {
		195	+	t.Fatalf("OneFileDiffLimits(-1) selects files [%d, %d)", from, to)
		196	+	}
		197	+	}
		198	+
		199	+	func TestDiffBudgetBytesOfEarlierFiles(t *testing.T) {
		200	+	b := NewDiffBudget(DiffLimits{MaxTotalBytes: 10})
		201	+	b.StartFile()
		202	+	if b.ChargeLine(4, true) != OmitNone {
		203	+	t.Fatal("the first file fits")
		204	+	}
		205	+	b.StartFile()
		206	+	if got := b.ChargeLine(7, true); got != OmitBudget {
		207	+	t.Fatalf("a file meeting the bytes of earlier files: got %v, want OmitBudget", got)
		208	+	}
		209	+	alone := NewDiffBudget(DiffLimits{MaxTotalBytes: 10})
		210	+	alone.StartFile()
		211	+	if got := alone.ChargeLine(11, true); got != OmitUnavailable {
		212	+	t.Fatalf("a file over the bytes on its own: got %v, want OmitUnavailable", got)
		213	+	}
193	}	214		}
194		215
195	func TestDiffBudgetContext(t *testing.T) {	216		func TestDiffBudgetContext(t *testing.T) {

M internal/domain/git.go +6 -1


@@ -592,8 +592,13 @@ type DiffLimits struct {
592	592		SkipGenerated bool
593	593		}
594	594
595		-	// OneFileDiffLimits reads the hunks of file i only, with no line cap and MaxTotalBytes as the sole guard.
	595	+	// OneFileDiffLimits reads the hunks of file i only, with no line cap and
	596	+	// MaxTotalBytes as the sole guard. i must be at least 0: From -1 with To 0
	597	+	// would mean the whole diff, so a negative i reads no file at all.
596	598		func OneFileDiffLimits(i int) DiffLimits {
	599	+	if i < 0 {
	600	+	return DiffLimits{From: 1, To: 1, MaxTotalBytes: DefaultDiffLimits().MaxTotalBytes}
	601	+	}
597	602		return DiffLimits{From: i, To: i + 1, MaxTotalBytes: DefaultDiffLimits().MaxTotalBytes}
598	603		}
599	604

 	SkipGenerated bool
 }
-// OneFileDiffLimits reads the hunks of file i only, with no line cap and MaxTotalBytes as the sole guard.
+// OneFileDiffLimits reads the hunks of file i only, with no line cap and
+// MaxTotalBytes as the sole guard. i must be at least 0: From -1 with To 0
+// would mean the whole diff, so a negative i reads no file at all.
 func OneFileDiffLimits(i int) DiffLimits {
+	if i < 0 {
+		return DiffLimits{From: 1, To: 1, MaxTotalBytes: DefaultDiffLimits().MaxTotalBytes}
+	}
 	return DiffLimits{From: i, To: i + 1, MaxTotalBytes: DefaultDiffLimits().MaxTotalBytes}
 }

M internal/readertest/diff.go +14 -4


@@ -89,8 +89,7 @@ func DiffOmissions(t *testing.T, mk Maker) {
89	89		f.Lines("z-big.txt", "line", 2500)
90	90		f.Commit("change")
91	91
92		-	limits := domain.DiffLimits{MaxTotalBytes: 1 << 20, MaxFileLines: 2000, SkipGenerated: true}
93		-	limits.MaxTotalBytes = 300
	92	+	limits := domain.DiffLimits{MaxTotalBytes: 300, MaxFileLines: 2000, SkipGenerated: true}
94	93		d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, limits)
95	94		require.NoError(t, err)
96	95		require.Len(t, d.Diff.Files, 3)

@@ -107,15 +106,26 @@ func DiffOmissions(t *testing.T, mk Maker) {
107	106		assert.Empty(t, byPath["z-big.txt"].Hunks)
108	107		assert.False(t, d.Diff.Truncated)
109	108
	109	+	// Read, go.sum overflows the bytes that a.txt, read before it, already
	110	+	// spent: it may fit on its own, so it is OmitBudget, loadable alone.
110	111		limits.SkipGenerated = false
111	112		d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, limits)
112	113		require.NoError(t, err)
113		-	for _, file := range d.Diff.Files {
	114	+	sum := -1
	115	+	for i, file := range d.Diff.Files {
114	116		if file.NewPath == "go.sum" {
115		-	assert.Equal(t, domain.OmitUnavailable, file.Omitted, "read, it overflows the bytes")
	117	+	sum = i
	118	+	assert.Equal(t, domain.OmitBudget, file.Omitted, "earlier files spent bytes")
116	119		assert.Empty(t, file.Hunks)
117	120		}
118	121		}
	122	+	require.GreaterOrEqual(t, sum, 0)
	123	+	// Alone, it is still over the bytes: OmitUnavailable, a dead end.
	124	+	alone := domain.DiffLimits{From: sum, To: sum + 1, MaxTotalBytes: 300}
	125	+	d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, alone)
	126	+	require.NoError(t, err)
	127	+	assert.Equal(t, domain.OmitUnavailable, d.Diff.Files[sum].Omitted, "over the bytes on its own")
	128	+	assert.Empty(t, d.Diff.Files[sum].Hunks)
119	129
120	130		// OneFileDiffLimits reads a file over 2,000 changed lines, and only it.
121	131		idx := -1

 	f.Lines("z-big.txt", "line", 2500)
 	f.Commit("change")
-	limits := domain.DiffLimits{MaxTotalBytes: 1 << 20, MaxFileLines: 2000, SkipGenerated: true}
+	limits := domain.DiffLimits{MaxTotalBytes: 300, MaxFileLines: 2000, SkipGenerated: true}
-	limits.MaxTotalBytes = 300
 	d, err := g.GetCommit(ctx, f.Name, domain.Revision{}, limits)
 	require.NoError(t, err)
 	require.Len(t, d.Diff.Files, 3)
 	assert.Empty(t, byPath["z-big.txt"].Hunks)
 	assert.False(t, d.Diff.Truncated)
+	// Read, go.sum overflows the bytes that a.txt, read before it, already
+	// spent: it may fit on its own, so it is OmitBudget, loadable alone.
 	limits.SkipGenerated = false
 	d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, limits)
 	require.NoError(t, err)
-	for _, file := range d.Diff.Files {
+	sum := -1
+	for i, file := range d.Diff.Files {
 		if file.NewPath == "go.sum" {
-			assert.Equal(t, domain.OmitUnavailable, file.Omitted, "read, it overflows the bytes")
+			sum = i
+			assert.Equal(t, domain.OmitBudget, file.Omitted, "earlier files spent bytes")
 			assert.Empty(t, file.Hunks)
 		}
 	}
+	require.GreaterOrEqual(t, sum, 0)
+	// Alone, it is still over the bytes: OmitUnavailable, a dead end.
+	alone := domain.DiffLimits{From: sum, To: sum + 1, MaxTotalBytes: 300}
+	d, err = g.GetCommit(ctx, f.Name, domain.Revision{}, alone)
+	require.NoError(t, err)
+	assert.Equal(t, domain.OmitUnavailable, d.Diff.Files[sum].Omitted, "over the bytes on its own")
+	assert.Empty(t, d.Diff.Files[sum].Hunks)
 	// OneFileDiffLimits reads a file over 2,000 changed lines, and only it.
 	idx := -1