[gedaiu]

Closes #1249

When two or more knowledge bases share the text before the first /, they're grouped under it (re-cinq/lore, re-cinq/floor → re-cinq, shown as lore and floor). Both sides of the slash must have text (both are trimmed), so /foo, foo/ and / stay plain rows. A prefix only one base has (CI/CD, A/B tests) also stays a plain row with its full name. The rule is the pure kbGroups / kbOptions in web/src/pages/kbGroups.ts.

Top-bar switcher and the chat input's base chip share KbMenuList. A group is one row, re-cinq ›, with the check mark when the current base is inside it. It opens a submenu to the right:

• Mouse: hovering opens it, and clicking opens it (never toggles). Pressing the group row isn't treated as an outside click, so it doesn't flicker.
• Touch: hover doesn't apply, so one tap opens it.
• Keyboard: Enter, Space or → opens it and focuses the first base, ↑/↓ move, Esc or ← closes it and returns focus to the row. The row has aria-haspopup="menu" and aria-expanded, and no aria-current.
• Narrow screens: under 640px there's no room for a side submenu, so the group expands in place, with its bases indented under it.
• Search: typing in the switcher's search lists full names flat, so re-cinq/lo still finds re-cinq/lore.

I kept a Popover rather than DropdownMenuSub. Sub needs a DropdownMenu root, and the switcher is a Popover with an autofocused search input that DropdownMenu's roving focus and typeahead would fight. The submenu is portalled, so the chat chip's outside-click handler ignores [data-kb-submenu].

API token base picker: SearchSelectOption gains optional group and short. When the dropdown isn't filtered, it shows a heading wherever the group changes (withGroupHeadings), with the short names indented. Arrow keys skip the headings. While typing it's flat full names, and chips always show the full name.

My knowledge bases: with the filter empty, each group gets a <th scope="rowgroup"> header row, and its bases show their short name (full name on hover).

Side effects, both intended: the chat chip's rows now use the switcher's padding and font size, and the switcher highlights the current base the way the chat chip already did.

Tests: kbGroups.test.ts (plain names, grouping, order and first-slash split, empty sides and trimming, a lone prefix stays plain, option shape), withGroupHeadings.test.ts, kbMenuKeys.test.ts (the key map). The web build and vitest pass.

I checked it in Chromium with Playwright against a running server: hover, the keyboard path in both hosts, a click on a hovered group, picking from the submenu inside the chat chip, flat search results, and a Pixel 7 touch context where the group expands inline and is on screen. All 26 checks pass.

Size. This adds 527 lines, just over the 500 in CONTRIBUTING. It stays one PR because every picker uses the same grouping (kbGroups, withGroupHeadings) and the same submenu (KbMenuList), and about a quarter of the additions are their tests. Shipping the grouping in some pickers and not others would make the base list look different depending on where it is opened. My knowledge bases is the one part that could move out (32 lines); I'm happy to split it into a follow-up if you prefer.

🤖 Generated with Claude Code

[WaylandYang]

Thank you for writing up #1249 and following it with the PR. Keeping the rule in a pure kbGroups with tests is the right shape. A few things before it can land:

1. DCO. None of the 8 commits has a Signed-off-by line, and CONTRIBUTING requires one on every commit. git rebase --signoff origin/dev && git push -f fixes it.

2. Keyboard access. Before this change, every base in the chat scope menu could be reached with Tab. Now the submenu is portalled to the end of <body>, onOpenAutoFocus is prevented (KbMenuList.tsx:124), and the Popover closes when focus moves outside it. So Enter on a group row opens the submenu, and the next Tab closes it again. The chat chip has no search to fall back on, so a keyboard user can no longer pick re-cinq/lore there. Please:

   • Open the group with Enter, Space or ArrowRight and move focus to its first item.
   • Close it with Escape or ArrowLeft and return focus to the row.
   • Add aria-haspopup="menu" and aria-expanded to the row.
   • Don't pass active={open} to the row (:105), because Row turns it into aria-current="true", which tells a screen reader the group is the current base.

   DropdownMenuSub in components/ui/dropdown-menu.tsx already does all of this. It also handles outside clicks across nested portals, so if it fits both hosts you could drop the [data-kb-submenu] exemption.

3. Hover and click. Hovering opens the submenu and clicking toggles it (:101, :112). On a touch screen a tap sends an emulated mouseenter and then a click, so the submenu opens and closes straight away. With a mouse, clicking a group row you're hovering on closes it on pointerdown and reopens it on click, so it flickers. Opening on hover only when pointerType === "mouse", and having the click set open to true instead of toggling it, avoids both. The description says you haven't clicked through it yet, so please try it in a browser, including a touch viewport.

4. Empty segments. The server accepts any 1–64 character name, so /foo, foo/ and / are all valid. kbGroups turns them into a group with an empty name or an item with an empty label, which gives a blank row in the menus and a blank header in My knowledge bases; in the token picker, /foo shows up as plain foo. Please group only when there is text on both sides of the first slash, trim the owner part so re-cinq /lore and re-cinq/floor don't become two identical-looking groups, and add a test for it.

One question: names that merely contain a slash (CI/CD, A/B tests) become a group with a single base. Is that intended for every user, or should a group need at least two bases?

Smaller points:

• The comment removed from Chat.tsx explained why the check mark has to go in trailing (an svg placed in the children drops to a second line). KbRow still depends on that, so please move the comment there.
• The chat scope rows now use the switcher's padding and font size, and the switcher now highlights the current base. If that's intended, a line in the description is enough.
• The My knowledge bases group header would work better as <th scope="rowgroup" colSpan={6}>, so screen readers know which rows it labels.

The size is fine and there are no new UI strings. Thanks again.

[gedaiu]

Thanks for the thorough review. Everything is in a425a45, rebased onto dev, and all commits are signed off.

1. DCO. Every commit now has Signed-off-by.
2. Keyboard.
   • Enter, Space or → on a group row opens it and focuses its first base; ↑/↓ move inside; Esc or ← closes it and returns focus to the row.
   • In the chat chip, Esc in the submenu no longer closes the whole menu.
   • The row has aria-haspopup="menu" and aria-expanded, and active={open} is gone, so there's no aria-current.
   • The key map is a pure kbMenuKeys.ts with tests.
   • I looked at DropdownMenuSub but kept the Popover: Sub needs a DropdownMenu root, and the top-bar switcher is a Popover with an autofocused search field that DropdownMenu's roving focus and typeahead would fight. So the [data-kb-submenu] exemption stays, with the reason in the header comment of KbMenuList.tsx.
3. Hover and click.
   • Hover opens and closes only when pointerType === "mouse".
   • A click always opens, and pressing the anchor row isn't treated as an outside interaction, so a click on a hovered row doesn't flicker.
   • I clicked through it in Chromium with Playwright, including a Pixel 7 touch context. That surfaced one more problem: on a phone the menu is nearly full width, and the side submenu had no room on either side and ended up off screen. Under 640px the group now expands in place, with its bases indented beneath it.
4. Empty segments. It groups only when both sides of the first slash have text, both trimmed, so re-cinq /lore and re-cinq/floor share one group. Tests cover /foo, foo/, / and the trimming.

On your question: a group now needs at least two bases. CI/CD or A/B tests alone stays a plain row with its full name. This applies to the menus, the token picker and the table, with tests.

Smaller points: the check-mark comment now sits on KbRow; the My knowledge bases header is <th scope="rowgroup" colSpan={6}>; and the description now says the padding and current-base highlight changes are intended.

One unrelated thing I noticed: in the switcher, Esc closes it but keeps the typed query for the next open. I haven't changed that here.

[WaylandYang]

Thank you for the quick and careful update. Everything from the review is addressed. I also appreciate the Playwright pass, and that you caught the narrow-screen problem while doing it. Requiring two bases before a group forms is a good answer to the CI/CD question.

Two small things left:

1. Esc in the switcher. onEscapeKeyDown={(e) => e.preventDefault()} (KbMenuList.tsx:234) cancels Escape even when focus isn't in the submenu. Radix only passes Escape to the topmost layer, which is the submenu. So if you hover a group in the top-bar switcher and press Esc while focus is still in the search box, nothing closes, and Esc used to close the switcher. Cancelling it only when focus is inside the submenu (the case onSubmenuKey already handles) fixes it:

   onEscapeKeyDown={(e) => {
     if (submenu.current?.contains(document.activeElement)) e.preventDefault();
   }}

2. Size. The PR now adds 524 lines, a little over the 500 in CONTRIBUTING. A sentence in the description on why it stays as one PR is enough. Moving the My knowledge bases table to a follow-up would also work.

You're right about Esc keeping the switcher's query: onOpenChange={setOpen} skips dismiss(). That was there before this PR, so a separate small PR would be welcome. Thanks again.

[gedaiu]

Thank you, and sorry this sat for two days.

1. Esc in the switcher: done, with your fix. Escape is cancelled only when focus is inside the submenu. I checked it in Chromium with Playwright: with a group hovered and focus in the search box, Esc now closes the submenu and the next Esc closes the switcher. Before the change it did nothing. From inside the submenu, Esc still closes only the submenu and returns focus to the group row.
2. Size: the description now says why it stays one PR, and offers to move My knowledge bases to a follow-up if you'd rather.

I also squashed the branch into one signed commit; the old history had a long bulleted body and a stray Co-authored-by trailer. It's rebased on dev, and pnpm test (306 passed) and pnpm build are clean.

I'll send the separate fix for Esc keeping the switcher's query once this one is in, to keep one PR open at a time.

[WaylandYang]

Thank you, gedaiu! Esc now cancels only when focus is inside the submenu, so from the search box it closes as before, and inside the submenu it steps back to the group row. The size note in the description is clear, and keeping all the pickers on the same grouping in one PR makes sense, so no need to split out My knowledge bases. Merging.