Skip to content

Commit 4fb6df6

Browse files
committed
Address Copilot boundary feedback
1 parent e269137 commit 4fb6df6

5 files changed

Lines changed: 96 additions & 9 deletions

File tree

crates/calciforge/src/proxy/helicone_router_tests.rs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,58 @@ async fn chat_completion_posts_to_configured_v1_path_without_duplication() {
144144
mock.assert_async().await;
145145
}
146146

147+
#[tokio::test]
148+
async fn chat_completion_accepts_helicone_streaming_response() {
149+
let mut server = mockito::Server::new_async().await;
150+
let body = concat!(
151+
"data: {\"id\":\"chatcmpl-stream\",\"object\":\"chat.completion.chunk\",\"created\":1,\"model\":\"openai/gpt-4o-mini\",\"choices\":[{\"index\":0,\"delta\":{\"role\":\"assistant\",\"content\":\"po\"},\"finish_reason\":null}]}\n\n",
152+
"data: {\"id\":\"chatcmpl-stream\",\"object\":\"chat.completion.chunk\",\"created\":1,\"model\":\"openai/gpt-4o-mini\",\"choices\":[{\"index\":0,\"delta\":{\"content\":\"ng\"},\"finish_reason\":\"stop\"}]}\n\n",
153+
"data: [DONE]\n\n",
154+
);
155+
let mock = server
156+
.mock("POST", "/v1/chat/completions")
157+
.match_body(Matcher::PartialJson(serde_json::json!({
158+
"model": "openai/gpt-4o-mini",
159+
"stream": true
160+
})))
161+
.with_status(200)
162+
.with_header("content-type", "text/event-stream; charset=utf-8")
163+
.with_body(body)
164+
.create_async()
165+
.await;
166+
167+
let router = HeliconeRouter::new(config(format!("{}/v1/", server.url()))).unwrap();
168+
let result = router
169+
.chat_completion(
170+
"openai/gpt-4o-mini".to_string(),
171+
vec![ChatMessage {
172+
role: "user".to_string(),
173+
content: Some(MessageContent::Text("hello".to_string())),
174+
name: None,
175+
tool_calls: None,
176+
tool_call_id: None,
177+
reasoning: None,
178+
reasoning_content: None,
179+
}],
180+
true,
181+
None,
182+
None,
183+
)
184+
.await
185+
.unwrap();
186+
187+
assert_eq!(
188+
result.choices[0]
189+
.message
190+
.content
191+
.as_ref()
192+
.and_then(MessageContent::to_text)
193+
.as_deref(),
194+
Some("pong")
195+
);
196+
mock.assert_async().await;
197+
}
198+
147199
#[tokio::test]
148200
async fn chat_completion_forwards_custom_headers() {
149201
let mut server = mockito::Server::new_async().await;

crates/calciforge/src/proxy/helicone_streaming.rs

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -200,12 +200,18 @@ pub(super) fn parse_streaming_chat_completion(
200200
for (index, accumulator) in choices_by_index {
201201
let mut tool_call_entries: Vec<(u32, ToolCall)> = Vec::new();
202202
for (tool_index, call) in accumulator.tool_calls {
203-
let Some(id) = call.id else {
204-
continue;
205-
};
206-
let Some(name) = call.name else {
207-
continue;
208-
};
203+
let id = call.id.ok_or_else(|| {
204+
BackendError::InvalidResponse(format!(
205+
"Helicone streaming tool call for model '{}' did not include an id",
206+
requested_model
207+
))
208+
})?;
209+
let name = call.name.ok_or_else(|| {
210+
BackendError::InvalidResponse(format!(
211+
"Helicone streaming tool call for model '{}' did not include a function name",
212+
requested_model
213+
))
214+
})?;
209215
tool_call_entries.push((
210216
tool_index,
211217
ToolCall {
@@ -433,6 +439,22 @@ mod tests {
433439
);
434440
}
435441

442+
#[test]
443+
fn rejects_incomplete_streamed_tool_calls() {
444+
let body = concat!(
445+
"data: {\"id\":\"chatcmpl-tools\",\"object\":\"chat.completion.chunk\",\"created\":6,\"model\":\"ollama/qwen3.6:27b\",\"choices\":[{\"index\":0,\"delta\":{\"role\":\"assistant\",\"tool_calls\":[{\"index\":0,\"type\":\"function\",\"function\":{\"arguments\":\"{}\"}}]},\"finish_reason\":\"tool_calls\"}]}\n\n",
446+
"data: [DONE]\n\n",
447+
);
448+
449+
let error = parse_streaming_chat_completion(body, "ollama/qwen3.6:27b")
450+
.expect_err("missing tool call id/name should not become a successful response");
451+
452+
assert!(
453+
error.to_string().contains("did not include an id"),
454+
"unexpected error: {error}"
455+
);
456+
}
457+
436458
proptest! {
437459
#[test]
438460
fn arbitrary_stream_bodies_never_panic(body in ".*") {

scripts/check-scenarios.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,11 @@ def fail(message: str) -> None:
3030

3131
def require_nonempty_list(scenario: dict, key: str) -> None:
3232
value = scenario.get(key)
33-
if not isinstance(value, list) or not value or not all(isinstance(item, str) and item for item in value):
33+
if (
34+
not isinstance(value, list)
35+
or not value
36+
or not all(isinstance(item, str) and item.strip() for item in value)
37+
):
3438
fail(f"{scenario.get('id', '<unknown>')}: {key} must be a non-empty list of strings")
3539

3640

scripts/model-gateway-route-aggression.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,21 @@
2424

2525

2626
DEFAULT_CONFIG_PATHS = (
27-
Path(os.environ.get("CALCIFORGE_CONFIG", "")),
2827
Path.home() / ".config" / "calciforge" / "config.toml",
2928
Path("/opt/homebrew/etc/calciforge/config.toml"),
3029
Path("/etc/calciforge/config.toml"),
3130
)
3231

3332

3433
def existing_default_config() -> Path | None:
34+
env_path = os.environ.get("CALCIFORGE_CONFIG", "").strip()
35+
if env_path:
36+
path = Path(env_path)
37+
if path.is_file():
38+
return path
39+
3540
for path in DEFAULT_CONFIG_PATHS:
36-
if str(path) and path.exists():
41+
if path.is_file():
3742
return path
3843
return None
3944

scripts/ollama-model-switch.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ if truthy "$warmup_enabled" && [[ "$target_loaded" != true ]]; then
9797
keep_alive="${CALCIFORGE_OLLAMA_KEEP_ALIVE:-24h}"
9898
warmup_timeout="${CALCIFORGE_OLLAMA_WARMUP_TIMEOUT_SECONDS:-120}"
9999
warmup_ctx="${CALCIFORGE_OLLAMA_WARMUP_CONTEXT:-1024}"
100+
if [[ ! "$warmup_ctx" =~ ^[0-9]+$ ]]; then
101+
echo "warning: invalid CALCIFORGE_OLLAMA_WARMUP_CONTEXT='$warmup_ctx'; using 1024" >&2
102+
warmup_ctx=1024
103+
fi
100104
payload="$(printf \
101105
'{"model":"%s","prompt":"Reply with exactly: ready","stream":false,"keep_alive":"%s","options":{"num_ctx":%s}}\n' \
102106
"$(json_escape "$target")" \

0 commit comments

Comments
 (0)