From d7eaaa31edc5b2ccd9768458717682a9076ca34c Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Tue, 29 Sep 2026 03:55:40 -0400 Subject: [PATCH 01/13] fix(chat): show the date on messages that are not from today, with the full time on hover A message showed only its clock time, so reopening an old thread repeated one time of day on every turn with no way to tell which day. formatMessageTime now adds the month and day for an earlier day this year and the year for an earlier year, in the viewer's locale and time zone, and the time element carries the full date and time in its title. Co-Authored-By: Claude Sonnet 5.5 --- .../src/features/chat/MessageCard.test.tsx | 20 ++++++ frontend/src/features/chat/MessageCard.tsx | 4 +- frontend/src/lib/messageTime.test.ts | 70 ++++++++++++++++++- frontend/src/lib/messageTime.ts | 41 +++++++++-- 4 files changed, 128 insertions(+), 7 deletions(-) diff --git a/frontend/src/features/chat/MessageCard.test.tsx b/frontend/src/features/chat/MessageCard.test.tsx index 2a92705c..c8319817 100644 --- a/frontend/src/features/chat/MessageCard.test.tsx +++ b/frontend/src/features/chat/MessageCard.test.tsx @@ -63,6 +63,26 @@ describe("MessageCard", () => { expect(screen.queryByRole("region", { name: "Cortex suggests remembering" })).toBeNull(); }); + it("shows the date on an older message and always carries the full time in a title", () => { + renderCard({ id: "assistant-1", role: "assistant", content: "Old answer.", timestamp: "2020-01-05T10:00:00Z" }); + + const time = document.querySelector("time"); + expect(time).not.toBeNull(); + expect(time).toHaveAttribute("datetime", "2020-01-05T10:00:00Z"); + // Long ago: the year is part of the visible text, not only the tooltip. + expect(time?.textContent).toContain("2020"); + expect(time?.getAttribute("title")).toContain("2020"); + }); + + it("shows only the clock time for a message from today", () => { + const now = new Date(); + renderCard({ id: "assistant-1", role: "assistant", content: "New answer.", timestamp: now.toISOString() }); + + const time = document.querySelector("time"); + expect(time?.textContent).not.toContain(String(now.getFullYear())); + expect(time?.getAttribute("title")).toContain(String(now.getFullYear())); + }); + it("drops the suggestions from the card once they have all been decided", () => { useChatStore.getState().setProposedMemories("assistant-1", ["User likes tea."]); renderCard({ id: "assistant-1", role: "assistant", content: "Noted." }, vi.fn()); diff --git a/frontend/src/features/chat/MessageCard.tsx b/frontend/src/features/chat/MessageCard.tsx index a8da0865..48dd3c7e 100644 --- a/frontend/src/features/chat/MessageCard.tsx +++ b/frontend/src/features/chat/MessageCard.tsx @@ -1,7 +1,7 @@ import { memo, useState } from "react"; import { Copy, FileText, GitBranch, Image as ImageIcon, RefreshCw } from "lucide-react"; import type { ChatAttachment, ChatMessage, GenerationStats } from "../../../../contracts/cortex-api"; -import { formatMessageTime } from "../../lib/messageTime"; +import { formatMessageTime, formatMessageTimeTitle } from "../../lib/messageTime"; import { useChatStore } from "../../stores/useChatStore"; import { MemoryProposals } from "./MemoryProposals"; import { MessageStats } from "./MessageStats"; @@ -102,5 +102,5 @@ export const MessageCard = memo(function MessageCard({ message, index, isFinalAs function MessageMeta({ timestamp, stats }: { timestamp?: string | null; stats?: GenerationStats | null }) { const displayTime = formatMessageTime(timestamp); if (!displayTime && !stats?.tokens_per_second && !stats?.stopped) return null; - return
{displayTime && }
; + return
{displayTime && }
; } diff --git a/frontend/src/lib/messageTime.test.ts b/frontend/src/lib/messageTime.test.ts index a38a2bf6..eb866f97 100644 --- a/frontend/src/lib/messageTime.test.ts +++ b/frontend/src/lib/messageTime.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; -import { formatMessageTime } from "./messageTime"; +import { formatMessageTime, formatMessageTimeTitle } from "./messageTime"; describe("formatMessageTime", () => { it("reads a timestamp without an offset as UTC, not local time", () => { @@ -29,3 +29,71 @@ describe("formatMessageTime", () => { expect(formatMessageTime("not a date")).toBeNull(); }); }); + +// Local-time constructors keep these independent of the machine's time zone: +// "same day" is decided in the viewer's zone, so the fixtures are built there. +const NOW = new Date(2026, 5, 25, 12, 0, 0); +const at = (year: number, month: number, day: number, hour: number, minute: number) => + new Date(year, month, day, hour, minute, 0).toISOString(); +const monthName = (iso: string) => new Intl.DateTimeFormat(undefined, { month: "short" }).format(Date.parse(iso)); +const clock = (iso: string) => new Intl.DateTimeFormat(undefined, { hour: "numeric", minute: "2-digit" }).format(Date.parse(iso)); + +describe("formatMessageTime date context", () => { + it("shows only the clock time for a message from today", () => { + const today = at(2026, 5, 25, 8, 5); + const shown = formatMessageTime(today, NOW); + expect(shown).toBe(clock(today)); + expect(shown).not.toContain(monthName(today)); + expect(shown).not.toContain("2026"); + }); + + it("adds the month and day for an earlier day this year, but not the year", () => { + const tenDaysAgo = at(2026, 5, 15, 9, 30); + const shown = formatMessageTime(tenDaysAgo, NOW); + expect(shown).toContain(monthName(tenDaysAgo)); + expect(shown).toContain("15"); + expect(shown).toContain(clock(tenDaysAgo)); + expect(shown).not.toContain("2026"); + }); + + it("adds the year for a message from an earlier year", () => { + const lastYear = at(2025, 11, 31, 22, 45); + const shown = formatMessageTime(lastYear, NOW); + expect(shown).toContain("2025"); + expect(shown).toContain(monthName(lastYear)); + expect(shown).toContain(clock(lastYear)); + }); + + it("treats last night as another day, not as today", () => { + // Just after midnight the previous evening's messages are still "yesterday". + const justAfterMidnight = new Date(2026, 5, 25, 0, 1, 0); + const lastNight = at(2026, 5, 24, 23, 59); + expect(formatMessageTime(lastNight, justAfterMidnight)).toContain(monthName(lastNight)); + expect(formatMessageTime(at(2026, 5, 25, 0, 0), justAfterMidnight)).not.toContain(monthName(lastNight)); + }); + + it("does not need a date for the same day-of-month in another month", () => { + const lastMonth = at(2026, 4, 25, 12, 0); + expect(formatMessageTime(lastMonth, NOW)).toContain(monthName(lastMonth)); + }); +}); + +describe("formatMessageTimeTitle", () => { + it("spells out the full date and time", () => { + const title = formatMessageTimeTitle(at(2026, 5, 15, 9, 30)); + expect(title).toContain("2026"); + expect(title).toContain("15"); + // Longer than the compact form: it carries the weekday and seconds too. + expect(title!.length).toBeGreaterThan(formatMessageTime(at(2026, 5, 15, 9, 30), NOW)!.length); + }); + + it("agrees with the compact form on which instant it describes, including a missing offset", () => { + expect(formatMessageTimeTitle("2026-01-01T12:00:00")).toBe(formatMessageTimeTitle("2026-01-01T12:00:00Z")); + }); + + it("returns null for missing or unparsable values", () => { + expect(formatMessageTimeTitle(null)).toBeNull(); + expect(formatMessageTimeTitle(undefined)).toBeNull(); + expect(formatMessageTimeTitle("not a date")).toBeNull(); + }); +}); diff --git a/frontend/src/lib/messageTime.ts b/frontend/src/lib/messageTime.ts index 3fec0f16..8151ea88 100644 --- a/frontend/src/lib/messageTime.ts +++ b/frontend/src/lib/messageTime.ts @@ -1,5 +1,5 @@ /** - * Format a stored message timestamp for display. + * Parse a stored message timestamp into epoch milliseconds. * * ECMAScript parses a zone-less date-time as *local* time, so a timestamp * stored without an offset renders shifted by the viewer's UTC offset. The @@ -7,10 +7,43 @@ * other producer -- or a row written by an older build -- from reintroducing * the shift. */ -export function formatMessageTime(value?: string | null): string | null { +function parseMessageTimestamp(value?: string | null): number | null { if (!value) return null; const normalized = /[Zz]|[+-]\d{2}:?\d{2}$/.test(value) ? value : `${value}Z`; const timestamp = Date.parse(normalized); - if (Number.isNaN(timestamp)) return null; - return new Intl.DateTimeFormat(undefined, { hour: "numeric", minute: "2-digit" }).format(timestamp); + return Number.isNaN(timestamp) ? null : timestamp; +} + +const CLOCK_TIME: Intl.DateTimeFormatOptions = { hour: "numeric", minute: "2-digit" }; + +/** + * Format a stored message timestamp for display, in the viewer's locale and + * time zone. A message from today shows only the clock time; an earlier one in + * the same year adds the month and day, and one from an earlier year adds the + * year, so a reopened three-week-old thread does not repeat one time of day on + * every turn with no way to tell which day it was. + * + * `now` is a parameter so the "same day" boundary can be tested. + */ +export function formatMessageTime(value?: string | null, now: Date = new Date()): string | null { + const timestamp = parseMessageTimestamp(value); + if (timestamp === null) return null; + const moment = new Date(timestamp); + const options: Intl.DateTimeFormatOptions = { ...CLOCK_TIME }; + if (moment.getFullYear() !== now.getFullYear()) { + options.year = "numeric"; + options.month = "short"; + options.day = "numeric"; + } else if (moment.getMonth() !== now.getMonth() || moment.getDate() !== now.getDate()) { + options.month = "short"; + options.day = "numeric"; + } + return new Intl.DateTimeFormat(undefined, options).format(timestamp); +} + +/** The full date and time, for a `title` (and so a tooltip) on the compact form above. */ +export function formatMessageTimeTitle(value?: string | null): string | null { + const timestamp = parseMessageTimestamp(value); + if (timestamp === null) return null; + return new Intl.DateTimeFormat(undefined, { dateStyle: "full", timeStyle: "medium" }).format(timestamp); } From c33adf157c0d0bebd946cbabad03f8bc6560fdd6 Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Tue, 29 Sep 2026 03:55:40 -0400 Subject: [PATCH 02/13] feat(markdown): wrap toggle for code, task-list markers, and a note where an image was dropped Fenced code could only scroll sideways: the toolbar now has a Wrap toggle that applies to every block and is remembered in local storage (falling back to the session when storage is blocked). GFM task lists showed a bullet and a checkbox; the marker is now removed and the checkbox styled. Images are still never loaded, but where one was dropped there is now a placeholder naming its alt text, with the source on hover, instead of nothing. Co-Authored-By: Claude Sonnet 5.5 --- .../features/markdown/SafeMarkdown.test.tsx | 166 +++++++++++++++++- .../src/features/markdown/SafeMarkdown.tsx | 73 +++++++- frontend/src/styles/tokens.css | 12 ++ 3 files changed, 246 insertions(+), 5 deletions(-) diff --git a/frontend/src/features/markdown/SafeMarkdown.test.tsx b/frontend/src/features/markdown/SafeMarkdown.test.tsx index c1f63dc2..8cc58839 100644 --- a/frontend/src/features/markdown/SafeMarkdown.test.tsx +++ b/frontend/src/features/markdown/SafeMarkdown.test.tsx @@ -1,5 +1,6 @@ import { fireEvent, render, screen, waitFor } from "@testing-library/react"; -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { declarationsOf, parseRules, selectorsOf, tokensCss } from "../../test/css"; import { SafeMarkdown } from "./SafeMarkdown"; describe("SafeMarkdown", () => { @@ -108,4 +109,167 @@ describe("SafeMarkdown", () => { expect(table).not.toHaveAttribute("node"); expect(consoleError).not.toHaveBeenCalled(); }); + + describe("code wrapping", () => { + const WRAP_KEY = "cortex.codeWrap"; + const TWO_BLOCKS = "```ts\nconst a = 1;\n```\n\n```py\nb = 2\n```"; + + afterEach(() => { + window.localStorage.clear(); + }); + + it("offers a Wrap toggle beside Copy that folds long lines and remembers the choice", () => { + render(); + + const toggle = screen.getByRole("button", { name: "Wrap ts code lines" }); + const block = document.querySelector(".code-block"); + // Off by default: code keeps scrolling sideways, as before. + expect(toggle).toHaveAttribute("aria-pressed", "false"); + expect(block).not.toHaveClass("code-block-wrap"); + // Both controls live in the toolbar, outside the
.
+      expect(toggle.closest(".code-block-toolbar")).not.toBeNull();
+      expect(screen.getByRole("button", { name: "Copy ts code" }).closest(".code-block-toolbar")).not.toBeNull();
+
+      fireEvent.click(toggle);
+
+      expect(toggle).toHaveAttribute("aria-pressed", "true");
+      expect(block).toHaveClass("code-block-wrap");
+      expect(window.localStorage.getItem(WRAP_KEY)).toBe("1");
+
+      fireEvent.click(toggle);
+      expect(block).not.toHaveClass("code-block-wrap");
+      expect(window.localStorage.getItem(WRAP_KEY)).toBe("0");
+    });
+
+    it("applies one choice to every block on the page", () => {
+      render();
+
+      fireEvent.click(screen.getByRole("button", { name: "Wrap ts code lines" }));
+
+      const blocks = document.querySelectorAll(".code-block");
+      expect(blocks).toHaveLength(2);
+      for (const block of blocks) expect(block).toHaveClass("code-block-wrap");
+      expect(screen.getByRole("button", { name: "Wrap py code lines" })).toHaveAttribute("aria-pressed", "true");
+    });
+
+    it("starts wrapped when the saved choice says so", () => {
+      window.localStorage.setItem(WRAP_KEY, "1");
+      render();
+
+      expect(document.querySelector(".code-block")).toHaveClass("code-block-wrap");
+      expect(screen.getByRole("button", { name: "Wrap ts code lines" })).toHaveAttribute("aria-pressed", "true");
+    });
+
+    it("still toggles for the session when storage refuses the write", () => {
+      const setItem = vi.spyOn(Storage.prototype, "setItem").mockImplementation(() => {
+        throw new DOMException("blocked", "SecurityError");
+      });
+      render();
+      const toggle = screen.getByRole("button", { name: "Wrap ts code lines" });
+
+      fireEvent.click(toggle);
+      expect(toggle).toHaveAttribute("aria-pressed", "true");
+      expect(document.querySelector(".code-block")).toHaveClass("code-block-wrap");
+
+      // With storage working again the next click is stored, and the
+      // session-only override is gone.
+      setItem.mockRestore();
+      fireEvent.click(toggle);
+      expect(toggle).toHaveAttribute("aria-pressed", "false");
+      expect(window.localStorage.getItem(WRAP_KEY)).toBe("0");
+    });
+
+    it("keeps working when storage cannot even be read", () => {
+      vi.spyOn(Storage.prototype, "getItem").mockImplementation(() => {
+        throw new DOMException("blocked", "SecurityError");
+      });
+      render();
+
+      expect(screen.getByRole("button", { name: "Wrap ts code lines" })).toHaveAttribute("aria-pressed", "false");
+      vi.restoreAllMocks();
+    });
+
+    it("does not change what Copy puts on the clipboard", async () => {
+      if (!navigator.clipboard) Object.defineProperty(navigator, "clipboard", { value: {}, configurable: true });
+      const writeText = vi.fn().mockResolvedValue(undefined);
+      Object.defineProperty(navigator.clipboard, "writeText", { value: writeText, configurable: true, writable: true });
+      window.localStorage.setItem(WRAP_KEY, "1");
+      render();
+
+      fireEvent.click(screen.getByRole("button", { name: "Copy ts code" }));
+      await waitFor(() => expect(writeText).toHaveBeenCalledWith("const answer = 42;"));
+    });
+  });
+
+  describe("task lists", () => {
+    it("renders each task as a disabled checkbox inside a list the stylesheet can unmark", () => {
+      render();
+
+      const list = document.querySelector("ul");
+      // The classes the stylesheet keys on to drop the bullet, so a task shows
+      // its checkbox alone instead of a bullet and a checkbox.
+      expect(list).toHaveClass("contains-task-list");
+      const items = document.querySelectorAll("li.task-list-item");
+      expect(items).toHaveLength(2);
+      const boxes = screen.getAllByRole("checkbox");
+      expect(boxes).toHaveLength(2);
+      for (const box of boxes) expect(box).toBeDisabled();
+      expect(boxes[0]).toBeChecked();
+      expect(boxes[1]).not.toBeChecked();
+    });
+
+    it("strips list markers and styles the checkbox in the stylesheet", () => {
+      const rules = parseRules(tokensCss);
+      const declarationsFor = (selector: string) => {
+        const rule = rules.find((candidate) => selectorsOf(candidate).includes(selector));
+        if (!rule) throw new Error(`tokens.css has no rule for ${selector}`);
+        return declarationsOf(rule);
+      };
+
+      expect(declarationsFor(".markdown-body .contains-task-list").get("list-style")).toBe("none");
+      expect(declarationsFor('.markdown-body .task-list-item input[type="checkbox"]').get("accent-color")).toBe("var(--accent)");
+      // Code that wraps must actually fold, in the same place the base rule pins it to `pre`.
+      expect(declarationsFor(".markdown-body .code-block-wrap pre > code").get("white-space")).toBe("pre-wrap");
+    });
+  });
+
+  describe("images", () => {
+    it("names a stripped image in place of a picture, with its source on hover", () => {
+      render();
+
+      expect(document.querySelector("img")).toBeNull();
+      const note = screen.getByText("Image: Quarterly chart");
+      expect(note).toHaveClass("markdown-image-placeholder");
+      expect(note).toHaveAttribute("title", "Image not loaded: https://example.com/chart.png");
+      // Still inline with the sentence it sat in.
+      expect(note.parentElement).toHaveTextContent("Look: Image: Quarterly chart end.");
+    });
+
+    it("says only that there was an image when it had no alt text", () => {
+      render();
+
+      const note = document.querySelector(".markdown-image-placeholder");
+      expect(note).toHaveTextContent(/^Image$/);
+      expect(note).toHaveAttribute("title", "Image not loaded: https://example.com/chart.png");
+    });
+
+    it("never loads the image or leaks react-markdown's node prop", () => {
+      render();
+
+      const note = document.querySelector(".markdown-image-placeholder");
+      expect(note).not.toHaveAttribute("node");
+      expect(note).not.toHaveAttribute("src");
+      expect(document.querySelector("img")).toBeNull();
+    });
+
+    it("keeps a source the sanitizer removed out of the note", () => {
+      // javascript: and data: sources do not survive sanitizing, so there is
+      // no address to show -- and none must be invented.
+      render();
+
+      const note = document.querySelector(".markdown-image-placeholder");
+      expect(note).toHaveTextContent("Image: sneaky");
+      expect(note).toHaveAttribute("title", "Image not loaded");
+    });
+  });
 });
diff --git a/frontend/src/features/markdown/SafeMarkdown.tsx b/frontend/src/features/markdown/SafeMarkdown.tsx
index c99fdf10..2e5eec02 100644
--- a/frontend/src/features/markdown/SafeMarkdown.tsx
+++ b/frontend/src/features/markdown/SafeMarkdown.tsx
@@ -1,4 +1,4 @@
-import { isValidElement, memo, useState, type ComponentProps, type ReactNode } from "react";
+import { isValidElement, memo, useState, useSyncExternalStore, type ComponentProps, type ReactNode } from "react";
 import ReactMarkdown, { type ExtraProps } from "react-markdown";
 import rehypeHighlight from "rehype-highlight";
 import rehypeSanitize from "rehype-sanitize";
@@ -40,6 +40,49 @@ function languageLabel(className: string | undefined): string {
   return languageClass?.replace(/^language-/, "") || "code";
 }
 
+const CODE_WRAP_STORAGE_KEY = "cortex.codeWrap";
+
+/**
+ * Whether fenced code wraps instead of scrolling sideways. One preference for
+ * every block on the page, remembered across launches: a person who wants long
+ * lines wrapped wants it everywhere, not per block. Storage can be blocked or
+ * absent (private windows, tests), so the choice then simply lasts until the
+ * page is closed.
+ */
+const codeWrapPreference = (() => {
+  // Only set while storage refuses the write; otherwise storage is the truth.
+  let sessionOnly: boolean | null = null;
+  const listeners = new Set<() => void>();
+  return {
+    subscribe(listener: () => void) {
+      listeners.add(listener);
+      return () => { listeners.delete(listener); };
+    },
+    get(): boolean {
+      if (sessionOnly !== null) return sessionOnly;
+      try {
+        return window.localStorage.getItem(CODE_WRAP_STORAGE_KEY) === "1";
+      } catch {
+        return false;
+      }
+    },
+    set(next: boolean) {
+      try {
+        window.localStorage.setItem(CODE_WRAP_STORAGE_KEY, next ? "1" : "0");
+        sessionOnly = null;
+      } catch {
+        sessionOnly = next;
+      }
+      for (const listener of listeners) listener();
+    },
+  };
+})();
+
+function useCodeWrap(): [boolean, (next: boolean) => void] {
+  const wrap = useSyncExternalStore(codeWrapPreference.subscribe, codeWrapPreference.get, () => false);
+  return [wrap, codeWrapPreference.set];
+}
+
 /**
  * Wraps the whole fenced block so the toolbar is a *sibling* of the scrolling
  * 
, not a child of it. Nested inside, the toolbar inherits the code's
@@ -48,15 +91,19 @@ function languageLabel(className: string | undefined): string {
  * the way right. As a sibling it stays pinned while only the code scrolls.
  */
 function Pre({ children }: ComponentProps<"pre">) {
+  const [wrap, setWrap] = useCodeWrap();
   const code = isValidElement<{ className?: string; children?: ReactNode }>(children) ? children : null;
   if (!code) return 
{children}
; const language = languageLabel(code.props.className); const value = childrenToText(code.props.children).replace(/\n$/, ""); return ( -
+
{language} - + + + +
{children}
@@ -88,10 +135,28 @@ function Table({ children, node: _node, ...props }: ComponentProps<"table"> & Ex ); } +/** + * Images are never loaded: a remote image would tell whoever hosts it that this + * answer was read, which a local-first app must not do on the model's say-so. + * They are not dropped without a trace either -- what the author meant to show + * is named in place, and the address is on hover for anyone who wants to open + * it deliberately. `node` is kept out of the DOM, as for `Link`. + */ +function Image({ alt, src, node: _node }: ComponentProps<"img"> & ExtraProps) { + void _node; + const description = alt?.trim(); + const source = typeof src === "string" ? src : ""; + return ( + + {description ? `Image: ${description}` : "Image"} + + ); +} + const components = { a: Link, pre: Pre, - img: () => null, + img: Image, table: Table, }; diff --git a/frontend/src/styles/tokens.css b/frontend/src/styles/tokens.css index 252b5e30..8c2e0226 100644 --- a/frontend/src/styles/tokens.css +++ b/frontend/src/styles/tokens.css @@ -511,6 +511,18 @@ a { color: inherit; text-decoration: none; } .hljs-addition { color: var(--success); } .code-copy { min-height: 25px; border: 1px solid var(--line); border-radius: var(--radius-xs); padding: 0 9px; background: transparent; color: var(--text-muted); font-size: var(--text-2xs); font-weight: var(--weight-semi); } .code-copy:hover { border-color: var(--line-strong); background: var(--surface-hover); color: var(--text); } +.code-block-actions { display: inline-flex; align-items: center; gap: 6px; } +/* Underlined as well as tinted: forced-colors mode drops the tint, and a + pressed toggle must not look the same as an unpressed one there. */ +.code-copy[aria-pressed="true"] { border-color: var(--accent); background: var(--accent-soft); color: var(--accent); text-decoration: underline; text-underline-offset: 3px; } +/* The wrap toggle: long lines fold onto the next row instead of scrolling. */ +.markdown-body .code-block-wrap pre > code { white-space: pre-wrap; overflow-wrap: anywhere; } +/* A GFM task list carries its own checkbox, so the list marker goes. */ +.markdown-body .contains-task-list { padding-left: 0; list-style: none; } +.markdown-body .contains-task-list .contains-task-list { padding-left: 1.5em; } +.markdown-body .task-list-item input[type="checkbox"] { margin: 0 0.5em 0.15em 0; vertical-align: middle; accent-color: var(--accent); } +/* An image the answer asked for, named in place of a picture that is never loaded. */ +.markdown-image-placeholder { display: inline-block; max-width: 100%; padding: 0 6px; border: 1px dashed var(--line-strong); border-radius: var(--radius-xs); color: var(--text-muted); font-size: var(--text-sm); } /* One treatment open and closed: the summary row never changes shape, so expanding reasoning reveals content instead of re-drawing a box. */ .reasoning, .sources { width: 100%; margin: 10px 0 0; border: 0; border-radius: 0; background: transparent; color: var(--text-muted); font-size: var(--text-sm); } From 84ee9f607aee3052c9ac52095f02b59c45881fd8 Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Tue, 29 Sep 2026 04:00:54 -0400 Subject: [PATCH 03/13] perf(frontend): stable poll timers, a slow poll for a ready runtime, and back-off on failure Changing a poll's interval restarted its effect and ran the callback at once, so every switch between the fast and idle cadence cost an extra request; it now only re-arms the timer. The llama.cpp status was polled every two seconds for as long as a GGUF model was selected; it is now polled every two seconds only while the runtime is downloading, starting or stopping or a reply is generating, and every fifteen seconds otherwise. A poll that fails three times in a row now doubles its wait up to thirty seconds and returns to normal on the first success, and a callback that rejects is no longer left as an unhandled rejection. Co-Authored-By: Claude Sonnet 5.5 --- frontend/src/app/App.pollRender.test.tsx | 62 +++++++ frontend/src/app/App.test.tsx | 52 ++++++ frontend/src/app/App.tsx | 63 ++++--- frontend/src/app/runtimeAvailability.test.ts | 35 +++- frontend/src/app/runtimeAvailability.ts | 30 +++ frontend/src/hooks/useVisiblePolling.test.ts | 181 ++++++++++++++++++- frontend/src/hooks/useVisiblePolling.ts | 105 ++++++++++- 7 files changed, 491 insertions(+), 37 deletions(-) diff --git a/frontend/src/app/App.pollRender.test.tsx b/frontend/src/app/App.pollRender.test.tsx index f0594778..664a7732 100644 --- a/frontend/src/app/App.pollRender.test.tsx +++ b/frontend/src/app/App.pollRender.test.tsx @@ -126,4 +126,66 @@ describe("App polling", () => { await pollOnce(fetcher); expect(shell.renders).toBeGreaterThan(afterStatus); }); + + describe("cadence", () => { + /** Record every period the app asks the browser to poll at, while still running the real timers. */ + function recordIntervalPeriods() { + const periods: number[] = []; + const realSetInterval = window.setInterval.bind(window); + const spy = vi.spyOn(window, "setInterval").mockImplementation(((handler: TimerHandler, timeout?: number, ...args: unknown[]) => { + if (timeout !== undefined) periods.push(timeout); + return realSetInterval(handler, timeout, ...args); + }) as typeof window.setInterval); + return { periods, restore: () => spy.mockRestore() }; + } + + it("polls llama.cpp status slowly once the runtime is ready", async () => { + window.sessionStorage.setItem("cortex.session.token", "local-session"); + const state: Backend = { + tasks: [], + llamacpp: { state: "ready", binary_present: true, loaded_model: "gguf:demo.Q4_K_M.gguf", models_directory: "C:\\models" }, + }; + const fetcher = backend(state); + const { periods, restore } = recordIntervalPeriods(); + try { + render(); + expect(await screen.findByRole("heading", { name: "New thread" })).toBeVisible(); + await pollOnce(fetcher); + + // Ready: every fifteen seconds. Two seconds is for a runtime that is changing. + expect(periods).toContain(15_000); + expect(periods).not.toContain(2000); + } finally { + restore(); + } + }); + + it("watches the runtime closely while it starts, without an extra request for the switch", async () => { + window.sessionStorage.setItem("cortex.session.token", "local-session"); + const state: Backend = { + tasks: [], + llamacpp: { state: "ready", binary_present: true, loaded_model: "gguf:demo.Q4_K_M.gguf", models_directory: "C:\\models" }, + }; + const fetcher = backend(state); + const { periods, restore } = recordIntervalPeriods(); + try { + render(); + expect(await screen.findByRole("heading", { name: "New thread" })).toBeVisible(); + await pollOnce(fetcher); + expect(periods).not.toContain(2000); + + // The next poll brings news: the runtime is starting. The interval now + // changes on the very result that arrived; that must not cost a request. + state.llamacpp = { state: "starting", binary_present: true, models_directory: "C:\\models" }; + const systemBefore = callsTo(fetcher, "/system"); + await pollOnce(fetcher); + + expect(useModelStore.getState().llamacppStatus?.state).toBe("starting"); + expect(periods).toContain(2000); + expect(callsTo(fetcher, "/system")).toBe(systemBefore + 1); + } finally { + restore(); + } + }); + }); }); diff --git a/frontend/src/app/App.test.tsx b/frontend/src/app/App.test.tsx index aa0ec5c7..f29e9433 100644 --- a/frontend/src/app/App.test.tsx +++ b/frontend/src/app/App.test.tsx @@ -1038,6 +1038,58 @@ describe("App", () => { expect(screen.getByText("Create a larger staged image preview.")).toBeVisible(); }); + it("reports an approval that went through even when the refresh after it fails", async () => { + // The poll now sees a failed task refresh (so it can back off); a decision + // that was recorded must still be reported as such, not as a failure. + window.sessionStorage.setItem("cortex.session.token", "local-session"); + const json = (body: unknown, status = 200) => new Response(JSON.stringify(body), { + status, + headers: { "Content-Type": "application/json" }, + }); + const pendingTask = { + job_id: "approval-job", + profile: "artifact.extended.v1", + status: "queued", + sequence: 2, + phase: "approval", + message: "Approval required.", + approval_state: "pending", + approval_reason: "Create a larger staged image preview.", + approval_expires_at: "2026-07-21T18:30:00Z", + can_cancel: false, + created_at: "2026-07-21T18:00:00Z", + updated_at: "2026-07-21T18:00:01Z", + }; + let taskListAnswered = false; + const fetcher = vi.fn(async (input, init) => { + const url = String(input); + if (url.endsWith("/system")) return json({ status: "ok", preview: true, session_required: true, execution_preview_available: true, started_at: "2026-07-21T18:00:00Z" }); + if (url.endsWith("/chat-groups")) return json([]); + if (url.endsWith("/chats")) return json([]); + if (url.endsWith("/settings")) return json({ settings: { models: { chat: "model-a", title: null }, appearance: { theme: "dark" } } }); + if (url.endsWith("/memories")) return json({ memos: [] }); + if (url.endsWith("/models")) return json({ required_models: [], optional_models: [], installed_models: ["model-a"], connection: { success: true, status: "connected", message: "Ready" } }); + if (url.includes("/execution/tasks")) { + // Answers once so the approval card appears, then the backend goes away. + if (taskListAnswered) return json({ detail: "Backend restarting." }, 503); + taskListAnswered = true; + return json({ tasks: [pendingTask] }); + } + if (url.endsWith("/execution/approval-job/approval") && init?.method === "POST") { + return json({ job_id: "approval-job", status: "queued", sequence: 3 }); + } + return json({ detail: "Unexpected test route." }, 404); + }); + + render(); + const user = userEvent.setup(); + await user.click(await screen.findByRole("button", { name: "Allow background task approval-job once" })); + + expect(await screen.findByText("Background task approved once.")).toBeVisible(); + expect(screen.queryByText("Could not record the approval decision.")).toBeNull(); + expect(screen.queryByText("Backend restarting.")).toBeNull(); + }); + it("does not start a second execution-task poll while the first is pending", async () => { window.sessionStorage.setItem("cortex.session.token", "local-session"); const json = (body: unknown, status = 200) => new Response(JSON.stringify(body), { diff --git a/frontend/src/app/App.tsx b/frontend/src/app/App.tsx index 8069eecf..0e9794d3 100644 --- a/frontend/src/app/App.tsx +++ b/frontend/src/app/App.tsx @@ -30,7 +30,7 @@ import { useSettingsStore } from "../stores/useSettingsStore"; import { RouteBoundary } from "./ErrorBoundary"; import { lazyRoute } from "./lazyRoute"; import { useToast } from "./ToastProvider"; -import { resolveRuntimeAvailability } from "./runtimeAvailability"; +import { llamacppPollInterval, resolveRuntimeAvailability } from "./runtimeAvailability"; type Props = { api?: CortexApi }; @@ -410,22 +410,21 @@ function AuthenticatedWorkspace({ api, onSessionExpired }: { api: CortexApi; onS if (route.kind === "not-found") navigate("/chat/new", { replace: true }); }, [route.kind]); - const refreshExecutionTasks = useCallback((): Promise => { + // Rejects when the request fails, so the poll can see the failure and back + // off. `refreshExecutionTasks` below is for the callers that refresh after an + // action of their own, which has already succeeded or failed on its own terms. + const fetchExecutionTasks = useCallback((): Promise => { const inFlight = executionTaskRefreshRef.current; if (inFlight) return inFlight; // Defer the request one microtask so the in-flight marker is installed // before an unusually eager fetch implementation can resolve or throw. const refresh = Promise.resolve().then(async () => { - try { - const response = await api.executionTasks({ includeTerminal: true, limit: 20 }); - const signature = JSON.stringify(response.tasks); - if (signature === executionTasksSignatureRef.current) return; - executionTasksSignatureRef.current = signature; - setExecutionTasks(response.tasks); - } catch { - // A failed poll keeps the last list and the next tick retries. - } + const response = await api.executionTasks({ includeTerminal: true, limit: 20 }); + const signature = JSON.stringify(response.tasks); + if (signature === executionTasksSignatureRef.current) return; + executionTasksSignatureRef.current = signature; + setExecutionTasks(response.tasks); }); executionTaskRefreshRef.current = refresh; void refresh.then( @@ -438,18 +437,25 @@ function AuthenticatedWorkspace({ api, onSessionExpired }: { api: CortexApi; onS ); return refresh; }, [api]); + // A failed refresh keeps the last list; the caller's own outcome is what it reports. + const refreshExecutionTasks = useCallback( + (): Promise => fetchExecutionTasks().catch(() => undefined), + [fetchExecutionTasks], + ); // A second is the right cadence while something is actually running or // waiting on approval. With nothing in flight it was still a SQLite query // every second for the life of the app, so back off -- slowly enough that a - // task started elsewhere still appears promptly. + // task started elsewhere still appears promptly. A failing poll keeps the + // last list and slows down until the backend answers again. const hasActiveExecutionTask = executionTasks.some( (task) => !EXECUTION_TERMINAL_STATUSES.has(task.status), ); useVisiblePolling( - refreshExecutionTasks, + fetchExecutionTasks, hasActiveExecutionTask ? 1000 : 5000, Boolean(system?.execution_preview_available), + { backoff: true }, ); // Only poll the local llama.cpp runtime state while a GGUF model is @@ -458,21 +464,28 @@ function AuthenticatedWorkspace({ api, onSessionExpired }: { api: CortexApi; onS // what lets the composer show live "loaded" / "starting" state instead // of only ever reflecting whatever was true at the last full page load. const selectedModelIsGGUF = isGGUFModel(settings?.models?.chat ?? null); + // Closely while the runtime is downloading, starting or stopping or a reply + // is being generated, and every fifteen seconds otherwise: a ready runtime + // was being asked every two seconds for as long as a GGUF model was selected. + const generationActive = useChatStore((state) => state.generation.jobId !== null); const refreshLlamacppStatus = useCallback(async () => { - try { - const response = await api.system(); - const next = response.llamacpp ?? null; - // Every response parses to a new object, and the store notifies on any - // new reference, so an unchanged status would re-render the shell every - // two seconds for as long as a GGUF model is selected. - if (JSON.stringify(next) !== JSON.stringify(useModelStore.getState().llamacppStatus)) { - setLlamacppStatus(next); - } - } catch { - // Keep the last known status; the next tick retries. + // A failed request rejects, so the poll counts it and backs off; the last + // known status stays on screen meanwhile. + const response = await api.system(); + const next = response.llamacpp ?? null; + // Every response parses to a new object, and the store notifies on any + // new reference, so an unchanged status would re-render the shell on every + // tick for as long as a GGUF model is selected. + if (JSON.stringify(next) !== JSON.stringify(useModelStore.getState().llamacppStatus)) { + setLlamacppStatus(next); } }, [api, setLlamacppStatus]); - useVisiblePolling(refreshLlamacppStatus, 2000, selectedModelIsGGUF); + useVisiblePolling( + refreshLlamacppStatus, + llamacppPollInterval(liveLlamacppStatus, generationActive), + selectedModelIsGGUF, + { backoff: true }, + ); const visibleExecutionTasks = system?.execution_preview_available ? executionTasks.filter((task) => shouldShowExecutionTask(task, system.started_at)) diff --git a/frontend/src/app/runtimeAvailability.test.ts b/frontend/src/app/runtimeAvailability.test.ts index e7f0234b..66f391e0 100644 --- a/frontend/src/app/runtimeAvailability.test.ts +++ b/frontend/src/app/runtimeAvailability.test.ts @@ -1,6 +1,11 @@ import { describe, expect, it } from "vitest"; import type { LlamaCppRuntimeStatus } from "../../../contracts/cortex-api"; -import { resolveRuntimeAvailability } from "./runtimeAvailability"; +import { + LLAMACPP_ACTIVE_POLL_MS, + LLAMACPP_IDLE_POLL_MS, + llamacppPollInterval, + resolveRuntimeAvailability, +} from "./runtimeAvailability"; const RESTART_MESSAGE = "The local model runtime did not exit cleanly; restart Cortex before trying again."; @@ -63,3 +68,31 @@ describe("resolveRuntimeAvailability for a GGUF model", () => { ).toEqual({ ready: true, reason: null, message: null }); }); }); + +describe("llamacppPollInterval", () => { + it("watches a runtime that is downloading, starting, or stopping closely", () => { + for (const state of ["downloading_binary", "starting", "stopping"] as const) { + expect(llamacppPollInterval({ state }, false)).toBe(LLAMACPP_ACTIVE_POLL_MS); + } + expect(llamacppPollInterval({ state: "stopping", last_error: "" }, false)).toBe(LLAMACPP_ACTIVE_POLL_MS); + }); + + it("polls a ready, idle, or failed runtime slowly", () => { + for (const state of ["ready", "idle", "failed"] as const) { + expect(llamacppPollInterval({ state }, false)).toBe(LLAMACPP_IDLE_POLL_MS); + } + expect(LLAMACPP_IDLE_POLL_MS).toBeGreaterThan(LLAMACPP_ACTIVE_POLL_MS); + }); + + it("polls slowly before there is a status, and for the parked needs-a-restart state", () => { + expect(llamacppPollInterval(null, false)).toBe(LLAMACPP_IDLE_POLL_MS); + expect(llamacppPollInterval(undefined, false)).toBe(LLAMACPP_IDLE_POLL_MS); + expect(llamacppPollInterval({ state: "stopping", last_error: RESTART_MESSAGE }, false)).toBe(LLAMACPP_IDLE_POLL_MS); + }); + + it("watches closely while a generation is running, whatever the runtime last said", () => { + for (const status of [null, { state: "ready" }, { state: "idle" }] satisfies (LlamaCppRuntimeStatus | null)[]) { + expect(llamacppPollInterval(status, true)).toBe(LLAMACPP_ACTIVE_POLL_MS); + } + }); +}); diff --git a/frontend/src/app/runtimeAvailability.ts b/frontend/src/app/runtimeAvailability.ts index 706f0fca..1787ea05 100644 --- a/frontend/src/app/runtimeAvailability.ts +++ b/frontend/src/app/runtimeAvailability.ts @@ -59,3 +59,33 @@ export function resolveRuntimeAvailability({ } return { ready: true, reason: null, message: null }; } + +/** How often the runtime is asked for its state while it is changing, or while a reply depends on it. */ +export const LLAMACPP_ACTIVE_POLL_MS = 2000; +/** How often it is asked otherwise: a ready or idle runtime rarely changes on its own. */ +export const LLAMACPP_IDLE_POLL_MS = 15_000; + +/** + * How often to poll the local GGUF runtime. A download, a start, and a stop + * each move through states the user is waiting on, and a running generation is + * what makes the runtime start, so those are watched closely. A runtime that + * is ready, idle, or failed changes only when the user acts, which is far less + * often than every two seconds for as long as a GGUF model stays selected. + * "Stopping" with an error is the parked, needs-a-restart state: it does not + * resolve by itself, so it is not watched closely either. + */ +export function llamacppPollInterval( + status: LlamaCppRuntimeStatus | null | undefined, + generationActive: boolean, +): number { + if (generationActive) return LLAMACPP_ACTIVE_POLL_MS; + switch (status?.state) { + case "downloading_binary": + case "starting": + return LLAMACPP_ACTIVE_POLL_MS; + case "stopping": + return status.last_error ? LLAMACPP_IDLE_POLL_MS : LLAMACPP_ACTIVE_POLL_MS; + default: + return LLAMACPP_IDLE_POLL_MS; + } +} diff --git a/frontend/src/hooks/useVisiblePolling.test.ts b/frontend/src/hooks/useVisiblePolling.test.ts index e2909e7c..e5fecb86 100644 --- a/frontend/src/hooks/useVisiblePolling.test.ts +++ b/frontend/src/hooks/useVisiblePolling.test.ts @@ -1,6 +1,6 @@ import { renderHook } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { useVisiblePolling } from "./useVisiblePolling"; +import { pollDelay, useVisiblePolling } from "./useVisiblePolling"; function setVisibility(state: DocumentVisibilityState) { Object.defineProperty(document, "visibilityState", { value: state, configurable: true }); @@ -94,4 +94,183 @@ describe("useVisiblePolling", () => { expect(poll).not.toHaveBeenCalled(); }); + + it("changing intervalMs does not fire an immediate tick", () => { + const poll = vi.fn(); + const { rerender } = renderHook( + ({ intervalMs }) => useVisiblePolling(poll, intervalMs, true), + { initialProps: { intervalMs: 1000 } }, + ); + expect(poll).toHaveBeenCalledTimes(1); + + // The caller flips the interval on the very result a poll just delivered; + // asking again straight away would repeat a question that was just answered. + rerender({ intervalMs: 5000 }); + expect(poll).toHaveBeenCalledTimes(1); + + // The old cadence is gone and the new one is in force. + vi.advanceTimersByTime(4999); + expect(poll).toHaveBeenCalledTimes(1); + vi.advanceTimersByTime(1); + expect(poll).toHaveBeenCalledTimes(2); + vi.advanceTimersByTime(5000); + expect(poll).toHaveBeenCalledTimes(3); + }); + + it("speeds up as well as slows down without an extra request", () => { + const poll = vi.fn(); + const { rerender } = renderHook( + ({ intervalMs }) => useVisiblePolling(poll, intervalMs, true), + { initialProps: { intervalMs: 15_000 } }, + ); + poll.mockClear(); + + rerender({ intervalMs: 2000 }); + expect(poll).not.toHaveBeenCalled(); + vi.advanceTimersByTime(2000); + expect(poll).toHaveBeenCalledTimes(1); + }); + + it("polls straight away again when it is re-enabled", () => { + const poll = vi.fn(); + const { rerender } = renderHook( + ({ enabled }) => useVisiblePolling(poll, 1000, enabled), + { initialProps: { enabled: true } }, + ); + rerender({ enabled: false }); + poll.mockClear(); + + rerender({ enabled: true }); + expect(poll).toHaveBeenCalledTimes(1); + }); + + it("keeps its cadence when a poll never answers", async () => { + const poll = vi.fn(() => new Promise(() => undefined)); + renderHook(() => useVisiblePolling(poll, 1000, true, { backoff: true })); + + await vi.advanceTimersByTimeAsync(3000); + + // The timer, not the request, sets the pace: a hung request must not stop the polls after it. + expect(poll).toHaveBeenCalledTimes(4); + }); +}); + +describe("pollDelay", () => { + it("keeps the base interval for the first two failures", () => { + expect(pollDelay(2000, 0)).toBe(2000); + expect(pollDelay(2000, 1)).toBe(2000); + expect(pollDelay(2000, 2)).toBe(2000); + }); + + it("doubles from the third failure, up to thirty seconds", () => { + expect([3, 4, 5, 6, 7, 8].map((failures) => pollDelay(2000, failures))).toEqual([4000, 8000, 16_000, 30_000, 30_000, 30_000]); + }); + + it("never waits less than the base interval, even when that already exceeds the cap", () => { + expect(pollDelay(60_000, 0)).toBe(60_000); + expect(pollDelay(60_000, 9)).toBe(60_000); + }); +}); + +describe("useVisiblePolling error backoff", () => { + beforeEach(() => { + vi.useFakeTimers(); + Object.defineProperty(document, "visibilityState", { value: "visible", configurable: true }); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + /** Milliseconds since the hook mounted at which each call happened. */ + function recordCalls(outcome: (call: number) => void | Promise) { + const start = Date.now(); + const at: number[] = []; + const poll = vi.fn(() => { + at.push(Date.now() - start); + return outcome(at.length); + }); + return { poll, at }; + } + + it("doubles the wait after three failures in a row, up to thirty seconds", async () => { + const { poll, at } = recordCalls(() => Promise.reject(new Error("backend down"))); + renderHook(() => useVisiblePolling(poll, 1000, true, { backoff: true })); + + await vi.advanceTimersByTimeAsync(70_000); + + // Three at the normal pace, then 2s, 4s, 8s, 16s, and 30s from there on. + expect(at).toEqual([0, 1000, 2000, 4000, 8000, 16_000, 32_000, 62_000]); + }); + + it("returns to the normal interval after one success", async () => { + const { poll, at } = recordCalls((call) => (call <= 4 ? Promise.reject(new Error("blip")) : Promise.resolve())); + renderHook(() => useVisiblePolling(poll, 1000, true, { backoff: true })); + + // Calls 1-4 fail (at 0, 1s, 2s, 4s), so the wait grew to 4s; call 5 at 8s succeeds. + await vi.advanceTimersByTimeAsync(8000); + expect(at).toEqual([0, 1000, 2000, 4000, 8000]); + + await vi.advanceTimersByTimeAsync(3000); + expect(at).toEqual([0, 1000, 2000, 4000, 8000, 9000, 10_000, 11_000]); + }); + + it("counts a callback that throws synchronously as a failure", async () => { + const { poll, at } = recordCalls(() => { throw new Error("boom"); }); + renderHook(() => useVisiblePolling(poll, 1000, true, { backoff: true })); + + await vi.advanceTimersByTimeAsync(6000); + + expect(at).toEqual([0, 1000, 2000, 4000]); + }); + + it("does not slow a poll that never fails, or one that fails only twice", async () => { + const { poll, at } = recordCalls((call) => (call === 1 || call === 2 ? Promise.reject(new Error("blip")) : Promise.resolve())); + renderHook(() => useVisiblePolling(poll, 1000, true, { backoff: true })); + + await vi.advanceTimersByTimeAsync(5000); + + expect(at).toEqual([0, 1000, 2000, 3000, 4000, 5000]); + }); + + it("leaves the cadence alone without the option, and does not leave a rejection unhandled", async () => { + const { poll, at } = recordCalls(() => Promise.reject(new Error("backend down"))); + renderHook(() => useVisiblePolling(poll, 1000, true)); + + await vi.advanceTimersByTimeAsync(6000); + + // An unhandled rejection would fail this run, so reaching here also proves it was caught. + expect(at).toEqual([0, 1000, 2000, 3000, 4000, 5000, 6000]); + }); + + it("starts again at the normal pace after being switched off and on", async () => { + const { poll, at } = recordCalls(() => Promise.reject(new Error("backend down"))); + const { rerender } = renderHook( + ({ enabled }) => useVisiblePolling(poll, 1000, enabled, { backoff: true }), + { initialProps: { enabled: true } }, + ); + await vi.advanceTimersByTimeAsync(4000); + expect(at).toEqual([0, 1000, 2000, 4000]); + + rerender({ enabled: false }); + poll.mockClear(); + rerender({ enabled: true }); + await vi.advanceTimersByTimeAsync(2000); + + // A fresh start owes nothing to the failures before it: an immediate poll, + // then the normal one-second pace (a leftover four-second wait would give one). + expect(poll).toHaveBeenCalledTimes(3); + }); + + it("still refreshes at once when the window comes back, however long the wait has grown", async () => { + const { poll, at } = recordCalls(() => Promise.reject(new Error("backend down"))); + renderHook(() => useVisiblePolling(poll, 1000, true, { backoff: true })); + await vi.advanceTimersByTimeAsync(20_000); + const before = at.length; + + setVisibility("hidden"); + setVisibility("visible"); + + expect(at.length).toBe(before + 1); + }); }); diff --git a/frontend/src/hooks/useVisiblePolling.ts b/frontend/src/hooks/useVisiblePolling.ts index 601d414c..28a7e2bc 100644 --- a/frontend/src/hooks/useVisiblePolling.ts +++ b/frontend/src/hooks/useVisiblePolling.ts @@ -1,5 +1,35 @@ import { useEffect, useRef } from "react"; +export type VisiblePollingOptions = { + /** + * Slow down while `callback` keeps failing. Once it has failed three times in + * a row (it throws, or its promise rejects), each further failure doubles the + * wait, up to thirty seconds; one success returns to the normal interval. A + * backend that is down or restarting is not asked every second, and the + * return of the window to the foreground still checks at once. + */ + backoff?: boolean; +}; + +/** Failures in a row that are tolerated before the wait starts to grow. */ +const BACKOFF_AFTER_FAILURES = 3; +const MAX_BACKOFF_MS = 30_000; + +/** + * How long to wait before the next tick, given the base interval and how many + * polls in a row have failed. Never shorter than the base interval, even when + * that is already longer than the cap. + */ +export function pollDelay(intervalMs: number, consecutiveFailures: number): number { + if (consecutiveFailures < BACKOFF_AFTER_FAILURES) return intervalMs; + const doublings = consecutiveFailures - BACKOFF_AFTER_FAILURES + 1; + return Math.min(intervalMs * 2 ** doublings, Math.max(MAX_BACKOFF_MS, intervalMs)); +} + +function isPromiseLike(value: unknown): value is PromiseLike { + return typeof value === "object" && value !== null && typeof (value as { then?: unknown }).then === "function"; +} + /** * Poll `callback` on an interval, but only while the window is actually being * looked at. @@ -11,15 +41,23 @@ import { useEffect, useRef } from "react"; * until the next tick. * * `enabled` is the other half: a poll that has nothing to watch should not run - * at all. Passing false stops the interval and clears the listener. + * at all. Passing false stops the timer and clears the listener, and turning it + * on runs the callback once straight away. + * + * Changing `intervalMs` only re-arms the timer with the new period. It does not + * run the callback: callers switch the interval on the very result a poll just + * delivered (fast while something is running, slow when idle), and an extra + * request per switch would answer a question that was just answered. * * The callback is held in a ref, so an inline arrow function does not restart - * the interval on every render. Only `enabled` and `intervalMs` do that. + * the timer on every render. A callback that throws or rejects counts as a + * failed poll (see `backoff`); it is never left as an unhandled rejection. */ export function useVisiblePolling( callback: () => void | Promise, intervalMs: number, enabled: boolean, + { backoff = false }: VisiblePollingOptions = {}, ): void { const callbackRef = useRef(callback); // Assigned in an effect rather than during render: a ref write in the render @@ -29,25 +67,72 @@ export function useVisiblePolling( callbackRef.current = callback; }); + // Whether the previous run of the effect below had polling on. It is how the + // effect tells "just turned on" (poll now) from "the interval changed" (do not). + const wasEnabledRef = useRef(false); + const failuresRef = useRef(0); + useEffect(() => { - if (!enabled) return undefined; + const justEnabled = enabled && !wasEnabledRef.current; + wasEnabledRef.current = enabled; + if (!enabled) { + failuresRef.current = 0; + return undefined; + } + if (justEnabled) failuresRef.current = 0; - const run = () => { - void callbackRef.current(); - }; + let stopped = false; + let timer: number | undefined; + let armedDelay = intervalMs; - run(); - const timer = window.setInterval(() => { + const delay = () => (backoff ? pollDelay(intervalMs, failuresRef.current) : intervalMs); + // The timer, not the request, sets the cadence: a poll that never answers + // cannot stop the ones after it. + const tick = () => { if (document.visibilityState === "visible") run(); - }, intervalMs); + }; + const arm = () => { + armedDelay = delay(); + timer = window.setInterval(tick, armedDelay); + }; + // A finished poll can change how long to wait (the third failure in a row, + // or the first success after some). The running timer has the old period, + // so it is replaced rather than left to fire early or late. + const settled = (failed: boolean) => { + if (stopped || !backoff) return; + failuresRef.current = failed ? failuresRef.current + 1 : 0; + if (delay() !== armedDelay) { + window.clearInterval(timer); + arm(); + } + }; + const run = () => { + let result: void | Promise; + try { + result = callbackRef.current(); + } catch { + settled(true); + return; + } + if (isPromiseLike(result)) { + void Promise.resolve(result).then(() => settled(false), () => settled(true)); + } else { + settled(false); + } + }; const handleVisibilityChange = () => { if (document.visibilityState === "visible") run(); }; + + // Armed before the first run: a finished run may need to replace the timer. + arm(); + if (justEnabled) run(); document.addEventListener("visibilitychange", handleVisibilityChange); return () => { + stopped = true; window.clearInterval(timer); document.removeEventListener("visibilitychange", handleVisibilityChange); }; - }, [enabled, intervalMs]); + }, [enabled, intervalMs, backoff]); } From cc60ab97e4ed2c652230c02f0f78ad2f924bd748 Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Tue, 29 Sep 2026 04:04:57 -0400 Subject: [PATCH 04/13] fix(chat): one bottom band for both transcripts, instant follow, and reduced motion for scripted scrolling The plain transcript treated the last 80px as the bottom while Virtuoso used its 4px default, so following a reply behaved differently either side of forty messages. Both now share one constant. Following streamed output assigned scrollTop on every frame under a stylesheet that smooth-scrolls, restarting an animation each time; it now scrolls instantly and only the reader's own Jump to latest animates. Neither the smooth jump nor Virtuoso's smooth follow honoured prefers-reduced-motion, which only the stylesheet did; a small useMediaQuery hook now lets both fall back to instant scrolling. Co-Authored-By: Claude Sonnet 5.5 --- .../chat/ChatPage.renderCost.test.tsx | 24 +++ frontend/src/features/chat/ChatPage.tsx | 3 +- .../src/features/chat/MessageList.test.tsx | 173 +++++++++++++++++- frontend/src/features/chat/MessageList.tsx | 43 ++++- frontend/src/hooks/useMediaQuery.test.ts | 110 +++++++++++ frontend/src/hooks/useMediaQuery.ts | 40 ++++ 6 files changed, 383 insertions(+), 10 deletions(-) create mode 100644 frontend/src/hooks/useMediaQuery.test.ts create mode 100644 frontend/src/hooks/useMediaQuery.ts diff --git a/frontend/src/features/chat/ChatPage.renderCost.test.tsx b/frontend/src/features/chat/ChatPage.renderCost.test.tsx index d27b17c7..d7870d71 100644 --- a/frontend/src/features/chat/ChatPage.renderCost.test.tsx +++ b/frontend/src/features/chat/ChatPage.renderCost.test.tsx @@ -173,6 +173,30 @@ describe("ChatPage while a reply streams", () => { expect(screen.queryByRole("button", { name: "Jump to latest" })).toBeNull(); }); + it("follows streamed output instantly, and animates only the jump the reader asks for", async () => { + // `.transcript` has `scroll-behavior: smooth`, so a follow that does not say + // "instant" restarts an animation on every streamed frame. + const scrollTo = vi.fn(); + Object.defineProperty(HTMLElement.prototype, "scrollTo", { configurable: true, value: scrollTo }); + try { + await openChatAndStartStreaming(); + stream("tok "); + await waitFor(() => expect(scrollTo).toHaveBeenCalled()); + for (const [options] of scrollTo.mock.calls) expect(options).toEqual({ top: 1000, behavior: "instant" }); + + // Scrolled away: the next frame offers the jump instead of following. + fireEvent.scroll(document.querySelector(".transcript")!); + stream("more "); + scrollTo.mockClear(); + fireEvent.click(await screen.findByRole("button", { name: "Jump to latest" })); + + expect(scrollTo).toHaveBeenCalledTimes(1); + expect(scrollTo).toHaveBeenCalledWith({ top: 1000, behavior: "smooth" }); + } finally { + Reflect.deleteProperty(HTMLElement.prototype, "scrollTo"); + } + }); + it("does not react to another thread's generation", async () => { renderChat(); await waitFor(() => expect(parsesOf(PERSISTED[0])).toBeGreaterThan(0)); diff --git a/frontend/src/features/chat/ChatPage.tsx b/frontend/src/features/chat/ChatPage.tsx index 678f8c66..e8ffdef8 100644 --- a/frontend/src/features/chat/ChatPage.tsx +++ b/frontend/src/features/chat/ChatPage.tsx @@ -872,7 +872,8 @@ export function ChatPage({ }; const jumpToLatest = () => { - messageListRef.current?.scrollToBottom(); + // The one scroll the reader asked for, so the one that may animate. + messageListRef.current?.scrollToBottom("smooth"); isNearTranscriptEnd.current = true; setShowJumpToLatest(false); }; diff --git a/frontend/src/features/chat/MessageList.test.tsx b/frontend/src/features/chat/MessageList.test.tsx index 22450cc1..cb198eb5 100644 --- a/frontend/src/features/chat/MessageList.test.tsx +++ b/frontend/src/features/chat/MessageList.test.tsx @@ -1,6 +1,6 @@ import { createRef, forwardRef, useEffect, useImperativeHandle } from "react"; import { fireEvent, render, screen, waitFor } from "@testing-library/react"; -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi, type Mock } from "vitest"; import type { VirtuosoHandle, VirtuosoProps } from "react-virtuoso"; import type { ChatMessage } from "../../../../contracts/cortex-api"; import { MessageList, type MessageListHandle } from "./MessageList"; @@ -324,3 +324,174 @@ describe("MessageList", () => { expect(transcript.scrollTop).toBe(900); }); }); + +describe("MessageList scrolling", () => { + type Captured = { props: VirtuosoProps | null; scrollTo: Mock<(location: ScrollToOptions) => void> }; + + /** The list with Virtuoso replaced by a stand-in that records the props it was given and the scrolls it was asked for. */ + async function loadWithVirtuoso() { + vi.resetModules(); + const captured: Captured = { props: null, scrollTo: vi.fn<(location: ScrollToOptions) => void>() }; + vi.doMock("react-virtuoso", () => ({ + Virtuoso: forwardRef>(function MockVirtuoso(props, ref) { + captured.props = props; + useImperativeHandle(ref, () => ({ + scrollTo: captured.scrollTo, + scrollToIndex: () => {}, + scrollBy: () => {}, + autoscrollToBottom: () => {}, + scrollIntoView: () => {}, + getState: () => { throw new Error("not implemented in this test double"); }, + })); + return
; + }), + })); + const { MessageList: MockedMessageList } = await import("./MessageList"); + return { MockedMessageList, captured }; + } + + function listProps(messages: ChatMessage[], isStreaming = false) { + return { + messages, + isStreaming, + finalAssistantId: null, + busy: false, + forkingMessageId: null, + onRegenerate: vi.fn(), + onFork: vi.fn(), + onNearEndChange: vi.fn(), + }; + } + + function preferReducedMotion(reduce: boolean) { + Object.defineProperty(window, "matchMedia", { + configurable: true, + writable: true, + value: vi.fn((query: string) => ({ + matches: reduce && query.includes("prefers-reduced-motion"), + media: query, + onchange: null, + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + addListener: vi.fn(), + removeListener: vi.fn(), + dispatchEvent: vi.fn(), + })), + }); + } + + afterEach(() => { + Reflect.deleteProperty(window, "matchMedia"); + vi.doUnmock("react-virtuoso"); + vi.resetModules(); + }); + + it("uses the same bottom band on the virtualized transcript as on the plain one", async () => { + const { MockedMessageList, captured } = await loadWithVirtuoso(); + render(); + + // Virtuoso's own default is 4px; a reader 60px from the end was "at the + // bottom" on a short chat and "away" on a long one. + expect(captured.props?.atBottomThreshold).toBe(80); + + const onNearEndChange = vi.fn(); + vi.resetModules(); + const { MessageList: PlainList } = await import("./MessageList"); + // Below the threshold the plain path is used; find the same band there. + render(); + const transcript = document.querySelector(".transcript") as HTMLDivElement; + Object.defineProperty(transcript, "scrollHeight", { value: 1000, configurable: true }); + Object.defineProperty(transcript, "clientHeight", { value: 400, configurable: true }); + + transcript.scrollTop = 521; // 79px from the end + fireEvent.scroll(transcript); + expect(onNearEndChange).toHaveBeenLastCalledWith(true); + transcript.scrollTop = 519; // 81px from the end + fireEvent.scroll(transcript); + expect(onNearEndChange).toHaveBeenLastCalledWith(false); + }); + + it("follows streamed output smoothly on the virtualized transcript, and not at all when idle", async () => { + const { MockedMessageList, captured } = await loadWithVirtuoso(); + const { rerender } = render(); + expect(captured.props?.followOutput).toBe("smooth"); + + rerender(); + expect(captured.props?.followOutput).toBe(false); + }); + + it("does not animate the follow when the person asked for reduced motion", async () => { + preferReducedMotion(true); + const { MockedMessageList, captured } = await loadWithVirtuoso(); + render(); + + // Virtuoso animates in script, which the stylesheet's reduced-motion rule cannot reach. + expect(captured.props?.followOutput).toBe("auto"); + }); + + it("scrolls the plain transcript instantly by default, so a streamed frame never restarts an animation", async () => { + const { MockedMessageList } = await loadWithVirtuoso(); + const ref = createRef(); + render(); + const transcript = document.querySelector(".transcript") as HTMLDivElement; + Object.defineProperty(transcript, "scrollHeight", { value: 900, configurable: true }); + const scrollTo = vi.fn(); + transcript.scrollTo = scrollTo; + + ref.current?.scrollToBottom(); + + // .transcript has `scroll-behavior: smooth`; only an explicit behavior overrides it. + expect(scrollTo).toHaveBeenCalledTimes(1); + expect(scrollTo).toHaveBeenCalledWith({ top: 900, behavior: "instant" }); + }); + + it("animates the explicit jump, unless reduced motion was asked for", async () => { + const { MockedMessageList } = await loadWithVirtuoso(); + const ref = createRef(); + render(); + const transcript = document.querySelector(".transcript") as HTMLDivElement; + Object.defineProperty(transcript, "scrollHeight", { value: 900, configurable: true }); + const scrollTo = vi.fn(); + transcript.scrollTo = scrollTo; + + ref.current?.scrollToBottom("smooth"); + expect(scrollTo).toHaveBeenLastCalledWith({ top: 900, behavior: "smooth" }); + }); + + it("makes the explicit jump instant when reduced motion was asked for", async () => { + preferReducedMotion(true); + const { MockedMessageList } = await loadWithVirtuoso(); + const ref = createRef(); + render(); + const transcript = document.querySelector(".transcript") as HTMLDivElement; + Object.defineProperty(transcript, "scrollHeight", { value: 900, configurable: true }); + const scrollTo = vi.fn(); + transcript.scrollTo = scrollTo; + + ref.current?.scrollToBottom("smooth"); + + expect(scrollTo).toHaveBeenCalledWith({ top: 900, behavior: "instant" }); + }); + + it("gives the virtualized transcript the same instant, smooth, and reduced-motion rules", async () => { + const { MockedMessageList, captured } = await loadWithVirtuoso(); + const ref = createRef(); + render(); + + ref.current?.scrollToBottom(); + expect(captured.scrollTo).toHaveBeenLastCalledWith({ top: Number.MAX_SAFE_INTEGER, behavior: "instant" }); + ref.current?.scrollToBottom("smooth"); + expect(captured.scrollTo).toHaveBeenLastCalledWith({ top: Number.MAX_SAFE_INTEGER, behavior: "smooth" }); + }); + + it("makes the virtualized jump instant under reduced motion", async () => { + preferReducedMotion(true); + const { MockedMessageList, captured } = await loadWithVirtuoso(); + const ref = createRef(); + render(); + + ref.current?.scrollToBottom("smooth"); + + expect(captured.scrollTo).toHaveBeenCalledWith({ top: Number.MAX_SAFE_INTEGER, behavior: "instant" }); + }); +}); diff --git a/frontend/src/features/chat/MessageList.tsx b/frontend/src/features/chat/MessageList.tsx index e774c50f..d5af54f8 100644 --- a/frontend/src/features/chat/MessageList.tsx +++ b/frontend/src/features/chat/MessageList.tsx @@ -1,12 +1,27 @@ import { forwardRef, useCallback, useImperativeHandle, useLayoutEffect, useRef, type ReactNode } from "react"; import { Virtuoso, type VirtuosoHandle } from "react-virtuoso"; import type { ChatMessage } from "../../../../contracts/cortex-api"; +import { usePrefersReducedMotion } from "../../hooks/useMediaQuery"; import { MessageCard } from "./MessageCard"; const VIRTUALIZE_THRESHOLD = 40; +/** + * How close to the end (in pixels) still counts as "at the bottom". Both the + * plain transcript and Virtuoso use it, so a reader is followed -- or not -- + * the same way on either side of the virtualization threshold. + */ +const NEAR_END_PX = 80; + export type MessageListHandle = { - scrollToBottom: () => void; + /** + * Scroll to the end. `"instant"` (the default) is for following streamed + * output, which asks on every frame: a smooth scroll restarts its animation + * each time, so the view lags behind the text and never settles. `"smooth"` + * is for the reader's own "Jump to latest", and becomes instant when the + * person has asked for reduced motion. + */ + scrollToBottom: (behavior?: "instant" | "smooth") => void; }; type Props = { @@ -40,6 +55,9 @@ export const MessageList = forwardRef(function Message const plainRef = useRef(null); const virtuosoRef = useRef(null); const virtualized = messages.length >= VIRTUALIZE_THRESHOLD; + // Scrolling started from script is not covered by the stylesheet's + // reduced-motion rule, so it is decided here. + const reducedMotion = usePrefersReducedMotion(); const wasVirtualizedRef = useRef(virtualized); const lastPlainScrollTopRef = useRef(0); const lastPlainNearEndRef = useRef(true); @@ -84,19 +102,25 @@ export const MessageList = forwardRef(function Message const Footer = useCallback(() => <>{trailingRef.current}, []); useImperativeHandle(ref, () => ({ - scrollToBottom: () => { + scrollToBottom: (requested = "instant") => { + const behavior = reducedMotion ? "instant" : requested; if (virtualized) { // Not scrollToIndex(last message): the in-flight streaming bubble // lives in the Footer slot, *below* the final item, so targeting the // last item leaves the answer being typed out of view for the whole // response. Scroll the virtualized scroller to its true bottom, which - // is what the plain path's scrollTop = scrollHeight already does. - virtuosoRef.current?.scrollTo({ top: Number.MAX_SAFE_INTEGER, behavior: "auto" }); + // is what the plain path does with the scroll height. + virtuosoRef.current?.scrollTo({ top: Number.MAX_SAFE_INTEGER, behavior }); } else if (plainRef.current) { - plainRef.current.scrollTop = plainRef.current.scrollHeight; + const node = plainRef.current; + // An explicit behavior wins over the stylesheet's `scroll-behavior: + // smooth` on .transcript; assigning scrollTop would not, and would + // start a fresh animation on every streamed frame. + if (typeof node.scrollTo === "function") node.scrollTo({ top: node.scrollHeight, behavior }); + else node.scrollTop = node.scrollHeight; } }, - }), [virtualized]); + }), [virtualized, reducedMotion]); const renderCard = (message: ChatMessage, index: number) => ( (function Message const node = plainRef.current; if (!node) return; lastPlainScrollTopRef.current = node.scrollTop; - lastPlainNearEndRef.current = node.scrollHeight - node.scrollTop - node.clientHeight < 80; + lastPlainNearEndRef.current = node.scrollHeight - node.scrollTop - node.clientHeight < NEAR_END_PX; onNearEndChange(lastPlainNearEndRef.current); }} > @@ -140,9 +164,12 @@ export const MessageList = forwardRef(function Message className="transcript transcript-virtual" data={messages} computeItemKey={(index, message) => message.id ?? `${message.role}-${index}`} - followOutput={isStreaming ? "smooth" : false} + // Virtuoso animates this itself, in script, so the stylesheet's + // reduced-motion rule does not reach it. + followOutput={isStreaming ? (reducedMotion ? "auto" : "smooth") : false} initialTopMostItemIndex={lastPlainNearEndRef.current ? messages.length - 1 : 0} alignToBottom + atBottomThreshold={NEAR_END_PX} atBottomStateChange={onNearEndChange} itemContent={(index, message) => renderCard(message, index)} components={{ Footer }} diff --git a/frontend/src/hooks/useMediaQuery.test.ts b/frontend/src/hooks/useMediaQuery.test.ts new file mode 100644 index 00000000..14ea885f --- /dev/null +++ b/frontend/src/hooks/useMediaQuery.test.ts @@ -0,0 +1,110 @@ +import { act, renderHook } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { REDUCED_MOTION_QUERY, useMediaQuery, usePrefersReducedMotion } from "./useMediaQuery"; + +type Listener = () => void; + +/** A controllable stand-in for one media query; `flip` changes the answer and notifies listeners. */ +function installMatchMedia(initial: boolean, { modern = true }: { modern?: boolean } = {}) { + let matches = initial; + const listeners = new Set(); + const list = { + get matches() { return matches; }, + media: "", + onchange: null, + addEventListener: modern ? vi.fn((_type: string, listener: Listener) => { listeners.add(listener); }) : undefined, + removeEventListener: modern ? vi.fn((_type: string, listener: Listener) => { listeners.delete(listener); }) : undefined, + addListener: vi.fn((listener: Listener) => { listeners.add(listener); }), + removeListener: vi.fn((listener: Listener) => { listeners.delete(listener); }), + dispatchEvent: vi.fn(), + }; + const matchMedia = vi.fn((query: string) => ({ ...list, media: query, get matches() { return matches; } })); + Object.defineProperty(window, "matchMedia", { configurable: true, writable: true, value: matchMedia }); + return { + matchMedia, + list, + listeners, + flip(next: boolean) { + matches = next; + for (const listener of [...listeners]) listener(); + }, + }; +} + +describe("useMediaQuery", () => { + afterEach(() => { + Reflect.deleteProperty(window, "matchMedia"); + }); + + it("reports the current answer and follows changes", () => { + const media = installMatchMedia(false); + const { result } = renderHook(() => useMediaQuery("(min-width: 900px)")); + expect(result.current).toBe(false); + expect(media.matchMedia).toHaveBeenCalledWith("(min-width: 900px)"); + + act(() => media.flip(true)); + expect(result.current).toBe(true); + + act(() => media.flip(false)); + expect(result.current).toBe(false); + }); + + it("stops listening when the component goes away", () => { + const media = installMatchMedia(false); + const { unmount } = renderHook(() => useMediaQuery("(min-width: 900px)")); + expect(media.listeners.size).toBe(1); + + unmount(); + + expect(media.listeners.size).toBe(0); + }); + + it("does not re-subscribe on every render", () => { + const media = installMatchMedia(false); + const { rerender } = renderHook(() => useMediaQuery("(min-width: 900px)")); + const subscriptions = media.list.addEventListener?.mock.calls.length; + + rerender(); + rerender(); + + expect(media.list.addEventListener?.mock.calls.length).toBe(subscriptions); + }); + + it("falls back to the deprecated listener pair on older webviews", () => { + const media = installMatchMedia(false, { modern: false }); + const { result, unmount } = renderHook(() => useMediaQuery("(min-width: 900px)")); + + act(() => media.flip(true)); + expect(result.current).toBe(true); + + unmount(); + expect(media.list.removeListener).toHaveBeenCalledTimes(1); + }); + + it("matches nothing where matchMedia does not exist", () => { + expect(typeof window.matchMedia).not.toBe("function"); + const { result } = renderHook(() => useMediaQuery("(min-width: 900px)")); + expect(result.current).toBe(false); + }); +}); + +describe("usePrefersReducedMotion", () => { + afterEach(() => { + Reflect.deleteProperty(window, "matchMedia"); + }); + + it("asks for the reduced-motion preference", () => { + const media = installMatchMedia(true); + const { result } = renderHook(() => usePrefersReducedMotion()); + + expect(media.matchMedia).toHaveBeenCalledWith(REDUCED_MOTION_QUERY); + expect(REDUCED_MOTION_QUERY).toBe("(prefers-reduced-motion: reduce)"); + expect(result.current).toBe(true); + }); + + it("is false when the person has not asked for it", () => { + installMatchMedia(false); + const { result } = renderHook(() => usePrefersReducedMotion()); + expect(result.current).toBe(false); + }); +}); diff --git a/frontend/src/hooks/useMediaQuery.ts b/frontend/src/hooks/useMediaQuery.ts new file mode 100644 index 00000000..8df8157d --- /dev/null +++ b/frontend/src/hooks/useMediaQuery.ts @@ -0,0 +1,40 @@ +import { useCallback, useSyncExternalStore } from "react"; + +export const REDUCED_MOTION_QUERY = "(prefers-reduced-motion: reduce)"; + +function mediaQueryList(query: string): MediaQueryList | undefined { + return typeof window !== "undefined" && typeof window.matchMedia === "function" + ? window.matchMedia(query) + : undefined; +} + +/** + * Whether a CSS media query currently matches, kept up to date as it changes. + * + * Where `matchMedia` does not exist (tests, some embedded webviews) nothing + * matches, which for the queries this app asks -- "reduce motion", for one -- + * means the ordinary behaviour. + */ +export function useMediaQuery(query: string): boolean { + const subscribe = useCallback((onChange: () => void) => { + const list = mediaQueryList(query); + if (!list) return () => undefined; + if (typeof list.addEventListener === "function") { + list.addEventListener("change", onChange); + return () => list.removeEventListener("change", onChange); + } + // Older WebView builds only have the deprecated pair. + list.addListener?.(onChange); + return () => list.removeListener?.(onChange); + }, [query]); + return useSyncExternalStore(subscribe, () => mediaQueryList(query)?.matches ?? false, () => false); +} + +/** + * True when the person has asked their system to reduce motion. The stylesheet + * already honours that for CSS animations and transitions; this is for motion + * that script starts, such as smooth programmatic scrolling, which CSS cannot reach. + */ +export function usePrefersReducedMotion(): boolean { + return useMediaQuery(REDUCED_MOTION_QUERY); +} From a78a24f914ace1580603ec61d13c9e348633dcb1 Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Tue, 29 Sep 2026 04:12:14 -0400 Subject: [PATCH 05/13] fix(a11y): Escape stops a response from anywhere on the page, and shortcuts ignore AltGr, repeats and IME Escape only stopped a response while the composer had focus, although the shortcuts dialog advertises it without that condition. A page-level handler now stops it while a response is running, leaving Escape to open dialogs, menus and listboxes, to other text fields, and to anything that already handled the key. useHotkey re-registered its listener on every render, fired again for every auto-repeat, fired during IME composition, and matched AltGr (reported as Ctrl+Alt) as Ctrl, so AltGr+K opened the palette while typing and holding Ctrl+K made it flicker. It now holds the handler in a ref, ignores repeats and composition, and requires Alt to be up for a modifier combo. Co-Authored-By: Claude Sonnet 5.5 --- frontend/src/features/chat/ChatPage.tsx | 9 ++ frontend/src/hooks/useHotkey.test.ts | 96 ++++++++++++++++ frontend/src/hooks/useHotkey.ts | 29 ++++- frontend/src/hooks/usePageEscape.test.ts | 140 +++++++++++++++++++++++ frontend/src/hooks/usePageEscape.ts | 50 ++++++++ 5 files changed, 319 insertions(+), 5 deletions(-) create mode 100644 frontend/src/hooks/usePageEscape.test.ts create mode 100644 frontend/src/hooks/usePageEscape.ts diff --git a/frontend/src/features/chat/ChatPage.tsx b/frontend/src/features/chat/ChatPage.tsx index e8ffdef8..0877c654 100644 --- a/frontend/src/features/chat/ChatPage.tsx +++ b/frontend/src/features/chat/ChatPage.tsx @@ -8,6 +8,7 @@ import { composerAttachmentKey, composerDraftKey, readComposerAttachments, readC import { useShallow } from "zustand/react/shallow"; import { useFileDropZone } from "../../hooks/useFileDropZone"; import { trackGeneration } from "../../hooks/useGenerationStream"; +import { usePageEscape } from "../../hooks/usePageEscape"; import { NEW_THREAD_OPTIONS_KEY, useChatStore } from "../../stores/useChatStore"; import { useSettingsStore } from "../../stores/useSettingsStore"; import { useUiStore } from "../../stores/useUiStore"; @@ -637,6 +638,14 @@ export function ChatPage({ } }; + // Escape stops a response from anywhere on the page, not only from the + // composer -- which is where focus is not, for a person who is reading the + // answer. Dialogs, menus and other text fields keep their own Escape, and the + // composer's own handling (which prevents default) is not repeated here. + // Only while a response is actually running: Stopping and Finishing have + // nothing left to stop. + usePageEscape(() => { void cancel(); }, composerPhase === "generating"); + const retryLastPrompt = async (): Promise => { if (!lastPrompt) return false; // A failed send deliberately leaves the text in the composer ("Your diff --git a/frontend/src/hooks/useHotkey.test.ts b/frontend/src/hooks/useHotkey.test.ts index 90500023..9aaea2d9 100644 --- a/frontend/src/hooks/useHotkey.test.ts +++ b/frontend/src/hooks/useHotkey.test.ts @@ -55,4 +55,100 @@ describe("useHotkey", () => { dispatchKey("k", { ctrlKey: true }); expect(handler).not.toHaveBeenCalled(); }); + + it("ignores auto-repeat, so holding Ctrl+K does not flicker the palette", () => { + const handler = vi.fn(); + renderHook(() => useHotkey("k", true, handler)); + + dispatchKey("k", { ctrlKey: true }); + dispatchKey("k", { ctrlKey: true, repeat: true }); + dispatchKey("k", { ctrlKey: true, repeat: true }); + + expect(handler).toHaveBeenCalledTimes(1); + }); + + it("does not fire in the middle of an IME composition", () => { + const handler = vi.fn(); + renderHook(() => useHotkey("k", true, handler)); + + dispatchKey("k", { ctrlKey: true, isComposing: true }); + + expect(handler).not.toHaveBeenCalled(); + }); + + it("does not take AltGr (reported as Ctrl+Alt) for Ctrl", () => { + const handler = vi.fn(); + renderHook(() => useHotkey("k", true, handler)); + + // AltGr+K types a character on layouts that have one there. + const altGr = dispatchKey("k", { ctrlKey: true, altKey: true }); + expect(handler).not.toHaveBeenCalled(); + expect(altGr.defaultPrevented).toBe(false); + + dispatchKey("k", { metaKey: true, altKey: true }); + expect(handler).not.toHaveBeenCalled(); + + // Plain Ctrl+K, and Cmd+K, still work. + dispatchKey("k", { ctrlKey: true }); + dispatchKey("k", { metaKey: true }); + expect(handler).toHaveBeenCalledTimes(2); + }); + + it("ignores a plain key while Alt is held", () => { + const handler = vi.fn(); + renderHook(() => useHotkey("?", false, handler)); + + dispatchKey("?", { altKey: true }); + + expect(handler).not.toHaveBeenCalled(); + }); + + it("keeps one listener however often the handler changes, and calls the latest handler", () => { + const added: string[] = []; + const removed: string[] = []; + // Record the calls but still perform them, so the hook keeps working. + const realAdd = window.addEventListener.bind(window); + const realRemove = window.removeEventListener.bind(window); + const trackAdd = vi.spyOn(window, "addEventListener").mockImplementation(((type: string, listener: EventListenerOrEventListenerObject, options?: boolean | AddEventListenerOptions) => { + added.push(type); + realAdd(type, listener, options); + }) as typeof window.addEventListener); + const trackRemove = vi.spyOn(window, "removeEventListener").mockImplementation(((type: string, listener: EventListenerOrEventListenerObject, options?: boolean | EventListenerOptions) => { + removed.push(type); + realRemove(type, listener, options); + }) as typeof window.removeEventListener); + try { + const first = vi.fn(); + const second = vi.fn(); + const third = vi.fn(); + const { rerender } = renderHook(({ handler }) => useHotkey("k", true, handler), { initialProps: { handler: first } }); + const keydownAdds = () => added.filter((type) => type === "keydown").length; + expect(keydownAdds()).toBe(1); + + // A fresh handler on every render used to remove and re-add the listener each time. + rerender({ handler: second }); + rerender({ handler: third }); + expect(keydownAdds()).toBe(1); + expect(removed.filter((type) => type === "keydown")).toHaveLength(0); + + dispatchKey("k", { ctrlKey: true }); + expect(third).toHaveBeenCalledTimes(1); + expect(first).not.toHaveBeenCalled(); + expect(second).not.toHaveBeenCalled(); + } finally { + trackAdd.mockRestore(); + trackRemove.mockRestore(); + } + }); + + it("re-registers when the key itself changes", () => { + const handler = vi.fn(); + const { rerender } = renderHook(({ key }) => useHotkey(key, true, handler), { initialProps: { key: "k" } }); + + rerender({ key: "j" }); + dispatchKey("k", { ctrlKey: true }); + expect(handler).not.toHaveBeenCalled(); + dispatchKey("j", { ctrlKey: true }); + expect(handler).toHaveBeenCalledTimes(1); + }); }); diff --git a/frontend/src/hooks/useHotkey.ts b/frontend/src/hooks/useHotkey.ts index 4f511112..4eb7f545 100644 --- a/frontend/src/hooks/useHotkey.ts +++ b/frontend/src/hooks/useHotkey.ts @@ -1,6 +1,6 @@ -import { useEffect } from "react"; +import { useEffect, useRef } from "react"; -function isEditableTarget(target: EventTarget | null): boolean { +export function isEditableTarget(target: EventTarget | null): boolean { const element = target as HTMLElement | null; if (!element) return false; return element.tagName === "INPUT" || element.tagName === "TEXTAREA" || element.isContentEditable; @@ -11,20 +11,39 @@ function isEditableTarget(target: EventTarget | null): boolean { * even while typing, matching how palette shortcuts behave elsewhere; plain * keys (e.g. "?") are suppressed while focus is in an editable field so they * don't hijack normal typing. + * + * Three things a shortcut must not do: + * - fire again for every auto-repeat while the key is held (the palette + * toggle flickered open and shut under a held Ctrl+K); + * - fire in the middle of an IME composition, where keys belong to the + * composition; + * - treat AltGr as Ctrl. Chromium reports AltGr as Ctrl+Alt, so on a layout + * that types characters with AltGr, Ctrl+K matched while the person was + * simply typing one. A modifier combo therefore requires Alt to be up. + * + * The handler is held in a ref, so a new inline function each render does not + * remove and re-add the window listener. */ export function useHotkey(key: string, withModifier: boolean, handler: () => void): void { + const handlerRef = useRef(handler); + // Assigned in an effect rather than during render, as react-hooks/refs requires. + useEffect(() => { + handlerRef.current = handler; + }); + useEffect(() => { const listener = (event: KeyboardEvent) => { + if (event.repeat || event.isComposing) return; if (event.key.toLowerCase() !== key.toLowerCase()) return; const modifierMatches = withModifier - ? event.ctrlKey || event.metaKey + ? (event.ctrlKey || event.metaKey) && !event.altKey : !event.ctrlKey && !event.metaKey && !event.altKey; if (!modifierMatches) return; if (!withModifier && isEditableTarget(event.target)) return; event.preventDefault(); - handler(); + handlerRef.current(); }; window.addEventListener("keydown", listener); return () => window.removeEventListener("keydown", listener); - }, [key, withModifier, handler]); + }, [key, withModifier]); } diff --git a/frontend/src/hooks/usePageEscape.test.ts b/frontend/src/hooks/usePageEscape.test.ts new file mode 100644 index 00000000..e75ac52b --- /dev/null +++ b/frontend/src/hooks/usePageEscape.test.ts @@ -0,0 +1,140 @@ +import { renderHook } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { usePageEscape } from "./usePageEscape"; + +function press(options: Partial = {}, target: EventTarget = document.body) { + const event = new KeyboardEvent("keydown", { key: "Escape", bubbles: true, cancelable: true, ...options }); + target.dispatchEvent(event); + return event; +} + +const added: HTMLElement[] = []; +function attach(element: T): T { + document.body.appendChild(element); + added.push(element); + return element; +} + +afterEach(() => { + for (const element of added.splice(0)) element.remove(); +}); + +describe("usePageEscape", () => { + it("runs the handler on Escape from the page, and takes the key", () => { + const handler = vi.fn(); + renderHook(() => usePageEscape(handler, true)); + + const event = press(); + + expect(handler).toHaveBeenCalledTimes(1); + expect(event.defaultPrevented).toBe(true); + }); + + it("does nothing while disabled, and starts when enabled", () => { + const handler = vi.fn(); + const { rerender } = renderHook(({ enabled }) => usePageEscape(handler, enabled), { initialProps: { enabled: false } }); + + const ignored = press(); + expect(handler).not.toHaveBeenCalled(); + expect(ignored.defaultPrevented).toBe(false); + + rerender({ enabled: true }); + press(); + expect(handler).toHaveBeenCalledTimes(1); + + rerender({ enabled: false }); + press(); + expect(handler).toHaveBeenCalledTimes(1); + }); + + it("ignores other keys", () => { + const handler = vi.fn(); + renderHook(() => usePageEscape(handler, true)); + + press({ key: "Enter" }); + press({ key: "e" }); + + expect(handler).not.toHaveBeenCalled(); + }); + + it("leaves an Escape that something already handled", () => { + const handler = vi.fn(); + renderHook(() => usePageEscape(handler, true)); + const button = attach(document.createElement("button")); + // What the composer (and any control that owns Escape) does before it gets here. + button.addEventListener("keydown", (event) => event.preventDefault()); + + press({}, button); + + expect(handler).not.toHaveBeenCalled(); + }); + + it.each(["dialog", "alertdialog", "menu", "listbox"])("leaves Escape to an open %s", (role) => { + const handler = vi.fn(); + renderHook(() => usePageEscape(handler, true)); + const overlay = attach(document.createElement("div")); + overlay.setAttribute("role", role); + + const event = press(); + + expect(handler).not.toHaveBeenCalled(); + expect(event.defaultPrevented).toBe(false); + + overlay.remove(); + press(); + expect(handler).toHaveBeenCalledTimes(1); + }); + + it("leaves Escape to a text field, where it cancels the edit", () => { + const handler = vi.fn(); + renderHook(() => usePageEscape(handler, true)); + + press({}, attach(document.createElement("input"))); + press({}, attach(document.createElement("textarea"))); + const editable = attach(document.createElement("div")); + Object.defineProperty(editable, "isContentEditable", { value: true }); + press({}, editable); + + expect(handler).not.toHaveBeenCalled(); + }); + + it("ignores auto-repeat, composition, and modifiers", () => { + const handler = vi.fn(); + renderHook(() => usePageEscape(handler, true)); + + press({ repeat: true }); + press({ isComposing: true }); + press({ ctrlKey: true }); + press({ metaKey: true }); + press({ altKey: true }); + press({ shiftKey: true }); + + expect(handler).not.toHaveBeenCalled(); + }); + + it("calls the latest handler without re-registering", () => { + const first = vi.fn(); + const second = vi.fn(); + const add = vi.spyOn(document, "addEventListener"); + const { rerender } = renderHook(({ handler }) => usePageEscape(handler, true), { initialProps: { handler: first } }); + const registrations = add.mock.calls.filter(([type]) => type === "keydown").length; + + rerender({ handler: second }); + press(); + + expect(add.mock.calls.filter(([type]) => type === "keydown")).toHaveLength(registrations); + expect(first).not.toHaveBeenCalled(); + expect(second).toHaveBeenCalledTimes(1); + add.mockRestore(); + }); + + it("removes its listener on unmount", () => { + const handler = vi.fn(); + const { unmount } = renderHook(() => usePageEscape(handler, true)); + + unmount(); + press(); + + expect(handler).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/hooks/usePageEscape.ts b/frontend/src/hooks/usePageEscape.ts new file mode 100644 index 00000000..6d7fac23 --- /dev/null +++ b/frontend/src/hooks/usePageEscape.ts @@ -0,0 +1,50 @@ +import { useEffect, useRef } from "react"; +import { isEditableTarget } from "./useHotkey"; + +/** + * The overlays that own Escape while they are open: dialogs (the shortcuts + * reference, confirmations, the command palette), the popovers that are marked + * up as dialogs, and open menus and listboxes. Each closes on Escape, and that + * one keypress must not also do something to the page underneath. + */ +const OVERLAY_SELECTOR = '[role="dialog"], [role="alertdialog"], [role="menu"], [role="listbox"]'; + +/** + * Runs `onEscape` when Escape is pressed on the page itself while `enabled`. + * + * It is for an action that is otherwise reachable only from one focused + * control (stopping a response from the composer). Escape is left alone when + * something else has a claim on it: + * - an element already handled it (`defaultPrevented`), which is how the + * composer's own Escape handling keeps this from running twice; + * - an overlay is open, whose Escape closes it; + * - focus is in some other text field, where Escape cancels the edit; + * - it is an auto-repeat, an IME composition, or carries a modifier. + * + * The handler is held in a ref, so a new function each render does not remove + * and re-add the listener. + */ +export function usePageEscape(onEscape: () => void, enabled: boolean): void { + const handlerRef = useRef(onEscape); + // Assigned in an effect rather than during render, as react-hooks/refs requires. + useEffect(() => { + handlerRef.current = onEscape; + }); + + useEffect(() => { + if (!enabled) return undefined; + const listener = (event: KeyboardEvent) => { + if (event.key !== "Escape" || event.defaultPrevented) return; + if (event.repeat || event.isComposing) return; + if (event.ctrlKey || event.metaKey || event.altKey || event.shiftKey) return; + if (isEditableTarget(event.target)) return; + if (document.querySelector(OVERLAY_SELECTOR)) return; + event.preventDefault(); + handlerRef.current(); + }; + // On the document, which the React handlers below the root have already + // run before, so an element's own handling (and its preventDefault) is seen. + document.addEventListener("keydown", listener); + return () => document.removeEventListener("keydown", listener); + }, [enabled]); +} From 4157972a5f827943322ed507164e02593aacceb0 Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Tue, 29 Sep 2026 04:12:20 -0400 Subject: [PATCH 06/13] feat(a11y): announce when a response starts, finishes, stops, or fails A screen reader was never told that a response began or ended: the streaming bubble is not a live region (and should not be, or every token would be read), the composer status reverts, and the pending bubble is gone at the first token. A visually hidden polite status now says Response started, Response complete with the token count when known, Response stopped, or Response failed, following the chat store so an outcome published by the stream host is heard whichever page was showing. A failed job's completion after its failure is not announced a second time, and an outcome already waiting when the page opened is not read out again. Includes page-level tests for Escape from the body, from the composer (once), and with an overlay or another field open. Co-Authored-By: Claude Sonnet 5.5 --- .../src/features/chat/ChatPage.a11y.test.tsx | 377 ++++++++++++++++++ frontend/src/features/chat/ChatPage.tsx | 2 + .../src/features/chat/ResponseAnnouncer.tsx | 71 ++++ 3 files changed, 450 insertions(+) create mode 100644 frontend/src/features/chat/ChatPage.a11y.test.tsx create mode 100644 frontend/src/features/chat/ResponseAnnouncer.tsx diff --git a/frontend/src/features/chat/ChatPage.a11y.test.tsx b/frontend/src/features/chat/ChatPage.a11y.test.tsx new file mode 100644 index 00000000..e39b4a35 --- /dev/null +++ b/frontend/src/features/chat/ChatPage.a11y.test.tsx @@ -0,0 +1,377 @@ +import { act, render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { ComponentProps } from "react"; +import type { ChatResponse } from "../../../../contracts/cortex-api"; +import { CortexApi } from "../../api/client"; +import { useChatStore } from "../../stores/useChatStore"; +import { ChatPage } from "./ChatPage"; +import { GenerationStreamHost } from "./GenerationStreamHost"; + +/** + * Keyboard and screen-reader behaviour of the chat page while a response runs: + * Escape from anywhere on the page, and the live region that says when a + * response started, finished, stopped, or failed. + */ + +function ChatWithHost(props: ComponentProps) { + return ( + <> + undefined} /> + + + ); +} + +const emptyChat = (id: string): ChatResponse => ({ + id, + title: "New Chat", + timestamp: "2026-01-01T00:00:00Z", + revision: 0, + messages: [], +}); + +type Harness = { + api: CortexApi; + cancelGeneration: ReturnType; + /** Deliver one stream event to the running job, then let the stream end. */ + finishWith: (event: string, data: Record, savedChat?: ChatResponse) => Promise; +}; + +/** A chat whose one job stays open until the test finishes it. */ +function harness(): Harness { + let emit: ((event: unknown) => void) | null = null; + let resolveStream: (() => void) | null = null; + let saved: ChatResponse = emptyChat("thread-a"); + const cancelGeneration = vi.fn(async () => ({ job_id: "job-1", kind: "generation", status: "cancelling", sequence: 2 })); + const api = { + chat: vi.fn(async () => saved), + generate: vi.fn().mockResolvedValue({ + job_id: "job-1", + kind: "generation", + status: "queued", + thread_id: "thread-a", + user_message_id: "user-1", + }), + regenerate: vi.fn(), + streamGeneration: vi.fn((_jobId, onEvent, options: { signal?: AbortSignal } = {}) => { + emit = onEvent as (event: unknown) => void; + return new Promise((resolve, reject) => { + options.signal?.addEventListener("abort", () => reject(new DOMException("Aborted", "AbortError")), { once: true }); + resolveStream = resolve; + }); + }), + generationStatus: vi.fn(), + cancelGeneration, + forkChat: vi.fn(), + stageChatAttachment: vi.fn(), + } as unknown as CortexApi; + return { + api, + cancelGeneration, + finishWith: async (event, data, savedChat) => { + await waitFor(() => expect(emit).not.toBeNull()); + if (savedChat) saved = savedChat; + await act(async () => { + emit!({ event_id: 1, event, job_id: "job-1", thread_id: "thread-a", data }); + resolveStream?.(); + }); + await waitFor(() => expect(useChatStore.getState().generation.jobId).toBeNull()); + }, + }; +} + +function renderChat(api: CortexApi) { + return render( + true} + onRescanModels={async () => undefined} + onThreadCreated={vi.fn()} + onForked={vi.fn()} + />, + ); +} + +const announcer = () => document.querySelector('.sr-only[role="status"]'); + +async function sendPrompt(user: ReturnType) { + await user.type(await screen.findByLabelText("Message Cortex"), "Tell me something"); + await user.click(screen.getByRole("button", { name: "Send message" })); + await screen.findByRole("button", { name: "Stop generating" }); +} + +/** + * Move focus off the composer, as it is for someone reading the answer. + * Sending hands focus back to the composer on the next frame, so wait for that + * first; blurring earlier would just be undone. + */ +async function focusThePage() { + const composer = screen.getByLabelText("Message Cortex"); + await waitFor(() => expect(document.activeElement).toBe(composer)); + composer.blur(); + expect(document.activeElement).toBe(document.body); +} + +const savedAnswer = (stats: Record): ChatResponse => ({ + ...emptyChat("thread-a"), + revision: 2, + messages: [ + { id: "user-1", role: "user", content: "Tell me something" }, + { id: "assistant-1", role: "assistant", content: "Something.", stats }, + ], +}); + +afterEach(() => { + document.body.querySelectorAll("[data-test-overlay]").forEach((node) => node.remove()); + window.sessionStorage.clear(); +}); + +describe("ChatPage Escape", () => { + it("stops the response when Escape is pressed with focus anywhere on the page", async () => { + const user = userEvent.setup(); + const { api, cancelGeneration } = harness(); + renderChat(api); + await sendPrompt(user); + await focusThePage(); + + await user.keyboard("{Escape}"); + + await waitFor(() => expect(cancelGeneration).toHaveBeenCalledWith("job-1")); + expect(cancelGeneration).toHaveBeenCalledTimes(1); + }); + + it("still stops from the composer, and only once", async () => { + const user = userEvent.setup(); + const { api, cancelGeneration } = harness(); + renderChat(api); + await sendPrompt(user); + screen.getByLabelText("Message Cortex").focus(); + + await user.keyboard("{Escape}"); + + await waitFor(() => expect(cancelGeneration).toHaveBeenCalledWith("job-1")); + // The composer handles it and prevents default; the page-level handler must not repeat it. + expect(cancelGeneration).toHaveBeenCalledTimes(1); + }); + + it.each([ + ["a dialog", "dialog"], + ["an alert dialog", "alertdialog"], + ["a menu", "menu"], + ["a listbox", "listbox"], + ])("leaves Escape to %s that is open", async (_name, role) => { + const user = userEvent.setup(); + const { api, cancelGeneration } = harness(); + renderChat(api); + await sendPrompt(user); + const overlay = document.createElement("div"); + overlay.setAttribute("role", role); + overlay.setAttribute("data-test-overlay", ""); + document.body.appendChild(overlay); + await focusThePage(); + + await user.keyboard("{Escape}"); + + expect(cancelGeneration).not.toHaveBeenCalled(); + }); + + it("stops again once the overlay has closed", async () => { + const user = userEvent.setup(); + const { api, cancelGeneration } = harness(); + renderChat(api); + await sendPrompt(user); + const overlay = document.createElement("div"); + overlay.setAttribute("role", "dialog"); + document.body.appendChild(overlay); + await focusThePage(); + await user.keyboard("{Escape}"); + expect(cancelGeneration).not.toHaveBeenCalled(); + + overlay.remove(); + await user.keyboard("{Escape}"); + + await waitFor(() => expect(cancelGeneration).toHaveBeenCalledTimes(1)); + }); + + it("leaves Escape to another text field, where it cancels the edit", async () => { + const user = userEvent.setup(); + const { api, cancelGeneration } = harness(); + renderChat(api); + await sendPrompt(user); + const field = document.createElement("input"); + field.setAttribute("data-test-overlay", ""); + document.body.appendChild(field); + field.focus(); + + await user.keyboard("{Escape}"); + + expect(cancelGeneration).not.toHaveBeenCalled(); + }); + + it("does nothing while no response is running", async () => { + const user = userEvent.setup(); + const { api, cancelGeneration } = harness(); + renderChat(api); + await screen.findByLabelText("Message Cortex"); + (document.activeElement as HTMLElement | null)?.blur(); + + await user.keyboard("{Escape}"); + + expect(cancelGeneration).not.toHaveBeenCalled(); + }); + + it("ignores Escape with a modifier held", async () => { + const user = userEvent.setup(); + const { api, cancelGeneration } = harness(); + renderChat(api); + await sendPrompt(user); + await focusThePage(); + + await user.keyboard("{Shift>}{Escape}{/Shift}"); + + expect(cancelGeneration).not.toHaveBeenCalled(); + }); +}); + +describe("ChatPage response announcements", () => { + it("is a polite status region, hidden from sight, and empty until something happens", async () => { + const { api } = harness(); + renderChat(api); + await screen.findByLabelText("Message Cortex"); + + const region = announcer(); + expect(region).not.toBeNull(); + expect(region).toHaveAttribute("aria-live", "polite"); + expect(region).toHaveClass("sr-only"); + expect(region).toBeEmptyDOMElement(); + }); + + it("says a response started, then how long the finished one was", async () => { + const user = userEvent.setup(); + const { api, finishWith } = harness(); + renderChat(api); + await sendPrompt(user); + await waitFor(() => expect(announcer()).toHaveTextContent("Response started")); + + await finishWith("generation.completed", { assistant_message_id: "assistant-1" }, savedAnswer({ eval_count: 1234 })); + + await waitFor(() => expect(announcer()).toHaveTextContent("Response complete, 1,234 tokens")); + }); + + it("says a response is complete without a count when none is known", async () => { + const user = userEvent.setup(); + const { api, finishWith } = harness(); + renderChat(api); + await sendPrompt(user); + + await finishWith("generation.completed", { assistant_message_id: "assistant-1" }, savedAnswer({})); + + await waitFor(() => expect(announcer()).toHaveTextContent(/^Response complete$/)); + }); + + it("says a response stopped after the person stopped it and the answer was kept", async () => { + const user = userEvent.setup(); + const { api, finishWith } = harness(); + renderChat(api); + await sendPrompt(user); + await user.click(screen.getByRole("button", { name: "Stop generating" })); + await screen.findByRole("button", { name: "Stopping response" }); + + await finishWith( + "generation.cancelled", + { message: "Job cancelled.", assistant_message_id: "assistant-1" }, + savedAnswer({ stopped: true, eval_count: 9 }), + ); + + await waitFor(() => expect(announcer()).toHaveTextContent(/^Response stopped$/)); + }); + + it("says a response stopped when there was nothing to keep, not that it failed", async () => { + const user = userEvent.setup(); + const { api, finishWith } = harness(); + renderChat(api); + await sendPrompt(user); + await user.click(screen.getByRole("button", { name: "Stop generating" })); + await screen.findByRole("button", { name: "Stopping response" }); + + await finishWith("generation.cancelled", { message: "Job cancelled." }); + + await waitFor(() => expect(announcer()).toHaveTextContent(/^Response stopped$/)); + }); + + it("says a response stopped after Escape from the page", async () => { + const user = userEvent.setup(); + const { api, cancelGeneration, finishWith } = harness(); + renderChat(api); + await sendPrompt(user); + await focusThePage(); + await user.keyboard("{Escape}"); + await waitFor(() => expect(cancelGeneration).toHaveBeenCalledTimes(1)); + + await finishWith( + "generation.cancelled", + { message: "Job cancelled.", assistant_message_id: "assistant-1" }, + savedAnswer({ stopped: true }), + ); + + await waitFor(() => expect(announcer()).toHaveTextContent(/^Response stopped$/)); + }); + + it("says a response failed, briefly, leaving the reason to the composer's alert", async () => { + const user = userEvent.setup(); + const { api, finishWith } = harness(); + renderChat(api); + await sendPrompt(user); + + await finishWith("generation.failed", { message: "The model failed." }); + + await waitFor(() => expect(announcer()).toHaveTextContent(/^Response failed$/)); + expect(await screen.findByRole("alert")).toHaveTextContent("The model failed."); + }); + + it("does not announce every streamed token", async () => { + const user = userEvent.setup(); + const { api } = harness(); + renderChat(api); + await sendPrompt(user); + await waitFor(() => expect(announcer()).toHaveTextContent("Response started")); + const seen: string[] = []; + const observer = new MutationObserver(() => seen.push(announcer()?.textContent ?? "")); + observer.observe(announcer()!, { childList: true, characterData: true, subtree: true }); + + act(() => { + for (let token = 0; token < 30; token += 1) useChatStore.getState().appendContentToken("job-1", "word "); + }); + await act(async () => { await Promise.resolve(); }); + observer.disconnect(); + + expect(seen).toEqual([]); + expect(announcer()).toHaveTextContent(/^Response started$/); + }); + + it("does not read out an outcome that was already waiting when the page opened", async () => { + useChatStore.getState().recordFailure({ threadId: "thread-a", message: "Earlier failure." }); + const { api } = harness(); + renderChat(api); + await screen.findByLabelText("Message Cortex"); + + expect(announcer()).toBeEmptyDOMElement(); + }); + + it("does not announce another thread's outcome", async () => { + const { api } = harness(); + renderChat(api); + await screen.findByLabelText("Message Cortex"); + + act(() => useChatStore.getState().recordFailure({ threadId: "thread-b", message: "Elsewhere." })); + + expect(announcer()).toBeEmptyDOMElement(); + }); +}); diff --git a/frontend/src/features/chat/ChatPage.tsx b/frontend/src/features/chat/ChatPage.tsx index 0877c654..92cdebaf 100644 --- a/frontend/src/features/chat/ChatPage.tsx +++ b/frontend/src/features/chat/ChatPage.tsx @@ -16,6 +16,7 @@ import { Dialog, DialogContent } from "../../shared/ui/Dialog"; import { MessageComposer, type ComposerPhase } from "./MessageComposer"; import { MessageList, type MessageListHandle } from "./MessageList"; import { PendingAssistantMessage } from "./PendingAssistantMessage"; +import { ResponseAnnouncer } from "./ResponseAnnouncer"; const DEFAULT_GENERATION_SETTINGS = { temperature: 0.7, @@ -898,6 +899,7 @@ export function ChatPage({
)}

{displayChatTitle(currentChat?.title, "New Chat")}

+ message.role === "assistant"); + const stats = lastAssistant?.stats; + if (stats?.stopped) return "Response stopped"; + const tokens = stats?.eval_count; + if (typeof tokens === "number" && tokens > 0) { + return `Response complete, ${tokens.toLocaleString()} ${tokens === 1 ? "token" : "tokens"}`; + } + return "Response complete"; +} + +/** + * A visually hidden, polite live region that says when a response starts, + * finishes, stops, or fails -- and nothing in between. + * + * The transcript itself is deliberately not a live region: a streaming answer + * changes many times a second, and a screen reader would read it back token by + * token. The composer's status line reads "Generating..." and then reverts, and + * the pending bubble is gone as soon as the first token arrives, so without + * this a person using a screen reader is never told a response began or ended. + * + * It follows the chat store rather than props: a stop or a failure ends up + * published there by the stream host, whichever page was showing. Only changes + * seen while this is mounted are announced, so an outcome that was already + * waiting when the page opened is not read out again. A failure is announced + * briefly; its reason is in the composer's alert, which is read on its own. + */ +export function ResponseAnnouncer({ threadId }: { threadId: string | null }) { + const [announcement, setAnnouncement] = useState(""); + const threadIdRef = useRef(threadId); + // The job whose outcome has been announced. A failed or stopped job publishes + // its failure and then, once the chat is reloaded, a completion as well; only + // the first says what happened, and the second must not overwrite it. + const settledJobRef = useRef(null); + // Assigned in an effect rather than during render, as react-hooks/refs requires. + useEffect(() => { + threadIdRef.current = threadId; + }); + + useEffect(() => useChatStore.subscribe((state, previous) => { + const { jobId, phase } = state.generation; + if (jobId && jobId !== previous.generation.jobId) { + setAnnouncement("Response started"); + } + // A stop the person asked for is "stopping" until the job ends, whether it + // ends as a kept answer (a completion) or as nothing to keep (a failure). + const userStopped = phase === "stopping"; + const outcome = (announce: string) => { + if (jobId && settledJobRef.current === jobId) return; + settledJobRef.current = jobId; + setAnnouncement(announce); + }; + const { lastCompletion, lastFailure } = state; + if (lastFailure && lastFailure !== previous.lastFailure && lastFailure.threadId === threadIdRef.current) { + outcome(userStopped ? "Response stopped" : "Response failed"); + } + if (lastCompletion && lastCompletion !== previous.lastCompletion && lastCompletion.threadId === threadIdRef.current) { + outcome(userStopped ? "Response stopped" : describeFinishedResponse(lastCompletion.chat)); + } + }), []); + + return
{announcement}
; +} From e6c5b9024e18afa0d2f02819d22cd2e229115eb0 Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Tue, 29 Sep 2026 04:22:08 -0400 Subject: [PATCH 07/13] fix(palette): search every chat, keep untitled chats apart, and cycle the theme through system The palette rendered only the eight newest chats, so cmdk could never find an older one although the placeholder promised chat search. With text typed it now searches all chats (scored, best first, at most fifty shown, and paused for 120ms of typing so a long history is not scored on every key), with the heading changing between Recent chats and Chats, and the text is forgotten when the palette closes. Chat rows now carry their id as the cmdk value and their title as a keyword, so several chats with the same title are separate, reachable rows rather than one shared highlight. The theme item cycled system into dark; it now goes system, light, dark and names the theme it would switch to. Co-Authored-By: Claude Sonnet 5.5 --- frontend/src/app/App.test.tsx | 4 +- frontend/src/app/App.tsx | 7 +- .../command-palette/CommandPalette.test.tsx | 195 +++++++++++++++++- .../command-palette/CommandPalette.tsx | 145 +++++++++++-- frontend/src/hooks/useDebouncedValue.test.ts | 52 +++++ frontend/src/hooks/useDebouncedValue.ts | 15 ++ frontend/src/lib/theme.test.ts | 26 +++ frontend/src/lib/theme.ts | 18 ++ 8 files changed, 438 insertions(+), 24 deletions(-) create mode 100644 frontend/src/hooks/useDebouncedValue.test.ts create mode 100644 frontend/src/hooks/useDebouncedValue.ts diff --git a/frontend/src/app/App.test.tsx b/frontend/src/app/App.test.tsx index f29e9433..c8b5ec34 100644 --- a/frontend/src/app/App.test.tsx +++ b/frontend/src/app/App.test.tsx @@ -517,7 +517,7 @@ describe("App", () => { const user = userEvent.setup(); await user.keyboard("{Control>}k{/Control}"); - await user.click(await screen.findByText("Toggle theme")); + await user.click(await screen.findByText("Change theme")); expect(await screen.findByText("Settings saved.")).toBeVisible(); expect(callsTo(fetcher, "/session/handoff")).toHaveLength(1); @@ -901,7 +901,7 @@ describe("App", () => { const user = userEvent.setup(); await user.keyboard("{Control>}k{/Control}"); - await user.click(await screen.findByText("Toggle theme")); + await user.click(await screen.findByText("Change theme")); await waitFor(() => expect(document.documentElement.dataset.theme).toBe("light")); expect(window.localStorage.getItem("cortex.theme")).toBe("light"); diff --git a/frontend/src/app/App.tsx b/frontend/src/app/App.tsx index 0e9794d3..5abee95e 100644 --- a/frontend/src/app/App.tsx +++ b/frontend/src/app/App.tsx @@ -21,7 +21,7 @@ import type { MemoryLoadState } from "../features/settings/MemoryPanel"; import { blockStrayFileDrops } from "../lib/attachments"; import { displayModelName, isGGUFModel, localModelNames } from "../lib/localModels"; import { chatPath, navigate, parseAppRoute, useNavigate, usePathname } from "../lib/navigation"; -import { applyStoredTheme, DEFAULT_THEME_PREFERENCE } from "../lib/theme"; +import { applyStoredTheme, DEFAULT_THEME_PREFERENCE, nextThemePreference } from "../lib/theme"; import { useAppliedTheme } from "../hooks/useAppliedTheme"; import { useVisiblePolling } from "../hooks/useVisiblePolling"; import { useChatStore } from "../stores/useChatStore"; @@ -871,7 +871,9 @@ function AuthenticatedWorkspace({ api, onSessionExpired }: { api: CortexApi; onS navigate("/settings"); }; const toggleTheme = () => { - const next = theme === "dark" ? "light" : "dark"; + // System, light, dark, and round again: "system" is a choice of its own, + // and used to be turned into "dark" by the first press. + const next = nextThemePreference(theme); void saveSettings({ ...settings, appearance: { ...settings.appearance, theme: next } }); }; @@ -890,6 +892,7 @@ function AuthenticatedWorkspace({ api, onSessionExpired }: { api: CortexApi; onS chats={chats} localModels={localModels} selectedModel={selectedModel} + theme={theme} onNewChat={() => navigate("/chat/new")} onOpenSettings={openSettings} onToggleTheme={toggleTheme} diff --git a/frontend/src/features/command-palette/CommandPalette.test.tsx b/frontend/src/features/command-palette/CommandPalette.test.tsx index 8fa46f2e..c920d8c7 100644 --- a/frontend/src/features/command-palette/CommandPalette.test.tsx +++ b/frontend/src/features/command-palette/CommandPalette.test.tsx @@ -1,4 +1,4 @@ -import { render, screen } from "@testing-library/react"; +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, expect, it, vi } from "vitest"; import type { ChatSummary } from "../../../../contracts/cortex-api"; @@ -14,6 +14,7 @@ function renderPalette(overrides: Partial[0]> chats, localModels: ["qwen3:8b", "granite4:tiny-h"], selectedModel: "qwen3:8b", + theme: "dark" as const, onNewChat: vi.fn(), onOpenSettings: vi.fn(), onToggleTheme: vi.fn(), @@ -74,3 +75,195 @@ describe("CommandPalette", () => { expect(props.onSelectChat).toHaveBeenCalledWith("chat-1"); }); }); + +/** Newest first, as the chat list arrives: "Topic 01" is the most recent. */ +const manyChats = (count: number, title = (n: number) => `Topic ${String(n).padStart(2, "0")}`): ChatSummary[] => + Array.from({ length: count }, (_, index) => ({ + id: `chat-${index + 1}`, + title: title(index + 1), + timestamp: "2026-01-01T00:00:00Z", + })); + +const chatRows = () => Array.from(document.querySelectorAll('[cmdk-item][data-value^="chat:"]')); + +describe("CommandPalette chat search", () => { + it("lists only the eight newest chats until something is typed", async () => { + renderPalette({ chats: manyChats(12) }); + useUiStore.getState().setCommandPaletteOpen(true); + + expect(await screen.findByText("Recent chats")).toBeVisible(); + expect(chatRows()).toHaveLength(8); + expect(screen.getByText("Topic 08")).toBeVisible(); + expect(screen.queryByText("Topic 09")).not.toBeInTheDocument(); + }); + + it("finds a chat older than the recent eight by typing its title", async () => { + const props = renderPalette({ chats: manyChats(12) }); + useUiStore.getState().setCommandPaletteOpen(true); + const user = userEvent.setup(); + + await user.type(await screen.findByPlaceholderText("Type a command or search chats…"), "Topic 12"); + + // The heading follows the mode: the list is no longer "recent". + expect(await screen.findByText("Topic 12")).toBeVisible(); + expect(screen.getByText("Chats")).toBeVisible(); + expect(screen.queryByText("Recent chats")).not.toBeInTheDocument(); + await user.click(screen.getByText("Topic 12")); + expect(props.onSelectChat).toHaveBeenCalledWith("chat-12"); + }); + + it("puts the best match first, not the newest", async () => { + renderPalette({ + chats: [ + { id: "recent-weak", title: "Notes on the weekly review", timestamp: "2026-01-02T00:00:00Z" }, + { id: "older-exact", title: "Review", timestamp: "2026-01-01T00:00:00Z" }, + ], + }); + useUiStore.getState().setCommandPaletteOpen(true); + const user = userEvent.setup(); + + await user.type(await screen.findByPlaceholderText("Type a command or search chats…"), "Review"); + + await screen.findByText("Review"); + const titles = chatRows().map((row) => row.textContent); + expect(titles).toEqual(["Review", "Notes on the weekly review"]); + }); + + it("shows at most fifty results however many chats match", async () => { + renderPalette({ chats: manyChats(400, (n) => `Planning notes ${n}`) }); + useUiStore.getState().setCommandPaletteOpen(true); + const user = userEvent.setup(); + + await user.type(await screen.findByPlaceholderText("Type a command or search chats…"), "Planning"); + + await waitFor(() => expect(chatRows().length).toBeGreaterThan(8)); + expect(chatRows()).toHaveLength(50); + }); + + it("does not search on every keystroke", async () => { + // Scoring every title for each key is what a long history cannot afford; the + // rows on screen are for the last query that paused, not for each letter. + renderPalette({ chats: manyChats(12) }); + useUiStore.getState().setCommandPaletteOpen(true); + const input = await screen.findByPlaceholderText("Type a command or search chats…"); + + // One change event and then synchronous assertions: no timer can run in + // between, so this is "straight after typing" exactly, on any machine. + fireEvent.change(input, { target: { value: "Topic 12" } }); + + // The older chat has not been searched for yet ... + expect(screen.queryByText("Topic 12")).not.toBeInTheDocument(); + // ... and no premature "No results." is shown while it catches up. + expect(screen.queryByText("No results.")).not.toBeInTheDocument(); + // Once typing has paused, the search runs. + expect(await screen.findByText("Topic 12")).toBeVisible(); + }); + + it("says there are no results once the search has settled on nothing", async () => { + renderPalette({ chats: manyChats(12) }); + useUiStore.getState().setCommandPaletteOpen(true); + const user = userEvent.setup(); + + await user.type(await screen.findByPlaceholderText("Type a command or search chats…"), "zzzzqq"); + + expect(await screen.findByText("No results.")).toBeVisible(); + }); + + it("returns to the recent chats as soon as the box is cleared", async () => { + renderPalette({ chats: manyChats(12) }); + useUiStore.getState().setCommandPaletteOpen(true); + const user = userEvent.setup(); + const input = await screen.findByPlaceholderText("Type a command or search chats…"); + await user.type(input, "Topic 12"); + await screen.findByText("Chats"); + + await user.clear(input); + + expect(screen.getByText("Recent chats")).toBeVisible(); + expect(chatRows()).toHaveLength(8); + }); + + it("forgets the search when the palette closes", async () => { + renderPalette({ chats: manyChats(12) }); + useUiStore.getState().setCommandPaletteOpen(true); + const user = userEvent.setup(); + await user.type(await screen.findByPlaceholderText("Type a command or search chats…"), "Topic 12"); + await screen.findByText("Chats"); + + act(() => useUiStore.getState().setCommandPaletteOpen(false)); + expect(screen.queryByPlaceholderText("Type a command or search chats…")).not.toBeInTheDocument(); + act(() => useUiStore.getState().setCommandPaletteOpen(true)); + + const reopened = await screen.findByPlaceholderText("Type a command or search chats…"); + expect(reopened).toHaveValue(""); + expect(screen.getByText("Recent chats")).toBeVisible(); + }); + + it("keeps two chats with the same title apart, each reachable and selectable", async () => { + const props = renderPalette({ + chats: [ + { id: "chat-a", title: "New Chat", timestamp: "2026-01-02T00:00:00Z" }, + { id: "chat-b", title: "New Chat", timestamp: "2026-01-01T00:00:00Z" }, + ], + }); + useUiStore.getState().setCommandPaletteOpen(true); + const user = userEvent.setup(); + await screen.findByText("Recent chats"); + const rows = chatRows(); + expect(rows).toHaveLength(2); + + // Pointing at the second highlights that row alone; before, both shared one value. + await user.hover(rows[1]); + expect(rows.map((row) => row.getAttribute("aria-selected"))).toEqual(["false", "true"]); + + await user.click(rows[1]); + expect(props.onSelectChat).toHaveBeenCalledWith("chat-b"); + }); + + it("matches a chat by its title, never by its id", async () => { + renderPalette({ chats: [{ id: "aabbccdd-1122-3344", title: "Budget", timestamp: "2026-01-01T00:00:00Z" }] }); + useUiStore.getState().setCommandPaletteOpen(true); + const user = userEvent.setup(); + + await user.type(await screen.findByPlaceholderText("Type a command or search chats…"), "aabbcc"); + + expect(await screen.findByText("No results.")).toBeVisible(); + expect(screen.queryByText("Budget")).not.toBeInTheDocument(); + }); +}); + +describe("CommandPalette theme item", () => { + it.each([ + ["system", "Next: Light"], + ["light", "Next: Dark"], + ["dark", "Next: System"], + ] as const)("names the theme it would switch to from %s", async (theme, hint) => { + renderPalette({ theme }); + useUiStore.getState().setCommandPaletteOpen(true); + + const item = (await screen.findByText("Change theme")).closest(".command-palette-item"); + expect(item).toHaveTextContent(hint); + }); + + it("runs the toggle once and closes the palette", async () => { + const props = renderPalette({ theme: "system" }); + useUiStore.getState().setCommandPaletteOpen(true); + const user = userEvent.setup(); + + await user.click(await screen.findByText("Change theme")); + + expect(props.onToggleTheme).toHaveBeenCalledOnce(); + expect(screen.queryByPlaceholderText("Type a command or search chats…")).not.toBeInTheDocument(); + }); + + it.each(["dark", "light", "system", "appearance"])("is found by searching for %s", async (word) => { + renderPalette(); + useUiStore.getState().setCommandPaletteOpen(true); + const user = userEvent.setup(); + + await user.type(await screen.findByPlaceholderText("Type a command or search chats…"), word); + + expect(await screen.findByText("Change theme")).toBeVisible(); + expect(screen.queryByText("Open settings")).not.toBeInTheDocument(); + }); +}); diff --git a/frontend/src/features/command-palette/CommandPalette.tsx b/frontend/src/features/command-palette/CommandPalette.tsx index 5329c4f2..dbcfef42 100644 --- a/frontend/src/features/command-palette/CommandPalette.tsx +++ b/frontend/src/features/command-palette/CommandPalette.tsx @@ -1,7 +1,10 @@ -import { Command } from "cmdk"; +import { Command, defaultFilter } from "cmdk"; import { Moon, Plus, Settings, Sparkles } from "lucide-react"; +import { useMemo, useState } from "react"; import type { ChatSummary } from "../../../../contracts/cortex-api"; import { displayChatTitle } from "../../lib/chatTitle"; +import { nextThemePreference, THEME_LABELS, type ThemePreference } from "../../lib/theme"; +import { useDebouncedValue } from "../../hooks/useDebouncedValue"; import { useHotkey } from "../../hooks/useHotkey"; import { useUiStore } from "../../stores/useUiStore"; @@ -9,6 +12,8 @@ type Props = { chats: ChatSummary[]; localModels: readonly string[]; selectedModel: string | null; + /** The saved theme preference; the theme item names the one it would switch to. */ + theme: ThemePreference; onNewChat: () => void; onOpenSettings: () => void; onToggleTheme: () => void; @@ -16,12 +21,47 @@ type Props = { onSelectChat: (threadId: string) => void; }; +/** With nothing typed, the palette offers this many of the newest chats. */ const RECENT_CHAT_LIMIT = 8; +/** Once something is typed, every chat is searched but at most this many are shown. */ +const MAX_CHAT_RESULTS = 50; +/** + * How long typing must pause before the chats are searched again. Scoring a + * title costs microseconds, but a long history is thousands of them on every + * keystroke; the commands and models, a handful, still filter on each key. + */ +const CHAT_SEARCH_DELAY_MS = 120; + +const CHAT_VALUE_PREFIX = "chat:"; + +/** + * Chat rows carry the chat id as their value -- so two chats with the same + * title (every untitled chat is "New Chat") are two rows that can each be + * reached, instead of one highlight shared by both -- and the title as a + * keyword. Scoring the value would match against the id, so those rows are + * scored on their title alone; every other row is scored as cmdk would. + */ +function paletteFilter(value: string, search: string, keywords?: string[]): number { + if (value.startsWith(CHAT_VALUE_PREFIX)) return defaultFilter(keywords?.join(" ") ?? "", search); + return defaultFilter(value, search, keywords); +} + +/** The best matches for `query` among all chats, best first, newest first among equals, capped. */ +function searchChats(chats: readonly ChatSummary[], query: string): ChatSummary[] { + const matches: { chat: ChatSummary; score: number; position: number }[] = []; + chats.forEach((chat, position) => { + const score = defaultFilter(displayChatTitle(chat.title), query); + if (score > 0) matches.push({ chat, score, position }); + }); + matches.sort((a, b) => b.score - a.score || a.position - b.position); + return matches.slice(0, MAX_CHAT_RESULTS).map((match) => match.chat); +} export function CommandPalette({ chats, localModels, selectedModel, + theme, onNewChat, onOpenSettings, onToggleTheme, @@ -32,10 +72,10 @@ export function CommandPalette({ const setOpen = useUiStore((state) => state.setCommandPaletteOpen); useHotkey("k", true, () => setOpen(!open)); - const close = () => setOpen(false); - const run = (action: () => void) => () => { + // Every command closes the palette once it has run. + const execute = (action: () => void) => { action(); - close(); + setOpen(false); }; return ( @@ -43,28 +83,86 @@ export function CommandPalette({ open={open} onOpenChange={setOpen} label="Command palette" + filter={paletteFilter} className="command-palette-root" overlayClassName="command-palette-overlay" contentClassName="command-palette-content" > - + execute(onNewChat)} + onOpenSettings={() => execute(onOpenSettings)} + onToggleTheme={() => execute(onToggleTheme)} + onSelectModel={(model) => execute(() => onSelectModel(model))} + onSelectChat={(id) => execute(() => onSelectChat(id))} + /> + + ); +} + +/** + * What is inside the dialog. It exists only while the palette is open, so the + * text typed into it is forgotten when the palette closes and the next opening + * starts empty, without any code to reset it. + */ +function PaletteBody({ + chats, + localModels, + selectedModel, + theme, + onNewChat, + onOpenSettings, + onToggleTheme, + onSelectModel, + onSelectChat, +}: Props) { + const [query, setQuery] = useState(""); + const trimmed = query.trim(); + const settledQuery = useDebouncedValue(trimmed, CHAT_SEARCH_DELAY_MS); + // Clearing the box shows the recent chats at once; only searching waits. + const chatQuery = trimmed === "" ? "" : settledQuery; + const searching = chatQuery !== ""; + const visibleChats = useMemo( + () => (searching ? searchChats(chats, chatQuery) : chats.slice(0, RECENT_CHAT_LIMIT)), + [chats, chatQuery, searching], + ); + // Until the search has caught up with what was typed, the chat rows on screen + // are for an earlier query, and "No results" would be premature. + const searchPending = trimmed !== chatQuery; + + return ( + <> + - No results. + {!searchPending && No results.} - + - + - - {localModels.length > 0 && ( {localModels.map((model) => ( - onSelectModel(model))}> + onSelectModel(model)}> )} - {chats.length > 0 && ( - - {chats.slice(0, RECENT_CHAT_LIMIT).map((chat) => ( - onSelectChat(chat.id))}> - {displayChatTitle(chat.title)} - - ))} + {visibleChats.length > 0 && ( + + {visibleChats.map((chat) => { + const title = displayChatTitle(chat.title); + return ( + onSelectChat(chat.id)} + > + {title} + + ); + })} )} - + ); } diff --git a/frontend/src/hooks/useDebouncedValue.test.ts b/frontend/src/hooks/useDebouncedValue.test.ts new file mode 100644 index 00000000..703aeab4 --- /dev/null +++ b/frontend/src/hooks/useDebouncedValue.test.ts @@ -0,0 +1,52 @@ +import { act, renderHook } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { useDebouncedValue } from "./useDebouncedValue"; + +describe("useDebouncedValue", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("starts with the value it was given", () => { + const { result } = renderHook(() => useDebouncedValue("first", 100)); + expect(result.current).toBe("first"); + }); + + it("follows a change only after it has stopped changing for the delay", () => { + const { result, rerender } = renderHook(({ value }) => useDebouncedValue(value, 100), { initialProps: { value: "a" } }); + + rerender({ value: "ab" }); + act(() => { vi.advanceTimersByTime(99); }); + expect(result.current).toBe("a"); + + act(() => { vi.advanceTimersByTime(1); }); + expect(result.current).toBe("ab"); + }); + + it("restarts the wait for every change, and settles on the last one", () => { + const { result, rerender } = renderHook(({ value }) => useDebouncedValue(value, 100), { initialProps: { value: "a" } }); + + rerender({ value: "ab" }); + act(() => { vi.advanceTimersByTime(60); }); + rerender({ value: "abc" }); + act(() => { vi.advanceTimersByTime(60); }); + // 120ms since the first change, but only 60 since the latest. + expect(result.current).toBe("a"); + + act(() => { vi.advanceTimersByTime(40); }); + expect(result.current).toBe("abc"); + }); + + it("does not update after it is unmounted", () => { + const { rerender, unmount } = renderHook(({ value }) => useDebouncedValue(value, 100), { initialProps: { value: "a" } }); + rerender({ value: "b" }); + + unmount(); + + expect(vi.getTimerCount()).toBe(0); + }); +}); diff --git a/frontend/src/hooks/useDebouncedValue.ts b/frontend/src/hooks/useDebouncedValue.ts new file mode 100644 index 00000000..c5fad37e --- /dev/null +++ b/frontend/src/hooks/useDebouncedValue.ts @@ -0,0 +1,15 @@ +import { useEffect, useState } from "react"; + +/** + * `value`, but only once it has stopped changing for `delayMs`. For work that + * is too costly to redo on every keystroke; the input itself should keep using + * the live value so typing is never delayed. + */ +export function useDebouncedValue(value: T, delayMs: number): T { + const [debounced, setDebounced] = useState(value); + useEffect(() => { + const timer = window.setTimeout(() => setDebounced(value), delayMs); + return () => window.clearTimeout(timer); + }, [value, delayMs]); + return debounced; +} diff --git a/frontend/src/lib/theme.test.ts b/frontend/src/lib/theme.test.ts index 37c5ddd6..fd743e15 100644 --- a/frontend/src/lib/theme.test.ts +++ b/frontend/src/lib/theme.test.ts @@ -5,10 +5,12 @@ import { applyStoredTheme, applyThemePreference, DEFAULT_THEME_PREFERENCE, + nextThemePreference, readStoredThemePreference, resolveTheme, syncNativeTitleBar, THEME_BACKGROUNDS, + THEME_LABELS, THEME_STORAGE_KEY, watchSystemTheme, type ThemePreference, @@ -68,6 +70,30 @@ describe("resolveTheme", () => { }); }); +describe("nextThemePreference", () => { + it.each<[ThemePreference, ThemePreference]>([ + ["system", "light"], + ["light", "dark"], + ["dark", "system"], + ])("moves %s to %s", (current, next) => { + expect(nextThemePreference(current)).toBe(next); + }); + + it("visits all three preferences before repeating, so system is never skipped", () => { + const seen: ThemePreference[] = []; + let preference: ThemePreference = "dark"; + for (let step = 0; step < 6; step += 1) { + preference = nextThemePreference(preference); + seen.push(preference); + } + expect(seen).toEqual(["system", "light", "dark", "system", "light", "dark"]); + }); + + it("has a label for every preference", () => { + expect(THEME_LABELS).toEqual({ system: "System", light: "Light", dark: "Dark" }); + }); +}); + describe("the stored preference", () => { it("reads back only a known preference", () => { expect(readStoredThemePreference()).toBeNull(); diff --git a/frontend/src/lib/theme.ts b/frontend/src/lib/theme.ts index ab07af41..5b2a25a5 100644 --- a/frontend/src/lib/theme.ts +++ b/frontend/src/lib/theme.ts @@ -41,6 +41,24 @@ export function isThemePreference(value: unknown): value is ThemePreference { return value === "light" || value === "dark" || value === "system"; } +const THEME_CYCLE: readonly ThemePreference[] = ["system", "light", "dark"]; + +/** + * The preference a "change theme" command moves to: system, then light, then + * dark, then back to system. "System" is a choice of its own, not a shorthand + * for dark, so a person who follows Windows is not silently pinned to dark by + * the first press. + */ +export function nextThemePreference(preference: ThemePreference): ThemePreference { + return THEME_CYCLE[(THEME_CYCLE.indexOf(preference) + 1) % THEME_CYCLE.length]; +} + +export const THEME_LABELS: Record = { + system: "System", + light: "Light", + dark: "Dark", +}; + export function resolveTheme(preference: ThemePreference, systemPrefersDark: boolean): ResolvedTheme { if (preference === "system") return systemPrefersDark ? "dark" : "light"; return preference; From ffa1bcc397b88dc0e12833cdfcdcd5a77d5b003a Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Tue, 29 Sep 2026 04:29:24 -0400 Subject: [PATCH 08/13] refactor(frontend): remove dead client methods, an unused hook, orphan CSS, and a duplicate defaults table Deletes what has no call site: the useRafBatchedText hook (the stream hook has its own batching helper that is the one in use), the client methods diagnostics, shutdown, executionStatus, startCodeExecution, startScratchCompute, startRecipeImageTransform, stageAttachment and streamExecution with the two tests that only exercised them, and the CSS for .rounded-picker, .input-with-icon, .field-value, .gguf-download-status and .execution-task-capabilities. ChatPage no longer keeps a second copy of the six generation defaults; the composer already falls back to the one in lib/generationParams. streamJob's return type loses a stray void. A new test fails for any class in tokens.css that no component names (with a short, checked allow-list for classes the source builds or a library emits), so the next orphan is caught when it is made. Co-Authored-By: Claude Sonnet 5.5 --- frontend/src/api/client.test.ts | 72 ++---------------- frontend/src/api/client.ts | 63 +--------------- frontend/src/features/chat/ChatPage.tsx | 15 +--- frontend/src/hooks/useRafBatchedText.test.ts | 30 -------- frontend/src/hooks/useRafBatchedText.ts | 29 ------- frontend/src/styles/orphanCss.test.ts | 79 ++++++++++++++++++++ frontend/src/styles/tokens.css | 27 +------ 7 files changed, 94 insertions(+), 221 deletions(-) delete mode 100644 frontend/src/hooks/useRafBatchedText.test.ts delete mode 100644 frontend/src/hooks/useRafBatchedText.ts create mode 100644 frontend/src/styles/orphanCss.test.ts diff --git a/frontend/src/api/client.test.ts b/frontend/src/api/client.test.ts index b7a6ab5f..92b02ffe 100644 --- a/frontend/src/api/client.test.ts +++ b/frontend/src/api/client.test.ts @@ -1,4 +1,5 @@ -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, expectTypeOf, it, vi } from "vitest"; +import type { SSEEvent } from "../../../contracts/cortex-api"; import { ApiError, CortexApi, describeApiError, isAbortedError, isDefinitiveRejection } from "./client"; describe("CortexApi", () => { @@ -304,6 +305,12 @@ describe("CortexApi", () => { expect(terminal).toMatchObject({ kind: "completed", status: "succeeded" }); }); + it("types a job stream's result as a terminal event or null, never void", () => { + // A `void` in the union let a caller "use" a result that was never there. + // Compile-time only: tsc rejects this file if the return type drifts. + expectTypeOf>().toEqualTypeOf>(); + }); + it("returns no terminal event when a job stream closes while still active", async () => { const sse = 'id: 1\ndata: {"id":1,"job_id":"job-1","kind":"progress","status":"running"}\n\n'; const fetcher = vi.fn().mockResolvedValue(new Response(sse, { status: 200 })); @@ -347,69 +354,6 @@ describe("CortexApi", () => { const request = fetcher.mock.calls[0]?.[1] as RequestInit; expect(new Headers(request.headers).get("Authorization")).toBe("Bearer session-1"); }); - - it("starts a typed recipe request on the recipe route", async () => { - const fetcher = vi.fn().mockResolvedValue(new Response(JSON.stringify({ - job_id: "recipe-job", - request_id: "recipe-request", - profile: "recipe.image.v1", - status: "queued", - sequence: 1, - }), { status: 202, headers: { "Content-Type": "application/json" } })); - window.sessionStorage.setItem("cortex.session.token", "session-1"); - const api = new CortexApi("/api/v1", fetcher); - - await api.startRecipeImageTransform({ - request_id: "recipe-request", - source_artifact_id: "artifact-1", - plan: { - schema_version: "artifact.transform.v1", - input_artifact_id: "artifact-1", - steps: [{ op: "grayscale" }], - output_format: "png", - }, - }); - - expect(fetcher).toHaveBeenCalledWith( - "/api/v1/execution/recipe/image", - expect.objectContaining({ - method: "POST", - body: expect.stringContaining('"source_artifact_id":"artifact-1"'), - }), - ); - const request = fetcher.mock.calls[0]?.[1] as RequestInit; - expect(new Headers(request.headers).get("Authorization")).toBe("Bearer session-1"); - }); - - it("stages a bounded attachment through the attachment route", async () => { - const fetcher = vi.fn().mockResolvedValue(new Response(JSON.stringify({ - job_id: "attachment-job", - request_id: "attachment-request", - profile: "attachment.stage.v1", - status: "succeeded", - sequence: 1, - artifact_id: "artifact-1", - mime_type: "image/png", - size: 4, - sha256: "a".repeat(64), - expires_at: "2026-07-20T00:00:00Z", - }), { status: 201, headers: { "Content-Type": "application/json" } })); - window.sessionStorage.setItem("cortex.session.token", "session-1"); - const api = new CortexApi("/api/v1", fetcher); - - await api.stageAttachment({ - request_id: "attachment-request", - content_base64: "iVBORw==", - }); - - expect(fetcher).toHaveBeenCalledWith( - "/api/v1/execution/attachments", - expect.objectContaining({ - method: "POST", - body: JSON.stringify({ request_id: "attachment-request", content_base64: "iVBORw==" }), - }), - ); - }); }); describe("session invalidation guards", () => { diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index da3ef40e..4ed516f4 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -1,32 +1,21 @@ import type { AddMemoryRequest, - AttachmentStageAccepted, - AttachmentStageRequest, ChatAttachment, ChatAttachmentStageRequest, ChatGroup, ChatResponse, ChatSummary, - CodeExecutionAccepted, - CodeExecutionRequest, CodeExecutionSourceResponse, CreateChatGroupRequest, CreateChatRequest, - DiagnosticsResponse, MoveChatToGroupRequest, UpdateChatGroupRequest, - ExecutionSSEEvent, ExecutionApprovalDecisionRequest, ExecutionStatusResponse, ExecutionTaskListResponse, - RecipeImageTransformAccepted, - RecipeImageTransformRequest, - ScratchComputeAccepted, - ScratchComputeRequest, ForkRequest, GenerationEvent, GenerationRequest, - ShutdownResponse, HuggingFaceFileListResponse, JobAccepted, JobStatusResponse, @@ -520,10 +509,6 @@ export class CortexApi { return this.request(`/jobs/${encodeURIComponent(jobId)}`); } - diagnostics(): Promise { - return this.request("/diagnostics"); - } - checkModels(): Promise { return this.request("/jobs/models", { method: "POST" }); } @@ -563,46 +548,12 @@ export class CortexApi { return this.request(`/execution/tasks${query ? `?${query}` : ""}`); } - executionStatus(jobId: string): Promise { - return this.request(`/execution/${encodeURIComponent(jobId)}`); - } - - startCodeExecution(payload: CodeExecutionRequest): Promise { - return this.request("/execution/code", { - method: "POST", - body: JSON.stringify(payload), - }); - } - executionSource(jobId: string): Promise { return this.request( `/execution/${encodeURIComponent(jobId)}/source`, ); } - startScratchCompute(payload: ScratchComputeRequest): Promise { - return this.request("/execution/scratch", { - method: "POST", - body: JSON.stringify(payload), - }); - } - - startRecipeImageTransform( - payload: RecipeImageTransformRequest, - ): Promise { - return this.request("/execution/recipe/image", { - method: "POST", - body: JSON.stringify(payload), - }); - } - - stageAttachment(payload: AttachmentStageRequest): Promise { - return this.request("/execution/attachments", { - method: "POST", - body: JSON.stringify(payload), - }); - } - stageChatAttachment(payload: ChatAttachmentStageRequest): Promise { return this.request("/attachments", { method: "POST", @@ -639,19 +590,11 @@ export class CortexApi { ); } - streamExecution( - jobId: string, - onEvent: (event: ExecutionSSEEvent) => void, - options: { signal?: AbortSignal; afterEventId?: number } = {}, - ): Promise { - return this.streamEvents(`/execution/${encodeURIComponent(jobId)}/events`, onEvent, options).then(() => undefined); - } - streamJob( jobId: string, onEvent: (event: SSEEvent) => void, options: { signal?: AbortSignal; afterEventId?: number } = {}, - ): Promise { + ): Promise { return this.streamEvents(`/jobs/${encodeURIComponent(jobId)}/events`, onEvent, options); } @@ -681,10 +624,6 @@ export class CortexApi { }); } - shutdown(): Promise { - return this.request("/system/shutdown", { method: "POST" }); - } - private async request( path: string, options: RequestInit & { authenticated?: boolean } = {}, diff --git a/frontend/src/features/chat/ChatPage.tsx b/frontend/src/features/chat/ChatPage.tsx index 92cdebaf..c684fcc1 100644 --- a/frontend/src/features/chat/ChatPage.tsx +++ b/frontend/src/features/chat/ChatPage.tsx @@ -18,16 +18,6 @@ import { MessageList, type MessageListHandle } from "./MessageList"; import { PendingAssistantMessage } from "./PendingAssistantMessage"; import { ResponseAnnouncer } from "./ResponseAnnouncer"; -const DEFAULT_GENERATION_SETTINGS = { - temperature: 0.7, - top_p: 0.9, - top_k: 40, - repeat_penalty: 1.1, - num_ctx: 8192, - seed: -1, - system_instructions: "", -}; - type Props = { api: CortexApi; threadId: string | null; @@ -112,7 +102,10 @@ export function ChatPage({ }))); const generationOptionsByThread = useChatStore((state) => state.generationOptionsByThread); const setThreadOptions = useChatStore((state) => state.setThreadOptions); - const generationDefaults = useSettingsStore((state) => state.settings?.generation) ?? DEFAULT_GENERATION_SETTINGS; + // Until the settings load there are none; the composer then falls back to the + // one set of built-in defaults (lib/generationParams), instead of this page + // keeping a second copy of them. + const generationDefaults = useSettingsStore((state) => state.settings?.generation); // How the last generation ended, published by GenerationStreamHost -- which // keeps consuming the stream while this page is not mounted. Handled by the // two effects further down. diff --git a/frontend/src/hooks/useRafBatchedText.test.ts b/frontend/src/hooks/useRafBatchedText.test.ts deleted file mode 100644 index a259319f..00000000 --- a/frontend/src/hooks/useRafBatchedText.test.ts +++ /dev/null @@ -1,30 +0,0 @@ -import { act, renderHook, waitFor } from "@testing-library/react"; -import { describe, expect, it } from "vitest"; -import { useRafBatchedText } from "./useRafBatchedText"; - -describe("useRafBatchedText", () => { - it("coalesces rapid push() calls into a single state update per animation frame", async () => { - const { result } = renderHook(() => useRafBatchedText()); - expect(result.current.text).toBe(""); - - act(() => { - result.current.push("Hel"); - result.current.push("lo"); - result.current.push(" world"); - }); - - await waitFor(() => expect(result.current.text).toBe("Hello world")); - }); - - it("reset() replaces the buffer and text immediately", async () => { - const { result } = renderHook(() => useRafBatchedText()); - act(() => result.current.push("draft")); - await waitFor(() => expect(result.current.text).toBe("draft")); - - act(() => result.current.reset()); - expect(result.current.text).toBe(""); - - act(() => result.current.push("next")); - await waitFor(() => expect(result.current.text).toBe("next")); - }); -}); diff --git a/frontend/src/hooks/useRafBatchedText.ts b/frontend/src/hooks/useRafBatchedText.ts deleted file mode 100644 index 0f0d05c1..00000000 --- a/frontend/src/hooks/useRafBatchedText.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { useCallback, useEffect, useRef, useState } from "react"; - -/** Coalesces many rapid push() calls into at most one state update per animation frame. */ -export function useRafBatchedText(initial = "") { - const [text, setText] = useState(initial); - const bufferRef = useRef(initial); - const rafRef = useRef(null); - - const push = useCallback((chunk: string) => { - bufferRef.current += chunk; - if (rafRef.current == null) { - rafRef.current = requestAnimationFrame(() => { - setText(bufferRef.current); - rafRef.current = null; - }); - } - }, []); - - const reset = useCallback((value = "") => { - bufferRef.current = value; - setText(value); - }, []); - - useEffect(() => () => { - if (rafRef.current != null) cancelAnimationFrame(rafRef.current); - }, []); - - return { text, push, reset }; -} diff --git a/frontend/src/styles/orphanCss.test.ts b/frontend/src/styles/orphanCss.test.ts new file mode 100644 index 00000000..64fff81b --- /dev/null +++ b/frontend/src/styles/orphanCss.test.ts @@ -0,0 +1,79 @@ +// @vitest-environment node +import { describe, expect, it } from "vitest"; +import { parseRules, selectorsOf, tokensCss } from "../test/css"; + +/** + * A stylesheet only ever grows unless something notices a rule that nothing + * uses any more: a component is deleted or renamed, its CSS stays, and the next + * reader takes it for live. This reads every class selector in `tokens.css` and + * fails for one that no component names. + */ + +/** Every non-test component and module, as source text. */ +const sources = Object.values( + import.meta.glob(["../**/*.ts", "../**/*.tsx", "!../**/*.test.ts", "!../**/*.test.tsx", "!../test/**"], { + query: "?raw", + import: "default", + eager: true, + }) as Record, +); + +/** + * Classes that are real but never appear literally in the source, and why. + * Each pattern must still match something in the stylesheet, so the list cannot + * quietly outlive the thing it excuses. + */ +const ALLOWED: ReadonlyArray<{ pattern: RegExp; reason: string }> = [ + { pattern: /^hljs-/, reason: "token classes rehype-highlight adds to highlighted code" }, + { pattern: /^(class|function)_$/, reason: "highlight.js sub-scopes, as in .hljs-title.function_" }, + { pattern: /^(contains-task-list|task-list-item)$/, reason: "emitted by remark-gfm for a Markdown task list" }, + { pattern: /^message-(user|assistant|system)$/, reason: "built as message-${role} in MessageCard" }, + { pattern: /^model-picker-status-/, reason: "built from the runtime tone in LocalModelMenu" }, + { pattern: /^runtime-badge-/, reason: "built from the runtime tone in MessageComposer" }, + { pattern: /^toast-(error|info|success)$/, reason: "built as toast-${kind} in ToastProvider" }, +]; + +const escapeRegExp = (text: string) => text.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + +/** Whether a component names the class as a whole word (`chat-row` is not found in `chat-row-active`). */ +function isNamedInSource(className: string): boolean { + const whole = new RegExp(`(^|[^\\w-])${escapeRegExp(className)}(?![\\w-])`); + return sources.some((text) => whole.test(text)); +} + +const stylesheetClasses = (() => { + const names = new Set(); + for (const rule of parseRules(tokensCss)) { + for (const selector of selectorsOf(rule)) { + for (const match of selector.matchAll(/\.(-?[_a-zA-Z][\w-]*)/g)) names.add(match[1]); + } + } + return [...names].sort(); +})(); + +describe("tokens.css class selectors", () => { + it("reads the stylesheet and the source it is checked against", () => { + // Guards the test itself: an empty glob or an unreadable stylesheet would + // make the check below pass for the wrong reason. + expect(stylesheetClasses.length).toBeGreaterThan(100); + expect(sources.length).toBeGreaterThan(40); + expect(isNamedInSource("code-block")).toBe(true); + expect(isNamedInSource("definitely-not-a-class-anyone-uses")).toBe(false); + // Whole words only: a longer class does not vouch for its prefix. + expect(isNamedInSource("chat-row-selec")).toBe(false); + }); + + it("are each used by a component, or explained", () => { + const orphans = stylesheetClasses.filter( + (name) => !isNamedInSource(name) && !ALLOWED.some(({ pattern }) => pattern.test(name)), + ); + + expect(orphans, `no component uses these classes; delete their rules or add them to ALLOWED with a reason:\n${orphans.join("\n")}`).toEqual([]); + }); + + it("keeps every allowed pattern in use", () => { + const unused = ALLOWED.filter(({ pattern }) => !stylesheetClasses.some((name) => pattern.test(name))); + + expect(unused.map(({ reason }) => reason)).toEqual([]); + }); +}); diff --git a/frontend/src/styles/tokens.css b/frontend/src/styles/tokens.css index 8c2e0226..5c611dd0 100644 --- a/frontend/src/styles/tokens.css +++ b/frontend/src/styles/tokens.css @@ -222,7 +222,7 @@ a { color: inherit; text-decoration: none; } .danger-icon, .danger-action { color: var(--danger); } .field-label { display: grid; gap: 8px; color: var(--text-muted); font-size: 0.79rem; font-weight: 680; } -.field-label input, .field-label select, .field-label textarea, .inline-form input, .input-with-icon input { +.field-label input, .field-label select, .field-label textarea, .inline-form input { width: 100%; min-height: 44px; border: 1px solid var(--line); @@ -234,31 +234,13 @@ a { color: inherit; text-decoration: none; } transition: border-color 150ms ease, background 150ms ease, box-shadow 150ms ease; } .field-label textarea { min-height: 104px; resize: vertical; line-height: 1.55; } -.field-label input:focus, .field-label select:focus, .field-label textarea:focus, .inline-form input:focus, .input-with-icon:focus-within { +.field-label input:focus, .field-label select:focus, .field-label textarea:focus, .inline-form input:focus { border-color: color-mix(in srgb, var(--accent) 72%, var(--line)); outline: 0; box-shadow: 0 0 0 4px var(--accent-soft); } -.input-with-icon { display: flex; align-items: center; gap: 9px; min-height: 46px; padding: 0 13px; border: 1px solid var(--line); border-radius: 7px; background: var(--surface-soft); color: var(--text-muted); } -.input-with-icon input { min-height: 42px; padding: 0; border: 0; background: transparent; box-shadow: none; } .field-error { margin: 0; color: var(--danger); font-size: 0.8rem; line-height: 1.45; } -.rounded-picker { position: relative; z-index: 2; width: 100%; } -.rounded-picker-open { z-index: 20; } -.rounded-picker-trigger { display: flex; align-items: center; justify-content: space-between; gap: 12px; width: 100%; min-height: 48px; border: 1px solid var(--line); border-radius: 6px; padding: 8px 12px; background: var(--surface-soft); color: var(--text); text-align: left; transition: border-color 150ms ease, box-shadow 150ms ease, background 150ms ease; } -.rounded-picker-trigger:hover:not(:disabled), .rounded-picker-open .rounded-picker-trigger { border-color: color-mix(in srgb, var(--accent) 55%, var(--line)); background: var(--surface-raised); box-shadow: 0 0 0 4px var(--accent-soft); } -.rounded-picker-trigger > svg { flex: 0 0 auto; color: var(--accent); transition: transform 150ms ease; } -.rounded-picker-open .rounded-picker-trigger > svg { transform: rotate(180deg); } -.rounded-picker-selection, .rounded-picker-option > span { display: grid; min-width: 0; gap: 2px; } -.rounded-picker-selection strong, .rounded-picker-option strong { overflow: hidden; font-size: 0.82rem; font-weight: 690; text-overflow: ellipsis; white-space: nowrap; } -.rounded-picker-selection small, .rounded-picker-option small { color: var(--text-faint); font-size: 0.71rem; font-weight: 560; } -.rounded-picker-list { position: absolute; z-index: 30; top: calc(100% + 8px); left: 0; display: grid; width: 100%; max-height: 260px; overflow-y: auto; padding: 5px; border: 1px solid var(--line-strong); border-radius: 6px; background: var(--surface-elevated); box-shadow: var(--shadow-md); } -.rounded-picker-option { display: flex; align-items: center; justify-content: space-between; gap: 12px; width: 100%; min-height: 46px; border: 0; border-radius: 4px; padding: 8px 10px; background: transparent; color: var(--text); text-align: left; } -.rounded-picker-option:hover { background: var(--surface-hover); } -.rounded-picker-option:focus-visible { background: var(--surface-hover); outline: 0; box-shadow: inset 0 0 0 2px var(--accent); } -.rounded-picker-option-active { background: var(--accent-soft); color: var(--accent); } -.rounded-picker-option-active small { color: inherit; opacity: 0.78; } - .select-trigger { display: flex; align-items: center; justify-content: space-between; gap: 12px; width: 100%; min-height: 48px; border: 1px solid var(--line); border-radius: 6px; padding: 8px 12px; background: var(--surface-soft); color: var(--text); text-align: left; transition: border-color 150ms ease, box-shadow 150ms ease, background 150ms ease; } .select-trigger:hover:not([data-disabled]), .select-trigger[aria-expanded="true"] { border-color: color-mix(in srgb, var(--accent) 55%, var(--line)); background: var(--surface-raised); box-shadow: 0 0 0 4px var(--accent-soft); } .select-trigger-icon { flex: 0 0 auto; color: var(--accent); transition: transform 150ms ease; } @@ -637,7 +619,6 @@ a { color: inherit; text-decoration: none; } .composer-stop-control { background: var(--surface-soft); color: var(--text-muted); box-shadow: none; } .composer-stop-control:hover:not(:disabled) { background: var(--surface-hover); color: var(--text); } .composer-control-spinner { animation: spin var(--spinner-duration) linear infinite; } -.execution-task-capabilities { color: var(--text-faint); font-size: var(--text-2xs); } .execution-task-warning { display: flex; align-items: flex-start; gap: 5px; padding: 6px 8px; border-radius: var(--radius-xs); background: var(--warning-soft); color: var(--warning); font-size: var(--text-2xs); font-weight: 620; line-height: 1.35; } .execution-task-code-content { display: grid; min-width: 0; flex: 1; gap: 9px; } .execution-task-code-heading { display: flex; align-items: flex-start; justify-content: space-between; gap: 10px; } @@ -760,7 +741,6 @@ a { color: inherit; text-decoration: none; } .translation-install-status-spinner { width: 14px; height: 14px; flex: 0 0 auto; border-width: 2px; border-color: color-mix(in srgb, currentColor 28%, transparent); border-top-color: currentColor; } .translation-install-status-copy { min-width: 0; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } .translation-install-status > strong { margin-left: auto; font-variant-numeric: tabular-nums; } -.field-value { float: right; color: var(--accent); } input[type="range"] { width: 100%; accent-color: var(--accent); } /* Sliders ------------------------------------------------------------------- @@ -953,7 +933,6 @@ input[type="range"] { width: 100%; accent-color: var(--accent); } .gguf-download-form { display: grid; gap: 10px; } .gguf-download-source-toggle { display: inline-flex; gap: 6px; } .gguf-download-fields { display: grid; gap: 8px; grid-template-columns: repeat(auto-fit, minmax(180px, 1fr)); } -.gguf-download-status { color: var(--text-muted); font-size: 0.76rem; } .model-missing { display: grid; gap: 9px; margin: 16px 0; padding: 13px; border: 1px solid color-mix(in srgb, var(--danger) 30%, transparent); border-left: 3px solid var(--danger); border-radius: 5px; background: var(--danger-soft); color: var(--danger); font-size: 0.8rem; } .model-optional-missing { border-color: color-mix(in srgb, var(--accent) 28%, var(--line)); background: var(--accent-soft); color: var(--text-muted); } .model-progress { display: grid; gap: 7px; margin-top: 18px; padding: 13px; border: 1px solid var(--line); border-radius: 5px; background: transparent; color: var(--text-muted); font-size: 0.78rem; } @@ -1139,7 +1118,6 @@ kbd { display: inline-block; border: 1px solid var(--line-strong); border-radius /* Focus: fields and wrappers whose ring is a box-shadow. */ :focus-visible, .field-label input:focus, .field-label select:focus, .field-label textarea:focus, .inline-form input:focus, - .input-with-icon:focus-within, .sidebar-search:focus-within, .composer-surface:has(textarea:focus-visible), .model-picker-search:focus-within, @@ -1153,7 +1131,6 @@ kbd { display: inline-block; border: 1px solid var(--line-strong); border-radius /* Focus and highlight inside a menu, listbox or palette: the row is only ever tinted, so outline it. Inset, because these sit in clipped popups. */ - .rounded-picker-option:focus-visible, .select-item[data-highlighted], .chat-row-menu-item:focus-visible, .model-picker-option[data-highlighted], From 3deac43dce5968aaf1dd7a72beae7e047f0af422 Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Tue, 29 Sep 2026 04:33:03 -0400 Subject: [PATCH 09/13] fix(tools): keep the showcase capture working with the three-way theme item The command palette item is now Change theme and cycles system, light, dark, so the screenshot script that pressed Toggle theme once to reach light and once to return to dark no longer found the item and would have ended on light. It now presses Change theme once (system, which its light colour-scheme emulation resolves to light) and twice to return to dark. Not run here: it needs the staged showcase server. Co-Authored-By: Claude Sonnet 5.5 --- tools/screenshots/capture_showcase.mjs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tools/screenshots/capture_showcase.mjs b/tools/screenshots/capture_showcase.mjs index 347260bd..3d9104ce 100644 --- a/tools/screenshots/capture_showcase.mjs +++ b/tools/screenshots/capture_showcase.mjs @@ -282,17 +282,21 @@ await settingsSection("System", "settings-system"); // --------------------------------------------------------------------------- // Light theme // --------------------------------------------------------------------------- -const toggleTheme = async () => { +// One press of "Change theme" moves the preference on: system, light, dark, and +// round again. From dark, one press is "system" (which the light colour-scheme +// emulation below turns into the light theme) and two more return to dark. The +// item also carries a "Next: ..." hint, so it is matched by a fragment of its text. +const changeTheme = async () => { await page.keyboard.press("Control+k"); await page.waitForTimeout(400); - await page.getByText("Toggle theme", { exact: true }).click(); + await page.getByText("Change theme").click(); await page.waitForTimeout(800); }; await page.goto(`${base}/`); await page.getByLabel("Message Cortex").waitFor({ state: "visible" }); await page.waitForTimeout(400); -await toggleTheme(); +await changeTheme(); // dark -> system await page.emulateMedia({ colorScheme: "light" }); await openChat("Reading a 4 GB CSV without exhausting memory"); @@ -307,7 +311,8 @@ await shot("command-palette-light"); await closeOverlay(); // Back to dark for the remaining shots. -await toggleTheme(); +await changeTheme(); // system -> light +await changeTheme(); // light -> dark await page.emulateMedia({ colorScheme: "dark" }); await page.waitForTimeout(500); From 3b629cfe2f73d5f9fff5c0e7c816372c1be521b3 Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Tue, 29 Sep 2026 04:38:51 -0400 Subject: [PATCH 10/13] fix(frontend): check the GGUF runtime once after each generation ends With the runtime polled every fifteen seconds while it is idle or ready, a reply that finished between two polls left the model picker saying the model was not loaded until the next one. A generation that ends now triggers one status check (only while a GGUF model is selected), which also keeps the one-request-per-poll accounting intact: starting a generation only changes the interval. Co-Authored-By: Claude Sonnet 5.5 --- frontend/src/app/App.pollRender.test.tsx | 55 ++++++++++++++++++++++-- frontend/src/app/App.tsx | 9 ++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/frontend/src/app/App.pollRender.test.tsx b/frontend/src/app/App.pollRender.test.tsx index 664a7732..1b8bfb4d 100644 --- a/frontend/src/app/App.pollRender.test.tsx +++ b/frontend/src/app/App.pollRender.test.tsx @@ -3,6 +3,7 @@ import type { ComponentProps } from "react"; import { afterEach, describe, expect, it, vi } from "vitest"; import type { ExecutionTaskSummary } from "../../../contracts/cortex-api"; import { CortexApi } from "../api/client"; +import { useChatStore } from "../stores/useChatStore"; import { useModelStore } from "../stores/useModelStore"; import { App } from "./App"; import { ToastProvider } from "./ToastProvider"; @@ -41,17 +42,20 @@ type Backend = { llamacpp: Record; }; -/** A workspace with the execution tray and a GGUF model selected, so both polls are live. */ -function backend(state: Backend) { +/** A workspace with the execution tray and a GGUF model (unless another is given) selected, so both polls are live. */ +function backend(state: Backend, chatModel = "gguf:demo.Q4_K_M.gguf") { return vi.fn(async (input) => { const url = String(input); if (url.endsWith("/system")) { return respond({ status: "ok", preview: true, session_required: true, execution_preview_available: true, started_at: "2026-07-21T18:00:00Z", llamacpp: state.llamacpp }); } if (url.includes("/execution/tasks")) return respond({ tasks: state.tasks }); + // A generation the store is told about stays open: the stream host attaches + // to it, and a job that answered 404 would be ended by the host at once. + if (url.includes("/generations/") && url.endsWith("/events")) return new Promise(() => undefined); if (url.endsWith("/chat-groups")) return respond([]); if (url.endsWith("/chats")) return respond([]); - if (url.endsWith("/settings")) return respond({ settings: { models: { chat: "gguf:demo.Q4_K_M.gguf", title: null }, appearance: { theme: "dark" } } }); + if (url.endsWith("/settings")) return respond({ settings: { models: { chat: chatModel, title: null }, appearance: { theme: "dark" } } }); if (url.endsWith("/models")) return respond({ required_models: [], optional_models: [], installed_models: ["gguf:demo.Q4_K_M.gguf"], models: [{ name: "gguf:demo.Q4_K_M.gguf" }], connection: { success: true, status: "connected", message: "Ready" } }); return respond({ detail: "Unexpected test route." }, 404); }); @@ -187,5 +191,50 @@ describe("App polling", () => { restore(); } }); + + it("checks the runtime once when a generation ends, instead of waiting for the slow poll", async () => { + window.sessionStorage.setItem("cortex.session.token", "local-session"); + const state: Backend = { + tasks: [], + llamacpp: { state: "idle", binary_present: true, models_directory: "C:\\models" }, + }; + const fetcher = backend(state); + render(); + expect(await screen.findByRole("heading", { name: "New thread" })).toBeVisible(); + await pollOnce(fetcher); + expect(useModelStore.getState().llamacppStatus?.state).toBe("idle"); + + // The runtime loaded the model while the reply was being written. + state.llamacpp = { state: "ready", binary_present: true, loaded_model: "gguf:demo.Q4_K_M.gguf", models_directory: "C:\\models" }; + const systemBefore = callsTo(fetcher, "/system"); + act(() => useChatStore.getState().beginGeneration("job-1", "thread-a")); + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 30)); }); + // Starting one changes the poll's interval, which costs no request of its own. + expect(callsTo(fetcher, "/system")).toBe(systemBefore); + + act(() => useChatStore.getState().endGeneration("job-1")); + + await waitFor(() => expect(useModelStore.getState().llamacppStatus?.state).toBe("ready")); + expect(callsTo(fetcher, "/system")).toBe(systemBefore + 1); + }); + + it("does not check the runtime for a generation that ends when no GGUF model is selected", async () => { + window.sessionStorage.setItem("cortex.session.token", "local-session"); + const state: Backend = { + tasks: [], + llamacpp: { state: "idle", binary_present: true, models_directory: "C:\\models" }, + }; + const fetcher = backend(state, "local-chat:7b"); + render(); + expect(await screen.findByRole("heading", { name: "New thread" })).toBeVisible(); + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 60)); }); + const systemBefore = callsTo(fetcher, "/system"); + + act(() => useChatStore.getState().beginGeneration("job-1", "thread-a")); + act(() => useChatStore.getState().endGeneration("job-1")); + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 60)); }); + + expect(callsTo(fetcher, "/system")).toBe(systemBefore); + }); }); }); diff --git a/frontend/src/app/App.tsx b/frontend/src/app/App.tsx index 5abee95e..a38de521 100644 --- a/frontend/src/app/App.tsx +++ b/frontend/src/app/App.tsx @@ -486,6 +486,15 @@ function AuthenticatedWorkspace({ api, onSessionExpired }: { api: CortexApi; onS selectedModelIsGGUF, { backoff: true }, ); + // A reply that finishes between two slow polls would leave the model picker + // saying "not loaded yet" for the model it just used, so one check follows + // every generation that ends. + const wasGenerationActiveRef = useRef(generationActive); + useEffect(() => { + const ended = wasGenerationActiveRef.current && !generationActive; + wasGenerationActiveRef.current = generationActive; + if (ended && selectedModelIsGGUF) refreshLlamacppStatus().catch(() => undefined); + }, [generationActive, selectedModelIsGGUF, refreshLlamacppStatus]); const visibleExecutionTasks = system?.execution_preview_available ? executionTasks.filter((task) => shouldShowExecutionTask(task, system.started_at)) From 9dda247f19367ced457db4349f99d4f42d17c8d4 Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Tue, 29 Sep 2026 04:52:11 -0400 Subject: [PATCH 11/13] test(a11y): pin that the app's own dialog and palette keep Escape while a response runs The page-level Escape rule is tested with synthetic dialog, menu and listbox nodes; this adds a test with the real shortcuts dialog (Base UI) and the real command palette (cmdk), checking that Escape closes each without stopping the response and that the same key stops it once both are closed. Co-Authored-By: Claude Sonnet 5.5 --- .../src/features/chat/ChatPage.a11y.test.tsx | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/frontend/src/features/chat/ChatPage.a11y.test.tsx b/frontend/src/features/chat/ChatPage.a11y.test.tsx index e39b4a35..28bdf9ef 100644 --- a/frontend/src/features/chat/ChatPage.a11y.test.tsx +++ b/frontend/src/features/chat/ChatPage.a11y.test.tsx @@ -5,6 +5,9 @@ import type { ComponentProps } from "react"; import type { ChatResponse } from "../../../../contracts/cortex-api"; import { CortexApi } from "../../api/client"; import { useChatStore } from "../../stores/useChatStore"; +import { useUiStore } from "../../stores/useUiStore"; +import { CommandPalette } from "../command-palette/CommandPalette"; +import { ShortcutsHelpDialog } from "../command-palette/ShortcutsHelpDialog"; import { ChatPage } from "./ChatPage"; import { GenerationStreamHost } from "./GenerationStreamHost"; @@ -182,6 +185,61 @@ describe("ChatPage Escape", () => { expect(cancelGeneration).not.toHaveBeenCalled(); }); + it("leaves Escape to the real shortcuts dialog and the real command palette", async () => { + // The synthetic roles above pin the rule; this pins that the app's own + // Base UI dialog and its cmdk palette actually carry one of those roles. + const user = userEvent.setup(); + const { api, cancelGeneration } = harness(); + render( + <> + true} + onRescanModels={async () => undefined} + onThreadCreated={vi.fn()} + onForked={vi.fn()} + /> + + + , + ); + await sendPrompt(user); + await focusThePage(); + + act(() => useUiStore.getState().setShortcutsDialogOpen(true)); + expect(await screen.findByRole("dialog", { name: "Keyboard shortcuts" })).toBeVisible(); + await user.keyboard("{Escape}"); + await waitFor(() => expect(screen.queryByRole("dialog", { name: "Keyboard shortcuts" })).not.toBeInTheDocument()); + expect(cancelGeneration).not.toHaveBeenCalled(); + + act(() => useUiStore.getState().setCommandPaletteOpen(true)); + expect(await screen.findByRole("dialog", { name: "Command palette" })).toBeVisible(); + await user.keyboard("{Escape}"); + await waitFor(() => expect(screen.queryByRole("dialog", { name: "Command palette" })).not.toBeInTheDocument()); + expect(cancelGeneration).not.toHaveBeenCalled(); + + // With both closed, the same key now stops the response. + (document.activeElement as HTMLElement | null)?.blur(); + await user.keyboard("{Escape}"); + await waitFor(() => expect(cancelGeneration).toHaveBeenCalledTimes(1)); + }); + it("stops again once the overlay has closed", async () => { const user = userEvent.setup(); const { api, cancelGeneration } = harness(); From b28320a5ea0879e2339f3a931c19253bde51d57c Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Tue, 29 Sep 2026 06:17:22 -0400 Subject: [PATCH 12/13] fix(frontend): keep the page-wide Escape out of a focused toast Main's Undo toast holds a delete that only goes to the backend when the toast closes, so a keypress that closes or acts on it is not free. The page-wide Escape (stop a response from anywhere) now leaves the key alone while focus is inside the toast region (data-keeps-escape), as it already does for dialogs, menus, listboxes and text fields. Tests: the hook ignores a notification target and takes Escape elsewhere; with the real ToastProvider beside ChatPage, Escape on a focused Undo button neither stops the response nor touches the toast, and stops it from anywhere else; the response announcer and an error or status toast are each read once. Also waits for the composer to take focus back before the text-field Escape test focuses its own field, which raced under load. Co-Authored-By: Claude Sonnet 5.5 --- frontend/src/app/ToastProvider.tsx | 2 +- .../src/features/chat/ChatPage.a11y.test.tsx | 99 +++++++++++++++++++ frontend/src/hooks/usePageEscape.test.ts | 16 +++ frontend/src/hooks/usePageEscape.ts | 11 +++ 4 files changed, 127 insertions(+), 1 deletion(-) diff --git a/frontend/src/app/ToastProvider.tsx b/frontend/src/app/ToastProvider.tsx index 0d80baf9..d4705c8d 100644 --- a/frontend/src/app/ToastProvider.tsx +++ b/frontend/src/app/ToastProvider.tsx @@ -16,7 +16,7 @@ export function ToastProvider({ children }: { children: ReactNode }) { the rest), so the region itself announces nothing and a toast is never read twice. */} -
+
{toasts.map((toast) => )}
diff --git a/frontend/src/features/chat/ChatPage.a11y.test.tsx b/frontend/src/features/chat/ChatPage.a11y.test.tsx index 28bdf9ef..cb832592 100644 --- a/frontend/src/features/chat/ChatPage.a11y.test.tsx +++ b/frontend/src/features/chat/ChatPage.a11y.test.tsx @@ -5,6 +5,7 @@ import type { ComponentProps } from "react"; import type { ChatResponse } from "../../../../contracts/cortex-api"; import { CortexApi } from "../../api/client"; import { useChatStore } from "../../stores/useChatStore"; +import { ToastProvider } from "../../app/ToastProvider"; import { useUiStore } from "../../stores/useUiStore"; import { CommandPalette } from "../command-palette/CommandPalette"; import { ShortcutsHelpDialog } from "../command-palette/ShortcutsHelpDialog"; @@ -263,6 +264,9 @@ describe("ChatPage Escape", () => { const { api, cancelGeneration } = harness(); renderChat(api); await sendPrompt(user); + // Sending hands focus back to the composer on the next frame; focusing the + // field before that would just be undone (see focusThePage). + await waitFor(() => expect(document.activeElement).toBe(screen.getByLabelText("Message Cortex"))); const field = document.createElement("input"); field.setAttribute("data-test-overlay", ""); document.body.appendChild(field); @@ -433,3 +437,98 @@ describe("ChatPage response announcements", () => { expect(announcer()).toBeEmptyDOMElement(); }); }); + +describe("ChatPage beside the app's toasts", () => { + afterEach(() => { + act(() => { + for (const toast of useUiStore.getState().toasts) useUiStore.getState().dismissToast(toast.id); + }); + }); + + function renderChatWithToasts(api: CortexApi) { + return render( + + true} + onRescanModels={async () => undefined} + onThreadCreated={vi.fn()} + onForked={vi.fn()} + /> + , + ); + } + + it("leaves Escape to the Undo button of a toast that has focus, and stops the response from anywhere else", async () => { + const user = userEvent.setup(); + const { api, cancelGeneration } = harness(); + renderChatWithToasts(api); + await sendPrompt(user); + await focusThePage(); + const undo = vi.fn(); + act(() => { useUiStore.getState().notify("Chat deleted.", "success", { action: { label: "Undo", onAction: undo }, durationMs: 60_000 }); }); + const undoButton = await screen.findByRole("button", { name: "Undo" }); + undoButton.focus(); + expect(document.activeElement).toBe(undoButton); + + await user.keyboard("{Escape}"); + + // The response keeps running, and the toast (and its way back) is untouched. + expect(cancelGeneration).not.toHaveBeenCalled(); + expect(screen.getByRole("button", { name: "Undo" })).toBeInTheDocument(); + expect(undo).not.toHaveBeenCalled(); + + undoButton.blur(); + await user.keyboard("{Escape}"); + + await waitFor(() => expect(cancelGeneration).toHaveBeenCalledTimes(1)); + expect(screen.getByRole("button", { name: "Undo" })).toBeInTheDocument(); + }); + + it("keeps the response announcer and an error toast to one reading each", async () => { + const user = userEvent.setup(); + const { api, finishWith } = harness(); + renderChatWithToasts(api); + await sendPrompt(user); + await waitFor(() => expect(announcer()).toHaveTextContent(/^Response started$/)); + + act(() => { useUiStore.getState().notify("Could not save memory.", "error"); }); + + // The toast is its own alert; the announcer neither repeats it nor changes. + const toastAlert = await screen.findByRole("alert"); + expect(toastAlert).toHaveTextContent("Could not save memory."); + expect(announcer()).toHaveTextContent(/^Response started$/); + + await finishWith("generation.failed", { message: "The model failed." }); + + // The failure is said briefly by the announcer and in full by the composer's alert; + // the toast is not read again, and the announcer is a polite status, never an alert. + await waitFor(() => expect(announcer()).toHaveTextContent(/^Response failed$/)); + const alerts = await screen.findAllByRole("alert"); + expect(alerts).toHaveLength(2); + expect(alerts.filter((alert) => alert.textContent?.includes("Could not save memory."))).toHaveLength(1); + expect(alerts.filter((alert) => alert.textContent?.includes("The model failed."))).toHaveLength(1); + expect(alerts).not.toContain(announcer()); + expect(screen.getAllByText("Could not save memory.")).toHaveLength(1); + expect(screen.getAllByText(/Response failed/)).toHaveLength(1); + }); + + it("does not read a status toast into the announcer either", async () => { + const user = userEvent.setup(); + const { api, finishWith } = harness(); + renderChatWithToasts(api); + await sendPrompt(user); + + await finishWith("generation.completed", { assistant_message_id: "assistant-1" }, savedAnswer({ eval_count: 5 })); + act(() => { useUiStore.getState().notify("Memory saved.", "success"); }); + + await waitFor(() => expect(announcer()).toHaveTextContent(/^Response complete, 5 tokens$/)); + expect(screen.getAllByText("Memory saved.")).toHaveLength(1); + }); +}); diff --git a/frontend/src/hooks/usePageEscape.test.ts b/frontend/src/hooks/usePageEscape.test.ts index e75ac52b..37baff94 100644 --- a/frontend/src/hooks/usePageEscape.test.ts +++ b/frontend/src/hooks/usePageEscape.test.ts @@ -98,6 +98,22 @@ describe("usePageEscape", () => { expect(handler).not.toHaveBeenCalled(); }); + it("leaves Escape to a notification whose button has focus, and takes it again elsewhere", () => { + const handler = vi.fn(); + renderHook(() => usePageEscape(handler, true)); + const notification = attach(document.createElement("div")); + notification.setAttribute("data-keeps-escape", ""); + const undo = notification.appendChild(document.createElement("button")); + + const inside = press({}, undo); + + expect(handler).not.toHaveBeenCalled(); + expect(inside.defaultPrevented).toBe(false); + + press({}, attach(document.createElement("button"))); + expect(handler).toHaveBeenCalledTimes(1); + }); + it("ignores auto-repeat, composition, and modifiers", () => { const handler = vi.fn(); renderHook(() => usePageEscape(handler, true)); diff --git a/frontend/src/hooks/usePageEscape.ts b/frontend/src/hooks/usePageEscape.ts index 6d7fac23..6e4db110 100644 --- a/frontend/src/hooks/usePageEscape.ts +++ b/frontend/src/hooks/usePageEscape.ts @@ -9,6 +9,15 @@ import { isEditableTarget } from "./useHotkey"; */ const OVERLAY_SELECTOR = '[role="dialog"], [role="alertdialog"], [role="menu"], [role="listbox"]'; +/** + * A notification (a toast) whose own buttons -- Undo on a deleted chat, + * Dismiss on an error -- have focus. Escape there is not a request to stop the + * response behind it, and a toast has no cancel of its own to give the key to: + * closing one would run whatever it was holding back, such as the delete that + * Undo is still able to reverse. So the key is left alone. + */ +const NOTIFICATION_SELECTOR = "[data-keeps-escape]"; + /** * Runs `onEscape` when Escape is pressed on the page itself while `enabled`. * @@ -19,6 +28,7 @@ const OVERLAY_SELECTOR = '[role="dialog"], [role="alertdialog"], [role="menu"], * composer's own Escape handling keeps this from running twice; * - an overlay is open, whose Escape closes it; * - focus is in some other text field, where Escape cancels the edit; + * - focus is inside a notification (`data-keeps-escape`), see above; * - it is an auto-repeat, an IME composition, or carries a modifier. * * The handler is held in a ref, so a new function each render does not remove @@ -38,6 +48,7 @@ export function usePageEscape(onEscape: () => void, enabled: boolean): void { if (event.repeat || event.isComposing) return; if (event.ctrlKey || event.metaKey || event.altKey || event.shiftKey) return; if (isEditableTarget(event.target)) return; + if (event.target instanceof Element && event.target.closest(NOTIFICATION_SELECTOR)) return; if (document.querySelector(OVERLAY_SELECTOR)) return; event.preventDefault(); handlerRef.current(); From 78dfa0294e8f09c4ac7da7a3d91c2bc13058014b Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Tue, 29 Sep 2026 06:17:22 -0400 Subject: [PATCH 13/13] test(frontend): pin that the command palette hides a chat pending deletion The palette searches every chat; it must agree with the sidebar while a deleted chat waits out its Undo window. Covers the recent list and search during the window, the chat coming back after Undo, and staying gone after the delete is sent. Co-Authored-By: Claude Sonnet 5.5 --- frontend/src/app/App.paletteDelete.test.tsx | 147 ++++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 frontend/src/app/App.paletteDelete.test.tsx diff --git a/frontend/src/app/App.paletteDelete.test.tsx b/frontend/src/app/App.paletteDelete.test.tsx new file mode 100644 index 00000000..f584d23c --- /dev/null +++ b/frontend/src/app/App.paletteDelete.test.tsx @@ -0,0 +1,147 @@ +import { act, fireEvent, render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { App } from "./App"; +import { CortexApi } from "../api/client"; +import { useUiStore } from "../stores/useUiStore"; +import { ToastProvider } from "./ToastProvider"; + +/** + * The command palette searches every chat, and a chat that was just deleted is + * hidden for the length of its Undo window without having been removed from the + * backend yet. The palette must agree with the sidebar about which chats exist. + */ + +const json = (body: unknown, status = 200) => new Response(JSON.stringify(body), { + status, + headers: { "Content-Type": "application/json" }, +}); + +const CHATS = [ + { id: "chat-a", title: "Alpha plan", timestamp: "2026-01-02T00:00:00Z" }, + { id: "chat-b", title: "Beta notes", timestamp: "2026-01-01T00:00:00Z" }, +]; + +/** The undo window in App.tsx. */ +const UNDO_WINDOW_MS = 6000; + +describe("command palette with a chat pending deletion", () => { + beforeAll(async () => { + await Promise.all([ + import("../features/chat/ChatPage"), + import("../features/settings/SettingsPanel"), + ]); + }, 120_000); + + afterEach(() => { + vi.useRealTimers(); + act(() => { useUiStore.setState({ toasts: [], commandPaletteOpen: false }); }); + window.sessionStorage.clear(); + window.history.replaceState({}, "", "/"); + }); + + function workspace() { + const deletes: string[] = []; + const fetcher = vi.fn(async (input, init) => { + const url = String(input); + const method = init?.method ?? "GET"; + const chatMatch = /\/chats\/([^/?]+)$/.exec(url); + if (chatMatch && method === "DELETE") { + deletes.push(decodeURIComponent(chatMatch[1])); + return new Response(null, { status: 204 }); + } + if (chatMatch && method === "GET") { + const chat = CHATS.find((candidate) => candidate.id === chatMatch[1]); + return chat ? json({ ...chat, revision: 1, messages: [] }) : json({ detail: "Not found." }, 404); + } + if (url.endsWith("/system")) return json({ status: "ok", preview: true, session_required: true, started_at: "2026-07-21T18:00:00Z" }); + if (url.endsWith("/chat-groups")) return json([]); + if (url.endsWith("/chats")) return json(CHATS); + if (url.endsWith("/settings")) return json({ settings: { models: { chat: null, title: null }, appearance: { theme: "dark" } } }); + if (url.endsWith("/models")) return json({ required_models: [], optional_models: [], installed_models: [], connection: { success: true, status: "connected", message: "Ready" } }); + return json({ detail: "Unexpected test route." }, 404); + }); + return { fetcher, deletes }; + } + + async function openWorkspace(fetcher: typeof fetch) { + window.sessionStorage.setItem("cortex.session.token", "local-session"); + window.history.replaceState({}, "", "/chat/new"); + render(); + expect(await screen.findByRole("button", { name: "Delete Alpha plan" }, { timeout: 10_000 })).toBeInTheDocument(); + } + + /** Confirm the dialog, then switch to fake timers so the undo window can be stepped through. */ + async function deleteAlphaAndControlTime() { + const user = userEvent.setup(); + await user.click(screen.getByRole("button", { name: "Delete Alpha plan" })); + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout", "Date"] }); + fireEvent.click(within(screen.getByRole("alertdialog")).getByRole("button", { name: "Delete chat" })); + await act(async () => { await vi.advanceTimersByTimeAsync(0); }); + } + + const passTime = (ms: number) => act(async () => { await vi.advanceTimersByTimeAsync(ms); }); + const toastRegion = () => within(document.querySelector(".toast-region") as HTMLElement); + + /** The palette's own rows: the sidebar lists the same titles outside the dialog. */ + const palette = () => within(screen.getByRole("dialog", { name: "Command palette" })); + + async function openPalette() { + act(() => useUiStore.getState().setCommandPaletteOpen(true)); + await passTime(0); + return palette(); + } + + async function searchPalette(text: string) { + fireEvent.change(palette().getByPlaceholderText(/search chats/i), { target: { value: text } }); + // Past the palette's typing pause, so the chat search has caught up. + await passTime(200); + } + + const closePalette = () => act(() => useUiStore.getState().setCommandPaletteOpen(false)); + + it("leaves the hidden chat out of the recent chats and out of search, and offers it again after Undo", async () => { + const { fetcher, deletes } = workspace(); + await openWorkspace(fetcher); + + await deleteAlphaAndControlTime(); + expect(toastRegion().getByRole("button", { name: "Undo" })).toBeInTheDocument(); + + const recent = await openPalette(); + expect(recent.getByText("Beta notes")).toBeInTheDocument(); + expect(recent.queryByText("Alpha plan")).not.toBeInTheDocument(); + + await searchPalette("Alpha"); + expect(palette().queryByText("Alpha plan")).not.toBeInTheDocument(); + expect(palette().getByText("No results.")).toBeInTheDocument(); + closePalette(); + await passTime(0); + + fireEvent.click(toastRegion().getByRole("button", { name: "Undo" })); + await passTime(0); + + const restored = await openPalette(); + expect(restored.getByText("Alpha plan")).toBeInTheDocument(); + expect(restored.getByText("Beta notes")).toBeInTheDocument(); + await searchPalette("Alpha"); + expect(palette().getByText("Alpha plan")).toBeInTheDocument(); + expect(palette().queryByText("Beta notes")).not.toBeInTheDocument(); + expect(deletes).toEqual([]); + }); + + it("still leaves the chat out once the Undo window has closed and the delete was sent", async () => { + const { fetcher, deletes } = workspace(); + await openWorkspace(fetcher); + + await deleteAlphaAndControlTime(); + await passTime(UNDO_WINDOW_MS); + await passTime(50); + expect(deletes).toEqual(["chat-a"]); + + const recent = await openPalette(); + expect(recent.getByText("Beta notes")).toBeInTheDocument(); + expect(recent.queryByText("Alpha plan")).not.toBeInTheDocument(); + await searchPalette("Alpha"); + expect(palette().queryByText("Alpha plan")).not.toBeInTheDocument(); + }); +});