Request a technical writer when ML labelling adds Technical Writing

What does this MR do and why?

When a contributor runs @gitlab-bot ready on a merge request that only changes files under doc/ or docs/, and CODEOWNERS names a technical writer for it, CommandMrRequestReview skips the merge request coach. Since !4198 (merged) it leaves the review request to AutomatedReviewRequestDoc, which reacts to workflowready for review being added. That processor did nothing when the merge request carried Technical Writing, a guard meant for a writer who is already involved. ML labelling can now apply that label as the merge request's group before the author runs the command, and when it does, the merge request ends up with no reviewer at all. gitlab-org/gitlab!253321 (merged) is where this was found.

This MR changes the guards in AutomatedReviewRequestDoc#applicable?, as worked out in review:

• Technical Writing no longer stops it.
• It stays out when one of the CODEOWNERS technical writers (most_specific_approvers) is already a reviewer, which is the question the label was standing in for.
• twdoing and twfinished still stop it. Writers add them as part of their review, including writers outside the most specific CODEOWNERS rule, whom the reviewer check does not count.
• twtriaged and the processor's own earlier comment no longer stop it. The processor adds both when it first asks a writer, so once that writer was no longer a reviewer, a new review request on a docs-only merge request reached nobody. The processor runs only when workflowready for review is added, so whether a writer from CODEOWNERS is a reviewer at that moment is what decides. The label's description says no more reminders are required from the bot, and the processor sends none on its own: it asks again only when a new review is requested and no writer from CODEOWNERS is a reviewer.

It still adds documentation and twtriaged. The tw:: labels are scoped, and it stays out while twdoing or twfinished is there, so it never replaces a writer's label.

CommandMrRequestReview does not change. On a docs-only merge request with a writer in CODEOWNERS it still skips the coach, and AutomatedReviewRequestDoc requests the writer with its usual comment. Nothing reads Technical Writing through Labels any more, so that constant goes.

Closes #1826 (closed)

The change is in three commits, each of which passes the full suite and RuboCop on its own:

1. Exercise the label guard in AutomatedReviewRequestDoc specs. The contexts for Technical Writing, twtriaged and tw::doing ran without the community contribution stubs, so the event was never a community contribution there and each passed for that reason alone: with the label guard deleted from applicable?, the spec on master stays green. They now fail on their own condition.
2. Request a technical writer when ML labelling adds Technical Writing. The fix, the specs that show it, and the processor's documentation text, which said it adds Technical Writing while it adds documentation and twtriaged.
3. Ask a technical writer again when a new review is requested. twtriaged and the earlier comment no longer stop it, and the hidden marker only the comment check read goes; twdoing and twfinished still do. With specs for a writer who was asked before and is no longer a reviewer, and for twfinished.

Expected impact & dry-runs

This changes one reactive processor and no policy, so no policy dry-run applies. I have not run it against the playground project.

For a community merge request with documentation changes, when workflowready for review is added:

	Before	After
Technical Writing applied (ML labelling)	No writer requested; on a docs-only merge request, no reviewer at all	A writer from CODEOWNERS requested
A writer from CODEOWNERS already a reviewer	The seeded writer requested as well, who can be a second writer	No writer requested
A writer asked before and no longer a reviewer	No writer requested; on a docs-only merge request, no reviewer at all	A writer from CODEOWNERS requested again
twdoing or twfinished applied	No writer requested	No writer requested

Merge requests that change docs and code get a coach from the command as before, and now also a writer when ML labelling applied Technical Writing.

I checked it in ruby:4.0.7, the image .gitlab-ci.yml uses, with the bundle from Gemfile.lock, on top of master at 4e9882cf:

• Each commit: the full suite and RuboCop on the files it touches. 0 failures and no offenses on all three.
• Each fix fails without its code: the second commit's spec, run against the first commit's code, fails in the three examples that describe its new behaviour; the third commit's spec, run against the second commit's code, fails in its four. Each passes against its own code.
• Each condition is pinned: removing the check on the writers' tw:: labels, removing the reviewer check, putting Technical Writing back in the label check, letting twtriaged stop it again, or bringing the earlier comment check back each fails the spec (4, 2, 1, 2 and 1 failing examples).
• The guard specs: with the label guard deleted from applicable?, the spec on master passes (20 examples, 0 failures) and the spec from the first commit fails in its 6 label examples.
• The specs and undercoverage jobs as CI runs them at the head: 6377 examples, 0 failures, line coverage 98.3%, and undercover -c against master reports no missing coverage in the changes.
• RuboCop as the lint-ruby job runs it (rubocop -P -E .) at the head: 667 files, no offenses.
• Danger's commit and title linters from gitlab-dangerfiles 4.12.0, the version in Gemfile.lock: no problems in the three commit messages or this title.

Full suite and RuboCop per commit

For each commit, bundle exec rspec over the whole suite and rubocop --force-exclusion on the Ruby files the commit touches; then rubocop -P -E . at the head.

=== 7212dc3f Exercise the label guard in AutomatedReviewRequestDoc specs
6375 examples, 0 failures
1 file inspected, no offenses detected
=== 18a62236 Request a technical writer when ML labelling adds Technical Writing
6377 examples, 0 failures
3 files inspected, no offenses detected
=== 90be5c79 Ask a technical writer again when a new review is requested
6377 examples, 0 failures
2 files inspected, no offenses detected
=== CI lint-ruby at head: rubocop -P -E .
667 files inspected, no offenses detected

The second commit's spec against the first commit's code

$ bundle exec rspec spec/processor/community/automated_review_request_doc_spec.rb
22 examples, 3 failures
rspec './spec/processor/community/automated_review_request_doc_spec.rb[1:2:8:1:2]' # Triage::AutomatedReviewRequestDoc#applicable? with technical writers in CODEOWNERS when one of them is already a reviewer does not process the event
rspec './spec/processor/community/automated_review_request_doc_spec.rb[1:2:8:1:1]' # Triage::AutomatedReviewRequestDoc#applicable? with technical writers in CODEOWNERS when one of them is already a reviewer is not applicable
rspec ./spec/processor/community/automated_review_request_doc_spec.rb:54 # Triage::AutomatedReviewRequestDoc#applicable? when MR has Technical Writing label, as ML labelling applies it is applicable

Against its own code: 22 examples, 0 failures.

The third commit's spec against the second commit's code

$ bundle exec rspec spec/processor/community/automated_review_request_doc_spec.rb
22 examples, 4 failures
rspec './spec/processor/community/automated_review_request_doc_spec.rb[1:4:1:1]' # Triage::AutomatedReviewRequestDoc#process when event is applicable posts a comment mentioning technical writer and labeling the merge request
rspec './spec/processor/community/automated_review_request_doc_spec.rb[1:4:3:1]' # Triage::AutomatedReviewRequestDoc#process when merge request changes docs/ posts a comment mentioning technical writer and labeling the merge request
rspec ./spec/processor/community/automated_review_request_doc_spec.rb:56 # Triage::AutomatedReviewRequestDoc#applicable? when MR has tw::triaged label from an earlier request is applicable
rspec ./spec/processor/community/automated_review_request_doc_spec.rb:110 # Triage::AutomatedReviewRequestDoc#applicable? with technical writers in CODEOWNERS when one of them was asked before and is no longer a reviewer is applicable

Against its own code: 22 examples, 0 failures.

Action items

• No environment variable is added.
• A writer removed after setting twdoing is not replaced while the label stays; the docs workflow has writers remove it when they stop for more than a week. A writer removed before setting a label is asked again, the same one, since the pick is seeded by the source branch. The review thread compares this with the other options.
• The handbook's Triage Operations page says the ready command picks a random coach when no reviewer is given, and does not mention the docs-only case !4198 (merged) added. Once this is merged I can open a handbook MR that describes it.