Skip to content

[Bug] A [resolved] line that declines to raise its point is promoted into a P2 finding #155

Description

@tzhouam

Split out of #141, which reported two published reviews with contradictory output. The first example (vllm-project/vllm-omni#6959, an explicitly-dropped request published inline) is fixed by #154. The second has a different mechanism, diagnosed while writing that fix, and changing it affects recall — so it is filed separately rather than bundled.

The mechanism

_promote_resolved_residuals (engine/steps/review/steps.py) re-files a [resolved] findings line as a minor review comment when the line states a residual. "States a residual" is decided by substring match:

_RESIDUAL_MARKERS = ("residual", "not covered", "does not cover", "still ",
                     "remains ", "left unfixed", "but ")

"but " is a bare conjunction, not a residual claim. A sentence of the shape

resolved by the request-unique key and fail-closed guard, but I am not raising it again

matches, and is promoted into a finding whose own text says it is not being raised — which is exactly what vllm-project/vllm-omni#6968 published: the validation section treats the TI2I guidance-mode concern as resolved, while the first item under Findings presents that same observation as P2.

The promoted comment also inherits the whole findings line as its comment, so the resolved-confirmation clause travels into the published finding rather than just the residual.

Why it is not a one-line fix

Promotion exists because it was measured to work: ~70% of ground truth on merged/amended heads is "a reviewer raised X, the fix landed — what does it still not cover?", and routing those answers into the scored channel was a real recall gain. Tightening the gate trades against that, and the eval that justified promotion would need re-running to size the trade. Deleting "but " is the obvious move and probably right, but "probably right" is not a number.

Options, roughly in order of how structural they are:

  1. Drop "but " from the markers; keep the rest. Smallest change, unmeasured.
  2. Require the promoted body to resolve to a file:line. A residual defect a reader cannot locate is not actionable, and [Bug] Strict reviews publish discarded, resolved, and non-actionable findings #141 asks that a published finding "clearly identify the remaining defect and required action". Structural rather than lexical, but drops genuinely unanchored residuals.
  3. Stop mining findings prose entirely: have the reviewer emit residuals as comments carrying disposition: publish and the [resolved] line as evidence. Removes the heuristic altogether; largest behaviour change, needs the eval.

Acceptance

  • A [resolved] line that declines to raise its point cannot become a published finding, by a rule that is not a keyword filter over prose ([Bug] Strict reviews publish discarded, resolved, and non-actionable findings #141's explicit constraint).
  • A genuine residual with an ask still publishes — a fixture from the measurement set, not a synthetic one.
  • Whatever is chosen, the recall effect is measured rather than asserted.

#141 stays open until this lands; its acceptance criteria are not met while promotion can still manufacture one of these.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KT4EvUTzzLeq243DG6T1mC

Activity

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