fix(functions): honor function_arguments_key when building the tool grammar - #11677
Open
Anai-Guo wants to merge 2 commits into
Open
fix(functions): honor function_arguments_key when building the tool grammar#11677Anai-Guo wants to merge 2 commits into
Anai-Guo wants to merge 2 commits into
Conversation
…rammar
All four call sites of `Functions.ToJSONStructure(name, args string)` pass
`FunctionsConfig.FunctionNameKey` as *both* arguments, so
`FunctionArgumentsKey` never reaches the grammar generator.
`ToJSONStructure` writes the two properties into the same map:
property[nameKey] = FunctionName{Const: function.Name}
property[argsKey] = Argument{...}
When `nameKey == argsKey` the second assignment overwrites the first, so a
model configured with `function_name_key` gets a grammar carrying only the
arguments object -- the `{"const": "<function name>"}` constraint is gone and
the grammar can no longer express which function was called.
With `function_name_key: function`, the generated property set collapses from
{"function": {"const": "get_weather"}, "arguments": {...}}
to
{"function": {"type": "object", "properties": {...}}}
Setting only `function_arguments_key` is equally broken in the other
direction: the grammar keeps emitting `arguments` while `ParseFunctionCall`
(pkg/functions/parse.go) looks up the configured key, so the parsed call comes
back with its arguments empty.
The default configuration is unaffected -- with both keys empty
`ToJSONStructure` falls back to `name`/`arguments` for both parameters, which
is why this went unnoticed.
The existing `ToJSONStructure()` unit test already calls the helper with two
distinct keys, so only the call sites were wrong. Extend that test with a case
that keeps both custom keys distinct and asserts the two properties survive.
Signed-off-by: Anai-Guo <antai12232931@outlook.com>
Route grammar construction through FunctionsConfig so the regression test covers the key wiring used by every endpoint. Assisted-by: Codex:gpt-5
Collaborator
|
I pushed a small follow-up that routes grammar construction through The DCO check rejects my bot-authored follow-up because project policy does not allow me to add a human |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
FunctionsConfigexposes two independent settings:All four call sites of
Functions.ToJSONStructure(name, args string)passFunctionNameKeyas both arguments, soFunctionArgumentsKeyneverreaches the grammar generator:
core/http/endpoints/openai/chat.go:352core/http/endpoints/openai/realtime_model.go:292core/http/endpoints/openresponses/responses.go:223core/http/endpoints/openresponses/websocket.go:286ToJSONStructurewrites both entries into the same map:When
nameKey == argsKey, the second assignment overwrites the first.Effect
Setting
function_name_keycollapses the generated property set from twoproperties to one, and the surviving one is the arguments object — the
{"const": "<function name>"}constraint is gone, so the grammar can nolonger express which function was called. Running the real
ToJSONStructureover a one-function list:ToJSONStructure("function", "function")(today, withfunction_name_key: function){"function":{"type":"object","properties":{"city":{"type":"string"}}}}ToJSONStructure("function", "parameters")(this PR){"function":{"const":"get_weather"},"parameters":{"type":"object","properties":{"city":{"type":"string"}}}}ToJSONStructure("", "")(default config){"name":{"const":"get_weather"},"arguments":{...}}Setting only
function_arguments_keyis broken in the other direction: thegrammar keeps emitting
arguments, whileParseFunctionCall(
pkg/functions/parse.go:944-949) — the one place that does readFunctionArgumentsKey— looks up the configured key, finds nothing, andreturns the call with empty arguments.
The default configuration is unaffected: with both fields empty,
ToJSONStructurefalls back toname/argumentsfor both parameters. Thatis why this went unnoticed.
Fix
Pass
FunctionArgumentsKeyas the second argument at the four call sites.FunctionArgumentsKeyis astringfield on the same struct asFunctionNameKey, so this is a one-token change per call site with nobehavior change for the default configuration.
Notes
ToJSONStructure()unit test already calls the helper with twodistinct keys (
"function","arguments"), i.e. the helper's contract wasright and only the call sites were wrong. This PR extends that test with a
case that keeps both custom keys distinct and asserts the two properties
survive; it fails against the collapsed structure.
pkg/functionscannot be compiled standalone here (pkg/grpc/protoisgenerated at build time), so the table above was produced by running the
unmodified
ToJSONStructure/function_structure.gosources in an isolatedmodule with a stubbed
xlog.gofmt -lis clean on all five files.{}on/v1/responses), but I could not confirm that: the config in that reportdoes not set either key, so this is offered as a separate, independently
reproducible defect rather than a fix for that issue.
Deliberately not in scope
The two streaming tool-call emitters
(
core/http/endpoints/openai/chat_stream_workers.go:45-50andcore/http/endpoints/openresponses/responses.go:1793-1797) hardcode"name"and
"arguments"when readingParseJSONIterativeoutput, so they also ignoreboth config keys. That is a separate defect needing a signature change and test
updates; I left it out to keep this PR reviewable and can follow up if wanted.
🤖 Generated with Claude Code