fix(webui): shared sort header, coherent group sorting, real expand-state tests

webui/src/components/DataTable.vue

@@ -1,9 +1,9 @@
 <script setup lang="ts" generic="TData">
 import type { ColumnDef, SortingState } from '@tanstack/vue-table'
-import { faSort, faSortDown, faSortUp } from '@fortawesome/free-solid-svg-icons'
 import { FlexRender, getCoreRowModel, getSortedRowModel, useVueTable } from '@tanstack/vue-table'
 import { ref } from 'vue'
 
+import SortHeaderButton from '@/components/SortHeaderButton.vue'
 import {
   Table,
   TableBody,
@@ -78,20 +78,7 @@ function ariaSort(dir: false | 'asc' | 'desc'): 'ascending' | 'descending' | und
           :class="header.column.columnDef.meta?.align === 'right' ? 'text-right' : undefined"
           :aria-sort="header.column.getCanSort() ? (ariaSort(header.column.getIsSorted()) ?? 'none') : undefined"
         >
-          <button
-            v-if="header.column.getCanSort()"
-            type="button"
-            class="-mx-1.5 inline-flex items-center gap-1 rounded px-1.5 py-0.5 hover:text-foreground focus-visible:ring-3 focus-visible:ring-ring/50 focus-visible:outline-1 focus-visible:outline-ring"
-            @click="header.column.toggleSorting(header.column.getIsSorted() === 'asc')"
-          >
-            <FlexRender v-if="!header.isPlaceholder" :render="header.column.columnDef.header" :props="header.getContext()" />
-            <FontAwesomeIcon
-              :icon="header.column.getIsSorted() === 'asc' ? faSortUp : header.column.getIsSorted() === 'desc' ? faSortDown : faSort"
-              class="size-3 shrink-0"
-              :class="header.column.getIsSorted() ? 'text-foreground' : 'text-muted-foreground/60'"
-              aria-hidden="true"
-            />
-          </button>
+          <SortHeaderButton v-if="header.column.getCanSort()" :header="header" class="-mx-1.5" />
           <FlexRender v-else-if="!header.isPlaceholder" :render="header.column.columnDef.header" :props="header.getContext()" />
         </TableHead>
       </TableRow>

webui/src/components/OverviewView.vue

@@ -39,9 +39,10 @@ const overview = computed(() => dashboard.overview)
 //
 // Expand STATE itself is unchanged: lib/provider-expand.ts's plain,
 // Vue-free two-set module (manuallyExpanded/manuallyCollapsed) — the
-// exact search-interaction bug fix a prior review flagged, checkable by
-// its own throwaway assertion script without mounting Vue. Only the
-// template-facing adapter changed: expandedProviderValues below is a
+// exact search-interaction bug fix a prior review flagged, covered by a
+// real vitest spec (lib/provider-expand.spec.ts) that exercises the
+// module directly, without mounting Vue. Only the template-facing
+// adapter changed: expandedProviderValues below is a
 // writable computed translating that Set-based state into the string[]
 // shape Accordion's `type="multiple"` v-model expects, and back —
 // clicking a trigger fires the setter with the new array; diffing it
@@ -262,9 +263,9 @@ const aliasEmptyMessage = computed(() =>
         <Accordion v-else v-model="expandedProviderValues" type="multiple" class="rounded-md border px-3">
           <AccordionItem v-for="p in filteredProviders" :key="p.name" :value="p.name">
             <AccordionTrigger>
-              <div class="flex flex-1 flex-wrap items-center gap-x-3 gap-y-1 pr-2 text-left">
+              <span class="flex flex-1 flex-wrap items-center gap-x-3 gap-y-1 pr-2 text-left">
                 <span class="font-medium">{{ p.name }}</span>
-                <Badge variant="secondary" class="font-normal">{{ p.type }}</Badge>
+                <Badge as="span" variant="secondary" class="font-normal">{{ p.type }}</Badge>
                 <span class="text-xs text-muted-foreground tabular-nums">{{ p.modelCount }} models</span>
                 <FontAwesomeIcon
                   v-if="p.lastErr"
@@ -273,7 +274,7 @@ const aliasEmptyMessage = computed(() =>
                   aria-hidden="true"
                 />
                 <span class="text-xs text-muted-foreground">refreshed {{ formatAgo(p.lastRefresh) || 'never' }}</span>
-              </div>
+              </span>
             </AccordionTrigger>
             <AccordionContent>
               <dl class="mb-3 grid grid-cols-2 gap-x-6 gap-y-1.5 text-sm sm:grid-cols-3">

webui/src/components/SortHeaderButton.vue

@@ -0,0 +1,57 @@
+<script setup lang="ts" generic="TData">
+import type { Header } from '@tanstack/vue-table'
+import { faSort, faSortDown, faSortUp } from '@fortawesome/free-solid-svg-icons'
+import { FlexRender } from '@tanstack/vue-table'
+import { computed } from 'vue'
+
+/**
+ * SortHeaderButton is the one clickable, sortable-column control every
+ * sortable header in the panel uses — DataTable.vue's real `<th>` header
+ * buttons AND UsageView.vue's Groups sort toolbar (which has no `<table>`
+ * to put a `<th>` in — see that view's own doc comment for why).
+ * Extracted so the button markup, sort-icon logic, click handler, and
+ * accessible name exist in exactly one place (vue.md: "if you've written
+ * it twice, you owe an abstraction"). Extra classes on this component
+ * (e.g. DataTable.vue's `-mx-1.5` cell-edge alignment) fall through onto
+ * the root `<button>` via Vue's default attribute inheritance.
+ */
+const props = defineProps<{
+  header: Header<TData, unknown>
+}>()
+
+const direction = computed(() => props.header.column.getIsSorted())
+
+const icon = computed(() => (direction.value === 'asc' ? faSortUp : direction.value === 'desc' ? faSortDown : faSort))
+
+/** headerLabel reads the column's own string header when it has one (every column def in this app uses a plain string) — falling back to the column id so the accessible name is never empty even for a hypothetical future function-header column. */
+const headerLabel = computed(() => {
+  const def = props.header.column.columnDef.header
+  return typeof def === 'string' ? def : props.header.column.id
+})
+
+const ariaLabel = computed(() => {
+  const state = direction.value === 'asc' ? 'ascending' : direction.value === 'desc' ? 'descending' : 'none'
+  return `Sort by ${headerLabel.value}, currently ${state}`
+})
+
+function onClick(): void {
+  props.header.column.toggleSorting(direction.value === 'asc')
+}
+</script>
+
+<template>
+  <button
+    type="button"
+    :aria-label="ariaLabel"
+    class="inline-flex items-center gap-1 rounded px-1.5 py-0.5 hover:text-foreground focus-visible:ring-3 focus-visible:ring-ring/50 focus-visible:outline-1 focus-visible:outline-ring"
+    @click="onClick"
+  >
+    <FlexRender v-if="!header.isPlaceholder" :render="header.column.columnDef.header" :props="header.getContext()" />
+    <FontAwesomeIcon
+      :icon="icon"
+      class="size-3 shrink-0"
+      :class="direction ? 'text-foreground' : 'text-muted-foreground/60'"
+      aria-hidden="true"
+    />
+  </button>
+</template>

webui/src/components/UsageView.vue

@@ -1,9 +1,9 @@
 <script setup lang="ts">
 import type { SortingState } from '@tanstack/vue-table'
-import { faSort, faSortDown, faSortUp } from '@fortawesome/free-solid-svg-icons'
-import { FlexRender, getCoreRowModel, getSortedRowModel, useVueTable } from '@tanstack/vue-table'
+import { getCoreRowModel, getSortedRowModel, useVueTable } from '@tanstack/vue-table'
 import { computed, ref } from 'vue'
 
+import SortHeaderButton from '@/components/SortHeaderButton.vue'
 import { Accordion, AccordionContent, AccordionItem, AccordionTrigger } from '@/components/ui/accordion'
 import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/components/ui/card'
 import { valueUpdater } from '@/components/ui/table'
@@ -59,10 +59,10 @@ function membersOf(group: AdminUsageEntryView): AdminUsageEntryView[] {
 // UsageTable (Users, and each group's own nested member list) drive a
 // second, unrendered useVueTable instance here purely for its
 // getSortedRowModel() — sortedGroupRows below. The small button toolbar
-// under the Groups heading toggles that sort exactly like DataTable.vue's
-// own sortable header buttons (same icons, same toggleSorting call,
-// same aria-sort), just laid out as a toolbar instead of <th>s, because
-// there is no <table> here to put a <th> in.
+// under the Groups heading toggles that sort using the identical
+// SortHeaderButton DataTable.vue's real <th>s use (same icons, same
+// toggleSorting call, same accessible name), just laid out as a toolbar
+// instead of <th>s, because there is no <table> here to put a <th> in.
 const groupColumns = usageColumns('Name', 'Members', memberCountOf)
 const groupSorting = ref<SortingState>([])
 const groupsTable = useVueTable({
@@ -83,10 +83,19 @@ const groupsTable = useVueTable({
 })
 const sortedGroupRows = computed(() => groupsTable.getRowModel().rows)
 
-/** ariaSort mirrors DataTable.vue's own helper — kept local since this toolbar isn't a <table> (aria-sort still applies to the toolbar buttons' own semantics, not a <th>, so it's set as a data attribute for styling/testing rather than the real ARIA property here). */
-function isSorted(id: string): false | 'asc' | 'desc' {
-  return groupsTable.getColumn(id)?.getIsSorted() ?? false
-}
+/**
+ * TOOLBAR_COLUMN_IDS restricts the toolbar to exactly the fields each
+ * trigger row displays (Name, Members, req/day, cost/day) — review fix:
+ * exposing all 11 usageColumns() fields as sortable when only 4 are
+ * visible per row let the accordion's order silently diverge from what
+ * the toolbar visibly claims to control. The full stat set stays
+ * available inside each expanded item's own detail grid, just not as a
+ * sort key here.
+ */
+const TOOLBAR_COLUMN_IDS = new Set(['id', 'secondary', 'reqDay', 'costDay'])
+const toolbarHeaders = computed(() =>
+  groupsTable.getHeaderGroups()[0].headers.filter((header) => TOOLBAR_COLUMN_IDS.has(header.column.id)),
+)
 
 /** Which groups are open — plain array state (no search-driven auto-expand exists for this view, unlike Providers, so the two-set provider-expand.ts module is not needed here). */
 const expandedGroups = ref<string[]>([])
@@ -129,28 +138,13 @@ const expandedGroups = ref<string[]>([])
           v-if="sortedGroupRows.length"
           class="mb-3 flex flex-wrap gap-x-4 gap-y-1 text-xs font-medium text-muted-foreground"
         >
-          <button
-            v-for="header in groupsTable.getHeaderGroups()[0].headers"
-            :key="header.id"
-            type="button"
-            class="inline-flex items-center gap-1 rounded px-1 py-0.5 hover:text-foreground focus-visible:ring-3 focus-visible:ring-ring/50 focus-visible:outline-1 focus-visible:outline-ring"
-            :data-sort="isSorted(header.column.id) || 'none'"
-            @click="header.column.toggleSorting(isSorted(header.column.id) === 'asc')"
-          >
-            <FlexRender :render="header.column.columnDef.header" :props="header.getContext()" />
-            <FontAwesomeIcon
-              :icon="isSorted(header.column.id) === 'asc' ? faSortUp : isSorted(header.column.id) === 'desc' ? faSortDown : faSort"
-              class="size-3 shrink-0"
-              :class="isSorted(header.column.id) ? 'text-foreground' : 'text-muted-foreground/50'"
-              aria-hidden="true"
-            />
-          </button>
+          <SortHeaderButton v-for="header in toolbarHeaders" :key="header.id" :header="header" />
         </div>
 
         <Accordion v-if="sortedGroupRows.length" v-model="expandedGroups" type="multiple" class="rounded-md border px-3">
           <AccordionItem v-for="row in sortedGroupRows" :key="row.original.id" :value="row.original.id">
             <AccordionTrigger>
-              <div class="flex flex-1 flex-wrap items-center gap-x-4 gap-y-1 pr-2 text-left">
+              <span class="flex flex-1 flex-wrap items-center gap-x-4 gap-y-1 pr-2 text-left">
                 <span class="font-medium">{{ row.original.id }}</span>
                 <span class="text-xs text-muted-foreground tabular-nums">{{ memberCountOf(row.original) || '0' }} members</span>
                 <span class="text-xs text-muted-foreground tabular-nums">
@@ -159,7 +153,7 @@ const expandedGroups = ref<string[]>([])
                 <span class="text-xs text-muted-foreground tabular-nums">
                   {{ row.original.storeDown ? '?' : formatCost(row.original.costPerDayMicroUsd) }}/day
                 </span>
-              </div>
+              </span>
             </AccordionTrigger>
             <AccordionContent>
               <dl class="mb-3 grid grid-cols-2 gap-x-6 gap-y-1.5 text-sm sm:grid-cols-4">

webui/src/lib/provider-expand.spec.ts

@@ -0,0 +1,118 @@
+import { describe, expect, it } from 'vitest'
+
+import {
+  type ExpandState,
+  clearExpandOverrides,
+  computeExpandedProviders,
+  toggleProviderExpand,
+} from './provider-expand'
+
+/** newState is a fresh ExpandState — every test starts from an empty, un-searched panel. */
+function newState(): ExpandState {
+  return { manuallyExpanded: new Set(), manuallyCollapsed: new Set() }
+}
+
+describe('computeExpandedProviders', () => {
+  it('returns manuallyExpanded verbatim when there is no active query', () => {
+    const state = newState()
+    state.manuallyExpanded.add('openai')
+    expect(computeExpandedProviders(state, false, ['openai', 'anthropic'])).toEqual(new Set(['openai']))
+  })
+
+  it('auto-expands every matching name while a query is active', () => {
+    const state = newState()
+    expect(computeExpandedProviders(state, true, ['openai', 'anthropic'])).toEqual(new Set(['openai', 'anthropic']))
+  })
+
+  it('unions manuallyExpanded with matches while a query is active', () => {
+    const state = newState()
+    state.manuallyExpanded.add('gemini') // manually opened before the search started, does not match it
+    expect(computeExpandedProviders(state, true, ['openai'])).toEqual(new Set(['gemini', 'openai']))
+  })
+
+  it('suppresses an auto-match the user explicitly collapsed', () => {
+    const state = newState()
+    state.manuallyCollapsed.add('openai')
+    expect(computeExpandedProviders(state, true, ['openai', 'anthropic'])).toEqual(new Set(['anthropic']))
+  })
+})
+
+describe('toggleProviderExpand', () => {
+  it('opening (currentlyExpanded=false) adds to manuallyExpanded and clears any collapse override', () => {
+    const state = newState()
+    state.manuallyCollapsed.add('openai')
+    toggleProviderExpand(state, 'openai', false)
+    expect(state.manuallyExpanded.has('openai')).toBe(true)
+    expect(state.manuallyCollapsed.has('openai')).toBe(false)
+  })
+
+  it('closing (currentlyExpanded=true) removes from manuallyExpanded and adds a collapse override', () => {
+    const state = newState()
+    state.manuallyExpanded.add('openai')
+    toggleProviderExpand(state, 'openai', true)
+    expect(state.manuallyExpanded.has('openai')).toBe(false)
+    expect(state.manuallyCollapsed.has('openai')).toBe(true)
+  })
+})
+
+describe('clearExpandOverrides', () => {
+  it('drops manuallyCollapsed but leaves manuallyExpanded untouched', () => {
+    const state = newState()
+    state.manuallyExpanded.add('openai')
+    state.manuallyCollapsed.add('anthropic')
+    clearExpandOverrides(state)
+    expect(state.manuallyCollapsed.size).toBe(0)
+    expect(state.manuallyExpanded.has('openai')).toBe(true)
+  })
+})
+
+describe('the review-flagged sequence: an auto-matched (never manually opened) provider, clicked closed', () => {
+  it('never enters manuallyExpanded, and stays collapsed after the query clears', () => {
+    const state = newState()
+    const matches = ['openai']
+
+    // 1. Search starts matching "openai" — it shows via auto-expand only,
+    //    manuallyExpanded is still empty.
+    let expanded = computeExpandedProviders(state, true, matches)
+    expect(expanded.has('openai')).toBe(true)
+    expect(state.manuallyExpanded.has('openai')).toBe(false)
+
+    // 2. User clicks it closed while it's only auto-expanded. The RAW bug
+    //    this module exists to fix: reading manuallyExpanded.has('openai')
+    //    directly (false) would have ADDED it to manuallyExpanded instead
+    //    of suppressing the auto-expand. toggleProviderExpand takes the
+    //    EFFECTIVE current state (expanded.has('openai') === true) instead.
+    toggleProviderExpand(state, 'openai', expanded.has('openai'))
+    expect(state.manuallyExpanded.has('openai')).toBe(false)
+    expect(state.manuallyCollapsed.has('openai')).toBe(true)
+
+    // 3. Still searching: the override suppresses the auto-expand.
+    expanded = computeExpandedProviders(state, true, matches)
+    expect(expanded.has('openai')).toBe(false)
+
+    // 4. Query clears: a stale override must never carry into a later,
+    //    unrelated search.
+    clearExpandOverrides(state)
+    expanded = computeExpandedProviders(state, false, [])
+    expect(expanded.has('openai')).toBe(false)
+  })
+})
+
+describe('a provider manually opened before search, still matches, clicked closed during search', () => {
+  it("the user's in-search click wins, and it stays closed after the query clears too", () => {
+    const state = newState()
+    state.manuallyExpanded.add('openai') // opened before any search existed
+    const matches = ['openai']
+
+    let expanded = computeExpandedProviders(state, true, matches)
+    expect(expanded.has('openai')).toBe(true)
+
+    toggleProviderExpand(state, 'openai', expanded.has('openai'))
+    expect(state.manuallyExpanded.has('openai')).toBe(false)
+    expect(state.manuallyCollapsed.has('openai')).toBe(true)
+
+    clearExpandOverrides(state)
+    expanded = computeExpandedProviders(state, false, [])
+    expect(expanded.has('openai')).toBe(false)
+  })
+})

webui/src/lib/provider-expand.ts

@@ -1,8 +1,10 @@
 /**
  * Pure expand/collapse state for OverviewView.vue's provider-model
  * accordion, extracted out of the component so the exact toggle sequence
- * a review flagged (see toggleProviderExpand's own comment) is checkable
- * in isolation, without mounting Vue.
+ * a review flagged (see toggleProviderExpand's own comment) is tested in
+ * isolation, without mounting Vue — see provider-expand.spec.ts (vitest),
+ * which exercises this module directly, including the full flagged
+ * sequence.
  *
  * Two sets track state:
  * - manuallyExpanded: providers the user explicitly opened, independent