Skip to content

qodo_open Double-Counts and Never Clears the Open-Source Qodo App's Threaded Findings #2252

Open

Description

@ptr727

pr_review.py's qodo_open reads Qodo's findings from its Code Review by Qodo comment and treats a finding as closed only when it carries Qodo's Resolved or Dismissed badge. That fit the paid app. The open-source app, qodo-free-for-open-source-projects and the only Qodo identity left since the paid app was uninstalled, behaves differently:

  • It opens a review thread for each finding, so the finding is already counted in unresolved=. qodo_open then counts it a second time and prints "no thread to resolve" for it.
  • Resolving the thread adds no badge to the findings comment, so qodo_open never drops back to zero. Merge Gate item 3 then sends the driving agent to re-answer findings that are already resolved, every round.

Measured with the open-source login tracked: on ptr727/PlexCleaner#999 and #1000, every Qodo thread is resolved, yet status reads qodo_open=3 and qodo_open=2.

What was tried, and why it was backed out

Three rounds on the branch that switched the login matched each comment finding to its thread by its heading. Each local review pass found new defects. The last one is decisive:

  • The number is not shared. The findings comment and the threads number the same finding independently, even within one round. Across 559 historical numbered findings (PlexCleaner from both logins, plus 200 ProjectTemplate PRs from the paid login), 359 have a thread with the same title under a different number. ProjectTemplate Settle the Workflow CI Contract Skill Against Its Sources #1252 shows it within a single round.
  • Prefix matching hides findings. A startswith match let a thread whose title extends a finding's title clear that finding, and a code-led title reduced to its number alone matched any thread with that number. Both are the silent direction: the finding drops out of qodo_open= and unresolved= alike.
  • Code spans and trailing labels need symmetric handling. Real titles never carry <code> or backticks (0 of 559), since Qodo renders code in a title as <b><i>...</i></b>, but a matcher that strips them on one side only breaks on the constructed case.
  • The regexes must stay linear. A lazy <[^>]*> and a trailing-label pattern anchored with $ both took quadratic time on a long line. The file's existing BARE_TAG does that job in linear time.

The direction the last round pointed to

Match on the title alone, with the number dropped. Normalize both sides the same way: the first numbered line, every code span removed, markup stripped with BARE_TAG, escapes removed, case folded. Never match an empty title. Let each thread clear at most one finding with that title, so a re-raised duplicate stays open. That keeps any error in the loud direction. Alternatively, decide that on the open-source app qodo_open should count only the findings that have no Qodo thread on the pull request at all, which may be simpler. Either way, every rule above deserves a test, and the PlexCleaner and ProjectTemplate history above is the corpus to check a design against.

Until this lands, a PlexCleaner pull request shows resolved Qodo findings as qodo_open. That is a loud error, never a hidden finding, so answer them by checking each one's thread state.

Relates #1404, #1465.

Activity

  1. ptr727 commented on Oct 29, 2026

    @ptr727
    OwnerAuthor

    New evidence, and a verification step the maintainer asked for (2026-10-29).

    Todo: verify pr_review.py's Qodo handling end to end against a live Qodo pull request. Qodo's open-source app reviews only a public repository past its star gate. In this fleet that is PlexCleaner alone, so no hub pull request can exercise this path, and the hub's tests see only constructed comments. The fix for this issue should be checked against a live PlexCleaner pull request, from the findings comment through to qodo_open=0, before it is called done.

    What ptr727/PlexCleaner#1060 adds. It has one Qodo finding. The thread was answered with evidence and resolved, and the finding was also answered in the conversation, quoted under a "Qodo open findings" heading. Qodo replied to that answer, saying:

    • the published review still lists the finding as open
    • resolving the inline thread does not change the status of the PR-level finding
    • the finding closes only when Qodo is asked explicitly to dismiss it by number

    So on the open-source app, neither resolving the thread nor answering in the conversation clears the finding. An explicit dismiss request to Qodo is the step that does. That is presumably also what adds the Dismissed badge qodo_open already reads.

    This gives the design a third option beside the two in the body: on a disproved or fixed finding, have the drive ask Qodo to dismiss it by number, and keep qodo_open reading the badge as it does today. That needs verifying on a live pull request first:

    • whether a dismiss request actually adds the badge to the findings comment
    • what wording Qodo accepts
    • whether the number it wants is the findings comment's number, which this issue already shows differs from the thread's
  2. ptr727 commented on Nov 1, 2026

    @ptr727
    OwnerAuthor

    Decision needed (decision triage, 2026-11-01)

    Question: How should qodo_open in scripts/pr_review_digest.py stop counting Qodo findings that are already answered, given the open-source Qodo app threads every finding but never adds the Resolved or Dismissed badge: match findings to threads by normalized title, count only unthreaded findings, or ask Qodo to dismiss each by number?

    Options, recommended first:

    1. Match on normalized title, one thread per finding (recommended): The design the issue body already worked out: drop the number (359 of 559 historical findings carry a different number in their thread), normalize both sides with the existing linear BARE_TAG, and let each thread clear at most one finding. Errors stay loud, no new outward write, and the 559-finding corpus can test it. Still needs an end-to-end check on a live PlexCleaner PR. No work in flight.
    2. Ask Qodo to dismiss each finding by number: Qodo said on PlexCleaner#1060 that only an explicit dismiss closes a finding, so the badge reader stays. But whether a dismiss adds the badge, the accepted wording, and which number are all unverified, and it adds one conversation write per finding per round. No work in flight.
    3. On the open-source app, count only unthreaded findings: Simplest to state, but deciding which findings lack a thread needs option 1's matching anyway, and a cruder count moves errors in the silent direction. No work in flight.

    Evidence: Gap live on develop: pr_review_digest.py:176-177, 215 count open sections from qodo_comment(). scripts/reviewers/qodo.json:19 closes a finding only on the Resolved|Dismissed badge. pr_review_digest.py:343 still says to answer in the conversation, which the 2026-10-29 comment here shows does not clear it. pr_merge.py:36, 40 list qodo_open. Prefix and number matching each hid findings (issue body). Only PlexCleaner passes Qodo's star gate, so no hub PR exercises this path.

    Blocks: none

  3. added
    decisionA question waiting on the maintainer, alone or beside whatever else the issue carries
    on Nov 1, 2026
  4. ptr727 commented on Nov 1, 2026

    @ptr727
    OwnerAuthor

    Maintainer answer (decision triage, 2026-11-01): Keep qodo_open reading Qodo's own Resolved/Dismissed badge, so the count matches Qodo's published review. This replaces the triage comment's title-matching recommendation, because Qodo marks a fixed finding Resolved on its own and a disproved finding Dismissed only on an explicit dismiss request: resolving the thread or answering in the conversation does not count (PlexCleaner#1060, and Qodo's docs). For a disproved finding, the drive posts a comment mentioning Qodo that asks it to dismiss that finding by number, with the reason, and pr_review_digest.py's remedy text says so. Before calling it done, verify on a live PlexCleaner PR that a dismiss request adds the badge, which wording Qodo accepts, and which number it wants. This is now work for the next burndown round.

  5. removed
    decisionA question waiting on the maintainer, alone or beside whatever else the issue carries
    on Nov 1, 2026
  6. ptr727 commented on Nov 1, 2026

    @ptr727
    OwnerAuthor

    More occurrences, from ptr727/PlexCleaner on 2026-11-01. The maintainer has now had to merge by hand three times because of this. The ask is to make this work for the next burndown round a priority.

    In each case pr_review.py merge refused with status=UNTHREADED_FINDINGS ... qodo_open=1 while every other Merge Gate item was met (merge=CLEAN, all checks green, unresolved=0):

    What makes it costly today:

    • The digest's remedy text still says "answer it in the PR conversation quoting the finding". Per the maintainer answer above, that never adds the badge. So a driver follows the remedy, re-runs the gate, and gets the same refusal.
    • drive-pr step 4 routes UNTHREADED_FINDINGS to the maintainer as a blocked merge. That is correct for a real open finding, but here it fires every time Qodo raises anything a driver declines.
    • The dismiss-request mechanism the maintainer chose is not verified yet: the wording Qodo accepts, the number it wants, and whether the badge follows. The next PlexCleaner PR carrying a declined Qodo finding is a live place to verify it.
  7. ptr727 commented on Nov 2, 2026

    @ptr727
    OwnerAuthor

    Released (2026-11-02): PR #3809 merged to develop and its branch is deleted, so this claim holds no files. This issue stays open: the remaining done-when item is the live check on a PlexCleaner pull request, which is outside this repository (see the comment on this issue about what landed).

    Backlog burndown claim (away run 2026-11-02, round 1, group G1). Branch: burndown/2252-merge-gate-reviews. Issues: #2252, #3789.

    Predicted file set:

    • scripts/pr_review.py, scripts/pr_review_digest.py, scripts/pr_review_checks.py, scripts/pr_review_answers.py, scripts/pr_merge.py
    • tests/test_pr_review.py, tests/test_pr_merge.py, tests/test_pr_review_copilot.py, tests/test_pr_review_size.py
    • .agents/skills/pr-review-conduct/ and .agents/skills/drive-pr/, with their generated trees (.github/skills/<name>/, .claude-plugin/fleet-skills/skills/<name>/, .claude-plugin/fleet-skills/.source-digests/<name>)

    Out of bounds for this group: scripts/README.md (group G4 holds it), scripts/local_review.py, .github/workflows/* and .github/actions/* (open PR #3801). The live PlexCleaner verification #2252 asks for is outside this repository and is not part of this group, so #2252 stays open after this pull request.

    Scope: develop only. The feature pull request carries Closes on promotion: for each issue it fully fixes.

  8. ptr727 commented on Nov 2, 2026

    @ptr727
    OwnerAuthor

    Landed on develop in #3809 (squash 2690b87). This issue stays open for the live check.

    What landed, per the maintainer answer of 2026-11-01:

    • qodo_open still reads Qodo's own Resolved/Dismissed badge, unchanged.
    • The digest's Qodo remedy line no longer says to answer in the conversation. It says resolving a thread or answering in the conversation adds no badge, and that Qodo marks a fixed finding Resolved itself. For a disproved finding, the driver answers with comment --ack <key>=<outcome>. That comment's body mentions @qodo-free-for-open-source-projects and asks it to dismiss finding <N> by that number, with the reason. <N> is the leading number of the finding's entry in Qodo's findings comment. One write asks Qodo for the badge and records the answer merge reads.
    • pr-review-conduct Merge Gate item 3 and drive-pr step 4 say the same. drive-pr routes an UNTHREADED_FINDINGS line naming qodo_open to that answer rather than to a blocked merge.

    Still open: the live check. It has to run on a PlexCleaner pull request carrying a declined Qodo finding. It confirms three things:

    • whether the dismiss request adds the Dismissed badge to the findings comment, so qodo_open drops
    • which wording Qodo accepts, including whether it responds to that mention
    • whether the number it wants is the findings comment's number, which the remedy line prints, or the thread's

    If the mention or the number turns out wrong, the fix is the remedy line in scripts/pr_review_answers.py unthreaded_lines and the two skill sentences.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    agentsAgents instructions

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions