diff --git a/bench/internal/lifecycle/report.go b/bench/internal/lifecycle/report.go index 7a58dadb..27e871e6 100644 --- a/bench/internal/lifecycle/report.go +++ b/bench/internal/lifecycle/report.go @@ -8,6 +8,7 @@ package lifecycle import ( "encoding/json" "fmt" + // nosemgrep: go.lang.security.audit.crypto.math_random.math-random-used -- bootstrap-CI resampling for benchmark statistics; not security-sensitive, and a seedable PRNG is required for reproducible intervals. "math/rand" "os" "sort" diff --git a/bench/internal/lifecycle/supersede_report.go b/bench/internal/lifecycle/supersede_report.go index 79529010..1ccdd7f4 100644 --- a/bench/internal/lifecycle/supersede_report.go +++ b/bench/internal/lifecycle/supersede_report.go @@ -3,6 +3,7 @@ package lifecycle import ( "encoding/json" "fmt" + // nosemgrep: go.lang.security.audit.crypto.math_random.math-random-used -- bootstrap-CI resampling for benchmark statistics; not security-sensitive, and a seedable PRNG is required for reproducible intervals. "math/rand" "os" "strings" diff --git a/dev/seed.sql b/dev/seed.sql index 541ad5ea..959fbb5d 100644 --- a/dev/seed.sql +++ b/dev/seed.sql @@ -579,7 +579,7 @@ SELECT 'apikey:admin', 'admin@apikey.local', (ARRAY['Sales Dashboard','Inventory Export','Store Comparison','Pipeline Notes','Regional Heatmap'])[(n % 5) + 1] || ' ' || lpad(n::text, 4, '0'), - 'Generated demo asset #' || n || ' — seeded so the portal library exercises pagination.', + 'Generated demo asset #' || n || ' - seeded so the portal library exercises pagination.', (ARRAY['text/html','text/csv','text/jsx','text/markdown','image/svg+xml'])[(n % 5) + 1], 'portal-assets', 'portal/apikey:admin/seed-' || lpad(n::text, 4, '0') || '/v1/content' @@ -1653,3 +1653,114 @@ VALUES ('evt-seed-12a', 'thr-seed-12', 'comment', 'admin@example.com', 'admin@example.com', 'A short caption would help non-analysts read this at a glance.', NULL, NULL, '{}'::jsonb, NOW() - interval '1 day') ON CONFLICT (id) DO NOTHING; + +-- ============================================================================ +-- Pagination volume (#972): enough rows per surface to exercise infinite +-- scroll / "Load more". Each block bulk-generates well over one page (page size +-- 50, except resources whose default page is 100) so the portal's list surfaces +-- (Resources, Collections, Feedback, Knowledge Pages, Settings > Users, Settings +-- > Changelog) reach past their first page in the dev environment. All owned by +-- apikey:admin (the acme-dev-key-2024 key) so they surface under "Mine"/admin +-- tabs, with staggered timestamps so DESC ordering is stable. ON CONFLICT keeps +-- the file re-runnable. +-- ============================================================================ + +-- 130 global resources so the admin All/Global tabs exceed the 100-row page. +INSERT INTO resources ( + id, scope, scope_id, category, filename, display_name, description, + mime_type, size_bytes, s3_key, uri, tags, uploader_sub, uploader_email, + created_at, updated_at +) +SELECT + 'res-seed-' || lpad(n::text, 4, '0'), + 'global', NULL, + (ARRAY['samples','playbooks','templates','references'])[(n % 4) + 1], + 'reference-' || lpad(n::text, 4, '0') + || (ARRAY['.md','.csv','.json','.txt'])[(n % 4) + 1], + (ARRAY['Onboarding Playbook','Sample Dataset','Report Template','Glossary Reference'])[(n % 4) + 1] + || ' ' || lpad(n::text, 4, '0'), + 'Generated demo resource #' || n || ' - seeded so the resource library exercises pagination.', + (ARRAY['text/markdown','text/csv','application/json','text/plain'])[(n % 4) + 1], + (ARRAY[2480, 9720, 4340, 1890])[(n % 4) + 1], + 'resources/global/res-seed-' || lpad(n::text, 4, '0'), + 'mcp://global/' || (ARRAY['samples','playbooks','templates','references'])[(n % 4) + 1] + || '/res-seed-' || lpad(n::text, 4, '0'), + ARRAY['demo', (ARRAY['onboarding','sample','template','glossary'])[(n % 4) + 1]], + 'apikey:admin', 'admin@apikey.local', + NOW() - (n || ' hours')::interval, NOW() - (n || ' hours')::interval +FROM generate_series(1, 130) AS n +ON CONFLICT (id) DO NOTHING; + +-- 60 collections so the Collections library exceeds the 50-row page. +INSERT INTO portal_collections ( + id, owner_id, owner_email, name, description, config, created_at, updated_at +) +SELECT + 'coll-seed-' || lpad(n::text, 4, '0'), + 'apikey:admin', 'admin@apikey.local', + (ARRAY['Weekly Review','Region Rollup','Category Deep Dive','Exec Briefing'])[(n % 4) + 1] + || ' ' || lpad(n::text, 4, '0'), + 'Generated demo collection #' || n || ' - seeded so the collections library exercises pagination.', + '{"thumbnail_size": "medium"}'::jsonb, + NOW() - (n || ' hours')::interval, NOW() - (n || ' hours')::interval +FROM generate_series(1, 60) AS n +ON CONFLICT (id) DO NOTHING; + +-- 60 knowledge pages so the Knowledge Pages list exceeds the 50-row page. +INSERT INTO portal_knowledge_pages ( + id, slug, title, summary, body, tags, created_by, created_email, + updated_by, current_version, created_at, updated_at +) +SELECT + 'kp-seed-vol-' || lpad(n::text, 4, '0'), + 'demo-knowledge-' || lpad(n::text, 4, '0'), + (ARRAY['Metric Definition','Data Source Note','Process Runbook','Domain Glossary'])[(n % 4) + 1] + || ' ' || lpad(n::text, 4, '0'), + 'Generated demo knowledge page #' || n || ' - seeded so the knowledge list exercises pagination.', + E'# Demo Knowledge Page ' || n || E'\n\nGenerated so the knowledge-pages list has enough volume to exercise infinite scroll.', + jsonb_build_array('demo', (ARRAY['metrics','sources','process','glossary'])[(n % 4) + 1]), + 'admin@apikey.local', 'admin@apikey.local', 'admin@apikey.local', 1, + NOW() - (n || ' hours')::interval, NOW() - (n || ' hours')::interval +FROM generate_series(1, 60) AS n +ON CONFLICT (id) DO NOTHING; + +-- 60 directory users so Settings > Users exceeds the 100-cap page in aggregate +-- with the named seed users above. +INSERT INTO users (email, first_name, last_name, source, confirmed, added_by, created_at, updated_at) +SELECT + 'demo.user' || lpad(n::text, 4, '0') || '@example.com', + (ARRAY['Alex','Blair','Casey','Drew','Emerson'])[(n % 5) + 1], + 'Demo' || lpad(n::text, 4, '0'), + 'admin', FALSE, 'admin@apikey.local', + NOW() - (n || ' hours')::interval, NOW() - (n || ' hours')::interval +FROM generate_series(1, 60) AS n +ON CONFLICT (email) DO NOTHING; + +-- 60 config changelog entries so Settings > Changelog reaches past its page. +-- id is SERIAL (no stable id to upsert on), so re-runs first delete the demo +-- rows by their distinct changed_by marker to stay idempotent. +DELETE FROM config_changelog WHERE changed_by = 'seed@apikey.local'; +INSERT INTO config_changelog (key, action, value_text, changed_by, changed_at) +SELECT + (ARRAY['server.description','enrichment.unwrap_json','audit.retention_days','personas.analyst.tools'])[(n % 4) + 1], + (ARRAY['set','set','set','delete'])[(n % 4) + 1], + CASE WHEN (n % 4) = 3 THEN NULL ELSE 'demo value ' || n END, + 'seed@apikey.local', + NOW() - (n || ' hours')::interval +FROM generate_series(1, 60) AS n; + +-- 60 standalone feedback threads so the Feedback General and Recent tabs exceed +-- the 50-row page. +INSERT INTO portal_threads + (id, kind, target_type, asset_id, collection_id, prompt_id, knowledge_page_id, anchor, target_version, + title, author_id, author_email, status, requires_resolution, validation_state, insight_id, created_at, updated_at) +SELECT + 'thr-seed-vol-' || lpad(n::text, 4, '0'), + (ARRAY['comment','question','suggestion','rating'])[(n % 4) + 1], + 'standalone', NULL, NULL, NULL, NULL, NULL, NULL, + 'Demo feedback thread ' || lpad(n::text, 4, '0'), + 'admin@apikey.local', 'admin@apikey.local', + 'open', FALSE, 'none', NULL, + NOW() - (n || ' hours')::interval, NOW() - (n || ' hours')::interval +FROM generate_series(1, 60) AS n +ON CONFLICT (id) DO NOTHING; diff --git a/internal/apidocs/docs.go b/internal/apidocs/docs.go index d1fac9fe..42dded98 100644 --- a/internal/apidocs/docs.go +++ b/internal/apidocs/docs.go @@ -2608,7 +2608,7 @@ const docTemplate = `{ "BearerAuth": [] } ], - "description": "Returns recent config change history entries.", + "description": "Returns a page of config change history entries, newest first, with the total count.", "produces": [ "application/json" ], @@ -2616,14 +2616,25 @@ const docTemplate = `{ "Config" ], "summary": "Get config changelog", + "parameters": [ + { + "type": "integer", + "description": "Max entries to return (default 50, max 200)", + "name": "limit", + "in": "query" + }, + { + "type": "integer", + "description": "Pagination offset (default 0)", + "name": "offset", + "in": "query" + } + ], "responses": { "200": { "description": "OK", "schema": { - "type": "array", - "items": { - "$ref": "#/definitions/configstore.ChangelogEntry" - } + "$ref": "#/definitions/admin.changelogListResponse" } }, "500": { @@ -11357,6 +11368,12 @@ const docTemplate = `{ "name": "q", "in": "query" }, + { + "type": "integer", + "description": "Max results to return (default 100, max 200)", + "name": "limit", + "in": "query" + }, { "type": "integer", "description": "Pagination offset (default 0)", @@ -12281,6 +12298,20 @@ const docTemplate = `{ } } }, + "admin.changelogListResponse": { + "type": "object", + "properties": { + "entries": { + "type": "array", + "items": { + "$ref": "#/definitions/configstore.ChangelogEntry" + } + }, + "total": { + "type": "integer" + } + } + }, "admin.changesetListResponse": { "type": "object", "properties": { diff --git a/internal/apidocs/swagger.json b/internal/apidocs/swagger.json index 4aaf67e2..546137d4 100644 --- a/internal/apidocs/swagger.json +++ b/internal/apidocs/swagger.json @@ -2602,7 +2602,7 @@ "BearerAuth": [] } ], - "description": "Returns recent config change history entries.", + "description": "Returns a page of config change history entries, newest first, with the total count.", "produces": [ "application/json" ], @@ -2610,14 +2610,25 @@ "Config" ], "summary": "Get config changelog", + "parameters": [ + { + "type": "integer", + "description": "Max entries to return (default 50, max 200)", + "name": "limit", + "in": "query" + }, + { + "type": "integer", + "description": "Pagination offset (default 0)", + "name": "offset", + "in": "query" + } + ], "responses": { "200": { "description": "OK", "schema": { - "type": "array", - "items": { - "$ref": "#/definitions/configstore.ChangelogEntry" - } + "$ref": "#/definitions/admin.changelogListResponse" } }, "500": { @@ -11351,6 +11362,12 @@ "name": "q", "in": "query" }, + { + "type": "integer", + "description": "Max results to return (default 100, max 200)", + "name": "limit", + "in": "query" + }, { "type": "integer", "description": "Pagination offset (default 0)", @@ -12275,6 +12292,20 @@ } } }, + "admin.changelogListResponse": { + "type": "object", + "properties": { + "entries": { + "type": "array", + "items": { + "$ref": "#/definitions/configstore.ChangelogEntry" + } + }, + "total": { + "type": "integer" + } + } + }, "admin.changesetListResponse": { "type": "object", "properties": { diff --git a/internal/apidocs/swagger.yaml b/internal/apidocs/swagger.yaml index dbcf5e41..802efbe7 100644 --- a/internal/apidocs/swagger.yaml +++ b/internal/apidocs/swagger.yaml @@ -379,6 +379,15 @@ definitions: version: type: string type: object + admin.changelogListResponse: + properties: + entries: + items: + $ref: '#/definitions/configstore.ChangelogEntry' + type: array + total: + type: integer + type: object admin.changesetListResponse: properties: data: @@ -5332,16 +5341,24 @@ paths: - Config /admin/config/changelog: get: - description: Returns recent config change history entries. + description: Returns a page of config change history entries, newest first, + with the total count. + parameters: + - description: Max entries to return (default 50, max 200) + in: query + name: limit + type: integer + - description: Pagination offset (default 0) + in: query + name: offset + type: integer produces: - application/json responses: "200": description: OK schema: - items: - $ref: '#/definitions/configstore.ChangelogEntry' - type: array + $ref: '#/definitions/admin.changelogListResponse' "500": description: Internal Server Error schema: @@ -10931,6 +10948,10 @@ paths: in: query name: q type: string + - description: Max results to return (default 100, max 200) + in: query + name: limit + type: integer - description: Pagination offset (default 0) in: query name: offset diff --git a/pkg/admin/config_handler.go b/pkg/admin/config_handler.go index 7e89ce26..d64a1583 100644 --- a/pkg/admin/config_handler.go +++ b/pkg/admin/config_handler.go @@ -15,9 +15,14 @@ import ( "github.com/txn2/mcp-data-platform/pkg/platform/instructions" ) -// defaultChangelogLimit is the maximum number of changelog entries to return. +// defaultChangelogLimit is the page size used when no limit is requested. const defaultChangelogLimit = 50 +// maxChangelogLimit caps a client-supplied page size so a single changelog +// request cannot pull an unbounded window (CWE-770), mirroring the clamp every +// other paginated store applies. +const maxChangelogLimit = 200 + // Config key aliases for whitelisted, hot-reloadable platform settings. // Source of truth lives in pkg/platform — these are local references so // the admin package depends on the same canonical wire-protocol keys @@ -508,19 +513,38 @@ func (h *Handler) deleteConfigEntry(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } +// changelogListResponse is the paginated config-changelog envelope: a page of +// entries plus the full-history total so the UI can page through every change +// (#972), not just the most-recent window. +type changelogListResponse struct { + Entries []configstore.ChangelogEntry `json:"entries"` + Total int `json:"total"` +} + // getConfigChangelog handles GET /api/v1/admin/config/changelog. // // @Summary Get config changelog -// @Description Returns recent config change history entries. +// @Description Returns a page of config change history entries, newest first, with the total count. // @Tags Config // @Produce json -// @Success 200 {array} configstore.ChangelogEntry +// @Param limit query int false "Max entries to return (default 50, max 200)" +// @Param offset query int false "Pagination offset (default 0)" +// @Success 200 {object} admin.changelogListResponse // @Failure 500 {object} problemDetail // @Security ApiKeyAuth // @Security BearerAuth // @Router /admin/config/changelog [get] func (h *Handler) getConfigChangelog(w http.ResponseWriter, r *http.Request) { - entries, err := h.deps.ConfigStore.Changelog(r.Context(), defaultChangelogLimit) + limit := intQueryParam(r.URL.Query().Get("limit"), defaultChangelogLimit) + if limit <= 0 { + limit = defaultChangelogLimit + } + if limit > maxChangelogLimit { + limit = maxChangelogLimit + } + offset := intQueryParam(r.URL.Query().Get("offset"), 0) + + entries, total, err := h.deps.ConfigStore.Changelog(r.Context(), limit, offset) if err != nil { writeError(w, http.StatusInternalServerError, "failed to load config changelog") return @@ -528,7 +552,7 @@ func (h *Handler) getConfigChangelog(w http.ResponseWriter, r *http.Request) { if entries == nil { entries = []configstore.ChangelogEntry{} } - writeJSON(w, http.StatusOK, entries) + writeJSON(w, http.StatusOK, changelogListResponse{Entries: entries, Total: total}) } // --- Redaction helpers --- diff --git a/pkg/admin/config_handler_test.go b/pkg/admin/config_handler_test.go index 3418f9ae..b86784f1 100644 --- a/pkg/admin/config_handler_test.go +++ b/pkg/admin/config_handler_test.go @@ -370,7 +370,7 @@ func TestDeleteConfigEntry(t *testing.T) { } func TestGetConfigChangelog(t *testing.T) { - t.Run("returns changelog entries", func(t *testing.T) { + t.Run("returns a changelog page with total and default paging", func(t *testing.T) { cs := &mockConfigStore{ mode: "database", changelog: []configstore.ChangelogEntry{ @@ -382,6 +382,7 @@ func TestGetConfigChangelog(t *testing.T) { ChangedAt: time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC), }, }, + changelogTotal: 137, } h := NewHandler(Deps{ConfigStore: cs, Config: testConfig()}, nil) @@ -390,10 +391,51 @@ func TestGetConfigChangelog(t *testing.T) { h.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) - var entries []configstore.ChangelogEntry - require.NoError(t, json.NewDecoder(w.Body).Decode(&entries)) - assert.Len(t, entries, 1) - assert.Equal(t, "server.description", entries[0].Key) + var resp changelogListResponse + require.NoError(t, json.NewDecoder(w.Body).Decode(&resp)) + assert.Len(t, resp.Entries, 1) + assert.Equal(t, "server.description", resp.Entries[0].Key) + assert.Equal(t, 137, resp.Total) + // No query params: default limit, offset 0. + assert.Equal(t, defaultChangelogLimit, cs.changelogLimit) + assert.Equal(t, 0, cs.changelogOffset) + }) + + t.Run("forwards client limit and offset", func(t *testing.T) { + cs := &mockConfigStore{mode: "database"} + h := NewHandler(Deps{ConfigStore: cs, Config: testConfig()}, nil) + + req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/api/v1/admin/config/changelog?limit=25&offset=50", http.NoBody) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, 25, cs.changelogLimit) + assert.Equal(t, 50, cs.changelogOffset) + }) + + t.Run("non-positive limit falls back to default", func(t *testing.T) { + cs := &mockConfigStore{mode: "database"} + h := NewHandler(Deps{ConfigStore: cs, Config: testConfig()}, nil) + + req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/api/v1/admin/config/changelog?limit=0", http.NoBody) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, defaultChangelogLimit, cs.changelogLimit) + }) + + t.Run("oversized limit is clamped to the max", func(t *testing.T) { + cs := &mockConfigStore{mode: "database"} + h := NewHandler(Deps{ConfigStore: cs, Config: testConfig()}, nil) + + req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/api/v1/admin/config/changelog?limit=100000000", http.NoBody) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, maxChangelogLimit, cs.changelogLimit) }) } diff --git a/pkg/admin/handler.go b/pkg/admin/handler.go index 9a39072b..21a5f835 100644 --- a/pkg/admin/handler.go +++ b/pkg/admin/handler.go @@ -107,7 +107,7 @@ type ConfigStore interface { Set(ctx context.Context, key, value, author string) error Delete(ctx context.Context, key, author string) error List(ctx context.Context) ([]configstore.Entry, error) - Changelog(ctx context.Context, limit int) ([]configstore.ChangelogEntry, error) + Changelog(ctx context.Context, limit, offset int) ([]configstore.ChangelogEntry, int, error) Mode() string } diff --git a/pkg/admin/testhelpers_test.go b/pkg/admin/testhelpers_test.go index 7feb8958..cfe0cda7 100644 --- a/pkg/admin/testhelpers_test.go +++ b/pkg/admin/testhelpers_test.go @@ -303,16 +303,19 @@ type mockConfigStore struct { // (e.g. the visibility-lock test for #343 bug 1). Single-goroutine // tests pay no measurable cost — Mutex.Lock on uncontended use is // a single CAS. - mu sync.Mutex - mode string - entries map[string]*configstore.Entry - changelog []configstore.ChangelogEntry - setErr error - setCalls int - deleteErr error - listErr error - getErr error - changelogErr error + mu sync.Mutex + mode string + entries map[string]*configstore.Entry + changelog []configstore.ChangelogEntry + changelogTotal int + changelogLimit int + changelogOffset int + setErr error + setCalls int + deleteErr error + listErr error + getErr error + changelogErr error } func (m *mockConfigStore) Get(_ context.Context, key string) (*configstore.Entry, error) { @@ -371,13 +374,19 @@ func (m *mockConfigStore) List(_ context.Context) ([]configstore.Entry, error) { return result, nil } -func (m *mockConfigStore) Changelog(_ context.Context, _ int) ([]configstore.ChangelogEntry, error) { +func (m *mockConfigStore) Changelog(_ context.Context, limit, offset int) ([]configstore.ChangelogEntry, int, error) { m.mu.Lock() defer m.mu.Unlock() + m.changelogLimit = limit + m.changelogOffset = offset if m.changelogErr != nil { - return nil, m.changelogErr + return nil, 0, m.changelogErr } - return m.changelog, nil + total := m.changelogTotal + if total == 0 { + total = len(m.changelog) + } + return m.changelog, total, nil } func (m *mockConfigStore) Mode() string { diff --git a/pkg/configstore/file.go b/pkg/configstore/file.go index 6aeb3922..d6b0b605 100644 --- a/pkg/configstore/file.go +++ b/pkg/configstore/file.go @@ -54,9 +54,9 @@ func (s *FileStore) List(_ context.Context) ([]Entry, error) { return entries, nil } -// Changelog returns an empty slice because file-based config has no change history. -func (*FileStore) Changelog(_ context.Context, _ int) ([]ChangelogEntry, error) { - return []ChangelogEntry{}, nil +// Changelog returns an empty page because file-based config has no change history. +func (*FileStore) Changelog(_ context.Context, _, _ int) ([]ChangelogEntry, int, error) { + return []ChangelogEntry{}, 0, nil } // Mode returns "file". diff --git a/pkg/configstore/file_test.go b/pkg/configstore/file_test.go index 91e374d3..4a128be4 100644 --- a/pkg/configstore/file_test.go +++ b/pkg/configstore/file_test.go @@ -79,13 +79,16 @@ func TestFileStore_List(t *testing.T) { func TestFileStore_Changelog_ReturnsEmpty(t *testing.T) { store := NewFileStore(map[string]string{}) - cl, err := store.Changelog(context.Background(), 10) + cl, total, err := store.Changelog(context.Background(), 10, 0) if err != nil { t.Fatalf("Changelog() error = %v", err) } if len(cl) != 0 { t.Errorf("Changelog() returned %d entries, want 0", len(cl)) } + if total != 0 { + t.Errorf("Changelog() total = %d, want 0", total) + } } func TestFileStore_Mode(t *testing.T) { diff --git a/pkg/configstore/postgres/store.go b/pkg/configstore/postgres/store.go index 4ceab6bc..14ad0a39 100644 --- a/pkg/configstore/postgres/store.go +++ b/pkg/configstore/postgres/store.go @@ -129,17 +129,26 @@ func (s *Store) List(ctx context.Context) ([]configstore.Entry, error) { return entries, nil } -// Changelog returns recent config changes, newest first. -func (s *Store) Changelog(ctx context.Context, limit int) ([]configstore.ChangelogEntry, error) { +// Changelog returns a page of config changes, newest first: at most `limit` +// entries starting at `offset`, plus the total number recorded so callers can +// page through the full history. +func (s *Store) Changelog(ctx context.Context, limit, offset int) ([]configstore.ChangelogEntry, int, error) { + var total int + if err := s.db.QueryRowContext(ctx, + `SELECT COUNT(*) FROM config_changelog`, + ).Scan(&total); err != nil { + return nil, 0, fmt.Errorf("counting config changelog: %w", err) + } + rows, err := s.db.QueryContext(ctx, `SELECT id, key, action, value_text, changed_by, changed_at FROM config_changelog ORDER BY changed_at DESC - LIMIT $1`, - limit, + LIMIT $1 OFFSET $2`, + limit, offset, ) if err != nil { - return nil, fmt.Errorf("querying config changelog: %w", err) + return nil, 0, fmt.Errorf("querying config changelog: %w", err) } defer rows.Close() //nolint:errcheck // best-effort cleanup @@ -148,7 +157,7 @@ func (s *Store) Changelog(ctx context.Context, limit int) ([]configstore.Changel var e configstore.ChangelogEntry var value sql.NullString if err := rows.Scan(&e.ID, &e.Key, &e.Action, &value, &e.ChangedBy, &e.ChangedAt); err != nil { - return nil, fmt.Errorf("scanning changelog entry: %w", err) + return nil, 0, fmt.Errorf("scanning changelog entry: %w", err) } if value.Valid { e.Value = &value.String @@ -156,9 +165,9 @@ func (s *Store) Changelog(ctx context.Context, limit int) ([]configstore.Changel entries = append(entries, e) } if err := rows.Err(); err != nil { - return nil, fmt.Errorf("iterating changelog entries: %w", err) + return nil, 0, fmt.Errorf("iterating changelog entries: %w", err) } - return entries, nil + return entries, total, nil } // Mode returns "database". diff --git a/pkg/configstore/postgres/store_test.go b/pkg/configstore/postgres/store_test.go index 6a5711ea..ed1604f4 100644 --- a/pkg/configstore/postgres/store_test.go +++ b/pkg/configstore/postgres/store_test.go @@ -297,17 +297,22 @@ func TestPostgresStore_Changelog_Success(t *testing.T) { AddRow(2, "server.name", "set", val, "admin", now). AddRow(1, "old.key", "delete", nil, "user1", now.Add(-time.Hour)) + mock.ExpectQuery("SELECT COUNT\\(\\*\\) FROM config_changelog"). + WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(42)) mock.ExpectQuery("SELECT id, key, action, value_text, changed_by, changed_at FROM config_changelog"). - WithArgs(10). + WithArgs(10, 5). WillReturnRows(rows) - entries, err := store.Changelog(context.Background(), 10) + entries, total, err := store.Changelog(context.Background(), 10, 5) if err != nil { t.Fatalf("Changelog() error = %v", err) } if len(entries) != 2 { t.Fatalf("Changelog() returned %d entries, want 2", len(entries)) } + if total != 42 { + t.Errorf("Changelog() total = %d, want 42", total) + } // First entry: "set" with a value. if entries[0].ID != 2 { @@ -336,27 +341,46 @@ func TestPostgresStore_Changelog_Empty(t *testing.T) { store, mock := newTestStore(t) rows := sqlmock.NewRows([]string{"id", "key", "action", "value_text", "changed_by", "changed_at"}) + mock.ExpectQuery("SELECT COUNT\\(\\*\\) FROM config_changelog"). + WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0)) mock.ExpectQuery("SELECT id, key, action, value_text, changed_by, changed_at FROM config_changelog"). - WithArgs(10). + WithArgs(10, 0). WillReturnRows(rows) - entries, err := store.Changelog(context.Background(), 10) + entries, total, err := store.Changelog(context.Background(), 10, 0) if err != nil { t.Fatalf("Changelog() error = %v", err) } if len(entries) != 0 { t.Errorf("Changelog() returned %d entries, want 0", len(entries)) } + if total != 0 { + t.Errorf("Changelog() total = %d, want 0", total) + } +} + +func TestPostgresStore_Changelog_CountError(t *testing.T) { + store, mock := newTestStore(t) + + mock.ExpectQuery("SELECT COUNT\\(\\*\\) FROM config_changelog"). + WillReturnError(errors.New(testDBError)) + + _, _, err := store.Changelog(context.Background(), 10, 0) + if err == nil { + t.Error("Changelog() expected error") + } } func TestPostgresStore_Changelog_DBError(t *testing.T) { store, mock := newTestStore(t) + mock.ExpectQuery("SELECT COUNT\\(\\*\\) FROM config_changelog"). + WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(3)) mock.ExpectQuery("SELECT id, key, action, value_text, changed_by, changed_at FROM config_changelog"). - WithArgs(10). + WithArgs(10, 0). WillReturnError(errors.New(testDBError)) - _, err := store.Changelog(context.Background(), 10) + _, _, err := store.Changelog(context.Background(), 10, 0) if err == nil { t.Error("Changelog() expected error") } diff --git a/pkg/configstore/store.go b/pkg/configstore/store.go index b5c858fd..beca504c 100644 --- a/pkg/configstore/store.go +++ b/pkg/configstore/store.go @@ -43,8 +43,10 @@ type Store interface { Delete(ctx context.Context, key, author string) error // List returns all config entries, ordered by key. List(ctx context.Context) ([]Entry, error) - // Changelog returns recent config changes, newest first. - Changelog(ctx context.Context, limit int) ([]ChangelogEntry, error) + // Changelog returns a page of config changes, newest first: at most `limit` + // entries starting at `offset`, plus the total number of changes recorded so + // callers can page through the full history. + Changelog(ctx context.Context, limit, offset int) ([]ChangelogEntry, int, error) // Mode returns the store mode: "file" or "database". Mode() string } diff --git a/pkg/resource/handler.go b/pkg/resource/handler.go index 1d65d0b3..ef198987 100644 --- a/pkg/resource/handler.go +++ b/pkg/resource/handler.go @@ -380,6 +380,7 @@ func narrowScopes(visible []ScopeFilter, scopeParam, scopeIDParam string) []Scop // @Param category query string false "Filter by category" // @Param tag query string false "Filter by tag" // @Param q query string false "Search display_name and description" +// @Param limit query int false "Max results to return (default 100, max 200)" // @Param offset query int false "Pagination offset (default 0)" // @Success 200 {object} resource.listResponse // @Failure 401 {object} resource.errorResponse @@ -405,12 +406,22 @@ func (h *Handler) handleList(w http.ResponseWriter, r *http.Request) { } } + // Honor a client-supplied page size (the portal's infinite scroll requests a + // fixed window per page); the store clamps it to MaxListLimit. An absent or + // non-positive value falls back to the default. + limit := DefaultListLimit + if l := r.URL.Query().Get("limit"); l != "" { + if v, err := strconv.Atoi(l); err == nil && v > 0 { + limit = v + } + } + filter := Filter{ Scopes: scopes, Category: r.URL.Query().Get("category"), Tag: r.URL.Query().Get("tag"), Query: r.URL.Query().Get("q"), - Limit: DefaultListLimit, + Limit: limit, Offset: offset, } diff --git a/pkg/resource/handler_test.go b/pkg/resource/handler_test.go index fb31b16e..df40021d 100644 --- a/pkg/resource/handler_test.go +++ b/pkg/resource/handler_test.go @@ -18,6 +18,9 @@ import ( type mockStore struct { resources map[string]*Resource + // lastListFilter records the filter passed to the most recent List call so + // tests can assert the handler forwards parsed pagination params. + lastListFilter Filter } func newMockStore() *mockStore { @@ -47,6 +50,7 @@ func (m *mockStore) GetByURI(_ context.Context, uri string) (*Resource, error) { } func (m *mockStore) List(_ context.Context, filter Filter) ([]Resource, int, error) { + m.lastListFilter = filter var result []Resource for _, r := range m.resources { for _, sf := range filter.Scopes { @@ -457,6 +461,40 @@ func TestHandleList_Success(t *testing.T) { } } +func TestHandleList_ForwardsLimitAndOffset(t *testing.T) { + tests := []struct { + name string + query string + wantLimit int + wantOff int + }{ + {"client limit and offset honored", "?limit=25&offset=50", 25, 50}, + {"absent limit falls back to default", "?offset=10", DefaultListLimit, 10}, + {"non-positive limit falls back to default", "?limit=0", DefaultListLimit, 0}, + {"invalid limit falls back to default", "?limit=abc", DefaultListLimit, 0}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + store := newMockStore() + h := newTestHandler(store, nil, okExtractor) + + req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/api/v1/resources"+tt.query, http.NoBody) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String()) + } + if store.lastListFilter.Limit != tt.wantLimit { + t.Errorf("Limit = %d, want %d", store.lastListFilter.Limit, tt.wantLimit) + } + if store.lastListFilter.Offset != tt.wantOff { + t.Errorf("Offset = %d, want %d", store.lastListFilter.Offset, tt.wantOff) + } + }) + } +} + func TestHandleList_Empty(t *testing.T) { store := newMockStore() h := newTestHandler(store, nil, okExtractor) diff --git a/pkg/resource/store.go b/pkg/resource/store.go index 9c80a1ec..b365ae86 100644 --- a/pkg/resource/store.go +++ b/pkg/resource/store.go @@ -27,8 +27,13 @@ type S3Client interface { DeleteObject(ctx context.Context, bucket, key string) error } -// DefaultListLimit is used when no limit is specified in a list query. -const DefaultListLimit = 100 +const ( + // DefaultListLimit is used when no limit is specified in a list query. + DefaultListLimit = 100 + // MaxListLimit caps a client-supplied page size so a single list request + // cannot pull an unbounded window. + MaxListLimit = 200 +) // --- PostgreSQL Store --- @@ -109,6 +114,9 @@ func (s *postgresStore) List(ctx context.Context, filter Filter) ([]Resource, in if limit <= 0 { limit = DefaultListLimit } + if limit > MaxListLimit { + limit = MaxListLimit + } // #nosec G202 -- dynamic scope filter requires concatenation selectQuery := ` SELECT id, scope, scope_id, category, filename, display_name, description, diff --git a/pkg/resource/store_pg_test.go b/pkg/resource/store_pg_test.go index 176ca13a..29425f2b 100644 --- a/pkg/resource/store_pg_test.go +++ b/pkg/resource/store_pg_test.go @@ -140,6 +140,39 @@ func TestPostgresStore_List(t *testing.T) { } } +func TestPostgresStore_List_ClampsLimit(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatal(err) + } + defer func() { _ = db.Close() }() + + store := NewPostgresStore(db) + + mock.ExpectQuery("SELECT COUNT").WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1)) + // A client-supplied limit above MaxListLimit must reach the SELECT clamped to + // MaxListLimit (the scope arg, then the clamped limit, then the offset). + rows := sqlmock.NewRows([]string{ + "id", "scope", "scope_id", "category", "filename", "display_name", "description", + "mime_type", "size_bytes", "s3_key", "uri", "tags", "uploader_sub", "uploader_email", + "created_at", "updated_at", + }) + mock.ExpectQuery("SELECT .+ FROM resources WHERE"). + WithArgs(string(ScopeGlobal), MaxListLimit, 7). + WillReturnRows(rows) + + if _, _, err := store.List(context.Background(), Filter{ + Scopes: []ScopeFilter{{Scope: ScopeGlobal}}, + Limit: MaxListLimit + 5000, + Offset: 7, + }); err != nil { + t.Fatalf("List: %v", err) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + func TestPostgresStore_List_EmptyScopes(t *testing.T) { db, _, err := sqlmock.New() if err != nil { diff --git a/ui/src/api/admin/hooks/config.ts b/ui/src/api/admin/hooks/config.ts index a1ba9589..0fa93392 100644 --- a/ui/src/api/admin/hooks/config.ts +++ b/ui/src/api/admin/hooks/config.ts @@ -1,5 +1,11 @@ import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; +import { + useOffsetInfiniteQuery, + toPaginated, + type InfiniteResult, +} from "@/api/portal/hooks/infinite"; import { apiFetch, apiFetchRaw } from "../client"; +import type { ConfigChangelogEntry, ConfigChangelogListResponse } from "../types"; // --------------------------------------------------------------------------- // API Keys @@ -88,10 +94,26 @@ export function useDeleteConfigEntry() { }); } -export function useConfigChangelog() { - return useQuery({ - queryKey: ["config", "changelog"], - queryFn: () => apiFetch("/config/changelog"), +// CHANGELOG_PAGE_SIZE matches the server-side default changelog window so the +// change history loads incrementally rather than capping at the most-recent +// page (#972). +export const CHANGELOG_PAGE_SIZE = 50; + +const changelogKey = (e: ConfigChangelogEntry): string => String(e.id); + +// useInfiniteConfigChangelog accumulates changelog pages so an admin can page +// back through the full config-change history, not just the most-recent window. +// The endpoint returns a `{entries,total}` envelope, adapted to the shared +// PaginatedResponse shape here. +export function useInfiniteConfigChangelog(): InfiniteResult { + return useOffsetInfiniteQuery({ + queryKey: ["config", "changelog", "infinite"], + pageSize: CHANGELOG_PAGE_SIZE, + keyOf: changelogKey, + fetchPage: (offset, limit) => + apiFetch( + `/config/changelog?limit=${limit}&offset=${offset}`, + ).then((r) => toPaginated(r.entries, r.total, limit, offset)), }); } diff --git a/ui/src/api/admin/hooks/prompts-users.ts b/ui/src/api/admin/hooks/prompts-users.ts index 7cb75900..8c892e9b 100644 --- a/ui/src/api/admin/hooks/prompts-users.ts +++ b/ui/src/api/admin/hooks/prompts-users.ts @@ -4,6 +4,11 @@ import { useQueryClient, keepPreviousData, } from "@tanstack/react-query"; +import { + useOffsetInfiniteQuery, + toPaginated, + type InfiniteResult, +} from "@/api/portal/hooks/infinite"; import { apiFetch, apiFetchRaw } from "../client"; import type { PromptListResponse, Prompt } from "../types"; import { REFETCH_INTERVAL } from "./shared"; @@ -120,6 +125,36 @@ export function useDirectoryUsers(q?: string) { }); } +// USER_PAGE_SIZE is the number of directory users requested per page. It stays +// at/under the admin store's hard cap (100) so the requested window is honored +// and the directory loads incrementally rather than capping at one page (#972). +export const USER_PAGE_SIZE = 50; + +const directoryUserKey = (u: import("../types").DirectoryUser): string => u.email; + +// useInfiniteDirectoryUsers is the paginated counterpart of useDirectoryUsers: it +// accumulates pages so a deployment with more than one page of users can reach +// all of them. The endpoint returns a `{users,total}` envelope, adapted to the +// shared PaginatedResponse shape here. `q` narrows the directory server-side. +export function useInfiniteDirectoryUsers( + q?: string, +): InfiniteResult { + return useOffsetInfiniteQuery({ + queryKey: ["users", "infinite", q ?? ""], + pageSize: USER_PAGE_SIZE, + keyOf: directoryUserKey, + fetchPage: (offset, limit) => { + const sp = new URLSearchParams(); + if (q) sp.set("q", q); + sp.set("limit", String(limit)); + sp.set("offset", String(offset)); + return apiFetch( + `/users?${sp.toString()}`, + ).then((r) => toPaginated(r.users, r.total, limit, offset)); + }, + }); +} + export function useCreateUser() { const qc = useQueryClient(); return useMutation({ diff --git a/ui/src/api/admin/types/config.ts b/ui/src/api/admin/types/config.ts index eafbca9b..001e780c 100644 --- a/ui/src/api/admin/types/config.ts +++ b/ui/src/api/admin/types/config.ts @@ -65,6 +65,13 @@ export interface ConfigChangelogEntry { changed_at: string; } +// ConfigChangelogListResponse is the paginated changelog envelope: a page of +// entries plus the full-history total so the UI can page through every change. +export interface ConfigChangelogListResponse { + entries: ConfigChangelogEntry[]; + total: number; +} + export interface EffectiveConfigEntry { key: string; value: string; diff --git a/ui/src/api/portal/datahub.ts b/ui/src/api/portal/datahub.ts index ad0370e9..749bca25 100644 --- a/ui/src/api/portal/datahub.ts +++ b/ui/src/api/portal/datahub.ts @@ -150,6 +150,15 @@ export function useDataHubConnections() { // --- catalog reads --- +// Catalog and context-document browse are deliberately NOT wired to offset-based +// infinite scroll (unlike the internal portal lists in #972). They page against +// an external DataHub via a `query:"*"` search that sends no sort criteria, so +// results come back in relevance/segment order that is not stable across +// requests: numeric start/count offset paging over it can drop or duplicate rows +// between page fetches. Catalog browse also returns no total. Wiring infinite +// scroll here would introduce a correctness bug worse than the current +// first-page cap; it stays gated on upstream mcp-datahub gaining a stable sort +// (or a scroll/search-after cursor). Search remains the escape hatch for now. export function useCatalogBrowse(conn: string, opts: { limit?: number; offset?: number } = {}) { const limit = opts.limit ?? 50; const offset = opts.offset ?? 0; diff --git a/ui/src/api/portal/hooks/assets.ts b/ui/src/api/portal/hooks/assets.ts index 50404d23..a369d28b 100644 --- a/ui/src/api/portal/hooks/assets.ts +++ b/ui/src/api/portal/hooks/assets.ts @@ -3,10 +3,14 @@ import { useInfiniteQuery, useMutation, useQueryClient, - type InfiniteData, } from "@tanstack/react-query"; -import { useMemo } from "react"; import { apiFetch, apiFetchRaw } from "../client"; +import { + nextOffset, + flattenPages, + useInfiniteResult, + type InfiniteResult, +} from "./infinite"; import type { Asset, AssetVersion, @@ -19,6 +23,14 @@ import type { ScoredAsset, } from "../types"; +// Re-exported so existing importers (assets.test.ts and page components) keep +// resolving these from "./assets" after the shared paging primitives moved to +// ./infinite (#972). +export { nextOffset, flattenPages, useInfiniteResult }; +export type { InfiniteResult }; +// InfiniteAssetsResult is the historical name for the flattened infinite view. +export type InfiniteAssetsResult = InfiniteResult; + // --- Branding (unauthenticated) --- export interface Branding { @@ -133,92 +145,6 @@ export const ASSET_PAGE_SIZE = 50; export const assetKey = (a: Asset): string => a.id; export const sharedKey = (s: SharedAsset): string => s.asset.id; -// nextOffset returns the offset for the next page: the count of rows already -// fetched (the server offset, not the de-duplicated row count), or undefined -// once every row has been fetched. The cap comes from the LATEST page's total -// so rows inserted after the first fetch are still reachable, and an empty -// trailing page ends pagination even if total is stale-high (rows deleted -// between the count and the fetch), so "Load more" can't spin forever. -export function nextOffset(pages: PaginatedResponse[]): number | undefined { - const last = pages[pages.length - 1]; - if (last && last.data.length === 0) return undefined; - const fetched = pages.reduce((n, p) => n + p.data.length, 0); - return fetched < (last?.total ?? 0) ? fetched : undefined; -} - -// InfiniteAssetsResult is the flattened view an infinite asset query exposes to -// list pages: a single accumulated page (all loaded rows merged) plus the -// controls needed to render a "Load more" affordance. -export interface InfiniteAssetsResult { - data: PaginatedResponse | undefined; - isLoading: boolean; - hasNextPage: boolean; - isFetchingNextPage: boolean; - fetchNextPage: () => void; -} - -// flattenPages merges every fetched page into one PaginatedResponse: rows are -// concatenated in fetch order and de-duplicated by keyOf (offset paging over a -// created_at DESC list can re-emit a row when inserts shift the window, which -// would otherwise collide React keys and overcount), total comes from the -// latest page (most current count), limit from the first, and per-asset share -// summaries are unioned across pages. Returns undefined before the first page -// resolves so callers can distinguish "loading" from "empty". -export function flattenPages( - pages: InfiniteData> | undefined, - keyOf: (item: T) => string, -): PaginatedResponse | undefined { - const list = pages?.pages; - const first = list?.[0]; - if (!list || !first) return undefined; - const last = list[list.length - 1] ?? first; - - const seen = new Set(); - const data: T[] = []; - for (const page of list) { - for (const item of page.data) { - const k = keyOf(item); - if (seen.has(k)) continue; - seen.add(k); - data.push(item); - } - } - - return { - data, - total: last.total, - limit: first.limit, - offset: 0, - share_summaries: Object.assign( - {}, - ...list.map((p) => p.share_summaries ?? {}), - ), - }; -} - -// useInfiniteResult adapts a TanStack infinite query into the flattened -// InfiniteAssetsResult the list pages consume, so useInfiniteAssets, -// useInfiniteSharedWithMe, and the admin variant share one merge/return path. -export function useInfiniteResult( - q: { - data: InfiniteData> | undefined; - isLoading: boolean; - hasNextPage: boolean; - isFetchingNextPage: boolean; - fetchNextPage: () => unknown; - }, - keyOf: (item: T) => string, -): InfiniteAssetsResult { - const data = useMemo(() => flattenPages(q.data, keyOf), [q.data, keyOf]); - return { - data, - isLoading: q.isLoading, - hasNextPage: q.hasNextPage, - isFetchingNextPage: q.isFetchingNextPage, - fetchNextPage: () => void q.fetchNextPage(), - }; -} - // useInfiniteAssets is the paginated counterpart of useAssets: it accumulates // pages so a caller with more than one page of assets can load them all, // exposing a single merged page plus fetchNextPage/hasNextPage controls. diff --git a/ui/src/api/portal/hooks/collections.ts b/ui/src/api/portal/hooks/collections.ts index 010f6c23..a4d355e1 100644 --- a/ui/src/api/portal/hooks/collections.ts +++ b/ui/src/api/portal/hooks/collections.ts @@ -1,5 +1,10 @@ import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; import { apiFetch, apiFetchRaw } from "../client"; +import { + useOffsetInfiniteQuery, + paginatedFetch, + type InfiniteResult, +} from "./infinite"; import type { Share, PaginatedResponse, @@ -14,17 +19,35 @@ import type { // --- Collections --- -export function useCollections(params?: { search?: string; limit?: number; offset?: number }) { - const searchParams = new URLSearchParams(); - if (params?.search) searchParams.set("search", params.search); - if (params?.limit) searchParams.set("limit", String(params.limit)); - if (params?.offset) searchParams.set("offset", String(params.offset)); - const qs = searchParams.toString(); +// COLLECTION_PAGE_SIZE matches the portal list default and stays under the API +// max (200) so the collections library loads incrementally rather than capping +// at a single page (#972). +export const COLLECTION_PAGE_SIZE = 50; - return useQuery({ - queryKey: ["collections", params], - queryFn: () => - apiFetch>(`/collections${qs ? `?${qs}` : ""}`), +const collectionKey = (c: Collection): string => c.id; +const sharedCollectionKey = (s: SharedCollection): string => s.collection.id; + +// useInfiniteCollections is the paginated counterpart of useCollections: it +// accumulates pages so a caller with more than one page of collections can load +// them all, exposing a single merged page plus fetchNextPage/hasNextPage. +export function useInfiniteCollections(): InfiniteResult { + return useOffsetInfiniteQuery({ + queryKey: ["collections", "infinite"], + pageSize: COLLECTION_PAGE_SIZE, + keyOf: collectionKey, + fetchPage: (offset, limit) => + paginatedFetch("/collections", offset, limit), + }); +} + +// useInfiniteSharedCollections is the paginated shared-with-me collections list. +export function useInfiniteSharedCollections(): InfiniteResult { + return useOffsetInfiniteQuery({ + queryKey: ["shared-collections", "infinite"], + pageSize: COLLECTION_PAGE_SIZE, + keyOf: sharedCollectionKey, + fetchPage: (offset, limit) => + paginatedFetch("/shared-collections", offset, limit), }); } @@ -223,21 +246,6 @@ export function useSharedPrompts() { }); } -export function useSharedCollections(params?: { limit?: number; offset?: number }) { - const searchParams = new URLSearchParams(); - if (params?.limit) searchParams.set("limit", String(params.limit)); - if (params?.offset) searchParams.set("offset", String(params.offset)); - const qs = searchParams.toString(); - - return useQuery({ - queryKey: ["shared-collections", params], - queryFn: () => - apiFetch>( - `/shared-collections${qs ? `?${qs}` : ""}`, - ), - }); -} - // --- Known-users directory for the share picker (#614) --- export function useDirectoryUsers(q: string, enabled = true) { diff --git a/ui/src/api/portal/hooks/infinite.test.ts b/ui/src/api/portal/hooks/infinite.test.ts new file mode 100644 index 00000000..c191d04d --- /dev/null +++ b/ui/src/api/portal/hooks/infinite.test.ts @@ -0,0 +1,41 @@ +import { describe, it, expect } from "vitest"; +import type { InfiniteData } from "@tanstack/react-query"; +import { nextOffset, flattenPages, toPaginated } from "./infinite"; +import type { PaginatedResponse } from "../types"; + +interface Row { + id: string; +} + +const rowKey = (r: Row): string => r.id; + +function infinite(pages: PaginatedResponse[]): InfiniteData> { + return { pages, pageParams: pages.map((_, i) => i * 50) }; +} + +describe("toPaginated", () => { + it("adapts a {rows,total} envelope into PaginatedResponse", () => { + const p = toPaginated([{ id: "a" }, { id: "b" }], 7, 50, 100); + expect(p.data.map((r) => r.id)).toEqual(["a", "b"]); + expect(p.total).toBe(7); + expect(p.limit).toBe(50); + expect(p.offset).toBe(100); + }); + + it("treats an undefined row list as empty (so a null-array response ends paging)", () => { + const p = toPaginated(undefined, 0, 50, 0); + expect(p.data).toEqual([]); + }); + + it("feeds nextOffset/flattenPages so an adapted surface reuses the shared path", () => { + // Two adapted pages of a resources-like envelope: 2 then 1 row of total 3. + const p0 = toPaginated([{ id: "a" }, { id: "b" }], 3, 2, 0); + const p1 = toPaginated([{ id: "c" }], 3, 2, 2); + // First page still has rows to fetch (2 of 3); after both, none remain. + expect(nextOffset([p0])).toBe(2); + expect(nextOffset([p0, p1])).toBeUndefined(); + const merged = flattenPages(infinite([p0, p1]), rowKey); + expect(merged?.data.map((r) => r.id)).toEqual(["a", "b", "c"]); + expect(merged?.total).toBe(3); + }); +}); diff --git a/ui/src/api/portal/hooks/infinite.ts b/ui/src/api/portal/hooks/infinite.ts new file mode 100644 index 00000000..aa70d787 --- /dev/null +++ b/ui/src/api/portal/hooks/infinite.ts @@ -0,0 +1,167 @@ +import { + useInfiniteQuery, + type InfiniteData, + type QueryKey, +} from "@tanstack/react-query"; +import { useMemo } from "react"; +import { apiFetch } from "../client"; +import type { PaginatedResponse } from "../types"; + +// Shared offset-based infinite-pagination primitives (#972). The portal's list +// surfaces all page the same way: a `limit`/`offset` window over a stable +// created_at/updated_at DESC ordering, with the server returning a full-set +// `total`. These helpers accumulate those pages into one merged list plus the +// controls a "Load more" affordance and the useInfiniteScroll hook consume, so +// every list (assets, collections, feedback, resources, knowledge pages, users, +// changelog) shares one implementation rather than reinventing paging per page. + +// nextOffset returns the offset for the next page: the count of rows already +// fetched (the server offset, not the de-duplicated row count), or undefined +// once every row has been fetched. The cap comes from the LATEST page's total +// so rows inserted after the first fetch are still reachable, and an empty +// trailing page ends pagination even if total is stale-high (rows deleted +// between the count and the fetch), so "Load more" can't spin forever. +export function nextOffset(pages: PaginatedResponse[]): number | undefined { + const last = pages[pages.length - 1]; + if (last && last.data.length === 0) return undefined; + const fetched = pages.reduce((n, p) => n + p.data.length, 0); + return fetched < (last?.total ?? 0) ? fetched : undefined; +} + +// InfiniteResult is the flattened view an infinite query exposes to list pages: +// a single accumulated page (all loaded rows merged) plus the controls needed to +// render a "Load more" affordance. `data` mirrors PaginatedResponse so a page +// keeps reading `.data.data` / `.data.total` exactly as it did for a single-page +// query, only gaining the load-more controls. +export interface InfiniteResult { + data: PaginatedResponse | undefined; + isLoading: boolean; + isError: boolean; + hasNextPage: boolean; + isFetchingNextPage: boolean; + fetchNextPage: () => void; + refetch: () => void; +} + +// flattenPages merges every fetched page into one PaginatedResponse: rows are +// concatenated in fetch order and de-duplicated by keyOf (offset paging over a +// created_at DESC list can re-emit a row when inserts shift the window, which +// would otherwise collide React keys and overcount), total comes from the +// latest page (most current count), limit from the first, and per-row share +// summaries are unioned across pages. Returns undefined before the first page +// resolves so callers can distinguish "loading" from "empty". +export function flattenPages( + pages: InfiniteData> | undefined, + keyOf: (item: T) => string, +): PaginatedResponse | undefined { + const list = pages?.pages; + const first = list?.[0]; + if (!list || !first) return undefined; + const last = list[list.length - 1] ?? first; + + const seen = new Set(); + const data: T[] = []; + for (const page of list) { + for (const item of page.data) { + const k = keyOf(item); + if (seen.has(k)) continue; + seen.add(k); + data.push(item); + } + } + + return { + data, + total: last.total, + limit: first.limit, + offset: 0, + share_summaries: Object.assign( + {}, + ...list.map((p) => p.share_summaries ?? {}), + ), + }; +} + +// useInfiniteResult adapts a TanStack infinite query into the flattened +// InfiniteResult a list page consumes, so every surface shares one merge/return +// path. +export function useInfiniteResult( + q: { + data: InfiniteData> | undefined; + isLoading: boolean; + isError: boolean; + hasNextPage: boolean; + isFetchingNextPage: boolean; + fetchNextPage: () => unknown; + refetch: () => unknown; + }, + keyOf: (item: T) => string, +): InfiniteResult { + const data = useMemo(() => flattenPages(q.data, keyOf), [q.data, keyOf]); + return { + data, + isLoading: q.isLoading, + isError: q.isError, + hasNextPage: q.hasNextPage, + isFetchingNextPage: q.isFetchingNextPage, + fetchNextPage: () => void q.fetchNextPage(), + refetch: () => void q.refetch(), + }; +} + +// toPaginated adapts a non-PaginatedResponse list envelope (e.g. resources +// `{resources,total}`, users `{users,total}`, knowledge pages `{pages,total}`) +// into the PaginatedResponse shape the shared helpers operate on, so those +// surfaces reuse the same flatten/nextOffset path as the natively-paginated +// ones. `offset` is the page param that produced this page. +export function toPaginated( + rows: T[] | undefined, + total: number, + limit: number, + offset: number, +): PaginatedResponse { + return { data: rows ?? [], total, limit, offset }; +} + +// useOffsetInfiniteQuery is the one-call shared entry point: given a query key, a +// page size, a key extractor, and a fetcher that returns a PaginatedResponse for +// a given offset, it wires the TanStack infinite query (offset page param, +// nextOffset stop condition) and returns the flattened InfiniteResult. Surfaces +// whose endpoint returns a different envelope adapt it to PaginatedResponse +// inside `fetchPage` (see toPaginated). +export function useOffsetInfiniteQuery(opts: { + queryKey: QueryKey; + pageSize: number; + keyOf: (item: T) => string; + fetchPage: (offset: number, limit: number) => Promise>; + enabled?: boolean; +}): InfiniteResult { + const { queryKey, pageSize, keyOf, fetchPage, enabled } = opts; + const q = useInfiniteQuery({ + queryKey, + enabled, + initialPageParam: 0, + queryFn: ({ pageParam }) => fetchPage(pageParam, pageSize), + getNextPageParam: (_last, all) => nextOffset(all), + }); + return useInfiniteResult(q, keyOf); +} + +// paginatedFetch is the common fetchPage body for endpoints that already return +// a PaginatedResponse: it appends limit/offset (plus any extra params) and +// fetches. Endpoints with a different envelope build their own fetchPage that +// maps the response through toPaginated. +export function paginatedFetch( + path: string, + offset: number, + limit: number, + extra?: Record, +): Promise> { + const sp = new URLSearchParams(); + for (const [k, v] of Object.entries(extra ?? {})) { + if (v !== undefined && v !== "") sp.set(k, v); + } + sp.set("limit", String(limit)); + sp.set("offset", String(offset)); + return apiFetch>(`${path}?${sp.toString()}`); +} diff --git a/ui/src/api/portal/hooks/knowledge-pages.ts b/ui/src/api/portal/hooks/knowledge-pages.ts index e6d05be4..bbc163c5 100644 --- a/ui/src/api/portal/hooks/knowledge-pages.ts +++ b/ui/src/api/portal/hooks/knowledge-pages.ts @@ -1,5 +1,10 @@ import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; import { apiFetch, apiFetchRaw } from "../client"; +import { + useOffsetInfiniteQuery, + toPaginated, + type InfiniteResult, +} from "./infinite"; import type { ResolvedRef } from "@/lib/entityRefs"; import type { KnowledgePage, @@ -11,6 +16,43 @@ import type { } from "../types"; import { MIN_SEARCH_LEN } from "./shared"; +// KNOWLEDGE_PAGE_SIZE is the number of knowledge pages requested per page. It is +// set to the store's search-limit cap (100): any larger value collapses to the +// store's small fallback (limit over 100 falls back to 20), so 100 is the +// largest honored window. This makes the first page cover a whole knowledgebase +// of up to 100 pages, strictly more than the previous fixed `limit: 200` +// request (which the store silently collapsed to 20), while still paginating +// beyond that. The tag facet is derived from the loaded pages and so widens as +// more load (#972). +export const KNOWLEDGE_PAGE_SIZE = 100; + +const knowledgePageKey = (p: KnowledgePage): string => p.id; + +// useInfiniteKnowledgePages accumulates knowledge-page list results so a +// deployment with more than one page of pages can reach all of them. The list +// endpoint returns a `{pages,total}` envelope, adapted to the shared +// PaginatedResponse shape here. +export function useInfiniteKnowledgePages(params?: { + tag?: string; + q?: string; +}): InfiniteResult { + return useOffsetInfiniteQuery({ + queryKey: ["knowledge-pages", "infinite", params], + pageSize: KNOWLEDGE_PAGE_SIZE, + keyOf: knowledgePageKey, + fetchPage: (offset, limit) => { + const sp = new URLSearchParams(); + if (params?.tag) sp.set("tag", params.tag); + if (params?.q) sp.set("q", params.q); + sp.set("limit", String(limit)); + sp.set("offset", String(offset)); + return apiFetch( + `/knowledge-pages?${sp.toString()}`, + ).then((r) => toPaginated(r.pages, r.total, limit, offset)); + }, + }); +} + // --- Unified knowledge search (#661) --- // useSearch fans one query across every source the caller can access (internal @@ -43,19 +85,6 @@ export function useSearch( // --- Knowledge pages (#633) --- -export function useKnowledgePages(params?: { tag?: string; q?: string; limit?: number; offset?: number }) { - const search = new URLSearchParams(); - if (params?.tag) search.set("tag", params.tag); - if (params?.q) search.set("q", params.q); - if (params?.limit) search.set("limit", String(params.limit)); - if (params?.offset) search.set("offset", String(params.offset)); - const qs = search.toString(); - return useQuery({ - queryKey: ["knowledge-pages", params], - queryFn: () => apiFetch(`/knowledge-pages${qs ? `?${qs}` : ""}`), - }); -} - export function useKnowledgePage(id: string | null) { return useQuery({ queryKey: ["knowledge-page", id], diff --git a/ui/src/api/portal/hooks/threads.ts b/ui/src/api/portal/hooks/threads.ts index 0a4c133f..b5f29ba8 100644 --- a/ui/src/api/portal/hooks/threads.ts +++ b/ui/src/api/portal/hooks/threads.ts @@ -1,5 +1,10 @@ import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; import { apiFetch } from "../client"; +import { + useOffsetInfiniteQuery, + paginatedFetch, + type InfiniteResult, +} from "./infinite"; import type { PaginatedResponse, Thread, @@ -16,6 +21,14 @@ import type { SignoffSummary, } from "../types"; +// THREAD_PAGE_SIZE matches the server-side default thread window and stays under +// the API max (200), so the feedback lists load incrementally rather than +// capping at a single page (#972). +export const THREAD_PAGE_SIZE = 50; + +const threadKey = (t: ThreadWithMeta): string => t.id; +const activityKey = (t: ThreadActivityItem): string => t.id; + // --- Feedback threads (#601) --- export interface ThreadListFilter { @@ -60,6 +73,29 @@ export function useThreads(filter: ThreadListFilter) { }); } +// useInfiniteThreads is the paginated counterpart of useThreads: it accumulates +// pages so a scope with more than one page of threads (e.g. the standalone +// General channel) can load them all. Like useThreads it stays disabled until +// the filter targets a scope the backend accepts. +export function useInfiniteThreads(filter: ThreadListFilter): InfiniteResult { + return useOffsetInfiniteQuery({ + queryKey: ["threads", "infinite", filter], + pageSize: THREAD_PAGE_SIZE, + keyOf: threadKey, + enabled: threadFilterScoped(filter), + fetchPage: (offset, limit) => + paginatedFetch("/threads", offset, limit, { + target_type: filter.target_type, + asset_id: filter.asset_id, + collection_id: filter.collection_id, + prompt_id: filter.prompt_id, + knowledge_page_id: filter.knowledge_page_id, + kind: filter.kind, + status: filter.status, + }), + }); +} + export function useThread(id: string) { return useQuery({ queryKey: ["thread", id], @@ -95,6 +131,33 @@ export function useSMEWorklist(enabled = true) { }); } +// useInfinitePractitionerWorklist / useInfiniteSMEWorklist are the paginated +// counterparts of the worklist hooks: they accumulate pages so a user with more +// than one page of open work can reach all of it. The non-infinite hooks above +// stay for lightweight tab-badge counts (Sidebar, FeedbackPage) that only need +// the first page's total. +export function useInfinitePractitionerWorklist(enabled = true): InfiniteResult { + return useOffsetInfiniteQuery({ + queryKey: ["worklist", "practitioner", "infinite"], + pageSize: THREAD_PAGE_SIZE, + keyOf: threadKey, + enabled, + fetchPage: (offset, limit) => + paginatedFetch("/worklist/practitioner", offset, limit), + }); +} + +export function useInfiniteSMEWorklist(enabled = true): InfiniteResult { + return useOffsetInfiniteQuery({ + queryKey: ["worklist", "sme", "infinite"], + pageSize: THREAD_PAGE_SIZE, + keyOf: threadKey, + enabled, + fetchPage: (offset, limit) => + paginatedFetch("/worklist/sme", offset, limit), + }); +} + // useFeedbackActivity fetches the unified feed (#617): every feedback thread on // an asset, collection, or prompt the caller can view, most recent first. With // no push notifications, this is how a user discovers new feedback on their work. @@ -106,6 +169,20 @@ export function useFeedbackActivity(enabled = true) { }); } +// useInfiniteFeedbackActivity is the paginated counterpart of useFeedbackActivity: +// it accumulates pages so a user with more than one page of recent feedback can +// reach older threads. +export function useInfiniteFeedbackActivity(enabled = true): InfiniteResult { + return useOffsetInfiniteQuery({ + queryKey: ["feedback", "activity", "infinite"], + pageSize: THREAD_PAGE_SIZE, + keyOf: activityKey, + enabled, + fetchPage: (offset, limit) => + paginatedFetch("/feedback/activity", offset, limit), + }); +} + // useSignoff fetches "signed off by N of M" for an asset or collection (#603). export function useSignoff(targetType: "assets" | "collections", id: string, enabled = true) { return useQuery({ diff --git a/ui/src/api/resources/hooks.ts b/ui/src/api/resources/hooks.ts index 54229ff8..2eb042ba 100644 --- a/ui/src/api/resources/hooks.ts +++ b/ui/src/api/resources/hooks.ts @@ -1,21 +1,32 @@ import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; +import { + useOffsetInfiniteQuery, + toPaginated, + type InfiniteResult, +} from "@/api/portal/hooks/infinite"; import { resourceFetch, resourceFetchRaw } from "./client"; import type { Resource, ResourceListResponse, ResourceUpdate } from "./types"; -export function useResources(params?: { +interface ResourceQuery { scope?: string; scope_id?: string; category?: string; tag?: string; q?: string; -}) { - const searchParams = new URLSearchParams(); - if (params?.scope) searchParams.set("scope", params.scope); - if (params?.scope_id) searchParams.set("scope_id", params.scope_id); - if (params?.category) searchParams.set("category", params.category); - if (params?.tag) searchParams.set("tag", params.tag); - if (params?.q) searchParams.set("q", params.q); - const qs = searchParams.toString(); +} + +function resourceParams(params: ResourceQuery | undefined): URLSearchParams { + const sp = new URLSearchParams(); + if (params?.scope) sp.set("scope", params.scope); + if (params?.scope_id) sp.set("scope_id", params.scope_id); + if (params?.category) sp.set("category", params.category); + if (params?.tag) sp.set("tag", params.tag); + if (params?.q) sp.set("q", params.q); + return sp; +} + +export function useResources(params?: ResourceQuery) { + const qs = resourceParams(params).toString(); return useQuery({ queryKey: ["resources", qs], @@ -23,6 +34,34 @@ export function useResources(params?: { }); } +// RESOURCE_PAGE_SIZE is the number of resources requested per page. The backend +// clamps a client limit to resource.MaxListLimit (200); this stays well under it +// so the library loads incrementally rather than capping at the default page. +export const RESOURCE_PAGE_SIZE = 50; + +const resourceKey = (r: Resource): string => r.id; + +// useInfiniteResources is the paginated counterpart of useResources: it +// accumulates pages so a deployment with more than one page of resources can +// reach all of them (#972). The list endpoint returns a `{resources,total}` +// envelope, adapted to the shared PaginatedResponse shape here. +export function useInfiniteResources(params?: ResourceQuery): InfiniteResult { + const qs = resourceParams(params).toString(); + return useOffsetInfiniteQuery({ + queryKey: ["resources", "infinite", qs], + pageSize: RESOURCE_PAGE_SIZE, + keyOf: resourceKey, + fetchPage: (offset, limit) => { + const sp = resourceParams(params); + sp.set("limit", String(limit)); + sp.set("offset", String(offset)); + return resourceFetch(`?${sp.toString()}`).then((r) => + toPaginated(r.resources, r.total, limit, offset), + ); + }, + }); +} + export function useResource(id: string) { return useQuery({ queryKey: ["resources", id], diff --git a/ui/src/components/InfiniteFooter.test.tsx b/ui/src/components/InfiniteFooter.test.tsx new file mode 100644 index 00000000..27ce1e9f --- /dev/null +++ b/ui/src/components/InfiniteFooter.test.tsx @@ -0,0 +1,26 @@ +import { describe, it, expect, vi } from "vitest"; +import { render, screen, fireEvent } from "@testing-library/react"; +import { InfiniteFooter } from "./InfiniteFooter"; + +describe("InfiniteFooter", () => { + it("renders nothing when there is no further page", () => { + const { container } = render( + , + ); + expect(container).toBeEmptyDOMElement(); + }); + + it("renders a Load more button and invokes onLoadMore on click", () => { + const onLoadMore = vi.fn(); + render(); + const btn = screen.getByRole("button", { name: /load more/i }); + fireEvent.click(btn); + expect(onLoadMore).toHaveBeenCalledTimes(1); + }); + + it("disables the button and shows progress while loading", () => { + render(); + const btn = screen.getByRole("button", { name: /loading more/i }); + expect(btn).toBeDisabled(); + }); +}); diff --git a/ui/src/components/InfiniteFooter.tsx b/ui/src/components/InfiniteFooter.tsx new file mode 100644 index 00000000..31450af2 --- /dev/null +++ b/ui/src/components/InfiniteFooter.tsx @@ -0,0 +1,51 @@ +import { useRef } from "react"; +import { useInfiniteScroll } from "@/hooks/useInfiniteScroll"; + +interface Props { + /** Whether another page can be loaded. */ + hasMore: boolean; + /** Whether a page fetch is currently in flight. */ + isLoadingMore: boolean; + /** Load the next page. */ + onLoadMore: () => void; +} + +/** + * InfiniteFooter is the shared "Load more" control for portal list surfaces + * (#972). It owns the scroll sentinel and wires useInfiniteScroll so the next + * page auto-loads as the list nears the bottom, and renders a manual "Load more" + * button as the fallback/indicator (auto-scroll is a no-op in degenerate layouts + * such as jsdom, so the button is always the reliable path). It renders nothing + * when there is no further page, so callers can drop it in unconditionally. + * + * Progress copy ("Showing X of Y") is intentionally left to each page, which + * phrases the count per its own scope/filters; this component owns only the + * mechanism every list shares. + */ +export function InfiniteFooter({ hasMore, isLoadingMore, onLoadMore }: Props) { + const sentinelRef = useRef(null); + useInfiniteScroll(sentinelRef, { + hasMore, + isLoading: isLoadingMore, + onLoadMore, + }); + + if (!hasMore) return null; + + return ( + <> + {/* Sentinel: observed to auto-load the next page on scroll. */} + + ); } diff --git a/ui/src/components/feedback/FeedbackPanel.tsx b/ui/src/components/feedback/FeedbackPanel.tsx index 9ab7101d..ea989902 100644 --- a/ui/src/components/feedback/FeedbackPanel.tsx +++ b/ui/src/components/feedback/FeedbackPanel.tsx @@ -1,7 +1,8 @@ import { useState } from "react"; import { X, Plus } from "lucide-react"; -import { useThreads } from "@/api/portal/hooks"; +import { useInfiniteThreads } from "@/api/portal/hooks"; import type { FeedbackTarget } from "@/api/portal/types"; +import { InfiniteFooter } from "@/components/InfiniteFooter"; import { ThreadList } from "./ThreadList"; import { ThreadDetail } from "./ThreadDetail"; import { NewThreadForm } from "./NewThreadForm"; @@ -19,7 +20,8 @@ type View = { kind: "list" } | { kind: "new" } | { kind: "detail"; threadId: str export function FeedbackPanel({ target, canModerate, onClose }: Props) { const filter = filterForTarget(target); - const { data, isLoading } = useThreads(filter); + const { data, isLoading, hasNextPage, isFetchingNextPage, fetchNextPage } = + useInfiniteThreads(filter); const { availableAnchor } = useTextQuoteAnchor(); const [view, setView] = useState({ kind: "list" }); @@ -64,11 +66,20 @@ export function FeedbackPanel({ target, canModerate, onClose }: Props) { {/* Body */}
{view.kind === "list" && ( - setView({ kind: "detail", threadId })} - /> + <> + setView({ kind: "detail", threadId })} + /> +
+ +
+ )} {view.kind === "new" && ( ({ - usePractitionerWorklist: vi.fn(), - useSMEWorklist: vi.fn(), + useInfinitePractitionerWorklist: vi.fn(), + useInfiniteSMEWorklist: vi.fn(), })); import { InboxPanel } from "./InboxPanel"; -import { usePractitionerWorklist, useSMEWorklist } from "@/api/portal/hooks"; +import { useInfinitePractitionerWorklist, useInfiniteSMEWorklist } from "@/api/portal/hooks"; -const mockPractitioner = vi.mocked(usePractitionerWorklist); -const mockSME = vi.mocked(useSMEWorklist); +const mockPractitioner = vi.mocked(useInfinitePractitionerWorklist); +const mockSME = vi.mocked(useInfiniteSMEWorklist); + +// The infinite worklist hook exposes the flattened page under `data` plus +// load-more controls; a single-page fixture has no further page. +function result(data: unknown, total: number) { + return { + data: { data, total }, + isLoading: false, + isError: false, + hasNextPage: false, + isFetchingNextPage: false, + fetchNextPage: vi.fn(), + } as never; +} function row(overrides: Record = {}) { return { @@ -34,8 +47,8 @@ function row(overrides: Record = {}) { describe("InboxPanel", () => { it("shows practitioner worklist items and opens a thread", () => { - mockPractitioner.mockReturnValue({ data: { data: [row()], total: 1 }, isLoading: false, isError: false } as never); - mockSME.mockReturnValue({ data: { data: [], total: 0 }, isLoading: false, isError: false } as never); + mockPractitioner.mockReturnValue(result([row()], 1)); + mockSME.mockReturnValue(result([], 0)); const onOpen = vi.fn(); render(); @@ -45,8 +58,8 @@ describe("InboxPanel", () => { }); it("switches to the SME tab and shows the empty state", () => { - mockPractitioner.mockReturnValue({ data: { data: [row()], total: 1 }, isLoading: false, isError: false } as never); - mockSME.mockReturnValue({ data: { data: [], total: 0 }, isLoading: false, isError: false } as never); + mockPractitioner.mockReturnValue(result([row()], 1)); + mockSME.mockReturnValue(result([], 0)); render(); fireEvent.click(screen.getByRole("button", { name: /awaiting my validation/i })); diff --git a/ui/src/components/feedback/InboxPanel.tsx b/ui/src/components/feedback/InboxPanel.tsx index e3a818c3..1130427b 100644 --- a/ui/src/components/feedback/InboxPanel.tsx +++ b/ui/src/components/feedback/InboxPanel.tsx @@ -1,7 +1,8 @@ import { useState } from "react"; import { Inbox, ClipboardCheck } from "lucide-react"; -import { usePractitionerWorklist, useSMEWorklist } from "@/api/portal/hooks"; +import { useInfinitePractitionerWorklist, useInfiniteSMEWorklist } from "@/api/portal/hooks"; import { cn } from "@/lib/utils"; +import { InfiniteFooter } from "@/components/InfiniteFooter"; import { KIND_LABEL, STATUS_CHIP, STATUS_LABEL, formatRelative } from "./meta"; type Tab = "practitioner" | "sme"; @@ -11,8 +12,8 @@ type Tab = "practitioner" | "sme"; // validation requests awaiting the SME's response. export function InboxPanel({ onOpenThread }: { onOpenThread?: (id: string) => void }) { const [tab, setTab] = useState("practitioner"); - const practitioner = usePractitionerWorklist(); - const sme = useSMEWorklist(); + const practitioner = useInfinitePractitionerWorklist(); + const sme = useInfiniteSMEWorklist(); const active = tab === "practitioner" ? practitioner : sme; const threads = active.data?.data ?? []; @@ -63,6 +64,13 @@ export function InboxPanel({ onOpenThread }: { onOpenThread?: (id: string) => vo ))} +
+ +
); diff --git a/ui/src/mocks/handlers.ts b/ui/src/mocks/handlers.ts index 9bb2144b..0a509fed 100644 --- a/ui/src/mocks/handlers.ts +++ b/ui/src/mocks/handlers.ts @@ -2278,8 +2278,10 @@ export const handlers = [ ); } + const limit = Number(url.searchParams.get("limit")) || 100; + const offset = Number(url.searchParams.get("offset")) || 0; return HttpResponse.json({ - resources: filtered, + resources: filtered.slice(offset, offset + limit), total: filtered.length, }); }), @@ -2399,8 +2401,14 @@ export const handlers = [ return HttpResponse.json(mockEffectiveConfig); }), - http.get(`${ADMIN_BASE}/config/changelog`, () => { - return HttpResponse.json(mockConfigChangelog); + http.get(`${ADMIN_BASE}/config/changelog`, ({ request }) => { + const url = new URL(request.url); + const limit = Number(url.searchParams.get("limit")) || 50; + const offset = Number(url.searchParams.get("offset")) || 0; + return HttpResponse.json({ + entries: mockConfigChangelog.slice(offset, offset + limit), + total: mockConfigChangelog.length, + }); }), // Platform-owned agent-instructions baseline (#646). Rendered read-only diff --git a/ui/src/pages/collections/CollectionsPage.tsx b/ui/src/pages/collections/CollectionsPage.tsx index 9db5a1e0..439ad83a 100644 --- a/ui/src/pages/collections/CollectionsPage.tsx +++ b/ui/src/pages/collections/CollectionsPage.tsx @@ -1,9 +1,10 @@ import { useEffect, useState } from "react"; import { Search, FolderOpen, Plus, LayoutGrid, List, Users, Globe } from "lucide-react"; -import { useCollections, useCreateCollection, useSearchCollections, useSharedCollections, useThreadCounts } from "@/api/portal/hooks"; +import { useInfiniteCollections, useCreateCollection, useSearchCollections, useInfiniteSharedCollections, useThreadCounts } from "@/api/portal/hooks"; import type { Collection, SharePermission } from "@/api/portal/types"; import { FeedbackCountBadge } from "@/components/feedback/FeedbackCountBadge"; import { AssetsTabs } from "@/components/AssetsTabs"; +import { InfiniteFooter } from "@/components/InfiniteFooter"; import { ScopeFilter, getStoredScope, storeScope, type Scope } from "@/components/ScopeFilter"; import { SharePermissionBadge } from "@/components/SharePermissionBadge"; import { AuthImg } from "@/components/AuthImg"; @@ -57,9 +58,15 @@ export function CollectionsPage({ onNavigate }: Props) { // scopes fall back to client-side matching. const semanticSearch = searching && scope === "mine"; - const { data, isLoading } = useCollections(); + // Browse and shared lists paginate: each accumulates pages so callers with + // more than one page of collections can load them all via "Load more" / + // infinite scroll (#972). Semantic search returns a ranked top-K, so it has no + // additional pages to fetch. + const mineQuery = useInfiniteCollections(); + const { data, isLoading } = mineQuery; const searchResults = useSearchCollections(semanticSearch ? debouncedSearch : ""); - const { data: sharedData, isLoading: sharedLoading } = useSharedCollections(); + const sharedQuery = useInfiniteSharedCollections(); + const { data: sharedData, isLoading: sharedLoading } = sharedQuery; const createMutation = useCreateCollection(); const mineCollections: Collection[] = semanticSearch @@ -109,6 +116,27 @@ export function CollectionsPage({ onNavigate }: Props) { collections.map((c) => c.id), ); + // Load-more spans whichever paginated lists the active scope draws from: the + // browse list (mine/all) and/or the shared list (shared/all). Semantic search + // returns a ranked top-K with no further pages. Deriving the controls from one + // activeQueries list keeps the button state and the fetch action in sync. + const activeQueries: { + hasNextPage: boolean; + isFetchingNextPage: boolean; + fetchNextPage: () => void; + }[] = []; + if (!semanticSearch) { + if (scope !== "shared") activeQueries.push(mineQuery); + if (scope !== "mine") activeQueries.push(sharedQuery); + } + const canLoadMore = activeQueries.some((q) => q.hasNextPage); + const loadingMore = activeQueries.some((q) => q.isFetchingNextPage); + function loadMore() { + for (const q of activeQueries) { + if (q.hasNextPage && !q.isFetchingNextPage) q.fetchNextPage(); + } + } + async function handleCreate() { if (createMutation.isPending) return; const result = await createMutation.mutateAsync({ @@ -168,7 +196,16 @@ export function CollectionsPage({ onNavigate }: Props) { {semanticSearch ? "Searching..." : "Loading..."} ) : displayItems.length === 0 ? ( - + // With more pages available (shared/all client-side filters only see the + // loaded rows), steer to "Load more" instead of a contradictory empty + // state next to a Load-more button. + canLoadMore ? ( +
+ No matching collections in the loaded set yet — load more to keep looking. +
+ ) : ( + + ) ) : viewMode === "grid" ? (
{displayItems.map(({ collection: coll, share }) => { @@ -317,6 +354,8 @@ export function CollectionsPage({ onNavigate }: Props) {
)} + + {scope === "mine" && semanticSearch ? ( collections.length > 0 && (

@@ -324,7 +363,11 @@ export function CollectionsPage({ onNavigate }: Props) {

) ) : scope === "mine" ? ( - data && data.total > data.limit && ( + // Only advertise a remaining count while more can actually be loaded. + // flattenPages de-dupes by id but total is the raw server count, so a + // concurrent insert that re-emits a deduped row would otherwise leave a + // permanent "Showing 119 of 120" with no Load-more control to close it. + data && canLoadMore && collections.length < data.total && (

Showing {collections.length} of {data.total} collections

diff --git a/ui/src/pages/knowledge-pages/KnowledgePageForm.test.tsx b/ui/src/pages/knowledge-pages/KnowledgePageForm.test.tsx index ecb638a5..a6dd6571 100644 --- a/ui/src/pages/knowledge-pages/KnowledgePageForm.test.tsx +++ b/ui/src/pages/knowledge-pages/KnowledgePageForm.test.tsx @@ -8,7 +8,7 @@ import { useCreateKnowledgePage } from "@/api/portal/hooks"; // useUpdateKnowledgePage; the rest are stubbed because the page module imports // them at load time. Idle defaults so the form renders without a live query. vi.mock("@/api/portal/hooks", () => ({ - useKnowledgePages: vi.fn(() => ({ data: undefined, isLoading: false })), + useInfiniteKnowledgePages: vi.fn(() => ({ data: undefined, isLoading: false })), useSearchKnowledgePages: vi.fn(() => ({ data: undefined, isLoading: false })), useKnowledgePage: vi.fn(() => ({ data: undefined, isLoading: false, isError: false })), useResolveRefs: vi.fn(() => ({ data: undefined })), diff --git a/ui/src/pages/knowledge-pages/parts/KnowledgePageList.tsx b/ui/src/pages/knowledge-pages/parts/KnowledgePageList.tsx index ff020d69..12fe87f8 100644 --- a/ui/src/pages/knowledge-pages/parts/KnowledgePageList.tsx +++ b/ui/src/pages/knowledge-pages/parts/KnowledgePageList.tsx @@ -1,13 +1,14 @@ import { useEffect, useMemo, useState } from "react"; import { Search, Plus } from "lucide-react"; import { - useKnowledgePages, + useInfiniteKnowledgePages, useSearchKnowledgePages, useThreadCounts, MIN_SEARCH_LEN, } from "@/api/portal/hooks"; import type { KnowledgePage } from "@/api/portal/types"; import { FilterChip } from "@/components/FilterChip"; +import { InfiniteFooter } from "@/components/InfiniteFooter"; import { useDebounced } from "@/lib/useDebounced"; import { visibleFacetTags } from "../tagFacet"; import { PageCard } from "./PageCard"; @@ -28,12 +29,15 @@ export function KnowledgePageList({ canEdit, onOpen, onCreate }: { canEdit: bool useEffect(() => { if (searching) setTagsExpanded(false); }, [searching]); - // A high limit so the tag facet and counts reflect the whole knowledgebase, - // not just the first page of results. - const list = useKnowledgePages({ limit: 200 }); + // Browse accumulates pages (#972). The first page loads up to 100 pages (the + // store's honored cap), so for any knowledgebase up to that size the tag facet + // below is complete on first render; beyond it the facet widens as more pages + // load, an improvement over the previous single fixed request that the store + // silently capped at 20. + const list = useInfiniteKnowledgePages(); const search = useSearchKnowledgePages(trimmed, { limit: 25 }); - const allPages = useMemo(() => list.data?.pages ?? [], [list.data]); + const allPages = useMemo(() => list.data?.data ?? [], [list.data]); const total = list.data?.total ?? allPages.length; // Tag facet (tag -> count), most-used first, derived from the loaded pages. @@ -167,6 +171,18 @@ export function KnowledgePageList({ canEdit, onOpen, onCreate }: { canEdit: bool )} + + {/* Browse paginates; search returns a ranked top-K with no further pages. + Rendered outside the list conditional so it also offers "Load more" + when a client-side tag filter empties the loaded set but more pages + remain (InfiniteFooter renders nothing once every page is loaded). */} + {!searching && ( + + )} ); } diff --git a/ui/src/pages/resources/ResourcesPage.tsx b/ui/src/pages/resources/ResourcesPage.tsx index 68c7fb72..9b198607 100644 --- a/ui/src/pages/resources/ResourcesPage.tsx +++ b/ui/src/pages/resources/ResourcesPage.tsx @@ -10,9 +10,10 @@ import { FileText, Loader2, } from "lucide-react"; -import { useResources } from "@/api/resources/hooks"; +import { useInfiniteResources } from "@/api/resources/hooks"; import { useAuthStore } from "@/stores/auth"; import { usePersonas } from "@/api/admin/hooks"; +import { InfiniteFooter } from "@/components/InfiniteFooter"; import { formatBytes } from "@/lib/format"; import type { Resource } from "@/api/resources/types"; import { CATEGORIES, scopeIcon, scopeLabel } from "./modals/shared"; @@ -85,9 +86,10 @@ export function ResourcesPage({ admin }: Props) { } } - const { data, isLoading } = useResources(queryParams); + const { data, isLoading, hasNextPage, isFetchingNextPage, fetchNextPage } = + useInfiniteResources(queryParams); - const resources = data?.resources ?? []; + const resources = data?.data ?? []; const total = data?.total ?? 0; // Build tabs based on mode. @@ -265,6 +267,12 @@ export function ResourcesPage({ admin }: Props) { )} + + {total > resources.length && (

Showing {resources.length} of {total} resources diff --git a/ui/src/pages/settings/ChangelogPage.tsx b/ui/src/pages/settings/ChangelogPage.tsx index e18c17f3..58e57c99 100644 --- a/ui/src/pages/settings/ChangelogPage.tsx +++ b/ui/src/pages/settings/ChangelogPage.tsx @@ -1,7 +1,8 @@ import { useState } from "react"; -import { useConfigChangelog } from "@/api/admin/hooks"; +import { useInfiniteConfigChangelog } from "@/api/admin/hooks"; import type { ConfigChangelogEntry } from "@/api/admin/types"; import { cn } from "@/lib/utils"; +import { InfiniteFooter } from "@/components/InfiniteFooter"; import { Clock, ChevronDown, @@ -37,8 +38,9 @@ function ErrorBanner({ message, onRetry }: { message: string; onRetry?: () => vo // --------------------------------------------------------------------------- export function ChangelogPage() { - const { data: changelog, isLoading, error, refetch } = useConfigChangelog(); - const entries = changelog ?? []; + const { data, isLoading, isError, refetch, hasNextPage, isFetchingNextPage, fetchNextPage } = + useInfiniteConfigChangelog(); + const entries = data?.data ?? []; return (

@@ -49,7 +51,7 @@ export function ChangelogPage() {

- {error && ( + {isError && ( void refetch()} @@ -61,7 +63,7 @@ export function ChangelogPage() {
Loading...
- ) : entries.length === 0 && !error ? ( + ) : entries.length === 0 && !isError ? (

No configuration changes recorded yet

@@ -70,11 +72,20 @@ export function ChangelogPage() {

) : ( -
- {entries.map((e: ConfigChangelogEntry) => ( - - ))} -
+ <> +
+ {entries.map((e: ConfigChangelogEntry) => ( + + ))} +
+
+ +
+ )} diff --git a/ui/src/pages/settings/UsersPanel.tsx b/ui/src/pages/settings/UsersPanel.tsx index 8e42d5d6..884208c5 100644 --- a/ui/src/pages/settings/UsersPanel.tsx +++ b/ui/src/pages/settings/UsersPanel.tsx @@ -1,6 +1,6 @@ import { useState, useCallback } from "react"; import { - useDirectoryUsers, + useInfiniteDirectoryUsers, useCreateUser, useUpdateUser, useDeleteUser, @@ -8,6 +8,7 @@ import { } from "@/api/admin/hooks"; import type { DirectoryUser } from "@/api/admin/types"; import { cn } from "@/lib/utils"; +import { InfiniteFooter } from "@/components/InfiniteFooter"; import { Plus, Trash2, X, Contact, ChevronUp, Pencil, Check, Search } from "lucide-react"; // --------------------------------------------------------------------------- @@ -33,8 +34,11 @@ export function UsersPanel() { const isReadOnly = systemInfo?.config_mode === "file"; const [query, setQuery] = useState(""); - const { data: userList, isLoading } = useDirectoryUsers(query.trim() || undefined); - const users = userList?.users ?? []; + // The directory paginates so a deployment with more than one page of users can + // reach all of them (#972); `query` narrows server-side. + const { data, isLoading, hasNextPage, isFetchingNextPage, fetchNextPage } = + useInfiniteDirectoryUsers(query.trim() || undefined); + const users = data?.data ?? []; const [showForm, setShowForm] = useState(false); const [deleteConfirm, setDeleteConfirm] = useState(null); @@ -144,6 +148,15 @@ export function UsersPanel() { )} + {users.length > 0 && ( +
+ +
+ )} );