Skip to content

Conversation

@gn00295120
Copy link
Contributor

Summary

This PR adds defensive checks before accessing array elements in chatcmpl_stream_handler.py to prevent IndexError when reasoning content arrays are empty.

Problem Analysis

Discovered During Code Review

While scanning the codebase for potential bugs, we identified two locations where array elements are accessed without verifying the array is non-empty:

Line 163 (accessing summary[0]):

# When this runs:
current_content = state.reasoning_content_index_and_output[1].summary[0]  # Potential IndexError

Line 208 (accessing content[0]):

# Original check:
if state.reasoning_content_index_and_output[1].content is None:  # Only checks None
    state.reasoning_content_index_and_output[1].content = [...]
current_text = state.reasoning_content_index_and_output[1].content[0]  # Potential IndexError if []

Root Cause

The code handles two different reasoning formats:

OpenAI Format (line 128):

ResponseReasoningItem(
    summary=[Summary(text="", type="summary_text")],  # ✅ Has element
    type="reasoning"
)

3rd Party Format (line 176):

ResponseReasoningItem(
    summary=[],  # ❌ Empty!
    content=[Content(text="", type="reasoning_text")],
    type="reasoning"
)

Potential Issue:

  1. 3rd party API sends delta.reasoning first → initializes with summary=[], content=[...]
  2. Later, if OpenAI format sends delta.reasoning_content → tries to access summary[0]IndexError

Trigger Probability: Low (same model unlikely to mix formats), but possible with:

  • Custom model providers implementing partial OpenAI compatibility
  • API gateways that normalize responses
  • Future format changes

Solution

Add defensive checks before array access (similar to PR #935 pattern):

Change 1: Line 154 - Check summary array

# Before (unsafe):
if reasoning_content and state.reasoning_content_index_and_output:
    current_content = state.reasoning_content_index_and_output[1].summary[0]  # ❌

# After (safe):
if reasoning_content and state.reasoning_content_index_and_output:
    if not state.reasoning_content_index_and_output[1].summary:
        state.reasoning_content_index_and_output[1].summary = [
            Summary(text="", type="summary_text")
        ]
    current_content = state.reasoning_content_index_and_output[1].summary[0]  # ✅

Change 2: Line 204 - Check content array

# Before (only checks None):
if state.reasoning_content_index_and_output[1].content is None:

# After (checks both None and []):
if not state.reasoning_content_index_and_output[1].content:

Why This Fix is Correct

Defensive Programming Best Practice

Following the same philosophy as PR #935:

"just checking the existence of the data and avoiding the runtime exception should make sense"

  • Empty arrays are valid (if unexpected) states
  • SDK should not crash on edge cases
  • Graceful degradation is better than runtime errors

Maintains Existing Behavior

  • When arrays are non-empty: identical behavior
  • When arrays are empty: initialize properly instead of crashing
  • All existing tests pass

Low Risk

  • Only adds safety checks, no logic changes
  • Preserves backwards compatibility
  • Handles future format variations

Testing

Existing Tests Pass

$ uv run pytest tests/ -k "reason" -v
7 passed

$ uv run pytest tests/test_openai_chatcompletions_stream.py tests/test_stream_events.py -v
7 passed in 6.10s

Type Safety

$ uv run mypy src/agents/models/chatcmpl_stream_handler.py
Success: no issues found

Code Quality

$ make format
$ make lint
All checks passed!

Impact

Affected Scenarios:

  • Custom model providers with partial OpenAI compatibility
  • 3rd-party APIs that may mix format conventions
  • Future reasoning format changes

Risk Assessment:

  • Low risk: Only adds defensive checks
  • Non-breaking: Maintains backward compatibility
  • Proactive: Prevents potential crashes before they occur

Research Process

This fix was discovered through:

  1. Systematic code scanning for array access patterns
  2. Analysis of initialization logic for different reasoning formats
  3. Comparison with similar fixes (PR Fix #604 Chat Completion model raises runtime error when response.choices is empty #935 pattern)
  4. Testing verification across all reasoning-related tests

Related Work


Note: This PR demonstrates proactive bug prevention through careful code analysis and defensive programming, rather than waiting for user reports.

Add defensive checks before accessing array elements in reasoning content
processing to prevent IndexError when arrays are empty.

Changes:
- Line 154: Check summary array is non-empty before accessing summary[0]
- Line 204: Change 'is None' to truthiness check to handle both None and []

This prevents crashes when 3rd-party APIs initialize with empty arrays and
later receive content in OpenAI format.
@Copilot Copilot AI review requested due to automatic review settings October 22, 2025 18:56
Copy link

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 adds defensive checks before accessing array elements in the stream handler to prevent IndexError when reasoning content arrays are empty, addressing potential edge cases where different API providers may initialize arrays differently.

Key Changes:

  • Added validation to ensure summary array is non-empty before accessing summary[0]
  • Modified content array check from is None to not content to handle both None and empty array cases
Comments suppressed due to low confidence (2)

src/agents/models/chatcmpl_stream_handler.py:169

  • The defensive check added at line 154-157 protects against empty summary arrays, but there's no test coverage verifying this edge case. Consider adding a test that simulates a scenario where reasoning_content_index_and_output[1].summary is initialized as an empty list to ensure the defensive check works as intended.
                    current_content = state.reasoning_content_index_and_output[1].summary[0]

src/agents/models/chatcmpl_stream_handler.py:214

  • The change at line 210 from checking is None to not content now handles empty arrays, but there's no test coverage for this scenario. Add a test case that verifies the behavior when reasoning_content_index_and_output[1].content is an empty list to ensure the fix prevents the IndexError described in the PR.
                    current_text = state.reasoning_content_index_and_output[1].content[0]

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@seratch seratch merged commit a714cf4 into openai:main Oct 23, 2025
8 checks passed
@gn00295120 gn00295120 deleted the fix-stream-handler-empty-content-check branch October 24, 2025 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants