-
Notifications
You must be signed in to change notification settings - Fork 124
feat(fireworks): Responses API for Fireworks AI #121
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
Conversation
njbrake
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the work on this! A few comments and changes needed.
|
Addressed the concerns previously raised, kindly let me know if any more changes are required. |
njbrake
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! A few more changes needed.
- You'll need to merge in the latest changes from main
- Run linting to surface the type errors. If you use uv:
uv run pre-commit run --all-files --verbosethis will run mypy and ruff on your code and surface a few of the missing typing issues - You need to handle both streaming and non streaming in your return. Your code works fine now for non-streaming, but if it's streaming, you'll need to re-yield each chunk as the any-llm type. I know it seems like a silly thing to do right now, but the idea is to decouple the fireworks output from the any-llm typing to make it flexible if fireworks decides to change their output information. You may actually be able to directly return the
response, it's possible that mypy will understand that the Any-LLM and OpenAI responses type are currently the same thing.
| response = Response.model_validate(response.model_dump()) | ||
|
|
||
| return response |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since currently the Any LLM response type is an alias to the OpenAI type, you may actually be able to make mypy happy by removing the model_validate line.
| response = Response.model_validate(response.model_dump()) | |
| return response | |
| return response |
|
Merged the latest commits from main and ran linting to surface all type errors. Still working on the streaming responses. |
| response = client.responses.create( | ||
| model=model, | ||
| input=input_data, | ||
| **kwargs, | ||
| ) | ||
| if not isinstance(response, Response | Stream): | ||
| msg = f"Responses API returned an unexpected type: {type(response)}" | ||
| raise ValueError(msg) | ||
| return response |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'm not entirely sure if mypy would be happy with this or not, but I think technically it might be fine to directly return the output. Not sure if mypy will be ok with that 😅
| response = client.responses.create( | |
| model=model, | |
| input=input_data, | |
| **kwargs, | |
| ) | |
| if not isinstance(response, Response | Stream): | |
| msg = f"Responses API returned an unexpected type: {type(response)}" | |
| raise ValueError(msg) | |
| return response | |
| return client.responses.create( | |
| model=model, | |
| input=input_data, | |
| **kwargs, | |
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a small issue, sorry for overlooking this earlier.
The returned here includes the part where the LLM thinks and we'll have to process this output in this way response.output[-1].content[0].text.split("</think>")[-1] to get only the relevant response.
Example response:
<think>
LLM thinking content here
</think>
Actual response
Just wanted to confirm with you once whether we can allow this kind of a response(that includes the thought and the actual reply) or return only the reply.
I will make the changes according to your reply
Codecov Report❌ Patch coverage is
... and 24 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
@Shiva4113 Thanks for the work on this! I'll make a few pushes to get it over the finish line and then will merge it in 🙏 |
- Updated fireworks dependency version to >=0.17.16 in pyproject.toml. - Refined response type checks in FireworksProvider to improve error handling. - Adjusted reasoning assignment logic to ensure it handles None cases gracefully. - Updated test configuration for Fireworks provider model mapping.
Related Issue
This Pull Request is related to the issue #111 .
Summary
Added support for using Fireworks as a provider with the Responses API.
Notes from Development
fireworks-ai(v0.19.18).any-llm-sdkafter runningpip install fireworks-ai, a dependency issue arises:reward-kit(v4.1) is incompatible withopenai(v1.99.6).pip install any-llm-sdk[fireworks]results in a version offireworks-aithat is too old to support the new Responses API feature.Additional Changes