From 147d62750539017757081746269ba0e4f49b401a Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 18 Mar 2026 15:50:50 -0400 Subject: [PATCH] fix: deduplicate PR insights, fix cost computation, simplify UI (#23251) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem The `/agents/settings/insights` page had several issues: 1. **Duplicate PRs** in "Recent Pull Requests" — multiple chats referencing the same PR URL each produced a row 2. **Wildly wrong costs** — the cost subquery summed ALL messages across the entire chat *tree* (`GROUP BY root_chat_id`), so every chat in a tree got the same inflated total. When aggregated, the same tree cost was counted N× per PR in that tree 3. **UI clutter** — too many stat cards, too many table columns, mixed naming conventions ## Fix ### Backend (SQL) - **Deduplicate by PR URL** using `DISTINCT ON (COALESCE(cds.url, c.id::text))` across all 4 queries - **Fix cost computation**: use two CTEs — `pr_costs` sums cost from ALL chats that reference a PR (so review chats contribute), `deduped` picks one row per PR for state/additions/deletions via DISTINCT ON - **Tests**: 3 subtests covering multi-chat cost summing, different PRs no duplication, and duplicate URL counted once ### Frontend - **3 stat cards** (down from 5): Merged, Merge rate, Cost / merge - **2-line chart** (down from 3): created (dashed) + merged (solid) - **4-column model table** (down from 7): Model, Merged, Merge rate, Cost/merge - **4-column recent table** (down from 7): Title, Status, Cost, Created — with `table-fixed` to prevent overflow - **Consistent naming**: no mixed PR/PRs abbreviation, contextual labels since page title establishes context --- coderd/chats.go | 2 +- coderd/database/querier.go | 23 + coderd/database/querier_test.go | 479 ++++++++++++++++++ coderd/database/queries.sql.go | 360 ++++++++++---- coderd/database/queries/chatinsights.sql | 336 +++++++++---- site/src/pages/AgentsPage/PRInsightsView.tsx | 493 ++++++------------- 6 files changed, 1151 insertions(+), 542 deletions(-) diff --git a/coderd/chats.go b/coderd/chats.go index 5514d5be96..3041c0b8ce 100644 --- a/coderd/chats.go +++ b/coderd/chats.go @@ -4330,7 +4330,7 @@ func (api *API) prInsights(rw http.ResponseWriter, r *http.Request) { modelEntries := make([]codersdk.PRInsightsModelBreakdown, 0, len(byModel)) for _, m := range byModel { entry := codersdk.PRInsightsModelBreakdown{ - ModelConfigID: m.ModelConfigID, + ModelConfigID: m.ModelConfigID.UUID, DisplayName: m.DisplayName, Provider: m.Provider, TotalPRs: m.TotalPrs, diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 4ab7bb53a7..90786a7d7d 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -341,16 +341,39 @@ type sqlcQuerier interface { // membership status for the prebuilds system user (org membership, group existence, group membership). GetOrganizationsWithPrebuildStatus(ctx context.Context, arg GetOrganizationsWithPrebuildStatusParams) ([]GetOrganizationsWithPrebuildStatusRow, error) // Returns PR metrics grouped by the model used for each chat. + // Uses two CTEs: pr_costs sums cost for the PR-linked chat and its + // direct children (that lack their own PR), and deduped picks one row + // per PR for state/additions/deletions/model (model comes from the + // most recent chat). GetPRInsightsPerModel(ctx context.Context, arg GetPRInsightsPerModelParams) ([]GetPRInsightsPerModelRow, error) // Returns individual PR rows with cost for the recent PRs table. + // Uses two CTEs: pr_costs sums cost for the PR-linked chat and its + // direct children (that lack their own PR), and deduped picks one row + // per PR for metadata. GetPRInsightsRecentPRs(ctx context.Context, arg GetPRInsightsRecentPRsParams) ([]GetPRInsightsRecentPRsRow, error) // PR Insights queries for the /agents analytics dashboard. // These aggregate data from chat_diff_statuses (PR metadata) joined // with chats and chat_messages (cost) to power the PR Insights view. + // + // Cost is computed per PR by summing the PR-linked chat's own cost plus + // the costs of any direct children (subagents) it spawned that do NOT + // have their own PR association. If a child chat has its own + // chat_diff_statuses entry (with a non-NULL pull_request_state), its + // cost is attributed to that child's PR instead — preventing + // double-counting when sibling chats create different PRs. + // Subagent trees are at most 2 levels deep (enforced by the + // application layer). PR metadata (state, additions, deletions) + // comes from the most recent chat via DISTINCT ON so that each PR + // is counted exactly once. // Returns aggregate PR metrics for the given date range. // The handler calls this twice (current + previous period) for trends. + // Uses two CTEs: pr_costs sums cost for the PR-linked chat and its + // direct children (that lack their own PR), and deduped picks one row + // per PR for state/additions/deletions. GetPRInsightsSummary(ctx context.Context, arg GetPRInsightsSummaryParams) (GetPRInsightsSummaryRow, error) // Returns daily PR counts grouped by state for the chart. + // Uses a CTE to deduplicate by PR URL so that multiple chats referencing + // the same pull request are only counted once (keeping the most recent chat). GetPRInsightsTimeSeries(ctx context.Context, arg GetPRInsightsTimeSeriesParams) ([]GetPRInsightsTimeSeriesRow, error) GetParameterSchemasByJobID(ctx context.Context, jobID uuid.UUID) ([]ParameterSchema, error) GetPrebuildMetrics(ctx context.Context) ([]GetPrebuildMetricsRow, error) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index af843c7fde..ec5bcfe988 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -9964,3 +9964,482 @@ func TestUpsertAISeats(t *testing.T) { require.NoError(t, err) require.False(t, alreadyExists) } + +func TestGetPRInsights(t *testing.T) { + t.Parallel() + if testing.Short() { + t.SkipNow() + } + + // setupChatInfra creates a fresh database with a user, chat provider, + // and model config. Returns the store, user ID, and model config ID. + setupChatInfra := func(t *testing.T) (database.Store, uuid.UUID, uuid.UUID) { + t.Helper() + store, _ := dbtestutil.NewDB(t) + ctx := context.Background() + dbgen.Organization(t, store, database.Organization{}) + user := dbgen.User(t, store, database.User{}) + + _, err := store.InsertChatProvider(ctx, database.InsertChatProviderParams{ + Provider: "anthropic", + DisplayName: "Anthropic", + APIKey: "test-key", + Enabled: true, + }) + require.NoError(t, err) + + mc, err := store.InsertChatModelConfig(ctx, database.InsertChatModelConfigParams{ + Provider: "anthropic", + Model: "claude-4", + DisplayName: "Claude 4", + CreatedBy: uuid.NullUUID{UUID: user.ID, Valid: true}, + UpdatedBy: uuid.NullUUID{UUID: user.ID, Valid: true}, + Enabled: true, + IsDefault: true, + ContextLimit: 128000, + CompressionThreshold: 80, + Options: json.RawMessage(`{}`), + }) + require.NoError(t, err) + + return store, user.ID, mc.ID + } + + createChat := func(t *testing.T, store database.Store, userID, mcID uuid.UUID, title string) database.Chat { + t.Helper() + chat, err := store.InsertChat(context.Background(), database.InsertChatParams{ + OwnerID: userID, + LastModelConfigID: mcID, + Title: title, + }) + require.NoError(t, err) + return chat + } + + // insertCostMessage inserts a single assistant message with the + // given total_cost_micros value. + insertCostMessage := func(t *testing.T, store database.Store, chatID, userID, mcID uuid.UUID, costMicros int64) { + t.Helper() + _, err := store.InsertChatMessages(context.Background(), database.InsertChatMessagesParams{ + ChatID: chatID, + CreatedBy: []uuid.UUID{userID}, + ModelConfigID: []uuid.UUID{mcID}, + Role: []database.ChatMessageRole{database.ChatMessageRoleAssistant}, + Content: []string{`[{"type":"text","text":"hello"}]`}, + ContentVersion: []int16{1}, + Visibility: []database.ChatMessageVisibility{database.ChatMessageVisibilityBoth}, + InputTokens: []int64{0}, + OutputTokens: []int64{0}, + TotalTokens: []int64{0}, + ReasoningTokens: []int64{0}, + CacheCreationTokens: []int64{0}, + CacheReadTokens: []int64{0}, + ContextLimit: []int64{0}, + Compressed: []bool{false}, + TotalCostMicros: []int64{costMicros}, + RuntimeMs: []int64{0}, + }) + require.NoError(t, err) + } + + // linkPR associates a chat with a pull request via + // UpsertChatDiffStatus. + linkPR := func(t *testing.T, store database.Store, chatID uuid.UUID, prURL, state, title string, additions, deletions, changed int32) { + t.Helper() + now := time.Now() + _, err := store.UpsertChatDiffStatus(context.Background(), database.UpsertChatDiffStatusParams{ + ChatID: chatID, + Url: sql.NullString{String: prURL, Valid: true}, + PullRequestState: sql.NullString{String: state, Valid: true}, + PullRequestTitle: title, + Additions: additions, + Deletions: deletions, + ChangedFiles: changed, + RefreshedAt: now, + StaleAt: now.Add(time.Hour), + }) + require.NoError(t, err) + } + + startDate := time.Now().Add(-24 * time.Hour) + endDate := time.Now().Add(time.Hour) + noOwner := uuid.NullUUID{} + + t.Run("MultipleChatsSamePR_CostSummed", func(t *testing.T) { + t.Parallel() + store, userID, mcID := setupChatInfra(t) + + chatA := createChat(t, store, userID, mcID, "chat-A") + insertCostMessage(t, store, chatA.ID, userID, mcID, 5_000_000) // $5 + + chatB := createChat(t, store, userID, mcID, "chat-B") + insertCostMessage(t, store, chatB.ID, userID, mcID, 3_000_000) // $3 + + prURL := "https://github.com/org/repo/pull/123" + linkPR(t, store, chatA.ID, prURL, "merged", "fix: something", 100, 20, 5) + linkPR(t, store, chatB.ID, prURL, "merged", "fix: something", 100, 20, 5) + + // Both chats reference the same PR. The pr_costs CTE sums + // cost across all chats for the same PR URL, so the total + // should be $5 + $3 = $8. The PR itself is counted once. + summary, err := store.GetPRInsightsSummary(context.Background(), database.GetPRInsightsSummaryParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + }) + require.NoError(t, err) + assert.Equal(t, int64(1), summary.TotalPrsCreated) + assert.Equal(t, int64(8_000_000), summary.TotalCostMicros) + + recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + LimitVal: 20, + }) + require.NoError(t, err) + require.Len(t, recent, 1) + assert.Equal(t, int64(8_000_000), recent[0].CostMicros) + }) + + t.Run("DifferentPRs_NoDuplication", func(t *testing.T) { + t.Parallel() + store, userID, mcID := setupChatInfra(t) + + chatA := createChat(t, store, userID, mcID, "chat-A") + insertCostMessage(t, store, chatA.ID, userID, mcID, 5_000_000) + linkPR(t, store, chatA.ID, "https://github.com/org/repo/pull/1", "merged", "feat: A", 50, 10, 2) + + chatB := createChat(t, store, userID, mcID, "chat-B") + insertCostMessage(t, store, chatB.ID, userID, mcID, 3_000_000) + linkPR(t, store, chatB.ID, "https://github.com/org/repo/pull/2", "open", "feat: B", 80, 30, 4) + + summary, err := store.GetPRInsightsSummary(context.Background(), database.GetPRInsightsSummaryParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + }) + require.NoError(t, err) + assert.Equal(t, int64(2), summary.TotalPrsCreated) + assert.Equal(t, int64(8_000_000), summary.TotalCostMicros) // $5 + $3 + assert.Equal(t, int64(1), summary.TotalPrsMerged) + + // RecentPRs ordered by created_at DESC: chatB is newer. + recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + LimitVal: 20, + }) + require.NoError(t, err) + require.Len(t, recent, 2) + // Costs must not be mixed across different PRs. + assert.Equal(t, int64(3_000_000), recent[0].CostMicros) // PR 2 (newer) + assert.Equal(t, int64(5_000_000), recent[1].CostMicros) // PR 1 (older) + }) + + // createChildChat creates a chat with ParentChatID and RootChatID + // set, simulating a subagent/child chat in a tree. + createChildChat := func(t *testing.T, store database.Store, userID, mcID, parentID, rootID uuid.UUID, title string) database.Chat { + t.Helper() + chat, err := store.InsertChat(context.Background(), database.InsertChatParams{ + OwnerID: userID, + LastModelConfigID: mcID, + Title: title, + ParentChatID: uuid.NullUUID{UUID: parentID, Valid: true}, + RootChatID: uuid.NullUUID{UUID: rootID, Valid: true}, + }) + require.NoError(t, err) + return chat + } + + t.Run("DuplicatePRUrl_CountedOnce", func(t *testing.T) { + t.Parallel() + store, userID, mcID := setupChatInfra(t) + + prURL := "https://github.com/org/repo/pull/99" + for i := 0; i < 3; i++ { + chat := createChat(t, store, userID, mcID, fmt.Sprintf("chat-%d", i)) + insertCostMessage(t, store, chat.ID, userID, mcID, 1_000_000) + linkPR(t, store, chat.ID, prURL, "merged", "fix: same PR", 40, 10, 3) + } + + summary, err := store.GetPRInsightsSummary(context.Background(), database.GetPRInsightsSummaryParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + }) + require.NoError(t, err) + assert.Equal(t, int64(1), summary.TotalPrsCreated) + assert.Equal(t, int64(1), summary.TotalPrsMerged) + + recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + LimitVal: 20, + }) + require.NoError(t, err) + require.Len(t, recent, 1) + }) + + t.Run("ChildChatCostsIncluded", func(t *testing.T) { + t.Parallel() + store, userID, mcID := setupChatInfra(t) + + // Parent chat with a $5 cost. + parent := createChat(t, store, userID, mcID, "parent-chat") + insertCostMessage(t, store, parent.ID, userID, mcID, 5_000_000) + + // Two child chats (subagents) with $2 each. Only the parent + // has a chat_diff_statuses entry, but the children's costs + // should be included via the tree join. + child1 := createChildChat(t, store, userID, mcID, parent.ID, parent.ID, "child-1") + insertCostMessage(t, store, child1.ID, userID, mcID, 2_000_000) + + child2 := createChildChat(t, store, userID, mcID, parent.ID, parent.ID, "child-2") + insertCostMessage(t, store, child2.ID, userID, mcID, 2_000_000) + + prURL := "https://github.com/org/repo/pull/42" + linkPR(t, store, parent.ID, prURL, "merged", "feat: tree cost", 60, 15, 3) + + // Summary should reflect $5 + $2 + $2 = $9 total. + summary, err := store.GetPRInsightsSummary(context.Background(), database.GetPRInsightsSummaryParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + }) + require.NoError(t, err) + assert.Equal(t, int64(1), summary.TotalPrsCreated) + assert.Equal(t, int64(1), summary.TotalPrsMerged) + assert.Equal(t, int64(9_000_000), summary.TotalCostMicros) + + // RecentPRs should return 1 row with the full tree cost. + recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + LimitVal: 20, + }) + require.NoError(t, err) + require.Len(t, recent, 1) + assert.Equal(t, int64(9_000_000), recent[0].CostMicros) + }) + + t.Run("SiblingPRs_NoCrossContamination", func(t *testing.T) { + t.Parallel() + store, userID, mcID := setupChatInfra(t) + + // Parent chat with $10 orchestration cost. + parent := createChat(t, store, userID, mcID, "parent") + insertCostMessage(t, store, parent.ID, userID, mcID, 10_000_000) + + // Child C1 ($5) creates PR1. + c1 := createChildChat(t, store, userID, mcID, parent.ID, parent.ID, "child-1") + insertCostMessage(t, store, c1.ID, userID, mcID, 5_000_000) + linkPR(t, store, c1.ID, "https://github.com/org/repo/pull/10", "merged", "feat: PR1", 50, 10, 2) + + // Child C2 ($3) creates PR2. + c2 := createChildChat(t, store, userID, mcID, parent.ID, parent.ID, "child-2") + insertCostMessage(t, store, c2.ID, userID, mcID, 3_000_000) + linkPR(t, store, c2.ID, "https://github.com/org/repo/pull/11", "open", "feat: PR2", 30, 5, 1) + + // With direct-branch attribution: + // PR1 cost = C1's own cost = $5 (parent NOT included — only children of C1) + // PR2 cost = C2's own cost = $3 + // Total = $8 (no double-counting of parent or siblings) + summary, err := store.GetPRInsightsSummary(context.Background(), database.GetPRInsightsSummaryParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + }) + require.NoError(t, err) + assert.Equal(t, int64(2), summary.TotalPrsCreated) + assert.Equal(t, int64(8_000_000), summary.TotalCostMicros) + + recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + LimitVal: 20, + }) + require.NoError(t, err) + require.Len(t, recent, 2) + // PR2 (newer) = $3, PR1 (older) = $5. + assert.Equal(t, int64(3_000_000), recent[0].CostMicros) + assert.Equal(t, int64(5_000_000), recent[1].CostMicros) + }) + + t.Run("ParentAndChildDifferentPRs_NoCrossContamination", func(t *testing.T) { + t.Parallel() + store, userID, mcID := setupChatInfra(t) + + // Parent P ($10) creates PR1. + parent := createChat(t, store, userID, mcID, "parent") + insertCostMessage(t, store, parent.ID, userID, mcID, 10_000_000) + linkPR(t, store, parent.ID, "https://github.com/org/repo/pull/20", "merged", "feat: parent PR", 80, 20, 4) + + // Child C1 ($5) has its own PR2. Because C1 has its own + // chat_diff_statuses entry, its cost should NOT be included + // under PR1 — it belongs to PR2 only. + c1 := createChildChat(t, store, userID, mcID, parent.ID, parent.ID, "child-1") + insertCostMessage(t, store, c1.ID, userID, mcID, 5_000_000) + linkPR(t, store, c1.ID, "https://github.com/org/repo/pull/21", "open", "feat: child PR", 30, 5, 1) + + // Child C2 ($2) has NO cds entry — pure subagent. + // Its cost should be included under PR1 (the parent's PR). + c2 := createChildChat(t, store, userID, mcID, parent.ID, parent.ID, "child-2") + insertCostMessage(t, store, c2.ID, userID, mcID, 2_000_000) + + // PR1 cost = parent ($10) + C2 ($2) = $12 (C1 excluded) + // PR2 cost = C1 ($5) + // Total = $17 (actual spend: $10 + $5 + $2 = $17) + summary, err := store.GetPRInsightsSummary(context.Background(), database.GetPRInsightsSummaryParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + }) + require.NoError(t, err) + assert.Equal(t, int64(2), summary.TotalPrsCreated) + assert.Equal(t, int64(17_000_000), summary.TotalCostMicros) + + recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + LimitVal: 20, + }) + require.NoError(t, err) + require.Len(t, recent, 2) + // PR2/C1 (newer) = $5, PR1/parent (older) = $12. + assert.Equal(t, int64(5_000_000), recent[0].CostMicros) + assert.Equal(t, int64(12_000_000), recent[1].CostMicros) + }) + + t.Run("EmptyURLNotCollapsed", func(t *testing.T) { + t.Parallel() + store, userID, mcID := setupChatInfra(t) + + // Two chats with empty-string URLs should be treated as + // separate PRs (NULLIF converts '' to NULL, falling back + // to c.id::text). + chatX := createChat(t, store, userID, mcID, "chat-X") + insertCostMessage(t, store, chatX.ID, userID, mcID, 4_000_000) + linkPR(t, store, chatX.ID, "", "open", "draft: X", 10, 2, 1) + + chatY := createChat(t, store, userID, mcID, "chat-Y") + insertCostMessage(t, store, chatY.ID, userID, mcID, 6_000_000) + linkPR(t, store, chatY.ID, "", "merged", "draft: Y", 20, 5, 2) + + summary, err := store.GetPRInsightsSummary(context.Background(), database.GetPRInsightsSummaryParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + }) + require.NoError(t, err) + assert.Equal(t, int64(2), summary.TotalPrsCreated) + assert.Equal(t, int64(10_000_000), summary.TotalCostMicros) + + recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + LimitVal: 20, + }) + require.NoError(t, err) + require.Len(t, recent, 2) + }) + + t.Run("ParentAndChildSameURL_DedupedWithCombinedCost", func(t *testing.T) { + t.Parallel() + store, userID, mcID := setupChatInfra(t) + + // Parent P ($10) links to a PR. + parent := createChat(t, store, userID, mcID, "parent") + insertCostMessage(t, store, parent.ID, userID, mcID, 10_000_000) + + // Child C ($5) also links to the same PR URL. + child := createChildChat(t, store, userID, mcID, parent.ID, parent.ID, "child") + insertCostMessage(t, store, child.ID, userID, mcID, 5_000_000) + + prURL := "https://github.com/org/repo/pull/50" + linkPR(t, store, parent.ID, prURL, "merged", "feat: shared PR", 70, 15, 3) + linkPR(t, store, child.ID, prURL, "merged", "feat: shared PR", 70, 15, 3) + + // Both parent and child have cds entries for the same URL. + // The PR should be counted once with combined cost $10 + $5 = $15. + summary, err := store.GetPRInsightsSummary(context.Background(), database.GetPRInsightsSummaryParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + }) + require.NoError(t, err) + assert.Equal(t, int64(1), summary.TotalPrsCreated) + assert.Equal(t, int64(15_000_000), summary.TotalCostMicros) + + recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + LimitVal: 20, + }) + require.NoError(t, err) + require.Len(t, recent, 1) + assert.Equal(t, int64(15_000_000), recent[0].CostMicros) + }) + + t.Run("ZeroCostChat_StillCounted", func(t *testing.T) { + t.Parallel() + store, userID, mcID := setupChatInfra(t) + + // A chat linked to a PR but with NO chat_messages at all. + // The PR should still appear with zero cost. + chat := createChat(t, store, userID, mcID, "zero-cost-chat") + linkPR(t, store, chat.ID, "https://github.com/org/repo/pull/60", "open", "feat: no messages", 25, 5, 2) + + summary, err := store.GetPRInsightsSummary(context.Background(), database.GetPRInsightsSummaryParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + }) + require.NoError(t, err) + assert.Equal(t, int64(1), summary.TotalPrsCreated) + assert.Equal(t, int64(0), summary.TotalCostMicros) + + recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + LimitVal: 20, + }) + require.NoError(t, err) + require.Len(t, recent, 1) + assert.Equal(t, int64(0), recent[0].CostMicros) + }) + + t.Run("MergedCostMicros_OnlyCountsMerged", func(t *testing.T) { + t.Parallel() + store, userID, mcID := setupChatInfra(t) + + // Merged PR with $5 cost. + chatMerged := createChat(t, store, userID, mcID, "chat-merged") + insertCostMessage(t, store, chatMerged.ID, userID, mcID, 5_000_000) + linkPR(t, store, chatMerged.ID, "https://github.com/org/repo/pull/70", "merged", "fix: merged", 40, 10, 2) + + // Open PR with $3 cost. + chatOpen := createChat(t, store, userID, mcID, "chat-open") + insertCostMessage(t, store, chatOpen.ID, userID, mcID, 3_000_000) + linkPR(t, store, chatOpen.ID, "https://github.com/org/repo/pull/71", "open", "feat: open", 20, 5, 1) + + // TotalCostMicros includes both ($5 + $3 = $8), but + // MergedCostMicros only includes the merged PR ($5). + summary, err := store.GetPRInsightsSummary(context.Background(), database.GetPRInsightsSummaryParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + }) + require.NoError(t, err) + assert.Equal(t, int64(8_000_000), summary.TotalCostMicros) + assert.Equal(t, int64(5_000_000), summary.MergedCostMicros) + }) +} diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 3221888f57..50ce3430be 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2415,33 +2415,68 @@ func (q *sqlQuerier) InsertChatFile(ctx context.Context, arg InsertChatFileParam } const getPRInsightsPerModel = `-- name: GetPRInsightsPerModel :many -SELECT - cmc.id AS model_config_id, - cmc.display_name, - cmc.provider, - COUNT(*)::bigint AS total_prs, - COUNT(*) FILTER (WHERE cds.pull_request_state = 'merged')::bigint AS merged_prs, - COALESCE(SUM(cds.additions), 0)::bigint AS total_additions, - COALESCE(SUM(cds.deletions), 0)::bigint AS total_deletions, - COALESCE(SUM(cc.cost_micros), 0)::bigint AS total_cost_micros, - COALESCE(SUM(cc.cost_micros) FILTER (WHERE cds.pull_request_state = 'merged'), 0)::bigint AS merged_cost_micros -FROM chat_diff_statuses cds -JOIN chats c ON c.id = cds.chat_id -JOIN chat_model_configs cmc ON cmc.id = c.last_model_config_id -LEFT JOIN ( +WITH pr_costs AS ( SELECT - COALESCE(ch.root_chat_id, ch.id) AS root_id, - COALESCE(SUM(cm.total_cost_micros), 0) AS cost_micros - FROM chat_messages cm - JOIN chats ch ON ch.id = cm.chat_id - WHERE cm.total_cost_micros IS NOT NULL - GROUP BY COALESCE(ch.root_chat_id, ch.id) -) cc ON cc.root_id = COALESCE(c.root_chat_id, c.id) -WHERE cds.pull_request_state IS NOT NULL - AND c.created_at >= $1::timestamptz - AND c.created_at < $2::timestamptz - AND ($3::uuid IS NULL OR c.owner_id = $3::uuid) -GROUP BY cmc.id, cmc.display_name, cmc.provider + prc.pr_key, + COALESCE(SUM(cc.cost_micros), 0) AS cost_micros + FROM ( + SELECT DISTINCT + COALESCE(NULLIF(cds.url, ''), c.id::text) AS pr_key, + related.id AS chat_id + FROM chat_diff_statuses cds + JOIN chats c ON c.id = cds.chat_id + JOIN chats related + ON related.id = c.id + OR (related.parent_chat_id = c.id + AND NOT EXISTS ( + SELECT 1 FROM chat_diff_statuses cds2 + WHERE cds2.chat_id = related.id + AND cds2.pull_request_state IS NOT NULL + )) + WHERE cds.pull_request_state IS NOT NULL + AND c.created_at >= $1::timestamptz + AND c.created_at < $2::timestamptz + AND ($3::uuid IS NULL OR c.owner_id = $3::uuid) + ) prc + LEFT JOIN LATERAL ( + SELECT COALESCE(SUM(cm.total_cost_micros), 0) AS cost_micros + FROM chat_messages cm + WHERE cm.chat_id = prc.chat_id + AND cm.total_cost_micros IS NOT NULL + ) cc ON TRUE + GROUP BY prc.pr_key +), +deduped AS ( + SELECT DISTINCT ON (COALESCE(NULLIF(cds.url, ''), c.id::text)) + COALESCE(NULLIF(cds.url, ''), c.id::text) AS pr_key, + cds.pull_request_state, + cds.additions, + cds.deletions, + cmc.id AS model_config_id, + cmc.display_name, + cmc.provider + FROM chat_diff_statuses cds + JOIN chats c ON c.id = cds.chat_id + LEFT JOIN chat_model_configs cmc ON cmc.id = c.last_model_config_id + WHERE cds.pull_request_state IS NOT NULL + AND c.created_at >= $1::timestamptz + AND c.created_at < $2::timestamptz + AND ($3::uuid IS NULL OR c.owner_id = $3::uuid) + ORDER BY COALESCE(NULLIF(cds.url, ''), c.id::text), c.created_at DESC, c.id DESC +) +SELECT + d.model_config_id, + COALESCE(d.display_name, 'Unknown')::text AS display_name, + COALESCE(d.provider, 'unknown')::text AS provider, + COUNT(*)::bigint AS total_prs, + COUNT(*) FILTER (WHERE d.pull_request_state = 'merged')::bigint AS merged_prs, + COALESCE(SUM(d.additions), 0)::bigint AS total_additions, + COALESCE(SUM(d.deletions), 0)::bigint AS total_deletions, + COALESCE(SUM(pc.cost_micros), 0)::bigint AS total_cost_micros, + COALESCE(SUM(pc.cost_micros) FILTER (WHERE d.pull_request_state = 'merged'), 0)::bigint AS merged_cost_micros +FROM deduped d +JOIN pr_costs pc ON pc.pr_key = d.pr_key +GROUP BY d.model_config_id, d.display_name, d.provider ORDER BY total_prs DESC ` @@ -2452,18 +2487,22 @@ type GetPRInsightsPerModelParams struct { } type GetPRInsightsPerModelRow struct { - ModelConfigID uuid.UUID `db:"model_config_id" json:"model_config_id"` - DisplayName string `db:"display_name" json:"display_name"` - Provider string `db:"provider" json:"provider"` - TotalPrs int64 `db:"total_prs" json:"total_prs"` - MergedPrs int64 `db:"merged_prs" json:"merged_prs"` - TotalAdditions int64 `db:"total_additions" json:"total_additions"` - TotalDeletions int64 `db:"total_deletions" json:"total_deletions"` - TotalCostMicros int64 `db:"total_cost_micros" json:"total_cost_micros"` - MergedCostMicros int64 `db:"merged_cost_micros" json:"merged_cost_micros"` + ModelConfigID uuid.NullUUID `db:"model_config_id" json:"model_config_id"` + DisplayName string `db:"display_name" json:"display_name"` + Provider string `db:"provider" json:"provider"` + TotalPrs int64 `db:"total_prs" json:"total_prs"` + MergedPrs int64 `db:"merged_prs" json:"merged_prs"` + TotalAdditions int64 `db:"total_additions" json:"total_additions"` + TotalDeletions int64 `db:"total_deletions" json:"total_deletions"` + TotalCostMicros int64 `db:"total_cost_micros" json:"total_cost_micros"` + MergedCostMicros int64 `db:"merged_cost_micros" json:"merged_cost_micros"` } // Returns PR metrics grouped by the model used for each chat. +// Uses two CTEs: pr_costs sums cost for the PR-linked chat and its +// direct children (that lack their own PR), and deduped picks one row +// per PR for state/additions/deletions/model (model comes from the +// most recent chat). func (q *sqlQuerier) GetPRInsightsPerModel(ctx context.Context, arg GetPRInsightsPerModelParams) ([]GetPRInsightsPerModelRow, error) { rows, err := q.db.QueryContext(ctx, getPRInsightsPerModel, arg.StartDate, arg.EndDate, arg.OwnerID) if err != nil { @@ -2498,51 +2537,100 @@ func (q *sqlQuerier) GetPRInsightsPerModel(ctx context.Context, arg GetPRInsight } const getPRInsightsRecentPRs = `-- name: GetPRInsightsRecentPRs :many -SELECT - c.id AS chat_id, - cds.pull_request_title AS pr_title, - cds.url AS pr_url, - cds.pr_number, - cds.pull_request_state AS state, - cds.pull_request_draft AS draft, - cds.additions, - cds.deletions, - cds.changed_files, - cds.commits, - cds.approved, - cds.changes_requested, - cds.reviewer_count, - cds.author_login, - cds.author_avatar_url, - COALESCE(cds.base_branch, '')::text AS base_branch, - COALESCE(cmc.display_name, cmc.model)::text AS model_display_name, - COALESCE(cc.cost_micros, 0)::bigint AS cost_micros, - c.created_at -FROM chat_diff_statuses cds -JOIN chats c ON c.id = cds.chat_id -JOIN chat_model_configs cmc ON cmc.id = c.last_model_config_id -LEFT JOIN ( +WITH pr_costs AS ( SELECT - COALESCE(ch.root_chat_id, ch.id) AS root_id, - COALESCE(SUM(cm.total_cost_micros), 0) AS cost_micros - FROM chat_messages cm - JOIN chats ch ON ch.id = cm.chat_id - WHERE cm.total_cost_micros IS NOT NULL - GROUP BY COALESCE(ch.root_chat_id, ch.id) -) cc ON cc.root_id = COALESCE(c.root_chat_id, c.id) -WHERE cds.pull_request_state IS NOT NULL - AND c.created_at >= $1::timestamptz - AND c.created_at < $2::timestamptz - AND ($3::uuid IS NULL OR c.owner_id = $3::uuid) -ORDER BY c.created_at DESC -LIMIT $4::int + prc.pr_key, + COALESCE(SUM(cc.cost_micros), 0) AS cost_micros + FROM ( + SELECT DISTINCT + COALESCE(NULLIF(cds.url, ''), c.id::text) AS pr_key, + related.id AS chat_id + FROM chat_diff_statuses cds + JOIN chats c ON c.id = cds.chat_id + JOIN chats related + ON related.id = c.id + OR (related.parent_chat_id = c.id + AND NOT EXISTS ( + SELECT 1 FROM chat_diff_statuses cds2 + WHERE cds2.chat_id = related.id + AND cds2.pull_request_state IS NOT NULL + )) + WHERE cds.pull_request_state IS NOT NULL + AND c.created_at >= $2::timestamptz + AND c.created_at < $3::timestamptz + AND ($4::uuid IS NULL OR c.owner_id = $4::uuid) + ) prc + LEFT JOIN LATERAL ( + SELECT COALESCE(SUM(cm.total_cost_micros), 0) AS cost_micros + FROM chat_messages cm + WHERE cm.chat_id = prc.chat_id + AND cm.total_cost_micros IS NOT NULL + ) cc ON TRUE + GROUP BY prc.pr_key +), +deduped AS ( + SELECT DISTINCT ON (COALESCE(NULLIF(cds.url, ''), c.id::text)) + COALESCE(NULLIF(cds.url, ''), c.id::text) AS pr_key, + c.id AS chat_id, + cds.pull_request_title AS pr_title, + cds.url AS pr_url, + cds.pr_number, + cds.pull_request_state AS state, + cds.pull_request_draft AS draft, + cds.additions, + cds.deletions, + cds.changed_files, + cds.commits, + cds.approved, + cds.changes_requested, + cds.reviewer_count, + cds.author_login, + cds.author_avatar_url, + COALESCE(cds.base_branch, '')::text AS base_branch, + COALESCE(cmc.display_name, cmc.model, 'Unknown')::text AS model_display_name, + c.created_at + FROM chat_diff_statuses cds + JOIN chats c ON c.id = cds.chat_id + LEFT JOIN chat_model_configs cmc ON cmc.id = c.last_model_config_id + WHERE cds.pull_request_state IS NOT NULL + AND c.created_at >= $2::timestamptz + AND c.created_at < $3::timestamptz + AND ($4::uuid IS NULL OR c.owner_id = $4::uuid) + ORDER BY COALESCE(NULLIF(cds.url, ''), c.id::text), c.created_at DESC, c.id DESC +) +SELECT chat_id, pr_title, pr_url, pr_number, state, draft, additions, deletions, changed_files, commits, approved, changes_requested, reviewer_count, author_login, author_avatar_url, base_branch, model_display_name, cost_micros, created_at FROM ( + SELECT + d.chat_id, + d.pr_title, + d.pr_url, + d.pr_number, + d.state, + d.draft, + d.additions, + d.deletions, + d.changed_files, + d.commits, + d.approved, + d.changes_requested, + d.reviewer_count, + d.author_login, + d.author_avatar_url, + d.base_branch, + d.model_display_name, + COALESCE(pc.cost_micros, 0)::bigint AS cost_micros, + d.created_at + FROM deduped d + JOIN pr_costs pc ON pc.pr_key = d.pr_key +) sub +ORDER BY sub.created_at DESC +LIMIT $1::int ` type GetPRInsightsRecentPRsParams struct { + LimitVal int32 `db:"limit_val" json:"limit_val"` StartDate time.Time `db:"start_date" json:"start_date"` EndDate time.Time `db:"end_date" json:"end_date"` OwnerID uuid.NullUUID `db:"owner_id" json:"owner_id"` - LimitVal int32 `db:"limit_val" json:"limit_val"` } type GetPRInsightsRecentPRsRow struct { @@ -2568,12 +2656,15 @@ type GetPRInsightsRecentPRsRow struct { } // Returns individual PR rows with cost for the recent PRs table. +// Uses two CTEs: pr_costs sums cost for the PR-linked chat and its +// direct children (that lack their own PR), and deduped picks one row +// per PR for metadata. func (q *sqlQuerier) GetPRInsightsRecentPRs(ctx context.Context, arg GetPRInsightsRecentPRsParams) ([]GetPRInsightsRecentPRsRow, error) { rows, err := q.db.QueryContext(ctx, getPRInsightsRecentPRs, + arg.LimitVal, arg.StartDate, arg.EndDate, arg.OwnerID, - arg.LimitVal, ) if err != nil { return nil, err @@ -2618,29 +2709,63 @@ func (q *sqlQuerier) GetPRInsightsRecentPRs(ctx context.Context, arg GetPRInsigh const getPRInsightsSummary = `-- name: GetPRInsightsSummary :one +WITH pr_costs AS ( + SELECT + prc.pr_key, + COALESCE(SUM(cc.cost_micros), 0) AS cost_micros + FROM ( + -- For each PR, include the chat that references it plus any + -- direct children (subagents) that do not have their own PR. + SELECT DISTINCT + COALESCE(NULLIF(cds.url, ''), c.id::text) AS pr_key, + related.id AS chat_id + FROM chat_diff_statuses cds + JOIN chats c ON c.id = cds.chat_id + JOIN chats related + ON related.id = c.id + OR (related.parent_chat_id = c.id + AND NOT EXISTS ( + SELECT 1 FROM chat_diff_statuses cds2 + WHERE cds2.chat_id = related.id + AND cds2.pull_request_state IS NOT NULL + )) + WHERE cds.pull_request_state IS NOT NULL + AND c.created_at >= $1::timestamptz + AND c.created_at < $2::timestamptz + AND ($3::uuid IS NULL OR c.owner_id = $3::uuid) + ) prc + LEFT JOIN LATERAL ( + SELECT COALESCE(SUM(cm.total_cost_micros), 0) AS cost_micros + FROM chat_messages cm + WHERE cm.chat_id = prc.chat_id + AND cm.total_cost_micros IS NOT NULL + ) cc ON TRUE + GROUP BY prc.pr_key +), +deduped AS ( + SELECT DISTINCT ON (COALESCE(NULLIF(cds.url, ''), c.id::text)) + COALESCE(NULLIF(cds.url, ''), c.id::text) AS pr_key, + cds.pull_request_state, + cds.additions, + cds.deletions + FROM chat_diff_statuses cds + JOIN chats c ON c.id = cds.chat_id + WHERE cds.pull_request_state IS NOT NULL + AND c.created_at >= $1::timestamptz + AND c.created_at < $2::timestamptz + AND ($3::uuid IS NULL OR c.owner_id = $3::uuid) + ORDER BY COALESCE(NULLIF(cds.url, ''), c.id::text), c.created_at DESC, c.id DESC +) SELECT COUNT(*)::bigint AS total_prs_created, - COUNT(*) FILTER (WHERE cds.pull_request_state = 'merged')::bigint AS total_prs_merged, - COUNT(*) FILTER (WHERE cds.pull_request_state = 'closed')::bigint AS total_prs_closed, - COALESCE(SUM(cds.additions), 0)::bigint AS total_additions, - COALESCE(SUM(cds.deletions), 0)::bigint AS total_deletions, - COALESCE(SUM(cc.cost_micros), 0)::bigint AS total_cost_micros, - COALESCE(SUM(cc.cost_micros) FILTER (WHERE cds.pull_request_state = 'merged'), 0)::bigint AS merged_cost_micros -FROM chat_diff_statuses cds -JOIN chats c ON c.id = cds.chat_id -LEFT JOIN ( - SELECT - COALESCE(ch.root_chat_id, ch.id) AS root_id, - COALESCE(SUM(cm.total_cost_micros), 0) AS cost_micros - FROM chat_messages cm - JOIN chats ch ON ch.id = cm.chat_id - WHERE cm.total_cost_micros IS NOT NULL - GROUP BY COALESCE(ch.root_chat_id, ch.id) -) cc ON cc.root_id = COALESCE(c.root_chat_id, c.id) -WHERE cds.pull_request_state IS NOT NULL - AND c.created_at >= $1::timestamptz - AND c.created_at < $2::timestamptz - AND ($3::uuid IS NULL OR c.owner_id = $3::uuid) + COUNT(*) FILTER (WHERE d.pull_request_state = 'merged')::bigint AS total_prs_merged, + COUNT(*) FILTER (WHERE d.pull_request_state = 'closed')::bigint AS total_prs_closed, + COALESCE(SUM(d.additions), 0)::bigint AS total_additions, + COALESCE(SUM(d.deletions), 0)::bigint AS total_deletions, + COALESCE(SUM(pc.cost_micros), 0)::bigint AS total_cost_micros, + COALESCE(SUM(pc.cost_micros) FILTER (WHERE d.pull_request_state = 'merged'), 0)::bigint AS merged_cost_micros +FROM deduped d +JOIN pr_costs pc ON pc.pr_key = d.pr_key ` type GetPRInsightsSummaryParams struct { @@ -2662,8 +2787,22 @@ type GetPRInsightsSummaryRow struct { // PR Insights queries for the /agents analytics dashboard. // These aggregate data from chat_diff_statuses (PR metadata) joined // with chats and chat_messages (cost) to power the PR Insights view. +// +// Cost is computed per PR by summing the PR-linked chat's own cost plus +// the costs of any direct children (subagents) it spawned that do NOT +// have their own PR association. If a child chat has its own +// chat_diff_statuses entry (with a non-NULL pull_request_state), its +// cost is attributed to that child's PR instead — preventing +// double-counting when sibling chats create different PRs. +// Subagent trees are at most 2 levels deep (enforced by the +// application layer). PR metadata (state, additions, deletions) +// comes from the most recent chat via DISTINCT ON so that each PR +// is counted exactly once. // Returns aggregate PR metrics for the given date range. // The handler calls this twice (current + previous period) for trends. +// Uses two CTEs: pr_costs sums cost for the PR-linked chat and its +// direct children (that lack their own PR), and deduped picks one row +// per PR for state/additions/deletions. func (q *sqlQuerier) GetPRInsightsSummary(ctx context.Context, arg GetPRInsightsSummaryParams) (GetPRInsightsSummaryRow, error) { row := q.db.QueryRowContext(ctx, getPRInsightsSummary, arg.StartDate, arg.EndDate, arg.OwnerID) var i GetPRInsightsSummaryRow @@ -2680,19 +2819,26 @@ func (q *sqlQuerier) GetPRInsightsSummary(ctx context.Context, arg GetPRInsights } const getPRInsightsTimeSeries = `-- name: GetPRInsightsTimeSeries :many +WITH deduped AS ( + SELECT DISTINCT ON (COALESCE(NULLIF(cds.url, ''), c.id::text)) + cds.pull_request_state, + c.created_at + FROM chat_diff_statuses cds + JOIN chats c ON c.id = cds.chat_id + WHERE cds.pull_request_state IS NOT NULL + AND c.created_at >= $1::timestamptz + AND c.created_at < $2::timestamptz + AND ($3::uuid IS NULL OR c.owner_id = $3::uuid) + ORDER BY COALESCE(NULLIF(cds.url, ''), c.id::text), c.created_at DESC, c.id DESC +) SELECT - date_trunc('day', c.created_at)::timestamptz AS date, + date_trunc('day', created_at)::timestamptz AS date, COUNT(*)::bigint AS prs_created, - COUNT(*) FILTER (WHERE cds.pull_request_state = 'merged')::bigint AS prs_merged, - COUNT(*) FILTER (WHERE cds.pull_request_state = 'closed')::bigint AS prs_closed -FROM chat_diff_statuses cds -JOIN chats c ON c.id = cds.chat_id -WHERE cds.pull_request_state IS NOT NULL - AND c.created_at >= $1::timestamptz - AND c.created_at < $2::timestamptz - AND ($3::uuid IS NULL OR c.owner_id = $3::uuid) -GROUP BY date_trunc('day', c.created_at) -ORDER BY date_trunc('day', c.created_at) + COUNT(*) FILTER (WHERE pull_request_state = 'merged')::bigint AS prs_merged, + COUNT(*) FILTER (WHERE pull_request_state = 'closed')::bigint AS prs_closed +FROM deduped +GROUP BY date_trunc('day', created_at) +ORDER BY date_trunc('day', created_at) ` type GetPRInsightsTimeSeriesParams struct { @@ -2709,6 +2855,8 @@ type GetPRInsightsTimeSeriesRow struct { } // Returns daily PR counts grouped by state for the chart. +// Uses a CTE to deduplicate by PR URL so that multiple chats referencing +// the same pull request are only counted once (keeping the most recent chat). func (q *sqlQuerier) GetPRInsightsTimeSeries(ctx context.Context, arg GetPRInsightsTimeSeriesParams) ([]GetPRInsightsTimeSeriesRow, error) { rows, err := q.db.QueryContext(ctx, getPRInsightsTimeSeries, arg.StartDate, arg.EndDate, arg.OwnerID) if err != nil { diff --git a/coderd/database/queries/chatinsights.sql b/coderd/database/queries/chatinsights.sql index 7cdb48097b..deb8a6ea5a 100644 --- a/coderd/database/queries/chatinsights.sql +++ b/coderd/database/queries/chatinsights.sql @@ -1,118 +1,266 @@ -- PR Insights queries for the /agents analytics dashboard. -- These aggregate data from chat_diff_statuses (PR metadata) joined -- with chats and chat_messages (cost) to power the PR Insights view. +-- +-- Cost is computed per PR by summing the PR-linked chat's own cost plus +-- the costs of any direct children (subagents) it spawned that do NOT +-- have their own PR association. If a child chat has its own +-- chat_diff_statuses entry (with a non-NULL pull_request_state), its +-- cost is attributed to that child's PR instead — preventing +-- double-counting when sibling chats create different PRs. +-- Subagent trees are at most 2 levels deep (enforced by the +-- application layer). PR metadata (state, additions, deletions) +-- comes from the most recent chat via DISTINCT ON so that each PR +-- is counted exactly once. -- name: GetPRInsightsSummary :one -- Returns aggregate PR metrics for the given date range. -- The handler calls this twice (current + previous period) for trends. +-- Uses two CTEs: pr_costs sums cost for the PR-linked chat and its +-- direct children (that lack their own PR), and deduped picks one row +-- per PR for state/additions/deletions. +WITH pr_costs AS ( + SELECT + prc.pr_key, + COALESCE(SUM(cc.cost_micros), 0) AS cost_micros + FROM ( + -- For each PR, include the chat that references it plus any + -- direct children (subagents) that do not have their own PR. + SELECT DISTINCT + COALESCE(NULLIF(cds.url, ''), c.id::text) AS pr_key, + related.id AS chat_id + FROM chat_diff_statuses cds + JOIN chats c ON c.id = cds.chat_id + JOIN chats related + ON related.id = c.id + OR (related.parent_chat_id = c.id + AND NOT EXISTS ( + SELECT 1 FROM chat_diff_statuses cds2 + WHERE cds2.chat_id = related.id + AND cds2.pull_request_state IS NOT NULL + )) + WHERE cds.pull_request_state IS NOT NULL + AND c.created_at >= @start_date::timestamptz + AND c.created_at < @end_date::timestamptz + AND (sqlc.narg('owner_id')::uuid IS NULL OR c.owner_id = sqlc.narg('owner_id')::uuid) + ) prc + LEFT JOIN LATERAL ( + SELECT COALESCE(SUM(cm.total_cost_micros), 0) AS cost_micros + FROM chat_messages cm + WHERE cm.chat_id = prc.chat_id + AND cm.total_cost_micros IS NOT NULL + ) cc ON TRUE + GROUP BY prc.pr_key +), +deduped AS ( + SELECT DISTINCT ON (COALESCE(NULLIF(cds.url, ''), c.id::text)) + COALESCE(NULLIF(cds.url, ''), c.id::text) AS pr_key, + cds.pull_request_state, + cds.additions, + cds.deletions + FROM chat_diff_statuses cds + JOIN chats c ON c.id = cds.chat_id + WHERE cds.pull_request_state IS NOT NULL + AND c.created_at >= @start_date::timestamptz + AND c.created_at < @end_date::timestamptz + AND (sqlc.narg('owner_id')::uuid IS NULL OR c.owner_id = sqlc.narg('owner_id')::uuid) + ORDER BY COALESCE(NULLIF(cds.url, ''), c.id::text), c.created_at DESC, c.id DESC +) SELECT COUNT(*)::bigint AS total_prs_created, - COUNT(*) FILTER (WHERE cds.pull_request_state = 'merged')::bigint AS total_prs_merged, - COUNT(*) FILTER (WHERE cds.pull_request_state = 'closed')::bigint AS total_prs_closed, - COALESCE(SUM(cds.additions), 0)::bigint AS total_additions, - COALESCE(SUM(cds.deletions), 0)::bigint AS total_deletions, - COALESCE(SUM(cc.cost_micros), 0)::bigint AS total_cost_micros, - COALESCE(SUM(cc.cost_micros) FILTER (WHERE cds.pull_request_state = 'merged'), 0)::bigint AS merged_cost_micros -FROM chat_diff_statuses cds -JOIN chats c ON c.id = cds.chat_id -LEFT JOIN ( - SELECT - COALESCE(ch.root_chat_id, ch.id) AS root_id, - COALESCE(SUM(cm.total_cost_micros), 0) AS cost_micros - FROM chat_messages cm - JOIN chats ch ON ch.id = cm.chat_id - WHERE cm.total_cost_micros IS NOT NULL - GROUP BY COALESCE(ch.root_chat_id, ch.id) -) cc ON cc.root_id = COALESCE(c.root_chat_id, c.id) -WHERE cds.pull_request_state IS NOT NULL - AND c.created_at >= @start_date::timestamptz - AND c.created_at < @end_date::timestamptz - AND (sqlc.narg('owner_id')::uuid IS NULL OR c.owner_id = sqlc.narg('owner_id')::uuid); + COUNT(*) FILTER (WHERE d.pull_request_state = 'merged')::bigint AS total_prs_merged, + COUNT(*) FILTER (WHERE d.pull_request_state = 'closed')::bigint AS total_prs_closed, + COALESCE(SUM(d.additions), 0)::bigint AS total_additions, + COALESCE(SUM(d.deletions), 0)::bigint AS total_deletions, + COALESCE(SUM(pc.cost_micros), 0)::bigint AS total_cost_micros, + COALESCE(SUM(pc.cost_micros) FILTER (WHERE d.pull_request_state = 'merged'), 0)::bigint AS merged_cost_micros +FROM deduped d +JOIN pr_costs pc ON pc.pr_key = d.pr_key; -- name: GetPRInsightsTimeSeries :many -- Returns daily PR counts grouped by state for the chart. +-- Uses a CTE to deduplicate by PR URL so that multiple chats referencing +-- the same pull request are only counted once (keeping the most recent chat). +WITH deduped AS ( + SELECT DISTINCT ON (COALESCE(NULLIF(cds.url, ''), c.id::text)) + cds.pull_request_state, + c.created_at + FROM chat_diff_statuses cds + JOIN chats c ON c.id = cds.chat_id + WHERE cds.pull_request_state IS NOT NULL + AND c.created_at >= @start_date::timestamptz + AND c.created_at < @end_date::timestamptz + AND (sqlc.narg('owner_id')::uuid IS NULL OR c.owner_id = sqlc.narg('owner_id')::uuid) + ORDER BY COALESCE(NULLIF(cds.url, ''), c.id::text), c.created_at DESC, c.id DESC +) SELECT - date_trunc('day', c.created_at)::timestamptz AS date, + date_trunc('day', created_at)::timestamptz AS date, COUNT(*)::bigint AS prs_created, - COUNT(*) FILTER (WHERE cds.pull_request_state = 'merged')::bigint AS prs_merged, - COUNT(*) FILTER (WHERE cds.pull_request_state = 'closed')::bigint AS prs_closed -FROM chat_diff_statuses cds -JOIN chats c ON c.id = cds.chat_id -WHERE cds.pull_request_state IS NOT NULL - AND c.created_at >= @start_date::timestamptz - AND c.created_at < @end_date::timestamptz - AND (sqlc.narg('owner_id')::uuid IS NULL OR c.owner_id = sqlc.narg('owner_id')::uuid) -GROUP BY date_trunc('day', c.created_at) -ORDER BY date_trunc('day', c.created_at); + COUNT(*) FILTER (WHERE pull_request_state = 'merged')::bigint AS prs_merged, + COUNT(*) FILTER (WHERE pull_request_state = 'closed')::bigint AS prs_closed +FROM deduped +GROUP BY date_trunc('day', created_at) +ORDER BY date_trunc('day', created_at); -- name: GetPRInsightsPerModel :many -- Returns PR metrics grouped by the model used for each chat. -SELECT - cmc.id AS model_config_id, - cmc.display_name, - cmc.provider, - COUNT(*)::bigint AS total_prs, - COUNT(*) FILTER (WHERE cds.pull_request_state = 'merged')::bigint AS merged_prs, - COALESCE(SUM(cds.additions), 0)::bigint AS total_additions, - COALESCE(SUM(cds.deletions), 0)::bigint AS total_deletions, - COALESCE(SUM(cc.cost_micros), 0)::bigint AS total_cost_micros, - COALESCE(SUM(cc.cost_micros) FILTER (WHERE cds.pull_request_state = 'merged'), 0)::bigint AS merged_cost_micros -FROM chat_diff_statuses cds -JOIN chats c ON c.id = cds.chat_id -JOIN chat_model_configs cmc ON cmc.id = c.last_model_config_id -LEFT JOIN ( +-- Uses two CTEs: pr_costs sums cost for the PR-linked chat and its +-- direct children (that lack their own PR), and deduped picks one row +-- per PR for state/additions/deletions/model (model comes from the +-- most recent chat). +WITH pr_costs AS ( SELECT - COALESCE(ch.root_chat_id, ch.id) AS root_id, - COALESCE(SUM(cm.total_cost_micros), 0) AS cost_micros - FROM chat_messages cm - JOIN chats ch ON ch.id = cm.chat_id - WHERE cm.total_cost_micros IS NOT NULL - GROUP BY COALESCE(ch.root_chat_id, ch.id) -) cc ON cc.root_id = COALESCE(c.root_chat_id, c.id) -WHERE cds.pull_request_state IS NOT NULL - AND c.created_at >= @start_date::timestamptz - AND c.created_at < @end_date::timestamptz - AND (sqlc.narg('owner_id')::uuid IS NULL OR c.owner_id = sqlc.narg('owner_id')::uuid) -GROUP BY cmc.id, cmc.display_name, cmc.provider + prc.pr_key, + COALESCE(SUM(cc.cost_micros), 0) AS cost_micros + FROM ( + SELECT DISTINCT + COALESCE(NULLIF(cds.url, ''), c.id::text) AS pr_key, + related.id AS chat_id + FROM chat_diff_statuses cds + JOIN chats c ON c.id = cds.chat_id + JOIN chats related + ON related.id = c.id + OR (related.parent_chat_id = c.id + AND NOT EXISTS ( + SELECT 1 FROM chat_diff_statuses cds2 + WHERE cds2.chat_id = related.id + AND cds2.pull_request_state IS NOT NULL + )) + WHERE cds.pull_request_state IS NOT NULL + AND c.created_at >= @start_date::timestamptz + AND c.created_at < @end_date::timestamptz + AND (sqlc.narg('owner_id')::uuid IS NULL OR c.owner_id = sqlc.narg('owner_id')::uuid) + ) prc + LEFT JOIN LATERAL ( + SELECT COALESCE(SUM(cm.total_cost_micros), 0) AS cost_micros + FROM chat_messages cm + WHERE cm.chat_id = prc.chat_id + AND cm.total_cost_micros IS NOT NULL + ) cc ON TRUE + GROUP BY prc.pr_key +), +deduped AS ( + SELECT DISTINCT ON (COALESCE(NULLIF(cds.url, ''), c.id::text)) + COALESCE(NULLIF(cds.url, ''), c.id::text) AS pr_key, + cds.pull_request_state, + cds.additions, + cds.deletions, + cmc.id AS model_config_id, + cmc.display_name, + cmc.provider + FROM chat_diff_statuses cds + JOIN chats c ON c.id = cds.chat_id + LEFT JOIN chat_model_configs cmc ON cmc.id = c.last_model_config_id + WHERE cds.pull_request_state IS NOT NULL + AND c.created_at >= @start_date::timestamptz + AND c.created_at < @end_date::timestamptz + AND (sqlc.narg('owner_id')::uuid IS NULL OR c.owner_id = sqlc.narg('owner_id')::uuid) + ORDER BY COALESCE(NULLIF(cds.url, ''), c.id::text), c.created_at DESC, c.id DESC +) +SELECT + d.model_config_id, + COALESCE(d.display_name, 'Unknown')::text AS display_name, + COALESCE(d.provider, 'unknown')::text AS provider, + COUNT(*)::bigint AS total_prs, + COUNT(*) FILTER (WHERE d.pull_request_state = 'merged')::bigint AS merged_prs, + COALESCE(SUM(d.additions), 0)::bigint AS total_additions, + COALESCE(SUM(d.deletions), 0)::bigint AS total_deletions, + COALESCE(SUM(pc.cost_micros), 0)::bigint AS total_cost_micros, + COALESCE(SUM(pc.cost_micros) FILTER (WHERE d.pull_request_state = 'merged'), 0)::bigint AS merged_cost_micros +FROM deduped d +JOIN pr_costs pc ON pc.pr_key = d.pr_key +GROUP BY d.model_config_id, d.display_name, d.provider ORDER BY total_prs DESC; -- name: GetPRInsightsRecentPRs :many -- Returns individual PR rows with cost for the recent PRs table. -SELECT - c.id AS chat_id, - cds.pull_request_title AS pr_title, - cds.url AS pr_url, - cds.pr_number, - cds.pull_request_state AS state, - cds.pull_request_draft AS draft, - cds.additions, - cds.deletions, - cds.changed_files, - cds.commits, - cds.approved, - cds.changes_requested, - cds.reviewer_count, - cds.author_login, - cds.author_avatar_url, - COALESCE(cds.base_branch, '')::text AS base_branch, - COALESCE(cmc.display_name, cmc.model)::text AS model_display_name, - COALESCE(cc.cost_micros, 0)::bigint AS cost_micros, - c.created_at -FROM chat_diff_statuses cds -JOIN chats c ON c.id = cds.chat_id -JOIN chat_model_configs cmc ON cmc.id = c.last_model_config_id -LEFT JOIN ( +-- Uses two CTEs: pr_costs sums cost for the PR-linked chat and its +-- direct children (that lack their own PR), and deduped picks one row +-- per PR for metadata. +WITH pr_costs AS ( SELECT - COALESCE(ch.root_chat_id, ch.id) AS root_id, - COALESCE(SUM(cm.total_cost_micros), 0) AS cost_micros - FROM chat_messages cm - JOIN chats ch ON ch.id = cm.chat_id - WHERE cm.total_cost_micros IS NOT NULL - GROUP BY COALESCE(ch.root_chat_id, ch.id) -) cc ON cc.root_id = COALESCE(c.root_chat_id, c.id) -WHERE cds.pull_request_state IS NOT NULL - AND c.created_at >= @start_date::timestamptz - AND c.created_at < @end_date::timestamptz - AND (sqlc.narg('owner_id')::uuid IS NULL OR c.owner_id = sqlc.narg('owner_id')::uuid) -ORDER BY c.created_at DESC + prc.pr_key, + COALESCE(SUM(cc.cost_micros), 0) AS cost_micros + FROM ( + SELECT DISTINCT + COALESCE(NULLIF(cds.url, ''), c.id::text) AS pr_key, + related.id AS chat_id + FROM chat_diff_statuses cds + JOIN chats c ON c.id = cds.chat_id + JOIN chats related + ON related.id = c.id + OR (related.parent_chat_id = c.id + AND NOT EXISTS ( + SELECT 1 FROM chat_diff_statuses cds2 + WHERE cds2.chat_id = related.id + AND cds2.pull_request_state IS NOT NULL + )) + WHERE cds.pull_request_state IS NOT NULL + AND c.created_at >= @start_date::timestamptz + AND c.created_at < @end_date::timestamptz + AND (sqlc.narg('owner_id')::uuid IS NULL OR c.owner_id = sqlc.narg('owner_id')::uuid) + ) prc + LEFT JOIN LATERAL ( + SELECT COALESCE(SUM(cm.total_cost_micros), 0) AS cost_micros + FROM chat_messages cm + WHERE cm.chat_id = prc.chat_id + AND cm.total_cost_micros IS NOT NULL + ) cc ON TRUE + GROUP BY prc.pr_key +), +deduped AS ( + SELECT DISTINCT ON (COALESCE(NULLIF(cds.url, ''), c.id::text)) + COALESCE(NULLIF(cds.url, ''), c.id::text) AS pr_key, + c.id AS chat_id, + cds.pull_request_title AS pr_title, + cds.url AS pr_url, + cds.pr_number, + cds.pull_request_state AS state, + cds.pull_request_draft AS draft, + cds.additions, + cds.deletions, + cds.changed_files, + cds.commits, + cds.approved, + cds.changes_requested, + cds.reviewer_count, + cds.author_login, + cds.author_avatar_url, + COALESCE(cds.base_branch, '')::text AS base_branch, + COALESCE(cmc.display_name, cmc.model, 'Unknown')::text AS model_display_name, + c.created_at + FROM chat_diff_statuses cds + JOIN chats c ON c.id = cds.chat_id + LEFT JOIN chat_model_configs cmc ON cmc.id = c.last_model_config_id + WHERE cds.pull_request_state IS NOT NULL + AND c.created_at >= @start_date::timestamptz + AND c.created_at < @end_date::timestamptz + AND (sqlc.narg('owner_id')::uuid IS NULL OR c.owner_id = sqlc.narg('owner_id')::uuid) + ORDER BY COALESCE(NULLIF(cds.url, ''), c.id::text), c.created_at DESC, c.id DESC +) +SELECT * FROM ( + SELECT + d.chat_id, + d.pr_title, + d.pr_url, + d.pr_number, + d.state, + d.draft, + d.additions, + d.deletions, + d.changed_files, + d.commits, + d.approved, + d.changes_requested, + d.reviewer_count, + d.author_login, + d.author_avatar_url, + d.base_branch, + d.model_display_name, + COALESCE(pc.cost_micros, 0)::bigint AS cost_micros, + d.created_at + FROM deduped d + JOIN pr_costs pc ON pc.pr_key = d.pr_key +) sub +ORDER BY sub.created_at DESC LIMIT @limit_val::int; diff --git a/site/src/pages/AgentsPage/PRInsightsView.tsx b/site/src/pages/AgentsPage/PRInsightsView.tsx index b238ff4bde..c0094b50b9 100644 --- a/site/src/pages/AgentsPage/PRInsightsView.tsx +++ b/site/src/pages/AgentsPage/PRInsightsView.tsx @@ -16,20 +16,11 @@ import { } from "components/Table/Table"; import dayjs from "dayjs"; import relativeTime from "dayjs/plugin/relativeTime"; -import { - ArrowDownRightIcon, - ArrowUpRightIcon, - CheckCircle2Icon, - CircleDotIcon, - CodeIcon, - ExternalLinkIcon, - MessageSquareTextIcon, -} from "lucide-react"; +import { CodeIcon, ExternalLinkIcon } from "lucide-react"; import type { FC } from "react"; import { Area, AreaChart, CartesianGrid, XAxis, YAxis } from "recharts"; import { cn } from "utils/cn"; import { formatCostMicros } from "utils/currency"; -import { DiffStatBadge } from "./DiffStats"; import { PrStateIcon } from "./GitPanel"; dayjs.extend(relativeTime); @@ -50,81 +41,28 @@ interface PRInsightsViewProps { // Helpers // --------------------------------------------------------------------------- -function pctChange(current: number, previous: number): number | null { - if (previous === 0) return current > 0 ? 100 : null; - return ((current - previous) / previous) * 100; -} - -function formatPct(value: number): string { - return `${value >= 0 ? "+" : ""}${Math.round(value)}%`; -} - function formatMergeRate(rate: number): string { return `${Math.round(rate * 100)}%`; } -function formatLinesShipped(n: number): string { - if (n >= 1_000_000) return `${(n / 1_000_000).toFixed(1)}M`; - if (n >= 1_000) return `${(n / 1_000).toFixed(1)}k`; - return n.toLocaleString(); -} - // --------------------------------------------------------------------------- // Sub-components // --------------------------------------------------------------------------- -const TrendBadge: FC<{ - current: number; - previous: number; - invert?: boolean; -}> = ({ current, previous, invert = false }) => { - const change = pctChange(current, previous); - if (change === null) return null; - - const isPositive = invert ? change < 0 : change > 0; - const isNegative = invert ? change > 0 : change < 0; - - if (isPositive) { - return ( - - - {formatPct(change)} - - ); - } - if (isNegative) { - return ( - - - {formatPct(change)} - - ); - } - return ( - - 0% - - ); -}; - const StatCard: FC<{ label: string; value: string; - trend?: React.ReactNode; detail?: string; -}> = ({ label, value, trend, detail }) => ( +}> = ({ label, value, detail }) => (

{label}

-
-

- {value} -

- {trend} -
- {detail && ( -

{detail}

- )} +

+ {value} +

+

+ {detail ?? "\u00A0"} +

); @@ -169,7 +107,7 @@ const PRStateBadge: FC<{ state: string; draft: boolean }> = ({ const InlineMergeBar: FC<{ rate: number }> = ({ rate }) => (
-
+
= ({ rate }) => ( const activityChartConfig = { prs_created: { label: "Created", - color: "hsl(var(--git-added-bright))", + color: "hsl(var(--content-disabled))", }, prs_merged: { label: "Merged", color: "hsl(var(--git-merged-bright))", }, - prs_closed: { - label: "Closed", - color: "hsl(var(--git-deleted-bright))", - }, } satisfies ChartConfig; function formatChartDate(dateStr: string): string { @@ -205,7 +139,7 @@ function formatChartDate(dateStr: string): string { } // --------------------------------------------------------------------------- -// Activity chart +// Activity chart — simplified to created vs merged // --------------------------------------------------------------------------- const ActivityChart: FC<{ @@ -215,14 +149,14 @@ const ActivityChart: FC<{ - - - - (v === 0 ? "" : String(v))} /> @@ -285,7 +208,8 @@ const ActivityChart: FC<{ fill="url(#fillCreated)" fillOpacity={1} stroke="var(--color-prs_created)" - strokeWidth={1.5} + strokeWidth={1} + strokeDasharray="4 3" /> - ); @@ -367,41 +282,6 @@ const TimeRangeFilter: FC<{
); -const ReviewBadge: FC<{ - approved: boolean | undefined; - changes_requested: boolean; - reviewer_count: number | undefined; -}> = ({ approved, changes_requested, reviewer_count }) => { - if (!reviewer_count) { - return No reviews; - } - - if (approved === true && !changes_requested) { - return ( - - - {reviewer_count} approved - - ); - } - - if (changes_requested) { - return ( - - - Changes requested - - ); - } - - return ( - - - {reviewer_count} reviewing - - ); -}; - // --------------------------------------------------------------------------- // Main view // --------------------------------------------------------------------------- @@ -415,7 +295,7 @@ export const PRInsightsView: FC = ({ const isEmpty = summary.total_prs_created === 0; return ( -
+
{/* ── Header ── */}
@@ -423,7 +303,7 @@ export const PRInsightsView: FC = ({ Pull Request Insights

- Code shipped by AI agents across your organization. + Code changes detected by Agents.

@@ -433,54 +313,22 @@ export const PRInsightsView: FC = ({ ) : ( <> - {/* ── Stat cards ── */} -
- - } - /> + {/* ── Stat cards — 3 headline metrics ── */} +
- } + detail={`${summary.total_prs_created.toLocaleString()} created`} /> - } /> - - } - /> + detail={`${formatCostMicros(summary.total_cost_micros)} total`} + />{" "}
{/* ── Activity chart ── */} @@ -501,177 +349,140 @@ export const PRInsightsView: FC = ({ ))}
-
+
- {/* ── Model performance ── */} - {by_model.length > 0 && ( -
-
- Performance by model -
-
- - - - Model - - PRs - - - Merged - - Merge rate - - Changes - - - Total cost - - - Cost / merge - - - - - {by_model.map((m) => ( - - - - {m.display_name} - - - {m.provider} - - - - {m.total_prs} - - - {m.merged_prs} - - - - - - - - - {formatCostMicros(m.total_cost_micros)} - - - {m.merged_prs > 0 - ? formatCostMicros(m.cost_per_merged_pr_micros) - : "—"} - + {/* ── Model breakdown + Recent PRs side by side ── */} +
+ {/* ── Model performance (simplified) ── */} + {by_model.length > 0 && ( +
+
+ By model +
+
+
+ + + Model + + Merged + + Merge rate + + Cost / merge + - ))} - -
-
-
- )} + + + {by_model.map((m) => ( + + + + {m.display_name} + + + + {m.merged_prs} + + /{m.total_prs} + + + + + + + {m.merged_prs > 0 + ? formatCostMicros(m.cost_per_merged_pr_micros) + : "—"} + + + ))} + + +
+ + )} - {/* ── Recent pull requests ── */} - {recent_prs.length > 0 && ( -
-
- Recent pull requests -
-
- - - - Pull request - Status - - Changes - - - Reviews - - Model - - Cost - - - Created - - - - - {recent_prs.map((pr) => ( - - - - {pr.pr_title} - - -
- - {pr.author_login} - · - #{pr.pr_number} - → - {pr.base_branch} -
- - - - - - -

- {pr.changed_files} file - {pr.changed_files !== 1 ? "s" : ""} -

- - - - {" "} - - {pr.model_display_name} - - - {formatCostMicros(pr.cost_micros)} - - - {dayjs(pr.created_at).format("MMM D, h:mm A")} - + {/* ── Recent pull requests (simplified) ── */} + {recent_prs.length > 0 && ( +
+
+ Recent +
+
+
+ + + + + + + + + Title + Status + + Cost + + + Created + - ))} - -
-
-
- )} + {" "} + + {recent_prs.map((pr) => ( + + + {" "} + + + {pr.pr_title} + + + +
+ {" "} + + {pr.author_login} + · + #{pr.pr_number} +
+ + + + + + {formatCostMicros(pr.cost_micros)} + + + {dayjs(pr.created_at).format("MMM D")} + + + ))} + + +
+ + )} +
)}