Skip to content

feat(filters): distinguish author and commenter in search filters#2959

Open
afonsojramos wants to merge 2 commits into
mainfrom
abuja-v2
Open

feat(filters): distinguish author and commenter in search filters#2959
afonsojramos wants to merge 2 commits into
mainfrom
abuja-v2

Conversation

@afonsojramos
Copy link
Copy Markdown
Member

@afonsojramos afonsojramos commented Jun 3, 2026

Summary

The author: search filter matched subject.user, but enrichment sets subject.user to the latest commenter (for the avatar), not the thread author. So author:coderabbitai as an exclude hid review requests on human-authored PRs whenever the bot left the last comment, and the filter named "author" was really matching commenters.

This splits that conflated identity into two distinct, forge-agnostic roles on the notification subject:

  • author: who created the thread (pull request / issue / discussion / release / commit)
  • commenter: who left the latest comment, when there is one

subject.user stays as the derived display identity (commenter ?? author), so the avatar and the user-type filter are unchanged. The search qualifiers now read these roles directly:

  • author: matches the thread author
  • commenter: (new) matches the latest comment author

The filter layer no longer inspects GitHub notification reasons, so it stays generic across forges. For commit notifications the commit author and the latest comment author are fetched in parallel, so both roles are populated without extra latency.

Closes #2914

@github-actions github-actions Bot added the bug Something isn't working label Jun 3, 2026
@setchy
Copy link
Copy Markdown
Member

setchy commented Jun 3, 2026

I've been thinking about this FR...

GitHub UI doesn't distinguish between author or commenter, since notifications always show the latest state - so I was tempted to close the FR as unplanned...

That said, alternatively, we could look to change the filter include/exclude tokens to support author: and commenter: prefixes which could search on the two different user attributes.

I'm not sure I'm a fan of changing the filter based on the notification subject code

@afonsojramos
Copy link
Copy Markdown
Member Author

@setchy fair... It is also forge specific code that is leaking A LOT tbh

@afonsojramos afonsojramos changed the title fix(filters): keep directly-addressed notifications when excluding by author feat(filters): distinguish author and commenter in search filters Jun 5, 2026
@github-actions github-actions Bot added enhancement New feature or enhancement to existing functionality and removed bug Something isn't working labels Jun 5, 2026
@afonsojramos
Copy link
Copy Markdown
Member Author

Reworked this to drop the reason-based approach and go with the author: / commenter: split you suggested, @setchy.

Enrichment now exposes two distinct roles on the subject (author for the thread creator, commenter for the latest comment author), and the filter qualifiers read those directly instead of branching on the notification reason. subject.user stays as the display identity (commenter ?? author), so avatars and the user-type filter are unchanged. The filter layer no longer touches GitHub reasons, so the forge-specific leak is gone too.

Net effect:

  • author: matches the actual author now, so excluding a bot no longer suppresses review requests where it only commented last (the original report).
  • commenter: is the new lever for the bot comment noise case.

Two caveats with the current implementation, however:

  1. commenter: is a hammer on a screw, since we no longer reason about why a notification arrived it will also hide a human PR where the bot happened to comment last even if you were review requested.
  2. Commit notifications only populate one of the roles, since we fetch either the comment or the commit, not both.

@afonsojramos
Copy link
Copy Markdown
Member Author

Followup: managed to fix the commit caveat. Commit notifications now fetch the commit and its latest comment in parallel, so both author and commenter are populated like the other types

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 5, 2026

@afonsojramos
Copy link
Copy Markdown
Member Author

So, final caveat: If one excludes commenter:coderabbitai, a teammate opens a PR and requests a review, then CodeRabbit auto-comments after the request and becomes the latest commenter. That review-request notification gets hidden even though you were asked to review. But maybe this is a disadvantage of excluding bot comments? Or an incentive to be selective of which commenters to exclude. But, to be honest, this was already the case, so nothing new.

Before: author: was the only tool, it secretly matched the last commenter, and you had no way to avoid that. Every user of that filter hit the bluntness.

Now: author: matches the real author, so it’s precise and no longer drops review requests. The old blunt “match whoever commented last” behavior still exists, but only if you deliberately reach for commenter:.

@setchy
Copy link
Copy Markdown
Member

setchy commented Jun 5, 2026

Thanks @afonsojramos.

Let me soak on this a bit.

We also have a Bot filter which includes dependabot, renovate, netlify. I wonder if we should expand that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or enhancement to existing functionality

Development

Successfully merging this pull request may close these issues.

author: exclude filter suppresses review_requested notifications on matching PRs

2 participants