Skip to content

Direct vehicle manipulation: click-select, drag-to-move, drag-to-rotate - #18

Merged
upgradedev merged 3 commits into
mainfrom
feat/direct-vehicle-manipulation
Jul 29, 2026
Merged

Direct vehicle manipulation: click-select, drag-to-move, drag-to-rotate#18
upgradedev merged 3 commits into
mainfrom
feat/direct-vehicle-manipulation

Conversation

@upgradedev

Copy link
Copy Markdown
Owner

Summary

Adds direct manipulation of vehicles on the review-adjust schematic, per the SmartDraw-style reference: click a vehicle to select it, drag to move it, drag a dedicated handle to rotate it. Strictly additive: the existing dropdown/ClockPicker controls are untouched and remain the accessible fallback.

The constrained vocabulary is the whole point of this repo (tests/security/, 46 tests, its own pen-test CI job), so every gesture snaps to, and can only ever write, one of the SceneGraph's existing legal enum values:

  • Move -> Movement.approach (N/E/S/W), snapped from the drag vector by angleToApproach.
  • Rotate -> Impact.clock_position (1..12), snapped by angleToClock, matching ClockPicker's own angle convention.

Both pure snap functions (frontend/src/lib/manipulate.ts) are total functions: every real-number input, including huge/tiny/non-finite ones, maps to a member of the closed enum. No continuous coordinate is ever written to the scene.

Why a new panel, not a hit-layer on the existing schematic

The SceneGraph has no continuous position field by design (src/claimscene/scene.py) — every geometric position is derived server-side by LayoutEngine, which the browser never runs, and /cases/preview-schematic returns only svg/warnings/contact_point/vehicle_count/road (no per-vehicle coordinates). The schematic is also deliberately rendered through <img src="data:image/svg+xml...">, specifically so a hostile SVG string can never execute script (see SchematicView.tsx's own comment). So VehiclePlacementPanel.tsx draws its own small, self-contained road template and vehicle markers (real <button>s over a decorative aria-hidden SVG, the same pattern ClockPicker already uses) in its own coordinate system, rendered above the untouched, still-authoritative SchematicPreview. This keeps the hardened <img> path completely alone and avoids ever drifting out of sync with the Python physics engine.

Requirement 7 (approval receipt), verified not assumed

src/claimscene/evaluation.py::diff_scenes is a generic, backend-side, per-field comparison that emits fixed rows including movements.{vid}.approach and impacts.{vid}.clock_position — it has no idea how the confirmed scene was edited. Every gesture here calls exactly the same patchMovement/setImpact pure helpers the dropdowns and ClockPicker already use, so the diff captures these edits automatically. Verified with: parity tests asserting a drag and the equivalent keyboard action both call onChange with output that's deep-equal to patchMovement(...)/setImpact(...), plus an e2e test proving a drag on the panel updates the same dropdown that a manual edit would.

Interaction model

  • Select: pointerdown on a vehicle marker (click or tap). Escape or a click on empty canvas deselects.
  • Move: drag the marker; candidate compass targets highlight live; releases past a 6px threshold commit the nearest of N/E/S/W. Keyboard: arrow keys on the focused marker.
  • Rotate: a separate handle appears once selected (never nested inside the marker button — axe's nested-interactive is a serious violation); dragging it snaps through the 12 clock positions with a live readout. Keyboard: [/] or shift+arrow on the marker itself (no need to leave the marker), or plain arrows/[/] once tabbed to the handle; Backspace/Delete clears back to "not struck".
  • Touch: touch-action: none on both draggable elements; both are 44x44px targets (verified at 375px).
  • AI-proposed vs human-confirmed: dashed ghost marker/handle when the frozen proposedScene differs from the live scene, mirroring the dashed/solid convention SchematicPreview already established.

Bugs found (and fixed) by testing against a real browser

jsdom can't synthesize real click-after-mismatched-pointer-target behavior, so these only surfaced running Playwright against a real browser locally:

  1. A real browser fires a synthetic click targeting the nearest common ancestor of the pointerdown/pointerup elements when they differ — which happens here because the marker snaps to its candidate slot mid-drag. That synthetic click was reaching the canvas and immediately undoing a just-completed selection. Fixed with a one-shot suppression flag, consumed by the very next click.
  2. That flag could leak past its own gesture if a drag ended without any click ever reaching the canvas (e.g. released outside the panel), silently swallowing the reviewer's next unrelated background click. Fixed by clearing it at the start of every new gesture, with a regression test.

Test plan

  • frontend/src/lib/manipulate.test.ts (35 tests): dense table-driven + fuzz tests proving both snap functions are total and always legal, including the round-trip with ClockPicker's own angle convention.
  • frontend/src/components/VehiclePlacementPanel.test.tsx (49 tests): selection, deselect (Escape/background click), move via pointer/keyboard (all 4 directions, below-threshold-is-a-click, pointercancel, non-primary-mouse-button ignored, touch pointerType), rotate via pointer/keyboard (drag, brackets, shift+arrow, Backspace clear, pointercancel), keyboard/pointer parity (byte-identical resulting scene), out-of-range-drag-never-illegal (both move and rotate), AI-proposed ghosting, multi-vehicle fan-out, vehicle-removed-while-selected, all 5 road layouts, and the two click-suppression regressions above.
  • ReviewStep.test.tsx (+1 test, existing tests unmodified): the panel and the dropdowns read/write the same shared scene.
  • journey.spec.ts / a11y.spec.ts / responsive.spec.ts (+4 Playwright tests, existing tests unmodified): full click-select/drag-move/drag-rotate/keyboard journey with dropdown/ClockPicker agreement; zero serious/critical axe violations with a vehicle selected; >=44px tap targets + touch-action: none + no horizontal overflow at 375px.
  • npx tsc --noEmit, npx vitest run --coverage (208 tests, 97.76%/90.93%/89.58%/97.76% stmts/branches/funcs/lines, all above the 90/90/85/85 gate), npm run build all clean.
  • All 4 new Playwright specs + the pre-existing (unmodified) full journey test + the pre-existing review-step a11y test verified passing individually against a real browser (system Edge, since the sandbox's disk space couldn't fit a fresh Playwright chromium download) — this is how both bugs above were actually caught.

No backend/schema files touched. No em-dashes in user-facing copy.

🤖 Generated with Claude Code

Efthimios Fousekis added 3 commits July 29, 2026 11:32
Prevents a drag whose pointerup never produces a click on the canvas
(e.g. released outside the panel) from leaving the suppression flag set
and swallowing the reviewer's next, unrelated background click.
@upgradedev
upgradedev merged commit 0101bde into main Jul 29, 2026
11 checks passed
@upgradedev
upgradedev deleted the feat/direct-vehicle-manipulation branch July 29, 2026 08:48
upgradedev pushed a commit that referenced this pull request Jul 29, 2026
README's Testing & CI section and SUBMISSION's accomplishments both
still quoted 126 frontend tests. PR #18 (direct vehicle manipulation)
added manipulate.ts and VehiclePlacementPanel.tsx with their test
files, taking Vitest to 208 across 24 test files.

Numbers taken from the green CI run on main (0101bde, run
30437023280), which is the same command CI gates on:
  - frontend job (npm run test:coverage): 208 passed (208)
  - python job (pytest tests/):           289 passed, 3 skipped

The backend stat was re-verified and is unchanged at 289, so only the
frontend number moves.
upgradedev pushed a commit that referenced this pull request Jul 29, 2026
The card's frontend stat went stale when PR #18 (direct vehicle
manipulation) added manipulate.ts and VehiclePlacementPanel.tsx with
their test files.

Authoritative source is the green CI run on main (0101bde, run
30437023280): vitest reports 208 passed across 24 test files, and
pytest still reports 289 passed, so the backend stat is unchanged.

render-cards.yml re-renders the PNG from this spec on push.
upgradedev pushed a commit that referenced this pull request Jul 29, 2026
README's Testing & CI section and SUBMISSION's accomplishments both
still quoted 126 frontend tests. PR #18 (direct vehicle manipulation)
added manipulate.ts and VehiclePlacementPanel.tsx with their test
files, taking Vitest to 208 across 24 test files.

Numbers taken from the green CI run on main (0101bde, run
30437023280), which is the same command CI gates on:
  - frontend job (npm run test:coverage): 208 passed (208)
  - python job (pytest tests/):           289 passed, 3 skipped

The backend stat was re-verified and is unchanged at 289, so only the
frontend number moves.
upgradedev added a commit that referenced this pull request Jul 29, 2026
…UBMISSION and gallery card (#20)

* chore(demo): update gallery-card-07 frontend test count (126 -> 208)

The card's frontend stat went stale when PR #18 (direct vehicle
manipulation) added manipulate.ts and VehiclePlacementPanel.tsx with
their test files.

Authoritative source is the green CI run on main (0101bde, run
30437023280): vitest reports 208 passed across 24 test files, and
pytest still reports 289 passed, so the backend stat is unchanged.

render-cards.yml re-renders the PNG from this spec on push.

* chore(demo): render gallery cards from demo/assets/src/ patch specs

* docs: refresh stale frontend test count (126 -> 208)

README's Testing & CI section and SUBMISSION's accomplishments both
still quoted 126 frontend tests. PR #18 (direct vehicle manipulation)
added manipulate.ts and VehiclePlacementPanel.tsx with their test
files, taking Vitest to 208 across 24 test files.

Numbers taken from the green CI run on main (0101bde, run
30437023280), which is the same command CI gates on:
  - frontend job (npm run test:coverage): 208 passed (208)
  - python job (pytest tests/):           289 passed, 3 skipped

The backend stat was re-verified and is unchanged at 289, so only the
frontend number moves.

* chore(demo): bump gallery-card-07 backend stat to 290

PR #19 (forensic-reconstruction register) landed on main while this
branch was open and net-added one backend test, so the card's backend
stat went stale too.

Green CI on main (66f3d85, run 30450005723): pytest 290 passed,
3 skipped; vitest 208 passed across 24 files.

Also refreshes the README line references in the spec notes: #19 added
a line to README's thesis section, shifting the cited lines from
190/364 to 191/365.

* chore(demo): render gallery cards from demo/assets/src/ patch specs

* docs: refresh backend test count (289 -> 290)

PR #19 landed on main mid-branch and net-added one backend test.
Green CI on main (66f3d85, run 30450005723): pytest 290 passed,
3 skipped.

---------

Co-authored-by: Efthimios Fousekis <tf@upgrade.dev>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant