makepad-ports/makepad_ports_deep_assessment.md

# Makepad Ports — Deep Assessment (v2, brutal)

**Date**: 2026-09-01 · **Commit**: `3cfeaa2` · **Scope**: `makepad-ports/{rider,koboyo,insurance}` crates
(`app.rs`, `model.rs`, `canvas_view.rs`, tests, `setup_makepad.sh`, patch) · **Method**: full static read
with file:line evidence; coverage measured separately (see `docs/coverage/`). Supersedes the 6.0/10
in `makepad_ports_assessment.md` — that assessment predates the UI-craft audit and missed the
reproducibility hole.

## Verdict

**5.4 / 10.** A well-engineered *engine* wearing an unfinished *product*. The model layers are the
real thing — pure, documented, unit-tested state machines. Everything the user actually touches is
scaffolding: three parallel app monoliths with zero animation infrastructure, commit-on-touch
semantics, no keyboard path, hand-rolled id-string routing, and a build script that silently
tracks a moving upstream branch while claiming a pinned commit. The 140 green tests measure the
engine and the DOM-shape of the chrome; they cannot measure the interaction contract because the
interaction contract was never implemented.

| Axis | Score | One line |
|---|---|---|
| Architecture | 6.0 | One good idea (model/app split) executed consistently; everything above it is triplicated monolith |
| Design / UX craft | 3.0 | Zero animator tracks in 3,903 lines of UI; commit-on-down everywhere; pointer-only chrome |
| Performance | 5.0 | Fine at demo scale; three wrong shapes that cliff at real scale (undo snapshots, chrome resync, per-move relayout) |
| Bugs / correctness | 6.5 | Engine solid; risks concentrated at the app seam (multi-touch, panics-by-invariant, copy-coupled tests) |
| Security / reproducibility | 4.0 | Unpinned `dev` checkout is a build-breaking and supply-chain hole; `curl \| sh`; token hygiene OK-ish |
| Code quality | 6.0 | Excellent comments, honest READMEs; write-only aliases, no CI, no clippy/fmt gate |
| Tests | 6.5 | Real breadth on models; UI layer asserts copy strings, not contracts; product was bent to fit the harness |

---

## 1. Architecture

**The good — and it is genuinely good.** Every crate splits into a Makepad-free `model.rs`
(state machine, physics, registry, camera math) and an `app.rs` shell. Insurance's `SheetDrag`
ports the HTML's drag math verbatim with the constants documented (`model.rs:402–406`); koboyo's
`model.rs` header maps every HTML behavior to its Rust owner (`model.rs:3–16`). This is why 115
of the 140 tests exist at all. Keep this. It is the foundation the rest of this document builds on.

**A1 — Three monoliths, zero shared code.** `app.rs` = 1,592 / 1,291 / 1,020 lines. Three separate
toast implementations (`insurance:1313`, `rider:1004`, `koboyo:589`), three ad-hoc DSL component
vocabularies (`Card`/`RoundBtn`/`IconTile` vs rider's `Mut`/`Wht`/`Deep`), three navigation
handlers. The README's hard-won "DSL lessons" list exists precisely because every crate re-learns
the same lessons with no shared crate to encode them. Cost: every fix lands three times or drifts.

**A2 — God-function routing.** `handle_actions` is 239 lines (koboyo), 164 (rider), 125
(insurance) of sequential `if ui.view(cx, &[id]).finger_down(actions)` chains. This is a
hand-rolled router with no table, no exhaustiveness, no type-safety: adding one button means
editing a 200-line function whose match order is load-bearing (`return`s short-circuit).
The `live_id!` names it dispatches on are duplicated strings with zero compile-time link to the
DSL — the exact bug class that already cost 6 rider tests (README bug 1).

**A3 — Retained-mode framework driven as immediate-mode.** `refresh_chrome` (insurance 89 lines,
rider 74) re-derives *all* chrome — active page, tab tints, dots, sheet visibility, labels — from
`self.flow` on essentially every action. Correct-by-brute-force, no dirty tracking, and it
structurally forecloses animation: any transition started on a widget is stomped by the next
full resync. The zero-animator finding in `skills/makepad-ui-craft/PORTS-AUDIT.md` is not an
omission on top of this architecture; it is a consequence of it.

**A4 — The product was bent to fit the test harness.** Insurance runs a 390×1280 window because
headless `click()` targets widget centers and content past the window edge passes `wait_visible`
but can't be clicked (README bug 8). The fix chosen was *remove scrolling from the product*
rather than *teach the harness to scroll*. That is the tail wagging the dog, and it means the
ports don't reproduce the one thing every 390×824 phone app has: a scroll container.

**A5 — Physics constants are folklore, not spec.** The sheet thresholds (−26/−12 @ −0.2 px·ms⁻¹,
+36/+18 @ +0.28, −140 clamp, 8 px tap) live as inline literals + comments. Nothing names them as
one `MotionSpec`; plan 003 will add velocity handoff and the constants will now matter in two
places. Name them once.

## 2. Design / UX craft

Fully documented with Before/After tables in `skills/makepad-ui-craft/PORTS-AUDIT.md`; summary of
the brutal part:

- **D1 (HIGH)** — ~40 tappables across all three crates fire actions on `FingerDown`
  (`insurance:1427–1531`, `rider:1108–1214`, `koboyo:822–883`). No tap-cancel, no press feedback,
  and rider mixes commit-on-down Views with commit-on-up `Button`s on the same screens — identical
  looks, different semantics.
- **D2 (HIGH)** — **Zero `Animator` tracks in 3,903 lines of UI.** Every toast, drawer, popover,
  sheet-settle and screen change is a `set_visible`/`refresh_chrome` snap.
- **D3** — Chrome is pointer-only. Plain-View buttons have no focus state, no keyboard activation,
  no semantics. Koboyo has a 76-line `handle_keys` for canvas tools while its own chrome can't
  take focus. This fails jakubkrehel/better-accessibility wholesale, not in detail.
- **D4** — Emoji as icons (`insurance:139,264`), brand hex repeated per widget
  (`#7b5cf6` at `insurance:122,170`…), nested radii unrelated to padding (`insurance:262`:
  outer 20 / pad 14 / inner 13), koboyo focuses the search field before the drawer is visible
  (`koboyo:608–617`).
- **Correct restraint that must be preserved**: koboyo's 1:1 canvas gestures and wheel zoom;
  rider's unanimated PageFlip; the sheet's 1:1 drag tracking.

## 3. Performance

Fine at demo scale. Three shapes are wrong and will cliff:

- **P1 — Undo is 60 deep copies of the world.** `koboyo/model.rs:555,564,575`:
  `undo_stack.push(self.elements.clone())` on every committed edit, `HISTORY_MAX = 60`. A freehand
  stroke with hundreds of points makes every subsequent edit clone it again — O(scene) memory and
  time per edit, 60 retained copies. At toy scale invisible; on a real canvas this is the first
  thing that dies. Command-pattern deltas or `Arc`-shared structural snapshots are the fixes.
- **P2 — Per-pointer-move relayout.** The sheet drag writes
  `script_apply_eval!(…{ margin: Inset{…bottom: #(b)} })` per `FingerMove`
  (`insurance:1570`) — a script eval + full relayout per event at input rate, for motion that
  should be a shader-instance offset (repaint only). Same pattern class in the other crates
  (9 `script_apply_eval` call sites total).
- **P3 — O(everything) chrome resync per action** (A3). Every tab press rewrites all chrome.
- **P4 — Per-keystroke allocation storm, wrong shape.** `koboyo/model.rs:281–293`:
  `search_tools` builds `format!("{} {} {}")` + `to_lowercase()` for each of 112 tools on every
  keystroke, then `rebuild_drawer` re-marshals the whole `PortalList` (`app.rs:806–807`).
  Precompute a lowercase haystack per tool once; 112 items forgives you today, the shape doesn't scale.
- **P5 — Linear scans on the hot pointer path.** `erase_at`/`select_at`/`lasso_hit`
  (`koboyo/model.rs:633,655`) scan all elements per pointer move while erasing/selecting. No
  spatial index. Acceptable below ~1k elements — the threshold is not documented anywhere.
- **P6 — Test economics.** Headless UI tests boot the full app per test (~185 s per crate for 8–9
  tests). That is a per-test tax that discourages exactly the interaction tests this codebase
  is missing (see §6).

## 4. Bugs & correctness risks

- **B1 — No multi-touch guard on the sheet drag.** `insurance:1559–1585` matches
  `Hit::FingerDown/Move/Up` without checking the digit/device that started the drag. A second
  finger mid-drag re-enters `FingerMove` with alien coordinates. The skill's gesture checklist
  calls this out explicitly; the HTML original effectively got this free from Pointer Events
  capture semantics. *Not verified on hardware — flagged from code shape.*
- **B2 — Panic-by-invariant.** `koboyo/model.rs:852` `.position(...).unwrap()` (tool group must
  exist in `groups`); `insurance/model.rs:647` `is_some() && .as_ref().unwrap()` (safe today,
  fragile idiom — this is what `if let Some` is for). The invariants hold by construction *now*;
  nothing enforces them at the seam where the registry is edited.
- **B3 — Tests coupled to copy.** 166 `text_exact`/`wait_visible`/`click()` locator uses across
  the three `ui.rs` suites; README bug 9 records the `"🔋 89% Charged"` breakage. Any copy edit —
  including plan 005's emoji removal — breaks UI tests by design. Locators should target ids,
  not marketing strings.
- **B4 — Focus before existence.** `koboyo:608–617` sets key focus onto a widget inside a
  panel that isn't visible yet in the same dispatch. Works by accident of ordering today.
- **B5 — Radius/padding math unverified.** `insurance:262` and siblings (`:273+`) — flagged in
  the audit; the side paddings were never measured. Either the math is wrong or it's accidental.

## 5. Security & reproducibility

- **S1 (the real one) — The build is not reproducible and trusts a moving target.**
  `setup_makepad.sh` does `git clone --depth 1 --branch dev` — **the tip of `dev`, unpinned** —
  while the README claims `b41e740`. Any upstream push can (a) break the patch, (b) change
  behavior under test, (c) inject code you never reviewed into a build you run with local
  network access. This contradicts the repo's own "verified from scratch" claim: what was
  verified was *that day's* dev tip. Pin the SHA in the script (`git fetch origin b41e740 &&
  git checkout b41e740`), fail hard if the patch doesn't apply.
- **S2 — `curl | sh` rustup install, unpinned toolchain** (script installs "stable", today
  1.98.0; README recorded 1.97.1 — silent drift already happened). Pin via `rustup-init`
  checksum or at minimum `--default-toolchain <version>`.
- **S3 — Vendored skills are agent-executed instructions.** `skills/vendor/**` is third-party
  Markdown that agents in this workspace read as doctrine — a prompt-injection surface by
  definition (Emil's own `improve-animations` Hard Rule 4 makes the point). Policy: `vendor/`
  is *data*; only `skills/makepad-ui-craft/` is executable doctrine; diff vendor updates.
- **S4 — Credential hygiene.** The gitdab token was passed on process command lines (visible in
  `ps` for the push duration) and pasted in the conversation; it was never written to disk or
  git config. Rotate it; move to a credential helper or env var piped via stdin.
- **S5 — Application surface itself is small** (no network, no file IO beyond resources, zero
  `unsafe` in the ports — verified by grep). The risk lives in the build chain, not the apps.

## 6. Code quality & tests

- **Q1** — Comment discipline is genuinely excellent (model headers mapping HTML→Rust, bug
  postmortems in the README). This is the codebase's best habit. Keep it.
- **Q2** — Write-only naming: rider's `Mut`, `Wht`, `Deep` label styles; single-letter loop ids;
  `fx` effect bags. Fine for the author-of-the-week, hostile to everyone after.
- **Q3** — No CI, no gate. 140 green is enforced by nothing: no workflow, no pre-push hook, no
  clippy, no rustfmt config. The setup script is the CI, and it runs on demand only.
- **Q4** — Test pyramid is honest at the bottom (78 unit + 37 integration on pure models), thin
  and brittle at the top (25 UI tests asserting string presence and screen reachability). There
  are **no interaction-contract tests** — tap-cancel, drag-release-settle, toast-interrupt —
  because the contracts don't exist yet (D1/D2). Plans 001–003 add both the behavior and its tests.
- **Q5** — Coverage was never measured before this assessment. The instrumented run and its
  gaps live in `docs/coverage/COVERAGE.md`; uncovered branches found there get tests in the same
  tranche (see roadmap Phase 0).

## 7. Priority matrix

| # | Finding | Sev | Owner plan |
|---|---|---|---|
| S1 | Unpinned upstream checkout | HIGH | Phase 0 |
| D1 | Commit-on-down, no press contract | HIGH | Plan 001 |
| D2 | Zero animation infrastructure | HIGH | Plans 000/002–004 |
| A2/A3 | God-router + full chrome resync | HIGH (enabler) | Phase 5 |
| P1 | Undo deep-copies the scene | MED (cliff) | Phase 5 |
| D3 | No keyboard/focus path in chrome | MED | Phase 2 |
| B1 | Multi-touch drag hijack | MED | Plan 003 |
| B3 | Copy-coupled UI tests | MED | Phase 0/5 |
| P2 | Per-move relayout | MED | Plan 003 |
| A4 | No scroll container (test-shaped product) | MED | Phase 5 |
| S2/S4 | Toolchain pin, token rotation | MED | Phase 0 |
| P4/P5 | Search allocs, linear scans | LOW (document threshold) | Phase 5 |
| Q2/B2 | Naming, panic idioms | LOW | Phase 5 |

Execution phases: `plans/ROADMAP.md`.