[Cache] Fix support for MessageBag and string#1754
Conversation
Guikingone
commented
Mar 12, 2026
Guikingone
commented
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| Docs? | no |
| Issues | Fix #1734 - Part of #1742 |
| License | MIT |
Guikingone
changed the title
MessageBagMessageBag and string
|
@villermen If you want to continue the discussion 🙂 |
Guikingone
changed the title
MessageBag and stringMessageBag and string
Guikingone
force-pushed
the
fix/cached_platform
branch
2 times, most recently
from
3e3333d to
e41e1be
Compare
| \is_array($input) => json_encode($input), | ||
| $input instanceof MessageBag => $input->getId()->toString(), | ||
| \is_array($input) => md5(json_encode($input)), | ||
| $input instanceof MessageBag => $input->latestAs(Role::User)->getId()->toRfc4122(), |
There was a problem hiding this comment.
Continued from https://github.com/symfony/ai/pull/1742/changes#r2925408069
One solution might be to use the latest user input as "cache key" and check if in the new bag, the input still the same, not perfect but as bags are just "bags" ... 🤔
You're referring to using the input, but what I'm seeing is usage of the ID only. IMO, there's an inherent problem in using IDs. The content of the message can change without affecting the ID.
What CachePlatform provides is an identical result for identical inputs without invoking the wrapped platform. Since this is not using the input at all, I think it kinda misses the point.
Why is normalizing the bag with MessageBagNormalizer not an option in your eyes? ($this->serializer->normalize($input)) Am I missing something that makes this impossible?
There was a problem hiding this comment.
Why is normalizing the bag with MessageBagNormalizer not an option in your eyes?
($this->serializer->normalize($input))Am I missing something that makes this impossible?
The MessageBag cannot be normalized without loosing its own identifier, could be updated, not an issue to be honest, It's not done for now as I'm wondering about the serialization of messages.
carsonbot
changed the title
MessageBag and stringMessageBag and string
Guikingone
force-pushed
the
fix/cached_platform
branch
from
e41e1be to
ccb14e4
Compare
Guikingone
force-pushed
the
fix/cached_platform
branch
from
ccb14e4 to
d622138
Compare
|
@villermen @push-mann I pushed a new version with an |
…ound-trip The cache never hit when the input was a MessageBag because the key was derived from a random Uuid::v7() instead of the message content (symfony#2192). The key is now built through the platform Contract serializer (without a model bound to the context), so it reflects the whole message bag deterministically and stays in sync with the request payload. To make the serializer lossless and complete, base normalizers are added for DocumentUrl, File (covering Video and Document), Collection, Template, Thinking, ExecutableCode and CodeExecution; AssistantMessageNormalizer no longer drops non-text parts (emitted under content_parts); SystemMessageNormalizer stringifies Template content. Contract::normalize() exposes a model-free entry point. The cache layer is also hardened so it never breaks the request: - ToolCallResult is normalized inline, so it is cacheable with the default serializer (which no longer requires a dedicated tool-call normalizer); - TextResult and ToolCall signatures are preserved across the round-trip, matching ThinkingResult (Gemini/Vertex AI thoughtSignature survives a hit); - streaming responses bypass the cache instead of throwing; - the store and restore paths fail open: an uncacheable result is returned live and a stale/corrupted entry is dropped and re-fetched; - the constructor cacheKey now acts as the default namespace, the key uses a "." delimiter to avoid collisions, and CachePlatform::invalidateTags() drops entries by model or namespace tag. Fix symfony#2192
Guikingone
force-pushed
the
fix/cached_platform
branch
from
78b301e to
d2959be
Compare
| public function invoke(string|Model $model, array|string|object $input, array $options = []): DeferredResult | ||
| { | ||
| if (null === $this->cache || !\array_key_exists('prompt_cache_key', $options) || '' === $options['prompt_cache_key']) { | ||
| $namespace = $options['prompt_cache_key'] ?? $this->cacheKey; |
There was a problem hiding this comment.
is it possible to expose this array-key as public const so it's usable from outside when setting the option?
|
Caching works well now for me 🙂 @Guikingone I also came across #2160 while investigating. Not sure whether it's relevant here but i think it addresses at least caching for content-inputs. |
villermen
left a comment
villermen
left a comment
There was a problem hiding this comment.
Had to read in a bit deeper than before when I just reported the issue I observed without really using the library itself 😅 My earlier suggestion was based on the assumption this normalizing logic was already in place, so making it available is a solid improvement in my eyes. Thanks for taking the time, and good work!
I've found nothing serious in my review, but ideally would like to see a test on the new main normalizer before merge.
There was a problem hiding this comment.
Unrelated observation: I don't really get this pattern Contract uses. It is supposed to be extended, but returns self. That means implementing classes will never be instantiated.
@chr-hertel Wouldn't it make more sense to have this class be abstract and provide a method with the default normalizers to (not) use? Something along the lines of...
abstract class Contract
{
/**
* @return NormalizerInterface[]
*/
public static function getDefaultNormalizers(): array
{
// ...
}
}| } else { | ||
| $additional[] = $this->normalizer->normalize($part, $format, $context); |
There was a problem hiding this comment.
Suggestion: This feels like a straight up improvement over typechecking against all potential part types. I don't know how this project does TODO management, but I would consider deprecating the dedicated "tool_calls" and "content" keys in favor of just a "parts" key with everything in it in this case. Would also save you from having to call it "additional" =)
| } | ||
|
|
||
| if ([] !== $additional) { | ||
| $array['content_parts'] = $additional; |
There was a problem hiding this comment.
This PR does not add a test for this new key. I suggest adding it in the Contract variety of AssistantMessageNormalizerTest.
4 participants
