update template asynch #2

mschoi · 2025-01-27T00:08:52+09:00

mschoi commented

2025-01-27 00:08:52 +09:00

Summary (요약)

Fill me

Describe your changes (주요 변화)

Fill me

Issue number and link (관련 이슈)

Fill me

PR Type

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

To Reveiwer

리뷰어에게 하고싶은 메세지

Reference

N/A

## Summary (요약) - Fill me ## Describe your changes (주요 변화) - Fill me ## Issue number and link (관련 이슈) - Fill me ## PR Type  - [ ] Bugfix - [ ] Feature - [ ] Code style update (formatting, local variables) - [ ] Refactoring (no functional changes, no api changes) - [ ] Build related changes - [ ] CI related changes - [ ] Documentation content changes - [ ] angular.io application / infrastructure changes - [ ] Other... Please describe: ## To Reveiwer - 리뷰어에게 하고싶은 메세지 ## Reference - N/A

mschoi added 1 commit 2025-01-27 00:08:53 +09:00

update template asynch

Code Review / review (pull_request) Failing after 21s

Details

89dc1efda7

mschoi added 2 commits 2025-01-27 00:15:19 +09:00

update aynch client 37b9320798

change test model

Code Review / review (pull_request) Successful in 23s

Details

d82ff13439

mschoi reviewed 2025-01-27 00:15:41 +09:00

mschoi left a comment

Code Structure & Architecture

The code structure is generally modular, but there are opportunities to improve readability and maintainability. Consider organizing the utility functions into a separate module or class to encapsulate related functionalities. This will help in managing the code better as it grows.

Refactoring Opportunities

The get_diff function currently prints errors directly to the console. Consider using a logging framework to handle error messages, which will provide more flexibility in managing output and error levels.
The parse_diff function is quite complex and could benefit from breaking down into smaller helper functions. For example, extracting the logic for parsing file patterns and hunks into separate functions could make the code easier to understand and maintain.

Potential Future Problems

The current implementation relies heavily on environment variables for configuration. Consider using a configuration file or a configuration management library to handle these settings. This approach will make it easier to manage configurations across different environments and reduce the risk of missing or incorrect environment variables.
The use of aiohttp for asynchronous HTTP requests is appropriate, but ensure that the session is properly closed after use to prevent resource leaks. Consider using context managers or explicitly closing the session after requests are completed.
The Model class uses multiple third-party libraries for AI model interactions. Ensure that these dependencies are well-documented and version-controlled to prevent compatibility issues in the future. It might also be beneficial to abstract these interactions further to allow for easier swapping of AI providers if needed.

### Code Structure & Architecture - The code structure is generally modular, but there are opportunities to improve readability and maintainability. Consider organizing the utility functions into a separate module or class to encapsulate related functionalities. This will help in managing the code better as it grows. ### Refactoring Opportunities - The `get_diff` function currently prints errors directly to the console. Consider using a logging framework to handle error messages, which will provide more flexibility in managing output and error levels. - The `parse_diff` function is quite complex and could benefit from breaking down into smaller helper functions. For example, extracting the logic for parsing file patterns and hunks into separate functions could make the code easier to understand and maintain. ### Potential Future Problems - The current implementation relies heavily on environment variables for configuration. Consider using a configuration file or a configuration management library to handle these settings. This approach will make it easier to manage configurations across different environments and reduce the risk of missing or incorrect environment variables. - The use of `aiohttp` for asynchronous HTTP requests is appropriate, but ensure that the session is properly closed after use to prevent resource leaks. Consider using context managers or explicitly closing the session after requests are completed. - The `Model` class uses multiple third-party libraries for AI model interactions. Ensure that these dependencies are well-documented and version-controlled to prevent compatibility issues in the future. It might also be beneficial to abstract these interactions further to allow for easier swapping of AI providers if needed.

@@ -1,5 +1,6 @@
 """Code Reviewer for Gitea."""
 import asyncio

@@ -2,3 +2,4 @@
 import asyncio
 import fnmatch
 import json

@@ -8,2 +7,3 @@
 from openai import OpenAI
 from anthropic import AsyncAnthropic
 from openai import AsyncOpenAI

@@ -7,6 +8,7 @@ import re
 from typing import Any
 import requests
 import aiohttp

				`@@ -4,8 +4,16 @@ from enum import Enum`
				`from typing import Any`

@@ -4,8 +4,16 @@ from enum import Enum
 from typing import Any
 import google.generativeai as genai

@@ -9,0 +11,4 @@
 class GoogleReponse(typing.TypedDict):
     """The response from Google model."""

@@ -9,0 +13,4 @@
     """The response from Google model."""
     lineNumber: int
     reviewComment: str

@@ -9,3 +16,4 @@
     reviewComment: str
 class ModelProvider(Enum):

@@ -60,3 +62,3 @@
     hunk_pattern = re.compile(
         r"^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@(.*?)(?=^@@ |$)",
         r"@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@(.*?)?(?=@@ -\d+(?:,\d+)? \+\d+(?:,\d+)? @@|\Z)",
         re.MULTILINE | re.DOTALL,

@@ -101,2 +84,4 @@
             continue
         output_diff_text = []
         for hunk_match in hunk_pattern.finditer(diff_text):

@@ -103,0 +97,4 @@
                     new_idx += 1
                 else:
                     output_diff_text.append(f"{old_idx} {new_idx} {line}")
                     old_idx += 1

@@ -135,3 +136,3 @@
 def analyze_single_chunks(
 async def analyze_single_chunks(
     single_chunk_model: Model, parsed_diff: list[dict[str, Any]]

@@ -153,3 +152,3 @@
         chunk = diff["chunk"]
         response = single_chunk_model.get_response_single_chunk(
         response = await single_chunk_model.get_response_single_chunk(
             file, title, description, chunk

@@ -167,0 +165,4 @@
     title = EVENT_DATA["pull_request"]["title"]
     description = EVENT_DATA["pull_request"]["body"]
     tasks = [process_single_chunk(diff) for diff in parsed_diff]
     results = await asyncio.gather(*tasks)

@@ -169,3 +174,3 @@
 def get_file_content(file: str) -> str | None:
 async def get_file_content(file: str) -> str | None:
     """Get file content from Gitea.

@@ -190,2 +191,2 @@
         print(f"Failed to get file content: {e}")
         return None
         async with aiohttp.ClientSession(headers=HEADERS) as session:
             async with session.get(url) as response:

@@ -193,3 +201,3 @@
 def analyze_full_context(
 async def analyze_full_context(
     full_context_model: Model, parsed_diff: list[dict[str, Any]]

@@ -216,0 +221,4 @@
         return f"File: {file}\n{content}\nDiff: {chunk}"
     tasks = [get_file_data(diff) for diff in parsed_diff]
     file_contents_list = await asyncio.gather(*tasks)

@@ -251,2 +263,2 @@
 def main() -> None:
     """Code Reviewer for Gitea."""
 async def main() -> None:
     """Code Reviewer for Gitea: Asynchronous version."""

@@ -273,3 +285,4 @@
     )
     print("diff: ", diff)
     parsed_diff = parse_diff(diff)

@@ -280,3 +302,3 @@
 if __name__ == "__main__":
     main()
     asyncio.run(main())

@@ -79,16 +87,18 @@ class Model:
         """
         match self.provider:

@@ -80,3 +88,3 @@
         match self.provider:
             case ModelProvider.OPENAI:
                 return OpenAI(api_key=api_key)
                 return AsyncOpenAI(api_key=api_key)

@@ -83,2 +90,3 @@
                 return AsyncOpenAI(api_key=api_key)
             case ModelProvider.ANTHROPIC:
                 return Anthropic(api_key=api_key)
                 return AsyncAnthropic(api_key=api_key)

@@ -91,3 +101,3 @@
     def request(self, prompt: str) -> str:
     async def request(self, prompt: str) -> str:
         """Request the model to generate a response.

@@ -132,3 +148,3 @@
                 return response.text.strip()
     def get_response_single_chunk(
     async def get_response_single_chunk(

@@ -151,3 +167,3 @@
     def get_response_full_context(
     async def get_response_full_context(
         self, title: str, description: str, file_contents: list[str]
     ) -> str:

@@ -179,1 +194,4 @@
     """[{{"lineNumber": int, "reviewComment": str}}] \n"""
     "- lineNumber is about the line number of the code that in new file. \n"
     "- lineNumber can be found at the front of each line. \n"
     "- At the first number is old line number, the second number is new line number. \n"

@@ -103,0 +88,4 @@
             old_idx = int(hunk_match.group(1))
             new_idx = int(hunk_match.group(3))
             remain_text = hunk_match.group(5).splitlines()
             for line in remain_text: