Skip to content

fix(panel): skip live-append re-render while the full-message overlay is open (#85) - #99

Merged
rz1989s merged 2 commits into
mainfrom
fix/85-overlay-rebuild-churn
Sep 2, 2026
Merged

rz1989s merged 2 commits into
mainfrom
fix/85-overlay-rebuild-churn

Conversation

@rz1989s

@rz1989s rz1989s commented Sep 2, 2026

Copy link
Copy Markdown
Member

Fixes the churn half of #85 (part 1). Part 2 (widget conversation tail) is dispositioned on the issue per its own note.

Problem

While the full-message overlay was open on a LIVE timeline, every appended event fired renderShell(), rebuilding the entire panel — including a fresh SelectList for the overlay body, which resets the user's scroll position and selection on every append. Watching a live run while reading one of its messages was unusable.

Fix (surgical, at the churn source)

openRunTimeline's live-append subscription now skips renderShell() while fullMessageEvent is set:

  • Overlay content comes from the stable fullMessageEvent — a rebuild renders identical output while churning scroll/selection.
  • Nothing beneath the overlay is visible; no visual state depends on the re-render.
  • Data stays current: runTimeline + selectedEventIndex still update per append; closing the overlay (esc) triggers the next full render, which picks up every accumulated append.
  • Guard deliberately lives in the subscription, NOT at the top of renderShell — a top-level guard would also swallow the render immediately after setting fullMessageEvent (the open action), so the overlay would never appear.
  • Tail-follow when the overlay is closed is untouched (append still rebuilds the timeline).

Why a guard in renderShell was rejected

Considered and rejected: if (this.fullMessageEvent) return at the top of renderShell breaks the open path (tl.onSelect sets fullMessageEvent then calls renderShell() — the early-return would eat exactly that call). It would also silently suppress store-refresh re-renders mid-overlay, entangling unrelated call sites. The subscription is the only site with the churn.

Tests (behavioral — real RunLog, real headless FleetPanel, real key driving)

  • overlay open + append → messageBodyList identity stable, children untouched, runTimeline still grows
  • overlay closed + append → timeline rebuilt (tail-follow preserved, pinned so the fix can't over-suppress)
  • esc after appends → overlay closed, full render includes the events appended underneath

RED was verified first: the churn test failed against main with exactly the identity assertion. 824/824 (was 821), pnpm typecheck clean — both standalone. (The first cut passed tsx tests but failed typecheck — Panelish missing a key + a tautological assert; both gates caught it, per the house rule.)

Part 2 disposition

Widget conversation tail: the issue itself notes it's "only worth doing when a consumer asks; the live overlay covers the need today" — proposing close-wontfix-until-consumer on the issue.

… is open (#85)

Every appended event on a live timeline rebuilt the whole panel — including a
fresh SelectList for the open full-message overlay, resetting its scroll and
selection. The overlay's content comes from the stable fullMessageEvent and
nothing beneath it is visible, so the live subscription now skips renderShell
while it is open. Data stays current (runTimeline/selectedEventIndex still
update); closing the overlay triggers the next full render, which picks up
every accumulated append.

Behavioral tests (real RunLog + headless FleetPanel): overlay body-list identity
stable across appends; tail-follow preserved when the overlay is closed;
post-close render includes events appended underneath. 824/824.
@rz1989s
rz1989s merged commit eb0541f into main Sep 2, 2026
1 check passed
@rz1989s
rz1989s deleted the fix/85-overlay-rebuild-churn branch September 2, 2026 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant