Skip to content

Fix finish_reason logic in Azure AI client streaming response#6963

Merged
ekzhu merged 4 commits intomicrosoft:mainfrom
litterzhang:users/weizhang3/fix-azure-ai-client-finish-reason-bug
Sep 16, 2025
Merged

Fix finish_reason logic in Azure AI client streaming response#6963
ekzhu merged 4 commits intomicrosoft:mainfrom
litterzhang:users/weizhang3/fix-azure-ai-client-finish-reason-bug

Conversation

@litterzhang
Copy link
Contributor

Summary

This PR fixes a bug in the Azure AI client where the finish_reason logic for TOOL_CALLS was being processed after a potential None check, which could cause a mismatch when choice is None.

Changes

  • Moved the finish_reason logic for TOOL_CALLS from lines 557-558 to be part of the choice processing block (after line 525)
  • This ensures the finish_reason is properly set when choice exists and prevents potential mismatches

Bug Description

The original code had lines 557-558 checking \choice.finish_reason\ after line 554 where \ inish_reason\ could be None and throw an exception. The logic should be moved to within the loop where \choice\ is guaranteed to exist, specifically after line 525 where \ inish_reason\ is initially set.

Testing

  • Code compiles without errors
  • Unit tests pass (if applicable)

Fixes the finish_reason mismatch bug when choice can be None in streaming responses.

@litterzhang litterzhang marked this pull request as ready for review August 22, 2025 04:24
@litterzhang
Copy link
Contributor Author

Hi, could any contributors review this PR ? Without this change, I have to change the local code when using azure_ai_client.

@codecov
Copy link

codecov bot commented Sep 16, 2025

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 80.95%. Comparing base (c469fc0) to head (37df5e8).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...t/src/autogen_ext/models/azure/_azure_ai_client.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6963   +/-   ##
=======================================
  Coverage   80.95%   80.95%           
=======================================
  Files         237      237           
  Lines       18258    18258           
=======================================
  Hits        14780    14780           
  Misses       3478     3478           
Flag Coverage Δ
unittests 80.95% <50.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
@ekzhu ekzhu enabled auto-merge (squash) September 16, 2025 09:40
@ekzhu ekzhu disabled auto-merge September 16, 2025 09:48
@ekzhu ekzhu merged commit 6f67b95 into microsoft:main Sep 16, 2025
72 of 73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants