bug #1143 [Platform] Keep metadata set by result converters instead of overwriting in DeferredResult (chr-hertel)

This PR was merged into the main branch.

Discussion
----------

[Platform] Keep metadata set by result converters instead of overwriting in DeferredResult

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| Docs?         | no
| Issues        |
| License       | MIT

Currently, metadata that gets set by result converters is overwritten in `DeferredResult` which is unfortunate since it completely eliminates the option for that layer to work with that `Metadata` object.

should work after this PR tho.

Commits
-------

f0b26a5 Keep metadata set by result converters instead of overwriting in DeferredResult

Author: chr-hertel

src/platform/src/Metadata/Metadata.php

@@ -48,6 +48,14 @@ public function set(array $metadata): void
         $this->metadata = $metadata;
     }
 
+    /**
+     * @param array<string, mixed> $metadata
+     */
+    public function merge(array $metadata): void
+    {
+        $this->metadata = array_merge($this->metadata, $metadata);
+    }
+
     public function add(string $key, mixed $value): void
     {
         $this->metadata[$key] = $value;

src/platform/src/Result/DeferredResult.php

@@ -50,7 +50,7 @@ public function getResult(): ResultInterface
                 $this->convertedResult->setRawResult($this->rawResult);
             }
 
-            $this->convertedResult->getMetadata()->set($this->getMetadata()->all());
+            $this->convertedResult->getMetadata()->merge($this->getMetadata()->all());
 
             if (null !== $tokenUsageExtractor = $this->resultConverter->getTokenUsageExtractor()) {
                 if (null !== $tokenUsage = $tokenUsageExtractor->extract($this->rawResult, $this->options)) {

src/platform/tests/Metadata/MetadataTest.php

@@ -76,6 +76,19 @@ public function testItCanSetEntireMetadataArray()
         $this->assertSame(['key2' => 'value2', 'key3' => 'value3'], $metadata->all());
     }
 
+    public function testItCanMergeMetadataArrays()
+    {
+        $metadata = new Metadata(['key0' => 'value0', 'key1' => 'value1']);
+        $metadata->merge(['key2' => 'value2', 'key1' => 'newValue1']);
+
+        $this->assertTrue($metadata->has('key0'));
+        $this->assertTrue($metadata->has('key1'));
+        $this->assertTrue($metadata->has('key2'));
+        $this->assertSame('value0', $metadata->get('key0'));
+        $this->assertSame('newValue1', $metadata->get('key1'));
+        $this->assertSame('value2', $metadata->get('key2'));
+    }
+
     public function testItImplementsJsonSerializable()
     {
         $metadata = new Metadata(['key' => 'value']);

src/platform/tests/Result/DeferredResultTest.php

@@ -14,11 +14,13 @@
 use PHPUnit\Framework\TestCase;
 use Symfony\AI\Platform\Result\BaseResult;
 use Symfony\AI\Platform\Result\DeferredResult;
+use Symfony\AI\Platform\Result\InMemoryRawResult;
 use Symfony\AI\Platform\Result\RawHttpResult;
 use Symfony\AI\Platform\Result\RawResultInterface;
 use Symfony\AI\Platform\Result\ResultInterface;
 use Symfony\AI\Platform\Result\TextResult;
 use Symfony\AI\Platform\ResultConverterInterface;
+use Symfony\AI\Platform\Test\PlainConverter;
 use Symfony\Contracts\HttpClient\ResponseInterface as SymfonyHttpResponse;
 
 final class DeferredResultTest extends TestCase
@@ -119,6 +121,21 @@ public function testItPassesOptionsToConverter()
         $deferredResult->getResult();
     }
 
+    public function testItKeepsResultMetadata()
+    {
+        $result = new TextResult('Hello World');
+        $result->getMetadata()->add('foo', 'bar');
+        $converter = new PlainConverter($result);
+
+        $deferredResult = new DeferredResult($converter, new InMemoryRawResult());
+        $deferredResult->getMetadata()->add('key', 'value');
+
+        $unwrappedResult = $deferredResult->getResult();
+
+        $this->assertSame('bar', $unwrappedResult->getMetadata()->get('foo'));
+        $this->assertSame('value', $unwrappedResult->getMetadata()->get('key'));
+    }
+
     /**
      * Workaround for low deps because mocking the ResponseInterface leads to an exception with
      * mock creation "Type Traversable|object|array|string|null contains both object and a class type"