emrg: fix GUI message display bugs — empty-state overlay, typing cursor residue, false from-other-client label - #543
Conversation
…or residue, false from-other-client label Rant 14:11 (host-verified screenshots) reported three GUI display bugs after the first message in a session: 1. Welcome screen (#empty-state) stayed visible on top of the message area — updateEmptyState() was only evaluated on session switch/new, never after append/clear. Fix: Chat.append()/clear() call App.updateEmptyState?.() so welcome visibility always follows chat content. 2. Typing cursor (▍) never disappeared + own replies mislabeled "from other client" — G122 16ms delta batching: done/error/cancelled bypass the buffer, so buffered deltas flushed after done reached the renderer, which then created an orphan node with ownStreamRequestId already null (false label) and no done ever arriving (cursor forever). Fix (main.js): flush deltaBuf synchronously before terminal events (webContents.send preserves order). Defense (chat.js): doneRids set drops stale deltas; cancelled/error clear typing on in-flight nodes. +3 renderer smoke regression tests; docs synced (npm test 88 -> 91).
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (independent verification on ba8b59a)
Checked out the branch and verified each fix against the rant's root-cause analysis:
- main.js flush ordering:
flushDeltaBuf()runs synchronously before done/error/cancelled are forwarded; the pending 16ms timer is cleared first, andwebContents.sendpreserves order on the channel — buffered deltas can no longer arrive after the terminal event. The extracted named function is also reused by the timer path (no behavior change there). - chat.js doneRids defense: stale deltas for finished streams are dropped before node creation (no orphan nodes, no false from-other-client label). UUID requestIds are never reused, and the 500-entry cap bounds long-running growth.
clear()resets the set on session switch. - chat.js welcome screen:
append()/clear()now callApp.updateEmptyState?.(), so welcome visibility follows chat content on every add/remove — not just session switch. - app.js cancelled/error:
clearTyping()on in-flight nodes is correct — the daemon's session-levelcancelledevent carries no request_id, and mid-round cancellation emits no done frame, so clearing all in-flight typing cursors is the only sound option.
Verification: npm test 91/91 on the branch (incl. the 3 new regression tests), Python 508 passed (doc-count guard satisfied, README/Agent.md/README.cn.md synced 88→91), CI run 31153709355 green.
Minor non-blocking observation: the disconnected path doesn't flush deltaBuf — a daemon death mid-stream could still deliver one buffered batch post-reconnect. Pre-existing edge, partially mitigated by the doneRids/groupNodes guards; can be addressed separately if it ever surfaces.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (independent verification on ba8b59a)
Checked out the branch and verified each fix against the rant root-cause analysis:
- main.js flush ordering: flushDeltaBuf() runs synchronously before done/error/cancelled are forwarded; the pending 16ms timer is cleared first, and webContents.send preserves order on the channel — buffered deltas can no longer arrive after the terminal event. The extracted named function is reused by the timer path with no behavior change.
- chat.js doneRids defense: stale deltas for finished streams are dropped before node creation (no orphan nodes, no false from-other-client label). UUID requestIds are never reused; the 500-entry cap bounds long-running growth; clear() resets the set on session switch.
- chat.js welcome screen: append()/clear() now call App.updateEmptyState?.(), so welcome visibility follows chat content on every add/remove — not just session switch.
- app.js cancelled/error: clearTyping() on in-flight nodes is correct — the daemon session-level cancelled event carries no request_id, and mid-round cancellation emits no done frame, so clearing all in-flight typing cursors is the only sound option.
Verification: npm test 91/91 on the branch (incl. the 3 new regression tests), Python 508 passed (doc-count guard satisfied, README/Agent.md/README.cn.md synced 88→91), CI run 31153709355 green.
Minor non-blocking observation: the disconnected path does not flush deltaBuf — a daemon death mid-stream could still deliver one buffered batch post-reconnect. Pre-existing edge, partially mitigated by the doneRids/groupNodes guards; can be addressed separately if it ever surfaces.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 2 (independent re-verification, different angle: test quality + edge cases)
Branch unchanged since last review (ba8b59a, +96/-13). This pass scrutinized the three new regression tests rather than the fix code:
- Welcome-screen test asserts hidden=true after addUserMessage — valid: only the new append() hook can produce this state. (Mock limitation noted: the sandbox classList.toggle ignores the force argument, so an inverted force usage would not be caught here — consistent with the existing suite's mock fidelity, non-blocking.)
- Stale-delta test calls Chat.handleDone directly then replays a delta — correctly isolates the renderer-side doneRids guard independent of the main.js flush; without doneRids the replayed delta would create an orphan node and fail the count assertion.
- Cancelled test drives the real App.handleEvent path (not Chat.clearTyping directly) — verifies the full event wiring, and the querySelector-null mock fallback keeps the assertion meaningful.
Re-ran on the branch: npm test 91/91 (all 3 rant-14:11 tests green), Python 508 passed. No new commits since the previous cycle's review; CI 31153709355 remains green.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 3 (independent re-verification, third angle: regression-surface audit)
Branch unchanged (ba8b59a). This pass audited every call site of the modified functions for regressions:
- append() has 4 callers (user message / assistant node / system message / tool row) — all are chat content, so hiding the welcome screen on any of them is correct.
- Chat.clear() has 4 callers (boot / rewind / switchSession / newSession); switchSession and newSession already called updateEmptyState() explicitly — the new hook makes those double calls, which is idempotent and harmless.
- clearTyping() is wired exactly at cancelled and stream-error; the session-busy error branch correctly does NOT call it (no in-flight nodes exist on that path).
- doneRids lifecycle is sound: add on done, check on delta, clear on session switch, 500-entry cap for long-running sessions.
Final verification on the branch: npm test 91/91, Python 508 passed, import OK, CI 31153709355 green. Three independent cycle reviews complete (fix correctness / test discriminating power / regression surface) — merge gate satisfied.
… entry (#544) Co-authored-by: EMRG Evolution <emrg@argszero.dev>
…p tolerance (#552) Version bump 0.2.10 → 0.2.11 across all 6 version sources (pyproject.toml / emrg/__init__.py / gui/package.json / uv.lock / make-installer.sh / build-runtime.sh). Release for Windows verification: - #541 LLM gzip body tolerance - #543 GUI message display fixes (#544 quick-ref) - #545 Windows GCM silent-fail Stage 1 - #546 Windows TUI CJK input + legacy arrow keys + /rant visibility - #548/#549/#550 GitHub auth in GUI (PAT + device flow + banner, Stage 2) - #551 quick-ref All 548 tests green. Co-authored-by: EMRG Evolution <emrg@argszero.dev>
Fixes three GUI message display bugs (host-verified screenshots, rant 14:11) that appear after sending the first message in a session:
Bug A — welcome screen never hides
updateEmptyState()was only evaluated on session switch / new session; the streaming path (Chat.addUserMessage/ delta / done) appended nodes without re-evaluating it, so#empty-statestayed overlaid on the message area.Fix:
Chat.append()andChat.clear()now callApp.updateEmptyState?.()— welcome visibility always follows chat content.Bug B — typing cursor residue + false "from other client" label (same race)
G122 batches
message_deltaon a 16ms timer, butdonebypasses the buffer. When the final deltas were still buffered,donereached the renderer first:groupNodesentry deleted,ownStreamRequestIdcleared — then the 16ms flush delivered a stale delta that created an orphan node withisOwn=false(false label) that never receivesdone(cursor forever).Fix:
flushDeltaBuf()runs synchronously beforedone/error/cancelledevents (webContents.sendpreserves order → deltas can never arrive after the terminal event)doneRidsset drops any residual stale delta (defense in depth, UUID requestIds never reused, capped at 500)cancelled/ stream-errorclear the typing class on in-flight nodes (mid-round cancel emits no done frame, only the session-levelcancelledevent)Verification
npm test91/91 green (was 88 + 3 new); Python 508 passed (incl. doc-count guard); docs synced