Skip to content

Fix incorrect JSON schema for FunctionCall and FunctionResponse #220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
felixarntz merged 3 commits into from
Mar 18, 2026
Merged

Fix incorrect JSON schema for FunctionCall and FunctionResponse #220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
da3090b
Use anyOf instead of oneOf in FunctionCall and FunctionResponse.
felixarntz Mar 18, 2026
da4986b
Fix FunctionResponse ID and name not being nullable (only one is requ…
felixarntz Mar 18, 2026
22d2d16
Align FunctionResponse::fromArray implementation.
felixarntz Mar 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions src/Common/AbstractDataTransferObject.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,13 @@ private function convertEmptyArraysToObjects($data, array $schema)
}
}

// Handle oneOf schemas - just use the first one
if (isset($schema['oneOf']) && is_array($schema['oneOf'])) {
foreach ($schema['oneOf'] as $possibleSchema) {
if (is_array($possibleSchema)) {
return $this->convertEmptyArraysToObjects($data, $possibleSchema);
// Handle oneOf/anyOf schemas - just use the first one
foreach (['oneOf', 'anyOf'] as $keyword) {
if (isset($schema[$keyword]) && is_array($schema[$keyword])) {
foreach ($schema[$keyword] as $possibleSchema) {
if (is_array($possibleSchema)) {
return $this->convertEmptyArraysToObjects($data, $possibleSchema);
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Tools/DTO/FunctionCall.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public static function getJsonSchema(): array
'description' => 'The arguments to pass to the function.',
],
],
'oneOf' => [
'anyOf' => [
[
'required' => [self::KEY_ID],
],
Expand Down
46 changes: 27 additions & 19 deletions src/Tools/DTO/FunctionResponse.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*
* @since 0.1.0
*
* @phpstan-type FunctionResponseArrayShape array{id: string, name: string, response: mixed}
* @phpstan-type FunctionResponseArrayShape array{id?: string, name?: string, response: mixed}
*
* @extends AbstractDataTransferObject<FunctionResponseArrayShape>
*/
Expand All @@ -25,14 +25,14 @@ class FunctionResponse extends AbstractDataTransferObject
public const KEY_NAME = 'name';
public const KEY_RESPONSE = 'response';
/**
* @var string The ID of the function call this is responding to.
* @var string|null The ID of the function call this is responding to.
*/
private string $id;
private ?string $id;

/**
* @var string The name of the function that was called.
* @var string|null The name of the function that was called.
*/
private string $name;
private ?string $name;

/**
* @var mixed The response data from the function.
Expand All @@ -44,12 +44,17 @@ class FunctionResponse extends AbstractDataTransferObject
*
* @since 0.1.0
*
* @param string $id The ID of the function call this is responding to.
* @param string $name The name of the function that was called.
* @param string|null $id The ID of the function call this is responding to.
* @param string|null $name The name of the function that was called.
* @param mixed $response The response data from the function.
* @throws InvalidArgumentException If neither id nor name is provided.
*/
public function __construct(string $id, string $name, $response)
public function __construct(?string $id, ?string $name, $response)
{
if ($id === null && $name === null) {
throw new InvalidArgumentException('At least one of id or name must be provided.');
}

$this->id = $id;
$this->name = $name;
$this->response = $response;
Expand Down Expand Up @@ -114,7 +119,7 @@ public static function getJsonSchema(): array
'description' => 'The response data from the function.',
],
],
'oneOf' => [
'anyOf' => [
[
'required' => [self::KEY_RESPONSE, self::KEY_ID],
],
Expand All @@ -134,11 +139,19 @@ public static function getJsonSchema(): array
*/
public function toArray(): array
{
return [
self::KEY_ID => $this->id,
self::KEY_NAME => $this->name,
self::KEY_RESPONSE => $this->response,
];
$data = [];

if ($this->id !== null) {
$data[self::KEY_ID] = $this->id;
}

if ($this->name !== null) {
$data[self::KEY_NAME] = $this->name;
}

$data[self::KEY_RESPONSE] = $this->response;

return $data;
}

/**
Expand All @@ -150,11 +163,6 @@ public static function fromArray(array $array): self
{
static::validateFromArrayData($array, [self::KEY_RESPONSE]);

// Validate that at least one of id or name is provided
if (!array_key_exists(self::KEY_ID, $array) && !array_key_exists(self::KEY_NAME, $array)) {
throw new InvalidArgumentException('At least one of id or name must be provided.');
}

return new self(
Copy link
Copy Markdown
Member

@gziolo gziolo Mar 18, 2026

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
Copy Markdown
Member Author

@felixarntz felixarntz Mar 18, 2026

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.

$array[self::KEY_ID] ?? null,
$array[self::KEY_NAME] ?? null,
Expand Down
76 changes: 76 additions & 0 deletions tests/unit/Common/AbstractDataTransferObjectTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,82 @@ public static function getJsonSchema(): array
$this->assertStringContainsString('"data":{}', $json);
}

/**
* Tests handling of anyOf schemas uses first schema without validation.
*
* @return void
*/
public function testAnyOfSchemaHandling(): void
{
$testObject = new class extends AbstractDataTransferObject {
public function toArray(): array
{
return [
'dynamicField' => [
'type' => 'objectType',
'data' => [],
],
];
}

public static function fromArray(array $array): self
{
return new static();
}

public static function getJsonSchema(): array
{
return [
'type' => 'object',
'properties' => [
'dynamicField' => [
'anyOf' => [
[
'type' => 'object',
'properties' => [
'type' => [
'type' => 'string',
'const' => 'objectType'
],
'data' => [
'type' => 'object',
'properties' => []
],
],
'required' => ['type', 'data'],
],
[
'type' => 'object',
'properties' => [
'type' => [
'type' => 'string',
'const' => 'arrayType'
],
'data' => [
'type' => 'array',
'items' => ['type' => 'string']
],
],
'required' => ['type', 'data'],
],
],
],
],
];
}
};

$result = $testObject->jsonSerialize();

$this->assertIsArray($result);
$this->assertIsArray($result['dynamicField']);
$this->assertInstanceOf(stdClass::class, $result['dynamicField']['data']);

$json = json_encode($result);
$this->assertIsString($json);
$this->assertStringContainsString('"data":{}', $json);
}

/**
* Tests that arrays of objects are processed recursively.
*
Expand Down
10 changes: 5 additions & 5 deletions tests/unit/Tools/DTO/FunctionCallTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,15 +127,15 @@ public function testJsonSchema(): void
$this->assertContains('array', $schema['properties'][FunctionCall::KEY_ARGS]['type']);
$this->assertContains('null', $schema['properties'][FunctionCall::KEY_ARGS]['type']);

// Check oneOf for required fields
$this->assertArrayHasKey('oneOf', $schema);
$this->assertCount(2, $schema['oneOf']);
// Check anyOf for required fields
$this->assertArrayHasKey('anyOf', $schema);
$this->assertCount(2, $schema['anyOf']);

// First option: only id required
$this->assertEquals([FunctionCall::KEY_ID], $schema['oneOf'][0]['required']);
$this->assertEquals([FunctionCall::KEY_ID], $schema['anyOf'][0]['required']);

// Second option: only name required
$this->assertEquals([FunctionCall::KEY_NAME], $schema['oneOf'][1]['required']);
$this->assertEquals([FunctionCall::KEY_NAME], $schema['anyOf'][1]['required']);
}

/**
Expand Down
Loading
Loading