diff --git a/CONTEXT.md b/CONTEXT.md index d3ecee3387..80f8c8a00e 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 organization- 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** | Organization-only, 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 available only to organization-owned reviews and 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 organization-only, 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/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/app/(app)/code-reviews/ReviewAgentPageClient.tsx b/apps/web/src/app/(app)/code-reviews/ReviewAgentPageClient.tsx index ad5934cc4c..432aaba5ed 100644 --- a/apps/web/src/app/(app)/code-reviews/ReviewAgentPageClient.tsx +++ b/apps/web/src/app/(app)/code-reviews/ReviewAgentPageClient.tsx @@ -11,7 +11,7 @@ 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, ExternalLink, ListChecks, Rocket, Settings2 } from 'lucide-react'; import { useTRPC } from '@/lib/trpc/utils'; import { useQuery } from '@tanstack/react-query'; import Link from 'next/link'; 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..141003712f --- /dev/null +++ b/apps/web/src/components/code-reviews/analytics/AnalyticsBreakdownBars.tsx @@ -0,0 +1,417 @@ +'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'; + +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 => { + const severityCounts = [ + { label: 'Critical', count: row.critical }, + { label: 'Warning', count: row.warning }, + { label: 'Suggestion', count: row.suggestion }, + ]; + const visibleSummary = severityCounts + .filter(severity => severity.count > 0) + .map(severity => `${severity.label} ${severity.count.toLocaleString()}`) + .join(' / '); + const accessibleSummary = severityCounts + .map(severity => `${severity.label} ${severity.count.toLocaleString()}`) + .join(', '); + + return ( +
+
+ {labels[row.value]} + {row.total.toLocaleString()} +
+