Merge pull request #11359 from BuGlessRB/open/diff-fence-scoped-notify

fix(cli): stop the diff formatter repainting every block when a run lands

docs/CLI.md

@@ -155,9 +155,9 @@ In the full-screen TUI it runs off the render path: the built-in rows are drawn
 first and replaced once its output is ready, so a slow formatter cannot freeze
 the UI.
 
-On any failure, timeout, or empty output the built-in renderer is kept. Like
-`[cli].update_channel` it is user/global only — a project-local `reasonix.toml`
-cannot set it.
+On any failure, timeout, empty output, or a diff larger than 1 MiB, the
+built-in renderer is kept. Like `[cli].update_channel` it is user/global only —
+a project-local `reasonix.toml` cannot set it.
 
 `[agent].embedded_diff_detection = true` marks a shell result whose whole output
 is a unified diff — e.g. `bash` running `git diff` — so the CLI/TUI and desktop

docs/CLI.zh-CN.md

@@ -136,8 +136,9 @@ diff 卡片，以及整段输出就是 diff 的 shell 结果（见下方
 全屏 TUI 中它在渲染路径之外运行：先画内置行，待其输出就绪后再替换，因此慢格式化器不会
 卡住界面。
 
-命令失败、超时或输出为空时保留内置渲染器。与 `[cli].update_channel` 一样，它仅属于
-用户/全局配置，项目内的 `reasonix.toml` 无法设置。
+命令失败、超时、输出为空或 diff 超过 1 MiB 时保留内置渲染器。与
+`[cli].update_channel` 一样，它仅属于用户/全局配置，项目内的 `reasonix.toml`
+无法设置。
 
 `[agent].embedded_diff_detection = true` 会把整段输出为 unified diff 的 shell 结果
 （例如 `bash` 运行 `git diff`）标记出来，让 CLI/TUI 与桌面端把它画成彩色 diff 而不是

internal/frontend/termrender/diffformatter.go

@@ -9,6 +9,7 @@ package termrender
 import (
 	"bytes"
 	"context"
+	"hash/maphash"
 	"os"
 	"os/exec"
 	"strconv"
@@ -49,8 +50,8 @@ func configureDiffFormatter(cfg *config.Config) {
 	}
 	diffFormatMu.Lock()
 	activeDiffFormatter = argv
-	diffFormatCache, diffFormatOrder = map[diffFormatKey]diffFormatEntry{}, nil
-	diffFormatInflight = map[diffFormatKey]bool{}
+	diffFormatCache, diffFormatOrder = map[DiffKey]diffFormatEntry{}, nil
+	diffFormatInflight = map[DiffKey]bool{}
 	diffFormatDisabled = false
 	diffFormatMu.Unlock()
 }
@@ -62,17 +63,18 @@ func SetDiffFormatterForTest(argv []string) (restore func()) {
 	diffFormatMu.Lock()
 	prev := activeDiffFormatter
 	activeDiffFormatter = argv
-	diffFormatCache, diffFormatOrder = map[diffFormatKey]diffFormatEntry{}, nil
-	diffFormatInflight = map[diffFormatKey]bool{}
+	diffFormatCache, diffFormatOrder = map[DiffKey]diffFormatEntry{}, nil
+	diffFormatInflight = map[DiffKey]bool{}
 	diffFormatMu.Unlock()
 	return func() { SetDiffFormatterForTest(prev) }
 }
 
 // SetDiffFormatNotify registers fn as the re-render hook: the formatter calls it
-// from its goroutine once a background run lands, so the frontend can repaint and
-// pick up the result. Pass nil to run the formatter inline instead — a renderer
-// that cannot re-render then still shows the output on the frame it draws.
-func SetDiffFormatNotify(fn func()) {
+// from its goroutine with the key that landed, so the frontend can repaint only
+// the rows that asked for it. Pass nil to run the formatter inline instead — a
+// renderer that cannot re-render then still shows the output on the frame it
+// draws.
+func SetDiffFormatNotify(fn func(DiffKey)) {
 	diffFormatMu.Lock()
 	diffFormatNotify = fn
 	diffFormatMu.Unlock()
@@ -184,11 +186,27 @@ func formatToolDiff(diff string, width int) ([]string, bool) {
 	return rows, true
 }
 
-const diffFormatCacheMax = 32
+const diffFormatCacheMax = 128
 
-type diffFormatKey struct {
-	content string
-	width   int
+// DiffKey identifies a memoized [cli].diff_formatter result: a 128-bit hash of
+// the diff text and the width it was laid out to. Hashing rather than holding
+// the text keeps the memo from keeping a second copy of a diff a block already
+// carries, so a landed background run can still re-render only the rows that
+// asked for its key without the memo paying for the input again.
+type DiffKey struct {
+	Hash  [2]uint64
+	Width int
+}
+
+// diffKeySeeds give the two hash halves; two seeds make a 128-bit key, so two
+// distinct diffs collide with probability below 2^-115 at the memo's size.
+var diffKeySeeds = [2]maphash.Seed{maphash.MakeSeed(), maphash.MakeSeed()}
+
+func diffKeyOf(content string, width int) DiffKey {
+	return DiffKey{
+		Hash:  [2]uint64{maphash.String(diffKeySeeds[0], content), maphash.String(diffKeySeeds[1], content)},
+		Width: width,
+	}
 }
 
 type diffFormatEntry struct {
@@ -198,20 +216,47 @@ type diffFormatEntry struct {
 
 var (
 	diffFormatMu       sync.Mutex
-	diffFormatCache    = map[diffFormatKey]diffFormatEntry{}
-	diffFormatOrder    []diffFormatKey
-	diffFormatInflight = map[diffFormatKey]bool{}
+	diffFormatCache    = map[DiffKey]diffFormatEntry{}
+	diffFormatOrder    []DiffKey
+	diffFormatInflight = map[DiffKey]bool{}
 	// diffFormatDisabled latches once a run hits diffFormatTimeout. A formatter
 	// that hangs once hangs again, and each retry costs a full timeout — worst on
 	// a resize, which changes the width key and so misses the per-(content,width)
 	// cache. Once latched the built-in renderer is used.
 	diffFormatDisabled bool
 	// diffFormatNotify, when set, makes the run asynchronous and is called from
-	// the formatter goroutine once a result lands, so the frontend repaints and
-	// picks it up. Nil runs the formatter inline.
-	diffFormatNotify func()
+	// the formatter goroutine with the landed key once a result arrives, so the
+	// frontend repaints and picks it up. Nil runs the formatter inline.
+	diffFormatNotify func(DiffKey)
+	// renderKeySink, when non-nil, receives every key a render consults. Set only
+	// for the duration of a RenderKeys call, on the one goroutine that renders.
+	renderKeySink func(DiffKey)
 )
 
+// RenderKeys runs render and returns its output with every formatter key it
+// consulted, so the caller can remember which keys its rows depend on. The keys
+// come back whether the memo served them or a background run was started.
+func RenderKeys(render func() string) (string, []DiffKey) {
+	var keys []DiffKey
+	prev := renderKeySink
+	renderKeySink = func(k DiffKey) { keys = append(keys, k) }
+	out := render()
+	renderKeySink = prev
+	return out, keys
+}
+
+// HasDiffFormat reports whether the memo still holds a result for key. A
+// frontend that scopes its repaint by key checks this before dropping a block's
+// rows: re-rendering a block whose key the memo has since evicted would re-run
+// the formatter and evict another live key, a cascade that never settles, so the
+// evicted block keeps the rows it already shows.
+func HasDiffFormat(key DiffKey) bool {
+	diffFormatMu.Lock()
+	defer diffFormatMu.Unlock()
+	_, ok := diffFormatCache[key]
+	return ok
+}
+
 // formatDiffCached returns the formatter's output for (diff, width), memoized
 // per key. With a re-render hook set (SetDiffFormatNotify) the run goes to a
 // background goroutine and this returns ok=false at once, so the caller draws
@@ -221,7 +266,10 @@ func formatDiffCached(diff string, width int) (string, bool) {
 	if len(activeDiffFormatter) == 0 {
 		return "", false
 	}
-	key := diffFormatKey{content: diff, width: width}
+	key := diffKeyOf(diff, width)
+	if renderKeySink != nil {
+		renderKeySink(key)
+	}
 	diffFormatMu.Lock()
 	if diffFormatDisabled {
 		diffFormatMu.Unlock()
@@ -254,7 +302,7 @@ func formatDiffCached(diff string, width int) (string, bool) {
 			notify := diffFormatNotify
 			diffFormatMu.Unlock()
 			if ok && notify != nil {
-				notify()
+				notify(key)
 			}
 		}()
 		return "", false
@@ -272,7 +320,7 @@ func formatDiffCached(diff string, width int) (string, bool) {
 
 // storeDiffFormat records a result under key, evicting the oldest entry when the
 // memo is full. Callers hold diffFormatMu.
-func storeDiffFormat(key diffFormatKey, text string, ok bool) {
+func storeDiffFormat(key DiffKey, text string, ok bool) {
 	if _, seen := diffFormatCache[key]; seen {
 		return
 	}

internal/frontend/termrender/diffformatter_test.go

@@ -319,7 +319,7 @@ func TestFormatDiffCachedAsyncWhenNotifyHookSet(t *testing.T) {
 	setDiffFormatter([]string{"cat"})
 
 	done := make(chan struct{}, 1)
-	SetDiffFormatNotify(func() { done <- struct{}{} })
+	SetDiffFormatNotify(func(DiffKey) { done <- struct{}{} })
 	defer SetDiffFormatNotify(nil)
 
 	if _, ok := formatDiffCached("body", 80); ok {

internal/frontend/tui/diffformat_test.go

@@ -1,6 +1,7 @@
 package tui
 
 import (
+	"fmt"
 	"runtime"
 	"strings"
 	"testing"
@@ -14,15 +15,15 @@ import (
 
 // diffFormatHarness installs a stdin→stdout formatter (cat) and a re-render
 // hook, so a formatter run is asynchronous exactly as the full-screen TUI makes
-// it. The returned channel receives one signal per landed run.
-func diffFormatHarness(t *testing.T) (landed <-chan struct{}) {
+// it. The returned channel receives the key of each landed run.
+func diffFormatHarness(t *testing.T) (landed <-chan termrender.DiffKey) {
 	t.Helper()
 	if runtime.GOOS == "windows" {
 		t.Skip("no portable stdin→stdout filter")
 	}
 	restore := termrender.SetDiffFormatterForTest([]string{"cat"})
-	ch := make(chan struct{}, 8)
-	termrender.SetDiffFormatNotify(func() { ch <- struct{}{} })
+	ch := make(chan termrender.DiffKey, 256)
+	termrender.SetDiffFormatNotify(func(k termrender.DiffKey) { ch <- k })
 	t.Cleanup(func() {
 		termrender.SetDiffFormatNotify(nil)
 		restore()
@@ -30,13 +31,16 @@ func diffFormatHarness(t *testing.T) (landed <-chan struct{}) {
 	return ch
 }
 
-// waitLanded fails unless a formatter run lands within the deadline.
-func waitLanded(t *testing.T, landed <-chan struct{}) {
+// waitLanded fails unless a formatter run lands within the deadline, and returns
+// the key it landed for.
+func waitLanded(t *testing.T, landed <-chan termrender.DiffKey) termrender.DiffKey {
 	t.Helper()
 	select {
-	case <-landed:
+	case k := <-landed:
+		return k
 	case <-time.After(5 * time.Second):
 		t.Fatal("formatter run never landed")
+		return termrender.DiffKey{}
 	}
 }
 
@@ -76,9 +80,9 @@ func TestLandedDiffResultReplacesTheSettledShellDiffRows(t *testing.T) {
 	if !strings.Contains(before, "+1 -1") {
 		t.Fatalf("built-in rows not drawn while the run was in flight:\n%s", before)
 	}
-	waitLanded(t, landed)
+	key := waitLanded(t, landed)
 
-	m.Update(diffFormattedMsg{})
+	m.Update(diffFormattedMsg{key: key})
 
 	after := strings.Join(cachedRows(m), "\n")
 	if strings.Contains(after, "+1 -1") {
@@ -101,9 +105,9 @@ func TestLandedDiffResultReplacesTheSettledWriteCardRows(t *testing.T) {
 	if !strings.Contains(before, "1 - OLD_LINE") {
 		t.Fatalf("built-in card body not drawn while the run was in flight:\n%s", before)
 	}
-	waitLanded(t, landed)
+	key := waitLanded(t, landed)
 
-	m.Update(diffFormattedMsg{})
+	m.Update(diffFormattedMsg{key: key})
 
 	after := strings.Join(cachedRows(m), "\n")
 	if strings.Contains(after, "1 - OLD_LINE") {
@@ -158,8 +162,8 @@ func TestFormattedShellDiffRowsFitTheTranscriptWidth(t *testing.T) {
 	apply(m, eventwire.Event{Kind: "tool_dispatch", Tool: &eventwire.Tool{ID: "t1", Name: "bash", Args: `{"command":"git diff"}`}})
 	apply(m, eventwire.Event{Kind: "tool_result", Tool: &eventwire.Tool{ID: "t1", Name: "bash", Output: diff, OutputDiff: true}})
 	_ = cachedRows(m)
-	waitLanded(t, landed)
-	m.Update(diffFormattedMsg{})
+	key := waitLanded(t, landed)
+	m.Update(diffFormattedMsg{key: key})
 
 	cw := m.contentWidth()
 	for _, l := range rawRows(m) {
@@ -168,3 +172,38 @@ func TestFormattedShellDiffRowsFitTheTranscriptWidth(t *testing.T) {
 		}
 	}
 }
+
+// A transcript holding more diff keys than the memo must still settle: each
+// landed run is fed back the way the program feeds it, and a run may only be
+// answered by repainting the block that asked for its key, so a repaint cannot
+// spawn runs for blocks it never touched. Keyless, every landed run dropped
+// every diff block and a transcript past the memo size never converged.
+func TestManyDiffBlocksSettleRatherThanLoop(t *testing.T) {
+	const n = 160 // more diff keys than diffFormatCacheMax
+	landed := diffFormatHarness(t)
+	m, _ := testModel(t)
+	for i := range n {
+		id := fmt.Sprintf("t%d", i)
+		diff := fmt.Sprintf("diff --git a/f%d.go b/f%d.go\n--- a/f%d.go\n+++ b/f%d.go\n@@ -1 +1 @@\n-old%d\n+new%d\n", i, i, i, i, i, i)
+		apply(m,
+			eventwire.Event{Kind: "tool_dispatch", Tool: &eventwire.Tool{ID: id, Name: "bash", Args: `{"command":"git diff"}`}},
+			eventwire.Event{Kind: "tool_result", Tool: &eventwire.Tool{ID: id, Name: "bash", Output: diff, OutputDiff: true}},
+		)
+	}
+	_ = cachedRows(m) // first render: every distinct block starts one run
+
+	answered := 0
+	for {
+		select {
+		case k := <-landed:
+			m.Update(diffFormattedMsg{key: k})
+			_ = cachedRows(m)
+			answered++
+			if answered > 4*n {
+				t.Fatalf("the landed-run repaint keeps spawning runs (%d answered)", answered)
+			}
+		case <-time.After(time.Second):
+			return // quiesced: no run landed for a second
+		}
+	}
+}

internal/frontend/tui/program.go

@@ -59,7 +59,7 @@ func Run(ctx context.Context, opts Options) error {
 	// path: built-in rows now, formatted ones when the run lands. Inline writes to
 	// the scrollback, where a row cannot be repainted, so it stays inline.
 	if !opts.Inline {
-		termrender.SetDiffFormatNotify(func() { p.Send(diffFormattedMsg{}) })
+		termrender.SetDiffFormatNotify(func(k termrender.DiffKey) { p.Send(diffFormattedMsg{key: k}) })
 		defer termrender.SetDiffFormatNotify(nil)
 	}
 	_, err := p.Run()
@@ -147,9 +147,11 @@ type (
 	}
 	statusTickMsg struct{}
 	// diffFormattedMsg is the diff formatter's background run reporting a
-	// result; the model repaints on it so the formatted rows replace the
-	// built-in ones drawn while the run was in flight.
-	diffFormattedMsg struct{}
+	// result; the model repaints the rows that asked for that key so the
+	// formatted rows replace the built-in ones drawn while the run was in flight.
+	diffFormattedMsg struct {
+		key termrender.DiffKey
+	}
 )
 
 func newModel(ctx context.Context, opts Options) *model {
@@ -295,7 +297,7 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 	case statusTickMsg:
 		return m, tea.Batch(m.fetchStatus(), tickStatus())
 	case diffFormattedMsg:
-		m.invalidateDiffBlocks()
+		m.invalidateDiffKey(msg.key)
 		return m, m.commit()
 	case actionMsg:
 		if msg.err != nil {
@@ -452,7 +454,7 @@ func (m *model) settledChunk(it *Item) (settledPrint, bool) {
 	row := *it
 	p := settledPrint{render: func(w int, hideRail bool) string {
 		return withThought(&row, shown, w, renderSayPart(row.Text[shown:end], shown == 0, w, hideRail))
-	}, diff: drawsThroughDiffFormatter(it)}
+	}}
 	if m.scr != nil && shown == 0 && row.Reasoning != "" {
 		row.Fold = foldShut
 		p.row = &row

internal/frontend/tui/render.go

@@ -214,21 +214,6 @@ func diffRows(out string, width int, f outputFold) []string {
 	return rows
 }
 
-// drawsThroughDiffFormatter reports a row whose settled render may consult the
-// configured [cli].diff_formatter: a tool card with a diff body, or an answer
-// carrying a ```diff / ```patch fence. A block drawn from one must drop its
-// cached rows when a background run lands, or it keeps the built-in rows it
-// showed while the run was in flight.
-func drawsThroughDiffFormatter(it *Item) bool {
-	switch it.Kind {
-	case ItemTool:
-		return it.Tool != nil && (it.Tool.Diff != "" || it.Tool.OutputDiff)
-	case ItemSay:
-		return strings.Contains(it.Text, "```diff") || strings.Contains(it.Text, "```patch")
-	}
-	return false
-}
-
 // renderUsage is what one model request cost, under a quiet rule: history,
 // so it stays in the scrollback in a quieter voice than the footer.
 func renderUsage(u *eventwire.Usage, width int) string {

internal/frontend/tui/screen.go

@@ -48,9 +48,8 @@ type block struct {
 	hideRail bool
 	cells    ansi.Method
 	lines    []string
-	// diff marks a block whose rows draw through the diff formatter, so a
-	// landed background run must drop its cached lines.
-	diff bool
+	// keys are the diff-formatter keys the block's last render consulted.
+	keys []termrender.DiffKey
 	// row is the settled row the block draws, when it draws one: a shell
 	// call's output opens and shuts through it.
 	row *Item
@@ -60,7 +59,9 @@ func (b *block) at(width int, hideRail bool) []string {
 	cells := termrender.Cells()
 	if b.lines == nil || b.width != width || b.hideRail != hideRail || b.cells != cells {
 		b.width, b.hideRail, b.cells = width, hideRail, cells
-		b.lines = wrapLines(b.render(width, hideRail), width)
+		out, keys := termrender.RenderKeys(func() string { return b.render(width, hideRail) })
+		b.keys = keys
+		b.lines = wrapLines(out, width)
 	}
 	return b.lines
 }
@@ -116,8 +117,6 @@ func wrapLines(out string, width int) []string {
 type settledPrint struct {
 	render func(width int, hideRail bool) string
 	row    *Item
-	// diff marks a print whose rows draw through the diff formatter.
-	diff bool
 }
 
 // settledRow keeps a copy of the row to draw from. Full screen, a shell
@@ -126,7 +125,7 @@ func (m *model) settledRow(row Item, shown int) settledPrint {
 	if m.scr != nil && (row.Kind == ItemTool || row.Kind == ItemSay && row.Reasoning != "" && shown == 0) {
 		row.Fold = foldShut
 	}
-	return settledPrint{render: func(w int, hideRail bool) string { return renderItem(&row, w, shown, hideRail) }, row: &row, diff: drawsThroughDiffFormatter(&row)}
+	return settledPrint{render: func(w int, hideRail bool) string { return renderItem(&row, w, shown, hideRail) }, row: &row}
 }
 
 // publish sends what settled where this screen keeps it: blocks of the full
@@ -134,7 +133,7 @@ func (m *model) settledRow(row Item, shown int) settledPrint {
 func (m *model) publish(out []settledPrint) tea.Cmd {
 	if m.scr != nil {
 		for _, p := range out {
-			m.scr.blocks = append(m.scr.blocks, block{render: p.render, row: p.row, diff: p.diff})
+			m.scr.blocks = append(m.scr.blocks, block{render: p.render, row: p.row})
 		}
 		return nil
 	}
@@ -151,21 +150,27 @@ func (m *model) emit(render func(int, bool) string) tea.Cmd {
 	return m.publish([]settledPrint{{render: render}})
 }
 
-// invalidateDiffBlocks drops the cached rows of every block that draws through
-// the diff formatter. A background run reports its result long after the block
-// that asked for it was drawn and its rows cached, so without this the built-in
-// rows it showed while the run was in flight would stay on screen.
-func (m *model) invalidateDiffBlocks() {
-	if m.scr == nil {
+// invalidateDiffKey drops the cached rows of the blocks whose last render
+// consulted key, but only while the memo still holds it. Dropping every diff
+// block would re-render — and so re-spawn — blocks that never asked for it;
+// dropping an evicted key's block would re-run the formatter and evict another
+// live key, a cascade that never settles, so that block keeps its rows.
+func (m *model) invalidateDiffKey(key termrender.DiffKey) {
+	if m.scr == nil || !termrender.HasDiffFormat(key) {
 		return
 	}
 	for i := range m.scr.blocks {
-		if m.scr.blocks[i].diff {
+		if m.scr.blocks[i].hasKey(key) {
 			m.scr.blocks[i].lines = nil
 		}
 	}
 }
 
+// hasKey reports whether the block's last render consulted key.
+func (b *block) hasKey(key termrender.DiffKey) bool {
+	return slices.Contains(b.keys, key)
+}
+
 // fillScreen pushes whatever the terminal shows into its scrollback before a
 // burst of prints. The renderer places a print by scrolling it in above a
 // frame it takes to sit at the bottom of the screen; on a screen not yet full