From e77703604359a304d91d293638b46f2e4d0a1601 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Sat, 31 Oct 2026 15:38:30 +0100 Subject: [PATCH 1/3] feat(code-review): Team tab with a per-person scorecard for reviews and CI One row per GitHub login from PR reviews and CI runs: merged, time to merge, fix rate at merge, critical findings per PR, first-try green, reruns, shipped clean and runner time, shown next to the team median. Ranked is opt-in and ranks rates only, for people with at least five reviewed pull requests. Bots are left out, and CI runs with no job that ran (fork runs waiting for approval) are dropped, so they no longer read as hours of failed CI. --- .../src/routes/internal/code-review.http.ts | 5 + apps/api/src/runtime/service-graph.ts | 2 + apps/web/src/lab/entries/code-review-team.ts | 241 +++++++++ apps/web/src/lab/registry.ts | 8 + apps/web/src/page/code-review/index.ts | 12 +- apps/web/src/page/code-review/kit/api.ts | 3 + apps/web/src/page/code-review/kit/layout.ts | 5 +- apps/web/src/page/code-review/kit/retained.ts | 4 + apps/web/src/page/code-review/tabs-update.ts | 25 + apps/web/src/page/code-review/tabs.ts | 3 + .../src/page/code-review/team/derive.test.ts | 117 +++++ apps/web/src/page/code-review/team/derive.ts | 154 ++++++ apps/web/src/page/code-review/team/model.ts | 41 ++ .../page/code-review/team/team.scene.test.ts | 121 +++++ apps/web/src/page/code-review/team/update.ts | 138 +++++ apps/web/src/page/code-review/team/view.ts | 485 ++++++++++++++++++ apps/web/src/route/app-route.ts | 1 + apps/web/src/route/routers.ts | 5 + apps/web/src/route/search.ts | 24 + apps/web/src/ui/icons.generated.ts | 154 ++++++ .../pr-review/CodeReviewTeamService.test.ts | 309 +++++++++++ .../pr-review/CodeReviewTeamService.ts | 362 +++++++++++++ packages/domain/src/http/code-review.ts | 69 +++ .../src/__sql_baseline__/catalog.sql | 31 ++ .../query-engine/src/benchmark/builders.ts | 12 + packages/query-engine/src/ch/index.ts | 2 + .../query-engine/src/ch/queries/ci-runs.ts | 65 +++ 27 files changed, 2396 insertions(+), 2 deletions(-) create mode 100644 apps/web/src/lab/entries/code-review-team.ts create mode 100644 apps/web/src/page/code-review/team/derive.test.ts create mode 100644 apps/web/src/page/code-review/team/derive.ts create mode 100644 apps/web/src/page/code-review/team/model.ts create mode 100644 apps/web/src/page/code-review/team/team.scene.test.ts create mode 100644 apps/web/src/page/code-review/team/update.ts create mode 100644 apps/web/src/page/code-review/team/view.ts create mode 100644 packages/backend/src/services/pr-review/CodeReviewTeamService.test.ts create mode 100644 packages/backend/src/services/pr-review/CodeReviewTeamService.ts diff --git a/apps/api/src/routes/internal/code-review.http.ts b/apps/api/src/routes/internal/code-review.http.ts index a57293be08..7f07219f42 100644 --- a/apps/api/src/routes/internal/code-review.http.ts +++ b/apps/api/src/routes/internal/code-review.http.ts @@ -2,12 +2,14 @@ import { HttpApiBuilder } from "effect/http-api" import { CurrentTenant, MapleInternalApi } from "@maple/domain/http" import { Effect } from "effect" import { CiAnalyticsService } from "@maple/backend/services/pr-review/CiAnalyticsService" +import { CodeReviewTeamService } from "@maple/backend/services/pr-review/CodeReviewTeamService" import { PrReviewAnalyticsService } from "@maple/backend/services/pr-review/PrReviewAnalyticsService" export const HttpCodeReviewLive = HttpApiBuilder.group(MapleInternalApi, "codeReview", (handlers) => Effect.gen(function* () { const analytics = yield* PrReviewAnalyticsService const ci = yield* CiAnalyticsService + const teams = yield* CodeReviewTeamService return handlers .handle("analytics", ({ query }) => @@ -40,5 +42,8 @@ export const HttpCodeReviewLive = HttpApiBuilder.group(MapleInternalApi, "codeRe .handle("ciRuns", ({ query }) => Effect.flatMap(CurrentTenant.Context, (tenant) => ci.listRuns(tenant, query)), ) + .handle("team", ({ query }) => + Effect.flatMap(CurrentTenant.Context, (tenant) => teams.team(tenant, query)), + ) }), ) diff --git a/apps/api/src/runtime/service-graph.ts b/apps/api/src/runtime/service-graph.ts index d64c2ba605..570eac9ac3 100644 --- a/apps/api/src/runtime/service-graph.ts +++ b/apps/api/src/runtime/service-graph.ts @@ -96,6 +96,7 @@ import { TelemetryReadService } from "@maple/backend/services/warehouse/Telemetr import { OrgClickHouseSettingsService } from "@maple/backend/services/org/OrgClickHouseSettingsService" import { VcsSourceService } from "@maple/backend/services/integrations/vcs/VcsSourceService" import { CiAnalyticsService } from "@maple/backend/services/pr-review/CiAnalyticsService" +import { CodeReviewTeamService } from "@maple/backend/services/pr-review/CodeReviewTeamService" import { PrReviewAnalyticsService } from "@maple/backend/services/pr-review/PrReviewAnalyticsService" /** Services consumed by HTTP routes; each service owns its implementation dependencies. */ @@ -193,6 +194,7 @@ export const HttpServicesLive = Layer.mergeAll( SourceRemovalService.layer, PrReviewAnalyticsService.layer, CiAnalyticsService.layer, + CodeReviewTeamService.layer, VcsCommitService.layer, VcsSourceService.layer, ChatWorkspaceService.layer, diff --git a/apps/web/src/lab/entries/code-review-team.ts b/apps/web/src/lab/entries/code-review-team.ts new file mode 100644 index 0000000000..b1191fc392 --- /dev/null +++ b/apps/web/src/lab/entries/code-review-team.ts @@ -0,0 +1,241 @@ +import { OrgId } from "@maple/domain" +import { CodeReviewTeam } from "@maple/domain/http" +import { Clock, Effect, Result, Schema } from "effect" +import { Command } from "foldkit" +import type { PageContext } from "../../page/contract" +import * as Team from "../../page/code-review/team/model" +import { init, routeChanged, update } from "../../page/code-review/team/update" +import { view } from "../../page/code-review/team/view" +import { CodeReviewTeamSearch } from "../../route/search" +import { defineShellLab } from "../shell-lab" + +// `/lab/code-review-team`: the Code Review Team tab over a month of a seven-person fixture team, +// inside the real shell. The reads answer from the fixture; sorting and Ranked follow the URL +// the page asks for. + +const DAY = 86_400_000 +const HOUR_MS = 3_600_000 + +const member = ( + login: string, + counts: Readonly<{ + prs: number + merged: number + mergeHours: number | null + findings: number + critical: number + resolved: number + openAtMerge: number + clean: number + regressed: number + ci: number + firstTry: number + green: number + reruns: number + cancelled: number + runnerHours: number + }>, +) => ({ + login, + pullRequests: counts.prs, + mergedPullRequests: counts.merged, + medianMergeSeconds: counts.mergeHours === null ? null : counts.mergeHours * 3_600, + findings: counts.findings, + criticalFindings: counts.critical, + resolvedFindings: counts.resolved, + openAtMergeFindings: counts.openAtMerge, + shippedClean: counts.clean, + shippedRegressed: counts.regressed, + ciRuns: counts.ci, + firstTryRuns: counts.firstTry, + firstTryGreen: counts.green, + reruns: counts.reruns, + cancelledRuns: counts.cancelled, + runnerMs: counts.runnerHours * HOUR_MS, +}) + +// Logins no GitHub account holds, so the avatars fall back to initials. +const teamAt = (nowMs: number) => + Schema.decodeUnknownSync(CodeReviewTeam)({ + startTime: nowMs - 30 * DAY, + endTime: nowMs, + totals: { + pullRequests: 101, + mergedPullRequests: 88, + medianMergeSeconds: 4.5 * 3_600, + medianFirstReviewSeconds: 250, + defaultBranchRuns: 640, + defaultBranchFailed: 22, + runnerMs: 396 * HOUR_MS, + p95QueueMs: 48_000, + }, + members: [ + member("acme-ada", { + prs: 46, + merged: 41, + mergeHours: 3.1, + findings: 88, + critical: 6, + resolved: 61, + openAtMerge: 9, + clean: 30, + regressed: 1, + ci: 1_240, + firstTry: 860, + green: 790, + reruns: 14, + cancelled: 330, + runnerHours: 210, + }), + member("acme-jonas", { + prs: 22, + merged: 18, + mergeHours: 7.8, + findings: 40, + critical: 5, + resolved: 22, + openAtMerge: 10, + clean: 12, + regressed: 2, + ci: 520, + firstTry: 380, + green: 322, + reruns: 9, + cancelled: 120, + runnerHours: 80, + }), + member("acme-leo", { + prs: 0, + merged: 0, + mergeHours: null, + findings: 0, + critical: 0, + resolved: 0, + openAtMerge: 0, + clean: 0, + regressed: 0, + ci: 120, + firstTry: 90, + green: 81, + reruns: 2, + cancelled: 20, + runnerHours: 12, + }), + member("acme-mei", { + prs: 6, + merged: 6, + mergeHours: 4.2, + findings: 5, + critical: 0, + resolved: 4, + openAtMerge: 0, + clean: 5, + regressed: 0, + ci: 90, + firstTry: 70, + green: 66, + reruns: 0, + cancelled: 15, + runnerHours: 9, + }), + member("acme-priya", { + prs: 15, + merged: 14, + mergeHours: 1.6, + findings: 18, + critical: 1, + resolved: 15, + openAtMerge: 1, + clean: 11, + regressed: 0, + ci: 300, + firstTry: 220, + green: 214, + reruns: 1, + cancelled: 70, + runnerHours: 40, + }), + member("acme-sam", { + prs: 3, + merged: 2, + mergeHours: 50, + findings: 6, + critical: 1, + resolved: 2, + openAtMerge: 2, + clean: 1, + regressed: 0, + ci: 40, + firstTry: 30, + green: 24, + reruns: 3, + cancelled: 8, + runnerHours: 4, + }), + member("acme-tomas", { + prs: 9, + merged: 7, + mergeHours: 26, + findings: 30, + critical: 4, + resolved: 9, + openAtMerge: 8, + clean: 4, + regressed: 1, + ci: 260, + firstTry: 180, + green: 141, + reruns: 12, + cancelled: 50, + runnerHours: 35, + }), + ], + }) + +const context: PageContext = { + orgId: Schema.decodeSync(OrgId)("org_lab_team"), + pinnedNamespace: null, + timeZone: "UTC", +} + +const ServeFixtureTeam = Command.define("ServeFixtureCodeReviewTeam", { + args: { key: Schema.String }, + messages: [Team.Message.CompletedFetchTeam], + execute: ({ key }) => + Effect.map(Clock.currentTimeMillis, (nowMs) => + Team.Message.CompletedFetchTeam({ key, result: Result.succeed(teamAt(nowMs)) }), + ), +}) + +const decodeSearch = Schema.decodeUnknownSync(CodeReviewTeamSearch) +type PageStep = ReturnType + +/** The page's reads answered by the fixture; the GitHub status read is dropped (no repositories). */ +const served = (result: PageStep) => ({ + model: result.model, + commands: (result.commands ?? []).flatMap((command) => { + if (command.name === "FetchCodeReviewTeam") + return [ServeFixtureTeam({ key: result.model.team.key ?? "" })] + return command.name === "FetchCodeReviewGithubStatus" ? [] : [command] + }), +}) + +/** A navigation the page asks for becomes the lab's own route change. */ +const followed = (result: PageStep) => { + const out = result.outMessage + if (out === undefined || out._tag !== "RequestedNavigation") return served(result) + const search = decodeSearch(Object.fromEntries(new URL(out.url, "https://lab.local").searchParams)) + return served(routeChanged(result.model, { search }, context)) +} + +export const lab = defineShellLab("Code review team", { + Model: Team.Model, + Message: Team.Message, + Flags: Schema.Struct({}), + flags: Effect.succeed({}), + init: () => served(init({ search: {} }, context)), + update: (model, message) => followed(update(model, message, context)), + view: (model, h) => view(model, { context }, h), + currentPath: "/code-review/team", + breadcrumbs: [{ label: "Code Review", href: "/code-review" }, { label: "Team" }], +}) diff --git a/apps/web/src/lab/registry.ts b/apps/web/src/lab/registry.ts index d8f9ad6cbd..62b6b48a0e 100644 --- a/apps/web/src/lab/registry.ts +++ b/apps/web/src/lab/registry.ts @@ -234,6 +234,14 @@ export const LAB_ENTRIES: ReadonlyArray = [ kind: "lab", session: "none", }, + { + path: "/lab/code-review-team", + title: "Code review team", + description: + "The Code Review Team tab over a month of a seven-person fixture team: KPIs, highlights, the scorecard against the team median and the Ranked view.", + kind: "lab", + session: "none", + }, { path: "/lab/service-api", title: "Service API tab", diff --git a/apps/web/src/page/code-review/index.ts b/apps/web/src/page/code-review/index.ts index 2e456f5531..846a1aa332 100644 --- a/apps/web/src/page/code-review/index.ts +++ b/apps/web/src/page/code-review/index.ts @@ -8,6 +8,7 @@ import { view as issuesView } from "./issues/view" import { subscriptions as pullRequestsSubscriptions } from "./pull-requests/subscription" import { view as pullRequestsView } from "./pull-requests/view" import { view as settingsView } from "./settings/view" +import { view as teamView } from "./team/view" import { Message, Model } from "./tabs" import { breadcrumbs, contextChanged, init, routeChanged, update } from "./tabs-update" @@ -39,6 +40,14 @@ const view = Submodel.defineView((model, inputs, viewInputs: inputs, toParentMessage: (message) => Message.GotCiMessage({ message }), }) + case "Team": + return h.submodel({ + slotId: "code-review-team", + model: tab.model, + view: teamView, + viewInputs: inputs, + toParentMessage: (message) => Message.GotTeamMessage({ message }), + }) case "Issues": return h.submodel({ slotId: "code-review-issues", @@ -66,7 +75,7 @@ const subscriptions = Subscription.lift(pullRequestsSubscriptions), Message, Resources> /** - * `/code-review` and its tabs (Analytics, Pull requests, CI, Issues, Settings) as one instance per + * `/code-review` and its tabs (Analytics, Pull requests, CI, Team, Issues, Settings) as one instance per * org: a tab change keeps the instance, so what one tab read serves the next. Filters and windows * are search params; each tab's own update handles them. */ @@ -78,6 +87,7 @@ export const page = definePage( "CodeReview", "CodeReviewPullRequests", "CodeReviewCi", + "CodeReviewTeam", "CodeReviewIssues", "CodeReviewSettings", ], diff --git a/apps/web/src/page/code-review/kit/api.ts b/apps/web/src/page/code-review/kit/api.ts index 83b67bbaff..df5144b770 100644 --- a/apps/web/src/page/code-review/kit/api.ts +++ b/apps/web/src/page/code-review/kit/api.ts @@ -4,6 +4,7 @@ import type { CodeReviewCiRunsQuery, CodeReviewFindingsQuery, CodeReviewListQuery, + CodeReviewTeamQuery, PrReviewId, } from "@maple/domain/http" import { toEpochMs } from "@maple/ui/lib/time-format" @@ -45,3 +46,5 @@ export const getCiAnalytics = (query: CodeReviewCiQuery) => call((api) => api.codeReview.ciAnalytics({ query })) export const listCiRuns = (query: CodeReviewCiRunsQuery) => call((api) => api.codeReview.ciRuns({ query })) + +export const getTeam = (query: CodeReviewTeamQuery) => call((api) => api.codeReview.team({ query })) diff --git a/apps/web/src/page/code-review/kit/layout.ts b/apps/web/src/page/code-review/kit/layout.ts index 71b3bfd988..157eeed316 100644 --- a/apps/web/src/page/code-review/kit/layout.ts +++ b/apps/web/src/page/code-review/kit/layout.ts @@ -7,7 +7,7 @@ import type { WindowSearch } from "./frame" // `CodeReviewLayout`, `CodeReviewTabs` and `NothingInWindow`. -export type CodeReviewTab = "analytics" | "pull-requests" | "ci" | "issues" | "settings" +export type CodeReviewTab = "analytics" | "pull-requests" | "ci" | "team" | "issues" | "settings" export type CarriedSearch = WindowSearch & Readonly<{ repo?: string; author?: string }> @@ -20,6 +20,7 @@ const TABS: ReadonlyArray<{ tab: CodeReviewTab; path: string; label: string; ico icon: "PullRequestIcon", }, { tab: "ci", path: "/code-review/ci", label: "CI", icon: "PlayRotateClockwiseIcon" }, + { tab: "team", path: "/code-review/team", label: "Team", icon: "UsersIcon" }, { tab: "issues", path: "/code-review/issues", label: "Issues", icon: "CircleWarningIcon" }, { tab: "settings", path: "/code-review/settings", label: "Settings", icon: "GearIcon" }, ] @@ -47,6 +48,8 @@ const hrefFor = (tab: CodeReviewTab, search: Readonly): Retained reviews: { ...base.reviews, ...next.reviews }, findings: { ...base.findings, ...next.findings }, ciRuns: { ...base.ciRuns, ...next.ciRuns }, + team: { ...base.team, ...next.team }, details: { ...base.details, ...next.details }, githubStatus: { ...base.githubStatus, ...next.githubStatus }, }) diff --git a/apps/web/src/page/code-review/tabs-update.ts b/apps/web/src/page/code-review/tabs-update.ts index 59c0d88ad5..299120a66e 100644 --- a/apps/web/src/page/code-review/tabs-update.ts +++ b/apps/web/src/page/code-review/tabs-update.ts @@ -8,6 +8,7 @@ import type * as CiTab from "./ci/model" import type * as IssuesTab from "./issues/model" import type * as PullRequestsTab from "./pull-requests/model" import type * as SettingsTab from "./settings/model" +import type * as TeamTab from "./team/model" import * as Ci from "./ci/update" import * as Issues from "./issues/update" import type { Carried } from "./kit/frame-update" @@ -15,6 +16,7 @@ import { breadcrumbsOf } from "./kit/layout" import { emptyRetained, mergeRetained, type Retained } from "./kit/retained" import * as PullRequests from "./pull-requests/update" import * as Settings from "./settings/update" +import * as Team from "./team/update" import { Message, type Model, type Tab } from "./tabs" type Return = PageReturn @@ -26,6 +28,7 @@ export type CodeReviewRoute = Extract< | "CodeReview" | "CodeReviewPullRequests" | "CodeReviewCi" + | "CodeReviewTeam" | "CodeReviewIssues" | "CodeReviewSettings" } @@ -46,12 +49,14 @@ const lift = ( const toAnalytics = (model: AnalyticsTab.Model): Tab => ({ _tag: "Analytics", model }) const toPullRequests = (model: PullRequestsTab.Model): Tab => ({ _tag: "PullRequests", model }) const toCi = (model: CiTab.Model): Tab => ({ _tag: "Ci", model }) +const toTeam = (model: TeamTab.Model): Tab => ({ _tag: "Team", model }) const toIssues = (model: IssuesTab.Model): Tab => ({ _tag: "Issues", model }) const toSettings = (model: SettingsTab.Model): Tab => ({ _tag: "Settings", model }) const wrapAnalytics = (message: AnalyticsMessage.Message) => Message.GotAnalyticsMessage({ message }) const wrapPullRequests = (message: PullRequestsTab.Message) => Message.GotPullRequestsMessage({ message }) const wrapCi = (message: CiTab.Message) => Message.GotCiMessage({ message }) +const wrapTeam = (message: TeamTab.Message) => Message.GotTeamMessage({ message }) const wrapIssues = (message: IssuesTab.Message) => Message.GotIssuesMessage({ message }) const wrapSettings = (message: SettingsTab.Message) => Message.GotSettingsMessage({ message }) @@ -64,6 +69,8 @@ const tabTagOf = (route: CodeReviewRoute): Tab["_tag"] => { return "PullRequests" case "CodeReviewCi": return "Ci" + case "CodeReviewTeam": + return "Team" case "CodeReviewIssues": return "Issues" case "CodeReviewSettings": @@ -81,6 +88,8 @@ const enter = (base: Omit, route: CodeReviewRoute, context: PageCo return lift(base, PullRequests.init(route, context, carried), toPullRequests, wrapPullRequests) case "CodeReviewCi": return lift(base, Ci.init(route, context, carried), toCi, wrapCi) + case "CodeReviewTeam": + return lift(base, Team.init(route, context, carried), toTeam, wrapTeam) case "CodeReviewIssues": return lift(base, Issues.init(route, context, carried), toIssues, wrapIssues) case "CodeReviewSettings": @@ -107,6 +116,11 @@ const leaving = (model: Model): Readonly<{ retained: Retained; nowMs: number }> retained: mergeRetained(model.retained, Ci.retainedOf(tab.model)), nowMs: tab.model.frame.nowMs, } + case "Team": + return { + retained: mergeRetained(model.retained, Team.retainedOf(tab.model)), + nowMs: tab.model.frame.nowMs, + } case "Issues": return { retained: mergeRetained(model.retained, Issues.retainedOf(tab.model)), @@ -145,6 +159,10 @@ export const routeChanged = (model: Model, route: CodeReviewRoute, context: Page return tab._tag === "Ci" ? lift(model, Ci.routeChanged(tab.model, route, context), toCi, wrapCi) : { model } + case "CodeReviewTeam": + return tab._tag === "Team" + ? lift(model, Team.routeChanged(tab.model, route, context), toTeam, wrapTeam) + : { model } case "CodeReviewIssues": return tab._tag === "Issues" ? lift(model, Issues.routeChanged(tab.model, route, context), toIssues, wrapIssues) @@ -168,6 +186,8 @@ export const contextChanged = (model: Model, context: PageContext): Return => { ) case "Ci": return lift(model, Ci.contextChanged(tab.model, context), toCi, wrapCi) + case "Team": + return lift(model, Team.contextChanged(tab.model, context), toTeam, wrapTeam) case "Issues": return lift(model, Issues.contextChanged(tab.model, context), toIssues, wrapIssues) case "Settings": @@ -194,6 +214,10 @@ export const update = (model: Model, message: Message, context: PageContext): Re : { model }, GotCiMessage: ({ message: inner }) => tab._tag === "Ci" ? lift(model, Ci.update(tab.model, inner, context), toCi, wrapCi) : { model }, + GotTeamMessage: ({ message: inner }) => + tab._tag === "Team" + ? lift(model, Team.update(tab.model, inner, context), toTeam, wrapTeam) + : { model }, GotIssuesMessage: ({ message: inner }) => tab._tag === "Issues" ? lift(model, Issues.update(tab.model, inner, context), toIssues, wrapIssues) @@ -209,6 +233,7 @@ const BREADCRUMB_TAB = { Analytics: "analytics", PullRequests: "pull-requests", Ci: "ci", + Team: "team", Issues: "issues", Settings: "settings", } as const diff --git a/apps/web/src/page/code-review/tabs.ts b/apps/web/src/page/code-review/tabs.ts index 42917fe528..5d0309a671 100644 --- a/apps/web/src/page/code-review/tabs.ts +++ b/apps/web/src/page/code-review/tabs.ts @@ -7,6 +7,7 @@ import * as IssuesModel from "./issues/model" import { Retained } from "./kit/retained" import * as PullRequestsModel from "./pull-requests/model" import * as SettingsModel from "./settings/model" +import * as TeamModel from "./team/model" // The Code Review tabs as one page instance (switching tabs reuses what the last one read): the // active tab's Model and what it hands on. @@ -15,6 +16,7 @@ export const Tab = Schema.Union([ Schema.TaggedStruct("Analytics", { model: AnalyticsModel.Model }), Schema.TaggedStruct("PullRequests", { model: PullRequestsModel.Model }), Schema.TaggedStruct("Ci", { model: CiModel.Model }), + Schema.TaggedStruct("Team", { model: TeamModel.Model }), Schema.TaggedStruct("Issues", { model: IssuesModel.Model }), Schema.TaggedStruct("Settings", { model: SettingsModel.Model }), ]) @@ -33,6 +35,7 @@ export const Message = defineMessageUnion({ GotAnalyticsMessage: { message: AnalyticsMessage.Message }, GotPullRequestsMessage: { message: PullRequestsModel.Message }, GotCiMessage: { message: CiModel.Message }, + GotTeamMessage: { message: TeamModel.Message }, GotIssuesMessage: { message: IssuesModel.Message }, GotSettingsMessage: { message: SettingsModel.Message }, }) diff --git a/apps/web/src/page/code-review/team/derive.test.ts b/apps/web/src/page/code-review/team/derive.test.ts new file mode 100644 index 0000000000..0606f8a22b --- /dev/null +++ b/apps/web/src/page/code-review/team/derive.test.ts @@ -0,0 +1,117 @@ +import { CodeReviewTeamMember } from "@maple/domain/http" +import { describe, expect, test } from "vitest" +import { highlightsOf, metricOf, scorecardRows, teamMedian } from "./derive" + +const member = (login: string, counts: Partial = {}) => + new CodeReviewTeamMember({ + login, + pullRequests: 0, + mergedPullRequests: 0, + medianMergeSeconds: null, + findings: 0, + criticalFindings: 0, + resolvedFindings: 0, + openAtMergeFindings: 0, + shippedClean: 0, + shippedRegressed: 0, + ciRuns: 0, + firstTryRuns: 0, + firstTryGreen: 0, + reruns: 0, + cancelledRuns: 0, + runnerMs: 0, + ...counts, + }) + +const ada = member("ada", { + pullRequests: 8, + mergedPullRequests: 6, + medianMergeSeconds: 7_200, + resolvedFindings: 9, + openAtMergeFindings: 1, + firstTryRuns: 20, + firstTryGreen: 19, +}) +const lin = member("Lin", { + pullRequests: 6, + mergedPullRequests: 4, + medianMergeSeconds: 3_600, + resolvedFindings: 3, + openAtMergeFindings: 3, + firstTryRuns: 12, + firstTryGreen: 9, +}) +const sam = member("sam", { pullRequests: 2, resolvedFindings: 4 }) +const ci = member("bea", { ciRuns: 4, firstTryRuns: 3, firstTryGreen: 3 }) +const team = [ada, lin, sam, ci] + +const logins = (rows: ReturnType) => rows.map((row) => row.member.login) + +describe("metricOf", () => { + test("fix rate counts findings fixed against the ones that shipped open", () => { + expect(metricOf(ada, "fixRate")).toBe(0.9) + expect(metricOf(lin, "fixRate")).toBe(0.5) + }) + + test("a column with nothing to measure is null, not zero", () => { + expect(metricOf(ci, "fixRate")).toBeNull() + expect(metricOf(sam, "firstTryGreen")).toBeNull() + expect(metricOf(sam, "ciTime")).toBeNull() + }) +}) + +describe("teamMedian", () => { + test("is the middle person over the people the column measures", () => { + expect(teamMedian(team, "mergeTime")).toBe(5_400) + expect(teamMedian(team, "fixRate")).toBe(0.9) + }) +}) + +describe("scorecardRows", () => { + test("unranked by person: everyone, by login whatever the case", () => { + const rows = scorecardRows(team, "person", false) + expect(logins(rows)).toEqual(["ada", "bea", "Lin", "sam"]) + expect(rows.every((row) => row.rank === null)).toBe(true) + }) + + test("a column reads best first: fastest merge first, unmeasured people last", () => { + expect(logins(scorecardRows(team, "mergeTime", false))).toEqual(["Lin", "ada", "bea", "sam"]) + expect(logins(scorecardRows(team, "firstTryGreen", false))).toEqual(["bea", "ada", "Lin", "sam"]) + }) + + test("ranked leaves out people under five pull requests and numbers the rest", () => { + const rows = scorecardRows(team, "fixRate", true) + expect(rows.map((row) => [row.member.login, row.rank])).toEqual([ + ["ada", 1], + ["Lin", 2], + ]) + }) + + test("ranked never orders by a count: merged pull requests falls back to the fix rate", () => { + expect(logins(scorecardRows(team, "merged", true))).toEqual(["ada", "Lin"]) + }) + + test("equal values share a rank", () => { + const twin = member("zed", { pullRequests: 5, resolvedFindings: 9, openAtMergeFindings: 1 }) + const rows = scorecardRows([lin, ada, twin], "fixRate", true) + expect(rows.map((row) => [row.member.login, row.rank])).toEqual([ + ["ada", 1], + ["zed", 1], + ["Lin", 3], + ]) + }) +}) + +describe("highlightsOf", () => { + test("calls out who fixed most, the steadiest CI and the quickest merge, each with enough to judge", () => { + expect(highlightsOf(team).map((item) => [item.key, item.login, item.value])).toEqual([ + ["fixed", "ada", 9], + ["steady", "ada", 0.95], + ["fast", "Lin", 3_600], + ]) + }) + + test("a team of one has nothing to compare", () => { + expect(highlightsOf([ada])).toEqual([]) + }) +}) diff --git a/apps/web/src/page/code-review/team/derive.ts b/apps/web/src/page/code-review/team/derive.ts new file mode 100644 index 0000000000..cb3549db9f --- /dev/null +++ b/apps/web/src/page/code-review/team/derive.ts @@ -0,0 +1,154 @@ +import type { CodeReviewTeamMember } from "@maple/domain/http" +import type { CodeReviewTeamSort } from "../../../route/search" + +// The scorecard's numbers from the members' counts: rates, the team median of each, the order, the +// ranked view and the highlights. + +/** Ranked view: people with fewer reviewed pull requests than this are left out. */ +export const RANK_MIN_PULL_REQUESTS = 5 +/** A highlight on first-try CI needs this many judged first attempts. */ +const STEADY_CI_MIN_RUNS = 10 +/** A highlight on merge time needs this many merged pull requests. */ +const FAST_MERGE_MIN_MERGED = 3 + +export type Metric = Exclude + +const ratio = (part: number, whole: number) => (whole === 0 ? null : part / whole) + +/** One member's value of a column; null when the member has nothing it is measured over. */ +export const metricOf = (member: CodeReviewTeamMember, metric: Metric): number | null => { + switch (metric) { + case "merged": + return member.mergedPullRequests + case "mergeTime": + return member.medianMergeSeconds + case "fixRate": + return ratio(member.resolvedFindings, member.resolvedFindings + member.openAtMergeFindings) + case "criticalPerPr": + return ratio(member.criticalFindings, member.pullRequests) + case "firstTryGreen": + return ratio(member.firstTryGreen, member.firstTryRuns) + case "rerunRate": + return ratio(member.reruns, member.ciRuns) + case "shippedClean": + return ratio(member.shippedClean, member.shippedClean + member.shippedRegressed) + case "ciTime": + return member.ciRuns === 0 ? null : member.runnerMs + } +} + +/** Lower is better for these; the rest read highest first. */ +export const ASCENDING: ReadonlySet = new Set(["mergeTime", "criticalPerPr", "rerunRate"]) + +/** Rates and medians only: ranking a count would rank how much someone works, not how. */ +export const RANKABLE: ReadonlySet = new Set([ + "mergeTime", + "fixRate", + "criticalPerPr", + "firstTryGreen", + "rerunRate", + "shippedClean", +]) + +const medianOf = (values: ReadonlyArray): number | null => { + if (values.length === 0) return null + const sorted = values.toSorted((a, b) => a - b) + const mid = Math.floor(sorted.length / 2) + return sorted.length % 2 === 1 ? (sorted[mid] ?? null) : ((sorted[mid - 1] ?? 0) + (sorted[mid] ?? 0)) / 2 +} + +/** The team median of a column: the middle person, over the people it measures. */ +export const teamMedian = (members: ReadonlyArray, metric: Metric): number | null => + medianOf( + members.flatMap((member) => { + const value = metricOf(member, metric) + return value === null ? [] : [value] + }), + ) + +export type ScoredRow = Readonly<{ member: CodeReviewTeamMember; rank: number | null }> + +const byLogin = (a: CodeReviewTeamMember, b: CodeReviewTeamMember) => + a.login.localeCompare(b.login, "en", { sensitivity: "base" }) + +/** The column a ranked view orders by: a count asked for falls back to the fix rate. */ +export const rankedMetric = (sort: CodeReviewTeamSort): Metric => + sort !== "person" && RANKABLE.has(sort) ? sort : "fixRate" + +/** Ordered by the column (best first, people it does not measure last); ranked numbers them. */ +export const scorecardRows = ( + members: ReadonlyArray, + sort: CodeReviewTeamSort, + ranked: boolean, +): ReadonlyArray => { + const metric: Metric | null = ranked ? rankedMetric(sort) : sort === "person" ? null : sort + const pool = ranked ? members.filter((member) => member.pullRequests >= RANK_MIN_PULL_REQUESTS) : members + if (metric === null) return pool.toSorted(byLogin).map((member) => ({ member, rank: null })) + const direction = ASCENDING.has(metric) ? 1 : -1 + const ordered = pool.toSorted((a, b) => { + const left = metricOf(a, metric) + const right = metricOf(b, metric) + if (left === null || right === null) return left === right ? byLogin(a, b) : left === null ? 1 : -1 + return left === right ? byLogin(a, b) : (left - right) * direction + }) + if (!ranked) return ordered.map((member) => ({ member, rank: null })) + // Equal values share a rank; people the column does not measure get none. + let rank = 0 + let previous: number | null = null + return ordered.map((member, index) => { + const value = metricOf(member, metric) + if (value === null) return { member, rank: null } + if (value !== previous) rank = index + 1 + previous = value + return { member, rank } + }) +} + +export type Highlight = Readonly<{ + key: "fixed" | "steady" | "fast" + title: string + login: string + /** The number behind it, formatted by the view. */ + value: number +}> + +const best = ( + members: ReadonlyArray, + valueOf: (member: CodeReviewTeamMember) => number | null, + lowest: boolean, +): { login: string; value: number } | undefined => { + const scored = members.flatMap((member) => { + const value = valueOf(member) + return value === null ? [] : [{ login: member.login, value }] + }) + return scored + .toSorted((a, b) => + a.value === b.value ? a.login.localeCompare(b.login) : (a.value - b.value) * (lowest ? 1 : -1), + ) + .at(0) +} + +/** A few habits worth calling out, never a ranking: a team of one has nothing to compare. */ +export const highlightsOf = (members: ReadonlyArray): ReadonlyArray => { + if (members.length < 2) return [] + const fixed = best( + members, + (member) => (member.resolvedFindings > 0 ? member.resolvedFindings : null), + false, + ) + const steady = best( + members, + (member) => (member.firstTryRuns >= STEADY_CI_MIN_RUNS ? metricOf(member, "firstTryGreen") : null), + false, + ) + const fast = best( + members, + (member) => (member.mergedPullRequests >= FAST_MERGE_MIN_MERGED ? member.medianMergeSeconds : null), + true, + ) + return [ + ...(fixed === undefined ? [] : [{ key: "fixed" as const, title: "Most findings fixed", ...fixed }]), + ...(steady === undefined ? [] : [{ key: "steady" as const, title: "Steadiest CI", ...steady }]), + ...(fast === undefined ? [] : [{ key: "fast" as const, title: "Quickest to merge", ...fast }]), + ] +} diff --git a/apps/web/src/page/code-review/team/model.ts b/apps/web/src/page/code-review/team/model.ts new file mode 100644 index 0000000000..7396dff66c --- /dev/null +++ b/apps/web/src/page/code-review/team/model.ts @@ -0,0 +1,41 @@ +import { CodeReviewTeam, CodeReviewTeamQuery } from "@maple/domain/http" +import { Effect, Schema } from "effect" +import { Command } from "foldkit" +import { defineMessageUnion } from "foldkit/message" +import { KeyedSchema } from "../../../data/keyed" +import { LoadFailure } from "../../../data/load-failure" +import { CodeReviewTeamSearch, CodeReviewTeamSort } from "../../../route/search" +import { getTeam } from "../kit/api" +import { Frame, FrameMessage } from "../kit/frame" +import { ResponseCache } from "../kit/page-steps" + +// `/code-review/team`. + +export const Model = Schema.Struct({ + search: CodeReviewTeamSearch, + frame: Frame, + /** `codeReview.team` for the repository and window. */ + team: KeyedSchema(CodeReviewTeam, LoadFailure), + teamCache: ResponseCache(CodeReviewTeam), +}) +export type Model = typeof Model.Type + +export const Message = defineMessageUnion({ + GotFrameMessage: { message: FrameMessage }, + CompletedFetchTeam: { key: Schema.String, result: Schema.Result(CodeReviewTeam, LoadFailure) }, + ClickedRetry: {}, + ClickedClearFilters: {}, + ClickedSort: { sort: CodeReviewTeamSort }, + SelectedView: { view: Schema.Literals(["scorecard", "ranked"]) }, +}) +export type Message = typeof Message.Type + +export const FetchTeam = Command.define("FetchCodeReviewTeam", { + args: { key: Schema.String, query: CodeReviewTeamQuery }, + messages: [Message.CompletedFetchTeam], + execute: ({ key, query }) => + getTeam(query).pipe( + Effect.result, + Effect.map((result) => Message.CompletedFetchTeam({ key, result })), + ), +}) diff --git a/apps/web/src/page/code-review/team/team.scene.test.ts b/apps/web/src/page/code-review/team/team.scene.test.ts new file mode 100644 index 0000000000..1677cead4e --- /dev/null +++ b/apps/web/src/page/code-review/team/team.scene.test.ts @@ -0,0 +1,121 @@ +import { CodeReviewTeam } from "@maple/domain/http" +import { Result, Schema } from "effect" +import { expect as expectView, given, role, scene, text, withViewInputs } from "foldkit/scene" +import { describe, expect, test } from "vitest" +import { FrameMessage } from "../kit/frame" +import { context, names, NOW, window } from "../kit/test-fixtures" +import { Message, type Model } from "./model" +import { init, update } from "./update" +import { view as pageView } from "./view" + +type Search = Parameters[0]["search"] + +const pageUpdate = (model: Model, message: Message) => update(model, message, context) +const view = withViewInputs(pageView, { context })() +const resolved = Message.GotFrameMessage({ + message: FrameMessage.CompletedResolveWindow({ window, nowMs: NOW, mode: "Load" }), +}) +const windowed = (search: Search = {}) => pageUpdate(init({ search }, context).model, resolved) + +const member = (login: string, pullRequests: number, resolvedFindings: number) => ({ + login, + pullRequests, + mergedPullRequests: pullRequests, + medianMergeSeconds: 3_600, + findings: resolvedFindings + 1, + criticalFindings: 0, + resolvedFindings, + openAtMergeFindings: 1, + shippedClean: 1, + shippedRegressed: 0, + ciRuns: 20, + firstTryRuns: 16, + firstTryGreen: 12, + reruns: 1, + cancelledRuns: 3, + runnerMs: 3_600_000, +}) + +const teamOf = (members: ReadonlyArray>, runnerMs = 0) => + Schema.decodeUnknownSync(CodeReviewTeam)({ + startTime: NOW - 30 * 86_400_000, + endTime: NOW, + totals: { + pullRequests: 12, + mergedPullRequests: 10, + medianMergeSeconds: 7_200, + medianFirstReviewSeconds: 240, + defaultBranchRuns: 40, + defaultBranchFailed: 2, + runnerMs, + p95QueueMs: 30_000, + }, + members, + }) + +const answered = (model: Model, team: CodeReviewTeam) => + pageUpdate(model, Message.CompletedFetchTeam({ key: model.team.key ?? "", result: Result.succeed(team) })) + .model + +describe("code review team", () => { + test("entry reads the team once", () => { + expect(names(windowed().commands)).toEqual(["FetchCodeReviewTeam"]) + }) + + test("sorting by a column puts it in the URL, and a second click goes back to by person", () => { + const model = windowed().model + expect(pageUpdate(model, Message.ClickedSort({ sort: "fixRate" })).outMessage).toMatchObject({ + url: "/code-review/team?sort=fixRate", + }) + const sorted = windowed({ sort: "fixRate" }).model + expect(pageUpdate(sorted, Message.ClickedSort({ sort: "fixRate" })).outMessage).toMatchObject({ + url: "/code-review/team", + }) + }) + + test("the ranked view is a URL of its own", () => { + expect( + pageUpdate(windowed().model, Message.SelectedView({ view: "ranked" })).outMessage, + ).toMatchObject({ + url: "/code-review/team?ranked=true", + }) + }) + + test("the scorecard shows everyone next to the team median, with highlights", () => { + scene( + { update: pageUpdate, view }, + given(answered(windowed().model, teamOf([member("ada", 8, 9), member("lin", 2, 3)]))), + expectView(role("row", { name: /Team median/ })).toExist(), + expectView(role("row", { name: /ada/ })).toExist(), + expectView(role("row", { name: /lin/ })).toExist(), + expectView(text("Most findings fixed")).toExist(), + ) + }) + + test("ranked leaves out people with too few pull requests and numbers the rest", () => { + scene( + { update: pageUpdate, view }, + given( + answered( + windowed({ ranked: true }).model, + teamOf([member("ada", 8, 9), member("lin", 2, 3)]), + ), + ), + expectView(text("Ranked by fix rate")).toExist(), + expectView(role("row", { name: /ada/ })).toExist(), + expectView(role("row", { name: /lin/ })).toBeAbsent(), + // A count is never a ranking: its header does not sort. + expectView(role("button", { name: "Merged" })).toBeAbsent(), + expectView(role("button", { name: "Fixed" })).toExist(), + ) + }) + + test("an org with no reviews and no CI points at the GitHub integration", () => { + scene( + { update: pageUpdate, view }, + given(answered(windowed().model, teamOf([]))), + expectView(text("No team activity yet")).toExist(), + expectView(role("link", { name: "Open the GitHub integration" })).toExist(), + ) + }) +}) diff --git a/apps/web/src/page/code-review/team/update.ts b/apps/web/src/page/code-review/team/update.ts new file mode 100644 index 0000000000..1424ac336c --- /dev/null +++ b/apps/web/src/page/code-review/team/update.ts @@ -0,0 +1,138 @@ +import { Command } from "foldkit" +import * as Keyed from "../../../data/keyed" +import { requestKey } from "../../../data/request-key" +import type { TimeWindow } from "../../../data/time-range" +import { AppRoute, hrefOf } from "../../../route" +import type { CodeReviewTeamSearch } from "../../../route/search" +import { type PageContext, PageOutMessage, type PageReturn } from "../../contract" +import { epochWindow } from "../kit/api" +import type { FrameOut, FrameReturn } from "../kit/frame" +import { + type Carried, + frameRetained, + frameRouteChanged, + initFrame, + noCarry, + updateFrame, +} from "../kit/frame-update" +import { cachedStep, remember } from "../kit/page-steps" +import type { Retained } from "../kit/retained" +import { FetchTeam, Message, type Model } from "./model" + +export type Search = typeof CodeReviewTeamSearch.Type +type Return = PageReturn + +/** A month: enough merged pull requests per person for a median to mean something. */ +export const DEFAULT_PRESET = "30d" +const FRAME_ID = "code-review-team" + +const searchHref = (search: Search) => hrefOf(AppRoute.CodeReviewTeam({ search })) + +const navigate = (model: Model, patch: Partial): Return => ({ + model, + outMessage: PageOutMessage.RequestedNavigation({ + url: searchHref({ ...model.search, ...patch }), + isReplace: false, + }), +}) + +const request = (model: Model, context: PageContext, window: TimeWindow, mode: Keyed.RequestMode): Return => { + const query = { ...epochWindow(window), repositoryId: model.search.repo } + const step = cachedStep( + model.teamCache, + model.frame.nowMs, + model.team, + requestKey(context.orgId, "codeReview.team", { query }), + mode, + (team) => ({ team }), + (key) => FetchTeam({ key, query }), + ) + return { model: { ...model, ...step.patch }, commands: step.commands } +} + +const applyOut = (model: Model, out: FrameOut, context: PageContext): Return => { + switch (out._tag) { + case "PickedRepository": + return navigate(model, { repo: out.repo }) + // The team tab has no author or workflow filter. + case "PickedOption": + return { model } + case "PickedWindow": + return navigate(model, out.window) + case "RefusedWindow": + return { model, outMessage: out.toast } + case "ResolvedWindow": + return request(model, context, out.window, out.mode) + } +} + +const liftFrame = (model: Model, result: FrameReturn, context: PageContext): Return => { + const next: Model = { ...model, frame: result.frame } + const commands = Command.mapMessages(result.commands, (message) => Message.GotFrameMessage({ message })) + const out = result.out === null ? { model: next } : applyOut(next, result.out, context) + return { ...out, commands: [...commands, ...(out.commands ?? [])] } +} + +export const init = ( + route: { readonly search: Search }, + context: PageContext, + carried: Carried = noCarry, +): Return => { + const frame = initFrame(FRAME_ID, route.search, undefined, DEFAULT_PRESET, context, carried) + const model: Model = { + search: route.search, + frame: frame.frame, + team: Keyed.idle(), + teamCache: carried.retained.team, + } + return liftFrame(model, frame, context) +} + +export const routeChanged = ( + model: Model, + route: { readonly search: Search }, + context: PageContext, +): Return => + searchHref(model.search) === searchHref(route.search) + ? { model } + : liftFrame( + { ...model, search: route.search }, + frameRouteChanged( + model.frame, + model.search, + route.search, + undefined, + DEFAULT_PRESET, + context, + ), + context, + ) + +export const contextChanged = (model: Model, context: PageContext): Return => + model.frame.window === null ? { model } : request(model, context, model.frame.window, "Load") + +export const update = (model: Model, message: Message, context: PageContext): Return => + Message.match(message, { + GotFrameMessage: ({ message: inner }) => + liftFrame(model, updateFrame(model.frame, inner, model.search, DEFAULT_PRESET, context), context), + CompletedFetchTeam: ({ key, result }) => ({ + model: { + ...model, + team: Keyed.accept(model.team, key, result), + teamCache: remember(model.teamCache, model.frame.nowMs, key, result), + }, + }), + ClickedRetry: () => + model.frame.window === null ? { model } : request(model, context, model.frame.window, "Refresh"), + ClickedClearFilters: () => navigate(model, { repo: undefined }), + // A second click on the sorted column goes back to by person. + ClickedSort: ({ sort }) => + navigate(model, { sort: sort === "person" || sort === model.search.sort ? undefined : sort }), + SelectedView: ({ view }) => navigate(model, { ranked: view === "ranked" ? true : undefined }), + }) + +/** What this tab hands the next one. */ +export const retainedOf = (model: Model): Partial => ({ + ...frameRetained(model.frame), + team: model.teamCache, +}) diff --git a/apps/web/src/page/code-review/team/view.ts b/apps/web/src/page/code-review/team/view.ts new file mode 100644 index 0000000000..42aac6827d --- /dev/null +++ b/apps/web/src/page/code-review/team/view.ts @@ -0,0 +1,485 @@ +import type { CodeReviewTeam, CodeReviewTeamMember, CodeReviewTeamTotals } from "@maple/domain/http" +import { formatCount, formatSpan } from "../../../components/code-review/code-review-format" +import { EMPTY_VALUE, formatPercent } from "@maple/ui/lib/format" +import { cn } from "@maple/ui/lib/utils" +import { AsyncData, Submodel } from "foldkit" +import type { Html, HtmlBuilder } from "foldkit/html" +import { AppRoute, hrefOf } from "../../../route" +import { linkButton } from "../../../ui/button" +import { empty, emptyDescription, emptyHeader, emptyMessage, emptyTitle } from "../../../ui/empty" +import { errorState } from "../../../ui/error-state" +import { icon } from "../../../ui/icon" +import { panel, panelHeader } from "../../../ui/panel" +import { segmentedSelect } from "../../../ui/segmented-select" +import { skeleton } from "../../../ui/skeleton" +import { table, tableBody, tableCell, tableHead, tableHeader, tableRow } from "../../../ui/table" +import type { PageViewInputs } from "../../contract" +import { nothingInWindow } from "../../kit/nothing-in-window" +import { statRail, statRailItem } from "../../kit/stat-rail" +import * as TimeRangePicker from "../../kit/time-range-picker" +import { authorLabel } from "../kit/author" +import { filtersView } from "../kit/filters-view" +import { codeReviewLayout } from "../kit/layout" +import { + ASCENDING, + highlightsOf, + type Metric, + metricOf, + RANK_MIN_PULL_REQUESTS, + RANKABLE, + rankedMetric, + scorecardRows, + teamMedian, +} from "./derive" +import { Message, type Model } from "./model" +import { DEFAULT_PRESET } from "./update" + +// `CodeReviewTeamPage`: team KPIs, a few highlights, and the per-person scorecard against the team +// median, ranked only on request. + +type H = HtmlBuilder + +const rateOf = (rate: number | null) => (rate === null ? EMPTY_VALUE : formatPercent(rate)) +const spanOf = (seconds: number | null) => (seconds === null ? EMPTY_VALUE : formatSpan(seconds)) +/** Runner time in hours past the first: days of CI read as a calendar span, not as work. */ +const runnerTimeOf = (ms: number) => + ms < 3_600_000 ? formatSpan(ms / 1000) : `${formatCount(Math.round(ms / 3_600_000))}h` + +type Column = Readonly<{ + metric: Metric + label: string + hint: string + /** Width and the breakpoint it shows from. */ + className: string + format: (value: number | null) => string + /** The counts behind a member's value. */ + detail: (member: CodeReviewTeamMember) => string +}> + +const COLUMNS: ReadonlyArray = [ + { + metric: "merged", + label: "Merged", + hint: "Pull requests merged, of the ones first reviewed in this window", + className: "hidden w-24 sm:table-cell", + format: (value) => (value === null ? EMPTY_VALUE : formatCount(value)), + detail: (member) => + `${member.mergedPullRequests} of ${member.pullRequests} reviewed pull requests merged`, + }, + { + metric: "mergeTime", + label: "To merge", + hint: "Median time from a pull request's first review to its merge", + className: "hidden w-24 md:table-cell", + format: spanOf, + detail: (member) => `Median over ${member.mergedPullRequests} merged pull requests`, + }, + { + metric: "fixRate", + label: "Fixed", + hint: "Findings fixed in a later push, over those fixed plus those still open when the pull request merged", + className: "w-24", + format: rateOf, + detail: (member) => + `${member.resolvedFindings} fixed, ${member.openAtMergeFindings} still open at merge`, + }, + { + metric: "criticalPerPr", + label: "Critical / PR", + hint: "Critical findings per reviewed pull request", + className: "hidden w-28 lg:table-cell", + format: (value) => (value === null ? EMPTY_VALUE : value.toFixed(2)), + detail: (member) => `${member.criticalFindings} critical over ${member.pullRequests} pull requests`, + }, + { + metric: "firstTryGreen", + label: "First-try green", + hint: "CI runs that passed on their first attempt, over first attempts that passed or failed", + className: "w-32", + format: rateOf, + detail: (member) => `${member.firstTryGreen} of ${member.firstTryRuns} first attempts passed`, + }, + { + metric: "rerunRate", + label: "Reruns", + hint: "Runs that were a rerun of a workflow, over every run the person started", + className: "hidden w-24 xl:table-cell", + format: rateOf, + detail: (member) => `${member.reruns} reruns over ${member.ciRuns} runs`, + }, + { + metric: "shippedClean", + label: "Shipped clean", + hint: "Merged pull requests the post-merge look found clean, over those it judged", + className: "hidden w-28 lg:table-cell", + format: rateOf, + detail: (member) => + `${member.shippedClean} clean, ${member.shippedRegressed} regressed after the deploy`, + }, + { + metric: "ciTime", + label: "CI time", + hint: "Time the person's jobs spent on a runner, queue left out", + className: "hidden w-24 md:table-cell", + format: (value) => (value === null ? EMPTY_VALUE : runnerTimeOf(value)), + detail: (member) => `${member.ciRuns} runs, ${member.cancelledRuns} cancelled`, + }, +] + +/** How the panel title names the column a ranked view orders by. */ +const RANKED_BY: Readonly> = { + merged: "pull requests merged", + mergeTime: "time to merge", + fixRate: "fix rate", + criticalPerPr: "critical findings per pull request", + firstTryGreen: "first-try green", + rerunRate: "rerun rate", + shippedClean: "shipped clean", + ciTime: "CI time", +} + +const headlineStrip = (h: H, totals: CodeReviewTeamTotals): Html => { + const failRate = + totals.defaultBranchRuns === 0 ? null : totals.defaultBranchFailed / totals.defaultBranchRuns + const stats: ReadonlyArray<{ label: string; value: string; subline?: string; hint: string }> = [ + { + label: "Merged", + value: formatCount(totals.mergedPullRequests), + subline: `of ${formatCount(totals.pullRequests)} reviewed`, + hint: "Pull requests by people, first reviewed in this window, that merged", + }, + { + label: "Review to merge", + value: spanOf(totals.medianMergeSeconds), + subline: "median", + hint: "Median time from a pull request's first review to its merge", + }, + { + label: "First review", + value: spanOf(totals.medianFirstReviewSeconds), + subline: "median", + hint: "Median time from a pull request's first review being queued to it being posted", + }, + { + label: "Default branch red", + value: rateOf(failRate), + subline: `of ${formatCount(totals.defaultBranchRuns)} pushes`, + hint: "CI runs on pushes to a repository's default branch that failed", + }, + { + label: "CI time per merge", + value: + totals.mergedPullRequests === 0 + ? EMPTY_VALUE + : formatSpan(totals.runnerMs / 1000 / totals.mergedPullRequests), + subline: `${runnerTimeOf(totals.runnerMs)} in total`, + hint: "Runner time of every CI run, bots included, per merged pull request", + }, + { + label: "Runner wait", + value: totals.p95QueueMs === null ? EMPTY_VALUE : formatSpan(totals.p95QueueMs / 1000), + subline: "p95", + hint: "How long the slowest 5% of jobs waited for a runner", + }, + ] + return statRail( + h, + { columns: 3, className: "xl:grid-cols-6" }, + stats.map((stat) => + h.div( + [h.Key(stat.label), h.Title(stat.hint)], + [ + statRailItem(h, { + compact: true, + eyebrow: stat.label, + value: stat.value, + subline: stat.subline, + }), + ], + ), + ), + ) +} + +const highlightLine = (key: "fixed" | "steady" | "fast", value: number) => { + switch (key) { + case "fixed": + return `${formatCount(value)} findings fixed before merge` + case "steady": + return `${formatPercent(value)} green on the first try` + case "fast": + return `${formatSpan(value)} median to merge` + } +} + +const highlights = (h: H, members: ReadonlyArray): Html | null => { + const items = highlightsOf(members) + if (items.length === 0) return null + return h.section( + [h.AriaLabel("Highlights")], + [ + statRail( + h, + // One per row on a phone: a login and its line need the width. + { columns: 3, className: "grid-cols-1" }, + items.map((item) => + h.div( + [h.Key(item.key)], + [ + statRailItem(h, { + compact: true, + eyebrow: item.title, + value: authorLabel(h, item.login, "max-w-full text-base"), + subline: highlightLine(item.key, item.value), + }), + ], + ), + ), + ), + ], + ) +} + +const sortHead = (h: H, column: Column, model: Model, ranked: boolean): Html => { + const sort = model.search.sort ?? "person" + const isActive = ranked ? rankedMetric(sort) === column.metric : sort === column.metric + const isAscending = ASCENDING.has(column.metric) + const className = cn("px-3 text-right font-normal", column.className) + // Ranked orders by rates alone: a count column is a plain header there. + if (ranked && !RANKABLE.has(column.metric)) + return tableHead(h, [h.span([h.Title(column.hint)], [column.label])], className) + return tableHead( + h, + [ + h.button( + [ + h.Type("button"), + h.Title(column.hint), + h.OnClick(Message.ClickedSort({ sort: column.metric })), + h.Class( + cn( + "inline-flex items-center gap-1 hover:text-foreground active:text-foreground focus-visible:underline focus-visible:outline-none", + isActive && "text-foreground", + ), + ), + ], + [ + column.label, + ...(isActive + ? [icon(h, isAscending ? "ArrowUpIcon" : "ArrowDownIcon", { className: "size-3" })] + : []), + ], + ), + ], + className, + [h.Attribute("aria-sort", isActive ? (isAscending ? "ascending" : "descending") : "none")], + ) +} + +const personHead = (h: H, model: Model): Html => { + const isActive = (model.search.sort ?? "person") === "person" && model.search.ranked !== true + return tableHead( + h, + [ + h.button( + [ + h.Type("button"), + h.OnClick(Message.ClickedSort({ sort: "person" })), + h.Class( + cn( + "inline-flex items-center gap-1 hover:text-foreground active:text-foreground focus-visible:underline focus-visible:outline-none", + isActive && "text-foreground", + ), + ), + ], + ["Person"], + ), + ], + "px-4 font-normal", + [h.Attribute("aria-sort", isActive ? "ascending" : "none")], + ) +} + +const medianRow = (h: H, members: ReadonlyArray, ranked: boolean): Html => + tableRow( + h, + [ + ...(ranked ? [tableCell(h, [], "w-10 px-3")] : []), + tableCell(h, ["Team median"], "px-4 py-2 text-xs font-medium text-muted-foreground"), + ...COLUMNS.map((column) => + tableCell( + h, + [column.format(teamMedian(members, column.metric))], + cn("px-3 text-right text-xs tabular-nums text-muted-foreground", column.className), + ), + ), + ], + "bg-surface-sunken", + [h.Key("median")], + ) + +const memberRow = (h: H, member: CodeReviewTeamMember, rank: number | null, ranked: boolean): Html => + tableRow( + h, + [ + ...(ranked + ? [ + tableCell( + h, + [rank === null ? EMPTY_VALUE : String(rank)], + "w-10 px-3 tabular-nums text-muted-foreground", + ), + ] + : []), + tableCell(h, [authorLabel(h, member.login, "max-w-full")], "px-4 py-2.5"), + ...COLUMNS.map((column) => + tableCell( + h, + [column.format(metricOf(member, column.metric))], + cn("px-3 text-right tabular-nums", column.className), + [h.Title(column.detail(member))], + ), + ), + ], + undefined, + [h.Key(member.login)], + ) + +const scorecard = (h: H, model: Model, members: ReadonlyArray): Html => { + const ranked = model.search.ranked === true + const rows = scorecardRows(members, model.search.sort ?? "person", ranked) + const rankedBy = RANKED_BY[rankedMetric(model.search.sort ?? "person")] + return panel(h, {}, [ + panelHeader(h, { + title: ranked ? `Ranked by ${rankedBy}` : "Scorecard", + action: segmentedSelect(h, { + size: "sm", + ariaLabel: "Scorecard view", + value: ranked ? "ranked" : "scorecard", + onChange: (view) => Message.SelectedView({ view }), + options: [ + { value: "scorecard", label: "Everyone" }, + { value: "ranked", label: "Ranked" }, + ], + }), + }), + rows.length === 0 + ? emptyMessage(h, { className: "px-4 py-6" }, [ + ranked + ? `Nobody has ${RANK_MIN_PULL_REQUESTS} reviewed pull requests in this window yet` + : "Nobody in this window", + ]) + : table(h, { size: "sm", className: "table-fixed" }, [ + tableHeader( + h, + [ + tableRow(h, [ + ...(ranked ? [tableHead(h, ["#"], "w-10 px-3 font-normal")] : []), + personHead(h, model), + ...COLUMNS.map((column) => sortHead(h, column, model, ranked)), + ]), + ], + { className: "bg-surface-sunken" }, + ), + tableBody(h, [ + medianRow(h, members, ranked), + ...rows.map((row) => memberRow(h, row.member, row.rank, ranked)), + ]), + ]), + h.footer( + [h.Class("border-t px-4 py-3 text-xs text-muted-foreground")], + [ + ranked + ? `Ranked on rates and medians only, people with at least ${RANK_MIN_PULL_REQUESTS} reviewed pull requests. Bots and CI runs that only waited for approval are left out.` + : "Each person's numbers next to the team median. Bots, and CI runs that only waited for approval, are left out. Hover a value for the counts behind it.", + ], + ), + ]) +} + +const teamSkeleton = (h: H): Html => + h.div( + [h.Class("flex flex-col gap-6")], + [skeleton(h, "h-[118px] w-full rounded-md"), skeleton(h, "h-80 w-full rounded-md")], + ) + +/** The on-ramp for an org with neither reviews nor CI runs. */ +const noTeamData = (h: H): Html => + empty( + h, + [ + emptyHeader(h, [ + emptyTitle(h, ["No team activity yet"]), + emptyDescription(h, [ + "The team view reads pull request reviews and CI runs from the GitHub integration: turn on reviews for a repository, or CI traces, and everyone who opens pull requests or pushes shows up here.", + ]), + ]), + linkButton( + h, + { + variant: "outline", + size: "sm", + href: hrefOf(AppRoute.SourcesNew({ search: { integration: "github" } })), + }, + ["Open the GitHub integration"], + ), + ], + "rounded-md border border-dashed md:py-14", + ) + +const teamView = (h: H, model: Model, team: CodeReviewTeam): Html => { + const highlighted = highlights(h, team.members) + return h.div( + [h.Class("flex flex-col gap-6")], + [ + headlineStrip(h, team.totals), + ...(highlighted === null ? [] : [highlighted]), + scorecard(h, model, team.members), + ], + ) +} + +const content = (h: H, model: Model): Html => + AsyncData.matchData(model.team.data, { + onEmpty: () => teamSkeleton(h), + onFailure: (failure) => + errorState(h, { failure, title: "Couldn't load the team", onRetry: Message.ClickedRetry() }), + onData: (team) => { + if (team.members.length > 0) return teamView(h, model, team) + if (model.search.repo === undefined && team.totals.runnerMs === 0) return noTeamData(h) + return nothingInWindow(h, { + title: "Nobody in this window", + description: + model.search.repo === undefined + ? "No pull request was reviewed and no CI run finished in this window. Widen the window." + : "Nothing matches this repository. Widen the window or clear the filter.", + onClear: model.search.repo === undefined ? undefined : Message.ClickedClearFilters(), + }) + }, + }) + +export const view = Submodel.defineView((model, _inputs, h) => { + const filters = filtersView(h, { + frame: model.frame, + search: model.search, + reviewedOnly: false, + kind: "author", + options: [], + selected: undefined, + defaultPreset: DEFAULT_PRESET, + presets: TimeRangePicker.LONG_RANGE_PRESET_VALUES, + toMessage: (message) => Message.GotFrameMessage({ message }), + isFailed: AsyncData.isFailure(model.team.data), + }) + return codeReviewLayout( + { + h, + active: "team", + search: model.search, + toolbar: filters.actions, + // The repository alone: the team is everyone, never one author. + filters: filters.filters.slice(0, 1), + }, + [content(h, model)], + ) +}) diff --git a/apps/web/src/route/app-route.ts b/apps/web/src/route/app-route.ts index d318d4887b..dfe74808ca 100644 --- a/apps/web/src/route/app-route.ts +++ b/apps/web/src/route/app-route.ts @@ -40,6 +40,7 @@ export const AppRoute = defineRouteUnion({ StatusPage: { pageId: Schema.String, search: Search.StatusPageSearch }, Anomaly: { incidentId: AnomalyIncidentId }, CodeReviewCi: { search: Search.CodeReviewCiSearch }, + CodeReviewTeam: { search: Search.CodeReviewTeamSearch }, CodeReviewIssues: { search: Search.CodeReviewIssuesSearch }, CodeReviewPullRequests: { search: Search.CodeReviewPullRequestsSearch }, CodeReviewSettings: {}, diff --git a/apps/web/src/route/routers.ts b/apps/web/src/route/routers.ts index 74407ecf2a..4a1336dc21 100644 --- a/apps/web/src/route/routers.ts +++ b/apps/web/src/route/routers.ts @@ -121,6 +121,11 @@ const routers = { search(Search.CodeReviewCiSearch), Route.mapTo(AppRoute.CodeReviewCi), ), + CodeReviewTeam: pipe( + path("code-review", "team"), + search(Search.CodeReviewTeamSearch), + Route.mapTo(AppRoute.CodeReviewTeam), + ), CodeReviewIssues: pipe( path("code-review", "issues"), search(Search.CodeReviewIssuesSearch), diff --git a/apps/web/src/route/search.ts b/apps/web/src/route/search.ts index 7449980fa9..b3bb0e80fd 100644 --- a/apps/web/src/route/search.ts +++ b/apps/web/src/route/search.ts @@ -122,6 +122,28 @@ const CodeReviewListSearchFields = { review: Schema.optional(PrReviewId), } +/** The Team scorecard's sort column; `person` is by login. */ +export const CodeReviewTeamSort = Schema.Literals([ + "person", + "merged", + "mergeTime", + "fixRate", + "criticalPerPr", + "firstTryGreen", + "rerunRate", + "shippedClean", + "ciTime", +]) +export type CodeReviewTeamSort = typeof CodeReviewTeamSort.Type + +const CodeReviewTeamSearchFields = { + repo: Schema.optional(VcsRepositoryId), + ...TimeRangeSearchFields, + sort: Schema.optional(CodeReviewTeamSort), + /** Ranked: people with enough pull requests, numbered by the sort column. */ + ranked: BooleanParam, +} + const CodeReviewCiSearchFields = { ...CodeReviewSearchFields, pipeline: Schema.optional(CiPipelineName), @@ -349,6 +371,8 @@ export const StatusPageSearch = Schema.Struct({ export const CodeReviewCiSearch = Schema.Struct(CodeReviewCiSearchFields) +export const CodeReviewTeamSearch = Schema.Struct(CodeReviewTeamSearchFields) + export const CodeReviewIssuesSearch = Schema.Struct(CodeReviewIssuesSearchFields) export const CodeReviewPullRequestsSearch = Schema.Struct(CodeReviewListSearchFields) diff --git a/apps/web/src/ui/icons.generated.ts b/apps/web/src/ui/icons.generated.ts index 29a60b3303..23e6d5f215 100644 --- a/apps/web/src/ui/icons.generated.ts +++ b/apps/web/src/ui/icons.generated.ts @@ -18281,6 +18281,160 @@ export const ICON_TREES = { ], ], ], + UsersIcon: [ + "svg", + [ + ["xmlns", "http://www.w3.org/2000/svg"], + ["viewBox", "0 0 24 24"], + ["width", "7777"], + ["height", "7777"], + ["class", "__icon_class__"], + ["fill", "none"], + ["aria-hidden", "true"], + ], + [ + [ + "path", + [ + ["d", "M7 4H9"], + ["stroke", "currentColor"], + ["stroke-width", "2"], + ["stroke-linecap", "square"], + ], + [], + ], + [ + "path", + [ + ["d", "M7 10H9"], + ["stroke", "currentColor"], + ["stroke-width", "2"], + ["stroke-linecap", "square"], + ], + [], + ], + [ + "path", + [ + ["d", "M11 6L11 8"], + ["stroke", "currentColor"], + ["stroke-width", "2"], + ["stroke-linecap", "square"], + ], + [], + ], + [ + "path", + [ + ["d", "M5 6L5 8"], + ["stroke", "currentColor"], + ["stroke-width", "2"], + ["stroke-linecap", "square"], + ], + [], + ], + [ + "path", + [ + ["d", "M17.5 4H18.5"], + ["stroke", "currentColor"], + ["stroke-width", "2"], + ["stroke-linecap", "square"], + ], + [], + ], + [ + "path", + [ + ["d", "M17.5 9H18.5"], + ["stroke", "currentColor"], + ["stroke-width", "2"], + ["stroke-linecap", "square"], + ], + [], + ], + [ + "path", + [ + ["d", "M20.5 6L20.5 7"], + ["stroke", "currentColor"], + ["stroke-width", "2"], + ["stroke-linecap", "square"], + ], + [], + ], + [ + "path", + [ + ["d", "M15.5 6L15.5 7"], + ["stroke", "currentColor"], + ["stroke-width", "2"], + ["stroke-linecap", "square"], + ], + [], + ], + [ + "path", + [ + ["d", "M5 14H11"], + ["stroke", "currentColor"], + ["stroke-width", "2"], + ["stroke-linecap", "square"], + ], + [], + ], + [ + "path", + [ + ["d", "M16 13H20"], + ["stroke", "currentColor"], + ["stroke-width", "2"], + ["stroke-linecap", "square"], + ], + [], + ], + [ + "path", + [ + ["d", "M1 18V20H15V18"], + ["stroke", "currentColor"], + ["stroke-width", "2"], + ["stroke-linecap", "square"], + ], + [], + ], + [ + "path", + [ + ["d", "M19 18H22V15"], + ["stroke", "currentColor"], + ["stroke-width", "2"], + ["stroke-linecap", "square"], + ], + [], + ], + [ + "path", + [ + ["d", "M3 16H3.01"], + ["stroke", "currentColor"], + ["stroke-width", "2"], + ["stroke-linecap", "square"], + ], + [], + ], + [ + "path", + [ + ["d", "M13 16H13.01"], + ["stroke", "currentColor"], + ["stroke-width", "2"], + ["stroke-linecap", "square"], + ], + [], + ], + ], + ], XmarkIcon: [ "svg", [ diff --git a/packages/backend/src/services/pr-review/CodeReviewTeamService.test.ts b/packages/backend/src/services/pr-review/CodeReviewTeamService.test.ts new file mode 100644 index 0000000000..1ab2d4d643 --- /dev/null +++ b/packages/backend/src/services/pr-review/CodeReviewTeamService.test.ts @@ -0,0 +1,309 @@ +import { afterEach, assert, describe, it } from "@effect/vitest" +import { Effect, Layer, Schema } from "effect" +import { + GitCommitSha, + OrgId, + PrReviewId, + RoleName, + UserId, + VcsInstallationId, + VcsRepositoryId, +} from "@maple/domain/http" +import * as PG from "@maple-dev/effect-orm/postgres" +import { PrReviewFindings, PrReviews, VcsRepositories } from "@maple/db/tables" +import { compiledQueryOf } from "@maple/query-engine/execution" +import { Database } from "@maple/backend/platform/DatabaseLive" +import { cleanupTestDbs, createTestDb, type TestDb } from "@maple/backend/platform/test-pglite" +import type { TenantContext } from "@maple/backend/services/auth/AuthService" +import { WarehouseQueryService } from "@maple/backend/services/warehouse/WarehouseQueryService" +import { makeWarehouseServiceStub } from "@maple/backend/testing/warehouse-test-support" +import { CodeReviewTeamService, median } from "./CodeReviewTeamService" + +const trackedDbs: TestDb[] = [] +afterEach(() => cleanupTestDbs(trackedDbs)) + +const ORG = Schema.decodeUnknownSync(OrgId)("org_team") +const OTHER_ORG = Schema.decodeUnknownSync(OrgId)("org_other") +const REPO = Schema.decodeUnknownSync(VcsRepositoryId)("00000000-0000-4000-8000-000000000001") +const OTHER_REPO = Schema.decodeUnknownSync(VcsRepositoryId)("00000000-0000-4000-8000-000000000002") +const INSTALLATION = Schema.decodeUnknownSync(VcsInstallationId)("00000000-0000-4000-8000-0000000000aa") +const reviewId = (n: number) => + Schema.decodeUnknownSync(PrReviewId)(`00000000-0000-4000-9000-${String(n).padStart(12, "0")}`) +const tenant: TenantContext = { + orgId: ORG, + userId: Schema.decodeUnknownSync(UserId)("user_team"), + roles: [Schema.decodeUnknownSync(RoleName)("org:admin")], + authMode: "self_hosted", +} + +const HOUR = 3_600_000 +const START = Date.UTC(2026, 9, 1) +const at = (hours: number) => START + hours * HOUR +const window = { startTime: START, endTime: at(240) } + +const postMerge = (verdict: "clean" | "regressed") => ({ + deploy: { + service: "api", + environment: "production", + commitSha: "b".repeat(40), + firstSeen: at(30), + exact: true, + }, + windowMinutes: 60, + operations: [], + newIssues: [], + linkedIssues: [], + missing: [], + verdict, +}) + +const review = (input: { + n: number + number: number + author: string + created: number + orgId?: OrgId + repositoryId?: VcsRepositoryId + mergedAt?: number + verdict?: "clean" | "regressed" +}) => ({ + id: reviewId(input.n), + orgId: input.orgId ?? ORG, + repositoryId: input.repositoryId ?? REPO, + number: input.number, + headSha: Schema.decodeUnknownSync(GitCommitSha)("a".repeat(39) + String(input.n % 10)), + url: `https://github.com/acme/web/pull/${input.number}`, + authorLogin: input.author, + status: "completed" as const, + finishedAt: input.created + 120_000, + mergedAt: input.mergedAt ?? null, + postMergeStatus: input.verdict === undefined ? null : ("reported" as const), + postMergeJson: input.verdict === undefined ? null : postMerge(input.verdict), + createdAt: input.created, + updatedAt: input.created, +}) + +const finding = (n: number, number: number, severity: "critical" | "warn", status: "open" | "resolved") => ({ + id: `finding-${n}`, + orgId: ORG, + repositoryId: REPO, + number, + reviewId: reviewId(number === 10 ? 1 : 3), + handle: `F${n}`, + path: "src/index.ts", + line: n, + category: "correctness" as const, + severity, + title: `Finding ${n}`, + status, + createdAt: at(1), + updatedAt: at(1), +}) + +const seed = Effect.gen(function* () { + const database = yield* Database + yield* database.execute((db) => + db.run( + PG.insertInto(VcsRepositories).values( + [ + { id: REPO, orgId: ORG, fullName: "acme/web", defaultBranch: "trunk" }, + { id: OTHER_REPO, orgId: OTHER_ORG, fullName: "other/api", defaultBranch: "main" }, + ].map((repo) => ({ + ...repo, + provider: "github" as const, + installationId: INSTALLATION, + externalRepoId: repo.id, + owner: repo.fullName.split("/")[0] ?? "", + name: repo.fullName.split("/")[1] ?? "", + htmlUrl: `https://github.com/${repo.fullName}`, + createdAt: START, + updatedAt: START, + })), + ), + ), + ) + yield* database.execute((db) => + db.run( + PG.insertInto(PrReviews).values([ + // ada: #10 reviewed twice, merged 10h after its first review, shipped clean. + review({ + n: 1, + number: 10, + author: "ada", + created: at(0), + mergedAt: at(10), + verdict: "clean", + }), + review({ n: 2, number: 10, author: "ada", created: at(4), mergedAt: at(10) }), + // ada: #11 still open, with an open finding that did not ship. + review({ n: 3, number: 11, author: "ada", created: at(20) }), + // lin: #12 merged after 2h, regressed. + review({ + n: 4, + number: 12, + author: "lin", + created: at(30), + mergedAt: at(32), + verdict: "regressed", + }), + review({ n: 5, number: 13, author: "dependabot[bot]", created: at(40), mergedAt: at(41) }), + // Before the window, and another org: neither may count. + review({ n: 6, number: 9, author: "ada", created: at(-5) }), + // #9 re-reviewed in the window: still not first reviewed in it. + review({ n: 8, number: 9, author: "ada", created: at(6), mergedAt: at(7) }), + review({ + n: 7, + number: 1, + author: "eve", + created: at(5), + orgId: OTHER_ORG, + repositoryId: OTHER_REPO, + }), + ]), + ), + ) + yield* database.execute((db) => + db.run( + PG.insertInto(PrReviewFindings).values([ + finding(1, 10, "critical", "resolved"), + finding(2, 10, "warn", "open"), + finding(3, 11, "warn", "open"), + // Fixed on #11, which has not merged: not judged yet. + finding(4, 11, "warn", "resolved"), + ]), + ), + ) +}) + +const ciRow = (actor: string, extra: Record = {}) => ({ + actor, + runs: 10, + firstTryRuns: 8, + firstTryGreen: 6, + reruns: 1, + cancelledRuns: 2, + runnerMs: 600_000, + defaultBranchRuns: 4, + defaultBranchFailed: 1, + ...extra, +}) + +const withService = ( + rows: Record>>, + body: ( + service: CodeReviewTeamService["Service"], + reads: Array<{ context: string; sql: string }>, + ) => Effect.Effect, +) => + Effect.gen(function* () { + const testDb = createTestDb(trackedDbs) + const reads: Array<{ context: string; sql: string }> = [] + const warehouse = makeWarehouseServiceStub({ + compiledQuery: (_tenant, compiled, options) => { + const query = compiledQueryOf(compiled) + const context = options?.context ?? "" + reads.push({ context, sql: query.sql }) + return query.decodeRows(rows[context] ?? []).pipe(Effect.orDie) + }, + }) + return yield* Effect.gen(function* () { + yield* seed + const service = yield* CodeReviewTeamService + return yield* body(service, reads) + }).pipe( + Effect.provide( + Layer.effect(CodeReviewTeamService, CodeReviewTeamService.make).pipe( + Layer.provideMerge( + Layer.mergeAll(testDb.layer, Layer.succeed(WarehouseQueryService, warehouse)), + ), + ), + ), + ) + }) + +describe("CodeReviewTeamService.team", () => { + it.effect("folds reviews, findings and CI runs into one row per person, bots left out", () => + withService( + { + ciActorStats: [ + ciRow("ada"), + ciRow("JeremyFunk", { runs: 3, runnerMs: 60_000 }), + ciRow("dependabot[bot]", { + runnerMs: 30_000, + defaultBranchRuns: 0, + defaultBranchFailed: 0, + }), + ], + ciQueueTotals: [{ jobs: 40, p50QueueMs: 2_000, p95QueueMs: 9_000 }], + }, + (service) => + Effect.gen(function* () { + const team = yield* service.team(tenant, window) + assert.deepStrictEqual( + team.members.map((member) => member.login), + ["ada", "JeremyFunk", "lin"], + ) + const [ada, jeremy, lin] = team.members + assert.strictEqual(ada?.pullRequests, 2) + assert.strictEqual(ada?.mergedPullRequests, 1) + assert.strictEqual(ada?.medianMergeSeconds, 10 * 3_600) + assert.strictEqual(ada?.findings, 4) + assert.strictEqual(ada?.criticalFindings, 1) + // Fix rate is judged at the merge: #10 alone, one fixed and one shipped open. + assert.strictEqual(ada?.resolvedFindings, 1) + assert.strictEqual(ada?.openAtMergeFindings, 1) + assert.strictEqual(ada?.shippedClean, 1) + assert.strictEqual(ada?.ciRuns, 10) + assert.strictEqual(ada?.firstTryGreen, 6) + // A CI actor without a reviewed pull request still has a row. + assert.strictEqual(jeremy?.pullRequests, 0) + assert.strictEqual(jeremy?.ciRuns, 3) + assert.strictEqual(lin?.shippedRegressed, 1) + assert.strictEqual(lin?.shippedClean, 0) + assert.strictEqual(lin?.ciRuns, 0) + + assert.strictEqual(team.totals.pullRequests, 3) + assert.strictEqual(team.totals.mergedPullRequests, 2) + assert.strictEqual(team.totals.medianMergeSeconds, 6 * 3_600) + assert.strictEqual(team.totals.medianFirstReviewSeconds, 120) + // Runner time and the default branch count bots too: they share the runners. + assert.strictEqual(team.totals.runnerMs, 690_000) + assert.strictEqual(team.totals.defaultBranchRuns, 8) + assert.strictEqual(team.totals.defaultBranchFailed, 2) + assert.strictEqual(team.totals.p95QueueMs, 9_000) + }), + ), + ) + + it.effect("judges default-branch runs by each repository's own default branch", () => + withService({}, (_service, reads) => + Effect.gen(function* () { + const service = yield* CodeReviewTeamService + yield* service.team(tenant, { ...window, repositoryId: REPO }) + const sql = reads.find((read) => read.context === "ciActorStats")?.sql ?? "" + assert.include(sql, "ci_runs.Owner = 'acme' AND ci_runs.Repository = 'web'") + assert.include(sql, "ci_runs.Branch = 'trunk'") + assert.notInclude(sql, "'main'") + }), + ), + ) + + it.effect("reads nothing for a repository id that is not the org's", () => + withService({ ciActorStats: [ciRow("eve")] }, (service, reads) => + Effect.gen(function* () { + const team = yield* service.team(tenant, { ...window, repositoryId: OTHER_REPO }) + assert.deepStrictEqual(team.members, []) + assert.strictEqual(team.totals.pullRequests, 0) + assert.deepStrictEqual(reads, []) + }), + ), + ) +}) + +describe("median", () => { + it("is the middle value, or the mean of the two middle ones", () => { + assert.strictEqual(median([5, 1, 3]), 3) + assert.strictEqual(median([4, 1, 3, 2]), 2.5) + assert.strictEqual(median([]), null) + }) +}) diff --git a/packages/backend/src/services/pr-review/CodeReviewTeamService.ts b/packages/backend/src/services/pr-review/CodeReviewTeamService.ts new file mode 100644 index 0000000000..01b370ef4e --- /dev/null +++ b/packages/backend/src/services/pr-review/CodeReviewTeamService.ts @@ -0,0 +1,362 @@ +import { + CodeReviewTeam, + CodeReviewTeamMember, + type CodeReviewTeamQuery, + CodeReviewTeamTotals, + PrReviewPersistenceError, + type VcsRepositoryId, + type WarehouseReadError, +} from "@maple/domain/http" +import type { OrgId } from "@maple/domain/primitives" +import * as PG from "@maple-dev/effect-orm/postgres" +import { PrReviewFindings, PrReviews, VcsRepositories } from "@maple/db/tables" +import { CH, formatWarehouseDateTime } from "@maple/query-engine" +import { Context, Effect, Layer } from "effect" +import { Database } from "@maple/backend/platform/DatabaseLive" +import { makeDbExecute } from "@maple/backend/platform/db-execute" +import type { TenantContext } from "@maple/backend/services/auth/AuthService" +import { WarehouseQueryService } from "@maple/backend/services/warehouse/WarehouseQueryService" +import { ciWindow } from "./CiAnalyticsService" + +type TeamError = PrReviewPersistenceError | WarehouseReadError + +export interface CodeReviewTeamServiceApi { + readonly team: ( + tenant: TenantContext, + query: CodeReviewTeamQuery, + ) => Effect.Effect +} + +/** Pull requests read per window: past this the oldest are left out, which a span annotation shows. */ +const MAX_PULL_REQUESTS = 20_000 +/** How far back a pull request's earlier reviews are looked for, to tell a first review from a re-review. */ +const FIRST_REVIEW_LOOKBACK_MS = 180 * 86_400_000 + +/** GitHub Apps act as `[bot]`; a team page is about people. */ +export const isBotLogin = (login: string) => login.endsWith("[bot]") + +export const median = (values: ReadonlyArray): number | null => { + if (values.length === 0) return null + const sorted = values.toSorted((a, b) => a - b) + const mid = Math.floor(sorted.length / 2) + return sorted.length % 2 === 1 ? (sorted[mid] ?? null) : ((sorted[mid - 1] ?? 0) + (sorted[mid] ?? 0)) / 2 +} + +/** One pull request's reviews, folded. */ +export interface TeamPullRequest { + readonly repositoryId: string + readonly number: number + readonly author: string | null + readonly mergeSeconds: number | null + readonly firstReviewSeconds: number | null + readonly shippedClean: boolean + readonly shippedRegressed: boolean +} + +/** One pull request's tracked findings, by status. */ +export interface TeamFindingCounts { + readonly repositoryId: string + readonly number: number + readonly findings: number + readonly critical: number + readonly resolved: number + readonly open: number +} + +export interface TeamCiActor { + readonly actor: string + readonly runs: number + readonly firstTryRuns: number + readonly firstTryGreen: number + readonly reruns: number + readonly cancelledRuns: number + readonly runnerMs: number + readonly defaultBranchRuns: number + readonly defaultBranchFailed: number +} + +type MemberCounts = { + -readonly [K in keyof CodeReviewTeamMember]: CodeReviewTeamMember[K] +} + +const emptyMember = (login: string): MemberCounts => ({ + login, + pullRequests: 0, + mergedPullRequests: 0, + medianMergeSeconds: null, + findings: 0, + criticalFindings: 0, + resolvedFindings: 0, + openAtMergeFindings: 0, + shippedClean: 0, + shippedRegressed: 0, + ciRuns: 0, + firstTryRuns: 0, + firstTryGreen: 0, + reruns: 0, + cancelledRuns: 0, + runnerMs: 0, +}) + +/** The pull request authors and CI actors as one row per login, bots left out. */ +export const buildTeam = (input: { + readonly startTime: number + readonly endTime: number + readonly pullRequests: ReadonlyArray + readonly findings: ReadonlyArray + readonly ci: ReadonlyArray + readonly p95QueueMs: number | null +}): CodeReviewTeam => { + const members = new Map() + const mergeSeconds = new Map>() + const memberOf = (login: string) => { + const found = members.get(login) + if (found !== undefined) return found + const created = emptyMember(login) + members.set(login, created) + return created + } + const findingsOf = new Map(input.findings.map((row) => [`${row.repositoryId}:${row.number}`, row])) + const people = input.pullRequests.filter((pr) => pr.author !== null && !isBotLogin(pr.author)) + + for (const pr of people) { + if (pr.author === null) continue + const member = memberOf(pr.author) + const merged = pr.mergeSeconds !== null + member.pullRequests += 1 + if (merged) { + member.mergedPullRequests += 1 + mergeSeconds.set(pr.author, [...(mergeSeconds.get(pr.author) ?? []), pr.mergeSeconds]) + } + if (pr.shippedRegressed) member.shippedRegressed += 1 + else if (pr.shippedClean) member.shippedClean += 1 + const found = findingsOf.get(`${pr.repositoryId}:${pr.number}`) + if (found === undefined) continue + member.findings += found.findings + member.criticalFindings += found.critical + // Fix rate is judged at the merge: an open pull request has not shipped its findings yet. + if (!merged) continue + member.resolvedFindings += found.resolved + member.openAtMergeFindings += found.open + } + for (const row of input.ci) { + if (row.actor === "" || isBotLogin(row.actor)) continue + const member = memberOf(row.actor) + member.ciRuns += row.runs + member.firstTryRuns += row.firstTryRuns + member.firstTryGreen += row.firstTryGreen + member.reruns += row.reruns + member.cancelledRuns += row.cancelledRuns + member.runnerMs += row.runnerMs + } + + const merged = people.flatMap((pr) => (pr.mergeSeconds === null ? [] : [pr.mergeSeconds])) + return new CodeReviewTeam({ + startTime: input.startTime, + endTime: input.endTime, + totals: new CodeReviewTeamTotals({ + pullRequests: people.length, + mergedPullRequests: merged.length, + medianMergeSeconds: median(merged), + medianFirstReviewSeconds: median( + people.flatMap((pr) => (pr.firstReviewSeconds === null ? [] : [pr.firstReviewSeconds])), + ), + defaultBranchRuns: input.ci.reduce((sum, row) => sum + row.defaultBranchRuns, 0), + defaultBranchFailed: input.ci.reduce((sum, row) => sum + row.defaultBranchFailed, 0), + runnerMs: input.ci.reduce((sum, row) => sum + row.runnerMs, 0), + p95QueueMs: input.p95QueueMs, + }), + members: [...members.values()] + .map( + (member) => + new CodeReviewTeamMember({ + ...member, + medianMergeSeconds: median(mergeSeconds.get(member.login) ?? []), + }), + ) + .toSorted((a, b) => a.login.localeCompare(b.login, "en", { sensitivity: "base" })), + }) +} + +/** A quantile over no rows is NaN or null. */ +const finiteOrNull = (value: number | null | undefined) => + value !== null && value !== undefined && Number.isFinite(value) ? value : null + +export class CodeReviewTeamService extends Context.Service()( + "@maple/backend/services/pr-review/CodeReviewTeamService", + { + make: Effect.gen(function* () { + const database = yield* Database + const warehouse = yield* WarehouseQueryService + const dbExecute = makeDbExecute( + database, + "CodeReviewTeamService", + (error) => new PrReviewPersistenceError({ message: error.message }), + ) + + const readPullRequests = ( + orgId: OrgId, + window: { readonly start: number; readonly end: number }, + repositoryId: VcsRepositoryId | undefined, + ) => + dbExecute((db) => + db.run( + PG.from(PrReviews) + .select(($) => { + const completed = $.status.eq("completed") + const reported = (verdict: string) => + PG.sql( + PG.bool, + )`coalesce(bool_or(${$.postMergeStatus} = 'reported' and ${$.postMergeJson}->>'verdict' = ${verdict}), false)` + return { + repositoryId: $.repositoryId, + number: $.number, + author: PG.sql(PG.nullable(PG.text))`max(${$.authorLogin})`, + lastCreatedAt: PG.max($.createdAt), + // First review to the merge. + mergeSeconds: PG.sql( + PG.nullable(PG.float8), + )`extract(epoch from (max(${$.mergedAt}) - min(${$.createdAt})))::float8`, + firstReviewSeconds: PG.sql( + PG.nullable(PG.float8), + )`extract(epoch from (min(${$.finishedAt}) filter (where ${completed}) - min(${$.createdAt})))::float8`, + shippedClean: reported("clean"), + shippedRegressed: reported("regressed"), + } + }) + .where(($) => [ + $.orgId.eq(orgId), + $.createdAt.gte(window.start - FIRST_REVIEW_LOOKBACK_MS), + $.createdAt.lt(window.end), + repositoryId === undefined ? undefined : $.repositoryId.eq(repositoryId), + ]) + .groupBy("repositoryId", "number") + // First reviewed in the window: a re-review of an older pull request is not one. + // In epoch ms: an aggregate loses the column's timestamp codec. + .having(($) => [ + PG.sql(PG.float8)`extract(epoch from min(${$.createdAt})) * 1000`.gte( + window.start, + ), + ]) + .orderBy(["lastCreatedAt", "desc"]) + .limit(MAX_PULL_REQUESTS), + ), + ) + + const readFindings = ( + orgId: OrgId, + window: { readonly start: number; readonly end: number }, + repositoryId: VcsRepositoryId | undefined, + ) => + dbExecute((db) => + db.run( + PG.from(PrReviewFindings) + .select(($) => ({ + repositoryId: $.repositoryId, + number: $.number, + findings: PG.count(), + critical: PG.countIf($.severity.eq("critical")), + resolved: PG.countIf($.status.eq("resolved")), + open: PG.countIf($.status.eq("open")), + })) + .where(($) => [ + $.orgId.eq(orgId), + // The same reach as the pull requests; findings of ones not read are dropped. + $.createdAt.gte(window.start - FIRST_REVIEW_LOOKBACK_MS), + $.createdAt.lt(window.end), + repositoryId === undefined ? undefined : $.repositoryId.eq(repositoryId), + ]) + .groupBy("repositoryId", "number"), + ), + ) + + const team = Effect.fn("CodeReviewTeamService.team")(function* ( + tenant: TenantContext, + query: CodeReviewTeamQuery, + ) { + const orgId = tenant.orgId + const { startTime, endTime } = ciWindow(query) + yield* Effect.annotateCurrentSpan({ + orgId, + "maple.code_review.span_ms": endTime - startTime, + "maple.code_review.filtered": query.repositoryId !== undefined, + }) + const repositories = yield* dbExecute((db) => + db.run( + PG.from(VcsRepositories) + .select(($) => ({ + id: $.id, + owner: $.owner, + name: $.name, + defaultBranch: $.defaultBranch, + })) + .where(($) => [$.orgId.eq(orgId)]), + ), + ) + const scoped = + query.repositoryId === undefined + ? repositories + : repositories.filter((repo) => repo.id === query.repositoryId) + const repository = query.repositoryId === undefined ? undefined : scoped[0] + // An id that is not the org's reads as an empty team, never another org's runs. + if (query.repositoryId !== undefined && repository === undefined) + return buildTeam({ + startTime, + endTime, + pullRequests: [], + findings: [], + ci: [], + p95QueueMs: null, + }) + + const scope: CH.CiScopeOpts = + repository === undefined ? {} : { owner: repository.owner, repository: repository.name } + const defaultBranches = scoped.map((repo) => ({ + owner: repo.owner, + repository: repo.name, + branch: repo.defaultBranch, + })) + const chWindow = { + orgId, + startTime: formatWarehouseDateTime(startTime), + endTime: formatWarehouseDateTime(endTime), + } + const window = { start: startTime, end: endTime } + const [pullRequests, findings, ci, queue] = yield* Effect.all( + [ + readPullRequests(orgId, window, query.repositoryId), + readFindings(orgId, window, query.repositoryId), + warehouse.compiledQuery( + tenant, + CH.compile(CH.ciActorStatsQuery({ ...scope, defaultBranches }), chWindow), + { profile: "aggregation", context: "ciActorStats" }, + ), + warehouse.compiledQuery(tenant, CH.compile(CH.ciQueueTotalsQuery(scope), chWindow), { + profile: "aggregation", + context: "ciQueueTotals", + }), + ], + { concurrency: "unbounded" }, + ) + yield* Effect.annotateCurrentSpan({ + "maple.code_review.pull_requests": pullRequests.length, + "maple.code_review.pull_requests_capped": pullRequests.length >= MAX_PULL_REQUESTS, + "maple.code_review.ci_actors": ci.length, + }) + const jobs = queue[0] + return buildTeam({ + startTime, + endTime, + pullRequests, + findings, + ci, + p95QueueMs: jobs === undefined || jobs.jobs === 0 ? null : finiteOrNull(jobs.p95QueueMs), + }) + }) + + return { team } satisfies CodeReviewTeamServiceApi + }), + }, +) { + static readonly layer = Layer.effect(this, this.make).pipe(Layer.provide(WarehouseQueryService.layer)) +} diff --git a/packages/domain/src/http/code-review.ts b/packages/domain/src/http/code-review.ts index 0218f8bfe6..72bcc6471f 100644 --- a/packages/domain/src/http/code-review.ts +++ b/packages/domain/src/http/code-review.ts @@ -374,6 +374,68 @@ export class CodeReviewCiRunsResponse extends Schema.Class + +/** Counts, not rates: the page divides, so a team median is over the same inputs. */ +export class CodeReviewTeamMember extends Schema.Class("CodeReviewTeamMember")({ + login: Schema.String, + /** Pull requests first reviewed in the window. */ + pullRequests: Schema.Number, + mergedPullRequests: Schema.Number, + /** First review to merge, over merged pull requests; null when none merged. */ + medianMergeSeconds: Schema.NullOr(Schema.Number), + findings: Schema.Number, + criticalFindings: Schema.Number, + /** Fixed in a later push, on merged pull requests: the fix rate is judged at the merge. */ + resolvedFindings: Schema.Number, + /** Still open when the pull request merged. */ + openAtMergeFindings: Schema.Number, + /** Merged pull requests the post-merge look judged. */ + shippedClean: Schema.Number, + shippedRegressed: Schema.Number, + /** Runs the person started that ran a job; runs that only waited for approval are left out. */ + ciRuns: Schema.Number, + /** First attempts that passed or failed: the base of the first-try green rate. */ + firstTryRuns: Schema.Number, + firstTryGreen: Schema.Number, + reruns: Schema.Number, + /** Cancelled runs, mostly a newer push taking over. */ + cancelledRuns: Schema.Number, + /** Time jobs spent on a runner, queue left out. */ + runnerMs: Schema.Number, +}) {} + +export class CodeReviewTeamTotals extends Schema.Class("CodeReviewTeamTotals")({ + pullRequests: Schema.Number, + mergedPullRequests: Schema.Number, + medianMergeSeconds: Schema.NullOr(Schema.Number), + /** Queued to posted, the first review of each pull request. */ + medianFirstReviewSeconds: Schema.NullOr(Schema.Number), + /** Pushes to a repository's default branch that passed or failed. */ + defaultBranchRuns: Schema.Number, + defaultBranchFailed: Schema.Number, + /** Every run's runner time, bots included. */ + runnerMs: Schema.Number, + p95QueueMs: Schema.NullOr(Schema.Number), +}) {} + +export class CodeReviewTeam extends Schema.Class("CodeReviewTeam")({ + /** The window read, epoch ms; a longer one is cut to what CI runs keep. */ + startTime: Schema.Number, + endTime: Schema.Number, + totals: CodeReviewTeamTotals, + /** By login. */ + members: Schema.Array(CodeReviewTeamMember), +}) {} + export class CodeReviewApiGroup extends HttpApiGroup.make("codeReview") .add( HttpApiEndpoint.get("analytics", "/analytics", { @@ -417,5 +479,12 @@ export class CodeReviewApiGroup extends HttpApiGroup.make("codeReview") error: [PrReviewPersistenceError, ...warehouseReadHttpErrors], }), ) + .add( + HttpApiEndpoint.get("team", "/team", { + query: CodeReviewTeamQuery, + success: CodeReviewTeam, + error: [PrReviewPersistenceError, ...warehouseReadHttpErrors], + }), + ) .prefix("/internal/code-review") .middleware(SessionAuthorization) {} diff --git a/packages/query-engine/src/__sql_baseline__/catalog.sql b/packages/query-engine/src/__sql_baseline__/catalog.sql index a7bfe4a8b4..528ab6aa16 100644 --- a/packages/query-engine/src/__sql_baseline__/catalog.sql +++ b/packages/query-engine/src/__sql_baseline__/catalog.sql @@ -407,6 +407,37 @@ SELECT LIMIT 200 FORMAT JSON +-- builder:ci-runs:ciActorStatsQuery:allWorkflows [d856dc60] +SELECT + ci_runs.Actor AS actor, + count() AS runs, + countIf((ci_runs.Attempt <= 1 AND (ci_runs.Result = 'success' OR ci_runs.Result IN ('failure', 'timeout', 'error')))) AS firstTryRuns, + countIf((ci_runs.Attempt <= 1 AND ci_runs.Result = 'success')) AS firstTryGreen, + countIf(ci_runs.Attempt > 1) AS reruns, + countIf(ci_runs.Result = 'cancellation') AS cancelledRuns, + sum(jobs.runnerMs) AS runnerMs, + countIf(((ci_runs.TriggerEvent = 'push' AND ((ci_runs.Owner = 'acme' AND ci_runs.Repository = 'web') AND ci_runs.Branch = 'main')) AND (ci_runs.Result = 'success' OR ci_runs.Result IN ('failure', 'timeout', 'error')))) AS defaultBranchRuns, + countIf(((ci_runs.TriggerEvent = 'push' AND ((ci_runs.Owner = 'acme' AND ci_runs.Repository = 'web') AND ci_runs.Branch = 'main')) AND ci_runs.Result IN ('failure', 'timeout', 'error'))) AS defaultBranchFailed + FROM ci_runs + INNER JOIN (SELECT + ci_jobs.TraceId AS TraceId, + sum(ci_jobs.Duration) / 1000000 - sum(ci_jobs.QueueMs) AS runnerMs + FROM ci_jobs + WHERE ci_jobs.OrgId = 'org_sql_catalog' + AND ci_jobs.Timestamp >= '2026-01-01 10:30:00' + AND ci_jobs.Timestamp <= '2026-01-03 14:15:00' + AND ci_jobs.Result != 'skip' + AND ci_jobs.Duration > 0 + GROUP BY TraceId) AS jobs ON ci_runs.TraceId = jobs.TraceId + WHERE ci_runs.OrgId = 'org_sql_catalog' + AND ci_runs.Timestamp >= '2026-01-01 10:30:00' + AND ci_runs.Timestamp <= '2026-01-03 14:15:00' + AND ci_runs.Result != 'skip' + GROUP BY actor + ORDER BY runs DESC + LIMIT 5000 + FORMAT JSON + -- builder:ci-runs:ciFailedJobsQuery:default [fc2d17c0] SELECT ci_jobs.TraceId AS traceId, diff --git a/packages/query-engine/src/benchmark/builders.ts b/packages/query-engine/src/benchmark/builders.ts index 074d4ef18e..3694bf5175 100644 --- a/packages/query-engine/src/benchmark/builders.ts +++ b/packages/query-engine/src/benchmark/builders.ts @@ -1863,6 +1863,18 @@ export const builderFixtures: ReadonlyArray = [ label: "allWorkflows", compile: () => CH.compileUnsafe(CH.ciQueueTotalsQuery({}), window), }, + { + module: "ci-runs", + name: "ciActorStatsQuery", + label: "allWorkflows", + compile: () => + CH.compileUnsafe( + CH.ciActorStatsQuery({ + defaultBranches: [{ owner: "acme", repository: "web", branch: "main" }], + }), + window, + ), + }, { module: "pr-review", name: "operationTrafficHourlyQuery", diff --git a/packages/query-engine/src/ch/index.ts b/packages/query-engine/src/ch/index.ts index d8bd25a617..0332bc58bc 100644 --- a/packages/query-engine/src/ch/index.ts +++ b/packages/query-engine/src/ch/index.ts @@ -757,6 +757,7 @@ export { // Queries — CI pipeline runs (GitHub Actions traces) export { CI_FAILED_RESULTS, + ciActorStatsQuery, ciFailedJobsQuery, ciJobStatsQuery, ciPipelineStatsQuery, @@ -767,6 +768,7 @@ export { ciRunsSpanMs, ciRunTotalsQuery, ciRunsQuery, + type CiActorStatsOpts, type CiFailedJobsOpts, type CiRedStreak, type CiRunsOpts, diff --git a/packages/query-engine/src/ch/queries/ci-runs.ts b/packages/query-engine/src/ch/queries/ci-runs.ts index 0c3f5d416f..935c2c04c9 100644 --- a/packages/query-engine/src/ch/queries/ci-runs.ts +++ b/packages/query-engine/src/ch/queries/ci-runs.ts @@ -265,3 +265,68 @@ export function ciQueueTotalsQuery(opts: CiScopeOpts) { .where(($) => ranJobs($, opts)) .format("JSON") } + +export interface CiActorStatsOpts extends CiScopeOpts { + /** Each repository's default branch, for the default-branch failure rate. */ + readonly defaultBranches: ReadonlyArray<{ + readonly owner: string + readonly repository: string + readonly branch: string + }> + readonly limit?: number +} + +/** + * Per run actor (`maple.cicd.actor`): runs, first-try results, reruns and runner time. Only runs + * with a job that ran count: a fork's run waiting for approval spans hours and does nothing. A + * job's span covers its wait for a runner, so the queue is taken off its runner time. + */ +export function ciActorStatsQuery(opts: CiActorStatsOpts) { + const ranJobsPerRun = from(CiJobs) + .select(($) => ({ + TraceId: $.TraceId, + runnerMs: CH.sum($.Duration).div(1000000).sub(CH.sum($.QueueMs)), + })) + .where(($) => ranJobs($, opts)) + .groupBy("TraceId") + return ( + from(CiRuns) + .innerJoinQuery(ranJobsPerRun, "jobs", (run, jobs) => run.TraceId.eq(jobs.TraceId)) + .select(($) => { + // 0 when the span carried no attempt: that is a first run, not a rerun. + const firstTry = $.Attempt.lte(1) + const passed = $.Result.eq("success") + const failed = CH.inList($.Result, CI_FAILED_RESULTS) + const isDefaultBranch = opts.defaultBranches + .map((repo) => + $.Owner.eq(repo.owner) + .and($.Repository.eq(repo.repository)) + .and($.Branch.eq(repo.branch)), + ) + .reduce( + (all, next) => (all === undefined ? next : all.or(next)), + undefined, + ) + const onDefaultBranch = $.TriggerEvent.eq("push").and( + isDefaultBranch ?? CH.inList($.Branch, []), + ) + return { + actor: $.Actor, + runs: CH.count(), + firstTryRuns: CH.countIf(firstTry.and(passed.or(failed))), + firstTryGreen: CH.countIf(firstTry.and(passed)), + reruns: CH.countIf($.Attempt.gt(1)), + cancelledRuns: CH.countIf($.Result.eq("cancellation")), + runnerMs: CH.sum($.jobs.runnerMs), + defaultBranchRuns: CH.countIf(onDefaultBranch.and(passed.or(failed))), + defaultBranchFailed: CH.countIf(onDefaultBranch.and(failed)), + } + }) + .where(($) => ranRuns($, opts)) + .groupBy("actor") + .orderBy(["runs", "desc"]) + // Totals are summed from these rows: the cap is far above any team. + .limit(opts.limit ?? 5_000) + .format("JSON") + ) +} From 5f87b24ee5d065cff4568f3f92f3a4c874e74222 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Sat, 31 Oct 2026 15:44:44 +0100 Subject: [PATCH 2/3] fix(code-review): satisfies for the ranked-by labels; note the empty default-branch list --- apps/web/src/page/code-review/team/view.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/web/src/page/code-review/team/view.ts b/apps/web/src/page/code-review/team/view.ts index 42aac6827d..c24f2d2ccd 100644 --- a/apps/web/src/page/code-review/team/view.ts +++ b/apps/web/src/page/code-review/team/view.ts @@ -127,7 +127,7 @@ const COLUMNS: ReadonlyArray = [ ] /** How the panel title names the column a ranked view orders by. */ -const RANKED_BY: Readonly> = { +const RANKED_BY = { merged: "pull requests merged", mergeTime: "time to merge", fixRate: "fix rate", @@ -136,7 +136,7 @@ const RANKED_BY: Readonly> = { rerunRate: "rerun rate", shippedClean: "shipped clean", ciTime: "CI time", -} +} satisfies Readonly> const headlineStrip = (h: H, totals: CodeReviewTeamTotals): Html => { const failRate = From 359f925cb941c4a7a3d4547c0b8cf191f93244b5 Mon Sep 17 00:00:00 2001 From: Makisuo Date: Sat, 31 Oct 2026 15:44:59 +0100 Subject: [PATCH 3/3] docs(query-engine): an empty default-branch list matches nothing --- packages/query-engine/src/ch/queries/ci-runs.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/query-engine/src/ch/queries/ci-runs.ts b/packages/query-engine/src/ch/queries/ci-runs.ts index 935c2c04c9..c3568aa11b 100644 --- a/packages/query-engine/src/ch/queries/ci-runs.ts +++ b/packages/query-engine/src/ch/queries/ci-runs.ts @@ -307,6 +307,7 @@ export function ciActorStatsQuery(opts: CiActorStatsOpts) { (all, next) => (all === undefined ? next : all.or(next)), undefined, ) + // No repository: an empty `inList` compiles to `1 = 0`, so nothing counts as the default branch. const onDefaultBranch = $.TriggerEvent.eq("push").and( isDefaultBranch ?? CH.inList($.Branch, []), )