diff --git a/desktop/src/features/messages/lib/timelineSnapshot.test.mjs b/desktop/src/features/messages/lib/timelineSnapshot.test.mjs index 54217ec28..7738d27f0 100644 --- a/desktop/src/features/messages/lib/timelineSnapshot.test.mjs +++ b/desktop/src/features/messages/lib/timelineSnapshot.test.mjs @@ -2,6 +2,7 @@ import assert from "node:assert/strict"; import test from "node:test"; import { + classifyTimelineMessageDelta, BOTTOM_THRESHOLD_PX, buildDayGroupBoundaries, isDeferredTimelineSnapshotStale, @@ -339,6 +340,29 @@ test("no-tearing: stale snapshot keeps all three decisions internally consistent assert.equal(latestKey, "b"); }); +test("classifyTimelineMessageDelta: detects older-history prepends", () => { + const previous = [message({ id: "a" }), message({ id: "b" })]; + const current = [message({ id: "older" }), ...previous]; + + assert.equal(classifyTimelineMessageDelta({ current, previous }), "prepend"); +}); + +test("classifyTimelineMessageDelta: detects latest-message appends", () => { + const previous = [message({ id: "a" }), message({ id: "b" })]; + const current = [...previous, message({ id: "c" })]; + + assert.equal(classifyTimelineMessageDelta({ current, previous }), "append"); +}); + +test("classifyTimelineMessageDelta: unchanged snapshots do not count as arrivals", () => { + const previous = [message({ id: "a" }), message({ id: "b" })]; + + assert.equal( + classifyTimelineMessageDelta({ current: previous, previous }), + "none", + ); +}); + // --- deferred reply-list render state (thread side pane) -------------------- // // When MessageThreadPanel gates its reply render behind useDeferredValue, the diff --git a/desktop/src/features/messages/lib/timelineSnapshot.ts b/desktop/src/features/messages/lib/timelineSnapshot.ts index 50a3a29c4..2c492fec5 100644 --- a/desktop/src/features/messages/lib/timelineSnapshot.ts +++ b/desktop/src/features/messages/lib/timelineSnapshot.ts @@ -203,6 +203,46 @@ export function selectTimelineBodySurface({ return renderState; } +export type TimelineMessageDelta = "prepend" | "append" | "replace" | "none"; + +export function classifyTimelineMessageDelta({ + current, + previous, +}: { + current: readonly Pick[]; + previous: readonly Pick[]; +}): TimelineMessageDelta { + if (previous.length === 0 || current.length === 0) { + return previous.length === current.length ? "none" : "replace"; + } + + const previousFirstId = previous[0]?.id; + const previousLastId = previous[previous.length - 1]?.id; + const currentFirstId = current[0]?.id; + const currentLastId = current[current.length - 1]?.id; + + if (previousFirstId === currentFirstId && previousLastId === currentLastId) { + if (previous.length === current.length) { + return "none"; + } + return current.length > previous.length ? "append" : "replace"; + } + + if ( + previousFirstId !== undefined && + currentFirstId !== previousFirstId && + current.some((message) => message.id === previousFirstId) + ) { + return "prepend"; + } + + if (previousLastId !== undefined && currentLastId !== previousLastId) { + return "append"; + } + + return "replace"; +} + export type TimelineSnapshotIdentity = { channelId: string | null; }; diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index 6a18a9684..cd4db35bf 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -1,5 +1,6 @@ import * as React from "react"; +import { classifyTimelineMessageDelta } from "@/features/messages/lib/timelineSnapshot"; import type { TimelineMessage } from "@/features/messages/types"; /** @@ -165,6 +166,7 @@ export function useAnchoredScroll({ const prevLastMessageIdRef = React.useRef(undefined); const prevFirstMessageIdRef = React.useRef(undefined); const prevMessageCountRef = React.useRef(0); + const prevMessagesRef = React.useRef([]); const handledTargetIdRef = React.useRef(null); const highlightTimeoutRef = React.useRef(null); // Tracks a pending rAF queued by pinToBottomOnMount so it can be cancelled @@ -194,6 +196,7 @@ export function useAnchoredScroll({ prevLastMessageIdRef.current = undefined; prevFirstMessageIdRef.current = undefined; prevMessageCountRef.current = 0; + prevMessagesRef.current = []; handledTargetIdRef.current = null; forceBottomOnNextAppendRef.current = false; settlingRef.current = false; @@ -363,6 +366,7 @@ export function useAnchoredScroll({ prevLastMessageIdRef.current = messages[messages.length - 1]?.id; prevFirstMessageIdRef.current = messages[0]?.id; prevMessageCountRef.current = messages.length; + prevMessagesRef.current = messages; return; } @@ -370,7 +374,6 @@ export function useAnchoredScroll({ const lastMessage = messages[messages.length - 1]; const firstMessage = messages[0]; const prevLastId = prevLastMessageIdRef.current; - const prevFirstId = prevFirstMessageIdRef.current; const prevCount = prevMessageCountRef.current; const newLatestArrived = lastMessage !== undefined && lastMessage.id !== prevLastId; @@ -378,12 +381,13 @@ export function useAnchoredScroll({ // signal. The relay can deliver a message that sorts ahead of an existing // same-second row, so the list grows without the *last* id changing — // `newLatestArrived` misses that case and the unread counter never bumps. + const prevMessages = prevMessagesRef.current; const messagesArrived = messages.length - prevCount; - const frontChanged = - firstMessage !== undefined && - prevFirstId !== undefined && - firstMessage.id !== prevFirstId; - const isPrepend = frontChanged && !newLatestArrived; + const isPrepend = + classifyTimelineMessageDelta({ + current: messages, + previous: prevMessages, + }) === "prepend"; // One-shot: an outbound send armed `scrollToBottomOnNextUpdate`. When the // resulting append lands, snap to bottom regardless of the current anchor, @@ -399,6 +403,7 @@ export function useAnchoredScroll({ prevLastMessageIdRef.current = lastMessage?.id; prevFirstMessageIdRef.current = firstMessage?.id; prevMessageCountRef.current = messages.length; + prevMessagesRef.current = messages; return; } @@ -436,6 +441,7 @@ export function useAnchoredScroll({ prevLastMessageIdRef.current = lastMessage?.id; prevFirstMessageIdRef.current = firstMessage?.id; prevMessageCountRef.current = messages.length; + prevMessagesRef.current = messages; }, [ isLoading, messages, diff --git a/desktop/tests/e2e/persona-env-vars.spec.ts b/desktop/tests/e2e/persona-env-vars.spec.ts index a6f1f4f41..b0888b664 100644 --- a/desktop/tests/e2e/persona-env-vars.spec.ts +++ b/desktop/tests/e2e/persona-env-vars.spec.ts @@ -58,6 +58,23 @@ async function invokeTauri( ); } +type DropdownName = "Anthropic" | "Default" | "OpenAI" | "Custom"; + +async function selectDropdownOption( + trigger: import("@playwright/test").Locator, + name: DropdownName, +) { + await expect(trigger).toBeEnabled(); + await trigger.click(); + const item = trigger + .page() + .getByRole("menuitemradio", { name, exact: true }) + .last(); + await expect(item).toBeVisible(); + await item.click(); + await expect(item).toBeHidden(); +} + async function openModelMenu( page: import("@playwright/test").Page, model: import("@playwright/test").Locator, @@ -300,10 +317,7 @@ test("persona model options follow the selected LLM provider", async ({ await expect(model).toContainText("Default model"); // Switch to OpenAI — the API-key field appears and is labelled correctly. - await llmProvider.click(); - await page - .getByRole("menuitemradio", { name: "OpenAI", exact: true }) - .click(); + await selectDropdownOption(llmProvider, "OpenAI"); const providerApiKey = page.getByTestId("persona-provider-api-key"); await expect(page.getByText("OpenAI API key")).toBeVisible(); await expect(providerApiKey).toBeVisible(); @@ -317,10 +331,7 @@ test("persona model options follow the selected LLM provider", async ({ .click(); // Switch to Anthropic — API-key field label changes and value clears. - await llmProvider.click(); - await page - .getByRole("menuitemradio", { name: "Anthropic", exact: true }) - .click(); + await selectDropdownOption(llmProvider, "Anthropic"); await expect(page.getByText("Anthropic API key")).toBeVisible(); await expect(providerApiKey).toHaveValue(""); await expect(model).toBeVisible(); @@ -330,17 +341,7 @@ test("persona model options follow the selected LLM provider", async ({ await expect(model).toBeVisible(); // Switch to Default (no explicit provider) — model resets to "Default model". - await llmProvider.click(); - // Wait for Radix menu animations to settle before locating the menu item. - // The prior approach held a filtered locator across the open→animate boundary - // and clicked a node that Radix was still re-mounting, producing - // "element is not stable" / "element was detached from the DOM" failures. - // Matching the OpenAI/Anthropic steps above: wait for animations, then - // locate fresh at click-time so no stale reference crosses the re-render. - await waitForAnimations(page); - await page - .getByRole("menuitemradio", { name: "Default", exact: true }) - .click(); + await selectDropdownOption(llmProvider, "Default"); await expect(model).toBeVisible(); await expect(model).toContainText("Default model"); });