Skip to content

Fix #2210: coalesce portal sync to latest geometry - #2214

Merged
austinywang merged 1 commit into
mainfrom
issue-2210-sidebar-resize-fix
Mar 30, 2026
Merged

austinywang merged 1 commit into
mainfrom
issue-2210-sidebar-resize-fix

Conversation

@austinywang

@austinywang austinywang commented Mar 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • coalesce external terminal portal geometry sync to the latest request instead of letting the first pending request win
  • avoid sidebar toggle ancestor/frame churn resizing Ghostty at stale intermediate widths, which could trigger extra prompt redraw/reflow passes

Testing

  • built and launched a tagged Debug app with CMUX_SKIP_ZIG_BUILD=1 ./scripts/reload.sh --tag sidebar-resize-fix --launch
  • did not run local tests per repo policy
  • behavioral verification is ready in the launched tagged app

Demo Video

For UI or behavior changes, include a short demo video (GitHub upload, Loom, or other direct link).

  • Video URL or attachment: N/A

Review Trigger (Copy/Paste as PR comment)

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

Checklist

  • I tested the change locally
  • I added or updated tests for behavior changes
  • I updated docs/changelog if needed
  • I requested bot reviews after my latest commit (copy/paste block above or equivalent)
  • All code review bot comments are resolved
  • All human review comments are resolved

Summary by CodeRabbit

  • Bug Fixes
    • Improved window geometry synchronization reliability by implementing request prioritization that discards stale operations, ensuring windows respond correctly to geometry changes without conflicting updates.

Summary by cubic

Coalesces terminal portal geometry syncs so the latest size wins. Fixes #2210 by preventing sidebar toggles from applying stale widths and cutting extra PTY redraws.

  • Bug Fixes
    • Added generation counters to drop outdated requests in scheduleExternalGeometrySynchronize and scheduleExternalGeometrySynchronizeForAllWindows.
    • Coalesced async sync scheduling across windows and respected live-resize/drag state to avoid intermediate layout churn.

Written for commit 9087fc5. Summary will update on new commits.

@vercel

vercel Bot commented Mar 26, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Mar 26, 2026 10:16pm

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@coderabbitai

coderabbitai Bot commented Mar 26, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8ba7c3b3-a259-4fcc-b737-3ba5b69d9fb5

📥 Commits

Reviewing files that changed from the base of the PR and between ccaebbd and 9087fc5.

📒 Files selected for processing (1)
  • Sources/TerminalWindowPortal.swift

📝 Walkthrough

Walkthrough

Introduced per-portal and global "latest-request-wins" generation-based coalescing for external geometry synchronization. Added generation counters to WindowTerminalPortal and TerminalWindowPortalRegistry that increment on each sync scheduling call. Queued async callbacks now check if their generation is stale, rescheduling instead of executing outdated syncs.

Changes

Cohort / File(s) Summary
External Geometry Synchronization Coalescing
Sources/TerminalWindowPortal.swift
Added externalGeometrySyncGeneration counter to WindowTerminalPortal and externalGeometrySyncForAllWindowsGeneration to TerminalWindowPortalRegistry. Queued performSync callbacks now check generation validity before execution; stale generations are discarded and rescheduled rather than executed. Implements latest-request-wins semantics for geometry sync operations.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant Portal as WindowTerminalPortal
    participant AsyncQueue as Async Queue
    participant SyncOp as Sync Operation

    Caller->>Portal: scheduleExternalGeometrySynchronize() [Gen=1]
    Portal->>Portal: externalGeometrySyncGeneration = 1
    Portal->>AsyncQueue: Queue performSync(Gen=1)
    
    Caller->>Portal: scheduleExternalGeometrySynchronize() [Gen=2]
    Portal->>Portal: externalGeometrySyncGeneration = 2
    Portal->>AsyncQueue: Queue performSync(Gen=2)
    
    AsyncQueue->>Portal: Execute performSync(Gen=1)
    Portal->>Portal: Check: Gen=1 < current Gen=2?
    rect rgba(255, 100, 100, 0.5)
        Portal->>Portal: STALE - clear flag
        Portal->>AsyncQueue: Reschedule with Gen=2
    end
    
    AsyncQueue->>Portal: Execute performSync(Gen=2)
    Portal->>Portal: Check: Gen=2 == current Gen=2?
    rect rgba(100, 200, 100, 0.5)
        Portal->>SyncOp: Execute synchronizeAllEntries()
        SyncOp->>SyncOp: Sync geometry
    end
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly Related PRs

Poem

🐰 A generation check so clever and spry,
Latest requests win—the stale ones say bye!
Coalescing with grace, both portal and all,
Geometry syncs now heed the rightmost call! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix #2210: coalesce portal sync to latest geometry' clearly summarizes the main change—implementing latest-request-wins coalescing for external geometry synchronization in the portal system.
Description check ✅ Passed The pull request description covers the required Summary and Testing sections with specific implementation details and testing approach, though the Demo Video and Review Trigger sections are incomplete or partially addressed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-2210-sidebar-resize-fix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes issue #2210 by introducing a generation counter to implement "latest-request-wins" coalescing for external terminal portal geometry syncs. Previously, when multiple rapid geometry change events fired (e.g., during sidebar toggle), the first pending async request would win — meaning the PTY could be resized to a stale intermediate width. The fix applies the same generation-counter pattern to both the per-portal scheduleExternalGeometrySynchronize() and the registry-wide scheduleExternalGeometrySynchronizeForAllWindows().\n\nKey changes:\n- Adds externalGeometrySyncGeneration: UInt64 (instance) and externalGeometrySyncForAllWindowsGeneration: UInt64 (static) counters, incremented with wrapping arithmetic (&+=) on every call.\n- Each scheduleExternalGeometrySync* call captures the generation value at dispatch time; when the async fires, it compares the captured generation against the current one.\n- On mismatch, the in-flight request clears its pending flag and re-schedules itself with fresh state (current isDragEvent / requiresSettledLayout), effectively ensuring only the latest geometry snapshot reaches synchronizeAllEntriesFromExternalGeometryChange.\n- Prevents Ghostty from receiving multiple PTY resize events at transient intermediate widths during sidebar animation.

Confidence Score: 5/5

This PR is safe to merge — it is a focused, correct coalescing fix with no regressions risk.

The change is minimal (two symmetrical generation-counter additions, one per coalescing site), all code runs exclusively on the main queue so there are no concurrency concerns, wrapping arithmetic prevents any UInt64 overflow edge case, and the re-schedule path correctly re-evaluates isDragEvent / requiresSettledLayout with fresh state. The logic mirrors a well-understood generation stamp pattern used in AppKit coalescing. No public APIs change, and the author verified behavior in a tagged Debug build.

No files require special attention.

Important Files Changed

Filename Overview
Sources/TerminalWindowPortal.swift Adds generation-counter coalescing to both the per-portal and registry-wide geometry sync schedulers; logic is correct, thread-safe (main queue only), and uses wrapping arithmetic to handle UInt64 rollover safely.

Reviews (1): Last reviewed commit: "Fix #2210: coalesce portal sync to lates..." | Re-trigger Greptile

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

@austinywang
austinywang merged commit d95158e into main Mar 30, 2026
16 checks passed

This branch was successfully deployed

1 active deployment
Preview — 9087fc52 Deployed Mar 26, 2026 by vercel[bot]
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.

Toggling sidebar (Cmd+B) corrupts terminal prompt — ghost prompts stack on right side

1 participant