fix(web): validate monospace fonts on pick instead of scanning the catalog - #7494
eggfriedrice24 wants to merge 9 commits into
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: pingdotgg/t3code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe font utilities now cache resolved family probe results. ChangesFont validation flow
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant FontFamilyPicker
participant isMonospaceFamily
participant Canvas
participant toastManager
FontFamilyPicker->>isMonospaceFamily: validate selected family
isMonospaceFamily->>Canvas: measure glyph advances
Canvas-->>isMonospaceFamily: return measurements
isMonospaceFamily-->>FontFamilyPicker: return verdict
FontFamilyPicker->>toastManager: show error for proportional family
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The picker now shows all installed font families while validating monospace compatibility only when selected, preserving the current font for invalid choices. No actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation The implementation matches the main coding objectives in
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR changes the existing code and terminal font-selection workflow: the full catalog is now shown, picks trigger synchronous validation and toast rejection, and cached verdicts are shared with typed input and terminal rechecks. Although the catalog-wide freeze is addressed and default font values remain unchanged, the cross-surface runtime and user-facing behavior warrants human review. No code changes detected at You can add or adjust custom eligibility rules. Learn more. |
…space-pick-validation
There was a problem hiding this comment.
One finding: the new page-lifetime verdict cache in appearanceFonts.ts silently disables the post-load monospace re-validation that terminal/ghostty/surface.ts depends on. Details inline. The FontFamilyPicker changes themselves (full catalog + bounded probe on pick, stackedThreadToast for the rejection) match the existing toast and combobox conventions.
Posted via Macroscope — UI Consistency
|
Hi @juliusmarminge @maria-rcks, apologies for tagging you directly, I know you have a lot on your plates. Flagging this because it is actively disrupting our team: accidentally opening the Appearance settings freezes the app and forces a full close and reopen. This PR fixes that by validating monospace fonts on pick instead of scanning the whole catalog. Branch is freshly updated with main and checks are green. A review whenever you can spare a moment would mean a lot. |
…space-pick-validation # Conflicts: # apps/web/src/appearanceFonts.test.ts
|
Rebased on main after the conflict from #8738 (test import block only). Addressed the Macroscope finding on the verdict cache in b4b2fea. CI green, approved by @alex73630, ready to merge. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8a57cd2. Configure here.
Dismissing prior approval to re-evaluate 40b0c40

What Changed
isMonospaceFamilyverdicts (pass and fail) are cached per normalized family list, shared by the picker, the typed input fallback, and the terminal surface, so remounts (toggling Advanced typography, reopening Appearance) never re-probe.Why
Opening Settings > Appearance in the desktop app silently grants local font access and enumerates every installed family; each monospace picker then ran up to 32 synchronous canvas
measureTextcalls per family during render. That loop scales with the installed catalog: with 710 families the Electron renderer froze for over five minutes, and fontconfig catalogs in the thousands fare worse. Batching the scan (#5642) still froze Electron in its author's own testing, because the total work is unchanged. Validating only the picked family bounds every interaction to at most one probe, which #5656 lists as the smallest viable approach. Builds on the diagnosis by @alex73630 in #5642 and supersedes its batching approach.Fixes #5656
Verification:
vp test run --project unit src/appearanceFonts.test.ts: 18 passed, 3 new for the verdict cache; targeted typecheck, lint, and format checks.--font-mono, a proportional pick (DejaVu Sans) is rejected with the toast and the previous font is kept.UI Changes
The monospace pickers now list all installed families, and a rejected proportional pick shows an error toast ("X" isn't monospace / Code and terminal need a fixed-width font, so the current font was kept). Happy to add screenshots on request.
Checklist
Model: Claude Fable 5. Harness: Claude Code in T3 Code.
🤖 Generated with Claude Code
Note
Low Risk
Performance and validation-timing change in appearance settings only; monospace enforcement remains on pick/commit with fail-open canvas behavior unchanged.
Overview
Fixes Appearance settings freezing when large font catalogs are installed by stopping upfront monospace classification of every enumerated family in code/terminal pickers. Those pickers now list the full installed catalog (like the interface picker) and run one cached canvas probe only when the user picks a family.
Adds
createCachedFamilyProbesoisMonospaceFamilyshares normalized family-list verdicts across the picker, typed-input validation, and terminal surfaces. Verdicts are stored only after every name in the stack resolves viafamiliesResolve, so late-loading faces can be re-probed instead of locking in a fallback measurement.When
requireMonospaceis on, a proportional pick does not commit—an error toast explains the rejection and the current font is kept; Default still commits without probing.Reviewed by Cursor Bugbot for commit 40b0c40. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Validate monospace fonts on pick in
FontFamilyPickerinstead of scanning catalogcreateCachedFamilyProbeandfamiliesResolveto appearanceFonts.ts to cache probe results by normalized family list, deferring the cache until all families resolve.requireMonospaceis true shows an error toast and rejects the selection.Macroscope summarized 40b0c40.
Summary by CodeRabbit
New Features
Bug Fixes