fix: ensure the first user message always displays after the session info (#988)
Previously, if the first user message was sent with the command
invocation, e.g.:
```
$ cargo run --bin codex 'hello'
```
Then the user message was added as the first entry in the history and
then `is_first_event` would be `false` here:
031df77dfb/codex-rs/tui/src/conversation_history_widget.rs (L178-L179)
which would prevent the "welcome" message with things like the the model
version from displaying.
The fix in this PR is twofold:
* Reorganize the logic so the `ChatWidget` constructor stores
`initial_user_message` rather than sending it right away. Now inside
`handle_codex_event()`, it waits for the `SessionConfigured` event and
sends the `initial_user_message`, if it exists.
* In `conversation_history_widget.rs`, `add_session_info()` checks to
see whether a `WelcomeMessage` exists in the history when determining
the value of `has_welcome_message`. By construction, we expect that
`WelcomeMessage` is always the first message (in which case the existing
`let is_first_event = self.entries.is_empty();` logic would be sound),
but we decide to be extra defensive in case an `EventMsg::Error` is
processed before `EventMsg::SessionConfigured`.
This commit is contained in:
@@ -44,6 +44,7 @@ pub(crate) struct ChatWidget<'a> {
|
||||
bottom_pane: BottomPane<'a>,
|
||||
input_focus: InputFocus,
|
||||
config: Config,
|
||||
initial_user_message: Option<UserMessage>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Eq, PartialEq)]
|
||||
@@ -52,6 +53,28 @@ enum InputFocus {
|
||||
BottomPane,
|
||||
}
|
||||
|
||||
struct UserMessage {
|
||||
text: String,
|
||||
image_paths: Vec<PathBuf>,
|
||||
}
|
||||
|
||||
impl From<String> for UserMessage {
|
||||
fn from(text: String) -> Self {
|
||||
Self {
|
||||
text,
|
||||
image_paths: Vec::new(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn create_initial_user_message(text: String, image_paths: Vec<PathBuf>) -> Option<UserMessage> {
|
||||
if text.is_empty() && image_paths.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(UserMessage { text, image_paths })
|
||||
}
|
||||
}
|
||||
|
||||
impl ChatWidget<'_> {
|
||||
pub(crate) fn new(
|
||||
config: Config,
|
||||
@@ -93,7 +116,7 @@ impl ChatWidget<'_> {
|
||||
}
|
||||
});
|
||||
|
||||
let mut chat_widget = Self {
|
||||
Self {
|
||||
app_event_tx: app_event_tx.clone(),
|
||||
codex_op_tx,
|
||||
conversation_history: ConversationHistoryWidget::new(),
|
||||
@@ -103,14 +126,11 @@ impl ChatWidget<'_> {
|
||||
}),
|
||||
input_focus: InputFocus::BottomPane,
|
||||
config,
|
||||
};
|
||||
|
||||
if initial_prompt.is_some() || !initial_images.is_empty() {
|
||||
let text = initial_prompt.unwrap_or_default();
|
||||
chat_widget.submit_user_message_with_images(text, initial_images);
|
||||
initial_user_message: create_initial_user_message(
|
||||
initial_prompt.unwrap_or_default(),
|
||||
initial_images,
|
||||
),
|
||||
}
|
||||
|
||||
chat_widget
|
||||
}
|
||||
|
||||
pub(crate) fn handle_key_event(&mut self, key_event: KeyEvent) {
|
||||
@@ -141,19 +161,15 @@ impl ChatWidget<'_> {
|
||||
}
|
||||
InputFocus::BottomPane => match self.bottom_pane.handle_key_event(key_event) {
|
||||
InputResult::Submitted(text) => {
|
||||
self.submit_user_message(text);
|
||||
self.submit_user_message(text.into());
|
||||
}
|
||||
InputResult::None => {}
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
fn submit_user_message(&mut self, text: String) {
|
||||
// Forward to codex and update conversation history.
|
||||
self.submit_user_message_with_images(text, vec![]);
|
||||
}
|
||||
|
||||
fn submit_user_message_with_images(&mut self, text: String, image_paths: Vec<PathBuf>) {
|
||||
fn submit_user_message(&mut self, user_message: UserMessage) {
|
||||
let UserMessage { text, image_paths } = user_message;
|
||||
let mut items: Vec<InputItem> = Vec::new();
|
||||
|
||||
if !text.is_empty() {
|
||||
@@ -207,6 +223,13 @@ impl ChatWidget<'_> {
|
||||
// composer can navigate through past messages.
|
||||
self.bottom_pane
|
||||
.set_history_metadata(event.history_log_id, event.history_entry_count);
|
||||
|
||||
if let Some(user_message) = self.initial_user_message.take() {
|
||||
// If the user provided an initial message, add it to the
|
||||
// conversation history.
|
||||
self.submit_user_message(user_message);
|
||||
}
|
||||
|
||||
self.request_redraw();
|
||||
}
|
||||
EventMsg::AgentMessage(AgentMessageEvent { message }) => {
|
||||
|
||||
@@ -175,8 +175,18 @@ impl ConversationHistoryWidget {
|
||||
/// Note `model` could differ from `config.model` if the agent decided to
|
||||
/// use a different model than the one requested by the user.
|
||||
pub fn add_session_info(&mut self, config: &Config, event: SessionConfiguredEvent) {
|
||||
let is_first_event = self.entries.is_empty();
|
||||
self.add_to_history(HistoryCell::new_session_info(config, event, is_first_event));
|
||||
// In practice, SessionConfiguredEvent should always be the first entry
|
||||
// in the history, but it is possible that an error could be sent
|
||||
// before the session info.
|
||||
let has_welcome_message = self
|
||||
.entries
|
||||
.iter()
|
||||
.any(|entry| matches!(entry.cell, HistoryCell::WelcomeMessage { .. }));
|
||||
self.add_to_history(HistoryCell::new_session_info(
|
||||
config,
|
||||
event,
|
||||
!has_welcome_message,
|
||||
));
|
||||
}
|
||||
|
||||
pub fn add_user_message(&mut self, message: String) {
|
||||
|
||||
Reference in New Issue
Block a user