-
-
Notifications
You must be signed in to change notification settings - Fork 354
feat: add reasoning content support to OpenAI adapter #1938
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
Add reasoning support to the OpenAI adapter's chat_output function similar to the DeepSeek adapter implementation. This allows the OpenAI adapter to handle reasoning content from models that support it, such as o1 and o3 series models. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Add the content field to the output table when a delta contains reasoning. Previously, the output table omitted the content field in this case, which caused the caller to receive incomplete responses. This patch restores the correct behavior by setting `output.content` when `delta.content` is provided after handling reasoning.
|
Thanks for this. I'll need test coverage (as we have for DeepSeek) before I can merge this. Also, please stick to the PR template in the future - it just reduces the visual debt when you're trawling through lots of PRs. |
Add a stub file containing OpenAI reasoning stream chunks and a corresponding test case that verifies reasoning content is correctly accumulated during streaming. The new test confirms that reasoning tokens are captured and that the final assistant message is produced as expected.
I've updated it according to the template. |
| role = delta.role, | ||
| } | ||
|
|
||
| if delta.reasoning then |
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.
Can we make the name of the reasoning field configurable (maybe via opts)? A lot of providers (deepseek, llama.cpp, etc.) implement a slightly modified openai-api that sends the reasoning under a different name. For example, the DeepSeek API calls it reasoning_content. Since a lot of the other adapters reuse code from the openai adapter, it's probably worth it to make it more "feature-complete" so that it'll be easier to maintain/create other adapters.
|
Thanks for implementing this! Is it compatible with all |
This modification was actually made to use openai_compatible to request openrouter. Openrouter, in turn, returns the thought process through the reasoning field. I actually considered writing a dedicated openrouter_compatible and then separately implementing chat_output and providing a configurable reasoning field to support this capability. However, considering their complexity and redundancy, I decided against it. If you're open to implementing a configurable delta.custom_field adapter mechanism that supports the OpenRouter API for inference, perhaps it would be better if you handled it. @Davidyz @znculee |
|
@SDGLBL Thanks! I'm mainly using local models with this plugin, and I just confirmed that this branch is already working with the local Ollama (gpt-oss:20b) as follows. |
But I'm using OpenRouter, which apparently doesn't support Ollama as an adapter. Ollama supports this feature simply because the Ollama adapter handles the reasoning field. Here |
The thing is, the custom handler will probably be 99% identical to the openai adapter, with the custom field being the only thing that sets them apart. I think it'd be nice to just make the openai adapter more versatile. But at the end of the day, there's no standardized way of doing this, so it's ok if you don't want to include this here. |
|
Agree with @Davidyz, and @SDGLBL rather than extending |
|
Hello everyone, just wanted to share my two cents. I believe |
|
|
After some further research, I'd like to re-raise my point on the configurable reasoning output field name. llama.cpp, vllm and alibaba (qwen) support listing models via the Just to clarify, I'm suggesting something like: adapter.opts = {
stream = true,
tools = true,
vision = true,
-- users can override this to fit their needs
reasoning_field = 'reasoning'
}and in the output = {
role = delta.role,
}
if delta[opts.reasoning_field] then
output.reasoning = {
content = delta[opts.reasoning_field],
}
elseif delta.content then
output.content = delta.content
end@olimorris any thoughts on this? |
|
This PR is stale because it has been open for 30 days with no activity. |
|
This PR is stale because it has been open for 30 days with no activity. |
|
Any updates on this? @olimorris I'm tempted to add reasoning output to the gemini adapter, which might introduce some changes to the openai adapter and might have conflicts with this PR here. It'll be easier if we could decide what happens with this PR first. |
|
Hi All, Apologies for not having the chance to look at this sooner. Actually reviewing a PR such as this is non-trivial and I am tightly time-boxing my time on CodeCompanion these days. If a PR like this doesn't include links to OpenAI documentation then I have to manually review every single line alongside their docs. "How are we processing the reasoning output?". "Does it need to be passed back to OpenAI on the next turn?". All such questions that came up when I added support for the Responses API. After a quick review, I can't accept this PR without adding reasoning support for when streaming is turned off. I believe OpenAI still have some models on the completions endpoint that don't allow streaming and there's always the chance they release new ones that don't support it anyway. |
I don't think the standard OpenAI API contains anything about reasoning output or even a summary. OpenAI-compatible providers that actually provide reasoning output tend to come up with their own implementation for this. DeepSeek, for example, uses an extra field in the delta to transfer the string (some other providers follow this convention). Google Gemini takes a different approach (see #2306). As for the non-streaming format, I'm not sure about the openrouter format, but I'm pretty sure the deepseek format work with non-streaming requests. That's why I proposed these changes: it's non-intrusive when used with the official OpenAI endpoint, and since it's just the deepseek format if you set Update: Examples of deepseek and openrouter using the official openai-python package to work with reasoning tokens. |
|
This PR is stale because it has been open for 30 days with no activity. |
|
I believe #2359 superseded this PR? Should be safe to close this imo. |
1 similar comment
|
I believe #2359 superseded this PR? Should be safe to close this imo. |
Description
Add reasoning content support to the OpenAI adapter's
chat_outputfunction, enabling proper handling of reasoning content from OpenAI models that support it, such as oss series models.Why This Change?
OpenAI's oss series models on openrouter return reasoning tokens that provide insight into the model's thinking process. Without this support, the CodeCompanion chat buffer would lose this valuable reasoning content during streaming responses.
Data Structure Changes
Before:
After:
Related Issue(s)
None - This is a feature enhancement to support new model capabilities.
Screenshots
Checklist
CodeCompanion.hasin the init.lua file for my new feature (N/A - reasoning is an adapter enhancement, not a user-facing feature)make allto ensure docs are generated, tests pass and my formatting is applied