Skip to content

Commit 36e0c61

Browse files
committed
fix(tui): address second review findings
- Abort agent task on quit to prevent hanging on active API streams. Log panics from the agent task instead of silently dropping them. - Use saturating_add in scroll_down to prevent u16 wrapping. - Show bash commands inline in TUI tool call display. - Extract push_message_lines helper to deduplicate build_text logic. - Make ChatMessage and ChatRole private (only used within chat.rs). - Fix theme doc referencing unimplemented config section.
1 parent ca58d1f commit 36e0c61

4 files changed

Lines changed: 52 additions & 50 deletions

File tree

crates/oxide-code/src/main.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,12 @@ async fn run_tui(client: &Client, model: &str) -> Result<()> {
9494

9595
tui::terminal::restore();
9696

97-
if let Ok(Err(e)) = agent_handle.await {
98-
warn!("agent loop error: {e}");
97+
// Cancel the agent loop — it may be blocked on an API stream.
98+
agent_handle.abort();
99+
match agent_handle.await {
100+
Ok(Err(e)) => warn!("agent loop error: {e}"),
101+
Err(e) if !e.is_cancelled() => warn!("agent task panicked: {e}"),
102+
_ => {}
99103
}
100104

101105
result

crates/oxide-code/src/tui/app.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,13 @@ impl App {
123123
// TODO: Render thinking in a dimmed / collapsible block.
124124
self.status_bar.set_status(Status::Streaming);
125125
}
126-
AgentEvent::ToolCallStart { name, .. } => {
126+
AgentEvent::ToolCallStart { name, input, .. } => {
127127
self.chat.commit_streaming();
128-
self.chat.push_tool_call(&name, None);
128+
// Show bash commands inline for visibility.
129+
let title = (name == "bash")
130+
.then(|| input.get("command").and_then(serde_json::Value::as_str))
131+
.flatten();
132+
self.chat.push_tool_call(&name, title);
129133
self.status_bar.set_status(Status::ToolRunning);
130134
}
131135
AgentEvent::ToolCallEnd { title, .. } => {

crates/oxide-code/src/tui/components/chat.rs

Lines changed: 39 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,15 @@ use crate::tui::theme::Theme;
1111

1212
// ── Chat Message ──
1313

14-
/// A rendered chat message. Stores the role and content text for display.
15-
/// Future PRs will add tool call blocks, markdown rendering, and collapsed
16-
/// sections.
14+
/// A rendered chat message with role and content text.
1715
#[derive(Debug, Clone)]
18-
pub(crate) struct ChatMessage {
19-
pub(crate) role: ChatRole,
20-
pub(crate) content: String,
16+
struct ChatMessage {
17+
role: ChatRole,
18+
content: String,
2119
}
2220

2321
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
24-
pub(crate) enum ChatRole {
22+
enum ChatRole {
2523
User,
2624
Assistant,
2725
}
@@ -119,7 +117,7 @@ impl ChatView {
119117
.content_height
120118
.get()
121119
.saturating_sub(self.viewport_height);
122-
self.scroll_offset = (self.scroll_offset + lines).min(max);
120+
self.scroll_offset = self.scroll_offset.saturating_add(lines).min(max);
123121
if self.scroll_offset >= max {
124122
self.auto_scroll = true;
125123
}
@@ -130,49 +128,47 @@ impl ChatView {
130128
let mut lines: Vec<Line<'_>> = Vec::new();
131129

132130
for msg in &self.messages {
133-
// Single blank line between messages.
134-
if !lines.is_empty() {
135-
lines.push(Line::raw(""));
136-
}
137-
138-
// Role label.
139-
let (label, style) = match msg.role {
140-
ChatRole::User => ("❯ You", self.theme.accent()),
141-
ChatRole::Assistant => ("⟡ Assistant", self.theme.secondary()),
142-
};
143-
lines.push(Line::from(vec![
144-
Span::raw(" "),
145-
Span::styled(label, style),
146-
]));
147-
148-
// Content lines (immediately after label, no blank line).
149-
for line in msg.content.trim().lines() {
150-
lines.push(Line::from(vec![
151-
Span::raw(" "),
152-
Span::styled(line, self.theme.text()),
153-
]));
154-
}
131+
self.push_message_lines(&mut lines, msg.role, &msg.content);
155132
}
156133

157134
// Streaming buffer (not yet committed).
158135
if !self.streaming_buffer.is_empty() {
159-
if !lines.is_empty() {
160-
lines.push(Line::raw(""));
161-
}
162-
lines.push(Line::from(vec![
163-
Span::raw(" "),
164-
Span::styled("⟡ Assistant", self.theme.secondary()),
165-
]));
166-
for line in self.streaming_buffer.trim().lines() {
167-
lines.push(Line::from(vec![
168-
Span::raw(" "),
169-
Span::styled(line, self.theme.text()),
170-
]));
171-
}
136+
self.push_message_lines(&mut lines, ChatRole::Assistant, &self.streaming_buffer);
172137
}
173138

174139
Text::from(lines)
175140
}
141+
142+
/// Append a role label and content lines for a single message.
143+
fn push_message_lines<'a>(
144+
&'a self,
145+
lines: &mut Vec<Line<'a>>,
146+
role: ChatRole,
147+
content: &'a str,
148+
) {
149+
// Single blank line between messages.
150+
if !lines.is_empty() {
151+
lines.push(Line::raw(""));
152+
}
153+
154+
// Role label.
155+
let (label, style) = match role {
156+
ChatRole::User => ("❯ You", self.theme.accent()),
157+
ChatRole::Assistant => ("⟡ Assistant", self.theme.secondary()),
158+
};
159+
lines.push(Line::from(vec![
160+
Span::raw(" "),
161+
Span::styled(label, style),
162+
]));
163+
164+
// Content lines (immediately after label, no blank line).
165+
for line in content.trim().lines() {
166+
lines.push(Line::from(vec![
167+
Span::raw(" "),
168+
Span::styled(line, self.theme.text()),
169+
]));
170+
}
171+
}
176172
}
177173

178174
impl Component for ChatView {

crates/oxide-code/src/tui/theme.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@ use ratatui::style::{Color, Modifier, Style};
55
/// The default theme uses Catppuccin Mocha colors with a transparent
66
/// background (respects the user's terminal theme). All components reference
77
/// the theme via [`Theme::default()`] rather than hardcoding colors.
8-
///
9-
/// Users can override individual color slots in the `[tui.theme]` config
10-
/// section. The built-in palette is named `"default"`.
8+
/// Named color slots are designed for future user-configurable overrides.
119
#[expect(
1210
dead_code,
1311
reason = "all color slots are part of the theme API; not all consumed yet"

0 commit comments

Comments
 (0)