mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-09 22:31:08 +00:00
feat: group consecutive list_files tool calls into single UI block
Consolidate consecutive listFilesTopLevel/listFilesRecursive ask messages into a single 'Roo wants to view multiple directories' block, matching the existing read_file batching pattern.
This commit is contained in:
parent
5b0897beb9
commit
056465f456
6 changed files with 253 additions and 4 deletions
|
|
@ -845,6 +845,12 @@ export interface ClineSayTool {
|
|||
startLine?: number
|
||||
}>
|
||||
}>
|
||||
batchDirs?: Array<{
|
||||
path: string
|
||||
recursive: boolean
|
||||
isOutsideWorkspace?: boolean
|
||||
key: string
|
||||
}>
|
||||
question?: string
|
||||
imageData?: string // Base64 encoded image data for generated images
|
||||
// Properties for runSlashCommand tool
|
||||
|
|
|
|||
47
webview-ui/src/components/chat/BatchListFilesPermission.tsx
Normal file
47
webview-ui/src/components/chat/BatchListFilesPermission.tsx
Normal file
|
|
@ -0,0 +1,47 @@
|
|||
import { memo } from "react"
|
||||
|
||||
import { ToolUseBlock, ToolUseBlockHeader } from "../common/ToolUseBlock"
|
||||
import { PathTooltip } from "../ui/PathTooltip"
|
||||
|
||||
interface DirPermissionItem {
|
||||
path: string
|
||||
recursive: boolean
|
||||
isOutsideWorkspace?: boolean
|
||||
key: string
|
||||
}
|
||||
|
||||
interface BatchListFilesPermissionProps {
|
||||
dirs: DirPermissionItem[]
|
||||
ts: number
|
||||
}
|
||||
|
||||
export const BatchListFilesPermission = memo(({ dirs = [], ts }: BatchListFilesPermissionProps) => {
|
||||
if (!dirs?.length) {
|
||||
return null
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="pt-[5px]">
|
||||
<div className="flex flex-col gap-0 border border-border rounded-md p-1">
|
||||
{dirs.map((dir) => {
|
||||
return (
|
||||
<div key={`${dir.path}-${ts}`} className="flex items-center gap-2">
|
||||
<ToolUseBlock className="flex-1">
|
||||
<ToolUseBlockHeader>
|
||||
<PathTooltip content={dir.path}>
|
||||
<span className="whitespace-nowrap overflow-hidden text-ellipsis text-left mr-2 rtl">
|
||||
{dir.path}
|
||||
</span>
|
||||
</PathTooltip>
|
||||
<div className="flex-grow"></div>
|
||||
</ToolUseBlockHeader>
|
||||
</ToolUseBlock>
|
||||
</div>
|
||||
)
|
||||
})}
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
})
|
||||
|
||||
BatchListFilesPermission.displayName = "BatchListFilesPermission"
|
||||
|
|
@ -40,6 +40,7 @@ import { Mention } from "./Mention"
|
|||
import { CheckpointSaved } from "./checkpoints/CheckpointSaved"
|
||||
import { FollowUpSuggest } from "./FollowUpSuggest"
|
||||
import { BatchFilePermission } from "./BatchFilePermission"
|
||||
import { BatchListFilesPermission } from "./BatchListFilesPermission"
|
||||
import { BatchDiffApproval } from "./BatchDiffApproval"
|
||||
import { ProgressIndicator } from "./ProgressIndicator"
|
||||
import { Markdown } from "./Markdown"
|
||||
|
|
@ -741,7 +742,24 @@ export const ChatRowContent = ({
|
|||
</>
|
||||
)
|
||||
}
|
||||
case "listFilesTopLevel":
|
||||
case "listFilesTopLevel": {
|
||||
// Check if this is a batch directory listing request
|
||||
const isBatchDirRequest = message.type === "ask" && tool.batchDirs && Array.isArray(tool.batchDirs)
|
||||
|
||||
if (isBatchDirRequest) {
|
||||
return (
|
||||
<>
|
||||
<div style={headerStyle}>
|
||||
<ListTree className="w-4 shrink-0" aria-label="List files icon" />
|
||||
<span style={{ fontWeight: "bold" }}>
|
||||
{t("chat:directoryOperations.wantsToViewMultipleDirectories")}
|
||||
</span>
|
||||
</div>
|
||||
<BatchListFilesPermission dirs={tool.batchDirs || []} ts={message?.ts} />
|
||||
</>
|
||||
)
|
||||
}
|
||||
|
||||
return (
|
||||
<>
|
||||
<div style={headerStyle}>
|
||||
|
|
@ -767,7 +785,25 @@ export const ChatRowContent = ({
|
|||
</div>
|
||||
</>
|
||||
)
|
||||
case "listFilesRecursive":
|
||||
}
|
||||
case "listFilesRecursive": {
|
||||
// Check if this is a batch directory listing request
|
||||
const isBatchDirRequest = message.type === "ask" && tool.batchDirs && Array.isArray(tool.batchDirs)
|
||||
|
||||
if (isBatchDirRequest) {
|
||||
return (
|
||||
<>
|
||||
<div style={headerStyle}>
|
||||
<FolderTree className="w-4 shrink-0" aria-label="Folder tree icon" />
|
||||
<span style={{ fontWeight: "bold" }}>
|
||||
{t("chat:directoryOperations.wantsToViewMultipleDirectories")}
|
||||
</span>
|
||||
</div>
|
||||
<BatchListFilesPermission dirs={tool.batchDirs || []} ts={message?.ts} />
|
||||
</>
|
||||
)
|
||||
}
|
||||
|
||||
return (
|
||||
<>
|
||||
<div style={headerStyle}>
|
||||
|
|
@ -793,6 +829,7 @@ export const ChatRowContent = ({
|
|||
</div>
|
||||
</>
|
||||
)
|
||||
}
|
||||
case "searchFiles":
|
||||
return (
|
||||
<>
|
||||
|
|
|
|||
|
|
@ -1154,8 +1154,21 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
|
|||
}
|
||||
}
|
||||
|
||||
// Helper to check if a message is a list_files ask that should be batched
|
||||
const isListFilesAsk = (msg: ClineMessage): boolean => {
|
||||
if (msg.type !== "ask" || msg.ask !== "tool") return false
|
||||
try {
|
||||
const tool = JSON.parse(msg.text || "{}")
|
||||
return (
|
||||
(tool.tool === "listFilesTopLevel" || tool.tool === "listFilesRecursive") && !tool.batchDirs // Don't re-batch already batched
|
||||
)
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// Consolidate consecutive read_file ask messages into batches
|
||||
const result: ClineMessage[] = []
|
||||
const readFileBatched: ClineMessage[] = []
|
||||
let i = 0
|
||||
while (i < filtered.length) {
|
||||
const msg = filtered[i]
|
||||
|
|
@ -1199,10 +1212,67 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
|
|||
_batchedMessages: batch,
|
||||
} as ClineMessage & { _batchedMessages: ClineMessage[] }
|
||||
|
||||
result.push(syntheticMessage)
|
||||
readFileBatched.push(syntheticMessage)
|
||||
i = j // Skip past all batched messages
|
||||
} else {
|
||||
// Single read_file ask, keep as-is
|
||||
readFileBatched.push(msg)
|
||||
i++
|
||||
}
|
||||
} else {
|
||||
readFileBatched.push(msg)
|
||||
i++
|
||||
}
|
||||
}
|
||||
|
||||
// Consolidate consecutive list_files ask messages into batches
|
||||
const result: ClineMessage[] = []
|
||||
i = 0
|
||||
while (i < readFileBatched.length) {
|
||||
const msg = readFileBatched[i]
|
||||
|
||||
// Check if this starts a sequence of list_files asks
|
||||
if (isListFilesAsk(msg)) {
|
||||
// Collect all consecutive list_files asks
|
||||
const batch: ClineMessage[] = [msg]
|
||||
let j = i + 1
|
||||
while (j < readFileBatched.length && isListFilesAsk(readFileBatched[j])) {
|
||||
batch.push(readFileBatched[j])
|
||||
j++
|
||||
}
|
||||
|
||||
if (batch.length > 1) {
|
||||
// Create a synthetic batch message
|
||||
const batchDirs = batch.map((batchMsg) => {
|
||||
try {
|
||||
const tool = JSON.parse(batchMsg.text || "{}")
|
||||
return {
|
||||
path: tool.path || "",
|
||||
recursive: tool.tool === "listFilesRecursive",
|
||||
isOutsideWorkspace: tool.isOutsideWorkspace || false,
|
||||
key: tool.path || "",
|
||||
}
|
||||
} catch {
|
||||
return { path: "", recursive: false, key: "" }
|
||||
}
|
||||
})
|
||||
|
||||
// Use the first message as the base, but add batchDirs
|
||||
const firstTool = JSON.parse(msg.text || "{}")
|
||||
const syntheticMessage: ClineMessage = {
|
||||
...msg,
|
||||
text: JSON.stringify({
|
||||
...firstTool,
|
||||
batchDirs,
|
||||
}),
|
||||
// Store original messages for response handling
|
||||
_batchedMessages: batch,
|
||||
} as ClineMessage & { _batchedMessages: ClineMessage[] }
|
||||
|
||||
result.push(syntheticMessage)
|
||||
i = j // Skip past all batched messages
|
||||
} else {
|
||||
// Single list_files ask, keep as-is
|
||||
result.push(msg)
|
||||
i++
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,87 @@
|
|||
import { render, screen } from "@/utils/test-utils"
|
||||
|
||||
import { TranslationProvider } from "@/i18n/__mocks__/TranslationContext"
|
||||
|
||||
import { BatchListFilesPermission } from "../BatchListFilesPermission"
|
||||
|
||||
describe("BatchListFilesPermission", () => {
|
||||
const mockDirs = [
|
||||
{
|
||||
key: "apps/cli",
|
||||
path: "apps/cli",
|
||||
recursive: false,
|
||||
isOutsideWorkspace: false,
|
||||
},
|
||||
{
|
||||
key: "apps/web-roo-code",
|
||||
path: "apps/web-roo-code",
|
||||
recursive: false,
|
||||
isOutsideWorkspace: false,
|
||||
},
|
||||
{
|
||||
key: "packages/core",
|
||||
path: "packages/core",
|
||||
recursive: true,
|
||||
isOutsideWorkspace: false,
|
||||
},
|
||||
]
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
it("renders directory list correctly", () => {
|
||||
render(
|
||||
<TranslationProvider>
|
||||
<BatchListFilesPermission dirs={mockDirs} ts={Date.now()} />
|
||||
</TranslationProvider>,
|
||||
)
|
||||
|
||||
expect(screen.getByText("apps/cli")).toBeInTheDocument()
|
||||
expect(screen.getByText("apps/web-roo-code")).toBeInTheDocument()
|
||||
expect(screen.getByText("packages/core")).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it("renders nothing when dirs array is empty", () => {
|
||||
const { container } = render(
|
||||
<TranslationProvider>
|
||||
<BatchListFilesPermission dirs={[]} ts={Date.now()} />
|
||||
</TranslationProvider>,
|
||||
)
|
||||
|
||||
expect(container.firstChild).toBeNull()
|
||||
})
|
||||
|
||||
it("re-renders when timestamp changes", () => {
|
||||
const { rerender } = render(
|
||||
<TranslationProvider>
|
||||
<BatchListFilesPermission dirs={mockDirs} ts={1000} />
|
||||
</TranslationProvider>,
|
||||
)
|
||||
|
||||
expect(screen.getByText("apps/cli")).toBeInTheDocument()
|
||||
|
||||
rerender(
|
||||
<TranslationProvider>
|
||||
<BatchListFilesPermission dirs={mockDirs} ts={2000} />
|
||||
</TranslationProvider>,
|
||||
)
|
||||
|
||||
expect(screen.getByText("apps/cli")).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it("renders all directories in a single container", () => {
|
||||
render(
|
||||
<TranslationProvider>
|
||||
<BatchListFilesPermission dirs={mockDirs} ts={Date.now()} />
|
||||
</TranslationProvider>,
|
||||
)
|
||||
|
||||
// All directories should be within a single bordered container
|
||||
const container = screen.getByText("apps/cli").closest(".border.border-border.rounded-md")
|
||||
expect(container).toBeInTheDocument()
|
||||
|
||||
// All 3 dirs should be inside this container
|
||||
expect(container?.querySelectorAll(".flex.items-center.gap-2")).toHaveLength(mockDirs.length)
|
||||
})
|
||||
})
|
||||
|
|
@ -235,6 +235,8 @@
|
|||
"didViewRecursive": "Roo recursively viewed all files in this directory",
|
||||
"wantsToViewRecursiveOutsideWorkspace": "Roo wants to recursively view all files in this directory (outside workspace)",
|
||||
"didViewRecursiveOutsideWorkspace": "Roo recursively viewed all files in this directory (outside workspace)",
|
||||
"wantsToViewMultipleDirectories": "Roo wants to view multiple directories",
|
||||
"didViewMultipleDirectories": "Roo viewed multiple directories",
|
||||
"wantsToSearch": "Roo wants to search this directory for <code>{{regex}}</code>",
|
||||
"didSearch": "Roo searched this directory for <code>{{regex}}</code>",
|
||||
"wantsToSearchOutsideWorkspace": "Roo wants to search this directory (outside workspace) for <code>{{regex}}</code>",
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue