From 9fffe94250525d3a64452e62069d5656cb209905 Mon Sep 17 00:00:00 2001 From: web-dev0521 Date: Wed, 3 Jun 2026 00:12:24 -0600 Subject: [PATCH] feat(analytics): persist recommendation outcome events Adds 'rejected' as a seventh outcome state (PR with CHANGES_REQUESTED review decision after action timestamp), and separates surface and snapshotId on every persisted outcome record so later analytics can slice by product surface and context snapshot. - src/types.ts: add "rejected" to AgentRecommendationOutcomeState; add surface/snapshotId fields to AgentRecommendationOutcomeRecord; add rejected count to AgentRecommendationOutcomeSummary totals and AgentRecommendationOutcomeRepoSummary. - migrations/0018: ALTER TABLE adds surface and snapshot_id columns. - src/db/schema.ts: matching Drizzle column definitions. - src/db/repositories.ts: upsert/mapper/parse/bucket/totals/repo- summary all handle rejected and the new columns. - src/services/recommendation-outcomes.ts: fetch context snapshot per run in evaluateRecommendationOutcomes; thread surface/snapshotId through classifyRecommendationOutcome and baseOutcome; add rejected branch in pullRequestOutcomeState. - tests: fixture for all seven outcome states, surface/snapshotId capture round-trip, and privacy serialization assertion. --- ...ecommendation_outcome_surface_snapshot.sql | 2 + src/db/repositories.ts | 15 ++- src/db/schema.ts | 2 + src/services/decision-pack.ts | 1 + src/services/recommendation-outcomes.ts | 27 +++- src/types.ts | 6 +- test/unit/decision-pack.test.ts | 6 + test/unit/recommendation-outcomes.test.ts | 119 +++++++++++++++++- 8 files changed, 170 insertions(+), 8 deletions(-) create mode 100644 migrations/0018_recommendation_outcome_surface_snapshot.sql diff --git a/migrations/0018_recommendation_outcome_surface_snapshot.sql b/migrations/0018_recommendation_outcome_surface_snapshot.sql new file mode 100644 index 0000000000..e01fedc32f --- /dev/null +++ b/migrations/0018_recommendation_outcome_surface_snapshot.sql @@ -0,0 +1,2 @@ +ALTER TABLE agent_recommendation_outcomes ADD COLUMN surface TEXT; +ALTER TABLE agent_recommendation_outcomes ADD COLUMN snapshot_id TEXT; diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 008d5f2d7d..a42886406e 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -1482,7 +1482,7 @@ function maxIso(left: string | null | undefined, right: string | null | undefine } function outcomeStateBuckets(outcomes: AgentRecommendationOutcomeRecord[]): AgentRecommendationOutcomeSummary["states"] { - const states: AgentRecommendationOutcomeState[] = ["accepted", "merged", "improved", "closed", "stale", "ignored"]; + const states: AgentRecommendationOutcomeState[] = ["accepted", "merged", "improved", "closed", "rejected", "stale", "ignored"]; return states.flatMap((state) => { const count = outcomes.filter((outcome) => outcome.outcomeState === state).length; return count > 0 ? [{ state, count }] : []; @@ -1494,6 +1494,7 @@ function recommendationOutcomeTotals( maintainerLaneTotal: number, ): AgentRecommendationOutcomeSummary["totals"] { const accepted = outcomes.filter((outcome) => outcome.outcomeState === "accepted").length; + const rejected = outcomes.filter((outcome) => outcome.outcomeState === "rejected").length; const merged = outcomes.filter((outcome) => outcome.outcomeState === "merged").length; const improved = outcomes.filter((outcome) => outcome.outcomeState === "improved").length; const closed = outcomes.filter((outcome) => outcome.outcomeState === "closed").length; @@ -1502,13 +1503,14 @@ function recommendationOutcomeTotals( return { total: outcomes.length, accepted, + rejected, ignored, stale, merged, closed, improved, positive: accepted + merged + improved, - negative: closed + stale + ignored, + negative: closed + rejected + stale + ignored, maintainerLaneTotal, }; } @@ -1532,6 +1534,7 @@ function summarizeRecommendationOutcomeRepos(outcomes: AgentRecommendationOutcom repoFullName: firstRepo.outcomeRepoFullName ?? firstRepo.targetRepoFullName ?? "unknown/repo", total: totals.total, accepted: totals.accepted, + rejected: totals.rejected, ignored: totals.ignored, stale: totals.stale, merged: totals.merged, @@ -2427,6 +2430,8 @@ export async function upsertAgentRecommendationOutcome(env: Env, outcome: AgentR runId: outcome.runId, actorLogin: boundedString(outcome.actorLogin, 100), actionType: outcome.actionType, + surface: outcome.surface ?? null, + snapshotId: outcome.snapshotId ?? null, targetRepoFullName: outcome.targetRepoFullName ? boundedString(outcome.targetRepoFullName, 200) : null, targetPullNumber: outcome.targetPullNumber ?? null, targetIssueNumber: outcome.targetIssueNumber ?? null, @@ -2452,6 +2457,8 @@ export async function upsertAgentRecommendationOutcome(env: Env, outcome: AgentR set: { actorLogin: values.actorLogin, actionType: values.actionType, + surface: values.surface, + snapshotId: values.snapshotId, targetRepoFullName: values.targetRepoFullName, targetPullNumber: values.targetPullNumber, targetIssueNumber: values.targetIssueNumber, @@ -3202,6 +3209,8 @@ function toAgentRecommendationOutcomeRecord(row: typeof agentRecommendationOutco runId: row.runId, actorLogin: row.actorLogin, actionType: parseAgentActionType(row.actionType), + surface: row.surface ? parseAgentSurface(row.surface) : null, + snapshotId: row.snapshotId ?? null, targetRepoFullName: row.targetRepoFullName, targetPullNumber: row.targetPullNumber, targetIssueNumber: row.targetIssueNumber, @@ -3996,7 +4005,7 @@ function parseAgentSafetyClass(value: string): AgentSafetyClass { } function parseAgentRecommendationOutcomeState(value: string): AgentRecommendationOutcomeState { - if (value === "accepted" || value === "ignored" || value === "stale" || value === "merged" || value === "closed" || value === "improved") return value; + if (value === "accepted" || value === "rejected" || value === "ignored" || value === "stale" || value === "merged" || value === "closed" || value === "improved") return value; return "ignored"; } diff --git a/src/db/schema.ts b/src/db/schema.ts index f1502e5bd6..fa52ecb698 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -484,6 +484,8 @@ export const agentRecommendationOutcomes = sqliteTable( targetRepoFullName: text("target_repo_full_name"), targetPullNumber: integer("target_pull_number"), targetIssueNumber: integer("target_issue_number"), + surface: text("surface"), + snapshotId: text("snapshot_id"), outcomeState: text("outcome_state").notNull(), outcomeTargetType: text("outcome_target_type").notNull(), outcomeRepoFullName: text("outcome_repo_full_name"), diff --git a/src/services/decision-pack.ts b/src/services/decision-pack.ts index 4456fdc0ee..ad1c3def44 100644 --- a/src/services/decision-pack.ts +++ b/src/services/decision-pack.ts @@ -1023,6 +1023,7 @@ function emptyRecommendationOutcomeFeedback(login: string): AgentRecommendationO totals: { total: 0, accepted: 0, + rejected: 0, ignored: 0, stale: 0, merged: 0, diff --git a/src/services/recommendation-outcomes.ts b/src/services/recommendation-outcomes.ts index c31e15b844..a0ac18accc 100644 --- a/src/services/recommendation-outcomes.ts +++ b/src/services/recommendation-outcomes.ts @@ -1,5 +1,6 @@ import { listAgentActions, + listAgentContextSnapshots, listAgentRunsForActor, listContributorIssues, listContributorPullRequests, @@ -40,9 +41,14 @@ export async function evaluateRecommendationOutcomes( listContributorIssues(env, login), ]); const completedRuns = runs.filter((run) => run.status === "completed"); - const actionGroups = await Promise.all(completedRuns.map(async (run) => ({ run, actions: await listAgentActions(env, run.id) }))); - const classifications = actionGroups.flatMap(({ run, actions }) => - actions.map((action) => classifyRecommendationOutcome({ run, action, pullRequests, issues, evaluatedAt, staleAfterMs, ignoredAfterMs })), + const actionGroups = await Promise.all( + completedRuns.map(async (run) => { + const [actions, snapshots] = await Promise.all([listAgentActions(env, run.id), listAgentContextSnapshots(env, run.id)]); + return { run, actions, snapshotId: snapshots[0]?.id ?? null }; + }), + ); + const classifications = actionGroups.flatMap(({ run, actions, snapshotId }) => + actions.map((action) => classifyRecommendationOutcome({ run, action, pullRequests, issues, evaluatedAt, staleAfterMs, ignoredAfterMs, snapshotId })), ); const classified = classifications.filter((outcome): outcome is AgentRecommendationOutcomeRecord => outcome !== null); const outcomes = []; @@ -63,6 +69,7 @@ export function classifyRecommendationOutcome(args: { evaluatedAt: string; staleAfterMs: number; ignoredAfterMs: number; + snapshotId?: string | null | undefined; }): AgentRecommendationOutcomeRecord | null { const actionAt = timestamp(args.action.createdAt ?? args.run.updatedAt ?? args.run.createdAt); const evaluatedAt = timestamp(args.evaluatedAt); @@ -82,6 +89,7 @@ export function classifyRecommendationOutcome(args: { actionAt, actionAgeMs, staleAfterMs: args.staleAfterMs, + snapshotId: args.snapshotId ?? null, }); } @@ -98,6 +106,7 @@ export function classifyRecommendationOutcome(args: { actionAt, actionAgeMs, staleAfterMs: args.staleAfterMs, + snapshotId: args.snapshotId ?? null, }); } @@ -112,6 +121,7 @@ export function classifyRecommendationOutcome(args: { actionAt, actionAgeMs, staleAfterMs: args.staleAfterMs, + snapshotId: args.snapshotId ?? null, }); } @@ -126,11 +136,13 @@ export function classifyRecommendationOutcome(args: { actionAt, actionAgeMs, staleAfterMs: args.staleAfterMs, + snapshotId: args.snapshotId ?? null, }); } if (actionAgeMs < args.ignoredAfterMs) return null; return baseOutcome(args.run, args.action, { + snapshotId: args.snapshotId ?? null, outcomeState: "ignored", outcomeTargetType: targetRepoFullName ? "repository" : "none", outcomeRepoFullName: targetRepoFullName ?? null, @@ -153,10 +165,12 @@ function outcomeFromPullRequest(args: { actionAt: number; actionAgeMs: number; staleAfterMs: number; + snapshotId?: string | null | undefined; }): AgentRecommendationOutcomeRecord { const state = pullRequestOutcomeState(args.pr, args.action, args.actionAt, args.actionAgeMs, args.staleAfterMs); const maintainerLane = isMaintainerLane(args.run.actorLogin, args.pr.repoFullName, args.pr.authorAssociation); return baseOutcome(args.run, args.action, { + snapshotId: args.snapshotId ?? null, outcomeState: state, outcomeTargetType: "pull_request", outcomeRepoFullName: args.pr.repoFullName, @@ -184,10 +198,12 @@ function outcomeFromIssue(args: { actionAt: number; actionAgeMs: number; staleAfterMs: number; + snapshotId?: string | null | undefined; }): AgentRecommendationOutcomeRecord { const state = issueOutcomeState(args.issue, args.actionAt, args.actionAgeMs, args.staleAfterMs); const maintainerLane = isMaintainerLane(args.run.actorLogin, args.issue.repoFullName, args.issue.authorAssociation); return baseOutcome(args.run, args.action, { + snapshotId: args.snapshotId ?? null, outcomeState: state, outcomeTargetType: "issue", outcomeRepoFullName: args.issue.repoFullName, @@ -208,6 +224,7 @@ function baseOutcome( run: AgentRunRecord, action: AgentActionRecord, outcome: { + snapshotId?: string | null | undefined; outcomeState: AgentRecommendationOutcomeState; outcomeTargetType: AgentRecommendationOutcomeTargetType; outcomeRepoFullName?: string | null | undefined; @@ -226,6 +243,8 @@ function baseOutcome( runId: run.id, actorLogin: run.actorLogin, actionType: action.actionType, + surface: run.surface, + snapshotId: outcome.snapshotId ?? null, targetRepoFullName: action.targetRepoFullName, targetPullNumber: action.targetPullNumber, targetIssueNumber: action.targetIssueNumber, @@ -262,6 +281,7 @@ function pullRequestOutcomeState( if (pr.state === "closed" && updatedAt >= actionAt) return "closed"; const positiveOpenSignal = pr.reviewDecision === "APPROVED" || pr.mergeableState === "clean"; if (action.targetPullNumber && positiveOpenSignal && updatedAt >= actionAt) return "improved"; + if (action.targetPullNumber && pr.reviewDecision === "CHANGES_REQUESTED" && updatedAt >= actionAt) return "rejected"; if (createdAt >= actionAt || updatedAt > actionAt) return "accepted"; if (actionAgeMs >= staleAfterMs) return "stale"; return "ignored"; @@ -280,6 +300,7 @@ function pullRequestOutcomeReason(state: AgentRecommendationOutcomeState, pr: Pu if (state === "merged") return `${pr.repoFullName}#${pr.number} merged after the recommendation snapshot.`; if (state === "closed") return `${pr.repoFullName}#${pr.number} closed without a merge after the recommendation snapshot.`; if (state === "improved") return `${pr.repoFullName}#${pr.number} remains open but now has approval or clean mergeability evidence.`; + if (state === "rejected") return `${pr.repoFullName}#${pr.number} received a changes-requested review decision after the recommendation snapshot.`; if (state === "accepted") return `${pr.repoFullName}#${pr.number} shows later cached activity matching the recommendation.`; if (state === "stale") return `${pr.repoFullName}#${pr.number} remains open with no later activity past the stale-outcome window.`; return `${pr.repoFullName}#${pr.number} is visible but has no later positive or terminal outcome yet.`; diff --git a/src/types.ts b/src/types.ts index 651e83ce7d..16e28420b4 100644 --- a/src/types.ts +++ b/src/types.ts @@ -684,7 +684,7 @@ export type AgentContextSnapshotRecord = { createdAt?: string | null | undefined; }; -export type AgentRecommendationOutcomeState = "accepted" | "ignored" | "stale" | "merged" | "closed" | "improved"; +export type AgentRecommendationOutcomeState = "accepted" | "rejected" | "ignored" | "stale" | "merged" | "closed" | "improved"; export type AgentRecommendationOutcomeTargetType = "pull_request" | "issue" | "repository" | "none"; export type AgentRecommendationOutcomeConfidence = "high" | "medium" | "low"; @@ -694,6 +694,8 @@ export type AgentRecommendationOutcomeRecord = { runId: string; actorLogin: string; actionType: AgentActionType; + surface?: AgentSurface | null | undefined; + snapshotId?: string | null | undefined; targetRepoFullName?: string | null | undefined; targetPullNumber?: number | null | undefined; targetIssueNumber?: number | null | undefined; @@ -721,6 +723,7 @@ export type AgentRecommendationOutcomeRepoSummary = { repoFullName: string; total: number; accepted: number; + rejected: number; ignored: number; stale: number; merged: number; @@ -740,6 +743,7 @@ export type AgentRecommendationOutcomeSummary = { totals: { total: number; accepted: number; + rejected: number; ignored: number; stale: number; merged: number; diff --git a/test/unit/decision-pack.test.ts b/test/unit/decision-pack.test.ts index 1a0f052746..15f6f9578f 100644 --- a/test/unit/decision-pack.test.ts +++ b/test/unit/decision-pack.test.ts @@ -159,6 +159,7 @@ describe("decision-pack service", () => { repoFullName: "owner/direct", total: 4, accepted: 1, + rejected: 0, ignored: 1, stale: 0, merged: 1, @@ -193,6 +194,7 @@ describe("decision-pack service", () => { repoFullName: "owner/direct", total: 6, accepted: 0, + rejected: 0, ignored: 2, stale: 2, merged: 0, @@ -213,6 +215,7 @@ describe("decision-pack service", () => { repoFullName: "owner/direct", total: 4, accepted: 1, + rejected: 0, ignored: 1, stale: 0, merged: 1, @@ -249,6 +252,7 @@ describe("decision-pack service", () => { repoFullName: "owner/direct", total: 0, accepted: 0, + rejected: 0, ignored: 0, stale: 0, merged: 0, @@ -269,6 +273,7 @@ describe("decision-pack service", () => { repoFullName: "owner/direct", total: 2, accepted: 0, + rejected: 0, ignored: 1, stale: 0, merged: 0, @@ -315,6 +320,7 @@ describe("decision-pack service", () => { totals: { total: 1, accepted: 1, + rejected: 0, ignored: 0, stale: 0, merged: 0, diff --git a/test/unit/recommendation-outcomes.test.ts b/test/unit/recommendation-outcomes.test.ts index 7e09c74ad1..e52bda8c87 100644 --- a/test/unit/recommendation-outcomes.test.ts +++ b/test/unit/recommendation-outcomes.test.ts @@ -3,13 +3,14 @@ import { createAgentRun, getAgentRecommendationOutcomeSummary, listAgentRecommendationOutcomes, + persistAgentContextSnapshot, replaceAgentActions, upsertAgentRecommendationOutcome, upsertIssueFromGitHub, upsertPullRequestFromGitHub, } from "../../src/db/repositories"; import { classifyRecommendationOutcome, evaluateRecommendationOutcomes } from "../../src/services/recommendation-outcomes"; -import type { AgentActionRecord, AgentRunRecord, GitHubIssuePayload, GitHubPullRequestPayload, IssueRecord, PullRequestRecord } from "../../src/types"; +import type { AgentActionRecord, AgentContextSnapshotRecord, AgentRunRecord, GitHubIssuePayload, GitHubPullRequestPayload, IssueRecord, PullRequestRecord } from "../../src/types"; import { createTestEnv } from "../helpers/d1"; describe("recommendation outcome feedback", () => { @@ -351,6 +352,122 @@ describe("recommendation outcome feedback", () => { ).toMatchObject({ maintainerLane: true }); }); + it("classifies rejected, and all seven outcome states are reachable from PR fixtures", () => { + const run = runRecord("run-rejected", "dev", "2026-05-01T00:00:00.000Z"); + const base = { + run, + evaluatedAt: "2026-06-01T00:00:00.000Z", + staleAfterMs: 14 * 24 * 60 * 60 * 1000, + ignoredAfterMs: 7 * 24 * 60 * 60 * 1000, + }; + + expect( + classifyRecommendationOutcome({ + ...base, + action: action(run, 0, { targetRepoFullName: "owner/rejected-pr", targetPullNumber: 200 }), + pullRequests: [prRecord(200, "owner/rejected-pr", { updatedAt: "2026-05-10T00:00:00.000Z", reviewDecision: "CHANGES_REQUESTED" })], + issues: [], + }), + ).toMatchObject({ outcomeState: "rejected", outcomeTargetType: "pull_request", outcomePullNumber: 200, confidence: "high" }); + + expect( + classifyRecommendationOutcome({ + ...base, + action: action(run, 1, { targetRepoFullName: "owner/merged-pr", targetPullNumber: 201 }), + pullRequests: [prRecord(201, "owner/merged-pr", { state: "closed", mergedAt: "2026-05-05T00:00:00.000Z", updatedAt: "2026-05-05T00:00:00.000Z" })], + issues: [], + }), + ).toMatchObject({ outcomeState: "merged" }); + + expect( + classifyRecommendationOutcome({ + ...base, + action: action(run, 2, { targetRepoFullName: "owner/closed-pr", targetPullNumber: 202 }), + pullRequests: [prRecord(202, "owner/closed-pr", { state: "closed", updatedAt: "2026-05-05T00:00:00.000Z" })], + issues: [], + }), + ).toMatchObject({ outcomeState: "closed" }); + + expect( + classifyRecommendationOutcome({ + ...base, + action: action(run, 3, { targetRepoFullName: "owner/improved-pr", targetPullNumber: 203 }), + pullRequests: [prRecord(203, "owner/improved-pr", { updatedAt: "2026-05-05T00:00:00.000Z", reviewDecision: "APPROVED" })], + issues: [], + }), + ).toMatchObject({ outcomeState: "improved" }); + + expect( + classifyRecommendationOutcome({ + ...base, + action: action(run, 4, { targetRepoFullName: "owner/accepted-pr", targetPullNumber: 204 }), + pullRequests: [prRecord(204, "owner/accepted-pr", { updatedAt: "2026-05-05T00:00:00.000Z" })], + issues: [], + }), + ).toMatchObject({ outcomeState: "accepted" }); + + expect( + classifyRecommendationOutcome({ + ...base, + action: action(run, 5, { targetRepoFullName: "owner/stale-pr", targetPullNumber: 205 }), + pullRequests: [prRecord(205, "owner/stale-pr", { createdAt: "2026-04-01T00:00:00.000Z", updatedAt: "2026-04-01T00:00:00.000Z" })], + issues: [], + }), + ).toMatchObject({ outcomeState: "stale" }); + + expect( + classifyRecommendationOutcome({ + ...base, + action: action(run, 6, { targetRepoFullName: "owner/ignored-repo" }), + pullRequests: [], + issues: [], + }), + ).toMatchObject({ outcomeState: "ignored" }); + }); + + it("captures surface and snapshotId on persisted outcome records", async () => { + const env = createTestEnv(); + const run = runRecord("run-surface", "dev", "2026-05-01T00:00:00.000Z"); + await createAgentRun(env, run); + const snap: AgentContextSnapshotRecord = { + id: "snap-surface-001", + runId: run.id, + repoSignalSnapshotIds: [], + freshnessWarnings: [], + payload: {}, + }; + await persistAgentContextSnapshot(env, snap); + await replaceAgentActions(env, run.id, [action(run, 0, { targetRepoFullName: "owner/surface-pr", targetPullNumber: 300 })]); + await upsertPullRequestFromGitHub(env, "owner/surface-pr", pr(300, { state: "closed", merged_at: "2026-05-05T00:00:00.000Z", created_at: "2026-05-02T00:00:00.000Z", updated_at: "2026-05-05T00:00:00.000Z" })); + + const result = await evaluateRecommendationOutcomes(env, "dev", { now: "2026-06-01T00:00:00.000Z" }); + expect(result.outcomes).toHaveLength(1); + const outcome = result.outcomes[0]!; + expect(outcome.surface).toBe("api"); + expect(outcome.snapshotId).toBe("snap-surface-001"); + expect(outcome.outcomeState).toBe("merged"); + }); + + it("does not expose private outcome fields in public-safe action summaries or serialized event JSON", async () => { + const env = createTestEnv(); + const run = runRecord("run-privacy", "dev", "2026-05-01T00:00:00.000Z"); + await createAgentRun(env, run); + await replaceAgentActions(env, run.id, [action(run, 0, { targetRepoFullName: "owner/private-pr", targetPullNumber: 400, publicSafeSummary: "Open a small scoped PR." })]); + await upsertPullRequestFromGitHub(env, "owner/private-pr", pr(400, { state: "closed", merged_at: "2026-05-05T00:00:00.000Z", created_at: "2026-05-02T00:00:00.000Z", updated_at: "2026-05-05T00:00:00.000Z" })); + + const result = await evaluateRecommendationOutcomes(env, "dev", { now: "2026-06-01T00:00:00.000Z" }); + const outcome = result.outcomes[0]!; + + const privateSerialized = JSON.stringify(outcome); + expect(privateSerialized).not.toMatch(/scoreabilit|reward|payout|wallet|hotkey|coldkey|raw trust/i); + + const summary = await getAgentRecommendationOutcomeSummary(env, "dev", { now: "2026-06-01T00:00:00.000Z" }); + expect(summary.privateSummary).not.toMatch(/scoreabilit|reward|payout|wallet|hotkey|coldkey|raw trust/i); + expect(summary.totals.total).toBe(1); + expect(summary.totals.merged).toBe(1); + expect(summary.totals.rejected).toBe(0); + }); + it("maps legacy recommendation outcome rows to safe enum defaults", async () => { const env = createTestEnv(); const run = runRecord("legacy-run", "dev", "2026-05-01T00:00:00.000Z");