From the gopherbot log today, I saw the following group of logs appear over and over. If you look at those cls (for example, https://go-review.googlesource.com/c/website/+/405954), they already have reviewers assigned. So, the info in the log messages is not true.

The messages are saying "Setting review [...] would have no effect, continuing". That is a mechanism in gopherbot that was added in CL 236438 to avoid making Gerrit API requests that would fail. As far as I can tell, the log message is accurate, please let me know if I misunderstood.

The general problem of gopherbot taking some repetitive actions is tracked in issue #28320. The current mechanism of determining whether there are enough human reviewers relies on heuristics because there's not enough information available, and there hasn't been progress on that for a while.

On the other hand, I was trying to figure out why CLs like https://go-review.googlesource.com/c/tools/+/408376 never get reviewers assigned automatically. I couldn't find relevant logs that indicate gopherbot did reviewer assignment for cl/408376.

That CL is already submitted, and it's generally easier to investigate this type of problem when it is still ongoing. If you observe this happening again this week, can you please ping me directly and I'm happy to look. Thanks.

Thanks for looking into this @dmitshur

Misleading log message I refer to is:

2022/05/25 16:10:46 No reviewers or cc: https://go-review.googlesource.com/c/website/+/405954

It's still unclear to me why gopherbot keeps looking into those cls (https://go-review.googlesource.com/c/website/+/405954) that already have reviewers. From #28320 title, I thought it was about gopherbot's actions not being applied. But in this case, there is no action gopherbot should take except updating some internal state gopherbot maintains.

That CL is already submitted, and it's generally easier to investigate this type of problem when it is still ongoing. If you observe this happening again this week, can you please ping me directly and I'm happy to look.

For example https://go-review.googlesource.com/c/tools/+/408375 is not getting any reviewer. According to the dev.golang.org/owners I think gopherbot should be able to pick matloob and other go/analysis owners.

Change https://go.dev/cl/409654 mentions this issue: cmd/gopherbot: improve clarity of log message

Ah, thanks, that log line is indeed misleading. Sent CL 409654 to improve it.

It's still unclear to me why gopherbot keeps looking into those cls (https://go-review.googlesource.com/c/website/+/405954) that already have reviewers.

I understand the heuristics implemented in humanReviewersOnChange think the existing reviewer (myself) is the original author of the PR, so it's waiting for a second human (what it thinks would be the first human reviewer) to be added.

For example https://go-review.googlesource.com/c/tools/+/408375 is not getting any reviewer. According to the dev.golang.org/owners I think gopherbot should be able to pick matloob and other go/analysis owners.

I've done some printf debugging in dry-run mode, and see that humanReviewersOnChange reports a single human reviewer, Gerrit account ID 5190 which is your Gerrit account. It comes from the call to humanReviewersInMetas:

line "Reviewer: Gerrit User 5190 <5190@62eb7196-b449-3ce5-99f1-c037f21e1705>" resulted in a human reviewer being found

That is definitely a false positive. Some more debugging/investigation would need to happen to move this forward.

Used a short program to print the meta commits on that CL. It seems that leaving the Ru-TryBot+1 vote is what confused humanReviewersInMetas into thinking there's already a reviewer:

[...]

meta 1:
Update patch set 1

Patch Set 1: Run-TryBot+1

Patch-set: 1
Reviewer: Gerrit User 5190 <5190@62eb7196-b449-3ce5-99f1-c037f21e1705>
Label: Run-TryBot=+1, f62fac12d7c8a14999c95b9f2ea711b6a15df2c2

[...]

This change may be a surprise to some users. I frequently send changes without reviewers, but not in the WIP or DO NOT REVIEW state, just to run trybots on it. It's coincidentally never triggered assigning reviewers because of this bug.

We may want to either poll or at least notify people who have trybot access.

I'll send my patch regardless.

Change https://go.dev/cl/417479 mentions this issue: cmd/gopherbot: omit owner from assign reviewers

Thanks for making progress and mailing a CL @toothrot.

I know there's at least one existing mechanism to opt out of gopherbot's automatic reviewer behavior (see here). Perhaps you and others may want to use it, and perhaps there can/should be additional ways to opt out.

I suspect there are two broad categories of contributors—new contributors and recurring contributors—and gopherbot's automatic reviewer assignment task serves them differently. My general understanding is that if we find the reviewer assignement task to be net helpful and worth keeping, it's likely worth making it work better and more predictably, so if there's a poll, my vote would to fix this as a next step. (If the task is not net helpful, we should disable it entirely.)

Fair enough! It's probably less of a big deal then I thought at first. It just clicked for me that my workflow was changing as a part of it while working on it.