Skip to content

fix(chatgpt): use pure sleeps and cheap generation checks in polling loops#2099

Open
Astro-Han wants to merge 4 commits into
jackwener:mainfrom
Astro-Han:fix/2095-chatgpt-cheap-polling
Open

fix(chatgpt): use pure sleeps and cheap generation checks in polling loops#2099
Astro-Han wants to merge 4 commits into
jackwener:mainfrom
Astro-Han:fix/2095-chatgpt-cheap-polling

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Description

Part 1 of 2 for the renderer-CPU problem in long-running chatgpt ask (part 2 will add session busy arbitration).

During a 10-20 min ChatGPT response, the chatgpt.com renderer climbs to ~700% CPU and >4GB RSS. Two mechanisms in the polling loops are responsible:

  1. page.wait(3) between polls is not a sleep: its numeric path injects waitForDomStableJs, which installs a MutationObserver on document.body with {childList, subtree, attributes} and only resolves after 500ms of quiet. A streaming response never goes quiet, so every poll runs a whole-body observer for the full 3s cap, firing on every mutation, re-armed continuously for the entire generation.
  2. isGenerating read document.body.innerText every poll, forcing a synchronous full-page reflow and allocating a conversation-sized string in the renderer heap, cost growing with response length.

Changes:

  • Add page.sleep(seconds): a pure client-side setTimeout with no page evaluation. wait(n) semantics are unchanged for post-navigation settles that benefit from DOM-stable early return.
  • Convert the chatgpt polling loops (waitForChatGPTResponse, waitForChatGPTDetailRows, waitForChatGPTDeepResearchResult, waitForChatGPTImages poll interval, upload preview poll, ask pre-send settle) to page.sleep. One-shot post-goto settles keep page.wait.
  • Rewrite isGenerating without body.innerText: stop-button test id, control aria-labels, and a textContent (no reflow) scan scoped to the last conversation turn + composer container.
  • getVisibleMessages gains { textOnly } to skip per-turn innerHTML during response polling; read/detail markdown paths are untouched and their output is byte-identical.

ask.js waitForConversationUrl and sendChatGPTMessage are deliberately untouched to avoid conflicting with #2094.

Related issue: #2095

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 🌐 New site adapter
  • 📝 Documentation
  • ♻️ Refactor
  • 🔧 CI / build / tooling

Checklist

  • I ran the checks relevant to this PR
  • I updated tests or docs if needed
  • I included output or screenshots when useful

Documentation (if adding/modifying an adapter)

  • Added doc page under docs/adapters/ (if new adapter) — N/A, no new adapter
  • Updated docs/adapters/index.md table (if new adapter) — N/A
  • Updated sidebar in docs/.vitepress/config.mts (if new adapter) — N/A
  • Updated README.md / README.zh-CN.md when command discoverability changed — N/A, no command surface change
  • Used positional args for the command's primary subject unless a named flag is clearly better (no arg changes)
  • Normalized expected adapter failures to CliError subclasses instead of raw Error (unchanged; existing CommandExecutionError/TimeoutError paths kept)

Screenshots / Output

npx tsc --noEmit          # clean
npm test                  # 548 files, 5954 passed | 1 skipped
npx vitest run clis/chatgpt/ src/browser/base-page.test.ts src/pipeline/
                          # 11 files, 278 passed

New tests: BasePage.sleep never evaluates page script; the chatgpt response-wait loop polls via sleep and never hits the DOM-stable numeric wait path.

Live Validation (added after review rounds)

Dogfooded during real ChatGPT sessions while this PR was under review; two failures of the 1.8.6 isGenerating were reproduced and fixed here:

  1. Project page false positive (1.8.6): chatgpt ask --project ... on an empty project page failed with "conversation is still generating" — the whole-body innerText scan matched thinking-status text inside the conversation-list previews. This PR's scoped scan is immune; the same command succeeded with this branch.
  2. Content-mention false positive (found in round-2 re-review, fixed in 813747c): a completed conversation whose answer text merely discussed the literal words Thinking / 正在思考 was reported as generating, permanently blocking follow-up sends. Red-green verified live: Generating 2→0 on the same conversation before/after the fix.
  3. Idle model-label false positive (round-2 finding, fixed in 902082d): with the Thinking model selected, the composer's model button (aria-label="Thinking") matched the aria scan on an idle conversation. Scan patterns are now per-scope: composer scope no longer treats bare Thinking as a generating signal (English streaming state is covered by stop-button).

External review: 3 rounds of GPT Pro review on this PR; final verdict after 902082d: no remaining substantive findings.

…loops (jackwener#2095)

During a long `chatgpt ask` (10-20 min answers) the chatgpt.com renderer hit
~700% CPU and >4GB RSS. Root causes, all in the poll loops that run for the
whole generation:

- `page.wait(n>=1)` does not sleep client-side; it injects a whole-body
  MutationObserver (DOM-stable probe) that never goes quiet while the answer
  streams, so it re-arms and fires on every mutation for the full interval.
  Add `page.sleep(seconds)` (bare setTimeout, no page evaluation) to BasePage
  and IPage, and switch the poll-interval waits to it: waitForChatGPTResponse,
  waitForChatGPTDetailRows, waitForChatGPTDeepResearchResult,
  waitForChatGPTImages, waitForChatGPTUploadPreview, and the ask pre-send
  settle loop. One-shot post-navigation settle waits keep `page.wait` for its
  DOM-stable early return.

- `isGenerating` read `document.body.innerText` every poll, forcing a full-page
  reflow and a conversation-sized string allocation. Rewrite it to cheap
  signals: stop-button test id, control aria-labels, and a `textContent`
  (no reflow) scan scoped to the composer + last turn.

- `getVisibleMessages` read both innerHTML and innerText per turn. Add a
  `textOnly` option that skips innerHTML and use it from the response poll
  loop, whose output is text-only; read/detail markdown paths are unchanged.
Astro-Han added 3 commits July 7, 2026 15:01
…an (jackwener#2095)

The scoped text fallback only looked at article conversation turns, but
CONVERSATION_MESSAGE_SELECTOR supports bare [data-message-author-role]
nodes too. On that DOM shape a plain-text Thinking pill (no stop button,
no aria-label) would read as idle and waitForChatGPTResponse could
return a truncated answer. Prefer the article turn (wider container),
fall back to the last role-attribute node when articles are absent.

Addresses the P2 from external review of PR jackwener#2099.
…ting (jackwener#2095)

Scanning whole-scope textContent flags any finished answer that merely
mentions "Thinking" / "正在思考" (prose or backticked code spans) as
still generating — e.g. a conversation reviewing this very code —
permanently blocking follow-up sends. Count only short leaf elements
outside .markdown/pre/code as status pills.

Verified against a live conversation whose messages discuss isGenerating:
detail reported Generating=true before, false after; a real streaming
pill still matches (leaf, short, outside rendered content).
…te (jackwener#2095)

'Thinking' is a supported idle model label (CHATGPT_MODEL_TARGETS.advanced)
rendered as a composer-form button, so both the page-wide aria-label match
and the composer-scope leaf scan flagged an idle conversation with that
model selected as generating forever, blocking sends. Drop bare 'Thinking'
from aria-label matching (the stop button covers English streaming states)
and only count it inside the last conversation turn.

Addresses the round-2 P2 from external review of PR jackwener#2099.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant