From 5191f8d4f674767f4f18df67c0ce76ea73d2b596 Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Wed, 8 Jul 2026 16:56:59 -0700 Subject: [PATCH 1/5] review: thumbs-sweep reaction seeding (opt-in) and self-reaction filtering --- .changeset/review-thumbs-seeding.md | 5 + workflows/review/lib/thumbs-sweep.test.ts | 121 +++++++++++++++++++++- workflows/review/lib/thumbs-sweep.ts | 87 ++++++++++++++-- 3 files changed, 205 insertions(+), 8 deletions(-) create mode 100644 .changeset/review-thumbs-seeding.md diff --git a/.changeset/review-thumbs-seeding.md b/.changeset/review-thumbs-seeding.md new file mode 100644 index 00000000..eed3b2d5 --- /dev/null +++ b/.changeset/review-thumbs-seeding.md @@ -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). diff --git a/workflows/review/lib/thumbs-sweep.test.ts b/workflows/review/lib/thumbs-sweep.test.ts index 759b82e8..82d249e4 100644 --- a/workflows/review/lib/thumbs-sweep.test.ts +++ b/workflows/review/lib/thumbs-sweep.test.ts @@ -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, private readonly existingFollowups: string[] = [], ) {} + addReactions( + grain: FeedbackGrain, + commentId: number, + contents: readonly string[], + ): Promise { + this.seededCalls.push({grain, commentId, contents}); + return Promise.resolve(); + } + listBotComments(grain: FeedbackGrain): Promise { return Promise.resolve(this.commentsByGrain[grain] ?? []); } @@ -74,7 +88,7 @@ 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 => ({ @@ -82,6 +96,111 @@ const noComments = (): Record => ({ 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"]); diff --git a/workflows/review/lib/thumbs-sweep.ts b/workflows/review/lib/thumbs-sweep.ts index dc554d77..c2b63640 100644 --- a/workflows/review/lib/thumbs-sweep.ts +++ b/workflows/review/lib/thumbs-sweep.ts @@ -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; @@ -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; }; /** @@ -100,6 +113,16 @@ export type ThumbsSweepConfig = { export interface ThumbsSweepPort { /** Bot-authored comments at `grain`, each carrying its current reactions. */ listBotComments(grain: FeedbackGrain): Promise; + /** + * 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; /** * 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 @@ -129,11 +152,13 @@ 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. */ @@ -141,6 +166,8 @@ 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; }; /** @@ -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 @@ -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"); } @@ -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({ @@ -298,6 +367,7 @@ export const sweepThumbs = async ( downvotes, posted: false, reason: "no-downvote", + seeded, }); continue; } @@ -310,6 +380,7 @@ export const sweepThumbs = async ( downvotes, posted: false, reason: "already-followed-up", + seeded, }); continue; } @@ -327,6 +398,7 @@ export const sweepThumbs = async ( downvotes, posted: true, reason: "posted", + seeded, }); } } @@ -334,5 +406,6 @@ export const sweepThumbs = async ( return { actions, followupsPosted: actions.filter((a) => a.posted).length, + reactionsSeeded: actions.reduce((n, a) => n + a.seeded.length, 0), }; }; From ea3201495847502037e6ccea47e5a2b09cc501ea Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Wed, 8 Jul 2026 17:21:12 -0700 Subject: [PATCH 2/5] review: octokit thumbs-sweep port, sweep CLI, and live-counters report The production wiring for the review-feedback signal (design doc, 'The tuning loop' -> 'Signal to turn on'): - thumbs-sweep-github.ts: the octokit-backed ThumbsSweepPort. Reviewer comments are identified per grain (risks/patterns marker for summary comments; the code-owned Conventional-Comment label prefixes for inline ones, which carry no hidden marker at current releases), with per-reactor logins so seeded reactions never count as signal. Traversal is bounded to PRs updated in the last 14 days (capped) and every API request is counted for the auditable job summary. - run-thumbs-sweep.ts: env-configured CLI for the consumer repos' scheduled sweep workflows; writes the SweepResult JSON to stdout and a Markdown digest to GITHUB_STEP_SUMMARY. - counters-report.ts: aggregates downloaded per-run artifacts into the weekly live-counters job summary, synthesizing the run summary from gh-aw engine artifacts (safeoutputs.jsonl, safe-output-items.jsonl, agent_usage.json) when summary.json is absent, tolerating the known staging-path nesting. - counters.ts: accept the production validator artifact shape ({"claims": [...]} with three-state verification) and an optional fallback source for unjoinable decisions. - The review package gains its first pinned runtime dependency (octokit); consumer sweep workflows npm-install it in the checkout. No review semantics change. --- .changeset/review-feedback-signal.md | 5 + package.json | 1 + pnpm-lock.yaml | 324 +++++++++++- workflows/review/README.md | 29 ++ workflows/review/lib/counters-report.test.ts | 220 ++++++++ workflows/review/lib/counters-report.ts | 368 +++++++++++++ workflows/review/lib/counters.ts | 62 ++- workflows/review/lib/run-thumbs-sweep.ts | 157 ++++++ .../review/lib/thumbs-sweep-github.test.ts | 286 +++++++++++ workflows/review/lib/thumbs-sweep-github.ts | 483 ++++++++++++++++++ workflows/review/package.json | 5 +- 11 files changed, 1924 insertions(+), 16 deletions(-) create mode 100644 .changeset/review-feedback-signal.md create mode 100644 workflows/review/lib/counters-report.test.ts create mode 100644 workflows/review/lib/counters-report.ts create mode 100644 workflows/review/lib/run-thumbs-sweep.ts create mode 100644 workflows/review/lib/thumbs-sweep-github.test.ts create mode 100644 workflows/review/lib/thumbs-sweep-github.ts diff --git a/.changeset/review-feedback-signal.md b/.changeset/review-feedback-signal.md new file mode 100644 index 00000000..e83b0218 --- /dev/null +++ b/.changeset/review-feedback-signal.md @@ -0,0 +1,5 @@ +--- +"review": minor +--- + +Turn on the review-feedback signal: the production GitHub wiring for the thumbs sweep and the live counters. `thumbs-sweep-github.ts` is the octokit-backed `ThumbsSweepPort` (reviewer comments identified per grain — the risks/patterns marker for summary comments, the code-owned Conventional-Comment label taxonomy for inline ones — with per-reactor logins, bounded to PRs updated in the last 14 days), and `run-thumbs-sweep.ts` is the env-configured CLI the consumer repos' scheduled sweep workflows run. `counters-report.ts` aggregates a directory of downloaded per-run artifacts into the live-counters job summary, synthesizing the run summary from the gh-aw engine artifacts (`safeoutputs.jsonl`, `safe-output-items.jsonl`, `agent_usage.json`) when `summary.json` is absent, and tolerating the known gh-aw artifact staging-path nesting. `counters.ts` now also understands the production validator artifact shape (`{"claims": […]}` with three-state `verification`) and can attribute unjoinable decisions to a fallback source. The `review` package gains its first runtime dependency, `octokit` (pinned exactly); consumer sweep workflows install it with `npm install --omit=dev` in the checked-out `workflows/review` directory. No review semantics change. diff --git a/package.json b/package.json index f81bb81a..ea78e395 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "eslint-plugin-prettier": "^4.0.0", "fast-glob": "^3.3.3", "memfs": "^4.51.0", + "octokit": "^5.0.5", "prettier": "^2.6.2", "typescript": "^5.9.3", "vitest": "^4.0.10" diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f679e932..bb87254b 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -47,6 +47,9 @@ importers: memfs: specifier: ^4.51.0 version: 4.51.0 + octokit: + specifier: ^5.0.5 + version: 5.0.5 prettier: specifier: ^2.6.2 version: 2.6.2 @@ -104,7 +107,11 @@ importers: actions/shared-node-cache: {} - workflows/review: {} + workflows/review: + dependencies: + octokit: + specifier: 5.0.5 + version: 5.0.5 packages: @@ -567,6 +574,113 @@ packages: resolution: {integrity: sha512-oGB+UxlgWcgQkgwo8GcEGwemoTFt3FIO9ababBmaGwXIoBKZ+GTy0pP185beGg7Llih/NSHSV2XAs1lnznocSg==} engines: {node: '>= 8'} + '@octokit/app@16.1.2': + resolution: {integrity: sha512-8j7sEpUYVj18dxvh0KWj6W/l6uAiVRBl1JBDVRqH1VHKAO/G5eRVl4yEoYACjakWers1DjUkcCHyJNQK47JqyQ==} + engines: {node: '>= 20'} + + '@octokit/auth-app@8.2.0': + resolution: {integrity: sha512-vVjdtQQwomrZ4V46B9LaCsxsySxGoHsyw6IYBov/TqJVROrlYdyNgw5q6tQbB7KZt53v1l1W53RiqTvpzL907g==} + engines: {node: '>= 20'} + + '@octokit/auth-oauth-app@9.0.3': + resolution: {integrity: sha512-+yoFQquaF8OxJSxTb7rnytBIC2ZLbLqA/yb71I4ZXT9+Slw4TziV9j/kyGhUFRRTF2+7WlnIWsePZCWHs+OGjg==} + engines: {node: '>= 20'} + + '@octokit/auth-oauth-device@8.0.3': + resolution: {integrity: sha512-zh2W0mKKMh/VWZhSqlaCzY7qFyrgd9oTWmTmHaXnHNeQRCZr/CXy2jCgHo4e4dJVTiuxP5dLa0YM5p5QVhJHbw==} + engines: {node: '>= 20'} + + '@octokit/auth-oauth-user@6.0.2': + resolution: {integrity: sha512-qLoPPc6E6GJoz3XeDG/pnDhJpTkODTGG4kY0/Py154i/I003O9NazkrwJwRuzgCalhzyIeWQ+6MDvkUmKXjg/A==} + engines: {node: '>= 20'} + + '@octokit/auth-token@6.0.0': + resolution: {integrity: sha512-P4YJBPdPSpWTQ1NU4XYdvHvXJJDxM6YwpS0FZHRgP7YFkdVxsWcpWGy/NVqlAA7PcPCnMacXlRm1y2PFZRWL/w==} + engines: {node: '>= 20'} + + '@octokit/auth-unauthenticated@7.0.3': + resolution: {integrity: sha512-8Jb1mtUdmBHL7lGmop9mU9ArMRUTRhg8vp0T1VtZ4yd9vEm3zcLwmjQkhNEduKawOOORie61xhtYIhTDN+ZQ3g==} + engines: {node: '>= 20'} + + '@octokit/core@7.0.6': + resolution: {integrity: sha512-DhGl4xMVFGVIyMwswXeyzdL4uXD5OGILGX5N8Y+f6W7LhC1Ze2poSNrkF/fedpVDHEEZ+PHFW0vL14I+mm8K3Q==} + engines: {node: '>= 20'} + + '@octokit/endpoint@11.0.3': + resolution: {integrity: sha512-FWFlNxghg4HrXkD3ifYbS/IdL/mDHjh9QcsNyhQjN8dplUoZbejsdpmuqdA76nxj2xoWPs7p8uX2SNr9rYu0Ag==} + engines: {node: '>= 20'} + + '@octokit/graphql@9.0.3': + resolution: {integrity: sha512-grAEuupr/C1rALFnXTv6ZQhFuL1D8G5y8CN04RgrO4FIPMrtm+mcZzFG7dcBm+nq+1ppNixu+Jd78aeJOYxlGA==} + engines: {node: '>= 20'} + + '@octokit/oauth-app@8.0.3': + resolution: {integrity: sha512-jnAjvTsPepyUaMu9e69hYBuozEPgYqP4Z3UnpmvoIzHDpf8EXDGvTY1l1jK0RsZ194oRd+k6Hm13oRU8EoDFwg==} + engines: {node: '>= 20'} + + '@octokit/oauth-authorization-url@8.0.0': + resolution: {integrity: sha512-7QoLPRh/ssEA/HuHBHdVdSgF8xNLz/Bc5m9fZkArJE5bb6NmVkDm3anKxXPmN1zh6b5WKZPRr3697xKT/yM3qQ==} + engines: {node: '>= 20'} + + '@octokit/oauth-methods@6.0.2': + resolution: {integrity: sha512-HiNOO3MqLxlt5Da5bZbLV8Zarnphi4y9XehrbaFMkcoJ+FL7sMxH/UlUsCVxpddVu4qvNDrBdaTVE2o4ITK8ng==} + engines: {node: '>= 20'} + + '@octokit/openapi-types@27.0.0': + resolution: {integrity: sha512-whrdktVs1h6gtR+09+QsNk2+FO+49j6ga1c55YZudfEG+oKJVvJLQi3zkOm5JjiUXAagWK2tI2kTGKJ2Ys7MGA==} + + '@octokit/openapi-webhooks-types@12.1.0': + resolution: {integrity: sha512-WiuzhOsiOvb7W3Pvmhf8d2C6qaLHXrWiLBP4nJ/4kydu+wpagV5Fkz9RfQwV2afYzv3PB+3xYgp4mAdNGjDprA==} + + '@octokit/plugin-paginate-graphql@6.0.0': + resolution: {integrity: sha512-crfpnIoFiBtRkvPqOyLOsw12XsveYuY2ieP6uYDosoUegBJpSVxGwut9sxUgFFcll3VTOTqpUf8yGd8x1OmAkQ==} + engines: {node: '>= 20'} + peerDependencies: + '@octokit/core': '>=6' + + '@octokit/plugin-paginate-rest@14.0.0': + resolution: {integrity: sha512-fNVRE7ufJiAA3XUrha2omTA39M6IXIc6GIZLvlbsm8QOQCYvpq/LkMNGyFlB1d8hTDzsAXa3OKtybdMAYsV/fw==} + engines: {node: '>= 20'} + peerDependencies: + '@octokit/core': '>=6' + + '@octokit/plugin-rest-endpoint-methods@17.0.0': + resolution: {integrity: sha512-B5yCyIlOJFPqUUeiD0cnBJwWJO8lkJs5d8+ze9QDP6SvfiXSz1BF+91+0MeI1d2yxgOhU/O+CvtiZ9jSkHhFAw==} + engines: {node: '>= 20'} + peerDependencies: + '@octokit/core': '>=6' + + '@octokit/plugin-retry@8.1.0': + resolution: {integrity: sha512-O1FZgXeiGb2sowEr/hYTr6YunGdSAFWnr2fyW39Ah85H8O33ELASQxcvOFF5LE6Tjekcyu2ms4qAzJVhSaJxTw==} + engines: {node: '>= 20'} + peerDependencies: + '@octokit/core': '>=7' + + '@octokit/plugin-throttling@11.0.3': + resolution: {integrity: sha512-34eE0RkFCKycLl2D2kq7W+LovheM/ex3AwZCYN8udpi6bxsyjZidb2McXs69hZhLmJlDqTSP8cH+jSRpiaijBg==} + engines: {node: '>= 20'} + peerDependencies: + '@octokit/core': ^7.0.0 + + '@octokit/request-error@7.1.0': + resolution: {integrity: sha512-KMQIfq5sOPpkQYajXHwnhjCC0slzCNScLHs9JafXc4RAJI+9f+jNDlBNaIMTvazOPLgb4BnlhGJOTbnN0wIjPw==} + engines: {node: '>= 20'} + + '@octokit/request@10.0.11': + resolution: {integrity: sha512-+s7HUxjfFqOMS9VlIwDffq0MikjSAK0gSpG73W+meAvVAvX4MBrHYTK5Bj3Uot55qFT4gzUtfzE4mGWY4Br8/Q==} + engines: {node: '>= 20'} + + '@octokit/types@16.0.0': + resolution: {integrity: sha512-sKq+9r1Mm4efXW1FCk7hFSeJo4QKreL/tTbR0rz/qx/r1Oa2VV83LTA/H/MuCOX7uCIJmQVRKBcbmWoySjAnSg==} + + '@octokit/webhooks-methods@6.0.0': + resolution: {integrity: sha512-MFlzzoDJVw/GcbfzVC1RLR36QqkTLUf79vLVO3D+xn7r0QgxnFoLZgtrzxiQErAjFUOdH6fas2KeQJ1yr/qaXQ==} + engines: {node: '>= 20'} + + '@octokit/webhooks@14.2.0': + resolution: {integrity: sha512-da6KbdNCV5sr1/txD896V+6W0iamFWrvVl8cHkBSPT+YlvmT3DwXa4jxZnQc+gnuTEqSWbBeoSZYTayXH9wXcw==} + engines: {node: '>= 20'} + '@oxc-resolver/binding-android-arm-eabi@11.19.1': resolution: {integrity: sha512-aUs47y+xyXHUKlbhqHUjBABjvycq6YSD7bpxSW7vplUmdzAlJ93yXY6ZR0c1o1x5A/QKbENCvs3+NlY8IpIVzg==} cpu: [arm] @@ -877,6 +991,9 @@ packages: '@tybys/wasm-util@0.10.1': resolution: {integrity: sha512-9tTaPJLSiejZKx+Bmog4uSubteqTvFrVrURwkmHixBo0G4seD0zUxp98E1DzUBJxLQ3NPwXrGKDiVjwx/DpPsg==} + '@types/aws-lambda@8.10.162': + resolution: {integrity: sha512-Fn658grtLOci1oxi1391vvDWJRKNGWRSqfxRkmN/Iy3c0tQH1USMKEXcPYHLvope+ZgTFocx9FRQJx1muBL6qw==} + '@types/chai@5.2.3': resolution: {integrity: sha512-Mw558oeA9fFbv65/y4mHtXDs9bPnFMZAL/jxdPFUpOHHIXX91mcgEHbS5Lahr+pwZFR8A7GQleRWeI6cGFC2UA==} @@ -1124,10 +1241,16 @@ packages: base64-js@1.5.1: resolution: {integrity: sha512-AKpaYlHn8t4SVbOHCy+b5+KKgvR4vrsD8vbvrbiQJps7fKDTkjkDry6ji0rUJjC0kzbNePLwzxq8iypo41qeWA==} + before-after-hook@4.0.0: + resolution: {integrity: sha512-q6tR3RPqIB1pMiTRMFcZwuG5T8vwp+vUvEG0vuI6B+Rikh5BfPp2fQ82c925FOs+b0lcFQ8CFrL+KbilfZFhOQ==} + better-path-resolve@1.0.0: resolution: {integrity: sha512-pbnl5XzGBdrFU/wT4jqmJVPn2B6UHPBOhzMQkY/SPUPB6QtUXtmBHBIwCbXJol93mOpGMnQyP/+BB19q04xj7g==} engines: {node: '>=4'} + bottleneck@2.19.5: + resolution: {integrity: sha512-VHiNCbI1lKdl44tGrhNfU3lup0Tj/ZBMJB5/2ZbNXRCPuRCO7ed2mgcK4r17y+KB2EfuYuRaVlwNbAeaWGSpbw==} + brace-expansion@1.1.11: resolution: {integrity: sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==} @@ -1224,6 +1347,10 @@ packages: concat-map@0.0.1: resolution: {integrity: sha512-/Srv4dswyQNBfohGpz9o6Yb3Gz3SrUDqBH5rTuhGR7ahtlbYKnVxw2bCFMRljaA7EXHaXZ8wsHdodFvbkhKmqg==} + content-type@2.0.0: + resolution: {integrity: sha512-j/O/d7GcZCyNl7/hwZAb606rzqkyvaDctLmckbxLzHvFBzTJHuGEdodATcP3yIRoDrLHkIATJuvzbFlp/ki2cQ==} + engines: {node: '>=18'} + cosmiconfig-toml-loader@1.0.0: resolution: {integrity: sha512-H/2gurFWVi7xXvCyvsWRLCMekl4tITJcX0QEsDMpzxtuxDyM59xLatYNg4s/k9AA/HdtCYfj2su8mgA0GSDLDA==} @@ -2062,6 +2189,9 @@ packages: json-stable-stringify-without-jsonify@1.0.1: resolution: {integrity: sha512-Bdboy+l7tA3OGW6FjyFHWkP5LuByj1Tk33Ljyq0axyzdk9//JSi2u3fP1QSmd1KNwq6VOKYGlAu87CisVir6Pw==} + json-with-bigint@3.5.8: + resolution: {integrity: sha512-eq/4KP6K34kwa7TcFdtvnftvHCD9KvHOGGICWwMFc4dOOKF5t4iYqnfLK8otCRCRv06FXOzGGyqE8h8ElMvvdw==} + json5@1.0.2: resolution: {integrity: sha512-g1MWMLBiz8FKi1e4w0UyVL3w+iJceWAFBAaBnnGKOpNa5f8TLktkbre1+s6oICydWAm+HRUGTmI+//xv2hvXYA==} hasBin: true @@ -2289,6 +2419,10 @@ packages: obug@2.1.1: resolution: {integrity: sha512-uTqF9MuPraAQ+IsnPf366RG4cP9RtUi7MLO1N3KEc+wb0a6yKpeL0lmk2IB1jY5KHPAlTc6T/JRdC/YqxHNwkQ==} + octokit@5.0.5: + resolution: {integrity: sha512-4+/OFSqOjoyULo7eN7EA97DE0Xydj/PW5aIckxqQIoFjFwqXKuFCvXUJObyJfBF9Khu4RL/jlDRI9FPaMGfPnw==} + engines: {node: '>= 20'} + once@1.4.0: resolution: {integrity: sha512-lNaJgI+2Q5URQBkccEKHTQOPaXdUxnZZElQTZY0MFUAuaEqe1E+Nyvgdz/aIyNi6Z9MzO5dv1H8n58/GELp3+w==} @@ -2765,6 +2899,10 @@ packages: resolution: {integrity: sha512-65P7iz6X5yEr1cwcgvQxbbIw7Uk3gOy5dIdtZ4rDveLqhrdJP+Li/Hx6tyK0NEb+2GCyneCMJiGqrADCSNk8sQ==} engines: {node: '>=8.0'} + toad-cache@3.7.4: + resolution: {integrity: sha512-m1TdR/rvT7kgGJZhspNtXdsdYk0fddFpJJFlG5s+UkPFo6lkLoZ3YLOaovPYjq1R75NP5JfeTlSHaOsE09peCg==} + engines: {node: '>=20'} + tr46@0.0.3: resolution: {integrity: sha512-N3WMsuqV66lT30CrXNbEjx4GEwlow3v6rr4mCcv6prnfwhS01rkgyFdjPNBYd9br7LpXV1+Emh01fHnq2Gdgrw==} @@ -2842,6 +2980,12 @@ packages: undici-types@7.18.2: resolution: {integrity: sha512-AsuCzffGHJybSaRrmr5eHr81mwJU3kjw6M+uprWvCXiNeN9SOGwQ3Jn8jb8m3Z6izVgknn1R0FTCEAP2QrLY/w==} + universal-github-app-jwt@2.2.2: + resolution: {integrity: sha512-dcmbeSrOdTnsjGjUfAlqNDJrhxXizjAz94ija9Qw8YkZ1uu0d+GoZzyH+Jb9tIIqvGsadUfwg+22k5aDqqwzbw==} + + universal-user-agent@7.0.3: + resolution: {integrity: sha512-TmnEAEAsBJVZM/AADELsK76llnwcf9vMKuPz8JflO1frO8Lchitr0fNaN9d+Ap0BjKtqWqd/J17qeDnXh8CL2A==} + universalify@0.1.2: resolution: {integrity: sha512-rBJeI5CXAlmy1pV+617WB9J63U6XcazHHF2f2dbJix4XzpUF0RS3Zbj0FGIOCAva5P/d/GBOYaACQ1w+0azUkg==} engines: {node: '>= 4.0.0'} @@ -3591,6 +3735,154 @@ snapshots: '@nodelib/fs.scandir': 2.1.5 fastq: 1.13.0 + '@octokit/app@16.1.2': + dependencies: + '@octokit/auth-app': 8.2.0 + '@octokit/auth-unauthenticated': 7.0.3 + '@octokit/core': 7.0.6 + '@octokit/oauth-app': 8.0.3 + '@octokit/plugin-paginate-rest': 14.0.0(@octokit/core@7.0.6) + '@octokit/types': 16.0.0 + '@octokit/webhooks': 14.2.0 + + '@octokit/auth-app@8.2.0': + dependencies: + '@octokit/auth-oauth-app': 9.0.3 + '@octokit/auth-oauth-user': 6.0.2 + '@octokit/request': 10.0.11 + '@octokit/request-error': 7.1.0 + '@octokit/types': 16.0.0 + toad-cache: 3.7.4 + universal-github-app-jwt: 2.2.2 + universal-user-agent: 7.0.3 + + '@octokit/auth-oauth-app@9.0.3': + dependencies: + '@octokit/auth-oauth-device': 8.0.3 + '@octokit/auth-oauth-user': 6.0.2 + '@octokit/request': 10.0.11 + '@octokit/types': 16.0.0 + universal-user-agent: 7.0.3 + + '@octokit/auth-oauth-device@8.0.3': + dependencies: + '@octokit/oauth-methods': 6.0.2 + '@octokit/request': 10.0.11 + '@octokit/types': 16.0.0 + universal-user-agent: 7.0.3 + + '@octokit/auth-oauth-user@6.0.2': + dependencies: + '@octokit/auth-oauth-device': 8.0.3 + '@octokit/oauth-methods': 6.0.2 + '@octokit/request': 10.0.11 + '@octokit/types': 16.0.0 + universal-user-agent: 7.0.3 + + '@octokit/auth-token@6.0.0': {} + + '@octokit/auth-unauthenticated@7.0.3': + dependencies: + '@octokit/request-error': 7.1.0 + '@octokit/types': 16.0.0 + + '@octokit/core@7.0.6': + dependencies: + '@octokit/auth-token': 6.0.0 + '@octokit/graphql': 9.0.3 + '@octokit/request': 10.0.11 + '@octokit/request-error': 7.1.0 + '@octokit/types': 16.0.0 + before-after-hook: 4.0.0 + universal-user-agent: 7.0.3 + + '@octokit/endpoint@11.0.3': + dependencies: + '@octokit/types': 16.0.0 + universal-user-agent: 7.0.3 + + '@octokit/graphql@9.0.3': + dependencies: + '@octokit/request': 10.0.11 + '@octokit/types': 16.0.0 + universal-user-agent: 7.0.3 + + '@octokit/oauth-app@8.0.3': + dependencies: + '@octokit/auth-oauth-app': 9.0.3 + '@octokit/auth-oauth-user': 6.0.2 + '@octokit/auth-unauthenticated': 7.0.3 + '@octokit/core': 7.0.6 + '@octokit/oauth-authorization-url': 8.0.0 + '@octokit/oauth-methods': 6.0.2 + '@types/aws-lambda': 8.10.162 + universal-user-agent: 7.0.3 + + '@octokit/oauth-authorization-url@8.0.0': {} + + '@octokit/oauth-methods@6.0.2': + dependencies: + '@octokit/oauth-authorization-url': 8.0.0 + '@octokit/request': 10.0.11 + '@octokit/request-error': 7.1.0 + '@octokit/types': 16.0.0 + + '@octokit/openapi-types@27.0.0': {} + + '@octokit/openapi-webhooks-types@12.1.0': {} + + '@octokit/plugin-paginate-graphql@6.0.0(@octokit/core@7.0.6)': + dependencies: + '@octokit/core': 7.0.6 + + '@octokit/plugin-paginate-rest@14.0.0(@octokit/core@7.0.6)': + dependencies: + '@octokit/core': 7.0.6 + '@octokit/types': 16.0.0 + + '@octokit/plugin-rest-endpoint-methods@17.0.0(@octokit/core@7.0.6)': + dependencies: + '@octokit/core': 7.0.6 + '@octokit/types': 16.0.0 + + '@octokit/plugin-retry@8.1.0(@octokit/core@7.0.6)': + dependencies: + '@octokit/core': 7.0.6 + '@octokit/request-error': 7.1.0 + '@octokit/types': 16.0.0 + bottleneck: 2.19.5 + + '@octokit/plugin-throttling@11.0.3(@octokit/core@7.0.6)': + dependencies: + '@octokit/core': 7.0.6 + '@octokit/types': 16.0.0 + bottleneck: 2.19.5 + + '@octokit/request-error@7.1.0': + dependencies: + '@octokit/types': 16.0.0 + + '@octokit/request@10.0.11': + dependencies: + '@octokit/endpoint': 11.0.3 + '@octokit/request-error': 7.1.0 + '@octokit/types': 16.0.0 + content-type: 2.0.0 + json-with-bigint: 3.5.8 + universal-user-agent: 7.0.3 + + '@octokit/types@16.0.0': + dependencies: + '@octokit/openapi-types': 27.0.0 + + '@octokit/webhooks-methods@6.0.0': {} + + '@octokit/webhooks@14.2.0': + dependencies: + '@octokit/openapi-webhooks-types': 12.1.0 + '@octokit/request-error': 7.1.0 + '@octokit/webhooks-methods': 6.0.0 + '@oxc-resolver/binding-android-arm-eabi@11.19.1': optional: true @@ -3805,6 +4097,8 @@ snapshots: tslib: 2.8.1 optional: true + '@types/aws-lambda@8.10.162': {} + '@types/chai@5.2.3': dependencies: '@types/deep-eql': 4.0.2 @@ -4090,10 +4384,14 @@ snapshots: base64-js@1.5.1: {} + before-after-hook@4.0.0: {} + better-path-resolve@1.0.0: dependencies: is-windows: 1.0.2 + bottleneck@2.19.5: {} + brace-expansion@1.1.11: dependencies: balanced-match: 1.0.2 @@ -4196,6 +4494,8 @@ snapshots: concat-map@0.0.1: {} + content-type@2.0.0: {} + cosmiconfig-toml-loader@1.0.0: dependencies: '@iarna/toml': 2.2.5 @@ -5184,6 +5484,8 @@ snapshots: json-stable-stringify-without-jsonify@1.0.1: {} + json-with-bigint@3.5.8: {} + json5@1.0.2: dependencies: minimist: 1.2.8 @@ -5421,6 +5723,20 @@ snapshots: obug@2.1.1: {} + octokit@5.0.5: + dependencies: + '@octokit/app': 16.1.2 + '@octokit/core': 7.0.6 + '@octokit/oauth-app': 8.0.3 + '@octokit/plugin-paginate-graphql': 6.0.0(@octokit/core@7.0.6) + '@octokit/plugin-paginate-rest': 14.0.0(@octokit/core@7.0.6) + '@octokit/plugin-rest-endpoint-methods': 17.0.0(@octokit/core@7.0.6) + '@octokit/plugin-retry': 8.1.0(@octokit/core@7.0.6) + '@octokit/plugin-throttling': 11.0.3(@octokit/core@7.0.6) + '@octokit/request-error': 7.1.0 + '@octokit/types': 16.0.0 + '@octokit/webhooks': 14.2.0 + once@1.4.0: dependencies: wrappy: 1.0.2 @@ -5956,6 +6272,8 @@ snapshots: dependencies: is-number: 7.0.0 + toad-cache@3.7.4: {} + tr46@0.0.3: {} tree-dump@1.1.0(tslib@2.8.1): @@ -6043,6 +6361,10 @@ snapshots: undici-types@7.18.2: {} + universal-github-app-jwt@2.2.2: {} + + universal-user-agent@7.0.3: {} + universalify@0.1.2: {} unixify@1.0.0: diff --git a/workflows/review/README.md b/workflows/review/README.md index 3e347133..b916763a 100644 --- a/workflows/review/README.md +++ b/workflows/review/README.md @@ -169,6 +169,35 @@ run by default; every other row is opt-in via `ROUTING` and earns its line throu the eval suite. Per-role Fable-5 / Sonnet experiment arms are eval-suite measurements to run after the suite exists. +### Feedback signal: thumbs sweep and live counters + +Two small scheduled workflows in each consumer repo turn on the tuning loop's +production signal. Both are plain GitHub Actions YAML (not gh-aw), both check +out this repo at the pinned `review-v*` tag and run lib scripts with +`npx -y tsx`, and neither touches review semantics: + +- **Thumbs sweep** (`lib/run-thumbs-sweep.ts`, every 1-2 hours): collects 👍/👎 + reactions on the reviewer's comments at both grains (inline review comments, + identified by the code-owned Conventional-Comment label prefixes; the + risks/patterns summary comment, identified by its hidden marker), seeds the + visible 👍/👎 nudge pair on any reviewer comment lacking it + (`REVIEW_SWEEP_SEED_REACTIONS=true`), and posts exactly one "why?" follow-up + per newly-downvoted comment. Idempotent across restarts via the hidden + follow-up markers; bounded to PRs updated in the last 14 days. Needs only + `pull-requests: write`. The sweep run needs `npm install --omit=dev` in the + checked-out `workflows/review/` first (the sweep's `octokit` dependency is + pinned exactly in `package.json`); the other lib scripts remain + dependency-free. Each run's `SweepResult` and API-request count land in the + job summary. +- **Live counters** (`lib/counters-report.ts`, weekly): the workflow downloads + the review runs' per-run artifacts (bounded window), and the script + aggregates them with `lib/counters.ts` into the job summary — verdict mix, + comments/run, validator drop rate, cost/run. Needs only `actions: read`. + +The reviewer posts as `github-actions[bot]` (gh-aw safe outputs use the +workflow's own token), so that login is both the sweep's `botLogin` filter and +the author of its seeded reactions and follow-ups. + ### Required secrets / variables - `ANTHROPIC_API_KEY` — used by the `claude` engine. diff --git a/workflows/review/lib/counters-report.test.ts b/workflows/review/lib/counters-report.test.ts new file mode 100644 index 00000000..593dc53c --- /dev/null +++ b/workflows/review/lib/counters-report.test.ts @@ -0,0 +1,220 @@ +import {mkdtempSync, mkdirSync, writeFileSync, rmSync} from "node:fs"; +import {tmpdir} from "node:os"; +import {join} from "node:path"; + +import {describe, it, expect, afterEach} from "vitest"; + +import { + findArtifactFile, + loadRunDir, + parseJsonl, + renderCountersMarkdown, + synthesizeSummaryFromGhAw, + UNATTRIBUTED_SOURCE, +} from "./counters-report.ts"; +import {computeRunCounters, joinValidatorDecisions} from "./counters.ts"; + +describe("parseJsonl", () => { + it("keeps valid records and drops blank/malformed lines", () => { + const text = '{"type":"add_comment"}\n\nnot json\n{"type":"x"}'; + expect(parseJsonl(text)).toEqual([{type: "add_comment"}, {type: "x"}]); + }); +}); + +describe("synthesizeSummaryFromGhAw", () => { + it("derives verdict, posted count, and cost from the engine artifacts", () => { + const summary = synthesizeSummaryFromGhAw({ + safeOutputs: [ + '{"type":"create_pull_request_review_comment","body":"a"}', + '{"type":"submit_pull_request_review","event":"REQUEST_CHANGES"}', + '{"type":"upload_artifact"}', + ].join("\n"), + postedItems: [ + '{"type":"create_pull_request_review_comment","url":"u1"}', + '{"type":"add_comment","url":"u2"}', + ].join("\n"), + agentUsage: { + input_tokens: 1000, + output_tokens: 500, + ai_credits: 430.924, + }, + }); + + expect(summary["verdict"]).toBe("REQUEST_CHANGES"); + // The handler log (what actually posted) wins over emitted intent. + expect(summary["postedCommentCount"]).toBe(2); + expect(summary["cost"]).toEqual({usd: 4.30924, tokens: 1500}); + }); + + it("counts emitted intent when the handler log is absent", () => { + const summary = synthesizeSummaryFromGhAw({ + safeOutputs: '{"type":"add_comment","body":"guidance"}', + }); + expect(summary["postedCommentCount"]).toBe(1); + expect(summary["verdict"]).toBeUndefined(); + }); + + it("yields an empty posted count with no artifacts at all", () => { + expect(synthesizeSummaryFromGhAw({})["postedCommentCount"]).toBe(0); + }); +}); + +describe("validator vocabulary folding (counters.ts)", () => { + it("accepts the production {claims: [...]} shape with three-state verification", () => { + const decisions = joinValidatorDecisions( + undefined, + { + claims: [ + {id: "v1", verification: "refuted"}, + {id: "v2", verification: "plausible"}, + {id: "v3", verification: "confirmed"}, + {id: "v4", verification: "unsure"}, // unknown -> skipped + ], + }, + UNATTRIBUTED_SOURCE, + ); + expect(decisions).toEqual([ + {source: UNATTRIBUTED_SOURCE, decision: "drop"}, + {source: UNATTRIBUTED_SOURCE, decision: "keep"}, + {source: UNATTRIBUTED_SOURCE, decision: "keep"}, + ]); + }); + + it("still skips unattributed entries when no fallback source is given", () => { + const decisions = joinValidatorDecisions(undefined, [ + {id: "v1", verdict: "drop"}, + ]); + expect(decisions).toEqual([]); + }); + + it("prefers the claims.json source join when it matches", () => { + const decisions = joinValidatorDecisions( + [{id: "v1", source: "security-auth"}], + [{id: "v1", verdict: "keep"}], + UNATTRIBUTED_SOURCE, + ); + expect(decisions).toEqual([ + {source: "security-auth", decision: "keep"}, + ]); + }); +}); + +describe("loadRunDir", () => { + const cleanups: string[] = []; + afterEach(() => { + for (const dir of cleanups.splice(0)) { + rmSync(dir, {recursive: true, force: true}); + } + }); + + const makeRunDir = (): string => { + const dir = mkdtempSync(join(tmpdir(), "counters-report-")); + cleanups.push(dir); + return dir; + }; + + it("tolerates gh-aw staging-path nesting and synthesizes the summary", () => { + const dir = makeRunDir(); + // The known staging-path bug: artifact contents nested under the + // absolute staging path instead of the artifact root. + const nested = join(dir, "tmp", "gh-aw", "review", "out"); + mkdirSync(nested, {recursive: true}); + writeFileSync( + join(nested, "claim-validator.json"), + JSON.stringify({ + claims: [ + {id: "v1", verification: "refuted"}, + {id: "v2", verification: "confirmed"}, + ], + }), + ); + writeFileSync( + join(dir, "safeoutputs.jsonl"), + '{"type":"submit_pull_request_review","event":"APPROVE"}\n' + + '{"type":"create_pull_request_review_comment"}\n', + ); + writeFileSync( + join(dir, "agent_usage.json"), + JSON.stringify({ + input_tokens: 10, + output_tokens: 5, + ai_credits: 200, + }), + ); + + const {run, hasVerdict} = loadRunDir(dir); + expect(hasVerdict).toBe(true); + expect(run.verdict).toBe("APPROVE"); + expect(run.postedCommentCount).toBe(1); + expect(run.cost).toEqual({usd: 2, tokens: 15}); + expect(run.validatorDecisions).toEqual([ + {source: UNATTRIBUTED_SOURCE, decision: "drop"}, + {source: UNATTRIBUTED_SOURCE, decision: "keep"}, + ]); + }); + + it("flags a run with no submitted review as verdict-less", () => { + const dir = makeRunDir(); + writeFileSync( + join(dir, "safeoutputs.jsonl"), + '{"type":"add_comment","body":"guidance"}\n', + ); + const {run, hasVerdict} = loadRunDir(dir); + expect(hasVerdict).toBe(false); + expect(run.verdict).toBe("HOLD_FOR_HUMAN"); // documented safe fallback + }); + + it("prefers an explicit summary.json over synthesis", () => { + const dir = makeRunDir(); + writeFileSync( + join(dir, "summary.json"), + JSON.stringify({verdict: "REQUEST_CHANGES", postedCommentCount: 4}), + ); + writeFileSync( + join(dir, "safeoutputs.jsonl"), + '{"type":"submit_pull_request_review","event":"APPROVE"}\n', + ); + const {run} = loadRunDir(dir); + expect(run.verdict).toBe("REQUEST_CHANGES"); + expect(run.postedCommentCount).toBe(4); + }); + + it("findArtifactFile returns the shallowest match", () => { + const dir = makeRunDir(); + mkdirSync(join(dir, "deep", "deeper"), {recursive: true}); + writeFileSync(join(dir, "deep", "deeper", "x.json"), "{}"); + writeFileSync(join(dir, "x.json"), "{}"); + expect(findArtifactFile(dir, "x.json")).toBe(join(dir, "x.json")); + expect(findArtifactFile(dir, "missing.json")).toBeUndefined(); + }); +}); + +describe("renderCountersMarkdown", () => { + it("renders the aggregate tables with the verdict-fallback footnote", () => { + const counters = computeRunCounters([ + { + runId: "1", + verdict: "APPROVE", + postedCommentCount: 3, + validatorDecisions: [ + {source: "(unknown)", decision: "keep"}, + {source: "(unknown)", decision: "drop"}, + ], + cost: {usd: 4.2}, + }, + { + runId: "2", + verdict: "HOLD_FOR_HUMAN", + postedCommentCount: 0, + validatorDecisions: [], + }, + ]); + const markdown = renderCountersMarkdown(counters, 1); + expect(markdown).toContain("Aggregated over **2** review runs"); + expect(markdown).toContain("| 1 | 0 | 1 |"); + expect(markdown).toContain("*1 run(s) submitted no review"); + expect(markdown).toContain("| (unknown) | 2 | 1 | 50.0% |"); + expect(markdown).toContain("$4.20 total"); + expect(markdown).toContain("thumbs-sweep run's job summary"); + }); +}); diff --git a/workflows/review/lib/counters-report.ts b/workflows/review/lib/counters-report.ts new file mode 100644 index 00000000..fed7de36 --- /dev/null +++ b/workflows/review/lib/counters-report.ts @@ -0,0 +1,368 @@ +/** + * CLI that turns a directory of downloaded per-run review artifacts into the + * live-counters report — the "wire the counters somewhere someone looks" half + * of `counters.ts`. The consumer repos' weekly `review-counters` workflows run: + * + * npx -y tsx gh-aw-review-lib/workflows/review/lib/counters-report.ts + * + * where `` holds one subdirectory per review run (named by run id), + * each containing whatever artifacts the workflow downloaded for that run. No + * GitHub calls happen here — the workflow does the (bounded) artifact download + * with `gh api`; this script is a pure aggregation over local files, so the + * counters themselves stay deterministic and unit-testable. + * + * Three artifact layouts are understood, newest first: + * + * 1. The conventional layout `counters.ts` documents (`claims.json`, + * `out/claim-validator.json`, `summary.json`). + * 2. Today's production reality: runs upload `out/**` (per-sub-agent JSON) + * but no `claims.json` or `summary.json`. Validator decisions then count + * under the `(unknown)` source, and the run summary is synthesized from + * two gh-aw engine artifacts every run already has: + * - `safeoutputs.jsonl` (the agent's emitted safe outputs) -> verdict + * (the `submit_pull_request_review` event) and, as a fallback, + * intended comment counts; + * - `safe-output-items.jsonl` (the processed handler log) -> comments + * actually posted; + * - `agent_usage.json` -> model cost (`ai_credits`, 1 credit = $0.01) + * and token totals. + * 3. Anything older/malformed: the affected counters degrade per + * `counters.ts`'s normalisation rules rather than failing the report. + * + * Files are located by basename anywhere under the run directory (shallowest + * match wins), which tolerates the known gh-aw artifact staging-path nesting + * rather than assuming exact zip layouts. + */ + +import {readdirSync, readFileSync, appendFileSync} from "node:fs"; +import {basename, join} from "node:path"; + +import { + computeRunCounters, + normalizeRunArtifacts, + type RunArtifacts, + type RunCounters, +} from "./counters.ts"; + +const isRecord = (value: unknown): value is Record => + typeof value === "object" && value !== null && !Array.isArray(value); + +/** Parse a JSONL string into records, skipping blank/malformed lines. */ +export const parseJsonl = (text: string): Record[] => { + const records: Record[] = []; + for (const line of text.split("\n")) { + const trimmed = line.trim(); + if (trimmed === "") { + continue; + } + try { + const parsed: unknown = JSON.parse(trimmed); + if (isRecord(parsed)) { + records.push(parsed); + } + } catch { + // A malformed line drops a data point, never the report. + } + } + return records; +}; + +/** The gh-aw engine artifacts a run summary can be synthesized from. */ +export type GhAwRunFiles = { + /** `safeoutputs.jsonl` — the agent's emitted safe outputs (with `event`). */ + safeOutputs?: string; + /** `safe-output-items.jsonl` — the handler's log of posted items. */ + postedItems?: string; + /** `agent_usage.json` — parsed engine usage roll-up. */ + agentUsage?: unknown; +}; + +const COMMENT_ITEM_TYPES = new Set([ + "create_pull_request_review_comment", + "add_comment", +]); + +/** + * Synthesize the `summary.json`-shaped object `normalizeRunArtifacts` expects + * from the gh-aw engine artifacts. Pure. Fields the artifacts cannot answer are + * left absent so normalisation applies its documented fallbacks (in particular + * a run with no submitted review normalises to `HOLD_FOR_HUMAN`; the report + * calls those runs out separately rather than hiding them). + */ +export const synthesizeSummaryFromGhAw = ( + files: GhAwRunFiles, +): Record => { + const summary: Record = {}; + + const emitted = + files.safeOutputs === undefined ? [] : parseJsonl(files.safeOutputs); + const posted = + files.postedItems === undefined ? [] : parseJsonl(files.postedItems); + + // Verdict: the review-submission event the agent emitted. (The processed + // items log records the posted review's URL but not its event, so the + // emitted output is the only artifact that carries APPROVE vs + // REQUEST_CHANGES.) + for (const entry of emitted) { + if (entry["type"] !== "submit_pull_request_review") { + continue; + } + const event = entry["event"]; + if (event === "APPROVE" || event === "REQUEST_CHANGES") { + summary["verdict"] = event; + } + } + + // Posted comments: prefer the handler's log of what actually landed; + // fall back to the agent's emitted intent when the log is absent. + const countComments = (entries: Record[]): number => + entries.filter((entry) => { + const type = entry["type"]; + return typeof type === "string" && COMMENT_ITEM_TYPES.has(type); + }).length; + summary["postedCommentCount"] = + files.postedItems !== undefined + ? countComments(posted) + : countComments(emitted); + + // Cost: gh-aw's agent_usage.json (ai_credits are cents). + if (isRecord(files.agentUsage)) { + const credits = files.agentUsage["ai_credits"]; + const inputTokens = files.agentUsage["input_tokens"]; + const outputTokens = files.agentUsage["output_tokens"]; + const cost: Record = {}; + if (typeof credits === "number" && Number.isFinite(credits)) { + cost["usd"] = credits / 100; + } + if ( + typeof inputTokens === "number" && + typeof outputTokens === "number" && + Number.isFinite(inputTokens) && + Number.isFinite(outputTokens) + ) { + cost["tokens"] = inputTokens + outputTokens; + } + if (Object.keys(cost).length > 0) { + summary["cost"] = cost; + } + } + + return summary; +}; + +/** + * Source label for validator decisions that cannot be joined to a claim + * (production artifacts do not include `claims.json`; see module docs). + */ +export const UNATTRIBUTED_SOURCE = "(unknown)"; + +/** One loaded run plus provenance the report footnotes. */ +export type LoadedRun = { + run: RunArtifacts; + /** Whether the run carried (or could synthesize) a real verdict. */ + hasVerdict: boolean; +}; + +type ReadFile = (path: string) => string; + +const defaultReadFile: ReadFile = (path) => readFileSync(path, "utf8"); + +/** + * Find a file by basename anywhere under `dir`, shallowest first (tolerates + * the gh-aw artifact staging-path nesting). Returns the path or undefined. + */ +export const findArtifactFile = ( + dir: string, + name: string, +): string | undefined => { + let frontier: string[] = [dir]; + while (frontier.length > 0) { + const next: string[] = []; + for (const current of frontier) { + let entries; + try { + entries = readdirSync(current, {withFileTypes: true}); + } catch { + continue; + } + for (const entry of entries) { + const path = join(current, entry.name); + if (entry.isFile() && entry.name === name) { + return path; + } + if (entry.isDirectory()) { + next.push(path); + } + } + } + frontier = next; + } + return undefined; +}; + +const readIfFound = ( + dir: string, + name: string, + readFile: ReadFile, +): string | undefined => { + const path = findArtifactFile(dir, name); + if (path === undefined) { + return undefined; + } + try { + return readFile(path); + } catch { + return undefined; + } +}; + +const parseIfFound = ( + dir: string, + name: string, + readFile: ReadFile, +): unknown => { + const text = readIfFound(dir, name, readFile); + if (text === undefined) { + return undefined; + } + try { + return JSON.parse(text) as unknown; + } catch { + return undefined; + } +}; + +/** Load one run directory into {@link RunArtifacts}, best-effort. */ +export const loadRunDir = ( + dir: string, + readFile: ReadFile = defaultReadFile, +): LoadedRun => { + const claims = parseIfFound(dir, "claims.json", readFile); + const validator = parseIfFound(dir, "claim-validator.json", readFile); + + let summary = parseIfFound(dir, "summary.json", readFile); + if (!isRecord(summary)) { + const files: GhAwRunFiles = {}; + const safeOutputs = readIfFound(dir, "safeoutputs.jsonl", readFile); + if (safeOutputs !== undefined) { + files.safeOutputs = safeOutputs; + } + const postedItems = readIfFound( + dir, + "safe-output-items.jsonl", + readFile, + ); + if (postedItems !== undefined) { + files.postedItems = postedItems; + } + const agentUsage = parseIfFound(dir, "agent_usage.json", readFile); + if (agentUsage !== undefined) { + files.agentUsage = agentUsage; + } + summary = synthesizeSummaryFromGhAw(files); + } + + const hasVerdict = isRecord(summary) && summary["verdict"] !== undefined; + const run = normalizeRunArtifacts( + {claims, validator, summary}, + basename(dir), + UNATTRIBUTED_SOURCE, + ); + return {run, hasVerdict}; +}; + +const pct = (value: number): string => `${(value * 100).toFixed(1)}%`; + +/** Render the weekly counters report as job-summary Markdown. */ +export const renderCountersMarkdown = ( + counters: RunCounters, + runsWithoutVerdict: number, +): string => { + const lines = [ + "## Review live counters", + "", + `Aggregated over **${counters.runCount}** review runs.`, + "", + "### Verdict mix", + "", + "| APPROVE | REQUEST_CHANGES | HOLD_FOR_HUMAN* |", + "| --- | --- | --- |", + `| ${counters.verdictMix.APPROVE} | ${counters.verdictMix.REQUEST_CHANGES} | ${counters.verdictMix.HOLD_FOR_HUMAN} |`, + "", + `*${runsWithoutVerdict} run(s) submitted no review this window (e.g. the` + + " redundant-approval skip) and count under the HOLD_FOR_HUMAN fallback.", + "", + "### Comments and validator", + "", + `- Posted comments: **${counters.totalComments}** total,` + + ` **${counters.commentsPerRun.toFixed(2)}**/run`, + `- Validator drop rate (all sources): **${pct( + counters.overallValidatorDropRate, + )}**`, + ]; + + if (counters.validatorDropBySource.length > 0) { + lines.push( + "", + "| Source | Judged | Dropped | Drop rate |", + "| --- | --- | --- | --- |", + ); + for (const source of counters.validatorDropBySource) { + lines.push( + `| ${source.source} | ${source.total} | ${ + source.dropped + } | ${pct(source.dropRate)} |`, + ); + } + } + + lines.push("", "### Thumbs and cost", ""); + lines.push( + counters.thumbs.agreeRate === null + ? "- Thumbs: none recorded in per-run artifacts (live tallies are" + + " reported by each thumbs-sweep run's job summary)" + : `- Thumbs: ${counters.thumbs.up} 👍 / ${counters.thumbs.down} 👎` + + ` (agree rate ${pct(counters.thumbs.agreeRate)})`, + ); + lines.push( + counters.cost.usdPerRun === null + ? "- Cost: not recorded" + : `- Cost: $${(counters.cost.totalUsd ?? 0).toFixed(2)} total,` + + ` $${counters.cost.usdPerRun.toFixed(2)}/run`, + ); + lines.push(""); + return lines.join("\n"); +}; + +const main = (): void => { + const runsDir = process.argv[2]; + if (runsDir === undefined) { + throw new Error("usage: counters-report.ts "); + } + + const runDirs = readdirSync(runsDir, {withFileTypes: true}) + .filter((entry) => entry.isDirectory()) + .map((entry) => join(runsDir, entry.name)) + .sort(); + + const loaded = runDirs.map((dir) => loadRunDir(dir)); + const counters = computeRunCounters(loaded.map(({run}) => run)); + const runsWithoutVerdict = loaded.filter( + ({hasVerdict}) => !hasVerdict, + ).length; + + const markdown = renderCountersMarkdown(counters, runsWithoutVerdict); + process.stdout.write(`${JSON.stringify(counters, null, 2)}\n`); + + const summaryPath = process.env["GITHUB_STEP_SUMMARY"]; + if (summaryPath !== undefined && summaryPath.trim() !== "") { + appendFileSync(summaryPath, markdown); + } else { + process.stderr.write(markdown); + } +}; + +// Run only when invoked directly, never on import (tests). +if (typeof require !== "undefined" && require.main === module) { + main(); +} diff --git a/workflows/review/lib/counters.ts b/workflows/review/lib/counters.ts index 0a5ed351..9d2ff6e3 100644 --- a/workflows/review/lib/counters.ts +++ b/workflows/review/lib/counters.ts @@ -278,9 +278,12 @@ export type RawRunArtifacts = { /** `claims.json`: each entry carries at least `id` and `source`. */ claims?: unknown; /** - * `out/claim-validator.json`: per-claim `keep`/`drop` verdicts. Accepts a - * bare array or an object with a `results`/`decisions` array; each entry - * carries an `id` and a `verdict` (`keep`/`drop`). + * `out/claim-validator.json`: per-claim validation outcomes. Accepts a bare + * array or an object with a `results`/`decisions`/`claims` array (the + * production validator writes `{"claims": […]}`). Each entry carries an + * `id` and either a `verdict` (`keep`/`drop`) or the three-state + * `verification` (`confirmed`/`plausible`/`refuted`), which folds to + * keep/keep/drop for the drop-rate counter. */ validator?: unknown; /** The run summary: verdict, posted-comment count, thumbs, cost. */ @@ -298,19 +301,51 @@ const extractValidatorEntries = (validator: unknown): unknown[] => { if (Array.isArray(validator["decisions"])) { return validator["decisions"]; } + if (Array.isArray(validator["claims"])) { + return validator["claims"]; + } } return []; }; /** - * Join `claims.json` (id -> source) with `claim-validator.json` (id -> verdict) - * into per-source {@link ValidatorDecision}s. A validator entry whose `id` has no - * matching claim, or whose verdict is neither `keep` nor `drop`, is skipped — a - * malformed artifact drops a data point rather than corrupting the counter. + * Fold a validator entry's outcome to the binary `keep`/`drop` the counters + * aggregate. Two vocabularies appear in real artifacts: the original + * `verdict: keep|drop`, and the three-state `verification: + * confirmed|plausible|refuted` (the recall/precision rebalance) — a `plausible` + * claim is kept-but-downgraded, so for drop-rate purposes it counts as `keep`. + */ +const decisionOf = ( + entry: Record, +): "keep" | "drop" | undefined => { + const verdict = entry["verdict"] ?? entry["verification"]; + if ( + verdict === "keep" || + verdict === "confirmed" || + verdict === "plausible" + ) { + return "keep"; + } + if (verdict === "drop" || verdict === "refuted") { + return "drop"; + } + return undefined; +}; + +/** + * Join `claims.json` (id -> source) with `claim-validator.json` (id -> outcome) + * into per-source {@link ValidatorDecision}s. A validator entry with no + * recognisable outcome is skipped — a malformed artifact drops a data point + * rather than corrupting the counter. An entry whose `id` has no matching claim + * is skipped too, unless `fallbackSource` is given, in which case it is counted + * under that source: production runs upload `out/**` but not `claims.json`, so + * without the fallback every real decision would be discarded and the overall + * drop rate would read as zero activity. */ export const joinValidatorDecisions = ( claims: unknown, validator: unknown, + fallbackSource?: string, ): ValidatorDecision[] => { const sourceById = new Map(); if (Array.isArray(claims)) { @@ -331,18 +366,15 @@ export const joinValidatorDecisions = ( continue; } const id = entry["id"]; - const verdict = entry["verdict"]; - if (typeof id !== "string") { - continue; - } - if (verdict !== "keep" && verdict !== "drop") { + const decision = decisionOf(entry); + if (typeof id !== "string" || decision === undefined) { continue; } - const source = sourceById.get(id); + const source = sourceById.get(id) ?? fallbackSource; if (source === undefined) { continue; } - decisions.push({source, decision: verdict}); + decisions.push({source, decision}); } return decisions; }; @@ -392,6 +424,7 @@ const normalizeCost = ( export const normalizeRunArtifacts = ( raw: RawRunArtifacts, fallbackRunId: string, + fallbackSource?: string, ): RunArtifacts => { const summary: Record = isRecord(raw.summary) ? raw.summary @@ -416,6 +449,7 @@ export const normalizeRunArtifacts = ( const validatorDecisions = joinValidatorDecisions( raw.claims, raw.validator, + fallbackSource, ); const thumbs = normalizeThumbs(summary); diff --git a/workflows/review/lib/run-thumbs-sweep.ts b/workflows/review/lib/run-thumbs-sweep.ts new file mode 100644 index 00000000..8d529faf --- /dev/null +++ b/workflows/review/lib/run-thumbs-sweep.ts @@ -0,0 +1,157 @@ +/** + * CLI entry for the thumbs feedback sweep — the script the consumer repos' + * scheduled `review-feedback` workflows run: + * + * cd gh-aw-review-lib/workflows/review && npm install --omit=dev && + * npx -y tsx lib/run-thumbs-sweep.ts + * + * (The `npm install` supplies `octokit`, this package's one runtime + * dependency — unlike the router/investigation-cap scripts, the sweep talks to + * the GitHub API. It is pinned exactly in `package.json`, so a consumer run + * resolves the same client version this release was tested with.) + * + * All configuration is environment variables, so the consumer workflow is pure + * YAML with no arguments to quote: + * + * GITHUB_TOKEN required; the workflow's token + * (`pull-requests: write` is the only scope the + * sweep needs). + * GITHUB_REPOSITORY `owner/repo`; provided by Actions. + * REVIEW_SWEEP_BOT_LOGIN login the reviewer posts as + * (default `github-actions[bot]`). + * REVIEW_SWEEP_SEED_REACTIONS `true` to seed the 👍/👎 nudge pair + * (default `false`). + * REVIEW_SWEEP_LOOKBACK_DAYS PR-activity window (default 14). + * REVIEW_SWEEP_MAX_PULLS traversal cap (default 200). + * + * Output: the full {@link SweepResult} as JSON on stdout, and — when + * `GITHUB_STEP_SUMMARY` is set — a Markdown digest appended to the job summary + * so every sweep run is auditable from the Actions UI. + */ + +import {appendFileSync} from "node:fs"; + +import {Octokit} from "octokit"; + +import {sweepThumbs, type SweepResult} from "./thumbs-sweep.ts"; +import { + GithubThumbsSweepPort, + type OctokitRequestFn, + type SweepTraversalStats, +} from "./thumbs-sweep-github.ts"; + +const env = (name: string): string | undefined => { + const value = process.env[name]; + return value === undefined || value.trim() === "" ? undefined : value; +}; + +const intEnv = (name: string): number | undefined => { + const raw = env(name); + if (raw === undefined) { + return undefined; + } + const value = Number.parseInt(raw, 10); + if (!Number.isInteger(value) || value <= 0) { + throw new Error(`${name} must be a positive integer, got: ${raw}`); + } + return value; +}; + +/** Render the auditable Markdown digest for the job summary. */ +export const renderSweepSummary = ( + result: SweepResult, + stats: SweepTraversalStats, +): string => { + const byReason = new Map(); + for (const action of result.actions) { + byReason.set(action.reason, (byReason.get(action.reason) ?? 0) + 1); + } + const downvoted = result.actions.filter((a) => a.downvotes > 0); + + const lines = [ + "## Thumbs feedback sweep", + "", + `- Reviewer comments swept: **${result.actions.length}** across ${stats.pullsScanned} recently-active PRs`, + `- Live thumbs observed (bot's own seeds excluded): **${stats.thumbs.up} 👍 / ${stats.thumbs.down} 👎**`, + `- Follow-ups posted this sweep: **${result.followupsPosted}**` + + ` (already followed up: ${ + byReason.get("already-followed-up") ?? 0 + })`, + `- Nudge reactions seeded this sweep: **${result.reactionsSeeded}**`, + `- GitHub API requests used: ${stats.apiRequests}`, + ]; + + if (downvoted.length > 0) { + lines.push( + "", + "| Grain | Comment | 👎 | Action |", + "| --- | --- | --- | --- |", + ); + for (const action of downvoted) { + lines.push( + `| ${action.grain} | ${action.commentId} | ${action.downvotes} | ${action.reason} |`, + ); + } + } + + lines.push(""); + return lines.join("\n"); +}; + +const main = async (): Promise => { + const token = env("GITHUB_TOKEN"); + if (token === undefined) { + throw new Error("GITHUB_TOKEN is required"); + } + const repository = env("GITHUB_REPOSITORY"); + if (repository === undefined || !repository.includes("/")) { + throw new Error("GITHUB_REPOSITORY must be set to owner/repo"); + } + const [owner, repo] = repository.split("/", 2) as [string, string]; + + const botLogin = env("REVIEW_SWEEP_BOT_LOGIN") ?? "github-actions[bot]"; + const seedReactions = env("REVIEW_SWEEP_SEED_REACTIONS") === "true"; + const lookbackDays = intEnv("REVIEW_SWEEP_LOOKBACK_DAYS"); + const maxPulls = intEnv("REVIEW_SWEEP_MAX_PULLS"); + + // `Octokit` from the `octokit` package ships the throttling/retry plugins, + // so secondary-rate-limit pauses are handled by the client instead of + // failing the scheduled run. + const octokit = new Octokit({auth: token}); + const request: OctokitRequestFn = (route, params) => + octokit.request(route, params); + + const port = new GithubThumbsSweepPort(request, { + owner, + repo, + botLogin, + ...(lookbackDays !== undefined ? {lookbackDays} : {}), + ...(maxPulls !== undefined ? {maxPulls} : {}), + }); + + const result = await sweepThumbs(port, { + owner, + repo, + botLogin, + seedReactions, + }); + const stats = port.stats(); + + process.stdout.write(`${JSON.stringify({result, stats}, null, 2)}\n`); + + const summaryPath = env("GITHUB_STEP_SUMMARY"); + if (summaryPath !== undefined) { + appendFileSync(summaryPath, renderSweepSummary(result, stats)); + } +}; + +// Run only when invoked directly (`npx tsx lib/run-thumbs-sweep.ts`), never +// on import (tests import `renderSweepSummary`). Same guard as the other lib +// CLIs (`investigation-cap.ts`). +if (typeof require !== "undefined" && require.main === module) { + main().catch((error: unknown) => { + // eslint-disable-next-line no-console + console.error(error); + process.exitCode = 1; + }); +} diff --git a/workflows/review/lib/thumbs-sweep-github.test.ts b/workflows/review/lib/thumbs-sweep-github.test.ts new file mode 100644 index 00000000..4e92a748 --- /dev/null +++ b/workflows/review/lib/thumbs-sweep-github.test.ts @@ -0,0 +1,286 @@ +import {describe, it, expect} from "vitest"; + +import { + GithubThumbsSweepPort, + INLINE_COMMENT_PREFIXES, + isReviewerInlineBody, + isReviewerSummaryBody, + type OctokitRequestFn, +} from "./thumbs-sweep-github.ts"; +import {buildFollowupMarker, sweepThumbs} from "./thumbs-sweep.ts"; + +/** + * Tests for the octokit-backed port. A fake `request` function dispatches on + * the route template and records writes, so the traversal, the two-grain + * classification, the reaction resolution, and the write routing are all + * exercised without a network. + */ + +const BOT = "github-actions[bot]"; +const NOW = Date.parse("2026-07-08T00:00:00Z"); +const RECENT = "2026-07-07T12:00:00Z"; // inside the 14-day window +const STALE = "2026-05-01T00:00:00Z"; // far outside it + +const SUMMARY_BODY = [ + "", + "## Review Guidance", + "", +].join("\n"); + +type RecordedWrite = {route: string; params: Record}; + +/** + * A fake GitHub: one recent PR (#7) carrying reviewer comments at both grains + * plus decoys, and one stale PR (#1) that must never be traversed. + */ +const makeFakeGithub = () => { + const writes: RecordedWrite[] = []; + + const inlineComments = [ + { + id: 101, + user: {login: BOT}, + body: "**issue (blocking):** off-by-one in the prune loop.", + reactions: {total_count: 2}, + }, + { + // The sweep's own earlier follow-up reply: idempotency source, + // never a candidate. + id: 102, + user: {login: BOT}, + body: `${buildFollowupMarker("inline", 999)}\nThanks!`, + reactions: {total_count: 0}, + }, + { + // Bot-authored but not templated like a finding -> ignored. + id: 103, + user: {login: BOT}, + body: "some other workflow's inline note", + reactions: {total_count: 5}, + }, + { + // Human comment -> ignored. + id: 104, + user: {login: "human-dev"}, + body: "**issue (blocking):** looks reviewer-shaped but human.", + reactions: {total_count: 1}, + }, + ]; + + const issueComments = [ + { + id: 201, + user: {login: BOT}, + body: SUMMARY_BODY, + reactions: {total_count: 0}, + }, + { + // Bot-authored, no risks/patterns marker -> another workflow's. + id: 202, + user: {login: BOT}, + body: "Test results: all green", + reactions: {total_count: 3}, + }, + ]; + + const reactionsByComment: Record = { + 101: [ + {content: "-1", user: {login: "human-dev"}}, + {content: "+1", user: {login: BOT}}, // seeded; must not count + ], + }; + + const request: OctokitRequestFn = async (route, params = {}) => { + if (route === "GET /repos/{owner}/{repo}/pulls") { + const page = params["page"] as number; + return { + data: + page === 1 + ? [ + {number: 7, updated_at: RECENT}, + {number: 1, updated_at: STALE}, + ] + : [], + }; + } + if ( + route === "GET /repos/{owner}/{repo}/pulls/{pull_number}/comments" + ) { + expect(params["pull_number"]).toBe(7); + return {data: params["page"] === 1 ? inlineComments : []}; + } + if ( + route === "GET /repos/{owner}/{repo}/issues/{issue_number}/comments" + ) { + expect(params["issue_number"]).toBe(7); + return {data: params["page"] === 1 ? issueComments : []}; + } + if ( + route === + "GET /repos/{owner}/{repo}/pulls/comments/{comment_id}/reactions" + ) { + const id = params["comment_id"] as number; + return { + data: params["page"] === 1 ? reactionsByComment[id] ?? [] : [], + }; + } + if ( + route === + "GET /repos/{owner}/{repo}/issues/comments/{comment_id}/reactions" + ) { + return {data: []}; + } + if (route.startsWith("POST ")) { + writes.push({route, params}); + return {data: {}}; + } + throw new Error(`unexpected route: ${route}`); + }; + + return {request, writes}; +}; + +const makePort = (request: OctokitRequestFn) => + new GithubThumbsSweepPort(request, { + owner: "Khan", + repo: "webapp", + botLogin: BOT, + now: NOW, + }); + +describe("comment identification", () => { + it("recognises every code-owned conventional label as an inline prefix", () => { + expect(INLINE_COMMENT_PREFIXES).toContain("**issue (blocking):**"); + expect(INLINE_COMMENT_PREFIXES).toContain( + "**suggestion (non-blocking, best-practice):**", + ); + expect(isReviewerInlineBody("**todo (blocking):** add the test.")).toBe( + true, + ); + expect(isReviewerInlineBody("regular prose")).toBe(false); + }); + + it("identifies the summary comment by its hidden marker", () => { + expect(isReviewerSummaryBody(SUMMARY_BODY)).toBe(true); + expect(isReviewerSummaryBody("Test results: all green")).toBe(false); + }); +}); + +describe("traversal and classification", () => { + it("lists reviewer comments at both grains, excluding decoys and follow-ups", async () => { + const {request} = makeFakeGithub(); + const port = makePort(request); + + const inline = await port.listBotComments("inline"); + expect(inline.map((c) => c.id)).toEqual([101]); + // Reactor logins survive so the sweep can exclude the bot's seeds. + expect(inline[0]?.reactions).toEqual([ + {content: "-1", user: "human-dev"}, + {content: "+1", user: BOT}, + ]); + + const summary = await port.listBotComments("summary"); + expect(summary.map((c) => c.id)).toEqual([201]); + + const followups = await port.listExistingFollowups(); + expect(followups).toHaveLength(1); + expect(followups[0]).toContain("comment-id=999"); + }); + + it("stays within the lookback window and reports auditable stats", async () => { + const {request} = makeFakeGithub(); + const port = makePort(request); + await port.listBotComments("inline"); + + const stats = port.stats(); + expect(stats.pullsScanned).toBe(1); // the stale PR is never traversed + // Only the human 👎 counts; the bot's seeded 👍 is excluded. + expect(stats.thumbs).toEqual({up: 0, down: 1}); + expect(stats.apiRequests).toBeGreaterThan(0); + }); +}); + +describe("writes", () => { + it("routes follow-ups per grain (inline reply vs PR comment)", async () => { + const {request, writes} = makeFakeGithub(); + const port = makePort(request); + await port.listBotComments("inline"); // populate the comment->PR map + + await port.postFollowup({ + grain: "inline", + commentId: 101, + body: "why?", + }); + await port.postFollowup({ + grain: "summary", + commentId: 201, + body: "why?", + }); + + expect(writes.map((w) => w.route)).toEqual([ + "POST /repos/{owner}/{repo}/pulls/{pull_number}/comments/{comment_id}/replies", + "POST /repos/{owner}/{repo}/issues/{issue_number}/comments", + ]); + expect(writes[0]?.params["pull_number"]).toBe(7); + expect(writes[0]?.params["comment_id"]).toBe(101); + expect(writes[1]?.params["issue_number"]).toBe(7); + }); + + it("rejects a follow-up for a comment the traversal never saw", async () => { + const {request} = makeFakeGithub(); + const port = makePort(request); + await port.listBotComments("inline"); + await expect( + port.postFollowup({grain: "inline", commentId: 555, body: "?"}), + ).rejects.toThrow(/unknown comment/); + }); + + it("seeds reactions on the grain-appropriate endpoint", async () => { + const {request, writes} = makeFakeGithub(); + const port = makePort(request); + await port.addReactions("inline", 101, ["+1", "-1"]); + await port.addReactions("summary", 201, ["+1"]); + + expect(writes.map((w) => [w.route, w.params["content"]])).toEqual([ + [ + "POST /repos/{owner}/{repo}/pulls/comments/{comment_id}/reactions", + "+1", + ], + [ + "POST /repos/{owner}/{repo}/pulls/comments/{comment_id}/reactions", + "-1", + ], + [ + "POST /repos/{owner}/{repo}/issues/comments/{comment_id}/reactions", + "+1", + ], + ]); + }); +}); + +describe("end-to-end with the sweep core", () => { + it("seeds missing nudges and follows up the human 👎 exactly once", async () => { + const {request, writes} = makeFakeGithub(); + const port = makePort(request); + + const result = await sweepThumbs(port, { + owner: "Khan", + repo: "webapp", + botLogin: BOT, + seedReactions: true, + }); + + // Comment 101 already has the bot's 👍 -> only 👎 is seeded there; + // comment 201 has neither -> both are seeded. + expect(result.reactionsSeeded).toBe(3); + // The human 👎 on 101 draws exactly one follow-up, threaded inline. + expect(result.followupsPosted).toBe(1); + const followupWrites = writes.filter((w) => + w.route.includes("replies"), + ); + expect(followupWrites).toHaveLength(1); + expect(String(followupWrites[0]?.params["body"])).toContain( + "review-thumbs-followup grain=inline comment-id=101", + ); + }); +}); diff --git a/workflows/review/lib/thumbs-sweep-github.ts b/workflows/review/lib/thumbs-sweep-github.ts new file mode 100644 index 00000000..e6c512bb --- /dev/null +++ b/workflows/review/lib/thumbs-sweep-github.ts @@ -0,0 +1,483 @@ +/** + * The octokit-backed {@link ThumbsSweepPort} — the production GitHub + * implementation of the side-effect boundary `thumbs-sweep.ts` defines. + * + * Division of labour (unchanged from the sweep module): `thumbs-sweep.ts` owns + * all control flow and idempotency; this module owns only the GitHub traversal — + * which comments are the reviewer's, at which grain, with which reactions — and + * the write calls (`postFollowup`, `addReactions`). It holds no sweep logic: a + * bug here can mis-list or mis-post, but it cannot re-ping or double-seed, + * because those rules live in the sweep core. + * + * How the reviewer's comments are identified (per grain): + * + * - `summary` — issue comments authored by `botLogin` that carry the + * workflow's hidden risks/patterns marker (`pr-reviewer:risks-and-patterns`, + * the exact marker line `review.md` Step 7 requires the comment to begin + * with). The marker, not the author, is what scopes the sweep to the + * reviewer: `github-actions[bot]` authors many other workflows' comments. + * - `inline` — pull-request review comments authored by `botLogin` whose body + * starts with one of the reviewer's code-owned Conventional-Comment labels + * (`**issue (blocking):** …`, `render-comment.ts`'s taxonomy). Inline + * comments carry no hidden marker at current releases, so the label grammar + * is the identifying signature; it is code-owned and templated, so the match + * is exact, not heuristic prose-sniffing. + * + * Comments containing a thumbs-followup marker are never candidates at either + * grain — they are returned through `listExistingFollowups` instead, which is + * what makes the sweep idempotent across restarts with no state store. + * + * API-call bounding: the traversal reads only pull requests updated within + * `lookbackDays` (default 14), newest first, capped at `maxPulls`. Reactions are + * fetched per comment only when the comment's reaction summary shows any + * reactions at all. Every request is counted and reported via {@link + * GithubThumbsSweepPort.stats} so each run's API budget is auditable. + */ + +import type { + BotComment, + FeedbackGrain, + PostedFollowup, + Reaction, + ThumbsSweepPort, +} from "./thumbs-sweep.ts"; +import {parseFollowupMarkers} from "./thumbs-sweep.ts"; +import {BLOCKING_LABELS, NON_BLOCKING_LABELS} from "./render-comment.ts"; + +/** + * The one octokit surface this module needs: `octokit.request`. Kept this + * narrow so tests fake a single function and the module never depends on + * octokit's types; the real client (constructed in `run-thumbs-sweep.ts`) + * satisfies it directly. + */ +export type OctokitRequestFn = ( + route: string, + params?: Record, +) => Promise<{data: unknown}>; + +/** + * The hidden marker that identifies the reviewer's risks/patterns summary + * comment (`review.md` Step 7 requires the comment to begin with this exact + * marker line). Matched as a substring so surrounding whitespace or the + * version marker never break identification. + */ +export const SUMMARY_COMMENT_MARKER = ""; + +/** + * Body prefixes that identify a reviewer inline comment: the code-owned + * Conventional-Comment label taxonomy, exactly as `renderComment` templates it + * (`**