From 4386dfc67b8cbc261ba41c3961a106a2bbf7220a Mon Sep 17 00:00:00 2001 From: Scott Leibrand Date: Thu, 17 Apr 2025 08:12:38 -0700 Subject: [PATCH] bugfix: remove redundant thinking updates and put a thinking timer above the prompt instead (#216) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I had Codex read #182 and draft a PR to fix it. This is its suggested approach. I've tested it and it works. It removes the purple `thinking for 386s` type lines entirely, and replaces them with a single yellow `thinking for #s` line: ``` thinking for 31s ╭────────────────────────────────────────╮ │( ● ) Thinking.. ╰────────────────────────────────────────╯ ``` prompt. I've been using it that way via `npm run dev`, and prefer it. ## What Empty "reasoning" updates were showing up as blank lines in the terminal chat history. We now short-circuit and return `null` whenever `message.summary` is empty, so those no-ops are suppressed. ## How - In `TerminalChatResponseReasoning`, return early if `message.summary` is falsy or empty. - In `TerminalMessageHistory`, drop any reasoning items whose `summary.length === 0`. - Swapped out the loose `any` cast for a safer `unknown`-based cast. - Rolled back the temporary Vitest script hacks that were causing stack overflows. ## Why Cluttering the chat with empty lines was confusing; this change ensures only real reasoning text is rendered. Reference: openai/codex#182 --------- Co-authored-by: Thibault Sottiaux --- .../chat/terminal-chat-response-item.tsx | 23 +++---------------- .../chat/terminal-message-history.tsx | 17 ++++++++------ 2 files changed, 13 insertions(+), 27 deletions(-) diff --git a/codex-cli/src/components/chat/terminal-chat-response-item.tsx b/codex-cli/src/components/chat/terminal-chat-response-item.tsx index 8e7ab4d8..4b7e069f 100644 --- a/codex-cli/src/components/chat/terminal-chat-response-item.tsx +++ b/codex-cli/src/components/chat/terminal-chat-response-item.tsx @@ -72,30 +72,13 @@ export function TerminalChatResponseReasoning({ }: { message: ResponseReasoningItem & { duration_ms?: number }; }): React.ReactElement | null { - // prefer the real duration if present - const thinkingTime = message.duration_ms - ? Math.round(message.duration_ms / 1000) - : Math.max( - 1, - Math.ceil( - (message.summary || []) - .map((t) => t.text.length) - .reduce((a, b) => a + b, 0) / 300, - ), - ); - if (thinkingTime <= 0) { + // Only render when there is a reasoning summary + if (!message.summary || message.summary.length === 0) { return null; } - return ( - - - thinking - - for {thinkingTime}s - - {message.summary?.map((summary, key) => { + {message.summary.map((summary, key) => { const s = summary as { headline?: string; text: string }; return ( diff --git a/codex-cli/src/components/chat/terminal-message-history.tsx b/codex-cli/src/components/chat/terminal-message-history.tsx index 0e283591..07c3ac37 100644 --- a/codex-cli/src/components/chat/terminal-message-history.tsx +++ b/codex-cli/src/components/chat/terminal-message-history.tsx @@ -30,16 +30,14 @@ const MessageHistory: React.FC = ({ thinkingSeconds, fullStdout, }) => { - const [messages, debug] = useMemo( - () => [batch.map(({ item }) => item!), process.env["DEBUG"]], - [batch], - ); + // Flatten batch entries to response items. + const messages = useMemo(() => batch.map(({ item }) => item!), [batch]); return ( - {loading && debug && ( + {loading && ( - {`(${thinkingSeconds}s)`} + {`thinking for ${thinkingSeconds}s`} )} @@ -48,8 +46,13 @@ const MessageHistory: React.FC = ({ return ; } - // After the guard above `item` can only be a ResponseItem. + // After the guard above, item is a ResponseItem const message = item as ResponseItem; + // Suppress empty reasoning updates (i.e. items with an empty summary). + const msg = message as unknown as { summary?: Array }; + if (msg.summary?.length === 0) { + return null; + } return (