From 74da145f4cda54627fcb5a09f3a27c0680af7de9 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Mon, 27 Jul 2026 17:39:00 -0700 Subject: [PATCH] feat(v2): load older messages in pod chat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Load older messages" worked on the anonymous showcase route but did nothing in a pod you had actually joined. Not a broken feature — it was never built there. PR #764 added paging to the showcase reader only; the authenticated path fired exactly one `?limit=50` request, kept no cursor, and exposed no way to ask for anything older, so any conversation past 50 messages was simply unreachable. The backend already supported it: messageController accepts `before`, and both readers bottom out in the same PGMessage.findByPodId. The entire gap was client-side. - useV2PodDetail gains hasMore / loadingOlder / loadOlder. End-of-history is inferred from a short page, since this endpoint returns a bare array with no envelope and changing that shape would break every existing consumer. - Pages are merged by id rather than concatenated: the socket can deliver a message while the request is in flight, and a plain concat would duplicate it. - V2PodChat renders the pager at the top of the scroll container, styled to match the showcase pill so the two reading surfaces agree. Two scroll defects had to be fixed or the feature would have looked broken: - the auto-scroll effect keyed on `messages.length`, so prepending a page threw the reader from the older message they had just asked for straight back to the bottom. It now keys on the newest message's id, so prepends are ignored. - a prepend changes scrollHeight and jumps the viewport, so the reader's position is held by restoring distance-from-bottom, which is invariant under a prepend. 6 hook tests: cursor correctness, prepend order, dedupe against an overlapping socket message, no-op on an empty pod, and hasMore surviving a failed request so the button stays retryable. Frontend suite 69/69, 316 tests. Lint at baseline. Needs a real-browser check after deploy — jsdom has no layout engine, so the scroll-anchor behaviour specifically is not covered by these tests. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01DMeWzgFxsfBcjoLVLewEES --- frontend/src/i18n/locales/en.json | 4 +- frontend/src/i18n/locales/zh-CN.json | 4 +- .../useV2PodDetailPagination.test.tsx | 136 ++++++++++++++++++ frontend/src/v2/components/V2PodChat.tsx | 50 ++++++- frontend/src/v2/hooks/useV2PodDetail.ts | 73 +++++++++- frontend/src/v2/v2.css | 20 +++ 6 files changed, 277 insertions(+), 10 deletions(-) create mode 100644 frontend/src/v2/__tests__/useV2PodDetailPagination.test.tsx diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index 3ee1f651..f477c8cb 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -324,7 +324,9 @@ }, "errors": { "uploadFailed": "Failed to upload file. Please try again." - } + }, + "loadOlder": "Load older messages", + "loadingOlder": "Loading…" }, "landing": { "nav": { diff --git a/frontend/src/i18n/locales/zh-CN.json b/frontend/src/i18n/locales/zh-CN.json index 8c7a10d1..c302b78e 100644 --- a/frontend/src/i18n/locales/zh-CN.json +++ b/frontend/src/i18n/locales/zh-CN.json @@ -324,7 +324,9 @@ }, "errors": { "uploadFailed": "文件上传失败，请重试。" - } + }, + "loadOlder": "加载更早的消息", + "loadingOlder": "加载中…" }, "landing": { "nav": { diff --git a/frontend/src/v2/__tests__/useV2PodDetailPagination.test.tsx b/frontend/src/v2/__tests__/useV2PodDetailPagination.test.tsx new file mode 100644 index 00000000..ed6ed136 --- /dev/null +++ b/frontend/src/v2/__tests__/useV2PodDetailPagination.test.tsx @@ -0,0 +1,136 @@ +// @ts-nocheck +// "Load older messages" worked on the anonymous showcase route but not in a +// pod you had actually joined — because it was never built there. The +// authenticated reader fired exactly one `?limit=50` request, tracked no +// cursor, and exposed no way to ask for anything older. The backend already +// accepted `before`, so the whole gap was client-side. +import { renderHook, act, waitFor } from '@testing-library/react'; +import { useV2PodDetail } from '../hooks/useV2PodDetail'; + +const mockApi = { get: jest.fn(), post: jest.fn() }; +jest.mock('../hooks/useV2Api', () => ({ useV2Api: () => mockApi })); + +jest.mock('../../context/SocketContext', () => ({ + useSocket: () => ({ + socket: { on: jest.fn(), off: jest.fn() }, + connected: false, + joinPod: jest.fn(), + leavePod: jest.fn(), + }), +})); +jest.mock('../../context/AuthContext', () => ({ + useAuth: () => ({ currentUser: { _id: 'me' } }), +})); + +const PAGE = 50; +const makePage = (prefix, count, startMinute) => Array.from({ length: count }, (_, i) => ({ + id: `${prefix}${i}`, + pod_id: 'p1', + user_id: 'u1', + content: `${prefix}-${i}`, + message_type: 'text', + created_at: new Date(Date.UTC(2026, 0, 1, 0, startMinute + i)).toISOString(), +})); + +const routeMock = (messagesByCall) => { + let call = 0; + mockApi.get.mockImplementation((url) => { + if (url.startsWith('/api/messages/')) { + const res = messagesByCall[Math.min(call, messagesByCall.length - 1)]; + call += 1; + return Promise.resolve(res); + } + if (url.includes('/agents')) return Promise.resolve({ agents: [] }); + return Promise.resolve({ _id: 'p1', name: 'Pod', members: [] }); + }); +}; + +describe('useV2PodDetail pagination', () => { + beforeEach(() => jest.clearAllMocks()); + + it('reports hasMore when the first page comes back full', async () => { + routeMock([makePage('a', PAGE, 100)]); + const { result } = renderHook(() => useV2PodDetail('p1')); + await waitFor(() => expect(result.current.messages).toHaveLength(PAGE)); + expect(result.current.hasMore).toBe(true); + }); + + it('reports no more history when the first page is short', async () => { + routeMock([makePage('a', 3, 100)]); + const { result } = renderHook(() => useV2PodDetail('p1')); + await waitFor(() => expect(result.current.messages).toHaveLength(3)); + expect(result.current.hasMore).toBe(false); + }); + + it('loadOlder requests `before` the oldest held message and PREPENDS the page', async () => { + const first = makePage('new', PAGE, 100); + const older = makePage('old', 10, 0); + routeMock([first, older]); + + const { result } = renderHook(() => useV2PodDetail('p1')); + await waitFor(() => expect(result.current.messages).toHaveLength(PAGE)); + + await act(async () => { await result.current.loadOlder(); }); + + const olderCall = mockApi.get.mock.calls + .map((c) => c[0]) + .find((u) => typeof u === 'string' && u.includes('before=')); + expect(olderCall).toContain(`before=${encodeURIComponent(first[0].created_at)}`); + + // Prepended, chronological, nothing lost. + expect(result.current.messages).toHaveLength(PAGE + 10); + expect(result.current.messages[0].id).toBe('old0'); + expect(result.current.messages[result.current.messages.length - 1].id).toBe(`new${PAGE - 1}`); + // A short older page means we have reached the beginning. + expect(result.current.hasMore).toBe(false); + }); + + it('does not duplicate a message the socket delivered while the page was in flight', async () => { + const first = makePage('a', PAGE, 100); + // The older page overlaps the newest list by one id. + const overlapping = [...makePage('old', 5, 0), first[0]]; + routeMock([first, overlapping]); + + const { result } = renderHook(() => useV2PodDetail('p1')); + await waitFor(() => expect(result.current.messages).toHaveLength(PAGE)); + + await act(async () => { await result.current.loadOlder(); }); + + const ids = result.current.messages.map((m) => m.id); + expect(new Set(ids).size).toBe(ids.length); + expect(result.current.messages).toHaveLength(PAGE + 5); + }); + + it('is a no-op with no messages, so the first render cannot fire a bogus cursor', async () => { + routeMock([[]]); + const { result } = renderHook(() => useV2PodDetail('p1')); + await waitFor(() => expect(result.current.loading).toBe(false)); + + await act(async () => { await result.current.loadOlder(); }); + + expect(mockApi.get.mock.calls.filter((c) => String(c[0]).includes('before='))) + .toHaveLength(0); + }); + + it('keeps hasMore true when a page request fails, so the button can be retried', async () => { + const first = makePage('a', PAGE, 100); + let call = 0; + mockApi.get.mockImplementation((url) => { + if (url.startsWith('/api/messages/')) { + call += 1; + if (call === 1) return Promise.resolve(first); + return Promise.reject(new Error('network')); + } + if (url.includes('/agents')) return Promise.resolve({ agents: [] }); + return Promise.resolve({ _id: 'p1', name: 'Pod', members: [] }); + }); + + const { result } = renderHook(() => useV2PodDetail('p1')); + await waitFor(() => expect(result.current.hasMore).toBe(true)); + + await act(async () => { await result.current.loadOlder(); }); + + expect(result.current.hasMore).toBe(true); + expect(result.current.loadingOlder).toBe(false); + }); +}); diff --git a/frontend/src/v2/components/V2PodChat.tsx b/frontend/src/v2/components/V2PodChat.tsx index 4d506b75..ca6f8c17 100644 --- a/frontend/src/v2/components/V2PodChat.tsx +++ b/frontend/src/v2/components/V2PodChat.tsx @@ -1,4 +1,6 @@ -import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import React, { + useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState, +} from 'react'; import V2Avatar from './V2Avatar'; import V2MessageBubble from './V2MessageBubble'; import { @@ -170,7 +172,10 @@ const Icon = ({ d }: { d: string }) => ( const V2PodChat: React.FC = ({ detail, firstRunVisible = false, inspectorCollapsed, onToggleInspector, onOpenMember, onOpenInvite, onOpenFile, onOpenMobileNav }) => { const { t, i18n } = useTranslation(); const numberFormatter = new Intl.NumberFormat(i18n.resolvedLanguage || i18n.language || 'en'); - const { pod, members, messages, agents, sendMessage, loading, error } = detail; + const { + pod, members, messages, agents, sendMessage, loading, error, + hasMore, loadingOlder, loadOlder, + } = detail; const api = useV2Api(); const { socket, connected } = useSocket(); const { currentUser } = useAuth(); @@ -190,6 +195,7 @@ const V2PodChat: React.FC = ({ detail, firstRunVisible = false, const deliveryHintShownPodsRef = useRef>(new Set()); const [mode, setMode] = useState(pod ? readMode(pod._id) : 'plan'); const messagesEndRef = useRef(null); + const messagesContainerRef = useRef(null); const fileInputRef = useRef(null); const composerInputRef = useRef(null); const mentionDropdownRef = useRef(null); @@ -302,9 +308,33 @@ const V2PodChat: React.FC = ({ detail, firstRunVisible = false, } }, [starterInviteUrl]); + // Auto-scroll belongs to NEW messages only. Keyed on `messages.length` this + // also fired when a page of history was prepended, yanking the reader from + // the older message they had just asked for straight back to the bottom — + // which reads as "load older is broken". Key on the newest message's id so + // prepends are ignored. + const newestMessageId = messages.length > 0 ? messages[messages.length - 1].id : null; useEffect(() => { messagesEndRef.current?.scrollIntoView({ behavior: 'smooth' }); - }, [messages.length]); + }, [newestMessageId]); + + // Prepending changes scrollHeight, so without this the viewport jumps. Hold + // the reader's position by restoring the distance from the BOTTOM, which is + // invariant under a prepend. + const scrollAnchorRef = useRef(null); + const handleLoadOlder = useCallback(async () => { + const el = messagesContainerRef.current; + scrollAnchorRef.current = el ? el.scrollHeight - el.scrollTop : null; + await loadOlder(); + }, [loadOlder]); + + useLayoutEffect(() => { + const el = messagesContainerRef.current; + const anchor = scrollAnchorRef.current; + if (!el || anchor == null) return; + el.scrollTop = el.scrollHeight - anchor; + scrollAnchorRef.current = null; + }, [messages]); // Removed: Lead-pill computation. The "Lead" label was just `idx === 0`, // which made whichever agent installed first (usually auto-installed @@ -869,7 +899,19 @@ const V2PodChat: React.FC = ({ detail, firstRunVisible = false, )} -
+
+ {hasMore && ( +
+ +
+ )} {error && (
{error} diff --git a/frontend/src/v2/hooks/useV2PodDetail.ts b/frontend/src/v2/hooks/useV2PodDetail.ts index 3ad31c18..09b38196 100644 --- a/frontend/src/v2/hooks/useV2PodDetail.ts +++ b/frontend/src/v2/hooks/useV2PodDetail.ts @@ -103,12 +103,29 @@ export interface UseV2PodDetailResult { agents: V2Agent[]; loading: boolean; error: string | null; + /** A full page came back, so older history probably exists. */ + hasMore: boolean; + loadingOlder: boolean; + loadOlder: () => Promise; refresh: () => Promise; sendMessage: (content: string, messageType?: string, replyToMessageId?: string) => Promise; } const REQUEST_TIMEOUT_MS = 8000; const SEND_TIMEOUT_MS = 20000; +// One page of history. The backend clamps `limit` to [1,200]; 50 matches the +// showcase reader so both surfaces page at the same rate. +const PAGE_SIZE = 50; + +// Merge two message lists by id, newest-last. Used when prepending a page of +// history: a plain concat would duplicate anything the socket delivered while +// the request was in flight. +const mergeMessagesById = (a: V2Message[], b: V2Message[]): V2Message[] => { + const byId = new Map(); + for (const m of a) byId.set(String(m.id), m); + for (const m of b) byId.set(String(m.id), m); + return chronologicalMessages([...byId.values()]); +}; const normalizeMessage = (raw: V2Message): V2Message => { const rawUserId = raw.userId; @@ -162,6 +179,8 @@ export const useV2PodDetail = (podId: string | null): UseV2PodDetailResult => { const [agents, setAgents] = useState([]); const [loading, setLoading] = useState(false); const [error, setError] = useState(null); + const [hasMore, setHasMore] = useState(false); + const [loadingOlder, setLoadingOlder] = useState(false); const fetchPod = useCallback(async (id: string) => { const result = await api.get(`/api/pods/${id}`, { timeout: REQUEST_TIMEOUT_MS }); @@ -170,16 +189,60 @@ export const useV2PodDetail = (podId: string | null): UseV2PodDetailResult => { const fetchMessages = useCallback(async (id: string) => { try { - const data = await api.get(`/api/messages/${id}?limit=50`, { timeout: REQUEST_TIMEOUT_MS }); + const data = await api.get( + `/api/messages/${id}?limit=${PAGE_SIZE}`, + { timeout: REQUEST_TIMEOUT_MS }, + ); const list = Array.isArray(data) ? data : []; setMessages(chronologicalMessages(list.map(normalizeMessage))); + // This endpoint returns a bare array with no envelope, so end-of-history + // is inferred: a short page means there is nothing older behind it. + setHasMore(list.length >= PAGE_SIZE); } catch (err) { const e = err as { response?: { status?: number } }; - if (e.response?.status === 404) setMessages([]); - else throw err; + if (e.response?.status === 404) { + setMessages([]); + setHasMore(false); + } else throw err; } }, [api]); + /** + * Prepend one page of older history. + * + * Anchors on the oldest message currently held and asks for everything + * strictly before it (`before` is already supported server-side and is the + * same cursor the showcase reader uses). Results are merged by id rather + * than concatenated, because the socket may have delivered messages while + * the request was in flight. + */ + const loadOlder = useCallback(async () => { + if (!podId || loadingOlder) return; + const oldest = messages[0]; + if (!oldest) return; + const cursor = oldest.created_at || oldest.createdAt; + if (!cursor) return; + + setLoadingOlder(true); + try { + const data = await api.get( + `/api/messages/${podId}?limit=${PAGE_SIZE}&before=${encodeURIComponent(cursor)}`, + { timeout: REQUEST_TIMEOUT_MS }, + ); + const older = (Array.isArray(data) ? data : []).map(normalizeMessage); + if (older.length === 0) { + setHasMore(false); + return; + } + setMessages((prev) => mergeMessagesById(older, prev)); + setHasMore(older.length >= PAGE_SIZE); + } catch { + // Leave hasMore alone so the same button can be retried. + } finally { + setLoadingOlder(false); + } + }, [api, podId, messages, loadingOlder]); + const fetchAgents = useCallback(async (id: string) => { try { const data = await api.get(`/api/registry/pods/${id}/agents`, { timeout: REQUEST_TIMEOUT_MS }); @@ -323,7 +386,9 @@ export const useV2PodDetail = (podId: string | null): UseV2PodDetailResult => { (m): m is V2PodMember => typeof m === 'object' && m !== null, ); - return { pod, members, messages, agents, loading, error, refresh, sendMessage }; + return { + pod, members, messages, agents, loading, error, hasMore, loadingOlder, loadOlder, refresh, sendMessage, + }; }; // Suppress unused import warning when MessagesResponse is not used below. diff --git a/frontend/src/v2/v2.css b/frontend/src/v2/v2.css index be9e1b77..2940faf6 100644 --- a/frontend/src/v2/v2.css +++ b/frontend/src/v2/v2.css @@ -1505,6 +1505,26 @@ font-weight: 600; } +/* "Load older messages" control — the pod chat equivalent of the showcase + pager. Same pill treatment so the two reading surfaces match. */ +.v2-chat__older { display: flex; justify-content: center; padding: 4px 0 12px; } +.v2-chat__older-btn { + padding: 7px 16px; + font: inherit; + font-size: 13px; + font-weight: 600; + color: var(--v2-text-secondary); + background: var(--v2-surface); + border: 1px solid var(--v2-border); + border-radius: 999px; + cursor: pointer; +} +.v2-chat__older-btn:hover:not(:disabled) { + color: var(--v2-text-primary); + border-color: var(--v2-text-tertiary); +} +.v2-chat__older-btn:disabled { opacity: 0.6; cursor: default; } + .v2-chat__messages { flex: 1; overflow-y: auto;