Repository navigation
[Bug] Strict reviews publish discarded, resolved, and non-actionable findings #141
Description
Problem
Recent vllm-omni-review-bot Strict reviews publish findings that contradict the same review's final selection notes: explicitly discarded requests and resolved observations still appear as active P2 findings, and a confirmation that no problem exists is posted as a P2 inline comment.
Maintainers and contributors must reconcile the bot's own output before deciding what work remains. The review summary, finding list, and inline comments should present one consistent set of actionable conclusions.
Observed examples
vllm-project/vllm-omni#6959 — discarded request and non-issue published inline
Review submitted on 2026-09-08 at 03:18:45 UTC, reviewed head:
2c36577b7c854262fbfb548cdc532c5369770a67
The summary says to retain two comments about the lock scope and non-default-source fallback coverage, drop the repo-id validation request because the evidence packet already covers it, and consolidate a duplicate lock-scope comment.
However, four P2 inline comments were published, including:
- The repo-id validation request that the summary explicitly says to drop.
- A confirmation that the inspected SPDX headers do not leave stale comments or docstrings. This reports no defect or corrective action but is presented as a P2 finding.
- The accessor lock-scope finding and a related cache-hit coverage request, which also need reconciliation with the summary's consolidation decision.
The inconsistency is observable without deciding whether every underlying test request would independently be useful.
vllm-project/vllm-omni#6968 — resolved observation reappears in Findings
Review submitted on 2026-09-04 at 03:57:29 UTC, reviewed head:
b5322024bc6069a908e7ff709a60c02ec6a2ebc7
The validation section treats the TI2I guidance-mode concern as resolved by the request-unique key and fail-closed guard. The first item under Findings nevertheless presents that same resolved observation as P2, while its text says it is not being raised again. The selection narrative and the eight-item Findings list also disagree.
In this example, the findings are embedded in the review body, not bot-authored inline comments.
Expected behavior
- Final selection decisions must be reflected in every published representation: summary, category counts, Findings section, and inline comments.
- Discarded, refuted, duplicate, and resolved observations must not reappear as active findings. Useful negative checks can remain concise evidence notes.
- A statement confirming that no problem exists must not receive a finding priority or request contributor action.
- A separately evidenced residual defect may still be reported after the original concern is resolved, but it must clearly identify the remaining defect and required action. Do not implement this as a keyword filter for
resolved. - Verification limitations should be distinguished from confirmed defects, with an explicit reason when missing validation warrants an actionable request.
- Internal selection instructions should be applied before publication, rather than appearing alongside a contradictory candidate list.
A possible implementation is one finalized structured result with explicit finding dispositions, from which all output forms are rendered. The exact implementation should follow investigation of the existing pipeline.
Acceptance criteria
- Add regression fixtures based on these two published reviews, covering both body findings and inline-comment publication.
- Explicitly excluded requests and no-issue confirmations cannot become active P2 findings during finalization, conversion, or rendering.
- Final finding identities, priorities, counts, and dispositions agree across the summary and published comments.
- Deduplication preserves materially distinct, supported residual defects while removing observations already resolved or consolidated.
- Zero-actionable-finding results remain valid even when the review collected many observations.
- Tests exercise the finalized-result-to-ReviewBot-publication boundary, not only prompt wording.
Observation boundary and related work
Both reports record executed_mode=strict, backend=cursor, and infermatrix-copilot==0.2.0. They record reviewbot_sha=unknown and model=auto; the exact deployed implementation revision and model have not been established.
The public artifacts were rechecked on 2026-09-09. This report concerns output consistency, not a fresh correctness assessment of either PR or a claim that the findings blocked merging: both GitHub reviews are COMMENTED. No reviewed-PR tests were rerun and no source-level root cause has been identified. Investigate the finalization, result conversion, and publication boundary before attributing the defect to a particular component.
Related:
- RFC: Strict review for a strong generator — deep passes, per-comment verification, self-proving evidence #72 — Strict review and per-comment verification.
- Direct review should suppress findings already raised in PR feedback #76 — suppression of previously raised/resolved findings in Direct mode. This report concerns contradictions within the same published Strict review.
Activity
tzhouam commentedon Sep 11, 2026
The first reported example is fixed and deployed. The second has a different mechanism, now diagnosed and split out — so this stays open rather than being closed on half the evidence.
Fixed: selection was prose, publication read a list
A Strict review emitted two things that were never reconciled:
summary, the model's prose, where every selection decision lived; andreview_comments, the list publication actually reads._render_review_summaryappends the prose, the body renderer and the inline publisher render the list, and nothing compared them. That is why vllm-project/vllm-omni#6959 published inline the very repo-id request its summary said to drop, and published a "no stale docstrings" confirmation carrying a severity.Selection is data now. Every candidate carries
disposition—publish|excluded|duplicate|resolved|no_issue— applied at two deliberate points:no_issuekeeps its evidence and loses its ask: a "Checked, no defect found" note, no severity, no priority. A review that collected observations and has nothing to request stays a valid zero-finding APPROVE.Per the constraint in this issue, nothing reads prose: only the four explicit values withhold. An absent disposition publishes (additive field); an unrecognized one publishes too — losing a real finding to a typo is worse than publishing one whose bookkeeping we cannot read — but is recorded in
declaredso the anomaly is visible.The contract now carries
finding_dispositions({anchor, disposition, declared}per candidate), so a consumer can prove the published set is the finalized set without receiving the withheld prose. Strict API 1.0.0 → 1.1.0.ReviewBot enforces it as a second lock (JiusiServe/omni-reviewbot#62): a result whose published list contradicts its own finalized set is refused rather than posted. Three narrowings keep that from firing on healthy reviews —
over_budgetis not a withholding decision, an anchor recorded both ways is not a contradiction, and fileless findings are not anchor-matched.Still open: vllm-project/vllm-omni#6968 has a different cause
Not the same bug wearing a different hat.
_promote_resolved_residualsre-files a[resolved]findings line as aminorcomment when the line "states a residual", decided by substring match against_RESIDUAL_MARKERS— a list containing the bare conjunction"but ". A line shaped likematches, and is promoted into a finding whose own text says it is not being raised. The promoted comment inherits the whole findings line, so the confirmation clause travels with it.
That path bypasses dispositions entirely — it manufactures a comment after the reviewer has spoken — so the fix above does not touch it. Tightening it trades against a measured recall gain (~70% of ground truth on merged heads is "the fix landed, what does it still not cover?"), which is a decision worth measuring rather than guessing.
Filed as #155 with three options and the acceptance criteria.
Against this issue's checklist
🤖 Generated with Claude Code
https://claude.ai/code/session_01KT4EvUTzzLeq243DG6T1mC
tzhouam commentedon Sep 29, 2026
The first mechanism (explicitly dropped requests published inline) was fixed by #154 and deployed. The second mechanism is tracked in #155. Closing this in favour of #155.