Refactor log parser shared summaries into submodule to reduce architecture violations

[Copilot]

The architecture report flagged oversized JS modules/functions, with actions/setup/js/log_parser_shared.cjs called out as a high-priority warning. This PR reduces structural debt in that area by splitting large summary/formatting logic into a dedicated submodule while preserving existing behavior.

• Scope addressed (highest-priority warning cluster)

  • Split log_parser_shared.cjs summary/rendering logic into a new module:
    • actions/setup/js/log_parser_format.cjs
  • Reduced log_parser_shared.cjs from 1703 lines to 976 lines (below the warning threshold).

• Module decomposition

  • Moved large functions out of log_parser_shared.cjs:
    • generateConversationMarkdown
    • formatToolUse
    • generatePlainTextSummary
    • generateCopilotCliStyleSummary
  • Kept log_parser_shared.cjs as the public integration layer and export surface.

• Dependency-injected formatter design

  • Introduced createLogParserFormatters(deps) in the new module.
  • log_parser_shared.cjs now wires core utilities/constants into the formatter factory and re-exports the resulting functions.
  • This keeps cross-parser behavior centralized without introducing circular imports.

• Follow-up cleanup included

  • Removed duplicated tool-result pairing logic inside the new formatter module by reusing a shared helper.

// log_parser_shared.cjs
const createLogParserFormatters = require("./log_parser_format.cjs");

const {
  generateConversationMarkdown,
  formatToolUse,
  generatePlainTextSummary,
  generateCopilotCliStyleSummary,
} = createLogParserFormatters({
  formatBashCommand,
  formatMcpName,
  formatToolDisplayName,
  formatResultPreview,
  formatToolCallAsDetails,
  formatMcpParameters,
  truncateString,
  estimateTokens,
  formatDuration,
  unfenceMarkdown,
  MAX_AGENT_TEXT_LENGTH,
  SIZE_LIMIT_WARNING,
});

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Refactors oversized log parser summary/formatting logic out of log_parser_shared.cjs into a dedicated formatter submodule to reduce architecture violations while keeping behavior consistent.

Changes:

• Introduced actions/setup/js/log_parser_format.cjs with a dependency-injected formatter factory.
• Removed large formatting/summary functions from log_parser_shared.cjs and re-exported them via the new module.
• Minor whitespace-only cleanup in a Go CLI spec test file.

Show a summary per file

File	Description
pkg/cli/spec_test.go	Removes a trailing blank line (formatting-only).
actions/setup/js/log_parser_shared.cjs	Wires and re-exports formatter functions from the new module; deletes inlined implementations.
actions/setup/js/log_parser_format.cjs	New module containing extracted markdown/plain-text summary and tool-use formatting logic via injected deps.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 3/3 changed files
• Comments generated: 3

[Copilot]

This can throw at runtime because the guard if (usage.input_tokens || usage.output_tokens) allows cases where one of the fields is undefined (e.g., only output_tokens is present), but the string interpolation still calls .toLocaleString() on usage.input_tokens / usage.output_tokens. Use the already-defaulted inputTokens / outputTokens variables for display (or apply ?? 0 when formatting) so the summary never crashes on partial usage payloads.

Suggested change

	lines.push(` Tokens: ${totalTokens.toLocaleString()} total (${usage.input_tokens.toLocaleString()} in / ${usage.output_tokens.toLocaleString()} out)`);
	lines.push(` Tokens: ${totalTokens.toLocaleString()} total (${inputTokens.toLocaleString()} in / ${outputTokens.toLocaleString()} out)`);

[Copilot]

The line counting logic relies on pushing a multi-line string as a single array element and then manually adjusting conversationLineCount. This is fragile and easy to break if formatResultPreview ever changes its output shape (or if additional multi-line blocks are added). Consider splitting resultPreview by \\n and appending each line via appendConversationLine(...) so the limit/truncation behavior is enforced in one place without manual counter corrections.

Suggested change

	if (!appendConversationLine(lines, resultPreview, state)) {
	return;
	}
	state.conversationLineCount += resultPreview.split("\n").length - 1;
	if (state.conversationLineCount >= state.maxConversationLines) {
	state.conversationTruncated = true;
	return;
	}
	for (const previewLine of resultPreview.split("\n")) {
	if (!appendConversationLine(lines, previewLine, state)) {
	return;
	}
	}

[Copilot]

The public factory API is only documented as returning an Object, which makes it harder to discover/validate what's expected by callers. Please document the specific return shape (e.g., { generateConversationMarkdown, formatToolUse, generatePlainTextSummary, generateCopilotCliStyleSummary }) and, if possible, the key expected dependencies in deps (even as a minimal JSDoc typedef) to prevent silent drift between log_parser_shared.cjs and the formatter module.

Suggested change

	* Creates formatter functions for log parsing summaries and rendering.
	* Dependencies are injected to avoid module cycles with log_parser_shared.cjs.
	*
	* @param {Object} deps - Dependency injection container
	* @returns {Object} Formatter functions
	* Minimal dependency contract injected from log_parser_shared.cjs.
	* Keeping this explicit helps prevent silent drift between the shared parser
	* helpers and this formatter factory.
	*
	* @typedef {Object} LogParserFormatterDeps
	* @property {(command: string) => string} formatBashCommand
	* @property {(name: string) => string} formatMcpName
	* @property {(toolName: string) => string} formatToolDisplayName
	* @property {(result: any, maxLength?: number) => string} formatResultPreview
	* @property {(toolCall: any) => string} formatToolCallAsDetails
	* @property {(params: any) => string} formatMcpParameters
	* @property {(value: string, maxLength: number) => string} truncateString
	* @property {(value: string) => number} estimateTokens
	* @property {(durationMs: number) => string} formatDuration
	* @property {(markdown: string) => string} unfenceMarkdown
	* @property {number} MAX_AGENT_TEXT_LENGTH
	* @property {string} SIZE_LIMIT_WARNING
	*/
	/**
	* Public formatter API returned by createLogParserFormatters().
	*
	* @typedef {Object} LogParserFormatters
	* @property {(logEntries: Array<any>, options: {formatToolCallback: Function, formatInitCallback: Function, summaryTracker?: any}) => {markdown: string, commandSummary: Array<string>, sizeLimitReached: boolean}} generateConversationMarkdown
	* @property {(content: any, toolResult?: any) => string} formatToolUse
	* @property {(logEntries: Array<any>, options?: any) => string} generatePlainTextSummary
	* @property {(logEntries: Array<any>, options?: any) => string} generateCopilotCliStyleSummary
	*/
	/**
	* Creates formatter functions for log parsing summaries and rendering.
	* Dependencies are injected to avoid module cycles with log_parser_shared.cjs.
	*
	* @param {LogParserFormatterDeps} deps - Dependency injection container
	* @returns {LogParserFormatters} Formatter functions

[pelikhan]

@copilot review all comments

[Copilot]

@copilot review all comments

Completed a full pass on the review threads and pushed fixes in 8642897: token summary formatting now safely uses defaulted values, preview output now appends/counts lines via the shared limiter path, and JSDoc now documents the formatter dependency/return contracts explicitly.