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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 59 additions & 1 deletion src/lib/review-anchor.test.ts
Original file line number Diff line number Diff line change
@@ -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).
Expand Down Expand Up @@ -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);
});
});
83 changes: 70 additions & 13 deletions src/lib/review-anchor.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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<string, Map<string, Change[]>>();
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);
}
};
}
4 changes: 2 additions & 2 deletions src/lib/server/export-reviews.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 })
Expand Down
26 changes: 18 additions & 8 deletions src/lib/server/review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand All @@ -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<SuggestionView[]> {
const rows = await db
.select({
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
13 changes: 9 additions & 4 deletions src/routes/api/assistant/review-reply/+server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -48,16 +48,21 @@ 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) {
error(400, 'The Assistant only replies in threads it opened.');
}

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
Expand Down
9 changes: 6 additions & 3 deletions src/routes/review/[token]/+page.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -88,15 +88,18 @@ 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,
reviewerName: reviewer.displayName,
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
Expand Down
9 changes: 6 additions & 3 deletions src/routes/stories/[id]/review/+page.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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,
Expand All @@ -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)
Expand Down