Skip to content

fix(web): bundle Source Serif 4 with Cyrillic coverage - #6089

Closed
roian6 wants to merge 1 commit into
nexu-io:mainfrom
roian6:fix-source-serif-6085
Closed

fix(web): bundle Source Serif 4 with Cyrillic coverage#6089
roian6 wants to merge 1 commit into
nexu-io:mainfrom
roian6:fix-source-serif-6085

Conversation

@roian6

@roian6 roian6 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Fixes #6085

Why

The web typography token already preferred Source Serif, but the web app did not ship that family. On machines without Source Serif Pro installed, Home and other serif headings therefore fell through to unrelated system serif fonts; Cyrillic users saw especially inconsistent metrics across operating systems.

This change adds the OFL-1.1 @fontsource-variable/source-serif-4@5.3.0 package to apps/web. The production build emits six self-hosted Unicode-range WOFF2 subsets. A Russian Home page requests the matching Latin and Cyrillic files from the local app origin; the Cyrillic-ext face is registered and remains lazily available for extended glyphs, with no font CDN or other external Source Serif request.

What users will see

Serif headings use the existing local Source Serif Pro preference when it is installed. Otherwise, Open Design now renders them with the same self-hosted Source Serif 4 Variable face, including Cyrillic and Cyrillic-ext coverage, instead of an OS-dependent fallback.

Surface area

  • UI — new page / dialog / panel / menu item / setting / empty state in apps/web or apps/desktop (including Electron menu bar)
  • Keyboard shortcut — new or changed
  • CLI / env var — new od subcommand or flag, new tools-dev / tools-pack / tools-pr flag, or new OD_* env var
  • API / contract — new /api/* endpoint, new SSE event, or changed shape in packages/contracts
  • Extension point — new entry under skills/, design-systems/, design-templates/, or craft/, or change to the skills protocol
  • i18n keys — added new translation keys (see TRANSLATIONS.md for the locale workflow)
  • New top-level dependency — adding any new entry to the root package.json (dependencies or devDependencies); workspace-package package.json files are out of scope. Include a paragraph on what we get vs. what bytes we ship (see CONTRIBUTING.md → Code style)
  • Default behavior change — changes what existing users experience without opting in (default model, default setting, file/SQLite schema, auto-network on startup, auto-install)
  • None — internal refactor, docs, tests, or translation update only

Screenshots

Captured from rebased current head 761feb67e0 using its production build.

Narrow — 375 × 812 — Russian Home

Russian Home at 375 by 812 on head 761feb67e0

Desktop — 1280 × 900 — Russian Home

Russian Home at 1280 by 900 on head 761feb67e0

The full Russian Source Serif heading renders without tofu, clipping, overlap, broken wrapping, or an obvious layout shift. The Cyrillic face is loaded, the Cyrillic-ext face is registered for on-demand glyph loading, and all observed Source Serif WOFF2 requests are served from the local app origin.

Bug fix verification

  • Test path: apps/web/tests/source-serif-font.test.ts
  • Yes — the three assertions failed on main because the dependency, global import, and matching variable-family token were absent, then all three passed after the source change.
  • The test resolves the imported Fontsource CSS and checks its actual Cyrillic/Cyrillic-ext variable WOFF2 @font-face declarations rather than inferring coverage from the package name.

Validation

All current-head gates below ran against rebased commit 761feb67e0:

  • pnpm --filter @open-design/web exec vitest run -c vitest.config.ts --maxWorkers=2 tests/source-serif-font.test.ts — 3 passed
  • pnpm guard — passed
  • pnpm typecheck — passed
  • pnpm --filter @open-design/web build — passed; emitted all six local Source Serif 4 WOFF2 subsets
  • Repository Nix hash generator in the pinned Nix image — passed; updated only webHash for the rebased lockfile
  • nix flake check --print-build-logs --keep-going — passed on an archive of the exact current head; all checks passed
  • Browser QA on the production build at 375×812 and 1280×900 — Cyrillic face loaded, Cyrillic-ext face registered, local WOFF2 requests only, and no external Source Serif requests

@lefarcen

Copy link
Copy Markdown
Contributor

Hey @roian6 — thanks for jumping on #6085 so quickly. The self-hosted Source Serif 4 + Cyrillic coverage direction looks right for this bug, and I’ve routed it through the normal review/validation flow for a visible typography change.

@lefarcen lefarcen added the size/S PR changes 20-100 lines label Jul 26, 2026
@lefarcen
lefarcen requested a review from mrcfps July 26, 2026 16:32
@lefarcen lefarcen added risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps type/bugfix Bug fix needs-design-review Awaiting design review (external UI change); cleared by the Odcrew App applying design-approved needs-validation Runtime change detected; needs human or /explore agent validation. labels Jul 26, 2026
@lefarcen

Copy link
Copy Markdown
Contributor

🧪 This PR has changes that need a manual QA pass before merge — please hold off self-merging for now; we’ll loop QA in once it’s merge-ready (and design/product have signed off, where applicable).

@mrcfps mrcfps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roian6 Thanks for the careful self-hosted font fix—the implementation and Cyrillic regression coverage are solid, and the focused font test, repository guard, workspace typecheck, and production web build all pass locally. I found one merge-blocking packaging issue: the dependency update leaves the generated Nix pnpm hash stale, which is exactly why the live Nix check fails. Please refresh that artifact as noted inline; after that, this should be in good shape. Thanks for making the offline typography behavior deterministic across platforms. 🙏

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

Comment thread apps/web/package.json
"dependencies": {
"@anthropic-ai/sdk": "0.32.1",
"@excalidraw/excalidraw": "0.18.1",
"@fontsource-variable/source-serif-4": "5.3.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking — refresh the Nix pnpm dependency hash for this dependency change. Adding this package also changes pnpm-lock.yaml, but this branch leaves nix/pnpm-deps.nix unchanged. The live nix flake check job now fails with a fixed-output hash mismatch for open-design-web-pnpm-deps, so the Nix web package cannot build; this is direct evidence from the changed dependency graph rather than an unrelated CI failure. The repository validation contract requires Nix-affecting lockfile changes to regenerate that artifact. Please run pnpm nix:update-hash, commit the resulting nix/pnpm-deps.nix update, and rerun nix flake check --print-build-logs --keep-going.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in dd27707c6: regenerated nix/pnpm-deps.nix with the repository hash updater, which updated only webHash.

Validation on the amended source:

  • nix flake check --print-build-logs --keep-going — all checks passed
  • focused Source Serif regression — 3/3 passed
  • pnpm guard — passed
  • pnpm typecheck — passed

@lefarcen

Copy link
Copy Markdown
Contributor

Hey @roian6 — the current blocker is the packaging/Nix follow-up from @mrcfps: this dependency update also needs the generated nix/pnpm-deps.nix artifact refreshed, which is why nix flake check is failing right now. Once that hash update is committed and the Nix check goes green, this should be back on track.

💡 To drive this PR to merge hands-free, paste this to your AI coding agent (Claude Code / Codex / opencode / Cursor …):
Take over nexu-io/open-design#6089 until it merges — read https://raw.githubusercontent.com/nexu-io/looper/main/skills/pr-takeover/SKILL.md and follow it.

@roian6
roian6 force-pushed the fix-source-serif-6085 branch from cc87740 to dd27707 Compare July 26, 2026 17:37

@mrcfps mrcfps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roian6 Thank you for the careful typography fix. I verified that the web app now bundles the variable Source Serif face locally, the serif token selects its exact family name, and the imported CSS includes Unicode-ranged Cyrillic and Cyrillic-ext WOFF2 faces without an external font request. The focused regression test passes 3/3, and the repository guard, full workspace typecheck, production web build, and current Nix flake CI check all pass on this head. The refreshed Nix dependency hash also resolves the prior review blocker. Nicely done—this makes the intended serif rendering deterministic and offline-safe across platforms. 🙏

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@open-design-crew

Copy link
Copy Markdown
Contributor

Thanks for the PR — appreciate the careful self-hosted font fix! 🙏 To verify the visible typography change, could you add labelled screenshots from the current head of the Russian Home page at one narrow viewport such as 375px and one desktop viewport such as 1280px? Please include a Source Serif heading with Cyrillic text and enough adjacent UI to show that the glyphs render without fallback tofu, clipping, or a layout shift. A before/after pair is welcome but not required. Once these visuals cover the changed Cyrillic rendering, this should be a quick approve.

@roian6

roian6 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — I captured the requested Russian Home visuals from the current PR head dd27707c6 using its production build.

Narrow — 375 × 812 — Russian Home

Russian Home at 375 by 812 on head dd27707c6

Desktop — 1280 × 900 — Russian Home

Russian Home at 1280 by 900 on head dd27707c6

The visible heading is “Что вы спроектируете сегодня вместе со своим агентом?”. In both captures, the computed serif token includes Source Serif 4 Variable, document.fonts.check(...) passes, and the Cyrillic/Cyrillic-ext faces report loaded. The matching WOFF2 requests are all served from the local app origin, with no external Source Serif request. I also checked the rendered images for missing-glyph tofu, heading clipping, overlap, and an obvious layout shift; none were present.

@lefarcen

Copy link
Copy Markdown
Contributor

Thanks @roian6 — these captures are exactly the kind of evidence the remaining design sign-off needed. The screenshots plus your notes on the local WOFF2 loads / no tofu / no clipping make the current head much easier to verify.

@open-design-crew open-design-crew Bot added the design-approved Design signed off (must be applied by the Odcrew App open-design-crew[bot]) label Jul 29, 2026
@lefarcen lefarcen removed the needs-design-review Awaiting design review (external UI change); cleared by the Odcrew App applying design-approved label Jul 29, 2026
@lefarcen
lefarcen requested a review from AmyShang-alt July 29, 2026 07:48
@AmyShang-alt AmyShang-alt added validated Runtime change validated (via /explore Pass or manual QA). and removed needs-validation Runtime change detected; needs human or /explore agent validation. labels Jul 29, 2026

@AmyShang-alt AmyShang-alt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA 通过(验证提交:dd27707c)。

已在隔离副本完成:

  • Source Serif 专项回归测试:3/3 通过
  • @open-design/web production build:通过
  • 构建产物包含 6 个 Source Serif 4 WOFF2 子集;Cyrillic 与 Cyrillic-ext 实际静态路径均返回 200(font/woff2)
  • Home 标题的计算字体栈包含 Source Serif 4 Variable,且浏览器从本地源加载 Source Serif WOFF2,无外部字体 CDN 请求。

范围为 Web 字体与静态资源,不需要 Electron 验证;无埋点变动。已将 needs-validation 替换为 validated。

备注:当前 Detect validation scopes 和 Static gate 仍处于 queued,合并仍应以这些必需 CI 成功为准。

@lefarcen
lefarcen marked this pull request as draft August 1, 2026 08:21
@roian6
roian6 force-pushed the fix-source-serif-6085 branch from dd27707 to 761feb6 Compare August 1, 2026 09:57
@roian6

roian6 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Resumed after the inactivity handoff and rebased the existing single commit onto current main without broadening the patch. The new head is 761feb67e0; it still changes only the original six Source Serif/lock/generated-hash paths.

The nix/pnpm-deps.nix conflict was resolved by running the repository hash generator against the rebased lockfile in the pinned Nix image. Current-head gates are green: focused font test (3/3), pnpm guard, pnpm typecheck, production web build, git diff --check, and nix flake check --print-build-logs --keep-going (all checks passed).

I also recaptured the requested Russian Home visuals from this exact head:

The full Cyrillic heading renders without tofu, clipping, overlap, or broken wrapping. The Cyrillic face loads locally, the Cyrillic-ext face is registered for on-demand glyph loading, and there are no external Source Serif requests. I’ve refreshed the PR body with the same current-head evidence and am returning the PR to Ready for review. Since the head changed to resolve the real merge conflict, the repository may require a fresh approval and CI run.

@roian6
roian6 marked this pull request as ready for review August 1, 2026 09:59
@lefarcen

lefarcen commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Thanks @roian6 — noted the rebase onto current main and the refreshed screenshots / Nix hash on 761feb67e0. I’ve re-requested review for this head; once the required checks finish on the rebased commit, this should be back in the normal merge path.

@lefarcen
lefarcen requested a review from mrcfps August 1, 2026 10:06

@mrcfps mrcfps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roian6 Thank you for carefully carrying this typography fix through the rebase. I verified on the current head that the web app imports the pinned self-hosted Source Serif 4 variable face, keeps local Source Serif Pro first in the stack, and emits six same-origin, Unicode-ranged WOFF2 subsets including Cyrillic and Cyrillic-ext. The focused regression passes 3/3, as do pnpm guard, the full workspace pnpm typecheck, and the production web build; the current-head Nix and landing-page checks also pass, so the regenerated web hash resolves the earlier blocker. The refreshed Russian captures show intact headings at both narrow and desktop viewports. Nicely done—this makes the intended serif rendering deterministic and offline-safe across platforms. 🙏

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

@roian6 friendly reminder: this PR appears to be waiting on author action (merge conflict, failing CI) and has had no human activity for more than 3 days.

When you have a chance, please reply here or push an update. To keep the queue manageable, PRs with no human activity for more than 5 days may be closed automatically, but they can be reopened when work resumes.

@roian6

roian6 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Holding this head unchanged after inspecting the current main conflict. PR #6142 intentionally replaced the old Source Serif typography contract with a self-hosted Albert Sans direction, so mechanically rebasing this patch would reverse part of the newly merged #5517 redesign rather than merely repair drift.

I documented the exact conflict and the separate Cyrillic-coverage concern on #6085: #6085 (comment)

No force-push was made; the reviewed head remains 761feb67e08ce61d3fa2dced5748778ec4f8a84b. Please advise whether this PR should be closed as superseded or whether the issue should be reframed around a Cyrillic-capable sans face consistent with the new design.

@lefarcen

lefarcen commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

@roian6 — your read is correct. PR #6142 (merged 2026-08-05) replaced the Source Serif contract in apps/web/src/styles/tokens.css with Albert Sans, so there's no clean rebase path here — applying this patch would roll back the accepted typography direction rather than resolve a routine conflict.

Flagging this to the team to formally close the PR. The Cyrillic coverage concern remains valid and is tracked on #6085 — a fresh approach shaped around the current Albert Sans stack is the right next move once the team confirms direction. Thanks for the careful conflict analysis and for keeping the reviewed head frozen.

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

@roian6 friendly reminder: this PR appears to be waiting on author action (outstanding trusted reviewer or maintainer feedback, merge conflict, failing CI) and has had no human activity for more than 3 days.

When you have a chance, please reply here or push an update. To keep the queue manageable, PRs with no human activity for more than 5 days may be closed automatically, but they can be reopened when work resumes.

@github-actions

Copy link
Copy Markdown
Contributor

Closing this PR for now because it appears to be waiting on author action (outstanding trusted reviewer or maintainer feedback, merge conflict, failing CI) and has had no human activity for more than 5 days.

This is only a queue-management step, not a rejection of the work. If you would like to continue, please leave a comment or push an update and reopen the PR when ready.

@github-actions github-actions Bot closed this Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

design-approved Design signed off (must be applied by the Odcrew App open-design-crew[bot]) risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps size/S PR changes 20-100 lines type/bugfix Bug fix validated Runtime change validated (via /explore Pass or manual QA).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Home headings fall back to random system serif — bundle Source Serif 4 (with Cyrillic) in the web app

4 participants