Fix: Corrects handling of empty function call args

[JasonTheAdams]

This attempts to address the bug found in this trac ticket: https://core.trac.wordpress.org/ticket/64483

Basically, when models send a function call they provide the arguments for the function every time, even when there are no arguments (e.g. get_current_time()). It's passed as an empty JSON object: args: {}. This is converted into an array, because empty lists and associative arrays can't be differentiated in PHP. As a result, an empty array is passed as the arguments of a new FunctionCall. This is wrong. An empty JSON object means "there are no arguments" which, in PHP, would be null.

This attempts to correct this in two places:

1. By normalizing an empty array in FunctionCall to null. This is because this is likely a common scenario Providers will run into, and passing an empty array is otherwise not meaningful.
2. When including the FunctionCall as part of a message history, it handles converting null into an empty object correctly, as the model would expect.

[github-actions]

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: JasonTheAdams <jason_the_adams@git.wordpress.org>
Co-authored-by: justlevine <justlevine@git.wordpress.org>
Co-authored-by: felixarntz <flixos90@git.wordpress.org>

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

[JasonTheAdams]

Having spoken with @felixarntz, I don't think this will work as it is. We can't generically solve this with normalization because we can't make the assumption that every function declaration's arguments is an object — it may be an array, in which case an empty array is valid. Alas, we'll need to carefully solve this on a per Provider basis. We'll iterate this PR to get there.

[justlevine]

We can't generically solve this with normalization because we can't make the assumption that every function declaration's arguments is an object — it may be an array, in which case an empty array is valid. Alas, we'll need to carefully solve this on a per Provider basis.

@JasonTheAdams @felixarntz since this sounds like a lot of work, I just want to double-check: is the need to coerce {} to null an issue for all FunctionCalls / declarations over PHP, or something that should really be handled on the consumer level if they need to differentiate?

If it's the latter (and therefore not a php client issue but a WP client issue ) we could probably handle the normalization in https://github.com/WordPress/wp-ai-client/blob/dacb6e8b73ff3097c41a256dfe26fb3d0cea9f48/includes/Builders/Helpers/Ability_Function_Resolver.php#L91 and rely on the fact that Abilities API has verbose schema checking. E.g (psuedocode from my phone):

# wp-ai-client/includes/Builders/Helpers/Ability_Function_Resolver.php

// Execute the ability.
$args   = $call->getArgs();

if ( empty( $args ) ) {
  // Coerce the falsey shape if it's not supported.
  $args = true === rest_validate_value_from_schema( $args, $ability->get_input_schema() ) ? $args : null;
}

$result = $ability->execute( $args );

It's an empty var so the double validation is negligible.

[felixarntz]

@justlevine I think there are multiple problems. Some of it may be related to logic from WP AI Client or Abilities API, but after looking at just the PHP AI Client SDK yesterday in relation to this, I noticed at least a few problems that come from this repo alone.

In a nutshell: The way providers handle "no args" differs, and we don't handle this scenario properly for some of them. And those differences cover both the FunctionDeclaration (how the tool shape itself is passed) and the FunctionCall (how the model returns a tool call).

• Some providers require the relevant parameters/input field to be omitted.
• Other providers require the relevant parameters/input field to be an empty object.

#176 was started as a result of this research. Unfortunately, the providers' documentation is not great on details like this in most cases. For all 3 providers, after careful review of the relevant API reference bits, we still have questions. So the only way to know for sure is to try different variants and see what the respective provider does. Obviously #176 will have far more benefits than just for this concrete problem, but it showed that only by testing the actual provider connection and behavior we'll have proper coverage - and we can even use it to learn about subtle nuances.

[JasonTheAdams]

Ok, @felixarntz! I've switched this around! Here's what I've done:

1. I adjusted it so 0-argument function calling is handled on a per-provider basis. It's nuanced, but it's really not that bad.
2. I added a bunch more integration tests that really gets into function calling, touching on FunctionDeclaration, FunctionCall, and FunctionResponse.

All the tests are passing for all the providers and I'm feeling pretty darn good about these new integration tests. I just wish we'd added them months ago. 😎