Skip to content

get_phabricator_revision drops inline comments that have no text (suggestion-only) #6699

Description

@daogottwald

What happens

get_phabricator_revision (and the phabricator://revision/D{id} resource) omits inline review comments whose comment text is empty. The comment does not render as blank -- it is absent entirely, with no header, no file, and no line number. A reader of the tool output has no way to tell a review comment was left there at all.

This bites hardest on "Suggest Edit" inlines. When a reviewer puts everything into the suggestion and leaves the text empty, Phabricator stores content.raw: "", so the entire comment disappears from the tool's output while the web UI shows a replacement.

Why

PhabricatorPatch._all_comments in bugbug/tools/core/platforms/phabricator.py filters on comment text:

@cached_property
def _all_comments(self) -> list:
    return [c for c in self.get_comments() if c.content.strip()]

For a general comment, empty text means there is nothing to show. For an inline, the location (path, line, length) and the suggestion are carried in transaction["fields"], which PhabricatorInlineComment.__init__ reads -- but the filter runs first, so those fields are never looked at.

Reproducing

D319190 comment 1710710 is a suggestion-only inline on browser/components/tabbrowser/docs/gbrowser.md line 13. transaction.search returns it with content.raw: ""; get_phabricator_revision(319190) does not mention it.

Related

mozilla-conduit/phabricator#102 (bug 2024394) adds hasSuggestion, suggestionText and isNewFile to the inline fields of transaction.search. It is merged but not yet deployed. Once it is, the suggestion text becomes available on the same transaction -- but only for comments this filter has not already discarded.

isNewFile also resolves the on_removed_code = None limitation noted at the same spot, which currently sends get_matching_hunk down its guess-the-side heuristic.

Metadata

Metadata

Assignees

Labels

Fields

Priority

None yet

Start date

None yet

Target date

None yet

Effort

None yet

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions