diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 40bd68f3aa..ee49dd2bb1 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2658,6 +2658,14 @@ func (q *querier) GetChatDiffStatusByChatID(ctx context.Context, chatID uuid.UUI return q.db.GetChatDiffStatusByChatID(ctx, chatID) } +func (q *querier) GetChatDiffStatusSummary(ctx context.Context) (database.GetChatDiffStatusSummaryRow, error) { + // Telemetry queries are called from system contexts only. + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil { + return database.GetChatDiffStatusSummaryRow{}, err + } + return q.db.GetChatDiffStatusSummary(ctx) +} + func (q *querier) GetChatDiffStatusesByChatIDs(ctx context.Context, chatIDs []uuid.UUID) ([]database.ChatDiffStatus, error) { if len(chatIDs) == 0 { return []database.ChatDiffStatus{}, nil diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index c4f20eddfc..8261a7ea6e 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -4192,6 +4192,10 @@ func (s *MethodTestSuite) TestSystemFunctions() { dbm.EXPECT().GetChatMessageSummariesPerChat(gomock.Any(), ts).Return([]database.GetChatMessageSummariesPerChatRow{}, nil).AnyTimes() check.Args(ts).Asserts(rbac.ResourceSystem, policy.ActionRead) })) + s.Run("GetChatDiffStatusSummary", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { + dbm.EXPECT().GetChatDiffStatusSummary(gomock.Any()).Return(database.GetChatDiffStatusSummaryRow{}, nil).AnyTimes() + check.Args().Asserts(rbac.ResourceSystem, policy.ActionRead) + })) s.Run("GetChatModelConfigsForTelemetry", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { dbm.EXPECT().GetChatModelConfigsForTelemetry(gomock.Any()).Return([]database.GetChatModelConfigsForTelemetryRow{}, nil).AnyTimes() check.Args().Asserts(rbac.ResourceSystem, policy.ActionRead) diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 2b05756d14..97752dd59d 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -1200,6 +1200,14 @@ func (m queryMetricsStore) GetChatDiffStatusByChatID(ctx context.Context, chatID return r0, r1 } +func (m queryMetricsStore) GetChatDiffStatusSummary(ctx context.Context) (database.GetChatDiffStatusSummaryRow, error) { + start := time.Now() + r0, r1 := m.s.GetChatDiffStatusSummary(ctx) + m.queryLatencies.WithLabelValues("GetChatDiffStatusSummary").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "GetChatDiffStatusSummary").Inc() + return r0, r1 +} + func (m queryMetricsStore) GetChatDiffStatusesByChatIDs(ctx context.Context, chatIDs []uuid.UUID) ([]database.ChatDiffStatus, error) { start := time.Now() r0, r1 := m.s.GetChatDiffStatusesByChatIDs(ctx, chatIDs) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index bda333e6af..8848d7c58f 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -2207,6 +2207,21 @@ func (mr *MockStoreMockRecorder) GetChatDiffStatusByChatID(ctx, chatID any) *gom return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChatDiffStatusByChatID", reflect.TypeOf((*MockStore)(nil).GetChatDiffStatusByChatID), ctx, chatID) } +// GetChatDiffStatusSummary mocks base method. +func (m *MockStore) GetChatDiffStatusSummary(ctx context.Context) (database.GetChatDiffStatusSummaryRow, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetChatDiffStatusSummary", ctx) + ret0, _ := ret[0].(database.GetChatDiffStatusSummaryRow) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetChatDiffStatusSummary indicates an expected call of GetChatDiffStatusSummary. +func (mr *MockStoreMockRecorder) GetChatDiffStatusSummary(ctx any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChatDiffStatusSummary", reflect.TypeOf((*MockStore)(nil).GetChatDiffStatusSummary), ctx) +} + // GetChatDiffStatusesByChatIDs mocks base method. func (m *MockStore) GetChatDiffStatusesByChatIDs(ctx context.Context, chatIds []uuid.UUID) ([]database.ChatDiffStatus, error) { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 1fef8ae578..bcf0031857 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -281,6 +281,13 @@ type sqlcQuerier interface { GetChatDebugStepsByRunID(ctx context.Context, runID uuid.UUID) ([]ChatDebugStep, error) GetChatDesktopEnabled(ctx context.Context) (bool, error) GetChatDiffStatusByChatID(ctx context.Context, chatID uuid.UUID) (ChatDiffStatus, error) + // Returns aggregate PR counts across all agent chats for telemetry. + // Deduplicates by PR URL so forked chats referencing the same pull + // request are counted once (using the most recently refreshed state). + // Total is derived from the three recognized state buckets and + // always equals open + merged + closed; other non-NULL states are + // intentionally excluded from these aggregates. + GetChatDiffStatusSummary(ctx context.Context) (GetChatDiffStatusSummaryRow, error) GetChatDiffStatusesByChatIDs(ctx context.Context, chatIds []uuid.UUID) ([]ChatDiffStatus, error) GetChatExploreModelOverride(ctx context.Context) (string, error) GetChatFileByID(ctx context.Context, id uuid.UUID) (ChatFile, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 3c6b2b0452..f5c150198a 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5809,6 +5809,48 @@ func (q *sqlQuerier) GetChatDiffStatusByChatID(ctx context.Context, chatID uuid. return i, err } +const getChatDiffStatusSummary = `-- name: GetChatDiffStatusSummary :one +WITH deduped AS ( + SELECT DISTINCT ON (COALESCE(NULLIF(cds.url, ''), c.id::text)) + cds.pull_request_state + FROM chat_diff_statuses cds + JOIN chats c ON c.id = cds.chat_id + WHERE cds.pull_request_state IN ('open', 'merged', 'closed') + ORDER BY COALESCE(NULLIF(cds.url, ''), c.id::text), cds.updated_at DESC, c.id DESC +) +SELECT + COUNT(*)::bigint AS total, + COUNT(*) FILTER (WHERE pull_request_state = 'open')::bigint AS open, + COUNT(*) FILTER (WHERE pull_request_state = 'merged')::bigint AS merged, + COUNT(*) FILTER (WHERE pull_request_state = 'closed')::bigint AS closed +FROM deduped +` + +type GetChatDiffStatusSummaryRow struct { + Total int64 `db:"total" json:"total"` + Open int64 `db:"open" json:"open"` + Merged int64 `db:"merged" json:"merged"` + Closed int64 `db:"closed" json:"closed"` +} + +// Returns aggregate PR counts across all agent chats for telemetry. +// Deduplicates by PR URL so forked chats referencing the same pull +// request are counted once (using the most recently refreshed state). +// Total is derived from the three recognized state buckets and +// always equals open + merged + closed; other non-NULL states are +// intentionally excluded from these aggregates. +func (q *sqlQuerier) GetChatDiffStatusSummary(ctx context.Context) (GetChatDiffStatusSummaryRow, error) { + row := q.db.QueryRowContext(ctx, getChatDiffStatusSummary) + var i GetChatDiffStatusSummaryRow + err := row.Scan( + &i.Total, + &i.Open, + &i.Merged, + &i.Closed, + ) + return i, err +} + const getChatDiffStatusesByChatIDs = `-- name: GetChatDiffStatusesByChatIDs :many SELECT chat_id, url, pull_request_state, changes_requested, additions, deletions, changed_files, refreshed_at, stale_at, created_at, updated_at, git_branch, git_remote_origin, pull_request_title, pull_request_draft, author_login, author_avatar_url, base_branch, pr_number, commits, approved, reviewer_count, head_branch @@ -6621,12 +6663,14 @@ func (q *sqlQuerier) GetChatsByWorkspaceIDs(ctx context.Context, ids []uuid.UUID const getChatsUpdatedAfter = `-- name: GetChatsUpdatedAfter :many SELECT - id, owner_id, created_at, updated_at, status, - (parent_chat_id IS NOT NULL)::bool AS has_parent, - root_chat_id, workspace_id, - mode, archived, last_model_config_id, client_type -FROM chats -WHERE updated_at > $1 + c.id, c.owner_id, c.created_at, c.updated_at, c.status, + (c.parent_chat_id IS NOT NULL)::bool AS has_parent, + c.root_chat_id, c.workspace_id, + c.mode, c.archived, c.last_model_config_id, c.client_type, + cds.pull_request_state +FROM chats c +LEFT JOIN chat_diff_statuses cds ON cds.chat_id = c.id +WHERE c.updated_at > $1 ` type GetChatsUpdatedAfterRow struct { @@ -6642,6 +6686,7 @@ type GetChatsUpdatedAfterRow struct { Archived bool `db:"archived" json:"archived"` LastModelConfigID uuid.UUID `db:"last_model_config_id" json:"last_model_config_id"` ClientType ChatClientType `db:"client_type" json:"client_type"` + PullRequestState sql.NullString `db:"pull_request_state" json:"pull_request_state"` } // Retrieves chats updated after the given timestamp for telemetry @@ -6669,6 +6714,7 @@ func (q *sqlQuerier) GetChatsUpdatedAfter(ctx context.Context, updatedAfter time &i.Archived, &i.LastModelConfigID, &i.ClientType, + &i.PullRequestState, ); err != nil { return nil, err } diff --git a/coderd/database/queries/chats.sql b/coderd/database/queries/chats.sql index 73d2806c06..72a0ad451d 100644 --- a/coderd/database/queries/chats.sql +++ b/coderd/database/queries/chats.sql @@ -934,6 +934,28 @@ SET WHERE chat_id = @chat_id::uuid; +-- name: GetChatDiffStatusSummary :one +-- Returns aggregate PR counts across all agent chats for telemetry. +-- Deduplicates by PR URL so forked chats referencing the same pull +-- request are counted once (using the most recently refreshed state). +-- Total is derived from the three recognized state buckets and +-- always equals open + merged + closed; other non-NULL states are +-- intentionally excluded from these aggregates. +WITH deduped AS ( + SELECT DISTINCT ON (COALESCE(NULLIF(cds.url, ''), c.id::text)) + cds.pull_request_state + FROM chat_diff_statuses cds + JOIN chats c ON c.id = cds.chat_id + WHERE cds.pull_request_state IN ('open', 'merged', 'closed') + ORDER BY COALESCE(NULLIF(cds.url, ''), c.id::text), cds.updated_at DESC, c.id DESC +) +SELECT + COUNT(*)::bigint AS total, + COUNT(*) FILTER (WHERE pull_request_state = 'open')::bigint AS open, + COUNT(*) FILTER (WHERE pull_request_state = 'merged')::bigint AS merged, + COUNT(*) FILTER (WHERE pull_request_state = 'closed')::bigint AS closed +FROM deduped; + -- name: GetChatCostSummary :one -- Aggregate cost summary for a single user within a date range. -- Only counts assistant-role messages. @@ -1298,12 +1320,14 @@ WHERE chats.id = deletable.id -- snapshot collection. Uses updated_at so that long-running chats -- still appear in each snapshot window while they are active. SELECT - id, owner_id, created_at, updated_at, status, - (parent_chat_id IS NOT NULL)::bool AS has_parent, - root_chat_id, workspace_id, - mode, archived, last_model_config_id, client_type -FROM chats -WHERE updated_at > @updated_after; + c.id, c.owner_id, c.created_at, c.updated_at, c.status, + (c.parent_chat_id IS NOT NULL)::bool AS has_parent, + c.root_chat_id, c.workspace_id, + c.mode, c.archived, c.last_model_config_id, c.client_type, + cds.pull_request_state +FROM chats c +LEFT JOIN chat_diff_statuses cds ON cds.chat_id = c.id +WHERE c.updated_at > @updated_after; -- name: GetChatMessageSummariesPerChat :many -- Aggregates message-level metrics per chat for messages created diff --git a/coderd/telemetry/telemetry.go b/coderd/telemetry/telemetry.go index f2bb525160..fe5d247caf 100644 --- a/coderd/telemetry/telemetry.go +++ b/coderd/telemetry/telemetry.go @@ -809,6 +809,19 @@ func (r *remoteReporter) createSnapshot() (*Snapshot, error) { } return nil }) + eg.Go(func() error { + row, err := r.options.Database.GetChatDiffStatusSummary(ctx) + if err != nil { + return xerrors.Errorf("get chat diff status summary: %w", err) + } + snapshot.ChatDiffStatusSummary = &ChatDiffStatusSummary{ + Total: row.Total, + Open: row.Open, + Merged: row.Merged, + Closed: row.Closed, + } + return nil + }) err := eg.Wait() if err != nil { @@ -1540,6 +1553,7 @@ type Snapshot struct { Chats []Chat `json:"chats"` ChatMessageSummaries []ChatMessageSummary `json:"chat_message_summaries"` ChatModelConfigs []ChatModelConfig `json:"chat_model_configs"` + ChatDiffStatusSummary *ChatDiffStatusSummary `json:"chat_diff_status_summary"` } // Deployment contains information about the host running Coder. @@ -2173,6 +2187,9 @@ func ConvertChat(dbChat database.GetChatsUpdatedAfterRow) Chat { c.Mode = &mode } c.ClientType = string(dbChat.ClientType) + if dbChat.PullRequestState.Valid { + c.PullRequestState = &dbChat.PullRequestState.String + } return c } @@ -2347,6 +2364,7 @@ type Chat struct { Archived bool `json:"archived"` LastModelConfigID uuid.UUID `json:"last_model_config_id"` ClientType string `json:"client_type"` + PullRequestState *string `json:"pull_request_state"` } // ChatMessageSummary contains per-chat aggregated message metrics @@ -2380,6 +2398,17 @@ type ChatModelConfig struct { IsDefault bool `json:"is_default"` } +// ChatDiffStatusSummary contains aggregate PR counts across all +// agent chats. Total counts unique PRs with a known state +// (open + merged + closed). Open, Merged, and Closed break that +// total down by state. +type ChatDiffStatusSummary struct { + Total int64 `json:"total"` + Open int64 `json:"open"` + Merged int64 `json:"merged"` + Closed int64 `json:"closed"` +} + func ConvertAIBridgeInterceptionsSummary(endTime time.Time, provider, model, client string, summary database.CalculateAIBridgeInterceptionsTelemetrySummaryRow) AIBridgeInterceptionsSummary { return AIBridgeInterceptionsSummary{ ID: uuid.New(), diff --git a/coderd/telemetry/telemetry_test.go b/coderd/telemetry/telemetry_test.go index 16fffbf657..f0fd563ef4 100644 --- a/coderd/telemetry/telemetry_test.go +++ b/coderd/telemetry/telemetry_test.go @@ -1670,6 +1670,16 @@ func TestChatsTelemetry(t *testing.T) { }) require.NoError(t, err) + // Associate a PR with the root chat so PullRequestState is populated. + rootChatNow := dbtime.Now() + _, err = db.UpsertChatDiffStatus(ctx, database.UpsertChatDiffStatusParams{ + ChatID: rootChat.ID, + PullRequestState: sql.NullString{String: "merged", Valid: true}, + RefreshedAt: rootChatNow, + StaleAt: rootChatNow, + }) + require.NoError(t, err) + // Insert messages for root chat: 2 user, 2 assistant, 1 tool. _, err = db.InsertChatMessages(ctx, database.InsertChatMessagesParams{ ChatID: rootChat.ID, @@ -1773,7 +1783,11 @@ func TestChatsTelemetry(t *testing.T) { assert.Equal(t, "computer_use", *foundRoot.Mode) assert.False(t, foundRoot.Archived) assert.Equal(t, "ui", foundRoot.ClientType) + require.NotNil(t, foundRoot.PullRequestState) + assert.Equal(t, "merged", *foundRoot.PullRequestState) + // Child chat assertions. + assert.Equal(t, childChat.ID, foundChild.ID) assert.Equal(t, user.ID, foundChild.OwnerID) assert.True(t, foundChild.HasParent) @@ -1785,7 +1799,10 @@ func TestChatsTelemetry(t *testing.T) { assert.Nil(t, foundChild.Mode) assert.False(t, foundChild.Archived) assert.Equal(t, "ui", foundChild.ClientType) + assert.Nil(t, foundChild.PullRequestState) + // --- Assert ChatMessageSummaries --- + require.Len(t, snapshot.ChatMessageSummaries, 2) summaryMap := make(map[uuid.UUID]telemetry.ChatMessageSummary) @@ -1853,3 +1870,124 @@ func TestChatsTelemetry(t *testing.T) { assert.True(t, cfg2.Enabled) assert.False(t, cfg2.IsDefault) } + +func TestChatDiffStatusSummaryTelemetry(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + db, _ := dbtestutil.NewDB(t) + + // Verify zero counts when no chat_diff_statuses exist. + _, emptySnapshot := collectSnapshot(ctx, t, db, nil) + require.NotNil(t, emptySnapshot.ChatDiffStatusSummary) + assert.Equal(t, int64(0), emptySnapshot.ChatDiffStatusSummary.Total) + assert.Equal(t, int64(0), emptySnapshot.ChatDiffStatusSummary.Open) + assert.Equal(t, int64(0), emptySnapshot.ChatDiffStatusSummary.Merged) + assert.Equal(t, int64(0), emptySnapshot.ChatDiffStatusSummary.Closed) + + // Set up minimal FK chain: provider -> model config -> chat. + user := dbgen.User(t, db, database.User{}) + org, err := db.GetDefaultOrganization(ctx) + require.NoError(t, err) + + _, err = db.InsertChatProvider(ctx, database.InsertChatProviderParams{ + Provider: "anthropic", + DisplayName: "Anthropic", + Enabled: true, + CentralApiKeyEnabled: true, + }) + require.NoError(t, err) + + modelCfg, err := db.InsertChatModelConfig(ctx, database.InsertChatModelConfigParams{ + Provider: "anthropic", + Model: "claude-sonnet-4-20250514", + DisplayName: "Claude Sonnet", + Enabled: true, + IsDefault: true, + ContextLimit: 200000, + CompressionThreshold: 70, + Options: json.RawMessage("{}"), + }) + require.NoError(t, err) + + // Helper to create a chat and upsert its diff status. + insertChatWithDiffStatus := func(prURL, state string) uuid.UUID { + t.Helper() + chat, chatErr := db.InsertChat(ctx, database.InsertChatParams{ + OrganizationID: org.ID, + OwnerID: user.ID, + LastModelConfigID: modelCfg.ID, + Title: "Chat " + state, + Status: database.ChatStatusCompleted, + ClientType: database.ChatClientTypeUi, + }) + require.NoError(t, chatErr) + now := dbtime.Now() + _, chatErr = db.UpsertChatDiffStatus(ctx, database.UpsertChatDiffStatusParams{ + ChatID: chat.ID, + Url: sql.NullString{String: prURL, Valid: prURL != ""}, + PullRequestState: sql.NullString{String: state, Valid: true}, + RefreshedAt: now, + StaleAt: now, + }) + require.NoError(t, chatErr) + return chat.ID + } + + // Insert: 1 merged, 1 open, 1 closed (each with unique URLs). + // For pull/1, first insert an older chat with stale "open" state, + // then a newer chat with refreshed "merged" state. The dedup + // query orders by cds.updated_at DESC, so "merged" should win. + insertChatWithDiffStatus("https://github.com/org/repo/pull/1", "open") + insertChatWithDiffStatus("https://github.com/org/repo/pull/1", "merged") + openChatID := insertChatWithDiffStatus("https://github.com/org/repo/pull/2", "open") + insertChatWithDiffStatus("https://github.com/org/repo/pull/3", "closed") + + // Insert a chat with NULL pull_request_state (no PR yet). + // This should be excluded from all counts. + noPRChat, err := db.InsertChat(ctx, database.InsertChatParams{ + OrganizationID: org.ID, + OwnerID: user.ID, + LastModelConfigID: modelCfg.ID, + Title: "Chat no PR", + Status: database.ChatStatusRunning, + ClientType: database.ChatClientTypeUi, + }) + require.NoError(t, err) + now := dbtime.Now() + _, err = db.UpsertChatDiffStatus(ctx, database.UpsertChatDiffStatusParams{ + ChatID: noPRChat.ID, + RefreshedAt: now, + StaleAt: now, + }) + require.NoError(t, err) + + _, snapshot := collectSnapshot(ctx, t, db, nil) + + // 3 unique PRs (deduped by URL), not 4 chat_diff_statuses rows. + require.NotNil(t, snapshot.ChatDiffStatusSummary) + assert.Equal(t, int64(3), snapshot.ChatDiffStatusSummary.Total) + assert.Equal(t, int64(1), snapshot.ChatDiffStatusSummary.Open) + assert.Equal(t, int64(1), snapshot.ChatDiffStatusSummary.Merged) + assert.Equal(t, int64(1), snapshot.ChatDiffStatusSummary.Closed) + + // Transition the "open" PR to "merged" via upsert on the same + // chat_id. The aggregate should reflect the new state. + now = dbtime.Now() + _, err = db.UpsertChatDiffStatus(ctx, database.UpsertChatDiffStatusParams{ + ChatID: openChatID, + Url: sql.NullString{String: "https://github.com/org/repo/pull/2", Valid: true}, + PullRequestState: sql.NullString{String: "merged", Valid: true}, + RefreshedAt: now, + StaleAt: now, + }) + require.NoError(t, err) + + _, snapshot2 := collectSnapshot(ctx, t, db, nil) + + require.NotNil(t, snapshot2.ChatDiffStatusSummary) + assert.Equal(t, int64(3), snapshot2.ChatDiffStatusSummary.Total) + assert.Equal(t, int64(0), snapshot2.ChatDiffStatusSummary.Open) + assert.Equal(t, int64(2), snapshot2.ChatDiffStatusSummary.Merged) + assert.Equal(t, int64(1), snapshot2.ChatDiffStatusSummary.Closed) +}