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

startup menu for ai_settings.yaml to support multiple configurations #4646

Closed

Conversation

bassie661
Copy link

@bassie661 bassie661 commented Jun 10, 2023

User description

Summary

I created this PR to add support for storing multiple configurations, thus allowing the user to 'create', 'edit', 'view' or 'delete' configurations and have their settings stored in ai_settings.yaml. The ai_settings file now saves configurations as dictionary and if a configuration exists it is showed as a menu option when autogpt is started.

When no configurations are found, or no ai_settings.yaml file, the user get's the usual generate_aiconfig_automatic questions and can choose to either let autogpt create the config from a single input or the user can choose "--manual" to manually input the first configuration. At the end of this process the first configuration is saved in the ai_settings file.

Next time when autogpt starts and/or when configurations are found in ai_settings.yaml, the prompt starts-up with the following menu:

image

Change

  • Support storing multiple AI configurations
  • Support choose existing AI configuration(s)
  • Support create new AI configuration(s)
  • Support edit existing AI configuration(s)
  • Support view existing AI configuration(s)
  • Support delete existing AI configuration(s)
    -> e.g. ai_name, ai_role, ai_goals[], api_budget, plugins
  • Support for plugins per AI configuration (add, delete, change), plugins are expected to land with add plugins list to aiconfig #4089

This PR is big but it was needed to restructure the old and add the new functionality to the '--manual' startup flow while keeping the functionality clear and maintainable.

Test Plan

  • integration\test_setup.py
  • unit\test_ai_config.py
  • unit\test_config.py
  • unit\test_prompt.py

PR Quality Checklist

[x] My pull request is atomic and focuses on a single change.
-> mostly true but reworking the old startup prompts was quite complex which inevitably needed more than a single change.

[x] I have thoroughly tested my changes with multiple different prompts.
[x] I have considered potential risks and mitigations for my changes.
[x] I have documented my changes clearly and comprehensively.
[x] I have not snuck in any "extra" small tweaks changes.
[x] I have run the following commands against my code to ensure it passes our linters: black . / isort . / mypy
autoflake --remove-all-unused-imports --recursive --ignore-init-module-imports autogpt tests --in-place


PR Type

Enhancement, Tests


Description

  • Added support for storing and managing multiple AI configurations, including plugins.
  • Refactored prompt handling to introduce a new interactive menu for configuration management.
  • Integrated automatic configuration generation with the new menu system.
  • Added comprehensive unit and integration tests for the new features.

Changes walkthrough 📝

Relevant files
Enhancement
5 files
ai_config.py
Support for multiple AI configurations with plugins           

autogpt/config/ai_config.py

  • Added support for plugins in AI configurations.
  • Introduced methods for loading, saving, and deleting multiple AI
    configurations.
  • Updated configuration format to support multiple configurations.
  • +234/-41
    config.py
    Added plugin management and ai_settings_filepath                 

    autogpt/config/config.py

  • Added methods to set plugins allowlist and denylist.
  • Introduced ai_settings_filepath attribute.
  • +13/-1   
    main.py
    Main entry point modifications for configuration management

    autogpt/main.py

  • Modified main entry point to support new configuration management.
  • Added start_agent_directly function for direct agent start from menu.
  • +59/-4   
    prompt.py
    Refactored prompt handling and configuration management   

    autogpt/prompts/prompt.py

  • Refactored prompt handling to support new configuration menu.
  • Added functions for managing AI configurations interactively.
  • +811/-44
    setup.py
    Integrated automatic configuration with new menu system   

    autogpt/setup.py

  • Removed manual configuration setup.
  • Integrated automatic configuration generation with new menu system.
  • +40/-162
    Tests
    4 files
    test_setup.py
    Integration tests for configuration management                     

    tests/integration/test_setup.py

  • Added integration tests for new configuration management features.
  • Tested various scenarios including creation, deletion, and editing of
    configurations.
  • +765/-39
    test_ai_config.py
    Unit tests for AIConfig class                                                       

    tests/unit/test_ai_config.py

  • Added unit tests for AIConfig class methods.
  • Tested loading, saving, and deleting configurations.
  • +421/-45
    test_config.py
    Unit tests for plugin management in Config class                 

    tests/unit/test_config.py

  • Added unit tests for new plugin management methods in Config class.
  • +30/-0   
    test_prompt.py
    Unit tests for prompt handling functions                                 

    tests/unit/test_prompt.py

  • Added unit tests for prompt handling functions.
  • Tested validation and management of AI names.
  • +162/-0 

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    bassie661 added 8 commits June 9, 2023 16:50
    - config\ai_config.py: added support for plugins + load_all, save and delete function
    
    - config\config.py: added set functions for plugins_allowlist and plugins_denylist
    
    - prompts\prompt.py: merged --manual functionality from setup.py and split construct_main_ai_config into separate functions for more reading and maintainability, to incorporate the new menu options and added color coding for better user experience.
    
    - setup.py: removed --manual function and added save feature to save the automatically created configuration
    
    - main.py: added cfg.ai_settings_filepath to eliminate using SAVE_FILE and changed to call start_prompt function instead of construct_main_ai_config, also added a new function (start_agent_directly) for the user to be able to start an agent directly from the menu after editing.
    @vercel
    Copy link

    vercel bot commented Jun 10, 2023

    The latest updates on your projects. Learn more about Vercel for Git ↗︎

    1 Ignored Deployment
    Name Status Preview Comments Updated (UTC)
    docs ⬜️ Ignored (Inspect) Jun 12, 2023 5:16pm

    @github-actions
    Copy link
    Contributor

    This PR exceeds the recommended size of 500 lines. Please make sure you are NOT addressing multiple issues with one PR.

    - added more consistency across functions: to utilize the same prompt structure
    - fixed [R]eturn prompt: handle_config function
    - added and shortened comments for several functions
    @github-actions
    Copy link
    Contributor

    This PR exceeds the recommended size of 500 lines. Please make sure you are NOT addressing multiple issues with one PR.

    - added better pytests to cover more code
    - fixed exceptions ai_config class for existing (but empty) yaml file
    - added: configuration saved log for generate_aiconfig_automatic in setup.py
    @github-actions
    Copy link
    Contributor

    This PR exceeds the recommended size of 500 lines. Please make sure you are NOT addressing multiple issues with one PR.

    recorded test cassettes
    @github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Jun 11, 2023
    @github-actions
    Copy link
    Contributor

    This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

    @github-actions
    Copy link
    Contributor

    This PR exceeds the recommended size of 500 lines. Please make sure you are NOT addressing multiple issues with one PR.

    @github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Jun 11, 2023
    @github-actions
    Copy link
    Contributor

    Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

    @github-actions
    Copy link
    Contributor

    This PR exceeds the recommended size of 500 lines. Please make sure you are NOT addressing multiple issues with one PR.

    @github-actions
    Copy link
    Contributor

    This PR exceeds the recommended size of 500 lines. Please make sure you are NOT addressing multiple issues with one PR.

    @github-actions
    Copy link
    Contributor

    This PR exceeds the recommended size of 500 lines. Please make sure you are NOT addressing multiple issues with one PR.

    @github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Jun 27, 2023
    @github-actions
    Copy link
    Contributor

    This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

    @lc0rp lc0rp modified the milestones: v0.4.4 Release, v0.4.5 Jul 4, 2023
    @collijk collijk self-assigned this Jul 14, 2023
    @lc0rp lc0rp modified the milestones: v0.4.5 Release, v0.4.6 Release Jul 14, 2023
    @collijk collijk mentioned this pull request Jul 17, 2023
    26 tasks
    @lc0rp lc0rp modified the milestones: v0.4.6 Release, v0.4.7 Release Jul 22, 2023
    @lc0rp lc0rp modified the milestones: v0.4.7 Release, v0.4.8 Aug 1, 2023
    @Swiftyos Swiftyos closed this Jun 28, 2024
    @bassie661
    Copy link
    Author

    same, thanks for nothing

    @Swiftyos
    Copy link
    Contributor

    Swiftyos commented Jun 28, 2024

    @bassie661 the PR has had no activity since June 2023 and looked stale. Apologies, I've reopened.

    Have you tested this with the latest version of AutoGPT?

    I'll close again if there has been no activity in 2 weeks.

    @Swiftyos Swiftyos reopened this Jun 28, 2024
    @Swiftyos Swiftyos requested a review from a team as a code owner June 28, 2024 21:48
    @Swiftyos Swiftyos requested review from majdyz and removed request for a team June 28, 2024 21:48
    @qodo-merge-pro qodo-merge-pro bot added enhancement New feature or request tests labels Jun 28, 2024
    Copy link

    PR Description updated to latest commit (b077bce)

    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 5
    🧪 Relevant tests No
    🔒 Security concerns No
    ⚡ Key issues to review Possible Bug:
    The AIConfig class has a method to_dict where the comment mentions that plugins are a list of objectives, which seems incorrect. This should be a list of plugins or extensions, not objectives.
    Code Duplication:
    The methods load and load_all in AIConfig class have overlapping functionality with slight variations for loading configurations. Consider refactoring to combine these methods to reduce redundancy and improve maintainability.
    Error Handling:
    In the save method of AIConfig, there is a potential unhandled exception if the YAML file becomes corrupted or has invalid format. This should be handled gracefully to inform the user of the issue without crashing the application.
    Performance Concern:
    The update_old_config method in AIConfig reads the entire configuration file into memory, which might not be efficient for very large configuration files. Consider streaming the file or processing it in chunks.

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Use context managers for file operations to ensure proper file handling

    Use a context manager for file operations to ensure the file is properly closed after
    writing, which is more reliable and Pythonic.

    tests/integration/test_setup.py [115-116]

    +with open(temp_config_file, "w") as temp_file:
    +    temp_file.write(config_content)
     
    -
    • Apply this suggestion
    Suggestion importance[1-10]: 10

    Why: The suggestion to use a context manager for file operations is correct and follows best practices in Python. It ensures that the file is properly closed after writing, which is more reliable and Pythonic.

    10
    Use the clean_input utility for consistent input handling

    Replace the direct use of the input function with the clean_input utility function from
    the autogpt.utils module to ensure consistent input handling and sanitization across the
    application.

    autogpt/prompts/prompt.py [186]

    -user_input = input(prompt_text).strip()
    +user_input = utils.clean_input(config, prompt_text).strip()
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: This suggestion improves input sanitization and consistency across the application by using the existing clean_input utility function. This is a best practice for maintaining code consistency and security.

    9
    Use a context manager for file operations to ensure proper resource management

    Consider using a context manager for file operations to ensure that resources are properly
    managed and files are closed after operations are completed. This can prevent potential
    file locking or resource leakage issues.

    tests/unit/test_ai_config.py [46-47]

    -config_file = config.ai_settings_filepath
    -config_file.write_text(old_yaml_content)
    +with open(config.ai_settings_filepath, 'w') as config_file:
    +    config_file.write(old_yaml_content)
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Using a context manager for file operations is a best practice that ensures files are properly closed, preventing potential file locking or resource leakage issues. This is a significant improvement for resource management.

    8
    Maintainability
    Use a pytest fixture to manage the configuration file path across tests

    To avoid hardcoding the file path in multiple test cases, consider using a fixture to
    manage the configuration file path. This can make the tests easier to maintain and modify.

    tests/unit/test_ai_config.py [46]

    -config_file = config.ai_settings_filepath
    +@pytest.fixture
    +def config_file_path(tmp_path):
    +    return tmp_path / "ai_settings.yaml"
     
    +def test_some_function(config_file_path):
    +    # Use config_file_path in your test
    +
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Using a pytest fixture to manage the configuration file path improves maintainability by avoiding hardcoding the file path in multiple test cases. This makes the tests easier to maintain and modify, which is highly beneficial for long-term code maintenance.

    9
    Possible bug
    Add missing import for the List type from the typing module

    The plugins parameter in the AIConfig class constructor is defined as Optional[List], but
    it is not imported from the typing module. This might lead to a runtime error if the type
    is not recognized. It is recommended to import List from the typing module explicitly.

    autogpt/config/ai_config.py [37]

    +from typing import List
     plugins: Optional[List] = None,
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: The suggestion correctly identifies a missing import for the List type, which could lead to runtime errors. This is a crucial fix to ensure the code runs without issues.

    9
    Possible issue
    Ensure num_goals does not exceed the maximum limit

    Add a check to ensure that the num_goals variable does not exceed a predefined maximum
    limit to prevent potential misuse or errors in goal setting.

    autogpt/prompts/prompt.py [146]

     num_goals = int(num_goals_input)
    +if num_goals > MAX_GOALS:  # Assuming MAX_GOALS is defined elsewhere as the maximum allowed goals
    +    logger.typewriter_log("Number of goals exceeds the maximum limit.", Fore.RED)
    +    continue
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Adding a check to ensure num_goals does not exceed a predefined maximum limit is crucial for preventing potential misuse or errors in goal setting. This enhances the robustness of the code.

    8
    Enhancement
    Simplify mocking user inputs in tests using MagicMock

    Instead of manually iterating through user_inputs with next(), consider using
    unittest.mock.MagicMock to automatically handle input sequences. This simplifies the test
    setup and makes it more readable.

    tests/integration/test_setup.py [41-44]

    -user_inputs_iterator = iter(user_inputs)
    -def mock_input(_prompt: str) -> str:
    -    return next(user_inputs_iterator)
    +mock_input = MagicMock(side_effect=user_inputs)
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Using MagicMock to handle input sequences simplifies the test setup and makes the code more readable. This is a good enhancement for the test code, although the existing approach is also functional.

    8
    Correct the description of the plugins attribute in the AIConfig class

    The description for the plugins attribute in the AIConfig class is incorrect. It is
    currently duplicated as "The list of objectives the AI is supposed to complete," which is
    the same as for ai_goals. This should be corrected to reflect the actual purpose of the
    plugins attribute.

    autogpt/config/ai_config.py [28]

    -plugins (list): The list of objectives the AI is supposed to complete.
    +plugins (list): The list of plugins configured for the AI.
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion accurately points out a misleading description for the plugins attribute. Correcting this improves code documentation and clarity.

    8
    Ensure the AI name is not an empty string in the delete method

    The delete method in the AIConfig class raises a ValueError if the AI name is not
    provided, but it does not check if the AI name is a non-empty string. It is recommended to
    add a check to ensure that the AI name is not just an empty string.

    autogpt/config/ai_config.py [282-285]

    -if ai_name == "":
    +if not ai_name.strip():
         raise ValueError(
    -        "No AI name provided. Please provide an AI name to delete its configuration."
    +        "No AI name provided or AI name is empty. Please provide a valid AI name to delete its configuration."
         )
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion enhances the robustness of the delete method by ensuring the AI name is not just an empty string, which is important for preventing potential errors.

    8

    @Swiftyos
    Copy link
    Contributor

    Swiftyos commented Jul 5, 2024

    @bassie661 are you planning on picking this up again?

    @bassie661
    Copy link
    Author

    bassie661 commented Jul 23, 2024 via email

    @Swiftyos
    Copy link
    Contributor

    Okay no worries, closing now then.

    @Swiftyos Swiftyos closed this Jul 25, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    conflicts Automatically applied to PRs with merge conflicts enhancement New feature or request function: config Review effort [1-5]: 5 size/xl tests
    Projects
    Status: Done
    Development

    Successfully merging this pull request may close these issues.

    5 participants