Skip to content

Commit 7b5d956

Browse files
committed
fix(otto): reuse known thread ids progressively
Known conversation reuse should stay on the preview-first path instead of hydrating the full thread, and stale history errors should stop leaking into newer TUI generations. Iteration: iter-03 Agent: software-engineer/a Made-with: Cursor
1 parent cf34c49 commit 7b5d956

File tree

5 files changed

+225
-17
lines changed

5 files changed

+225
-17
lines changed

crates/ascend-tools-cli/src/conversation.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,13 @@ pub(crate) fn resolve_conversation_flag(
301301
if resume {
302302
Ok(Some(client.latest_conversation_id()?))
303303
} else if let Some(conv) = conversation {
304-
Ok(Some(resolve_conversation_id_interactive(client, &conv)?))
304+
match client.resolve_otto_thread(Some(&conv), None) {
305+
Ok(thread_id) => Ok(thread_id),
306+
Err(Error::AmbiguousTitle { matches, .. }) => {
307+
pick_from_matches(&conv, &matches).map(Some)
308+
}
309+
Err(e) => Err(e.into()),
310+
}
305311
} else {
306312
Ok(thread)
307313
}

crates/ascend-tools-cli/tests/cli_integration.rs

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,79 @@ fn otto_run_jsonl_rejects_json_output_mode() {
303303
));
304304
}
305305

306+
#[test]
307+
fn otto_run_with_known_conversation_id_avoids_full_thread_get() {
308+
let mut server = Server::new();
309+
mock_auth(&mut server);
310+
311+
let preview_body = serde_json::json!({
312+
"id": "thread-cli-cont",
313+
"title": "Continuation thread",
314+
"messages": {
315+
"msg-1": {
316+
"id": "msg-1",
317+
"role": "user",
318+
"content": "hello",
319+
"created_at": "2026-01-01T00:00:00Z"
320+
}
321+
},
322+
"updated_at": "2026-01-01T00:00:00Z",
323+
"is_processing": false,
324+
"context_window_stats": null,
325+
"total_message_count": 1,
326+
"has_more": false,
327+
"oldest_message_id": "msg-1",
328+
"latest_message_id": "msg-1"
329+
})
330+
.to_string();
331+
let updates_body = format!(
332+
"event: thread.preview\ndata: {preview_body}\n\n\
333+
event: response.output_text.delta\ndata: {{\"delta\":\"continued\",\"item_id\":\"item-1\",\"content_index\":0,\"output_index\":0,\"type\":\"response.output_text.delta\"}}\n\n\
334+
event: thread.done\ndata: {{\"latest_message_id\":\"msg-2\"}}\n\n"
335+
);
336+
337+
let full_get = server
338+
.mock("GET", "/api/v1/otto/threads/thread-cli-cont")
339+
.expect(0)
340+
.create();
341+
let updates = server
342+
.mock("GET", "/api/v1/otto/threads/thread-cli-cont/updates")
343+
.match_header("accept", "text/event-stream")
344+
.with_status(200)
345+
.with_body(updates_body)
346+
.expect(2)
347+
.create();
348+
let send = server
349+
.mock("POST", "/api/v1/otto/threads/thread-cli-cont/messages")
350+
.match_header("authorization", "Bearer cli-token")
351+
.with_status(200)
352+
.with_header("content-type", "application/json")
353+
.with_body(r#"{"thread_id":"thread-cli-cont"}"#)
354+
.expect(1)
355+
.create();
356+
357+
let mut cmd = command_with_auth(&server);
358+
cmd.args([
359+
"-o",
360+
"json",
361+
"otto",
362+
"run",
363+
"continue",
364+
"--conversation",
365+
"thread-cli-cont",
366+
]);
367+
cmd.assert()
368+
.success()
369+
.stdout(predicate::str::contains("\"message\": \"continued\""))
370+
.stdout(predicate::str::contains(
371+
"\"thread_id\": \"thread-cli-cont\"",
372+
));
373+
374+
updates.assert();
375+
send.assert();
376+
full_get.assert();
377+
}
378+
306379
#[test]
307380
fn otto_conversation_open_emits_progressive_preview_json() {
308381
let mut server = Server::new();

crates/ascend-tools-core/src/client.rs

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -559,20 +559,23 @@ impl AscendClient {
559559
Err(e) => return Err(e),
560560
}
561561

562-
// Not a valid ID — use server-side title filter.
562+
self.resolve_conversation_title(title_or_id)
563+
}
564+
565+
fn resolve_conversation_title(&self, title: &str) -> Result<String> {
563566
let list = self.list_conversations(ConversationFilters {
564-
title: Some(title_or_id.to_string()),
567+
title: Some(title.to_string()),
565568
..Default::default()
566569
})?;
567570
match list.threads.len() {
568571
0 => Err(Error::NotFound {
569572
kind: "conversation".to_string(),
570-
title: title_or_id.to_string(),
573+
title: title.to_string(),
571574
}),
572575
1 => Ok(list.threads.into_iter().next().unwrap().id),
573576
_ => Err(Error::AmbiguousTitle {
574577
kind: "conversation".to_string(),
575-
title: title_or_id.to_string(),
578+
title: title.to_string(),
576579
matches: list
577580
.threads
578581
.iter()
@@ -600,16 +603,23 @@ impl AscendClient {
600603

601604
/// Resolve a `conversation` or `thread_id` parameter to an optional thread ID.
602605
///
603-
/// If `conversation` is `Some`, resolves it via `resolve_conversation_id`.
604-
/// Otherwise passes through `thread_id` as-is. Used by SDK bindings to
605-
/// avoid duplicating the resolution logic.
606+
/// If `conversation` is `Some`, accepts a known thread ID via the
607+
/// progressive preview path and falls back to title resolution when that
608+
/// preview probe returns 404. Otherwise passes through `thread_id` as-is.
609+
/// Used by SDK bindings to avoid duplicating the continuation logic.
606610
pub fn resolve_otto_thread(
607611
&self,
608612
conversation: Option<&str>,
609613
thread_id: Option<String>,
610614
) -> Result<Option<String>> {
611615
if let Some(conv) = conversation {
612-
Ok(Some(self.resolve_conversation_id(conv)?))
616+
match self.get_conversation_preview(conv) {
617+
Ok(_) => Ok(Some(conv.to_string())),
618+
Err(ref e) if e.http_status() == Some(404) => {
619+
Ok(Some(self.resolve_conversation_title(conv)?))
620+
}
621+
Err(e) => Err(e),
622+
}
613623
} else {
614624
Ok(thread_id)
615625
}

crates/ascend-tools-core/tests/client_http.rs

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,97 @@ fn get_conversation_messages_before_requests_before_limit() {
343343
assert_eq!(ordered, vec!["msg-18", "msg-19"]);
344344
}
345345

346+
#[test]
347+
fn resolve_otto_thread_reuses_known_id_via_progressive_preview() {
348+
let mut server = Server::new();
349+
let now = SystemTime::now()
350+
.duration_since(UNIX_EPOCH)
351+
.unwrap()
352+
.as_secs();
353+
mock_auth(&mut server, "token-resolve-thread-id", now + 3600, 1);
354+
355+
let preview_body = serde_json::json!({
356+
"id": "thread-known",
357+
"title": "Known thread",
358+
"messages": {},
359+
"updated_at": "2026-01-01T00:01:00Z",
360+
"is_processing": false,
361+
"context_window_stats": null,
362+
"total_message_count": 0,
363+
"has_more": false,
364+
"oldest_message_id": null,
365+
"latest_message_id": null
366+
})
367+
.to_string();
368+
369+
let full_get = server
370+
.mock("GET", "/api/v1/otto/threads/thread-known")
371+
.expect(0)
372+
.create();
373+
let updates = server
374+
.mock("GET", "/api/v1/otto/threads/thread-known/updates")
375+
.match_header("accept", "text/event-stream")
376+
.with_status(200)
377+
.with_body(format!("event: thread.preview\ndata: {preview_body}\n\n"))
378+
.expect(1)
379+
.create();
380+
381+
let client = test_client(&server);
382+
let resolved = client
383+
.resolve_otto_thread(Some("thread-known"), None)
384+
.unwrap();
385+
386+
assert_eq!(resolved.as_deref(), Some("thread-known"));
387+
updates.assert();
388+
full_get.assert();
389+
}
390+
391+
#[test]
392+
fn resolve_otto_thread_falls_back_to_title_search_after_preview_404() {
393+
let mut server = Server::new();
394+
let now = SystemTime::now()
395+
.duration_since(UNIX_EPOCH)
396+
.unwrap()
397+
.as_secs();
398+
mock_auth(&mut server, "token-resolve-thread-title", now + 3600, 1);
399+
400+
let updates = server
401+
.mock("GET", "/api/v1/otto/threads/Named%20thread/updates")
402+
.match_header("accept", "text/event-stream")
403+
.with_status(404)
404+
.with_header("content-type", "application/json")
405+
.with_body(r#"{"detail":"thread not found"}"#)
406+
.expect(1)
407+
.create();
408+
let list = server
409+
.mock("GET", "/api/v1/otto/threads")
410+
.match_query(Matcher::UrlEncoded("title".into(), "Named thread".into()))
411+
.with_status(200)
412+
.with_header("content-type", "application/json")
413+
.with_body(
414+
serde_json::json!({
415+
"threads": [{
416+
"id": "thread-from-title",
417+
"title": "Named thread",
418+
"updated_at": "2026-01-01T00:00:00Z"
419+
}],
420+
"total": 1
421+
})
422+
.to_string(),
423+
)
424+
.expect(1)
425+
.create();
426+
427+
let client = test_client(&server);
428+
let resolved = client
429+
.resolve_otto_thread(Some("Named thread"), None)
430+
.unwrap();
431+
432+
assert_eq!(resolved.as_deref(), Some("thread-from-title"));
433+
updates.assert();
434+
list.assert();
435+
}
436+
346437
#[test]
347438
fn otto_streaming_events_exposes_raw_updates() {
348439
let mut server = Server::new();

crates/ascend-tools-tui/src/lib.rs

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,10 @@ enum StreamMsg {
142142
generation: u64,
143143
messages: Vec<Message>,
144144
},
145-
ConversationHistoryError(String),
145+
ConversationHistoryError {
146+
generation: u64,
147+
error: String,
148+
},
146149
StopFinished {
147150
error: Option<String>,
148151
},
@@ -943,11 +946,13 @@ impl App {
943946
}
944947
}
945948
}
946-
StreamMsg::ConversationHistoryError(error) => {
947-
if self.messages.is_empty() {
948-
self.push_system(format!("Could not load recent history: {error}"));
949-
} else {
950-
self.push_system(format!("Could not load older history: {error}"));
949+
StreamMsg::ConversationHistoryError { generation, error } => {
950+
if generation == self.stream_generation {
951+
if self.messages.is_empty() {
952+
self.push_system(format!("Could not load recent history: {error}"));
953+
} else {
954+
self.push_system(format!("Could not load older history: {error}"));
955+
}
951956
}
952957
}
953958
StreamMsg::StopFinished { error } => {
@@ -2542,7 +2547,10 @@ pub fn run_tui(
25422547
});
25432548
});
25442549
if let Err(err) = result {
2545-
let _ = history_tx.send(StreamMsg::ConversationHistoryError(err.to_string()));
2550+
let _ = history_tx.send(StreamMsg::ConversationHistoryError {
2551+
generation: history_gen,
2552+
error: err.to_string(),
2553+
});
25462554
}
25472555
});
25482556
}
@@ -2842,13 +2850,33 @@ mod tests {
28422850
fn conversation_history_error_surfaces_when_history_is_empty() {
28432851
let mut app = test_app();
28442852

2845-
app.handle_stream_msg(StreamMsg::ConversationHistoryError("preview failed".into()));
2853+
app.handle_stream_msg(StreamMsg::ConversationHistoryError {
2854+
generation: 0,
2855+
error: "preview failed".into(),
2856+
});
28462857

28472858
assert!(app.messages.iter().any(|m| {
28482859
m.role == Role::System && m.content.contains("Could not load recent history")
28492860
}));
28502861
}
28512862

2863+
#[test]
2864+
fn stale_conversation_history_error_is_discarded_after_generation_advances() {
2865+
let mut app = test_app();
2866+
app.stream_generation = 2;
2867+
2868+
app.handle_stream_msg(StreamMsg::ConversationHistoryError {
2869+
generation: 1,
2870+
error: "stale failure".into(),
2871+
});
2872+
2873+
assert!(
2874+
!app.messages
2875+
.iter()
2876+
.any(|m| m.role == Role::System && m.content.contains("stale failure"))
2877+
);
2878+
}
2879+
28522880
#[test]
28532881
fn stream_conversation_history_fetches_preview_then_older_pages() {
28542882
let preview = test_preview(

0 commit comments

Comments
 (0)