From 896d69cd682c71ad5265a402410d117d644f25e8 Mon Sep 17 00:00:00 2001 From: Umputun Date: Wed, 19 Aug 2026 06:11:47 -0500 Subject: [PATCH 1/3] fix(worddiff): guard intra-line diff cost by LCS table size, not line length MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 500-byte per-line cap skipped word-diff on any pair where either side was longer, so a markdown paragraph living on one physical line silently got no highlighting while the status bar still showed the mode as on. Byte length is a poor proxy for the cost it was guarding: 500 repeated letters tokenize to a single token, 500 bytes of minified JSON to nearly 300. Gate on the LCS table size instead — minus tokens times plus tokens, budget 4M cells, about 32MB and 17ms for one pair, paid once per file load or word-diff toggle. The byte cap stays only as a cheap pre-filter at 20000 bytes so minified input never reaches the tokenizer. Related to #323 --- app/ui/worddiff/worddiff.go | 21 +++++++---- app/ui/worddiff/worddiff_test.go | 62 ++++++++++++++++++++++++++++++++ docs/ARCHITECTURE.md | 6 ++-- 3 files changed, 81 insertions(+), 8 deletions(-) diff --git a/app/ui/worddiff/worddiff.go b/app/ui/worddiff/worddiff.go index 4eb66115..ec8b3823 100644 --- a/app/ui/worddiff/worddiff.go +++ b/app/ui/worddiff/worddiff.go @@ -36,10 +36,16 @@ 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 this budget is ~32MB and ~17ms for a single +// pair, once per file load or word-diff toggle. byte length is a poor proxy for it: 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 +63,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 +78,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..be0d78a7 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) } @@ -332,6 +335,65 @@ func TestChangedRanges_SkipsVeryLongLines(t *testing.T) { assert.Nil(t, pr2, "asymmetric long minus should skip") } +// prose builds a line of roughly 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) { + minus, plus := prose(maxLineLenForDiff+1), prose(maxLineLenForDiff+1) + minusRanges, plusRanges := d.ComputeIntraRanges(minus, plus) + assert.Nil(t, minusRanges) + assert.Nil(t, plusRanges) + }) + + 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 From 2869330dfce86dd3388f41539851ab92465c6556 Mon Sep 17 00:00:00 2001 From: Umputun Date: Wed, 19 Aug 2026 06:26:23 -0500 Subject: [PATCH 2/3] test(worddiff): pin each cost guard in isolation, correct the budget's godoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The byte pre-filter subtest used two identical prose lines, which the cell budget and the identical-lines branch both reject on their own — it passed with the pre-filter deleted. It now uses a single-token pair that clears every other gate, with a control pair sized to fit the pre-filter, so the assertion can only be answered by the byte gate. The maxDiffCells godoc said the budget was paid once per file load. It bounds one pair; recomputeIntraRanges calls ComputeIntraRanges once per paired remove/add line, so a diff holding many long pairs pays it for each. Related to #323 --- app/ui/worddiff/worddiff.go | 8 +++++--- app/ui/worddiff/worddiff_test.go | 13 ++++++++++++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/app/ui/worddiff/worddiff.go b/app/ui/worddiff/worddiff.go index ec8b3823..bcab2b8b 100644 --- a/app/ui/worddiff/worddiff.go +++ b/app/ui/worddiff/worddiff.go @@ -42,9 +42,11 @@ type Pair struct { 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 this budget is ~32MB and ~17ms for a single -// pair, once per file load or word-diff toggle. byte length is a poor proxy for it: 500 repeated -// letters tokenize to one token, 500 bytes of minified JSON to nearly 300. +// 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. diff --git a/app/ui/worddiff/worddiff_test.go b/app/ui/worddiff/worddiff_test.go index be0d78a7..5b3b72c3 100644 --- a/app/ui/worddiff/worddiff_test.go +++ b/app/ui/worddiff/worddiff_test.go @@ -365,10 +365,21 @@ func TestComputeIntraRanges_CostGuards(t *testing.T) { d := New() t.Run("over byte pre-filter returns nil", func(t *testing.T) { - minus, plus := prose(maxLineLenForDiff+1), prose(maxLineLenForDiff+1) + // 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) { From 8264806e695470495cde15fa08d960f7c4f8858c Mon Sep 17 00:00:00 2001 From: Umputun Date: Wed, 19 Aug 2026 22:31:27 -0500 Subject: [PATCH 3/3] docs(worddiff): correct the test comment on what the byte cap guards TestChangedRanges_SkipsVeryLongLines still carried the pre-change rationale, a near-copy of the godoc this branch replaced, so the package held two contradictory reasons for one constant. The byte cap is a tokenizer pre-filter; maxDiffCells guards the LCS cost. The fixture below the comment is its own counter-example: 20001 repeated letters is a single token. Also correct prose's godoc, which returns exactly nbytes rather than roughly. Related to #323 --- app/ui/worddiff/worddiff_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/ui/worddiff/worddiff_test.go b/app/ui/worddiff/worddiff_test.go index 5b3b72c3..688ee652 100644 --- a/app/ui/worddiff/worddiff_test.go +++ b/app/ui/worddiff/worddiff_test.go @@ -312,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) @@ -335,7 +335,7 @@ func TestChangedRanges_SkipsVeryLongLines(t *testing.T) { assert.Nil(t, pr2, "asymmetric long minus should skip") } -// prose builds a line of roughly nbytes of space-separated words, deterministic across runs. +// 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