diff --git a/apps/gittensory-ui/src/components/site/app-panels/maintainer-panel-slop.test.tsx b/apps/gittensory-ui/src/components/site/app-panels/maintainer-panel-slop.test.tsx new file mode 100644 index 0000000000..8d6a94e83c --- /dev/null +++ b/apps/gittensory-ui/src/components/site/app-panels/maintainer-panel-slop.test.tsx @@ -0,0 +1,60 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; + +// A maintainer session + a path-aware data hook: the dashboard call resolves with reviewability rows (one +// with a slop assessment, one without), every other call stays in loading so sub-panels render harmlessly. +const { useSession } = vi.hoisted(() => ({ useSession: vi.fn() })); +vi.mock("@/lib/api/session", () => ({ useSession: () => useSession() })); + +const dashboard = { + metrics: [], + health: [], + reviewability: [ + { + pr: "acme/widgets#7", + title: "Tidy things", + author: "alice", + bucket: "review-now", + reason: "cached open PR", + slop: { risk: 72, band: "high" }, + }, + { + pr: "acme/widgets#8", + title: "Real feature", + author: "bob", + bucket: "watch", + reason: "linked issue #3", + slop: null, + }, + ], + settingsPreview: { removed: [], added: [] }, +}; + +vi.mock("@/lib/api/use-api-resource", () => ({ + useApiResource: (path: string) => + path.includes("maintainer-dashboard") + ? { status: "ready", data: dashboard, reload: () => {}, error: null } + : { status: "loading", data: null, reload: () => {}, error: null }, +})); +// AiReviewSettings.load() awaits apiFetch and reads `.ok`; a benign not-ok response keeps it from throwing. +vi.mock("@/lib/api/request", () => ({ apiFetch: vi.fn(async () => ({ ok: false })) })); +vi.mock("@/lib/api/origin", () => ({ getApiOrigin: () => "https://api.test" })); + +import { MaintainerPanel } from "@/components/site/app-panels/maintainer-panel"; + +describe("MaintainerPanel slop score column", () => { + it("renders a slop band + risk pill for an assessed PR and an em-dash for an unassessed one", () => { + useSession.mockReturnValue({ + session: { login: "maint", roles: ["maintainer"] }, + hydrated: true, + }); + render(); + + // The new column header. + expect(screen.getByText("Slop")).toBeTruthy(); + // Assessed PR → band + score rendered together. + expect(screen.getByText(/high\s*72/)).toBeTruthy(); + // Unassessed PR → a muted em-dash placeholder. + expect(screen.getByText("—")).toBeTruthy(); + }); +}); diff --git a/apps/gittensory-ui/src/components/site/app-panels/maintainer-panel.tsx b/apps/gittensory-ui/src/components/site/app-panels/maintainer-panel.tsx index 3ad1294417..15bf0bc723 100644 --- a/apps/gittensory-ui/src/components/site/app-panels/maintainer-panel.tsx +++ b/apps/gittensory-ui/src/components/site/app-panels/maintainer-panel.tsx @@ -43,6 +43,14 @@ const BUCKET_TONE: Record = { redirect: "blocked", }; +// Deterministic slop band → pill tone. Advisory only (it never blocks); the colour just signals severity. +const SLOP_BAND_TONE: Record = { + clean: "ok", + low: "info", + elevated: "warn", + high: "blocked", +}; + type MaintainerDashboard = { metrics: Array<{ label: string; value: number; spark: number[] }>; health: Array<{ @@ -60,6 +68,7 @@ type MaintainerDashboard = { author: string; bucket: string; reason: string; + slop?: { risk: number; band: string } | null; }>; settingsPreview: { removed: string[]; added: string[] }; }; @@ -286,6 +295,7 @@ function MaintainerDashboardView() { Title Author Bucket + Slop Reason @@ -305,6 +315,20 @@ function MaintainerDashboardView() { {row.bucket} + + {row.slop ? ( + + {row.slop.band} {row.slop.risk} + + ) : ( + + — + + )} + {row.reason} ))} diff --git a/migrations/0035_pull_request_slop_assessment.sql b/migrations/0035_pull_request_slop_assessment.sql new file mode 100644 index 0000000000..d493af844c --- /dev/null +++ b/migrations/0035_pull_request_slop_assessment.sql @@ -0,0 +1,7 @@ +-- Persist the latest deterministic slop assessment per cached pull request so the maintainer dashboard can +-- surface a slop score row without re-fetching changed files on every load. Written by the public-surface +-- processor ONLY when the repo opted into slop (slop_gate_mode != 'off'); NULL means "not assessed". These +-- are gittensory-COMPUTED signals, deliberately omitted from the GitHub-sync upsert's SET clause so a +-- subsequent sync never clobbers them. +ALTER TABLE pull_requests ADD COLUMN slop_risk INTEGER; +ALTER TABLE pull_requests ADD COLUMN slop_band TEXT; diff --git a/src/api/routes.ts b/src/api/routes.ts index 0932817dd4..957abbd47f 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -1008,6 +1008,9 @@ export function createApp() { author: pull.authorLogin ?? "unknown", bucket: pull.state === "open" ? "review-now" : "watch", reason: pull.linkedIssues.length > 0 ? `linked issue #${pull.linkedIssues[0]}` : "cached open PR without linked issue", + // Latest deterministic slop assessment for this PR (null unless the repo opted into slop). Lets the + // maintainer panel render a per-PR slop band; never a private/scoreability signal. + slop: typeof pull.slopRisk === "number" && pull.slopBand ? { risk: pull.slopRisk, band: pull.slopBand } : null, })), settingsPreview: buildMaintainerSettingsPreview(), }); diff --git a/src/db/repositories.ts b/src/db/repositories.ts index b010a4551c..cd04903282 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -3538,9 +3538,29 @@ function toPullRequestRecordFromRow(row: typeof pullRequests.$inferSelect): Pull closedAt: payload.closed_at, labels: parseJson(row.labelsJson, []), linkedIssues: parseJson(row.linkedIssuesJson, []), + slopRisk: row.slopRisk, + slopBand: row.slopBand, }; } +/** + * Persist the latest deterministic slop assessment onto an existing cached PR row. Kept separate from the + * GitHub-sync upsert (whose SET clause never touches these columns) so a later sync cannot clobber the + * score. A no-op when the PR row does not exist yet — the sync upsert creates it first. + */ +export async function updatePullRequestSlopAssessment( + env: Env, + repoFullName: string, + pullNumber: number, + assessment: { slopRisk: number; slopBand: string }, +): Promise { + const db = getDb(env.DB); + await db + .update(pullRequests) + .set({ slopRisk: assessment.slopRisk, slopBand: assessment.slopBand, updatedAt: nowIso() }) + .where(and(eq(pullRequests.repoFullName, repoFullName), eq(pullRequests.number, pullNumber))); +} + function toIssueRecord(repoFullName: string, issue: GitHubIssuePayload): IssueRecord { /* v8 ignore start -- GitHub REST row normalization covers sparse provider payloads at representative persistence call sites. */ return { diff --git a/src/db/schema.ts b/src/db/schema.ts index 909127c6d5..ae99a8fe64 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -265,6 +265,9 @@ export const pullRequests = sqliteTable( linkedIssuesJson: text("linked_issues_json").notNull().default("[]"), lastSeenOpenAt: text("last_seen_open_at"), payloadJson: text("payload_json").notNull().default("{}"), + // Latest deterministic slop assessment (gittensory-computed; written separately from the GitHub sync). + slopRisk: integer("slop_risk"), + slopBand: text("slop_band"), createdAt: text("created_at").notNull().$defaultFn(() => nowIso()), updatedAt: text("updated_at").notNull().$defaultFn(() => nowIso()), }, diff --git a/src/queue/processors.ts b/src/queue/processors.ts index e3ec949298..1e14ef753a 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -27,6 +27,7 @@ import { listPullRequests, listPullRequestFiles, listRecentMergedPullRequests, + updatePullRequestSlopAssessment, listRepoLabels, listRepoPullRequestFiles, listRepoSyncStates, @@ -1152,6 +1153,9 @@ async function maybePublishPrPublicSurface( }); slopRisk = slop.slopRisk; advisory.findings.push(...slop.findings); + // Persist the assessment so the maintainer dashboard can show a per-PR slop score row without + // re-fetching changed files. Best-effort: a write hiccup must not abort gate evaluation. + await updatePullRequestSlopAssessment(env, repoFullName, pr.number, { slopRisk: slop.slopRisk, slopBand: slop.band }).catch(() => undefined); // AI-assisted slop advisory (#533, opt-in). Reuses the already-fetched files; appends at most one // advisory-only finding. Deliberately does NOT update slopRisk — only the deterministic core blocks. if (settings.slopAiAdvisory) { diff --git a/src/types.ts b/src/types.ts index 439e1e79fb..0411303939 100644 --- a/src/types.ts +++ b/src/types.ts @@ -338,6 +338,10 @@ export type PullRequestRecord = { closedAt?: string | null | undefined; labels: string[]; linkedIssues: number[]; + /** Latest deterministic slop assessment (0-100) and band, persisted by the public-surface processor when + * the repo opted into slop. `null`/absent = not assessed (slop off, or PR not yet processed). */ + slopRisk?: number | null | undefined; + slopBand?: string | null | undefined; }; export type IssueRecord = { diff --git a/test/integration/api.test.ts b/test/integration/api.test.ts index 250efdb55b..5cb94b9f63 100644 --- a/test/integration/api.test.ts +++ b/test/integration/api.test.ts @@ -26,6 +26,7 @@ import { recordAuditEvent, upsertIssueFromGitHub, upsertPullRequestFromGitHub, + updatePullRequestSlopAssessment, persistScoringModelSnapshot, upsertRepositoryFromGitHub, upsertRepositorySettings, @@ -2314,14 +2315,16 @@ describe("api routes", () => { labels: [], body: "No linked issue here.", }); + // PR2: a persisted slop assessment surfaces on the dashboard row; an unassessed PR carries slop: null. + await updatePullRequestSlopAssessment(env, "entrius/allways-ui", 14, { slopRisk: 80, slopBand: "high" }); const maintainer = await app.request("/v1/app/maintainer-dashboard", { headers: apiHeaders(env) }, env); expect(maintainer.status).toBe(200); await expect(maintainer.json()).resolves.toMatchObject({ installations: expect.any(Array), health: expect.arrayContaining([expect.objectContaining({ status: "healthy" })]), reviewability: expect.arrayContaining([ - expect.objectContaining({ pr: "entrius/allways-ui#12" }), - expect.objectContaining({ pr: "entrius/allways-ui#14", author: "unknown", reason: "cached open PR without linked issue" }), + expect.objectContaining({ pr: "entrius/allways-ui#12", slop: null }), + expect.objectContaining({ pr: "entrius/allways-ui#14", author: "unknown", reason: "cached open PR without linked issue", slop: { risk: 80, band: "high" } }), ]), settingsPreview: { added: expect.any(Array), removed: expect.any(Array) }, }); diff --git a/test/unit/data-spine.test.ts b/test/unit/data-spine.test.ts index 3f2d8a961d..0e00649c18 100644 --- a/test/unit/data-spine.test.ts +++ b/test/unit/data-spine.test.ts @@ -33,6 +33,7 @@ import { upsertInstallationHealth, upsertIssueFromGitHub, upsertPullRequestFromGitHub, + updatePullRequestSlopAssessment, upsertPullRequestFile, upsertPullRequestReview, upsertRecentMergedPullRequest, @@ -349,4 +350,24 @@ describe("data spine repositories", () => { expect(await listContributorPullRequests(env, "jsonbored")).toMatchObject([{ repoFullName: "owner/repo", number: 1 }]); expect(await listContributorIssues(env, "JSONBORED")).toEqual(expect.arrayContaining([expect.objectContaining({ repoFullName: "owner/repo", number: 10 }), expect.objectContaining({ repoFullName: "owner/repo", number: 11 })])); }); + + it("persists a per-PR slop assessment, round-trips it via the cached record, and keeps latest-wins (PR2)", async () => { + const env = createTestEnv(); + await upsertPullRequestFromGitHub(env, "owner/sloppr", { number: 5, title: "Churn", state: "open", user: { login: "alice" }, labels: [], body: "x" }); + // Unassessed by default (slop off, or PR not yet processed). + expect((await getPullRequest(env, "owner/sloppr", 5))?.slopRisk ?? null).toBeNull(); + expect((await getPullRequest(env, "owner/sloppr", 5))?.slopBand ?? null).toBeNull(); + + await updatePullRequestSlopAssessment(env, "owner/sloppr", 5, { slopRisk: 72, slopBand: "high" }); + const assessed = await getPullRequest(env, "owner/sloppr", 5); + expect(assessed?.slopRisk).toBe(72); + expect(assessed?.slopBand).toBe("high"); + + // Latest assessment wins on the next run. + await updatePullRequestSlopAssessment(env, "owner/sloppr", 5, { slopRisk: 10, slopBand: "low" }); + expect((await getPullRequest(env, "owner/sloppr", 5))?.slopBand).toBe("low"); + + // No-op (no throw) when the PR row does not exist yet. + await expect(updatePullRequestSlopAssessment(env, "owner/sloppr", 999, { slopRisk: 5, slopBand: "low" })).resolves.toBeUndefined(); + }); }); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 7ace91aa63..3193f0ed59 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -1000,6 +1000,11 @@ describe("queue processors", () => { gateCheckMode: "enabled", linkedIssueGateMode: "off", aiReviewMode: "block", + // Also exercise the opt-in slop advisory in the same surface pass: it persists a per-PR assessment + // and runs the (advisory-only) AI slop pass, but never blocks — the gate still fails on the AI + // consensus defect alone. + slopGateMode: "advisory", + slopAiAdvisory: true, }); let gatePatchBody: { conclusion?: string; output?: { title?: string; text?: string } } = {}; vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {