Repository navigation
docs: plan the cross-session search in eight tasks - #43
Merged
Merged
Conversation
The spec in #40 said what to build. This says how, in the order it gets built, with the exact signatures and the test that proves each step. Two places where the plan refines the spec rather than restating it: - Counting pushed lines. The spec says to count every linefeed in the primary screen. A linefeed above the bottom line pushes nothing into history, so that overcounts. The plan takes the history_size delta, which is exact below the scrollback cap, and only estimates when the delta is zero because the cap is reached. - Enter verifies. Since the count can still drift, Enter checks that the line it computed still holds the query and hands over to find when it does not. Redactor::redact is behind #[cfg(test)] today; the result rows need it, so Task 4 takes the gate off. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat: give cmd-shift-f its own action instead of falling through to find Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: keep shifted symbol keys working on the physical key path Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: count the lines pushed into history so a row can be dated Grid::history_size() plateaus at the scrollback cap, so a row carries no usable age once the history is full. Wrap the vte Handler on its way to the Term and count the rows pushed into the history ourselves, keeping a small ring of (pushed, Instant) marks beside it. Counting hooks every path that can grow the history, not just linefeed and scroll_up: a wrapped line, NEL, SU, DL at the top of the screen and ED 2 all reach Grid::scroll_up through Term's own linefeed, so counting only the Handler's linefeed would miss them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * perf: take the line delta once per advance instead of per character Counting the wrapped lines in Handler::input cost 0.6ns per character, 5-8% of the parse stage on text-heavy loads, which spec 4 rules out. Read history_size() when the wrapper is built and again when it is done instead, and take max(delta, what the line-level hooks counted): below the scrollback cap the delta is the truth and already contains the wraps the hooks cannot see, and at the cap the hooks are the only signal. The one advance that crosses the cap undercounts, bounded by that advance. Ticks now keeps one mark per second rather than one per read, so the 4096-mark ring spans about 68 minutes instead of a few busy minutes; the spec asks for one second of resolution. The --throughput harness now runs through the wrapper, because it claims to measure the real read path. Measured over both: no difference outside the harness's own run-to-run spread. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: pick the counting rule by the cap, not by whichever count is larger max(delta, hooks) let an over-counting hook win below the cap: a main screen program that narrows the scroll region with DECSTBM scrolls inside the region and pushes nothing into the history, while the line-level hooks see a linefeed on the bottom row and count it. Below the cap the delta is the truth, so use it; the hooks are only the answer once the history can no longer grow. Telling those two zero-delta cases apart needs the configured scrollback, so Counting::new takes it again and the reader threads it through ReaderFlags. The tests now rebuild the wrapper once per read, as the reader does, instead of feeding a whole script through one advance. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: count past the cap in the read that reaches it, and date bursts right Two defects the review found. The advance that straddles the cap counted only the delta, so every push after the history filled was lost. One read carries up to 64KB, which is thousands of linefeeds, and rows produced in that read then compressed into fewer serials than there were rows -- the serial is supposed to be unique per row. finish() now takes max(delta, hooked) when the history reached the cap during the advance; which rule applies is still decided by the cap, so a narrowed scroll region below the cap stays at zero. The tests feed a whole script through one advance now, the way the reader does, instead of splitting it per line. The tick carried the count from after the read, so every row of a read landed above the newest mark and resolved to the previous one. After an idle gap that dates a whole burst minutes early, which is exactly what the search must not do. Record the count from before the read instead; the residual error is one screen height of rows dated slightly new. Also: restore the scroll-region over-count to Counting's documented limits -- it is gone below the cap, not above it, since every hook reads the screen rather than the region -- and correct the ring's footprint to the 96KB a 24-byte mark actually costs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: mark the ticks only on a read that pushed a line A read that pushes nothing still took a mark, and because equal counts are not stacked it took the one the next pushing read needed: after a burst, the prompt redraw marked (N, t_prompt), and the output of the next command -- minutes later -- also carried before == N and was dropped, so its rows resolved to the prompt's time. A prompt sits between every pair of interactive bursts, so this defeated dating in the ordinary case. A mark (p, t) claims the rows from p up to the next mark, so a read that pushed nothing has no range to claim. Ticks::mark_read(before, after, at) now owns that decision, next to the gap and ordering guards it shares, and the reader hands it both counts once per read outside the lock. Marks are no longer spent on prompt redraws, so the ring reaches further back than the 68 minutes a mark per second would give. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: match a line case-insensitively and keep the hit centered Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: scan every session newest first, under a per-session cap Walk each session's screen and scrollback from the newest line back, releasing the term lock every `chunk` lines so the pty reader keeps running, and date each hit from the session's ticks after the lock is dropped. Cap at 20 hits per session and 200 overall, fold consecutive identical lines into one, and match the row only after the redactor has run so a masked credential cannot surface in plain text. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: date every line hit and leave alt-screen hits unnumbered Pin only name, title and branch hits to the top of the list. Keying that rule on `at` put a quiet session's undated screen rows above the newest hits of every other session, and the cap on the total then handed them the seats; undated rows now sort last instead. Seed the ticks ring at construction, so a session that has not pushed a line yet still dates its rows, to session start. Default builds a seeded ring too, leaving the test-only constructor as the one unseeded route; the ring tests that drive push and mark_read directly take it. Drop the serial from a hit found on the alternate screen. `pushed` does not advance there, so the number aliased an unrelated main-screen history row: time_of dated the hit from that row and the jump would have aimed at it. Those hits take their time from the newest mark instead. Clear the stale dead-code markers in lines.rs now that the scan reads serial, line_of and time_of. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: date a quiet session's rows from its construction time The seed mark took the slot `(0, t_start)`, so the first read that actually pushed a line lost its own mark to the "same count, no mark" guard and its rows were dated to session start. The case that hurts is the one this feature exists for: a session that sat at a prompt for twenty minutes before the user typed the request they are now searching for. That row is in the first pushed screenful. Store the construction time in Ticks and leave the ring empty. time_of and newest fall back to it, so a session that has never pushed a line still answers, while the first pushing read's mark is now accepted. The recording rule is untouched. The test-only unseeded constructor goes with the seed. scan's comparator moves into `newest_first`, so the undated-line branch -- a guard now rather than a live path -- is tested directly instead of through a hole in the fixture. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: run the scan off the UI thread and drop stale generations The scan walks 14 sessions by 10000 lines; the UI thread has about 0.3ms of slack per frame, so it never calls scan itself. A keystroke only stamps typed_at, the draw end starts the walk once typing has paused for 150ms, and the worker hands the hits back as UiEvent::Scanned. Every result carries the generation it was started for, so a slow walk that lands after a newer one is dropped instead of replacing it. The generation counter lives on State, not in the box, so reopening the box cannot collide with a walk still running. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: drop the hits when the word is erased, and test the debounce Erasing the word returned from start_scan before the generation was bumped, so a walk still running for the erased word came back, matched accept and installed its hits; with no walk in flight the old hits simply stayed. accept is the only other writer of hits, so nothing afterwards corrected either case. The generation now advances whether or not a walk is issued, the empty branch clears the hits through CrossSearch::clear_hits, and it asks for a redraw when it actually dropped something -- start_scan runs after the frame was presented, so the stale list is already on screen. The debounce decision moves out of draw into scan_due, which returns Idle, Start or WakeAt and is now under test. The Start arm carries no instant, so starting a walk cannot also register a wake-up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: put the search box in the left pane and list the hits there Nothing on this branch had run yet: with no caller for the key, across was never Some, so the spawn, the 150ms debounce, the wake-up and the Scanned arm had never executed once. ⌘⇧F now opens the box, the box is drawn in the header band whether or not it holds focus, and typing in it queues a scan. While the word is non-blank the session list is replaced by the hits. A hit takes two lines: the first is the ordinary row -- state dot, name, ⌘ label -- and the second is the matched text, trimmed around the match with fit_center so the word itself cannot be the part that falls off, with search_hit laid under it. A blank word is not a zero-hit list; it is the session list again, and showing_hits decides that by the same rule start_scan uses to refuse to walk. Hit.id is looked up by id, never by position, because the session may have exited while the walk ran; such a row still shows its text. Hits lag the typed word by one walk, so an empty list says "searching…" while one is queued and "no match" only once none is. The result list takes no reordering and no drag, and the layout reserves no third line. The draw path reads nothing but the hits already in hand -- no scan, no Term lock, no work proportional to the scrollback. draw_terminal no longer claims cursor_px while the box has it, so the IME candidate window follows the box. TERMIT_PROBE_OVERLAY=across fills in four synthetic hits -- long, multi-byte, a title hit, and one naming a session that does not exist -- so --probe can show the list without a window. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: say searching while a scan runs, keep the box inside its bounds, allow paste - scan_note tells a running scan from a finished empty one - a note is always drawn when no hit row fits - preedit in the search box is clipped to the box and uses its colour - clicking the box or a hit no longer schedules a state save - Cmd+V pastes into the query with newlines turned into spaces - HitRow.closed replaces the exit = Some(0) stand-in Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: keep the tail of a long preedit visible in the search box - clip the preedit with fit_head so the end being typed stays - split_box_width reserves MIN_PREEDIT for the preedit while composing - the probe preedit is now long enough to overflow the box Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: let the preedit use all the room the query left unused The reserved minimum applies to the query budget only; the preedit now measures its room from where the query actually ended. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: jump to a hit by its serial and fall back to find Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: leave the viewport alone when a hit's line has fallen out of history Pull the landing decision into a pure function, build the needle with scan::needle so the recheck matches what the scan matched, and recheck the row with the scan's own predicate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: measure the scan and record the alt-screen hole Add a 14 x 10000 line scan section to --bench (about 49ms median, worst case: no early stop), document it as performance.md 7.15 with the static argument for why frames are not dropped, mark the live frame-log check as not yet done, and note the alt-screen hole and the new key in the README. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: tighten three claims in the scan notes State why the bench word appears once per session, narrow the draw-path claim to the draw functions (start_scan is reached at the end of a frame but only spawns the worker), count chunks per session, and name only vim and less in the README. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: mark the tick where the read starts writing `serial = pushed + line` and `line = 0` is the top of the screen, not the cursor. Marking a read at the pushed total it started with therefore claimed every row from the top of the screen downwards, painting up to `screen_lines - 1` rows that an earlier read had written with this read's time. An agent printing one line twenty minutes later gave its previous answer's last 46 rows a fresh timestamp, and those rows are the ones the user is looking for. `Counting` now records the cursor line it started on, the reader takes it from the first `Counting` of the read, and the tick lands on `before + start_line` — the first row this read actually writes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: publish the pushed total under the term lock The reader dropped the `term` guard and only then stored the pushed total, so a scanner that took the lock in that gap saw an advanced grid with a stale total. That session's serials came out one read low — thousands of lines with a 64KiB read — `time_of` returned a far older tick, and the rows the user had just watched scroll past sank down the list or fell off the truncation. The `Enter` path re-checks the row and degrades safely; the ordering path has no such check. The store is an atomic, so moving it inside the guard adds no lock and keeps the reader's `term` -> `ticks` order intact. `scan.rs`'s claim that the total is read under the grid's own lock is now true on the writing side too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: hand a case-insensitive pattern from a hit to the screen search The cross-session scan lowercases unconditionally, but `RegexSearch::new` is smart-case. Opening a hit put the typed word straight into `⌘F`, so a word with any uppercase letter in it — `SES`, `AWS`, `TODO`, `Error` — that had matched a lowercase row landed on the row and then highlighted nothing, and `find_step` either jumped to an unrelated older match or did not move. The alternate-screen path, whose only job is handing the word over, broke the same way. `ScreenSearch` grows an `ignore_case` flag that prefixes `(?i)` after the escaping, set only on the hand-off from `Enter`. `⌘F`'s own smart-case is untouched, and `query` keeps the typed characters so the bar reads the same. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: keep the counting when a read clears the scrollback A read that pushed lines and then emitted ED 3 (`\x1b[3J`) threw its whole count away: the history difference came out zero, `after >= cap` was false, and `finish()` returned the difference instead of the thousands of lines the per-line path had hooked. `clear` emits ED 3 whenever terminfo carries `E3`, so this is a daily command. The damage is bounded — the lines are gone and `open_hit`'s row re-check catches a stale serial — but the count itself has a hole, and every later serial in that session comes out low. The shrink test `after < before` alone does not see it: alacritty clears the history only when it is non-empty, so a read that pushes first and clears after leaves both sides at zero. `Counting` therefore marks the discard where it happens, and `finish()` takes the hooked count on either signal. RIS is left on the general shrink test and noted as a hole. Also records the ED 3 and over-the-cap cases in the module's hole list, which only named wrapping, DECSTBM and `resize`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: make the tick lookups total `Ticks::time_of` and `Ticks::newest` both fall back to the session's creation time, so neither can return `None`. Handing back an `Option` anyway forced a guarded ordering in `newest_first` and a test for a state no scan can produce, and left `Hit.at` readable as either "not a line" or "no time could be found". Both now return `Instant`. `Hit.at` keeps its `Option` with one meaning: `None` iff the hit is a name, title or branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: enter the cross-session search through one door `⌘⇧F` and clicking the search box each set `across` up with different preconditions: the key path cleared `picker`, `search` and `find` and opened the pane, the click path cleared only `find`. The two are now one `enter_across`, so neither can be fixed alone. The divergence was not reachable in practice — `on_click` closes `picker`, `search` and `rename` and returns before it ever reaches the search box — but it only held because of an ordering two functions apart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: record the wrap hole, rename a test, reuse the scan's needle Three small ones from the branch review: - The scan reads one grid row at a time and ignores `Flags::WRAPLINE`, so a word broken across a wrap is invisible to it while `⌘F`, which walks the grid as a stream, finds it. Not a spec violation and not worth joining rows for — stitching them raises the question of which row a hit's byte offset belongs to — but it is the second surprise a user meets, so it goes next to the alternate-screen hole in the README. - `全体の上限で止まる` promised an early exit that does not exist: `scan` collects each session's allowance and then truncates. The per-session limit is what bounds the work. Renamed to say truncate. - `probe.rs` lowercased with `str::to_lowercase`, which is the exact mismatch `scan::needle` exists to prevent (final sigma). It calls `needle` now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: cover the un-flagged history shrink, and fix two stale names Three small ones from the re-review: - `初期化で履歴が縮んでも数えは残る` covers `finish()`'s `after < before` arm, which until now no test touched: the ED 3 test reaches it but also sets `dropped`, so it passes with the arm deleted. A read that pushes three lines and then resets the terminal drops the history without setting the flag, so the shrink is the only signal — 9 with the arm, 6 without. That arm is all that stands between an un-flagged shrink and a pushed total losing thousands of lines, which is item 2's failure mode arriving by another door. - `間の空いた出力は前の出力の時刻にならない` still said the tick goes in at the pushed total as read, which `mark_read`'s own doc now contradicts. The test is right — it drives `push` directly — so the sentence now says so and points at the read-level test for the rule. - `旗を立てなければ_smart_case_のままである` carried Latin identifiers into a test name; this project writes them as whole Japanese sentences. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: measure what the line counting costs on the read path `Counting` sits between the parser and the `Term`, so every session's reads go through it whether anyone ever searches or not, and the numbers in 7.13 predate it. The first review ran `--throughput` three times and saw `scrolling` at 202.6 / 196.5 / 187.7ms against a recorded 179ms, but `light_cells` swung 114-136ms over the same three runs, so nothing could be concluded. Measured as an A/B instead: `main` in a second worktree, both release builds, the two binaries run alternately rather than one after the other, 11 rounds twice over, medians and a per-round paired comparison. The cost is per line feed and nothing per cell, which explains the shape of the table. `scrolling` is 16MiB of bare newlines — 16.8M feeds — and is slower in 22 of 22 pairs by 21.5ms, three times the paired spread: about 1.3ns a feed, four cycles, which matches what `at_bottom()` does. That is the only load the per-feed figure comes from. `scrolling_region` feeds the same count from the bottom row, so its per-feed work should be identical, yet its delta is 7ms smaller — that gap sits inside the paired spread and is recorded as unresolved rather than explained away. Loads that write cells show no difference; `light_cells` and `medium_cells` lean slower but their spread swallows the magnitude, and that is said plainly rather than turned into a number. 7.13's table now points at 7.16 instead of reading as current. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
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.
The implementation plan for the spec merged in #40. Nothing is implemented here — this is the task-by-task plan, each task ending in a test that proves it.
docs/superpowers/plans/2026-10-28-cross-session-search.md⌘⇧Fstops falling through to⌘F(the physical-key branch ignores Shift today)src/lines.rs— counts the lines pushed into history, so a row can be datedsrc/scan.rs— case-insensitive plain match, and a third fit mode that keeps the hit centered⌘F--bench, record the numbers and the alt-screen holeTwo places where the plan refines the spec rather than restating it:
linefeedin the primary screen. A linefeed above the bottom line pushes nothing into history, so that overcounts. The plan takes thehistory_size()delta — exact below the scrollback cap — and only estimates when the delta is zero because the cap is reached.⌘F's own search when it does not.One thing the spec assumed that the code does not have:
Redactor::redactis behind#[cfg(test)]. The result rows need it, so Task 4 takes the gate off.🤖 Generated with Claude Code