Skip to content
Closed
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
89 changes: 48 additions & 41 deletions lib/llm/src/backend.rs

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.

Hi @bzsuni, can you share more context / motivation on this change?

vLLM and SGLang behave differently on this functionality.

Before this PR, dynamo's stop sequence behavior fully matches sglang's stop sequence behavior across the board, but only matches vllm behavior on some of the cases, see below:

For decoded fragment "there", cells show the selected stop string:

Stop list Dynamo before PR Dynamo after PR SGLang 0.5.19 vLLM 0.28.0
["her", "he"] "her" "her" "her" "he"
["he", "her"] "he" "he" "he" "he"
["re", "here"] "re" "here" "re" "re"
["here", "re"] "here" "here" "here" "here"
["here", "the"] "here" "the" "here" "the"

After this PR, dynamo behavior does not fully match either vllm or sglang, it is a hybrid of the two.

I'm not sure if this is a net win, so I'm looking for more context/motivation from you on the change here to better understand the need.

@bzsuni bzsuni Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the detailed comparison, @rmccorm4. I looked into this more and I think my original assumption in this PR was wrong.

Dynamo already processes engine output token-by-token in

dynamo/lib/llm/src/backend.rs

Lines 1043 to 1072 in 0951489

pub fn process_token_ids(&mut self, token_ids: &[TokenIdType]) -> Result<SeqResult> {
let mut text: Option<String> = None;
let mut tokens = Vec::with_capacity(token_ids.len());
for token_id in token_ids {
let StepResult {
token,
released_text,
stop_trigger,
} = self.step(*token_id)?;
// `text` accumulates the caller-visible content, which can lag behind and later
// release more than one step's worth at once. `tokens[i]` always reports
// token_ids[i]'s own decoded text, independent of that withholding, so per-token
// consumers (logprobs) stay aligned with token_ids.
if let Some(released_text) = &released_text {
text.get_or_insert_with(|| String::with_capacity(token_ids.len()))
.push_str(released_text);
}
tokens.push(token);
if let Some(stop_trigger) = stop_trigger {
return Ok(SeqResult {
tokens,
text,
stop_trigger: Some(stop_trigger),
});
}
}
it loops over token_ids, calls step() for each token, and returns as soon as one triggers a stop. So the speculative-decoding issue that motivated vLLM#49391 doesn't directly apply here.

The remaining Dynamo case is much narrower: multiple stop strings matching inside the decoded text of a single token. SGLang doesn't really define this case either — its detokenizer still has an explicit TODO for multiple stop strings being hit

So the earliest-start rule in this PR was based on my incorrect assumption. I don't see enough correctness benefit to justify changing Dynamo's existing semantics here, so I'm going to close this. Thanks for catching it.

Original file line number Diff line number Diff line change
Expand Up @@ -860,51 +860,58 @@ impl Decoder {
let release_start = self.jail.len() - self.jailed_bytes;
self.jail.push_str(token_text);

// Select the earliest match, preserving stop-list order for ties.
// Check hidden stop sequences first (excluded from output)
for seq in &self.hidden_stop_sequences {
if let Some(offset) = galil_seiferas::gs_find(self.jail.as_bytes(), seq.as_bytes())
{
// return only new bytes after release_start .. offset (excluding stop sequence)
// example: seq = "ox", token = "boxes", return "b"
//
// we might have returned a partial match, if so, then offset < release_start
// in that case, we return no text
let partial_token = (offset >= release_start)
.then(|| self.jail[release_start..offset].to_string())
.filter(|s| !s.is_empty());
self.jailed_bytes = 0;
// `token` (this step's own raw decoded text) is reported unchanged so
// that SeqResult.tokens[i] keeps describing token_ids[i] for logprobs;
// only the caller-visible `released_text` excludes the matched sequence.
// `token_text`'s last use was the `push_str` above, so `token` itself
// (not yet borrowed at this point) can move here instead of cloning.
return Ok(StepResult::with_stop_trigger(
token,
partial_token,
StopTrigger::HiddenStopSequenceDetected(seq.to_string()),
));
}
if let Some((seq, offset)) = self
.hidden_stop_sequences
.iter()
.filter_map(|seq| {
galil_seiferas::gs_find(self.jail.as_bytes(), seq.as_bytes())
.map(|offset| (seq, offset))
})
.min_by_key(|(_, offset)| *offset)
{
// return only new bytes after release_start .. offset (excluding stop sequence)
// example: seq = "ox", token = "boxes", return "b"
//
// we might have returned a partial match, if so, then offset < release_start
// in that case, we return no text
let partial_token = (offset >= release_start)
.then(|| self.jail[release_start..offset].to_string())
.filter(|s| !s.is_empty());
self.jailed_bytes = 0;
// `token` (this step's own raw decoded text) is reported unchanged so
// that SeqResult.tokens[i] keeps describing token_ids[i] for logprobs;
// only the caller-visible `released_text` excludes the matched sequence.
return Ok(StepResult::with_stop_trigger(
token,
partial_token,
StopTrigger::HiddenStopSequenceDetected(seq.to_string()),
));
}

// Check visible stop sequences (included in output)
for seq in &self.visible_stop_sequences {
if let Some(offset) = galil_seiferas::gs_find(self.jail.as_bytes(), seq.as_bytes())
{
// For visible stop sequences, include the stop string in the output
// Return all text from release_start up to and including the stop sequence
let stop_end = offset + seq.len();
let token_with_stop = (stop_end > release_start)
.then(|| self.jail[release_start..stop_end].to_string())
.filter(|s| !s.is_empty());
self.jailed_bytes = 0;
// Same reasoning as the hidden-sequence branch above: `token` can move
// here instead of cloning.
return Ok(StepResult::with_stop_trigger(
token,
token_with_stop,
StopTrigger::VisibleStopSequenceDetected(seq.to_string()),
));
}
if let Some((seq, offset)) = self
.visible_stop_sequences
.iter()
.filter_map(|seq| {
galil_seiferas::gs_find(self.jail.as_bytes(), seq.as_bytes())
.map(|offset| (seq, offset))
})
.min_by_key(|(_, offset)| *offset)
{
// For visible stop sequences, include the stop string in the output
// Return all text from release_start up to and including the stop sequence
let stop_end = offset + seq.len();
let token_with_stop = (stop_end > release_start)
.then(|| self.jail[release_start..stop_end].to_string())
.filter(|s| !s.is_empty());
self.jailed_bytes = 0;
return Ok(StepResult::with_stop_trigger(
token,
token_with_stop,
StopTrigger::VisibleStopSequenceDetected(seq.to_string()),
));
}

// No complete match. Withhold the longest tail of `jail` that is still a viable
Expand Down
22 changes: 22 additions & 0 deletions lib/llm/tests/test_stop_behavior.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,3 +344,25 @@ fn hidden_stop_sequence_survives_self_similar_prefix_run() {
"one token report per input token id"
);
}

#[test]
fn earliest_stop_sequence_wins_over_list_order() {
for include_stop_str in [false, true] {
let mut decoder = make_decoder(None, None, None, Some(vec!["re", "he"]), include_stop_str);
let result = decoder.process_token_ids(&[THERE]).unwrap();

assert_eq!(
result.text.as_deref(),
Some(if include_stop_str { "the" } else { "t" }),
);
match result.stop_trigger {
Some(StopTrigger::VisibleStopSequenceDetected(stop)) if include_stop_str => {
assert_eq!(stop, "he");
}
Some(StopTrigger::HiddenStopSequenceDetected(stop)) if !include_stop_str => {
assert_eq!(stop, "he");
}
trigger => panic!("unexpected stop trigger: {trigger:?}"),
}
}
}
Loading