Skip to content

fix(ui): fill the review stream on first paint - #922

Merged
benvinegar merged 2 commits into
mainfrom
fix/startup-blank-first-paint
Aug 30, 2026
Merged

benvinegar merged 2 commits into
mainfrom
fix/startup-blank-first-paint

Conversation

@benvinegar

Copy link
Copy Markdown
Member

Problem

On startup, a nowrap review could leave the lower part of the diff pane blank until the user scrolled. File windowing treated an unmeasured OpenTUI scrollbox height as 0, so only the leading file plus one overscan neighbor mounted. Short files then failed to fill a tall first frame.

Approach

  • Estimate the first-paint viewport from renderer height minus the pane's screen top.
  • Pass that estimate into file windowing while the measured height is still 0.
  • Re-read the scrollbox once after a frame in case yoga layout finishes without resized / layout-changed.

Non-goals: changing overscan, wrap-line windowing, or scroll/selection behavior after the viewport is measured.

Tests

  • bun run typecheck
  • bun run lint
  • bun test src/ui/lib/viewportTiming.test.ts src/ui/lib/fileRenderWindow.test.ts src/ui/components/panes/DiffPane.test.tsx src/ui/components/ui-components.test.tsx
  • bun test test/pty/layout.test.ts -t "the first nowrap frame fills a tall viewport"

Manual: reproduced on Linux in a Herdr pane (hunk diff, ~80 rows, nowrap split). Installed 0.20.1 showed two files then blank; source with this change filled the stream.

Platforms

Linux only for the manual check. PTY coverage is Unix-only.

File windowing treated an unmeasured scrollbox height as 0, so only the
leading file plus one overscan neighbor mounted until the user scrolled.
Use the estimated viewport height for that first paint and re-read once
after layout.
@vercel

vercel Bot commented Aug 30, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hunk-web Ignored Ignored Preview Aug 30, 2026 8:03pm

Request Review

@greptile-apps

greptile-apps Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fills the initial nowrap review stream by using an estimated viewport height until OpenTUI publishes measured geometry.

  • Extracts and tests viewport-height estimation and fallback helpers.
  • Adds a one-frame follow-up viewport read after initial layout.
  • Extends component and PTY coverage for tall viewports containing many short files.

Confidence Score: 4/5

The implementation appears safe to merge, with the non-blocking requirement that the new UI behavior be covered through the repository’s prescribed Storybook and Chromatic workflow.

The viewport fallback is limited to unmeasured startup geometry and is replaced by the measured height, while the only accepted concern is that one new UI test bypasses the required visual regression path.

Files Needing Attention: src/ui/components/ui-components.test.tsx

Important Files Changed

Filename Overview
src/ui/components/panes/DiffPane.tsx Uses an estimated viewport for initial nowrap file windowing and schedules a follow-up geometry read; no blocking correctness issue identified.
src/ui/lib/viewportTiming.ts Adds small, bounded helpers for estimating and resolving the render-only viewport height.
src/ui/components/ui-components.test.tsx Adds useful first-paint coverage through a traditional component test, contrary to the repository's prescribed Storybook and Chromatic UI test workflow.
test/pty/layout.test.ts Adds end-to-end PTY coverage demonstrating that the initial nowrap frame reaches later short files.
test/pty/harness.ts Adds a focused many-short-files fixture supporting the new PTY layout regression test.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[DiffPane first render] --> B{Measured viewport height available?}
    B -->|Yes| C[Use measured height]
    B -->|No| D[Estimate renderer height minus screen top]
    C --> E[Build file render window]
    D --> E
    D --> F[Read viewport again after one frame]
    F --> C
Loading
Prompt To Fix All With AI
### Issue 1
src/ui/components/ui-components.test.tsx:1361
**Traditional UI test bypasses Chromatic**

This adds DiffPane coverage through the traditional component-test harness instead of the required Storybook play test and Chromatic snapshot, leaving the visual first-paint behavior outside the repository's prescribed UI regression workflow.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(ui): fill the review stream on first..." | Re-trigger Greptile

Comment thread src/ui/components/ui-components.test.tsx
@benvinegar
benvinegar merged commit dfa9aa4 into main Aug 30, 2026
12 checks passed
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