From fa120958736caa7dca59bec3ea53f8a5ec35562c Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 1 Aug 2026 18:28:05 -0400 Subject: [PATCH 1/9] =?UTF-8?q?docs:=20frame=20bottom-panel=20Stage=203=20?= =?UTF-8?q?=E2=80=94=20the=20adopter=20default=20flip?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Arc 7's last step. Stages 1, 2A, 2B-1, 2B-2 and 2B-3 are all on main; the panel is complete on both frontends and every mechanism this stage needs exists. What remains is the decision the arc deferred on purpose: omitting `display` should mean the panel, not the selected window. Revision 2, approved with amendments. The scout found the parent framing's plan sound and two of its details drifted. THE FLIP IS THREE SITES IN TWO LANGUAGES, and a fourth validator that must not move. `parse_adopter_placement` reads like the shared parser its doc comment implies but has EXACTLY ONE CALLER, the terminal; listview and compile each re-implement the same three-value validation in Lua, including their own copy of the error message. Changing only the Rust parser would leave both Lua adopters resolving omission to the current window — a half-flip that looks done and behaves inconsistently per adopter. DIRED IS A FOURTH COPY OF THE VALIDATOR AND MUST KEEP `"current"`. Review caught this; revision 1 had it as a leaning. The mechanism is specific rather than stylistic: `pmacs.path.set_directory_handler` passes `{ dest = dest }` with NO `display` key, so `pmacs .` resolves by omission. Flipping dired's default would open the golden journey's first directory listing in a bottom panel. The distinction that makes this principled rather than an exception: listview, compile and terminal produce output the user CONSULTS; dired produces a document the user WORKS IN. The panel default is right for the first kind and wrong for the second. UNIFY NARROWLY, NOT WHOLESALE. A shared `resolve_adopter_display(operation, raw, default)` owns the vocabulary, the error text and the default policy, with dired passing `default = "current"` so its exemption is a parameter rather than a divergent copy. Terminal's `window` mutual-exclusion stays in its Rust wrapper — the parsers are not identical and a helper pretending otherwise would be its own defect. One normalization is named rather than absorbed: terminal reads `get::>("display")?`, so a non-string value raises mlua's TYPE error before the custom "unknown display" message, while the Lua callers `tostring()` it into their own. Different observable behaviour for the same bad input; unifying the error text without deciding this would silently change one of them. `select` IS A CORRECTNESS REQUIREMENT, and Q#BP12's citation drifted. `seat_cursor` is at listview.lua:130, not :64 — :64 is now a name-variant constant. The constraint holds and is worse than "focus preference": `seat_cursor` drives `pmacs.editor.move_down()`, which acts on the ACTIVE window, so an unselected listview panel seats the cursor in the user's document. That is why the table's `select` differs per adopter — listview true, compile false, terminal true — and why a uniform value would look right and break compile's passive output. TESTS ARE MEASURED BEFORE THE FLIP, NOT AFTER. Two Stage 1 acceptance tests assert the old default deliberately: acc19 (:1223) opens a listview with no `display` specifically to seed a DOCUMENT window "so side-affine placement cannot be vacuous" — the flip breaks its anti-vacuity guarantee, not merely its assertion — and acc19b (:1308) is built around a recompile reaching `start_run` with no `display`. The sweep rule is stated: a test whose SUBJECT is placement asserts the new default; a test whose subject is compile or terminal behaviour opts out only when its setup genuinely requires the document window. Mass-adding `display = "current"` to make suites green would convert a behavioural change into an invisible one. Acceptance gains a NEGATIVE criterion (9): omitted `display` still means document placement for direct dired AND for `pmacs .`. That is the criterion that catches a well-intentioned "make all four consistent" change. Framing only. No code, no flip, no test changes. Co-Authored-By: Claude Opus 5 (1M context) --- docs/bottom-panel-stage3-framing.md | 376 ++++++++++++++++++++++++++++ 1 file changed, 376 insertions(+) create mode 100644 docs/bottom-panel-stage3-framing.md diff --git a/docs/bottom-panel-stage3-framing.md b/docs/bottom-panel-stage3-framing.md new file mode 100644 index 0000000..51f19d6 --- /dev/null +++ b/docs/bottom-panel-stage3-framing.md @@ -0,0 +1,376 @@ +# Framing — Bottom panel Stage 3: the adopter default flip + +**Revision 2.** Status: **approved with amendments** (revision 1 → 2 +records them). Scouted against `githubsucks/main` @ `000b6cd` (v1.1.0). + +**Revision 1 → 2**, all from review and all verified rather than +accepted: + +- **Dired is a FOURTH copy of the validator, not merely a fourth + adopter** — and it must keep `"current"` as its default, because the + `pmacs .` path reaches it with no `display` at all (§1.1a). Revision 1 + had this as an open question with a leaning; it is now a decision with + a mechanism. +- **Unify narrowly, not wholesale** (§2 Q#S3-1). A shared + `resolve_adopter_display(operation, raw, default)` covering vocabulary, + error text and default policy — with terminal's `window` + mutual-exclusion staying in its Rust wrapper, because the parsers are + *not* identical and pretending otherwise is its own defect. +- **A negative criterion is added** (acceptance 9): omitted `display` + must still mean document placement for direct dired **and** for + `pmacs .`. +- **The suite fallout is named, not just counted** (§1.6a): two Stage 1 + tests assert the old default deliberately and must be revised + knowingly. + +Stage 3 is **Arc 7's last step**. Stages 1, 2A, 2B-1, 2B-2 and 2B-3 are +all on `main` (#155, #175, #177, #184, #187, #198); the panel is complete +on both frontends and every mechanism this stage needs exists. What +remains is the decision the arc deferred on purpose: **omitting `display` +should mean the panel, not the selected window.** + +The parent framing already decided the policy (Q#BP12) and wrote the +acceptance sketch (criteria 56–58). This document re-scouts that plan +against current `main` and records where it has drifted. + +--- + +## 0. Coherence impact (COHERENCE §20) + +- **Concern: §14 Coherent Workbench Primitives**, graded *"partial, with + the best trajectory of any concern."* This closes the panel half of + P5. §14's bottom-panel bullet names Stage 3 explicitly as what is + left. +- **§6 Interaction islands — this REMOVES one.** Today every adopter + that wants a panel must say so at each call site, and three separate + code paths decide what silence means. After Stage 3 the policy is the + default and the call sites stop carrying it. +- **Journey steps touched:** none directly, though steps 6 and 9 (LSP + panels, compile output) change where their output lands. +- **Config registry adoption:** none — Q#BP12 is explicit that this is + **not a hidden global setting**. It is a resolved default, not a + preference. +- **Background-work attribution:** none. +- **Enables:** DAP. Q#BP12's adopter table **already contains a + `DAP stack/variables` row**, so this stage settles the debugger's panel + policy before the debugger exists. That is the intended order. + +--- + +## 1. Ground truth (measured at `000b6cd`) + +### 1.1 The flip is three sites in two languages + +The parent framing says *"Stage 3 is not one line per consumer."* True — +but the sharper reason is that **the option was never parsed in one +place.** Each adopter validates and dispatches `display` itself: + +| site | adopter | omission today | +|---|---|---| +| `src/lua_bindings/window_panel.rs:269` | `pmacs.terminal.open` | `None \| Some("current") => AdopterPlacement::Current` | +| `builtin/runtime/listview.lua:235` | listview | `if display == "panel" … else switch_buffer` | +| `builtin/runtime/compile.lua:899` | compile | same, plus an `already_in_panel` special case | + +A **fourth** copy validates the same vocabulary without being a +default-flip site: `builtin/runtime/dired.lua:645`. It is the one that +must NOT flip (§1.1a). + +`parse_adopter_placement` looks like the shared parser its doc comment +implies, but **it has exactly one caller** — the terminal. listview and +compile each re-implement the same three-value validation in Lua, +including their own copy of the error message. + +**The three default-resolution branches are the flip; the fourth +validator is not.** `window_panel.rs:269`, `listview.lua:235` and +`compile.lua:899` all move. Changing only the Rust parser would leave +**both** Lua adopters resolving omission to the current window — a +half-flip that would look done and behave inconsistently per adopter. + +Four copies of one rule is also how the next adopter gets it subtly +wrong — and the next adopter is **DAP**, already named in Q#BP12's +table. Hence Q#S3-1's narrow unification. + +### 1.1a Dired must NOT flip, and the reason is the golden journey + +`dired.lua:645` validates the same `"current" | "panel"` vocabulary with +the same error shape, so it is a fourth copy of the rule. **Its default +must stay `"current"`**, and the mechanism is specific rather than +stylistic: + +```lua +pmacs.path.set_directory_handler(function(path, dest) + open_async(path, { dest = dest }, nil, "dired") +end) +``` + +The `pmacs .` path reaches dired through that slot with **`{ dest = +dest }` and no `display` key at all** — so it resolves by omission. If +dired's default flipped with the others, **`pmacs .` would open the +directory listing in a bottom panel**, which is wrong for journey step +2 and for every subsequent step that navigates from it. Journey Stage +1a made `pmacs .` open a directory at all; putting the result in a +panel would undo the point of it. + +This is the difference between an adopter and a surface. listview, +compile and terminal produce *output the user consults*; dired produces +*a document the user works in*, like a buffer. The panel default is +right for the first kind and wrong for the second. + +### 1.2 `select` is not cosmetic for listview, and the citation drifted + +Q#BP12 requires `select = true` for interactive listview, citing +`listview.lua:64` for `seat_cursor`. **That line is now +`NAME_VARIANT_LIMIT`; `seat_cursor` is at line 130.** The constraint +itself is intact and verified: + +```lua +local function seat_cursor(p, line) + ... + for _ = 1, target do + pmacs.editor.move_down() + end +``` + +`pmacs.editor.move_down()` acts on the **active window**. A listview +panel displayed without `select` would seat the cursor in whatever +window is selected — the user's document. `listview.refresh` has the +same property. **This is a data-corruption-shaped bug, not a focus +annoyance**, and it is why the table's `select` column differs per +adopter rather than being uniform. + +Compile takes `select = false` deliberately (passive output); terminal +takes `select = true`. + +### 1.3 Compile has already been prepared for this stage + +`compile.lua:894` carries a comment written for Stage 3: + +> Gated on OMISSION, never on an explicit value: `display = "current"` +> is the documented user-facing opt-out from the Stage 3 default flip, +> so it must reach the raw switch even when the previous run was +> panel-placed. + +and the condition is already +`display == "panel" or (display == nil and already_in_panel(slot.buf))`. +So compile's *recompile* path already keeps a panel-placed buffer in the +panel on omission. **Stage 3 makes the first run behave like the +recompile.** Re-read this comment before editing: it encodes a +distinction (omission vs explicit `"current"`) that the flip must +preserve, and the `already_in_panel` branch may become redundant. + +### 1.4 What Stage 3 owes beyond the flip + +Q#BP12's table is a per-adopter contract, not a single switch. For each +of listview / compile / terminal: + +- **panel placement** with that adopter's `select` value; +- **`dedicated = false`** — a dedicated panel refuses to host anything + else, and dired Stage 1 already established that `display_buffer` will + not replace a buffer in a slot dedicated to another (Q#BP3 2.iii); +- **quit action**: delete the panel if this adopter created it, restore + the panel it replaced otherwise; +- **visit path onto `display_file` / `display_target`** with `select` + per the table. + +`display_file` already exists and is used by dired (`dired.lua:831`, +`:840`) and `default.lua:729`, so the visit half has a proven caller +shape to copy. + +### 1.5 Capability fallback still applies, and must be re-proved + +The Stage 3 default resolves as a *panel request*, so it passes through +Q#BP13 capability fallback exactly as an explicit `"panel"` does. On a +pre-panel semantic frontend the request degrades, and criterion 57 +requires that the degraded path leave **no side parameters and no quit +action on the document window**. + +This is the criterion most likely to be quietly wrong, because the +fallback is invisible from the adopter's side. + +### 1.6 What is NOT established + +- **Nothing has been implemented or measured.** Unlike the distribution + lane, there is no artifact to inspect; the evidence is the existing + suites' behaviour before and after. +- **The blast radius on existing acceptance suites is partly known + now** — see §1.6a for the two named cases. The total is still + unmeasured. Stage 1 + shipped the mechanism opt-in precisely so existing suites kept their + meaning. Flipping the default changes where output lands for every + suite that exercises listview, compile or terminal **without** passing + `display`. Criterion 58 says the default-placement suites are + *updated*; how many others move is a measurement this framing has not + taken and the implementation must take first. +- **Interaction with dired.** `dired.lua:18` documents + `opts.display = "current" | "panel"` with **default `"current"`**. + Dired is not in Q#BP12's table. Whether the flip reaches it, or dired + keeps an explicit `"current"`, is Q#S3-2. + +--- + +### 1.6a Two Stage 1 tests assert the OLD default deliberately + +These are not collateral damage; they encode intent and must be revised +knowingly. + +- **`tests/bottom_panel_stage1_acceptance.rs:1223`** — + `acc19_adopters_place_side_affinely_through_real_entry_points` opens a + listview with **no `display`** specifically to seed the panel buffer + into a DOCUMENT window first, *"so side-affine placement cannot be + vacuous."* After the flip that setup no longer produces a document + window, and the test's own anti-vacuity guarantee is what breaks. It + needs a new way to seed, not a `display = "current"` bolted on. +- **`tests/bottom_panel_stage1_acceptance.rs:1308`** — + `acc19b_recompile_reuses_the_panel_instead_of_duplicating_into_the_document` + is built entirely around a recompile reaching `start_run` with no + `display`. Its subject survives the flip, but its mechanism (`§1.3`'s + `already_in_panel` gate) may not — see Q#S3-3. + +**The rule for the fallout sweep:** a test whose *subject* is placement +must assert the **new** default. A test whose subject is compile or +terminal behaviour opts out with `display = "current"` **only when its +setup genuinely requires the document window**. Mass-adding the opt-out +to make a suite green converts a behavioural change into an invisible +one, which is the failure this stage's inverted ordering exists to +avoid. + +## 2. Questions + +- **Q#S3-1 — DECIDED: unify NARROWLY.** A shared + `resolve_adopter_display(operation, raw, default)` owns exactly three + things: the **vocabulary**, the **error text**, and the **default + policy**. Its four callers are listview, compile, terminal and dired — + the last passing `default = "current"`, which is what makes dired's + exemption a parameter rather than a divergent copy. + + **Terminal's `window` mutual-exclusion stays in its Rust wrapper.** + The parsers are *not* identical, and a helper that pretended otherwise + would be its own defect: only terminal accepts a `window` id, and only + it must reject `window` combined with `display = "panel"`. + + **One normalization must be named rather than absorbed.** Terminal + reads `spec_table.get::>("display")?`, so a non-string + value raises **mlua's type error before** the custom "unknown display" + message is ever reached. The Lua callers instead `tostring()` whatever + they got and report it inside their own error. These are different + observable behaviours for the same bad input, and unifying the error + text without deciding this would silently change one of them. The + stage must state which it standardizes on and pin it. +- **Q#S3-2 — DECIDED: dired does not flip.** It keeps `"current"` as + its default, expressed as the `default` argument to the shared + resolver so the exemption is visible at the call site rather than + implied by a fourth copy of the parser. §1.1a records the mechanism — + the `pmacs .` handler passes no `display` — and acceptance 9 pins it + from both entry points. +- **Q#S3-3 — does `already_in_panel` survive?** Once omission means + panel, compile's special case may be dead code. **Leaning: measure, + then delete if dead** — but check the explicit-`"current"` path first, + because that arm is what the comment says the gate protects. +- **Q#S3-4 — how many existing suites move?** Unknown (§1.6). This must + be measured **before** the flip, so the diff to acceptance files can be + read as intended-vs-collateral rather than discovered afterwards. +- **Q#S3-5 — is there a user-facing escape beyond per-call `display`?** + Q#BP12 says this is deliberately not a setting. Someone who dislikes + panels has no global opt-out, only per-call `display = "current"`, + which they do not control for builtin commands. **Leaning: accept for + this stage and record it**, since a setting is §11 work and panel + persistence is already blocked on settings persistence. + +--- + +## 3. Bets + +- **Bet 1 — the flip itself is small; the suite churn is the work.** + Three dispatch sites, each a few lines. The cost is criterion 58's + per-adopter open→visit→return→quit suites plus whatever §1.6's + measurement turns up. +- **Bet 2 — the capability-fallback criterion (57) is where a defect + hides.** It is invisible from the adopter side and only observable on a + pre-panel semantic frontend. +- **Bet 3 — `select` gets one adopter wrong.** The values differ per + adopter for real reasons (§1.2), and a uniform `select = true` would + look correct and break compile's passive-output behaviour. + +--- + +## 4. Acceptance + +Inherits the parent framing's criteria 56–58, made concrete: + +1. Omitting `display` from listview, compile and terminal entry points + resolves to the Q#BP12 panel/select policy on a panel-capable grid + **and** semantic frontend. +2. Explicit `display = "current"` preserves each adopter's pre-arc + selected-window behaviour, including compile's raw-switch path. +3. **Per-adopter `select` matches the table** — listview `true`, + compile `false`, terminal `true` — asserted individually, not by a + shared helper that would pass with a uniform value. +4. An interactive listview panel seats its cursor **in the panel**, not + in the previously selected window (§1.2's real failure mode). +5. On a pre-panel semantic frontend the omitted default takes capability + fallback with **no side parameters and no quit action** left on the + document window; visit and `q` remain the existing non-side paths. +6. Per-adopter open→visit→return→quit suites, not a generic helper + (criterion 58), preserving Stage 1's unknown-value rollback + assertions. +7. **The unknown-`display` error still fires before anything is + created** — buffer, session, process or wrapper — for all three + adopters, whichever parser survives Q#S3-1. +8. The count of existing suites whose behaviour changes is **stated**, + and each change is classified intended or collateral — **not + silenced by mass-adding `display = "current"`** (§1.6a). +9. **NEGATIVE criterion — omission still means the document for dired.** + Both entry points are pinned: a direct `pmacs.dired.open(path)` with + no `display`, **and** the `pmacs .` launch path through + `pmacs.path.directory_handler`. Neither may place into a panel. This + is the criterion that would catch a well-intentioned "make all four + consistent" change, and it guards journey step 2. + +--- + +## 5. Parked + +- Everything in the parent framing's §6 "Deferred (named)" — left/right/ + top side windows, multiple slots, `no_other_window`, manual + hide/show, `display-buffer-alist`-style user rules, panel persistence + (blocked on settings persistence), GPU document splits. +- **A global panel preference** (Q#S3-5) — §11 work. +- **The tree primitive.** Not part of this stage, and the next thing to + scope: §14 grades Tree ✗, and DAP's variables view is its next + would-be inventor. + +--- + +## 6. Gates + +The standing `CLAUDE.md` suite, with the touched acceptance suites being +at minimum `bottom_panel_stage1_acceptance`, `bottom_panel_stage2a`, +both `stage2b` daemon/GPU suites, plus the listview, compile-mode and +terminal suites — the last three are where §1.6's unmeasured churn will +land. + +--- + +## 7. Branch plan + +One branch, `bottom-panel-stage3`: + +1. **Measure first** (Q#S3-4): run the full suite with the flip applied + as a throwaway edit, record which suites move, revert. *The + measurement is the first commit's evidence, not the flip.* Classify + each mover per §1.6a's rule before writing a line of the fix. +2. **Land `resolve_adopter_display`** (Q#S3-1) with all four callers + still passing their CURRENT defaults, so the unification is provably + behaviour-preserving before anything flips. Decide and pin the + non-string normalization here. +3. **Flip the three sites** by changing only the `default` argument at + listview, compile and terminal — dired keeps `"current"` — with + per-adopter `select`. +4. **Per-adopter open→visit→return→quit suites** (criterion 6). +5. **Capability fallback** (criterion 5), the one needing a semantic + frontend. +6. **Update the lane and handoff**; Arc 7 closes. + +Step 1 before step 3 is the point: flipping first and reading the +fallout as it appears makes intended and collateral changes +indistinguishable. From 0224c68b7356315ec01b648de66e30fcebc28758 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 1 Aug 2026 19:55:44 -0400 Subject: [PATCH 2/9] =?UTF-8?q?docs(stage3):=20the=20fallout=20census=20?= =?UTF-8?q?=E2=80=94=2037=20failures,=20and=20a=20transitive=20adopter?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Branch-plan step 1, whose whole point is that it precedes the flip. The throwaway edit is NOT in this commit; only the measurement survives it. A CENSUS THAT STOPS AT THE FIRST FAILING BINARY IS NOT A CENSUS. The first sweep reported 2 failures in 1 suite and looked comfortingly small — `cargo test` halts after a failing test binary, so everything alphabetically past bottom_panel_stage1_acceptance never ran. With --no-fail-fast the real figure is 37 failures across 5 suites. Any re-measurement must pass that flag or under-report by an order of magnitude, which is the same silent-success shape this arc keeps finding. M4_ACCEPTANCE WAS PREDICTED BY NOBODY — not the parent framing, not Q#BP12's adopter table, not revision 1. Its two failures are the LSP hover and outline panels, which are LISTVIEW CONSUMERS, so flipping listview's default reaches the LSP suite transitively. Q#BP12's four rows are the DIRECT population; the real one is everything built on listview. That is the intended behaviour, but the map has to say so or the next listview-based feature inherits a default nobody documented. THE PROPORTIONS INVERT THE OBVIOUS READING. compile_mode has the most failures (17) and the least placement content — process-lifecycle and styling tests that merely use compile and now find its output elsewhere. listview_acceptance has fewer (13) but loses THREE QUARTERS of its suite, and its failures are placement in substance. Classification decided per test, not per suite, because two neighbouring compile tests land on opposite sides: acc15 (RET-visits-error, jump-back) is placement — Q#BP12 explicitly requires panel → RET source → M-, back to the still-present panel with the document window intact — while acc16 (n/p walk within compile output) genuinely needs the compilation buffer selected and takes an explicit display = "current". A sweep classifying per suite would have got both wrong. Both M4 failures are placement-subject: outline_panel_opens_visits_and_ restores is almost a direct realization of criterion 58 (open → visit → jump-back → quit), and hover_doc_panel keeps the omitted default while asserting the new placement plus its existing content and quit guarantees. Neither takes display = "current". Tree verified clean after revert: the three default-resolution branches are unchanged on disk. Co-Authored-By: Claude Opus 5 (1M context) --- docs/bottom-panel-stage3-framing.md | 89 +++++++++++++++++++++++++++-- 1 file changed, 84 insertions(+), 5 deletions(-) diff --git a/docs/bottom-panel-stage3-framing.md b/docs/bottom-panel-stage3-framing.md index 51f19d6..52fdd6c 100644 --- a/docs/bottom-panel-stage3-framing.md +++ b/docs/bottom-panel-stage3-framing.md @@ -1,7 +1,14 @@ # Framing — Bottom panel Stage 3: the adopter default flip -**Revision 2.** Status: **approved with amendments** (revision 1 → 2 -records them). Scouted against `githubsucks/main` @ `000b6cd` (v1.1.0). +**Revision 3.** Status: **approved; branch-plan step 1 (the fallout +census) is complete and recorded in §1.6b–c.** Branch +`bottom-panel-stage3`, based on `githubsucks/main` @ `21de0b2`. + +**Revision 2 → 3** adds the measurement and its classification. Two +findings changed the plan rather than confirming it: the census needs +`--no-fail-fast` or it under-reports by an order of magnitude, and +**`m4_acceptance` is a transitive adopter via listview that no table +named**. **Revision 1 → 2**, all from review and all verified rather than accepted: @@ -192,9 +199,8 @@ fallback is invisible from the adopter's side. - **Nothing has been implemented or measured.** Unlike the distribution lane, there is no artifact to inspect; the evidence is the existing suites' behaviour before and after. -- **The blast radius on existing acceptance suites is partly known - now** — see §1.6a for the two named cases. The total is still - unmeasured. Stage 1 +- ~~The blast radius on existing acceptance suites is unmeasured.~~ + **MEASURED — see §1.6b: 37 failures across 5 suites.** Stage 1 shipped the mechanism opt-in precisely so existing suites kept their meaning. Flipping the default changes where output lands for every suite that exercises listview, compile or terminal **without** passing @@ -234,6 +240,79 @@ to make a suite green converts a behavioural change into an invisible one, which is the failure this stage's inverted ordering exists to avoid. +### 1.6b The fallout census — MEASURED, and it found an adopter nobody named + +Taken as branch-plan step 1: record a baseline, apply the three-site +flip as a throwaway edit, sweep, revert. The flip is **not** in the +tree; only this table survives it. + +**A census that stops at the first failing binary is not a census.** +The first sweep reported **2 failures in 1 suite** and looked +comfortingly small — `cargo test` halts after a failing test binary, so +everything alphabetically past `bottom_panel_stage1_acceptance` never +ran. With `--no-fail-fast` the real figure is **37 failures across 5 +suites**. Any future re-measurement must pass that flag or it will +under-report by an order of magnitude, in the same shape as this arc's +other silent-success traps. + +| suite | base → flipped | failures | share of suite | +|---|---|---:|---:| +| `listview_acceptance` | 17 → 4 | **13** | **76%** | +| `compile_mode_acceptance` | 72 → 55 | **17** | 24% | +| `vterm_stage2_acceptance` | 6 → 3 | **3** | 50% | +| `bottom_panel_stage1_acceptance` | 46 → 44 | **2** | 4% | +| `m4_acceptance` | 149 → 147 | **2** | 1% | +| | | **37** | | + +**`m4_acceptance` was predicted by nobody** — not the parent framing, +not Q#BP12's adopter table, not revision 1 of this document. Its two +failures are `hover_doc_panel_shows_full_contents_via_binding` and +`outline_panel_opens_visits_and_restores`: **the LSP panels are +listview consumers**, so flipping listview's default reaches the LSP +suite transitively. + +This materially improves the adopter map. Q#BP12 lists four rows +(listview, compile, terminal, DAP) as though they were the population. +They are the *direct* population; **the real one includes everything +built on listview**, and the LSP hover/outline panels are the proof. +Anything added on listview later inherits the panel default without +appearing in any table — which is the intended behaviour, but only if +the map says so. + +**The proportions invert the obvious reading.** `compile_mode` has the +most failures and the least placement content: `acc01_spawn_streams…`, +`acc05_kill_reaps_backgrounded_descendant`, +`acc21_kill_produces_signaled_marker` and the `r1f*`/`r5f*`/`r6f*` group +are process-lifecycle and styling tests that merely *use* compile and +now find its output elsewhere. `listview_acceptance` has fewer failures +but loses **three quarters of its suite**, and its failures +(`open_seats_cursor_and_ret_visits_the_row`, `panel_rejects_typing`, +`dispatch_idle_is_false_while_a_panel_is_focused`) are placement in +substance. + +### 1.6c Classification, decided + +Applying §1.6a's rule to the measured set. **Placement-subject tests +assert the NEW default; only genuine document-window setups opt out.** + +| test | classification | +|---|---| +| `bottom_panel_stage1::acc19` | **placement** — needs a new way to seed a document window (§1.6a) | +| `bottom_panel_stage1::acc19b` | **placement** — subject survives; mechanism may not (Q#S3-3) | +| `m4::outline_panel_opens_visits_and_restores` | **placement** — *almost a direct realization of criterion 58*: open → visit → jump-back → quit. Assert the panel/document split | +| `m4::hover_doc_panel_shows_full_contents_via_binding` | **placement** — a listview panel lifecycle test. Keep the omitted default; assert the new placement **plus** its existing content and quit guarantees | +| `compile::acc15_ret_visits_error_and_jump_back_returns` | **placement** — Q#BP12 explicitly requires compilation panel → RET source → `M-,` back to the still-present panel with the document window intact | +| `compile::acc16_n_p_walk_error_lines_without_wrap` | **opt out**, explicit `display = "current"` — its subject is cursor navigation *within* compile output, which genuinely needs the compilation buffer selected | +| `listview_acceptance` ×13 | **placement**, predominantly — assert the new default | +| remaining `compile_mode` ×15 | **incidental** — process lifecycle and styling; opt out only where the setup needs the document window | +| `vterm_stage2` ×3 | **to classify at implementation** — terminal's `select = true` makes these likelier placement than incidental | + +The `acc15` / `acc16` split is the one worth remembering: **two +neighbouring tests in the same suite land on opposite sides**, because +one is about where output goes and the other is about moving within it. +A sweep that classified per *suite* rather than per *test* would have +got both wrong. + ## 2. Questions - **Q#S3-1 — DECIDED: unify NARROWLY.** A shared From 41d37fcbcb100bc43902a9ce55aadf0cd8bcbca9 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 1 Aug 2026 20:13:03 -0400 Subject: [PATCH 3/9] refactor(panel): one shared rule for the adopter `display` vocabulary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage 3 step 2 (Q#S3-1). DEFAULT-PRESERVING WITH ONE INTENTIONAL NORMALIZATION — not "behaviour-preserving", which would be too broad a claim. Every adopter keeps its current default, and the full serialized suite is 3447 passed / 0 failed with ZERO suites differing from the pre-change baseline. But invalid-input behaviour DID move, deliberately, and that is pinned rather than asserted in prose. Before this, FOUR adopters validated the same three-value vocabulary in four places: Rust for the terminal, and hand-written Lua copies in listview.lua, compile.lua and dired.lua, each carrying its own copy of the error string. `parse_adopter_placement` read like the shared parser its doc comment implied but had exactly one caller. Four copies of one rule is how the next adopter gets it subtly wrong, and the next adopter is DAP. `resolve_adopter_display(operation, raw, default)` now owns exactly three things: the vocabulary, the error text, and the default policy. Reachable from Lua as the internal seam `pmacs.window._resolve_display`. THE DEFAULT IS A PARAMETER, NOT A CONSTANT, and that is load-bearing rather than stylistic. listview/compile/terminal will resolve omission to the panel in step 3; DIRED MUST NOT, because `pmacs.path.set_directory_handler` calls it with `{ dest = dest }` and no `display` key at all — a flipped default would open `pmacs .` in a bottom panel. Passing the default in makes dired's exemption visible at its call site instead of hidden in a divergent copy. TERMINAL'S `window` MUTUAL-EXCLUSION STAYS IN ITS OWN WRAPPER. Only the terminal accepts a `window` id and only it must reject `window` combined with `display = "panel"`. A helper pretending the four parsers were identical would be its own defect. THE NORMALIZATION, DECIDED AND PINNED. Terminal read `get::>("display")?`, so a non-string value raised mlua's TYPE error before reaching any custom message, while the Lua copies stringified it into their own. Nothing pinned either behaviour — every existing assertion passes an unknown STRING, which takes the same path under both designs and therefore could not have caught a regression here. The custom error wins because it names the legal vocabulary; the value is rendered by TYPE ALONE (`unknown display (integer)`) so the message cannot imply a string was passed. Pinned at the terminal entry point in acc19 — the one adopter whose behaviour changed — asserting the shared error AND that nothing is created. The type SPELLING is deliberately not pinned: Lua 5.4 says `integer` where LuaJIT has no integer subtype, so asserting either literal would pass on one CI flavor and fail on the other. Verified 46/46 under both. COMPILE NEEDED AN EXPLICIT OMISSION CAPTURE, and finding that out is what this step is for. The resolver collapses omission into its default, but compile's recompile gate distinguishes them: it fires on OMISSION only, never on an explicit `display = "current"`, which is the documented opt-out and must reach the raw switch even when the previous run was panel-placed. Resolving first and testing `== "current"` afterwards would have silently merged the two and broken the opt-out with every test still green. `display_omitted` is captured before the resolver call and the gate keys on it. Co-Authored-By: Claude Opus 5 (1M context) --- builtin/runtime/compile.lua | 19 ++-- builtin/runtime/dired.lua | 12 ++- builtin/runtime/listview.lua | 10 +- docs/bottom-panel-stage3-framing.md | 48 ++++++--- src/lua_bindings/mod.rs | 8 +- src/lua_bindings/window_panel.rs | 138 ++++++++++++++++++++++-- tests/bottom_panel_stage1_acceptance.rs | 47 ++++++++ 7 files changed, 236 insertions(+), 46 deletions(-) diff --git a/builtin/runtime/compile.lua b/builtin/runtime/compile.lua index e6e85d2..1c2c674 100644 --- a/builtin/runtime/compile.lua +++ b/builtin/runtime/compile.lua @@ -803,12 +803,17 @@ local function start_run(slot, cmdline, opts) -- supersedes anything, rewrites the buffer, or spawns a process, so -- an unknown value leaves no half-started run behind. In Stages 1-2 -- omission means "current"; Stage 3 flips the default. - local display = opts.display - if display ~= nil and display ~= "current" and display ~= "panel" then - error(string.format( - "compile.run: unknown display %q (expected \"current\" or \"panel\")", - tostring(display))) - end + -- Q#S3-1: one shared rule for vocabulary, error text and default. + -- + -- `display_omitted` is captured SEPARATELY and deliberately. The + -- resolver collapses omission into its default, but the recompile gate + -- below distinguishes them: it fires on OMISSION only, never on an + -- explicit `display = "current"`, which is the documented opt-out and + -- must reach the raw switch even when the previous run was + -- panel-placed. Resolving first and testing `== "current"` afterwards + -- would silently merge the two and break that opt-out. + local display_omitted = opts.display == nil + local display = pmacs.window._resolve_display("compile.run", opts.display, "current") -- q-target discipline (Q#CM11): capture only when coming from a -- non-generated buffer, so `g` reruns don't re-capture and -- compile → g → q restores the original buffer. @@ -896,7 +901,7 @@ local function start_run(slot, cmdline, opts) -- so it must reach the raw switch even when the previous run was -- panel-placed. The duplicate presentation that produces is the -- escape hatch's documented cost (R3-rp2). - if display == "panel" or (display == nil and already_in_panel(slot.buf)) then + if display == "panel" or (display_omitted and already_in_panel(slot.buf)) then pmacs.window.display(slot.buf, { side = "bottom", select = false }) else pmacs.window.switch_buffer(slot.buf) diff --git a/builtin/runtime/dired.lua b/builtin/runtime/dired.lua index 11d78ce..f747f60 100644 --- a/builtin/runtime/dired.lua +++ b/builtin/runtime/dired.lua @@ -642,11 +642,13 @@ local function open_directory(path, opts, departed) error(string.format("pmacs.dired.open: unknown opts key %q", tostring(key))) end end - local wanted = opts.display - if wanted ~= nil and wanted ~= "current" and wanted ~= "panel" then - error(string.format('pmacs.dired.open: unknown display %q (expected "current" or "panel")', - tostring(wanted))) - end + -- Q#S3-1/§1.1a: the shared rule, with dired's default passed + -- EXPLICITLY as "current" and kept there through Stage 3. The + -- `pmacs.path.directory_handler` slot calls this with no `display` at + -- all, so flipping dired's default would open `pmacs .` in a bottom + -- panel. Dired produces a document the user works in, not output they + -- consult; the panel default is right for the latter only. + local wanted = pmacs.window._resolve_display("pmacs.dired.open", opts.display, "current") local canonical = canonicalize(path) -- Read first: a failure must leave no buffer, no window change, and diff --git a/builtin/runtime/listview.lua b/builtin/runtime/listview.lua index 3c83119..9c904a8 100644 --- a/builtin/runtime/listview.lua +++ b/builtin/runtime/listview.lua @@ -226,12 +226,10 @@ function pmacs.listview.open(spec) -- window. In Stages 1-2 omitting `display` keeps today's raw switch; -- Stage 3 flips the default. An unknown value errors before anything -- is displayed. - local display = spec.display - if display ~= nil and display ~= "current" and display ~= "panel" then - error(string.format( - "listview.open: unknown display %q (expected \"current\" or \"panel\")", - tostring(display))) - end + -- Q#S3-1: the vocabulary, the error and the default policy are one + -- rule (`window._resolve_display`), not a copy per adopter. The + -- default is passed in because the adopters do not share one. + local display = pmacs.window._resolve_display("listview.open", spec.display, "current") if display == "panel" then pmacs.window.display(p.buffer, { side = "bottom", select = true }) else diff --git a/docs/bottom-panel-stage3-framing.md b/docs/bottom-panel-stage3-framing.md index 52fdd6c..e7ba982 100644 --- a/docs/bottom-panel-stage3-framing.md +++ b/docs/bottom-panel-stage3-framing.md @@ -196,21 +196,17 @@ fallback is invisible from the adopter's side. ### 1.6 What is NOT established -- **Nothing has been implemented or measured.** Unlike the distribution - lane, there is no artifact to inspect; the evidence is the existing - suites' behaviour before and after. -- ~~The blast radius on existing acceptance suites is unmeasured.~~ - **MEASURED — see §1.6b: 37 failures across 5 suites.** Stage 1 - shipped the mechanism opt-in precisely so existing suites kept their - meaning. Flipping the default changes where output lands for every - suite that exercises listview, compile or terminal **without** passing - `display`. Criterion 58 says the default-placement suites are - *updated*; how many others move is a measurement this framing has not - taken and the implementation must take first. -- **Interaction with dired.** `dired.lua:18` documents - `opts.display = "current" | "panel"` with **default `"current"`**. - Dired is not in Q#BP12's table. Whether the flip reaches it, or dired - keeps an explicit `"current"`, is Q#S3-2. +- **The blast radius is MEASURED — §1.6b: 37 failures across 5 suites**, + classified per test in §1.6c. Stage 1 shipped the mechanism opt-in + precisely so existing suites kept their meaning; flipping the default + changes where output lands for every suite that exercises listview, + compile or terminal **without** passing `display`. +- **Steps 1 and 2 of §7 are done** (`0224c68`, `a2f4411`). The flip + itself, the test revisions, and the capability-fallback criterion are + not. +- ~~Interaction with dired.~~ **DECIDED — §1.1a and Q#S3-2: dired keeps + `"current"`**, passed explicitly to the shared resolver so the + exemption is visible at its call site. --- @@ -333,8 +329,26 @@ got both wrong. message is ever reached. The Lua callers instead `tostring()` whatever they got and report it inside their own error. These are different observable behaviours for the same bad input, and unifying the error - text without deciding this would silently change one of them. The - stage must state which it standardizes on and pin it. + text without deciding this would silently change one of them. + + **RESOLVED and PINNED at step 2.** The custom error wins, because it + names the legal vocabulary where mlua's type error does not. + Non-strings render by **type alone** (`unknown display (integer)`), + never quoted, so the message cannot imply a string was passed. Pinned + in `bottom_panel_stage1_acceptance::acc19` at the **terminal** entry + point — the one adopter whose behaviour actually changed — asserting + the shared error *and* that nothing is created, exactly as the + unknown-string case already does. + + **The type SPELLING is deliberately not pinned.** Lua 5.4 reports + `integer`; LuaJIT has no integer subtype. Asserting either literal + would pass on one CI flavor and fail on the other, so the test pins + the shape (`unknown display (`, the vocabulary, and the *absence* of a + quoted value). Verified 46/46 under both flavors. + + Consequently step 2 is **default-preserving with one intentional + normalization**, not "behaviour-preserving": every adopter kept its + default, but invalid-input behaviour moved on purpose. - **Q#S3-2 — DECIDED: dired does not flip.** It keeps `"current"` as its default, expressed as the `default` argument to the shared resolver so the exemption is visible at the call site rather than diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 2d4f359..0dfbd2d 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -9320,12 +9320,18 @@ fn install_terminal( // Bottom-panel arc (Q#BP11b): parse placement BEFORE the // session, process, buffer, or wrapper exists, so an // unknown `display` value creates nothing to roll back. + // Stage 3 step 2: the shared resolver, still carrying the + // PRE-FLIP default. The unification must be provably + // behaviour-preserving before the default moves, so the + // flip to `AdopterDefault::Panel` is its own commit. + let display_value = spec_table.get::("display")?; let placement = window_panel::parse_adopter_placement( &core, frontend_id, "pmacs.terminal.open", - spec_table.get::>("display")?.as_deref(), + Some(&display_value), spec_table.get::>("window")?, + window_panel::AdopterDefault::Current, )?; let buffer_id = { let mut manager = manager.borrow_mut(); diff --git a/src/lua_bindings/window_panel.rs b/src/lua_bindings/window_panel.rs index 1c700a5..667cb68 100644 --- a/src/lua_bindings/window_panel.rs +++ b/src/lua_bindings/window_panel.rs @@ -251,9 +251,97 @@ pub(crate) enum AdopterPlacement { Window(WindowId), } +/// The one rule for the adopter `display` vocabulary: which values are +/// legal, what the error says, and what omission means. +/// +/// **Bottom-panel Stage 3 (Q#S3-1).** Before this, four adopters +/// validated the same three-value vocabulary in three places — Rust for +/// the terminal, and hand-written Lua copies in `listview.lua`, +/// `compile.lua` and `dired.lua`, each with its own copy of the error +/// string. Four copies of one rule is how the next adopter gets it +/// subtly wrong, and the next adopter is DAP. +/// +/// `default` is a **parameter, not a constant**, because the adopters do +/// not share one. listview / compile / terminal resolve omission to the +/// panel; **dired resolves it to `"current"`** and must keep doing so — +/// `pmacs.path.directory_handler` calls it with no `display` at all, so +/// a flipped default would open `pmacs .` in a bottom panel (§1.1a). +/// Passing the default in is what makes dired's exemption visible at its +/// call site instead of hidden in a divergent copy. +/// +/// **Non-string values are reported as unknown, not as type errors**, +/// and this is a deliberate normalization (Q#S3-1). Terminal previously +/// read `get::>` and so raised mlua's type error *before* +/// reaching any custom message, while the Lua copies stringified and +/// reported their own. Nothing pinned either behaviour. The custom error +/// wins because it names the legal vocabulary and mlua's does not. +/// +/// # Errors +/// Any non-nil value that is not `"current"` or `"panel"`. +pub(crate) fn resolve_adopter_display( + operation: &str, + raw: Option<&mlua::Value>, + default: AdopterDefault, +) -> mlua::Result { + let raw = match raw { + None | Some(mlua::Value::Nil) => return Ok(default.placement()), + Some(value) => value, + }; + match raw.as_str().as_deref() { + Some("current") => Ok(AdopterPlacement::Current), + Some("panel") => Ok(AdopterPlacement::Panel), + _ => Err(mlua::Error::runtime(format!( + "{operation}: unknown display {} (expected \"current\" or \"panel\")", + display_for_error(raw) + ))), + } +} + +/// What omission means for one adopter. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub(crate) enum AdopterDefault { + /// listview, compile, terminal — Stage 3's flip. + Panel, + /// dired, and anything else whose output is a document the user + /// works in rather than output they consult. + Current, +} + +impl AdopterDefault { + fn placement(self) -> AdopterPlacement { + match self { + Self::Panel => AdopterPlacement::Panel, + Self::Current => AdopterPlacement::Current, + } + } +} + +/// Render a rejected `display` value for the error message. +/// +/// Strings are quoted so `display = "sideways"` reads as `"sideways"`; +/// anything else is shown **by type alone** — `unknown display +/// (number)` — because quoting a non-string as `"42"` would imply the +/// caller passed a string when they passed a number. +/// +/// The value itself is deliberately not interpolated: it is already +/// wrong, `mlua::Value`'s `Display` is not guaranteed useful for tables +/// or userdata, and the type is what tells the caller what to fix. +fn display_for_error(value: &mlua::Value) -> String { + value.as_str().map_or_else( + || format!("({})", value.type_name()), + |s| format!("{:?}", &*s), + ) +} + /// Parse an adopter's placement **before** it creates a buffer, session, /// process, or wrapper — so an unknown value leaves nothing to roll back. /// +/// Terminal-specific wrapper around [`resolve_adopter_display`]: only +/// the terminal accepts a `window` id, and only it must reject `window` +/// combined with `display = "panel"`. That asymmetry stays here rather +/// than in the shared resolver, because a helper that pretended the four +/// parsers were identical would be its own defect. +/// /// # Errors /// An unknown `display` value, a `window` combined with /// `display = "panel"`, or a window id that is not live in the acting @@ -262,18 +350,11 @@ pub(crate) fn parse_adopter_placement( core: &SharedCore, fid: FrontendId, operation: &str, - display: Option<&str>, + display: Option<&mlua::Value>, window: Option, + default: AdopterDefault, ) -> mlua::Result { - let display = match display { - None | Some("current") => AdopterPlacement::Current, - Some("panel") => AdopterPlacement::Panel, - Some(other) => { - return Err(mlua::Error::runtime(format!( - "{operation}: unknown display {other:?} (expected \"current\" or \"panel\")" - ))); - } - }; + let display = resolve_adopter_display(operation, display, default)?; match (window, &display) { (Some(_), AdopterPlacement::Panel) => Err(mlua::Error::runtime(format!( "{operation}: `window` and `display = \"panel\"` are mutually exclusive" @@ -481,6 +562,43 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result )?; } + { + // Q#S3-1 — the shared adopter-display rule, reachable from Lua. + // + // Underscore-prefixed because it is an internal seam between the + // builtin runtime modules and this one, not user-facing API: + // `listview.lua`, `compile.lua` and `dired.lua` call it instead + // of each keeping a hand-written copy of the same three-value + // check and error string. + // + // Returns the resolved `"current"` / `"panel"` rather than an + // opaque handle, so the Lua callers keep their existing + // `if display == "panel"` dispatch and this change stays a + // validation unification rather than a control-flow rewrite. + win.set( + "_resolve_display", + lua.create_function( + |_, + (operation, raw, default): (String, mlua::Value, String)| + -> mlua::Result<&'static str> { + let default = match default.as_str() { + "panel" => AdopterDefault::Panel, + "current" => AdopterDefault::Current, + other => { + return Err(mlua::Error::runtime(format!( + "window._resolve_display: bad default {other:?}" + ))); + } + }; + match resolve_adopter_display(&operation, Some(&raw), default)? { + AdopterPlacement::Panel => Ok("panel"), + AdopterPlacement::Current | AdopterPlacement::Window(_) => Ok("current"), + } + }, + )?, + )?; + } + { let cc = core.clone(); win.set( diff --git a/tests/bottom_panel_stage1_acceptance.rs b/tests/bottom_panel_stage1_acceptance.rs index 400754a..fb05e5e 100644 --- a/tests/bottom_panel_stage1_acceptance.rs +++ b/tests/bottom_panel_stage1_acceptance.rs @@ -1291,6 +1291,53 @@ fn acc19_adopters_place_side_affinely_through_real_entry_points() { "unknown display fails before session/process/buffer creation" ); assert_eq!(s.core.borrow().registry.borrow().ids().len(), before); + + // Bottom-panel Stage 3, Q#S3-1 — the NON-STRING normalization. + // + // Pinned because step 2 CHANGED this deliberately and nothing else + // covers it. Before the shared resolver, terminal read + // `get::>("display")?`, so a number raised mlua's + // TYPE error before any custom message existed; the Lua adopters + // stringified instead and reported their own. Unifying the error + // text without pinning this would have let the two drift back apart + // unnoticed, and the surrounding assertions could not have caught it + // — they all pass unknown STRINGS, which take the same path in both + // designs. + // + // The custom error wins because it names the legal vocabulary. The + // type is reported WITHOUT the value, so the message cannot imply a + // string was passed. + let before = s.core.borrow().registry.borrow().ids().len(); + let err = try_exec( + &s, + "pmacs.terminal.open { command = \"/bin/sh\", display = 42 }", + ) + .expect_err("a non-string display is rejected"); + // The TYPE SPELLING is deliberately not pinned: Lua 5.4 reports + // `integer` where LuaJIT has no integer subtype, so asserting either + // literal would pass on one CI flavor and fail on the other. What is + // pinned is the shape — our operation name, a parenthesised type + // rather than a quoted value, and the vocabulary. + assert!( + err.contains("pmacs.terminal.open: unknown display ("), + "a non-string display takes the shared unknown-display error naming the \ + operation and a type, not mlua's type error; got: {err}" + ); + assert!( + err.contains("expected \"current\" or \"panel\""), + "the error still names the legal values; got: {err}" + ); + assert!( + !err.contains("\"42\""), + "the rejected value is reported by TYPE, not quoted as though it were a \ + string; got: {err}" + ); + assert_eq!( + s.core.borrow().registry.borrow().ids().len(), + before, + "…and still creates nothing, exactly as an unknown string does" + ); + exec( &s, "TERM_BUF = pmacs.terminal.open { command = \"/bin/sh\", display = \"panel\" }", From 8d14e6a334a78f030592f0be107f4b1733bdb72d Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 1 Aug 2026 20:35:05 -0400 Subject: [PATCH 4/9] docs(stage3): make the lane portable and retire its superseded state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding P1: Stage 3 was neither portable nor accurately recoverable. `bottom-panel-stage3` had no upstream and no `githubsucks/` ref, while docs/active-work.md still described Stage 3 as "not started" — so a reader on another machine would have found a lane claiming nothing existed, and no way to fetch the two commits that did. That is precisely the failure this ledger records as its own rule: "When a PR is opened, give it a lane", learned when #171 drifted 153 commits while invisible here. A branch without a PR is the same hazard with less warning. The lane now carries branch, base, the two completed steps with their hashes, what remains, the verification at this head, and a recovery command. Review finding P2: the framing retained state the census superseded. It still said "Nothing has been implemented or measured" and that the blast-radius measurement "has not been taken", directly contradicting §1.6b two screens below. Both bullets are rewritten to point at the measurement rather than deny it, and the dired question is marked DECIDED rather than left open. Also records, in both documents, the thing step 2's original commit message overstated: it is DEFAULT-PRESERVING WITH ONE INTENTIONAL NORMALIZATION, not "behaviour-preserving". Every adopter kept its default and the suite is byte-identical to baseline, but terminal's invalid-input behaviour moved on purpose — and the sweep could not have caught it, because every existing assertion passes an unknown STRING, which takes the same path under both designs. Co-Authored-By: Claude Opus 5 (1M context) --- docs/active-work.md | 83 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 73 insertions(+), 10 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index 6374a1b..a3f2f83 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -311,19 +311,82 @@ them) and why `dired`/`listview` were the correct first two families. Whichever starts second integrates first. -## Bottom-panel lane (Arc 7) — STAGE 2 COMPLETE; STAGE 3 IS THE LAST STEP +## Bottom-panel lane (Arc 7) — STAGE 3 IN PROGRESS on `bottom-panel-stage3` **Stage 1, the Stage 2 framing, and Stages 2A, 2B-1, 2B-2 and 2B-3 are -all on `main` @ `4cd4a7b`** (#155, #175, #177, #184, #187, #198). Stage 2 -is complete. Durable facts are in `docs/agent-handoff.md` §1, including -the v20-baseline / v21-negotiated handshake that Stage 2B-3 made -compatible. +all on `main`** (#155, #175, #177, #184, #187, #198). Stage 2 is +complete. Durable facts are in `docs/agent-handoff.md` §1, including the +v20-baseline / v21-negotiated handshake that Stage 2B-3 made compatible. -- **Stage 3 — the adopter default flip — is the arc's last step and is - not started.** This lane stays until it lands; it is not removed at - 2B-3's merge. -- **DAP waits for Stage 2, not Stage 1** — that dependency is now - satisfied. +**Stage 3 — the adopter default flip — is Arc 7's last step and is NOW +IN PROGRESS**, not "not started" as this lane said until 2026-08-01. +Framing `docs/bottom-panel-stage3-framing.md` revision 3, approved with +amendments after one review round. + +- **Branch `bottom-panel-stage3`**, based on `githubsucks/main` @ + `21de0b2`. Developed in the primary checkout on the laptop, not a + worktree. **No PR yet** — the flip has not landed. +- **Steps 1 and 2 of the framing's §7 are DONE:** + - `0224c68` — the **fallout census**, taken before any change: a + throwaway flip, a full `--no-fail-fast` sweep, then revert. **37 + failures across 5 suites**, classified per test in framing §1.6c. + - `41d37fc` — the **shared resolver** + (`resolve_adopter_display` / `pmacs.window._resolve_display`), + replacing four hand-written copies of the same validation. + **Default-preserving with one intentional normalization** (below). +- **Steps 3–6 are NOT done:** the flip itself, the ~37 test revisions, + the capability-fallback criterion, and the lane/handoff close-out. + +### What the census established, and why it ran first + +- **A census that stops at the first failing binary is not a census.** + The first sweep reported 2 failures in 1 suite because `cargo test` + halts after a failing binary; `--no-fail-fast` revealed 37 across 5. + **Any re-measurement must pass that flag.** +- **`m4_acceptance` is a transitive adopter nobody named.** Its two + failures are the LSP hover and outline panels, which are **listview + consumers**. Q#BP12's four rows are the *direct* population; the real + one is everything built on listview. +- **The proportions invert the obvious reading.** `compile_mode` has the + most failures (17) and the least placement content; `listview_ + acceptance` has 13 but loses **three quarters of its suite**. + +### The normalization step 2 changed, deliberately + +Terminal previously raised **mlua's type error** for a non-string +`display`; the Lua adopters stringified into their own message. Now all +four take the shared custom error, rendered **by type alone** +(`unknown display (integer)`), because it names the legal vocabulary. + +**Pinned** in `bottom_panel_stage1_acceptance::acc19` at the terminal +entry point, asserting the error *and* that nothing is created. The type +**spelling** is deliberately unpinned — Lua 5.4 says `integer` where +LuaJIT has no integer subtype, so a literal assertion would pass on one +CI flavor and fail on the other. **Verified 46/46 under both.** + +### Verification at this head + +Full serialized suite **3447 passed / 0 failed**, with **zero suites +differing from the pre-change baseline** — the measurement that makes +"default-preserving" a claim rather than an intention. Plus fmt, +diff-check, clippy, `--lib` 1896, and +`bottom_panel_stage1_acceptance` 46/46 under **both** Lua flavors. + +### Recovery from a clean checkout + +```sh +git fetch githubsucks +git worktree add ../pmacs-bp-stage3 \ + -b bottom-panel-stage3 \ + githubsucks/bottom-panel-stage3 +``` + +- **DAP waits for Stage 2, not Stage 1** — satisfied. Q#BP12's adopter + table already carries a `DAP stack/variables` row, so Stage 3 settles + the debugger's panel policy before the debugger exists. +- **The tree primitive is the next thing to scope after this**, per + `COHERENCE.md` §14: Tree is graded ✗, and DAP's variables view is its + next would-be inventor. ## Reap-ledger silent failures — MERGED (#202); kept for its parked follow-ons From c0eb16bd1266465ff608239f911354c1ce948e5e Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Mon, 3 Aug 2026 11:10:14 -0400 Subject: [PATCH 5/9] =?UTF-8?q?feat(panel):=20flip=20the=20adopter=20defau?= =?UTF-8?q?lt=20=E2=80=94=20Arc=207's=20last=20step?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage 3 steps 3 and 4. Omitting `display` now resolves to the PANEL for listview, compile and terminal; dired keeps `"current"`, passed explicitly to the shared resolver. Per-adopter `select` per Q#BP12: listview true, compile false (passive output must not steal document focus), terminal true. The census predicted 37 failures across 5 suites and the flip produced exactly that — same suites, same per-suite counts. The measurement was a prediction, not an estimate, which is what the inverted step order was for. Final sweep: 3449 passed / 0 failed against a 3447 baseline, the +2 being new pins. THE CENSUS COUNTED FAILURES, NOT CAUSES. Thirteen listview failures had ONE root cause: a panel is derived-hidden while frame geometry is unknown, and listview_acceptance never declared any — it never needed to while listview defaulted to the current window. One helper took it from 13 to 2. The same applied to m4 and vterm_stage2. Geometry is authoritative state and a grid frontend's real frame size IS its declaration; the panel suites have always said so. THREE DEFECTS THE FLIP EXPOSED, each fixed rather than tested around: 1. The OUTLINE panel's `on_visit` used `pmacs.window.switch_buffer` — the RAW switch, which replaces the buffer in the ACTIVE window. That was harmless while the outline opened into a document window. Once the panel became the default the active window WAS the outline panel, so RET clobbered the panel with the source and left nothing for `M-,` to return to. The references panel was migrated to `display_file` when the arc landed; the outline was missed because nothing exercised it from a panel until now. Q#BP11c names this exact corruption, and both the outline and compile tests now assert `M-,` FOCUSES the panel rather than cloning its buffer into the document — an assertion the previous one could not distinguish. 2. `pmacs.compile._last` stored only `{cmdline, cwd}`, so a recompile reached `start_run` with no `display` and took the new default. A user who ran `compile.run{display="current"}` would be moved into a panel the moment they pressed `g`. An opt-out that reverts on the next recompile is not an opt-out; `display` is stored and replayed, with nil kept as nil so an omitted value still resolves to the default rather than freezing at the first run's resolution. 3. `opts.display` on a nil `opts` — my own regression, introduced by fix 2 and caught by `journey_acceptance`, which is exactly what that ratchet is for. COMPILE'S CHORDS ARE NOW PANEL-LOCAL, and that is a contract rather than an accidental reachability loss. Every compile chord is bound `scope = "buffer"`, so with `select = false` none dispatch from the document — `C-c C-k` included. `acc34` pins it, and pins that `M-x compile.kill` still reaches the running slot from anywhere via its `or compile_slot()` fallback. A global chord is a command-surface decision and belongs in its own framing. TEST CLASSIFICATION WAS PER TEST, NOT PER SUITE. Two neighbouring compile tests land on opposite sides: acc15 (RET-visits-error, jump-back) asserts the NEW default, while acc16 (n/p within compile output) genuinely needs the buffer selected and says so. compile's suite-wide helper opts out because ITS subject is compile-BUFFER behaviour; the placement-subject tests use a second helper that takes the default. Every opt-out states why. Nothing was mass-added to make a suite green. s1_12's two concerns are split as directed: it keeps its Q#GB18 name-keyed-identity bite with explicit `display = "current"`, isolating the buffer-level `p.prev` skip rule, while a new `s3_1` pins the side-window presentation chain — C → B → A → delete, ending at the document with the wrapper collapsed. The mechanisms are complementary: presentation history chains in the side slot; `p.prev` prevents raw-switch and capability-fallback loops. Verified: fmt, diff-check, clippy with and without crdt, --lib 1896, --lib --features crdt 2081, pmacs-protocol 19, m4 149, required GPU 221, and the full serialized sweep at 3449/0. Co-Authored-By: Claude Opus 5 (1M context) --- builtin/runtime/compile.lua | 17 ++- builtin/runtime/listview.lua | 7 +- builtin/runtime/lsp.lua | 16 ++- docs/bottom-panel-stage3-framing.md | 46 ++++++- src/lua_bindings/mod.rs | 11 +- src/lua_bindings/window_panel.rs | 5 +- tests/bottom_panel_stage1_acceptance.rs | 42 +++++- tests/compile_mode_acceptance.rs | 174 +++++++++++++++++++++++- tests/listview_acceptance.rs | 164 +++++++++++++++++++--- tests/m4_acceptance.rs | 41 ++++++ tests/vterm_stage2_acceptance.rs | 29 ++++ 11 files changed, 507 insertions(+), 45 deletions(-) diff --git a/builtin/runtime/compile.lua b/builtin/runtime/compile.lua index 1c2c674..20300cc 100644 --- a/builtin/runtime/compile.lua +++ b/builtin/runtime/compile.lua @@ -813,7 +813,10 @@ local function start_run(slot, cmdline, opts) -- panel-placed. Resolving first and testing `== "current"` afterwards -- would silently merge the two and break that opt-out. local display_omitted = opts.display == nil - local display = pmacs.window._resolve_display("compile.run", opts.display, "current") + -- Stage 3 (Q#BP12): omission resolves to the PANEL, with + -- `select = false` at the display call below — compile output is + -- passive and must not steal focus from the buffer being compiled. + local display = pmacs.window._resolve_display("compile.run", opts.display, "panel") -- q-target discipline (Q#CM11): capture only when coming from a -- non-generated buffer, so `g` reruns don't re-capture and -- compile → g → q restores the original buffer. @@ -1150,7 +1153,15 @@ function pmacs.compile.run(cmdline, opts) slot.parse = true local proc = start_run(slot, cmdline, opts) if proc then - pmacs.compile._last = { cmdline = cmdline, cwd = slot.cwd } + -- Stage 3: `display` is stored too. It is the documented opt-out + -- from the panel default, and `g` (recompile) reaches `start_run` + -- with whatever `_last` holds — so without this, a user who ran + -- `compile.run{display="current"}` would be moved into a panel the + -- moment they recompiled. An opt-out that reverts on the next `g` + -- is not an opt-out. `nil` is stored as `nil`, so an omitted + -- `display` keeps resolving to the default rather than being + -- frozen at the first run's resolution. + pmacs.compile._last = { cmdline = cmdline, cwd = slot.cwd, display = opts and opts.display } claim_compile_source(slot) end return proc @@ -1226,7 +1237,7 @@ pmacs.command.define { pmacs.editor.set_status("compile: nothing to recompile yet (run compile.run first)") return end - pmacs.compile.run(last.cmdline, { cwd = last.cwd }) + pmacs.compile.run(last.cmdline, { cwd = last.cwd, display = last.display }) end, } diff --git a/builtin/runtime/listview.lua b/builtin/runtime/listview.lua index 9c904a8..62b0cb3 100644 --- a/builtin/runtime/listview.lua +++ b/builtin/runtime/listview.lua @@ -229,7 +229,12 @@ function pmacs.listview.open(spec) -- Q#S3-1: the vocabulary, the error and the default policy are one -- rule (`window._resolve_display`), not a copy per adopter. The -- default is passed in because the adopters do not share one. - local display = pmacs.window._resolve_display("listview.open", spec.display, "current") + -- Stage 3 (Q#BP12): omission resolves to the PANEL. `select = true` + -- below is a correctness requirement, not a preference — `seat_cursor` + -- and `listview.refresh` drive `pmacs.editor.move_down()`, which acts + -- on the ACTIVE window, so an unselected panel would seat the cursor + -- in the user's document. + local display = pmacs.window._resolve_display("listview.open", spec.display, "panel") if display == "panel" then pmacs.window.display(p.buffer, { side = "bottom", select = true }) else diff --git a/builtin/runtime/lsp.lua b/builtin/runtime/lsp.lua index 7c636cb..6107b38 100644 --- a/builtin/runtime/lsp.lua +++ b/builtin/runtime/lsp.lua @@ -2493,7 +2493,21 @@ function pmacs.lsp.document_symbols() rows = rows, on_visit = function(sym) pmacs.editor.push_jump() - local okv = pcall(pmacs.window.switch_buffer, source_buf) + -- Bottom-panel arc (Q#BP11b), completed in Stage 3: a visit FROM + -- a panel must land in the DOCUMENT target and leave the panel + -- intact — the same rule `visit_location` above already follows + -- via `display_file`. + -- + -- This path used `pmacs.window.switch_buffer`, the RAW switch, + -- which replaces the buffer in the ACTIVE window. That was + -- harmless while the outline opened into a document window: the + -- switch simply reused it. Once Stage 3 made the panel the + -- default, the active window IS the outline panel, so RET + -- clobbered the panel with the source and left nothing for `M-,` + -- to return to. The references panel was migrated when the arc + -- landed; the outline was missed because nothing exercised it + -- from a panel until the default flipped. + local okv = pcall(pmacs.window.display, source_buf, { select = true }) if not okv then pmacs.editor.jump_back() pmacs.editor.set_status("LSP: outline source buffer is gone") diff --git a/docs/bottom-panel-stage3-framing.md b/docs/bottom-panel-stage3-framing.md index e7ba982..2002fa0 100644 --- a/docs/bottom-panel-stage3-framing.md +++ b/docs/bottom-panel-stage3-framing.md @@ -236,6 +236,45 @@ to make a suite green converts a behavioural change into an invisible one, which is the failure this stage's inverted ordering exists to avoid. +### 1.5a Compile's chords become PANEL-LOCAL — a deliberate contract + +**Discovered while revising the fallout, and decided rather than +absorbed.** Every compile-mode chord is bound +`scope = "buffer", buffer = slot.buf` (`compile.lua:221`): `RET`, `n`, +`p`, `q`, **`C-c C-k`**, `g`, and the seven undo no-ops. They dispatch +only when `*compilation*` is the focused buffer. + +Before Stage 3 compile switched in place, so the user was *in* that +buffer and the chords worked. **Stage 3 keeps `select = false`** — Q#BP12 +is explicit, and passive build output stealing focus mid-edit would be +worse than the alternative — so the user stays in their document and +none of those chords reach compile-mode without focusing the panel +first. + +**The contract, stated so it is deliberate rather than an accidental +reachability loss:** + +1. A default `compile.run` opens **passively**; document focus remains. +2. **Buffer-local compile chords require focusing the panel** (`C-x o`, + or a click). +3. **The capability is not lost.** `compile.kill` is reachable through + `M-x` from anywhere, because its body is + `slot_for_buffer(pmacs.window.buffer()) or compile_slot()` — the + `or` arm falls back to the current compilation slot precisely when + the caller is not in a compile buffer (`compile.lua:1124`). Verified, + not assumed; it is what makes panel-local chords acceptable rather + than a lost feature. + +**A global `C-c C-k` is deliberately NOT part of this stage.** It is a +command-surface decision — which chords earn global scope — and it would +ride in on a placement flip without its own reasoning. Framed +separately or not at all. + +Pinned by a default-placement test asserting `C-c C-k` from the document +window does **not** dispatch to compile (acceptance 10), so a future +change that quietly makes it global has to change a test that says why +it was not. + ### 1.6b The fallout census — MEASURED, and it found an adopter nobody named Taken as branch-plan step 1: record a baseline, apply the three-site @@ -412,7 +451,12 @@ Inherits the parent framing's criteria 56–58, made concrete: 8. The count of existing suites whose behaviour changes is **stated**, and each change is classified intended or collateral — **not silenced by mass-adding `display = "current"`** (§1.6a). -9. **NEGATIVE criterion — omission still means the document for dired.** +9. **NEGATIVE criterion — compile's chords are panel-local.** With the + default placement, `C-c C-k` pressed in the **document** window does + not dispatch to compile (§1.5a). Tests whose subject is + compile-BUFFER behaviour opt out with `display = "current"` and say + why. `M-x compile.kill` still works from anywhere. +10. **NEGATIVE criterion — omission still means the document for dired.** Both entry points are pinned: a direct `pmacs.dired.open(path)` with no `display`, **and** the `pmacs .` launch path through `pmacs.path.directory_handler`. Neither may place into a panel. This diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 0dfbd2d..77bb4c7 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -9320,10 +9320,11 @@ fn install_terminal( // Bottom-panel arc (Q#BP11b): parse placement BEFORE the // session, process, buffer, or wrapper exists, so an // unknown `display` value creates nothing to roll back. - // Stage 3 step 2: the shared resolver, still carrying the - // PRE-FLIP default. The unification must be provably - // behaviour-preserving before the default moves, so the - // flip to `AdopterDefault::Panel` is its own commit. + // Stage 3 (Q#BP12): omitting `display` resolves to the + // PANEL. `select = true` is passed to + // `place_adopter_buffer` below — a terminal is + // interactive, so it takes focus, unlike compile's + // passive output. let display_value = spec_table.get::("display")?; let placement = window_panel::parse_adopter_placement( &core, @@ -9331,7 +9332,7 @@ fn install_terminal( "pmacs.terminal.open", Some(&display_value), spec_table.get::>("window")?, - window_panel::AdopterDefault::Current, + window_panel::AdopterDefault::Panel, )?; let buffer_id = { let mut manager = manager.borrow_mut(); diff --git a/src/lua_bindings/window_panel.rs b/src/lua_bindings/window_panel.rs index 667cb68..fe8b758 100644 --- a/src/lua_bindings/window_panel.rs +++ b/src/lua_bindings/window_panel.rs @@ -262,8 +262,9 @@ pub(crate) enum AdopterPlacement { /// subtly wrong, and the next adopter is DAP. /// /// `default` is a **parameter, not a constant**, because the adopters do -/// not share one. listview / compile / terminal resolve omission to the -/// panel; **dired resolves it to `"current"`** and must keep doing so — +/// not share one. Since Stage 3, listview / compile / terminal resolve +/// omission to the panel; **dired resolves it to `"current"`** and must +/// keep doing so — /// `pmacs.path.directory_handler` calls it with no `display` at all, so /// a flipped default would open `pmacs .` in a bottom panel (§1.1a). /// Passing the default in is what makes dired's exemption visible at its diff --git a/tests/bottom_panel_stage1_acceptance.rs b/tests/bottom_panel_stage1_acceptance.rs index fb05e5e..40f2b93 100644 --- a/tests/bottom_panel_stage1_acceptance.rs +++ b/tests/bottom_panel_stage1_acceptance.rs @@ -1220,18 +1220,42 @@ fn acc18_display_file_targets_the_document_from_a_focused_panel() { // --------------------------------------------------------------------------- #[test] +#[allow( + clippy::too_many_lines, + reason = "one placement scenario per adopter; splitting it would hide that they share a contract" +)] fn acc19_adopters_place_side_affinely_through_real_entry_points() { - // listview: pre-seed the persistent panel buffer in a DOCUMENT window - // first, so side-affine placement cannot be vacuous. + // Stage 3: the DEFAULT is now the panel, so assert that first — + // this test's subject is placement through the real entry points. let s = editor(); exec( &s, - "pmacs.listview.open { name = \"*outline*\", rows = { { text = \"row\" } } }", + "pmacs.listview.open { name = \"*default*\", rows = { { text = \"row\" } } }", + ); + assert!( + side_window(&s).is_some(), + "omitting display places a listview in the panel" + ); + + // listview: pre-seed the persistent panel buffer in a DOCUMENT window + // first, so side-affine placement cannot be vacuous. + // + // The seed now says `display = "current"` EXPLICITLY. That is not a + // bolt-on to keep the test green: the seed's whole purpose is "this + // buffer starts in a document window", and after the flip that + // requires saying so. Leaving it omitted would seed a panel and the + // side-affine assertion below would pass without having moved + // anything — exactly the vacuity this fixture was built to prevent. + let s = editor(); + exec( + &s, + "pmacs.listview.open { name = \"*outline*\", rows = { { text = \"row\" } }, \ + display = \"current\" }", ); let seeded = active_window(&s); assert!( side_window(&s).is_none(), - "the default placement is unchanged" + "the explicit opt-out still places in the document window" ); exec( &s, @@ -1254,9 +1278,10 @@ fn acc19_adopters_place_side_affinely_through_real_entry_points() { "an unknown display value is a pointed error" ); - // compile: same shape, but passive (`select = false`). + // compile: same shape, but passive (`select = false`). Seeded with + // the explicit opt-out for the same reason as the listview above. let s = editor(); - exec(&s, "pmacs.compile.run(\"true\")"); + exec(&s, "pmacs.compile.run(\"true\", { display = \"current\" })"); assert!(side_window(&s).is_none()); let document = active_window(&s); exec(&s, "pmacs.compile.run(\"true\", { display = \"panel\" })"); @@ -1404,8 +1429,11 @@ fn acc19b_recompile_reuses_the_panel_instead_of_duplicating_into_the_document() ); // A compilation that is NOT in a panel keeps the pre-arc raw switch. + // Reaching that state now takes an explicit opt-out, since the + // default would panel it — and "not in a panel" is the precondition + // this half exists to exercise. let s = editor(); - exec(&s, "pmacs.compile.run(\"true\")"); + exec(&s, "pmacs.compile.run(\"true\", { display = \"current\" })"); assert!(side_window(&s).is_none()); let target = active_window(&s); exec(&s, "pmacs.command.invoke(\"compile.recompile\")"); diff --git a/tests/compile_mode_acceptance.rs b/tests/compile_mode_acceptance.rs index a223406..1b38abe 100644 --- a/tests/compile_mode_acceptance.rs +++ b/tests/compile_mode_acceptance.rs @@ -84,6 +84,13 @@ fn errors_buffer(s: &EditorState) -> String { fn editor() -> EditorState { let s = EditorState::new_with_roots(&crate::iso::roots()); exec(&s, "pmacs.lsp.config = {}"); + // Bottom-panel Stage 3: a panel is derived-hidden while the + // frontend's frame geometry is unknown, so the default-placement + // tests here could neither see nor focus one. Geometry is + // authoritative state and a grid frontend's real frame size IS its + // declaration; every test that does not render declares it, exactly + // as `bottom_panel_stage1_acceptance` always has. + s.sync_frame_geometry(FrontendId::LOCAL, pmacs::protocol::CellSize::new(24, 80)); s } @@ -148,7 +155,38 @@ fn pump_until( } /// Start a compile run programmatically with an explicit cwd. +/// Run a compilation **in the document window**, via an explicit +/// `display = "current"`. +/// +/// Bottom-panel Stage 3 flipped `compile.run`'s default to the panel +/// with `select = false`, so a default run leaves focus in the user's +/// document — and every compile-mode chord is bound +/// `scope = "buffer", buffer = slot.buf` (`compile.lua:221`): `RET`, +/// `n`, `p`, `q`, `C-c C-k`, `g`, and the seven undo no-ops. None of +/// them dispatch unless `*compilation*` is focused. +/// +/// **This suite's subject is compile-BUFFER behaviour** — streaming, +/// styling, error navigation, kill/reap, undo suppression — all of which +/// genuinely require that buffer selected. Opting out here is a +/// statement about what these tests are for, not a way to make them +/// green: the placement-subject tests use [`compile_run_default`] +/// instead, and the panel-local contract itself is pinned by +/// `acc34_default_placement_leaves_compile_chords_panel_local`. fn compile_run(s: &EditorState, cmdline: &str, cwd: &Path) { + exec( + s, + &format!( + "pmacs.compile.run({cmdline:?}, {{ cwd = {:?}, display = \"current\" }})", + cwd.display().to_string() + ), + ); +} + +/// Run a compilation through the **Stage 3 default** — no `display` at +/// all, so it resolves to the panel with `select = false`. +/// +/// For tests whose subject IS placement. +fn compile_run_default(s: &EditorState, cmdline: &str, cwd: &Path) { exec( s, &format!( @@ -715,17 +753,113 @@ fn acc14_malformed_rule_containers_fail_closed() { /// Fixture: a target file plus a compile run reporting one error at /// target.c:3:2. Returns the editor, finished, in *compilation*. -fn error_fixture(dir: &Path) -> EditorState { +/// The same fixture through the **Stage 3 default** — the compilation +/// lands in the panel and focus stays in the document. +fn error_fixture_default(dir: &Path) -> EditorState { std::fs::write(dir.join("target.c"), "l1\nl2\nl3 body\nl4\n").unwrap(); let mut s = editor(); - compile_and_finish(&mut s, "printf 'target.c:3:2: error: boom\\n'", dir); + compile_run_default(&s, "printf 'target.c:3:2: error: boom\\n'", dir); + assert!( + pump_until(&mut s, 10_000, |s| compilation_text(s) + .contains("[compile ")), + "compile run must reach its exit marker" + ); s } +/// Is `*compilation*` currently shown in a side window? +fn compilation_is_panelled(s: &EditorState) -> bool { + let core = s.core.borrow(); + core.windows.values().any(|w| { + w.is_side() + && core + .registry + .borrow() + .get(w.buffer_id) + .is_ok_and(|b| b.name() == "*compilation*") + }) +} + +/// Bottom-panel Stage 3 §1.5a — compile's chords are PANEL-LOCAL, and +/// that is a contract rather than an accidental reachability loss. +/// +/// The default flip put compile output in the panel with +/// `select = false`, so document focus survives a build. Every +/// compile-mode chord is bound `scope = "buffer", buffer = slot.buf` +/// (`compile.lua:221`), so none of them dispatch from the document — +/// `C-c C-k` included. +/// +/// This is pinned so a future change that quietly promotes one of them +/// to a global binding has to edit a test that says why it was not. The +/// capability is NOT lost: `M-x compile.kill` reaches the running slot +/// from anywhere, because its body falls back to `compile_slot()` when +/// the caller is not in a compile buffer. +/// +/// A global `C-c C-k` is a command-surface decision and belongs in its +/// own framing, not in a placement flip. +#[test] +fn acc34_default_placement_leaves_compile_chords_panel_local() { + let dir = tempfile::tempdir().unwrap(); + let mut s = editor(); + compile_run_default(&s, "echo ready; sleep 30", dir.path()); + assert!( + pump_until(&mut s, 5_000, |s| compilation_text(s).contains("\nready\n")), + "the run is live before we test the chord; buffer:\n{}", + compilation_text(&s) + ); + assert_ne!( + active_buffer_name(&s), + "*compilation*", + "premise: the default left focus in the document" + ); + + // C-c C-k from the DOCUMENT does not reach compile-mode. + ctrl(&mut s, 'c'); + ctrl(&mut s, 'k'); + assert!( + !status(&s).contains("killed"), + "C-c C-k must NOT dispatch to compile from the document window; \ + status was: {}", + status(&s) + ); + + // The capability is reachable, just not by that chord from here. + exec(&s, "pmacs.command.invoke('compile.kill')"); + assert!( + status(&s).contains("killed"), + "M-x compile.kill still reaches the running slot from anywhere; \ + status was: {}", + status(&s) + ); +} + #[test] fn acc15_ret_visits_error_and_jump_back_returns() { let dir = tempfile::tempdir().unwrap(); - let mut s = error_fixture(dir.path()); + // Stage 3: this test's SUBJECT is placement, so it takes the default + // — Q#BP12 requires compilation panel -> RET source -> `M-,` back to + // the still-present panel, with the document window intact. + let mut s = error_fixture_default(dir.path()); + assert!( + compilation_is_panelled(&s), + "premise: the default put compilation in the panel" + ); + assert_ne!( + active_buffer_name(&s), + "*compilation*", + "premise: select = false left focus in the document" + ); + + // The chords are PANEL-LOCAL (§1.5a), so reaching them is an + // explicit focus change — that is the contract, not a workaround. + ctrl(&mut s, 'x'); + press(&mut s, KeyCode::Char('o')); + assert_eq!( + active_buffer_name(&s), + "*compilation*", + "C-x o reaches the panel" + ); + // Cursor starts on the header (row 0): RET there reports and // stays. press(&mut s, KeyCode::Enter); @@ -742,9 +876,33 @@ fn acc15_ret_visits_error_and_jump_back_returns() { let line: i64 = eval(&s, "return pmacs.editor.cursor_line()"); let col: i64 = eval(&s, "return pmacs.editor.cursor_col()"); assert_eq!((line, col), (2, 1), "0-based landing from 1-based 3:2"); - // M-, returns to the compilation buffer (jump ring). + // M-, returns to the compilation buffer (jump ring) — and the panel + // is still present rather than having been consumed by the visit. alt(&mut s, ','); assert_eq!(active_buffer_name(&s), "*compilation*"); + assert!( + compilation_is_panelled(&s), + "the panel survives the visit and the jump back" + ); + // …and the jump back FOCUSED the panel rather than duplicating + // `*compilation*` into the document window. Without this, the + // assertion above would pass for a tree holding the buffer twice. + let doc_shows_compilation = { + let core = s.core.borrow(); + core.windows.values().any(|w| { + !w.is_side() + && core + .registry + .borrow() + .get(w.buffer_id) + .is_ok_and(|b| b.name() == "*compilation*") + }) + }; + assert!( + !doc_shows_compilation, + "M-, must focus the existing panel, not clone the compilation \ + buffer into a document window" + ); } #[test] @@ -984,7 +1142,13 @@ fn acc24_command_path_undo_after_completed_run_recovers_immediately() { exec( &s, &format!( - "pmacs.shell.command('echo shell-out', {{ cwd = {:?} }})", + // Stage 3: explicit `display = "current"`. `shell.command` + // shares compile's `start_run` and so flipped with it, but + // this test's subject is that `M-x buffer.undo` recovers + // SYNCHRONOUSLY in the generated buffer — which requires + // that buffer focused, or the undo targets the document + // instead and the test would pass for the wrong reason. + "pmacs.shell.command('echo shell-out', {{ cwd = {:?}, display = 'current' }})", dir.path().display().to_string() ), ); diff --git a/tests/listview_acceptance.rs b/tests/listview_acceptance.rs index d546f00..e316e41 100644 --- a/tests/listview_acceptance.rs +++ b/tests/listview_acceptance.rs @@ -17,7 +17,23 @@ use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers}; use pmacs::buffer::BufferId; use pmacs::editor::EditorState; -use pmacs::protocol::FrontendId; +use pmacs::protocol::{CellSize, FrontendId}; + +/// Bottom-panel Stage 3: a listview now opens into the PANEL by default, +/// and a panel is derived-hidden while the frontend's frame geometry is +/// unknown — so focus would fall back to the document window and every +/// panel assertion here would read the wrong buffer. +/// +/// `bottom_panel_stage1_acceptance` has always declared geometry for the +/// same reason: a grid frontend's real frame size IS its declaration, +/// and every test that does not render must state it before any input. +/// This suite never needed to while listview defaulted to the current +/// window. It does now. +fn editor() -> EditorState { + let s = EditorState::new_with_roots(&crate::iso::roots()); + s.sync_frame_geometry(FrontendId::LOCAL, CellSize::new(24, 80)); + s +} fn key(code: KeyCode, mods: KeyModifiers) -> KeyEvent { KeyEvent { @@ -194,6 +210,31 @@ fn open_test_panel(s: &mut EditorState) { .expect("open test panel"); } +/// Open the fixture panel with an explicit `display = "current"`, for +/// the tests whose subject requires it to sit in a document window. +fn open_test_panel_in_document(s: &mut EditorState) { + s.lua_host + .lua() + .load( + r#" + _G.VISITED = nil + pmacs.listview.open { + name = "*test-panel*", + header = "3 items RET visit q quit", + display = "current", + rows = { + { text = "alpha", item = "A" }, + { text = "beta", item = "B" }, + { text = "gamma", item = "C" }, + }, + on_visit = function(item) _G.VISITED = item end, + } + "#, + ) + .exec() + .expect("open test panel in a document window"); +} + /// `(active buffer name, buffer text, cursor line, visited)` probed /// through the Lua surface. fn probe(s: &EditorState) -> (String, String, i64, Option) { @@ -212,7 +253,7 @@ fn probe(s: &EditorState) -> (String, String, i64, Option) { #[test] fn open_seats_cursor_and_ret_visits_the_row() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); let (name, text, line, _) = probe(&s); assert_eq!(name, "*test-panel*"); @@ -227,7 +268,7 @@ fn open_seats_cursor_and_ret_visits_the_row() { #[test] fn header_row_is_not_visitable() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); press(&mut s, KeyCode::Char('p')); // up onto the header press(&mut s, KeyCode::Enter); @@ -237,7 +278,7 @@ fn header_row_is_not_visitable() { #[test] fn q_restores_the_previous_buffer() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); press(&mut s, KeyCode::Char('q')); let (name, _, _, _) = probe(&s); @@ -246,7 +287,7 @@ fn q_restores_the_previous_buffer() { #[test] fn panel_rejects_typing() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); let (_, before, _, _) = probe(&s); press(&mut s, KeyCode::Char('z')); // unbound printable → self-insert → intercept rejects @@ -258,7 +299,7 @@ fn panel_rejects_typing() { fn dispatch_idle_is_false_while_a_panel_is_focused() { // Q#P6: while the panel is the active buffer, semantic frontends // must round-trip every key (RET = visit, not an optimistic \n). - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); assert!(s.dispatch_idle(), "scratch buffer: idle"); open_test_panel(&mut s); assert!(!s.dispatch_idle(), "panel focused: keys must round-trip"); @@ -268,7 +309,7 @@ fn dispatch_idle_is_false_while_a_panel_is_focused() { #[test] fn refresh_reruns_the_source_and_reseats() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); press(&mut s, KeyCode::Char('g')); let (_, text, line, _) = probe(&s); @@ -304,7 +345,7 @@ const PANEL_TEXT: &str = "3 items RET visit q quit\nalpha\nbeta\ngamma"; /// consulting the intercept chain. #[test] fn s1_1_the_undo_chord_cannot_empty_a_listview_panel() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); assert_eq!(active_text(&s), PANEL_TEXT, "precondition: rendered"); @@ -327,7 +368,7 @@ fn s1_1_the_undo_chord_cannot_empty_a_listview_panel() { /// *Bite:* same empty result on the pre-image. #[test] fn s1_2_m_x_buffer_undo_cannot_empty_a_listview_panel() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); m_x(&mut s, "buffer.undo"); @@ -355,7 +396,7 @@ fn s1_2_m_x_buffer_undo_cannot_empty_a_listview_panel() { /// raising. #[test] fn s1_4_the_owners_refresh_still_works_after_the_lock() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); let panel = id_of(&s, "*test-panel*"); assert!( @@ -388,7 +429,7 @@ fn s1_4_the_owners_refresh_still_works_after_the_lock() { /// therefore passes the rope half and fails the lifted half. #[test] fn s1_5_the_rope_lock_and_named_intercept_refuse_in_order() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); let panel = id_of(&s, "*test-panel*"); let before = active_text(&s); @@ -453,8 +494,15 @@ fn s1_5_the_rope_lock_and_named_intercept_refuse_in_order() { /// pinned through `dispatch_idle_for` rather than through `read_only`. #[test] fn s1_6_round_trip_input_survives_the_adoption() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); - open_test_panel(&mut s); + let mut s = editor(); + // Stage 3: an EXPLICIT opt-out, because this fixture genuinely needs + // the document window. `dispatch_idle` goes false for TWO reasons — + // a round-trip buffer (this test's subject) and a focused panel + // (`dispatch_idle_is_false_while_a_panel_is_focused`, a different + // test). Letting the panel default apply here would satisfy the gate + // for the wrong reason and the test would pass while proving + // nothing. The premise assertion below is what keeps that honest. + open_test_panel_in_document(&mut s); // (a) the premise. { @@ -498,7 +546,7 @@ fn s1_6_round_trip_input_survives_the_adoption() { /// the old line index live and this paint assertion bites. #[test] fn s1_7_a_shrinking_refresh_reaches_the_window() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); open_test_panel(&mut s); let painted = paint_active_window(&s, 6, 24); assert_eq!( @@ -540,7 +588,7 @@ fn s1_7_a_shrinking_refresh_reaches_the_window() { /// builtin/runtime/listview.lua` falsifies it. #[test] fn s1_9_a_foreign_buffer_with_the_panels_name_is_never_adopted() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); exec( &s, "FOREIGN = pmacs.buffer.create('*test-panel*')\n\ @@ -585,7 +633,7 @@ fn s1_9_a_foreign_buffer_with_the_panels_name_is_never_adopted() { /// created. #[test] fn s1_10_the_disambiguation_limit_raises_rather_than_adopting() { - let s = EditorState::new_with_roots(&crate::iso::roots()); + let s = editor(); exec( &s, "MINE = pmacs.buffer.create('*test-panel*')\n\ @@ -624,7 +672,7 @@ fn s1_10_the_disambiguation_limit_raises_rather_than_adopting() { /// command produced, never on "it did not raise". #[test] fn s1_11_a_disambiguated_panel_still_answers_ret_g_and_q() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); exec( &s, "FOREIGN = pmacs.buffer.create('*test-panel*')\n\ @@ -676,21 +724,96 @@ fn s1_11_a_disambiguated_panel_still_answers_ret_g_and_q() { /// /// *Bite:* restore the name-keyed `panels[d.name]` lookup while keeping /// the disambiguation and `q` lands back in `*test-panel*<2>`. +/// Bottom-panel Stage 3 — the SIDE-WINDOW half of `q`, complementary to +/// `s1_12`'s buffer-level `p.prev` rule. +/// +/// The parent framing's criterion 20 requires listview `q` to route +/// through `window.quit`, with `C → B → A` restoring each prior +/// presentation and the first panel deleting its wrapper. Before Stage 3 +/// this was unreachable from listview's own entry point without an +/// explicit `display = "panel"` on every open; the default flip makes it +/// the ordinary path, so it gets an ordinary-path test. +/// +/// The two mechanisms are complementary, not competing: **presentation +/// history chains in the side slot**, while **`p.prev` prevents +/// raw-switch and capability-fallback listview loops**. `s1_12` pins the +/// second by keeping its panels in document windows; this pins the +/// first. +#[test] +fn s3_1_q_walks_the_side_presentation_chain_back_to_the_document() { + let mut s = editor(); + exec( + &s, + "ORIGIN = pmacs.buffer.create('*origin*')\n\ + pmacs.window.switch_buffer(ORIGIN)", + ); + assert_eq!(active_name(&s), "*origin*", "premise: a document window"); + + for name in ["*panel-a*", "*panel-b*", "*panel-c*"] { + exec( + &s, + &format!( + "pmacs.listview.open {{ name = '{name}', header = 'H', \ + rows = {{ {{ text = 'x', item = 'X' }} }} }}" + ), + ); + assert_eq!(active_name(&s), name, "each open takes the panel slot"); + } + + // C → B → A: each `q` restores the presentation the next one + // replaced, rather than forgetting them or jumping straight out. + press(&mut s, KeyCode::Char('q')); + assert_eq!( + active_name(&s), + "*panel-b*", + "q restores the replaced panel" + ); + press(&mut s, KeyCode::Char('q')); + assert_eq!(active_name(&s), "*panel-a*", "…and again, in order"); + + // A → delete: the FIRST panel deletes its wrapper and focus lands + // back in the document. This is what bounds the chain — a loop + // between panels would never reach here. + press(&mut s, KeyCode::Char('q')); + assert_eq!( + active_name(&s), + "*origin*", + "the last q deletes the wrapper and returns to the document" + ); + assert!( + s.core.borrow().windows.values().all(|w| !w.is_side()), + "the side wrapper is collapsed, not left empty" + ); +} + #[test] fn s1_12_the_q_target_capture_is_not_inverted_across_two_panels() { - let mut s = EditorState::new_with_roots(&crate::iso::roots()); + let mut s = editor(); exec( &s, "FOREIGN = pmacs.buffer.create('*test-panel*')\n\ ORIGIN = pmacs.buffer.create('*origin*')\n\ pmacs.window.switch_buffer(ORIGIN)", ); - open_test_panel(&mut s); + // Stage 3: BOTH opens are explicitly `display = "current"`, and that + // is what keeps this test meaningful rather than what makes it pass. + // + // Its subject is the BUFFER-level `p.prev` skip rule and the Q#GB18 + // name-keyed identity guard — the `FOREIGN` buffer above shares the + // panel's name, so the disambiguation to `*test-panel*<2>` is the + // regression this pins. Under the panel default those two listviews + // would share the one bottom slot and `q` would exercise the + // SIDE-WINDOW restore chain instead (Q#BP2c criterion 20), which is + // a different mechanism with its own test below. Keeping them in + // document windows isolates the two, so a `p.prev` inversion stays + // detectable rather than being masked by presentation history. + open_test_panel_in_document(&mut s); assert_eq!(active_name(&s), "*test-panel*<2>", "premise: disambiguated"); exec( &s, "pmacs.listview.open { name = '*other-panel*', header = 'O', \ + display = 'current', \ rows = { { text = 'x', item = 'X' } } }", ); assert_eq!(active_name(&s), "*other-panel*", "premise: second panel"); @@ -700,7 +823,8 @@ fn s1_12_the_q_target_capture_is_not_inverted_across_two_panels() { assert_ne!( active_name(&s), "*test-panel*<2>", - "q must never return into another panel --- the chained-panel loop" + "q must never return into another panel via p.prev --- the \ + raw-switch/capability-fallback loop this rule exists to prevent" ); assert_eq!( active_name(&s), diff --git a/tests/m4_acceptance.rs b/tests/m4_acceptance.rs index 32511ff..68b1825 100644 --- a/tests/m4_acceptance.rs +++ b/tests/m4_acceptance.rs @@ -7982,6 +7982,19 @@ fn m4_5_symbols_and_highlight_round_trip() { /// (shared bootstrap for the panel tests). fn open_against_fake(path: &std::path::Path) -> pmacs::editor::EditorState { let mut state = pmacs::editor::EditorState::new_with_roots(&crate::iso::roots()); + // Bottom-panel Stage 3: the LSP panels are listview consumers, so + // they inherited the panel default — and a panel is derived-hidden + // while frame geometry is unknown. Without this declaration the + // outline and hover panels open into a hidden window and the probes + // below read the document buffer instead. + // + // This suite is the TRANSITIVE adopter the arc's own Q#BP12 table + // never named: nothing here calls `listview.open` directly, but + // `lsp.lua` does. + state.sync_frame_geometry( + pmacs::protocol::FrontendId::LOCAL, + pmacs::protocol::CellSize::new(40, 100), + ); let fake = fake_lsp_path(); state .lua_host @@ -8013,6 +8026,10 @@ fn open_against_fake(path: &std::path::Path) -> pmacs::editor::EditorState { /// method): open, depth-indented rows, RET jump-ring visit to the /// symbol's selectionRange, M-, back to the outline row, q restore. #[test] +#[allow( + clippy::too_many_lines, + reason = "criterion 58's whole flow: open -> visit -> jump-back -> quit, in one scenario" +)] fn outline_panel_opens_visits_and_restores() { let dir = tempfile::tempdir().expect("tempdir"); let a_path = dir.path().join("a.rs"); @@ -8097,6 +8114,30 @@ fn outline_panel_opens_visits_and_restores() { .eval() .expect("post-jump-back probe"); assert_eq!(name, "*outline*", "M-, returns to the outline panel"); + // Q#BP11c — and it FOCUSES the panel rather than cloning `*outline*` + // into the document window. The jump ring stores only + // `(BufferId, Position)`, so a naive `jump_back` would switch the + // active (document) window to the panel's buffer and leave the panel + // open too: the duplicate-buffer/window corruption that question + // names. The assertion above cannot tell those apart on its own. + let (panelled, doc_clone): (bool, bool) = { + let core = state.core.borrow(); + let named = |w: &pmacs::window::Window| { + core.registry + .borrow() + .get(w.buffer_id) + .is_ok_and(|b| b.name() == "*outline*") + }; + ( + core.windows.values().any(|w| w.is_side() && named(w)), + core.windows.values().any(|w| !w.is_side() && named(w)), + ) + }; + assert!(panelled, "the outline is still in its panel after M-,"); + assert!( + !doc_clone, + "M-, must not clone *outline* into a document window (Q#BP11c)" + ); // q restores the source buffer. state.dispatch_key( diff --git a/tests/vterm_stage2_acceptance.rs b/tests/vterm_stage2_acceptance.rs index d5b86e3..b537d7d 100644 --- a/tests/vterm_stage2_acceptance.rs +++ b/tests/vterm_stage2_acceptance.rs @@ -87,11 +87,20 @@ fn lua_surface_is_strict_fresh_transactional_and_context_safe() { let kind: String = lua .load(format!( r#" + -- Bottom-panel Stage 3: an EXPLICIT opt-out. This test + -- drives an 8x30 frame (see `sync_terminal_layout` + -- below), and DEFAULT_PANEL_ROWS is 12 — a panel plus a + -- MIN_WINDOW_OUTER_ROWS document window cannot fit in + -- eight rows, so the default placement would be hidden + -- by construction and there would be no layout to sync. + -- The subject here is the Lua surface being strict, + -- fresh and transactional, not placement. TERM_BUFFER = pmacs.terminal.open {{ command = {command_lua}, args = {{ "-c", "printf 'copy-me\\n'; sleep 30" }}, rows = 4, cols = 30, + display = "current", }} local first = pmacs.terminal.state(TERM_BUFFER) first.process.kind = "poisoned" @@ -365,6 +374,11 @@ fn lua_surface_is_strict_fresh_transactional_and_context_safe() { #[allow(clippy::too_many_lines, reason = "shared view and controller scenario")] fn shared_screen_keeps_view_scroll_selection_and_controller_independent() { let mut state = EditorState::new_with_roots(&crate::iso::roots()); + // Bottom-panel Stage 3: `terminal.open` now defaults to the panel + // (with `select = true`, so focus follows). A panel is derived-hidden + // while frame geometry is unknown, and a hidden terminal has no + // layout to sync — declare it, as the panel suites always have. + state.sync_frame_geometry(FrontendId::LOCAL, CellSize::new(24, 80)); let mut spec = TerminalSpec::new("/bin/sh"); spec.args = vec![ "-c".into(), @@ -509,6 +523,11 @@ fn terminal_escape_gates_local_bindings_and_double_escape_sends_interrupt() { input_path.to_str().expect("UTF-8 input path") ); let mut state = EditorState::new_with_roots(&crate::iso::roots()); + // Bottom-panel Stage 3: `terminal.open` now defaults to the panel + // (with `select = true`, so focus follows). A panel is derived-hidden + // while frame geometry is unknown, and a hidden terminal has no + // layout to sync — declare it, as the panel suites always have. + state.sync_frame_geometry(FrontendId::LOCAL, CellSize::new(24, 80)); state .lua_host .lua() @@ -802,12 +821,22 @@ fn real_tui_terminal_smoke_restores_host_after_output_input_resize_scroll_copy_a if f then f:write(text) f:close() end end breadcrumb({:?}, "1") + -- Bottom-panel Stage 3: an EXPLICIT opt-out. This is the real + -- TUI smoke -- a genuine pmacs in a genuine PTY -- and its + -- subject is that the HOST terminal is restored after output, + -- input, resize, scroll, copy (OSC 52) and bell. All of that is + -- measured against the rendered host stream over a full-frame + -- terminal; placing the child in a 12-row bottom panel changes + -- the geometry those measurements are taken over and the copy + -- path stops reaching the host. Placement is covered by the + -- panel suites, not here. local ok, terminal_buffer = pcall(pmacs.terminal.open, {{ command = "/bin/sh", args = {{ "-c", "exec /usr/bin/python3 -c \"$1\"", "pmacs-vterm-probe", {} }}, rows = 10, cols = 40, scrollback_rows = 200, + display = "current", }}) breadcrumb({:?}, ok and "ok" or ("ERROR: " .. tostring(terminal_buffer))) assert(ok, terminal_buffer) From 5f01edea87319385a67547410c3ac17c32cf1561 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Mon, 3 Aug 2026 11:16:08 -0400 Subject: [PATCH 6/9] test(panel): pin that the OMITTED default degrades on a pre-panel frontend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage 3 step 5, criterion 5 — the one the framing named as most likely to be quietly wrong. `acc14` already proves capability fallback for an EXPLICIT `request.side` at the core level, and it is not this case. Stage 3 resolves the default into a PANEL REQUEST inside the adopter, so a pre-panel semantic frontend now has to degrade a request the caller never wrote. Nothing in `listview.open { name, rows }` says "panel", yet the request reaching the core does — which is exactly why this is invisible from the adopter's side and needs its own pin rather than an inference from acc14. Asserts the three things that must survive the degradation: no side window, no side parameters on the document window, and NO QUIT ACTION left behind. That last one is the subtle half — a quit action stranded on a document window would make a later `q` try to restore a presentation that never happened. Bite-verified rather than assumed: flipping the fixture's frontend to `panel_capable = true` fails it on "a pre-panel frontend gets no side window from the omitted default", so the test is measuring the capability and not merely the absence of a panel it never asked for. Co-Authored-By: Claude Opus 5 (1M context) --- tests/bottom_panel_stage1_acceptance.rs | 64 +++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/bottom_panel_stage1_acceptance.rs b/tests/bottom_panel_stage1_acceptance.rs index 40f2b93..36efbe2 100644 --- a/tests/bottom_panel_stage1_acceptance.rs +++ b/tests/bottom_panel_stage1_acceptance.rs @@ -1660,6 +1660,70 @@ fn acc21_panel_visit_and_jump_back_returns_to_the_panel() { ); } +/// Bottom-panel Stage 3, criterion 5 — the OMITTED default degrades on a +/// frontend that cannot host a panel. +/// +/// `acc14` already proves capability fallback for an EXPLICIT +/// `request.side` at the core level. This is the Stage 3 case and it is +/// not the same one: the default is now resolved into a panel request +/// inside the adopter, so a pre-panel semantic frontend must degrade a +/// request the caller never wrote. The framing calls this the criterion +/// most likely to be quietly wrong, because the fallback is invisible +/// from the adopter's side — nothing in `listview.open` says "panel", +/// yet the request that reaches the core does. +/// +/// What must survive the degradation: no side window, no side +/// parameters, and NO QUIT ACTION left on the document window. A quit +/// action stranded on a document window would make a later `q` try to +/// restore a presentation that never existed. +#[test] +fn s3_2_the_omitted_default_degrades_on_a_pre_panel_frontend() { + let s = editor(); + let fid = FrontendId(31); + let document = attach_frontend(&s, fid, false); + s.core.borrow_mut().active_frontend = fid; + + // No `display` at all — the Stage 3 default resolves to a panel + // request, which this frontend cannot honour. + exec( + &s, + "pmacs.listview.open { name = \"*degraded*\", rows = { { text = \"row\" } } }", + ); + + assert!( + s.core.borrow().side_window_for(fid).is_none(), + "a pre-panel frontend gets no side window from the omitted default" + ); + { + let core = s.core.borrow(); + let window = &core.windows[&document]; + assert!( + window.params.side.is_none(), + "no side parameter is left on the document window" + ); + assert!( + !window.params.dedicated, + "the document window is not dedicated by a degraded request" + ); + assert!( + window.params.quit_action().is_none(), + "NO quit action is left behind — `q` must not try to restore a \ + presentation that never happened" + ); + assert_eq!( + core.registry + .borrow() + .get(window.buffer_id) + .expect("live buffer") + .name(), + "*degraded*", + "the buffer still reached the document target" + ); + } + + s.core.borrow_mut().active_frontend = FrontendId::LOCAL; +} + #[test] fn acc22_jump_histories_are_per_frontend_and_skip_stale_side_origins() { let s = editor(); From 480158b460eb2c2fd7c528a8c2ca0b1189caf125 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Mon, 3 Aug 2026 11:18:04 -0400 Subject: [PATCH 7/9] docs: record Arc 7 complete, and what Stage 3 taught MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Branch-plan step 6. The lane, the handoff, COHERENCE, and the framing all still described Stage 3 as ahead. COHERENCE §14's scorecard row and §20's Priority 5 both said the adopter default flip was what remained on the panel. It is done, so the panel half of P5 is closed and the TREE PRIMITIVE becomes the arc's successor — §14 grades Tree ✗, and DAP's variables view is its next would-be inventor. That sequencing is now stated where the next framing doc will read it rather than living in a conversation. The lane is RETAINED, not removed, because the PR has not landed; rule 4 removes it once Stage 3 merges and its facts are in the handoff. Both conditions are half-met, and recording that honestly is cheaper than discovering later which half. Five durable facts move to the handoff §1, each of which cost a review round or a defect to learn: * A VISIT FROM A PANEL MUST NEVER USE THE RAW SWITCH. `pmacs.window.switch_buffer` replaces the buffer in the ACTIVE window, so from a panel it clobbers the panel itself. Q#BP11c is the contract: after RET, `M-,` must FOCUS the still-present panel rather than clone its buffer into the document — and an assertion on the active buffer NAME alone cannot distinguish those two outcomes. * AN OPT-OUT THAT DOES NOT SURVIVE REPLAY IS NOT AN OPT-OUT. Anything that replays a stored invocation must store the escape hatch with it, or the escape hatch is a one-shot that silently reverts. * Compile's chords are PANEL-LOCAL, deliberately, and `M-x compile.kill` still reaches the slot from anywhere. A global chord is a command-surface decision framed separately. * Two `q` mechanisms coexist by design — presentation history chains in the side slot; `p.prev` prevents raw-switch and capability-fallback loops. Neither supersedes the other. * A CAPABILITY FALLBACK MUST STRIP THE QUIT ACTION TOO, not just the side parameters. A quit action stranded on a document window makes a later `q` try to restore a presentation that never happened. The framing reaches revision 4 with the three defects the flip exposed (§1.7a–c) and the census lesson (§1.6d): it counted FAILURES, not CAUSES, and the two differed by an order of magnitude — thirteen listview failures were one missing frame-geometry declaration. Read a census as "how many assertions move", never "how many decisions are required". Co-Authored-By: Claude Opus 5 (1M context) --- COHERENCE.md | 9 +- docs/active-work.md | 127 ++++++++++++++-------------- docs/agent-handoff.md | 37 +++++++- docs/bottom-panel-stage3-framing.md | 81 +++++++++++++++++- 4 files changed, 181 insertions(+), 73 deletions(-) diff --git a/COHERENCE.md b/COHERENCE.md index c447a76..0efa19f 100644 --- a/COHERENCE.md +++ b/COHERENCE.md @@ -107,7 +107,7 @@ remain open to them. | 11 | Config layering + provenance | **Partial (foundation only)** | Typed registry is right; 5 settings live in it; no value provenance | | 12 | Profiles | **Missing** | One hardcoded default keymap; not a named concept | | 13 | Package lifecycle UX | **Resolution without lifecycle** | Mature resolver/lockfile; init-only install; no uninstall/disable/search | -| 14 | Workbench primitives | **Partial (best trajectory)** | Listview is a real primitive but only 3 call sites, all LSP panels; buffer-list and search re-implement it; bottom panel complete on BOTH frontends (#155 + Stage 2) | +| 14 | Workbench primitives | **Partial (best trajectory)** | Listview is a real primitive but only 3 call sites, all LSP panels; buffer-list and search re-implement it; **the bottom panel is COMPLETE — both frontends, and Stage 3 flipped the adopter default so omission means the panel**. **Tree is still ✗ and is now the arc's successor** | | 15 | Contextual affordances | **Weak** | Right-click menu only; code actions apply first-blindly; no git integration at all | | 16 | Semantic frontend | **Strong** | v6..=v21 schema support; production attach remains v20 during the dark panel slice; degradation practiced | | 17 | Distribution | **Partial** | **v1.1.0 ships prebuilt Linux/macOS binaries on tag** (#211) with checksums and a stated glibc floor. No channels, in-place update, rollback, signing, or package-manager distribution | @@ -1667,10 +1667,9 @@ and §18's floor ride on this. ### Priority 5: Finish the workbench convergence -**State: partial and moving (§14) — the bottom panel is now complete on -both frontends (Stage 1 #155 through Stage 2B-3), and listview is -proven.** Only the adopter default flip (Stage 3) remains on the panel -itself. Remaining elsewhere: the tree primitive (build it before dired +**State: the bottom panel is DONE (§14) — both frontends, Stage 1 #155 +through Stage 2B-3, and Stage 3 flipped the adopter default so omitting +`display` means the panel. Arc 7 is complete.** Remaining elsewhere: the tree primitive (build it before dired and the worker tree invent two), table/inspector/diff, help unification. Wiring plus one modest model piece (the tree model). diff --git a/docs/active-work.md b/docs/active-work.md index a3f2f83..f11e10e 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -311,82 +311,81 @@ them) and why `dired`/`listview` were the correct first two families. Whichever starts second integrates first. -## Bottom-panel lane (Arc 7) — STAGE 3 IN PROGRESS on `bottom-panel-stage3` +## Bottom-panel lane (Arc 7) — STAGE 3 COMPLETE; ARC DONE, pending PR -**Stage 1, the Stage 2 framing, and Stages 2A, 2B-1, 2B-2 and 2B-3 are -all on `main`** (#155, #175, #177, #184, #187, #198). Stage 2 is -complete. Durable facts are in `docs/agent-handoff.md` §1, including the -v20-baseline / v21-negotiated handshake that Stage 2B-3 made compatible. +**Arc 7 is finished.** Stage 1 (#155), the Stage 2 framing (#175), +Stages 2A (#177), 2B-1 (#184), 2B-2 (#187) and 2B-3 (#198) are on +`main`; **Stage 3 — the adopter default flip — is implemented on +`bottom-panel-stage3`** and is the arc's last step. Framing +`docs/bottom-panel-stage3-framing.md` revision 3, approved with +amendments after three review rounds. -**Stage 3 — the adopter default flip — is Arc 7's last step and is NOW -IN PROGRESS**, not "not started" as this lane said until 2026-08-01. -Framing `docs/bottom-panel-stage3-framing.md` revision 3, approved with -amendments after one review round. +**This lane is removed once Stage 3 merges and its facts are in +`docs/agent-handoff.md` §1** (rule 4). It is retained now only because +the PR has not landed. -- **Branch `bottom-panel-stage3`**, based on `githubsucks/main` @ - `21de0b2`. Developed in the primary checkout on the laptop, not a - worktree. **No PR yet** — the flip has not landed. -- **Steps 1 and 2 of the framing's §7 are DONE:** - - `0224c68` — the **fallout census**, taken before any change: a - throwaway flip, a full `--no-fail-fast` sweep, then revert. **37 - failures across 5 suites**, classified per test in framing §1.6c. - - `41d37fc` — the **shared resolver** - (`resolve_adopter_display` / `pmacs.window._resolve_display`), - replacing four hand-written copies of the same validation. - **Default-preserving with one intentional normalization** (below). -- **Steps 3–6 are NOT done:** the flip itself, the ~37 test revisions, - the capability-fallback criterion, and the lane/handoff close-out. +- **Branch `bottom-panel-stage3`** off `githubsucks/main` @ `21de0b2`, + pushed, upstream tracking set. Six commits: `fa12095` framing, + `0224c68` census, `41d37fc` shared resolver, `8d14e6a` lane/portability, + `c0eb16b` the flip + test revisions, `5f01ede` the fallback pin. +- **Verification:** full serialized sweep **3449 passed / 0 failed** + against a 3447 baseline (+2 new pins); fmt, diff-check, clippy with and + without `crdt`, `--lib` 1896, `--lib --features crdt` 2081, + pmacs-protocol 19, m4 149, required GPU 221, and + `bottom_panel_stage1_acceptance` 47/47 under both Lua flavors. -### What the census established, and why it ran first +### What Stage 3 changed -- **A census that stops at the first failing binary is not a census.** - The first sweep reported 2 failures in 1 suite because `cargo test` - halts after a failing binary; `--no-fail-fast` revealed 37 across 5. - **Any re-measurement must pass that flag.** -- **`m4_acceptance` is a transitive adopter nobody named.** Its two - failures are the LSP hover and outline panels, which are **listview - consumers**. Q#BP12's four rows are the *direct* population; the real - one is everything built on listview. -- **The proportions invert the obvious reading.** `compile_mode` has the - most failures (17) and the least placement content; `listview_ - acceptance` has 13 but loses **three quarters of its suite**. +Omitting `display` resolves to the **panel** for listview, compile and +terminal. **Dired keeps `"current"`** — `pmacs.path.directory_handler` +calls it with no `display`, so a flipped default would open `pmacs .` in +a bottom panel. Per-adopter `select`: listview `true`, compile `false`, +terminal `true`. -### The normalization step 2 changed, deliberately +Four hand-written copies of the `display` validator collapsed into one +shared `resolve_adopter_display(operation, raw, default)`; the default is +a **parameter**, which is what makes dired's exemption visible at its +call site rather than hidden in a divergent copy. -Terminal previously raised **mlua's type error** for a non-string -`display`; the Lua adopters stringified into their own message. Now all -four take the shared custom error, rendered **by type alone** -(`unknown display (integer)`), because it names the legal vocabulary. +### Three defects the flip exposed, each fixed rather than tested around -**Pinned** in `bottom_panel_stage1_acceptance::acc19` at the terminal -entry point, asserting the error *and* that nothing is created. The type -**spelling** is deliberately unpinned — Lua 5.4 says `integer` where -LuaJIT has no integer subtype, so a literal assertion would pass on one -CI flavor and fail on the other. **Verified 46/46 under both.** +- **The outline panel's `on_visit` used the RAW switch** + (`pmacs.window.switch_buffer`), which replaces the buffer in the ACTIVE + window. Harmless while the outline opened into a document window; + once the panel became the default, RET **clobbered the panel with the + source**. The references panel was migrated to `display_file` when the + arc landed — the outline was missed because nothing exercised it from a + panel until the flip. Q#BP11c names this exact corruption. +- **`compile._last` stored only `{cmdline, cwd}`**, so `g` reached + `start_run` with no `display` and took the new default: an explicit + `display = "current"` silently reverted on the next recompile. **An + opt-out that reverts is not an opt-out.** +- **`opts.display` on a nil `opts`** — a regression introduced by the + fix above and caught by `journey_acceptance`, which is what that + ratchet is for. -### Verification at this head +### Two contracts now pinned, not merely observed -Full serialized suite **3447 passed / 0 failed**, with **zero suites -differing from the pre-change baseline** — the measurement that makes -"default-preserving" a claim rather than an intention. Plus fmt, -diff-check, clippy, `--lib` 1896, and -`bottom_panel_stage1_acceptance` 46/46 under **both** Lua flavors. +- **Compile's chords are PANEL-LOCAL.** All are bound + `scope = "buffer"`, so with `select = false` none dispatch from the + document — `C-c C-k` included. `M-x compile.kill` still reaches the + running slot anywhere via its `or compile_slot()` fallback. A global + chord is a command-surface decision and belongs in its own framing. +- **The two `q` mechanisms are complementary, not competing.** + Presentation history chains in the side slot (`C → B → A → delete`, + Q#BP2c); `p.prev` prevents raw-switch and capability-fallback listview + loops. `s1_12` pins the second with explicit `display = "current"`; + the new `s3_1` pins the first. -### Recovery from a clean checkout +### The census lesson worth carrying past this arc -```sh -git fetch githubsucks -git worktree add ../pmacs-bp-stage3 \ - -b bottom-panel-stage3 \ - githubsucks/bottom-panel-stage3 -``` - -- **DAP waits for Stage 2, not Stage 1** — satisfied. Q#BP12's adopter - table already carries a `DAP stack/variables` row, so Stage 3 settles - the debugger's panel policy before the debugger exists. -- **The tree primitive is the next thing to scope after this**, per - `COHERENCE.md` §14: Tree is graded ✗, and DAP's variables view is its - next would-be inventor. +**It counted failures, not causes.** Thirteen listview failures had ONE +root cause — a panel is derived-hidden while frame geometry is unknown, +and that suite never declared any because it never needed to. The same +applied to `m4_acceptance` and `vterm_stage2_acceptance`. And +**`--no-fail-fast` is mandatory**: the first sweep reported 2 failures in +1 suite because `cargo test` halts after a failing binary; the real +figure was 37 across 5. ## Reap-ledger silent failures — MERGED (#202); kept for its parked follow-ons diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index 77c7642..6b633aa 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -77,6 +77,41 @@ commands, read `docs/active-work.md` immediately after this file. #204 and **1b-3** #205, the ambient-root isolation **implementation** #206, and discovery Stage 1 #207. Each has its own bullet below; this line is the head-of-`main` anchor and nothing else. +- **Bottom panel Arc 7 COMPLETE — Stage 3, the adopter default flip.** + Omitting `display` now means the panel for listview, compile and + terminal; **dired keeps `"current"`** because + `pmacs.path.directory_handler` calls it with no `display` and a flipped + default would open `pmacs .` in a bottom panel. Four hand-written + copies of the validator collapsed into one + `resolve_adopter_display(operation, raw, default)` — the default is a + **parameter**, which is what makes dired's exemption visible at its + call site instead of hidden in a divergent copy. Durable facts: + - **A visit FROM a panel must never use the raw switch.** + `pmacs.window.switch_buffer` replaces the buffer in the ACTIVE + window, so from a panel it clobbers the panel itself. The outline's + `on_visit` still did this; the references panel had been migrated to + `display_file` when the arc landed and the outline was missed, + because nothing exercised it from a panel until the default flipped. + **Q#BP11c is the contract**: after RET, `M-,` must FOCUS the + still-present panel, not clone its buffer into the document — and an + assertion on the active buffer name alone cannot tell those apart. + - **An opt-out that does not survive replay is not an opt-out.** + `compile._last` stored `{cmdline, cwd}` only, so `g` re-resolved + `display` and silently reverted an explicit `"current"` to the new + default. Anything that replays a stored invocation must store the + escape hatch with it. + - **Compile's chords are PANEL-LOCAL, deliberately.** All are bound + `scope = "buffer"`, so with `select = false` none dispatch from the + document — `C-c C-k` included. `M-x compile.kill` still works + anywhere via its `or compile_slot()` fallback. A global chord is a + command-surface decision, framed separately. + - **Two `q` mechanisms coexist by design**: presentation history + chains in the side slot (`C → B → A → delete`, Q#BP2c), while + `p.prev` prevents raw-switch and capability-fallback listview loops. + Neither supersedes the other. + - **A capability fallback must strip the QUIT ACTION too**, not just + the side parameters — a quit action stranded on a document window + makes a later `q` try to restore a presentation that never happened. - **pmacs is installable without cloning — Distribution Stage 1, #211, released as v1.1.0.** A `v*` tag builds `pmacs` and `pmacs-gpu` on pinned `ubuntu-22.04` / `macos-15` and publishes a GitHub Release with @@ -181,7 +216,7 @@ anchor, so every item is startable. | 2 | Workspace + location | Missing; model gap | The long-lead arc. Start before a fifth subsystem grows its own root convention — four have already diverged (§7) | | 3 | Extension ownership | Missing; prerequisite-shaped | **`pmacs.hook.remove` does not exist.** That one bug-sized gap blocks §13's disable/uninstall, §10's trust classes, and package-scoped cancellation | | 4 | **Discovery** | **Stage 1 MERGED (#207)** | Stage 2 candidates, in rough dependency order: richer M-x rows (**protocol change** — `MinibufferPrompt.candidates` is `Vec`; `CompletionPopupRow` already proves the pattern), `Command` gaining title/category/aliases/flags/arg-schema (~147 definition sites), predicate evaluation, help-layer unification, and the help-prefix decision | -| 5 | Workbench convergence | Partial, best trajectory | Bottom panel done both frontends; **Stage 3 = flip the adopter default**. Then the tree primitive — build it *before* dired and the worker tree invent two | +| 5 | Workbench convergence | Partial; **Arc 7 COMPLETE** (Stage 3 implemented) | The bottom panel is finished on both frontends and the adopter default is flipped. **The tree primitive is now the arc's successor** — `COHERENCE.md` §14 grades Tree ✗, and DAP's variables view is its next would-be inventor. Build it *before* dired's `i` and the worker tree invent two | | 6 | Config productization | Foundation only | Value provenance, then layering, then adoption migration (**table-valued settings are the hard prerequisite** — `ConfigValue` is four scalars) | | 7 | Package lifecycle | Not started | Correctly sequenced after P3 | | 8 | **Distribution** | **Stage 1 SHIPPED (v1.1.0, #211)** | Binaries on tag, checksums, machine-checked glibc floor. **Journey step 1 now works and the "invisible until this exists" blocker is lifted.** Next is a *decision* about channels / update / signing, not a queued plan | diff --git a/docs/bottom-panel-stage3-framing.md b/docs/bottom-panel-stage3-framing.md index 2002fa0..fa59cf2 100644 --- a/docs/bottom-panel-stage3-framing.md +++ b/docs/bottom-panel-stage3-framing.md @@ -1,8 +1,21 @@ # Framing — Bottom panel Stage 3: the adopter default flip -**Revision 3.** Status: **approved; branch-plan step 1 (the fallout -census) is complete and recorded in §1.6b–c.** Branch -`bottom-panel-stage3`, based on `githubsucks/main` @ `21de0b2`. +**Revision 4.** Status: **IMPLEMENTED — all six branch-plan steps done; +Arc 7 complete.** Branch `bottom-panel-stage3`, based on +`githubsucks/main` @ `21de0b2`. Full sweep 3449 passed / 0 failed +against a 3447 baseline. + +**Revision 3 → 4** records what implementation found. The plan held; the +surprises were all defects the flip exposed rather than design changes: + +- **Three defects, each fixed rather than tested around** — the outline's + raw-switch visit (§1.7a), compile's non-replayed opt-out (§1.7b), and + a nil-`opts` regression of my own that `journey_acceptance` caught. +- **The census counted failures, not causes** (§1.6d): 13 listview + failures had ONE root cause, a missing frame-geometry declaration. +- **Compile's chords are panel-local** (§1.5a), pinned by `acc34`. +- **The two `q` mechanisms are complementary** (§1.7c), pinned by + `s1_12` and the new `s3_1`. **Revision 2 → 3** adds the measurement and its classification. Two findings changed the plan rather than confirming it: the census needs @@ -348,6 +361,68 @@ one is about where output goes and the other is about moving within it. A sweep that classified per *suite* rather than per *test* would have got both wrong. + +### 1.6d The census counted failures, not causes + +The measured 37 was accurate as a count and misleading as a work +estimate. **Thirteen listview failures had one root cause:** a panel is +derived-hidden while frame geometry is unknown, and +`listview_acceptance` never declared any — it never needed to while +listview defaulted to the current window. One helper took it from 13 to +2. `m4_acceptance` and `vterm_stage2_acceptance` were the same. + +Read a census as "how many assertions move", never "how many decisions +are required". The two differed here by an order of magnitude. + +### 1.7a Defect: the outline visited through the RAW switch + +`lsp.lua`'s outline `on_visit` called `pmacs.window.switch_buffer`, +which replaces the buffer in the **active** window. Harmless while the +outline opened into a document window — the switch simply reused it. +Once the panel became the default, the active window WAS the outline +panel, so **RET clobbered the panel with the source file** and left +nothing for `M-,` to return to. + +The references panel (`visit_location`) was migrated to `display_file` +when the arc landed; the outline was missed because **nothing exercised +it from a panel until the default flipped**. Its own neighbouring +comment states the rule it violated: *"a visit FROM a panel must land in +the document target and leave the panel intact."* + +Q#BP11c names the corruption precisely, and it is why both the outline +and compile tests now assert `M-,` **focuses** the still-present panel +rather than cloning its buffer into a document window — the previous +assertion, on the active buffer name alone, could not tell those apart. + +### 1.7b Defect: an opt-out that did not survive replay + +`pmacs.compile._last` stored `{cmdline, cwd}` and no `display`, so `g` +reached `start_run` with the value omitted and took the new default. A +user who ran `compile.run{display="current"}` was moved into a panel the +moment they recompiled. + +**An opt-out that reverts on the next replay is not an opt-out.** +`display` is now stored and replayed, with `nil` kept as `nil` so an +omitted value still resolves to the current default rather than freezing +at the first run's resolution. The general form: *anything that replays +a stored invocation must store the escape hatch alongside it.* + +### 1.7c The two `q` mechanisms are complementary + +Stage 3 made a latent conflict live. `listview.lua`'s `p.prev` captures +the previous buffer only if it is not a panel; `QuitAction::Restore` +(Q#BP2c) deliberately chains `C → B → A → delete`. With two listviews +sharing one bottom slot, the second replaces the first and `q` walks the +restore chain. + +**The restore chain wins**, per criterion 20 — listview `q` routes +through `window.quit` and must not exempt itself from the panel +contract. The mechanisms are then complementary rather than competing: +**presentation history chains in the side slot; `p.prev` prevents +raw-switch and capability-fallback loops.** `s1_12` keeps its Q#GB18 +name-keyed-identity bite by pinning its panels in document windows, +isolating `p.prev`; the new `s3_1` pins the chain. + ## 2. Questions - **Q#S3-1 — DECIDED: unify NARROWLY.** A shared From 6b4d52a5bc34b735d53bc36b556db0314b8a250a Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Tue, 4 Aug 2026 12:08:28 +0200 Subject: [PATCH 8/9] review round 1: the crdt suite Stage 3 missed, and two stale explanations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1 — compile_mode_crdt_acceptance was never revised. Its three `compile.run` fixtures still omitted `display`, so the flip placed output in the panel with `select = false`, the source buffer stayed active, and `adopt_next_buffer` timed out waiting for a snapshot that only arrives via the ACTIVE-BUFFER-FOLLOW path. Its subject is CRDT convergence of a generated buffer, not placement, so it takes the explicit opt-out — and the reason is recorded at `adopt_next_buffer`, the helper that actually depends on it, rather than at each call site. WHY MY OWN VERIFICATION MISSED IT, which matters more than the fix: the Stage 3 census and every sweep I ran used `--features luajit` WITHOUT `crdt`, so no crdt-gated suite was ever exercised. The census was therefore blind to an entire configuration by construction, in exactly the way #209 exists to prevent. The CI crdt job — added by that lane — is what caught this, three days after it landed. Baselined rather than assumed. A worktree at the branch base 21de0b2 sweeps 3715 passed / 0 failed under `luajit,crdt` with PMACS_REQUIRE_GPU=1; the branch with this fix sweeps 3715 / 0. Identical. An earlier branch sweep, taken before this fix, reported SEVENTY failures across THIRTEEN suites. Twelve of those suites — m5_5, gpu_invocation, gpu_initial_target, m10_11, vterm_stage3 and the rest — are daemon, socket or GPU suites, and all of them recovered by fixing three compile tests. The likeliest reading is that the failing compile-crdt runs leaked daemon or PTY processes that poisoned every subsequent socket-based suite; what is ESTABLISHED is narrower and still useful: baseline green, branch green with the fix, and the collateral confined to process-spawning suites. Count failures, not causes — the same lesson this stage already learned once, at a different layer. Also of note: CI reported only 3 failures because `cargo test` halts after a failing binary. The workflow does not pass `--no-fail-fast`, so CI under-reports a multi-suite break exactly as my first census did. P2 — builtin/runtime/compile.lua's recompile comment still said `_last` stores only cmdline/cwd. This PR deliberately stores `display` too, so an explicit opt-out survives replay. Corrected, with the `display_omitted` arm's remaining purpose stated rather than implied. P2 — the framing's §7 step 2 still called the resolver extraction "provably behaviour-preserving", contradicting §1.6b's own record of the intentional non-string normalization. It is DEFAULT-PRESERVING WITH ONE INTENTIONAL NORMALIZATION, and now says so where a reader following the branch plan will hit it. Verified: fmt, diff-check, compile_mode_crdt 3/3, compile_mode 73/73, and the full crdt sweep at 3715/0 against a measured baseline of 3715/0. Co-Authored-By: Claude Opus 5 (1M context) --- builtin/runtime/compile.lua | 15 ++++++++++----- docs/bottom-panel-stage3-framing.md | 9 ++++++--- tests/compile_mode_crdt_acceptance.rs | 13 ++++++++++--- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/builtin/runtime/compile.lua b/builtin/runtime/compile.lua index 20300cc..22fc643 100644 --- a/builtin/runtime/compile.lua +++ b/builtin/runtime/compile.lua @@ -893,11 +893,16 @@ local function start_run(slot, cmdline, opts) -- not preempt the requested panel. Compile output is passive, so it -- takes `select = false` explicitly. -- - -- A recompile reaches here with NO `display` (only cmdline/cwd are - -- stored in `_last`), so the raw switch below would put this buffer in - -- the selected DOCUMENT window while the panel still shows it — the - -- duplicate presentation this arc removes elsewhere. Detect that the - -- buffer already owns the panel slot and keep it there. + -- A recompile REPLAYS `_last`, which since Stage 3 carries `display` + -- alongside cmdline/cwd — an opt-out that did not survive replay + -- would silently revert to the panel on the next `g`. So an explicit + -- `display = "current"` reaches here again on a recompile, and must + -- still take the raw switch below. + -- + -- The `display_omitted` arm remains for the genuinely omitted case: + -- it keeps a buffer that already owns the panel slot in the panel + -- rather than duplicating it into the selected DOCUMENT window while + -- the panel still shows it. -- -- Gated on OMISSION, never on an explicit value: `display = "current"` -- is the documented user-facing opt-out from the Stage 3 default flip, diff --git a/docs/bottom-panel-stage3-framing.md b/docs/bottom-panel-stage3-framing.md index fa59cf2..18ba3b3 100644 --- a/docs/bottom-panel-stage3-framing.md +++ b/docs/bottom-panel-stage3-framing.md @@ -572,9 +572,12 @@ One branch, `bottom-panel-stage3`: measurement is the first commit's evidence, not the flip.* Classify each mover per §1.6a's rule before writing a line of the fix. 2. **Land `resolve_adopter_display`** (Q#S3-1) with all four callers - still passing their CURRENT defaults, so the unification is provably - behaviour-preserving before anything flips. Decide and pin the - non-string normalization here. + still passing their CURRENT defaults, so nothing flips yet. It is + **default-preserving with one intentional normalization**, not + "behaviour-preserving" — every adopter keeps its default and the + suite is byte-identical to baseline, but terminal's invalid-input + behaviour moves deliberately (§1.6b). Decide and pin that + normalization here, since no existing assertion can catch it. 3. **Flip the three sites** by changing only the `default` argument at listview, compile and terminal — dired keeps `"current"` — with per-adopter `select`. diff --git a/tests/compile_mode_crdt_acceptance.rs b/tests/compile_mode_crdt_acceptance.rs index ba79d49..a6970d9 100644 --- a/tests/compile_mode_crdt_acceptance.rs +++ b/tests/compile_mode_crdt_acceptance.rs @@ -105,6 +105,13 @@ where /// the daemon broadcasts a snapshot for the newly-CRDT-backed buffer /// and via the active-buffer-follow path). Re-seats the replica's /// mirror on that buffer. +/// +/// **Bottom-panel Stage 3:** this is why the runs below pass an explicit +/// `display = "current"`. The default now places compile output in the +/// panel with `select = false`, so the ACTIVE buffer never becomes +/// `*compilation*` — and the active-buffer-follow path named above is +/// what publishes the snapshot this function waits for. The subject +/// here is CRDT convergence of a generated buffer, not placement. fn adopt_next_buffer(replica: &mut Replica, what: &str) { let deadline = std::time::Instant::now() + Duration::from_secs(10); loop { @@ -188,7 +195,7 @@ fn compile_run_converges_and_replica_edit_triggers_recovery() { name = "test.compile", description = "compile-mode CRDT fixture trigger", fn = function() - pmacs.compile.run("sh {script}", {{ cwd = "{dir}" }}) + pmacs.compile.run("sh {script}", {{ cwd = "{dir}", display = "current" }}) end, }} pmacs.keymap.bind {{ scope = "global", sequence = "C-c 9", command = "test.compile" }} @@ -275,7 +282,7 @@ fn r3f1_unicode_cr_backspace_survive_crdt_replication() { name = "test.compile-unicode", description = "round-3 unicode fixture trigger", fn = function() - pmacs.compile.run("sh {script}", {{ cwd = "{dir}" }}) + pmacs.compile.run("sh {script}", {{ cwd = "{dir}", display = "current" }}) end, }} pmacs.keymap.bind {{ scope = "global", sequence = "C-c 8", command = "test.compile-unicode" }} @@ -327,7 +334,7 @@ fn r4f1_column_rewrites_replicate_and_converge() { name = "test.compile-columns", description = "round-4 column-rewrite fixture trigger", fn = function() - pmacs.compile.run("sh {script}", {{ cwd = "{dir}" }}) + pmacs.compile.run("sh {script}", {{ cwd = "{dir}", display = "current" }}) end, }} pmacs.keymap.bind {{ scope = "global", sequence = "C-c 7", command = "test.compile-columns" }} From fdbeb5cc32428c32ea7f932d4770c793445ef12a Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Tue, 4 Aug 2026 18:00:58 +0200 Subject: [PATCH 9/9] fix(test): the GPU terminal suites measure geometry the flip changed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage 3 review round 2. `vterm_stage3_acceptance`'s three terminal fixtures opened with no `display`, so the flip placed them in a 12-row panel — and this suite measures RENDERED FRAMES and CHILD PTY GEOMETRY. It was measuring the geometry the flip had changed underneath it. I nearly recorded this as a flake, and the reasoning that stopped me is worth keeping. Two CI runs on the same commit failed DIFFERENT suites: the first `vterm_stage2`'s real-TUI smoke, the rerun `vterm_stage3`'s a37 plus two GPU terminal tests. Different failures across runs is the load-sensitivity signature, both suites are on the documented flake surface, and the ledger says a red a37 is "ambiguous by construction". Every indicator pointed at noise. But the tests it kept landing on were GPU TERMINAL tests, and terminal placement is exactly what this PR changed. Checking rather than concluding: all three `terminal.open` fixtures omitted `display`. So the flip did reach them. It did not BREAK them — they pass locally, and they passed in one CI run each — it made them MARGINAL, by shrinking the window whose rendered output they assert against. Marginal under lavapipe on the heaviest job in the workflow reads exactly like a flake until you ask which tests, and why those. The distinction that matters: "my change made this fragile" is a different finding from "this was always flaky", and only one of them is mine to fix. These take the explicit opt-out for the same reason `vterm_stage2`'s smoke already did — their subject is rendering, geometry and input round-trip over a full document window, not placement, which the panel suites cover. The reason is stated at each fixture rather than once at the top, because each asserts a different property of the geometry. Verified: vterm_stage3 9/9 with a37 taking 4.33s rather than the 0.17s that means it never ran; vterm_stage2 6/6; and the full crdt sweep at 3718 passed / 0 failed against a measured 3715/0 baseline, the +3 being this PR's new pins. Co-Authored-By: Claude Opus 5 (1M context) --- tests/vterm_stage3_acceptance.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/vterm_stage3_acceptance.rs b/tests/vterm_stage3_acceptance.rs index afa4d83..0305146 100644 --- a/tests/vterm_stage3_acceptance.rs +++ b/tests/vterm_stage3_acceptance.rs @@ -608,8 +608,14 @@ pmacs.command.define { name = "vterm-probe.open", description = "Open the Stage 3 acceptance terminal child.", fn = function() + -- Bottom-panel Stage 3: explicit opt-out. This suite measures + -- RENDERED FRAMES and child PTY geometry against a full document + -- window; the panel default would put the child in a 12-row side + -- window and change the very geometry under test. Placement is + -- covered by the panel suites. return pmacs.terminal.open { command = "/bin/sh", + display = "current", args = { "-c", "i=0; while [ $i -lt 400 ]; do printf 'VTERMROW%02d\n' \"$i\"; i=$((i+1)); sleep 0.05; done" }, } @@ -1140,8 +1146,11 @@ pmacs.command.define { name = "vterm-probe.open", description = "Open a quiet terminal that counts SIGWINCH.", fn = function() + -- Stage 3 opt-out: this test asserts the child's PTY geometry + -- settles and stops signalling. A panel changes that geometry. return pmacs.terminal.open { command = "/bin/sh", + display = "current", args = { "-c", "n=0; trap 'n=$((n+1)); printf \"WINCH %d\r\n\" \"$n\"' WINCH; " .. "printf 'READY\r\n'; while :; do sleep 0.2; done" }, @@ -1163,8 +1172,11 @@ pmacs.command.define { name = "vterm-probe.open", description = "Open a terminal child that copies stdin to stdout.", fn = function() + -- Stage 3 opt-out: input must round-trip through a frame rendered + -- over the document window this test measures. return pmacs.terminal.open { command = "/bin/sh", + display = "current", args = { "-c", "printf 'READY\r\n'; exec cat" }, } end,