Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 44 additions & 7 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"

# LLM / Rig framework
rig = { version = "0.31", package = "rig-core", features = ["derive"] }
rig = { version = "0.33", package = "rig-core", features = ["derive"] }

# HTTP clients for LLM providers
reqwest = { version = "0.13", features = ["json", "stream", "form", "query", "gzip"] }
Expand Down
8 changes: 8 additions & 0 deletions src/agent/compactor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,9 @@ pub fn estimate_history_tokens(history: &[Message]) -> usize {
chars += estimate_assistant_content_chars(item);
}
}
Message::System { content } => {
chars += content.len();
}
}
}

Expand Down Expand Up @@ -381,6 +384,11 @@ fn render_messages_as_transcript(messages: &[Message]) -> String {
}
}
}
Message::System { content } => {
output.push_str("System: ");
output.push_str(content);
output.push('\n');
}
}
}

Expand Down
5 changes: 5 additions & 0 deletions src/agent/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,10 @@ impl Worker {
}
}
}
rig::message::Message::System { content } => {
let _ = writeln!(log, "[{index}] System:");
let _ = writeln!(log, " {content}");
}
}
}

Expand Down Expand Up @@ -1130,6 +1134,7 @@ fn build_worker_recap(messages: &[rig::message::Message]) -> String {
}
}
}
rig::message::Message::System { .. } => {}
}
}

Expand Down
17 changes: 17 additions & 0 deletions src/conversation/worker_transcript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ pub enum TranscriptStep {
/// the correct `Message::User` role instead of treating everything as
/// `Message::Assistant`.
UserText { text: String },
/// System-originated text (preamble, system prompt).
///
/// Distinct from `UserText` so that system messages preserve their role
/// when round-tripped through the transcript.
SystemText { text: String },
/// Tool execution result.
ToolResult {
call_id: String,
Expand Down Expand Up @@ -363,6 +368,13 @@ pub fn transcript_to_history(steps: &[TranscriptStep]) -> Vec<rig::message::Mess
});
}
}
TranscriptStep::SystemText { text } => {
if !text.is_empty() {
messages.push(Message::System {
content: text.clone(),
});
}
}
TranscriptStep::ToolResult {
call_id,
name: _,
Expand Down Expand Up @@ -462,6 +474,11 @@ fn convert_history(history: &[rig::message::Message]) -> Vec<TranscriptStep> {
}
}
}
rig::message::Message::System { content } => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Message::System is getting persisted as TranscriptStep::UserText, which transcript_to_history() later reconstructs as Message::User on resume. If that’s intentional (e.g., providers without a system role), a quick comment here would help avoid someone “fixing” this later and changing behavior.

steps.push(TranscriptStep::SystemText {
text: content.clone(),
});
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}

Expand Down
49 changes: 49 additions & 0 deletions src/llm/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1306,6 +1306,10 @@ pub fn convert_messages_to_anthropic(messages: &OneOrMany<Message>) -> Vec<serde
(!parts.is_empty())
.then(|| serde_json::json!({"role": "assistant", "content": parts}))
}
Message::System { content } => Some(serde_json::json!({

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small guard: if Message::System ever comes through empty (or whitespace), skipping it avoids emitting an empty Anthropic message.

Suggested change
Message::System { content } => Some(serde_json::json!({
Message::System { content } => (!content.trim().is_empty()).then(|| serde_json::json!({
"role": "user",
"content": content,
})),

"role": "user",
"content": content,
})),
})
.collect()
}
Expand Down Expand Up @@ -1367,6 +1371,12 @@ fn convert_messages_to_openai(messages: &OneOrMany<Message>) -> Vec<serde_json::

result.extend(tool_results);
}
Message::System { content } => {
result.push(serde_json::json!({

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth double-checking the source of Message::System here: stream_openai() already injects request.preamble as a role: system message. If rig v0.33 is emitting that same preamble as Message::System, this would send it twice (and demote it to user). If Message::System is meant as additional system content, mapping to role: system might preserve instruction priority.

"role": "user",
"content": content,
}));
}
Message::Assistant { content, .. } => {
let mut text_parts = Vec::new();
let mut reasoning_parts = Vec::new();
Expand Down Expand Up @@ -1486,6 +1496,12 @@ fn convert_messages_to_openai_responses(messages: &OneOrMany<Message>) -> Vec<se
}));
}
}
Message::System { content } => {
result.push(serde_json::json!({
"role": "user",
"content": content,
}));
}
Message::Assistant { content, .. } => {
let mut text_parts = Vec::new();
let mut reasoning_parts = Vec::new();
Expand Down Expand Up @@ -3068,6 +3084,39 @@ mod tests {
assert!(error.to_string().contains("stop_reason: max_tokens"));
}

#[test]
fn convert_messages_to_anthropic_maps_system_to_user_role() {
let messages = OneOrMany::one(Message::System {
content: "You are a helpful assistant".to_string(),
});
let converted = convert_messages_to_anthropic(&messages);
assert_eq!(converted.len(), 1);
assert_eq!(converted[0]["role"], "user");
assert_eq!(converted[0]["content"], "You are a helpful assistant");
}

#[test]
fn convert_messages_to_openai_maps_system_to_user_role() {
let messages = OneOrMany::one(Message::System {
content: "You are a helpful assistant".to_string(),
});
let converted = convert_messages_to_openai(&messages);
assert_eq!(converted.len(), 1);
assert_eq!(converted[0]["role"], "user");
assert_eq!(converted[0]["content"], "You are a helpful assistant");
}

#[test]
fn convert_messages_to_openai_responses_maps_system_to_user_role() {
let messages = OneOrMany::one(Message::System {
content: "You are a helpful assistant".to_string(),
});
let converted = convert_messages_to_openai_responses(&messages);
assert_eq!(converted.len(), 1);
assert_eq!(converted[0]["role"], "user");
assert_eq!(converted[0]["content"], "You are a helpful assistant");
}

#[test]
fn parse_openai_response_handles_content_array_parts() {
let body = serde_json::json!({
Expand Down
3 changes: 3 additions & 0 deletions src/tools/worker_inspect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ impl Tool for WorkerInspectTool {
worker_transcript::TranscriptStep::UserText { text } => {
summary.push_str(&format!("**User:** {text}\n\n"));
}
worker_transcript::TranscriptStep::SystemText { text } => {
summary.push_str(&format!("**System:** {text}\n\n"));
}
worker_transcript::TranscriptStep::ToolResult {
name, text, ..
} => {
Expand Down
Loading