refactor(tui): complete Step 3 and Step 4

- Step 3: fully adopt  and ID newtypes
  * Replace string action checks in history_cell + chatwidget
  * Keep labels (Read/Searched/List Files/Ran) identical
  * Wrap stream/exec/tool ids in StreamId/ExecCallId/ToolCallId
- Step 4: streaming API cleanup
  * Add  and refactor callers
  * Encapsulate  mutations inside streaming module
  * Keep behavior identical; zero warnings

Validated with ./build-fast.sh (clean).
This commit is contained in:
James Peter
2025-09-02 21:08:47 +10:00
parent b173ae1a49
commit b7c660283e
3 changed files with 62 additions and 52 deletions
+19 -25
View File
@@ -441,7 +441,7 @@ impl ChatWidget<'_> {
let codex_protocol::models::ReasoningItemReasoningSummary::SummaryText { text } = s;
// Reasoning cell – use the existing reasoning output styling
let sink = crate::streaming::controller::AppEventHistorySink(self.app_event_tx.clone());
self.stream_state.current_kind = Some(crate::streaming::StreamKind::Reasoning);
streaming::begin(self, StreamKind::Reasoning, None);
let _ = self.stream.apply_final_reasoning(&text, &sink);
// finalize immediately for static replay
self.stream.finalize(crate::streaming::StreamKind::Reasoning, true, &sink);
@@ -1477,16 +1477,17 @@ impl ChatWidget<'_> {
} else {
// Compute header label based on parsed action and status
let exec_label = |e: &history_cell::ExecCell| -> &'static str {
let action = history_cell::action_from_parsed(&e.parsed);
use crate::history_cell::ExecAction as EA;
let action = history_cell::action_enum_from_parsed(&e.parsed);
match (&e.output, action) {
(None, "read") => "Read",
(None, "search") => "Searched",
(None, "list") => "List Files",
(None, _) => "Running...",
(Some(o), "read") if o.exit_code == 0 => "Read",
(Some(o), "search") if o.exit_code == 0 => "Searched",
(Some(o), "list") if o.exit_code == 0 => "List Files",
(Some(o), _) if o.exit_code == 0 => "Ran",
(None, EA::Read) => "Read",
(None, EA::Search) => "Searched",
(None, EA::List) => "List Files",
(None, EA::Run) => "Running...",
(Some(o), EA::Read) if o.exit_code == 0 => "Read",
(Some(o), EA::Search) if o.exit_code == 0 => "Searched",
(Some(o), EA::List) if o.exit_code == 0 => "List Files",
(Some(o), EA::Run) if o.exit_code == 0 => "Ran",
_ => "",
}
};
@@ -1960,9 +1961,7 @@ impl ChatWidget<'_> {
self.finalize_all_running_due_to_answer();
// Use StreamController for final answer
let sink = AppEventHistorySink(self.app_event_tx.clone());
self.stream_state.current_kind = Some(StreamKind::Answer);
// Seed the controller with this id so finalize targets the right streaming cell
self.stream.begin_with_id(StreamKind::Answer, Some(id.clone()), &sink);
streaming::begin(self, StreamKind::Answer, Some(id.clone()));
let _finished = self.stream.apply_final_answer(&message, &sink);
// Stream finishing is handled by StreamController
self.last_assistant_message = Some(message);
@@ -1992,7 +1991,7 @@ impl ChatWidget<'_> {
return;
}
// Stream answer delta through StreamController
self.handle_streaming_delta(StreamKind::Answer, id.clone(), delta);
streaming::delta_text(self, StreamKind::Answer, id.clone(), delta);
// Show responding state while assistant streams
self.bottom_pane.update_status_text("responding".to_string());
}
@@ -2009,8 +2008,7 @@ impl ChatWidget<'_> {
self.finalize_all_running_due_to_answer();
// Use StreamController for final reasoning
let sink = AppEventHistorySink(self.app_event_tx.clone());
self.stream_state.current_kind = Some(StreamKind::Reasoning);
self.stream.begin_with_id(StreamKind::Reasoning, Some(id.clone()), &sink);
streaming::begin(self, StreamKind::Reasoning, Some(id.clone()));
// The StreamController now properly handles duplicate detection and prevents
// re-injecting content when we're already finishing a stream
@@ -2028,7 +2026,7 @@ impl ChatWidget<'_> {
return;
}
// Stream reasoning delta through StreamController
self.handle_streaming_delta(StreamKind::Reasoning, id.clone(), delta);
streaming::delta_text(self, StreamKind::Reasoning, id.clone(), delta);
// Show thinking state while reasoning streams
self.bottom_pane.update_status_text("thinking".to_string());
}
@@ -2060,13 +2058,10 @@ impl ChatWidget<'_> {
}
EventMsg::TaskComplete(TaskCompleteEvent { last_agent_message: _ }) => {
// Finalize any active streams
let sink = AppEventHistorySink(self.app_event_tx.clone());
if self.stream.is_write_cycle_active() {
// Finalize both streams
self.stream_state.current_kind = Some(StreamKind::Reasoning);
self.stream.finalize(StreamKind::Reasoning, true, &sink);
self.stream_state.current_kind = Some(StreamKind::Answer);
self.stream.finalize(StreamKind::Answer, true, &sink);
// Finalize both streams via streaming facade
streaming::finalize(self, StreamKind::Reasoning, true);
streaming::finalize(self, StreamKind::Answer, true);
}
// Remove this id from the active set (it may be a sub‑agent)
self.active_task_ids.remove(&id);
@@ -2155,8 +2150,7 @@ impl ChatWidget<'_> {
}
// Use StreamController for final raw reasoning
let sink = AppEventHistorySink(self.app_event_tx.clone());
self.stream_state.current_kind = Some(StreamKind::Reasoning);
self.stream.begin_with_id(StreamKind::Reasoning, Some(id.clone()), &sink);
streaming::begin(self, StreamKind::Reasoning, Some(id.clone()));
let _finished = self.stream.apply_final_reasoning(&text, &sink);
// Stream finishing is handled by StreamController
self.stream_state.closed_reasoning_ids.insert(StreamId(id.clone()));
+22 -6
View File
@@ -28,20 +28,36 @@ pub(super) fn handle_streaming_delta(
return;
}
tracing::debug!("handle_streaming_delta kind={:?}, delta={:?}", kind, delta);
delta_text(chat, kind, id, delta);
chat.mark_needs_redraw();
}
// New facade: begin a stream for a kind, with optional id
pub(super) fn begin(chat: &mut ChatWidget<'_>, kind: StreamKind, id: Option<String>) {
chat.stream_state.current_kind = Some(kind);
let sink = AppEventHistorySink(chat.app_event_tx.clone());
chat.stream.begin_with_id(kind, id, &sink);
}
// New facade: apply a delta (ensures begin is called for this id/kind)
pub(super) fn delta_text(chat: &mut ChatWidget<'_>, kind: StreamKind, id: String, delta: String) {
chat.stream_state.current_kind = Some(kind);
let sink = AppEventHistorySink(chat.app_event_tx.clone());
chat.stream.begin_with_id(kind, Some(id), &sink);
chat.stream.push_and_maybe_commit(&delta, &sink);
chat.mark_needs_redraw();
}
// New facade: finalize a specific kind and optionally follow bottom
pub(super) fn finalize(chat: &mut ChatWidget<'_>, kind: StreamKind, follow_bottom: bool) {
let sink = AppEventHistorySink(chat.app_event_tx.clone());
chat.stream_state.current_kind = Some(kind);
chat.stream.finalize(kind, follow_bottom, &sink);
}
pub(super) fn finalize_active_stream(chat: &mut ChatWidget<'_>) {
let sink = AppEventHistorySink(chat.app_event_tx.clone());
if chat.stream.is_write_cycle_active() {
chat.stream_state.current_kind = Some(StreamKind::Reasoning);
chat.stream.finalize(StreamKind::Reasoning, true, &sink);
chat.stream_state.current_kind = Some(StreamKind::Answer);
chat.stream.finalize(StreamKind::Answer, true, &sink);
finalize(chat, StreamKind::Reasoning, true);
finalize(chat, StreamKind::Answer, true);
chat.height_manager
.borrow_mut()
.record_event(HeightEvent::HistoryFinalize);
+21 -21
View File
@@ -4345,7 +4345,7 @@ fn new_parsed_command(
output: Option<&CommandOutput>,
start_time: Option<Instant>,
) -> Vec<Line<'static>> {
let action = action_from_parsed(parsed_commands);
let action = action_enum_from_parsed(&parsed_commands.to_vec());
let ctx_path = first_context_path(parsed_commands);
let mut lines: Vec<Line> = vec![match output {
None => {
@@ -4357,16 +4357,16 @@ fn new_parsed_command(
};
// Running state per action
let header = match action {
"read" => "Read".to_string(),
"search" => "Searched".to_string(),
"list" => "List Files".to_string(),
_ => match &ctx_path {
ExecAction::Read => "Read".to_string(),
ExecAction::Search => "Searched".to_string(),
ExecAction::List => "List Files".to_string(),
ExecAction::Run => match &ctx_path {
Some(p) => format!("Running... in {p}"),
None => "Running...".to_string(),
},
};
// Use non-bold styling for informational actions; use info color
if matches!(action, "read" | "search" | "list") {
if matches!(action, ExecAction::Read | ExecAction::Search | ExecAction::List) {
Line::styled(
format!("{header}{duration_str}"),
Style::default().fg(crate::colors::info()),
@@ -4382,16 +4382,16 @@ fn new_parsed_command(
}
Some(o) if o.exit_code == 0 => {
let done = match action {
"read" => "Read".to_string(),
"search" => "Searched".to_string(),
"list" => "List Files".to_string(),
_ => match &ctx_path {
ExecAction::Read => "Read".to_string(),
ExecAction::Search => "Searched".to_string(),
ExecAction::List => "List Files".to_string(),
ExecAction::Run => match &ctx_path {
Some(p) => format!("Ran in {p}"),
None => "Ran".to_string(),
},
};
// Color by action: informational (Read/Search/List) use normal text; execution uses primary
if matches!(action, "read" | "search" | "list") {
if matches!(action, ExecAction::Read | ExecAction::Search | ExecAction::List) {
Line::styled(done, Style::default().fg(crate::colors::text()))
} else {
Line::styled(
@@ -4407,16 +4407,16 @@ fn new_parsed_command(
// can still see what operation was attempted. Error details are
// rendered below via `output_lines`.
let done = match action {
"read" => "Read".to_string(),
"search" => "Searched".to_string(),
"list" => "List Files".to_string(),
_ => match &ctx_path {
ExecAction::Read => "Read".to_string(),
ExecAction::Search => "Searched".to_string(),
ExecAction::List => "List Files".to_string(),
ExecAction::Run => match &ctx_path {
Some(p) => format!("Ran in {p}"),
None => "Ran".to_string(),
},
};
// Use the same styling as success to keep headers stable/recognizable.
if matches!(action, "read" | "search" | "list") {
if matches!(action, ExecAction::Read | ExecAction::Search | ExecAction::List) {
Line::styled(done, Style::default().fg(crate::colors::text()))
} else {
Line::styled(
@@ -4444,10 +4444,10 @@ fn new_parsed_command(
// Restrict displayed entries to the primary action for this cell.
// For the generic "run" header, allow Run/Test/Lint/Format entries.
let expected_label: Option<&'static str> = match action {
"read" => Some("Read"),
"search" => Some("Search"),
"list" => Some("List Files"),
_ => None,
ExecAction::Read => Some("Read"),
ExecAction::Search => Some("Search"),
ExecAction::List => Some("List Files"),
ExecAction::Run => None,
};
for parsed in parsed_commands.iter() {
@@ -4727,7 +4727,7 @@ fn new_parsed_command(
}
// Show stdout for real run commands; keep read/search/list concise unless error
let show_stdout = action == "run";
let show_stdout = matches!(action, ExecAction::Run);
let use_angle_pipe = show_stdout; // add "> " prefix for run output
lines.extend(output_lines(output, !show_stdout, use_angle_pipe));
lines.push(Line::from(""));