Skip to content

perf(tui): lazily seed virtual history heights - #16523

Merged
kshitijk4poor merged 1 commit into
NousResearch:mainfrom
kshitijk4poor:fix/tui-lazy-height-seeding
Apr 27, 2026
Merged

kshitijk4poor merged 1 commit into
NousResearch:mainfrom
kshitijk4poor:fix/tui-lazy-height-seeding

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Contributor

Summary

  • stop rebuilding a full initialHeights Map for every transcript row on every useMainApp() render
  • lazily seed missing virtual-history heights inside useVirtualHistory() from an estimate callback
  • add focused tests for the new lazy-seeding helper

Problem

The TUI already has transcript virtualization, markdown caching, and measured-height caching, but useMainApp() was still doing one avoidable O(n) allocation-heavy step on every render:

  • build a fresh Map<string, number> covering every transcript row
  • look up cached heights or recompute estimatedMsgHeight() for misses
  • pass that freshly-built map straight into useVirtualHistory()

That work happened even though useVirtualHistory() already walks the full item list internally to build offsets. In long sessions, that meant each React render paid an extra full-transcript scan plus a large temporary Map allocation before the hook could do its own work.

Root cause

We were materializing the full initial-height snapshot in useMainApp() instead of letting the virtualization layer seed missing heights on demand.

Concretely, the old path did this on every render:

  • iterate virtualRows
  • allocate a new Map
  • fill every row entry from heightCache or estimatedMsgHeight(...)

The virtualization hook only needed:

  • a stable cache bucket for known heights
  • a way to estimate a height when an item is missing from that bucket

What changed

1. useMainApp()

Replaced the eager initialHeights useMemo() map build with:

  • the existing stable heightCache bucket passed directly as initialHeights
  • a new estimateRowHeight(index) callback passed as estimateHeight

2. useVirtualHistory()

Added ensureVirtualItemHeight() and used it when:

  • building offsets
  • computing viewport coverage / overscan coverage

That helper:

  • reuses cached heights when present
  • lazily seeds a missing height from estimateHeight(index, key)
  • falls back to the numeric default estimate when no callback is provided
  • normalizes values to at least 1 row

Why this is safe

This does not change the measured-height lifecycle:

  • mounted rows still replace estimates with actual Yoga-measured heights
  • onHeightsChange still syncs measured heights back into the per-session bucket
  • width-bucket invalidation still works the same way
  • the hook still owns offset construction and range coverage logic

This is a refactor of where missing estimates are materialized, not a behavior change to how row heights are ultimately measured.

Validation

Focused tests added

  • ui-tui/src/__tests__/useVirtualHistoryHeights.test.ts
    • reuses cached heights without invoking the estimator
    • lazily seeds missing heights from the estimator
    • falls back to the default estimate
    • clamps non-positive estimates to a minimum of 1 row

Full validation run

From ui-tui/:

npm test -- --run src/__tests__/useVirtualHistoryHeights.test.ts src/__tests__/virtualHistoryClamp.test.ts src/__tests__/virtualHeights.test.ts
npm run type-check
npm test
npm run build

Results:

  • targeted vitest suite: passed
  • npm run type-check: passed
  • full ui-tui vitest suite: 45 files / 370 tests passed
  • production build: passed

Synthetic microbenchmark

I also ran a small local Node microbenchmark to quantify the exact work removed: rebuilding a 10k-entry initialHeights map 200 times versus reusing the existing cache bucket.

Observed locally:

{"label":"old-build-initial-heights-map","ms":238.15,"heapDeltaKB":355,"lastSize":10000}
{"label":"new-pass-existing-cache-only","ms":0.01,"heapDeltaKB":0,"lastSize":6666}

This benchmark is synthetic and intentionally narrow: it measures only the removed eager map-build/allocation step, not whole-app frame time. But it does confirm that the deleted work was non-trivial and allocation-heavy.

Why this helps

For long transcripts, this removes a full-transcript allocation/scanning pass from the hot render path in useMainApp(). That should reduce:

  • React-side render overhead during transcript updates
  • temporary heap churn from large per-render Map allocations
  • avoidable repeated estimatedMsgHeight() work for rows that never needed eager materialization

Scope

Focused perf cleanup only:

  • no UX/behavior changes intended
  • no protocol changes
  • no Python/gateway changes
  • no markdown/rendering semantics changes

@alt-glitch alt-glitch added type/perf Performance improvement or optimization P3 Low — cosmetic, nice to have comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Apr 27, 2026
@kshitijk4poor
kshitijk4poor merged commit 98d75de into NousResearch:main Apr 27, 2026
4 of 5 checks passed
02356abc pushed a commit to 02356abc/hermes-agent that referenced this pull request May 14, 2026
dannyJ848 pushed a commit to dannyJ848/hermes-agent that referenced this pull request May 17, 2026
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
@kshitijk4poor
kshitijk4poor deleted the fix/tui-lazy-height-seeding branch August 5, 2026 07:08
prmartinow pushed a commit to prmartinow/hermes-agent that referenced this pull request Aug 26, 2026
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have type/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants