From db4c76b558a6cf9912ab93e55addb53043d7c319 Mon Sep 17 00:00:00 2001 From: Christophe Vila Date: Sun, 19 Jul 2026 16:31:24 +0200 Subject: [PATCH] Paginate the Review Queue with cursor-based infinite scroll Fetching every needs_review activity's laps and per-second samples up front gets expensive once there are many -- that work is now deferred to the current page only. GET /api/review-queue/ takes limit/before and returns {items, next_cursor, total}; sorting/cursor filtering still needs each activity's cheap summary row, but only the requested page's items get their laps/samples fetched. Frontend loads 10 at a time and fetches the next page when the list's bottom sentinel scrolls into view. Selecting a specific-kind filter (not "All") loads the rest of the backlog up front, since filtering only the items scrolled into view so far would hide matches further down. --- backend/internal/api/api_test.go | 102 ++++++++++++++++++++++++--- backend/internal/api/review.go | 83 ++++++++++++++++++---- frontend/src/App.css | 5 ++ frontend/src/api/client.ts | 15 +++- frontend/src/pages/ReviewQueue.tsx | 109 ++++++++++++++++++++++++++--- frontend/src/types/api.ts | 6 ++ 6 files changed, 284 insertions(+), 36 deletions(-) diff --git a/backend/internal/api/api_test.go b/backend/internal/api/api_test.go index bc08ab5..6fcd172 100644 --- a/backend/internal/api/api_test.go +++ b/backend/internal/api/api_test.go @@ -4,8 +4,10 @@ import ( "bytes" "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" + "net/url" "path/filepath" "strconv" "testing" @@ -182,16 +184,20 @@ func TestReviewQueueResolve(t *testing.T) { if rec.Code != http.StatusOK { t.Fatalf("review queue status = %d", rec.Code) } - var queue []map[string]any - json.Unmarshal(rec.Body.Bytes(), &queue) - if len(queue) != 1 { - t.Fatalf("expected 1 item in review queue, got %d: %s", len(queue), rec.Body.String()) + var page struct { + Items []map[string]any `json:"items"` + NextCursor *string `json:"next_cursor"` + Total int `json:"total"` } - laps, _ := queue[0]["laps"].([]any) + json.Unmarshal(rec.Body.Bytes(), &page) + if len(page.Items) != 1 || page.Total != 1 { + t.Fatalf("expected 1 item in review queue (total=1), got %d items, total=%d: %s", len(page.Items), page.Total, rec.Body.String()) + } + laps, _ := page.Items[0]["laps"].([]any) if len(laps) != 1 { t.Fatalf("expected 1 lap in review queue item, got %d: %s", len(laps), rec.Body.String()) } - samples, _ := queue[0]["samples"].([]any) + samples, _ := page.Items[0]["samples"].([]any) if len(samples) != 1 { t.Fatalf("expected 1 sample in review queue item, got %d: %s", len(samples), rec.Body.String()) } @@ -202,9 +208,87 @@ func TestReviewQueueResolve(t *testing.T) { } rec = doJSON(t, router, http.MethodGet, "/api/review-queue/", nil) - json.Unmarshal(rec.Body.Bytes(), &queue) - if len(queue) != 0 { - t.Fatalf("expected empty review queue after resolve, got %d", len(queue)) + json.Unmarshal(rec.Body.Bytes(), &page) + if len(page.Items) != 0 || page.Total != 0 { + t.Fatalf("expected empty review queue after resolve, got %d items, total=%d", len(page.Items), page.Total) + } +} + +func TestReviewQueue_PaginatesByCursor(t *testing.T) { + s, db := newTestServer(t) + ctx := newCtx() + + // 5 activities, newest first once sorted: 2026-07-05 .. 2026-07-01. + for i := 1; i <= 5; i++ { + activityID, err := db.UpsertActivity(ctx, store.Activity{ + GarminActivityID: int64(i), ActivityType: "running", + StartTimeUTC: fmt.Sprintf("2026-07-0%d 06:00:00", i), RawJSON: "{}", + }) + if err != nil { + t.Fatalf("UpsertActivity: %v", err) + } + if _, err := db.InsertKindAssignment(ctx, store.KindAssignment{ + ActivityID: activityID, AssignmentSource: store.AssignmentSourceRuleEngine, Status: store.AssignmentStatusNeedsReview, + CandidateKindsJSON: "[]", + }); err != nil { + t.Fatalf("InsertKindAssignment: %v", err) + } + } + + router := s.Router() + + type page struct { + Items []map[string]any `json:"items"` + NextCursor *string `json:"next_cursor"` + Total int `json:"total"` + } + getPage := func(query string) page { + t.Helper() + rec := doJSON(t, router, http.MethodGet, "/api/review-queue/"+query, nil) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, body = %s", rec.Code, rec.Body.String()) + } + var p page + if err := json.Unmarshal(rec.Body.Bytes(), &p); err != nil { + t.Fatalf("unmarshal: %v", err) + } + return p + } + startTime := func(item map[string]any) string { + return item["activity"].(map[string]any)["StartTimeUTC"].(string) + } + + first := getPage("?limit=2") + if len(first.Items) != 2 || first.Total != 5 { + t.Fatalf("first page: expected 2 items (total=5), got %d items, total=%d", len(first.Items), first.Total) + } + if startTime(first.Items[0]) != "2026-07-05 06:00:00" || startTime(first.Items[1]) != "2026-07-04 06:00:00" { + t.Fatalf("first page not newest-first: %v", first.Items) + } + if first.NextCursor == nil { + t.Fatal("expected a next_cursor on the first page") + } + + second := getPage("?limit=2&before=" + url.QueryEscape(*first.NextCursor)) + if len(second.Items) != 2 { + t.Fatalf("second page: expected 2 items, got %d", len(second.Items)) + } + if startTime(second.Items[0]) != "2026-07-03 06:00:00" || startTime(second.Items[1]) != "2026-07-02 06:00:00" { + t.Fatalf("second page not continuing newest-first: %v", second.Items) + } + if second.NextCursor == nil { + t.Fatal("expected a next_cursor on the second page") + } + + third := getPage("?limit=2&before=" + url.QueryEscape(*second.NextCursor)) + if len(third.Items) != 1 { + t.Fatalf("third page: expected 1 remaining item, got %d", len(third.Items)) + } + if startTime(third.Items[0]) != "2026-07-01 06:00:00" { + t.Fatalf("third page wrong item: %v", third.Items) + } + if third.NextCursor != nil { + t.Fatalf("expected no next_cursor on the last page, got %v", *third.NextCursor) } } diff --git a/backend/internal/api/review.go b/backend/internal/api/review.go index 08c891c..776ac7f 100644 --- a/backend/internal/api/review.go +++ b/backend/internal/api/review.go @@ -11,20 +11,44 @@ import ( "smartrun/backend/internal/store" ) +// defaultReviewQueuePageSize matches the frontend's initial/incremental +// page size for its infinite-scroll list. +const defaultReviewQueuePageSize = 10 + +type reviewQueueItem struct { + store.KindAssignment + Activity store.Activity `json:"activity"` + Laps []store.Lap `json:"laps"` + Samples []store.Sample `json:"samples"` +} + +// handleReviewQueue is cursor-paginated: fetching every needs_review +// activity's laps and (especially) per-second samples is expensive once +// there are many of them, so that work only happens for the requested page, +// not the full backlog. Sorting/cursor filtering still needs each +// activity's summary row (a cheap indexed lookup, no laps/samples), which +// happens for the whole backlog -- only the heavy per-item fetches are +// deferred to the page actually being returned. func (s *Server) handleReviewQueue(w http.ResponseWriter, r *http.Request) { + limit := defaultReviewQueuePageSize + if v := r.URL.Query().Get("limit"); v != "" { + if n, err := strconv.Atoi(v); err == nil && n > 0 { + limit = n + } + } + cursor := r.URL.Query().Get("before") // activity StartTimeUTC of the last item on the previous page + queue, err := s.DB.ReviewQueue(r.Context()) if err != nil { writeError(w, http.StatusInternalServerError, err.Error()) return } - type item struct { - store.KindAssignment - Activity store.Activity `json:"activity"` - Laps []store.Lap `json:"laps"` - Samples []store.Sample `json:"samples"` + type withActivity struct { + assignment store.KindAssignment + activity store.Activity } - items := make([]item, 0, len(queue)) + all := make([]withActivity, 0, len(queue)) for _, a := range queue { activity, ok, err := s.DB.GetActivity(r.Context(), a.ActivityID) if err != nil { @@ -34,7 +58,32 @@ func (s *Server) handleReviewQueue(w http.ResponseWriter, r *http.Request) { if !ok { continue } - laps, err := s.DB.LapsForActivity(r.Context(), a.ActivityID) + all = append(all, withActivity{assignment: a, activity: activity}) + } + + // Most recent run first, by when the activity actually happened (not + // when the rule engine flagged it), so the queue reads like a log. + sort.Slice(all, func(i, j int) bool { + return all[i].activity.StartTimeUTC > all[j].activity.StartTimeUTC + }) + + total := len(all) + + if cursor != "" { + idx := 0 + for idx < len(all) && all[idx].activity.StartTimeUTC >= cursor { + idx++ + } + all = all[idx:] + } + hasMore := len(all) > limit + if len(all) > limit { + all = all[:limit] + } + + items := make([]reviewQueueItem, 0, len(all)) + for _, wa := range all { + laps, err := s.DB.LapsForActivity(r.Context(), wa.assignment.ActivityID) if err != nil { writeError(w, http.StatusInternalServerError, err.Error()) return @@ -42,21 +91,25 @@ func (s *Server) handleReviewQueue(w http.ResponseWriter, r *http.Request) { // Per-second telemetry, not just per-lap averages, so the chart can // show real within-lap variation instead of one flat segment per lap // (most activities only have a handful of laps). - samples, err := s.DB.SamplesForActivity(r.Context(), a.ActivityID) + samples, err := s.DB.SamplesForActivity(r.Context(), wa.assignment.ActivityID) if err != nil { writeError(w, http.StatusInternalServerError, err.Error()) return } - items = append(items, item{KindAssignment: a, Activity: activity, Laps: laps, Samples: samples}) + items = append(items, reviewQueueItem{KindAssignment: wa.assignment, Activity: wa.activity, Laps: laps, Samples: samples}) } - // Most recent run first, by when the activity actually happened (not - // when the rule engine flagged it), so the queue reads like a log. - sort.Slice(items, func(i, j int) bool { - return items[i].Activity.StartTimeUTC > items[j].Activity.StartTimeUTC - }) + var nextCursor *string + if hasMore && len(items) > 0 { + c := items[len(items)-1].Activity.StartTimeUTC + nextCursor = &c + } - writeJSON(w, http.StatusOK, items) + writeJSON(w, http.StatusOK, map[string]any{ + "items": items, + "next_cursor": nextCursor, + "total": total, + }) } func (s *Server) handleResolveReview(w http.ResponseWriter, r *http.Request) { diff --git a/frontend/src/App.css b/frontend/src/App.css index 1e30239..690f557 100644 --- a/frontend/src/App.css +++ b/frontend/src/App.css @@ -183,6 +183,11 @@ button:disabled { gap: 1rem; } +.review-list-sentinel { + padding: 1rem 0; + text-align: center; +} + .review-item { border: 1px solid #2a2d35; border-radius: 8px; diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index f18cf6b..bb0dab8 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -6,7 +6,7 @@ import type { Profile, ProgressionMetric, ProgressionPoint, - ReviewQueueItem, + ReviewQueuePage, SyncRun, SyncStatus, WorkoutKind, @@ -82,8 +82,17 @@ export const api = { updateProfile: (profile: Profile) => request("/api/profile", { method: "PUT", body: JSON.stringify(profile) }), - // Review queue - reviewQueue: () => request("/api/review-queue/"), + // Review queue -- cursor-paginated: pass the previous page's next_cursor + // as `before` to fetch the next one. Fetching every activity's laps and + // per-second samples up front gets expensive once there are many, so the + // frontend loads it incrementally (infinite scroll) instead of all at once. + reviewQueue: (params?: { limit?: number; before?: string }) => { + const q = new URLSearchParams(); + if (params?.limit) q.set("limit", String(params.limit)); + if (params?.before) q.set("before", params.before); + const qs = q.toString(); + return request(`/api/review-queue/${qs ? `?${qs}` : ""}`); + }, resolveReview: (activityId: number, workoutKindId: number) => request<{ status: string }>(`/api/review-queue/${activityId}/resolve`, { method: "POST", diff --git a/frontend/src/pages/ReviewQueue.tsx b/frontend/src/pages/ReviewQueue.tsx index 8b65c62..a10ccdf 100644 --- a/frontend/src/pages/ReviewQueue.tsx +++ b/frontend/src/pages/ReviewQueue.tsx @@ -1,10 +1,11 @@ -import { useEffect, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { api } from "../api/client"; import { ExpectedVsActualChart } from "../components/charts/ExpectedVsActualChart"; import { RawDataModal } from "../components/RawDataModal"; import type { ReviewQueueItem, ScoredKind, WorkoutKind } from "../types/api"; const UNSORTED = "__unsorted__"; +const PAGE_SIZE = 10; function candidates(item: ReviewQueueItem): ScoredKind[] { try { @@ -24,28 +25,106 @@ function formatPace(avgSpeedMps: number | null): string | null { export function ReviewQueue() { const [items, setItems] = useState([]); + const [total, setTotal] = useState(0); + const [nextCursor, setNextCursor] = useState(null); + const [initialLoading, setInitialLoading] = useState(true); + const [loadingMore, setLoadingMore] = useState(false); const [kinds, setKinds] = useState([]); const [filterKindId, setFilterKindId] = useState(""); const [error, setError] = useState(null); const [resolvingId, setResolvingId] = useState(null); const [rawDataItem, setRawDataItem] = useState(null); + // Fetching every activity's laps and per-second samples is expensive once + // there are many activities, so the list loads incrementally (infinite + // scroll) instead of all at once -- see the paginated GET /api/review-queue/. + const allLoaded = !initialLoading && nextCursor === null; + // A single in-flight request is shared by every concurrent caller (the + // scroll observer, a filter switch's loadAll, ...): each gets the same + // promise back and genuinely awaits its completion, rather than a boolean + // guard that would let a second caller's loop spin against an + // already-in-progress fetch with nothing to await. + const inFlightRef = useRef | null>(null); + // The authoritative cursor for control flow, updated synchronously the + // instant a response arrives -- NOT derived from the nextCursor state in + // the render body. A ref written that way only picks up a new value once + // React actually re-renders, which isn't guaranteed to happen between two + // iterations of loadAll's tight loop; it was reading a stale cursor and + // re-fetching the same page twice. nextCursor (state) still exists + // separately, purely to drive rendering (e.g. allLoaded). + const nextCursorRef = useRef(null); + + const loadMore = useCallback((): Promise => { + if (inFlightRef.current) return inFlightRef.current; + if (nextCursorRef.current === null) return Promise.resolve(); + setLoadingMore(true); + const promise = (async () => { + try { + const page = await api.reviewQueue({ limit: PAGE_SIZE, before: nextCursorRef.current ?? undefined }); + nextCursorRef.current = page.next_cursor; + setItems((prev) => [...prev, ...page.items]); + setNextCursor(page.next_cursor); + setTotal(page.total); + } catch (e) { + setError(String(e)); + } finally { + setLoadingMore(false); + inFlightRef.current = null; + } + })(); + inFlightRef.current = promise; + return promise; + }, []); + + // Loads every remaining page in sequence. Used when a specific-kind filter + // is selected: filtering only the items loaded so far would hide matches + // that simply haven't scrolled into view yet, so switching to a filter + // (other than "All") loads the whole backlog once, up front. + const loadAll = useCallback(async () => { + while (nextCursorRef.current !== null) { + await loadMore(); + } + }, [loadMore]); + function reload() { - Promise.all([api.reviewQueue(), api.listWorkoutKinds()]) - .then(([reviewItems, workoutKinds]) => { - setItems(reviewItems); + setItems([]); + setInitialLoading(true); + Promise.all([api.reviewQueue({ limit: PAGE_SIZE }), api.listWorkoutKinds()]) + .then(([page, workoutKinds]) => { + nextCursorRef.current = page.next_cursor; + setItems(page.items); + setNextCursor(page.next_cursor); + setTotal(page.total); setKinds(workoutKinds); }) - .catch((e) => setError(String(e))); + .catch((e) => setError(String(e))) + .finally(() => setInitialLoading(false)); } useEffect(reload, []); + // Infinite scroll: load the next page once the sentinel at the bottom of + // the list becomes visible. + const sentinelRef = useRef(null); + useEffect(() => { + const node = sentinelRef.current; + if (!node || filterKindId !== "") return; + const observer = new IntersectionObserver( + (entries) => { + if (entries[0]?.isIntersecting) loadMore(); + }, + { rootMargin: "200px" }, + ); + observer.observe(node); + return () => observer.disconnect(); + }, [loadMore, filterKindId, items.length]); + async function resolve(activityId: number, kindId: number) { setResolvingId(activityId); try { await api.resolveReview(activityId, kindId); setItems((prev) => prev.filter((i) => i.ActivityID !== activityId)); + setTotal((prev) => Math.max(0, prev - 1)); } catch (e) { setError(String(e)); } finally { @@ -63,7 +142,9 @@ export function ReviewQueue() { }, [items, filterKindId]); function toggleFilter(id: string) { - setFilterKindId((prev) => (prev === id ? "" : id)); + const next = filterKindId === id ? "" : id; + setFilterKindId(next); + if (next !== "") loadAll(); } // Race is assigned automatically from Garmin metadata (eventType.typeKey @@ -75,14 +156,14 @@ export function ReviewQueue() {

Review Queue

{error &&

{error}

} - {items.length > 0 && ( + {total > 0 && (
); diff --git a/frontend/src/types/api.ts b/frontend/src/types/api.ts index ace89f9..be435f4 100644 --- a/frontend/src/types/api.ts +++ b/frontend/src/types/api.ts @@ -140,6 +140,12 @@ export interface ReviewQueueItem extends KindAssignment { samples: Sample[]; } +export interface ReviewQueuePage { + items: ReviewQueueItem[]; + next_cursor: string | null; + total: number; +} + export interface ProgressionPoint { date: string; activity_id: number;