Skip to content

Fix: correctly pass api_base and add --max-run-time CLI argument in main_full_repo #319

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

cuso4huang
Copy link

@cuso4huang cuso4huang commented Apr 17, 2025

User description

What this PR does

This PR fixes two issues encountered when running the main_full_repo entry point:

  1. api_base not passed to AICaller:
    The CLI argument --api_base was accepted but never passed into AICaller, making it ineffective.
    ✅ Fixed by modifying the initialization of AICaller to include api_base.

  2. Missing --max-run-time argument:
    The CLI argument parser parse_args_full_repo did not include --max-run-time, which led to runtime errors like:
    'Namespace' object has no attribute 'max_run_time'
    ✅ Added the missing argument to argparse.

Optionally, I also added max_tokens manually to the AICaller init call with a default value (8192) to avoid instantiation errors.


Suggested labels:

bug, CLI, enhancement

Let me know if any changes are needed — happy to update!


PR Type

Bug fix, Enhancement


Description

  • Fixed missing api_base argument in AICaller initialization.

  • Added --max-run-time argument to CLI parser with default value.

  • Set default max_tokens value in AICaller initialization.


Changes walkthrough 📝

Relevant files
Bug fix
main_full_repo.py
Fix `AICaller` initialization with additional arguments   

cover_agent/main_full_repo.py

  • Added api_base and max_tokens arguments to AICaller initialization.
  • Ensures proper instantiation of AICaller with required parameters.
  • +1/-1     
    Enhancement
    utils.py
    Add `--max-run-time` argument to CLI parser                           

    cover_agent/utils.py

  • Added --max-run-time argument to CLI parser.
  • Provided default value and help text for --max-run-time.
  • +6/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No major issues detected

    Copy link

    qodo-merge-for-open-source bot commented Apr 17, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Learned
    best practice
    Add defensive programming to handle potentially missing command line arguments
    Suggestion Impact:The commit implemented exactly what was suggested - adding a getattr check for args.api_base with a default value of None before passing it to the AICaller constructor

    code diff:

    -        ai_caller = AICaller(model=args.model,api_base=args.api_base, max_tokens=8192)
    +        api_base = getattr(args, 'api_base', None)
    +        ai_caller = AICaller(model=args.model, api_base=api_base, max_tokens=8192)

    The code is passing args.api_base to the AICaller constructor without checking
    if it exists or has a valid value. If api_base is not provided in the command
    line arguments, this could lead to a runtime error. Add a check to ensure
    api_base has a valid value before using it.

    cover_agent/main_full_repo.py [31]

    -ai_caller = AICaller(model=args.model,api_base=args.api_base, max_tokens=8192)
    +api_base = getattr(args, 'api_base', None)
    +ai_caller = AICaller(model=args.model, api_base=api_base, max_tokens=8192)

    [Suggestion has been applied]

    Suggestion importance[1-10]: 6
    Low
    General
    Fix spacing in parameters
    Suggestion Impact:The commit implemented the spacing fix suggested by adding a space after the comma between parameters. However, the commit also made another change by extracting the api_base parameter into a separate variable.

    code diff:

    -        ai_caller = AICaller(model=args.model,api_base=args.api_base, max_tokens=8192)
    +        api_base = getattr(args, 'api_base', None)
    +        ai_caller = AICaller(model=args.model, api_base=api_base, max_tokens=8192)

    Add a space after the comma between parameters for better code readability and
    to follow PEP 8 style guidelines.

    cover_agent/main_full_repo.py [31]

    -ai_caller = AICaller(model=args.model,api_base=args.api_base, max_tokens=8192)
    +ai_caller = AICaller(model=args.model, api_base=args.api_base, max_tokens=8192)

    [Suggestion has been applied]

    Suggestion importance[1-10]: 3

    __

    Why: The suggestion correctly identifies a minor style issue where there's no space after the comma between parameters. While this is a valid PEP 8 style improvement for readability, it's a relatively minor cosmetic change with no functional impact.

    Low
    • Update

    @EmbeddedDevops1
    Copy link
    Collaborator

    EmbeddedDevops1 commented Apr 24, 2025

    @cuso4huang Thank you for putting this PR and issue together. Are you able to post a sample run output from the command line (including the command that you used to run this)?

    @cuso4huang
    Copy link
    Author

    @cuso4huang Thank you for putting this PR and issue together. Are you able to post a sample run output from the command line (including the command that you used to run this)?

    Hi,this is the result

    (qodo) cuso4@cuso4 qodo-cover (main) $ poetry run cover-agent-full-repo \
      --project-language="python" \
      --project-root="/home/cuso4/qodo-cover/templated_tests/python_fastapi" \
      --code-coverage-report-path="/home/cuso4/qodo-cover/templated_tests/python_fastapi/coverage.xml" \
      --test-command="coverage run -m pytest /home/cuso4/qodo-cover/templated_tests/python_fastapi/tests --cov=/home/cuso4/qodo-cover/templated_tests/python_fastapi/app --cov-report=xml --cov-report=term" \
      --model="openai/deepseek-chat" \
      --api-base="https://api.deepseek.com"
    ============
    Test files to be extended:
    /home/cuso4/qodo-cover/templated_tests/python_fastapi/tests/test_app.py
    ============
    
    
    Initializing language server...
    LSP server initialized.
    /home/cuso4/miniconda3/envs/qodo/lib/python3.11/site-packages/tree_sitter/__init__.py:36: FutureWarning: Language(path, name) is deprecated. Use Language(ptr, name) instead.
      warn("{} is deprecated. Use {} instead.".format(old, new), FutureWarning)
    Context files for test file '/home/cuso4/qodo-cover/templated_tests/python_fastapi/tests/test_app.py':
    /home/cuso4/qodo-cover/templated_tests/python_fastapi/app/app.py
    
    
    Analyzing test file against context files...
    Printing results from LLM model...
    is_this_a_unit_test: 1
    main_file: |
      app/app.py

    Test file: /home/cuso4/qodo-cover/templated_tests/python_fastapi/tests/test_app.py,
    is a unit test file for source file: /home/cuso4/qodo-cover/templated_tests/python_fastapi/app/app.py
    Converting test command: coverage run -m pytest /home/cuso4/qodo-cover/templated_tests/python_fastapi/tests --cov=/home/cuso4/qodo-cover/templated_tests/python_fastapi/app --cov-report=xml --cov-report=term
    to run only a single test: coverage run -m pytest tests/test_app.py --cov=/home/cuso4/qodo-cover/templated_tests/python_fastapi/app --cov-report=xml --cov-report=term
    Streaming results from LLM model...
    language: python
    testing_framework: pytest
    number_of_tests: 2
    test_headers_indentation: 0

    
    Streaming results from LLM model...
    language: python
    testing_framework: pytest
    number_of_tests: 2
    relevant_line_number_to_insert_tests_after: 29
    relevant_line_number_to_insert_imports_after: 5
    

    2025-04-24 09:38:39,672 - cover_agent.UnitTestValidator - INFO - Running build/test command to generate coverage report: "coverage run -m pytest tests/test_app.py --cov=/home/cuso4/qodo-cover/templated_tests/python_fastapi/app --cov-report=xml --cov-report=term"
    2025-04-24 09:38:42,413 - cover_agent.UnitTestValidator - INFO - Initial coverage: 63.64%
    2025-04-24 09:38:42,415 - cover_agent.CoverAgent - INFO - Current Coverage: 63.64%
    2025-04-24 09:38:42,415 - cover_agent.CoverAgent - INFO - Desired Coverage: 70%
    Streaming results from LLM model...
    language: python
    existing_test_function_signature: |
    def test_add(num1, num2, expected):
    new_tests:

    • test_behavior: |
      Test that the current_date endpoint returns today's date in ISO format
      lines_to_cover: |
      [23]
      test_name: |
      test_current_date
      test_code: |
      def test_current_date():
      """Test that the current_date endpoint returns today's date"""
      response = client.get("/current-date")
      assert response.status_code == 200
      assert response.json() == {"date": date.today().isoformat()}
      new_imports_code: |
      ""
      test_tags: happy path
    • test_behavior: |
      Test that the subtract endpoint correctly subtracts two numbers
      lines_to_cover: |
      [46]
      test_name: |
      test_subtract
      test_code: |
      @pytest.mark.parametrize(
      "num1,num2,expected",
      [
      (5, 3, 2),
      (-5, 3, -8),
      (0, 0, 0),
      (100, 50, 50),
      ]
      )
      def test_subtract(num1, num2, expected):
      """Test that the subtract endpoint works correctly"""
      response = client.get(f"/subtract/{num1}/{num2}")
      assert response.status_code == 200
      assert response.json() == {"result": expected}
      new_imports_code: |
      ""
      test_tags: happy path
    • test_behavior: |
      Test that the divide endpoint raises an HTTPException when dividing by zero
      lines_to_cover: |
      [76, 77]
      test_name: |
      test_divide_by_zero
      test_code: |
      def test_divide_by_zero():
      """Test that dividing by zero raises an HTTPException"""
      response = client.get("/divide/5/0")
      assert response.status_code == 400
      assert response.json() == {"detail": "Cannot divide by zero"}
      new_imports_code: |
      ""
      test_tags: edge case
    • test_behavior: |
      Test that the square endpoint correctly calculates the square of a number
      lines_to_cover: |
      [86]
      test_name: |
      test_square
      test_code: |
      @pytest.mark.parametrize(
      "number,expected",
      [
      (5, 25),
      (-3, 9),
      (0, 0),
      (10, 100),
      ]
      )
      def test_square(number, expected):
      """Test that the square endpoint works correctly"""
      response = client.get(f"/square/{number}")
      assert response.status_code == 200
      assert response.json() == {"result": expected}
      new_imports_code: |
      ""
      test_tags: happy path
    
    2025-04-24 09:39:14,698 - cover_agent.UnitTestValidator - INFO - Running test with the following command: "coverage run -m pytest tests/test_app.py --cov=/home/cuso4/qodo-cover/templated_tests/python_fastapi/app --cov-report=xml --cov-report=term"
    2025-04-24 09:39:17,062 - cover_agent.UnitTestValidator - INFO - Test passed and coverage increased. Current coverage: 65.91%
    2025-04-24 09:39:17,063 - cover_agent.UnitTestValidator - INFO - Running test with the following command: "coverage run -m pytest tests/test_app.py --cov=/home/cuso4/qodo-cover/templated_tests/python_fastapi/app --cov-report=xml --cov-report=term"
    2025-04-24 09:39:18,941 - cover_agent.UnitTestValidator - INFO - Test passed and coverage increased. Current coverage: 68.18%
    2025-04-24 09:39:18,942 - cover_agent.UnitTestValidator - INFO - Running test with the following command: "coverage run -m pytest tests/test_app.py --cov=/home/cuso4/qodo-cover/templated_tests/python_fastapi/app --cov-report=xml --cov-report=term"
    2025-04-24 09:39:20,539 - cover_agent.UnitTestValidator - INFO - Test passed and coverage increased. Current coverage: 72.73%
    2025-04-24 09:39:20,539 - cover_agent.UnitTestValidator - INFO - Running test with the following command: "coverage run -m pytest tests/test_app.py --cov=/home/cuso4/qodo-cover/templated_tests/python_fastapi/app --cov-report=xml --cov-report=term"
    2025-04-24 09:39:22,066 - cover_agent.UnitTestValidator - INFO - Test passed and coverage increased. Current coverage: 75.0%
    2025-04-24 09:39:22,104 - cover_agent.UnitTestValidator - INFO - Running build/test command to generate coverage report: "coverage run -m pytest tests/test_app.py --cov=/home/cuso4/qodo-cover/templated_tests/python_fastapi/app --cov-report=xml --cov-report=term"
    2025-04-24 09:39:23,714 - cover_agent.UnitTestValidator - INFO - Initial coverage: 75.0%
    2025-04-24 09:39:23,714 - cover_agent.CoverAgent - INFO - Reached above target coverage of 70% (Current Coverage: 75.0%) in 0 iterations.
    2025-04-24 09:39:23,714 - cover_agent.CoverAgent - INFO - Total number of input tokens used for LLM model openai/deepseek-chat: 3601
    2025-04-24 09:39:23,714 - cover_agent.CoverAgent - INFO - Total number of output tokens used for LLM model openai/deepseek-chat: 657
    (qodo) cuso4@cuso4 qodo-cover (main) $ 
    

    @EmbeddedDevops1
    Copy link
    Collaborator

    @cuso4huang we're ready to merge but I see there are conflicts. Can you rebase and resolve please?

    @EmbeddedDevops1
    Copy link
    Collaborator

    Sorry @cuso4huang can you rebase once more? You’re next in the queue.

    @EmbeddedDevops1
    Copy link
    Collaborator

    @cuso4huang Thanks for the rebase. I am noticing that the unit tests are failing. Did you take a look after rebasing?

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    AICaller not initialized properly with api_key, api_base, and missing max_tokens argument in parse_args_full_repo
    3 participants