diff --git a/src/lib/review-anchor.test.ts b/src/lib/review-anchor.test.ts index 0dabdc5..736957d 100644 --- a/src/lib/review-anchor.test.ts +++ b/src/lib/review-anchor.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { reanchorPoint, reanchorRange } from './review-anchor'; +import { createAnchorMapper, reanchorPoint, reanchorRange } from './review-anchor'; const BASE = 'The quick brown fox jumps over the lazy dog.'; // Anchor "brown fox" = [10, 19). @@ -84,3 +84,61 @@ describe('reanchorPoint', () => { expect(reanchorPoint(BASE, BASE, BASE.length + 1)).toBeNull(); }); }); + +describe('createAnchorMapper', () => { + // The mapper reuses one diff per pair of texts, so what it returns has to + // stay identical to re-anchoring each anchor on its own. + const CASES: Array<[string, string]> = [ + [BASE, BASE], + [BASE, 'A very ' + BASE], + [BASE, BASE.replace('lazy dog', 'sleeping cat')], + [BASE, BASE.replace('brown fox', 'brown furry fox')], + [BASE, BASE.replace('brown fox', 'red vixen')], + [BASE, BASE.replace('brown fox ', '')] + ]; + + it('matches reanchorRange on every offset', () => { + const mapper = createAnchorMapper(); + for (const [base, current] of CASES) { + for (let start = 0; start < base.length; start++) { + for (let end = start; end <= base.length + 1; end++) { + expect(mapper.range(base, current, start, end)).toEqual( + reanchorRange(base, current, start, end) + ); + } + } + } + }); + + it('matches reanchorPoint on every position', () => { + const mapper = createAnchorMapper(); + for (const [base, current] of CASES) { + for (let position = -1; position <= base.length + 1; position++) { + expect(mapper.point(base, current, position)).toEqual( + reanchorPoint(base, current, position) + ); + } + } + }); + + it('keeps separate results for different current texts against one base', () => { + const mapper = createAnchorMapper(); + const shifted = 'A very ' + BASE; + const rewritten = BASE.replace('brown fox', 'red vixen'); + expect(mapper.range(BASE, shifted, START, END)).toEqual({ start: START + 7, end: END + 7 }); + expect(mapper.range(BASE, rewritten, START, END)).toBeNull(); + // Repeating the first pair still maps through its own diff, not the second's. + expect(mapper.range(BASE, shifted, START, END)).toEqual({ start: START + 7, end: END + 7 }); + }); + + it('diffs each pair of texts once however many anchors map through it', () => { + const mapper = createAnchorMapper(); + const current = BASE.replace('lazy dog', 'sleeping cat'); + const first = mapper.range(BASE, current, START, END); + const t0 = performance.now(); + for (let i = 0; i < 1000; i++) mapper.range(BASE, current, START, END); + // A thousand cached maps cost far less than a thousand diffs would. + expect(performance.now() - t0).toBeLessThan(50); + expect(mapper.range(BASE, current, START, END)).toEqual(first); + }); +}); diff --git a/src/lib/review-anchor.ts b/src/lib/review-anchor.ts index bb41dcc..429df21 100644 --- a/src/lib/review-anchor.ts +++ b/src/lib/review-anchor.ts @@ -1,4 +1,4 @@ -import { diffChars } from 'diff'; +import { diffChars, type Change } from 'diff'; // Re-anchors a review thread's character range after the text changed. // Comments pin a [start, end) range against the revision they were made on; @@ -15,12 +15,31 @@ export function reanchorRange( ): { start: number; end: number } | null { if (start < 0 || end > baseText.length || start >= end) return null; if (baseText === currentText) return { start, end }; + return mapRange(diffChars(baseText, currentText), start, end); +} + +// Re-anchors a single position (a pure insertion point). The position +// survives if it lies in or at the boundary of unchanged text. +export function reanchorPoint( + baseText: string, + currentText: string, + position: number +): number | null { + if (position < 0 || position > baseText.length) return null; + if (baseText === currentText) return position; + return mapPoint(diffChars(baseText, currentText), position); +} +function mapRange( + parts: Change[], + start: number, + end: number +): { start: number; end: number } | null { let basePos = 0; let currentPos = 0; let newStart = -1; let newEnd = -1; - for (const part of diffChars(baseText, currentText)) { + for (const part of parts) { const length = part.value.length; if (part.added) { currentPos += length; @@ -45,19 +64,10 @@ export function reanchorRange( return { start: newStart, end: newEnd }; } -// Re-anchors a single position (a pure insertion point). The position -// survives if it lies in or at the boundary of unchanged text. -export function reanchorPoint( - baseText: string, - currentText: string, - position: number -): number | null { - if (position < 0 || position > baseText.length) return null; - if (baseText === currentText) return position; - +function mapPoint(parts: Change[], position: number): number | null { let basePos = 0; let currentPos = 0; - for (const part of diffChars(baseText, currentText)) { + for (const part of parts) { const length = part.value.length; if (part.added) { currentPos += length; @@ -75,3 +85,50 @@ export function reanchorPoint( } return null; } + +// Every thread and suggestion anchored to the same scene revision re-anchors +// against the same two texts, and a character diff of a long scene costs +// hundreds of milliseconds. A mapper diffs each pair of texts once and maps +// every anchor through that one result. Make one per request and hand it to +// everything that re-anchors, so the cost is per revision rather than per +// comment. +export type AnchorMapper = { + range( + baseText: string, + currentText: string, + start: number, + end: number + ): { start: number; end: number } | null; + point(baseText: string, currentText: string, position: number): number | null; +}; + +export function createAnchorMapper(): AnchorMapper { + // Nested by base text then current text, so neither key has to be + // concatenated into a third copy of the scene. + const cache = new Map>(); + const diff = (baseText: string, currentText: string): Change[] => { + let byCurrent = cache.get(baseText); + if (!byCurrent) { + byCurrent = new Map(); + cache.set(baseText, byCurrent); + } + let parts = byCurrent.get(currentText); + if (!parts) { + parts = diffChars(baseText, currentText); + byCurrent.set(currentText, parts); + } + return parts; + }; + return { + range(baseText, currentText, start, end) { + if (start < 0 || end > baseText.length || start >= end) return null; + if (baseText === currentText) return { start, end }; + return mapRange(diff(baseText, currentText), start, end); + }, + point(baseText, currentText, position) { + if (position < 0 || position > baseText.length) return null; + if (baseText === currentText) return position; + return mapPoint(diff(baseText, currentText), position); + } + }; +} diff --git a/src/lib/server/export-reviews.ts b/src/lib/server/export-reviews.ts index 238cd85..b3f2d2a 100644 --- a/src/lib/server/export-reviews.ts +++ b/src/lib/server/export-reviews.ts @@ -2,7 +2,7 @@ import { and, eq, isNull } from 'drizzle-orm'; import type { Database } from './auth.ts'; import { scenes } from './db/schema.ts'; import { listThreads } from './review.ts'; -import { reanchorRange } from '../review-anchor.ts'; +import { createAnchorMapper } from '../review-anchor.ts'; import type { ExportReviewThread, ReviewLoader } from './export.ts'; // Long selections are trimmed: the excerpt locates the thread, the scene @@ -15,7 +15,7 @@ const EXCERPT_MAX = 240; // export builders does not drag in the review module. export function reviewLoader(db: Database): ReviewLoader { return async (storyId) => { - const threads = await listThreads(db, storyId, reanchorRange); + const threads = await listThreads(db, storyId, createAnchorMapper().range); if (threads.length === 0) return []; const sceneRows = await db .select({ id: scenes.id, title: scenes.title, bodyMd: scenes.bodyMd }) diff --git a/src/lib/server/review.ts b/src/lib/server/review.ts index 753bd12..a5bd80b 100644 --- a/src/lib/server/review.ts +++ b/src/lib/server/review.ts @@ -15,7 +15,7 @@ import { import { hashToken } from './tokens.ts'; import { signToken, verifyToken } from './crypto.ts'; import { recordRevision } from './revisions.ts'; -import { reanchorPoint, reanchorRange } from '../review-anchor.ts'; +import { createAnchorMapper, type AnchorMapper } from '../review-anchor.ts'; import { wordCount } from '../word-count.ts'; import { normaliseAssistantName } from './llm/prompts/persona.ts'; import { EMAIL_RE } from './signup.ts'; @@ -634,24 +634,26 @@ export type SuggestionView = { }; // Re-anchors the proposed range onto the current text: a real range maps -// through reanchorRange, a pure insertion point through reanchorPoint. +// through the mapper's range, a pure insertion point through its point. function mapSuggestionRange( + mapper: AnchorMapper, baseText: string, currentText: string, start: number, end: number ): { start: number; end: number } | null { if (start === end) { - const point = reanchorPoint(baseText, currentText, start); + const point = mapper.point(baseText, currentText, start); return point === null ? null : { start: point, end: point }; } - return reanchorRange(baseText, currentText, start, end); + return mapper.range(baseText, currentText, start, end); } // Where a decided suggestion now sits, so the Done view can point at it: the // accepted replacement (the new text it left behind) or the rejected original // (still in place). Null when there is nothing to mark or it cannot be found. function decidedAnchor( + mapper: AnchorMapper, status: 'accepted' | 'rejected', baseText: string, currentText: string, @@ -662,20 +664,24 @@ function decidedAnchor( if (status === 'rejected') { // Rejection left the text untouched, so the original passage maps as-is. if (start === end) return null; // a rejected insertion left no text - return reanchorRange(baseText, currentText, start, end); + return mapper.range(baseText, currentText, start, end); } // Accepted: the replacement now occupies the spot the original held. if (replacement.length === 0) return null; // a deletion leaves nothing to mark - const point = reanchorPoint(baseText, currentText, start); + const point = mapper.point(baseText, currentText, start); if (point === null) return null; const at = { start: point, end: point + replacement.length }; return at.end <= currentText.length ? at : null; } +// The mapper is shared with listThreads when both run for the same story, so +// each scene revision is diffed once for the whole page rather than once per +// thread and once per suggestion. export async function listSuggestions( db: Database, storyId: string, - viewer?: ReviewViewer + viewer?: ReviewViewer, + mapper: AnchorMapper = createAnchorMapper() ): Promise { const rows = await db .select({ @@ -701,8 +707,9 @@ export async function listSuggestions( // anchors to where it now sits, so the Done view can point at it. const anchor = row.suggestion.status === 'pending' - ? mapSuggestionRange(row.baseBody, row.currentBody, rangeStart, rangeEnd) + ? mapSuggestionRange(mapper, row.baseBody, row.currentBody, rangeStart, rangeEnd) : decidedAnchor( + mapper, row.suggestion.status, row.baseBody, row.currentBody, @@ -819,7 +826,10 @@ export async function decideSuggestion( .where(eq(scenes.id, row.scene.id)) .for('update'); if (!scene) return { ok: false, reason: 'That scene does not exist.' }; + // One anchor against the text just read under the lock, so there is + // nothing for a shared mapper to reuse. const anchor = mapSuggestionRange( + createAnchorMapper(), row.baseBody, scene.bodyMd, row.suggestion.rangeStart, diff --git a/src/routes/api/assistant/review-reply/+server.ts b/src/routes/api/assistant/review-reply/+server.ts index 7944d8b..9f4aff7 100644 --- a/src/routes/api/assistant/review-reply/+server.ts +++ b/src/routes/api/assistant/review-reply/+server.ts @@ -13,7 +13,7 @@ import { assembleContext, buildSystemMessage } from '$lib/server/llm/context/ass import { buildReviewReplyMessage, excerptAround } from '$lib/server/llm/prompts/review-reply'; import { complete } from '$lib/server/llm/gateway'; import { addComment, listSuggestions, listThreads } from '$lib/server/review'; -import { reanchorRange } from '$lib/review-anchor'; +import { createAnchorMapper } from '$lib/review-anchor'; import { isUuid } from '$lib/slug'; import type { ChatMessage } from '$lib/server/llm/providers/types'; @@ -48,8 +48,11 @@ export const POST: RequestHandler = async ({ request, locals }) => { const layout = await requireAssistantGate(userId, story.id); - // The full thread view carries the re-anchored range and display names. - const threads = await listThreads(db, story.id, reanchorRange, { userId }); + // The full thread view carries the re-anchored range and display names. One + // mapper covers the threads and the suggestion lookup below, so a scene + // revision is diffed once rather than once per anchor. + const anchors = createAnchorMapper(); + const threads = await listThreads(db, story.id, anchors.range, { userId }); const thread = threads.find((t) => t.id === threadId); if (!thread) error(404, 'That thread does not exist.'); if (!thread.comments[0]?.isAssistant) { @@ -57,7 +60,9 @@ export const POST: RequestHandler = async ({ request, locals }) => { } const suggestion = thread.suggestionId - ? ((await listSuggestions(db, story.id)).find((s) => s.id === thread.suggestionId) ?? null) + ? ((await listSuggestions(db, story.id, undefined, anchors)).find( + (s) => s.id === thread.suggestionId + ) ?? null) : null; const [scene] = await db diff --git a/src/routes/review/[token]/+page.server.ts b/src/routes/review/[token]/+page.server.ts index 11ace6c..61dfdcc 100644 --- a/src/routes/review/[token]/+page.server.ts +++ b/src/routes/review/[token]/+page.server.ts @@ -22,7 +22,7 @@ import { } from '$lib/server/review'; import { gatherStory } from '$lib/server/export'; import { reviewMentionData } from '$lib/server/mention-entities'; -import { reanchorRange } from '$lib/review-anchor'; +import { createAnchorMapper } from '$lib/review-anchor'; import { rateLimit } from '$lib/server/rate-limit'; import { notifyReviewActivity } from '$lib/server/notify'; @@ -88,6 +88,9 @@ export const load: PageServerLoad = async ({ params, cookies, locals }) => { sceneIds: scenes.map((scene) => scene.id), restrictToMentioned: true }); + // Threads and suggestions re-anchor against the same scene revisions, so one + // mapper covers both and each revision is diffed once for the page. + const anchors = createAnchorMapper(); return { state: 'review' as const, storyTitle: resolved.storyTitle, @@ -95,8 +98,8 @@ export const load: PageServerLoad = async ({ params, cookies, locals }) => { canSuggest: resolved.invitation.canSuggest, chapters: content.chapters, scenes, - threads: await listThreads(db, storyId, reanchorRange, { reviewerId: reviewer.id }), - suggestions: await listSuggestions(db, storyId, { reviewerId: reviewer.id }), + threads: await listThreads(db, storyId, anchors.range, { reviewerId: reviewer.id }), + suggestions: await listSuggestions(db, storyId, { reviewerId: reviewer.id }, anchors), mentionEntities: mentions.entities, mentionMembers: mentions.storyMembers, mentionPins: mentions.pins diff --git a/src/routes/stories/[id]/review/+page.server.ts b/src/routes/stories/[id]/review/+page.server.ts index 7e690c9..83f9000 100644 --- a/src/routes/stories/[id]/review/+page.server.ts +++ b/src/routes/stories/[id]/review/+page.server.ts @@ -20,7 +20,7 @@ import { gatherStory } from '$lib/server/export'; import { storyPreferences } from '$lib/server/preferences'; import { storyPageSetup } from '$lib/server/page-setup'; import { reviewMentionData } from '$lib/server/mention-entities'; -import { reanchorRange } from '$lib/review-anchor'; +import { createAnchorMapper } from '$lib/review-anchor'; import { queueSceneMentions } from '$lib/server/jobs'; import { assistantLayout, saveStoryLlmOverride } from '$lib/server/llm/config'; import { listChat } from '$lib/server/llm/chat-history'; @@ -38,6 +38,9 @@ export const load: PageServerLoad = async ({ params, locals }) => { // navigation as the editor. The author's review takes the full cast // (restrictToMentioned is false), so reviewMentionData needs no scene ids and // nothing here waits on gatherStory. + // Threads and suggestions re-anchor against the same scene revisions, so one + // mapper covers both and each revision is diffed once for the page. + const anchors = createAnchorMapper(); const [ content, mentions, @@ -56,8 +59,8 @@ export const load: PageServerLoad = async ({ params, locals }) => { restrictToMentioned: false }), listTrashedScenes(db, story.id), - listThreads(db, story.id, reanchorRange, { userId: locals.user!.id }), - listSuggestions(db, story.id, { userId: locals.user!.id }), + listThreads(db, story.id, anchors.range, { userId: locals.user!.id }), + listSuggestions(db, story.id, { userId: locals.user!.id }, anchors), storyPreferences(db, locals.user!.id, story.id), storyPageSetup(db, story.id), assistantLayout(db, locals.user!.id, story.id)