fix: have frontend populate "stop" field in OpenAI request

[furionw]

Overview:

Documented in #4755

When frontend receives an OpenAI request,

• it does deserialize it correctly so that NvCreateCompletionRequest.inner.stop has it
• it doesn't implement get_stop

because of this, the Preprocessor is not able to build the request with proper stop field. Therefore the backend cannot honor the OpenAI request stop.

This PR fixes that.

Details:

Update get_stop function to return the values for the (backend) request builder.

Testing

• added unit test
• test locally by running frontend and engine

Summary by CodeRabbit

• Bug Fixes

  • Enhanced handling of stop sequences in completion requests to properly process various stop parameter formats.

• Tests

  • Added comprehensive unit tests for stop parameter extraction.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[furionw]

copy from #4771

[coderabbitai]

Walkthrough

Implemented Stop extraction logic in get_stop for NvCreateCompletionRequest, handling both String and StringArray variants from the Stop type. Added corresponding unit tests covering None, single, and multiple stop scenarios, following existing test patterns.

Changes

Cohort / File(s)	Summary
Stop Extraction Implementation lib/llm/src/protocols/openai/completions.rs	Implemented Stop decoding logic in get_stop method for NvCreateCompletionRequest; maps String to vec![s], StringArray to array, returns None for absent stops. Added unit tests for none, single, and multiple stop cases.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• Stop extraction logic follows straightforward mapping patterns (String, StringArray)
• Unit tests mirror established test conventions
• Changes localized to single file with no public API modifications
• Consider verifying comprehensive coverage of Stop type variants

Poem

🐰 A stop here, a stop there,
Extracting logic with utmost care,
One or many, now they're clear,
Tests ensure no bugs appear! 🎯

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main fix: implementing the 'stop' field population in OpenAI requests at the frontend level.
Description check	✅ Passed	The description covers all required template sections with sufficient detail about the issue, changes made, and testing performed.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

lib/llm/src/protocols/openai/completions.rs (2)

240-245: Stop conditions correctly propagated from inner request

The new get_stop implementation cleanly maps the optional stop field into Option<Vec<String>>, handling both single-string and array forms and returning None when unset. This matches the behavior used in chat completions and should allow the backend to honor OpenAI stop values as intended. As a minor future improvement, you could consider extracting a small shared helper for this mapping to avoid duplication with the chat completions implementation, but it's not required for this fix.

---

502-532: New stop tests look correct and cover key shapes

test_stop exercises the important cases: no stop, a single string, and multiple stops, and the expectations align with the new get_stop behavior. This should guard against regressions in stop propagation. If you want slightly broader coverage later, you could add a case for an empty stop array, but it's not necessary for this PR.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5588d77 and 24240a2.

📒 Files selected for processing (1)

• lib/llm/src/protocols/openai/completions.rs (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

lib/llm/src/protocols/openai/completions.rs (3)

lib/llm/src/protocols/openai/responses.rs (1)

• get_stop (146-148)

lib/llm/src/protocols/openai/chat_completions.rs (1)

• get_stop (230-235)

lib/llm/src/protocols/openai.rs (1)

• get_stop (58-58)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)

• GitHub Check: trtllm (amd64)
• GitHub Check: trtllm (arm64)
• GitHub Check: sglang (amd64)
• GitHub Check: sglang (arm64)
• GitHub Check: operator (arm64)
• GitHub Check: operator (amd64)
• GitHub Check: vllm (arm64)
• GitHub Check: vllm (amd64)
• GitHub Check: Build and Test - dynamo
• GitHub Check: tests (launch/dynamo-run)
• GitHub Check: tests (lib/runtime/examples)
• GitHub Check: tests (.)
• GitHub Check: tests (lib/bindings/python)
• GitHub Check: clippy (lib/bindings/python)
• GitHub Check: clippy (launch/dynamo-run)
• GitHub Check: clippy (.)