fix(guide): rysuj ramkę celu pod kursorem, nie nad nim

guidebot_recorder/guide/annotate.py

@@ -2,7 +2,7 @@
 
 from __future__ import annotations
 
-from guidebot_recorder.guide.geometry import Rect, Shape, clipped_arrow, rect_from_box
+from guidebot_recorder.guide.geometry import Shape, clipped_arrow, rect_from_box
 from guidebot_recorder.guide.model import Annotation
 from guidebot_recorder.models.scenario import ResolvedHighlight
 from guidebot_recorder.overlay.geometry import Ellipse, ellipse_around, fit_to_bounds
@@ -15,7 +15,15 @@
 CLICK_OUTER = 30.0
 
 #: Actions whose target box gets a red frame. `highlight` keeps its own ellipse.
-_FRAMED = frozenset({"click", "type", "hover", "select"})
+#:
+#: The frame itself is **not** an :class:`Annotation`. Every other mark is drawn
+#: as SVG over the finished screenshot, but that layer necessarily covers the
+#: cursor — a PNG is flat pixels, and nothing can slip under part of one after
+#: the fact. So the frame is drawn into the live page before the shutter, where
+#: the browser composites it beneath the cursor. :mod:`~guidebot_recorder.guide.stills`
+#: does the drawing; this set and :func:`target_shape` still decide *whether* and
+#: *where*, so the policy lives in one place regardless of who paints it.
+FRAMED_ACTIONS = frozenset({"click", "type", "hover", "select"})
 
 _Point = tuple[float, float]
 
@@ -101,7 +109,7 @@ def annotations_for(
 
     ``row_box``/``row_center`` describe the option row of a ``select:`` step whose
     list was photographed **open**, and they are what splits that one action's
-    marks across two boxes: the **frame** stays on the control, so the reader sees
+    marks across two boxes: the frame stays on the control, so the reader sees
     which field they are in, while the **star** and the arrow's tip go to the row,
     because clicking that row is literally what happens next. Every other action
     puts all three on the same box. The row is fed through the same
@@ -110,9 +118,12 @@ def annotations_for(
     — rather than a select-only variant that could drift from them.
 
     With no row geometry — ``mode: native``, where the option list is an OS popup
-    no screenshot can hold — a ``select`` is marked like any other framed action:
-    an arrow to the control's frame and the frame itself, with no star, because
-    nothing visible is being clicked.
+    no screenshot can hold — a ``select`` gets an arrow to the control and no
+    star, because nothing visible is being clicked.
+
+    The frame is deliberately absent from the list this returns: it is painted
+    into the page before the screenshot so the cursor stays on top of it. See
+    :data:`FRAMED_ACTIONS`.
     """
 
     anns: list[Annotation] = []
@@ -130,9 +141,6 @@ def annotations_for(
             (x1, y1), (x2, y2) = segment
             anns.append(Annotation(kind="arrow", x1=x1, y1=y1, x2=x2, y2=y2))
 
-    if action in _FRAMED and isinstance(shape, Rect):
-        anns.append(Annotation(kind="frame", x=shape.x, y=shape.y, w=shape.w, h=shape.h))
-
     if action == "click" and center is not None:
         anns.append(_star(center))
     elif action == "select" and row_center is not None:

guidebot_recorder/guide/layout.py

@@ -39,7 +39,9 @@
 .slide .subtitle { font-size: 24px; color: #555; margin-top: 4mm; }
 .arrow { stroke: #e11; stroke-width: 4; fill: none; marker-end: url(#ah); }
 .star { stroke: #e11; stroke-width: 4; fill: none; stroke-linecap: round; }
-.frame { stroke: #e11; stroke-width: 4; fill: rgba(238,17,17,0.08); }
+/* No `.frame` rule: the target outline is painted into the page before the
+   screenshot (see `stills._framed_screenshot`), because an SVG rectangle over
+   the finished PNG would cover the cursor. */
 /* The marker colour is per step, so only the shape lives here — `stroke` is set
    on the element itself. */
 .highlight { stroke-width: 5; fill: none; stroke-linecap: round; }
@@ -86,10 +88,6 @@ def _svg(anns: list[Annotation], size: tuple[int, int]) -> str:
             parts.append(f'<line class="arrow" x1="{a.x1}" y1="{a.y1}" x2="{a.x2}" y2="{a.y2}"/>')
         elif a.kind == "click":
             parts.extend(_star(a))
-        elif a.kind == "frame":
-            parts.append(
-                f'<rect class="frame" x="{a.x}" y="{a.y}" width="{a.w}" height="{a.h}" rx="4"/>'
-            )
         elif a.kind == "highlight":
             # The colour comes from the scenario, so it is escaped like any other
             # author-supplied text before it lands in an attribute.

guidebot_recorder/guide/model.py

@@ -13,7 +13,7 @@
 class Annotation:
     """One overlay mark, in screenshot pixels. Only the fields for `kind` are set."""
 
-    kind: Literal["arrow", "click", "frame", "highlight"]
+    kind: Literal["arrow", "click", "highlight"]
     # arrow: prev target edge -> current target edge
     x1: float | None = None
     y1: float | None = None
@@ -24,11 +24,6 @@ class Annotation:
     cy: float | None = None
     r_inner: float | None = None
     r_outer: float | None = None
-    # frame: rectangle around the box of any targeted action (click / type / hover / select)
-    x: float | None = None
-    y: float | None = None
-    w: float | None = None
-    h: float | None = None
     # highlight: ellipse around the target box (radii, plus its own colour —
     # the marker colour is per step, so it cannot live in the stylesheet)
     rx: float | None = None

guidebot_recorder/guide/replay.py

@@ -27,9 +27,10 @@
 from tqdm import tqdm
 
 from guidebot_recorder.diagnostics import step_banner
+from guidebot_recorder.guide.annotate import FRAMED_ACTIONS
 from guidebot_recorder.guide.model import GuidePage, page_text
 from guidebot_recorder.guide.prolog import GuideError
-from guidebot_recorder.guide.stills import _OpenListFrame, _screenshot
+from guidebot_recorder.guide.stills import _framed_screenshot, _OpenListFrame, _screenshot
 from guidebot_recorder.guide.trail import _CursorTrail
 from guidebot_recorder.models.action import CachedAction
 from guidebot_recorder.models.compiled import CompiledAction
@@ -360,6 +361,33 @@ async def _wait_page(run: _StepRun) -> None:
         tqdm.write(run.banner("pomijam: oczekiwanie nierozwiązane — uruchom `compile`"))
 
 
+def _frame_rect(run: _StepRun) -> dict | None:
+    """The box this step outlines in the page, or ``None`` when it outlines none."""
+
+    return run.box if run.act in FRAMED_ACTIONS else None
+
+
+async def _still(run: _StepRun) -> tuple[Path, tuple[int, int]]:
+    """This step's screenshot, with its target outlined *beneath* the cursor.
+
+    The overlay is read off the recorder, which already owns it, rather than
+    carried a second time on the capture. Read by plain attribute access and not
+    ``getattr(..., None)``: every recorder has the attribute (the real one always
+    assigns it, ``None`` included), so a default could only ever paper over a
+    rename — and it would do so by quietly dropping the outline from every
+    screenshot while the suite stayed green.
+    """
+
+    cap = run.capture
+    return await _framed_screenshot(
+        cap.page,
+        cap.shots_dir,
+        run.index,
+        overlay=cap.recorder.overlay,
+        rect=_frame_rect(run),
+    )
+
+
 async def _approach_target(run: _StepRun) -> bool:
     """Point the cursor at the target, reporting whether the step can go on.
 
@@ -387,12 +415,12 @@ async def _approach_target(run: _StepRun) -> bool:
 
 
 async def _type_frame(run: _StepRun) -> bool:
-    cap, step = run.capture, run.step
+    step = run.step
     text = (step.enter_text.text if step.enter_text else None) or run.action.input_text
     if text is None:
         raise GuideError(run.banner("brak zamrożonego tekstu — uruchom `compile`"))
     await run.locator.fill(text)
-    run.shot, run.size = await _screenshot(cap.page, cap.shots_dir, run.index)  # frame AFTER typing
+    run.shot, run.size = await _still(run)  # frame AFTER typing
     return True
 
 
@@ -411,7 +439,7 @@ async def _select_frame(run: _StepRun) -> bool:
     await cap.await_selects_ready(run)
     # Named `still`, not `frame`: in this codebase `frame` means a
     # Playwright frame everywhere else.
-    still = _OpenListFrame(cap.page, cap.shots_dir, run.index)
+    still = _OpenListFrame(cap.page, cap.shots_dir, run.index, overlay=cap.recorder.overlay)
     try:
         # `ripple=False` for the same reason `_approach_target` uses it: a still
         # capture wants a clean frame, not a click ring frozen mid-animation.
@@ -448,7 +476,7 @@ async def _select_frame(run: _StepRun) -> bool:
 
 
 async def _highlight_frame(run: _StepRun) -> bool:
-    cap, step = run.capture, run.step
+    step = run.step
     if step.highlight is None:
         raise GuideError(
             run.banner(
@@ -459,8 +487,8 @@ async def _highlight_frame(run: _StepRun) -> bool:
     # Deliberately no action on the element: `highlight` never touches the page,
     # and `_click_or_hover_frame` would click it. The mark itself is drawn onto
     # the page by the annotation, not by the browser.
-    run.mark = step.highlight.resolved(cap.scenario.config.highlight)
-    run.shot, run.size = await _screenshot(cap.page, cap.shots_dir, run.index)
+    run.mark = step.highlight.resolved(run.capture.scenario.config.highlight)
+    run.shot, run.size = await _still(run)
     return True
 
 
@@ -495,9 +523,8 @@ async def _click_into_popup(run: _StepRun) -> Page:
 
 
 async def _click_or_hover_frame(run: _StepRun) -> bool:
-    cap = run.capture
     # frame BEFORE click/hover
-    run.shot, run.size = await _screenshot(cap.page, cap.shots_dir, run.index)
+    run.shot, run.size = await _still(run)
     if run.act == "hover":
         await run.locator.hover()
     elif _opens_popup(run.action):

guidebot_recorder/guide/stills.py

@@ -19,6 +19,7 @@
 
 from playwright.async_api import Page
 
+from guidebot_recorder.overlay.overlay import Overlay
 from guidebot_recorder.recorder.recorder import SelectReveal
 
 
@@ -30,6 +31,40 @@ async def _screenshot(page: Page, shots_dir: Path, index: int) -> tuple[Path, tu
     return path, (size["width"], size["height"])
 
 
+async def _framed_screenshot(
+    page: Page,
+    shots_dir: Path,
+    index: int,
+    *,
+    overlay: Overlay | None,
+    rect: dict | None,
+) -> tuple[Path, tuple[int, int]]:
+    """Photograph the page with ``rect`` outlined in it, then take the outline down.
+
+    The outline is painted into the live page rather than drawn over the finished
+    image, and that is the only way round to get it *under* the cursor: a PNG is
+    flat pixels, so an SVG rectangle added afterwards covers everything below it,
+    the cursor included. In the page the browser composites the two, and the
+    cursor's own maximal z-index still wins.
+
+    ``rect`` is ``None`` for a step that gets no frame (``highlight`` draws an
+    ellipse instead) and for a page with no target at all, and then this is a
+    plain :func:`_screenshot`.
+
+    The take-down is in a ``finally`` because the frame lives in the page, not in
+    this process: an exception between paint and clear would leave it standing in
+    every screenshot that followed, silently marking the wrong element.
+    """
+
+    if rect is None or overlay is None:
+        return await _screenshot(page, shots_dir, index)
+    await overlay.frame(page, rect["x"], rect["y"], rect["width"], rect["height"])
+    try:
+        return await _screenshot(page, shots_dir, index)
+    finally:
+        await overlay.clear_frame(page)
+
+
 class _OpenListFrame:
     """The `select:` step's screenshot, taken while its option list is unfurled.
 
@@ -48,14 +83,26 @@ class _OpenListFrame:
     screen.
     """
 
-    def __init__(self, page: Page, shots_dir: Path, index: int) -> None:
+    def __init__(
+        self, page: Page, shots_dir: Path, index: int, *, overlay: Overlay | None = None
+    ) -> None:
         self._page = page
         self._shots_dir = shots_dir
         self._index = index
+        self._overlay = overlay
         self.shot: Path | None = None
         self.size: tuple[int, int] | None = None
         self.reveal: SelectReveal | None = None
 
     async def __call__(self, reveal: SelectReveal) -> None:
         self.reveal = reveal
-        self.shot, self.size = await _screenshot(self._page, self._shots_dir, self._index)
+        # The frame goes on the *control*, and only this callback knows where that
+        # is: the box measured before the choreography belongs to the collapsed
+        # control, which for a page-enhanced select was never on screen at all.
+        self.shot, self.size = await _framed_screenshot(
+            self._page,
+            self._shots_dir,
+            self._index,
+            overlay=self._overlay,
+            rect=reveal.control_box,
+        )

guidebot_recorder/overlay/cursor_effects.js

@@ -293,6 +293,73 @@
     return true;
   }
 
+  //: Stroke width of the target outline, in px — the same 4 the guide's SVG
+  //: `.frame` rule used, so the mark did not change size when it moved layers.
+  const FRAME_BORDER = 4;
+
+  //: Deliberately one below `cursor.js`'s MAX_Z_INDEX (2147483647). That single
+  //: digit IS the feature: it is what puts the outline under the cursor instead
+  //: of over it. Raise it to the max and the two tie, with paint order — and so
+  //: the bug — decided by which element happens to be appended last.
+  const FRAME_Z_INDEX = "2147483645";
+
+  /**
+   * A persistent outline around a target box, drawn BELOW the cursor.
+   *
+   * Not a variant of `highlight()` above, despite the similar shape: that one is
+   * the film's one-off pulse and removes itself after ~900 ms. This one stays
+   * until `clearFrame()`, because its whole job is to be standing there when the
+   * PDF guide takes its screenshot.
+   *
+   * The z-index is the entire point of drawing it here at all. The guide used to
+   * draw this rectangle as SVG *over* the finished PNG, which necessarily put it
+   * over the cursor too — a PNG is flat pixels, so nothing can slip underneath
+   * part of it after the fact. Drawn into the live page it is composited by the
+   * browser, where the cursor's `MAX_Z_INDEX` still wins.
+   *
+   * The box is grown by half the border width so the outline straddles the
+   * target's edge exactly like the SVG `stroke` it replaces — CSS borders are
+   * drawn inside the box, an SVG stroke is centred on the path.
+   *
+   * The look is fixed rather than configurable: these are the very values the
+   * guide's `.frame` stylesheet rule used, and nothing has ever wanted a second
+   * set. An options bag here would be four branches no caller reaches.
+   */
+  function frame(x, y, width, height) {
+    const values = [x, y, width, height].map(Number);
+    if (!values.every(Number.isFinite) || values[2] < 0 || values[3] < 0) {
+      throw new TypeError("frame bounds must be finite with non-negative size");
+    }
+    const root = mountRoot();
+    if (!root) {
+      scheduleMount();
+      return false;
+    }
+    // One frame at a time: a step marks one target, and a leftover from an
+    // earlier step would be photographed as if it belonged to this one.
+    clearFrame();
+
+    const half = FRAME_BORDER / 2;
+    const box = document.createElement("div");
+    box.setAttribute("data-guidebot-frame", "");
+    styleTransient(box, FRAME_Z_INDEX);
+    setImportant(box, "left", `${values[0] - half}px`);
+    setImportant(box, "top", `${values[1] - half}px`);
+    setImportant(box, "width", `${values[2] + FRAME_BORDER}px`);
+    setImportant(box, "height", `${values[3] + FRAME_BORDER}px`);
+    setImportant(box, "border", `${FRAME_BORDER}px solid #e11`);
+    setImportant(box, "border-radius", "4px");
+    setImportant(box, "background", "rgba(238, 17, 17, .08)");
+    root.appendChild(box);
+    return true;
+  }
+
+  function clearFrame() {
+    for (const box of document.querySelectorAll("[data-guidebot-frame]")) {
+      box.remove();
+    }
+  }
+
   function hide() {
     hidden = true;
     const cursor = document.querySelector(CURSOR_SELECTOR);
@@ -312,6 +379,8 @@
     moveTo,
     ripple,
     highlight,
+    frame,
+    clearFrame,
     encircle,
     hide,
     show,