[Chat] Fix MessageNormalizer round-trip for AssistantMessage with tool calls#1996
Open
Amoifr wants to merge 1 commit intosymfony:mainfrom
Open
[Chat] Fix MessageNormalizer round-trip for AssistantMessage with tool calls#1996Amoifr wants to merge 1 commit intosymfony:mainfrom
Amoifr wants to merge 1 commit intosymfony:mainfrom
Conversation
…e with tool calls
When a conversation containing an AssistantMessage (or ToolCallMessage)
with tool calls was persisted via MessageNormalizer and later restored,
denormalize() crashed on `Undefined array key "function"`.
Root cause: denormalize() reads the OpenAI-style wrapped shape
`{id, function: {name, arguments}}`, but normalize() produced the
flat ObjectNormalizer fallback shape `{id, name, arguments}` because
ToolCallNormalizer was never registered as a service in ai-bundle.
The chat component's round-trip tests only passed because they wire
ToolCallNormalizer manually.
Two complementary changes:
- ai-bundle: register ToolCallNormalizer as a `serializer.normalizer`
service so normalize() produces the wrapped shape the denormalizer
expects.
- chat: make MessageNormalizer::denormalize() tolerate both shapes,
restoring conversations that were persisted with the flat form
before this fix landed.
Fix symfony#1972
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a conversation containing an
AssistantMessage(orToolCallMessage) with tool calls is persisted throughMessageNormalizerand later restored,denormalize()crashes onUndefined array key "function".Root cause
MessageNormalizer::denormalize()reads the OpenAI-style wrapped shape:{id, function: {name, arguments: JSON string}}.MessageNormalizer::normalize()delegates the tool call payload to the outerSerializer. WhenToolCallNormalizeris in the chain, the wrapped shape is produced; otherwise the defaultObjectNormalizerfalls back to the flat shape{id, name, arguments: array}.ToolCallNormalizerwas never registered as a service inai-bundle. In real Symfony apps, the outer serializer therefore produced the flat shape, and denormalize crashed. The chat component's existing round-trip tests only passed because they wireToolCallNormalizermanually.Fix (two complementary changes)
Symfony\AI\Platform\Contract\Normalizer\Result\ToolCallNormalizerwith theserializer.normalizertag, so the outer serializer produces the wrapped shape the denormalizer expects.MessageNormalizer::denormalize()tolerate both shapes (wrapped and flat). This lets apps restore conversations that were persisted before this fix landed (the reporter's own DB rows are in the flat form).Test plan
src/chat— 41 tests pass, including 2 new regression tests exercising the flat-shape path withoutToolCallNormalizerin the chain (testItCanDenormalizeAssistantMessageWithFlatToolCalls,testItCanDenormalizeToolCallMessageWithFlatToolCall).src/ai-bundle— 239 tests pass (+1 from this PR verifying the new service registration and tag), identical error count to main (8 pre-existing errors tied to the optionalsymfony/ai-ovh-platformpackage, unrelated to this change).phpstanclean onsrc/chat.Notes
toolCallFromPayload()helper centralizes the tolerance so there's a single place to revisit if we ever want to drop the flat-shape fallback later.