Skip to content

Commit d8d7c7e

Browse files
committed
Preserve picked browser context while writing annotations
- Keep element descriptions available across preview reloads and navigation - Restore picked elements, drawings, terminal context, and composer drafts in chat - Update browser picking, menus, pending context UI, and related tests
1 parent 796d8b5 commit d8d7c7e

20 files changed

Lines changed: 988 additions & 198 deletions

apps/desktop/src/preview/PreviewAutomation.ts

Lines changed: 106 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,6 @@ import * as Schema from "effect/Schema";
6767

6868
/** Enough to explain a failure without letting a chatty page grow unboundedly. */
6969
const MAX_CONSOLE_ENTRIES = 200;
70-
/** Long enough to choose deliberately, short enough not to strand inspect mode. */
71-
const PICK_TIMEOUT_MS = 60_000;
7270
/** Drag-and-drop and text selection depend on held movement between press and release. */
7371
const DRAG_MOVE_STEPS = 10;
7472

@@ -1096,59 +1094,103 @@ export const make = Effect.sync(function PreviewAutomationMake() {
10961094
expression: buildPickOverlayScript(colorScheme, mode),
10971095
});
10981096

1099-
const picked = yield* Effect.tryPromise({
1100-
try: () =>
1101-
new Promise<PickResult | null>((resolve) => {
1102-
let done = false;
1103-
const finish = (value: PickResult | null) => {
1104-
if (done) return;
1105-
done = true;
1106-
contents.debugger.off("message", onMessage);
1107-
clearTimeout(timer);
1108-
resolve(value);
1109-
};
1110-
const onMessage = (
1111-
_event: unknown,
1112-
method: string,
1113-
params: Record<string, unknown>,
1114-
) => {
1115-
if (method !== "Runtime.bindingCalled" || params.name !== PICK_OVERLAY_BINDING) {
1116-
return;
1117-
}
1118-
try {
1119-
const payload = JSON.parse(String(params.payload ?? "{}")) as {
1120-
count?: number;
1121-
note?: string | null;
1122-
styleChanges?: ReadonlyArray<{ property: string; from: string; to: string }>;
1123-
cancelled?: boolean;
1124-
};
1125-
finish(
1126-
payload.cancelled === true || payload.count === undefined || payload.count < 1
1127-
? null
1128-
: {
1129-
count: payload.count,
1130-
note: payload.note ?? null,
1131-
styleChanges: payload.styleChanges ?? [],
1132-
},
1133-
);
1134-
} catch {
1135-
finish(null);
1136-
}
1137-
};
1138-
contents.debugger.on("message", onMessage);
1139-
// Reaching for another tool settles this one, rather than leaving
1140-
// it waiting on a binding that will never be called.
1141-
supersede = () => {
1142-
pendingPicks.delete(webContentsId);
1143-
finish(null);
1144-
};
1145-
// Picking is a deliberate act; if the user wanders off, stop
1146-
// waiting rather than leaving the overlay armed forever.
1147-
const timer = setTimeout(() => finish(null), PICK_TIMEOUT_MS);
1148-
}),
1149-
catch: (cause) =>
1150-
new PreviewCommandError({ webContentsId, method: "Runtime.bindingCalled", cause }),
1151-
});
1097+
// The wait is a loop rather than one promise, because the overlay now
1098+
// speaks twice: once when elements are chosen -- so they can be described
1099+
// while they still exist -- and once when the note is settled. Between
1100+
// the two the page is free to die (a dev server reloading it, most
1101+
// often), and the early description is what lets the annotation survive
1102+
// that. No timeout: writing a note takes as long as it takes, and an
1103+
// armed pick is settled by escape, by another tool, or by the page
1104+
// itself navigating away before anything was chosen.
1105+
type PickEvent =
1106+
| { readonly kind: "chosen"; readonly count: number }
1107+
| { readonly kind: "final"; readonly result: PickResult | null };
1108+
const eventQueue: PickEvent[] = [];
1109+
let closed = false;
1110+
let notifyWaiter: (() => void) | null = null;
1111+
const push = (event: PickEvent) => {
1112+
if (closed) return;
1113+
eventQueue.push(event);
1114+
notifyWaiter?.();
1115+
};
1116+
const onMessage = (_event: unknown, method: string, params: Record<string, unknown>) => {
1117+
if (method === "Page.frameNavigated") {
1118+
const frame = params.frame as { parentId?: string } | undefined;
1119+
// Main frame only. The document the overlay lived in is gone; any
1120+
// last words it had (the pagehide auto-attach below) arrive on this
1121+
// same ordered stream before this does, so a note in progress is
1122+
// already in the queue and this settles only a truly idle pick.
1123+
if (frame?.parentId === undefined) {
1124+
push({ kind: "final", result: null });
1125+
}
1126+
return;
1127+
}
1128+
if (method !== "Runtime.bindingCalled" || params.name !== PICK_OVERLAY_BINDING) {
1129+
return;
1130+
}
1131+
try {
1132+
const payload = JSON.parse(String(params.payload ?? "{}")) as {
1133+
chosen?: number;
1134+
count?: number;
1135+
note?: string | null;
1136+
styleChanges?: ReadonlyArray<{ property: string; from: string; to: string }>;
1137+
cancelled?: boolean;
1138+
};
1139+
if (typeof payload.chosen === "number") {
1140+
push({ kind: "chosen", count: payload.chosen });
1141+
return;
1142+
}
1143+
push({
1144+
kind: "final",
1145+
result:
1146+
payload.cancelled === true || payload.count === undefined || payload.count < 1
1147+
? null
1148+
: {
1149+
count: payload.count,
1150+
note: payload.note ?? null,
1151+
styleChanges: payload.styleChanges ?? [],
1152+
},
1153+
});
1154+
} catch {
1155+
push({ kind: "final", result: null });
1156+
}
1157+
};
1158+
contents.debugger.on("message", onMessage);
1159+
// Reaching for another tool settles this one, rather than leaving it
1160+
// waiting on a binding that will never be called.
1161+
supersede = () => {
1162+
pendingPicks.delete(webContentsId);
1163+
push({ kind: "final", result: null });
1164+
};
1165+
const nextEvent = () =>
1166+
new Promise<PickEvent>((resolve) => {
1167+
const take = () => {
1168+
const event = eventQueue.shift();
1169+
if (event === undefined) {
1170+
notifyWaiter = take;
1171+
return;
1172+
}
1173+
notifyWaiter = null;
1174+
resolve(event);
1175+
};
1176+
take();
1177+
});
1178+
1179+
let earlyDescribed: DesktopPreviewPickedElement[] = [];
1180+
let picked: PickResult | null = null;
1181+
for (;;) {
1182+
const event = yield* Effect.promise(() => nextEvent());
1183+
if (event.kind === "chosen") {
1184+
earlyDescribed = yield* describeStashed(contents, PICK_OVERLAY_STASH, event.count).pipe(
1185+
Effect.orElseSucceed(() => []),
1186+
);
1187+
continue;
1188+
}
1189+
picked = event.result;
1190+
break;
1191+
}
1192+
closed = true;
1193+
contents.debugger.off("message", onMessage);
11521194

11531195
// A superseded pick owns nothing on the page any more: its overlay was
11541196
// replaced, and tearing down would take its replacement with it.
@@ -1161,7 +1203,15 @@ export const make = Effect.sync(function PreviewAutomationMake() {
11611203
// the overlay's handles on the elements it chose.
11621204
const described: DesktopPreviewPickedElement[] = [];
11631205
if (current && picked !== null) {
1164-
const elements = yield* describeStashed(contents, PICK_OVERLAY_STASH, picked.count);
1206+
let elements = yield* describeStashed(contents, PICK_OVERLAY_STASH, picked.count).pipe(
1207+
Effect.orElseSucceed((): DesktopPreviewPickedElement[] => []),
1208+
);
1209+
// Fewer than were chosen means the document died with the stash --
1210+
// the note arrived from pagehide -- and the descriptions taken at
1211+
// choose time are the ones that still know the elements.
1212+
if (elements.length < picked.count && earlyDescribed.length > elements.length) {
1213+
elements = earlyDescribed;
1214+
}
11651215
for (const element of elements) {
11661216
described.push({
11671217
...element,

apps/desktop/src/preview/pickOverlayScript.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,10 @@ export function buildPickOverlayScript(
304304
// rectangle caught in region mode; the note field appears once it is filled.
305305
let chosen = [];
306306
307+
// Registered while the note field is open, removed on settle: if the
308+
// document dies mid-note, what was typed attaches rather than vanishing.
309+
let onPageHide = null;
310+
307311
// The properties worth trying on the spot. Enough to describe a visual
308312
// change precisely, not so many that the panel becomes a style editor: the
309313
// agent makes the real change, this only says what to aim for.
@@ -377,6 +381,20 @@ export function buildPickOverlayScript(
377381
input.placeholder = placeholder;
378382
toggle.hidden = !styleable;
379383
384+
// Tell the host what was chosen now, while the elements exist: the page
385+
// is free to reload out from under a half-written note (a dev server
386+
// rebuilding, most often), and a description taken at choose time is what
387+
// lets the annotation survive that.
388+
window[STASH] = chosen;
389+
window[BINDING](JSON.stringify({ chosen: chosen.length }));
390+
391+
// The page going away is not the user changing their mind: attach what
392+
// was typed. Removed in dispose, so a settled pick leaves nothing armed.
393+
onPageHide = () => {
394+
attach(input.value.trim(), [...tweaks.values()]);
395+
};
396+
window.addEventListener("pagehide", onPageHide);
397+
380398
const place = () => {
381399
const rect = anchorRect();
382400
const below = rect.bottom + 8;
@@ -732,6 +750,10 @@ export function buildPickOverlayScript(
732750
};
733751
734752
function dispose() {
753+
if (onPageHide !== null) {
754+
window.removeEventListener("pagehide", onPageHide);
755+
onPageHide = null;
756+
}
735757
cursorStyle.remove();
736758
document.removeEventListener("mousemove", onMove, true);
737759
document.removeEventListener("mousedown", onRegionDown, true);

apps/web/src/components/ChatView.logic.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,26 @@ describe("deriveComposerSendState", () => {
321321
expect(state.sendableTranscriptHighlightContexts).toHaveLength(1);
322322
expect(state.hasSendableContent).toBe(true);
323323
});
324+
325+
it("treats browser annotations as sendable without any typed text", () => {
326+
// A picked element or a drawing is a complete message: the annotation says
327+
// where, its note says what.
328+
const withPickedElement = deriveComposerSendState({
329+
prompt: "",
330+
attachmentCount: 0,
331+
terminalContexts: [],
332+
pickedElementContextCount: 1,
333+
});
334+
const withDrawing = deriveComposerSendState({
335+
prompt: "",
336+
attachmentCount: 0,
337+
terminalContexts: [],
338+
drawingContextCount: 1,
339+
});
340+
341+
expect(withPickedElement.hasSendableContent).toBe(true);
342+
expect(withDrawing.hasSendableContent).toBe(true);
343+
});
324344
});
325345

326346
describe("desktopCapturedScreenshotToFile", () => {

apps/web/src/components/ChatView.logic.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1420,6 +1420,8 @@ export function deriveComposerSendState(options: {
14201420
terminalContexts: ReadonlyArray<TerminalContextDraft>;
14211421
transcriptHighlightContexts?: ReadonlyArray<TranscriptHighlightContextDraft> | undefined;
14221422
fileSelectionContextCount?: number | undefined;
1423+
pickedElementContextCount?: number | undefined;
1424+
drawingContextCount?: number | undefined;
14231425
}): {
14241426
trimmedPrompt: string;
14251427
sendableTerminalContexts: TerminalContextDraft[];
@@ -1437,12 +1439,17 @@ export function deriveComposerSendState(options: {
14371439
sendableTerminalContexts,
14381440
sendableTranscriptHighlightContexts,
14391441
expiredTerminalContextCount,
1442+
// Every kind of attached context counts: a picked element or a drawing is
1443+
// a complete message by itself -- the annotation says where, the note says
1444+
// what -- and demanding words on top of it makes people type "see above".
14401445
hasSendableContent:
14411446
trimmedPrompt.length > 0 ||
14421447
options.attachmentCount > 0 ||
14431448
sendableTerminalContexts.length > 0 ||
14441449
sendableTranscriptHighlightContexts.length > 0 ||
1445-
(options.fileSelectionContextCount ?? 0) > 0,
1450+
(options.fileSelectionContextCount ?? 0) > 0 ||
1451+
(options.pickedElementContextCount ?? 0) > 0 ||
1452+
(options.drawingContextCount ?? 0) > 0,
14461453
};
14471454
}
14481455

apps/web/src/components/ChatView.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4295,6 +4295,8 @@ export default function ChatView(props: ChatViewProps) {
42954295
terminalContexts: composerTerminalContexts,
42964296
transcriptHighlightContexts: composerTranscriptHighlightContexts,
42974297
fileSelectionContextCount: composerFileSelectionContexts.length,
4298+
pickedElementContextCount: composerPickedElementContexts.length,
4299+
drawingContextCount: composerDrawingContexts.length,
42984300
});
42994301
if (showPlanFollowUpPrompt && activeProposedPlan) {
43004302
const followUp = resolvePlanFollowUpSubmission({
@@ -6238,6 +6240,7 @@ export default function ChatView(props: ChatViewProps) {
62386240
revertTurnCountByUserMessageId={revertTurnCountByUserMessageId}
62396241
onRevertUserMessage={onRevertUserMessage}
62406242
onContinueInNewThread={onContinueMessageInNewThread}
6243+
onRevealPickedElement={isElectron ? revealPickedElement : undefined}
62416244
onAddTranscriptHighlightContext={addTranscriptHighlightContextToDraft}
62426245
isRevertingCheckpoint={isRevertingCheckpoint}
62436246
onImageExpand={onExpandTimelineImage}
@@ -6513,7 +6516,7 @@ export default function ChatView(props: ChatViewProps) {
65136516
flexGrow={browserExpanded ? 1 : 1 - splitChatFraction}
65146517
onClose={handleCloseBrowser}
65156518
onPickElement={appendPickedElementToComposer}
6516-
onScreenshot={(shot) => composerRef.current?.addScreenshotAttachment(shot)}
6519+
onScreenshot={(shot) => composerRef.current?.addScreenshotAttachment(shot) ?? false}
65176520
onDrawing={(drawing) => composerRef.current?.addDrawingContext(drawing)}
65186521
pendingReveal={pendingElementReveal}
65196522
onRevealHandled={() => setPendingElementReveal(null)}

apps/web/src/components/EditorOpenOptions.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ export function EditorOpenRadioGroup({
132132
}}
133133
>
134134
{options.map(({ label, Icon: OptionIcon, value }) => (
135-
<MenuRadioItem key={value} value={value} closeOnClick>
135+
<MenuRadioItem key={value} value={value} variant="fill" closeOnClick>
136136
<span className="flex items-center gap-2">
137137
<OptionIcon aria-hidden="true" className="text-muted-foreground" />
138138
{label}

0 commit comments

Comments
 (0)