DistributedCachingChatClient's keys aren't stable across OSes

[SteveSandersonMS]

Description

I know that DistributedCachingChatClient.GetCacheKey is documented with:

    /// The generated cache key is not guaranteed to be stable across releases of the library.

... but isn't it expected to be stable across OSes? I think it should be because this is a valid scenario:

• Multiple web servers are hosting an app that uses MEAI. Some servers might be Windows; others Linux
• They all connect to a shared Redis instance that acts as the storage for DistributedCachingChatClient

In this case, we wouldn't expect all the Windows servers to have one set of cache entries and all the Linux servers to have a different set.

However, the cache keys do vary across the two OSes. I think the underlying reason is that AIJsonUtilities.DefaultOptions enables indentation and linebreaks during JSON serialization, and so you get different linebreaks going into the hash.

Reproduction Steps

var dcc = new MyDistributedCacheClient();
Console.WriteLine(dcc.GetCacheKey([new ChatMessage(ChatRole.User, "")]));

class MyDistributedCacheClient() : DistributedCachingChatClient(new SomeIChatClient(), new SomeDistributedCache())
{
    public string GetCacheKey(IEnumerable<ChatMessage> messages, ChatOptions? options = null)
        => base.GetCacheKey(messages, options);
}

Expected behavior

On Windows and Linux, should write the same string to the console.

Actual behavior

On Windows, outputs E51327685BFC9E6C0D5B61C513E2F5FB9EDD2C9177EC75379B059EE71466383451E959B1FE9C0865596E4EA44CE8D6C6
On Linux, outputs 68F57BBAD7E8DC570361582A11E973AA5B05665766DF422D727C5937738ECB0559AB9338AA68FCFD49467169CEFF9B2A

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

[stephentoub]

@eiriktsarpalis, what's the right way to address this with AIJsonUtilities?

[eiriktsarpalis]

Hmm, I'm not intently familiar with what is being serialized to produce cache keys but in principle I think it's hard to guarantee stability when it comes to serialization. There are two layers one needs to contend with:

1. Assuming your inputs are pure JSON values, one needs to convert those to a normal form: remove indentation, sort properties according to a deterministic order while accounting for potential duplicate properties.
2. Things become more complicated once you add a serialization layer into the mix. There are many factors that could change the serialization output including OS, runtime (mono reflection reports members using subtly different ordering), ignore conditions, property binding case sensitivity, unbound property handling, changing the order of declared properties, etc.

If we control the type that's being used to generate the key, my recommendation would be to write a method that deterministically maps key values to a string.

[stephentoub]

I'm not intently familiar with what is being serialized to produce cache keys

extensions/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/DistributedCachingChatClient.cs

Line 128 in 4d33003

return AIJsonUtilities.HashDataToString([CacheVersion, messages, options, .. additionalValues], _jsonSerializerOptions);

[eiriktsarpalis]

Hmm, the fact that it takes object doesn't help much. We could perhaps address the issue at hand by serializing each object to an intermediate JsonNode, recursively sorting properties to a normal form and then hash the result without using indentation. It still wouldn't address the concerns in point 2) though.

[SteveSandersonMS]

The way I'm working around it is by subclassing DistributedCachingChatClient and overriding GetCacheKey:

    private class StableKeysDistributedCachingChatClient(IChatClient innerClient, IDistributedCache storage)
        : DistributedCachingChatClient(innerClient, storage)
    {
        private static readonly JsonSerializerOptions _jsonSerializerOptions = new()
        {
            TypeInfoResolverChain = { new DefaultJsonTypeInfoResolver() },
            Converters = { new JsonStringEnumConverter() },
            WriteIndented = false,
        };

        protected override string GetCacheKey(IEnumerable<ChatMessage> messages, ChatOptions? options, params scoped ReadOnlySpan<object?> additionalValues)
        {
            const int CacheVersion = 1;
            return AIJsonUtilities.HashDataToString([CacheVersion, messages, options, .. additionalValues], _jsonSerializerOptions);
        }
    }

That is, simply disabling indentation in the call to HashDataToString. Note that indentation is still enabled in the values (as opposed to keys) written to the IDistributedCache because that's helpful for debugging. That might be sufficient to handle 90% of cases.

I'm not sure how likely property reordering is.

[stephentoub]

@eiriktsarpalis, your PR said it "partially" addressed this. What is left to do?

[eiriktsarpalis]

Nothing actionable, per #6469 (comment) there are many moving parts that make this particular hashing strategy inherently unstable. I think we can close this.