From 83f339ae76f44f500318f87b19d8ce3d144f2a37 Mon Sep 17 00:00:00 2001 From: Maggie Appleton <5599295+MaggieAppleton@users.noreply.github.com> Date: Thu, 29 Oct 2026 06:47:41 -0700 Subject: [PATCH] Let the Planner re-link an answered decision's lost written prose When every paragraph a conversation or Planner decision's write-up was anchored to could no longer be found, the decision card showed "Prose removed". The Planner could re-anchor the decision to the paragraph that now holds that prose and the call succeeded, but it only changed the decision's related blocks: the written prose kept its orphaned anchors, so the card kept saying the prose was removed, even after a restart. An explicit reviewed placement now also replaces the written prose of an answered single-question decision that is not a follow-up, but only when that prose is lost (it has anchors and every one is orphaned). Prose that still resolves is never touched, so routine anchor_plan reviews with multi-block placements or no blocks keep working and keep the write-up. A write-up must be re-linked to one paragraph whose digest still matches, or to no blocks to record it has none. A Planner decision whose dropped follow-up left multi-block prose may name a bounded passage, by the same rules as a follow-up target (a paragraph, then contiguous paragraphs, tables or diagrams, at most eight). Anything else is rejected and leaves the orphaned state untouched. The change is staged with the rest of the re-anchor and committed before it is published. The module is renamed decision-reanchor.ts and its function reanchorDecisionRelationships, since it now handles more than follow-up scopes; the follow-up passage check is shared between the two. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- apps/server/src/agent/document-ops.ts | 4 +- .../conversation-prose-reanchor.test.ts | 278 ++++++++++++++++++ .../questions/decision-followup-reanchor.ts | 72 ----- .../server/src/questions/decision-reanchor.ts | 120 ++++++++ 4 files changed, 400 insertions(+), 74 deletions(-) create mode 100644 apps/server/src/questions/conversation-prose-reanchor.test.ts delete mode 100644 apps/server/src/questions/decision-followup-reanchor.ts create mode 100644 apps/server/src/questions/decision-reanchor.ts 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; +}