client icon indicating copy to clipboard operation
client copied to clipboard

Add support for Responses API

Open momostafa opened this issue 9 months ago • 82 comments

What:

Adds the new feature Responses API, supporting:

  • Create Response
  • List Input Items (from a Response)
  • Streaming Create Response
  • Retrieve Response
  • Delete Response

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.

Nerdy Breakdown:

This endpoint was massively larger than any other OpenAI endpoint to date, rivaling the complexity of the Threads feature. This meant as the typed objects expanded we ran into issues with PHPStan due to sealed/unsealed arrays not supported alongside massive unions breaking type checking.

This module introduced @phpstan-type and @phpstan-import-type which allowed the base Type to be defined on a child object, which was imported by the parent to either use/expand. This continued all the way up to the base Response class.


Authored via: @momostafa & @iBotPeaches

Fixes: #535 Laravel PR: https://github.com/openai-php/laravel/pull/147

momostafa avatar Mar 22 '25 14:03 momostafa

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

momostafa avatar Mar 25 '25 20:03 momostafa

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.

iBotPeaches avatar Mar 27 '25 16:03 iBotPeaches

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 avatar Mar 31 '25 00:03 momostafa

@iBotPeaches Please check my last commit https://github.com/openai-php/client/pull/541/commits/412f35c7b8d4bce6f2622738a830a95edbc27e6b

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

momostafa avatar Mar 31 '25 00:03 momostafa

@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.

iBotPeaches avatar Apr 04 '25 18:04 iBotPeaches

@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 avatar Apr 04 '25 22:04 momostafa

@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 https://github.com/momostafa/openai-client/commit/29436e8492a83f8dc90d4a8dc2841b8649bdbaa9 Thank you

momostafa avatar Apr 05 '25 00:04 momostafa

@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 avatar Apr 05 '25 00:04 momostafa

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

momostafa avatar Apr 06 '25 07:04 momostafa

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

momostafa avatar Apr 06 '25 12:04 momostafa

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.

iBotPeaches avatar Apr 09 '25 14:04 iBotPeaches

@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.

elnurvl avatar Apr 10 '25 14:04 elnurvl

@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 avatar Apr 12 '25 00:04 momostafa

@iBotPeaches Hi, Please check last commit https://github.com/openai-php/client/pull/541/commits/caf4413e807e55c057cabb6f5290e4cd9f9fb3db

  • 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.

momostafa avatar Apr 13 '25 05:04 momostafa

thanks @momostafa! I'll free up a few hours and finish this up either tonight or Monday night.

iBotPeaches avatar Apr 13 '25 10:04 iBotPeaches

I'll take it from here @momostafa - thanks for getting us 90% of the way there! I did some quick basic things

  • inlined README changes to main README
  • removed debug log files committed
  • fixed up extra changes that were added

Later tonight I'll work on:

  • Removing the phpstan ignores
  • Fixing up the Fake-aspect due to collision of Responses name
  • Fix up tests
  • Confirm in real life testing that regular usage & stream usage works

iBotPeaches avatar Apr 13 '25 14:04 iBotPeaches

thanks @momostafa! I'll free up a few hours and finish this up either tonight or Monday night.

Thanks a lot, I appreciate it

momostafa avatar Apr 13 '25 17:04 momostafa

@momostafa - just some thoughts in case you want to take them for learning. I knocked out a chunk of work - probably another day or two to finalize this.

  • No need for the complex logic you had for the Responses/* collision. We know that wasn't perfect so expanding that to `OpenAI//Responses//* allows it to be scoped to the right part and not collide with our new folder.
  • You had the tests/mocking for $metadata, but did not collect/parse it.
  • You had Pest assertions off raw arrays, which doesn't work. You'd have to wrap the entire array in an expect() clause for that to work. For now I'm removing that and will go back and augment the assertions based on the array.
  • Functions sometimes just included wrong thing - i.e a stream function returning a payload, or a stream getting an array - just looked like some confusion

iBotPeaches avatar Apr 15 '25 00:04 iBotPeaches

Had a bit more time before bed. Notes:

  • Streaming needs some work - everything is parsing to same class. The structure changes if the event is output_item, content_part, output_text, function_call, file_search_call, refusal, etc
  • stream tests are encoding the array directly. They should be moved into the respective Fixture file

iBotPeaches avatar Apr 15 '25 01:04 iBotPeaches

@momostafa - just some thoughts in case you want to take them for learning. I knocked out a chunk of work - probably another day or two to finalize this.

  • No need for the complex logic you had for the Responses/* collision. We know that wasn't perfect so expanding that to `OpenAI//Responses//* allows it to be scoped to the right part and not collide with our new folder.
  • You had the tests/mocking for $metadata, but did not collect/parse it.
  • You had Pest assertions off raw arrays, which doesn't work. You'd have to wrap the entire array in an expect() clause for that to work. For now I'm removing that and will go back and augment the assertions based on the array.
  • Functions sometimes just included wrong thing - i.e a stream function returning a payload, or a stream getting an array - just looked like some confusion

Thank you, for your time fixing what is needed and your inputs certainly a good learning curve for me. The whole streaming thing was very confusing for me as well as the multiple tests, testing folders... was driving me crazy : )))

momostafa avatar Apr 15 '25 05:04 momostafa

Thank you, for your time fixing what is needed and your inputs certainly a good learning curve for me. The whole streaming thing was very confusing for me as well as the multiple tests, testing folders... was driving me crazy : )))

No worries. I very much need this feature too so will get this done. I think you got a bit confused as you hit arrays/objects, which is why the tests weren't working well. Generally anytime you are typing an array/object - that suggests you need to make a class that represents that data structure. I'm basically going down the list and fixing any array/object.

  • See usage converted into objects here - https://github.com/openai-php/client/pull/541/commits/18bf40e761d99b019f5d6ccbdeda9269545c30a7
  • See error converted into objects here - https://github.com/openai-php/client/pull/541/commits/e9d3d95611ac1d2374987df6209f083402734e24

Hopefully those examples help explain the changes.

iBotPeaches avatar Apr 15 '25 10:04 iBotPeaches

Thank you, for your time fixing what is needed and your inputs certainly a good learning curve for me. The whole streaming thing was very confusing for me as well as the multiple tests, testing folders... was driving me crazy : )))

No worries. I very much need this feature too so will get this done. I think you got a bit confused as you hit arrays/objects, which is why the tests weren't working well. Generally anytime you are typing an array/object - that suggests you need to make a class that represents that data structure. I'm basically going down the list and fixing any array/object.

  • See usage converted into objects here - 18bf40e
  • See error converted into objects here - e9d3d95

Hopefully those examples help explain the changes.

Thanks for pointing out at such patterns I was totally away from coding for 6+ years and just got back about 7 month ago and I am still catching up with all PHP changes as well as learning about Laravel for the first time. Besides that PHP is totally neglected by AI providers and we don't have the leverage of having a PHP SDK (like Python & TypeScript) that would eliminate a lot of confusion and guess work...!

momostafa avatar Apr 15 '25 15:04 momostafa

Thanks for pointing out at such patterns I was totally away from coding for 6+ years and just got back about 7 month ago and I am still catching up with all PHP changes as well as learning about Laravel for the first time. Besides that PHP is totally neglected by AI providers and we don't have the leverage of having a PHP SDK (like Python & TypeScript) that would eliminate a lot of confusion and guess work...!

No worries. Glad you started the PR - its always hard to start such a large task with nothing so I'm happy I got work from a base. I'm working on the outputs chunk which is a pain in the butt.

Which is basically an array of output messages (5 different types) and some of those types are massively large.

iBotPeaches avatar Apr 15 '25 15:04 iBotPeaches

Thanks for pointing out at such patterns I was totally away from coding for 6+ years and just got back about 7 month ago and I am still catching up with all PHP changes as well as learning about Laravel for the first time. Besides that PHP is totally neglected by AI providers and we don't have the leverage of having a PHP SDK (like Python & TypeScript) that would eliminate a lot of confusion and guess work...!

No worries. Glad you started the PR - its always hard to start such a large task with nothing so I'm happy I got work from a base. I'm working on the outputs chunk which is a pain in the butt.

Screenshot 2025-04-15 at 11 29 12 AM Which is basically an array of output messages (5 different types) and some of those types are massively large.

Screenshot 2025-04-15 at 11 32 01 AM

Woow sorry, I didn't know that I had to split the output into several classes like that... I could have followed this path if you had instructed me earlier

momostafa avatar Apr 16 '25 00:04 momostafa

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:

  • [x] fully type the tools array
  • [x] meld all docblock changes from child classes into main Create, Retrieve and List
  • [x] copy changes over to retrieve/list
  • [x] 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.

iBotPeaches avatar Apr 17 '25 01:04 iBotPeaches

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:

  • [x] 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

momostafa avatar Apr 18 '25 08:04 momostafa

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 :)

iBotPeaches avatar Apr 18 '25 11:04 iBotPeaches

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 avatar Apr 18 '25 19:04 momostafa

@iBotPeaches Hi, Please check https://github.com/momostafa/openai-client/commit/e811af7daeed083e45f4d83c08a131f75201af77 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

momostafa avatar Apr 20 '25 03:04 momostafa

@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.

iBotPeaches avatar Apr 20 '25 10:04 iBotPeaches