Skip to content

orb(ai-review): 'AI reviewers agree' is never verified — consensus fires whenever BOTH have any blocker, not the same one (427 closes) #9074

Description

@JSONbored

The defect

consensusDefectOf (src/services/ai-review.ts ~1966-1984) returns a defect whenever:

a.blockers.length > 0 && b.blockers.length > 0

It never compares the two blocker texts. The doc comment two lines above states the justification — "Requiring two independent models to AGREE is itself the precision mechanism" — which the code does not implement.

Scenario: reviewer A says "SQL injection in src/db.ts"; reviewer B says "the new helper lacks a doc comment" (a nit it mislabelled). The function returns {title: "SQL injection in src/db.ts", confidence: min(a,b)} with split: false. The published close reason (src/queue/ai-review-orchestration.ts ~758) reads:

AI reviewers agree on a likely critical defect: SQL injection in src/db.ts

That is a false claim of agreement, posted publicly on a contributor's PR, as the reason their work was auto-closed under one-shot rules.

Blast radius (live ledger)

427 agent.action.close events carry AI reviewers agree on a likely critical defect. Every one of them asserted an agreement the code never checked.

The mitigation does not hold

The tie-break judge is the intended guard, but src/services/ai-review.ts ~2664-2690:

if (tieBreak.orderUnstable) { incr(...); console.warn(...); }
else if (tieBreak.verdict !== "inconclusive") { combined = mapDual...; }
// no else — `combined` keeps consensusDefectOf's naive result

When both judge calls return null (provider down, 429, config error — all of which runDualAiTieBreakJudgeCall ~2236 breaks on), the resolver returns {stable:false, verdict:"inconclusive", orderUnstable:false}. Neither branch fires, so the permissive consensus stands and the gate closes on it — built from two reviewers the system had already determined disagree. There is no metric and no log for this path (orderUnstable is explicitly false, so the order-instability counter does not fire either).

Compounding: blocker text is never validated against the diff

parseModelReview validates blockers with typeof x === "string" and a 6-item slice (~903). Nothing checks the cited file is in the PR, the line exists, or the symbol appears in the diff.

Meanwhile inlineFindings — the channel with zero gate authority — get real validation: the path must exist in the PR file set and the line must be a commentable RIGHT-side line (src/review/inline-comments-select.ts ~80-81).

So the validated channel cannot block, and the channel that auto-closes the PR is unvalidated. A hallucinated "src/auth/session.ts:214 dereferences user before the null check" on a PR that never touched src/auth/ becomes the verbatim public close reason.

Fix

  1. Require actual overlap before calling it consensus — normalized-token Jaccard or a shared file-path reference between a.blockers[0] and b.blockers[0]. No overlap → split (hold), not defect. Keep the tie-break judge as the promoter split→defect rather than a demoter.
  2. Distinguish "judge unavailable" from "judge said inconclusive." On unavailable, degrade to split/hold rather than keeping defect. Add loopover_ai_review_tiebreak_unavailable_total.
  3. Validate blocker references against the diff — extract path[:line] (the system prompt already mandates citing the file) and cross-check against changedPaths. A blocker citing a file absent from the diff → demote to nit, mirroring the existing demoteCiClaimBlockers / demoteEvidenceAbsenceBlockers machinery.
  4. Until (1) lands, soften the public wording — it should not assert agreement the system did not establish.

Acceptance

  • Two reviewers flagging unrelated defects produce a hold, not a close.
  • A blocker citing a file outside the diff never becomes a close reason.

Refs #8961, #8833.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.orbGittensory Orb related - maintainer self-hosting analytics.

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions