101 lines
3.6 KiB
Markdown
101 lines
3.6 KiB
Markdown
|
|
# 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
|