Compare commits

...

3 Commits

Author SHA1 Message Date
mAi
e53f4b1a5e docs(debug): sherlock frame-select — not a regression, render-order clutter
Playwright drives the deployed image four ways. The frame <rect> has its
own pointerdown handler that calls startDrag; startDrag calls
e.stopPropagation() before the pan-start handler runs; the selector at
main.js:2087 already includes [data-frame-id]. Click that lands on the
rect → frame selected, inspector switches to the Frame panel. Verified.

m's complaint is real but cause is render order: frames paint first;
devices, ports, cables, clamps, IO markers all paint on top. Any click
on the frame interior that happens to land on one of those elements
hits that element, not the frame. For LOFT's big 'Entertainment' frame
the visible canvas portion is ~180×424 px and partly covered by yellow
cable polylines.

Not a one-line bug. Three UX options ordered by effort in the doc:
make the frame label the selection grip (drop pointer-events:none + add
pointerdown), parent-frame breadcrumb on the device inspector, modifier
key to escape to the enclosing frame. None applied — picasso/perseus
call.
2026-05-16 19:30:08 +02:00
mAi
1f246c0047 docs(debug): sherlock +Port "does nothing" root cause + fix
Playwright drives the live deployment through three repro scenarios
(empty canvas / on device / on existing port). POST /api/.../ports
returns 201 every time and a new circle appears in the DOM — +Port is
not a no-op. But snapToDeviceEdge collapses the click region down to a
handful of edge-midpoint coordinates with no de-dup, so new ports stack
pixel-perfect on existing ones. Plus placePortAt is the only "place"
function that doesn't set state.selection on the new entity, so the
inspector stays on the device panel and the new port gets no .selected
halo — the canvas + panel both look unchanged from m's view.

Primary fix (one line): set state.selection = {kind: "port", id} in
placePortAt before render(). Verified live by intercepting the POST and
dispatching a pointerdown on the new circle — inspector switches to the
PORT panel with edge picker and the port gets its halo.

Secondary: snapToDeviceEdge could walk along the chosen edge when the
calculated position is already occupied (~8px tolerance). Optional.

Unrelated bug surfaced: startDrag.onUp does e.currentTarget.classList...
on a recycled async event. Spams "Cannot read properties of null" in
the console on every click-only device select. Trivial closure-capture
fix included in the doc.
2026-05-16 11:09:09 +02:00
mAi
3a43762e8c docs(debug): sherlock +Dev click root cause + one-line fix
Playwright repro confirms +Dev → click inside frame or empty canvas drops
silently. Root cause: input.focus() inside promptInline runs during the
pointerdown handler, then the browser's default mousedown action blurs
the input because the SVG click target isn't focusable. The input's blur
listener calls done(null) → fo.remove() before m can type.

Fix verified live by monkey-patching the deployed page: e.preventDefault()
on the canvas pointerdown when a tool is armed keeps the FO focused.

Diff sketch + trace + secondary cleanup tip in the doc. Picasso to apply.
2026-05-15 23:14:52 +02:00
3 changed files with 341 additions and 0 deletions

98
docs/sherlock-+dev-bug.md Normal file
View File

@@ -0,0 +1,98 @@
# +Dev click silently drops — root cause & fix
**Reporter:** sherlock (Playwright debug shift, 2026-05-15 ~23:10)
**Target:** http://mdock:7777 (deployed at e12b449)
**Verdict:** Bug reproduced; root cause identified; one-line fix verified live.
## Repro (Playwright, headless Chromium 1217)
1. GET `http://mdock:7777/?project=1` (LOFT). Snapshot returns one frame (`Entertainment`).
2. Click `#tool-device` — armed correctly (`.canvas-wrap.tool-device`, button `.armed`, cursor `crosshair` both over the empty SVG and over frame rects).
3. Click inside the frame (rect centre).
4. Expected: inline-namer `<foreignObject><input>` appears focused, m types name, Enter, `POST /api/projects/1/devices` fires, device renders.
5. Observed: no FO visible, no POST, tool disarms silently. Same failure when clicking on **empty** canvas (outside any frame).
## Trace (instrumented JS)
```
[782ms] svg.append foreignObject ← placeDeviceAt → promptInline appended it
[782ms] focusout BUTTON #tool-device
[783ms] focusin INPUT ← input.focus() succeeded
[783ms] svg-bubble-pd rect ← pointerdown still bubbling
[783ms] mutation.remove foreignObject ← FO ripped out
```
Patched `Element.prototype.remove` traces the offender:
```
at Element.remove (anon)
at done (http://mdock:7777/main.js:585:36)
at HTMLInputElement.<anonymous> (http://mdock:7777/main.js:592:42)
```
That's `input.addEventListener("blur", () => done(input.value.trim() || null))` in `promptInline`. The input was focused at 783 ms and blurred ~6 ms later, so `done(null)` removed the FO before m could type.
## Root cause
`placeDeviceAt` calls `promptInline` synchronously from inside the canvas `pointerdown` handler. `promptInline` does `input.focus()` synchronously. After the pointerdown handler returns, the browser performs the default mousedown action — focus the nearest focusable ancestor of the click target, else **blur the active element**. The click target is an SVG `<rect>` (or the `<svg>` root) which is not focusable, so the freshly-focused input gets blurred. The input's own `blur` listener fires `done(null)``fo.remove()`.
This is independent of the previous fix at 94869f3 (which corrected the ordering of `onCanvasPointerDown` so the tool branches run before the "click on existing element" early-return). Routing is now correct, which is what unmasks the focus-blur bug. `+ Frame` is not affected because there `promptInline` runs from the pointer**up** callback, after pointer events have finished, so no mousedown is pending to steal focus.
The cursor + Cache-Control fix at 28a376a addressed only the visual cursor lie and the redeploy staleness. It does not touch the click handler.
## Fix (one line)
`web/static/main.js` around line 472, in `onCanvasPointerDown`:
```diff
if (state.tool === "frame") {
+ e.preventDefault();
startFrameRubberBand(e, p);
return;
}
if (state.tool === "device") {
+ e.preventDefault();
placeDeviceAt(p);
return;
}
```
`e.preventDefault()` on the pointerdown suppresses the compatibility mousedown's default focus-shift, so the input keeps focus and m can type. The `+ Frame` branch gets the same treatment for symmetry and to prevent a subtle text-selection side effect during rubber-band drag (it's not strictly required for the focus issue there because `+ Frame` focuses on pointerup).
Per-call site rationale, not a blanket `if (state.tool) e.preventDefault()` at the top, because future tools that intentionally rely on default click behavior (e.g. a cable-drawing tool that needs link-clicks on ports) might want different semantics.
### Verified live
Monkey-patched the deployed page with a capture-phase listener that calls `e.preventDefault()` while `#tool-device.armed` exists. Result: device created, named `pc-sherlock`, persisted (POST `/api/projects/1/devices` 200). Screenshot: `/tmp/sherlock/fix_verified.png`.
## Secondary observation (not blocking)
After the fix, pressing `Enter` on the inline-namer logs a `pageerror`:
```
Failed to execute 'remove' on 'Element':
The node to be removed is no longer a child of this node.
Perhaps it was moved in a 'blur' event handler?
```
`done()` is called twice (once from Enter keydown, once from the resulting blur). The `if (activeNamer === fo)` guard on the second entry is recursive-safe because the first `done()` runs `fo.remove()` *before* setting `activeNamer = null`, so the synchronous blur fires inside the first remove() and re-enters with the guard still true. Cheap fix: reorder the two lines so the flag clears first:
```diff
const done = (val) => {
- if (activeNamer === fo) { fo.remove(); activeNamer = null; }
+ if (activeNamer === fo) { activeNamer = null; fo.remove(); }
resolve(val);
};
```
Functional impact: none — the device is still created. Cosmetic console error only. Worth folding into the same patch.
## Artifacts (sherlock side, /tmp)
- `repro.py` — initial reproduction, screenshots before/after click.
- `repro2.py` — event-trace instrumentation (capture+bubble pointerdowns, focus, mutations).
- `repro3.py``remove()` stack-trace capture that named `done() <- blur listener`.
- `verify_fix.py` — applied the proposed fix in-page; device created end-to-end.
- `test_empty.py` — confirmed the bug bites on empty canvas too, not just inside frames.
- `run.log`, `run2.log`, `run3.log`, `verify.log`, `empty.log` — full transcripts.
- `fix_verified.png` — post-fix screenshot with `pc-sherlock` device visible inside the frame.

144
docs/sherlock-+port-bug.md Normal file
View File

@@ -0,0 +1,144 @@
# +Port "still does nothing" — root cause & fix
**Reporter:** sherlock (Playwright debug shift 2, 2026-05-16 ~11:00)
**Target:** http://mdock:7777 (deployed image c361bf38, picasso fix 3276cfe)
**Verdict:** +Port is **not actually a no-op** — it successfully creates the port server-side AND client-side every time. The user-visible failure is **invisible stacking**: the new port renders at the exact same pixel as a pre-existing port on the same edge midpoint. m sees no canvas change and no panel switch, so the click feels dead.
## What I ran
Five Playwright drives against the live deployment, real Chromium (1217), full console + network + DOM mutation capture. Scripts and transcripts: `/tmp/sherlock/port_*`.
| Scenario | Click target | POST result | DOM change | What m sees |
|---|---|---|---|---|
| (a) empty canvas inside frame | (443, 626) screen | 201, port id 40 created | new circle at TV bottom-left | maybe a new dot, but device list still selected, no halo |
| (b) on the device body (TV center) | (563, 454) screen | 201, port id 41 | new circle at **(686.4, 643)** — same cx,cy as ports 37, 38, 39 already at that point | nothing — new dot stacks pixel-perfect under existing ones |
| (c) directly on an existing port circle | (538, 445) screen | 201, port id 42 | new circle at **(636.4, 643)** — same cx,cy as port 27 | nothing — stacked |
In every scenario:
- `state.tool === "port"` after the +Port click (inferred from `.canvas-wrap.tool-port` + cursor `crosshair`)
- `POST /api/projects/1/devices/2/ports` returns 201 with the new port row
- `state.ports.push(port)` happens, `render()` redraws, the new circle exists in the DOM
- The inspector port list grows by one row
- **The canvas does not visibly change.**
After 7 +Port placements on the TV, the inspector lists 7 ports, but only ~3 dots are visible on the device — the rest are stacked at identical (cx, cy):
```
ports on TV after the test run:
port 27 fill=#e03131 cx=636.4 cy=643.0 ← original
port 37 fill=#e03131 cx=686.4 cy=643.0
port 38 fill=#e03131 cx=686.4 cy=643.0 ← stacks on 37
port 39 fill=#1971c2 cx=686.4 cy=643.0 ← stacks on 37,38
port 40 fill=#e03131 cx=636.4 cy=678.0
port 41 fill=#e03131 cx=686.4 cy=643.0 ← stacks on 37,38,39
port 42 fill=#e03131 cx=636.4 cy=643.0 ← stacks on 27
```
## Why it stacks
`snapToDeviceEdge` (main.js:15601574) projects the click to the nearest of the four device edges and clamps the parallel coordinate to `[0, device.width]` (or `device.height`). For the dominant click region — anywhere over the device body — `snapToDeviceEdge` collapses a wide band of clicks down to a handful of discrete `(xOff, yOff)` tuples. For a 100×35 device, clicking anywhere in the bottom half + roughly the horizontal middle resolves to `(xOff=50, yOff=35)`. Two clicks in that band produce two ports at identical coordinates. There is no de-dup against existing ports.
This is geometry, not a regression — but it means the +Port tool has the same visible signature as a no-op tool for the most natural click region.
## Why m's panel feedback is also weak
`placePortAt` (main.js:16851706) finishes with `state.ports.push(port); armTool(null); render();`. It does **not** set `state.selection = { kind: "port", id: port.id }`. So:
- `state.selection` stays on the device (set in the +Port-arming click).
- The inspector continues to render the device panel; the port list grows by a row, but the panel doesn't switch.
- The new port circle does not get the `.selected` halo (drop-shadow) defined in style.css.
`placeDeviceAt`, `placeIOMarkerAt`, `startFrameRubberBand`, and `finishCableDrawAt` all set `state.selection` to the new entity. `placePortAt` is the odd one out.
## Proposed fix (verified live)
Two changes in `web/static/main.js`, one for stacking, one for feedback. The second is sufficient on its own to make the tool feel responsive.
### A. Select the newly-placed port (primary)
```diff
async function placePortAt(p) {
...
try {
const port = await createPort(state.active.id, did, {
type_id: tid,
x_offset: snap.xOff,
y_offset: snap.yOff,
});
state.ports.push(port);
+ state.selection = { kind: "port", id: port.id };
armTool(null);
render();
```
Effect verified live by intercepting the POST response and dispatching a pointerdown on the new port circle (simulates the selection that the fix would set internally). Result:
- Inspector immediately switches from the **DEVICE** panel to the **PORT** panel showing cable-type swatch, label input, edge dropdown (Top/Right/Bottom/Left), Delete button.
- The new port circle gets the `.selected` class → drop-shadow halo, visually distinct even when stacked.
- Screenshot: `/tmp/sherlock/port_fix_verified.png`.
m clicking +Port now produces unambiguous feedback (panel switch + halo), AND he can immediately move the new port to the right edge with the dropdown — exactly the workflow when you wanted "another port over there".
### B. De-dup snap position (secondary, optional)
If two ports on the same device land within ~8 px of each other on the same edge, walk along that edge in 12 px increments until a free slot is found (or distribute the existing ones evenly — pick the lighter change). This eliminates pixel-perfect stacks for power users; (A) alone covers the perception bug.
Sketch:
```js
function snapToDeviceEdge(device, x, y, existingPortsOnDevice) {
const raw = snapRaw(device, x, y); // current logic
if (!existingPortsOnDevice?.length) return raw;
const isOnEdge = (port, edge) =>
(edge === "left" && port.x_offset === 0) ||
(edge === "right" && port.x_offset === device.width) ||
(edge === "top" && port.y_offset === 0) ||
(edge === "bottom" && port.y_offset === device.height);
const sibs = existingPortsOnDevice.filter((p) => isOnEdge(p, raw.edge));
// Walk along the parallel axis until no sibling within 8px tolerance.
...
}
```
Then `placePortAt` passes `state.ports.filter((p) => p.device_id === did)`.
Not blocking — m's complaint is fixed by (A). File (B) as a polish ticket.
## Unrelated bug surfaced during repro (separate fix, separate scope)
Every click-only device selection throws this pageerror:
```
TypeError: Cannot read properties of null (reading 'classList')
at SVGSVGElement.onUp (http://mdock:7777/main.js:1846:21)
```
Source: `startDrag.onUp` does `e.currentTarget.classList.remove("dragging")` (main.js:1846). `e` is the closure-captured pointerdown event; by the time `onUp` runs (async, after pointerup), the browser has nulled out `e.currentTarget`. Click-only path aborts at line 1846 before the trailing `render()` (which is redundant here because `startDrag` already calls render at line 1794, so the user-visible state is fine — but the console error spams every click).
Trivial fix: capture the target into a closure before defining the async handlers.
```diff
function startDrag(e, kind, id) {
...
+ const dragTarget = /** @type {Element} */ (e.currentTarget);
...
- e.currentTarget.classList.add("dragging");
+ dragTarget.classList.add("dragging");
...
const onUp = async (ev) => {
...
- e.currentTarget.classList.remove("dragging");
+ dragTarget.classList.remove("dragging");
```
Not directly responsible for the +Port complaint, but it's noise in every console session and the fix is one line. Worth bundling with the port-select patch.
## Artifacts
Under `/tmp/sherlock/`:
- `port_repro.py`, `port_repro2.py`, `port_repro3.py` — staged reproductions
- `trace_pe.py` — pageerror stack capture
- `verify_port_fix.py` — applied the proposed selection-after-placement fix in-page
- `port_run.log`, `port_run2.log`, `port_run3.log`, `pe.log`, `verify_port.log` — full transcripts
- `port_after_redo.png`, `port_scenario_*.png` — pre-fix screenshots showing the inspector-list-grows-but-canvas-doesn't symptom
- `port_fix_verified.png` — post-fix screenshot, inspector switched to **PORT** panel with edge picker

View File

@@ -0,0 +1,99 @@
# "I cannot change frames" — repro & verdict
**Reporter:** sherlock (Playwright debug shift 3, 2026-05-16 ~19:30)
**Target:** http://mdock:7777 (CableGUI, deployed at 79e17a5)
**Hypothesis under test:** kandinsky's pan-on-left-drag (2933bb8) regressed frame select because the "isEmpty" selector missed `[data-frame-id]`.
**Verdict:** **Hypothesis is wrong.** The selector at main.js:2087 already lists `[data-frame-id]`, the frame `<rect>` has its own pointerdown handler that calls `startDrag`, and `startDrag` calls `e.stopPropagation()` synchronously before the pan-start handler could fire. Frame selection works mechanically on the deployed image — verified four ways. m's complaint is real, but it's a **render-order / clutter** problem, not a click-handler regression.
## What I ran (4 Playwright drives against the deployed image)
### Selection works on a click that lands on the frame rect
`frame_repro2.py` TEST 2 — Esc to deselect, then click 2 px inside frame 1's left edge:
```
before: selected_frames=[] inspector=None
after: selected_frames=['1'] inspector='Frame'
pointerdowns: svg-capture target=rect.frame-rect frame=1
pan changes: []
```
Frame 1's inspector panel appears with name="Entertainment", x/y/w/h editable, "Delete frame" button. No pan-start fired.
`frame_clean_test.py` — Esc, then click frame 3 (Network) bottom-left corner where nothing else is rendered:
```
top element at click point: rect.frame-rect (frame=3)
after click: selected_frames=['3'] inspector='Frame'
```
### Selection works on a click + small drag too
`frame_repro2.py` TEST 3 — 5 px drag (past the 3 px pan threshold) on frame interior:
- `startDrag(e, "frame", f.id)` fires at the frame rect's `pointerdown`. It is the **first** pointerdown handler in the chain.
- `startDrag` (main.js:2839) calls `e.stopPropagation()` synchronously when no tool is armed → pan-start handler never gets the event.
- The 5 px movement is handled by `startDrag`'s own `onMove` (frame drag), not by the empty-canvas pan path.
- Result: frame stays selected, frame nudges 5 px to the right. Inspector panel stays correct.
50 px drag (TEST 4): same — frame stays selected, moves further.
## Why m experiences "doesn't select"
The frame `<rect>` paints **first**; everything else paints on top of it in this order (from index.html):
```
canvas-frames ← frame rects (BOTTOM)
canvas-devices ← device rects + port circles inside each device <g>
canvas-ports
canvas-cables ← cable polylines (often crossing through frame interior)
canvas-clamps
canvas-io ← IO marker diamonds
```
So any click on the frame's visible interior that happens to land on a device, port, port circle, cable polyline, clamp, or IO diamond hits THAT element, not the frame. The frame rect only "wins" the hit-test on pixels that are not covered by anything else.
Two concrete observations from the LOFT snapshot:
- Frame 1 "Entertainment" is huge (SVG 1340×848 at viewBox 2000×1500). Most of it renders past the canvas-wrap's right edge (x > 1220 px on a 1500 px viewport) and is occluded by the inspector `<aside>`. Only a ~180 × 424 px strip on the visible canvas right side is selectable. That strip is partly covered by devices and yellow cable polylines crossing diagonally.
- Frame 3 "Network": grid hit-test, 9 sample points → 5 hit the frame, 2 hit cable polylines that cross through its centre. `frame_clean_test.py` confirmed: bottom-left corner click selects; centre click selects the cable.
For m, "I click on the frame and it doesn't select" is **literally true at the click point he's choosing** — but the click is going to the cable/device on top, not the frame underneath. There is no regression in the dispatch.
## Recommendation (no code change applied)
Since the click-handler chain is correct, this is a UX call for picasso/perseus to make, not a one-line patch sherlock should apply. Three options ordered by effort:
1. **Frame label is the selection grip.** Drop `pointer-events: none` from `.frame-label` (style.css:183) and add a pointerdown handler on the label that calls `startDrag(e, "frame", id)`. The label sits in the top-left of the frame, never overlaps a device, and is always visible — m gets a deterministic spot to click. This is a 2-line patch.
2. **Selection breadcrumb on the device inspector.** When a selected device has `frame_id`, render `parent frame: <name>` as a clickable chip that calls `state.selection = {kind: "frame", id: frame.id}; render()`. Lets m drill from "I selected the wrong thing" → frame in one click.
3. **Modifier-click to escape to frame.** Alt-click (or Ctrl-click) on any element promotes the hit-test to "select the enclosing frame, if any". Discoverable via tooltip on the inspector header.
Option (1) is the cheapest and matches user mental model ("click the label to select that thing"). Probably worth pairing with the empty-canvas pan UX so m can hold Shift to disable pan when he wants to clear selection — but that's a separate refinement.
## What the head's hypothesis *would* look like if it were the bug
For completeness — if `[data-frame-id]` were missing from the selector in `onCanvasPointerDown`:
```js
// (hypothetical broken version)
if (e.target instanceof Element && e.target.closest("[data-device-id], [data-io-id]")) return;
// ^^ no frame-id
if (e.button === 0 && state.cableDrawFromPortID == null) {
startEmptyCanvasGesture(e);
}
```
…then frame clicks would fall into `startEmptyCanvasGesture`. Sub-3 px clicks would still deselect (no good for m), and any tiny hand-jitter would promote to a pan. m would see "click frame → view shifts, no selection." That symptom matches m's wording — but the actual code is correct, so this isn't what's happening.
The current code (main.js:2087):
```js
if (e.target instanceof Element && e.target.closest("[data-device-id], [data-frame-id], [data-io-id], [data-clamp-id], [data-port-id], [data-cable-id]")) return;
```
All six known target types are listed. No regression here.
## Artifacts
Under `/tmp/sherlock/`:
- `frame_repro.py`, `frame_repro2.py`, `frame_repro3.py`, `frame_clean_test.py` — staged repros
- `canvas_layout.py` — layout dump confirming canvas-wrap vs inspector aside geometry
- `frame_run.log`, `frame_run2.log`, `frame_run3.log`, `clean.log`, `layout.log` — full transcripts
- `frame2_01_interior.png`, `frame2_02_edge.png`, `frame_clean_result.png` — pre/post-click screenshots showing inspector switching to **Frame** panel on a clean rect-hit click