diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 64376e87b0..c361282c75 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -3401,11 +3401,11 @@ func (q *querier) GetPRInsightsPerModel(ctx context.Context, arg database.GetPRI return q.db.GetPRInsightsPerModel(ctx, arg) } -func (q *querier) GetPRInsightsRecentPRs(ctx context.Context, arg database.GetPRInsightsRecentPRsParams) ([]database.GetPRInsightsRecentPRsRow, error) { +func (q *querier) GetPRInsightsPullRequests(ctx context.Context, arg database.GetPRInsightsPullRequestsParams) ([]database.GetPRInsightsPullRequestsRow, error) { if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceDeploymentConfig); err != nil { return nil, err } - return q.db.GetPRInsightsRecentPRs(ctx, arg) + return q.db.GetPRInsightsPullRequests(ctx, arg) } func (q *querier) GetPRInsightsSummary(ctx context.Context, arg database.GetPRInsightsSummaryParams) (database.GetPRInsightsSummaryRow, error) { diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 97e0942736..b156a84e2b 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -2261,9 +2261,9 @@ func (s *MethodTestSuite) TestTemplate() { dbm.EXPECT().GetPRInsightsPerModel(gomock.Any(), arg).Return([]database.GetPRInsightsPerModelRow{}, nil).AnyTimes() check.Args(arg).Asserts(rbac.ResourceDeploymentConfig, policy.ActionRead) })) - s.Run("GetPRInsightsRecentPRs", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { - arg := database.GetPRInsightsRecentPRsParams{} - dbm.EXPECT().GetPRInsightsRecentPRs(gomock.Any(), arg).Return([]database.GetPRInsightsRecentPRsRow{}, nil).AnyTimes() + s.Run("GetPRInsightsPullRequests", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { + arg := database.GetPRInsightsPullRequestsParams{} + dbm.EXPECT().GetPRInsightsPullRequests(gomock.Any(), arg).Return([]database.GetPRInsightsPullRequestsRow{}, nil).AnyTimes() check.Args(arg).Asserts(rbac.ResourceDeploymentConfig, policy.ActionRead) })) s.Run("GetTelemetryTaskEvents", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 506defebaa..3f029c8cd6 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -1992,11 +1992,11 @@ func (m queryMetricsStore) GetPRInsightsPerModel(ctx context.Context, arg databa return r0, r1 } -func (m queryMetricsStore) GetPRInsightsRecentPRs(ctx context.Context, arg database.GetPRInsightsRecentPRsParams) ([]database.GetPRInsightsRecentPRsRow, error) { +func (m queryMetricsStore) GetPRInsightsPullRequests(ctx context.Context, arg database.GetPRInsightsPullRequestsParams) ([]database.GetPRInsightsPullRequestsRow, error) { start := time.Now() - r0, r1 := m.s.GetPRInsightsRecentPRs(ctx, arg) - m.queryLatencies.WithLabelValues("GetPRInsightsRecentPRs").Observe(time.Since(start).Seconds()) - m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "GetPRInsightsRecentPRs").Inc() + r0, r1 := m.s.GetPRInsightsPullRequests(ctx, arg) + m.queryLatencies.WithLabelValues("GetPRInsightsPullRequests").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "GetPRInsightsPullRequests").Inc() return r0, r1 } diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index e99ba8a929..4a719331c5 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -3692,19 +3692,19 @@ func (mr *MockStoreMockRecorder) GetPRInsightsPerModel(ctx, arg any) *gomock.Cal return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPRInsightsPerModel", reflect.TypeOf((*MockStore)(nil).GetPRInsightsPerModel), ctx, arg) } -// GetPRInsightsRecentPRs mocks base method. -func (m *MockStore) GetPRInsightsRecentPRs(ctx context.Context, arg database.GetPRInsightsRecentPRsParams) ([]database.GetPRInsightsRecentPRsRow, error) { +// GetPRInsightsPullRequests mocks base method. +func (m *MockStore) GetPRInsightsPullRequests(ctx context.Context, arg database.GetPRInsightsPullRequestsParams) ([]database.GetPRInsightsPullRequestsRow, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetPRInsightsRecentPRs", ctx, arg) - ret0, _ := ret[0].([]database.GetPRInsightsRecentPRsRow) + ret := m.ctrl.Call(m, "GetPRInsightsPullRequests", ctx, arg) + ret0, _ := ret[0].([]database.GetPRInsightsPullRequestsRow) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetPRInsightsRecentPRs indicates an expected call of GetPRInsightsRecentPRs. -func (mr *MockStoreMockRecorder) GetPRInsightsRecentPRs(ctx, arg any) *gomock.Call { +// GetPRInsightsPullRequests indicates an expected call of GetPRInsightsPullRequests. +func (mr *MockStoreMockRecorder) GetPRInsightsPullRequests(ctx, arg any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPRInsightsRecentPRs", reflect.TypeOf((*MockStore)(nil).GetPRInsightsRecentPRs), ctx, arg) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPRInsightsPullRequests", reflect.TypeOf((*MockStore)(nil).GetPRInsightsPullRequests), ctx, arg) } // GetPRInsightsSummary mocks base method. diff --git a/coderd/database/querier.go b/coderd/database/querier.go index d8802d6dba..162bfcd93b 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -418,11 +418,12 @@ type sqlcQuerier interface { // 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. + // Returns all individual PR rows with cost for the selected time range. // 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) + // per PR for metadata. A safety-cap LIMIT guards against unexpectedly + // large result sets from direct API callers. + GetPRInsightsPullRequests(ctx context.Context, arg GetPRInsightsPullRequestsParams) ([]GetPRInsightsPullRequestsRow, 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. diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index b1f52054cc..20b5635faf 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -10408,11 +10408,10 @@ func TestGetPRInsights(t *testing.T) { assert.Equal(t, int64(1), summary.TotalPrsCreated) assert.Equal(t, int64(8_000_000), summary.TotalCostMicros) - recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + recent, err := store.GetPRInsightsPullRequests(context.Background(), database.GetPRInsightsPullRequestsParams{ StartDate: startDate, EndDate: endDate, OwnerID: noOwner, - LimitVal: 20, }) require.NoError(t, err) require.Len(t, recent, 1) @@ -10442,11 +10441,10 @@ func TestGetPRInsights(t *testing.T) { assert.Equal(t, int64(1), summary.TotalPrsMerged) // RecentPRs ordered by created_at DESC: chatB is newer. - recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + recent, err := store.GetPRInsightsPullRequests(context.Background(), database.GetPRInsightsPullRequestsParams{ StartDate: startDate, EndDate: endDate, OwnerID: noOwner, - LimitVal: 20, }) require.NoError(t, err) require.Len(t, recent, 2) @@ -10491,11 +10489,10 @@ func TestGetPRInsights(t *testing.T) { assert.Equal(t, int64(1), summary.TotalPrsCreated) assert.Equal(t, int64(1), summary.TotalPrsMerged) - recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + recent, err := store.GetPRInsightsPullRequests(context.Background(), database.GetPRInsightsPullRequestsParams{ StartDate: startDate, EndDate: endDate, OwnerID: noOwner, - LimitVal: 20, }) require.NoError(t, err) require.Len(t, recent, 1) @@ -10533,11 +10530,10 @@ func TestGetPRInsights(t *testing.T) { 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{ + recent, err := store.GetPRInsightsPullRequests(context.Background(), database.GetPRInsightsPullRequestsParams{ StartDate: startDate, EndDate: endDate, OwnerID: noOwner, - LimitVal: 20, }) require.NoError(t, err) require.Len(t, recent, 1) @@ -10575,11 +10571,10 @@ func TestGetPRInsights(t *testing.T) { assert.Equal(t, int64(2), summary.TotalPrsCreated) assert.Equal(t, int64(8_000_000), summary.TotalCostMicros) - recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + recent, err := store.GetPRInsightsPullRequests(context.Background(), database.GetPRInsightsPullRequestsParams{ StartDate: startDate, EndDate: endDate, OwnerID: noOwner, - LimitVal: 20, }) require.NoError(t, err) require.Len(t, recent, 2) @@ -10621,11 +10616,10 @@ func TestGetPRInsights(t *testing.T) { assert.Equal(t, int64(2), summary.TotalPrsCreated) assert.Equal(t, int64(17_000_000), summary.TotalCostMicros) - recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + recent, err := store.GetPRInsightsPullRequests(context.Background(), database.GetPRInsightsPullRequestsParams{ StartDate: startDate, EndDate: endDate, OwnerID: noOwner, - LimitVal: 20, }) require.NoError(t, err) require.Len(t, recent, 2) @@ -10658,11 +10652,10 @@ func TestGetPRInsights(t *testing.T) { assert.Equal(t, int64(2), summary.TotalPrsCreated) assert.Equal(t, int64(10_000_000), summary.TotalCostMicros) - recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + recent, err := store.GetPRInsightsPullRequests(context.Background(), database.GetPRInsightsPullRequestsParams{ StartDate: startDate, EndDate: endDate, OwnerID: noOwner, - LimitVal: 20, }) require.NoError(t, err) require.Len(t, recent, 2) @@ -10695,11 +10688,10 @@ func TestGetPRInsights(t *testing.T) { assert.Equal(t, int64(1), summary.TotalPrsCreated) assert.Equal(t, int64(15_000_000), summary.TotalCostMicros) - recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + recent, err := store.GetPRInsightsPullRequests(context.Background(), database.GetPRInsightsPullRequestsParams{ StartDate: startDate, EndDate: endDate, OwnerID: noOwner, - LimitVal: 20, }) require.NoError(t, err) require.Len(t, recent, 1) @@ -10724,11 +10716,10 @@ func TestGetPRInsights(t *testing.T) { assert.Equal(t, int64(1), summary.TotalPrsCreated) assert.Equal(t, int64(0), summary.TotalCostMicros) - recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + recent, err := store.GetPRInsightsPullRequests(context.Background(), database.GetPRInsightsPullRequestsParams{ StartDate: startDate, EndDate: endDate, OwnerID: noOwner, - LimitVal: 20, }) require.NoError(t, err) require.Len(t, recent, 1) @@ -10767,11 +10758,10 @@ func TestGetPRInsights(t *testing.T) { require.Len(t, byModel, 1) assert.Equal(t, modelName, byModel[0].DisplayName) - recent, err := store.GetPRInsightsRecentPRs(context.Background(), database.GetPRInsightsRecentPRsParams{ + recent, err := store.GetPRInsightsPullRequests(context.Background(), database.GetPRInsightsPullRequestsParams{ StartDate: startDate, EndDate: endDate, OwnerID: noOwner, - LimitVal: 20, }) require.NoError(t, err) require.Len(t, recent, 1) @@ -10803,6 +10793,30 @@ func TestGetPRInsights(t *testing.T) { assert.Equal(t, int64(8_000_000), summary.TotalCostMicros) assert.Equal(t, int64(5_000_000), summary.MergedCostMicros) }) + + t.Run("AllPRsReturnedWithSafetyCap", func(t *testing.T) { + t.Parallel() + store, userID, mcID := setupChatInfra(t) + + // Create 25 distinct PRs — more than the old LIMIT 20 — and + // verify all are returned. + const prCount = 25 + for i := range prCount { + 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, + fmt.Sprintf("https://github.com/org/repo/pull/%d", 100+i), + "merged", fmt.Sprintf("fix: pr-%d", i), 10, 2, 1) + } + + recent, err := store.GetPRInsightsPullRequests(context.Background(), database.GetPRInsightsPullRequestsParams{ + StartDate: startDate, + EndDate: endDate, + OwnerID: noOwner, + }) + require.NoError(t, err) + assert.Len(t, recent, prCount, "all PRs within the date range should be returned") + }) } func TestChatPinOrderQueries(t *testing.T) { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 98138e5fac..14a3bbbc12 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3218,7 +3218,7 @@ func (q *sqlQuerier) GetPRInsightsPerModel(ctx context.Context, arg GetPRInsight return items, nil } -const getPRInsightsRecentPRs = `-- name: GetPRInsightsRecentPRs :many +const getPRInsightsPullRequests = `-- name: GetPRInsightsPullRequests :many WITH pr_costs AS ( SELECT prc.pr_key, @@ -3238,9 +3238,9 @@ WITH pr_costs AS ( 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) + 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 @@ -3275,9 +3275,9 @@ deduped AS ( 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) + 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 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 ( @@ -3305,17 +3305,16 @@ SELECT chat_id, pr_title, pr_url, pr_number, state, draft, additions, deletions, JOIN pr_costs pc ON pc.pr_key = d.pr_key ) sub ORDER BY sub.created_at DESC -LIMIT $1::int +LIMIT 500 ` -type GetPRInsightsRecentPRsParams struct { - LimitVal int32 `db:"limit_val" json:"limit_val"` +type GetPRInsightsPullRequestsParams struct { 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"` } -type GetPRInsightsRecentPRsRow struct { +type GetPRInsightsPullRequestsRow struct { ChatID uuid.UUID `db:"chat_id" json:"chat_id"` PrTitle string `db:"pr_title" json:"pr_title"` PrUrl sql.NullString `db:"pr_url" json:"pr_url"` @@ -3337,24 +3336,20 @@ type GetPRInsightsRecentPRsRow struct { CreatedAt time.Time `db:"created_at" json:"created_at"` } -// Returns individual PR rows with cost for the recent PRs table. +// Returns all individual PR rows with cost for the selected time range. // 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, - ) +// per PR for metadata. A safety-cap LIMIT guards against unexpectedly +// large result sets from direct API callers. +func (q *sqlQuerier) GetPRInsightsPullRequests(ctx context.Context, arg GetPRInsightsPullRequestsParams) ([]GetPRInsightsPullRequestsRow, error) { + rows, err := q.db.QueryContext(ctx, getPRInsightsPullRequests, arg.StartDate, arg.EndDate, arg.OwnerID) if err != nil { return nil, err } defer rows.Close() - var items []GetPRInsightsRecentPRsRow + var items []GetPRInsightsPullRequestsRow for rows.Next() { - var i GetPRInsightsRecentPRsRow + var i GetPRInsightsPullRequestsRow if err := rows.Scan( &i.ChatID, &i.PrTitle, diff --git a/coderd/database/queries/chatinsights.sql b/coderd/database/queries/chatinsights.sql index 895b4e681a..9eda12a41a 100644 --- a/coderd/database/queries/chatinsights.sql +++ b/coderd/database/queries/chatinsights.sql @@ -173,11 +173,12 @@ JOIN pr_costs pc ON pc.pr_key = d.pr_key GROUP BY d.model_config_id, d.display_name, d.model, d.provider ORDER BY total_prs DESC; --- name: GetPRInsightsRecentPRs :many --- Returns individual PR rows with cost for the recent PRs table. +-- name: GetPRInsightsPullRequests :many +-- Returns all individual PR rows with cost for the selected time range. -- 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. +-- per PR for metadata. A safety-cap LIMIT guards against unexpectedly +-- large result sets from direct API callers. WITH pr_costs AS ( SELECT prc.pr_key, @@ -264,4 +265,4 @@ SELECT * FROM ( JOIN pr_costs pc ON pc.pr_key = d.pr_key ) sub ORDER BY sub.created_at DESC -LIMIT @limit_val::int; +LIMIT 500; diff --git a/coderd/exp_chats.go b/coderd/exp_chats.go index 67e219bbfd..deb554fcc8 100644 --- a/coderd/exp_chats.go +++ b/coderd/exp_chats.go @@ -5626,7 +5626,7 @@ func (api *API) prInsights(rw http.ResponseWriter, r *http.Request) { previousSummary database.GetPRInsightsSummaryRow timeSeries []database.GetPRInsightsTimeSeriesRow byModel []database.GetPRInsightsPerModelRow - recentPRs []database.GetPRInsightsRecentPRsRow + recentPRs []database.GetPRInsightsPullRequestsRow ) eg, egCtx := errgroup.WithContext(ctx) @@ -5674,11 +5674,10 @@ func (api *API) prInsights(rw http.ResponseWriter, r *http.Request) { eg.Go(func() error { var err error - recentPRs, err = api.Database.GetPRInsightsRecentPRs(egCtx, database.GetPRInsightsRecentPRsParams{ + recentPRs, err = api.Database.GetPRInsightsPullRequests(egCtx, database.GetPRInsightsPullRequestsParams{ StartDate: startDate, EndDate: endDate, OwnerID: ownerID, - LimitVal: 20, }) return err }) @@ -5788,10 +5787,10 @@ func (api *API) prInsights(rw http.ResponseWriter, r *http.Request) { } httpapi.Write(ctx, rw, http.StatusOK, codersdk.PRInsightsResponse{ - Summary: summary, - TimeSeries: tsEntries, - ByModel: modelEntries, - RecentPRs: prEntries, + Summary: summary, + TimeSeries: tsEntries, + ByModel: modelEntries, + PullRequests: prEntries, }) } diff --git a/codersdk/chats.go b/codersdk/chats.go index 676c66da88..e73f32d2bf 100644 --- a/codersdk/chats.go +++ b/codersdk/chats.go @@ -2411,10 +2411,10 @@ func (c *ExperimentalClient) GetChatsByWorkspace(ctx context.Context, workspaceI // PRInsightsResponse is the response from the PR insights endpoint. type PRInsightsResponse struct { - Summary PRInsightsSummary `json:"summary"` - TimeSeries []PRInsightsTimeSeriesEntry `json:"time_series"` - ByModel []PRInsightsModelBreakdown `json:"by_model"` - RecentPRs []PRInsightsPullRequest `json:"recent_prs"` + Summary PRInsightsSummary `json:"summary"` + TimeSeries []PRInsightsTimeSeriesEntry `json:"time_series"` + ByModel []PRInsightsModelBreakdown `json:"by_model"` + PullRequests []PRInsightsPullRequest `json:"recent_prs"` } // PRInsightsSummary contains aggregate PR metrics for a time period, diff --git a/site/src/pages/AgentsPage/components/ChatCostSummaryView.tsx b/site/src/pages/AgentsPage/components/ChatCostSummaryView.tsx index 0107f3d821..63fe96e81a 100644 --- a/site/src/pages/AgentsPage/components/ChatCostSummaryView.tsx +++ b/site/src/pages/AgentsPage/components/ChatCostSummaryView.tsx @@ -21,6 +21,7 @@ import { } from "#/components/Tooltip/Tooltip"; import { formatTokenCount } from "#/utils/analytics"; import { formatCostMicros } from "#/utils/currency"; +import { paginateItems } from "#/utils/paginateItems"; interface ChatCostSummaryViewProps { summary: TypesGen.ChatCostSummary | undefined; @@ -95,25 +96,19 @@ export const ChatCostSummaryView: FC = ({ } const modelPageSize = 10; - const modelMaxPage = Math.max( - 1, - Math.ceil(summary.by_model.length / modelPageSize), - ); - const clampedModelPage = Math.min(modelPage, modelMaxPage); - const pagedModels = summary.by_model.slice( - (clampedModelPage - 1) * modelPageSize, - clampedModelPage * modelPageSize, - ); + const { + pagedItems: pagedModels, + clampedPage: clampedModelPage, + hasPreviousPage: hasModelPrev, + hasNextPage: hasModelNext, + } = paginateItems(summary.by_model, modelPageSize, modelPage); const chatPageSize = 10; - const chatMaxPage = Math.max( - 1, - Math.ceil(summary.by_chat.length / chatPageSize), - ); - const clampedChatPage = Math.min(chatPage, chatMaxPage); - const pagedChats = summary.by_chat.slice( - (clampedChatPage - 1) * chatPageSize, - clampedChatPage * chatPageSize, - ); + const { + pagedItems: pagedChats, + clampedPage: clampedChatPage, + hasPreviousPage: hasChatPrev, + hasNextPage: hasChatNext, + } = paginateItems(summary.by_chat, chatPageSize, chatPage); const usageLimit = summary.usage_limit; const showUsageLimitCard = usageLimit?.is_limited === true; @@ -333,10 +328,8 @@ export const ChatCostSummaryView: FC = ({ currentPage={clampedModelPage} pageSize={modelPageSize} onPageChange={setModelPage} - hasPreviousPage={clampedModelPage > 1} - hasNextPage={ - clampedModelPage * modelPageSize < summary.by_model.length - } + hasPreviousPage={hasModelPrev} + hasNextPage={hasModelNext} /> )} @@ -403,10 +396,8 @@ export const ChatCostSummaryView: FC = ({ currentPage={clampedChatPage} pageSize={chatPageSize} onPageChange={setChatPage} - hasPreviousPage={clampedChatPage > 1} - hasNextPage={ - clampedChatPage * chatPageSize < summary.by_chat.length - } + hasPreviousPage={hasChatPrev} + hasNextPage={hasChatNext} /> )} diff --git a/site/src/pages/AgentsPage/components/PRInsightsView.tsx b/site/src/pages/AgentsPage/components/PRInsightsView.tsx index 75fd81cf04..2d6a26ba9b 100644 --- a/site/src/pages/AgentsPage/components/PRInsightsView.tsx +++ b/site/src/pages/AgentsPage/components/PRInsightsView.tsx @@ -1,7 +1,7 @@ import dayjs from "dayjs"; import relativeTime from "dayjs/plugin/relativeTime"; import { CodeIcon, ExternalLinkIcon } from "lucide-react"; -import type { FC } from "react"; +import { type FC, useState } from "react"; import { Area, AreaChart, CartesianGrid, XAxis, YAxis } from "recharts"; import type * as TypesGen from "#/api/typesGenerated"; import { Button } from "#/components/Button/Button"; @@ -11,6 +11,7 @@ import { ChartTooltip, ChartTooltipContent, } from "#/components/Chart/Chart"; +import { PaginationWidgetBase } from "#/components/PaginationWidget/PaginationWidgetBase"; import { Table, TableBody, @@ -21,6 +22,7 @@ import { } from "#/components/Table/Table"; import { cn } from "#/utils/cn"; import { formatCostMicros } from "#/utils/currency"; +import { paginateItems } from "#/utils/paginateItems"; import { PrStateIcon } from "./GitPanel/GitPanel"; dayjs.extend(relativeTime); @@ -286,6 +288,8 @@ const TimeRangeFilter: FC<{ // Main view // --------------------------------------------------------------------------- +const RECENT_PRS_PAGE_SIZE = 10; + export const PRInsightsView: FC = ({ data, timeRange, @@ -294,6 +298,18 @@ export const PRInsightsView: FC = ({ const { summary, time_series, by_model, recent_prs } = data; const isEmpty = summary.total_prs_created === 0; + // Client-side pagination for recent PRs table. + // Page resets to 1 on data refresh because the parent unmounts this + // component during loading. Clamping ensures the page is valid if the + // list shrinks without a full remount. + const [recentPrsPage, setRecentPrsPage] = useState(1); + const { + pagedItems: pagedRecentPrs, + clampedPage: clampedRecentPrsPage, + hasPreviousPage: hasRecentPrsPrev, + hasNextPage: hasRecentPrsNext, + } = paginateItems(recent_prs, RECENT_PRS_PAGE_SIZE, recentPrsPage); + return (
{/* ── Header ── */} @@ -354,8 +370,8 @@ export const PRInsightsView: FC = ({
- {/* ── Model breakdown + Recent PRs side by side ── */} -
+ {/* ── Model breakdown + Recent PRs ── */} +
{/* ── Model performance (simplified) ── */} {by_model.length > 0 && (
@@ -413,7 +429,7 @@ export const PRInsightsView: FC = ({ {recent_prs.length > 0 && (
- Recent + Pull requests
@@ -436,7 +452,7 @@ export const PRInsightsView: FC = ({ {" "} - {recent_prs.map((pr) => ( + {pagedRecentPrs.map((pr) => ( = ({
+ {recent_prs.length > RECENT_PRS_PAGE_SIZE && ( +
+ +
+ )}
)}
diff --git a/site/src/utils/paginateItems.test.ts b/site/src/utils/paginateItems.test.ts new file mode 100644 index 0000000000..74a848d382 --- /dev/null +++ b/site/src/utils/paginateItems.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it } from "vitest"; +import { paginateItems } from "./paginateItems"; + +// 25 items numbered 1–25 for readable assertions. +const items = Array.from({ length: 25 }, (_, i) => i + 1); + +describe("paginateItems", () => { + it("returns the first page of items", () => { + const result = paginateItems(items, 10, 1); + expect(result.pagedItems).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]); + expect(result.clampedPage).toBe(1); + expect(result.totalPages).toBe(3); + expect(result.hasPreviousPage).toBe(false); + expect(result.hasNextPage).toBe(true); + }); + + it("returns a partial last page", () => { + const result = paginateItems(items, 10, 3); + expect(result.pagedItems).toEqual([21, 22, 23, 24, 25]); + expect(result.clampedPage).toBe(3); + expect(result.totalPages).toBe(3); + expect(result.hasPreviousPage).toBe(true); + expect(result.hasNextPage).toBe(false); + }); + + it("clamps currentPage down when beyond total pages", () => { + const result = paginateItems(items, 10, 99); + expect(result.clampedPage).toBe(3); + expect(result.pagedItems).toEqual([21, 22, 23, 24, 25]); + expect(result.hasPreviousPage).toBe(true); + expect(result.hasNextPage).toBe(false); + }); + + it("clamps currentPage up when 0", () => { + const result = paginateItems(items, 10, 0); + expect(result.clampedPage).toBe(1); + expect(result.pagedItems).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]); + expect(result.hasPreviousPage).toBe(false); + expect(result.hasNextPage).toBe(true); + }); + + it("clamps currentPage up when negative", () => { + const result = paginateItems(items, 10, -5); + expect(result.clampedPage).toBe(1); + expect(result.pagedItems).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]); + expect(result.hasPreviousPage).toBe(false); + expect(result.hasNextPage).toBe(true); + }); + + it("returns empty pagedItems with clampedPage=1 for an empty array", () => { + const result = paginateItems([], 10, 1); + expect(result.pagedItems).toEqual([]); + expect(result.clampedPage).toBe(1); + expect(result.totalPages).toBe(1); + expect(result.hasPreviousPage).toBe(false); + expect(result.hasNextPage).toBe(false); + }); + + it("reports hasPreviousPage correctly for middle pages", () => { + const result = paginateItems(items, 10, 2); + expect(result.hasPreviousPage).toBe(true); + expect(result.hasNextPage).toBe(true); + }); +}); diff --git a/site/src/utils/paginateItems.ts b/site/src/utils/paginateItems.ts new file mode 100644 index 0000000000..f2a38054d5 --- /dev/null +++ b/site/src/utils/paginateItems.ts @@ -0,0 +1,25 @@ +export function paginateItems( + items: readonly T[], + pageSize: number, + currentPage: number, +): { + pagedItems: T[]; + clampedPage: number; + totalPages: number; + hasPreviousPage: boolean; + hasNextPage: boolean; +} { + const totalPages = Math.max(1, Math.ceil(items.length / pageSize)); + const clampedPage = Math.max(1, Math.min(currentPage, totalPages)); + const pagedItems = items.slice( + (clampedPage - 1) * pageSize, + clampedPage * pageSize, + ); + return { + pagedItems, + clampedPage, + totalPages, + hasPreviousPage: clampedPage > 1, + hasNextPage: clampedPage * pageSize < items.length, + }; +}