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

Asymmetric context #1114

Merged
merged 11 commits into from
Aug 11, 2024
Merged

Asymmetric context #1114

merged 11 commits into from
Aug 11, 2024

Conversation

mrT23
Copy link
Collaborator

@mrT23 mrT23 commented Aug 11, 2024

PR Type

Enhancement, Tests


Description

  • Implemented separate parameters for extending patches before and after changes (patch_extra_lines_before and patch_extra_lines_after)
  • Updated extend_patch function and related PR processing logic to support the new patch extension parameters
  • Added tests for the new patch extension functionality
  • Updated configuration and documentation to reflect the new patch extension parameters
  • Increased max_context_tokens for PR code suggestions from 10000 to 16000
  • Added support for the 'claude-3-5-sonnet' model
  • Enabled extra lines in code suggestions by default

Changes walkthrough 📝

Relevant files
Configuration changes
__init__.py
Add new Claude model                                                                         

pr_agent/algo/init.py

  • Added 'claude-3-5-sonnet' model with a token limit of 100000
+1/-0     
configuration.toml
Update configuration for patch extension                                 

pr_agent/settings/configuration.toml

  • Changed patch_extra_lines to separate patch_extra_lines_before and
    patch_extra_lines_after
  • Increased max_context_tokens for PR code suggestions from 10000 to
    16000
  • +3/-2     
    Enhancement
    git_patch_processing.py
    Refactor patch extension logic                                                     

    pr_agent/algo/git_patch_processing.py

  • Modified extend_patch function to accept separate parameters for lines
    before and after
  • Updated logic to extend patches based on new parameters
  • +8/-19   
    pr_processing.py
    Update PR diff generation with new patch extension             

    pr_agent/algo/pr_processing.py

  • Updated get_pr_diff and pr_generate_extended_diff functions to use
    separate before and after line parameters
  • Modified patch extension calls to use new parameters
  • +15/-17 
    pr_code_suggestions.py
    Enable extra lines in code suggestions                                     

    pr_agent/tools/pr_code_suggestions.py

  • Changed disable_extra_lines parameter to False in _prepare_prediction
    method
  • +1/-1     
    Tests
    test_extend_patch.py
    Update and add tests for patch extension                                 

    tests/unittest/test_extend_patch.py

  • Updated existing tests to use new patch extension parameters
  • Added new test class PRProcessingTest with tests for extended patches
  • +65/-45 
    Documentation
    additional_configurations.md
    Update documentation for patch extension config                   

    docs/docs/usage-guide/additional_configurations.md

  • Updated configuration example to use separate patch_extra_lines_before
    and patch_extra_lines_after parameters
  • +2/-1     

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

    …s handling
    
    - Added unit tests in `test_extend_patch.py` and `test_pr_generate_extended_diff.py` to verify patch extension functionality with extra lines.
    - Updated `pr_processing.py` to include `patch_extra_lines_before` and `patch_extra_lines_after` settings.
    - Modified `configuration.toml` to adjust `patch_extra_lines_before` to 4 and `max_context_tokens` to 16000.
    - Enabled extra lines in `pr_code_suggestions.py`.
    - Added new model `claude-3-5-sonnet` to `__init__.py`.
    @codiumai-pr-agent-pro codiumai-pr-agent-pro bot added enhancement New feature or request Tests labels Aug 11, 2024
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🏅 Score: 85
    🧪 PR contains tests
    🔒 No security concerns identified
    🔀 Multiple PR themes

    Sub-PR theme: Implement separate parameters for extending patches before and after changes

    Relevant files:

    • pr_agent/algo/git_patch_processing.py
    • pr_agent/algo/pr_processing.py
    • tests/unittest/test_extend_patch.py

    Sub-PR theme: Update configuration and increase token limits for PR code suggestions

    Relevant files:

    • pr_agent/algo/init.py
    • pr_agent/tools/pr_code_suggestions.py
    • pr_agent/settings/configuration.toml

    ⚡ Key issues to review

    Code Refactoring
    The extend_patch function has been significantly modified to support separate parameters for extending patches before and after changes. This change might affect other parts of the codebase that rely on this function.

    Configuration Update
    The PR processing logic has been updated to use separate parameters for extending patches before and after changes. Ensure that this change is consistently applied throughout the codebase.

    - Renamed test class to `TestExtendedPatchMoreLines` in `test_extend_patch.py`
    - Imported `pr_generate_extended_diff` in `test_extend_patch.py`
    - Updated `patch_extra_lines_before` to 4 in `additional_configurations.md`
    @mrT23
    Copy link
    Collaborator Author

    mrT23 commented Aug 11, 2024

    /improve

    @Codium-ai Codium-ai deleted a comment from codiumai-pr-agent-pro bot Aug 11, 2024
    @mrT23
    Copy link
    Collaborator Author

    mrT23 commented Aug 11, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Add error handling and logging to the model setting function

    The set_claude_model() function modifies global settings. Consider adding a warning
    log or raising an exception if the model is not available or if there are any issues
    setting it.

    pr_agent/git_providers/utils.py [56-63]

     def set_claude_model():
         """
         set the claude-sonnet-3.5 model easily (even by users), just by stating: --config.model='claude-3-5-sonnet'
         """
         model_claude = "bedrock/anthropic.claude-3-5-sonnet-20240620-v1:0"
    -    get_settings().set('config.model', model_claude)
    -    get_settings().set('config.model_turbo', model_claude)
    -    get_settings().set('config.fallback_models', [model_claude])
    +    try:
    +        get_settings().set('config.model', model_claude)
    +        get_settings().set('config.model_turbo', model_claude)
    +        get_settings().set('config.fallback_models', [model_claude])
    +        get_logger().info(f"Successfully set Claude model to {model_claude}")
    +    except Exception as e:
    +        get_logger().warning(f"Failed to set Claude model: {e}")
     
    Suggestion importance[1-10]: 7

    Why: This suggestion adds important error handling and logging, improving the robustness of the function.

    7
    Performance
    Evaluate the need for extra lines in code suggestions to optimize performance

    The disable_extra_lines parameter is set to False, which means extra lines will be
    included. This might increase the token count and potentially affect performance.
    Consider evaluating if this is the intended behavior for code suggestions.

    pr_agent/tools/pr_code_suggestions.py [285-289]

     self.patches_diff = get_pr_diff(self.git_provider,
                                      self.token_handler,
                                      model,
                                      add_line_numbers_to_hunks=True,
    -                                disable_extra_lines=False)
    +                                disable_extra_lines=True)  # Consider disabling extra lines for code suggestions
     
    Suggestion importance[1-10]: 5

    Why: The suggestion raises a valid point about performance, but the optimal setting depends on the specific use case.

    5
    Best practice
    Use a more descriptive variable name for the caught exception

    Consider using a more descriptive variable name instead of e in the exception
    handling. This improves code readability and makes it easier to understand the
    nature of the exception being caught.

    pr_agent/algo/git_patch_processing.py [63-65]

    -except Exception as e:
    +except Exception as error:
         if get_settings().config.verbosity_level >= 2:
    -         get_logger().error(f"Failed to extend patch: {e}")
    +         get_logger().error(f"Failed to extend patch: {error}")
     
    Suggestion importance[1-10]: 3

    Why: The suggestion is correct but minor, improving code readability slightly.

    3

    @mrT23 mrT23 merged commit 2525392 into main Aug 11, 2024
    2 checks passed
    @mrT23 mrT23 deleted the tr/patch_extra_lines_before_and_after branch August 11, 2024 11:04
    @mrT23 mrT23 changed the title Tr/patch extra lines before and after Asymmetric context Aug 12, 2024
    @hussam789 hussam789 restored the tr/patch_extra_lines_before_and_after branch August 26, 2024 13:14
    @mrT23 mrT23 deleted the tr/patch_extra_lines_before_and_after branch September 16, 2024 06:04
    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.

    3 participants