Skip to content

Add support for Responses API #541

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

Draft
wants to merge 51 commits into
base: main
Choose a base branch
from
Draft

Conversation

momostafa
Copy link

@momostafa momostafa commented Mar 22, 2025

What:

  • New Feature

Adds the new feature Responses API: https://platform.openai.com/docs/api-reference/responses

OpenAI's most advanced interface for generating model responses. Supports text and image inputs, and text outputs. Create stateful interactions with the model, using the output of previous responses as input. Extend the model's capabilities with built-in tools for file search, web search, computer use, and more. Allow the model access to external systems and data using function calling.


Fixes: #535
Laravel PR: openai-php/laravel#147

Copy link
Collaborator

@iBotPeaches iBotPeaches left a comment

Choose a reason for hiding this comment

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

I started going through this, but it doesn't really follow the pattern at all :/

Some top level comments

  • Place the Contract into src/Contracts/Resources
  • Drop the Transporter/Payload iterations - you can reuse that logic.
  • Move the Response API into src/Resources
  • Split the Response/Resource into src/Resources & src/Responses
  • Introduce some heavy tests.

@momostafa
Copy link
Author

Thanks for reviewing the PR, I will go over it again and let you know

@iBotPeaches
Copy link
Collaborator

Can you run some of the tooling locally? Without tests and the bar having to be at 100% and some linting errors - this has a bit further to go.

composer test:lint
composer test:types
composer test:unit
composer test:type-coverage min=100

I put them in order of complexity.

@momostafa
Copy link
Author

Can you run some of the tooling locally? Without tests and the bar having to be at 100% and some linting errors - this has a bit further to go.

composer test:lint
composer test:types
composer test:unit
composer test:type-coverage min=100

I put them in order of complexity.

Please find attached test results, sorry I don't have time to fix all errors. Phpstan already gave me a lot of headaches.

test:unit.txt
test:types.txt
test:type-coverage min=100.txt
test:lint.txt

@momostafa
Copy link
Author

momostafa commented Mar 31, 2025

@iBotPeaches Please check my last commit 412f35c

with above commit I tested live all models and all are working.
sorry but I don't have time to go further if there are anything needed to be changed etc...
I hope someone else can carry on from where i finished.

Thank you for your understanding

@iBotPeaches
Copy link
Collaborator

@momostafa - thanks for your work so far. I triggered the pipeline so you can see the failures right now. I'll look for some time if you've tapped out to take this further.

@momostafa
Copy link
Author

@momostafa - thanks for your work so far. I triggered the pipeline so you can see the failures right now. I'll look for some time if you've tapped out to take this further.

You are welcome and thank you for your time looking into my PR. I am seriously overloaded but since there only 2 fails I will work on it tonight and I will get back to you.

@momostafa
Copy link
Author

@momostafa - thanks for your work so far. I triggered the pipeline so you can see the failures right now. I'll look for some time if you've tapped out to take this further.

Hi, Now test:lint pass on my local machine please check momostafa@29436e8
Thank you

@momostafa
Copy link
Author

@momostafa - thanks for your work so far. I triggered the pipeline so you can see the failures right now. I'll look for some time if you've tapped out to take this further.

Sorry... I will check the other failing

@momostafa
Copy link
Author

dd263ac
@iBotPeaches I have ran all tests several times and all passed 100% please check.
Thank you

Copy link
Collaborator

@iBotPeaches iBotPeaches left a comment

Choose a reason for hiding this comment

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

Getting closer! A few things here and there, but the major aspect I see missing is the tests. You produced some fixtures, but nothing to assert any of the work you did - works.

  • You'll need a Resource test, ie like Assistants
  • You'll need a Response test for all types of responses, ie like Assistants
  • Finally, a simple test to assert the mocking/pass-through all works with a more full body Response test - ie w/ Assistants again

@momostafa
Copy link
Author

Getting closer! A few things here and there, but the major aspect I see missing is the tests. You produced some fixtures, but nothing to assert any of the work you did - works.

  • You'll need a Resource test, ie like Assistants
  • You'll need a Response test for all types of responses, ie like Assistants
  • Finally, a simple test to assert the mocking/pass-through all works with a more full body Response test - ie w/ Assistants again

Yeah getting closer : ) thank you for your patience and detailed inputs on what's needs to be fixed. I will try to resolve it today

@iBotPeaches iBotPeaches marked this pull request as draft April 8, 2025 22:32
@iBotPeaches
Copy link
Collaborator

Yeah getting closer : ) thank you for your patience and detailed inputs on what's needs to be fixed. I will try to resolve it today

No worries, I'm excited to get this as well. Thanks for continuing to work on it. I know this is a big new endpoint, which I'll probably migrate all my Chat endpoints towards once completed.

@elnurvl
Copy link

elnurvl commented Apr 10, 2025

@momostafa , thank you for this great contribution! I am also looking forward to see this merged.

Question: Does the documentation in the description reflect the current state of this PR?

In your readme file this can be found:

// Example: Stream a response
$stream = $responses->createStreamed([
    'model' => 'gpt-4o',
    'input' => [ // Changed to input to match implementation, assuming input is not a valid parameter
        [
            'content' => 'The quick brown fox jumps over the lazy fox.',
        ],
    ],
]);

According to the Responses API specification, content receives an array of inputs(e.g. text, file), not a plain string:

curl "https://api.openai.com/v1/responses" \
    -H "Content-Type: application/json" \
    -H "Authorization: Bearer $OPENAI_API_KEY" \
    -d '{
        "model": "gpt-4o",
        "input": [
            {
                "role": "user",
                "content": [
                    {
                        "type": "input_file",
                        "filename": "draconomicon.pdf",
                        "file_data": "...base64 encoded PDF bytes here..."
                    },
                    {
                        "type": "input_text",
                        "text": "What is the first dragon in the book?"
                    }
                ]
            }
        ]
    }'

Also, it is also not mentioned, what would be the implied role if it is not given in input. user?

Updating the repository README file as part of this PR would be very helpful.

I had to modify OpenAI\Testing\Responses\Concerns\Fakeable
as    $class = str_replace('Responses\\', 'Testing\\Responses\\Fixtures\\', static::class).'Fixture';
was conflicting with newly added Responses folder and added docblock explaining the modification and tested against all files.

Updated readme can be found at README-RESPONSES.md

Added dedicated ClientFake for Responses tests/Testing/ClientFakeResponses.php
@momostafa momostafa requested a review from iBotPeaches April 12, 2025 00:01
@momostafa
Copy link
Author

@momostafa , thank you for this great contribution! I am also looking forward to see this merged.

Question: Does the documentation in the description reflect the current state of this PR?

In your readme file this can be found:

// Example: Stream a response
$stream = $responses->createStreamed([
    'model' => 'gpt-4o',
    'input' => [ // Changed to input to match implementation, assuming input is not a valid parameter
        [
            'content' => 'The quick brown fox jumps over the lazy fox.',
        ],
    ],
]);

According to the Responses API specification, content receives an array of inputs(e.g. text, file), not a plain string:

curl "https://api.openai.com/v1/responses" \
    -H "Content-Type: application/json" \
    -H "Authorization: Bearer $OPENAI_API_KEY" \
    -d '{
        "model": "gpt-4o",
        "input": [
            {
                "role": "user",
                "content": [
                    {
                        "type": "input_file",
                        "filename": "draconomicon.pdf",
                        "file_data": "...base64 encoded PDF bytes here..."
                    },
                    {
                        "type": "input_text",
                        "text": "What is the first dragon in the book?"
                    }
                ]
            }
        ]
    }'

Also, it is also not mentioned, what would be the implied role if it is not given in input. user?

Updating the repository README file as part of this PR would be very helpful.

You are most welcome, I am glad to be able to make a small contribution to the community. Sorry for the delay since last update as it was quite a challenge to pass Pest tests as finally I found that fake function at Fakeable trait.
$class = str_replace('Responses\', 'Testing\Responses\Fixtures\', static::class).'Fixture';
was conflicting with the newly created folder Responses and I had to modify the function please see full details at the docblock on top of fake function OpenAI\Testing\Responses\Concerns\Fakeable::fake

I have submitted a review and I hope it will pass this time.

Thank you
I

@momostafa
Copy link
Author

@iBotPeaches Hi, Please check last commit caf4413

  • Test:lint passed
  • Test:types passed
  • Test:type-coverage min=100 passed
  • Test:unit 12 failed that I can't solve

Please review and I appreciate if you can fix what is left so we can all benefit of using the Response API.

Thank you for your understanding and guidance through the process.

@iBotPeaches
Copy link
Collaborator

iBotPeaches commented Apr 17, 2025

This is going to be a large large pr. Making my way through it. I didn't fully understand the scope of the Responses API until I sat down and dove into it. It has support for everything (reasoning, tool call, computer calling, file searching, function call, regular text) on top of custom tools and formats.

If I had to summarize remaining work:

  • fully type the tools array
  • meld all docblock changes from child classes into main Create, Retrieve and List
  • copy changes over to retrieve/list
  • increase coverage to 100%, by adding the tests I previously removed that were trying to assert on an array

With Easter near, will lose a few days of time, but otherwise I'm hoping to get this all done & merged before April is over.

@momostafa
Copy link
Author

This is going to be a large large pr. Making my way through it. I didn't fully understand the scope of the Responses API until I sat down and dove into it. It has support for everything (reasoning, tool call, computer calling, file searching, function call, regular text) on top of custom tools and formats.

If I had to summarize remaining work:

  • fully type the tools array
  • meld all docblock changes from child classes into main Create, Retrieve and List
  • copy changes over to retrieve/list
  • increase coverage to 100%, by adding the tests I previously removed that were trying to assert on an array

With Easter near, will lose a few days of time, but otherwise I'm hoping to get this all done & merged before April is over.

Hi, I feel sorry that I overwhelmed you with the list of tasks that keeps growing.... I though it will just be fixing unit test and I was expecting that Streaming would require some changes or even total refactoring as I wasn't confident about it.
While drafting the PR I was planning to make separate classes for each tool similar to Assistant, then I decide better keep it simple as a first phase and after acceptance, Tool calls and Function calls can be added as update/updates by me and maybe by other contributors.

Please give me example class for 1 tool call and a Task list/structure of all the Tool & Functions that you want to integrate and I will do it ASAP I already digested the whole Responses API (Except streaming :) was a confusing for me how to match it with the codebase pattern.

Wish you Happy Easter in advance

@iBotPeaches
Copy link
Collaborator

Hi, I feel sorry that I overwhelmed you with the list of tasks that keeps growing.... I though it will just be fixing unit test and I was expecting that Streaming would require some changes or even total refactoring as I wasn't confident about it. While drafting the PR I was planning to make separate classes for each tool similar to Assistant, then I decide better keep it simple as a first phase and after acceptance, Tool calls and Function calls can be added as update/updates by me and maybe by other contributors.

Don't feel bad - I didn't know it was going to be large. I'm of the opinion that we should launch this with near everything typed - otherwise downstream users deal with untyped arrays and a future release gives them typed classes. Its not the best developer experience, so yeah I did increase the scope of this work to get to that fully typed setup like other endpoints have.

Please give me example class for 1 tool call and a Task list/structure of all the Tool & Functions that you want to integrate and I will do it ASAP I already digested the whole Responses API (Except streaming :) was a confusing for me how to match it with the codebase pattern.

Do you want to take ListInputItems? I won't touch that, I'm almost done with Retrieve/Create Response so I wouldn't touch those. Basically the InputItem result has this data array with a ton of types

These types are iterations of existing types (more properties or less). I would create a new folder in Responses/InputItems and make a type for each one:

  • InputMessage
  • OutputMessage
  • FileSearchToolCall
  • ComputerToolCall
  • ComputerToolCallOutput
  • WebSearchToolCall
  • FunctionToolCall
  • FunctionToolCallOutput

These types I've already made a good chunk of, but none of them seem to be a 100% match. So you can make new class objects in that folder and the array parsing can follow what you see here - you are welcome to take the parsing from any class I did that helps you type those objects quicker. (ps start with WebSearchToolCall as easiest). As any class that has inner object/arrays have to be further typed.

If you get done with that - we are basically taking all those sub-classes and writing smaller tests. See this file for an example

As you can tell in that example - it uses the existing threadMessageResource object, but only returns the FileCitation chunk to test that parsing. We will do the same thing. We create a large example of a InputItemList that has 1 every type of response, etc. We then make tiny test files for each chunk. Since the test line here about testing for an array doesn't test the parsing of each child item.

Thanks! Happy to split working on this with you. Push code often and I'll do the same. Don't worry about breaking the build for now.

Wish you Happy Easter in advance

Thanks :)

@momostafa
Copy link
Author

momostafa commented Apr 18, 2025

Thanks! Happy to split working on this with you. Push code often and I'll do the same. Don't worry about breaking the build for now.

You are welcome Connor. I will happily do all of the above and thanks for the detailed instructions. I will review all of the above tonight and start fresh from tomorrow and I will be committing after each completion of each sub class / task

@momostafa
Copy link
Author

@iBotPeaches Hi, Please check momostafa@e811af7
The remaining types will be much faster as I have already documented and prepared the related arrays till ComputerToolCallOutput

  • OutputMessage
  • FileSearchToolCall
  • ComputerToolCall
  • ComputerToolCallOutput
  • WebSearchToolCall
  • FunctionToolCall
  • FunctionToolCallOutput

@iBotPeaches
Copy link
Collaborator

@iBotPeaches Hi, Please check momostafa@e811af7 The remaining types will be much faster as I have already documented and prepared the related arrays till ComputerToolCallOutput

Looking good! I saw you fixed the camelCase already. One thing I'm not sure about is the newlines/spaces in the php array shape docblocks. It increases readability majorly, but I vaguely it remember not being supported like that in some IDE which is what forced those massive 1 line array array shape docblocks. Don't worry about changing that for now - I'll dig into that.

Comment on lines +110 to +129
$data = array_map(
function (array $item): array {
$content = array_map(
fn (array $contentItem): InputMessageContentInputText|InputMessageContentInputImage|InputMessageContentInputFile => match ($contentItem['type']) {
'input_text' => InputMessageContentInputText::from($contentItem),
'input_image' => InputMessageContentInputImage::from($contentItem),
'input_file' => InputMessageContentInputFile::from($contentItem),
},
$item['content'],
);
return [
'type' => $item['type'],
'id' => $item['id'],
'status' => $item['status'],
'role' => $item['role'],
'content' => $content,
];
},
$attributes['data'],
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@momostafa - I wouldn't expect a double array_map. I think you want to map these items into an InputMessage as that contains the inner object that would then handle the array_map of the child content items.

Copy link
Author

Choose a reason for hiding this comment

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

I totally got blind at this part by the time I got to this file my eyes was were soaring after copy/past from api docs then making my own docs then going 10X+ times over each array. I will fix that

@momostafa
Copy link
Author

@iBotPeaches Hi, Please check momostafa@e811af7 The remaining types will be much faster as I have already documented and prepared the related arrays till ComputerToolCallOutput

Looking good! I saw you fixed the camelCase already. One thing I'm not sure about is the newlines/spaces in the php array shape docblocks. It increases readability majorly, but I vaguely it remember not being supported like that in some IDE which is what forced those massive 1 line array array shape docblocks. Don't worry about changing that for now - I'll dig into that.

Glad that you found it good with any major issues. About the expanded docblocks I am aware of the limitation I will refactor once you approve on this initial start I made like that to help me verify that everything in place.

@iBotPeaches
Copy link
Collaborator

The docblocks expanded into 1 line is a pain point for this repo for sure. Been working on a little tool to make it easier, since I lose track in the massive docblock of the Response API

Screenshot 2025-04-20 at 9 16 34 AM

I'll deploy this somewhere once its perfect if it helps people out.

@momostafa
Copy link
Author

momostafa commented Apr 20, 2025

I made a comparison between array properties and type hints of below Classes:
Responses/Output/OutputMessage.php
Responses/Responses/Output/OutputMessageContentOutputText.php
Responses/Responses/Output/OutputMessageContentOutputTextAnnotationsFileCitation.php
Responses/Responses/Output/OutputMessageContentOutputTextAnnotationsFilePath.php
Responses/Responses/Output/OutputMessageContentOutputTextAnnotationsUrlCitation.php
Against attached Output Types Doc.md

Findings

  1. OutputMessage.php
    Properties & Type Hints:

public readonly array $content
Array of OutputMessageContentOutputText or OutputMessageContentRefusal
public readonly string $id
public readonly string $role
public readonly string $status ('in_progress'|'completed'|'incomplete')
public readonly string $type ('message')
Documentation Comparison:

{
  "id": "...",
  "type": "message",
  "role": "user",
  "status": "in_progress",
  "content": [ ... ]
}

✔ Matches the Documentation:

  1. OutputMessageContentOutputText.php
    Properties & Type Hints:

public readonly array $annotations
Array of OutputMessageContentOutputTextAnnotationsFileCitation or OutputMessageContentOutputTextAnnotationsFilePath or OutputMessageContentOutputTextAnnotationsUrlCitation
public readonly string $text
public readonly string $type ('output_text')
Documentation Comparison:

{
  "type": "output_text",
  "text": "...",
  "annotations": [ ... ]
}

✔ Matches the Documentation:

  1. OutputMessageContentOutputTextAnnotationsFileCitation.php
    Properties & Type Hints:

public readonly string $fileId
public readonly int $index
public readonly string $type ('file_citation')
Documentation Comparison:

{
  "type": "file_citation",
  "index": ...,
  "file_id": "..."
}

✔ Matches the Documentation but filename is present in the Documentation for file_citation, but missing in the class
I have checked the Create Response API docs and it has filename property in the file_citation object.

{
  "id": "resp_67ccf4c55fc48190b71bd0463ad3306d09504fb6872380d7",
  "object": "response",
  "created_at": 1741485253,
  "status": "completed",
  "error": null,
  "incomplete_details": null,
  "instructions": null,
  "max_output_tokens": null,
  "model": "gpt-4.1-2025-04-14",
  "output": [
    {
      "type": "file_search_call",
      "id": "fs_67ccf4c63cd08190887ef6464ba5681609504fb6872380d7",
      "status": "completed",
      "queries": [
        "attributes of an ancient brown dragon"
      ],
      "results": null
    },
    {
      "type": "message",
      "id": "msg_67ccf4c93e5c81909d595b369351a9d309504fb6872380d7",
      "status": "completed",
      "role": "assistant",
      "content": [
        {
          "type": "output_text",
          "text": "The attributes of an ancient brown dragon include...",
          "annotations": [
            {
              "type": "file_citation",
              "index": 320,
              "file_id": "file-4wDz5b167pAf72nx1h9eiN",
              "filename": "dragons.pdf"
            }   
          ]
        }
      ]
    }
  ]
}

#source: https://platform.openai.com/docs/api-reference/responses/create File Search Output example

  1. OutputMessageContentOutputTextAnnotationsFilePath.php
    Properties & Type Hints:

public readonly string $fileId
public readonly int $index
public readonly string $type ('file_path')
Documentation Comparison:

{
  "type": "file_path",
  "index": ...,
  "file_id": "..."
}

✔ Matches the Documentation:

  1. OutputMessageContentOutputTextAnnotationsUrlCitation.php
    Properties & Type Hints:

public readonly int $endIndex
public readonly int $startIndex
public readonly string $title
public readonly string $type ('url_citation')
public readonly string $url
Documentation Comparison:

{
  "type": "url_citation",
  "start_index": ...,
  "end_index": ...,
  "title": "...",
  "url": "..."
}

✔ Matches the Documentation:

#Conclusion

All properties and type hints match the Documentation. I don't think there is a need to recreate the classes. under Responses/Input/

  • What is your take on this?
  • if you agree that no need to recreate the classes, then should I compare other types as well before proceeding?

Output Types Doc.md

@iBotPeaches
Copy link
Collaborator

If they match 100%, reuse them and I'll fix the naming of classes to reflect that once all done.

@momostafa
Copy link
Author

The docblocks expanded into 1 line is a pain point for this repo for sure. Been working on a little tool to make it easier, since I lose track in the massive docblock of the Response API

Screenshot 2025-04-20 at 9 16 34 AM I'll deploy this somewhere once its perfect if it helps people out.

Looks amazing sure will be very helpful

@momostafa
Copy link
Author

filename

If they match 100%, reuse them and I'll fix the naming of classes to reflect that once all done.

you just need to add filename to OutputMessageContentOutputTextAnnotationsFileCitation.php
Properties & Type Hints and the match will be 100%

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.

Missing Responses API
3 participants