Skip to content

Commit a37cda9

Browse files
MaanavDCopilot
andcommitted
rust: address responses API review feedback
- Make responses_client/responses_types modules private; expose only the re-exported public surface (matching audio_client/chat_client/etc). - Introduce ResponseCreateOptions for per-call overrides so callers no longer need to materialize a full ResponseCreateRequest; ResponseCreateRequest stays as the wire-serialized request body. - Move SSE parser tests inline into responses_client.rs so they exercise parse_sse_stream directly instead of duplicating the framing logic in tests/unit. - Skip vision_image_base64 only when FOUNDRY_VISION_MODEL_ID is unset; when set, surface real failures so regressions are caught. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 815c0db commit a37cda9

8 files changed

Lines changed: 173 additions & 211 deletions

File tree

sdk/rust/examples/responses.rs

Lines changed: 6 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -107,27 +107,11 @@ async fn main() -> Result<()> {
107107
.await?;
108108
println!("Turn 1: {}", first.output_text());
109109

110-
let follow_up_opts = foundry_local_sdk::ResponseCreateRequest {
111-
model: model.info().id.clone(),
112-
input: ResponseInput::Text("What is my favourite number?".into()),
110+
let follow_up_opts = foundry_local_sdk::ResponseCreateOptions {
113111
previous_response_id: Some(first.id.clone()),
114-
instructions: None,
115-
tools: None,
116-
tool_choice: None,
117-
stream: None,
118112
store: Some(true),
119113
temperature: Some(0.0),
120-
top_p: None,
121-
max_output_tokens: None,
122-
frequency_penalty: None,
123-
presence_penalty: None,
124-
seed: None,
125-
truncation: None,
126-
parallel_tool_calls: None,
127-
metadata: None,
128-
user: None,
129-
reasoning: None,
130-
text: None,
114+
..Default::default()
131115
};
132116

133117
let second = client
@@ -155,27 +139,12 @@ async fn main() -> Result<()> {
155139
strict: None,
156140
};
157141

158-
let tool_opts = foundry_local_sdk::ResponseCreateRequest {
159-
model: model.info().id.clone(),
160-
input: ResponseInput::Text("What is 123 + 456? Use the add tool.".into()),
142+
let tool_opts = foundry_local_sdk::ResponseCreateOptions {
161143
tools: Some(vec![add_tool]),
162144
tool_choice: Some(json!("required")),
163-
instructions: None,
164-
previous_response_id: None,
165-
stream: None,
166145
store: Some(true),
167146
temperature: Some(0.0),
168-
top_p: None,
169-
max_output_tokens: None,
170-
frequency_penalty: None,
171-
presence_penalty: None,
172-
seed: None,
173-
truncation: None,
174-
parallel_tool_calls: None,
175-
metadata: None,
176-
user: None,
177-
reasoning: None,
178-
text: None,
147+
..Default::default()
179148
};
180149

181150
let tool_response = client
@@ -208,27 +177,11 @@ async fn main() -> Result<()> {
208177
status: None,
209178
}]);
210179

211-
let final_opts = foundry_local_sdk::ResponseCreateRequest {
212-
model: model.info().id.clone(),
213-
input: result_input.clone(),
180+
let final_opts = foundry_local_sdk::ResponseCreateOptions {
214181
previous_response_id: Some(tool_response.id.clone()),
215-
instructions: None,
216-
tools: None,
217-
tool_choice: None,
218-
stream: None,
219182
store: Some(true),
220183
temperature: Some(0.0),
221-
top_p: None,
222-
max_output_tokens: None,
223-
frequency_penalty: None,
224-
presence_penalty: None,
225-
seed: None,
226-
truncation: None,
227-
parallel_tool_calls: None,
228-
metadata: None,
229-
user: None,
230-
reasoning: None,
231-
text: None,
184+
..Default::default()
232185
};
233186

234187
let final_response = client.create(result_input, Some(final_opts)).await?;

sdk/rust/src/foundry_local_manager.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use crate::configuration::{Configuration, FoundryLocalConfig, Logger};
1313
use crate::detail::core_interop::CoreInterop;
1414
use crate::detail::ModelLoadManager;
1515
use crate::error::{FoundryLocalError, Result};
16-
use crate::openai::responses_client::ResponsesClient;
16+
use crate::openai::ResponsesClient;
1717
use crate::types::{EpDownloadResult, EpInfo};
1818

1919
/// Global singleton holder — only stores a successfully initialised manager.

sdk/rust/src/lib.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ pub use async_openai::types::chat::{
4848
pub use crate::openai::{
4949
Annotation, DeleteResponseResult, FunctionToolDefinition, IncompleteDetails,
5050
InputItemsListResponse, InputTokensDetails, ListResponsesOptions, ListResponsesResult, LogProb,
51-
MessageContent, OutputTokensDetails, ReasoningConfig, ResponseCreateRequest, ResponseError,
52-
ResponseInput, ResponseItem, ResponseObject, ResponseUsage, ResponsesClient,
53-
ResponsesClientSettings, ResponsesContentPart, SseStream, StreamingEvent, TextConfig,
54-
TextFormat,
51+
MessageContent, OutputTokensDetails, ReasoningConfig, ResponseCreateOptions,
52+
ResponseCreateRequest, ResponseError, ResponseInput, ResponseItem, ResponseObject,
53+
ResponseUsage, ResponsesClient, ResponsesClientSettings, ResponsesContentPart, SseStream,
54+
StreamingEvent, TextConfig, TextFormat,
5555
};

sdk/rust/src/openai/mod.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ mod chat_client;
33
mod embedding_client;
44
mod json_stream;
55
mod live_audio_client;
6-
pub mod responses_client;
7-
pub mod responses_types;
6+
mod responses_client;
7+
mod responses_types;
88

99
pub use self::audio_client::{
1010
AudioClient, AudioClientSettings, AudioTranscriptionResponse, AudioTranscriptionStream,
@@ -22,6 +22,6 @@ pub use self::responses_types::{
2222
Annotation, ContentPart as ResponsesContentPart, DeleteResponseResult, FunctionToolDefinition,
2323
IncompleteDetails, InputItemsListResponse, InputTokensDetails, ListResponsesOptions,
2424
ListResponsesResult, LogProb, MessageContent, OutputTokensDetails, ReasoningConfig,
25-
ResponseCreateRequest, ResponseError, ResponseInput, ResponseItem, ResponseObject,
26-
ResponseUsage, StreamingEvent, TextConfig, TextFormat,
25+
ResponseCreateOptions, ResponseCreateRequest, ResponseError, ResponseInput, ResponseItem,
26+
ResponseObject, ResponseUsage, StreamingEvent, TextConfig, TextFormat,
2727
};

sdk/rust/src/openai/responses_client.rs

Lines changed: 95 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ use crate::error::{FoundryLocalError, Result};
1515

1616
use super::responses_types::{
1717
DeleteResponseResult, FunctionToolDefinition, InputItemsListResponse, ListResponsesResult,
18-
ReasoningConfig, ResponseCreateRequest, ResponseInput, ResponseObject, StreamingEvent,
19-
TextConfig,
18+
ReasoningConfig, ResponseCreateOptions, ResponseCreateRequest, ResponseInput, ResponseObject,
19+
StreamingEvent, TextConfig,
2020
};
2121

2222
// ============================================================================
@@ -159,7 +159,7 @@ impl ResponsesClient {
159159
pub async fn create(
160160
&self,
161161
input: ResponseInput,
162-
options: Option<ResponseCreateRequest>,
162+
options: Option<ResponseCreateOptions>,
163163
) -> Result<ResponseObject> {
164164
self.validate_input(&input)?;
165165
if let Some(ref opts) = options {
@@ -193,7 +193,7 @@ impl ResponsesClient {
193193
pub async fn create_streaming(
194194
&self,
195195
input: ResponseInput,
196-
options: Option<ResponseCreateRequest>,
196+
options: Option<ResponseCreateOptions>,
197197
) -> Result<SseStream> {
198198
self.validate_input(&input)?;
199199
if let Some(ref opts) = options {
@@ -340,13 +340,13 @@ impl ResponsesClient {
340340
fn build_request(
341341
&self,
342342
input: ResponseInput,
343-
options: Option<ResponseCreateRequest>,
343+
options: Option<ResponseCreateOptions>,
344344
stream: bool,
345345
) -> Result<ResponseCreateRequest> {
346346
// Determine model: options override self.model_id
347347
let model = options
348348
.as_ref()
349-
.map(|o| o.model.clone())
349+
.and_then(|o| o.model.clone())
350350
.filter(|m| !m.trim().is_empty())
351351
.or_else(|| self.model_id.clone())
352352
.ok_or_else(|| FoundryLocalError::Validation {
@@ -383,11 +383,9 @@ impl ResponsesClient {
383383

384384
// Apply per-call overrides
385385
if let Some(opts) = options {
386-
if !opts.model.trim().is_empty() {
387-
req.model = opts.model;
386+
if let Some(m) = opts.model.filter(|m| !m.trim().is_empty()) {
387+
req.model = m;
388388
}
389-
// Only override input if the caller passed an options object with explicit input;
390-
// in practice options.input will always be overwritten by the positional `input`.
391389
if let Some(v) = opts.instructions {
392390
req.instructions = Some(v);
393391
}
@@ -635,3 +633,90 @@ where
635633
}
636634
}
637635
}
636+
637+
// ============================================================================
638+
// Inline tests
639+
// ============================================================================
640+
//
641+
// These tests live alongside `parse_sse_stream` so they exercise the real
642+
// implementation rather than reimplementing SSE framing in an external test
643+
// crate. Anything that only depends on public APIs lives in `tests/unit/`.
644+
645+
#[cfg(test)]
646+
mod tests {
647+
use super::*;
648+
use async_stream::stream;
649+
650+
/// Drive `parse_sse_stream` from a hand-constructed byte stream and collect
651+
/// its yielded events.
652+
async fn collect_events(chunks: Vec<&'static str>) -> Vec<StreamingEvent> {
653+
let byte_stream = stream! {
654+
for chunk in chunks {
655+
yield Ok::<Bytes, reqwest::Error>(Bytes::from_static(chunk.as_bytes()));
656+
}
657+
};
658+
659+
let parsed = parse_sse_stream(byte_stream);
660+
let mut parsed = std::pin::pin!(parsed);
661+
662+
let mut events = Vec::new();
663+
use tokio_stream::StreamExt as _;
664+
while let Some(event) = parsed.next().await {
665+
events.push(event.expect("SSE event failed to parse"));
666+
}
667+
events
668+
}
669+
670+
#[tokio::test]
671+
async fn parses_complete_event_block() {
672+
let payload = "data: {\"type\":\"response.output_text.delta\",\"item_id\":\"i1\",\
673+
\"output_index\":0,\"content_index\":0,\"delta\":\"Hi\",\"sequence_number\":1}\n\n\
674+
data: [DONE]\n\n";
675+
676+
let events = collect_events(vec![payload]).await;
677+
assert_eq!(events.len(), 1);
678+
assert!(matches!(
679+
events[0],
680+
StreamingEvent::OutputTextDelta { ref delta, .. } if delta == "Hi"
681+
));
682+
}
683+
684+
#[tokio::test]
685+
async fn done_signal_terminates_stream() {
686+
let payload = "data: [DONE]\n\n\
687+
data: {\"type\":\"response.output_text.delta\",\"item_id\":\"i1\",\
688+
\"output_index\":0,\"content_index\":0,\"delta\":\"after-done\",\
689+
\"sequence_number\":2}\n\n";
690+
691+
let events = collect_events(vec![payload]).await;
692+
assert!(events.is_empty(), "events after [DONE] must be ignored");
693+
}
694+
695+
#[tokio::test]
696+
async fn handles_event_split_across_chunks() {
697+
// Split a single SSE block across two byte chunks to make sure the
698+
// parser buffers correctly.
699+
let part1 = "data: {\"type\":\"response.output_text.delta\",\
700+
\"item_id\":\"i1\",\"output_index\":0,\"content_index\":0,";
701+
let part2 = "\"delta\":\"split\",\"sequence_number\":3}\n\ndata: [DONE]\n\n";
702+
703+
let events = collect_events(vec![part1, part2]).await;
704+
assert_eq!(events.len(), 1);
705+
assert!(matches!(
706+
events[0],
707+
StreamingEvent::OutputTextDelta { ref delta, .. } if delta == "split"
708+
));
709+
}
710+
711+
#[tokio::test]
712+
async fn skips_event_lines_and_blank_blocks() {
713+
let payload = "event: response.output_text.delta\n\
714+
data: {\"type\":\"response.output_text.delta\",\"item_id\":\"i1\",\
715+
\"output_index\":0,\"content_index\":0,\"delta\":\"ok\",\"sequence_number\":4}\n\n\
716+
\n\n\
717+
data: [DONE]\n\n";
718+
719+
let events = collect_events(vec![payload]).await;
720+
assert_eq!(events.len(), 1);
721+
}
722+
}

sdk/rust/src/openai/responses_types.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,40 @@ pub struct ResponseCreateRequest {
279279
pub text: Option<TextConfig>,
280280
}
281281

282+
/// Per-call overrides for [`ResponsesClient::create`] and
283+
/// [`ResponsesClient::create_streaming`].
284+
///
285+
/// Every field is optional — the client merges these on top of
286+
/// [`ResponsesClientSettings`] and the constructor-supplied model. Unlike
287+
/// [`ResponseCreateRequest`] (the wire-serialised request body), this type is
288+
/// intended purely as caller-friendly input and never needs to be fully
289+
/// populated.
290+
///
291+
/// [`ResponsesClient::create`]: crate::ResponsesClient::create
292+
/// [`ResponsesClient::create_streaming`]: crate::ResponsesClient::create_streaming
293+
/// [`ResponsesClientSettings`]: crate::ResponsesClientSettings
294+
#[derive(Debug, Clone, Default)]
295+
pub struct ResponseCreateOptions {
296+
pub model: Option<String>,
297+
pub instructions: Option<String>,
298+
pub previous_response_id: Option<String>,
299+
pub tools: Option<Vec<FunctionToolDefinition>>,
300+
pub tool_choice: Option<Value>,
301+
pub store: Option<bool>,
302+
pub temperature: Option<f32>,
303+
pub top_p: Option<f32>,
304+
pub max_output_tokens: Option<u32>,
305+
pub frequency_penalty: Option<f32>,
306+
pub presence_penalty: Option<f32>,
307+
pub seed: Option<u32>,
308+
pub truncation: Option<String>,
309+
pub parallel_tool_calls: Option<bool>,
310+
pub metadata: Option<HashMap<String, String>>,
311+
pub user: Option<String>,
312+
pub reasoning: Option<ReasoningConfig>,
313+
pub text: Option<TextConfig>,
314+
}
315+
282316
// ============================================================================
283317
// Response Object
284318
// ============================================================================

0 commit comments

Comments
 (0)