Skip to content

perf(ui): evaluate retaining the sidebar mounting budget - #1188

Draft
benvinegar wants to merge 1 commit into
mainfrom
fix/sidebar-follow-navigation-latency
Draft

benvinegar wants to merge 1 commit into
mainfrom
fix/sidebar-follow-navigation-latency

Conversation

@benvinegar

@benvinegar benvinegar commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Status: experiment not validated for shipping

Do not merge this as the navigation latency fix. Follow-up confidence testing recovers the cold test-renderer result but does not establish a compiled-CLI responsiveness benefit. This candidate should be withdrawn or reworked, rather than promoted on the benchmark result alone. Returned to draft after these tests.

Proposed policy

The candidate separates the sidebar's mounted-row allocation budget from its measured visible viewport, retaining a terminal-height minimum mounting budget. Actual pane height still governs visibility and selected-row reveal; other planner callers retain their existing default.

This preserves #1085's correctness changes, but correctness and boundedness do not by themselves justify the allocation policy. No protocol, original benchmark, threshold or accepted-regression setting changes.

What the original benchmark established

The unchanged benchmark, 16 sequential AB/BA samples per revision on Bun 1.4.2, recovered hunk_nav_press_p95_ms from 43.58 to 36.11 ms (−17.14%), using its existing nearest-rank aggregation.

#1085 publishes the measured sidebar height of 24 in a 28-row terminal, shrinking the initial mounted window from 32 entries to 28. The six timed selected files remain visible: traces show no viewport notifications, reveal retries or sidebar scrolls during those presses. Much measured time is in the deferred act flush. An accurate smaller initial estimate did not fully recover performance, and disabling Bun DFG/FTL eliminated the candidate's advantage. The exact lower-level mechanism remains unproven; startup row destruction alone is not an established root cause.

Follow-up confidence tests

Clean frozen worktrees: pre-#1085 08c3da70, main ab300399, candidate c8bfddad. Same dependencies and Bun 1.4.2. Each revision received 16 renderer runs and 16 compiled-binary PTY runs, sequentially with all revision-pair orderings balanced 8/8. No concurrent tests/builds and no excluded primary samples. Existing benchmark files remain unchanged.

The added renderer probe tests six cold presses, 100 warmup presses, adjacent visible files, 50-file forward/backward crossings, hunk/file navigation and 40-key batches. Its cold result reproduces the gap: pre/main/candidate 36.24 / 44.43 / 36.29 ms. Warmed inside-window hunk p95s are 19.63 / 19.10 / 18.87 ms; other warmed results are smaller and mixed.

The compiled artifacts use the repository's unmodified shipping build:bin path. They review a real 180-file Git diff in a 240×28 PTY. Timing starts before input delivery and ends at an observed native synchronized-output frame containing the requested header and code. No polling or idle/fixed-sleep delay is included in this interval. Held tests deliver keys independently at 30 Hz and retain every input's progress latency. Each app is awaited through exit before the next run.

Compiled CLI input-to-observed-frame results

Conventional median of 16 session p95s, ms. Positive change is slower. This aggregation differs from the original benchmark runner's lower-middle convention.

Phase Pre-#1085 Main Candidate Candidate vs main
Cold hunk 24.65 24.37 23.92 −1.8%
Warm hunk, inside window 15.69 15.51 16.31 +5.1%
Warm file, inside window 15.44 15.33 15.41 +0.5%
Warm hunk, forward crossing 21.22 22.04 22.75 +3.3%
Warm hunk, backward crossing 21.76 21.77 22.29 +2.4%
Warm file, forward crossing 22.77 22.60 24.09 +6.6%
Warm file, backward crossing 21.62 21.83 21.73 −0.5%
Held hunk forward, 30 Hz 16.46 14.46 16.76 +16.0%
Held hunk backward, 30 Hz 19.33 19.98 19.33 −3.3%
Held file forward, 30 Hz 14.47 13.87 15.18 +9.4%
Held file backward, 30 Hz 14.20 14.55 14.51 −0.3%
Hunk 40-key batch 37.82 38.55 39.31 +2.0%
File 40-key batch 41.23 43.54 41.24 −5.3%

These results do not establish a consistent product-level improvement. Most nominal paired-bootstrap intervals span zero. The approximately 17–23% cold-renderer gap does not reproduce as cold compiled-CLI first-paint latency in this fixture; that does not rule out costs in other workflows.

Pre-#1085 batches are not a correctness-parity comparator because its stale-bounds reveal can leave selection offscreen. Four additional strict sessions per main/candidate required the selected sidebar marker together with the target header and code. In all 2,736 press/batch requests, the first observed target-main frame already included the visible sidebar selection. Strict held sequences also crossed the window edge successfully. A separate framing audit found no combined or partial-next-frame chunks in 962 observed native frames. Measurements are PTY-output observations, not physical display scanout.

Work-count evidence

Separate temporary instrumentation, excluded from performance samples and removed afterward, counts React/sidebar/window/file-row renders, row identities/destruction, viewport updates, reveals/scrolls and native layouts.

  • Main deletes four offscreen file rows at startup; candidate retains them.
  • After warmup, main mounts 32 file rows and candidate mounts 36.
  • For 64 warmed inside-window hunk presses, both perform 192 sidebar renders, 64 window plans, 128 file-row renders and 64 reveal lookups. Both have zero viewport events, scrolls, row mounts or destructions, and 127 root layout calculations in the diagnostic.
  • For 50 forward crossing presses, both perform 177 sidebar renders, 77 window plans, 127 file-row renders, 27 viewport changes/scrolls and 27 row mounts/destructions.
  • For four 40-key hunk batches, both perform 16 sidebar renders, 8 window plans, 72 file-row renders, 4 scrolls and 68 mounts/destructions.

Thus the demonstrated change is startup retention and a permanently larger row set—not reduced steady navigation work. The retention regression test enforces that policy, not a product performance guarantee.

Correctness and scope

After restoring all instrumentation, fresh checks pass:

  • Sidebar/window unit tests: 22 pass.
  • Compiled-binary sidebar-follow PTY tests: 2 pass (held navigation, manual scroll, collapse/projection and resize).
  • bun run typecheck, git diff --check: pass; tracked worktrees clean.

Earlier full validation of this commit remains: 2,418 unit passes/3 skips; 263 integration passes plus the known comment-target failure reproduced on unchanged main; 11 TTY smoke passes; lint, dependency and format checks passed. Those full suites were not rerun during this follow-up. No assertions or skips were weakened. macOS/Windows were not tested locally.

Raw distributions, scripts, hashes, counter patches, bootstrap intervals and fresh logs are retained locally under /home/bentlegen/Projects/hunk-sidebar-fix/tmp/sidebar-navigation-confidence/; the detailed report is CONFIDENCE.md. Original investigation evidence remains under tmp/sidebar-navigation-regression/. These artifacts are ignored and uncommitted.

No follow-up source commit/push, merge, tag, installation or publication was performed. The release worktree remains clean on release/0.24.0-beta.1. Related release preparation: #1187.

This PR description was generated by Pi using GPT-6.1 (gpt-6.1-sol)

@vercel

vercel Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
hunk-web Ignored Ignored Preview Oct 14, 2026 12:45pm UTC

Request Review

@benvinegar
benvinegar marked this pull request as ready for review October 14, 2026 13:31
@greptile-apps

greptile-apps Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

The sidebar keeps its terminal-sized mounted window after measuring a smaller pane. Measured height still controls which rows are visible and when selection needs scrolling.

  • The bundled sidebar keeps its bounded row window after measuring the pane.

Acknowledged by benvinegar:

  • The lower-level performance mechanism is not fully established.
  • The comment-target test failure also occurs on unchanged main and is not attributed to this PR.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  T[Terminal height] --> B[Minimum mounted window]
  V[Measured pane height] --> M[Mounted window uses the larger height]
  B --> M
  M --> R[Mount rows with overscan]
  S[Selected file] --> R
  V --> F[Check selected-row visibility]
  F --> N[Scroll only when needed]
  R --> P[Spacers preserve full list height]
Loading

Reviews (1) · Last reviewed commit: "fix(ui): retain the bounded sidebar wind..." · Reviewed by Greptile

@benvinegar
benvinegar marked this pull request as draft October 14, 2026 14:35
@benvinegar benvinegar changed the title fix(ui): retain the bounded sidebar window after measurement perf(ui): evaluate retaining the sidebar mounting budget Oct 14, 2026

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant