From 8737af203d4368d6486e7ea31737a8759b57b547 Mon Sep 17 00:00:00 2001 From: Luke Parker <10430890+Hona@users.noreply.github.com> Date: Fri, 16 Oct 2026 10:16:28 +1000 Subject: [PATCH] feat(app): add commentary timeline category (#55248) --- .../component-tests/timeline-detail.spec.ts | 9 +-- packages/app/src/runtime/i18n/en.ts | 1 + .../app/src/session/timeline/controller.tsx | 1 + packages/app/src/settings/model.tsx | 1 + .../src/components/message-part.css | 5 ++ .../src/timeline/commentary.stories.tsx | 70 +++++++++++++++++++ packages/session-ui/src/timeline/detail.ts | 8 ++- .../session-ui/src/timeline/projection.ts | 47 +++++++++++-- .../src/timeline/rows-current.test.ts | 48 +++++++++++++ .../src/timeline/session-timeline-row.tsx | 9 +++ .../session-ui/src/tools/tool-renderer.tsx | 7 +- 11 files changed, 195 insertions(+), 11 deletions(-) create mode 100644 packages/session-ui/src/timeline/commentary.stories.tsx diff --git a/packages/app/component-tests/timeline-detail.spec.ts b/packages/app/component-tests/timeline-detail.spec.ts index d8b71e74cf5..001bfc2a4f0 100644 --- a/packages/app/component-tests/timeline-detail.spec.ts +++ b/packages/app/component-tests/timeline-detail.spec.ts @@ -8,7 +8,7 @@ story("maps grouped and collapsed switches to the timeline settings", async ({ m const collapsed = shell.getByRole("switch", { name: "Shell collapsed", exact: true }) const value = component.locator('[data-slot="timeline-detail-fixture-value"]') - await expect(component.getByRole("switch")).toHaveCount(9) + await expect(component.getByRole("switch")).toHaveCount(10) await expect(component.getByText("Activity", { exact: true })).toHaveCount(0) await expect(grouped).toBeChecked() await expect(collapsed).toBeChecked() @@ -138,7 +138,8 @@ story("keeps visibility and switches in sync with the preset slider", async ({ m await slider.focus() await slider.press("Home") await expect(slider).toHaveAttribute("aria-valuetext", "Messages only") - await expect(component.getByRole("switch")).toHaveCount(0) + // Messages only hides every activity but keeps commentary, which is text. + await expect(component.getByRole("switch")).toHaveAccessibleName("Commentary grouped") await expect(component.locator('[data-slot="timeline-detail-unavailable"]')).toHaveCount(9) await expect(component.locator('[data-action="timeline-detail-visibility"][aria-pressed="false"]')).toHaveCount(6) await component.getByRole("button", { name: "Shell visibility" }).click() @@ -147,9 +148,9 @@ story("keeps visibility and switches in sync with the preset slider", async ({ m await slider.focus() await slider.press("End") await expect(slider).toHaveAttribute("aria-valuetext", "Everything") - await expect(component.getByRole("switch")).toHaveCount(9) + await expect(component.getByRole("switch")).toHaveCount(10) await expect(component.locator('[data-slot="timeline-detail-unavailable"]')).toHaveCount(0) - await expect(component.locator('[data-action="timeline-detail-visibility"][aria-pressed="true"]')).toHaveCount(6) + await expect(component.locator('[data-action="timeline-detail-visibility"][aria-pressed="true"]')).toHaveCount(7) await expect(component.getByRole("switch", { checked: true })).toHaveCount(0) }) diff --git a/packages/app/src/runtime/i18n/en.ts b/packages/app/src/runtime/i18n/en.ts index 6ee0b5aaffa..fc964b49763 100644 --- a/packages/app/src/runtime/i18n/en.ts +++ b/packages/app/src/runtime/i18n/en.ts @@ -813,6 +813,7 @@ export const dict = { "settings.timeline.category.subagents": "Subagents", "settings.timeline.category.notices": "Notices", "settings.timeline.category.tools": "Other tools", + "settings.timeline.category.commentary": "Commentary", "settings.general.row.language.title": "Language", "settings.general.row.language.description": "Change the display language for OpenCode", diff --git a/packages/app/src/session/timeline/controller.tsx b/packages/app/src/session/timeline/controller.tsx index bca709af12c..69bdcb8a993 100644 --- a/packages/app/src/session/timeline/controller.tsx +++ b/packages/app/src/session/timeline/controller.tsx @@ -158,6 +158,7 @@ export function createTimelineController(input: { session: TimelineSessionSource subagents: { ...detail.subagents }, notices: { ...detail.notices }, tools: { ...detail.tools }, + commentary: { ...detail.commentary }, } }) diff --git a/packages/app/src/settings/model.tsx b/packages/app/src/settings/model.tsx index 8efa3015c3b..47c4307b9a1 100644 --- a/packages/app/src/settings/model.tsx +++ b/packages/app/src/settings/model.tsx @@ -114,6 +114,7 @@ const generalSchema = Persistence.struct({ subagents: placementOnlySchema, notices: placementOnlySchema, tools: placementOnlySchema, + commentary: placementOnlySchema, }), showCustomAgents: Schema.Boolean, showTurnDiffs: Schema.Boolean, diff --git a/packages/session-ui/src/components/message-part.css b/packages/session-ui/src/components/message-part.css index 1a1cec4e212..f957f10117d 100644 --- a/packages/session-ui/src/components/message-part.css +++ b/packages/session-ui/src/components/message-part.css @@ -743,6 +743,11 @@ width: 100%; } + /* Grouped commentary follows the list rhythm instead of the timeline's text spacing. */ + > [data-component="text-part"] { + margin-top: 0; + } + > [data-component="tool-part-wrapper"] > [data-component="collapsible"] > [data-slot="collapsible-trigger"]:not([data-hide-details="true"]) { diff --git a/packages/session-ui/src/timeline/commentary.stories.tsx b/packages/session-ui/src/timeline/commentary.stories.tsx new file mode 100644 index 00000000000..bb298f42aac --- /dev/null +++ b/packages/session-ui/src/timeline/commentary.stories.tsx @@ -0,0 +1,70 @@ +import type { SessionMessageAssistant } from "@opencode/client/promise" +import type { SessionDocument } from "../document" +import { CURRENT_SESSION_ID, STORY_MODEL, STORY_TIME } from "../storybook/current-session-fixtures" +import { storyTool } from "../storybook/current-session-scenarios" +import { CurrentSessionProviders } from "../storybook/current-session-story" +import { timelinePresets, type TimelinePlacement } from "./detail" +import { SessionTimeline } from "./session-timeline" + +export default { + title: "OpenCode/Conversation/Commentary", + id: "current-session-commentary", + component: SessionTimeline, + parameters: { layout: "fullscreen" }, + args: { commentary: "grouped" }, + argTypes: { commentary: { control: { type: "inline-radio" }, options: ["separate", "grouped", "hidden"] } }, +} + +const step = (id: string, offset: number, content: SessionMessageAssistant["content"]) => + ({ + id, + type: "assistant", + agent: "build", + model: STORY_MODEL, + content, + time: { created: STORY_TIME + offset, completed: STORY_TIME + offset + 900 }, + }) satisfies SessionDocument["messages"][number] + +const document = { + sessionID: CURRENT_SESSION_ID, + status: { type: "idle" }, + diffs: [], + messages: [ + { + id: "msg_commentary_user", + type: "user", + text: "The timeline setting resets after a restart. Can you fix it?", + metadata: { agent: "build", model: STORY_MODEL }, + time: { created: STORY_TIME }, + }, + step("msg_commentary_1", 100, [ + { type: "text", text: "I'll find where the timeline setting is stored." }, + storyTool("commentary_grep", "grep", "completed", { pattern: "timelineDetail" }), + storyTool("commentary_read", "read", "completed", { filePath: "src/settings/model.tsx" }), + ]), + step("msg_commentary_2", 1100, [ + { type: "text", text: "The stored value is missing the new category, so decoding falls back. Running the tests." }, + storyTool("commentary_shell", "shell", "completed", { command: "bun test settings" }, { output: "12 pass" }), + ]), + step("msg_commentary_3", 2100, [ + { type: "text", text: "Tests pass. Writing up the fix.", state: { phase: "commentary" } }, + { + type: "text", + text: "Fixed. Stored settings that lack a category now take that category from the default preset, so the value survives a restart.", + }, + ]), + ], +} satisfies SessionDocument + +export const Placement = { + render: (args: { commentary: TimelinePlacement }) => ( +
+ + + +
+ ), +} diff --git a/packages/session-ui/src/timeline/detail.ts b/packages/session-ui/src/timeline/detail.ts index a7868f14c88..3716935a55c 100644 --- a/packages/session-ui/src/timeline/detail.ts +++ b/packages/session-ui/src/timeline/detail.ts @@ -1,7 +1,7 @@ import type { SessionMessageAssistant, SessionMessageInfo } from "@opencode/client/promise" import { shellResultFailed } from "../message/current-tool-state" -export const timelineCategories = ["shell", "edit", "thinking", "subagents", "notices", "tools"] as const +export const timelineCategories = ["shell", "edit", "thinking", "subagents", "notices", "tools", "commentary"] as const export type TimelineCategory = (typeof timelineCategories)[number] @@ -16,6 +16,7 @@ export type TimelineDetail = { subagents: { placement: TimelinePlacement } notices: { placement: TimelinePlacement } tools: { placement: TimelinePlacement } + commentary: { placement: TimelinePlacement } } export const timelinePresets = [ @@ -28,6 +29,7 @@ export const timelinePresets = [ subagents: { placement: "separate" }, notices: { placement: "separate" }, tools: { placement: "separate" }, + commentary: { placement: "separate" }, }, }, { @@ -39,6 +41,7 @@ export const timelinePresets = [ subagents: { placement: "separate" }, notices: { placement: "grouped" }, tools: { placement: "grouped" }, + commentary: { placement: "separate" }, }, }, { @@ -50,6 +53,7 @@ export const timelinePresets = [ subagents: { placement: "grouped" }, notices: { placement: "grouped" }, tools: { placement: "grouped" }, + commentary: { placement: "separate" }, }, }, { @@ -61,6 +65,7 @@ export const timelinePresets = [ subagents: { placement: "grouped" }, notices: { placement: "hidden" }, tools: { placement: "hidden" }, + commentary: { placement: "separate" }, }, }, { @@ -72,6 +77,7 @@ export const timelinePresets = [ subagents: { placement: "hidden" }, notices: { placement: "hidden" }, tools: { placement: "hidden" }, + commentary: { placement: "separate" }, }, }, ] as const satisfies readonly { id: string; value: TimelineDetail }[] diff --git a/packages/session-ui/src/timeline/projection.ts b/packages/session-ui/src/timeline/projection.ts index 03dffd59235..f1c30ec27d1 100644 --- a/packages/session-ui/src/timeline/projection.ts +++ b/packages/session-ui/src/timeline/projection.ts @@ -356,6 +356,20 @@ export namespace Timeline { turns.findLast((turn) => !pendingInputIDs?.has(turn.id) || turn.entries.some((entry) => entry.type === "idle")) ?.id ?? turns.at(-1)?.id + const commentary = new Set( + detail && detail.commentary.placement !== "separate" + ? turns.flatMap((turn) => + turnCommentary(turn.entries, turn.id !== activeMessageID || status.type === "idle", (content) => + isRenderable(content, showReasoning, detail), + ), + ) + : [], + ) + + const visible = (content: Content, showReasoning: boolean, detail?: TimelineDetail) => + !(detail?.commentary.placement === "hidden" && commentary.has(content)) && + isRenderable(content, showReasoning, detail) + const visibleNotice = (message: Notice) => !detail || detail.notices.placement !== "hidden" || timelineNoticeRequired(message) @@ -372,7 +386,7 @@ export namespace Timeline { entry.type === "diff" || !!entry.message.error || !!entry.message.retry || - entry.message.content.some((content) => isRenderable(content, showReasoning, detail)), + entry.message.content.some((content) => visible(content, showReasoning, detail)), ) }) : turns @@ -407,8 +421,9 @@ export namespace Timeline { turn.id === activeMessageID, shellToolDefaultOpen, editToolDefaultOpen, - isRenderable, + visible, detail, + commentary, ) }), ] @@ -465,6 +480,7 @@ export namespace Timeline { editToolDefaultOpen = false, isRenderable = renderable, detail?: TimelineDetail, + commentary: ReadonlySet = new Set(), ) { const rows: TimelineRow.TimelineRow[] = [] const assistantMessages = entries.flatMap((entry) => (entry.type === "assistant" ? [entry.message] : [])) @@ -524,7 +540,7 @@ export namespace Timeline { const appendGroups = (items: typeof refs) => { let offset = 0 - groupContent(items, shellToolDefaultOpen, editToolDefaultOpen, detail).forEach((group) => { + groupContent(items, shellToolDefaultOpen, editToolDefaultOpen, commentary, detail).forEach((group) => { const tool = group.type !== "part" || items[offset]?.content.type !== "text" offset += group.type === "part" ? 1 : group.refs.length rows.push( @@ -648,6 +664,26 @@ function shellFailed(message: SessionMessageShell) { ) } +// Text that a tool call follows in the same turn is commentary; text after the last tool call is the answer. +// OpenAI Responses models label each message with a phase, which wins over position. A settled turn +// without an answer keeps its last text, so an interrupted turn still ends with something to read. +function turnCommentary(entries: Entry[], settled: boolean, visible: (content: Content) => boolean) { + const contents = entries.flatMap((entry) => (entry.type === "assistant" ? entry.message.content : [])) + const lastTool = contents.findLastIndex((content) => content.type === "tool") + + const commentary = contents.filter( + (content, index) => + content.type === "text" && + (content.state?.phase === "commentary" || (content.state?.phase !== "final_answer" && index < lastTool)), + ) + + const texts = contents.filter((content) => content.type === "text" && visible(content)) + + if (!settled || texts.some((content) => !commentary.includes(content))) return commentary + + return commentary.filter((content) => content !== texts.at(-1)) +} + function groupMessages(rows: TimelineRow.TimelineRow[], detail: TimelineDetail, separate: ReadonlySet) { return rows.reduce((result, row) => { const previous = result.at(-1) @@ -853,6 +889,7 @@ function groupContent( items: { messageID: string; partID: string; content: Content }[], shellToolDefaultOpen: boolean, editToolDefaultOpen: boolean, + commentary: ReadonlySet, detail?: TimelineDetail, ): PartGroup[] { const groups: PartGroup[] = [] @@ -890,6 +927,7 @@ function groupContent( shellToolDefaultOpen, editToolDefaultOpen, adjacent?.type === "context" && adjacent.tools, + commentary, detail, ) @@ -919,11 +957,12 @@ function contentGroupType( shellExpanded: boolean, editExpanded: boolean, hasContextGroup: boolean, + commentary: ReadonlySet, detail?: TimelineDetail, ) { if (content.type === "tool") return toolGroupType(content, shellExpanded, editExpanded, hasContextGroup, detail) - if (content.type !== "reasoning") return undefined + if (content.type === "text") return commentary.has(content) ? "context" : undefined return detail && detail.thinking.placement !== "grouped" ? undefined : "context" } diff --git a/packages/session-ui/src/timeline/rows-current.test.ts b/packages/session-ui/src/timeline/rows-current.test.ts index 6ebf476d901..9eb93268b3e 100644 --- a/packages/session-ui/src/timeline/rows-current.test.ts +++ b/packages/session-ui/src/timeline/rows-current.test.ts @@ -554,6 +554,54 @@ describe("current session timeline rows", () => { ]) }) + test.each([ + ["grouped", "assistant-part:context:context:msg_a1:msg_a1:text:0"], + ["hidden", "assistant-part:context:context:msg_a1:tool_read"], + ] as const)("%s commentary keeps the answer of each turn", (placement, group) => { + const step = (id: string, content: SessionMessageAssistant["content"]) => + ({ + id, + type: "assistant", + agent: "build", + model: { id: "model", providerID: "provider" }, + content, + time: { created: 2, completed: 3 }, + }) satisfies SessionMessageInfo + + const source = [ + { id: "msg_user", type: "user", text: "fix", time: { created: 1 } }, + step("msg_a1", [{ type: "text", text: "Checking." }, storyTool("tool_read", "read", "completed", {})]), + step("msg_a2", [ + { type: "text", text: "Checks pass.", state: { phase: "commentary" } }, + { type: "text", text: "Fixed." }, + ]), + { id: "msg_stop", type: "user", text: "stop", time: { created: 4 } }, + step("msg_a3", [{ type: "text", text: "Looking." }, storyTool("tool_grep", "grep", "completed", {})]), + ] satisfies SessionMessageInfo[] + + const rows = Timeline.constructSessionMessageRows( + source, + false, + { type: "idle" }, + undefined, + false, + false, + undefined, + { ...timelinePresets[2].value, commentary: { placement } }, + ).rows + + // The interrupted second turn has no answer, so its last commentary stays visible. + expect(rows.map(TimelineRow.key)).toEqual([ + "user-message:msg_user", + group, + "assistant-part:part:part:msg_a2:msg_a2:text:1", + "turn-gap:msg_stop", + "user-message:msg_stop", + "assistant-part:part:part:msg_a3:msg_a3:text:0", + "assistant-part:context:context:msg_a3:tool_grep", + ]) + }) + test("keeps assistant errors and retries before later notices", () => { const result = Timeline.constructSessionMessageRows( [ diff --git a/packages/session-ui/src/timeline/session-timeline-row.tsx b/packages/session-ui/src/timeline/session-timeline-row.tsx index 6515f49d891..8dd9c0a8718 100644 --- a/packages/session-ui/src/timeline/session-timeline-row.tsx +++ b/packages/session-ui/src/timeline/session-timeline-row.tsx @@ -205,6 +205,15 @@ export function createSessionTimelineRowRenderer(input: { ] const message = input.projection.messageByID().get(ref.messageID) + if (content?.type === "text" && message?.type === "assistant") + return [ + { + type: "text", + id: ref.partID, + render: () => , + }, + ] + if (ref.messageID !== ref.partID || !message) return [] if (message.type === "shell") diff --git a/packages/session-ui/src/tools/tool-renderer.tsx b/packages/session-ui/src/tools/tool-renderer.tsx index 315c3ca41b4..91abc49fa5a 100644 --- a/packages/session-ui/src/tools/tool-renderer.tsx +++ b/packages/session-ui/src/tools/tool-renderer.tsx @@ -454,7 +454,7 @@ function ExaOutput(props: { output?: string }) { export type ContextGroupPart = | SessionMessageAssistantTool | (SessionMessageAssistantReasoning & { id: string; streaming?: boolean }) - | { type: "notice" | "shell"; id: string; render: () => JSX.Element } + | { type: "notice" | "shell" | "text"; id: string; render: () => JSX.Element } export function CurrentContextToolGroup(props: { parts: ContextGroupPart[] @@ -650,7 +650,10 @@ export function CurrentContextToolGroup(props: { const callback = createMemo(() => { const value = item() - return !Array.isArray(value) && (value.type === "notice" || value.type === "shell") ? value : undefined + return !Array.isArray(value) && + (value.type === "notice" || value.type === "shell" || value.type === "text") + ? value + : undefined }) return (