From 810a70acb7862ed224e7cb6bf0ada2744dab0ebf Mon Sep 17 00:00:00 2001 From: chriscrosstalk <49691103+chriscrosstalk@users.noreply.github.com> Date: Mon, 20 Apr 2026 16:05:27 -0700 Subject: [PATCH] fix(ZIM): accumulate across Kiwix pages to prevent empty Content Explorer (#746) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When many ZIMs are already installed locally, a single Kiwix catalog page (12 items) could return 12 already-installed items, which zim_service would fully filter out client-side. The endpoint returned items: [] with has_more: true, and the frontend's infinite-scroll guard (flatData.length > 0) blocked fetchNextPage — leaving the user with "No records found" despite plenty of uninstalled ZIMs available. Backend now accumulates across up to 5 Kiwix fetches (60 items each) until it has enough post-filter results to return, dedupes by entry id, advances currentStart by actual entries returned (not requested), and returns a next_start cursor. The frontend consumes that cursor instead of computing Kiwix offsets locally, and the flatData.length > 0 guard is removed so the existing on-mount effect drives bounded auto-fetch when a short page lands. The pre-existing has_more off-by-one (compared totalResults against the input start rather than the post-fetch position) is fixed implicitly. Diagnosis credit: @johno10661. Closes #731 Co-authored-by: Claude Opus 4.7 (1M context) --- admin/app/services/zim_service.ts | 155 ++++++++++-------- .../pages/settings/zim/remote-explorer.tsx | 27 ++- admin/types/zim.ts | 1 + 3 files changed, 100 insertions(+), 83 deletions(-) diff --git a/admin/app/services/zim_service.ts b/admin/app/services/zim_service.ts index bee4309..c6f427c 100644 --- a/admin/app/services/zim_service.ts +++ b/admin/app/services/zim_service.ts @@ -57,84 +57,105 @@ export class ZimService { query?: string }): Promise { const LIBRARY_BASE_URL = 'https://browse.library.kiwix.org/catalog/v2/entries' + // Kiwix returns pages of content unaware of what the user has installed locally. When + // the installed set is large, a single 12-item Kiwix page can come back with everything + // already installed → 0 post-filter items → frontend deadlock (#731). Accumulate across + // upstream pages so we return a useful batch. Bounded by MAX_KIWIX_FETCHES so a heavily + // saturated install doesn't hang a single request; the frontend scroll loop + auto-fetch + // effect handle continuation. + const KIWIX_PAGE_SIZE = 60 + const MAX_KIWIX_FETCHES = 5 - const res = await axios.get(LIBRARY_BASE_URL, { - params: { - start: start, - count: count, - lang: 'eng', - ...(query ? { q: query } : {}), - }, - responseType: 'text', - }) - - const data = res.data const parser = new XMLParser({ ignoreAttributes: false, attributeNamePrefix: '', textNodeName: '#text', }) - const result = parser.parse(data) - if (!isRawListRemoteZimFilesResponse(result)) { - throw new Error('Invalid response format from remote library') - } - - const entries = result.feed.entry - ? Array.isArray(result.feed.entry) - ? result.feed.entry - : [result.feed.entry] - : [] - - const filtered = entries.filter((entry: any) => { - return isRawRemoteZimFileEntry(entry) - }) - - const mapped: (RemoteZimFileEntry | null)[] = filtered.map((entry: RawRemoteZimFileEntry) => { - const downloadLink = entry.link.find((link: any) => { - return ( - typeof link === 'object' && - 'rel' in link && - 'length' in link && - 'href' in link && - 'type' in link && - link.type === 'application/x-zim' - ) - }) - - if (!downloadLink) { - return null - } - - // downloadLink['href'] will end with .meta4, we need to remove that to get the actual download URL - const download_url = downloadLink['href'].substring(0, downloadLink['href'].length - 6) - const file_name = download_url.split('/').pop() || `${entry.title}.zim` - const sizeBytes = parseInt(downloadLink['length'], 10) - - return { - id: entry.id, - title: entry.title, - updated: entry.updated, - summary: entry.summary, - size_bytes: sizeBytes || 0, - download_url: download_url, - author: entry.author.name, - file_name: file_name, - } - }) - - // Filter out any null entries (those without a valid download link) - // or files that already exist in the local storage + // Snapshot locally-installed files once — the filesystem won't change mid-request. const existing = await this.list() const existingKeys = new Set(existing.files.map((file) => file.name)) - const withoutExisting = mapped.filter( - (entry): entry is RemoteZimFileEntry => entry !== null && !existingKeys.has(entry.file_name) - ) + + const accumulated: RemoteZimFileEntry[] = [] + const seenIds = new Set() + let currentStart = start + let totalResults = 0 + + for (let i = 0; i < MAX_KIWIX_FETCHES; i++) { + const res = await axios.get(LIBRARY_BASE_URL, { + params: { + start: currentStart, + count: KIWIX_PAGE_SIZE, + lang: 'eng', + ...(query ? { q: query } : {}), + }, + responseType: 'text', + }) + + const parsed = parser.parse(res.data) + if (!isRawListRemoteZimFilesResponse(parsed)) { + throw new Error('Invalid response format from remote library') + } + totalResults = parsed.feed.totalResults + + const rawEntries = parsed.feed.entry + ? Array.isArray(parsed.feed.entry) + ? parsed.feed.entry + : [parsed.feed.entry] + : [] + + // Empty upstream response — bail even if totalResults suggests more (transient Kiwix + // hiccup or totalResults drift between pages). Prevents a pointless spin. + if (rawEntries.length === 0) break + + // Advance by actual returned count, not requested count. Short pages at the tail + // would otherwise cause us to skip entries on the next fetch. + currentStart += rawEntries.length + + for (const raw of rawEntries) { + if (!isRawRemoteZimFileEntry(raw)) continue + const entry = raw as RawRemoteZimFileEntry + + const downloadLink = entry.link.find( + (link: any) => + typeof link === 'object' && + 'rel' in link && + 'length' in link && + 'href' in link && + 'type' in link && + link.type === 'application/x-zim' + ) + if (!downloadLink) continue + + // downloadLink['href'] ends with .meta4; strip that to get the actual .zim URL. + const download_url = downloadLink['href'].substring(0, downloadLink['href'].length - 6) + const file_name = download_url.split('/').pop() || `${entry.title}.zim` + if (existingKeys.has(file_name)) continue + if (seenIds.has(entry.id)) continue + seenIds.add(entry.id) + + const sizeBytes = parseInt(downloadLink['length'], 10) + accumulated.push({ + id: entry.id, + title: entry.title, + updated: entry.updated, + summary: entry.summary, + size_bytes: sizeBytes || 0, + download_url, + author: entry.author.name, + file_name, + }) + } + + if (accumulated.length >= count) break + if (currentStart >= totalResults) break + } return { - items: withoutExisting, - has_more: result.feed.totalResults > start, - total_count: result.feed.totalResults, + items: accumulated, + has_more: currentStart < totalResults, + total_count: totalResults, + next_start: currentStart, } } diff --git a/admin/inertia/pages/settings/zim/remote-explorer.tsx b/admin/inertia/pages/settings/zim/remote-explorer.tsx index b515a50..9d1ca3e 100644 --- a/admin/inertia/pages/settings/zim/remote-explorer.tsx +++ b/admin/inertia/pages/settings/zim/remote-explorer.tsx @@ -83,8 +83,10 @@ export default function ZimRemoteExplorer() { useInfiniteQuery({ queryKey: ['remote-zim-files', query], queryFn: async ({ pageParam = 0 }) => { - const pageParsed = parseInt((pageParam as number).toString(), 10) - const start = isNaN(pageParsed) ? 0 : pageParsed * 12 + // pageParam is an opaque Kiwix offset returned by the backend as `next_start`. + // The backend accumulates across multiple upstream pages when needed (#731), so the + // frontend can't derive the next offset from a 12-item page assumption. + const start = typeof pageParam === 'number' ? pageParam : 0 const res = await api.listRemoteZimFiles({ start, count: 12, query: query || undefined }) if (!res) { throw new Error('Failed to fetch remote ZIM files.') @@ -92,12 +94,7 @@ export default function ZimRemoteExplorer() { return res.data }, initialPageParam: 0, - getNextPageParam: (_lastPage, pages) => { - if (!_lastPage.has_more) { - return undefined // No more pages to fetch - } - return pages.length - }, + getNextPageParam: (lastPage) => (lastPage.has_more ? lastPage.next_start : undefined), refetchOnWindowFocus: false, placeholderData: keepPreviousData, }) @@ -119,18 +116,16 @@ export default function ZimRemoteExplorer() { (parentRef?: HTMLDivElement | null) => { if (parentRef) { const { scrollHeight, scrollTop, clientHeight } = parentRef - //once the user has scrolled within 200px of the bottom of the table, fetch more data if we can - if ( - scrollHeight - scrollTop - clientHeight < 200 && - !isFetching && - hasMore && - flatData.length > 0 - ) { + // Fetch more when near the bottom. The `flatData.length > 0` guard that used to be + // here caused the #731 deadlock when a heavily-saturated install returned an empty + // page with has_more=true — removing it lets the existing on-mount/on-data effect + // below drive bounded auto-fetch until hasMore flips false. + if (scrollHeight - scrollTop - clientHeight < 200 && !isFetching && hasMore) { fetchNextPage() } } }, - [fetchNextPage, isFetching, hasMore, flatData.length] + [fetchNextPage, isFetching, hasMore] ) const virtualizer = useVirtualizer({ diff --git a/admin/types/zim.ts b/admin/types/zim.ts index cfd040a..96ed081 100644 --- a/admin/types/zim.ts +++ b/admin/types/zim.ts @@ -16,6 +16,7 @@ export type ListRemoteZimFilesResponse = { items: RemoteZimFileEntry[] has_more: boolean total_count: number + next_start: number } export type RawRemoteZimFileEntry = {