-
Notifications
You must be signed in to change notification settings - Fork 327
Improve action log step summary: 2-line tool previews and nicer agent messages #24558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -994,6 +994,63 @@ function formatToolCallAsDetails(options) { | |
| return `<details>\n<summary>${fullSummary}</summary>\n\n${detailsContent}\n</details>\n\n`; | ||
| } | ||
|
|
||
| /** | ||
| * Formats a tool result content into a preview string showing the first 2 non-empty lines. | ||
| * Uses tree-branch characters (├, └) for visual hierarchy in copilot-cli style. | ||
| * | ||
| * Examples: | ||
| * 1 line: " └ result text" | ||
| * 2 lines: " ├ line 1\n └ line 2" | ||
| * 3+ lines: " ├ line 1\n └ line 2 (+ 1 more)" | ||
| * | ||
| * @param {string} resultText - The result text to preview | ||
| * @param {number} [maxLineLength=80] - Maximum characters per preview line | ||
| * @returns {string} Formatted preview string, or empty string if no content | ||
| */ | ||
| function formatResultPreview(resultText, maxLineLength = 80) { | ||
| if (!resultText) return ""; | ||
|
|
||
| // Scan line-by-line to avoid building a full array for large outputs. | ||
| // Normalize CRLF by stripping trailing \r from each line. | ||
| let firstLine = ""; | ||
| let secondLine = ""; | ||
| let nonEmptyLineCount = 0; | ||
| let start = 0; | ||
|
|
||
| while (start <= resultText.length) { | ||
| const newlineIndex = resultText.indexOf("\n", start); | ||
| const end = newlineIndex === -1 ? resultText.length : newlineIndex; | ||
| // Strip trailing \r to handle Windows CRLF line endings | ||
| const rawLine = resultText.substring(start, end).replace(/\r$/, ""); | ||
|
|
||
| if (rawLine.trim()) { | ||
| nonEmptyLineCount += 1; | ||
| if (nonEmptyLineCount === 1) { | ||
| const truncated = rawLine.substring(0, maxLineLength); | ||
| firstLine = rawLine.length > maxLineLength ? truncated + "..." : truncated; | ||
| } else if (nonEmptyLineCount === 2) { | ||
| const truncated = rawLine.substring(0, maxLineLength); | ||
| secondLine = rawLine.length > maxLineLength ? truncated + "..." : truncated; | ||
| } | ||
| } | ||
|
|
||
| if (newlineIndex === -1) { | ||
| break; | ||
| } | ||
| start = newlineIndex + 1; | ||
| } | ||
|
|
||
| if (nonEmptyLineCount === 0) return ""; | ||
| if (nonEmptyLineCount === 1) { | ||
| return ` └ ${firstLine}`; | ||
| } | ||
| if (nonEmptyLineCount === 2) { | ||
| return ` ├ ${firstLine}\n └ ${secondLine}`; | ||
| } | ||
|
|
||
| return ` ├ ${firstLine}\n └ ${secondLine} (+ ${nonEmptyLineCount - 2} more)`; | ||
| } | ||
|
|
||
| /** | ||
| * Generates a lightweight plain text summary optimized for raw text rendering. | ||
| * This is designed for console output (core.info) instead of markdown step summaries. | ||
|
|
@@ -1065,14 +1122,15 @@ function generatePlainTextSummary(logEntries, options = {}) { | |
| displayText = displayText.substring(0, MAX_AGENT_TEXT_LENGTH) + `... [truncated: showing first ${MAX_AGENT_TEXT_LENGTH} of ${text.length} chars]`; | ||
| } | ||
|
|
||
| // Split into lines and add Agent prefix | ||
| // Split into lines: first line gets "◆ " prefix, continuation lines are indented | ||
| const textLines = displayText.split("\n"); | ||
| for (const line of textLines) { | ||
| for (let i = 0; i < textLines.length; i++) { | ||
| if (conversationLineCount >= MAX_CONVERSATION_LINES) { | ||
| conversationTruncated = true; | ||
| break; | ||
| } | ||
| lines.push(`Agent: ${line}`); | ||
| const prefix = i === 0 ? "◆ " : " "; | ||
| lines.push(`${prefix}${textLines[i]}`); | ||
| conversationLineCount++; | ||
| } | ||
| lines.push(""); // Add blank line after agent response | ||
|
|
@@ -1100,38 +1158,28 @@ function generatePlainTextSummary(logEntries, options = {}) { | |
| const cmd = formatBashCommand(input.command || ""); | ||
| displayName = `$ ${cmd}`; | ||
|
|
||
| // Show result preview if available | ||
| // Show first 2 lines of result using copilot-cli tree-branch style | ||
| if (toolResult && toolResult.content) { | ||
| const resultText = typeof toolResult.content === "string" ? toolResult.content : String(toolResult.content); | ||
| const resultLines = resultText.split("\n").filter(l => l.trim()); | ||
| if (resultLines.length > 0) { | ||
| const previewLine = resultLines[0].substring(0, 80); | ||
| if (resultLines.length > 1) { | ||
| resultPreview = ` └ ${resultLines.length} lines...`; | ||
| } else if (previewLine) { | ||
| resultPreview = ` └ ${previewLine}`; | ||
| } | ||
| } | ||
| resultPreview = formatResultPreview(resultText); | ||
| } | ||
| } else if (toolName.startsWith("mcp__")) { | ||
| // Format MCP tool names like github-list_pull_requests | ||
| const formattedName = formatMcpName(toolName).replace("::", "-"); | ||
| displayName = formatToolDisplayName(formattedName, input); | ||
|
|
||
| // Show result preview if available | ||
| // Show first 2 lines of result using copilot-cli tree-branch style | ||
| if (toolResult && toolResult.content) { | ||
| const resultText = typeof toolResult.content === "string" ? toolResult.content : JSON.stringify(toolResult.content); | ||
| const truncated = resultText.length > 80 ? resultText.substring(0, 80) + "..." : resultText; | ||
| resultPreview = ` └ ${truncated}`; | ||
| resultPreview = formatResultPreview(resultText); | ||
| } | ||
| } else { | ||
| displayName = formatToolDisplayName(toolName, input); | ||
|
|
||
| // Show result preview if available | ||
| // Show first 2 lines of result using copilot-cli tree-branch style | ||
| if (toolResult && toolResult.content) { | ||
| const resultText = typeof toolResult.content === "string" ? toolResult.content : String(toolResult.content); | ||
| const truncated = resultText.length > 80 ? resultText.substring(0, 80) + "..." : resultText; | ||
| resultPreview = ` └ ${truncated}`; | ||
| resultPreview = formatResultPreview(resultText); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1140,7 +1188,7 @@ function generatePlainTextSummary(logEntries, options = {}) { | |
|
|
||
| if (resultPreview) { | ||
| lines.push(resultPreview); | ||
| conversationLineCount++; | ||
| conversationLineCount += resultPreview.split("\n").length; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| } | ||
|
|
||
| lines.push(""); // Add blank line after tool execution | ||
|
|
@@ -1279,14 +1327,15 @@ function generateCopilotCliStyleSummary(logEntries, options = {}) { | |
| displayText = displayText.substring(0, MAX_AGENT_TEXT_LENGTH) + `... [truncated: showing first ${MAX_AGENT_TEXT_LENGTH} of ${text.length} chars]`; | ||
| } | ||
|
|
||
| // Split into lines and add Agent prefix | ||
| // Split into lines: first line gets "◆ " prefix, continuation lines are indented | ||
| const textLines = displayText.split("\n"); | ||
| for (const line of textLines) { | ||
| for (let i = 0; i < textLines.length; i++) { | ||
| if (conversationLineCount >= MAX_CONVERSATION_LINES) { | ||
| conversationTruncated = true; | ||
| break; | ||
| } | ||
| lines.push(`Agent: ${line}`); | ||
| const prefix = i === 0 ? "◆ " : " "; | ||
| lines.push(`${prefix}${textLines[i]}`); | ||
| conversationLineCount++; | ||
| } | ||
| lines.push(""); // Add blank line after agent response | ||
|
|
@@ -1314,38 +1363,28 @@ function generateCopilotCliStyleSummary(logEntries, options = {}) { | |
| const cmd = formatBashCommand(input.command || ""); | ||
| displayName = `$ ${cmd}`; | ||
|
|
||
| // Show result preview if available | ||
| // Show first 2 lines of result using copilot-cli tree-branch style | ||
| if (toolResult && toolResult.content) { | ||
| const resultText = typeof toolResult.content === "string" ? toolResult.content : String(toolResult.content); | ||
| const resultLines = resultText.split("\n").filter(l => l.trim()); | ||
| if (resultLines.length > 0) { | ||
| const previewLine = resultLines[0].substring(0, 80); | ||
| if (resultLines.length > 1) { | ||
| resultPreview = ` └ ${resultLines.length} lines...`; | ||
| } else if (previewLine) { | ||
| resultPreview = ` └ ${previewLine}`; | ||
| } | ||
| } | ||
| resultPreview = formatResultPreview(resultText); | ||
| } | ||
| } else if (toolName.startsWith("mcp__")) { | ||
| // Format MCP tool names like github-list_pull_requests | ||
| const formattedName = formatMcpName(toolName).replace("::", "-"); | ||
| displayName = formatToolDisplayName(formattedName, input); | ||
|
|
||
| // Show result preview if available | ||
| // Show first 2 lines of result using copilot-cli tree-branch style | ||
| if (toolResult && toolResult.content) { | ||
| const resultText = typeof toolResult.content === "string" ? toolResult.content : JSON.stringify(toolResult.content); | ||
| const truncated = resultText.length > 80 ? resultText.substring(0, 80) + "..." : resultText; | ||
| resultPreview = ` └ ${truncated}`; | ||
| resultPreview = formatResultPreview(resultText); | ||
| } | ||
| } else { | ||
| displayName = formatToolDisplayName(toolName, input); | ||
|
|
||
| // Show result preview if available | ||
| // Show first 2 lines of result using copilot-cli tree-branch style | ||
| if (toolResult && toolResult.content) { | ||
| const resultText = typeof toolResult.content === "string" ? toolResult.content : String(toolResult.content); | ||
| const truncated = resultText.length > 80 ? resultText.substring(0, 80) + "..." : resultText; | ||
| resultPreview = ` └ ${truncated}`; | ||
| resultPreview = formatResultPreview(resultText); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1354,7 +1393,7 @@ function generateCopilotCliStyleSummary(logEntries, options = {}) { | |
|
|
||
| if (resultPreview) { | ||
| lines.push(resultPreview); | ||
| conversationLineCount++; | ||
| conversationLineCount += resultPreview.split("\n").length; | ||
| } | ||
|
|
||
| lines.push(""); // Add blank line after tool execution | ||
|
|
@@ -1628,6 +1667,7 @@ module.exports = { | |
| formatToolUse, | ||
| parseLogEntries, | ||
| formatToolCallAsDetails, | ||
| formatResultPreview, | ||
| generatePlainTextSummary, | ||
| generateCopilotCliStyleSummary, | ||
| wrapAgentLogInSection, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1742,9 +1742,9 @@ describe("log_parser_shared.cjs", () => { | |
| const result = generatePlainTextSummary(logEntries, { parserName: "Agent" }); | ||
|
|
||
| expect(result).toContain("Conversation:"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The updated test expectations correctly reflect the new |
||
| expect(result).toContain("Agent: I'll help you with that task."); | ||
| expect(result).toContain("◆ I'll help you with that task."); | ||
| expect(result).toContain("✓ $ echo hello"); | ||
| expect(result).toContain("Agent: The command executed successfully!"); | ||
| expect(result).toContain("◆ The command executed successfully!"); | ||
| }); | ||
|
|
||
| it("should truncate long agent responses", async () => { | ||
|
|
@@ -1764,7 +1764,7 @@ describe("log_parser_shared.cjs", () => { | |
|
|
||
| const result = generatePlainTextSummary(logEntries, { parserName: "Agent" }); | ||
|
|
||
| expect(result).toContain("Agent: " + "a".repeat(2000) + "... [truncated: showing first 2000 of 2100 chars]"); | ||
| expect(result).toContain("◆ " + "a".repeat(2000) + "... [truncated: showing first 2000 of 2100 chars]"); | ||
| expect(result).not.toContain("a".repeat(2001)); | ||
| }); | ||
|
|
||
|
|
@@ -1784,9 +1784,9 @@ describe("log_parser_shared.cjs", () => { | |
|
|
||
| const result = generatePlainTextSummary(logEntries, { parserName: "Agent" }); | ||
|
|
||
| expect(result).toContain("Agent: Line 1"); | ||
| expect(result).toContain("Agent: Line 2"); | ||
| expect(result).toContain("Agent: Line 3"); | ||
| expect(result).toContain("◆ Line 1"); | ||
| expect(result).toContain(" Line 2"); | ||
| expect(result).toContain(" Line 3"); | ||
| }); | ||
| }); | ||
|
|
||
|
|
@@ -1827,11 +1827,12 @@ describe("log_parser_shared.cjs", () => { | |
| expect(result).toContain("Conversation:"); | ||
|
|
||
| // Check for Agent message | ||
| expect(result).toContain("Agent: I'll help you explore the repository structure first."); | ||
| expect(result).toContain("◆ I'll help you explore the repository structure first."); | ||
|
|
||
| // Check for tool execution with success icon | ||
| // Check for tool execution with success icon and first 2 lines of output | ||
| expect(result).toContain("✓ $ ls -la"); | ||
| expect(result).toContain(" └ 3 lines..."); | ||
| expect(result).toContain(" ├ file1.txt"); | ||
| expect(result).toContain(" └ file2.txt (+ 1 more)"); | ||
|
|
||
| // Check for Statistics section | ||
| expect(result).toContain("Statistics:"); | ||
|
|
@@ -1884,7 +1885,7 @@ describe("log_parser_shared.cjs", () => { | |
|
|
||
| const result = generateCopilotCliStyleSummary(logEntries, { parserName: "Agent" }); | ||
|
|
||
| expect(result).toContain("Agent: " + "a".repeat(2000) + "... [truncated: showing first 2000 of 2100 chars]"); | ||
| expect(result).toContain("◆ " + "a".repeat(2000) + "... [truncated: showing first 2000 of 2100 chars]"); | ||
| }); | ||
|
|
||
| it("should skip internal file operation tools", async () => { | ||
|
|
@@ -1940,9 +1941,9 @@ describe("log_parser_shared.cjs", () => { | |
|
|
||
| const result = generateCopilotCliStyleSummary(logEntries, { parserName: "Agent" }); | ||
|
|
||
| expect(result).toContain("Agent: Line 1"); | ||
| expect(result).toContain("Agent: Line 2"); | ||
| expect(result).toContain("Agent: Line 3"); | ||
| expect(result).toContain("◆ Line 1"); | ||
| expect(result).toContain(" Line 2"); | ||
| expect(result).toContain(" Line 3"); | ||
| }); | ||
|
|
||
| it("should truncate conversation when it exceeds max lines", async () => { | ||
|
|
@@ -1970,6 +1971,68 @@ describe("log_parser_shared.cjs", () => { | |
| }); | ||
| }); | ||
|
|
||
| describe("formatResultPreview", () => { | ||
| it("should return empty string for empty or falsy input", async () => { | ||
| const { formatResultPreview } = await import("./log_parser_shared.cjs"); | ||
|
|
||
| expect(formatResultPreview("")).toBe(""); | ||
| expect(formatResultPreview(null)).toBe(""); | ||
| expect(formatResultPreview(undefined)).toBe(""); | ||
| expect(formatResultPreview(" \n \n ")).toBe(""); | ||
| }); | ||
|
|
||
| it("should format single non-empty line with └", async () => { | ||
| const { formatResultPreview } = await import("./log_parser_shared.cjs"); | ||
|
|
||
| expect(formatResultPreview("hello")).toBe(" └ hello"); | ||
| expect(formatResultPreview("\nhello\n")).toBe(" └ hello"); | ||
| }); | ||
|
|
||
| it("should format exactly two non-empty lines with ├ and └", async () => { | ||
| const { formatResultPreview } = await import("./log_parser_shared.cjs"); | ||
|
|
||
| expect(formatResultPreview("line1\nline2")).toBe(" ├ line1\n └ line2"); | ||
| }); | ||
|
|
||
| it("should show (+ N more) for three or more non-empty lines", async () => { | ||
| const { formatResultPreview } = await import("./log_parser_shared.cjs"); | ||
|
|
||
| expect(formatResultPreview("line1\nline2\nline3")).toBe(" ├ line1\n └ line2 (+ 1 more)"); | ||
| expect(formatResultPreview("line1\nline2\nline3\nline4\nline5")).toBe(" ├ line1\n └ line2 (+ 3 more)"); | ||
| }); | ||
|
|
||
| it("should truncate lines exceeding maxLineLength and append ellipsis", async () => { | ||
| const { formatResultPreview } = await import("./log_parser_shared.cjs"); | ||
|
|
||
| const longLine = "a".repeat(100); | ||
| const result = formatResultPreview(longLine, 80); | ||
| expect(result).toBe(` └ ${"a".repeat(80)}...`); | ||
| }); | ||
|
|
||
| it("should not add ellipsis when line exactly fits maxLineLength", async () => { | ||
| const { formatResultPreview } = await import("./log_parser_shared.cjs"); | ||
|
|
||
| const exactLine = "a".repeat(80); | ||
| const result = formatResultPreview(exactLine, 80); | ||
| expect(result).toBe(` └ ${"a".repeat(80)}`); | ||
| expect(result).not.toContain("..."); | ||
| }); | ||
|
|
||
| it("should handle Windows CRLF line endings without trailing \\r", async () => { | ||
| const { formatResultPreview } = await import("./log_parser_shared.cjs"); | ||
|
|
||
| expect(formatResultPreview("line1\r\nline2\r\n")).toBe(" ├ line1\n └ line2"); | ||
| expect(formatResultPreview("only\r\n")).toBe(" └ only"); | ||
| }); | ||
|
|
||
| it("should skip blank lines when counting", async () => { | ||
| const { formatResultPreview } = await import("./log_parser_shared.cjs"); | ||
|
|
||
| expect(formatResultPreview("\n\nfirst\n\nsecond\n\n")).toBe(" ├ first\n └ second"); | ||
| expect(formatResultPreview("\n\nonly\n\n")).toBe(" └ only"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("formatSafeOutputsPreview", () => { | ||
| it("should return empty string for empty content", async () => { | ||
| const { formatSafeOutputsPreview } = await import("./log_parser_shared.cjs"); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice addition of
formatResultPreviewas a reusable helper! Extracting this into its own function makes it much cleaner than the duplicated inline logic that existed before. The tree-branch characters (├, └) give a nice visual hierarchy consistent with copilot-cli style.