hanzoai/dev
2
0
Fork 0
mirror of https://github.com/hanzoai/dev.git synced 2026-10-25 03:32:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(tui): finish step 3 partials — ID newtypes + broader ExecAction usage

Browse Source 
- Add newtypes: ExecCallId, ToolCallId, StreamId
- Migrate ChatWidget state maps/sets to newtypes and update call sites
- Switch remaining stream-id guards to newtype wrappers
- Use ExecAction in exec begin/end; leave deep history_cell formatting on strings for now

Build: ./build-fast.sh passing (no warnings).
This commit is contained in:
James Peter
2025-09-02 20:15:04 +10:00
parent 1518768e1c
commit c3f989c900
3 changed files with 45 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/tui/src/chatwidget.rs
+27 -19
View File
@@ -188,7 +188,7 @@ pub(crate) struct ChatWidget<'a> {
interrupts: interrupts::InterruptManager,
// Guard for out-of-order exec events: track call_ids that already ended
ended_call_ids: HashSet<String>,
ended_call_ids: HashSet<ExecCallId>,
// Accumulated diff/session state
diffs: DiffsState,
@@ -264,6 +264,14 @@ enum AgentStatus {
use self::message::create_initial_user_message;
// Newtype IDs for clarity across exec/tools/streams
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub(super) struct ExecCallId(pub String);
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub(super) struct ToolCallId(pub String);
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub(super) struct StreamId(pub String);
impl ChatWidget<'_> {
/// Hide the bottom spinner/status if the UI is idle (no streams, tools, agents, or tasks).
fn maybe_hide_spinner(&mut self) {
@@ -1959,7 +1967,7 @@ impl ChatWidget<'_> {
// Stream finishing is handled by StreamController
self.last_assistant_message = Some(message);
// Mark this id closed for further answer deltas in this turn
self.stream_state.closed_answer_ids.insert(id.clone());
self.stream_state.closed_answer_ids.insert(StreamId(id.clone()));
// Defensive: this turn's task should be considered complete for UI purposes
self.active_task_ids.remove(&id);
// Defensive: clear transient agents-preparing state unless we see real updates
@@ -1979,7 +1987,7 @@ impl ChatWidget<'_> {
EventMsg::AgentMessageDelta(AgentMessageDeltaEvent { delta }) => {
tracing::debug!("AgentMessageDelta: {:?}", delta);
// Ignore late deltas for ids that have already finalized in this turn
if self.stream_state.closed_answer_ids.contains(&id) {
if self.stream_state.closed_answer_ids.contains(&StreamId(id.clone())) {
tracing::debug!("Ignoring Answer delta for closed id={}", id);
return;
}
@@ -1993,7 +2001,7 @@ impl ChatWidget<'_> {
// Guard: duplicate final Reasoning for the same id can arrive due to provider
// retries or thread replays. If we've already finalized this id in the current
// turn, ignore to avoid duplicating reasoning content in history.
if self.stream_state.closed_reasoning_ids.contains(&id) {
if self.stream_state.closed_reasoning_ids.contains(&StreamId(id.clone())) {
tracing::warn!("Ignoring duplicate AgentReasoning for closed id={}", id);
return;
}
@@ -2009,13 +2017,13 @@ impl ChatWidget<'_> {
let _finished = self.stream.apply_final_reasoning(&text, &sink);
// Stream finishing is handled by StreamController
// Mark this id closed for further reasoning deltas in this turn
self.stream_state.closed_reasoning_ids.insert(id.clone());
self.stream_state.closed_reasoning_ids.insert(StreamId(id.clone()));
self.mark_needs_redraw();
}
EventMsg::AgentReasoningDelta(AgentReasoningDeltaEvent { delta }) => {
tracing::debug!("AgentReasoningDelta: {:?}", delta);
// Ignore late deltas for ids that have already finalized in this turn
if self.stream_state.closed_reasoning_ids.contains(&id) {
if self.stream_state.closed_reasoning_ids.contains(&StreamId(id.clone())) {
tracing::debug!("Ignoring Reasoning delta for closed id={}", id);
return;
}
@@ -2070,7 +2078,7 @@ impl ChatWidget<'_> {
if !self.tools_state.running_web_search.is_empty() {
// Replace each running web search cell in-place with a completed one
// Iterate over a snapshot of keys to avoid borrow issues
let entries: Vec<(String, (usize, Option<String>))> = self
let entries: Vec<(ToolCallId, (usize, Option<String>))> = self
.tools_state.running_web_search
.iter()
.map(|(k, v)| (k.clone(), v.clone()))
@@ -2133,7 +2141,7 @@ impl ChatWidget<'_> {
}
EventMsg::AgentReasoningRawContentDelta(AgentReasoningRawContentDeltaEvent { delta }) => {
// Treat raw reasoning content the same as summarized reasoning
if self.stream_state.closed_reasoning_ids.contains(&id) {
if self.stream_state.closed_reasoning_ids.contains(&StreamId(id.clone())) {
tracing::debug!("Ignoring RawContent delta for closed id={}", id);
return;
}
@@ -2141,7 +2149,7 @@ impl ChatWidget<'_> {
}
EventMsg::AgentReasoningRawContent(AgentReasoningRawContentEvent { text }) => {
tracing::debug!("AgentReasoningRawContent event with text: {:?}...", text.chars().take(100).collect::<String>());
if self.stream_state.closed_reasoning_ids.contains(&id) {
if self.stream_state.closed_reasoning_ids.contains(&StreamId(id.clone())) {
tracing::warn!("Ignoring duplicate AgentReasoningRawContent for closed id={}", id);
return;
}
@@ -2151,7 +2159,7 @@ impl ChatWidget<'_> {
self.stream.begin_with_id(StreamKind::Reasoning, Some(id.clone()), &sink);
let _finished = self.stream.apply_final_reasoning(&text, &sink);
// Stream finishing is handled by StreamController
self.stream_state.closed_reasoning_ids.insert(id.clone());
self.stream_state.closed_reasoning_ids.insert(StreamId(id.clone()));
self.mark_needs_redraw();
}
EventMsg::TokenCount(token_usage) => {
@@ -2318,7 +2326,7 @@ impl ChatWidget<'_> {
self.history_push(cell);
// Track index so we can replace it on completion
if let Some(last_idx) = self.history_cells.len().checked_sub(1) {
self.tools_state.running_custom_tools.insert(call_id.clone(), last_idx);
self.tools_state.running_custom_tools.insert(ToolCallId(call_id.clone()), last_idx);
}
// Update border status based on tool
@@ -2354,7 +2362,7 @@ impl ChatWidget<'_> {
success,
content,
);
if let Some(idx) = self.tools_state.running_custom_tools.remove(&call_id) {
if let Some(idx) = self.tools_state.running_custom_tools.remove(&ToolCallId(call_id)) {
if idx < self.history_cells.len() {
self.history_replace_at(idx, Box::new(completed));
} else {
@@ -2376,7 +2384,7 @@ impl ChatWidget<'_> {
success,
content,
);
if let Some(idx) = self.tools_state.running_custom_tools.remove(&call_id) {
if let Some(idx) = self.tools_state.running_custom_tools.remove(&ToolCallId(call_id)) {
if idx < self.history_cells.len() {
self.history_replace_at(idx, Box::new(completed));
} else {
@@ -3517,7 +3525,7 @@ impl ChatWidget<'_> {
// If we already finalized this id in the current turn with identical content,
// drop this event to avoid duplicates (belt-and-suspenders against upstream repeats).
if let Some(ref want) = id {
if self.stream_state.closed_answer_ids.contains(want) {
if self.stream_state.closed_answer_ids.contains(&StreamId(want.clone())) {
if let Some(existing_idx) = self
.history_cells
.iter()
@@ -6938,20 +6946,20 @@ fn coalesce_read_ranges_in_lines(lines: &mut Vec<ratatui::text::Line<'static>>)
}
#[derive(Default)]
struct ExecState {
running_commands: HashMap<String, RunningCommand>,
running_commands: HashMap<ExecCallId, RunningCommand>,
running_read_agg_index: Option<usize>,
}
#[derive(Default)]
struct ToolState {
running_custom_tools: HashMap<String, usize>,
running_web_search: HashMap<String, (usize, Option<String>)>,
running_custom_tools: HashMap<ToolCallId, usize>,
running_web_search: HashMap<ToolCallId, (usize, Option<String>)>,
}
#[derive(Default)]
struct StreamState {
current_kind: Option<StreamKind>,
closed_answer_ids: HashSet<String>,
closed_reasoning_ids: HashSet<String>,
closed_answer_ids: HashSet<StreamId>,
closed_reasoning_ids: HashSet<StreamId>,
next_seq: u64,
seq_answer_final: Option<u64>,
drop_streaming: bool,
codex-rs/tui/src/chatwidget/exec_tools.rs
+14 -14
View File
@@ -33,7 +33,7 @@ pub(super) fn finalize_exec_cell_at(
pub(super) fn finalize_all_running_as_interrupted(chat: &mut ChatWidget<'_>) {
let interrupted_msg = "Interrupted by user".to_string();
let stdout_empty = String::new();
let running: Vec<(String, Option<usize>)> = chat
let running: Vec<(super::ExecCallId, Option<usize>)> = chat
.exec
.running_commands
.iter()
@@ -47,7 +47,7 @@ pub(super) fn finalize_all_running_as_interrupted(chat: &mut ChatWidget<'_>) {
chat.exec.running_commands.clear();
if !chat.tools_state.running_custom_tools.is_empty() {
let entries: Vec<(String, usize)> = chat
let entries: Vec<(super::ToolCallId, usize)> = chat
.tools_state
.running_custom_tools
.iter()
@@ -71,7 +71,7 @@ pub(super) fn finalize_all_running_as_interrupted(chat: &mut ChatWidget<'_>) {
}
if !chat.tools_state.running_web_search.is_empty() {
let entries: Vec<(String, (usize, Option<String>))> = chat
let entries: Vec<(super::ToolCallId, (usize, Option<String>))> = chat
.tools_state
.running_web_search
.iter()
@@ -132,7 +132,7 @@ pub(super) fn finalize_all_running_as_interrupted(chat: &mut ChatWidget<'_>) {
pub(super) fn finalize_all_running_due_to_answer(chat: &mut ChatWidget<'_>) {
let note = "Completed (final answer received)".to_string();
let stdout_empty = String::new();
let running: Vec<(String, Option<usize>)> = chat
let running: Vec<(super::ExecCallId, Option<usize>)> = chat
.exec
.running_commands
.iter()
@@ -146,7 +146,7 @@ pub(super) fn finalize_all_running_due_to_answer(chat: &mut ChatWidget<'_>) {
chat.exec.running_commands.clear();
if !chat.tools_state.running_custom_tools.is_empty() {
let entries: Vec<(String, usize)> = chat
let entries: Vec<(super::ToolCallId, usize)> = chat
.tools_state
.running_custom_tools
.iter()
@@ -170,7 +170,7 @@ pub(super) fn finalize_all_running_due_to_answer(chat: &mut ChatWidget<'_>) {
}
if !chat.tools_state.running_web_search.is_empty() {
let entries: Vec<(String, (usize, Option<String>))> = chat
let entries: Vec<(super::ToolCallId, (usize, Option<String>))> = chat
.tools_state
.running_web_search
.iter()
@@ -298,7 +298,7 @@ pub(super) fn try_merge_completed_exec_at(chat: &mut ChatWidget<'_>, idx: usize)
}
pub(super) fn handle_exec_begin_now(chat: &mut ChatWidget<'_>, ev: ExecCommandBeginEvent) {
if chat.ended_call_ids.contains(&ev.call_id) { return; }
if chat.ended_call_ids.contains(&super::ExecCallId(ev.call_id.clone())) { return; }
for cell in &chat.history_cells { cell.trigger_fade(); }
let parsed_command = ev.parsed_cmd.clone();
let action = history_cell::action_enum_from_parsed(&parsed_command);
@@ -306,7 +306,7 @@ pub(super) fn handle_exec_begin_now(chat: &mut ChatWidget<'_>, ev: ExecCommandBe
if matches!(action, history_cell::ExecAction::Read) {
chat.exec.running_commands.insert(
ev.call_id.clone(),
super::ExecCallId(ev.call_id.clone()),
super::RunningCommand { command: ev.command.clone(), parsed: parsed_command.clone(), history_index: None },
);
let agg_index = match chat.exec.running_read_agg_index {
@@ -346,7 +346,7 @@ pub(super) fn handle_exec_begin_now(chat: &mut ChatWidget<'_>, ev: ExecCommandBe
chat.history_push(cell);
let idx = if chat.history_cells.len() > 0 { chat.history_cells.len() - 1 } else { before_len };
chat.exec.running_commands.insert(
ev.call_id.clone(),
super::ExecCallId(ev.call_id.clone()),
super::RunningCommand { command: ev.command.clone(), parsed: parsed_command, history_index: Some(idx) },
);
if !chat.tools_state.running_web_search.is_empty() {
@@ -355,7 +355,7 @@ pub(super) fn handle_exec_begin_now(chat: &mut ChatWidget<'_>, ev: ExecCommandBe
let preview = chat
.exec
.running_commands
.get(&ev.call_id)
.get(&super::ExecCallId(ev.call_id.clone()))
.map(|rc| rc.command.join(" "))
.unwrap_or_else(|| "command".to_string());
let preview_short = if preview.chars().count() > 40 {
@@ -371,9 +371,9 @@ pub(super) fn handle_exec_begin_now(chat: &mut ChatWidget<'_>, ev: ExecCommandBe
}
pub(super) fn handle_exec_end_now(chat: &mut ChatWidget<'_>, ev: ExecCommandEndEvent) {
chat.ended_call_ids.insert(ev.call_id.clone());
chat.ended_call_ids.insert(super::ExecCallId(ev.call_id.clone()));
let ExecCommandEndEvent { call_id, exit_code, duration: _, stdout, stderr } = ev;
let cmd = chat.exec.running_commands.remove(&call_id);
let cmd = chat.exec.running_commands.remove(&super::ExecCallId(call_id.clone()));
chat.height_manager.borrow_mut().record_event(HeightEvent::RunEnd);
let (command, parsed, history_index) = cmd
.map(|cmd| (cmd.command, cmd.parsed, cmd.history_index))
@@ -464,7 +464,7 @@ pub(super) fn maybe_move_last_before_final_assistant_exec(chat: &mut ChatWidget<
if chat.history_cells.is_empty() { return; }
let last_idx = chat.history_cells.len() - 1; if last_idx <= assistant_idx { return; }
let cell = chat.history_cells.remove(last_idx); chat.history_cells.insert(assistant_idx, cell);
if let Some(rc) = chat.exec.running_commands.get_mut(call_id) { rc.history_index = Some(assistant_idx); }
if let Some(rc) = chat.exec.running_commands.get_mut(&super::ExecCallId(call_id.to_string())) { rc.history_index = Some(assistant_idx); }
chat.invalidate_height_cache(); chat.request_redraw();
}
@@ -473,7 +473,7 @@ pub(super) fn maybe_move_last_before_final_assistant_tool(chat: &mut ChatWidget<
if chat.history_cells.is_empty() { return; }
let last_idx = chat.history_cells.len() - 1; if last_idx <= assistant_idx { return; }
let cell = chat.history_cells.remove(last_idx); chat.history_cells.insert(assistant_idx, cell);
if let Some(idx) = chat.tools_state.running_custom_tools.get_mut(call_id) { *idx = assistant_idx; }
if let Some(idx) = chat.tools_state.running_custom_tools.get_mut(&super::ToolCallId(call_id.to_string())) { *idx = assistant_idx; }
chat.invalidate_height_cache(); chat.request_redraw();
}
codex-rs/tui/src/chatwidget/tools.rs
+4 -4
View File
@@ -12,14 +12,14 @@ pub(super) fn web_search_begin(chat: &mut ChatWidget<'_>, call_id: String, query
let cell = history_cell::new_running_web_search(query.clone());
chat.history_push(cell);
if let Some(last_idx) = chat.history_cells.len().checked_sub(1) {
chat.tools_state.running_web_search.insert(call_id, (last_idx, query));
chat.tools_state.running_web_search.insert(super::ToolCallId(call_id), (last_idx, query));
}
chat.bottom_pane.update_status_text("Searched".to_string());
chat.mark_needs_redraw();
}
pub(super) fn web_search_complete(chat: &mut ChatWidget<'_>, call_id: String, query: Option<String>) {
if let Some((idx, maybe_query)) = chat.tools_state.running_web_search.remove(&call_id) {
if let Some((idx, maybe_query)) = chat.tools_state.running_web_search.remove(&super::ToolCallId(call_id)) {
let mut target_idx = None;
if idx < chat.history_cells.len() {
let is_ws = chat.history_cells[idx]
@@ -63,7 +63,7 @@ pub(super) fn mcp_begin(chat: &mut ChatWidget<'_>, ev: McpToolCallBeginEvent) {
let cell = history_cell::new_running_mcp_tool_call(invocation);
chat.history_push(cell);
if let Some(last_idx) = chat.history_cells.len().checked_sub(1) {
chat.tools_state.running_custom_tools.insert(call_id, last_idx);
chat.tools_state.running_custom_tools.insert(super::ToolCallId(call_id), last_idx);
}
}
@@ -71,7 +71,7 @@ pub(super) fn mcp_end(chat: &mut ChatWidget<'_>, ev: McpToolCallEndEvent) {
let McpToolCallEndEvent { call_id, duration, invocation, result } = ev;
let success = !result.as_ref().map(|r| r.is_error.unwrap_or(false)).unwrap_or(false);
let completed = history_cell::new_completed_mcp_tool_call(80, invocation, duration, success, result);
if let Some(idx) = chat.tools_state.running_custom_tools.remove(&call_id) {
if let Some(idx) = chat.tools_state.running_custom_tools.remove(&super::ToolCallId(call_id)) {
if idx < chat.history_cells.len() {
chat.history_replace_at(idx, completed);
return;