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

Moving to LiteLLM with env var approach. Closes #15. #24

Merged
merged 8 commits into from
May 22, 2024

Conversation

EmbeddedDevops1
Copy link
Collaborator

@EmbeddedDevops1 EmbeddedDevops1 commented May 22, 2024

PR Type

Enhancement, Documentation, Tests


Description

  • Migrated from OpenAI to LiteLLM for language model interactions in AICaller.
  • Removed dependency on OPENAI_API_KEY environment variable.
  • Updated UnitTestGenerator to log total token usage.
  • Renamed CLI argument --openai-model to --model.
  • Updated tests to reflect LiteLLM integration and removed obsolete tests.
  • Updated README with new instructions for using LiteLLM and selecting models.
  • Added litellm and boto3 as dependencies in pyproject.toml.

Changes walkthrough 📝

Relevant files
Enhancement
AICaller.py
Migrate from OpenAI to LiteLLM in AICaller                             

cover_agent/AICaller.py

  • Replaced OpenAI API calls with LiteLLM.
  • Removed environment variable dependency for API key.
  • Simplified the call_model method.
  • Removed token counting functionality.
  • +14/-55 
    UnitTestGenerator.py
    Update UnitTestGenerator for LiteLLM integration                 

    cover_agent/UnitTestGenerator.py

  • Updated generate_tests method to log total token usage.
  • Adjusted method call to align with LiteLLM changes.
  • +4/-4     
    main.py
    Rename CLI argument for model selection                                   

    cover_agent/main.py

    • Renamed --openai-model argument to --model.
    +3/-3     
    Tests
    test_AICaller.py
    Update tests for LiteLLM integration in AICaller                 

    tests/test_AICaller.py

  • Removed tests related to OpenAI API key and token counting.
  • Added tests for LiteLLM integration.
  • +31/-60 
    Documentation
    README.md
    Update README for LiteLLM usage and model selection           

    README.md

  • Updated instructions to reflect the switch to LiteLLM.
  • Added section on using other LLMs with LiteLLM.
  • +17/-1   
    Dependencies
    pyproject.toml
    Add LiteLLM and boto3 dependencies                                             

    pyproject.toml

    • Added litellm and boto3 as dependencies.
    +2/-0     

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

    @EmbeddedDevops1 EmbeddedDevops1 linked an issue May 22, 2024 that may be closed by this pull request
    @EmbeddedDevops1
    Copy link
    Collaborator Author

    /describe

    @qodo-merge-pro qodo-merge-pro bot added documentation Improvements or additions to documentation enhancement New feature or request tests Review effort [1-5]: 3 labels May 22, 2024
    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    3, because the PR involves multiple changes across several files, including the migration from OpenAI API to LiteLLM, removal of token counting, and updates to unit tests and documentation. The changes are significant but well-documented, making the review process moderately complex.

    🧪 Relevant tests

    Yes

    ⚡ Possible issues

    Possible Bug: The call_model method in AICaller.py has been significantly refactored to use LiteLLM. While the new implementation seems correct, the removal of token counting and changes in how responses are handled could introduce bugs if not thoroughly tested, especially in edge cases where the model's response might not be as expected.

    Performance Concern: The use of time.sleep(0.01) in the streaming loop of call_model might introduce unnecessary delays in processing model responses. This could impact the performance, especially when handling large volumes of data or in a production environment.

    🔒 Security concerns

    No

    @EmbeddedDevops1
    Copy link
    Collaborator Author

    /improve

    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add a check to ensure the expected structure exists before accessing it

    To handle the case where litellm.completion might not return the expected structure, add a
    check to ensure chunk.choices[0].delta.content exists before accessing it.

    cover_agent/AICaller.py [33]

    -print(chunk.choices[0].delta.content or "", end="", flush=True)
    +content = chunk.choices[0].delta.content if chunk.choices and chunk.choices[0].delta else ""
    +print(content, end="", flush=True)
     
    Suggestion importance[1-10]: 8

    Why: This suggestion correctly identifies a potential issue where the code may attempt to access properties of an object that might not exist, which could lead to an AttributeError. Adding a check enhances robustness and prevents runtime errors.

    8
    Best practice
    Move the logger message inside the else block to ensure it is only logged when not in dry run mode

    To ensure the logger message is only logged when not in dry run mode, move the logging
    statement inside the else block.

    cover_agent/UnitTestGenerator.py [176-178]

    -self.logger.info(
    -    f"Total token used count for LLM model {LLM_model}: {prompt_token_count + response_token_count}"
    -)
    +if not dry_run:
    +    response, prompt_token_count, response_token_count = ai_caller.call_model(prompt=self.prompt, max_tokens=max_tokens)
    +    self.logger.info(
    +        f"Total token used count for LLM model {LLM_model}: {prompt_token_count + response_token_count}"
    +    )
     
    Suggestion importance[1-10]: 7

    Why: The suggestion correctly improves the logic to ensure that logging of token counts only occurs when not in dry run mode, which is a good practice to avoid misleading log entries during test scenarios.

    7
    Maintainability
    Remove redundant instantiation of AICaller in the test_call_model_simplified method

    To ensure that the ai_caller fixture is used correctly, remove the redundant instantiation
    of AICaller in the test_call_model_simplified method.

    tests/test_AICaller.py [15-17]

    -ai_caller = AICaller("test-model")
     response, prompt_tokens, response_tokens = ai_caller.call_model("Hello, world!", max_tokens=4096)
     
    Suggestion importance[1-10]: 6

    Why: The suggestion correctly points out unnecessary code duplication by using the ai_caller fixture directly, which improves code cleanliness and reduces the potential for errors in test setups.

    6
    Align the default and help parameters of the --model argument for better readability

    To improve readability, align the default and help parameters of the --model argument with
    the other arguments.

    cover_agent/main.py [69-71]

    +"--model",
    +default="gpt-4o",
    +help="Which LLM model to use. Default: %(default)s.",
     
    -
    Suggestion importance[1-10]: 5

    Why: This suggestion aims to improve the readability and consistency of the argument definitions in the code, which is beneficial for maintainability but not critical.

    5

    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Test
    Add a test case to verify the default value of max_tokens in call_model

    Add a test case to verify the behavior of call_model when max_tokens is not provided,
    ensuring it defaults to 4096.

    tests/test_AICaller.py [10-25]

     @patch('cover_agent.AICaller.AICaller.call_model')
     def test_call_model_simplified(self, mock_call_model):
         # Setup the mock to return a predefined response
         mock_call_model.return_value = ("Hello world!", 2, 10)
     
         ai_caller = AICaller("test-model")
         # Explicitly provide the default value of max_tokens
         response, prompt_tokens, response_tokens = ai_caller.call_model("Hello, world!", max_tokens=4096)
     
         # Assertions to check if the returned values are as expected
         assert response == "Hello world!"
         assert prompt_tokens == 2
         assert response_tokens == 10
     
         # Check if call_model was called correctly
         mock_call_model.assert_called_once_with("Hello, world!", max_tokens=4096)
     
    +    # Test default max_tokens
    +    response, prompt_tokens, response_tokens = ai_caller.call_model("Hello, world!")
    +    mock_call_model.assert_called_with("Hello, world!", max_tokens=4096)
    +
    Suggestion importance[1-10]: 8

    Why: This suggestion is very valuable as it adds a test case to verify the behavior of call_model when max_tokens is not provided, ensuring it defaults to 4096. This is crucial for ensuring the method behaves as expected under default conditions.

    8
    Enhancement
    Make the response pacing delay conditional based on an optional parameter

    The time.sleep(0.01) call inside the loop can be made conditional based on an optional
    parameter to allow more flexibility in controlling the pacing of the response.

    cover_agent/AICaller.py [35]

    -time.sleep(0.01)  # Optional: Delay to simulate more 'natural' response pacing
    +if self.simulate_pacing:
    +    time.sleep(0.01)  # Optional: Delay to simulate more 'natural' response pacing
     
    Suggestion importance[1-10]: 7

    Why: The suggestion correctly identifies an enhancement in the response pacing mechanism, making it more flexible by introducing a conditional delay based on an optional parameter. This is a good improvement for usability and control.

    7
    Possible issue
    Ensure token count variables are defined before logging their values

    Add a check to ensure prompt_token_count and response_token_count are defined before
    logging their values to avoid potential NameError.

    cover_agent/UnitTestGenerator.py [176-178]

    -self.logger.info(
    -    f"Total token used count for LLM model {LLM_model}: {prompt_token_count + response_token_count}"
    -)
    +if 'prompt_token_count' in locals() and 'response_token_count' in locals():
    +    self.logger.info(
    +        f"Total token used count for LLM model {LLM_model}: {prompt_token_count + response_token_count}"
    +    )
     
    Suggestion importance[1-10]: 6

    Why: The suggestion correctly addresses a potential issue where the variables might not be defined before they are used, which could lead to a NameError. This is a good preventive measure to ensure robustness.

    6
    Best practice
    Validate the --model argument to ensure it is not an empty string

    Validate the --model argument to ensure it is not an empty string, providing a meaningful
    error message if it is.

    cover_agent/main.py [69-72]

     parser.add_argument(
         "--model",
         default="gpt-4o",
         help="Which LLM model to use. Default: %(default)s.",
    +    type=str,
    +    required=True
     )
     
    Suggestion importance[1-10]: 5

    Why: The suggestion to validate the --model argument to ensure it is not empty is a good practice to avoid runtime errors. However, the improved code provided does not correctly implement the validation as it suggests making the argument required, which contradicts the default value provided.

    5

    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    CI Failure Feedback 🧐

    Action: build (macos-latest)

    Failed stage: Test Executable [❌]

    Failure summary:

    The action failed because of an unhandled exception in the script 'main'. The specific error was a
    ValueError raised in the tiktoken/registry.py file at line 68. The error message indicates that an
    unknown encoding cl100k_base was requested, and no plugins were found to handle this encoding.

    Relevant error logs:
    1:  ##[group]Operating System
    2:  macOS
    ...
    
    507:  File "PyInstaller/loader/pyimod02_importers.py", line 419, in exec_module
    508:  File "litellm/__init__.py", line 695, in <module>
    509:  File "<frozen importlib._bootstrap>", line 1360, in _find_and_load
    510:  File "<frozen importlib._bootstrap>", line 1331, in _find_and_load_unlocked
    511:  File "<frozen importlib._bootstrap>", line 935, in _load_unlocked
    512:  File "PyInstaller/loader/pyimod02_importers.py", line 419, in exec_module
    513:  File "litellm/utils.py", line 57, in <module>
    514:  File "tiktoken/registry.py", line 68, in get_encoding
    515:  ValueError: Unknown encoding cl100k_base. Plugins found: []
    516:  [4154] Failed to execute script 'main' due to unhandled exception!
    517:  ##[error]Process completed with exit code 1.
    

    ✨ CI feedback usage guide:

    The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
    The tool analyzes the failed checks and provides several feedbacks:

    • Failed stage
    • Failed test name
    • Failure summary
    • Relevant error logs

    In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

    /checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"
    

    where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

    Configuration options

    • enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
    • excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
    • enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
    • persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
    • final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

    See more information about the checks tool in the docs.

    @EmbeddedDevops1 EmbeddedDevops1 force-pushed the 15-feature-request-support-locally-running-llm branch from c34b615 to c90459a Compare May 22, 2024 04:44
    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    CI Failure Feedback 🧐

    Action: build (macos-latest)

    Failed stage: Test Executable [❌]

    Failure summary:

    The action failed because the script 'main' encountered an unhandled exception. The specific error
    was a FileNotFoundError, indicating that the file anthropic_tokenizer.json could not be found in the
    directory /var/folders/2f/8t5k6yr535sdw0s4glnpxrzm0000gn/T/_MEIgGGl4x/litellm/llms/tokenizers/.

    Relevant error logs:
    1:  ##[group]Operating System
    2:  macOS
    ...
    
    510:  File "<frozen importlib._bootstrap>", line 1360, in _find_and_load
    511:  File "<frozen importlib._bootstrap>", line 1331, in _find_and_load_unlocked
    512:  File "<frozen importlib._bootstrap>", line 935, in _load_unlocked
    513:  File "PyInstaller/loader/pyimod02_importers.py", line 419, in exec_module
    514:  File "litellm/utils.py", line 59, in <module>
    515:  File "importlib/resources/_legacy.py", line 25, in wrapper
    516:  File "importlib/resources/_legacy.py", line 62, in open_text
    517:  File "pathlib.py", line 1013, in open
    518:  FileNotFoundError: [Errno 2] No such file or directory: '/var/folders/2f/8t5k6yr535sdw0s4glnpxrzm0000gn/T/_MEIgGGl4x/litellm/llms/tokenizers/anthropic_tokenizer.json'
    519:  [3772] Failed to execute script 'main' due to unhandled exception!
    520:  ##[error]Process completed with exit code 1.
    

    ✨ CI feedback usage guide:

    The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
    The tool analyzes the failed checks and provides several feedbacks:

    • Failed stage
    • Failed test name
    • Failure summary
    • Relevant error logs

    In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

    /checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"
    

    where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

    Configuration options

    • enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
    • excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
    • enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
    • persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
    • final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

    See more information about the checks tool in the docs.

    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    CI Failure Feedback 🧐

    Action: build (ubuntu-latest)

    Failed stage: Test Executable [❌]

    Failure summary:

    The action failed because the script main encountered a FileNotFoundError. The specific file that
    could not be found is anthropic_tokenizer.json located at /tmp/_MEIwrPZle/litellm/llms/tokenizers/.
    This error occurred during the execution of the module litellm/utils.py.

    Relevant error logs:
    1:  ##[group]Operating System
    2:  Ubuntu
    ...
    
    527:  File "<frozen importlib._bootstrap>", line 1360, in _find_and_load
    528:  File "<frozen importlib._bootstrap>", line 1331, in _find_and_load_unlocked
    529:  File "<frozen importlib._bootstrap>", line 935, in _load_unlocked
    530:  File "PyInstaller/loader/pyimod02_importers.py", line 419, in exec_module
    531:  File "litellm/utils.py", line 59, in <module>
    532:  File "importlib/resources/_legacy.py", line 25, in wrapper
    533:  File "importlib/resources/_legacy.py", line 62, in open_text
    534:  File "pathlib.py", line 1013, in open
    535:  FileNotFoundError: [Errno 2] No such file or directory: '/tmp/_MEIwrPZle/litellm/llms/tokenizers/anthropic_tokenizer.json'
    536:  [2658] Failed to execute script 'main' due to unhandled exception!
    537:  ##[error]Process completed with exit code 1.
    

    ✨ CI feedback usage guide:

    The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
    The tool analyzes the failed checks and provides several feedbacks:

    • Failed stage
    • Failed test name
    • Failure summary
    • Relevant error logs

    In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

    /checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"
    

    where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

    Configuration options

    • enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
    • excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
    • enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
    • persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
    • final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

    See more information about the checks tool in the docs.

    @qodo-ai qodo-ai deleted a comment from qodo-merge-pro bot May 22, 2024
    @qodo-ai qodo-ai deleted a comment from qodo-merge-pro bot May 22, 2024
    @qodo-ai qodo-ai deleted a comment from qodo-merge-pro bot May 22, 2024
    Copy link
    Contributor

    qodo-merge-pro bot commented May 22, 2024

    CI Failure Feedback 🧐

    (Checks updated until commit 658aef5)

    Action: build (windows-latest)

    Failed stage: Test Executable [❌]

    Failure summary:

    The action failed due to a syntax error in a PowerShell script:

  • The if statement on line 2 of the script D:\a_temp\e76c4522-2506-4553-8384-46261faaa035.ps1 is
    missing a '(' after 'if'.
  • This caused a ParserError and the process completed with exit code 1.

  • Relevant error logs:
    1:  ##[group]Operating System
    2:  Microsoft Windows Server 2022
    ...
    
    481:  �[36;1m  ./dist/cover-agent-windows-latest.exe --help�[0m
    482:  �[36;1melse�[0m
    483:  �[36;1m  ./dist/cover-agent-windows-latest --help�[0m
    484:  �[36;1mfi�[0m
    485:  shell: C:\Program Files\PowerShell\7\pwsh.EXE -command ". '{0}'"
    486:  env:
    487:  pythonLocation: C:\hostedtoolcache\windows\Python\3.12.3\x64
    488:  ##[endgroup]
    489:  �[31;1mParserError: �[0mD:\a\_temp\e76c4522-2506-4553-8384-46261faaa035.ps1:2�[0m
    490:  �[31;1m�[0m�[36;1mLine |�[0m
    491:  �[31;1m�[0m�[36;1m�[36;1m   2 | �[0m if�[36;1m �[0m[ "windows-latest" = "windows-latest" ]; then�[0m
    492:  �[31;1m�[0m�[36;1m�[36;1m�[0m�[36;1m�[0m�[36;1m     | �[31;1m   ~�[0m
    493:  �[31;1m�[0m�[36;1m�[36;1m�[0m�[36;1m�[0m�[36;1m�[31;1m�[31;1m�[36;1m     | �[31;1mMissing '(' after 'if' in if statement.�[0m
    494:  ##[error]Process completed with exit code 1.
    

    ✨ CI feedback usage guide:

    The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
    The tool analyzes the failed checks and provides several feedbacks:

    • Failed stage
    • Failed test name
    • Failure summary
    • Relevant error logs

    In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

    /checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"
    

    where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

    Configuration options

    • enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
    • excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
    • enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
    • persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
    • final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

    See more information about the checks tool in the docs.

    @mrT23 mrT23 self-requested a review May 22, 2024 07:03
    @mrT23 mrT23 merged commit e9654cc into main May 22, 2024
    6 checks passed
    @EmbeddedDevops1 EmbeddedDevops1 deleted the 15-feature-request-support-locally-running-llm branch May 22, 2024 16:31
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    documentation Improvements or additions to documentation enhancement New feature or request Review effort [1-5]: 3 tests
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    Feature Request: Support Locally Running LLM
    3 participants