From 80b01c6a2f47fd2abcc43eed1aa08801a7bd8d88 Mon Sep 17 00:00:00 2001 From: hayaksi1 <193020925+hayaksi1@users.noreply.github.com> Date: Sat, 27 Jun 2026 11:06:15 +0300 Subject: [PATCH 1/3] Warn when an encrypted search runs before the index has finished building When a search runs in an encrypted room while the local Seshat index is still crawling not-yet-indexed history, results can silently come back partial. SearchWarning now subscribes to the event index's changedCheckpoint progress and shows a polite (role=status) notice while the crawl is in progress, clearing automatically the moment indexing finishes. --- .../views/elements/SearchWarning.tsx | 61 ++++++++- apps/web/src/i18n/strings/en_EN.json | 3 +- .../views/elements/SearchWarning-test.tsx | 129 +++++++++++++++++- 3 files changed, 189 insertions(+), 4 deletions(-) diff --git a/apps/web/src/components/views/elements/SearchWarning.tsx b/apps/web/src/components/views/elements/SearchWarning.tsx index c8c0ec0bffe..fe86527fd74 100644 --- a/apps/web/src/components/views/elements/SearchWarning.tsx +++ b/apps/web/src/components/views/elements/SearchWarning.tsx @@ -6,10 +6,12 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -import React, { type JSX, type ReactNode } from "react"; +import React, { type JSX, type ReactNode, useEffect, useState } from "react"; import { logger } from "matrix-js-sdk/src/logger"; +import { type Room } from "matrix-js-sdk/src/matrix"; import EventIndexPeg from "../../../indexing/EventIndexPeg"; +import type EventIndex from "../../../indexing/EventIndex"; import { _t } from "../../../languageHandler"; import SdkConfig from "../../../SdkConfig"; import dis from "../../../dispatcher/dispatcher"; @@ -28,9 +30,64 @@ interface IProps { showLogo?: boolean; } +/** + * Track whether the given event index is still crawling not-yet-indexed history. + * + * The index reports its progress via `currentRoom()`, which returns the room currently being + * crawled, or `null` once every checkpoint has drained and the index is fully built. It emits a + * `changedCheckpoint` event whenever that progresses, so we subscribe to it and re-evaluate, which + * means the warning auto-clears the moment indexing finishes. + * + * @param index The event index to observe, or `null` if there is no index. + * @returns `true` while the crawl is still in progress, `false` otherwise. + */ +function useIsCrawlInProgress(index: EventIndex | null): boolean { + const [crawlInProgress, setCrawlInProgress] = useState(() => + index ? index.currentRoom() !== null : false, + ); + + useEffect(() => { + if (!index) { + setCrawlInProgress(false); + return; + } + + const onChangedCheckpoint = (currentRoom: Room | null): void => { + setCrawlInProgress(currentRoom !== null); + }; + + // Re-sync in case the crawl state changed between the initial render and the subscription. + setCrawlInProgress(index.currentRoom() !== null); + index.on("changedCheckpoint", onChangedCheckpoint); + + return () => { + index.removeListener("changedCheckpoint", onChangedCheckpoint); + }; + }, [index]); + + return crawlInProgress; +} + export default function SearchWarning({ isRoomEncrypted, kind, showLogo = true }: IProps): JSX.Element { + const eventIndex = EventIndexPeg.get(); + const crawlInProgress = useIsCrawlInProgress(eventIndex); + if (!isRoomEncrypted) return <>; - if (EventIndexPeg.get()) return <>; + + if (eventIndex) { + // The index exists but is still draining its checkpoint queue, so searches over + // not-yet-indexed history may silently return partial results (#32253). Warn the user. + if (crawlInProgress && kind === WarningKind.Search) { + // This warning appears dynamically while a search panel is already open (the crawler + // finishes draining mid-session), so mark it as a polite live region for screen readers. + return ( +
+ {_t("seshat|warning_kind_search_partial")} +
+ ); + } + return <>; + } if (EventIndexPeg.error) { return ( diff --git a/apps/web/src/i18n/strings/en_EN.json b/apps/web/src/i18n/strings/en_EN.json index c6f15639676..e5051732dfa 100644 --- a/apps/web/src/i18n/strings/en_EN.json +++ b/apps/web/src/i18n/strings/en_EN.json @@ -2476,7 +2476,8 @@ "warning_kind_files": "This version of %(brand)s does not support viewing some encrypted files", "warning_kind_files_app": "Use the Desktop app to see all encrypted files", "warning_kind_search": "This version of %(brand)s does not support searching encrypted messages", - "warning_kind_search_app": "Use the Desktop app to search encrypted messages" + "warning_kind_search_app": "Use the Desktop app to search encrypted messages", + "warning_kind_search_partial": "Results may be incomplete because your search index is still being built." }, "setting": { "help_about": { diff --git a/apps/web/test/unit-tests/components/views/elements/SearchWarning-test.tsx b/apps/web/test/unit-tests/components/views/elements/SearchWarning-test.tsx index 117870d7e90..2c5db54207a 100644 --- a/apps/web/test/unit-tests/components/views/elements/SearchWarning-test.tsx +++ b/apps/web/test/unit-tests/components/views/elements/SearchWarning-test.tsx @@ -6,15 +6,53 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -import { render } from "jest-matrix-react"; +import { act, render } from "jest-matrix-react"; import React from "react"; +import { type Room } from "matrix-js-sdk/src/matrix"; import SdkConfig from "../../../../../src/SdkConfig"; import SearchWarning, { WarningKind } from "../../../../../src/components/views/elements/SearchWarning"; +import EventIndexPeg from "../../../../../src/indexing/EventIndexPeg"; +import { type default as EventIndex } from "../../../../../src/indexing/EventIndex"; + +/** + * A minimal fake EventIndex exposing only the surface that SearchWarning consumes: + * `currentRoom()` (null once the crawl is complete) and the `changedCheckpoint` emitter. + */ +class FakeEventIndex { + private listeners = new Map void>>(); + + public constructor(private current: Room | null) {} + + public currentRoom(): Room | null { + return this.current; + } + + public on(event: string, listener: (...args: any[]) => void): void { + if (!this.listeners.has(event)) this.listeners.set(event, new Set()); + this.listeners.get(event)!.add(listener); + } + + public removeListener(event: string, listener: (...args: any[]) => void): void { + this.listeners.get(event)?.delete(listener); + } + + /** Test helper: simulate the crawler advancing (or finishing, when `current` is null). */ + public emitChangedCheckpoint(current: Room | null): void { + this.current = current; + this.listeners.get("changedCheckpoint")?.forEach((listener) => listener(current)); + } +} describe("", () => { + afterEach(() => { + EventIndexPeg.index = null; + EventIndexPeg.error = undefined; + }); + describe("with desktop builds available", () => { beforeEach(() => { + EventIndexPeg.index = null; SdkConfig.put({ brand: "Element", desktop_builds: { @@ -42,4 +80,93 @@ describe("", () => { expect(asFragment()).toMatchSnapshot(); }); }); + + describe("with the event index present", () => { + const setIndex = (index: FakeEventIndex): void => { + EventIndexPeg.index = index as unknown as EventIndex; + }; + + it("renders the partial-index warning while the crawl is still in progress", () => { + setIndex(new FakeEventIndex({ name: "Encrypted room" } as Room)); + + const { queryByText, queryByRole } = render( + , + ); + + expect( + queryByText("Results may be incomplete because your search index is still being built."), + ).toBeInTheDocument(); + // The notice appears dynamically while the panel is open, so it must be a live region (#32253). + expect(queryByRole("status")).toBeInTheDocument(); + }); + + it("renders nothing once the index has finished crawling", () => { + // currentRoom() returns null when the crawl is complete (fully indexed). + setIndex(new FakeEventIndex(null)); + + const { container } = render(); + + expect(container).toBeEmptyDOMElement(); + }); + + it("does not warn for the Files kind even while crawling", () => { + setIndex(new FakeEventIndex({ name: "Encrypted room" } as Room)); + + const { container } = render(); + + expect(container).toBeEmptyDOMElement(); + }); + + it("clears the warning when a changedCheckpoint event reports completion", () => { + const index = new FakeEventIndex({ name: "Encrypted room" } as Room); + setIndex(index); + + const { queryByText } = render(); + + expect( + queryByText("Results may be incomplete because your search index is still being built."), + ).toBeInTheDocument(); + + // The crawler drains its last checkpoint: currentRoom() becomes null. + act(() => { + index.emitChangedCheckpoint(null); + }); + + expect( + queryByText("Results may be incomplete because your search index is still being built."), + ).not.toBeInTheDocument(); + }); + + it("renders nothing when the room is not encrypted even while crawling", () => { + setIndex(new FakeEventIndex({ name: "Encrypted room" } as Room)); + + const { container } = render(); + + expect(container).toBeEmptyDOMElement(); + }); + }); + + describe("with no event index (web build)", () => { + beforeEach(() => { + EventIndexPeg.index = null; + EventIndexPeg.error = undefined; + SdkConfig.put({ + brand: "Element", + desktop_builds: { + available: true, + logo: "https://logo", + url: "https://url", + }, + }); + }); + + it("still renders the desktop/enable-search warning", () => { + const { container } = render(); + + expect(container.querySelector(".mx_SearchWarning")).toBeInTheDocument(); + expect(container.textContent).toContain("to search encrypted messages"); + // It is the desktop/enable-search affordance, not the partial-index notice. + expect(container.textContent).not.toContain("your search index is still being built"); + }); + }); }); From dc321c58d21ef26c359f2f3ad5bb125ea59fd115 Mon Sep 17 00:00:00 2001 From: hayaksi1 <193020925+hayaksi1@users.noreply.github.com> Date: Tue, 14 Jul 2026 19:11:32 +0300 Subject: [PATCH 2/3] 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. --- .../views/elements/SearchWarning.tsx | 62 ++++--- .../views/rooms/RoomSearchAuxPanel.tsx | 8 +- .../views/elements/SearchWarning-test.tsx | 151 ++++++++++++++---- 3 files changed, 168 insertions(+), 53 deletions(-) diff --git a/apps/web/src/components/views/elements/SearchWarning.tsx b/apps/web/src/components/views/elements/SearchWarning.tsx index fe86527fd74..b92e98bb844 100644 --- a/apps/web/src/components/views/elements/SearchWarning.tsx +++ b/apps/web/src/components/views/elements/SearchWarning.tsx @@ -6,12 +6,12 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -import React, { type JSX, type ReactNode, useEffect, useState } from "react"; +import React, { type JSX, type ReactNode, useCallback, useEffect, useState } from "react"; import { logger } from "matrix-js-sdk/src/logger"; -import { type Room } from "matrix-js-sdk/src/matrix"; import EventIndexPeg from "../../../indexing/EventIndexPeg"; import type EventIndex from "../../../indexing/EventIndex"; +import { SearchScope } from "../../../Searching"; import { _t } from "../../../languageHandler"; import SdkConfig from "../../../SdkConfig"; import dis from "../../../dispatcher/dispatcher"; @@ -28,23 +28,45 @@ interface IProps { isRoomEncrypted?: boolean; kind: WarningKind; showLogo?: boolean; + /** The scope of the search being warned about; only meaningful for {@link WarningKind.Search}. */ + scope?: SearchScope; + /** The room being searched. Mirrors `SearchInfo.roomId`: `undefined` when searching all rooms. */ + roomId?: string; } /** - * Track whether the given event index is still crawling not-yet-indexed history. + * Track whether the index still has history left to crawl that is relevant to the given search. * - * The index reports its progress via `currentRoom()`, which returns the room currently being - * crawled, or `null` once every checkpoint has drained and the index is fully built. It emits a - * `changedCheckpoint` event whenever that progresses, so we subscribe to it and re-evaluate, which - * means the warning auto-clears the moment indexing finishes. + * Seshat has no notion of a room being *fully* indexed: {@link EventIndex.isRoomIndexed} only + * reports whether the index holds any events for a room, not whether it holds all of them. The + * closest available signal is whether the crawler still holds an outstanding checkpoint for the + * room, exposed via {@link EventIndex.crawlingRooms}. A room-scoped search therefore asks about + * that room alone, while an all-rooms search is affected by any outstanding checkpoint. + * + * Note that the absence of a checkpoint is not proof of completeness: a room also has no + * checkpoint if it never had a back-pagination token to crawl from, if its checkpoint was dropped + * because the server rejected the request, or before the initial checkpoints have been seeded. So + * this signal under-warns rather than over-warns. + * + * The index emits `changedCheckpoint` on each checkpoint transition, but the payload carries only + * the globally-current room, so we re-read the checkpoint set on each event rather than trust it. * * @param index The event index to observe, or `null` if there is no index. - * @returns `true` while the crawl is still in progress, `false` otherwise. + * @param scope The scope of the search, if this warning is being rendered for one. + * @param roomId The room being searched, or `undefined` when searching all rooms. + * @returns `true` while history relevant to the search is still being crawled, `false` otherwise. */ -function useIsCrawlInProgress(index: EventIndex | null): boolean { - const [crawlInProgress, setCrawlInProgress] = useState(() => - index ? index.currentRoom() !== null : false, - ); +function useIsCrawlInProgress(index: EventIndex | null, scope?: SearchScope, roomId?: string): boolean { + const readCrawlInProgress = useCallback((): boolean => { + if (!index) return false; + const { crawlingRooms } = index.crawlingRooms(); + // Fall back to the global check when we don't know which room is being searched: the room + // id may still be undefined while a room alias is being resolved. + if (scope !== SearchScope.Room || roomId === undefined) return crawlingRooms.size > 0; + return crawlingRooms.has(roomId); + }, [index, scope, roomId]); + + const [crawlInProgress, setCrawlInProgress] = useState(readCrawlInProgress); useEffect(() => { if (!index) { @@ -52,31 +74,31 @@ function useIsCrawlInProgress(index: EventIndex | null): boolean { return; } - const onChangedCheckpoint = (currentRoom: Room | null): void => { - setCrawlInProgress(currentRoom !== null); + const onChangedCheckpoint = (): void => { + setCrawlInProgress(readCrawlInProgress()); }; // Re-sync in case the crawl state changed between the initial render and the subscription. - setCrawlInProgress(index.currentRoom() !== null); + onChangedCheckpoint(); index.on("changedCheckpoint", onChangedCheckpoint); return () => { index.removeListener("changedCheckpoint", onChangedCheckpoint); }; - }, [index]); + }, [index, readCrawlInProgress]); return crawlInProgress; } -export default function SearchWarning({ isRoomEncrypted, kind, showLogo = true }: IProps): JSX.Element { +export default function SearchWarning({ isRoomEncrypted, kind, showLogo = true, scope, roomId }: IProps): JSX.Element { const eventIndex = EventIndexPeg.get(); - const crawlInProgress = useIsCrawlInProgress(eventIndex); + const crawlInProgress = useIsCrawlInProgress(eventIndex, scope, roomId); if (!isRoomEncrypted) return <>; if (eventIndex) { - // The index exists but is still draining its checkpoint queue, so searches over - // not-yet-indexed history may silently return partial results (#32253). Warn the user. + // The index exists but still has history to crawl for this search, so it may silently + // return partial results (#32253). Warn the user. if (crawlInProgress && kind === WarningKind.Search) { // This warning appears dynamically while a search panel is already open (the crawler // finishes draining mid-session), so mark it as a polite live region for screen readers. diff --git a/apps/web/src/components/views/rooms/RoomSearchAuxPanel.tsx b/apps/web/src/components/views/rooms/RoomSearchAuxPanel.tsx index 14c7ad511ed..b2a7119d3a7 100644 --- a/apps/web/src/components/views/rooms/RoomSearchAuxPanel.tsx +++ b/apps/web/src/components/views/rooms/RoomSearchAuxPanel.tsx @@ -45,7 +45,13 @@ const RoomSearchAuxPanel: React.FC = ({ searchInfo, isRoomEncrypted, onSe ) : ( )} - +
diff --git a/apps/web/test/unit-tests/components/views/elements/SearchWarning-test.tsx b/apps/web/test/unit-tests/components/views/elements/SearchWarning-test.tsx index 2c5db54207a..871a97aa99e 100644 --- a/apps/web/test/unit-tests/components/views/elements/SearchWarning-test.tsx +++ b/apps/web/test/unit-tests/components/views/elements/SearchWarning-test.tsx @@ -14,18 +14,24 @@ import SdkConfig from "../../../../../src/SdkConfig"; import SearchWarning, { WarningKind } from "../../../../../src/components/views/elements/SearchWarning"; import EventIndexPeg from "../../../../../src/indexing/EventIndexPeg"; import { type default as EventIndex } from "../../../../../src/indexing/EventIndex"; +import { SearchScope } from "../../../../../src/Searching"; + +const SEARCHED_ROOM = "!searched:example.org"; +const OTHER_ROOM = "!other:example.org"; +const PARTIAL_WARNING = "Results may be incomplete because your search index is still being built."; /** * A minimal fake EventIndex exposing only the surface that SearchWarning consumes: - * `currentRoom()` (null once the crawl is complete) and the `changedCheckpoint` emitter. + * `crawlingRooms()` (the rooms with outstanding crawler checkpoints) and the `changedCheckpoint` + * emitter. */ class FakeEventIndex { private listeners = new Map void>>(); - public constructor(private current: Room | null) {} + public constructor(private crawling: string[] = []) {} - public currentRoom(): Room | null { - return this.current; + public crawlingRooms(): { crawlingRooms: Set; totalRooms: Set } { + return { crawlingRooms: new Set(this.crawling), totalRooms: new Set(this.crawling) }; } public on(event: string, listener: (...args: any[]) => void): void { @@ -37,10 +43,15 @@ class FakeEventIndex { this.listeners.get(event)?.delete(listener); } - /** Test helper: simulate the crawler advancing (or finishing, when `current` is null). */ - public emitChangedCheckpoint(current: Room | null): void { - this.current = current; - this.listeners.get("changedCheckpoint")?.forEach((listener) => listener(current)); + /** + * Test helper: simulate the crawler advancing (or finishing, when `crawling` is empty). + * + * The real index emits only the globally-current room, which cannot answer a per-room + * question, so `currentRoom` is passed through purely to prove consumers ignore it. + */ + public emitChangedCheckpoint(crawling: string[], currentRoom: Room | null = null): void { + this.crawling = crawling; + this.listeners.get("changedCheckpoint")?.forEach((listener) => listener(currentRoom)); } } @@ -86,61 +97,137 @@ describe("", () => { EventIndexPeg.index = index as unknown as EventIndex; }; - it("renders the partial-index warning while the crawl is still in progress", () => { - setIndex(new FakeEventIndex({ name: "Encrypted room" } as Room)); + it("warns a room-scoped search while the searched room is still being crawled", () => { + setIndex(new FakeEventIndex([SEARCHED_ROOM])); const { queryByText, queryByRole } = render( - , + , ); - expect( - queryByText("Results may be incomplete because your search index is still being built."), - ).toBeInTheDocument(); + expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument(); // The notice appears dynamically while the panel is open, so it must be a live region (#32253). expect(queryByRole("status")).toBeInTheDocument(); }); + it("does not warn a room-scoped search when only an unrelated room is still being crawled", () => { + setIndex(new FakeEventIndex([OTHER_ROOM])); + + const { container } = render( + , + ); + + expect(container).toBeEmptyDOMElement(); + }); + + it("warns an all-rooms search while any room is still being crawled", () => { + setIndex(new FakeEventIndex([OTHER_ROOM])); + + const { queryByText } = render( + , + ); + + expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument(); + }); + + it("falls back to the global check when the searched room is not yet known", () => { + // RoomView can render before a room alias has resolved to an id. + setIndex(new FakeEventIndex([OTHER_ROOM])); + + const { queryByText } = render( + , + ); + + expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument(); + }); + it("renders nothing once the index has finished crawling", () => { - // currentRoom() returns null when the crawl is complete (fully indexed). - setIndex(new FakeEventIndex(null)); + setIndex(new FakeEventIndex([])); - const { container } = render(); + const { container } = render( + , + ); expect(container).toBeEmptyDOMElement(); }); it("does not warn for the Files kind even while crawling", () => { - setIndex(new FakeEventIndex({ name: "Encrypted room" } as Room)); + setIndex(new FakeEventIndex([SEARCHED_ROOM])); const { container } = render(); expect(container).toBeEmptyDOMElement(); }); - it("clears the warning when a changedCheckpoint event reports completion", () => { - const index = new FakeEventIndex({ name: "Encrypted room" } as Room); + it("clears the warning when the searched room's checkpoint drains", () => { + const index = new FakeEventIndex([SEARCHED_ROOM]); setIndex(index); - const { queryByText } = render(); + const { queryByText } = render( + , + ); - expect( - queryByText("Results may be incomplete because your search index is still being built."), - ).toBeInTheDocument(); + expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument(); - // The crawler drains its last checkpoint: currentRoom() becomes null. act(() => { - index.emitChangedCheckpoint(null); + index.emitChangedCheckpoint([]); }); - expect( - queryByText("Results may be incomplete because your search index is still being built."), - ).not.toBeInTheDocument(); + expect(queryByText(PARTIAL_WARNING)).not.toBeInTheDocument(); }); - it("renders nothing when the room is not encrypted even while crawling", () => { - setIndex(new FakeEventIndex({ name: "Encrypted room" } as Room)); + it("keeps warning when the crawler moves to another room but the searched room is still queued", () => { + const index = new FakeEventIndex([SEARCHED_ROOM, OTHER_ROOM]); + setIndex(index); + + const { queryByText } = render( + , + ); - const { container } = render(); + // The event payload names a different room; the searched room is still outstanding, so + // the warning must survive. This fails if the handler trusts the payload. + act(() => { + index.emitChangedCheckpoint([SEARCHED_ROOM], { roomId: OTHER_ROOM } as Room); + }); + + expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument(); + }); + + it("renders nothing when the room is not encrypted even while crawling", () => { + setIndex(new FakeEventIndex([SEARCHED_ROOM])); + + const { container } = render( + , + ); expect(container).toBeEmptyDOMElement(); }); From 21d39c18ba31e5a40f5f58dbe74077391d73908f Mon Sep 17 00:00:00 2001 From: hayaksi1 <193020925+hayaksi1@users.noreply.github.com> Date: Tue, 14 Jul 2026 20:07:08 +0300 Subject: [PATCH 3/3] Also warn when the searched room has not been indexed at all The crawl set cannot see a room that has no checkpoint: before the initial checkpoints have been seeded, such a room is absent from it and looks identical to one that has been fully crawled. That is the case issue #32253 describes, so ask isRoomIndexed() as well, and warn when the index holds no events for the room being searched. Only ask it while the crawler still has work outstanding. That is what the warning claims, and the index has no event for its contents changing -- changedCheckpoint fires on checkpoint transitions only, and an idle crawler is silent -- so a warning raised once the crawler had drained would never be re-evaluated and would stick. Rename the hook to useIsIndexIncomplete, as it no longer answers the narrower question of whether a crawl is in progress. Re-seed it from the checkpoint set on each scope or room change so that the previous search's answer is not left on screen while the lookup is in flight, but not on each checkpoint change, which would blink an already-earned warning off and on again. Also reword the crawlingRooms() doc, which described the set as the rooms being crawled when it holds every queued checkpoint too. --- .../views/elements/SearchWarning.tsx | 97 +++++++--- apps/web/src/indexing/EventIndex.ts | 5 +- .../views/elements/SearchWarning-test.tsx | 182 +++++++++++++++--- 3 files changed, 233 insertions(+), 51 deletions(-) diff --git a/apps/web/src/components/views/elements/SearchWarning.tsx b/apps/web/src/components/views/elements/SearchWarning.tsx index b92e98bb844..091a32ec234 100644 --- a/apps/web/src/components/views/elements/SearchWarning.tsx +++ b/apps/web/src/components/views/elements/SearchWarning.tsx @@ -35,47 +35,89 @@ interface IProps { } /** - * Track whether the index still has history left to crawl that is relevant to the given search. + * Track whether the index is still missing history that is relevant to the given search. * - * Seshat has no notion of a room being *fully* indexed: {@link EventIndex.isRoomIndexed} only - * reports whether the index holds any events for a room, not whether it holds all of them. The - * closest available signal is whether the crawler still holds an outstanding checkpoint for the - * room, exposed via {@link EventIndex.crawlingRooms}. A room-scoped search therefore asks about - * that room alone, while an all-rooms search is affected by any outstanding checkpoint. + * A room-scoped search is incomplete if the crawler still holds a checkpoint for the room + * ({@link EventIndex.crawlingRooms}, which covers both the checkpoint being crawled right now and + * those still queued behind it), or if the index holds no events for it at all + * ({@link EventIndex.isRoomIndexed}) — which is how a room looks before its checkpoint has been + * seeded, and is the case the checkpoint set alone cannot see. * - * Note that the absence of a checkpoint is not proof of completeness: a room also has no - * checkpoint if it never had a back-pagination token to crawl from, if its checkpoint was dropped - * because the server rejected the request, or before the initial checkpoints have been seeded. So - * this signal under-warns rather than over-warns. + * The second question is only asked while the crawler still has work outstanding, for two reasons. + * It is what the warning claims ("your search index is still being built"), and the index has no + * event for its contents changing — `changedCheckpoint` fires only on checkpoint transitions, and + * an idle crawler is silent — so a warning raised once the crawler has drained would never be + * re-evaluated and would stick. * - * The index emits `changedCheckpoint` on each checkpoint transition, but the payload carries only - * the globally-current room, so we re-read the checkpoint set on each event rather than trust it. + * Neither signal proves completeness: `isRoomIndexed` reports only that the index holds *some* + * events for a room, not all of them, and a room has no checkpoint if it never had a + * back-pagination token to crawl from or if its checkpoint was dropped because the server rejected + * the request. So this under-warns rather than over-warns. + * + * An all-rooms search cannot ask the per-room question, so it uses the checkpoint set alone. + * + * The `changedCheckpoint` payload carries only the globally-current room and so cannot answer a + * per-room question: we re-read the index on each event rather than trust it. * * @param index The event index to observe, or `null` if there is no index. * @param scope The scope of the search, if this warning is being rendered for one. * @param roomId The room being searched, or `undefined` when searching all rooms. - * @returns `true` while history relevant to the search is still being crawled, `false` otherwise. + * @returns `true` while the index is known to be missing history for the search, `false` otherwise. */ -function useIsCrawlInProgress(index: EventIndex | null, scope?: SearchScope, roomId?: string): boolean { - const readCrawlInProgress = useCallback((): boolean => { - if (!index) return false; +function useIsIndexIncomplete(index: EventIndex | null, scope?: SearchScope, roomId?: string): boolean { + const readCheckpoints = useCallback((): { relevant: boolean; anyOutstanding: boolean } => { + if (!index) return { relevant: false, anyOutstanding: false }; const { crawlingRooms } = index.crawlingRooms(); // Fall back to the global check when we don't know which room is being searched: the room // id may still be undefined while a room alias is being resolved. - if (scope !== SearchScope.Room || roomId === undefined) return crawlingRooms.size > 0; - return crawlingRooms.has(roomId); + const roomScoped = scope === SearchScope.Room && roomId !== undefined; + return { + relevant: roomScoped ? crawlingRooms.has(roomId) : crawlingRooms.size > 0, + anyOutstanding: crawlingRooms.size > 0, + }; }, [index, scope, roomId]); - const [crawlInProgress, setCrawlInProgress] = useState(readCrawlInProgress); + // The checkpoint half of the answer is known synchronously, so seed from it rather than + // rendering an unwarned search for a room we already know is being crawled. + const [incomplete, setIncomplete] = useState(() => readCheckpoints().relevant); useEffect(() => { if (!index) { - setCrawlInProgress(false); + setIncomplete(false); return; } + // Guards against a slow isRoomIndexed() response overwriting a newer one, or landing after + // the scope changed or the component unmounted. + let generation = 0; + + // Answer this scope and room from the checkpoint set up front, so that the previous + // search's result is not left on screen while the first lookup below is in flight. Doing + // this per effect run rather than per event matters: a checkpoint change is not a new + // question, and resetting on one would blink an already-earned warning off and on again. + setIncomplete(readCheckpoints().relevant); + + const update = async (): Promise => { + const current = ++generation; + const { relevant, anyOutstanding } = readCheckpoints(); + + if (relevant) { + setIncomplete(true); + return; + } + if (!anyOutstanding || scope !== SearchScope.Room || roomId === undefined) { + setIncomplete(false); + return; + } + + // Nothing is queued for this room yet, but the index may hold nothing for it at all. + // `undefined` means there is no index manager to ask, which is not evidence either way. + const indexed = await index.isRoomIndexed(roomId); + if (current === generation) setIncomplete(indexed === false); + }; + const onChangedCheckpoint = (): void => { - setCrawlInProgress(readCrawlInProgress()); + void update(); }; // Re-sync in case the crawl state changed between the initial render and the subscription. @@ -83,23 +125,24 @@ function useIsCrawlInProgress(index: EventIndex | null, scope?: SearchScope, roo index.on("changedCheckpoint", onChangedCheckpoint); return () => { + generation++; index.removeListener("changedCheckpoint", onChangedCheckpoint); }; - }, [index, readCrawlInProgress]); + }, [index, scope, roomId, readCheckpoints]); - return crawlInProgress; + return incomplete; } export default function SearchWarning({ isRoomEncrypted, kind, showLogo = true, scope, roomId }: IProps): JSX.Element { const eventIndex = EventIndexPeg.get(); - const crawlInProgress = useIsCrawlInProgress(eventIndex, scope, roomId); + const indexIncomplete = useIsIndexIncomplete(eventIndex, scope, roomId); if (!isRoomEncrypted) return <>; if (eventIndex) { - // The index exists but still has history to crawl for this search, so it may silently - // return partial results (#32253). Warn the user. - if (crawlInProgress && kind === WarningKind.Search) { + // The index is still missing history for this search, so it may silently return partial + // results (#32253). Warn the user. + if (indexIncomplete && kind === WarningKind.Search) { // This warning appears dynamically while a search panel is already open (the crawler // finishes draining mid-session), so mark it as a polite live region for screen readers. return ( diff --git a/apps/web/src/indexing/EventIndex.ts b/apps/web/src/indexing/EventIndex.ts index 2e3539fc5aa..8c81523447e 100644 --- a/apps/web/src/indexing/EventIndex.ts +++ b/apps/web/src/indexing/EventIndex.ts @@ -992,7 +992,10 @@ export default class EventIndex extends EventEmitter { } public crawlingRooms(): { - /** The rooms that we are currently crawling. */ + /** + * The rooms with an outstanding crawler checkpoint: the one being crawled right now, and + * those still queued behind it. + */ crawlingRooms: Set; /** All the encrypted rooms known by the MatrixClient. */ diff --git a/apps/web/test/unit-tests/components/views/elements/SearchWarning-test.tsx b/apps/web/test/unit-tests/components/views/elements/SearchWarning-test.tsx index 871a97aa99e..6722bd0dc8a 100644 --- a/apps/web/test/unit-tests/components/views/elements/SearchWarning-test.tsx +++ b/apps/web/test/unit-tests/components/views/elements/SearchWarning-test.tsx @@ -1,4 +1,5 @@ /* +Copyright 2026 hayaksi1 Copyright 2024 New Vector Ltd. Copyright 2024 The Matrix.org Foundation C.I.C. @@ -22,18 +23,30 @@ const PARTIAL_WARNING = "Results may be incomplete because your search index is /** * A minimal fake EventIndex exposing only the surface that SearchWarning consumes: - * `crawlingRooms()` (the rooms with outstanding crawler checkpoints) and the `changedCheckpoint` - * emitter. + * `crawlingRooms()` (the rooms with outstanding crawler checkpoints), `isRoomIndexed()` (whether + * the index holds any events for a room) and the `changedCheckpoint` emitter. */ class FakeEventIndex { private listeners = new Map void>>(); - public constructor(private crawling: string[] = []) {} + /** + * @param crawling Rooms with an outstanding crawler checkpoint. + * @param indexed Rooms the index holds events for. Defaults to `undefined`, meaning + * `isRoomIndexed` resolves `undefined`, as it does when there is no index manager. + */ + public constructor( + private crawling: string[] = [], + private indexed?: string[], + ) {} public crawlingRooms(): { crawlingRooms: Set; totalRooms: Set } { return { crawlingRooms: new Set(this.crawling), totalRooms: new Set(this.crawling) }; } + public async isRoomIndexed(roomId: string): Promise { + return this.indexed === undefined ? undefined : this.indexed.includes(roomId); + } + public on(event: string, listener: (...args: any[]) => void): void { if (!this.listeners.has(event)) this.listeners.set(event, new Set()); this.listeners.get(event)!.add(listener); @@ -97,8 +110,13 @@ describe("", () => { EventIndexPeg.index = index as unknown as EventIndex; }; - it("warns a room-scoped search while the searched room is still being crawled", () => { - setIndex(new FakeEventIndex([SEARCHED_ROOM])); + /** Let the pending isRoomIndexed() lookup settle and React re-render off the back of it. */ + const settle = async (): Promise => { + await act(async () => {}); + }; + + it("warns a room-scoped search while the searched room is still being crawled", async () => { + setIndex(new FakeEventIndex([SEARCHED_ROOM], [])); const { queryByText, queryByRole } = render( ", () => { roomId={SEARCHED_ROOM} />, ); + await settle(); expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument(); // The notice appears dynamically while the panel is open, so it must be a live region (#32253). expect(queryByRole("status")).toBeInTheDocument(); }); - it("does not warn a room-scoped search when only an unrelated room is still being crawled", () => { - setIndex(new FakeEventIndex([OTHER_ROOM])); + it("does not warn a room-scoped search when only an unrelated room is still being crawled", async () => { + setIndex(new FakeEventIndex([OTHER_ROOM], [SEARCHED_ROOM])); + + const { container } = render( + , + ); + await settle(); + + expect(container).toBeEmptyDOMElement(); + }); + + it("warns a room-scoped search when the index holds no events for the searched room", async () => { + // Nothing is queued for the searched room, but it has never been indexed: this is how a + // room looks before its checkpoint has been seeded, and the crawl set alone cannot see + // it. The crawler is still working through another room, so indexing is still under way. + setIndex(new FakeEventIndex([OTHER_ROOM], [])); + + const { queryByText } = render( + , + ); + await settle(); + + expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument(); + }); + + it("does not warn about an unindexed room once the crawler has drained", async () => { + // The index holds nothing for the room, but the crawler has no work left, so nothing + // more is coming: the warning would never clear itself, and its claim that the index is + // "still being built" would be untrue. + setIndex(new FakeEventIndex([], [])); const { container } = render( ", () => { roomId={SEARCHED_ROOM} />, ); + await settle(); expect(container).toBeEmptyDOMElement(); }); - it("warns an all-rooms search while any room is still being crawled", () => { - setIndex(new FakeEventIndex([OTHER_ROOM])); + it("keeps an earned warning up while an unrelated checkpoint change is re-checked", async () => { + // The warning here is earned asynchronously: the room is not queued, but the index + // holds nothing for it. An unrelated crawler transition must not blink it off while + // the fresh lookup is in flight. + const index = new FakeEventIndex([OTHER_ROOM], []); + setIndex(index); + + const { queryByText } = render( + , + ); + await settle(); + expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument(); + + // The crawler moves on to a third room; nothing about the searched room changed. + act(() => { + index.emitChangedCheckpoint([OTHER_ROOM, "!third:example.org"]); + }); + + // Asserted before the lookup settles: the warning must not have been dropped. + expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument(); + + await settle(); + expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument(); + }); + + it("treats an unanswerable isRoomIndexed as no evidence for a room-scoped search", async () => { + setIndex(new FakeEventIndex([OTHER_ROOM], undefined)); + + const { container } = render( + , + ); + await settle(); + + expect(container).toBeEmptyDOMElement(); + }); + + it("does not carry a warning from a previously searched room across a scope change", async () => { + // The crawler is working through OTHER_ROOM, so an all-rooms search warns. Switching to + // a room-scoped search of a fully-indexed room must not leave that warning on screen + // while the isRoomIndexed lookup for the new room is in flight. + setIndex(new FakeEventIndex([OTHER_ROOM], [SEARCHED_ROOM])); + + const { queryByText, rerender } = render( + , + ); + await settle(); + expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument(); + + rerender( + , + ); + + // Asserted before settling: the stale answer must be gone as soon as the effect for + // the new scope has run, rather than lingering for the isRoomIndexed round-trip. + expect(queryByText(PARTIAL_WARNING)).not.toBeInTheDocument(); + }); + + it("warns an all-rooms search while any room is still being crawled", async () => { + setIndex(new FakeEventIndex([OTHER_ROOM], [])); const { queryByText } = render( , ); + await settle(); expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument(); }); - it("falls back to the global check when the searched room is not yet known", () => { + it("falls back to the global check when the searched room is not yet known", async () => { // RoomView can render before a room alias has resolved to an id. - setIndex(new FakeEventIndex([OTHER_ROOM])); + setIndex(new FakeEventIndex([OTHER_ROOM], [])); const { queryByText } = render( ", () => { roomId={undefined} />, ); + await settle(); expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument(); }); - it("renders nothing once the index has finished crawling", () => { - setIndex(new FakeEventIndex([])); + it("renders nothing once the index has finished crawling", async () => { + setIndex(new FakeEventIndex([], [])); const { container } = render( , ); + await settle(); expect(container).toBeEmptyDOMElement(); }); - it("does not warn for the Files kind even while crawling", () => { - setIndex(new FakeEventIndex([SEARCHED_ROOM])); + it("does not warn for the Files kind even while crawling", async () => { + setIndex(new FakeEventIndex([SEARCHED_ROOM], [])); const { container } = render(); + await settle(); expect(container).toBeEmptyDOMElement(); }); - it("clears the warning when the searched room's checkpoint drains", () => { - const index = new FakeEventIndex([SEARCHED_ROOM]); + it("clears the warning when the searched room's checkpoint drains", async () => { + const index = new FakeEventIndex([SEARCHED_ROOM], [SEARCHED_ROOM]); setIndex(index); const { queryByText } = render( @@ -185,18 +319,19 @@ describe("", () => { roomId={SEARCHED_ROOM} />, ); + await settle(); expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument(); - act(() => { + await act(async () => { index.emitChangedCheckpoint([]); }); expect(queryByText(PARTIAL_WARNING)).not.toBeInTheDocument(); }); - it("keeps warning when the crawler moves to another room but the searched room is still queued", () => { - const index = new FakeEventIndex([SEARCHED_ROOM, OTHER_ROOM]); + it("keeps warning when the crawler moves to another room but the searched room is still queued", async () => { + const index = new FakeEventIndex([SEARCHED_ROOM, OTHER_ROOM], [SEARCHED_ROOM]); setIndex(index); const { queryByText } = render( @@ -207,18 +342,19 @@ describe("", () => { roomId={SEARCHED_ROOM} />, ); + await settle(); // The event payload names a different room; the searched room is still outstanding, so // the warning must survive. This fails if the handler trusts the payload. - act(() => { + await act(async () => { index.emitChangedCheckpoint([SEARCHED_ROOM], { roomId: OTHER_ROOM } as Room); }); expect(queryByText(PARTIAL_WARNING)).toBeInTheDocument(); }); - it("renders nothing when the room is not encrypted even while crawling", () => { - setIndex(new FakeEventIndex([SEARCHED_ROOM])); + it("renders nothing when the room is not encrypted even while crawling", async () => { + setIndex(new FakeEventIndex([SEARCHED_ROOM], [])); const { container } = render(