fix(tui): clarify Codex response errors
This commit is contained in:
@@ -134,6 +134,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
- **Interrupted sub-agent lifecycle (#3080).** API-timeout interruptions now
|
||||
emit `MailboxMessage::Interrupted`, render terminal interrupted cards, and
|
||||
reconcile stale running fanout counts from manager snapshots.
|
||||
- **OpenAI Codex stream diagnostics and active tool collapse (#3146).** The
|
||||
Responses bridge now reports nested `response.failed` /
|
||||
`response.incomplete` errors instead of `unknown`, and dense successful
|
||||
in-flight tool bursts collapse into the same calm activity metadata row as
|
||||
committed history.
|
||||
- **OpenAI Codex reasoning tiers.** Switching from DeepSeek to `openai-codex`
|
||||
now normalizes stale reasoning state into Responses-compatible
|
||||
`low`/`medium`/`high`/`xhigh` tiers. Startup, `/config`, and the model
|
||||
|
||||
@@ -134,6 +134,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
- **Interrupted sub-agent lifecycle (#3080).** API-timeout interruptions now
|
||||
emit `MailboxMessage::Interrupted`, render terminal interrupted cards, and
|
||||
reconcile stale running fanout counts from manager snapshots.
|
||||
- **OpenAI Codex stream diagnostics and active tool collapse (#3146).** The
|
||||
Responses bridge now reports nested `response.failed` /
|
||||
`response.incomplete` errors instead of `unknown`, and dense successful
|
||||
in-flight tool bursts collapse into the same calm activity metadata row as
|
||||
committed history.
|
||||
- **OpenAI Codex reasoning tiers.** Switching from DeepSeek to `openai-codex`
|
||||
now normalizes stale reasoning state into Responses-compatible
|
||||
`low`/`medium`/`high`/`xhigh` tiers. Startup, `/config`, and the model
|
||||
|
||||
@@ -328,15 +328,8 @@ impl DeepSeekClient {
|
||||
});
|
||||
}
|
||||
}
|
||||
"error" => {
|
||||
let msg = event
|
||||
.get("message")
|
||||
.and_then(|m| m.as_str())
|
||||
.unwrap_or("Unknown error");
|
||||
let code = event
|
||||
.get("code")
|
||||
.and_then(|c| c.as_str())
|
||||
.unwrap_or("unknown");
|
||||
"error" | "response.failed" | "response.incomplete" => {
|
||||
let (code, msg) = responses_event_error_details(&event);
|
||||
yield Err(anyhow::anyhow!(
|
||||
"Responses API error [{code}]: {msg}"
|
||||
));
|
||||
@@ -621,6 +614,52 @@ fn codex_responses_reasoning_effort(raw: &str) -> Option<&'static str> {
|
||||
}
|
||||
}
|
||||
|
||||
fn responses_event_error_details(event: &Value) -> (String, String) {
|
||||
let event_type = string_at(event, "/type").unwrap_or("error");
|
||||
let code = first_string_at(
|
||||
event,
|
||||
&[
|
||||
"/code",
|
||||
"/error/code",
|
||||
"/response/error/code",
|
||||
"/response/incomplete_details/reason",
|
||||
"/response/status",
|
||||
],
|
||||
)
|
||||
.unwrap_or("unknown");
|
||||
let message = first_string_at(
|
||||
event,
|
||||
&[
|
||||
"/message",
|
||||
"/error/message",
|
||||
"/response/error/message",
|
||||
"/response/incomplete_details/reason",
|
||||
],
|
||||
)
|
||||
.map_or_else(
|
||||
|| format!("{event_type} event received"),
|
||||
|message| {
|
||||
if message == code && event_type == "response.incomplete" {
|
||||
format!("response incomplete: {message}")
|
||||
} else {
|
||||
message.to_string()
|
||||
}
|
||||
},
|
||||
);
|
||||
(code.to_string(), message)
|
||||
}
|
||||
|
||||
fn first_string_at<'a>(value: &'a Value, paths: &[&str]) -> Option<&'a str> {
|
||||
paths.iter().find_map(|path| string_at(value, path))
|
||||
}
|
||||
|
||||
fn string_at<'a>(value: &'a Value, path: &str) -> Option<&'a str> {
|
||||
value.pointer(path).and_then(Value::as_str).and_then(|s| {
|
||||
let trimmed = s.trim();
|
||||
(!trimmed.is_empty()).then_some(trimmed)
|
||||
})
|
||||
}
|
||||
|
||||
/// Parse a composite tool_use_id back to (call_id, item_id).
|
||||
/// Composite format: "call_id|item_id"
|
||||
fn parse_tool_use_id(id: &str) -> (String, String) {
|
||||
@@ -712,6 +751,45 @@ mod tests {
|
||||
assert!(body.get("reasoning_effort").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn responses_failed_event_reports_nested_error() {
|
||||
let event = json!({
|
||||
"type": "response.failed",
|
||||
"response": {
|
||||
"id": "resp_123",
|
||||
"error": {
|
||||
"code": "rate_limit_exceeded",
|
||||
"message": "Please retry later"
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
let (code, message) = responses_event_error_details(&event);
|
||||
|
||||
assert_eq!(code, "rate_limit_exceeded");
|
||||
assert_eq!(message, "Please retry later");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn responses_incomplete_event_reports_reason() {
|
||||
let event = json!({
|
||||
"type": "response.incomplete",
|
||||
"response": {
|
||||
"id": "resp_123",
|
||||
"status": "incomplete",
|
||||
"error": null,
|
||||
"incomplete_details": {
|
||||
"reason": "content_filter"
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
let (code, message) = responses_event_error_details(&event);
|
||||
|
||||
assert_eq!(code, "content_filter");
|
||||
assert_eq!(message, "response incomplete: content_filter");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn responses_input_includes_user_role_tool_results() {
|
||||
let request = MessageRequest {
|
||||
|
||||
@@ -829,17 +829,34 @@ impl ToolRunActivitySummary {
|
||||
|
||||
/// Detect contiguous runs of successful, low-risk tool cells.
|
||||
///
|
||||
/// Failed, running, shell, patch, review, diff, and plan-update cells split
|
||||
/// runs so important state never disappears into a summary row.
|
||||
/// Failed, running, patch, review, diff, and plan-update cells split runs so
|
||||
/// important state never disappears into a summary row. Successful command
|
||||
/// cells can join dense runs; Alt+V / expansion keeps their raw details
|
||||
/// available without making routine verifier/shell work dominate the default
|
||||
/// transcript.
|
||||
pub fn detect_tool_runs(history: &[HistoryCell], min_size: usize) -> Vec<ToolRun> {
|
||||
detect_tool_runs_from_slices(history, &[], min_size)
|
||||
}
|
||||
|
||||
/// Detect contiguous runs across committed history plus the active in-flight
|
||||
/// tail. `ToolRun::start` is always the virtual transcript index:
|
||||
/// `history.len() + active_offset` for active entries.
|
||||
pub fn detect_tool_runs_from_slices(
|
||||
history: &[HistoryCell],
|
||||
active_entries: &[HistoryCell],
|
||||
min_size: usize,
|
||||
) -> Vec<ToolRun> {
|
||||
if min_size == 0 {
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
let mut runs = Vec::new();
|
||||
let mut index = 0;
|
||||
while index < history.len() {
|
||||
if !is_collapsible_tool_cell(&history[index]) {
|
||||
let total_len = history.len().saturating_add(active_entries.len());
|
||||
while index < total_len {
|
||||
if !cell_at_virtual_index(history, active_entries, index)
|
||||
.is_some_and(is_collapsible_tool_cell)
|
||||
{
|
||||
index += 1;
|
||||
continue;
|
||||
}
|
||||
@@ -847,8 +864,13 @@ pub fn detect_tool_runs(history: &[HistoryCell], min_size: usize) -> Vec<ToolRun
|
||||
let start = index;
|
||||
let mut names: Vec<String> = Vec::new();
|
||||
let mut activity = ToolRunActivitySummary::default();
|
||||
while index < history.len() && is_collapsible_tool_cell(&history[index]) {
|
||||
if let HistoryCell::Tool(tool) = &history[index] {
|
||||
while index < total_len
|
||||
&& cell_at_virtual_index(history, active_entries, index)
|
||||
.is_some_and(is_collapsible_tool_cell)
|
||||
{
|
||||
if let Some(HistoryCell::Tool(tool)) =
|
||||
cell_at_virtual_index(history, active_entries, index)
|
||||
{
|
||||
let name = tool_display_name(tool);
|
||||
if !names.iter().any(|existing| existing == name) {
|
||||
names.push(name.to_string());
|
||||
@@ -873,12 +895,26 @@ pub fn detect_tool_runs(history: &[HistoryCell], min_size: usize) -> Vec<ToolRun
|
||||
runs
|
||||
}
|
||||
|
||||
fn cell_at_virtual_index<'a>(
|
||||
history: &'a [HistoryCell],
|
||||
active_entries: &'a [HistoryCell],
|
||||
index: usize,
|
||||
) -> Option<&'a HistoryCell> {
|
||||
history
|
||||
.get(index)
|
||||
.or_else(|| active_entries.get(index.checked_sub(history.len())?))
|
||||
}
|
||||
|
||||
fn is_collapsible_tool_cell(cell: &HistoryCell) -> bool {
|
||||
matches!(cell, HistoryCell::Tool(tool) if tool.is_success() && !tool.is_collapsible_guard())
|
||||
}
|
||||
|
||||
fn generic_tool_name_is_collapse_guard(name: &str) -> bool {
|
||||
let normalized = name.trim().to_ascii_lowercase();
|
||||
if is_metadata_tool_name(&normalized) {
|
||||
return false;
|
||||
}
|
||||
|
||||
normalized.contains("patch")
|
||||
|| normalized.contains("write")
|
||||
|| normalized.contains("edit")
|
||||
@@ -886,11 +922,23 @@ fn generic_tool_name_is_collapse_guard(name: &str) -> bool {
|
||||
|| normalized.contains("remove")
|
||||
|| normalized.contains("commit")
|
||||
|| normalized.contains("push")
|
||||
|| normalized.contains("shell")
|
||||
|| normalized.contains("exec")
|
||||
|| normalized.contains("review")
|
||||
}
|
||||
|
||||
fn is_metadata_tool_name(name: &str) -> bool {
|
||||
matches!(
|
||||
name,
|
||||
"update_plan"
|
||||
| "todo_write"
|
||||
| "todo_add"
|
||||
| "todo_update"
|
||||
| "checklist_write"
|
||||
| "checklist_add"
|
||||
| "checklist_update"
|
||||
| "checklist_list"
|
||||
)
|
||||
}
|
||||
|
||||
fn tool_display_name(tool: &ToolCell) -> &str {
|
||||
match tool {
|
||||
ToolCell::Generic(cell) => cell.name.as_str(),
|
||||
@@ -930,8 +978,7 @@ fn classify_tool_name_activity(name: &str) -> ToolRunActivity {
|
||||
"edit_file" | "apply_patch" | "write_file" | "diff" => ToolRunActivity::Edit,
|
||||
"agent_open" | "agent_eval" | "agent_close" | "agent_spawn" | "tool_agent" | "rlm_open"
|
||||
| "rlm_eval" | "rlm_configure" | "rlm_close" | "rlm" => ToolRunActivity::Delegate,
|
||||
"update_plan" | "todo_write" | "todo_add" | "todo_update" | "checklist_write"
|
||||
| "checklist_add" | "checklist_update" => ToolRunActivity::Metadata,
|
||||
_ if is_metadata_tool_name(&normalized) => ToolRunActivity::Metadata,
|
||||
_ if normalized.contains("search")
|
||||
|| normalized.contains("grep")
|
||||
|| normalized.contains("find") =>
|
||||
|
||||
@@ -131,7 +131,11 @@ impl ChatWidget {
|
||||
|
||||
let history_len = app.history.len();
|
||||
let tool_runs = if app.tool_collapse_active() {
|
||||
crate::tui::history::detect_tool_runs(&app.history, app.tool_collapse_threshold)
|
||||
crate::tui::history::detect_tool_runs_from_slices(
|
||||
&app.history,
|
||||
active_entries,
|
||||
app.tool_collapse_threshold,
|
||||
)
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
@@ -201,7 +205,12 @@ impl ChatWidget {
|
||||
.find(|run| run.start == idx && collapsed_run_starts.contains(&idx))
|
||||
{
|
||||
filtered_cells.push(tool_run_summary_cell(run));
|
||||
filtered_revs.push(tool_run_summary_revision(run, &app.history_revisions));
|
||||
filtered_revs.push(tool_run_summary_revision(
|
||||
run,
|
||||
&app.history_revisions,
|
||||
history_len,
|
||||
app.active_cell_revision,
|
||||
));
|
||||
filtered_to_original.push(idx);
|
||||
continue;
|
||||
}
|
||||
@@ -217,13 +226,25 @@ impl ChatWidget {
|
||||
if app.collapsed_cells.contains(&original_idx) {
|
||||
continue;
|
||||
}
|
||||
if collapsed_tool_indices.contains(&original_idx) {
|
||||
continue;
|
||||
}
|
||||
if let Some(run) = tool_runs.iter().find(|run| {
|
||||
run.start == original_idx && collapsed_run_starts.contains(&original_idx)
|
||||
}) {
|
||||
filtered_cells.push(tool_run_summary_cell(run));
|
||||
filtered_revs.push(tool_run_summary_revision(
|
||||
run,
|
||||
&app.history_revisions,
|
||||
history_len,
|
||||
active_rev,
|
||||
));
|
||||
filtered_to_original.push(original_idx);
|
||||
continue;
|
||||
}
|
||||
filtered_cells.push(cell.clone());
|
||||
let salt = (i as u64).wrapping_add(1);
|
||||
filtered_revs.push(
|
||||
active_rev
|
||||
.wrapping_mul(0x9E37_79B9_7F4A_7C15)
|
||||
.wrapping_add(salt),
|
||||
);
|
||||
filtered_revs.push(active_entry_revision(active_rev, salt));
|
||||
filtered_to_original.push(original_idx);
|
||||
}
|
||||
}
|
||||
@@ -379,14 +400,29 @@ fn tool_run_summary_cell(run: &ToolRun) -> HistoryCell {
|
||||
}))
|
||||
}
|
||||
|
||||
fn tool_run_summary_revision(run: &ToolRun, revisions: &[u64]) -> u64 {
|
||||
fn tool_run_summary_revision(
|
||||
run: &ToolRun,
|
||||
revisions: &[u64],
|
||||
history_len: usize,
|
||||
active_rev: u64,
|
||||
) -> u64 {
|
||||
let mut revision = 0xA11C_EA5E_D00D_2692u64 ^ ((run.start as u64) << 32) ^ (run.count as u64);
|
||||
for idx in run.start..run.start.saturating_add(run.count) {
|
||||
revision = revision.rotate_left(7) ^ revisions.get(idx).copied().unwrap_or(u64::MAX);
|
||||
let cell_revision = revisions.get(idx).copied().unwrap_or_else(|| {
|
||||
let active_idx = idx.saturating_sub(history_len);
|
||||
active_entry_revision(active_rev, (active_idx as u64).wrapping_add(1))
|
||||
});
|
||||
revision = revision.rotate_left(7) ^ cell_revision;
|
||||
}
|
||||
revision
|
||||
}
|
||||
|
||||
fn active_entry_revision(active_rev: u64, salt: u64) -> u64 {
|
||||
active_rev
|
||||
.wrapping_mul(0x9E37_79B9_7F4A_7C15)
|
||||
.wrapping_add(salt)
|
||||
}
|
||||
|
||||
impl Renderable for ChatWidget {
|
||||
fn render(&self, _area: Rect, buf: &mut Buffer) {
|
||||
// Use the passed render area, not self.content_area — those can
|
||||
@@ -2083,7 +2119,15 @@ fn composer_top_right_chrome(app: &App, area_width: u16) -> Option<Line<'static>
|
||||
}
|
||||
|
||||
fn should_render_empty_state(app: &App) -> bool {
|
||||
app.history.is_empty() && !app.is_loading && !app.is_compacting && !app.is_purging
|
||||
let active_is_empty = app
|
||||
.active_cell
|
||||
.as_ref()
|
||||
.map_or(true, crate::tui::active_cell::ActiveCell::is_empty);
|
||||
app.history.is_empty()
|
||||
&& active_is_empty
|
||||
&& !app.is_loading
|
||||
&& !app.is_compacting
|
||||
&& !app.is_purging
|
||||
}
|
||||
|
||||
fn build_empty_state_lines(app: &App, area: Rect) -> Vec<Line<'static>> {
|
||||
@@ -2740,6 +2784,7 @@ mod tests {
|
||||
use crate::config::{ApiProvider, Config};
|
||||
use crate::localization::Locale;
|
||||
use crate::palette;
|
||||
use crate::tui::active_cell::ActiveCell;
|
||||
use crate::tui::app::{App, ComposerDensity, ToolCollapseMode, TuiOptions};
|
||||
use crate::tui::history::{GenericToolCell, HistoryCell, ToolCell, ToolStatus};
|
||||
use crate::tui::scrolling::{TranscriptLineMeta, TranscriptScroll};
|
||||
@@ -2876,6 +2921,40 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn chat_widget_collapses_dense_active_tool_runs_by_default() {
|
||||
let mut app = create_test_app();
|
||||
app.tool_collapse_mode = ToolCollapseMode::Compact;
|
||||
app.tool_collapse_threshold = 3;
|
||||
let active = app.active_cell.get_or_insert_with(ActiveCell::new);
|
||||
active.push_untracked(success_tool_cell("read_file"));
|
||||
active.push_untracked(success_tool_cell("list_dir"));
|
||||
active.push_untracked(success_tool_cell("web_search"));
|
||||
app.bump_active_cell_revision();
|
||||
|
||||
let area = Rect {
|
||||
x: 0,
|
||||
y: 0,
|
||||
width: 80,
|
||||
height: 8,
|
||||
};
|
||||
let mut buf = Buffer::empty(area);
|
||||
let widget = ChatWidget::new(&mut app, area);
|
||||
widget.render(area, &mut buf);
|
||||
let rendered = buffer_text(&buf, area);
|
||||
|
||||
assert_eq!(app.collapsed_cell_map, vec![0]);
|
||||
assert!(
|
||||
rendered.contains("Explored 2 files, 1 search"),
|
||||
"{rendered}"
|
||||
);
|
||||
assert!(!rendered.contains("activity_group"), "{rendered}");
|
||||
assert!(
|
||||
!rendered.contains("full output from list_dir"),
|
||||
"{rendered}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn chat_widget_expands_dense_tool_runs_on_demand() {
|
||||
let mut app = create_test_app();
|
||||
|
||||
Reference in New Issue
Block a user