Skip to content

Mkulakow/legacy pipeline finish reason#4169

Open
michalkulakowski wants to merge 3 commits intomainfrom
mkulakow/legacy_pipeline_finish_reason
Open

Mkulakow/legacy pipeline finish reason#4169
michalkulakowski wants to merge 3 commits intomainfrom
mkulakow/legacy_pipeline_finish_reason

Conversation

@michalkulakowski
Copy link
Copy Markdown
Collaborator

🛠 Summary

JIRA/Issue if applicable.
Describe the changes.

🧪 Checklist

  • Unit tests added.
  • The documentation updated.
  • Change follows security best practices.
    ``

Copilot AI review requested due to automatic review settings April 29, 2026 13:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates OVMS legacy LLM pipeline response handling to propagate the actual GenAI finish_reason (instead of always reporting stop), and bumps OpenVINO/GenAI dependency pins to a newer nightly.

Changes:

  • Propagate legacy pipeline finish_reason into the final streamed chunk and into OpenAI Chat/Completions unary responses.
  • Enable finish-reason checking for the legacy LLM flow test case.
  • Update OpenVINO / OpenVINO GenAI source pins and nightly package versions (including export-model requirements).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
versions.mk Updates OV/GenAI commit pins and GenAI nightly package URLs to 2026.2.0.0.dev20260428.
src/test/llm/llmnode_test.cpp Enables finish-reason validation for lm_legacy_regular in parameterized tests.
src/llm/language_model/legacy/servable.cpp Uses results.finish_reasons[0] (when available) for the final streaming chunk finish reason.
src/llm/apis/openai_completions.cpp Switches unary finish_reason serialization to use results.finish_reasons[...] for Encoded/VLM results.
demos/common/export_models/requirements.txt Bumps openvino / openvino-tokenizers nightly versions to 2026.2.0.dev20260428.

Comment on lines 409 to 418
for (int i = 0; i < results.tokens.size(); i++) {
const std::vector<int64_t>& tokens = results.tokens[i];
SPDLOG_LOGGER_TRACE(llm_calculator_logger, "Generated tokens: {}", tokens);
ParsedOutput parsedOutput = parseOutputIfNeeded(tokens);
jsonResponse.StartObject();
// finish_reason: "stop" in regular scenario, "tool_calls" if output contains tool calls
auto finishReason = mapFinishReason(ov::genai::GenerationFinishReason::STOP, !parsedOutput.toolCalls.empty());
auto finishReason = mapFinishReason(results.finish_reasons[index], !parsedOutput.toolCalls.empty());
jsonResponse.FinishReason(finishReason.value_or("unknown"));
// index: integer; Choice index, only n=1 supported anyway
jsonResponse.Index(index++);
jsonResponse.StartObject();
// finish_reason: "stop" in regular scenario, "tool_calls" if output contains tool calls
auto finishReason = mapFinishReason(ov::genai::GenerationFinishReason::STOP, !parsedOutput.toolCalls.empty());
auto finishReason = mapFinishReason(results.finish_reasons[index], !parsedOutput.toolCalls.empty());
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants