Skip to content

Commit 5a2bb24

Browse files
kartaa-bot[bot]kartaa-bot[bot]
committed
fix(review): honor codex reaction signals
1 parent d554d17 commit 5a2bb24

3 files changed

Lines changed: 184 additions & 33 deletions

File tree

‎internal/github/activity.go‎

Lines changed: 79 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,22 @@ query($owner: String!, $repo: String!, $pr: Int!) {
3535
totalCount
3636
}
3737
}
38+
eyesReactions: reactions(content: EYES, first: 100) {
39+
nodes {
40+
user {
41+
__typename
42+
login
43+
}
44+
}
45+
}
46+
thumbsUpReactions: reactions(content: THUMBS_UP, first: 100) {
47+
nodes {
48+
user {
49+
__typename
50+
login
51+
}
52+
}
53+
}
3854
reviewThreads(first: 100) {
3955
totalCount
4056
nodes {
@@ -86,7 +102,9 @@ type activityResponse struct {
86102
TotalCount int `json:"totalCount"`
87103
} `json:"reactors"`
88104
} `json:"reactionGroups"`
89-
ReviewThreads struct {
105+
EyesReactions reactionConnection `json:"eyesReactions"`
106+
ThumbsUpReactions reactionConnection `json:"thumbsUpReactions"`
107+
ReviewThreads struct {
90108
TotalCount int `json:"totalCount"`
91109
Nodes []struct {
92110
ID string `json:"id"`
@@ -118,6 +136,15 @@ type activityResponse struct {
118136
} `json:"repository"`
119137
}
120138

139+
type reactionConnection struct {
140+
Nodes []struct {
141+
User *struct {
142+
TypeName string `json:"__typename"`
143+
Login string `json:"login"`
144+
} `json:"user"`
145+
} `json:"nodes"`
146+
}
147+
121148
// threadEntry groups thread metadata for sorted fingerprinting.
122149
// Sorting IDs alone would mis-pair parallel slices.
123150
type threadEntry struct {
@@ -231,7 +258,10 @@ func (c *Client) ProbeActivity(ctx context.Context, owner, repo string, pr int)
231258
snap.PREditorLogin = pr_.Editor.Login
232259
snap.PREditorType = pr_.Editor.TypeName
233260
}
234-
snap.PRReviewSignal = classifyPRReviewSignal(pr_.Body, snap.PREditorType, snap.PREditorLogin)
261+
snap.PRReviewSignal = combinePRReviewSignals(
262+
classifyPRReviewSignal(pr_.Body, snap.PREditorType, snap.PREditorLogin),
263+
classifyPRReactionSignal(pr_.EyesReactions, pr_.ThumbsUpReactions),
264+
)
235265
if pr_.UpdatedAt != "" {
236266
snap.PRUpdatedAt, _ = time.Parse(time.RFC3339, pr_.UpdatedAt)
237267
}
@@ -283,6 +313,8 @@ func classifyPRReviewSignal(body, editorType, editorLogin string) domain.PRRevie
283313
if !isCodexBotEditor(editorType, editorLogin) {
284314
return domain.PRReviewSignalNone
285315
}
316+
foundReviewing := false
317+
foundApproved := false
286318
for _, line := range strings.Split(body, "\n") {
287319
line = strings.TrimSpace(line)
288320
line = strings.Trim(line, "-*#> \t")
@@ -291,15 +323,59 @@ func classifyPRReviewSignal(body, editorType, editorLogin string) domain.PRRevie
291323
}
292324
lower := strings.ToLower(line)
293325
if hasThumbsUpToken(line, lower) && isCompactReviewMarkerLine(line, lower, stripThumbsUpTokens) {
294-
return domain.PRReviewSignalApproved
326+
foundApproved = true
295327
}
296328
if hasEyesToken(line, lower) && isCompactReviewMarkerLine(line, lower, stripEyesTokens) {
329+
foundReviewing = true
330+
}
331+
}
332+
if foundReviewing {
333+
return domain.PRReviewSignalReviewing
334+
}
335+
if foundApproved {
336+
return domain.PRReviewSignalApproved
337+
}
338+
return domain.PRReviewSignalNone
339+
}
340+
341+
func classifyPRReactionSignal(eyes, thumbsUp reactionConnection) domain.PRReviewSignal {
342+
if hasCodexReaction(eyes) {
343+
return domain.PRReviewSignalReviewing
344+
}
345+
if hasCodexReaction(thumbsUp) {
346+
return domain.PRReviewSignalApproved
347+
}
348+
return domain.PRReviewSignalNone
349+
}
350+
351+
func combinePRReviewSignals(signals ...domain.PRReviewSignal) domain.PRReviewSignal {
352+
foundApproved := false
353+
for _, signal := range signals {
354+
switch signal {
355+
case domain.PRReviewSignalReviewing:
297356
return domain.PRReviewSignalReviewing
357+
case domain.PRReviewSignalApproved:
358+
foundApproved = true
298359
}
299360
}
361+
if foundApproved {
362+
return domain.PRReviewSignalApproved
363+
}
300364
return domain.PRReviewSignalNone
301365
}
302366

367+
func hasCodexReaction(reactions reactionConnection) bool {
368+
for _, reaction := range reactions.Nodes {
369+
if reaction.User == nil {
370+
continue
371+
}
372+
if isCodexBotEditor(reaction.User.TypeName, reaction.User.Login) {
373+
return true
374+
}
375+
}
376+
return false
377+
}
378+
303379
func isCodexBotEditor(typeName, login string) bool {
304380
normalizedLogin := strings.TrimSuffix(strings.ToLower(login), "[bot]")
305381
if normalizedLogin == "chatgpt-codex-connector" {

‎internal/github/activity_test.go‎

Lines changed: 102 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,13 @@ func TestClassifyPRReviewSignal(t *testing.T) {
218218
editorLogin: "chatgpt-codex-connector",
219219
want: domain.PRReviewSignalApproved,
220220
},
221+
{
222+
name: "codex editor eyes dominate earlier thumbs up",
223+
body: "Codex review complete 👍\nCodex review 👀",
224+
editorType: "Bot",
225+
editorLogin: "chatgpt-codex-connector",
226+
want: domain.PRReviewSignalReviewing,
227+
},
221228
{
222229
name: "non codex editor standalone thumbs up",
223230
body: "👍",
@@ -250,36 +257,106 @@ func TestClassifyPRReviewSignal(t *testing.T) {
250257
}
251258
}
252259

253-
func TestCanFastSettleReview(t *testing.T) {
254-
if !CanFastSettleReview(&domain.ActivitySnapshot{
255-
HeadSHA: "abc123",
256-
PRReviewSignal: domain.PRReviewSignalApproved,
257-
}) {
258-
t.Fatal("approved PR signal with no unresolved threads should fast-settle")
260+
func TestClassifyPRReactionSignal(t *testing.T) {
261+
if got := classifyPRReactionSignal(codexReactionConnection(), reactionConnection{}); got != domain.PRReviewSignalReviewing {
262+
t.Fatalf("eyes reaction signal = %q, want %q", got, domain.PRReviewSignalReviewing)
259263
}
260264

261-
if CanFastSettleReview(&domain.ActivitySnapshot{
262-
HeadSHA: "abc123",
263-
PRReviewSignal: domain.PRReviewSignalApproved,
264-
ThreadCount: 101,
265-
ThreadIDs: make([]string, 100),
266-
}) {
267-
t.Fatal("incomplete thread probe should block fast-settle")
265+
if got := classifyPRReactionSignal(reactionConnection{}, codexReactionConnection()); got != domain.PRReviewSignalApproved {
266+
t.Fatalf("thumbs-up reaction signal = %q, want %q", got, domain.PRReviewSignalApproved)
268267
}
269268

270-
if CanFastSettleReview(&domain.ActivitySnapshot{
271-
HeadSHA: "abc123",
272-
PRReviewSignal: domain.PRReviewSignalApproved,
273-
UnresolvedThreadCount: 1,
274-
}) {
275-
t.Fatal("unresolved threads should block fast-settle")
269+
if got := classifyPRReactionSignal(codexReactionConnection(), codexReactionConnection()); got != domain.PRReviewSignalReviewing {
270+
t.Fatalf("eyes should dominate thumbs-up reaction, got %q", got)
276271
}
277272

278-
if CanFastSettleReview(&domain.ActivitySnapshot{
279-
HeadSHA: "abc123",
280-
PRReviewSignal: domain.PRReviewSignalApproved,
281-
ReviewDecision: string(domain.ReviewChangesRequested),
282-
}) {
283-
t.Fatal("changes-requested review decision should block fast-settle")
273+
if got := classifyPRReactionSignal(humanReactionConnection(), codexReactionConnection()); got != domain.PRReviewSignalApproved {
274+
t.Fatalf("non-codex eyes reaction should not block codex thumbs-up, got %q", got)
275+
}
276+
277+
if got := combinePRReviewSignals(domain.PRReviewSignalApproved, domain.PRReviewSignalReviewing); got != domain.PRReviewSignalReviewing {
278+
t.Fatalf("eyes should dominate combined signals, got %q", got)
279+
}
280+
}
281+
282+
func TestCanFastSettleReview(t *testing.T) {
283+
tests := []struct {
284+
name string
285+
snap *domain.ActivitySnapshot
286+
want bool
287+
}{
288+
{
289+
name: "approved signal with no unresolved threads",
290+
snap: &domain.ActivitySnapshot{
291+
HeadSHA: "abc123",
292+
PRReviewSignal: domain.PRReviewSignalApproved,
293+
},
294+
want: true,
295+
},
296+
{
297+
name: "incomplete thread probe",
298+
snap: &domain.ActivitySnapshot{
299+
HeadSHA: "abc123",
300+
PRReviewSignal: domain.PRReviewSignalApproved,
301+
ThreadCount: 101,
302+
ThreadIDs: make([]string, 100),
303+
},
304+
want: false,
305+
},
306+
{
307+
name: "unresolved threads",
308+
snap: &domain.ActivitySnapshot{
309+
HeadSHA: "abc123",
310+
PRReviewSignal: domain.PRReviewSignalApproved,
311+
UnresolvedThreadCount: 1,
312+
},
313+
want: false,
314+
},
315+
{
316+
name: "changes requested decision",
317+
snap: &domain.ActivitySnapshot{
318+
HeadSHA: "abc123",
319+
PRReviewSignal: domain.PRReviewSignalApproved,
320+
ReviewDecision: string(domain.ReviewChangesRequested),
321+
},
322+
want: false,
323+
},
324+
}
325+
326+
for _, tt := range tests {
327+
t.Run(tt.name, func(t *testing.T) {
328+
if got := CanFastSettleReview(tt.snap); got != tt.want {
329+
t.Fatalf("CanFastSettleReview() = %v, want %v", got, tt.want)
330+
}
331+
})
332+
}
333+
}
334+
335+
func codexReactionConnection() reactionConnection {
336+
return reactionConnectionWithUser("Bot", "chatgpt-codex-connector")
337+
}
338+
339+
func humanReactionConnection() reactionConnection {
340+
return reactionConnectionWithUser("User", "alice")
341+
}
342+
343+
func reactionConnectionWithUser(typeName, login string) reactionConnection {
344+
return reactionConnection{
345+
Nodes: []struct {
346+
User *struct {
347+
TypeName string `json:"__typename"`
348+
Login string `json:"login"`
349+
} `json:"user"`
350+
}{
351+
{
352+
User: &struct {
353+
TypeName string `json:"__typename"`
354+
Login string `json:"login"`
355+
}{
356+
TypeName: typeName,
357+
Login: login,
358+
},
359+
},
360+
},
284361
}
285362
}

‎internal/github/watcher.go‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -238,11 +238,9 @@ func watchReviewsWithProbe(
238238
}
239239
now := clock()
240240
status := &domain.WatchStatus{
241-
Timestamp: now,
242-
OverallStatus: domain.StatusPass,
243-
ReviewPhase: domain.ReviewPhaseSettled,
244-
ReviewConfidence: domain.ReviewConfidenceMedium,
245-
Final: true,
241+
Timestamp: now,
242+
OverallStatus: domain.StatusPass,
243+
Final: true,
246244
}
247245
monitor := domain.NewReviewMonitor(
248246
domain.ReviewPhaseSettled,

0 commit comments

Comments
 (0)