Skip to content

Direct review should suppress findings already raised in PR feedback #76

Closed

Description

@congw729

Problem

The production Direct imreview workflow reviews the frozen diff without reading the PR's existing discussion, review summaries, or review threads. As a result, it can return findings that maintainers or reviewers have already raised.

The full/Strict PR workflow already builds a PR context bundle and tells the reviewer not to repeat existing concerns, but this behavior was not carried into Direct mode. Evaluation runs intentionally support PR_CONTEXT_MODE=no_discussion to prevent ground-truth leakage; that evaluation-only boundary should not define production behavior.

Proposed behavior

Keep Direct independent during source analysis, then perform a final duplicate-classification pass against existing PR feedback:

  • new: no equivalent existing concern; include normally.
  • duplicate: equivalent concern already exists; suppress a new comment.
  • extends-existing: materially stronger evidence or impact; point to or reply to the existing thread rather than opening a duplicate.
  • resolved/outdated: do not revive unless the pinned head still contains the defect.

The Direct MCP can remain thin and network-free. The host imreview workflow should fetch the bounded PR discussion/review context and retain it in the shared evidence packet. Evaluation must retain an explicit no-discussion mode.

Acceptance criteria

  • Direct PR reviews inspect bounded existing conversation comments, review summaries, and thread-aware inline comments before final output.
  • Candidate source findings are generated independently before duplicate classification.
  • Final output suppresses duplicate findings and distinguishes extensions from genuinely new findings.
  • Resolved and outdated threads are represented in the classification.
  • Local/worktree reviews continue to work without GitHub context.
  • Evaluation can explicitly disable discussion context.
  • Tests cover duplicate, extension, resolved/outdated, unavailable-context, and local-review behavior.

Motivation

PR vllm-project/vllm-omni#5666 exposed the gap: Direct review independently found two valid issues, but both had already been raised in the PR discussion. The findings were technically correct but would have produced redundant review noise if posted.

Activity

  1. congw729 commented on Aug 13, 2026

    @congw729
    ContributorAuthor
  2. tzhouam commented on Aug 26, 2026

    @tzhouam
    Collaborator

    Implemented and shipped. cb31e65 added the Direct feedback deduplication gate and 1d90819 completed the contract (PR #78): after source findings are frozen independently, the workflow fetches bounded PR feedback (latest 20 conversation comments, 20 review summaries, 50 thread-aware inline threads with resolved/outdated state), classifies every candidate (new / duplicate / extends_existing / resolved_or_outdated), and suppresses duplicates. existing_feedback_status distinguishes checked / unavailable / disabled-for-eval (PR_CONTEXT_MODE=no_discussion stays an explicit evaluation-only mode) / not-applicable for local reviews — matching this issue's acceptance criteria point by point. Closing.

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions