openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-13 11:47:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

feedback

Browse Source
This commit is contained in:
easong-openai
2025-08-04 19:35:23 -07:00
parent 0b4e0c3f13
commit 71c3bd3a79
8 changed files with 103 additions and 124 deletions
Download Patch File Download Diff File Expand all files Collapse all files

69
codex-rs/core/src/conversation_history.rs
View File

@@ -177,17 +177,16 @@ mod tests {
h.record_items([&a1, &a2]);
let items = h.contents();
assert_eq!(items.len(), 1, "adjacent assistant messages should merge");
if let ResponseItem::Message { role, content, .. } = &items[0] {
assert_eq!(role, "assistant");
let text = match &content[0] {
ContentItem::OutputText { text } => text,
_ => panic!("expected OutputText"),
};
assert_eq!(text, "Hello, world!");
} else {
panic!("expected Message");
}
assert_eq!(
items,
vec![ResponseItem::Message {
id: None,
role: "assistant".to_string(),
content: vec![ContentItem::OutputText {
text: "Hello, world!".to_string()
}]
}]
);
}
#[test]
@@ -201,17 +200,16 @@ mod tests {
h.record_items([&final_msg]);
let items = h.contents();
assert_eq!(items.len(), 1);
if let ResponseItem::Message { role, content, .. } = &items[0] {
assert_eq!(role, "assistant");
let text = match &content[0] {
ContentItem::OutputText { text } => text,
_ => panic!("expected OutputText"),
};
assert_eq!(text, "Hello, world!");
} else {
panic!("expected Message");
}
assert_eq!(
items,
vec![ResponseItem::Message {
id: None,
role: "assistant".to_string(),
content: vec![ContentItem::OutputText {
text: "Hello, world!".to_string()
}]
}]
);
}
#[test]
@@ -233,13 +231,24 @@ mod tests {
h.record_items([&u, &a]);
let items = h.contents();
assert_eq!(items.len(), 2);
match (&items[0], &items[1]) {
(ResponseItem::Message { role: r0, .. }, ResponseItem::Message { role: r1, .. }) => {
assert_eq!(r0, "user");
assert_eq!(r1, "assistant");
}
_ => panic!("expected two Message items"),
}
assert_eq!(
items,
vec![
ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![ContentItem::OutputText {
text: "hi".to_string()
}]
},
ResponseItem::Message {
id: None,
role: "assistant".to_string(),
content: vec![ContentItem::OutputText {
text: "hello".to_string()
}]
}
]
);
}
}

17
codex-rs/core/src/models.rs
View File

@@ -9,7 +9,7 @@ use serde::ser::Serializer;
use crate::protocol::InputItem;
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum ResponseInputItem {
Message {
@@ -26,7 +26,7 @@ pub enum ResponseInputItem {
},
}
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum ContentItem {
InputText { text: String },
@@ -34,7 +34,7 @@ pub enum ContentItem {
OutputText { text: String },
}
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum ResponseItem {
Message {
@@ -107,7 +107,7 @@ impl From<ResponseInputItem> for ResponseItem {
}
}
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
#[serde(rename_all = "snake_case")]
pub enum LocalShellStatus {
Completed,
@@ -115,13 +115,13 @@ pub enum LocalShellStatus {
Incomplete,
}
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum LocalShellAction {
Exec(LocalShellExecAction),
}
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
pub struct LocalShellExecAction {
pub command: Vec<String>,
pub timeout_ms: Option<u64>,
@@ -130,7 +130,7 @@ pub struct LocalShellExecAction {
pub user: Option<String>,
}
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum ReasoningItemReasoningSummary {
SummaryText { text: String },
@@ -185,10 +185,9 @@ pub struct ShellToolCallParams {
pub timeout_ms: Option<u64>,
}
#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq)]
pub struct FunctionCallOutputPayload {
pub content: String,
#[expect(dead_code)]
pub success: Option<bool>,
}

56
codex-rs/tui/src/bottom_pane/mod.rs
View File

@@ -9,6 +9,7 @@ use codex_file_search::FileMatch;
use crossterm::event::KeyEvent;
use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use ratatui::text::Line;
use ratatui::widgets::WidgetRef;
mod approval_modal_view;
@@ -30,6 +31,7 @@ pub(crate) enum CancellationEvent {
pub(crate) use chat_composer::ChatComposer;
pub(crate) use chat_composer::InputResult;
use crate::status_indicator_widget::StatusIndicatorWidget;
use approval_modal_view::ApprovalModalView;
use status_indicator_view::StatusIndicatorView;
@@ -50,7 +52,7 @@ pub(crate) struct BottomPane<'a> {
/// Optional live, multi‑line status/"live cell" rendered directly above
/// the composer while a task is running. Unlike `active_view`, this does
/// not replace the composer; it augments it.
live_status: Option<crate::status_indicator_widget::StatusIndicatorWidget>,
live_status: Option<StatusIndicatorWidget>,
/// Optional transient ring shown above the composer. This is a rendering-only
/// container used during development before we wire it to ChatWidget events.
@@ -100,24 +102,22 @@ impl BottomPane<'_> {
.map(|r| r.desired_height(width))
.unwrap_or(0);
if let Some(view) = self.active_view.as_ref() {
let view_height = if let Some(view) = self.active_view.as_ref() {
// Add a single blank spacer line between live ring and status view when active.
let spacer = if self.live_ring.is_some() && self.status_view_active {
1
} else {
0
};
overlay_status_h
.saturating_add(ring_h)
.saturating_add(spacer)
.saturating_add(view.desired_height(width))
.saturating_add(Self::BOTTOM_PAD_LINES)
spacer + view.desired_height(width)
} else {
overlay_status_h
.saturating_add(ring_h)
.saturating_add(self.composer.desired_height(width))
.saturating_add(Self::BOTTOM_PAD_LINES)
}
self.composer.desired_height(width)
};
overlay_status_h
.saturating_add(ring_h)
.saturating_add(view_height)
.saturating_add(Self::BOTTOM_PAD_LINES)
}
pub fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> {
@@ -206,10 +206,7 @@ impl BottomPane<'_> {
// present an overlay above the composer.
if !handled_by_view {
if self.live_status.is_none() {
self.live_status =
Some(crate::status_indicator_widget::StatusIndicatorWidget::new(
self.app_event_tx.clone(),
));
self.live_status = Some(StatusIndicatorWidget::new(self.app_event_tx.clone()));
}
if let Some(status) = &mut self.live_status {
status.update_text(text);
@@ -256,10 +253,8 @@ impl BottomPane<'_> {
if let Some(mut view) = self.active_view.take() {
if !view.should_hide_when_task_is_done() {
self.active_view = Some(view);
self.status_view_active = false;
} else {
self.status_view_active = false;
}
self.status_view_active = false;
}
}
}
@@ -337,11 +332,7 @@ impl BottomPane<'_> {
}
/// Set the rows and cap for the transient live ring overlay.
pub(crate) fn set_live_ring_rows(
&mut self,
max_rows: u16,
rows: Vec<ratatui::text::Line<'static>>,
) {
pub(crate) fn set_live_ring_rows(&mut self, max_rows: u16, rows: Vec<Line<'static>>) {
let mut w = live_ring_widget::LiveRingWidget::new();
w.set_max_rows(max_rows);
w.set_rows(rows);
@@ -390,7 +381,7 @@ impl WidgetRef for &BottomPane<'_> {
}
}
if let Some(ov) = &self.active_view {
if let Some(view) = &self.active_view {
if y_offset < area.height {
// Reserve bottom padding lines; keep at least 1 line for the view.
let avail = area.height - y_offset;
@@ -401,7 +392,7 @@ impl WidgetRef for &BottomPane<'_> {
width: area.width,
height: avail - pad,
};
ov.render(view_rect, buf);
view.render(view_rect, buf);
}
} else if y_offset < area.height {
let composer_rect = Rect {
@@ -485,18 +476,7 @@ mod tests {
}
lines.push(s.trim_end().to_string());
}
assert!(
lines[0].contains("two"),
"top row should be 'two': {lines:?}"
);
assert!(
lines[1].contains("three"),
"middle row should be 'three': {lines:?}"
);
assert!(
lines[2].contains("four"),
"bottom row should be 'four': {lines:?}"
);
assert_eq!(lines, vec!["two", "three", "four"]);
}
#[test]

8
codex-rs/tui/src/chatwidget.rs
View File

@@ -250,11 +250,10 @@ impl ChatWidget<'_> {
self.request_redraw();
}
EventMsg::AgentMessage(AgentMessageEvent { message }) => {
EventMsg::AgentMessage(AgentMessageEvent { message: _ }) => {
// Final assistant answer: commit all remaining rows and close with
// a blank line. Use the final text if provided, otherwise rely on
// streamed deltas already in the builder.
let _ = message; // Already streamed via deltas in most providers.
self.finalize_stream(StreamKind::Answer);
self.request_redraw();
}
@@ -272,9 +271,8 @@ impl ChatWidget<'_> {
self.stream_push_and_maybe_commit(&delta);
self.request_redraw();
}
EventMsg::AgentReasoning(AgentReasoningEvent { text }) => {
EventMsg::AgentReasoning(AgentReasoningEvent { text: _ }) => {
// Final reasoning: commit remaining rows and close with a blank.
let _ = text; // Deltas carried the content; finalize below.
self.finalize_stream(StreamKind::Reasoning);
self.request_redraw();
}
@@ -597,7 +595,7 @@ impl ChatWidget<'_> {
lines.push(ratatui::text::Line::from(r.text));
}
// Close the block with a blank line for readability.
lines.push(ratatui::text::Line::from(String::new()));
lines.push(ratatui::text::Line::from(""));
self.app_event_tx.send(AppEvent::InsertHistory(lines));
}

2
codex-rs/tui/src/history_cell.rs
View File

@@ -223,8 +223,6 @@ impl HistoryCell {
}
}
// Removed unused new_agent_message and new_agent_reasoning constructors.
pub(crate) fn new_active_exec_command(command: Vec<String>) -> Self {
let command_escaped = strip_bash_lc_and_escape(&command);

6
codex-rs/tui/src/lib.rs
View File

@@ -25,19 +25,13 @@ mod bottom_pane;
mod chatwidget;
mod citation_regex;
mod cli;
#[cfg(feature = "vt100-tests")]
pub mod custom_terminal;
#[cfg(not(feature = "vt100-tests"))]
mod custom_terminal;
mod exec_command;
mod file_search;
mod get_git_diff;
mod git_warning_screen;
mod history_cell;
#[cfg(feature = "vt100-tests")]
pub mod insert_history;
#[cfg(not(feature = "vt100-tests"))]
mod insert_history;
pub mod live_wrap;
mod log_layer;
mod markdown;

48
codex-rs/tui/src/live_wrap.rs
View File

@@ -182,15 +182,15 @@ impl RowBuilder {
}
}
/// Take a prefix of `s` whose visible width is at most `max_cols`.
/// Take a prefix of `text` whose visible width is at most `max_cols`.
/// Returns (prefix, suffix, prefix_width).
pub fn take_prefix_by_width(s: &str, max_cols: usize) -> (String, &str, usize) {
if max_cols == 0 || s.is_empty() {
return (String::new(), s, 0);
pub fn take_prefix_by_width(text: &str, max_cols: usize) -> (String, &str, usize) {
if max_cols == 0 || text.is_empty() {
return (String::new(), text, 0);
}
let mut cols = 0usize;
let mut end_idx = 0usize;
for (i, ch) in s.char_indices() {
for (i, ch) in text.char_indices() {
let ch_width = UnicodeWidthChar::width(ch).unwrap_or(0);
if cols.saturating_add(ch_width) > max_cols {
break;
@@ -201,24 +201,34 @@ pub fn take_prefix_by_width(s: &str, max_cols: usize) -> (String, &str, usize) {
break;
}
}
let prefix = s[..end_idx].to_string();
let suffix = &s[end_idx..];
let prefix = text[..end_idx].to_string();
let suffix = &text[end_idx..];
(prefix, suffix, cols)
}
#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
#[test]
fn rows_do_not_exceed_width_ascii() {
let mut rb = RowBuilder::new(10);
rb.push_fragment("hello world this is a test");
let rows = rb.rows();
assert!(!rows.is_empty());
for r in rows {
assert!(r.width() <= 10, "row exceeds width: {r:?}");
}
let rows = rb.rows().to_vec();
assert_eq!(
rows,
vec![
Row {
text: "hello worl".to_string(),
explicit_break: false
},
Row {
text: "d this is ".to_string(),
explicit_break: false
}
]
);
}
#[test]
@@ -226,9 +236,17 @@ mod tests {
// 😀 is width 2; 你/好 are width 2.
let mut rb = RowBuilder::new(6);
rb.push_fragment("😀😀 你好");
for r in rb.rows() {
assert!(r.width() <= 6, "row exceeds width: {r:?}");
}
let rows = rb.rows().to_vec();
// At width 6, we expect the first row to fit exactly two emojis and a space
// (2 + 2 + 1 = 5) plus one more column for the first CJK char (2 would overflow),
// so only the two emojis and the space fit; the rest remains buffered.
assert_eq!(
rows,
vec![Row {
text: "😀😀 ".to_string(),
explicit_break: false
}]
);
}
#[test]

21
codex-rs/tui/tests/vt100_history.rs
View File

@@ -137,25 +137,8 @@ fn hist_003_emoji_and_cjk() {
let text = String::from("😀😀😀😀😀 你好世界");
let lines = vec![Line::from(text.clone())];
let buf = scenario.run_insert(lines);
let mut parser = vt100::Parser::new(6, 20, 0);
parser.process(&buf);
let screen = parser.screen();
// Reconstruct string by concatenating non-space cells; ensure all emojis and CJK are present.
let mut reconstructed = String::new();
for row in 0..6 {
for col in 0..20 {
if let Some(cell) = screen.cell(row, col) {
let cont = cell.contents();
if let Some(ch) = cont.chars().next() {
if ch != ' ' {
reconstructed.push(ch);
}
}
}
}
}
let rows = scenario.screen_rows_from_bytes(&buf);
let reconstructed: String = rows.join("").chars().filter(|c| *c != ' ').collect();
for ch in text.chars().filter(|c| !c.is_whitespace()) {
assert!(
reconstructed.contains(ch),