Skip to content

Commit ff36308

Browse files
hayaksi1claude
andcommitted
Scope the partial-index warning to the room being searched
The warning was driven by `currentRoom() !== null`, which is a global signal: it is non-null while the crawler has any outstanding checkpoint for any room. Searching a fully-crawled room while an unrelated room was still being crawled therefore claimed the results may be incomplete when they were not. Drive it from `crawlingRooms()` instead, which reports the rooms with outstanding checkpoints by id, and pass the search scope and room id in from RoomSearchAuxPanel: a room-scoped search now asks only about that room, while an all-rooms search still reacts to any outstanding checkpoint. Using room ids throughout also avoids `currentRoom()` returning null, and so under-reporting a crawl, when the js-sdk does not know the room at the head of the queue. The `changedCheckpoint` payload only carries the globally-current room and cannot answer a per-room question, so the handler re-reads the checkpoint set. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 8b0dfed commit ff36308

3 files changed

Lines changed: 168 additions & 53 deletions

File tree

apps/web/src/components/views/elements/SearchWarning.tsx

Lines changed: 42 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
66
Please see LICENSE files in the repository root for full details.
77
*/
88

9-
import React, { type JSX, type ReactNode, useEffect, useState } from "react";
9+
import React, { type JSX, type ReactNode, useCallback, useEffect, useState } from "react";
1010
import { logger } from "matrix-js-sdk/src/logger";
11-
import { type Room } from "matrix-js-sdk/src/matrix";
1211

1312
import EventIndexPeg from "../../../indexing/EventIndexPeg";
1413
import type EventIndex from "../../../indexing/EventIndex";
14+
import { SearchScope } from "../../../Searching";
1515
import { _t } from "../../../languageHandler";
1616
import SdkConfig from "../../../SdkConfig";
1717
import dis from "../../../dispatcher/dispatcher";
@@ -28,55 +28,77 @@ interface IProps {
2828
isRoomEncrypted?: boolean;
2929
kind: WarningKind;
3030
showLogo?: boolean;
31+
/** The scope of the search being warned about; only meaningful for {@link WarningKind.Search}. */
32+
scope?: SearchScope;
33+
/** The room being searched. Mirrors `SearchInfo.roomId`: `undefined` when searching all rooms. */
34+
roomId?: string;
3135
}
3236

3337
/**
34-
* Track whether the given event index is still crawling not-yet-indexed history.
38+
* Track whether the index still has history left to crawl that is relevant to the given search.
3539
*
36-
* The index reports its progress via `currentRoom()`, which returns the room currently being
37-
* crawled, or `null` once every checkpoint has drained and the index is fully built. It emits a
38-
* `changedCheckpoint` event whenever that progresses, so we subscribe to it and re-evaluate, which
39-
* means the warning auto-clears the moment indexing finishes.
40+
* Seshat has no notion of a room being *fully* indexed: {@link EventIndex.isRoomIndexed} only
41+
* reports whether the index holds any events for a room, not whether it holds all of them. The
42+
* closest available signal is whether the crawler still holds an outstanding checkpoint for the
43+
* room, exposed via {@link EventIndex.crawlingRooms}. A room-scoped search therefore asks about
44+
* that room alone, while an all-rooms search is affected by any outstanding checkpoint.
45+
*
46+
* Note that the absence of a checkpoint is not proof of completeness: a room also has no
47+
* checkpoint if it never had a back-pagination token to crawl from, if its checkpoint was dropped
48+
* because the server rejected the request, or before the initial checkpoints have been seeded. So
49+
* this signal under-warns rather than over-warns.
50+
*
51+
* The index emits `changedCheckpoint` on each checkpoint transition, but the payload carries only
52+
* the globally-current room, so we re-read the checkpoint set on each event rather than trust it.
4053
*
4154
* @param index The event index to observe, or `null` if there is no index.
42-
* @returns `true` while the crawl is still in progress, `false` otherwise.
55+
* @param scope The scope of the search, if this warning is being rendered for one.
56+
* @param roomId The room being searched, or `undefined` when searching all rooms.
57+
* @returns `true` while history relevant to the search is still being crawled, `false` otherwise.
4358
*/
44-
function useIsCrawlInProgress(index: EventIndex | null): boolean {
45-
const [crawlInProgress, setCrawlInProgress] = useState<boolean>(() =>
46-
index ? index.currentRoom() !== null : false,
47-
);
59+
function useIsCrawlInProgress(index: EventIndex | null, scope?: SearchScope, roomId?: string): boolean {
60+
const readCrawlInProgress = useCallback((): boolean => {
61+
if (!index) return false;
62+
const { crawlingRooms } = index.crawlingRooms();
63+
// Fall back to the global check when we don't know which room is being searched: the room
64+
// id may still be undefined while a room alias is being resolved.
65+
if (scope !== SearchScope.Room || roomId === undefined) return crawlingRooms.size > 0;
66+
return crawlingRooms.has(roomId);
67+
}, [index, scope, roomId]);
68+
69+
const [crawlInProgress, setCrawlInProgress] = useState<boolean>(readCrawlInProgress);
4870

4971
useEffect(() => {
5072
if (!index) {
5173
setCrawlInProgress(false);
5274
return;
5375
}
5476

55-
const onChangedCheckpoint = (currentRoom: Room | null): void => {
56-
setCrawlInProgress(currentRoom !== null);
77+
const onChangedCheckpoint = (): void => {
78+
setCrawlInProgress(readCrawlInProgress());
5779
};
5880

5981
// Re-sync in case the crawl state changed between the initial render and the subscription.
60-
setCrawlInProgress(index.currentRoom() !== null);
82+
onChangedCheckpoint();
6183
index.on("changedCheckpoint", onChangedCheckpoint);
6284

6385
return () => {
6486
index.removeListener("changedCheckpoint", onChangedCheckpoint);
6587
};
66-
}, [index]);
88+
}, [index, readCrawlInProgress]);
6789

6890
return crawlInProgress;
6991
}
7092

71-
export default function SearchWarning({ isRoomEncrypted, kind, showLogo = true }: IProps): JSX.Element {
93+
export default function SearchWarning({ isRoomEncrypted, kind, showLogo = true, scope, roomId }: IProps): JSX.Element {
7294
const eventIndex = EventIndexPeg.get();
73-
const crawlInProgress = useIsCrawlInProgress(eventIndex);
95+
const crawlInProgress = useIsCrawlInProgress(eventIndex, scope, roomId);
7496

7597
if (!isRoomEncrypted) return <></>;
7698

7799
if (eventIndex) {
78-
// The index exists but is still draining its checkpoint queue, so searches over
79-
// not-yet-indexed history may silently return partial results (#32253). Warn the user.
100+
// The index exists but still has history to crawl for this search, so it may silently
101+
// return partial results (#32253). Warn the user.
80102
if (crawlInProgress && kind === WarningKind.Search) {
81103
// This warning appears dynamically while a search panel is already open (the crawler
82104
// finishes draining mid-session), so mark it as a polite live region for screen readers.

apps/web/src/components/views/rooms/RoomSearchAuxPanel.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,13 @@ const RoomSearchAuxPanel: React.FC<Props> = ({ searchInfo, isRoomEncrypted, onSe
4545
) : (
4646
<InlineSpinner />
4747
)}
48-
<SearchWarning kind={WarningKind.Search} isRoomEncrypted={isRoomEncrypted} showLogo={false} />
48+
<SearchWarning
49+
kind={WarningKind.Search}
50+
isRoomEncrypted={isRoomEncrypted}
51+
showLogo={false}
52+
scope={scope}
53+
roomId={searchInfo?.roomId}
54+
/>
4955
</div>
5056
</div>
5157
<div className="mx_RoomSearchAuxPanel_buttons">

apps/web/test/unit-tests/components/views/elements/SearchWarning-test.tsx

Lines changed: 119 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,24 @@ import SdkConfig from "../../../../../src/SdkConfig";
1414
import SearchWarning, { WarningKind } from "../../../../../src/components/views/elements/SearchWarning";
1515
import EventIndexPeg from "../../../../../src/indexing/EventIndexPeg";
1616
import { type default as EventIndex } from "../../../../../src/indexing/EventIndex";
17+
import { SearchScope } from "../../../../../src/Searching";
18+
19+
const SEARCHED_ROOM = "!searched:example.org";
20+
const OTHER_ROOM = "!other:example.org";
21+
const PARTIAL_WARNING = "Results may be incomplete because your search index is still being built.";
1722

1823
/**
1924
* A minimal fake EventIndex exposing only the surface that SearchWarning consumes:
20-
* `currentRoom()` (null once the crawl is complete) and the `changedCheckpoint` emitter.
25+
* `crawlingRooms()` (the rooms with outstanding crawler checkpoints) and the `changedCheckpoint`
26+
* emitter.
2127
*/
2228
class FakeEventIndex {
2329
private listeners = new Map<string, Set<(...args: any[]) => void>>();
2430

25-
public constructor(private current: Room | null) {}
31+
public constructor(private crawling: string[] = []) {}
2632

27-
public currentRoom(): Room | null {
28-
return this.current;
33+
public crawlingRooms(): { crawlingRooms: Set<string>; totalRooms: Set<string> } {
34+
return { crawlingRooms: new Set(this.crawling), totalRooms: new Set(this.crawling) };
2935
}
3036

3137
public on(event: string, listener: (...args: any[]) => void): void {
@@ -37,10 +43,15 @@ class FakeEventIndex {
3743
this.listeners.get(event)?.delete(listener);
3844
}
3945

40-
/** Test helper: simulate the crawler advancing (or finishing, when `current` is null). */
41-
public emitChangedCheckpoint(current: Room | null): void {
42-
this.current = current;
43-
this.listeners.get("changedCheckpoint")?.forEach((listener) => listener(current));
46+
/**
47+
* Test helper: simulate the crawler advancing (or finishing, when `crawling` is empty).
48+
*
49+
* The real index emits only the globally-current room, which cannot answer a per-room
50+
* question, so `currentRoom` is passed through purely to prove consumers ignore it.
51+
*/
52+
public emitChangedCheckpoint(crawling: string[], currentRoom: Room | null = null): void {
53+
this.crawling = crawling;
54+
this.listeners.get("changedCheckpoint")?.forEach((listener) => listener(currentRoom));
4455
}
4556
}
4657

@@ -86,61 +97,137 @@ describe("<SearchWarning />", () => {
8697
EventIndexPeg.index = index as unknown as EventIndex;
8798
};
8899

89-
it("renders the partial-index warning while the crawl is still in progress", () => {
90-
setIndex(new FakeEventIndex({ name: "Encrypted room" } as Room));
100+
it("warns a room-scoped search while the searched room is still being crawled", () => {
101+
setIndex(new FakeEventIndex([SEARCHED_ROOM]));
91102

92103
const { queryByText, queryByRole } = render(
93-
<SearchWarning isRoomEncrypted={true} kind={WarningKind.Search} />,
104+
<SearchWarning
105+
isRoomEncrypted={true}
106+
kind={WarningKind.Search}
107+
scope={SearchScope.Room}
108+
roomId={SEARCHED_ROOM}
109+
/>,
94110
);
95111

96-
expect(
97-
queryByText("Results may be incomplete because your search index is still being built."),
98-
).toBeInTheDocument();
112+
expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument();
99113
// The notice appears dynamically while the panel is open, so it must be a live region (#32253).
100114
expect(queryByRole("status")).toBeInTheDocument();
101115
});
102116

117+
it("does not warn a room-scoped search when only an unrelated room is still being crawled", () => {
118+
setIndex(new FakeEventIndex([OTHER_ROOM]));
119+
120+
const { container } = render(
121+
<SearchWarning
122+
isRoomEncrypted={true}
123+
kind={WarningKind.Search}
124+
scope={SearchScope.Room}
125+
roomId={SEARCHED_ROOM}
126+
/>,
127+
);
128+
129+
expect(container).toBeEmptyDOMElement();
130+
});
131+
132+
it("warns an all-rooms search while any room is still being crawled", () => {
133+
setIndex(new FakeEventIndex([OTHER_ROOM]));
134+
135+
const { queryByText } = render(
136+
<SearchWarning isRoomEncrypted={true} kind={WarningKind.Search} scope={SearchScope.All} />,
137+
);
138+
139+
expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument();
140+
});
141+
142+
it("falls back to the global check when the searched room is not yet known", () => {
143+
// RoomView can render before a room alias has resolved to an id.
144+
setIndex(new FakeEventIndex([OTHER_ROOM]));
145+
146+
const { queryByText } = render(
147+
<SearchWarning
148+
isRoomEncrypted={true}
149+
kind={WarningKind.Search}
150+
scope={SearchScope.Room}
151+
roomId={undefined}
152+
/>,
153+
);
154+
155+
expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument();
156+
});
157+
103158
it("renders nothing once the index has finished crawling", () => {
104-
// currentRoom() returns null when the crawl is complete (fully indexed).
105-
setIndex(new FakeEventIndex(null));
159+
setIndex(new FakeEventIndex([]));
106160

107-
const { container } = render(<SearchWarning isRoomEncrypted={true} kind={WarningKind.Search} />);
161+
const { container } = render(
162+
<SearchWarning isRoomEncrypted={true} kind={WarningKind.Search} scope={SearchScope.All} />,
163+
);
108164

109165
expect(container).toBeEmptyDOMElement();
110166
});
111167

112168
it("does not warn for the Files kind even while crawling", () => {
113-
setIndex(new FakeEventIndex({ name: "Encrypted room" } as Room));
169+
setIndex(new FakeEventIndex([SEARCHED_ROOM]));
114170

115171
const { container } = render(<SearchWarning isRoomEncrypted={true} kind={WarningKind.Files} />);
116172

117173
expect(container).toBeEmptyDOMElement();
118174
});
119175

120-
it("clears the warning when a changedCheckpoint event reports completion", () => {
121-
const index = new FakeEventIndex({ name: "Encrypted room" } as Room);
176+
it("clears the warning when the searched room's checkpoint drains", () => {
177+
const index = new FakeEventIndex([SEARCHED_ROOM]);
122178
setIndex(index);
123179

124-
const { queryByText } = render(<SearchWarning isRoomEncrypted={true} kind={WarningKind.Search} />);
180+
const { queryByText } = render(
181+
<SearchWarning
182+
isRoomEncrypted={true}
183+
kind={WarningKind.Search}
184+
scope={SearchScope.Room}
185+
roomId={SEARCHED_ROOM}
186+
/>,
187+
);
125188

126-
expect(
127-
queryByText("Results may be incomplete because your search index is still being built."),
128-
).toBeInTheDocument();
189+
expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument();
129190

130-
// The crawler drains its last checkpoint: currentRoom() becomes null.
131191
act(() => {
132-
index.emitChangedCheckpoint(null);
192+
index.emitChangedCheckpoint([]);
133193
});
134194

135-
expect(
136-
queryByText("Results may be incomplete because your search index is still being built."),
137-
).not.toBeInTheDocument();
195+
expect(queryByText(PARTIAL_WARNING)).not.toBeInTheDocument();
138196
});
139197

140-
it("renders nothing when the room is not encrypted even while crawling", () => {
141-
setIndex(new FakeEventIndex({ name: "Encrypted room" } as Room));
198+
it("keeps warning when the crawler moves to another room but the searched room is still queued", () => {
199+
const index = new FakeEventIndex([SEARCHED_ROOM, OTHER_ROOM]);
200+
setIndex(index);
201+
202+
const { queryByText } = render(
203+
<SearchWarning
204+
isRoomEncrypted={true}
205+
kind={WarningKind.Search}
206+
scope={SearchScope.Room}
207+
roomId={SEARCHED_ROOM}
208+
/>,
209+
);
142210

143-
const { container } = render(<SearchWarning isRoomEncrypted={false} kind={WarningKind.Search} />);
211+
// The event payload names a different room; the searched room is still outstanding, so
212+
// the warning must survive. This fails if the handler trusts the payload.
213+
act(() => {
214+
index.emitChangedCheckpoint([SEARCHED_ROOM], { roomId: OTHER_ROOM } as Room);
215+
});
216+
217+
expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument();
218+
});
219+
220+
it("renders nothing when the room is not encrypted even while crawling", () => {
221+
setIndex(new FakeEventIndex([SEARCHED_ROOM]));
222+
223+
const { container } = render(
224+
<SearchWarning
225+
isRoomEncrypted={false}
226+
kind={WarningKind.Search}
227+
scope={SearchScope.Room}
228+
roomId={SEARCHED_ROOM}
229+
/>,
230+
);
144231

145232
expect(container).toBeEmptyDOMElement();
146233
});

0 commit comments

Comments
 (0)