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
31 changes: 26 additions & 5 deletions crates/symphony/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ enum Call {
Json(json::Assembler),
Tagged(tagged::Assembler),
Dsml(tagged::dsml::Assembler),
Keyed(tagged::keyed::Assembler),
}

impl Call {
Expand All @@ -111,6 +112,9 @@ impl Call {
Some(CallSyntax::Json) | None => Self::Json(json::Assembler::new(index, id)),
Some(CallSyntax::Tagged) => Self::Tagged(tagged::Assembler::new(index, id)),
Some(CallSyntax::Dsml) => Self::Dsml(tagged::dsml::Assembler::new(index, id)),
Some(CallSyntax::Keyed(tags)) => {
Self::Keyed(tagged::keyed::Assembler::new(index, id, tags))
}
}
}

Expand All @@ -122,6 +126,10 @@ impl Call {
assembler.feed(text, out);
text.len()
}
Self::Keyed(assembler) => {
assembler.feed(text, declared, out);
text.len()
}
}
}

Expand All @@ -130,14 +138,16 @@ impl Call {
Self::Json(assembler) => assembler.started(),
Self::Tagged(assembler) => assembler.started(),
Self::Dsml(assembler) => assembler.started(),
Self::Keyed(assembler) => assembler.started(),
}
}

/// Ends the call the way the region closed, and says whether the terminal that closed it was
/// taken as the call's end. The JSON assembler has one ending, and the engine names a block its
/// terminal closed (`close_call`); the Qwen tagged one closes the call for the client when the
/// block ended, and leaves it open when the stream was cut; the DSML one takes the invoke's
/// closing tag, and only that terminal, as `ToolCallEnd`'s bytes.
/// closing tag, and only that terminal, as `ToolCallEnd`'s bytes; the keyed one takes the
/// call's closing marker.
fn end(self, closed: Closed, terminal: &str, out: &mut Events) -> bool {
match (self, closed) {
(Self::Json(assembler), _) => assembler.finish(out),
Expand All @@ -155,6 +165,11 @@ impl Call {
}
(Self::Dsml(assembler), Closed::ByMarker) => assembler.close("", out),
(Self::Dsml(assembler), Closed::ByEnd) => assembler.finish(out),
(Self::Keyed(assembler), Closed::ByMarker) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Nit: The rebase onto the updated #2841 left this arm passing every terminal to the keyed assembler as the call's end. The base fix (c9234c9 / 3c548eb) stopped doing that for DSML, "so no call's end holds the next call's opening and no structural marker is reported as malformed when no call had started".

Ling and IQuest both have .transition("call", "call_open", "call"). So when a <tool_call> arrives inside an open call, it reaches this arm with terminal = "<tool_call>". Two cases:

  • A started call. For <tool_call>f\n<arg_key>a</arg_key>\n<arg_value>1</arg_value>\n<tool_call>g\n</tool_call>, call 0 ends with ToolCallEnd { index: 0, source: "<tool_call>" }. The next call's opener, and its token count through tokens.relabel, are attributed to the previous call's end.
  • A block with no name. For <tool_call>\n<tool_call>g\n</tool_call>, keyed::Assembler::close appends the terminal to carried and reports it as Malformed(WITHOUT_A_NAME) (keyed.rs:212–214). The chat adapter treats Malformed text as content (adapt/chat.rs:45, :150), so the client receives a literal <tool_call> in content next to the real call.

Byte conservation still holds, so contract.rs doesn't catch this. But the doc this push edited above ("the keyed one takes the call's closing marker") and the doc on keyed::Assembler::close ("terminal is the closing marker the engine read") don't hold for these two tables. Hy4 isn't affected, because only call_close leaves its call state.

One fix, matching DSML: when the terminal is the next call's opening, close the call with no bytes and return false, so the engine drops the terminal as Wrapper. terminal() already knows next, and self.format.emits(next) == Emits::Arguments means the terminal opens another call. close_call / end could take that as a flag:

(Self::Keyed(assembler), Closed::ByMarker) if !opens_next => {
    assembler.close(terminal, out);
    return true;
}
(Self::Keyed(assembler), Closed::ByMarker) => assembler.close("", out),

A Ling contract case such as <tool_call>f\n<arg_key>city</arg_key>\n<arg_value>a</arg_value>\n<tool_call>\n<tool_call>get_weather\n</tool_call> would cover both shapes.

assembler.close(terminal, out);
return true;
}
(Self::Keyed(assembler), Closed::ByEnd) => assembler.finish(out),
}
false
}
Expand Down Expand Up @@ -319,12 +334,18 @@ impl Engine {
let Some(call) = self.call.take() else {
return false;
};
if call.started() {
self.calls += 1;
}
let started_before = call.started();
let mut finished = Events::new();
let taken = call.end(closed, terminal, &mut finished);
for event in finished.drain() {
let finished = finished.drain();
// A keyed call with no arguments is named only at its end, so the end's events count too.
let started_at_the_end = finished
.iter()
.any(|event| matches!(event, Event::ToolCallStart { .. }));
if started_before || started_at_the_end {
self.calls += 1;
}
for event in finished {
let event = match event {
Event::Malformed {
text,
Expand Down
7 changes: 7 additions & 0 deletions crates/symphony/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
//! [`Engine`]: crate::Engine
//! [`formats`]: crate::formats

use crate::tagged::keyed;

/// How the model writes a call between the call markers.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum CallSyntax {
Expand All @@ -44,6 +46,11 @@ pub enum CallSyntax {
/// the state is the invoke tag's opening, and the one that leaves it is the invoke's closing
/// tag, which the call's end carries.
Dsml,
/// The call's name as text, then `<arg_key>` and `<arg_value>` pairs, in the family's spelling
/// of the four tags, typed by the request's tools: GLM, Hy4, Ling, IQuest. The terminals that
/// enter and leave the state are the call's own markers, and the call's end carries the
/// closing one.
Keyed(keyed::Tags),
}

/// What the text inside a state is.
Expand Down
241 changes: 241 additions & 0 deletions crates/symphony/src/formats/hy4.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,241 @@
//! Hy4: reasoning between `<think:opensource>` and `</think:opensource>`, the turn's calls between
//! `<tool_calls:opensource>` and `</tool_calls:opensource>`, each call between
//! `<tool_call:opensource>` and `</tool_call:opensource>` as its name and keyed arguments in Hy4's
//! spelling of the tags ([`keyed::Tags::HY4`]), typed by the request's tools. Everything else is
//! content. The prompt opens the thought, and an empty thought writes `</think:opensource>` at
//! once. A turn opens with `<|hy_start:opensource|>assistant<|hy_middle:opensource|>`, so the
//! prompt's replay starts there. Recorded as `hy4-preview` (tencent/Hy4-preview).

use crate::{
format::{CallSyntax, Emits, Format},
tagged::keyed,
};

/// The Hy4 table.
pub fn hy4() -> Format {
Format::new("hy4")
.terminal("think_open", "<think:opensource>")
.terminal("think_close", "</think:opensource>")
.terminal("calls_open", "<tool_calls:opensource>")
.terminal("calls_close", "</tool_calls:opensource>")
.terminal("call_open", "<tool_call:opensource>")
.terminal("call_close", "</tool_call:opensource>")
.state("content", Emits::Content)
.state("reasoning", Emits::Reasoning)
.state("calls", Emits::Wrapper)
.state("call", Emits::Arguments)
.transition("content", "think_open", "reasoning")
.transition("reasoning", "think_close", "content")
.transition("content", "calls_open", "calls")
.transition("calls", "call_open", "call")
.transition("call", "call_close", "calls")
.transition("calls", "calls_close", "content")
.calls(CallSyntax::Keyed(keyed::Tags::HY4))
.opens_turn("<|hy_start:opensource|>assistant<|hy_middle:opensource|>")
}

#[cfg(test)]
mod tests {
use openai_protocol::common::{Function, Tool};
use serde_json::json as value;

use super::*;
use crate::{
engine::Engine,
event::{Event, Events},
input::{EngineFinish, Input},
parser::Parser,
tagged::Declared,
};

fn declared() -> Declared {
Declared::of(&[Tool {
tool_type: "function".to_string(),
function: Function {
name: "get_weather".to_string(),
description: None,
parameters: value!({"type": "object", "properties": {
"city": {"type": "string"},
"days": {"type": "integer"},
}}),
strict: None,
},
}])
}

const PROMPT: &str = "<think:opensource>\n";
const OUTPUT: &str = concat!(
"The user asks.</think:opensource><tool_calls:opensource>",
"<tool_call:opensource>get_weather<arg_key:opensource>city</arg_key:opensource>",
"<arg_value:opensource>Paris</arg_value:opensource><arg_key:opensource>days",
"</arg_key:opensource><arg_value:opensource>3</arg_value:opensource>",
"</tool_call:opensource>",
"<tool_call:opensource>get_weather</tool_call:opensource></tool_calls:opensource>"
);

fn run(prompt: &str, output: &str) -> Vec<Event> {
let mut parser = Engine::new(hy4(), declared());
let mut out = Events::new();
parser
.feed(
Input::Prompt {
token_ids: &[],
text: prompt,
},
&mut out,
)
.expect("prompt");
parser
.feed(
Input::Delta {
token_ids: &[],
text: output,
spans: &[],
},
&mut out,
)
.expect("delta");
parser
.feed(
Input::End {
finish: EngineFinish::Stop,
},
&mut out,
)
.expect("end");
out.drain()
}

fn bytes(events: &[Event]) -> String {
events
.iter()
.map(|e| match e {
Event::Content(t) | Event::Reasoning(t) => t.text.as_str(),
Event::Dropped { text, .. } | Event::Malformed { text, .. } => text.text.as_str(),
Event::ToolCallStart { source, .. }
| Event::ToolCallArguments { source, .. }
| Event::ToolCallEnd { source, .. } => source.text.as_str(),
Event::ReasoningStart | Event::ReasoningEnd | Event::Finish { .. } => "",
})
.collect()
}

fn arguments(events: &[Event]) -> Vec<(String, String)> {
let mut calls: Vec<(String, String)> = Vec::new();
for event in events {
match event {
Event::ToolCallStart { name, .. } => calls.push((name.clone(), String::new())),
Event::ToolCallArguments { json, .. } => {
if let Some(last) = calls.last_mut() {
last.1.push_str(json);
}
}
_ => {}
}
}
calls
}

#[test]
fn the_prompt_is_replayed_from_hy4s_own_turn_opener() {
Comment thread
slin1237 marked this conversation as resolved.
// A call marker quoted in the user's turn, then the generation prompt opening the thought:
// the replay starts at Hy4's own turn opener, so the output is the thought and then
// content (smg #2842, Alex's probe with the rendered prompt).
let prompt = "<|hy_start:opensource|>user<|hy_middle:opensource|>Why did you print \
<tool_calls:opensource> there?<|hy_start:opensource|>assistant\
<|hy_middle:opensource|><think:opensource>\n";
let output = "The user asks about the tag.</think:opensource>It opens a call.";
let events = run(prompt, output);
assert_eq!(events[0], Event::ReasoningStart);
assert_eq!(bytes(&events), output);
assert!(arguments(&events).is_empty());
let reasoning: String = events
.iter()
.filter_map(|event| match event {
Event::Reasoning(text) => Some(text.text.as_str()),
_ => None,
})
.collect();
let content: String = events
.iter()
.filter_map(|event| match event {
Event::Content(text) => Some(text.text.as_str()),
_ => None,
})
.collect();
assert_eq!(reasoning, "The user asks about the tag.");
assert_eq!(content, "It opens a call.");
}

#[test]
fn a_recorded_output_gives_its_calls_typed_by_the_tools_and_every_byte() {
let events = run(PROMPT, OUTPUT);
assert_eq!(bytes(&events), OUTPUT);
assert_eq!(
arguments(&events),
[
(
"get_weather".to_string(),
r#"{"city": "Paris", "days": 3}"#.to_string()
),
("get_weather".to_string(), "{}".to_string()),
]
);
assert!(matches!(
events.last(),
Some(Event::Finish { tool_calls: 2, .. })
));
let reasoning: String = events
.iter()
.filter_map(|event| match event {
Event::Reasoning(text) => Some(text.text.as_str()),
_ => None,
})
.collect();
assert_eq!(reasoning, "The user asks.");
}

#[test]
fn every_chunking_says_the_same_and_accounts_for_every_byte() {
let whole = arguments(&run(PROMPT, OUTPUT));
for cut in 1..OUTPUT.len() {
if !OUTPUT.is_char_boundary(cut) {
continue;
}
let mut parser = Engine::new(hy4(), declared());
let mut out = Events::new();
parser
.feed(
Input::Prompt {
token_ids: &[],
text: PROMPT,
},
&mut out,
)
.expect("prompt");
for piece in [&OUTPUT[..cut], &OUTPUT[cut..]] {
parser
.feed(
Input::Delta {
token_ids: &[],
text: piece,
spans: &[],
},
&mut out,
)
.expect("delta");
}
parser
.feed(
Input::End {
finish: EngineFinish::Stop,
},
&mut out,
)
.expect("end");
let events = out.drain();
assert_eq!(bytes(&events), OUTPUT, "cut at {cut}");
assert_eq!(arguments(&events), whole, "cut at {cut}");
}
}
}
Loading
Loading