diff --git a/app/ui/worddiff/worddiff.go b/app/ui/worddiff/worddiff.go index 4eb66115..bcab2b8b 100644 --- a/app/ui/worddiff/worddiff.go +++ b/app/ui/worddiff/worddiff.go @@ -36,10 +36,18 @@ type Pair struct { AddIdx int } -// maxLineLenForDiff caps intra-line diff to lines up to this many bytes. -// longer lines skip word-level highlighting to avoid O(m*n) LCS memory blowup -// on pathological input (minified files, very long configs). -const maxLineLenForDiff = 500 +// maxLineLenForDiff caps intra-line diff to lines up to this many bytes. it is a cheap +// pre-filter that keeps pathological input (minified bundles, single-line JSON) out of the +// tokenizer; maxDiffCells is the actual cost guard. +const maxLineLenForDiff = 20000 + +// maxDiffCells caps the LCS table at this many cells (minus tokens * plus tokens). +// the table dominates cost at 8 bytes per cell, so the budget is ~32MB and ~17ms per pair. it bounds +// one pair and nothing wider: recomputeIntraRanges calls ComputeIntraRanges once per paired +// remove/add line, so a file whose diff holds many long pairs pays this for each of them. +// byte length is a poor proxy for the cost: 500 repeated letters tokenize to one token, +// 500 bytes of minified JSON to nearly 300. +const maxDiffCells = 4_000_000 // similarityThreshold is the minimum percentage of common tokens for highlighting. // pairs with less than this percentage of common content get no intra-line overlay. @@ -57,8 +65,8 @@ var tokenPattern = regexp.MustCompile(`[\pL\pN_]+|\s+|[^\pL\pN_\s]+`) // ComputeIntraRanges computes changed byte-offset ranges for a pair of minus/plus lines. // returns ranges for the minus line and plus line respectively. -// returns nil ranges if either line is empty, exceeds maxLineLenForDiff, -// or fails the similarity gate (< 30% common non-whitespace tokens). +// returns nil ranges if either line is empty, exceeds maxLineLenForDiff, would need more than +// maxDiffCells LCS cells, or fails the similarity gate (< 30% common non-whitespace tokens). func (d *Differ) ComputeIntraRanges(minusLine, plusLine string) ([]Range, []Range) { if minusLine == "" || plusLine == "" { return nil, nil @@ -72,6 +80,9 @@ func (d *Differ) ComputeIntraRanges(minusLine, plusLine string) ([]Range, []Rang if len(minusToks) == 0 || len(plusToks) == 0 { return nil, nil } + if len(minusToks)*len(plusToks) > maxDiffCells { + return nil, nil + } keepMinus, keepPlus := d.lcsKeptTokens(minusToks, plusToks) minusRanges := d.buildChangedRanges(minusToks, keepMinus) diff --git a/app/ui/worddiff/worddiff_test.go b/app/ui/worddiff/worddiff_test.go index 7bfb5584..688ee652 100644 --- a/app/ui/worddiff/worddiff_test.go +++ b/app/ui/worddiff/worddiff_test.go @@ -22,6 +22,9 @@ func changedRangesHelper(d *Differ, minusLine, plusLine string) ([]Range, []Rang if len(minusToks) == 0 || len(plusToks) == 0 { return nil, nil } + if len(minusToks)*len(plusToks) > maxDiffCells { + return nil, nil + } keepMinus, keepPlus := d.lcsKeptTokens(minusToks, plusToks) return d.buildChangedRanges(minusToks, keepMinus), d.buildChangedRanges(plusToks, keepPlus) } @@ -309,8 +312,8 @@ func TestChangedRanges_MultibytePrecision(t *testing.T) { func TestChangedRanges_SkipsVeryLongLines(t *testing.T) { d := New() - // lines above maxLineLenForDiff must skip intra-line diff to prevent - // LCS memory blowup on pathological input (minified content). + // the byte cap is a pre-filter that keeps long lines out of the tokenizer; + // maxDiffCells is what guards the LCS cost itself. longMinus := strings.Repeat("a", maxLineLenForDiff+1) longPlus := strings.Repeat("b", maxLineLenForDiff+1) @@ -332,6 +335,76 @@ func TestChangedRanges_SkipsVeryLongLines(t *testing.T) { assert.Nil(t, pr2, "asymmetric long minus should skip") } +// prose builds a line of exactly nbytes of space-separated words, deterministic across runs. +func prose(nbytes int) string { + words := []string{"the", "review", "annotation", "paragraph", "diff", "highlight", "maintainer", "line"} + var sb strings.Builder + for i := 0; sb.Len() < nbytes; i++ { + sb.WriteString(words[i%len(words)]) + sb.WriteByte(' ') + } + return sb.String()[:nbytes] +} + +// pins issue #323: a prose pair far past the old 500-byte cap must still be word-diffed. +// the guard is the LCS table size, not the byte length of either line. +func TestComputeIntraRanges_LongProsePairIsDiffed(t *testing.T) { + d := New() + minus := prose(3602) + plus := minus + " and a trailing sentence appended to the paragraph" + + minusRanges, plusRanges := d.ComputeIntraRanges(minus, plus) + assert.Empty(t, minusRanges, "nothing removed from the minus line") + require.NotEmpty(t, plusRanges, "the appended tail must be highlighted") + // whitespace tokens are excluded, so the tail arrives as one range per word + assert.Equal(t, len(minus)+1, plusRanges[0].Start, "highlight starts at the first appended word") + assert.Equal(t, len(plus), plusRanges[len(plusRanges)-1].End, "highlight runs to the end of the line") +} + +func TestComputeIntraRanges_CostGuards(t *testing.T) { + d := New() + + t.Run("over byte pre-filter returns nil", func(t *testing.T) { + // one long run is a single token, so the pair clears the cell budget and the similarity + // gate — the byte pre-filter is the only thing that can reject it + minus := strings.Repeat("a", maxLineLenForDiff+1) + plus := minus + " b" + require.LessOrEqual(t, len(d.tokenizeLineWithOffsets(minus))*len(d.tokenizeLineWithOffsets(plus)), maxDiffCells, + "fixture must clear the cell budget so only the byte pre-filter can reject it") + + minusRanges, plusRanges := d.ComputeIntraRanges(minus, plus) + assert.Nil(t, minusRanges) + assert.Nil(t, plusRanges) + + // the same shape sized to fit the pre-filter is diffed, so the nil above is the byte gate + under := strings.Repeat("a", maxLineLenForDiff-2) + _, underPlus := d.ComputeIntraRanges(under, under+" b") + require.Len(t, underPlus, 1, "pair under the pre-filter must still be diffed") + }) + + t.Run("token product over budget returns nil", func(t *testing.T) { + minus, plus := prose(maxLineLenForDiff), prose(maxLineLenForDiff-5)+" zzzz" + require.LessOrEqual(t, len(minus), maxLineLenForDiff, "must clear the byte pre-filter") + require.LessOrEqual(t, len(plus), maxLineLenForDiff, "must clear the byte pre-filter") + require.Greater(t, len(d.tokenizeLineWithOffsets(minus))*len(d.tokenizeLineWithOffsets(plus)), maxDiffCells, + "fixture must exceed the cell budget") + + minusRanges, plusRanges := d.ComputeIntraRanges(minus, plus) + assert.Nil(t, minusRanges) + assert.Nil(t, plusRanges) + }) + + t.Run("few tokens over the old byte cap are diffed", func(t *testing.T) { + // a single long run tokenizes to one token, so cost is trivial regardless of byte length + minus := strings.Repeat("a", 4000) + plus := strings.Repeat("a", 4000) + " b" + minusRanges, plusRanges := d.ComputeIntraRanges(minus, plus) + assert.Empty(t, minusRanges) + require.Len(t, plusRanges, 1) + assert.Equal(t, "b", plus[plusRanges[0].Start:plusRanges[0].End]) + }) +} + func TestPassesSimilarityGateFromKeep(t *testing.T) { d := New() tests := []struct { diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index c583bd50..b34af507 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -293,8 +293,10 @@ Single stateless type `Differ` grouping all word-diff algorithms: - `InsertHighlightMarkers()` — ANSI-aware highlight insertion, shared by both word-diff and search highlighting -30% similarity gate discards ranges for dissimilar pairs. Ranges are byte offsets on tab-replaced -content, aligning with `prepareLineContent` output. +30% similarity gate discards ranges for dissimilar pairs. Cost is guarded by a budget on the LCS +table size (token count of one line times the other), with a byte pre-filter that keeps minified +input out of the tokenizer — line length alone is not the cost driver. Ranges are byte offsets on +tab-replaced content, aligning with `prepareLineContent` output. ### app/highlight/ — syntax highlighting