Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/review-thumbs-seeding.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"review": minor
---

Thumbs-sweep: opt-in reaction seeding and self-reaction filtering. With `seedReactions: true` in the sweep config, each sweep seeds one thumbs-up and one thumbs-down (from the bot) on any reviewer comment lacking them, so readers see the feedback affordance without opening the reaction picker. Feedback counts now exclude the bot's own reactions everywhere, so seeded nudges never trigger follow-ups or count as signal; a reaction with no attributed login still counts as a real user's. The port gains an optional `addReactions` method, required only when seeding is enabled (enabling it without an implementation fails loudly).
124 changes: 123 additions & 1 deletion workflows/review/lib/thumbs-sweep.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,26 @@ const down = (): {content: string} => ({content: "-1"});
*/
class FakePort implements ThumbsSweepPort {
posted: PostedFollowup[] = [];
seededCalls: Array<{
grain: FeedbackGrain;
commentId: number;
contents: readonly string[];
}> = [];

constructor(
private readonly commentsByGrain: Record<FeedbackGrain, BotComment[]>,
private readonly existingFollowups: string[] = [],
) {}

addReactions(
grain: FeedbackGrain,
commentId: number,
contents: readonly string[],
): Promise<void> {
this.seededCalls.push({grain, commentId, contents});
return Promise.resolve();
}

listBotComments(grain: FeedbackGrain): Promise<BotComment[]> {
return Promise.resolve(this.commentsByGrain[grain] ?? []);
}
Expand All @@ -74,14 +88,122 @@ class FakePort implements ThumbsSweepPort {
const makeComment = (
grain: FeedbackGrain,
id: number,
reactions: Array<{content: string}> = [],
reactions: Array<{content: string; user?: string}> = [],
): BotComment => ({grain, id, reactions});

const noComments = (): Record<FeedbackGrain, BotComment[]> => ({
inline: [],
summary: [],
});

describe("reaction seeding and self-reaction filtering", () => {
const BOT = VALID_CONFIG.botLogin;
const seedingConfig: ThumbsSweepConfig = {
...VALID_CONFIG,
seedReactions: true,
};

it("seeds one thumbs-up and one thumbs-down on an unseeded comment", async () => {
const port = new FakePort({
inline: [makeComment("inline", 1)],
summary: [],
});
const result = await sweepThumbs(port, seedingConfig);
expect(port.seededCalls).toEqual([
{grain: "inline", commentId: 1, contents: ["+1", "-1"]},
]);
expect(result.reactionsSeeded).toBe(2);
});

it("is idempotent: an already-seeded comment gets nothing", async () => {
const port = new FakePort({
inline: [
makeComment("inline", 1, [
{content: "+1", user: BOT},
{content: "-1", user: BOT},
]),
],
summary: [],
});
const result = await sweepThumbs(port, seedingConfig);
expect(port.seededCalls).toEqual([]);
expect(result.reactionsSeeded).toBe(0);
});

it("adds only the missing seed when one is already placed", async () => {
const port = new FakePort({
inline: [makeComment("inline", 1, [{content: "+1", user: BOT}])],
summary: [],
});
await sweepThumbs(port, seedingConfig);
expect(port.seededCalls).toEqual([
{grain: "inline", commentId: 1, contents: ["-1"]},
]);
});

it("never seeds when seedReactions is off (the default)", async () => {
const port = new FakePort({
inline: [makeComment("inline", 1)],
summary: [],
});
const result = await sweepThumbs(port, VALID_CONFIG);
expect(port.seededCalls).toEqual([]);
expect(result.reactionsSeeded).toBe(0);
});

it("the bot's own seeded thumbs-down never triggers a follow-up", async () => {
const port = new FakePort({
inline: [makeComment("inline", 1, [{content: "-1", user: BOT}])],
summary: [],
});
const result = await sweepThumbs(port, seedingConfig);
expect(port.posted).toEqual([]);
expect(result.actions[0]?.downvotes).toBe(0);
expect(result.actions[0]?.reason).toBe("no-downvote");
});

it("a real user's thumbs-down still triggers alongside the bot's seeds", async () => {
const port = new FakePort({
inline: [
makeComment("inline", 1, [
{content: "+1", user: BOT},
{content: "-1", user: BOT},
{content: "-1", user: "a-real-dev"},
]),
],
summary: [],
});
const result = await sweepThumbs(port, seedingConfig);
expect(port.posted).toHaveLength(1);
expect(result.actions[0]?.downvotes).toBe(1);
});

it("a reaction with no login is treated as a real user's", async () => {
const port = new FakePort({
inline: [makeComment("inline", 1, [{content: "-1"}])],
summary: [],
});
const result = await sweepThumbs(port, VALID_CONFIG);
expect(result.actions[0]?.downvotes).toBe(1);
});

it("rejects a non-boolean seedReactions in config", () => {
const validation = validateSweepConfig({
...VALID_CONFIG,
seedReactions: "yes",
});
expect(validation.ok).toBe(false);
});

it("throws when seeding is enabled but the port lacks addReactions", async () => {
const port = new FakePort({inline: [], summary: []});
(port as {addReactions?: unknown}).addReactions = undefined;
await expect(sweepThumbs(port, seedingConfig)).rejects.toThrow(
/does not implement addReactions/,
);
});
});

describe("exported constants", () => {
it("FEEDBACK_GRAINS is exactly inline + summary", () => {
expect([...FEEDBACK_GRAINS]).toEqual(["inline", "summary"]);
Expand Down
87 changes: 80 additions & 7 deletions workflows/review/lib/thumbs-sweep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,16 @@ export type DownvoteReason = typeof DOWNVOTE_REASONS[number];
export type Reaction = {
/** GitHub reaction content, e.g. `"+1"` / `"-1"` (other emoji are ignored). */
content: string;
/** Reactor login — carried for audit/logging only; not used for idempotency. */
/**
* Reactor login. Used to exclude the bot's own seeded reactions from
* feedback counts; a reaction with no login is treated as a real user's.
*/
user?: string;
};

/** The two reactions the sweep can seed on a bot comment as a feedback nudge. */
export const SEED_REACTIONS = ["+1", "-1"] as const;

/** A bot-authored comment (at one grain) together with its current reactions. */
export type BotComment = {
grain: FeedbackGrain;
Expand All @@ -90,6 +96,13 @@ export type ThumbsSweepConfig = {
owner: string;
repo: string;
botLogin: string;
/**
* When true, the sweep seeds one 👍 and one 👎 (from the bot itself) on any
* of its comments that lack them, so readers see the feedback affordance.
* Seeded reactions are the bot's own and never count as feedback: every
* count in this module excludes reactions whose `user` is `botLogin`.
*/
seedReactions?: boolean;
};

/**
Expand All @@ -100,6 +113,16 @@ export type ThumbsSweepConfig = {
export interface ThumbsSweepPort {
/** Bot-authored comments at `grain`, each carrying its current reactions. */
listBotComments(grain: FeedbackGrain): Promise<BotComment[]>;
/**
* Add the given reactions (as the bot) to a comment. Only called when the
* config enables `seedReactions`, and only with reactions from
* {@link SEED_REACTIONS} that the bot has not already placed.
*/
addReactions?(
grain: FeedbackGrain,
commentId: number,
contents: readonly string[],
): Promise<void>;
/**
* Bodies of every follow-up already posted by prior sweeps. The sweep scans
* these for its marker to decide what has already been pinged — this is the
Expand Down Expand Up @@ -129,18 +152,22 @@ export type SweepActionReason =
export type SweepAction = {
grain: FeedbackGrain;
commentId: number;
/** Number of 👎 currently on the comment (0 when none). */
/** Number of 👎 currently on the comment (0 when none; the bot's own seeded 👎 never counts). */
downvotes: number;
/** Whether a follow-up was posted this sweep. */
posted: boolean;
reason: SweepActionReason;
/** Nudge reactions seeded on this comment this sweep (empty when none). */
seeded: readonly string[];
};

/** Aggregate outcome of one sweep. */
export type SweepResult = {
actions: SweepAction[];
/** Count of follow-ups posted this sweep (invariant: <= new-downvote count). */
followupsPosted: number;
/** Count of nudge reactions seeded this sweep (0 unless `seedReactions`). */
reactionsSeeded: number;
};

/**
Expand Down Expand Up @@ -207,12 +234,30 @@ export const renderFollowupBody = (
const key = (grain: FeedbackGrain, commentId: number): string =>
`${grain}:${commentId}`;

/** Count the negative reactions (👎/😕, per {@link NEGATIVE_REACTIONS}) on a comment. */
const countDownvotes = (comment: BotComment): number =>
comment.reactions.filter((r) =>
(NEGATIVE_REACTIONS as readonly string[]).includes(r.content),
/**
* Count the negative reactions (👎/😕, per {@link NEGATIVE_REACTIONS}) on a
* comment. The bot's own reactions (its seeded nudges) are never feedback.
*/
const countDownvotes = (comment: BotComment, botLogin: string): number =>
comment.reactions.filter(
(r) =>
r.user !== botLogin &&
(NEGATIVE_REACTIONS as readonly string[]).includes(r.content),
).length;

/** The seed reactions the bot has not yet placed on this comment. */
const missingSeeds = (
comment: BotComment,
botLogin: string,
): readonly string[] => {
const placed = new Set(
comment.reactions
.filter((r) => r.user === botLogin)
.map((r) => r.content),
);
return SEED_REACTIONS.filter((s) => !placed.has(s));
};

/**
* Validate a {@link ThumbsSweepConfig}. Returns every problem (like the finding
* validator) so a misconfigured deploy is fully diagnosable. Both consumer repos
Expand All @@ -238,6 +283,12 @@ export const validateSweepConfig = (input: unknown): ConfigValidation => {
if (!isNonEmptyString(cfg["repo"])) {
errors.push("repo: required non-empty string");
}
if (
cfg["seedReactions"] !== undefined &&
typeof cfg["seedReactions"] !== "boolean"
) {
errors.push("seedReactions: must be a boolean when present");
}
if (!isNonEmptyString(cfg["botLogin"])) {
errors.push("botLogin: required non-empty string");
}
Expand Down Expand Up @@ -286,10 +337,28 @@ export const sweepThumbs = async (

const actions: SweepAction[] = [];

const seedingEnabled = config.seedReactions === true;
if (seedingEnabled && typeof port.addReactions !== "function") {
throw new Error(
"seedReactions is enabled but the port does not implement addReactions",
);
}

for (const grain of FEEDBACK_GRAINS) {
const comments = await port.listBotComments(grain);
for (const comment of comments) {
const downvotes = countDownvotes(comment);
// Seed the feedback nudge (one 👍 + one 👎 from the bot) on any of
// its comments that lack them. Idempotent: only the missing seeds
// are added, and the counts below never see the bot's reactions.
let seeded: readonly string[] = [];
if (seedingEnabled) {
seeded = missingSeeds(comment, config.botLogin);
if (seeded.length > 0) {
await port.addReactions?.(grain, comment.id, seeded);
}
}

const downvotes = countDownvotes(comment, config.botLogin);

if (downvotes === 0) {
actions.push({
Expand All @@ -298,6 +367,7 @@ export const sweepThumbs = async (
downvotes,
posted: false,
reason: "no-downvote",
seeded,
});
continue;
}
Expand All @@ -310,6 +380,7 @@ export const sweepThumbs = async (
downvotes,
posted: false,
reason: "already-followed-up",
seeded,
});
continue;
}
Expand All @@ -327,12 +398,14 @@ export const sweepThumbs = async (
downvotes,
posted: true,
reason: "posted",
seeded,
});
}
}

return {
actions,
followupsPosted: actions.filter((a) => a.posted).length,
reactionsSeeded: actions.reduce((n, a) => n + a.seeded.length, 0),
};
};
Loading