💭 I'm checking FontFamilyPicker.tsx, which now uses a toast to reject non-monospace fonts, and I want to verify the toast API usage is consistent with the shared toast module.

1. browse_code — apps/web/src/components/settings/FontFamilyPicker.tsx

2. browse_code — apps/web/src/components/ui/toast*

💭 I want to check toast.tsx to see how stackedThreadToast and toastManager are exported and used.

3. git_tools — stackedThreadToast

💭 I'm considering whether showing all fonts but rejecting non-monospace picks via toast could create a UI consistency issue, since the Combobox's displayed value stays tied to the previously selected family when a rejected pick occurs. That said, this is more of a product/UX design decision than a strict UI consistency violation, so I want to verify the underlying font-filtering logic itself—checking cssFontFamilies and measureMonospaceFamilies in appearanceFonts.ts to confirm the refactor behaves correctly.

4. browse_code — apps/web/src/appearanceFonts.ts

💭 I should check other places isMonospaceFamily is used, particularly whether the plain-input fallback for font family also needs monospace validation now that the picker previously handled filtering.

5. git_tools — requireMonospace|isMonospaceFamily|FontFamilyPicker

6. browse_code — apps/web/src/components/settings/SettingsPanels.tsx

💭 I'm checking how FontFamilySettingsRow handles invalid values in the plain input path, which uses inline validation to reject non-monospace families. The new picker path shows a toast instead, so I want to see if that's an inconsistency with the established pattern.

7. browse_code — apps/web/src/components/settings/SettingsPanels.tsx

💭 I'm weighing whether the toast-only rejection feedback in the picker path versus inline aria-invalid feedback in the plain-input path counts as a real inconsistency—leaning toward no, since toasts are used broadly and the picker control can't hold an invalid draft anyway. I'm now looking more closely at a potential accessibility issue in the Combobox behavior when a non-matching value is entered. There's a bigger concern though: since the picker no longer filters options up front, every family row displays identically regardless of whether it's actually valid, giving no visual cue about which choices will be rejected—so users only find out after selecting and seeing a toast. That's a real affordance regression compared to the previous approach of not offering invalid options at all, though I recognize the whole point of removing upfront filtering was to avoid the freezing issue, so there's a real tension to work through here. I'm also double-checking whether isMonospaceFamily's conversion from a function declaration to a const arrow function introduces any hoisting problems in appearanceFonts.ts, but since it's only referenced at its definition point, that seems fine. I'm now thinking about whether the caching mechanism could misfire for dynamically loaded web fonts — if a probe runs before a bundled font finishes loading via document.fonts, it might measure the fallback and incorrectly cache the family as monospace, which would persist even after the real font loads.

8. git_tools — @font-face|new FontFace|document.fonts

💭 I want to check surface.ts around lines 90-130 to see how isMonospaceFamily interacts with font loading.

9. browse_code — apps/web/src/terminal/ghostty/surface.ts

💭 I'm realizing that loadTerminalFontFamily loads fonts first and then re-resolves through terminalFontFamily/isMonospaceFamily specifically because probing before loading gives a false verdict — the sequence exists to validate the real font face only after it's available. With the new cache, if something calls terminalFontFamily before the load completes, that timing could produce a stale or incorrect result. I want to check where both functions are actually called to see whether this pre-load-then-post-load ordering can really happen in practice.

10. git_tools — terminalFontFamily|loadTerminalFontFamily

11. browse_code — apps/web/src/terminal/ghostty/surface.ts

💭 I'm realizing that caching the monospace verdict permanently defeats the purpose of onFontsLoaded re-checking after fallback: since the cache stores the original (fallback) result, a font that loads later and is actually monospace-incompatible would incorrectly still return the stale cached verdict instead of being re-measured. I'm also considering whether the symbols Nerd Font addition matters here — it carries no text glyphs so it shouldn't affect metrics — but the core issue is that the caching change conflicts with surface.ts's explicit contract to revalidate fixed-width status on loadingdone, since the docstring's justification about installed fonts being session-stable doesn't apply to terminal fonts loaded dynamically at runtime.

12. git_tools — font-face|fontsource|.woff

💭 I'm considering whether the loadingdone revalidation actually matters here, since only the symbols font is dynamically loaded. There's a documented scenario where a font face becomes available after an earlier fallback measurement, which could apply to local fonts in Electron when using document.fonts.load() with a specific location. Even so, this cache turns surface.ts's onFontsLoaded guard into a no-op, and since surface.ts is a call site of isMonospaceFamily and falls within the included path patterns, it's technically in scope. But since surface.ts isn't part of the diff itself, I can't leave an inline comment there since comments need to land on changed lines. I'm now weighing a separate concern: FontFamilyPicker shows all installed families with no signal about which pass the monospace check, and rejection surfaces as a global toast — while the plain-input fallback for the same setting communicates rejection inline via aria-invalid. That's a real divergence in how the two paths of the same control report the same failure, though accessibility is roughly preserved since the toast is likely announced. There's also a more concrete bug I'm noticing: handlePick closes the combobox before checking the selection, and if rejected it just returns without calling onSelect, which could leave the Base UI Combobox in an inconsistent state. I also confirm the toast copy stays consistent stylistically with other toasts in the app, and I check whether createCachedFamilyProbe and its caching behavior for null/empty families lines up with the test expectations — it does. Now I'm looking at whether measureMonospaceFamilies receiving the normalized family list changes behavior compared to how isMonospaceFamily previously handled it. Turning to the performance angle, filtering all families once per memo used to run inline, and the PR claims removing that filtering avoids blocking the renderer, which lines up with existing toast conventions elsewhere in the repo for surfacing failures. One thing worth noting is that the picker row now renders proportional fonts using their own font-family style, so the list visually mixes monospace and non-monospace previews. I want to check keyboard behavior too — pressing Enter on a non-monospace entry with autoHighlight active closes the popup and triggers the error toast, with focus presumably falling back to the trigger, which seems acceptable. The more significant issue is how the caching interacts with the documented revalidation logic in surface.ts, so I want to check the surface.test.ts test around line 463 that covers loadTerminalFontFamily with a proportional font — it uses a custom environment with its own load/resolve functions, so it likely doesn't exercise the isMonospaceFamily cache path, but I should look at the surrounding tests to see how measurement gets mocked and whether anything could break.

13. browse_code — apps/web/src/terminal/ghostty/surface.test.ts

💭 I see the problem now: the cache defeats the post-load revalidation in surface.ts, since the first font probe happens before the actual face is loaded, measuring against the fallback font and returning true. That verdict then gets cached and frozen, so the later validation against the real loaded face never has a chance to override it. This is a legitimate regression for the "UI Consistency" check since it produces a visible runtime bug in terminal rendering—ragged glyph advances and a stranded cursor. The right fix is to avoid caching when the measurement fell back to a font that isn't actually available yet: either skip caching negative/unresolved verdicts, or invalidate the cache on the loadingdone event from document.fonts. For the picker no longer marking non-monospace rows, I'm inclined not to flag this as a failure—it looks like a deliberate tradeoff toward post-pick rejection for performance reasons, and the resulting UI behavior (toast on rejection) still reads as coherent, so I'll keep this review focused on the concrete caching bug. Checking a few more spots: the code row's description mentioning "terminal" is a minor copy nit not worth failing over, and the toast helpers (stackedThreadToast, toastManager.add) are used consistently across settings and thread contexts, so no inconsistency there either. I'm leaning toward flagging the cache-freezing-the-guard issue as a genuine regression since it breaks a documented contract, so I want to trace exactly when isMonospaceFamily gets called relative to font loading — specifically how loadTerminalFontFamily awaits the font variants before calling terminalFontFamily(family). The onFontsLoaded path calls terminalFontFamily later, on a subsequent font-loading event, and the surrounding comment suggests this deliberately re-checks a face that may have become available after the earlier fallback measurement, anticipating a stale "monospace" verdict — so the design already accounts for this scenario rather than being an oversight. Since isMonospaceFamily is cached, that re-run just returns the same stale answer, so the guard can never self-correct — a real regression regardless of how rarely it triggers. The custom family probed doesn't include the later-added symbols font, which is fine, but the same caching bug also leaks into SettingsPanels's inline validation, where a family typed before its face loads gets permanently marked monospace. That's the one finding worth flagging. Now I need to confirm appearanceFonts.ts is in scope for review — it is. I'm figuring out exactly which lines in the diff to anchor the comment to, since multi-line review comments need both endpoints to fall within the changed hunk; I'll target the line where the verdict gets cached, using a single-line comment to avoid range issues. For the fix itself, I'm considering only caching the verdict when the font family is actually available, since isFontFamilyAvailable already exists earlier in the same file and can be reused for that check without adding new dependencies.

14. git_tools — function cssFontFamilies

💭 Since cssFontFamilies is idempotent for already-quoted names, isFontFamilyAvailable works fine as-is. I'm drafting a diff that only caches the verdict when the font is genuinely resolvable, rather than caching fallback results, so a later correction can still happen after fonts load. I'm weighing the extra cost of calling isFontFamilyAvailable on each uncached probe, but it's negligible relative to the existing overhead. For the picker's use case, families come from queryLocalFonts and are already installed, so they'll be marked available and cached correctly—and locally-installed fonts measured as "available" do have final metrics, so this holds. I want to phrase this as a suggestion, noting the alternative of clearing the cache on document.fonts loadingdone, and I need to double check this doesn't break the existing test that expects both verdicts to be cached, since that test supplies its own custom probe. Actually, tracing through that scenario more carefully, I realize gating cache writes on isFontFamilyAvailable inside the generic helper would break jsdom tests, since canvas getContext returns null there and the probe would never cache. So the fix needs to live in the concrete isMonospaceFamily wrapper, not the generic createCachedFamilyProbe helper, to keep the helper injectable for tests. A cleaner alternative: keep the cache as-is, but register a one-time listener on document.fonts' loadingdone event that clears all probe caches—tracking each cache in a shared set so they can all be reset together. That preserves the performance benefit within a session while still revalidating once real fonts finish loading. I'll propose this direction. Now I'm identifying the core issue: caching every monospace verdict for the page's lifetime defeats the post-load re-validation that surface.ts depends on. Since measureMonospaceFamilies returns true when a face is absent (measuring the generic monospace fallback instead), the first optimistic probe becomes permanently frozen, so onFontsLoaded's re-check can never catch a proportional face once its real metrics load — leaving the terminal stuck with the ragged grid this guard was built to prevent. I want to suggest only caching verdicts that were measured against an actually loaded face, not a fallback. I'm mapping out exactly which line to flag in the PR comment — pinpointing the verdicts.set call that caches the probe result, with the comment anchored at that assignment and the range starting a line above where the cache lookup begins.

15. modify_pr — 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.