fix(nav): the sidebar section label was not a control

__tests__/unit/hooks/useNavigationStorage.test.ts

@@ -82,9 +82,20 @@ describe('buildInitialCollapsedSections', () => {
     expect(collapsed.has('settings')).toBe(false);
   });
 
-  it('on mobile collapses collapsible sections with priority > 3', () => {
+  it('gives the same defaults on mobile as on desktop — config is the only input', () => {
+    // Viewport used to override the config with `priority > 3`, so the same
+    // account saw different sections open on a phone than on a laptop and the
+    // declared `defaultExpanded` was a lie on one of them.
+    Object.defineProperty(window, 'innerWidth', { value: 1280, writable: true });
+    const onDesktop = buildInitialCollapsedSections(basicSections);
     Object.defineProperty(window, 'innerWidth', { value: 375, writable: true });
-    const mobileSections: NavSection[] = [
+    const onMobile = buildInitialCollapsedSections(basicSections);
+    expect([...onMobile].sort()).toEqual([...onDesktop].sort());
+  });
+
+  it('honours defaultExpanded regardless of priority', () => {
+    Object.defineProperty(window, 'innerWidth', { value: 375, writable: true });
+    const sections: NavSection[] = [
       { id: 'low', title: 'Low', items: [], collapsible: true, defaultExpanded: true, priority: 2 },
       {
         id: 'high',
@@ -95,9 +106,7 @@ describe('buildInitialCollapsedSections', () => {
         priority: 4,
       },
     ];
-    const collapsed = buildInitialCollapsedSections(mobileSections);
-    expect(collapsed.has('low')).toBe(false);
-    expect(collapsed.has('high')).toBe(true);
+    expect(buildInitialCollapsedSections(sections).size).toBe(0);
   });
 
   it('returns empty set when no collapsible sections', () => {
@@ -178,6 +187,26 @@ describe('useNavigationStorage', () => {
     expect(onStateLoaded).toHaveBeenCalledTimes(1);
   });
 
+  it('keeps an empty saved set — expanding every section survives a reload', () => {
+    // "[]" means the user opened everything. Reading it as "nothing saved" and
+    // falling back to the defaults made that click silently revert on reload.
+    window.localStorage.setItem(STORAGE_KEYS.COLLAPSED_SECTIONS, '[]');
+
+    renderHook(() => useNavigationStorage(true, basicSections, { onStateLoaded, onLoadFailed }));
+
+    expect(onStateLoaded).toHaveBeenCalledWith(
+      expect.objectContaining({ collapsedSections: new Set<string>() })
+    );
+  });
+
+  it('falls back to config defaults only when nothing was ever saved', () => {
+    renderHook(() => useNavigationStorage(true, basicSections, { onStateLoaded, onLoadFailed }));
+
+    expect(onStateLoaded).toHaveBeenCalledWith(
+      expect.objectContaining({ collapsedSections: new Set(['discover']) })
+    );
+  });
+
   it('loads saved state from localStorage', () => {
     window.localStorage.setItem(STORAGE_KEYS.SIDEBAR_OPEN, 'true');
     window.localStorage.setItem(STORAGE_KEYS.SIDEBAR_COLLAPSED, 'true');

package.json

@@ -34,6 +34,7 @@
     "check:sizes": "node scripts/check-file-sizes.js",
     "check:duplication": "node scripts/check-duplication.mjs",
     "check:dead-fields": "node scripts/check-dead-fields.mjs",
+    "check:dead-labels": "node scripts/check-dead-labels.mjs",
     "check:schema-columns": "node scripts/check-schema-columns.mjs",
     "check:currency-units": "node scripts/check-currency-units.mjs",
     "check:rpc-exists": "node scripts/check-rpc-exists.mjs",
@@ -43,7 +44,7 @@
     "check:user-scoped-deletes": "node scripts/check-user-scoped-deletes.mjs",
     "check:ai-models": "node scripts/check-ai-models.mjs",
     "check:mdx": "node scripts/check-mdx.mjs",
-    "verify": "npm run ci:docs && npm run check:accent-ink && npm run type-check && npm run type-check:scripts && npm run check:sizes && npm run audit:routes && npm run lint && npm run check:duplication && npm run check:dead-fields && npm run check:migration-versions && npm run check:schema-columns && npm run check:currency-units && npm run check:rpc-exists && npm run check:one-current-user && npm run check:app-locale && npm run check:client-ip && npm run check:user-scoped-deletes && npm run check:mdx && npm run test:unit -- --watchAll=false",
+    "verify": "npm run ci:docs && npm run check:accent-ink && npm run type-check && npm run type-check:scripts && npm run check:sizes && npm run audit:routes && npm run lint && npm run check:duplication && npm run check:dead-fields && npm run check:dead-labels && npm run check:migration-versions && npm run check:schema-columns && npm run check:currency-units && npm run check:rpc-exists && npm run check:one-current-user && npm run check:app-locale && npm run check:client-ip && npm run check:user-scoped-deletes && npm run check:mdx && npm run test:unit -- --watchAll=false",
     "audit:schema": "node scripts/db/audit-schema-drift.mjs",
     "audit:routes": "node scripts/audit-routes.mjs",
     "gen:types": "bash scripts/db/gen-types.sh",

scripts/check-dead-labels.mjs

@@ -0,0 +1,80 @@
+#!/usr/bin/env node
+/**
+ * A heading that sits BESIDE a chevron-only toggle is a dead click.
+ *
+ * The shape:
+ *
+ *   <div className="flex ... justify-between">
+ *     <h3>Fund</h3>                  <- looks like the control, isn't
+ *     <button onClick={toggle}>      <- the only real target, ~44px wide
+ *       <ChevronDown />
+ *     </button>
+ *   </div>
+ *
+ * The visitor aims at the word, nothing happens, and the arrow beside it is a
+ * small target in a full-width row of apparent affordance. This landed twice
+ * in the same sidebar — once on the ContextSwitcher avatar ("should be
+ * clickable", reported by two different visitors), once on the Fund /
+ * Coordinate / Finance section headers — so the class is closed by a gate
+ * instead of a third fix.
+ *
+ * The correct shape puts the label INSIDE the control, which is what
+ * HeaderNavigation already does:
+ *
+ *   <button onClick={toggle} aria-expanded={open}>
+ *     {item.name}
+ *     <ChevronDown aria-hidden="true" />
+ *   </button>
+ *
+ * Heuristic, so it is deliberately narrow: a heading, then within 12 lines a
+ * <button> containing a Chevron, and no heading inside that button. Anything
+ * that trips it is either the bug or a row that should be restructured anyway.
+ */
+import { readFileSync, readdirSync } from 'node:fs';
+import { join, relative, extname } from 'node:path';
+
+const ROOT = process.cwd();
+const LOOKAHEAD = 12;
+const HEADING = /<h[1-6]\b/;
+const BUTTON = /<button\b/;
+const CHEVRON = /<Chevron(Down|Up|Right|Left)\b/;
+
+function walk(dir, files = []) {
+  for (const entry of readdirSync(dir, { withFileTypes: true })) {
+    if (entry.name === 'node_modules' || entry.name === '.next') continue;
+    const full = join(dir, entry.name);
+    if (entry.isDirectory()) walk(full, files);
+    else if (extname(entry.name) === '.tsx') files.push(full);
+  }
+  return files;
+}
+
+const violations = [];
+for (const file of walk(join(ROOT, 'src', 'components'))) {
+  const lines = readFileSync(file, 'utf8').split('\n');
+  lines.forEach((line, i) => {
+    if (!HEADING.test(line)) return;
+
+    const text = lines.slice(i, i + LOOKAHEAD).join('\n');
+    const button = BUTTON.exec(text);
+    if (!button || !CHEVRON.test(text)) return;
+
+    // A heading AFTER the <button> means the label is inside the control —
+    // the correct shape, not a violation.
+    const headingInsideButton = [...text.matchAll(/<h[1-6]\b/g)].some(m => m.index > button.index);
+    if (headingInsideButton) return;
+
+    violations.push(`${relative(ROOT, file)}:${i + 1}\n  ${line.trim()}`);
+  });
+}
+
+if (violations.length > 0) {
+  console.error(
+    `check:dead-labels — ${violations.length} heading(s) sitting beside a chevron-only toggle.\n` +
+      'Put the label inside the button so the whole row is the target:\n'
+  );
+  violations.forEach(v => console.error(`  ${v}\n`));
+  process.exit(1);
+}
+
+console.log('check:dead-labels — no headings stranded beside a chevron-only toggle.');

src/components/sidebar/SidebarNavigation.tsx

@@ -54,7 +54,7 @@ export function SidebarNavigation({
       >
         {visibleSections.map(section => {
           const isCollapsed = collapsedSections.has(section.id);
-          const hasActiveItem = section.items.some(item => item.href && isItemActive(item.href));
+          const sectionItemsId = `sidebar-section-${section.id}`;
 
           return (
             <div key={section.id} className="space-y-1">
@@ -63,31 +63,54 @@ export function SidebarNavigation({
                 <div className="mx-2 my-2 border-t border-default" />
               )}
 
-              {/* Section Header - Hidden on desktop (icons only), visible on mobile (expanded) */}
-              {isExpanded && (
-                <div className="flex items-center justify-between px-3 mb-1">
-                  <h3 className="text-xs font-semibold uppercase text-fg-secondary">
-                    {section.title}
-                  </h3>
-                  {section.collapsible && (
-                    <button
-                      onClick={() => toggleSection(section.id)}
-                      className="flex min-h-11 min-w-11 items-center justify-center rounded-md p-2 transition-colors hover:bg-surface-raised active:bg-surface-raised touch-manipulation"
-                      aria-label={`${navigationLabels.SECTION_TOGGLE} ${section.title}`}
-                    >
-                      {isCollapsed ? (
-                        <ChevronRight className="w-4 h-4 text-fg-tertiary" />
-                      ) : (
-                        <ChevronDown className="w-4 h-4 text-fg-tertiary" />
-                      )}
-                    </button>
-                  )}
-                </div>
-              )}
+              {/* Section Header - Hidden on desktop (icons only), visible on mobile (expanded).
+                  The WHOLE row toggles, not just the chevron: a label that looks
+                  like a control but isn't reads as a dead click, and the arrow
+                  alone is a 44px target inside a full-width row of affordance. */}
+              {isExpanded &&
+                (section.collapsible ? (
+                  <button
+                    type="button"
+                    onClick={() => toggleSection(section.id)}
+                    aria-expanded={!isCollapsed}
+                    // Only reference the panel while it exists — aria-controls
+                    // pointing at an unrendered id is an invalid-value violation.
+                    aria-controls={isCollapsed ? undefined : sectionItemsId}
+                    className="flex w-full min-h-11 items-center justify-between gap-2 rounded-md px-3 mb-1 text-left transition-colors hover:bg-surface-raised active:bg-surface-raised touch-manipulation"
+                    aria-label={`${navigationLabels.SECTION_TOGGLE} ${section.title}`}
+                  >
+                    <h3 className="text-xs font-semibold uppercase text-fg-secondary">
+                      {section.title}
+                    </h3>
+                    {isCollapsed ? (
+                      <ChevronRight
+                        className="w-4 h-4 shrink-0 text-fg-tertiary"
+                        aria-hidden="true"
+                      />
+                    ) : (
+                      <ChevronDown
+                        className="w-4 h-4 shrink-0 text-fg-tertiary"
+                        aria-hidden="true"
+                      />
+                    )}
+                  </button>
+                ) : (
+                  section.title && (
+                    <div className="flex items-center px-3 mb-1">
+                      <h3 className="text-xs font-semibold uppercase text-fg-secondary">
+                        {section.title}
+                      </h3>
+                    </div>
+                  )
+                ))}
 
-              {/* Section Items - always show on desktop, respect collapse on mobile */}
-              {(!isExpanded || !section.collapsible || !isCollapsed || hasActiveItem) && (
-                <div className={`space-y-1 ${isExpanded ? 'px-2' : ''}`}>
+              {/* Section Items - always show on desktop, respect collapse on mobile.
+                  An explicit toggle always wins: keeping a section open because it
+                  happens to hold the active page made the chevron lie and the
+                  click look dead. Landing on a page inside a collapsed section is
+                  handled at load time, not by overriding the user here. */}
+              {(!isExpanded || !section.collapsible || !isCollapsed) && (
+                <div id={sectionItemsId} className={`space-y-1 ${isExpanded ? 'px-2' : ''}`}>
                   {section.items.map(item => (
                     <SidebarNavItem
                       key={item.name}

src/hooks/useNavigationStorage.ts

@@ -10,14 +10,22 @@ const STORAGE_KEYS = {
   COLLAPSED_SECTIONS: 'orangecat_collapsed_sections',
 } as const;
 
+/**
+ * `defaultExpanded` in the nav config is the ONLY thing that decides which
+ * sections open on a first visit.
+ *
+ * This used to branch on viewport — `isMobile ? section.priority > 3 : ...` —
+ * so on a phone the declared config was ignored in favour of a bare magic
+ * number, and the same account saw different sections open on a phone than on
+ * a laptop with nothing recording why. A config field that a second rule can
+ * silently override is not a source of truth. One field, one behaviour: to
+ * change what opens by default, edit `defaultExpanded` in config/navigation.
+ */
 export function buildInitialCollapsedSections(sections: NavSection[]): Set<string> {
-  const isMobile = typeof window !== 'undefined' && window.innerWidth < 1024;
   const collapsed = new Set<string>();
   sections.forEach(section => {
-    if (section.collapsible) {
-      if (isMobile ? section.priority > 3 : !section.defaultExpanded) {
-        collapsed.add(section.id);
-      }
+    if (section.collapsible && !section.defaultExpanded) {
+      collapsed.add(section.id);
     }
   });
   return collapsed;
@@ -49,14 +57,14 @@ export function useNavigationStorage(
       const savedCollapsedState = localStorage.getItem(STORAGE_KEYS.SIDEBAR_COLLAPSED);
       const isSidebarCollapsed = savedCollapsedState ? JSON.parse(savedCollapsedState) : false;
 
+      // An empty saved set means "the user expanded everything" — it is NOT the
+      // same as "nothing was ever saved". Keying off size made expand-all
+      // silently revert to the defaults on the next load.
       const savedCollapsedSections = localStorage.getItem(STORAGE_KEYS.COLLAPSED_SECTIONS);
-      const collapsedFromStorage = savedCollapsedSections
-        ? new Set<string>(JSON.parse(savedCollapsedSections))
-        : new Set<string>();
-
-      const defaultCollapsed = buildInitialCollapsedSections(sections);
       const collapsedSections =
-        collapsedFromStorage.size > 0 ? collapsedFromStorage : defaultCollapsed;
+        savedCollapsedSections === null
+          ? buildInitialCollapsedSections(sections)
+          : new Set<string>(JSON.parse(savedCollapsedSections));
 
       onStateLoaded({ isSidebarOpen, isSidebarCollapsed, collapsedSections });
     } catch (error) {

tests/unit/components/sidebar/SidebarNavigation.test.tsx

@@ -0,0 +1,102 @@
+/**
+ * SidebarNavigation — section headers must be controls, and a toggle must toggle.
+ *
+ * George reported clicking "Fund" / "Coordinate" / "Finance" did nothing: the
+ * label was an <h3> and only the small chevron beside it carried the onClick.
+ * This is the second time this class landed in the same sidebar (see
+ * ContextSwitcher.test.tsx — "the avatar must be a control, not a dead pixel"),
+ * so it gets a guard rather than a third fix.
+ *
+ * Three assertions, one per way the toggle used to lie to the user:
+ *  1. the whole header row is one control — clicking the WORD toggles
+ *  2. an explicit collapse wins even when the section holds the active page
+ *  3. the chevron's state matches what is actually rendered (aria-expanded)
+ */
+
+import React from 'react';
+import { render, screen, fireEvent } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import { Home } from 'lucide-react';
+import { SidebarNavigation } from '@/components/sidebar/SidebarNavigation';
+import type { NavSection } from '@/hooks/useNavigation';
+
+jest.mock('next/navigation', () => ({
+  useRouter: () => ({ push: jest.fn() }),
+  usePathname: () => '/projects',
+}));
+
+jest.mock('@/stores/messaging', () => ({
+  useUnreadCount: () => 0,
+}));
+
+const SECTIONS: NavSection[] = [
+  {
+    id: 'main',
+    title: '',
+    priority: 1,
+    collapsible: false,
+    items: [{ name: 'Home', href: '/dashboard', icon: Home }],
+  },
+  {
+    id: 'fund',
+    title: 'Fund',
+    priority: 3,
+    collapsible: true,
+    items: [{ name: 'Projects', href: '/projects', icon: Home }],
+  },
+];
+
+// /projects is active — it lives inside the collapsible "Fund" section
+const isItemActive = (href: string) => href === '/projects';
+
+function renderNav(collapsed: string[], toggleSection = jest.fn()) {
+  const view = render(
+    <SidebarNavigation
+      sections={SECTIONS}
+      bottomItems={[]}
+      isExpanded
+      collapsedSections={new Set(collapsed)}
+      isItemActive={isItemActive}
+      toggleSection={toggleSection}
+    />
+  );
+  return { toggleSection, ...view };
+}
+
+describe('SidebarNavigation section headers', () => {
+  it('toggles when the section WORD is clicked, not just the chevron', () => {
+    const { toggleSection } = renderNav([]);
+
+    // The label itself must sit inside a real control.
+    const header = screen.getByRole('heading', { name: 'Fund' });
+    const control = header.closest('button');
+    expect(control).not.toBeNull();
+
+    fireEvent.click(header);
+    expect(toggleSection).toHaveBeenCalledWith('fund');
+  });
+
+  it('honours an explicit collapse even when the section holds the active page', () => {
+    renderNav(['fund']);
+
+    // "Projects" is the active route; collapsing "Fund" used to be a no-op
+    // because an active child forced the list to stay rendered.
+    expect(screen.queryByText('Projects')).not.toBeInTheDocument();
+  });
+
+  it('reports expanded state that matches what is rendered', () => {
+    const { unmount } = renderNav(['fund']);
+    expect(screen.getByRole('heading', { name: 'Fund' }).closest('button')).toHaveAttribute(
+      'aria-expanded',
+      'false'
+    );
+    unmount();
+
+    renderNav([]);
+    expect(screen.getByRole('heading', { name: 'Fund' }).closest('button')).toHaveAttribute(
+      'aria-expanded',
+      'true'
+    );
+    expect(screen.getByText('Projects')).toBeInTheDocument();
+  });
+});