Skip to content

fix(ToolMap): remove static duplicate parameters handling in mapping openrouter#1004

Open
Aldiwildan77 wants to merge 1 commit intoprism-php:mainfrom
Aldiwildan77:fix/remove-duplicate-params-in-openrouter-toolmap
Open

fix(ToolMap): remove static duplicate parameters handling in mapping openrouter#1004
Aldiwildan77 wants to merge 1 commit intoprism-php:mainfrom
Aldiwildan77:fix/remove-duplicate-params-in-openrouter-toolmap

Conversation

@Aldiwildan77
Copy link
Copy Markdown

Problem

ToolMap::map() contains two parameters keys in the same PHP array for the function definition:

'function' => [
    'name' => $tool->name(),
    'description' => $tool->description(),
    // KEY 1 (conditional spread)
    ...$tool->hasParameters() ? [
        'parameters' => (function () use ($tool): array {
            $properties = $tool->parametersAsArray();

            return [
                'type' => 'object',
                'properties' => $properties === [] ? new \stdClass : $properties,
                'required' => $tool->requiredParameters(),
            ];
        })(),
    ] : [],
    // KEY 2 (unconditional) — always overwrites KEY 1
    'parameters' => [
        'type' => 'object',
        'properties' => $tool->hasParameters() ? $tool->parametersAsArray() : (object) [],
        'required' => $tool->requiredParameters(),
    ],
],

In PHP, duplicate array keys cause the last one to win. This means:

  1. KEY 1 is dead code — its output is always overwritten by KEY 2, including the stdClass empty-properties fallback that ensures correct JSON serialization as {} instead of []
  2. Tools without parameters receive an unnecessary parameters block with { "properties": {}, "required": [] } because KEY 2 is unconditional — the hasParameters() check in KEY 1 never takes effect

Proof

A mock test confirms KEY 2 overwrites KEY 1. When hasParameters() is true and parametersAsArray() returns []:

  • KEY 1 produces 'properties' => new \stdClass (serializes as JSON {})
  • KEY 2 produces 'properties' => [] (serializes as JSON [])

With the buggy code, the result is [] — proving KEY 2 overwrote KEY 1.

Fix

Remove the unconditional duplicate parameters key (KEY 2). The conditional spread (KEY 1) already handles both cases correctly:

  • hasParameters() === true → spreads parameters with proper schema and stdClass fallback
  • hasParameters() === false → spreads nothing → no parameters key in output

Tests

Added 4 new tests (5 total, 18 assertions):

Test What it proves
generates parameters only once from conditional spread Mixed tools (with/without params) produce correct output — parameterized tools get schema, parameterless tools get no parameters key
proves parameters is generated by conditional spread not static duplicate Uses a mock to verify properties is stdClass (from KEY 1), not [] (from KEY 2) — proving KEY 1 is now in control
sends correct tool payload to OpenRouter when streaming End-to-end: verifies the actual HTTP request payload sent to OpenRouter via Http::assertSent — parameterless tools have no parameters in the request body
maps tools with strict mode Existing — strict provider option still works correctly

Before / After

Before (tool without parameters):

{
  "type": "function",
  "function": {
    "name": "get_time",
    "description": "Returns the current time",
    "parameters": {
      "type": "object",
      "properties": {},
      "required": []
    }
  }
}

After (tool without parameters):

{
  "type": "function",
  "function": {
    "name": "get_time",
    "description": "Returns the current time"
  }
}

Test Check Result

Screenshot 2026-04-08 at 23 34 24

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant