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)",