Custom Serializer for Message Creation - Resolves Issue 552 #717
Conversation
BenjaminDavidPinter
requested review from
ShivangiReja,
joseharriaga and
trrwilson
as code owners
|
@BenjaminDavidPinter: Thanks very much for your contribution and your interest in improving the OpenAI developer experience. We appreciate the efforts that you put into investigating and writing this up as well. It sounds like the long-term solution would be to fix the spec such that the models are distinct. In the short-term, I think your approach here makes sense. I'm going to ask @ShivangiReja and @christothes to add some additional perspective. |
|
@joseharriaga: Would you please take a look at the investigation write-up here and then let's sync on thoughts to push upstream for spec changes? |
|
@jsquire thanks! I'll fix up the conflicts when I get home tonight, and we can wrap this one up (for now), until we get those models split out in the spec. |
|
Thanks, @BenjaminDavidPinter! I'm going to run the tests and then wait for Jose to get back for final approval. |
| using OpenAI.Tests.Utility; | ||
| using OpenAI.Tests.Utility; | ||
| using OpenAI.VectorStores; | ||
| using OpenAI.VectorStores; |
There was a problem hiding this comment.
Could you fix the duplicated namespaces?
| { | ||
| // First, we need to upload a simple test file. | ||
| OpenAIFileClient fileClient = GetTestClient<OpenAIFileClient>(TestScenario.Files); | ||
| OpenAIFile testFile = fileClient.UploadFile( |
There was a problem hiding this comment.
Switch to the async version of this method: UploadFileAsync
|
|
||
| AssistantClient client = GetTestClient(); | ||
|
|
||
| var thread = client.CreateThread(); |
There was a problem hiding this comment.
- Switch to the async version of this method:
CreateThreadAsync. - Then call
Validate(thread);
| AssistantClient client = GetTestClient(); | ||
|
|
||
| var thread = client.CreateThread(); | ||
| var assistant = client.CreateAssistant("gpt-4o-mini"); |
There was a problem hiding this comment.
- Switch to the async version of this method:
CreateAssistantAsync - Then call
Validate(assistant);
| var assistant = client.CreateAssistant("gpt-4o-mini"); | ||
|
|
||
| var message = await client.CreateMessageAsync( | ||
| thread.Value.Id, |
There was a problem hiding this comment.
nit: Add indentation.
| new MessageCreationAttachment(testFile.Id, new List<ToolDefinition>() { ToolDefinition.CreateFileSearch() }), | ||
| new MessageCreationAttachment(testFile.Id, new List<ToolDefinition>() { ToolDefinition.CreateCodeInterpreter() }) | ||
| ] | ||
| }); |
There was a problem hiding this comment.
Call Validate(message);
|
I approved the PR. Just added a few comments. 🙂 |
3 participants
Fixes Issue 552
I'm actually surprised this hasn't come up more often, the FileSearch/CodeGen combo allows for really simple data analysis.
Anyway; I discovered the root cause, and here what I changed.
Root cause
The file_search tool definition in the spec (models.tsp) is shared between Assistant creation and Message tool serialization.
The generated serializer (FileSearchToolDefinition.Serialization.cs) force emits a "file_search" object when serializing the tool, which ends up injected into the message attachment payload (an unsupported property for that request).
Solution
I did not edit the generated FileSearchToolDefinition.Serialization.cs. Instead I intercepted message-level tool serialization in MessageCreationAttachment.cs.
I buffer the generated JSON for each tool, parse it, remove any "file_search" property, then write the cleaned JSON into the message attachment payload. This prevents the unwanted property from being sent.
Why this approach: generated code can’t be changed directly, so post-processing the generated output in the custom serializer is the safest fix that keeps generated behavior intact while preventing invalid request payloads.
Testing
I was able to recreate the issue with the added test.
After implementing the fix, the test passes.
All tests with Category "Assistant" pass.
Recommendation
Split the spec definition so assistant-level tool properties (like file_search) are not shared with message-level tool representations. Use separate types for tools used in assistant creation vs tools embedded in messages, or mark server-only/readOnly fields so the generator will not emit them into request models. If the Responses API already addresses this separation, consider aligning the assistants spec with that approach.