Repository navigation
feat(tabs): search sessions by name on the vertical rail - #580
Conversation
A "Search sessions" box at the top of the vertical tab rail narrows the list to the tabs whose name matches (case-insensitive substring; a web tab matches by its title). It searches every group, collapsed ones included: while a search runs the projection draws every group open and the header will not toggle, and the stored per-device collapse state is left alone. Groups with no match hide, an empty result says "No sessions match", and the flat rail (no groups) filters the same way. Escape or the clear button empties it; leaving the vertical orientation resets it. It is a view filter only: rows get the same tab-filtered-out class the sidebar filter box uses, through one shared _applyTabListFilter() over the pure CodemanTabSearch matcher in constants.js. Grouping, order, Alt+N badges and drag are untouched, nothing is persisted or sent to the server. The sidebar keeps matching name plus working directory. In the grouped tree, hidden rows and the headers of emptied groups leave the roving walk and posinset/setsize, and the tab stop moves onto a visible item. zh-CN strings added.
|
Opened as a draft only because GitHub won't let my account open a regular PR on this repo; it's ready for review. This is the first of three ports to the vertical rail (search here, then a Focus section, then a per-case default group, which builds on #528). It's independent of #528 and merges onto current master on its own. |
|
Crazy that you came with that up just right now, I was thinking about the same just a few minutes ago ;-) |
|
@aakhter sorry about that, and thanks for sticking with drafts the whole time. My best guess is GitHub's open pull request limit for contributors without write access: drafts don't count toward it, which matches what you've been seeing since the rail stack had several PRs open at once. I've added you to the repo's bypass list, so the limit no longer applies to you (nothing else about your permissions changes). Can you try marking this one ready for review? If GitHub still blocks it, tell me what it says and I'll keep digging. |
Ark0N
left a comment
Ark0N
left a comment
There was a problem hiding this comment.
Went through the whole thing. Really nice work, and reusing the sidebar filter instead of building a second one was the right call. The projection trick (collapsed groups drawn open while searching, the stored collapse never touched) is clean, and the structure-stale check means typing doesn't rebuild the rail on every key.
What I ran locally on the PR head: the rail/tab/sidebar unit files incl. yours (8 files, 222 passed), the browser files for tab-rail-search, session-sidebar-ux, tab-layout-editing and tab-rail-resize (21 passed), plus typecheck, lint, format:check, check:frontend-syntax, check:public-assets and check:browser-excludes, all clean. CI is green too.
Two things to fix, both small:
1. Escape in the search box also closes the side panels. The global key handler in setupEventListeners() sits on document in the capture phase, so it runs before the input's own onkeydown. By the time handleTabRailSearchKeydown calls stopPropagation(), the global Escape branch has already run closeAllPanels() (which collapses the monitor and subagents panels if they're open), closeHelp() and closeSessionManager(). I checked it in Chromium by calling the real setupEventListeners() inside your browser harness: one Escape in the box cleared the search and fired all three. So the comment in that handler describes the opposite of what happens. The fix is to claim it inside the global Escape branch, the same way the group menu and the grouped-rail drag already claim theirs:
if (e.target?.id === 'tabRailSearch' && this._tabRailSearch) {
e.preventDefault();
this.clearTabRailSearch();
return;
}Your browser test can't see it because the harness never installs that handler. A test that does, and asserts an open panel stays open, would pin it.
2. In the sidebar, an emptied case box now stays on screen with a "0" count. _applyTabListFilter() puts tab-filtered-out on the box, but the only rule that hides .tab-cluster.tab-filtered-out is scoped to .tab-rail, and the sidebar rule only covers .session-tab.tab-filtered-out. Checked in Chromium with the sidebar layout plus the Case tab layout: filtering down to one session left the other case box painted with its label and a count of "0" (it said "2" before this PR). Adding the cluster to the sidebar rule fixes it and makes the PR description true:
html[data-session-list="sidebar"] .session-tab.tab-filtered-out,
html[data-session-list="sidebar"] .tab-cluster.tab-filtered-out {
display: none !important;
}That also hides the empty web-tab box, which already lingered there before this PR.
On your two questions:
- Drag while searching: I'd turn it off. A drop is saved for every device (session order or the tab layout), and where it lands relative to the rows the search is hiding is something you only see after clearing it. Search is for finding a tab, so the one-line guard plus a test is the predictable option.
- The sidebar picking up hidden boxes and match counts: fine by me, once the CSS from point 2 is in.
One thing I'd like your view on, not blocking: a row that needs you (red permission prompt) disappears from the rail when it doesn't match the search, and since the search draws every group open, no collapsed header carries its alert either. The collapse was built so a prompt behind it is never invisible. Keeping alerted rows visible during a search would be one way to keep that promise. Could also be overkill since the search is something you just typed, so tell me what you think. Same idea, more loosely, for the active tab: a session you start with Run while a search is up lands hidden if its name doesn't match.
Tiny optional nit: on the grouped rail _applyTabTreePositions() now runs twice per render (once in the render path, once at the filter tail), and the second pass only matters when a search is active or the filter just changed something. Cheap either way, so only if you're in there anyway.
With 1 and 2 fixed this looks good to go from my side.
|
Decided the alert question: rows with an alert stay visible during a search, even when their name doesn't match. Same idea as the collapsed group header: a prompt waiting on you should never be hidden by a view filter. How I'd do it:
No new wiring needed for when the alert comes and goes: Tests I'd want: a non-matching row with an alert stays painted during a search and keeps its group visible; it hides again after the alert clears and the rail re-renders; and the pure core keeps a flagged row while still reporting zero matches. |
A session with a tab alert (red action or yellow idle, whatever tabAlerts holds, the same set a collapsed group header surfaces) now stays visible while the rail search or the sidebar filter is narrowing the list, even when its name does not match. A prompt waiting on you should never be hidden by a view filter. The pure CodemanTabSearch.filter decides it: a row passed with keep: true is never hidden. It counts toward its group, so the group stays on screen and the header number is the rows left showing, but not toward matchCount, so "No sessions match" still shows above a lone alerted row. _applyTabListFilter() flags session rows from tabAlerts; web tabs carry no alerts and are never kept. No new wiring: updateTabAlertFromHooks() and _onSessionWorking() already call renderSessionTabs(), and both render paths end in the shared filter.
|
Thanks, done in d3fc07e. Going down your list:
Tests (written first, all five failed before the change): the pure core keeps a flagged row with zero matches and counts it in its section; a non-matching red row stays painted with its group visible during a rail search; a yellow idle row stays visible under a no-match search with the "No sessions match" note showing; the row hides again once the alert clears and the rail re-renders (driven through Also updated the CLAUDE.md sentence and the wiki line about what the search hides. |
The global key handler in setupEventListeners() sits on document in the capture phase, so it ran before the box's inline onkeydown: the Escape that cleared the search also ran closeAllPanels() (collapsing the Monitor and Subagents panels), closeHelp() and closeSessionManager(). Calling stopPropagation() from the inline handler came too late. The global Escape branch now claims an Escape whose target is #tabRailSearch while the box holds text, the same way it already claims one for the group menu, the grouped-rail drag and the Tiles count menu, and routes it to handleTabRailSearchKeydown(). An empty box still leaves Escape to the global handler, and an Escape that cancels an IME composition is left to the IME. Tests: the unit test installs the real global listener and asserts that Escape with text in the box closes nothing, that an empty box closes as before, and that an Escape outside the box is not claimed. The browser test installs setupEventListeners() for real in Chromium, so the capture-before-inline ordering is the shipped one. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A keystroke in the rail search box that does not change the group structure only toggles classes, so hidden rows collapsed and the rows below them moved up while the lineage lines and the subagent/ultracode connectors kept pointing at the old positions until the next render. The sidebar filter box had the same gap for its connectors. _applyTabListFilter() now notes whether anything it touched actually appeared or disappeared (a row, a group or case box, the state headings via tabs-filtering, the "No sessions match" line) and calls updateConnectionLines() only then. That covers both boxes and the alert re-render, costs nothing on the re-apply every render tail runs when nothing changed (the incremental path's lineage gate keeps its meaning), and coalesces with a render's own redraw. Same flag, owner's optional nit from the review: the grouped tree's posinset/roving-stop fix-up at the filter tail now runs only while a search hides something or right after one changed what shows, since both render paths already set them over an unfiltered tree. Tests: unit tests for the rail keystroke path (flat and grouped rail, no render taken), the empty note, the sidebar box, and no redraw when nothing moved; the browser test checks in Chromium that a class-only keystroke moves a row up and redraws exactly once. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Since the sidebar filter and the rail search share _applyTabListFilter(), the sidebar's filter box also marks a .tab-cluster it emptied in the by-case tab layout and rewrites its count. But the only rule hiding .tab-cluster.tab-filtered-out was the rail-scoped one, so in the sidebar the emptied box stayed painted with its label and a count of 0 while its rows were hidden. The sidebar rule now hides the marked case box as well. It stays scoped to html[data-session-list="sidebar"], so a leaked class still cannot hide anything on the header strip. An alerted row still counts toward its box (the owner's call on #580), so a box holding a row that needs the user never hides. Tests: a unit test renders the sidebar in the by-case layout and checks the emptied box is marked with a 0 count, that an alerted row keeps its box on screen and counted, and that clearing restores the totals; a stylesheet check pins the scoped selector; the browser test measures the shipped styles.css in Chromium: the marked box is not painted in the sidebar layout, an unmarked one is, and the header layout hides nothing. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
CodemanTabSearch lower-cased the query and each row with toLocaleLowerCase(), i.e. the browser's locale. Under a Turkish or Azeri locale "API Review" lowers to "apı review", so a search for "api" missed it, and the sidebar filter box, which used locale-independent toLowerCase() before #580, changed matching with it. Both now use toLowerCase(), like every other frontend search. Test: a vm context of its own whose toLocaleLowerCase behaves like the Turkish locale (the prototype is that context's alone) checks that "API" still normalizes to "api" and that "API Review" matches. The test file's overview also lists the pins the #580 fixes added. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
From the owner's review of #580 rather than the bot's report: "Drag while searching: I'd turn it off. A drop is saved for every device (session order or the tab layout), and where it lands relative to the rows the search is hiding is something you only see after clearing it." The PR head left drag on. Both rail drags now refuse while the search narrows the list: the grouped rail's pointer drag in _onTabLayoutPointerDown(), and the flat manual rail's HTML5 drag in its dragstart listener. The flat rail is refused in the listener, not by flipping `draggable`, because a keystroke in the box does not re-render the rows, so a cleared search drags again with the same rows. The sidebar filter box, the header strip and the keyboard moves (Ctrl+Shift+{ }, the row menu) are unchanged. Tests: a grouped-rail press during a search starts no drag and one after clearing does; a flat-rail dragstart during a search is refused and one after clearing goes through. CLAUDE.md and the Dashboard wiki row say so. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
CLAUDE.md gained a rule for the rail's Search sessions box and linked it to architecture-invariants#session-list-layout-header-strip-vs-left-sidebar, which said nothing about it, and the "Collapse is per-device" bullet under Owner tab layouts had an exception it did not record. - Session list layout: one paragraph on the shared _applyTabListFilter() over the pure CodemanTabSearch (classes only, layout-scoped hide rules, rail matches the name and the sidebar name + folder, locale-independent lower-casing), the alert-row keep (owner decision), the data-total count restore, the tree walk and roving-stop fix-up, the projection opening every group, the connector redraw, the global Escape claim, no drag while searching, and the reset off the rail. - Owner tab layouts: the collapse bullet notes that a search draws every group open and refuses toggles without writing the stored set. - CLAUDE.md: the Escape claim and the connector redraw as one clause on the existing rail search rule. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A floating subagent or ultracode window whose parent session's row the
rail search hid drew its connector from the viewport's top-left corner.
The hidden row is display:none, and getBoundingClientRect() still
answers it with an all-zero DOMRect, which is truthy, so the
`if (!tabRect) continue` paths never skipped it and _tabAnchor() put the
line at (0, 0) ("M 0 0 C 350 0, 350 400, 700 400" in Chromium). The
sidebar filter already had the same defect; #580 brought it to the rail,
and the keystroke redraw from 5f373a9 now ran it on every keystroke that
hides the parent.
A new _paintedSessionTab() in app.js returns the parent's row only while
getClientRects() is non-empty, and every place a floating window
measures its parent tab goes through it:
- the shared `tab:<id>` rect cache, filled by the subagent connectors
and by both ultracode connector layers (all three fill it, so guarding
only the first would let the next one cache the zero rect itself);
- the ultracode window's spawn position, which now cascades;
- the subagent window's fly-from-tab spawn, the same defect, not in the
report, now positioned as a window without a tab;
- the ultracode genie on minimize, which now tears down at once.
Lineage lines need nothing: they measure on a cache miss and computeTree
already drops zero-size rects.
Tests: in the gate, with the page laid out by hand (jsdom has no
layout), a subagent window, an ultracode run window and an ultracode
agent window from a hidden row draw no line and draw it again from the
row once the search is cleared, an ultracode window spawns from a
painted row and cascades from a hidden one, and the genie is skipped for
a hidden row. In the browser suite, real Chromium shows the hidden row's
rect is all zero and the connector is gone, then back after Clear. All
four fail on the previous commit. CLAUDE.md and the invariants'
Connectors bullet record the rule.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…earch (#580) 183efac turned drag off while the rail search narrows the list, as the owner asked, by refusing the flat manual rail's dragstart. That also refused a drag that never reorders anything: with the tile grid open, a found tab dragged onto a tile or an empty cell set no draggedTabId, so _acceptTabDrops() (tile-grid.js) saw nothing and the drop was lost. On the PR head that drop replaced or swapped the tile. The owner's reason covers the reorder only: a reorder drop is saved for every device, relative to rows the search hides, while the tile grid is per-device and lands beside nothing hidden. dragstart is unconditional again. The rows refuse the reorder instead: their dragover returns before preventDefault while _tabRailSearchActive() (the browser shows no-drop and fires no drop there), and their drop returns before touching sessionOrder, for anything above the row that might let a drop through. `draggable` stays on, so a cleared search reorders with the same rows. The grouped rail's pointer drag keeps its refusal in _onTabLayoutPointerDown(): it has no drop target besides the rail. Test: during a search that leaves both rows showing, dragstart is not cancelled and sets draggedTabId, dragover on another row is not accepted, a drop there leaves sessionOrder alone and saves nothing, the real _acceptTabDrops() binder accepts the drag on a tile and hands it the id, and after clearing the same rows reorder. It fails on the previous commit, without the dragover guard, and without the drop guard. CLAUDE.md, the invariants, the Dashboard wiki row and the release changeset now say reordering by drag is off and a tile drop still works. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Owner decision: searching for a tab and dragging it into a group is a useful flow, so the drag-off guard is reverted and the docs and changeset say drag works during a search. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Merged and shipped in 1.41.0, thanks @aakhter, and glad the bypass list let you mark it ready! Rail search reuses the sidebar's filter as one shared path, and the alert rows stay visible during a search as we discussed. On your drag question: drag stays on during a search. Searching for a tab and dragging it into a group is a useful flow. At merge I applied the review's small fixes, each with a test:
|
2 participants
Summary
Adds a "Search sessions" box to the top of the vertical tab rail. Type part of a name and the rail narrows to the tabs that match. It works across every group, including collapsed ones, and on the flat rail when you have no groups. Escape or the × clears it.
It's a view filter and nothing more. Grouping, order, Alt+N badges and drag are untouched, and nothing gets saved or sent to the server.
Why I reused the sidebar filter instead of adding a second one
applySidebarFilter()already does most of this. It hides rows with atab-filtered-outclass, re-applies after both render paths, and the flat keyboard walk and the grouped tree walk already skip filtered rows. So I generalized it rather than writing a parallel filter:_applyTabListFilter().applySidebarFilter(query)sets the sidebar's needle and calls it, so every existing render-tail call and test stub keeps working. The rail box sets its own needle (_tabRailSearch) and calls the same function. Only one of the two hosts owns#sessionTabsat any time, so only one needle is ever live.CodemanTabSearchin constants.js (needle normalization, plus which rows hide and how many matches each section has). That's the part with unit tests.The projection change is one line. A collapsed group's rows aren't in the DOM at all (
project()only renders the selected row), so a class filter alone can't reveal a match inside one. While a search is active,_projectTabGroups()passescollapsedGroupIds: []. The stored per-device collapse set and its localStorage key never change, andtoggleTabGroupCollapsed()refuses while searching, so clearing the search puts the collapse back exactly as it was. Starting or ending a search re-renders only when that changes the structure (the existing_isTabGroupStructureStale()check). Other keystrokes just re-apply classes.Behaviour
applyTabOrientation()resets it when the list leaves the vertical rail, before the render, so a horizontal strip never inherits a filter it has no box for.Tests
test/tab-rail-search.test.ts(24 tests, in the gate): pureCodemanTabSearch(including a kept row with zero matches); grouped rail through the real CodemanApp in JSDOM (collapsed match revealed, collapse storage untouched, toggle refused, counts, badges unchanged, web tab title vs URL/cwd, empty state, tree walk/roving stop/posinset, survives full and incremental re-renders, no fetch/order/layout change); flat rail; horizontal strip ignores it; Escape/clear/focus; reset on orientation change; sidebar keeps its own matching; alerted rows (red and yellow) stay painted with their group, hide again once the alert clears viaupdateTabAlertFromHooks(), and the sidebar box keeps them too; markup + zh-CN via the real i18n.js over index.html.test/tab-rail-search.browser.test.ts(4 tests, added toBROWSER_TEST_GLOBS): real Chromium with the shipped#tabRailmarkup lifted from index.html and the real styles.css. Covers painted rows/groups, clicking a revealed match, the clear button, Escape, the empty state, the arrow-key walk visiting only matches, and the box hidden off the rail.keepin the pure core fails 5 unit tests. All restored.Gates run (all exit 0):
npm run typecheck,npm run lint,npm run format:check,npm run check:frontend-syntax,npm run check:public-assets,npm run check:browser-excludes,npm run build.Targeted tests:
npm test --on 26 rail/tab/sidebar/i18n files, 414 passed (tab-rail-search, tab-layout-rail, tab-layout-editing, session-list-layout, tab-orientation, tab-triage, tab-clusters, tab-harness-logo, session-sidebar-ux, tab-layout-browser, tab-layout-settings-i18n, tile-grid-i18n, i18n-branding and friends).npm run test:browser --on tab-rail-search, tab-layout-editing, tab-rail-resize, tab-activation and session-sidebar-ux: 5 files, 36 passed.Screenshot
Desktop rail (DSF 1) with "api" typed. Planning is collapsed on this device, and its matching row shows anyway with the chevron dimmed. Non-matching rows and the Ungrouped section are hidden, and the Alt+N badges keep their numbers (1 and 3). I couldn't attach the image from the CLI; happy to post it if it helps the review.
Two calls I'd like your view on
_applyTabListFilter(), filtering the sidebar in a clustered layout also hides a case box that ends up empty and updates its count. Its matching itself (name plus folder) is unchanged, and the existing sidebar tests pass.