[Frontend] split append tool output #28333
Conversation
Signed-off-by: Andrew Xia <axia@fb.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| class ConversationContext(ABC): | ||
| @abstractmethod | ||
| def append_output(self, output) -> None: | ||
| def append_output(self, output: RequestOutput) -> None: | ||
| pass | ||
|
|
||
| @abstractmethod | ||
| def append_tool_output(self, output) -> None: | ||
| pass |
There was a problem hiding this comment.
The commit introduces a new abstract method append_tool_output on ConversationContext but only adds concrete implementations for the production contexts. tests/entrypoints/openai/test_serving_responses.py still defines MockConversationContext without overriding this method, so constructing the mock (MockConversationContext()) now raises TypeError: Can't instantiate abstract class … with abstract method append_tool_output and the test suite fails before any assertions run. Please add a stub implementation to the mock or adjust the base class contract.
Useful? React with 👍 / 👎.
Signed-off-by: Andrew Xia <axia@fb.com>
qandrew
requested review from
DarkLight1337,
NickLucche,
robertgshaw2-redhat and
simon-mo
as code owners
github-project-automation
bot
moved this from To Triage
to Ready
in gpt-oss Issues & Enhancements
github-actions
bot
added
the
ready
github-project-automation
bot
moved this from Ready
to Done
in gpt-oss Issues & Enhancements
Signed-off-by: Andrew Xia <axia@fb.com> Co-authored-by: Andrew Xia <axia@fb.com> Signed-off-by: George D. Torres <gdavtor@gmail.com>
Signed-off-by: Andrew Xia <axia@fb.com> Co-authored-by: Andrew Xia <axia@fb.com> Signed-off-by: Bram Wasti <bwasti@meta.com>
Signed-off-by: Andrew Xia <axia@fb.com> Co-authored-by: Andrew Xia <axia@fb.com>
Signed-off-by: Andrew Xia <axia@fb.com> Co-authored-by: Andrew Xia <axia@fb.com> Signed-off-by: Xingyu Liu <charlotteliu12x@gmail.com>
Signed-off-by: Andrew Xia <axia@fb.com> Co-authored-by: Andrew Xia <axia@fb.com>
3 participants
Purpose
append_output currently does two things, it takes an input from the engine (as type RequestOutput) and it takes input from a MCP tool call. These are two fundamentally different functions, so this PR separates them. No functional changes expected.
Test Plan
Test Result
This passes
pytest -sv tests/entrypoints/openai/test_response_api_with_harmony.py::test_function_calling_multi_turn