fix(frontend): select the earliest matching stop sequence

[bzsuni]

Overview

When multiple stop strings match in one decoded fragment, the Rust frontend selects the first match in the request's stop list instead of the earliest match in the text.

For example, decoding there with stop: ["re", "he"] returns the, while reversing the list returns t. The first ordering leaves the earlier "he" stop sequence in the returned text.

This change selects the stop match with the earliest text position. If multiple stop strings match at the same position, it keeps the existing list-order precedence.

Details

Decoder::step now compares match offsets before truncating the decoded text and reporting the selected stop string.

Reproduction

A local Rust check used a local copy of the DeepSeek-R1-Distill-Qwen-1.5B tokenizer, which encoded there as the single token 18532, and passed it through the actual Decoder with stop strings excluded from output:

Stop list	Before	After
["re", "he"]	the	t
["he", "re"]	t	t

This exercised the tokenizer and decoder without model inference or an HTTP server.

The regression test in test_stop_behavior.rs uses ["re", "he"] in both include_stop_str_in_output modes, checking the output text and stop reason.

Validation

The new regression test fails before the fix and passes afterward. The full stop-behavior test target also passes: 16 passed.

cargo test --locked --offline -j 4 -p dynamo-llm --no-default-features \
  --test test_stop_behavior

Related Issues

🚫 This PR is NOT linked to an issue:

• Confirmed — no related issue

Summary by CodeRabbit

• Bug Fixes
  • Stop sequences are now selected based on their earliest occurrence in generated text, regardless of their configured order.
  • When multiple stop sequences match at the same position, configured list order determines which one is selected.
  • Hidden stop sequences no longer appear in released output, while visible stop sequences remain included as configured.
  • Output handling remains consistent with log-probability alignment.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[github-actions]

👋 Hi bzsuni! Thank you for contributing to ai-dynamo/dynamo.

Just a reminder: The NVIDIA Test Github Validation CI runs an essential subset of the testing framework to quickly catch errors.Your PR reviewers may elect to test the changes comprehensively before approving your changes.

🚀

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d1d6e7d4-bb53-43a4-baec-804ba22b7e41

📥 Commits

Reviewing files that changed from the base of the PR and between b6a5b70 and f234f8a.

📒 Files selected for processing (2)

• lib/llm/src/backend.rs
• lib/llm/tests/test_stop_behavior.rs

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

---

Walkthrough

Decoder::step now selects the earliest matching stop sequence instead of the first configured match. Tests cover precedence for visible and hidden stop sequences.

Changes

Stop sequence precedence

Layer / File(s)	Summary
Earliest stop-match selection lib/llm/src/backend.rs, lib/llm/tests/test_stop_behavior.rs	Decoder::step evaluates all matching hidden and visible stop sequences. It selects the earliest match and preserves configured order for ties. Hidden matches exclude the stop text from released output. Visible matches include it. Tests cover both stop-list orders and visibility modes.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to f234f

The updated stop precedence behavior has no identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the main change: selecting the earliest matching stop sequence.
Description check	✅ Passed	The description explains the bug, the implementation, regression coverage, validation results, and related-issue status. It does not include the optional “Where should the reviewer start?” section, bu…

---

_{Comment @coderabbitai help to get the list of available commands.}

[rmccorm4]

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]

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.