Synced from GitHub
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
fix(web): erase terminal cursor fringes between blinks#11424
What changed
Snap terminal row boundaries to device pixels and use those same boundaries for the cursor, backgrounds, selection, and text clipping. Adjacent rows share an edge, so repainting the cursor row does not touch its neighbors.
The production change is confined to the shared web/desktop renderer. Blink timing, product defaults, and the terminal protocol stay unchanged. Mobile uses its separate native renderer.
Why
At fractional display scales, repainting an antialiased row boundary blends over the cursor edge instead of fully erasing it. This leaves faint horizontal lines during the blink's off phase. Pixel-aligned boundaries fix that without repainting the entire terminal on every blink.
Closes #11402.
mermaid
| 1 | flowchart LR |
| 2 | A[Cursor on] --> B[Blink off] |
| 3 | B --> C[Repaint cursor row] |
| 4 | C --> D[Shared pixel edges erase the cursor completely] |
UI changes
Before/after Chromium reproduction using the actual renderer from b1e223e2b0 and this branch, with identical input. The PNGs are captured at 2x density; the inset enlarges the original canvas pixels 8x. These are isolated renderer captures, not full-app screenshots.
Short before/after video. Full comparison and integrated-terminal evidence.
Verification
- 66 focused tests pass in
renderer.test.tsandsurface.test.ts. The new geometry tests fail at 125% and 150% on the base renderer. - Web package typecheck and targeted lint/format checks pass.
- All 97 real-canvas comparisons pass after the fix, covering four cursor styles, light/dark themes, text, selections, adjacent rows, four display scales, and a fractional CSS origin.
- Tested the full app in Chromium with isolated state and synthetic shell output. At 125%, the live cursor leaves no residual pixels, including after scrollback appears.
- Scoped Fallow audit passes with one thread and no introduced findings. Existing findings were left untouched.
Checklist
- This PR is small and focused
- I explained what changed and why
- I included before/after screenshots for any UI changes
- I included a video for animation/interaction changes
Model: GPT-5. Harness: OpenAI Codex.
Summary by CodeRabbit
Bug Fixes
- Improved terminal rendering across different display scales by aligning row boundaries to device pixels.
- Corrected row heights for backgrounds, selections, text, decorations, and cursor rendering to maintain consistent vertical alignment.
- Limited cursor updates to the affected row when appropriate, improving repaint behavior.
Tests
- Added coverage for pixel alignment, contiguous row boundaries, and targeted cursor repainting across multiple display scales.
Checks
- SuccessfulCodeRabbitGitHub
Review completed
- Successful
Skipped - Not affected
- Skipped
- Skipped
- Skipped
- SuccessfulRemove preview downloadGitHub
- SuccessfulCheckGitHub
- SuccessfulTransfer report artifactGitHub
- Skipped
- SkippedEAS PreviewGitHub
- Skipped
- SuccessfulLabel PR sizeGitHub
- SuccessfulLabel PR 11424GitHub
- SuccessfulPrepare PR size configGitHub
- SuccessfulCollect PR targetsGitHub
- SkippedDeploy web previewGitHub
- SuccessfulTest Server 6GitHub
- SuccessfulMobile Native ChangesGitHub
- SuccessfulTest Server 3GitHub
- SuccessfulTypecheckGitHub
- SuccessfulTest WebGitHub
- SuccessfulTestGitHub
- SuccessfulTest Server 5GitHub
- SuccessfulBuildGitHub
- SuccessfulTest Server 2GitHub
- SuccessfulLintGitHub
- SuccessfulTest Server 4GitHub
- SuccessfulTest Server 1GitHub
- SuccessfulRelease SmokeGitHub
- SuccessfulRustGitHub
- SuccessfulPOST to CursorGitHub
- SuccessfulNative fingerprint diffGitHub
- Skipped
- Skipped
- Skipped
- SuccessfulRemove preview downloadGitHub
- SkippedBuild preview JS bundleGitHub
- Successful
- Skipped
- SkippedDeploy web previewGitHub
- SkippedEAS PreviewGitHub
- Skipped
- Skipped
- SkippedDeploy web previewGitHub
- Skipped
- SuccessfulLabel PR sizeGitHub
- SuccessfulLabel PR 11424GitHub
- Skipped
- SkippedRemove preview downloadGitHub
- Skipped
- SkippedEAS PreviewGitHub
- SuccessfulTest Server 1GitHub
- SuccessfulTest Server 2GitHub
- SuccessfulRustGitHub
- SuccessfulTest Server 3GitHub
- SuccessfulRelease SmokeGitHub
- SuccessfulCheckGitHub
- SuccessfulMobile Native ChangesGitHub
- SuccessfulTestGitHub
- SuccessfulCollect PR targetsGitHub
- SuccessfulPrepare PR size configGitHub
- Skipped[code]smithGitHub
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/u/199175422?s=80)
Synced from GitHub
Codex Review Summary
This comment shows the latest Codex review activity on this pull request.
| Review | Status | Commit | Review trigger |
|---|---|---|---|
| 📝 Code Review | ✅ Completed 2026-09-12T13:41:13.315318Z | 48311fb | PR opened |
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review" or "@codex security review".
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.
Synced from GitHub
Synced from GitHub
Approvability
Verdict: Approved at 48311fb
Macroscope's review found this PR approvable — This is a small, self-contained terminal rendering bug fix that aligns row and cursor edges to device pixels to prevent blink artifacts. Production impact is confined to existing canvas rendering, with targeted fractional-scale and cursor repaint tests added.
No code changes detected at ff04249. Prior analysis still applies.
You can add or adjust custom eligibility rules. Learn more.
Synced from GitHub
Review Change StackReview Change Stack
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Advanced
Run ID: dd4dd2e5-85bb-415f-bf81-5eae2da592c5
📥 Commits
Reviewing files that changed from the base of the PR and between b1e223e2b0d87124883b1410ab52dd6a1338e40d and 48311fb4d5fdc7196bda9538f951176b6493de0b.
📒 Files selected for processing (3)
apps/web/src/terminal/ghostty/renderer.test.tsapps/web/src/terminal/ghostty/renderer.tsapps/web/src/terminal/ghostty/surface.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📝 Walkthrough
Walkthrough
Ghostty terminal rendering now snaps row boundaries to device pixels. Backgrounds, text clipping, decorations, and cursors use the computed row geometry. Tests cover multiple display scales and canvas transform mocks.
Changes
Terminal rendering
| Layer / File(s) | Summary |
|---|---|
Transform-aware row geometry and rendering apps/web/src/terminal/ghostty/renderer.ts, apps/web/src/terminal/ghostty/renderer.test.ts, apps/web/src/terminal/ghostty/surface.test.ts | renderGhosttySnapshot snaps row boundaries using the canvas transform. Row backgrounds, selection, text clipping, decorations, and cursors use the computed row height. Tests verify pixel alignment, contiguous rows, repaint behavior, and getTransform() mocks. |
Priority: ⬇️ Low
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix · Severity of issue fixed: Low
Suggested reviewers: stienswout
Merge Risk: ⚪ Minimal · up to ff042
The terminal rendering change has focused coverage for pixel alignment and blink repaint behavior, with no actionable current-head risk identified.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly describes the primary fix: removing residual terminal cursor edges during blinking. |
| Description check | ✅ Passed | The description includes the required change summary, rationale, UI evidence, verification details, and completed checklist items. It is focused and aligned with the template. |
| Linked Issues check | ✅ Passed | Issue #11402 requires the cursor to disappear during the blink-off phase without residual cell edges, while preserving the background and text. renderer.ts snaps shared row boundaries with the canva… |
| Out of Scope Changes check | ✅ Passed | The changes stay within Issue #11402. They modify the shared Ghostty web renderer, add focused renderer coverage, and update canvas test doubles with getTransform(). They do not change blink timing,… |
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
Comment @coderabbitai help to get the list of available commands.
Synced from GitHub
Independently validated 48311fb4d5fdc7196bda9538f951176b6493de0b in Chromium using the actual renderer from that commit and base b1e223e2b0d87124883b1410ab52dd6a1338e40d.
- The 125% scale reproduction leaves 26 residual pixels on the base and zero on this PR after the cursor blinks off.
- 192 canvas comparisons passed across DPRs 1, 1.25, 1.5, and 2; integer and fractional row origins; light/dark, glyph, colored background, selection, and strikethrough cases; and all four cursor styles. Each case runs three blink cycles followed by hiding the cursor with a changed row, then verifies the final canvas exactly matches its original cursor-off pixels.
Base and PR renderer comparison at 125% scale
Blink recording · Evidence and test details
These are isolated renderer captures, not a separate Electron application test.
Model: GPT-6. Harness: Codex.
Added 1 commit
- fix(web): erase terminal cursor fringes between blinksff04249
Note
Written by claude-opus-5-5 on behalf of Maria
Hi! We are cleaning up open PRs, and this one appears to have been created with an older model (gpt-5). If this change is really important, we recommend rebuilding the PR with a newer model if possible.
Note
Written by claude-opus-5-5 on behalf of Maria
Reopening, this was closed by mistake. Sorry for the noise!
Note
Written by claude-opus-5-5 on behalf of Maria
Closing again after a second look, sorry for the back and forth. This PR was opened more than two weeks ago. If this change is still important, please rebuild it on current main with a newer model and note the model in the PR description.