fix(app): load older history when chat is shorter than viewport#1294
fix(app): load older history when chat is shorter than viewport#1294kongjiadongyuan wants to merge 2 commits into
Conversation
|
| Filename | Overview |
|---|---|
| packages/app/src/agent-stream/strategy.ts | Adds StreamNearHistoryStartInput type alias and three new exported functions for near history start detection; logic is straightforward and well-tested. |
| packages/app/src/agent-stream/strategy-web.tsx | Extracts maybeLoadOlderHistory into a useStableEvent and calls it from the ResizeObserver, the agentId-reset rAF, and a new effect on hasOlderHistory/isLoadingOlderHistory; offset calculation correctly handles short-content and follow-output modes. |
| packages/app/src/agent-stream/strategy-native.tsx | Mirrors the web-side changes: maybeLoadOlderHistory replaces inline distance math in the scroll handler and is also called from handleListLayout, handleContentSizeChange, the agentId rAF, and the new hasOlderHistory/isLoadingOlderHistory effect. |
| packages/app/src/agent-stream/render-strategy.test.ts | Adds three pure-strategy tests for isNearHistoryStartForStreamRenderStrategy covering short-content true path for both forward and inverted strategies, and the negative (long content) path. |
| packages/app/src/agent-stream/strategy-web.test.tsx | Adds a JSDOM/createRoot component-mounting test verifying the ResizeObserver wiring; the test pattern was already flagged in a previous review thread for this file. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Component Mount / agentId change] --> B[requestAnimationFrame]
B --> C[historyStartReadyRef = true]
C --> D[maybeLoadOlderHistory]
E[ResizeObserver fires] --> F[updateScrollMetrics]
F --> D
G[handleScroll / handleListLayout / handleContentSizeChange] --> D
H[hasOlderHistory or isLoadingOlderHistory changes] --> D
D --> I{Guards pass?}
I -- No --> J[return early]
I -- Yes --> K{followOutput?}
K -- true --> L[offsetY = max 0 scrollHeight minus clientHeight]
K -- false --> M[offsetY = scrollTop]
L --> N{isNearHistoryStart?}
M --> N
N -- No --> O[return]
N -- Yes --> P[onNearHistoryStart called]
P --> Q[isLoadingOlderHistory blocks further calls]
Reviews (2): Last reviewed commit: "chore(app): reuse stream scroll input ty..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| expect(onNearHistoryStart).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it("fires near-history-start after measurement when content is too short to scroll", async () => { | ||
| const strategy = createWebStreamStrategy({ isMobileBreakpoint: true }); | ||
| const viewportRef = React.createRef<StreamViewportHandle>(); | ||
| const onNearHistoryStart = vi.fn(); | ||
| container = document.createElement("div"); | ||
| document.body.appendChild(container); | ||
| root = createRoot(container); | ||
|
|
||
| act(() => { | ||
| root?.render( | ||
| <> | ||
| {strategy.render({ | ||
| agentId: "agent", | ||
| segments: { | ||
| historyVirtualized: [], | ||
| historyMounted: [userMessage(1), userMessage(2)], | ||
| liveHead: [], | ||
| }, | ||
| boundary: { | ||
| hasVirtualizedHistory: false, | ||
| hasMountedHistory: true, | ||
| hasLiveHead: false, | ||
| }, | ||
| renderers: createRenderers(vi.fn()), | ||
| listEmptyComponent: null, | ||
| viewportRef, | ||
| routeBottomAnchorRequest: null, | ||
| isAuthoritativeHistoryReady: true, | ||
| onNearBottomChange: vi.fn(), | ||
| onNearHistoryStart, | ||
| isLoadingOlderHistory: false, | ||
| hasOlderHistory: true, | ||
| scrollEnabled: true, | ||
| listStyle: null, | ||
| baseListContentContainerStyle: null, | ||
| forwardListContentContainerStyle: null, | ||
| })} | ||
| </>, | ||
| ); | ||
| }); | ||
|
|
||
| const scrollContainer = container.querySelector('[data-testid="agent-chat-scroll"]'); | ||
| expect(scrollContainer).toBeInstanceOf(HTMLElement); | ||
|
|
||
| await act(async () => { | ||
| await new Promise((resolve) => requestAnimationFrame(resolve)); | ||
| }); | ||
| expect(onNearHistoryStart).toHaveBeenCalledTimes(0); | ||
|
|
||
| Object.defineProperty(scrollContainer, "clientHeight", { configurable: true, value: 400 }); | ||
| Object.defineProperty(scrollContainer, "scrollHeight", { configurable: true, value: 200 }); | ||
| Object.defineProperty(scrollContainer, "scrollTop", { configurable: true, value: 0 }); | ||
| act(() => { | ||
| for (const callback of resizeObserverCallbacks) { | ||
| callback([], {} as ResizeObserver); | ||
| } | ||
| }); | ||
|
|
||
| expect(onNearHistoryStart).toHaveBeenCalledTimes(1); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
maybeLoadOlderHistory
The project style guide explicitly flags JSDOM component-mounting tests as a banned pattern ("React components are not the unit — extract the logic and unit-test the extraction; cover component behavior through app E2E with a real harness"). The onNearHistoryStart firing condition under short content is pure scroll-math already covered by the new pure-strategy tests in render-strategy.test.ts. The part that isn't covered — that maybeLoadOlderHistory is wired to the ResizeObserver and fires after the historyStartReadyRef rAF gate — is precisely the kind of integration that belongs in an E2E test, not a JSDOM simulation. Consider whether this test adds confidence beyond render-strategy.test.ts or whether the ResizeObserver wiring should be verified in E2E.
Rule Used: # Code Review Pattern Reference: Slop, Tests, Feat... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
Why
When the initial chat content is shorter than the viewport, there is no scrollable top edge. The existing lazy history loader only ran from scroll handlers, so users could get stuck with a short partial timeline even though
hasOlderHistorywas true.Tests
npm run test --workspace=@getpaseo/app -- src/agent-stream/render-strategy.test.ts src/agent-stream/strategy-web.test.tsx --bail=1npm run lint -- packages/app/src/agent-stream/strategy.ts packages/app/src/agent-stream/strategy-native.tsx packages/app/src/agent-stream/strategy-web.tsx packages/app/src/agent-stream/render-strategy.test.ts packages/app/src/agent-stream/strategy-web.test.tsxnpm run typecheck --workspace=@getpaseo/app