aboutsummaryrefslogtreecommitdiffstats
path: root/pi-ext/NOTES.md
blob: db62e7a3bc49b9a5156122fa7b800a68357a7126 (plain) (blame)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
# Notes / TODO

## ✅ FIXED: tool-box flicker #4 (off-screen top border + external render ticks)

**Symptom:** an in-flight tool box taller than the viewport, scrolled so its TOP
is clipped off-screen, flickers violently (full-screen redraw) the entire time
it is active; stops the instant it settles.

**Cause:** two things compound. (1) `obnoxious-working.ts` (and pi's own default
spinner) animate via `ui.requestRender()` at ~10–16fps, so the whole UI
re-renders even when the tool is silent. (2) The active tool-box render did not
cache (`cw = -1`) and recomputed a wall-clock `pulsePaint()` every render, so the
**top border** (owned by the *call* slot) changed color on every tick. tui.js
only diff-renders what's on screen: `if (firstChanged < prevViewportTop)
fullRender(true)`. A recolored top border sitting *above* the viewport is
`firstChanged < viewportTop` → full clear+repaint, every spinner frame.

(The *result* slot only recolors its **bottom** border, which stays in-viewport,
so it was never the culprit. Streaming appends new lines at the bottom too, so it
never mutates off-screen lines.)

**Fix:** while active **and** the result slot has started streaming (box can be
tall), FREEZE the call slot's pulse color, caching the built lines per
`(width, inner)`. The command/header is static, so the top border becomes
byte-identical between render ticks → `firstChanged` never lands above the
viewport → no full redraw. The pre-output phase (box still short + fully visible)
keeps the live pulse and the ticking runtime. Net: top border no longer pulses
while output streams (usually off-screen anyway); the result slot's bottom border
still pulses in-viewport.

## ✅ FIXED: tool-box flicker × 3 (streaming rails, startup storm, streaming timer)

**Final invariant: the pulse NEVER forces a repaint.** The pulse color is a pure
function of wall-clock time (`Date.now() % CYCLE_MS`) and there is NO animation
timer at all. It advances only on renders that were going to happen anyway —
chiefly the natural re-renders a streaming tool already triggers ("the stream
hydrates the pulse", irregular cadence). A silent long-running tool just holds a
static active color until it streams output or finishes. This killed the third
bug: a tall (taller-than-screen) bash box flickered because the old 10fps timer
forced a full `updateDisplay()` repaint on top of the stream's own re-renders.

Borders: only the top/bottom pulse. Side rails are a STATIC color (never carry
the pulse) so they can't recolor body lines per frame: gray dotted `┊` while
in-flight, solidifying to a status-colored `│` once the tool finalizes.

### Earlier iterations (kept for history)

Two distinct bugs in `slop-tool-boxes.ts`, both from the pending-tool border
pulse (d3454b8), fixed in the follow-up commit:

1. **Streaming flicker** (flicker as tokens stream into an in-flight edit/write):
   the pulse recolored the side rails `│` on *every body line every frame*, so
   each new token repainted the whole (growing) box instead of just the new
   line. **Fix:** only the horizontal top/bottom borders pulse; side rails/body
   use a static status color, so streaming redraws only the new content line.

2. **Startup render storm** (`pi -c` flickers so hard it never accepts input):
   "pending" was inferred from the default row status at render time, and
   `ensureState` defaults to `pending`. On session restore every finished tool
   row re-rendered as "pending" → all added to the animation set → a 10fps
   invalidate storm that never settled. **Fix:** animation is now driven ONLY by
   live `tool_execution_start`/`tool_execution_end` events keyed by toolCallId.
   Restored rows fire no start event, so they never animate. Timer stops when no
   toolCallId is active (plus agent_settled/session_shutdown safety net).

Guard test: `slop-tool-boxes` self-test asserts restored rows don't pulse, live
rows pulse only between start/end, and rails stay static during a run.

---

## (historical) ⚠️ Redraw flicker during tool calls

**Symptom:** A strong redraw/flicker effect during a `write` tool call — severe
enough to trigger a photosensitive seizure. This is a health-safety bug, not
cosmetic.

**Prime suspect:** the pending-tool **border pulse** added in `slop-tool-boxes.ts`
(commit `d3454b8`). While a tool is in progress it runs a 10fps timer that calls
each pending row's `context.invalidate()` → `updateDisplay()` + `ui.requestRender()`.
If a full-frame repaint (or a terminal clear+redraw) happens each tick, that is a
~10Hz full-screen flash — squarely in the photosensitive-seizure danger band
(roughly 3–30 Hz).

**Why `write` specifically:** `write`/`edit` rows can change height as content
streams in (diff/preview), so a repaint that isn't a clean in-place overwrite may
clear and redraw the whole transcript region each pulse frame → flicker.

**Things to check when investigating:**
- Does `ui.requestRender()` do an in-place diffed redraw, or a full clear+repaint?
  If full, the pulse must not drive it at 10fps.
- Is the flicker from our pulse, or from `write`'s native streaming render, or the
  interaction of the two (pulse invalidation forcing native re-render mid-stream)?
- Does it also happen with `edit` (diff) and long `bash` output, or only `write`?
- Repro with only `slop-tool-boxes.ts` loaded vs. with the last-prompt pulse too.

**Candidate mitigations (pick after diagnosis):**
- Lower pulse rate hard (e.g. 2–4 fps) or make it opt-in / respect a
  "reduced motion" setting/env (`PI_REDUCED_MOTION`).
- Don't pulse `write`/`edit` rows at all while their content is still streaming;
  only pulse a static, settled-height frame.
- Repaint only the border glyphs, never triggering `updateDisplay()` (which
  rebuilds the native component) — drive a lighter render path.
- Provide a global kill switch to disable all border pulses.

**Interim:** offer to disable the tool-box pulse (or drop it to a low fps) on
request; the last-prompt topbar pulse is smaller but should be checked too.