fix(idle-detector): 500ms grace timer after readyPattern, immune to TUI redraws#224
Open
flipped0119 wants to merge 1 commit into
Open
fix(idle-detector): 500ms grace timer after readyPattern, immune to TUI redraws#224flipped0119 wants to merge 1 commit into
flipped0119 wants to merge 1 commit into
Conversation
…UI redraws When a CLI's readyPattern fires (input prompt visible), IdleDetector previously fell back to the full 2s QUIESCENCE_MS wait. Hermes and Codex TUIs redraw their status bars every ~1ms — clearing the quiescence timer on every feed() — so idle was never declared until the redraws stopped, adding ~13s to the detection latency on long sessions. Add READY_GRACE_MS = 500 grace window that: * Arms once when readyPattern first matches * Is NOT reset by subsequent feed() calls (TUI status-bar churn) * Triggers markIdle() after 500ms — matches completionPattern's 500ms Spurious spinners in the post-ready output are already blocked by the existing `!readySeen` guard at the top of feed(); we don't re-check here. Spike: full startup banner 3001ms → 1500ms; 11s of 1ms-spaced redraws 13099ms → 600ms (22x speedup). Adds 4 unit tests covering: arm-once, no-reset-by-feed, markIdle fires after 500ms, no interaction with completionPattern.
Owner
|
@flipped0119 先谢谢这个 PR,思路和测试都很清楚 🙏 提速本身是真需求。但我在 review 时有一个收益/风险错配的担忧想跟你对齐一下,也想了解你那边的实际观测,再决定怎么合。 我的担忧新的 grace 路径是「readyPattern 命中后 arm 一次 500ms、后续 feed 不重置、到点直接
具体到受影响的 CLI(有 readyPattern、无 completionPattern):
为什么我特别在意 codex-app
想请教你几个问题
一个可能更稳的方向(供讨论)如果痛点确实主要在启动,能不能让 grace 路径只在屏幕近端确实安静下来时才快判(比如保留对最近真实输出/spinner 的一个轻量检查),干活中途有持续输出就照常推迟?这样启动提速照拿,中途误判不发生。或者更根上的:把 codex-app 也接上 codex 的 rollout 日志兜底,盯屏快不快就无所谓了。 想先听听你的实际效果和考虑,再一起定怎么合 🙏 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a CLI's
readyPatternfires (input prompt visible),IdleDetectorpreviously fell back to the full 2sQUIESCENCE_MSwait. Hermes and Codex TUIs redraw their status bars every ~1ms — clearing the quiescence timer on everyfeed()— so idle was never declared until the redraws stopped, adding ~13s to the detection latency on long sessions.Fix
Add a
READY_GRACE_MS = 500grace window that:readyPatternfirst matchesfeed()calls (TUI status-bar churn)markIdle()after 500ms — matchescompletionPattern's 500msSpurious spinners in the post-ready output are already blocked by the existing
!readySeenguard at the top offeed(); we don't re-check them here.Spike (vs. master)
❯❯+ 6s of redraws❯+ 11s of 1ms redraws22x speedup on the worst case.
Tests
Added 4 unit tests in
test/idle-detector.test.ts:feed()calls do NOT reset the timermarkIdle()fires after 500mscompletionPattern48/48 tests pass on
test/idle-detector.test.ts; full suite remains green.Scope
This PR is scoped to
IdleDetectoronly. The Hermes adapter's missingreadyPattern(which masks the same symptom for the Hermes CLI specifically) is fixed separately in PR #223.