From ede189b76a0a927cfdf59e5d5d932982886fc5b3 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Tue, 11 Aug 2026 09:49:19 +0000 Subject: [PATCH 1/7] feat: render chat summaries as a headline plus bullets The whole-chat summary shown in the Summary tab rendered as a single prose blob. Three layers each independently prevented structure: the generation prompt banned all markdown, the normalizer collapsed every newline into a space, and the panel rendered the result into one plain

. Validation was also looser than the prompt asked, permitting up to 1000 runes and six sentences in a 448px column. Generation now returns a structured headline plus 2-4 bullets, which a shared serializer renders to markdown stored in the existing summary column. No migration is needed. Both summary producers go through formatChatSummaryMarkdown so the subagent path cannot bypass the format. Subagent summaries are extracted from an existing report rather than generated, so they pass no bullets and the serializer returns their snippet unchanged. Validation moved onto the struct, before serialization. Applying the old sentence cap to serialized markdown would have silently stopped working, since bullets routinely omit trailing punctuation. Per-field normalization also preserves backticks, which the shared normalizeShortTextOutput strips, so a headline ending in an inline code span keeps a balanced pair. The panel renders the markdown through InlineMarkdown with a narrow allowlist, bounded to max-h-48 with a Show more toggle. Legacy prose summaries continue to render; summaries regenerate after three completed turns, so both shapes coexist without a backfill. --- coderd/x/chatd/chatd.go | 5 +- coderd/x/chatd/quickgen.go | 116 +++++++++-- coderd/x/chatd/summarygen_internal_test.go | 190 +++++++++++++++++- .../components/ChatSummary.stories.tsx | 90 ++++++++- .../AgentsPage/components/ChatSummary.tsx | 108 +++++++++- .../components/ChatSummaryPanel.stories.tsx | 13 +- 6 files changed, 485 insertions(+), 37 deletions(-) diff --git a/coderd/x/chatd/chatd.go b/coderd/x/chatd/chatd.go index 9d37df09967..4b05aa3acff 100644 --- a/coderd/x/chatd/chatd.go +++ b/coderd/x/chatd/chatd.go @@ -4948,7 +4948,10 @@ func (p *Server) storeSubagentReportSummary( slog.F("chat_id", chat.ID), slog.Error(err)) return } - summary := subagentReportSummarySnippet(report) + // Subagent summaries are extracted from an existing report rather than + // generated, so they have no bullets; the serializer keeps the headline + // as-is and only adds structure when bullets exist. + summary := formatChatSummaryMarkdown(subagentReportSummarySnippet(report), nil) if summary == "" { return } diff --git a/coderd/x/chatd/quickgen.go b/coderd/x/chatd/quickgen.go index ccc73b3ed30..ce8d5e3b69f 100644 --- a/coderd/x/chatd/quickgen.go +++ b/coderd/x/chatd/quickgen.go @@ -969,11 +969,13 @@ func generateManualTitle( } const chatSummaryGenerationPrompt = "You summarize an AI coding chat for a quick-reference popover. " + - "Populate the summary field with 1 to 3 plain sentences describing what the conversation is about and what was accomplished or attempted. " + + "Populate the headline field with one sentence naming what the conversation is about and its outcome. " + + "Populate the bullets field with 2 to 4 short bullets covering what was done or attempted, each a single line. " + "Write about the conversation in the third person. " + - "Preserve specific identifiers such as PR numbers, repo names, file paths, function names, and error messages. " + + "Preserve specific identifiers such as PR numbers, repo names, file paths, function names, and error messages, " + + "wrapping them in backticks. " + "Do not address the user, give instructions, or continue the task. " + - "No markdown, lists, headings, code fences, or surrounding quotes." + "No headings, code fences, tables, or nested lists." const ( // Bound the transcript so the summary call stays cheap and within context; @@ -982,14 +984,20 @@ const ( // Cap a single turn so one long message cannot dominate the budget. summaryTranscriptPerMessageMaxRunes = 4000 summaryMaxOutputTokens = 512 - // Reject pathologically long or verbose summaries, with slack over the - // 1-3 sentence target. - summaryMaxRunes = 1000 - summaryMaxSentences = 6 + // Reject pathologically long or verbose summaries. The caps apply to the + // structured fields before serialization, plus a ceiling on the rendered + // markdown so the panel stays scannable. + summaryMaxRunes = 600 + summaryHeadlineMaxRunes = 200 + summaryHeadlineMaxSentences = 2 + summaryBulletMaxRunes = 160 + summaryMinBullets = 2 + summaryMaxBullets = 4 ) type generatedChatSummary struct { - Summary string `json:"summary" description:"1-3 sentence summary of the whole chat"` + Headline string `json:"headline" description:"One sentence naming what the chat is about and its outcome"` + Bullets []string `json:"bullets" description:"2-4 short bullets, each one line, covering what was done or attempted"` } // renderChatSummaryTranscript renders chat history as plain text for summary @@ -1123,7 +1131,7 @@ func generateChatSummary( result, genErr = object.Generate[generatedChatSummary](retryCtx, model, fantasy.ObjectCall{ Prompt: prompt, SchemaName: "chat_summary", - SchemaDescription: "Summarize the whole chat in 1-3 sentences.", + SchemaDescription: "Summarize the whole chat as a one-sentence headline plus 2-4 short bullets.", MaxOutputTokens: &maxOutputTokens, }) return genErr @@ -1136,22 +1144,94 @@ func generateChatSummary( return "", usage, xerrors.Errorf("generate chat summary: %w", err) } - summary := normalizeShortTextOutput(result.Object.Summary) + summary := generatedChatSummary{ + Headline: normalizeSummaryField(result.Object.Headline), + Bullets: normalizeSummaryBullets(result.Object.Bullets), + } if err := validateGeneratedChatSummary(summary); err != nil { return "", result.Usage, err } - return summary, result.Usage, nil + return formatChatSummaryMarkdown(summary.Headline, summary.Bullets), result.Usage, nil } -func validateGeneratedChatSummary(summary string) error { - if summary == "" { - return xerrors.New("generated chat summary was empty") +// normalizeSummaryField collapses internal whitespace so a field stays on one +// line, and strips surrounding quotes. Unlike normalizeShortTextOutput it +// preserves backticks, so a field ending in an inline code span keeps a +// balanced pair. +func normalizeSummaryField(text string) string { + text = strings.TrimSpace(text) + if text == "" { + return "" } - if len([]rune(summary)) > summaryMaxRunes { - return xerrors.Errorf("generated chat summary exceeded %d runes", summaryMaxRunes) + + text = strings.Trim(text, "\"'") + return strings.Join(strings.Fields(text), " ") +} + +// normalizeSummaryBullets normalizes each bullet and drops the ones that +// normalize to nothing, so blank model output does not render as an empty +// list item. +func normalizeSummaryBullets(bullets []string) []string { + normalized := make([]string, 0, len(bullets)) + for _, bullet := range bullets { + if bullet = normalizeSummaryField(bullet); bullet != "" { + normalized = append(normalized, bullet) + } } - if countSentenceTerminators(summary) > summaryMaxSentences { - return xerrors.Errorf("generated chat summary exceeded %d sentences", summaryMaxSentences) + return normalized +} + +// formatChatSummaryMarkdown renders the stored summary as a headline paragraph +// followed by an optional bullet list, separated by a blank line. Both summary +// producers go through here so the stored format stays consistent. +func formatChatSummaryMarkdown(headline string, bullets []string) string { + headline = strings.TrimSpace(headline) + if len(bullets) == 0 { + return headline + } + + var out strings.Builder + _, _ = out.WriteString(headline) + _, _ = out.WriteString("\n") + for _, bullet := range bullets { + if bullet = strings.TrimSpace(bullet); bullet != "" { + _, _ = out.WriteString("\n- ") + _, _ = out.WriteString(bullet) + } + } + return strings.TrimSpace(out.String()) +} + +// validateGeneratedChatSummary checks the structured fields before they are +// serialized. Validating here rather than over the rendered markdown keeps the +// sentence cap meaningful: bullets routinely omit trailing punctuation, so a +// sentence count over the serialized string would pass almost anything. +func validateGeneratedChatSummary(summary generatedChatSummary) error { + if summary.Headline == "" { + return xerrors.New("generated chat summary headline was empty") + } + if len([]rune(summary.Headline)) > summaryHeadlineMaxRunes { + return xerrors.Errorf("generated chat summary headline exceeded %d runes", summaryHeadlineMaxRunes) + } + if countSentenceTerminators(summary.Headline) > summaryHeadlineMaxSentences { + return xerrors.Errorf("generated chat summary headline exceeded %d sentences", summaryHeadlineMaxSentences) + } + if len(summary.Bullets) < summaryMinBullets || len(summary.Bullets) > summaryMaxBullets { + return xerrors.Errorf( + "generated chat summary had %d bullets, want %d to %d", + len(summary.Bullets), summaryMinBullets, summaryMaxBullets, + ) + } + for _, bullet := range summary.Bullets { + if len([]rune(bullet)) > summaryBulletMaxRunes { + return xerrors.Errorf("generated chat summary bullet exceeded %d runes", summaryBulletMaxRunes) + } + if strings.ContainsAny(bullet, "\n\r") { + return xerrors.New("generated chat summary bullet contained a newline") + } + } + if rendered := formatChatSummaryMarkdown(summary.Headline, summary.Bullets); len([]rune(rendered)) > summaryMaxRunes { + return xerrors.Errorf("generated chat summary exceeded %d runes", summaryMaxRunes) } return nil } diff --git a/coderd/x/chatd/summarygen_internal_test.go b/coderd/x/chatd/summarygen_internal_test.go index 824dbb3c6db..0a1908228b5 100644 --- a/coderd/x/chatd/summarygen_internal_test.go +++ b/coderd/x/chatd/summarygen_internal_test.go @@ -225,10 +225,98 @@ func TestShouldGenerateChatSummary(t *testing.T) { func TestValidateGeneratedChatSummary(t *testing.T) { t.Parallel() - require.Error(t, validateGeneratedChatSummary("")) - require.Error(t, validateGeneratedChatSummary(strings.Repeat("a", summaryMaxRunes+1))) - require.Error(t, validateGeneratedChatSummary("One. Two. Three. Four. Five. Six. Seven.")) - require.NoError(t, validateGeneratedChatSummary("Implemented the summary feature. Added tests.")) + validBullets := []string{"Traced the race in `cache.go`", "Added a regression test"} + + tests := []struct { + name string + summary generatedChatSummary + wantErr bool + }{ + { + name: "Valid", + summary: generatedChatSummary{Headline: "Fixed the flaky CI job.", Bullets: validBullets}, + }, + { + name: "EmptyHeadline", + summary: generatedChatSummary{Bullets: validBullets}, + wantErr: true, + }, + { + name: "HeadlineTooLong", + summary: generatedChatSummary{ + Headline: strings.Repeat("a", summaryHeadlineMaxRunes+1), + Bullets: validBullets, + }, + wantErr: true, + }, + { + name: "HeadlineTooManySentences", + summary: generatedChatSummary{ + Headline: "One. Two. Three.", + Bullets: validBullets, + }, + wantErr: true, + }, + { + name: "TooFewBullets", + summary: generatedChatSummary{Headline: "Fixed it.", Bullets: []string{"Only one"}}, + wantErr: true, + }, + { + name: "NoBullets", + summary: generatedChatSummary{Headline: "Fixed it."}, + wantErr: true, + }, + { + name: "TooManyBullets", + summary: generatedChatSummary{ + Headline: "Fixed it.", + Bullets: []string{"One", "Two", "Three", "Four", "Five"}, + }, + wantErr: true, + }, + { + name: "BulletTooLong", + summary: generatedChatSummary{ + Headline: "Fixed it.", + Bullets: []string{"Fine", strings.Repeat("b", summaryBulletMaxRunes+1)}, + }, + wantErr: true, + }, + { + name: "BulletWithNewline", + summary: generatedChatSummary{ + Headline: "Fixed it.", + Bullets: []string{"Fine", "Broken\nacross lines"}, + }, + wantErr: true, + }, + { + name: "SerializedTooLong", + summary: generatedChatSummary{ + Headline: strings.Repeat("a", summaryHeadlineMaxRunes), + Bullets: []string{ + strings.Repeat("b", summaryBulletMaxRunes), + strings.Repeat("c", summaryBulletMaxRunes), + strings.Repeat("d", summaryBulletMaxRunes), + }, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := validateGeneratedChatSummary(tt.summary) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + }) + } } func TestCountSentenceTerminators(t *testing.T) { @@ -239,10 +327,96 @@ func TestCountSentenceTerminators(t *testing.T) { require.Equal(t, 3, countSentenceTerminators("One. Two! Three?")) require.Equal(t, 0, countSentenceTerminators("auth.rbac.Policy")) - // Dotted identifiers must not push a valid summary over the sentence cap. - require.NoError(t, validateGeneratedChatSummary( - "Refactored pkg.cmd.server and auth.rbac.Policy in main.go and util.go. Added coverage in foo_test.go.", - )) + // Dotted identifiers must not push a valid headline over the sentence cap. + require.NoError(t, validateGeneratedChatSummary(generatedChatSummary{ + Headline: "Refactored pkg.cmd.server and auth.rbac.Policy in main.go and util.go.", + Bullets: []string{"Updated call sites", "Added coverage in foo_test.go"}, + })) +} + +func TestNormalizeSummaryField(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + text string + want string + }{ + {name: "Empty", text: " ", want: ""}, + {name: "CollapsesNewlines", text: "Fixed the race\nin cache.go", want: "Fixed the race in cache.go"}, + {name: "CollapsesRuns", text: "Fixed the\t\trace", want: "Fixed the race"}, + {name: "StripsSurroundingQuotes", text: `"Fixed the race"`, want: "Fixed the race"}, + { + // normalizeShortTextOutput would strip this trailing backtick and + // leave an unbalanced inline code span. + name: "PreservesTrailingBacktick", + text: "Fixed `cache.go`", + want: "Fixed `cache.go`", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + require.Equal(t, tt.want, normalizeSummaryField(tt.text)) + }) + } +} + +func TestNormalizeSummaryBullets(t *testing.T) { + t.Parallel() + + require.Equal(t, + []string{"First bullet", "Second bullet"}, + normalizeSummaryBullets([]string{" First\nbullet ", " ", "Second bullet", ""}), + ) + require.Empty(t, normalizeSummaryBullets(nil)) +} + +func TestFormatChatSummaryMarkdown(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + headline string + bullets []string + want string + }{ + { + name: "HeadlineOnly", + headline: "Fixed the flaky CI job.", + want: "Fixed the flaky CI job.", + }, + { + // A blank line must separate the paragraph from the list, or + // CommonMark folds the first bullet into the headline paragraph. + name: "HeadlineAndBullets", + headline: "Fixed the flaky CI job.", + bullets: []string{"Traced the race", "Added a test"}, + want: "Fixed the flaky CI job.\n\n- Traced the race\n- Added a test", + }, + { + name: "DropsEmptyBullets", + headline: "Fixed it.", + bullets: []string{"Kept", " ", "Also kept"}, + want: "Fixed it.\n\n- Kept\n- Also kept", + }, + { + name: "AllBulletsEmptyKeepsHeadline", + headline: "Fixed it.", + bullets: []string{" ", ""}, + want: "Fixed it.", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + require.Equal(t, tt.want, formatChatSummaryMarkdown(tt.headline, tt.bullets)) + }) + } } func TestSubagentReportSummarySnippet(t *testing.T) { diff --git a/site/src/pages/AgentsPage/components/ChatSummary.stories.tsx b/site/src/pages/AgentsPage/components/ChatSummary.stories.tsx index d680df898e7..cb3afd0617c 100644 --- a/site/src/pages/AgentsPage/components/ChatSummary.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatSummary.stories.tsx @@ -1,13 +1,20 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; -import { expect, within } from "storybook/test"; +import { expect, userEvent, waitFor, within } from "storybook/test"; import { ChatSummary } from "./ChatSummary"; +const MARKDOWN_SUMMARY = [ + "Investigated the flaky CI job in `coderd/x/chatd` and landed a fix.", + "", + "- Traced the failure to a cache-layer race in `chatd.go`", + "- Added a regression test covering the race", + "- Opened PR #26649", +].join("\n"); + const meta: Meta = { title: "pages/AgentsPage/ChatSummary", component: ChatSummary, args: { - summary: - "Investigated the flaky CI job, traced it to a race in the cache layer, and added a regression test.", + summary: MARKDOWN_SUMMARY, createdAt: "2024-05-01T12:00:00Z", updatedAt: "2024-05-02T15:30:00Z", costMicros: 1_250_000, @@ -39,6 +46,83 @@ export const WithSummary: Story = { }, }; +export const HeadlineAndBullets: Story = { + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect( + canvas.getByText(/Investigated the flaky CI job/), + ).toBeInTheDocument(); + + const list = canvas.getByRole("list"); + await expect(within(list).getAllByRole("listitem")).toHaveLength(3); + + // Identifiers wrapped in backticks render as inline code, not literal + // backticks. + await expect(canvas.getByText("chatd.go")).toBeInTheDocument(); + await expect(canvas.queryByText(/`/)).not.toBeInTheDocument(); + + // Short content fits the bound, so no toggle is offered. + await expect( + canvas.queryByRole("button", { name: "Show more" }), + ).not.toBeInTheDocument(); + }, +}; + +// Summaries generated before the structured format are plain prose. They must +// still render, since there is no backfill. +export const LegacyProseSummary: Story = { + args: { + summary: + "Investigated the flaky CI job, traced it to a race in the cache layer, and added a regression test.", + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect( + canvas.getByText(/traced it to a race in the cache layer/), + ).toBeInTheDocument(); + await expect(canvas.queryByRole("list")).not.toBeInTheDocument(); + }, +}; + +// A legacy prose summary that happens to start with "1. " parses as an ordered +// list. `ol` is allowlisted so the items keep a list parent instead of +// rendering as orphan `li` elements. +export const LegacyOrderedList: Story = { + args: { summary: "1. Fixed the race\n2. Added a test" }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const list = canvas.getByRole("list"); + await expect(list.tagName).toBe("OL"); + await expect(within(list).getAllByRole("listitem")).toHaveLength(2); + }, +}; + +export const LongSummaryExpands: Story = { + args: { + summary: [ + "Audited the whole chat pipeline and shipped a batch of fixes.", + "", + ...Array.from( + { length: 12 }, + (_, i) => + `- Reviewed subsystem number ${i + 1} and applied the corresponding fix so the behaviour matches the specification`, + ), + ].join("\n"), + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + + const showMore = await canvas.findByRole("button", { name: "Show more" }); + await userEvent.click(showMore); + + await waitFor(async () => { + await expect( + canvas.getByRole("button", { name: "Show less" }), + ).toBeInTheDocument(); + }); + }, +}; + export const NoSummary: Story = { args: { summary: null }, play: async ({ canvasElement }) => { diff --git a/site/src/pages/AgentsPage/components/ChatSummary.tsx b/site/src/pages/AgentsPage/components/ChatSummary.tsx index 41ca7328bcf..5a3df60656e 100644 --- a/site/src/pages/AgentsPage/components/ChatSummary.tsx +++ b/site/src/pages/AgentsPage/components/ChatSummary.tsx @@ -1,10 +1,21 @@ -import type { FC, ReactNode } from "react"; +import { + type FC, + type ReactNode, + useCallback, + useLayoutEffect, + useRef, + useState, +} from "react"; +import { InlineMarkdown } from "#/components/Markdown/InlineMarkdown"; import { Skeleton } from "#/components/Skeleton/Skeleton"; import { formatCostMicros } from "#/utils/currency"; import { DATE_FORMAT, formatDateTime } from "#/utils/time"; const EMPTY_VALUE = "-"; +/** Compact list spacing that keeps markers inside the narrow summary column. */ +const LIST_CLASSES = "my-2 flex flex-col gap-1 pl-5"; + interface ChatSummaryProps { summary: string | null; createdAt: string; @@ -40,9 +51,7 @@ export const ChatSummary: FC = ({ return (

{trimmedSummary ? ( -

- {trimmedSummary} -

+ ) : (

{isSubagent ? "Summary pending agent completion." : "No summary yet."} @@ -88,6 +97,97 @@ export const ChatSummary: FC = ({ ); }; +interface ChatSummaryBodyProps { + summary: string; +} + +/** + * Renders the stored summary markdown (a headline paragraph plus an optional + * bullet list) inside a bounded box, revealing a toggle only when the content + * actually overflows. `max-height` is used instead of `line-clamp` because + * `line-clamp` relies on `display: -webkit-box`, which clamps unreliably once + * the content contains nested block children such as `

  • `. + */ +const ChatSummaryBody: FC = ({ summary }) => { + const contentRef = useRef(null); + const [isExpanded, setIsExpanded] = useState(false); + const [isOverflowing, setIsOverflowing] = useState(false); + + const measure = useCallback(() => { + const content = contentRef.current; + if (!content) { + return; + } + // Measure against the collapsed bound, which only applies while + // collapsed; once expanded the box grows and would always measure as + // fitting, hiding the "Show less" affordance. + setIsOverflowing(content.scrollHeight > content.clientHeight); + }, []); + + useLayoutEffect(() => { + if (isExpanded) { + return; + } + measure(); + + const content = contentRef.current; + if (!content || typeof ResizeObserver === "undefined") { + return; + } + // The right panel is resizable, and the observer fires immediately on + // observe(), so this also covers a summary swap that changes the box + // height. A swap that leaves the height pinned to the clamp cannot + // change the overflow verdict, so no summary dependency is needed. + const observer = new ResizeObserver(measure); + observer.observe(content); + return () => observer.disconnect(); + }, [measure, isExpanded]); + + return ( +
    +
    +

    {children}

    , + ul: ({ children }) => ( +
      {children}
    + ), + ol: ({ children }) => ( +
      {children}
    + ), + li: ({ children }) => ( +
  • {children}
  • + ), + }} + > + {summary} + +
+ + {(isOverflowing || isExpanded) && ( + + )} + + ); +}; + interface ChatSummaryRowProps { label: string; children: ReactNode; diff --git a/site/src/pages/AgentsPage/components/ChatSummaryPanel.stories.tsx b/site/src/pages/AgentsPage/components/ChatSummaryPanel.stories.tsx index 896ee03d98f..e6fb51621ae 100644 --- a/site/src/pages/AgentsPage/components/ChatSummaryPanel.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatSummaryPanel.stories.tsx @@ -71,17 +71,24 @@ type Story = StoryObj; export const WithSummary: Story = { beforeEach: () => mockRequests({ - summary: - "Investigated the flaky CI job, traced it to a cache-layer race, and added a regression test.", + summary: [ + "Investigated the flaky CI job and landed a fix.", + "", + "- Traced it to a cache-layer race in `chatd.go`", + "- Added a regression test covering the race", + ].join("\n"), }), play: async ({ canvasElement }) => { const canvas = within(canvasElement); await waitFor(() => { expect( - canvas.getByText(/traced it to a cache-layer race/), + canvas.getByText(/Traced it to a cache-layer race/), ).toBeInTheDocument(); expect(canvas.getByText("$1.25")).toBeInTheDocument(); }); + expect( + within(canvas.getByRole("list")).getAllByRole("listitem"), + ).toHaveLength(2); }, }; From 634f89079ae5141fe06efb37ac727d8643d354da Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Tue, 11 Aug 2026 10:16:48 +0000 Subject: [PATCH 2/7] fix(site/src/pages/AgentsPage/components): address summary panel review Observe overflow on an inner unclamped element rather than the clamped box. The clamped box stops growing at its max height, so a summary that arrives via an in-place cache update while the box is already pinned there resized nothing and stayed clipped with no toggle. Drop useCallback, which site/AGENTS.md prohibits under src/pages/AgentsPage since React Compiler already memoizes callbacks there. The measurement closure now lives inside the effect. Render link text without an anchor. Clipping below the collapsed bound is visual only, so a mounted anchor stayed reachable by keyboard and screen readers while invisible. --- .../components/ChatSummary.stories.tsx | 68 +++++++++++-- .../AgentsPage/components/ChatSummary.tsx | 96 +++++++++++-------- 2 files changed, 112 insertions(+), 52 deletions(-) diff --git a/site/src/pages/AgentsPage/components/ChatSummary.stories.tsx b/site/src/pages/AgentsPage/components/ChatSummary.stories.tsx index cb3afd0617c..8b56effbde3 100644 --- a/site/src/pages/AgentsPage/components/ChatSummary.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatSummary.stories.tsx @@ -1,4 +1,5 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; +import { useState } from "react"; import { expect, userEvent, waitFor, within } from "storybook/test"; import { ChatSummary } from "./ChatSummary"; @@ -10,6 +11,16 @@ const MARKDOWN_SUMMARY = [ "- Opened PR #26649", ].join("\n"); +const LONG_SUMMARY = [ + "Audited the whole chat pipeline and shipped a batch of fixes.", + "", + ...Array.from( + { length: 12 }, + (_, i) => + `- Reviewed subsystem number ${i + 1} and applied the corresponding fix so the behaviour matches the specification`, + ), +].join("\n"); + const meta: Meta = { title: "pages/AgentsPage/ChatSummary", component: ChatSummary, @@ -97,18 +108,55 @@ export const LegacyOrderedList: Story = { }, }; -export const LongSummaryExpands: Story = { +// Links are rendered as plain text. `overflow-hidden` clips the collapsed +// content visually only, so a mounted anchor below the bound would still be +// reachable by keyboard and screen readers while invisible. +export const LinksRenderAsPlainText: Story = { args: { - summary: [ - "Audited the whole chat pipeline and shipped a batch of fixes.", - "", - ...Array.from( - { length: 12 }, - (_, i) => - `- Reviewed subsystem number ${i + 1} and applied the corresponding fix so the behaviour matches the specification`, - ), - ].join("\n"), + summary: + "Investigated the failure in [PR #26649](https://example.com/pr) and fixed it.", + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect(canvas.getByText(/PR #26649/)).toBeInTheDocument(); + await expect(canvas.queryByRole("link")).not.toBeInTheDocument(); }, +}; + +// A cache update can replace the summary in place, without remounting the +// panel, so the overflow toggle has to re-evaluate on new content. +export const SummaryReplacedInPlace: Story = { + render: (args) => { + const [summary, setSummary] = useState(MARKDOWN_SUMMARY); + return ( +
+ + +
+ ); + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect( + canvas.queryByRole("button", { name: "Show more" }), + ).not.toBeInTheDocument(); + + await userEvent.click( + canvas.getByRole("button", { name: "Simulate update" }), + ); + + await waitFor(async () => { + await expect( + canvas.getByRole("button", { name: "Show more" }), + ).toBeInTheDocument(); + }); + }, +}; + +export const LongSummaryExpands: Story = { + args: { summary: LONG_SUMMARY }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); diff --git a/site/src/pages/AgentsPage/components/ChatSummary.tsx b/site/src/pages/AgentsPage/components/ChatSummary.tsx index 5a3df60656e..27b06c245e5 100644 --- a/site/src/pages/AgentsPage/components/ChatSummary.tsx +++ b/site/src/pages/AgentsPage/components/ChatSummary.tsx @@ -1,7 +1,6 @@ import { type FC, type ReactNode, - useCallback, useLayoutEffect, useRef, useState, @@ -107,72 +106,85 @@ interface ChatSummaryBodyProps { * actually overflows. `max-height` is used instead of `line-clamp` because * `line-clamp` relies on `display: -webkit-box`, which clamps unreliably once * the content contains nested block children such as `
  • `. + * + * Overflow is measured on the clamped box but observed on an inner unclamped + * element, so both panel resizes and in-place summary updates re-evaluate the + * toggle. */ const ChatSummaryBody: FC = ({ summary }) => { + const clampRef = useRef(null); const contentRef = useRef(null); const [isExpanded, setIsExpanded] = useState(false); const [isOverflowing, setIsOverflowing] = useState(false); - const measure = useCallback(() => { - const content = contentRef.current; - if (!content) { - return; - } - // Measure against the collapsed bound, which only applies while - // collapsed; once expanded the box grows and would always measure as - // fitting, hiding the "Show less" affordance. - setIsOverflowing(content.scrollHeight > content.clientHeight); - }, []); - useLayoutEffect(() => { + // Overflow only needs measuring while collapsed. Skipping the expanded + // state preserves the verdict computed while collapsed, so the toggle + // stays visible; collapsing reruns this effect and remeasures. if (isExpanded) { return; } + const clamp = clampRef.current; + const content = contentRef.current; + if (!clamp || !content) { + return; + } + const measure = () => + setIsOverflowing(clamp.scrollHeight > clamp.clientHeight); measure(); - const content = contentRef.current; - if (!content || typeof ResizeObserver === "undefined") { + if (typeof ResizeObserver === "undefined") { return; } - // The right panel is resizable, and the observer fires immediately on - // observe(), so this also covers a summary swap that changes the box - // height. A swap that leaves the height pinned to the clamp cannot - // change the overflow verdict, so no summary dependency is needed. + // Observe the inner element rather than the clamped one. The clamped box + // stops growing at its max height, so a summary that arrives via a cache + // update while the box is already pinned there would resize nothing and + // stay clipped with no toggle. The inner element is unclamped, so its + // height tracks the content and its width tracks the resizable panel. const observer = new ResizeObserver(measure); observer.observe(content); return () => observer.disconnect(); - }, [measure, isExpanded]); + }, [isExpanded]); return (
    -

    {children}

    , - ul: ({ children }) => ( -
      {children}
    - ), - ol: ({ children }) => ( -
      {children}
    - ), - li: ({ children }) => ( -
  • {children}
  • - ), - }} - > - {summary} - +
    + ( +

    {children}

    + ), + ul: ({ children }) => ( +
      {children}
    + ), + ol: ({ children }) => ( +
      {children}
    + ), + li: ({ children }) => ( +
  • {children}
  • + ), + // Render link text without an anchor. Clipping below the + // collapsed bound is visual only, so a mounted anchor would + // stay reachable by keyboard and screen readers while + // invisible. Summaries are generated text, not navigation. + a: ({ children }) => <>{children}, + }} + > + {summary} +
    +
    {(isOverflowing || isExpanded) && ( From dff5f061b7f1db9a3834411c592c5f6837ba30cf Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Tue, 11 Aug 2026 10:24:17 +0000 Subject: [PATCH 3/7] docs(coderd/x/chatd): correct stale chat summary format comments generateChatSummary now returns a headline plus bullets, but its doc comment still described a 1-3 sentence summary. The subagent snippet bound in chatd.go referenced the same obsolete contract. --- coderd/x/chatd/chatd.go | 2 +- coderd/x/chatd/quickgen.go | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/coderd/x/chatd/chatd.go b/coderd/x/chatd/chatd.go index 4b05aa3acff..284ae56abfc 100644 --- a/coderd/x/chatd/chatd.go +++ b/coderd/x/chatd/chatd.go @@ -4726,7 +4726,7 @@ const ( // Subagent summaries reuse the final report instead of generating // text, so their work timeout only covers two database round trips. subagentReportSummaryTimeout = 15 * time.Second - // Bound the extracted report snippet near the 1-3 sentence + // Bound the extracted report snippet near the headline of the // generated summaries that root chats get, so subagent and parent // summary panels read the same. subagentReportSummaryMaxRunes = 300 diff --git a/coderd/x/chatd/quickgen.go b/coderd/x/chatd/quickgen.go index ce8d5e3b69f..debd2f5f917 100644 --- a/coderd/x/chatd/quickgen.go +++ b/coderd/x/chatd/quickgen.go @@ -1096,9 +1096,10 @@ func boundTranscriptHeadTail(lines []string, maxRunes int) string { return out.String() } -// generateChatSummary generates a 1-3 sentence whole-chat summary from a -// transcript. A blank or invalid result returns an error so callers preserve -// any existing summary rather than clearing it. +// generateChatSummary generates a whole-chat summary from a transcript as a +// one-sentence headline plus 2-4 bullets, serialized to markdown by +// formatChatSummaryMarkdown. A blank or invalid result returns an error so +// callers preserve any existing summary rather than clearing it. func generateChatSummary( ctx context.Context, model fantasy.LanguageModel, From 9af00845474c96d1797d113c4ab31d6dad887998 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Tue, 11 Aug 2026 10:35:47 +0000 Subject: [PATCH 4/7] fix(site/src/pages/AgentsPage/components): wrap long identifiers in summaries Summaries preserve identifiers verbatim and wrap them in backticks, so a single token can be wider than the panel with no natural break opportunity. The summary box sized itself to that token, escaping its column, and the clipped remainder was unreachable because the toggle only responds to vertical overflow. --- .../components/ChatSummary.stories.tsx | 33 +++++++++++++++++++ .../AgentsPage/components/ChatSummary.tsx | 6 +++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/site/src/pages/AgentsPage/components/ChatSummary.stories.tsx b/site/src/pages/AgentsPage/components/ChatSummary.stories.tsx index 8b56effbde3..f13057ce0c4 100644 --- a/site/src/pages/AgentsPage/components/ChatSummary.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatSummary.stories.tsx @@ -123,6 +123,39 @@ export const LinksRenderAsPlainText: Story = { }, }; +// The generation prompt preserves identifiers and wraps them in backticks, so +// a single token can be wider than the panel. It has to wrap, because the +// collapsed bound only reveals its toggle for vertical overflow. +export const LongIdentifierWraps: Story = { + args: { + summary: + "Fixed `TestValidateGeneratedChatSummaryHeadlineExceedsBothTheRuneAndSentenceCaps` in `coderd/x/chatd/summarygen_internal_test.go`.", + }, + // Narrower than the panel's 360px minimum, so the identifier cannot fit on + // one line. + decorators: [ + (Story) => ( +
    + +
    + ), + ], + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const column = canvas.getByTestId("summary-column"); + const identifier = canvas.getByText( + "TestValidateGeneratedChatSummaryHeadlineExceedsBothTheRuneAndSentenceCaps", + ); + + // The identifier has no natural break opportunity, so without a + // word-breaking rule it renders on one line and escapes the column, + // where overflow-hidden clips it with no toggle to reveal it. + await expect(identifier.getBoundingClientRect().right).toBeLessThanOrEqual( + column.getBoundingClientRect().right + 1, + ); + }, +}; + // A cache update can replace the summary in place, without remounting the // panel, so the overflow toggle has to re-evaluate on new content. export const SummaryReplacedInPlace: Story = { diff --git a/site/src/pages/AgentsPage/components/ChatSummary.tsx b/site/src/pages/AgentsPage/components/ChatSummary.tsx index 27b06c245e5..2bcae26f83a 100644 --- a/site/src/pages/AgentsPage/components/ChatSummary.tsx +++ b/site/src/pages/AgentsPage/components/ChatSummary.tsx @@ -150,7 +150,11 @@ const ChatSummaryBody: FC = ({ summary }) => {
    From 90f83d6752a634fc987c94348577000900842458 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Tue, 11 Aug 2026 10:42:48 +0000 Subject: [PATCH 5/7] test(site/src/pages/AgentsPage/components): drop geometry assertion from summary story FE10 prohibits DOM-geometry assertions because pixel values vary with font rendering across environments. The long-identifier story keeps its narrow column as a visual regression fixture and asserts semantically instead. --- .../components/ChatSummary.stories.tsx | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/site/src/pages/AgentsPage/components/ChatSummary.stories.tsx b/site/src/pages/AgentsPage/components/ChatSummary.stories.tsx index f13057ce0c4..c31ebd19822 100644 --- a/site/src/pages/AgentsPage/components/ChatSummary.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatSummary.stories.tsx @@ -131,28 +131,24 @@ export const LongIdentifierWraps: Story = { summary: "Fixed `TestValidateGeneratedChatSummaryHeadlineExceedsBothTheRuneAndSentenceCaps` in `coderd/x/chatd/summarygen_internal_test.go`.", }, - // Narrower than the panel's 360px minimum, so the identifier cannot fit on - // one line. + // Pinned narrower than the panel's 360px minimum so the identifier cannot + // fit on one line. The wrapped layout itself is covered by visual + // regression snapshots; per FE10 the assertion here stays semantic rather + // than measuring geometry. decorators: [ (Story) => ( -
    +
    ), ], play: async ({ canvasElement }) => { const canvas = within(canvasElement); - const column = canvas.getByTestId("summary-column"); - const identifier = canvas.getByText( - "TestValidateGeneratedChatSummaryHeadlineExceedsBothTheRuneAndSentenceCaps", - ); - - // The identifier has no natural break opportunity, so without a - // word-breaking rule it renders on one line and escapes the column, - // where overflow-hidden clips it with no toggle to reveal it. - await expect(identifier.getBoundingClientRect().right).toBeLessThanOrEqual( - column.getBoundingClientRect().right + 1, - ); + await expect( + canvas.getByText( + "TestValidateGeneratedChatSummaryHeadlineExceedsBothTheRuneAndSentenceCaps", + ), + ).toBeVisible(); }, }; From 32cd37b6e010d8b9f9c5c3b4578618649645a34a Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Tue, 11 Aug 2026 10:55:00 +0000 Subject: [PATCH 6/7] perf(site/src/pages/AgentsPage/components): settle summary resize measurement Dragging the right panel resized the summary every frame, and each observer callback read layout and set state even though the overflow verdict is a boolean that flips at most a couple of times per drag. Waiting for the resize to settle skips those reads entirely while a drag is in flight. --- .../AgentsPage/components/ChatSummary.tsx | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/site/src/pages/AgentsPage/components/ChatSummary.tsx b/site/src/pages/AgentsPage/components/ChatSummary.tsx index 2bcae26f83a..121620853b3 100644 --- a/site/src/pages/AgentsPage/components/ChatSummary.tsx +++ b/site/src/pages/AgentsPage/components/ChatSummary.tsx @@ -15,6 +15,13 @@ const EMPTY_VALUE = "-"; /** Compact list spacing that keeps markers inside the narrow summary column. */ const LIST_CLASSES = "my-2 flex flex-col gap-1 pl-5"; +/** + * How long resizing must settle before overflow is remeasured. Dragging the + * right panel resizes the summary every frame, and the overflow verdict is a + * boolean that flips at most a couple of times per drag. + */ +const RESIZE_SETTLE_MS = 100; + interface ChatSummaryProps { summary: string | null; createdAt: string; @@ -141,9 +148,21 @@ const ChatSummaryBody: FC = ({ summary }) => { // update while the box is already pinned there would resize nothing and // stay clipped with no toggle. The inner element is unclamped, so its // height tracks the content and its width tracks the resizable panel. - const observer = new ResizeObserver(measure); + // + // Debounce rather than schedule on an animation frame: observer + // callbacks are already delivered at most once per frame, so a frame + // callback would defer the same work instead of doing less of it. + // Settling skips the layout reads entirely while a drag is in flight. + let settleTimeout: ReturnType | undefined; + const observer = new ResizeObserver(() => { + clearTimeout(settleTimeout); + settleTimeout = setTimeout(measure, RESIZE_SETTLE_MS); + }); observer.observe(content); - return () => observer.disconnect(); + return () => { + clearTimeout(settleTimeout); + observer.disconnect(); + }; }, [isExpanded]); return ( From 1b6328dccf944fc63a9b9f9b14c394ea6f95fc52 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Tue, 11 Aug 2026 11:06:46 +0000 Subject: [PATCH 7/7] fix(site/src/pages/AgentsPage/components): reset summary expansion per chat Switching chats swaps the panel's props instead of remounting it. An uncached chat happened to reset the expanded state, because the panel renders nothing while loading and that unmounts the summary, but a cached chat resolves synchronously and kept the previous chat's expanded state, offering to collapse a summary that may not overflow at all. --- .../components/ChatSummaryPanel.stories.tsx | 84 ++++++++++++++++++- .../components/ChatSummaryPanel.tsx | 5 ++ 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/site/src/pages/AgentsPage/components/ChatSummaryPanel.stories.tsx b/site/src/pages/AgentsPage/components/ChatSummaryPanel.stories.tsx index e6fb51621ae..0cf15c6143b 100644 --- a/site/src/pages/AgentsPage/components/ChatSummaryPanel.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatSummaryPanel.stories.tsx @@ -1,6 +1,6 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; -import type { FC } from "react"; -import { expect, spyOn, waitFor, within } from "storybook/test"; +import { type FC, useState } from "react"; +import { expect, spyOn, userEvent, waitFor, within } from "storybook/test"; import { API } from "#/api/api"; import type * as TypesGen from "#/api/typesGenerated"; import { MockChat } from "#/testHelpers/chatEntities"; @@ -8,6 +8,17 @@ import { withDashboardProvider } from "#/testHelpers/storybook"; import { ChatSummaryPanel } from "./ChatSummaryPanel"; const ROOT_CHAT_ID = "root-chat-id"; +const OTHER_CHAT_ID = "other-chat-id"; + +const LONG_SUMMARY = [ + "Audited the whole chat pipeline and shipped a batch of fixes.", + "", + ...Array.from( + { length: 12 }, + (_, i) => + `- Reviewed subsystem number ${i + 1} and applied the corresponding fix so the behaviour matches the specification`, + ), +].join("\n"); const mockCost: TypesGen.ChatCost = { chat_id: MockChat.id, @@ -163,3 +174,72 @@ export const GatewayUnavailable: Story = { expect(API.experimental.getChatCost).not.toHaveBeenCalled(); }, }; + +// Navigating between chats swaps `chatId` on the existing panel instead of +// remounting it, so the disclosure state must not carry over. Without a reset +// the next chat renders fully expanded behind a "Show less" button, even when +// its own summary fits. +export const ExpansionResetsBetweenChats: Story = { + beforeEach: () => { + spyOn(API.experimental, "getChat").mockImplementation(async (chatId) => ({ + ...MockChat, + id: chatId, + summary: chatId === MockChat.id ? LONG_SUMMARY : "A summary that fits.", + })); + spyOn(API.experimental, "getChatCost").mockResolvedValue(mockCost); + }, + render: (args) => { + const [chatId, setChatId] = useState(MockChat.id); + return ( +
    + + +
    + ); + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const switchChat = canvas.getByRole("button", { name: "Switch chat" }); + const expand = async () => { + const showMore = await canvas.findByRole("button", { + name: "Show more", + }); + await userEvent.click(showMore); + await expect( + canvas.getByRole("button", { name: "Show less" }), + ).toBeInTheDocument(); + }; + + // Warm both chats in the query cache first. While a chat is still + // uncached the panel renders nothing, which unmounts the summary and + // resets the disclosure state as a side effect. Once cached, the data is + // returned synchronously and the panel keeps the same summary instance + // across the switch, which is where the state can leak. + await expand(); + await userEvent.click(switchChat); + await waitFor(async () => { + await expect(canvas.getByText("A summary that fits.")).toBeVisible(); + }); + await userEvent.click(switchChat); + + await expand(); + await userEvent.click(switchChat); + await waitFor(async () => { + await expect(canvas.getByText("A summary that fits.")).toBeVisible(); + }); + + // The switched-to summary fits, so it must not offer to collapse. + await expect( + canvas.queryByRole("button", { name: "Show less" }), + ).not.toBeInTheDocument(); + }, +}; diff --git a/site/src/pages/AgentsPage/components/ChatSummaryPanel.tsx b/site/src/pages/AgentsPage/components/ChatSummaryPanel.tsx index 7f4e0b0bede..8bc9c5a21a4 100644 --- a/site/src/pages/AgentsPage/components/ChatSummaryPanel.tsx +++ b/site/src/pages/AgentsPage/components/ChatSummaryPanel.tsx @@ -32,6 +32,11 @@ export const ChatSummaryPanel: FC = ({ } else if (chatData) { content = (