Skip to content

fix(desktop): keep long tool sessions responsive (rebased #68503) - #69275

Closed
iwillwill-ALLWILL wants to merge 4 commits into
NousResearch:mainfrom
iwillwill-ALLWILL:fix/long-tool-session-responsive-rebased
Closed

fix(desktop): keep long tool sessions responsive (rebased #68503)#69275
iwillwill-ALLWILL wants to merge 4 commits into
NousResearch:mainfrom
iwillwill-ALLWILL:fix/long-tool-session-responsive-rebased

Conversation

@iwillwill-ALLWILL

@iwillwill-ALLWILL iwillwill-ALLWILL commented Jul 22, 2026

Copy link
Copy Markdown

Summary

Rebased and hardened continuation of #68503 by @frizikk, whose branch became non-mergeable after #69019 changed the same thread-list virtualization code.

This keeps Hermes Desktop responsive when opening long, tool-heavy sessions without deleting, truncating, or mutating transcript data.

The first three commits preserve Jakub Wolniewicz's original authorship. The final commit contains review-driven hardening and regression tests from @iwillwill-ALLWILL.

Supersedes #68503.

Fixes #68467.

Root cause and approach

A representative transcript (266 messages, 143 tool calls, 650,451 content characters) could synchronously mount and highlight enough history to block the renderer for seconds even while the backend was idle.

The change:

  • mounts at most 20 complete semantic history groups initially and progressively reveals older groups;
  • enforces one turn-wide 20-tool budget across interleaved narration and assistant messages;
  • preserves oldest-visible identities, scroll position, and keyboard focus while revealing history;
  • discovers preview artifacts from authoritative transcript state without mounting hidden heavy tool rows;
  • renders complete plain/copyable code immediately, then viewport-gates Shiki enhancement in a shared lazy Web Worker;
  • bounds worker retry, cancellation, late-reply handling, generation retirement, and final lease cleanup;
  • preserves fix(desktop): stop long-session transcript from drifting to old turns #69019's live-tail content-visibility behavior while resolving the original PR's conflicts.

Additional hardening after independent review

A second review found two correctness bugs in the original candidate. Both now have fail-before/fix-after regression coverage:

  1. A lifetime preview-target Set could suppress a same-path artifact after restore/edit cleared the preview store and the regenerated timeline produced that target again. Registration now tracks bounded per-session tool-occurrence keys plus a clear generation, and batches all newly observed targets into one idempotent atom update. This preserves dismiss semantics and prevents historical-target replay churn.
  2. Shiki tokens from old code/language props could survive into one committed frame before a passive effect cleared them. Highlight state now carries its source identity and is synchronously ignored unless it matches current props.

The hardening commit also keeps ref mutation out of React state updater callbacks and adds coverage for:

  • session-key changes while history is expanded;
  • tool-window boundaries at 0, 1, 19, 20, 21, 40, and 41 calls;
  • stable oldest-visible tool identity after append;
  • removed saved pagination keys;
  • restore-and-reproduce preview targets;
  • one-write reconciliation for 101 historical targets and sequential eviction semantics;
  • dismissed previews remaining dismissed when unrelated targets append;
  • stale highlighter props at commit time.

Production Electron A/B

Fresh minified production renderers were compared on the same macOS arm64 machine, with the same 266-turn synthetic mixed-Markdown transcript and three runs per side.

Metric current main this branch delta
transcript mount 144.0 ms 122.7 ms -14.8%
total renderer long-task time 251 ms 72 ms -71.3%
max renderer long task 176 ms 72 ms -59.1%
keystroke p50 9.5 ms 2.7 ms -71.6%
keystroke p95 16.6 ms 16.2 ms -2.4%
keystroke p99 17.5 ms 17.5 ms 0.0%
keystrokes over 16 ms 12 7 -41.7%

The final exact-HEAD run also passed the repository's committed performance gate. The large keystroke improvement is treated as host variance rather than claimed as a feature gain; the merge-safety signal is that no same-machine metric regressed materially, while transcript long-task time remained substantially lower.

A separate production Electron probe loaded a real fenced TypeScript block and observed 25 styled token spans after deferred highlighting, proving that the emitted worker asset loaded and returned tokens under the production renderer/CSP path.

Validation

Run after rebasing onto current origin/main:

npm --workspace apps/desktop run typecheck
npm --workspace apps/desktop run lint
npm --workspace apps/desktop test
npm --workspace apps/desktop run build
git diff --check origin/main...HEAD

Results:

  • typecheck: passed;
  • ESLint: 0 errors (9 pre-existing warnings outside this diff);
  • Full Desktop suite: 284 files (283 passed, 1 skipped); 2,550 tests passed, 3 skipped;
  • focused changed suites: 63/63 passed;
  • production renderer/Electron build: passed, including the emitted lazy Shiki worker;
  • git diff --check: passed;
  • current-main RED proof: all 7 representative long-session assertions fail without the implementation;
  • every review-blocker regression test was observed failing before its fix and passing afterward.

Scope and attribution

  • 20 files, all under apps/desktop/src;
  • no dependencies, lockfiles, generated output, logs, credentials, fixtures containing private data, or backend/Python changes;
  • original three commits remain authored by Jakub Wolniewicz (@frizikk);
  • the replacement exists because fix(desktop): keep long tool sessions responsive #68503 is DIRTY against current main; it is not presented as independently authored work.

Type of change

  • Bug fix
  • Tests
  • New feature
  • Security fix
  • Documentation/config change

Checklist

  • Read the contributing guide
  • Conventional commit messages
  • Searched existing issues/PRs and explicitly linked the superseded PR
  • Desktop-only scope with no unrelated changes
  • Regression tests added
  • Typecheck, lint, complete Desktop test suite, production build, and diff check passed
  • Tested on macOS arm64 in a production Electron renderer
  • Considered Worker/IntersectionObserver fallback behavior for cross-platform compatibility

frizikk and others added 4 commits July 22, 2026 19:20
Normalize transient Worker constructor failures into the generation retry contract while preserving the original cause. Cover bounded shared recovery and plain fallback, and apply the repository formatter to the eight reviewed files.
Reconcile preview artifacts by tool occurrence and batch additions into one atom update so timeline restore can re-register targets without resurrecting dismissals or replaying historical status writes.

Prevent stale Shiki tokens from reaching a commit after code props change, release obsolete highlight state, and keep pagination ref writes outside React state updater callbacks.

Add regression coverage for restore/reproduce, bounded preview writes, dismiss semantics, session pagination reset, exact tool-page boundaries, and stale highlighter props.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 22, 2026
egilewski added a commit to egilewski/hermes-agent that referenced this pull request Jul 23, 2026
The macOS freeze report lost its original traces and cannot be reproduced on
the Linux development host, leaving Electron-wide hangs and transcript
rendering stalls indistinguishable.

Add a test-only macOS arm64 Electron harness that drives five real local
mock-backed chat exchanges, exercises renderer-heavy synthetic transcripts,
and probes composer, Settings, IPC, event-loop, and process responsiveness.
Run an exact counterbalanced A/B against the long-session candidate while
preserving logs, profiles, screenshots, and failure samples. The harness is
injected only into ephemeral diagnostic builds and does not change the
shipping renderer.

Refs NousResearch#63047
Related NousResearch#69275
egilewski added a commit to egilewski/hermes-agent that referenced this pull request Jul 23, 2026
The short-session hang harness needs native Apple-silicon execution, but the
fork has no provider configuration and GitHub Actions is disabled.

Add a CircleCI M4 Pro job with pinned Node and checksum-verified uv tooling,
run the exact baseline/candidate diagnostic, and retain artifacts on failure.
Limit the workflow to the diagnostic branch and ignore tags so enabling the
project cannot start this job on unrelated branches.

Refs NousResearch#63047
Related NousResearch#69275
egilewski added a commit to egilewski/hermes-agent that referenced this pull request Jul 23, 2026
The macOS hang diagnostic clicked submit immediately after CDP inserted text,
before the composer's animation-frame-coalesced state flush enabled the send
control. Native CircleCI runs therefore failed before the first exchange and
misclassified harness startup as a reproduced freeze.

Poll the real submit control within the existing five-second exchange budget
before clicking it. A stalled renderer still fails closed at the same freeze
threshold, while normal next-frame propagation no longer creates a false
positive.

Refs NousResearch#63047
Related NousResearch#69275
egilewski added a commit to egilewski/hermes-agent that referenced this pull request Jul 23, 2026
The first native CircleCI runs showed that the diagnostic submitted before
the composer's derived button state settled, treated every harness exception
as a product freeze, and uploaded the isolated Hermes home with its artifacts.

Drive real chat submission through the contenteditable Enter path, distinguish
five-second renderer/lifecycle failures from harness errors, and invalidate
comparisons when setup or fixture contracts fail. Keep per-run sandboxes
outside the artifact tree with guaranteed cleanup, and share semantic summary
validation between both providers with focused regression coverage.

Refs NousResearch#63047
Related NousResearch#69275
egilewski added a commit to egilewski/hermes-agent that referenced this pull request Jul 23, 2026
Native macOS evidence showed both refs timing out before exchange one while
the main and renderer processes remained idle. The CDP Enter packet omitted
Chromium's return text fields, and a condition that stayed false was treated
as an unresponsive renderer.

Match Playwright's Enter packet, split exchange operations, require mock
request dispatch, and classify only stalled CDP evaluations or lifecycle
signals as reproductions. Responsive unmet chat and Settings conditions now
invalidate the diagnostic.

Refs NousResearch#63047
Related NousResearch#69275
egilewski added a commit to egilewski/hermes-agent that referenced this pull request Jul 23, 2026
The macOS ARM64 runner left both target processes idle while every run waited
forever for a synthetic keyDown command. Use Chromium's rawKeyDown packet,
matching the repository's existing Electron diagnostic, and assert the exact
Enter event sequence.

Refs NousResearch#63047
Related NousResearch#69275
egilewski added a commit to egilewski/hermes-agent that referenced this pull request Jul 23, 2026
CircleCI's Electron target leaves CDP keyboard dispatches unacknowledged even
while both app processes remain idle. Activate the enabled submit control in
the real composer instead, preserving the production submit handler, mock
request, transcript, and Settings responsiveness checks.

Refs NousResearch#63047
Related NousResearch#69275
egilewski added a commit to egilewski/hermes-agent that referenced this pull request Jul 23, 2026
Return the CDP evaluation before activating the real submit control. The
following mock-request wait and renderer probe can then distinguish a working
submission from a renderer that freezes inside its submit handler.

Refs NousResearch#63047
Related NousResearch#69275
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the substantial investigation, realistic regression coverage, and preservation of the earlier contributors' attribution.

Automated hermes-sweeper review found that the renderer-responsiveness protection is already implemented on current main:

  • apps/desktop/src/components/assistant-ui/thread/list.tsx:45-57 bounds the synchronous initial paint to 20 parts; :241-280 restores retained history through an interruptible transition and keeps the newest weighted groups mounted first.
  • apps/desktop/src/components/assistant-ui/thread/list.tsx:425-445 applies content-visibility:auto to older visible turns while preserving the live tail.
  • The linked issue Desktop: long tool-heavy sessions freeze composer (typing/send dead) while backend is idle #68467 was closed as implemented on main, and the maintainer-associated review on fix(desktop): keep long tool sessions responsive #68503 likewise concluded that its alternate pagination/worker implementation was no longer needed for the reported root failure.
  • Main subsequently retained unconditional retained-history backfill in 7f87b672455c, because suppressing it during streaming broke the real large-session resume invariant.

Closing as implemented on main. This is an automated hermes-sweeper review.

@teknium1 teknium1 closed this Jul 30, 2026
@teknium1 teknium1 added sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/sessions Session lifecycle, resume, persistence, history labels Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Desktop: long tool-heavy sessions freeze composer (typing/send dead) while backend is idle

4 participants