mirror of
https://github.com/instructkr/claude-code.git
synced 2026-05-16 10:56:45 +00:00
fix: REPL display, /compact panic, identity leak, DeepSeek reasoning, thinking blocks
Five interrelated fixes from parallel Hephaestus sessions: 1. fix(repl): display assistant text after spinner (#2981, #2982, #2937) - Added final_assistant_text() call after run_turn spinner completes - REPL now shows response text like run_prompt_json does 2. fix(compact): handle Thinking content blocks (#2985) - Added ContentBlock::Thinking variant throughout compact summarizer - Prevents panic when /compact encounters thinking blocks 3. fix(prompt): provider-aware model identity (#2822) - New ModelFamilyIdentity enum (Claude vs Generic) - Non-Anthropic models no longer say 'I am Claude' - model_family_identity_for() detects provider and sets identity 4. fix(openai): preserve DeepSeek reasoning_content (#2821) - Stream parser now captures reasoning_content from OpenAI-compat - Emits ThinkingDelta/SignatureDelta events for reasoning models - Thinking blocks included in conversation history for re-send 5. feat(runtime): Thinking block support across codebase - AssistantEvent::Thinking variant in conversation.rs - ContentBlock::Thinking in session serialization - Thinking-aware compact summarization - Tests for thinking block ordering and content Closes #2981, #2982, #2937, #2985, #2822, #2821
This commit is contained in:
@@ -213,6 +213,7 @@ fn summarize_messages(messages: &[ConversationMessage]) -> String {
|
||||
ContentBlock::ToolUse { name, .. } => Some(name.as_str()),
|
||||
ContentBlock::ToolResult { tool_name, .. } => Some(tool_name.as_str()),
|
||||
ContentBlock::Text { .. } => None,
|
||||
ContentBlock::Thinking { .. } => None,
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
tool_names.sort_unstable();
|
||||
@@ -317,6 +318,9 @@ fn merge_compact_summaries(existing_summary: Option<&str>, new_summary: &str) ->
|
||||
fn summarize_block(block: &ContentBlock) -> String {
|
||||
let raw = match block {
|
||||
ContentBlock::Text { text } => text.clone(),
|
||||
ContentBlock::Thinking { thinking, .. } => {
|
||||
format!("thinking ({} chars)", thinking.chars().count())
|
||||
}
|
||||
ContentBlock::ToolUse { name, input, .. } => format!("tool_use {name}({input})"),
|
||||
ContentBlock::ToolResult {
|
||||
tool_name,
|
||||
@@ -378,6 +382,7 @@ fn collect_key_files(messages: &[ConversationMessage]) -> Vec<String> {
|
||||
ContentBlock::Text { text } => text.as_str(),
|
||||
ContentBlock::ToolUse { input, .. } => input.as_str(),
|
||||
ContentBlock::ToolResult { output, .. } => output.as_str(),
|
||||
ContentBlock::Thinking { thinking, .. } => thinking.as_str(),
|
||||
})
|
||||
.flat_map(extract_file_candidates)
|
||||
.collect::<Vec<_>>();
|
||||
@@ -400,6 +405,7 @@ fn first_text_block(message: &ConversationMessage) -> Option<&str> {
|
||||
ContentBlock::Text { text } if !text.trim().is_empty() => Some(text.as_str()),
|
||||
ContentBlock::ToolUse { .. }
|
||||
| ContentBlock::ToolResult { .. }
|
||||
| ContentBlock::Thinking { .. }
|
||||
| ContentBlock::Text { .. } => None,
|
||||
})
|
||||
}
|
||||
@@ -450,6 +456,10 @@ fn estimate_message_tokens(message: &ConversationMessage) -> usize {
|
||||
ContentBlock::ToolResult {
|
||||
tool_name, output, ..
|
||||
} => (tool_name.len() + output.len()) / 4 + 1,
|
||||
ContentBlock::Thinking {
|
||||
thinking,
|
||||
signature,
|
||||
} => thinking.len() / 4 + signature.as_ref().map_or(0, |value| value.len() / 4 + 1),
|
||||
})
|
||||
.sum()
|
||||
}
|
||||
|
||||
@@ -28,6 +28,10 @@ pub struct ApiRequest {
|
||||
/// Streamed events emitted while processing a single assistant turn.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum AssistantEvent {
|
||||
Thinking {
|
||||
thinking: String,
|
||||
signature: Option<String>,
|
||||
},
|
||||
TextDelta(String),
|
||||
ToolUse {
|
||||
id: String,
|
||||
@@ -721,6 +725,16 @@ fn build_assistant_message(
|
||||
|
||||
for event in events {
|
||||
match event {
|
||||
AssistantEvent::Thinking {
|
||||
thinking,
|
||||
signature,
|
||||
} => {
|
||||
flush_text_block(&mut text, &mut blocks);
|
||||
blocks.push(ContentBlock::Thinking {
|
||||
thinking,
|
||||
signature,
|
||||
});
|
||||
}
|
||||
AssistantEvent::TextDelta(delta) => text.push_str(&delta),
|
||||
AssistantEvent::ToolUse { id, name, input } => {
|
||||
flush_text_block(&mut text, &mut blocks);
|
||||
@@ -1723,6 +1737,47 @@ mod tests {
|
||||
.contains("assistant stream produced no content"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_assistant_message_places_thinking_block_before_text_and_tool_use() {
|
||||
// given
|
||||
let events = vec![
|
||||
AssistantEvent::Thinking {
|
||||
thinking: "pondering".to_string(),
|
||||
signature: Some("sig".to_string()),
|
||||
},
|
||||
AssistantEvent::TextDelta("hello".to_string()),
|
||||
AssistantEvent::ToolUse {
|
||||
id: "tool-1".to_string(),
|
||||
name: "echo".to_string(),
|
||||
input: "payload".to_string(),
|
||||
},
|
||||
AssistantEvent::MessageStop,
|
||||
];
|
||||
|
||||
// when
|
||||
let (message, _, _) = build_assistant_message(events)
|
||||
.expect("assistant message should preserve thinking, text, and tool blocks");
|
||||
|
||||
// then
|
||||
assert_eq!(
|
||||
message.blocks,
|
||||
vec![
|
||||
ContentBlock::Thinking {
|
||||
thinking: "pondering".to_string(),
|
||||
signature: Some("sig".to_string()),
|
||||
},
|
||||
ContentBlock::Text {
|
||||
text: "hello".to_string(),
|
||||
},
|
||||
ContentBlock::ToolUse {
|
||||
id: "tool-1".to_string(),
|
||||
name: "echo".to_string(),
|
||||
input: "payload".to_string(),
|
||||
},
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn static_tool_executor_rejects_unknown_tools() {
|
||||
// given
|
||||
|
||||
@@ -131,8 +131,8 @@ pub use policy_engine::{
|
||||
PolicyEngine, PolicyRule, ReconcileReason, ReviewStatus,
|
||||
};
|
||||
pub use prompt::{
|
||||
load_system_prompt, prepend_bullets, ContextFile, ProjectContext, PromptBuildError,
|
||||
SystemPromptBuilder, FRONTIER_MODEL_NAME, SYSTEM_PROMPT_DYNAMIC_BOUNDARY,
|
||||
load_system_prompt, prepend_bullets, ContextFile, ModelFamilyIdentity, ProjectContext,
|
||||
PromptBuildError, SystemPromptBuilder, FRONTIER_MODEL_NAME, SYSTEM_PROMPT_DYNAMIC_BOUNDARY,
|
||||
};
|
||||
pub use recovery_recipes::{
|
||||
attempt_recovery, recipe_for, EscalationPolicy, FailureScenario, RecoveryContext,
|
||||
|
||||
@@ -43,6 +43,24 @@ pub const FRONTIER_MODEL_NAME: &str = "Claude Opus 4.6";
|
||||
const MAX_INSTRUCTION_FILE_CHARS: usize = 4_000;
|
||||
const MAX_TOTAL_INSTRUCTION_CHARS: usize = 12_000;
|
||||
|
||||
/// Neutral identity for the model family line in generated prompts.
|
||||
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
|
||||
pub enum ModelFamilyIdentity {
|
||||
#[default]
|
||||
Claude,
|
||||
Generic,
|
||||
}
|
||||
|
||||
impl ModelFamilyIdentity {
|
||||
#[must_use]
|
||||
pub const fn family_label(self) -> &'static str {
|
||||
match self {
|
||||
Self::Claude => FRONTIER_MODEL_NAME,
|
||||
Self::Generic => "an AI assistant",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Contents of an instruction file included in prompt construction.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct ContextFile {
|
||||
@@ -97,6 +115,7 @@ pub struct SystemPromptBuilder {
|
||||
output_style_prompt: Option<String>,
|
||||
os_name: Option<String>,
|
||||
os_version: Option<String>,
|
||||
model_family: Option<ModelFamilyIdentity>,
|
||||
append_sections: Vec<String>,
|
||||
project_context: Option<ProjectContext>,
|
||||
config: Option<RuntimeConfig>,
|
||||
@@ -122,6 +141,12 @@ impl SystemPromptBuilder {
|
||||
self
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_model_family(mut self, model_family: ModelFamilyIdentity) -> Self {
|
||||
self.model_family = Some(model_family);
|
||||
self
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_project_context(mut self, project_context: ProjectContext) -> Self {
|
||||
self.project_context = Some(project_context);
|
||||
@@ -179,9 +204,10 @@ impl SystemPromptBuilder {
|
||||
|| "unknown".to_string(),
|
||||
|context| context.current_date.clone(),
|
||||
);
|
||||
let identity = self.model_family.unwrap_or_default();
|
||||
let mut lines = vec!["# Environment context".to_string()];
|
||||
lines.extend(prepend_bullets(vec![
|
||||
format!("Model family: {FRONTIER_MODEL_NAME}"),
|
||||
format!("Model family: {}", identity.family_label()),
|
||||
format!("Working directory: {cwd}"),
|
||||
format!("Date: {date}"),
|
||||
format!(
|
||||
@@ -434,12 +460,14 @@ pub fn load_system_prompt(
|
||||
current_date: impl Into<String>,
|
||||
os_name: impl Into<String>,
|
||||
os_version: impl Into<String>,
|
||||
model_family: ModelFamilyIdentity,
|
||||
) -> Result<Vec<String>, PromptBuildError> {
|
||||
let cwd = cwd.into();
|
||||
let project_context = ProjectContext::discover_with_git(&cwd, current_date.into())?;
|
||||
let config = ConfigLoader::default_for(&cwd).load()?;
|
||||
Ok(SystemPromptBuilder::new()
|
||||
.with_os(os_name, os_version)
|
||||
.with_model_family(model_family)
|
||||
.with_project_context(project_context)
|
||||
.with_runtime_config(config)
|
||||
.build())
|
||||
@@ -522,7 +550,8 @@ mod tests {
|
||||
use super::{
|
||||
collapse_blank_lines, display_context_path, normalize_instruction_content,
|
||||
render_instruction_content, render_instruction_files, truncate_instruction_content,
|
||||
ContextFile, ProjectContext, SystemPromptBuilder, SYSTEM_PROMPT_DYNAMIC_BOUNDARY,
|
||||
ContextFile, ModelFamilyIdentity, ProjectContext, SystemPromptBuilder,
|
||||
SYSTEM_PROMPT_DYNAMIC_BOUNDARY,
|
||||
};
|
||||
use crate::config::ConfigLoader;
|
||||
use std::fs;
|
||||
@@ -804,13 +833,19 @@ mod tests {
|
||||
std::env::set_var("HOME", &root);
|
||||
std::env::set_var("CLAW_CONFIG_HOME", root.join("missing-home"));
|
||||
std::env::set_current_dir(&root).expect("change cwd");
|
||||
let prompt = super::load_system_prompt(&root, "2026-03-31", "linux", "6.8")
|
||||
.expect("system prompt should load")
|
||||
.join(
|
||||
"
|
||||
let prompt = super::load_system_prompt(
|
||||
&root,
|
||||
"2026-03-31",
|
||||
"linux",
|
||||
"6.8",
|
||||
ModelFamilyIdentity::Claude,
|
||||
)
|
||||
.expect("system prompt should load")
|
||||
.join(
|
||||
"
|
||||
|
||||
",
|
||||
);
|
||||
);
|
||||
std::env::set_current_dir(previous).expect("restore cwd");
|
||||
if let Some(value) = original_home {
|
||||
std::env::set_var("HOME", value);
|
||||
@@ -828,6 +863,50 @@ mod tests {
|
||||
fs::remove_dir_all(root).expect("cleanup temp dir");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn renders_default_claude_model_family_identity() {
|
||||
// given: a prompt builder without an explicit model family override
|
||||
let project_context = ProjectContext {
|
||||
cwd: PathBuf::from("/tmp/project"),
|
||||
current_date: "2026-03-31".to_string(),
|
||||
..ProjectContext::default()
|
||||
};
|
||||
|
||||
// when: rendering the system prompt environment section
|
||||
let prompt = SystemPromptBuilder::new()
|
||||
.with_os("linux", "6.8")
|
||||
.with_project_context(project_context)
|
||||
.render();
|
||||
|
||||
// then: the Claude model family label is preserved by default
|
||||
assert!(prompt.contains("Model family: Claude Opus 4.6"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn renders_generic_model_family_identity_without_claude_label() {
|
||||
// given: a prompt builder with generic model family identity
|
||||
let project_context = ProjectContext {
|
||||
cwd: PathBuf::from("/tmp/project"),
|
||||
current_date: "2026-03-31".to_string(),
|
||||
..ProjectContext::default()
|
||||
};
|
||||
|
||||
// when: rendering the system prompt environment section
|
||||
let prompt = SystemPromptBuilder::new()
|
||||
.with_os("linux", "6.8")
|
||||
.with_model_family(ModelFamilyIdentity::Generic)
|
||||
.with_project_context(project_context)
|
||||
.render();
|
||||
let model_family_line = prompt
|
||||
.lines()
|
||||
.find(|line| line.contains("Model family:"))
|
||||
.expect("model family line should render");
|
||||
|
||||
// then: the model family line is neutral and excludes Claude Opus 4.6
|
||||
assert_eq!(model_family_line, " - Model family: an AI assistant");
|
||||
assert!(!model_family_line.contains("Claude Opus 4.6"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn renders_claude_code_style_sections_with_project_context() {
|
||||
let root = temp_dir();
|
||||
|
||||
@@ -30,6 +30,10 @@ pub enum ContentBlock {
|
||||
Text {
|
||||
text: String,
|
||||
},
|
||||
Thinking {
|
||||
thinking: String,
|
||||
signature: Option<String>,
|
||||
},
|
||||
ToolUse {
|
||||
id: String,
|
||||
name: String,
|
||||
@@ -737,6 +741,22 @@ impl ContentBlock {
|
||||
object.insert("type".to_string(), JsonValue::String("text".to_string()));
|
||||
object.insert("text".to_string(), JsonValue::String(text.clone()));
|
||||
}
|
||||
Self::Thinking {
|
||||
thinking,
|
||||
signature,
|
||||
} => {
|
||||
object.insert(
|
||||
"type".to_string(),
|
||||
JsonValue::String("thinking".to_string()),
|
||||
);
|
||||
object.insert("thinking".to_string(), JsonValue::String(thinking.clone()));
|
||||
if let Some(signature) = signature {
|
||||
object.insert(
|
||||
"signature".to_string(),
|
||||
JsonValue::String(signature.clone()),
|
||||
);
|
||||
}
|
||||
}
|
||||
Self::ToolUse { id, name, input } => {
|
||||
object.insert(
|
||||
"type".to_string(),
|
||||
@@ -783,6 +803,13 @@ impl ContentBlock {
|
||||
"text" => Ok(Self::Text {
|
||||
text: required_string(object, "text")?,
|
||||
}),
|
||||
"thinking" => Ok(Self::Thinking {
|
||||
thinking: required_string(object, "thinking")?,
|
||||
signature: object
|
||||
.get("signature")
|
||||
.and_then(JsonValue::as_str)
|
||||
.map(String::from),
|
||||
}),
|
||||
"tool_use" => Ok(Self::ToolUse {
|
||||
id: required_string(object, "id")?,
|
||||
name: required_string(object, "name")?,
|
||||
@@ -1208,6 +1235,36 @@ mod tests {
|
||||
assert_eq!(restored.session_id, session.session_id);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn persists_assistant_thinking_block_round_trip_through_jsonl() {
|
||||
// given
|
||||
let mut session = Session::new();
|
||||
session
|
||||
.push_message(ConversationMessage::assistant(vec![
|
||||
ContentBlock::Thinking {
|
||||
thinking: "trace the path through session persistence".to_string(),
|
||||
signature: Some("sig-123".to_string()),
|
||||
},
|
||||
]))
|
||||
.expect("thinking block should append");
|
||||
let path = temp_session_path("thinking-jsonl");
|
||||
|
||||
// when
|
||||
session.save_to_path(&path).expect("session should save");
|
||||
let restored = Session::load_from_path(&path).expect("session should load");
|
||||
fs::remove_file(&path).expect("temp file should be removable");
|
||||
|
||||
// then
|
||||
assert_eq!(restored, session);
|
||||
assert_eq!(
|
||||
restored.messages[0].blocks[0],
|
||||
ContentBlock::Thinking {
|
||||
thinking: "trace the path through session persistence".to_string(),
|
||||
signature: Some("sig-123".to_string()),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn loads_legacy_session_json_object() {
|
||||
let path = temp_session_path("legacy");
|
||||
|
||||
Reference in New Issue
Block a user