Repository navigation
🤖 fix: keep Geist Mono's 1.5.1 vertical metrics so the phone static view stays the LCP - #7638
Conversation
…iew stays the phone LCP
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
🟢 e2e stories (Haiku): 14 passed22 agent steps · 7 replayed from cache · 15 model calls · 46.3k tokens (24% cached) All 14 tests in 9 files
e2e 0.20.0 · 1m 10s · web · run artifacts 🟢 e2e Electron stories (Haiku): 3 passed3 agent steps · 3 model calls · 7.9k tokens (22% cached) All 3 tests in 3 files
e2e 0.20.0 · 18.7s · electron · run artifacts 🟢 e2e agent repros (Haiku): no tests selectedSpend: $0.00 of the $1.00 cap (proxy ledger) e2e 0.20.0 · 5.3s · 2 targets (web, phone) · run artifacts |
|
VM A/B rule for this PR (perf owner, workspace 8e18f4aada), declared before any data. Design: same-session VM A/B, 6+6 seeded runs per page and device (ABBA order), base = main GOOD (merge) only if all hold:
BAD (no merge): desktop LCP head - base > +40 ms, or mobile TBT head - base > +50 ms with the same long task growing, or mobile LCP median > 2,258.2 ms. Anything in between: one more 3+3 block, then judge on all runs. Separately from the A/B: the 31 pending Pixel snapshots need a person to confirm that only Geist Mono text (code, inline code, terminal) moved back to its pre-#7625 line box. Generated with |
|
Readiness record for head
Decision: ready with a tracked follow-up (Pixel snapshot review by a maintainer). Generated with |
Fixes #7637
Summary
geist1.8.1 (from #7625) made Geist Mono's text box taller: ascent/descent went from 955/245 to 1005/295 units, so a 12 px code line's text box grew from 14 px to 16 px. The phone static workspace view paints the seeded code line before Geist Mono loads (fallback monospace, 14 px tall). The live app's code line now paints larger than that copy, so Chrome records a second, later LCP entry and Lighthouse simulates LCP at the end of the JS load. This PR addsascent-override/descent-override/line-gap-overrideto the Geist Mono@font-face, which restores 1.5.1's vertical metrics and keeps the bump.Background
/workspace/<id>the early script paints the transcript tail before the app loads. Geist Mono is deliberately not preloaded (🤖 perf: compress the SPA page and preload its first-view fonts in xum server (T3 C step 1) #6216, comment instaticAssets.ts), so code in the static view paints in the fallback monospace.unitsPerEm1000,fsSelection.USE_TYPO_METRICSset in both):The ascent minus descent is the same (710), so the overrides move no glyph: the baseline in a fixed line-height stays where it is. They only shrink the inline text box back to 1.2 em, which also restores the pre-bump height of inline-code backgrounds and of
line-height: normalmonospace text (for example the terminal default font).Bisect
Check: seed like the auditor (
seed.mjs, 4 text rows), runxum serverwithenv -i,XUM_DISABLE_TELEMETRY=1, a tempXUM_ROOTand a free port, then open the workspace page in headless Chromium (Playwright; 412x823, DPR 1.75,isMobile,hasTouch, a Moto G Power UA, service workers blocked, a fresh browser per run). It records everylargest-contentful-paintentry. PASS = no entry outside the static view after the static paint.Reverting
geistalone flips the check, so I did not bisect the Tailwind pair or the other packages.Validation
make first-run-view-parity:parity.jsonis identical on base and on this PR. The paragraph boxes match, and both sides show the same shell pixel diff in one 14x14 icon box, so that diff was there before this PR.make check-phone-chrome: the phone skeleton matches the live page.make static-checkpasses.@font-face.No test added. The defect is a rendered text-box size in a real browser with real font files. A happy-dom or build-time check cannot measure that, and a test that reads the CSS would only repeat the three values. The browser check above is the reliable gate, and the Lighthouse VM covers it after merge.
Risks
Low. The change is CSS only and touches the Geist Mono face only. Glyph positions stay the same. Text boxes of monospace text go back to their pre-#7625 height. Geist (sans) also changed its metrics in 1.8.1 (920/220/100 → 1005/295/0). I did not change it: it is preloaded, so the static view and the live app already paint it with the same font.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high