Skip to content

Conversation

@rumpl
Copy link
Member

@rumpl rumpl commented Jan 16, 2026

With this update jsonschema decided that slices and such have a type of [null, string], which is correct in golang world, but it breaks Gemini so we need to tweak and remove all the array defined types and take only the type itself.

Also moved the code a bit and unified the way we convert parameters in openai

@rumpl rumpl requested a review from a team as a code owner January 16, 2026 15:36
rumpl added 3 commits January 16, 2026 16:44
These tests only test provider schemas so no need for them to be in
tools

Signed-off-by: Djordje Lukic <[email protected]>
With this update jsonschema decided that slices and such have a type of
[null, string], which is correct in golang world, but it breaks Gemini
so we need to tweak and remove all the array-defined types and take only
the type itself.

Signed-off-by: Djordje Lukic <[email protected]>
out := cagentExec(t, "testdata/fs_tools.yaml", "--model=mistral/mistral-small", "How many files in testdata/working_dir? Only output the number.")

require.Equal(t, "\n--- Agent: root ---\n\nCalling list_directory(path: \"testdata/working_dir\")\n\nlist_directory response → \"FILE README.me\\n\"\n1", out)
// NOTE: If you look at the LLM response, Mistral says it sees 2 files, yours truly got tired of re-running this test to get it to say "1".
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😂

}

// The Response API requires every parameter to be required
parameters = makeAllRequired(parameters)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this safe to remove? wasn't expecting this change here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

back then it wasn't. @rumpl have you tested it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't removed, I moved these to ConvertParametersToSchema so both Responses and Chat APIs will get the same treatment.

Yes I have tested this, our e2e tests still work and I tested with the responses api too

@krissetto krissetto merged commit 6d75e27 into docker:main Jan 16, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants