Skip to content

Commit c23f1ac

Browse files
lcompleteclaude
andcommitted
fix(extension,mcp): improve scroll pin behavior and cap MCP search page
- Use <= threshold in isScrollPinnedToBottom and raise threshold to 160px - Switch useScrollPinToBottom to useLayoutEffect and direct scrollTop assignment to avoid rAF/scrollIntoView jitter during streaming - Cap SearchContentTool page to MAX_LUCENE_RESULT_WINDOW/MAX_LIMIT (20) to keep Lucene collection bounded; reflect constraint in schema and execute clamping - Add comprehensive hook integration tests and page-clamping unit test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 166817a commit c23f1ac

6 files changed

Lines changed: 241 additions & 13 deletions

File tree

Lines changed: 175 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,191 @@
1+
/** @jest-environment jsdom */
2+
3+
import React from "react";
4+
import { createRoot, type Root } from "react-dom/client";
5+
import { act } from "react-dom/test-utils";
16
import {
27
isScrollPinnedToBottom,
38
shouldShowScrollToBottomButton,
49
} from "../sidepanel/utils/scrollToBottom";
10+
import { useScrollPinToBottom } from "../sidepanel/hooks/useScrollPinToBottom";
11+
12+
(
13+
globalThis as typeof globalThis & {
14+
IS_REACT_ACT_ENVIRONMENT?: boolean;
15+
}
16+
).IS_REACT_ACT_ENVIRONMENT = true;
17+
18+
type ScrollHookSnapshot = {
19+
handleScroll: () => void;
20+
scrollToBottom: (behavior?: ScrollBehavior) => void;
21+
};
22+
23+
const TEST_THRESHOLD_PX = 160;
24+
25+
let latestHook: ScrollHookSnapshot | null = null;
26+
27+
function ScrollHarness({ messages }: { messages: string[] }) {
28+
const hook = useScrollPinToBottom({
29+
messages,
30+
thresholdPx: TEST_THRESHOLD_PX,
31+
});
32+
latestHook = hook;
33+
34+
return React.createElement(
35+
"div",
36+
{ ref: hook.scrollContainerRef, "data-testid": "scroll-container" },
37+
React.createElement(
38+
"div",
39+
null,
40+
React.createElement("div", { ref: hook.messagesEndRef })
41+
)
42+
);
43+
}
44+
45+
function renderScrollHarness(messages: string[] = ["hello"]) {
46+
const container = document.createElement("div");
47+
document.body.appendChild(container);
48+
const root = createRoot(container);
49+
50+
act(() => {
51+
root.render(React.createElement(ScrollHarness, { messages }));
52+
});
53+
54+
const scrollContainer = container.querySelector(
55+
'[data-testid="scroll-container"]'
56+
);
57+
if (!(scrollContainer instanceof HTMLDivElement)) {
58+
throw new Error("Scroll container not found");
59+
}
60+
61+
return { container, root, scrollContainer };
62+
}
63+
64+
function rerenderScrollHarness(root: Root, messages: string[]) {
65+
act(() => {
66+
root.render(React.createElement(ScrollHarness, { messages }));
67+
});
68+
}
69+
70+
function setScrollMetrics(
71+
element: HTMLDivElement,
72+
metrics: { clientHeight: number; scrollHeight: number; scrollTop: number }
73+
) {
74+
Object.defineProperty(element, "clientHeight", {
75+
configurable: true,
76+
value: metrics.clientHeight,
77+
});
78+
Object.defineProperty(element, "scrollHeight", {
79+
configurable: true,
80+
value: metrics.scrollHeight,
81+
});
82+
Object.defineProperty(element, "scrollTop", {
83+
configurable: true,
84+
value: metrics.scrollTop,
85+
writable: true,
86+
});
87+
}
88+
89+
function unmount(root: Root, container: HTMLDivElement) {
90+
act(() => {
91+
root.unmount();
92+
});
93+
container.remove();
94+
}
95+
96+
beforeEach(() => {
97+
latestHook = null;
98+
});
99+
100+
afterEach(() => {
101+
document.body.innerHTML = "";
102+
});
5103

6104
describe("sidepanel scroll-to-bottom helpers", () => {
7105
it("treats positions within the threshold as pinned to the bottom", () => {
8106
expect(isScrollPinnedToBottom(12, 96)).toBe(true);
9107
expect(isScrollPinnedToBottom(95, 96)).toBe(true);
10-
expect(isScrollPinnedToBottom(96, 96)).toBe(false);
108+
expect(isScrollPinnedToBottom(96, 96)).toBe(true);
109+
expect(isScrollPinnedToBottom(97, 96)).toBe(false);
11110
});
12111

13112
it("shows the down-arrow button only when there are messages and the view is not pinned", () => {
14113
expect(shouldShowScrollToBottomButton(0, false)).toBe(false);
15114
expect(shouldShowScrollToBottomButton(3, true)).toBe(false);
16115
expect(shouldShowScrollToBottomButton(3, false)).toBe(true);
17116
});
18-
});
117+
});
118+
119+
describe("useScrollPinToBottom", () => {
120+
it("keeps output pinned to the bottom while already at the bottom", () => {
121+
const { container, root, scrollContainer } = renderScrollHarness();
122+
setScrollMetrics(scrollContainer, {
123+
clientHeight: 300,
124+
scrollHeight: 720,
125+
scrollTop: 420,
126+
});
127+
128+
Object.defineProperty(scrollContainer, "scrollHeight", {
129+
configurable: true,
130+
value: 900,
131+
});
132+
rerenderScrollHarness(root, ["hello", "streaming"]);
133+
134+
expect(scrollContainer.scrollTop).toBe(600);
135+
unmount(root, container);
136+
});
137+
138+
it("stops auto-scrolling after the user scrolls away", () => {
139+
const { container, root, scrollContainer } = renderScrollHarness();
140+
setScrollMetrics(scrollContainer, {
141+
clientHeight: 300,
142+
scrollHeight: 720,
143+
scrollTop: 120,
144+
});
145+
146+
act(() => {
147+
latestHook?.handleScroll();
148+
});
149+
150+
Object.defineProperty(scrollContainer, "scrollHeight", {
151+
configurable: true,
152+
value: 900,
153+
});
154+
rerenderScrollHarness(root, ["hello", "streaming"]);
155+
156+
expect(scrollContainer.scrollTop).toBe(120);
157+
unmount(root, container);
158+
});
159+
160+
it("resumes auto-scrolling when the user returns near the bottom", () => {
161+
const { container, root, scrollContainer } = renderScrollHarness();
162+
setScrollMetrics(scrollContainer, {
163+
clientHeight: 300,
164+
scrollHeight: 720,
165+
scrollTop: 120,
166+
});
167+
168+
act(() => {
169+
latestHook?.handleScroll();
170+
});
171+
172+
setScrollMetrics(scrollContainer, {
173+
clientHeight: 300,
174+
scrollHeight: 900,
175+
scrollTop: 460,
176+
});
177+
178+
act(() => {
179+
latestHook?.handleScroll();
180+
});
181+
182+
Object.defineProperty(scrollContainer, "scrollHeight", {
183+
configurable: true,
184+
value: 1000,
185+
});
186+
rerenderScrollHarness(root, ["hello", "streaming"]);
187+
188+
expect(scrollContainer.scrollTop).toBe(700);
189+
unmount(root, container);
190+
});
191+
});

app/extension/src/sidepanel/SidepanelApp.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ function getChromeApi(): ChromeApi | undefined {
140140
return (globalThis as typeof globalThis & { chrome?: ChromeApi }).chrome;
141141
}
142142

143-
const SCROLL_PIN_THRESHOLD_PX = 96;
143+
const SCROLL_PIN_THRESHOLD_PX = 160;
144144

145145
export const SidepanelApp: FC = () => {
146146
const { t } = useI18n();

app/extension/src/sidepanel/hooks/useScrollPinToBottom.ts

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import { useCallback, useEffect, useRef, useState, type RefObject } from "react";
1+
import {
2+
useCallback,
3+
useLayoutEffect,
4+
useRef,
5+
useState,
6+
type RefObject,
7+
} from "react";
28

39
import {
410
isScrollPinnedToBottom,
@@ -39,7 +45,24 @@ export function useScrollPinToBottom<T>(
3945
const scrollToBottom = useCallback((behavior: ScrollBehavior = "smooth") => {
4046
pinnedRef.current = true;
4147
setShowScrollToBottom(false);
42-
messagesEndRef.current?.scrollIntoView({ block: "end", behavior });
48+
49+
const scrollContainer = scrollContainerRef.current;
50+
if (!scrollContainer) {
51+
messagesEndRef.current?.scrollIntoView({ block: "end", behavior });
52+
return;
53+
}
54+
55+
const top = Math.max(
56+
scrollContainer.scrollHeight - scrollContainer.clientHeight,
57+
0
58+
);
59+
60+
if (behavior === "auto" || typeof scrollContainer.scrollTo !== "function") {
61+
scrollContainer.scrollTop = top;
62+
return;
63+
}
64+
65+
scrollContainer.scrollTo({ top, behavior });
4366
}, []);
4467

4568
const handleScroll = useCallback(() => {
@@ -58,7 +81,7 @@ export function useScrollPinToBottom<T>(
5881
);
5982
}, [messages.length, thresholdPx]);
6083

61-
useEffect(() => {
84+
useLayoutEffect(() => {
6285
if (messages.length === 0) {
6386
pinnedRef.current = true;
6487
setShowScrollToBottom(false);
@@ -72,8 +95,7 @@ export function useScrollPinToBottom<T>(
7295
return;
7396
}
7497

75-
const frame = window.requestAnimationFrame(() => scrollToBottom("auto"));
76-
return () => window.cancelAnimationFrame(frame);
98+
scrollToBottom("auto");
7799
}, [messages, scrollToBottom]);
78100

79101
return {

app/extension/src/sidepanel/utils/scrollToBottom.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ export function isScrollPinnedToBottom(
22
distanceToBottom: number,
33
thresholdPx: number
44
): boolean {
5-
return distanceToBottom < thresholdPx;
5+
return distanceToBottom <= thresholdPx;
66
}
77

88
export function shouldShowScrollToBottomButton(

app/server/huntly-server/src/main/java/com/huntly/server/mcp/tool/SearchContentTool.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.huntly.interfaces.external.query.SearchQuery;
55
import com.huntly.server.mcp.McpUtils;
66
import com.huntly.server.service.LuceneService;
7+
import com.huntly.server.util.PageSizeUtils;
78
import org.apache.commons.lang3.StringUtils;
89
import org.springframework.stereotype.Component;
910

@@ -19,6 +20,10 @@
1920
@Component
2021
public class SearchContentTool implements McpTool {
2122

23+
private static final int MAX_LIMIT = PageSizeUtils.MAX_PAGE_SIZE;
24+
private static final int MAX_LUCENE_RESULT_WINDOW = 10_000;
25+
private static final int MAX_PAGE = Math.max(1, MAX_LUCENE_RESULT_WINDOW / MAX_LIMIT);
26+
2227
private final LuceneService luceneService;
2328
private final McpUtils mcpUtils;
2429

@@ -84,14 +89,15 @@ public Map<String, Object> getInputSchema() {
8489
properties.put("limit", Map.of(
8590
"type", "integer",
8691
"minimum", 1,
87-
"maximum", 500,
92+
"maximum", MAX_LIMIT,
8893
"description", "Number of results to return, max 500"
8994
));
9095
properties.put("page", Map.of(
9196
"type", "integer",
9297
"minimum", 1,
98+
"maximum", MAX_PAGE,
9399
"default", 1,
94-
"description", "Page number for search results, starting from 1. Use with limit to inspect additional pages."
100+
"description", "Page number for search results, starting from 1. Use with limit to inspect additional pages. Capped to " + MAX_PAGE + " so Lucene collection stays bounded when limit is at its maximum."
95101
));
96102
properties.put("title_only", Map.of(
97103
"type", "boolean",
@@ -114,8 +120,8 @@ public Object execute(Map<String, Object> arguments) {
114120
String startDate = mcpUtils.getStringArg(arguments, "start_date");
115121
String endDate = mcpUtils.getStringArg(arguments, "end_date");
116122
String dateField = mcpUtils.getStringArg(arguments, "date_field");
117-
int limit = Math.min(Math.max(mcpUtils.getIntArg(arguments, "limit", 50), 1), 500);
118-
int page = Math.max(mcpUtils.getIntArg(arguments, "page", 1), 1);
123+
int limit = Math.min(Math.max(mcpUtils.getIntArg(arguments, "limit", 50), 1), MAX_LIMIT);
124+
int page = Math.min(Math.max(mcpUtils.getIntArg(arguments, "page", 1), 1), MAX_PAGE);
119125
boolean titleOnly = mcpUtils.getBoolArg(arguments, "title_only", false);
120126

121127
if (StringUtils.isBlank(query)) {

app/server/huntly-server/src/test/java/com/huntly/server/mcp/SearchContentToolTest.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ void inputSchemaDocumentsAdvancedSearchQuotingAndUnsortedPagination() {
2929
Map<String, Object> query = asMap(properties.get("query"));
3030
Map<String, Object> libraryFilter = asMap(properties.get("library_filter"));
3131
Map<String, Object> dateField = asMap(properties.get("date_field"));
32+
Map<String, Object> page = asMap(properties.get("page"));
3233

3334
assertThat(tool.getDescription())
3435
.contains("collection:\"Daily Reads\"")
@@ -40,6 +41,7 @@ void inputSchemaDocumentsAdvancedSearchQuotingAndUnsortedPagination() {
4041
assertThat(properties).containsKeys("start_date", "end_date", "date_field");
4142
assertThat(asList(dateField.get("enum"))).contains("created_at", "collected_at", "last_read_at");
4243
assertThat(properties).containsKey("page");
44+
assertThat(page.get("maximum")).isEqualTo(20);
4345
}
4446

4547
@Test
@@ -96,6 +98,31 @@ void executeMapsFiltersAndReturnsPaginationMetadata() {
9698
assertThat(items).extracting(McpPageItem::getId).containsExactly(7L);
9799
}
98100

101+
@Test
102+
void executeClampsPageToSafeMaximumForLuceneCollection() {
103+
LuceneService luceneService = mock(LuceneService.class);
104+
SearchContentTool tool = new SearchContentTool(luceneService, new McpUtils());
105+
PageSearchResult searchResult = new PageSearchResult();
106+
searchResult.setItems(List.of());
107+
when(luceneService.searchPages(any(SearchQuery.class))).thenReturn(searchResult);
108+
109+
Object rawResponse = tool.execute(Map.of(
110+
"query", "machine learning",
111+
"limit", 500,
112+
"page", Integer.MAX_VALUE
113+
));
114+
115+
ArgumentCaptor<SearchQuery> queryCaptor = ArgumentCaptor.forClass(SearchQuery.class);
116+
verify(luceneService).searchPages(queryCaptor.capture());
117+
SearchQuery searchQuery = queryCaptor.getValue();
118+
assertThat(searchQuery.getSize()).isEqualTo(500);
119+
assertThat(searchQuery.getPage()).isEqualTo(20);
120+
121+
Map<String, Object> response = asMap(rawResponse);
122+
assertThat(response.get("page")).isEqualTo(20);
123+
assertThat(response.get("page_size")).isEqualTo(500);
124+
}
125+
99126
@SuppressWarnings("unchecked")
100127
private Map<String, Object> asMap(Object value) {
101128
return (Map<String, Object>) value;

0 commit comments

Comments
 (0)