Skip to content

fix(desktop): reduce long-history streaming work - #69151

Closed
frizikk wants to merge 1 commit into
NousResearch:mainfrom
frizikk:fix/desktop-stream-history-scaling
Closed

fix(desktop): reduce long-history streaming work#69151
frizikk wants to merge 1 commit into
NousResearch:mainfrom
frizikk:fix/desktop-stream-history-scaling

Conversation

@frizikk

@frizikk frizikk commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #69120.

Desktop stream rendering currently repeats several dominant transcript-sized scans for every incoming delta. This PR records single-message updates at the source, forwards validated pending-tail deltas without re-exporting/re-indexing the settled repository, keeps historical runtime messages reference-stable, and limits timeline geometry work to rendered prompts. Completion, authoritative replay, deletion, branch changes, and session changes still use the complete reconcile path.

The immutable source and visible-message arrays still require shallow reference copies; this PR targets the measured conversion, repository traversal, signature, and DOM-layout costs rather than replacing the Desktop transcript data structure.

It also adds a report-only stream-history performance scenario. History is mounted before recorders start; the harness verifies the source message count, waits for two paints, and applies a configurable settle period before measuring subsequent streaming.

Draft #68724 is complementary: it removes requestAnimationFrame from production delta flushing for hidden windows. This PR does not change that production scheduler. Its synthetic perf driver also uses timer-only flushing, so it does not reintroduce a hidden-window rAF dependency.

Related Issue

Fixes #69120

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • apps/desktop/src/app/chat/runtime-repository.ts
    • preserve the normalized ThreadMessage objects produced by the existing conversion cache instead of recreating the complete branch array;
    • forward strictly validated pending-tail updates as a single repository delta.
  • apps/desktop/src/app/session/hooks/use-message-stream/index.ts
    • update the active tail directly and attach predecessor provenance, avoiding a JavaScript callback over every settled message.
  • apps/desktop/src/lib/chat-messages.ts
    • preserve single-tail provenance through same-session local-error reconciliation; authoritative hydration still uses the complete merge path.
  • apps/desktop/src/lib/incremental-external-store-runtime.ts
    • apply validated tail deltas via direct repository lookup without exporting/indexing both complete repositories;
    • compare message identity and parentId on complete reconciles, avoid duplicate full exports, and reset the head only when necessary;
    • retain authoritative deletion and disjoint-session replacement semantics.
  • apps/desktop/src/components/assistant-ui/thread/timeline.tsx
    • replace one DOM selector per historical timeline entry with one bounded rendered-node scan;
    • use binary geometry lookup for the active rendered prompt;
    • retain the previous user-prompt signature when a runtime notification changes only the assistant tail.
  • apps/desktop/src/components/assistant-ui/thread/list.tsx
    • retain unchanged grouping metadata for text-only assistant-tail growth.
  • apps/desktop/scripts/perf/scenarios/stream-history.mjs
    • add a report-only long-history stream scenario with source-count verification, a configurable settle delay, and deterministic completion waiting.
  • Regression tests cover pending-tail provenance and direct delta application, completion fallback, reference stability, authoritative deletion, session replacement, branch-parent changes, headId, and logarithmically bounded timeline geometry reads.

How to Test

  1. Run the Desktop correctness/build gate:
cd apps/desktop
NODE_OPTIONS='--max-old-space-size=8192 --no-webstorage' LANG=C.UTF-8 LC_ALL=C.UTF-8 npm run check
  1. Run lint:
cd apps/desktop
NODE_OPTIONS='--max-old-space-size=8192 --no-webstorage' LANG=C.UTF-8 LC_ALL=C.UTF-8 npm run lint
  1. Run the production history benchmark:
cd apps/desktop
npm run perf -- stream-history --spawn --prod --runs 5 --historyTurns 200
npm run perf -- stream-history --spawn --prod --runs 5 --historyTurns 1000

Median of five runs on the same Linux/Wayland machine:

Preloaded history Metric Before After Change
400 messages frame p95 54.9 ms 32.4 ms -41.0%
400 messages frame p99 61.9 ms 37.7 ms -39.1%
400 messages frames >33 ms 338 27 -92.0%
400 messages max long task 387 ms 133 ms -65.6%
2,000 messages frame p95 209.0 ms 91.9 ms -56.0%
2,000 messages frame p99 234.9 ms 108.1 ms -54.0%
2,000 messages frames >33 ms 481 390 -18.9%
2,000 messages max long task 591 ms 226 ms -61.8%

Empty-history control remained within run-to-run noise: frame p95 22.4 ms before and 23.2 ms after, with one frame over 33 ms in each sample.

The benchmark was recorded around base a2c2ec63322b791d6d1a7a024640064d96e7f0ae. The branch was subsequently rebased onto current main (91546b8337068891cc0a6b834d89d0d9270fb3ec) to resolve the PR's merge conflict. The only rebase conflict was the import list in apps/desktop/src/lib/chat-messages.test.ts; it preserves both current-main's collectUnspokenTurnSpeech import and this PR's getChatMessageListUpdate import. The published rebased head is 854075304e540c0605e0b762b5d0ac4e5a4ff985.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass — N/A: this PR changes only Desktop TypeScript/React and its performance harness; the complete Desktop gate below passed
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Linux 7.1.4-1-cachyos, Wayland, Node.js 25.4.0

Documentation & Housekeeping

  • I've updated relevant documentation — the Desktop perf README documents stream-history
  • cli-config.yaml.example update is N/A — no config keys changed
  • CONTRIBUTING.md / AGENTS.md update is N/A — no contributor workflow changed
  • I've considered cross-platform impact — production changes use browser/TypeScript APIs; the Windows checker found no applicable scanned files; perf results are explicitly Linux/Wayland and are not compared with the committed macOS ARM baseline
  • Tool descriptions/schemas update is N/A — no Hermes tool behavior changed

Screenshots / Logs

Initial local verification on the pre-rebase base 3e953ed815ffb1e35277a77eb3e764d39dcf36f7:

  • npm run check — PASS (typecheck, complete Desktop Vitest suite, production build, and Linux package)
  • npm run lint — PASS with 9 existing warnings outside the changed files and 0 errors
  • python scripts/check-windows-footguns.py --diff origin/main — no applicable files (0 scanned)
  • git diff --check — PASS
  • focused regressions for production provenance publication, repository identity, bounded provenance lifetime, stale-delta fallback, incremental reconcile, and timeline DOM bounds — PASS (56 tests)

Post-rebase validation on 91546b8337068891cc0a6b834d89d0d9270fb3ec:

  • focused Desktop Vitest: the four changed test files — PASS (56 tests)
  • npm --workspace apps/desktop run typecheck — PASS
  • ESLint on every changed .ts/.tsx file — PASS
  • npm --workspace apps/desktop run build — PASS (CSS and bundle-size warnings only)
  • git diff --check — PASS

No UI screenshot is included because the change intentionally preserves rendering and interaction output; the reproducible artifact is the report-only performance scenario and its JSON metrics.

@frizikk frizikk closed this Jul 22, 2026
@frizikk frizikk reopened this Jul 22, 2026
@alt-glitch alt-glitch added type/perf Performance improvement or optimization 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
Reduce dominant transcript-sized conversion, repository, signature, and DOM scans while preserving authoritative fallback semantics.\n\nAdd a repeatable preloaded-history streaming benchmark and focused regression coverage for tail provenance, stale-delta fallback, repository identity, and bounded timeline geometry.
@OutThisLife

Copy link
Copy Markdown
Collaborator

Superseded by #71835.

Your diagnosis was right, and it's what the fix is built on — the three costs you identified (timeline geometry, discarded conversion identity, whole-repository reconcile per delta) all reproduced on main. I measured them directly against the @assistant-ui internals: fromBranchableArray preserved reference identity for 0 of 500 messages, and a single-token delta produced 500 addOrUpdateMessage calls plus 2 full exports.

Two things moved since you opened this. The timeline half was overtaken by #71789 and #71780, which idle the rail entirely on hidden panes and skip the rect walk while following the bottom. That left the repository half, and it turned out not to need the provenance-token protocol: the identity cache in useRuntimeMessageRepository is already sufficient once fromBranchableArray stops discarding it. Normalizing on the cache miss makes settled turns reference-stable, and the reconcile can then use a plain identity check to write only what moved — no Symbol-keyed delta channel, no changes to chat-messages.ts or the perf driver.

You're credited as co-author on both commits. Thanks for the writeup and the profiles — the CPU attribution is what made this quick to confirm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Desktop streaming cost grows with transcript length

3 participants