Skip to content

fix: remove stop_reason field and add deserialization test for ChatCo…#127

Merged
Evrard-Nil merged 1 commit into
mainfrom
fix/chat-completion-do-not-parse-stop-reason
Oct 31, 2025
Merged

fix: remove stop_reason field and add deserialization test for ChatCo…#127
Evrard-Nil merged 1 commit into
mainfrom
fix/chat-completion-do-not-parse-stop-reason

Conversation

@Evrard-Nil
Copy link
Copy Markdown
Collaborator

…mpletionResponse

@cursor
Copy link
Copy Markdown

cursor Bot commented Oct 31, 2025

You have run out of free Bugbot PR reviews for this billing cycle. This will reset on November 9.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

@Evrard-Nil Evrard-Nil temporarily deployed to Cloud API test env October 31, 2025 15:41 — with GitHub Actions Inactive
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

This PR removes the provider-specific stop_reason field from ChatCompletionResponseChoice to simplify the struct, relying solely on the standard finish_reason field. A new test case is added for GLM model deserialization.

  • Removed stop_reason field and its documentation from ChatCompletionResponseChoice struct
  • Added test_chat_completion_response_deserialization_glm test case demonstrating deserialization of GLM model responses
Comments suppressed due to low confidence (1)

crates/inference_providers/src/models.rs:655

  • The stop_reason field has been removed from ChatCompletionResponseChoice struct but still appears in this test's JSON. While serde ignores unknown fields during deserialization, this field should be removed from the test data to maintain consistency with the struct definition and avoid confusion.
                "stop_reason":null,

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

},
"logprobs": null,
"finish_reason": "stop",
"stop_reason": 151336,
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

The stop_reason field is included in the test JSON but has been removed from the ChatCompletionResponseChoice struct definition. This field will be silently ignored during deserialization. Consider removing it from the test data to accurately reflect the struct's schema, or add a comment explaining why it's present if testing that unknown fields are gracefully ignored.

Suggested change
"stop_reason": 151336,

Copilot uses AI. Check for mistakes.
@Evrard-Nil Evrard-Nil merged commit fb21b70 into main Oct 31, 2025
7 checks passed
@PierreLeGuen PierreLeGuen deleted the fix/chat-completion-do-not-parse-stop-reason branch October 31, 2025 16:25
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.

3 participants