diff --git a/apps/extension/package.json b/apps/extension/package.json index a23dd32..86f3174 100644 --- a/apps/extension/package.json +++ b/apps/extension/package.json @@ -12,7 +12,7 @@ "typecheck": "tsc -p tsconfig.json" }, "dependencies": { - "@mdbase-dev/ui": "0.1.0-beta.123", + "@mdbase-dev/ui": "0.1.0-beta.124", "@mdbase-reader/connect": "workspace:*", "@mdbase-reader/core": "workspace:*", "@mdbase-reader/platform": "workspace:*", diff --git a/apps/reader/package.json b/apps/reader/package.json index 2e257b8..5431452 100644 --- a/apps/reader/package.json +++ b/apps/reader/package.json @@ -21,7 +21,7 @@ }, "dependencies": { "@citation-js/plugin-csl": "0.8.2", - "@mdbase-dev/ui": "0.1.0-beta.123", + "@mdbase-dev/ui": "0.1.0-beta.124", "@mdbase-reader/connect": "workspace:*", "@mdbase-reader/core": "workspace:*", "@mdbase-reader/markdown-editor": "workspace:*", diff --git a/apps/reader/scripts/audit-annotation-workbench.mjs b/apps/reader/scripts/audit-annotation-workbench.mjs index 1b5c31f..4205aad 100644 --- a/apps/reader/scripts/audit-annotation-workbench.mjs +++ b/apps/reader/scripts/audit-annotation-workbench.mjs @@ -1,12 +1,14 @@ import { expect } from "@playwright/test"; -export async function auditAnnotationWorkbench(page, { screenshot, open }) { +export async function auditAnnotationWorkbench(page, { screenshot, open, blockWrites }) { const tools = page.getByRole("complementary", { name: "Source workspace" }); await tools.getByRole("button", { name: "Edit", exact: true }).first().focus(); await page.keyboard.press("Enter"); await expect(page.getByRole("button", { name: "Back to reading position" })).toHaveCount(0); const editor = tools.getByRole("textbox", { name: "Comment" }); + blockWrites(true); await editor.fill("[test] Dock-safe annotation draft."); + await expect(tools.getByRole("alert")).toContainText("offline"); const annotationId = await tools .locator(".annotation-card.is-editing") .getAttribute("data-annotation-id"); @@ -16,13 +18,24 @@ export async function auditAnnotationWorkbench(page, { screenshot, open }) { .filter({ has: page.locator(".dv-default-tab-content", { hasText: "Annotations —" }) }); const panelId = await tab.getAttribute("data-panel-id"); const workbench = page.locator(`[data-session-id="${panelId}"]`); + // Promotion deliberately closes the native sidebar and releases its editor lease. + // The memory-only buffer survives; there is no mounted writer to transfer from yet. + await expect(tools).toHaveCount(0); await workbench .locator(`[data-annotation-id="${annotationId}"]`) - .getByRole("button", { name: "Edit here", exact: true }) + .getByRole("button", { name: "Resume edits", exact: true }) .click(); const otherEditor = workbench.getByRole("textbox", { name: "Comment" }); await expect(otherEditor).toHaveValue(/Dock-safe annotation draft/u); await expect(editor).toHaveCount(0); + blockWrites(false); + await workbench.getByRole("button", { name: "Retry save", exact: true }).click(); + await expect(otherEditor).toHaveCount(0); + await workbench + .locator(`[data-annotation-id="${annotationId}"]`) + .getByRole("button", { name: "Edit", exact: true }) + .click(); + await expect(otherEditor).toHaveValue(/Dock-safe annotation draft/u); const menu = async (label) => { await tab.click({ button: "right" }); await page.getByRole("menuitem", { name: label, exact: true }).click(); @@ -35,6 +48,16 @@ export async function auditAnnotationWorkbench(page, { screenshot, open }) { await tab.click(); if (!(await tools.isVisible())) await page.getByRole("button", { name: "Toggle right sidebar" }).click(); + // Reopening the sidebar remounts its locally selected editor. Establish the workbench's + // ownership explicitly before testing transfer back, rather than assuming a hidden lease. + await expect(editor).toHaveValue(/Dock-safe annotation draft/u); + await expect(otherEditor).toHaveCount(0); + await workbench + .locator(`[data-annotation-id="${annotationId}"]`) + .getByRole("button", { name: "Edit here", exact: true }) + .click(); + await expect(otherEditor).toHaveValue(/Dock-safe annotation draft/u); + await expect(editor).toHaveCount(0); await tools .locator(`[data-annotation-id="${annotationId}"]`) .getByRole("button", { name: "Edit here", exact: true }) @@ -76,7 +99,7 @@ export async function auditAnnotationWorkbench(page, { screenshot, open }) { await page.reload(); await expect(tools.locator(".annotation-card")).toHaveCount(1); return [ - "Annotation editing transfers explicitly between inspector and workbench; docking preserves the draft without simultaneous writers", + "Annotation promotion resumes an uncommitted buffer after the sidebar closes; Edit here transfers the single editor back after docking", "Linked note insertion, deletion warning, cancellation, and deletion persistence work", ]; } diff --git a/apps/reader/scripts/audit-annotations.mjs b/apps/reader/scripts/audit-annotations.mjs index 1c31517..1d82ddf 100644 --- a/apps/reader/scripts/audit-annotations.mjs +++ b/apps/reader/scripts/audit-annotations.mjs @@ -161,7 +161,7 @@ export async function auditAnnotations(page, { screenshot, blockWrites }) { completed.push( "Local annotation search, comment filtering, sort controls, and return-to-reading work", ); - completed.push(...(await auditAnnotationWorkbench(page, { screenshot, open }))); + completed.push(...(await auditAnnotationWorkbench(page, { screenshot, open, blockWrites }))); await expect(page.frameLocator("iframe.html-viewer:visible").locator("h1")).toBeVisible(); await page.emulateMedia({ colorScheme: "dark", reducedMotion: "reduce" }); await screenshot("annotation-dark-inspector"); diff --git a/docs/annotation-loading-performance.md b/docs/annotation-loading-performance.md index a2702ae..2052080 100644 --- a/docs/annotation-loading-performance.md +++ b/docs/annotation-loading-performance.md @@ -5,8 +5,10 @@ selection performs no annotation queries or reads. Changing structural filters l collection for local filtering; typing a search does not broaden the saved view's selection. If view execution fails, Reader explains the fallback and applies its filters locally. -The Connect annotation repository queries in bounded scopes of 100 paths. Whole-record reads -remain limited to four concurrent workers. It hydrates a small initial batch of 16, then +The Connect annotation repository queries in bounded scopes of 100 paths. With beta.124, +`read-many-documents-v1` authorities hydrate editable annotations with revision-bearing +`readMany` projections, removing redundant per-record revision reads. Older authorities retain +whole-record reads limited to four concurrent workers. It hydrates a small initial batch of 16, then batches of up to 64, hydrating each query page before requesting the next. Early cumulative snapshots are published through 128 annotations; afterward, publication waits for roughly 25% growth, with an immediate final snapshot. This bounds cumulative copying without delaying the @@ -20,10 +22,14 @@ source panes. Concurrent readers of the same path share one request. Cancelling does not cancel others; cancelling the last subscriber aborts the underlying read. Failures and abandoned responses are not cached. -The cache holds at most 2,000 records for at most 15 seconds. Query results do not expose record -revisions, so this is bounded reuse, **not** indefinite revision-validated caching. Queries still +The legacy cache holds at most 2,000 records for at most 15 seconds. Revisionless query rows +must match every observable content/file fact before reuse; an advertised revision must also +match. This is bounded reuse, **not** indefinite semantic-cache validation. Qualified batches +install their own content/revision pair without casting projections into full documents. Queries still run for each load to determine membership. Local creates and body updates install their returned -record revisions immediately; deletes evict records and abort pending reads. A save overtaking a +record revisions immediately; deletes evict records and abort pending reads. A source reference changed +between discovery and hydration is resolved again before mapping. Qualified batch errors remain +visible; missing records are omitted rather than retried as point reads. A save overtaking a read cannot be overwritten by that read's older response. Use **View options → Refresh annotations** to rerun the saved view and bypass cached bodies @@ -43,5 +49,13 @@ source-cache completeness, count subscriptions, and search within saved selectio Opt-in Connect diagnostics now time `list-views` and `execute-view` alongside record reads and query pages. Compare cold and warm loads, time to first visible batch, total time, and whole-record read counts. For a saved view selecting 20 records from a collection of 1,000, cold hydration now -requires 20 whole-record reads rather than 1,000. This is a request-count reduction, not a measured -latency claim; authority query costs and the active transport still need runtime profiling. +requires 20 legacy whole-record reads rather than 1,000. On qualified authorities, the 16/64 +progressive batching fixture hydrates 130 annotations in three `readMany` calls and zero point +reads. These are consumer fixture counts, not a measured latency or wire-request claim (typed +batches also preselect membership on the authority). + +Path/ID and annotation-source discovery uses `output: "metadata"` only after positive +`supportsAuthorityFeature("query-metadata-v1")` evidence; legacy ordinary queries remain. +Library views still retain full frontmatter for arbitrary columns. Document reopening and file +exports use SDK `files.stat`, including its negotiated legacy listing fallback, instead of Reader +folder enumeration/caches. Every lookup refreshes metadata; downloads remain revision-pinned. diff --git a/docs/reader-improvement-audit.md b/docs/reader-improvement-audit.md index f59b191..957f0d3 100644 --- a/docs/reader-improvement-audit.md +++ b/docs/reader-improvement-audit.md @@ -146,6 +146,27 @@ arrangement recovery. The active annotation stripe is also removed. This follow-up is deployed as `ca73f7765cee-production-mu3t04ql`; live metadata, manifest, HTML, entry assets and production backend configuration were verified. +## SDK beta.124 / consumer canary + +The pre-existing annotation-workbench `Edit here` timeout was a **stale audit expectation**, +not a Connect regression. Promoting a tool deliberately closes the native sidebar +(`ReaderWorkspaceView.onPromote`), unmounting its editor and releasing the lease. The retained +memory-only buffer therefore offers **Resume edits**, not **Edit here**. Reopening the sidebar +remounts its selected editor and claims the lease again. + +The audit now blocks saves to prove that promotion retains an uncommitted buffer, explicitly +resumes/retries it, and establishes ownership after reopening the sidebar before asserting +**Edit here** transfers in both directions. The single-textbox and exact-buffer assertions remain; +no app behavior, timing allowance, or SDK failure is bypassed. + +Validation: full fixture audit **40 scenarios passed** (`/tmp/reader-audit-10fLSX`); +annotation-only **13 passed** (`/tmp/reader-audit-KHOR5i`); both PDF-touch scripts passed +(`/tmp/pdf-touch-handles-f7AizA`, `/tmp/reader-audit-Qlo2WE`). An earlier full run hit the native +touch-docking assertion and an earlier PDF-touch run missed the selection toolbar; reruns passed +without changes to those checks. Accessibility still reports one command-palette contrast finding +(`button[role="option"][type="button"]:nth-child(2) > span > small`), outside this change. +These are isolated fixture audits, not authenticated LAB acceptance. Owned browsers are closed. + ## Validation and limits Workspace tests, typechecking, production builds, architecture checks and spec diff --git a/package.json b/package.json index 7583426..8c13b77 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,7 @@ "devDependencies": { "@callumalpass/mdbase": "0.3.0-rc.5", "@eslint/js": "^9.39.2", - "@mdbase-dev/connect-dev": "0.1.0-beta.123", + "@mdbase-dev/connect-dev": "0.1.0-beta.124", "@types/node": "^22.19.11", "eslint": "^9.39.2", "eslint-import-resolver-typescript": "^4.4.5", diff --git a/packages/connect/package.json b/packages/connect/package.json index 2487492..3270168 100644 --- a/packages/connect/package.json +++ b/packages/connect/package.json @@ -11,7 +11,7 @@ "typecheck": "tsc -p tsconfig.json" }, "dependencies": { - "@mdbase-dev/connect": "0.1.0-beta.123", + "@mdbase-dev/connect": "0.1.0-beta.124", "@mdbase-reader/core": "workspace:*", "@mdbase-reader/migration": "workspace:*", "yaml": "^2.9.0" diff --git a/packages/connect/src/annotation-concurrency.test.ts b/packages/connect/src/annotation-concurrency.test.ts index 0b95cf6..bb32a34 100644 --- a/packages/connect/src/annotation-concurrency.test.ts +++ b/packages/connect/src/annotation-concurrency.test.ts @@ -6,6 +6,12 @@ import { ConnectAnnotationRepository } from "./annotation-repository.js"; import type { ReaderConnectClient } from "./repository-client.js"; import type { ConnectOutcome, QueryPage, RecordDocument } from "@mdbase-dev/connect"; +const legacyAuthorityFeatures = { + supportsAuthorityFeature: vi.fn(() => + Promise.resolve({ ok: true as const, value: false, diagnostics: [] }), + ), +}; + function success(value: Value): ConnectOutcome { return { ok: true, value, diagnostics: [] }; } @@ -61,6 +67,7 @@ describe("Connect annotation concurrency", () => { return success(annotationDocument(input.path)); }); const repository = new ConnectAnnotationRepository({ + ...legacyAuthorityFeatures, queryPages, read, } as unknown as ReaderConnectClient); @@ -104,6 +111,7 @@ describe("Connect annotation listing", () => { ); }); const repository = new ConnectAnnotationRepository({ + ...legacyAuthorityFeatures, queryPages, read, } as unknown as ReaderConnectClient); diff --git a/packages/connect/src/annotation-loading.test.ts b/packages/connect/src/annotation-loading.test.ts index f8b3830..4cc9d7c 100644 --- a/packages/connect/src/annotation-loading.test.ts +++ b/packages/connect/src/annotation-loading.test.ts @@ -4,7 +4,13 @@ import { describe, expect, it, vi, type Mock } from "vitest"; import { ConnectAnnotationRepository } from "./annotation-repository.js"; import type { ReaderConnectClient } from "./repository-client.js"; -import type { ConnectOutcome, QueryPage, RecordDocument } from "@mdbase-dev/connect"; +import type { ConnectOutcome, QueryInput, QueryPage, RecordDocument } from "@mdbase-dev/connect"; + +const legacyAuthorityFeatures = { + supportsAuthorityFeature: vi.fn(() => + Promise.resolve({ ok: true as const, value: false, diagnostics: [] }), + ), +}; const collection = collectionId("reading"); function document(index: number): RecordDocument { @@ -33,19 +39,25 @@ function page(documents: RecordDocument[]): ConnectOutcome { function repository(documents: RecordDocument[]): { read: Mock; - queryPages: Mock; + queryPages: Mock<(input: QueryInput) => AsyncGenerator>>; repo: ConnectAnnotationRepository; } { const read = vi.fn(({ path }) => Promise.resolve(ok(documents.find((entry) => entry.path === path)!)), ); - const queryPages = vi.fn(async function* () { - yield await Promise.resolve(page(documents)); - }); + const queryPages = vi.fn<(input: QueryInput) => AsyncGenerator>>( + async function* () { + yield await Promise.resolve(page(documents)); + }, + ); return { read, queryPages, - repo: new ConnectAnnotationRepository({ read, queryPages } as unknown as ReaderConnectClient), + repo: new ConnectAnnotationRepository({ + ...legacyAuthorityFeatures, + read, + queryPages, + } as unknown as ReaderConnectClient), }; } diff --git a/packages/connect/src/annotation-query.test.ts b/packages/connect/src/annotation-query.test.ts index bb8531e..b90a9f6 100644 --- a/packages/connect/src/annotation-query.test.ts +++ b/packages/connect/src/annotation-query.test.ts @@ -7,6 +7,12 @@ import { annotationPathsForSource } from "./annotation-query.js"; import type { ReaderConnectClient } from "./repository-client.js"; import type { QueryInput } from "@mdbase-dev/connect"; +const legacyAuthorityFeatures = { + supportsAuthorityFeature: vi.fn(() => + Promise.resolve({ ok: true as const, value: false, diagnostics: [] }), + ), +}; + it("asks mdbase which links reach the source, keeps legacy IDs, and follows renames", async () => { let sourcePath = 'sources/A "quoted" title.md'; // How mdbase resolves each link: a record path, or null when it reaches no record. @@ -31,11 +37,13 @@ it("asks mdbase which links reach the source, keeps legacy IDs, and follows rena const references = Object.keys(links); const results = id !== undefined - ? [{ path: sourcePath, effectiveFrontmatter: { id: "src_1" } }] + ? [{ path: sourcePath, effectiveFrontmatter: { id: "src_1" }, file: {}, types: [] }] : references .map((source, i) => ({ path: `annotations/${String(i)}.md`, effectiveFrontmatter: { source }, + file: {}, + types: ["reader-annotation"], })) .filter(({ effectiveFrontmatter: { source } }) => target !== undefined @@ -44,7 +52,10 @@ it("asks mdbase which links reach the source, keeps legacy IDs, and follows rena ); yield await Promise.resolve({ ok: true, value: { results }, diagnostics: [] }); }); - const client = { queryPages } as unknown as ReaderConnectClient; + const client = { + ...legacyAuthorityFeatures, + queryPages, + } as unknown as ReaderConnectClient; const controller = new AbortController(); const options = { signal: controller.signal }; expect(await annotationPathsForSource(client, sourceId("src_1"), options)).toEqual([ diff --git a/packages/connect/src/annotation-query.ts b/packages/connect/src/annotation-query.ts index e4aaecd..6a3f6e4 100644 --- a/packages/connect/src/annotation-query.ts +++ b/packages/connect/src/annotation-query.ts @@ -4,7 +4,14 @@ import { annotationSourceReference } from "./annotation-source.js"; import { outcomeValue, recordPathById } from "./repository-client.js"; import type { ReaderConnectClient } from "./repository-client.js"; -import type { QueryInput, QueryRecord } from "@mdbase-dev/connect"; +import type { + QueryInput, + QueryRecord, + QueryMetadataRecord, + QueryPage, + QueryMetadataPage, + ReadManyRecord, +} from "@mdbase-dev/connect"; import type { ReaderRequestOptions, SourceId } from "@mdbase-reader/core"; /** Filter Reader's persisted source references before transferring annotation metadata. */ @@ -13,10 +20,18 @@ export async function annotationPathsForSource( source: SourceId, options: ReaderRequestOptions, ): Promise { + return [...(await annotationCandidatesForSource(client, source, options)).keys()]; +} + +export async function annotationCandidatesForSource( + client: ReaderConnectClient, + source: SourceId, + options: ReaderRequestOptions, +): Promise> { const records = await sourceAnnotations(client, source, options, { frontmatterMode: "effective", }); - return records.map(({ path }) => path); + return new Map(records.map((record) => [record.path, sourceValue(record)])); } /** @@ -28,7 +43,7 @@ export async function annotationRecordsAt( client: ReaderConnectClient, paths: readonly string[], options: ReaderRequestOptions, -): Promise> { +): Promise> { const result = outcomeValue( await client.readMany(paths, { types: ["reader-annotation"], @@ -55,7 +70,7 @@ async function sourceAnnotations( source: SourceId, options: ReaderRequestOptions, detail: Pick, -): Promise { +): Promise<(QueryRecord | QueryMetadataRecord)[]> { // Never infer identity from a filename: sources can be renamed independently of their IDs. const path = await recordPathById(client, source, options); const [linked, legacy] = await Promise.all([ @@ -70,10 +85,7 @@ async function sourceAnnotations( detail, options, // contains() is only a candidate filter; aliases and prefix collisions must not match. - (record) => - annotationSourceReference( - (record.effectiveFrontmatter ?? record.frontmatter)?.["source"], - ) === source, + (record) => annotationSourceReference(sourceValue(record)) === source, ), ]); return [...linked, ...legacy]; @@ -84,14 +96,26 @@ async function annotationRecords( where: string, detail: Pick, options: ReaderRequestOptions, - accept: (record: QueryRecord) => boolean = () => true, -): Promise { - const records: QueryRecord[] = []; - for await (const outcome of client.queryPages( - { types: ["reader-annotation"], where, ...detail }, - { ...options, firstPageSize: 100, pageSize: 100 }, - )) { - for (const record of outcomeValue(outcome, "find source annotations").results) { + accept: (record: QueryRecord | QueryMetadataRecord) => boolean = () => true, +): Promise<(QueryRecord | QueryMetadataRecord)[]> { + const records: (QueryRecord | QueryMetadataRecord)[] = []; + const input = { types: ["reader-annotation"], where, ...detail }; + const paging = { ...options, firstPageSize: 100, pageSize: 100 }; + const metadata = outcomeValue( + await client.supportsAuthorityFeature("query-metadata-v1", options), + "discover metadata queries", + ); + const pages = metadata + ? client.queryPages( + { ...input, output: "metadata", includeBody: false, select: ["source"] }, + paging, + ) + : client.queryPages(input, paging); + for await (const outcome of pages) { + for (const record of outcomeValue( + outcome, + "find source annotations", + ).results) { if (accept(record)) { records.push(record); } @@ -99,3 +123,9 @@ async function annotationRecords( } return records; } + +function sourceValue(record: QueryRecord | QueryMetadataRecord): unknown { + return "file" in record + ? (record.effectiveFrontmatter ?? record.frontmatter)?.["source"] + : record.values["source"]; +} diff --git a/packages/connect/src/annotation-record-cache.ts b/packages/connect/src/annotation-record-cache.ts index 3daabe9..1ab9f41 100644 --- a/packages/connect/src/annotation-record-cache.ts +++ b/packages/connect/src/annotation-record-cache.ts @@ -34,7 +34,7 @@ export class AnnotationRecordCache { /** * The cached document when a fresh query result shows the file unchanged, whatever its age. - * Query results carry no revision, so this is how a listing keeps one without a read. + * Legacy query results may omit revisions, so they must match every observable fact. */ current(record: QueryRecord): RecordDocument | null { const cached = this.#records.get(record.path); @@ -149,6 +149,7 @@ function unchanged(document: RecordDocument, record: QueryRecord): boolean { const sameFact = (left: unknown, right: unknown): boolean => left === undefined || right === undefined || left === right; return ( + (record.revision === undefined || record.revision === document.revision) && record.body !== undefined && record.body === (document.body ?? "") && record.frontmatter !== undefined && diff --git a/packages/connect/src/annotation-repository.test.ts b/packages/connect/src/annotation-repository.test.ts index d50c863..890b10e 100644 --- a/packages/connect/src/annotation-repository.test.ts +++ b/packages/connect/src/annotation-repository.test.ts @@ -7,6 +7,12 @@ import type { ReaderConnectClient } from "./repository-client.js"; import type { ConnectOutcome, RecordDocument } from "@mdbase-dev/connect"; import type { Annotation } from "@mdbase-reader/core"; +const legacyAuthorityFeatures = { + supportsAuthorityFeature: vi.fn(() => + Promise.resolve({ ok: true as const, value: false, diagnostics: [] }), + ), +}; + function success(value: Value): ConnectOutcome { return { ok: true, value, diagnostics: [] }; } @@ -16,6 +22,7 @@ describe("ConnectAnnotationRepository updates", () => { const document = annotationDocument(); const update = vi.fn(() => Promise.resolve(success(document))); const repository = new ConnectAnnotationRepository({ + ...legacyAuthorityFeatures, update, } as unknown as ReaderConnectClient); const original = annotationFixture(); @@ -53,6 +60,7 @@ describe("ConnectAnnotationRepository deletion", () => { Promise.resolve(success({ path: original.path ?? "", deleted: true })), ); const repository = new ConnectAnnotationRepository({ + ...legacyAuthorityFeatures, preflightDelete, deleteWithProgress, } as unknown as ReaderConnectClient); @@ -79,6 +87,7 @@ describe("ConnectAnnotationRepository deletion", () => { const original = annotationFixture(); const deleteWithProgress = vi.fn(); const repository = new ConnectAnnotationRepository({ + ...legacyAuthorityFeatures, deleteWithProgress, } as unknown as ReaderConnectClient); diff --git a/packages/connect/src/annotation-repository.ts b/packages/connect/src/annotation-repository.ts index 45504be..82a9de6 100644 --- a/packages/connect/src/annotation-repository.ts +++ b/packages/connect/src/annotation-repository.ts @@ -1,4 +1,4 @@ -import { annotationPathsForSource, annotationRecordsAt } from "./annotation-query.js"; +import { annotationCandidatesForSource, annotationRecordsAt } from "./annotation-query.js"; import { AnnotationRecordCache } from "./annotation-record-cache.js"; import * as sources from "./annotation-source.js"; import { annotationFromDocument, annotationFrontmatter } from "./mapping.js"; @@ -11,7 +11,14 @@ import { } from "./repository-client.js"; import type { ReaderConnectClient } from "./repository-client.js"; -import type { DeletePreflightResult, RecordDocument, QueryRecord } from "@mdbase-dev/connect"; +import type { + DeletePreflightResult, + QueryRecord, + QueryMetadataRecord, + QueryPage, + QueryMetadataPage, + ReadManyRecord, +} from "@mdbase-dev/connect"; import type { Annotation, AnnotationListOptions, @@ -60,9 +67,16 @@ export class ConnectAnnotationRepository implements AnnotationRepository { collection: CollectionId, options: AnnotationListOptions = {}, ): Promise { + if (options.paths?.size === 0) { + return []; + } // Keep the first batches responsive; grow publication intervals as the list grows. // Hydration stays bounded even when cumulative UI snapshots become less frequent. const annotations: Annotation[] = []; + const batchReads = outcomeValue( + await this.client.supportsAuthorityFeature("read-many-documents-v1", options), + "discover annotation batches", + ); let published = 0; const publish = (): void => { if (options.onProgress) { @@ -75,19 +89,34 @@ export class ConnectAnnotationRepository implements AnnotationRepository { options.signal?.throwIfAborted(); // A small first batch minimizes time to content; larger later batches limit rerenders. const batchSize = annotations.length === 0 ? 16 : 64; - const batch = await mapConcurrent( - page.slice(offset, offset + batchSize), - readerConnectBulkConcurrency, - async (entry) => { - const document = await this.#records.read(entry.path, options, options.refresh); - try { - return await this.#map(collection, document, entry.source); - } catch { - // Invalid records do not belong in the overview. - return null; - } - }, - ); + const entries = page.slice(offset, offset + batchSize); + const matches = batchReads + ? await annotationRecordsAt( + this.client, + entries.map(({ path }) => path), + options, + ) + : new Map(); + const batch = await mapConcurrent(entries, readerConnectBulkConcurrency, async (entry) => { + if (batchReads && !matches.has(entry.path)) { + return null; + } + const document = + revisionedAnnotation(matches.get(entry.path)) ?? + (await this.#records.read(entry.path, options, options.refresh)); + try { + return await this.#map( + collection, + document, + document.effectiveFrontmatter["source"] === entry.reference + ? entry.source + : undefined, + ); + } catch { + // Invalid records do not belong in the overview. + return null; + } + }); options.signal?.throwIfAborted(); annotations.push(...batch.filter((annotation) => annotation !== null)); offset += batchSize; @@ -110,23 +139,45 @@ export class ConnectAnnotationRepository implements AnnotationRepository { source: SourceId, options: ReaderRequestOptions = {}, ): Promise { - const paths = await annotationPathsForSource(this.client, source, options); + const candidates = await annotationCandidatesForSource(this.client, source, options); + const paths = [...candidates.keys()]; options.signal?.throwIfAborted(); - // Without the records' current content every annotation is read, as before. - const matches = await annotationRecordsAt(this.client, paths, options).catch(() => { - options.signal?.throwIfAborted(); - return new Map(); - }); + if (paths.length === 0) { + return []; + } + const batchReads = outcomeValue( + await this.client.supportsAuthorityFeature("read-many-documents-v1", options), + "discover annotation batches", + ); + const pending = annotationRecordsAt(this.client, paths, options); + const matches = batchReads + ? await pending + : await pending.catch(() => { + options.signal?.throwIfAborted(); + return new Map(); + }); const annotations = await mapConcurrent(paths, readerConnectBulkConcurrency, async (path) => { - // Query results carry no revision; read only records the cache cannot vouch for. + if (batchReads && !matches.has(path)) { + return null; + } + // Qualified batches pair content with its revision; legacy rows still need revalidation. const match = matches.get(path); const document = - (match && this.#records.current(match)) ?? (await this.#records.read(path, options, true)); - // The query matched these by the target of their link, so the source is known. - return this.#map(collection, document, source); + revisionedAnnotation(match) ?? + (match && this.#records.current(match)) ?? + (await this.#records.read(path, options, true)); + // Discovery and hydration are separate reads: re-resolve a changed source reference. + return this.#map( + collection, + document, + document.effectiveFrontmatter["source"] === candidates.get(path) ? source : undefined, + ); }); options.signal?.throwIfAborted(); - return annotations.filter((annotation) => annotation.sourceId === source); + return annotations.filter( + (annotation): annotation is Annotation => + annotation !== null && annotation.sourceId === source, + ); } async create(annotation: Annotation, _idempotencyKey: MutationId): Promise { @@ -228,7 +279,7 @@ export class ConnectAnnotationRepository implements AnnotationRepository { /** Maps a record; `source` is its resolved source when the caller already knows it. */ async #map( collection: CollectionId, - record: RecordDocument, + record: Parameters[1], source?: SourceId, ): Promise { return annotationFromDocument( @@ -251,8 +302,8 @@ export class ConnectAnnotationRepository implements AnnotationRepository { /** Every annotation with the source mdbase resolves its link to, in one query. */ async #annotationSources( options: ReaderRequestOptions, - ): Promise<{ readonly path: string; readonly source: SourceId }[]> { - const listed: { path: string; source: SourceId }[] = []; + ): Promise<{ readonly path: string; readonly source: SourceId; readonly reference: unknown }[]> { + const listed: { path: string; source: SourceId; reference: unknown }[] = []; for await (const page of this.#annotationSourcePages(options)) { listed.push(...page); } @@ -261,28 +312,40 @@ export class ConnectAnnotationRepository implements AnnotationRepository { async *#annotationSourcePages( options: AnnotationListOptions, - ): AsyncGenerator<{ readonly path: string; readonly source: SourceId }[]> { + ): AsyncGenerator< + { readonly path: string; readonly source: SourceId; readonly reference: unknown }[] + > { + if (options.paths?.size === 0) { + return; + } + const metadata = outcomeValue( + await this.client.supportsAuthorityFeature("query-metadata-v1", options), + "discover metadata queries", + ); for (const scope of annotationQueryScopes(options.paths)) { const selected = scope ? new Set(scope) : null; options.signal?.throwIfAborted(); - for await (const outcome of this.client.queryPages( - sources.withResolvedSource({ - types: ["reader-annotation"], - select: ["id", "source"], - frontmatterMode: "effective", - ...(scope - ? { where: scope.map((path) => `file.path == ${JSON.stringify(path)}`).join(" || ") } - : {}), - }), - { - ...(options.signal ? { signal: options.signal } : {}), - ...(options.replaceableFamily ? { replaceableFamily: options.replaceableFamily } : {}), - pageSize: 500, - }, - )) { - yield outcomeValue(outcome, "query annotations").results.flatMap((record) => - annotationSourceEntry(record, selected), - ); + const input = sources.withResolvedSource({ + types: ["reader-annotation"], + select: ["id", "source"], + frontmatterMode: "effective", + ...(scope + ? { where: scope.map((path) => `file.path == ${JSON.stringify(path)}`).join(" || ") } + : {}), + }); + const paging = { + ...(options.signal ? { signal: options.signal } : {}), + ...(options.replaceableFamily ? { replaceableFamily: options.replaceableFamily } : {}), + pageSize: 500, + }; + const pages = metadata + ? this.client.queryPages({ ...input, output: "metadata", includeBody: false }, paging) + : this.client.queryPages(input, paging); + for await (const outcome of pages) { + yield outcomeValue( + outcome, + "query annotations", + ).results.flatMap((record) => annotationSourceEntry(record, selected)); } } } @@ -302,16 +365,31 @@ function annotationQueryScopes(paths?: ReadonlySet): (string[] | null)[] } function annotationSourceEntry( - record: QueryRecord, + record: QueryRecord | QueryMetadataRecord, selected: ReadonlySet | null, -): { path: string; source: SourceId }[] { - const fields = record.effectiveFrontmatter ?? record.frontmatter; +): { path: string; source: SourceId; reference: unknown }[] { + const fields = + "file" in record ? (record.effectiveFrontmatter ?? record.frontmatter) : record.values; const source = sources.annotationSourceFromResult(record); return sources.stringField(fields?.["id"]) && source && (!selected || selected.has(record.path)) - ? [{ path: record.path, source }] + ? [{ path: record.path, source, reference: fields?.["source"] }] : []; } +function revisionedAnnotation( + record: ReadManyRecord | undefined, +): Parameters[1] | null { + return record?.revision && record.frontmatter && record.effectiveFrontmatter + ? { + path: record.path, + revision: record.revision, + frontmatter: record.frontmatter, + effectiveFrontmatter: record.effectiveFrontmatter, + ...(record.body === undefined ? {} : { body: record.body }), + } + : null; +} + function canonicalIdentity(annotation: Annotation): { readonly path: string; readonly recordRevision: NonNullable; diff --git a/packages/connect/src/annotation-source-listing.test.ts b/packages/connect/src/annotation-source-listing.test.ts index fb1217d..83fc7f5 100644 --- a/packages/connect/src/annotation-source-listing.test.ts +++ b/packages/connect/src/annotation-source-listing.test.ts @@ -16,6 +16,12 @@ import type { RecordDocument, } from "@mdbase-dev/connect"; +const legacyAuthorityFeatures = { + supportsAuthorityFeature: vi.fn(() => + Promise.resolve({ ok: true as const, value: false, diagnostics: [] }), + ), +}; + interface StoredAnnotation { frontmatter: JsonObject; body: string; @@ -42,7 +48,9 @@ function authority( links: Readonly>, onQuery: (input: QueryInput) => void = () => undefined, ): { - queryPages: Mock; + queryPages: Mock< + (input: QueryInput, options?: QueryPagesOptions) => AsyncGenerator> + >; readMany: Mock; read: Mock<(input: ReadInput) => Promise>>; client: ReaderConnectClient; @@ -142,7 +150,12 @@ function authority( queryPages, readMany, read, - client: { queryPages, readMany, read } as unknown as ReaderConnectClient, + client: { + ...legacyAuthorityFeatures, + queryPages, + readMany, + read, + } as unknown as ReaderConnectClient, }; } diff --git a/packages/connect/src/annotation-source.test.ts b/packages/connect/src/annotation-source.test.ts index 0462058..2bab421 100644 --- a/packages/connect/src/annotation-source.test.ts +++ b/packages/connect/src/annotation-source.test.ts @@ -7,6 +7,12 @@ import { ConnectAnnotationRepository } from "./annotation-repository.js"; import type { ReaderConnectClient } from "./repository-client.js"; import type { ConnectOutcome, QueryInput, RecordDocument } from "@mdbase-dev/connect"; +const legacyAuthorityFeatures = { + supportsAuthorityFeature: vi.fn(() => + Promise.resolve({ ok: true as const, value: false, diagnostics: [] }), + ), +}; + const collection = collectionId("reading"); const ok = (value: T): ConnectOutcome => ({ ok: true, value, diagnostics: [] }); @@ -47,6 +53,7 @@ function collectionOf( }; const queries: QueryInput[] = []; const client = { + ...legacyAuthorityFeatures, read: ({ path }: { path: string }) => Promise.resolve(ok(byPath.get(path)!)), readMany: (paths: readonly string[]) => Promise.resolve( diff --git a/packages/connect/src/annotation-source.ts b/packages/connect/src/annotation-source.ts index c5a07f6..c9d7f39 100644 --- a/packages/connect/src/annotation-source.ts +++ b/packages/connect/src/annotation-source.ts @@ -3,7 +3,13 @@ import { sourceId, type ReaderRequestOptions, type SourceId } from "@mdbase-read import { outcomeValue } from "./repository-client.js"; import type { ReaderConnectClient } from "./repository-client.js"; -import type { QueryInput, QueryRecord } from "@mdbase-dev/connect"; +import type { + QueryInput, + QueryRecord, + QueryMetadataRecord, + QueryPage, + QueryMetadataPage, +} from "@mdbase-dev/connect"; /** * mdbase resolves an annotation's `source` link, so Reader never guesses a link's target from its @@ -32,13 +38,18 @@ export function withResolvedSource(input: QueryInput): QueryInput { * reference that resolves to no record, the ID it was written as. Undefined for a broken link. */ export function annotationSourceFromResult( - record: Pick, + record: + Pick | QueryMetadataRecord, ): SourceId | undefined { const resolved = stringField(record.values?.[resolvedSource]); if (resolved) { return sourceId(resolved); } - return legacySourceId((record.effectiveFrontmatter ?? record.frontmatter)?.["source"]); + return legacySourceId( + "frontmatter" in record || "effectiveFrontmatter" in record + ? (record.effectiveFrontmatter ?? record.frontmatter)?.["source"] + : record.values?.["source"], + ); } /** Resolves one annotation's source through mdbase. */ @@ -47,16 +58,25 @@ export async function resolveAnnotationSource( path: string, options: ReaderRequestOptions = {}, ): Promise { - for await (const outcome of client.queryPages( - withResolvedSource({ - types: ["reader-annotation"], - where: `file.path == ${JSON.stringify(path)}`, - select: ["source"], - frontmatterMode: "effective", - }), - { ...options, firstPageSize: 1, pageSize: 1 }, - )) { - const [record] = outcomeValue(outcome, "resolve annotation source").results; + const input = withResolvedSource({ + types: ["reader-annotation"], + where: `file.path == ${JSON.stringify(path)}`, + select: ["source"], + frontmatterMode: "effective", + }); + const metadata = outcomeValue( + await client.supportsAuthorityFeature("query-metadata-v1", options), + "discover metadata queries", + ); + const paging = { ...options, firstPageSize: 1, pageSize: 1 }; + const pages = metadata + ? client.queryPages({ ...input, output: "metadata", includeBody: false }, paging) + : client.queryPages(input, paging); + for await (const outcome of pages) { + const [record] = outcomeValue( + outcome, + "resolve annotation source", + ).results; const source = record ? annotationSourceFromResult(record) : undefined; if (source) { return source; diff --git a/packages/connect/src/collection-files.test.ts b/packages/connect/src/collection-files.test.ts index 1d7750a..b5feb7e 100644 --- a/packages/connect/src/collection-files.test.ts +++ b/packages/connect/src/collection-files.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it, vi } from "vitest"; import { ConnectCollectionFileRepository } from "./collection-files.js"; +import type { ReaderFileClient } from "./documents.js"; import type { CollectionFileDescriptor } from "@mdbase-dev/connect"; import type { Mock } from "vitest"; @@ -17,7 +18,7 @@ describe("bounded collection file reads", () => { active--; return new Blob(["PNG"]); }); - const { repository, list } = readFixture(download); + const { repository, stat } = readFixture(download); const files = await Promise.all( Array.from({ length: 11 }, (_, index) => repository.read(collectionId("reading"), `files/${String(index)}.png`), @@ -25,7 +26,7 @@ describe("bounded collection file reads", () => { ); expect(files).toHaveLength(11); expect(maximum).toBe(2); - expect(list.mock.calls.length).toBeLessThanOrEqual(2); + expect(stat).toHaveBeenCalledTimes(11); expect(download).toHaveBeenCalledTimes(11); }); @@ -94,12 +95,14 @@ describe("bounded collection file reads", () => { function readFixture(download: (file: CollectionFileDescriptor) => Promise): { repository: ConnectCollectionFileRepository; - list: Mock<() => AsyncGenerator>; + stat: Mock; } { - const list = vi.fn(async function* (): AsyncGenerator { - await Promise.resolve(); - for (let index = 0; index < 11; index++) { - yield { + const stat = vi.fn((target) => { + const index = Number(target.path?.split("/").at(-1)?.split(".")[0]); + return Promise.resolve({ + ok: true, + diagnostics: [], + value: { fileId: `file-${String(index)}`, path: `files/${String(index)}.png`, revision: "file-revision", @@ -108,14 +111,14 @@ function readFixture(download: (file: CollectionFileDescriptor) => Promise mediaType: "image/png", mediaClass: "image", modifiedAt: "2026-09-24T00:00:00Z", - }; - } + }, + }); }); - return { repository: new ConnectCollectionFileRepository({ list, download }), list }; + return { repository: new ConnectCollectionFileRepository({ stat, download }), stat }; } describe("ConnectCollectionFileRepository", () => { - it("exports an exact original revision and reuses its folder index", async () => { + it("exports exact revisions using fresh point metadata rather than a folder index", async () => { const revision: `sha256:${string}` = `sha256:${"a".repeat(64)}`; const cropRevision: `sha256:${string}` = `sha256:${"b".repeat(64)}`; const list = vi.fn(async function* (): AsyncGenerator { @@ -144,7 +147,15 @@ describe("ConnectCollectionFileRepository", () => { const download = vi.fn((descriptor: { readonly path: string }) => Promise.resolve(new Blob([descriptor.path.endsWith("pdf") ? "PDF!" : "PNG"])), ); - const repository = new ConnectCollectionFileRepository({ list, download }); + const stat = vi.fn(async (target) => { + for await (const file of list()) { + if (file.path === target.path) { + return { ok: true, value: file, diagnostics: [] }; + } + } + return { ok: true, value: null, diagnostics: [] }; + }); + const repository = new ConnectCollectionFileRepository({ stat, download }); await expect( repository.read( @@ -158,7 +169,10 @@ describe("ConnectCollectionFileRepository", () => { bytes: new Uint8Array([80, 68, 70, 33]), }); await repository.read(collectionId("reading"), "files/reading/crop.png"); - expect(list).toHaveBeenCalledOnce(); + expect(stat.mock.calls.map(([target]) => target)).toEqual([ + { path: "files/reading/paper.pdf" }, + { path: "files/reading/crop.png" }, + ]); expect(download).toHaveBeenCalledTimes(2); }); @@ -178,7 +192,10 @@ describe("ConnectCollectionFileRepository", () => { }; }; const repository = new ConnectCollectionFileRepository({ - list, + stat: async () => { + const result = await list().next(); + return { ok: true, value: result.value ?? null, diagnostics: [] }; + }, download: vi.fn(), }); diff --git a/packages/connect/src/collection-files.ts b/packages/connect/src/collection-files.ts index 6e75fda..3737f4d 100644 --- a/packages/connect/src/collection-files.ts +++ b/packages/connect/src/collection-files.ts @@ -1,6 +1,7 @@ import { ConnectDocumentError, type ReaderFileClient } from "./documents.js"; +import { outcomeValue } from "./repository-client.js"; -import type { CollectionFileDescriptor, MdbaseConnection } from "@mdbase-dev/connect"; +import type { MdbaseConnection } from "@mdbase-dev/connect"; import type { CollectionFileRepository, CollectionId, @@ -14,8 +15,6 @@ import type { const MAX_ACTIVE_READS = 2; export class ConnectCollectionFileRepository implements CollectionFileRepository { - readonly #descriptorsByPath = new Map(); - readonly #loadedFolders = new Set(); readonly #readWaiters: (() => void)[] = []; #activeReads = 0; @@ -79,7 +78,7 @@ export class ConnectCollectionFileRepository implements CollectionFileRepository options: ReaderRequestOptions, ): Promise { const path = portableFilePath(file); - const descriptor = await this.#find(path, options); + const descriptor = outcomeValue(await this.files.stat({ path }, options), "find export file"); if (!descriptor) { throw new ConnectDocumentError("export file", "file_not_found"); } @@ -95,24 +94,6 @@ export class ConnectCollectionFileRepository implements CollectionFileRepository bytes: new Uint8Array(await blob.arrayBuffer()), }; } - - async #find( - path: string, - options: ReaderRequestOptions, - ): Promise { - const cached = this.#descriptorsByPath.get(path); - if (cached) { - return cached; - } - const folder = parentFolder(path); - if (!this.#loadedFolders.has(folder)) { - for await (const descriptor of this.files.list({ folder, pageSize: 100, ...options })) { - this.#descriptorsByPath.set(descriptor.path, descriptor); - } - this.#loadedFolders.add(folder); - } - return this.#descriptorsByPath.get(path) ?? null; - } } function abortableRead(pending: Promise, signal?: AbortSignal): Promise { @@ -141,11 +122,6 @@ function portableFilePath(link: string): string { return wikilink?.[1] ?? link.trim(); } -function parentFolder(path: string): string { - const separator = path.lastIndexOf("/"); - return separator > 0 ? path.slice(0, separator) : ""; -} - function mediaTypeFromPath(path: string): string { const extension = path.split(".").at(-1)?.toLocaleLowerCase(); if (extension === "pdf") { diff --git a/packages/connect/src/documents.test.ts b/packages/connect/src/documents.test.ts index d2f1faf..0fe3a71 100644 --- a/packages/connect/src/documents.test.ts +++ b/packages/connect/src/documents.test.ts @@ -1,9 +1,10 @@ +import { connectFailure, connectProblem } from "@mdbase-dev/connect/advanced"; import { collectionId, fileId, fileRevision, type DocumentTarget } from "@mdbase-reader/core"; -import { describe, expect, it, vi } from "vitest"; +import { describe, expect, it, vi, type Mock } from "vitest"; import { ConnectDocumentRepository } from "./documents.js"; -import type { ConnectDocumentError, ReaderFileClient } from "./documents.js"; +import type { ReaderFileClient } from "./documents.js"; import type { CollectionFileDescriptor } from "@mdbase-dev/connect"; const descriptor: CollectionFileDescriptor = { @@ -16,217 +17,145 @@ const descriptor: CollectionFileDescriptor = { mediaClass: "pdf", modifiedAt: "2026-08-09T12:00:00Z", }; - -function files(items: readonly CollectionFileDescriptor[]): ReaderFileClient { - return { - async *list(): AsyncIterable { - await Promise.resolve(); - yield* items; - }, - download: vi.fn().mockResolvedValue(new Blob(["pdf"], { type: "application/pdf" })), - }; -} - const target: DocumentTarget = { fileId: fileId("file-01"), file: "[[files/example.pdf]]", revision: fileRevision(descriptor.contentDigest), }; +const collection = collectionId("reading"); +function files(items: readonly CollectionFileDescriptor[]): { + stat: Mock; + download: Mock; +} { + return { + stat: vi.fn((target) => + Promise.resolve({ + ok: true, + diagnostics: [], + value: + items.find((item) => + "fileId" in target ? item.fileId === target.fileId : item.path === target.path, + ) ?? null, + }), + ), + download: vi.fn().mockResolvedValue(new Blob(["pdf"], { type: "application/pdf" })), + }; +} +function urls(): { create: Mock<() => string>; revoke: Mock<(url: string) => void> } { + let sequence = 0; + return { create: vi.fn(() => `blob:reader-${String(++sequence)}`), revoke: vi.fn() }; +} describe("ConnectDocumentRepository", () => { - it("downloads the exact requested revision and retains its object URL for reuse", async () => { + it("refreshes metadata on each open while reusing leased object URLs for unchanged bytes", async () => { const client = files([descriptor]); - const urls = { create: vi.fn(() => "blob:reader-file"), revoke: vi.fn() }; - const repository = new ConnectDocumentRepository(client, urls); - const handle = await repository.open(collectionId("reading"), target); - - expect(handle).toMatchObject({ + const objectUrls = urls(); + const repository = new ConnectDocumentRepository(client, objectUrls); + const first = await repository.open(collection, target); + expect(first).toMatchObject({ fileId: "file-01", revision: descriptor.contentDigest, mediaType: "application/pdf", - url: "blob:reader-file", }); - expect(client.download).toHaveBeenCalledWith(descriptor); - await handle.close(); - await handle.close(); - expect(urls.revoke).not.toHaveBeenCalled(); - - const reopened = await repository.open(collectionId("reading"), target); - expect(reopened.url).toBe("blob:reader-file"); - expect(client.download).toHaveBeenCalledOnce(); - await reopened.close(); - repository.dispose(); - expect(urls.revoke).toHaveBeenCalledTimes(1); - }); - - it("opens the current bytes and reports their revision when source metadata is stale", async () => { - const changed = { ...descriptor, contentDigest: `sha256:${"b".repeat(64)}` as const }; - const client = files([changed]); - const repository = new ConnectDocumentRepository(client); - const handle = await repository.open(collectionId("reading"), target); - expect(handle.revision).toBe(changed.contentDigest); - expect(client.download).toHaveBeenCalledWith(changed); - await handle.close(); - }); - - it("does not reuse stale file metadata after the same file ID changes", async () => { - let current = descriptor; - const client: ReaderFileClient = { - async *list() { - await Promise.resolve(); - yield current; - }, - download: vi.fn().mockResolvedValue(new Blob(["pdf"], { type: "application/pdf" })), - }; - const repository = new ConnectDocumentRepository(client); - const first = await repository.open(collectionId("reading"), target); await first.close(); - current = { ...descriptor, contentDigest: `sha256:${"b".repeat(64)}` }; - const next = await repository.open(collectionId("reading"), target); - expect(next.revision).toBe(current.contentDigest); - expect(client.download).toHaveBeenCalledTimes(2); + await first.close(); + const next = await repository.open(collection, target); + expect(next.url).toBe(first.url); + expect(client.stat).toHaveBeenCalledTimes(2); + expect(client.stat).toHaveBeenCalledWith({ fileId: target.fileId }, {}); + expect(client.download).toHaveBeenCalledExactlyOnceWith(descriptor); + expect(objectUrls.revoke).not.toHaveBeenCalled(); await next.close(); + repository.dispose(); + expect(objectUrls.revoke).toHaveBeenCalledExactlyOnceWith(first.url); }); - it("scopes file discovery to the selected folder and refreshes descriptors on each open", async () => { - const list = vi.fn(async function* (options?: { - readonly folder?: string; - }): AsyncIterable { - await Promise.resolve(); - expect(options).toEqual({ folder: "files/example", pageSize: 100 }); - yield { ...descriptor, path: "files/example/article.pdf" }; - }); - const client = { - list, - download: vi.fn().mockResolvedValue(new Blob(["pdf"], { type: "application/pdf" })), - } satisfies ReaderFileClient; - const repository = new ConnectDocumentRepository(client, { - create: vi.fn(() => "blob:reader-file"), - revoke: vi.fn(), - }); - const nestedTarget = { ...target, file: "[[files/example/article.pdf]]" }; - - const first = await repository.open(collectionId("reading"), nestedTarget); + it("opens current bytes after the same ID moves or changes, even with stale source metadata", async () => { + const client = files([descriptor]); + const repository = new ConnectDocumentRepository(client, urls()); + const first = await repository.open(collection, target); await first.close(); - const second = await repository.open(collectionId("reading"), nestedTarget); - await second.close(); - - expect(list).toHaveBeenCalledTimes(2); - }); - - it("stops listing the folder once the file ID is found", async () => { - const pulled: string[] = []; - const client: ReaderFileClient = { - async *list() { - for (const item of [ - { ...descriptor, fileId: "file-00", path: "files/other.pdf" }, - descriptor, - { ...descriptor, fileId: "file-02", path: "files/later.pdf" }, - ]) { - await Promise.resolve(); - pulled.push(item.fileId); - yield item; - } - }, - download: vi.fn().mockResolvedValue(new Blob(["pdf"], { type: "application/pdf" })), + const changed = { + ...descriptor, + path: "elsewhere/moved.pdf", + contentDigest: `sha256:${"b".repeat(64)}` as const, }; - const handle = await new ConnectDocumentRepository(client).open( - collectionId("reading"), - target, - ); - - expect(pulled).toEqual(["file-00", "file-01"]); - await handle.close(); + client.stat.mockImplementation(files([changed]).stat); + const next = await repository.open(collection, target); + expect(next.revision).toBe(changed.contentDigest); + expect(client.download).toHaveBeenLastCalledWith(changed); + expect(client.download).toHaveBeenCalledTimes(2); + await next.close(); }); -}); -describe("ConnectDocumentRepository recovery and caching", () => { - it("recovers a migrated file reference only when its path and digest are exact", async () => { - const migrated = { - ...descriptor, - fileId: "file-02", - path: "files/example.pdf", - } satisfies CollectionFileDescriptor; + it("recovers a migrated ID by exact path and digest only", async () => { + const migrated = { ...descriptor, fileId: "file-02" }; const client = files([migrated]); - const repository = new ConnectDocumentRepository(client, { - create: vi.fn(() => "blob:reader-file"), - revoke: vi.fn(), - }); - - const handle = await repository.open(collectionId("reading"), target); - + const handle = await new ConnectDocumentRepository(client, urls()).open(collection, target); expect(handle.fileId).toBe("file-02"); + expect(client.stat.mock.calls.map(([target]) => target)).toEqual([ + { fileId: "file-01" }, + { path: descriptor.path }, + ]); expect(client.download).toHaveBeenCalledWith(migrated); await handle.close(); }); - it("rejects a path match when the referenced digest is stale", async () => { - const migrated = { - ...descriptor, - fileId: "file-02", - path: "files/example.pdf", - contentDigest: `sha256:${"b".repeat(64)}` as const, - } satisfies CollectionFileDescriptor; - const repository = new ConnectDocumentRepository(files([migrated])); + it("rejects a migrated path when the referenced digest is stale", async () => { + const client = files([ + { ...descriptor, fileId: "file-02", contentDigest: `sha256:${"b".repeat(64)}` }, + ]); + await expect(new ConnectDocumentRepository(client).open(collection, target)).rejects.toThrow( + "file_not_found", + ); + expect(client.download).not.toHaveBeenCalled(); + }); - await expect(repository.open(collectionId("reading"), target)).rejects.toEqual( - expect.objectContaining>({ - message: "mdbase Connect could not open document: file_not_found", - }), + it("reports a missing descriptor", async () => { + await expect(new ConnectDocumentRepository(files([])).open(collection, target)).rejects.toThrow( + "file_not_found", ); }); - it("forwards cancellation through descriptor lookup and download", async () => { - const controller = new AbortController(); - const list = vi.fn(async function* (options?: { - readonly signal?: AbortSignal; - }): AsyncIterable { - await Promise.resolve(); - expect(options?.signal).toBe(controller.signal); - yield descriptor; - }); - const download = vi.fn().mockResolvedValue(new Blob(["pdf"], { type: "application/pdf" })); - const repository = new ConnectDocumentRepository( - { list, download }, - { - create: vi.fn(() => "blob:reader-file"), - revoke: vi.fn(), - }, + it("does not downgrade stat failures to path lookup or download", async () => { + const client = files([descriptor]); + client.stat.mockResolvedValue(connectFailure(connectProblem("access_denied", "Denied"))); + await expect(new ConnectDocumentRepository(client).open(collection, target)).rejects.toThrow( + "Denied", ); + expect(client.stat).toHaveBeenCalledOnce(); + expect(client.download).not.toHaveBeenCalled(); + }); - const handle = await repository.open(collectionId("reading"), target, { - signal: controller.signal, + it("forwards cancellation through stat and download", async () => { + const signal = new AbortController().signal; + const client = files([descriptor]); + const handle = await new ConnectDocumentRepository(client, urls()).open(collection, target, { + signal, }); - - expect(download).toHaveBeenCalledWith(descriptor, { signal: controller.signal }); + expect(client.stat).toHaveBeenCalledWith({ fileId: target.fileId }, { signal }); + expect(client.download).toHaveBeenCalledWith(descriptor, { signal }); await handle.close(); }); - it("evicts the least recently used closed document but never an open handle", async () => { - const secondDescriptor = { + it("evicts closed URLs but never an open handle", async () => { + const second = { ...descriptor, fileId: "file-02", path: "files/second.pdf", contentDigest: `sha256:${"b".repeat(64)}` as const, - } satisfies CollectionFileDescriptor; - const client = files([descriptor, secondDescriptor]); - let urlSequence = 0; - const urls = { - create: vi.fn(() => `blob:reader-${String(++urlSequence)}`), - revoke: vi.fn(), }; - const repository = new ConnectDocumentRepository(client, urls, 1); - const first = await repository.open(collectionId("reading"), target); - const second = await repository.open(collectionId("reading"), { - fileId: fileId(secondDescriptor.fileId), - file: secondDescriptor.path, - revision: fileRevision(secondDescriptor.contentDigest), + const objectUrls = urls(); + const repository = new ConnectDocumentRepository(files([descriptor, second]), objectUrls, 1); + const first = await repository.open(collection, target); + const next = await repository.open(collection, { + fileId: fileId(second.fileId), + file: second.path, + revision: fileRevision(second.contentDigest), }); - - expect(urls.revoke).not.toHaveBeenCalled(); - await second.close(); - expect(urls.revoke).toHaveBeenCalledTimes(1); - expect(urls.revoke).toHaveBeenCalledWith(second.url); + expect(objectUrls.revoke).not.toHaveBeenCalled(); + await next.close(); + expect(objectUrls.revoke).toHaveBeenCalledExactlyOnceWith(next.url); await first.close(); repository.dispose(); }); diff --git a/packages/connect/src/documents.ts b/packages/connect/src/documents.ts index c31f850..3bc7910 100644 --- a/packages/connect/src/documents.ts +++ b/packages/connect/src/documents.ts @@ -8,14 +8,13 @@ import { type DocumentTarget, } from "@mdbase-reader/core"; +import { outcomeValue } from "./repository-client.js"; + import type { CollectionFileDescriptor, MdbaseConnection } from "@mdbase-dev/connect"; export interface ReaderFileClient { - list(options?: { - readonly folder?: string; - readonly pageSize?: number; - readonly signal?: AbortSignal; - }): AsyncIterable; + // The SDK owns files-stat-v1 discovery and the legacy paginated-list fallback. + stat: MdbaseConnection["files"]["stat"]; download(file: CollectionFileDescriptor, options?: DocumentOpenOptions): Promise; } @@ -131,24 +130,19 @@ export class ConnectDocumentRepository implements DocumentRepository { ): Promise { // Look up current metadata on every open: a previously discovered descriptor may // describe bytes that have since been replaced at the same file ID. - const path = portableFilePath(target.file); - const folder = parentFolder(path); - let migratedByPath: CollectionFileDescriptor | null = null; - for await (const descriptor of this.files.list({ - ...(folder ? { folder } : {}), - pageSize: 100, - ...options, - })) { - // A file ID match always wins, so the rest of the folder need not be listed. - if (descriptor.fileId === target.fileId) { - return descriptor; - } - // A path alone is not enough to assume a new file ID is the same document. - if (descriptor.path === path && descriptor.contentDigest === target.revision) { - migratedByPath = descriptor; - } + const current = outcomeValue( + await this.files.stat({ fileId: target.fileId }, options), + "find document", + ); + if (current) { + return current; } - return migratedByPath; + const migrated = outcomeValue( + await this.files.stat({ path: portableFilePath(target.file) }, options), + "find migrated document", + ); + // A path alone is not enough to assume a new file ID is the same document. + return migrated?.contentDigest === target.revision ? migrated : null; } } @@ -168,11 +162,6 @@ function portableFilePath(link: string): string { return wikilink?.[1] ?? link.trim(); } -function parentFolder(path: string): string | undefined { - const separator = path.lastIndexOf("/"); - return separator > 0 ? path.slice(0, separator) : undefined; -} - function mediaTypeFromPath(path: string): string { const extension = path.split(".").at(-1)?.toLocaleLowerCase(); if (extension === "pdf") { diff --git a/packages/connect/src/repositories.test.ts b/packages/connect/src/repositories.test.ts index cfbd9bb..233d9a4 100644 --- a/packages/connect/src/repositories.test.ts +++ b/packages/connect/src/repositories.test.ts @@ -11,6 +11,12 @@ import { import type { ConnectOutcome, QueryPage, QueryResult, RecordDocument } from "@mdbase-dev/connect"; +const legacyAuthorityFeatures = { + supportsAuthorityFeature: vi.fn(() => + Promise.resolve({ ok: true as const, value: false, diagnostics: [] }), + ), +}; + function success(value: Value): ConnectOutcome { return { ok: true, value, diagnostics: [] }; } @@ -51,7 +57,12 @@ describe("ConnectSourceRepository", () => { yield* queryStream(result.value.results); } }; - const client = { query, queryPages, read } as unknown as ReaderConnectClient; + const client = { + ...legacyAuthorityFeatures, + query, + queryPages, + read, + } as unknown as ReaderConnectClient; const repository = new ConnectSourceRepository(client); const page = await repository.list({ collectionId: collectionId("reading"), limit: 20 }); const selected = await repository.get(collectionId("reading"), sourceId("src_01")); @@ -65,7 +76,9 @@ describe("ConnectSourceRepository", () => { expect(page.items[0]?.title).toBe("Crime and Punishment"); expect(selected?.body).toBe("Notes"); }); +}); +describe("Connect source transclusion", () => { it("makes source transclusion idempotent", async () => { const document = { path: "sources/crime.md", @@ -77,6 +90,7 @@ describe("ConnectSourceRepository", () => { file: {}, } satisfies RecordDocument; const client = { + ...legacyAuthorityFeatures, queryPages: vi.fn(() => queryStream([ { @@ -113,6 +127,7 @@ describe("ConnectSourceRepository", () => { file: {}, } satisfies RecordDocument; const client = { + ...legacyAuthorityFeatures, queryPages: vi.fn(() => queryStream([ { path: document.path, effectiveFrontmatter: document.frontmatter, types: [], file: {} }, @@ -159,6 +174,7 @@ describe("ConnectAnnotationRepository", () => { } satisfies RecordDocument; const create = vi.fn(() => Promise.resolve(success(document))); const repository = new ConnectAnnotationRepository({ + ...legacyAuthorityFeatures, create, } as unknown as ReaderConnectClient); @@ -239,6 +255,7 @@ describe("Connect annotation reads", () => { ), ); const repository = new ConnectAnnotationRepository({ + ...legacyAuthorityFeatures, queryPages, readMany, read, @@ -274,8 +291,8 @@ describe("Connect annotation reads", () => { ]); await repository.listForSource(collectionId("reading"), sourceId("src_02")); - // Each listing: an ID lookup and legacy-reference query; one unscoped index query. - expect(queryPages).toHaveBeenCalledTimes(5); + // Two discovery queries per listing, one index query, and a changed-reference revalidation. + expect(queryPages).toHaveBeenCalledTimes(6); expect(readMany).toHaveBeenCalledTimes(2); }); }); @@ -327,6 +344,7 @@ describe("Connect annotation listing fallback", () => { ), ); const repository = new ConnectAnnotationRepository({ + ...legacyAuthorityFeatures, queryPages, readMany, read, diff --git a/packages/connect/src/repository-client.test.ts b/packages/connect/src/repository-client.test.ts index 422bfbb..912bed5 100644 --- a/packages/connect/src/repository-client.test.ts +++ b/packages/connect/src/repository-client.test.ts @@ -102,11 +102,16 @@ describe("Reader Connect SDK integration", () => { expect(query).toHaveBeenCalledTimes(2); expect(create).toHaveBeenCalledOnce(); }); +}); +describe("Reader cursor lifetimes", () => { it("uses SDK cursor iteration and releases it on an early record match", async () => { let iteratorClosed = false; const queryPages = vi.fn(() => pages()); - const client = { queryPages } as unknown as ReturnType; + const client = { + supportsAuthorityFeature: vi.fn(() => Promise.resolve(success(false))), + queryPages, + } as unknown as ReturnType; async function* pages(): AsyncGenerator> { try { @@ -153,6 +158,7 @@ describe("Reader record lookup by ID", () => { file: {}, })); const client = { + supportsAuthorityFeature: vi.fn(() => Promise.resolve(success(false))), queryPages: vi.fn(async function* () { yield await Promise.resolve( success({ results, page: 0, offset: 0, loaded: 3, complete: true }), diff --git a/packages/connect/src/repository-client.ts b/packages/connect/src/repository-client.ts index 178518d..b432a3e 100644 --- a/packages/connect/src/repository-client.ts +++ b/packages/connect/src/repository-client.ts @@ -11,6 +11,8 @@ import type { ConnectProblem, MdbaseConnection, QueryInput, + QueryMetadataInput, + QueryMetadataPage, QueryPage, QueryResult, ReadInput, @@ -29,12 +31,20 @@ export interface ReaderQueryPagesOptions extends ReaderRequestOptions { } export interface ReaderConnectClient { + supportsAuthorityFeature( + id: string, + options?: ReaderRequestOptions, + ): Promise>; read(input: ReadInput, options?: ReaderRequestOptions): Promise>; readMany( paths: readonly string[], options?: ReadManyOptions, ): Promise>; query(input: QueryInput, options?: ReaderRequestOptions): Promise>; + queryPages( + input: QueryMetadataInput, + options?: ReaderQueryPagesOptions, + ): AsyncIterable>; queryPages( input: QueryInput, options?: ReaderQueryPagesOptions, @@ -123,18 +133,29 @@ export async function recordPathById( id: string, options: ReaderRequestOptions = {}, ): Promise { - for await (const outcome of client.queryPages( - { - where: `id == ${JSON.stringify(id)}`, - frontmatterMode: "effective", - }, - { ...options, firstPageSize: 50, pageSize: 50 }, - )) { - const page = outcomeValue(outcome, "query records"); + const input: QueryInput = { + where: `id == ${JSON.stringify(id)}`, + frontmatterMode: "effective", + }; + const metadata = outcomeValue( + await client.supportsAuthorityFeature("query-metadata-v1", options), + "discover metadata queries", + ); + const pages = metadata + ? client.queryPages( + { ...input, output: "metadata", includeBody: false, select: ["id"] }, + options, + ) + : client.queryPages(input, { ...options, firstPageSize: 50, pageSize: 50 }); + for await (const outcome of pages) { + const page = outcomeValue(outcome, "query records"); let match: string | null = null; - for (const { path, effectiveFrontmatter, frontmatter } of page.results) { - const candidate = (effectiveFrontmatter ?? frontmatter)?.["id"]; - match ??= candidate === id ? path : null; + for (const record of page.results) { + const candidate = + "file" in record + ? (record.effectiveFrontmatter ?? record.frontmatter)?.["id"] + : record.values["id"]; + match ??= candidate === id ? record.path : null; } if (match) { return match; @@ -149,7 +170,33 @@ export async function recordPathById( */ export function connectClient(connection: MdbaseConnection): ReaderConnectClient { const route = (): string => connection.route; + function queryPages( + input: QueryMetadataInput, + options?: ReaderQueryPagesOptions, + ): AsyncIterable>; + function queryPages( + input: QueryInput, + options?: ReaderQueryPagesOptions, + ): AsyncIterable>; + function queryPages( + input: QueryInput | QueryMetadataInput, + options?: ReaderQueryPagesOptions, + ): AsyncIterable> { + const paging = { + ...(options?.firstPageSize === undefined ? {} : { firstPageSize: options.firstPageSize }), + ...(options?.pageSize === undefined ? {} : { pageSize: options.pageSize }), + ...connectOptions(options), + }; + return readerDiagnostics.pages( + route, + input.output === "metadata" + ? connection.queryPages(input, paging) + : connection.queryPages(input, paging), + ); + } return { + supportsAuthorityFeature: (id, options) => + connection.supportsAuthorityFeature(id, connectOptions(options)), read: (input, options) => readerDiagnostics.measure("read", route, () => connection.read(input, connectOptions(options)), @@ -160,15 +207,7 @@ export function connectClient(connection: MdbaseConnection): ReaderConnectClient readerDiagnostics.measure("query", route, () => connection.query(input, connectOptions(options)), ), - queryPages: (input, options) => - readerDiagnostics.pages( - route, - connection.queryPages(input, { - ...(options?.firstPageSize === undefined ? {} : { firstPageSize: options.firstPageSize }), - ...(options?.pageSize === undefined ? {} : { pageSize: options.pageSize }), - ...connectOptions(options), - }), - ), + queryPages, create: (input) => readerDiagnostics.measure("create", route, () => connection.create(input)), update: (input) => readerDiagnostics.measure("update", route, () => connection.update(input)), preflightDelete: (input) => diff --git a/packages/connect/src/source-attachments.test.ts b/packages/connect/src/source-attachments.test.ts index 0997671..96b303d 100644 --- a/packages/connect/src/source-attachments.test.ts +++ b/packages/connect/src/source-attachments.test.ts @@ -13,12 +13,21 @@ import { ConnectSourceImportRepository } from "./source-imports.js"; import type { ReaderConnectClient } from "./repository-client.js"; import type { PlannedSourceAttachment } from "@mdbase-reader/core"; +const legacyAuthorityFeatures = { + supportsAuthorityFeature: vi.fn(() => + Promise.resolve({ ok: true as const, value: false, diagnostics: [] }), + ), +}; + describe("sources without documents", () => { it("creates the record with its kind and web address and no documents", async () => { const upload = vi.fn(); const create = vi.fn(() => Promise.resolve(success(recordDocument()))); const repository = new ConnectSourceImportRepository( - { create } as unknown as ReaderConnectClient, + { + ...legacyAuthorityFeatures, + create, + } as unknown as ReaderConnectClient, { upload }, ); @@ -58,7 +67,11 @@ describe("attaching a file to an existing source", () => { const read = vi.fn(() => Promise.resolve(success(existing))); const update = vi.fn(() => Promise.resolve(success(existing))); const repository = new ConnectSourceImportRepository( - { read, update } as unknown as ReaderConnectClient, + { + ...legacyAuthorityFeatures, + read, + update, + } as unknown as ReaderConnectClient, { upload }, ); @@ -97,7 +110,11 @@ describe("attaching a file to an existing source", () => { it("refuses when the stored bytes differ or the record path now holds another source", async () => { const mismatched = new ConnectSourceImportRepository( - { read: vi.fn(), update: vi.fn() } as unknown as ReaderConnectClient, + { + ...legacyAuthorityFeatures, + read: vi.fn(), + update: vi.fn(), + } as unknown as ReaderConnectClient, { upload: vi.fn(() => Promise.resolve(fileDescriptor({ contentDigest: "sha256:other" }))) }, ); await expect(mismatched.attachFile(attachment())).rejects.toThrow("did not match"); @@ -106,6 +123,7 @@ describe("attaching a file to an existing source", () => { const update = vi.fn(); const elsewhere = new ConnectSourceImportRepository( { + ...legacyAuthorityFeatures, read: vi.fn(() => Promise.resolve(success(moved))), update, } as unknown as ReaderConnectClient, diff --git a/packages/connect/src/source-body-repository.test.ts b/packages/connect/src/source-body-repository.test.ts index f37ecb6..4f87e9b 100644 --- a/packages/connect/src/source-body-repository.test.ts +++ b/packages/connect/src/source-body-repository.test.ts @@ -6,6 +6,12 @@ import { ConnectSourceRepository } from "./source-repository.js"; import type { ReaderConnectClient } from "./repository-client.js"; import type { ConnectOutcome, QueryPage, RecordDocument } from "@mdbase-dev/connect"; +const legacyAuthorityFeatures = { + supportsAuthorityFeature: vi.fn(() => + Promise.resolve({ ok: true as const, value: false, diagnostics: [] }), + ), +}; + function success(value: Value): ConnectOutcome { return { ok: true, value, diagnostics: [] }; } @@ -39,6 +45,7 @@ function repositoryFixture(): { ); return { repository: new ConnectSourceRepository({ + ...legacyAuthorityFeatures, queryPages, read, update, diff --git a/packages/connect/src/source-citation-repository.test.ts b/packages/connect/src/source-citation-repository.test.ts index c323769..dc67fcd 100644 --- a/packages/connect/src/source-citation-repository.test.ts +++ b/packages/connect/src/source-citation-repository.test.ts @@ -6,6 +6,12 @@ import { ConnectSourceRepository } from "./source-repository.js"; import type { ReaderConnectClient } from "./repository-client.js"; import type { ConnectOutcome, QueryPage, RecordDocument } from "@mdbase-dev/connect"; +const legacyAuthorityFeatures = { + supportsAuthorityFeature: vi.fn(() => + Promise.resolve({ ok: true as const, value: false, diagnostics: [] }), + ), +}; + function success(value: Value): ConnectOutcome { return { ok: true, value, diagnostics: [] }; } @@ -36,6 +42,7 @@ describe("Connect source citation metadata", () => { ), ); const repository = new ConnectSourceRepository({ + ...legacyAuthorityFeatures, queryPages, update, } as unknown as ReaderConnectClient); diff --git a/packages/connect/src/source-reading-repository.test.ts b/packages/connect/src/source-reading-repository.test.ts index b2f55a9..1eb1b52 100644 --- a/packages/connect/src/source-reading-repository.test.ts +++ b/packages/connect/src/source-reading-repository.test.ts @@ -6,6 +6,12 @@ import { ConnectSourceRepository } from "./source-repository.js"; import type { ReaderConnectClient } from "./repository-client.js"; import type { ConnectOutcome, QueryPage, RecordDocument } from "@mdbase-dev/connect"; +const legacyAuthorityFeatures = { + supportsAuthorityFeature: vi.fn(() => + Promise.resolve({ ok: true as const, value: false, diagnostics: [] }), + ), +}; + function success(value: Value): ConnectOutcome { return { ok: true, value, diagnostics: [] }; } @@ -37,6 +43,7 @@ describe("Connect source reading state", () => { const read = vi.fn(() => Promise.resolve(success(current))); const update = vi.fn(() => Promise.resolve(success(updated))); const repository = new ConnectSourceRepository({ + ...legacyAuthorityFeatures, queryPages, read, update, @@ -87,6 +94,7 @@ describe("Connect source reading state with a current caller revision", () => { read = vi.fn(), ): ConnectSourceRepository { return new ConnectSourceRepository({ + ...legacyAuthorityFeatures, queryPages: vi.fn(() => singleQueryPage({ path: "sources/crime.md", diff --git a/packages/connect/src/wave-b.test.ts b/packages/connect/src/wave-b.test.ts new file mode 100644 index 0000000..830de22 --- /dev/null +++ b/packages/connect/src/wave-b.test.ts @@ -0,0 +1,255 @@ +import { connectFailure, connectProblem } from "@mdbase-dev/connect/advanced"; +import { collectionId, sourceId } from "@mdbase-reader/core"; +import { describe, expect, it, vi, type Mock } from "vitest"; + +import { ConnectAnnotationRepository } from "./annotation-repository.js"; +import { connectClient, recordPathById } from "./repository-client.js"; + +import type { ReaderConnectClient } from "./repository-client.js"; +import type { + ConnectOutcome, + MdbaseConnection, + QueryInput, + QueryMetadataInput, + QueryMetadataPage, + ReadManyRecord, +} from "@mdbase-dev/connect"; + +function ok(value: T): ConnectOutcome { + return { ok: true, value, diagnostics: [] }; +} +function fixture(count = 2): { + client: ReaderConnectClient; + records: ReadManyRecord[]; + readMany: Mock; + queryPages: Mock< + (input: QueryInput | QueryMetadataInput) => AsyncGenerator> + >; + read: Mock; + repository: ConnectAnnotationRepository; +} { + const records = Array.from({ length: count }, (_, index): ReadManyRecord => { + const fields = { + id: `ann_${String(index)}`, + source: "src_1", + annotation_type: "note", + created_at: "2026-08-09T00:00:00Z", + }; + return { + path: `annotations/${String(index)}.md`, + revision: `rev-${String(index)}`, + types: ["reader-annotation"], + file: {}, + frontmatter: fields, + effectiveFrontmatter: fields, + body: `Body ${String(index)}`, + }; + }); + const supportsAuthorityFeature = vi.fn((id: string) => + Promise.resolve(ok(["query-metadata-v1", "read-many-documents-v1"].includes(id))), + ); + const queryPages = vi.fn(async function* ( + input: QueryInput | QueryMetadataInput, + ): AsyncGenerator> { + expect(input.output).toBe("metadata"); + const results = input.where?.startsWith("id ==") + ? [{ path: "sources/one.md", types: [], revision: "source-rev", values: { id: "src_1" } }] + : input.where?.includes('record["source"]') + ? [] + : records.map((record) => ({ + path: record.path, + revision: record.revision!, + types: record.types, + values: { + id: String(record.effectiveFrontmatter!["id"]), + source: "src_1", + reader_source_id: null, + }, + })); + yield await Promise.resolve( + ok({ + output: "metadata" as const, + results, + page: 0, + offset: 0, + loaded: results.length, + complete: true, + }), + ); + }); + const readMany = vi.fn((paths) => + Promise.resolve( + ok({ + results: paths.map((path) => { + const record = records.find((record) => record.path === path); + return record + ? { status: "found" as const, path, record } + : { status: "missing" as const, path }; + }), + errors: [], + }), + ), + ); + const read = vi.fn(() => { + throw new Error("Unexpected point read"); + }); + const client = { + supportsAuthorityFeature, + queryPages, + readMany, + read, + } as unknown as ReaderConnectClient; + return { + client, + records, + readMany, + queryPages, + read, + repository: new ConnectAnnotationRepository(client), + }; +} +const collection = collectionId("reading"); + +describe("wave B annotation hydration", () => { + it("replaces 130 point hydrations with three revision-bearing progressive batches", async () => { + const { repository, read, readMany } = fixture(130); + const lengths: number[] = []; + const annotations = await repository.listAll(collection, { + onProgress: (records) => lengths.push(records.length), + }); + expect(annotations).toHaveLength(130); + expect(lengths).toEqual([16, 80, 130]); + expect(readMany.mock.calls.map(([paths]) => paths.length)).toEqual([16, 64, 50]); + expect(annotations[0]).toMatchObject({ body: "Body 0", recordRevision: "rev-0" }); + expect(read).not.toHaveBeenCalled(); + }); + + it("hydrates a source list without N revision reads and installs changed content with its token", async () => { + const { repository, records, read, readMany } = fixture(); + await repository.listForSource(collection, sourceId("src_1")); + records[0] = { ...records[0]!, revision: "external-rev", body: "External edit" }; + const listed = await repository.listForSource(collection, sourceId("src_1")); + expect(listed[0]).toMatchObject({ body: "External edit", recordRevision: "external-rev" }); + expect(readMany).toHaveBeenCalledTimes(2); + expect(read).not.toHaveBeenCalled(); + }); + + it("omits a deleted record between discovery and hydration without a point-read retry", async () => { + const { repository, readMany, read } = fixture(1); + readMany.mockResolvedValue( + ok({ results: [{ status: "missing", path: "annotations/0.md" }], errors: [] }), + ); + expect(await repository.listForSource(collection, sourceId("src_1"))).toEqual([]); + expect(read).not.toHaveBeenCalled(); + }); + + it("re-evaluates a source reference changed during hydration", async () => { + const { repository, records, queryPages, read } = fixture(1); + const original = queryPages.getMockImplementation()!; + queryPages.mockImplementation(async function* (input) { + if (input.where?.startsWith("file.path ==")) { + yield ok({ + output: "metadata" as const, + results: [ + { + path: records[0]!.path, + revision: "moved-rev", + types: [], + values: { source: "src_2", reader_source_id: "src_2" }, + }, + ], + page: 0, + offset: 0, + loaded: 1, + complete: true, + }); + } else { + yield* original(input); + if (input.where?.includes("contains")) { + records[0] = { + ...records[0]!, + revision: "moved-rev", + effectiveFrontmatter: { ...records[0]!.effectiveFrontmatter, source: "src_2" }, + }; + } + } + }); + expect(await repository.listForSource(collection, sourceId("src_1"))).toEqual([]); + expect(read).not.toHaveBeenCalled(); + }); + + it("keeps advertised batch failures visible rather than silently downgrading to N reads", async () => { + const { repository, readMany, read } = fixture(1); + readMany.mockResolvedValue( + ok({ + results: [{ status: "error", path: "annotations/0.md", batch: 0 }], + errors: [ + { + batch: 0, + paths: ["annotations/0.md"], + failure: connectFailure(connectProblem("access_denied", "Denied")), + }, + ], + }), + ); + await expect(repository.listForSource(collection, sourceId("src_1"))).rejects.toThrow("Denied"); + expect(read).not.toHaveBeenCalled(); + }); + + it("retains point hydration when a batch row has no revision", async () => { + const { repository, records, read } = fixture(1); + const record = records[0]!; + read.mockResolvedValue( + ok({ + ...record, + revision: "point-rev", + frontmatter: record.frontmatter!, + effectiveFrontmatter: record.effectiveFrontmatter!, + }), + ); + delete record.revision; + expect((await repository.listForSource(collection, sourceId("src_1")))[0]?.recordRevision).toBe( + "point-rev", + ); + expect(read).toHaveBeenCalledOnce(); + }); +}); + +describe("wave B metadata discovery", () => { + it("uses selected values without a frontmatter envelope and releases the iterator on a match", async () => { + const { client, queryPages } = fixture(); + expect(await recordPathById(client, "src_1")).toBe("sources/one.md"); + expect(queryPages).toHaveBeenCalledWith( + expect.objectContaining({ output: "metadata", select: ["id"] }), + {}, + ); + }); + + it("propagates discovery failures before issuing any query", async () => { + const { client, queryPages } = fixture(); + client.supportsAuthorityFeature = () => + Promise.resolve(connectFailure(connectProblem("access_denied", "Discovery denied"))); + await expect(recordPathById(client, "src_1")).rejects.toThrow("Discovery denied"); + expect(queryPages).not.toHaveBeenCalled(); + }); + + it("delegates connection-lifetime feature discovery and metadata paging to the SDK", async () => { + const supportsAuthorityFeature = vi.fn(() => Promise.resolve(ok(true))); + const queryPages = vi.fn(async function* () { + yield await Promise.resolve( + ok({ output: "metadata", results: [], page: 0, offset: 0, loaded: 0, complete: true }), + ); + }); + const client = connectClient({ + supportsAuthorityFeature, + queryPages, + } as unknown as MdbaseConnection); + const signal = new AbortController().signal; + await client.supportsAuthorityFeature("query-metadata-v1", { signal }); + for await (const page of client.queryPages({ output: "metadata" }, { signal, pageSize: 100 })) { + expect(page.ok).toBe(true); + } + expect(supportsAuthorityFeature).toHaveBeenCalledWith("query-metadata-v1", { signal }); + expect(queryPages).toHaveBeenCalledWith({ output: "metadata" }, { signal, pageSize: 100 }); + }); +}); diff --git a/packages/markdown-editor/package.json b/packages/markdown-editor/package.json index cf8aa1c..95a67df 100644 --- a/packages/markdown-editor/package.json +++ b/packages/markdown-editor/package.json @@ -21,7 +21,7 @@ "@codemirror/state": "^6.5.2", "@codemirror/view": "^6.43.8", "@lezer/highlight": "^1.2.3", - "@mdbase-dev/ui": "0.1.0-beta.123", + "@mdbase-dev/ui": "0.1.0-beta.124", "@mdbase-reader/core": "workspace:*", "react": "^19.2.8" }, diff --git a/packages/ui/package.json b/packages/ui/package.json index a486888..4e8a581 100644 --- a/packages/ui/package.json +++ b/packages/ui/package.json @@ -14,7 +14,7 @@ "dependencies": { "@fontsource/atkinson-hyperlegible": "^5.3.0", "@fontsource/azeret-mono": "^5.3.0", - "@mdbase-dev/ui": "0.1.0-beta.123", + "@mdbase-dev/ui": "0.1.0-beta.124", "@phosphor-icons/react": "^2.1.10", "react": "^19.2.8" }, diff --git a/patches/@mdbase-dev__connect@0.1.0-beta.123.patch b/patches/@mdbase-dev__connect@0.1.0-beta.123.patch deleted file mode 100644 index 96b0621..0000000 --- a/patches/@mdbase-dev__connect@0.1.0-beta.123.patch +++ /dev/null @@ -1,20 +0,0 @@ -diff --git a/dist/application-session.js b/dist/application-session.js -index d564f053dcf938157ab17bd9d46af0434ee9648f..aca77a9c648fb7be5f34a3e2a206dcf695dd7570 100644 ---- a/dist/application-session.js -+++ b/dist/application-session.js -@@ -479,6 +479,15 @@ export class MdbaseApplicationSession { - timeStartupRead("setup-assessment", () => connection.assessCollectionSetup(initialInput, { ...options, signal: controller.signal })), verification - ]); - } -+ catch (error) { -+ // A route/selection refresh replaces this verification. Its cancelled -+ // assessment must not tear down startup's newer readiness work. -+ if (generation !== this.verificationGeneration && controller.signal.aborted -+ && (error === controller.signal.reason -+ || (error instanceof MdbaseConnectError && error.code === "operation_cancelled"))) -+ return; -+ throw error; -+ } - finally { - options?.signal?.removeEventListener("abort", abortAssessment); - controller.abort(); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9ff42e9..428326d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4,11 +4,6 @@ settings: autoInstallPeers: true excludeLinksFromLockfile: false -patchedDependencies: - '@mdbase-dev/connect@0.1.0-beta.123': - hash: 17ef7459d87276a5b4ddef4e2a993c4dcf06a05ffd885a6401de3a54abb23238 - path: patches/@mdbase-dev__connect@0.1.0-beta.123.patch - importers: .: @@ -20,8 +15,8 @@ importers: specifier: ^9.39.2 version: 9.39.5 '@mdbase-dev/connect-dev': - specifier: 0.1.0-beta.123 - version: 0.1.0-beta.123 + specifier: 0.1.0-beta.124 + version: 0.1.0-beta.124 '@types/node': specifier: ^22.19.11 version: 22.20.1 @@ -93,8 +88,8 @@ importers: apps/extension: dependencies: '@mdbase-dev/ui': - specifier: 0.1.0-beta.123 - version: 0.1.0-beta.123(@codemirror/view@6.43.8)(react@19.2.8) + specifier: 0.1.0-beta.124 + version: 0.1.0-beta.124(@codemirror/view@6.43.8)(react@19.2.8) '@mdbase-reader/connect': specifier: workspace:* version: link:../../packages/connect @@ -139,8 +134,8 @@ importers: specifier: 0.8.2 version: 0.8.2(@citation-js/core@0.8.2) '@mdbase-dev/ui': - specifier: 0.1.0-beta.123 - version: 0.1.0-beta.123(@codemirror/view@6.43.8)(react@19.2.8) + specifier: 0.1.0-beta.124 + version: 0.1.0-beta.124(@codemirror/view@6.43.8)(react@19.2.8) '@mdbase-reader/connect': specifier: workspace:* version: link:../../packages/connect @@ -229,8 +224,8 @@ importers: packages/connect: dependencies: '@mdbase-dev/connect': - specifier: 0.1.0-beta.123 - version: 0.1.0-beta.123(patch_hash=17ef7459d87276a5b4ddef4e2a993c4dcf06a05ffd885a6401de3a54abb23238) + specifier: 0.1.0-beta.124 + version: 0.1.0-beta.124 '@mdbase-reader/core': specifier: workspace:* version: link:../core @@ -276,8 +271,8 @@ importers: specifier: ^1.2.3 version: 1.2.3 '@mdbase-dev/ui': - specifier: 0.1.0-beta.123 - version: 0.1.0-beta.123(@codemirror/view@6.43.8)(react@19.2.8) + specifier: 0.1.0-beta.124 + version: 0.1.0-beta.124(@codemirror/view@6.43.8)(react@19.2.8) '@mdbase-reader/core': specifier: workspace:* version: link:../core @@ -416,8 +411,8 @@ importers: specifier: ^5.3.0 version: 5.3.0 '@mdbase-dev/ui': - specifier: 0.1.0-beta.123 - version: 0.1.0-beta.123(@codemirror/view@6.43.8)(react@19.2.8) + specifier: 0.1.0-beta.124 + version: 0.1.0-beta.124(@codemirror/view@6.43.8)(react@19.2.8) '@phosphor-icons/react': specifier: ^2.1.10 version: 2.1.10(react-dom@19.2.8(react@19.2.8))(react@19.2.8) @@ -1141,18 +1136,18 @@ packages: '@marijn/find-cluster-break@1.0.3': resolution: {integrity: sha512-FY+MKLBoTsLNJF/eLWaOsXGdz6uh3Iu1axjPf6TUq92IYumcTcXWHoS747JARLkcdlJ/Waiaxc5wQfFO8jC6NA==} - '@mdbase-dev/connect-dev@0.1.0-beta.123': - resolution: {integrity: sha512-tK8LWKRZGL4z2yA5oR1tZP8gBbXdzURgHKFNe3KJ0/sjRPYxBcW8gGW+Mgv4w96zhraM8CifoqB0FLks8nyHxw==} + '@mdbase-dev/connect-dev@0.1.0-beta.124': + resolution: {integrity: sha512-QWvJg6SnNShu+yRz0e4Ac30mak9W9Hz9hQqdKtGjTG92oWopaH+bOEqOAiKci21wjmm8MivkKkGLwp2zrJSlxg==} hasBin: true - '@mdbase-dev/connect-protocol@0.1.0-beta.123': - resolution: {integrity: sha512-93A/K+zUtCjsnCTX2K/GDa9/ffnshl8ebnd0K4bO8E4EFLLBOcLIJ6+5z3pVTEG1NfsSrFffN9D9oLMb5NAkjg==} + '@mdbase-dev/connect-protocol@0.1.0-beta.124': + resolution: {integrity: sha512-p1jA+AqpqfhnV+foOXtoQg8oVK9oSe+W40MwzbW3x7kEeQvxRRxtmGDeVaqBlob3Fn+4Owx0G1KSRu3BzlnphA==} - '@mdbase-dev/connect@0.1.0-beta.123': - resolution: {integrity: sha512-LquiTAhumNaE2E96RimFPaxMcClH28cgYPtGPAAWD5L+PQjcv6aR8m/h7s0k933W0sJjY0BMkRjtX+aFu4fo5g==} + '@mdbase-dev/connect@0.1.0-beta.124': + resolution: {integrity: sha512-lV53SpYZjMqYD+tD5NDONsQAIPRHoFDm9OzF+SoG0hibqX5cPMu47LD9igZQIb63XFqvy+MOzjFz4SyGtlRWSg==} - '@mdbase-dev/ui@0.1.0-beta.123': - resolution: {integrity: sha512-KaHQfUelNoXWtGgvde6li1IWtImckBML9FQ4cJ3DFXlHD70hedYF8ly7mz3U4TRu98RNCPCVWmugXaP3pE/LAw==} + '@mdbase-dev/ui@0.1.0-beta.124': + resolution: {integrity: sha512-75xiQ9PhKjVyk9R+xaE0APpNn5MprfrkXwXhVQM5eT3uTCCZ0YwFCTeIAFsjEB6MNItp4pujoyebSIPQKwXg7w==} peerDependencies: '@codemirror/view': ^6.0.0 react: ^19.2.0 @@ -4311,23 +4306,23 @@ snapshots: '@marijn/find-cluster-break@1.0.3': {} - '@mdbase-dev/connect-dev@0.1.0-beta.123': + '@mdbase-dev/connect-dev@0.1.0-beta.124': dependencies: - '@mdbase-dev/connect': 0.1.0-beta.123(patch_hash=17ef7459d87276a5b4ddef4e2a993c4dcf06a05ffd885a6401de3a54abb23238) - '@mdbase-dev/connect-protocol': 0.1.0-beta.123 + '@mdbase-dev/connect': 0.1.0-beta.124 + '@mdbase-dev/connect-protocol': 0.1.0-beta.124 ajv: 8.20.0 yaml: 2.9.1 - '@mdbase-dev/connect-protocol@0.1.0-beta.123': + '@mdbase-dev/connect-protocol@0.1.0-beta.124': dependencies: ajv: 8.20.0 ajv-formats: 3.0.1(ajv@8.20.0) - '@mdbase-dev/connect@0.1.0-beta.123(patch_hash=17ef7459d87276a5b4ddef4e2a993c4dcf06a05ffd885a6401de3a54abb23238)': + '@mdbase-dev/connect@0.1.0-beta.124': dependencies: - '@mdbase-dev/connect-protocol': 0.1.0-beta.123 + '@mdbase-dev/connect-protocol': 0.1.0-beta.124 - '@mdbase-dev/ui@0.1.0-beta.123(@codemirror/view@6.43.8)(react@19.2.8)': + '@mdbase-dev/ui@0.1.0-beta.124(@codemirror/view@6.43.8)(react@19.2.8)': dependencies: '@fontsource/atkinson-hyperlegible': 5.3.0 '@fontsource/azeret-mono': 5.3.0 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 6afea67..f4232ad 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -6,8 +6,3 @@ packages: onlyBuiltDependencies: - electron - esbuild - -# Temporary: fixes superseded startup assessment cancellation in beta.123 -# (mdbase-dev/mdbase-connect#557). Remove when upgrading past beta.123. -patchedDependencies: - "@mdbase-dev/connect@0.1.0-beta.123": patches/@mdbase-dev__connect@0.1.0-beta.123.patch