Skip to content

Commit

Permalink
Renamed ReviewAddressor to ReviewFixer
Browse files Browse the repository at this point in the history
  • Loading branch information
srtab committed Sep 1, 2024
1 parent cc0ae8c commit 68fe9e0
Show file tree
Hide file tree
Showing 13 changed files with 65 additions and 45 deletions.
5 changes: 1 addition & 4 deletions daiv/automation/agents/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,7 @@ def should_continue_iteration(self, single_iteration: bool = False) -> bool:
if self.iterations == 0:
return True

if single_iteration:
return False

if self.memory[-1].model_instance is not None:
if single_iteration or self.iterations >= self.max_iterations or self.memory[-1].model_instance is not None:
return False

if (
Expand Down
7 changes: 0 additions & 7 deletions daiv/automation/coders/review_addressor/tools.py

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,21 @@
from automation.agents.models import Message
from automation.coders.base import STOP_MESSAGE, CodebaseCoder
from automation.coders.tools import CodeActionTools, CodeInspectTools
from automation.coders.typings import ReviewAddressorInvoke
from automation.coders.typings import ReviewFixerInvoke
from codebase.base import FileChange

from .models import RequestFeedback
from .prompts import ReviewAddressorPrompts, ReviewCommentorPrompts
from .prompts import ReviewCommentorPrompts, ReviewFixerPrompts


class ReviewCommentorCoder(CodebaseCoder[ReviewAddressorInvoke, RequestFeedback | None]):
class ReviewCommentorCoder(CodebaseCoder[ReviewFixerInvoke, RequestFeedback | None]):
"""
Coder to review the comments left in a diff extracted from a pull request.
The coder will review the comments and ask for more information if needed.
"""

def invoke(self, *args, **kwargs: Unpack[ReviewAddressorInvoke]) -> RequestFeedback | None:
def invoke(self, *args, **kwargs: Unpack[ReviewFixerInvoke]) -> RequestFeedback | None:
"""
Invoke the coder to review the comments in the pull request.
"""
Expand Down Expand Up @@ -50,14 +50,14 @@ def invoke(self, *args, **kwargs: Unpack[ReviewAddressorInvoke]) -> RequestFeedb
return cast(RequestFeedback, response)


class ReviewAddressorCoder(CodebaseCoder[ReviewAddressorInvoke, list[FileChange]]):
class ReviewFixerCoder(CodebaseCoder[ReviewFixerInvoke, list[FileChange]]):
"""
Coder to address the review comments left on a pull request.
The coder will address the comments and make the necessary changes in the codebase.
"""

def invoke(self, *args, **kwargs: Unpack[ReviewAddressorInvoke]) -> list[FileChange]:
def invoke(self, *args, **kwargs: Unpack[ReviewFixerInvoke]) -> list[FileChange]:
"""
Invoke the coder to address the review comments in the codebase.
"""
Expand All @@ -69,9 +69,7 @@ def invoke(self, *args, **kwargs: Unpack[ReviewAddressorInvoke]) -> list[FileCha
ref=kwargs["source_ref"],
)

memory = [
Message(role="system", content=ReviewAddressorPrompts.format_system(kwargs["file_path"], kwargs["diff"]))
]
memory = [Message(role="system", content=ReviewFixerPrompts.format_system(kwargs["file_path"], kwargs["diff"]))]

for note in kwargs["notes"]:
# add previous notes to thread the conversation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@

class RequestFeedback(BaseModel):
"""
This tool is used to ask for feedback if you are unable to address the comments or the comments are ambiguous.
This tool is used to ask for feedback when less well-specified comments, where the user's requests are
vague or incomplete.
"""

questions: list[str] = Field(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,15 @@ def format_system(diff: str):
return textwrap.dedent(
"""\
### Instructions ###
Act as an exceptional senior software engineer that is responsible for addressing code review left on a pull request you worked on.
Act as an talented senior software engineer who is responsible for addressing a comment let of a pull request. Identify every single one of the user's requests let in this comment. Be complete. The changes should be atomic.
The user will interact with the comments left on the code review. The unified diff has been extracted from the file where the comments were made, and shows only the specific lines of code where they were made.
The unified diff below has been extracted from the file where the comments were made, and shows only the specific lines of code where they were made.
It's absolutely vital that you completely and correctly execute your task.
### Guidelines ###
- Before you start asking questions:
* Think out loud step-by-step;
* Use the supplied tools to obtain more detail about the codebase and to help you think about the problem and avoid asking unnecessary questions;
- Be straightforward on the context you need;
- Think out loud step-by-step, breaking down the problem and your approach;
- For less well-specified comments, where the user's requests are vague or incomplete, use the supplied tools to obtain more details about the codebase and help you infer the user's intent. If this is not enough, ask for it;
- Your task is completed when there's no feedback to request.
### Examples ###
Expand All @@ -38,12 +36,12 @@ def format_system(diff: str):
{diff}
### Task ###
Analyze and verify if there's clear what you need to change on the unified diff based on the user feedback. If the comments are ambiguous or off-topic (not related with this context), ask for more context about the intended changes.
Analyze the user's comment and codebase to understand if there's clear what you need to change on the unified diff and ask for more information if needed.
""" # noqa: E501
).format(diff=diff)


class ReviewAddressorPrompts:
class ReviewFixerPrompts:
@staticmethod
def format_system(file_path: str, diff: str = ""):
"""
Expand All @@ -65,7 +63,7 @@ def format_system(file_path: str, diff: str = ""):
return textwrap.dedent(
"""\
### Instructions ###
Act as a exceptional senior software engineer that is responsible for writing code to address code review left on a pull request you worked on.
Act as a exceptional senior software engineer that is responsible for writing code to fix code review left on a pull request you worked on.
Given the available tools and below task, which corresponds to an important step in writing code,
convert the task into code.
It's absolutely vital that you completely and correctly execute your task.
Expand All @@ -85,7 +83,7 @@ def format_system(file_path: str, diff: str = ""):
You must use the provided tools/functions to do so.
### Task ###
Address the requested code changes on file {file_path} based on the user feedback.
Apply the requested code changes on file {file_path} based on the user feedback.
{diff_task}
{diff}
Expand Down
4 changes: 2 additions & 2 deletions daiv/automation/coders/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def codebase_search(self, query: str) -> str:

search_results_str = "No search results found."

if search_results := self.codebase_index.search_with_reranker(self.repo_id, query):
if search_results := self.codebase_index.search_with_reranker(self.repo_id, query, k=5):
search_results_str = "### Search results ###"
for reranked_score, result in search_results:
logger.debug(
Expand All @@ -117,7 +117,7 @@ def codebase_search(self, query: str) -> str:
file path: {file_path}
```{language}
{content}
```\n\n
```
"""
).format(
file_path=result.document.metadata["source"],
Expand Down
2 changes: 1 addition & 1 deletion daiv/automation/coders/typings.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class CodebaseInvoke(Invoke):
source_ref: str


class ReviewAddressorInvoke(CodebaseInvoke):
class ReviewFixerInvoke(CodebaseInvoke):
file_path: str
notes: list[Note]
diff: str | None = None
Expand Down
19 changes: 13 additions & 6 deletions daiv/codebase/api/callbacks_gitlab.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,17 @@
from automation.coders.refactor.coder_simple import SimpleRefactorCoder
from automation.coders.refactor.prompts import RefactorPrompts
from codebase.api.callbacks import BaseCallback
from codebase.api.models import Issue, IssueAction, MergeRequest, Note, NoteableType, NoteAction, Project, User
from codebase.api.models import (
LABEL_DAIV,
Issue,
IssueAction,
MergeRequest,
Note,
NoteableType,
NoteAction,
Project,
User,
)
from codebase.clients import RepoClient
from codebase.tasks import handle_mr_feedback, update_index_repository

Expand All @@ -19,8 +29,6 @@

logger = logging.getLogger(__name__)

LABEL_DAIV = "daiv"


class IssueCallback(BaseCallback):
"""
Expand All @@ -33,10 +41,9 @@ class IssueCallback(BaseCallback):
object_attributes: Issue

def accept_callback(self) -> bool:
client = RepoClient.create_instance()
return (
self.object_attributes.action in [IssueAction.OPEN, IssueAction.UPDATE, IssueAction.REOPEN]
and client.current_user.id == self.object_attributes.assignee_id
and self.object_attributes.is_daiv()
)

async def process_callback(self):
Expand Down Expand Up @@ -181,7 +188,7 @@ def accept_callback(self) -> bool:
and self.merge_request
and not self.merge_request.work_in_progress
and self.merge_request.state == "opened"
and client.current_user.id == self.merge_request.assignee_id
and self.merge_request.is_daiv()
)

async def process_callback(self):
Expand Down
20 changes: 20 additions & 0 deletions daiv/codebase/api/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

from pydantic import BaseModel

LABEL_DAIV = "daiv"


class IssueAction(StrEnum):
"""
Expand All @@ -15,6 +17,10 @@ class IssueAction(StrEnum):
CLOSE = "close"


class Label(BaseModel):
title: str


class Issue(BaseModel):
"""
Gitlab Issue
Expand All @@ -27,6 +33,13 @@ class Issue(BaseModel):
state: str
assignee_id: int | None
action: IssueAction
labels: list[Label]

def is_daiv(self) -> bool:
"""
Check if the issue is a DAIV issue
"""
return any(label.title == LABEL_DAIV for label in self.labels)


class MergeRequest(BaseModel):
Expand All @@ -43,6 +56,13 @@ class MergeRequest(BaseModel):
source_branch: str
target_branch: str
assignee_id: int | None
labels: list[Label]

def is_daiv(self) -> bool:
"""
Check if the merge request is a DAIV merge request
"""
return any(label.title == LABEL_DAIV for label in self.labels)


class NoteableType(StrEnum):
Expand Down
7 changes: 5 additions & 2 deletions daiv/codebase/clients.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ def get_issue_notes(self, repo_id: str, issue_id: int) -> list[Note]:

@abc.abstractmethod
def get_issue_related_merge_requests(
self, repo_id: str, issue_id: int, assignee_id: int | None = None
self, repo_id: str, issue_id: int, assignee_id: int | None = None, label: str | None = None
) -> list[MergeRequest]:
pass

Expand Down Expand Up @@ -280,7 +280,9 @@ def set_repository_webhooks(
self,
repo_id: str,
url: str,
events: list[Literal["push_events", "merge_requests_events", "issues_events", "pipeline_events"]],
events: list[
Literal["push_events", "merge_requests_events", "issues_events", "pipeline_events", "note_events"]
],
push_events_branch_filter: str | None = None,
enable_ssl_verification: bool = True,
):
Expand All @@ -302,6 +304,7 @@ def set_repository_webhooks(
"merge_requests_events": "merge_requests_events" in events,
"issues_events": "issues_events" in events,
"pipeline_events": "pipeline_events" in events,
"note_events": "note_events" in events,
"enable_ssl_verification": enable_ssl_verification,
}
if push_events_branch_filter:
Expand Down
2 changes: 1 addition & 1 deletion daiv/codebase/management/commands/setup_webhooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def handle(self, *args, **options):
repo_client.set_repository_webhooks(
project.slug,
f"{options["base_url"]}/api/codebase/callbacks/{settings.CODEBASE_CLIENT}/",
["push_events", "merge_requests_events", "issues_events"],
["push_events", "issues_events", "note_events"],
push_events_branch_filter=project.default_branch,
enable_ssl_verification=not options["disable_ssl_verification"],
)
Expand Down
7 changes: 5 additions & 2 deletions daiv/codebase/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

from automation.agents.models import Usage
from automation.coders.change_describer.coder import ChangesDescriberCoder
from automation.coders.review_addressor.coder import ReviewAddressorCoder, ReviewCommentorCoder
from automation.coders.review_fixer.coder import ReviewCommentorCoder, ReviewFixerCoder
from codebase.base import Discussion, FileChange, Note, NoteDiffPositionType, NotePositionType, NoteType
from codebase.clients import RepoClient
from codebase.indexes import CodebaseIndex
Expand Down Expand Up @@ -205,6 +205,9 @@ def _handle_diff_notes(usage: Usage, client: RepoClient, discussion_to_address:
diff=discussion_to_address.diff,
)

if not feedback:
return

if feedback.questions:
client.create_merge_request_discussion_note(
discussion_to_address.repo_id,
Expand All @@ -218,7 +221,7 @@ def _handle_diff_notes(usage: Usage, client: RepoClient, discussion_to_address:
file_changes: list[FileChange] = []

if feedback.code_changes_needed:
file_changes = ReviewAddressorCoder(usage=usage).invoke(
file_changes = ReviewFixerCoder(usage=usage).invoke(
source_repo_id=discussion_to_address.repo_id,
source_ref=discussion_to_address.merge_request_source_branch,
file_path=discussion_to_address.patch_file.path,
Expand Down

0 comments on commit 68fe9e0

Please sign in to comment.