This thread got auto-closed because I missed @jkaplan ‘s response, so creating a new one.
Github introduced a new UI recently where outdated comments automatically collapse and become very difficult to see/find, especially in PRs with a long commit/review history.
A problem arises when you toss a PR into the merge queue and also have your repo configured to require all comments be resolved prior to merge. There is no obvious sign that once CI finishes your PR will be blocked from merge because of the unresolved bugbot comment. This has resulted in many people in my org thinking that their PR was on its way to getting merged, only to find out that isn’t the case hours or sometimes even days later.
Current behavior:
Comment marked as outdated → resolve if fixed
Ideal behavior:
Comment marked as outdated → immediate resolve
Bugbot detects issue persists → leaves another comment
Bugbot detects issue was actually resolved → no further action necessary
Totally hear you, feel like there’s no really clean solution here. The solution you’re proposing would mean we comment multiple times for the same bug, which can be really annoying for many users who have notifications on, especially if it’s a false positive bug that they don’t want repeated. Also, due to the probabilistic nature of LLMs, it’s entirely possible that a follow-up run doesn’t find the same bug, so there’s no guarantee it would be commented again.
It sounds like your org blocks PRs from merging if there are unresolved comments? Would they be open to relaxing the restriction to allow unresolved comments by bots? GitHub API allows you to tell if a commenter is human or bot.
Yeah I definitely get the hesitation to spam the same comment over and over. I would imagine that you guys pass the list of previous bugs found on subsequent bugbot runs? That being said, looking at my bugbot GH notifications that I get through Slack, I only get one ping per bugbot review, not one per bugbot comment. But maybe that is just some setting Slack or I have set somewhere.
Right now there is no way to relax the restriction like that out of the box. Would require some custom Github action which, from a product perspective, feels like the wrong solution.
One thing I would think about from your end is that as models get better over time, the determinism is going to be less and less of a problem. Not sure I would design the product functionality around that in the long term, though perhaps to not consider it now would be premature.
Put another way, in a world where LLMs are perfectly accurate, I’m pretty convinced my proposed solution above is the best experience.
