Conversation
A pane rendering Hangul would garble mid-screen and stay that way until an unrelated window resize repaired it. The WebGL renderer caches rasterized glyphs in fixed-size atlas pages, and the GPU can bind only so many at once. ASCII fills one page; a CJK viewport needs hundreds of double-width glyphs keyed by character *and* color, so it hits the page limit and forces a merge that renumbers every cached glyph. WebglRenderer rebuilds the model when that happens, up to 32 times per frame — but each rebuild rasterizes again and can trip another merge, so a CJK viewport never converges. The loop then gives up and paints a model half-built against page indexes the last merge invalidated. Nothing marks those rows dirty again, so the garble persists until a resize forces a full rebuild. - Request another frame when the retry budget is exhausted, capped at 4 consecutive frames so an oversubscribed atlas cannot spin the render loop. - Raise the atlas page size to 1024px: 4x the glyphs per page, so a CJK viewport fits the texture budget and the merge path stays cold. Also add CJK entries to the terminal font chain. Every font in it was Latin-only, so Hangul fell to a proportional system face whose advance is not two cells wide and drifted out of the grid. Coding faces with exact dual-width metrics come first, then platform defaults native-before-foreign, each listed under both its English and localized family name — a CJK-locale OS registers these faces under the localized name only. The chain moves to lib/terminal-font-family.ts, which folds in the second, already-drifted copy the pane defaults carried.
Two changes to how Hangul/Kana/Han resolve in a terminal pane. The built-in chain now prefers the platform's own Korean face over a foreign one that merely happens to be installed. Neither Apple SD Gothic Neo nor Malgun Gothic has exact dual-width metrics, so there was no reason for a Windows font to outrank the macOS default — but with Malgun ahead, a Mac with Office installed rendered Hangul differently from one without. The chain is still code-only, though, and a machine with none of the dual-width coding faces installed had no way to reach a better one: the Font Family picker sets the primary font, which never answers for CJK. Add terminalCjkFontFamily, surfaced as a CJK Font Family row in the Terminal typography Advanced disclosure, reusing the installed-font autocomplete the primary picker uses. The chosen font sits behind the Nerd Fonts rather than at the front of the chain, so it answers for CJK without claiming the PUA glyphs Powerline prompts draw from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s real The font pickers showed a five-entry list that looks like the real thing but is the hardcoded fallback: system_profiler SPFontsDataType takes ~20s on a Mac with a large catalog (285 families here), and until it lands the renderer shows its curated placeholder with no sign that anything is still loading. Warm the main-process cache two seconds after the window shows, so the list is ready long before settings can be opened. Failures still fall back as before.
Owner
Author
|
CI validation complete: 45 checks, sole failure is the fork-irrelevant community-PR labeler (missing upstream secrets). Upstream PR stablyai#13779 is now ready for review. |
tonite31
pushed a commit
that referenced
this pull request
Aug 17, 2026
…vivor as the crasher (stablyai#14662) * fix(crash-reporting): keep a pre-gone process-metrics sample so the crashed process's working set survives its crash report * feat(crash-reporting): renderer peak/private bytes and gone-time system memory in crash details * fix(crash-reporting): macOS system-memory fields and an era-invariant pin for peak/private metrics * test(crash-reporting): kill five mutation survivors in the pre-gone sampler coverage Adversarial review found these mutations survived the suite: - dropping the immediate sample at startPreGoneProcessMetricsSampling() - removing the double-start idempotence guard - widening renderer peak/private aggregation to all buckets - a failed sweep erasing the previous good sample - the recorder hardcoding 'renderer' instead of event.processType Each now has a binding assertion; also documents that the crashed-process-absent flag is bucket-level only. * fix(crash-reporting): prove crasher absence by vanished pid, not bucket count alone The absent flag was bucket-level, so any surviving same-type process (a webview guest, the dashboard popout, another utility) silently cleared it — and webviewTag guests make multi-renderer sessions the norm. The pre-gone sample now keeps per-process pid/bucket/workingSet identities; a sampled same-bucket pid missing from the live set proves absence and reports the vanished process's own working set (processMetricsVanished*), so the crasher's size is no longer summed with surviving guests. Also: split gone-time system memory into its own module (max-lines), pin peak/private aggregation as a true max, clamp garbage negative working sets and backwards clocks, pin live-metrics precedence over incoming detail keys, and verify the sampler timer is unref'd by behavior. * fix(crash-reporting): bucket-aware vanished-pid check with consume-once attribution Loop-3 hardening of the vanished-pid logic: - Live pids now carry their bucket: a recycled pid living on as a different process type still reads as a vanished sampled process (the bare pid set misread the crasher as alive). - Vanished pids are attributed once. In a crash loop with no sweep between deaths, record #2 confidently inherited the FIRST crasher's pid and working set (dedupe window is only 2s, so both records ship); it now degrades to the honest bucket-count arm instead. - An ambiguous multi-process VanishedWorkingSetMB sum is bounded by VanishedLargestWorkingSetMB so no single-process reading of the sum survives triage. - Killer tests for the remaining mutation survivors: unreadable gone-time metrics prove nothing (flag/vanished stay off), pid-less sampled metrics never vanish, fractional-MB rounding, negative system-memory clamp, and full per-family precedence over colliding incoming detail keys. * fix(crash-reporting): flag consumed and blind-era vanished attribution instead of going silent Loop-4 hardening of the consume-once attribution: - processMetricsVanishedAlreadyReportedCount: a record whose vanished pids were consumed by a prior report now says so, instead of being indistinguishable from "nothing vanished" while its PreGone mirrors still show the prior crasher's era. - processMetricsVanishedAmbiguousWithEarlierCrash: consume-once only consumed when the gone-time read succeeded; a crash recorded blind (getAppMetrics threw) left its pid unconsumed, so the next record in the same era confidently emitted THAT crash's pid and working set as its own. Blind buckets now taint the era until a fresh sweep. - Pin two behaviors that were correct but unpinned: a failed sweep must not clear attribution state, and an ambiguous vanished pair is consumed too. - Document gone-time system memory reading healthier than at kill time, and the bound on the attribution set. - Split the suppressed-breadcrumb builder into its own module (max-lines). * fix(crash-reporting): extend vanished ambiguity to consumed eras and pin two unpinned behaviors Loop-5 findings: - processMetricsVanishedAmbiguousWithEarlierCrash fired only for the blind era; a partially-consumed era has the same shape (an earlier crash's unsampled respawn is as plausible a crasher as the newly vanished pid), yet emitted a confident VanishedPid with no flag. - Pin the > largest tie-break (first-enumerated wins) instead of re-accepting it as an equivalent mutant every loop. - The suppressed-breadcrumb type field had zero coverage after the module split — removing the whole block passed the suite. * refactor(crash-reporting): cut per-pid vanished attribution, keep the stateless absence proof Five review loops found defects in the same subsystem: the per-pid vanished attribution outputs (consume-once set, blind-era taint, consumed-era ambiguity). The absence proof they fed does not need any of it — a sampled same-bucket pid missing from the live enumeration (including cross-bucket pid recycle) is stateless and idempotent, so it stays true for every record of a crash loop with zero module-level attribution state. Dropped: processMetricsVanished{Count,WorkingSetMB,Pid, LargestWorkingSetMB,AlreadyReportedCount,AmbiguousWithEarlierCrash}, attributedVanishedPids, metricsBlindCrashBuckets, and the tests that existed only to defend them. Kept and still pinned: the 60s pre-gone sampler and its lifecycle, PreGone* mirrors + SampleAgeMs, renderer peak/private, gone-time system memory, the recorder processType binding, the live/PreGone era invariant, and the browser-pane case (webview guests keep the renderer bucket alive) that motivated the PR — now asserted via the absence flag plus PreGone mirrors alone. Documented the two honest limits: PreGone values are sample-time (up-to-60s understatement, bounded by AgeMs and lifetime peaks), and same-bucket pid recycle inside the sweep window is a false negative for the absence proof. * test(crash-reporting): pin PreGoneLargest to the crasher's own size, not the bucket's running sum Loop-6 mutation battery found one survivor in the cut's re-anchored suite: mutating Largest to carry the bucket's running sum survived every test, because no fixture put a same-bucket sibling BEFORE the largest process. That is the summing-bug family loop 2 found live. The webview-guest test now enumerates the guest first and asserts PreGoneLargest{Pid,Type,WorkingSetMB} carry the crasher's individual 4380, alongside the 4680 bucket total. Also restores the false-positive caveat the cut's comment dropped: a legitimately closed sampled process can trip the absence flag if the crasher's row somehow survives the live enumeration (pre-existing, unchanged by the cut). * fix(crash-reporting): mark pre-gone attribution ambiguous PreGone mirrors are whole-app snapshots, so a larger surviving Tab can own Largest and renderer-wide peak/private fields. Emit an explicit ambiguity boundary, prove the counterexample, and use Electron's pid plus creationTime identity to catch same-bucket PID reuse without adding stateful attribution.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fork-internal PR to run the check matrix before the upstream PR leaves draft. Not for merge.