From 5750f0f3401d52677705a35e42856b45b4979152 Mon Sep 17 00:00:00 2001 From: Toray Altas Date: Wed, 21 Jan 2026 09:35:40 -0500 Subject: [PATCH] refactor: remove line change tracking from subtask system Remove the line change tracking feature (+added/-removed) that was displaying git diff statistics for delegated subtasks. This feature added complexity across the entire stack without sufficient value. Changes: - Remove DiffStats type and line change fields from schemas - Remove extraction, aggregation, and persistence logic - Remove UI displays in TaskHeader and TodoListDisplay - Remove all associated tests and utilities - Preserve all cost and token tracking functionality Impact: - 20 files modified - 1,154 net lines removed - All tests passing - TypeScript compilation successful Original feature from PR #10765 --- packages/types/src/history.ts | 2 - packages/types/src/todo.ts | 2 - packages/types/src/tool-params.ts | 8 - packages/types/src/vscode-extension-host.ts | 8 - .../__tests__/taskMetadata.spec.ts | 128 +-------- src/core/task-persistence/taskMetadata.ts | 11 - src/core/tools/UpdateTodoListTool.ts | 19 +- .../__tests__/updateTodoListTool.spec.ts | 152 +--------- src/core/webview/ClineProvider.ts | 2 - ...openParentFromDelegation.writeback.spec.ts | 6 - .../__tests__/aggregateTaskCosts.spec.ts | 96 ------- src/core/webview/aggregateTaskCosts.ts | 34 --- src/shared/__tests__/messageUtils.spec.ts | 262 +----------------- src/shared/__tests__/typeGuards.spec.ts | 42 +-- src/shared/messageUtils.ts | 57 +--- src/shared/typeGuards.ts | 14 - webview-ui/src/components/chat/TaskHeader.tsx | 80 ------ .../src/components/chat/TodoListDisplay.tsx | 66 +---- .../chat/__tests__/TodoListDisplay.spec.tsx | 205 -------------- webview-ui/src/types/subtasks.ts | 4 - 20 files changed, 22 insertions(+), 1176 deletions(-) diff --git a/packages/types/src/history.ts b/packages/types/src/history.ts index 06e246e0dc..b4d84cb9a5 100644 --- a/packages/types/src/history.ts +++ b/packages/types/src/history.ts @@ -16,8 +16,6 @@ export const historyItemSchema = z.object({ cacheWrites: z.number().optional(), cacheReads: z.number().optional(), totalCost: z.number(), - linesAdded: z.number().optional(), - linesRemoved: z.number().optional(), size: z.number().optional(), workspace: z.string().optional(), mode: z.string().optional(), diff --git a/packages/types/src/todo.ts b/packages/types/src/todo.ts index 328beeb359..0530f92054 100644 --- a/packages/types/src/todo.ts +++ b/packages/types/src/todo.ts @@ -18,8 +18,6 @@ export const todoItemSchema = z.object({ subtaskId: z.string().optional(), // ID of the linked subtask (child task) for direct cost/token attribution tokens: z.number().optional(), // Total tokens (in + out) for linked subtask cost: z.number().optional(), // Total cost for linked subtask - added: z.number().optional(), - removed: z.number().optional(), }) export type TodoItem = z.infer diff --git a/packages/types/src/tool-params.ts b/packages/types/src/tool-params.ts index 8037f9a55e..f8708b0c2b 100644 --- a/packages/types/src/tool-params.ts +++ b/packages/types/src/tool-params.ts @@ -36,11 +36,3 @@ 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/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index 49294f08c6..1448c08c80 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -188,19 +188,11 @@ export interface ExtensionMessage { totalCost: number ownCost: number childrenCost: number - ownAdded?: number - ownRemoved?: number - childrenAdded?: number - childrenRemoved?: number - totalAdded?: number - totalRemoved?: number childDetails?: { id: string name: string tokens: number cost: number - added: number - removed: number status: "active" | "completed" | "delegated" hasNestedChildren: boolean }[] diff --git a/src/core/task-persistence/__tests__/taskMetadata.spec.ts b/src/core/task-persistence/__tests__/taskMetadata.spec.ts index 52f8ec2a0d..eefc405431 100644 --- a/src/core/task-persistence/__tests__/taskMetadata.spec.ts +++ b/src/core/task-persistence/__tests__/taskMetadata.spec.ts @@ -1,125 +1,3 @@ -import { describe, it, expect, vi, beforeEach } from "vitest" - -// Hoisted mocks to avoid initialization ordering issues -const hoisted = vi.hoisted(() => ({ - getTaskDirectoryPathMock: vi.fn().mockResolvedValue("/mock/task/dir"), - getFolderSizeLooseMock: vi.fn().mockResolvedValue(0), - getApiMetricsMock: vi.fn().mockReturnValue({ - totalTokensIn: 0, - totalTokensOut: 0, - totalCacheWrites: 0, - totalCacheReads: 0, - totalCost: 0, - contextTokens: 0, - }), -})) - -vi.mock("get-folder-size", () => ({ - default: { - loose: hoisted.getFolderSizeLooseMock, - }, -})) - -vi.mock("../../../utils/storage", () => ({ - getTaskDirectoryPath: hoisted.getTaskDirectoryPathMock, -})) - -vi.mock("../../../shared/getApiMetrics", () => ({ - getApiMetrics: hoisted.getApiMetricsMock, -})) - -// Import after mocks -import { taskMetadata } from "../taskMetadata" - -describe("taskMetadata() line change parsing", () => { - beforeEach(() => { - hoisted.getTaskDirectoryPathMock.mockClear() - hoisted.getFolderSizeLooseMock.mockClear() - hoisted.getApiMetricsMock.mockClear() - }) - - it("computes linesAdded/linesRemoved from tool message diffStats", async () => { - const result = await taskMetadata({ - taskId: "task-1", - taskNumber: 1, - globalStoragePath: "/mock/global", - workspace: "/mock/workspace", - messages: [ - { ts: 1, type: "say", say: "text", text: "Task" } as any, - { - ts: 2, - type: "ask", - ask: "tool", - text: JSON.stringify({ diffStats: { added: 5, removed: 2 } }), - } as any, - ], - }) - - expect(result.historyItem.linesAdded).toBe(5) - expect(result.historyItem.linesRemoved).toBe(2) - }) - - it("aggregates linesAdded/linesRemoved from batch tool message batchDiffs[].diffStats", async () => { - const result = await taskMetadata({ - taskId: "task-2", - taskNumber: 2, - globalStoragePath: "/mock/global", - workspace: "/mock/workspace", - messages: [ - { ts: 1, type: "say", say: "text", text: "Task" } as any, - { - ts: 2, - type: "ask", - ask: "tool", - text: JSON.stringify({ - batchDiffs: [ - { path: "a.ts", diffStats: { added: 1, removed: 1 } }, - { path: "b.ts", diffStats: { added: 2, removed: 3 } }, - ], - }), - } as any, - ], - }) - - expect(result.historyItem.linesAdded).toBe(3) - expect(result.historyItem.linesRemoved).toBe(4) - }) - - it("ignores partial tool messages", async () => { - const result = await taskMetadata({ - taskId: "task-3", - taskNumber: 3, - globalStoragePath: "/mock/global", - workspace: "/mock/workspace", - messages: [ - { ts: 1, type: "say", say: "text", text: "Task" } as any, - { - ts: 2, - type: "ask", - ask: "tool", - partial: true, - text: JSON.stringify({ diffStats: { added: 10, removed: 10 } }), - } as any, - ], - }) - - expect(result.historyItem.linesAdded).toBeUndefined() - expect(result.historyItem.linesRemoved).toBeUndefined() - }) - - it("ignores invalid JSON in tool message text gracefully", async () => { - const result = await taskMetadata({ - taskId: "task-4", - taskNumber: 4, - globalStoragePath: "/mock/global", - workspace: "/mock/workspace", - messages: [ - { ts: 1, type: "say", say: "text", text: "Task" } as any, - { ts: 2, type: "ask", ask: "tool", text: "{not-json" } as any, - ], - }) - - expect(result.historyItem.linesAdded).toBeUndefined() - expect(result.historyItem.linesRemoved).toBeUndefined() - }) -}) +// This file previously tested line change tracking in taskMetadata, +// which has been removed as part of eliminating line change tracking. +// The file is kept as a placeholder for future taskMetadata tests. diff --git a/src/core/task-persistence/taskMetadata.ts b/src/core/task-persistence/taskMetadata.ts index ed6858a766..cf8d9adb52 100644 --- a/src/core/task-persistence/taskMetadata.ts +++ b/src/core/task-persistence/taskMetadata.ts @@ -8,7 +8,6 @@ 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 }) @@ -56,8 +55,6 @@ export async function taskMetadata({ let tokenUsage: ReturnType let taskDirSize: number let taskMessage: ClineMessage | undefined - let linesAdded: number | undefined - let linesRemoved: number | undefined if (!hasMessages) { // Handle no messages case @@ -96,12 +93,6 @@ export async function taskMetadata({ } else { taskDirSize = cachedSize } - - const lineStats = getLineStatsFromToolApprovalMessages(messages) - if (lineStats.foundAnyStats) { - linesAdded = lineStats.linesAdded - linesRemoved = lineStats.linesRemoved - } } // Create historyItem once with pre-calculated values. @@ -124,8 +115,6 @@ export async function taskMetadata({ cacheWrites: tokenUsage.totalCacheWrites, cacheReads: tokenUsage.totalCacheReads, totalCost: tokenUsage.totalCost, - ...(typeof linesAdded === "number" ? { linesAdded } : {}), - ...(typeof linesRemoved === "number" ? { linesRemoved } : {}), size: taskDirSize, workspace, mode, diff --git a/src/core/tools/UpdateTodoListTool.ts b/src/core/tools/UpdateTodoListTool.ts index c87cc28b0b..5e3a47afc9 100644 --- a/src/core/tools/UpdateTodoListTool.ts +++ b/src/core/tools/UpdateTodoListTool.ts @@ -32,12 +32,7 @@ export class UpdateTodoListTool extends BaseTool<"update_todo_list"> { const historyHasMetadata = Array.isArray(previousFromHistory) && previousFromHistory.some( - (t) => - t?.subtaskId !== undefined || - t?.tokens !== undefined || - t?.cost !== undefined || - t?.added !== undefined || - t?.removed !== undefined, + (t) => t?.subtaskId !== undefined || t?.tokens !== undefined || t?.cost !== undefined, ) const previousTodos: TodoItem[] = @@ -86,8 +81,6 @@ export class UpdateTodoListTool extends BaseTool<"update_todo_list"> { subtaskId: t.subtaskId, tokens: t.tokens, cost: t.cost, - added: t.added, - removed: t.removed, })) const approvalMsg = JSON.stringify({ @@ -228,7 +221,7 @@ function normalizeStatus(status: string | undefined): TodoStatus { } /** - * Preserve metadata (subtaskId, tokens, cost, added, removed) from previous todos onto next todos. + * Preserve metadata (subtaskId, tokens, cost) from previous todos onto next todos. * * Matching strategy (in priority order): * 1. **Subtask ID match**: If the next todo has a `subtaskId`, match against previous todos with @@ -347,8 +340,6 @@ function preserveTodoMetadata(nextTodos: TodoItem[], previousTodos: TodoItem[]): subtaskId: next.subtaskId ?? matchedPrev.subtaskId, tokens: next.tokens ?? matchedPrev.tokens, cost: next.cost ?? matchedPrev.cost, - added: next.added ?? matchedPrev.added, - removed: next.removed ?? matchedPrev.removed, } continue } @@ -383,8 +374,6 @@ function preserveTodoMetadata(nextTodos: TodoItem[], previousTodos: TodoItem[]): subtaskId: next.subtaskId ?? prev.subtaskId, tokens: next.tokens ?? prev.tokens, cost: next.cost ?? prev.cost, - added: next.added ?? prev.added, - removed: next.removed ?? prev.removed, } usedPreviousIndices.add(i) } @@ -417,8 +406,6 @@ function preserveTodoMetadata(nextTodos: TodoItem[], previousTodos: TodoItem[]): subtaskId: targetNext.subtaskId ?? orphanedPrev.subtaskId, tokens: targetNext.tokens ?? orphanedPrev.tokens, cost: targetNext.cost ?? orphanedPrev.cost, - added: targetNext.added ?? orphanedPrev.added, - removed: targetNext.removed ?? orphanedPrev.removed, } result[targetNextIndex] = updatedTarget @@ -518,8 +505,6 @@ function tryParseTodoItemsJson(raw: string): { parsed?: TodoItem[]; error?: stri ...(typeof t.subtaskId === "string" ? { subtaskId: t.subtaskId } : {}), ...(typeof t.tokens === "number" ? { tokens: t.tokens } : {}), ...(typeof t.cost === "number" ? { cost: t.cost } : {}), - ...(typeof t.added === "number" ? { added: t.added } : {}), - ...(typeof t.removed === "number" ? { removed: t.removed } : {}), } normalized.push(normalizedItem) diff --git a/src/core/tools/__tests__/updateTodoListTool.spec.ts b/src/core/tools/__tests__/updateTodoListTool.spec.ts index af96c352f7..766030bc02 100644 --- a/src/core/tools/__tests__/updateTodoListTool.spec.ts +++ b/src/core/tools/__tests__/updateTodoListTool.spec.ts @@ -263,7 +263,7 @@ describe("UpdateTodoListTool.execute", () => { setPendingTodoList([]) }) - it("should preserve per-row metadata (subtaskId/tokens/cost/added/removed) when only statuses change (bulk markdown rewrite)", async () => { + it("should preserve per-row metadata (subtaskId/tokens/cost) when only statuses change (bulk markdown rewrite)", async () => { /** * Regression test: a bulk markdown rewrite often changes the derived todo `id` * (since [`parseMarkdownChecklist()`](../UpdateTodoListTool.ts:337) hashes @@ -280,8 +280,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: `subtask-${idx + 1}`, tokens: 1000 + idx, cost: 0.01 * (idx + 1), - added: 10 * (idx + 1), - removed: idx, })) const task = { @@ -312,8 +310,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: "subtask-1", tokens: 1000, cost: 0.01, - added: 10, - removed: 0, }), ) @@ -324,8 +320,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: "subtask-2", tokens: 1001, cost: 0.02, - added: 20, - removed: 1, }), ) @@ -336,8 +330,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: "subtask-3", tokens: 1002, cost: 0.03, - added: 30, - removed: 2, }), ) @@ -348,8 +340,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: "subtask-4", tokens: 1003, cost: 0.04, - added: 40, - removed: 3, }), ) }) @@ -363,8 +353,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: `subtask-${idx + 1}`, tokens: 100 + idx, cost: 0.01 * (idx + 1), - added: 10 * (idx + 1), - removed: idx, })) const task = { @@ -394,8 +382,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: "subtask-1", tokens: 100, cost: 0.01, - added: 10, - removed: 0, }), ) expect(task.todoList[1]).toEqual( @@ -405,8 +391,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: "subtask-2", tokens: 101, cost: 0.02, - added: 20, - removed: 1, }), ) expect(task.todoList[2]).toEqual( @@ -416,8 +400,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: "subtask-3", tokens: 102, cost: 0.03, - added: 30, - removed: 2, }), ) }) @@ -431,8 +413,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: "subtask-1", tokens: 111, cost: 0.11, - added: 11, - removed: 1, }, { id: "id-2", @@ -441,8 +421,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: "subtask-2", tokens: 222, cost: 0.22, - added: 22, - removed: 2, }, ] @@ -482,8 +460,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: "subtask-2", tokens: 222, cost: 0.22, - added: 22, - removed: 2, }), ) @@ -495,8 +471,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: "subtask-1", tokens: 111, cost: 0.11, - added: 11, - removed: 1, }), ) }) @@ -547,50 +521,6 @@ describe("UpdateTodoListTool.execute", () => { ) }) - it("should treat added/removed as metadata and prefer history todos when present", async () => { - const md = "[ ] Task 1" - - const previousFromMemory = parseMarkdownChecklist(md) - const previousFromHistory: TodoItem[] = previousFromMemory.map((t) => ({ - ...t, - added: 10, - removed: 3, - })) - - const task = { - todoList: previousFromMemory, - clineMessages: [ - { - type: "ask", - ask: "tool", - text: JSON.stringify({ tool: "updateTodoList", todos: previousFromHistory }), - }, - ], - consecutiveMistakeCount: 0, - recordToolError: vi.fn(), - didToolFailInCurrentTurn: false, - say: vi.fn(), - } as any - - const tool = new UpdateTodoListTool() - await tool.execute({ todos: md }, task, { - pushToolResult: vi.fn(), - handleError: vi.fn(), - askApproval: vi.fn().mockResolvedValue(true), - removeClosingTag: vi.fn(), - toolProtocol: "xml", - }) - - expect(task.todoList).toHaveLength(1) - expect(task.todoList[0]).toEqual( - expect.objectContaining({ - content: "Task 1", - added: 10, - removed: 3, - }), - ) - }) - it("should preserve metadata by subtaskId even when content (and derived id) changes", async () => { // This test simulates the "user edited todo list" flow. The tool re-applies metadata // after approval; subtaskId should be used as the primary match when content/id changes. @@ -601,8 +531,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: "subtask-1", tokens: 123, cost: 0.01, - added: 10, - removed: 3, })) const task = { @@ -621,7 +549,7 @@ describe("UpdateTodoListTool.execute", () => { content: "New text", status: "completed", subtaskId: "subtask-1", - // tokens/cost/added/removed intentionally omitted to verify preservation + // tokens/cost intentionally omitted to verify preservation }, ] @@ -646,46 +574,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: "subtask-1", tokens: 123, cost: 0.01, - added: 10, - removed: 3, - }), - ) - }) - - it("should preserve added/removed through normalization", async () => { - const md = "[x] Task 1" - - const previousFromMemory: TodoItem[] = parseMarkdownChecklist("[ ] Task 1").map((t) => ({ - ...t, - added: 10, - removed: 3, - })) - - const task = { - todoList: previousFromMemory, - clineMessages: [], - consecutiveMistakeCount: 0, - recordToolError: vi.fn(), - didToolFailInCurrentTurn: false, - say: vi.fn(), - } as any - - const tool = new UpdateTodoListTool() - await tool.execute({ todos: md }, task, { - pushToolResult: vi.fn(), - handleError: vi.fn(), - askApproval: vi.fn().mockResolvedValue(true), - removeClosingTag: vi.fn(), - toolProtocol: "xml", - }) - - expect(task.todoList).toHaveLength(1) - expect(task.todoList[0]).toEqual( - expect.objectContaining({ - content: "Task 1", - status: "completed", - added: 10, - removed: 3, }), ) }) @@ -700,8 +588,6 @@ describe("UpdateTodoListTool.execute", () => { status: "pending", tokens: 111, cost: 0.11, - added: 11, - removed: 1, }, { id: "legacy-2", @@ -709,8 +595,6 @@ describe("UpdateTodoListTool.execute", () => { status: "pending", tokens: 222, cost: 0.22, - added: 22, - removed: 2, }, ] @@ -743,8 +627,6 @@ describe("UpdateTodoListTool.execute", () => { status: "completed", tokens: 111, cost: 0.11, - added: 11, - removed: 1, }), ) expect(task2).toEqual( @@ -753,8 +635,6 @@ describe("UpdateTodoListTool.execute", () => { status: "pending", tokens: 222, cost: 0.22, - added: 22, - removed: 2, }), ) }) @@ -769,8 +649,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: delegatedSubtaskId, tokens: 1234, cost: 0.12, - added: 10, - removed: 2, }, ] @@ -804,8 +682,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: delegatedSubtaskId, tokens: 1234, cost: 0.12, - added: 10, - removed: 2, }), ) @@ -819,8 +695,6 @@ describe("UpdateTodoListTool.execute", () => { expect(task.todoList[1].subtaskId).toBeUndefined() expect(task.todoList[1].tokens).toBeUndefined() expect(task.todoList[1].cost).toBeUndefined() - expect(task.todoList[1].added).toBeUndefined() - expect(task.todoList[1].removed).toBeUndefined() }) it("should carry forward metadata from unmatched delegated todos even when the previous id is non-synthetic (sequential updates)", async () => { @@ -833,8 +707,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: delegatedSubtaskId, tokens: 1234, cost: 0.12, - added: 10, - removed: 2, }, { id: "other-1", @@ -871,8 +743,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: delegatedSubtaskId, tokens: 1234, cost: 0.12, - added: 10, - removed: 2, }), ) // Ensure the carried-over todo now has a non-synthetic ID (this is the regression scenario). @@ -897,8 +767,6 @@ describe("UpdateTodoListTool.execute", () => { subtaskId: delegatedSubtaskId, tokens: 1234, cost: 0.12, - added: 10, - removed: 2, }), ) @@ -912,9 +780,7 @@ describe("UpdateTodoListTool.execute", () => { const md = "[x] Task 1\n[ ] Task 2" // status changes for Task 1 -> derived id changes const previousFromMemory: TodoItem[] = parseMarkdownChecklist(initialMd).map((t) => - t.content === "Task 1" - ? { ...t, tokens: 111, cost: 0.11, added: 11, removed: 1 } - : { ...t, tokens: 222, cost: 0.22, added: 22, removed: 2 }, + t.content === "Task 1" ? { ...t, tokens: 111, cost: 0.11 } : { ...t, tokens: 222, cost: 0.22 }, ) const task = { @@ -946,8 +812,6 @@ describe("UpdateTodoListTool.execute", () => { status: "completed", tokens: 111, cost: 0.11, - added: 11, - removed: 1, }), ) @@ -957,8 +821,6 @@ describe("UpdateTodoListTool.execute", () => { status: "pending", tokens: 222, cost: 0.22, - added: 22, - removed: 2, }), ) }) @@ -968,9 +830,7 @@ describe("UpdateTodoListTool.execute", () => { const md = "[x] Task 1 (updated)\n[ ] Task 2" const previousFromMemory: TodoItem[] = parseMarkdownChecklist(initialMd).map((t) => - t.content === "Task 1" - ? { ...t, tokens: 111, cost: 0.11, added: 11, removed: 1 } - : { ...t, tokens: 222, cost: 0.22, added: 22, removed: 2 }, + t.content === "Task 1" ? { ...t, tokens: 111, cost: 0.11 } : { ...t, tokens: 222, cost: 0.22 }, ) const task = { @@ -1004,8 +864,6 @@ describe("UpdateTodoListTool.execute", () => { ) expect(updated?.tokens).toBeUndefined() expect(updated?.cost).toBeUndefined() - expect(updated?.added).toBeUndefined() - expect(updated?.removed).toBeUndefined() expect(task2).toEqual( expect.objectContaining({ @@ -1013,8 +871,6 @@ describe("UpdateTodoListTool.execute", () => { status: "pending", tokens: 222, cost: 0.22, - added: 22, - removed: 2, }), ) }) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 15f321451a..0150b3f061 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -3440,8 +3440,6 @@ export class ClineProvider linkedTodo.tokens = (childHistoryItem?.tokensIn || 0) + (childHistoryItem?.tokensOut || 0) linkedTodo.cost = childHistoryItem?.totalCost || 0 - linkedTodo.added = childHistoryItem?.linesAdded || 0 - linkedTodo.removed = childHistoryItem?.linesRemoved || 0 this.log( `[TODO-DEBUG] reopenParentFromDelegation persisting system_update_todos ${JSON.stringify({ diff --git a/src/core/webview/__tests__/ClineProvider.reopenParentFromDelegation.writeback.spec.ts b/src/core/webview/__tests__/ClineProvider.reopenParentFromDelegation.writeback.spec.ts index f7d00d435b..7b64ef73f3 100644 --- a/src/core/webview/__tests__/ClineProvider.reopenParentFromDelegation.writeback.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.reopenParentFromDelegation.writeback.spec.ts @@ -148,8 +148,6 @@ describe("ClineProvider.reopenParentFromDelegation() writeback", () => { tokensIn: 10, tokensOut: 5, totalCost: 0.123, - linesAdded: 7, - linesRemoved: 2, } as unknown as HistoryItem, } @@ -212,8 +210,6 @@ describe("ClineProvider.reopenParentFromDelegation() writeback", () => { subtaskId: child2TaskId, tokens: 15, cost: 0.123, - added: 7, - removed: 2, }), ) @@ -228,8 +224,6 @@ describe("ClineProvider.reopenParentFromDelegation() writeback", () => { ) expect(updatedChild1Row?.tokens).toBeUndefined() expect(updatedChild1Row?.cost).toBeUndefined() - expect(updatedChild1Row?.added).toBeUndefined() - expect(updatedChild1Row?.removed).toBeUndefined() expect(updatedPrepRow).toEqual(expect.objectContaining({ id: "t1b", content: "Prep", status: "completed" })) expect((updatedPrepRow as any)?.subtaskId).toBeUndefined() diff --git a/src/core/webview/__tests__/aggregateTaskCosts.spec.ts b/src/core/webview/__tests__/aggregateTaskCosts.spec.ts index c11741af15..2b2d297f7b 100644 --- a/src/core/webview/__tests__/aggregateTaskCosts.spec.ts +++ b/src/core/webview/__tests__/aggregateTaskCosts.spec.ts @@ -53,15 +53,11 @@ describe("aggregateTaskCostsRecursive", () => { parent: { id: "parent", totalCost: 1.0, - linesAdded: 2, - linesRemoved: 1, childIds: ["child-1"], } as unknown as HistoryItem, "child-1": { id: "child-1", totalCost: 0.5, - linesAdded: 3, - linesRemoved: 2, childIds: [], } as unknown as HistoryItem, } @@ -73,12 +69,6 @@ describe("aggregateTaskCostsRecursive", () => { expect(result.ownCost).toBe(1.0) expect(result.childrenCost).toBe(0.5) expect(result.totalCost).toBe(1.5) - expect(result.ownAdded).toBe(2) - expect(result.ownRemoved).toBe(1) - expect(result.childrenAdded).toBe(3) - expect(result.childrenRemoved).toBe(2) - expect(result.totalAdded).toBe(5) - expect(result.totalRemoved).toBe(3) expect(result.childBreakdown).toHaveProperty("child-1") const child1 = result.childBreakdown?.["child-1"] expect(child1).toBeDefined() @@ -124,22 +114,16 @@ describe("aggregateTaskCostsRecursive", () => { parent: { id: "parent", totalCost: 1.0, - linesAdded: 2, - linesRemoved: 2, childIds: ["child"], } as unknown as HistoryItem, child: { id: "child", totalCost: 0.5, - linesAdded: 3, - linesRemoved: 1, childIds: ["grandchild"], } as unknown as HistoryItem, grandchild: { id: "grandchild", totalCost: 0.25, - linesAdded: 1, - linesRemoved: 4, childIds: [], } as unknown as HistoryItem, } @@ -152,22 +136,12 @@ describe("aggregateTaskCostsRecursive", () => { expect(result.childrenCost).toBe(0.75) // child (0.5) + grandchild (0.25) expect(result.totalCost).toBe(1.75) - expect(result.ownAdded).toBe(2) - expect(result.ownRemoved).toBe(2) - // children totals include all descendants - expect(result.childrenAdded).toBe(4) // child (3) + grandchild (1) - expect(result.childrenRemoved).toBe(5) // child (1) + grandchild (4) - expect(result.totalAdded).toBe(6) - expect(result.totalRemoved).toBe(7) - // Verify child breakdown const child = result.childBreakdown?.["child"] expect(child).toBeDefined() expect(child!.ownCost).toBe(0.5) expect(child!.childrenCost).toBe(0.25) expect(child!.totalCost).toBe(0.75) - expect(child!.totalAdded).toBe(4) - expect(child!.totalRemoved).toBe(5) // Verify grandchild breakdown const grandchild = child!.childBreakdown?.["grandchild"] @@ -175,8 +149,6 @@ describe("aggregateTaskCostsRecursive", () => { expect(grandchild!.ownCost).toBe(0.25) expect(grandchild!.childrenCost).toBe(0) expect(grandchild!.totalCost).toBe(0.25) - expect(grandchild!.totalAdded).toBe(1) - expect(grandchild!.totalRemoved).toBe(4) }) it("should detect and prevent circular references", async () => { @@ -184,15 +156,11 @@ describe("aggregateTaskCostsRecursive", () => { "task-a": { id: "task-a", totalCost: 1.0, - linesAdded: 2, - linesRemoved: 3, childIds: ["task-b"], } as unknown as HistoryItem, "task-b": { id: "task-b", totalCost: 0.5, - linesAdded: 4, - linesRemoved: 1, childIds: ["task-a"], // Circular reference back to task-a } as unknown as HistoryItem, } @@ -205,8 +173,6 @@ describe("aggregateTaskCostsRecursive", () => { expect(result.ownCost).toBe(1.0) expect(result.childrenCost).toBe(0.5) // Only task-b's own cost, circular ref returns 0 expect(result.totalCost).toBe(1.5) - expect(result.totalAdded).toBe(6) // task-a (2) + task-b (4) - expect(result.totalRemoved).toBe(4) // task-a (3) + task-b (1) // Verify warning was logged expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("Circular reference detected: task-a")) @@ -244,15 +210,11 @@ describe("aggregateTaskCostsRecursive", () => { root: { id: "root", totalCost: 1.0, - linesAdded: 1, - linesRemoved: 0, childIds: ["child-ok", "child-missing"], } as unknown as HistoryItem, "child-ok": { id: "child-ok", totalCost: 0.5, - linesAdded: 3, - linesRemoved: 2, childIds: [], } as unknown as HistoryItem, } @@ -285,16 +247,10 @@ describe("aggregateTaskCostsRecursive", () => { expect(result.ownCost).toBe(1.0) expect(result.childrenCost).toBe(0.5) expect(result.totalCost).toBe(1.5) - expect(result.childrenAdded).toBe(3) - expect(result.childrenRemoved).toBe(2) - expect(result.totalAdded).toBe(4) - expect(result.totalRemoved).toBe(2) const childOk = result.childBreakdown?.["child-ok"] expect(childOk).toBeDefined() expect(childOk!.totalCost).toBe(0.5) - expect(childOk!.totalAdded).toBe(3) - expect(childOk!.totalRemoved).toBe(2) // Missing child should not crash aggregation. expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("Task child-missing not found")) @@ -435,23 +391,11 @@ describe("buildSubtaskDetails", () => { ownCost: 0.5, childrenCost: 0, totalCost: 0.5, - ownAdded: 0, - ownRemoved: 0, - childrenAdded: 0, - childrenRemoved: 0, - totalAdded: 10, - totalRemoved: 5, }, "child-2": { ownCost: 0.3, childrenCost: 0.2, totalCost: 0.5, - ownAdded: 0, - ownRemoved: 0, - childrenAdded: 0, - childrenRemoved: 0, - totalAdded: 1, - totalRemoved: 2, }, } @@ -483,8 +427,6 @@ describe("buildSubtaskDetails", () => { expect(child1!.name).toBe("First subtask") expect(child1!.tokens).toBe(150) // 100 + 50 expect(child1!.cost).toBe(0.5) - expect(child1!.added).toBe(10) - expect(child1!.removed).toBe(5) expect(child1!.status).toBe("completed") expect(child1!.hasNestedChildren).toBe(false) @@ -493,8 +435,6 @@ describe("buildSubtaskDetails", () => { expect(child2!.name).toBe("Second subtask with nested children") expect(child2!.tokens).toBe(300) // 200 + 100 expect(child2!.cost).toBe(0.5) - expect(child2!.added).toBe(1) - expect(child2!.removed).toBe(2) expect(child2!.status).toBe("active") expect(child2!.hasNestedChildren).toBe(true) // childrenCost > 0 }) @@ -507,12 +447,6 @@ describe("buildSubtaskDetails", () => { ownCost: 1.0, childrenCost: 0, totalCost: 1.0, - ownAdded: 0, - ownRemoved: 0, - childrenAdded: 0, - childrenRemoved: 0, - totalAdded: 0, - totalRemoved: 0, }, } @@ -542,12 +476,6 @@ describe("buildSubtaskDetails", () => { ownCost: 1.0, childrenCost: 0, totalCost: 1.0, - ownAdded: 0, - ownRemoved: 0, - childrenAdded: 0, - childrenRemoved: 0, - totalAdded: 0, - totalRemoved: 0, }, } @@ -575,23 +503,11 @@ describe("buildSubtaskDetails", () => { ownCost: 0.5, childrenCost: 0, totalCost: 0.5, - ownAdded: 0, - ownRemoved: 0, - childrenAdded: 0, - childrenRemoved: 0, - totalAdded: 0, - totalRemoved: 0, }, "missing-child": { ownCost: 0.3, childrenCost: 0, totalCost: 0.3, - ownAdded: 0, - ownRemoved: 0, - childrenAdded: 0, - childrenRemoved: 0, - totalAdded: 0, - totalRemoved: 0, }, } @@ -630,12 +546,6 @@ describe("buildSubtaskDetails", () => { ownCost: 0.5, childrenCost: 0, totalCost: 0.5, - ownAdded: 0, - ownRemoved: 0, - childrenAdded: 0, - childrenRemoved: 0, - totalAdded: 0, - totalRemoved: 0, }, } @@ -662,12 +572,6 @@ describe("buildSubtaskDetails", () => { ownCost: 0.5, childrenCost: 0, totalCost: 0.5, - ownAdded: 0, - ownRemoved: 0, - childrenAdded: 0, - childrenRemoved: 0, - totalAdded: 0, - totalRemoved: 0, }, } diff --git a/src/core/webview/aggregateTaskCosts.ts b/src/core/webview/aggregateTaskCosts.ts index 2d163be9c9..ee8cb14acd 100644 --- a/src/core/webview/aggregateTaskCosts.ts +++ b/src/core/webview/aggregateTaskCosts.ts @@ -8,8 +8,6 @@ export interface SubtaskDetail { name: string // First 50 chars of task description tokens: number // tokensIn + tokensOut cost: number // Aggregated total cost - added: number // Aggregated total lines added - removed: number // Aggregated total lines removed status: "active" | "completed" | "delegated" hasNestedChildren: boolean // Has its own subtasks } @@ -18,12 +16,6 @@ export interface AggregatedCosts { ownCost: number // This task's own API costs childrenCost: number // Sum of all direct children costs (recursive) totalCost: number // ownCost + childrenCost - ownAdded: number // This task's own lines added - ownRemoved: number // This task's own lines removed - childrenAdded: number // Sum of all descendant lines added - childrenRemoved: number // Sum of all descendant lines removed - totalAdded: number // ownAdded + childrenAdded - totalRemoved: number // ownRemoved + childrenRemoved childBreakdown?: { // Optional detailed breakdown [childId: string]: AggregatedCosts @@ -51,12 +43,6 @@ export async function aggregateTaskCostsRecursive( ownCost: 0, childrenCost: 0, totalCost: 0, - ownAdded: 0, - ownRemoved: 0, - childrenAdded: 0, - childrenRemoved: 0, - totalAdded: 0, - totalRemoved: 0, } } visited.add(taskId) @@ -69,21 +55,11 @@ export async function aggregateTaskCostsRecursive( ownCost: 0, childrenCost: 0, totalCost: 0, - ownAdded: 0, - ownRemoved: 0, - childrenAdded: 0, - childrenRemoved: 0, - totalAdded: 0, - totalRemoved: 0, } } const ownCost = history.totalCost || 0 - const ownAdded = history.linesAdded || 0 - const ownRemoved = history.linesRemoved || 0 let childrenCost = 0 - let childrenAdded = 0 - let childrenRemoved = 0 const childBreakdown: { [childId: string]: AggregatedCosts } = {} // Recursively aggregate child costs @@ -95,8 +71,6 @@ export async function aggregateTaskCostsRecursive( new Set(visited), // Create new Set to allow sibling traversal ) childrenCost += childAggregated.totalCost - childrenAdded += childAggregated.totalAdded - childrenRemoved += childAggregated.totalRemoved childBreakdown[childId] = childAggregated } } @@ -105,12 +79,6 @@ export async function aggregateTaskCostsRecursive( ownCost, childrenCost, totalCost: ownCost + childrenCost, - ownAdded, - ownRemoved, - childrenAdded, - childrenRemoved, - totalAdded: ownAdded + childrenAdded, - totalRemoved: ownRemoved + childrenRemoved, childBreakdown, } @@ -144,8 +112,6 @@ export async function buildSubtaskDetails( name: truncateTaskName(history.task, 50), tokens: (history.tokensIn || 0) + (history.tokensOut || 0), cost: costs.totalCost, - added: costs.totalAdded, - removed: costs.totalRemoved, status: history.status || "completed", hasNestedChildren: costs.childrenCost > 0, }) diff --git a/src/shared/__tests__/messageUtils.spec.ts b/src/shared/__tests__/messageUtils.spec.ts index f1d16857eb..f3554fd9d1 100644 --- a/src/shared/__tests__/messageUtils.spec.ts +++ b/src/shared/__tests__/messageUtils.spec.ts @@ -1,259 +1,3 @@ -// 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, - }) - }) - }) -}) +// This file previously tested getLineStatsFromToolApprovalMessages, +// which has been removed as part of eliminating line change tracking. +// The file is kept as a placeholder for future messageUtils tests. diff --git a/src/shared/__tests__/typeGuards.spec.ts b/src/shared/__tests__/typeGuards.spec.ts index 837e1d8df7..45ad67686b 100644 --- a/src/shared/__tests__/typeGuards.spec.ts +++ b/src/shared/__tests__/typeGuards.spec.ts @@ -1,6 +1,6 @@ // npx vitest run src/shared/__tests__/typeGuards.spec.ts -import { isFiniteNumber, isDiffStats } from "../typeGuards" +import { isFiniteNumber } from "../typeGuards" describe("typeGuards", () => { describe("isFiniteNumber", () => { @@ -27,44 +27,4 @@ describe("typeGuards", () => { 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 index 048248727b..f284e57405 100644 --- a/src/shared/messageUtils.ts +++ b/src/shared/messageUtils.ts @@ -1,55 +1,2 @@ -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 } -} +// This file was emptied as part of removing line change processing from backend +// The getLineStatsFromToolApprovalMessages() function has been removed diff --git a/src/shared/typeGuards.ts b/src/shared/typeGuards.ts index ffd14ecb41..a884f66f68 100644 --- a/src/shared/typeGuards.ts +++ b/src/shared/typeGuards.ts @@ -1,5 +1,3 @@ -import type { DiffStats } from "@roo-code/types" - /** * Type guard to check if a value is a finite number * @param value - The value to check @@ -8,15 +6,3 @@ import type { DiffStats } from "@roo-code/types" 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) -} diff --git a/webview-ui/src/components/chat/TaskHeader.tsx b/webview-ui/src/components/chat/TaskHeader.tsx index a1f50f8560..9d067bf6cf 100644 --- a/webview-ui/src/components/chat/TaskHeader.tsx +++ b/webview-ui/src/components/chat/TaskHeader.tsx @@ -160,50 +160,6 @@ const TaskHeader = ({ return costs }, [todos, subtaskDetails]) - const aggregatedLineChanges = useMemo(() => { - const ownAdded = (currentTaskItem as any)?.linesAdded - const ownRemoved = (currentTaskItem as any)?.linesRemoved - - const processedSubtasks = new Set() - let childrenAdded = 0 - let childrenRemoved = 0 - - if (Array.isArray(subtaskDetails)) { - for (const subtask of subtaskDetails) { - if (!subtask?.id || typeof subtask.id !== "string") continue - if (processedSubtasks.has(subtask.id)) continue - processedSubtasks.add(subtask.id) - - if (typeof subtask.added === "number" && Number.isFinite(subtask.added)) { - childrenAdded += subtask.added - } - if (typeof subtask.removed === "number" && Number.isFinite(subtask.removed)) { - childrenRemoved += subtask.removed - } - } - } - - const totalAdded = (typeof ownAdded === "number" && Number.isFinite(ownAdded) ? ownAdded : 0) + childrenAdded - const totalRemoved = - (typeof ownRemoved === "number" && Number.isFinite(ownRemoved) ? ownRemoved : 0) + childrenRemoved - - const hasAdded = totalAdded > 0 - const hasRemoved = totalRemoved > 0 - const hasAnyLineChanges = hasAdded || hasRemoved - const formatted = [hasAdded ? `+${totalAdded}` : null, hasRemoved ? `−${totalRemoved}` : null] - .filter(Boolean) - .join(" ") - - return { - totalAdded, - totalRemoved, - hasAdded, - hasRemoved, - hasAnyLineChanges, - formatted, - } - }, [currentTaskItem, subtaskDetails]) - const tooltipCostData = useMemo( () => getTaskHeaderCostTooltipData({ @@ -383,20 +339,6 @@ const TaskHeader = ({ )} - {aggregatedLineChanges.hasAnyLineChanges && ( - - {aggregatedLineChanges.hasAdded && ( - - +{aggregatedLineChanges.totalAdded} - - )} - {aggregatedLineChanges.hasRemoved && ( - - −{aggregatedLineChanges.totalRemoved} - - )} - - )} {showBrowserGlobe && (
e.stopPropagation()}> @@ -572,28 +514,6 @@ const TaskHeader = ({ )} - {aggregatedLineChanges.hasAnyLineChanges && ( - - - {t("common:stats.lines")} - - - - {aggregatedLineChanges.hasAdded && ( - - +{aggregatedLineChanges.totalAdded} - - )} - {aggregatedLineChanges.hasRemoved && ( - - −{aggregatedLineChanges.totalRemoved} - - )} - - - - )} - {/* Size display */} {!!currentTaskItem?.size && currentTaskItem.size > 0 && ( diff --git a/webview-ui/src/components/chat/TodoListDisplay.tsx b/webview-ui/src/components/chat/TodoListDisplay.tsx index c3da1c717b..1dd5288f61 100644 --- a/webview-ui/src/components/chat/TodoListDisplay.tsx +++ b/webview-ui/src/components/chat/TodoListDisplay.tsx @@ -116,34 +116,6 @@ export function TodoListDisplay({ todos, subtaskDetails, onSubtaskClick }: TodoL const displayCost = todo.cost ?? subtaskById?.cost const shouldShowCost = typeof displayTokens === "number" && typeof displayCost === "number" - const todoAddedIsFinite = typeof todo.added === "number" && Number.isFinite(todo.added) - const todoRemovedIsFinite = typeof todo.removed === "number" && Number.isFinite(todo.removed) - - const displayAdded = todoAddedIsFinite ? todo.added : subtaskById?.added - const displayRemoved = todoRemovedIsFinite ? todo.removed : subtaskById?.removed - - const displayAddedIsFinite = typeof displayAdded === "number" && Number.isFinite(displayAdded) - const displayRemovedIsFinite = - typeof displayRemoved === "number" && Number.isFinite(displayRemoved) - const hasValidSubtaskLink = typeof todo.subtaskId === "string" && todo.subtaskId.length > 0 - - // Upstream aggregation may coerce missing stats to 0. - // To avoid showing misleading `+0/−0` for in-progress/pending rows, - // only render 0 while running if it was explicitly provided on the todo itself. - const canRenderAdded = - displayAddedIsFinite && - (todoStatus === "completed" || displayAdded !== 0 || todoAddedIsFinite) - const canRenderRemoved = - displayRemovedIsFinite && - (todoStatus === "completed" || displayRemoved !== 0 || todoRemovedIsFinite) - - const shouldShowLineChanges = hasValidSubtaskLink && (canRenderAdded || canRenderRemoved) - - const isAddedPositive = canRenderAdded && (displayAdded as number) > 0 - const isRemovedPositive = canRenderRemoved && (displayRemoved as number) > 0 - const isAddedZero = canRenderAdded && displayAdded === 0 - const isRemovedZero = canRenderRemoved && displayRemoved === 0 - return (
  • {/* Token count and cost display */} - {(shouldShowCost || shouldShowLineChanges) && ( + {shouldShowCost && ( - {shouldShowCost && ( - <> - - {formatLargeNumber(displayTokens)} - - - ${displayCost.toFixed(2)} - - - )} - {shouldShowLineChanges && ( - - - {canRenderAdded ? `+${displayAdded}` : "\u00A0"} - - - {canRenderRemoved ? `−${displayRemoved}` : "\u00A0"} - - - )} + + {formatLargeNumber(displayTokens)} + + + ${displayCost.toFixed(2)} + )}
    • diff --git a/webview-ui/src/components/chat/__tests__/TodoListDisplay.spec.tsx b/webview-ui/src/components/chat/__tests__/TodoListDisplay.spec.tsx index 67650153f1..8d0e16494b 100644 --- a/webview-ui/src/components/chat/__tests__/TodoListDisplay.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/TodoListDisplay.spec.tsx @@ -34,8 +34,6 @@ describe("TodoListDisplay", () => { name: "Task 1: Change background colour", tokens: 95400, cost: 0.22, - added: 10, - removed: 4, status: "completed", hasNestedChildren: false, }, @@ -44,8 +42,6 @@ describe("TodoListDisplay", () => { name: "Task 2: Add timestamp to bottom", tokens: 95000, cost: 0.24, - added: 3, - removed: 2, status: "completed", hasNestedChildren: false, }, @@ -183,207 +179,6 @@ describe("TodoListDisplay", () => { }) }) - describe("line change display", () => { - it("uses todo.added/todo.removed when present", () => { - const todosWithDirectLineChanges = [ - { - id: "1", - content: "Task 1: Change background colour", - status: "completed", - subtaskId: "subtask-1", - added: 7, - removed: 9, - }, - ] - render() - - // Expand - const header = screen.getByText("1 to-dos done") - fireEvent.click(header) - - // Line changes are rendered as separate colored spans - expect(screen.getByText("+7")).toBeInTheDocument() - expect(screen.getByText("−9")).toBeInTheDocument() - }) - - it("shows +0/−0 for completed subtask when fallback metrics are explicitly zero", () => { - const todosMissingDirectLineChanges = [ - { - id: "1", - content: "Task 1: Zero changes", - status: "completed", - subtaskId: "subtask-1", - }, - ] - const subtaskDetailsWithZeroLineChanges: SubtaskDetail[] = [ - { - id: "subtask-1", - name: "Task 1: Zero changes", - tokens: 1, - cost: 0.01, - added: 0, - removed: 0, - status: "completed", - hasNestedChildren: false, - }, - ] - - render( - , - ) - - // Expand - const header = screen.getByText("1 to-dos done") - fireEvent.click(header) - - const addedEl = screen.getByText("+0") - const removedEl = screen.getByText("−0") - - expect(addedEl).toBeInTheDocument() - expect(removedEl).toBeInTheDocument() - - // Zero values should be visually muted (not green/red emphasized) - expect(addedEl.className).toContain("opacity-50") - expect(addedEl.className).not.toContain("text-vscode-charts-green") - - expect(removedEl.className).toContain("opacity-50") - expect(removedEl.className).not.toContain("text-vscode-charts-red") - }) - - it("in-progress: does not show +0/−0 when zeros only come from fallback", () => { - const todosMissingDirectLineChanges = [ - { - id: "1", - content: "Task 1: Zero changes (running)", - status: "in_progress", - subtaskId: "subtask-1", - }, - ] - const subtaskDetailsWithZeroLineChanges: SubtaskDetail[] = [ - { - id: "subtask-1", - name: "Task 1: Zero changes (running)", - tokens: 1, - cost: 0.01, - added: 0, - removed: 0, - status: "active", - hasNestedChildren: false, - }, - ] - - render( - , - ) - - // Expand - const header = screen.getByText("Task 1: Zero changes (running)") - fireEvent.click(header) - - expect(screen.queryByText("+0")).not.toBeInTheDocument() - expect(screen.queryByText("−0")).not.toBeInTheDocument() - }) - - it("in-progress: shows +0/−0 when explicitly present on todo", () => { - const todosWithDirectLineChanges = [ - { - id: "1", - content: "Task 1: Zero changes (explicit)", - status: "in_progress", - subtaskId: "subtask-1", - added: 0, - removed: 0, - }, - ] - - render() - - // Expand - const header = screen.getByText("Task 1: Zero changes (explicit)") - fireEvent.click(header) - - const addedEl = screen.getByText("+0") - const removedEl = screen.getByText("−0") - expect(addedEl).toBeInTheDocument() - expect(removedEl).toBeInTheDocument() - - expect(addedEl.className).toContain("opacity-50") - expect(removedEl.className).toContain("opacity-50") - }) - - it("falls back to subtaskDetails when todo added/removed are missing", () => { - const todosMissingDirectLineChanges = [ - { - id: "1", - content: "Task 1: Change background colour", - status: "completed", - subtaskId: "subtask-1", - }, - ] - render() - - // Expand - const header = screen.getByText("1 to-dos done") - fireEvent.click(header) - - // Line changes are rendered as separate colored spans - expect(screen.getByText("+10")).toBeInTheDocument() - expect(screen.getByText("−4")).toBeInTheDocument() - }) - - it("hides line deltas when no data available (no subtaskId)", () => { - const todosNoSubtaskLink = [{ id: "1", content: "No link todo", status: "completed" }] - render() - - // Expand - const header = screen.getByText("1 to-dos done") - fireEvent.click(header) - - expect(screen.queryByText(/\+\d+/)).not.toBeInTheDocument() - expect(screen.queryByText(/−\d+/)).not.toBeInTheDocument() - }) - - it("hides line deltas when all values are undefined (subtaskId present)", () => { - const todosWithLinkButNoLineChanges = [ - { - id: "1", - content: "Task 1: Change background colour", - status: "completed", - subtaskId: "subtask-1", - }, - ] - const subtaskDetailsWithoutLineChanges: SubtaskDetail[] = [ - { - id: "subtask-1", - name: "Task 1: Change background colour", - tokens: 95400, - cost: 0.22, - status: "completed", - hasNestedChildren: false, - } as unknown as SubtaskDetail, - ] - render( - , - ) - - // Expand - const header = screen.getByText("1 to-dos done") - fireEvent.click(header) - - expect(screen.queryByText(/\+\d+/)).not.toBeInTheDocument() - expect(screen.queryByText(/−\d+/)).not.toBeInTheDocument() - }) - }) - describe("click handler", () => { it("should call onSubtaskClick when a todo with subtaskId is clicked", () => { const onSubtaskClick = vi.fn() diff --git a/webview-ui/src/types/subtasks.ts b/webview-ui/src/types/subtasks.ts index e0d2bd7322..a4cd97ed4f 100644 --- a/webview-ui/src/types/subtasks.ts +++ b/webview-ui/src/types/subtasks.ts @@ -5,10 +5,6 @@ export type SubtaskDetail = { name: string /** tokensIn + tokensOut */ tokens: number - /** Total lines added across the subtask */ - added: number - /** Total lines removed across the subtask */ - removed: number /** Aggregated total cost */ cost: number status: "active" | "completed" | "delegated"