Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate llm_config passed to ConversableAgent (issue #1522) #1654

Merged
merged 8 commits into from
Feb 15, 2024

Conversation

gunnarku
Copy link
Contributor

@gunnarku gunnarku commented Feb 12, 2024

Based on #1522 (and a follow-up discussion in this PR), this commit implements the additional validation checks in ConversableAgent.

Add the following validation and raise ValueError if:

  • The llm_config is None (validated in ConversableAgent).
  • The llm_config has no model specified and config_list is empty (validated in OpenAIWrapper).
  • The config_list has at least one entry, but not all the entries have the model is specified (validated in OpenAIWrapper).

The rest of the changes are code churn to adjust or add the test cases.

Why are these changes needed?

The root cause is #1522. The intent here is to enforce the contract for input parameters that are passed to ConversableAgent. With these changes the following happens:

from autogen import UserProxyAgent, AssistantAgent

# Throws an error because "config_list" is empty and "llm_config" does not
# have a value for "model" specified.
assistant = AssistantAgent("assistant", llm_config={"config_list": []})

# Throws an error because "config_list" has no "model" specified.
assistant = AssistantAgent("assistant", llm_config={"config_list":
                                                    [{"model":"", "api_key":
                                                      "sk-mysecretkey"}]})

# Works.
assistant = AssistantAgent("assistant", llm_config={"config_list": 
                                                   [{"model":"gpt-4", "api_key": "sk-mysecretkey"}]})

# Throws an error because one of the "config_list" entries has an empty model.
assistant = AssistantAgent("assistant", llm_config={"config_list":
                                                    [{"model":"gpt-4",
                                                      "api_key": "sk-mysecretkey"},
                                                     {"model":"", "api_key":
                                                      "sk-mysecretkey"}]})

# Throws an error because no "llm_config" is specified.
assistant = AssistantAgent("assistant")

# Works.
assistant = AssistantAgent("assistant", llm_config=False)

# Works: "llm_config" specifies the "model" attribute.
assistant = AssistantAgent("assistant", llm_config={"model": "gpt-4"})
proxy = UserProxyAgent("user")

proxy.initiate_chat(assistant)

Related issue number

Hopefully fixes #1522.

Checks

@codecov-commenter
Copy link

codecov-commenter commented Feb 13, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (f68c09b) 37.85% compared to head (49d897a) 49.68%.

Files Patch % Lines
autogen/oai/client.py 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1654       +/-   ##
===========================================
+ Coverage   37.85%   49.68%   +11.83%     
===========================================
  Files          50       50               
  Lines        5733     5742        +9     
  Branches     1297     1410      +113     
===========================================
+ Hits         2170     2853      +683     
+ Misses       3391     2659      -732     
- Partials      172      230       +58     
Flag Coverage Δ
unittests 49.59% <80.00%> (+11.74%) ⬆️

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.

@sonichi sonichi added this pull request to the merge queue Feb 15, 2024
Merged via the queue into microsoft:main with commit d677b47 Feb 15, 2024
49 of 57 checks passed
@sonichi sonichi mentioned this pull request Feb 15, 2024
3 tasks
afourney added a commit that referenced this pull request Feb 15, 2024
* Logging (#1146)

* WIP:logging

* serialize request, response and client

* Fixed code formatting.

* Updated to use a global package, and added some test cases. Still very-much a draft.

* Update work in progress.

* adding cost

* log new agent

* update log_completion test in test_agent_telemetry

* tests

* fix formatting

* Added additional telemetry for wrappers and clients.

* WIP: add test for oai client and oai wrapper table

* update test_telemetry

* fix format

* More tests, update doc and clean up

* small fix for session id - moved to start_logging and return from start_logging

* update start_logging type to return str, add notebook to demonstrate use of telemetry

* add ability to get log dataframe

* precommit formatting fixes

* formatting fix

* Remove pandas dependency from telemetry and only use in notebook

* formatting fixes

* log query exceptions

* fix formatting

* fix ci

* fix comment - add notebook link in doc and fix groupchat serialization

* small fix

* do not serialize Agent

* formatting

* wip

* fix test

* serialization bug fix for soc moderator

* fix test and clean up

* wip: add version table

* fix test

* fix test

* fix test

* make the logging interface more general and fix client model logging

* fix format

* fix formatting and tests

* fix

* fix comment

* Renaming telemetry to logging

* update notebook

* update doc

* formatting

* formatting and clean up

* fix doc

* fix link and title

* fix notebook format and fix comment

* format

* try fixing agent test and update migration guide

* fix link

* debug print

* debug

* format

* add back tests

* fix tests

---------

Co-authored-by: Adam Fourney <adamfo@microsoft.com>
Co-authored-by: Victor Dibia <victordibia@microsoft.com>
Co-authored-by: Chi Wang <wang.chi@microsoft.com>

* Validate llm_config passed to ConversableAgent (issue #1522) (#1654)

* Validate llm_config passed to ConversableAgent

Based on #1522, this commit implements the additional validation checks in
`ConversableAgent.`

Add the following validation and `raise ValueError` if:

 - The `llm_config` is `None`.
 - The `llm_config` is valid, but `config_list` is missing or lacks elements.
 - The `config_list` is valid, but no `model` is specified.

The rest of the changes are code churn to adjust or add the test cases.

* Validate llm_config passed to ConversableAgent

Based on #1522, this commit implements the additional validation checks in
`ConversableAgent.`

Add the following validation and `raise ValueError` if:

 - The `llm_config` is `None` (validated in `ConversableAgent`).
 - The `llm_config` has no `model` specified and `config_list` is empty
   (validated in `OpenAIWrapper`).
 - The `config_list` has at least one entry, but not all the entries have
   the `model` is specified (validated in `OpenAIWrapper`).

The rest of the changes are code churn to adjust or add the test cases.

* Validate llm_config passed to ConversableAgent

Based on #1522, this commit implements the additional validation checks in
`ConversableAgent.`

Add the following validation and `raise ValueError` if:

 - The `llm_config` is `None` (validated in `ConversableAgent`).
 - The `llm_config` has no `model` specified and `config_list` is empty
   (validated in `OpenAIWrapper`).
 - The `config_list` has at least one entry, but not all the entries have
   the `model` is specified (validated in `OpenAIWrapper`).

The rest of the changes are code churn to adjust or add the test cases.

* Validate llm_config passed to ConversableAgent

Based on #1522, this commit implements the additional validation checks in
`ConversableAgent.`

Add the following validation and `raise ValueError` if:

 - The `llm_config` is `None` (validated in `ConversableAgent`).
 - The `llm_config` has no `model` specified and `config_list` is empty
   (validated in `OpenAIWrapper`).
 - The `config_list` has at least one entry, but not all the entries have
   the `model` is specified (validated in `OpenAIWrapper`).

The rest of the changes are code churn to adjust or add the test cases.

* Validate llm_config passed to ConversableAgent

Based on #1522, this commit implements the additional validation checks in
`ConversableAgent.`

Add the following validation and `raise ValueError` if:

 - The `llm_config` is `None` (validated in `ConversableAgent`).
 - The `llm_config` has no `model` specified and `config_list` is empty
   (validated in `OpenAIWrapper`).
 - The `config_list` has at least one entry, but not all the entries have
   the `model` is specified (validated in `OpenAIWrapper`).

The rest of the changes are code churn to adjust or add the test cases.

* Fix the test_web_surfer issue

For anyone reading this: you need to `pip install markdownify` for the
`import WebSurferAgent` to succeed. That is needed to run the
`test_web_surfer.py` locally.

Test logic needs `llm_config` that is not `None` and that is not
`False`.

Let us pray that this works as part of GitHub actions ...

* One more fix for llm_config validation contract

* update test_utils

* remove stale files

---------

Co-authored-by: Adam Fourney <adamfo@microsoft.com>
Co-authored-by: Victor Dibia <victordibia@microsoft.com>
Co-authored-by: Chi Wang <wang.chi@microsoft.com>
Co-authored-by: Gunnar Kudrjavets <gunnarku@users.noreply.github.com>
whiskyboy pushed a commit to whiskyboy/autogen that referenced this pull request Apr 17, 2024
…microsoft#1654)

* Validate llm_config passed to ConversableAgent

Based on microsoft#1522, this commit implements the additional validation checks in
`ConversableAgent.`

Add the following validation and `raise ValueError` if:

 - The `llm_config` is `None`.
 - The `llm_config` is valid, but `config_list` is missing or lacks elements.
 - The `config_list` is valid, but no `model` is specified.

The rest of the changes are code churn to adjust or add the test cases.

* Validate llm_config passed to ConversableAgent

Based on microsoft#1522, this commit implements the additional validation checks in
`ConversableAgent.`

Add the following validation and `raise ValueError` if:

 - The `llm_config` is `None` (validated in `ConversableAgent`).
 - The `llm_config` has no `model` specified and `config_list` is empty
   (validated in `OpenAIWrapper`).
 - The `config_list` has at least one entry, but not all the entries have
   the `model` is specified (validated in `OpenAIWrapper`).

The rest of the changes are code churn to adjust or add the test cases.

* Validate llm_config passed to ConversableAgent

Based on microsoft#1522, this commit implements the additional validation checks in
`ConversableAgent.`

Add the following validation and `raise ValueError` if:

 - The `llm_config` is `None` (validated in `ConversableAgent`).
 - The `llm_config` has no `model` specified and `config_list` is empty
   (validated in `OpenAIWrapper`).
 - The `config_list` has at least one entry, but not all the entries have
   the `model` is specified (validated in `OpenAIWrapper`).

The rest of the changes are code churn to adjust or add the test cases.

* Validate llm_config passed to ConversableAgent

Based on microsoft#1522, this commit implements the additional validation checks in
`ConversableAgent.`

Add the following validation and `raise ValueError` if:

 - The `llm_config` is `None` (validated in `ConversableAgent`).
 - The `llm_config` has no `model` specified and `config_list` is empty
   (validated in `OpenAIWrapper`).
 - The `config_list` has at least one entry, but not all the entries have
   the `model` is specified (validated in `OpenAIWrapper`).

The rest of the changes are code churn to adjust or add the test cases.

* Validate llm_config passed to ConversableAgent

Based on microsoft#1522, this commit implements the additional validation checks in
`ConversableAgent.`

Add the following validation and `raise ValueError` if:

 - The `llm_config` is `None` (validated in `ConversableAgent`).
 - The `llm_config` has no `model` specified and `config_list` is empty
   (validated in `OpenAIWrapper`).
 - The `config_list` has at least one entry, but not all the entries have
   the `model` is specified (validated in `OpenAIWrapper`).

The rest of the changes are code churn to adjust or add the test cases.

* Fix the test_web_surfer issue

For anyone reading this: you need to `pip install markdownify` for the
`import WebSurferAgent` to succeed. That is needed to run the
`test_web_surfer.py` locally.

Test logic needs `llm_config` that is not `None` and that is not
`False`.

Let us pray that this works as part of GitHub actions ...

* One more fix for llm_config validation contract
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Confusing error message when llm_config is not initialized or is None.
4 participants