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
42 changes: 42 additions & 0 deletions crates/skippy-server/src/frontend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ mod prefix_cache;
mod prompting;
mod request;
mod speculative;
mod tool_emulation;
mod util;

#[cfg(test)]
use prompting::parse_emulated_chat_output;
mod wire_messages;

use self::{
Expand Down Expand Up @@ -1775,6 +1779,7 @@ struct SplitMultimodalGeneration<'a> {
downstream_wire_condition: WireCondition,
lane_pool: Arc<PersistentStageLanePool>,
prediction_return: Option<PredictionReturnReceiver>,
emulation_active: bool,
}

struct EmbeddedLocalOutput {
Expand Down Expand Up @@ -1990,6 +1995,27 @@ enum TokenControl {
Stop,
}

/// Whether generation for this request is running under tool-call emulation:
/// the request carries tools and the rendered prompt metadata does not report
/// native tool support (empty grammar triggers). Used to enable early
/// generation stop once a complete emulated tool call has been produced.
fn emulation_generation_active(
hook_request: Option<&ChatCompletionRequest>,
prompt: &PreparedGenerationPrompt,
) -> bool {
let Some(request) = hook_request else {
return false;
};
if !tool_calls_requested(request) {
return false;
}
prompt
.chat_parse_metadata
.as_deref()
.map(|metadata| !tool_emulation::template_supports_native_tool_calls(metadata))
.unwrap_or(false)
}

struct TextGenerationCollector<'a, F>
where
F: FnMut(&str) -> OpenAiResult<()>,
Expand All @@ -2004,6 +2030,7 @@ where
completion_tokens: usize,
finish_reason: FinishReason,
metrics: GenerationMetrics,
emulation_active: bool,
}

impl<'a, F> TextGenerationCollector<'a, F>
Expand All @@ -2027,9 +2054,19 @@ where
completion_tokens: 0,
finish_reason: finish_reason_for_generation(true),
metrics: GenerationMetrics::default(),
emulation_active: false,
}
}

/// Enables early generation stop once a complete emulated tool call is
/// generated. Mirrors goose's `tool_call_emitted -> Stop` so the model does
/// not ramble after emitting a call. Only active for emulated tools
/// requests; native requests are unaffected.
fn with_emulation_stop(mut self, emulation_active: bool) -> Self {
self.emulation_active = emulation_active;
self
}

fn push_token(&mut self, token: i32) -> OpenAiResult<TokenControl> {
let eog_timer = Instant::now();
if token_is_eog_with_runtime(&self.runtime, token)? {
Expand Down Expand Up @@ -2066,6 +2103,11 @@ where
self.finish_reason = finish_reason_for_generation(false);
return Ok(TokenControl::Stop);
}
if self.emulation_active && tool_emulation::emulated_tool_call_complete(&self.text) {
self.emit_safe_delta(true)?;
self.finish_reason = finish_reason_for_generation(false);
return Ok(TokenControl::Stop);
}
self.emit_safe_delta(false)?;
Ok(TokenControl::Continue)
}
Expand Down
13 changes: 10 additions & 3 deletions crates/skippy-server/src/frontend/generation_flow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,10 @@ impl StageOpenAiBackend {
};
let chat_sampling_metadata = prompt.chat_parse_metadata.as_deref();

let emulation_active = emulation_generation_active(hook_request.as_ref(), &prompt);
let mut collector =
TextGenerationCollector::new(self.runtime.clone(), stop_values, on_text_chunk);
TextGenerationCollector::new(self.runtime.clone(), stop_values, on_text_chunk)
.with_emulation_stop(emulation_active);
let cache_stats = match self.mode.clone() {
OpenAiBackendMode::LocalRuntime => self.generate_local_tokens(
LocalGeneration {
Expand Down Expand Up @@ -225,6 +227,7 @@ impl StageOpenAiBackend {
.map(|hub| hub.register(ids.request_id, ids.session_id))
.transpose()
.map_err(openai_backend_error)?;
let emulation_active = emulation_generation_active(hook_request.as_ref(), &prompt);
return self.generate_split_multimodal_text(
SplitMultimodalGeneration {
prompt,
Expand All @@ -239,6 +242,7 @@ impl StageOpenAiBackend {
downstream_wire_condition,
lane_pool,
prediction_return,
emulation_active,
},
on_text_chunk,
);
Expand All @@ -262,6 +266,7 @@ impl StageOpenAiBackend {
.iter()
.map(String::as_str)
.collect::<Vec<_>>();
let emulation_active = emulation_generation_active(hook_request.as_ref(), &prompt);
let session_id = ids.session_label.clone();
let prefill_timer = PhaseTimer::start();
let (prefill, mut token_signal, mut signal_window) = {
Expand Down Expand Up @@ -382,7 +387,8 @@ impl StageOpenAiBackend {
}

let mut collector =
TextGenerationCollector::new(self.runtime.clone(), stop_values, on_text_chunk);
TextGenerationCollector::new(self.runtime.clone(), stop_values, on_text_chunk)
.with_emulation_stop(emulation_active);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
let result = (|| {
let decode_timer = PhaseTimer::start();
let mut decoded_tokens = 0usize;
Expand Down Expand Up @@ -567,7 +573,8 @@ impl StageOpenAiBackend {
.map(String::as_str)
.collect::<Vec<_>>();
let mut collector =
TextGenerationCollector::new(self.runtime.clone(), stop_values, on_text_chunk);
TextGenerationCollector::new(self.runtime.clone(), stop_values, on_text_chunk)
.with_emulation_stop(request.emulation_active);
let wire_sampling = wire_sampling_config(&request.sampling);
let session_id = request.ids.session_id;
let request_id = request.ids.request_id;
Expand Down
149 changes: 129 additions & 20 deletions crates/skippy-server/src/frontend/prompting.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
use super::*;

/// Output of a single chat-template application, before it is folded into a
/// [`PreparedGenerationPrompt`].
struct RenderedChatPrompt {
prompt: String,
media: Vec<MediaInput>,
metadata_json: String,
}

impl StageOpenAiBackend {
pub(super) fn prepare_chat_prompt(
&self,
Expand All @@ -13,29 +21,82 @@ impl StageOpenAiBackend {
.map_err(|_| OpenAiError::backend("runtime lock poisoned"))?;
runtime.media_marker()
};

// Native path: render with tools and use the template's own metadata.
let native = self.render_chat_prompt(request, &options, &marker, None, true)?;

// If the request carries tools but the template does not support native
// tool calling, re-render with server-side tool-call emulation: strip
// tools, inject a text-convention instruction, and rewrite history so
// the template never sees tool roles. Tool-capable templates keep the
// native prompt unchanged.
if tool_calls_requested(request)
&& tool_emulation::should_emulate_tool_calls(&native.metadata_json)
&& let Some(tools) = request.tools.as_ref()
&& let Some(instruction) = tool_emulation::build_emulation_instruction(tools)
{
let rewritten =
tool_emulation::rewrite_history_for_emulation(&request.messages, &instruction);
let emulated =
self.render_chat_prompt(request, &options, &marker, Some(&rewritten), false)?;
Comment on lines +26 to +41

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Avoid probing unsupported templates with the original tool-role history.

Line 26 renders the native prompt before emulation is selected, so non-tool-capable templates can still see role: "tool" / assistant tool_calls and fail before the rewrite at Lines 38-41 runs. Probe capability with sanitized/minimal messages or metadata first, then render either the native or rewritten prompt once.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/skippy-server/src/frontend/prompting.rs` around lines 26 - 41, The
chat prompt rendering in prompting.rs is probing template capability by
rendering the full native prompt first, which can expose unsupported tool roles
before emulation is chosen. Update the flow around render_chat_prompt,
tool_emulation::template_supports_native_tool_calls, and
tool_emulation::rewrite_history_for_emulation so capability is checked using
sanitized/minimal data or metadata before any full render, then render only once
with either the native messages or the rewritten emulation history.

Comment on lines +35 to +41

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve tool_choice semantics in the emulation instruction.

The emulation path strips tool_choice at Line 41, but build_emulation_instruction(tools) does not encode tool_choice: "required" or a forced function choice. This can let the model answer normally or call another tool even when the request requires a specific tool.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/skippy-server/src/frontend/prompting.rs` around lines 35 - 41, The
emulation path in render_chat_prompt currently rewrites history with
build_emulation_instruction(tools) but drops request.tool_choice semantics, so
the forced-tool requirement is lost. Update
tool_emulation::build_emulation_instruction (and any caller like
rewrite_history_for_emulation in prompting.rs) to encode tool_choice, especially
“required” and any explicit function selection, into the emulation instruction
so the model is constrained to the requested tool. Ensure the rewritten prompt
preserves the original tool_choice behavior instead of allowing free-form
answers or different tools.

return Ok(PreparedGenerationPrompt {
text: emulated.prompt,
media: emulated.media,
chat_parse_metadata: Some(emulated.metadata_json),
});
}

Ok(PreparedGenerationPrompt {
text: native.prompt,
media: native.media,
chat_parse_metadata: Some(native.metadata_json),
})
}

/// Applies the chat template for the given messages. When `messages` is
/// `None`, the request's own messages are used. When `include_tools` is
/// false, `tools`/`tool_choice` are omitted (used by the emulation path).
fn render_chat_prompt(
&self,
request: &ChatCompletionRequest,
options: &ChatTemplateOptions,
marker: &str,
messages: Option<&[openai_frontend::ChatMessage]>,
include_tools: bool,
) -> OpenAiResult<RenderedChatPrompt> {
let source_messages = messages.unwrap_or(&request.messages);
let mut media = Vec::new();
let template_messages = request
.messages
let template_messages = source_messages
.iter()
.map(|message| chat_message_generation_value(message, &marker, &mut media))
.map(|message| chat_message_generation_value(message, marker, &mut media))
.collect::<OpenAiResult<Vec<_>>>()?;
let messages_json = serde_json::to_string(&template_messages).map_err(|error| {
OpenAiError::invalid_request(format!("serialize messages: {error}"))
})?;
let tools_json = request
.tools
.as_ref()
.map(serde_json::to_string)
.transpose()
.map_err(|error| OpenAiError::invalid_request(format!("serialize tools: {error}")))?;
let tool_choice_json = request
.tool_choice
.as_ref()
.map(serde_json::to_string)
.transpose()
.map_err(|error| {
OpenAiError::invalid_request(format!("serialize tool_choice: {error}"))
})?;
let tools_json = if include_tools {
request
.tools
.as_ref()
.map(serde_json::to_string)
.transpose()
.map_err(|error| {
OpenAiError::invalid_request(format!("serialize tools: {error}"))
})?
} else {
None
};
let tool_choice_json = if include_tools {
request
.tool_choice
.as_ref()
.map(serde_json::to_string)
.transpose()
.map_err(|error| {
OpenAiError::invalid_request(format!("serialize tool_choice: {error}"))
})?
} else {
None
};
let runtime = self
.runtime
.lock()
Expand All @@ -53,10 +114,10 @@ impl StageOpenAiBackend {
},
)
.map_err(openai_backend_error)?;
Ok(PreparedGenerationPrompt {
text: result.prompt,
Ok(RenderedChatPrompt {
prompt: result.prompt,
media,
chat_parse_metadata: Some(result.metadata_json),
metadata_json: result.metadata_json,
})
}

Expand All @@ -70,6 +131,19 @@ impl StageOpenAiBackend {
let Some(metadata) = metadata else {
return Ok(None);
};

// Emulation path: when tools were requested but the prompt was rendered
// without native tool support, the model emitted TOOL_CALL text lines.
// Parse those into OpenAI tool_calls instead of using the native parser.
if tool_calls_requested(request)
&& !tool_emulation::template_supports_native_tool_calls(metadata)
{
if !is_partial {
eprintln!("EMU_RAW_TEXT<<<{}>>>", text);
}
return Ok(parse_emulated_chat_output(text, request, is_partial));
}

let parsed_json = {
let runtime = self
.runtime
Expand Down Expand Up @@ -204,3 +278,38 @@ impl StageOpenAiBackend {
&& hook_request.as_ref().is_some_and(chat_mesh_hooks_enabled)
}
}

/// Parses generated text produced under tool-call emulation into a
/// [`ParsedChatMessage`]. During streaming (`is_partial`) the trailing
/// incomplete line is held back and tool calls are withheld, so a half-formed
/// `TOOL_CALL` marker is never streamed as content and calls are emitted once,
/// on finalization — matching native tool-call streaming semantics.
pub(super) fn parse_emulated_chat_output(
text: &str,
request: &ChatCompletionRequest,
is_partial: bool,
) -> Option<ParsedChatMessage> {
let allowed = request_allowed_tool_names(request);
let scan_text = if is_partial {
text.rsplit_once('\n')
.map(|(head, _)| head)
.unwrap_or_default()
Comment on lines +293 to +296

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not withhold all partial prose while streaming.

This drops every trailing line during partial parsing, so a normal one-line answer streams no content until finalization. Only hold the trailing segment when it could be a partial TOOL_CALL marker; otherwise parse/emit prose normally.

Suggested shape
-    let scan_text = if is_partial {
-        text.rsplit_once('\n')
-            .map(|(head, _)| head)
-            .unwrap_or_default()
+    let scan_text = if is_partial && trailing_segment_may_be_tool_call(text) {
+        text.rsplit_once('\n').map(|(head, _)| head).unwrap_or_default()
     } else {
         text
     };
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/skippy-server/src/frontend/prompting.rs` around lines 290 - 293, The
partial parsing logic in prompting::scan_text currently strips everything after
the last newline whenever is_partial is true, which suppresses normal one-line
prose during streaming. Update the scan_text handling so it only withholds the
trailing segment when it might be an incomplete TOOL_CALL marker, and otherwise
lets prose flow through normally; use the existing is_partial path in
prompting.rs to distinguish tool-call fragments from regular text.

} else {
text
};
let parse = tool_emulation::parse_emulated_tool_calls(scan_text, &allowed);
let tool_calls = if is_partial || parse.tool_calls.is_empty() {
None
} else {
let mut calls = parse.tool_calls;
if request.parallel_tool_calls == Some(false) {
calls.truncate(1);
}
Some(Value::Array(calls))
};
Some(ParsedChatMessage {
content: parse.content,
reasoning_content: None,
tool_calls,
})
}
Loading
Loading