diff --git a/e2e/citation-dedupe.e2e.ts b/e2e/citation-dedupe.e2e.ts new file mode 100644 index 0000000..78de15e --- /dev/null +++ b/e2e/citation-dedupe.e2e.ts @@ -0,0 +1,89 @@ +import { expect, test } from "@playwright/test" + +test.setTimeout(60_000) + +test("keeps duplicate source labels clickable for separate documents", async ({ + context, + page, +}) => { + let firstSourceChunkRequests = 0 + let secondSourceChunkRequests = 0 + + await context.addCookies([ + { + name: "better-auth.session_token", + value: "playwright", + url: "http://localhost:3000", + }, + ]) + + await page.route("**/api/sources/source_first/chunks**", async (route) => { + firstSourceChunkRequests += 1 + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + chunks: [ + { + chunkId: "chunk_first", + documentId: "doc_first", + sectionPath: "Root", + type: "text", + content: "First report source content.", + sourceTitle: "report.pdf", + }, + ], + pagination: { + page: 1, + pageSize: 50, + total: 1, + totalPages: 1, + }, + }), + }) + }) + await page.route("**/api/sources/source_second/chunks**", async (route) => { + secondSourceChunkRequests += 1 + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + chunks: [ + { + chunkId: "chunk_second", + documentId: "doc_second", + sectionPath: "Root", + type: "text", + content: "Second report source content.", + sourceTitle: "report.pdf", + }, + ], + pagination: { + page: 1, + pageSize: 50, + total: 1, + totalPages: 1, + }, + }), + }) + }) + + await page.goto("/e2e/citation-dedupe") + + const chatPanel = page.getByTestId("desktop-chat-panel") + const duplicateSourceLinks = chatPanel.getByRole("button", { + name: "Open source report.pdf", + }) + await expect(duplicateSourceLinks).toHaveCount(2) + + const secondSourceRequestsBeforeClick = secondSourceChunkRequests + await duplicateSourceLinks.nth(1).click() + await expect(page.getByText("Second report source content.")).toBeVisible() + expect(secondSourceChunkRequests).toBeGreaterThan( + secondSourceRequestsBeforeClick, + ) + + await duplicateSourceLinks.first().click() + await expect(page.getByText("First report source content.")).toBeVisible() + expect(firstSourceChunkRequests).toBeGreaterThan(0) +}) diff --git a/playwright.config.ts b/playwright.config.ts index e59bfbf..77f738b 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -1,5 +1,8 @@ import { defineConfig, devices } from "@playwright/test" +const shouldUseExternalWebServer = + process.env.PLAYWRIGHT_EXTERNAL_WEB_SERVER === "1" + export default defineConfig({ testDir: "./e2e", testMatch: "**/*.e2e.ts", @@ -17,10 +20,14 @@ export default defineConfig({ use: { ...devices["Desktop Chrome"] }, }, ], - webServer: { - command: "pnpm dev", - url: "http://localhost:3000", - reuseExistingServer: true, - timeout: 60_000, - }, + ...(shouldUseExternalWebServer + ? {} + : { + webServer: { + command: "pnpm dev", + url: "http://localhost:3000", + reuseExistingServer: true, + timeout: 60_000, + }, + }), }) diff --git a/src/app/e2e/citation-dedupe/page.tsx b/src/app/e2e/citation-dedupe/page.tsx new file mode 100644 index 0000000..8304a0e --- /dev/null +++ b/src/app/e2e/citation-dedupe/page.tsx @@ -0,0 +1,83 @@ +import { notFound } from "next/navigation" + +import { WorkspaceShell } from "@/components/workspace-shell" +import type { ChatMessageView, ChatThreadView } from "@/domains/chat/types" +import type { SourceView } from "@/domains/sources/types" + +const duplicateTitleSources: SourceView[] = [ + { + id: "source_first", + title: "report.pdf", + mimeType: "application/pdf", + status: "ready", + documentId: "doc_first", + chunkCount: 1, + }, + { + id: "source_second", + title: "report.pdf", + mimeType: "application/pdf", + status: "ready", + documentId: "doc_second", + chunkCount: 1, + }, +] + +const chatThreads: ChatThreadView[] = [ + { + id: "thread_1", + title: "Duplicate source labels", + createdAt: "2026-05-17T00:00:00.000Z", + updatedAt: "2026-05-17T00:00:00.000Z", + }, +] + +const chatMessages: ChatMessageView[] = [ + { + id: "assistant_1", + role: "assistant", + content: "Both reports are relevant to the answer.", + citations: [ + { + chunkType: "text", + score: 0.91, + source: { + documentId: "doc_first", + sourceFileName: "report.pdf", + sectionPath: "Root", + }, + }, + { + chunkType: "text", + score: 0.89, + source: { + documentId: "doc_second", + sourceFileName: "report.pdf", + sectionPath: "Root", + }, + }, + ], + }, +] + +export default function CitationDedupeTestPage() { + if (process.env.NODE_ENV === "production") notFound() + + return ( + + ) +} diff --git a/src/app/page.test.ts b/src/app/page.test.ts index f3d9612..f7e3525 100644 --- a/src/app/page.test.ts +++ b/src/app/page.test.ts @@ -21,6 +21,7 @@ vi.mock("@/domains/workspace/initial-state", () => ({ })) import { HomeContent } from "./page" +import { makeWorkspaceInitialStateFailureFixture } from "@/test/workspace-initial-state-failure-fixture" describe("Home", () => { afterEach(() => { @@ -42,18 +43,16 @@ describe("Home", () => { }) it("logs a readable page-load failure before rethrowing", async () => { - mocks.loadWorkspaceShellInitialState.mockRejectedValue( - new Error("database connection refused"), - ) + const failure = makeWorkspaceInitialStateFailureFixture() - await expect(HomeContent()).rejects.toThrow( - "Workspace initial state failed: database connection refused", - ) + mocks.loadWorkspaceShellInitialState.mockRejectedValue(failure.error) + + await expect(HomeContent()).rejects.toThrow(failure.boundaryMessage) expect(mocks.logger.error).toHaveBeenCalledWith( "workspace: initial state failed", - expect.objectContaining({ - error: expect.stringContaining("database connection refused"), - }), + { + error: failure.rootCauseMessage, + }, ) }) }) diff --git a/src/app/page.tsx b/src/app/page.tsx index 0838292..2ebc597 100644 --- a/src/app/page.tsx +++ b/src/app/page.tsx @@ -2,7 +2,7 @@ import { Suspense } from "react" import { WorkspaceShell } from "@/components/workspace-shell" import { loadWorkspaceShellInitialState } from "@/domains/workspace/initial-state" import { effectOperation } from "@/lib/effect-operation" -import { formatUnknownForLog } from "@/lib/format-log-value" +import { summarizeUnknownError } from "@/lib/format-log-value" import { logger } from "@/lib/logger" import { connection } from "next/server" @@ -27,7 +27,7 @@ async function loadWorkspaceInitialState(): ReturnType< return await loadWorkspaceShellInitialState() } catch (error) { logger.error("workspace: initial state failed", { - error: formatUnknownForLog(error), + error: summarizeUnknownError(error), }) throw effectOperation.createBoundaryError( "Workspace initial state failed", diff --git a/src/components/chat-message-list.tsx b/src/components/chat-message-list.tsx index 1e8aa06..33703b5 100644 --- a/src/components/chat-message-list.tsx +++ b/src/components/chat-message-list.tsx @@ -13,6 +13,12 @@ import type { ChatMessageView, } from "@/domains/chat/types"; +type DisplayCitation = { + readonly citation: ChatCitationView; + readonly citationId: string; + readonly label: string; +}; + export type ChatMessageListProps = { readonly isDisabled?: boolean; readonly isSending?: boolean; @@ -230,22 +236,22 @@ function MessageBubble({ ); } + const displayCitations = getDisplayCitations( + message, + sourceTitlesByDocumentId, + ); + return (

{message.content}

- {message.citations && message.citations.length > 0 && ( + {displayCitations.length > 0 && (

Sources used

- {message.citations.map((cite, i) => { - const citationId = chatPanelModel.getCitationId(message.id, i); - const label = chatPanelModel.getCitationLabel( - cite, - sourceTitlesByDocumentId, - ); + {displayCitations.map(({ citation, citationId, label }) => { const isPending = citationId === pendingCitationId; return ( @@ -253,7 +259,7 @@ function MessageBubble({ key={citationId} type="button" disabled={!onCitationClick || isPending} - onClick={() => onCitationClick?.(cite, citationId)} + onClick={() => onCitationClick?.(citation, citationId)} className="inline-flex max-w-full cursor-pointer items-center gap-1 whitespace-normal rounded-sm px-0.5 py-0 text-left text-[11px] font-semibold text-primary underline decoration-primary/45 underline-offset-4 transition-colors hover:text-primary/80 hover:decoration-primary focus:outline-none focus:ring-4 focus:ring-ring/15 focus:ring-offset-2 focus:ring-offset-background disabled:cursor-wait disabled:opacity-75" aria-label={`Open source ${label}`} > @@ -269,3 +275,58 @@ function MessageBubble({
); } + +function getDisplayCitations( + message: ChatMessageView, + sourceTitlesByDocumentId: Readonly>, +): readonly DisplayCitation[] { + const seenKeys = new Set(); + const displayCitations: DisplayCitation[] = []; + + for (const [index, citation] of (message.citations ?? []).entries()) { + const label = chatPanelModel.getCitationLabel( + citation, + sourceTitlesByDocumentId, + ); + const key = getCitationDisplayKey(citation, label); + if (seenKeys.has(key)) continue; + + seenKeys.add(key); + displayCitations.push({ + citation, + citationId: chatPanelModel.getCitationId(message.id, index), + label, + }); + } + + return displayCitations; +} + +function getCitationDisplayKey( + citation: ChatCitationView, + label: string, +): string { + const documentId = getTrimmedCitationField(citation.source.documentId); + if (documentId) { + return joinCitationDisplayKeyParts(["document", documentId, label]); + } + + return joinCitationDisplayKeyParts([ + "fallback", + getTrimmedCitationField(citation.source.sourceFileName) ?? "", + getTrimmedCitationField(citation.source.sectionPath) ?? "", + getTrimmedCitationField(citation.description) ?? "", + label, + ]); +} + +function getTrimmedCitationField(value: string | undefined): string | null { + const trimmedValue = value?.trim() ?? ""; + return trimmedValue.length > 0 ? trimmedValue : null; +} + +function joinCitationDisplayKeyParts(parts: readonly string[]): string { + return parts + .map((part: string): string => `${part.length}:${part}`) + .join("|"); +} diff --git a/src/components/chat-panel.test.ts b/src/components/chat-panel.test.ts index d716e1c..bffe21a 100644 --- a/src/components/chat-panel.test.ts +++ b/src/components/chat-panel.test.ts @@ -101,6 +101,128 @@ describe("ChatPanel", () => { ); }); + it("deduplicates assistant sources by displayed label while keeping the first click target", async () => { + const user = userEvent.setup(); + const onCitationClick = vi.fn(); + const firstCitation = { + chunkType: "text", + score: 0.9, + description: "document-wDh6N9QBSgbdAjjweXN8xbw0vTTo5J.pdf", + source: { + documentId: "doc_micron_q1", + sourceFileName: "document-wDh6N9QBSgbdAjjweXN8xbw0vTTo5J.pdf", + sectionPath: "Root", + }, + } as const; + + render( + React.createElement(C, { + sourceTitlesByDocumentId: { + doc_micron_q1: "Micron Q1-26 Earnings Deck_R.pdf", + doc_micron_q2: "Q2 2026 Earnings Deck.pdf", + }, + messages: [ + { + id: "assistant_1", + role: "assistant", + content: "Diluted EPS is discussed in the earnings decks.", + citations: [ + firstCitation, + { + chunkType: "text", + score: 0.86, + source: { + documentId: "doc_micron_q1", + sourceFileName: "Micron Q1-26 Earnings Deck_R.pdf", + sectionPath: "Root", + }, + }, + { + chunkType: "text", + score: 0.82, + source: { + documentId: "doc_micron_q2", + sourceFileName: "Q2 2026 Earnings Deck.pdf", + sectionPath: "Mark Murphy / Non-GAAP operating results", + }, + }, + ], + }, + ], + onCitationClick, + }), + ); + + const duplicatedSourceLinks = screen.getAllByRole("button", { + name: "Open source Micron Q1-26 Earnings Deck_R.pdf", + }); + + expect(duplicatedSourceLinks).toHaveLength(1); + expect( + screen.getByRole("button", { + name: "Open source Q2 2026 Earnings Deck.pdf ยท Mark Murphy / Non-GAAP operating results", + }), + ).toBeTruthy(); + + await user.click(duplicatedSourceLinks[0]); + + expect(onCitationClick).toHaveBeenCalledWith(firstCitation, "assistant_1:0"); + }); + + it("keeps separate source links when different documents share one displayed label", async () => { + const user = userEvent.setup(); + const onCitationClick = vi.fn(); + const firstCitation = { + chunkType: "text", + score: 0.9, + source: { + documentId: "doc_first", + sourceFileName: "report.pdf", + sectionPath: "Root", + }, + } as const; + const secondCitation = { + chunkType: "text", + score: 0.88, + source: { + documentId: "doc_second", + sourceFileName: "report.pdf", + sectionPath: "Root", + }, + } as const; + + render( + React.createElement(C, { + sourceTitlesByDocumentId: { + doc_first: "report.pdf", + doc_second: "report.pdf", + }, + messages: [ + { + id: "assistant_1", + role: "assistant", + content: "Both reports are relevant.", + citations: [firstCitation, secondCitation], + }, + ], + onCitationClick, + }), + ); + + const duplicatedLabelLinks = screen.getAllByRole("button", { + name: "Open source report.pdf", + }); + + expect(duplicatedLabelLinks).toHaveLength(2); + + await user.click(duplicatedLabelLinks[1]); + + expect(onCitationClick).toHaveBeenCalledWith( + secondCitation, + "assistant_1:1", + ); + }); + it("renders citation links as button-backed links with per-citation loading feedback", async () => { const user = userEvent.setup(); const onCitationClick = vi.fn(); diff --git a/src/components/parsed-chunk-card.test.ts b/src/components/parsed-chunk-card.test.ts index 350a1fb..b269c08 100644 --- a/src/components/parsed-chunk-card.test.ts +++ b/src/components/parsed-chunk-card.test.ts @@ -12,7 +12,7 @@ describe("ParsedChunkCard", () => { }); it("renders text chunks with source, summary, content, and keywords", () => { - const { container } = render( + render( React.createElement(ParsedChunkCard, { chunk: { chunkId: "text_1", @@ -108,6 +108,27 @@ describe("ParsedChunkCard", () => { expect(screen.getByTestId("chunk-card-shell-text_1").getAttribute("role")).toBeNull(); }); + it("hides the original file button when a chunk has no page numbers", () => { + render( + React.createElement(ParsedChunkCard, { + chunk: { + chunkId: "text_1", + type: "text", + content: "Revenue details do not include page metadata.", + sourceTitle: "report.pdf", + }, + isFocused: false, + isOriginalPreviewAvailable: true, + onChunkClick: vi.fn(), + onReferenceClick: vi.fn(), + }), + ); + + expect( + screen.queryByRole("button", { name: /original file/i }), + ).toBeNull(); + }); + it("keeps original file buttons quiet when preview is not supported", async () => { const user = userEvent.setup(); const chunk = { diff --git a/src/components/parsed-chunk-card.tsx b/src/components/parsed-chunk-card.tsx index a75d41b..386d596 100644 --- a/src/components/parsed-chunk-card.tsx +++ b/src/components/parsed-chunk-card.tsx @@ -128,6 +128,7 @@ function ChunkSourcePanel({ readonly onChunkClick?: (chunk: ParsedChunkView) => void; }): ReactNode { const sourceMetadata = parsedChunkCardModel.getSourceMetadata(chunk); + const firstPageNumber = getFirstValidPageNumber(chunk); return (
- {onChunkClick ? ( + {onChunkClick && firstPageNumber !== null ? ( @@ -180,12 +182,24 @@ function ChunkSourcePanel({ ); } +function getFirstValidPageNumber(chunk: ParsedChunkView): number | null { + const pageNums = chunk.pageNums ?? []; + const validPageNums = pageNums.filter( + (pageNum) => Number.isFinite(pageNum) && pageNum > 0, + ); + if (validPageNums.length === 0) return null; + + return Math.min(...validPageNums); +} + function OpenOriginalButton({ chunk, + firstPageNumber, isOriginalPreviewAvailable, onChunkClick, }: { readonly chunk: ParsedChunkView; + readonly firstPageNumber: number; readonly isOriginalPreviewAvailable: boolean; readonly onChunkClick: (chunk: ParsedChunkView) => void; }): ReactNode { @@ -203,24 +217,18 @@ function OpenOriginalButton({ onClick={() => onChunkClick(chunk)} > - {getOpenOriginalButtonLabel(chunk, isOriginalPreviewAvailable)} + {getOpenOriginalButtonLabel(firstPageNumber, isOriginalPreviewAvailable)} ); } function getOpenOriginalButtonLabel( - chunk: ParsedChunkView, + firstPageNumber: number, isOriginalPreviewAvailable: boolean, ): string { if (!isOriginalPreviewAvailable) return "Open original file"; - const pageNums = chunk.pageNums ?? []; - const validPageNums = pageNums.filter( - (pageNum) => Number.isFinite(pageNum) && pageNum > 0, - ); - if (validPageNums.length === 0) return "Open original file"; - - return `Open page ${Math.min(...validPageNums)} in original file`; + return `Open page ${firstPageNumber} in original file`; } function ChunkSummaryPanel({ diff --git a/src/components/workspace-citation-focus.test.ts b/src/components/workspace-citation-focus.test.ts index feb1bca..2d2803a 100644 --- a/src/components/workspace-citation-focus.test.ts +++ b/src/components/workspace-citation-focus.test.ts @@ -93,6 +93,82 @@ describe("useWorkspaceCitationFocus", () => { requestId: 1, }); }); + + it("opens the source without fetching chunks when the citation has no exact target hint", async () => { + const fetchChunks = vi.fn(async () => [prefetchedChunk]); + const selectSource = vi.fn(); + const sourceOnlyCitation: ChatCitationView = { + chunkType: "text", + score: 0.5, + source: { + documentId: "document_1", + sourceFileName: "Contract.pdf", + sectionPath: "Root", + }, + }; + + const { result } = renderHook(() => + useWorkspaceCitationFocus({ + fetchChunks, + initialPrefetchedChunksBySourceId: { + source_1: [prefetchedChunk], + }, + onSelectSource: selectSource, + selectedSourceId: null, + sources: [readySource], + }), + { wrapper: createSWRWrapper }, + ); + + await act(async () => { + await result.current.handleCitationClick( + sourceOnlyCitation, + "message_1:0", + ); + }); + + expect(fetchChunks).not.toHaveBeenCalled(); + expect(selectSource).toHaveBeenLastCalledWith("source_1"); + expect(result.current.prefetchedChunksBySourceId).toEqual({}); + expect(result.current.focusedChunk.chunkId).toBeNull(); + expect(result.current.pendingCitationId).toBeNull(); + }); + + it("reuses cached chunks for a different source without refetching", async () => { + const fetchChunks = vi.fn(async () => [prefetchedChunk]); + const selectSource = vi.fn(); + const otherSource: SourceView = { + id: "source_2", + title: "Other.pdf", + mimeType: "application/pdf", + status: "ready", + documentId: "document_2", + }; + + const { result } = renderHook(() => + useWorkspaceCitationFocus({ + fetchChunks, + initialPrefetchedChunksBySourceId: { + source_1: [prefetchedChunk], + }, + onSelectSource: selectSource, + selectedSourceId: "source_2", + sources: [readySource, otherSource], + }), + { wrapper: createSWRWrapper }, + ); + + await act(async () => { + await result.current.handleCitationClick(citation, "message_1:0"); + }); + + expect(fetchChunks).not.toHaveBeenCalled(); + expect(selectSource).toHaveBeenLastCalledWith("source_1"); + expect(result.current.focusedChunk.chunkId).toBe("chunk_1"); + expect(Object.keys(result.current.prefetchedChunksBySourceId)).toContain( + "source_1", + ); + }); }); function createSWRWrapper({ diff --git a/src/components/workspace-citation-focus.ts b/src/components/workspace-citation-focus.ts index d428bf0..711e912 100644 --- a/src/components/workspace-citation-focus.ts +++ b/src/components/workspace-citation-focus.ts @@ -85,7 +85,7 @@ export function useWorkspaceCitationFocus({ onSelectSource(sourceId) if (sourceId) { setPrefetchedChunksBySourceId((current) => - removeRecordKey(current, sourceId), + workspaceCitationState.removePrefetchedChunks(current, sourceId), ) } requestChunkFocus(null) @@ -119,6 +119,37 @@ export function useWorkspaceCitationFocus({ return } + if (!workspaceCitationState.hasExactCitationTargetHint(citation)) { + setPrefetchedChunksBySourceId((current) => + workspaceCitationState.removePrefetchedChunks(current, source.id), + ) + if (selectedSourceId !== source.id) onSelectSource(source.id) + requestChunkFocus(null) + return + } + + const cachedChunks = prefetchedChunksBySourceId[source.id] + if (cachedChunks) { + const cachedFocusId = + workspaceCitationState.getLoadedCitationChunkId({ + citation, + selectedSourceId: source.id, + sourceId: source.id, + selectedChunks: cachedChunks, + hasMoreSelectedChunks: false, + }) + setPrefetchedChunksBySourceId((current) => + workspaceCitationState.upsertPrefetchedChunks( + current, + source.id, + cachedChunks, + ), + ) + if (selectedSourceId !== source.id) onSelectSource(source.id) + requestChunkFocus(cachedFocusId) + return + } + requestChunkFocus(null) const chunks = await fetchChunks(source.id) setPrefetchedChunksBySourceId((current) => @@ -148,6 +179,7 @@ export function useWorkspaceCitationFocus({ fetchChunks, hasMoreSelectedChunks, onSelectSource, + prefetchedChunksBySourceId, requestChunkFocus, selectedChunks, selectedSourceId, @@ -170,12 +202,3 @@ export function useWorkspaceCitationFocus({ selectedSource, } } - -function removeRecordKey( - record: Readonly>, - key: string, -): Record { - const nextRecord = { ...record } - delete nextRecord[key] - return nextRecord -} diff --git a/src/components/workspace-citation-state.test.ts b/src/components/workspace-citation-state.test.ts index 2ffc308..ce5851e 100644 --- a/src/components/workspace-citation-state.test.ts +++ b/src/components/workspace-citation-state.test.ts @@ -1,10 +1,23 @@ import { describe, expect, it } from "vitest" -import { workspaceCitationState } from "./workspace-citation-state" +import { + maxPrefetchedChunkSources, + workspaceCitationState, +} from "./workspace-citation-state" import type { ChatCitationView } from "@/domains/chat/types" import type { ParsedChunkView } from "@/domains/chunks/types" import type { SourceView } from "@/domains/sources/types" +function makeChunk(chunkId: string, documentId: string): ParsedChunkView { + return { + chunkId, + documentId, + type: "text", + content: `${chunkId} content`, + sourceTitle: `${documentId}.pdf`, + } +} + describe("workspaceCitationState", () => { it("finds the Source and loaded Parsed Chunk for a Citation", () => { const source: SourceView = { @@ -77,4 +90,154 @@ describe("workspaceCitationState", () => { }), ).toBeNull() }) + + describe("hasExactCitationTargetHint", () => { + it("returns true when the citation has content text", () => { + const citation: ChatCitationView = { + chunkType: "text", + score: 0.5, + content: "Revenue grew this quarter.", + source: { documentId: "document_1", sourceFileName: "Contract.pdf" }, + } + + expect( + workspaceCitationState.hasExactCitationTargetHint(citation), + ).toBe(true) + }) + + it("returns true when the citation has a meaningful section path", () => { + const citation: ChatCitationView = { + chunkType: "text", + score: 0.5, + source: { + documentId: "document_1", + sectionPath: "Revenue", + }, + } + + expect( + workspaceCitationState.hasExactCitationTargetHint(citation), + ).toBe(true) + }) + + it("returns false for source-only citations with no useful target hint", () => { + const citation: ChatCitationView = { + chunkType: "text", + score: 0.5, + source: { + documentId: "document_1", + sourceFileName: "Contract.pdf", + sectionPath: "Root", + }, + } + + expect( + workspaceCitationState.hasExactCitationTargetHint(citation), + ).toBe(false) + }) + + it("returns false for empty content and missing section path", () => { + const citation: ChatCitationView = { + chunkType: "text", + score: 0.5, + content: " ", + source: { documentId: "document_1" }, + } + + expect( + workspaceCitationState.hasExactCitationTargetHint(citation), + ).toBe(false) + }) + }) + + describe("upsertPrefetchedChunks bounded LRU", () => { + it("keeps insertion order so the newest source appears last", () => { + const next = workspaceCitationState.upsertPrefetchedChunks( + workspaceCitationState.upsertPrefetchedChunks({}, "source_a", [ + makeChunk("a", "doc_a"), + ]), + "source_b", + [makeChunk("b", "doc_b")], + ) + + expect(Object.keys(next)).toEqual(["source_a", "source_b"]) + }) + + it("refreshes recency for an existing source by moving it to the end", () => { + const seed = workspaceCitationState.upsertPrefetchedChunks( + workspaceCitationState.upsertPrefetchedChunks({}, "source_a", [ + makeChunk("a", "doc_a"), + ]), + "source_b", + [makeChunk("b", "doc_b")], + ) + + const refreshed = workspaceCitationState.upsertPrefetchedChunks( + seed, + "source_a", + [makeChunk("a", "doc_a"), makeChunk("a2", "doc_a")], + ) + + expect(Object.keys(refreshed)).toEqual(["source_b", "source_a"]) + expect(refreshed["source_a"]?.length).toBe(2) + }) + + it(`evicts the oldest entries when more than ${maxPrefetchedChunkSources} sources are stored`, () => { + const seeded = Array.from({ length: maxPrefetchedChunkSources }).reduce< + Readonly> + >( + (acc, _value, index) => + workspaceCitationState.upsertPrefetchedChunks( + acc, + `source_${index}`, + [makeChunk(`chunk_${index}`, `doc_${index}`)], + ), + {}, + ) + + const next = workspaceCitationState.upsertPrefetchedChunks( + seeded, + "source_overflow", + [makeChunk("chunk_overflow", "doc_overflow")], + ) + + const keys = Object.keys(next) + expect(keys.length).toBe(maxPrefetchedChunkSources) + expect(keys).not.toContain("source_0") + expect(keys).toContain("source_overflow") + expect(keys[keys.length - 1]).toBe("source_overflow") + }) + }) + + describe("removePrefetchedChunks", () => { + it("returns the same record when the source is not cached", () => { + const seed = workspaceCitationState.upsertPrefetchedChunks({}, "source_a", [ + makeChunk("a", "doc_a"), + ]) + + const next = workspaceCitationState.removePrefetchedChunks( + seed, + "missing", + ) + + expect(next).toBe(seed) + }) + + it("removes only the requested source", () => { + const seed = workspaceCitationState.upsertPrefetchedChunks( + workspaceCitationState.upsertPrefetchedChunks({}, "source_a", [ + makeChunk("a", "doc_a"), + ]), + "source_b", + [makeChunk("b", "doc_b")], + ) + + const next = workspaceCitationState.removePrefetchedChunks( + seed, + "source_a", + ) + + expect(Object.keys(next)).toEqual(["source_b"]) + }) + }) }) diff --git a/src/components/workspace-citation-state.ts b/src/components/workspace-citation-state.ts index 224fa5e..d8331af 100644 --- a/src/components/workspace-citation-state.ts +++ b/src/components/workspace-citation-state.ts @@ -24,13 +24,22 @@ type WorkspaceCitationStateModule = { readonly getLoadedCitationChunkId: ( input: LoadedCitationChunkInput, ) => string | null + readonly hasExactCitationTargetHint: ( + citation: ChatCitationView, + ) => boolean readonly upsertPrefetchedChunks: ( current: PrefetchedChunksBySourceId, sourceId: string, chunks: readonly ParsedChunkView[], ) => Record + readonly removePrefetchedChunks: ( + current: PrefetchedChunksBySourceId, + sourceId: string, + ) => PrefetchedChunksBySourceId } +export const maxPrefetchedChunkSources = 5 + function findCitationSource( sources: readonly SourceView[], citation: ChatCitationView, @@ -55,19 +64,63 @@ function getLoadedCitationChunkId( return focusedChunk?.chunkId ?? null } +function hasExactCitationTargetHint(citation: ChatCitationView): boolean { + if (typeof citation.content === "string" && citation.content.trim().length > 0) { + return true + } + + const sectionPath = citation.source.sectionPath + if (typeof sectionPath !== "string") return false + + const trimmed = sectionPath.trim() + if (trimmed.length === 0) return false + if (trimmed === "Root") return false + + return true +} + function upsertPrefetchedChunks( current: PrefetchedChunksBySourceId, sourceId: string, chunks: readonly ParsedChunkView[], ): Record { - return { - ...current, - [sourceId]: [...chunks], + const next: Record = {} + + for (const [existingSourceId, existingChunks] of Object.entries(current)) { + if (existingSourceId === sourceId) continue + next[existingSourceId] = existingChunks + } + next[sourceId] = [...chunks] + + const orderedKeys = Object.keys(next) + if (orderedKeys.length <= maxPrefetchedChunkSources) return next + + const evictionCount = orderedKeys.length - maxPrefetchedChunkSources + for (let index = 0; index < evictionCount; index += 1) { + delete next[orderedKeys[index]!] + } + + return next +} + +function removePrefetchedChunks( + current: PrefetchedChunksBySourceId, + sourceId: string, +): PrefetchedChunksBySourceId { + if (!Object.prototype.hasOwnProperty.call(current, sourceId)) return current + + const next: Record = {} + for (const [existingSourceId, existingChunks] of Object.entries(current)) { + if (existingSourceId === sourceId) continue + next[existingSourceId] = existingChunks } + return next } export const workspaceCitationState: WorkspaceCitationStateModule = { findCitationSource, getLoadedCitationChunkId, + hasExactCitationTargetHint, upsertPrefetchedChunks, + removePrefetchedChunks, } diff --git a/src/lib/effect-operation.test.ts b/src/lib/effect-operation.test.ts index 04c3b97..93204c0 100644 --- a/src/lib/effect-operation.test.ts +++ b/src/lib/effect-operation.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest" import { effectOperation } from "./effect-operation" import { formatUnknownForLog } from "./format-log-value" +import { makeWorkspaceInitialStateFailureFixture } from "@/test/workspace-initial-state-failure-fixture" describe("effectOperation", () => { it("adds operation context to Promise failures", async () => { @@ -53,9 +54,48 @@ describe("effectOperation", () => { ) expect(boundaryError.message).toBe( - "Workspace initial state failed: Effect operation workspace.load failed", + "Workspace initial state failed: database connection refused", ) expect(boundaryError.cause).toBe(error) } }) + + it("surfaces driver causes through database query wrappers", async () => { + const databaseConnectionError = new Error( + "connect ECONNREFUSED 127.0.0.1:5434", + ) + const queryError = new Error( + 'Failed query: select "id" from "workspaces" where "user_id" = $1', + { cause: databaseConnectionError }, + ) + + try { + await Effect.runPromise( + effectOperation.tryPromise("workspace.load", () => + Promise.reject(queryError), + ), + ) + throw new Error("Expected operation to fail.") + } catch (error) { + const boundaryError = effectOperation.createBoundaryError( + "Workspace initial state failed", + error, + ) + + expect(boundaryError.message).toBe( + "Workspace initial state failed: connect ECONNREFUSED 127.0.0.1:5434", + ) + } + }) + + it("surfaces deeply nested driver defects through Effect FiberFailure cause objects", () => { + const failure = makeWorkspaceInitialStateFailureFixture() + + const boundaryError = effectOperation.createBoundaryError( + "Workspace initial state failed", + failure.error, + ) + + expect(boundaryError.message).toBe(failure.boundaryMessage) + }) }) diff --git a/src/lib/format-log-value.ts b/src/lib/format-log-value.ts index f241a32..002c497 100644 --- a/src/lib/format-log-value.ts +++ b/src/lib/format-log-value.ts @@ -7,6 +7,7 @@ type LogValue = | { readonly [key: string]: LogValue } const maxDepth = 8 +const maxSummaryDepth = 16 const maxStackLines = 12 export function formatUnknownForLog(value: unknown): string { @@ -117,21 +118,23 @@ function findErrorMessage( depth: number, seenObjects: WeakSet, ): string | null { - if (depth >= maxDepth) return null + if (depth >= maxSummaryDepth) return null if (typeof value === "string" && value.trim().length > 0) return value if (typeof value !== "object" || value === null) return null if (seenObjects.has(value)) return null seenObjects.add(value) if (value instanceof Error) { - if (isSpecificMessage(value.message)) return value.message - return ( + const nestedMessage = findErrorMessage(value.cause, depth + 1, seenObjects) ?? findSymbolErrorMessage(value, depth, seenObjects) - ) + const ownMessage = isSpecificMessage(value.message) ? value.message : null + + if (ownMessage && !isWrapperMessage(value, ownMessage)) return ownMessage + return nestedMessage ?? ownMessage } - for (const key of ["failure", "error", "cause"] as const) { + for (const key of ["failure", "error", "cause", "defect"] as const) { const nestedMessage = findErrorMessage( readObjectProperty(value, key), depth + 1, @@ -171,6 +174,18 @@ function isSpecificMessage(message: string): boolean { ) } +function isWrapperMessage(error: Error, message: string): boolean { + const tag = readObjectProperty(error, "_tag") + return ( + tag === "EffectOperationError" || + error.name.includes("EffectOperationError") || + error.name.includes("FiberFailure") || + message.startsWith("Effect operation ") || + message.startsWith("Failed query:") || + message.endsWith(" failed") + ) +} + function getObjectPropertyKeys(value: object): readonly (string | symbol)[] { return [ ...Object.getOwnPropertyNames(value), diff --git a/src/test/workspace-initial-state-failure-fixture.ts b/src/test/workspace-initial-state-failure-fixture.ts new file mode 100644 index 0000000..2c9d68a --- /dev/null +++ b/src/test/workspace-initial-state-failure-fixture.ts @@ -0,0 +1,45 @@ +const databaseConnectionMessage = "connect ECONNREFUSED 127.0.0.1:5434" +const fiberCauseSymbol = Symbol.for("effect/Runtime/FiberFailure/Cause") +const workspaceOperationMessage = + "Workspace initial state getOptionalAuthenticated failed" + +export function makeWorkspaceInitialStateFailureFixture(): { + readonly boundaryMessage: string + readonly error: Error + readonly rootCauseMessage: string +} { + const databaseConnectionError = new Error(databaseConnectionMessage) + const queryError = new Error( + 'Failed query: select "id" from "workspaces" where "user_id" = $1', + ) + Object.defineProperty(queryError, fiberCauseSymbol, { + value: { + _tag: "Die", + defect: databaseConnectionError, + }, + }) + const unknownException = new Error( + "An unknown error occurred in Effect.tryPromise", + { cause: queryError }, + ) + const operationError = new Error(workspaceOperationMessage, { + cause: unknownException, + }) + operationError.name = "EffectOperationError" + Object.defineProperty(operationError, "_tag", { + value: "EffectOperationError", + }) + const outerFiberFailure = new Error(workspaceOperationMessage) + Object.defineProperty(outerFiberFailure, fiberCauseSymbol, { + value: { + _tag: "Fail", + error: operationError, + }, + }) + + return { + boundaryMessage: `Workspace initial state failed: ${databaseConnectionMessage}`, + error: outerFiberFailure, + rootCauseMessage: databaseConnectionMessage, + } +}