feat(hytte-plugin-niri-layouts): golden picks 75/25 or the golden cut by the screen's logical width (#1052)

[vibechoom]

What / why

Annika (#1052, 2026-09-10): "A golden ratio cut rather than the 25% cut would be better for smaller screens" / "Can we make this adaptive?" golden's 75 % / 25 % split (#1019) was tuned for her ultrawide; on a laptop or 1080p/1440p screen a 25 % narrow column is too cramped to use.

The rule

One breakpoint, on the logical width of the output the focused workspace lives on (layout::GOLDEN_BREAKPOINT = 2560):

logical width	wide	narrow
≥ 2560 px	75 %	25 % (unchanged, #1019)
< 2560 px	61.8 %	38.2 % (the golden cut)

Logical, not physical — a HiDPI output at 2x scale counts by its post-scale (usable) width, which is what niri's own LogicalOutput::width already reports. Missing output / no logical geometry (headless) falls back to the wide pair (the exact behaviour every build before this had) and logs one debug line explaining why.

The pure width → pair decision is layout::golden_pair, unit-tested directly against the decision table (2559 → golden cut, 2560 → wide pair, unresolvable width → wide pair). niri::apply adds one Request::Outputs alongside the existing Windows/Workspaces/FocusedOutput fetch — always, even for equal/split, so the shape of what's asked for never depends on which button was pressed — resolves the focused workspace's own output name via the new layout::target_output_name (the same targeting rule plan() uses internally, so the two can never pick different workspaces), looks up that output's logical width, and hands the resolved pair into plan().

Both pairs are pinned at the wire as literal bytes in niri.rs's tests ({"SetProportion":75.0} / 25.0 and {"SetProportion":61.8} / 38.2), matching the existing pinning style from #1019/#1026's review (never derived from the Rust constants, so a rounding regression would still be caught).

The CLI --help and the chip tooltip name both pairs and the breakpoint rather than a live per-screen answer — neither one actually dials niri to render that text (--help never connects to the socket at all; the tooltip is a pure, synchronous string with no IPC round trip), so they document the rule instead of a snapshot of one screen.

Lane

crates/hytte-plugin-niri-layouts/src/{layout.rs,niri.rs,cli.rs,main.rs} + docs/live-verify.md's niri-layouts section. plugin.rs needed no changes (the chip already renders Layout::tooltip() verbatim). watch.rs and flake.nix/nix/ untouched, per the #1053 lane split.

Mutation table (falsified after committing, md5 before/after matched on clean revert)

mutation	result
flip the breakpoint comparison (< → >=)	5 tests red (golden_pair_picks_by_the_breakpoint, golden_pair_is_consistent_well_away_from_the_breakpoint, both niri.rs pair tests, the narrow wire-bytes test)
swap the two pairs (wide ↔ narrow values)	8 tests red (both fraction-pinning tests, four niri.rs wire/pair tests, the missing-output fallback test)
drop the Outputs request	4 tests red (golden_asks_outputs_exactly_once_per_apply, queries_all_four_snapshots_before_acting, both narrow-pair niri.rs tests, since outputs then resolves to nothing)

Gates (all foreground, in the devShell)

• nix fmt -- --no-cache — reformatted 2 files initially (own new code + a markdown wrap in live-verify.md), clean on the final pass — exit 0
• cargo clippy -p hytte-plugin-niri-layouts --all-targets -- -D warnings — exit 0 (the crate doesn't define the system-tests feature, so that flag from the usual recipe doesn't apply here — confirmed via cargo package does not contain this feature before dropping it)
• cargo clippy --workspace --all-targets --features system-tests -- -D warnings — exit 0
• cargo test -p hytte-plugin-niri-layouts — 101 passed, 0 failed
• cargo run -p hytte-plugin-niri-layouts -- --help — reads correctly, names both pairs and the breakpoint

Live-verify caveat

Nothing here has drawn a pixel — this environment has no niri session. docs/live-verify.md gets a new (#1052) checklist item under the niri-layouts section: it needs two outputs of different logical widths straddling 2560 px (an external monitor plus a laptop panel, or niri msg output <name> scale <n> to push a HiDPI panel's logical width across the line without new hardware), confirms golden picks 75/25 on the wide one and 61.8/38.2 on the narrow one, that the pick follows the focused workspace's own output rather than whichever monitor the shell started on, that unplugging the external monitor falls back to 75/25 with one debug line in the journal, and that the CLI hat's apply golden matches the chip's split on each output. Your glass is the acceptance, same as #1019/#1038.

Fold-in — review round 1 (@ 8baa851)

The adversarial review (#1056 (comment)) returned MERGE with no behavioural defect and a handful of doc/test-strength findings, all folded in — full per-finding table and refreshed mutation/gate results in #1056 (comment). Summary:

• MED-1: layout.rs's GOLDEN_NARROW_MAJOR doc contradicted the code (claimed a 2560-wide screen gets the golden cut; it doesn't — < GOLDEN_BREAKPOINT gives it 75/25). Doc rewritten to match the code exactly; GOLDEN_BREAKPOINT and its comparison are unchanged, pending your read on which side of exactly-2560 you want — flagged in the doc and in docs/live-verify.md's checklist item.
• MED-2: pinned the "apply resolves Golden's width through the same targeting rule plan() uses" invariant, previously untested at the apply level (the obvious wrong refactor passed the whole suite). No production code change — the line was already correct, only unpinned.
• LOW-1: the missing-width fallback diagnostic now derives its pair text from the actual constants instead of a hand-typed "75 % / 25 %", and is pinned via a Transport::log seam mirroring watch.rs's existing capture pattern.
• LOW-2: one test cross-checks the hand-typed "2560" strings (cli::USAGE, the tooltip, main.rs's docs) against GOLDEN_BREAKPOINT directly, so a future bump that misses one of them fails there.
• NIT-1/2/3: strengthened three tests (unfiltered request-order assertion, HashMap-keyed output lookup instead of a flattened Vec + linear scan, a literal assertion alongside a self-comparison).

Refs #1052 #1019

🤖 Generated with Claude Code

https://claude.ai/code/session_0157rpXf1ks2hZEfdB47UJ6v

[vibechoom]

Adversarial review — PR #1056 @ d7e690b

Verdict: MERGE — no behavioural defect found. The rule shipped is the rule Annika confirmed ("sounds fine I guess?" on the #1052 triage table), the wire bytes are right for both pairs, the boundary is pinned on both sides, and all three claimed mutations reproduce exactly. Findings below are one doc line that contradicts the shipped behaviour at exactly the boundary case, two unpinned mechanisms (both proven with mutants that pass 101/101), and nits. None block; MED-1 wants one word from @annikahannig either way.

Read-only worktree at d7e690b, mutants applied and reverted with md5sum verified back to baseline after every one.

---

MED-1 — the doc contradicts the code for a 2560-wide screen, and the triage's own reading column put 1440p on the other side

crates/hytte-plugin-niri-layouts/src/layout.rs:50-51:

A 25 % narrow column is fine at 2560 px and wider, but on a 1920- or 2560-wide laptop screen it leaves too little room to use the narrow column for anything

Those two clauses contradict each other in consecutive lines, and the second one describes behaviour the code does not have: golden_pair is width < GOLDEN_BREAKPOINT, so a 2560-wide screen gets 75/25, not the golden cut. golden_pair_picks_by_the_breakpoint (layout.rs:842-846) pins it that way deliberately.

This is not just a comment slip, because the same off-by-one sits in the triage table Annika approved: its < 2560 row is labelled "laptop / 1080p / 1440p: the golden cut" — but 1440p is 2560×1440, i.e. exactly 2560 logical px wide, so it lands in the >= 2560 row and gets 75/25. Same for a 2560×1600 laptop panel at scale 1. The number Annika said yes to is >= 2560 → 75/25, and the code implements that number exactly, so this PR is faithful — but the two places that explain the number both assume the opposite for the most common monitor size on the market.

Failure scenario: Annika plugs in a 2560×1440 monitor, clicks golden, gets the 25 % sliver she filed #1052 about, and the code comment tells the next maintainer that case was meant to get 0.618.

Fix: one word from @annikahannig — does a 2560-wide screen want the golden cut (<=, or GOLDEN_BREAKPOINT = 2561) or 75/25 (today)? Either way, delete "or 2560-wide" from layout.rs:51 so the doc stops describing behaviour the code does not have, and drop "1440p" from the triage's narrow row if 2560 stays exclusive. If the answer is "yes, 1440p should get the cut", the only edits are the comparison and the three boundary assertions.

MED-2 — apply's "same targeting rule as plan" invariant is not pinned; the obvious wrong refactor passes 101/101

niri.rs:72. The PR body sells this line as load-bearing: "resolves the focused workspace's own output name via the new layout::target_output_name (the same rule plan() uses internally, so the two can never pick different workspaces)". Nothing tests it at the apply level.

Mutant M7 — replace it with the obvious-looking inline:

let target_output = output.as_deref();   // instead of layout::target_output_name(&workspaces, output.as_deref())

→ 101 passed, 0 failed. Clean green.

The two disagree exactly on the fallback path the crate already treats as real (no_focused_output_still_plans_off_the_focused_workspace, niri.rs:619): when niri reports FocusedOutput: None, plan falls back to the globally focused workspace but the raw name is None, so golden_pair(None) → the wide pair. Concretely: on a laptop, any moment niri has no focused output (hot-plug, an output going away, the compositor between focus grabs), golden silently sends 75/25 instead of 61.8/38.2 — the exact bug #1052 exists to prevent, and it would not show up in the mutation table because target_output_name's own unit test (target_output_name_falls_back_like_plan_does) covers the fallback in isolation and is never wired into apply.

The fixing test (verified: passes on d7e690b, reddens M7 and only M7 — 102 passed / 1 failed):

    /// **The `target_output_name` invariant, at `apply`.** With no focused
    /// output niri-side, `plan` falls back to the globally focused workspace —
    /// so the *width* has to be resolved through that workspace's own output
    /// too, not off the (absent) focused-output name.
    #[test]
    fn golden_resolves_the_width_through_the_workspace_when_no_output_is_focused() {
        let mut niri = Fake::with_output_width(vec![tile(10, 1), tile(20, 2)], 1920);
        niri.focused_output = None;

        apply(&mut niri, Layout::Golden).expect("the fallback path");

        assert_eq!(
            niri.widths(),
            vec![(10, 61.8), (20, 38.2)],
            "the target workspace still sits on a 1920 px output, so it gets the golden cut"
        );
    }

Worth adding a sibling with two outputs of different widths too (the scenario docs/live-verify.md's new item asks Annika to check by hand — "the pick follows the focused workspace's own output"); no apply-level test today has more than one output. Caveat, measured: such a test only catches a wrong-output mutant probabilistically (1 red in 6 runs), because outputs() flattens the HashMap — see NIT-2.

LOW-1 — the fallback diagnostic is entirely unpinned, and live-verify asks Annika to look for it

niri.rs:74-81. Mutant M6 — delete the whole if layout == Layout::Golden && logical_width.is_none() { eprintln!(…) } block → 101 passed, 0 failed.

docs/live-verify.md:451-453 asks her to unplug the external monitor and confirm "the journal should carry one golden: no logical width … debug line". Nothing in the suite would notice if that line stopped being emitted, or if its wording drifted away from what live-verify tells her to grep for. watch.rs:513 shows the crate already has a captured-stderr shape available for this.

Same block, niri.rs:77: the message interpolates GOLDEN_BREAKPOINT but hardcodes (75 % / 25 %), so a change to GOLDEN_WIDE_MAJOR/MINOR makes the diagnostic lie while every test stays green. Use the constants, or drop the parenthetical.

LOW-2 — the five human-facing "2560" strings are literals never cross-checked against GOLDEN_BREAKPOINT

cli.rs:28 (USAGE), layout.rs:178 (tooltip), main.rs:48 and main.rs:75 (module docs), plus docs/live-verify.md ×3. Every one is hand-typed; the assertions that guard them (every_tooltip_states_its_own_percentages at layout.rs:818-821, the_usage_text_names_both_golden_pairs_and_the_breakpoint at cli.rs:171) also assert the literal "2560".

Mutant M4a — GOLDEN_BREAKPOINT = 2561 → 2 red (the_breakpoint_is_twenty_five_sixty, golden_pair_picks_by_the_breakpoint). Both are fixed by editing the number in the test, which leaves the tooltip and --help still saying 2560 and both string tests still green. Given MED-1 may move this constant next round, that is a live hazard, not a hypothetical.

Fix: keep the literal assertions (they are the right anti-closed-loop pinning for the percentages) and add one derived assertion per string, e.g. assert!(golden.contains(&GOLDEN_BREAKPOINT.to_string())) — literal and derived together catch drift in both directions.

NIT-1 — queries_all_four_snapshots_before_acting does not pin "before acting"

niri.rs:477-493. It asserts on Fake::queries() (niri.rs:295-301), which filters actions out, so request ordering relative to actions is invisible to it.

Mutant M9 — inject ask(transport, Request::Action(Action::FocusWindow { id: 1 })) between the Workspaces and Outputs fetches → the test passes. An action was sent before two of the four snapshots and the test named "before acting" did not notice.

Not reachable given the current function shape (all four are bound before plan is called), so: nit. Assert on the first four entries of niri.seen instead of on the filtered list, and the name becomes true.

NIT-2 — outputs() throws away the map keys and linear-searches by Output.name

niri.rs:133-138. niri replies with HashMap<String, Output> keyed by connector name; this does into_values().collect() into a Vec, and logical_width_of (layout.rs:269-275) then linear-searches it comparing output.name. Two costs: it assumes the map key and the name field never diverge (if they ever did, the lookup silently misses → wide pair, one log line, no test), and it makes every multi-output apply test order-nondeterministic, which is why the sibling test suggested in MED-2 only catches a wrong-output mutant 1 run in 6. Keeping the HashMap and doing outputs.get(name) is O(1), deterministic, and drops the find.

NIT-3 — equal_and_split_ignore_the_output_width is a weaker assertion than it looks

niri.rs:526-539 compares narrow.widths() == wide.widths() at two columns, where Equal and Split both produce [50.0, 50.0] — so the two halves of the test are comparing the same value against itself twice over. A three-column fixture makes it actually discriminating.

---

Attacks that came back clean (stating them so they are not re-run)

• Logical vs physical is right, and logical.width is the only defensible source. niri-ipc 26.4.0 documents LogicalOutput.width as "Width in logical pixels" in "the compositor's coordinate space" — post-scale and post-transform, since the logical rect is what a multi-output layout is arranged in. It is also the same rectangle SetProportion is a proportion of, which is the real argument: the breakpoint and the split read the same number. There is no working_area/work_area field anywhere in niri-ipc 26.4.0 (grepped), so there is no better source to have used. The code reads logical.width, never modes[].width (which is documented "physical pixels"). M5 — read logical.height instead → 2 red, so the field choice is pinned.
• Portrait outputs therefore read their rotated width: a 1440×2560 portrait monitor is 1440 logical px wide → golden cut. That is the correct reading of "how much room is there for columns", but it is a behaviour nobody has stated out loud — worth one line in the live-verify item if Annika ever rotates a panel.
• Wire bytes. All four constants are f64 literals (0.75/0.25/0.618/0.382, layout.rs:33-59), never 1.0 - x, and percent() (niri.rs:112) is the single * 100.0. Checked the products serialise clean: 61.8 and 38.2 with no …0000004 tail, pinned as literal bytes at niri.rs:464-469 alongside the existing 75/25 pins at niri.rs:440-445. Matches feat(hytte-plugin-niri-layouts): three one-click column layouts for the focused niri workspace — preem chip + standalone CLI (#1019) #1026's pinning style exactly.
• Boundary, both sides. M4a (2561) → 2 red; M4b (< → <=, i.e. 2560 gets the cut) → 1 red. 2559 / 2560 / None are all asserted at layout.rs:836-852.
• Four requests per apply. The extra Outputs round trip is one more short-lived socket connect per click (SocketTransport reconnects per request) — fine, and fetching it unconditionally rather than only for golden is the right call. The exact four-request order and count are pinned by the assert_eq! on the full vec (niri.rs:482-492), and golden_asks_outputs_exactly_once_per_apply guards against one-per-column.
• Tooltip length. 116 chars. Node::Icon's tooltip maps straight to set_tooltip_text with no cap anywhere in the proto or wire_map; GTK wraps. Non-issue.
• --help run for real, exit 0, reads correctly and names both pairs plus the breakpoint.
• The #1052 answer. Annika confirmed on-issue; she did not name a different number. Nothing to carry into a fix round on that axis beyond MED-1's boundary question.

Mutation table (mine, re-run at d7e690b; md5 verified back to baseline after each)

#	mutation	file:line	result
M1	flip the breakpoint comparison (< → >=)	layout.rs:79	5 red — matches the PR body
M2	swap the wide and narrow pairs	layout.rs:33-59	8 red — matches the PR body
M3	drop the Outputs request (outputs() returns empty)	niri.rs:133	4 red — matches the PR body
M4a	GOLDEN_BREAKPOINT 2560 → 2561	layout.rs:19	2 red
M4b	boundary inclusive (< → <=)	layout.rs:79	1 red
M5	logical_width_of reads logical.height	layout.rs:274	2 red
M6	delete the missing-width diagnostic	niri.rs:74-81	0 red → LOW-1
M7	resolve the width off the focused-output name, not target_output_name	niri.rs:72	0 red → MED-2
M8	logical_width_of ignores name, takes any output	layout.rs:269	2 red (unit only; 0 at apply)
M9	send an Action between the Workspaces and Outputs fetches	niri.rs:69	0 red → NIT-1
—	MED-2's supplied test, on clean d7e690b	niri.rs (added)	103 passed / 0 failed
—	MED-2's supplied test, against M7	niri.rs (added)	102 passed / 1 failed (kills it)

Gates (fresh, foreground, in the devShell, RUSTC_WRAPPER unset)

gate	exit
cargo clippy --workspace --all-targets --features system-tests -- -D warnings	0 (0 warnings, 0 errors)
cargo test -p hytte-plugin-niri-layouts	0 — 101 passed, 0 failed
cargo run -p hytte-plugin-niri-layouts -- --help	0

Both re-run on the restored baseline after the mutation campaign; md5sum of all six crate sources identical to the pre-mutation recording.

[vibechoom]

Fold-in — PR #1056 @ 8baa851

Folded the review's findings (#1056 (comment)) into two commits: d2c9a13 (the fixes) and 8baa851 (a correction to a doc comment I shipped in the first commit and then couldn't stand behind — see the NIT-2 row below).

Findings

#	file:line	change
MED-1	layout.rs:48-64 (GOLDEN_NARROW_MAJOR doc)	Value/comparison deliberately unchanged — still width < GOLDEN_BREAKPOINT (2560), still 75/25 at 2560 and wider. Rewrote the doc to say exactly that (it previously claimed a 1920- or 2560-wide screen gets the golden cut, which contradicts the code), and named the ambiguity out loud: a screen reporting exactly 2560 logical px — a 1440p monitor at 1×, or a 4K panel read back at 1.5× — reads as "wide" today. Mirrored the same note into docs/live-verify.md's (#1052) checklist item (docs/live-verify.md:458-463) so a live-verify pass surfaces the same open question rather than just quoting the tooltip. Pending your call on which side of 2560 that boundary should sit — nothing here presumes an answer.
MED-2	niri.rs:670-711 (new tests)	Added the reviewer's supplied killing test verbatim (golden_resolves_the_width_through_the_workspace_when_no_output_is_focused, niri.rs:670) — green on d7e690b+fix, and a sibling with two differently-sized outputs (golden_resolves_the_width_through_the_focused_workspaces_own_output_with_two_outputs, niri.rs:698) so a wrong-name resolution can't hide behind both names mapping to the same width. apply's own targeting line (niri.rs:88) needed no code change — it already calls layout::target_output_name, matching the PR body's claim — it was only unpinned.
LOW-1	niri.rs:37 (Transport::log), niri.rs:57-61 (SocketTransport::log), niri.rs:115-124 (missing_width_diagnostic), niri.rs:625-640 (test)	The fallback diagnostic now builds its pair text from GOLDEN_WIDE_MAJOR/GOLDEN_WIDE_MINOR instead of a hand-typed "(75 % / 25 %)", and is pinned end-to-end via a Transport::log seam mirroring watch.rs's existing Backend::log capture pattern (Fake::logs, niri.rs mod fake).
LOW-2	layout.rs:967-991 (new test)	hardcoded_breakpoint_strings_track_the_constant checks cli::USAGE, Layout::Golden.tooltip(), and main.rs's module docs (via include_str!) against GOLDEN_BREAKPOINT.to_string() directly — no new dependency (no const_format/formatcp in the workspace), so the &'static str literals stay literal but a bump to the constant that misses updating one of them now fails in this test rather than shipping quietly. Left docs/live-verify.md's own quoted copy out of the automated check (it lives outside the crate's include_str!-safe surface and isn't part of the crane source filter for this crate) — flagged instead in the MED-1 live-verify note above.
NIT-1	niri.rs:516-538 (queries_all_four_snapshots_before_acting)	Now asserts the first four entries of the raw, unfiltered seen log instead of the action-filtered queries() view, so an action sent between two snapshots is visible to the "before acting" half of the test's name.
NIT-2	niri.rs:157-167 (outputs()), layout.rs:280-291 (logical_width_of)	Both keep niri's Outputs reply as the HashMap<String, Output> it already is, keyed by connector name, instead of flattening to a Vec and linear-searching by Output.name — drops the assumption that the map key and that field never diverge, and the O(n) scan. Caveat I want to be upfront about: I initially wrote the MED-2 sibling test's doc comment claiming this made that test's determinism-against-a-wrong-output-mutant land — checked it empirically (temporarily reverted just the Vec/HashMap choice, kept the test) and that claim didn't hold: a keyed lookup by the correct name is order-independent either way, so NIT-2 doesn't change that test's determinism. What NIT-2 actually buys is the assumption removal and the scan, not that. Fixed the comment in 8baa851 rather than ship the overclaim.
NIT-3	niri.rs:573-596 (equal_and_split_ignore_the_output_width)	Added a literal [(10, 50.0), (20, 50.0)] assertion alongside the existing narrow-vs-wide comparison, on both the Equal and Split halves, so the test can't pass purely by both sides being wrong the same way.

Mutation table (fresh, d7e690b + this fold-in, md5 verified back to baseline after every one)

#	mutation	before this fold-in	after
M1	flip the breakpoint comparison (< → >=)	5 red	7 red (2 new MED-2 tests also hit it)
M2	swap the wide/narrow pairs	8 red	10 red (same reason)
M3	drop the Outputs request (outputs() returns empty)	4 red	4 red (same tests, different names)
M4a	GOLDEN_BREAKPOINT 2560 → 2561	2 red	3 red — hardcoded_breakpoint_strings_track_the_constant now also fires (LOW-2 fixed)
M4b	boundary inclusive (< → <=)	1 red	1 red (unchanged)
M5	logical_width_of reads logical.height	2 red	2 red (unchanged)
M6	delete the missing-width diagnostic block	0 red	1 red — golden_logs_the_fallback_pair_from_the_constants_when_width_is_unknown (LOW-1 fixed)
M7	apply resolves width off output.as_deref() instead of target_output_name(...)	0 red	1 red — golden_resolves_the_width_through_the_workspace_when_no_output_is_focused (MED-2 fixed)
M8	logical_width_of ignores name, takes any output	2 red (unit only)	2 red (unit only — see the NIT-2 caveat above, this class stays probabilistic at the apply level by design)
M9	send an Action between the Workspaces and Outputs fetches	0 red	1 red (+5 collateral in wire-byte tests, expected) — queries_all_four_snapshots_before_acting (NIT-1 fixed)

Gates (foreground, in the devShell, RUSTC_WRAPPER unset)

gate	exit
nix fmt -- --no-cache (first pass, reformatted 1 file; second pass, 0 changed)	0
cargo clippy -p hytte-plugin-niri-layouts --all-targets -- -D warnings	0
cargo clippy --workspace --all-targets --features system-tests -- -D warnings	0
cargo test -p hytte-plugin-niri-layouts, run ×6 for the NIT-2 determinism check	0 every run — 105 passed, 0 failed each time (up from 101: +4 new tests)

🤖 Generated with Claude Code

https://claude.ai/code/session_0157rpXf1ks2hZEfdB47UJ6v