From b3c9544b1e3bb758e31a176f2439e9fc69736b62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sat, 25 Jul 2026 09:28:21 +0200 Subject: [PATCH 1/3] =?UTF-8?q?fix(guide):=20rysuj=20ramk=C4=99=20celu=20p?= =?UTF-8?q?od=20kursorem,=20nie=20nad=20nim?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Czerwony prostokąt obrysowujący cel był adnotacją SVG doklejaną nad gotowym zrzutem — a kursor jest w tym zrzucie wypalony. PNG to płaskie piksele, więc warstwa dodana później przykrywa wszystko pod sobą, łącznie z kursorem, który czytelnik ma śledzić. Półprzezroczyste wypełnienie ramki kładło się na grocie strzałki. Ramka przenosi się więc tam, gdzie warstwy jeszcze istnieją: do żywej strony, przed migawkę. `cursor_effects.js` dostaje trwałe `frame()`/`clearFrame()` na z-index 2147483645 — pod kursorem (2147483647), tak jak filmowy błysk `highlight()`. Przeglądarka składa obie warstwy i dopiero wtedy spłaszcza je do PNG. `_framed_screenshot` obejmuje migawkę malowaniem i sprzątaniem; sprzątanie w `finally`, bo ramka żyje w stronie, nie w tym procesie — wyjątek pomiędzy zostawiłby ją na każdym kolejnym zrzucie, obrysowującą zły element. Dla `select:` maluje ją callback ujawnienia, bo tylko on zna pudełko kontrolki: zmierzone wcześniej należy do kontrolki zwiniętej. Adnotacja `kind="frame"` znika całkiem — razem z gałęzią w `_svg`, regułą `.frame` i polami x/y/w/h w `Annotation`. Zostawiona byłaby martwą ścieżką, która wygląda na działającą. Geometrię nadal wyznacza `target_shape`, więc polityka „co i gdzie obrysować" została w jednym miejscu. Zweryfikowane na zrzutach: kursor przecina dolną krawędź ramki i jest w całości widoczny, zarówno na kroku `click`, jak i `type` w popupie. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PEZwp8YivARFrZSjrSUnD5 --- guidebot_recorder/guide/annotate.py | 26 +++++---- guidebot_recorder/guide/layout.py | 8 ++- guidebot_recorder/guide/model.py | 7 +-- guidebot_recorder/guide/replay.py | 44 ++++++++++++---- guidebot_recorder/guide/stills.py | 48 ++++++++++++++++- guidebot_recorder/overlay/cursor_effects.js | 58 +++++++++++++++++++++ guidebot_recorder/overlay/overlay.py | 34 ++++++++++-- tests/unit/guide/_capture_helpers.py | 23 ++++++++ tests/unit/guide/test_annotate.py | 37 ++++++++----- tests/unit/guide/test_capture_popup.py | 34 ++++++++++++ tests/unit/guide/test_capture_select.py | 38 +++++++++----- tests/unit/guide/test_layout.py | 31 ++++++----- 12 files changed, 315 insertions(+), 73 deletions(-) diff --git a/guidebot_recorder/guide/annotate.py b/guidebot_recorder/guide/annotate.py index 528abc6..476f87f 100644 --- a/guidebot_recorder/guide/annotate.py +++ b/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: diff --git a/guidebot_recorder/guide/layout.py b/guidebot_recorder/guide/layout.py index f7fd575..879fc9b 100644 --- a/guidebot_recorder/guide/layout.py +++ b/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'') elif a.kind == "click": parts.extend(_star(a)) - elif a.kind == "frame": - parts.append( - f'' - ) 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. diff --git a/guidebot_recorder/guide/model.py b/guidebot_recorder/guide/model.py index 4e59ce3..a689265 100644 --- a/guidebot_recorder/guide/model.py +++ b/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 diff --git a/guidebot_recorder/guide/replay.py b/guidebot_recorder/guide/replay.py index c139b3a..b986a1d 100644 --- a/guidebot_recorder/guide/replay.py +++ b/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,30 @@ 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. + + ``overlay`` is read off the recorder rather than carried on the capture: the + recorder already owns it, and a stand-in recorder without one simply gets an + unframed still instead of an attribute error. + """ + + cap = run.capture + return await _framed_screenshot( + cap.page, + cap.shots_dir, + run.index, + overlay=getattr(cap.recorder, "overlay", None), + 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 +412,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 +436,9 @@ 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=getattr(cap.recorder, "overlay", None) + ) 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 +475,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 +486,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 +522,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): diff --git a/guidebot_recorder/guide/stills.py b/guidebot_recorder/guide/stills.py index b788eeb..7e567cf 100644 --- a/guidebot_recorder/guide/stills.py +++ b/guidebot_recorder/guide/stills.py @@ -30,6 +30,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, + 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 +82,24 @@ 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=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, + ) diff --git a/guidebot_recorder/overlay/cursor_effects.js b/guidebot_recorder/overlay/cursor_effects.js index 663ecb2..4ddf4ad 100644 --- a/guidebot_recorder/overlay/cursor_effects.js +++ b/guidebot_recorder/overlay/cursor_effects.js @@ -293,6 +293,62 @@ return true; } + /** + * 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. + */ + function frame(x, y, width, height, options = {}) { + 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 width_ = Number.isFinite(Number(options.borderWidth)) + ? Number(options.borderWidth) + : 4; + const half = width_ / 2; + const box = document.createElement("div"); + box.setAttribute("data-guidebot-frame", ""); + styleTransient(box, "2147483645"); + setImportant(box, "left", `${values[0] - half}px`); + setImportant(box, "top", `${values[1] - half}px`); + setImportant(box, "width", `${values[2] + width_}px`); + setImportant(box, "height", `${values[3] + width_}px`); + setImportant(box, "border", `${width_}px solid ${options.color || "#e11"}`); + setImportant(box, "border-radius", `${options.radius ?? 4}px`); + setImportant(box, "background", options.fill || "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 +368,8 @@ moveTo, ripple, highlight, + frame, + clearFrame, encircle, hide, show, diff --git a/guidebot_recorder/overlay/overlay.py b/guidebot_recorder/overlay/overlay.py index c9345fb..24c6a99 100644 --- a/guidebot_recorder/overlay/overlay.py +++ b/guidebot_recorder/overlay/overlay.py @@ -11,11 +11,15 @@ from guidebot_recorder.models.config import CursorConfig, Viewport from guidebot_recorder.overlay.geometry import Ellipse, ellipse_perimeter +#: Names that must all be callable for the injected script to count as current. +#: A page still carrying an older script fails this and gets re-injected, so a +#: newly added effect is the thing that makes the check notice — which is why +#: every new API name belongs in this list, not just the original five. _API_IS_READY = """() => { const api = window.__guidebot_cursor; - return !!api && ["ensure", "moveTo", "ripple", "highlight", "encircle"].every( - (name) => typeof api[name] === "function" - ); + return !!api && [ + "ensure", "moveTo", "ripple", "highlight", "frame", "clearFrame", "encircle" + ].every((name) => typeof api[name] === "function"); }""" #: Bounds on one lap of the `highlight` ellipse. Deliberately NOT the cursor's @@ -134,6 +138,30 @@ async def move_to( ) self.pos = target + async def frame(self, page: Page, x: float, y: float, width: float, height: float) -> None: + """Outline ``(x, y, width, height)`` in the page, beneath the cursor. + + Viewport coordinates, the same space :meth:`move_to` takes and the same + space ``Recorder.point`` reports its boxes in — so a caller can outline + exactly what it just pointed at without converting anything. + + Drawn in the page rather than composed over the finished screenshot + because a screenshot has no layers: an outline added afterwards covers + the cursor, and the cursor is what the reader is meant to follow. + """ + + await self.ensure(page) + await page.evaluate( + "([x, y, w, h]) => window.__guidebot_cursor.frame(x, y, w, h)", + [float(x), float(y), float(width), float(height)], + ) + + async def clear_frame(self, page: Page) -> None: + """Remove the outline, if one is up. Safe to call when none is.""" + + await self.ensure(page) + await page.evaluate("() => window.__guidebot_cursor.clearFrame()") + def lap_duration(self, rx: float, ry: float) -> float: """Duration (ms) of one lap around an ellipse, at the cursor's own speed.""" diff --git a/tests/unit/guide/_capture_helpers.py b/tests/unit/guide/_capture_helpers.py index c0313c6..32817d5 100644 --- a/tests/unit/guide/_capture_helpers.py +++ b/tests/unit/guide/_capture_helpers.py @@ -77,6 +77,28 @@ async def select_option(self, label): FAKE_CONTROL_CENTER = (140.0, 70.5) +class FakeOverlay: + """Records the outlines a capture paints into the page. + + The red frame is no longer an SVG annotation — it is drawn in the live page + before the shutter so the cursor stays on top of it — so this is where a test + now checks *that a step framed its target*, and with which box. + """ + + def __init__(self, events: list[str] | None = None): + self.events = events if events is not None else [] + self.frames: list[tuple[float, float, float, float]] = [] + self.clears = 0 + + async def frame(self, page, x, y, width, height): + self.frames.append((x, y, width, height)) + self.events.append("frame") + + async def clear_frame(self, page): + self.clears += 1 + self.events.append("clear_frame") + + class FakeRecorder: #: The row a `select` reports to its `on_revealed` hook; `None` stands for #: `mode: native`, which unfurls nothing and so has no row to mark. @@ -84,6 +106,7 @@ class FakeRecorder: def __init__(self, events: list[str] | None = None): self.frame = object() + self.overlay = FakeOverlay(events) self.events = events if events is not None else [] self.wait_for_calls: list[tuple] = [] self.wait_seconds_calls: list[float] = [] diff --git a/tests/unit/guide/test_annotate.py b/tests/unit/guide/test_annotate.py index 15ebcec..21c5040 100644 --- a/tests/unit/guide/test_annotate.py +++ b/tests/unit/guide/test_annotate.py @@ -7,6 +7,7 @@ from guidebot_recorder.guide.annotate import ( CLICK_INNER, CLICK_OUTER, + FRAMED_ACTIONS, annotations_for, cursor_shape, target_shape, @@ -33,10 +34,19 @@ def _kinds(anns): return [a.kind for a in anns] -def test_click_frames_the_target_and_stars_the_cursor(): +def test_click_stars_the_cursor_and_leaves_the_frame_to_the_page(): + """Ramka nie jest adnotacją — maluje ją strona, pod kursorem. + + Adnotacje to warstwa SVG nad gotowym PNG-iem, więc ramka narysowana tutaj + zawsze przykryłaby kursor. Geometrię nadal wyznacza `target_shape`, tyle że + konsumuje ją `stills._framed_screenshot`, a nie `layout`. + """ + anns = annotations_for("click", prev_cursor=(5.0, 5.0), center=CENTER, box=BOX) - assert set(_kinds(anns)) == {"arrow", "frame", "click"} + assert set(_kinds(anns)) == {"arrow", "click"} + assert "click" in FRAMED_ACTIONS + assert target_shape("click", box=BOX) == Rect(x=10.0, y=20.0, w=100.0, h=40.0) star = next(a for a in anns if a.kind == "click") assert (star.cx, star.cy) == CENTER assert (star.r_inner, star.r_outer) == (CLICK_INNER, CLICK_OUTER) @@ -45,16 +55,18 @@ def test_click_frames_the_target_and_stars_the_cursor(): def test_no_arrow_without_prev_cursor(): anns = annotations_for("click", prev_cursor=None, center=CENTER, box=BOX) - assert _kinds(anns) == ["frame", "click"] + assert _kinds(anns) == ["click"] @pytest.mark.parametrize("action", ["type", "hover", "select"]) def test_every_targeted_action_frames_its_box(action): + """Ramka zniknęła z adnotacji, ale nie z przewodnika — przeniosła się do strony.""" + anns = annotations_for(action, prev_cursor=None, center=CENTER, box=BOX) - assert _kinds(anns) == ["frame"] - frame = anns[0] - assert (frame.x, frame.y, frame.w, frame.h) == (10.0, 20.0, 100.0, 40.0) + assert _kinds(anns) == [] + assert action in FRAMED_ACTIONS + assert target_shape(action, box=BOX) == Rect(x=10.0, y=20.0, w=100.0, h=40.0) def test_missing_box_omits_rect_marks(): @@ -81,9 +93,8 @@ def test_select_without_a_row_is_marked_like_any_other_framed_action(): anns = annotations_for("select", prev_cursor=(5.0, 5.0), center=CENTER, box=BOX) - assert set(_kinds(anns)) == {"arrow", "frame"} - frame = next(a for a in anns if a.kind == "frame") - assert (frame.x, frame.y, frame.w, frame.h) == (10.0, 20.0, 100.0, 40.0) + assert set(_kinds(anns)) == {"arrow"} + assert target_shape("select", box=BOX) == Rect(x=10.0, y=20.0, w=100.0, h=40.0) def test_select_with_an_open_list_stars_the_row_and_frames_the_control(): @@ -91,13 +102,13 @@ def test_select_with_an_open_list_stars_the_row_and_frames_the_control(): "select", prev_cursor=(5.0, 5.0), center=CENTER, box=BOX, row_box=ROW, row_center=ROW_CENTER ) - assert set(_kinds(anns)) == {"arrow", "frame", "click"} + assert set(_kinds(anns)) == {"arrow", "click"} # the star marks the option about to be clicked... star = next(a for a in anns if a.kind == "click") assert (star.cx, star.cy) == ROW_CENTER - # ...the frame stays on the control, so the field is still legible... - frame = next(a for a in anns if a.kind == "frame") - assert (frame.x, frame.y, frame.w, frame.h) == (10.0, 20.0, 100.0, 40.0) + # ...the frame stays on the control, so the field is still legible (drawn in + # the page, so it is `target_shape` and not an annotation that says where)... + assert target_shape("select", box=BOX) == Rect(x=10.0, y=20.0, w=100.0, h=40.0) # ...and the arrow stops at the *row's* rim, not the control's: coming from # the top-left it enters through the row's top edge. arrow = next(a for a in anns if a.kind == "arrow") diff --git a/tests/unit/guide/test_capture_popup.py b/tests/unit/guide/test_capture_popup.py index 0f9fdc3..5cac35d 100644 --- a/tests/unit/guide/test_capture_popup.py +++ b/tests/unit/guide/test_capture_popup.py @@ -309,6 +309,40 @@ async def point(self, target, ripple=False): assert paused == [popup] +async def test_a_click_paints_its_frame_into_the_page_around_the_shutter(tmp_path, monkeypatch): + """Ramka celu powstaje w stronie, a nie w SVG — i znika zaraz po migawce. + + Kolejność jest tu całą treścią: pomaluj, zrób zdjęcie, posprzątaj. Ramka, + która przetrwałaby migawkę, obrysowałaby zły element na każdym kolejnym + zrzucie; ramka rysowana po migawce (jako SVG) przykryłaby kursor. + """ + + monkeypatch.setattr(capture, "reuse_failure", _async_none) + events: list[str] = [] + main, _popup = _windows(events) + recorder = FakeRecorder(events) + scenario = Scenario(config=_cfg(), steps=[Step(click="przycisk zapisu")]) + + pages = await capture_pages( + scenario, + _compiled([_click()]), + main, + recorder, + tmp_path / "shots", + timeout=15.0, + ) + + assert [e for e in events if e in ("frame", "shot:main", "clear_frame")] == [ + "frame", + "shot:main", + "clear_frame", + ] + # The box `Recorder.point` reported, outlined verbatim. + assert recorder.overlay.frames == [(0.0, 0.0, 10.0, 10.0)] + # And nothing about it leaks into the SVG layer. + assert "frame" not in {a.kind for a in pages[0].annotations} + + async def test_close_window_without_a_popup_is_a_no_op(tmp_path): """`closeWindow` w scenariuszu bez popupu nie ma czego zamykać. diff --git a/tests/unit/guide/test_capture_select.py b/tests/unit/guide/test_capture_select.py index 94682be..5c0db20 100644 --- a/tests/unit/guide/test_capture_select.py +++ b/tests/unit/guide/test_capture_select.py @@ -63,7 +63,9 @@ async def test_select_step_is_photographed_while_its_list_is_open(tmp_path, monk pages = await capture_pages( scenario, _compiled([action]), page, recorder, tmp_path / "shots", timeout=15.0 ) - assert events == ["open", "screenshot", "select:Zakres lat"] + # The outline goes up and comes down around the shutter, and the whole + # bracket sits between opening the list and choosing from it. + assert events == ["open", "frame", "screenshot", "clear_frame", "select:Zakres lat"] assert len(pages) == 1 assert pages[0].screenshot is not None @@ -71,25 +73,32 @@ async def test_select_step_is_photographed_while_its_list_is_open(tmp_path, monk async def test_select_marks_the_option_row_and_frames_the_control(tmp_path, monkeypatch): monkeypatch.setattr(capture, "reuse_failure", _async_none) scenario, action = _select_scenario_and_action() + recorder = FakeRecorder() pages = await capture_pages( scenario, _compiled([action]), FakePage(), - FakeRecorder(), + recorder, tmp_path / "shots", timeout=15.0, ) annotations = {a.kind: a for a in pages[0].annotations} - assert set(annotations) == {"frame", "click"} - # The frame is the control the reader is in — NOT the row, and not the box - # the cursor approach measured, which by frame time is stale. - rect = annotations["frame"] - assert (rect.x, rect.y, rect.w, rect.h) == ( - FAKE_CONTROL["x"], - FAKE_CONTROL["y"], - FAKE_CONTROL["width"], - FAKE_CONTROL["height"], - ) + assert set(annotations) == {"click"} + # The frame is painted into the page, under the cursor, so it is the overlay + # — not the annotation list — that records it. It outlines the control the + # reader is in: NOT the row, and not the box the cursor approach measured, + # which by frame time is stale. + assert recorder.overlay.frames == [ + ( + FAKE_CONTROL["x"], + FAKE_CONTROL["y"], + FAKE_CONTROL["width"], + FAKE_CONTROL["height"], + ) + ] + # ...and it does not survive the shutter: left standing it would mark the + # wrong element in every screenshot that followed. + assert recorder.overlay.clears == 1 assert (annotations["click"].cx, annotations["click"].cy) == FAKE_ROW_CENTER @@ -138,7 +147,10 @@ async def test_native_mode_keeps_the_collapsed_frame_and_its_single_mark(tmp_pat scenario, _compiled([action]), FakePage(), recorder, tmp_path / "shots", timeout=15.0 ) assert [call[2] for call in recorder.select_calls] == [True] # `native=True` - assert {a.kind for a in pages[0].annotations} == {"frame"} + assert {a.kind for a in pages[0].annotations} == set() + # Nothing visible is clicked under `native`, so the frame is the only mark — + # and it lives in the page now. + assert len(recorder.overlay.frames) == 1 async def test_the_still_capture_asks_for_no_click_ring(tmp_path, monkeypatch): diff --git a/tests/unit/guide/test_layout.py b/tests/unit/guide/test_layout.py index 25ba3c3..df76057 100644 --- a/tests/unit/guide/test_layout.py +++ b/tests/unit/guide/test_layout.py @@ -49,21 +49,27 @@ def test_click_star_arms_run_from_inner_to_outer_radius(): assert '' in html -def test_frame_annotation_renders_rounded_rect(): - pages = [_shot_page([Annotation(kind="frame", x=10.0, y=20.0, w=300.0, h=40.0)])] - html = render_html(pages, title="x") - assert '' in html - assert '`/`` with no `stroke` is invisible: the arms and frames get - # their red stroke only from the `.star` / `.frame` CSS rules. Rename either - # class in the stylesheet and every PDF silently loses its stars or frames, - # with the whole suite still green — so pin the rules by name here. +def test_stylesheet_defines_the_star_rule(): + # A `` with no `stroke` is invisible: the star's arms get their red + # stroke only from the `.star` CSS rule. Rename the class in the stylesheet + # and every PDF silently loses its stars with the whole suite still green — + # so pin the rule by name here. html = render_html([_shot_page([])], title="x") assert ".star {" in html - assert ".frame {" in html def test_a_select_steps_marks_render_through_the_one_shared_arrowhead(): @@ -81,7 +87,6 @@ def test_a_select_steps_marks_render_through_the_one_shared_arrowhead(): _shot_page( [ Annotation(kind="arrow", x1=10.0, y1=10.0, x2=200.0, y2=300.0), - Annotation(kind="frame", x=10.0, y=20.0, w=300.0, h=40.0), Annotation(kind="click", cx=100.0, cy=200.0, r_inner=16.0, r_outer=30.0), ] ) @@ -152,7 +157,7 @@ def test_annotations_stay_in_the_stills_own_coordinate_system(): screenshot=Path("/tmp/popup.png"), text="t", heading=None, - annotations=[Annotation(kind="frame", x=10.0, y=20.0, w=300.0, h=40.0)], + annotations=[Annotation(kind="click", cx=100.0, cy=200.0, r_inner=16.0, r_outer=30.0)], screenshot_size=(600, 700), canvas_size=(1376, 800), ) From 3c1245df2be08f4eaf7b5a6a4b4e5d8e5e76b052 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sat, 25 Jul 2026 09:39:33 +0200 Subject: [PATCH 2/3] =?UTF-8?q?refactor(guide):=20domknij=20ramk=C4=99=20p?= =?UTF-8?q?o=20self-review=20=E2=80=94=20testy=20w=20przegl=C4=85darce,=20?= =?UTF-8?q?bez=20cichych=20domy=C5=9Blnych?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trzy rzeczy, które wyszły przy przeglądzie własnej zmiany: 1. `cursor_effects.js` nie miał żadnego pokrycia w przeglądarce, mimo że testy overlay jeżdżą po prawdziwym Chromium. Zepsuty z-index przeszedłby przez wszystkie 184 testy — jedyną weryfikacją były oględziny zrzutu. Dochodzi pięć testów, w tym ten pilnujący jedynej własności, o którą w tej zmianie chodzi: ramka MUSI mieć niższy z-index niż kursor. Porównanie jest relacyjne, nie dosłowną stałą. Sprawdzone mutacją: podniesienie z-indeksu do maksimum i usunięcie rozsunięcia o pół obrysu — oba warianty czerwienią się. 2. `getattr(cap.recorder, "overlay", None)` maskowałoby zmianę nazwy atrybutu: ramka zniknęłaby ze wszystkich zrzutów, a testy zostałyby zielone. Każdy Recorder — prawdziwy i każdy dubler — ten atrybut ma, więc domyślna wartość była nieosiągalna. Zwykły dostęp, głośny AttributeError. 3. `frame()` przyjmowało `options`, których nikt nie podaje — cztery gałęzie bez pokrycia. Wartości wracają do stałych (tych samych, co dawna reguła `.frame`), a grubość obrysu i z-index dostają nazwane stałe z uzasadnieniem. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PEZwp8YivARFrZSjrSUnD5 --- guidebot_recorder/guide/replay.py | 15 ++-- guidebot_recorder/guide/stills.py | 7 +- guidebot_recorder/overlay/cursor_effects.js | 33 ++++++--- tests/unit/overlay/test_cursor_js.py | 82 +++++++++++++++++++++ 4 files changed, 117 insertions(+), 20 deletions(-) diff --git a/guidebot_recorder/guide/replay.py b/guidebot_recorder/guide/replay.py index b986a1d..9196643 100644 --- a/guidebot_recorder/guide/replay.py +++ b/guidebot_recorder/guide/replay.py @@ -370,9 +370,12 @@ def _frame_rect(run: _StepRun) -> dict | None: async def _still(run: _StepRun) -> tuple[Path, tuple[int, int]]: """This step's screenshot, with its target outlined *beneath* the cursor. - ``overlay`` is read off the recorder rather than carried on the capture: the - recorder already owns it, and a stand-in recorder without one simply gets an - unframed still instead of an attribute error. + 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 @@ -380,7 +383,7 @@ async def _still(run: _StepRun) -> tuple[Path, tuple[int, int]]: cap.page, cap.shots_dir, run.index, - overlay=getattr(cap.recorder, "overlay", None), + overlay=cap.recorder.overlay, rect=_frame_rect(run), ) @@ -436,9 +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, overlay=getattr(cap.recorder, "overlay", None) - ) + 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. diff --git a/guidebot_recorder/guide/stills.py b/guidebot_recorder/guide/stills.py index 7e567cf..8943428 100644 --- a/guidebot_recorder/guide/stills.py +++ b/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 @@ -35,7 +36,7 @@ async def _framed_screenshot( shots_dir: Path, index: int, *, - overlay, + overlay: Overlay | None, rect: dict | None, ) -> tuple[Path, tuple[int, int]]: """Photograph the page with ``rect`` outlined in it, then take the outline down. @@ -82,7 +83,9 @@ class _OpenListFrame: screen. """ - def __init__(self, page: Page, shots_dir: Path, index: int, *, overlay=None) -> 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 diff --git a/guidebot_recorder/overlay/cursor_effects.js b/guidebot_recorder/overlay/cursor_effects.js index 4ddf4ad..1bca351 100644 --- a/guidebot_recorder/overlay/cursor_effects.js +++ b/guidebot_recorder/overlay/cursor_effects.js @@ -293,6 +293,16 @@ 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. * @@ -310,8 +320,12 @@ * 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, options = {}) { + 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"); @@ -325,20 +339,17 @@ // earlier step would be photographed as if it belonged to this one. clearFrame(); - const width_ = Number.isFinite(Number(options.borderWidth)) - ? Number(options.borderWidth) - : 4; - const half = width_ / 2; + const half = FRAME_BORDER / 2; const box = document.createElement("div"); box.setAttribute("data-guidebot-frame", ""); - styleTransient(box, "2147483645"); + styleTransient(box, FRAME_Z_INDEX); setImportant(box, "left", `${values[0] - half}px`); setImportant(box, "top", `${values[1] - half}px`); - setImportant(box, "width", `${values[2] + width_}px`); - setImportant(box, "height", `${values[3] + width_}px`); - setImportant(box, "border", `${width_}px solid ${options.color || "#e11"}`); - setImportant(box, "border-radius", `${options.radius ?? 4}px`); - setImportant(box, "background", options.fill || "rgba(238, 17, 17, .08)"); + 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; } diff --git a/tests/unit/overlay/test_cursor_js.py b/tests/unit/overlay/test_cursor_js.py index d1d9d8e..1db5d4f 100644 --- a/tests/unit/overlay/test_cursor_js.py +++ b/tests/unit/overlay/test_cursor_js.py @@ -222,3 +222,85 @@ async def test_hidden_flag_survives_ensure(page: Page) -> None: "getComputedStyle(document.querySelector('[data-guidebot-cursor]')).display" ) assert disp2 == "block" + + +# --- the target outline, and the one property that is the whole point --------- + + +async def test_frame_is_stacked_below_the_cursor(page: Page) -> None: + """Ramka musi mieć NIŻSZY z-index niż kursor. To jest cała ta funkcja. + + Ramka przeniosła się z SVG (rysowanego nad gotowym PNG) do strony właśnie po + to, żeby kursor został na wierzchu — zrzut nie ma warstw, więc później już + nic się pod niego nie wsunie. Gdyby ktoś podniósł ten z-index do maksimum, + obie warstwy zrównałyby się i o wyniku decydowałaby kolejność dopisania do + DOM — czyli błąd wracałby losowo. Porównanie liczbowe, nie dosłowna wartość: + test ma pilnować relacji, a nie utrwalać stałej. + """ + + await page.set_content("
") + await _inject(page, {}) + z = await page.evaluate( + "() => { window.__guidebot_cursor.frame(10, 20, 100, 40);" + " const zi = (sel) => Number(getComputedStyle(document.querySelector(sel)).zIndex);" + " return [zi('[data-guidebot-frame]'), zi('[data-guidebot-cursor]')]; }" + ) + assert z[0] < z[1], f"ramka {z[0]} musi być pod kursorem {z[1]}" + + +async def test_frame_straddles_the_targets_edge_like_the_svg_stroke_it_replaced( + page: Page, +) -> None: + """Obrys ma leżeć okrakiem na krawędzi celu, tak jak robił to `stroke` w SVG. + + CSS rysuje `border` do wewnątrz, a `stroke` w SVG centruje na ścieżce — więc + bez rozsunięcia pudełka o połowę grubości ramka byłaby ciaśniejsza niż przed + przenosinami. Sprawdzamy prostokąt zewnętrzny: 4 px obrysu ma wystawać po + 2 px na stronę. + """ + + await page.set_content("
") + await _inject(page, {}) + rect = await page.evaluate( + "() => { window.__guidebot_cursor.frame(10, 20, 100, 40);" + " const r = document.querySelector('[data-guidebot-frame]').getBoundingClientRect();" + " return [r.x, r.y, r.width, r.height]; }" + ) + assert rect == [8, 18, 104, 44] + + +async def test_a_second_frame_replaces_the_first(page: Page) -> None: + """Jeden krok obrysowuje jeden cel — resztka po poprzednim kłamałaby na zdjęciu.""" + + await page.set_content("
") + await _inject(page, {}) + count = await page.evaluate( + "() => { window.__guidebot_cursor.frame(0, 0, 10, 10);" + " window.__guidebot_cursor.frame(50, 50, 20, 20);" + " return document.querySelectorAll('[data-guidebot-frame]').length; }" + ) + assert count == 1 + + +async def test_clear_frame_takes_the_outline_down_and_tolerates_no_frame(page: Page) -> None: + """Sprzątanie musi być idempotentne: `_framed_screenshot` woła je z `finally`.""" + + await page.set_content("
") + await _inject(page, {}) + left = await page.evaluate( + "() => { window.__guidebot_cursor.frame(10, 20, 100, 40);" + " window.__guidebot_cursor.clearFrame();" + " window.__guidebot_cursor.clearFrame();" + " return document.querySelectorAll('[data-guidebot-frame]').length; }" + ) + assert left == 0 + + +async def test_frame_rejects_bounds_that_would_silently_draw_nothing(page: Page) -> None: + """Ujemny rozmiar albo NaN to błąd wywołania, nie niewidzialny prostokąt.""" + + await page.set_content("
") + await _inject(page, {}) + for args in ("10, 20, -1, 40", "NaN, 0, 10, 10"): + with pytest.raises(Exception, match="frame bounds"): + await page.evaluate(f"() => window.__guidebot_cursor.frame({args})") From f515611a4dc19278434dcb3c6206603c3397d066 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pasternak?= Date: Sat, 25 Jul 2026 10:15:37 +0200 Subject: [PATCH 3/3] test(guide): przepnij testy integracyjne ramki na malowanie w stronie MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI złapało to, czego nie złapał mój self-review: przejrzałem tylko `tests/unit/`, a pięć testów integracyjnych asertowało na adnotacji `kind="frame"`, usuniętej razem z rysowaniem ramki w SVG. Te testy mierzą realną geometrię w prawdziwej przeglądarce — czy obrysowane pudełko to faktycznie pudełko kontrolki — więc skasowanie ich byłoby utratą pokrycia dokładnie tam, gdzie jest najcenniejsze. Zamiast tego `_frame_spy.py` nagrywa wywołania `Overlay.frame`, a asercje zachowują sens jeden do jednego: `_only_frame(painted)` w miejsce `_only(annotations, "frame")`. Szpieg woła oryginał dalej. Kilka z tych testów czyta potem piksele ze zrzutu, więc połknięcie wywołania po cichu zmieniłoby obraz, który badają. Pełny `pytest -m "not network"` lokalnie: 1604 passed, 1 skipped. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PEZwp8YivARFrZSjrSUnD5 --- tests/integration/_frame_spy.py | 55 +++++++++++++++++++ tests/integration/test_guide.py | 10 +++- tests/integration/test_guide_select_reveal.py | 29 ++++++++-- 3 files changed, 86 insertions(+), 8 deletions(-) create mode 100644 tests/integration/_frame_spy.py diff --git a/tests/integration/_frame_spy.py b/tests/integration/_frame_spy.py new file mode 100644 index 0000000..63c77b9 --- /dev/null +++ b/tests/integration/_frame_spy.py @@ -0,0 +1,55 @@ +"""Recording the outlines a guide run paints into the page. + +The target outline used to be an :class:`Annotation`, so an integration test +could read it straight off the finished ``GuidePage``. It is now drawn into the +live page before the shutter — that is the only way to get it *beneath* the +cursor, since a screenshot has no layers — and so it leaves no trace in the +returned pages at all. + +These tests still have to check the same thing they always did: that the box the +guide outlines is the real control's box, measured in a real browser. So instead +of reading an annotation they record the calls, and assert on those. + +The spy **calls through** to the real :meth:`Overlay.frame`. The outline must +still be painted: several of these tests go on to read pixels out of the +screenshot, and a spy that swallowed the call would quietly change the picture +they inspect. + +Explicitly imported, like every other helper here — ``tests/`` has no +``conftest.py`` by design (decision D4). +""" + +from __future__ import annotations + +from dataclasses import dataclass + +from guidebot_recorder.overlay.overlay import Overlay + + +@dataclass(frozen=True, slots=True) +class FramedBox: + """One outline the run painted, in viewport pixels.""" + + x: float + y: float + w: float + h: float + + +def record_frames(monkeypatch) -> list[FramedBox]: + """Patch :meth:`Overlay.frame` to append to the returned list, then call through. + + Patched on the class rather than on an instance because the run under test + builds its own ``Overlay`` (sometimes more than one — a popup gets its own + ``Recorder``), and the test never holds a reference to it. + """ + + painted: list[FramedBox] = [] + original = Overlay.frame + + async def spy(self, page, x, y, width, height): + painted.append(FramedBox(x, y, width, height)) + await original(self, page, x, y, width, height) + + monkeypatch.setattr(Overlay, "frame", spy) + return painted diff --git a/tests/integration/test_guide.py b/tests/integration/test_guide.py index 05c17e1..db36595 100644 --- a/tests/integration/test_guide.py +++ b/tests/integration/test_guide.py @@ -16,6 +16,8 @@ from guidebot_recorder.recorder.compile import run_compile, run_compile_in_browser from guidebot_recorder.resolver.reasoner import ReasonerResult +from ._frame_spy import record_frames + pytestmark = pytest.mark.integration FIXTURE = Path(__file__).parent / "fixtures" / "app.html" @@ -288,7 +290,7 @@ async def test_guide_select_actually_executes_and_unlocks_next_step(tmp_path): assert count == 3 -async def test_capture_pages_executes_select_and_scroll_on_the_live_page(tmp_path): +async def test_capture_pages_executes_select_and_scroll_on_the_live_page(tmp_path, monkeypatch): """Drive `capture_pages` directly against a live `Page` (bypassing `run_guide`, which only returns a page count and closes the context, so it cannot show any live-DOM effect). @@ -322,6 +324,8 @@ async def test_capture_pages_executes_select_and_scroll_on_the_live_page(tmp_pat path = tmp_path / "select-scroll-direct.scenario.yaml" path.write_text(DIRECT_CAPTURE_SCENARIO_TEMPLATE.format(url=url), encoding="utf-8") + painted = record_frames(monkeypatch) + async with async_playwright() as pw: browser = await pw.chromium.launch(headless=True) try: @@ -371,7 +375,9 @@ async def test_capture_pages_executes_select_and_scroll_on_the_live_page(tmp_pat select_page = pages[1] assert select_page.kind == "step" assert select_page.screenshot is not None - assert any(annotation.kind == "frame" for annotation in select_page.annotations) + # The outline is painted into the page (so the cursor stays on top of it), + # not returned as an annotation — so the spy is where it now shows up. + assert len(painted) == 1 async def test_option_that_vanished_after_compile_fails_as_a_sentence_not_a_timeout(tmp_path): diff --git a/tests/integration/test_guide_select_reveal.py b/tests/integration/test_guide_select_reveal.py index 9bb3047..7bb7176 100644 --- a/tests/integration/test_guide_select_reveal.py +++ b/tests/integration/test_guide_select_reveal.py @@ -30,6 +30,8 @@ from guidebot_recorder.scenario.loader import load_scenario from guidebot_recorder.selects import install_selects +from ._frame_spy import FramedBox, record_frames + pytestmark = pytest.mark.integration FIXTURE = Path(__file__).parent / "fixtures" / "guide-select.html" @@ -258,6 +260,13 @@ def _only(annotations: list[Annotation], kind: str) -> Annotation: return matching[0] +def _only_frame(painted: list[FramedBox]) -> FramedBox: + """The one outline the run painted — same shape of assertion as :func:`_only`.""" + + assert len(painted) == 1, f"expected exactly one painted frame, got {painted}" + return painted[0] + + async def _measure_open_list(browser: Browser, path: Path) -> tuple[dict, dict, Path]: """Measure the fixture independently of the run under test. @@ -364,6 +373,7 @@ async def test_the_select_page_frames_the_control_and_points_the_arrow_at_the_ro browser = await playwright.chromium.launch(headless=True) try: await run_compile_in_browser(path, browser, SelectReasoner()) + painted = record_frames(monkeypatch) pages = await _guide_with_pages(path, tmp_path / "marks.pdf", browser, monkeypatch) row, _backdrop, _closed = await _measure_open_list(browser, path) context = await browser.new_context( @@ -378,7 +388,9 @@ async def test_the_select_page_frames_the_control_and_points_the_arrow_at_the_ro await browser.close() assert control is not None - framed = _only(pages[1].annotations, "frame") + # The outline is painted into the page, beneath the cursor, so it is the spy + # and not the annotation list that says which box got marked. + framed = _only_frame(painted) assert (framed.x, framed.y) == pytest.approx((control["x"], control["y"]), abs=1.0) assert (framed.w, framed.h) == pytest.approx((control["width"], control["height"]), abs=1.0) # The arrow starts at the previous cursor position, which the navigate step @@ -413,6 +425,7 @@ async def test_a_display_none_widget_is_driven_instead_of_timing_out( browser = await playwright.chromium.launch(headless=True) try: await run_compile_in_browser(path, browser, SelectReasoner()) + painted = record_frames(monkeypatch) pages = await _guide_with_pages(path, tmp_path / "enhanced.pdf", browser, monkeypatch) finally: await browser.close() @@ -424,8 +437,8 @@ async def test_a_display_none_widget_is_driven_instead_of_timing_out( assert click.cx is not None and click.cy is not None # The widget stands where the hidden original is, so the framed control must # be the widget's box — a `display: none` element has no box to frame. - framed = _only(pages[1].annotations, "frame") - assert framed.w is not None and framed.w > 8 + framed = _only_frame(painted) + assert framed.w > 8 async def test_native_mode_keeps_todays_collapsed_frame( @@ -439,6 +452,7 @@ async def test_native_mode_keeps_todays_collapsed_frame( browser = await playwright.chromium.launch(headless=True) try: await run_compile_in_browser(path, browser, SelectReasoner()) + painted = record_frames(monkeypatch) pages = await _guide_with_pages(path, tmp_path / "native.pdf", browser, monkeypatch) _row, _backdrop, closed = await _measure_open_list(browser, path) finally: @@ -447,8 +461,10 @@ async def test_native_mode_keeps_todays_collapsed_frame( select_page = pages[1] assert select_page.screenshot is not None # Marked like any other framed action: the control is framed and nothing is - # starred, because under `native` nothing visible is clicked. - assert {a.kind for a in select_page.annotations} == {"frame"} + # starred, because under `native` nothing visible is clicked. The frame lives + # in the page now, so the annotation list is empty and the spy holds the mark. + assert {a.kind for a in select_page.annotations} == set() + assert len(painted) == 1 # And nothing was unfurled over the backdrop. _width, _height, rows = _read_png(select_page.screenshot) reference = _read_png(closed)[2] @@ -475,12 +491,13 @@ async def test_the_marks_are_page_coordinates_when_the_site_runs_in_the_chrome_s browser = await playwright.chromium.launch(headless=True) try: await run_compile_in_browser(path, browser, SelectReasoner()) + painted = record_frames(monkeypatch) pages = await _guide_with_pages(path, tmp_path / "chrome.pdf", browser, monkeypatch) finally: await browser.close() click = _only(pages[1].annotations, "click") - framed = _only(pages[1].annotations, "frame") + framed = _only_frame(painted) # The control sits at the very top of the fixture, so an un-offset mark would # land above the bar; both marks must be below it and inside the viewport. assert framed.y > CHROME_HEIGHT