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
69 changes: 65 additions & 4 deletions src/app/api/sources/[sourceId]/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const mocks = vi.hoisted(() => {
ensureWorkspace: vi.fn(),
fetchDemoCatalog: vi.fn(),
findSourceInWorkspace: vi.fn(),
findByKnowhereDocumentId: vi.fn(),
getCurrentUser: vi.fn(),
hideDemoSource: vi.fn(),
makeKnowhereClient: vi.fn(),
Expand Down Expand Up @@ -55,6 +56,7 @@ vi.mock("@/domains/sources/background-reconcile", () => ({
vi.mock("@/domains/sources/service", () => ({
sourceService: {
findInWorkspace: mocks.findSourceInWorkspace,
findByKnowhereDocumentId: mocks.findByKnowhereDocumentId,
hideDemoSource: mocks.hideDemoSource,
retrySourceToKnowhere: mocks.retrySourceToKnowhere,
softDelete: mocks.softDeleteSource,
Expand Down Expand Up @@ -116,11 +118,17 @@ describe("PATCH /api/sources/[sourceId]", () => {
);
});

it("rejects archive requests for unlocalized remote source ids", async () => {
it("archives unlocalized remote Knowhere documents", async () => {
mocks.requireUser.mockResolvedValue({ id: "user_1" });
mocks.ensureWorkspace.mockResolvedValue({ id: "workspace_1" });
mocks.findSourceInWorkspace.mockResolvedValue(null);
mocks.findByKnowhereDocumentId.mockResolvedValue(null);
mocks.fetchDemoCatalog.mockResolvedValue({ sources: [] });
mocks.ensureApiKeyForWorkspace.mockResolvedValue("jwt_123");
mocks.makeKnowhereClient.mockReturnValue({
documents: { archive: mocks.archive },
});
mocks.archive.mockResolvedValue(undefined);

const response = await PATCH(
new NextRequest(
Expand All @@ -138,18 +146,71 @@ describe("PATCH /api/sources/[sourceId]", () => {
);

await expect(response.json()).resolves.toEqual({
message: "Source not found.",
id: "knowhere-doc:default:doc_remote",
archived: true,
});
expect(response.status).toBe(404);
expect(response.status).toBe(200);
expect(mocks.findSourceInWorkspace).toHaveBeenCalledWith(
"workspace_1",
"knowhere-doc:default:doc_remote",
);
expect(mocks.archive).not.toHaveBeenCalled();
expect(mocks.archive).toHaveBeenCalledWith("doc_remote");
expect(mocks.findByKnowhereDocumentId).toHaveBeenCalledWith(
"workspace_1",
"doc_remote",
);
expect(mocks.softDeleteSource).not.toHaveBeenCalled();
expect(mocks.deleteBlob).not.toHaveBeenCalled();
});

it("soft-deletes a matching local row when archiving a remote source id", async () => {
mocks.requireUser.mockResolvedValue({ id: "user_1" });
mocks.ensureWorkspace.mockResolvedValue({ id: "workspace_1" });
mocks.findSourceInWorkspace.mockResolvedValue(null);
mocks.findByKnowhereDocumentId.mockResolvedValue({
id: "source_1",
knowhereDocumentId: "doc_remote",
originalBlobPathname: "source-uploads/upload_1/document.pdf",
demoKey: null,
});
mocks.fetchDemoCatalog.mockResolvedValue({ sources: [] });
mocks.ensureApiKeyForWorkspace.mockResolvedValue("jwt_123");
mocks.makeKnowhereClient.mockReturnValue({
documents: { archive: mocks.archive },
});
mocks.archive.mockResolvedValue(undefined);
mocks.softDeleteSource.mockResolvedValue(true);

const response = await PATCH(
new NextRequest(
"http://localhost:3001/api/sources/knowhere-doc:default:doc_remote",
{
method: "PATCH",
body: JSON.stringify({ archived: true }),
},
),
{
params: Promise.resolve({
sourceId: "knowhere-doc:default:doc_remote",
}),
},
);

await expect(response.json()).resolves.toEqual({
id: "knowhere-doc:default:doc_remote",
archived: true,
});
expect(response.status).toBe(200);
expect(mocks.archive).toHaveBeenCalledWith("doc_remote");
expect(mocks.softDeleteSource).toHaveBeenCalledWith(
"workspace_1",
"source_1",
);
expect(mocks.deleteBlob).toHaveBeenCalledWith(
"source-uploads/upload_1/document.pdf",
);
});

it("does not fail an already-soft-deleted source when original Blob cleanup fails", async () => {
mocks.requireUser.mockResolvedValue({ id: "user_1" });
mocks.ensureWorkspace.mockResolvedValue({ id: "workspace_1" });
Expand Down
88 changes: 87 additions & 1 deletion src/components/chunks-panel-state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ describe("chunksPanelState", () => {
])
})

it("deduplicates page-asset chunks by page number", () => {
it("deduplicates singleton page-asset chunks with the same page number", () => {
const chunks: ParsedChunkView[] = [
{
chunkId: "page_4_first",
Expand Down Expand Up @@ -208,6 +208,92 @@ describe("chunksPanelState", () => {
).toEqual(["page_4_first", "page_5"])
})

it("keeps overlapping page-memory section chunks that share a boundary page", () => {
const chunks: ParsedChunkView[] = [
{
chunkId: "kenneth",
type: "page",
content: "IR introduction.",
sectionPath: "call.pdf/Root/Kenneth Dorell",
sourceTitle: "call.pdf",
pageNums: [1],
pageAssets: [
{
pageNumber: 1,
assetUrl: "https://assets.example/page-1.png",
contentType: "image/png",
},
],
},
{
chunkId: "zuckerberg",
type: "page",
content: "[SAME-AS call.pdf/Root/Kenneth Dorell p1] CEO remarks.",
sectionPath: "call.pdf/Root/Mark Zuckerberg, CEO",
sourceTitle: "call.pdf",
pageNums: [1, 2],
pageAssets: [
{
pageNumber: 1,
assetUrl: "https://assets.example/page-1.png",
contentType: "image/png",
},
{
pageNumber: 2,
assetUrl: "https://assets.example/page-2.png",
contentType: "image/png",
},
],
},
{
chunkId: "outlook",
type: "page",
content: "Q2 outlook.",
sectionPath: "call.pdf/Root/Moving to our financial outlook.",
sourceTitle: "call.pdf",
pageNums: [8],
pageAssets: [
{
pageNumber: 8,
assetUrl: "https://assets.example/page-8.png",
contentType: "image/png",
},
],
},
{
chunkId: "capex",
type: "page",
content: "[SAME-AS call.pdf/Root/Moving to our financial outlook. p8] Q&A.",
sectionPath: "call.pdf/Root/Turning to the expense and capex outlooks.",
sourceTitle: "call.pdf",
pageNums: [8, 9, 10],
pageAssets: [
{
pageNumber: 8,
assetUrl: "https://assets.example/page-8.png",
contentType: "image/png",
},
{
pageNumber: 9,
assetUrl: "https://assets.example/page-9.png",
contentType: "image/png",
},
{
pageNumber: 10,
assetUrl: "https://assets.example/page-10.png",
contentType: "image/png",
},
],
},
]

expect(
chunksPanelState
.getPageAssetChunksWithoutDuplicatePages(chunks)
.map((chunk) => chunk.chunkId),
).toEqual(["kenneth", "zuckerberg", "outlook", "capex"])
})

it("hides table asset chunks from page-asset lists", () => {
const chunks: ParsedChunkView[] = [
{
Expand Down
35 changes: 24 additions & 11 deletions src/components/chunks-panel-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,29 +192,42 @@ function dedupeChunksById(
function getPageAssetChunksWithoutDuplicatePages(
chunks: readonly ParsedChunkView[],
): readonly ParsedChunkView[] {
const seenPageNumbers = new Set<number>()
const seenSingletonPageNumbers = new Set<number>()

return chunks.filter((chunk) => {
// Page-memory table assets currently store a file path, not HTML.
if (chunk.type === "table") return false
if (chunk.type !== "page") return true

const pageNumber = getPageAssetChunkPageNumber(chunk)
if (pageNumber === null) return true
if (seenPageNumbers.has(pageNumber)) return false
const pageNumbers = getPageAssetChunkPageNumbers(chunk)
if (pageNumbers.length === 0) return true
if (pageNumbers.length > 1) return true

seenPageNumbers.add(pageNumber)
const pageNumber = pageNumbers[0]!
if (seenSingletonPageNumbers.has(pageNumber)) return false

seenSingletonPageNumbers.add(pageNumber)
return true
})
}

function getPageAssetChunkPageNumber(chunk: ParsedChunkView): number | null {
const pageAssetNumbers = (chunk.pageAssets ?? [])
.map((pageAsset) => pageAsset.pageNumber)
.filter(isPositivePageNumber)
if (pageAssetNumbers.length > 0) return Math.min(...pageAssetNumbers)
function getPageAssetChunkPageNumbers(
chunk: ParsedChunkView,
): readonly number[] {
const pageAssetNumbers = uniquePositivePageNumbers(
(chunk.pageAssets ?? []).map((pageAsset) => pageAsset.pageNumber),
)
if (pageAssetNumbers.length > 0) return pageAssetNumbers

return uniquePositivePageNumbers(chunk.pageNums ?? [])
}

return getFirstPageNumber(chunk)
function uniquePositivePageNumbers(
pageNumbers: readonly number[],
): readonly number[] {
return [...new Set(pageNumbers.filter(isPositivePageNumber))].sort(
(left, right) => left - right,
)
}

function createMutableSectionTreeNode(input: {
Expand Down
59 changes: 59 additions & 0 deletions src/components/chunks-panel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,65 @@ describe("ChunksPanel", () => {
.toBeNull();
});

it("keeps overlapping page-memory sections that share a first page image", async () => {
mockVisibleVirtualViewport();

render(
React.createElement(C, {
chunks: [
{
chunkId: "kenneth",
type: "page",
content: "IR introduction",
sectionPath: "call.pdf/Root/Kenneth Dorell",
sourceTitle: "call.pdf",
pageNums: [1],
pageAssets: [
{
pageNumber: 1,
assetUrl: "https://assets.example/page-1.png",
contentType: "image/png",
},
],
},
{
chunkId: "zuckerberg",
type: "page",
content: "CEO remarks",
sectionPath: "call.pdf/Root/Mark Zuckerberg, CEO",
sourceTitle: "call.pdf",
pageNums: [1, 2],
pageAssets: [
{
pageNumber: 1,
assetUrl: "https://assets.example/page-1.png",
contentType: "image/png",
},
{
pageNumber: 2,
assetUrl: "https://assets.example/page-2.png",
contentType: "image/png",
},
],
},
],
selectedSource: "call.pdf",
selectedSourceView: {
id: "source_1",
title: "call.pdf",
mimeType: "application/pdf",
status: "ready",
documentPresentation: { kind: "page-assets", pageCount: 2 },
},
}),
);

selectListView();
expect(await screen.findByTestId("chunk-card-shell-kenneth")).toBeTruthy();
expect(screen.getByTestId("chunk-card-shell-zuckerberg")).toBeTruthy();
expect(screen.getByRole("img", { name: "Page 2" })).toBeTruthy();
});

it("renders page chunks normally when no page assets exist", async () => {
mockVisibleVirtualViewport();

Expand Down
9 changes: 9 additions & 0 deletions src/domains/chat/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,15 @@ describe("answerQuestionWithRetrieval", () => {
status: "ready",
currentJobResultId: "job_remote",
sourceFileName: "remote.pdf",
documentMetadata: {
createdByClient: "cli",
},
},
{
documentId: "doc_untagged",
namespace: "default",
status: "ready",
sourceFileName: "dummy.pdf",
},
],
});
Expand Down
2 changes: 2 additions & 0 deletions src/domains/chat/knowhere-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type {
import type { Source } from "@/infrastructure/db/schema"
import {
listRemoteLibraryDocuments,
isNotebookVisibleRemoteDocument,
type RemoteLibraryDocument,
} from "@/domains/sources/remote-library"
import type { SearchSources } from "./contracts"
Expand Down Expand Up @@ -128,6 +129,7 @@ async function listVisibleRemoteDocuments(input: {
return documents
.filter(
(document) =>
isNotebookVisibleRemoteDocument(document) &&
document.status === "ready" &&
!localDocumentIds.has(document.documentId) &&
!input.excludedDocumentIds.has(document.documentId),
Expand Down
Loading
Loading