fix(daemon): confine attention repaints to bar rows

commit: 791d18bf11c751f98dc8a05b44443cf061efbf80
author: brice · 3mo ago · 2026-07-03 14:35 UTC
committer: brice · 3mo ago · 2026-07-03 15:32 UTC
parent: 3a57f9d
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

4 files · +131 -7

M internal/usecase/daemon/attention.go +2 -2


@@ -32,7 +32,7 @@ func (d Daemon) noteAttention(sess session, tb *tab) {
32	32		tb.attention = true
33	33		sess.mu.Unlock()
34	34
35		-	d.repaintAttachedClients(sess)
	35	+	d.repaintAllAttachedClients()
36	36		d.pokeAttentionTicker()
37	37		}
38	38

@@ -112,7 +112,7 @@ func (d Daemon) repaintAttachedClients(sess session) {
112	112		ac := sess.client
113	113		sess.mu.Unlock()
114	114		if ac != nil {
115		-	d.paint(sess, ac, true)
	115	+	d.paint(sess, ac, false)
116	116		}
117	117		}
118	118


@@ -32,7 +32,7 @@ func (d Daemon) noteAttention(sess session, tb *tab) {
32		tb.attention = true	32		tb.attention = true
33		sess.mu.Unlock()	33		sess.mu.Unlock()
34			34
35	-	d.repaintAttachedClients(sess)	35	+	d.repaintAllAttachedClients()
36		d.pokeAttentionTicker()	36		d.pokeAttentionTicker()
37		}	37		}
38			38

@@ -112,7 +112,7 @@ func (d Daemon) repaintAttachedClients(sess session) {
112		ac := sess.client	112		ac := sess.client
113		sess.mu.Unlock()	113		sess.mu.Unlock()
114		if ac != nil {	114		if ac != nil {
115	-	d.paint(sess, ac, true)	115	+	d.paint(sess, ac, false)
116		}	116		}
117		}	117		}
118			118

M internal/usecase/daemon/attention_test.go +125 -2


@@ -11,6 +11,8 @@ import (
11	11
12	12		"github.com/bnema/vev/internal/domain"
13	13		"github.com/bnema/vev/internal/ports"
	14	+	"github.com/bnema/vev/internal/usecase/keys"
	15	+	"github.com/bnema/vev/pkg/renderer"
14	16		"github.com/bnema/vev/pkg/vt"
15	17		)
16	18

@@ -122,7 +124,14 @@ func TestAckAttentionClearsOnlyPaintedVisibleTab(t *testing.T) {
122	124
123	125		d.paint(sess, ac, true)
124	126		_ = mustOutputData(t, sends)
125		-	_ = mustOutputData(t, sends) // deferred repaint after acknowledging the visible tab
	127	+	// The deferred repaint (from ackAttention) targets all attached clients,
	128	+	// including this one, but the bars it would draw are already exactly what
	129	+	// the full paint above just sent: no second frame should follow.
	130	+	select {
	131	+	case f := <-sends:
	132	+	t.Fatalf("unexpected extra frame after ack: %v", f.Type)
	133	+	case <-time.After(50 * time.Millisecond):
	134	+	}
126	135
127	136		sess.mu.Lock()
128	137		defer sess.mu.Unlock()

@@ -165,7 +174,13 @@ func TestSwitchTabClearsAttentionEndToEnd(t *testing.T) {
165	174		require.True(t, sess.switchTab(1))
166	175		d.paint(sess, ac, true)
167	176		data := mustOutputData(t, sends)
168		-	_ = mustOutputData(t, sends) // deferred repaint after acknowledging the visited tab
	177	+	// As above: the deferred all-clients repaint has nothing new to say about
	178	+	// this client's bars, so no second frame follows.
	179	+	select {
	180	+	case f := <-sends:
	181	+	t.Fatalf("unexpected extra frame after ack: %v", f.Type)
	182	+	case <-time.After(50 * time.Millisecond):
	183	+	}
169	184
170	185		require.NotContains(t, string(data), "")
171	186		sess.mu.Lock()

@@ -195,6 +210,114 @@ func TestCloseRingingTabClearsClientBar(t *testing.T) {
195	210		require.NotContains(t, string(after), "")
196	211		}
197	212
	213	+	// TestAnimationRepaintConfinedToBarRows is the regression test for the pulse
	214	+	// animator: repaintAttachedClients must diff against the warm barCache
	215	+	// (reset=false), so a repaint with nothing new to say sends no frame at all,
	216	+	// and a repaint following only a pulse-frame advance touches the bar rows
	217	+	// without re-emitting screen content.
	218	+	func TestAnimationRepaintConfinedToBarRows(t *testing.T) {
	219	+	d, sess, ac, sends, releases := newManualTabSession(t, 2)
	220	+	defer releases[0]()
	221	+	defer releases[1]()
	222	+	sess.active = 0
	223	+	sess.tabs[0].screen.Write([]byte("MIDSCREENMARKER"))
	224	+	sess.mu.Lock()
	225	+	sess.tabs[1].attention = true
	226	+	sess.tabs[1].attentionAt = time.Unix(1, 0)
	227	+	sess.mu.Unlock()
	228	+
	229	+	d.paint(sess, ac, true)
	230	+	full := mustOutputData(t, sends)
	231	+	require.Contains(t, string(full), "MIDSCREENMARKER")
	232	+
	233	+	// Nothing changed: a repaint against the warm cache must send nothing.
	234	+	d.repaintAttachedClients(sess)
	235	+	select {
	236	+	case f := <-sends:
	237	+	t.Fatalf("unexpected frame on unchanged repaint: %v", f.Type)
	238	+	case <-time.After(50 * time.Millisecond):
	239	+	}
	240	+
	241	+	// Advance the pulse frame (only the bells' rendered style changes) and
	242	+	// repaint: the bytes sent must not repaint screen content.
	243	+	d.advanceAttentionFrame()
	244	+	d.repaintAttachedClients(sess)
	245	+	data := mustOutputData(t, sends)
	246	+	require.NotContains(t, string(data), "MIDSCREENMARKER")
	247	+	}
	248	+
	249	+	// TestComposeClientFrameWithStateAttentionFrameChangeDamagesOnlyBars pins the
	250	+	// same invariant at the composeClientFrameWithState level: with a warm
	251	+	// barCache and no screen damage, changing only bars.attentionFrame must
	252	+	// produce damage confined to row 0 (top bar) and the last row (bottom bar).
	253	+	func TestComposeClientFrameWithStateAttentionFrameChangeDamagesOnlyBars(t *testing.T) {
	254	+	d, sess, _, _, releases := newManualTabSession(t, 2)
	255	+	defer releases[0]()
	256	+	defer releases[1]()
	257	+	sess.active = 0
	258	+	sess.mu.Lock()
	259	+	sess.tabs[1].attention = true
	260	+	sess.tabs[1].attentionAt = time.Unix(1, 0)
	261	+	sess.mu.Unlock()
	262	+
	263	+	tb := sess.tabs[0]
	264	+	var cache barCache
	265	+	bars := d.barStateFor(sess, "")
	266	+	_, damage := composeClientFrameWithState(bars, tb, true, &cache)
	267	+	require.Equal(t, []renderer.Damage{renderer.FullRedraw()}, damage)
	268	+	tb.screen.ClearDamage()
	269	+
	270	+	bars.attentionFrame++
	271	+	_, damage = composeClientFrameWithState(bars, tb, false, &cache)
	272	+	require.NotEmpty(t, damage)
	273	+	lastRow := tb.screen.Frame.Height + 1
	274	+	for _, dmg := range damage {
	275	+	require.Contains(t, []int{0, lastRow}, dmg.Y, "expected damage confined to bar rows, got y=%d", dmg.Y)
	276	+	}
	277	+	}
	278	+
	279	+	// TestCloseRingingTabRefreshesOtherSessionBottomBar covers Fix 2c: closing a
	280	+	// ringing tab in one session must refresh the bottom bar of a client attached
	281	+	// to a DIFFERENT session, since that client's bar shows the ringing session's
	282	+	// bell until it stops.
	283	+	func TestCloseRingingTabRefreshesOtherSessionBottomBar(t *testing.T) {
	284	+	pA1, releaseA1 := newBlockingPTY(t)
	285	+	pA2, releaseA2 := newBlockingPTY(t)
	286	+	pB, releaseB := newBlockingPTY(t)
	287	+	defer releaseA1()
	288	+	defer releaseA2()
	289	+	defer releaseB()
	290	+
	291	+	d, sessA, _, sendsA := newManualSessionWithPTYs(t, pA1, pA2)
	292	+	sessA.name = "ringer"
	293	+	ringing := sessA.tabs[1]
	294	+	sessA.mu.Lock()
	295	+	ringing.attention = true
	296	+	ringing.attentionAt = time.Unix(1, 0)
	297	+	sessA.mu.Unlock()
	298	+	d.setAttentionFrame(1) // pulse frame 0 renders the bell glyph as blank
	299	+
	300	+	trB, sendsB := newCapturingTransport(t)
	301	+	acB := &attachedClient{tr: trB, rend: renderer.New(renderer.Capabilities{}), size: domain.Size{Cols: 80, Rows: 24}}
	302	+	sctxB, cancelB := context.WithCancel(d.serveCtx)
	303	+	t.Cleanup(cancelB)
	304	+	tbB := &tab{pty: pB, screen: vt.NewScreen(80, 23), dirty: make(chan struct{}, 1), size: domain.Size{Cols: 80, Rows: 23}, ctx: sctxB, cancel: cancelB}
	305	+	sessB := &session{id: "sessB", name: "other", ctx: sctxB, cancel: cancelB, tabs: []*tab{tbB}, client: acB}
	306	+	acB.setSession(sessB)
	307	+	acB.keys = keys.NewRouter(d.clock, daemonKeyHandler{d: d, ac: acB})
	308	+	d.sessions[sessB.id] = sessB
	309	+
	310	+	d.paint(sessB, acB, true)
	311	+	before := mustOutputData(t, sendsB)
	312	+	require.Contains(t, string(before), string(attentionGlyph))
	313	+
	314	+	d.closeTab(sessA, ringing, true)
	315	+	_ = mustOutputData(t, sendsA)
	316	+
	317	+	after := mustOutputData(t, sendsB)
	318	+	require.NotContains(t, string(after), string(attentionGlyph))
	319	+	}
	320	+
198	321		func TestAltAJumpAttentionSelectsOldestLocalTab(t *testing.T) {
199	322		d, sess, ac, sends, releases := newManualTabSession(t, 3)
200	323		defer releases[0]()


@@ -11,6 +11,8 @@ import (
11			11
12		"github.com/bnema/vev/internal/domain"	12		"github.com/bnema/vev/internal/domain"
13		"github.com/bnema/vev/internal/ports"	13		"github.com/bnema/vev/internal/ports"
			14	+	"github.com/bnema/vev/internal/usecase/keys"
			15	+	"github.com/bnema/vev/pkg/renderer"
14		"github.com/bnema/vev/pkg/vt"	16		"github.com/bnema/vev/pkg/vt"
15		)	17		)
16			18

@@ -122,7 +124,14 @@ func TestAckAttentionClearsOnlyPaintedVisibleTab(t *testing.T) {
122			124
123		d.paint(sess, ac, true)	125		d.paint(sess, ac, true)
124		_ = mustOutputData(t, sends)	126		_ = mustOutputData(t, sends)
125	-	_ = mustOutputData(t, sends) // deferred repaint after acknowledging the visible tab	127	+	// The deferred repaint (from ackAttention) targets all attached clients,
			128	+	// including this one, but the bars it would draw are already exactly what
			129	+	// the full paint above just sent: no second frame should follow.
			130	+	select {
			131	+	case f := <-sends:
			132	+	t.Fatalf("unexpected extra frame after ack: %v", f.Type)
			133	+	case <-time.After(50 * time.Millisecond):
			134	+	}
126			135
127		sess.mu.Lock()	136		sess.mu.Lock()
128		defer sess.mu.Unlock()	137		defer sess.mu.Unlock()

@@ -165,7 +174,13 @@ func TestSwitchTabClearsAttentionEndToEnd(t *testing.T) {
165		require.True(t, sess.switchTab(1))	174		require.True(t, sess.switchTab(1))
166		d.paint(sess, ac, true)	175		d.paint(sess, ac, true)
167		data := mustOutputData(t, sends)	176		data := mustOutputData(t, sends)
168	-	_ = mustOutputData(t, sends) // deferred repaint after acknowledging the visited tab	177	+	// As above: the deferred all-clients repaint has nothing new to say about
			178	+	// this client's bars, so no second frame follows.
			179	+	select {
			180	+	case f := <-sends:
			181	+	t.Fatalf("unexpected extra frame after ack: %v", f.Type)
			182	+	case <-time.After(50 * time.Millisecond):
			183	+	}
169			184
170		require.NotContains(t, string(data), "")	185		require.NotContains(t, string(data), "")
171		sess.mu.Lock()	186		sess.mu.Lock()

@@ -195,6 +210,114 @@ func TestCloseRingingTabClearsClientBar(t *testing.T) {
195		require.NotContains(t, string(after), "")	210		require.NotContains(t, string(after), "")
196		}	211		}
197			212
			213	+	// TestAnimationRepaintConfinedToBarRows is the regression test for the pulse
			214	+	// animator: repaintAttachedClients must diff against the warm barCache
			215	+	// (reset=false), so a repaint with nothing new to say sends no frame at all,
			216	+	// and a repaint following only a pulse-frame advance touches the bar rows
			217	+	// without re-emitting screen content.
			218	+	func TestAnimationRepaintConfinedToBarRows(t *testing.T) {
			219	+	d, sess, ac, sends, releases := newManualTabSession(t, 2)
			220	+	defer releases[0]()
			221	+	defer releases[1]()
			222	+	sess.active = 0
			223	+	sess.tabs[0].screen.Write([]byte("MIDSCREENMARKER"))
			224	+	sess.mu.Lock()
			225	+	sess.tabs[1].attention = true
			226	+	sess.tabs[1].attentionAt = time.Unix(1, 0)
			227	+	sess.mu.Unlock()
			228	+
			229	+	d.paint(sess, ac, true)
			230	+	full := mustOutputData(t, sends)
			231	+	require.Contains(t, string(full), "MIDSCREENMARKER")
			232	+
			233	+	// Nothing changed: a repaint against the warm cache must send nothing.
			234	+	d.repaintAttachedClients(sess)
			235	+	select {
			236	+	case f := <-sends:
			237	+	t.Fatalf("unexpected frame on unchanged repaint: %v", f.Type)
			238	+	case <-time.After(50 * time.Millisecond):
			239	+	}
			240	+
			241	+	// Advance the pulse frame (only the bells' rendered style changes) and
			242	+	// repaint: the bytes sent must not repaint screen content.
			243	+	d.advanceAttentionFrame()
			244	+	d.repaintAttachedClients(sess)
			245	+	data := mustOutputData(t, sends)
			246	+	require.NotContains(t, string(data), "MIDSCREENMARKER")
			247	+	}
			248	+
			249	+	// TestComposeClientFrameWithStateAttentionFrameChangeDamagesOnlyBars pins the
			250	+	// same invariant at the composeClientFrameWithState level: with a warm
			251	+	// barCache and no screen damage, changing only bars.attentionFrame must
			252	+	// produce damage confined to row 0 (top bar) and the last row (bottom bar).
			253	+	func TestComposeClientFrameWithStateAttentionFrameChangeDamagesOnlyBars(t *testing.T) {
			254	+	d, sess, _, _, releases := newManualTabSession(t, 2)
			255	+	defer releases[0]()
			256	+	defer releases[1]()
			257	+	sess.active = 0
			258	+	sess.mu.Lock()
			259	+	sess.tabs[1].attention = true
			260	+	sess.tabs[1].attentionAt = time.Unix(1, 0)
			261	+	sess.mu.Unlock()
			262	+
			263	+	tb := sess.tabs[0]
			264	+	var cache barCache
			265	+	bars := d.barStateFor(sess, "")
			266	+	_, damage := composeClientFrameWithState(bars, tb, true, &cache)
			267	+	require.Equal(t, []renderer.Damage{renderer.FullRedraw()}, damage)
			268	+	tb.screen.ClearDamage()
			269	+
			270	+	bars.attentionFrame++
			271	+	_, damage = composeClientFrameWithState(bars, tb, false, &cache)
			272	+	require.NotEmpty(t, damage)
			273	+	lastRow := tb.screen.Frame.Height + 1
			274	+	for _, dmg := range damage {
			275	+	require.Contains(t, []int{0, lastRow}, dmg.Y, "expected damage confined to bar rows, got y=%d", dmg.Y)
			276	+	}
			277	+	}
			278	+
			279	+	// TestCloseRingingTabRefreshesOtherSessionBottomBar covers Fix 2c: closing a
			280	+	// ringing tab in one session must refresh the bottom bar of a client attached
			281	+	// to a DIFFERENT session, since that client's bar shows the ringing session's
			282	+	// bell until it stops.
			283	+	func TestCloseRingingTabRefreshesOtherSessionBottomBar(t *testing.T) {
			284	+	pA1, releaseA1 := newBlockingPTY(t)
			285	+	pA2, releaseA2 := newBlockingPTY(t)
			286	+	pB, releaseB := newBlockingPTY(t)
			287	+	defer releaseA1()
			288	+	defer releaseA2()
			289	+	defer releaseB()
			290	+
			291	+	d, sessA, _, sendsA := newManualSessionWithPTYs(t, pA1, pA2)
			292	+	sessA.name = "ringer"
			293	+	ringing := sessA.tabs[1]
			294	+	sessA.mu.Lock()
			295	+	ringing.attention = true
			296	+	ringing.attentionAt = time.Unix(1, 0)
			297	+	sessA.mu.Unlock()
			298	+	d.setAttentionFrame(1) // pulse frame 0 renders the bell glyph as blank
			299	+
			300	+	trB, sendsB := newCapturingTransport(t)
			301	+	acB := &attachedClient{tr: trB, rend: renderer.New(renderer.Capabilities{}), size: domain.Size{Cols: 80, Rows: 24}}
			302	+	sctxB, cancelB := context.WithCancel(d.serveCtx)
			303	+	t.Cleanup(cancelB)
			304	+	tbB := &tab{pty: pB, screen: vt.NewScreen(80, 23), dirty: make(chan struct{}, 1), size: domain.Size{Cols: 80, Rows: 23}, ctx: sctxB, cancel: cancelB}
			305	+	sessB := &session{id: "sessB", name: "other", ctx: sctxB, cancel: cancelB, tabs: []*tab{tbB}, client: acB}
			306	+	acB.setSession(sessB)
			307	+	acB.keys = keys.NewRouter(d.clock, daemonKeyHandler{d: d, ac: acB})
			308	+	d.sessions[sessB.id] = sessB
			309	+
			310	+	d.paint(sessB, acB, true)
			311	+	before := mustOutputData(t, sendsB)
			312	+	require.Contains(t, string(before), string(attentionGlyph))
			313	+
			314	+	d.closeTab(sessA, ringing, true)
			315	+	_ = mustOutputData(t, sendsA)
			316	+
			317	+	after := mustOutputData(t, sendsB)
			318	+	require.NotContains(t, string(after), string(attentionGlyph))
			319	+	}
			320	+
198		func TestAltAJumpAttentionSelectsOldestLocalTab(t *testing.T) {	321		func TestAltAJumpAttentionSelectsOldestLocalTab(t *testing.T) {
199		d, sess, ac, sends, releases := newManualTabSession(t, 3)	322		d, sess, ac, sends, releases := newManualTabSession(t, 3)
200		defer releases[0]()	323		defer releases[0]()

M internal/usecase/daemon/render.go +1 -1


@@ -282,7 +282,7 @@ func (d Daemon) paint(sess session, ac *attachedClient, reset bool) {
282	282		ac.promptMu.Unlock()
283	283		}
284	284		if sess.ackAttention(tb) {
285		-	defer d.repaintAttachedClients(sess)
	285	+	defer d.repaintAllAttachedClients()
286	286		}
287	287		bars := d.barStateFor(sess, copyFeedback)
288	288		bars.theme = ac.getTheme()


@@ -282,7 +282,7 @@ func (d Daemon) paint(sess session, ac *attachedClient, reset bool) {
282		ac.promptMu.Unlock()	282		ac.promptMu.Unlock()
283		}	283		}
284		if sess.ackAttention(tb) {	284		if sess.ackAttention(tb) {
285	-	defer d.repaintAttachedClients(sess)	285	+	defer d.repaintAllAttachedClients()
286		}	286		}
287		bars := d.barStateFor(sess, copyFeedback)	287		bars := d.barStateFor(sess, copyFeedback)
288		bars.theme = ac.getTheme()	288		bars.theme = ac.getTheme()

M internal/usecase/daemon/session.go +3 -2


@@ -263,8 +263,9 @@ func (d Daemon) closeTab(sess session, tb *tab, repaint bool) {
263	263		_ = tb.pty.Close()
264	264		if repaint && ac != nil {
265	265		d.paint(sess, ac, true)
266		-	} else if ringing {
267		-	d.repaintAttachedClients(sess)
	266	+	}
	267	+	if ringing {
	268	+	d.repaintAllAttachedClients()
268	269		}
269	270		}
270	271

 	_ = tb.pty.Close()
 	if repaint && ac != nil {
 		d.paint(sess, ac, true)
-	} else if ringing {
+	}
-		d.repaintAttachedClients(sess)
+	if ringing {
+		d.repaintAllAttachedClients()
 	}
 }