Fix some issues with UI getting out of sync. (#7)
Reviewed-on: #7 Co-authored-by: Drew Galbraith <drew@tiramisu.one> Co-committed-by: Drew Galbraith <drew@tiramisu.one>
This commit is contained in:
parent
0fcdf4ed0d
commit
af080710cc
8 changed files with 199 additions and 84 deletions
|
|
@ -4,7 +4,7 @@ use tokio::sync::mpsc;
|
|||
use tracing::debug;
|
||||
|
||||
use super::AppState;
|
||||
use crate::core::types::{Role, UIEvent};
|
||||
use crate::core::types::{Role, StampedEvent, UIEvent};
|
||||
|
||||
/// Drain all pending [`UIEvent`]s from `event_rx` and apply them to `state`.
|
||||
///
|
||||
|
|
@ -19,15 +19,25 @@ use crate::core::types::{Role, UIEvent};
|
|||
/// | `ToolResult` | Display tool result |
|
||||
/// | `TurnComplete` | No structural change; logged at debug level |
|
||||
/// | `Error(msg)` | Push `(Assistant, "[error] {msg}")` |
|
||||
pub(super) fn drain_ui_events(event_rx: &mut mpsc::Receiver<UIEvent>, state: &mut AppState) {
|
||||
while let Ok(event) = event_rx.try_recv() {
|
||||
match event {
|
||||
pub(super) fn drain_ui_events(event_rx: &mut mpsc::Receiver<StampedEvent>, state: &mut AppState) {
|
||||
while let Ok(stamped) = event_rx.try_recv() {
|
||||
// Discard events from before the most recent :clear.
|
||||
if stamped.epoch < state.epoch {
|
||||
debug!(
|
||||
event_epoch = stamped.epoch,
|
||||
state_epoch = state.epoch,
|
||||
"dropping stale event"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
match stamped.event {
|
||||
UIEvent::StreamDelta(chunk) => {
|
||||
if let Some((Role::Assistant, content)) = state.messages.last_mut() {
|
||||
content.push_str(&chunk);
|
||||
} else {
|
||||
state.messages.push((Role::Assistant, chunk));
|
||||
}
|
||||
state.content_changed = true;
|
||||
}
|
||||
UIEvent::ToolApprovalRequest {
|
||||
tool_use_id,
|
||||
|
|
@ -47,6 +57,7 @@ pub(super) fn drain_ui_events(event_rx: &mut mpsc::Receiver<UIEvent>, state: &mu
|
|||
state
|
||||
.messages
|
||||
.push((Role::Assistant, format!("[{tool_name}] {input_summary}")));
|
||||
state.content_changed = true;
|
||||
}
|
||||
UIEvent::ToolResult {
|
||||
tool_name,
|
||||
|
|
@ -58,6 +69,7 @@ pub(super) fn drain_ui_events(event_rx: &mut mpsc::Receiver<UIEvent>, state: &mu
|
|||
Role::Assistant,
|
||||
format!("[{tool_name} {prefix}] {output_summary}"),
|
||||
));
|
||||
state.content_changed = true;
|
||||
}
|
||||
UIEvent::TurnComplete => {
|
||||
debug!("turn complete");
|
||||
|
|
@ -69,6 +81,7 @@ pub(super) fn drain_ui_events(event_rx: &mut mpsc::Receiver<UIEvent>, state: &mu
|
|||
state
|
||||
.messages
|
||||
.push((Role::Assistant, format!("[error] {msg}")));
|
||||
state.content_changed = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -86,12 +99,17 @@ pub struct PendingApproval {
|
|||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// Wrap a [`UIEvent`] in a [`StampedEvent`] at epoch 0 for tests.
|
||||
fn stamp(event: UIEvent) -> StampedEvent {
|
||||
StampedEvent { epoch: 0, event }
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn drain_appends_to_existing_assistant() {
|
||||
let (tx, mut rx) = tokio::sync::mpsc::channel(8);
|
||||
let mut state = AppState::new();
|
||||
state.messages.push((Role::Assistant, "hello".to_string()));
|
||||
tx.send(UIEvent::StreamDelta(" world".to_string()))
|
||||
tx.send(stamp(UIEvent::StreamDelta(" world".to_string())))
|
||||
.await
|
||||
.unwrap();
|
||||
drop(tx);
|
||||
|
|
@ -104,7 +122,7 @@ mod tests {
|
|||
let (tx, mut rx) = tokio::sync::mpsc::channel(8);
|
||||
let mut state = AppState::new();
|
||||
state.messages.push((Role::User, "hi".to_string()));
|
||||
tx.send(UIEvent::StreamDelta("hello".to_string()))
|
||||
tx.send(stamp(UIEvent::StreamDelta("hello".to_string())))
|
||||
.await
|
||||
.unwrap();
|
||||
drop(tx);
|
||||
|
|
@ -118,11 +136,11 @@ mod tests {
|
|||
async fn drain_tool_approval_sets_pending() {
|
||||
let (tx, mut rx) = tokio::sync::mpsc::channel(8);
|
||||
let mut state = AppState::new();
|
||||
tx.send(UIEvent::ToolApprovalRequest {
|
||||
tx.send(stamp(UIEvent::ToolApprovalRequest {
|
||||
tool_use_id: "t1".to_string(),
|
||||
tool_name: "write_file".to_string(),
|
||||
input_summary: "path: foo.txt".to_string(),
|
||||
})
|
||||
}))
|
||||
.await
|
||||
.unwrap();
|
||||
drop(tx);
|
||||
|
|
@ -136,11 +154,11 @@ mod tests {
|
|||
async fn drain_tool_result_adds_message() {
|
||||
let (tx, mut rx) = tokio::sync::mpsc::channel(8);
|
||||
let mut state = AppState::new();
|
||||
tx.send(UIEvent::ToolResult {
|
||||
tx.send(stamp(UIEvent::ToolResult {
|
||||
tool_name: "read_file".to_string(),
|
||||
output_summary: "file contents...".to_string(),
|
||||
is_error: false,
|
||||
})
|
||||
}))
|
||||
.await
|
||||
.unwrap();
|
||||
drop(tx);
|
||||
|
|
@ -148,4 +166,42 @@ mod tests {
|
|||
assert_eq!(state.messages.len(), 1);
|
||||
assert!(state.messages[0].1.contains("read_file result"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn drain_discards_stale_epoch_events() {
|
||||
let (tx, mut rx) = tokio::sync::mpsc::channel(8);
|
||||
let mut state = AppState::new();
|
||||
state.epoch = 2;
|
||||
// Event from epoch 1 should be discarded.
|
||||
tx.send(StampedEvent {
|
||||
epoch: 1,
|
||||
event: UIEvent::StreamDelta("ghost".to_string()),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
// Event from epoch 2 should be accepted.
|
||||
tx.send(StampedEvent {
|
||||
epoch: 2,
|
||||
event: UIEvent::StreamDelta("real".to_string()),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
drop(tx);
|
||||
drain_ui_events(&mut rx, &mut state);
|
||||
assert_eq!(state.messages.len(), 1);
|
||||
assert_eq!(state.messages[0].1, "real");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn drain_sets_content_changed() {
|
||||
let (tx, mut rx) = tokio::sync::mpsc::channel(8);
|
||||
let mut state = AppState::new();
|
||||
assert!(!state.content_changed);
|
||||
tx.send(stamp(UIEvent::StreamDelta("hi".to_string())))
|
||||
.await
|
||||
.unwrap();
|
||||
drop(tx);
|
||||
drain_ui_events(&mut rx, &mut state);
|
||||
assert!(state.content_changed);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -198,6 +198,7 @@ fn execute_command(buf: &str, state: &mut AppState) -> Option<LoopControl> {
|
|||
match buf.trim() {
|
||||
"quit" | "q" => Some(LoopControl::Quit),
|
||||
"clear" => {
|
||||
state.epoch += 1;
|
||||
state.messages.clear();
|
||||
state.scroll = 0;
|
||||
Some(LoopControl::ClearHistory)
|
||||
|
|
|
|||
|
|
@ -26,7 +26,7 @@ use ratatui::Terminal;
|
|||
use ratatui::backend::CrosstermBackend;
|
||||
use tokio::sync::mpsc;
|
||||
|
||||
use crate::core::types::{Role, UIEvent, UserAction};
|
||||
use crate::core::types::{Role, StampedEvent, UserAction};
|
||||
|
||||
/// Errors that can occur in the TUI layer.
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
|
|
@ -78,6 +78,12 @@ pub struct AppState {
|
|||
pub sandbox_yolo: bool,
|
||||
/// Whether network access is currently allowed.
|
||||
pub network_allowed: bool,
|
||||
/// Monotonic epoch incremented on `:clear`. Events with an older epoch are
|
||||
/// discarded by `drain_ui_events` to prevent ghost messages.
|
||||
pub epoch: u64,
|
||||
/// Set by `drain_ui_events` when message content changes; consumed by
|
||||
/// `update_scroll` to auto-follow only when new content arrives.
|
||||
pub content_changed: bool,
|
||||
}
|
||||
|
||||
impl AppState {
|
||||
|
|
@ -94,6 +100,8 @@ impl AppState {
|
|||
pending_approval: None,
|
||||
sandbox_yolo: false,
|
||||
network_allowed: false,
|
||||
epoch: 0,
|
||||
content_changed: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -150,7 +158,7 @@ pub fn install_panic_hook() {
|
|||
/// returns `Ok(())`.
|
||||
pub async fn run(
|
||||
action_tx: mpsc::Sender<UserAction>,
|
||||
mut event_rx: mpsc::Receiver<UIEvent>,
|
||||
mut event_rx: mpsc::Receiver<StampedEvent>,
|
||||
sandbox_yolo: bool,
|
||||
) -> Result<(), TuiError> {
|
||||
install_panic_hook();
|
||||
|
|
@ -194,7 +202,9 @@ pub async fn run(
|
|||
break;
|
||||
}
|
||||
Some(input::LoopControl::ClearHistory) => {
|
||||
let _ = action_tx.send(UserAction::ClearHistory).await;
|
||||
let _ = action_tx
|
||||
.send(UserAction::ClearHistory { epoch: state.epoch })
|
||||
.await;
|
||||
}
|
||||
Some(input::LoopControl::ToolApproval {
|
||||
tool_use_id,
|
||||
|
|
|
|||
|
|
@ -38,15 +38,18 @@ pub(super) fn update_scroll(state: &mut AppState, area: Rect) {
|
|||
let max_scroll = total_lines.saturating_sub(viewport_height);
|
||||
|
||||
match state.mode {
|
||||
// In Insert mode, auto-follow the bottom of the conversation.
|
||||
Mode::Insert => {
|
||||
// In Insert mode, auto-follow only when new content has arrived.
|
||||
Mode::Insert if state.content_changed => {
|
||||
state.scroll = max_scroll;
|
||||
}
|
||||
// In Normal/Command mode, the user controls scroll -- just clamp to bounds.
|
||||
// Otherwise, just clamp to bounds.
|
||||
_ => {
|
||||
state.scroll = state.scroll.min(max_scroll);
|
||||
}
|
||||
}
|
||||
// Reset unconditionally -- this is the single place that consumes the flag,
|
||||
// regardless of which scroll branch ran above.
|
||||
state.content_changed = false;
|
||||
}
|
||||
|
||||
/// Compute the overlay rectangle for the command palette, centered on `output_area`.
|
||||
|
|
@ -298,6 +301,7 @@ mod tests {
|
|||
for i in 0..50 {
|
||||
state.messages.push((Role::User, format!("message {i}")));
|
||||
}
|
||||
state.content_changed = true; // simulate new content arriving
|
||||
let area = Rect::new(0, 0, 80, 24);
|
||||
update_scroll(&mut state, area);
|
||||
assert!(state.scroll > 0, "expected scroll > 0 with 50 messages");
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue