diff --git a/apps/server/src/chat/draft.test.ts b/apps/server/src/chat/draft.test.ts index 90d0d503e..9d4d378a4 100644 --- a/apps/server/src/chat/draft.test.ts +++ b/apps/server/src/chat/draft.test.ts @@ -269,6 +269,8 @@ test("the draft instruction comes from the plan and refuses an unsettled one", a let built = draftInstruction(plan, "ana", true); expect(built.text).toContain("Chopin approves and starts these tasks automatically"); expect(built.text).toContain("do not describe them as unapproved or unstarted"); + expect(built.text).toContain("do not add a task that prototypes that passage again"); + expect(prepare.text).not.toContain("Prototyping"); expect(built.text).not.toContain("Do not approve or start implementation."); expect(built.said).toBe(prepare.said); expect(draftRefusal(plan)).toBeUndefined(); diff --git a/apps/server/src/experiments/spikes.test.ts b/apps/server/src/experiments/spikes.test.ts index 5eb18408d..d8bac2bbf 100644 --- a/apps/server/src/experiments/spikes.test.ts +++ b/apps/server/src/experiments/spikes.test.ts @@ -66,6 +66,8 @@ test("a submitted spike renders as a titled callout with findings and screenshot expect(source).toContain(`title="Drag handles work on touch with a 44px target"`); expect(source).toContain("- Pointer events fire on iOS Safari."); expect(source).toContain("**Recommendation:** Keep drag handles"); + // The recommendation leads; the findings that support it follow. + expect(source.indexOf("**Recommendation:**")).toBeLessThan(source.indexOf("- Pointer events")); expect(source).toContain(`![Prototype screenshot 1](${IMAGE})`); room.validate(source); let node = parse(source).children[0]; @@ -75,7 +77,7 @@ test("a submitted spike renders as a titled callout with findings and screenshot test("running and stopped spikes render their own callouts without naming a machine", () => { let running = serialize({ type: "root", children: [spikeCallout(spike("running"))] }); expect(running).toContain(`title="Prototyping…"`); - expect(running).toContain("maggie's coding agent"); + expect(running).toContain("@maggie’s coding agent"); room.validate(running); let stopped = spike("failed"); stopped.progress = "Agent stopped: cancelled"; @@ -86,7 +88,7 @@ test("running and stopped spikes render their own callouts without naming a mach expect(renderKey(spike("requested"))).toBe("queued"); expect(renderKey(spike("queued"))).toBe("queued"); let queued = serialize({ type: "root", children: [spikeCallout(spike("queued"))] }); - expect(queued).toContain(`title="Queued"`); + expect(queued).toContain(`title="Prototype queued"`); expect(queued).not.toContain("Prototyping"); room.validate(queued); expect(renderKey(spike("interrupted"))).toBe("stopped"); diff --git a/apps/server/src/experiments/spikes.ts b/apps/server/src/experiments/spikes.ts index f9a98802a..876968b3e 100644 --- a/apps/server/src/experiments/spikes.ts +++ b/apps/server/src/experiments/spikes.ts @@ -55,7 +55,7 @@ function paragraph(...children: PhrasingContent[]): BlockContent { return { type: "paragraph", children }; } -/** Canonical report MDX: bold headline, findings, recommendation, then screenshots. */ +/** Canonical report MDX: bold headline, the recommendation first, findings, then screenshots. */ export function spikeReport(input: SpikeSubmission): string { let items: ListItem[] = input.findings.map(finding => ({ type: "listItem", @@ -66,11 +66,11 @@ export function spikeReport(input: SpikeSubmission): string { type: "root", children: [ paragraph({ type: "strong", children: [{ type: "text", value: input.headline }] }), - { type: "list", ordered: false, spread: false, children: items }, paragraph( { type: "strong", children: [{ type: "text", value: "Recommendation:" }] }, { type: "text", value: ` ${input.recommendation}` }, ), + { type: "list", ordered: false, spread: false, children: items }, ...input.images.map((url, index) => paragraph({ type: "image", url, alt: `Prototype screenshot ${index + 1}` }) ), @@ -121,19 +121,19 @@ export function spikeCallout(value: Investigation): RootContent { ]); } if (key === "queued") { - return callout(spike.callout, "note", "Queued", [ + return callout(spike.callout, "note", "Prototype queued", [ paragraph({ type: "text", - value: `${spike.login}'s coding agent will build a quick prototype to test the passage ` - + "above once it finishes its current work. Delete this callout to cancel it.", + value: `@${spike.login}’s coding agent will prototype the passage above when it’s free. ` + + "Delete this callout to cancel.", }), ]); } return callout(spike.callout, "note", "Prototyping…", [ paragraph({ type: "text", - value: `${spike.login}'s coding agent is building a quick prototype to test the passage ` - + "above. Delete this callout to stop it.", + value: `@${spike.login}’s coding agent is testing the passage above with a quick prototype. ` + + "Delete this callout to stop it.", }), ]); } diff --git a/apps/server/src/tasks/builds.ts b/apps/server/src/tasks/builds.ts index 34f53abde..a57d8db85 100644 --- a/apps/server/src/tasks/builds.ts +++ b/apps/server/src/tasks/builds.ts @@ -774,8 +774,12 @@ export function reportRebuild( `Living-document rebuild of revisions ${live.baseRevision} to ${target.revision}.`, goal: task.goal, acceptance: [ - `Committed on ${task.pullRequest}.`, - `Reflects the document change since revision ${live.baseRevision}.`, + `Updates pull request ${ + task.pullRequest.startsWith(prefix) + ? `#${task.pullRequest.slice(prefix.length)}` + : task.pullRequest + }.`, + "Matches the document as edited.", ], dependsOn: [], })), @@ -884,8 +888,10 @@ export function liveSnapshot(plan: Plan, connections: Connection[]): LiveSnapsho // The first build's tasks, then each rebuild's appended version, with their last reported state. let first = plan.builds.find(build => build.id === live.buildId)?.graphVersion; let history = plan.graph ? historyFor(plan.graph, plan.lifecycle) : []; + // Only versions that ran: a Planner draft started since would list unbuilt duplicates. let tasks = (plan.graph?.versions ?? []).filter(version => first !== undefined && version.number >= first + && history.some(item => item.run.graphVersion === version.number) ).flatMap(version => { let progress = history.findLast(item => item.run.graphVersion === version.number)?.progress; return version.definition.tasks.map(task => { diff --git a/apps/server/src/tasks/draft.ts b/apps/server/src/tasks/draft.ts index e7075e5c3..2b36f666a 100644 --- a/apps/server/src/tasks/draft.ts +++ b/apps/server/src/tasks/draft.ts @@ -28,7 +28,7 @@ export function draftInstruction( build = false, ): { text: string; said: string } { let close = build - ? "Do not approve or start implementation yourself: Chopin approves and starts these tasks automatically once they are saved, so do not describe them as unapproved or unstarted." + ? "Do not approve or start implementation yourself: Chopin approves and starts these tasks automatically once they are saved, so do not describe them as unapproved or unstarted. A note Callout titled “Prototyping…” marks a passage a prototype is already testing, and its result lands in the document before these tasks start: do not add a task that prototypes that passage again; have the task that implements it follow the prototype's result." : "Do not approve or start implementation."; let graph = plan.graph?.versions.at(-1); if (!graph) { diff --git a/apps/server/src/tasks/routes.test.ts b/apps/server/src/tasks/routes.test.ts index 08c9b58ea..3720ff345 100644 --- a/apps/server/src/tasks/routes.test.ts +++ b/apps/server/src/tasks/routes.test.ts @@ -823,6 +823,16 @@ test("report_rebuild appends a completed version, records commits and advances t }); expect(snapshot.live.baseSource).toBeUndefined(); expect(snapshot.lifecycle.history.at(-1).outcome.kind).toBe("implemented"); + expect(snapshot.live.tasks.at(-1)).toMatchObject({ + id: "rebuild-2-1", + acceptance: ["Updates pull request #7.", "Matches the document as edited."], + }); + // A Planner draft started after the build is not built work, so the list ignores it. + plan.graph!.versions.push({ ...structuredClone(plan.graph!.versions[0]!), number: 3 }); + plan.graph!.versions.at(-1)!.state = "draft"; + let drafted = await (await context.call(context.path)).json(); + expect(drafted.live.tasks).toEqual(snapshot.live.tasks); + plan.graph!.versions.pop(); let live = plan.live; await Plan.close(plan); closed = true; diff --git a/apps/web/src/build-model.test.ts b/apps/web/src/build-model.test.ts index 1f7645c42..0c584793d 100644 --- a/apps/web/src/build-model.test.ts +++ b/apps/web/src/build-model.test.ts @@ -5,20 +5,29 @@ import { advanceFirstBuild, ago, attentionHint, + blockerLabelled, + blockerText, buildPhase, draftKey, draftRefusalCopy, elapsed, firstBuildStep, + linkParts, + liveTaskGroups, + liveTaskState, pullRequestCommits, pullRequestNumber, + shortUrl, shouldAutoDraft, startedBy, startingHint, startingLabel, syncHint, + syncLabel, syncStatus, + syncTooltip, taskStartsOpen, + unfinishedReason, waitingLabel, } from "./build-model"; @@ -439,7 +448,16 @@ describe("living document sync", () => { expect(syncStatus(live({ outOfSync: true, rebuild: rebuild(state) }))) .toEqual({ kind: "building" }); } - expect(syncStatus({ ...live(), build: build("running") })).toEqual({ kind: "building" }); + expect(syncStatus({ ...live(), build: build("running") })) + .toEqual({ kind: "building", first: true }); + }); + + it("keeps the first build's label while it finishes after delivering", () => { + let finishing = syncStatus({ ...live(), build: build("running") })!; + expect(syncLabel(finishing)).toBe("Building…"); + expect(syncTooltip(finishing, undefined, "me")).toBe("Finishing the first build"); + expect(syncLabel(syncStatus(live({ outOfSync: true, rebuild: rebuild("running") }))!)) + .toBe("Syncing…"); }); it("explains a first build queued behind another document's build", () => { @@ -473,14 +491,19 @@ describe("living document sync", () => { it("explains why it is out of sync", () => { let pending = live({ outOfSync: true, rebuild: rebuild("stopped") }); expect(syncStatus(pending)).toEqual({ kind: "out-of-sync", reason: "pending", outstanding: 0 }); - expect(syncHint(syncStatus(pending), pending, "me")).toBe("Changes will build shortly"); + expect(syncHint(syncStatus(pending), pending, "me")).toBe("Your edits will sync shortly"); let waiting = { ...live({ outOfSync: true, builderConnected: false }), builtBy: "jev" }; expect(syncStatus(waiting)).toEqual({ kind: "out-of-sync", reason: "waiting", outstanding: 0 }); expect(syncHint(syncStatus(waiting), waiting, "me")).toBe("Waiting for @jev’s agent"); expect(syncHint(syncStatus(waiting), waiting, "u")).toBe("Waiting for your agent"); let failed = live({ outOfSync: true, rebuild: rebuild("failed") }); expect(syncStatus(failed)).toEqual({ kind: "out-of-sync", reason: "failed", outstanding: 0 }); - expect(syncHint(syncStatus(failed), failed, "me")).toBe("The last rebuild failed"); + expect(syncHint(syncStatus(failed), failed, "me")).toBe("The next edit will try again"); + expect(syncLabel({ kind: "out-of-sync", reason: "failed", outstanding: 0 })).toBe( + "Sync failed", + ); + expect(syncLabel({ kind: "out-of-sync", reason: "waiting", outstanding: 0 })) + .toBe("Out of sync"); }); it("needs attention while a blocked task waits for an edit, even in sync", () => { @@ -492,27 +515,135 @@ describe("living document sync", () => { expect(syncStatus(stuck)).toEqual({ kind: "needs-attention", outstanding: 2 }); expect(syncHint(syncStatus(stuck), stuck, "me")) .toBe( - "“Store graphs” is blocked: Which database? (and 1 other). Edit the document to retry.", + "“Store graphs” is blocked: Which database? (and 1 other). Edit the document to retry them.", ); let edited = live({ outstandingTasks, outOfSync: true }); expect(syncStatus(edited)).toEqual({ kind: "out-of-sync", reason: "pending", outstanding: 2 }); expect(syncHint(syncStatus(edited), edited, "me")) - .toBe("Changes will build shortly · will also retry 2 blocked tasks"); + .toBe("Your edits and 2 blocked tasks will sync shortly"); + let waiting = { ...live({ outstandingTasks, outOfSync: true, builderConnected: false }) }; + expect(syncHint(syncStatus(waiting), waiting, "u")) + .toBe("Waiting for your agent to sync your edits and 2 blocked tasks"); + let failed = live({ outstandingTasks, outOfSync: true, rebuild: rebuild("failed") }); + expect(syncHint(syncStatus(failed), failed, "me")) + .toBe("The next edit will retry the sync and 2 blocked tasks"); + expect(syncLabel(syncStatus(stuck)!)).toBe("Needs attention"); + expect(syncTooltip(syncStatus(stuck)!, stuck, "me")).toContain( + "Edit the document to retry them.", + ); // Syncing beats needing attention. expect(syncStatus(live({ outstandingTasks, rebuild: rebuild("running") }))) .toEqual({ kind: "building" }); }); + it("leaves the blocker to the task row in the Build view's brief hint", () => { + let stuck = live({ + outstandingTasks: [ + { id: "a", title: "Store graphs", state: "blocked", blocker: "Which database?" }, + { id: "b", title: "Render graphs", state: "queued" }, + ], + }); + expect(attentionHint(stuck, true)) + .toBe("Edit the document to retry “Store graphs” and 1 other"); + let unfinished = live({ outstandingTasks: [{ id: "a", title: "Ship", state: "queued" }] }); + expect(attentionHint(unfinished, true)).toBe("Edit the document to retry “Ship”"); + }); + + it("caps a long blocker in the tooltip at a word boundary", () => { + let blocker = `The repository has no app.${" Should I scaffold one?".repeat(10)}`; + let hint = attentionHint(live({ + outstandingTasks: [{ id: "a", title: "Ship", state: "blocked", blocker }], + }))!; + expect(hint).toMatch(/^“Ship” is blocked: The repository has no app\. Should I .*…/); + expect(hint.endsWith("… Edit the document to retry it.")).toBe(true); + expect(hint.length).toBeLessThan(180); + }); + + it("says why an unfinished task stopped: its last report, else where the agent stopped", () => { + expect(unfinishedReason(undefined)).toBe("Your agent stopped before opening a pull request"); + expect(unfinishedReason({ pullRequest: { url: PR, state: "open" } })) + .toBe("Your agent stopped before finishing this task"); + expect(unfinishedReason({ summary: " Tests still fail on CI. " })) + .toBe("Tests still fail on CI."); + let task = (progress: object) => ({ + id: "a", + title: "Ship", + context: "", + goal: "", + acceptance: [], + dependsOn: [], + progress: { id: "a", state: "queued" as const, ...progress }, + }); + let quiet = live({ + outstandingTasks: [{ id: "a", title: "Ship", state: "queued" }], + tasks: [task({})], + }); + expect(attentionHint(quiet)).toBe( + "“Ship” didn’t finish: your agent stopped before opening a pull request. Edit the document to retry it.", + ); + let reported = live({ + outstandingTasks: [{ id: "a", title: "Ship", state: "queued" }], + tasks: [task({ summary: "Waiting on a review of https://github.com/o/r/pull/7." })], + }); + expect(attentionHint(reported)).toBe( + "“Ship” didn’t finish: Waiting on a review of o/r#7. Edit the document to retry it.", + ); + }); + + it("drops an agent's stacked Blocked: labels and keeps its own", () => { + expect(blockerText("Blocked: Blocker - Which database?")).toBe("Which database?"); + expect(blockerText("Blocked by CI")).toBe("Blocked by CI"); + expect(blockerLabelled("Awaiting human confirmation: Jev must confirm")).toBe(true); + expect(blockerLabelled("Which database? Postgres: or SQLite")).toBe(false); + expect(blockerLabelled("https://github.com/o/r/pull/1 fails")).toBe(false); + let hint = attentionHint(live({ + outstandingTasks: [{ + id: "a", + title: "Ship", + state: "blocked", + blocker: "Blocked: Awaiting review: see https://github.com/o/r/pull/28#issuecomment-339.", + }], + })); + expect(hint).toBe( + "“Ship” is blocked: Awaiting review: see o/r#28 comment. Edit the document to retry it.", + ); + }); + + it("shortens GitHub links and leaves other links whole", () => { + expect(shortUrl("https://github.com/o/r/pull/28")).toBe("o/r#28"); + expect(shortUrl("https://github.com/o/r/issues/3#issuecomment-12")).toBe("o/r#3 comment"); + expect(shortUrl("https://github.com/o/r/pull/28/files")).toBe("o/r#28"); + expect(shortUrl("https://example.com/o/r/pull/28")).toBeUndefined(); + expect( + linkParts("on PR #28 (https://github.com/o/r/pull/28#issuecomment-9). See https://x.dev/a."), + ).toEqual([ + { text: "on PR #28 (" }, + { text: "o/r#28 comment", href: "https://github.com/o/r/pull/28#issuecomment-9" }, + { text: "). See " }, + { text: "https://x.dev/a", href: "https://x.dev/a" }, + { text: "." }, + ]); + expect(linkParts("no links")).toEqual([{ text: "no links" }]); + }); + + it("shows a task the first build left unfinished as needing attention until a sync runs", () => { + expect(liveTaskState("queued", true, false)).toBe("blocked"); + expect(liveTaskState("queued", true, true)).toBe("queued"); + expect(liveTaskState("queued", false, false)).toBe("queued"); + expect(liveTaskState("in_progress", true, false)).toBe("in_progress"); + expect(liveTaskState("completed", true, false)).toBe("completed"); + }); + it("ends a blocker's hint with one stop, whatever punctuation it brought", () => { let hint = (blocker: string) => attentionHint(live({ outstandingTasks: [{ id: "a", title: "Ship", state: "blocked", blocker }], })); expect(hint("I can't decide this myself.")).toBe( - "“Ship” is blocked: I can't decide this myself. Edit the document to retry.", + "“Ship” is blocked: I can't decide this myself. Edit the document to retry it.", ); expect(hint("Which database?")).toBe( - "“Ship” is blocked: Which database? Edit the document to retry.", + "“Ship” is blocked: Which database? Edit the document to retry it.", ); }); @@ -526,7 +657,7 @@ describe("living document sync", () => { sync: { kind: "needs-attention", outstanding: 1 }, }); expect(attentionHint(stopped)).toBe( - "“Ship” is blocked: Pick a host. Edit the document to retry.", + "“Ship” is blocked: Pick a host. Edit the document to retry it.", ); }); @@ -534,6 +665,27 @@ describe("living document sync", () => { expect(syncStatus(live({ rebuild: rebuild("failed") }))).toEqual({ kind: "in-sync" }); expect(syncHint({ kind: "in-sync" }, live(), "me")).toBeUndefined(); }); + + it("names every state with one sync vocabulary and a tooltip", () => { + expect(syncLabel({ kind: "in-sync" })).toBe("In sync"); + expect(syncLabel({ kind: "building" })).toBe("Syncing…"); + expect(syncTooltip({ kind: "in-sync" }, live(), "me")).toBe("Pull requests match the document"); + expect(syncTooltip({ kind: "building" }, live(), "me")) + .toBe("Updating pull requests to match the document"); + let pending = live({ outOfSync: true, rebuild: rebuild("stopped") }); + expect(syncTooltip(syncStatus(pending)!, pending, "me")).toBe("Your edits will sync shortly"); + }); + + it("splits the first build's tasks from those later syncs added", () => { + let tasks = [{ id: "workspace" }, { id: "rebuild-2-1" }, { id: "notes" }, { + id: "rebuild-3-1", + }]; + expect(liveTaskGroups(tasks)).toEqual({ + first: [{ id: "workspace" }, { id: "notes" }], + since: [{ id: "rebuild-2-1" }, { id: "rebuild-3-1" }], + }); + expect(liveTaskGroups([{ id: "workspace" }]).since).toEqual([]); + }); }); describe("living document commits", () => { diff --git a/apps/web/src/build-model.ts b/apps/web/src/build-model.ts index c4d7ffcca..8594ec52c 100644 --- a/apps/web/src/build-model.ts +++ b/apps/web/src/build-model.ts @@ -29,7 +29,8 @@ export type BuildPhase = * them, so a document otherwise in sync with any `needs-attention`. */ export type SyncStatus = - | { kind: "building" } + /** `first` while the first build finishes, so it keeps its own label until it ends. */ + | { kind: "building"; first?: true } | { kind: "in-sync" } | { kind: "needs-attention"; outstanding: number } | { kind: "out-of-sync"; reason: "pending" | "waiting" | "failed"; outstanding: number }; @@ -44,11 +45,11 @@ const RUNNING = ["queued", "starting", "running"]; export function syncStatus(snapshot: Snapshot | undefined): SyncStatus | undefined { let live = snapshot?.live; if (!snapshot || !live) return; + if (live.rebuild && RUNNING.includes(live.rebuild.state)) return { kind: "building" }; if ( - live.rebuild && RUNNING.includes(live.rebuild.state) - || snapshot.build && RUNNING.includes(snapshot.build.state) + snapshot.build && RUNNING.includes(snapshot.build.state) || snapshot.lifecycle.execution.state === "active" - ) return { kind: "building" }; + ) return { kind: "building", first: true }; let outstanding = live.outstandingTasks?.length ?? 0; if (!live.outOfSync) { return outstanding ? { kind: "needs-attention", outstanding } : { kind: "in-sync" }; @@ -63,12 +64,13 @@ export function syncStatus(snapshot: Snapshot | undefined): SyncStatus | undefin }; } -export const SYNC_LABEL: Record = { - building: "Building…", - "in-sync": "In sync", - "needs-attention": "Needs attention", - "out-of-sync": "Out of sync", -}; +/** One vocabulary for a living document: in sync, out of sync, syncing, or a failed sync. */ +export function syncLabel(status: SyncStatus): string { + if (status.kind === "building") return status.first ? "Building…" : "Syncing…"; + if (status.kind === "in-sync") return "In sync"; + if (status.kind === "needs-attention") return "Needs attention"; + return status.reason === "failed" ? "Sync failed" : "Out of sync"; +} /** Why the pull requests lag the document, naming the builder whose agent must run them. */ export function syncHint( @@ -78,28 +80,146 @@ export function syncHint( ): string | undefined { if (status?.kind === "needs-attention") return attentionHint(snapshot); if (status?.kind !== "out-of-sync") return; - let retry = status.outstanding - ? ` · will also retry ${plural(status.outstanding, "blocked task", "blocked tasks")}` - : ""; - if (status.reason === "pending") return `Changes will build shortly${retry}`; - if (status.reason === "failed") return `The last rebuild failed${retry}`; - if (snapshot?.live?.user === userId) return `Waiting for your agent${retry}`; - return snapshot?.builtBy - ? `Waiting for @${snapshot.builtBy}’s agent${retry}` - : `Waiting for the builder’s agent${retry}`; -} - -/** What an unfinished task is stuck on, and that an edit retries it. */ -export function attentionHint(snapshot: Snapshot | undefined): string | undefined { + let tasks = status.outstanding + ? plural(status.outstanding, "blocked task", "blocked tasks") + : undefined; + if (status.reason === "pending") { + return tasks ? `Your edits and ${tasks} will sync shortly` : "Your edits will sync shortly"; + } + // Every edit schedules another sync, so a failure is retried by the next one. + if (status.reason === "failed") { + return tasks + ? `The next edit will retry the sync and ${tasks}` + : "The next edit will try again"; + } + let agent = snapshot?.live?.user === userId + ? "your agent" + : snapshot?.builtBy + ? `@${snapshot.builtBy}’s agent` + : "the builder’s agent"; + return tasks ? `Waiting for ${agent} to sync your edits and ${tasks}` : `Waiting for ${agent}`; +} + +/** The longest blocker a tooltip quotes; the Build view's task row shows all of it. */ +const BLOCKER_CHARS = 120; + +/** Labels an agent puts before its reason, which the row already says: "Blocked: …". */ +const BLOCKER_LABEL = /^\s*(?:blocked|blocker)\s*[:–—-]\s*/i; +const LINK = /https?:\/\/[^\s<>()"“”]+[^\s<>()"“”.,;:!?'’]/g; + +/** An agent's blocker without the "Blocked:" labels it stacked in front. */ +export function blockerText(raw: string): string { + let text = raw.trim(); + while (BLOCKER_LABEL.test(text)) text = text.replace(BLOCKER_LABEL, ""); + return text; +} + +/** Whether a reason opens with its own short label, such as "Awaiting review: …". */ +export function blockerLabelled(text: string): boolean { + return /^[^\s:][^:.?!]{0,40}:\s/.test(text) && !/^https?:/.test(text); +} + +/** A GitHub pull request or issue link as `owner/repo#N`, noting a link to one comment. */ +export function shortUrl(url: string): string | undefined { + let match = url.match( + /^https:\/\/github\.com\/([\w.-]+\/[\w.-]+)\/(?:pull|issues)\/(\d+)(?:[/?#](\S*))?$/, + ); + if (!match) return; + let comment = /(?:^|[#&])(?:issuecomment|discussion_r|pullrequestreview)-?\d/.test( + match[3] ?? "", + ); + return `${match[1]}#${match[2]}${comment ? " comment" : ""}`; +} + +/** Prose with its links split out, each labelled for reading. */ +export function linkParts(text: string): Array<{ text: string; href?: string }> { + let parts: Array<{ text: string; href?: string }> = []; + let last = 0; + for (let match of text.matchAll(LINK)) { + if (match.index > last) parts.push({ text: text.slice(last, match.index) }); + parts.push({ text: shortUrl(match[0]) ?? match[0], href: match[0] }); + last = match.index + match[0].length; + } + if (last < text.length) parts.push({ text: text.slice(last) }); + return parts; +} + +/** One line of an agent's text for a tooltip: links shortened, capped at a word boundary. */ +function quote(raw: string): string { + let text = linkParts(blockerText(raw)).map(part => part.text).join("") + .replace(/\s+/g, " ").replace(/\.+$/, ""); + return text.length > BLOCKER_CHARS + ? `${text.slice(0, BLOCKER_CHARS).replace(/\s+\S*$/, "")}…` + : text; +} + +/** + * Why a task the build left unfinished stopped: the agent's last report, or where it + * stopped when it said nothing. + */ +export function unfinishedReason( + progress: { summary?: string; pullRequest?: unknown } | undefined, +): string { + let summary = progress?.summary?.trim(); + if (summary) return summary; + return progress?.pullRequest + ? "Your agent stopped before finishing this task" + : "Your agent stopped before opening a pull request"; +} + +/** + * What an unfinished task is stuck on, and that an edit retries it. `brief` names only the + * task to retry, for the Build view, whose task row already says why. + */ +export function attentionHint( + snapshot: Snapshot | undefined, + brief = false, +): string | undefined { let tasks = snapshot?.live?.outstandingTasks ?? []; if (!tasks.length) return; let first = tasks.find(task => task.blocker) ?? tasks[0]!; + if (brief) { + let others = tasks.length > 1 ? ` and ${plural(tasks.length - 1, "other", "others")}` : ""; + return `Edit the document to retry “${first.title}”${others}`; + } + let progress = snapshot?.live?.tasks.find(task => task.id === first.id)?.progress; let reason = first.blocker - ? `“${first.title}” is blocked: ${first.blocker.trim().replace(/\.+$/, "")}` - : `“${first.title}” didn’t finish`; + ? `“${first.title}” is blocked: ${quote(first.blocker)}` + : `“${first.title}” didn’t finish: ${ + quote(unfinishedReason(progress)).replace(/^Your agent/, "your agent") + }`; let more = tasks.length > 1 ? ` (and ${plural(tasks.length - 1, "other", "others")})` : ""; let said = `${reason}${more}`; - return `${said}${/[?!…]$/.test(said) ? "" : "."} Edit the document to retry.`; + return `${said}${/[?!…]$/.test(said) ? "" : "."} Edit the document to retry ${ + tasks.length > 1 ? "them" : "it" + }.`; +} + +/** + * How a living document's task row reads: a task the first build left unfinished shows + * as needing attention, not as queued, until a sync picks it up again. + */ +export function liveTaskState( + state: TaskState, + outstanding: boolean, + syncing: boolean, +): TaskState { + return outstanding && !syncing && state === "queued" ? "blocked" : state; +} + +/** The header's tooltip: what the sync status means, or why it lags. */ +export function syncTooltip( + status: SyncStatus, + snapshot: Snapshot | undefined, + userId: string | undefined, +): string { + if (status.kind === "in-sync") return "Pull requests match the document"; + if (status.kind === "building") { + return status.first + ? "Finishing the first build" + : "Updating pull requests to match the document"; + } + return syncHint(status, snapshot, userId) ?? "Pull requests lag the document"; } /** Why a first build has not started yet, when the viewer's agent is finishing a prototype. */ @@ -125,6 +245,15 @@ export function startingLabel( return hint ? { label: "Queued", queued: true, hint } : { label: "Building…", queued: false }; } +/** + * A living document's tasks split at its first build: the tasks it started with, then the + * ones each later sync added. The server names a sync's tasks `rebuild--`. + */ +export function liveTaskGroups(tasks: T[]): { first: T[]; since: T[] } { + let since = tasks.filter(task => task.id.startsWith("rebuild-")); + return { first: tasks.filter(task => !since.includes(task)), since }; +} + /** One pull request's living-document commits, newest first. */ export function pullRequestCommits(snapshot: Snapshot | undefined, url: string) { return (snapshot?.live?.commits ?? []).map((commit, index) => ({ commit, index })) diff --git a/apps/web/src/build-plan-button.tsx b/apps/web/src/build-plan-button.tsx index 7a5846c04..ade81f0ee 100644 --- a/apps/web/src/build-plan-button.tsx +++ b/apps/web/src/build-plan-button.tsx @@ -1,4 +1,4 @@ -import { CheckIcon, LoaderIcon } from "@chopin/icons"; +import { CheckIcon, ClockIcon, DecisionIcon, LoaderIcon } from "@chopin/icons"; import { useEffect, useReducer, useRef, useState } from "react"; import { @@ -7,9 +7,9 @@ import { draftRefusalCopy, firstBuildStep, startingLabel, - SYNC_LABEL, - syncHint, + syncLabel, syncStatus, + syncTooltip, waitingLabel, } from "./build-model"; import { @@ -21,6 +21,7 @@ import { import type { Implementation, ImplementationSnapshot } from "@chopin/protocol/implementation"; import type { FirstBuild } from "./build-model"; +import type { WorkspaceDocumentView } from "./workspace-model"; import type { Wire } from "./wire"; const RELOAD_ON = [ @@ -35,14 +36,21 @@ const RELOAD_ON = [ * start them on the viewer's local agent. The Build view owns every later build. * Once that build has delivered, the slot quietly reports whether the pull * requests still match the living document. + * + * Every state but Build plan points at a view, and hides while that view shows, + * because the view's own status line already says the same thing. */ export function BuildPlanButton( - { onNeedsAgent, onShowBuild, onShowDecisions, room, userId, wire }: { + { onNeedsAgent, onShowBuild, onShowDecisions, onWaiting, room, showing, userId, wire }: { /** No local agent could take the build; the Build view explains how to start one. */ onNeedsAgent: () => void; onShowBuild: () => void; onShowDecisions: () => void; + /** Whether the one-click build waits on open decisions, which Decisions then says. */ + onWaiting?: (decisions: boolean) => void; room: string; + /** The view the document pane shows now, if it is visible. */ + showing?: WorkspaceDocumentView; userId?: string; wire?: Wire; }, @@ -146,62 +154,77 @@ export function BuildPlanButton( }); }, [step.next, snapshot?.revision]); + let waitingOnDecisions = !syncStatus(snapshot) && step.view === "waiting" + && waitingLabel(snapshot).target === "decisions"; + useEffect(() => { + onWaiting?.(waitingOnDecisions); + }, [waitingOnDecisions]); + useEffect(() => () => onWaiting?.(false), []); + let sync = syncStatus(snapshot); if (sync) { - let hint = syncHint(sync, snapshot, userId); + if (showing === "build") return null; + let hint = syncTooltip(sync, snapshot, userId); return ( ); } if (step.view === "hidden") return null; if (step.view === "waiting") { let { label, target } = waitingLabel(snapshot); - let hint = "Building starts once this is resolved"; + if (showing === target) return null; + let hint = target === "decisions" + ? "The build starts once the open decisions are answered" + : "The build starts once the document is updated"; return ( ); } if (step.view === "working") { + if (showing === "build") return null; let { hint, label, queued } = startingLabel(snapshot); return ( @@ -222,12 +245,16 @@ export function BuildPlanButton( <> {failure && {failure}} diff --git a/apps/web/src/build-view.css b/apps/web/src/build-view.css index b5d4c6664..9e878e31b 100644 --- a/apps/web/src/build-view.css +++ b/apps/web/src/build-view.css @@ -40,11 +40,32 @@ min-width: 0; flex: 1; align-items: center; - gap: calc(var(--spacing) * 2); + /* The task rows' dot column: a 12px glyph and this gap put the label over their titles. */ + gap: calc(var(--spacing) * 3); margin: 0; color: var(--color-text-primary); font-size: var(--text-sm); font-weight: var(--font-weight-medium); + /* Agent reasons can carry long URLs; they wrap rather than widen the page. */ + overflow-wrap: anywhere; +} + +/* A status glyph: tertiary, beside the label it describes. */ +.build-status-icon { + flex: none; + color: var(--color-text-tertiary); +} + +/* A status dot or glyph stays on the first line when the reason wraps. */ +.build-status-line > :is(.build-task-dot, .build-status-icon) { + align-self: flex-start; + margin-block-start: calc((1lh - var(--spacing) * 3) / 2); +} + +/* The reason or count after a status, quieter than the status itself. */ +.build-status-detail { + color: var(--color-text-tertiary); + font-weight: var(--font-weight-normal); } .build-elapsed { @@ -162,6 +183,8 @@ .build-tasks { display: grid; + /* An auto column grows to a long title's width and pushes capsules past the edge. */ + grid-template-columns: minmax(0, 1fr); margin: calc(var(--spacing) * 4) 0 0; padding: 0; list-style: none; @@ -226,9 +249,10 @@ background: var(--color-success-graphic); } +/* The same warning family as the reason's ink, so the dot and its text read as one. */ .build-task-dot[data-state="blocked"] { - border-color: var(--color-warning-graphic); - background: var(--color-warning-graphic); + border-color: var(--color-warning); + background: var(--color-warning); } .build-task-pr { @@ -285,6 +309,7 @@ padding: 0 0 calc(var(--spacing) * 3) calc(var(--spacing) * 6); color: var(--color-text-secondary); font-size: var(--text-sm); + overflow-wrap: anywhere; } .build-task-acceptance { @@ -295,11 +320,62 @@ list-style: disc; } +.build-task-summary { + margin: 0; + color: var(--color-text-primary); +} + +/* Tasks a living document's later syncs added, after the first build's. */ +.build-since { + margin-block-start: calc(var(--spacing) * 6); +} + +.build-since-heading { + margin: 0; + color: var(--color-text-tertiary); + font-size: var(--text-xs); + font-weight: var(--font-weight-medium); +} + +.build-since .build-tasks { + margin-block-start: calc(var(--spacing) * 2); +} + +/* + * A sync that needed no code change: quieter than a task, its check in the dot column and its + * time on the first line however long the summary runs. + */ +.build-no-change { + display: flex; + align-items: flex-start; + gap: calc(var(--spacing) * 3); + padding-block: calc((var(--spacing) * 10 - 1lh) / 2); + border-block-end: var(--edge-width) solid var(--color-edge); + color: var(--color-text-secondary); + font-size: var(--text-sm); + overflow-wrap: anywhere; +} + +.build-no-change > .build-status-icon { + margin-block-start: calc((1lh - var(--spacing) * 3) / 2); +} + .build-task-blocker { margin: 0; color: var(--color-warning-ink); } +.build-task-blocker a { + color: inherit; + text-decoration: underline; + text-decoration-color: color-mix(in srgb, currentColor 40%, transparent); + text-underline-offset: 2px; +} + +.build-task-blocker a:hover { + text-decoration-color: currentColor; +} + /* A living document's commits on a task's pull request, newest first. */ .build-task-commits { display: grid; diff --git a/apps/web/src/build-view.tsx b/apps/web/src/build-view.tsx index 038fd7d4e..ad3a8a025 100644 --- a/apps/web/src/build-view.tsx +++ b/apps/web/src/build-view.tsx @@ -1,24 +1,32 @@ +import { CheckIcon, ClockIcon } from "@chopin/icons"; import { useEffect, useId, useRef, useState } from "react"; import { advanceDraft, ago, + attentionHint, + blockerLabelled, + blockerText, buildPhase, buildProgress, draftInFlight, draftKey, draftRefusalCopy, elapsed, + linkParts, + liveTaskGroups, + liveTaskState, plural, pullRequestCommits, pullRequestNumber, shouldAutoDraft, startedBy, startingLabel, - SYNC_LABEL, syncHint, + syncLabel, TASK_STATE_LABEL, taskStartsOpen, + unfinishedReason, } from "./build-model"; import { cancelBuildRequest, @@ -88,6 +96,7 @@ export function BuildView( let [open, setOpen] = useState>({}); let [linked, setLinked] = useState(); let [now, setNow] = useState(() => Date.now()); + let sinceHeading = useId(); let [request, setRequestState] = useState(() => requests.get(room)); let setRequest = (next: DraftRequest | undefined) => { if (next) requests.set(room, next); @@ -410,18 +419,19 @@ export function BuildView( status = plural(tasks.length, "task", "tasks"); if (canEdit) action = primary; } else if (phase.kind === "building") { - since = snapshot?.build && elapsed(snapshot.build.createdAt, now); let who = startedBy(snapshot, userId); let { hint, queued } = startingLabel(snapshot); + // A queued build has not started, so it has no time to count. + since = !queued && snapshot?.build ? elapsed(snapshot.build.createdAt, now) : undefined; status = ( <> {queued - ?