mirror of
https://github.com/openai/codex.git
synced 2026-10-29 19:08:43 +00:00
Pass turn id with feedback uploads
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -468,6 +468,12 @@
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"turnId": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
@@ -9447,6 +9447,12 @@
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"turnId": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
@@ -27,6 +27,12 @@
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"turnId": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
@@ -2,4 +2,4 @@
|
||||
|
||||
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
|
||||
|
||||
export type FeedbackUploadParams = { classification: string, reason?: string | null, threadId?: string | null, includeLogs: boolean, extraLogFiles?: Array<string> | null, };
|
||||
export type FeedbackUploadParams = { classification: string, reason?: string | null, threadId?: string | null, turnId?: string | null, includeLogs: boolean, extraLogFiles?: Array<string> | null, };
|
||||
|
||||
@@ -1741,6 +1741,8 @@ pub struct FeedbackUploadParams {
|
||||
pub reason: Option<String>,
|
||||
#[ts(optional = nullable)]
|
||||
pub thread_id: Option<String>,
|
||||
#[ts(optional = nullable)]
|
||||
pub turn_id: Option<String>,
|
||||
pub include_logs: bool,
|
||||
#[ts(optional = nullable)]
|
||||
pub extra_log_files: Option<Vec<PathBuf>>,
|
||||
|
||||
@@ -6887,6 +6887,7 @@ impl CodexMessageProcessor {
|
||||
classification,
|
||||
reason,
|
||||
thread_id,
|
||||
turn_id,
|
||||
include_logs,
|
||||
extra_log_files,
|
||||
} = params;
|
||||
@@ -6929,6 +6930,7 @@ impl CodexMessageProcessor {
|
||||
snapshot.upload_feedback(
|
||||
&classification,
|
||||
reason.as_deref(),
|
||||
turn_id.as_deref(),
|
||||
include_logs,
|
||||
&attachment_paths,
|
||||
Some(session_source),
|
||||
|
||||
@@ -218,16 +218,58 @@ impl CodexLogSnapshot {
|
||||
Ok(path)
|
||||
}
|
||||
|
||||
fn upload_tags(
|
||||
&self,
|
||||
classification: &str,
|
||||
reason: Option<&str>,
|
||||
turn_id: Option<&str>,
|
||||
session_source: Option<&SessionSource>,
|
||||
) -> BTreeMap<String, String> {
|
||||
let cli_version = env!("CARGO_PKG_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()),
|
||||
]);
|
||||
if let Some(turn_id) = turn_id {
|
||||
tags.insert(String::from("turn_id"), turn_id.to_string());
|
||||
}
|
||||
if let Some(source) = session_source {
|
||||
tags.insert(String::from("session_source"), source.to_string());
|
||||
}
|
||||
if let Some(r) = reason {
|
||||
tags.insert(String::from("reason"), r.to_string());
|
||||
}
|
||||
|
||||
let reserved = [
|
||||
"thread_id",
|
||||
"turn_id",
|
||||
"classification",
|
||||
"cli_version",
|
||||
"session_source",
|
||||
"reason",
|
||||
];
|
||||
for (key, value) in &self.tags {
|
||||
if reserved.contains(&key.as_str()) {
|
||||
continue;
|
||||
}
|
||||
if let Entry::Vacant(entry) = tags.entry(key.clone()) {
|
||||
entry.insert(value.clone());
|
||||
}
|
||||
}
|
||||
tags
|
||||
}
|
||||
|
||||
/// Upload feedback to Sentry with optional attachments.
|
||||
pub fn upload_feedback(
|
||||
&self,
|
||||
classification: &str,
|
||||
reason: Option<&str>,
|
||||
turn_id: Option<&str>,
|
||||
include_logs: bool,
|
||||
extra_log_files: &[PathBuf],
|
||||
session_source: Option<SessionSource>,
|
||||
) -> Result<()> {
|
||||
use std::collections::BTreeMap;
|
||||
use std::fs;
|
||||
use std::str::FromStr;
|
||||
use std::sync::Arc;
|
||||
@@ -249,34 +291,7 @@ impl CodexLogSnapshot {
|
||||
..Default::default()
|
||||
});
|
||||
|
||||
let cli_version = env!("CARGO_PKG_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()),
|
||||
]);
|
||||
if let Some(source) = session_source.as_ref() {
|
||||
tags.insert(String::from("session_source"), source.to_string());
|
||||
}
|
||||
if let Some(r) = reason {
|
||||
tags.insert(String::from("reason"), r.to_string());
|
||||
}
|
||||
|
||||
let reserved = [
|
||||
"thread_id",
|
||||
"classification",
|
||||
"cli_version",
|
||||
"session_source",
|
||||
"reason",
|
||||
];
|
||||
for (key, value) in &self.tags {
|
||||
if reserved.contains(&key.as_str()) {
|
||||
continue;
|
||||
}
|
||||
if let Entry::Vacant(entry) = tags.entry(key.clone()) {
|
||||
entry.insert(value.clone());
|
||||
}
|
||||
}
|
||||
let tags = self.upload_tags(classification, reason, turn_id, session_source.as_ref());
|
||||
|
||||
let level = match classification {
|
||||
"bug" | "bad_result" | "safety_check" => Level::Error,
|
||||
@@ -459,4 +474,28 @@ mod tests {
|
||||
pretty_assertions::assert_eq!(snap.tags.get("model").map(String::as_str), Some("gpt-5"));
|
||||
pretty_assertions::assert_eq!(snap.tags.get("cached").map(String::as_str), Some("true"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn upload_tags_include_turn_id_and_preserve_reserved_fields() {
|
||||
let snap = CodexLogSnapshot {
|
||||
bytes: Vec::new(),
|
||||
tags: BTreeMap::from([
|
||||
(String::from("custom"), String::from("tag")),
|
||||
(String::from("turn_id"), String::from("stale-turn")),
|
||||
]),
|
||||
thread_id: String::from("thread-1"),
|
||||
};
|
||||
|
||||
let tags = snap.upload_tags(
|
||||
"bug",
|
||||
Some("broken"),
|
||||
Some("turn-123"),
|
||||
Some(&SessionSource::Cli),
|
||||
);
|
||||
|
||||
pretty_assertions::assert_eq!(tags.get("thread_id").map(String::as_str), Some("thread-1"));
|
||||
pretty_assertions::assert_eq!(tags.get("turn_id").map(String::as_str), Some("turn-123"));
|
||||
pretty_assertions::assert_eq!(tags.get("custom").map(String::as_str), Some("tag"));
|
||||
pretty_assertions::assert_eq!(tags.get("reason").map(String::as_str), Some("broken"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -47,6 +47,7 @@ pub(crate) enum FeedbackAudience {
|
||||
pub(crate) struct FeedbackNoteView {
|
||||
category: FeedbackCategory,
|
||||
snapshot: codex_feedback::CodexLogSnapshot,
|
||||
turn_id: Option<String>,
|
||||
rollout_path: Option<PathBuf>,
|
||||
app_event_tx: AppEventSender,
|
||||
include_logs: bool,
|
||||
@@ -62,6 +63,7 @@ impl FeedbackNoteView {
|
||||
pub(crate) fn new(
|
||||
category: FeedbackCategory,
|
||||
snapshot: codex_feedback::CodexLogSnapshot,
|
||||
turn_id: Option<String>,
|
||||
rollout_path: Option<PathBuf>,
|
||||
app_event_tx: AppEventSender,
|
||||
include_logs: bool,
|
||||
@@ -70,6 +72,7 @@ impl FeedbackNoteView {
|
||||
Self {
|
||||
category,
|
||||
snapshot,
|
||||
turn_id,
|
||||
rollout_path,
|
||||
app_event_tx,
|
||||
include_logs,
|
||||
@@ -99,6 +102,7 @@ impl FeedbackNoteView {
|
||||
let result = self.snapshot.upload_feedback(
|
||||
classification,
|
||||
reason_opt,
|
||||
self.turn_id.as_deref(),
|
||||
self.include_logs,
|
||||
&log_file_paths,
|
||||
Some(SessionSource::Cli),
|
||||
@@ -593,6 +597,7 @@ mod tests {
|
||||
category,
|
||||
snapshot,
|
||||
None,
|
||||
None,
|
||||
tx,
|
||||
true,
|
||||
FeedbackAudience::External,
|
||||
|
||||
@@ -127,6 +127,7 @@ use codex_protocol::protocol::TokenUsageInfo;
|
||||
use codex_protocol::protocol::TurnAbortReason;
|
||||
use codex_protocol::protocol::TurnCompleteEvent;
|
||||
use codex_protocol::protocol::TurnDiffEvent;
|
||||
use codex_protocol::protocol::TurnStartedEvent;
|
||||
use codex_protocol::protocol::UndoCompletedEvent;
|
||||
use codex_protocol::protocol::UndoStartedEvent;
|
||||
use codex_protocol::protocol::UserMessageEvent;
|
||||
@@ -599,6 +600,7 @@ pub(crate) struct ChatWidget {
|
||||
// Set when commentary output completes; once stream queues go idle we restore the status row.
|
||||
pending_status_indicator_restore: bool,
|
||||
thread_id: Option<ThreadId>,
|
||||
last_turn_id: Option<String>,
|
||||
thread_name: Option<String>,
|
||||
forked_from: Option<ThreadId>,
|
||||
frame_requester: FrameRequester,
|
||||
@@ -1253,6 +1255,7 @@ impl ChatWidget {
|
||||
let view = crate::bottom_pane::FeedbackNoteView::new(
|
||||
category,
|
||||
snapshot,
|
||||
self.last_turn_id.clone(),
|
||||
rollout,
|
||||
self.app_event_tx.clone(),
|
||||
include_logs,
|
||||
@@ -2865,6 +2868,7 @@ impl ChatWidget {
|
||||
retry_status_header: None,
|
||||
pending_status_indicator_restore: false,
|
||||
thread_id: None,
|
||||
last_turn_id: None,
|
||||
thread_name: None,
|
||||
forked_from: None,
|
||||
queued_user_messages: VecDeque::new(),
|
||||
@@ -3044,6 +3048,7 @@ impl ChatWidget {
|
||||
retry_status_header: None,
|
||||
pending_status_indicator_restore: false,
|
||||
thread_id: None,
|
||||
last_turn_id: None,
|
||||
thread_name: None,
|
||||
forked_from: None,
|
||||
saw_plan_update_this_turn: false,
|
||||
@@ -3212,6 +3217,7 @@ impl ChatWidget {
|
||||
retry_status_header: None,
|
||||
pending_status_indicator_restore: false,
|
||||
thread_id: None,
|
||||
last_turn_id: None,
|
||||
thread_name: None,
|
||||
forked_from: None,
|
||||
queued_user_messages: VecDeque::new(),
|
||||
@@ -4360,7 +4366,8 @@ impl ChatWidget {
|
||||
self.on_agent_reasoning_final();
|
||||
}
|
||||
EventMsg::AgentReasoningSectionBreak(_) => self.on_reasoning_section_break(),
|
||||
EventMsg::TurnStarted(_) => {
|
||||
EventMsg::TurnStarted(TurnStartedEvent { turn_id, .. }) => {
|
||||
self.last_turn_id = Some(turn_id);
|
||||
if !is_resume_initial_replay {
|
||||
self.on_task_started();
|
||||
}
|
||||
|
||||
@@ -1702,6 +1702,7 @@ async fn make_chatwidget_manual(
|
||||
retry_status_header: None,
|
||||
pending_status_indicator_restore: false,
|
||||
thread_id: None,
|
||||
last_turn_id: None,
|
||||
thread_name: None,
|
||||
forked_from: None,
|
||||
frame_requester: FrameRequester::test_dummy(),
|
||||
@@ -7934,6 +7935,23 @@ async fn replayed_turn_started_does_not_mark_task_running() {
|
||||
|
||||
assert!(!chat.bottom_pane.is_task_running());
|
||||
assert!(chat.bottom_pane.status_widget().is_none());
|
||||
assert_eq!(chat.last_turn_id.as_deref(), Some("turn-1"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn live_turn_started_records_last_turn_id_for_feedback() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
chat.handle_codex_event(Event {
|
||||
id: "turn-feedback".into(),
|
||||
msg: EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-feedback".to_string(),
|
||||
model_context_window: None,
|
||||
collaboration_mode_kind: ModeKind::Default,
|
||||
}),
|
||||
});
|
||||
|
||||
assert_eq!(chat.last_turn_id.as_deref(), Some("turn-feedback"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
Reference in New Issue
Block a user