fix: add missing 'items' to array schemas in Codex tool parameters #1275
Conversation
Summary of ChangesHello @huynguyen03dev, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements a crucial fix to ensure compatibility with OpenAI's strict schema validation for Codex tool parameters. It introduces a new utility that proactively modifies tool schemas by adding a default Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a utility function, FixCodexToolSchemas, to automatically add a missing items property to array schemas in tool parameters before sending requests to the Codex API. This change addresses a strict schema validation requirement from OpenAI. The fix is applied in both Execute and ExecuteStream methods of the CodexExecutor. The new utility is well-tested with a comprehensive suite of unit tests covering various scenarios. The overall implementation is solid. I've included one suggestion to refactor a recursive function for improved performance and readability.
| func findArrayTypePaths(node gjson.Result, path string) []string { | ||
| var paths []string | ||
|
|
||
| if node.IsObject() { | ||
| if isArrayType(node) { | ||
| paths = append(paths, path) | ||
| } | ||
| node.ForEach(func(key, value gjson.Result) bool { | ||
| newPath := key.String() | ||
| if path != "" { | ||
| newPath = path + "." + key.String() | ||
| } | ||
| paths = append(paths, findArrayTypePaths(value, newPath)...) | ||
| return true | ||
| }) | ||
| } else if node.IsArray() { | ||
| for i, elem := range node.Array() { | ||
| newPath := strconv.Itoa(i) | ||
| if path != "" { | ||
| newPath = path + "." + strconv.Itoa(i) | ||
| } | ||
| paths = append(paths, findArrayTypePaths(elem, newPath)...) | ||
| } | ||
| } | ||
|
|
||
| return paths | ||
| } |
There was a problem hiding this comment.
The recursive implementation of findArrayTypePaths can be inefficient due to the use of append(slice, anotherSlice...) within the recursion. Each call creates a new slice, leading to multiple allocations. A more performant and arguably clearer approach is to use a closure to append to a single paths slice, avoiding the overhead of repeated slice allocations.
// findArrayTypePaths recursively finds all paths where type="array".
func findArrayTypePaths(node gjson.Result, path string) []string {
var paths []string
var find func(gjson.Result, string)
find = func(n gjson.Result, p string) {
if n.IsObject() {
if isArrayType(n) {
paths = append(paths, p)
}
n.ForEach(func(key, value gjson.Result) bool {
childPath := key.String()
if p != "" {
childPath = p + "." + childPath
}
find(value, childPath)
return true
})
} else if n.IsArray() {
for i, elem := range n.Array() {
childPath := strconv.Itoa(i)
if p != "" {
childPath = p + "." + childPath
}
find(elem, childPath)
}
}
}
find(node, path)
return paths
}
OpenAI's strict schema validation rejects array-typed schemas without
an 'items' definition. This fix adds 'items: {}' to any array schema
missing it before sending requests to the Codex API.
Fixes error: Invalid schema for function 'tb__ask_user': In
context=('properties', 'options'), array schema missing items.
Co-authored-by: Amp <[email protected]>
huynguyen03dev
force-pushed
the
fix/codex-array-schema-missing-items
branch
from
908f240 to
f7100c8
Compare
|
@luispater can you take a look at this problem? This also happens when using amp deepmode, and forward the request to amp without mapping. |
1 participant
Problem
OpenAI's strict schema validation rejects tool schemas where array properties lack an
itemsdefinition:Invalid schema for function 'tb__ask_user': In context=('properties', 'options'), array schema missing items.
Solution
Add
FixCodexToolSchemas()utility that automatically addsitems: {}to any array-typed schema missing it before sending requests to the Codex API.Changes
internal/util/codex_schema.go- Schema fixing utilityinternal/util/codex_schema_test.go- Unit testsinternal/runtime/executor/codex_executor.go- Apply fix before sending requestsTesting