Add specs for #9155: search sessions by custom tab and pane names#9639
Add specs for #9155: search sessions by custom tab and pane names#9639landkirk wants to merge 1 commit intowarpdotdev:masterfrom
Conversation
|
I'm starting a first review of this spec-only pull request. I completed the review and posted feedback on this pull request. Comment I'm re-reviewing this spec-only pull request in response to a review request. I completed the review and posted feedback on this pull request. Comment You can view the conversation on Warp. I completed the review and posted feedback on this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This spec PR defines product behavior and an implementation plan for making custom tab and pane names searchable from the command palette and vertical-tabs sidebar.
Concerns
- The sidebar implementation plan targets
matching_tab_indices, but visible sidebar filtering is computed separately inrender_groups; following the plan as written would leave the rendered sidebar behavior unfixed. - The whitespace-normalization plan does not cover tab-name display in the sidebar even though the product invariant requires whitespace-only and trimmed custom names to affect display, not just search indexing.
- Supplemental security pass: no security-specific design concerns found in the spec diff.
Verdict
Found: 1 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
… names The command palette indexes neither custom tab nor custom pane names; the vertical-tabs sidebar drops the custom tab name in Panes mode. Users who rename tabs and panes expect those names to be findable. product.md: 21 testable behavior invariants. tech.md: add custom_tab_name and custom_pane_name to SessionNavigationData and drop the !uses_outer_group_container gate at both sidebar filter sites (matching_tab_indices for keyboard nav, render_groups for the visible list). No code in this PR.
landkirk
force-pushed
the
landkirk/search-by-tab-name
branch
from
8e38cc4 to
9f38452
Compare
|
/oz-review |
There was a problem hiding this comment.
Overview
This spec defines custom tab and pane names as searchable inputs for the command palette and vertical-tabs sidebar, with proposed data model, search, render, and sidebar filtering changes.
Concerns
- The implementation plan does not satisfy the stated arbitrary substring contract for the Tantivy command-palette backend without a query or indexing change.
- The product invariants expand highlighting to prompt matches, but prompt highlight support is not specified in the tech plan.
- The pane-name source API is described incorrectly, and tech doc links to repository files are relative to the spec directory.
Verdict
Found: 0 critical, 3 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
|
||
| 8. **Multi-window.** Custom tab and pane names from every open window participate in search wherever cross-window session search already reaches today, consistent with existing cross-window behavior. | ||
|
|
||
| 9. **No duplicated rows on multi-field match (command palette).** When a query matches a session on more than one field — any combination of custom tab name, custom pane name, prompt, command, hint — the session appears exactly once in the result list. Every matched field is reflected in highlighting (see invariant 13). |
There was a problem hiding this comment.
prompt, to be highlighted, but the current renderer only tracks command/hint highlights and the tech plan only adds custom-name ranges. Either add prompt highlight ranges/rendering/tests or scope this invariant to the fields the UI can actually highlight.
|
|
||
| Two concrete gaps to close: | ||
|
|
||
| - **Command palette:** `SessionNavigationData` carries no custom-name fields, and `searchable_session_string_and_ranges` indexes only prompt + command + hint ([search.rs:127-190](app/src/search/command_palette/navigation/search.rs#L127-L190)). Both names need to enter the document and the searchable string. |
There was a problem hiding this comment.
💡 [SUGGESTION] Links like app/src/... resolve relative to specs/GH9155/ and will 404 in GitHub. Use ../../app/src/... for repository-root paths throughout tech.md.
| Populate in [`PaneGroup::pane_sessions`](app/src/pane_group/mod.rs#L2353-L2361): | ||
|
|
||
| - `custom_tab_name` from `pane_group.custom_title(app)`. | ||
| - `custom_pane_name` from `pane.custom_vertical_tabs_title()` ([pane/mod.rs:735](app/src/pane_group/pane/mod.rs#L735)). |
There was a problem hiding this comment.
TerminalPane does not expose custom_vertical_tabs_title(). Specify reading the pane configuration (pane.pane_configuration().as_ref(app).custom_vertical_tabs_title()) or adding a helper, otherwise the implementation either will not compile or will skip pane names.
|
|
||
| Construction follows the same pattern as `command_indices` (filter matched indices into the named range, subtract `range.start`). Both backends call `SessionHighlightIndices::new` with the same ranges struct, so fuzzy and Tantivy paths get the highlight extension for free. Multi-byte handling continues to flow through `byte_indices_to_char_indices` on the Tantivy path (PRODUCT invariant 4). | ||
|
|
||
| The Tantivy schema at [search.rs:259-266](app/src/search/command_palette/navigation/search.rs#L259) is **unchanged**: custom names live in the same single `session` field. Ranking via per-field boost was considered and explicitly dropped — the simpler concat keeps highlights, multi-byte handling, and tie-break (last-focus-ts) untouched with no schema delta. Both backends rebuild from `all_sessions` per query — Tantivy via `searcher.build_index(documents)` at [search.rs:278-296](app/src/search/command_palette/navigation/search.rs#L278-L296), fuzzy by in-memory iteration — so renames take effect on the next search with no invalidation hook (PRODUCT invariant 6). |
There was a problem hiding this comment.
SimpleFullTextSearcher matches full terms, the last-word prefix, and edit-distance-1 fuzzy terms, so an interior substring such as ploy will not reliably match deploy. Specify the query/indexing change needed for arbitrary substring matches, or narrow the product contract.
There was a problem hiding this comment.
Overview
This spec PR defines the behavior contract and implementation plan for making custom tab and pane names searchable in the command palette and vertical-tabs sidebar. The scope is mostly clear and the security pass did not identify a design-level security concern.
Concerns
- The tech plan does not satisfy the product requirement that custom pane names search in every sidebar mode: Summary mode still only indexes the tab custom title and generated summary fields.
- The whitespace normalization plan is internally inconsistent: the product contract preserves interior whitespace for custom names, while the proposed sidebar path relies on helpers that collapse whitespace.
- The command-palette two-label visual treatment remains underspecified without a default when no Figma is available.
Verdict
Found: 0 critical, 2 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| let title_override = pane_group.custom_title(app); | ||
| ``` | ||
|
|
||
| Both edits are render-safe: the `PaneProps` constructed in these branches are throwaway match-only objects consumed by `pane_matches_query` and discarded; they are never rendered. The render-side gate lives independently at [vertical_tabs.rs:1777](app/src/workspace/view/vertical_tabs.rs#L1777) (`displayed_tab_title_override`, used by `render_tab_group` at lines 1828 / 1881) and is **untouched** — the tab name still renders only as a group header in Panes mode, not on each pane row (PRODUCT invariant 17). The sidebar already includes the tab `custom_title` in Summary mode and the pane `custom_vertical_tabs_title` in FocusedSession + Panes modes; no other sidebar changes are required (PRODUCT invariants 1, 10, 11). Sidebar remains window-scoped — both filter sites iterate `workspace.tabs` for the current window only — so PRODUCT invariant 8 (multi-window) is satisfied via the palette's existing `all_sessions(app)` window walk, not by any sidebar change. |
There was a problem hiding this comment.
custom_title and generated summary data into summary_search_text_fragments; it never indexes PaneConfiguration::custom_vertical_tabs_title, so a pane-name query in Summary mode would still miss. Add a Summary-mode plan/test for pane custom names, or narrow the product invariant.
|
|
||
| Both edits are render-safe: the `PaneProps` constructed in these branches are throwaway match-only objects consumed by `pane_matches_query` and discarded; they are never rendered. The render-side gate lives independently at [vertical_tabs.rs:1777](app/src/workspace/view/vertical_tabs.rs#L1777) (`displayed_tab_title_override`, used by `render_tab_group` at lines 1828 / 1881) and is **untouched** — the tab name still renders only as a group header in Panes mode, not on each pane row (PRODUCT invariant 17). The sidebar already includes the tab `custom_title` in Summary mode and the pane `custom_vertical_tabs_title` in FocusedSession + Panes modes; no other sidebar changes are required (PRODUCT invariants 1, 10, 11). Sidebar remains window-scoped — both filter sites iterate `workspace.tabs` for the current window only — so PRODUCT invariant 8 (multi-window) is satisfied via the palette's existing `all_sessions(app)` window walk, not by any sidebar change. | ||
|
|
||
| Whitespace-only handling is already correct: `search_fragments_contain_query` ([vertical_tabs.rs:2854](app/src/workspace/view/vertical_tabs.rs#L2854)) skips whitespace-only fragments today. Names are normalized at the source via `normalize_custom_name` so display offsets agree with palette match ranges. |
There was a problem hiding this comment.
normalize_custom_name trims only, but the sidebar search helpers normalize fragments with split_whitespace().join(" "), so interior whitespace in custom names is not preserved in the sidebar search index. Specify a custom-name-specific sidebar path that preserves interior whitespace, or change the product contract.
| - When only a custom tab name is set: the tab name is shown as a single leading label. | ||
| - When only a custom pane name is set: the pane name is shown as a single leading label. | ||
| - When both are set: both are shown together with the tab name first followed by the pane name, visually distinguished as two segments so the user can tell tab-level context apart from pane-level context. | ||
| - **Open question:** the exact visual treatment between the two segments (separator character, spacing, weight, color) is a design decision and should match the Figma when one is provided. |
There was a problem hiding this comment.
💡 [SUGGESTION] This visual treatment is still open even though the tech plan asks implementation to render two distinguishable segments. Define the default separator/spacing/weight when no Figma is available, or mark design as a pre-implementation dependency.
|
I'm going to cancel and scope this down to the tab side-bar search and the top-level command search individually. Sorry for any churn. |
1 participant
Description
Spec PR for #9155 — make user-set custom tab and pane names searchable on both session-search surfaces (command palette + vertical-tabs sidebar).
Today's two surfaces handle custom names inconsistently: the command palette indexes neither name, and the sidebar drops the custom tab name in Panes mode even though it renders on screen. Users who rename tabs and panes expect those names to be the most reliable way to find a session.
This PR adds the behavior contract and implementation plan; no code yet.
specs/GH9155/product.md— 21 numbered, testable behavior invariants covering both surfaces, both name kinds, edge cases (whitespace-only, multi-byte, restored sessions, multi-window), and result-row rendering.specs/GH9155/tech.md— implementation plan: extendSessionNavigationDatawithcustom_tab_name/custom_pane_name(normalized via a sharednormalize_custom_name), prepend them in the palette's searchable string with per-field highlight ranges, and drop the!uses_outer_group_containergate atmatching_tab_indicesto close the sidebar's Panes-mode gap.Linked Issue
Refs #9155.
ready-to-specorready-to-implement.Testing
No code in this PR.
tech.md's "Testing and validation" section maps each of the 21 product invariants to a concrete unit / integration / snapshot test that the implementation PR will land.Agent Mode