# 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 `. - **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`.