Improve feedback (#5661)

<img width="1099" height="153" alt="image"
src="https://github.com/user-attachments/assets/2c901884-8baf-4b1b-b2c4-bcb61ff42be8"
/>

<img width="1082" height="125" alt="image"
src="https://github.com/user-attachments/assets/6336e6c9-9ace-46df-a383-a807ceffa524"
/>

<img width="1102" height="103" alt="image"
src="https://github.com/user-attachments/assets/78883682-7e44-4fa3-9e04-57f7df4766fd"
/>
This commit is contained in:
Ahmed Ibrahim
2025-10-24 22:28:14 -07:00
committed by GitHub
parent 0533bd2e7c
commit 71f838389b
15 changed files with 758 additions and 189 deletions

View File

@@ -55,7 +55,7 @@ pub enum SandboxErr {
#[derive(Error, Debug)] #[derive(Error, Debug)]
pub enum CodexErr { pub enum CodexErr {
// todo(aibrahim): git rid of this error carrying the dangling artifacts // todo(aibrahim): git rid of this error carrying the dangling artifacts
#[error("turn aborted")] #[error("turn aborted. Something went wrong? Hit `/feedback` to report the issue.")]
TurnAborted { TurnAborted {
dangling_artifacts: Vec<ProcessedResponseItem>, dangling_artifacts: Vec<ProcessedResponseItem>,
}, },
@@ -91,7 +91,7 @@ pub enum CodexErr {
/// Returned by run_command_stream when the user pressed CtrlC (SIGINT). Session uses this to /// Returned by run_command_stream when the user pressed CtrlC (SIGINT). Session uses this to
/// surface a polite FunctionCallOutput back to the model instead of crashing the CLI. /// surface a polite FunctionCallOutput back to the model instead of crashing the CLI.
#[error("interrupted (Ctrl-C)")] #[error("interrupted (Ctrl-C). Something went wrong? Hit `/feedback` to report the issue.")]
Interrupted, Interrupted,
/// Unexpected HTTP status code. /// Unexpected HTTP status code.

View File

@@ -167,8 +167,18 @@ impl CodexLogSnapshot {
Ok(path) Ok(path)
} }
pub fn upload_to_sentry(&self) -> Result<()> { /// Uploads feedback to Sentry with both the in-memory Codex logs and an optional
/// rollout file attached. Also records metadata such as classification,
/// reason (free-form note), and CLI version as Sentry tags or message.
pub fn upload_feedback_with_rollout(
&self,
classification: &str,
reason: Option<&str>,
cli_version: &str,
rollout_path: Option<&std::path::Path>,
) -> Result<()> {
use std::collections::BTreeMap; use std::collections::BTreeMap;
use std::fs;
use std::str::FromStr; use std::str::FromStr;
use std::sync::Arc; use std::sync::Arc;
@@ -182,22 +192,47 @@ impl CodexLogSnapshot {
use sentry::transports::DefaultTransportFactory; use sentry::transports::DefaultTransportFactory;
use sentry::types::Dsn; use sentry::types::Dsn;
// Build Sentry client
let client = Client::from_config(ClientOptions { let client = Client::from_config(ClientOptions {
dsn: Some(Dsn::from_str(SENTRY_DSN).map_err(|e| anyhow!("invalid DSN: {}", e))?), dsn: Some(Dsn::from_str(SENTRY_DSN).map_err(|e| anyhow!("invalid DSN: {}", e))?),
transport: Some(Arc::new(DefaultTransportFactory {})), transport: Some(Arc::new(DefaultTransportFactory {})),
..Default::default() ..Default::default()
}); });
let tags = BTreeMap::from([(String::from("thread_id"), self.thread_id.to_string())]); // Tags: thread id, classification, cli_version
let mut tags = BTreeMap::from([
(String::from("thread_id"), self.thread_id.to_string()),
(String::from("classification"), classification.to_string()),
(String::from("cli_version"), cli_version.to_string()),
]);
// Reason (freeform) include entire note as a tag; keep title in message.
if let Some(r) = reason {
tags.insert(String::from("reason"), r.to_string());
}
// Elevate level for error-like classifications
let level = match classification {
"bug" | "bad_result" => Level::Error,
_ => Level::Info,
};
let mut envelope = Envelope::new();
// Title is the message in Sentry: "[Classification]: Codex session <thread_id>"
let title = format!(
"[{}]: Codex session {}",
display_classification(classification),
self.thread_id
);
let event = Event { let event = Event {
level: Level::Error, level,
message: Some("Codex Log Upload ".to_string() + &self.thread_id), message: Some(title),
tags, tags,
..Default::default() ..Default::default()
}; };
let mut envelope = Envelope::new();
envelope.add_item(EnvelopeItem::Event(event)); envelope.add_item(EnvelopeItem::Event(event));
// Attachment 1: Codex logs snapshot
envelope.add_item(EnvelopeItem::Attachment(Attachment { envelope.add_item(EnvelopeItem::Attachment(Attachment {
buffer: self.bytes.clone(), buffer: self.bytes.clone(),
filename: String::from("codex-logs.log"), filename: String::from("codex-logs.log"),
@@ -205,11 +240,96 @@ impl CodexLogSnapshot {
ty: None, ty: None,
})); }));
// Attachment 2: rollout file (if provided and readable)
if let Some((path, data)) = rollout_path.and_then(|p| fs::read(p).ok().map(|d| (p, d))) {
// Name the file by suffix so users can spot it.
let fname = path
.file_name()
.map(|s| s.to_string_lossy().to_string())
.unwrap_or_else(|| "rollout.jsonl".to_string());
envelope.add_item(EnvelopeItem::Attachment(Attachment {
buffer: data,
filename: fname,
content_type: Some("application/jsonl".to_string()),
ty: None,
}));
}
client.send_envelope(envelope); client.send_envelope(envelope);
client.flush(Some(Duration::from_secs(UPLOAD_TIMEOUT_SECS))); client.flush(Some(Duration::from_secs(UPLOAD_TIMEOUT_SECS)));
Ok(()) Ok(())
} }
/// Upload a metadata-only feedback event (no attachments). Includes classification,
/// optional reason, CLI version and thread ID as tags.
pub fn upload_feedback_metadata_only(
&self,
classification: &str,
reason: Option<&str>,
cli_version: &str,
) -> Result<()> {
use std::collections::BTreeMap;
use std::str::FromStr;
use std::sync::Arc;
use sentry::Client;
use sentry::ClientOptions;
use sentry::protocol::Envelope;
use sentry::protocol::EnvelopeItem;
use sentry::protocol::Event;
use sentry::protocol::Level;
use sentry::transports::DefaultTransportFactory;
use sentry::types::Dsn;
let client = Client::from_config(ClientOptions {
dsn: Some(Dsn::from_str(SENTRY_DSN).map_err(|e| anyhow!("invalid DSN: {}", e))?),
transport: Some(Arc::new(DefaultTransportFactory {})),
..Default::default()
});
let mut tags = BTreeMap::from([
(String::from("thread_id"), self.thread_id.to_string()),
(String::from("classification"), classification.to_string()),
(String::from("cli_version"), cli_version.to_string()),
]);
if let Some(r) = reason {
tags.insert(String::from("reason"), r.to_string());
}
let level = match classification {
"bug" | "bad_result" => Level::Error,
_ => Level::Info,
};
let mut envelope = Envelope::new();
// Title is the message in Sentry: "[Classification]: Codex session <thread_id>"
let title = format!(
"[{}]: Codex session {}",
display_classification(classification),
self.thread_id
);
let event = Event {
level,
message: Some(title),
tags,
..Default::default()
};
envelope.add_item(EnvelopeItem::Event(event));
client.send_envelope(envelope);
client.flush(Some(Duration::from_secs(UPLOAD_TIMEOUT_SECS)));
Ok(())
}
}
fn display_classification(classification: &str) -> String {
match classification {
"bug" => "Bug".to_string(),
"bad_result" => "Bad result".to_string(),
"good_result" => "Good result".to_string(),
_ => "Other".to_string(),
}
} }
#[cfg(test)] #[cfg(test)]

View File

@@ -360,6 +360,15 @@ impl App {
AppEvent::OpenFullAccessConfirmation { preset } => { AppEvent::OpenFullAccessConfirmation { preset } => {
self.chat_widget.open_full_access_confirmation(preset); self.chat_widget.open_full_access_confirmation(preset);
} }
AppEvent::OpenFeedbackNote {
category,
include_logs,
} => {
self.chat_widget.open_feedback_note(category, include_logs);
}
AppEvent::OpenFeedbackConsent { category } => {
self.chat_widget.open_feedback_consent(category);
}
AppEvent::PersistModelSelection { model, effort } => { AppEvent::PersistModelSelection { model, effort } => {
let profile = self.active_profile.as_deref(); let profile = self.active_profile.as_deref();
match persist_model_selection(&self.config.codex_home, profile, &model, effort) match persist_model_selection(&self.config.codex_home, profile, &model, effort)

View File

@@ -101,4 +101,23 @@ pub(crate) enum AppEvent {
/// Open the approval popup. /// Open the approval popup.
FullScreenApprovalRequest(ApprovalRequest), FullScreenApprovalRequest(ApprovalRequest),
/// Open the feedback note entry overlay after the user selects a category.
OpenFeedbackNote {
category: FeedbackCategory,
include_logs: bool,
},
/// Open the upload consent popup for feedback after selecting a category.
OpenFeedbackConsent {
category: FeedbackCategory,
},
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum FeedbackCategory {
BadResult,
GoodResult,
Bug,
Other,
} }

View File

@@ -1,165 +1,432 @@
use crate::app_event::AppEvent; use std::cell::RefCell;
use crate::app_event_sender::AppEventSender; use std::path::PathBuf;
use crate::history_cell;
use crate::history_cell::PlainHistoryCell; use crossterm::event::KeyCode;
use crate::render::renderable::Renderable; use crossterm::event::KeyEvent;
use crossterm::event::KeyModifiers;
use ratatui::buffer::Buffer; use ratatui::buffer::Buffer;
use ratatui::layout::Rect; use ratatui::layout::Rect;
use ratatui::style::Stylize; use ratatui::style::Stylize;
use ratatui::text::Line; use ratatui::text::Line;
use std::path::PathBuf; use ratatui::text::Span;
use ratatui::widgets::Clear;
use ratatui::widgets::Paragraph;
use ratatui::widgets::StatefulWidgetRef;
use ratatui::widgets::Widget;
use super::BottomPane; use crate::app_event::AppEvent;
use super::SelectionAction; use crate::app_event::FeedbackCategory;
use super::SelectionItem; use crate::app_event_sender::AppEventSender;
use super::SelectionViewParams; use crate::history_cell;
use crate::render::renderable::Renderable;
use super::CancellationEvent;
use super::bottom_pane_view::BottomPaneView;
use super::popup_consts::standard_popup_hint_line;
use super::textarea::TextArea;
use super::textarea::TextAreaState;
const BASE_ISSUE_URL: &str = "https://github.com/openai/codex/issues/new?template=2-bug-report.yml"; const BASE_ISSUE_URL: &str = "https://github.com/openai/codex/issues/new?template=2-bug-report.yml";
pub(crate) struct FeedbackView; /// Minimal input overlay to collect an optional feedback note, then upload
/// both logs and rollout with classification + metadata.
pub(crate) struct FeedbackNoteView {
category: FeedbackCategory,
snapshot: codex_feedback::CodexLogSnapshot,
rollout_path: Option<PathBuf>,
app_event_tx: AppEventSender,
include_logs: bool,
impl FeedbackView { // UI state
pub fn show( textarea: TextArea,
bottom_pane: &mut BottomPane, textarea_state: RefCell<TextAreaState>,
file_path: PathBuf, complete: bool,
}
impl FeedbackNoteView {
pub(crate) fn new(
category: FeedbackCategory,
snapshot: codex_feedback::CodexLogSnapshot, snapshot: codex_feedback::CodexLogSnapshot,
) { rollout_path: Option<PathBuf>,
bottom_pane.show_selection_view(Self::selection_params(file_path, snapshot)); app_event_tx: AppEventSender,
include_logs: bool,
) -> Self {
Self {
category,
snapshot,
rollout_path,
app_event_tx,
include_logs,
textarea: TextArea::new(),
textarea_state: RefCell::new(TextAreaState::default()),
complete: false,
}
} }
fn selection_params( fn submit(&mut self) {
file_path: PathBuf, let note = self.textarea.text().trim().to_string();
snapshot: codex_feedback::CodexLogSnapshot, let reason_opt = if note.is_empty() {
) -> SelectionViewParams { None
let header = FeedbackHeader::new(file_path); } else {
Some(note.as_str())
};
let rollout_path_ref = self.rollout_path.as_deref();
let classification = feedback_classification(self.category);
let thread_id = snapshot.thread_id.clone(); let cli_version = crate::version::CODEX_CLI_VERSION;
let mut thread_id = self.snapshot.thread_id.clone();
let upload_action_tread_id = thread_id.clone(); let result = if self.include_logs {
let upload_action: SelectionAction = Box::new(move |tx: &AppEventSender| { self.snapshot.upload_feedback_with_rollout(
match snapshot.upload_to_sentry() { classification,
Ok(()) => { reason_opt,
let issue_url = format!( cli_version,
"{BASE_ISSUE_URL}&steps=Uploaded%20thread:%20{upload_action_tread_id}", rollout_path_ref,
); )
tx.send(AppEvent::InsertHistoryCell(Box::new(PlainHistoryCell::new(vec![ } else {
Line::from( self.snapshot
"• Codex logs uploaded. Please open an issue using the following URL:", .upload_feedback_metadata_only(classification, reason_opt, cli_version)
), };
match result {
Ok(()) => {
let issue_url = format!("{BASE_ISSUE_URL}&steps=Uploaded%20thread:%20{thread_id}");
let prefix = if self.include_logs {
"• Feedback uploaded."
} else {
"• Feedback recorded (no logs)."
};
self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
history_cell::PlainHistoryCell::new(vec![
Line::from(format!(
"{prefix} Please open an issue using the following URL:"
)),
"".into(), "".into(),
Line::from(vec![" ".into(), issue_url.cyan().underlined()]), Line::from(vec![" ".into(), issue_url.cyan().underlined()]),
"".into(), "".into(),
Line::from(vec![" Or mention your thread ID ".into(), upload_action_tread_id.clone().bold(), " in an existing issue.".into()]) Line::from(vec![
])))); " Or mention your thread ID ".into(),
} std::mem::take(&mut thread_id).bold(),
Err(e) => { " in an existing issue.".into(),
tx.send(AppEvent::InsertHistoryCell(Box::new( ]),
history_cell::new_error_event(format!("Failed to upload logs: {e}")),
)));
}
}
});
let upload_item = SelectionItem {
name: "Yes".to_string(),
description: Some(
"Share the current Codex session logs with the team for troubleshooting."
.to_string(),
),
actions: vec![upload_action],
dismiss_on_select: true,
..Default::default()
};
let no_action: SelectionAction = Box::new(move |tx: &AppEventSender| {
let issue_url = format!("{BASE_ISSUE_URL}&steps=Thread%20ID:%20{thread_id}",);
tx.send(AppEvent::InsertHistoryCell(Box::new(
PlainHistoryCell::new(vec![
Line::from("• Please open an issue using the following URL:"),
"".into(),
Line::from(vec![" ".into(), issue_url.cyan().underlined()]),
"".into(),
Line::from(vec![
" Or mention your thread ID ".into(),
thread_id.clone().bold(),
" in an existing issue.".into(),
]), ]),
]), )));
))); }
}); Err(e) => {
self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
history_cell::new_error_event(format!("Failed to upload feedback: {e}")),
)));
}
}
self.complete = true;
}
}
let no_item = SelectionItem { impl BottomPaneView for FeedbackNoteView {
name: "No".to_string(), fn handle_key_event(&mut self, key_event: KeyEvent) {
actions: vec![no_action], match key_event {
dismiss_on_select: true, KeyEvent {
..Default::default() code: KeyCode::Esc, ..
}; } => {
self.on_ctrl_c();
let cancel_item = SelectionItem { }
name: "Cancel".to_string(), KeyEvent {
dismiss_on_select: true, code: KeyCode::Enter,
..Default::default() modifiers: KeyModifiers::NONE,
}; ..
} => {
SelectionViewParams { self.submit();
header: Box::new(header), }
items: vec![upload_item, no_item, cancel_item], KeyEvent {
..Default::default() code: KeyCode::Enter,
..
} => {
self.textarea.input(key_event);
}
other => {
self.textarea.input(other);
}
} }
} }
}
struct FeedbackHeader { fn on_ctrl_c(&mut self) -> CancellationEvent {
file_path: PathBuf, self.complete = true;
} CancellationEvent::Handled
impl FeedbackHeader {
fn new(file_path: PathBuf) -> Self {
Self { file_path }
} }
fn lines(&self) -> Vec<Line<'static>> { fn is_complete(&self) -> bool {
vec![ self.complete
Line::from("Do you want to upload logs before reporting issue?".bold()), }
"".into(),
Line::from( fn handle_paste(&mut self, pasted: String) -> bool {
"Logs may include the full conversation history of this Codex process, including prompts, tool calls, and their results.", if pasted.is_empty() {
), return false;
Line::from( }
"These logs are retained for 90 days and are used solely for troubleshooting and diagnostic purposes.", self.textarea.insert_str(&pasted);
), true
"".into(), }
Line::from(vec![
"You can review the exact content of the logs before theyre uploaded at:".into(), fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> {
]), if area.height < 2 || area.width <= 2 {
Line::from(self.file_path.display().to_string().dim()), return None;
"".into(), }
] let text_area_height = self.input_height(area.width).saturating_sub(1);
if text_area_height == 0 {
return None;
}
let top_line_count = 1u16; // title only
let textarea_rect = Rect {
x: area.x.saturating_add(2),
y: area.y.saturating_add(top_line_count).saturating_add(1),
width: area.width.saturating_sub(2),
height: text_area_height,
};
let state = *self.textarea_state.borrow();
self.textarea.cursor_pos_with_state(textarea_rect, state)
} }
} }
impl Renderable for FeedbackHeader { impl Renderable for FeedbackNoteView {
fn desired_height(&self, width: u16) -> u16 {
1u16 + self.input_height(width) + 3u16
}
fn render(&self, area: Rect, buf: &mut Buffer) { fn render(&self, area: Rect, buf: &mut Buffer) {
if area.width == 0 || area.height == 0 { if area.height == 0 || area.width == 0 {
return; return;
} }
for (i, line) in self.lines().into_iter().enumerate() { let (title, placeholder) = feedback_title_and_placeholder(self.category);
let y = area.y.saturating_add(i as u16); let input_height = self.input_height(area.width);
if y >= area.y.saturating_add(area.height) {
break; // Title line
let title_area = Rect {
x: area.x,
y: area.y,
width: area.width,
height: 1,
};
let title_spans: Vec<Span<'static>> = vec![gutter(), title.bold()];
Paragraph::new(Line::from(title_spans)).render(title_area, buf);
// Input line
let input_area = Rect {
x: area.x,
y: area.y.saturating_add(1),
width: area.width,
height: input_height,
};
if input_area.width >= 2 {
for row in 0..input_area.height {
Paragraph::new(Line::from(vec![gutter()])).render(
Rect {
x: input_area.x,
y: input_area.y.saturating_add(row),
width: 2,
height: 1,
},
buf,
);
} }
let line_area = Rect::new(area.x, y, area.width, 1).intersection(area);
line.render(line_area, buf); let text_area_height = input_area.height.saturating_sub(1);
if text_area_height > 0 {
if input_area.width > 2 {
let blank_rect = Rect {
x: input_area.x.saturating_add(2),
y: input_area.y,
width: input_area.width.saturating_sub(2),
height: 1,
};
Clear.render(blank_rect, buf);
}
let textarea_rect = Rect {
x: input_area.x.saturating_add(2),
y: input_area.y.saturating_add(1),
width: input_area.width.saturating_sub(2),
height: text_area_height,
};
let mut state = self.textarea_state.borrow_mut();
StatefulWidgetRef::render_ref(&(&self.textarea), textarea_rect, buf, &mut state);
if self.textarea.text().is_empty() {
Paragraph::new(Line::from(placeholder.dim())).render(textarea_rect, buf);
}
}
}
let hint_blank_y = input_area.y.saturating_add(input_height);
if hint_blank_y < area.y.saturating_add(area.height) {
let blank_area = Rect {
x: area.x,
y: hint_blank_y,
width: area.width,
height: 1,
};
Clear.render(blank_area, buf);
}
let hint_y = hint_blank_y.saturating_add(1);
if hint_y < area.y.saturating_add(area.height) {
Paragraph::new(standard_popup_hint_line()).render(
Rect {
x: area.x,
y: hint_y,
width: area.width,
height: 1,
},
buf,
);
} }
} }
}
fn desired_height(&self, width: u16) -> u16 { impl FeedbackNoteView {
self.lines() fn input_height(&self, width: u16) -> u16 {
.iter() let usable_width = width.saturating_sub(2);
.map(|line| line.desired_height(width)) let text_height = self.textarea.desired_height(usable_width).clamp(1, 8);
.sum() text_height.saturating_add(1).min(9)
}
}
fn gutter() -> Span<'static> {
"".cyan()
}
fn feedback_title_and_placeholder(category: FeedbackCategory) -> (String, String) {
match category {
FeedbackCategory::BadResult => (
"Tell us more (bad result)".to_string(),
"(optional) Write a short description to help us further".to_string(),
),
FeedbackCategory::GoodResult => (
"Tell us more (good result)".to_string(),
"(optional) Write a short description to help us further".to_string(),
),
FeedbackCategory::Bug => (
"Tell us more (bug)".to_string(),
"(optional) Write a short description to help us further".to_string(),
),
FeedbackCategory::Other => (
"Tell us more (other)".to_string(),
"(optional) Write a short description to help us further".to_string(),
),
}
}
fn feedback_classification(category: FeedbackCategory) -> &'static str {
match category {
FeedbackCategory::BadResult => "bad_result",
FeedbackCategory::GoodResult => "good_result",
FeedbackCategory::Bug => "bug",
FeedbackCategory::Other => "other",
}
}
// Build the selection popup params for feedback categories.
pub(crate) fn feedback_selection_params(
app_event_tx: AppEventSender,
) -> super::SelectionViewParams {
super::SelectionViewParams {
title: Some("How was this?".to_string()),
items: vec![
make_feedback_item(
app_event_tx.clone(),
"bug",
"Crash, error message, hang, or broken UI/behavior.",
FeedbackCategory::Bug,
),
make_feedback_item(
app_event_tx.clone(),
"bad result",
"Output was off-target, incorrect, incomplete, or unhelpful.",
FeedbackCategory::BadResult,
),
make_feedback_item(
app_event_tx.clone(),
"good result",
"Helpful, correct, highquality, or delightful result worth celebrating.",
FeedbackCategory::GoodResult,
),
make_feedback_item(
app_event_tx,
"other",
"Slowness, feature suggestion, UX feedback, or anything else.",
FeedbackCategory::Other,
),
],
..Default::default()
}
}
fn make_feedback_item(
app_event_tx: AppEventSender,
name: &str,
description: &str,
category: FeedbackCategory,
) -> super::SelectionItem {
let action: super::SelectionAction = Box::new(move |_sender: &AppEventSender| {
app_event_tx.send(AppEvent::OpenFeedbackConsent { category });
});
super::SelectionItem {
name: name.to_string(),
description: Some(description.to_string()),
actions: vec![action],
dismiss_on_select: true,
..Default::default()
}
}
/// Build the upload consent popup params for a given feedback category.
pub(crate) fn feedback_upload_consent_params(
app_event_tx: AppEventSender,
category: FeedbackCategory,
) -> super::SelectionViewParams {
use super::popup_consts::standard_popup_hint_line;
let yes_action: super::SelectionAction = Box::new({
let tx = app_event_tx.clone();
move |sender: &AppEventSender| {
let _ = sender;
tx.send(AppEvent::OpenFeedbackNote {
category,
include_logs: true,
});
}
});
let no_action: super::SelectionAction = Box::new({
let tx = app_event_tx;
move |sender: &AppEventSender| {
let _ = sender;
tx.send(AppEvent::OpenFeedbackNote {
category,
include_logs: false,
});
}
});
super::SelectionViewParams {
title: Some("Upload logs?".to_string()),
footer_hint: Some(standard_popup_hint_line()),
items: vec![
super::SelectionItem {
name: "Yes".to_string(),
description: Some(
"Share the current Codex session logs with the team for troubleshooting."
.to_string(),
),
actions: vec![yes_action],
dismiss_on_select: true,
..Default::default()
},
super::SelectionItem {
name: "No".to_string(),
description: Some("".to_string()),
actions: vec![no_action],
dismiss_on_select: true,
..Default::default()
},
],
..Default::default()
} }
} }
@@ -167,22 +434,19 @@ impl Renderable for FeedbackHeader {
mod tests { mod tests {
use super::*; use super::*;
use crate::app_event::AppEvent; use crate::app_event::AppEvent;
use crate::bottom_pane::list_selection_view::ListSelectionView; use crate::app_event_sender::AppEventSender;
use crate::style::user_message_style;
use codex_feedback::CodexFeedback;
use codex_protocol::ConversationId;
use insta::assert_snapshot;
use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use ratatui::style::Color;
use tokio::sync::mpsc::unbounded_channel;
fn buffer_to_string(buffer: &Buffer) -> String { fn render(view: &FeedbackNoteView, width: u16) -> String {
(0..buffer.area.height) let height = view.desired_height(width);
let area = Rect::new(0, 0, width, height);
let mut buf = Buffer::empty(area);
view.render(area, &mut buf);
let mut lines: Vec<String> = (0..area.height)
.map(|row| { .map(|row| {
let mut line = String::new(); let mut line = String::new();
for col in 0..buffer.area.width { for col in 0..area.width {
let symbol = buffer[(buffer.area.x + col, buffer.area.y + row)].symbol(); let symbol = buf[(area.x + col, area.y + row)].symbol();
if symbol.is_empty() { if symbol.is_empty() {
line.push(' '); line.push(' ');
} else { } else {
@@ -191,34 +455,49 @@ mod tests {
} }
line.trim_end().to_string() line.trim_end().to_string()
}) })
.collect::<Vec<_>>() .collect();
.join("\n")
while lines.first().is_some_and(|l| l.trim().is_empty()) {
lines.remove(0);
}
while lines.last().is_some_and(|l| l.trim().is_empty()) {
lines.pop();
}
lines.join("\n")
}
fn make_view(category: FeedbackCategory) -> FeedbackNoteView {
let (tx_raw, _rx) = tokio::sync::mpsc::unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let snapshot = codex_feedback::CodexFeedback::new().snapshot(None);
FeedbackNoteView::new(category, snapshot, None, tx, true)
} }
#[test] #[test]
fn renders_feedback_view_header() { fn feedback_view_bad_result() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>(); let view = make_view(FeedbackCategory::BadResult);
let app_event_tx = AppEventSender::new(tx_raw); let rendered = render(&view, 60);
let snapshot = CodexFeedback::new().snapshot(Some( insta::assert_snapshot!("feedback_view_bad_result", rendered);
ConversationId::from_string("550e8400-e29b-41d4-a716-446655440000").unwrap(), }
));
let file_path = PathBuf::from("/tmp/codex-feedback.log");
let params = FeedbackView::selection_params(file_path.clone(), snapshot); #[test]
let view = ListSelectionView::new(params, app_event_tx); fn feedback_view_good_result() {
let view = make_view(FeedbackCategory::GoodResult);
let rendered = render(&view, 60);
insta::assert_snapshot!("feedback_view_good_result", rendered);
}
let width = 72; #[test]
let height = view.desired_height(width).max(1); fn feedback_view_bug() {
let area = Rect::new(0, 0, width, height); let view = make_view(FeedbackCategory::Bug);
let mut buf = Buffer::empty(area); let rendered = render(&view, 60);
view.render(area, &mut buf); insta::assert_snapshot!("feedback_view_bug", rendered);
}
let rendered = #[test]
buffer_to_string(&buf).replace(&file_path.display().to_string(), "<LOG_PATH>"); fn feedback_view_other() {
assert_snapshot!("feedback_view_render", rendered); let view = make_view(FeedbackCategory::Other);
let rendered = render(&view, 60);
let cell_style = buf[(area.x, area.y)].style(); insta::assert_snapshot!("feedback_view_other", rendered);
let expected_bg = user_message_style().bg.unwrap_or(Color::Reset);
assert_eq!(cell_style.bg.unwrap_or(Color::Reset), expected_bg);
} }
} }

View File

@@ -28,12 +28,14 @@ mod list_selection_view;
mod prompt_args; mod prompt_args;
pub(crate) use list_selection_view::SelectionViewParams; pub(crate) use list_selection_view::SelectionViewParams;
mod feedback_view; mod feedback_view;
pub(crate) use feedback_view::feedback_selection_params;
pub(crate) use feedback_view::feedback_upload_consent_params;
mod paste_burst; mod paste_burst;
pub mod popup_consts; pub mod popup_consts;
mod scroll_state; mod scroll_state;
mod selection_popup_common; mod selection_popup_common;
mod textarea; mod textarea;
pub(crate) use feedback_view::FeedbackView; pub(crate) use feedback_view::FeedbackNoteView;
#[derive(Debug, Clone, Copy, PartialEq, Eq)] #[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum CancellationEvent { pub(crate) enum CancellationEvent {

View File

@@ -0,0 +1,9 @@
---
source: tui/src/bottom_pane/feedback_view.rs
expression: rendered
---
▌ Tell us more (bad result)
▌ (optional) Write a short description to help us further
Press enter to confirm or esc to go back

View File

@@ -0,0 +1,9 @@
---
source: tui/src/bottom_pane/feedback_view.rs
expression: rendered
---
▌ Tell us more (bug)
▌ (optional) Write a short description to help us further
Press enter to confirm or esc to go back

View File

@@ -0,0 +1,9 @@
---
source: tui/src/bottom_pane/feedback_view.rs
expression: rendered
---
▌ Tell us more (good result)
▌ (optional) Write a short description to help us further
Press enter to confirm or esc to go back

View File

@@ -0,0 +1,9 @@
---
source: tui/src/bottom_pane/feedback_view.rs
expression: rendered
---
▌ Tell us more (other)
▌ (optional) Write a short description to help us further
Press enter to confirm or esc to go back

View File

@@ -276,6 +276,8 @@ pub(crate) struct ChatWidget {
last_rendered_width: std::cell::Cell<Option<usize>>, last_rendered_width: std::cell::Cell<Option<usize>>,
// Feedback sink for /feedback // Feedback sink for /feedback
feedback: codex_feedback::CodexFeedback, feedback: codex_feedback::CodexFeedback,
// Current session rollout path (if known)
current_rollout_path: Option<PathBuf>,
} }
struct UserMessage { struct UserMessage {
@@ -322,6 +324,7 @@ impl ChatWidget {
self.bottom_pane self.bottom_pane
.set_history_metadata(event.history_log_id, event.history_entry_count); .set_history_metadata(event.history_log_id, event.history_entry_count);
self.conversation_id = Some(event.session_id); self.conversation_id = Some(event.session_id);
self.current_rollout_path = Some(event.rollout_path.clone());
let initial_messages = event.initial_messages.clone(); let initial_messages = event.initial_messages.clone();
let model_for_header = event.model.clone(); let model_for_header = event.model.clone();
self.session_header.set_model(&model_for_header); self.session_header.set_model(&model_for_header);
@@ -343,6 +346,36 @@ impl ChatWidget {
} }
} }
pub(crate) fn open_feedback_note(
&mut self,
category: crate::app_event::FeedbackCategory,
include_logs: bool,
) {
// Build a fresh snapshot at the time of opening the note overlay.
let snapshot = self.feedback.snapshot(self.conversation_id);
let rollout = if include_logs {
self.current_rollout_path.clone()
} else {
None
};
let view = crate::bottom_pane::FeedbackNoteView::new(
category,
snapshot,
rollout,
self.app_event_tx.clone(),
include_logs,
);
self.bottom_pane.show_view(Box::new(view));
self.request_redraw();
}
pub(crate) fn open_feedback_consent(&mut self, category: crate::app_event::FeedbackCategory) {
let params =
crate::bottom_pane::feedback_upload_consent_params(self.app_event_tx.clone(), category);
self.bottom_pane.show_selection_view(params);
self.request_redraw();
}
fn on_agent_message(&mut self, message: String) { fn on_agent_message(&mut self, message: String) {
// If we have a stream_controller, then the final agent message is redundant and will be a // If we have a stream_controller, then the final agent message is redundant and will be a
// duplicate of what has already been streamed. // duplicate of what has already been streamed.
@@ -496,7 +529,7 @@ impl ChatWidget {
if reason != TurnAbortReason::ReviewEnded { if reason != TurnAbortReason::ReviewEnded {
self.add_to_history(history_cell::new_error_event( self.add_to_history(history_cell::new_error_event(
"Conversation interrupted - tell the model what to do differently".to_owned(), "Conversation interrupted - tell the model what to do differently. Something went wrong? Hit `/feedback` to report the issue.".to_owned(),
)); ));
} }
@@ -958,6 +991,7 @@ impl ChatWidget {
needs_final_message_separator: false, needs_final_message_separator: false,
last_rendered_width: std::cell::Cell::new(None), last_rendered_width: std::cell::Cell::new(None),
feedback, feedback,
current_rollout_path: None,
} }
} }
@@ -1025,6 +1059,7 @@ impl ChatWidget {
needs_final_message_separator: false, needs_final_message_separator: false,
last_rendered_width: std::cell::Cell::new(None), last_rendered_width: std::cell::Cell::new(None),
feedback, feedback,
current_rollout_path: None,
} }
} }
@@ -1129,23 +1164,11 @@ impl ChatWidget {
} }
match cmd { match cmd {
SlashCommand::Feedback => { SlashCommand::Feedback => {
let snapshot = self.feedback.snapshot(self.conversation_id); // Step 1: pick a category (UI built in feedback_view)
match snapshot.save_to_temp_file() { let params =
Ok(path) => { crate::bottom_pane::feedback_selection_params(self.app_event_tx.clone());
crate::bottom_pane::FeedbackView::show( self.bottom_pane.show_selection_view(params);
&mut self.bottom_pane, self.request_redraw();
path,
snapshot,
);
self.request_redraw();
}
Err(e) => {
self.add_to_history(history_cell::new_error_event(format!(
"Failed to save feedback logs: {e}"
)));
self.request_redraw();
}
}
} }
SlashCommand::New => { SlashCommand::New => {
self.app_event_tx.send(AppEvent::NewSession); self.app_event_tx.send(AppEvent::NewSession);

View File

@@ -0,0 +1,11 @@
---
source: tui/src/chatwidget/tests.rs
expression: popup
---
How was this?
1. bug Crash, error message, hang, or broken UI/behavior.
2. bad result Output was off-target, incorrect, incomplete, or unhelpful.
3. good result Helpful, correct, highquality, or delightful result worth
celebrating.
4. other Slowness, feature suggestion, UX feedback, or anything else.

View File

@@ -0,0 +1,11 @@
---
source: tui/src/chatwidget/tests.rs
expression: popup
---
Upload logs?
1. Yes Share the current Codex session logs with the team for
troubleshooting.
2. No
Press enter to confirm or esc to go back

View File

@@ -0,0 +1,5 @@
---
source: tui/src/chatwidget/tests.rs
expression: last
---
■ Conversation interrupted - tell the model what to do differently. Something went wrong? Hit `/feedback` to report the issue.

View File

@@ -299,6 +299,7 @@ fn make_chatwidget_manual() -> (
needs_final_message_separator: false, needs_final_message_separator: false,
last_rendered_width: std::cell::Cell::new(None), last_rendered_width: std::cell::Cell::new(None),
feedback: codex_feedback::CodexFeedback::new(), feedback: codex_feedback::CodexFeedback::new(),
current_rollout_path: None,
}; };
(widget, rx, op_rx) (widget, rx, op_rx)
} }
@@ -998,6 +999,37 @@ fn interrupt_exec_marks_failed_snapshot() {
assert_snapshot!("interrupt_exec_marks_failed", exec_blob); assert_snapshot!("interrupt_exec_marks_failed", exec_blob);
} }
// Snapshot test: after an interrupted turn, a gentle error message is inserted
// suggesting the user to tell the model what to do differently and to use /feedback.
#[test]
fn interrupted_turn_error_message_snapshot() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual();
// Simulate an in-progress task so the widget is in a running state.
chat.handle_codex_event(Event {
id: "task-1".into(),
msg: EventMsg::TaskStarted(TaskStartedEvent {
model_context_window: None,
}),
});
// Abort the turn (like pressing Esc) and drain inserted history.
chat.handle_codex_event(Event {
id: "task-1".into(),
msg: EventMsg::TurnAborted(codex_core::protocol::TurnAbortedEvent {
reason: TurnAbortReason::Interrupted,
}),
});
let cells = drain_insert_history(&mut rx);
assert!(
!cells.is_empty(),
"expected error message to be inserted after interruption"
);
let last = lines_to_single_string(cells.last().unwrap());
assert_snapshot!("interrupted_turn_error_message", last);
}
/// Opening custom prompt from the review popup, pressing Esc returns to the /// Opening custom prompt from the review popup, pressing Esc returns to the
/// parent popup, pressing Esc again dismisses all panels (back to normal mode). /// parent popup, pressing Esc again dismisses all panels (back to normal mode).
#[test] #[test]
@@ -1175,6 +1207,28 @@ fn model_reasoning_selection_popup_snapshot() {
assert_snapshot!("model_reasoning_selection_popup", popup); assert_snapshot!("model_reasoning_selection_popup", popup);
} }
#[test]
fn feedback_selection_popup_snapshot() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual();
// Open the feedback category selection popup via slash command.
chat.dispatch_command(SlashCommand::Feedback);
let popup = render_bottom_popup(&chat, 80);
assert_snapshot!("feedback_selection_popup", popup);
}
#[test]
fn feedback_upload_consent_popup_snapshot() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual();
// Open the consent popup directly for a chosen category.
chat.open_feedback_consent(crate::app_event::FeedbackCategory::Bug);
let popup = render_bottom_popup(&chat, 80);
assert_snapshot!("feedback_upload_consent_popup", popup);
}
#[test] #[test]
fn reasoning_popup_escape_returns_to_model_popup() { fn reasoning_popup_escape_returns_to_model_popup() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); let (mut chat, _rx, _op_rx) = make_chatwidget_manual();