From 600b8c3ec6938ec79f88eef195b0a6dd7c33e7e4 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Wed, 17 Jun 2026 16:44:36 +0300 Subject: [PATCH 1/5] feat(code-review): add review analytics --- CONTEXT.md | 19 +- .../code-reviews/ReviewAgentPageClient.tsx | 30 +- .../code-reviews/ReviewAgentPageClient.tsx | 30 +- .../[reviewId]/route.test.ts | 95 + .../code-review-status/[reviewId]/route.ts | 196 +- .../analytics/AnalyticsBreakdownBars.tsx | 365 + .../analytics/AnalyticsTables.tsx | 526 + .../analytics/CodeReviewAnalyticsPanel.tsx | 602 + .../src/lib/agent-config/db/agent-configs.ts | 92 +- .../code-reviews/analytics/contracts.test.ts | 269 + .../lib/code-reviews/analytics/contracts.ts | 291 + .../src/lib/code-reviews/analytics/db.test.ts | 228 + apps/web/src/lib/code-reviews/analytics/db.ts | 786 + .../code-reviews/analytics/settings.test.ts | 177 + .../lib/code-reviews/analytics/settings.ts | 138 + .../lib/code-reviews/db/code-reviews.test.ts | 35 + .../src/lib/code-reviews/db/code-reviews.ts | 76 +- .../dispatch/dispatch-pending-reviews.test.ts | 115 +- .../dispatch/dispatch-pending-reviews.ts | 26 +- .../src/routers/code-reviews-router.test.ts | 166 +- apps/web/src/routers/code-reviews-router.ts | 5 +- .../code-review-analytics-router.test.ts | 255 + .../code-review-analytics-router.ts | 98 + .../code-reviews/code-reviews-router.ts | 3 + .../organization-code-reviews-router.ts | 5 +- packages/db/src/migrations/0166_open_xorn.sql | 62 + .../db/src/migrations/meta/0166_snapshot.json | 31357 ++++++++++++++++ packages/db/src/migrations/meta/_journal.json | 7 + packages/db/src/schema-types.ts | 101 + packages/db/src/schema.test.ts | 42 + packages/db/src/schema.ts | 143 + 31 files changed, 36204 insertions(+), 136 deletions(-) create mode 100644 apps/web/src/components/code-reviews/analytics/AnalyticsBreakdownBars.tsx create mode 100644 apps/web/src/components/code-reviews/analytics/AnalyticsTables.tsx create mode 100644 apps/web/src/components/code-reviews/analytics/CodeReviewAnalyticsPanel.tsx create mode 100644 apps/web/src/lib/code-reviews/analytics/contracts.test.ts create mode 100644 apps/web/src/lib/code-reviews/analytics/contracts.ts create mode 100644 apps/web/src/lib/code-reviews/analytics/db.test.ts create mode 100644 apps/web/src/lib/code-reviews/analytics/db.ts create mode 100644 apps/web/src/lib/code-reviews/analytics/settings.test.ts create mode 100644 apps/web/src/lib/code-reviews/analytics/settings.ts create mode 100644 apps/web/src/routers/code-reviews/code-review-analytics-router.test.ts create mode 100644 apps/web/src/routers/code-reviews/code-review-analytics-router.ts create mode 100644 packages/db/src/migrations/0166_open_xorn.sql create mode 100644 packages/db/src/migrations/meta/0166_snapshot.json diff --git a/CONTEXT.md b/CONTEXT.md index d3ecee3387..47dd1b39f2 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -2,12 +2,13 @@ ## Scope -Kilo Code Cloud hosts Kilo Code agents, integrations, and automation. This contract currently defines Security Agent language and notification ownership boundaries used across sync, web, email, remediation, tests, and product documentation. +Kilo Code Cloud hosts Kilo Code agents, integrations, and automation. This contract defines Code Reviewer and Security Agent language plus ownership boundaries used across review execution, analytics, sync, web, email, remediation, tests, and product documentation. ## Contexts | Context | Owns | Location | Notes | |---|---|---|---| +| **Code Reviewer** | Pull request and merge request review execution, Code Review Findings, review settings, and Review Analytics | `apps/web/src/lib/code-reviews/`, `apps/web/src/components/code-reviews/` | A Code Reviewer owner is either one user or one organization; Review Analytics collection is owner- and platform-scoped | | **Security Agent** | Security Findings, owner-scoped policy, settings, Auto Remediation, and user-visible outcomes | `apps/web/src/lib/security-agent/`, `apps/web/src/components/security-agent/`, `.specs/security-agent.md` | A Security Agent owner is either one user or one organization | | **Security Sync** | Dependabot synchronization, finding persistence, notification eligibility, recipient intent materialization, and durable notification state | `services/security-sync/` | Event state remains owner-scoped; email sending does not occur inside finding persistence transactions | | **Security Agent Email Delivery** | Dispatch-time revalidation, email rendering, owner-aware links, and Mailgun delivery | `apps/web/src/app/api/internal/security-agent/`, `apps/web/src/lib/email.ts`, `apps/web/src/emails/` | Accepts notification identity only and loads current data before sending | @@ -17,6 +18,10 @@ Kilo Code Cloud hosts Kilo Code agents, integrations, and automation. This contr | Term | Agent meaning | Use this when | Avoid | |---|---|---|---| +| **Code Reviewer** | Agent that reviews pull requests and merge requests and may raise Code Review Findings | Naming the product capability, settings, review execution, and analytics | Security Agent, review bot | +| **Code Review Finding** | Model-generated issue newly raised by Code Reviewer during one review execution | Referring to Code Reviewer output or its controlled analytics taxonomy | Security Finding, confirmed bug, verified vulnerability | +| **Review Analytics** | Opt-in, prospective collection of bounded classifications for completed reviews and newly raised Code Review Findings | Referring to the Code Reviewer Analytics tab, collection setting, coverage, or aggregate metrics | Security Agent analytics, historical backfill | +| **AI-estimated impact** | Code Reviewer's low, medium, or high estimate of a change's reach and consequence, independent of diff size, change type, complexity, and finding count | Referring to impact classifications or derived impact points | Developer quality, individual performance, delivered impact | | **Security Agent** | Agent that syncs, analyzes, and helps resolve repository Security Findings | Naming product capability, settings, routes, and behavior | Security Reviews | | **Security Finding** | Vulnerability item owned by one user or organization for a repository, usually synced from Dependabot | Referring to Kilo's persisted vulnerability domain object | Security review, alert | | **Auto Remediation** | Security Agent feature that automatically starts Security Remediations for eligible Security Findings | Referring to policy-driven remediation admission | Auto Fix | @@ -32,6 +37,9 @@ Kilo Code Cloud hosts Kilo Code agents, integrations, and automation. This contr ## Relationships +- A **Code Review Finding** belongs to one captured Code Reviewer review result and contains only controlled taxonomy values in Review Analytics. +- **Review Analytics** enrollment is snapshotted when a Code Reviewer execution attempt is dispatched; changing the setting does not change an in-flight attempt. +- **AI-estimated impact** describes a reviewed change and remains independent from Code Review Finding counts. - A **Security Finding** belongs to exactly one Security Agent owner: one user or one organization. - A **Security Finding** can create at most one **Security Agent Notification** of each kind per **Notification Recipient**. - A **New-finding Notification** depends on first insertion into Kilo, not source alert creation time. @@ -42,6 +50,10 @@ Kilo Code Cloud hosts Kilo Code agents, integrations, and automation. This contr ## Agent Rules +- Use **Code Review Finding** for an issue raised by Code Reviewer. Never call it a **Security Finding**, even when its category is `security`. +- Describe Review Analytics values as model-generated signals: use "findings raised" and **AI-estimated impact**, not confirmed bugs, verified vulnerabilities, or developer quality. +- Keep Review Analytics prospective and opt-in. Missing, invalid, or omitted structured results are unavailable coverage states, not zero-finding reviews. +- Do not persist finding prose, code, paths, lines, symbols, prompts, raw manifests, or full assistant output in Review Analytics. - Use **Security Finding** for Kilo's persisted domain object. Use "Dependabot alert" only for external source object at GitHub boundary. - Use exact notification kind when discussing eligibility or history: **New-finding Notification**, **SLA Warning Notification**, or **SLA Breach Notification**. - Treat "new" as first insertion for owner in Kilo. Updates and reopening do not make finding new again. @@ -55,6 +67,8 @@ Kilo Code Cloud hosts Kilo Code agents, integrations, and automation. This contr | Ambiguous term | Problem | Canonical decision | |---|---|---| +| finding | Can mean Code Reviewer output or the Security Agent's persisted vulnerability object | Use **Code Review Finding** for Code Reviewer output and **Security Finding** only for the Security Agent domain object | +| impact | Can imply delivered business value, diff size, complexity, or individual performance | Use **AI-estimated impact** only for the model-generated reach-and-consequence classification | | alert | Can mean external Dependabot alert, persisted Security Finding, or outgoing notification | Use "Dependabot alert" at source boundary, **Security Finding** after persistence, and exact notification kind for outgoing event | | notification email | Conflates durable semantic event with retryable provider side effect | Use **Security Agent Notification** for event and **Email Delivery** for send attempt | | new finding | Can mean newly created at source, first observed, inserted, updated, or reopened | For notification policy, it means first insertion into Kilo for owner | @@ -63,6 +77,8 @@ Kilo Code Cloud hosts Kilo Code agents, integrations, and automation. This contr ## Context Boundaries +- **Code Reviewer** owns review execution, Code Review Findings, Review Analytics settings, and user-visible aggregate review signals. +- Review Analytics stores bounded taxonomy observations separately from Security Agent `security_findings` and does not establish a cross-review finding lifecycle. - **Security Agent** owns product policy, settings, permissions, and user-visible finding/remediation outcomes. - **Security Sync** owns finding synchronization, notification event admission, recipient intent materialization, deduplication, and durable state transitions. - **Security Agent Email Delivery** may revalidate and deliver an existing notification but must not create notification eligibility or copy mutable finding data into Worker request. @@ -71,5 +87,6 @@ Kilo Code Cloud hosts Kilo Code agents, integrations, and automation. This contr ## Decision References +- `.plans/code-review-analytics.md` defines prospective Review Analytics collection, taxonomy, persistence, and metric semantics. - `.specs/security-agent.md` defines Security Agent Auto Remediation and notification guarantees. - `.plans/security-agent-notifications.md` records notification implementation and rollout design. diff --git a/apps/web/src/app/(app)/code-reviews/ReviewAgentPageClient.tsx b/apps/web/src/app/(app)/code-reviews/ReviewAgentPageClient.tsx index ad5934cc4c..9c85e857cc 100644 --- a/apps/web/src/app/(app)/code-reviews/ReviewAgentPageClient.tsx +++ b/apps/web/src/app/(app)/code-reviews/ReviewAgentPageClient.tsx @@ -6,12 +6,20 @@ import { ReviewConfigForm } from '@/components/code-reviews/ReviewConfigForm'; import { CodeReviewActionRequiredAlert } from '@/components/code-reviews/CodeReviewActionRequiredAlert'; import { CodeReviewJobsCard } from '@/components/code-reviews/CodeReviewJobsCard'; import { ReviewMemoryPanel } from '@/components/code-reviews/ReviewMemoryPanel'; +import { CodeReviewAnalyticsPanel } from '@/components/code-reviews/analytics/CodeReviewAnalyticsPanel'; import { Alert, AlertDescription, AlertTitle } from '@/components/ui/alert'; import { Badge } from '@/components/ui/badge'; import { Button } from '@/components/ui/button'; import { SetPageTitle } from '@/components/SetPageTitle'; import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs'; -import { Rocket, ExternalLink, Settings2, ListChecks, Brain } from 'lucide-react'; +import { + Brain, + ChartColumnIncreasing, + ExternalLink, + ListChecks, + Rocket, + Settings2, +} from 'lucide-react'; import { useTRPC } from '@/lib/trpc/utils'; import { useQuery } from '@tanstack/react-query'; import Link from 'next/link'; @@ -164,7 +172,7 @@ export function ReviewAgentPageClient({ {/* GitHub Configuration Tabs */} - + Config @@ -185,6 +193,10 @@ export function ReviewAgentPageClient({ Memory + + + Analytics + @@ -219,6 +231,10 @@ export function ReviewAgentPageClient({ )} + + + + @@ -250,7 +266,7 @@ export function ReviewAgentPageClient({ {/* GitLab Configuration Tabs */} - + Config @@ -263,6 +279,10 @@ export function ReviewAgentPageClient({ Jobs + + + Analytics + @@ -298,6 +318,10 @@ export function ReviewAgentPageClient({ )} + + + + diff --git a/apps/web/src/app/(app)/organizations/[id]/code-reviews/ReviewAgentPageClient.tsx b/apps/web/src/app/(app)/organizations/[id]/code-reviews/ReviewAgentPageClient.tsx index 96e1fbb33a..3acabb7a23 100644 --- a/apps/web/src/app/(app)/organizations/[id]/code-reviews/ReviewAgentPageClient.tsx +++ b/apps/web/src/app/(app)/organizations/[id]/code-reviews/ReviewAgentPageClient.tsx @@ -6,12 +6,20 @@ import { ReviewConfigForm } from '@/components/code-reviews/ReviewConfigForm'; import { CodeReviewActionRequiredAlert } from '@/components/code-reviews/CodeReviewActionRequiredAlert'; import { CodeReviewJobsCard } from '@/components/code-reviews/CodeReviewJobsCard'; import { ReviewMemoryPanel } from '@/components/code-reviews/ReviewMemoryPanel'; +import { CodeReviewAnalyticsPanel } from '@/components/code-reviews/analytics/CodeReviewAnalyticsPanel'; import { Alert, AlertDescription, AlertTitle } from '@/components/ui/alert'; import { Badge } from '@/components/ui/badge'; import { Button } from '@/components/ui/button'; import { SetPageTitle } from '@/components/SetPageTitle'; import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs'; -import { Rocket, ExternalLink, Settings2, ListChecks, Brain } from 'lucide-react'; +import { + Brain, + ChartColumnIncreasing, + ExternalLink, + ListChecks, + Rocket, + Settings2, +} from 'lucide-react'; import { useTRPC } from '@/lib/trpc/utils'; import { useQuery } from '@tanstack/react-query'; import Link from 'next/link'; @@ -177,7 +185,7 @@ export function ReviewAgentPageClient({ {/* GitHub Configuration Tabs */} - + Config @@ -198,6 +206,10 @@ export function ReviewAgentPageClient({ Memory + + + Analytics + @@ -232,6 +244,10 @@ export function ReviewAgentPageClient({ )} + + + + @@ -266,7 +282,7 @@ export function ReviewAgentPageClient({ {/* GitLab Configuration Tabs */} - + Config @@ -279,6 +295,10 @@ export function ReviewAgentPageClient({ Jobs + + + Analytics + @@ -315,6 +335,10 @@ export function ReviewAgentPageClient({ )} + + + + diff --git a/apps/web/src/app/api/internal/code-review-status/[reviewId]/route.test.ts b/apps/web/src/app/api/internal/code-review-status/[reviewId]/route.test.ts index 3cff92ae95..838b859191 100644 --- a/apps/web/src/app/api/internal/code-review-status/[reviewId]/route.test.ts +++ b/apps/web/src/app/api/internal/code-review-status/[reviewId]/route.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, jest, beforeEach } from '@jest/globals'; import type { NextRequest } from 'next/server'; import type * as codeReviewsDbModule from '@/lib/code-reviews/db/code-reviews'; +import type * as analyticsDbModule from '@/lib/code-reviews/analytics/db'; import type * as platformIntegrationsModule from '@/lib/integrations/db/platform-integrations'; import type { CloudAgentCodeReview, @@ -35,6 +36,9 @@ const mockGetLatestCodeReviewAttempt = jest.fn() as jest.MockedFunction< const mockCreateInfraRetryAttemptIfMissing = jest.fn() as jest.MockedFunction< typeof codeReviewsDbModule.createInfraRetryAttemptIfMissing >; +const mockFinalizeCompletedCodeReviewWithAnalytics = jest.fn() as jest.MockedFunction< + typeof analyticsDbModule.finalizeCompletedCodeReviewWithAnalytics +>; const mockGetIntegrationById = jest.fn() as jest.MockedFunction< typeof platformIntegrationsModule.getIntegrationById >; @@ -98,6 +102,10 @@ jest.mock('@/lib/code-reviews/db/code-reviews', () => ({ createInfraRetryAttemptIfMissing: mockCreateInfraRetryAttemptIfMissing, })); +jest.mock('@/lib/code-reviews/analytics/db', () => ({ + finalizeCompletedCodeReviewWithAnalytics: mockFinalizeCompletedCodeReviewWithAnalytics, +})); + jest.mock('@/lib/code-reviews/client/code-review-worker-client', () => ({ codeReviewWorkerClient: { retryReviewFresh: mockRetryReviewFresh, @@ -261,6 +269,7 @@ function makeAttempt( session_id: null, cli_session_id: null, execution_id: null, + analytics_enabled_at_dispatch: null, status: 'running', error_message: null, terminal_reason: null, @@ -391,6 +400,7 @@ beforeEach(async () => { mockGetSessionUsageFromBilling.mockResolvedValue(null); mockUpdateCodeReviewUsage.mockResolvedValue(undefined); mockUpdateCodeReviewStatusIfNonTerminal.mockResolvedValue(true); + mockFinalizeCompletedCodeReviewWithAnalytics.mockResolvedValue({ outcome: 'applied' }); mockAppendPreviousReviewSummaryHistory.mockImplementation((body: string) => body); mockBuildReviewSummaryFooter.mockImplementation( (footer: { usage?: unknown; reviewGuidance?: { used: boolean } }) => @@ -459,6 +469,91 @@ describe('POST /api/internal/code-review-status/[reviewId]', () => { }); }); + describe('analytics completion callbacks', () => { + it('rejects invalid callback payloads at runtime', async () => { + const response = await POST(makeRequest({ status: 'unknown' }), makeParams(REVIEW_ID)); + + expect(response.status).toBe(400); + await expect(response.json()).resolves.toEqual({ error: 'Invalid callback payload' }); + expect(mockGetCodeReviewById).not.toHaveBeenCalled(); + }); + + it('finalizes an enrolled captured result before provider side effects', async () => { + mockGetCodeReviewById.mockResolvedValue(makeReview()); + const attempt = makeAttempt({ analytics_enabled_at_dispatch: true }); + mockGetLatestCodeReviewAttempt.mockResolvedValue(attempt); + const marker = + ''; + + const response = await POST( + makeRequest({ + status: 'completed', + sessionId: 'agent-session', + lastAssistantMessageText: `Review complete.\n${marker}`, + }), + makeParams(REVIEW_ID) + ); + + expect(response.status).toBe(200); + expect(mockFinalizeCompletedCodeReviewWithAnalytics).toHaveBeenCalledWith( + expect.objectContaining({ + codeReviewId: REVIEW_ID, + capture: expect.objectContaining({ + status: 'captured', + manifest: expect.objectContaining({ findings: [expect.any(Object)] }), + }), + }) + ); + expect(mockUpdateCodeReviewAttemptForCallback).not.toHaveBeenCalled(); + expect(mockUpdateCodeReviewStatus).not.toHaveBeenCalled(); + expect(mockUpdateCheckRun).toHaveBeenCalled(); + }); + + it.each([ + ['missing', { lastAssistantMessageText: 'Review complete.' }], + ['invalid', { lastAssistantMessageText: '' }], + [ + 'omitted', + { + lastAssistantMessageTextTruncation: { + originalUtf8ByteLength: 200000, + retainedUtf8ByteLength: 0, + }, + }, + ], + ] as const)('maps assistant output to %s coverage', async (expectedStatus, payload) => { + mockGetCodeReviewById.mockResolvedValue(makeReview()); + mockGetLatestCodeReviewAttempt.mockResolvedValue( + makeAttempt({ analytics_enabled_at_dispatch: true }) + ); + + await POST(makeRequest({ status: 'completed', ...payload }), makeParams(REVIEW_ID)); + + expect(mockFinalizeCompletedCodeReviewWithAnalytics).toHaveBeenCalledWith( + expect.objectContaining({ capture: { status: expectedStatus } }) + ); + }); + + it('does not replay provider completion side effects for analytics repair', async () => { + mockGetCodeReviewById.mockResolvedValue(makeReview({ status: 'completed' })); + mockGetLatestCodeReviewAttempt.mockResolvedValue( + makeAttempt({ status: 'completed', analytics_enabled_at_dispatch: true }) + ); + mockFinalizeCompletedCodeReviewWithAnalytics.mockResolvedValue({ outcome: 'repaired' }); + + const response = await POST( + makeRequest({ status: 'completed', lastAssistantMessageText: 'Review complete.' }), + makeParams(REVIEW_ID) + ); + + expect(response.status).toBe(200); + expect(mockGetIntegrationById).not.toHaveBeenCalled(); + expect(mockUpdateCheckRun).not.toHaveBeenCalled(); + expect(mockAddReactionToPR).not.toHaveBeenCalled(); + expect(mockTryDispatchPendingReviews).not.toHaveBeenCalled(); + }); + }); + describe('normalization', () => { it('maps interrupted status to cancelled with interrupted terminal reason', async () => { mockGetCodeReviewById.mockResolvedValue(makeReview()); diff --git a/apps/web/src/app/api/internal/code-review-status/[reviewId]/route.ts b/apps/web/src/app/api/internal/code-review-status/[reviewId]/route.ts index 2f7ffc0a7e..687ed6513f 100644 --- a/apps/web/src/app/api/internal/code-review-status/[reviewId]/route.ts +++ b/apps/web/src/app/api/internal/code-review-status/[reviewId]/route.ts @@ -18,6 +18,7 @@ import type { NextRequest } from 'next/server'; import { NextResponse } from 'next/server'; +import * as z from 'zod'; import { updateCodeReviewStatus, updateCodeReviewStatusIfNonTerminal, @@ -88,37 +89,48 @@ import { type CodeReviewActionRequiredReason, } from '@/lib/code-reviews/action-required'; import type { Owner } from '@/lib/code-reviews/core'; +import { parseCodeReviewAnalyticsManifest } from '@/lib/code-reviews/analytics/contracts'; +import { finalizeCompletedCodeReviewWithAnalytics } from '@/lib/code-reviews/analytics/db'; + +const CallbackTextTruncationSchema = z + .object({ + originalUtf8ByteLength: z.number().int().nonnegative(), + retainedUtf8ByteLength: z.number().int().nonnegative(), + }) + .strict(); + +const StatusUpdatePayloadSchema = z + .object({ + sessionId: z.string().optional(), + cliSessionId: z.string().optional(), + cloudAgentSessionId: z.string().optional(), + executionId: z.string().optional(), + messageId: z.string().optional(), + kiloSessionId: z.string().optional(), + idempotencyKey: z.string().optional(), + status: z.enum(['running', 'completed', 'failed', 'cancelled', 'interrupted']), + errorMessage: z.string().optional(), + terminalReason: z.enum(CODE_REVIEW_TERMINAL_REASONS).optional(), + modelNotFoundRuntimeDiagnostics: z.unknown().optional(), + failure: z.unknown().optional(), + failureStage: z.unknown().optional(), + clientError: z.unknown().optional(), + errorMessageTruncation: CallbackTextTruncationSchema.optional(), + lastSeenBranch: z.string().optional(), + gateResult: z.enum(['pass', 'fail']).optional(), + lastAssistantMessageText: z.string().optional(), + lastAssistantMessageTextTruncation: CallbackTextTruncationSchema.optional(), + }) + .refine( + payload => + !( + payload.lastAssistantMessageText !== undefined && + payload.lastAssistantMessageTextTruncation?.retainedUtf8ByteLength === 0 + ), + { message: 'Assistant text cannot be present when it was omitted' } + ); -/** - * Payload from the orchestrator DO (legacy format). - */ -type OrchestratorPayload = { - sessionId?: string; - cliSessionId?: string; - status: 'running' | 'completed' | 'failed' | 'cancelled'; - errorMessage?: string; - terminalReason?: CodeReviewTerminalReason; - gateResult?: 'pass' | 'fail'; -}; - -/** - * Payload from cloud-agent-next callback (ExecutionCallbackPayload). - */ -type CloudAgentNextCallbackPayload = { - sessionId?: string; - cloudAgentSessionId?: string; - executionId?: string; - kiloSessionId?: string; - status: 'completed' | 'failed' | 'interrupted'; - errorMessage?: string; - terminalReason?: CodeReviewTerminalReason; - modelNotFoundRuntimeDiagnostics?: unknown; - failure?: unknown; - lastSeenBranch?: string; - gateResult?: 'pass' | 'fail'; -}; - -type StatusUpdatePayload = OrchestratorPayload | CloudAgentNextCallbackPayload; +type StatusUpdatePayload = z.infer; type TerminalOwnerResolution = { owner: Owner; @@ -980,24 +992,18 @@ export async function POST( return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); } - const rawPayload: StatusUpdatePayload = await req.json(); + const rawBody: unknown = await req.json(); + const parsedPayload = StatusUpdatePayloadSchema.safeParse(rawBody); + if (!parsedPayload.success) { + return NextResponse.json({ error: 'Invalid callback payload' }, { status: 400 }); + } + + const rawPayload = parsedPayload.data; const attemptId = callbackAttemptId || undefined; const { status, sessionId, cliSessionId, errorMessage, terminalReason, gateResult, failure } = normalizePayload(rawPayload); - const executionId = 'executionId' in rawPayload ? rawPayload.executionId : undefined; - - // Validate payload - if (!status) { - return NextResponse.json({ error: 'Missing required field: status' }, { status: 400 }); - } - - // Warn on unexpected gateResult values so agent-side typos surface early - const validGateResult = gateResult === 'pass' || gateResult === 'fail' ? gateResult : undefined; - if (gateResult && !validGateResult) { - logExceptInTest('[code-review-status] Unexpected gateResult value, ignoring', { - gateResult, - }); - } + const executionId = rawPayload.executionId; + const validGateResult = gateResult; const loggableErrorMessage = getLoggableStatusErrorMessage(errorMessage, terminalReason); logExceptInTest('[code-review-status] Received status update', { @@ -1018,35 +1024,78 @@ export async function POST( return NextResponse.json({ error: 'Review not found' }, { status: 404 }); } - const attempt = await updateCodeReviewAttemptForCallback({ - codeReviewId: reviewId, - attemptId: attemptId ?? undefined, - status, - sessionId, - cliSessionId, - executionId, - errorMessage, - terminalReason, - startedAt: status === 'running' ? new Date() : undefined, - completedAt: - status === 'completed' || status === 'failed' || status === 'cancelled' - ? new Date() - : undefined, - }); + const callbackCompletedAt = new Date(); + let attempt: CloudAgentCodeReviewAttempt; + let latestAttempt = await getLatestCodeReviewAttempt(reviewId); + let analyticsCompletionApplied = false; - const latestAttempt = await getLatestCodeReviewAttempt(reviewId); - const isStaleAttempt = !!latestAttempt && attempt.id !== latestAttempt.id; - if (isStaleAttempt) { - logExceptInTest('[code-review-status] Stale callback updated old attempt, skipping parent', { - reviewId, - attemptId: attempt.id, - latestAttemptId: latestAttempt?.id, - requestedStatus: status, + if (status === 'completed' && latestAttempt?.analytics_enabled_at_dispatch === true) { + const capture = parseCodeReviewAnalyticsManifest(rawPayload.lastAssistantMessageText, { + assistantTextWasOmitted: + rawPayload.lastAssistantMessageText === undefined && + rawPayload.lastAssistantMessageTextTruncation?.retainedUtf8ByteLength === 0, }); - return NextResponse.json({ - success: true, - message: 'Stale callback from superseded attempt', + const completionResult = await finalizeCompletedCodeReviewWithAnalytics({ + codeReviewId: reviewId, + sourceAttemptId: attemptId, + sessionId, + cliSessionId, + executionId, + completedAt: callbackCompletedAt, + capture, }); + + if (completionResult.outcome !== 'applied') { + return NextResponse.json({ + success: true, + message: + completionResult.outcome === 'stale' + ? 'Stale callback from superseded attempt' + : completionResult.outcome === 'terminal' + ? 'Review already in terminal state' + : 'Review completion already processed', + outcome: completionResult.outcome, + currentStatus: completionResult.currentStatus, + terminalReason: completionResult.terminalReason, + }); + } + + attempt = latestAttempt; + analyticsCompletionApplied = true; + } else { + attempt = await updateCodeReviewAttemptForCallback({ + codeReviewId: reviewId, + attemptId: attemptId ?? undefined, + status, + sessionId, + cliSessionId, + executionId, + errorMessage, + terminalReason, + startedAt: status === 'running' ? callbackCompletedAt : undefined, + completedAt: + status === 'completed' || status === 'failed' || status === 'cancelled' + ? callbackCompletedAt + : undefined, + }); + + latestAttempt = await getLatestCodeReviewAttempt(reviewId); + const isStaleAttempt = !!latestAttempt && attempt.id !== latestAttempt.id; + if (isStaleAttempt) { + logExceptInTest( + '[code-review-status] Stale callback updated old attempt, skipping parent', + { + reviewId, + attemptId: attempt.id, + latestAttemptId: latestAttempt?.id, + requestedStatus: status, + } + ); + return NextResponse.json({ + success: true, + message: 'Stale callback from superseded attempt', + }); + } } // Determine valid transitions based on incoming status @@ -1077,6 +1126,7 @@ export async function POST( const updatesLatestAttemptSession = sessionId && latestAttempt?.id === attempt.id && attempt.session_id === sessionId; if ( + !analyticsCompletionApplied && sessionId && review.session_id && sessionId !== review.session_id && @@ -1262,7 +1312,9 @@ export async function POST( status === 'cancelled' && isModelNotFoundCodeReviewTerminalReason(terminalReason, errorMessage); - if (isModelNotFoundCancellation) { + if (analyticsCompletionApplied) { + // Parent and accepted attempt completion were claimed with analytics in one transaction. + } else if (isModelNotFoundCancellation) { const claimedTerminalUpdate = await updateCodeReviewStatusIfNonTerminal( reviewId, status, diff --git a/apps/web/src/components/code-reviews/analytics/AnalyticsBreakdownBars.tsx b/apps/web/src/components/code-reviews/analytics/AnalyticsBreakdownBars.tsx new file mode 100644 index 0000000000..14c04f431e --- /dev/null +++ b/apps/web/src/components/code-reviews/analytics/AnalyticsBreakdownBars.tsx @@ -0,0 +1,365 @@ +'use client'; + +import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/components/ui/card'; +import { cn } from '@/lib/utils'; + +type ChangeType = + | 'bug_fix' + | 'feature' + | 'refactor' + | 'maintenance' + | 'dependency' + | 'test' + | 'documentation' + | 'mixed' + | 'other'; + +type ComplexityLevel = 'low' | 'medium' | 'high'; + +type FindingCategory = + | 'security' + | 'correctness' + | 'reliability' + | 'data_integrity' + | 'performance' + | 'compatibility' + | 'maintainability' + | 'test_quality' + | 'documentation' + | 'accessibility' + | 'other'; + +type SecurityClass = + | 'auth_access' + | 'injection' + | 'data_protection' + | 'request_resource_boundary' + | 'deserialization_object_integrity' + | 'dependency_supply_chain' + | 'memory_safety' + | 'availability' + | 'concurrency' + | 'security_configuration' + | 'other'; + +type DistributionRow = { + value: T; + count: number; + lowConfidenceCount: number; +}; + +type SeverityBreakdownRow = { + value: T; + total: number; + critical: number; + warning: number; + suggestion: number; +}; + +type ImpactBreakdown = { + impact: Record<'low' | 'medium' | 'high' | 'unclassified', number>; + complexity: DistributionRow[]; + changeTypes: DistributionRow[]; +}; + +type AnalyticsBreakdownBarsProps = { + impactBreakdown: ImpactBreakdown; + findingBreakdown: SeverityBreakdownRow[]; + securityBreakdown: SeverityBreakdownRow[]; +}; + +type BarColor = 'bg-chart-1' | 'bg-chart-2' | 'bg-chart-3' | 'bg-chart-4' | 'bg-chart-5'; + +type BarItem = { + key: string; + label: string; + count: number; + detail?: string; + color: BarColor; +}; + +const impactLabels = { + low: 'Low', + medium: 'Medium', + high: 'High', + unclassified: 'Unclassified (low confidence)', +} as const; + +const impactColors = { + low: 'bg-chart-2', + medium: 'bg-chart-3', + high: 'bg-chart-5', + unclassified: 'bg-chart-4', +} as const satisfies Record; + +const complexityLabels = { + low: 'Low', + medium: 'Medium', + high: 'High', +} as const satisfies Record; + +const changeTypeLabels = { + bug_fix: 'Bug fix', + feature: 'Feature', + refactor: 'Refactor', + maintenance: 'Maintenance', + dependency: 'Dependency', + test: 'Test', + documentation: 'Documentation', + mixed: 'Mixed', + other: 'Other', +} as const satisfies Record; + +const findingCategoryLabels = { + security: 'Security', + correctness: 'Correctness', + reliability: 'Reliability', + data_integrity: 'Data integrity', + performance: 'Performance', + compatibility: 'Compatibility', + maintainability: 'Maintainability', + test_quality: 'Test quality', + documentation: 'Documentation', + accessibility: 'Accessibility', + other: 'Other', +} as const satisfies Record; + +const securityClassLabels = { + auth_access: 'Authentication and access', + injection: 'Injection', + data_protection: 'Data protection', + request_resource_boundary: 'Request and resource boundaries', + deserialization_object_integrity: 'Deserialization and object integrity', + dependency_supply_chain: 'Dependency and supply chain', + memory_safety: 'Memory safety', + availability: 'Availability', + concurrency: 'Concurrency', + security_configuration: 'Security configuration', + other: 'Other', +} as const satisfies Record; + +const complexityOrder: ComplexityLevel[] = ['low', 'medium', 'high']; +const impactOrder: Array = [ + 'low', + 'medium', + 'high', + 'unclassified', +]; + +function lowConfidenceDetail(count: number): string | undefined { + if (count === 0) return undefined; + return `${count.toLocaleString()} low confidence`; +} + +function DistributionBarList({ items, label }: { items: BarItem[]; label: string }) { + const maxCount = Math.max(...items.map(item => item.count), 1); + + return ( +
+ {items.map(item => ( +
+
+
+ {item.label} + {item.detail && ( + {item.detail} + )} +
+ {item.count.toLocaleString()} +
+ + ))} +
+ ); +} + +function SeverityBarList({ + rows, + labels, + label, +}: { + rows: SeverityBreakdownRow[]; + labels: Record; + label: string; +}) { + const maxTotal = Math.max(...rows.map(row => row.total), 1); + + return ( +
+ {rows.map(row => ( +
+
+ {labels[row.value]} + {row.total.toLocaleString()} +
+ diff --git a/apps/web/src/components/code-reviews/analytics/CodeReviewAnalyticsPanel.tsx b/apps/web/src/components/code-reviews/analytics/CodeReviewAnalyticsPanel.tsx index 1314fb332a..5c998bf09a 100644 --- a/apps/web/src/components/code-reviews/analytics/CodeReviewAnalyticsPanel.tsx +++ b/apps/web/src/components/code-reviews/analytics/CodeReviewAnalyticsPanel.tsx @@ -35,7 +35,7 @@ import { AnalyticsBreakdownBars } from './AnalyticsBreakdownBars'; import { AnalyticsTables } from './AnalyticsTables'; type CodeReviewAnalyticsPanelProps = { - organizationId?: string; + organizationId: string; platform: 'github' | 'gitlab'; }; @@ -48,10 +48,7 @@ function formatCount(value: number): string { return value.toLocaleString(); } -function AnalyticsLoadingState({ - platform, - organizationId, -}: Pick) { +function AnalyticsLoadingState({ platform }: Pick) { const changeLabel = platform === 'github' ? 'Tracked PRs' : 'Tracked MRs'; return ( @@ -127,7 +124,7 @@ function AnalyticsLoadingState({ - {platform === 'github' && organizationId && ( + {platform === 'github' && ( @@ -363,7 +360,7 @@ export function CodeReviewAnalyticsPanel({ () => ({ platform, ...dateRange, - ...(organizationId ? { organizationId } : {}), + organizationId, ...(repository ? { repository } : {}), }), [dateRange, organizationId, platform, repository] @@ -420,7 +417,7 @@ export function CodeReviewAnalyticsPanel({ ); if (dashboardQuery.isPending && !dashboard) { - return ; + return ; } if (!dashboard) { @@ -467,7 +464,7 @@ export function CodeReviewAnalyticsPanel({ setEnabledMutation.mutate({ platform, enabled, - ...(organizationId ? { organizationId } : {}), + organizationId, }) } /> @@ -591,7 +588,6 @@ export function CodeReviewAnalyticsPanel({ diff --git a/apps/web/src/lib/code-reviews/analytics/db.test.ts b/apps/web/src/lib/code-reviews/analytics/db.test.ts index cf762405c8..c78869a395 100644 --- a/apps/web/src/lib/code-reviews/analytics/db.test.ts +++ b/apps/web/src/lib/code-reviews/analytics/db.test.ts @@ -4,9 +4,11 @@ import { code_review_analytics_findings, code_review_analytics_results, kilocode_users, + organizations, } from '@kilocode/db/schema'; import { eq } from 'drizzle-orm'; +import { createTestOrganization } from '@/tests/helpers/organization.helper'; import { insertTestUser } from '@/tests/helpers/user.helper'; import { createCodeReview, @@ -33,23 +35,34 @@ const capturedManifest: CodeReviewAnalyticsManifest = { describe('Code Reviewer analytics completion persistence', () => { let userId: string; + let organizationId: string; const reviewIds: string[] = []; beforeAll(async () => { userId = (await insertTestUser()).id; + organizationId = ( + await createTestOrganization( + `Review Analytics Persistence ${crypto.randomUUID()}`, + userId, + 0, + {}, + false + ) + ).id; }); afterAll(async () => { for (const reviewId of reviewIds) { await db.delete(cloud_agent_code_reviews).where(eq(cloud_agent_code_reviews.id, reviewId)); } + await db.delete(organizations).where(eq(organizations.id, organizationId)); await db.delete(kilocode_users).where(eq(kilocode_users.id, userId)); }); async function createRunningReview(analyticsEnabledAtDispatch: boolean) { const suffix = crypto.randomUUID(); const reviewId = await createCodeReview({ - owner: { type: 'user', id: userId, userId }, + owner: { type: 'org', id: organizationId, userId }, repoFullName: `analytics/repo-${suffix}`, prNumber: 1, prUrl: `https://github.com/analytics/repo-${suffix}/pull/1`, @@ -225,4 +238,43 @@ describe('Code Reviewer analytics completion persistence', () => { expect(staleResults).toEqual([]); expect(newerAttempt.attempt_number).toBe(2); }); + + it('rejects analytics persistence for personal reviews', async () => { + const suffix = crypto.randomUUID(); + const reviewId = await createCodeReview({ + owner: { type: 'user', id: userId, userId }, + repoFullName: `analytics/personal-${suffix}`, + prNumber: 1, + prUrl: `https://github.com/analytics/personal-${suffix}/pull/1`, + prTitle: 'Personal analytics test', + prAuthor: 'octocat', + prAuthorGithubId: '1234', + baseRef: 'main', + headRef: 'feature', + headSha: suffix, + platform: 'github', + }); + reviewIds.push(reviewId); + const attempt = await createCodeReviewAttempt({ + codeReviewId: reviewId, + status: 'running', + analyticsEnabledAtDispatch: true, + }); + await updateCodeReviewStatus(reviewId, 'running'); + + await expect( + finalizeCompletedCodeReviewWithAnalytics({ + codeReviewId: reviewId, + sourceAttemptId: attempt.id, + completedAt: new Date(), + capture: { status: 'captured', manifest: capturedManifest }, + }) + ).resolves.toEqual({ outcome: 'stale' }); + + const results = await db + .select() + .from(code_review_analytics_results) + .where(eq(code_review_analytics_results.code_review_id, reviewId)); + expect(results).toEqual([]); + }); }); diff --git a/apps/web/src/lib/code-reviews/analytics/db.ts b/apps/web/src/lib/code-reviews/analytics/db.ts index 072bea2af9..0ed68ee7b8 100644 --- a/apps/web/src/lib/code-reviews/analytics/db.ts +++ b/apps/web/src/lib/code-reviews/analytics/db.ts @@ -60,6 +60,7 @@ export async function finalizeCompletedCodeReviewWithAnalytics(input: { .limit(1); if ( + review.owned_by_organization_id === null || !attempt || (input.sourceAttemptId !== undefined && input.sourceAttemptId !== attempt.id) || (input.sessionId !== undefined && @@ -294,10 +295,7 @@ export type CodeReviewAnalyticsDashboard = { securityBreakdown: CodeReviewAnalyticsSeverityBreakdownRow[]; repositories: CodeReviewAnalyticsRepositoryRow[]; contributors: { - capability: - | 'available' - | 'organization_scope_required' - | 'stable_gitlab_author_attribution_unavailable'; + capability: 'available' | 'stable_gitlab_author_attribution_unavailable'; rows: CodeReviewAnalyticsContributorRow[]; }; }; @@ -322,10 +320,7 @@ function numberValue(value: unknown): number { } function analyticsBaseCte(input: DashboardInput, applyRepositoryFilter: boolean) { - const ownerCondition = - input.owner.type === 'org' - ? sql`${cloud_agent_code_reviews.owned_by_organization_id} = ${input.owner.id}` - : sql`${cloud_agent_code_reviews.owned_by_user_id} = ${input.owner.id}`; + const ownerCondition = sql`${cloud_agent_code_reviews.owned_by_organization_id} = ${input.owner.id}`; const repositoryCondition = applyRepositoryFilter && input.repository ? sql`AND ${cloud_agent_code_reviews.repo_full_name} = ${input.repository}` @@ -408,18 +403,11 @@ function analyticsBaseCte(input: DashboardInput, applyRepositoryFilter: boolean) export async function getCodeReviewAnalyticsDashboard( input: DashboardInput ): Promise { - const ownerCondition = - input.owner.type === 'org' - ? and( - eq(agent_configs.owned_by_organization_id, input.owner.id), - eq(agent_configs.agent_type, 'code_review'), - eq(agent_configs.platform, input.platform) - ) - : and( - eq(agent_configs.owned_by_user_id, input.owner.id), - eq(agent_configs.agent_type, 'code_review'), - eq(agent_configs.platform, input.platform) - ); + const ownerCondition = and( + eq(agent_configs.owned_by_organization_id, input.owner.id), + eq(agent_configs.agent_type, 'code_review'), + eq(agent_configs.platform, input.platform) + ); const [config] = await input.db .select({ config: agent_configs.config }) .from(agent_configs) @@ -597,11 +585,7 @@ export async function getCodeReviewAnalyticsDashboard( let contributorRows: CodeReviewAnalyticsContributorRow[] = []; const contributorCapability = - input.owner.type !== 'org' - ? 'organization_scope_required' - : input.platform !== 'github' - ? 'stable_gitlab_author_attribution_unavailable' - : 'available'; + input.platform === 'github' ? 'available' : 'stable_gitlab_author_attribution_unavailable'; if (contributorCapability === 'available') { const contributorResult = await input.db.execute<{ diff --git a/apps/web/src/lib/code-reviews/analytics/settings.test.ts b/apps/web/src/lib/code-reviews/analytics/settings.test.ts index b09e75a0a5..bac036e3b7 100644 --- a/apps/web/src/lib/code-reviews/analytics/settings.test.ts +++ b/apps/web/src/lib/code-reviews/analytics/settings.test.ts @@ -44,14 +44,6 @@ describe('Code Reviewer analytics settings', () => { }); afterEach(async () => { - await db - .delete(agent_configs) - .where( - and( - eq(agent_configs.agent_type, 'code_review'), - eq(agent_configs.owned_by_user_id, userId) - ) - ); await db .delete(agent_configs) .where( @@ -68,7 +60,7 @@ describe('Code Reviewer analytics settings', () => { }); it('inserts a complete main-disabled Code Reviewer config for a missing row', async () => { - const owner = { type: 'user' as const, id: userId }; + const owner = { type: 'org' as const, id: organizationId }; await expect(isReviewAnalyticsEnabled({ owner, platform: 'github' })).resolves.toBe(false); await expect( @@ -84,7 +76,7 @@ describe('Code Reviewer analytics settings', () => { where: and( eq(agent_configs.agent_type, 'code_review'), eq(agent_configs.platform, 'github'), - eq(agent_configs.owned_by_user_id, userId) + eq(agent_configs.owned_by_organization_id, organizationId) ), }); @@ -106,9 +98,9 @@ describe('Code Reviewer analytics settings', () => { }); it('atomically updates only analytics state on an existing row', async () => { - const owner = { type: 'user' as const, id: userId }; + const owner = { type: 'org' as const, id: organizationId }; await db.insert(agent_configs).values({ - owned_by_user_id: userId, + owned_by_organization_id: organizationId, agent_type: 'code_review', platform: 'github', config: { @@ -140,7 +132,7 @@ describe('Code Reviewer analytics settings', () => { where: and( eq(agent_configs.agent_type, 'code_review'), eq(agent_configs.platform, 'github'), - eq(agent_configs.owned_by_user_id, userId) + eq(agent_configs.owned_by_organization_id, organizationId) ), }); @@ -154,7 +146,6 @@ describe('Code Reviewer analytics settings', () => { it('looks up analytics state by owner and platform', async () => { const organizationOwner = { type: 'org' as const, id: organizationId }; - const personalOwner = { type: 'user' as const, id: userId }; await setReviewAnalyticsEnabled({ owner: organizationOwner, @@ -169,9 +160,6 @@ describe('Code Reviewer analytics settings', () => { await expect( isReviewAnalyticsEnabled({ owner: organizationOwner, platform: 'github' }) ).resolves.toBe(false); - await expect( - isReviewAnalyticsEnabled({ owner: personalOwner, platform: 'gitlab' }) - ).resolves.toBe(false); }); }); }); diff --git a/apps/web/src/lib/code-reviews/analytics/settings.ts b/apps/web/src/lib/code-reviews/analytics/settings.ts index fd4a645fe1..fe4dd284ae 100644 --- a/apps/web/src/lib/code-reviews/analytics/settings.ts +++ b/apps/web/src/lib/code-reviews/analytics/settings.ts @@ -6,7 +6,7 @@ import type { CodeReviewAgentConfig } from '@/lib/agent-config/core/types'; import { db } from '@/lib/drizzle'; import { agent_configs } from '@kilocode/db/schema'; -export type ReviewAnalyticsOwner = { type: 'org'; id: string } | { type: 'user'; id: string }; +export type ReviewAnalyticsOwner = { type: 'org'; id: string }; export type ReviewAnalyticsPlatform = 'github' | 'gitlab'; const ReviewAnalyticsSettingsSchema = z.object({ @@ -50,36 +50,23 @@ export async function setReviewAnalyticsEnabled(input: { ${JSON.stringify(input.enabled)}::jsonb, true )`; - const values = - input.owner.type === 'org' - ? { - owned_by_organization_id: input.owner.id, - owned_by_user_id: null, - agent_type: 'code_review', - platform: input.platform, - config, - is_enabled: false, - created_by: input.createdBy, - } - : { - owned_by_organization_id: null, - owned_by_user_id: input.owner.id, - agent_type: 'code_review', - platform: input.platform, - config, - is_enabled: false, - created_by: input.createdBy, - }; - const target = - input.owner.type === 'org' - ? [agent_configs.owned_by_organization_id, agent_configs.agent_type, agent_configs.platform] - : [agent_configs.owned_by_user_id, agent_configs.agent_type, agent_configs.platform]; - const [saved] = await db .insert(agent_configs) - .values(values) + .values({ + owned_by_organization_id: input.owner.id, + owned_by_user_id: null, + agent_type: 'code_review', + platform: input.platform, + config, + is_enabled: false, + created_by: input.createdBy, + }) .onConflictDoUpdate({ - target, + target: [ + agent_configs.owned_by_organization_id, + agent_configs.agent_type, + agent_configs.platform, + ], set: { config: updatedConfig, updated_at: new Date().toISOString(), @@ -117,21 +104,16 @@ async function getReviewAnalyticsConfigRow(input: { owner: ReviewAnalyticsOwner; platform: ReviewAnalyticsPlatform; }) { - const conditions = [ - eq(agent_configs.agent_type, 'code_review'), - eq(agent_configs.platform, input.platform), - ]; - - if (input.owner.type === 'org') { - conditions.push(eq(agent_configs.owned_by_organization_id, input.owner.id)); - } else { - conditions.push(eq(agent_configs.owned_by_user_id, input.owner.id)); - } - const [config] = await db .select() .from(agent_configs) - .where(and(...conditions)) + .where( + and( + eq(agent_configs.agent_type, 'code_review'), + eq(agent_configs.platform, input.platform), + eq(agent_configs.owned_by_organization_id, input.owner.id) + ) + ) .limit(1); return config ?? null; diff --git a/apps/web/src/lib/code-reviews/dispatch/dispatch-pending-reviews.test.ts b/apps/web/src/lib/code-reviews/dispatch/dispatch-pending-reviews.test.ts index 1051161027..ce12cede7a 100644 --- a/apps/web/src/lib/code-reviews/dispatch/dispatch-pending-reviews.test.ts +++ b/apps/web/src/lib/code-reviews/dispatch/dispatch-pending-reviews.test.ts @@ -1341,8 +1341,7 @@ describe('tryDispatchPendingReviews', () => { it('snapshots analytics enrollment and appends the protocol only when enabled', async () => { const timestamp = minutesAgo(1); - const owner = { type: 'user', id: testUser.id } satisfies ReviewOwner; - await setTestUserBalance(DEFAULT_TIER_BALANCE_MICRODOLLARS); + const owner = { type: 'org', id: testOrganizationId } satisfies ReviewOwner; mockGetAgentConfigForOwner.mockResolvedValue({ id: 'test-agent-config', config: { review_analytics_enabled: true }, @@ -1363,8 +1362,8 @@ describe('tryDispatchPendingReviews', () => { .returning({ id: cloud_agent_code_reviews.id }); await tryDispatchPendingReviews({ - type: 'user', - id: testUser.id, + type: 'org', + id: testOrganizationId, userId: testUser.id, }); @@ -1381,10 +1380,16 @@ describe('tryDispatchPendingReviews', () => { ); }); - it('dispatches the ordinary prompt with an explicit disabled snapshot', async () => { + it('ignores enabled analytics config for personal reviews', async () => { const timestamp = minutesAgo(1); const owner = { type: 'user', id: testUser.id } satisfies ReviewOwner; await setTestUserBalance(DEFAULT_TIER_BALANCE_MICRODOLLARS); + mockGetAgentConfigForOwner.mockResolvedValue({ + id: 'test-agent-config', + config: { review_analytics_enabled: true }, + is_enabled: true, + runtime_state: {}, + }); const [review] = await db .insert(cloud_agent_code_reviews) @@ -1414,7 +1419,7 @@ describe('tryDispatchPendingReviews', () => { expect(dispatchedPayload.sessionInput.prompt).toBe('Review this change.'); }); - it('keeps an existing enabled snapshot after collection is disabled', async () => { + it('ignores a legacy enabled analytics snapshot for personal reviews', async () => { const timestamp = minutesAgo(1); const owner = { type: 'user', id: testUser.id } satisfies ReviewOwner; await setTestUserBalance(DEFAULT_TIER_BALANCE_MICRODOLLARS); @@ -1443,6 +1448,38 @@ describe('tryDispatchPendingReviews', () => { userId: testUser.id, }); + const dispatchedPayload = mockDispatchReview.mock.calls[0]?.[0]; + expect(dispatchedPayload.sessionInput.prompt).toBe('Review this change.'); + }); + + it('keeps an existing organization analytics snapshot after collection is disabled', async () => { + const timestamp = minutesAgo(1); + const owner = { type: 'org', id: testOrganizationId } satisfies ReviewOwner; + + const [review] = await db + .insert(cloud_agent_code_reviews) + .values( + reviewValues({ + owner, + status: 'pending', + createdAt: timestamp, + updatedAt: timestamp, + }) + ) + .returning({ id: cloud_agent_code_reviews.id }); + await db.insert(cloud_agent_code_review_attempts).values({ + code_review_id: review.id, + attempt_number: 1, + status: 'pending', + analytics_enabled_at_dispatch: true, + }); + + await tryDispatchPendingReviews({ + type: 'org', + id: testOrganizationId, + userId: testUser.id, + }); + const dispatchedPayload = mockDispatchReview.mock.calls[0]?.[0]; expect(dispatchedPayload.sessionInput.prompt).toContain('kilo-review-analytics:v1'); }); diff --git a/apps/web/src/lib/code-reviews/dispatch/dispatch-pending-reviews.ts b/apps/web/src/lib/code-reviews/dispatch/dispatch-pending-reviews.ts index 3b34594000..229e2e84bb 100644 --- a/apps/web/src/lib/code-reviews/dispatch/dispatch-pending-reviews.ts +++ b/apps/web/src/lib/code-reviews/dispatch/dispatch-pending-reviews.ts @@ -484,9 +484,14 @@ async function dispatchReservedReview(reservation: ReservedReview, owner: Owner) agentConfig, platform, }); - const analyticsPrompt = appendCodeReviewAnalyticsPromptAppendix(payload.sessionInput.prompt); + const analyticsPrompt = + owner.type === 'org' + ? appendCodeReviewAnalyticsPromptAppendix(payload.sessionInput.prompt) + : null; const shouldEnrollAnalytics = - getReviewAnalyticsEnabledFromConfig(agentConfig.config) && analyticsPrompt !== null; + owner.type === 'org' && + getReviewAnalyticsEnabledFromConfig(agentConfig.config) && + analyticsPrompt !== null; if (!(await reviewIsStillReserved(review.id, dispatchReservationId))) { logExceptInTest('[dispatchReview] Review reservation changed after preparation', { @@ -497,7 +502,8 @@ async function dispatchReservedReview(reservation: ReservedReview, owner: Owner) const agentVersion = 'v2'; const attempt = await ensureCurrentCodeReviewAttemptFromReview(review, shouldEnrollAnalytics); - const analyticsEnabledAtDispatch = attempt.analytics_enabled_at_dispatch === true; + const analyticsEnabledAtDispatch = + owner.type === 'org' && attempt.analytics_enabled_at_dispatch === true; let dispatchPayload = payload; if (analyticsEnabledAtDispatch) { diff --git a/apps/web/src/routers/code-reviews/code-review-analytics-router.test.ts b/apps/web/src/routers/code-reviews/code-review-analytics-router.test.ts index bbf061a260..3606e701f0 100644 --- a/apps/web/src/routers/code-reviews/code-review-analytics-router.test.ts +++ b/apps/web/src/routers/code-reviews/code-review-analytics-router.test.ts @@ -223,6 +223,25 @@ describe('Code Reviewer analytics router', () => { ]); }); + it('requires organization scope for reads and settings changes', async () => { + const caller = await createCallerForUser(ownerId); + const now = Date.now(); + + await expect( + caller.codeReviews.analytics.getDashboard({ + platform: 'github', + startDate: new Date(now - 60_000).toISOString(), + endDate: new Date(now + 60_000).toISOString(), + } as never) + ).rejects.toMatchObject({ code: 'BAD_REQUEST' }); + await expect( + caller.codeReviews.analytics.setEnabled({ + platform: 'github', + enabled: true, + } as never) + ).rejects.toMatchObject({ code: 'BAD_REQUEST' }); + }); + it('allows owner toggles while rejecting member mutation and non-member reads', async () => { const memberCaller = await createCallerForUser(memberId); const ownerCaller = await createCallerForUser(ownerId); diff --git a/apps/web/src/routers/code-reviews/code-review-analytics-router.ts b/apps/web/src/routers/code-reviews/code-review-analytics-router.ts index 7b3ec953a3..4b916cfcc0 100644 --- a/apps/web/src/routers/code-reviews/code-review-analytics-router.ts +++ b/apps/web/src/routers/code-reviews/code-review-analytics-router.ts @@ -1,10 +1,7 @@ import * as z from 'zod'; import { getCodeReviewAnalyticsDashboard } from '@/lib/code-reviews/analytics/db'; -import { - setReviewAnalyticsEnabled, - type ReviewAnalyticsOwner, -} from '@/lib/code-reviews/analytics/settings'; +import { setReviewAnalyticsEnabled } from '@/lib/code-reviews/analytics/settings'; import { readDb } from '@/lib/drizzle'; import { createTRPCRouter, baseProcedure } from '@/lib/trpc/init'; import { timedUsageQuery } from '@/lib/usage-query'; @@ -16,7 +13,7 @@ const PlatformSchema = z.enum(['github', 'gitlab']); const GetDashboardInputSchema = z .object({ - organizationId: z.uuid().optional(), + organizationId: z.uuid(), platform: PlatformSchema, startDate: z.string().datetime(), endDate: z.string().datetime(), @@ -37,31 +34,23 @@ const GetDashboardInputSchema = z ); const SetEnabledInputSchema = z.object({ - organizationId: z.uuid().optional(), + organizationId: z.uuid(), platform: PlatformSchema, enabled: z.boolean(), }); export const codeReviewAnalyticsRouter = createTRPCRouter({ getDashboard: baseProcedure.input(GetDashboardInputSchema).query(async ({ ctx, input }) => { - let owner: ReviewAnalyticsOwner; - let canManage: boolean; - - if (input.organizationId) { - const role = await ensureOrganizationAccess(ctx, input.organizationId); - owner = { type: 'org', id: input.organizationId }; - canManage = role === 'owner' || role === 'billing_manager'; - } else { - owner = { type: 'user', id: ctx.user.id }; - canManage = true; - } + const role = await ensureOrganizationAccess(ctx, input.organizationId); + const owner = { type: 'org' as const, id: input.organizationId }; + const canManage = role === 'owner' || role === 'billing_manager'; return timedUsageQuery( { db: readDb, route: 'codeReviews.analytics.getDashboard', queryLabel: 'code_review_analytics_dashboard', - scope: owner.type === 'org' ? 'org' : 'user', + scope: 'org', period: `${input.startDate}/${input.endDate}`, }, tx => @@ -78,16 +67,10 @@ export const codeReviewAnalyticsRouter = createTRPCRouter({ }), setEnabled: baseProcedure.input(SetEnabledInputSchema).mutation(async ({ ctx, input }) => { - const owner: ReviewAnalyticsOwner = input.organizationId - ? { type: 'org', id: input.organizationId } - : { type: 'user', id: ctx.user.id }; - - if (input.organizationId) { - await ensureOrganizationAccess(ctx, input.organizationId, ['owner', 'billing_manager']); - } + await ensureOrganizationAccess(ctx, input.organizationId, ['owner', 'billing_manager']); const enabled = await setReviewAnalyticsEnabled({ - owner, + owner: { type: 'org', id: input.organizationId }, platform: input.platform, enabled: input.enabled, createdBy: ctx.user.id, From a0ff158f42ba573102f3490f3bd5b38acd61c881 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Thu, 18 Jun 2026 11:28:20 +0300 Subject: [PATCH 3/5] fix(code-review): track PRs without critical findings --- .../analytics/AnalyticsTables.tsx | 50 +++++++++---------- apps/web/src/lib/code-reviews/analytics/db.ts | 17 ++----- .../code-review-analytics-router.test.ts | 13 +++-- 3 files changed, 36 insertions(+), 44 deletions(-) diff --git a/apps/web/src/components/code-reviews/analytics/AnalyticsTables.tsx b/apps/web/src/components/code-reviews/analytics/AnalyticsTables.tsx index 563402d57d..10693210d4 100644 --- a/apps/web/src/components/code-reviews/analytics/AnalyticsTables.tsx +++ b/apps/web/src/components/code-reviews/analytics/AnalyticsTables.tsx @@ -30,14 +30,13 @@ type ContributorRow = { displayName: string; limitedIdentity: boolean; limitedData: boolean; - rank: number | null; trackedPrs: number; estimatedImpactPoints: number; highImpactPrs: number; criticalFindings: number; warningFindings: number; suggestionFindings: number; - prsWithoutCriticalOrWarningFindings: number; + prsWithoutCriticalFindings: number; }; type ContributorCapability = 'available' | 'stable_gitlab_author_attribution_unavailable'; @@ -62,13 +61,13 @@ type RepositorySortKey = | 'suggestionFindings'; type ContributorSortKey = | 'displayName' + | 'prsWithoutCriticalFindings' | 'trackedPrs' | 'estimatedImpactPoints' | 'highImpactPrs' | 'criticalFindings' | 'warningFindings' - | 'suggestionFindings' - | 'prsWithoutCriticalOrWarningFindings'; + | 'suggestionFindings'; type SortableTableHeadProps = { label: string; @@ -318,6 +317,13 @@ function ContributorLeaderboard({ rows }: { rows: ContributorRow[] }) { case 'displayName': result = compareText(left.displayName, right.displayName, sort.direction); break; + case 'prsWithoutCriticalFindings': + result = compareNumbers( + left.prsWithoutCriticalFindings, + right.prsWithoutCriticalFindings, + sort.direction + ); + break; case 'trackedPrs': result = compareNumbers(left.trackedPrs, right.trackedPrs, sort.direction); break; @@ -344,13 +350,6 @@ function ContributorLeaderboard({ rows }: { rows: ContributorRow[] }) { sort.direction ); break; - case 'prsWithoutCriticalOrWarningFindings': - result = compareNumbers( - left.prsWithoutCriticalOrWarningFindings, - right.prsWithoutCriticalOrWarningFindings, - sort.direction - ); - break; } return result || left.contributorKey.localeCompare(right.contributorKey); }); @@ -387,13 +386,20 @@ function ContributorLeaderboard({ rows }: { rows: ContributorRow[] }) { - Rank selectSort('displayName')} /> + selectSort('prsWithoutCriticalFindings')} + align="right" + /> selectSort('suggestionFindings')} align="right" /> - selectSort('prsWithoutCriticalOrWarningFindings')} - align="right" - /> {sortedRows.length === 0 ? ( - + No contributor analytics for this selection. ) : ( sortedRows.map(row => ( - - {row.rank === null ? Limited data : row.rank} -
{row.displayName || 'Unknown author'} + {row.limitedData && Limited data} {row.limitedIdentity && Limited identity}
+ + {row.prsWithoutCriticalFindings.toLocaleString()} + {row.trackedPrs.toLocaleString()} @@ -484,9 +483,6 @@ function ContributorLeaderboard({ rows }: { rows: ContributorRow[] }) { {row.suggestionFindings.toLocaleString()} - - {row.prsWithoutCriticalOrWarningFindings.toLocaleString()} - )) )} diff --git a/apps/web/src/lib/code-reviews/analytics/db.ts b/apps/web/src/lib/code-reviews/analytics/db.ts index 0ed68ee7b8..e6336f28c3 100644 --- a/apps/web/src/lib/code-reviews/analytics/db.ts +++ b/apps/web/src/lib/code-reviews/analytics/db.ts @@ -267,14 +267,13 @@ export type CodeReviewAnalyticsContributorRow = { displayName: string; limitedIdentity: boolean; limitedData: boolean; - rank: number | null; trackedPrs: number; estimatedImpactPoints: number; highImpactPrs: number; criticalFindings: number; warningFindings: number; suggestionFindings: number; - prsWithoutCriticalOrWarningFindings: number; + prsWithoutCriticalFindings: number; }; export type CodeReviewAnalyticsDashboard = { @@ -598,7 +597,7 @@ export async function getCodeReviewAnalyticsDashboard( critical_findings: number | string; warning_findings: number | string; suggestion_findings: number | string; - prs_without_critical_or_warning_findings: number | string; + prs_without_critical_findings: number | string; }>(sql` ${analyticsBaseCte(input, true)}, logical_findings AS ( @@ -647,7 +646,7 @@ export async function getCodeReviewAnalyticsDashboard( COALESCE(SUM(critical_findings), 0) AS critical_findings, COALESCE(SUM(warning_findings), 0) AS warning_findings, COALESCE(SUM(suggestion_findings), 0) AS suggestion_findings, - COUNT(*) FILTER (WHERE critical_findings = 0 AND warning_findings = 0) AS prs_without_critical_or_warning_findings + COUNT(*) FILTER (WHERE critical_findings = 0) AS prs_without_critical_findings FROM contributor_prs GROUP BY contributor_key ORDER BY @@ -659,26 +658,20 @@ export async function getCodeReviewAnalyticsDashboard( LIMIT 50 `); - let rank = 0; contributorRows = contributorResult.rows.map(row => { const trackedPrs = numberValue(row.tracked_prs); - const limitedData = trackedPrs < 5; - if (!limitedData) rank += 1; return { contributorKey: row.contributor_key, displayName: row.display_name, limitedIdentity: row.limited_identity, - limitedData, - rank: limitedData ? null : rank, + limitedData: trackedPrs < 5, trackedPrs, estimatedImpactPoints: numberValue(row.estimated_impact_points), highImpactPrs: numberValue(row.high_impact_prs), criticalFindings: numberValue(row.critical_findings), warningFindings: numberValue(row.warning_findings), suggestionFindings: numberValue(row.suggestion_findings), - prsWithoutCriticalOrWarningFindings: numberValue( - row.prs_without_critical_or_warning_findings - ), + prsWithoutCriticalFindings: numberValue(row.prs_without_critical_findings), }; }); } diff --git a/apps/web/src/routers/code-reviews/code-review-analytics-router.test.ts b/apps/web/src/routers/code-reviews/code-review-analytics-router.test.ts index 3606e701f0..e7cd23f72c 100644 --- a/apps/web/src/routers/code-reviews/code-review-analytics-router.test.ts +++ b/apps/web/src/routers/code-reviews/code-review-analytics-router.test.ts @@ -137,7 +137,11 @@ describe('Code Reviewer analytics router', () => { repository, prNumber: 2, headSha: crypto.randomUUID(), - analyticsManifest: manifest({ type: 'feature', impact: 'high' }), + analyticsManifest: manifest({ + type: 'feature', + impact: 'high', + findings: [{ severity: 'suggestion', category: 'performance', securityClass: null }], + }), }); await captureReview({ repository, @@ -187,7 +191,7 @@ describe('Code Reviewer analytics router', () => { expect(dashboard.summary).toEqual({ trackedReviews: 6, trackedPrsOrMrs: 5, - totalFindings: 4, + totalFindings: 5, criticalFindings: 1, warningFindings: 2, highImpactChanges: 2, @@ -207,18 +211,17 @@ describe('Code Reviewer analytics router', () => { estimatedImpactPoints: 9, criticalFindings: 1, warningFindings: 2, - suggestionFindings: 1, + suggestionFindings: 2, }), ]); expect(dashboard.contributors.capability).toBe('available'); expect(dashboard.contributors.rows).toEqual([ expect.objectContaining({ contributorKey: 'github-id:1234', - rank: 1, limitedData: false, trackedPrs: 5, estimatedImpactPoints: 9, - prsWithoutCriticalOrWarningFindings: 3, + prsWithoutCriticalFindings: 4, }), ]); }); From 12cf0a684640f7a4d8536f607aa0357ba3139120 Mon Sep 17 00:00:00 2001 From: Alex Alecu Date: Thu, 18 Jun 2026 11:52:44 +0300 Subject: [PATCH 4/5] feat(code-review): make analytics breakdowns easier to scan --- .../AnalyticsBreakdownBars.stories.tsx | 103 ++++++++ .../analytics/AnalyticsBreakdownBars.tsx | 236 +++++++++++------- .../analytics/CodeReviewAnalyticsPanel.tsx | 63 +++-- 3 files changed, 293 insertions(+), 109 deletions(-) create mode 100644 apps/storybook/stories/code-reviews/AnalyticsBreakdownBars.stories.tsx diff --git a/apps/storybook/stories/code-reviews/AnalyticsBreakdownBars.stories.tsx b/apps/storybook/stories/code-reviews/AnalyticsBreakdownBars.stories.tsx new file mode 100644 index 0000000000..c7c47a6925 --- /dev/null +++ b/apps/storybook/stories/code-reviews/AnalyticsBreakdownBars.stories.tsx @@ -0,0 +1,103 @@ +import type { ComponentProps } from 'react'; +import type { Meta, StoryObj } from '@storybook/nextjs'; + +import { AnalyticsBreakdownBars } from '@/components/code-reviews/analytics/AnalyticsBreakdownBars'; + +type AnalyticsBreakdownBarsArgs = ComponentProps; + +const populatedArgs = { + impactBreakdown: { + impact: { + low: 5, + medium: 11, + high: 6, + unclassified: 2, + }, + complexity: [ + { value: 'high', count: 6, lowConfidenceCount: 1 }, + { value: 'low', count: 8, lowConfidenceCount: 0 }, + { value: 'medium', count: 10, lowConfidenceCount: 1 }, + ], + changeTypes: [ + { value: 'feature', count: 5, lowConfidenceCount: 1 }, + { value: 'documentation', count: 1, lowConfidenceCount: 0 }, + { value: 'bug_fix', count: 5, lowConfidenceCount: 0 }, + { value: 'other', count: 1, lowConfidenceCount: 0 }, + { value: 'refactor', count: 4, lowConfidenceCount: 0 }, + { value: 'test', count: 2, lowConfidenceCount: 0 }, + { value: 'maintenance', count: 3, lowConfidenceCount: 1 }, + { value: 'mixed', count: 1, lowConfidenceCount: 0 }, + { value: 'dependency', count: 2, lowConfidenceCount: 0 }, + ], + }, + findingBreakdown: [ + { value: 'correctness', total: 10, critical: 1, warning: 6, suggestion: 3 }, + { value: 'security', total: 6, critical: 2, warning: 3, suggestion: 1 }, + { value: 'reliability', total: 5, critical: 0, warning: 4, suggestion: 1 }, + { value: 'maintainability', total: 4, critical: 0, warning: 1, suggestion: 3 }, + { value: 'performance', total: 3, critical: 0, warning: 2, suggestion: 1 }, + { value: 'test_quality', total: 2, critical: 0, warning: 0, suggestion: 2 }, + { value: 'data_integrity', total: 1, critical: 1, warning: 0, suggestion: 0 }, + ], + securityBreakdown: [ + { value: 'auth_access', total: 2, critical: 1, warning: 1, suggestion: 0 }, + { value: 'injection', total: 2, critical: 1, warning: 1, suggestion: 0 }, + { + value: 'dependency_supply_chain', + total: 1, + critical: 0, + warning: 0, + suggestion: 1, + }, + { + value: 'request_resource_boundary', + total: 1, + critical: 0, + warning: 1, + suggestion: 0, + }, + ], +} satisfies AnalyticsBreakdownBarsArgs; + +const emptyOptionalDataArgs = { + impactBreakdown: { + impact: { + low: 0, + medium: 0, + high: 0, + unclassified: 0, + }, + complexity: [], + changeTypes: [], + }, + findingBreakdown: [], + securityBreakdown: [], +} satisfies AnalyticsBreakdownBarsArgs; + +const meta = { + title: 'Code Reviews/Analytics/AnalyticsBreakdownBars', + component: AnalyticsBreakdownBars, + parameters: { + layout: 'fullscreen', + }, + decorators: [ + Story => ( +
+
+ +
+
+ ), + ], +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const Populated: Story = { + args: populatedArgs, +}; + +export const EmptyOptionalData: Story = { + args: emptyOptionalDataArgs, +}; diff --git a/apps/web/src/components/code-reviews/analytics/AnalyticsBreakdownBars.tsx b/apps/web/src/components/code-reviews/analytics/AnalyticsBreakdownBars.tsx index 14c04f431e..141003712f 100644 --- a/apps/web/src/components/code-reviews/analytics/AnalyticsBreakdownBars.tsx +++ b/apps/web/src/components/code-reviews/analytics/AnalyticsBreakdownBars.tsx @@ -1,5 +1,8 @@ 'use client'; +import type { ReactNode } from 'react'; +import { useId } from 'react'; + import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/components/ui/card'; import { cn } from '@/lib/utils'; @@ -155,9 +158,9 @@ function DistributionBarList({ items, label }: { items: BarItem[]; label: string const maxCount = Math.max(...items.map(item => item.count), 1); return ( -
+
{items.map(item => ( -
+
{item.label} @@ -191,38 +194,80 @@ function SeverityBarList({ const maxTotal = Math.max(...rows.map(row => row.total), 1); return ( -
- {rows.map(row => ( -
-
- {labels[row.value]} - {row.total.toLocaleString()} -
-