Add OpenAI Responses API adapter and developer role support - #15
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds OpenAI Responses API flavor and adapter, expands message roles to include "developer" with role coercion via prepareMessages(), updates prompt formatting and tests, adds an example script, and bumps package version to 0.6.0. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant FP as Freeplay Client
participant PromptAPI as Prompts API
participant OpenAI as OpenAI Responses API
participant Records as Recordings API
User->>FP: invoke example / run flow
FP->>PromptAPI: getFormatted(projectId, template, env, vars)
PromptAPI-->>FP: FormattedPrompt (messages, tools, promptInfo)
FP->>FP: prepareMessages(messages, roleSupport) -> toLLMSyntax / build params
FP->>OpenAI: responses.create(messages, model, params)
OpenAI-->>FP: completion/result
FP->>FP: FormattedPrompt.allMessages(completion.output)
FP->>Records: recordings.create(metadata, callInfo)
Records-->>FP: recorded
FP-->>User: log success / error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/openaiResponses.test.ts (1)
145-160: Consider verifying the warning message content.The test mocks
console.warnbut doesn't verify the warning message. Consider adding an assertion to ensure users see the expected coercion warning.💡 Suggested improvement
const warnSpy = jest.spyOn(console, "warn").mockImplementation(); const result = prepareMessages(messages, adapter.roleSupport); - warnSpy.mockRestore(); + expect(warnSpy).toHaveBeenCalledWith( + "Role 'developer' is not natively supported by this flavor. Coercing to 'system'." + ); + warnSpy.mockRestore(); expect(result).toEqual([🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/openaiResponses.test.ts` around lines 145 - 160, The test "developer coerced to system for openai_chat" currently mocks console.warn but doesn't assert the warning content; update the test that constructs OpenAILLMAdapter and calls prepareMessages to assert the mocked warnSpy was called with the expected coercion message (e.g., using expect(warnSpy).toHaveBeenCalledWith(...) or a regex) after prepareMessages runs, keeping warnSpy.mockRestore() afterward; reference the OpenAILLMAdapter instantiation and the call to prepareMessages(messages, adapter.roleSupport) so you assert the precise warning emitted when a "developer" role is coerced to "system".examples/ts-cjs/openaiResponsesApi.ts (1)
39-48: Consider making the schema name configurable.The schema name
"COTReasoning"is hardcoded. If the prompt template defines an output schema, the name might need to come from the schema itself or be configurable.💡 Suggested improvement
if (formattedPrompt.outputSchema) { + const schemaName = formattedPrompt.outputSchema.json_schema?.name || "ResponseSchema"; responseParams.text = { format: { type: "json_schema", strict: true, schema: formattedPrompt.outputSchema, - name: "COTReasoning", + name: schemaName, }, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/ts-cjs/openaiResponsesApi.ts` around lines 39 - 48, The hardcoded schema name "COTReasoning" in the responseParams.text block should be made configurable: change the code that sets responseParams.text.format.name to derive the name from the provided formattedPrompt.outputSchema (e.g., use formattedPrompt.outputSchema.name if present) or from a new configuration parameter passed into the function that builds responseParams; ensure you fall back to a sensible default (e.g., "COTReasoning") when neither is provided so existing behavior remains stable. Locate the assignment around responseParams.text and modify it to prefer formattedPrompt.outputSchema.name or a supplied config value before using the hardcoded string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@examples/ts-cjs/openaiResponsesApi.ts`:
- Around line 39-48: The hardcoded schema name "COTReasoning" in the
responseParams.text block should be made configurable: change the code that sets
responseParams.text.format.name to derive the name from the provided
formattedPrompt.outputSchema (e.g., use formattedPrompt.outputSchema.name if
present) or from a new configuration parameter passed into the function that
builds responseParams; ensure you fall back to a sensible default (e.g.,
"COTReasoning") when neither is provided so existing behavior remains stable.
Locate the assignment around responseParams.text and modify it to prefer
formattedPrompt.outputSchema.name or a supplied config value before using the
hardcoded string.
In `@test/openaiResponses.test.ts`:
- Around line 145-160: The test "developer coerced to system for openai_chat"
currently mocks console.warn but doesn't assert the warning content; update the
test that constructs OpenAILLMAdapter and calls prepareMessages to assert the
mocked warnSpy was called with the expected coercion message (e.g., using
expect(warnSpy).toHaveBeenCalledWith(...) or a regex) after prepareMessages
runs, keeping warnSpy.mockRestore() afterward; reference the OpenAILLMAdapter
instantiation and the call to prepareMessages(messages, adapter.roleSupport) so
you assert the precise warning emitted when a "developer" role is coerced to
"system".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 63bf4975-0e7b-40ea-afcb-05b4c60fdccd
⛔ Files ignored due to path filters (1)
examples/ts-cjs/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
CHANGELOG.mdexamples/ts-cjs/openaiResponsesApi.tsexamples/ts-cjs/package.jsonpackage.jsonsrc/model.tssrc/resources/prompts.tstest/openaiResponses.test.tstest/sdk.test.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/ts-cjs/openaiResponsesApi.ts`:
- Around line 17-22: The example calls fpClient.prompts.getFormatted(...) but
doesn't force the Responses formatter, so if the stored template flavor is e.g.
openai_chat the returned llmPrompt and toolSchema will be shaped for the wrong
API and openaiClient.responses.create(...) will fail; update the getFormatted
call (fpClient.prompts.getFormatted) to explicitly request the Responses
formatter/flavor (pass the option that forces "responses" or equivalent
formatter) so the returned formattedPrompt, llmPrompt and toolSchema are
compatible with openaiClient.responses.create.
In `@src/resources/prompts.ts`:
- Around line 472-477: effectivePromptInfo currently only overrides flavorName
so promptInfo.provider can remain out of sync with the selected flavor; update
effectivePromptInfo to also set provider to the provider for finalFlavor (e.g.,
derive it from LLMAdapters.adapterForFlavor(finalFlavor) or a flavor->provider
mapping) so that subsequent calls like getCallInfo() and recording use the
correct provider; ensure this change is made before prepareMessages/toLLMSyntax
and any recording logic that reads promptInfo.provider.
- Around line 542-544: The allMessages(newMessage: unknown) method currently
appends raw OpenAI Responses API items which may include non-message types and
items without a role; update allMessages to only return normalized
ProviderMessage entries by filtering the incoming newMessage(s) to include only
items that represent actual messages (e.g., item.type === 'message' or items
that have a defined role) and mapping those items to the ProviderMessage shape
(ensure role, content/text, and optional name are set) before concatenating with
this.messages so downstream functions like bind() and prepareMessages() can
safely access .role without runtime errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e9fa9437-85dd-4643-a113-ac8d14444c49
📒 Files selected for processing (5)
examples/ts-cjs/openaiResponsesApi.tssrc/model.tssrc/resources/prompts.tstest/openaiResponses.test.tstest/sdk.test.ts
✅ Files skipped from review due to trivial changes (1)
- test/openaiResponses.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/sdk.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/resources/prompts.ts (1)
546-548:⚠️ Potential issue | 🟠 MajorOnly append normalized message items here.
The Responses API
outputis an Items array, not aProviderMessage[]; it can contain non-message items such asfunction_call,function_call_output, and reasoning-related items, and the response docs explicitly warn callers not to assumeoutputentries are assistant messages. Appending that array verbatim here means the nextprepareMessages()pass can hitmessage.role === undefinedand either mis-handle history or throw. Filter to actual message items and normalize them intoProviderMessagebefore concatenating. (platform.openai.com)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/resources/prompts.ts` around lines 546 - 548, The allMessages method currently appends newMessage items verbatim which can include non-message entries from the Responses API; update allMessages (and the normalization it uses) to first filter newMessage items to only real message items (e.g., entries with a role or type indicating assistant/user/system) and map/normalize each into a ProviderMessage shape before concatenating to this.messages, so that prepareMessages() will never receive items missing message.role; reference the allMessages function and the ProviderMessage type for where to apply the filter+normalize.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/resources/prompts.ts`:
- Line 262: The FormattedPrompt currently stores the original template messages
(this.messages) instead of the processed ProviderMessage array returned by
prepareMessages(), causing allMessages() and systemContent to reflect messages
never actually sent; update FormattedPrompt to store the prepared messages (or
widen its internal messages field to ProviderMessage[]), so after calling
prepareMessages() you replace this.messages with the returned ProviderMessage[];
ensure functions like allMessages(), systemContent and any history concatenation
use the stored ProviderMessage[] so role rewrites (e.g., developer -> system)
persist through turns.
---
Duplicate comments:
In `@src/resources/prompts.ts`:
- Around line 546-548: The allMessages method currently appends newMessage items
verbatim which can include non-message entries from the Responses API; update
allMessages (and the normalization it uses) to first filter newMessage items to
only real message items (e.g., entries with a role or type indicating
assistant/user/system) and map/normalize each into a ProviderMessage shape
before concatenating to this.messages, so that prepareMessages() will never
receive items missing message.role; reference the allMessages function and the
ProviderMessage type for where to apply the filter+normalize.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: be982a9b-3f29-433f-b490-540f989c0efd
📒 Files selected for processing (2)
src/resources/prompts.tstest/openaiResponses.test.ts
✅ Files skipped from review due to trivial changes (1)
- test/openaiResponses.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/resources/prompts.ts (1)
550-552:⚠️ Potential issue | 🟠 Major
allMessages()still can’t safely round-trip Responsesoutput.
response.outputcan includefunction_call,function_call_output, andreasoningitems, and reasoning/tool-loop turns are meant to feed those items back on later requests. This method returns them asProviderMessage[], but the nextbind()/prepareMessages()path assumes every entry has.role, so stateless Responses history will either fail or force callers to drop required items. Use a separate Responses-item history type here, or reject non-message items instead of typing them asProviderMessage[]. (platform.openai.com)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/resources/prompts.ts` around lines 550 - 552, allMessages(newMessage: unknown) incorrectly treats arbitrary response.output items as ProviderMessage[]; that breaks later bind()/prepareMessages() which expect a .role on every entry. Fix by changing allMessages to either (a) filter/validate and only return proper ProviderMessage objects (reject or throw if an item lacks .role), or (b) introduce a separate ResponsesHistory type for function_call / function_call_output /reasoning items and return a union (e.g., ProviderMessage | ResponseItem) so callers can handle non-message entries explicitly; update callers of allMessages, bind(), and prepareMessages() to handle the new type or to only accept entries with .role. Ensure references to response.output, function_call, function_call_output, and reasoning are preserved in the new ResponsesHistory type if you choose the union approach.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/model.ts`:
- Around line 159-162: OPENAI_RESPONSES_ROLE_SUPPORT incorrectly includes
"system" and "tool" and OpenAIResponsesAdapter.toLLMSyntax() currently strips
system messages and misserializes tool outputs; update
OPENAI_RESPONSES_ROLE_SUPPORT to remove "system" and "tool" from the supported
set, modify OpenAIResponsesAdapter.toLLMSyntax() to preserve system messages in
the output (do not filter them out), and either implement proper serialization
of tool messages as Responses API function_call_output items or make the adapter
reject/translate the "tool" role until that serialization is added; apply the
same change to the other role-support constant referenced similarly elsewhere in
the file.
---
Duplicate comments:
In `@src/resources/prompts.ts`:
- Around line 550-552: allMessages(newMessage: unknown) incorrectly treats
arbitrary response.output items as ProviderMessage[]; that breaks later
bind()/prepareMessages() which expect a .role on every entry. Fix by changing
allMessages to either (a) filter/validate and only return proper ProviderMessage
objects (reject or throw if an item lacks .role), or (b) introduce a separate
ResponsesHistory type for function_call / function_call_output /reasoning items
and return a union (e.g., ProviderMessage | ResponseItem) so callers can handle
non-message entries explicitly; update callers of allMessages, bind(), and
prepareMessages() to handle the new type or to only accept entries with .role.
Ensure references to response.output, function_call, function_call_output, and
reasoning are preserved in the new ResponsesHistory type if you choose the union
approach.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 84e29d8b-39d4-4ec4-872e-02f2ea8ea2e8
📒 Files selected for processing (3)
src/model.tssrc/resources/prompts.tstest/openaiResponses.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/openaiResponses.test.ts
Summary
openai_responsesflavor withOpenAIResponsesAdapter— strips system messages, wraps messages in{type: "message", ...}format, and formats tool schemas in the flat Responses API styleprepareMessages()—developerrole passes through foropenai_responses, coerces tosystemforopenai_chat, throws for unsupported flavorstoolrole support for OpenAI adapters (bothOpenAILLMAdapterandOpenAIResponsesAdapter)allMessages()now accepts arrays to support Responses APIcompletion.outputopenai_responsesexamples/ts-cjs/openaiResponsesApi.tsMirrors Python SDK changes from freeplayai/freeplay-python#17 and freeplayai/freeplay-python#18.
Note
Add OpenAI Responses API adapter and developer role support across all flavors
OpenAIResponsesAdapter(newopenai_responsesflavor) in src/model.ts that dropssystem-role messages and wraps remaining messages as{type: 'message', ...}for the OpenAI Responses API.RoleSupportpolicies per adapter to validate, coerce, or reject unsupported roles via a newprepareMessagesfunction;developerrole is coerced tosystemfor standard OpenAI and added toStrictChatMessage.BoundPrompt.formatin src/resources/prompts.ts to apply role coercion before callingtoLLMSyntax, andformatToolSchema/formatOutputSchemato handle theopenai_responsesformat.FormattedPrompt.allMessagesnow accepts either a single message or an array of messages.Changes since #15 opened
flavorNameparameter toprepareMessagesfunction and updated unsupported role error messages to include the specific flavor name with guidance directing users to update their prompt template in Freeplay to use a flavor supporting the unsupported role [945466e]BoundPrompt.formatPromptmethod to pass flavor name argument when callingprepareMessages[945466e]openaiResponses.test.tsto pass flavor name argument toprepareMessagesfunction calls [945466e]Macroscope summarized aa6f90c.
Summary by CodeRabbit
New Features
Tests
Chores