diff --git a/codex-rs/core/src/error.rs b/codex-rs/core/src/error.rs index e733b3c6..0901e426 100644 --- a/codex-rs/core/src/error.rs +++ b/codex-rs/core/src/error.rs @@ -55,7 +55,7 @@ pub enum SandboxErr { #[derive(Error, Debug)] pub enum CodexErr { // 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 { dangling_artifacts: Vec, }, @@ -91,7 +91,7 @@ pub enum CodexErr { /// Returned by run_command_stream when the user pressed Ctrl‑C (SIGINT). Session uses this to /// 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, /// Unexpected HTTP status code. diff --git a/codex-rs/feedback/src/lib.rs b/codex-rs/feedback/src/lib.rs index 23996463..fc696f2a 100644 --- a/codex-rs/feedback/src/lib.rs +++ b/codex-rs/feedback/src/lib.rs @@ -167,8 +167,18 @@ impl CodexLogSnapshot { 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::fs; use std::str::FromStr; use std::sync::Arc; @@ -182,22 +192,47 @@ impl CodexLogSnapshot { use sentry::transports::DefaultTransportFactory; use sentry::types::Dsn; + // Build Sentry client 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 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 " + let title = format!( + "[{}]: Codex session {}", + display_classification(classification), + self.thread_id + ); let event = Event { - level: Level::Error, - message: Some("Codex Log Upload ".to_string() + &self.thread_id), + level, + message: Some(title), tags, ..Default::default() }; - let mut envelope = Envelope::new(); envelope.add_item(EnvelopeItem::Event(event)); + + // Attachment 1: Codex logs snapshot envelope.add_item(EnvelopeItem::Attachment(Attachment { buffer: self.bytes.clone(), filename: String::from("codex-logs.log"), @@ -205,11 +240,96 @@ impl CodexLogSnapshot { 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.flush(Some(Duration::from_secs(UPLOAD_TIMEOUT_SECS))); 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 " + 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)] diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 5a47e40f..c8eab729 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -360,6 +360,15 @@ impl App { AppEvent::OpenFullAccessConfirmation { 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 } => { let profile = self.active_profile.as_deref(); match persist_model_selection(&self.config.codex_home, profile, &model, effort) diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 8592032d..8b14b0be 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -101,4 +101,23 @@ pub(crate) enum AppEvent { /// Open the approval popup. 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, } diff --git a/codex-rs/tui/src/bottom_pane/feedback_view.rs b/codex-rs/tui/src/bottom_pane/feedback_view.rs index 9c75e32b..fce3a795 100644 --- a/codex-rs/tui/src/bottom_pane/feedback_view.rs +++ b/codex-rs/tui/src/bottom_pane/feedback_view.rs @@ -1,165 +1,432 @@ -use crate::app_event::AppEvent; -use crate::app_event_sender::AppEventSender; -use crate::history_cell; -use crate::history_cell::PlainHistoryCell; -use crate::render::renderable::Renderable; +use std::cell::RefCell; +use std::path::PathBuf; + +use crossterm::event::KeyCode; +use crossterm::event::KeyEvent; +use crossterm::event::KeyModifiers; use ratatui::buffer::Buffer; use ratatui::layout::Rect; use ratatui::style::Stylize; 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 super::SelectionAction; -use super::SelectionItem; -use super::SelectionViewParams; +use crate::app_event::AppEvent; +use crate::app_event::FeedbackCategory; +use crate::app_event_sender::AppEventSender; +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"; -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, + app_event_tx: AppEventSender, + include_logs: bool, -impl FeedbackView { - pub fn show( - bottom_pane: &mut BottomPane, - file_path: PathBuf, + // UI state + textarea: TextArea, + textarea_state: RefCell, + complete: bool, +} + +impl FeedbackNoteView { + pub(crate) fn new( + category: FeedbackCategory, snapshot: codex_feedback::CodexLogSnapshot, - ) { - bottom_pane.show_selection_view(Self::selection_params(file_path, snapshot)); + rollout_path: Option, + 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( - file_path: PathBuf, - snapshot: codex_feedback::CodexLogSnapshot, - ) -> SelectionViewParams { - let header = FeedbackHeader::new(file_path); + fn submit(&mut self) { + let note = self.textarea.text().trim().to_string(); + let reason_opt = if note.is_empty() { + None + } 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 upload_action: SelectionAction = Box::new(move |tx: &AppEventSender| { - match snapshot.upload_to_sentry() { - Ok(()) => { - let issue_url = format!( - "{BASE_ISSUE_URL}&steps=Uploaded%20thread:%20{upload_action_tread_id}", - ); - tx.send(AppEvent::InsertHistoryCell(Box::new(PlainHistoryCell::new(vec![ - Line::from( - "• Codex logs uploaded. Please open an issue using the following URL:", - ), + let result = if self.include_logs { + self.snapshot.upload_feedback_with_rollout( + classification, + reason_opt, + cli_version, + rollout_path_ref, + ) + } else { + self.snapshot + .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(), Line::from(vec![" ".into(), issue_url.cyan().underlined()]), "".into(), - Line::from(vec![" Or mention your thread ID ".into(), upload_action_tread_id.clone().bold(), " in an existing issue.".into()]) - ])))); - } - Err(e) => { - 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(), + Line::from(vec![ + " Or mention your thread ID ".into(), + std::mem::take(&mut thread_id).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 { - name: "No".to_string(), - actions: vec![no_action], - dismiss_on_select: true, - ..Default::default() - }; - - let cancel_item = SelectionItem { - name: "Cancel".to_string(), - dismiss_on_select: true, - ..Default::default() - }; - - SelectionViewParams { - header: Box::new(header), - items: vec![upload_item, no_item, cancel_item], - ..Default::default() +impl BottomPaneView for FeedbackNoteView { + fn handle_key_event(&mut self, key_event: KeyEvent) { + match key_event { + KeyEvent { + code: KeyCode::Esc, .. + } => { + self.on_ctrl_c(); + } + KeyEvent { + code: KeyCode::Enter, + modifiers: KeyModifiers::NONE, + .. + } => { + self.submit(); + } + KeyEvent { + code: KeyCode::Enter, + .. + } => { + self.textarea.input(key_event); + } + other => { + self.textarea.input(other); + } } } -} -struct FeedbackHeader { - file_path: PathBuf, -} - -impl FeedbackHeader { - fn new(file_path: PathBuf) -> Self { - Self { file_path } + fn on_ctrl_c(&mut self) -> CancellationEvent { + self.complete = true; + CancellationEvent::Handled } - fn lines(&self) -> Vec> { - vec![ - Line::from("Do you want to upload logs before reporting issue?".bold()), - "".into(), - Line::from( - "Logs may include the full conversation history of this Codex process, including prompts, tool calls, and their results.", - ), - Line::from( - "These logs are retained for 90 days and are used solely for troubleshooting and diagnostic purposes.", - ), - "".into(), - Line::from(vec![ - "You can review the exact content of the logs before they’re uploaded at:".into(), - ]), - Line::from(self.file_path.display().to_string().dim()), - "".into(), - ] + fn is_complete(&self) -> bool { + self.complete + } + + fn handle_paste(&mut self, pasted: String) -> bool { + if pasted.is_empty() { + return false; + } + self.textarea.insert_str(&pasted); + true + } + + fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> { + if area.height < 2 || area.width <= 2 { + return None; + } + 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) { - if area.width == 0 || area.height == 0 { + if area.height == 0 || area.width == 0 { return; } - for (i, line) in self.lines().into_iter().enumerate() { - let y = area.y.saturating_add(i as u16); - if y >= area.y.saturating_add(area.height) { - break; + let (title, placeholder) = feedback_title_and_placeholder(self.category); + let input_height = self.input_height(area.width); + + // Title line + let title_area = Rect { + x: area.x, + y: area.y, + width: area.width, + height: 1, + }; + let title_spans: Vec> = 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 { - self.lines() - .iter() - .map(|line| line.desired_height(width)) - .sum() +impl FeedbackNoteView { + fn input_height(&self, width: u16) -> u16 { + let usable_width = width.saturating_sub(2); + let text_height = self.textarea.desired_height(usable_width).clamp(1, 8); + 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, high‑quality, 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 { use super::*; use crate::app_event::AppEvent; - use crate::bottom_pane::list_selection_view::ListSelectionView; - 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; + use crate::app_event_sender::AppEventSender; - fn buffer_to_string(buffer: &Buffer) -> String { - (0..buffer.area.height) + fn render(view: &FeedbackNoteView, width: u16) -> String { + 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 = (0..area.height) .map(|row| { let mut line = String::new(); - for col in 0..buffer.area.width { - let symbol = buffer[(buffer.area.x + col, buffer.area.y + row)].symbol(); + for col in 0..area.width { + let symbol = buf[(area.x + col, area.y + row)].symbol(); if symbol.is_empty() { line.push(' '); } else { @@ -191,34 +455,49 @@ mod tests { } line.trim_end().to_string() }) - .collect::>() - .join("\n") + .collect(); + + 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::(); + let tx = AppEventSender::new(tx_raw); + let snapshot = codex_feedback::CodexFeedback::new().snapshot(None); + FeedbackNoteView::new(category, snapshot, None, tx, true) } #[test] - fn renders_feedback_view_header() { - let (tx_raw, _rx) = unbounded_channel::(); - let app_event_tx = AppEventSender::new(tx_raw); - let snapshot = CodexFeedback::new().snapshot(Some( - ConversationId::from_string("550e8400-e29b-41d4-a716-446655440000").unwrap(), - )); - let file_path = PathBuf::from("/tmp/codex-feedback.log"); + fn feedback_view_bad_result() { + let view = make_view(FeedbackCategory::BadResult); + let rendered = render(&view, 60); + insta::assert_snapshot!("feedback_view_bad_result", rendered); + } - let params = FeedbackView::selection_params(file_path.clone(), snapshot); - let view = ListSelectionView::new(params, app_event_tx); + #[test] + 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; - let height = view.desired_height(width).max(1); - let area = Rect::new(0, 0, width, height); - let mut buf = Buffer::empty(area); - view.render(area, &mut buf); + #[test] + fn feedback_view_bug() { + let view = make_view(FeedbackCategory::Bug); + let rendered = render(&view, 60); + insta::assert_snapshot!("feedback_view_bug", rendered); + } - let rendered = - buffer_to_string(&buf).replace(&file_path.display().to_string(), ""); - assert_snapshot!("feedback_view_render", rendered); - - let cell_style = buf[(area.x, area.y)].style(); - let expected_bg = user_message_style().bg.unwrap_or(Color::Reset); - assert_eq!(cell_style.bg.unwrap_or(Color::Reset), expected_bg); + #[test] + fn feedback_view_other() { + let view = make_view(FeedbackCategory::Other); + let rendered = render(&view, 60); + insta::assert_snapshot!("feedback_view_other", rendered); } } diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 48bc0e9d..cf9e3547 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -28,12 +28,14 @@ mod list_selection_view; mod prompt_args; pub(crate) use list_selection_view::SelectionViewParams; mod feedback_view; +pub(crate) use feedback_view::feedback_selection_params; +pub(crate) use feedback_view::feedback_upload_consent_params; mod paste_burst; pub mod popup_consts; mod scroll_state; mod selection_popup_common; mod textarea; -pub(crate) use feedback_view::FeedbackView; +pub(crate) use feedback_view::FeedbackNoteView; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum CancellationEvent { diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_view_bad_result.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_view_bad_result.snap new file mode 100644 index 00000000..465f0f9c --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_view_bad_result.snap @@ -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 diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_view_bug.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_view_bug.snap new file mode 100644 index 00000000..a0b56601 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_view_bug.snap @@ -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 diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_view_good_result.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_view_good_result.snap new file mode 100644 index 00000000..73074d61 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_view_good_result.snap @@ -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 diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_view_other.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_view_other.snap new file mode 100644 index 00000000..80e4ffef --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__feedback_view__tests__feedback_view_other.snap @@ -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 diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 833ae5d8..81661aa3 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -276,6 +276,8 @@ pub(crate) struct ChatWidget { last_rendered_width: std::cell::Cell>, // Feedback sink for /feedback feedback: codex_feedback::CodexFeedback, + // Current session rollout path (if known) + current_rollout_path: Option, } struct UserMessage { @@ -322,6 +324,7 @@ impl ChatWidget { self.bottom_pane .set_history_metadata(event.history_log_id, event.history_entry_count); self.conversation_id = Some(event.session_id); + self.current_rollout_path = Some(event.rollout_path.clone()); let initial_messages = event.initial_messages.clone(); let model_for_header = event.model.clone(); 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) { // If we have a stream_controller, then the final agent message is redundant and will be a // duplicate of what has already been streamed. @@ -496,7 +529,7 @@ impl ChatWidget { if reason != TurnAbortReason::ReviewEnded { 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, last_rendered_width: std::cell::Cell::new(None), feedback, + current_rollout_path: None, } } @@ -1025,6 +1059,7 @@ impl ChatWidget { needs_final_message_separator: false, last_rendered_width: std::cell::Cell::new(None), feedback, + current_rollout_path: None, } } @@ -1129,23 +1164,11 @@ impl ChatWidget { } match cmd { SlashCommand::Feedback => { - let snapshot = self.feedback.snapshot(self.conversation_id); - match snapshot.save_to_temp_file() { - Ok(path) => { - crate::bottom_pane::FeedbackView::show( - &mut self.bottom_pane, - 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(); - } - } + // Step 1: pick a category (UI built in feedback_view) + let params = + crate::bottom_pane::feedback_selection_params(self.app_event_tx.clone()); + self.bottom_pane.show_selection_view(params); + self.request_redraw(); } SlashCommand::New => { self.app_event_tx.send(AppEvent::NewSession); diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_selection_popup.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_selection_popup.snap new file mode 100644 index 00000000..4a982420 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_selection_popup.snap @@ -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, high‑quality, or delightful result worth + celebrating. + 4. other Slowness, feature suggestion, UX feedback, or anything else. diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_upload_consent_popup.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_upload_consent_popup.snap new file mode 100644 index 00000000..092e961d --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__feedback_upload_consent_popup.snap @@ -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 diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupted_turn_error_message.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupted_turn_error_message.snap new file mode 100644 index 00000000..60715e58 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupted_turn_error_message.snap @@ -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. diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index e25e04e0..6ec87765 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -299,6 +299,7 @@ fn make_chatwidget_manual() -> ( needs_final_message_separator: false, last_rendered_width: std::cell::Cell::new(None), feedback: codex_feedback::CodexFeedback::new(), + current_rollout_path: None, }; (widget, rx, op_rx) } @@ -998,6 +999,37 @@ fn interrupt_exec_marks_failed_snapshot() { 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 /// parent popup, pressing Esc again dismisses all panels (back to normal mode). #[test] @@ -1175,6 +1207,28 @@ fn model_reasoning_selection_popup_snapshot() { 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] fn reasoning_popup_escape_returns_to_model_popup() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual();