fix(diagrams): enable sequence annotations

skills/htmlit/SKILL.md

@@ -177,21 +177,12 @@ links the whole thread).
 **Only comments and freeform messages arrive in `prompts` - highlights do not.**
 
 - `prompts[].prompt` is what the user typed.
-- `prompts[].tag` is `"text"` (a commented text selection), an element tag like
-  `"h1"`/`"img"`/`"g"` (a non-text target such as an image or a Mermaid node,
-  where text can't be selected), or `"message"` (freeform chat).
+- `prompts[].tag` is `"text"` (a commented text selection), an element tag like `"h1"`/`"img"`/`"g"` (a non-text target such as an image or a Mermaid element, where text can't be selected), or `"message"` (freeform chat).
 - `prompts[].range` (when present) pins the exact commented span: `container` is
   a CSS selector and `start`/`end` are character offsets into that element's text,
   with the commented `text`. It is `null` for element/diagram targets and freeform
   messages, where `selector` (if any) locates the target.
-- `prompts[].commentId` (non-empty for a text-span comment **or a diagram
-  node/edge comment**) is the id of the "you asked here" anchor left on that
-  target - a purple underline on text, or a purple outline box on the diagram
-  element. **When you answer such a comment, set
-  `data-htmlit-answer-for="<commentId>"` on your `data-htmlit-answer` block**
-  (see below) - the anchor then links to your answer, and clicking it in the page
-  scrolls to and flashes that answer. (For a freeform `message` it is empty; just
-  answer normally.)
+- `prompts[].commentId` (non-empty for a text-span comment **or a diagram element comment**) is the id of the "you asked here" anchor left on that target - a purple underline on text, or a purple outline box on the diagram element. **When you answer such a comment, set `data-htmlit-answer-for="<commentId>"` on your `data-htmlit-answer` block** (see below) - the anchor then links to your answer, and clicking it in the page scrolls to and flashes that answer. (For a freeform `message` it is empty; just answer normally.)
 - `prompts[].selector` is a CSS selector for the target (the range's container
   for text, or the element itself). `type` is `feedback`, `ended`, or (with
   `--once`) `timeout`.
@@ -307,13 +298,7 @@ The injected client auto-renders, on first load and after every morph:
   numbers) with a **Copy** button to grab the syntax in place. A diagram
   inside a hidden tab or a closed `<details>` renders **automatically the moment it
   is revealed**, so it is safe to put diagrams in tabs/accordions - they never
-  collapse. Manual
-  arrangement is remembered per diagram
-  source, so it survives a theme switch or a live morph. (A Mermaid node/edge is
-  **clicked** to leave a comment - its text can't be text-selected - while a drag
-  still pans/moves it, so commenting and rearrange coexist without a mode. The SVG
-  shows the normal arrow cursor at rest and switches to a grabbing cursor only
-  while you actually drag.)
+  collapse. Manual arrangement is remembered per diagram source, so it survives a theme switch or a live morph. (A Mermaid flowchart node/edge or sequence participant/message/note is **clicked** to leave a comment - its text can't be text-selected - while a drag still pans/moves it, so commenting and rearrange coexist without a mode. The SVG shows the normal arrow cursor at rest and switches to a grabbing cursor only while you actually drag.)
 - **Code** - `<pre><code class="language-python">...</code></pre>` is highlighted
   by highlight.js and rendered with a **language name** and a **Copy** button on top
   and a pinned **line-number gutter** (the gutter stays put while long lines scroll
@@ -376,14 +361,7 @@ small floating menu offers two actions:
   are saved with the review but are a live-review marker only - stripped from your
   snapshot and from the HTML/PDF exports.
 
-A **Mermaid** node/edge (whose text isn't selectable, since dragging pans the
-diagram) is targeted with a **click**, which offers **Ask agent** only (a text
-note doesn't apply to a diagram shape). Sending leaves a persistent purple
-**outline box** on that node/edge - the diagram equivalent of a text comment
-anchor - that re-resolves across Mermaid re-renders and, once you answer, becomes
-the clickable jump to your answer. The CSS selector is never shown to the user,
-but comments deliver it (and a `commentId`) to you in `prompts[]`; highlights and
-anchors stay local.
+A **Mermaid** flowchart node/edge or sequence participant/message/note (whose text isn't selectable, since dragging pans the diagram) is targeted with a **click**, which offers **Ask agent** only (a text note doesn't apply to a diagram shape). Sending leaves a persistent purple **outline box** on that element - the diagram equivalent of a text comment anchor - that re-resolves across Mermaid re-renders and, once you answer, becomes the clickable jump to your answer. The CSS selector is never shown to the user, but comments deliver it (and a `commentId`) to you in `prompts[]`; highlights and anchors stay local.
 
 ## Visual guidance
 

skills/htmlit/client/annotate.js

@@ -10,6 +10,8 @@ var pending = null;
 var selMenuOpen = false;
 var hlSeq = 0;
 
+export const DIAGRAM_TARGET_ATTR = "data-htmlit-diagram-key";
+
 export function getPending() { return pending; }
 export function hasPendingOrMenu() { return !!(pending || selMenuOpen); }
 
@@ -18,7 +20,7 @@ export function annotateEl(el) {
   buildElPending(el);
   openSelMenu();
 }
-// A click (not a drag) on a diagram node/edge. If it already carries a comment
+// A click (not a drag) on a diagram element. If it already carries a comment
 // anchor: jump to the answer when there is one, else open the anchor's menu
 // (comment again / remove). Otherwise start a fresh comment on it.
 export function diagramClick(el) {
@@ -113,15 +115,19 @@ export function inMermaid(node) {
   var el = node && (node.nodeType === 1 ? node : node.parentElement);
   return !!(el && el.closest && el.closest(".mermaid"));
 }
-// A stable key for a clicked diagram node/edge: the diagram's source text plus the
-// element's id with the (per-render) svg-id prefix stripped, so it re-resolves even
-// after Mermaid re-renders the SVG (theme switch, morph) with a fresh svg id.
+// A stable key for a clicked diagram element: prefer an explicit target key for
+// Mermaid shapes without ids, otherwise strip the per-render SVG prefix from its id.
 export function diagramKeyOf(el) {
   var frame = el && el.closest && el.closest(".mermaid");
   if (!frame || !frame.dataset || !frame.dataset.htmlitSrc) return null;
   var svg = frame.querySelector("svg");
+  var keyed = el && el.closest ? el.closest("[" + DIAGRAM_TARGET_ATTR + "]") : null;
+  if (svg && keyed && keyed.closest("svg") === svg) {
+    var targetKey = keyed.getAttribute(DIAGRAM_TARGET_ATTR);
+    if (targetKey) return { src: frame.dataset.htmlitSrc, nodeKey: targetKey };
+  }
   var idEl = el.id ? el : (el.closest ? el.closest("[id]") : null);
-  if (!svg || !idEl || (idEl.closest && !idEl.closest(".mermaid"))) return null;
+  if (!svg || !idEl || idEl === svg || (idEl.closest && !idEl.closest(".mermaid"))) return null;
   var prefix = svg.id ? svg.id + "-" : "";
   var id = idEl.id || "";
   var key = prefix && id.indexOf(prefix) === 0 ? id.slice(prefix.length) : id;
@@ -136,6 +142,10 @@ export function resolveDiagramTarget(t) {
     if (!f.dataset || f.dataset.htmlitSrc !== t.src) continue;
     var svg = f.querySelector("svg");
     if (!svg) return null;
+    var keyed = svg.querySelectorAll("[" + DIAGRAM_TARGET_ATTR + "]");
+    for (var k = 0; k < keyed.length; k++) {
+      if (keyed[k].getAttribute(DIAGRAM_TARGET_ATTR) === t.nodeKey) return keyed[k];
+    }
     var prefix = svg.id ? svg.id + "-" : "";
     var els = svg.querySelectorAll("[id]");
     for (var j = 0; j < els.length; j++) {
@@ -305,7 +315,7 @@ export function addCommentAnchor(range, prompt, id) {
   appState.highlights.push({ id: id || newAnchorId(), kind: HighlightKind.COMMENT, prompt: prompt || "", comments: [prompt || ""],
     range: { container: range.container, start: range.start, end: range.end, text: range.text } });
 }
-// A comment anchor on a diagram node/edge: no text range, just a stable target.
+// A comment anchor on a diagram element: no text range, just a stable target.
 export function addCommentAnchorEl(target, text, prompt, id) {
   if (!target || !target.src) return;
   var existing = findAnchorByTarget(target);
@@ -403,7 +413,7 @@ export function commentItemFromPending(prompt) {
     var ex = findAnchorBySpan(rng);
     return { selector: pending.range.container, tag: "text", text: pending.text, prompt: prompt, note: false, range: rng, target: null, commentId: (ex && ex.id) || newAnchorId() };
   }
-  // diagram node/edge (a fresh click, or re-commenting an existing diagram anchor)
+  // diagram element (a fresh click, or re-commenting an existing diagram anchor)
   var diag = pending.diagram || pending.target || null;
   if (diag && diag.src) {
     var exd = findAnchorByTarget(diag);

skills/htmlit/client/diagram.js

@@ -5,11 +5,47 @@ import { theme } from "./theme.js";
 import { renderHighlights, scheduleReposition } from "./overlays.js";
 import { positionRail } from "./rail.js";
 import { makeCopyButton, makeLinenos, lineCount, highlightMermaidSource } from "./codeblock.js";
-import { clearPending, diagramClick } from "./annotate.js";
+import { clearPending, DIAGRAM_TARGET_ATTR, diagramClick } from "./annotate.js";
 
 export const diagramLayouts = {}; // diagram source text -> saved arrangement
 export const diagramRegistry = []; // { svg, fit } per live diagram, for export/print
 
+function directChildMatches(el, selector) {
+  for (var i = 0; i < el.children.length; i++) if (el.children[i].matches(selector)) return true;
+  return false;
+}
+
+function setupSequenceTargets(svg) {
+  var actorGroups = Array.prototype.filter.call(svg.querySelectorAll("g"), function (group) {
+    return directChildMatches(group, "rect.actor, text.actor.actor-box");
+  });
+  actorGroups.forEach(function (group, index) {
+    group.setAttribute(DIAGRAM_TARGET_ATTR, "sequence-actor-" + index);
+  });
+
+  var noteGroups = Array.prototype.filter.call(svg.querySelectorAll("g"), function (group) {
+    return directChildMatches(group, "rect.note, text.noteText");
+  });
+  noteGroups.forEach(function (group, index) {
+    group.setAttribute(DIAGRAM_TARGET_ATTR, "sequence-note-" + index);
+  });
+
+  var messages = Array.prototype.slice.call(svg.querySelectorAll("text.messageText"));
+  var messageLines = Array.prototype.slice.call(svg.querySelectorAll(".messageLine0, .messageLine1"));
+  messages.forEach(function (message, index) {
+    message.setAttribute(DIAGRAM_TARGET_ATTR, "sequence-message-" + index);
+  });
+
+  return function (el) {
+    if (!el || !el.closest) return null;
+    var keyed = el.closest("[" + DIAGRAM_TARGET_ATTR + "]");
+    if (keyed && keyed.closest("svg") === svg) return keyed;
+    var line = el.closest(".messageLine0, .messageLine1");
+    var lineIndex = line ? messageLines.indexOf(line) : -1;
+    return lineIndex >= 0 ? (messages[lineIndex] || null) : null;
+  };
+}
+
 /**
  * @typedef {{x: number, y: number}} Point
  * @typedef {{left: number, top: number, right: number, bottom: number}} Rect
@@ -190,7 +226,7 @@ export function enhanceMermaid() {
     try { setupDiagram(svg); } catch (e) {}
   });
   // The diagram SVG is now in the DOM, so any "you asked here" boxes on its
-  // nodes/edges can be (re)resolved and drawn.
+  // elements can be (re)resolved and drawn.
   if (typeof renderHighlights === "function") renderHighlights();
   if (typeof positionRail === "function") positionRail();
 }
@@ -200,6 +236,7 @@ export function setupDiagram(svg) {
   var box = svg.parentNode;
   var sourceKey = (box && box.dataset && box.dataset.htmlitSrc) || svg.id || "";
   var space = svg.querySelector("g.root") || svg;
+  var sequenceTarget = setupSequenceTargets(svg);
 // Guarantee the frame is the bar's containing block AND clips it, as inline
 // styles - so the zoom bar can never escape the frame if the injected
 // stylesheet hasn't applied yet during a live morph (a reload used to "fix" it).
@@ -566,6 +603,7 @@ if (box && box.classList && box.classList.contains("mermaid")) {
     var u = toUser(e.clientX, e.clientY);
     var nodeEl = closest("g.node"), labelEl = closest("g.edgeLabels g.edgeLabel"), pathEl = closest("g.edgePaths path");
     var node = nodeEl && nodeByEl(nodeEl);
+    var annotationTarget = sequenceTarget(t);
     // A label is not independently movable - grabbing it bends its edge, so the
     // label always stays attached to the line.
     var edge = (pathEl && edgeByPath(pathEl)) || (labelEl && edgeByLabel(labelEl)) || null;
@@ -574,7 +612,7 @@ if (box && box.classList && box.classList.contains("mermaid")) {
     // prefer it over the edge path (whose bounding box spans both nodes and would
     // "highlight the whole square"). Fall back to the path when there is no label.
     else if (edge && edge.src && edge.tgt && edge.src !== edge.tgt) drag = { type: "edge", edge: edge, hit: labelEl || edge.label || pathEl || edge.el };
-    else drag = { type: "pan", sx: e.clientX, sy: e.clientY };
+    else drag = { type: "pan", hit: annotationTarget, sx: e.clientX, sy: e.clientY };
     drag.moved = false; drag.cx0 = e.clientX; drag.cy0 = e.clientY;
     try { svg.setPointerCapture(e.pointerId); } catch (_) {}
   });
@@ -615,10 +653,11 @@ if (box && box.classList && box.classList.contains("mermaid")) {
     try { svg.releasePointerCapture(e.pointerId); } catch (_) {}
     var d = drag; drag = null; svg.style.cursor = "default";
     if (d.moved) { save(); return; }
-    // A click (no drag) acts on the pressed node/edge - text isn't selectable in a
+    // A click (no drag) acts on the pressed diagram element - text isn't selectable in a
     // diagram. diagramClick() routes it: jump to an answered anchor, open an
     // existing anchor's menu, or start a fresh comment. A background click dismisses.
-    if (d.type === "node" || d.type === "edge") diagramClick(d.hit || (d.node && d.node.el) || (d.edge && d.edge.el));
+    var hit = d.hit || (d.node && d.node.el) || (d.edge && d.edge.el);
+    if (hit) diagramClick(hit);
     else clearPending();
   }
   svg.addEventListener("pointerup", endDrag);

tests/client/fixtures/sequence.html

@@ -0,0 +1,16 @@
+<!doctype html>
+<html lang="en">
+<head>
+  <meta charset="utf-8" />
+  <title>htmlit sequence annotation fixture</title>
+</head>
+<body>
+  <h1>Sequence annotation fixture</h1>
+  <pre><code class="language-mermaid">sequenceDiagram
+  participant A as "Caller"
+  participant B as "Worker"
+  A->>B: "Resolve request"
+  Note over A,B: "The first attempt can race"
+  B-->>A: "Return result"</code></pre>
+</body>
+</html>

tests/client/sequence-annotation.test.mjs

@@ -0,0 +1,126 @@
+/**
+ * Browser regression test for comments on Mermaid sequence diagrams.
+ *
+ * Sequence actors, messages, and notes do not use the flowchart g.node /
+ * g.edgePaths structure, and most of their SVG elements have no id. A click
+ * therefore used to fall through as a background pan, leaving no annotation
+ * menu or stable target for the comment anchor.
+ */
+
+import assert from "node:assert/strict";
+import { dirname, join } from "node:path";
+import { fileURLToPath } from "node:url";
+import { test } from "node:test";
+
+import { startReview } from "./harness.mjs";
+
+const HERE = dirname(fileURLToPath(import.meta.url));
+const FIXTURE = join(HERE, "fixtures", "sequence.html");
+
+async function clickSequenceText(page, selector, text) {
+  const point = await page.evaluate(
+    ({ selector, text }) => {
+      const target = [...document.querySelectorAll(`.mermaid ${selector}`)]
+        .find((el) => (el.textContent || "").includes(text));
+      if (!target) return null;
+      target.scrollIntoView({ block: "center" });
+      const rect = target.getBoundingClientRect();
+      return { x: rect.left + rect.width / 2, y: rect.top + rect.height / 2 };
+    },
+    { selector, text },
+  );
+  assert.ok(point, `expected ${selector} containing ${text}`);
+  await page.mouse.click(point.x, point.y);
+}
+
+async function menuDisplay(page) {
+  return page.evaluate(() => {
+    const chrome = document.getElementById("htmlit-chrome");
+    const menu = chrome.shadowRoot.querySelector(".selmenu");
+    return getComputedStyle(menu).display;
+  });
+}
+
+test("sequence diagram elements can be commented and re-resolve after reload", async (t) => {
+  let browser;
+  try {
+    const { default: puppeteer } = await import("puppeteer");
+    browser = await puppeteer.launch({ args: ["--no-sandbox", "--disable-setuid-sandbox"] });
+  } catch (err) {
+    return t.skip(`no browser available: ${err.message}`);
+  }
+
+  let review;
+  let page;
+  try {
+    review = await startReview(FIXTURE);
+    page = await browser.newPage();
+    await page.setViewport({ width: 1200, height: 900 });
+    await page.goto(review.url, { waitUntil: "domcontentloaded" });
+    await page.waitForFunction(
+      () => {
+        const svg = document.querySelector(".mermaid svg");
+        return svg && svg.__htmlitDrag && svg.querySelector(".messageText");
+      },
+      { timeout: 20000 },
+    );
+
+    for (const [selector, text] of [
+      ["text.actor.actor-box", "Caller"],
+      ["text.noteText", "first attempt can race"],
+    ]) {
+      await clickSequenceText(page, selector, text);
+      assert.equal(await menuDisplay(page), "flex", `clicking ${text} should open the annotation menu`);
+      await page.keyboard.press("Escape");
+    }
+
+    await clickSequenceText(page, ".messageText", "Resolve request");
+    assert.equal(await menuDisplay(page), "flex", "clicking a sequence message should open the annotation menu");
+
+    const highlightSaved = page.waitForResponse(
+      (response) => response.url().endsWith(`/api/${review.key}/highlights`) && response.request().method() === "POST",
+    );
+    const targetKey = await page.evaluate(() => {
+      const root = document.getElementById("htmlit-chrome").shadowRoot;
+      root.querySelector('[data-act="hlcomment"]').click();
+      const input = root.querySelector(".pop .ce");
+      input.textContent = "Why can this race?";
+      input.dispatchEvent(new InputEvent("input", { bubbles: true, inputType: "insertText", data: "Why can this race?" }));
+      root.querySelector('[data-act="popadd"]').click();
+      const message = [...document.querySelectorAll(".mermaid .messageText")]
+        .find((el) => (el.textContent || "").includes("Resolve request"));
+      return message && message.getAttribute("data-htmlit-diagram-key");
+    });
+    await highlightSaved;
+
+    assert.match(targetKey || "", /^sequence-message-\d+$/, "the sequence message should receive a stable target key");
+
+    await page.evaluate(() => {
+      document.getElementById("htmlit-chrome").shadowRoot.querySelector('[data-act="send"]').click();
+    });
+    const feedback = await fetch(`${review.base}/api/poll?key=${review.key}&timeout=3`).then((response) => response.json());
+    const prompt = (feedback.prompts || []).find((item) => item.prompt === "Why can this race?");
+    assert.ok(prompt, "the sequence comment should be delivered to the agent");
+    assert.equal(prompt.tag, "text");
+    assert.ok(prompt.commentId, "the sequence comment should carry an anchor id");
+
+    await page.reload({ waitUntil: "domcontentloaded" });
+    await page.waitForFunction(
+      (key) => {
+        const message = [...document.querySelectorAll(".mermaid .messageText")]
+          .find((el) => (el.textContent || "").includes("Resolve request"));
+        const chrome = document.getElementById("htmlit-chrome");
+        return message &&
+          message.getAttribute("data-htmlit-diagram-key") === key &&
+          chrome &&
+          chrome.shadowRoot.querySelectorAll(".hls .cmbox").length === 1;
+      },
+      { timeout: 20000 },
+      targetKey,
+    );
+  } finally {
+    if (page) await page.close();
+    if (review) await review.stop();
+    await browser.close();
+  }
+});