Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 61 additions & 22 deletions src/components/DocumentViewer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,12 @@ import { NativePdfEmbed, PdfCanvasViewer } from "@/components/document-viewer/pd
import { NonPdfSourcePreview } from "@/components/document-viewer/non-pdf-source-preview";
import { clearCachedSignedUrl, getCachedSignedUrl, setCachedSignedUrl } from "@/lib/signed-url-cache";
import { readLocalProjectIdentity, unsafeLocalProjectMessage } from "@/lib/local-project-identity";
import { documentPageHref } from "@/lib/document-viewer-navigation";
import {
documentLoadKey,
documentPageHref,
isFullDocumentReload,
nextLoadedDocumentKey,
} from "@/lib/document-viewer-navigation";
import { formatClinicalDate } from "@/lib/source-metadata";
import { partitionViewerImages } from "@/lib/image-filtering";
import { isLocalNoAuthMode } from "@/lib/client-env";
Expand Down Expand Up @@ -1600,18 +1605,25 @@ export function DocumentViewer({
const [shellScrollContainer, setShellScrollContainer] = useState<HTMLElement | null>(null);
useEffect(() => {
let cancelled = false;
let observer: MutationObserver | null = null;
// #main-content is the app-shell scroll container: it mounts once (usually
// before this effect runs) and persists for the viewer's lifetime. Resolve it
// synchronously, and only fall back to observing the DOM until it appears —
// then disconnect, rather than reacting to every app-wide mutation forever.
const sync = () => {
if (cancelled) return;
const main = window.document.getElementById("main-content");
if (!main) return;
setShellScrollContainer((current) => (current === main ? current : main));
observer?.disconnect();
observer = null;
};
const frame = window.requestAnimationFrame(sync);
const observer = new MutationObserver(sync);
observer = new MutationObserver(sync);
observer.observe(window.document.body, { childList: true, subtree: true });
sync();
return () => {
cancelled = true;
window.cancelAnimationFrame(frame);
observer.disconnect();
observer?.disconnect();
};
}, []);
const scrollHidden = useHideOnScroll(shellScrollContainer ? { scrollContainer: shellScrollContainer } : {});
Expand Down Expand Up @@ -1823,6 +1835,12 @@ export function DocumentViewer({

useEffect(() => () => refreshControllerRef.current?.abort(), []);

// Distinguishes a full document (re)load — a new documentId or an explicit
// retry (previewAttempt) — from page/chunk navigation on the already-loaded
// document. Navigation only re-windows the detail; a full load also resets the
// preview and re-issues signed URLs.
const loadedKeyRef = useRef<string | null>(null);

useEffect(() => {
if (!canViewSourceDocuments && authStatus === "loading") {
return () => undefined;
Expand All @@ -1833,8 +1851,12 @@ export function DocumentViewer({

const controller = new AbortController();
const authRequest = registerAuthRequest(controller);
const loadKey = documentLoadKey(documentId, previewAttempt);
const isFullReload = isFullDocumentReload(loadedKeyRef.current, loadKey);
const reset = window.setTimeout(() => {
if (!controller.signal.aborted) {
// Skip the reset on navigation so the mounted PDF and current content stay
// visible (no loading flash) while the new page window loads in the background.
if (!controller.signal.aborted && isFullReload) {
setLoadingDocument(true);
setViewerError(null);
setPreviewError(null);
Expand Down Expand Up @@ -1871,27 +1893,38 @@ export function DocumentViewer({
if (!response.ok) throw new Error(payload.error || "Document details could not be loaded.");
return payload;
});
const signedUrlPair = fetchSignedUrlPair(signedUrlEndpoint, downloadSignedUrlEndpoint, {
signal: controller.signal,
headers: clientDemoMode ? undefined : authorizationHeader,
onUnauthorized: markSessionExpired,
useCache: true,
});
// Navigation keeps the current signed URLs; only a full load re-issues them.
const signedUrlPair = isFullReload
? fetchSignedUrlPair(signedUrlEndpoint, downloadSignedUrlEndpoint, {
signal: controller.signal,
headers: clientDemoMode ? undefined : authorizationHeader,
onUnauthorized: markSessionExpired,
useCache: true,
})
: Promise.resolve(null);

return Promise.all([Promise.allSettled([detailRequest]), signedUrlPair]);
})
.then(([[detailResult], [signedUrlResult, signedDownloadUrlResult]]) => {
.then(([[detailResult], signedUrlPair]) => {
if (controller.signal.aborted || !isAuthEpochCurrent(authRequest.epoch)) return;

if (detailResult.status === "fulfilled") {
const detailLoaded = detailResult.status === "fulfilled";
// Advance the loaded key only on a successful detail load. A failed full
// load must stay "not loaded" so the next page/chunk navigation is still
// treated as a full reload — re-fetching signed URLs and refreshing the
// error — rather than a cheap navigation that skips that recovery.
loadedKeyRef.current = nextLoadedDocumentKey(loadedKeyRef.current, loadKey, detailLoaded);

if (detailLoaded) {
const detail = detailResult.value;
setDocument(detail.document ?? null);
setPages(detail.pages ?? []);
setImages(detail.images ?? []);
setTableFacts(detail.tableFacts ?? []);
setChunks(detail.chunks ?? []);
setIndexHealth(detail.indexHealth ?? null);
} else {
} else if (isFullReload) {
// Only clear the viewer on a full load; a transient detail failure
// during navigation keeps the current content on screen.
setDocument(null);
setPages([]);
setImages([]);
Expand All @@ -1911,11 +1944,14 @@ export function DocumentViewer({
}
}

applySignedUrlResults(signedUrlResult, signedDownloadUrlResult, signedUrlEndpoint, downloadSignedUrlEndpoint);
if (signedUrlPair) {
const [signedUrlResult, signedDownloadUrlResult] = signedUrlPair;
applySignedUrlResults(signedUrlResult, signedDownloadUrlResult, signedUrlEndpoint, downloadSignedUrlEndpoint);
}
})
.catch((error) => {
if (controller.signal.aborted || !isAuthEpochCurrent(authRequest.epoch)) return;
setViewerError(error instanceof Error ? error.message : "Document could not be loaded.");
if (isFullReload) setViewerError(error instanceof Error ? error.message : "Document could not be loaded.");
})
.finally(() => {
if (!controller.signal.aborted) setLoadingDocument(false);
Expand Down Expand Up @@ -2191,20 +2227,23 @@ export function DocumentViewer({
// The PDF signed URL has a 10-min TTL and pdf.js holds a dead reference once it
// expires. When the canvas reports an expiry, drop the cached URLs and re-run
// the fetch pipeline to mint fresh ones (bounded so a broken URL can't loop).
const handleSignedUrlExpired = () => {
// Stable identity (useCallback) so the memoised PdfCanvasViewer isn't re-rendered
// — and its page re-rastered — every time an unrelated parent state (source-search
// keystroke, composer focus, online/offline) changes.
const handleSignedUrlExpired = useCallback(() => {
if (signedUrlRefreshCountRef.current >= 2) return;
signedUrlRefreshCountRef.current += 1;
const signedUrlEndpoint = `/api/documents/${documentId}/signed-url`;
clearCachedSignedUrl(signedUrlEndpoint);
clearCachedSignedUrl(`${signedUrlEndpoint}?download=true`);
refreshSignedUrls();
};
}, [documentId, refreshSignedUrls]);
// A successful reload means the refreshed URL was accepted, so the recovery
// worked — restore the budget for the next (unrelated) TTL expiry. A broken
// URL never loads, so it never resets, and the cap still stops its loop.
const handlePdfLoadSuccess = () => {
const handlePdfLoadSuccess = useCallback(() => {
signedUrlRefreshCountRef.current = 0;
};
}, []);
const handleDocumentRenamed = (updatedDocument: ClinicalDocument) => {
setDocument((current) => (current?.id === updatedDocument.id ? { ...current, ...updatedDocument } : current));
};
Expand Down
6 changes: 3 additions & 3 deletions src/components/document-viewer/non-pdf-source-preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

/* eslint-disable @next/next/no-img-element */

import { useState } from "react";
import { memo, useState } from "react";
import { CircleAlert, Download, ExternalLink, FileText } from "lucide-react";

import { cn, floatingControl } from "@/components/ui-primitives";
Expand All @@ -21,7 +21,7 @@ const placeholderSurface =
* - other (DOCX/XLSX/…) → an honest "download to view" affordance,
* - no signed URL yet → the original placeholder.
*/
export function NonPdfSourcePreview({
export const NonPdfSourcePreview = memo(function NonPdfSourcePreview({
fileType,
title,
signedUrl,
Expand Down Expand Up @@ -87,7 +87,7 @@ export function NonPdfSourcePreview({
</div>
</div>
);
}
});

/**
* Inline image with a failure fallback. The source is a direct signed URL owned
Expand Down
22 changes: 17 additions & 5 deletions src/components/document-viewer/pdf-canvas-viewer.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"use client";

import { type KeyboardEvent as ReactKeyboardEvent, useCallback, useEffect, useRef, useState } from "react";
import { type KeyboardEvent as ReactKeyboardEvent, memo, useCallback, useEffect, useRef, useState } from "react";
import {
ChevronLeft,
ChevronRight,
Expand Down Expand Up @@ -44,7 +44,11 @@ function isLikelyExpiredUrl(error: unknown): boolean {
);
}

export function PdfCanvasViewer({
// Memoised: this is the heaviest subtree in the document view (it holds the
// pdf.js document and re-rasters the canvas). With stable props from the parent
// it skips re-render when unrelated parent state (search, composer, connectivity)
// changes, so a keystroke elsewhere never re-rasterises the page.
export const PdfCanvasViewer = memo(function PdfCanvasViewer({
url,
title,
initialPage,
Expand Down Expand Up @@ -553,14 +557,22 @@ export function PdfCanvasViewer({
</div>
</div>
);
}
});

function nativePdfEmbedUrl(url: string, initialPage: number) {
const page = Math.max(1, Math.trunc(initialPage || 1));
return `${url.split("#")[0]}#page=${page}`;
}

export function NativePdfEmbed({ url, title, initialPage }: { url: string; title: string; initialPage: number }) {
export const NativePdfEmbed = memo(function NativePdfEmbed({
url,
title,
initialPage,
}: {
url: string;
title: string;
initialPage: number;
}) {
return (
<div className="overflow-hidden rounded-lg border border-[color:var(--border)] bg-[color:var(--surface-raised)] shadow-[var(--shadow-tight)]">
<iframe
Expand All @@ -572,4 +584,4 @@ export function NativePdfEmbed({ url, title, initialPage }: { url: string; title
/>
</div>
);
}
});
36 changes: 36 additions & 0 deletions src/lib/document-viewer-navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,39 @@ export function documentPageHref(documentId: string, page: number) {
const params = new URLSearchParams({ page: String(normalizedPage) });
return `/documents/${encodeURIComponent(documentId)}?${params.toString()}#pdf-preview-section`;
}

/**
* Identity of a full document load. A new `documentId` or an explicit retry
* (`previewAttempt`) starts a fresh full load; page/chunk navigation within the
* same document keeps the same key.
*/
export function documentLoadKey(documentId: string, previewAttempt: number): string {
return `${documentId}::${previewAttempt}`;
}

/**
* Whether the pending load is a *full* (re)load rather than navigation on an
* already-loaded document. A full load resets the viewer, re-issues signed URLs
* and surfaces load errors; navigation only re-windows the detail in place.
*
* The remembered key is advanced only after a *successful* detail load (see
* {@link nextLoadedDocumentKey}), so a failed full load stays "not loaded" and
* the next run retries it as a full load rather than a cheap navigation.
*/
export function isFullDocumentReload(loadedKey: string | null, loadKey: string): boolean {
return loadedKey !== loadKey;
}

/**
* The load key to remember once a load settles. Only a successful detail load
* marks the document loaded; a failed load keeps the previous key so the next
* navigation is still treated as a full reload (re-fetching signed URLs and
* updating the viewer error) instead of skipping that recovery work.
*/
export function nextLoadedDocumentKey(
previousKey: string | null,
loadKey: string,
detailLoaded: boolean,
): string | null {
return detailLoaded ? loadKey : previousKey;
}
39 changes: 38 additions & 1 deletion tests/document-viewer-navigation-source.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { describe, expect, it } from "vitest";

import { documentPageHref } from "../src/lib/document-viewer-navigation";
import {
documentLoadKey,
documentPageHref,
isFullDocumentReload,
nextLoadedDocumentKey,
} from "../src/lib/document-viewer-navigation";

describe("document viewer useful-page navigation", () => {
it("creates a destination-page URL without carrying an unrelated citation chunk", () => {
Expand Down Expand Up @@ -30,3 +35,35 @@ describe("document viewer useful-page navigation", () => {
expect(documentPageHref("doc id&value", 2)).toBe("/documents/doc%20id%26value?page=2#pdf-preview-section");
});
});

describe("document viewer full-reload vs navigation gating", () => {
it("keys a load by documentId + previewAttempt", () => {
expect(documentLoadKey("doc-1", 0)).toBe("doc-1::0");
expect(documentLoadKey("doc-1", 2)).toBe("doc-1::2");
});

it("treats a first load, a new document, and an explicit retry as full reloads", () => {
// No prior load → full reload.
expect(isFullDocumentReload(null, documentLoadKey("doc-1", 0))).toBe(true);
// Same document + attempt → navigation, not a full reload.
expect(isFullDocumentReload("doc-1::0", documentLoadKey("doc-1", 0))).toBe(false);
// Different document → full reload.
expect(isFullDocumentReload("doc-1::0", documentLoadKey("doc-2", 0))).toBe(true);
// Retry (previewAttempt bump) on the same document → full reload.
expect(isFullDocumentReload("doc-1::0", documentLoadKey("doc-1", 1))).toBe(true);
});

it("advances the loaded key only after a successful detail load", () => {
const loadKey = documentLoadKey("doc-1", 0);

// Success stamps the key so later navigation is treated as navigation.
expect(nextLoadedDocumentKey(null, loadKey, true)).toBe("doc-1::0");
// Regression guard: a failed full load must NOT stamp the key, so the next
// page/chunk navigation still evaluates as a full reload and re-fetches
// signed URLs / refreshes the error instead of skipping that recovery.
expect(nextLoadedDocumentKey(null, loadKey, false)).toBeNull();
expect(isFullDocumentReload(nextLoadedDocumentKey(null, loadKey, false), loadKey)).toBe(true);
// A failed navigation keeps the previously loaded key intact.
expect(nextLoadedDocumentKey("doc-1::0", loadKey, false)).toBe("doc-1::0");
});
});
Loading