Merge pull request #581 from srtab/feat/quick-actions-refactor

Refactored quick actions to be more like commands.

Author: srtab

.coveragerc

@@ -26,3 +26,4 @@ exclude_lines =
     # Don't complain if tests don't hit defensive assertion code:
     raise AssertionError
     raise NotImplementedError
+    if TYPE_CHECKING:

CHANGELOG.md

@@ -40,6 +40,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Refactored repository configuration file schema to be more flexible and easier to use. **BREAKING CHANGE**
 - Moved tools from `daiv/automation/tools` to `daiv/automation/agents/tools`.
 - Moved quick actions from `daiv/automation/quick_actions` to `daiv/quick_actions`.
+- Migrated quick action `help` to activate as `@daiv /help` instead of `@daiv help`. **BREAKING CHANGE**
+- Migrated quick action `plan execute` to activate as `@daiv /approve-plan` instead of `@daiv plan execute`. **BREAKING CHANGE**
+- Migrated quick action `plan revise` to activate as `@daiv /revise-plan` instead of `@daiv plan revise`. **BREAKING CHANGE**
 - Updated project dependencies.
 - Updated documentation.
 

daiv/automation/agents/review_addressor/prompts.py

@@ -96,7 +96,7 @@
 ### Step 2 • Final reply shown to the reviewer
 Immediately emit plain text (no phases, no tool names):
 - First-person voice ("I suggest…", "I noticed…").
-- Match the reviewer’s language only if detection ≥80% confidence; otherwise use English.
+- Match the reviewer's language only if detection ≥80% confidence; otherwise use English.
 - Be technically precise; reference code generically or link to exact lines (e.g., [`src/module/file.ts#L120-L135`](src/module/file.ts#L120-L135)).
 - Keep it concise yet complete; include a tiny snippet (≤ 8 lines) **only if it materially clarifies or shows the fix**.
 - If static analysis is insufficient, say so briefly and propose one minimal next step (optionally append **`(confidence: low/med/high)`**).

daiv/codebase/base.py

@@ -199,7 +199,6 @@ class Discussion(BaseModel):
     id: str
     resolve_id: str | None = None  # The id of the comment to resolve, only used for GitHub.
     notes: list[Note] = Field(default_factory=list)
-    is_reply: bool = False
     is_thread: bool = False
     is_resolvable: bool = False
 

daiv/codebase/clients/base.py

@@ -114,21 +114,23 @@ def get_issue(self, repo_id: str, issue_id: int) -> Issue:
         pass
 
     @abc.abstractmethod
-    def create_issue_comment(self, repo_id: str, issue_id: int, body: str) -> str | None:
+    def create_issue_comment(
+        self, repo_id: str, issue_id: int, body: str, reply_to_id: str | None = None, as_thread: bool = False
+    ) -> str | None:
         pass
 
     @abc.abstractmethod
-    def update_issue_comment(self, repo_id: str, issue_id: int, comment_id: int, body: str) -> str | None:
+    def update_issue_comment(
+        self, repo_id: str, issue_id: int, comment_id: int, body: str, reply_to_id: str | None = None
+    ) -> str | None:
         pass
 
     @abc.abstractmethod
     def create_issue_note_emoji(self, repo_id: str, issue_id: int, emoji: Emoji, note_id: str):
         pass
 
     @abc.abstractmethod
-    def get_issue_discussion(
-        self, repo_id: str, issue_id: int, discussion_id: str, only_resolvable: bool = True
-    ) -> Discussion:
+    def get_issue_comment(self, repo_id: str, issue_id: int, comment_id: str) -> Discussion:
         pass
 
     @abc.abstractmethod
@@ -137,16 +139,6 @@ def get_issue_related_merge_requests(
     ) -> list[MergeRequest]:
         pass
 
-    @abc.abstractmethod
-    def create_issue_discussion_note(
-        self, repo_id: str, issue_id: int, body: str, discussion_id: str | None = None
-    ) -> str | None:
-        pass
-
-    @abc.abstractmethod
-    def update_issue_discussion_note(self, repo_id: str, issue_id: int, discussion_id: str, note_id: str, body: str):
-        pass
-
     @abc.abstractmethod
     @cached_property
     def current_user(self) -> User:
@@ -161,17 +153,15 @@ def get_merge_request_latest_pipelines(self, repo_id: str, merge_request_id: int
         pass
 
     @abc.abstractmethod
-    def get_merge_request_discussion(
-        self, repo_id: str, merge_request_id: int, discussion_id: str, only_resolvable: bool = True
-    ) -> Discussion:
+    def get_merge_request_review_comments(self, repo_id: str, merge_request_id: int) -> list[Discussion]:
         pass
 
     @abc.abstractmethod
-    def get_merge_request_review_comments(self, repo_id: str, merge_request_id: int) -> list[Discussion]:
+    def get_merge_request_comments(self, repo_id: str, merge_request_id: int) -> list[Discussion]:
         pass
 
     @abc.abstractmethod
-    def get_merge_request_comments(self, repo_id: str, merge_request_id: int) -> list[Discussion]:
+    def get_merge_request_comment(self, repo_id: str, merge_request_id: int, comment_id: str) -> Discussion:
         pass
 
     @abc.abstractmethod

daiv/codebase/clients/github/api/callbacks.py

@@ -6,7 +6,7 @@
 from quick_actions.base import Scope
 from quick_actions.parser import QuickActionCommand, parse_quick_action
 from quick_actions.registry import quick_action_registry
-from quick_actions.tasks import execute_quick_action_task
+from quick_actions.tasks import execute_issue_task, execute_merge_request_task
 
 from codebase.api.callbacks import BaseCallback
 from codebase.clients import RepoClient
@@ -100,8 +100,6 @@ def accept_callback(self) -> bool:
     async def process_callback(self):
         """
         Trigger the task to address the review feedback or issue comment like the plan approval use case.
-
-        GitLab Note Webhook is called multiple times, one per note/discussion.
         """
         if self._is_quick_action:
             logger.info("Found quick action in note: '%s'", self._quick_action_command.raw)
@@ -110,18 +108,26 @@ async def process_callback(self):
                 self.repository.full_name, self.issue.number, Emoji.THUMBSUP, self.comment.id
             )
 
-            await sync_to_async(
-                execute_quick_action_task.si(
-                    repo_id=self.repository.full_name,
-                    discussion_id="",  # GitHub doesn't have discussions like GitLab.
-                    note_id=self.comment.id,
-                    issue_id=self.issue.number,
-                    merge_request_id=None,
-                    action_verb=self._quick_action_command.verb,
-                    action_args=" ".join(self._quick_action_command.args),
-                    action_scope=Scope.ISSUE,
-                ).delay
-            )()
+            if self._action_scope == Scope.ISSUE:
+                await sync_to_async(
+                    execute_issue_task.si(
+                        repo_id=self.repository.full_name,
+                        comment_id=self.comment.id,
+                        action_command=self._quick_action_command.command,
+                        action_args=" ".join(self._quick_action_command.args),
+                        issue_id=self.issue.number,
+                    ).delay
+                )()
+            elif self._action_scope == Scope.MERGE_REQUEST:
+                await sync_to_async(
+                    execute_merge_request_task.si(
+                        repo_id=self.repository.full_name,
+                        comment_id=self.comment.id,
+                        action_command=self._quick_action_command.command,
+                        action_args=" ".join(self._quick_action_command.args),
+                        merge_request_id=self.issue.number,
+                    ).delay
+                )()
 
         elif self._is_merge_request_review:
             # The webhook doesn't provide the source branch, so we need to fetch it from the merge request.
@@ -168,25 +174,36 @@ def _quick_action_command(self) -> QuickActionCommand | None:
         if not quick_action_command:
             return None
 
-        action_classes = quick_action_registry.get_actions(verb=quick_action_command.verb, scope=Scope.ISSUE)
+        action_classes = quick_action_registry.get_actions(
+            command=quick_action_command.command, scope=self._action_scope
+        )
 
         if not action_classes:
             logger.warning(
-                "Quick action '%s' not found in registry for scope '%s'", quick_action_command.verb, Scope.ISSUE
+                "Quick action '%s' not found in registry for scope '%s'",
+                quick_action_command.command,
+                self._action_scope,
             )
             return None
 
         if len(action_classes) > 1:
             logger.warning(
                 "Multiple quick actions found for '%s' in registry for scope '%s': %s",
-                quick_action_command.verb,
-                Scope.ISSUE,
-                [a.verb for a in action_classes],
+                quick_action_command.command,
+                self._action_scope,
+                [a.command for a in action_classes],
             )
             return None
 
         return quick_action_command
 
+    @property
+    def _action_scope(self) -> Scope:
+        """
+        Get the scope of the quick action.
+        """
+        return Scope.MERGE_REQUEST if self.issue.is_pull_request() else Scope.ISSUE
+
 
 class PullRequestReviewCallback(GitHubCallback):
     """

daiv/codebase/clients/github/client.py

@@ -199,21 +199,6 @@ def get_issue(self, repo_id: str, issue_id: int) -> Issue:
             labels=[label.name for label in issue.labels],
         )
 
-    def create_issue_comment(self, repo_id: str, issue_id: int, body: str) -> str | None:
-        """
-        Comment on an issue.
-
-        Returns:
-            The comment ID.
-        """
-        return self.client.get_repo(repo_id, lazy=True).get_issue(issue_id).create_comment(body).id
-
-    def update_issue_comment(self, repo_id: str, issue_id: int, comment_id: int, body: str):
-        """
-        Update a comment on an issue.
-        """
-        self.client.get_repo(repo_id, lazy=True).get_issue(issue_id).get_comment(comment_id).edit(body)
-
     def create_issue_note_emoji(self, repo_id: str, issue_id: int, emoji: Emoji, note_id: str):
         """
         Create an emoji in a note of an issue.
@@ -225,56 +210,56 @@ def create_issue_note_emoji(self, repo_id: str, issue_id: int, emoji: Emoji, not
             emoji_reaction
         )
 
-    def get_issue_discussion(
-        self, repo_id: str, issue_id: int, discussion_id: str, only_resolvable: bool = True
-    ) -> Discussion:
+    def get_issue_comment(self, repo_id: str, issue_id: int, comment_id: str) -> Discussion:
         """
-        Get a discussion from an issue.
+        Get a comment from an issue.
 
-        For GitHub, there's no distinction between discussions and notes.
+        For GitHub, there's no distinction between comments and notes.
 
         Args:
             repo_id: The repository ID.
             issue_id: The issue ID.
-            discussion_id: The discussion ID. This is not used for GitHub.
-            only_resolvable: Whether to only return resolvable notes. This is not used for GitHub.
+            comment_id: The comment ID.
 
         Returns:
             The discussion object.
         """
         issue = self.client.get_repo(repo_id, lazy=True).get_issue(issue_id)
         # GitHub doesn't have discussions like GitLab. This is a workaround to get the notes of an issue.
-        return Discussion(id=discussion_id, notes=self._serialize_comments(issue.get_comments()), is_reply=False)
+        return Discussion(id=comment_id, notes=self._serialize_comments(issue.get_comments()))
 
-    def create_issue_discussion_note(
-        self, repo_id: str, issue_id: int, body: str, discussion_id: str | None = None
+    def create_issue_comment(
+        self, repo_id: str, issue_id: int, body: str, reply_to_id: str | None = None, as_thread: bool = False
     ) -> str | None:
         """
-        Create a comment on an issue.
-
-        For GitHub, there's no distinction between discussions and notes. This method creates a comment on the issue.
+        Comment on an issue.
 
         Args:
             repo_id: The repository ID.
             issue_id: The issue ID.
             body: The comment body.
-            discussion_id: The discussion ID. This is not used for GitHub.
+            reply_to_id: The ID of the comment to reply to. This is not supported for GitHub.
+            as_thread: Whether to create a thread. This is not supported for GitHub.
+
+        Returns:
+            The comment ID.
         """
-        # GitHub doesn't have discussions like GitLab. This is a workaround to create a comment on the issue.
-        return self.create_issue_comment(repo_id, issue_id, body)
+        return self.client.get_repo(repo_id, lazy=True).get_issue(issue_id).create_comment(body).id
 
-    def update_issue_discussion_note(self, repo_id: str, issue_id: int, discussion_id: str, note_id: str, body: str):
+    def update_issue_comment(
+        self, repo_id: str, issue_id: int, comment_id: int, body: str, reply_to_id: str | None = None
+    ):
         """
         Update a comment on an issue.
 
         Args:
             repo_id: The repository ID.
             issue_id: The issue ID.
-            discussion_id: The discussion ID. This is not used for GitHub.
-            note_id: The note ID.
+            comment_id: The comment ID.
             body: The comment body.
+            reply_to_id: The ID of the comment to reply to. This is not supported for GitHub.
         """
-        self.update_issue_comment(repo_id, issue_id, note_id, body)
+        self.client.get_repo(repo_id, lazy=True).get_issue(issue_id).get_comment(comment_id).edit(body)
 
     @cached_property
     def current_user(self) -> User:
@@ -557,17 +542,14 @@ def get_merge_request_diff(self, repo_id: str, merge_request_id: int) -> PatchSe
         headers, data = self.client.requester.requestJsonAndCheck("GET", pr.diff_url, follow_302_redirect=True)
         return PatchSet.from_string(data["data"])
 
-    def get_merge_request_discussion(
-        self, repo_id: str, merge_request_id: int, discussion_id: str, only_resolvable: bool = True
-    ) -> Discussion:
+    def get_merge_request_comment(self, repo_id: str, merge_request_id: int, comment_id: str) -> Discussion:
         """
-        Get a discussion from a merge request.
+        Get a comment from a merge request.
 
         Args:
             repo_id: The repository ID.
             merge_request_id: The merge request ID.
-            discussion_id: The discussion ID.
-            only_resolvable: Whether to only return resolvable notes (review comments).
+            comment_id: The comment ID.
 
         Returns:
             The discussion object.
@@ -576,22 +558,14 @@ def get_merge_request_discussion(
 
         comment = None
         try:
-            comment = pr.get_issue_comment(discussion_id)
+            comment = pr.get_issue_comment(comment_id)
         except UnknownObjectException:
-            comment = pr.get_review_comment(discussion_id)
-
-        if only_resolvable:
-            if isinstance(comment, IssueComment) and comment.reactions.get("rocket", 0) > 0:
-                comment = None
-            elif isinstance(comment, PullRequestComment):
-                unresolved_comment_ids, _ = self._unresolved_comment_ids(repo_id, merge_request_id)
-                if comment.id not in unresolved_comment_ids:
-                    comment = None
+            comment = pr.get_review_comment(comment_id)
 
         if comment is None:
-            return Discussion(id=discussion_id, notes=[])
+            return Discussion(id=comment_id, notes=[])
 
-        return Discussion(id=discussion_id, notes=self._serialize_comments([comment], from_merge_request=True))
+        return Discussion(id=comment_id, notes=self._serialize_comments([comment], from_merge_request=True))
 
     def get_merge_request_review_comments(self, repo_id: str, merge_request_id: int) -> list[Discussion]:
         """