Skip to content

refactor(locallibrary): split the player's ui.rs and app.rs by view - #462

Queued
botandbang wants to merge 5 commits into
mainfrom
bnb/tui-split
Queued

refactor(locallibrary): split the player's ui.rs and app.rs by view#462
botandbang wants to merge 5 commits into
mainfrom
bnb/tui-split

Conversation

@botandbang

@botandbang botandbang commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

What and why

Fixes LOC-161.

apps/locallibrary/src/tui/ui.rs (about 3,100 lines) and app.rs (about 4,000) hold every view, so every design change would touch both. This splits them by view with no change in what's drawn or how it behaves. It also adds a budget for what a frame costs, so the work that follows has a number to hold to, and brings docs/player.md and GLOSSARY.md up to date.

Added

  • apps/locallibrary/src/tui/frames/tests.rs: draws frames of the Library view, the full-screen deck and the Drives view at 160x45, with the 3,886-track fixture open, a demo track playing with its real analysis into a test mixer, and the playhead moved a fixed step a frame. No wall clock and no worker decides what's drawn.
  • A counting allocator in src/unit.rs, the binary's unit tests, so nothing is added to the shipped binary.
  • benchmarks/budgets/tui.json (allocations and bytes per frame, on every pull request that runs the Linux jobs) and benchmarks/budgets/tui-instructions.json (instructions per frame under cachegrind, in .github/actions/release-budgets). The second starts as null until the first run on the real runner.
  • benchmarks/budgets/tui-debug-instructions.json (instructions per frame under cachegrind in the debug profile, on every pull request that runs the Linux jobs), measured by the budgets job over 200 frames from its archive's unit binary. These catch a regression and aren't what a user's frame costs: the binary is unoptimised and built for coverage. Its limits start as null too.
  • .github/actions/release-budgets/frame-instructions.sh: the shell that measures a frame, used by that action and the budgets job. It takes the views from the budget file and runs them side by side.
  • apps/locallibrary/src/tui/docs/tests.rs (tui/docs.rs and tui/frames.rs hold the two test modules): generates the key tables in docs/player.md from KEYMAP and writes docs/player/*.svg, each of Library, Drives, Snapshots, Profile and Mixes at 80x24 and 160x45, dark and truecolor, from the test backend's buffer. Each test fails when a file differs and prints the command that updates it (LOCALLIBRARY_BLESS=1, as the repository's other generated files do).
  • Glossary entries for Drive, Snapshot, History, Prepare, Crate, Deck, Pads, Recording, Panel and Suggestion, and a usage note on Sign in.
  • 34 tests in tui/app/tests.rs, tui/ui/tests.rs and tui/theme/tests.rs, 17 of them Hegel properties, for what the moved files had no test of. tui/audio/tests.rs gains seek_fails, which leaves a track as a seek it couldn't make does, for what the player makes of that.

Changed

  • ui.rs is now ui/{chrome,library,player,drives,overlays}.rs and app.rs is app/{background,chrome,drives,input,library,loaded,mouse,player,playing,saved}.rs, with one impl App per file. The roots keep draw, App and the view enums. The second commit is a pure move: outside imports, use paths, visibility and the section banners it dropped, a line-by-line comparison finds only two signatures rustfmt rewrapped because their visibility got longer. No test file changed.
  • docs/player.md's hand-kept key table is replaced by generated tables, one per view and group.
  • release-budgets and the weekly budgets job get 60 minutes in place of 30, for the second release build.
  • The budgets job's filter also runs tui::frames::tests, it extracts its archive to a folder the next step reads, and it installs valgrind.
  • The guard that keeps a test's app from opening a browser is compiled only with debug assertions, where never_open exists, so the unit binary builds in the release profile. Every test but the frame test still runs with debug assertions.
  • TrackInfo's fields are visible to the tui module, for the frame test.

Removed

The hand-kept key table.

How it was tested

Rendering. A temporary hook, not in this PR, wrote every cell (symbol, colours, modifiers) of every frame the tui:: tests draw to a file per test, as drawn and again in dark, light and plain, four runs on each side with Hegel seeded. It normalised temporary folder names, clock times, drive and snapshot ids and the rows showing the machine's real free space, and held the animation clock at zero on both sides.

  • 159 tests draw the same frames on every run of the unsplit code, 25,982 frames a run. All four runs of the split code match them byte for byte: 103,928 frames, 454,220,576 cells.
  • The other 30 tests depend on when a worker's result lands, so they differ between runs of the same code. Of their 104,192 frames across the split runs, 102,420 appear in an unsplit run. The rest are the same kind of difference: taking one more run against three unsplit runs gives 546 new frames when that run is unsplit too, and 681 when it's split.

Frame cost. Allocations and bytes per frame are identical before and after. On macOS, 5 runs each gave the same numbers:

160x45, per frame allocations bytes
Library view 1,251 147,629
full-screen deck 735 109,882
Drives view 291 74,245

The limits in tui.json come from 5 identical runs in a Linux container (rust:1.96.1-bookworm, aarch64) on this branch: the same allocations, with 147,577, 109,882 and 67,249 bytes, a temporary folder's path being shorter there. Lowering library.allocations to 1,200 fails tools/budgets/cli.ts check with 1251 allocations is over its limit of 1200.

Instructions per frame, from the release-profile test binary under /usr/bin/time -l on macOS, medians of 5 runs of 600 frames less 5 of none:

before after
Library view 4,911,024 4,925,608 (+0.3%)
full-screen deck 4,453,819 4,453,411
Drives view 3,245,733 3,245,846

The 600-frame runs spread by about 0.2%. Valgrind isn't available on macOS, so those aren't cachegrind's numbers, and the release-profile step in .github/actions/release-budgets first runs on this PR. It uses the shell below, and its build line was run as written.

Debug frame instructions. The budgets job's two steps ran as written in a Linux container (rust:1.96.1-bookworm, aarch64, valgrind 3.19.0) limited to 4 CPUs, with no target/ in the checkout, as on CI. The archive came from cargo llvm-cov nextest-archive -p locallibrary-cli --test unit, the build job's command for that one package, and the filter left out binary(counters), which that archive doesn't hold. Five runs of 200 frames:

160x45, per frame lowest highest spread
Library view 65,202,807 65,213,581 0.017%
full-screen deck 64,632,353 64,641,268 0.014%
Drives view 36,978,737 36,984,416 0.015%

The runs don't match exactly, because the test's wait for the fixture's workers polls a clock. A run of 20 frames was 0.5% off on the deck, which is why the step uses 200. Lowering library to 63,900,000 fails tools/budgets/cli.ts check with 65206801 instructions is over its limit of 63900000, ratchet sets the null limits, and tui.json has the same hash before and after the step. A binary that fails stops the step with exit 123.

The step took 44 s with the machine otherwise quiet, and 65 to 91 s beside other builds; a fifth run's time is left out, as the Mac slept through it. A release-budgets job ran 2,632 million instructions under cachegrind in 15.45 s, about 3.2 times slower than here, which puts the step at about 2.5 minutes on a runner. That's an estimate. CI is x86_64 with a newer valgrind, so its instruction counts will differ from these, and its first run on this PR gives the real time and sets the limits. Neither x86_64 nor the release-profile binary went through this shell here.

Startup. Medians of 15 runs at 120x40 with the two release binaries alternating, before then after:

world first frame ready instructions peak
no drive 19.0 ms, 20.2 ms 19.0 ms, 20.2 ms 250,720,806, 251,361,551 (+0.3%) 10.2 MiB, 10.3 MiB
demo drive 18.5 ms, 18.9 ms 65.1 ms, 63.9 ms 264,457,630, 267,091,004 (+1.0%) 10.6 MiB, 10.6 MiB
3,886 tracks 20.0 ms, 20.2 ms 367.5 ms, 366.8 ms 1,477,185,517, 1,483,230,419 (+0.4%) 21.5 MiB, 21.6 MiB

The release binary is 53,388,272 bytes on both sides.

Coverage of the moved files. The lcov of cargo coverage leaves 14 lines uncovered in them, none of which a test can reach:

  • app/input.rs:200: Act::None is only built by keymap entries with no keys and no name, so no key or palette item performs it.
  • app/input.rs:528 and app/mouse.rs:219: activate's last arm and click_row's View::Profile | View::Mixes arm. Both are those two views, and every caller stops at them first.
  • app/player.rs:337: only opened_next queues a track on the audio side and it marks the upcoming track queued, and every path that drops that track drops the queued audio too.
  • app/player.rs:406-407 and 534-536: start_current is false only with no track at the queue's position, which playing_queue and next_pos have just ruled out.
  • ui/overlays.rs:73: Menu::sub_items only holds Rate and Colour.
  • tui/mod.rs:81, 141, 142, 146: these need a real terminal, for run's exit message and for event_loop's mouse and other events.

Checks. These ran before this branch's last merge of main, so CI is the gate on the final tree. cargo fmt --check, clippy for the workspace and for --no-default-features --features tui,account, cargo coverage (4,342 tests passed; 97.77% lines, 96.69% functions, 96.01% regions), tools/hegel-coverage.sh (1,535 tests passed; two runs read 95.60 / 94.13 / 93.66 and 95.64 / 94.19 / 93.73 for lines, functions and regions, against 95.56 / 94.24 / 93.60 before the new tests. Hegel's draws aren't seeded: those two runs of the same code differ on 409 functions in the lcov, mostly outside the player), tools/policy.sh, ast-grep scan, ast-grep test --skip-snapshot-tests, pnpm test:tools (442 tests passed), actionlint, zizmor and shellcheck. No existing test changed. The new tests were each broken once: an edited row in player.md and an edited colour in a screenshot fail with the update command, and a wrong expected string fails the frame test. The 34 coverage tests were each seen failing at their own assertion, with the line each one covers changed. The exception is the arm of the replay check for a drive with no export: the plan fails there either way, so that test was broken by forcing an offer instead. The screenshots come out the same on macOS and Linux and under another timezone, and two rendered through QuickLook look like the player.

Checklist

  • Tests cover the change
  • For locallibrary: no user-facing change, so no changelog line
  • The READMEs, docs/ and --help still say what the code does

@botandbang
test(locallibrary): hold a frame of the player to budgets
53a347b 
The player is about to be redrawn view by view, so what a frame costs needs a number that can't
drift. A test draws frames of the Library view, the full-screen deck and the Drives view at 160x45,
with the 3,886-track fixture open, a demo track playing with its real analysis into a test mixer,
and the playhead moved a fixed step a frame. The `unit` binary gets a counting allocator, so the
`budgets` job holds what a frame allocates to benchmarks/budgets/tui.json on every pull request.

The release-budgets action takes a frame's instructions under cachegrind, from a release-profile
test binary, as a run of 600 frames less a run of none. That's a second release build, so the two
jobs that run the action get an hour. `TrackInfo`'s fields open up to the tui module for the test,
and the guard against opening a browser from a test is now only compiled with debug assertions,
where the function it calls exists, so the test binary builds in the release profile.
@botandbang
refactor(locallibrary): split the player's ui.rs and app.rs by view
1ff9b69 
This is a pure move, with no change in behaviour. ui.rs (3,100 lines) and app.rs (4,000) held every
view, so each design change would have touched them both. ui.rs becomes chrome, library, player,
drives and overlays, and app.rs becomes background, chrome, drives, input, library, loaded, mouse,
player, playing and saved, with one `impl App` per file and the `App` struct left in the root.

The code moved as it was. What changed with it: `pub(super)` became `pub(in crate::tui)` and
`super::x` became `crate::tui::x` where a file moved a level down, items another file now needs got
`pub(super)`, imports went to the files that use them, and the section banners went, as each file
is one section now. The test modules are untouched, so the roots import what they find through
`use super::*`.
@botandbang
docs(locallibrary): generate the player's key tables and screenshots
ca93d2e 
docs/player.md's key table was kept by hand. It's now generated
from KEYMAP, a table per view and group, and a test fails if the file differs and says how to
update it. The same test module writes each view's screenshot at 80x24 and 160x45 as SVG, from the
buffer the tests draw. player.md links to them rather than embedding them, as the website's
Markdown doesn't carry images over, and its markers are Markdown comments because it shows HTML
comments as text.

GLOSSARY.md gets the words the player's design work uses: Drive, Snapshot, History, Prepare, Crate,
Deck, Pads, Recording, Panel and Suggestion, and a note that sign in and sign out are what the
player and docs say, with `login` and `logout` kept as the DJ account's CLI subcommands.
@botandbang
test(locallibrary): cover what the split moved
c6dccaf 
The split touched every line of the player's ui.rs and app.rs, and some of those lines had no test
of their own: the player starting on a saved view or a drive named on the command line, the Drives
view reading status, a drive plugged in reaching plugins, fields that can't be read, clicks in the
details panel and the sidebar, matches from a newer read of the drive, an output that panics as it
opens, and parts of the details panel, the plays column and an export's confirmation. A few only
ran when a property test happened to draw them.

These tests run them from the outside, with properties where a rule holds for every input. They
start the real `App::new` with `LOCALLIBRARY_AUDIO=silent` and `LOCALLIBRARY_VOLUMES` pointing at a
temporary folder, so they touch neither the machine's drives nor its audio devices. Whether the
terminal has 24-bit colour is read once a process, so that test runs a process per terminal. No
production code changes.

Fixes LOC-161
@botandbang
ci(locallibrary): count a frame's instructions on every pull request
dbf6b86 
A pull request only counted what a frame allocates, so a frame that got slower without allocating
more had to wait for the weekly release-profile run. The `budgets` job now also runs the frame test
under cachegrind, from the debug `unit` binary in its archive, and holds that to
benchmarks/budgets/tui-debug-instructions.json. The binary is unoptimised and built for coverage,
so these numbers catch a regression and don't say what a user's frame costs.

The shell moves out of the release-budgets action into frame-instructions.sh beside it, and both
use it. It takes the views from the budget file and runs them side by side, because reading the
fixture takes most of a debug run: six runs in a row would use up about 3 minutes on a runner before
counting a frame. Setup polls a clock, so the numbers move by under 0.02% between runs, well inside
the file's 1%. The limits stay null until CI's first run measures them on its own architecture.
@botandbang
botandbang requested a review from bnb as a code owner November 5, 2026 17:45
@botandbang botandbang added the all-os Run the macOS and Windows checks on this pull request label Nov 5, 2026
@linear-code

linear-code Bot commented Nov 5, 2026

Copy link
Copy Markdown

LOC-161

@github-actions github-actions Bot added documentation Improvements or additions to documentation github_actions Pull requests that update GitHub Actions code locallibrary locallibrary and the loclib-* crates locallibrary: cli The locallibrary command line locallibrary: player The terminal player and its waveforms tests Tests, fixtures and test harnesses tooling Repo config, lint and build tooling, and GitHub settings labels Nov 5, 2026
@bnb
bnb added this pull request to stack #465 November 5, 2026 17:47
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

Size

Compared with main. KiB are 1,024 bytes.

This pull request main Change
Client JavaScript 771.5 KiB 771.5 KiB 0 B (0%)
Client JavaScript, brotli 259.3 KiB 259.3 KiB 0 B (0%)
Client CSS 94.5 KiB 94.5 KiB 0 B (0%)
Client CSS, brotli 13.6 KiB 13.6 KiB 0 B (0%)

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

Rust coverage

From run 382. Both are held to the thresholds in .cargo/config.toml.

Tests Lines Functions Regions
All 97.76% (88087/90107) 97.14% (14329/14751) 96.14% (194676/202490)
Hegel property tests alone 96.2% (86687/90107) 95.47% (14083/14751) 94.46% (191275/202490)

@botandbang
botandbang added this pull request to the merge queue Nov 5, 2026
Any commits made after this event will not be merged.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bnb bnb bnb approved these changes

Assignees

No one assigned

Labels

all-os Run the macOS and Windows checks on this pull request documentation Improvements or additions to documentation github_actions Pull requests that update GitHub Actions code locallibrary: cli The locallibrary command line locallibrary: player The terminal player and its waveforms locallibrary locallibrary and the loclib-* crates tests Tests, fixtures and test harnesses tooling Repo config, lint and build tooling, and GitHub settings

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@botandbang @bnb