Skip to content

fix(openai_compat): accept object tool call arguments#1292

Merged
yinwm merged 1 commit intosipeed:mainfrom
qs3c:fix/1287-openai-compat-tool-call-args-object
Mar 11, 2026
Merged

fix(openai_compat): accept object tool call arguments#1292
yinwm merged 1 commit intosipeed:mainfrom
qs3c:fix/1287-openai-compat-tool-call-args-object

Conversation

@qs3c
Copy link
Contributor

@qs3c qs3c commented Mar 10, 2026

Summary

  • accept OpenAI-compatible tool call arguments returned as either JSON strings or JSON objects
  • preserve the existing raw fallback/logging path for malformed or unsupported payloads
  • add a regression test for object-form tool call arguments

Fixes #1287.

Testing

  • go test ./pkg/providers/openai_compat
  • go test ./pkg/providers/... (fails on existing CodexCliProvider PATH-based tests on Windows; unrelated to this change)

@sipeed-bot sipeed-bot bot added type: bug Something isn't working domain: provider go Pull requests that update go code labels Mar 10, 2026
Copy link

@nikolasdehor nikolasdehor left a comment

Choose a reason for hiding this comment

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

Solid fix for handling providers that return tool call arguments as JSON objects instead of JSON strings.

Review notes:

  1. decodeToolCallArguments -- the function handles all three cases correctly:

    • null/empty: returns empty map
    • string (standard OpenAI format): unmarshals the string as JSON
    • object (non-standard but valid): uses the map directly
    • other types: logs and falls back to raw string
  2. json.RawMessage for Arguments field -- this is the correct type to use when the wire format can be either a string or an object. The previous string type would fail to unmarshal object-form arguments.

  3. Graceful fallback -- malformed or unsupported argument types still produce a result with a "raw" key, preserving debuggability. This matches the previous behavior.

  4. Test coverage -- the new test TestProviderChat_ParsesToolCallsWithObjectArguments creates a realistic server response with object-form arguments and verifies both string and boolean argument values are preserved.

  5. Minor: the bytes.TrimSpace call on raw before checking for "null" is good defensive coding -- some providers may return " null " with whitespace.

LGTM.

@tsterbak
Copy link

What is missing for this PR to get merged?

Would be great to get it working again :)

Copy link
Collaborator

@yinwm yinwm left a comment

Choose a reason for hiding this comment

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

Reviewed and compared with #1379. This PR is more complete:

  • Preserves original arguments string via ToolCall.Function.Arguments
  • Adds Type field for completeness
  • Better test coverage including Function field validation

The other PR (#1379) loses the raw arguments string which may be needed downstream.

LGTM 👍

@yinwm yinwm merged commit 49204df into sipeed:main Mar 11, 2026
4 checks passed
dj-oyu pushed a commit to dj-oyu/picoclaw that referenced this pull request Mar 14, 2026
dj-oyu pushed a commit to dj-oyu/picoclaw that referenced this pull request Mar 16, 2026
j0904 pushed a commit to j0904/picoclaw that referenced this pull request Mar 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain: provider go Pull requests that update go code type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Tool calling fails

4 participants