Rework cache key handling in caching client / generator

[stephentoub]

• Expose the default cache key helper so that customization doesn't require re-implementing the whole thing.
• Make it easy to incorporate additional state into the cache key.
• Avoid serializing all of the values for the key into a new byte[], at least on .NET 8+. There, we can serialize directly into a stream that targets an IncrementalHash.
• Include Chat/EmbeddingGenerationOptions in the cache key by default.

Microsoft Reviewers: Open in CodeFlow

[SteveSandersonMS]

Looks good

[shyamnamboodiripad]

@stephentoub Just a thought - should this function also be overridable? If a derived class were to take over the cache key computation by overriding GetCacheKey(bool, IList...) above, would it be surprising that calling base.GetCacheKey(ReadOnlySpan<object?>) in further derived classes does something different / uses a completely different hashing algorithm than calling `base.GetCacheKey(bool, IList...)?

[stephentoub]

would it be surprising that calling base.GetCacheKey(ReadOnlySpan<object?>) in further derived classes does something different / uses a completely different hashing algorithm than calling `base.GetCacheKey(bool, IList...)?

Not sure I follow. base.GetCacheKey(bool, IList, ...) uses base.GetCacheKey(ROS). The intent is the latter represents the default algorithm for handling multiple objects. An override of GetCacheKey(bool, IList, ...) can delegate to either of the methods on the base.

Could you elaborate on the concern? I'm not clear on what making it virtual would solve. Maybe it needs a different name?

[shyamnamboodiripad]

Ah here is an example of what I meant. Consider the case where we have EvaluationCachingChatClient that is derived from DistributedCachingChatClient and that ships as part of a hypothetical Evaluation library 😄 EvaluationCachingChatClient takes over the cache key computation by overriding GetCacheKey and uses a its own custom hashing algorithm for this.

Now consider the case where a consumer of this Evaluation library wants to inherit EvaluationCachingChatClient and tweak the hash computation to include a few additional custom parameters without changing the hashing algorithm used in EvaluationCachingChatClient. If this derived implementation were to call the base.GetCacheKey(ReadOnlySpan<object?>) for this, the key that is returned could look very different from the key returned from base.GetCacheKey(bool, IList...).

Depending on how different the caching algorithm used in the two base types happens to be, the returned key could have different lengths, different set of allowed characters (which could be important if the key is used as the name of a file or directory on disk for example) etc. This could be especially problematic in cases like below where the derived class wants to keep the base implementation unchanged in the default case and override only it under certain conditions.

<Pseudo Code>
class MyCustomEvaluationCachingChatClient : EvaluationCachingChatClient
{
    override GetCacheKey(bool, IList...)
    {
        if (condition)
        {
             // Compute a custom key - I want to use same hashing algorithm as the immediate base type - but the below call uses the hashing algorithm from another base class
             return base.GetCacheKey(<ReadOnlySpan<object?> with custom parameters>);
        }
        
        // Preserve the same key computation as the immediate base type.
        return base.GetCacheKey(bool, IList...);
    }
}

Each level in the inheritance hierarchy has a choice between the following options -

• only change what parameters are included in the hash key computation without changing the hashing algorithm being used OR
• completely take over the hash key computation and control both the set of parameters that are included in the hash as well as the hashing algorithm that is used

And regardless of which choice is made in a particular inheritance level, it would be ideal if subsequent derived implementations can also retain the same flexibility to

• only tweak just what parameters are included in the hash while keeping the same hashing algorithm as their immediate base class OR
• completely take over the hash key computation

Having the ability to individually override GetCacheKey(bool, IList) and GetCacheKey(ReadOnlySpan<object?>) could be one way to accomplish the above. Yes, I agree we should probably give different names to the two functions for clarity in this case.

Another option may be to eliminate GetCacheKey(ReadOnlySpan<object?>) from DistributedCachingChatClient and make it available as a standalone static helper. However, in this case, every level in the inheritance hierarchy that wants to use some custom hashing algorithm would need to make similar static helpers available for their own derived types...

I realize my response is a bit rambling and perhaps I am over thinking it 😄, but I hope this clarifies the concern. Happy to sync up offline to discuss if needed.