From 5b86162b74415985138b1953b51be3c51d88620c Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 27 Sep 2026 22:46:55 -0700 Subject: [PATCH 1/4] fix(v2): the sidebar's minute tick re-reads the row times (TASK-184) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The row time is a datum from the last /api/pods and the tick only recomputed the label, so a tab left open aged a frozen timestamp and read further from the truth the longer it stayed open. The tick now re-reads the datum, and so does becoming visible — the moment a row is actually read. useV2Pods.refresh takes {silent}, which swaps the data without touching loading/error: the sidebar renders a spinner while loading, so a non-silent poll would blink the whole list once a minute, and one failed poll would replace the last good rows with an error. Foreground only, so a background tab costs nothing. Tests: V2PodsSidebar.timestampFreshness (the label moves when the datum does; a hidden tab does not poll; the tick is a minute) and useV2PodsSilentRefresh (silent keeps loading false and keeps the last good rows on failure; the loud default is asserted beside it as the control). --- .../v2/__tests__/V2PodsSidebar.scale.test.tsx | 1 + .../V2PodsSidebar.timestampFreshness.test.tsx | 124 ++++++++++++++++++ .../__tests__/useV2PodsSilentRefresh.test.tsx | 95 ++++++++++++++ frontend/src/v2/components/V2PodsSidebar.tsx | 29 +++- frontend/src/v2/hooks/useV2Pods.ts | 20 ++- 5 files changed, 260 insertions(+), 9 deletions(-) create mode 100644 frontend/src/v2/__tests__/V2PodsSidebar.timestampFreshness.test.tsx create mode 100644 frontend/src/v2/__tests__/useV2PodsSilentRefresh.test.tsx diff --git a/frontend/src/v2/__tests__/V2PodsSidebar.scale.test.tsx b/frontend/src/v2/__tests__/V2PodsSidebar.scale.test.tsx index 93ef54ebd..a8fc75114 100644 --- a/frontend/src/v2/__tests__/V2PodsSidebar.scale.test.tsx +++ b/frontend/src/v2/__tests__/V2PodsSidebar.scale.test.tsx @@ -86,6 +86,7 @@ const renderSidebar = (pods, selectedPodId = 'sharpen', extra = {}) => render( attentionCountByPod={{ sharpen: 2, connectors: 1, naming: 0, hq: 91 }} podsState={{ pods, loading: false, error: null, createPod: mockCreatePod, patchLastMessage: jest.fn(), + refresh: jest.fn(() => Promise.resolve()), }} {...extra} /> diff --git a/frontend/src/v2/__tests__/V2PodsSidebar.timestampFreshness.test.tsx b/frontend/src/v2/__tests__/V2PodsSidebar.timestampFreshness.test.tsx new file mode 100644 index 000000000..630687be4 --- /dev/null +++ b/frontend/src/v2/__tests__/V2PodsSidebar.timestampFreshness.test.tsx @@ -0,0 +1,124 @@ +// @ts-nocheck +import React from 'react'; +import { act, render, screen } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import { i18nReady } from '../../i18n'; +import V2PodsSidebar from '../components/V2PodsSidebar'; + +// TASK-184. The row time is a DATUM from the last /api/pods, and the minute +// tick only recomputes the label — so a tab left open kept ageing a frozen +// timestamp and read further from the truth the longer it stayed open. This +// test drives the surface: the label moves when the datum does, and does not +// move while the tab is hidden. + +const NOW = Date.parse('2026-09-28T05:00:00.000Z'); +const iso = (msAgo: number) => new Date(NOW - msAgo).toISOString(); + +const row = (createdAt: string) => ([{ + _id: 'p1', + name: 'Sharpen', + type: 'team', + members: [{ _id: 'me', username: 'me', isBot: false }], + lastMessage: { content: 'x', createdAt, username: 'a' }, +}]); + +const mockRefresh = jest.fn(); +let mockSetPods = () => {}; +// `jest.mock` factories may only close over `mock`-prefixed names. +const mockInitialPods = row(new Date(NOW - 2 * 3600 * 1000).toISOString()); + +jest.mock('../hooks/useV2Pods', () => ({ + useV2Pods: () => { + const { useState } = require('react'); + const [pods, setPods] = useState(() => mockInitialPods); + mockSetPods = setPods; + return { + pods, + loading: false, + error: null, + createPod: jest.fn(), + patchLastMessage: jest.fn(), + refresh: mockRefresh, + }; + }, +})); + +jest.mock('../hooks/useV2Api', () => ({ + useV2Api: () => ({ get: jest.fn(() => Promise.resolve([])), post: jest.fn(), patch: jest.fn(), del: jest.fn() }), +})); + +jest.mock('../hooks/useV2Pinned', () => ({ + useV2Pinned: () => ({ pinned: new Set(), toggle: jest.fn(), isPinned: () => false }), +})); + +jest.mock('../../context/AuthContext', () => ({ + useAuth: () => ({ currentUser: { _id: 'me', username: 'me' } }), +})); + +let visibility = 'visible'; + +const renderSidebar = () => render( + + + , +); + +describe('V2PodsSidebar — row time follows the datum', () => { + beforeAll(async () => { await i18nReady; }); + + beforeEach(() => { + jest.clearAllMocks(); + jest.useFakeTimers({ now: NOW }); + visibility = 'visible'; + Object.defineProperty(document, 'visibilityState', { + configurable: true, + get: () => visibility, + }); + // The poll re-reads the row times; the surface must show the new datum. + mockRefresh.mockImplementation(() => { + mockSetPods(row(iso(0))); + return Promise.resolve(); + }); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('re-reads the datum on the minute tick and re-renders the label', async () => { + renderSidebar(); + expect(await screen.findByText('2h')).toBeInTheDocument(); + + await act(async () => { jest.advanceTimersByTime(60 * 1000); }); + + expect(mockRefresh).toHaveBeenCalledWith({ silent: true }); + // NOW + the tick's own minute = 1 minute after the refreshed datum. + expect(await screen.findByText('1m')).toBeInTheDocument(); + expect(screen.queryByText('2h')).not.toBeInTheDocument(); + }); + + it('does not poll a hidden tab, and re-reads the moment it becomes visible', async () => { + renderSidebar(); + expect(await screen.findByText('2h')).toBeInTheDocument(); + + visibility = 'hidden'; + await act(async () => { jest.advanceTimersByTime(10 * 60 * 1000); }); + expect(mockRefresh).not.toHaveBeenCalled(); + expect(screen.getByText('2h')).toBeInTheDocument(); + + visibility = 'visible'; + await act(async () => { document.dispatchEvent(new Event('visibilitychange')); }); + expect(mockRefresh).toHaveBeenCalledWith({ silent: true }); + }); + + it('the tick is a minute, not a render', async () => { + renderSidebar(); + await screen.findByText('2h'); + + await act(async () => { jest.advanceTimersByTime(59 * 1000); }); + expect(mockRefresh).not.toHaveBeenCalled(); + + await act(async () => { jest.advanceTimersByTime(1 * 1000); }); + expect(mockRefresh).toHaveBeenCalledTimes(1); + }); +}); diff --git a/frontend/src/v2/__tests__/useV2PodsSilentRefresh.test.tsx b/frontend/src/v2/__tests__/useV2PodsSilentRefresh.test.tsx new file mode 100644 index 000000000..fef4dce7e --- /dev/null +++ b/frontend/src/v2/__tests__/useV2PodsSilentRefresh.test.tsx @@ -0,0 +1,95 @@ +import { act, renderHook, waitFor } from '@testing-library/react'; +import { useV2Pods } from '../hooks/useV2Pods'; + +const mockApi = { + get: jest.fn(), + post: jest.fn(), + patch: jest.fn(), + del: jest.fn(), +}; + +jest.mock('../hooks/useV2Api', () => ({ + useV2Api: () => mockApi, +})); + +const row = (createdAt: string) => ({ + _id: 'p1', + name: 'Sharpen', + lastMessage: { content: 'x', createdAt, username: 'a' }, +}); + +// TASK-184. A poll that re-reads the sidebar's row times must not be +// observable as a state change: `loading` blanks the list into a spinner +// (V2PodsSidebar renders `v2-spinner` while loading), so a non-silent refresh +// on the minute tick would blink the whole sidebar once a minute, and a single +// failed poll would replace the last good rows with an error. Both arms are +// asserted here — silent, and the loud default that proves silent differs. +describe('useV2Pods silent refresh', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('re-reads the list in place without entering the loading state', async () => { + mockApi.get.mockResolvedValue([row('2026-09-28T04:00:00.000Z')]); + const { result } = renderHook(() => useV2Pods()); + await waitFor(() => expect(result.current.loading).toBe(false)); + + let release: (value: unknown) => void = () => {}; + mockApi.get.mockImplementationOnce(() => new Promise((resolve) => { release = resolve; })); + let pending: Promise = Promise.resolve(); + await act(async () => { + pending = result.current.refresh({ silent: true }); + }); + + // In flight: the loud path is `true` here (next test), so this is the + // observable difference the option buys. + expect(result.current.loading).toBe(false); + + await act(async () => { + release([row('2026-09-28T05:00:00.000Z')]); + await pending; + }); + expect(result.current.loading).toBe(false); + expect(result.current.pods[0].lastMessage?.createdAt).toBe('2026-09-28T05:00:00.000Z'); + }); + + it('keeps the last good rows and the last error when a silent read fails', async () => { + mockApi.get.mockResolvedValue([row('2026-09-28T04:00:00.000Z')]); + const { result } = renderHook(() => useV2Pods()); + await waitFor(() => expect(result.current.loading).toBe(false)); + + mockApi.get.mockRejectedValueOnce(new Error('network down')); + await act(async () => { + await result.current.refresh({ silent: true }); + }); + + expect(result.current.error).toBeNull(); + expect(result.current.pods).toHaveLength(1); + expect(result.current.pods[0].lastMessage?.createdAt).toBe('2026-09-28T04:00:00.000Z'); + }); + + it('the loud default does enter the loading state and does surface a failure', async () => { + mockApi.get.mockResolvedValue([row('2026-09-28T04:00:00.000Z')]); + const { result } = renderHook(() => useV2Pods()); + await waitFor(() => expect(result.current.loading).toBe(false)); + + let release: (value: unknown) => void = () => {}; + mockApi.get.mockImplementationOnce(() => new Promise((resolve) => { release = resolve; })); + let pending: Promise = Promise.resolve(); + await act(async () => { + pending = result.current.refresh(); + }); + expect(result.current.loading).toBe(true); + await act(async () => { + release([row('2026-09-28T05:00:00.000Z')]); + await pending; + }); + expect(result.current.loading).toBe(false); + + mockApi.get.mockRejectedValueOnce(new Error('network down')); + await act(async () => { + await result.current.refresh(); + }); + expect(result.current.error).toBe('network down'); + }); +}); diff --git a/frontend/src/v2/components/V2PodsSidebar.tsx b/frontend/src/v2/components/V2PodsSidebar.tsx index 6c482acc3..8c09bd064 100644 --- a/frontend/src/v2/components/V2PodsSidebar.tsx +++ b/frontend/src/v2/components/V2PodsSidebar.tsx @@ -197,7 +197,7 @@ const V2PodsSidebar: React.FC = ({ const { pinned, toggle: togglePin } = useV2Pinned(); const ownPodsState = useV2Pods(); const { - pods, loading, error, createPod, + pods, loading, error, createPod, refresh: refreshPods, } = podsState || ownPodsState; const searchRef = useRef(null); const [query, setQuery] = useState(''); @@ -219,14 +219,35 @@ const V2PodsSidebar: React.FC = ({ }, [createReturnToConnectors]); // The visit log is written by the layout when a pod opens; re-read it here so - // Recent reorders without a reload. Times refresh once a minute. + // Recent reorders without a reload. useEffect(() => { setVisits(readPodVisits()); }, [selectedPodId]); + + // Every row's time is a DATUM from the last /api/pods; the minute tick only + // recomputes the label, so a tab left open keeps ageing a frozen timestamp and + // reads further from the truth the longer it stays open (TASK-184). So the tick + // also re-reads the datum, and so does becoming visible — the moment a row is + // actually read. Silent, so a poll never blinks the list into its spinner; and + // foreground-only, so a background tab costs nothing. + const refreshPodsRef = useRef(refreshPods); useEffect(() => { - const timer = window.setInterval(() => setNow(Date.now()), 60 * 1000); - return () => window.clearInterval(timer); + refreshPodsRef.current = refreshPods; + }, [refreshPods]); + const tick = useCallback(() => { + setNow(Date.now()); + if (document.visibilityState === 'visible') void refreshPodsRef.current?.({ silent: true }); }, []); + useEffect(() => { + const timer = window.setInterval(tick, 60 * 1000); + // visibilitychange fires in both directions; `tick` decides what to do. + const onVisibility = () => tick(); + document.addEventListener('visibilitychange', onVisibility); + return () => { + window.clearInterval(timer); + document.removeEventListener('visibilitychange', onVisibility); + }; + }, [tick]); // ⌘K / Ctrl+K focuses the search box from anywhere in the shell. useEffect(() => { diff --git a/frontend/src/v2/hooks/useV2Pods.ts b/frontend/src/v2/hooks/useV2Pods.ts index a3025ebc9..e93271b0a 100644 --- a/frontend/src/v2/hooks/useV2Pods.ts +++ b/frontend/src/v2/hooks/useV2Pods.ts @@ -35,7 +35,11 @@ export interface UseV2PodsResult { pods: V2Pod[]; loading: boolean; error: string | null; - refresh: () => Promise; + // `silent` re-reads the list in place: it swaps the data on success WITHOUT + // touching `loading`/`error`, so a background poll cannot blink a list into + // its spinner or replace the last good rows with a transient failure. + // Non-silent is the mount/retry path and owns the loading state. + refresh: (options?: { silent?: boolean }) => Promise; createPod: ( name: string, description?: string, @@ -52,17 +56,23 @@ export const useV2Pods = (): UseV2PodsResult => { const [loading, setLoading] = useState(true); const [error, setError] = useState(null); - const refresh = useCallback(async () => { - setLoading(true); - setError(null); + const refresh = useCallback(async (options?: { silent?: boolean }) => { + const silent = Boolean(options?.silent); + if (!silent) { + setLoading(true); + setError(null); + } try { const data = await api.get('/api/pods'); setPods(Array.isArray(data) ? data : []); } catch (err) { + // A failed poll keeps the last good rows and the last error: silence is + // the point. Only the mount/retry path may surface a failure. + if (silent) return; const e = err as { response?: { data?: { error?: string; msg?: string } }; message?: string }; setError(e.response?.data?.error || e.response?.data?.msg || e.message || 'Failed to load pods'); } finally { - setLoading(false); + if (!silent) setLoading(false); } }, [api]); From 8a972c930d7a9f184e10da27f37b4b8c6197c998 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 27 Sep 2026 22:53:14 -0700 Subject: [PATCH 2/4] fix(v2): a successful poll clears a stale error, so a failed mount recovers (TASK-184) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sprint-review's finding, measured end to end: mount fails, so `error` is set and V2PodsSidebar (:501/:502) renders the message INSTEAD of the rows; the next silent poll succeeded every 60s, swapped the data, and could not clear the one state that hides the list. The poll refreshed data nobody could see. A successful read is positive evidence the stored error is stale, so the success path clears it in both modes. The invariant that mattered is preserved: a failed poll still never SETS an error, so a transient failure cannot replace the last good rows. The complement is tested too — a silent failure does not clear an error it did not earn. New cell: V2PodsSidebar.pollRecovery mocks the API LAYER rather than the hook, so the real useV2Pods and the real sidebar run end to end: error and no rows, then a successful tick brings the rows back with no reload; and while the poll keeps failing the error stays and no rows appear. Also corrected in the PR body: a hidden tab skips the REQUEST, not the work — the interval still fires and still calls setNow. --- .../V2PodsSidebar.pollRecovery.test.tsx | 99 +++++++++++++++++++ .../__tests__/useV2PodsSilentRefresh.test.tsx | 32 ++++++ frontend/src/v2/hooks/useV2Pods.ts | 16 ++- 3 files changed, 143 insertions(+), 4 deletions(-) create mode 100644 frontend/src/v2/__tests__/V2PodsSidebar.pollRecovery.test.tsx diff --git a/frontend/src/v2/__tests__/V2PodsSidebar.pollRecovery.test.tsx b/frontend/src/v2/__tests__/V2PodsSidebar.pollRecovery.test.tsx new file mode 100644 index 000000000..23dcd1143 --- /dev/null +++ b/frontend/src/v2/__tests__/V2PodsSidebar.pollRecovery.test.tsx @@ -0,0 +1,99 @@ +// @ts-nocheck +import React from 'react'; +import { act, render, screen } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import { i18nReady } from '../../i18n'; +import V2PodsSidebar from '../components/V2PodsSidebar'; + +// TASK-184, the recovery cell. This file mocks the API LAYER, not the hook, so +// the real useV2Pods and the real sidebar are exercised end to end: a first +// load that fails must not leave a tab that polls successfully every minute +// showing an error and no rows forever. + +const NOW = Date.parse('2026-09-28T05:00:00.000Z'); +const mockGet = jest.fn(); +// Stable identity: a fresh object per render would change `refresh`'s dependency +// and re-run the mount effect forever (the real hook memoizes this). +const mockApi = { get: mockGet, post: jest.fn(), patch: jest.fn(), del: jest.fn() }; + +jest.mock('../hooks/useV2Api', () => ({ + useV2Api: () => mockApi, +})); + +jest.mock('../hooks/useV2Pinned', () => ({ + useV2Pinned: () => ({ pinned: new Set(), toggle: jest.fn(), isPinned: () => false }), +})); + +jest.mock('../../context/AuthContext', () => ({ + useAuth: () => ({ currentUser: { _id: 'me', username: 'me' } }), +})); + +// The real hook fetches on mount, so the first paint is `loading: true` and the +// settled state arrives a few microtasks later; fake timers do not advance +// microtasks, so drain them explicitly instead of waiting on a timer. +const settle = async () => { + await act(async () => { await Promise.resolve(); }); +}; + +const podRow = [{ + _id: 'p1', + name: 'Sharpen', + type: 'team', + members: [{ _id: 'me', username: 'me', isBot: false }], + lastMessage: { + content: 'x', + createdAt: new Date(NOW - 2 * 3600 * 1000).toISOString(), + username: 'a', + }, +}]; + +describe('V2PodsSidebar — a failed mount recovers on the next poll', () => { + beforeAll(async () => { await i18nReady; }); + + beforeEach(() => { + jest.clearAllMocks(); + jest.useFakeTimers({ now: NOW }); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('shows the rows again once a poll succeeds, without a reload', async () => { + mockGet.mockRejectedValueOnce(new Error('network down')); + render( + + + , + ); + + await settle(); + expect(screen.getByText('network down')).toBeInTheDocument(); + expect(screen.queryByText('Sharpen')).not.toBeInTheDocument(); + + mockGet.mockResolvedValueOnce(podRow); + await act(async () => { jest.advanceTimersByTime(60 * 1000); }); + + expect(mockGet).toHaveBeenCalledWith('/api/pods'); + expect(screen.getByText('Sharpen')).toBeInTheDocument(); + expect(screen.queryByText('network down')).not.toBeInTheDocument(); + }); + + it('keeps the error while the poll keeps failing', async () => { + mockGet.mockRejectedValueOnce(new Error('network down')); + render( + + + , + ); + await settle(); + expect(screen.getByText('network down')).toBeInTheDocument(); + + mockGet.mockRejectedValue(new Error('still down')); + await act(async () => { jest.advanceTimersByTime(3 * 60 * 1000); }); + await settle(); + + expect(screen.getByText('network down')).toBeInTheDocument(); + expect(screen.queryByText('Sharpen')).not.toBeInTheDocument(); + }); +}); diff --git a/frontend/src/v2/__tests__/useV2PodsSilentRefresh.test.tsx b/frontend/src/v2/__tests__/useV2PodsSilentRefresh.test.tsx index fef4dce7e..406ffc502 100644 --- a/frontend/src/v2/__tests__/useV2PodsSilentRefresh.test.tsx +++ b/frontend/src/v2/__tests__/useV2PodsSilentRefresh.test.tsx @@ -92,4 +92,36 @@ describe('useV2Pods silent refresh', () => { }); expect(result.current.error).toBe('network down'); }); + it('a successful poll clears a stale error instead of refreshing data nobody can see', async () => { + // The cell the first draft missed: the mount FAILED, so `error` is set and + // V2PodsSidebar (:501/:502) renders the error INSTEAD of the rows. A poll + // that swaps the data but cannot clear the error leaves the view broken + // while fetching correctly every minute. + mockApi.get.mockRejectedValueOnce(new Error('network down')); + const { result } = renderHook(() => useV2Pods()); + await waitFor(() => expect(result.current.error).toBe('network down')); + expect(result.current.pods).toHaveLength(0); + + mockApi.get.mockResolvedValueOnce([row('2026-09-28T05:00:00.000Z')]); + await act(async () => { + await result.current.refresh({ silent: true }); + }); + + expect(result.current.error).toBeNull(); + expect(result.current.pods).toHaveLength(1); + }); + + it('a silent failure still never clears an error it did not earn', async () => { + // The complement: only a SUCCESS is evidence the error is stale. + mockApi.get.mockRejectedValueOnce(new Error('network down')); + const { result } = renderHook(() => useV2Pods()); + await waitFor(() => expect(result.current.error).toBe('network down')); + + mockApi.get.mockRejectedValueOnce(new Error('still down')); + await act(async () => { + await result.current.refresh({ silent: true }); + }); + + expect(result.current.error).toBe('network down'); + }); }); diff --git a/frontend/src/v2/hooks/useV2Pods.ts b/frontend/src/v2/hooks/useV2Pods.ts index e93271b0a..23cb82748 100644 --- a/frontend/src/v2/hooks/useV2Pods.ts +++ b/frontend/src/v2/hooks/useV2Pods.ts @@ -35,10 +35,11 @@ export interface UseV2PodsResult { pods: V2Pod[]; loading: boolean; error: string | null; - // `silent` re-reads the list in place: it swaps the data on success WITHOUT - // touching `loading`/`error`, so a background poll cannot blink a list into - // its spinner or replace the last good rows with a transient failure. - // Non-silent is the mount/retry path and owns the loading state. + // `silent` re-reads the list in place: it swaps the data on success without + // setting `loading` or an error, so a background poll cannot blink a list + // into its spinner or replace the last good rows with a transient failure. + // Non-silent is the mount/retry path and owns the loading state. Both modes + // clear an existing error on success — see the success path below. refresh: (options?: { silent?: boolean }) => Promise; createPod: ( name: string, @@ -65,6 +66,13 @@ export const useV2Pods = (): UseV2PodsResult => { try { const data = await api.get('/api/pods'); setPods(Array.isArray(data) ? data : []); + // A successful read is evidence the stored error is stale, so it clears + // it — in BOTH modes. Without this the silent poll could never recover + // the view it was added for: V2PodsSidebar renders the list only on + // `!loading && !error`, so a failed first load hid the rows behind an + // error that a later successful poll could refresh the data but never + // un-hide. Silence still means a poll never SETS a failure. + setError(null); } catch (err) { // A failed poll keeps the last good rows and the last error: silence is // the point. Only the mount/retry path may surface a failure. From b8048c722b00dd9e58ca47657fb386bc2e5780da Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 27 Sep 2026 23:31:31 -0700 Subject: [PATCH 3/4] fix(v2): the pod list opts out of scroll anchoring, so a fresh poll does not drag it (TASK-184) The minute tick added in 5b86162b re-reads /api/pods, and the rows re-sort by lastMessage. A message to a pod in view therefore moves a row, and native scroll anchoring drags the list to preserve that row offset: ux-lead measured 4 of 17 rows staying in view with Everything open and scrolled. Opting the scroller out holds 16 of 17. Same property the chat transcript uses for the opposite reason. There, anchoring is disabled only during a managed history prepend so late media above the viewport stays stable; these rows are fixed-height text with no late layout to lose. The guard in v2-layout-invariants reads the LAST .v2-pods__list block, because the selector is declared twice (v2.css:1096 from #251 and v2.css:10997 from #1578) and the later top-level block wins every conflicting declaration. The first block is fully superseded; that is noted rather than cleaned up here. --- .../v2/__tests__/v2-layout-invariants.test.ts | 20 +++++++++++++++++++ frontend/src/v2/v2.css | 7 +++++++ 2 files changed, 27 insertions(+) diff --git a/frontend/src/v2/__tests__/v2-layout-invariants.test.ts b/frontend/src/v2/__tests__/v2-layout-invariants.test.ts index 4ff57469b..919d4eb2f 100644 --- a/frontend/src/v2/__tests__/v2-layout-invariants.test.ts +++ b/frontend/src/v2/__tests__/v2-layout-invariants.test.ts @@ -1157,6 +1157,26 @@ describe('v2 layout invariants (CSS rule presence)', () => { expect(thread).toContain('delete el.dataset.historyAnchor'); }); + test('the pod list opts out of native anchoring, because the minute poll re-sorts it under the reader', () => { + // ux-lead gate on #1989: with Everything open and scrolled, a message to a + // pod in view moves that row to the top, native anchoring drags the list + // ~14 rows and 4 of 17 rows stayed in view. Opting the scroller out holds + // 16 of 17. Same property the chat transcript uses, for the opposite + // reason — see the note on the declaration in v2.css. + const list = lastRuleBody(v2, '.v2-pods__list'); + // The rule being reset must still exist, or a green here means nothing. + expect(list).toContain('overflow-y: auto'); + expect(list).toContain('overflow-anchor: none'); + expect(podsSidebar).toContain('className="v2-pods__list"'); + // `.v2-pods__list` is declared twice (from #251 and #1578 "sidebar at + // scale"), both top-level, so the LATER block wins every conflicting + // declaration and is the effective rule — `ruleBody`, the first match, + // reads the superseded one. This asserts that distinction still exists: if + // the duplicate is ever removed, re-read this guard rather than letting it + // silently switch which block it pins. + expect(list).not.toBe(ruleBody(v2, '.v2-pods__list')); + }); + test('history recovery is positioned against the chat viewport, outside the scroller', () => { const transcript = ruleBody(v2, '.v2-thread__transcript'); expect(transcript).toContain('position: relative'); diff --git a/frontend/src/v2/v2.css b/frontend/src/v2/v2.css index f271359ae..072a303ef 100644 --- a/frontend/src/v2/v2.css +++ b/frontend/src/v2/v2.css @@ -11002,6 +11002,13 @@ body.modern-ui.v2-canvas { gap: 0; padding: 0 0 8px; overflow-y: auto; + /* The minute poll re-reads /api/pods and the rows re-sort by lastMessage, so + a message to any pod moves a row and native anchoring drags the list to + keep that row's viewport offset — 4 of 17 rows stayed in view (ux-lead + gate on #1989). Opting this scroller out holds 16 of 17. Unlike + .v2-chat__messages, which keeps anchoring at rest for late media above the + viewport, these rows are fixed-height text with no late layout to lose. */ + overflow-anchor: none; } .v2-pods__section { From 177ba436fa0f7824d03c8eb2b207baba9b65a226 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 29 Sep 2026 04:34:43 -0700 Subject: [PATCH 4/4] test(v2): drop an assertion that could never fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sprint-review's code-delta gate on #1989 found it and they were right, in the direction I did not expect: not a false red, an inert line. `expect(list).not.toBe(ruleBody(v2, '.v2-pods__list'))` was written to pin the choice of `lastRuleBody` — read the LATER, effective `.v2-pods__list` block rather than the superseded one — and to force a re-read if the duplicate were ever de-duped. It could not do either. `lastRuleBody` slices from the leading `\n`; `ruleBody` slices from `lineStart + 1` and drops it. The two therefore differ by whitespace whatever the stylesheet contains, so the assertion is true by construction and can never fire. Measured both directions rather than reasoned from the helpers: - raw compare, superseded block deleted -> 153/153 GREEN (inert) - trimmed compare, same deletion -> 1 failed (it bites) so the mechanism is the whitespace, and trimming is not a fix: it reds on the harmless de-dup the original comment claimed to guard against. Deleted, and the comment now records why not to re-add it. The property is still carried, in both hazard directions, by the two `toContain`s above: the declaration missing from the effective block, or the effective block removed so the lookup falls through to the old one. 118/118 suites, 1034 tests, tsc 0, eslint 0 errors (the file's 6 warnings are unchanged from base). --- .../v2/__tests__/v2-layout-invariants.test.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/frontend/src/v2/__tests__/v2-layout-invariants.test.ts b/frontend/src/v2/__tests__/v2-layout-invariants.test.ts index 919d4eb2f..0135624e0 100644 --- a/frontend/src/v2/__tests__/v2-layout-invariants.test.ts +++ b/frontend/src/v2/__tests__/v2-layout-invariants.test.ts @@ -1170,11 +1170,18 @@ describe('v2 layout invariants (CSS rule presence)', () => { expect(podsSidebar).toContain('className="v2-pods__list"'); // `.v2-pods__list` is declared twice (from #251 and #1578 "sidebar at // scale"), both top-level, so the LATER block wins every conflicting - // declaration and is the effective rule — `ruleBody`, the first match, - // reads the superseded one. This asserts that distinction still exists: if - // the duplicate is ever removed, re-read this guard rather than letting it - // silently switch which block it pins. - expect(list).not.toBe(ruleBody(v2, '.v2-pods__list')); + // declaration and is the effective rule — hence `lastRuleBody` here, which + // reads it, rather than `ruleBody`, which reads the superseded one. + // + // Do NOT add `expect(list).not.toBe(ruleBody(...))` to pin that choice. It + // cannot fail: `lastRuleBody` slices from the leading newline and `ruleBody` + // from `lineStart + 1`, so the two differ by whitespace whatever the sheet + // contains (measured — deleting the superseded block outright leaves this + // suite 153/153 green). Trimming the compare to make it bite is worse: it + // then reds on that very de-dup, which is harmless. The two `toContain`s + // above are what carry the property, and they fail in both hazard + // directions — declaration missing from the effective block, or the + // effective block gone so this falls through to the old one. }); test('history recovery is positioned against the chat viewport, outside the scroller', () => {