Skip to content

Commit e485923

Browse files
1jehuangquangdang46
authored andcommitted
fix(render): show reasoning before answer on session resume
Providers persist an assistant turn as [Text, ReasoningTrace, ToolUse], so the resume/re-render path appended reasoning markup into the message text in stored block order, displaying the thinking *after* the answer. Live streaming shows reasoning before the answer, so resumed sessions looked wrong. Accumulate reasoning separately and prepend it to the answer text at each flush point to match live ordering. Adds a regression test using the real stored block order.
1 parent 93f4121 commit e485923

2 files changed

Lines changed: 62 additions & 5 deletions

File tree

crates/jcode-base/src/session/render.rs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,12 @@ pub fn render_messages_and_images_with_compacted_history(
381381
};
382382
let message_role = msg.role.clone();
383383
let mut text = String::new();
384+
// Reasoning is accumulated separately so it can be rendered *before* the
385+
// answer text, matching the live streaming order. Providers persist the
386+
// assistant turn as `[Text, ReasoningTrace, ToolUse]`, so appending
387+
// reasoning into `text` in block order would otherwise show the thinking
388+
// *after* the answer on resume/re-render.
389+
let mut reasoning = String::new();
384390
let mut tool_calls: Vec<String> = Vec::new();
385391
let mut current_tool: Option<ToolCall> = None;
386392
let mut last_image_idx: Option<usize> = None;
@@ -421,13 +427,16 @@ pub fn render_messages_and_images_with_compacted_history(
421427
content,
422428
..
423429
} => {
424-
if !text.is_empty() {
430+
let combined = format!("{}{}", reasoning, text);
431+
if !combined.is_empty() {
425432
if role == "user" && !is_attached_image_label_text(&text) {
426433
user_prompt_count += 1;
427434
}
435+
text.clear();
436+
reasoning.clear();
428437
rendered.push(RenderedMessage {
429438
role: role.to_string(),
430-
content: std::mem::take(&mut text),
439+
content: combined,
431440
tool_calls: tool_calls.clone(),
432441
tool_data: None,
433442
});
@@ -452,7 +461,7 @@ pub fn render_messages_and_images_with_compacted_history(
452461
});
453462
}
454463
ContentBlock::Reasoning { text: t } | ContentBlock::ReasoningTrace { text: t } => {
455-
text.push_str(&format_reasoning_markup(t));
464+
reasoning.push_str(&format_reasoning_markup(t));
456465
}
457466
ContentBlock::AnthropicThinking { .. } | ContentBlock::OpenAIReasoning { .. } => {}
458467
ContentBlock::Image { media_type, data } => {
@@ -480,13 +489,14 @@ pub fn render_messages_and_images_with_compacted_history(
480489
}
481490
}
482491

483-
if !text.is_empty() {
492+
let combined = format!("{}{}", reasoning, text);
493+
if !combined.is_empty() {
484494
if role == "user" && !is_attached_image_label_text(&text) {
485495
user_prompt_count += 1;
486496
}
487497
rendered.push(RenderedMessage {
488498
role: role.to_string(),
489-
content: text,
499+
content: combined,
490500
tool_calls,
491501
tool_data: None,
492502
});

crates/jcode-base/src/session_tests/cases.rs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,6 +1066,53 @@ fn test_render_messages_honors_system_display_role_override() {
10661066
assert!(rendered[0].content.contains("Background Task Completed"));
10671067
}
10681068

1069+
#[test]
1070+
fn test_render_messages_renders_reasoning_before_answer_in_stored_order() {
1071+
// Regression: providers persist the assistant turn as `[Text, ReasoningTrace,
1072+
// ToolUse]` (see agent/turn_loops.rs push order). On resume/re-render the
1073+
// reasoning must still appear *before* the answer text to match the live
1074+
// streaming order, even though the Text block is stored first.
1075+
use jcode_render_core::REASONING_SENTINEL;
1076+
1077+
let _env_lock = lock_env();
1078+
let _mode = EnvVarGuard::set("JCODE_REASONING_DISPLAY", "full");
1079+
crate::config::invalidate_config_cache();
1080+
1081+
let mut session = Session::create_with_id(
1082+
"session_render_reasoning_order_test".to_string(),
1083+
None,
1084+
Some("render reasoning order test".to_string()),
1085+
);
1086+
1087+
session.add_message(
1088+
Role::Assistant,
1089+
vec![
1090+
ContentBlock::Text {
1091+
text: "Here is the answer.".to_string(),
1092+
cache_control: None,
1093+
},
1094+
ContentBlock::ReasoningTrace {
1095+
text: "step one\nstep two".to_string(),
1096+
},
1097+
],
1098+
);
1099+
1100+
let rendered = render_messages(&session);
1101+
assert_eq!(rendered.len(), 1);
1102+
let content = &rendered[0].content;
1103+
assert!(
1104+
content.contains(&format!("*{0}step one{0}*", REASONING_SENTINEL)),
1105+
"expected reasoning markup, got: {content:?}"
1106+
);
1107+
assert!(content.contains("Here is the answer."));
1108+
let reasoning_pos = content.find("step two").unwrap();
1109+
let answer_pos = content.find("Here is the answer.").unwrap();
1110+
assert!(
1111+
reasoning_pos < answer_pos,
1112+
"reasoning should precede the answer text even when stored after it: {content:?}"
1113+
);
1114+
}
1115+
10691116
#[test]
10701117
fn test_render_messages_renders_persisted_reasoning() {
10711118
use jcode_render_core::REASONING_SENTINEL;

0 commit comments

Comments
 (0)