From 704d0f4a81ded3976b999e1b5434f09229e99577 Mon Sep 17 00:00:00 2001 From: Ronen Mars Date: Sat, 1 Aug 2026 06:24:12 +0300 Subject: [PATCH] fix(conversation): stop scroll-up blank gaps and anchor teleports Scrolling up through a paused conversation opened progressively larger blank regions and could teleport the viewport. Three defects compound, all measured against the real 80-message session that produced the report. The delta poll resumed from the max message index already held, but the streamer's after_index window is inclusive, so every tick re-fetched the tail message and stored it as a duplicate-id row. The paused conversation grew from 80 to 156 rows during one capture, each append adding ~342pt of content and firing an anchor pass under the user's scroll. drawDistance was left at the 250px iOS default while single markdown-table rows measure 3,114pt against a 654pt viewport, so any real flick outran the render window and just-passed rows unmounted into visible blanks. getItemType lumped collapsed 46pt thinking headers together with tool and diff cards, poisoning the per-type average height FlashList uses to place unmeasured rows. Content height thrashed by up to 19,898pt in a single relayout, which maintainVisibleContentPosition absorbed as the observed viewport jumps. After the fixes the same scripted protocol shows zero data churn while paused, relayouts capped at 1,742pt, and every offset correction matching its paired content shift within 40pt. Three existing tests mocked after_index as exclusive, which encoded the bug rather than the server contract; their first-hop cursors are corrected alongside the fix. --- .../unit/hooks/useConversations.test.tsx | 49 +++- .../conversation/ConversationHistoryList.tsx | 124 ++++++---- e2e/07_conversation_scroll_gaps.yaml | 224 ++++++++++++++++++ e2e/fixtures/conv-scroll-gaps.json | 210 ++++++++++++++++ e2e/mock-server.js | 32 +++ hooks/useConversations.ts | 26 +- package.json | 2 +- 7 files changed, 611 insertions(+), 56 deletions(-) create mode 100644 e2e/07_conversation_scroll_gaps.yaml create mode 100644 e2e/fixtures/conv-scroll-gaps.json diff --git a/__tests__/unit/hooks/useConversations.test.tsx b/__tests__/unit/hooks/useConversations.test.tsx index db065e48..e3810c6c 100644 --- a/__tests__/unit/hooks/useConversations.test.tsx +++ b/__tests__/unit/hooks/useConversations.test.tsx @@ -405,7 +405,7 @@ describe('useConversation — { resume } delta on the tail view', () => { // Tail first page: messages 0..2, total 3. No has_more_newer field (plain tail). metaHandlers.srv_resume = () => Promise.resolve({ status: 200, etag: '"v1"', body: rawConversationPage('c_r', ['a', 'b', 'c']) }) - // after_index=2 delta: two new messages 3,4 out of total 5, no more newer. + // after_index=3 delta: two new messages 3,4 out of total 5, no more newer. handlers.srv_resume = (path) => { paths.push(path) return Promise.resolve( @@ -422,13 +422,41 @@ describe('useConversation — { resume } delta on the tail view', () => { await result.current.fetchNewerPage() await waitFor(() => expect(result.current.data!.messages.length).toBe(5)) - expect(paths.some((p) => p.includes('after_index=2'))).toBe(true) + // The server window is [after_index, …) INCLUSIVE, so resuming from held + // max index 2 must request after_index=3 — asking with 2 re-downloads the + // tail message and appends a duplicate-id row on every delta poll. + expect(paths.some((p) => p.includes('after_index=3'))).toBe(true) + expect(paths.some((p) => p.includes('after_index=2'))).toBe(false) expect(paths.some((p) => p.includes('msg_limit=80'))).toBe(true) // Delta path must NOT send If-None-Match (it's a plain get, not getWithMeta). const indexes = result.current.data!.messages.map((m) => m.messageIndex) expect(indexes).toEqual([0, 1, 2, 3, 4]) }) + it('drops duplicate message indexes when a delta page overlaps already-held rows', async () => { + setActiveServers(['srv_dup']) + // Tail first page: messages 0..2. + metaHandlers.srv_dup = () => + Promise.resolve({ status: 200, etag: '"v1"', body: rawConversationPage('c_d', ['a', 'b', 'c']) }) + // Overlapping delta: the server re-sends index 2 alongside the new index 3 + // (inclusive-window behavior). The flatten must keep exactly one row per + // message_index — duplicate ids break FlashList's keyExtractor and its + // maintainVisibleContentPosition anchor (phantom blank space + scroll jumps). + handlers.srv_dup = () => + Promise.resolve(rawAnchoredPage('c_d', 2, 2, 4, { has_more_newer: false, next_after_index: null })) + + const { result } = await renderHook(() => useConversation('srv_dup', 'c_d'), { wrapper: createWrapper() }) + await waitFor(() => expect(result.current.data!.messages.length).toBe(3)) + + await result.current.fetchNewerPage() + await waitFor(() => expect(result.current.data!.messages.length).toBe(4)) + + const indexes = result.current.data!.messages.map((m) => m.messageIndex) + expect(indexes).toEqual([0, 1, 2, 3]) + const ids = result.current.data!.messages.map((m) => m.id) + expect(new Set(ids).size).toBe(ids.length) + }) + it('does not expose a resume cursor when no messages are cached (fresh install)', async () => { setActiveServers(['srv_fresh']) metaHandlers.srv_fresh = () => @@ -796,7 +824,9 @@ describe('useConversation — consolidated delta trigger', () => { const { result } = await renderHook(() => useConversation('srv_mt', 'c_mt'), { wrapper }) - await waitFor(() => expect(paths.filter((p) => p.includes('after_index=2'))).toHaveLength(1)) + // Warm cache holds 0..2, and the server window is inclusive of after_index, + // so resuming asks for the first index we do NOT have: 3. + await waitFor(() => expect(paths.filter((p) => p.includes('after_index=3'))).toHaveLength(1)) await waitFor(() => expect(result.current.data!.messages.length).toBe(4)) // Only the delta fired, never a tail (-1) fetch. expect(paths.every((p) => p.includes('after_index'))).toBe(true) @@ -805,10 +835,12 @@ describe('useConversation — consolidated delta trigger', () => { it('drains a >80-message backlog across sequential after_index pages, guard stamped once', async () => { setActiveServers(['srv_drain']) const paths: string[] = [] - // Warm cache ends at index 2 (cursor 2). Backlog: 3 pages. + // Warm cache holds 0..2, so the first hop asks for index 3 — the server + // window is [after_index, after_index + limit), inclusive of the cursor. + // Backlog: 3 pages. handlers.srv_drain = (path) => { paths.push(path) - if (path.includes('after_index=2')) { + if (path.includes('after_index=3')) { // 80 new (3..82), more newer. return Promise.resolve(rawAnchoredPage('c_dr', 3, 80, 243, { has_more_newer: true, next_after_index: 83 })) } @@ -828,7 +860,7 @@ describe('useConversation — consolidated delta trigger', () => { // Exactly three sequential after_index GETs. const afterPaths = paths.filter((p) => p.includes('after_index')) expect(afterPaths).toHaveLength(3) - expect(afterPaths[0]).toContain('after_index=2') + expect(afterPaths[0]).toContain('after_index=3') expect(afterPaths[1]).toContain('after_index=83') expect(afterPaths[2]).toContain('after_index=163') // Full range 0..242, no gap. @@ -993,10 +1025,11 @@ describe('useConversation — drain etag (item 3)', () => { it('(e) strips the mismatched hop and stops — no resetQueries, hop-1 kept, resumable', async () => { setActiveServers(['srv_etag']) const paths: string[] = [] - // Warm cache ends at index 2 (cursor 2). Two-hop backlog, but hop 2's etag differs. + // Warm cache holds 0..2, so hop 1 asks for index 3 (the window is inclusive + // of after_index). Two-hop backlog, but hop 2's etag differs. handlers.srv_etag = (path) => { paths.push(path) - if (path.includes('after_index=2')) { + if (path.includes('after_index=3')) { // hop 1: 80 new (3..82), more newer, etag "A". return Promise.resolve(rawAnchoredPage('c_et', 3, 80, 243, { has_more_newer: true, next_after_index: 83, etag: '"A"' })) } diff --git a/components/conversation/ConversationHistoryList.tsx b/components/conversation/ConversationHistoryList.tsx index 8e2cde3e..0b4ad14e 100644 --- a/components/conversation/ConversationHistoryList.tsx +++ b/components/conversation/ConversationHistoryList.tsx @@ -107,27 +107,34 @@ export const ConversationHistoryList = forwardRef, Convers const animateIdsRef = useRef>(new Set()) const seededRef = useRef(false) const [animateEpoch, setAnimateEpoch] = useState(0) + // Keyed on the messages array identity so the O(messages) walk runs only + // when data actually changes, not on every unrelated re-render (FAB state, + // theme). Ids can only be added on a data change, so this is behavior- + // preserving; the epoch bump stays a render-phase update, which is how the + // first render of a new tail row already carries animateIn=true. /* eslint-disable react-hooks/refs -- render-time seen/animate id cache; see note above */ - if (!seededRef.current) { - seededRef.current = true - animateIdsRef.current = new Set() // seed nothing → history renders silently - messages.forEach((m) => animateIdsRef.current.add(`seen:${m.id}`)) - } else { - const set = animateIdsRef.current - const tailStart = Math.max(0, messages.length - ANIMATE_TAIL_WINDOW) - let grew = false - messages.forEach((m, i) => { - const seenKey = `seen:${m.id}` - if (!set.has(seenKey)) { - set.add(seenKey) - if (i >= tailStart) { - set.add(m.id) - grew = true + useMemo(() => { + if (!seededRef.current) { + seededRef.current = true + animateIdsRef.current = new Set() // seed nothing → history renders silently + messages.forEach((m) => animateIdsRef.current.add(`seen:${m.id}`)) + } else { + const set = animateIdsRef.current + const tailStart = Math.max(0, messages.length - ANIMATE_TAIL_WINDOW) + let grew = false + messages.forEach((m, i) => { + const seenKey = `seen:${m.id}` + if (!set.has(seenKey)) { + set.add(seenKey) + if (i >= tailStart) { + set.add(m.id) + grew = true + } } - } - }) - if (grew) setAnimateEpoch((e) => e + 1) - } + }) + if (grew) setAnimateEpoch((e) => e + 1) + } + }, [messages]) const animateIds = animateIdsRef.current /* eslint-enable react-hooks/refs */ @@ -146,17 +153,26 @@ export const ConversationHistoryList = forwardRef, Convers [lastMessageId, highlight, highlightIndex, onMatchLayout, animateEpoch], ) - // Distinguish row shapes so FlashList only recycles cells of the same kind; - // without this a recycled tool-card cell can bleed under a text row. + // Item type drives two FlashList v2 mechanisms: the recycling pool AND the + // per-type running-average height used to place rows that haven't been + // measured yet. Real conversations span ~46pt (collapsed Reasoning header) + // to ~3,100pt (markdown-table answers), so lumping every thinking/tool/diff + // row into one 'tool' pool poisons that average — measured as ±10-20k pt + // content-size swings that shove the mVCP anchor around while scrolling up + // (the blank-gap / viewport-teleport bug). Split by the row's dominant + // shape so each pool's average tracks rows that actually look alike. const getItemType = useCallback((item: Message) => { - const hasToolOrDiff = item.content.some( - (b) => - b.type === 'thinking' || - b.type === 'tool_use' || - b.type === 'tool_result' || - b.type === 'diff', - ) - if (hasToolOrDiff) return 'tool' + let hasThinking = false + let hasTool = false + let hasDiff = false + for (const b of item.content) { + if (b.type === 'thinking') hasThinking = true + else if (b.type === 'tool_use' || b.type === 'tool_result') hasTool = true + else if (b.type === 'diff') hasDiff = true + } + if (hasDiff) return 'diff' + if (hasTool) return 'tool' + if (hasThinking) return 'thinking' return item.role === 'user' ? 'user' : 'assistant' }, []) @@ -170,6 +186,30 @@ export const ConversationHistoryList = forwardRef, Convers [disableAutoAnchor], ) + // FlashList re-lays-out the header/footer whenever the element identity + // changes, and an inline conditional recreates it on every parent render — + // during an onStartReached page fetch that churn feeds extra mVCP anchor + // corrections (upstream guidance: keep these stable, Shopify/flash-list#1844). + // Memoized so identity only changes when the fetching state itself flips. + const listHeader = useMemo( + () => + isFetchingOlder ? ( + + + + ) : null, + [isFetchingOlder, styles, theme], + ) + const listFooter = useMemo( + () => + isFetchingNewer ? ( + + + + ) : null, + [isFetchingNewer, styles, theme], + ) + // Only drives the two scroll FABs — no pagination or scroll logic here. const handleScroll = useCallback((e: NativeSyntheticEvent) => { const { contentOffset, contentSize, layoutMeasurement } = e.nativeEvent @@ -194,6 +234,16 @@ export const ConversationHistoryList = forwardRef, Convers keyExtractor={(m) => m.id} renderItem={renderItem} getItemType={getItemType} + // drawDistance is PIXELS of pre-rendered runway, not rows, and the + // iOS default is 250 — split 70/30 toward the scroll direction, so + // scrolling up pre-renders only ~350px above the viewport and evicts + // rows ~150px below it. A single table answer here measures ~3,100px + // (≈5 viewports), so the default buffer is outrun by any real flick + // and just-passed rows unmount into visible blanks. 2000px keeps the + // engaged window ahead of momentum scrolling and clears the known-bad + // "item taller than 2×drawDistance" mVCP-correction regime + // (Shopify/flash-list#2136) for rows up to 4,000px. + drawDistance={2000} contentContainerStyle={contentContainerStyle} maintainVisibleContentPosition={maintainVisibleContentPosition} onLoad={onReady} @@ -203,20 +253,8 @@ export const ConversationHistoryList = forwardRef, Convers onStartReachedThreshold={0.3} onEndReached={onEndReached} onEndReachedThreshold={0.3} - ListHeaderComponent={ - isFetchingOlder ? ( - - - - ) : null - } - ListFooterComponent={ - isFetchingNewer ? ( - - - - ) : null - } + ListHeaderComponent={listHeader} + ListFooterComponent={listFooter} /> {showScrollTop ? ( \n}\nexport function SkillRow1({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow2({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow3({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow4({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow5({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow6({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow7({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow8({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow9({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow10({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow11({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow12({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow13({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow14({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow15({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow16({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow17({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow18({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow19({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow20({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow21({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow22({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow23({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow24({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow25({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow26({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow27({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow28({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow29({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow30({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow31({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow32({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow33({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow34({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow35({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow36({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow37({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow38({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow39({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow40({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow41({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow42({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow43({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow44({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow45({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow46({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow47({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow48({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow49({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow50({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow51({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow52({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow53({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow54({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow55({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow56({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow57({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow58({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\nexport function SkillRow59({ keyword, seniority }: { keyword: string; seniority: string }) {\n return \n}\n```" + }, + { + "message_index": 14, + "role": "assistant", + "timestamp": "2026-07-31T18:58:00.000Z", + "text": "Ran the export check:", + "content": [ + { + "type": "tool_use", + "id": "tu-1", + "name": "Bash", + "input": { + "command": "npm run export-pdf" + } + }, + { + "type": "tool_result", + "tool_use_id": "tu-1", + "content": "Exported Ronen_Mars_CV_frontend.pdf (2 pages, 184 KB)" + } + ] + }, + { + "message_index": 15, + "role": "assistant", + "timestamp": "2026-07-31T18:58:00.000Z", + "text": "TABLE-TWO-MARKER — here is the full keyword table with the new **Relevance** column (`core` / `strong` / `supporting` / `peripheral`):\n\n| Keyword | Category | Seniority | Fit | Relevance |\n|---|---|---|---|---|\n| React | Framework | senior | frontend | core |\n| TypeScript | Language | senior | backend | strong |\n| JavaScript | Architecture | senior | both | supporting |\n| Design Systems | Testing | senior | frontend | peripheral |\n| Component Libraries | DevOps | senior | backend | core |\n| Redux | Cloud | senior | both | strong |\n| Zustand | API | senior | frontend | supporting |\n| Node.js | Real-time | senior | backend | peripheral |\n| GraphQL | Framework | senior | both | core |\n| WebSockets | Language | senior | frontend | strong |\n| AWS | Architecture | senior | backend | supporting |\n| Docker | Testing | senior | both | peripheral |\n| GitHub Actions | DevOps | senior | frontend | core |\n| Jest | Cloud | senior | backend | strong |\n| Cypress | API | senior | both | supporting |\n| React Testing Library | Real-time | senior | frontend | peripheral |\n| Microservices | Framework | senior | backend | core |\n| Monorepo | Language | senior | both | strong |\n| Accessibility | Architecture | senior | frontend | supporting |\n| Vue.js | Testing | senior | backend | peripheral |\n| AngularJS | DevOps | senior | both | core |\n| Angular | Cloud | senior | frontend | strong |\n| PostgreSQL | API | senior | backend | supporting |\n| MongoDB | Real-time | senior | both | peripheral |\n| Redis | Framework | senior | frontend | core |\n| Express | Language | senior | backend | strong |\n| NestJS | Architecture | senior | both | supporting |\n| Next.js | Testing | senior | frontend | peripheral |\n| Tailwind | DevOps | senior | backend | core |\n| Storybook | Cloud | senior | both | strong |\n| Webpack | API | senior | frontend | supporting |\n| Vite | Real-time | senior | backend | peripheral |\n| Babel | Framework | senior | both | core |\n| ESLint | Language | senior | frontend | strong |\n| Prettier | Architecture | senior | backend | supporting |\n| Figma | Testing | senior | both | peripheral |\n| Claude Code | DevOps | senior | frontend | core |\n| Playwright | Cloud | senior | backend | strong |\n| Maestro | API | senior | both | supporting |\n| CI/CD | Real-time | senior | frontend | peripheral |\n| REST APIs | Framework | senior | backend | core |\n| OAuth | Language | senior | both | strong |\n| JWT | Architecture | senior | frontend | supporting |\n| WebRTC | Testing | senior | backend | peripheral |\n| Service Workers | DevOps | senior | both | core |\n| PWA | Cloud | senior | frontend | strong |\n| SSR | API | senior | backend | supporting |\n| i18n | Real-time | senior | both | peripheral |\n| a11y | Framework | senior | frontend | core |\n| Performance | Language | senior | backend | strong |\n| Profiling | Architecture | senior | both | supporting |\n| Sentry | Testing | senior | frontend | peripheral |\n| Analytics | DevOps | senior | backend | core |\n| Firebase | Cloud | senior | both | strong |\n| Supabase | API | senior | frontend | supporting |\n| Stripe | Real-time | senior | backend | peripheral |\n| Twilio | Framework | senior | both | core |\n| Kubernetes | Language | senior | frontend | strong |\n| Terraform | Architecture | senior | backend | supporting |\n| Bash | Testing | senior | both | peripheral |\n| Python | DevOps | senior | frontend | core |\n| Go | Cloud | senior | backend | strong |\n| Rust | API | senior | both | supporting |\n| Java | Real-time | senior | frontend | peripheral |\n| Kotlin | Framework | senior | backend | core |" + }, + { + "message_index": 16, + "role": "user", + "timestamp": "2026-07-31T18:58:00.000Z", + "text": "QUESTION-MARKER The plan needs a review pass over each section so the skill stays focused on measurable improvements. The plan needs a review pass over each section so the skill stays focused on measurable improvements." + }, + { + "message_index": 17, + "role": "assistant", + "timestamp": "2026-07-31T18:58:00.000Z", + "text": "", + "content": [ + { + "type": "thinking", + "thinking": "", + "signature": "sig-redacted" + } + ] + }, + { + "message_index": 18, + "role": "assistant", + "timestamp": "2026-07-31T18:58:00.000Z", + "text": "WRAP-UP-MARKER The plan needs a review pass over each section so the skill stays focused on measurable improvements. The plan needs a review pass over each section so the skill stays focused on measurable improvements. The plan needs a review pass over each section so the skill stays focused on measurable improvements. The plan needs a review pass over each section so the skill stays focused on measurable improvements. The plan needs a review pass over each section so the skill stays focused on measurable improvements." + }, + { + "message_index": 19, + "role": "user", + "timestamp": "2026-07-31T18:58:00.000Z", + "text": "done" + }, + { + "message_index": 20, + "role": "assistant", + "timestamp": "2026-07-31T18:58:00.000Z", + "text": "SPEC-MARKER The plan needs a review pass over each section so the skill stays focused on measurable improvements. The plan needs a review pass over each section so the skill stays focused on measurable improvements. The plan needs a review pass over each section so the skill stays focused on measurable improvements.", + "content": [ + { + "type": "thinking", + "thinking": "", + "signature": "sig-redacted" + } + ] + }, + { + "message_index": 21, + "role": "assistant", + "timestamp": "2026-07-31T18:58:00.000Z", + "text": "FINAL-SCROLL-GAPS-MESSAGE — that one's ambiguous too. Quick status: we're paused so you can edit the keywords file. Two ways forward: say done and I write the spec doc, or tell me a change and I adjust the design first." + } + ], + "message_pagination": { + "total": 22, + "before_index": 22, + "from_index": 0, + "has_more_older": false, + "next_before_index": null + } +} diff --git a/e2e/mock-server.js b/e2e/mock-server.js index 4df3118e..ba477169 100755 --- a/e2e/mock-server.js +++ b/e2e/mock-server.js @@ -151,6 +151,38 @@ async function handleRequest(req, res) { if (conversationMatch[1] === 'conv-codex-1') { return json(res, 200, readFixture('codex-conversation-detail.json')) } + // Scroll-gaps regression fixture (07_conversation_scroll_gaps.yaml): row + // heights mirroring the real conversation that reproduced the blank-gap + // bug — two ~3,000pt markdown-table answers and a tall code fence between + // 50-350pt rows. The delta poll's after_index window is served with the + // real streamer's INCLUSIVE [after_index, after_index + limit) semantics: + // a client that resumes from its max held index re-downloads the tail row + // every poll (the duplicate-append bug this fixture regression-tests), + // while a client that resumes from maxHeld + 1 gets an empty page. + if (conversationMatch[1] === 'conv-scroll-gaps') { + // readFixture returns the raw string; this route needs the parsed object. + const fixture = JSON.parse(readFixture('conv-scroll-gaps.json')) + if (url.searchParams.has('after_index')) { + const total = fixture.message_pagination.total + const limit = Math.max(1, parseInt(url.searchParams.get('msg_limit') ?? '80', 10) || 80) + const from = Math.min(Math.max(parseInt(url.searchParams.get('after_index'), 10) || 0, 0), total) + const to = Math.min(total, from + limit) + return json(res, 200, { + meta: fixture.meta, + messages: fixture.messages.slice(from, to), + message_pagination: { + total, + before_index: to, + from_index: from, + has_more_older: from > 0, + next_before_index: from > 0 ? from : null, + has_more_newer: to < total, + next_after_index: to < total ? to : null, + }, + }) + } + return json(res, 200, fixture) + } if (conversationMatch[1] === 'conv-search-anchor') { if (url.searchParams.has('anchor_index')) { return json(res, 200, readFixture('conv-search-anchor.json')) diff --git a/hooks/useConversations.ts b/hooks/useConversations.ts index dedec224..92b22c6d 100644 --- a/hooks/useConversations.ts +++ b/hooks/useConversations.ts @@ -353,14 +353,25 @@ function mergeConversationPages(pages: RawConversationDetail[]): ConversationDet } const first = pages[0] const convId = first.meta.id + // Server pages can overlap (the after_index window is inclusive of its + // cursor, and anchored windows widen backward near the tail), so the same + // message_index may arrive in more than one cached page. The adapted id is + // `${convId}-${index}`, and FlashList's keyExtractor + its + // maintainVisibleContentPosition anchor both require unique keys — duplicate + // ids reserve phantom layout space and misplace the scroll anchor. Dedup at + // this single choke point; first (oldest-page) occurrence wins. + const seenIndexes = new Set() const messages: Message[] = [...pages] .reverse() .flatMap((page) => (page.messages ?? []) .filter((m) => !(m.role === 'user' && typeof m.text === 'string' && isCodexInjectedContext(m.text))) - .map((m, i) => - adaptRawMessage(m, convId, m.message_index ?? (page.message_pagination?.from_index ?? 0) + i), - ), + .flatMap((m, i) => { + const idx = m.message_index ?? (page.message_pagination?.from_index ?? 0) + i + if (seenIndexes.has(idx)) return [] + seenIndexes.add(idx) + return [adaptRawMessage(m, convId, idx)] + }), ) return { @@ -417,7 +428,14 @@ export function useConversation( // If-None-Match would be misleading dead code. if (typeof pageParam === 'object') { const isResume = 'resume' in pageParam - const cursor = isResume ? pageParam.resume : pageParam.after + // The server window is [after_index, after_index + limit) — INCLUSIVE + // of the cursor. { after } carries the server's own next_after_index + // (already the first index we don't have), but { resume } is the + // client-derived max index we DO have — so it must be bumped by one. + // Without the +1 every delta drain re-fetches the tail message, and + // each re-fetch appends a duplicate-id row (see mergeConversationPages) + // that grows the list by one phantom message per poll tick. + const cursor = isResume ? pageParam.resume + 1 : pageParam.after params.set('msg_limit', String(isResume ? CONVERSATION_MESSAGE_LIMIT : CONVERSATION_ANCHORED_LIMIT)) params.set('after_index', String(cursor)) return api.get( diff --git a/package.json b/package.json index a34e0324..773b599c 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "test:integration": "jest --ci --testPathPattern='__tests__/integration'", "test:e2e": "jest --ci --testPathPattern='__tests__/e2e'", "test:i18n": "jest --ci --testPathPattern='__tests__/i18n'", - "test:e2e:mock": "node e2e/check-sim.js && node e2e/ensure-release-build.js && (MOCK_PORTS=7071,7072 node e2e/mock-server.js & MOCK_PID=$!; sleep 1; maestro test --debug-output e2e/_artifacts/debug e2e/launch.yaml e2e/browse.yaml e2e/session_lifecycle.yaml e2e/server_drag_reorder.yaml e2e/bug6_bottom_bar_inset.yaml e2e/pty_turn_divider.yaml e2e/feat1_tree_drill_new_session.yaml e2e/feat2_export_in_info_shelf.yaml e2e/codex_parity.yaml e2e/voice_dictation.yaml e2e/settings_qr_scanner.yaml e2e/feedback_flow.yaml e2e/05_chat_flow.yaml e2e/06_search_anchor.yaml; STATUS=$?; kill $MOCK_PID 2>/dev/null || true; exit $STATUS)", + "test:e2e:mock": "node e2e/check-sim.js && node e2e/ensure-release-build.js && (MOCK_PORTS=7071,7072 node e2e/mock-server.js & MOCK_PID=$!; sleep 1; maestro test --debug-output e2e/_artifacts/debug e2e/launch.yaml e2e/browse.yaml e2e/session_lifecycle.yaml e2e/server_drag_reorder.yaml e2e/bug6_bottom_bar_inset.yaml e2e/pty_turn_divider.yaml e2e/feat1_tree_drill_new_session.yaml e2e/feat2_export_in_info_shelf.yaml e2e/codex_parity.yaml e2e/voice_dictation.yaml e2e/settings_qr_scanner.yaml e2e/feedback_flow.yaml e2e/05_chat_flow.yaml e2e/06_search_anchor.yaml e2e/07_conversation_scroll_gaps.yaml; STATUS=$?; kill $MOCK_PID 2>/dev/null || true; exit $STATUS)", "test:e2e:parallel-fetch": "node e2e/check-sim.js && (MOCK_PORT=7073 MOCK_TOTAL_CONVERSATIONS=30 MOCK_TOTAL_SESSIONS=25 MOCK_PAGE_DELAY_MS=3000 node e2e/pagination-mock-server.js & MOCK_PID=$!; sleep 1; maestro test --debug-output e2e/_artifacts/debug e2e/parallel-fetch-progress.yaml; STATUS=$?; kill $MOCK_PID 2>/dev/null || true; exit $STATUS)", "test:e2e:parallel-fetch:non-merged": "node e2e/check-sim.js && (MOCK_PORT=7073 MOCK_TOTAL_CONVERSATIONS=30 MOCK_TOTAL_SESSIONS=25 MOCK_PAGE_DELAY_MS=3000 node e2e/pagination-mock-server.js & MOCK_PID=$!; sleep 1; maestro test --debug-output e2e/_artifacts/debug e2e/non-merged-conv-loading.yaml; STATUS=$?; kill $MOCK_PID 2>/dev/null || true; exit $STATUS)", "test:e2e:ts1": "node e2e/check-sim.js && node e2e/ensure-release-build.js && (MOCK_PORTS=7071,7072 node e2e/mock-server.js & MOCK_PID=$!; sleep 1; maestro test --debug-output e2e/_artifacts/debug e2e/ts1_onboarding_pairing.yaml; STATUS=$?; kill $MOCK_PID 2>/dev/null || true; exit $STATUS)",