[$250] PR Reviewer Checklist fails if more than 1 comment contains `# Reviewer Checklist` text

[amyevans]

In #82448 (comment), MelvinBot posted the comment:

The failing check is `checklist`, which was triggered by a `pull_request_review` event. This check looks for a comment containing `# Reviewer Checklist` with all items checked off — it's a process gate, not a code issue.

**To fix this:** A reviewer needs to post the [Reviewer Checklist](https://raw.githubusercontent.com/Expensify/App/main/contributingGuides/REVIEWER_CHECKLIST.md) as a comment on this PR with all items checked. Once posted, the check will pass on the next run.

No code changes are needed.

Later I posted the completed Reviewer Checklist in #82448 (comment).

However, the PR Reviewer Checklist check continued to fail. The workflow does a simple indexOf('# Reviewer Checklist') search through comments in chronological order and stops at the first match, so it was never finding my checklist.

Issue Owner

Current Issue Owner: @truph01

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~022027170879042388990
• Upwork Job ID: 2027170879042388990
• Last Price Increase: 2026-02-26

[melvin-bot]

Triggered auto assignment to @dylanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[MelvinBot]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The PR Reviewer Checklist CI check fails even after a reviewer posts a completed checklist, if an earlier comment on the PR contains the text # Reviewer Checklist anywhere in its body (e.g., inside backticks or a code block). In the reported case, MelvinBot posted a comment that included `# Reviewer Checklist` as inline code, and the workflow matched that comment instead of the actual completed checklist.

What is the root cause of that problem?

The reviewer checklist workflow (reviewerChecklist.ts) iterates through all PR comments in chronological order and stops at the first comment where indexOf('# Reviewer Checklist') matches (lines 51–68). Two problems combine to cause the bug:

1. indexOf is too broad (line 63) — it matches the substring # Reviewer Checklist anywhere in the comment body, including inside backtick-formatted code, blockquotes, or other non-checklist contexts.
2. The loop breaks on the first match (lines 53–55) — once it finds any comment containing that substring, it stops looking. If the first match is a non-checklist comment (like MelvinBot's), the actual completed checklist is never evaluated.

What changes do you think we should make in order to solve the problem?

Modify the loop in checkIssueForCompletedChecklist in reviewerChecklist.ts (lines 46–68) so that instead of stopping at the first comment that contains # Reviewer Checklist, it checks all matching comments and passes if any of them has a fully completed checklist. Specifically:

• Remove the early break on first match (lines 53–55).
• Track the "best" result across all matching comments — if any comment has all items checked and zero unfinished items, the check should pass.
• Keep the existing validation logic (min/max completed items, zero unfinished items) but apply it per-comment and succeed if any comment passes.

Then rebuild the index.js bundle.

What alternative solutions did you explore? (Optional)

1. Iterate in reverse order (newest first) — This would fix the immediate symptom but is fragile: if someone posts a non-checklist comment containing the text after the real checklist, it would break again.
2. Use a stricter match (e.g., require # Reviewer Checklist to appear at the start of a line, not inside backticks/code blocks) — This would help but is hard to make robust against all markdown contexts, and still wouldn't handle the case of someone posting an incomplete checklist before the completed one.

Checking all matching comments and passing if any is complete is the most robust approach.

---

Next Steps for Contributor+ team: Reply with @MelvinBot implement this to create a draft PR, or @MelvinBot <your feedback> to refine this analysis.

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @truph01 (External)

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~022027170879042388990

[yuvrajangadsingh]

Proposal

Please re-state the problem that we are trying to solve in this issue.

the PR Reviewer Checklist CI check fails even after a reviewer posts a completed checklist, because an earlier comment (from MelvinBot) contained # Reviewer Checklist as inline code and the workflow matched that instead of the real checklist.

What is the root cause of that problem?

the matching logic in reviewerChecklist.ts (line 62) uses indexOf('# Reviewer Checklist') on the comment body after all newlines are already stripped (lines 58-59). this creates two problems:

1. stripping newlines first makes it impossible to distinguish a markdown heading from text inside backtick code formatting - everything becomes one flat string
2. indexOf then matches the substring anywhere in that flat string, including inside inline code like `# Reviewer Checklist`

so when MelvinBot posts a comment mentioning the checklist text in backticks, it matches as a false positive. the comment has zero checkbox items, so the check fails immediately since 0 >= minCompletedItems is false.

this is different from just "breaking on first match" - even if you iterated all comments, the fundamental issue is that non-checklist comments can false-positive match. for example, a reviewer could quote the heading in a discussion comment after posting the real checklist, and iterating all would count that quote too.

What changes do you think we should make in order to solve the problem?

strip code formatting from the comment before doing the indexOf check. in checkIssueForCompletedChecklist in reviewerChecklist.ts, after stripping whitespace on line 59, strip inline code and code blocks before matching:

const commentWithoutCode = comment?.replace(/`+[^`]*`+/g, '') ?? '';

if (commentWithoutCode.indexOf(reviewerChecklistContains) !== -1) {

this removes text inside backticks (both single and triple) before the heading check, so MelvinBot's inline code reference won't match. the checkbox counting on lines 64-65 should also use commentWithoutCode to avoid counting checkboxes inside code blocks.

the existing first-match-and-break logic stays as-is since it works correctly once false positives are eliminated.

[nkdengineer]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The PR Reviewer Checklist GitHub Action (checklist) can fail even after a reviewer posts the completed Reviewer Checklist as a comment. The check searches comments for one that contains # Reviewer Checklist and validates that all items are checked. Because it uses the first such comment in chronological order, it may validate the wrong comment (e.g. a bot or discussion comment that merely mentions “# Reviewer Checklist”) and never consider the reviewer’s later, completed checklist. So the process gate fails despite the reviewer having done the right thing.

What is the root cause of that problem?

The logic in .github/actions/javascript/reviewerChecklist/reviewerChecklist.ts iterates over combinedComments in index order and stops at the first comment where indexOf('# Reviewer Checklist') !== -1. It then counts completed/unfinished checklist items only in that comment and passes or fails based on it. There is no rule that the matching comment must be the actual checklist (e.g. the one from the reviewer with all boxes checked). So any earlier comment that contains the string “# Reviewer Checklist” (for example MelvinBot’s explanation: “This check looks for a comment containing # Reviewer Checklist…”) is chosen. That comment does not contain a full checklist with all items checked, so the run fails. The reviewer’s subsequent comment, which does contain the completed checklist, is never evaluated.

What changes do you think we should make in order to solve the problem?

Use the last comment that contains # Reviewer Checklist instead of the first. That way, when a bot or other message mentions the checklist earlier and the reviewer posts the real checklist later, the script validates the reviewer’s comment. Implementation: when iterating, do not break on the first match; keep updating the “chosen” comment (or its finished/unfinished counts) whenever a comment contains the checklist string. After the loop, run the existing pass/fail logic on that last matching comment. If no comment matches, keep the current behavior (fail with “No PR Reviewer Checklist was found”).

Related file: .github/actions/javascript/reviewerChecklist/reviewerChecklist.ts

Pseudocode (change in the loop that finds the checklist):

• Current (bug): The loop breaks as soon as the first comment containing # Reviewer Checklist is found, so that comment is the one validated.
• New: Do not break on match; on every comment that contains # Reviewer Checklist, overwrite numberOfFinishedChecklistItems and numberOfUnfinishedChecklistItems. After the loop, the counts correspond to the last such comment.

for (let i = 0; i < combinedComments.length; i++) {
    // REMOVE: if (foundReviewerChecklist) { break; }
    const comment = combinedComments[i] with whitespace normalized;
    if (comment contains "# Reviewer Checklist") {
        foundReviewerChecklist = true;
        numberOfFinishedChecklistItems = count(comment, /- \[x\]/gi);
        numberOfUnfinishedChecklistItems = count(comment, /- \[ \]/g);
        // Do not break — keep iterating so the last matching comment wins.
    }
}
// Existing pass/fail logic below is unchanged.

Concrete change in file: In .github/actions/javascript/reviewerChecklist/reviewerChecklist.ts, delete the block at lines 52–55 that does if (foundReviewerChecklist) { break; }. The rest of the loop body stays the same; the final counts will then come from the last comment that contains the checklist heading.

What alternative solutions did you explore? (Optional)

• Filter by comment author (e.g. ignore bot comments): Would require defining which authors count and could break if bots are renamed or new bots are added. Using the last matching comment is simpler and does not depend on actor identity.
• Require the matching comment to look like the real checklist (e.g. minimum number of - [x] lines): Could help but adds heuristics and might need updates when the checklist length changes. Prefer the single rule “use the last occurrence” for clarity and stability.

[fl-sean03]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The PR Reviewer Checklist CI check fails even when a reviewer has posted a completed checklist. This happens because the script finds the first comment containing # Reviewer Checklist instead of the last (most recent) one. When any earlier comment (e.g., a bot message, a discussion comment, or a re-review from a prior round) also contains that text, the script evaluates the wrong comment and fails.

What is the root cause of that problem?

In .github/actions/javascript/reviewerChecklist/reviewerChecklist.ts, the checkIssueForCompletedChecklist function iterates combinedComments from index 0 and breaks immediately upon finding the first matching comment:

for (let i = 0; i < combinedComments.length; i++) {
    if (foundReviewerChecklist) {
        break;  // stops at the FIRST match — this is the bug
    }
    // ...
    if (comment?.indexOf(reviewerChecklistContains) !== -1) {
        foundReviewerChecklist = true;
        // counts items in the first match only
    }
}

Comments are appended in chronological order (review comments first, then regular comments), so the first match is always the oldest. The reviewer's completed checklist — posted later — is never reached.

What changes do you think we should make in order to solve the problem?

Iterate in reverse so the loop finds the last (most recent) matching comment first, then breaks immediately. This is O(1) in the happy path and requires only a one-line change to the loop bounds:

File: .github/actions/javascript/reviewerChecklist/reviewerChecklist.ts

Change:

// Before (finds FIRST match):
for (let i = 0; i < combinedComments.length; i++) {
    if (foundReviewerChecklist) {
        break;
    }
    ...
}

// After (finds LAST match):
for (let i = combinedComments.length - 1; i >= 0; i--) {
    if (foundReviewerChecklist) {
        break;
    }
    ...
}

No other changes needed. The existing break logic remains intact; iterating in reverse means it now stops at the newest checklist comment. The compiled index.js must also be regenerated from the TypeScript source.

Testing plan

Add a test scenario with two comments both containing # Reviewer Checklist:

1. An earlier comment with incomplete/zero checked items (simulating a bot mention or stale checklist)
2. A later comment with all items checked (the real reviewer checklist)

The fix should cause the script to evaluate the later comment and pass CI.

Happy to submit a PR once assigned.