fix(ui): suppress duplicate identical TodoList panels in a single turn (#9692)

* fix: deduplicate identical TodoList panels in single turn * fix(ui): address review - type unchanged, fix reminders, fix snapshot * fix(ui): address review comments - extract reminder helper, fix TS types, and add no-op test coverage --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-08-31 10:16:57 +00:00 · 2026-08-26 07:30:14 +00:00 · 2026-08-26 07:30:14 +00:00 · c7f7b2d975
commit c7f7b2d975
parent 53b0e4b57b
8 changed files with 326 additions and 29 deletions
--- a/packages/cli/src/ui/components/messages/ToolMessage.test.tsx
+++ b/packages/cli/src/ui/components/messages/ToolMessage.test.tsx
@ -16,6 +16,7 @@ import type {
  AnsiOutput,
  AnsiOutputDisplay,
  Config,
+  TodoResultDisplay,
 } from '@qwen-code/qwen-code-core';
 import type { LoadedSettings } from '../../../config/settings.js';
 import { getScreenBuffer } from '../../selection/screen-buffer.js';
@ -141,6 +142,14 @@ vi.mock('./ToolConfirmationMessage.js', () => ({
  },
 }));

+vi.mock('../TodoDisplay.js', () => ({
+  TodoDisplay: ({
+    todos,
+  }: {
+    todos: Array<{ content: string; status: string }>;
+  }) => <Text>{todos.map((t) => t.content).join(', ')}</Text>,
+}));
+
 // Mock settings
 const mockSettings: LoadedSettings = {
  merged: {
@ -851,6 +860,57 @@ describe('<ToolMessage />', () => {
    expect(lastFrame()).toMatch(/MockDiff:--- a\/file\.txt/);
  });

+  it('suppresses todo panel when resultDisplay has unchanged flag', () => {
+    const { lastFrame } = renderWithContext(
+      <ToolMessage
+        {...baseProps}
+        name="TodoWrite"
+        description="Update todos"
+        resultDisplay={
+          {
+            type: 'todo_list',
+            todos: [
+              { id: '1', content: 'Task A', status: 'in_progress' },
+              { id: '2', content: 'Task B', status: 'pending' },
+            ],
+            unchanged: true,
+          } as TodoResultDisplay
+        }
+        forceShowResult
+      />,
+      StreamingState.Idle,
+    );
+    const output = lastFrame() ?? '';
+    expect(output).toContain('TodoWrite');
+    // TodoDisplay should NOT render when unchanged is true
+    expect(output).not.toContain('Task A');
+    expect(output).not.toContain('Task B');
+    expect(output).not.toContain('in_progress');
+  });
+
+  it('renders todo panel normally when unchanged flag is absent', () => {
+    const { lastFrame } = renderWithContext(
+      <ToolMessage
+        {...baseProps}
+        name="TodoWrite"
+        description="Update todos"
+        resultDisplay={{
+          type: 'todo_list',
+          todos: [
+            { id: '1', content: 'Task A', status: 'in_progress' },
+            { id: '2', content: 'Task B', status: 'pending' },
+          ],
+        }}
+        forceShowResult
+      />,
+      StreamingState.Idle,
+    );
+    const output = lastFrame() ?? '';
+    expect(output).toContain('TodoWrite');
+    expect(output).toContain('Task A');
+    expect(output).toContain('Task B');
+  });
+
  it('diff results are not collapsed for completed collapsible tools (bypass shouldCollapseResult)', () => {
    const diffResult = {
      fileDiff: '--- a/file.txt\n+++ b/file.txt\n@@ -1 +1 @@\n-old\n+new',
--- a/packages/cli/src/ui/components/messages/ToolMessage.tsx
+++ b/packages/cli/src/ui/components/messages/ToolMessage.tsx
@ -334,7 +334,12 @@ const useResultDisplayRenderer = (
 */
 const TodoResultRenderer: React.FC<{ data: TodoResultDisplay }> = ({
  data,
-}) => <TodoDisplay todos={data.todos} />;
+}) => {
+  if (data.unchanged) {
+    return null;
+  }
+  return <TodoDisplay todos={data.todos} />;
+};

 const PlanResultRenderer: React.FC<{
  data: PlanResultDisplay;
--- a/packages/cli/src/ui/utils/todoSnapshot.test.ts
+++ b/packages/cli/src/ui/utils/todoSnapshot.test.ts
@ -466,4 +466,79 @@ describe('sticky todo layout helpers', () => {
      expect(getStickyTodoMaxVisibleItemsForMode(8, true)).toBe(1);
    });
  });
+
+  describe('unchanged snapshot guard', () => {
+    it('falls back to previous snapshot when the latest committed snapshot is unchanged', () => {
+      const history = [
+        makeCustomTodoToolGroup(
+          [{ id: '1', content: 'Old Task', status: 'in_progress' }],
+          1,
+        ),
+        makeGeminiHistoryItem('Response', 2),
+        makeGeminiHistoryItem('Response 2', 3),
+        {
+          type: 'tool_group' as const,
+          tools: [
+            {
+              callId: 'todo-unchanged',
+              name: 'TodoWrite',
+              description: 'Update todos',
+              resultDisplay: {
+                type: 'todo_list' as const,
+                todos: [
+                  { id: '1', content: 'Old Task', status: 'in_progress' },
+                ],
+                unchanged: true,
+              },
+              status: ToolCallStatus.Success,
+              confirmationDetails: undefined,
+            },
+          ],
+          id: 4,
+        },
+      ] as HistoryItem[];
+
+      // It should skip the unchanged snapshot and return the previous valid one
+      expect(getStickyTodos(history, [])).toEqual([
+        { id: '1', content: 'Old Task', status: 'in_progress' },
+      ]);
+    });
+
+    it('allows history snapshot to be used when pending snapshot is unchanged', () => {
+      const history = [
+        makeCustomTodoToolGroup(
+          [{ id: '1', content: 'History Task', status: 'pending' }],
+          1,
+        ),
+        makeGeminiHistoryItem('Response', 2),
+        makeGeminiHistoryItem('Response 2', 3),
+      ] as HistoryItem[];
+
+      const pendingHistoryItems = [
+        {
+          type: 'tool_group' as const,
+          tools: [
+            {
+              callId: 'todo-pending-unchanged',
+              name: 'TodoWrite',
+              description: 'Update todos',
+              resultDisplay: {
+                type: 'todo_list' as const,
+                todos: [
+                  { id: '1', content: 'History Task', status: 'pending' },
+                ],
+                unchanged: true,
+              },
+              status: ToolCallStatus.Success,
+              confirmationDetails: undefined,
+            },
+          ],
+        },
+      ] as HistoryItemWithoutId[];
+
+      expect(getStickyTodos(history, pendingHistoryItems)).toEqual([
+        { id: '1', content: 'History Task', status: 'pending' },
+      ]);
+    });
+  });
 });
--- a/packages/cli/src/ui/utils/todoSnapshot.ts
+++ b/packages/cli/src/ui/utils/todoSnapshot.ts
@ -62,6 +62,9 @@ function extractTodosFromResultDisplay(
      candidate['type'] === 'todo_list' &&
      Array.isArray(candidate['todos'])
    ) {
+      if (candidate['unchanged'] === true) {
+        return null;
+      }
      return candidate['todos'] as TodoItem[];
    }
  }
--- a/packages/core/src/tools/todoWrite.test.ts
+++ b/packages/core/src/tools/todoWrite.test.ts
@ -322,19 +322,136 @@ describe('TodoWriteTool', () => {
          todos: [{ id: '1', content: 'Done', status: 'completed' }],
        }),
      );
-      mockFs.mkdir.mockResolvedValue(undefined);
-      mockAtomicWrite.mockResolvedValue(undefined);
+
+      const result = await promptIdContext.run('todo-prompt', () =>
+        tool
+          .build({
+            todos: [{ id: '1', content: 'Done', status: 'completed' }],
+          })
+          .execute(mockAbortSignal),
+      );
+
+      // Identical todos → no-op short-circuit, no write
+      expect(mockAtomicWrite).not.toHaveBeenCalled();
+      expect(result.returnDisplay).toMatchObject({
+        planId: 'finished-plan',
+        unchanged: true,
+      });
+      expect(mockConfig.setActiveTodoReminder).toHaveBeenCalledWith(
+        'todo-prompt',
+        undefined,
+      );
+    });
+
+    it('should short-circuit with unchanged flag when todos are identical', async () => {
+      const existingTodos: TodoItem[] = [
+        { id: '1', content: 'Task 1', status: 'in_progress' },
+        { id: '2', content: 'Task 2', status: 'pending' },
+      ];
+
+      mockFs.readFile.mockResolvedValue(
+        JSON.stringify({ planId: 'plan-abc', todos: existingTodos }),
+      );
+
+      const result = await promptIdContext.run('todo-prompt', () =>
+        tool.build({ todos: existingTodos }).execute(mockAbortSignal),
+      );
+
+      // No file write or hooks should fire
+      expect(mockAtomicWrite).not.toHaveBeenCalled();
+      expect(mockFs.mkdir).not.toHaveBeenCalled();
+
+      // Display signals unchanged to UI layer
+      expect(result.returnDisplay).toMatchObject({
+        type: 'todo_list',
+        planId: 'plan-abc',
+        todos: existingTodos,
+        changes: { created: [], completed: [] },
+        unchanged: true,
+      });
+
+      // LLM content tells model no change occurred
+      expect(result.llmContent).toContain('already up to date');
+      expect(result.llmContent).toContain('No changes were needed');
+      expect(result.llmContent).not.toContain('modified successfully');
+
+      expect(mockConfig.setActiveTodoReminder).toHaveBeenCalledWith(
+        'todo-prompt',
+        expect.stringContaining('Task 1'),
+      );
+    });
+
+    it('should not fire hooks on no-op todo_write', async () => {
+      const existingTodos: TodoItem[] = [
+        { id: '1', content: 'Task 1', status: 'pending' },
+      ];
+
+      const mockHookSystem = {
+        fireTodoCreatedEvent: vi.fn(),
+        fireTodoCompletedEvent: vi.fn(),
+      };
+      mockConfig = {
+        getSessionId: () => 'test-session-123',
+        getHookSystem: () => mockHookSystem,
+      } as unknown as Config;
+      tool = new TodoWriteTool(mockConfig);
+
+      mockFs.readFile.mockResolvedValue(
+        JSON.stringify({ todos: existingTodos }),
+      );
+
+      await tool.build({ todos: existingTodos }).execute(mockAbortSignal);
+
+      expect(mockHookSystem.fireTodoCreatedEvent).not.toHaveBeenCalled();
+      expect(mockHookSystem.fireTodoCompletedEvent).not.toHaveBeenCalled();
+    });
+
+    it('should short-circuit with unchanged flag when modified_by_user yields identical todos', async () => {
+      const existingTodos: TodoItem[] = [
+        { id: '1', content: 'Task 1', status: 'in_progress' },
+      ];
+      mockFs.readFile.mockResolvedValue(
+        JSON.stringify({ planId: 'plan-abc', todos: existingTodos }),
+      );
+
+      const modifiedContent = JSON.stringify({ todos: existingTodos });

      const result = await tool
        .build({
-          todos: [{ id: '1', content: 'Done', status: 'completed' }],
+          todos: [],
+          modified_by_user: true,
+          modified_content: modifiedContent,
        })
        .execute(mockAbortSignal);

-      expect(result.returnDisplay).toMatchObject({ planId: 'finished-plan' });
-      expect(
-        JSON.parse(mockAtomicWrite.mock.calls[0][1] as string),
-      ).toMatchObject({ planId: 'finished-plan' });
+      expect(mockAtomicWrite).not.toHaveBeenCalled();
+      expect(result.returnDisplay).toMatchObject({ unchanged: true });
+    });
+
+    it('should return an error result if modified_content has invalid parsed todos', async () => {
+      const existingTodos: TodoItem[] = [
+        { id: '1', content: 'Task 1', status: 'in_progress' },
+      ];
+      mockFs.readFile.mockResolvedValue(
+        JSON.stringify({ planId: 'plan-abc', todos: existingTodos }),
+      );
+
+      // Parsing an invalid todo list (empty content)
+      const modifiedContent = JSON.stringify({
+        todos: [{ id: '1', content: '', status: 'pending' }],
+      });
+
+      const result = await tool
+        .build({
+          todos: [],
+          modified_by_user: true,
+          modified_content: modifiedContent,
+        })
+        .execute(mockAbortSignal);
+
+      expect(mockAtomicWrite).not.toHaveBeenCalled();
+      // execute catches validation errors and returns an error string
+      expect(result.returnDisplay).toContain('non-empty "content"');
    });

    it('should start a new plan after the previous plan completed', async () => {
--- a/packages/core/src/tools/todoWrite.ts
+++ b/packages/core/src/tools/todoWrite.ts
@ -270,6 +270,26 @@ class TodoWriteToolInvocation extends BaseToolInvocation<
    this.operationType = operationType;
  }

+  private refreshActiveTodoReminder(todos: TodoItem[]): void {
+    const promptId = promptIdContext.getStore();
+    if (!promptId) return;
+
+    const unfinishedTodos = todos.filter((todo) => todo.status !== 'completed');
+    const serializedTodos = escapeSystemReminderTags(
+      unfinishedTodos
+        .map((todo) => `- [${todo.status}] ${todo.content}`)
+        .join('\n'),
+    );
+    const todoContext = serializedTodos.slice(0, MAX_ACTIVE_TODO_CONTEXT_CHARS);
+
+    this.config.setActiveTodoReminder(
+      promptId,
+      unfinishedTodos.length > 0
+        ? `<system-reminder>\nThe current task still has unfinished todo items:\n${todoContext}${serializedTodos.length > todoContext.length ? '\n[truncated]' : ''}\nKeep the todo list current and continue the task. Do not treat a successful intermediate tool call as task completion.\n</system-reminder>`
+        : undefined,
+    );
+  }
+
  getDescription(): string {
    return this.operationType === 'create' ? 'Create todos' : 'Update todos';
  }
@ -298,6 +318,31 @@ class TodoWriteToolInvocation extends BaseToolInvocation<
      if (validationError) throw new Error(validationError);
      const finalTodos = candidateTodos as TodoItem[];

+      if (isDeepStrictEqual(oldTodos, finalTodos)) {
+        debugLogger.debug(
+          '[TodoWriteTool] No-op: todos unchanged, skipping write/hooks',
+        );
+
+        this.refreshActiveTodoReminder(finalTodos);
+
+        const todoResultDisplay = {
+          type: 'todo_list' as const,
+          ...(previousPlan.planId ? { planId: previousPlan.planId } : {}),
+          todos: finalTodos,
+          changes: { created: [], completed: [] },
+          unchanged: true,
+        };
+
+        return {
+          llmContent: `Todo list is already up to date. No changes were needed.
+
+<system-reminder>
+Your todo list was not modified because it is already current. Continue with your existing tasks.
+</system-reminder>`,
+          returnDisplay: todoResultDisplay,
+        };
+      }
+
      // 2. Detect changes
      const changes = detectTodoChanges(oldTodos, finalTodos);
      const oldTodosMap = new Map(oldTodos.map((t) => [t.id, t]));
@ -382,27 +427,7 @@ class TodoWriteToolInvocation extends BaseToolInvocation<

      // 4. Write new todos AFTER all validation passes
      await writeTodosToFile(finalTodos, activePlanId, sessionId);
-      const unfinishedTodos = finalTodos.filter(
-        (todo) => todo.status !== 'completed',
-      );
-      const promptId = promptIdContext.getStore();
-      if (promptId) {
-        const serializedTodos = escapeSystemReminderTags(
-          unfinishedTodos
-            .map((todo) => `- [${todo.status}] ${todo.content}`)
-            .join('\n'),
-        );
-        const todoContext = serializedTodos.slice(
-          0,
-          MAX_ACTIVE_TODO_CONTEXT_CHARS,
-        );
-        this.config.setActiveTodoReminder(
-          promptId,
-          unfinishedTodos.length > 0
-            ? `<system-reminder>\nThe current task still has unfinished todo items:\n${todoContext}${serializedTodos.length > todoContext.length ? '\n[truncated]' : ''}\nKeep the todo list current and continue the task. Do not treat a successful intermediate tool call as task completion.\n</system-reminder>`
-            : undefined,
-        );
-      }
+      this.refreshActiveTodoReminder(finalTodos);

      // 5. POST-WRITE PHASE: Execute hooks for side effects (logging, HTTP sync, etc.)
      // These hooks can now safely perform side effects knowing data is persisted
--- a/packages/core/src/tools/tools.ts
+++ b/packages/core/src/tools/tools.ts
@ -890,6 +890,7 @@ export interface TodoResultDisplay {
    status: 'pending' | 'in_progress' | 'completed';
    blockedBy?: string[];
  }>;
+  unchanged?: boolean;
 }

 export interface PlanResultDisplay {
--- a/packages/core/src/utils/toolResultDisplayCompaction.test.ts
+++ b/packages/core/src/utils/toolResultDisplayCompaction.test.ts
@ -67,6 +67,17 @@ describe('toolResultDisplayCompaction', () => {
    expect(compacted).toContain('truncated from');
  });

+  it('should preserve the unchanged flag through compaction', () => {
+    const display = {
+      type: 'todo_list' as const,
+      todos: [{ id: '1', content: 'Task', status: 'pending' as const }],
+      changes: { created: [], completed: [] },
+      unchanged: true,
+    };
+    const compacted = compactToolResultDisplayForHistory(display);
+    expect((compacted as TodoResultDisplay).unchanged).toBe(true);
+  });
+
  it('uses saved session wording when compacting recording strings', () => {
    const value = `start-${'x'.repeat(
      MAX_RETAINED_TOOL_RESULT_DISPLAY_CHARS,