Skip to content

Add reasoning content - fix on #494 #871

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jun 27, 2025

Conversation

axion66
Copy link
Contributor

@axion66 axion66 commented Jun 15, 2025

my personal update on #494

Key changes:

  • Updated ResponseReasoningItem to use the correct structure with id, summary, and type="reasoning" fields
  • Fixed ResponseReasoningSummaryPartAddedEvent to include summary_index parameter and use a dictionary for the part parameter
  • Fixed ResponseReasoningSummaryTextDeltaEvent to include summary_index parameter
  • Updated ResponseReasoningSummaryPartDoneEvent to include summary_index and use a dictionary for part
  • Changed how the Summary object is accessed in tests (using .text property instead of subscript notation)
  • Updated chatcmpl_converter.py to use the Summary class instead of a dictionary for better handlign

Fixes #494

@rm-openai
Copy link
Collaborator

this looks great - can you please fix lint and i can merge?

@Betula-L
Copy link

Any progress for this PR?

@seratch seratch added documentation Improvements or additions to documentation feature:chat-completions labels Jun 25, 2025
@benjaminbascary97
Copy link

Im waiting on this to be merged :(

#940

I opened an issue if someone could help me? Sorry if this is not the place

@axion66
Copy link
Contributor Author

axion66 commented Jun 26, 2025

hi!

my bad. I didn't check the pytest version.

I will update soon

@axion66
Copy link
Contributor Author

axion66 commented Jun 26, 2025

for better readability, I also modified the tests/test_reasoning to include some functions, but it may fail some lints.. which i might need to work on

@axion66
Copy link
Contributor Author

axion66 commented Jun 26, 2025

@rm-openai
Could you try the workflow?
Thanks


Oh

I was wondering what the problem is, and found out that my tool is based on pytest, while the actual check was based on ruff (via uv run ruff). This led to automatic unsolvable pull..

Hold up for few minutes. my bad

@axion66 axion66 force-pushed the add-reasoning-content branch from dcdd038 to 39963a8 Compare June 27, 2025 08:22
@axion66
Copy link
Contributor Author

axion66 commented Jun 27, 2025

I updated the branch to up-to-date, which now should set the mandatory param "prompt" = None for functions get_response and stream_response.

@rm-openai rm-openai merged commit fcafae1 into openai:main Jun 27, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation feature:chat-completions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants