소스 검색

refactor(tui): simplify inline tool spacing (#33097)

Dax 2 달 전
부모
커밋
009f3799cd

+ 11 - 36
packages/tui/src/routes/session/index.tsx

@@ -91,6 +91,8 @@ const GO_UPSELL_ACCOUNT_RATE_LIMIT_DONT_SHOW = "go_upsell_account_rate_limit_don
 const GO_UPSELL_WINDOW = 86_400_000 // 24 hrs
 const GO_UPSELL_PROVIDERS = new Set(["opencode", "opencode-go"])
 
+export const alwaysSeparate = new WeakSet<BoxRenderable>()
+
 type RetryAction = Extract<SessionStatus, { type: "retry" }>["action"]
 
 function goUpsellKeys(action: RetryAction) {
@@ -160,7 +162,6 @@ const context = createContext<{
   showTimestamps: () => boolean
   showDetails: () => boolean
   showGenericToolOutput: () => boolean
-  userMessageIDs: () => ReadonlySet<string>
   diffWrapMode: () => "word" | "none"
   providers: () => ReadonlyMap<string, Provider>
   sync: ReturnType<typeof useSync>
@@ -218,14 +219,6 @@ export function Session() {
         )
       : [],
   )
-  const userMessageIDs = createMemo(
-    () =>
-      new Set(
-        messages()
-          .filter((message) => message.role === "user")
-          .map((message) => message.id),
-      ),
-  )
   const permissions = createMemo(() => {
     if (session()?.parentID) return []
     return children().flatMap((x) => sync.data.permission[x.id] ?? [])
@@ -1158,7 +1151,6 @@ export function Session() {
           showTimestamps,
           showDetails,
           showGenericToolOutput,
-          userMessageIDs,
           diffWrapMode,
           providers,
           sync,
@@ -1395,6 +1387,7 @@ function UserMessage(props: {
       <Show when={text()}>
         <box
           id={props.message.id}
+          ref={(el: BoxRenderable) => alwaysSeparate.add(el)}
           border={["left"]}
           borderColor={color()}
           customBorderChars={SplitBorder.customBorderChars}
@@ -1532,7 +1525,7 @@ function AssistantMessage(props: { message: AssistantMessage; parts: Part[]; las
       </Show>
       <Show when={props.message.error && props.message.error.name !== "MessageAbortedError"}>
         <box
-          id={`assistant-error-${props.message.id}`}
+          ref={(el: BoxRenderable) => alwaysSeparate.add(el)}
           border={["left"]}
           paddingTop={1}
           paddingBottom={1}
@@ -1547,7 +1540,7 @@ function AssistantMessage(props: { message: AssistantMessage; parts: Part[]; las
       </Show>
       <Switch>
         <Match when={props.last || final() || props.message.error?.name === "MessageAbortedError"}>
-          <box id={`assistant-summary-${props.message.id}`} paddingLeft={3}>
+          <box ref={(el: BoxRenderable) => alwaysSeparate.add(el)} paddingLeft={3}>
             <text marginTop={1}>
               <span
                 style={{
@@ -1613,7 +1606,7 @@ function ReasoningPart(props: { last: boolean; part: ReasoningPart; message: Ass
   return (
     <Show when={content()}>
       <box
-        id={`text-${props.part.messageID}-${props.part.id}`}
+        ref={(el: BoxRenderable) => alwaysSeparate.add(el)}
         paddingLeft={3}
         marginTop={1}
         flexDirection="column"
@@ -1695,7 +1688,7 @@ function TextPart(props: { last: boolean; part: TextPart; message: AssistantMess
   const { theme, syntax } = useTheme()
   return (
     <Show when={props.part.text.trim()}>
-      <box id={`text-${props.part.messageID}-${props.part.id}`} paddingLeft={3} marginTop={1} flexShrink={0}>
+      <box ref={(el: BoxRenderable) => alwaysSeparate.add(el)} paddingLeft={3} marginTop={1} flexShrink={0}>
         <markdown
           syntaxStyle={syntax()}
           streaming={true}
@@ -1845,7 +1838,6 @@ function InlineTool(props: {
   pending: string
   failure?: string
   spinner?: boolean
-  subagent?: boolean
   children: JSX.Element
   part: ToolPart
   onClick?: () => void
@@ -1886,7 +1878,6 @@ function InlineTool(props: {
 
   return (
     <InlineToolRow
-      id={`tool-inline-${props.subagent ? "subagent-" : ""}${props.part.messageID}-${props.part.id}`}
       icon={props.icon}
       iconColor={props.iconColor}
       color={fg()}
@@ -1899,8 +1890,6 @@ function InlineTool(props: {
       pending={props.pending}
       failure={props.failure}
       spinner={props.spinner}
-      subagent={props.subagent}
-      separateAfter={(id) => id !== undefined && ctx.userMessageIDs().has(id)}
       onMouseOver={() => clickable() && setHover(true)}
       onMouseOut={() => setHover(false)}
       onMouseUp={() => {
@@ -1918,7 +1907,6 @@ function InlineTool(props: {
 }
 
 export function InlineToolRow(props: {
-  id?: string
   icon: string
   iconColor?: RGBA
   color?: RGBA
@@ -1931,32 +1919,20 @@ export function InlineToolRow(props: {
   pending: string
   failure?: string
   spinner?: boolean
-  subagent?: boolean
   children: JSX.Element
-  separateAfter?: (id: string | undefined) => boolean
   onMouseOver?: () => void
   onMouseOut?: () => void
   onMouseUp?: () => void
 }) {
   return (
     <box
-      id={props.id}
       paddingLeft={3}
       onMouseOver={props.onMouseOver}
       onMouseOut={props.onMouseOut}
       onMouseUp={props.onMouseUp}
       ref={(el: BoxRenderable) => {
         setPreLayoutSiblingMargin(el, (previous) => {
-          const previousInline = previous?.id.startsWith("tool-inline-") ?? false
-          const previousSubagent = previous?.id.startsWith("tool-inline-subagent-") ?? false
-          return previous?.id.startsWith("text-") ||
-            previous?.id.startsWith("tool-block-") ||
-            previous?.id.startsWith("assistant-error-") ||
-            previous?.id.startsWith("assistant-summary-") ||
-            (previousInline && previousSubagent !== Boolean(props.subagent)) ||
-            props.separateAfter?.(previous?.id)
-            ? 1
-            : 0
+          return previous instanceof BoxRenderable && (previous.height > 1 || alwaysSeparate.has(previous)) ? 1 : 0
         })
       }}
     >
@@ -2018,7 +1994,7 @@ function BlockTool(props: {
   const error = createMemo(() => (props.part?.state.status === "error" ? props.part.state.error : undefined))
   return (
     <box
-      id={props.part ? `tool-block-${props.part.messageID}-${props.part.id}` : undefined}
+      ref={(el: BoxRenderable) => alwaysSeparate.add(el)}
       border={["left"]}
       paddingTop={1}
       paddingBottom={1}
@@ -2184,8 +2160,8 @@ function Read(props: ToolProps) {
         Read {pathFormatter.format(stringValue(props.input.filePath))} {input(props.input, ["filePath"])}
       </InlineTool>
       <For each={loaded()}>
-        {(filepath, index) => (
-          <box id={`tool-inline-loaded-${props.part.messageID}-${props.part.id}-${index()}`} paddingLeft={3}>
+        {(filepath) => (
+          <box paddingLeft={3}>
             <text paddingLeft={3} fg={theme.textMuted}>
               ↳ Loaded {pathFormatter.format(filepath)}
             </text>
@@ -2305,7 +2281,6 @@ function Task(props: ToolProps) {
   return (
     <InlineTool
       icon={props.part.state.status === "completed" ? "✓" : "│"}
-      subagent={true}
       color={retry() ? theme.error : undefined}
       spinner={isRunning()}
       complete={stringValue(props.input.description)}

+ 9 - 6
packages/tui/test/cli/tui/__snapshots__/inline-tool-wrap-snapshot.test.tsx.snap

@@ -3,6 +3,7 @@
 exports[`TUI inline tool wrapping snapshots consecutive grep, glob, and read rows at a narrow width 1`] = `
 "   ✱ Grep "OPENCODE.*DB|database|sqlite|drizzle|dev.*db|data.
      *dir|xdg|APPDATA" in packages/opencode/src (151 matches)
+
    ✱ Glob "**/*db*" in packages/opencode (6 matches)
    → Read packages/opencode/src/storage/db.ts [offset=1, limit=130]
    → Read packages/opencode/src/index.ts [offset=1, limit=100]
@@ -13,10 +14,12 @@ exports[`TUI inline tool wrapping snapshots consecutive grep, glob, and read row
 exports[`TUI inline tool wrapping snapshots expanded tool errors under the tool text 1`] = `
 "   ✱ Grep "OPENCODE.*DB|database|sqlite|drizzle|dev.*db|data.
      *dir|xdg|APPDATA" in packages/opencode/src (151 matches)
+
    ✱ Glob "**/*db*" in packages/opencode (6 matches)
    → Read packages/opencode/src/storage/db.ts [offset=1, limit=130]
    → Read packages/opencode/src/index.ts [offset=1, limit=100]
      No LSP server available for this file type.
+
    ✱ Grep "export const OPENCODE_DB|OPENCODE_DB|OPENCODE_DEV|Global\\.
      Path\\.data|data =" in packages/opencode/src (115 matches)"
 `;
@@ -32,6 +35,7 @@ exports[`TUI inline tool wrapping keeps separation after a shell output block 1`
 
    ✱ Grep "OPENCODE.*DB|database|sqlite|drizzle|dev.*db|data.
      *dir|xdg|APPDATA" in packages/opencode/src (151 matches)
+
    ✱ Glob "**/*db*" in packages/opencode (6 matches)
    → Read packages/opencode/src/storage/db.ts [offset=1, limit=130]
    → Read packages/opencode/src/index.ts [offset=1, limit=100]
@@ -46,6 +50,7 @@ exports[`TUI inline tool wrapping keeps separation after a padded user message 1
 
    ✱ Grep "OPENCODE.*DB|database|sqlite|drizzle|dev.*db|data.
      *dir|xdg|APPDATA" in packages/opencode/src (151 matches)
+
    ✱ Glob "**/*db*" in packages/opencode (6 matches)
    → Read packages/opencode/src/storage/db.ts [offset=1, limit=130]
    → Read packages/opencode/src/index.ts [offset=1, limit=100]
@@ -53,9 +58,8 @@ exports[`TUI inline tool wrapping keeps separation after a padded user message 1
      Path\\.data|data =" in packages/opencode/src (115 matches)"
 `;
 
-exports[`TUI inline tool wrapping separates a contiguous subagent group from inline tools 1`] = `
+exports[`TUI inline tool wrapping separates after a multi-line task row 1`] = `
 "   ✱ Grep "Task" (2 matches)
-
    ⠙ Explore Task — Inspect active task spacing
    ✓ General Task — Confirm completed task spacing
      ↳ 1 toolcall · 501ms
@@ -63,22 +67,21 @@ exports[`TUI inline tool wrapping separates a contiguous subagent group from inl
    → Read src/cli/cmd/tui/routes/session/index.tsx"
 `;
 
-exports[`TUI inline tool wrapping separates a subagent group after an expanded read 1`] = `
+exports[`TUI inline tool wrapping does not treat task rows differently from other inline rows 1`] = `
 "   → Read src/cli/cmd/tui/routes/session/index.tsx
    ↳ Loaded src/cli/cmd/tui/routes/session/tools.tsx
-
    ✓ Explore Task — Inspect active task spacing
      ↳ 1 toolcall · 501ms"
 `;
 
-exports[`TUI inline tool wrapping separates a subagent from the previous assistant summary 1`] = `
+exports[`TUI inline tool wrapping separates an inline row from the previous assistant summary 1`] = `
 "   ▣ Build · Little Frank · 53.1s
 
    ✓ Build Task — Review changes
      ↳ 48 toolcalls · 1m 40s"
 `;
 
-exports[`TUI inline tool wrapping separates a subagent from the previous assistant error 1`] = `
+exports[`TUI inline tool wrapping separates an inline row from the previous assistant error 1`] = `
 "│
 │  Managed inference requires an active Member plan

+ 42 - 30
packages/tui/test/cli/tui/inline-tool-wrap-snapshot.test.tsx

@@ -1,6 +1,6 @@
 import { afterEach, describe, expect, test } from "bun:test"
 import { createSignal, For, Show } from "solid-js"
-import type { ScrollBoxRenderable } from "@opentui/core"
+import type { BoxRenderable, ScrollBoxRenderable } from "@opentui/core"
 import { testRender, type JSX } from "@opentui/solid"
 import {
   formatCompletedSubagentDetail,
@@ -13,6 +13,7 @@ import {
   parseQuestionAnswers,
   parseQuestions,
   parseTodos,
+  alwaysSeparate,
   toolDisplay,
 } from "../../../src/routes/session"
 
@@ -53,7 +54,14 @@ const tools: readonly ToolFixture[] = [
 
 function ShellOutput() {
   return (
-    <box id="tool-block-shell" marginTop={1} paddingTop={1} paddingBottom={1} paddingLeft={2} gap={1}>
+    <box
+      ref={(el: BoxRenderable) => alwaysSeparate.add(el)}
+      marginTop={1}
+      paddingTop={1}
+      paddingBottom={1}
+      paddingLeft={2}
+      gap={1}
+    >
       <text paddingLeft={3}># List files</text>
       <box gap={1}>
         <text>$ ls</text>
@@ -65,7 +73,7 @@ function ShellOutput() {
 
 function UserMessage() {
   return (
-    <box id="message-user">
+    <box ref={(el: BoxRenderable) => alwaysSeparate.add(el)}>
       <box paddingTop={1} paddingBottom={1} paddingLeft={2}>
         <text>Check whether the next tool remains separated.</text>
       </box>
@@ -88,7 +96,6 @@ function Fixture(props: { errorExpanded?: boolean; before?: "shell" | "user" })
               failed={Boolean(item.error)}
               error={item.error}
               errorExpanded={props.errorExpanded}
-              separateAfter={(id) => id === "message-user"}
             >
               {item.label}
             </InlineToolRow>
@@ -99,61 +106,67 @@ function Fixture(props: { errorExpanded?: boolean; before?: "shell" | "user" })
   )
 }
 
-function SubagentGroupFixture() {
+function TaskRowsFixture() {
   return (
     <box flexDirection="column" width={72}>
-      <InlineToolRow id="tool-inline-before" icon="✱" complete={true} pending="">
+      <InlineToolRow icon="✱" complete={true} pending="">
         Grep "Task" (2 matches)
       </InlineToolRow>
-      <InlineToolRow id="tool-inline-subagent-one" icon="⠙" complete={true} pending="" subagent={true}>
+      <InlineToolRow icon="⠙" complete={true} pending="">
         Explore Task — Inspect active task spacing
       </InlineToolRow>
-      <InlineToolRow id="tool-inline-subagent-two" icon="✓" complete={true} pending="" subagent={true}>
+      <InlineToolRow icon="✓" complete={true} pending="">
         {"General Task — Confirm completed task spacing\n↳ 1 toolcall · 501ms"}
       </InlineToolRow>
-      <InlineToolRow id="tool-inline-after" icon="→" complete={true} pending="">
+      <InlineToolRow icon="→" complete={true} pending="">
         Read src/cli/cmd/tui/routes/session/index.tsx
       </InlineToolRow>
     </box>
   )
 }
 
-function LoadedReadBeforeSubagentFixture() {
+function LoadedReadBeforeTaskFixture() {
   return (
     <box flexDirection="column" width={72}>
-      <InlineToolRow id="tool-inline-read" icon="→" complete={true} pending="">
+      <InlineToolRow icon="→" complete={true} pending="">
         Read src/cli/cmd/tui/routes/session/index.tsx
       </InlineToolRow>
-      <box id="tool-inline-loaded-read-child" paddingLeft={3}>
+      <box paddingLeft={3}>
         <text paddingLeft={3}>↳ Loaded src/cli/cmd/tui/routes/session/tools.tsx</text>
       </box>
-      <InlineToolRow id="tool-inline-subagent-after-read" icon="✓" complete={true} pending="" subagent={true}>
+      <InlineToolRow icon="✓" complete={true} pending="">
         {"Explore Task — Inspect active task spacing\n↳ 1 toolcall · 501ms"}
       </InlineToolRow>
     </box>
   )
 }
 
-function AssistantSummaryBeforeSubagentFixture() {
+function AssistantSummaryBeforeInlineFixture() {
   return (
     <box flexDirection="column" width={72}>
-      <box id="assistant-summary-message-one" paddingLeft={3}>
+      <box ref={(el: BoxRenderable) => alwaysSeparate.add(el)} paddingLeft={3}>
         <text>▣ Build · Little Frank · 53.1s</text>
       </box>
-      <InlineToolRow id="tool-inline-subagent-one" icon="✓" complete={true} pending="" subagent={true}>
+      <InlineToolRow icon="✓" complete={true} pending="">
         {"Build Task — Review changes\n↳ 48 toolcalls · 1m 40s"}
       </InlineToolRow>
     </box>
   )
 }
 
-function AssistantErrorBeforeSubagentFixture() {
+function AssistantErrorBeforeInlineFixture() {
   return (
     <box flexDirection="column" width={72}>
-      <box id="assistant-error-message-one" border={["left"]} paddingTop={1} paddingBottom={1} paddingLeft={2}>
+      <box
+        ref={(el: BoxRenderable) => alwaysSeparate.add(el)}
+        border={["left"]}
+        paddingTop={1}
+        paddingBottom={1}
+        paddingLeft={2}
+      >
         <text>Managed inference requires an active Member plan</text>
       </box>
-      <InlineToolRow id="tool-inline-subagent-one" icon="✓" complete={true} pending="" subagent={true}>
+      <InlineToolRow icon="✓" complete={true} pending="">
         {"Build Task — Review changes\n↳ 48 toolcalls · 1m 40s"}
       </InlineToolRow>
     </box>
@@ -170,7 +183,7 @@ function StickyScrollFixture(props: { separated: boolean; scroll: (scroll: Scrol
         <text>Second row</text>
       </box>
       <Show when={props.separated}>
-        <box id="text-before-tool">
+        <box ref={(el: BoxRenderable) => alwaysSeparate.add(el)}>
           <text>Assistant text</text>
         </box>
       </Show>
@@ -200,6 +213,7 @@ function FailedCompleteToolFixture() {
 async function renderFrame(component: () => JSX.Element, options: { width: number; height: number }) {
   testSetup = await testRender(component, options)
   await testSetup.renderOnce()
+  await testSetup.renderOnce()
 
   return testSetup
     .captureCharFrame()
@@ -294,22 +308,20 @@ describe("TUI inline tool wrapping", () => {
     expect(await renderFrame(() => <Fixture before="user" />, { width: 72, height: 14 })).toMatchSnapshot()
   })
 
-  test("separates a contiguous subagent group from inline tools", async () => {
-    expect(await renderFrame(() => <SubagentGroupFixture />, { width: 72, height: 10 })).toMatchSnapshot()
+  test("separates after a multi-line task row", async () => {
+    expect(await renderFrame(() => <TaskRowsFixture />, { width: 72, height: 10 })).toMatchSnapshot()
   })
 
-  test("separates a subagent group after an expanded read", async () => {
-    expect(await renderFrame(() => <LoadedReadBeforeSubagentFixture />, { width: 72, height: 8 })).toMatchSnapshot()
+  test("does not treat task rows differently from other inline rows", async () => {
+    expect(await renderFrame(() => <LoadedReadBeforeTaskFixture />, { width: 72, height: 8 })).toMatchSnapshot()
   })
 
-  test("separates a subagent from the previous assistant summary", async () => {
-    expect(
-      await renderFrame(() => <AssistantSummaryBeforeSubagentFixture />, { width: 72, height: 5 }),
-    ).toMatchSnapshot()
+  test("separates an inline row from the previous assistant summary", async () => {
+    expect(await renderFrame(() => <AssistantSummaryBeforeInlineFixture />, { width: 72, height: 5 })).toMatchSnapshot()
   })
 
-  test("separates a subagent from the previous assistant error", async () => {
-    expect(await renderFrame(() => <AssistantErrorBeforeSubagentFixture />, { width: 72, height: 7 })).toMatchSnapshot()
+  test("separates an inline row from the previous assistant error", async () => {
+    expect(await renderFrame(() => <AssistantErrorBeforeInlineFixture />, { width: 72, height: 7 })).toMatchSnapshot()
   })
 
   test("updates sticky-bottom geometry when a text separator mounts and unmounts", async () => {