Repository navigation
Conversation
|
marking as draft because some fixes are coming |
planetf1
left a comment
There was a problem hiding this comment.
I ran this against the real OpenAI SDK (openai 2.24.0, with TestClient(app) as the SDK's http_client, so no network needed). The plain non-streaming text path works well, and /v1/chat/completions is unaffected. Beyond that, a few places in the request and stream formats differ from the Responses API, and most of #1635's SDK criteria fail as a result.
Blocking: SDK tool shapes, structured output via text.format, the stream event sequence, streamed turns not being stored, stream not reaching serve(), and instructions carrying over with previous_response_id. Should fix: reasoning-token usage, the server-side tools example, and SDK-level integration tests. The rest are non-blocking suggestions and small nits, all inline.
| """ | ||
|
|
||
| type: Literal["function", "web_search", "file_search", "mcp", "code_interpreter"] | ||
| function: FunctionDefinition | None = None |
There was a problem hiding this comment.
The OpenAI SDK sends Responses-style tools, but ResponseTool only accepts the Chat Completions shape. Reproduced with openai 2.24.0, using OpenAI(http_client=TestClient(app)) and a stub serve():
- Tools. The SDK's function tool is flat:
{"type": "function", "name", "parameters", "strict"}(openai/types/responses/function_tool_param.py). Only the nestedfunction: {...}form is modelled here, sonameandparametersare dropped andserve()receives[{"type": "function"}]. With the server-side tools example,_resolve_toolsthen hitstool_def["function"]["name"], raisesKeyError, and the client gets a 500. tool_choice. The SDK form is{"type": "function", "name": ...}. It's passed through unchanged, and the example server readstool_choice["function"]["name"], so it's ignored.- Tool results.
ResponseInputItemrequiresrole, so afunction_call_outputitem gets a 400. An SDK client can't send a tool result back.
This is the "SDK works without modifications" and "function tool calling works end-to-end" criteria in #1635. Changing the wire shape after people have built against it would be a breaking change. Could we accept the SDK shapes at the edge and normalize them into what serve() gets today? The nested form could stay as an alias so the current examples keep working.
There was a problem hiding this comment.
Fixed. Flat SDK tools and tool_choice are normalised to the nested form, and function_call_output is accepted. Checked with the SDK at e02ab99.
| parallel_tool_calls: bool | None = True | ||
| context_management: ContextManagementConfig | None = None | ||
| prompt_cache_options: PromptCacheOptions | None = None | ||
| format: ResponseFormat | None = None |
There was a problem hiding this comment.
The SDK sends structured output as text={"format": {"type": "json_schema", "name", "schema", "strict"}}, flat, and Responses.create() has no format argument. Reproduced with openai 2.24.0:
client_responses_format.py:65andREADME.md:383fail withTypeError: Responses.create() got an unexpected keyword argument 'format'.- A real
text.formatrequest is accepted but ignored:serve()getsformat=None, andtextends up inmodel_optionsas a raw key.
Could this read text.format in the flat shape (keeping top-level format as an alias if you like) and keep text out of model_options?
There was a problem hiding this comment.
Fixed. text.format reaches serve() as the model and no longer leaks into model_options. Top-level format stays as an alias.
| output_index = 0 | ||
| content_index = 0 | ||
| # Generate a message item ID for the output_text events | ||
| item_id = f"msg_{uuid.uuid4().hex[:24]}" |
There was a problem hiding this comment.
client.responses.stream() fails with IndexError: list index out of range on the first text delta. That's the call the streaming client example makes (client_responses_streaming.py:30). Reproduced with openai 2.24.0.
The SDK's stream helper builds its snapshot from response.output_item.added and response.content_part.added, then indexes snapshot.output[event.output_index] on each output_text.delta (openai/lib/streaming/responses/_responses.py:330-354). This generator never sends those two events, so the list is still empty when the first delta arrives.
The tool-call events have a related problem. The function_call_arguments.delta and .done payloads (L296-305) have no type or sequence_number. The SDK reads the data body, not the event: line, so they decode as ResponseAudioDeltaEvent with type=None and never match response.function_call_arguments.done. response.failed (L329) is the same. The stream tests check the event: line, which is why they pass.
The fix is the SDK's per-item sequence: output_item.added, content_part.added, the deltas, output_text.done, content_part.done, output_item.done, plus an output_item.added/.done pair around each function call, with type and sequence_number on every event. When there's no text, skip the message item, otherwise it shares output_index 0 with the first function call.
There was a problem hiding this comment.
Fixed. responses.stream() now completes, and the function-call events carry type and sequence_number. Checked with the SDK.
|
|
||
| storing = request.store is not False | ||
|
|
||
| if request.stream: |
There was a problem hiding this comment.
On the streaming path nothing is written to _response_store. The if request.stream: branch returns before the store write at L542, and stream_response_chunks only uses store to set expires_at.
So a streamed response says it's retained until expires_at, but GET /v1/responses/{id} returns 404, and so does a follow-up turn using previous_response_id. Reproduced with openai 2.24.0: after a streamed turn the entry isn't in the store, and the chained create() raises NotFoundError.
Agent clients usually stream and chain with previous_response_id, so this breaks multi-turn for them. Could the generator store the response, with history built from accumulated_text, just before it emits response.completed? Passing in a small store callback from app.py would keep the store logic in one place.
There was a problem hiding this comment.
Fixed. Streamed turns are stored, so retrieve and previous_response_id chaining work after a stream. Checked with the SDK.
| "input", | ||
| "instructions", | ||
| "model", | ||
| "stream", |
There was a problem hiding this comment.
stream is excluded here and never mapped to ModelOption.STREAM, whereas the chat path maps it (L208). So serve() can't tell the client asked to stream, and the "one per streamed token" behaviour in the stream_response_chunks docstring (streaming.py:197) never happens.
With the streaming example server, ModelOption.STREAM is missing, so it takes the await_result=True branch (m_serve_example_streaming.py:26-41). The whole generation finishes, and then the client gets it as a single delta. Reproduced with openai 2.24.0: stream=True reached serve() with model_options={'temperature': 1.0}.
Mapping it the way the chat path does should be enough, as the generator already handles an uncomputed thunk through astream().
There was a problem hiding this comment.
Fixed. stream=True now reaches serve() as ModelOption.STREAM.
| ] | ||
|
|
||
| from typing import Any, Literal | ||
| from typing import Any, Literal, Union |
| Using `curl`: | ||
|
|
||
| ```bash | ||
| curl http://localhost:8000/v1/responses \ |
There was a problem hiding this comment.
The new snippets here and at L134 use port 8000, but the CLI default is 8080 (commands.py:20). The 8000 at L76 predates this PR.
There was a problem hiding this comment.
Fixed. The snippets now use 8080.
|
|
||
| import pytest | ||
|
|
||
| import cli.serve.app as app_module |
There was a problem hiding this comment.
This file tests cli.serve.app, so it probably belongs in test/cli/ with the other serve tests.
There was a problem hiding this comment.
Fixed. The file is now in test/cli/.
| if output.value: | ||
| items.append( | ||
| ResponseOutputMessage( | ||
| id=f"msg_{uuid.uuid4().hex[:24]}", |
There was a problem hiding this comment.
This mints a fresh message id, so it won't match the item_id the stream used (streaming.py:253). Reusing one id would let a client tie the deltas to the final item.
There was a problem hiding this comment.
Fixed. The streamed item id and the delta item_id now match.
| ), | ||
| host: str = typer.Option("0.0.0.0", help="Host to bind to"), | ||
| port: int = typer.Option(8080, help="Port to bind to"), | ||
| response_ttl: int = typer.Option( |
There was a problem hiding this comment.
--response-ttl has no minimum, so 0 or a negative value is accepted.
There was a problem hiding this comment.
Fixed. --response-ttl now has min=1.
Add support for the OpenAI Responses API alongside the existing /v1/chat/completions endpoint. Both routes are registered on the same server and share the same user-supplied serve() function. New endpoint behaviour: - Accepts ResponseRequest (input, instructions, tools, conversation, previous_response_id, max_output_tokens, temperature, etc.) - Converts string or message-array input to the internal ChatMessage format; maps the `developer` role to `system` - Returns a typed Response object with an output[] array, output_text convenience field, and ResponseUsage (including reasoning_tokens) - Streaming emits semantic SSE events: response.created, response.in_progress, response.output_text.delta, response.output_text.done, response.function_call_arguments.done, response.completed (or response.failed on error) - Tool calls produce ResponseFunctionCall output items - background=true returns 400 (not yet supported) New models in cli/serve/models.py: ResponseRequest, ResponseInputItem, InputContent, ResponseTool, WebSearchTool, FileSearchTool, MCPTool, ContextManagementConfig, PromptCacheOptions, Response, ResponseError New helpers in mellea/helpers/openai_compatible_helpers.py: ResponseUsage, ResponseOutputItem, ResponseOutputMessage, ResponseFunctionCall, OutputTextContent, build_response_usage(), build_response_output_items() Assisted-by: IBM Bob Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
Update m-serve.md to mention both endpoints in the intro, add /v1/responses to the exposed routes list, and add a "Responses API" section with curl, OpenAI SDK, and streaming examples. Fix the API Endpoints section in docs/examples/m_serve/README.md, which incorrectly listed POST /generate (not a real route). Replace it with the actual registered routes: /v1/chat/completions and /v1/responses. Assisted-by: IBM Bob Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
Add client_responses.py, client_responses_streaming.py, and client_responses_tool_calling.py alongside the existing Chat Completions clients in simple/, streaming/, and tool-calling/. Each uses the same server program as its sibling client — no server changes are needed to use /v1/responses. client_responses.py: minimal smoke test using client.responses.create() and output_text. client_responses_streaming.py: toggleable streaming/non-streaming using client.responses.stream() and response.output_text.delta events. client_responses_tool_calling.py: three scenarios (weather, stock price, tool call + follow-up) reading function_call items from the output[] array. Update docs/examples/m_serve/README.md to list the new files and note that all Responses API clients share the same server program. Assisted-by: IBM Bob Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
Assisted-by: IBM Bob Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
Assisted-by: IBM Bob Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
…s API Assisted-by: IBM Bob Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
Fitting the new responses paradigm, we can continue a conversation w/o passing everything back and forth. At this time, there is no store that persists a server restart. There is a time-to-live (TTL) option as a simplistic way to not hog memory forever. Store, compaction, cancel, etc... would likely be future enhancements. Assisted-by: IBM Bob Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
…letion IDs Replace uuid.uuid4().hex[:N] with secrets.choice over ascii_letters+digits for both response_id (resp_...) and completion_id (chatcmpl-...) generation. The hex approach produced only lowercase a-f plus 0-9, which does not match the mixed-case alphanumeric format used by the OpenAI API. The new IDs use the full base-62 alphabet (0-9a-zA-Z), matching the OpenAI spec. Also fix test_previous_response_id_echoed, which was passing an unknown ID and expecting it to be echoed back. The correct behaviour (404 for unknown previous_response_id) meant the endpoint returned a JSONResponse, causing an AttributeError. The test now does a proper two-step round-trip: store a response first, then reference it via previous_response_id. Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com> Assisted-by: IBM Bob
Add expires_at: int | None to the Response model. Set to created_at + TTL when store=True, None when store=False. Updates README to document in-memory-only behaviour and loss on restart. Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com> Assisted-by: IBM Bob
web_search, file_search, mcp, and code_interpreter were accepted by the schema but had no implementation, causing an internal AssertionError deep in the backend. Now _build_model_options_from_response_request raises ValueError for any non-function tool type, which the existing handler converts to a 400 with an actionable message. Remove the unused WebSearchTool, FileSearchTool, and MCPTool model classes — the type literals on ResponseTool are sufficient for parsing before rejection. Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com> Assisted-by: IBM Bob
stream_response_chunks ignored the request's include list and always emitted usage in response.completed. Pass request.include through from make_responses_endpoint and gate usage on "usage" in include. Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com> Assisted-by: IBM Bob
Response.output was typed as Union[ResponseOutputMessage, ResponseFunctionCall, ResponseOutputItem] but Pydantic was matching items against the base class first, stripping the content and role fields from serialized message items. Reorder the Union so ResponseOutputMessage and ResponseFunctionCall are listed before the base ResponseOutputItem, ensuring subclass fields are preserved on serialization. Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
stream_response_chunks() had two gaps:
1. response.completed emitted only {id, status, usage}; SDK clients
expect the full Response object. Now builds a complete Response and
serializes it as {"type": "response.completed", "response": ...}.
2. Tool call arguments were only emitted as .done with no preceding
.delta. Now emits response.function_call_arguments.delta (full
arguments as one chunk) before .done, which is spec-conformant when
the backend doesn't stream arguments incrementally.
Also adds the missing SDK envelope fields to every event:
- type field matching the event name
- sequence_number counter
- response.output_item.added before content events
- response.content_part.added before text delta/done events
Passes store, ttl, and previous_response_id into the generator so
the completed envelope has the correct expires_at and
previous_response_id fields.
Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
Supports structured output same as chat/completions. Follows OpenAI API spec. Does not support json_object mode yet (same as completions). Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
Consistent with chat/completions implementation. Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
Fix streaming events for OpenAI SDK spec. Fix tool call index. Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
…ning Streaming responses were not written to the in-memory store. Add an on_complete callback to stream_response_chunks, invoked after response.completed is sent, and wire it up in make_responses_endpoint to save the response when store=True. Assisted-by: IBM Bob Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
…d context on chained turns In the Responses API, request instructions are scoped per turn and should not carry over across turns via previous_response_id. Separate input and instruction conversion so that: - Instructions are excluded from stored conversation history. - New instructions on continuation requests are placed at the beginning of the context rather than after prior turns. Assisted-by: IBM Bob Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
Forward stream=True into model_options as ModelOption.STREAM so backends stream incrementally from the provider on /v1/responses. Assisted-by: IBM Bob Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
…output Accept flat tool declarations, flat tool_choice, and function_call_output input items as sent by the OpenAI SDK, normalizing them into internal chat message and model option structures. Assisted-by: IBM Bob Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
Accept text={"format": {...}} as sent by the OpenAI SDK, lifting the
flat format into the canonical top-level format field at parse time.
ResponseFormat now accepts both the nested Chat Completions shape and
the flat Responses API shape, and text/verbosity are kept out of
model_options. Fix the response-format example and README to the SDK
shape.
Assisted-by: Claude Code
Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
…point build_response_usage read reasoning_tokens, input_tokens_details and output_tokens_details from the top level of generation.usage, but no backend writes those keys: OpenAI and LiteLLM store the Chat Completions usage dict, where reasoning tokens live under completion_tokens_details.reasoning_tokens and cached tokens under prompt_tokens_details.cached_tokens. As a result usage.reasoning_tokens was always None and SDK clients reading the spec location (usage.output_tokens_details.reasoning_tokens) hit AttributeError because the details objects were never emitted. Read both values from the Chat Completions locations, keeping the top-level reasoning_tokens probe as the preferred source, and always emit the details objects with zeros when the backend reports no breakdown, matching the OpenAI Responses API usage shape. The top-level reasoning_tokens field requested by generative-computing#1635 stays, None when unknown. Assisted-by: Claude Code Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
… example MelleaSession(backend) defaults to SimpleContext, whose view_for_generation() always returns [], so the tool results added to the session context — and the original question — never reached the model on the re-prompt. The re-prompt also passed tool_calls=False and dropped ModelOption.TOOLS, capping the "agentic loop" at one round. Use a ChatContext so history and tool results are sent, keep the tools advertised on re-prompts, and relax tool_choice to auto so a pinned choice cannot force the same tool call every round. Apply the same fixes to the __main__ smoke test. Assisted-by: Claude Code Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
The test asserted the literal substring '--response-ttl' in the CLI runner output, but rich colorizes the option name — inserting ANSI codes between the dashes — whenever a color-forcing env var like FORCE_COLOR is set, failing the assertion on a correctly-rejected value. Strip ANSI escape codes before asserting. Assisted-by: Claude Code Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
The responses endpoint tests called make_responses_endpoint() and stream_response_chunks() directly, so request parsing, the SSE event sequence, and streamed-turn storage were never exercised the way SDK clients consume them — that is how the stream-sequence, tool-shape, text.format, and streamed-chaining bugs reached CI. Drive /v1/responses through the real OpenAI SDK with TestClient as its http_client (no network): create(), responses.stream(), a tool round trip with function_call_output, text.format structured output, and previous_response_id after a streamed turn, with the lifespan and GET route wired like the real server. TestEvictExpiredResponses now drives the real _evict_expired_responses coroutine with asyncio.sleep patched instead of a copied loop body, and _lifespan's eviction scheduling is asserted directly. Assisted-by: Claude Code Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
The responses path always passed requirements=None to serve(), while the chat path passes request.requirements. A requirements-aware serve() program silently lost its requirements on this endpoint, and because ResponseRequest allows extra fields, a requirements key sent through the SDK's extra_body landed in model_options as a raw key instead. Declare requirements on ResponseRequest (mirroring ChatCompletionRequest), exclude it from model_options, and forward request.requirements to serve(), matching the chat endpoint. Assisted-by: Claude Code Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
conversation={"id": ...} and context_management=[{...}] — the shapes the
OpenAI SDK sends — failed validation with a 400, even though both fields
are deferred (excluded from model_options and not yet implemented).
Collapse the conversation object to its id string with a before-validator
so the field stays a string and the echo round-trips, and retype
context_management to accept the SDK's list form alongside the existing
dict form. Both remain accepted-and-ignored.
Assisted-by: Claude Code
Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
input_file blocks and blocks with no convertible payload were only logged and skipped, so a message containing just an input_file block completed normally and serve() received an empty user turn — the model got nothing and the client got no error. Raise ValueError from the content converter instead; the endpoint's existing handler turns it into a 400, matching how invalid image data is treated (the docstring already claimed this). An empty content list still converts to None. Assisted-by: Claude Code Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
The response store had no entry cap and store=True is the default, so on an unauthenticated server memory grew with the conversation chain — each entry keeps a full copy of the history. GET also served entries after their expires_at until the next background sweep caught up. Add a --max-stored-responses flag (default 1000, entries dropped oldest-first) enforced at both store write sites, and have GET reject entries past their expiry using the stored expires_at (falling back to stored_at + TTL). Also fix the store comment: the sweep is started by the app lifespan, not run_server(). Assisted-by: Claude Code Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
The example built its ChatContext from every input message — including the current user turn — and then passed that same turn again as the instruct description, so the model saw the latest question twice. Build the context from the prior turns only; the current turn (always the last message) is passed via the instruct description exactly once. Assisted-by: Claude Code Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
|
Existing feedback has been addressed, but I'm looking into some additional issues after more review. |
The documented OpenAI SDK pattern for tool calling without storage is input=response.output + [function_call_output(...)], but those replayed items were rejected: function_call items failed the input type Literal, replayed assistant messages' output_text content blocks failed the content type Literal, and a Chat-Completions-shaped assistant item's tool_calls parsed but was silently dropped — ChatMessage had no tool_calls field, so pydantic's default extra=ignore discarded it. Add the tool_calls field to ChatMessage (the Chat Completions wire shape the backends already speak), accept function_call items and output_text blocks, and convert function_call items into assistant messages whose tool_calls pair with the following function_call_output results. Consecutive function_call items merge into a single assistant message so parallel calls keep every tool result directly after the assistant turn that declared it. Assisted-by: Claude Code Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
The response store serialized history as {role, content} only, dropping
tool_call_id from tool messages and never recording the assistant's
function calls. A turn chained via previous_response_id after a
tool-call response therefore replayed an unpaired role:"tool" message —
no tool_call_id, no preceding assistant tool_calls — which OpenAI-style
providers reject with a 400.
Carry tool_call_id and tool_calls through the stored history (keys only
when set), thread the assistant's function calls into both store writes,
and replay via ChatMessage(**m) so the pairing survives the round trip.
Assisted-by: Claude Code
Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
…nse_id Add an SDK integration test that streams a function-call response, then continues it with a function_call_output through previous_response_id. It asserts the replayed input is user/assistant/tool, that the assistant message carries the stored tool_calls (id, name, arguments), and that the tool message keeps its tool_call_id. This exercises the streaming store path (_save_streamed_response) and ChatMessage rehydration, which had no direct coverage. Assisted-by: IBM Bob Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
Malformed tool-turn input items parsed successfully and produced wire
messages providers reject: a function_call without call_id or name
became a tool call with None fields, a function_call_output without
call_id became a tool message paired with no call, and
ResponseInputItem.tool_calls was list[Any], passing arbitrary shapes
through to serve().
Require call_id and name on function_call items and call_id on
function_call_output items at parse time, type tool_calls as
ChatCompletionMessageToolCall (the existing Chat Completions wire-shape
model, dumped to dicts for ChatMessage), and default a missing
arguments value to "{}" — an empty string is not valid JSON for the
backend.
Assisted-by: Claude Code
Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
GET /v1/responses/{id} rejected entries past their expiry, but the
previous_response_id lookup only checked presence — so within the sweep
window (up to a minute past expiry) the two read paths disagreed: GET
returned 404 while a chained turn happily replayed the expired history.
Share one expiry-checking lookup helper between both read paths.
Assisted-by: Claude Code
Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
…ptions Standard OpenAI Responses API request parameters that ResponseRequest does not declare (user, reasoning, truncation, metadata, service_tier, max_tool_calls, prompt, prompt_cache_key, prompt_cache_retention, safety_identifier, stream_options, top_logprobs) fell through extra="allow" into model_options and from there into the backend request, where strict providers reject unknown options with opaque errors. Exclude them at the edge — they are metadata/routing, not generation parameters. The chat endpoint already does the same for 'user'. Assisted-by: Claude Code Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
The streaming path persisted the raw accumulated deltas as the stored history's assistant turn, while the non-streaming path persists the post-processed output.value. When post-processing rewrites the value (e.g. json_schema validation), a turn chained on a streamed response saw different — unvalidated — assistant content than the identical non-streamed request. Store output.value in the streamed history (the thunk is computed by the time the store callback runs). The stream envelope and the retrievable response keep the streamed text, matching what the client watched arrive. Assisted-by: Claude Code Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
_build_history_from_messages stored multimodal message content as pydantic model instances inside history entries typed and documented as plain dicts. In-process replay happened to re-validate them, but anything JSON-serializing the history — e.g. the cross-restart persistence described as future work — would fail on the models. Dump content-list elements to plain dicts when storing; ChatMessage re-validates them back into models on replay. Assisted-by: Claude Code Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
…ents The example clients connected to http://0.0.0.0:8080 — a bind address, not a connect target. It happens to work as a loopback alias on Linux and macOS but fails outright on Windows and some container setups. Point every client connect URL (and the README snippets that copy them) at localhost. Server bind addresses are unchanged. Assisted-by: Claude Code Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>
|
After addressing the review feedback above, ran a fresh self-review of the full diff and fixed everything it turned up — one commit per fix, grouped by theme below. Tool-call round trips
Store / session behavior
Edge validation & examples
New SDK-level integration tests cover the |
planetf1
left a comment
There was a problem hiding this comment.
Approving. I rechecked every point from my earlier review against e02ab99 with the OpenAI SDK, and all are fixed (details in the thread replies).
Nit, non-blocking: a message with content: [] or content: null still returns 200 and serve() receives a None user turn. A 400 would match how input_file is handled, but fine as a follow-up.
Pull Request
Issue
Fixes #1635
Description
Adds responses endpoint consistent with the OpenAI API and our chat/completions endpoint.
This brings the updated way of interacting which can include server-side storing of context and server-side tools.
The server-side storage is very basic in-memory with time-to-live setting. Storage that lasts across restarts and is more management options could be added later.
Testing
Attribution
Adding a new component, requirement, sampling strategy, or tool?
If your PR adds or modifies one of the types below, check the matching box. A checklist of type-specific review items will be posted as a comment.
NOTE: Please ensure you have an issue that has been acknowledged by a core contributor and routed you to open a pull request against this repository. Otherwise, please open an issue before continuing with this pull request.