feat: add pinned preview - #137
Merged
Merged
Conversation
…C-18, AC-19, AC-22, AC-25, AC-36
…-12, AC-14, AC-18, AC-19, AC-25, AC-36
… AC-4, AC-5, AC-6, AC-7, AC-8, AC-9, AC-10, AC-19, AC-23
…AC-6, AC-7, AC-11, AC-24, AC-33, AC-34, AC-35
…-17, AC-26, AC-27, AC-31, AC-32, AC-36
Keep the finder because the narrow PTY can hide an active tree selection, while its unique UNPIN match preserves the active-preview assertion. Remove its state-dependent z toggle and the help-overlay barrier, then dismiss the committed search and send two q keys in one write so either finder zoom polarity exits cleanly.
The panic fix was right, removing TreeScrollRight from unavailable_from_pinned was not: the plan (T-7) requires rejecting the pinned L arm with the standard pinned-action notice, and AC-32 requires an unavailable action to produce a visible notice. Restore the policy entry and its AC-31/AC-32 assertion, and keep Effects::noop() in the match arm purely as a defensive net so a future policy change cannot turn a keypress into a process abort.
The coverage pass replaced a 250ms wall-clock poll with an immediate provider-count read, which is deterministic but weaker: dispatch_render runs the provider on a spawned thread, so an immediate count read can pass before a wrongly dispatched render increments it. Expose Controller::render_seq (bumped synchronously inside dispatch_render, before the worker spawns) and assert on that, with the provider count as corroboration. Verified by injecting a dispatch_render into the pin path: the oracle fails, as it must.
From pinned focus, q/Esc quit past the active preview's still-highlighted search (and vice versa): the close layer only consulted the focused preview. Dismiss the focused preview's search first, then the other preview's while its pane is on screen; a hidden pane's stale search no longer costs a keypress that visibly changes nothing.
scroll_pinned_h and scroll_pinned_to_line cloned every content line per event to dodge a two-field borrow; borrow through the snapshot instead (the pattern the pinned search already uses). set_pinned_viewport gains the same unchanged-size early return as its active twin, so idle draws stop re-walking the frozen document.
The Presenter draws only the active/pinned projections; the thirteen alias fields were read by no src code yet cost a second full deep clone of the rendered content on every frame, pin or no pin. Tests migrate to the projection fields (and to the Controller accessors where they were probing controller state, not the draw model). Every pre-existing snapshot is byte-identical.
PreviewDocument and PreviewProjection now hold Arc<Text>, so building the per-frame active and pinned projections is a pointer bump instead of a deep copy of every styled line, and a wrap/view presentation change re-shares the lines instead of re-copying them. The one remaining draw-side copy (Paragraph needs an owned Text) is documented in place.
docs/usage.md listed 11 unavailable-from-pinned keys while the policy rejects 12 — add L. The new divider test asserts interior columns map proportionally (including one step inside each clamp bound), so a snap-to-extremes or wrong-origin mapping fails instead of hiding behind the 20/80 endpoints.
smarzban
added a commit
that referenced
this pull request
Aug 4, 2026
…e tells The last outstanding case from the review. Three tests proved "no render was dispatched" by sleeping 50ms and hoping a wrong render would have landed by then; two more slept 50ms to give a superseded render a chance to arrive. - Adds Controller::render_seq (a read-only accessor; both dispatch_render and dispatch_reflow bump latest_seq synchronously BEFORE spawning the worker), and the three absence tests now assert an unchanged seq. That is a real proof, not a race: verified by dropping the `width_changed` gate so a height-only change reflows — the test fails. - The two superseded-render tests wait on an in-flight counter reaching zero instead of a fixed sleep, so they wait for the worker actually finishing rather than guessing how long that takes. HONEST LIMIT, recorded in the code and in AGENTS.md rather than glossed: making that wait deterministic does not make the detection reliable. Renders run a thread each and the polling loop drains earlier results while waiting for the newest, so perturbing poll's `seq == latest_seq` guard does NOT fail those two tests. They remain end-to-end sanity checks; the guard's real proof would be a unit test feeding poll a stale-seq result directly. Better to write the limit down than to leave a future reader trusting them past it. No 50ms blind sleeps remain in the file. Note for PR #137: it adds an identical render_seq on its branch, so it will need its copy dropped when it rebases onto main.
Brings v1.15.0, the agent-skill --cwd fix (#141) and the determinism work (#142) onto the branch. Four resolutions worth naming: - src/controller/mod.rs: both sides had added `render_seq` at different offsets, so git merged BOTH copies silently rather than conflicting. Dropped this branch's earlier copy and kept main's, whose doc describes the real single ordered worker (this branch's said "before it spawns the worker", which was wrong — there is one long-lived worker, not a thread per render). - CHANGELOG.md: the merge landed the pinned-preview entry inside the RELEASED 1.15.0 section, which would have claimed an unshipped feature was in that release. The repo's own pinned_preview_docs_cover_the_frozen_reference_contract guard caught it; moved to Unreleased. The advisory-notices entry stays deleted per the release decision. - AGENTS.md: took main's wording in both hunks — it replaced the two facts this branch had merely bumped (Intent::ALL's count, the ratatui patch version) with an instruction to read them from the source. - tests/docs_consistency.rs: kept both sides' additions (this branch's ARCHITECTURE const and main's --cwd drift guard). Green after merge: cargo test, fmt, clippy.
…e away Applies the 2026-08-09 spec amendment (AC-12, AC-14 to AC-18, AC-22 amended; AC-41, AC-42 added). - Order is now tree | active | pinned, so the preview that tracks the cursor sits beside the tree and the frozen snapshot is parked at the edge. The session ratio still names the pinned share, now measured from the right. - Pinning never reflows or collapses. Below the 40-column preview floor the geometry is identical to the same session with no pin: the pin is simply not drawn, and a persistent "Pinned: <path> - widen to view" notice sits on the active preview. The held pin survives at full fidelity and reappears when there is room. Previously `p` at any ordinary width left only the tree. - Focus cycles tree, active, pinned, and skips an undrawn pin; resizing is inert while the pin is hidden. - The pinned title shows the captured branch and root only. The path led the title and clipped the origin away on a narrow pane, defeating AC-12's whole purpose; it stays available through the widen notice and `y`/`Y`. - A finder confirm from pinned focus moves focus to the active preview, where the file opened. Changed-file jumps still keep pinned focus per AC-31. - Replacing a pin because the branch changed now names both branch states instead of silently turning `p` from unpin into replace.
Amends AC-12 again. `Pinned: [main]` for a pin from the worktree you are viewing, `Pinned: [main @ other-checkout]` when it came from another one. Repeating the current worktree's own name distinguished nothing and cost the width that clipped the origin away in the first place. The comparison is over whole root paths, decided in the controller where both are known, never over basenames: two worktrees of one repo routinely share a basename, and comparing those would report a foreign pin as local, which is exactly the confusion AC-12 exists to prevent. Intended consequence, recorded in the spec: a worktree switch leaves the frozen snapshot untouched (AC-11) but does change whether its title names a worktree, because what it has to distinguish itself from has changed. tests/reroot.rs pins that behaviour.
… search prompt open poll() routed an open Search prompt to refresh_search unconditionally, but refresh_search dispatches by prompt_target: a prompt opened on the pinned preview recomputed only the frozen pinned matches and skipped the is_reflow branch, leaving the active preview's committed matches computed against pre-reflow rendered lines (wrong highlights, n/N jumping to stale rows). The active side now takes the promptless rule whenever the prompt belongs to the pinned side.
pi-empanel writes run artifacts here. They are machine-local and should not ship with the plugin.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds one session-only pinned reference beside the tree-driven active preview. A pin freezes the
currently displayed file view and its viewport/search state, while the active side continues to
follow tree selection. The feature supports independent focus, scrolling, search, captured-path
copy, responsive three-region/single-region layouts, keyboard and mouse divider resizing, and
worktree-safe pin identity.
Acceptance criteria
AC-30 was withdrawn before build: displayed-text selection/copy from pinned focus is deliberately
out of scope; captured-path copy remains covered by AC-28 and AC-29.
Coverage
Verification
Fresh ship-boundary verification at
9e181f8passed:cargo fmt --checkcargo clippy --all-targets -- -D warningscargo test(including the pinned-preview E2E)cargo build --releasecargo audit(the repository's already-allowedRUSTSEC-2026-0190warning foranyhow 1.0.102)Agent SDLC corroboration passed:
sdlc-check 0.18.0: all checks passed — 0 findings, 0 notes.Provenance
The run entered at the brief stage from the owner prompt (ingested 2026-07-30); acceptance criteria,
design, stack, and plan were produced in this run. The only mid-build amendment added the necessary
internal
src/controller/pinned.rsaccessors to T-7; it changed no scope, criterion, component, ortask ordering. No links are untraced.
The full local spec and task ledger live under gitignored
docs/specs/; this PR body publishes theacceptance contract, coverage, and proof map for review.