From 978623337c12dc1721fe5236eadbf9a5ad929f03 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Fri, 7 Aug 2026 10:32:48 +0000
Subject: [PATCH 1/2] feat(navigation): adopt the shared mode nav for
 Factsheets
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Factsheets is the one of the eight remaining modes with a genuine second
destination: `/factsheets` is the browse home (category chips + featured
grid) and `/factsheets/search` is a separate component with filters, a
view toggle and result rows. It registered a single action-only entry,
which rendered one <button> whose only effect was focusing a composer
already visible on the same screen.

Replace that with two routed entries — Topics and Search — and add the
mode to MODE_NAV_ADOPTED_MODES. Two items fit the 22rem band with no
overflow slot, the same shape as DSM.

Details worth knowing:

- Topics carries no `focus: true`, unlike the Search/Find entry of every
  other adopted mode. Those tabs are their mode's search affordance;
  Topics is a browse destination, and autofocusing would open the phone
  keyboard over the topics the user asked to see.
- The Search tab carries q, category and run. It is the current tab on
  /factsheets/search, and dropping `run` flips hasSubmittedModeSearch,
  which re-places the composer for a click that changed nothing.
- factsheets gets its own activeModeSecondaryNavigationId branch. The
  array-index-0 fallback would otherwise mark Topics current on every
  unmatched path.
- The icon is BookOpenText, the Factsheets hero glyph, not LayoutGrid —
  the search page already uses LayoutGrid for its card/list view toggle.

Adopting expires the incidental protection that kept the bar off
/factsheets/[slug] (it had fewer than MODE_NAV_MIN_ITEMS destinations).
What keeps that route clear now is only the hasLocalInformationPageNavigation
early return, so it is pinned at render rather than left to the count.

Verification: typecheck and lint clean; 5 focused unit files (95 tests)
pass; full offline suite 518/519 files, with the single failure
(tests/pr-handoff-stop.test.ts) confirmed pre-existing by re-running it
with these changes stashed. ui-mode-nav-density: 55 passed, including 7
new factsheets rows showing both labels in full at every band >=352px
and collapsing at 351px. Both new cross-file pins were mutation-checked:
removing the density table row fails 2 tests in mode-nav-contract, and
dropping the `run` carry fails the href test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01823Ctwj4vinGhGuRNyK7oE
---
 src/components/mode-nav/registry-mode-nav.tsx | 15 +++++-
 src/lib/mode-secondary-navigation.ts          | 45 +++++++++++++++-
 tests/mode-nav-addon-slot.dom.test.tsx        |  8 ++-
 tests/mode-secondary-navigation.test.ts       | 51 ++++++++++++++++++-
 tests/page-secondary-navigation.dom.test.tsx  | 46 +++++++++++++++++
 tests/ui-mode-nav-density.spec.ts             |  1 +
 6 files changed, 161 insertions(+), 5 deletions(-)

diff --git a/src/components/mode-nav/registry-mode-nav.tsx b/src/components/mode-nav/registry-mode-nav.tsx
index bd8f8f9978..f5f3bb9544 100644
--- a/src/components/mode-nav/registry-mode-nav.tsx
+++ b/src/components/mode-nav/registry-mode-nav.tsx
@@ -1,6 +1,14 @@
 "use client";
 
-import { GitCompareArrows, ListChecks, Network, Search, Stethoscope, type LucideIcon } from "lucide-react";
+import {
+  BookOpenText,
+  GitCompareArrows,
+  ListChecks,
+  Network,
+  Search,
+  Stethoscope,
+  type LucideIcon,
+} from "lucide-react";
 
 import { ModeNav, type ModeNavItem } from "@/components/mode-nav/mode-nav";
 import { appModeDefinition, type AppModeId } from "@/lib/app-modes";
@@ -23,6 +31,11 @@ const iconByItemId: Record<RoutedModeSecondaryNavigationId, LucideIcon> = {
   compare: GitCompareArrows,
   builder: ListChecks,
   map: Network,
+  // The Factsheets hero glyph (`factsheets-home-page.tsx`), so the tab wears the
+  // same mark as the surface it points at. Not LayoutGrid: the search page uses
+  // that for its card/list view toggle, and one glyph must not mean two things
+  // on the same screen.
+  topics: BookOpenText,
 };
 
 /**
diff --git a/src/lib/mode-secondary-navigation.ts b/src/lib/mode-secondary-navigation.ts
index f57391e7fc..f71ee39945 100644
--- a/src/lib/mode-secondary-navigation.ts
+++ b/src/lib/mode-secondary-navigation.ts
@@ -62,7 +62,20 @@ export const modeSecondaryNavigationRegistry = {
     { id: "brief", label: "Brief Intervention", action: "therapy-brief" },
     { id: "sheets", label: "Patient Sheets", action: "therapy-sheets" },
   ],
-  factsheets: [{ id: "search", label: "Search", action: "search" }],
+  // Two genuinely distinct surfaces: `/factsheets` is the browse home (category
+  // chips + a featured grid) and `/factsheets/search` is a separate component
+  // with filters, a view toggle and result rows. `/factsheets/[slug]` is a
+  // record and never reaches here — `hasLocalInformationPageNavigation` returns
+  // null for it first.
+  // No `focus: true` on Topics, unlike the Search/Find entry of every mode
+  // above. Those tabs are the mode's search affordance, so focusing the composer
+  // on arrival is the point. Topics is a browse destination — autofocusing there
+  // would open the phone keyboard over the topics the user asked to see. The
+  // search affordance for this mode is the Search tab.
+  factsheets: [
+    { id: "topics", label: "Topics", href: appModeHomeHref("factsheets") },
+    { id: "search", label: "Search", href: "/factsheets/search" },
+  ],
 } as const satisfies Record<AppModeId, readonly ModeSecondaryNavigationEntry[]>;
 
 type RegistryEntry = (typeof modeSecondaryNavigationRegistry)[AppModeId][number];
@@ -94,6 +107,7 @@ export const MODE_NAV_ADOPTED_MODES = [
   "specifiers",
   "formulation",
   "differentials",
+  "factsheets",
 ] as const satisfies readonly AppModeId[];
 
 export function modeUsesHeaderModeNav(modeId: AppModeId): boolean {
@@ -139,6 +153,14 @@ export function activeModeSecondaryNavigationId(modeId: AppModeId, pathname: str
     if (pathname === `/${modeId}` || pathname.startsWith(`/${modeId}?`)) return "search";
     return null;
   }
+  if (modeId === "factsheets") {
+    if (pathname === "/factsheets/search" || pathname.startsWith("/factsheets/search?")) return "search";
+    if (pathname === "/factsheets" || pathname.startsWith("/factsheets?")) return "topics";
+    // `/factsheets/<slug>` is a record. It cannot reach `ModeNav` today —
+    // `hasLocalInformationPageNavigation` returns null for it first — but the
+    // array-index-0 fallback below would mark Topics current if it ever did.
+    return null;
+  }
   return modeSecondaryNavigationRegistry[modeId][0]?.id ?? null;
 }
 
@@ -165,6 +187,10 @@ export function isModeSecondaryNavigationRoute(params: {
       pathname === "/formulation/builder" || pathname === "/formulation/compare" || pathname === "/formulation/map"
     );
   }
+  // Same shape as `dsm` above: list the routed destination that is not the mode
+  // home. The clean `/factsheets` home stays out so its `ModeHomeTemplate` tiles
+  // remain the single answer to "where can I go"; it reaches the bar through the
+  // `hasSubmittedSearch` early return, and Topics is marked current there.
   if (modeId === "factsheets") return pathname === "/factsheets/search";
   if (modeId === "therapy-compass") return pathname !== "/therapy-compass";
   return false;
@@ -284,5 +310,22 @@ export function modeSecondaryNavigationHref(params: {
     ]);
   }
 
+  if (modeId === "factsheets") {
+    // Search carries the live query and category filter so switching tabs does
+    // not silently discard them. Topics goes to the clean browse home: it reads
+    // neither param, so appending them would only produce a misleading URL.
+    if (itemId !== "search") return href;
+    const category = currentSearchParams.get("category");
+    return navigationHrefWithParams(href, [
+      ...(query ? ([["q", query]] as const) : []),
+      ...(category ? ([["category", category]] as const) : []),
+      // `run` travels with the query, as it does for dsm. Search is the current
+      // tab on /factsheets/search, and dropping `run` from its own link flips
+      // `hasSubmittedModeSearch` (global-search-shell.tsx:421) to false, which
+      // re-places the composer — a layout jump from clicking where you already are.
+      ...(query && currentSearchParams.get("run") === "1" ? ([["run", "1"]] as const) : []),
+    ]);
+  }
+
   return href;
 }
diff --git a/tests/mode-nav-addon-slot.dom.test.tsx b/tests/mode-nav-addon-slot.dom.test.tsx
index 646b8600da..02bebe93d2 100644
--- a/tests/mode-nav-addon-slot.dom.test.tsx
+++ b/tests/mode-nav-addon-slot.dom.test.tsx
@@ -127,7 +127,13 @@ describe("header addon slot ownership", () => {
     // `documents` owns the slot on every detail route and has one registered
     // destination. Adopting it would be a deletion decision about that lone
     // entry, not a port — deliberately out of this rollout's scope.
-    for (const modeId of ["documents", "answer", "prescribing", "tools", "factsheets"] as const) {
+    //
+    // `factsheets` left this list when it gained a real second destination:
+    // `/factsheets` (browse) and `/factsheets/search` are separate components,
+    // so it is a port rather than a deletion. The others still have one surface
+    // each, and ModeNav renders nothing below two items — adopting them would
+    // remove the control they have and put nothing back (PR #1645).
+    for (const modeId of ["documents", "answer", "prescribing", "tools"] as const) {
       expect([...MODE_NAV_ADOPTED_MODES]).not.toContain(modeId);
       expect(modeUsesHeaderModeNav(modeId)).toBe(false);
     }
diff --git a/tests/mode-secondary-navigation.test.ts b/tests/mode-secondary-navigation.test.ts
index bca5432276..0407478d77 100644
--- a/tests/mode-secondary-navigation.test.ts
+++ b/tests/mode-secondary-navigation.test.ts
@@ -24,7 +24,7 @@ const expectedLabels: Record<AppModeId, string[]> = {
   prescribing: ["Search"],
   tools: ["Search"],
   "therapy-compass": ["Search", "Recommend", "Compare", "Pathways", "Brief Intervention", "Patient Sheets"],
-  factsheets: ["Search"],
+  factsheets: ["Topics", "Search"],
 };
 
 const cleanLandingPath: Record<AppModeId, string> = {
@@ -134,6 +134,39 @@ describe("mode secondary navigation registry", () => {
         currentSearchParams: new URLSearchParams("q=confusion&ids=delirium%2Cdementia"),
       }),
     ).toBe("/differentials/presentations?q=confusion&ids=delirium%2Cdementia");
+
+    // Search is the CURRENT tab on /factsheets/search, so its own link must not
+    // reset what you are looking at. `run` is carried with the query because
+    // dropping it flips hasSubmittedModeSearch and re-places the composer.
+    expect(
+      modeSecondaryNavigationHref({
+        modeId: "factsheets",
+        itemId: "search",
+        href: "/factsheets/search",
+        currentSearchParams: new URLSearchParams("q=sertraline&category=Medicines&run=1"),
+      }),
+    ).toBe("/factsheets/search?q=sertraline&category=Medicines&run=1");
+
+    // The browse home's category chips link with a category and no query.
+    expect(
+      modeSecondaryNavigationHref({
+        modeId: "factsheets",
+        itemId: "search",
+        href: "/factsheets/search",
+        currentSearchParams: new URLSearchParams("category=Medicines"),
+      }),
+    ).toBe("/factsheets/search?category=Medicines");
+
+    // Topics is the mode home: it reads neither param, so carrying them there
+    // would only put dead query string into a URL people share.
+    expect(
+      modeSecondaryNavigationHref({
+        modeId: "factsheets",
+        itemId: "topics",
+        href: "/factsheets",
+        currentSearchParams: new URLSearchParams("q=sertraline&category=Medicines&run=1"),
+      }),
+    ).toBe("/factsheets");
   });
 
   it("adopts only modes with two or more routed destinations (explicit list, not silent derivation)", () => {
@@ -142,7 +175,13 @@ describe("mode secondary navigation registry", () => {
     // remaining mode still has two routed entries, while the negative check
     // below only inspects modes with fewer than two. A mode silently losing the
     // bar is the regression this list exists to make impossible.
-    expect([...MODE_NAV_ADOPTED_MODES].sort()).toEqual(["differentials", "dsm", "formulation", "specifiers"]);
+    expect([...MODE_NAV_ADOPTED_MODES].sort()).toEqual([
+      "differentials",
+      "dsm",
+      "factsheets",
+      "formulation",
+      "specifiers",
+    ]);
 
     for (const modeId of MODE_NAV_ADOPTED_MODES) {
       expect(
@@ -165,6 +204,14 @@ describe("mode secondary navigation registry", () => {
     expect(activeModeSecondaryNavigationId("dsm", "/dsm/diagnoses/major-depressive-disorder")).toBeNull();
     expect(activeModeSecondaryNavigationId("specifiers", "/specifiers/builder")).toBe("builder");
     expect(activeModeSecondaryNavigationId("specifiers", "/specifiers")).toBe("search");
+
+    // Factsheets records cannot reach ModeNav today (hasLocalInformationPageNavigation
+    // returns null for them first), but the registry fallback would mark the
+    // first entry — Topics — current on any unmatched path, so the mode needs
+    // its own branch rather than inheriting that default.
+    expect(activeModeSecondaryNavigationId("factsheets", "/factsheets/sertraline")).toBeNull();
+    expect(activeModeSecondaryNavigationId("factsheets", "/factsheets")).toBe("topics");
+    expect(activeModeSecondaryNavigationId("factsheets", "/factsheets/search")).toBe("search");
   });
 
   it("matches workflow destinations by path segment, not substring", () => {
diff --git a/tests/page-secondary-navigation.dom.test.tsx b/tests/page-secondary-navigation.dom.test.tsx
index 8262f8a37d..0c17b75c2f 100644
--- a/tests/page-secondary-navigation.dom.test.tsx
+++ b/tests/page-secondary-navigation.dom.test.tsx
@@ -177,6 +177,52 @@ describe("PageSecondaryNavigation", () => {
     ]);
   });
 
+  it("gives factsheets both destinations and keeps the current tab's filter state", () => {
+    render(
+      <PageSecondaryNavigation
+        modeId="factsheets"
+        pathname="/factsheets/search"
+        hasSubmittedSearch={false}
+        searchParamString="q=sertraline&category=Medicines&run=1"
+        onSearch={vi.fn()}
+      />,
+    );
+    const bar = screen.getByTestId("mode-nav");
+    expect(bar).toHaveAttribute("aria-label", "Factsheets pages");
+    expect([...bar.querySelectorAll("li a")].map((link) => link.textContent)).toEqual(["Topics", "Search"]);
+    expect(screen.getByRole("link", { name: "Search" })).toHaveAttribute("aria-current", "page");
+    // Search is the tab you are already on. Its own link must not reset the
+    // category filter you are reading, nor drop `run` — that flips
+    // hasSubmittedModeSearch and re-places the composer for a no-op click.
+    expect(screen.getByRole("link", { name: "Search" })).toHaveAttribute(
+      "href",
+      "/factsheets/search?q=sertraline&category=Medicines&run=1",
+    );
+    // Topics is the browse home: it reads neither param, and carries no
+    // focus=1 either — autofocusing the composer there would open the phone
+    // keyboard over the topics the user just asked to browse.
+    expect(screen.getByRole("link", { name: "Topics" })).toHaveAttribute("href", "/factsheets");
+  });
+
+  it("keeps the newly adopted factsheets bar off its record routes", () => {
+    // Until adoption, `/factsheets/<slug>` was bar-free incidentally: the mode
+    // had one destination and ModeNav renders nothing below two. That
+    // protection expired the moment factsheets got a second one. What keeps the
+    // record route clear now is only the hasLocalInformationPageNavigation
+    // early return, so pin it at render rather than trusting the count.
+    expect(hasLocalInformationPageNavigation("/factsheets/sertraline")).toBe(true);
+    render(
+      <PageSecondaryNavigation
+        modeId="factsheets"
+        pathname="/factsheets/sertraline"
+        hasSubmittedSearch
+        onSearch={vi.fn()}
+      />,
+    );
+    expect(screen.queryByTestId("mode-nav")).toBeNull();
+    expect(screen.queryByTestId("secondary-navigation")).toBeNull();
+  });
+
   it("replaces mode navigation with only the information sections present in the record", async () => {
     render(
       <div>
diff --git a/tests/ui-mode-nav-density.spec.ts b/tests/ui-mode-nav-density.spec.ts
index a56c9db889..75331ab187 100644
--- a/tests/ui-mode-nav-density.spec.ts
+++ b/tests/ui-mode-nav-density.spec.ts
@@ -67,6 +67,7 @@ const MODES = [
   { modeId: "specifiers", route: "/specifiers/compare", items: 4 },
   { modeId: "formulation", route: "/formulation/compare", items: 4 },
   { modeId: "differentials", route: "/differentials/diagnoses", items: 3 },
+  { modeId: "factsheets", route: "/factsheets/search", items: 2 },
 ];
 
 /**

From 4b24c3435f3ec9b2fd9a7c6db78c5197f873d86a Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Fri, 7 Aug 2026 10:37:16 +0000
Subject: [PATCH 2/2] docs(ledger): record the factsheets mode-nav adoption
 review (PR #1674)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01823Ctwj4vinGhGuRNyK7oE
---
 docs/branch-review-ledger.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md
index f67efb8a1e..7f8733fac8 100644
--- a/docs/branch-review-ledger.md
+++ b/docs/branch-review-ledger.md
@@ -698,3 +698,4 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie
 | 2026-08-07 | cursor/grok-quick-wins-a2c0 (PR #1651) | 2563b4adf4f0c0580ca371acfee94f9ae1ca3d0f | prlanded | MERGED; squash tip empty vs branch tip 4655643770dba7ca4078f1586c0461fd91657bf9; Grok-safe quick wins (tokens, a11y, perf, gates) | content tree empty vs squash; no provider-backed checks run |
 | 2026-08-07 | cursor/viewer-phase0-gesture-a11y-1db8 (PR #1660) | 097dfd245f798f8105eeb6c1cf4fc077f969496f | prlanded | MERGED; squash tip empty vs branch tip 810cfc9b4a1c476a0dcc995bffb00d7329a85686; Phase 0 gesture INP, preview a11y, image decode | content tree empty vs squash; no provider-backed checks run |
 | 2026-08-07 | claude/pr-handoff-loop-prevention-54y5zr (PR #1670) | dfe2946110e0ff93bd4acc571ae79c26b79a7a85 | PR #1670 heavy review-and-fix | synced origin/main (behind-but-clean DIRTY cleared); fixed CodeRabbit checks-cell to name exact #1649 gates + incomplete verify:pr-local/ui + no provider checks; Bugbot none; no P0/P1; #258/#ledger delta accurate; merge-tree clean; threads cleared | verify:cheap 519 files/5493 passed; verify:pr-local docs scope (format+docs+ledger+outstanding-issues); check:branch-review-ledger; check:outstanding-issues; no provider gates |
+| 2026-08-07 | claude/handover-review-nlhuln | 978623337c12dc1721fe5236eadbf9a5ad929f03 | mode nav remaining modes: factsheets adoption (PR #1674) | Adopted the shared ModeNav for factsheets (Topics + Search); replaced the action-only entry, added the activeId branch, q/category/run carry, BookOpenText icon; three pinned adopted-mode lists updated together; record-route protection pinned at render now the item-count protection has expired | lint clean; typecheck clean; test 518/519 files (pr-handoff-stop failure confirmed pre-existing via stashed re-run); focused 5 files 95 tests; ui-mode-nav-density 55 passed incl 7 new factsheets rows; two mutation checks confirmed red; format committed; verify:pr-local blocked at check:installed-lock-parity (playwright 1.62.0 vs 1.62.1) |