feat: add finish reason = tool_calls for stream=False and phi-4 detect token start fix

[elyasmnvidian]

Overview:

For stream=False, when there is a tool call the finish reason is currently "stop" when it should be "tool_calls"

Summary by CodeRabbit

• New Features

  • None

• Bug Fixes

  • Correctly reports the finish reason as “ToolCalls” when tool calls are present in streamed responses; otherwise preserves the original finish reason.
  • Ensures tool call data is attached to choices when present and remains absent when not provided.

• Tests

  • Added comprehensive unit tests covering finish reason behavior across scenarios and validating tool call parsing and content consistency.

[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.

[coderabbitai]

Walkthrough

Adds finish_reason override logic in DeltaChoice -> ChatChoice conversion to set ToolCalls when non-empty tool_calls are present; otherwise preserves the original finish_reason. Introduces comprehensive unit tests covering override and non-override scenarios, tool_call parsing, and content consistency. No public API changes.

Changes

Cohort / File(s)	Summary
OpenAI chat_completions aggregator logic lib/llm/src/protocols/openai/chat_completions/aggregator.rs	Modify From to override finish_reason to ToolCalls when delta.tool_calls is Some and non-empty; otherwise keep existing finish_reason. Add unit tests covering override cases, preservation when None/empty, and tool_call parsing/attachment to ChatChoice.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Caller
  participant Aggregator as Aggregator (DeltaChoice -> ChatChoice)
  participant Delta as DeltaChoice
  participant Chat as ChatChoice

  Caller->>Aggregator: convert(Delta)
  Aggregator->>Delta: inspect tool_calls and finish_reason
  alt tool_calls present and non-empty
    note over Aggregator: Override finish_reason = ToolCalls
  else tool_calls absent or empty
    note over Aggregator: Preserve delta.finish_reason
  end
  Aggregator->>Chat: build ChatChoice (message, tool_calls, finish_reason)
  Aggregator-->>Caller: ChatChoice

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat: Tool calling support stream=True #2932 — Adjusts OpenAI chat_completions aggregation for per-delta tool_call handling and ChatChoice construction, including finish_reason logic.
• refactor: refactored using Choice and CompletionFinishReason #1635 — Updates finish_reason representation and conversion in OpenAI protocol aggregators, similar to From handling here.

Poem

A rabbit taps keys with a whiskery grin,
“ToolCalls it is, when the tools hop in!”
If none appear, we keep the sign,
The finish_reason stays aligned.
Tests burrow deep, carrots in sight—
Choices aggregated, tidy and right. 🥕

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The provided description only fills the "Overview" and does not follow the repository template: it is missing the required "Details", "Where should the reviewer start?", and "Related Issues" sections and omits information about which files were changed, the tests added, and any verification steps.	Please update the PR description to follow the template by adding a "Details" section that describes the code changes (e.g., lib/llm/src/protocols/openai/chat_completions/aggregator.rs and the new unit tests and the finish_reason override), a "Where should the reviewer start?" section listing files and tests to inspect, a "Related Issues" line (for example "closes #xxx" if applicable), and brief testing/verification steps and compatibility notes (including that no public API signatures changed).

✅ Passed checks (2 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title Check	✅ Passed	The title clearly describes the primary change—setting the finish reason to "tool_calls" for stream=False—which matches the PR objectives and the DeltaChoice -> ChatChoice modification in the diff; however it also mentions "phi-4 detect token start fix," which is not reflected in the provided raw_summary, so the title combines two distinct items.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

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.}

[ayushag-nv]

lgtm !

[rmccorm4]

need to fix clippy checks in failed github actions