diff --git a/AGENTS.md b/AGENTS.md index 269795d1..5af4a7b2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -215,7 +215,7 @@ No need to explicitly document the telemetry behaviors. Nothing (Tailwind, DaisyUI, Mermaid, layout safety CSS) is auto-injected into artifacts; agents choose a design direction via the single-sourced `DESIGN_PRIORITY_RULE` (see AXI integration above). The `lavish-axi design` Mermaid snippet chooses dark or light rendering from the effective artifact page background and re-renders when a page-theme or OS appearance change alters that appearance, so do not hardcode one Mermaid theme. - For rendered Mermaid SVGs outside `.mermaid` containers, the injected SDK retains dependency-free viewBox pan (drag) and zoom (wheel) in explore mode, then freezes it in annotation mode so a click resolves cleanly to one node instead of panning. It enhances on load and `DOMContentLoaded` and re-runs through a throttled `MutationObserver` because Mermaid renders asynchronously and can re-render. Enhancement touches only the live SVG's `viewBox` and listeners, never the saved artifact, so the diagram still renders identically when opened directly. Node detection, label extraction, and target validation live in `src/mermaid-node.js` so they are unit-testable and shared with the server. -- Any helper `createArtifactSdk` calls must reach the browser through `serializeModuleHelpers` in `createSdkJs`, which turns every export of a shared module (`src/mermaid-node.js`, `src/table-cell.js`) into a same-scope `const`. A module-private function called from the SDK closure compiles fine and only `ReferenceError`s on the first click, so put new helpers in one of those wholesale-serialized modules and export them; a helper may then reference only its own arguments, browser globals, or its sibling exports - never a module-level constant, which is not serialized. Those modules must export functions and nothing else: only functions survive `toString()`, so `serializeModuleHelpers` throws on any other export rather than shipping a `Set` or `RegExp` that would arrive as an empty `{}`. `test/artifact-sdk-bundle.test.js` boots the served bundle and drives a real click, which is what catches an unreachable helper; the module-level unit tests cannot. +- Any helper `createArtifactSdk` calls must reach the browser as a same-scope `const` in `createSdkJs`. `src/artifact-sdk.js`'s own exports (everything except `createArtifactSdk` itself) are serialized automatically by iterating `Object.entries` of the module - a new exported helper needs no hand-kept entry in `src/server.js`. Helpers imported from the other shared modules (`src/mermaid-node.js`, `src/table-cell.js`) still go through `serializeModuleHelpers`, which turns every export of those modules into a same-scope `const`; a module-private function called from the SDK closure compiles fine and only `ReferenceError`s on the first click, so put new cross-module helpers in one of those wholesale-serialized modules and export them. Either way, a helper may reference only its own arguments, browser globals, or its sibling exports - never a module-level constant, which is not serialized. `serializeModuleHelpers` throws if `src/mermaid-node.js`/`src/table-cell.js` export anything but a function, since only functions survive `toString()`; the automatic `artifact-sdk.js` path additionally accepts non-function exports (JSON-stringified) for constants like `MODE_TOGGLE_HOTKEY_KEY`. `test/artifact-sdk-bundle.test.js` boots the served bundle and drives a real click, which is what catches an unreachable helper; the module-level unit tests cannot. - Table-cell annotations attach `src/table-cell.js`'s semantic row/column names as `target` only. The clicked element's own `selector`, `tag`, and `text` keep describing that element, because the on-screen highlight outlines exactly what was clicked. Both coordinates stay silent rather than name a row or column they cannot prove, because a confidently wrong name reads as authoritative and is worse than none: a rowspan is clipped to its own row group, so only one starting in an earlier row of that group (including `rowspan="0"`, which runs to the end of it) makes a row's DOM order stop being its rendered order, and that suppresses the row's positional heading - only a declared `scope="row"` heading survives it - while the column label needs the header row unshifted the same way, plus a row whose colspans sum to the header's and a cell that does not straddle a grouped header. A grouped header's `` therefore costs the leaf header row its names and leaves every `` row nameable. Spans come from the browser-parsed `rowSpan`/`colSpan` whenever present, because HTML's integer rules stop at the first non-digit and render `rowspan="2x"` as a real two-row span that `Number` reads as `NaN`. Resolving a cell walks its whole table, so `context()` computes the target only under `{ table: true }`, which the annotation card passes and `snapshot()` - which calls `context()` for every element in the document - deliberately does not. diff --git a/README.md b/README.md index 6d70dc0d..e682d540 100644 --- a/README.md +++ b/README.md @@ -197,6 +197,9 @@ pnpm link On wider screens, queued annotation preview pills and chat history share a scrollable Conversation panel above a sticky composer, so long feedback queues do not push the text box or send controls off screen. The browser chrome keeps editing actions in the overflow menu (copy path, reload artifact, copy DOM snapshot, export standalone HTML, publish link, end session), while the composer exposes **Send & End** beside **Send to Agent** to submit queued prompts and user-ended attribution together. - **Reviewing on a phone** - Below 860px wide, the artifact takes the whole screen above a **Conversation** dock, and the conversation opens as a bottom sheet over it: tap the dock, swipe it up, or press the chevron to raise it; tap the dimmed artifact, swipe the sheet down, press the chevron, or press Escape to lower it. The dock reports what matters while the sheet is down - how many prompts are queued, a reply that arrived while you were reading, or whether the agent is listening - and the sheet stays open across a reload of the review page. The sheet sizes itself to the visible viewport and respects safe-area insets; if the keyboard or attachments leave little room, conversation content yields or scrolls while the send actions remain pinned above the bottom edge. In landscape the sheet covers the top bar as well. Wider screens keep the side-by-side layout. +- **Annotation indicators** - Every annotated element carries a small dot in the artifact, and sent annotations stay listed in the Conversation panel for the rest of the session, so it is always visible what has already been reviewed. + Navigation runs both ways: clicking a dot scrolls to and highlights its row in the panel, and clicking a panel row (a queued pill or a sent annotation) scrolls the element into view and flashes a marker around it. + The dots are only clickable in annotate mode, so they never intercept clicks meant for the artifact while exploring. - **Keyboard shortcuts** - In the chrome composer, Enter sends queued prompts and Shift+Enter inserts a newline. In the annotation card, Enter queues the annotation, Shift+Enter inserts a newline, and Ctrl+Enter (Cmd+Enter on macOS) queues it and sends all queued prompts immediately. Cmd+I or Ctrl+I toggles between annotate and explore mode from either the browser chrome or the artifact iframe, including while focus is in a textarea or control. diff --git a/src/artifact-sdk.js b/src/artifact-sdk.js index 2491298a..928a91e9 100644 --- a/src/artifact-sdk.js +++ b/src/artifact-sdk.js @@ -441,6 +441,24 @@ export function deriveAttachmentNoticeState(state = {}) { return ""; } +// Multiple annotations can target the same element; the on-page badge collapses them to one dot +// keyed by resolved element identity, keeping the earliest (first-listed) id. `resolve` is +// injected so this stays DOM-shape-agnostic and unit-testable with plain objects. +export function dedupeAnnotationTargets(targets, resolve) { + const seen = new Set(); + const result = []; + for (const entry of Array.isArray(targets) ? targets : []) { + const id = String(entry?.id || ""); + const selector = String(entry?.selector || ""); + if (!id || !selector) continue; + const el = resolve(selector); + if (!el || seen.has(el)) continue; + seen.add(el); + result.push({ id, el, selector }); + } + return result; +} + /** * @param {*} deriveQueueKey * @param {*} [isNativeInteractive] @@ -468,6 +486,122 @@ export function createArtifactSdk( let selected = null; let ignoreNextClick = false; let shadow = null; + let annotationTargets = []; + /** @type {Array<{ id: string, el: Element, node: HTMLDivElement, selector: string }>} */ + let annotationBadges = []; + let annotationBadgeFrame = 0; + let annotationBadgeSettleFrames = 0; + let annotationBadgeObserver = null; + let annotationBadgeMutationObserver = null; + // A scroll or resize keeps arriving while a smooth scroll or a CSS transition is still running, + // so each trigger re-arms a short settle window rather than a single frame. + const ANNOTATION_BADGE_SETTLE_FRAMES = 20; + + function setAnnotationTargets(targets) { + annotationTargets = Array.isArray(targets) ? targets : []; + renderAnnotationBadges(); + } + + function renderAnnotationBadges() { + const root = ensureShadow(); + for (const badge of annotationBadges) badge.node.remove(); + annotationBadges = dedupeAnnotationTargets(annotationTargets, safeQuerySelector).map(({ id, el, selector }) => { + const node = document.createElement("div"); + node.className = "lavish-annotation-badge"; + node.addEventListener("click", (event) => { + event.stopPropagation(); + postArtifactMessage("lavish:openAnnotation", { id }); + }); + root.appendChild(node); + return { id, el, node, selector }; + }); + observeAnnotationBadgeTargets(); + observeAnnotationBadgeMutations(); + syncAnnotationBadgeInteractivity(); + positionAnnotationBadges(); + } + + // A tracked element can be replaced outright (removed and re-inserted as a new node matching + // the same selector) with no resize at all, which the ResizeObserver above never sees. Watching + // for childList changes catches that swap too, so a badge re-resolves and repositions promptly + // instead of sitting on a detached element until the next scroll or resize happens to fire. + function observeAnnotationBadgeMutations() { + if (typeof MutationObserver === "undefined" || !document.documentElement) return; + if (!annotationBadgeMutationObserver) { + annotationBadgeMutationObserver = new MutationObserver(() => scheduleAnnotationBadgeSync()); + } + annotationBadgeMutationObserver.disconnect(); + if (!annotationBadges.length) return; + annotationBadgeMutationObserver.observe(document.documentElement, { childList: true, subtree: true }); + } + + // Badges only hit-test while annotating: in explore mode a dot pinned to an element's top-right + // corner would otherwise swallow clicks meant for whatever the artifact draws there. + function syncAnnotationBadgeInteractivity() { + for (const badge of annotationBadges) badge.node.classList.toggle("is-idle", !annotationMode); + } + + // Element-level geometry changes (an accordion opening, an image loading, a font swap) move a + // badge's anchor without firing scroll or resize, so the targets are observed directly. + function observeAnnotationBadgeTargets() { + if (typeof ResizeObserver !== "function") return; + if (!annotationBadgeObserver) { + annotationBadgeObserver = new ResizeObserver(() => scheduleAnnotationBadgeSync()); + } + annotationBadgeObserver.disconnect(); + if (!annotationBadges.length) return; + annotationBadgeObserver.observe(document.documentElement); + for (const badge of annotationBadges) annotationBadgeObserver.observe(badge.el); + } + + // A dynamic artifact can replace an annotated element with a new node matching the same + // selector (e.g. a re-rendered list row): the old element goes detached but its badge would + // otherwise keep tracking that dead node's last-known geometry forever. Re-resolve through the + // stored selector whenever the tracked element is no longer in the document, so the badge + // follows the replacement; if nothing resolves, hide the badge rather than draw it at a stale + // position. + function positionAnnotationBadges() { + for (const badge of annotationBadges) { + if (!badge.el.isConnected) { + const replacement = badge.selector ? safeQuerySelector(badge.selector) : null; + if (replacement) { + if (annotationBadgeObserver) annotationBadgeObserver.observe(replacement); + badge.el = replacement; + } else { + badge.node.style.display = "none"; + continue; + } + } + badge.node.style.display = ""; + const rect = badge.el.getBoundingClientRect(); + badge.node.style.left = rect.right - 6 + "px"; + badge.node.style.top = rect.top - 6 + "px"; + } + } + + // Badges are persistent (unlike the one-shot reveal-marker pulse), so their position has to + // track scroll, resize, and layout changes. Repositioning is driven by those events and bounded + // to a short settle window instead of an always-on frame loop: a permanent loop reads layout + // every frame for the whole review, which never lets the artifact's renderer idle. + function scheduleAnnotationBadgeSync(frames = ANNOTATION_BADGE_SETTLE_FRAMES) { + if (!annotationBadges.length) return; + annotationBadgeSettleFrames = Math.max(annotationBadgeSettleFrames, frames); + if (!annotationBadgeFrame) annotationBadgeFrame = window.requestAnimationFrame(annotationBadgeLoop); + } + + function annotationBadgeLoop() { + annotationBadgeFrame = 0; + if (!annotationBadges.length) { + annotationBadgeSettleFrames = 0; + return; + } + positionAnnotationBadges(); + annotationBadgeSettleFrames -= 1; + if (annotationBadgeSettleFrames > 0) annotationBadgeFrame = window.requestAnimationFrame(annotationBadgeLoop); + } + + window.addEventListener("scroll", () => scheduleAnnotationBadgeSync(), true); + window.addEventListener("resize", () => scheduleAnnotationBadgeSync()); let counter = 0; const ids = new WeakMap(); @@ -1197,13 +1331,28 @@ export function createArtifactSdk( // Freeze Mermaid pan/zoom while annotating so nodes sit at stable screen // positions and a click resolves cleanly to one node instead of panning. setMermaidFrozen(annotationMode); + syncAnnotationBadgeInteractivity(); + } + + // crypto.randomUUID exists only in a secure context, and Lavish is documented to bind to a LAN + // address (LAVISH_AXI_HOST), where the artifact is plain http and it is undefined. getRandomValues + // is available everywhere, so identifying a prompt never depends on the origin being secure. + function promptId() { + const webCrypto = window.crypto; + if (typeof webCrypto?.randomUUID === "function") return webCrypto.randomUUID(); + if (typeof webCrypto?.getRandomValues === "function") { + const bytes = webCrypto.getRandomValues(new Uint8Array(16)); + return [...bytes].map((byte) => byte.toString(16).padStart(2, "0")).join(""); + } + return Date.now().toString(16) + "-" + Math.random().toString(16).slice(2); } function queuePrompt(prompt, options = {}) { const originElement = options.element || document.activeElement || document.body; - /** @type {{ uid: string, prompt: string, selector: string, tag: string, text: string, target?: unknown, attachments?: Array<{ id: string, name?: string }>, _lavishQueueKey?: string }} */ + /** @type {{ id: string, uid: string, prompt: string, selector: string, tag: string, text: string, target?: unknown, attachments?: Array<{ id: string, name?: string }>, _lavishQueueKey?: string }} */ const item = { ...context(originElement), + id: promptId(), prompt: String(prompt || ""), }; const queueKey = typeof deriveQueueKey === "function" ? deriveQueueKey(originElement, options) : ""; @@ -2207,7 +2356,7 @@ export function createArtifactSdk( shadow = host.attachShadow({ mode: "open" }); const style = document.createElement("style"); - style.textContent = `:host{all:initial;position:fixed;z-index:2147483647;left:0;top:0;color-scheme:dark;--ink-900:#0f1115;--ink-800:#11141a;--ink-700:#171a21;--ink-600:#1c212b;--steel-700:#2a2f3a;--steel-600:#303745;--steel-500:#3c4557;--steel-400:#8c96aa;--steel-300:#aeb6c6;--steel-200:#b9c0cf;--steel-100:#d8deea;--cream-50:#fffbf3;--cream-100:#f7f3ea;--cream-200:#e8e1cf;--brass-500:#f4c95d;--brass-400:#ffd877;--brass-ink:#17130a;--bg:var(--ink-900);--bg-panel:var(--ink-800);--bg-elevated:var(--ink-600);--fg:var(--cream-100);--fg-faint:var(--steel-300);--border:var(--steel-600);--accent:#f4c95d;--accent-hover:#ffd877;--font-sans:Geist,ui-sans-serif,system-ui,-apple-system,"Segoe UI",sans-serif;--font-mono:"Geist Mono",ui-monospace,SFMono-Regular,Menlo,Consolas,monospace;--radius-md:10px;--radius-xl:14px;--shadow-floating:0 20px 70px rgba(0,0,0,.35);font-family:var(--font-sans)}*{box-sizing:border-box}:focus-visible{outline:2px solid var(--accent);outline-offset:2px}.lavish-text-highlight{position:fixed;pointer-events:none;background:rgba(244,201,93,.28);border-radius:2px;box-shadow:0 0 0 1px rgba(244,201,93,.45)}.lavish-annotation-card{position:fixed;width:min(320px,calc(100vw - 24px));padding:12px;border-radius:var(--radius-xl);background:var(--bg-panel);color:var(--fg);border:1px solid var(--accent);box-shadow:var(--shadow-floating);font:14px/1.4 var(--font-sans)}.lavish-heading{font-weight:700;margin-bottom:6px}.lavish-annotation-card textarea{width:100%;min-height:86px;resize:vertical;border-radius:var(--radius-md);border:1px solid var(--border);background:var(--bg);color:var(--fg);padding:9px;font:inherit;font-family:var(--font-sans)}.lavish-annotation-card textarea::placeholder{color:var(--fg-faint)}.lavish-annotation-card .lavish-hint{margin-top:6px;font-size:11px;color:var(--fg-faint)}.lavish-annotation-card .lavish-hint-alert{color:#ff9d7a;font-weight:700}.lavish-annotation-card .lavish-row{display:flex;gap:8px;justify-content:flex-end;margin-top:8px}.lavish-annotation-card button{border:0;border-radius:var(--radius-md);padding:8px 10px;font-family:var(--font-sans);font-size:13px;font-weight:700;cursor:pointer}.lavish-annotation-card button:active{opacity:.85}.lavish-annotation-card .lavish-send{background:var(--accent);color:var(--brass-ink)}.lavish-annotation-card .lavish-send:hover{background:var(--accent-hover)}.lavish-annotation-card .lavish-cancel{background:var(--steel-700);color:var(--fg)}.lavish-annotation-card.is-dropping{outline:2px dashed var(--accent);outline-offset:3px}.lavish-attachments{display:flex;flex-direction:column;gap:6px;margin-top:8px;max-height:176px;overflow-y:auto}.lavish-attachment-chip{display:flex;align-items:center;gap:8px;padding:6px;border-radius:var(--radius-md);background:var(--bg);border:1px solid var(--border)}.lavish-attachment-chip.is-error{border-color:#e0623d}.lavish-attachment-thumb{width:32px;height:32px;border-radius:6px;object-fit:cover;background:var(--ink-700);flex:0 0 auto}.lavish-attachment-thumb-empty{display:inline-block}.lavish-attachment-body{display:flex;flex-direction:column;gap:1px;min-width:0;flex:1 1 auto}.lavish-attachment-name{font-size:12px;font-weight:600;white-space:nowrap;overflow:hidden;text-overflow:ellipsis}.lavish-attachment-status{font-size:11px;color:var(--fg-faint)}.lavish-attachment-status-error{color:#ff9d7a}.lavish-attachment-retry{flex:0 0 auto;padding:4px 8px;font-size:11px;font-weight:700;border-radius:8px;background:var(--steel-700);color:var(--fg);cursor:pointer;border:0}.lavish-attachment-remove{flex:0 0 auto;display:flex;align-items:center;justify-content:center;width:22px;height:22px;padding:0!important;border-radius:50%;background:transparent;color:rgba(255,255,255,.85);cursor:pointer;border:0}.lavish-attachment-remove:hover{background:rgba(255,255,255,.14);color:#fff}.lavish-attach-row{margin-top:8px}.lavish-attach{display:inline-flex;align-items:center;gap:6px;padding:6px 9px!important;background:var(--steel-700)!important;color:var(--fg)!important;font-size:12px!important}.lavish-attach:hover{background:var(--steel-600)!important}.lavish-reveal-marker{position:fixed;pointer-events:none;border:2px solid var(--accent);border-radius:4px;box-shadow:0 0 0 4px rgba(244,201,93,.22);animation:lavish-reveal-pulse 2.4s var(--ease,ease-out) forwards}@keyframes lavish-reveal-pulse{0%{opacity:0}12%{opacity:1}70%{opacity:1}100%{opacity:0}}`; + style.textContent = `:host{all:initial;position:fixed;z-index:2147483647;left:0;top:0;color-scheme:dark;--ink-900:#0f1115;--ink-800:#11141a;--ink-700:#171a21;--ink-600:#1c212b;--steel-700:#2a2f3a;--steel-600:#303745;--steel-500:#3c4557;--steel-400:#8c96aa;--steel-300:#aeb6c6;--steel-200:#b9c0cf;--steel-100:#d8deea;--cream-50:#fffbf3;--cream-100:#f7f3ea;--cream-200:#e8e1cf;--brass-500:#f4c95d;--brass-400:#ffd877;--brass-ink:#17130a;--bg:var(--ink-900);--bg-panel:var(--ink-800);--bg-elevated:var(--ink-600);--fg:var(--cream-100);--fg-faint:var(--steel-300);--border:var(--steel-600);--accent:#f4c95d;--accent-hover:#ffd877;--font-sans:Geist,ui-sans-serif,system-ui,-apple-system,"Segoe UI",sans-serif;--font-mono:"Geist Mono",ui-monospace,SFMono-Regular,Menlo,Consolas,monospace;--radius-md:10px;--radius-xl:14px;--shadow-floating:0 20px 70px rgba(0,0,0,.35);font-family:var(--font-sans)}*{box-sizing:border-box}:focus-visible{outline:2px solid var(--accent);outline-offset:2px}.lavish-text-highlight{position:fixed;pointer-events:none;background:rgba(244,201,93,.28);border-radius:2px;box-shadow:0 0 0 1px rgba(244,201,93,.45)}.lavish-annotation-card{position:fixed;width:min(320px,calc(100vw - 24px));padding:12px;border-radius:var(--radius-xl);background:var(--bg-panel);color:var(--fg);border:1px solid var(--accent);box-shadow:var(--shadow-floating);font:14px/1.4 var(--font-sans)}.lavish-heading{font-weight:700;margin-bottom:6px}.lavish-annotation-card textarea{width:100%;min-height:86px;resize:vertical;border-radius:var(--radius-md);border:1px solid var(--border);background:var(--bg);color:var(--fg);padding:9px;font:inherit;font-family:var(--font-sans)}.lavish-annotation-card textarea::placeholder{color:var(--fg-faint)}.lavish-annotation-card .lavish-hint{margin-top:6px;font-size:11px;color:var(--fg-faint)}.lavish-annotation-card .lavish-hint-alert{color:#ff9d7a;font-weight:700}.lavish-annotation-card .lavish-row{display:flex;gap:8px;justify-content:flex-end;margin-top:8px}.lavish-annotation-card button{border:0;border-radius:var(--radius-md);padding:8px 10px;font-family:var(--font-sans);font-size:13px;font-weight:700;cursor:pointer}.lavish-annotation-card button:active{opacity:.85}.lavish-annotation-card .lavish-send{background:var(--accent);color:var(--brass-ink)}.lavish-annotation-card .lavish-send:hover{background:var(--accent-hover)}.lavish-annotation-card .lavish-cancel{background:var(--steel-700);color:var(--fg)}.lavish-annotation-card.is-dropping{outline:2px dashed var(--accent);outline-offset:3px}.lavish-attachments{display:flex;flex-direction:column;gap:6px;margin-top:8px;max-height:176px;overflow-y:auto}.lavish-attachment-chip{display:flex;align-items:center;gap:8px;padding:6px;border-radius:var(--radius-md);background:var(--bg);border:1px solid var(--border)}.lavish-attachment-chip.is-error{border-color:#e0623d}.lavish-attachment-thumb{width:32px;height:32px;border-radius:6px;object-fit:cover;background:var(--ink-700);flex:0 0 auto}.lavish-attachment-thumb-empty{display:inline-block}.lavish-attachment-body{display:flex;flex-direction:column;gap:1px;min-width:0;flex:1 1 auto}.lavish-attachment-name{font-size:12px;font-weight:600;white-space:nowrap;overflow:hidden;text-overflow:ellipsis}.lavish-attachment-status{font-size:11px;color:var(--fg-faint)}.lavish-attachment-status-error{color:#ff9d7a}.lavish-attachment-retry{flex:0 0 auto;padding:4px 8px;font-size:11px;font-weight:700;border-radius:8px;background:var(--steel-700);color:var(--fg);cursor:pointer;border:0}.lavish-attachment-remove{flex:0 0 auto;display:flex;align-items:center;justify-content:center;width:22px;height:22px;padding:0!important;border-radius:50%;background:transparent;color:rgba(255,255,255,.85);cursor:pointer;border:0}.lavish-attachment-remove:hover{background:rgba(255,255,255,.14);color:#fff}.lavish-attach-row{margin-top:8px}.lavish-attach{display:inline-flex;align-items:center;gap:6px;padding:6px 9px!important;background:var(--steel-700)!important;color:var(--fg)!important;font-size:12px!important}.lavish-attach:hover{background:var(--steel-600)!important}.lavish-reveal-marker{position:fixed;pointer-events:none;border:2px solid var(--accent);border-radius:4px;box-shadow:0 0 0 4px rgba(244,201,93,.22);animation:lavish-reveal-pulse 2.4s var(--ease,ease-out) forwards}@keyframes lavish-reveal-pulse{0%{opacity:0}12%{opacity:1}70%{opacity:1}100%{opacity:0}}.lavish-annotation-badge{position:fixed;width:10px;height:10px;border-radius:50%;background:var(--accent);box-shadow:0 0 0 2px var(--ink-900);cursor:pointer;pointer-events:auto;z-index:2147483647}.lavish-annotation-badge.is-idle{pointer-events:none;opacity:.65}`; shadow.appendChild(style); return shadow; } @@ -2450,6 +2599,7 @@ export function createArtifactSdk( if (!isTrustedAttachmentResult(event, { parentWindow: parent, nonce: ATTACHMENT_NONCE })) return; activeAttachments?.handleResult(msg.localId, msg.ok, msg.id, msg.error); } + if (msg.type === "lavish:setAnnotationTargets") setAnnotationTargets(msg.targets); if (msg.type === "lavish:requestSnapshot") { postArtifactMessage("lavish:snapshot", { snapshot: snapshot() }); } @@ -2468,14 +2618,25 @@ export function createArtifactSdk( target.scrollIntoView({ block: "center", inline: "center", behavior: "smooth" }); const root = ensureShadow(); for (const el of [...root.querySelectorAll(".lavish-reveal-marker")]) el.remove(); - const rect = target.getBoundingClientRect(); const marker = document.createElement("div"); marker.className = "lavish-reveal-marker"; - marker.style.left = rect.left + "px"; - marker.style.top = rect.top + "px"; - marker.style.width = Math.max(rect.width, 4) + "px"; - marker.style.height = Math.max(rect.height, 4) + "px"; root.appendChild(marker); + + // The marker is position:fixed but scrollIntoView is asynchronous under `behavior: "smooth"`, + // so a rect read now is pre-scroll and would strand the box over whatever the element used to + // be next to - the "ghost box" that only looked right once the element was already in view. + // Re-read the rect every frame for the pulse's lifetime instead, which also keeps the marker + // on target through any layout shift the scroll triggers. + const track = () => { + if (!marker.isConnected) return; + const rect = target.getBoundingClientRect(); + marker.style.left = rect.left + "px"; + marker.style.top = rect.top + "px"; + marker.style.width = Math.max(rect.width, 4) + "px"; + marker.style.height = Math.max(rect.height, 4) + "px"; + window.requestAnimationFrame(track); + }; + track(); window.setTimeout(() => marker.remove(), 2400); } diff --git a/src/chrome-client.js b/src/chrome-client.js index 6bbbf356..142c92b2 100644 --- a/src/chrome-client.js +++ b/src/chrome-client.js @@ -93,6 +93,9 @@ function describeAttachmentRejection(rejected, caps) { return "Not sent — " + detail + ". Remove or fix the image, then send again."; } +/** @type {Array<{ id: string, selector: string, tag: string, text: string, prompt: string, at: string, target?: unknown }>} */ +let sentAnnotations = Array.isArray(sessionData.initialAnnotations) ? sessionData.initialAnnotations.slice() : []; + function isModeToggleHotkeyEvent(event) { if (event.shiftKey || event.altKey) return false; return Boolean(event.metaKey || event.ctrlKey) && String(event.key || "").toLowerCase() === MODE_TOGGLE_HOTKEY_KEY; @@ -101,6 +104,7 @@ function isModeToggleHotkeyEvent(event) { const frame = /** @type {HTMLIFrameElement} */ (document.getElementById("artifact")); const panelScroll = /** @type {HTMLDivElement} */ (document.getElementById("panelScroll")); const annotationPills = /** @type {HTMLDivElement} */ (document.getElementById("annotationPills")); +const annotationsSent = /** @type {HTMLDivElement} */ (document.getElementById("annotationsSent")); const chatLog = /** @type {HTMLDivElement} */ (document.getElementById("chatLog")); const chatComposer = /** @type {HTMLDivElement} */ (document.getElementById("chatComposer")); const chatInput = /** @type {HTMLTextAreaElement} */ (document.getElementById("chatInput")); @@ -366,13 +370,77 @@ function promptTargetLabel(prompt) { return String(prompt?.selector || ""); } +// The row itself is the affordance - a separate pin control was redundant with it. Clicks on +// nested controls (the pill's remove button) still win, because they stop propagation first. +function bindRevealTargets(container, selectorFor) { + for (const child of container.children) { + const row = /** @type {HTMLElement} */ (child); + const selector = selectorFor(row); + if (!selector) continue; + row.classList.add("reveal-target"); + row.addEventListener("click", () => postToFrame({ type: "lavish:revealElement", selector })); + } +} + +function annotationTargetsList() { + const list = []; + for (const prompt of queued) { + if (prompt.id && prompt.selector) list.push({ id: prompt.id, selector: prompt.selector, target: prompt.target }); + } + for (const item of sentAnnotations) { + if (item.id && item.selector) list.push({ id: item.id, selector: item.selector, target: item.target }); + } + return list; +} + +function postAnnotationTargets() { + postToFrame({ type: "lavish:setAnnotationTargets", targets: annotationTargetsList() }); +} + +function renderAnnotations() { + annotationsSent.replaceChildren(); + for (const item of sentAnnotations) { + const entry = document.createElement("div"); + entry.className = "annotation-entry"; + entry.dataset.annotationId = item.id || ""; + entry.dataset.selector = item.selector || ""; + const text = document.createElement("span"); + text.className = "annotation-entry-text"; + text.textContent = item.prompt; + entry.appendChild(text); + annotationsSent.appendChild(entry); + } + bindRevealTargets(annotationsSent, (row) => row.dataset.selector || ""); + postAnnotationTargets(); +} + +// A badge can stand for either a queued prompt or an already-sent one - both are in the badge +// list - so the lookup spans both rows, otherwise clicking a queued annotation's badge is a +// silent no-op until the queue is sent. +function openAnnotationEntry(id) { + const target = String(id || ""); + if (!target) return; + const rows = [...annotationPills.children, ...annotationsSent.children]; + const entry = /** @type {HTMLElement | undefined} */ ( + rows.find((child) => /** @type {HTMLElement} */ (child).dataset?.annotationId === target) + ); + if (!entry) return; + scrollElementIntoView(entry); + entry.classList.add("annotation-highlight"); + setTimeout(() => entry.classList.remove("annotation-highlight"), 2400); +} + function render() { annotationPills.innerHTML = queued .map((prompt, index) => { const targetLabel = promptTargetLabel(prompt); const showLocator = targetLabel && prompt.selector && targetLabel !== prompt.selector; return ( - '
' + + '
' + escapeHtml( prompt.prompt || (attachmentCount(prompt) ? (prompt.tag === "message" ? "Image message" : "Image annotation") : ""), @@ -403,9 +471,11 @@ function render() { const closeButton = /** @type {HTMLButtonElement} */ (button); closeButton.addEventListener("click", (event) => removeQueuedPrompt(Number(closeButton.dataset.index), event)); } + bindRevealTargets(annotationPills, (row) => row.dataset.selector || ""); updateSendState(); scrollPanelToBottom(); renderSheetSummary(); + postAnnotationTargets(); } function updateSendState() { @@ -1240,12 +1310,25 @@ async function submitQueuedOnce() { } throw new Error("failed to submit queued prompts"); } + const sentAt = new Date().toISOString(); for (const prompt of prompts) { const index = queued.indexOf(prompt); if (index !== -1) queued.splice(index, 1); + if (prompt.tag !== "message" && prompt.selector) { + sentAnnotations.push({ + id: prompt.id || "", + selector: prompt.selector, + target: prompt.target, + tag: prompt.tag, + text: prompt.text, + prompt: prompt.prompt, + at: sentAt, + }); + } } persistQueuedPrompts(); render(); + renderAnnotations(); if (shouldEndSession) { endAfterSubmit = false; markSessionEnded(); @@ -2804,6 +2887,7 @@ window.addEventListener("message", (event) => { if (msg.type === "lavish:sendQueuedPrompts") sendQueued(); if (msg.type === "lavish:endSession") endSession(); if (msg.type === "lavish:toggleAnnotationMode") toggleAnnotationMode(); + if (msg.type === "lavish:openAnnotation") openAnnotationEntry(msg.id); }); // The sandboxed artifact iframe can't reach the loopback server (opaque origin), @@ -3103,6 +3187,7 @@ document.addEventListener( frame.addEventListener("load", () => { if (artifactSpokeToken !== artifactLoadToken) armArtifactAvailabilityProbe(artifactLoadToken); postToFrame({ type: "lavish:setAnnotationMode", enabled: annotation && !ended }); + postAnnotationTargets(); // Replay the pre-reload scroll position so hot reloads don't jump the artifact to the top. postToFrame({ type: "lavish:restoreScroll", x: lastScroll.x, y: lastScroll.y }); if (lastReviewState) postToFrame({ type: "lavish:restoreReviewState", state: lastReviewState }); @@ -3141,6 +3226,7 @@ render(); setChromeOutdated(false); setWarningsDrawerOpen(false); renderWarnings(); +renderAnnotations(); initialChat.forEach((item) => addChat(item.role, item.text)); retiredDrafts.forEach((text) => renderRetiredDraft(text)); setAgentPresence("waiting"); diff --git a/src/chrome.css b/src/chrome.css index f9b73ca1..6588fa67 100644 --- a/src/chrome.css +++ b/src/chrome.css @@ -1178,6 +1178,41 @@ body.lavish { .pill-wrap:focus-within .pill-tooltip { display: block; } +.reveal-target { + cursor: pointer; +} +.annotations-sent { + display: flex; + flex-direction: column; + gap: 6px; + min-width: 0; + padding: 0 16px 12px; + flex: 0 0 auto; +} +.annotations-sent:empty { + display: none; +} +.annotation-entry { + display: flex; + align-items: center; + gap: 6px; + max-width: 100%; + border: 1px solid var(--border-subtle); + border-radius: var(--radius-lg); + background: transparent; + color: var(--fg-muted); + padding: 6px 8px; + font-size: 12px; +} +.annotation-entry-text { + overflow: hidden; + white-space: nowrap; + text-overflow: ellipsis; +} +.annotation-highlight { + outline: 2px solid var(--accent); + outline-offset: 2px; +} .composer textarea { width: 100%; max-width: 100%; diff --git a/src/server.js b/src/server.js index 3df8eef1..290c4308 100644 --- a/src/server.js +++ b/src/server.js @@ -10,25 +10,8 @@ import { fileURLToPath } from "node:url"; import chokidar from "chokidar"; import express from "express"; -import { - classifySevereTextOverflow, - classifyMaterialRectEscape, - createArtifactSdk, - deriveAttachmentNoticeState, - deriveLavishQueueKey, - findStableLayoutFindings, - isMaterialPageOverflow, - isModeToggleHotkeyEvent, - isNativeInteractiveControl, - isNearTotalOcclusion, - isTrustedAttachmentResult, - attachmentSizeError, - acceptedImageTypes, - classifyAttachmentBatch, - partitionDroppedFiles, - planClipboardPaste, - MODE_TOGGLE_HOTKEY_KEY, -} from "./artifact-sdk.js"; +import { createArtifactSdk, MODE_TOGGLE_HOTKEY_KEY } from "./artifact-sdk.js"; +import * as artifactSdk from "./artifact-sdk.js"; import { activeLayoutWarningCount, resolveDiagnosticViewportClasses, @@ -2094,6 +2077,7 @@ export function createChromeHtml( initialEnded: session.status === "ended", initialEndedBy: session.ended_by || null, initialChat: session.chat || [], + initialAnnotations: session.annotations || [], // Bootstrapping the inbox from the server is what makes it survive a browser refresh or a // reconnect: the chrome never owns warning state, it only renders it. initialLayoutWarnings: serializeLayoutWarnings(session.layout_warnings), @@ -2123,7 +2107,7 @@ ${faviconTag}
LavishEditor
-
+
Checking layout.
One moment.

Lavish is waiting for fonts and final geometry before revealing this artifact.

@@ -2186,6 +2170,16 @@ export function createSdkJs( ) { const mermaidHelperSource = serializeModuleHelpers(mermaidNode); const tableHelperSource = serializeModuleHelpers(tableCellHelpers); + // Same treatment for artifact-sdk.js's own helpers. A hand-kept list here is a silent + // ReferenceError waiting to happen: a helper called from inside createArtifactSdk but left out + // of the list passes build, lint, and typecheck and only fails in the browser, where it kills + // the feature with no error anywhere else. + const sdkHelperDecls = Object.entries(artifactSdk) + .filter(([name]) => name !== "createArtifactSdk") + .map(([name, value]) => + typeof value === "function" ? `const ${name}=${value.toString()};` : `const ${name}=${JSON.stringify(value)};`, + ) + .join("\n"); const revisionNumber = Number(artifactRevision); const revision = Number.isFinite(revisionNumber) && revisionNumber >= 0 ? Math.trunc(revisionNumber) : 0; const loadToken = String(artifactLoadToken || "").slice(0, 200); @@ -2202,22 +2196,8 @@ export function createSdkJs( const key=${JSON.stringify(key)}; const artifactRevision=${revision}; const artifactLoadToken=${JSON.stringify(loadToken)}; -const deriveQueueKey=${deriveLavishQueueKey.toString()}; -const isNativeInteractiveControl=${isNativeInteractiveControl.toString()}; -const MODE_TOGGLE_HOTKEY_KEY=${JSON.stringify(MODE_TOGGLE_HOTKEY_KEY)}; -const isModeToggleHotkeyEvent=${isModeToggleHotkeyEvent.toString()}; -const classifySevereTextOverflow=${classifySevereTextOverflow.toString()}; -const classifyMaterialRectEscape=${classifyMaterialRectEscape.toString()}; -const isMaterialPageOverflow=${isMaterialPageOverflow.toString()}; -const findStableLayoutFindings=${findStableLayoutFindings.toString()}; -const isNearTotalOcclusion=${isNearTotalOcclusion.toString()}; -const attachmentSizeError=${attachmentSizeError.toString()}; -const classifyAttachmentBatch=${classifyAttachmentBatch.toString()}; -const partitionDroppedFiles=${partitionDroppedFiles.toString()}; -const planClipboardPaste=${planClipboardPaste.toString()}; -const acceptedImageTypes=${acceptedImageTypes.toString()}; -const isTrustedAttachmentResult=${isTrustedAttachmentResult.toString()}; -const deriveAttachmentNoticeState=${deriveAttachmentNoticeState.toString()}; +${sdkHelperDecls} +const deriveQueueKey=deriveLavishQueueKey; ${mermaidHelperSource.declarations} const mermaidHelpers={ ${mermaidHelperSource.names.join(", ")} }; ${tableHelperSource.declarations} diff --git a/src/session-store.js b/src/session-store.js index 57110528..e16eebf8 100644 --- a/src/session-store.js +++ b/src/session-store.js @@ -113,6 +113,7 @@ export class SessionStore { delivered_attachments: Array.isArray(existing.delivered_attachments) ? existing.delivered_attachments : [], dom_snapshot: existing.dom_snapshot || "", chat: existing.chat || [], + annotations: existing.annotations || [], updated_at: new Date().toISOString(), }; state.sessions[key] = session; @@ -237,10 +238,29 @@ export class SessionStore { ? [] : acceptedPrompts .filter((prompt) => prompt.tag === "message" && prompt.prompt) - .map((prompt) => ({ role: "user", text: prompt.prompt, at: new Date().toISOString() })); + .map((prompt) => ({ role: "user", text: prompt.prompt, at })); + // Annotation-tagged prompts never reach session.chat (only tag === "message" does) and + // session.prompts is a write-only outbox drained by takeFeedback, so without this they + // leave no visible trace once sent. This is the durable, human-facing record of them. A + // restore replays a batch that was already accepted (and recorded) the first time it ran + // through this method, so it must not record the annotations a second time. + const newAnnotations = restoring + ? [] + : acceptedPrompts + .filter((prompt) => prompt.tag !== "message" && prompt.selector) + .map((prompt) => ({ + id: prompt.id || "", + selector: prompt.selector, + tag: prompt.tag, + text: prompt.text, + prompt: prompt.prompt, + at, + ...(prompt.target ? { target: prompt.target } : {}), + })); const existingPrompts = Array.isArray(session.prompts) ? session.prompts : []; session.prompts = restoring ? [...acceptedPrompts, ...existingPrompts] : [...existingPrompts, ...acceptedPrompts]; session.chat = [...(session.chat || []), ...userMessages]; + session.annotations = [...(session.annotations || []), ...newAnnotations]; if (restoring) { const restoredFailures = Array.isArray(payload.artifact_failures) ? JSON.parse(JSON.stringify(payload.artifact_failures)) @@ -703,6 +723,8 @@ function normalizePrompt(prompt) { tag: String(prompt.tag || ""), text: String(prompt.text || ""), }; + const id = String(prompt.id || "").trim(); + if (id) normalized.id = id; const target = normalizeTarget(prompt.target); if (target) normalized.target = target; const { refs, malformed } = normalizeAttachmentRefs(prompt.attachments); diff --git a/test/annotation-badge.browser.test.js b/test/annotation-badge.browser.test.js new file mode 100644 index 00000000..32f257d3 --- /dev/null +++ b/test/annotation-badge.browser.test.js @@ -0,0 +1,146 @@ +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; +import { access, mkdtemp, readFile, rm } from "node:fs/promises"; +import http from "node:http"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; + +import { createSdkJs } from "../src/server.js"; + +const projectRoot = fileURLToPath(new URL("..", import.meta.url)); + +async function chromePath() { + const candidates = [ + process.env.CHROME_PATH, + "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome", + "/Applications/Chromium.app/Contents/MacOS/Chromium", + "/usr/bin/google-chrome", + "/usr/bin/chromium", + "/usr/bin/chromium-browser", + ].filter(Boolean); + for (const candidate of candidates) { + try { + await access(candidate); + return candidate; + } catch { + continue; + } + } + return ""; +} + +const page = ` +annotation badge + + +
spacer - grown later to move the target without a scroll or resize event
+
Annotated
+
bottom filler
+ + +`; + +// Badges are repositioned from scroll, resize, and ResizeObserver events rather than an always-on +// frame loop, so their correctness is exactly "does the dot still sit on its element after the page +// moved?" - a question only a real browser can answer. +test( + "annotation badges stay pinned to their element through scrolling and layout shifts", + { timeout: 120_000 }, + async (t) => { + const chrome = await chromePath(); + if (!chrome) { + t.skip("Chrome or Chromium is required for the annotation-badge regression"); + return; + } + const root = await mkdtemp(path.join(os.tmpdir(), "lavish-annotation-badge-")); + const files = new Map([ + ["index.html", page], + ["sdk.js", createSdkJs("annotation-badge-test")], + ["fixture.js", await readFile(path.join(projectRoot, "test/fixtures/annotation-badge.browser.js"), "utf8")], + ]); + + /** @type {(value: unknown) => void} */ + let report = () => {}; + const reported = new Promise((resolve) => { + report = resolve; + }); + + const server = http.createServer((request, response) => { + if (request.method === "POST" && request.url === "/result") { + let body = ""; + request.on("data", (chunk) => { + body += chunk; + }); + request.on("end", () => { + response.writeHead(204).end(); + try { + report(JSON.parse(body)); + } catch (error) { + report({ pass: false, error: `unparseable result: ${String(error)}` }); + } + }); + return; + } + const name = request.url === "/" ? "index.html" : decodeURIComponent(String(request.url).slice(1)); + const body = files.get(name); + if (body === undefined) { + response.writeHead(404).end(); + return; + } + response.writeHead(200, { + "content-type": name.endsWith(".js") ? "text/javascript; charset=utf-8" : "text/html; charset=utf-8", + "cache-control": "no-store", + }); + response.end(body); + }); + + await new Promise((resolve) => server.listen(0, "127.0.0.1", () => resolve(undefined))); + const address = server.address(); + if (!address || typeof address === "string") throw new Error("test server did not bind to a TCP port"); + + const browser = spawn( + chrome, + [ + "--headless=new", + "--disable-gpu", + "--disable-dev-shm-usage", + "--no-sandbox", + "--no-first-run", + `--user-data-dir=${path.join(root, "chrome-profile")}`, + "--window-size=1200,800", + `http://127.0.0.1:${address.port}/`, + ], + { stdio: "ignore" }, + ); + + try { + const result = await Promise.race([ + reported, + new Promise((resolve) => setTimeout(() => resolve(null), 60_000).unref()), + ]); + + assert.ok(result, "browser fixture did not report a result"); + assert.equal(result.pass, true, result.error); + assert.equal(result.badgeCount, 1, "one badge is drawn for one annotated element"); + + for (const stage of ["initial", "afterScroll", "afterLayoutShift", "afterReplace"]) { + assert.ok(result[stage], `no badge position was measured ${stage}`); + assert.ok( + result[stage].dx <= 1 && result[stage].dy <= 1, + `the badge must sit on its element ${stage}, drifted by ${JSON.stringify(result[stage])}`, + ); + } + } finally { + const exited = new Promise((resolve) => browser.once("exit", resolve)); + browser.kill("SIGKILL"); + await exited; + server.closeAllConnections(); + await new Promise((resolve) => server.close(() => resolve(undefined))); + await rm(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } + }, +); diff --git a/test/artifact-sdk.test.js b/test/artifact-sdk.test.js index 3176e5f6..0fea893e 100644 --- a/test/artifact-sdk.test.js +++ b/test/artifact-sdk.test.js @@ -4,6 +4,7 @@ import test from "node:test"; import { classifyMaterialRectEscape, classifySevereTextOverflow, + dedupeAnnotationTargets, deriveLavishQueueKey, findStableLayoutFindings, isMaterialPageOverflow, @@ -324,3 +325,40 @@ test("isModeToggleHotkeyEvent ignores other keys even with a modifier held", () assert.equal(isModeToggleHotkeyEvent({ key: "e", metaKey: true }), false); assert.equal(isModeToggleHotkeyEvent({ key: "Enter", metaKey: true }), false); }); + +test("dedupeAnnotationTargets keeps the earliest id per resolved element", () => { + const elA = { name: "a" }; + const elB = { name: "b" }; + const resolve = (selector) => ({ "sel-a": elA, "sel-b": elB })[selector] || null; + + const result = dedupeAnnotationTargets( + [ + { id: "1", selector: "sel-a" }, + { id: "2", selector: "sel-a" }, + { id: "3", selector: "sel-b" }, + ], + resolve, + ); + + assert.deepEqual(result, [ + { id: "1", el: elA, selector: "sel-a" }, + { id: "3", el: elB, selector: "sel-b" }, + ]); +}); + +test("dedupeAnnotationTargets drops entries with no id, no selector, or an unresolved selector", () => { + const elA = { name: "a" }; + const resolve = (selector) => (selector === "sel-a" ? elA : null); + + const result = dedupeAnnotationTargets( + [ + { id: "", selector: "sel-a" }, + { id: "1", selector: "" }, + { id: "2", selector: "sel-missing" }, + { id: "3", selector: "sel-a" }, + ], + resolve, + ); + + assert.deepEqual(result, [{ id: "3", el: elA, selector: "sel-a" }]); +}); diff --git a/test/chrome-client-queue.test.js b/test/chrome-client-queue.test.js index 9c2c9f82..7884c942 100644 --- a/test/chrome-client-queue.test.js +++ b/test/chrome-client-queue.test.js @@ -692,6 +692,89 @@ test("chrome client falls back to the locator when a table cell has no row or co assert.doesNotMatch(html, /Locator/); }); +test("sending a queued annotation moves it into the sent-annotations section without a server round-trip", async () => { + const chrome = await createChromeHarness(); + + chrome.sendFrameMessage({ + type: "lavish:queuePrompt", + prompt: { id: "ann-1", prompt: "Make this warmer", selector: "h1", tag: "h1", text: "Hello" }, + }); + assert.equal(chrome.queued().length, 1); + + chrome.element("send").onclick(); + chrome.sendFrameMessage({ type: "lavish:snapshot", snapshot: "" }); + await flushPromises(); + + assert.equal(chrome.queued().length, 0); + const entry = chrome.element("annotationsSent").children[0]; + assert.ok(entry, "a sent-annotation entry was appended"); + assert.equal(entry.dataset.annotationId, "ann-1"); +}); + +test("clicking a sent annotation row asks the artifact iframe to reveal its element", async () => { + const chrome = await createChromeHarness(); + + chrome.sendFrameMessage({ + type: "lavish:queuePrompt", + prompt: { id: "ann-1", prompt: "Make this warmer", selector: "h1", tag: "h1", text: "Hello" }, + }); + chrome.element("send").onclick(); + chrome.sendFrameMessage({ type: "lavish:snapshot", snapshot: "" }); + await flushPromises(); + + const entry = chrome.element("annotationsSent").children[0]; + assert.equal(entry.classList.contains("reveal-target"), true); + entry.dispatch("click", { stopPropagation() {} }); + + const revealMessage = chrome.postedToFrame.at(-1); + assert.equal(revealMessage.type, "lavish:revealElement"); + assert.equal(revealMessage.selector, "h1"); +}); + +test("an openAnnotation message from the artifact scrolls the matching sent-annotation entry into view", async () => { + const chrome = await createChromeHarness(); + + chrome.sendFrameMessage({ + type: "lavish:queuePrompt", + prompt: { id: "ann-1", prompt: "Make this warmer", selector: "h1", tag: "h1", text: "Hello" }, + }); + chrome.element("send").onclick(); + chrome.sendFrameMessage({ type: "lavish:snapshot", snapshot: "" }); + await flushPromises(); + + chrome.sendFrameMessage({ type: "lavish:openAnnotation", id: "ann-1" }); + + const entry = chrome.element("annotationsSent").children[0]; + assert.ok(entry.scrolledIntoView, "the entry was scrolled into view"); +}); + +test("chrome client posts the current annotation targets to the iframe after queueing and after send", async () => { + const chrome = await createChromeHarness(); + + chrome.sendFrameMessage({ + type: "lavish:queuePrompt", + prompt: { id: "ann-1", prompt: "Make this warmer", selector: "h1", tag: "h1", text: "Hello" }, + }); + + const queuedTargets = chrome.postedToFrame.filter((message) => message.type === "lavish:setAnnotationTargets"); + assert.ok(queuedTargets.length > 0, "targets were posted after queueing"); + const beforeSend = queuedTargets.at(-1).targets; + assert.equal(beforeSend.length, 1); + assert.equal(beforeSend[0].id, "ann-1"); + assert.equal(beforeSend[0].selector, "h1"); + + chrome.element("send").onclick(); + chrome.sendFrameMessage({ type: "lavish:snapshot", snapshot: "" }); + await flushPromises(); + + const afterSend = chrome.postedToFrame + .filter((message) => message.type === "lavish:setAnnotationTargets") + .at(-1).targets; + assert.equal(afterSend.length, 1); + assert.equal(afterSend[0].id, "ann-1"); + assert.equal(afterSend[0].selector, "h1"); +}); + test("chrome client scrolls new chat bubbles into view above queued prompts", async () => { const chrome = await createChromeHarness(); const panelScroll = chrome.element("panelScroll"); @@ -3812,12 +3895,15 @@ test("chrome send and end with an empty composer nudges instead of ending", asyn }, }); chrome.element("sendHint").hidden = true; + // Startup itself posts the current (empty) annotation-target list once; only the messages + // caused by the click below are under test. + const baselinePostCount = chrome.postedToFrame.length; chrome.element("sendAndEnd").onclick(); await flushPromises(); assert.equal(posts.length, 0); - assert.equal(chrome.postedToFrame.length, 0); + assert.equal(chrome.postedToFrame.length, baselinePostCount); assert.equal(chrome.element("sendHint").hidden, false); assert.equal(chrome.element("chatInput").focused, true); assert.equal(chrome.element("chatInput").disabled, false); @@ -4058,6 +4144,9 @@ test("artifact relays cannot invoke whiteboard persistence", async () => { return whiteboardFetch(url); }, }); + // Startup itself posts the current (empty) annotation-target list once; only the messages + // caused by the forged relay below are under test. + const baselinePostCount = chrome.postedToFrame.length; chrome.sendFrameMessage({ type: "lavish:whiteboardRelay", @@ -4067,7 +4156,7 @@ test("artifact relays cannot invoke whiteboard persistence", async () => { await flushPromises(); assert.equal(calls.length, 0); - assert.equal(chrome.postedToFrame.length, 0); + assert.equal(chrome.postedToFrame.length, baselinePostCount); }); test("unverified whiteboard frames cannot invoke whiteboard persistence", async () => { diff --git a/test/cli-version.test.js b/test/cli-version.test.js index 0ba8d163..915e9576 100644 --- a/test/cli-version.test.js +++ b/test/cli-version.test.js @@ -15,9 +15,12 @@ const execFileAsync = promisify(execFile); const BIN = fileURLToPath(new URL("../bin/lavish-axi.js", import.meta.url)); // A regression to the pre-fast-path behavior costs the full telemetry drain (up to -// 1000ms) plus process startup. Windows process startup is substantially slower on -// hosted runners, so give it more headroom while staying below the drain timeout. -const VERSION_BUDGET_MS = process.platform === "win32" ? 750 : 500; +// 1000ms) plus process startup. This budget sits far below that and far above the +// ~60ms the fast path actually needs, so it catches the regression without flaking. +// Windows CI runners pay much higher child-process spawn overhead than macOS/Linux +// runners for the same fast path, so the budget is widened there; it still sits well +// below the full telemetry-drain regression cost it's guarding against. +const VERSION_BUDGET_MS = process.platform === "win32" ? 2500 : 500; // Accepts the telemetry connection and never answers, so a regression pays the whole // drain timeout instead of a fast connection refusal. diff --git a/test/fixtures/annotation-badge.browser.js b/test/fixtures/annotation-badge.browser.js new file mode 100644 index 00000000..370edf45 --- /dev/null +++ b/test/fixtures/annotation-badge.browser.js @@ -0,0 +1,100 @@ +/* global document, window */ + +// Drives the injected SDK's annotation badges in a real browser: badges are position:fixed inside +// a shadow root, so "does the dot stay on its element?" is only answerable from live rects. +(() => { + const result = { pass: false }; + + function frame() { + return new Promise((resolve) => window.requestAnimationFrame(() => resolve())); + } + + async function settle(maxFrames = 240) { + let previous = NaN; + let stable = 0; + for (let i = 0; i < maxFrames; i += 1) { + await frame(); + const current = window.scrollY; + stable = current === previous ? stable + 1 : 0; + previous = current; + if (stable >= 4) return true; + } + return false; + } + + function badges() { + const host = document.querySelector(".lavish-annotation-root"); + if (!host || !host.shadowRoot) return []; + return [...host.shadowRoot.querySelectorAll(".lavish-annotation-badge")]; + } + + function drift(target) { + const node = badges()[0]; + if (!node) return null; + const a = node.getBoundingClientRect(); + const b = target.getBoundingClientRect(); + return { dx: Math.abs(a.left - (b.right - 6)), dy: Math.abs(a.top - (b.top - 6)) }; + } + + // Repositioning is event-driven and bounded, so give it a fixed number of frames to converge + // rather than guessing a settling delay. A badge that never tracks its element never converges, + // so the reported drift stays the real one and the assertion still fails on a regression. + async function settledDrift(target, maxFrames = 120) { + let last = drift(target); + for (let i = 0; i < maxFrames; i += 1) { + if (last && last.dx <= 1 && last.dy <= 1) return last; + await frame(); + last = drift(target); + } + return last; + } + + async function main() { + const target = document.getElementById("target"); + window.scrollTo(0, 0); + await settle(); + + window.postMessage({ type: "lavish:setAnnotationTargets", targets: [{ id: "ann-1", selector: "#target" }] }, "*"); + await frame(); + + result.badgeCount = badges().length; + result.initial = await settledDrift(target); + + window.scrollBy(0, 300); + await settle(); + result.afterScroll = await settledDrift(target); + + // Let the post-scroll settle window lapse first, so the next stage cannot be carried by frames + // that the scroll already scheduled. + for (let i = 0; i < 45; i += 1) await frame(); + + // Resize the annotated element itself, which moves the badge's anchor with no scroll or window + // resize event to react to - only observing the element catches this. + target.style.width = "240px"; + result.afterLayoutShift = await settledDrift(target); + + for (let i = 0; i < 45; i += 1) await frame(); + + // Replace the annotated element outright with a new node matching the same selector, as a + // dynamic artifact re-rendering a list row would. The old element goes detached with no resize + // or scroll event firing at all - only re-resolving through the selector catches this. + const replacement = document.createElement("div"); + replacement.id = "target"; + replacement.textContent = "Replaced"; + replacement.style.margin = "0 120px"; + replacement.style.padding = "24px"; + replacement.style.background = "#cfe"; + replacement.style.border = "1px solid #391"; + target.replaceWith(replacement); + result.afterReplace = await settledDrift(replacement); + + result.pass = true; + } + + main() + .catch((error) => { + result.error = String((error && error.stack) || error); + }) + .then(() => fetch("/result", { method: "POST", body: JSON.stringify(result) })) + .catch(() => {}); +})(); diff --git a/test/fixtures/reveal-marker.browser.js b/test/fixtures/reveal-marker.browser.js new file mode 100644 index 00000000..e8603959 --- /dev/null +++ b/test/fixtures/reveal-marker.browser.js @@ -0,0 +1,90 @@ +/* global document, window */ + +// Drives the injected SDK's reveal path in a real browser and reports geometry back through +// body[data-result]. The marker is position:fixed inside a shadow root, so "is it on the element?" +// is only answerable by comparing live rects after the browser has actually scrolled. +(() => { + const result = { pass: false }; + + function frame() { + return new Promise((resolve) => window.requestAnimationFrame(() => resolve())); + } + + // Wait until scrolling stops changing, so the assertion runs against a settled viewport rather + // than a guessed delay. Capped so a stuck scroll fails loudly instead of hanging the fixture. + async function settle(maxFrames = 240) { + let previous = NaN; + let stable = 0; + for (let i = 0; i < maxFrames; i += 1) { + await frame(); + const current = window.scrollY; + stable = current === previous ? stable + 1 : 0; + previous = current; + if (stable >= 4) return true; + } + return false; + } + + function marker() { + const host = document.querySelector(".lavish-annotation-root"); + if (!host || !host.shadowRoot) return null; + return host.shadowRoot.querySelector(".lavish-reveal-marker"); + } + + // How far the marker's top-left sits from the element it is supposed to be framing. + function drift(target) { + const node = marker(); + if (!node) return null; + const a = node.getBoundingClientRect(); + const b = target.getBoundingClientRect(); + return { dx: Math.abs(a.left - b.left), dy: Math.abs(a.top - b.top) }; + } + + async function main() { + const target = document.getElementById("target"); + window.scrollTo(0, 0); + await settle(); + + result.startedAtTop = window.scrollY === 0; + result.targetBelowFold = target.getBoundingClientRect().top > window.innerHeight; + + window.postMessage({ type: "lavish:revealElement", selector: "#target" }, "*"); + + result.scrollSettled = await settle(); + result.scrolledBy = Math.round(window.scrollY); + result.markerPresent = Boolean(marker()); + + // The original symptom: after a smooth scroll, is the box actually on the element? + result.afterScroll = drift(target); + + // The marker removes itself 2.4s after it is drawn, and the settling above is unbounded in + // wall-clock terms on a loaded runner. Re-arm it rather than measuring a marker that expired, + // so a slow machine reports a timing miss instead of a phantom regression. + if (!marker()) { + window.postMessage({ type: "lavish:revealElement", selector: "#target" }, "*"); + await settle(); + result.rearmedBeforeSecondScroll = true; + } + + // Deterministic proof of tracking, independent of how the browser implements smooth + // scrolling: a one-shot rect read can never survive a scroll that happens after it. + window.scrollBy(0, 240); + await settle(); + result.markerPresentAfterFurtherScroll = Boolean(marker()); + result.afterFurtherScroll = drift(target); + + result.pass = true; + } + + // Report over HTTP rather than through --dump-dom: the SDK keeps requestAnimationFrame loops + // running, so the renderer is never idle and Chrome's --virtual-time-budget never drains. + main() + .catch((error) => { + result.error = String((error && error.stack) || error); + }) + .then(() => { + document.body.dataset.result = JSON.stringify(result); + return fetch("/result", { method: "POST", body: JSON.stringify(result) }); + }) + .catch(() => {}); +})(); diff --git a/test/reveal-marker.browser.test.js b/test/reveal-marker.browser.test.js new file mode 100644 index 00000000..9251ba74 --- /dev/null +++ b/test/reveal-marker.browser.test.js @@ -0,0 +1,162 @@ +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; +import { access, mkdtemp, readFile, rm } from "node:fs/promises"; +import http from "node:http"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; + +import { createSdkJs } from "../src/server.js"; + +const projectRoot = fileURLToPath(new URL("..", import.meta.url)); + +async function chromePath() { + const candidates = [ + process.env.CHROME_PATH, + "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome", + "/Applications/Chromium.app/Contents/MacOS/Chromium", + "/usr/bin/google-chrome", + "/usr/bin/chromium", + "/usr/bin/chromium-browser", + ].filter(Boolean); + for (const candidate of candidates) { + try { + await access(candidate); + return candidate; + } catch { + continue; + } + } + return ""; +} + +const page = ` +reveal marker + + +
top filler - the target must start well below the fold
+
Reveal me
+
bottom filler - leaves room to scroll past the target
+ + +`; + +// A source-level assertion cannot prove the marker lands on the element - only that the code is +// shaped a certain way. This drives the real reveal path in a real browser: the box must sit on +// its element after the smooth scroll settles, and must stay there through a later scroll, which +// a one-shot getBoundingClientRect read can never do. +test("the reveal marker lands on its element and tracks it through scrolling", { timeout: 120_000 }, async (t) => { + const chrome = await chromePath(); + if (!chrome) { + t.skip("Chrome or Chromium is required for the reveal-marker regression"); + return; + } + const root = await mkdtemp(path.join(os.tmpdir(), "lavish-reveal-marker-")); + const files = new Map([ + ["index.html", page], + ["sdk.js", createSdkJs("reveal-marker-test")], + ["fixture.js", await readFile(path.join(projectRoot, "test/fixtures/reveal-marker.browser.js"), "utf8")], + ]); + + /** @type {(value: unknown) => void} */ + let report = () => {}; + const reported = new Promise((resolve) => { + report = resolve; + }); + + const server = http.createServer((request, response) => { + if (request.method === "POST" && request.url === "/result") { + let body = ""; + request.on("data", (chunk) => { + body += chunk; + }); + request.on("end", () => { + response.writeHead(204).end(); + try { + report(JSON.parse(body)); + } catch (error) { + report({ pass: false, error: `unparseable result: ${String(error)}` }); + } + }); + return; + } + const name = request.url === "/" ? "index.html" : decodeURIComponent(String(request.url).slice(1)); + const body = files.get(name); + if (body === undefined) { + response.writeHead(404).end(); + return; + } + response.writeHead(200, { + "content-type": name.endsWith(".js") ? "text/javascript; charset=utf-8" : "text/html; charset=utf-8", + "cache-control": "no-store", + }); + response.end(body); + }); + + await new Promise((resolve) => server.listen(0, "127.0.0.1", () => resolve(undefined))); + const address = server.address(); + if (!address || typeof address === "string") throw new Error("test server did not bind to a TCP port"); + + const browser = spawn( + chrome, + [ + "--headless=new", + "--disable-gpu", + "--disable-dev-shm-usage", + "--no-sandbox", + "--no-first-run", + `--user-data-dir=${path.join(root, "chrome-profile")}`, + "--window-size=1200,800", + `http://127.0.0.1:${address.port}/`, + ], + { stdio: "ignore" }, + ); + + try { + // unref so a fixture that reports promptly does not hold the event loop open for the + // whole fallback window. + const result = await Promise.race([ + reported, + new Promise((resolve) => setTimeout(() => resolve(null), 60_000).unref()), + ]); + + assert.ok(result, "browser fixture did not report a result"); + assert.equal(result.pass, true, result.error); + + // Preconditions - without these the assertions below would pass vacuously. + assert.equal(result.startedAtTop, true, "the page must start at the top so revealing has to scroll"); + assert.equal(result.targetBelowFold, true, "the target must start off-screen"); + assert.equal(result.scrollSettled, true, "scrolling never settled"); + assert.ok(result.scrolledBy > 0, "revealing the target scrolled the page"); + assert.equal(result.markerPresent, true, "a reveal marker was drawn"); + + // Without these, a marker that expired mid-fixture would surface as a null dereference rather + // than as the timing miss it is. + assert.ok(result.afterScroll, "the marker was gone before its position could be measured after the scroll settled"); + assert.equal( + result.markerPresentAfterFurtherScroll, + true, + "the marker was gone before its position could be measured after the second scroll", + ); + assert.ok(result.afterFurtherScroll, "no marker position was measured after the second scroll"); + + // 1px of tolerance for subpixel layout; the pre-fix bug drifts by the whole scroll distance. + assert.ok( + result.afterScroll.dy <= 1 && result.afterScroll.dx <= 1, + `the marker must frame its element once the scroll settles, drifted by ${JSON.stringify(result.afterScroll)}`, + ); + assert.ok( + result.afterFurtherScroll.dy <= 1 && result.afterFurtherScroll.dx <= 1, + `the marker must keep tracking through later scrolling, drifted by ${JSON.stringify(result.afterFurtherScroll)}`, + ); + } finally { + const exited = new Promise((resolve) => browser.once("exit", resolve)); + browser.kill("SIGKILL"); + await exited; + server.closeAllConnections(); + await new Promise((resolve) => server.close(() => resolve(undefined))); + await rm(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } +}); diff --git a/test/server.test.js b/test/server.test.js index ab3173d0..26637a76 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -31,6 +31,7 @@ import { resolveWatchTarget, serve, } from "../src/server.js"; +import * as artifactSdk from "../src/artifact-sdk.js"; import { canonicalFile, sessionKey, SessionStore } from "../src/session-store.js"; async function chromeClientSource() { @@ -548,6 +549,26 @@ test("artifact SDK script is valid JavaScript", () => { assert.doesNotThrow(() => new Function(js)); }); +// createSdkJs derives its artifact-sdk.js declarations from the module's exports, so every helper +// createArtifactSdk can reach is in scope by construction. Executing the emitted script proves the +// generated bundle actually parses and resolves rather than that its text mentions a name. +test("the emitted SDK resolves every artifact-sdk helper it references", () => { + const js = createSdkJs("abc"); + const referenced = Object.keys(artifactSdk).filter((name) => name !== "createArtifactSdk"); + assert.ok(referenced.length > 0, "artifact-sdk.js exports helpers for the SDK bundle"); + + // Run the bundle's declaration prologue - everything before it hands control to + // createArtifactSdk, which needs a live DOM - and ask for each helper by name. + const invocation = js.indexOf("\n(function createArtifactSdk"); + assert.ok(invocation > 0, "the emitted bundle invokes createArtifactSdk"); + const declarations = js.slice(js.indexOf("{") + 1, invocation); + const resolved = new Function(`${declarations}\nreturn { ${referenced.join(", ")} };`)(); + + for (const name of referenced) { + assert.notEqual(resolved[name], undefined, `${name} is not in scope in the emitted SDK bundle`); + } +}); + test("artifact SDK ignores Lavish-owned annotation UI", () => { const js = createSdkJs("abc"); @@ -1107,7 +1128,7 @@ test("chrome puts queued annotations above the chat composer as preview pills", assert.match(html, /id="annotationPills"/); assert.match( html, - /
<\/div>
<\/div><\/div>
/, + /
<\/div>
<\/div>
<\/div><\/div>
/, ); assert.match(js, /class="pill/); assert.match(js, /pill-preview/); diff --git a/test/session-store.test.js b/test/session-store.test.js index 3e8272b2..635fadef 100644 --- a/test/session-store.test.js +++ b/test/session-store.test.js @@ -68,6 +68,75 @@ test("queued prompts are returned with DOM snapshot context and then cleared", a } }); +test("annotation-tagged prompts create a durable annotation record separate from chat and the outbox", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "lavish-store-")); + try { + const stateFile = path.join(dir, "state.json"); + const artifact = path.join(dir, "artifact.html"); + await writeFile(artifact, "

Hello

"); + + const store = new SessionStore(stateFile); + const session = await store.upsertSession(artifact, "http://localhost:4387/session/test"); + await store.queuePrompts(session.key, { + prompts: [ + { id: "ann-1", uid: "1", prompt: "Make this warmer", selector: "h1", tag: "h1", text: "Hello" }, + { id: "", prompt: "Just a note", selector: "", tag: "message", text: "Freeform message" }, + ], + }); + + const stored = await store.findByKey(session.key); + assert.deepEqual(stored.annotations, [ + { + id: "ann-1", + selector: "h1", + tag: "h1", + text: "Hello", + prompt: "Make this warmer", + at: stored.annotations[0].at, + }, + ]); + assert.deepEqual(stored.chat, [{ role: "user", text: "Just a note", at: stored.chat[0].at }]); + + // Draining the outbox must not touch the durable annotation log. + await store.takeFeedback(session.key); + const afterDrain = await store.findByKey(session.key); + assert.equal(afterDrain.annotations.length, 1); + assert.equal(afterDrain.prompts.length, 0); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("annotation records preserve the target payload for text-range and Mermaid-node annotations", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "lavish-store-")); + try { + const stateFile = path.join(dir, "state.json"); + const artifact = path.join(dir, "artifact.html"); + await writeFile(artifact, "

Hello bright world

"); + + const store = new SessionStore(stateFile); + const session = await store.upsertSession(artifact, "http://localhost:4387/session/test"); + const target = { + type: "text-range", + text: "bright", + selector: "p#intro", + start: { selector: "p#intro", path: [0], offset: 6 }, + end: { selector: "p#intro", path: [0], offset: 12 }, + }; + await store.queuePrompts(session.key, { + prompts: [ + { id: "ann-2", uid: "", prompt: "Punch this up", selector: "p#intro", tag: "text", text: "bright", target }, + ], + }); + + const stored = await store.findByKey(session.key); + assert.equal(stored.annotations.length, 1); + assert.deepEqual(stored.annotations[0].target, target); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + test("queued text selection prompts preserve range anchors", async () => { const dir = await mkdtemp(path.join(tmpdir(), "lavish-store-")); try {