Add ChatCompletionCache along with AbstractStore for caching completions

[srjoglekar246]

Why are these changes needed?

Add a caching wrapper to ChatCompletionClient, that can use any store with get/set methods for storage.

Related issue number

Closes #4752

Checks

• I've included any doc changes needed for https://microsoft.github.io/autogen/. See https://microsoft.github.io/autogen/docs/Contribute#documentation to build and test documentation locally.
• Added unit tests

Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.

Files not reviewed (1)

• python/packages/autogen-core/src/autogen_core/store/abstract_store_base.py: Evaluated as low risk

[srjoglekar246]

@ekzhu do we need any documentation changes? I suppose API Reference gets updated automatically?

[codecov]

Codecov Report

Attention: Patch coverage is 97.19626% with 3 lines in your changes missing coverage. Please review.

Project coverage is 69.69%. Comparing base (2e1a9c7) to head (89c73fe).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...ages/autogen-core/src/autogen_core/_cache_store.py	85.71%	2 Missing ⚠️
...autogen_ext/models/cache/_chat_completion_cache.py	98.36%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4924      +/-   ##
==========================================
- Coverage   72.92%   69.69%   -3.24%     
==========================================
  Files         115      168      +53     
  Lines        6785    10605    +3820     
==========================================
+ Hits         4948     7391    +2443     
- Misses       1837     3214    +1377     

Flag	Coverage Δ
unittests	69.69% <97.19%> (-3.24%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ekzhu]

Aside from API documentation. We can discuss the cache in the following places:

https://github.com/microsoft/autogen/blob/main/python/packages/autogen-core/docs/src/user-guide/agentchat-user-guide/tutorial/models.ipynb

https://github.com/microsoft/autogen/blob/main/python/packages/autogen-core/docs/src/user-guide/core-user-guide/framework/model-clients.ipynb

[srjoglekar246]

@ekzhu added docs as you mentioned :-)

[jackgerrits]

I think we should spend some more time to see if the interface can be improved. Especially if this is going into core

[srjoglekar246]

@jackgerrits @ekzhu I am not sure if a simple protocol for duc-typing will work in all cases, unless we (or the user) implements a a wrapper around (say) redis. For eg with the latest change I get this error:

/tmp/tmpxi9c9pui.py
  /tmp/tmpxi9c9pui.py:23:52 - error: Argument of type "Redis" cannot be assigned to parameter "store" of type "CacheStore" in function "__init__"
    "Redis" is incompatible with protocol "CacheStore"
      "get" is an incompatible type
        Type "(name: KeyT) -> ResponseT" is not assignable to type "(key: str, default: Any | None = None) -> (Any | None)"
          Function accepts too many positional parameters; expected 1 but received 2
            Parameter name mismatch: "key" versus "name"
      "set" is an incompatible type
        Type "(name: KeyT, value: EncodableT, ex: ExpiryT | None = None, px: ExpiryT | None = None, nx: bool = False, xx: bool = False, keepttl: bool = False, get: bool = False, exat: AbsExpiryT | None = None, pxat: AbsExpiryT | None = None) -> ResponseT" is not assignable to type "(key: str, value: Any) -> (Any | None)"
          Parameter name mismatch: "key" versus "name" (reportArgumentType)
1 error, 0 warnings, 0 informations 
WARNING: there is a new pyright version available (v1.1.389 -> v1.1.391).
Please install the new version or set PYRIGHT_PYTHON_FORCE_VERSION to `latest

[ekzhu]

@jackgerrits could this be resolved by making the input types more flexible?

Also do we need the default argument in the parameter list?

For now we may not be able to get 100% type match as we are approximate duck typing. It is not going to pass the type check for all classes. We can ignore this typing issue for now -- add "type: ignore" to the example line and let's get this PR in. Fix this in a follow up work to add redis wrapper.

[srjoglekar246]

In that case the current impl should just work. Will let @jackgerrits chime in. Otherwise I can also add diskcache & redis builtin implementations.

[jackgerrits]

Sorry I haven't had time to look at this. Would like to get the patch release out first then we can merge as eric said. I'd love if we can get the types to behave nicely but I understand its a bit tricky when interfacing with external libs.

I do think that using a thin redis wrapper is the best thing to do here as it gives us some flexibility about dealing with issues

[srjoglekar246]

In that case @ekzhu should we merge this, and I can send a followup for the redis wrapper?

[ekzhu]

As jack said let's wait until the patch fix for user input is released on Monday. Then we'll merge this.

[ekzhu]

We can merge this for 0.4.2 once the doc fixes are completed.

Need more work.

[srjoglekar246]

@ekzhu @jackgerrits PTAL. I added specific implementations of CacheStore & RedisStore, and improved the documentation as you suggested.

[ekzhu]

LGTM

[jackgerrits]

The casts should be removable right?

Jack has approved per our discussion