refactor: extract utility functions from taskMetadata into shared modules

Extract isFiniteNumber, isDiffStats, and getLineStatsFromToolApprovalMessages
from taskMetadata.ts into reusable shared utilities for better code organization
and reusability.

Changes:
- Add DiffStats interface to @roo-code/types for shared type definition
- Create src/shared/typeGuards.ts with isFiniteNumber and isDiffStats type guards
- Create src/shared/messageUtils.ts with getLineStatsFromToolApprovalMessages
- Update taskMetadata.ts to import from new shared utilities (removes 58 lines)
- Add comprehensive test coverage (21 tests total) for extracted functions

All existing tests continue to pass, ensuring no regression.

packages/types/src/tool-params.ts

@@ -36,3 +36,11 @@ export interface GenerateImageParams {
 	path: string
 	image?: string
 }
+
+/**
+ * Statistics about code changes from a diff operation
+ */
+export interface DiffStats {
+	added: number
+	removed: number
+}

src/core/task-persistence/taskMetadata.ts

@@ -8,68 +8,11 @@ import { combineCommandSequences } from "../../shared/combineCommandSequences"
 import { getApiMetrics } from "../../shared/getApiMetrics"
 import { findLastIndex } from "../../shared/array"
 import { getTaskDirectoryPath } from "../../utils/storage"
+import { getLineStatsFromToolApprovalMessages } from "../../shared/messageUtils"
 import { t } from "../../i18n"
 
 const taskSizeCache = new NodeCache({ stdTTL: 30, checkperiod: 5 * 60 })
 
-type DiffStats = { added: number; removed: number }
-
-function isFiniteNumber(value: unknown): value is number {
-	return typeof value === "number" && Number.isFinite(value)
-}
-
-function isDiffStats(value: unknown): value is DiffStats {
-	if (!value || typeof value !== "object") return false
-
-	const v = value as { added?: unknown; removed?: unknown }
-	return isFiniteNumber(v.added) && isFiniteNumber(v.removed)
-}
-
-function getLineStatsFromToolApprovalMessages(messages: ClineMessage[]): {
-	linesAdded: number
-	linesRemoved: number
-	foundAnyStats: boolean
-} {
-	let linesAdded = 0
-	let linesRemoved = 0
-	let foundAnyStats = false
-
-	for (const m of messages) {
-		// Only count complete tool approval asks (avoid double-counting partial/streaming updates)
-		if (!(m.type === "ask" && m.ask === "tool" && m.partial !== true)) continue
-		if (typeof m.text !== "string" || m.text.length === 0) continue
-
-		let payload: unknown
-		try {
-			payload = JSON.parse(m.text)
-		} catch {
-			continue
-		}
-
-		if (!payload || typeof payload !== "object") continue
-		const p = payload as { diffStats?: unknown; batchDiffs?: unknown }
-
-		if (isDiffStats(p.diffStats)) {
-			linesAdded += p.diffStats.added
-			linesRemoved += p.diffStats.removed
-			foundAnyStats = true
-		}
-
-		if (Array.isArray(p.batchDiffs)) {
-			for (const batchDiff of p.batchDiffs) {
-				if (!batchDiff || typeof batchDiff !== "object") continue
-				const bd = batchDiff as { diffStats?: unknown }
-				if (!isDiffStats(bd.diffStats)) continue
-				linesAdded += bd.diffStats.added
-				linesRemoved += bd.diffStats.removed
-				foundAnyStats = true
-			}
-		}
-	}
-
-	return { linesAdded, linesRemoved, foundAnyStats }
-}
-
 export type TaskMetadataOptions = {
 	taskId: string
 	rootTaskId?: string

src/shared/__tests__/messageUtils.spec.ts

@@ -0,0 +1,259 @@
+// npx vitest run src/shared/__tests__/messageUtils.spec.ts
+
+import type { ClineMessage } from "@roo-code/types"
+import { getLineStatsFromToolApprovalMessages } from "../messageUtils"
+
+describe("messageUtils", () => {
+	describe("getLineStatsFromToolApprovalMessages", () => {
+		it("should return zero stats for empty messages array", () => {
+			const result = getLineStatsFromToolApprovalMessages([])
+
+			expect(result).toEqual({
+				linesAdded: 0,
+				linesRemoved: 0,
+				foundAnyStats: false,
+			})
+		})
+
+		it("should ignore non-tool ask messages", () => {
+			const messages: ClineMessage[] = [
+				{ type: "say", say: "text", text: "hello", ts: 1000 },
+				{ type: "ask", ask: "followup", text: '{"diffStats":{"added":10,"removed":5}}', ts: 1001 },
+			]
+
+			const result = getLineStatsFromToolApprovalMessages(messages)
+
+			expect(result).toEqual({
+				linesAdded: 0,
+				linesRemoved: 0,
+				foundAnyStats: false,
+			})
+		})
+
+		it("should ignore partial tool ask messages", () => {
+			const messages: ClineMessage[] = [
+				{
+					type: "ask",
+					ask: "tool",
+					partial: true,
+					text: '{"diffStats":{"added":10,"removed":5}}',
+					ts: 1000,
+				},
+			]
+
+			const result = getLineStatsFromToolApprovalMessages(messages)
+
+			expect(result).toEqual({
+				linesAdded: 0,
+				linesRemoved: 0,
+				foundAnyStats: false,
+			})
+		})
+
+		it("should extract stats from a valid tool ask with diffStats", () => {
+			const messages: ClineMessage[] = [
+				{
+					type: "ask",
+					ask: "tool",
+					text: '{"diffStats":{"added":10,"removed":5}}',
+					ts: 1000,
+				},
+			]
+
+			const result = getLineStatsFromToolApprovalMessages(messages)
+
+			expect(result).toEqual({
+				linesAdded: 10,
+				linesRemoved: 5,
+				foundAnyStats: true,
+			})
+		})
+
+		it("should extract stats from batchDiffs array", () => {
+			const messages: ClineMessage[] = [
+				{
+					type: "ask",
+					ask: "tool",
+					text: JSON.stringify({
+						batchDiffs: [{ diffStats: { added: 5, removed: 3 } }, { diffStats: { added: 15, removed: 7 } }],
+					}),
+					ts: 1000,
+				},
+			]
+
+			const result = getLineStatsFromToolApprovalMessages(messages)
+
+			expect(result).toEqual({
+				linesAdded: 20,
+				linesRemoved: 10,
+				foundAnyStats: true,
+			})
+		})
+
+		it("should handle both diffStats and batchDiffs in the same message", () => {
+			const messages: ClineMessage[] = [
+				{
+					type: "ask",
+					ask: "tool",
+					text: JSON.stringify({
+						diffStats: { added: 10, removed: 5 },
+						batchDiffs: [{ diffStats: { added: 5, removed: 3 } }, { diffStats: { added: 15, removed: 7 } }],
+					}),
+					ts: 1000,
+				},
+			]
+
+			const result = getLineStatsFromToolApprovalMessages(messages)
+
+			expect(result).toEqual({
+				linesAdded: 30,
+				linesRemoved: 15,
+				foundAnyStats: true,
+			})
+		})
+
+		it("should accumulate stats from multiple messages", () => {
+			const messages: ClineMessage[] = [
+				{
+					type: "ask",
+					ask: "tool",
+					text: '{"diffStats":{"added":10,"removed":5}}',
+					ts: 1000,
+				},
+				{
+					type: "ask",
+					ask: "tool",
+					text: '{"diffStats":{"added":20,"removed":15}}',
+					ts: 1001,
+				},
+			]
+
+			const result = getLineStatsFromToolApprovalMessages(messages)
+
+			expect(result).toEqual({
+				linesAdded: 30,
+				linesRemoved: 20,
+				foundAnyStats: true,
+			})
+		})
+
+		it("should ignore messages with invalid JSON", () => {
+			const messages: ClineMessage[] = [
+				{
+					type: "ask",
+					ask: "tool",
+					text: "not valid json",
+					ts: 1000,
+				},
+				{
+					type: "ask",
+					ask: "tool",
+					text: '{"diffStats":{"added":10,"removed":5}}',
+					ts: 1001,
+				},
+			]
+
+			const result = getLineStatsFromToolApprovalMessages(messages)
+
+			expect(result).toEqual({
+				linesAdded: 10,
+				linesRemoved: 5,
+				foundAnyStats: true,
+			})
+		})
+
+		it("should ignore messages with empty or missing text", () => {
+			const messages: ClineMessage[] = [
+				{
+					type: "ask",
+					ask: "tool",
+					text: "",
+					ts: 1000,
+				},
+				{
+					type: "ask",
+					ask: "tool",
+					ts: 1001,
+				},
+			]
+
+			const result = getLineStatsFromToolApprovalMessages(messages)
+
+			expect(result).toEqual({
+				linesAdded: 0,
+				linesRemoved: 0,
+				foundAnyStats: false,
+			})
+		})
+
+		it("should ignore invalid diffStats (non-finite numbers)", () => {
+			const messages: ClineMessage[] = [
+				{
+					type: "ask",
+					ask: "tool",
+					text: '{"diffStats":{"added":"10","removed":5}}',
+					ts: 1000,
+				},
+				{
+					type: "ask",
+					ask: "tool",
+					text: '{"diffStats":{"added":10,"removed":null}}',
+					ts: 1001,
+				},
+			]
+
+			const result = getLineStatsFromToolApprovalMessages(messages)
+
+			expect(result).toEqual({
+				linesAdded: 0,
+				linesRemoved: 0,
+				foundAnyStats: false,
+			})
+		})
+
+		it("should skip invalid items in batchDiffs array", () => {
+			const messages: ClineMessage[] = [
+				{
+					type: "ask",
+					ask: "tool",
+					text: JSON.stringify({
+						batchDiffs: [
+							{ diffStats: { added: 5, removed: 3 } },
+							null,
+							{ diffStats: { added: "invalid", removed: 7 } },
+							{ diffStats: { added: 15, removed: 10 } },
+						],
+					}),
+					ts: 1000,
+				},
+			]
+
+			const result = getLineStatsFromToolApprovalMessages(messages)
+
+			expect(result).toEqual({
+				linesAdded: 20,
+				linesRemoved: 13,
+				foundAnyStats: true,
+			})
+		})
+
+		it("should handle messages with no diffStats field", () => {
+			const messages: ClineMessage[] = [
+				{
+					type: "ask",
+					ask: "tool",
+					text: '{"someOtherField":"value"}',
+					ts: 1000,
+				},
+			]
+
+			const result = getLineStatsFromToolApprovalMessages(messages)
+
+			expect(result).toEqual({
+				linesAdded: 0,
+				linesRemoved: 0,
+				foundAnyStats: false,
+			})
+		})
+	})
+})

src/shared/__tests__/typeGuards.spec.ts

@@ -0,0 +1,70 @@
+// npx vitest run src/shared/__tests__/typeGuards.spec.ts
+
+import { isFiniteNumber, isDiffStats } from "../typeGuards"
+
+describe("typeGuards", () => {
+	describe("isFiniteNumber", () => {
+		it("should return true for finite numbers", () => {
+			expect(isFiniteNumber(0)).toBe(true)
+			expect(isFiniteNumber(42)).toBe(true)
+			expect(isFiniteNumber(-10)).toBe(true)
+			expect(isFiniteNumber(3.14)).toBe(true)
+			expect(isFiniteNumber(-0.5)).toBe(true)
+		})
+
+		it("should return false for non-finite numbers", () => {
+			expect(isFiniteNumber(Infinity)).toBe(false)
+			expect(isFiniteNumber(-Infinity)).toBe(false)
+			expect(isFiniteNumber(NaN)).toBe(false)
+		})
+
+		it("should return false for non-number types", () => {
+			expect(isFiniteNumber("42")).toBe(false)
+			expect(isFiniteNumber(null)).toBe(false)
+			expect(isFiniteNumber(undefined)).toBe(false)
+			expect(isFiniteNumber(true)).toBe(false)
+			expect(isFiniteNumber({})).toBe(false)
+			expect(isFiniteNumber([])).toBe(false)
+		})
+	})
+
+	describe("isDiffStats", () => {
+		it("should return true for valid DiffStats objects", () => {
+			expect(isDiffStats({ added: 0, removed: 0 })).toBe(true)
+			expect(isDiffStats({ added: 10, removed: 5 })).toBe(true)
+			expect(isDiffStats({ added: 100, removed: 200 })).toBe(true)
+		})
+
+		it("should return false for objects with non-finite numbers", () => {
+			expect(isDiffStats({ added: Infinity, removed: 5 })).toBe(false)
+			expect(isDiffStats({ added: 10, removed: NaN })).toBe(false)
+			expect(isDiffStats({ added: NaN, removed: Infinity })).toBe(false)
+		})
+
+		it("should return false for objects with non-number properties", () => {
+			expect(isDiffStats({ added: "10", removed: 5 })).toBe(false)
+			expect(isDiffStats({ added: 10, removed: "5" })).toBe(false)
+			expect(isDiffStats({ added: null, removed: 5 })).toBe(false)
+			expect(isDiffStats({ added: 10, removed: undefined })).toBe(false)
+		})
+
+		it("should return false for objects missing required properties", () => {
+			expect(isDiffStats({ added: 10 })).toBe(false)
+			expect(isDiffStats({ removed: 5 })).toBe(false)
+			expect(isDiffStats({})).toBe(false)
+		})
+
+		it("should return false for non-object types", () => {
+			expect(isDiffStats(null)).toBe(false)
+			expect(isDiffStats(undefined)).toBe(false)
+			expect(isDiffStats("string")).toBe(false)
+			expect(isDiffStats(42)).toBe(false)
+			expect(isDiffStats([])).toBe(false)
+			expect(isDiffStats(true)).toBe(false)
+		})
+
+		it("should ignore extra properties on valid objects", () => {
+			expect(isDiffStats({ added: 10, removed: 5, extra: "value" })).toBe(true)
+		})
+	})
+})

src/shared/messageUtils.ts

@@ -0,0 +1,55 @@
+import type { ClineMessage } from "@roo-code/types"
+import { isDiffStats } from "./typeGuards"
+
+/**
+ * Extract line statistics (added/removed) from tool approval messages in the message history.
+ * This function scans messages for diff statistics from completed tool approval requests,
+ * including both single file operations and batch operations.
+ *
+ * @param messages - Array of ClineMessage objects to analyze
+ * @returns Object containing total lines added, removed, and whether any stats were found
+ */
+export function getLineStatsFromToolApprovalMessages(messages: ClineMessage[]): {
+	linesAdded: number
+	linesRemoved: number
+	foundAnyStats: boolean
+} {
+	let linesAdded = 0
+	let linesRemoved = 0
+	let foundAnyStats = false
+
+	for (const m of messages) {
+		// Only count complete tool approval asks (avoid double-counting partial/streaming updates)
+		if (!(m.type === "ask" && m.ask === "tool" && m.partial !== true)) continue
+		if (typeof m.text !== "string" || m.text.length === 0) continue
+
+		let payload: unknown
+		try {
+			payload = JSON.parse(m.text)
+		} catch {
+			continue
+		}
+
+		if (!payload || typeof payload !== "object") continue
+		const p = payload as { diffStats?: unknown; batchDiffs?: unknown }
+
+		if (isDiffStats(p.diffStats)) {
+			linesAdded += p.diffStats.added
+			linesRemoved += p.diffStats.removed
+			foundAnyStats = true
+		}
+
+		if (Array.isArray(p.batchDiffs)) {
+			for (const batchDiff of p.batchDiffs) {
+				if (!batchDiff || typeof batchDiff !== "object") continue
+				const bd = batchDiff as { diffStats?: unknown }
+				if (!isDiffStats(bd.diffStats)) continue
+				linesAdded += bd.diffStats.added
+				linesRemoved += bd.diffStats.removed
+				foundAnyStats = true
+			}
+		}
+	}
+
+	return { linesAdded, linesRemoved, foundAnyStats }
+}

src/shared/typeGuards.ts

@@ -0,0 +1,22 @@
+import type { DiffStats } from "@roo-code/types"
+
+/**
+ * Type guard to check if a value is a finite number
+ * @param value - The value to check
+ * @returns true if the value is a number and is finite
+ */
+export function isFiniteNumber(value: unknown): value is number {
+	return typeof value === "number" && Number.isFinite(value)
+}
+
+/**
+ * Type guard to check if a value conforms to the DiffStats interface
+ * @param value - The value to check
+ * @returns true if the value has valid `added` and `removed` properties that are finite numbers
+ */
+export function isDiffStats(value: unknown): value is DiffStats {
+	if (!value || typeof value !== "object") return false
+
+	const v = value as { added?: unknown; removed?: unknown }
+	return isFiniteNumber(v.added) && isFiniteNumber(v.removed)
+}