Analysis notes: UI desync pop/push line count mismatch

Documents the root cause of the streaming display bug —
pop removes 1 line per entry but push produces N lines
(markdown, tool results). Includes concrete fix approach
using per-entry line count tracking.

Co-Authored-By: Proof of Concept <poc@bcachefs.org>
This commit is contained in:
Kent Overstreet 2026-04-07 11:54:30 -04:00
parent f387041aca
commit 382ebc95aa

View file

@ -0,0 +1,100 @@
# UI Desync Analysis — Pending Input + Entry Pop (2026-04-07)
## Context
The F1 conversation pane has a desync bug where entries aren't
properly removed when they change (streaming updates, compaction).
Qwen's fix restored the pending_display_count approach for pending
input, which works. The remaining issue is the **entry-level pop**.
## The Bug: Pop/Push Line Count Mismatch
In `sync_from_agent()` (chat.rs), Phase 1 pops changed entries and
Phase 2 pushes new ones. The push and pop paths produce different
numbers of display lines for the same entry.
### Push path (Phase 2, lines 512-536):
- **Conversation/ConversationAssistant**: `append_text(&text)` +
`flush_pending()`. In markdown mode, `flush_pending` runs
`parse_markdown()` which can produce N lines from the input text
(paragraph breaks, code blocks, etc.)
- **Tools**: `push_line(text, Color::Yellow)` — exactly 1 line.
- **ToolResult**: `text.lines().take(20)` — up to 20 lines, each
pushed separately.
### Pop path (Phase 1, lines 497-507):
```rust
for (target, _, _) in Self::route_entry(&popped) {
match target {
PaneTarget::Conversation | PaneTarget::ConversationAssistant
=> self.conversation.pop_line(),
PaneTarget::Tools | PaneTarget::ToolResult
=> self.tools.pop_line(),
}
}
```
This pops **one line per route_entry item**, not per display line.
### The mismatch:
| Target | Push lines | Pop lines | Delta |
|---------------------|-----------|-----------|----------|
| Conversation (md) | N (from parse_markdown) | 1 | N-1 stale lines |
| Tools | 1 | 1 | OK |
| ToolResult | up to 20 | 1 | up to 19 stale lines |
## When it matters
During **streaming**: the last assistant entry is modified on each
token batch. `sync_from_agent` detects the mismatch (line 485),
pops the old entry (1 line), pushes the new entry (N lines from
markdown). Next update: pops 1 line again, but there are now N
lines from the previous push. Stale lines accumulate.
## Fix approach
Track the actual number of display lines each entry produced.
Simplest: snapshot `conversation.lines.len()` before and after
pushing each entry in Phase 2. Store the deltas in a parallel
`Vec<(usize, usize)>` (conversation_lines, tools_lines) alongside
`last_entries`. Use these recorded counts when popping in Phase 1.
```rust
// Phase 2: push new entries (modified)
let conv_before = self.conversation.lines.len();
let tools_before = self.tools.lines.len();
for (target, text, marker) in Self::route_entry(entry) {
// ... existing push logic ...
}
let conv_delta = self.conversation.lines.len() - conv_before;
let tools_delta = self.tools.lines.len() - tools_before;
self.last_entry_line_counts.push((conv_delta, tools_delta));
// Phase 1: pop (modified)
while self.last_entries.len() > pop {
self.last_entries.pop();
let (conv_lines, tools_lines) = self.last_entry_line_counts.pop().unwrap();
for _ in 0..conv_lines { self.conversation.pop_line(); }
for _ in 0..tools_lines { self.tools.pop_line(); }
}
```
## Note on PaneState::evict()
`evict()` can remove old lines from the beginning when the pane
exceeds `MAX_PANE_LINES` (10,000). This could make the delta-based
approach slightly inaccurate for very old entries. But we only pop
recent entries (streaming updates are always at the tail), so
eviction doesn't affect the entries we're popping.
## Files
- `src/user/chat.rs:461-550` — sync_from_agent
- `src/user/chat.rs:282-298` — PaneState::append_text (markdown path)
- `src/user/chat.rs:261-276` — PaneState::flush_pending
- `src/user/chat.rs:206-219` — parse_markdown