Fix #2643 - groupchat model registration

[Matteo-Frattaroli]

Why are these changes needed?

When using a custom model client with GroupChatManager, the underlying GroupChat internally defines two ConversableAgents that receive the llm_config passed to GroupChatManager but don't handle model registration (and there's no way to do so externally if not subclassing GroupChat and overriding the two methods that do so). This contribution allows a user to pass a ModelClient or a list of ModelClient through a 'model_client_cls' attribute used for automatic registration of custom models only.

Related issue number

Closes #2643

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.
• I've added tests (if relevant) corresponding to the changes introduced in this PR.
• I've made sure all auto checks have passed.

[Matteo-Frattaroli]

@microsoft-github-policy-service agree

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 18.91892% with 30 lines in your changes missing coverage. Please review.

Project coverage is 20.41%. Comparing base (6279247) to head (40131dc).
Report is 107 commits behind head on main.

Files	Patch %	Lines
autogen/agentchat/groupchat.py	18.91%	30 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2696       +/-   ##
===========================================
- Coverage   32.90%   20.41%   -12.49%     
===========================================
  Files          94       95        +1     
  Lines       10235    10701      +466     
  Branches     2193     2294      +101     
===========================================
- Hits         3368     2185     -1183     
- Misses       6580     8361     +1781     
+ Partials      287      155      -132     

Flag	Coverage Δ
unittests	20.40% <18.91%> (-12.50%)	⬇️

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]

Add a unit test with mocks to check if the select speaker agent has gotten the propagated custom client.

[Matteo-Frattaroli]

@ekzhu @sonichi @marklysze I completed all the changes you requested but after merging main several builds are failing because of a missing package (namely "packaging").

@ekzhu I see you submitted two commits before because of this bug on requests.

I'm pretty sure that as for the latter, this problem with the "packaging" library is not related to my code. Could you maybe take a look at your earliest convenience?

Do you think after solving this the PR could be approved?

[marklysze]

@Matteo-Frattaroli, thanks for the hard work on this! Your code has come at just the right time as I'm testing a new custom client class for Mistral and I wanted to test Group Chat :).

My first run worked with your code as is, which is great... I'll test more...

[Matteo-Frattaroli]

@marklysze thank you for the positive feedback!

As I'm sure you noticed though there is some problem with the CI which is why I rolled back the latest merge from master to try and isolate the problem locally. Once this is fixed I'll resolve the conflicts (which is really just about a method i extracted to create the agents) and maybe ask for a new review.

Meanwhile if you have any directions on the current issue I can try to help (even if I know that you're probably already looking into it and are certainly more capable of doing so than me)

[thaind-taureau]

hi, thank you so much for your effort. i'm currently have issue when register custom model class and waiting to this PR to be merged. i've do some test with this PR and got some issue, can you please check it out:

1. Got runtime error when using async chat since selector param has been removed from a_select_speaker fuction but a_run_chat function still passing the param
2. Unable to pass kwargs when register custom model because in _register_client_from_config function, you calling register function without passing any param: agent.register_model_client(select_speaker_auto_model_client_cls). i think you can add a additional param when init grouchat (eg select_speaker_auto_model_client_cls_kwarg) and then pass it to register model function as kwarg
   i'm supper new to this project so not sure if i'm do it right and thank you again for your work here

[marklysze]

hi, thank you so much for your effort. i'm currently have issue when register custom model class and waiting to this PR to be merged. i've do some test with this PR and got some issue, can you please check it out:

1. Got runtime error when using async chat since selector param has been removed from a_select_speaker fuction but a_run_chat function still passing the param
2. Unable to pass kwargs when register custom model because in _register_client_from_config function, you calling register function without passing any param: agent.register_model_client(select_speaker_auto_model_client_cls). i think you can add a additional param when init grouchat (eg select_speaker_auto_model_client_cls_kwarg) and then pass it to register model function as kwarg
   i'm supper new to this project so not sure if i'm do it right and thank you again for your work here

Hey @thaind-taureau, thanks for your comment...

For #1, I've updated the async select speaker, a_select_speaker, are you able to test that, now?

For #2, can you show some code that you'd like to see so we can see if there's a way around it?

[thaind-taureau]

Hi @marklysze

1. im going to test it now
2. this is the workaround i'm currently using. i just add model_client_cls_kwargs as dictionary and then override the last line of _register_client_from_config function:

class CustomGroupChat(GroupChat):
    def __init__(
            self,
            model_client_cls_kwargs: Optional[Dict] = None, # Add this parameter
            *args, **kwargs
    ):
        super().__init__(*args, **kwargs)
        self.model_client_cls_kwargs = model_client_cls_kwargs

# override _register_client_from_config
    def _register_client_from_config(self, agent: Agent, config: Dict):
           # change the last line of function, keep everything else
          ...
            agent.register_model_client(select_speaker_auto_model_client_cls, **self.model_client_cls_kwargs) # I change this line

[thaind-taureau]

@marklysze
Update for #1: tested the code and it's work. thank you

[marklysze]

Hi @marklysze

1. im going to test it now
2. this is the workaround i'm currently using. i just add model_client_cls_kwargs as dictionary and then override the last line of _register_client_from_config function:

class CustomGroupChat(GroupChat):
    def __init__(
            self,
            model_client_cls_kwargs: Optional[Dict] = None, # Add this parameter
            *args, **kwargs
    ):
        super().__init__(*args, **kwargs)
        self.model_client_cls_kwargs = model_client_cls_kwargs

# override _register_client_from_config
    def _register_client_from_config(self, agent: Agent, config: Dict):
           # change the last line of function, keep everything else
          ...
            agent.register_model_client(select_speaker_auto_model_client_cls, **self.model_client_cls_kwargs) # I change this line

Thanks for testing number 1 @thaind-taureau, I'm glad that it worked.

For number 2, are you able to provide an example of the kwargs you're passing in?

[marklysze]

Add a unit test with mocks to check if the select speaker agent has gotten the propagated custom client.

Hey @ekzhu - the test has been added, I've corrected the formatting

[rysweet]

This PR is against AutoGen 0.2. AutoGen 0.2 has been moved to the 0.2 branch. Please rebase your PR on the 0.2 branch or update it to work with the new AutoGen 0.4 that is now in main.

[thaind-taureau]

Hi @marklysze

1. im going to test it now
2. this is the workaround i'm currently using. i just add model_client_cls_kwargs as dictionary and then override the last line of _register_client_from_config function:

class CustomGroupChat(GroupChat):
    def __init__(
            self,
            model_client_cls_kwargs: Optional[Dict] = None, # Add this parameter
            *args, **kwargs
    ):
        super().__init__(*args, **kwargs)
        self.model_client_cls_kwargs = model_client_cls_kwargs

# override _register_client_from_config
    def _register_client_from_config(self, agent: Agent, config: Dict):
           # change the last line of function, keep everything else
          ...
            agent.register_model_client(select_speaker_auto_model_client_cls, **self.model_client_cls_kwargs) # I change this line

Thanks for testing number 1 @thaind-taureau, I'm glad that it worked.

For number 2, are you able to provide an example of the kwargs you're passing in?

Hi @marklysze. Sorry for the late, but i'm not sure what you want to see. Please tell me if this not answer your question. Currently when using this workaround, i'm just passing a dictionary that conatin some extra object because my custom model class require some extra work , something like this:

model_client_cls_kwargs = {
    "user_id": user.id,
    "model": llm_model,
    "db_session": db_session
}
group_chat = CustomGroupChat(
    model_client_cls_kwargs=model_client_cls_kwargs
    ...
)