diff --git a/apps/server/src/agent/document-ops.ts b/apps/server/src/agent/document-ops.ts index f6a8bba84..f5d7a237e 100644 --- a/apps/server/src/agent/document-ops.ts +++ b/apps/server/src/agent/document-ops.ts @@ -13,7 +13,7 @@ import * as edit from "../plan/edit"; import * as room from "../plan/room"; import * as Questions from "../questions/service"; import * as QuestionStore from "../questions/store"; -import { reanchorScopes } from "../questions/decision-followup-reanchor"; +import { reanchorDecisionRelationships } from "../questions/decision-reanchor"; import { documentIdentity, implementationActive, publishStaged } from "../plan/service"; import type { Plan } from "../plan/service"; @@ -320,7 +320,7 @@ export async function applyAnchors( placements.push({ widget: update.widget, blocks: update.blocks }); } } - failures.push(...reanchorScopes(candidate, placements)); + failures.push(...reanchorDecisionRelationships(candidate, placements)); if (failures.length) return { ok: false, reason: "invalid", errors: failures }; let mutation = Questions.place(candidate, placements); Questions.rebase(candidate, source); diff --git a/apps/server/src/questions/conversation-prose-reanchor.test.ts b/apps/server/src/questions/conversation-prose-reanchor.test.ts new file mode 100644 index 000000000..ba23442b3 --- /dev/null +++ b/apps/server/src/questions/conversation-prose-reanchor.test.ts @@ -0,0 +1,278 @@ +import { expect, test } from "bun:test"; +import { applyAnchors } from "../agent/document-ops"; +import { createDecisionProseFixture } from "../agent/decision-prose.test-fixtures"; +import * as Room from "../plan/room"; +import * as Service from "../plan/service"; +import * as Questions from "./service"; +import type { DocumentRoom } from "../agent/tools"; + +let contexts: ReturnType["contexts"]; +let fixture = createDecisionProseFixture(() => contexts, value => contexts = value); +contexts = fixture.contexts; + +let origins: Array<"conversation" | "planner"> = ["conversation", "planner"]; + +const TABLE = "| Option | Cost |\n| - | - |\n| GitHub Apps | Low |"; + +async function setup( + origin: typeof origins[number], + { prose = [1], orphaned = true, table = false } = {}, +) { + let context = await fixture.opened( + `Original context.\n\nCurrent decision prose.\n\n${ + table ? `${TABLE}\n\n` : "" + }${fixture.WIDGET}`, + ); + let { plan, server } = context; + let record = plan.records.get(fixture.CARD)!; + record.origin = origin; + if (origin === "planner") record.childId = "decision-report"; + let lost = { + ...Room.anchorAt(plan.document, 0, Room.digests(plan.document)[0]!), + orphaned: true as const, + }; + record.prose = prose.map(index => ({ + ...Room.anchorAt(plan.document, index, Room.digests(plan.document)[index]!), + ...(orphaned ? { orphaned: true } : {}), + })); + record.anchors = { + widget: fixture.CARD, + questions: { [fixture.QUESTION]: { anchors: [lost], pending: true, reason: "orphaned" } }, + }; + await Service.persist(plan); + let target: DocumentRoom = { + id: plan.id, + plan, + server, + exclusive: action => Service.exclusive(plan, action), + publish: mutation => Service.publish(plan, server, plan.id, mutation), + persist: () => Service.persist(plan), + anchors: () => Service.anchors(plan, server, plan.id), + changes() {}, + }; + let args = { + revision: plan.revision, + anchors: [{ + widget: fixture.CARD, + question: fixture.QUESTION, + blocks: [{ index: 1, digest: Room.digests(plan.document)[1]! }], + }], + }; + return { ...context, target, args }; +} + +async function restart(context: Awaited>) { + await Service.close(context.plan); + contexts = contexts.filter(value => value.plan !== context.plan); + let plan = await Service.open(context.channel.id, context.backend, context.server); + contexts.push({ ...context, plan }); + return plan; +} + +test.each(origins)( + "explicit %s prose reanchoring commits before publication and survives restart", + async origin => { + let context = await setup(origin); + let { plan, target, args } = context; + let before = structuredClone(plan.records.get(fixture.CARD)!); + let source = Room.project(plan.document); + let broadcasts = context.broadcasts.length; + let commit = context.storage.collaboration.commit; + let release!: () => void; + let entered!: () => void; + let gate = new Promise(resolve => release = resolve); + let committing = new Promise(resolve => entered = resolve); + context.storage.collaboration.commit = async input => { + entered(); + await gate; + return commit(input); + }; + let pending = applyAnchors(target, args); + await committing; + try { + expect(plan.records.get(fixture.CARD)).toEqual(before); + expect(context.broadcasts).toHaveLength(broadcasts); + } finally { + release(); + context.storage.collaboration.commit = commit; + } + expect(await pending).toMatchObject({ ok: true }); + let record = plan.records.get(fixture.CARD)!; + expect(record).toEqual({ ...before, prose: record.prose, anchors: record.anchors }); + expect(record.prose).toHaveLength(1); + expect(Questions.meta(plan, record)).toMatchObject({ hasProse: true, proseOrphaned: false }); + expect(Room.matchesAnchor(plan.document, record.prose![0]!, 1)).toBe(true); + expect(record.choices).toEqual(before.choices); + expect(record.owner).toBe(before.owner); + expect(record.decidedAt).toBe(before.decidedAt); + expect(Room.project(plan.document)).toBe(source); + let restored = await restart(context); + expect(restored.records.get(fixture.CARD)).toEqual(record); + expect(Questions.meta(restored, restored.records.get(fixture.CARD)!)) + .toMatchObject({ hasProse: true, proseOrphaned: false }); + expect(Room.matchesAnchor(restored.document, restored.records.get(fixture.CARD)!.prose![0]!, 1)) + .toBe(true); + }, +); + +test.each(origins)( + "an explicit empty %s review clears orphaned prose without becoming pending or missing", + async origin => { + let context = await setup(origin); + let before = structuredClone(context.plan.records.get(fixture.CARD)!); + context.args.anchors[0]!.blocks = []; + expect(await applyAnchors(context.target, context.args)).toMatchObject({ ok: true }); + let restored = await restart(context); + let record = restored.records.get(fixture.CARD)!; + expect(record).toEqual({ ...before, prose: [], anchors: record.anchors }); + expect(record.prose).toEqual([]); + expect(record.anchors?.questions[fixture.QUESTION]).toEqual({ anchors: [], pending: false }); + expect(Questions.prose(restored)).toEqual([]); + expect(Questions.meta(restored, record)).toMatchObject({ + hasProse: false, + proseOrphaned: false, + }); + }, +); + +test.each(origins)( + "a %s decision without written prose keeps ordinary multi-block relationships", + async origin => { + let context = await setup(origin); + delete context.plan.records.get(fixture.CARD)!.prose; + await Service.persist(context.plan); + context.args.anchors[0]!.blocks = [0, 1].map(index => ({ + index, + digest: Room.digests(context.plan.document)[index]!, + })); + expect(await applyAnchors(context.target, context.args)).toMatchObject({ ok: true }); + let restored = await restart(context); + let record = restored.records.get(fixture.CARD)!; + expect(record.prose).toBeUndefined(); + expect(record.anchors?.questions[fixture.QUESTION].anchors).toHaveLength(2); + }, +); + +test.each( + origins.flatMap(origin => + (["stale", "digest", "nonparagraph", "multiple"] as const).map(kind => [origin, kind] as const) + ), +)( + "a %s decision's %s reanchor preserves the existing orphaned state", + async (origin, kind) => { + let context = await setup(origin); + let before = structuredClone(context.plan.records.get(fixture.CARD)); + let source = Room.project(context.plan.document); + let broadcasts = context.broadcasts.length; + if (kind === "stale") context.args.revision--; + if (kind === "digest") context.args.anchors[0]!.blocks[0]!.digest = `sha256:${"0".repeat(64)}`; + if (kind === "nonparagraph") { + context.args.anchors[0]!.blocks = [{ + index: 2, + digest: Room.digests(context.plan.document)[2]!, + }]; + } + if (kind === "multiple") { + context.args.anchors[0]!.blocks.unshift({ + index: 0, + digest: Room.digests(context.plan.document)[0]!, + }); + } + expect(await applyAnchors(context.target, context.args)).toMatchObject({ ok: false }); + expect(context.plan.records.get(fixture.CARD)).toEqual(before); + expect(Room.project(context.plan.document)).toBe(source); + expect(context.broadcasts).toHaveLength(broadcasts); + let restored = await restart(context); + expect(restored.records.get(fixture.CARD)).toEqual(before); + }, +); + +test.each(origins)( + "a rejected %s commit leaves live and durable prose orphaned and publishes nothing", + async origin => { + let context = await setup(origin); + let before = structuredClone(context.plan.records.get(fixture.CARD)); + let source = Room.project(context.plan.document); + let broadcasts = context.broadcasts.length; + let commit = context.storage.collaboration.commit; + context.storage.collaboration.commit = async () => { + throw new Error("reanchor commit failed"); + }; + try { + await expect(applyAnchors(context.target, context.args)).rejects.toThrow( + "reanchor commit failed", + ); + } finally { + context.storage.collaboration.commit = commit; + } + expect(context.plan.records.get(fixture.CARD)).toEqual(before); + expect(Room.project(context.plan.document)).toBe(source); + expect(context.broadcasts).toHaveLength(broadcasts); + let restored = await restart(context); + expect(restored.records.get(fixture.CARD)).toEqual(before); + }, +); + +function blocks(context: Awaited>, indices: number[]) { + return indices.map(index => ({ index, digest: Room.digests(context.plan.document)[index]! })); +} + +test.each(origins.flatMap(origin => [[origin, [0, 1]], [origin, []]] as const))( + "a %s decision with live prose keeps it when reviewed with blocks %p", + async (origin, indices) => { + let context = await setup(origin, { orphaned: false }); + let before = structuredClone(context.plan.records.get(fixture.CARD)!.prose); + context.args.anchors[0]!.blocks = blocks(context, [...indices]); + expect(await applyAnchors(context.target, context.args)).toMatchObject({ ok: true }); + let restored = await restart(context); + let record = restored.records.get(fixture.CARD)!; + // Placing the card may move the prose paragraph; it must still name that paragraph. + let index = Room.digests(restored.document).indexOf(before![0]!.digest); + expect(record.prose).toHaveLength(1); + expect(record.prose![0]).toMatchObject({ digest: before![0]!.digest }); + expect(record.prose![0]!.orphaned).toBeUndefined(); + expect(Room.matchesAnchor(restored.document, record.prose![0]!, index)).toBe(true); + expect(record.anchors?.questions[fixture.QUESTION].anchors).toHaveLength(indices.length); + expect(Questions.meta(restored, record)).toMatchObject({ + hasProse: true, + proseOrphaned: false, + }); + }, +); + +test.each([false, true])( + "a dropped follow-up's paragraph and table prose is not rejected when orphaned is %p", + async orphaned => { + let context = await setup("planner", { prose: [1, 2], orphaned, table: true }); + let before = structuredClone(context.plan.records.get(fixture.CARD)!.prose); + context.args.anchors[0]!.blocks = blocks(context, [1, 2]); + expect(await applyAnchors(context.target, context.args)).toMatchObject({ ok: true }); + let restored = await restart(context); + let record = restored.records.get(fixture.CARD)!; + if (!orphaned) expect(record.prose).toEqual(before); + expect(record.prose).toHaveLength(2); + expect(record.prose!.every(anchor => !anchor.orphaned)).toBe(true); + expect(Room.matchesAnchor(restored.document, record.prose![0]!, 1)).toBe(true); + expect(Room.matchesAnchor(restored.document, record.prose![1]!, 2)).toBe(true); + }, +); + +test("a dropped follow-up's lost passage takes the table after its named paragraph", async () => { + let context = await setup("planner", { prose: [1, 2], table: true }); + context.args.anchors[0]!.blocks = blocks(context, [1]); + expect(await applyAnchors(context.target, context.args)).toMatchObject({ ok: true }); + let record = (await restart(context)).records.get(fixture.CARD)!; + expect(record.prose).toHaveLength(2); + expect(record.prose!.every(anchor => !anchor.orphaned)).toBe(true); +}); + +test("a dropped follow-up's lost passage must begin with a paragraph", async () => { + let context = await setup("planner", { prose: [1, 2], table: true }); + let before = structuredClone(context.plan.records.get(fixture.CARD)); + context.args.anchors[0]!.blocks = blocks(context, [2]); + expect(await applyAnchors(context.target, context.args)).toMatchObject({ + ok: false, + errors: ["Decision prose must name one bounded passage or no blocks."], + }); + expect((await restart(context)).records.get(fixture.CARD)).toEqual(before); +}); diff --git a/apps/server/src/questions/decision-followup-reanchor.ts b/apps/server/src/questions/decision-followup-reanchor.ts deleted file mode 100644 index 1a3a620c8..000000000 --- a/apps/server/src/questions/decision-followup-reanchor.ts +++ /dev/null @@ -1,72 +0,0 @@ -import { parse } from "@chopin/dialect"; -import { SEECODE_LANGUAGE } from "@chopin/dialect/dialect"; -import * as Room from "../plan/room"; -import type { Plan } from "../plan/service"; -import type { Placement } from "./service-relationships"; - -/** Only an explicit reviewed placement may replace a lost follow-up target. */ -export function reanchorScopes(plan: Plan, updates: Placement[]): string[] { - let byWidget = new Map(updates.map(update => [update.widget, update])); - let scopes = new Set(updates.flatMap(update => { - let record = plan.records.get(update.widget); - return record?.origin === "planner" && record.followUp ? [record.followUp.scope] : []; - })); - let blocks = parse(Room.project(plan.document)).children; - let digests = Room.digests(plan.document); - let errors: string[] = []; - for (let scope of scopes) { - let peers = [...plan.records.values()].filter(record => record.followUp?.scope === scope); - if (peers.every(peer => byWidget.get(peer.id)?.blocks.length === 0)) continue; - if (peers.some(peer => !byWidget.get(peer.id)?.blocks.length)) { - errors.push( - `Re-anchor every decision sharing this scope together: ${ - peers.map(peer => peer.id).join(", ") - }`, - ); - continue; - } - let targets = peers.map(peer => { - let indices = new Set(); - for (let address of byWidget.get(peer.id)!.blocks) { - if (digests[address.index] !== address.digest) return; - indices.add(address.index); - if (blocks[address.index]?.type !== "paragraph") continue; - for (let index = address.index + 1; index < blocks.length; index++) { - let block = blocks[index]!; - if (block.type === "mdxJsxFlowElement" && block.name === "Questionnaire") continue; - if ( - block.type !== "table" && !(block.type === "code" && block.lang === SEECODE_LANGUAGE) - ) break; - indices.add(index); - } - } - let ordered = [...indices].sort((a, b) => a - b); - if (ordered.length > 8 || blocks[ordered[0]!]!.type !== "paragraph") return; - for (let index = ordered[0]!; index <= ordered.at(-1)!; index++) { - let block = blocks[index]!; - if (block.type === "mdxJsxFlowElement" && block.name === "Questionnaire") continue; - if ( - !indices.has(index) || block.type !== "paragraph" && block.type !== "table" - && !(block.type === "code" && block.lang === SEECODE_LANGUAGE) - ) return; - } - return ordered; - }); - let indices = targets[0]; - if (!indices || targets.some(target => JSON.stringify(target) !== JSON.stringify(indices))) { - errors.push( - "Decision scope targets must agree on one bounded passage without unrelated gaps.", - ); - continue; - } - let target = indices.map(index => Room.anchorAt(plan.document, index, digests[index]!)); - for (let peer of peers) { - plan.records.set(peer.id, { - ...peer, - followUp: { ...peer.followUp!, target }, - ...(peer.prose ? { prose: target } : {}), - }); - } - } - return errors; -} diff --git a/apps/server/src/questions/decision-reanchor.ts b/apps/server/src/questions/decision-reanchor.ts new file mode 100644 index 000000000..6fdb6c499 --- /dev/null +++ b/apps/server/src/questions/decision-reanchor.ts @@ -0,0 +1,120 @@ +import { parse } from "@chopin/dialect"; +import { SEECODE_LANGUAGE } from "@chopin/dialect/dialect"; +import * as Room from "../plan/room"; +import type { Plan } from "../plan/service"; +import * as Prose from "./prose"; +import type { Placement } from "./service-relationships"; + +type Block = ReturnType["children"][number]; + +/** + * The bounded passage a follow-up target may name: a paragraph, then contiguous paragraphs, + * tables or diagrams, at most eight. A named paragraph takes the tables and diagrams after it. + */ +function passage( + blocks: Block[], + digests: string[], + addresses: Placement["blocks"], +): number[] | undefined { + let indices = new Set(); + for (let address of addresses) { + if (digests[address.index] !== address.digest) return; + indices.add(address.index); + if (blocks[address.index]?.type !== "paragraph") continue; + for (let index = address.index + 1; index < blocks.length; index++) { + let block = blocks[index]!; + if (block.type === "mdxJsxFlowElement" && block.name === "Questionnaire") continue; + if ( + block.type !== "table" && !(block.type === "code" && block.lang === SEECODE_LANGUAGE) + ) break; + indices.add(index); + } + } + let ordered = [...indices].sort((a, b) => a - b); + if (!ordered.length || ordered.length > 8 || blocks[ordered[0]!]!.type !== "paragraph") return; + for (let index = ordered[0]!; index <= ordered.at(-1)!; index++) { + let block = blocks[index]!; + if (block.type === "mdxJsxFlowElement" && block.name === "Questionnaire") continue; + if ( + !indices.has(index) || block.type !== "paragraph" && block.type !== "table" + && !(block.type === "code" && block.lang === SEECODE_LANGUAGE) + ) return; + } + return ordered; +} + +/** + * Only an explicit reviewed placement may replace a lost follow-up target, or written prose + * whose every block was lost. Prose that still resolves is never rewritten by a placement. + */ +export function reanchorDecisionRelationships(plan: Plan, updates: Placement[]): string[] { + let byWidget = new Map(updates.map(update => [update.widget, update])); + let scopes = new Set(updates.flatMap(update => { + let record = plan.records.get(update.widget); + return record?.origin === "planner" && record.followUp ? [record.followUp.scope] : []; + })); + let blocks = parse(Room.project(plan.document)).children; + let digests = Room.digests(plan.document); + let errors: string[] = []; + for (let update of updates) { + let record = plan.records.get(update.widget); + if ( + !record || record.origin !== "conversation" && record.origin !== "planner" + || record.status !== "answered" || record.definition.questions.length !== 1 + || record.followUp || !Prose.orphaned(record.prose) + ) continue; + // A write-up is one paragraph; a dropped follow-up's prose was a bounded passage. + let bounded = record.origin === "planner" && record.prose!.length > 1; + let indices: number[] | undefined = []; + if (update.blocks.length) { + let [address] = update.blocks; + indices = bounded + ? passage(blocks, digests, update.blocks) + : update.blocks.length === 1 && blocks[address!.index]?.type === "paragraph" + && digests[address!.index] === address!.digest + ? [address!.index] + : undefined; + } + if (!indices) { + errors.push( + bounded + ? "Decision prose must name one bounded passage or no blocks." + : "Decision prose must name one paragraph or no blocks.", + ); + continue; + } + plan.records.set(record.id, { + ...record, + prose: indices.map(index => Room.anchorAt(plan.document, index, digests[index]!)), + }); + } + for (let scope of scopes) { + let peers = [...plan.records.values()].filter(record => record.followUp?.scope === scope); + if (peers.every(peer => byWidget.get(peer.id)?.blocks.length === 0)) continue; + if (peers.some(peer => !byWidget.get(peer.id)?.blocks.length)) { + errors.push( + `Re-anchor every decision sharing this scope together: ${ + peers.map(peer => peer.id).join(", ") + }`, + ); + continue; + } + let targets = peers.map(peer => passage(blocks, digests, byWidget.get(peer.id)!.blocks)); + let indices = targets[0]; + if (!indices || targets.some(target => JSON.stringify(target) !== JSON.stringify(indices))) { + errors.push( + "Decision scope targets must agree on one bounded passage without unrelated gaps.", + ); + continue; + } + let target = indices.map(index => Room.anchorAt(plan.document, index, digests[index]!)); + for (let peer of peers) { + plan.records.set(peer.id, { + ...peer, + followUp: { ...peer.followUp!, target }, + ...(peer.prose ? { prose: target } : {}), + }); + } + } + return errors; +}