chore(tui): polish maintainability

- Remove legacy string-based  (use ExecAction everywhere)
- Add ergonomic traits for newtype IDs (From<&str>, From<String>, AsRef<str>, Display)
- Replace remaining direct streaming mutations with streaming facade
- Update refactor docs: Step 3 and 4 marked DONE

Build: ./build-fast.sh clean
This commit is contained in:
James Peter
2025-09-02 21:14:34 +10:00
parent b7c660283e
commit 5f59faaacd
4 changed files with 30 additions and 26 deletions
+15
View File
@@ -272,6 +272,21 @@ pub(super) struct ToolCallId(pub String);
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub(super) struct StreamId(pub String);
impl From<String> for ExecCallId { fn from(s: String) -> Self { ExecCallId(s) } }
impl From<&str> for ExecCallId { fn from(s: &str) -> Self { ExecCallId(s.to_string()) } }
impl std::fmt::Display for ExecCallId { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_str(&self.0) } }
impl AsRef<str> for ExecCallId { fn as_ref(&self) -> &str { &self.0 } }
impl From<String> for ToolCallId { fn from(s: String) -> Self { ToolCallId(s) } }
impl From<&str> for ToolCallId { fn from(s: &str) -> Self { ToolCallId(s.to_string()) } }
impl std::fmt::Display for ToolCallId { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_str(&self.0) } }
impl AsRef<str> for ToolCallId { fn as_ref(&self) -> &str { &self.0 } }
impl From<String> for StreamId { fn from(s: String) -> Self { StreamId(s) } }
impl From<&str> for StreamId { fn from(s: &str) -> Self { StreamId(s.to_string()) } }
impl std::fmt::Display for StreamId { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_str(&self.0) } }
impl AsRef<str> for StreamId { fn as_ref(&self) -> &str { &self.0 } }
impl ChatWidget<'_> {
/// Hide the bottom spinner/status if the UI is idle (no streams, tools, agents, or tasks).
fn maybe_hide_spinner(&mut self) {
+5 -5
View File
@@ -226,11 +226,11 @@ pub(super) fn try_merge_completed_exec_at(chat: &mut ChatWidget<'_>, idx: usize)
return;
}
let to_kind = |e: &history_cell::ExecCell| -> history_cell::ExecKind {
match history_cell::action_from_parsed(&e.parsed) {
"read" => history_cell::ExecKind::Read,
"search" => history_cell::ExecKind::Search,
"list" => history_cell::ExecKind::List,
_ => history_cell::ExecKind::Run,
match history_cell::action_enum_from_parsed(&e.parsed) {
history_cell::ExecAction::Read => history_cell::ExecKind::Read,
history_cell::ExecAction::Search => history_cell::ExecKind::Search,
history_cell::ExecAction::List => history_cell::ExecKind::List,
history_cell::ExecAction::Run => history_cell::ExecKind::Run,
}
};
+1 -12
View File
@@ -4256,18 +4256,7 @@ fn exec_command_lines(
}
}
pub(crate) fn action_from_parsed(parsed_commands: &[ParsedCommand]) -> &'static str {
for parsed in parsed_commands.iter() {
match parsed {
ParsedCommand::Search { .. } => return "search",
ParsedCommand::Read { .. } => return "read",
ParsedCommand::ListFiles { .. } => return "list",
ParsedCommand::Noop { .. } => continue,
_ => return "run",
}
}
"run"
}
// Legacy helper removed in favor of ExecAction (action_enum_from_parsed)
fn first_context_path(parsed_commands: &[ParsedCommand]) -> Option<String> {
for parsed in parsed_commands.iter() {
+9 -9
View File
@@ -83,14 +83,14 @@ Build + policy:
- Added `history_push_and_maybe_merge`, `history_replace_and_maybe_merge`, and `history_maybe_merge_tool_with_previous` on `ChatWidget`.
- Adopted across exec completion and web search completion paths; explicit merge code removed from call sites.
3) Enum-ize exec actions and newtype IDs — IN PROGRESS
- Added `ExecAction` enum + `action_enum_from_parsed()`; switched `exec_tools.rs` to use it.
- Next: migrate remaining string-based action checks in `history_cell.rs` and `chatwidget.rs` to the enum.
- Newtypes for IDs still TODO.
3) Enum-ize exec actions and newtype IDs — DONE
- Added `ExecAction` enum + `action_enum_from_parsed()` and adopted across `history_cell`, `exec_tools`, and `chatwidget`.
- Introduced ID newtypes: `ExecCallId`, `ToolCallId`, `StreamId`; migrated state maps/sets and call sites.
- Removed legacy `action_from_parsed` helper.
4) Streaming API cleanup
- In `streaming.rs`, expose: `begin(kind,id)`, `delta(text)`, `finalize(kind, follow_bottom: bool)`.
- Remove remaining external touches of `stream_state.current_kind` (already mostly contained) and push that responsibility into the streaming module.
4) Streaming API cleanup — DONE
- In `streaming.rs`, added `begin(kind,id)`, `delta_text(kind,id,text)`, `finalize(kind, follow_bottom)` facades.
- Replaced direct `current_kind` mutations at call sites with the facade; kept internal control inside streaming module.
5) Tests (valuable quick wins)
- Unit tests
@@ -145,8 +145,8 @@ Build + policy:
- [x] Sweep remaining history mutations to use the shim
- [x] Consolidate history merge API (adopt helpers across call sites)
- [ ] Replace all action string checks with ExecAction enum
- [ ] Add ID newtypes (Exec/Tool/Stream) and migrate maps
- [x] Replace all action string checks with ExecAction enum
- [x] Add ID newtypes (Exec/Tool/Stream) and migrate maps
- [ ] Replace action strings with `ExecAction` enum
- [ ] Add ID newtypes (Exec/Tool/Stream)
- [ ] Tighten streaming API (begin/delta/finalize facade)