diff --git a/packages/types/src/tool-params.ts b/packages/types/src/tool-params.ts index f8708b0c2b..8037f9a55e 100644 --- a/packages/types/src/tool-params.ts +++ b/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 +} diff --git a/src/core/task-persistence/taskMetadata.ts b/src/core/task-persistence/taskMetadata.ts index 92607595a9..ed6858a766 100644 --- a/src/core/task-persistence/taskMetadata.ts +++ b/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 diff --git a/src/shared/__tests__/messageUtils.spec.ts b/src/shared/__tests__/messageUtils.spec.ts new file mode 100644 index 0000000000..f1d16857eb --- /dev/null +++ b/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, + }) + }) + }) +}) diff --git a/src/shared/__tests__/typeGuards.spec.ts b/src/shared/__tests__/typeGuards.spec.ts new file mode 100644 index 0000000000..837e1d8df7 --- /dev/null +++ b/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) + }) + }) +}) diff --git a/src/shared/messageUtils.ts b/src/shared/messageUtils.ts new file mode 100644 index 0000000000..048248727b --- /dev/null +++ b/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 } +} diff --git a/src/shared/typeGuards.ts b/src/shared/typeGuards.ts new file mode 100644 index 0000000000..ffd14ecb41 --- /dev/null +++ b/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) +}