Repository navigation
Collapse spinner terminal titles before ingress dedup - #9098
Conversation
Co-authored-by: Maxx Yung <maxxyung1@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds a public Braille spinner title normalization filter, integrates it into ChangesTerminal title churn
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 25✅ Passed checks (25 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review |
@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,631 of the 240,000 allowed lines of code this month. Reviews resume on 1 August 2026 (in 4 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedReview finished.
|
|
To use Codex here, create a Codex account and connect to github. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Closes #6291
Issue: #6291
Summary
GhosttyTitleUpdateIngressboundary before its existing duplicate check.TerminalTitleChurnFilterinCmuxTerminalCore; no new timers, observers, or downstream copies of the normalization rule.Root cause and current architecture
The issue was filed against the former direct
GHOSTTY_ACTION_SET_TITLE→NotificationCenterpath. Currentmainhas since added a per-viewGhosttyTitleUpdateIngress, newest-valueAsyncStream, and 50 ms per-surface dispatcher coalescing, so realistic spinner rates no longer reproduce the original total freeze.One structural gap remains: each spinner frame is a different raw string (
⠋ pnpm install,⠙ pnpm install, ...), so raw duplicate rejection never fires. Every frame still pays normalization-independentAsyncStream.yieldand executor scheduling before the dispatcher can coalesce it. Canonicalizing at the synchronous ingress makes the existing invariant semantic: equivalent terminal titles enqueue at most once per attachment.The downstream
TabManagerand toolbar paths are intentionally unchanged.Workspace.updatePanelTitlealready avoids identical mutations, and current titlebar observers already scope raw refreshes to the selected workspace. Duplicating normalization there would create multiple owners while retaining the hot-path cost.Adapted from the current-main port in #9094 and the original filter work in #6907; Maxx Yung is credited as co-author on the fix commit.
Reproduction
Quoted from #6291:
pnpm install # or: npm install, yarn install, cargo build (with progress), etc.Expected: sidebar clicks register immediately while the spinner is active.
Testing
Two-commit red/green structure:
test: expose spinner title ingress churnadds a behavioral regression to the already-wiredTypingHotPathRegressionTests.swift. Onmain, all ten commonpnpmspinner frames return as newly enqueued; the test requires only the first equivalent frame to enqueue while a genuinely different label still does.fix: collapse spinner titles before ingress dedupadds the normalizer, source integration, and pure package coverage.Completed locally:
arch -arm64e swift test --package-path Packages/macOS/CmuxTerminalCore --filter TerminalTitleChurnFilterTests— 4/4 passed../scripts/lint-pbxproj-test-wiring.sh— checked 617 files, passed.python3 scripts/check-package-resolved-policy.py— passed.python3 scripts/check-workspace-package-groups.py --check— passed.git diff --check— passed.The focused package tests cover the ten reported spinner frames, the full U+2800–U+28FF Braille Pattern range, multiple glyphs, leading whitespace, spinner-only titles, exact ordinary-title preservation, and non-leading Braille preservation. CI covers the app-host integration test; per issue instructions, no local
xcodebuild testor XCUITest was run.The historical
.github/swift-file-length-budget.tsvandscripts/swift_file_length_budget.pywere removed from currentmainin #8125. This PR changes neither budget TSV. New Swift files are 40 and 35 lines; the touched existing app-host test file remains under 500 lines at 491.Localization
No app-visible, CLI, settings, or web copy changed. The only documentation change is the developer-facing package README entry and test construction example, so no localization catalog keys are required. Changed Swift/Markdown files were audited for user-facing literals.
Demo Video
Cloud-mac dev-build dogfood and a verified full-screen recording will follow after required CI is green.
Review Trigger (Copy/Paste as PR comment)
Checklist
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes UI stalls when terminals emit animated spinner titles by collapsing leading Braille spinner prefixes before ingress deduplication. Spinner frames like "⠋ pnpm install" now enqueue once per label, while ordinary and non-spinner Braille titles stay unchanged.
CmuxTerminalCoreto strip known leading Braille spinner glyphs, trim whitespace, and drop spinner-only frames; preserves ordinary and non-spinner Braille titles byte-for-byte.GhosttyTitleUpdateIngress.submitbefore the duplicate check and AsyncStream enqueue to reject equivalent spinner frames early.CmuxTerminalCoreREADME with a short usage snippet.Written for commit 9fa57a4. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Documentation