Skip to content

Fix incorrect JSON schema for FunctionCall and FunctionResponse#220

Merged
felixarntz merged 3 commits intotrunkfrom
fix/function-calling-one-of
Mar 18, 2026
Merged

Fix incorrect JSON schema for FunctionCall and FunctionResponse#220
felixarntz merged 3 commits intotrunkfrom
fix/function-calling-one-of

Conversation

@felixarntz
Copy link
Member

  • FunctionCall and FunctionResponse are currently using oneOf, which is not correct. It is totally valid for a function call or a function response to have both id and name set. So this needs to be anyOf instead.
    • It doesn't show in any PHP-only usage, but as soon as you do something where the schema matters (e.g. REST API), this schema will lead to errors.
    • Regardless of whether or not it errors though, it is simply semantically incorrect.
  • Somewhat related, the FunctionResponse was not handling its id and name the same way as FunctionCall. The properties weren't nullable, even though they should be - only one is required, though both are allowed. This was already reflected in parts of the class, but not consistently.

This PR fixes both of these bugs.

@felixarntz felixarntz added this to the 1.3.1 milestone Mar 18, 2026
@felixarntz felixarntz added the [Type] Bug An existing feature does not function as intended label Mar 18, 2026
@codecov
Copy link

codecov bot commented Mar 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.11%. Comparing base (211e603) to head (22d2d16).
⚠️ Report is 4 commits behind head on trunk.

Additional details and impacted files
@@             Coverage Diff              @@
##              trunk     #220      +/-   ##
============================================
+ Coverage     88.07%   88.11%   +0.03%     
- Complexity     1209     1212       +3     
============================================
  Files            60       60              
  Lines          3925     3928       +3     
============================================
+ Hits           3457     3461       +4     
+ Misses          468      467       -1     
Flag Coverage Δ
unit 88.11% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link

github-actions bot commented Mar 18, 2026

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: felixarntz <flixos90@git.wordpress.org>
Co-authored-by: gziolo <gziolo@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Looks good to me! The oneOfanyOf fix is semantically correct and the FunctionResponse alignment with FunctionCall resolves a real inconsistency. Well tested.

@@ -156,8 +169,8 @@ public static function fromArray(array $array): self
}

return new self(
Copy link
Member

Choose a reason for hiding this comment

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

Non-blocking nitpick (caught by agent): this fromArray() now uses array_key_exists with a defensive (string) cast, while FunctionCall::fromArray() still uses $array[self::KEY_ID] ?? null. The difference is subtle — array_key_exists distinguishes between a missing key and a key explicitly set to null. Neither causes a real-world issue, but it's a minor inconsistency between two sibling classes doing the same thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Great point, updated in 22d2d16.

@felixarntz felixarntz merged commit 2d6efec into trunk Mar 18, 2026
10 of 11 checks passed
@felixarntz felixarntz deleted the fix/function-calling-one-of branch March 18, 2026 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants