From 23cb096cd76097760a90699fcad107dcde666712 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 08:14:17 +0000 Subject: [PATCH] fix: resolve issue #290 --- bert_e/exceptions.py | 7 ++ bert_e/git_host/base.py | 7 ++ bert_e/git_host/github/__init__.py | 5 ++ bert_e/git_host/mock.py | 3 + bert_e/tests/unit/test_comment_update.py | 89 ++++++++++++++++++++++++ bert_e/workflow/pr_utils.py | 34 ++++++++- 6 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 bert_e/tests/unit/test_comment_update.py diff --git a/bert_e/exceptions.py b/bert_e/exceptions.py index f9654dad..afb0d1d5 100644 --- a/bert_e/exceptions.py +++ b/bert_e/exceptions.py @@ -31,6 +31,9 @@ class TemplateException(BertE_Exception): template = None # whether to re-publish if the message is already in the history dont_repeat_if_in_history = -1 + # whether the message is a status that supersedes the previous one: it + # then updates the existing status comment instead of posting a new one + updatable = False def __init__(self, **kwargs): self.kwargs = kwargs @@ -59,6 +62,7 @@ class SilentException(BertE_Exception): # template for informative exceptions class InformationException(TemplateException): dont_repeat_if_in_history = NEVER_REPEAT + updatable = True # template exceptions @@ -131,12 +135,14 @@ class IncorrectFixVersion(TemplateException): code = 112 template = 'incorrect_fix_version.md' status = "failure" + updatable = True class BranchHistoryMismatch(TemplateException): code = 113 template = 'history_mismatch.md' status = "failure" + updatable = True class Conflict(TemplateException): @@ -226,6 +232,7 @@ class QueueOutOfOrder(TemplateException): class ResetComplete(TemplateException): code = 128 template = "reset_complete.md" + updatable = True class LossyResetWarning(TemplateException): diff --git a/bert_e/git_host/base.py b/bert_e/git_host/base.py index a38ab926..424dbb2d 100644 --- a/bert_e/git_host/base.py +++ b/bert_e/git_host/base.py @@ -277,6 +277,13 @@ def text(self) -> str: def id(self) -> int: """The comment's ID""" + def edit(self, msg: str) -> None: + """Replace the comment's contents with `msg`. + + Git hosts that cannot edit comments raise NotImplementedError. + """ + raise NotImplementedError('Comment edition is not supported.') + class AbstractPullRequest(metaclass=ABCMeta): @abstractmethod diff --git a/bert_e/git_host/github/__init__.py b/bert_e/git_host/github/__init__.py index 59515b1c..aa3c4dc7 100644 --- a/bert_e/git_host/github/__init__.py +++ b/bert_e/git_host/github/__init__.py @@ -1018,6 +1018,11 @@ def id(self) -> int: def delete(self) -> None: self.client.delete(self.data['url']) + def edit(self, msg: str) -> None: + self.data = self.update( + self.client, {'body': msg}, url=self.data['url'] + ).data + class CheckRun(base.AbstractGitHostObject): GET_URL = '/repos/{owner}/{repo}/check-runs/{id}' diff --git a/bert_e/git_host/mock.py b/bert_e/git_host/mock.py index 02138b94..d635cb71 100644 --- a/bert_e/git_host/mock.py +++ b/bert_e/git_host/mock.py @@ -409,6 +409,9 @@ class CommentController(Controller, base.AbstractComment): def delete(self): self.controlled.delete() + def edit(self, msg): + self['content'] = {"raw": msg, "markup": "markdown", "html": msg} + @property def author(self): return self['user']['username'].lower() diff --git a/bert_e/tests/unit/test_comment_update.py b/bert_e/tests/unit/test_comment_update.py new file mode 100644 index 00000000..e1225ca6 --- /dev/null +++ b/bert_e/tests/unit/test_comment_update.py @@ -0,0 +1,89 @@ +"""Status comments are edited in place instead of being re-posted.""" +from types import SimpleNamespace + +import pytest + +from bert_e import exceptions +from bert_e.workflow import pr_utils + + +class FakeComment: + def __init__(self, author, text, editable=True): + self.author = author + self.text = text + self.editable = editable + + def edit(self, msg): + if not self.editable: + raise NotImplementedError + self.text = msg + + +class FakePR: + def __init__(self, editable=True): + self.comments = [] + self.editable = editable + + def add_comment(self, msg): + self.comments.append(FakeComment('bert-e', msg, self.editable)) + + +@pytest.fixture +def settings(): + return SimpleNamespace(no_comment=False, interactive=False, + robot='bert-e') + + +def _send(settings, pr, msg, exc_cls): + pr_utils._send_comment(settings, pr, msg, + exc_cls.dont_repeat_if_in_history, + updatable=exc_cls.updatable) + + +def test_updatable_classes(): + assert exceptions.ResetComplete.updatable + assert exceptions.BranchHistoryMismatch.updatable + assert exceptions.IncorrectFixVersion.updatable + assert exceptions.IntegrationDataCreated.updatable + assert not exceptions.HelpMessage.updatable + assert not exceptions.StatusReport.updatable + + +def test_status_comment_is_updated(settings): + pr = FakePR() + _send(settings, pr, 'reset', exceptions.ResetComplete) + _send(settings, pr, 'history mismatch', exceptions.BranchHistoryMismatch) + assert len(pr.comments) == 1 + assert pr.comments[0].text.startswith('history mismatch') + assert pr.comments[0].text.endswith(pr_utils.STATUS_MARKER) + + +def test_identical_status_not_reposted(settings): + pr = FakePR() + _send(settings, pr, 'reset', exceptions.ResetComplete) + with pytest.raises(exceptions.CommentAlreadyExists): + _send(settings, pr, 'reset', exceptions.ResetComplete) + assert len(pr.comments) == 1 + + +def test_non_updatable_message_is_posted(settings): + pr = FakePR() + _send(settings, pr, 'reset', exceptions.ResetComplete) + _send(settings, pr, 'help', exceptions.HelpMessage) + assert len(pr.comments) == 2 + assert pr_utils.STATUS_MARKER not in pr.comments[1].text + + +def test_fallback_when_edit_unsupported(settings): + pr = FakePR(editable=False) + _send(settings, pr, 'reset', exceptions.ResetComplete) + _send(settings, pr, 'other', exceptions.BranchHistoryMismatch) + assert len(pr.comments) == 2 + + +def test_other_authors_comments_untouched(settings): + pr = FakePR() + pr.comments.append(FakeComment('someone', 'hi ' + pr_utils.STATUS_MARKER)) + _send(settings, pr, 'reset', exceptions.ResetComplete) + assert len(pr.comments) == 2 + assert pr.comments[0].text.startswith('hi') diff --git a/bert_e/workflow/pr_utils.py b/bert_e/workflow/pr_utils.py index a09f3991..4ff89e9d 100644 --- a/bert_e/workflow/pr_utils.py +++ b/bert_e/workflow/pr_utils.py @@ -21,6 +21,9 @@ LOG = logging.getLogger(__name__) +# Hidden marker identifying the comment holding the bot's latest status. +STATUS_MARKER = '' + def find_comment(pull_request: AbstractPullRequest, username=None, startswith=None, max_history=None) -> AbstractComment: @@ -50,8 +53,17 @@ def find_comment(pull_request: AbstractPullRequest, username=None, return comment +def find_status_comment(pull_request: AbstractPullRequest, + username) -> AbstractComment: + """Return the latest status comment posted by the bot, if any.""" + for comment in reversed(pull_request.comments): + if comment.author == username and comment.text.rstrip().endswith( + STATUS_MARKER): + return comment + + def _send_comment(settings, pull_request: AbstractPullRequest, msg: str, - dont_repeat_if_in_history=10) -> None: + dont_repeat_if_in_history=10, updatable=False) -> None: """Comment a pull request. Before posting: @@ -59,6 +71,8 @@ def _send_comment(settings, pull_request: AbstractPullRequest, msg: str, request comments history. Optionally (if settings.interactive is set) ask confirmation to the user. + If `updatable` is set, edit the bot's previous status comment in place + (when there is one) instead of posting a new comment. Raises: CommentAlreadyExists: if the comment was already posted. @@ -80,6 +94,21 @@ def _send_comment(settings, pull_request: AbstractPullRequest, msg: str, if not confirm('Do you want to send this comment?'): return + if updatable: + status_comment = find_status_comment(pull_request, settings.robot) + msg = f'{msg}\n\n{STATUS_MARKER}' + if status_comment is not None: + if status_comment.text.strip() == msg.strip(): + raise exceptions.CommentAlreadyExists( + "The status comment is already up to date." + ) + try: + LOG.debug('UPDATING STATUS COMMENT %s', msg) + status_comment.edit(msg) + return + except NotImplementedError: + LOG.debug('Comments cannot be edited, posting a new one.') + LOG.debug('SENDING MESSAGE %s', msg) pull_request.add_comment(msg) @@ -104,6 +133,7 @@ def notify_user(settings, pull_request: AbstractPullRequest, try: _send_bot_status(settings, pull_request, comment) _send_comment(settings, pull_request, str(comment), - comment.dont_repeat_if_in_history) + comment.dont_repeat_if_in_history, + updatable=comment.updatable) except exceptions.CommentAlreadyExists: LOG.info("Comment '%s' already posted", comment.__class__.__name__)