diff --git a/client/src/components/Assistant/AssistantSurface.tsx b/client/src/components/Assistant/AssistantSurface.tsx index f77faad14..03835c160 100644 --- a/client/src/components/Assistant/AssistantSurface.tsx +++ b/client/src/components/Assistant/AssistantSurface.tsx @@ -28,6 +28,7 @@ export default function AssistantSurface({ onOpen, onFocusDay, onRevealMap, + bookVersion, }: AssistantSurfaceProps) { const runtime = useAssistantRuntime(tripId); // The phone is always compact; a window is what its width says. @@ -39,8 +40,9 @@ export default function AssistantSurface({ onOpen, onFocusDay, onRevealMap, + bookVersion, }), - [tripId, variant, density, onOpen, onFocusDay, onRevealMap] + [tripId, variant, density, onOpen, onFocusDay, onRevealMap, bookVersion] ); return ( diff --git a/client/src/components/Assistant/assistantViewContext.ts b/client/src/components/Assistant/assistantViewContext.ts index 4d487ef27..49ee95fe1 100644 --- a/client/src/components/Assistant/assistantViewContext.ts +++ b/client/src/components/Assistant/assistantViewContext.ts @@ -27,6 +27,13 @@ export interface AssistantView { onFocusDay?: (dayId: number) => void; /** Brings the map into view where the chat covers it (the phone), before the map moves. */ onRevealMap?: () => void; + /** + * The stored book's version the book open in TREK Studio stands on, null + * before it has one; left out outside the Studio. A book card draws the page + * after as the server's trial made it only while the book is still at the + * version the trial ran on. + */ + bookVersion?: number | null; } export const AssistantViewContext = createContext(null); diff --git a/client/src/components/Assistant/cards/book/BookPageCard.test.tsx b/client/src/components/Assistant/cards/book/BookPageCard.test.tsx index 134944c42..aefdc1a16 100644 --- a/client/src/components/Assistant/cards/book/BookPageCard.test.tsx +++ b/client/src/components/Assistant/cards/book/BookPageCard.test.tsx @@ -1,13 +1,14 @@ /** - * FE-AI-BOOKPAGE-001 to FE-AI-BOOKPAGE-006 + * FE-AI-BOOKPAGE-001 to FE-AI-BOOKPAGE-007 * * The photo book's change rows in the Studio's chat: the pages a change works * on named as the Studio names them, its facts as badges (new pages, where a * moved page lands, the layout, new photos, findings to check), the page * before and after drawn from the book open in the Studio (each page of a - * delete alone), a thumbnail that shows its page, each finding named once, a - * receipt that opens the Studio on the page, and a discarded change struck - * through with no page drawn. + * delete alone, the page after as the server made it when the card carries + * it and the book is still at the trial's version), a thumbnail that shows + * its page, each finding named once, a receipt that opens the Studio on the + * page, and a discarded change struck through with no page drawn. */ import { type AssistantApplied, @@ -15,6 +16,7 @@ import { blankBook, blankSpread, type BookPageCard, + bookTextElementSchema, } from '@trek/shared'; import type { ReactNode } from 'react'; import { afterEach, describe, expect, it, vi } from 'vitest'; @@ -222,4 +224,36 @@ describe('the book page card', () => { }); expect(screen.queryByRole('button', { name: 'Show this page' })).toBeNull(); }); + + it('FE-AI-BOOKPAGE-007: a waiting edit draws the page after as the server made it, where the Studio alone could not run it, until the book moves on', async () => { + useStudioStore.setState({ doc: book(), activeSpread: 0 }); + // A caption the book on screen cannot word: run in the Studio it comes to nothing, so only the page before shows. + const caption = card({ ops: [{ op: 'caption', photoElementId: 'p-81' }] }); + const first = inView(); + expect(await screen.findAllByRole('button', { name: 'Show this page' })).toHaveLength(1); + expect(screen.queryByText('After')).toBeNull(); + first.unmount(); + + // The page the server's trial made, its caption worded from the journey, is drawn after it. + const words = bookTextElementSchema.parse({ + id: 't-cap', + kind: 'text', + frame: { x: 20, y: 150, w: 120, h: 8 }, + text: 'Tram 28 at dawn', + }); + const made = { ...blankSpread('inner', () => 'sp-3'), elements: [words] }; + const drawn = item('pending', { ...caption, after: made }, 'editBookPage'); + const second = inView(, { bookVersion: 4 }); + expect(await screen.findByText('After')).toBeInTheDocument(); + expect(screen.getByText('Before')).toBeInTheDocument(); + expect(screen.getByText('Tram 28 at dawn')).toBeInTheDocument(); + expect(screen.getAllByRole('button', { name: 'Show this page' })).toHaveLength(2); + second.unmount(); + + // The book moved on since the trial (another change applied, a save): its page is not what the change would leave. + inView(, { bookVersion: 5 }); + expect(await screen.findAllByRole('button', { name: 'Show this page' })).toHaveLength(1); + expect(screen.queryByText('After')).toBeNull(); + expect(screen.queryByText('Tram 28 at dawn')).toBeNull(); + }); }); diff --git a/client/src/components/Assistant/cards/book/BookPageThumbs.tsx b/client/src/components/Assistant/cards/book/BookPageThumbs.tsx index 87106f74b..0c51135e2 100644 --- a/client/src/components/Assistant/cards/book/BookPageThumbs.tsx +++ b/client/src/components/Assistant/cards/book/BookPageThumbs.tsx @@ -6,17 +6,18 @@ import { useStudioStore } from '../../../../store/studioStore'; import { SpreadThumb } from '../../../Studio/SpreadThumb'; import { useStudioPageHint } from '../../../Studio/useStudioPageHint'; import type { ChangeItem } from '../../assistantParts'; -import { useAssistantScope } from '../../assistantViewContext'; +import { useAssistantScope, useAssistantView } from '../../assistantViewContext'; import { type BookPageView, pageViewOf } from './bookCardModel'; /** * The page of a book change drawn small, from the book open in the Studio: * while the change waits, the page as it is and as the change would leave it - * (a delete draws each page it takes); once applied, the page as it is now. - * Each is the editor's own view under a scale, so the card can never look - * different from the page. Pressing one shows that page in the Studio. - * Nothing where the Studio does not hold the page, outside the Studio or on - * another book. + * (the page the server's trial made, which the card carries, while the book + * is at the version the trial ran on; a delete draws each page it takes); + * once applied, the page as it is now. Each is the editor's own view under a + * scale, so the card can never look different from the page. Pressing one + * shows that page in the Studio. Nothing where the Studio does not hold the + * page, outside the Studio or on another book. */ /** A thumbnail's height in pixels; its width follows the page. */ @@ -83,9 +84,10 @@ function PageViewThumbs({ view, page, onShow }: { view: BookPageView; page: Book export default function BookPageThumbs({ card, item }: { card: BookPageCard; item: ChangeItem }) { const scope = useAssistantScope(); + const version = useAssistantView().bookVersion ?? null; const doc = useStudioStore((s) => s.doc); const setActiveSpread = useStudioStore((s) => s.setActiveSpread); - const views = useMemo(() => pageViewOf(card, doc, item.state), [card, doc, item.state]); + const views = useMemo(() => pageViewOf(card, doc, item.state, version), [card, doc, item.state, version]); // The Studio's book is this card's only in this journey's chat; anywhere else it may be another or an old copy. const ownBook = scope.kind === 'journey' && scope.journeyId === card.journeyId; if (!ownBook || views.length === 0 || !doc) return null; diff --git a/client/src/components/Assistant/cards/book/bookCardModel.test.ts b/client/src/components/Assistant/cards/book/bookCardModel.test.ts index 5075b273a..8c7670356 100644 --- a/client/src/components/Assistant/cards/book/bookCardModel.test.ts +++ b/client/src/components/Assistant/cards/book/bookCardModel.test.ts @@ -1,10 +1,12 @@ /** - * FE-AI-BOOKPAGE-010 to FE-AI-BOOKPAGE-014 + * FE-AI-BOOKPAGE-010 to FE-AI-BOOKPAGE-016 * * The pure side of the book's page card: a page named as the Studio names * it, a layout and a finding by their words, the findings worth a look, the * spreads the card draws from the book on screen for each state of a change - * (each page of a delete alone), and where an applied change opens the Studio. + * (each page of a delete alone, the page after as the server made it when + * the card carries it and the book is still at the trial's version), and + * where an applied change opens the Studio. */ import { blankBook, blankSpread, type BookCardPage, type BookPageCard } from '@trek/shared'; import { describe, expect, it } from 'vitest'; @@ -136,4 +138,59 @@ describe('bookCardModel', () => { expect(studioTarget(3, 'sp-3')).toEqual({ kind: 'page', path: '/journey/3/studio?page=sp-3' }); expect(studioTarget(3, null)).toEqual({ kind: 'page', path: '/journey/3/studio' }); }); + + it('FE-AI-BOOKPAGE-015: at the version of the trial a waiting edit or cover draws the page after the server made, never a run of its own; a card without it runs its operations', () => { + const doc = book(); + const made = { ...blankSpread('inner', () => 'sp-3'), background: '#e6eef4' }; + // A caption the Studio's book cannot word is refused when run here; the server's page draws it all the same. + const caption = card({ ops: [{ op: 'caption', photoElementId: 'p-81' }], after: made }); + expect(pageViewOf(caption, doc, 'pending', 4)).toMatchObject([{ index: 2, before: { id: 'sp-3' } }]); + expect(pageViewOf(caption, doc, 'pending', 4)[0]?.after).toBe(made); + // Operations the book on screen would take are not run either: the page is the server's, frames and all. + expect(pageViewOf(card({ after: made }), doc, 'pending', 4)[0]?.after).toBe(made); + expect(pageViewOf(card({ after: made, ops: [] }), doc, 'pending', 4)[0]?.after).toBe(made); + const coverPage = { ...blankSpread('cover', () => 'sp-1'), background: '#f4efe6' }; + const cover = card({ + pages: [{ ...INNER, spreadId: 'sp-1', index: 0, role: 'cover', number: null }], + ops: [], + cover: { template: 'cover-full', photoId: null, title: 'Portugal', subtitle: null }, + after: coverPage, + }); + expect(pageViewOf(cover, doc, 'pending', 4)[0]?.after).toBe(coverPage); + + // Without the page (a card from before it, or a page too heavy to carry), or with another page, the operations run. + const photos = (view: ReturnType) => + view[0]?.after?.elements.filter((element) => element.kind === 'photo').length; + expect(photos(pageViewOf(card(), doc, 'pending', 4))).toBe(2); + expect(photos(pageViewOf(card({ after: { ...made, id: 'sp-4' } }), doc, 'pending', 4))).toBe(2); + // Once applied, the page is the book's as it is now. + expect(pageViewOf(card({ after: made }), doc, 'applied', 4)[0]?.after).toBe(doc.spreads[2]); + }); + + it('FE-AI-BOOKPAGE-016: once the book moved on from the trial, or its version is not known, the change is run on the book as it is', () => { + const doc = book(); + const made = { ...blankSpread('inner', () => 'sp-3'), background: '#e6eef4' }; + const photos = (view: ReturnType) => + view[0]?.after?.elements.filter((element) => element.kind === 'photo').length; + // Another change applied or a save since: the change will run on this book, so its operations are run on it. + expect(photos(pageViewOf(card({ after: made }), doc, 'pending', 5))).toBe(2); + // A caption this book cannot word then draws no page after, never the trial's page of an older book. + const caption = card({ ops: [{ op: 'caption', photoElementId: 'p-81' }], after: made }); + expect(pageViewOf(caption, doc, 'pending', 5)).toMatchObject([{ before: { id: 'sp-3' }, after: null }]); + // Nor does a card whose operations gave way to the page. + expect(pageViewOf(card({ after: made, ops: [] }), doc, 'pending', 5)).toMatchObject([{ after: null }]); + // A cover is set again on the cover as it is. + const cover = card({ + pages: [{ ...INNER, spreadId: 'sp-1', index: 0, role: 'cover', number: null }], + ops: [], + cover: { template: 'cover-full', photoId: null, title: 'Portugal', subtitle: null }, + after: { ...blankSpread('cover', () => 'sp-1'), background: '#f4efe6' }, + }); + const [covered] = pageViewOf(cover, doc, 'pending', 5); + expect(covered?.after?.id).toBe('sp-1'); + expect(covered?.after).not.toBe(cover.after); + // Where the Studio does not say its version, or the card names none, the operations run too. + expect(photos(pageViewOf(card({ after: made }), doc, 'pending'))).toBe(2); + expect(photos(pageViewOf(card({ after: made, baseVersion: null }), doc, 'pending', null))).toBe(2); + }); }); diff --git a/client/src/components/Assistant/cards/book/bookCardModel.ts b/client/src/components/Assistant/cards/book/bookCardModel.ts index ae7dbfe12..f1a5cd433 100644 --- a/client/src/components/Assistant/cards/book/bookCardModel.ts +++ b/client/src/components/Assistant/cards/book/bookCardModel.ts @@ -18,9 +18,13 @@ import type { ChangeState } from '../../assistantParts'; /** * The pure side of the book's page card: how a page is named, what a layout * and a finding are called, and which spreads the card draws. The page before - * and after a change comes from the Studio's own book with the shared engine, - * the one the server tried the change with, so the card shows what the change - * does to the page as it is on screen. + * a change comes from the Studio's own book; the page after is the one the + * server's trial of the change made with the journey's photos, words and + * figures, which the card carries, so it shows the page as the server would + * write it, while the Studio's book is at the version the trial ran on. A + * card without that page, or a book that moved on since, runs the card's + * operations on the Studio's book with the shared engine, the one the server + * tried the change with: the change will run on that book. */ type Translate = (key: string, params?: Record) => string; @@ -118,8 +122,18 @@ function coverArgs(cover: BookCardCover): Record { return Object.fromEntries(Object.entries(cover).filter(([, value]) => value !== null)); } -/** The page as the change would leave it, run on the Studio's book; null when the card carries nothing to run. */ -function afterOf(card: BookPageCard, doc: BookDocument, spreadId: string): BookSpread | null { +/** + * The page as the change would leave it: the one the server's trial made, + * when the card carries it for this page and the Studio's book is still at + * the version the trial ran on; else run on the Studio's book without the + * journey's photos, words and figures a binding or a travel element reads. + * A book that moved on (another change applied, a save by hand or by someone + * else) is the book the change will run on, so the trial's page would show + * the page as it no longer is. Null when the card carries nothing to run. + */ +function afterOf(card: BookPageCard, doc: BookDocument, spreadId: string, version: number | null): BookSpread | null { + const atTrial = card.baseVersion !== null && card.baseVersion === version; + if (atTrial && card.after?.id === spreadId) return card.after; let made: ReturnType | null = null; if (card.cover) made = setBookCover(doc, coverArgs(card.cover)); else if (card.ops.length > 0) made = applyBookOps(doc, { spreadId, ops: card.ops }); @@ -140,9 +154,15 @@ function spreadAt(doc: BookDocument, spreadId: string): { index: number; current * (a move shows its page alone, a delete each page it takes alone); once * applied, the page as it is now. Nothing for new pages not made yet, for a * change that did not run, or where the book on screen does not hold the - * page (another book, or none open). + * page (another book, or none open). `version` is the stored book's version + * the book on screen stands on, null where the Studio does not say. */ -export function pageViewOf(card: BookPageCard, doc: BookDocument | null, state: ChangeState): BookPageView[] { +export function pageViewOf( + card: BookPageCard, + doc: BookDocument | null, + state: ChangeState, + version: number | null = null +): BookPageView[] { if (!doc || GONE.has(state)) return []; if (card.change === 'willDelete' && state !== 'applied') { return card.pages.flatMap((page) => { @@ -155,5 +175,5 @@ export function pageViewOf(card: BookPageCard, doc: BookDocument | null, state: if (!spreadId || !found) return []; if (state === 'applied') return [{ index: found.index, before: null, after: found.current }]; if (card.change === 'new') return []; - return [{ index: found.index, before: found.current, after: afterOf(card, doc, spreadId) }]; + return [{ index: found.index, before: found.current, after: afterOf(card, doc, spreadId, version) }]; } diff --git a/client/src/components/Studio/StudioAssistantPanel.tsx b/client/src/components/Studio/StudioAssistantPanel.tsx index d656766e6..a42d3dc47 100644 --- a/client/src/components/Studio/StudioAssistantPanel.tsx +++ b/client/src/components/Studio/StudioAssistantPanel.tsx @@ -133,7 +133,16 @@ export function StudioPageStage({ assistant }: { assistant: StudioAssistant }) { ); } -export default function StudioAssistantPanel({ assistant, title }: { assistant: StudioAssistant; title: string }) { +export default function StudioAssistantPanel({ + assistant, + title, + bookVersion, +}: { + assistant: StudioAssistant; + title: string; + /** The stored book's version the open book stands on, for the book's cards in the chat. */ + bookVersion?: number | null; +}) { const { t } = useTranslation(); const navigate = useNavigate(); const thread = useAssistantThread(assistant.chatKey); @@ -174,6 +183,7 @@ export default function StudioAssistantPanel({ assistant, title }: { assistant: variant="window" density="compact" onOpen={onOpen} + bookVersion={bookVersion} /> ); @@ -198,8 +208,16 @@ export function StudioAssistantToggle({ assistant }: { assistant: StudioAssistan } /** The chat column at the end of the Studio's body, while it is open, named after the journey. */ -export function StudioAssistantColumn({ assistant, title }: { assistant: StudioAssistant; title?: string }) { +export function StudioAssistantColumn({ + assistant, + title, + bookVersion, +}: { + assistant: StudioAssistant; + title?: string; + bookVersion?: number | null; +}) { const { t } = useTranslation(); if (!assistant.open) return null; - return ; + return ; } diff --git a/client/src/components/Studio/StudioShell.tsx b/client/src/components/Studio/StudioShell.tsx index 2fab721b7..5d61cf7ae 100644 --- a/client/src/components/Studio/StudioShell.tsx +++ b/client/src/components/Studio/StudioShell.tsx @@ -134,7 +134,7 @@ export default function StudioShell() { t={s.t} locale={s.locale} /> - + diff --git a/client/src/pages/journeyStudio/useJourneyStudio.ts b/client/src/pages/journeyStudio/useJourneyStudio.ts index b88aca375..db561e6f6 100644 --- a/client/src/pages/journeyStudio/useJourneyStudio.ts +++ b/client/src/pages/journeyStudio/useJourneyStudio.ts @@ -777,6 +777,8 @@ export function useJourneyStudio() { saveNow: book.saveNow, /** Writes everything local and says whether it landed: before an assistant turn or apply. */ flushBook: book.flush, + /** The stored book's version the open book stands on, null before it has one: the chat's book cards read it. */ + bookVersion: book.record?.version ?? null, acceptTheirs: book.acceptTheirs, keepMine: book.keepMine, spread, diff --git a/server/src/nest/assistant/cards/book/book-change.ts b/server/src/nest/assistant/cards/book/book-change.ts index b49798244..f07d96315 100644 --- a/server/src/nest/assistant/cards/book/book-change.ts +++ b/server/src/nest/assistant/cards/book/book-change.ts @@ -1,3 +1,4 @@ +import { logWarn } from '../../../audit/audit-log.logger'; import type { CardReader, Described, ToolArgs } from '../card-types'; import { recordOf, type Row } from '../card-values'; import { Rows, text } from '../preview-format'; @@ -8,6 +9,8 @@ import { bookCardFindingSchema, bookCardPageSchema, bookPageOpSchema, + bookSpreadSchema, + MAX_CARD_CHARS, setBookCoverArgs, setBookCoverRequestSchema, type AssistantChangePreview, @@ -22,9 +25,11 @@ import { * The change card of a held book write and its receipt. Both are built from * the trial the turn's gate made of the call (book-trial.ts): the pages the * change works on with their place in the book, what it lays them out on and - * brings onto them, and the findings on the pages it leaves. The card carries - * the operations, so the Studio draws the page after with the same engine. - * Once applied, the pages come from the tool's answer. + * brings onto them, and the findings on the pages it leaves. The card of an + * edit or the cover carries the page the trial made, so the Studio draws the + * page after as the tool would write it while its book is at the trial's + * version, and the operations for a book that moved on since or a card that + * cannot carry that page. Once applied, the pages come from the tool's answer. */ /** What describing a held book write looked up: the gate's trial of it, null when there was none. */ @@ -44,11 +49,15 @@ const CHANGE_OF: Readonly> = { /** * Operations past this weight are not carried: twenty texts of two thousand - * characters would push the card past MAX_CARD_CHARS and lose it whole, so - * such a card shows its facts and no page after. + * characters would push the card past MAX_CARD_CHARS and lose it whole. Such + * a card shows its facts, and the page after only where it carries the page + * the trial made. */ const MAX_OPS_CHARS = 12_000; +/** The tools whose card the Studio draws the page after for: an edit and the cover. */ +const DRAWS_AFTER: ReadonlySet = new Set(['edit_book_page', 'set_book_cover']); + /** The trial of `tool` with these arguments, through the turn's reader; null without one or for a refused call. */ export async function loadBook(tool: string, args: ToolArgs, read: CardReader): Promise { const trial = trialFrom(await read(BOOK_TRIAL_READ, { tool, args })); @@ -115,6 +124,45 @@ function findingsOf(findings: readonly BookCardFinding[]): Pick spread.id === spreadId) : undefined; + if (!made) return card; + const parsed = bookSpreadSchema.safeParse(made); + if (!parsed.success) { + const issue = parsed.error.issues[0]; + logWarn(`[assistant] the page after of ${tool} does not fit the book: ${issue?.path.join('.')} ${issue?.message}`); + return card; + } + const full = { ...card, after: parsed.data }; + if (fitsCard(full)) return full; + const bare = { ...full, ops: [] }; + return fitsCard(bare) ? bare : card; +} + /** How many pages a change adds (a new book: all of them) or deletes; none for any other change. */ function countOf(change: BookPageCard['change'], trial: BookTrialRan): number { if (change === 'new') return trial.after.length; @@ -124,14 +172,15 @@ function countOf(change: BookPageCard['change'], trial: BookTrialRan): number { /** * The card under a held book write: the pages it works on as they stand (the * page edited, the cover, the page moved, the pages deleted, the page new ones - * follow), what it brings and what the check found. Null without a trial: a - * card needs the journey and the book as the server read them. + * follow), what it brings, what the check found and the page an edit or the + * cover leaves. Null without a trial: a card needs the journey and the book as + * the server read them. */ export function bookChangeCard(tool: string, args: ToolArgs, loaded: BookLoaded | null): BookPageCard | null { const trial = loaded?.trial ?? null; const change = CHANGE_OF[tool]; if (!trial || !change) return null; - return { + const card: BookPageCard = { kind: 'bookPage', journeyId: trial.journeyId, change, @@ -145,6 +194,7 @@ export function bookChangeCard(tool: string, args: ToolArgs, loaded: BookLoaded photosAdded: trial.photosAdded, ...findingsOf(trial.findings), }; + return withAfter(tool, card, trial); } /** A page of a write's answer (`get_book`'s row) as a card names it, numbered as the held card knew it. */ @@ -212,7 +262,8 @@ function receiptFindings(value: unknown, held: BookPageCard): Pick answeredPage(row, number) ?? []).slice(0, BOOK_OP_LIMITS.pagesPerCall); const kept = held.change === 'willDelete' || answered.length === 0; + const { after: _trialPage, ...facts } = held; return { - ...held, + ...facts, baseVersion: typeof answer.version === 'number' ? answer.version : held.baseVersion, pages: kept ? held.pages : answered, ...receiptFindings(answer.findings, held), diff --git a/server/src/nest/assistant/cards/book/book-trial.ts b/server/src/nest/assistant/cards/book/book-trial.ts index 8a9f27411..ff5a65dfc 100644 --- a/server/src/nest/assistant/cards/book/book-trial.ts +++ b/server/src/nest/assistant/cards/book/book-trial.ts @@ -51,6 +51,12 @@ export interface BookTrialRan { before: BookCardPage[]; /** The pages it leaves, as they would stand. */ after: BookCardPage[]; + /** + * The same pages as the engine made them, with the journey's photos, words + * and figures: what the change card draws after an edit or the cover, as + * the tool would write it. + */ + spreads: BookSpread[]; /** The findings on the pages it leaves, the most severe first. */ findings: BookFinding[]; /** @@ -235,6 +241,7 @@ function ranOf( version: input.version, before: pagesOf(before, targets), after: pagesOf(made.doc, ids), + spreads: made.doc.spreads.filter((spread) => ids.includes(spread.id)), findings, fresh, photosAdded, @@ -279,7 +286,8 @@ export function tryBookChange(tool: string, args: ToolArgs, input: BookTrialInpu /** A trial as the card's lookup gets it back from the reader; null for anything else. */ export function trialFrom(value: unknown): BookTrial | null { const record = recordOf(value); - if (record.ok === true && Array.isArray(record.after) && Array.isArray(record.findings)) return value as BookTrialRan; + const ran = record.ok === true && Array.isArray(record.after) && Array.isArray(record.spreads); + if (ran && Array.isArray(record.findings)) return value as BookTrialRan; if (record.ok === false && typeof record.refusal === 'string') return value as BookTrialRefused; return null; } diff --git a/server/src/nest/assistant/cards/book/index.ts b/server/src/nest/assistant/cards/book/index.ts index 20fa6b779..cb81a33f3 100644 --- a/server/src/nest/assistant/cards/book/index.ts +++ b/server/src/nest/assistant/cards/book/index.ts @@ -6,12 +6,13 @@ import type { AssistantChangeVerbKey } from '@trek/shared'; /** * The photo book in the Studio's chat. A held book write shows the pages it - * works on, before and after, drawn in the Studio from the operations the - * card carries, with what it brings and what the server's check of the result - * found; once applied, the pages as it left them. Edits, the cover and - * deleted pages are taken back with `restore_book_page`, new pages deleted - * again, a moved page moved back: an edit while its page is as the change - * left it, the others while nothing was saved to the book since. + * works on, before and after, drawn in the Studio from the page the server's + * trial made (or the operations the card carries, for a card without that + * page or a book moved on since), with what it brings and what the server's + * check of the result found; once applied, the pages as it left them. Edits, + * the cover and deleted pages are taken back with `restore_book_page`, new + * pages deleted again, a moved page moved back: an edit while its page is as + * the change left it, the others while nothing was saved to the book since. * * The check itself is the turn's gate (assistant-book-gate.ts): it tries the * call on the stored book, refuses what the engine refuses and sends a change diff --git a/server/tests/e2e/assistant-book-cards.e2e.test.ts b/server/tests/e2e/assistant-book-cards.e2e.test.ts index 711665a3b..6f59724df 100644 --- a/server/tests/e2e/assistant-book-cards.e2e.test.ts +++ b/server/tests/e2e/assistant-book-cards.e2e.test.ts @@ -6,16 +6,16 @@ * * Covered: a page edit that would leave empty frames goes back to the model * once, the same call made again is held with its page card (pages, the - * operations, the findings), applied with a receipt card, and taken back in - * one undo while nothing was saved since; an operation the engine refuses - * never reaches the user; new pages are not taken back once the book moved on, - * and neither is an edited page once that page was saved since (the restore - * checks the page as the change left it). A save to another page does not - * hold an edit's undo back, so of two changes to one page applied together - * "undo all" takes back the newer, then the older; changes apply in the order - * the decisions name them. A page is previewed with the ops of an edit before - * it is proposed, nothing saved; on this text-only model the preview is the - * page's description alone. + * operations, the findings, the page after as the trial made it), applied + * with a receipt card, and taken back in one undo while nothing was saved + * since; an operation the engine refuses never reaches the user; new pages + * are not taken back once the book moved on, and neither is an edited page + * once that page was saved since (the restore checks the page as the change + * left it). A save to another page does not hold an edit's undo back, so of + * two changes to one page applied together "undo all" takes back the newer, + * then the older; changes apply in the order the decisions name them. A page + * is previewed with the ops of an edit before it is proposed, nothing saved; + * on this text-only model the preview is the page's description alone. */ import { buildApp } from '../../src/bootstrap'; import { withRequestContext } from '../../src/nest/database/request-context'; @@ -240,6 +240,10 @@ describe('Studio book cards e2e', () => { code: 'empty_frame', severity: 'severe', }); + // The page after rides on the card as the server's trial made it: the layout's two frames, both empty. + const trialPage = change?.data.card?.kind === 'bookPage' ? change.data.card.after : undefined; + expect(trialPage?.id).toBe(j.inner); + expect(trialPage?.elements.filter((element) => element.kind === 'photo')).toHaveLength(2); const threadId = ofType(turn, 'data-trek-thread')[0]?.data.threadId ?? 0; const decided = await approve(j.base, j.userId, threadId, held?.toolCallId ?? ''); @@ -248,6 +252,8 @@ describe('Studio book cards e2e', () => { toolCallId: held?.toolCallId, card: { kind: 'bookPage', baseVersion: 2, pages: [{ spreadId: j.inner }] }, }); + // Once applied the Studio draws the page as it is: the receipt carries no page of the trial. + expect(receipt?.data.card).not.toHaveProperty('after'); // The pages the answer kept for the undo never reach the model or the chat. expect(JSON.stringify(ofType(decided, 'tool-output-available'))).not.toContain('previous_spreads'); expect(JSON.stringify(ofType(decided, 'tool-output-available'))).not.toContain('page_digests'); diff --git a/server/tests/unit/nest/assistant/cards/book/book-cards.test.ts b/server/tests/unit/nest/assistant/cards/book/book-cards.test.ts index dba71c1ee..e077c02c3 100644 --- a/server/tests/unit/nest/assistant/cards/book/book-cards.test.ts +++ b/server/tests/unit/nest/assistant/cards/book/book-cards.test.ts @@ -1,14 +1,16 @@ /** - * ASTCARD-BKPG-001 to ASTCARD-BKPG-011 + * ASTCARD-BKPG-001 to ASTCARD-BKPG-014 * * The photo book's cards in the Studio's chat: a held book write tried on the * stored book with the shared engine (the pages before and after, numbered * among the double pages, the photos it brings, the findings it leaves and the * new severe ones among them, the engine's refusal in the tool's words), the * change card built from that trial and checked against the shared union, the - * receipt from the tool's answer, the change row's words, and the way back - * from every book write but a new book. Cards go through the registry's guard, - * as a turn builds them. + * page an edit or the cover leaves as the trial made it with the journey's + * context (within the card's cap, in place of the operations if it must, and + * never one the book's schema refuses), the receipt from the tool's answer, + * the change row's words, and the way back from every book write but a new + * book. Cards go through the registry's guard, as a turn builds them. */ import { buildTripCard } from '../../../../../../src/nest/assistant/assistant-context'; import { toolOutputForProvider } from '../../../../../../src/nest/assistant/assistant-output'; @@ -42,11 +44,13 @@ import { applyBookOps, blankBook, bookPageCardSchema, + bookSpreadSchema, MAX_CARD_CHARS, type AssistantChangePreview, type BookDocument, type BookEditResult, type BookPageCard, + type BookTextElement, } from '@trek/shared'; import { beforeEach, describe, expect, it, vi } from 'vitest'; @@ -227,6 +231,8 @@ describe('the change card', () => { const heavy = { spreadId: 'sp-3', ops: words }; const light = cardOf(changeCardOf('edit_book_page', ctx, heavy, loaded(ran('edit_book_page', heavy)))); expect(light.ops).toEqual([]); + // The page they leave is light (one body text, rewritten twenty times), so it still rides for the Studio to draw. + expect(light.after?.id).toBe('sp-3'); expect(JSON.stringify(light).length).toBeLessThan(MAX_CARD_CHARS); }); @@ -444,3 +450,116 @@ describe('the receipt and the way back', () => { expect(trialFrom('text')).toBeNull(); }); }); + +describe('the page after', () => { + /** sp-3 holding photo 81, and a gallery that keeps the photo's caption: the context the server's trial has. */ + function captionable() { + const place = { op: 'place_photo', photoId: 81, target: 'new' }; + const doc = edited(applyBookOps(book(), { spreadId: 'sp-3', ops: [place] })); + const frame = doc.spreads[2]?.elements.find((element) => element.kind === 'photo')?.id ?? ''; + const base = input(doc); + const photos = [{ photoId: 81, caption: 'Tram 28 at dawn' }, { photoId: 82 }]; + return { doc, frame, from: { ...base, ctx: { ...base.ctx, photos } } }; + } + + /** A trial whose page holds `count` texts of `chars` characters each: a page as heavy as a test needs. */ + function weighed(trial: BookTrialRan, count: number, chars: number): BookTrialRan { + const page = trial.spreads[0]; + const words = page?.elements.find((element): element is BookTextElement => element.kind === 'text'); + if (!page || !words) throw new Error('the fixture page has no text'); + const elements = Array.from({ length: count }, (_, i) => ({ ...words, id: `t-w${i}`, text: 'w'.repeat(chars) })); + return { ...trial, spreads: [{ ...page, elements }] }; + } + + it('ASTCARD-BKPG-012: an edit and the cover carry their page as the trial made it, a caption read from the gallery', () => { + const { doc, frame, from } = captionable(); + const args = { spreadId: 'sp-3', ops: [{ op: 'caption', photoElementId: frame }] }; + const trial = ran('edit_book_page', args, from); + expect(trial.spreads.map((spread) => spread.id)).toEqual(['sp-3']); + const card = cardOf(changeCardOf('edit_book_page', ctx, args, loaded(trial))); + // The page itself, as the shared schema reads it: no second run of the operations, here or in the Studio. + expect(card.after).toEqual(bookSpreadSchema.parse(trial.spreads[0])); + expect(card.after?.id).toBe('sp-3'); + // The caption's words are the photo's in the journey, which only the server's context knows. + expect(card.after?.elements).toContainEqual( + expect.objectContaining({ + kind: 'text', + text: 'Tram 28 at dawn', + binding: expect.objectContaining({ source: 'photo.caption', photoId: 81 }), + }), + ); + // Run without that context, as the Studio runs a card's operations, the same edit is refused. + expect(applyBookOps(doc, args)).toMatchObject({ ok: false, error: { code: 'caption_missing' } }); + expect(card.ops).toHaveLength(1); + + const cover = { template: 'cover-full', photoId: 82, title: 'Portugal' }; + const covered = ran('set_book_cover', cover); + const coverCard = cardOf(changeCardOf('set_book_cover', ctx, cover, loaded(covered))); + expect(coverCard.after).toEqual(bookSpreadSchema.parse(covered.spreads[0])); + expect(coverCard.after?.id).toBe('sp-1'); + expect(coverCard.after?.elements).toContainEqual(expect.objectContaining({ kind: 'photo', photoId: 82 })); + expect(coverCard.after?.elements).toContainEqual(expect.objectContaining({ kind: 'text', text: 'Portugal' })); + + // Once applied the Studio draws the page as it is: the receipt carries no page of the trial. + const answer = { version: 4, pages: [{ index: 2, id: 'sp-3', role: 'inner', photoIds: [81] }], findings: [] }; + const preview: AssistantChangePreview = { + toolCallId: 'c1', + verbKey: 'editBookPage', + subject: '', + rows: [], + dayId: null, + placeIds: [], + undoable: true, + card, + }; + const receipt = cardOf(appliedCardOf('edit_book_page', { args, result: answer, preview })); + expect(receipt).toMatchObject({ baseVersion: 4, ops: card.ops }); + expect(receipt.after).toBeUndefined(); + }); + + it('ASTCARD-BKPG-013: the page rides within the cap, in place of the operations if it must; other writes carry none', () => { + const texts = Array.from({ length: 5 }, () => ({ op: 'text', role: 'body', text: 'w'.repeat(2000) })); + const args = { spreadId: 'sp-3', ops: texts }; + const trial = ran('edit_book_page', args); + // Two texts of 7,000 characters and the operations' 10,000 pass the cap together, not apart: the page goes + // without the operations, since the Studio draws the page from it. + const roomy = cardOf(changeCardOf('edit_book_page', ctx, args, loaded(weighed(trial, 2, 7000)))); + expect(roomy.ops).toEqual([]); + expect(roomy.after?.elements).toHaveLength(2); + expect(JSON.stringify(roomy).length).toBeLessThan(MAX_CARD_CHARS); + // Too heavy even then: the card keeps its operations, for the Studio to run, and no page. + const heavy = cardOf(changeCardOf('edit_book_page', ctx, args, loaded(weighed(trial, 3, 8000)))); + expect(heavy.ops).toHaveLength(5); + expect(heavy.after).toBeUndefined(); + expect(JSON.stringify(heavy).length).toBeLessThan(MAX_CARD_CHARS); + // A page that fits rides with the operations. + expect(cardOf(changeCardOf('edit_book_page', ctx, args, loaded(trial)))).toMatchObject({ after: { id: 'sp-3' } }); + expect(cardOf(changeCardOf('edit_book_page', ctx, args, loaded(trial))).ops).toHaveLength(5); + + // New pages, a move and a delete draw no page after: their cards carry none. + const add = { afterSpreadId: 'sp-3', count: 2 }; + expect(cardOf(changeCardOf('add_book_pages', ctx, add, loaded(ran('add_book_pages', add)))).after).toBeUndefined(); + const move = { spreadId: 'sp-3', toIndex: 4 }; + const moved = loaded(ran('move_book_page', move, input(threeSpreads()))); + expect(cardOf(changeCardOf('move_book_page', ctx, move, moved)).after).toBeUndefined(); + const gone = { spreadIds: ['sp-3'] }; + const deleting = loaded(ran('delete_book_pages', gone)); + expect(cardOf(changeCardOf('delete_book_pages', ctx, gone, deleting)).after).toBeUndefined(); + // A trial without its pages is no trial the card reads. + expect(trialFrom({ ok: true, after: [], findings: [] })).toBeNull(); + }); + + it('ASTCARD-BKPG-014: a page the book schema refuses never rides: the card goes out whole with its operations, logged', () => { + const args = { spreadId: 'sp-3', ops: [TWO_UP] }; + const trial = ran('edit_book_page', args); + const unreadable = { ...trial, spreads: trial.spreads.map((spread) => ({ ...spread, background: 'white' })) }; + const card = cardOf(changeCardOf('edit_book_page', ctx, args, loaded(unreadable))); + expect(card).toMatchObject({ kind: 'bookPage', pages: [{ spreadId: 'sp-3' }], ops: [TWO_UP] }); + expect(card.after).toBeUndefined(); + expect(logMock.logWarn).toHaveBeenCalledWith(expect.stringContaining('the page after of edit_book_page')); + // The same trial with a page it reads carries that page, and logs nothing. + vi.clearAllMocks(); + expect(cardOf(changeCardOf('edit_book_page', ctx, args, loaded(trial))).after?.id).toBe('sp-3'); + expect(logMock.logWarn).not.toHaveBeenCalled(); + }); +}); diff --git a/shared/src/assistant/cards/book-page.card.spec.ts b/shared/src/assistant/cards/book-page.card.spec.ts index 3c25f29a3..d7a159a8d 100644 --- a/shared/src/assistant/cards/book-page.card.spec.ts +++ b/shared/src/assistant/cards/book-page.card.spec.ts @@ -4,7 +4,7 @@ import { storedTrekCardSchema, trekCardSchema } from './index'; import { describe, expect, it } from 'vitest'; -// BOOKCARD-001 to BOOKCARD-006 +// BOOKCARD-001 to BOOKCARD-007 function page(over: Partial = {}): BookCardPage { return { spreadId: 'sp-lisbon', index: 2, role: 'inner', number: 1, photos: 3, ...over }; @@ -102,4 +102,18 @@ describe('the book page card', () => { }); expect(applied.entity).toBe('book_page'); }); + + it('BOOKCARD-007: carries the page after as the server made it; without one it reads, an unreadable one drops itself', () => { + const after = { id: 'sp-lisbon', role: 'inner', background: '#ffffff', elements: [], parked: [], entryId: null }; + expect(bookPageCardSchema.parse(card({ after })).after).toEqual(after); + // A card from before the page rode on it, as the thread keeps it, reads as it was. + expect(bookPageCardSchema.parse(card()).after).toBeUndefined(); + expect(storedTrekCardSchema.parse(card())).toMatchObject({ kind: 'bookPage', journeyId: 4 }); + // A page a later build cannot read drops itself, never the card: the Studio then runs the operations. + const unread = { ...after, background: 'white' }; + const kept = bookPageCardSchema.parse(card({ after: unread })); + expect(kept.after).toBeUndefined(); + expect(kept.ops).toHaveLength(2); + expect(storedTrekCardSchema.parse(card({ after: unread }))).toMatchObject({ kind: 'bookPage', journeyId: 4 }); + }); }); diff --git a/shared/src/assistant/cards/book-page.card.ts b/shared/src/assistant/cards/book-page.card.ts index 8783a5b17..a0eef3076 100644 --- a/shared/src/assistant/cards/book-page.card.ts +++ b/shared/src/assistant/cards/book-page.card.ts @@ -14,10 +14,13 @@ import { z } from 'zod'; /** * A change to a journey's photo book as the Studio's chat shows it: the pages * a held book write works on, what it lays them out on and brings onto them, - * and what the server's check of the result found. The card carries the - * page operations themselves, so the Studio draws the page before and after - * with the same engine and the same view the editor uses, never from an image. - * Once applied it names the pages as the change left them. + * and what the server's check of the result found. For an edit or the cover + * it carries the page after as the server's trial of the change made it, so + * the Studio draws the page before and after with the view the editor uses, + * never from an image, and the page after as the server would write it. The + * page operations ride along for a card without that page and for a book + * that moved on since the trial: the Studio runs them on its own book with + * the same engine. Once applied it names the pages as the change left them. */ /** How much of a change a card carries; the server cuts to the same numbers. */ @@ -83,9 +86,25 @@ export const bookPageCardSchema = z.object({ count: cardCountSchema, /** Where a moved page lands among the double pages, from 1. */ movedTo: z.number().int().min(1).max(MAX_SPREADS).nullable(), - /** `edit_book_page`'s operations, to draw the page after; empty for another tool or when too long to carry. */ + /** + * `edit_book_page`'s operations, to draw the page after where the card has + * no `after` or the Studio's book moved on from `baseVersion`; empty for + * another tool, when too long to carry, or when the page after needs their + * room. + */ ops: z.array(bookPageOpSchema).max(BOOK_OP_LIMITS.opsPerCall), cover: bookCardCoverSchema.nullable(), + /** + * The page an edit or the cover leaves, as the server's trial of the change + * made it with the journey's photos, words and figures: a caption from its + * photo's caption, a text from its entry, a map from the route. The Studio + * draws the page after from it while its book is at `baseVersion`. Absent + * on a card from before it, on a page too heavy to carry and on another + * tool's card, where the Studio runs `ops` or `cover` on its own book, as + * it does once its book moved on; absent once applied, where it draws the + * page as it is. One a later build cannot read drops itself, never the card. + */ + after: bookSpreadSchema.optional().catch(undefined), layout: bookCardLayoutSchema.nullable(), /** Photos the change brings onto its pages that were not on them. */ photosAdded: cardCountSchema,