Stop announcing a reviewer's agreement as a rebuttal

internal/crq/feedback.go

@@ -1158,22 +1158,31 @@ func threadRebuttal(thread reviewThread, bots map[string]struct{}) *dialect.Find
 	if !agentReplied {
 		return nil // the bot is talking to itself, not answering a decline
 	}
-	if dialect.IsReviewFindingWithdrawn(last.Body) {
+	verdict := dialect.ClassifyDeclineReply(last.Body)
+	if verdict == dialect.ReplyWithdrawn {
 		return nil // conceded — the decline stands
 	}
 	if dialect.IsNonActionableText(last.Body) {
 		return nil // a platform notice or ack, not a rebuttal (e.g. Codex's
 		// "create an environment" boilerplate posted as a thread reply)
 	}
-	// A contested decline deserves attention even when the finding's own severity
-	// is a nitpick, so floor an unknown severity at major.
+	// The verdict decides how this reads. Everything that was not a clear
+	// withdrawal used to be announced as a contest, so a bot AGREEING with the
+	// decline was reported as standing its ground — and an agent, told to
+	// re-address a rebuttal that did not exist, looped on the artifact.
+	//
+	// Surfacing an unclear reply is still right — a buried rebuttal is the worse
+	// failure — and it keeps the same major floor, so visibility is unchanged.
+	// Only the CLAIM is corrected, and the wording for it lives with the
+	// classifier rather than here.
 	severity := dialect.FloorSeverity(dialect.SeverityOf(last.Body), "major")
+	title := verdict.TitlePrefix()
 	return &dialect.Finding{
 		Bot:       last.Author.Login,
 		Severity:  severity,
 		Path:      firstNonEmpty(thread.Path, last.Path),
 		Line:      firstPositive(thread.Line, last.Line, last.OriginalLine),
-		Title:     "Reviewer contests your reply — re-address or reply again: " + dialect.TitleOf(last.Body),
+		Title:     title + dialect.TitleOf(last.Body),
 		Body:      strings.TrimSpace(last.Body),
 		ThreadID:  thread.ID,
 		CommentID: last.DatabaseID,

internal/crq/feedback_test.go

@@ -1931,6 +1931,8 @@ func TestThreadRebuttalSurfacesContestedResolvedThreads(t *testing.T) {
 		t.Fatal("a contested bot reply on a resolved thread must surface")
 	} else if got.Source != "review_reply" || got.ThreadID != "PRRT_x" || got.CommentID != 3 {
 		t.Fatalf("rebuttal finding mismatch: %#v", got)
+	} else if !strings.Contains(got.Title, "contests your reply") {
+		t.Errorf("a reply that really retains the finding must be reported as a contest, got %q", got.Title)
 	}
 
 	// Withdrawn rebuttal → not surfaced.
@@ -1947,10 +1949,18 @@ func TestThreadRebuttalSurfacesContestedResolvedThreads(t *testing.T) {
 	addThreadComment(&th, 1, "coderabbitai", "**Nitpick** rename this")
 	addThreadComment(&th, 2, "kristofferR", "Declined: name is intentional.")
 	addThreadComment(&th, 3, "coderabbitai", "Here is some additional context on the naming convention.")
+	// It surfaces at the same severity — never bury a possible rebuttal — but it
+	// must NOT be announced as a contest. Asserting one for every non-withdrawal
+	// reply meant a bot AGREEING with the decline read as standing its ground,
+	// and an agent told to re-address a rebuttal that did not exist looped on it.
 	if got := threadRebuttal(th, bots); got == nil {
 		t.Fatal("an ambiguous (non-withdrawal) reply must surface by default")
 	} else if got.Severity != "major" {
 		t.Fatalf("an unknown-severity rebuttal must floor at major, got %q", got.Severity)
+	} else if strings.Contains(got.Title, "contests your reply") {
+		t.Errorf("an ambiguous reply must not be asserted as a contest, got %q", got.Title)
+	} else if !strings.Contains(got.Title, "read it") {
+		t.Errorf("an ambiguous reply should tell the caller to read it, got %q", got.Title)
 	}
 
 	// No agent reply (just the bot's finding) → not a rebuttal.

internal/crq/replay_test.go

@@ -706,3 +706,68 @@ func TestReplayEmptyReviewShellDoesNotConverge(t *testing.T) {
 		t.Fatalf("real review must converge: %+v", report.ReviewedBy)
 	}
 }
+
+// --- a refusal is not a rate limit -----------------------------------------
+
+// TestReplaySkippedRefusalNeverBlocksTheAccount pins the fleet-wide property
+// behind CodeRabbit's "Review skipped" refusal.
+//
+// That notice ships with the rate-limit marker embedded, and reading it as a
+// timed block was catastrophic rather than merely wrong: the block is
+// account-WIDE, nothing about waiting clears "this PR has 119 files", and every
+// re-fire re-read the same notice and re-blocked. One oversized PR stalled review
+// for every repository.
+//
+// The classifier discriminates them (TestGoldenClassification), but the cost of
+// a regression lands here, so assert the consequence and not just the reading:
+// after observing the refusal, no account block exists and an unrelated PR still
+// fires.
+func TestReplaySkippedRefusalNeverBlocksTheAccount(t *testing.T) {
+	base := time.Date(2026, 7, 26, 9, 0, 0, 0, time.UTC)
+	f := newReplayFixture(t, base)
+	oversized, other := 700, 701
+	f.openPull("owner/repo", oversized, "aaaaaaaa1")
+	f.setCommitDate("aaaaaaaa1", base.Add(-time.Minute))
+	f.openPull("owner/repo", other, "bbbbbbbb2")
+	f.setCommitDate("bbbbbbbb2", base.Add(-time.Minute))
+
+	f.enqueue("owner/repo", oversized)
+	if res := f.pump(); res.Action != "fired" {
+		t.Fatalf("the oversized PR should fire once, got %#v", res)
+	}
+
+	// CodeRabbit refuses the head. The body carries the rate-limit marker.
+	f.clk.advance(time.Minute)
+	f.botComment("owner/repo", oversized, 900,
+		corpusMessage(t, "coderabbit/review-skipped-too-many-files.md"), f.clk.now())
+	f.pump()
+
+	// Prove the refusal was actually observed. Without this the assertions below
+	// would also pass if nothing had happened at all — and "no block" is only
+	// meaningful once the notice that used to create one has been read.
+	if r := f.round("owner/repo", oversized); r == nil || r.Phase == PhaseFired {
+		t.Fatalf("the refusal must move the round off fired, got %#v", r)
+	}
+
+	st, _, err := f.store.Load(f.ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if st.Account.BlockedUntil != nil {
+		t.Fatalf("a refusal must not block the account: BlockedUntil = %s", st.Account.BlockedUntil)
+	}
+
+	// And the block being absent has to mean something: an unrelated PR fires.
+	f.clk.advance(2 * time.Minute)
+	f.enqueue("owner/repo", other)
+	for i := 0; i < 4; i++ {
+		if f.reviewsPosted("owner/repo", other) > 0 {
+			break
+		}
+		f.clk.advance(time.Minute)
+		f.pump()
+	}
+	if got := f.reviewsPosted("owner/repo", other); got == 0 {
+		t.Fatal("an unrelated PR must still fire: one oversized PR cannot stall the fleet")
+	}
+}

internal/dialect/golden_test.go

@@ -257,6 +257,18 @@ func TestGoldenReplyVerdict(t *testing.T) {
 			if got := IsReviewFindingRetained(body); got != tc.retained {
 				t.Errorf("IsReviewFindingRetained = %v, want %v", got, tc.retained)
 			}
+			// The verdict is what callers act on, and its wording travels with it:
+			// only a stated rebuttal may claim to contest the decline.
+			verdict := ClassifyDeclineReply(body)
+			switch {
+			case tc.withdrawn && verdict != ReplyWithdrawn:
+				t.Errorf("ClassifyDeclineReply = %v, want ReplyWithdrawn", verdict)
+			case tc.retained && verdict != ReplyRetained:
+				t.Errorf("ClassifyDeclineReply = %v, want ReplyRetained", verdict)
+			}
+			if contests := strings.Contains(verdict.TitlePrefix(), "contests"); contests != (verdict == ReplyRetained) {
+				t.Errorf("TitlePrefix for %v claims contest=%v", verdict, contests)
+			}
 		})
 	}
 }

internal/dialect/reply.go

@@ -16,6 +16,46 @@ import "strings"
 // a rebuttal re-surfaces a settled finding and blocks convergence.
 const reviewWithdrawnMarker = "<review_comment_withdrawn>"
 
+// ReplyVerdict is what a bot's reply to a declined finding amounts to. It exists
+// so the CLASSIFICATION and the wording that describes it stay together: the
+// caller orchestrates, and does not decide how a verdict reads.
+type ReplyVerdict int
+
+const (
+	// ReplyUnclear is neither a withdrawal nor a stated rebuttal. It is still
+	// surfaced — a buried rebuttal is the worse failure — but it must not be
+	// announced as a contest, which is what made an agent re-address a rebuttal
+	// that did not exist.
+	ReplyUnclear ReplyVerdict = iota
+	// ReplyWithdrawn concedes: the decline stands and the thread is done.
+	ReplyWithdrawn
+	// ReplyRetained contests: the finding stands and the agent must answer it.
+	ReplyRetained
+)
+
+// ClassifyDeclineReply reads a bot's reply to a declined finding.
+func ClassifyDeclineReply(text string) ReplyVerdict {
+	switch {
+	case IsReviewFindingWithdrawn(text):
+		return ReplyWithdrawn
+	case IsReviewFindingRetained(text):
+		return ReplyRetained
+	default:
+		return ReplyUnclear
+	}
+}
+
+// TitlePrefix is how a verdict introduces itself in a finding title. Only a
+// stated rebuttal claims to contest; anything else asks the caller to read it.
+func (v ReplyVerdict) TitlePrefix() string {
+	switch v {
+	case ReplyRetained:
+		return "Reviewer contests your reply — re-address or reply again: "
+	default:
+		return "Reviewer replied after your decline — read it and confirm the decline stands: "
+	}
+}
+
 // IsReviewFindingWithdrawn reports whether a bot's reply concedes and withdraws
 // its finding — the agent's decline stands and the thread is done.
 func IsReviewFindingWithdrawn(text string) bool {