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 (returns 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 Buttons 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.