Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -1489,7 +1489,7 @@ When a user denies a plan and Claude resubmits, the UI shows what changed betwee

**State** (`packages/ui/hooks/usePlanDiff.ts`): Manages base version selection, diff computation, and version fetching. The server sends `previousPlan` with the initial `/api/plan` response; the hook auto-diffs against it. Users can select any prior version from the sidebar Version Browser.

**Diff annotations:** The clean diff view supports block-level annotation — hover over added/removed/modified sections to annotate entire blocks. Annotations carry a `diffContext` field (`added`/`removed`/`modified`). Exported feedback includes `[In diff content]` labels.
**Diff annotations:** The clean diff view supports block-level annotation — hover over added/removed/modified sections to annotate entire blocks. Annotations carry a `diffContext` field (`added`/`removed`/`modified`). Exported feedback includes `[In diff content]` labels. A diff annotation's `blockId` is `diff-block-N` (N indexes the diff against the selected base, not a document block), so the export cannot place it in the document: diff comments print together AFTER the document's other comments, in diff order (`sortAnnotationsInDocumentOrder` in `packages/ui/utils/parser.ts`); a document with no diff comments sorts exactly as before.

**Annotation hook** (`packages/ui/hooks/useAnnotationHighlighter.ts`): Annotation infrastructure used by `Viewer.tsx`. Manages web-highlighter lifecycle, toolbar/popover state, annotation creation, text-based restoration, and scroll-to-selected. The diff view uses its own block-level hover system instead.

Expand Down Expand Up @@ -1629,7 +1629,7 @@ Share links carry the FENCED form in the payload's `p` (`shareableDocumentMarkdo

**Raw-HTML annotate:** the sandboxed viewer never mutates the visited page's DOM. Committed annotations render as numbered placed comment markers plus overlay-projected highlight rectangles inside a shadow-rooted fixed overlay host: the durable anchor data (element selector, text snapshot, normalized selected point) is persisted, and the markers/highlights are disposable projections re-resolved from it on every reconcile. Shift-click multi-select joins additional elements to one comment (`htmlAdditionalTargets`).

**Element context (agent-facing).** A pinpoint also captures a bounded description of the element at click time, `elementContext` on the annotation (`HtmlElementContext` in `packages/ui/types.ts`; extra targets carry a smaller one as `context`), built by `buildElementContext` in the bridge and re-validated at the parent trust boundary by `parseHtmlElementContext`, which lives in `packages/core/html-anchor.ts` beside `parseHtmlElementAnchor` (#1549); `useHtmlAnnotation.ts` imports and re-exports it rather than mirroring it, and the host persistence helpers in that same core module (`buildPersistedHtmlAnchor`, `projectHostThreads`) run it too, so a host that persists and projects through core keeps the field instead of dropping it on save. It is purely descriptive, never read by restore. Fields: tag, id, author classes, an ancestor `path`, explicit-or-implicit `role`, accessible `name`, an ALLOWLISTED attribute set (href/src scrubbed of query and fragment, `data:` truncated to its media type), rendered `text` (300), a collapsed HTML `outline` (600 chars; two child levels, then one, then a per-tag count, whichever first fits), child count, viewport `rect`, nearest `landmark` and `heading`, a `component` hint from `data-component`/`data-testid` ancestry (deliberately no React fiber reads), and in live-app sessions the `page` route and title. Never captured: form values, `on*` handlers, `style`, script/style/template contents, full innerHTML. Hard cap 2 KiB serialized per primary (1 KiB per extra target), shedding outline → text → attrs → classes → path → heading → landmark → component. Under the persisted anchor's own 16 KiB budget `buildPersistedHtmlAnchor` sheds contexts — per-target first, from the end, then the primary — BEFORE it drops any target, since a context is descriptive and re-derivable on the next click while a dropped target loses a marker the reviewer placed; `DEFAULT_HTML_ANCHOR_MAX_BYTES` is unchanged and the dropped-target counts still count targets only. The export (`elementContextExportBlock` in `packages/ui/utils/parser.ts`) prints a 4-backtick `html` fence of the outline plus `selector` / `path` / `role` · `name` · `component` / `attrs` / `text` / `box` / `near` lines under the comment; a text-less pinpoint's placeholder quote names WHICH element it is, not just its kind: the hover label plus the accessible name (`ctxName`: aria-label, alt, title, …) plus, for media (`img`, `video`/`audio` and their `<source>`, `iframe`, `embed`, `object`, `input type=image`, svg `<image>`), the source FILE name through the same `ctxScrubUrl` scrub (`[element: Image "Team photo" (team.jpg)]`, `[element: Image (data:image/png)]`; brackets and double quotes are swapped so it stays one token; `describeTextlessElement` in the bridge, used for the primary and every shift-click extra target). Restore never reads that quote (the anchor binds a text-less element), so pins saved with the older bare `[element: Image]` keep restoring. The export rewrites the placeholder to `Feedback on the <nav> element — "Primary"` in the heading, plus the `src` file name for media (`Feedback on the <img> element (jane.png)`), while a pinpoint with real quoted text keeps its quote line and gains the block. In srcdoc sessions `ctxScrubUrl` also drops the session's `/api/html-assets/<token>/` prefix, so a `src` reads as the author's own document-relative path, percent-decoded (`jane doe.png`). **Ask AI** carries the same identity: a question asked from a pinpoint composer sends `elementIdentityForAskAI(draftTargets)` (`packages/ui/utils/parser.ts`, the identity lines of `elementContextExportBlock` with `includeOutline: false`, numbered per element) as the scope's `detail`, which `buildDefaultPrompt` appends under "Selected element:" and the chat never displays; SDK providers, "Ask this session" and the agent terminal all send that prompt. It carries no query strings: the live route and an `href`/`src` selector rung are scrubbed to `?…` there (the persisted annotation keeps them). Annotations without the field export byte-identically. The grouped live-app export omits the `route` line (the `## Page:` heading carries it); `exportAnnotationEntry` (one annotation, no number) includes it; it is a pure helper for hosts and tests, and the annotation panel's card chrome is unchanged. Both helpers also take `includeOutline` (default `true`): `false` prints the identity lines without the fenced outline, for model turns where the 600-char outline is the expensive part per annotation. Share links drop `elementContext` exactly like anchors. The feedback archive records identity only (`elementTag`, `elementSelector`, `elementPath`, `elementRole`, `elementName`, `pageUrl`), never the outline. No `BRIDGE_PROTOCOL_VERSION` bump: the field is additive in both directions.
**Element context (agent-facing).** A pinpoint also captures a bounded description of the element at click time, `elementContext` on the annotation (`HtmlElementContext` in `packages/ui/types.ts`; extra targets carry a smaller one as `context`), built by `buildElementContext` in the bridge and re-validated at the parent trust boundary by `parseHtmlElementContext`, which lives in `packages/core/html-anchor.ts` beside `parseHtmlElementAnchor` (#1549); `useHtmlAnnotation.ts` imports and re-exports it rather than mirroring it, and the host persistence helpers in that same core module (`buildPersistedHtmlAnchor`, `projectHostThreads`) run it too, so a host that persists and projects through core keeps the field instead of dropping it on save. It is purely descriptive, never read by restore. Fields: tag, id, author classes, an ancestor `path`, explicit-or-implicit `role`, accessible `name`, an ALLOWLISTED attribute set (href/src scrubbed of query and fragment, `data:` truncated to its media type), rendered `text` (300), a collapsed HTML `outline` (600 chars; two child levels, then one, then a per-tag count, whichever first fits), child count, viewport `rect`, nearest `landmark` and `heading`, a `component` hint from `data-component`/`data-testid` ancestry (deliberately no React fiber reads), and in live-app sessions the `page` route and title. Never captured: form values, `on*` handlers, `style`, script/style/template contents, full innerHTML. Hard cap 2 KiB serialized per primary (1 KiB per extra target), shedding outline → text → attrs → classes → path → heading → landmark → component → sourceName (one order, `ELEMENT_CONTEXT_SHED_ORDER` in core, mirrored by the bridge's `CTX_SHED_ORDER`; a test keeps them equal). Under the persisted anchor's own 16 KiB budget `buildPersistedHtmlAnchor` sheds contexts — per-target first, from the end, then the primary — BEFORE it drops any target, since a context is descriptive and re-derivable on the next click while a dropped target loses a marker the reviewer placed; `DEFAULT_HTML_ANCHOR_MAX_BYTES` is unchanged and the dropped-target counts still count targets only. The export (`elementContextExportBlock` in `packages/ui/utils/parser.ts`) prints a 4-backtick `html` fence of the outline plus `selector` / `path` / `role` · `name` · `component` / `attrs` / `text` / `box` / `near` lines under the comment; a text-less pinpoint's placeholder quote names WHICH element it is, not just its kind: the hover label plus the accessible name (`ctxName`: aria-label, alt, title, …) plus, for media (`img`, `video`/`audio` and their `<source>`, `iframe`, `embed`, `object`, `input type=image`, svg `<image>`), the source FILE name through the same `ctxScrubUrl` scrub (`[element: Image "Team photo" (team.jpg)]`, `[element: Image (data:image/png)]`; brackets and double quotes are swapped so it stays one token; `describeTextlessElement` in the bridge, used for the primary and every shift-click extra target). Restore never reads that quote (the anchor binds a text-less element), so pins saved with the older bare `[element: Image]` keep restoring. The export rewrites the placeholder to `Feedback on the <nav> element — "Primary"` in the heading, plus the media file name (`Feedback on the <img> element (jane.png)`), read from the context's `sourceName`: the bridge records the file `describeTextlessElement` names (src, a lazy `data-src` over a `data:` placeholder, `srcset`, `<picture><source srcset>`, a video's `<source>` or `poster`, scrubbed the same way), so the heading and the composer quote always name the same file; contexts saved without `sourceName` fall back to the scrubbed `src` attribute, while a pinpoint with real quoted text keeps its quote line and gains the block. In srcdoc sessions `ctxScrubUrl` also drops the session's `/api/html-assets/<token>/` prefix, so a `src` reads as the author's own document-relative path, percent-decoded (`jane doe.png`). **Ask AI** carries the same identity: a question asked from a pinpoint composer sends `elementIdentityForAskAI(draftTargets)` (`packages/ui/utils/parser.ts`, the identity lines of `elementContextExportBlock` with `includeOutline: false`, numbered per element) as the scope's `detail`, which `buildDefaultPrompt` appends under "Selected element:" and the chat never displays; SDK providers, "Ask this session" and the agent terminal all send that prompt. It carries no query strings: the live route and an `href`/`src` selector rung are scrubbed to `?…` there (the persisted annotation keeps them). Annotations without the field export byte-identically. The grouped live-app export omits the `route` line (the `## Page:` heading carries it); `exportAnnotationEntry` (one annotation, no number) includes it; it is a pure helper for hosts and tests, and the annotation panel's card chrome is unchanged. Both helpers also take `includeOutline` (default `true`): `false` prints the identity lines without the fenced outline, for model turns where the 600-char outline is the expensive part per annotation. Share links drop `elementContext` exactly like anchors. The feedback archive records identity only (`elementTag`, `elementSelector`, `elementPath`, `elementRole`, `elementName`, `pageUrl`), never the outline. No `BRIDGE_PROTOCOL_VERSION` bump: the field is additive in both directions.

**HTML and live-app interaction model:** raw-HTML sessions and live app sessions (`mode: "annotate-app"`) share one contract. Both open with pinpoint **armed** (`htmlAnnotateArmed` defaults to `true`, `packages/editor/App.tsx:493`; live sessions open armed like every other HTML surface, `App.tsx:2871`). `Esc` walks a ladder instead of exiting outright: a pending draft closes first, then the pinpoint hover outline clears, and only then does `Esc` drop the surface to **Interact**, where the bridge goes passive so clicks, forms, text selection, and SPA navigation reach the page natively (`packages/ui/components/html-viewer/bridge-script.ts:3066-3080`; committed markers stay visible and a marker click still opens its comment, and in Interact an open drag-comment draft still closes before `Esc` is handed back to the page). Vim owns its own ladder and is skipped here. The header **pen** button toggles Annotate/Interact (`packages/editor/components/AppHeader.tsx:386-404`, `aria-pressed`), as does `Mod+Shift+A` (`packages/ui/shortcuts/plan-review/htmlAnnotate.shortcuts.ts`) — a real toggle in BOTH directions, which is what makes it the answer to "Esc dropped me to Interact, how do I get back?". Disarming through either path tears down any pending draft, because the bridge's `set-annotate-mode(false)` handler clears every pending affordance (`bridge-script.ts:719-739`), exactly as the Esc ladder does. The bridge mirrors the chord inside the iframe on the capture phase and forwards it to the parent, so it works whichever document owns focus. Text drag-selection commenting is **always live**, on both surfaces and in both states, ungated from the armed flag and from the input method (`bridge-script.ts:384-386`, `:1420-1436`): while armed, a click pins an element and a drag selects text at the same time, and the one-shot `dragEndedClick` guard stops a completed drag's trailing click from re-pinning (`bridge-script.ts:1381-1390`).

Expand Down
24 changes: 24 additions & 0 deletions packages/core/html-anchor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -449,4 +449,28 @@ describe("parseHtmlElementContext", () => {
expect(oversized).toBeDefined();
expect(bytes(oversized)).toBeLessThanOrEqual(MAX_ELEMENT_CONTEXT_BYTES);
});

// Failure to catch: an image pin on a long live-app route whose context
// only fits WITHOUT `sourceName` being dropped whole (undefined) because
// `sourceName` was not in the shed order.
test("an oversized context sheds sourceName before it drops the whole context", () => {
const base = { tag: "img", role: "img", name: "Hero" };
const sourceName = "hero-banner-with-a-long-name.webp";
// A route sized so the context fits with a few bytes to spare, then
// overflows once sourceName rides along.
const slack = 8;
const urlLength = MAX_ELEMENT_CONTEXT_BYTES - bytes({ ...base, page: { url: "" } }) - slack;
const page = { url: `/${"r".repeat(urlLength - 1)}` };
expect(bytes({ ...base, page })).toBeLessThanOrEqual(MAX_ELEMENT_CONTEXT_BYTES);
expect(bytes({ ...base, sourceName, page })).toBeGreaterThan(MAX_ELEMENT_CONTEXT_BYTES);

const shed = parseHtmlElementContext({ ...base, sourceName, page });
expect(shed).toBeDefined();
expect(shed?.sourceName).toBeUndefined();
expect(shed?.page?.url).toBe(page.url);
expect(shed?.name).toBe("Hero");

// With room to spare it is kept.
expect(parseHtmlElementContext({ ...base, sourceName, page: { url: "/home" } })?.sourceName).toBe(sourceName);
});
});
24 changes: 22 additions & 2 deletions packages/core/html-anchor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ export interface HtmlElementContext {
component?: string;
/** Live-app sessions only: the route the element was seen on and the page title. */
page?: { url: string; title?: string };
/**
* Media elements only: the file the element shows (`team.jpg`,
* `data:image/png`), resolved by the bridge the same way the composer quote
* names it — `src`, a lazy `data-src`, `srcset`, `<picture><source>`, a
* `<video>`'s `<source>` or `poster` — after the query/fragment scrub.
*/
sourceName?: string;
}

/** Mirrors `MAX_ANCHOR_SELECTOR_LENGTH` in `@plannotator/ui`. */
Expand Down Expand Up @@ -112,6 +119,8 @@ const MAX_CONTEXT_LANDMARK_LENGTH = 80;
const MAX_CONTEXT_HEADING_LENGTH = 130;
const MAX_CONTEXT_COMPONENT_LENGTH = 100;
const MAX_CONTEXT_PAGE_TITLE_LENGTH = 200;
/** The bridge caps the name at 60; headroom for the parent's own collapse. */
const MAX_CONTEXT_SOURCE_NAME_LENGTH = 80;

/** Attribute names the context may carry (mirrors CONTEXT_ATTRS in the bridge). */
export const CONTEXT_ATTR_ALLOWLIST = new Set([
Expand Down Expand Up @@ -202,6 +211,16 @@ function contextBytes(value: unknown): number {
return new TextEncoder().encode(JSON.stringify(value)).length;
}

/**
* Fields an over-budget context sheds, first to last; mirrors `CTX_SHED_ORDER`
* in the bridge (a test keeps the two in step). `sourceName` goes last: the
* export heading falls back to the `src` attribute without it, and shedding it
* is still better than losing the whole context.
*/
export const ELEMENT_CONTEXT_SHED_ORDER: ReadonlyArray<keyof HtmlElementContext> = [
"outline", "text", "attrs", "classes", "path", "heading", "landmark", "component", "sourceName",
];

/**
* Validate a bridge-posted element context. Pure and fail-closed: returns
* `undefined` if `value` is not a valid element context or tag is missing/empty.
Expand Down Expand Up @@ -268,6 +287,8 @@ export function parseHtmlElementContext(value: unknown): HtmlElementContext | un
if (heading) context.heading = heading;
const component = collapseContextScalar(value.component, MAX_CONTEXT_COMPONENT_LENGTH);
if (component) context.component = component;
const sourceName = collapseContextScalar(value.sourceName, MAX_CONTEXT_SOURCE_NAME_LENGTH);
if (sourceName) context.sourceName = sourceName;
if (isRecord(value.page)) {
const url = collapseContextScalar(value.page.url, MAX_PAGE_URL_LENGTH);
if (url) {
Expand All @@ -277,8 +298,7 @@ export function parseHtmlElementContext(value: unknown): HtmlElementContext | un
}
}
// Serialized bound: shed the expendable fields in the bridge's order until the whole fits.
const shedOrder: Array<keyof HtmlElementContext> = ["outline", "text", "attrs", "classes", "path", "heading", "landmark", "component"];
for (const field of shedOrder) {
for (const field of ELEMENT_CONTEXT_SHED_ORDER) {
if (contextBytes(context) <= MAX_ELEMENT_CONTEXT_BYTES) break;
delete context[field];
}
Expand Down
Loading
Loading