Skip to content

streamfix #2530

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aaronvg merged 4 commits into from
Sep 27, 2025
Merged

streamfix #2530

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1a80bef
checkpoint
aaronvg Sep 27, 2025
f75ef09
Fix streaming not running the parser, vscode image rendering issue, o…
aaronvg Sep 27, 2025
fbf45fc
Merge branch 'canary' into streamfix
aaronvg Sep 27, 2025
2181a28
fix more tests
aaronvg Sep 27, 2025
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
288 changes: 171 additions & 117 deletions engine/baml-runtime/src/internal/llm_client/orchestrator/stream.rs
View file
Open in desktop

Large diffs are not rendered by default.

138 changes: 130 additions & 8 deletions engine/baml-runtime/src/internal/llm_client/primitive/openai/openai_client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,23 @@ impl ProviderStrategy {
.iter()
.map(|part| match part {
ChatMessagePart::Text(text) => {
let content_type = if msg.role == "assistant" {
"output_text"
} else {
"input_text"
};
Ok(json!({
"type": if msg.role == "assistant" { "output_text" } else { "input_text" },
"type": content_type,
"text": text
}))
}
ChatMessagePart::Media(media) => {
// For assistant role, we only support text outputs in Responses API
if msg.role == "assistant" {
anyhow::bail!(
"BAML internal error (openai-responses): assistant messages must be text; media not supported for assistant in Responses API"
);
}
match media.media_type {
baml_types::BamlMediaType::Image => {
let image_url = match &media.content {
Expand All @@ -165,7 +176,8 @@ impl ProviderStrategy {
}
};
Ok(json!({
"type": if msg.role == "assistant" { "output_image" } else { "input_image" },
"type": "input_image",
"detail": "auto",
"image_url": image_url
}))
}
Expand All @@ -177,7 +189,7 @@ impl ProviderStrategy {
.strip_prefix("audio/")
.unwrap_or(&mime_type);
Ok(json!({
"type": if msg.role == "assistant" { "output_audio" } else { "input_audio" },
"type": "input_audio",
"input_audio": {
"data": b64_media.base64,
"format": format
Expand All @@ -193,7 +205,7 @@ impl ProviderStrategy {
match &media.content {
baml_types::BamlMediaContent::Url(url_content) => {
Ok(json!({
"type": if msg.role == "assistant" { "output_file" } else { "input_file" },
"type": "input_file",
"file_url": url_content.url
}))
}
Expand All @@ -202,8 +214,9 @@ impl ProviderStrategy {
}
baml_types::BamlMediaContent::Base64(b64_media) => {
Ok(json!({
"type": if msg.role == "assistant" { "output_file" } else { "input_file" },
"file_data": format!("data:{};base64,{}", media.mime_type_as_ok()?, b64_media.base64)
"type": "input_file",
"file_data": format!("data:{};base64,{}", media.mime_type_as_ok()?, b64_media.base64),
"filename": "document.pdf"
}))
}
}
Expand All @@ -217,13 +230,23 @@ impl ProviderStrategy {
// Recursively handle the inner part, ignoring metadata for now
match inner_part.as_ref() {
ChatMessagePart::Text(text) => {
let content_type = if msg.role == "assistant" {
"output_text"
} else {
"input_text"
};
Ok(json!({
"type": if msg.role == "assistant" { "output_text" } else { "input_text" },
"type": content_type,
"text": text
}))
}
ChatMessagePart::Media(media) => {
// Handle media same as above - could refactor into helper function
if msg.role == "assistant" {
anyhow::bail!(
"BAML internal error (openai-responses): assistant messages must be text; media not supported for assistant in Responses API"
);
}
match media.media_type {
baml_types::BamlMediaType::Image => {
let image_url = match &media.content {
Expand All @@ -236,7 +259,8 @@ impl ProviderStrategy {
}
};
Ok(json!({
"type": if msg.role == "assistant" { "output_image" } else { "input_image" },
"type": "input_image",
"detail": "auto",
"image_url": image_url
}))
}
Expand Down Expand Up @@ -916,4 +940,102 @@ mod tests {
let endpoint = strategy.get_endpoint("https://api.openai.com/v1", true);
assert_eq!(endpoint, "https://api.openai.com/v1/completions");
}

#[test]
fn test_responses_api_builds_input_message_with_text_and_file() {
let strategy = ProviderStrategy::ResponsesApi;

// Properties include model
let mut props = BamlMap::new();
props.insert("model".into(), json!("gpt-5-mini"));

// Build a user message with text and file (PDF url)
let msg = RenderedChatMessage {
role: "user".to_string(),
allow_duplicate_role: false,
parts: vec![
ChatMessagePart::Text("what is in this file?".to_string()),
ChatMessagePart::Media(baml_types::BamlMedia::url(
BamlMediaType::Pdf,
"https://www.berkshirehathaway.com/letters/2024ltr.pdf".to_string(),
Some("application/pdf".to_string()),
)),
],
};

// chat_converter is not used in ResponsesApi branch; construct a minimal client
let responses_client = OpenAIClient {
name: "test".to_string(),
provider: "openai-responses".to_string(),
retry_policy: None,
context: RenderContext_Client {
name: "test".to_string(),
provider: "openai-responses".to_string(),
default_role: "user".to_string(),
allowed_roles: vec!["user".to_string(), "assistant".to_string()],
remap_role: HashMap::new(),
options: IndexMap::new(),
},
features: ModelFeatures {
chat: true,
completion: false,
max_one_system_prompt: false,
resolve_audio_urls: ResolveMediaUrls::Always,
resolve_image_urls: ResolveMediaUrls::Never,
resolve_pdf_urls: ResolveMediaUrls::Never,
resolve_video_urls: ResolveMediaUrls::Never,
allowed_metadata: AllowedRoleMetadata::All,
},
properties: ResolvedOpenAI {
base_url: "https://api.openai.com/v1".to_string(),
api_key: None,
role_selection: RolesSelection::default(),
allowed_metadata: AllowedRoleMetadata::All,
supported_request_modes: SupportedRequestModes::default(),
headers: IndexMap::new(),
properties: BamlMap::new(),
query_params: IndexMap::new(),
proxy_url: None,
finish_reason_filter: FinishReasonFilter::All,
client_response_type: ResponseType::OpenAIResponses,
},
client: reqwest::Client::new(),
};

let body_value = strategy
.build_body(either::Either::Right(&[msg]), &props, &responses_client)
.expect("should build body");

let obj = body_value.as_object().expect("body should be an object");
assert_eq!(obj.get("model"), Some(&json!("gpt-5-mini")));

let input = obj
.get("input")
.and_then(|v| v.as_array())
.expect("input should be array");
assert_eq!(input.len(), 1);

let first_msg = input[0].as_object().expect("message should be object");
assert_eq!(first_msg.get("role"), Some(&json!("user")));
let content = first_msg
.get("content")
.and_then(|v| v.as_array())
.expect("content should be array");
assert_eq!(content.len(), 2);

// Validate text part
let t = content[0].as_object().expect("text part object");
assert_eq!(t.get("type"), Some(&json!("input_text")));
assert_eq!(t.get("text"), Some(&json!("what is in this file?")));

// Validate file part
let f = content[1].as_object().expect("file part object");
assert_eq!(f.get("type"), Some(&json!("input_file")));
assert_eq!(
f.get("file_url"),
Some(&json!(
"https://www.berkshirehathaway.com/letters/2024ltr.pdf"
))
);
}
}
2 changes: 1 addition & 1 deletion engine/baml-runtime/src/internal/llm_client/primitive/stream_request.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub async fn make_stream_request(
std::future::ready(event.as_ref().is_ok_and(|e| e.data != "[DONE]"))
})
.map(|event| -> Result<serde_json::Value> { Ok(serde_json::from_str(&event?.data)?) })
.inspect(|event| log::trace!("{event:#?}"))
.inspect(|event| log::debug!("{event:#?}"))
.scan(
Ok(LLMCompleteResponse {
client: client_name.clone(),
Expand Down
4 changes: 3 additions & 1 deletion engine/baml-runtime/src/internal/llm_client/primitive/vertex/response_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,9 @@ pub fn scan_vertex_response_stream(
let inner = match accumulated {
Ok(accumulated) => accumulated,
// We'll just keep the first error and return it
Err(e) => return Ok(()),
Err(e) => {
return Ok(());
}
};

let event = VertexResponse::deserialize(&event_body)
Expand Down
12 changes: 11 additions & 1 deletion engine/baml-runtime/src/internal/llm_client/primitive/vertex/vertex_client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,10 @@ impl WithStreamChat for VertexClient {
self,
either::Either::Right(prompt),
Some(self.properties.model.clone()),
ResponseType::Vertex,
match self.properties.anthropic_version {
Some(ref anthropic_version) => ResponseType::Anthropic,
None => ResponseType::Vertex,
},
ctx,
)
.await
Expand Down Expand Up @@ -350,6 +353,13 @@ impl RequestBuilder for VertexClient {
}
}

// If this is an Anthropic-on-Vertex request and streaming is enabled, add `stream: true`
// to the JSON body to mirror Anthropic API behavior.
// See docs here: https://console.cloud.google.com/vertex-ai/publishers/anthropic/model-garden/claude-3-5-sonnet?authuser=1&hl=en&project=gloo-ai
if stream && self.properties.anthropic_version.is_some() {
json_body.insert("stream".into(), json!(true));
}

let req = req.json(&json_body);

Ok(req)
Expand Down
2 changes: 1 addition & 1 deletion engine/baml-runtime/src/tracingv2/publisher/publisher.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ impl TracePublisher {
{
Ok(response) => response,
Err(e) => {
tracing::error!("Failed to check BAML source upload status: {}", e);
tracing::warn!("Failed to check BAML source upload status: {}", e);
Copy link

@entelligence-ai-pr-reviews entelligence-ai-pr-reviews bot Sep 27, 2025

Choose a reason for hiding this comment

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

correctness: process_baml_src_upload_impl downgrades all BAML source upload check failures to warn, which may hide critical errors (e.g., authentication failures) that should abort publishing and not be silently ignored.

🤖 AI Agent Prompt for Cursor/Windsurf

📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue

In engine/baml-runtime/src/tracingv2/publisher/publisher.rs, line 550, the code downgrades all errors from the BAML source upload check to a warning. This can hide critical issues (like authentication failures) that should abort publishing. Change the log level from warn back to error to ensure critical failures are not silently ignored.
📝 Committable Code Suggestion

‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
tracing::warn!("Failed to check BAML source upload status: {}", e);
tracing::error!("Failed to check BAML source upload status: {}", e);

return Err(e.into());
}
};
Expand Down
Loading
Loading