[dotnet-code] Consolidate OpenAI tool schema conversion#535
[dotnet-code] Consolidate OpenAI tool schema conversion#535michelle-clayton-work wants to merge 2 commits into
Conversation
Share the internal schema conversion helper between chat completions and responses tool request construction so the Go provider shape stays closer to the central .NET FunctionDefinition model without changing API or request behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Consolidates duplicated OpenAI function-tool schema conversion logic in the Chat Completions and Responses request builders by reusing the existing internal schemaToMap helper, aiming to keep the Go OpenAI provider’s internal shape closer to the .NET FunctionDefinition model without changing the generated payloads.
Changes:
- Replaced inlined JSON marshal/unmarshal schema conversion in
chat.gowith a call toschemaToMap. - Replaced inlined JSON marshal/unmarshal schema conversion in
responses.gowith a call toschemaToMap.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| provider/openaiprovider/chat.go | Switches function-tool schema conversion to use schemaToMap when building chat completion tool payloads. |
| provider/openaiprovider/responses.go | Switches function-tool schema conversion to use schemaToMap when building responses tool payloads. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| schema := tl.Schema() | ||
| funcParams, err := schemaToMap(schema) | ||
| if err != nil { | ||
| return openai.ChatCompletionNewParams{}, fmt.Errorf("failed to convert function tool schema to JSON format: %w", err) | ||
| } |
| schema := tl.Schema() | ||
| funcParams, err := schemaToMap(schema) | ||
| if err != nil { | ||
| return responses.ResponseNewParams{}, fmt.Errorf("failed to convert schema for function tool %q (type %T) to JSON format: %w", name, schema, err) | ||
| } |
This comment has been minimized.
This comment has been minimized.
|
@copilot respond to PR comments |
Co-authored-by: michelle-clayton-work <262183035+michelle-clayton-work@users.noreply.github.com>
Fixed in commit |
Cross-Repo Consistency ReviewScope: This PR modifies only unexported provider internals ( Verdict: ✅ No parity issues. This is a Go-internal refactor with no user-visible API or behavioral change, so cross-repo alignment with .NET and Python implementations is unaffected.
|
Summary
Consolidated the duplicated function-tool schema conversion in the OpenAI chat completions and responses request builders through the existing unexported
schemaToMaphelper. This keeps the Go provider internals closer to the .NET provider's centralFunctionDefinitionshape while preserving the generated request payloads..NET Reference
dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/ChatCompletions/Models/Tool.cs- central tool/function definition model used when constructing OpenAI tool payloads.Public API and Behavior
No public Go API changed. No intentional behavior change was made.
Tests
gofmt -w provider/openaiprovider/chat.go provider/openaiprovider/responses.gogo test ./provider/openaiproviderNotes
Rejected candidates:
dotnet/src/Microsoft.Agents.AI/Skills/Decorators/FilteringAgentSkillsSource.cs- Go already exposes skill filtering throughContextProviderOptions.SkillFilter; adding a .NET-style source decorator would add new public API, so it was rejected.dotnet/src/Microsoft.Agents.AI.Workflows/Checkpointing/JsonConverterDictionarySupportBase.cs- Go checkpoint JSON uses explicit entry structs rather than dictionary-key converters, so there was no narrow structural cleanup that would improve parity without churn.dotnet/src/Microsoft.Agents.AI.Workflows/Specialized/Magentic/MagenticConstants.cs- no corresponding Go Magentic internals exist in the current sampled surface, so adding constants would be placeholder feature shape rather than portability cleanup.Open
[dotnet-code]PR #484 already covers workflow edge construction, so adjacent workflow-edge work was avoided.Closes #498