gpui-native-kit/docs/component-audit.md

Component audit — gpui-native-kit

Full audit of every component ahead of migrating signed onto this kit. Scope: all 7 crates (~21k LOC) — crates/core (30 components), crates/data, crates/editor, crates/layout, plus the foundation crates crates/theme, crates/icons, crates/utils, the facade, and every Cargo.toml feature graph.

Audit criteria, in priority order: no over-engineering, justified function arguments only, no memory leaks, best performance.

Method

Seven parallel read-only audits (one per component group) plus a manual audit of the foundation crates. Every gpui-behavior claim was verified against the pinned zed checkout (504a7f2, per Cargo.lock); every "unused param" claim was backed by workspace-wide greps (crates/, examples/, docs/, tests/). All 15 major findings were re-verified by reading the cited lines. No code was modified; static audit only, no tests run.

• Date: 2026-11-09
• gpui rev: 504a7f2 (zed checkout, per Cargo.lock)
• Revision audited: pre-signed migration state

---

Verdict per criterion

Criterion	Verdict
Memory leaks	Near-clean. Zero .detach() on component tasks/subscriptions anywhere in crates/ (grep-verified); tasks stored in fields, self-references via WeakEntity, undo history bounded (100 entries), icon cache bounded, keyed element state pruned by gpui per frame. One real leak: AnchorBoundsRegistry in utils (finding 11), plus one stuck-state bug in resize_handle (finding 10).
Over-engineering	A handful of cases: dead defensive machinery (dialog/command_palette bounds plumbing), drifted duplicate painter (scroll_area), pass-through wrappers (markdown), single-use indirection, one speculative persistence API (resizable).
Function arguments	~20 removal candidates found, all with zero-call-site evidence (list below). Feature graphs and module gating are clean (verified per-file against actual imports).
Performance	Three systemic hot-path issues (registry clone churn, uncapped caret blink, per-frame O(N) scan in diff_view) plus a recurring debug_selector pattern that allocates every frame in release builds across 7+ sites.

---

Major findings (15, all verified)

A. Shared keyed state across sibling instances — root cause: un-namespaced keys

gpui scopes use_keyed_state by the element-id path; a RenderOnce renders under its type name, so two instances of the same component under the same ancestor chain resolve to the same keyed state. Existing tests never catch this class because every test window mounts exactly one instance.

#	Location	Bug	Fix
1	crates/core/src/menu.rs:385,395-396	"menu-was-open" / "menu-scroll" / "menu-selection" shared by all Menus in a window → a closed sibling resets the open menu's selection every frame; arrow-key navigation and the selection highlight break with 2+ menus (the gallery ships 5 sibling dropdowns at examples/gallery/src/main.rs:2761-2860).	Wrap the three calls in window.with_element_namespace(id.clone(), …) — the instance id is already a parameter. context_menu.rs:67-77 is the correct in-repo pattern.
2	crates/core/src/dropdown_menu.rs:210	Same bug: "dropdown-open-origin" bare key.	Same namespacing.
3	crates/core/src/slider.rs:143	"slider-dragging" bare key → dragging one slider drives both.	Key by (self.id.clone(), "slider-dragging") — the pattern clipboard.rs:105 already uses.
4	crates/core/src/scrollbar.rs:144,157	Default key ElementId::Name("scrollbar") → two default-keyed scrollbars in one view share drag + autohide state (gallery renders two side by side at main.rs:3231,3257).	Make the id a required constructor parameter (Scrollbar::vertical(id, handle)) and delete .key(); 6 call sites, most already have ids to pass.

B. Correctness bugs

#	Location	Bug	Fix
5	crates/core/src/input.rs:2518 (+ :1802)	utf8_range_to_utf16 has a byte-for-byte copy of utf16_range_to_utf8's body — it never converts UTF-8→UTF-16. Every IME selection/marked-range report is wrong for non-ASCII text (CJK/emoji candidate anchoring). character_index_for_point likewise returns UTF-8 where the trait contract (gpui platform.rs) demands UTF-16. ASCII-only tests cannot catch this.	Implement the real conversion (encode_utf16().count()); add a non-ASCII regression test.
6	crates/core/src/combobox.rs:509-518	Chevron-open with a selected value refilters using the field text — which is the selected label — so only the selected option is listed. Inconsistent with Select (reopens listing all) and with query-focused open.	refilter("") when state.selected.is_some() in the chevron branch.
7	crates/core/src/command_palette.rs:481-510	The modal overlay is not deferred — z-order depends on mount position; in the gallery, alerts/toasts paint and receive clicks above the open modal's scrim. Also: the whole panel_bounds Rc<RefCell<Option<Bounds>>> + capture canvas is dead (the scrim is a sibling of the panel, so its mouse handler cannot fire for panel clicks).	Wrap in deferred(…) like alert.rs:303 / toast.rs:221; delete the bounds machinery, keep a bare canvas for the focus-steal paint.
8	crates/editor/src/code_view.rs:351	Mouse-down maps position with Pixels::ZERO for scroll while mouse-move correctly uses ScrollbarHandle::offset(...).y → clicking after scrolling selects the wrong row.	Pass the same offset in the down handler.
9	crates/editor/src/code_view.rs:102-108,137	tree-sitter old_tree reused across changed text without Tree::edit() — violates the documented parse contract → stale/garbled highlighting after edits (the gallery's Rust→Large sample toggle triggers it).	Delete the tree field and the old_tree param of highlight_with_tree; full reparse per change. Reintroduce incremental parsing only when a real consumer needs it.
10	crates/layout/src/resize_handle.rs:241-268	The move handler checks only the stored dragging flag, never event.pressed_button. macOS synthetic drag does not synthesize a MouseUpEvent on outside-release → the seam keeps resizing on hover until the next click (stuck interaction state).	In the move handler, clear dragging when event.pressed_button != Some(MouseButton::Left).

C. Memory / performance

#	Location	Bug	Fix
11	crates/utils/src/lib.rs:40-73	AnchorBoundsRegistry: app-global HashMap<ElementId, Bounds> that only inserts (never removes), keyed without WindowId (a two-window app can read a same-named element's bounds from the other window), and every record clones the whole map (cx.try_global().cloned() then set_global). Every Button records per frame (button.rs:244-248) → unbounded growth + O(N²)-ish per-frame allocation churn. Consumed by tooltip anchoring.	Key by (WindowId, ElementId); mutate in place instead of clone-and-set (verify the in-place global-update API in the pinned gpui before rewriting).
12	crates/core/src/input.rs:2049-2054	Caret blink is an uncapped Animation::new(CARET_BLINK_INTERVAL * 2).repeat() → full-window repaint + full text re-shaping at display refresh rate whenever a field is focused (~119 wasted repaints/s at 120 Hz). Violates the repo's own with_max_fps rule.	.with_max_fps(4.0) (blink period is 1 s; reduce_motion is already handled by gpui).
13	crates/editor/src/diff_view.rs:332-349	widest_row recomputed with an O(document) scan every render (walks every span of every row to feed with_width_from_item) — defeats virtualization on large diffs.	Compute once in new/set_versions, store in a field.
14	crates/data/src/tabs.rs:304,367,404	Index-keyed element ids on a drag-reorderable strip (dock reorders tabs via move_panel) — violates the stable-domain-id rule; ids permute on every drag, re-keying interaction state, the spinner, and the close button.	Add a stable per-tab key (Tab::new(key, label) or .key(SharedString)); stable keys already exist at every call site (tab.panel, labels). The index-based on_change/on_close API stays unchanged.
15	crates/core/src/menu.rs:461	Per-frame Vec<MenuItem> deep clone into the on_key_down closure — every label + handler cloned each frame the panel renders.	One Rc<[MenuItem]> per render; clone the Rc into the closure.

---

Param / dead-API removals

All verified by workspace-wide greps with zero call sites outside their own definitions/tests.

Item	Location	Note
Slider::value()	slider.rs:83-86	new already takes value
Separator::orientation()	separator.rs:46-49	third way to set one enum; only its own test uses it
Menu checked: Option<bool>	menu.rs:38,598	only ever compared to Some(true); checked(false) ≡ unset → plain bool (gallery/select call sites compile unchanged)
Tab::lead	tabs.rs:37,67-74,342-357	zero callers; also silently overrides icon when both set
PanelDescriptor::icon	dock.rs:61	zero of 45 constructor sites use it; delete the 5-hop plumbing (PanelTab.icon, TabView.icon, snapshot copy, open_panel copy, context-menu branch)
DropdownMenu::icon(), .tab_stop()	dropdown_menu.rs:76,88	untested and unexercised
Alert::icon() / .no_icon()	alert.rs:82,88	severity already derives an icon
ComboboxChanged event + EventEmitter impl	combobox.rs:181,212,352	zero subscribers workspace-wide; on_change covers it
CodeView::language() getter, clear_selection()	code_view.rs:150,227	zero callers
markdown plain_code() / highlight_code()	markdown.rs:199-203	cascade: delete the highlight_code flag + parse param; construction-time double parse goes with it
MarkdownTableCell.column	markdown.rs:104	derivable, unused by the renderer (only a test asserts it)
Input::submit_on() getter	input.rs:698	the setter set_submit_on is used; getter is not
Input::accessibility_label	input.rs:1879	placeholder/id fallback covers it
ScrollArea::accessibility_label	scroll_area.rs:190
scrollbar_state_key → pub(crate)	scroll_area.rs:200	only consumer is scrollbar.rs
ResizeHandle::accessibility_label, .disabled()	resize_handle.rs:70,64	disabled used only by its own test
Resizable::sizes() / set_sizes()	resizable.rs:38-49	built for a persistence feature that doesn't exist (docs/CHECKLIST lists persistence as a follow-up)
Side::Right arms in toggle_icon/toggle_label	shell.rs:112-124	the right strip renders WindowControls; arms unreachable
WindowControls::maximize_on_double_click	window_controls.rs:153	single internal caller → private
input into_bindings	input.rs:273	only called by bind_input_keys → private
render_tab(selected, selected_index)	tabs.rs:265-277	same fact passed twice → keep selected_index, derive selected
register_panel(cx) notify	dock.rs:195-199	notifies on first insert only, and nothing renders from panels → drop notify + the cx parameter (45 call sites, mechanical)
data item closures taking &mut Window	list.rs:102, sidebar.rs:148, tabs.rs:164, tree.rs:129	every in-repo caller passes `
input copy_selection(_window), caret_visible(&mut self)	input.rs:919,820	unused param / needless mut
accessibility_label batch	checkbox.rs:68, toggle.rs:65, switch.rs:68, link.rs:68, radio.rs:57,112, segmented.rs:114, popover.rs:83, dialog.rs:102, dropdown_menu.rs:112	zero external callers on all of them. A uniform library convention — decide once: keep the convention or delete the dead ones. Segmented's id-fallback announcing internal ids ("filter", "theme-demo-mode") should go regardless.

---

Over-engineering

• dialog.rs:186,236-252 — the panel's stop_propagation fires before the scrim handler can, making the scrim's panel_bounds.contains check unreachable → the whole BoundsCell + capture canvas is dead machinery. One dismissal mechanism suffices.
• scroll_area.rs:279-426 — a drifted ~150-line copy of scrollbar.rs's painter (no horizontal axis, no track-click jump, different autohide policy); ScrollAxes::Horizontal is promised but unimplemented, with zero call sites. Delete the copy; compose the real Scrollbar element. This is deletion of a copy, not a new abstraction.
• input.rs:525,563,1088 — Rc<RefCell<Option<InputLayout>>> with a single owner → plain Option<InputLayout> field.
• input.rs:494-496,2180,2191 — two always-Some Option<Rc<handler>> fields papered over with expect()s → non-optional Rc fields, delete the expects.
• input.rs:522,772,809 — caret_epoch_started: Option<Instant> is never None → plain Instant.
• markdown.rs:302-311 — CodeBuilder is a pass-through wrapper around String → use String::push_str directly.
• markdown.rs:296-299 — TableBuilder::None / Body { rows } is exactly Option<Vec<MarkdownTableRow>> → replace with the Option.
• markdown.rs:662 — Event::FootnoteReference arm is unreachable (ENABLE_FOOTNOTES not enabled) and contradicts the module doc → delete.
• avatar.rs:110-189 — user style refined twice (base + PixelAvatar), children re-clipped/re-rounded by the base anyway → delete PixelAvatar.style + its Styled impl + duplicate clipping.
• combobox.rs:329-332 — refilter returns a Vec<SharedString> that all five call sites ignore (per-keystroke collect for nothing) → return ().
• combobox.rs:320-333,430-433 — refilter doesn't clamp highlight (the command palette does); the identity round-trip position(visible[row]) == row is a bounds check in disguise → clamp in refilter, then highlight.filter(|&row| row < visible.len()).
• code_view.rs:251-253 — .max(0) after .clamp(0, len - 1) is a no-op.
• code_view.rs:282 / diff_view.rs:390 — char_width = text_size * 0.62 guessed monospace advance, duplicated in two files, drives hit-testing and row width → hoist to a theme metric.
• segmented.rs:167-171 — selectable: Vec<usize> = (0..len).collect() is always 0..len → index math.
• Cargo.toml (layout) — the dock feature enables resizable for nothing.

---

Performance (beyond the majors)

Eager format!/to_string for debug_selector

gpui's cfg-gate only disables the method body; arguments allocate every frame including release builds. The correct pattern (already used in clipboard.rs:126, status_dot.rs:97) captures cheap clones and formats inside the closure: .debug_selector(move || format!("{id}-row-{n}")).

Sites: switch.rs:136, field.rs:97-117 (×3), combobox.rs:601,679, command_palette.rs:337,413, tabs.rs:305,409, sidebar.rs:198, code_view.rs:559, diff_view.rs:404,542, input.rs:2044, menu.rs:286 (+ the popover/dialog a11y-label fallbacks).

Per-frame clones/collects in render

• segmented.rs:167-171 — deep clone of every SegmentedOption<T> + the 0..len Vec, per frame → store Rc<[SegmentedOption<T>]> at construction.
• select.rs:209-225 — whole options Vec cloned per frame and the trigger re-derives the label inside a closure → compute text once in render, move only the SharedString in.
• dock.rs:999-1111 — per pane per frame for the context menu: pane.clone(), tabs.clone(), three separate panels collects → capture one slim tuple Vec, collect once.
• alert.rs:307 — entries().to_vec() clones the entry Vec every render of AlertsView (borrow-scoping workaround; low priority).
• code_view.rs:478 — the whole Line cloned per visible row per frame; render_row only reads → borrow.
• menu.rs:286 + popover.rs:115, dropdown_menu.rs:205, dialog.rs:158 — per-frame SharedString::from(self.id.to_string()) fallback labels.

No-op notifies

• input.rs:897-916,1253-1261 — move_cursor/select_range/drag_select_to/scroll_by always cx.notify() + window.notify_text_input_changed(...): pressing Left at offset 0 or wheeling a non-scrollable field triggers a full re-render + IME re-query → early-return when cursor/anchor/scroll are unchanged.

Theme-token violations

Rule: crates/theme owns every color and metric — no raw rgb/rgba/hsla or layout px(...) elsewhere.

• button.rs:202-205 — hsla(0.0, 0.0, 0.0, 0.0) ×4 as a "no border" color → apply the border conditionally instead.
• dropdown_menu.rs:196 — hsla(0.0, 0.0, 0.0, 0.0) → gpui::transparent_black().
• avatar.rs:187 — Hsla { a: 0.16, ..main } + luminance multipliers 1.6/0.95/0.45/0.18 → theme token.
• separator.rs:72 — px(1.) → theme::BORDER_WIDTH (already exists, used by button).
• menu.rs:472, combobox.rs:666, command_palette.rs:404 — px(320.) popup max-height, three copies → one theme metric.
• tooltip.rs:79,92 — px(360.) max width, px(1.) nudge.
• input.rs:1195,1278 + :32 — px(1.5) caret width, px(1.) underline, SCROLL_LINES_HEIGHT = 20.0 → theme metrics (next to theme.scrollbar.*).
• resize_handle.rs:17 — KEYBOARD_STEP: f32 = 32.0.
• window_controls.rs:26,215-220,237 — TRAFFIC_LIGHT_PAD, rgba(...) Windows close-hover, caption width.
• window_frame.rs:13-15 — RESIZE_INSET, BORDER_WIDTH.

Input misc

• input.rs:1726 — every IME composition step pushes undo history; steps >300 ms apart fragment undo → merge unconditionally while marked_range is active.
• input.rs:1168 — entry.line.paint(...).ok() swallows a Result (failure = invisible text) → log like the shape path at input.rs:984 does.
• input.rs:820 — caret_visible(&mut self) doesn't mutate → &self.

Semantic inconsistencies

• alert.rs:264-269 — Alerts::clear() drops entries without running on_dismiss handlers (Toasts::clear runs them via start_exit → finish_exit) → run the handlers or document the difference.
• scroll_area.rs:403-407 vs scrollbar.rs:348-353 — autohide touch-on-drag vs touch-on-hover: two painters, two behaviors → pick one policy (subsumed by the scroll_area consolidation).
• dock.rs:316-362 — close_panel reconciles the displayed right view when its tab closes, but close_others/close_pane don't → the sidebar can keep showing a closed panel's inspector.
• combobox.rs:651-658 — "No matches" row is near-dead (empty results close the popup); the palette stays open showing "No matches" → pick one semantic.
• command_palette.rs:336 — row ids are list indexes ("command", row) for content that reorders every keystroke; Command.id is a stable SharedString → ("command", command.id.clone()).
• shell.rs:283-324 — hardcoded element ids not namespaced by the shell id ("sidebar-strip-left" etc.), unlike Dock which composes every id from dock_id → compose (id, "…").
• data a11y sets — sidebar.rs:270-298 counts headers in row sets; tree.rs:250-261 uses flat-list position/size instead of per-sibling → compute among real siblings.

---

Clean bills

badge, progress (fps-capped), spinner, status_dot, clipboard, empty_state, context_menu (the namespacing exemplar), list, editor/lib + editor/utils + highlight, layout/lib, theme (engine sound; watchers are app-lifetime; set_preset has one small redundant name lookup), icons (cache bounded, negatives cached), all Cargo.toml feature graphs, and all lifecycle plumbing: no detached tasks/subscriptions anywhere in crates/, WeakEntity used correctly, cx.notify() never from render, overlay state reclaimed by gpui's element-state eviction on dismiss.

---

Recommended fix order (before migrating signed)

1. P0 — shared keyed state sweep (findings 1-4): ~15 lines total; add a two-sibling regression test per component, since single-instance tests provably mask this class.
2. P0 — user-facing bugs: IME UTF-16 conversion (5), combobox chevron (6), palette z-order (7), code_view click (8) + incremental misuse (9), resize capture loss (10), tab element ids (14).
3. P1 — performance: registry (11), caret blink (12), widest_row (13), the debug_selector batch, the per-frame clone batch.
4. P2 — deletions: the param/dead-API table and the over-engineering list — this is where "no over-engineering" actually gets enforced, and it shrinks the API surface signed will migrate onto.